Media: Port Children/Descendants traversal optimisations to media (#22742 follow-up)

src/Umbraco.PublishedCache.HybridCache/DocumentCache.cs

@@ -7,34 +7,47 @@
 
 namespace Umbraco.Cms.Infrastructure.HybridCache;
 
+/// <summary>
+/// Provides access to published documents (content) held in the hybrid cache.
+/// </summary>
 public sealed class DocumentCache : IPublishedContentCache
 {
     private readonly IDocumentCacheService _documentCacheService;
-    private readonly IPublishedContentTypeCache _publishedContentTypeCache;
     private readonly IDocumentNavigationQueryService _documentNavigationQueryService;
-    private readonly IDocumentUrlService _documentUrlService;
-    private readonly Lazy<IPublishedUrlProvider> _publishedUrlProvider;
 
+    // TODO (V19): Remove the unused parameters from the constructor.
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="DocumentCache"/> class.
+    /// </summary>
+    /// <param name="documentCacheService">The service that retrieves and caches published document nodes.</param>
+    /// <param name="publishedContentTypeCache">The cache of published content types.</param>
+    /// <param name="documentNavigationQueryService">The service used to query the document navigation structure.</param>
+    /// <param name="documentUrlService">The service that resolves document URLs.</param>
+    /// <param name="publishedUrlProvider">A lazily resolved provider of published URLs.</param>
     public DocumentCache(
         IDocumentCacheService documentCacheService,
+#pragma warning disable IDE0060 // Remove unused parameter
         IPublishedContentTypeCache publishedContentTypeCache,
         IDocumentNavigationQueryService documentNavigationQueryService,
         IDocumentUrlService documentUrlService,
         Lazy<IPublishedUrlProvider> publishedUrlProvider)
+#pragma warning restore IDE0060 // Remove unused parameter
     {
         _documentCacheService = documentCacheService;
-        _publishedContentTypeCache = publishedContentTypeCache;
         _documentNavigationQueryService = documentNavigationQueryService;
-        _documentUrlService = documentUrlService;
-        _publishedUrlProvider = publishedUrlProvider;
     }
 
+    /// <inheritdoc/>
     public async Task<IPublishedContent?> GetByIdAsync(int id, bool? preview = null) => await _documentCacheService.GetByIdAsync(id, preview);
 
+    /// <inheritdoc/>
     public async Task<IPublishedContent?> GetByIdAsync(Guid key, bool? preview = null) => await _documentCacheService.GetByKeyAsync(key, preview);
 
+    /// <inheritdoc/>
     public IPublishedContent? GetById(bool preview, int contentId) => GetByIdAsync(contentId, preview).GetAwaiter().GetResult();
 
+    /// <inheritdoc/>
     public IPublishedContent? GetById(bool preview, Guid contentId)
     {
         // Sync fast path: when the converted-content L0 cache already holds the item we can
@@ -49,11 +62,21 @@
         return GetByIdAsync(contentId, preview).GetAwaiter().GetResult();
     }
 
-
+    /// <inheritdoc/>
     public IPublishedContent? GetById(int contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult();
 
+    /// <inheritdoc/>
     public IPublishedContent? GetById(Guid contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult();
 
+    /// <summary>
+    /// Gets the published documents at the root of the content tree.
+    /// </summary>
+    /// <param name="preview">A value indicating whether to consider unpublished content.</param>
+    /// <param name="culture">
+    /// The culture to filter root documents by. When <c>null</c>, all root documents are returned; otherwise only those
+    /// that are invariant or vary for the specified culture are returned.
+    /// </param>
+    /// <returns>The published documents at root level available for the specified culture.</returns>
     [Obsolete("This method is no longer used in Umbraco and is not defined on the interface. " +
         "Any usage can be replaced with a call to IDocumentNavigationQueryService.TryGetRootKeys to retrieve the document keys, " +
         "with each key passed to IPublishedContentCache.GetById to retrieve the IPublishedContent instances. Scheduled for removal in Umbraco 19.")]

src/Umbraco.PublishedCache.HybridCache/MediaCache.cs

@@ -5,26 +5,54 @@
 
 namespace Umbraco.Cms.Infrastructure.HybridCache;
 
+/// <summary>
+/// Provides access to published media items held in the hybrid cache.
+/// </summary>
 public sealed class MediaCache : IPublishedMediaCache
 {
     private readonly IMediaCacheService _mediaCacheService;
-    private readonly IPublishedContentTypeCache _publishedContentTypeCache;
     private readonly IMediaNavigationQueryService _mediaNavigationQueryService;
 
-    public MediaCache(IMediaCacheService mediaCacheService, IPublishedContentTypeCache publishedContentTypeCache, IMediaNavigationQueryService mediaNavigationQueryService)
+    // TODO (V19): Remove the unused parameters from the constructor.
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="MediaCache"/> class.
+    /// </summary>
+    /// <param name="mediaCacheService">The service that retrieves and caches published media nodes.</param>
+    /// <param name="publishedContentTypeCache">The cache of published content types.</param>
+    /// <param name="mediaNavigationQueryService">The service used to query the media navigation structure.</param>
+    public MediaCache(
+        IMediaCacheService mediaCacheService,
+#pragma warning disable IDE0060 // Remove unused parameter
+        IPublishedContentTypeCache publishedContentTypeCache,
+#pragma warning restore IDE0060 // Remove unused parameter
+        IMediaNavigationQueryService mediaNavigationQueryService)
     {
         _mediaCacheService = mediaCacheService;
-        _publishedContentTypeCache = publishedContentTypeCache;
         _mediaNavigationQueryService = mediaNavigationQueryService;
     }
 
+    /// <inheritdoc/>
     public async Task<IPublishedContent?> GetByIdAsync(int id) => await _mediaCacheService.GetByIdAsync(id);
 
+    /// <inheritdoc/>
     public async Task<IPublishedContent?> GetByIdAsync(Guid key) => await _mediaCacheService.GetByKeyAsync(key);
 
+    /// <inheritdoc/>
     public IPublishedContent? GetById(bool preview, int contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult();
 
-    public IPublishedContent? GetById(bool preview, Guid contentId)
+    /// <inheritdoc/>
+    /// <remarks>
+    /// Media has no draft/preview dimension, so preview is ignored and this delegates to the
+    /// single-argument Guid overload where the sync fast path lives.
+    /// </remarks>
+    public IPublishedContent? GetById(bool preview, Guid contentId) => GetById(contentId);
+
+    /// <inheritdoc/>
+    public IPublishedContent? GetById(int contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult();
+
+    /// <inheritdoc/>
+    public IPublishedContent? GetById(Guid contentId)
     {
         // Sync fast path: when the converted-content L0 cache already holds the item we can
         // return it without spinning up an async state machine. This is the dominant case on
@@ -38,11 +66,15 @@
         return GetByIdAsync(contentId).GetAwaiter().GetResult();
     }
 
-
-    public IPublishedContent? GetById(int contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult();
-
-    public IPublishedContent? GetById(Guid contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult();
-
+    /// <summary>
+    /// Gets the published media items at the root of the media tree.
+    /// </summary>
+    /// <param name="preview">A value indicating whether to consider unpublished items. Media has no draft state, so this has no effect.</param>
+    /// <param name="culture">
+    /// The culture to filter root media by. When <c>null</c>, all root media are returned; otherwise only those that are
+    /// invariant or vary for the specified culture are returned.
+    /// </param>
+    /// <returns>The published media items at root level available for the specified culture.</returns>
     public IEnumerable<IPublishedContent> GetAtRoot(bool preview, string? culture = null)
     {
         if (_mediaNavigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeys) is false)

src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs

@@ -104,16 +104,7 @@ public MediaCacheService(
     /// <inheritdoc />
     public long? GetApproximateBytes() => _publishedContentCache.ApproximateSizeInBytes;
 
-    public async Task<IPublishedContent?> GetByKeyAsync(Guid key)
-    {
-        Attempt<int> idAttempt = _idKeyMap.GetIdForKey(key, UmbracoObjectTypes.Media);
-        if (idAttempt.Success is false)
-        {
-            return null;
-        }
-
-        return await GetNodeAsync(key);
-    }
+    public Task<IPublishedContent?> GetByKeyAsync(Guid key) => GetNodeAsync(key);
 
     public async Task<IPublishedContent?> GetByIdAsync(int id)
     {

tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/MediaCacheSyncFastPathTests.cs

@@ -0,0 +1,109 @@
+using Moq;
+using NUnit.Framework;
+using Umbraco.Cms.Core.Models.PublishedContent;
+using Umbraco.Cms.Core.PublishedCache;
+using Umbraco.Cms.Core.Services.Navigation;
+using Umbraco.Cms.Infrastructure.HybridCache;
+
+namespace Umbraco.Cms.Tests.UnitTests.Umbraco.PublishedCache.HybridCache;
+
+/// <summary>
+/// Tests that <see cref="MediaCache"/> consults <see cref="IMediaCacheService.TryGetCached"/>
+/// first and skips the async fallback entirely when the L0 cache has the requested item.
+/// </summary>
+/// <remarks>
+/// The async path (<c>GetByKeyAsync</c>) is the slow case — id/key map lookup + distributed
+/// cache + database + factory work. On a warm site we want the per-key sync calls inside
+/// <c>FilterAvailable</c>'s lazy chain to take the fast path. Because <c>FilterAvailable</c>
+/// binds its method-group call (<c>candidateKeys.Select(GetById)</c>) to the single-argument
+/// <see cref="MediaCache.GetById(Guid)"/> overload, this fixture asserts the fast path fires
+/// for that overload (and the two-argument overload that delegates to it), not only the
+/// two-argument one.
+/// </remarks>
+[TestFixture]
+public class MediaCacheSyncFastPathTests
+{
+    [Test]
+    public void GetById_Guid_HitsL0ViaTryGetCached_ReturnsCachedAndSkipsAsync()
+    {
+        IPublishedContent expected = Mock.Of<IPublishedContent>();
+        var cacheService = CreateHitCacheService(expected);
+
+        MediaCache cache = CreateCache(cacheService);
+
+        IPublishedContent? actual = cache.GetById(Guid.NewGuid());
+
+        Assert.That(actual, Is.SameAs(expected));
+        cacheService.Verify(
+            s => s.GetByKeyAsync(It.IsAny<Guid>()),
+            Times.Never,
+            "Async path should not run when TryGetCached hits");
+    }
+
+    [Test]
+    public void GetById_PreviewGuid_HitsL0ViaTryGetCached_ReturnsCachedAndSkipsAsync()
+    {
+        IPublishedContent expected = Mock.Of<IPublishedContent>();
+        var cacheService = CreateHitCacheService(expected);
+
+        MediaCache cache = CreateCache(cacheService);
+
+        IPublishedContent? actual = cache.GetById(preview: true, contentId: Guid.NewGuid());
+
+        Assert.That(actual, Is.SameAs(expected));
+        cacheService.Verify(
+            s => s.GetByKeyAsync(It.IsAny<Guid>()),
+            Times.Never,
+            "Async path should not run when TryGetCached hits");
+    }
+
+    [Test]
+    public void GetById_Guid_MissesL0_FallsThroughToAsyncPath()
+    {
+        IPublishedContent expected = Mock.Of<IPublishedContent>();
+        var cacheService = new Mock<IMediaCacheService>();
+        cacheService
+            .Setup(s => s.TryGetCached(It.IsAny<Guid>(), out It.Ref<IPublishedContent?>.IsAny))
+            .Returns(new TryGetCachedDelegate((Guid _, out IPublishedContent? content) =>
+            {
+                content = null;
+                return false;
+            }));
+        cacheService
+            .Setup(s => s.GetByKeyAsync(It.IsAny<Guid>()))
+            .ReturnsAsync(expected);
+
+        MediaCache cache = CreateCache(cacheService);
+
+        IPublishedContent? actual = cache.GetById(Guid.NewGuid());
+
+        Assert.That(actual, Is.SameAs(expected));
+        cacheService.Verify(
+            s => s.GetByKeyAsync(It.IsAny<Guid>()),
+            Times.Once,
+            "Async path runs exactly once on TryGetCached miss");
+    }
+
+    private static Mock<IMediaCacheService> CreateHitCacheService(IPublishedContent expected)
+    {
+        var cacheService = new Mock<IMediaCacheService>();
+        cacheService
+            .Setup(s => s.TryGetCached(It.IsAny<Guid>(), out It.Ref<IPublishedContent?>.IsAny))
+            .Returns(new TryGetCachedDelegate((Guid _, out IPublishedContent? content) =>
+            {
+                content = expected;
+                return true;
+            }));
+        return cacheService;
+    }
+
+    private static MediaCache CreateCache(Mock<IMediaCacheService> cacheService)
+        => new(
+            cacheService.Object,
+            Mock.Of<IPublishedContentTypeCache>(),
+            Mock.Of<IMediaNavigationQueryService>());
+
+    // Moq cannot bind directly to ref / out parameters in the lambda overload, so we
+    // declare a delegate that matches the TryGetCached signature and pass it explicitly.
+    private delegate bool TryGetCachedDelegate(Guid key, out IPublishedContent? content);
+}