From 10c18e4162699017ff722330ff350db36e776eea Mon Sep 17 00:00:00 2001
From: Kenn Jacobsen <kja@umbraco.dk>
Date: Thu, 29 Feb 2024 14:38:32 +0100
Subject: [PATCH] Swap Newtonsoft.Json dependency for System.Text.Json in
 BlockEditorPropertyNotificationHandlerBase (#15790)

* Swap Newtonsoft.Json dependency for System.Text.Json in BlockEditorPropertyNotificationHandlerBase

* Review comments
---
 ...ckEditorPropertyNotificationHandlerBase.cs | 47 +++++------
 .../BlockEditorComponentTests.cs              | 84 +++++++++----------
 2 files changed, 62 insertions(+), 69 deletions(-)

diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyNotificationHandlerBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyNotificationHandlerBase.cs
index dccc6a7201..a2f5a3564e 100644
--- a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyNotificationHandlerBase.cs
+++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyNotificationHandlerBase.cs
@@ -1,6 +1,7 @@
-using System.Text.RegularExpressions;
+using System.Text.Json;
+using System.Text.Json.Nodes;
+using System.Text.RegularExpressions;
 using Microsoft.Extensions.Logging;
-using Newtonsoft.Json.Linq;
 using Umbraco.Cms.Core.Models.Blocks;
 using Umbraco.Extensions;
 
@@ -39,7 +40,7 @@ public abstract class BlockEditorPropertyNotificationHandlerBase<TBlockLayoutIte
             return rawJson;
         }
 
-        JObject? jObject = ParseObject(rawJson);
+        JsonObject? jObject = ParseObject(rawJson);
         if (jObject == null)
         {
             return rawJson;
@@ -71,26 +72,27 @@ public abstract class BlockEditorPropertyNotificationHandlerBase<TBlockLayoutIte
         return rawJson;
     }
 
-    private void TraverseProperty(JProperty property)
+    private void TraverseProperty(JsonNode property)
     {
-        if (property.Value is JArray jArray)
+        if (property is JsonArray jArray)
         {
-            foreach (JToken token in jArray)
+            foreach (JsonNode token in jArray.WhereNotNull())
             {
                 TraverseToken(token);
             }
         }
         else
         {
-            TraverseToken(property.Value);
+            TraverseToken(property);
         }
     }
 
-    private void TraverseToken(JToken token)
+    private void TraverseToken(JsonNode token)
     {
-        var obj = token as JObject;
-        if (obj == null && token is JValue { Value: string str })
+        var obj = token as JsonObject;
+        if (obj == null && token is JsonValue jsonValue && jsonValue.GetValueKind() is JsonValueKind.String)
         {
+            var str = jsonValue.GetValue<string>();
             obj = ParseObject(str);
         }
 
@@ -102,30 +104,27 @@ public abstract class BlockEditorPropertyNotificationHandlerBase<TBlockLayoutIte
         TraverseObject(obj);
     }
 
-    private void TraverseObject(JObject obj)
+    private void TraverseObject(JsonObject obj)
     {
-        var contentData = obj.SelectToken("$.contentData") as JArray;
-        var settingsData = obj.SelectToken("$.settingsData") as JArray;
-
         // we'll assume that the object is a data representation of a block based editor if it contains "contentData" and "settingsData".
-        if (contentData != null && settingsData != null)
+        if (obj["contentData"] is JsonArray contentData && obj["settingsData"] is JsonArray settingsData)
         {
             ParseUdis(contentData, settingsData);
             return;
         }
 
-        foreach (JProperty property in obj.Properties())
+        foreach (JsonNode property in obj.Select(n => n.Value).WhereNotNull())
         {
             TraverseProperty(property);
         }
     }
 
-    private void ParseUdis(JArray contentData, JArray settingsData)
+    private void ParseUdis(JsonArray contentData, JsonArray settingsData)
     {
         // grab all UDIs from the objects of contentData and settingsData
-        var udis = contentData.Select(c => c.SelectToken("$.udi"))
-            .Union(settingsData.Select(s => s.SelectToken("$.udi")))
-            .Select(udiToken => udiToken?.Value<string>()?.NullOrWhiteSpaceAsNull())
+        var udis = contentData.Select(c => c?["udi"])
+            .Union(settingsData.Select(s => s?["udi"]))
+            .Select(udiToken => udiToken?.GetValue<string>().NullOrWhiteSpaceAsNull())
             .ToArray();
 
         // the following is solely for avoiding functionality wise breakage. we should consider removing it eventually, but for the time being it's harmless.
@@ -139,16 +138,16 @@ public abstract class BlockEditorPropertyNotificationHandlerBase<TBlockLayoutIte
 
         _udisToReplace.AddRange(udis.WhereNotNull());
 
-        foreach (JObject item in contentData.Union(settingsData).OfType<JObject>())
+        foreach (JsonObject item in contentData.Union(settingsData).WhereNotNull().OfType<JsonObject>())
         {
-            foreach (JProperty property in item.Properties().Where(p => p.Name != "contentTypeKey" && p.Name != "udi"))
+            foreach (JsonNode property in item.Where(p => p.Key != "contentTypeKey" && p.Key != "udi").Select(p => p.Value).WhereNotNull())
             {
                 TraverseProperty(property);
             }
         }
     }
 
-    private JObject? ParseObject(string json)
+    private JsonObject? ParseObject(string json)
     {
         if (json.DetectIsJson() == false)
         {
@@ -157,7 +156,7 @@ public abstract class BlockEditorPropertyNotificationHandlerBase<TBlockLayoutIte
 
         try
         {
-            return JObject.Parse(json);
+            return JsonNode.Parse(json) as JsonObject;
         }
         catch (Exception)
         {
diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockEditorComponentTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockEditorComponentTests.cs
index 55c172afa7..607a39a7f0 100644
--- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockEditorComponentTests.cs
+++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockEditorComponentTests.cs
@@ -1,14 +1,14 @@
 // Copyright (c) Umbraco.
 // See LICENSE for more details.
 
-using System.Collections.Generic;
-using System.Linq;
 using Microsoft.Extensions.Logging;
 using Moq;
-using Newtonsoft.Json;
 using NUnit.Framework;
 using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.Models.Blocks;
 using Umbraco.Cms.Core.PropertyEditors;
+using Umbraco.Cms.Core.Serialization;
+using Umbraco.Cms.Infrastructure.Serialization;
 using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.PropertyEditors;
@@ -16,12 +16,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.PropertyEditors;
 [TestFixture]
 public class BlockEditorComponentTests
 {
-    private readonly JsonSerializerSettings _serializerSettings = new()
-    {
-        Formatting = Formatting.None,
-        NullValueHandling = NullValueHandling.Ignore,
-    };
-
     private const string ContentGuid1 = "036ce82586a64dfba2d523a99ed80f58";
     private const string ContentGuid2 = "48288c21a38a40ef82deb3eda90a58f6";
     private const string SettingsGuid1 = "ffd35c4e2eea4900abfa5611b67b2492";
@@ -29,6 +23,8 @@ public class BlockEditorComponentTests
     private const string SubContentGuid2 = "a062c06d6b0b44ac892b35d90309c7f8";
     private const string SubSettingsGuid1 = "4d998d980ffa4eee8afdc23c4abd6d29";
 
+    private readonly IJsonSerializer _jsonSerializer = new SystemTextJsonSerializer();
+
     [Test]
     public void Cannot_Have_Null_Udi()
     {
@@ -52,11 +48,11 @@ public class BlockEditorComponentTests
         var component = new BlockListPropertyNotificationHandler(Mock.Of<ILogger<BlockListPropertyNotificationHandler>>());
         var result = component.ReplaceBlockEditorUdis(json, GuidFactory);
 
+        Assert.AreEqual(3, guidMap.Count);
         var expected = ReplaceGuids(json, guidMap);
-        var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings);
-        var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings);
-        Console.WriteLine(expectedJson);
-        Console.WriteLine(resultJson);
+        var expectedJson = _jsonSerializer.Serialize( _jsonSerializer.Deserialize<BlockListValue>(expected));
+        var resultJson = _jsonSerializer.Serialize(_jsonSerializer.Deserialize<BlockListValue>(result));
+        Assert.IsNotEmpty(resultJson);
         Assert.AreEqual(expectedJson, resultJson);
     }
 
@@ -74,7 +70,7 @@ public class BlockEditorComponentTests
 
         // we need to ensure the escaped json is consistent with how it will be re-escaped after parsing
         // and this is how to do that, the result will also include quotes around it.
-        var innerJsonEscaped = JsonConvert.ToString(innerJson);
+        var innerJsonEscaped = Escape(innerJson);
 
         // get the json with the subFeatures as escaped
         var json = GetBlockListJson(innerJsonEscaped);
@@ -83,12 +79,12 @@ public class BlockEditorComponentTests
         var result = component.ReplaceBlockEditorUdis(json, GuidFactory);
 
         // the expected result is that the subFeatures data remains escaped
+        Assert.AreEqual(6, guidMap.Count);
         var expected = ReplaceGuids(GetBlockListJson(innerJsonEscaped), guidMap);
 
-        var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings);
-        var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings);
-        Console.WriteLine(expectedJson);
-        Console.WriteLine(resultJson);
+        var expectedJson = _jsonSerializer.Serialize( _jsonSerializer.Deserialize<BlockListValue>(expected));
+        var resultJson = _jsonSerializer.Serialize(_jsonSerializer.Deserialize<BlockListValue>(result));
+        Assert.IsNotEmpty(resultJson);
         Assert.AreEqual(expectedJson, resultJson);
     }
 
@@ -111,11 +107,11 @@ public class BlockEditorComponentTests
         var component = new BlockListPropertyNotificationHandler(Mock.Of<ILogger<BlockListPropertyNotificationHandler>>());
         var result = component.ReplaceBlockEditorUdis(json, GuidFactory);
 
+        Assert.AreEqual(6, guidMap.Count);
         var expected = ReplaceGuids(GetBlockListJson(innerJson), guidMap);
-        var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings);
-        var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings);
-        Console.WriteLine(expectedJson);
-        Console.WriteLine(resultJson);
+        var expectedJson = _jsonSerializer.Serialize( _jsonSerializer.Deserialize<BlockListValue>(expected));
+        var resultJson = _jsonSerializer.Serialize(_jsonSerializer.Deserialize<BlockListValue>(result));
+        Assert.IsNotEmpty(resultJson);
         Assert.AreEqual(expectedJson, resultJson);
     }
 
@@ -133,7 +129,7 @@ public class BlockEditorComponentTests
 
         // we need to ensure the escaped json is consistent with how it will be re-escaped after parsing
         // and this is how to do that, the result will also include quotes around it.
-        var innerJsonEscaped = JsonConvert.ToString(innerJson);
+        var innerJsonEscaped = Escape(innerJson);
 
         // Complex editor such as the grid
         var complexEditorJsonEscaped = GetGridJson(innerJsonEscaped);
@@ -144,13 +140,12 @@ public class BlockEditorComponentTests
         var result = component.ReplaceBlockEditorUdis(json, GuidFactory);
 
         // the expected result is that the subFeatures remains escaped
-        Assert.True(guidMap.Any());
+        Assert.AreEqual(6, guidMap.Count);
         var expected = ReplaceGuids(GetBlockListJson(GetGridJson(innerJsonEscaped)), guidMap);
 
-        var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings);
-        var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings);
-        Console.WriteLine(expectedJson);
-        Console.WriteLine(resultJson);
+        var expectedJson = _jsonSerializer.Serialize( _jsonSerializer.Deserialize<BlockListValue>(expected));
+        var resultJson = _jsonSerializer.Serialize(_jsonSerializer.Deserialize<BlockListValue>(result));
+        Assert.IsNotEmpty(resultJson);
         Assert.AreEqual(expectedJson, resultJson);
     }
 
@@ -168,7 +163,7 @@ public class BlockEditorComponentTests
 
         // we need to ensure the escaped json is consistent with how it will be re-escaped after parsing
         // and this is how to do that, the result will also include quotes around it.
-        var innerJsonEscaped = JsonConvert.ToString(innerJson);
+        var innerJsonEscaped = Escape(innerJson);
 
         var json = GetBlockGridJson(innerJsonEscaped);
 
@@ -176,13 +171,12 @@ public class BlockEditorComponentTests
         var result = component.ReplaceBlockEditorUdis(json, GuidFactory);
 
         // the expected result is that the subFeatures remains escaped
-        Assert.True(guidMap.Any());
+        Assert.AreEqual(13, guidMap.Count);
         var expected = ReplaceGuids(GetBlockGridJson(innerJsonEscaped), guidMap);
 
-        var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings);
-        var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings);
-        Console.WriteLine(expectedJson);
-        Console.WriteLine(resultJson);
+        var expectedJson = _jsonSerializer.Serialize( _jsonSerializer.Deserialize<BlockGridValue>(expected));
+        var resultJson = _jsonSerializer.Serialize(_jsonSerializer.Deserialize<BlockGridValue>(result));
+        Assert.IsNotEmpty(resultJson);
         Assert.AreEqual(expectedJson, resultJson);
     }
 
@@ -204,13 +198,12 @@ public class BlockEditorComponentTests
         var result = component.ReplaceBlockEditorUdis(json, GuidFactory);
 
         // the expected result is that the subFeatures remains unescaped
-        Assert.True(guidMap.Any());
+        Assert.AreEqual(13, guidMap.Count);
         var expected = ReplaceGuids(GetBlockGridJson(innerJson), guidMap);
 
-        var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings);
-        var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings);
-        Console.WriteLine(expectedJson);
-        Console.WriteLine(resultJson);
+        var expectedJson = _jsonSerializer.Serialize( _jsonSerializer.Deserialize<BlockGridValue>(expected));
+        var resultJson = _jsonSerializer.Serialize(_jsonSerializer.Deserialize<BlockGridValue>(result));
+        Assert.IsNotEmpty(resultJson);
         Assert.AreEqual(expectedJson, resultJson);
     }
 
@@ -238,13 +231,12 @@ public class BlockEditorComponentTests
         var result = component.ReplaceBlockEditorUdis(json, GuidFactory);
 
         // the expected result is that the subFeatures remains unaltered - the UDIs within should still exist
-        Assert.True(guidMap.Any());
+        Assert.AreEqual(10, guidMap.Count);
         var expected = ReplaceGuids(GetBlockGridJson(innerJson), guidMap);
 
-        var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings);
-        var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings);
-        Console.WriteLine(expectedJson);
-        Console.WriteLine(resultJson);
+        var expectedJson = _jsonSerializer.Serialize( _jsonSerializer.Deserialize<BlockGridValue>(expected));
+        var resultJson = _jsonSerializer.Serialize(_jsonSerializer.Deserialize<BlockGridValue>(result));
+        Assert.IsNotEmpty(resultJson);
         Assert.AreEqual(expectedJson, resultJson);
 
         Assert.True(result.Contains("umb://element/eb459ab17259495b90a3d2f6bb299826"));
@@ -294,7 +286,7 @@ public class BlockEditorComponentTests
             ""udi"": ""umb://element/" + settingsGuid1 + @""",
             ""featureName"": ""Setting 1"",
             ""featureDetails"": ""Setting 2""
-        },
+        }
     ]
 }";
 
@@ -471,4 +463,6 @@ public class BlockEditorComponentTests
 
         return json;
     }
+
+    private string Escape(string json) => $"\"{System.Web.HttpUtility.JavaScriptStringEncode(json)}\"";
 }