Skip to content

Performance: Use request cache for ContentSourceDto retrieval #21282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
AndyButland wants to merge 3 commits into from
Closed

Performance: Use request cache for ContentSourceDto retrieval #21282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
615e115
Use request cache on retrieval of ContentSourceDto in DatabaseCacheRe…
AndyButland Jan 3, 2026
bff162f
Added parameter so cache is only used when refreshing the memory cach…
AndyButland Jan 4, 2026
3336e2b
Fixed failing integration tests.
AndyButland Jan 4, 2026
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
40 changes: 37 additions & 3 deletions src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Diagnostics;

Check notice on line 1 in src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Code Duplication 

reduced similar code in: GetContentSourceAsync. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

Check notice on line 1 in src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ No longer an issue: Complex Conditional 

GetContentSourceAsync no longer has a complex conditional. A complex conditional is an expression inside a branch (e.g. if, for, while) which consists of multiple, logical operators such as AND/OR. The more logical operators in an expression, the more severe the code smell.

Check warning on line 1 in src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Primitive Obsession 

In this module, 33.3% of all function arguments are primitive types, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using NPoco;
Expand Down Expand Up @@ -141,20 +141,54 @@
}

/// <inheritdoc/>
[Obsolete("Please use the method overload takng all parameters. Scheduled for removal in Umbraco 19.")]
public async Task<ContentCacheNode?> GetContentSourceAsync(Guid key, bool preview = false)
=> await GetContentSourceAsync(key, preview, false);

/// <inheritdoc/>
public async Task<ContentCacheNode?> GetContentSourceAsync(Guid key, bool preview = false, bool useCache = false)
{
ContentSourceDto? dto = await GetContentSourceDto(key, useCache);

if (dto == null)
{
return null;
}

return CreateContentCacheNode(dto, preview);
}

private async Task<ContentSourceDto?> GetContentSourceDto(Guid key, bool useRequestCache)
{
var cacheKey = $"{nameof(DatabaseCacheRepository)}_ContentSourceDto_{key}";

ContentSourceDto? dto;
if (useRequestCache)
{
dto = AppCaches.RequestCache.GetCacheItem<ContentSourceDto>(cacheKey);
if (dto is not null)
{
return dto;
}
}

Sql<ISqlContext>? sql = SqlContentSourcesSelect()
.Append(SqlObjectTypeNotTrashed(SqlContext, Constants.ObjectTypes.Document))
.Append(SqlWhereNodeKey(SqlContext, key))
.Append(SqlOrderByLevelIdSortOrder(SqlContext));

ContentSourceDto? dto = await Database.FirstOrDefaultAsync<ContentSourceDto>(sql);
dto = await Database.FirstOrDefaultAsync<ContentSourceDto>(sql);

if (dto == null)
if (useRequestCache && dto is not null)
{
return null;
AppCaches.RequestCache.Set(cacheKey, dto);
}

return dto;
}

private ContentCacheNode? CreateContentCacheNode(ContentSourceDto dto, bool preview)
{
if (preview is false && dto.PubDataRaw is null && dto.PubData is null)
{
return null;
Expand Down
10 changes: 8 additions & 2 deletions src/Umbraco.PublishedCache.HybridCache/Persistence/IDatabaseCacheRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,16 @@ internal interface IDatabaseCacheRepository
Task DeleteContentItemAsync(int id);

/// <summary>
/// Gets a single cache node for a document key.
/// Gets a single cache node for a document key and preview status.
/// </summary>
[Obsolete("Please use the method overload takng all parameters. Scheduled for removal in Umbraco 19.")]
Task<ContentCacheNode?> GetContentSourceAsync(Guid key, bool preview = false);

/// <summary>
/// Gets a single cache node for a document key and preview status.
/// </summary>
Task<ContentCacheNode?> GetContentSourceAsync(Guid key, bool preview = false, bool useCache = false);

/// <summary>
/// Gets a collection of cache nodes for a collection of document keys.
/// </summary>
Expand All @@ -27,7 +33,7 @@ async Task<IEnumerable<ContentCacheNode>> GetContentSourcesAsync(IEnumerable<Gui
var contentCacheNodes = new List<ContentCacheNode>();
foreach (Guid key in keys)
{
ContentCacheNode? contentSource = await GetContentSourceAsync(key, preview);
ContentCacheNode? contentSource = await GetContentSourceAsync(key, preview, false);
if (contentSource is not null)
{
contentCacheNodes.Add(contentSource);
Expand Down
6 changes: 3 additions & 3 deletions src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public DocumentCacheService(
async cancel =>
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
ContentCacheNode? contentCacheNode = await _databaseCacheRepository.GetContentSourceAsync(key, preview);
ContentCacheNode? contentCacheNode = await _databaseCacheRepository.GetContentSourceAsync(key, preview, false);

// If we can resolve the content cache node, we still need to check if the ancestor path is published.
// This does cost some performance, but it's necessary to ensure that the content is actually published.
Expand Down Expand Up @@ -180,13 +180,13 @@ public async Task RefreshMemoryCacheAsync(Guid key)
using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.ContentTree);

ContentCacheNode? draftNode = await _databaseCacheRepository.GetContentSourceAsync(key, true);
ContentCacheNode? draftNode = await _databaseCacheRepository.GetContentSourceAsync(key, true, false);
if (draftNode is not null)
{
await _hybridCache.SetAsync(GetCacheKey(draftNode.Key, true), draftNode, GetEntryOptions(draftNode.Key, true), GenerateTags(key));
}

ContentCacheNode? publishedNode = await _databaseCacheRepository.GetContentSourceAsync(key, false);
ContentCacheNode? publishedNode = await _databaseCacheRepository.GetContentSourceAsync(key, false, true);
if (publishedNode is not null && _publishStatusQueryService.HasPublishedAncestorPath(publishedNode.Key))
{
var cacheKey = GetCacheKey(publishedNode.Key, false);
Expand Down
12 changes: 6 additions & 6 deletions ...raco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheMockTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ public void SetUp()
IsDraft = false,
};

_mockDatabaseCacheRepository.Setup(r => r.GetContentSourceAsync(It.IsAny<Guid>(), true))
_mockDatabaseCacheRepository.Setup(r => r.GetContentSourceAsync(It.IsAny<Guid>(), true, It.IsAny<bool>()))
.ReturnsAsync(draftTestCacheNode);
_mockDatabaseCacheRepository.Setup(r => r.GetContentSourcesAsync(It.IsAny<IEnumerable<Guid>>(), true))
.ReturnsAsync([draftTestCacheNode]);

_mockDatabaseCacheRepository.Setup(r => r.GetContentSourceAsync(It.IsAny<Guid>(), false))
_mockDatabaseCacheRepository.Setup(r => r.GetContentSourceAsync(It.IsAny<Guid>(), false, It.IsAny<bool>()))
.ReturnsAsync(publishedTestCacheNode);
_mockDatabaseCacheRepository.Setup(r => r.GetContentSourcesAsync(It.IsAny<IEnumerable<Guid>>(), false))
.ReturnsAsync([publishedTestCacheNode]);
Expand Down Expand Up @@ -148,7 +148,7 @@ public async Task Content_Is_Cached_By_Key()
var textPage2 = await _mockedCache.GetByIdAsync(Textpage.Key, true);
AssertTextPage(textPage);
AssertTextPage(textPage2);
_mockDatabaseCacheRepository.Verify(x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>()), Times.Exactly(1));
_mockDatabaseCacheRepository.Verify(x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>(), It.IsAny<bool>()), Times.Exactly(1));
}

[Test]
Expand All @@ -160,7 +160,7 @@ public async Task Content_Is_Cached_By_Id()
var textPage2 = await _mockedCache.GetByIdAsync(Textpage.Id, true);
AssertTextPage(textPage);
AssertTextPage(textPage2);
_mockDatabaseCacheRepository.Verify(x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>()), Times.Exactly(1));
_mockDatabaseCacheRepository.Verify(x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>(), It.IsAny<bool>()), Times.Exactly(1));
}

[Test]
Expand Down Expand Up @@ -219,7 +219,7 @@ public async Task Content_Is_Not_Seeded_If_Unpblished_By_Id()
var textPage = await _mockedCache.GetByIdAsync(Textpage.Id, true);
AssertTextPage(textPage);

_mockDatabaseCacheRepository.Verify(x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>()), Times.Exactly(1));
_mockDatabaseCacheRepository.Verify(x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>(), It.IsAny<bool>()), Times.Exactly(1));
}

[Test]
Expand All @@ -232,7 +232,7 @@ public async Task Content_Is_Not_Seeded_If_Unpublished_By_Key()
var textPage = await _mockedCache.GetByIdAsync(Textpage.Key, true);
AssertTextPage(textPage);

_mockDatabaseCacheRepository.Verify(x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>()), Times.Exactly(1));
_mockDatabaseCacheRepository.Verify(x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>(), It.IsAny<bool>()), Times.Exactly(1));
}

private void AssertTextPage(IPublishedContent textPage)
Expand Down
Loading