From e7f457d668d48d1d635b94ddde8df15a7be549c1 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 3 May 2024 08:19:55 +0200 Subject: [PATCH 1/9] Allow FileUpload and Imagecropper to upload files in the same way --- .../FileUploadPropertyEditor.cs | 67 ++++++++++- .../FileUploadPropertyValueEditor.cs | 108 +++++++++++++++--- .../ImageCropperPropertyValueEditor.cs | 10 +- .../ValueConverters/FileUploadValue.cs | 5 + .../ValueConverters/ImageCropperValue.cs | 7 +- .../TemporaryFileUploadValueBase.cs | 17 +++ 6 files changed, 190 insertions(+), 24 deletions(-) create mode 100644 src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/FileUploadValue.cs create mode 100644 src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs diff --git a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs index 1d1b5a633f1f..d2d2b2e4e94e 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs @@ -2,13 +2,18 @@ // See LICENSE for more details. using System.Diagnostics.CodeAnalysis; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Media; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; +using Umbraco.Cms.Core.PropertyEditors.ValueConverters; +using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; using Umbraco.Extensions; @@ -16,6 +21,7 @@ namespace Umbraco.Cms.Core.PropertyEditors; [DataEditor( Constants.PropertyEditors.Aliases.UploadField, + ValueType = ValueTypes.Json, ValueEditorIsReusable = true)] public class FileUploadPropertyEditor : DataEditor, IMediaUrlGenerator, INotificationHandler, INotificationHandler, @@ -25,9 +31,12 @@ public class FileUploadPropertyEditor : DataEditor, IMediaUrlGenerator, private readonly IContentService _contentService; private readonly IOptionsMonitor _contentSettings; private readonly IIOHelper _ioHelper; + private readonly IJsonSerializer _jsonSerializer; + private readonly ILogger _logger; private readonly MediaFileManager _mediaFileManager; private readonly UploadAutoFillProperties _uploadAutoFillProperties; + [Obsolete("Use non-obsolete constructor. This constructor will be removed V15.")] public FileUploadPropertyEditor( IDataValueEditorFactory dataValueEditorFactory, MediaFileManager mediaFileManager, @@ -35,6 +44,29 @@ public FileUploadPropertyEditor( UploadAutoFillProperties uploadAutoFillProperties, IContentService contentService, IIOHelper ioHelper) + :this( + dataValueEditorFactory, + mediaFileManager, + contentSettings, + uploadAutoFillProperties, + contentService, + ioHelper, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService>() + ) + { + + } + + public FileUploadPropertyEditor( + IDataValueEditorFactory dataValueEditorFactory, + MediaFileManager mediaFileManager, + IOptionsMonitor contentSettings, + UploadAutoFillProperties uploadAutoFillProperties, + IContentService contentService, + IIOHelper ioHelper, + IJsonSerializer jsonSerializer, + ILogger logger) : base(dataValueEditorFactory) { _mediaFileManager = mediaFileManager ?? throw new ArgumentNullException(nameof(mediaFileManager)); @@ -42,13 +74,15 @@ public FileUploadPropertyEditor( _uploadAutoFillProperties = uploadAutoFillProperties; _contentService = contentService; _ioHelper = ioHelper; + _jsonSerializer = jsonSerializer; + _logger = logger; SupportsReadOnly = true; } public bool TryGetMediaPath(string? propertyEditorAlias, object? value, [MaybeNullWhen(false)] out string mediaPath) { if (propertyEditorAlias == Alias && - value?.ToString() is var mediaPathValue && + TryDeserializeMediaPath(value, out var mediaPathValue) && !string.IsNullOrWhiteSpace(mediaPathValue)) { mediaPath = mediaPathValue; @@ -59,6 +93,37 @@ public bool TryGetMediaPath(string? propertyEditorAlias, object? value, [MaybeNu return false; } + private bool TryDeserializeMediaPath(object? propVal, out string? mediaPath) + { + if (propVal is not string stringValue) + { + mediaPath = null; + return false; + } + + if (!stringValue.DetectIsJson()) + { + // Assume the value is a plain string with the file path + mediaPath = stringValue; + } + else + { + try + { + mediaPath = _jsonSerializer.Deserialize(stringValue)?.Src; + return true; + } + catch (Exception ex) + { + _logger.LogError(ex, "Could not parse file upload value '{Json}'", stringValue); + } + } + + mediaPath = string.Empty; + return false; + } + + public void Handle(ContentCopiedNotification notification) { // get the upload field properties with a value diff --git a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs index 8bf735f898b0..ce2435a54945 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs @@ -1,11 +1,15 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.Models.TemporaryFile; +using Umbraco.Cms.Core.PropertyEditors.ValueConverters; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; @@ -21,11 +25,14 @@ namespace Umbraco.Cms.Core.PropertyEditors; internal class FileUploadPropertyValueEditor : DataValueEditor { private readonly MediaFileManager _mediaFileManager; + private readonly IJsonSerializer _jsonSerializer; private readonly ITemporaryFileService _temporaryFileService; private readonly IScopeProvider _scopeProvider; private readonly IFileStreamSecurityValidator _fileStreamSecurityValidator; + private readonly ILogger _logger; private ContentSettings _contentSettings; + [Obsolete("Use the non-obsolete constructor, scheduled for removal in V15")] public FileUploadPropertyValueEditor( DataEditorAttribute attribute, MediaFileManager mediaFileManager, @@ -37,12 +44,30 @@ public FileUploadPropertyValueEditor( ITemporaryFileService temporaryFileService, IScopeProvider scopeProvider, IFileStreamSecurityValidator fileStreamSecurityValidator) + :this(attribute, mediaFileManager, localizedTextService, shortStringHelper, contentSettings, jsonSerializer, ioHelper, temporaryFileService, scopeProvider, fileStreamSecurityValidator, StaticServiceProvider.Instance.GetRequiredService>()) + { + + } + + public FileUploadPropertyValueEditor( + DataEditorAttribute attribute, + MediaFileManager mediaFileManager, + ILocalizedTextService localizedTextService, + IShortStringHelper shortStringHelper, + IOptionsMonitor contentSettings, + IJsonSerializer jsonSerializer, + IIOHelper ioHelper, + ITemporaryFileService temporaryFileService, + IScopeProvider scopeProvider, + IFileStreamSecurityValidator fileStreamSecurityValidator, ILogger logger) : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) { _mediaFileManager = mediaFileManager ?? throw new ArgumentNullException(nameof(mediaFileManager)); + _jsonSerializer = jsonSerializer; _temporaryFileService = temporaryFileService; _scopeProvider = scopeProvider; _fileStreamSecurityValidator = fileStreamSecurityValidator; + _logger = logger; _contentSettings = contentSettings.CurrentValue ?? throw new ArgumentNullException(nameof(contentSettings)); contentSettings.OnChange(x => _contentSettings = x); @@ -70,39 +95,46 @@ public FileUploadPropertyValueEditor( /// public override object? FromEditor(ContentPropertyData editorValue, object? currentValue) { - var currentStringValue = currentValue as string; - currentStringValue = currentStringValue.NullOrWhiteSpaceAsNull(); - var editorStringValue = editorValue.Value as string; - editorStringValue = editorStringValue.NullOrWhiteSpaceAsNull(); + FileUploadValue? currentModelValue = null; + var currentPath = string.Empty; + try + { + if (currentValue is string currentStringValue) + { + currentModelValue = TryParseFileUploadValue(currentStringValue); + } + } + 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 an ImageCropperValue object."); + } + // no change? - if (editorStringValue == currentStringValue) + FileUploadValue? editorModelValue = TryParseFileUploadValue(editorValue.Value as string); + + if (Equals(editorModelValue, currentModelValue)) { return currentValue; } - var currentPath = currentStringValue; if (currentPath.IsNullOrWhiteSpace() == false) { currentPath = _mediaFileManager.FileSystem.GetRelativePath(currentPath); } // resetting the current value? - if (editorStringValue is null && currentPath.IsNullOrWhiteSpace() is false) + if (editorModelValue is null && currentPath.IsNullOrWhiteSpace() is false) { // delete the current file and clear the value of this property _mediaFileManager.FileSystem.DeleteFile(currentPath); return null; } - // uploading a file? - if (Guid.TryParse(editorStringValue, out Guid temporaryFileKey) == false) - { - return editorStringValue; - } - - TemporaryFileModel? file = TryGetTemporaryFile(temporaryFileKey); + var temporaryFileKey = editorModelValue?.TemporaryFileId; + TemporaryFileModel? file = temporaryFileKey is null ? null : TryGetTemporaryFile(temporaryFileKey!.Value); if (file == null) { // at this point the temporary file *should* have been validated by TemporaryFileUploadValidator, so we @@ -113,7 +145,7 @@ public FileUploadPropertyValueEditor( // schedule temporary file for deletion using IScope scope = _scopeProvider.CreateScope(); - _temporaryFileService.EnlistDeleteIfScopeCompletes(temporaryFileKey, _scopeProvider); + _temporaryFileService.EnlistDeleteIfScopeCompletes(temporaryFileKey!.Value, _scopeProvider); // ensure we have the required guids Guid contentKey = editorValue.ContentKey; @@ -139,7 +171,51 @@ public FileUploadPropertyValueEditor( scope.Complete(); - return filepath == null ? null : _mediaFileManager.FileSystem.GetUrl(filepath); + return + _jsonSerializer.Serialize(new FileUploadValue() + { + TemporaryFileId = null, + Src = filepath == null ? null : _mediaFileManager.FileSystem.GetUrl(filepath) + }); + + } + + private FileUploadValue? TryParseFileUploadValue(string? value) + { + try + { + if (value is null) + { + return null; + } + + if(_jsonSerializer.TryDeserialize(value, out FileUploadValue? modelValue)) + { + return modelValue; + } + + if(Guid.TryParse(value, out Guid temporaryFileId)) + { + return new FileUploadValue() + { + TemporaryFileId = temporaryFileId, + Src = null, + }; + } + + return new FileUploadValue() + { + TemporaryFileId = null, + Src = value, + }; + } + 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 FileUploadValue object."); + } + + return null; } private Guid? TryParseTemporaryFileKey(object? editorValue) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs index ef3b9883a034..48c9a8075c67 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs @@ -149,7 +149,7 @@ public ImageCropperPropertyValueEditor( using IScope scope = _scopeProvider.CreateScope(); TemporaryFileModel? file = null; - Guid? temporaryFileKey = TryParseTemporaryFileKey(editorImageCropperValue); + Guid? temporaryFileKey = editorImageCropperValue?.TemporaryFileId; if (temporaryFileKey.HasValue) { file = TryGetTemporaryFile(temporaryFileKey.Value); @@ -196,6 +196,7 @@ public ImageCropperPropertyValueEditor( } editorImageCropperValue.Src = filepath is null ? string.Empty : _mediaFileManager.FileSystem.GetUrl(filepath); + editorImageCropperValue.TemporaryFileId = null; return _jsonSerializer.Serialize(editorImageCropperValue); } @@ -231,6 +232,13 @@ public override string ConvertDbToString(IPropertyType propertyType, object? val } imageCropperValue.Prune(); + + if(Guid.TryParse(imageCropperValue.Src, out Guid temporaryFileId)) + { + imageCropperValue.TemporaryFileId = temporaryFileId; + imageCropperValue.Src = null; + } + return imageCropperValue; } catch (Exception ex) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/FileUploadValue.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/FileUploadValue.cs new file mode 100644 index 000000000000..b89465801269 --- /dev/null +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/FileUploadValue.cs @@ -0,0 +1,5 @@ +namespace Umbraco.Cms.Core.PropertyEditors.ValueConverters; + +public class FileUploadValue : TemporaryFileUploadValueBase +{ +} diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValue.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValue.cs index 5c9eafd5d6cd..9b4ff3881198 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValue.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/ImageCropperValue.cs @@ -11,13 +11,8 @@ namespace Umbraco.Cms.Core.PropertyEditors.ValueConverters; /// /// Represents a value of the image cropper value editor. /// -public class ImageCropperValue : IHtmlEncodedString, IEquatable +public class ImageCropperValue :TemporaryFileUploadValueBase, IHtmlEncodedString, IEquatable { - /// - /// Gets or sets the value source image. - /// - public string? Src { get; set; } = string.Empty; - /// /// Gets or sets the value focal point. /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs new file mode 100644 index 000000000000..46a75e36d3f5 --- /dev/null +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs @@ -0,0 +1,17 @@ +using System.Text.Json.Serialization; + +namespace Umbraco.Cms.Core.PropertyEditors.ValueConverters; + +public abstract class TemporaryFileUploadValueBase +{ + /// + /// Gets or sets the temporary file identifier that will replace an an existing value. + /// + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public Guid? TemporaryFileId { get; set; } + + /// + /// Gets or sets the value source image. + /// + public string? Src { get; set; } = string.Empty; +} From 279e198d7914839a55873c84160a11bd44219039 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 7 May 2024 08:52:02 +0200 Subject: [PATCH 2/9] Fixed issue with backward compatibility --- .../PropertyEditors/FileUploadPropertyEditor.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs index d2d2b2e4e94e..39e022b8fae8 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs @@ -105,6 +105,7 @@ private bool TryDeserializeMediaPath(object? propVal, out string? mediaPath) { // Assume the value is a plain string with the file path mediaPath = stringValue; + return true; } else { From a093f8bdb9d6b96c7ddde340060ea459b01f3baf Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 7 May 2024 09:37:33 +0200 Subject: [PATCH 3/9] Exclude from converter --- .../PropertyEditors/ValueConverters/JsonValueConverter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/JsonValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/JsonValueConverter.cs index a12d4a82f88d..2ed38cf3522a 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/JsonValueConverter.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/JsonValueConverter.cs @@ -22,7 +22,7 @@ public class JsonValueConverter : PropertyValueConverterBase, IDeliveryApiProper private readonly ILogger _logger; private readonly PropertyEditorCollection _propertyEditors; - private readonly string[] _excludedPropertyEditors = { Constants.PropertyEditors.Aliases.MediaPicker3 }; + private readonly string[] _excludedPropertyEditors = { Constants.PropertyEditors.Aliases.MediaPicker3, Constants.PropertyEditors.Aliases.UploadField }; /// /// Initializes a new instance of the class. From d9d61c67b34cf02602cc46bb7da57b6f5f7601a6 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 7 May 2024 10:18:49 +0200 Subject: [PATCH 4/9] Bugfix if editorvalue is not a string --- .../FileUploadPropertyValueEditor.cs | 2 +- .../TemporaryFileUploadValueBase.cs | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs index ce2435a54945..a71655ce18a8 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs @@ -113,7 +113,7 @@ public FileUploadPropertyValueEditor( // no change? - FileUploadValue? editorModelValue = TryParseFileUploadValue(editorValue.Value as string); + FileUploadValue? editorModelValue = TryParseFileUploadValue(editorValue.Value?.ToString()); if (Equals(editorModelValue, currentModelValue)) { diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs index 46a75e36d3f5..ce01cba04233 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs @@ -14,4 +14,28 @@ public abstract class TemporaryFileUploadValueBase /// Gets or sets the value source image. /// public string? Src { get; set; } = string.Empty; + + protected bool Equals(TemporaryFileUploadValueBase other) => Nullable.Equals(TemporaryFileId, other.TemporaryFileId) && Src == other.Src; + + public override bool Equals(object? obj) + { + if (ReferenceEquals(null, obj)) + { + return false; + } + + if (ReferenceEquals(this, obj)) + { + return true; + } + + if (obj.GetType() != this.GetType()) + { + return false; + } + + return Equals((TemporaryFileUploadValueBase)obj); + } + + public override int GetHashCode() => HashCode.Combine(TemporaryFileId, Src); } From e4b9de8d6af66e2cf90afd18ea9e1d24fbff5165 Mon Sep 17 00:00:00 2001 From: kjac Date: Wed, 8 May 2024 10:24:50 +0200 Subject: [PATCH 5/9] Fix file upload value editor + clean up + throw explicitly if editor value is invalid --- .../FileUploadPropertyEditor.cs | 18 ++- .../FileUploadPropertyValueEditor.cs | 129 +++++------------- .../ImageCropperPropertyValueEditor.cs | 44 ++---- .../ValueConverters/FileUploadValue.cs | 2 +- 4 files changed, 51 insertions(+), 142 deletions(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs index 39e022b8fae8..ac7151ce53ec 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs @@ -107,17 +107,15 @@ private bool TryDeserializeMediaPath(object? propVal, out string? mediaPath) mediaPath = stringValue; return true; } - else + + try { - try - { - mediaPath = _jsonSerializer.Deserialize(stringValue)?.Src; - return true; - } - catch (Exception ex) - { - _logger.LogError(ex, "Could not parse file upload value '{Json}'", stringValue); - } + mediaPath = _jsonSerializer.Deserialize(stringValue)?.Src; + return true; + } + catch (Exception ex) + { + _logger.LogError(ex, "Could not parse file upload value '{Json}'", stringValue); } mediaPath = string.Empty; diff --git a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs index a71655ce18a8..466c456058b7 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyValueEditor.cs @@ -1,12 +1,10 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; -using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.IO; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.Models.TemporaryFile; using Umbraco.Cms.Core.PropertyEditors.ValueConverters; @@ -29,10 +27,8 @@ internal class FileUploadPropertyValueEditor : DataValueEditor private readonly ITemporaryFileService _temporaryFileService; private readonly IScopeProvider _scopeProvider; private readonly IFileStreamSecurityValidator _fileStreamSecurityValidator; - private readonly ILogger _logger; private ContentSettings _contentSettings; - [Obsolete("Use the non-obsolete constructor, scheduled for removal in V15")] public FileUploadPropertyValueEditor( DataEditorAttribute attribute, MediaFileManager mediaFileManager, @@ -44,22 +40,6 @@ public FileUploadPropertyValueEditor( ITemporaryFileService temporaryFileService, IScopeProvider scopeProvider, IFileStreamSecurityValidator fileStreamSecurityValidator) - :this(attribute, mediaFileManager, localizedTextService, shortStringHelper, contentSettings, jsonSerializer, ioHelper, temporaryFileService, scopeProvider, fileStreamSecurityValidator, StaticServiceProvider.Instance.GetRequiredService>()) - { - - } - - public FileUploadPropertyValueEditor( - DataEditorAttribute attribute, - MediaFileManager mediaFileManager, - ILocalizedTextService localizedTextService, - IShortStringHelper shortStringHelper, - IOptionsMonitor contentSettings, - IJsonSerializer jsonSerializer, - IIOHelper ioHelper, - ITemporaryFileService temporaryFileService, - IScopeProvider scopeProvider, - IFileStreamSecurityValidator fileStreamSecurityValidator, ILogger logger) : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) { _mediaFileManager = mediaFileManager ?? throw new ArgumentNullException(nameof(mediaFileManager)); @@ -67,7 +47,6 @@ public FileUploadPropertyValueEditor( _temporaryFileService = temporaryFileService; _scopeProvider = scopeProvider; _fileStreamSecurityValidator = fileStreamSecurityValidator; - _logger = logger; _contentSettings = contentSettings.CurrentValue ?? throw new ArgumentNullException(nameof(contentSettings)); contentSettings.OnChange(x => _contentSettings = x); @@ -78,8 +57,20 @@ public FileUploadPropertyValueEditor( IsAllowedInDataTypeConfiguration)); } + public override object? ToEditor(IProperty property, string? culture = null, string? segment = null) + { + // the stored property value (if any) is the path to the file; convert it to the client model (FileUploadValue) + var propertyValue = property.GetValue(culture, segment); + return propertyValue is string stringValue + ? new FileUploadValue + { + Src = stringValue + } + : null; + } + /// - /// Converts the value received from the editor into the value can be stored in the database. + /// Converts the client model (FileUploadValue) into the value can be stored in the database (the file path). /// /// The value received from the editor. /// The current value of the property @@ -88,54 +79,36 @@ public FileUploadPropertyValueEditor( /// The is used to re-use the folder, if possible. /// /// The is value passed in from the editor. If the value is empty, we - /// must delete the currently selected file (). If the value is not empty, - /// it is assumed to contain a temporary file key, and we will attempt to replace the currently selected - /// file with the corresponding temporary file. + /// must delete the currently selected file (). /// /// public override object? FromEditor(ContentPropertyData editorValue, object? currentValue) { - - FileUploadValue? currentModelValue = null; - var currentPath = string.Empty; - try - { - if (currentValue is string currentStringValue) - { - currentModelValue = TryParseFileUploadValue(currentStringValue); - } - } - 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 an ImageCropperValue object."); - } - + FileUploadValue? editorModelValue = ParseFileUploadValue(editorValue.Value); // no change? - FileUploadValue? editorModelValue = TryParseFileUploadValue(editorValue.Value?.ToString()); - - if (Equals(editorModelValue, currentModelValue)) + if (editorModelValue?.TemporaryFileId.HasValue is not true) { return currentValue; } - if (currentPath.IsNullOrWhiteSpace() == false) - { - currentPath = _mediaFileManager.FileSystem.GetRelativePath(currentPath); - } + // the current editor value (if any) is the path to the file + var currentPath = currentValue is string currentStringValue + && currentStringValue.IsNullOrWhiteSpace() is false + ? _mediaFileManager.FileSystem.GetRelativePath(currentStringValue) + : null; // resetting the current value? - if (editorModelValue is null && currentPath.IsNullOrWhiteSpace() is false) + if (editorModelValue?.Src is null && currentPath.IsNullOrWhiteSpace() is false) { // delete the current file and clear the value of this property _mediaFileManager.FileSystem.DeleteFile(currentPath); return null; } - var temporaryFileKey = editorModelValue?.TemporaryFileId; - TemporaryFileModel? file = temporaryFileKey is null ? null : TryGetTemporaryFile(temporaryFileKey!.Value); - if (file == null) + Guid? temporaryFileKey = editorModelValue?.TemporaryFileId; + TemporaryFileModel? file = temporaryFileKey is null ? null : TryGetTemporaryFile(temporaryFileKey.Value); + if (file is null) { // at this point the temporary file *should* have been validated by TemporaryFileUploadValidator, so we // should never end up here. In case we do, let's attempt to at least be non-destructive by returning @@ -171,57 +144,23 @@ public FileUploadPropertyValueEditor( scope.Complete(); - return - _jsonSerializer.Serialize(new FileUploadValue() - { - TemporaryFileId = null, - Src = filepath == null ? null : _mediaFileManager.FileSystem.GetUrl(filepath) - }); - + return filepath is null ? null : _mediaFileManager.FileSystem.GetUrl(filepath); } - private FileUploadValue? TryParseFileUploadValue(string? value) + private FileUploadValue? ParseFileUploadValue(object? editorValue) { - try - { - if (value is null) - { - return null; - } - - if(_jsonSerializer.TryDeserialize(value, out FileUploadValue? modelValue)) - { - return modelValue; - } - - if(Guid.TryParse(value, out Guid temporaryFileId)) - { - return new FileUploadValue() - { - TemporaryFileId = temporaryFileId, - Src = null, - }; - } - - return new FileUploadValue() - { - TemporaryFileId = null, - Src = value, - }; - } - catch (Exception ex) + if (editorValue is null) { - // 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 FileUploadValue object."); + return null; } - return null; + return _jsonSerializer.TryDeserialize(editorValue, out FileUploadValue? modelValue) + ? modelValue + : throw new ArgumentException($"Could not parse editor value to a {nameof(FileUploadValue)} object."); } private Guid? TryParseTemporaryFileKey(object? editorValue) - => editorValue is string stringValue && Guid.TryParse(stringValue, out Guid temporaryFileKey) - ? temporaryFileKey - : null; + => ParseFileUploadValue(editorValue)?.TemporaryFileId; private TemporaryFileModel? TryGetTemporaryFile(Guid temporaryFileKey) => _temporaryFileService.GetAsync(temporaryFileKey).GetAwaiter().GetResult(); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs index 48c9a8075c67..60fa7d088903 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyValueEditor.cs @@ -1,7 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System.Text.Json.Nodes; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Cache; @@ -131,7 +130,7 @@ public ImageCropperPropertyValueEditor( currentPath = _mediaFileManager.FileSystem.GetRelativePath(currentPath); } - ImageCropperValue? editorImageCropperValue = TryParseImageCropperValue(editorValue.Value); + ImageCropperValue? editorImageCropperValue = ParseImageCropperValue(editorValue.Value); // ensure we have the required guids Guid contentKey = editorValue.ContentKey; @@ -221,47 +220,20 @@ public override string ConvertDbToString(IPropertyType propertyType, object? val return _jsonSerializer.Serialize(new { src = val, crops }); } - private ImageCropperValue? TryParseImageCropperValue(object? editorValue) + private ImageCropperValue? ParseImageCropperValue(object? editorValue) { - try - { - if (editorValue is null || - _jsonSerializer.TryDeserialize(editorValue, out ImageCropperValue? imageCropperValue) is false) - { - return null; - } - - imageCropperValue.Prune(); - - if(Guid.TryParse(imageCropperValue.Src, out Guid temporaryFileId)) - { - imageCropperValue.TemporaryFileId = temporaryFileId; - imageCropperValue.Src = null; - } - - return imageCropperValue; - } - catch (Exception ex) + if (editorValue is null) { - // 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."); + return null; } - return null; + return _jsonSerializer.TryDeserialize(editorValue, out ImageCropperValue? imageCropperValue) + ? imageCropperValue + : throw new ArgumentException($"Could not parse editor value to a {nameof(ImageCropperValue)} object."); } private Guid? TryParseTemporaryFileKey(object? editorValue) - { - ImageCropperValue? imageCropperValue = TryParseImageCropperValue(editorValue); - return imageCropperValue != null - ? TryParseTemporaryFileKey(imageCropperValue) - : null; - } - - private Guid? TryParseTemporaryFileKey(ImageCropperValue? editorValue) - => Guid.TryParse(editorValue?.Src, out Guid temporaryFileKey) - ? temporaryFileKey - : null; + => ParseImageCropperValue(editorValue)?.TemporaryFileId; private TemporaryFileModel? TryGetTemporaryFile(Guid temporaryFileKey) => _temporaryFileService.GetAsync(temporaryFileKey).GetAwaiter().GetResult(); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/FileUploadValue.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/FileUploadValue.cs index b89465801269..ff7930d8335a 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/FileUploadValue.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/FileUploadValue.cs @@ -1,5 +1,5 @@ namespace Umbraco.Cms.Core.PropertyEditors.ValueConverters; -public class FileUploadValue : TemporaryFileUploadValueBase +public sealed class FileUploadValue : TemporaryFileUploadValueBase { } From 97e1ce81de44ae5e6586ff913349ac1ec82093bd Mon Sep 17 00:00:00 2001 From: kjac Date: Wed, 8 May 2024 10:29:11 +0200 Subject: [PATCH 6/9] Revert changes to FileUploadPropertyEditor - the stored property data format remains the file path --- .../FileUploadPropertyEditor.cs | 65 +------------------ 1 file changed, 1 insertion(+), 64 deletions(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs index ac7151ce53ec..d3fbfcd2b008 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs @@ -2,18 +2,13 @@ // See LICENSE for more details. using System.Diagnostics.CodeAnalysis; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; -using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Media; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; -using Umbraco.Cms.Core.PropertyEditors.ValueConverters; -using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; using Umbraco.Extensions; @@ -31,12 +26,9 @@ public class FileUploadPropertyEditor : DataEditor, IMediaUrlGenerator, private readonly IContentService _contentService; private readonly IOptionsMonitor _contentSettings; private readonly IIOHelper _ioHelper; - private readonly IJsonSerializer _jsonSerializer; - private readonly ILogger _logger; private readonly MediaFileManager _mediaFileManager; private readonly UploadAutoFillProperties _uploadAutoFillProperties; - [Obsolete("Use non-obsolete constructor. This constructor will be removed V15.")] public FileUploadPropertyEditor( IDataValueEditorFactory dataValueEditorFactory, MediaFileManager mediaFileManager, @@ -44,29 +36,6 @@ public FileUploadPropertyEditor( UploadAutoFillProperties uploadAutoFillProperties, IContentService contentService, IIOHelper ioHelper) - :this( - dataValueEditorFactory, - mediaFileManager, - contentSettings, - uploadAutoFillProperties, - contentService, - ioHelper, - StaticServiceProvider.Instance.GetRequiredService(), - StaticServiceProvider.Instance.GetRequiredService>() - ) - { - - } - - public FileUploadPropertyEditor( - IDataValueEditorFactory dataValueEditorFactory, - MediaFileManager mediaFileManager, - IOptionsMonitor contentSettings, - UploadAutoFillProperties uploadAutoFillProperties, - IContentService contentService, - IIOHelper ioHelper, - IJsonSerializer jsonSerializer, - ILogger logger) : base(dataValueEditorFactory) { _mediaFileManager = mediaFileManager ?? throw new ArgumentNullException(nameof(mediaFileManager)); @@ -74,15 +43,13 @@ public FileUploadPropertyEditor( _uploadAutoFillProperties = uploadAutoFillProperties; _contentService = contentService; _ioHelper = ioHelper; - _jsonSerializer = jsonSerializer; - _logger = logger; SupportsReadOnly = true; } public bool TryGetMediaPath(string? propertyEditorAlias, object? value, [MaybeNullWhen(false)] out string mediaPath) { if (propertyEditorAlias == Alias && - TryDeserializeMediaPath(value, out var mediaPathValue) && + value?.ToString() is var mediaPathValue && !string.IsNullOrWhiteSpace(mediaPathValue)) { mediaPath = mediaPathValue; @@ -93,36 +60,6 @@ public bool TryGetMediaPath(string? propertyEditorAlias, object? value, [MaybeNu return false; } - private bool TryDeserializeMediaPath(object? propVal, out string? mediaPath) - { - if (propVal is not string stringValue) - { - mediaPath = null; - return false; - } - - if (!stringValue.DetectIsJson()) - { - // Assume the value is a plain string with the file path - mediaPath = stringValue; - return true; - } - - try - { - mediaPath = _jsonSerializer.Deserialize(stringValue)?.Src; - return true; - } - catch (Exception ex) - { - _logger.LogError(ex, "Could not parse file upload value '{Json}'", stringValue); - } - - mediaPath = string.Empty; - return false; - } - - public void Handle(ContentCopiedNotification notification) { // get the upload field properties with a value From ef88e69b9b5cd72534579c30ed75eae85de2a7fc Mon Sep 17 00:00:00 2001 From: kjac Date: Wed, 8 May 2024 10:29:58 +0200 Subject: [PATCH 7/9] Actually revert all changes to FileUploadPropertyEditor --- .../PropertyEditors/FileUploadPropertyEditor.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs index d3fbfcd2b008..1d1b5a633f1f 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs @@ -16,7 +16,6 @@ namespace Umbraco.Cms.Core.PropertyEditors; [DataEditor( Constants.PropertyEditors.Aliases.UploadField, - ValueType = ValueTypes.Json, ValueEditorIsReusable = true)] public class FileUploadPropertyEditor : DataEditor, IMediaUrlGenerator, INotificationHandler, INotificationHandler, From 68dd13a89d67e5cf068f25dd34bb3907555e50b4 Mon Sep 17 00:00:00 2001 From: kjac Date: Wed, 8 May 2024 10:31:24 +0200 Subject: [PATCH 8/9] Also revert JsonValueConverter --- .../PropertyEditors/ValueConverters/JsonValueConverter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/JsonValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/JsonValueConverter.cs index 2ed38cf3522a..a12d4a82f88d 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/JsonValueConverter.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/JsonValueConverter.cs @@ -22,7 +22,7 @@ public class JsonValueConverter : PropertyValueConverterBase, IDeliveryApiProper private readonly ILogger _logger; private readonly PropertyEditorCollection _propertyEditors; - private readonly string[] _excludedPropertyEditors = { Constants.PropertyEditors.Aliases.MediaPicker3, Constants.PropertyEditors.Aliases.UploadField }; + private readonly string[] _excludedPropertyEditors = { Constants.PropertyEditors.Aliases.MediaPicker3 }; /// /// Initializes a new instance of the class. From abcbd6210bfc4db370494a157f33b58c6cd3fc01 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 8 May 2024 12:31:54 +0200 Subject: [PATCH 9/9] Cleanup remove unused equality methods --- .../TemporaryFileUploadValueBase.cs | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs index ce01cba04233..46a75e36d3f5 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/TemporaryFileUploadValueBase.cs @@ -14,28 +14,4 @@ public abstract class TemporaryFileUploadValueBase /// Gets or sets the value source image. /// public string? Src { get; set; } = string.Empty; - - protected bool Equals(TemporaryFileUploadValueBase other) => Nullable.Equals(TemporaryFileId, other.TemporaryFileId) && Src == other.Src; - - public override bool Equals(object? obj) - { - if (ReferenceEquals(null, obj)) - { - return false; - } - - if (ReferenceEquals(this, obj)) - { - return true; - } - - if (obj.GetType() != this.GetType()) - { - return false; - } - - return Equals((TemporaryFileUploadValueBase)obj); - } - - public override int GetHashCode() => HashCode.Combine(TemporaryFileId, Src); }