From 1b8ce54e49cbd52f6ee771fcd895cdd48b680753 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Tue, 7 Mar 2023 14:31:27 +0100 Subject: [PATCH] Remove Json.NET dependencies for more property editors (#13912) * Make the Tags property editor JSON serialization agnostic * Make the Multiple Value editor JSON serializer agnostic * Make Media Picker 3 value converter work with System.Text.Json instead of Json.NET * Make Image Cropper value converter work with System.Text.Json instead of Json.NET * Remove Json.NET dependencies from ImageCropperTemplateExtensions * Update src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> * Update src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> * Update src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> * Update src/Umbraco.Infrastructure/PropertyEditors/TagsPropertyEditor.cs Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> * Update src/Umbraco.Infrastructure/PropertyEditors/TagsPropertyEditor.cs Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> * Update src/Umbraco.Infrastructure/PropertyEditors/TagsPropertyEditor.cs Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --------- Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --- .../ImageCropperPropertyValueEditor.cs | 60 ++++---- .../MediaPicker3PropertyEditor.cs | 138 +++++++++--------- .../PropertyEditors/MultipleValueEditor.cs | 22 +-- .../PropertyEditors/TagsPropertyEditor.cs | 88 ++++++----- .../ValueConverters/ImageCropperValue.cs | 83 ++--------- .../ImageCropperValueTypeConverter.cs | 40 ----- .../ImageCropperTemplateExtensions.cs | 15 +- .../Umbraco.Web.Common/ImageCropperTest.cs | 57 +++++--- 8 files changed, 218 insertions(+), 285 deletions(-) delete mode 100644 src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValueTypeConverter.cs diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs index d985f26a1d..c668e6c3fb 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs @@ -1,10 +1,9 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using System.Text.Json.Nodes; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -26,6 +25,7 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v private readonly IDataTypeService _dataTypeService; private readonly ILogger _logger; private readonly MediaFileManager _mediaFileManager; + private readonly IJsonSerializer _jsonSerializer; private ContentSettings _contentSettings; public ImageCropperPropertyValueEditor( @@ -42,6 +42,7 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v { _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _mediaFileManager = mediaFileSystem ?? throw new ArgumentNullException(nameof(mediaFileSystem)); + _jsonSerializer = jsonSerializer; _contentSettings = contentSettings.CurrentValue; _dataTypeService = dataTypeService; contentSettings.OnChange(x => _contentSettings = x); @@ -62,7 +63,7 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v ImageCropperValue? value; try { - value = JsonConvert.DeserializeObject(val.ToString()!); + value = _jsonSerializer.Deserialize(val.ToString()!); } catch { @@ -97,17 +98,16 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v var currentPath = string.Empty; try { - var svalue = currentValue as string; - JObject? currentJson = string.IsNullOrWhiteSpace(svalue) ? null : JObject.Parse(svalue); - if (currentJson != null && currentJson.TryGetValue("src", out JToken? src)) + if (currentValue is string currentStringValue) { - currentPath = src.Value(); + ImageCropperValue? currentImageCropperValue = _jsonSerializer.Deserialize(currentStringValue); + currentPath = currentImageCropperValue?.Src; } } catch (Exception ex) { // For some reason the value is invalid so continue as if there was no value there - _logger.LogWarning(ex, "Could not parse current db value to a JObject."); + _logger.LogWarning(ex, "Could not parse current db value to an ImageCropperValue object."); } if (string.IsNullOrWhiteSpace(currentPath) == false) @@ -115,23 +115,21 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v currentPath = _mediaFileManager.FileSystem.GetRelativePath(currentPath); } - // Get the new JSON and file path - var editorFile = string.Empty; - var editorJson = (JObject?)editorValue.Value; - if (editorJson is not null) - { - // Populate current file - if (editorJson["src"] != null) - { - editorFile = editorJson["src"]?.Value(); - } + ImageCropperValue? editorImageCropperValue = null; - // Clean up redundant/default data - ImageCropperValue.Prune(editorJson); - } - else + // 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 JsonObject jsonObject) { - editorJson = null; + try + { + editorImageCropperValue = _jsonSerializer.Deserialize(jsonObject.ToJsonString()); + editorImageCropperValue?.Prune(); + } + 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."); + } } // ensure we have the required guids @@ -163,17 +161,17 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v // if editorFile is empty then either there was nothing to begin with, // or it has been cleared and we need to remove the file - else the // value is unchanged. - if (string.IsNullOrWhiteSpace(editorFile) && string.IsNullOrWhiteSpace(currentPath) == false) + if (string.IsNullOrWhiteSpace(editorImageCropperValue?.Src) && string.IsNullOrWhiteSpace(currentPath) is false) { _mediaFileManager.FileSystem.DeleteFile(currentPath); return null; // clear } - return editorJson?.ToString(Formatting.None); // unchanged + return _jsonSerializer.Serialize(editorImageCropperValue); // unchanged } // process the file - var filepath = editorJson == null ? null : ProcessFile(file, cuid, puid); + var filepath = editorImageCropperValue == null ? null : ProcessFile(file, cuid, puid); // remove all temp files foreach (ContentPropertyFile f in uploads) @@ -188,13 +186,13 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v } // update json and return - if (editorJson == null) + if (editorImageCropperValue == null) { return null; } - editorJson["src"] = filepath == null ? string.Empty : _mediaFileManager.FileSystem.GetUrl(filepath); - return editorJson.ToString(Formatting.None); + editorImageCropperValue.Src = filepath is null ? string.Empty : _mediaFileManager.FileSystem.GetUrl(filepath); + return _jsonSerializer.Serialize(editorImageCropperValue); } public override string ConvertDbToString(IPropertyType propertyType, object? value) @@ -216,9 +214,7 @@ internal class ImageCropperPropertyValueEditor : DataValueEditor // TODO: core v ?.ConfigurationAs(); ImageCropperConfiguration.Crop[] crops = configuration?.Crops ?? Array.Empty(); - return JsonConvert.SerializeObject( - new { src = val, crops }, - new JsonSerializerSettings { Formatting = Formatting.None, NullValueHandling = NullValueHandling.Ignore }); + return _jsonSerializer.Serialize(new { src = val, crops }); } private string? ProcessFile(ContentPropertyFile file, Guid cuid, Guid puid) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs index 60dd2ce672..b4ab07d8b0 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs @@ -1,8 +1,5 @@ -using System.Runtime.Serialization; +using System.Text.Json.Nodes; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -126,23 +123,29 @@ public class MediaPicker3PropertyEditor : DataEditor public override object? FromEditor(ContentPropertyData editorValue, object? currentValue) { - if (editorValue.Value is JArray dtos) + // 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) { - if (editorValue.DataTypeConfiguration is MediaPicker3Configuration configuration) - { - dtos = PersistTempMedia(dtos, configuration); - } - - // Clean up redundant/default data - foreach (JObject? dto in dtos.Values()) - { - MediaWithCropsDto.Prune(dto); - } - - return dtos.ToString(Formatting.None); + return base.FromEditor(editorValue, currentValue); } - return base.FromEditor(editorValue, currentValue); + List? mediaWithCropsDtos = _jsonSerializer.Deserialize>(jsonArray.ToJsonString()); + if (mediaWithCropsDtos is null) + { + return base.FromEditor(editorValue, currentValue); + } + + if (editorValue.DataTypeConfiguration is MediaPicker3Configuration configuration) + { + // FIXME: handle temp files here once we implement file uploads (see old implementation "PersistTempMedia" in the commented out code below) + } + + foreach (MediaWithCropsDto mediaWithCropsDto in mediaWithCropsDtos) + { + mediaWithCropsDto.Prune(); + } + + return _jsonSerializer.Serialize(mediaWithCropsDtos); } internal static IEnumerable Deserialize(IJsonSerializer jsonSerializer, object? value) @@ -185,76 +188,77 @@ public class MediaPicker3PropertyEditor : DataEditor } } - private JArray PersistTempMedia(JArray jArray, MediaPicker3Configuration mediaPicker3Configuration) - { - var result = new JArray(); - foreach (JObject? dto in jArray.Values()) - { - if (dto is null) - { - continue; - } - - if (!dto.TryGetValue("tmpLocation", out JToken? temporaryLocation)) - { - // If it does not have a temporary path, it can be an already saved image or not-yet uploaded temp-image, check for media-key - if (dto.TryGetValue("mediaKey", out _)) - { - result.Add(dto); - } - - continue; - } - - var temporaryLocationString = temporaryLocation.Value(); - if (temporaryLocationString is null) - { - continue; - } - - GuidUdi? startNodeGuid = mediaPicker3Configuration.StartNodeId as GuidUdi ?? null; - JToken? mediaTypeAlias = dto.GetValue("mediaTypeAlias"); - IMedia mediaFile = _temporaryMediaService.Save(temporaryLocationString, startNodeGuid?.Guid, mediaTypeAlias?.Value()); - MediaWithCropsDto? mediaDto = _jsonSerializer.Deserialize(dto.ToString()); - if (mediaDto is null) - { - continue; - } - - mediaDto.MediaKey = mediaFile.GetUdi().Guid; - result.Add(JObject.Parse(_jsonSerializer.Serialize(mediaDto))); - } - - return result; - } + // private JArray PersistTempMedia(JArray jArray, MediaPicker3Configuration mediaPicker3Configuration) + // { + // var result = new JArray(); + // foreach (JObject? dto in jArray.Values()) + // { + // if (dto is null) + // { + // continue; + // } + // + // if (!dto.TryGetValue("tmpLocation", out JToken? temporaryLocation)) + // { + // // If it does not have a temporary path, it can be an already saved image or not-yet uploaded temp-image, check for media-key + // if (dto.TryGetValue("mediaKey", out _)) + // { + // result.Add(dto); + // } + // + // continue; + // } + // + // var temporaryLocationString = temporaryLocation.Value(); + // if (temporaryLocationString is null) + // { + // continue; + // } + // + // GuidUdi? startNodeGuid = mediaPicker3Configuration.StartNodeId as GuidUdi ?? null; + // JToken? mediaTypeAlias = dto.GetValue("mediaTypeAlias"); + // IMedia mediaFile = _temporaryMediaService.Save(temporaryLocationString, startNodeGuid?.Guid, mediaTypeAlias?.Value()); + // MediaWithCropsDto? mediaDto = _jsonSerializer.Deserialize(dto.ToString()); + // if (mediaDto is null) + // { + // continue; + // } + // + // mediaDto.MediaKey = mediaFile.GetUdi().Guid; + // result.Add(JObject.Parse(_jsonSerializer.Serialize(mediaDto))); + // } + // + // return result; + // } /// /// Model/DTO that represents the JSON that the MediaPicker3 stores. /// - [DataContract] internal class MediaWithCropsDto { - [DataMember(Name = "key")] public Guid Key { get; set; } - [DataMember(Name = "mediaKey")] public Guid MediaKey { get; set; } - [DataMember(Name = "crops")] public IEnumerable? Crops { get; set; } - [DataMember(Name = "focalPoint")] public ImageCropperValue.ImageCropperFocalPoint? FocalPoint { get; set; } /// /// Removes redundant crop data/default focal point. /// - /// The media with crops DTO. /// /// Because the DTO uses the same JSON keys as the image cropper value for crops and focal point, we can re-use the /// prune method. /// - public static void Prune(JObject? value) => ImageCropperValue.Prune(value); + internal void Prune() + { + Crops = Crops?.Where(crop => crop.Coordinates != null).ToArray(); + if (FocalPoint is { Top: 0.5m, Left: 0.5m }) + { + FocalPoint = null; + } + } /// /// Applies the configuration to ensure only valid crops are kept and have the correct width/height. diff --git a/src/Umbraco.Infrastructure/PropertyEditors/MultipleValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/MultipleValueEditor.cs index 2f9cf5f3aa..24315ca42e 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/MultipleValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/MultipleValueEditor.cs @@ -1,8 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Editors; @@ -20,15 +18,16 @@ namespace Umbraco.Cms.Core.PropertyEditors; /// public class MultipleValueEditor : DataValueEditor { + private readonly IJsonSerializer _jsonSerializer; + public MultipleValueEditor( ILocalizedTextService localizedTextService, IShortStringHelper shortStringHelper, IJsonSerializer jsonSerializer, IIOHelper ioHelper, DataEditorAttribute attribute) - : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) - { - } + : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) => + _jsonSerializer = jsonSerializer; /// /// Override so that we can return an array to the editor for multi-select values @@ -43,7 +42,7 @@ public class MultipleValueEditor : DataValueEditor string[]? result = null; if (json is not null) { - result = JsonConvert.DeserializeObject(json); + result = _jsonSerializer.Deserialize(json); } return result ?? Array.Empty(); @@ -58,17 +57,12 @@ public class MultipleValueEditor : DataValueEditor /// public override object? FromEditor(ContentPropertyData editorValue, object? currentValue) { - if (editorValue.Value is not JArray json || json.HasValues == false) + if (editorValue.Value is not IEnumerable stringValues || stringValues.Any() == false) { return null; } - var values = json.Select(item => item.Value()).ToArray(); - if (values.Length == 0) - { - return null; - } - - return JsonConvert.SerializeObject(values, Formatting.None); + var result = _jsonSerializer.Serialize(stringValues); + return result; } } diff --git a/src/Umbraco.Infrastructure/PropertyEditors/TagsPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/TagsPropertyEditor.cs index 1923950fe0..46118367cf 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/TagsPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/TagsPropertyEditor.cs @@ -3,8 +3,6 @@ using System.ComponentModel.DataAnnotations; using Microsoft.Extensions.DependencyInjection; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -96,6 +94,7 @@ public class TagsPropertyEditor : DataEditor internal class TagPropertyValueEditor : DataValueEditor, IDataValueTags { + private readonly IJsonSerializer _jsonSerializer; private readonly IDataTypeService _dataTypeService; public TagPropertyValueEditor( @@ -107,19 +106,41 @@ public class TagsPropertyEditor : DataEditor IDataTypeService dataTypeService) : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) { + _jsonSerializer = jsonSerializer; _dataTypeService = dataTypeService; } /// public IEnumerable GetTags(object? value, object? dataTypeConfiguration, int? languageId) { - var strValue = value?.ToString(); - if (string.IsNullOrWhiteSpace(strValue)) return Enumerable.Empty(); + TagConfiguration tagConfiguration = ConfigurationEditor.ConfigurationAs(dataTypeConfiguration) ?? new TagConfiguration(); - var tagConfiguration = ConfigurationEditor.ConfigurationAs(dataTypeConfiguration) ?? new TagConfiguration(); + var tags = ParseTags(value, tagConfiguration); + if (tags.Any() is false) + { + return Enumerable.Empty(); + } + + return tags.Select(x => new Tag + { + Group = tagConfiguration.Group, + Text = x, + LanguageId = languageId, + }); + } + + private string[] ParseTags(object? value, TagConfiguration tagConfiguration) + { + var strValue = value?.ToString(); + if (string.IsNullOrWhiteSpace(strValue)) + { + return Array.Empty(); + } if (tagConfiguration.Delimiter == default) + { tagConfiguration.Delimiter = ','; + } IEnumerable tags; @@ -132,9 +153,9 @@ public class TagsPropertyEditor : DataEditor case TagsStorageType.Json: try { - tags = JsonConvert.DeserializeObject(strValue)?.Select(x => x.Trim()) ?? Enumerable.Empty(); + tags = _jsonSerializer.Deserialize(strValue)?.Select(x => x.Trim()) ?? Enumerable.Empty(); } - catch (JsonException) + catch { //cannot parse, malformed tags = Enumerable.Empty(); @@ -146,46 +167,45 @@ public class TagsPropertyEditor : DataEditor throw new NotSupportedException($"Value \"{tagConfiguration.StorageType}\" is not a valid TagsStorageType."); } - return tags.Select(x => new Tag - { - Group = tagConfiguration.Group, - Text = x, - LanguageId = languageId, - }); + return tags.ToArray(); } - /// public override IValueRequiredValidator RequiredValidator => new RequiredJsonValueValidator(); + public override object? ToEditor(IProperty property, string? culture = null, string? segment = null) + { + var val = property.GetValue(culture, segment); + if (val is null) + { + return null; + } + + IDataType? dataType = _dataTypeService.GetDataType(property.PropertyType.DataTypeId); + TagConfiguration configuration = dataType?.ConfigurationObject as TagConfiguration ?? new TagConfiguration(); + var tags = ParseTags(val, configuration); + + return tags.Any() ? tags : null; + } + /// public override object? FromEditor(ContentPropertyData editorValue, object? currentValue) { - var value = editorValue.Value?.ToString(); - - if (string.IsNullOrEmpty(value)) + if (editorValue.Value is not IEnumerable stringValues) { return null; } - var tagConfiguration = editorValue.DataTypeConfiguration as TagConfiguration ?? new TagConfiguration(); + var trimmedTags = stringValues.Select(s => s.Trim()).Where(s => s.IsNullOrWhiteSpace() == false).ToArray(); + if (trimmedTags.Any() is false) + { + return null; + } + + TagConfiguration tagConfiguration = editorValue.DataTypeConfiguration as TagConfiguration ?? new TagConfiguration(); if (tagConfiguration.Delimiter == default) + { tagConfiguration.Delimiter = ','; - - string[] trimmedTags = Array.Empty(); - - if (editorValue.Value is JArray json) - { - trimmedTags = json.HasValues ? json.Select(x => x.Value()).OfType().ToArray() : Array.Empty(); - } - else if (string.IsNullOrWhiteSpace(value) == false) - { - trimmedTags = value.Split(Constants.CharArrays.Comma, StringSplitOptions.RemoveEmptyEntries); - } - - if (trimmedTags.Length == 0) - { - return null; } switch (tagConfiguration.StorageType) @@ -194,7 +214,7 @@ public class TagsPropertyEditor : DataEditor return string.Join(tagConfiguration.Delimiter.ToString(), trimmedTags).NullOrWhiteSpaceAsNull(); case TagsStorageType.Json: - return trimmedTags.Length == 0 ? null : JsonConvert.SerializeObject(trimmedTags); + return trimmedTags.Length == 0 ? null : _jsonSerializer.Serialize(trimmedTags); } return null; diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValue.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValue.cs index dbf1d41431..6c770eaf5e 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValue.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValue.cs @@ -1,14 +1,9 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System.ComponentModel; -using System.Runtime.Serialization; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; using Umbraco.Cms.Core.Media; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Strings; -using Umbraco.Cms.Infrastructure.Serialization; using Umbraco.Extensions; namespace Umbraco.Cms.Core.PropertyEditors.ValueConverters; @@ -16,35 +11,27 @@ namespace Umbraco.Cms.Core.PropertyEditors.ValueConverters; /// /// Represents a value of the image cropper value editor. /// -[JsonConverter(typeof(NoTypeConverterJsonConverter))] -[TypeConverter(typeof(ImageCropperValueTypeConverter))] -[DataContract(Name = "imageCropDataSet")] public class ImageCropperValue : IHtmlEncodedString, IEquatable { /// /// Gets or sets the value source image. /// - [DataMember(Name = "src")] public string? Src { get; set; } = string.Empty; /// /// Gets or sets the value focal point. /// - [DataMember(Name = "focalPoint")] public ImageCropperFocalPoint? FocalPoint { get; set; } /// /// Gets or sets the value crops. /// - [DataMember(Name = "crops")] public IEnumerable? Crops { get; set; } /// public string? ToHtmlString() => Src; - /// - public override string? ToString() - => HasCrops() || HasFocalPoint() ? JsonConvert.SerializeObject(this, Formatting.None) : Src; + public override string? ToString() => Src; /// /// Gets a crop. @@ -185,56 +172,20 @@ public class ImageCropperValue : IHtmlEncodedString, IEquatable /// Removes redundant crop data/default focal point. /// - /// The image cropper value. - public static void Prune(JObject? value) + internal void Prune() { - if (value is null) + Crops = Crops?.Where(crop => crop.Coordinates != null).ToArray(); + if (FocalPoint is { Top: 0.5m, Left: 0.5m }) { - throw new ArgumentNullException(nameof(value)); - } - - if (value.TryGetValue("crops", out JToken? crops)) - { - if (crops.HasValues) - { - foreach (JObject crop in crops.Values().WhereNotNull().ToList()) - { - if (crop.TryGetValue("coordinates", out JToken? coordinates) == false || - coordinates.HasValues == false) - { - // Remove crop without coordinates - crop.Remove(); - continue; - } - - // Width/height are already stored in the crop configuration - crop.Remove("width"); - crop.Remove("height"); - } - } - - if (crops.HasValues == false) - { - // Remove empty crops - value.Remove("crops"); - } - } - - if (value.TryGetValue("focalPoint", out JToken? focalPoint) && - (focalPoint.HasValues == false || - (focalPoint.Value("top") == 0.5m && focalPoint.Value("left") == 0.5m))) - { - // Remove empty/default focal point - value.Remove("focalPoint"); + FocalPoint = null; } } - [DataContract(Name = "imageCropFocalPoint")] public class ImageCropperFocalPoint : IEquatable { - [DataMember(Name = "left")] public decimal Left { get; set; } + public decimal Left { get; set; } - [DataMember(Name = "top")] public decimal Top { get; set; } + public decimal Top { get; set; } #region IEquatable @@ -272,16 +223,15 @@ public class ImageCropperValue : IHtmlEncodedString, IEquatable { - [DataMember(Name = "alias")] public string? Alias { get; set; } + public string? Alias { get; set; } - [DataMember(Name = "width")] public int Width { get; set; } + public int Width { get; set; } - [DataMember(Name = "height")] public int Height { get; set; } + public int Height { get; set; } - [DataMember(Name = "coordinates")] public ImageCropperCropCoordinates? Coordinates { get; set; } + public ImageCropperCropCoordinates? Coordinates { get; set; } #region IEquatable @@ -325,16 +275,15 @@ public class ImageCropperValue : IHtmlEncodedString, IEquatable { - [DataMember(Name = "x1")] public decimal X1 { get; set; } + public decimal X1 { get; set; } - [DataMember(Name = "y1")] public decimal Y1 { get; set; } + public decimal Y1 { get; set; } - [DataMember(Name = "x2")] public decimal X2 { get; set; } + public decimal X2 { get; set; } - [DataMember(Name = "y2")] public decimal Y2 { get; set; } + public decimal Y2 { get; set; } #region IEquatable @@ -357,7 +306,7 @@ public class ImageCropperValue : IHtmlEncodedString, IEquatable Equals(left, right); - public static bool operator !=(ImageCropperCropCoordinates left, ImageCropperCropCoordinates right) + public static bool operator !=(ImageCropperCropCoordinates? left, ImageCropperCropCoordinates? right) => !Equals(left, right); public override int GetHashCode() diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValueTypeConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValueTypeConverter.cs deleted file mode 100644 index 34528f3f40..0000000000 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValueTypeConverter.cs +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) Umbraco. -// See LICENSE for more details. - -using System.ComponentModel; -using System.Globalization; -using Newtonsoft.Json.Linq; -using Umbraco.Cms.Core.Composing; - -namespace Umbraco.Cms.Core.PropertyEditors.ValueConverters; - -/// -/// Converts to string or JObject (why?). -/// -public class ImageCropperValueTypeConverter : TypeConverter -{ - private static readonly Type[] _convertableTypes = { typeof(JObject) }; - - public override bool CanConvertTo(ITypeDescriptorContext? context, Type? destinationType) - { - if (destinationType is null) - { - return false; - } - - return _convertableTypes.Any(x => TypeHelper.IsTypeAssignableFrom(x, destinationType)) - || CanConvertFrom(context, destinationType); - } - - public override object? ConvertTo(ITypeDescriptorContext? context, CultureInfo? culture, object? value, Type destinationType) - { - if (value is not ImageCropperValue cropperValue) - { - return null; - } - - return TypeHelper.IsTypeAssignableFrom(destinationType) - ? JObject.FromObject(cropperValue) - : base.ConvertTo(context, culture, value, destinationType); - } -} diff --git a/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateExtensions.cs b/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateExtensions.cs index fdd9966fac..aa0963d94f 100644 --- a/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateExtensions.cs @@ -1,8 +1,9 @@ -using System.Globalization; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; -using Newtonsoft.Json; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.PropertyEditors.ValueConverters; +using Umbraco.Cms.Core.Serialization; namespace Umbraco.Extensions; @@ -11,12 +12,6 @@ namespace Umbraco.Extensions; /// public static class ImageCropperTemplateExtensions { - private static readonly JsonSerializerSettings ImageCropperValueJsonSerializerSettings = new() - { - Culture = CultureInfo.InvariantCulture, - FloatParseHandling = FloatParseHandling.Decimal, - }; - internal static ImageCropperValue DeserializeImageCropperValue(this string json) { ImageCropperValue? imageCrops = null; @@ -25,8 +20,8 @@ public static class ImageCropperTemplateExtensions { try { - imageCrops = - JsonConvert.DeserializeObject(json, ImageCropperValueJsonSerializerSettings); + IJsonSerializer? serializer = StaticServiceProvider.Instance.GetService(); + imageCrops = serializer?.Deserialize(json); } catch (Exception ex) { diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ImageCropperTest.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ImageCropperTest.cs index 711914a01e..3e3f9efdd7 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ImageCropperTest.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ImageCropperTest.cs @@ -1,17 +1,15 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System; -using System.Collections.Generic; using System.Globalization; -using System.Linq; using System.Text; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; +using Moq; using NUnit.Framework; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Media; using Umbraco.Cms.Core.Models; -using Umbraco.Cms.Core.PropertyEditors.ValueConverters; +using Umbraco.Cms.Core.Serialization; +using Umbraco.Cms.Infrastructure.Serialization; using Umbraco.Extensions; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common; @@ -30,6 +28,8 @@ public class ImageCropperTest [Test] public void CanConvertImageCropperDataSetSrcToString() { + SetupJsonSerializerServiceProvider(); + // cropperJson3 - has no crops var cropperValue = CropperJson2.DeserializeImageCropperValue(); var serialized = cropperValue.TryConvertTo(); @@ -38,30 +38,38 @@ public class ImageCropperTest } [Test] - public void CanConvertImageCropperDataSetJObject() + public void CanConvertJsonStringToImageCropperValue() { - // cropperJson3 - has no crops - var cropperValue = CropperJson2.DeserializeImageCropperValue(); - var serialized = cropperValue.TryConvertTo(); - Assert.IsTrue(serialized.Success); - Assert.AreEqual(cropperValue, serialized.Result.ToObject()); + SetupJsonSerializerServiceProvider(); + + // cropperJson1 - has crops + var cropperValue = CropperJson1.DeserializeImageCropperValue(); + Assert.AreEqual(MediaPath, cropperValue.Src); + Assert.IsNotNull(cropperValue.FocalPoint); + Assert.AreEqual(0.96m, cropperValue.FocalPoint.Left); + Assert.AreEqual(0.80827067669172936m, cropperValue.FocalPoint.Top); + Assert.IsNotNull(cropperValue.Crops); + Assert.AreEqual(1, cropperValue.Crops.Count()); + var crop = cropperValue.Crops.First(); + Assert.AreEqual("thumb", crop.Alias); + Assert.AreEqual(100, crop.Width); + Assert.AreEqual(100, crop.Height); + Assert.IsNotNull(crop.Coordinates); + Assert.AreEqual(0.58729977382575338m, crop.Coordinates.X1); + Assert.AreEqual(0.055768992440203169m, crop.Coordinates.Y1); + Assert.AreEqual(0m, crop.Coordinates.X2); + Assert.AreEqual(0.32457553600198386m, crop.Coordinates.Y2); } [Test] public void CanConvertImageCropperDataSetJsonToString() { + SetupJsonSerializerServiceProvider(); + var cropperValue = CropperJson1.DeserializeImageCropperValue(); var serialized = cropperValue.TryConvertTo(); Assert.IsTrue(serialized.Success); - Assert.IsTrue(serialized.Result.DetectIsJson()); - var obj = JsonConvert.DeserializeObject( - CropperJson1, - new JsonSerializerSettings - { - Culture = CultureInfo.InvariantCulture, - FloatParseHandling = FloatParseHandling.Decimal, - }); - Assert.AreEqual(cropperValue, obj); + Assert.AreEqual("/media/1005/img_0671.jpg", serialized.Result); } // [TestCase(CropperJson1, CropperJson1, true)] @@ -416,6 +424,13 @@ public class ImageCropperTest Assert.AreEqual(MediaPath + "?m=pad&w=400&h=400&bgcolor=fff", urlString); } + private void SetupJsonSerializerServiceProvider() + { + var serviceProvider = new Mock(); + serviceProvider.Setup(s => s.GetService(typeof(IJsonSerializer))).Returns(new SystemTextJsonSerializer()); + StaticServiceProvider.Instance = serviceProvider.Object; + } + internal class TestImageUrlGenerator : IImageUrlGenerator { public IEnumerable SupportedImageFileTypes => new[] { "jpeg", "jpg", "gif", "bmp", "png", "tiff", "tif" };