Improvements to media pickers/crop handling and URL generation

src/Umbraco.Core/Models/MediaWithCrops.cs

@@ -1,15 +1,86 @@
+using System;
 using Umbraco.Core.Models.PublishedContent;
 using Umbraco.Core.PropertyEditors.ValueConverters;
 
 namespace Umbraco.Core.Models
 {
     /// <summary>
-    /// Model used in Razor Views for rendering
+    /// Represents a media item with local crops.
     /// </summary>
-    public class MediaWithCrops
+    /// <seealso cref="Umbraco.Core.Models.PublishedContent.PublishedContentWrapped" />
+    public class MediaWithCrops : PublishedContentWrapped
     {
-        public IPublishedContent MediaItem { get; set; }
+        /// <summary>
+        /// Gets the media item.
+        /// </summary>
+        /// <value>
+        /// The media item.
+        /// </value>
+        [Obsolete("This instance now implements IPublishedContent by wrapping the media item, use the extension methods directly on MediaWithCrops or use the Content property to get the media item instead.")]
+        public IPublishedContent MediaItem => Content;
 
-        public ImageCropperValue LocalCrops { get; set; }
+        /// <summary>
+        /// Gets the content/media item.
+        /// </summary>
+        /// <value>
+        /// The content/media item.
+        /// </value>
+        public IPublishedContent Content => Unwrap();
+
+        /// <summary>
+        /// Gets the local crops.
+        /// </summary>
+        /// <value>
+        /// The local crops.
+        /// </value>
+        public ImageCropperValue LocalCrops { get; }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="MediaWithCrops" /> class.
+        /// </summary>
+        /// <param name="content">The content.</param>
+        /// <param name="localCrops">The local crops.</param>
+        public MediaWithCrops(IPublishedContent content, ImageCropperValue localCrops)
+            : base(content)
+        {
+            LocalCrops = localCrops;
+        }
+    }
+
+    /// <summary>
+    /// Represents a media item with local crops.
+    /// </summary>
+    /// <typeparam name="T">The type of the media item.</typeparam>
+    /// <seealso cref="Umbraco.Core.Models.PublishedContent.PublishedContentWrapped" />
+    public class MediaWithCrops<T> : MediaWithCrops
+        where T : IPublishedContent
+    {
+        /// <summary>
+        /// Gets the media item.
+        /// </summary>
+        /// <value>
+        /// The media item.
+        /// </value>
+        public new T Content { get; }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="MediaWithCrops{T}" /> class.
+        /// </summary>
+        /// <param name="content">The content.</param>
+        /// <param name="localCrops">The local crops.</param>
+        public MediaWithCrops(T content, ImageCropperValue localCrops)
+            : base(content, localCrops)
+        {
+            Content = content;
+        }
+
+        /// <summary>
+        /// Performs an implicit conversion from <see cref="MediaWithCrops{T}" /> to <see cref="T" />.
+        /// </summary>
+        /// <param name="mediaWithCrops">The media with crops.</param>
+        /// <returns>
+        /// The result of the conversion.
+        /// </returns>
+        public static implicit operator T(MediaWithCrops<T> mediaWithCrops) => mediaWithCrops.Content;
     }
 }

src/Umbraco.Core/PropertyEditors/ImageCropperConfiguration.cs

@@ -1,4 +1,8 @@
 using Newtonsoft.Json;
+using System.Collections.Generic;
+using System.Linq;
+using Umbraco.Core.PropertyEditors.ValueConverters;
+using static Umbraco.Core.PropertyEditors.ValueConverters.ImageCropperValue;
 
 namespace Umbraco.Core.PropertyEditors
 {
@@ -22,4 +26,35 @@ public class Crop
             public int Height { get; set; }
         }
     }
+
+    internal static class ImageCropperConfigurationExtensions
+    {
+        /// <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 static void ApplyConfiguration(this ImageCropperValue imageCropperValue, ImageCropperConfiguration configuration)
+        {
+            var crops = new List<ImageCropperCrop>();
+
+            var configuredCrops = configuration?.Crops;
+            if (configuredCrops != null)
+            {
+                foreach (var configuredCrop in configuredCrops)
+                {
+                    var crop = imageCropperValue.Crops?.FirstOrDefault(x => x.Alias == configuredCrop.Alias);
+
+                    crops.Add(new ImageCropperCrop
+                    {
+                        Alias = configuredCrop.Alias,
+                        Width = configuredCrop.Width,
+                        Height = configuredCrop.Height,
+                        Coordinates = crop?.Coordinates
+                    });
+                }
+            }
+
+            imageCropperValue.Crops = crops;
+        }
+    }
 }

src/Umbraco.Core/PropertyEditors/ValueConverters/ImageCropperValue.cs

@@ -140,54 +140,43 @@ public bool HasFocalPoint()
         /// Determines whether the value has a specified crop.
         /// </summary>
         public bool HasCrop(string alias)
-            => Crops.Any(x => x.Alias == alias);
+            => Crops != null && Crops.Any(x => x.Alias == alias);
 
         /// <summary>
         /// Determines whether the value has a source image.
         /// </summary>
         public bool HasImage()
             => !string.IsNullOrWhiteSpace(Src);
 
-        /// <summary>
-        /// Applies a configuration.
-        /// </summary>
-        /// <remarks>Ensures that all crops defined in the configuration exists in the value.</remarks>
-        internal void ApplyConfiguration(ImageCropperConfiguration configuration)
+        internal ImageCropperValue Merge(ImageCropperValue imageCropperValue)
         {
-            // merge the crop values - the alias + width + height comes from
-            // configuration, but each crop can store its own coordinates
-
-            var configuredCrops = configuration?.Crops;
-            if (configuredCrops == null) return;
-
-            //Use Crops if it's not null, otherwise create a new list
             var crops = Crops?.ToList() ?? new List<ImageCropperCrop>();
 
-            foreach (var configuredCrop in configuredCrops)
+            var incomingCrops = imageCropperValue?.Crops;
+            if (incomingCrops != null)
             {
-                var crop = crops.FirstOrDefault(x => x.Alias == configuredCrop.Alias);
-                if (crop != null)
+                foreach (var incomingCrop in incomingCrops)
                 {
-                    // found, apply the height & width
-                    crop.Width = configuredCrop.Width;
-                    crop.Height = configuredCrop.Height;
-                }
-                else
-                {
-                    // not found, add
-                    crops.Add(new ImageCropperCrop
+                    var crop = crops.FirstOrDefault(x => x.Alias == incomingCrop.Alias);
+                    if (crop == null)
+                    {
+                        // Add incoming crop
+                        crops.Add(incomingCrop);
+                    }
+                    else if (crop.Coordinates == null)
                     {
-                        Alias = configuredCrop.Alias,
-                        Width = configuredCrop.Width,
-                        Height = configuredCrop.Height
-                    });
+                        // Use incoming crop coordinates
+                        crop.Coordinates = incomingCrop.Coordinates;
+                    }
                 }
             }
 
-            // assume we don't have to remove the crops in value, that
-            // are not part of configuration anymore?
-
-            Crops = crops;
+            return new ImageCropperValue()
+            {
+                Src = !string.IsNullOrWhiteSpace(Src) ? Src : imageCropperValue?.Src,
+                Crops = crops,
+                FocalPoint = FocalPoint ?? imageCropperValue?.FocalPoint
+            };
         }
 
         #region IEquatable

src/Umbraco.Tests/PropertyEditors/ImageCropperTest.cs

@@ -82,8 +82,20 @@ public void CanConvertImageCropperPropertyEditor(string val1, string val2, bool
 
                 var mediaFileSystem = new MediaFileSystem(Mock.Of<IFileSystem>(), config, scheme, logger);
 
+                var imageCropperConfiguration = new ImageCropperConfiguration()
+                {
+                    Crops = new[]
+                    {
+                        new ImageCropperConfiguration.Crop()
+                        {
+                            Alias = "thumb",
+                            Width = 100,
+                            Height = 100
+                        }
+                    }
+                };
                 var dataTypeService = new TestObjects.TestDataTypeService(
-                    new DataType(new ImageCropperPropertyEditor(Mock.Of<ILogger>(), mediaFileSystem, Mock.Of<IContentSection>(), Mock.Of<IDataTypeService>())) { Id = 1 });
+                    new DataType(new ImageCropperPropertyEditor(Mock.Of<ILogger>(), mediaFileSystem, Mock.Of<IContentSection>(), Mock.Of<IDataTypeService>())) { Id = 1, Configuration = imageCropperConfiguration });
 
                 var factory = new PublishedContentTypeFactory(Mock.Of<IPublishedModelFactory>(), new PropertyValueConverterCollection(Array.Empty<IPropertyValueConverter>()), dataTypeService);