Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Collections.Concurrent;
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Scoping;
Expand All @@ -16,7 +17,7 @@ public class PublishStatusService : IPublishStatusManagementService, IPublishSta
private readonly ILanguageService _languageService;
private readonly IDocumentNavigationQueryService _documentNavigationQueryService;

private readonly IDictionary<Guid, ISet<string>> _publishedCultures = new Dictionary<Guid, ISet<string>>();
private readonly ConcurrentDictionary<Guid, ISet<string>> _publishedCultures = new();

private string? DefaultCulture { get; set; }

Expand Down Expand Up @@ -126,7 +127,7 @@ public async Task AddOrUpdateStatusAsync(Guid documentKey, CancellationToken can
/// <inheritdoc/>
public Task RemoveAsync(Guid documentKey, CancellationToken cancellationToken)
{
_publishedCultures.Remove(documentKey);
_publishedCultures.TryRemove(documentKey, out _);
Comment thread
AndyButland marked this conversation as resolved.
return Task.CompletedTask;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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();
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thread safety tests use random GUIDs that don't correspond to actual documents in the database. This means AddOrUpdateStatusAsync will query for non-existent documents and cache empty sets. Consider using actual document keys from the test setup (e.g., Textpage.Key, Subpage.Key) to test realistic scenarios where documents exist and have publish status data.

Copilot uses AI. Check for mistakes.
var exceptions = new List<Exception>();
var lockObj = new object();

// Act - run concurrent reads and writes
var tasks = new List<Task>();

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

Check warning on line 112 in tests/Umbraco.Tests.Integration/Umbraco.Core/Services/PublishStatusServiceTests.ThreadSafety.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Complex Method

Concurrent_Reads_And_Writes_Do_Not_Throw has a cyclomatic complexity of 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

[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<Exception>();
var lockObj = new object();

// Act - run concurrent initializations and queries
var tasks = new List<Task>();

// 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<Task> action)
{
using (ExecutionContext.SuppressFlow())
{
return Task.Run(action);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@
<Compile Update="Umbraco.Core\Services\PublishStatusServiceTests.Query.cs">
<DependentUpon>PublishStatusServiceTests.cs</DependentUpon>
</Compile>
<Compile Update="Umbraco.Core\Services\PublishStatusServiceTests.ThreadSafety.cs">
<DependentUpon>PublishStatusServiceTests.cs</DependentUpon>
</Compile>
<Compile Update="ManagementApi\Services\UserStartNodeEntitiesServiceTests.ChildUserAccessEntities.cs">
<DependentUpon>UserStartNodeEntitiesServiceTests.cs</DependentUpon>
</Compile>
Expand Down
Loading