From 5488c77e0e960899c28f64fd51bffd8759a533cd Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Tue, 21 Oct 2025 15:26:29 +0200 Subject: [PATCH 1/2] Bumped version to 16.3.2. --- src/Umbraco.Web.UI.Client/package.json | 2 +- version.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/package.json b/src/Umbraco.Web.UI.Client/package.json index 766ffa96ba..74bf09f5a9 100644 --- a/src/Umbraco.Web.UI.Client/package.json +++ b/src/Umbraco.Web.UI.Client/package.json @@ -1,7 +1,7 @@ { "name": "@umbraco-cms/backoffice", "license": "MIT", - "version": "16.3.1", + "version": "16.3.2", "type": "module", "exports": { ".": null, diff --git a/version.json b/version.json index 922b55eca9..7167818c6d 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "16.3.1", + "version": "16.3.2", "assemblyVersion": { "precision": "build" }, From 8aa9dc8f1938aae457d4e22dcf5c96272b627057 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Tue, 21 Oct 2025 09:57:29 +0200 Subject: [PATCH 2/2] Hybrid Cache: Resolve start-up errors with mis-matched types (#20554) * Be consistent in use of GetOrCreateAsync overload in exists and retrieval. Ensure nullability of ContentCacheNode is consistent in exists and retrieval. * Applied suggestion from code review. * Move seeding to Umbraco application starting rather than started, ensuring an initial request is served. * Tighten up hybrid cache exists check with locking around check and remove, and use of cancellation token. --- .../UmbracoBuilderExtensions.cs | 3 +- .../Extensions/HybridCacheExtensions.cs | 67 +++++++++++++------ .../SeedingNotificationHandler.cs | 6 +- .../Services/DocumentCacheService.cs | 4 +- .../Services/MediaCacheService.cs | 4 +- .../DocumentHybridCacheTests.cs | 60 ++++++++++++++++- .../Extensions/HybridCacheExtensionsTests.cs | 55 +++++++-------- 7 files changed, 141 insertions(+), 58 deletions(-) diff --git a/src/Umbraco.PublishedCache.HybridCache/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.PublishedCache.HybridCache/DependencyInjection/UmbracoBuilderExtensions.cs index ca625dacdf..e97c60fd62 100644 --- a/src/Umbraco.PublishedCache.HybridCache/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.PublishedCache.HybridCache/DependencyInjection/UmbracoBuilderExtensions.cs @@ -1,4 +1,3 @@ - using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; @@ -74,7 +73,7 @@ public static class UmbracoBuilderExtensions builder.AddNotificationAsyncHandler(); builder.AddNotificationAsyncHandler(); builder.AddNotificationAsyncHandler(); - builder.AddNotificationAsyncHandler(); + builder.AddNotificationAsyncHandler(); builder.AddCacheSeeding(); return builder; } diff --git a/src/Umbraco.PublishedCache.HybridCache/Extensions/HybridCacheExtensions.cs b/src/Umbraco.PublishedCache.HybridCache/Extensions/HybridCacheExtensions.cs index ee1b7aefda..ccd5897494 100644 --- a/src/Umbraco.PublishedCache.HybridCache/Extensions/HybridCacheExtensions.cs +++ b/src/Umbraco.PublishedCache.HybridCache/Extensions/HybridCacheExtensions.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using Microsoft.Extensions.Caching.Hybrid; namespace Umbraco.Cms.Infrastructure.HybridCache.Extensions; @@ -7,19 +8,24 @@ namespace Umbraco.Cms.Infrastructure.HybridCache.Extensions; /// internal static class HybridCacheExtensions { + // Per-key semaphores to ensure the GetOrCreateAsync + RemoveAsync sequence + // executes atomically for a given cache key. + private static readonly ConcurrentDictionary _keyLocks = new(); + /// /// Returns true if the cache contains an item with a matching key. /// /// An instance of /// The name (key) of the item to search for in the cache. + /// The cancellation token. /// True if the item exists already. False if it doesn't. /// /// Hat-tip: https://github.com/dotnet/aspnetcore/discussions/57191 /// Will never add or alter the state of any items in the cache. /// - public static async Task ExistsAsync(this Microsoft.Extensions.Caching.Hybrid.HybridCache cache, string key) + public static async Task ExistsAsync(this Microsoft.Extensions.Caching.Hybrid.HybridCache cache, string key, CancellationToken token) { - (bool exists, _) = await TryGetValueAsync(cache, key); + (bool exists, _) = await TryGetValueAsync(cache, key, token).ConfigureAwait(false); return exists; } @@ -29,34 +35,55 @@ internal static class HybridCacheExtensions /// The type of the value of the item in the cache. /// An instance of /// The name (key) of the item to search for in the cache. + /// The cancellation token. /// A tuple of and the object (if found) retrieved from the cache. /// /// Hat-tip: https://github.com/dotnet/aspnetcore/discussions/57191 /// Will never add or alter the state of any items in the cache. /// - public static async Task<(bool Exists, T? Value)> TryGetValueAsync(this Microsoft.Extensions.Caching.Hybrid.HybridCache cache, string key) + public static async Task<(bool Exists, T? Value)> TryGetValueAsync(this Microsoft.Extensions.Caching.Hybrid.HybridCache cache, string key, CancellationToken token) { var exists = true; - T? result = await cache.GetOrCreateAsync( - key, - null!, - (_, _) => - { - exists = false; - return new ValueTask(default(T)!); - }, - new HybridCacheEntryOptions(), - null, - CancellationToken.None); + // Acquire a per-key semaphore so that GetOrCreateAsync and the possible RemoveAsync + // complete without another thread retrieving/creating the same key in-between. + SemaphoreSlim sem = _keyLocks.GetOrAdd(key, _ => new SemaphoreSlim(1, 1)); - // In checking for the existence of the item, if not found, we will have created a cache entry with a null value. - // So remove it again. - if (exists is false) + await sem.WaitAsync().ConfigureAwait(false); + + try { - await cache.RemoveAsync(key); - } + T? result = await cache.GetOrCreateAsync( + key, + cancellationToken => + { + exists = false; + return default; + }, + new HybridCacheEntryOptions(), + null, + token).ConfigureAwait(false); - return (exists, result); + // In checking for the existence of the item, if not found, we will have created a cache entry with a null value. + // So remove it again. Because we're holding the per-key lock there is no chance another thread + // will observe the temporary entry between GetOrCreateAsync and RemoveAsync. + if (exists is false) + { + await cache.RemoveAsync(key).ConfigureAwait(false); + } + + return (exists, result); + } + finally + { + sem.Release(); + + // Only remove the semaphore mapping if it still points to the same instance we used. + // This avoids removing another thread's semaphore or corrupting the map. + if (_keyLocks.TryGetValue(key, out SemaphoreSlim? current) && ReferenceEquals(current, sem)) + { + _keyLocks.TryRemove(key, out _); + } + } } } diff --git a/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/SeedingNotificationHandler.cs b/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/SeedingNotificationHandler.cs index 0581bd2654..38a6618c70 100644 --- a/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/SeedingNotificationHandler.cs +++ b/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/SeedingNotificationHandler.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.Options; +using Microsoft.Extensions.Options; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Events; @@ -9,7 +9,7 @@ using Umbraco.Cms.Infrastructure.HybridCache.Services; namespace Umbraco.Cms.Infrastructure.HybridCache.NotificationHandlers; -internal sealed class SeedingNotificationHandler : INotificationAsyncHandler +internal sealed class SeedingNotificationHandler : INotificationAsyncHandler { private readonly IDocumentCacheService _documentCacheService; private readonly IMediaCacheService _mediaCacheService; @@ -24,7 +24,7 @@ internal sealed class SeedingNotificationHandler : INotificationAsyncHandler(cacheKey); + var existsInCache = await _hybridCache.ExistsAsync(cacheKey, cancellationToken).ConfigureAwait(false); if (existsInCache is false) { uncachedKeys.Add(key); @@ -278,7 +278,7 @@ internal sealed class DocumentCacheService : IDocumentCacheService return false; } - return await _hybridCache.ExistsAsync(GetCacheKey(keyAttempt.Result, preview)); + return await _hybridCache.ExistsAsync(GetCacheKey(keyAttempt.Result, preview), CancellationToken.None); } public async Task RefreshContentAsync(IContent content) diff --git a/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs b/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs index 65b8f91945..46d782bdbe 100644 --- a/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs +++ b/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs @@ -133,7 +133,7 @@ internal sealed class MediaCacheService : IMediaCacheService return false; } - return await _hybridCache.ExistsAsync($"{keyAttempt.Result}"); + return await _hybridCache.ExistsAsync($"{keyAttempt.Result}", CancellationToken.None); } public async Task RefreshMediaAsync(IMedia media) @@ -170,7 +170,7 @@ internal sealed class MediaCacheService : IMediaCacheService var cacheKey = GetCacheKey(key, false); - var existsInCache = await _hybridCache.ExistsAsync(cacheKey); + var existsInCache = await _hybridCache.ExistsAsync(cacheKey, CancellationToken.None); if (existsInCache is false) { uncachedKeys.Add(key); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheTests.cs index 0c4207331a..088f14cd31 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheTests.cs @@ -7,6 +7,7 @@ using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Sync; +using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; using Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; @@ -25,10 +26,10 @@ internal sealed class DocumentHybridCacheTests : UmbracoIntegrationTestWithConte private IPublishedContentCache PublishedContentHybridCache => GetRequiredService(); - private IContentEditingService ContentEditingService => GetRequiredService(); - private IContentPublishingService ContentPublishingService => GetRequiredService(); + private IDocumentCacheService DocumentCacheService => GetRequiredService(); + private const string NewName = "New Name"; private const string NewTitle = "New Title"; @@ -460,6 +461,61 @@ internal sealed class DocumentHybridCacheTests : UmbracoIntegrationTestWithConte Assert.IsNull(textPage); } + [Test] + public async Task Can_Get_Published_Content_By_Id_After_Previous_Check_Where_Not_Found() + { + // Arrange + var testPageKey = Guid.NewGuid(); + + // Act & Assert + // - assert we cannot get the content that doesn't yet exist from the cache + var testPage = await PublishedContentHybridCache.GetByIdAsync(testPageKey); + Assert.IsNull(testPage); + + testPage = await PublishedContentHybridCache.GetByIdAsync(testPageKey); + Assert.IsNull(testPage); + + // - create and publish the content + var testPageContent = ContentEditingBuilder.CreateBasicContent(ContentType.Key, testPageKey); + var createResult = await ContentEditingService.CreateAsync(testPageContent, Constants.Security.SuperUserKey); + Assert.IsTrue(createResult.Success); + var publishResult = await ContentPublishingService.PublishAsync(testPageKey, CultureAndSchedule, Constants.Security.SuperUserKey); + Assert.IsTrue(publishResult.Success); + + // - assert we can now get the content from the cache + testPage = await PublishedContentHybridCache.GetByIdAsync(testPageKey); + Assert.IsNotNull(testPage); + } + + [Test] + public async Task Can_Get_Published_Content_By_Id_After_Previous_Exists_Check() + { + // Act + var hasContentForTextPageCached = await DocumentCacheService.HasContentByIdAsync(PublishedTextPageId); + Assert.IsTrue(hasContentForTextPageCached); + var textPage = await PublishedContentHybridCache.GetByIdAsync(PublishedTextPageId); + + // Assert + AssertPublishedTextPage(textPage); + } + + [Test] + public async Task Can_Do_Exists_Check_On_Created_Published_Content() + { + var testPageKey = Guid.NewGuid(); + var testPageContent = ContentEditingBuilder.CreateBasicContent(ContentType.Key, testPageKey); + var createResult = await ContentEditingService.CreateAsync(testPageContent, Constants.Security.SuperUserKey); + Assert.IsTrue(createResult.Success); + var publishResult = await ContentPublishingService.PublishAsync(testPageKey, CultureAndSchedule, Constants.Security.SuperUserKey); + Assert.IsTrue(publishResult.Success); + + var testPage = await PublishedContentHybridCache.GetByIdAsync(testPageKey); + Assert.IsNotNull(testPage); + + var hasContentForTextPageCached = await DocumentCacheService.HasContentByIdAsync(testPage.Id); + Assert.IsTrue(hasContentForTextPageCached); + } + private void AssertTextPage(IPublishedContent textPage) { Assert.Multiple(() => diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/Extensions/HybridCacheExtensionsTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/Extensions/HybridCacheExtensionsTests.cs index 152fe28b4e..8da30cd118 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/Extensions/HybridCacheExtensionsTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/Extensions/HybridCacheExtensionsTests.cs @@ -1,3 +1,4 @@ +using System; using Microsoft.Extensions.Caching.Hybrid; using Moq; using NUnit.Framework; @@ -33,15 +34,15 @@ public class HybridCacheExtensionsTests _cacheMock .Setup(cache => cache.GetOrCreateAsync( key, - null!, - It.IsAny>>(), + It.IsAny>>(), + It.IsAny>, CancellationToken, ValueTask>>(), It.IsAny(), null, CancellationToken.None)) .ReturnsAsync(expectedValue); // Act - var exists = await HybridCacheExtensions.ExistsAsync(_cacheMock.Object, key); + var exists = await HybridCacheExtensions.ExistsAsync(_cacheMock.Object, key, CancellationToken.None); // Assert Assert.IsTrue(exists); @@ -56,24 +57,24 @@ public class HybridCacheExtensionsTests _cacheMock .Setup(cache => cache.GetOrCreateAsync( key, - null!, - It.IsAny>>(), + It.IsAny>>(), + It.IsAny>, CancellationToken, ValueTask>>(), It.IsAny(), null, CancellationToken.None)) .Returns(( string key, - object? state, - Func> factory, + Func> state, + Func>, CancellationToken, ValueTask> factory, HybridCacheEntryOptions? options, IEnumerable? tags, CancellationToken token) => { - return factory(state!, token); + return factory(state, token); }); // Act - var exists = await HybridCacheExtensions.ExistsAsync(_cacheMock.Object, key); + var exists = await HybridCacheExtensions.ExistsAsync(_cacheMock.Object, key, CancellationToken.None); // Assert Assert.IsFalse(exists); @@ -89,15 +90,15 @@ public class HybridCacheExtensionsTests _cacheMock .Setup(cache => cache.GetOrCreateAsync( key, - null!, - It.IsAny>>(), + It.IsAny>>(), + It.IsAny>, CancellationToken, ValueTask>>(), It.IsAny(), null, CancellationToken.None)) .ReturnsAsync(expectedValue); // Act - var (exists, value) = await HybridCacheExtensions.TryGetValueAsync(_cacheMock.Object, key); + var (exists, value) = await HybridCacheExtensions.TryGetValueAsync(_cacheMock.Object, key, CancellationToken.None); // Assert Assert.IsTrue(exists); @@ -114,15 +115,15 @@ public class HybridCacheExtensionsTests _cacheMock .Setup(cache => cache.GetOrCreateAsync( key, - null!, - It.IsAny>>(), + It.IsAny>>(), + It.IsAny>, CancellationToken, ValueTask>>(), It.IsAny(), null, CancellationToken.None)) .ReturnsAsync(expectedValue); // Act - var (exists, value) = await HybridCacheExtensions.TryGetValueAsync(_cacheMock.Object, key); + var (exists, value) = await HybridCacheExtensions.TryGetValueAsync(_cacheMock.Object, key, CancellationToken.None); // Assert Assert.IsTrue(exists); @@ -138,15 +139,15 @@ public class HybridCacheExtensionsTests _cacheMock .Setup(cache => cache.GetOrCreateAsync( key, - null!, - It.IsAny>>(), + It.IsAny>>(), + It.IsAny>, CancellationToken, ValueTask>>(), It.IsAny(), null, CancellationToken.None)) .ReturnsAsync(null!); // Act - var (exists, value) = await HybridCacheExtensions.TryGetValueAsync(_cacheMock.Object, key); + var (exists, value) = await HybridCacheExtensions.TryGetValueAsync(_cacheMock.Object, key, CancellationToken.None); // Assert Assert.IsTrue(exists); @@ -160,16 +161,16 @@ public class HybridCacheExtensionsTests string key = "test-key"; _cacheMock.Setup(cache => cache.GetOrCreateAsync( - key, - null, - It.IsAny>>(), - It.IsAny(), - null, - CancellationToken.None)) + key, + It.IsAny>>(), + It.IsAny>, CancellationToken, ValueTask>>(), + It.IsAny(), + null, + CancellationToken.None)) .Returns(( string key, - object? state, - Func> factory, + Func> state, + Func>, CancellationToken, ValueTask> factory, HybridCacheEntryOptions? options, IEnumerable? tags, CancellationToken token) => @@ -178,7 +179,7 @@ public class HybridCacheExtensionsTests }); // Act - var (exists, value) = await HybridCacheExtensions.TryGetValueAsync(_cacheMock.Object, key); + var (exists, value) = await HybridCacheExtensions.TryGetValueAsync(_cacheMock.Object, key, CancellationToken.None); // Assert Assert.IsFalse(exists);