yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Prune Image Cropper and Media Picker (v3) values (#11805)

Browse Source 
* Clean up redundant/default Umbraco.ImageCropper data

* Fix ToString() and add HasCrops() method

* Re-use crop/focal point pruning for Umbraco.MediaPicker3

* Fix ImageCropperTest

Co-authored-by: Elitsa Marinovska <elm@umbraco.dk>
This commit is contained in:
Ronald Barendse
2022-01-05 11:11:27 +01:00
committed by GitHub
parent a54c5bb21d
commit 75bb8051bf
4 changed files with 174 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
src/Umbraco.Core/PropertyEditors/ValueConverters/ImageCropperValue.cs
View File

@@ -1,12 +1,11 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Globalization;
using System.Linq;
using System.Runtime.Serialization;
using System.Text;
using System.Web;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Umbraco.Core.Composing;
using Umbraco.Core.Models;
using Umbraco.Core.Serialization;
@@ -18,14 +17,14 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters
/// </summary>
[JsonConverter(typeof(NoTypeConverterJsonConverter<ImageCropperValue>))]
[TypeConverter(typeof(ImageCropperValueTypeConverter))]
[DataContract(Name="imageCropDataSet")]
[DataContract(Name = "imageCropDataSet")]
public class ImageCropperValue : IHtmlString, IEquatable<ImageCropperValue>
{
/// <summary>
/// Gets or sets the value source image.
/// </summary>
[DataMember(Name="src")]
public string Src { get; set;}
[DataMember(Name = "src")]
public string Src { get; set; }
/// <summary>
/// Gets or sets the value focal point.
@@ -41,9 +40,7 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters
/// <inheritdoc />
public override string ToString()
{
return Crops != null ? (Crops.Any() ? JsonConvert.SerializeObject(this) : Src) : string.Empty;
}
=> HasCrops() || HasFocalPoint() ? JsonConvert.SerializeObject(this, Formatting.None) : Src;
/// <inheritdoc />
public string ToHtmlString() => Src;
@@ -134,13 +131,19 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters
/// </summary>
/// <returns></returns>
public bool HasFocalPoint()
=> FocalPoint != null && (FocalPoint.Left != 0.5m || FocalPoint.Top != 0.5m);
=> FocalPoint is ImageCropperFocalPoint focalPoint && (focalPoint.Left != 0.5m || focalPoint.Top != 0.5m);
/// <summary>
/// Determines whether the value has crops.
/// </summary>
public bool HasCrops()
=> Crops is IEnumerable<ImageCropperCrop> crops && crops.Any();
/// <summary>
/// Determines whether the value has a specified crop.
/// </summary>
public bool HasCrop(string alias)
=> Crops != null && Crops.Any(x => x.Alias == alias);
=> Crops is IEnumerable<ImageCropperCrop> crops && crops.Any(x => x.Alias == alias);
/// <summary>
/// Determines whether the value has a source image.
@@ -179,6 +182,51 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters
};
}
/// <summary>
/// Removes redundant crop data/default focal point.
/// </summary>
/// <param name="value">The image cropper value.</param>
/// <returns>
/// The cleaned up value.
/// </returns>
public static void Prune(JObject value)
{
if (value is null) throw new ArgumentNullException(nameof(value));
if (value.TryGetValue("crops", out var crops))
{
if (crops.HasValues)
{
foreach (var crop in crops.Values<JObject>().ToList())
{
if (crop.TryGetValue("coordinates", out var 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 var focalPoint) &&
(focalPoint.HasValues == false || (focalPoint.Value<decimal>("top") == 0.5m && focalPoint.Value<decimal>("left") == 0.5m)))
{
// Remove empty/default focal point
value.Remove("focalPoint");
}
}
#region IEquatable
/// <inheritdoc />
@@ -212,8 +260,8 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters
// properties are, practically, readonly
// ReSharper disable NonReadonlyMemberInGetHashCode
var hashCode = Src?.GetHashCode() ?? 0;
hashCode = (hashCode*397) ^ (FocalPoint?.GetHashCode() ?? 0);
hashCode = (hashCode*397) ^ (Crops?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (FocalPoint?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (Crops?.GetHashCode() ?? 0);
return hashCode;
// ReSharper restore NonReadonlyMemberInGetHashCode
}
@@ -258,7 +306,7 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters
{
// properties are, practically, readonly
// ReSharper disable NonReadonlyMemberInGetHashCode
return (Left.GetHashCode()*397) ^ Top.GetHashCode();
return (Left.GetHashCode() * 397) ^ Top.GetHashCode();
// ReSharper restore NonReadonlyMemberInGetHashCode
}
}
@@ -312,9 +360,9 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters
// properties are, practically, readonly
// ReSharper disable NonReadonlyMemberInGetHashCode
var hashCode = Alias?.GetHashCode() ?? 0;
hashCode = (hashCode*397) ^ Width;
hashCode = (hashCode*397) ^ Height;
hashCode = (hashCode*397) ^ (Coordinates?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ Width;
hashCode = (hashCode * 397) ^ Height;
hashCode = (hashCode * 397) ^ (Coordinates?.GetHashCode() ?? 0);
return hashCode;
// ReSharper restore NonReadonlyMemberInGetHashCode
}
@@ -339,7 +387,7 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters
public decimal Y2 { get; set; }
#region IEquatable
/// <inheritdoc />
public bool Equals(ImageCropperCropCoordinates other)
=> ReferenceEquals(this, other) || Equals(this, other);
@@ -369,9 +417,9 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters
// properties are, practically, readonly
// ReSharper disable NonReadonlyMemberInGetHashCode
var hashCode = X1.GetHashCode();
hashCode = (hashCode*397) ^ Y1.GetHashCode();
hashCode = (hashCode*397) ^ X2.GetHashCode();
hashCode = (hashCode*397) ^ Y2.GetHashCode();
hashCode = (hashCode * 397) ^ Y1.GetHashCode();
hashCode = (hashCode * 397) ^ X2.GetHashCode();
hashCode = (hashCode * 397) ^ Y2.GetHashCode();
return hashCode;
// ReSharper restore NonReadonlyMemberInGetHashCode
}

6
src/Umbraco.Tests/PropertyEditors/ImageCropperTest.cs
View File

@@ -29,13 +29,13 @@ namespace Umbraco.Tests.PropertyEditors
{
private const string CropperJson1 = "{\"focalPoint\": {\"left\": 0.96,\"top\": 0.80827067669172936},\"src\": \"/media/1005/img_0671.jpg\",\"crops\": [{\"alias\":\"thumb\",\"width\": 100,\"height\": 100,\"coordinates\": {\"x1\": 0.58729977382575338,\"y1\": 0.055768992440203169,\"x2\": 0,\"y2\": 0.32457553600198386}}]}";
private const string CropperJson2 = "{\"focalPoint\": {\"left\": 0.98,\"top\": 0.80827067669172936},\"src\": \"/media/1005/img_0672.jpg\",\"crops\": [{\"alias\":\"thumb\",\"width\": 100,\"height\": 100,\"coordinates\": {\"x1\": 0.58729977382575338,\"y1\": 0.055768992440203169,\"x2\": 0,\"y2\": 0.32457553600198386}}]}";
private const string CropperJson3 = "{\"focalPoint\": {\"left\": 0.98,\"top\": 0.80827067669172936},\"src\": \"/media/1005/img_0672.jpg\",\"crops\": []}";
private const string CropperJson3 = "{\"focalPoint\": {\"left\": 0.5,\"top\": 0.5},\"src\": \"/media/1005/img_0672.jpg\",\"crops\": []}";
private const string MediaPath = "/media/1005/img_0671.jpg";
[Test]
public void CanConvertImageCropperDataSetSrcToString()
{
//cropperJson3 - has not crops
//cropperJson3 - has no crops
var cropperValue = CropperJson3.DeserializeImageCropperValue();
var serialized = cropperValue.TryConvertTo<string>();
Assert.IsTrue(serialized.Success);
@@ -45,7 +45,7 @@ namespace Umbraco.Tests.PropertyEditors
[Test]
public void CanConvertImageCropperDataSetJObject()
{
//cropperJson3 - has not crops
//cropperJson3 - has no crops
var cropperValue = CropperJson3.DeserializeImageCropperValue();
var serialized = cropperValue.TryConvertTo<JObject>();
Assert.IsTrue(serialized.Success);

39
src/Umbraco.Web/PropertyEditors/ImageCropperPropertyValueEditor.cs
View File

@@ -1,6 +1,6 @@
using Newtonsoft.Json.Linq;
using System;
using System;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Umbraco.Core;
using Umbraco.Core.IO;
using Umbraco.Core.Logging;
@@ -66,31 +66,42 @@ namespace Umbraco.Web.PropertyEditors
/// </remarks>
public override object FromEditor(ContentPropertyData editorValue, object currentValue)
{
// get the current path
// Get the current path
var currentPath = string.Empty;
try
{
var svalue = currentValue as string;
var currentJson = string.IsNullOrWhiteSpace(svalue) ? null : JObject.Parse(svalue);
if (currentJson != null && currentJson["src"] != null)
currentPath = currentJson["src"].Value<string>();
if (currentJson != null && currentJson.TryGetValue("src", out var src))
{
currentPath = src.Value<string>();
}
}
catch (Exception ex)
{
// for some reason the value is invalid so continue as if there was no value there
// For some reason the value is invalid, so continue as if there was no value there
_logger.Warn<ImageCropperPropertyValueEditor>(ex, "Could not parse current db value to a JObject.");
}
if (string.IsNullOrWhiteSpace(currentPath) == false)
currentPath = _mediaFileSystem.GetRelativePath(currentPath);
// get the new json and path
JObject editorJson = null;
// Get the new JSON and file path
var editorFile = string.Empty;
if (editorValue.Value != null)
if (editorValue.Value is JObject editorJson)
{
editorJson = editorValue.Value as JObject;
if (editorJson != null && editorJson["src"] != null)
// Populate current file
if (editorJson["src"] != null)
{
editorFile = editorJson["src"].Value<string>();
}
// Clean up redundant/default data
ImageCropperValue.Prune(editorJson);
}
else
{
editorJson = null;
}
// ensure we have the required guids
@@ -118,7 +129,7 @@ namespace Umbraco.Web.PropertyEditors
return null; // clear
}
return editorJson?.ToString(); // unchanged
return editorJson?.ToString(Formatting.None); // unchanged
}
// process the file
@@ -135,7 +146,8 @@ namespace Umbraco.Web.PropertyEditors
// update json and return
if (editorJson == null) return null;
editorJson["src"] = filepath == null ? string.Empty : _mediaFileSystem.GetUrl(filepath);
return editorJson.ToString();
return editorJson.ToString(Formatting.None);
}
private string ProcessFile(ContentPropertyData editorValue, ContentPropertyFile file, string currentPath, Guid cuid, Guid puid)
@@ -160,7 +172,6 @@ namespace Umbraco.Web.PropertyEditors
return filepath;
}
public override string ConvertDbToString(PropertyType propertyType, object value, IDataTypeService dataTypeService)
{
if (value == null || string.IsNullOrEmpty(value.ToString()))

81
src/Umbraco.Web/PropertyEditors/MediaPicker3PropertyEditor.cs
View File

@@ -1,8 +1,9 @@
using Newtonsoft.Json;
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.Serialization;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Umbraco.Core;
using Umbraco.Core.Logging;
using Umbraco.Core.Models;
@@ -50,7 +51,36 @@ namespace Umbraco.Web.PropertyEditors
{
var value = property.GetValue(culture, segment);
return Deserialize(value);
var dtos = Deserialize(value).ToList();
var dataType = dataTypeService.GetDataType(property.PropertyType.DataTypeId);
if (dataType?.Configuration != null)
{
var configuration = dataType.ConfigurationAs<MediaPicker3Configuration>();
foreach (var dto in dtos)
{
dto.ApplyConfiguration(configuration);
}
}
return dtos;
}
public override object FromEditor(ContentPropertyData editorValue, object currentValue)
{
if (editorValue.Value is JArray dtos)
{
// Clean up redundant/default data
foreach (var dto in dtos.Values<JObject>())
{
MediaWithCropsDto.Prune(dto);
}
return dtos.ToString(Formatting.None);
}
return base.FromEditor(editorValue, currentValue);
}
public IEnumerable<UmbracoEntityReference> GetReferences(object value)
@@ -117,6 +147,51 @@ namespace Umbraco.Web.PropertyEditors
[DataMember(Name = "focalPoint")]
public ImageCropperValue.ImageCropperFocalPoint FocalPoint { get; set; }
/// <summary>
/// Applies the configuration to ensure only valid crops are kept and have the correct width/height.
/// </summary>
/// <param name="configuration">The configuration.</param>
public void ApplyConfiguration(MediaPicker3Configuration configuration)
{
var crops = new List<ImageCropperValue.ImageCropperCrop>();
var configuredCrops = configuration?.Crops;
if (configuredCrops != null)
{
foreach (var configuredCrop in configuredCrops)
{
var crop = Crops?.FirstOrDefault(x => x.Alias == configuredCrop.Alias);
crops.Add(new ImageCropperValue.ImageCropperCrop
{
Alias = configuredCrop.Alias,
Width = configuredCrop.Width,
Height = configuredCrop.Height,
Coordinates = crop?.Coordinates
});
}
}
Crops = crops;
if (configuration?.EnableLocalFocalPoint == false)
{
FocalPoint = null;
}
}
/// <summary>
/// Removes redundant crop data/default focal point.
/// </summary>
/// <param name="value">The media with crops DTO.</param>
/// <returns>
/// The cleaned up value.
/// </returns>
/// <remarks>
/// 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.
/// </remarks>
public static void Prune(JObject value) => ImageCropperValue.Prune(value);
}
}
}