diff --git a/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs b/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs index 211d36f65f..1c32dc3de6 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs @@ -3,12 +3,14 @@ using System.Globalization; using System.Runtime.Serialization; using System.Xml.Linq; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.Models.Validation; using Umbraco.Cms.Core.PropertyEditors.Validators; using Umbraco.Cms.Core.Serialization; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Strings; using Umbraco.Extensions; @@ -20,6 +22,9 @@ namespace Umbraco.Cms.Core.PropertyEditors; [DataContract] public class DataValueEditor : IDataValueEditor { + private const string ContentCacheKeyFormat = nameof(DataValueEditor) + "_Content_{0}"; + private const string MediaCacheKeyFormat = nameof(DataValueEditor) + "_Media_{0}"; + private readonly IJsonSerializer? _jsonSerializer; private readonly IShortStringHelper _shortStringHelper; @@ -415,4 +420,155 @@ public class DataValueEditor : IDataValueEditor return value.TryConvertTo(valueType); } + + /// + /// Retrieves a instance by its unique identifier, using the provided request cache to avoid redundant + /// lookups within the same request. + /// + /// + /// This method caches content lookups for the duration of the current request to improve performance when the same content + /// item may be accessed multiple times. This is particularly useful in scenarios involving multiple languages or blocks. + /// + /// The unique identifier of the content item to retrieve. + /// The request-scoped cache used to store and retrieve content items for the duration of the current request. + /// The content service used to fetch the content item if it is not found in the cache. + /// The instance corresponding to the specified key, or null if no such content item exists. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static IContent? GetAndCacheContentById(Guid key, IRequestCache requestCache, IContentService contentService) + { + if (requestCache.IsAvailable is false) + { + return contentService.GetById(key); + } + + var cacheKey = string.Format(ContentCacheKeyFormat, key); + IContent? content = requestCache.GetCacheItem(cacheKey); + if (content is null) + { + content = contentService.GetById(key); + if (content is not null) + { + requestCache.Set(cacheKey, content); + } + } + + return content; + } + + /// + /// Adds the specified item to the request cache using its unique key. + /// + /// The content item to cache. + /// The request cache in which to store the content item. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static void CacheContentById(IContent content, IRequestCache requestCache) + { + if (requestCache.IsAvailable is false) + { + return; + } + + var cacheKey = string.Format(ContentCacheKeyFormat, content.Key); + requestCache.Set(cacheKey, content); + } + + /// + /// Retrieves a instance by its unique identifier, using the provided request cache to avoid redundant + /// lookups within the same request. + /// + /// + /// This method caches media lookups for the duration of the current request to improve performance when the same media + /// item may be accessed multiple times. This is particularly useful in scenarios involving multiple languages or blocks. + /// + /// The unique identifier of the media item to retrieve. + /// The request-scoped cache used to store and retrieve media items for the duration of the current request. + /// The media service used to fetch the media item if it is not found in the cache. + /// The instance corresponding to the specified key, or null if no such media item exists. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static IMedia? GetAndCacheMediaById(Guid key, IRequestCache requestCache, IMediaService mediaService) + { + if (requestCache.IsAvailable is false) + { + return mediaService.GetById(key); + } + + var cacheKey = string.Format(MediaCacheKeyFormat, key); + IMedia? media = requestCache.GetCacheItem(cacheKey); + + if (media is null) + { + media = mediaService.GetById(key); + if (media is not null) + { + requestCache.Set(cacheKey, media); + } + } + + return media; + } + + /// + /// Adds the specified item to the request cache using its unique key. + /// + /// The media item to cache. + /// The request cache in which to store the media item. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static void CacheMediaById(IMedia media, IRequestCache requestCache) + { + if (requestCache.IsAvailable is false) + { + return; + } + + var cacheKey = string.Format(MediaCacheKeyFormat, media.Key); + requestCache.Set(cacheKey, media); + } + + /// + /// Determines whether the content item identified by the specified key is present in the request cache. + /// + /// The unique identifier for the content item to check for in the cache. + /// The request cache in which to look for the content item. + /// true if the content item is already cached in the request cache; otherwise, false. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static bool IsContentAlreadyCached(Guid key, IRequestCache requestCache) + { + if (requestCache.IsAvailable is false) + { + return false; + } + + var cacheKey = string.Format(ContentCacheKeyFormat, key); + return requestCache.GetCacheItem(cacheKey) is not null; + } + + /// + /// Determines whether the media item identified by the specified key is present in the request cache. + /// + /// The unique identifier for the media item to check for in the cache. + /// The request cache in which to look for the media item. + /// true if the media item is already cached in the request cache; otherwise, false. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static bool IsMediaAlreadyCached(Guid key, IRequestCache requestCache) + { + if (requestCache.IsAvailable is false) + { + return false; + } + + var cacheKey = string.Format(MediaCacheKeyFormat, key); + return requestCache.GetCacheItem(cacheKey) is not null; + } } diff --git a/src/Umbraco.Core/PropertyEditors/ICacheReferencedEntities.cs b/src/Umbraco.Core/PropertyEditors/ICacheReferencedEntities.cs new file mode 100644 index 0000000000..cf655a9167 --- /dev/null +++ b/src/Umbraco.Core/PropertyEditors/ICacheReferencedEntities.cs @@ -0,0 +1,19 @@ +namespace Umbraco.Cms.Core.PropertyEditors; + +/// +/// Optionally implemented by property editors, this defines a contract for caching entities that are referenced in block values. +/// +[Obsolete("This interface is available for support of request caching retrieved entities in property value editors that implement it. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] +public interface ICacheReferencedEntities +{ + /// + /// Caches the entities referenced by the provided block data values. + /// + /// An enumerable collection of block values that may contain the entities to be cached. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + void CacheReferencedEntities(IEnumerable values); +} diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs index a2e7f4bee9..07a735b8e1 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs @@ -107,6 +107,8 @@ public abstract class BlockEditorPropertyValueEditor : BlockVal BlockEditorData? currentBlockEditorData = SafeParseBlockEditorData(currentValue); BlockEditorData? blockEditorData = SafeParseBlockEditorData(editorValue.Value); + CacheReferencedEntities(blockEditorData); + // We can skip MapBlockValueFromEditor if both editorValue and currentValue values are empty. if (IsBlockEditorDataEmpty(currentBlockEditorData) && IsBlockEditorDataEmpty(blockEditorData)) { diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs index 8128643d9a..333df828e9 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs @@ -43,6 +43,45 @@ public abstract class BlockValuePropertyValueEditorBase : DataV _languageService = languageService; } + /// + /// Caches referenced entities for all property values with supporting property editors within the specified block editor data + /// optimising subsequent retrieval of entities when parsing and converting property values. + /// + /// + /// This method iterates through all property values associated with data editors in the provided + /// block editor data and invokes caching for referenced entities where supported by the property editor. + /// + /// The block editor data containing content and settings property values to analyze for referenced entities. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected void CacheReferencedEntities(BlockEditorData? blockEditorData) + { + // Group property values by their associated data editor alias. + IEnumerable> valuesByDataEditors = (blockEditorData?.BlockValue.ContentData ?? []).Union(blockEditorData?.BlockValue.SettingsData ?? []) + .SelectMany(x => x.Values) + .Where(x => x.EditorAlias is not null && x.Value is not null) + .GroupBy(x => x.EditorAlias!); + + // Iterate through each group and cache referenced entities if supported by the data editor. + foreach (IGrouping valueByDataEditor in valuesByDataEditors) + { + IDataEditor? dataEditor = _propertyEditors[valueByDataEditor.Key]; + if (dataEditor is null) + { + continue; + } + + IDataValueEditor valueEditor = dataEditor.GetValueEditor(); + + if (valueEditor is ICacheReferencedEntities valueEditorWithPrecaching) + { + valueEditorWithPrecaching.CacheReferencedEntities(valueByDataEditor.Select(x => x.Value!)); + } + } + } + + /// public abstract IEnumerable GetReferences(object? value); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs index d4a9e492bd..50fe850e04 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs @@ -52,10 +52,8 @@ public class MediaPicker3PropertyEditor : DataEditor /// /// Defines the value editor for the media picker property editor. /// - internal sealed class MediaPicker3PropertyValueEditor : DataValueEditor, IDataValueReference + internal sealed class MediaPicker3PropertyValueEditor : DataValueEditor, IDataValueReference, ICacheReferencedEntities { - private const string MediaCacheKeyFormat = nameof(MediaPicker3PropertyValueEditor) + "_Media_{0}"; - private readonly IDataTypeConfigurationCache _dataTypeReadCache; private readonly IJsonSerializer _jsonSerializer; private readonly IMediaImportService _mediaImportService; @@ -107,6 +105,27 @@ public class MediaPicker3PropertyEditor : DataEditor Validators.Add(validators); } + /// + public void CacheReferencedEntities(IEnumerable values) + { + var mediaKeys = values + .SelectMany(value => Deserialize(_jsonSerializer, value)) + .Select(dto => dto.MediaKey) + .Distinct() + .Where(x => IsMediaAlreadyCached(x, _appCaches.RequestCache) is false) + .ToList(); + if (mediaKeys.Count == 0) + { + return; + } + + IEnumerable mediaItems = _mediaService.GetByIds(mediaKeys); + foreach (IMedia media in mediaItems) + { + CacheMediaById(media, _appCaches.RequestCache); + } + } + /// public IEnumerable GetReferences(object? value) { @@ -208,31 +227,13 @@ public class MediaPicker3PropertyEditor : DataEditor foreach (MediaWithCropsDto mediaWithCropsDto in mediaWithCropsDtos) { - IMedia? media = GetMediaById(mediaWithCropsDto.MediaKey); + IMedia? media = GetAndCacheMediaById(mediaWithCropsDto.MediaKey, _appCaches.RequestCache, _mediaService); mediaWithCropsDto.MediaTypeAlias = media?.ContentType.Alias ?? unknownMediaType; } return mediaWithCropsDtos.Where(m => m.MediaTypeAlias != unknownMediaType).ToList(); } - private IMedia? GetMediaById(Guid key) - { - // Cache media lookups in case the same media is handled multiple times across a save operation, - // which is possible, particularly if we have multiple languages and blocks. - var cacheKey = string.Format(MediaCacheKeyFormat, key); - IMedia? media = _appCaches.RequestCache.GetCacheItem(cacheKey); - if (media is null) - { - media = _mediaService.GetById(key); - if (media is not null) - { - _appCaches.RequestCache.Set(cacheKey, media); - } - } - - return media; - } - private List HandleTemporaryMediaUploads(List mediaWithCropsDtos, MediaPicker3Configuration configuration) { var invalidDtos = new List(); @@ -240,7 +241,7 @@ public class MediaPicker3PropertyEditor : DataEditor foreach (MediaWithCropsDto mediaWithCropsDto in mediaWithCropsDtos) { // if the media already exist, don't bother with it - if (GetMediaById(mediaWithCropsDto.MediaKey) != null) + if (GetAndCacheMediaById(mediaWithCropsDto.MediaKey, _appCaches.RequestCache, _mediaService) != null) { continue; } @@ -480,18 +481,7 @@ public class MediaPicker3PropertyEditor : DataEditor foreach (var typeAlias in distinctTypeAliases) { - // Cache media type lookups since the same media type is likely to be used multiple times in validation, - // particularly if we have multiple languages and blocks. - var cacheKey = string.Format(MediaTypeCacheKeyFormat, typeAlias); - string? typeKey = _appCaches.RequestCache.GetCacheItem(cacheKey); - if (typeKey is null) - { - typeKey = _mediaTypeService.Get(typeAlias)?.Key.ToString(); - if (typeKey is not null) - { - _appCaches.RequestCache.Set(cacheKey, typeKey); - } - } + string? typeKey = GetMediaTypeKey(typeAlias); if (typeKey is null || allowedTypes.Contains(typeKey) is false) { @@ -506,6 +496,31 @@ public class MediaPicker3PropertyEditor : DataEditor return []; } + + private string? GetMediaTypeKey(string typeAlias) + { + // Cache media type lookups since the same media type is likely to be used multiple times in validation, + // particularly if we have multiple languages and blocks. + string? GetMediaTypeKeyFromService(string typeAlias) => _mediaTypeService.Get(typeAlias)?.Key.ToString(); + + if (_appCaches.RequestCache.IsAvailable is false) + { + return GetMediaTypeKeyFromService(typeAlias); + } + + var cacheKey = string.Format(MediaTypeCacheKeyFormat, typeAlias); + string? typeKey = _appCaches.RequestCache.GetCacheItem(cacheKey); + if (typeKey is null) + { + typeKey = GetMediaTypeKeyFromService(typeAlias); + if (typeKey is not null) + { + _appCaches.RequestCache.Set(cacheKey, typeKey); + } + } + + return typeKey; + } } /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs index 2f0a2b279b..2359e5537d 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs @@ -3,7 +3,10 @@ using System.ComponentModel.DataAnnotations; using System.Runtime.Serialization; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; @@ -19,14 +22,16 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Core.PropertyEditors; -public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference +public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference, ICacheReferencedEntities { private readonly ILogger _logger; private readonly IPublishedUrlProvider _publishedUrlProvider; private readonly IJsonSerializer _jsonSerializer; private readonly IContentService _contentService; private readonly IMediaService _mediaService; + private readonly AppCaches _appCaches; + [Obsolete("Please use the constructor taking all parameters. Scheduled for removal in Umbraco 19.")] public MultiUrlPickerValueEditor( ILogger logger, ILocalizedTextService localizedTextService, @@ -37,19 +42,102 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference IIOHelper ioHelper, IContentService contentService, IMediaService mediaService) + : this( + logger, + localizedTextService, + shortStringHelper, + attribute, + publishedUrlProvider, + jsonSerializer, + ioHelper, + contentService, + mediaService, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + public MultiUrlPickerValueEditor( + ILogger logger, + ILocalizedTextService localizedTextService, + IShortStringHelper shortStringHelper, + DataEditorAttribute attribute, + IPublishedUrlProvider publishedUrlProvider, + IJsonSerializer jsonSerializer, + IIOHelper ioHelper, + IContentService contentService, + IMediaService mediaService, + AppCaches appCaches) : base(shortStringHelper, jsonSerializer, ioHelper, attribute) { - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _logger = logger; _publishedUrlProvider = publishedUrlProvider; - _jsonSerializer = jsonSerializer; _contentService = contentService; _mediaService = mediaService; + _appCaches = appCaches; + Validators.Add(new TypedJsonValidatorRunner( _jsonSerializer, new MinMaxValidator(localizedTextService))); } + /// + public void CacheReferencedEntities(IEnumerable values) + { + var dtos = values + .Select(value => + { + var asString = value is string str ? str : value.ToString(); + if (string.IsNullOrEmpty(asString)) + { + return null; + } + + return _jsonSerializer.Deserialize>(asString); + }) + .WhereNotNull() + .SelectMany(x => x) + .Where(x => x.Type == Constants.UdiEntityType.Document || x.Type == Constants.UdiEntityType.Media) + .ToList(); + + IList contentKeys = GetKeys(Constants.UdiEntityType.Document, dtos); + IList mediaKeys = GetKeys(Constants.UdiEntityType.Media, dtos); + + if (contentKeys.Count > 0) + { + IEnumerable contentItems = _contentService.GetByIds(contentKeys); + foreach (IContent content in contentItems) + { + CacheContentById(content, _appCaches.RequestCache); + } + } + + if (mediaKeys.Count > 0) + { + IEnumerable mediaItems = _mediaService.GetByIds(mediaKeys); + foreach (IMedia media in mediaItems) + { + CacheMediaById(media, _appCaches.RequestCache); + } + } + } + + private IList GetKeys(string entityType, IEnumerable dtos) => + dtos + .Where(x => x.Type == entityType) + .Select(x => x.Unique ?? (x.Udi is not null ? x.Udi.Guid : Guid.Empty)) + .Where(x => x != Guid.Empty) + .Distinct() + .Where(x => IsAlreadyCached(x, entityType) is false) + .ToList(); + + private bool IsAlreadyCached(Guid key, string entityType) => entityType switch + { + Constants.UdiEntityType.Document => IsContentAlreadyCached(key, _appCaches.RequestCache), + Constants.UdiEntityType.Media => IsMediaAlreadyCached(key, _appCaches.RequestCache), + _ => false, + }; + public IEnumerable GetReferences(object? value) { var asString = value == null ? string.Empty : value is string str ? str : value.ToString(); @@ -105,7 +193,7 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference if (dto.Udi.EntityType == Constants.UdiEntityType.Document) { url = _publishedUrlProvider.GetUrl(dto.Udi.Guid, UrlMode.Relative, culture); - IContent? c = _contentService.GetById(dto.Udi.Guid); + IContent? c = GetAndCacheContentById(dto.Udi.Guid, _appCaches.RequestCache, _contentService); if (c is not null) { @@ -119,7 +207,7 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference else if (dto.Udi.EntityType == Constants.UdiEntityType.Media) { url = _publishedUrlProvider.GetMediaUrl(dto.Udi.Guid, UrlMode.Relative, culture); - IMedia? m = _mediaService.GetById(dto.Udi.Guid); + IMedia? m = GetAndCacheMediaById(dto.Udi.Guid, _appCaches.RequestCache, _mediaService); if (m is not null) { published = m.Trashed is false; @@ -207,6 +295,12 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference [DataMember(Name = "target")] public string? Target { get; set; } + [DataMember(Name = "unique")] + public Guid? Unique { get; set; } + + [DataMember(Name = "type")] + public string? Type { get; set; } + [DataMember(Name = "udi")] public GuidUdi? Udi { get; set; } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/Validators/MultiUrlPickerValueEditorValidationTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/Validators/MultiUrlPickerValueEditorValidationTests.cs index dae1d885b2..c9264d5935 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/Validators/MultiUrlPickerValueEditorValidationTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/Validators/MultiUrlPickerValueEditorValidationTests.cs @@ -2,6 +2,7 @@ using System.ComponentModel.DataAnnotations; using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models.Validation; using Umbraco.Cms.Core.PropertyEditors; @@ -68,7 +69,8 @@ internal class MultiUrlPickerValueEditorValidationTests new SystemTextJsonSerializer(new DefaultJsonSerializerEncoderFactory()), Mock.Of(), Mock.Of(), - Mock.Of()) + Mock.Of(), + AppCaches.Disabled) { ConfigurationObject = new MultiUrlPickerConfiguration(), };