Performance: Request cache referenced entities when saving documents with block editors (#20590)

* Added request cache to content and media lookups in mult URL picker.

* Allow property editors to cache referenced entities from block data.

* Update src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Add obsoletions.

* Minor spellcheck

* Ensure request cache is available before relying on it.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: kjac <kja@umbraco.dk>
This commit is contained in:
Andy Butland
2025-10-31 10:49:26 +01:00
parent 5e87dead44
commit 96ecef0a92
7 changed files with 368 additions and 41 deletions

View File

@@ -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);
}
/// <summary>
/// Retrieves a <see cref="IContent"/> instance by its unique identifier, using the provided request cache to avoid redundant
/// lookups within the same request.
/// </summary>
/// <remarks>
/// 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.
/// </remarks>
/// <param name="key">The unique identifier of the content item to retrieve.</param>
/// <param name="requestCache">The request-scoped cache used to store and retrieve content items for the duration of the current request.</param>
/// <param name="contentService">The content service used to fetch the content item if it is not found in the cache.</param>
/// <returns>The <see cref="IContent"/> instance corresponding to the specified key, or null if no such content item exists.</returns>
[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<IContent?>(cacheKey);
if (content is null)
{
content = contentService.GetById(key);
if (content is not null)
{
requestCache.Set(cacheKey, content);
}
}
return content;
}
/// <summary>
/// Adds the specified <see cref="IContent"/> item to the request cache using its unique key.
/// </summary>
/// <param name="content">The content item to cache.</param>
/// <param name="requestCache">The request cache in which to store the content item.</param>
[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);
}
/// <summary>
/// Retrieves a <see cref="IMedia"/> instance by its unique identifier, using the provided request cache to avoid redundant
/// lookups within the same request.
/// </summary>
/// <remarks>
/// 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.
/// </remarks>
/// <param name="key">The unique identifier of the media item to retrieve.</param>
/// <param name="requestCache">The request-scoped cache used to store and retrieve media items for the duration of the current request.</param>
/// <param name="mediaService">The media service used to fetch the media item if it is not found in the cache.</param>
/// <returns>The <see cref="IMedia"/> instance corresponding to the specified key, or null if no such media item exists.</returns>
[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<IMedia?>(cacheKey);
if (media is null)
{
media = mediaService.GetById(key);
if (media is not null)
{
requestCache.Set(cacheKey, media);
}
}
return media;
}
/// <summary>
/// Adds the specified <see cref="IMedia"/> item to the request cache using its unique key.
/// </summary>
/// <param name="media">The media item to cache.</param>
/// <param name="requestCache">The request cache in which to store the media item.</param>
[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);
}
/// <summary>
/// Determines whether the content item identified by the specified key is present in the request cache.
/// </summary>
/// <param name="key">The unique identifier for the content item to check for in the cache.</param>
/// <param name="requestCache">The request cache in which to look for the content item.</param>
/// <returns>true if the content item is already cached in the request cache; otherwise, false.</returns>
[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<IContent?>(cacheKey) is not null;
}
/// <summary>
/// Determines whether the media item identified by the specified key is present in the request cache.
/// </summary>
/// <param name="key">The unique identifier for the media item to check for in the cache.</param>
/// <param name="requestCache">The request cache in which to look for the media item.</param>
/// <returns>true if the media item is already cached in the request cache; otherwise, false.</returns>
[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<IMedia?>(cacheKey) is not null;
}
}

View File

@@ -0,0 +1,19 @@
namespace Umbraco.Cms.Core.PropertyEditors;
/// <summary>
/// Optionally implemented by property editors, this defines a contract for caching entities that are referenced in block values.
/// </summary>
[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
{
/// <summary>
/// Caches the entities referenced by the provided block data values.
/// </summary>
/// <param name="values">An enumerable collection of block values that may contain the entities to be cached.</param>
[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<object> values);
}

View File

@@ -135,6 +135,8 @@ public abstract class BlockEditorPropertyValueEditor<TValue, TLayout> : BlockVal
BlockEditorData<TValue, TLayout>? currentBlockEditorData = SafeParseBlockEditorData(currentValue);
BlockEditorData<TValue, TLayout>? blockEditorData = SafeParseBlockEditorData(editorValue.Value);
CacheReferencedEntities(blockEditorData);
// We can skip MapBlockValueFromEditor if both editorValue and currentValue values are empty.
if (IsBlockEditorDataEmpty(currentBlockEditorData) && IsBlockEditorDataEmpty(blockEditorData))
{

View File

@@ -43,6 +43,45 @@ public abstract class BlockValuePropertyValueEditorBase<TValue, TLayout> : DataV
_languageService = languageService;
}
/// <summary>
/// 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.
/// </summary>
/// <remarks>
/// 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.
/// </remarks>
/// <param name="blockEditorData">The block editor data containing content and settings property values to analyze for referenced entities.</param>
[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<TValue, TLayout>? blockEditorData)
{
// Group property values by their associated data editor alias.
IEnumerable<IGrouping<string, BlockPropertyValue>> 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<string, BlockPropertyValue> 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!));
}
}
}
/// <inheritdoc />
public abstract IEnumerable<UmbracoEntityReference> GetReferences(object? value);

View File

@@ -52,10 +52,8 @@ public class MediaPicker3PropertyEditor : DataEditor
/// <summary>
/// Defines the value editor for the media picker property editor.
/// </summary>
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);
}
/// <inheritdoc/>
public void CacheReferencedEntities(IEnumerable<object> 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<IMedia> mediaItems = _mediaService.GetByIds(mediaKeys);
foreach (IMedia media in mediaItems)
{
CacheMediaById(media, _appCaches.RequestCache);
}
}
/// <inheritdoc/>
public IEnumerable<UmbracoEntityReference> 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<IMedia?>(cacheKey);
if (media is null)
{
media = _mediaService.GetById(key);
if (media is not null)
{
_appCaches.RequestCache.Set(cacheKey, media);
}
}
return media;
}
private List<MediaWithCropsDto> HandleTemporaryMediaUploads(List<MediaWithCropsDto> mediaWithCropsDtos, MediaPicker3Configuration configuration)
{
var invalidDtos = new List<MediaWithCropsDto>();
@@ -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<string?>(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<string?>(cacheKey);
if (typeKey is null)
{
typeKey = GetMediaTypeKeyFromService(typeAlias);
if (typeKey is not null)
{
_appCaches.RequestCache.Set(cacheKey, typeKey);
}
}
return typeKey;
}
}
/// <summary>

View File

@@ -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<MultiUrlPickerValueEditor> _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<MultiUrlPickerValueEditor> 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<AppCaches>())
{
}
public MultiUrlPickerValueEditor(
ILogger<MultiUrlPickerValueEditor> 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<LinkDisplay[], MultiUrlPickerConfiguration>(
_jsonSerializer,
new MinMaxValidator(localizedTextService)));
}
/// <inheritdoc/>
public void CacheReferencedEntities(IEnumerable<object> values)
{
var dtos = values
.Select(value =>
{
var asString = value is string str ? str : value.ToString();
if (string.IsNullOrEmpty(asString))
{
return null;
}
return _jsonSerializer.Deserialize<List<LinkDto>>(asString);
})
.WhereNotNull()
.SelectMany(x => x)
.Where(x => x.Type == Constants.UdiEntityType.Document || x.Type == Constants.UdiEntityType.Media)
.ToList();
IList<Guid> contentKeys = GetKeys(Constants.UdiEntityType.Document, dtos);
IList<Guid> mediaKeys = GetKeys(Constants.UdiEntityType.Media, dtos);
if (contentKeys.Count > 0)
{
IEnumerable<IContent> contentItems = _contentService.GetByIds(contentKeys);
foreach (IContent content in contentItems)
{
CacheContentById(content, _appCaches.RequestCache);
}
}
if (mediaKeys.Count > 0)
{
IEnumerable<IMedia> mediaItems = _mediaService.GetByIds(mediaKeys);
foreach (IMedia media in mediaItems)
{
CacheMediaById(media, _appCaches.RequestCache);
}
}
}
private IList<Guid> GetKeys(string entityType, IEnumerable<LinkDto> 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<UmbracoEntityReference> 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; }

View File

@@ -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<IIOHelper>(),
Mock.Of<IContentService>(),
Mock.Of<IMediaService>())
Mock.Of<IMediaService>(),
AppCaches.Disabled)
{
ConfigurationObject = new MultiUrlPickerConfiguration(),
};