From 26f1e77813642f3c2badc9bf3ef2a1cd7e1a0383 Mon Sep 17 00:00:00 2001 From: Rasmus John Pedersen Date: Wed, 30 Oct 2019 21:57:50 +0100 Subject: [PATCH] Resolve media paths from data editors # Conflicts: # src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs # src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs --- src/Umbraco.Core/ContentExtensions.cs | 15 ++++---- src/Umbraco.Core/Models/MediaExtensions.cs | 35 ++++-------------- .../Factories/ContentBaseFactory.cs | 3 +- .../IDataEditorWithMediaPath.cs | 6 +-- .../Routing/MediaUrlProviderTests.cs | 29 +++++++++++---- .../Services/MediaServiceTests.cs | 2 +- .../Entities/MockedContentTypes.cs | 37 +++++++++++++++++-- .../FileUploadPropertyEditor.cs | 5 +-- .../ImageCropperPropertyEditor.cs | 10 ++--- .../Routing/DefaultMediaUrlProvider.cs | 32 +++++++++------- 10 files changed, 102 insertions(+), 72 deletions(-) diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index a7d40b0b7d..3edad0c963 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -13,6 +13,7 @@ using Umbraco.Core.IO; using Umbraco.Core.Models; using Umbraco.Core.Models.Entities; using Umbraco.Core.Models.Membership; +using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; @@ -23,6 +24,8 @@ namespace Umbraco.Core // this ain't pretty private static IMediaFileSystem _mediaFileSystem; private static IMediaFileSystem MediaFileSystem => _mediaFileSystem ?? (_mediaFileSystem = Current.MediaFileSystem); + private static readonly PropertyEditorCollection _propertyEditors; + private static PropertyEditorCollection PropertyEditors = _propertyEditors ?? (_propertyEditors = Current.PropertyEditors); #region IContent @@ -162,14 +165,12 @@ namespace Umbraco.Core // Fixes https://github.com/umbraco/Umbraco-CMS/issues/3937 - Assigning a new file to an // existing IMedia with extension SetValue causes exception 'Illegal characters in path' string oldpath = null; - if (property.GetValue(culture, segment) is string svalue) + var value = property.GetValue(culture, segment); + + if (PropertyEditors.TryGet(propertyTypeAlias, out var editor) + && editor is IDataEditorWithMediaPath dataEditor) { - if (svalue.DetectIsJson()) - { - // the property value is a JSON serialized image crop data set - grab the "src" property as the file source - var jObject = JsonConvert.DeserializeObject(svalue); - svalue = jObject != null ? jObject.GetValueAsString("src") : svalue; - } + var svalue = dataEditor.GetMediaPath(value); oldpath = MediaFileSystem.GetRelativePath(svalue); } diff --git a/src/Umbraco.Core/Models/MediaExtensions.cs b/src/Umbraco.Core/Models/MediaExtensions.cs index 1166698adb..96f183b6e6 100644 --- a/src/Umbraco.Core/Models/MediaExtensions.cs +++ b/src/Umbraco.Core/Models/MediaExtensions.cs @@ -1,10 +1,8 @@ -using System; -using System.Linq; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; +using System.Linq; +using Umbraco.Core.Composing; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Logging; -using Umbraco.Core.PropertyEditors.ValueConverters; +using Umbraco.Core.PropertyEditors; namespace Umbraco.Core.Models { @@ -18,29 +16,12 @@ namespace Umbraco.Core.Models if (!media.Properties.TryGetValue(propertyAlias, out var property)) return string.Empty; - // TODO: would need to be adjusted to variations, when media become variants - if (!(property.GetValue() is string jsonString)) - return string.Empty; - - if (property.PropertyType.PropertyEditorAlias == Constants.PropertyEditors.Aliases.UploadField) - return jsonString; - - if (property.PropertyType.PropertyEditorAlias == Constants.PropertyEditors.Aliases.ImageCropper) + if (Current.PropertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out var editor) + && editor is IDataEditorWithMediaPath dataEditor) { - if (jsonString.DetectIsJson() == false) - return jsonString; - - try - { - var json = JsonConvert.DeserializeObject(jsonString); - if (json["src"] != null) - return json["src"].Value(); - } - catch (Exception ex) - { - logger.Error(ex, "Could not parse the string '{JsonString}' to a json object", jsonString); - return string.Empty; - } + // TODO: would need to be adjusted to variations, when media become variants + var value = property.GetValue(); + return dataEditor.GetMediaPath(value); } // Without knowing what it is, just adding a string here might not be very nice diff --git a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs index f9221ea576..2b2bed1d9e 100644 --- a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs @@ -300,7 +300,8 @@ namespace Umbraco.Core.Persistence.Factories && propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out var editor) && editor is IDataEditorWithMediaPath dataEditor) { - path = dataEditor.GetMediaPath(property); + var value = property.GetValue(); + path = dataEditor.GetMediaPath(value); } var dto = new MediaVersionDto diff --git a/src/Umbraco.Core/PropertyEditors/IDataEditorWithMediaPath.cs b/src/Umbraco.Core/PropertyEditors/IDataEditorWithMediaPath.cs index af42ebf4d6..aea7d28758 100644 --- a/src/Umbraco.Core/PropertyEditors/IDataEditorWithMediaPath.cs +++ b/src/Umbraco.Core/PropertyEditors/IDataEditorWithMediaPath.cs @@ -1,9 +1,7 @@ -using Umbraco.Core.Models; - -namespace Umbraco.Core.PropertyEditors +namespace Umbraco.Core.PropertyEditors { public interface IDataEditorWithMediaPath : IDataEditor { - string GetMediaPath(Property property, string culture = null, string segment = null); + string GetMediaPath(object value); } } diff --git a/src/Umbraco.Tests/Routing/MediaUrlProviderTests.cs b/src/Umbraco.Tests/Routing/MediaUrlProviderTests.cs index 6489417dc7..2f960d498d 100644 --- a/src/Umbraco.Tests/Routing/MediaUrlProviderTests.cs +++ b/src/Umbraco.Tests/Routing/MediaUrlProviderTests.cs @@ -4,13 +4,18 @@ using Moq; using Newtonsoft.Json; using NUnit.Framework; using Umbraco.Core; +using Umbraco.Core.Configuration.UmbracoSettings; +using Umbraco.Core.IO; +using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.PropertyEditors; using Umbraco.Core.PropertyEditors.ValueConverters; +using Umbraco.Core.Services; using Umbraco.Tests.PublishedContent; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.Testing; +using Umbraco.Web.PropertyEditors; using Umbraco.Web.Routing; namespace Umbraco.Tests.Routing @@ -25,7 +30,17 @@ namespace Umbraco.Tests.Routing { base.SetUp(); - _mediaUrlProvider = new DefaultMediaUrlProvider(); + var logger = Mock.Of(); + var mediaFileSystemMock = Mock.Of(); + var contentSection = Mock.Of(); + var dataTypeService = Mock.Of(); + + var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(new IDataEditor[] + { + new FileUploadPropertyEditor(logger, mediaFileSystemMock, contentSection), + new ImageCropperPropertyEditor(logger, mediaFileSystemMock, contentSection, dataTypeService), + })); + _mediaUrlProvider = new DefaultMediaUrlProvider(propertyEditors); } public override void TearDown() @@ -54,10 +69,10 @@ namespace Umbraco.Tests.Routing const string expected = "/media/rfeiw584/test.jpg"; var configuration = new ImageCropperConfiguration(); - var imageCropperValue = new ImageCropperValue + var imageCropperValue = JsonConvert.SerializeObject(new ImageCropperValue { Src = expected - }; + }); var umbracoContext = GetUmbracoContext("/", mediaUrlProviders: new[] { _mediaUrlProvider }); var publishedContent = CreatePublishedContent(Constants.PropertyEditors.Aliases.ImageCropper, imageCropperValue, configuration); @@ -121,8 +136,8 @@ namespace Umbraco.Tests.Routing PropertyType = umbracoFilePropertyType, }; - property.SetValue("en", enMediaUrl, true); - property.SetValue("da", daMediaUrl); + property.SetSourceValue("en", enMediaUrl, true); + property.SetSourceValue("da", daMediaUrl); var contentType = new PublishedContentType(666, "alias", PublishedItemType.Content, Enumerable.Empty(), new [] { umbracoFilePropertyType }, ContentVariation.Culture); var publishedContent = new SolidPublishedContent(contentType) {Properties = new[] {property}}; @@ -131,7 +146,7 @@ namespace Umbraco.Tests.Routing Assert.AreEqual(daMediaUrl, resolvedUrl); } - private static IPublishedContent CreatePublishedContent(string propertyEditorAlias, object propertyValue, object dataTypeConfiguration) + private static IPublishedContent CreatePublishedContent(string propertyEditorAlias, string propertyValue, object dataTypeConfiguration) { var umbracoFilePropertyType = CreatePropertyType(propertyEditorAlias, dataTypeConfiguration, ContentVariation.Nothing); @@ -147,7 +162,7 @@ namespace Umbraco.Tests.Routing new SolidPublishedProperty { Alias = "umbracoFile", - SolidValue = propertyValue, + SolidSourceValue = propertyValue, SolidHasValue = true, PropertyType = umbracoFilePropertyType } diff --git a/src/Umbraco.Tests/Services/MediaServiceTests.cs b/src/Umbraco.Tests/Services/MediaServiceTests.cs index 17711fbd31..b3dc274c5e 100644 --- a/src/Umbraco.Tests/Services/MediaServiceTests.cs +++ b/src/Umbraco.Tests/Services/MediaServiceTests.cs @@ -184,7 +184,7 @@ namespace Umbraco.Tests.Services public void Can_Get_Media_With_Crop_By_Path() { var mediaService = ServiceContext.MediaService; - var mediaType = MockedContentTypes.CreateImageMediaType("Image2"); + var mediaType = MockedContentTypes.CreateImageMediaTypeWithCrop("Image2"); ServiceContext.MediaTypeService.Save(mediaType); var media = MockedMedia.CreateMediaImageWithCrop(mediaType, -1); diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs index e93e8e8740..41786a5fd7 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs @@ -420,10 +420,39 @@ namespace Umbraco.Tests.TestHelpers.Entities var contentCollection = new PropertyTypeCollection(false); contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.UploadField, ValueStorageType.Nvarchar) { Alias = Constants.Conventions.Media.File, Name = "File", Description = "", Mandatory = false, SortOrder = 1, DataTypeId = -90 }); - contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer) { Alias = Constants.Conventions.Media.Width, Name = "Width", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -90 }); - contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer) { Alias = Constants.Conventions.Media.Height, Name = "Height", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -90 }); - contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer) { Alias = Constants.Conventions.Media.Bytes, Name = "Bytes", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -90 }); - contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Nvarchar) { Alias = Constants.Conventions.Media.Extension, Name = "File Extension", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -90 }); + contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer) { Alias = Constants.Conventions.Media.Width, Name = "Width", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -92 }); + contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer) { Alias = Constants.Conventions.Media.Height, Name = "Height", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -92 }); + contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer) { Alias = Constants.Conventions.Media.Bytes, Name = "Bytes", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -92 }); + contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Nvarchar) { Alias = Constants.Conventions.Media.Extension, Name = "File Extension", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -92 }); + + mediaType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Media", SortOrder = 1 }); + + //ensure that nothing is marked as dirty + mediaType.ResetDirtyProperties(false); + + return mediaType; + } + + public static MediaType CreateImageMediaTypeWithCrop(string alias = Constants.Conventions.MediaTypes.Image) + { + var mediaType = new MediaType(-1) + { + Alias = alias, + Name = "Image", + Description = "ContentType used for images", + Icon = ".sprTreeDoc3", + Thumbnail = "doc.png", + SortOrder = 1, + CreatorId = 0, + Trashed = false + }; + + var contentCollection = new PropertyTypeCollection(false); + contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.ImageCropper, ValueStorageType.Ntext) { Alias = Constants.Conventions.Media.File, Name = "File", Description = "", Mandatory = false, SortOrder = 1, DataTypeId = 1043 }); + contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer) { Alias = Constants.Conventions.Media.Width, Name = "Width", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -92 }); + contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer) { Alias = Constants.Conventions.Media.Height, Name = "Height", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -92 }); + contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Integer) { Alias = Constants.Conventions.Media.Bytes, Name = "Bytes", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -92 }); + contentCollection.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Nvarchar) { Alias = Constants.Conventions.Media.Extension, Name = "File Extension", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -92 }); mediaType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Media", SortOrder = 1 }); diff --git a/src/Umbraco.Web/PropertyEditors/FileUploadPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/FileUploadPropertyEditor.cs index 3efeb01e6e..052af18aa1 100644 --- a/src/Umbraco.Web/PropertyEditors/FileUploadPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/FileUploadPropertyEditor.cs @@ -43,8 +43,7 @@ namespace Umbraco.Web.PropertyEditors return editor; } - public string GetMediaPath(Property property, string culture = null, string segment = null) => - property.GetValue(culture)?.ToString(); + public string GetMediaPath(object value) => value?.ToString(); /// /// Gets a value indicating whether a property is an upload field. @@ -55,7 +54,7 @@ namespace Umbraco.Web.PropertyEditors { return property.PropertyType.PropertyEditorAlias == Constants.PropertyEditors.Aliases.UploadField; } - + /// /// Ensures any files associated are removed /// diff --git a/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyEditor.cs index 1a4845c74e..b0e5bf30bd 100644 --- a/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyEditor.cs @@ -48,8 +48,7 @@ namespace Umbraco.Web.PropertyEditors _autoFillProperties = new UploadAutoFillProperties(_mediaFileSystem, logger, _contentSettings); } - public string GetMediaPath(Property property, string culture = null, string segment = null) => - GetFileSrcFromPropertyValue(property.GetValue(culture), out _); + public string GetMediaPath(object value) => GetFileSrcFromPropertyValue(value, out _, false); /// /// Creates the corresponding property value editor. @@ -66,7 +65,7 @@ namespace Umbraco.Web.PropertyEditors /// /// Gets a value indicating whether a property is an image cropper field. /// - /// The property. + /// The property. /// A value indicating whether a property is an image cropper field, and (optionally) has a non-empty value. private static bool IsCropperField(Property property) { @@ -135,8 +134,9 @@ namespace Umbraco.Web.PropertyEditors /// /// /// The deserialized value + /// Should the path returned be the application relative path /// - private string GetFileSrcFromPropertyValue(object propVal, out JObject deserializedValue) + private string GetFileSrcFromPropertyValue(object propVal, out JObject deserializedValue, bool relative = true) { deserializedValue = null; if (propVal == null || !(propVal is string str)) return null; @@ -144,7 +144,7 @@ namespace Umbraco.Web.PropertyEditors deserializedValue = GetJObject(str, true); if (deserializedValue?["src"] == null) return null; var src = deserializedValue["src"].Value(); - return _mediaFileSystem.GetRelativePath(src); + return relative ? _mediaFileSystem.GetRelativePath(src) : src; } /// diff --git a/src/Umbraco.Web/Routing/DefaultMediaUrlProvider.cs b/src/Umbraco.Web/Routing/DefaultMediaUrlProvider.cs index 02dc4ebf29..dacd314f94 100644 --- a/src/Umbraco.Web/Routing/DefaultMediaUrlProvider.cs +++ b/src/Umbraco.Web/Routing/DefaultMediaUrlProvider.cs @@ -1,7 +1,7 @@ using System; -using Umbraco.Core; +using Umbraco.Core.Composing; using Umbraco.Core.Models.PublishedContent; -using Umbraco.Core.PropertyEditors.ValueConverters; +using Umbraco.Core.PropertyEditors; namespace Umbraco.Web.Routing { @@ -10,13 +10,24 @@ namespace Umbraco.Web.Routing /// public class DefaultMediaUrlProvider : IMediaUrlProvider { + private readonly PropertyEditorCollection _propertyEditors; + + public DefaultMediaUrlProvider(PropertyEditorCollection propertyEditors) + { + _propertyEditors = propertyEditors ?? throw new ArgumentNullException(nameof(propertyEditors)); + } + + [Obsolete("Use the constructor with all parameters instead")] + public DefaultMediaUrlProvider() : this(Current.PropertyEditors) + { + } + /// public virtual UrlInfo GetMediaUrl(UmbracoContext umbracoContext, IPublishedContent content, - string propertyAlias, - UrlMode mode, string culture, Uri current) + string propertyAlias, UrlMode mode, string culture, Uri current) { var prop = content.GetProperty(propertyAlias); - var value = prop?.GetValue(culture); + var value = prop?.GetSourceValue(culture); if (value == null) { return null; @@ -25,15 +36,10 @@ namespace Umbraco.Web.Routing var propType = prop.PropertyType; string path = null; - switch (propType.EditorAlias) + if (_propertyEditors.TryGet(propType.EditorAlias, out var editor) + && editor is IDataEditorWithMediaPath dataEditor) { - case Constants.PropertyEditors.Aliases.UploadField: - path = value.ToString(); - break; - case Constants.PropertyEditors.Aliases.ImageCropper: - //get the url from the json format - path = value is ImageCropperValue stronglyTyped ? stronglyTyped.Src : value.ToString(); - break; + path = dataEditor.GetMediaPath(value); } var url = AssembleUrl(path, current, mode);