From 576b360cce40ffe79119b6816561d25294469d1f Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Mon, 15 Apr 2024 08:51:00 +0200 Subject: [PATCH] Removed leftover System.Text.Json dependencies (#16040) --- .../Serialization/IJsonSerializer.cs | 12 +++++ .../ImageCropperPropertyValueEditor.cs | 23 ++++---- .../MediaPicker3PropertyEditor.cs | 11 +--- .../PropertyEditors/SliderPropertyEditor.cs | 14 ++--- ...emTextConfigurationEditorJsonSerializer.cs | 8 +-- .../Serialization/SystemTextJsonSerializer.cs | 9 +--- .../SystemTextJsonSerializerBase.cs | 35 ++++++++++++ .../SystemTextJsonSerializerTests.cs | 53 +++++++++++++++++++ 8 files changed, 121 insertions(+), 44 deletions(-) create mode 100644 src/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializerBase.cs create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializerTests.cs diff --git a/src/Umbraco.Core/Serialization/IJsonSerializer.cs b/src/Umbraco.Core/Serialization/IJsonSerializer.cs index ba05652262..27bd956f41 100644 --- a/src/Umbraco.Core/Serialization/IJsonSerializer.cs +++ b/src/Umbraco.Core/Serialization/IJsonSerializer.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace Umbraco.Cms.Core.Serialization; /// @@ -24,4 +26,14 @@ public interface IJsonSerializer /// A representation of the JSON value. /// T? Deserialize(string input); + + /// + /// Attempts to parse an object that represents a JSON structure - i.e. a JSON object or a JSON array - to a strongly typed representation. + /// + /// The target type of the JSON value. + /// The object input to parse. + /// The parsed result, or null if the parsing fails. + /// True if the parsing results in a non-null value, false otherwise. + bool TryDeserialize(object input, [NotNullWhen(true)] out T? value) + where T : class; } diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs index 77376361e7..ef3b9883a0 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs @@ -222,20 +222,21 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v private ImageCropperValue? TryParseImageCropperValue(object? editorValue) { - // FIXME: consider creating an object deserialization method on IJsonSerializer instead of relying on deserializing serialized JSON here (and likely other places as well) - if (editorValue is JsonObject jsonObject) + try { - try + if (editorValue is null || + _jsonSerializer.TryDeserialize(editorValue, out ImageCropperValue? imageCropperValue) is false) { - ImageCropperValue? imageCropperValue = _jsonSerializer.Deserialize(jsonObject.ToJsonString()); - imageCropperValue?.Prune(); - return imageCropperValue; - } - catch (Exception ex) - { - // For some reason the value is invalid - log error and continue as if no value was saved - _logger.LogWarning(ex, "Could not parse editor value to an ImageCropperValue object."); + return null; } + + imageCropperValue.Prune(); + return imageCropperValue; + } + catch (Exception ex) + { + // For some reason the value is invalid - log error and continue as if no value was saved + _logger.LogWarning(ex, "Could not parse editor value to an ImageCropperValue object."); } return null; diff --git a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs index dfb70ba591..3f4b5f3cf6 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs @@ -1,4 +1,3 @@ -using System.Text.Json.Nodes; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -112,14 +111,8 @@ public class MediaPicker3PropertyEditor : DataEditor public override object? FromEditor(ContentPropertyData editorValue, object? currentValue) { - // FIXME: consider creating an object deserialization method on IJsonSerializer instead of relying on deserializing serialized JSON here (and likely other places as well) - if (editorValue.Value is not JsonArray jsonArray) - { - return base.FromEditor(editorValue, currentValue); - } - - List? mediaWithCropsDtos = _jsonSerializer.Deserialize>(jsonArray.ToJsonString()); - if (mediaWithCropsDtos is null) + if (editorValue.Value is null || + _jsonSerializer.TryDeserialize(editorValue.Value, out List? mediaWithCropsDtos) is false) { return base.FromEditor(editorValue, currentValue); } diff --git a/src/Umbraco.Infrastructure/PropertyEditors/SliderPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/SliderPropertyEditor.cs index cd675ff73e..8321915677 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/SliderPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/SliderPropertyEditor.cs @@ -1,7 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System.Text.Json.Nodes; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Editors; @@ -73,16 +72,9 @@ public class SliderPropertyEditor : DataEditor } public override object? FromEditor(ContentPropertyData editorValue, object? currentValue) - { - // FIXME: do not rely explicitly on concrete JSON implementation here - consider creating an object deserialization method on IJsonSerializer (see also MultiUrlPickerValueEditor) - if (editorValue.Value is not JsonNode jsonNode) - { - return null; - } - - SliderRange? range = _jsonSerializer.Deserialize(jsonNode.ToJsonString()); - return range?.ToString(); - } + => editorValue.Value is not null && _jsonSerializer.TryDeserialize(editorValue.Value, out SliderRange? sliderRange) + ? sliderRange.ToString() + : null; internal class SliderRange { diff --git a/src/Umbraco.Infrastructure/Serialization/SystemTextConfigurationEditorJsonSerializer.cs b/src/Umbraco.Infrastructure/Serialization/SystemTextConfigurationEditorJsonSerializer.cs index 944d769be1..02c667f540 100644 --- a/src/Umbraco.Infrastructure/Serialization/SystemTextConfigurationEditorJsonSerializer.cs +++ b/src/Umbraco.Infrastructure/Serialization/SystemTextConfigurationEditorJsonSerializer.cs @@ -5,7 +5,7 @@ using Umbraco.Cms.Core.Serialization; namespace Umbraco.Cms.Infrastructure.Serialization; /// -public sealed class SystemTextConfigurationEditorJsonSerializer : IConfigurationEditorJsonSerializer +public sealed class SystemTextConfigurationEditorJsonSerializer : SystemTextJsonSerializerBase, IConfigurationEditorJsonSerializer { private readonly JsonSerializerOptions _jsonSerializerOptions; @@ -30,9 +30,5 @@ public sealed class SystemTextConfigurationEditorJsonSerializer : IConfiguration } }; - /// - public string Serialize(object? input) => JsonSerializer.Serialize(input, _jsonSerializerOptions); - - /// - public T? Deserialize(string input) => JsonSerializer.Deserialize(input, _jsonSerializerOptions); + protected override JsonSerializerOptions JsonSerializerOptions => _jsonSerializerOptions; } diff --git a/src/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializer.cs b/src/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializer.cs index c713783e0d..033256f7a9 100644 --- a/src/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializer.cs +++ b/src/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializer.cs @@ -1,11 +1,10 @@ using System.Text.Json; using System.Text.Json.Serialization; -using Umbraco.Cms.Core.Serialization; namespace Umbraco.Cms.Infrastructure.Serialization; /// -public sealed class SystemTextJsonSerializer : IJsonSerializer +public sealed class SystemTextJsonSerializer : SystemTextJsonSerializerBase { private readonly JsonSerializerOptions _jsonSerializerOptions; @@ -26,9 +25,5 @@ public sealed class SystemTextJsonSerializer : IJsonSerializer } }; - /// - public string Serialize(object? input) => JsonSerializer.Serialize(input, _jsonSerializerOptions); - - /// - public T? Deserialize(string input) => JsonSerializer.Deserialize(input, _jsonSerializerOptions); + protected override JsonSerializerOptions JsonSerializerOptions => _jsonSerializerOptions; } diff --git a/src/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializerBase.cs b/src/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializerBase.cs new file mode 100644 index 0000000000..cfffb32a99 --- /dev/null +++ b/src/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializerBase.cs @@ -0,0 +1,35 @@ +using System.Diagnostics.CodeAnalysis; +using System.Text.Json; +using System.Text.Json.Nodes; +using Umbraco.Cms.Core.Serialization; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Infrastructure.Serialization; + +public abstract class SystemTextJsonSerializerBase : IJsonSerializer +{ + protected abstract JsonSerializerOptions JsonSerializerOptions { get; } + + /// + public string Serialize(object? input) => JsonSerializer.Serialize(input, JsonSerializerOptions); + + /// + public T? Deserialize(string input) => JsonSerializer.Deserialize(input, JsonSerializerOptions); + + /// + public bool TryDeserialize(object input, [NotNullWhen(true)] out T? value) + where T : class + { + var jsonString = input switch + { + JsonNode jsonNodeValue => jsonNodeValue.ToJsonString(), + string stringValue when stringValue.DetectIsJson() => stringValue, + _ => null + }; + + value = jsonString.IsNullOrWhiteSpace() + ? null + : Deserialize(jsonString); + return value != null; + } +} diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializerTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializerTests.cs new file mode 100644 index 0000000000..fff2509f8a --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Serialization/SystemTextJsonSerializerTests.cs @@ -0,0 +1,53 @@ +using System.Text.Json.Nodes; +using NUnit.Framework; +using Umbraco.Cms.Infrastructure.Serialization; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Serialization; + +[TestFixture] +public class SystemTextJsonSerializerTests +{ + [Test] + public void TryDeserialize_Can_Handle_JsonObject() + { + var json = JsonNode.Parse("{\"myProperty\":\"value\"}"); + var subject = new SystemTextJsonSerializer(); + Assert.IsTrue(subject.TryDeserialize(json!, out MyItem myItem)); + Assert.AreEqual("value", myItem.MyProperty); + } + + [Test] + public void TryDeserialize_Can_Handle_JsonArray() + { + var json = JsonNode.Parse("[{\"myProperty\":\"value1\"},{\"myProperty\":\"value2\"}]"); + var subject = new SystemTextJsonSerializer(); + Assert.IsTrue(subject.TryDeserialize(json!, out MyItem[] myItems)); + Assert.AreEqual(2, myItems.Length); + Assert.Multiple(() => + { + Assert.AreEqual("value1", myItems[0].MyProperty); + Assert.AreEqual("value2", myItems[1].MyProperty); + }); + } + + [Test] + public void TryDeserialize_Can_Handle_JsonString() + { + var json = "{\"myProperty\":\"value\"}"; + var subject = new SystemTextJsonSerializer(); + Assert.IsTrue(subject.TryDeserialize(json, out MyItem myItem)); + Assert.AreEqual("value", myItem.MyProperty); + } + + [Test] + public void TryDeserialize_Cannot_Handle_RandomString() + { + var subject = new SystemTextJsonSerializer(); + Assert.IsFalse(subject.TryDeserialize("something something", out _)); + } + + private class MyItem + { + public required string MyProperty { get; set; } + } +}