diff --git a/src/Umbraco.PublishedCache.HybridCache/DocumentCache.cs b/src/Umbraco.PublishedCache.HybridCache/DocumentCache.cs index b5b49cd92d8b..db8a138bb1be 100644 --- a/src/Umbraco.PublishedCache.HybridCache/DocumentCache.cs +++ b/src/Umbraco.PublishedCache.HybridCache/DocumentCache.cs @@ -7,34 +7,47 @@ namespace Umbraco.Cms.Infrastructure.HybridCache; +/// +/// Provides access to published documents (content) held in the hybrid cache. +/// public sealed class DocumentCache : IPublishedContentCache { private readonly IDocumentCacheService _documentCacheService; - private readonly IPublishedContentTypeCache _publishedContentTypeCache; private readonly IDocumentNavigationQueryService _documentNavigationQueryService; - private readonly IDocumentUrlService _documentUrlService; - private readonly Lazy _publishedUrlProvider; + // TODO (V19): Remove the unused parameters from the constructor. + + /// + /// Initializes a new instance of the class. + /// + /// The service that retrieves and caches published document nodes. + /// The cache of published content types. + /// The service used to query the document navigation structure. + /// The service that resolves document URLs. + /// A lazily resolved provider of published URLs. public DocumentCache( IDocumentCacheService documentCacheService, +#pragma warning disable IDE0060 // Remove unused parameter IPublishedContentTypeCache publishedContentTypeCache, IDocumentNavigationQueryService documentNavigationQueryService, IDocumentUrlService documentUrlService, Lazy publishedUrlProvider) +#pragma warning restore IDE0060 // Remove unused parameter { _documentCacheService = documentCacheService; - _publishedContentTypeCache = publishedContentTypeCache; _documentNavigationQueryService = documentNavigationQueryService; - _documentUrlService = documentUrlService; - _publishedUrlProvider = publishedUrlProvider; } + /// public async Task GetByIdAsync(int id, bool? preview = null) => await _documentCacheService.GetByIdAsync(id, preview); + /// public async Task GetByIdAsync(Guid key, bool? preview = null) => await _documentCacheService.GetByKeyAsync(key, preview); + /// public IPublishedContent? GetById(bool preview, int contentId) => GetByIdAsync(contentId, preview).GetAwaiter().GetResult(); + /// 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 @@ public DocumentCache( return GetByIdAsync(contentId, preview).GetAwaiter().GetResult(); } - + /// public IPublishedContent? GetById(int contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult(); + /// public IPublishedContent? GetById(Guid contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult(); + /// + /// Gets the published documents at the root of the content tree. + /// + /// A value indicating whether to consider unpublished content. + /// + /// The culture to filter root documents by. When null, all root documents are returned; otherwise only those + /// that are invariant or vary for the specified culture are returned. + /// + /// The published documents at root level available for the specified culture. [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.")] diff --git a/src/Umbraco.PublishedCache.HybridCache/MediaCache.cs b/src/Umbraco.PublishedCache.HybridCache/MediaCache.cs index 97bbd3eba5e1..fa1dcd35f7c2 100644 --- a/src/Umbraco.PublishedCache.HybridCache/MediaCache.cs +++ b/src/Umbraco.PublishedCache.HybridCache/MediaCache.cs @@ -5,26 +5,54 @@ namespace Umbraco.Cms.Infrastructure.HybridCache; +/// +/// Provides access to published media items held in the hybrid cache. +/// 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. + + /// + /// Initializes a new instance of the class. + /// + /// The service that retrieves and caches published media nodes. + /// The cache of published content types. + /// The service used to query the media navigation structure. + 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; } + /// public async Task GetByIdAsync(int id) => await _mediaCacheService.GetByIdAsync(id); + /// public async Task GetByIdAsync(Guid key) => await _mediaCacheService.GetByKeyAsync(key); + /// public IPublishedContent? GetById(bool preview, int contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult(); - public IPublishedContent? GetById(bool preview, Guid contentId) + /// + /// + /// 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. + /// + public IPublishedContent? GetById(bool preview, Guid contentId) => GetById(contentId); + + /// + public IPublishedContent? GetById(int contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult(); + + /// + 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 @@ public MediaCache(IMediaCacheService mediaCacheService, IPublishedContentTypeCac return GetByIdAsync(contentId).GetAwaiter().GetResult(); } - - public IPublishedContent? GetById(int contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult(); - - public IPublishedContent? GetById(Guid contentId) => GetByIdAsync(contentId).GetAwaiter().GetResult(); - + /// + /// Gets the published media items at the root of the media tree. + /// + /// A value indicating whether to consider unpublished items. Media has no draft state, so this has no effect. + /// + /// The culture to filter root media by. When null, all root media are returned; otherwise only those that are + /// invariant or vary for the specified culture are returned. + /// + /// The published media items at root level available for the specified culture. public IEnumerable GetAtRoot(bool preview, string? culture = null) { if (_mediaNavigationQueryService.TryGetRootKeys(out IEnumerable rootKeys) is false) diff --git a/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs b/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs index da6f4df15a7c..ada5d4a66a10 100644 --- a/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs +++ b/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs @@ -104,16 +104,7 @@ public MediaCacheService( /// public long? GetApproximateBytes() => _publishedContentCache.ApproximateSizeInBytes; - public async Task GetByKeyAsync(Guid key) - { - Attempt idAttempt = _idKeyMap.GetIdForKey(key, UmbracoObjectTypes.Media); - if (idAttempt.Success is false) - { - return null; - } - - return await GetNodeAsync(key); - } + public Task GetByKeyAsync(Guid key) => GetNodeAsync(key); public async Task GetByIdAsync(int id) { diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/MediaCacheSyncFastPathTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/MediaCacheSyncFastPathTests.cs new file mode 100644 index 000000000000..342c971c0db2 --- /dev/null +++ b/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; + +/// +/// Tests that consults +/// first and skips the async fallback entirely when the L0 cache has the requested item. +/// +/// +/// The async path (GetByKeyAsync) 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 +/// FilterAvailable's lazy chain to take the fast path. Because FilterAvailable +/// binds its method-group call (candidateKeys.Select(GetById)) to the single-argument +/// 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. +/// +[TestFixture] +public class MediaCacheSyncFastPathTests +{ + [Test] + public void GetById_Guid_HitsL0ViaTryGetCached_ReturnsCachedAndSkipsAsync() + { + IPublishedContent expected = Mock.Of(); + 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()), + Times.Never, + "Async path should not run when TryGetCached hits"); + } + + [Test] + public void GetById_PreviewGuid_HitsL0ViaTryGetCached_ReturnsCachedAndSkipsAsync() + { + IPublishedContent expected = Mock.Of(); + 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()), + Times.Never, + "Async path should not run when TryGetCached hits"); + } + + [Test] + public void GetById_Guid_MissesL0_FallsThroughToAsyncPath() + { + IPublishedContent expected = Mock.Of(); + var cacheService = new Mock(); + cacheService + .Setup(s => s.TryGetCached(It.IsAny(), out It.Ref.IsAny)) + .Returns(new TryGetCachedDelegate((Guid _, out IPublishedContent? content) => + { + content = null; + return false; + })); + cacheService + .Setup(s => s.GetByKeyAsync(It.IsAny())) + .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()), + Times.Once, + "Async path runs exactly once on TryGetCached miss"); + } + + private static Mock CreateHitCacheService(IPublishedContent expected) + { + var cacheService = new Mock(); + cacheService + .Setup(s => s.TryGetCached(It.IsAny(), out It.Ref.IsAny)) + .Returns(new TryGetCachedDelegate((Guid _, out IPublishedContent? content) => + { + content = expected; + return true; + })); + return cacheService; + } + + private static MediaCache CreateCache(Mock cacheService) + => new( + cacheService.Object, + Mock.Of(), + Mock.Of()); + + // 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); +}