Skip to content

Commit

Permalink
Prune Image Cropper and Media Picker (v3) values (#11805)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@ronaldbarendse @elit0451
ronaldbarendse and elit0451 authored Jan 5, 2022
1 parent a54c5bb commit 75bb805
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 40 deletions.
88 changes: 68 additions & 20 deletions src/Umbraco.Core/PropertyEditors/ValueConverters/ImageCropperValue.cs
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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.
Expand All @@ -41,9 +40,7 @@ public class ImageCropperValue : IHtmlString, IEquatable<ImageCropperValue>

/// <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;
Expand Down Expand Up @@ -134,13 +131,19 @@ public string GetCropUrl(int width, int height, IImageUrlGenerator imageUrlGener
/// </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.
Expand Down Expand Up @@ -179,6 +182,51 @@ internal ImageCropperValue Merge(ImageCropperValue imageCropperValue)
};
}

/// <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 />
Expand Down Expand Up @@ -212,8 +260,8 @@ public override int GetHashCode()
// 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
}
Expand Down Expand Up @@ -258,7 +306,7 @@ public override int GetHashCode()
{
// properties are, practically, readonly
// ReSharper disable NonReadonlyMemberInGetHashCode
return (Left.GetHashCode()*397) ^ Top.GetHashCode();
return (Left.GetHashCode() * 397) ^ Top.GetHashCode();
// ReSharper restore NonReadonlyMemberInGetHashCode
}
}
Expand Down Expand Up @@ -312,9 +360,9 @@ public override int GetHashCode()
// 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
}
Expand All @@ -339,7 +387,7 @@ public class ImageCropperCropCoordinates : IEquatable<ImageCropperCropCoordinate
public decimal Y2 { get; set; }

#region IEquatable

/// <inheritdoc />
public bool Equals(ImageCropperCropCoordinates other)
=> ReferenceEquals(this, other) || Equals(this, other);
Expand Down Expand Up @@ -369,9 +417,9 @@ public override int GetHashCode()
// 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
}
Expand Down
6 changes: 3 additions & 3 deletions src/Umbraco.Tests/PropertyEditors/ImageCropperTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ public class ImageCropperTest
{
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);
Expand All @@ -45,7 +45,7 @@ public void CanConvertImageCropperDataSetSrcToString()
[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);
Expand Down
39 changes: 25 additions & 14 deletions src/Umbraco.Web/PropertyEditors/ImageCropperPropertyValueEditor.cs
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -66,31 +66,42 @@ public override object ToEditor(Property property, IDataTypeService dataTypeServ
/// </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
Expand Down Expand Up @@ -118,7 +129,7 @@ public override object FromEditor(ContentPropertyData editorValue, object curren
return null; // clear
}

return editorJson?.ToString(); // unchanged
return editorJson?.ToString(Formatting.None); // unchanged
}

// process the file
Expand All @@ -135,7 +146,8 @@ public override object FromEditor(ContentPropertyData editorValue, object curren
// 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)
Expand All @@ -160,7 +172,6 @@ private string ProcessFile(ContentPropertyData editorValue, ContentPropertyFile
return filepath;
}


public override string ConvertDbToString(PropertyType propertyType, object value, IDataTypeService dataTypeService)
{
if (value == null || string.IsNullOrEmpty(value.ToString()))
Expand Down
81 changes: 78 additions & 3 deletions src/Umbraco.Web/PropertyEditors/MediaPicker3PropertyEditor.cs
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -50,7 +51,36 @@ public override object ToEditor(Property property, IDataTypeService dataTypeServ
{
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)
Expand Down Expand Up @@ -117,6 +147,51 @@ internal class MediaWithCropsDto

[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);
}
}
}
Expand Down

0 comments on commit 75bb805

Please sign in to comment.