diff --git a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs index 0cb3c52ae287..583303422127 100644 --- a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs @@ -399,7 +399,6 @@ private void HandleNavigationForSingleContent(IContent content) private async Task HandlePublishedAsync(JsonPayload payload, CancellationToken cancellationToken) { - if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll)) { await _publishStatusManagementService.InitializeAsync(cancellationToken); @@ -414,16 +413,20 @@ private async Task HandlePublishedAsync(JsonPayload payload, CancellationToken c { await _publishStatusManagementService.RemoveAsync(payload.Key.Value, cancellationToken); } - else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshNode)) + else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshNode) && HasPublishStatusUpdates(payload)) { await _publishStatusManagementService.AddOrUpdateStatusAsync(payload.Key.Value, cancellationToken); } - else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshBranch)) + else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshBranch) && HasPublishStatusUpdates(payload)) { await _publishStatusManagementService.AddOrUpdateStatusWithDescendantsAsync(payload.Key.Value, cancellationToken); } } + private static bool HasPublishStatusUpdates(JsonPayload payload) => + (payload.PublishedCultures is not null && payload.PublishedCultures.Length > 0) || + (payload.UnpublishedCultures is not null && payload.UnpublishedCultures.Length > 0); + private void HandleIdKeyMap(JsonPayload payload) { // We only need to flush the ID/Key map when content is deleted. diff --git a/src/Umbraco.Core/Services/PublishStatus/PublishStatusService.cs b/src/Umbraco.Core/Services/PublishStatus/PublishStatusService.cs index 43b954a7f7b5..4f04853f51b8 100644 --- a/src/Umbraco.Core/Services/PublishStatus/PublishStatusService.cs +++ b/src/Umbraco.Core/Services/PublishStatus/PublishStatusService.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using Microsoft.Extensions.Logging; using Umbraco.Cms.Core.Persistence.Repositories; using Umbraco.Cms.Core.Scoping; @@ -16,7 +17,7 @@ public class PublishStatusService : IPublishStatusManagementService, IPublishSta private readonly ILanguageService _languageService; private readonly IDocumentNavigationQueryService _documentNavigationQueryService; - private readonly IDictionary> _publishedCultures = new Dictionary>(); + private readonly ConcurrentDictionary> _publishedCultures = new(); private string? DefaultCulture { get; set; } @@ -126,7 +127,7 @@ public async Task AddOrUpdateStatusAsync(Guid documentKey, CancellationToken can /// public Task RemoveAsync(Guid documentKey, CancellationToken cancellationToken) { - _publishedCultures.Remove(documentKey); + _publishedCultures.TryRemove(documentKey, out _); return Task.CompletedTask; } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/PublishStatusServiceTests.ThreadSafety.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/PublishStatusServiceTests.ThreadSafety.cs new file mode 100644 index 000000000000..151f106a10f0 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/PublishStatusServiceTests.ThreadSafety.cs @@ -0,0 +1,185 @@ +using NUnit.Framework; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Services; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; + +internal sealed partial class PublishStatusServiceTests +{ + [Test] + public async Task Concurrent_Reads_And_Writes_Do_Not_Throw() + { + // Arrange + var sut = CreatePublishedStatusService(); + await sut.InitializeAsync(CancellationToken.None); + + const int numberOfOperations = 1000; + var documentKeys = Enumerable.Range(0, 100).Select(_ => Guid.NewGuid()).ToArray(); + var exceptions = new List(); + var lockObj = new object(); + + // Act - run concurrent reads and writes + var tasks = new List(); + + // Writers - AddOrUpdateStatusAsync + for (var i = 0; i < numberOfOperations; i++) + { + var key = documentKeys[i % documentKeys.Length]; + tasks.Add(RunWithSuppressedExecutionContext(async () => + { + try + { + await sut.AddOrUpdateStatusAsync(key, CancellationToken.None); + } + catch (Exception ex) + { + lock (lockObj) + { + exceptions.Add(ex); + } + } + })); + } + + // Readers - IsDocumentPublished + for (var i = 0; i < numberOfOperations; i++) + { + var key = documentKeys[i % documentKeys.Length]; + tasks.Add(RunWithSuppressedExecutionContext(() => + { + try + { + _ = sut.IsDocumentPublished(key, DefaultCulture); + } + catch (Exception ex) + { + lock (lockObj) + { + exceptions.Add(ex); + } + } + + return Task.CompletedTask; + })); + } + + // Readers - IsDocumentPublishedInAnyCulture + for (var i = 0; i < numberOfOperations; i++) + { + var key = documentKeys[i % documentKeys.Length]; + tasks.Add(RunWithSuppressedExecutionContext(() => + { + try + { + _ = sut.IsDocumentPublishedInAnyCulture(key); + } + catch (Exception ex) + { + lock (lockObj) + { + exceptions.Add(ex); + } + } + + return Task.CompletedTask; + })); + } + + // Removers + for (var i = 0; i < numberOfOperations; i++) + { + var key = documentKeys[i % documentKeys.Length]; + tasks.Add(RunWithSuppressedExecutionContext(async () => + { + try + { + await sut.RemoveAsync(key, CancellationToken.None); + } + catch (Exception ex) + { + lock (lockObj) + { + exceptions.Add(ex); + } + } + })); + } + + await Task.WhenAll(tasks); + + // Assert + Assert.IsEmpty(exceptions, $"Expected no exceptions but got {exceptions.Count}: {string.Join(", ", exceptions.Select(e => e.Message))}"); + } + + [Test] + public async Task Concurrent_Initialize_And_Queries_Do_Not_Throw() + { + // Arrange + var sut = CreatePublishedStatusService(); + + // Publish some content first so InitializeAsync has data to load + ContentService.PublishBranch(Textpage, PublishBranchFilter.IncludeUnpublished, ["*"]); + + const int numberOfOperations = 100; + var exceptions = new List(); + var lockObj = new object(); + + // Act - run concurrent initializations and queries + var tasks = new List(); + + // Multiple initializations (simulates cache rebuild) + for (var i = 0; i < 10; i++) + { + tasks.Add(RunWithSuppressedExecutionContext(async () => + { + try + { + await sut.InitializeAsync(CancellationToken.None); + } + catch (Exception ex) + { + lock (lockObj) + { + exceptions.Add(ex); + } + } + })); + } + + // Concurrent reads during initialization + for (var i = 0; i < numberOfOperations; i++) + { + tasks.Add(RunWithSuppressedExecutionContext(() => + { + try + { + _ = sut.IsDocumentPublished(Textpage.Key, DefaultCulture); + _ = sut.IsDocumentPublishedInAnyCulture(Subpage.Key); + _ = sut.HasPublishedAncestorPath(Subpage2.Key); + } + catch (Exception ex) + { + lock (lockObj) + { + exceptions.Add(ex); + } + } + + return Task.CompletedTask; + })); + } + + await Task.WhenAll(tasks); + + // Assert + Assert.IsEmpty(exceptions, $"Expected no exceptions but got {exceptions.Count}: {string.Join(", ", exceptions.Select(e => e.Message))}"); + } + + private static Task RunWithSuppressedExecutionContext(Func action) + { + using (ExecutionContext.SuppressFlow()) + { + return Task.Run(action); + } + } +} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj b/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj index 935828e92dde..fb192f687b2e 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj +++ b/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj @@ -280,6 +280,9 @@ PublishStatusServiceTests.cs + + PublishStatusServiceTests.cs + UserStartNodeEntitiesServiceTests.cs