Add request caching around published content factory (#19990)

* Add request caching around published content factory. * Fixed ordering of log message parameters. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Invert if to reduce nesting --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: mole <nikolajlauridsen@protonmail.ch>
2025-08-27 13:01:44 +02:00
parent 019563b226
commit f0c5ecf086
3 changed files with 259 additions and 15 deletions
--- a/src/Umbraco.PublishedCache.HybridCache/Factories/IPublishedContentFactory.cs
+++ b/src/Umbraco.PublishedCache.HybridCache/Factories/IPublishedContentFactory.cs
@@ -1,12 +1,25 @@
-using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.PublishedContent;

 namespace Umbraco.Cms.Infrastructure.HybridCache.Factories;

+/// <summary>
+/// Defines a factory to create <see cref="IPublishedContent"/> and <see cref="IPublishedMember"/> from a <see cref="ContentCacheNode"/> or <see cref="IMember"/>.
+/// </summary>
 internal interface IPublishedContentFactory
 {
+    /// <summary>
+    /// Converts a <see cref="ContentCacheNode"/> to an <see cref="IPublishedContent"/> if document type.
+    /// </summary>
    IPublishedContent? ToIPublishedContent(ContentCacheNode contentCacheNode, bool preview);
+
+    /// <summary>
+    /// Converts a <see cref="ContentCacheNode"/> to an <see cref="IPublishedContent"/> of media type.
+    /// </summary>
    IPublishedContent? ToIPublishedMedia(ContentCacheNode contentCacheNode);

+    /// <summary>
+    /// Converts a <see cref="IMember"/> to an <see cref="IPublishedMember"/>.
+    /// </summary>
    IPublishedMember ToPublishedMember(IMember member);
 }
--- a/src/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactory.cs
+++ b/src/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactory.cs
@@ -1,29 +1,61 @@
-using Umbraco.Cms.Core.Models;
+using Microsoft.Extensions.Logging;
+using Umbraco.Cms.Core.Cache;
+using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PublishedCache;
+using Umbraco.Extensions;

 namespace Umbraco.Cms.Infrastructure.HybridCache.Factories;

+/// <summary>
+/// Defines a factory to create <see cref="IPublishedContent"/> and <see cref="IPublishedMember"/> from a <see cref="ContentCacheNode"/> or <see cref="IMember"/>.
+/// </summary>
 internal sealed class PublishedContentFactory : IPublishedContentFactory
 {
    private readonly IElementsCache _elementsCache;
    private readonly IVariationContextAccessor _variationContextAccessor;
    private readonly IPublishedContentTypeCache _publishedContentTypeCache;
+    private readonly ILogger<PublishedContentFactory> _logger;
+    private readonly AppCaches _appCaches;

-
+    /// <summary>
+    /// Initializes a new instance of the <see cref="PublishedContentFactory"/> class.
+    /// </summary>
    public PublishedContentFactory(
        IElementsCache elementsCache,
        IVariationContextAccessor variationContextAccessor,
-        IPublishedContentTypeCache publishedContentTypeCache)
+        IPublishedContentTypeCache publishedContentTypeCache,
+        ILogger<PublishedContentFactory> logger,
+        AppCaches appCaches)
    {
        _elementsCache = elementsCache;
        _variationContextAccessor = variationContextAccessor;
        _publishedContentTypeCache = publishedContentTypeCache;
+        _logger = logger;
+        _appCaches = appCaches;
    }

+    /// <inheritdoc/>
    public IPublishedContent? ToIPublishedContent(ContentCacheNode contentCacheNode, bool preview)
    {
-        IPublishedContentType contentType = _publishedContentTypeCache.Get(PublishedItemType.Content, contentCacheNode.ContentTypeId);
+        var cacheKey = $"{nameof(PublishedContentFactory)}DocumentCache_{contentCacheNode.Id}_{preview}";
+        IPublishedContent? publishedContent = _appCaches.RequestCache.GetCacheItem<IPublishedContent?>(cacheKey);
+        if (publishedContent is not null)
+        {
+            _logger.LogDebug(
+                "Using cached IPublishedContent for document {ContentCacheNodeName} ({ContentCacheNodeId}).",
+                contentCacheNode.Data?.Name ?? "No Name",
+                contentCacheNode.Id);
+            return publishedContent;
+        }
+
+        _logger.LogDebug(
+            "Creating IPublishedContent for document {ContentCacheNodeName} ({ContentCacheNodeId}).",
+            contentCacheNode.Data?.Name ?? "No Name",
+            contentCacheNode.Id);
+
+        IPublishedContentType contentType =
+            _publishedContentTypeCache.Get(PublishedItemType.Content, contentCacheNode.ContentTypeId);
        var contentNode = new ContentNode(
            contentCacheNode.Id,
            contentCacheNode.Key,
@@ -34,19 +66,42 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory
            preview ? contentCacheNode.Data : null,
            preview ? null : contentCacheNode.Data);

-        IPublishedContent? model = GetModel(contentNode, preview);
+        publishedContent = GetModel(contentNode, preview);

        if (preview)
        {
-            return model ?? GetPublishedContentAsDraft(model);
+            publishedContent ??= GetPublishedContentAsDraft(publishedContent);
        }

-        return model;
+        if (publishedContent is not null)
+        {
+            _appCaches.RequestCache.Set(cacheKey, publishedContent);
+        }
+
+        return publishedContent;
    }

+    /// <inheritdoc/>
    public IPublishedContent? ToIPublishedMedia(ContentCacheNode contentCacheNode)
    {
-        IPublishedContentType contentType = _publishedContentTypeCache.Get(PublishedItemType.Media, contentCacheNode.ContentTypeId);
+        var cacheKey = $"{nameof(PublishedContentFactory)}MediaCache_{contentCacheNode.Id}";
+        IPublishedContent? publishedContent = _appCaches.RequestCache.GetCacheItem<IPublishedContent?>(cacheKey);
+        if (publishedContent is not null)
+        {
+            _logger.LogDebug(
+                "Using cached IPublishedContent for media {ContentCacheNodeName} ({ContentCacheNodeId}).",
+                contentCacheNode.Data?.Name ?? "No Name",
+                contentCacheNode.Id);
+            return publishedContent;
+        }
+
+        _logger.LogDebug(
+            "Creating IPublishedContent for media {ContentCacheNodeName} ({ContentCacheNodeId}).",
+            contentCacheNode.Data?.Name ?? "No Name",
+            contentCacheNode.Id);
+
+        IPublishedContentType contentType =
+            _publishedContentTypeCache.Get(PublishedItemType.Media, contentCacheNode.ContentTypeId);
        var contentNode = new ContentNode(
            contentCacheNode.Id,
            contentCacheNode.Key,
@@ -57,14 +112,40 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory
            null,
            contentCacheNode.Data);

-        return GetModel(contentNode, false);
+        publishedContent = GetModel(contentNode, false);
+
+        if (publishedContent is not null)
+        {
+            _appCaches.RequestCache.Set(cacheKey, publishedContent);
+        }
+
+        return publishedContent;
    }

+    /// <inheritdoc/>
    public IPublishedMember ToPublishedMember(IMember member)
    {
-        IPublishedContentType contentType = _publishedContentTypeCache.Get(PublishedItemType.Member, member.ContentTypeId);
+        string cacheKey = $"{nameof(PublishedContentFactory)}MemberCache_{member.Id}";
+        IPublishedMember? publishedMember = _appCaches.RequestCache.GetCacheItem<IPublishedMember?>(cacheKey);
+        if (publishedMember is not null)
+        {
+            _logger.LogDebug(
+                "Using cached IPublishedMember for member {MemberName} ({MemberId}).",
+                member.Username,
+                member.Id);

-        // Members are only "mapped" never cached, so these default values are a bit wierd, but they are not used.
+            return publishedMember;
+        }
+
+        _logger.LogDebug(
+            "Creating IPublishedMember for member {MemberName} ({MemberId}).",
+            member.Username,
+            member.Id);
+
+        IPublishedContentType contentType =
+            _publishedContentTypeCache.Get(PublishedItemType.Member, member.ContentTypeId);
+
+        // Members are only "mapped" never cached, so these default values are a bit weird, but they are not used.
        var contentData = new ContentData(
            member.Name,
            null,
@@ -85,7 +166,11 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory
            contentType,
            null,
            contentData);
-        return new PublishedMember(member, contentNode, _elementsCache, _variationContextAccessor);
+        publishedMember = new PublishedMember(member, contentNode, _elementsCache, _variationContextAccessor);
+
+        _appCaches.RequestCache.Set(cacheKey, publishedMember);
+
+        return publishedMember;
    }

    private static Dictionary<string, PropertyData[]> GetPropertyValues(IPublishedContentType contentType, IMember member)
@@ -134,7 +219,6 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory
                _variationContextAccessor);
    }

-
    private static IPublishedContent? GetPublishedContentAsDraft(IPublishedContent? content) =>
        content == null ? null :
            // an object in the cache is either an IPublishedContentOrMedia,
@@ -149,7 +233,7 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory
            content = wrapped.Unwrap();
        }

-        if (!(content is PublishedContent inner))
+        if (content is not PublishedContent inner)
        {
            throw new InvalidOperationException("Innermost content is not PublishedContent.");
        }
--- a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactoryTests.cs
+++ b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactoryTests.cs
@@ -0,0 +1,147 @@
+using NUnit.Framework;
+using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.Cache;
+using Umbraco.Cms.Core.Models.PublishedContent;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Infrastructure.HybridCache;
+using Umbraco.Cms.Infrastructure.HybridCache.Factories;
+using Umbraco.Cms.Tests.Common.Builders;
+using Umbraco.Cms.Tests.Common.Builders.Extensions;
+using Umbraco.Cms.Tests.Common.Testing;
+using Umbraco.Cms.Tests.Integration.Testing;
+
+namespace Umbraco.Cms.Tests.Integration.Umbraco.PublishedCache.HybridCache;
+
+[TestFixture]
+[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
+internal sealed class PublishedContentFactoryTests : UmbracoIntegrationTestWithContent
+{
+    private IPublishedContentFactory PublishedContentFactory => GetRequiredService<IPublishedContentFactory>();
+
+    private IPublishedValueFallback PublishedValueFallback => GetRequiredService<IPublishedValueFallback>();
+
+    private IMediaService MediaService => GetRequiredService<IMediaService>();
+
+    private IMediaTypeService MediaTypeService => GetRequiredService<IMediaTypeService>();
+
+    private IMemberService MemberService => GetRequiredService<IMemberService>();
+
+    private IMemberTypeService MemberTypeService => GetRequiredService<IMemberTypeService>();
+
+    protected override void CustomTestSetup(IUmbracoBuilder builder)
+    {
+        var requestCache = new DictionaryAppCache();
+        var appCaches = new AppCaches(
+                NoAppCache.Instance,
+                requestCache,
+                new IsolatedCaches(type => NoAppCache.Instance));
+        builder.Services.AddUnique(appCaches);
+    }
+
+    [Test]
+    public void Can_Create_Published_Content_For_Document()
+    {
+        var contentCacheNode = new ContentCacheNode
+        {
+            Id = Textpage.Id,
+            Key = Textpage.Key,
+            ContentTypeId = Textpage.ContentType.Id,
+            CreateDate = Textpage.CreateDate,
+            CreatorId = Textpage.CreatorId,
+            SortOrder = Textpage.SortOrder,
+            Data = new ContentData(
+                Textpage.Name,
+                "text-page",
+                Textpage.VersionId,
+                Textpage.UpdateDate,
+                Textpage.WriterId,
+                Textpage.TemplateId,
+                true,
+                new Dictionary<string, PropertyData[]>
+                {
+                    {
+                        "title", new[]
+                        {
+                            new PropertyData
+                            {
+                                Value = "Test title",
+                                Culture = string.Empty,
+                                Segment = string.Empty,
+                            },
+                        }
+                    },
+                },
+                null),
+        };
+        var result = PublishedContentFactory.ToIPublishedContent(contentCacheNode, false);
+        Assert.IsNotNull(result);
+        Assert.AreEqual(Textpage.Id, result.Id);
+        Assert.AreEqual(Textpage.Name, result.Name);
+        Assert.AreEqual("Test title", result.Properties.Single(x => x.Alias == "title").Value<string>(PublishedValueFallback));
+
+        // Verify that requesting the same content again returns the same instance (from request cache).
+        var result2 = PublishedContentFactory.ToIPublishedContent(contentCacheNode, false);
+        Assert.AreSame(result, result2);
+    }
+
+    [Test]
+    public async Task Can_Create_Published_Content_For_Media()
+    {
+        var mediaType = new MediaTypeBuilder().Build();
+        mediaType.AllowedAsRoot = true;
+        await MediaTypeService.CreateAsync(mediaType, Constants.Security.SuperUserKey);
+
+        var media = new MediaBuilder()
+            .WithMediaType(mediaType)
+            .WithName("Media 1")
+            .Build();
+        MediaService.Save(media);
+
+        var contentCacheNode = new ContentCacheNode
+        {
+            Id = media.Id,
+            Key = media.Key,
+            ContentTypeId = media.ContentType.Id,
+            Data = new ContentData(
+                media.Name,
+                null,
+                0,
+                media.UpdateDate,
+                media.WriterId,
+                null,
+                false,
+                new Dictionary<string, PropertyData[]>(),
+                null),
+        };
+        var result = PublishedContentFactory.ToIPublishedMedia(contentCacheNode);
+        Assert.IsNotNull(result);
+        Assert.AreEqual(media.Id, result.Id);
+        Assert.AreEqual(media.Name, result.Name);
+
+        // Verify that requesting the same content again returns the same instance (from request cache).
+        var result2 = PublishedContentFactory.ToIPublishedMedia(contentCacheNode);
+        Assert.AreSame(result, result2);
+    }
+
+    [Test]
+    public async Task Can_Create_Published_Member_For_Member()
+    {
+        var memberType = new MemberTypeBuilder().Build();
+        await MemberTypeService.CreateAsync(memberType, Constants.Security.SuperUserKey);
+
+        var member = new MemberBuilder()
+            .WithMemberType(memberType)
+            .WithName("Member 1")
+            .Build();
+        MemberService.Save(member);
+
+        var result = PublishedContentFactory.ToPublishedMember(member);
+        Assert.IsNotNull(result);
+        Assert.AreEqual(member.Id, result.Id);
+        Assert.AreEqual(member.Name, result.Name);
+
+        // Verify that requesting the same content again returns the same instance (from request cache).
+        var result2 = PublishedContentFactory.ToPublishedMember(member);
+        Assert.AreSame(result, result2);
+    }
+}