Swap Newtonsoft.Json dependency for System.Text.Json in BlockEditorPropertyNotificationHandlerBase (#15790)

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

* Review comments
This commit is contained in:
Kenn Jacobsen
2024-02-29 14:38:32 +01:00
committed by GitHub
parent 7d1feb4ad5
commit 10c18e4162
2 changed files with 62 additions and 69 deletions

View File

@@ -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)
{

View File

@@ -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)}\"";
}