Add request caching around published content factory

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);
 }

src/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactory.cs

@@ -1,52 +1,107 @@
-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,
             contentCacheNode.SortOrder,
             contentCacheNode.CreateDate,
             contentCacheNode.CreatorId,
             contentType,
             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);
+        }
+
+        if (publishedContent is not null)
+        {
+            _appCaches.RequestCache.Set(cacheKey, publishedContent);
         }
 
-        return model;
+        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 @@
             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);
+
+            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 wierd, but they are not used.
+        // 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 @@
             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 @@
                 _variationContextAccessor);
     }
 
-
     private static IPublishedContent? GetPublishedContentAsDraft(IPublishedContent? content) =>
         content == null ? null :
             // an object in the cache is either an IPublishedContentOrMedia,
@@ -149,7 +233,7 @@
             content = wrapped.Unwrap();
         }
 
-        if (!(content is PublishedContent inner))
+        if (content is not PublishedContent inner)
         {
             throw new InvalidOperationException("Innermost content is not PublishedContent.");
         }

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);
+    }
+}