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
@@ -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 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, 30.4% 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 @@ -143,14 +143,9 @@
/// <inheritdoc/>
public async Task<ContentCacheNode?> GetContentSourceAsync(Guid key, bool preview = false)
{
Sql<ISqlContext>? sql = SqlContentSourcesSelect()
.Append(SqlObjectTypeNotTrashed(SqlContext, Constants.ObjectTypes.Document))
.Append(SqlWhereNodeKey(SqlContext, key))
.Append(SqlOrderByLevelIdSortOrder(SqlContext));
ContentSourceDto? dto = await GetContentSourceDto(key);

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

if (dto == null)
if (dto is null)
{
return null;
}
Expand All @@ -160,6 +155,39 @@
return null;
}

return CreateContentNodeKit(preview, dto);
}

/// <inheritdoc/>
public async Task<(ContentCacheNode? Draft, ContentCacheNode? Published)> GetContentSourceForPublishStatesAsync(Guid key)
{
ContentSourceDto? dto = await GetContentSourceDto(key);

if (dto is null)
{
return (null, null);
}

ContentCacheNode draftNode = CreateContentNodeKit(true, dto);
Comment thread
AndyButland marked this conversation as resolved.
ContentCacheNode? publishedNode = dto.PubDataRaw is null && dto.PubData is null
? null
: CreateContentNodeKit(false, dto);

return (draftNode, publishedNode);
}

private async Task<ContentSourceDto?> GetContentSourceDto(Guid key)
{
Sql<ISqlContext>? sql = SqlContentSourcesSelect()
.Append(SqlObjectTypeNotTrashed(SqlContext, Constants.ObjectTypes.Document))
.Append(SqlWhereNodeKey(SqlContext, key))
.Append(SqlOrderByLevelIdSortOrder(SqlContext));

return await Database.FirstOrDefaultAsync<ContentSourceDto>(sql);
}

private ContentCacheNode CreateContentNodeKit(bool preview, ContentSourceDto dto)
{
IContentCacheDataSerializer serializer =
_contentCacheDataSerializerFactory.Create(ContentCacheDataSerializerEntityType.Document);
return CreateContentNodeKit(dto, serializer, preview);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,28 @@ internal interface IDatabaseCacheRepository
/// <summary>
/// Gets a single cache node for a document key.
/// </summary>
/// <param name="key">The document key.</param>
/// <param name="preview">A flag indicating whether to get the draft (preview) version or the published version.</param>
Task<ContentCacheNode?> GetContentSourceAsync(Guid key, bool preview = false);

/// <summary>
/// Gets both draft and published cache nodes for a document key in a single query.
/// </summary>
/// <param name="key">The document key.</param>
/// <returns>A tuple containing the draft and published cache nodes (either may be null).</returns>
// TODO (V18): Remove the default implementation on this method.
async Task<(ContentCacheNode? Draft, ContentCacheNode? Published)> GetContentSourceForPublishStatesAsync(Guid key)
{
ContentCacheNode? draftNode = await GetContentSourceAsync(key, preview: true);
ContentCacheNode? publishedNode = await GetContentSourceAsync(key, preview: false);
return (draftNode, publishedNode);
}

/// <summary>
/// Gets a collection of cache nodes for a collection of document keys.
/// </summary>
/// <param name="keys">The document keys.</param>
/// <param name="preview">A flag indicating whether to get the draft (preview) version or the published version.</param>
Comment thread
AndyButland marked this conversation as resolved.
// TODO (V18): Remove the default implementation on this method.
async Task<IEnumerable<ContentCacheNode>> GetContentSourcesAsync(IEnumerable<Guid> keys, bool preview = false)
{
Expand Down
Original file line number Diff line number Diff line change
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, ContentCacheNode? publishedNode) = await _databaseCacheRepository.GetContentSourceForPublishStatesAsync(key);

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);
if (publishedNode is not null && _publishStatusQueryService.HasPublishedAncestorPath(publishedNode.Key))
{
var cacheKey = GetCacheKey(publishedNode.Key, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,20 @@
_mockDatabaseCacheRepository.Setup(r => r.GetContentSourcesAsync(It.IsAny<IEnumerable<Guid>>(), false))
.ReturnsAsync([publishedTestCacheNode]);

_mockDatabaseCacheRepository.Setup(r => r.GetContentSourceForPublishStatesAsync(It.IsAny<Guid>()))
.ReturnsAsync((draftTestCacheNode, publishedTestCacheNode));

_mockDatabaseCacheRepository.Setup(r => r.GetContentByContentTypeKey(It.IsAny<IReadOnlyCollection<Guid>>(), ContentCacheDataSerializerEntityType.Document)).Returns(
new List<ContentCacheNode>()
{
draftTestCacheNode,
});

_mockDatabaseCacheRepository.Setup(r => r.DeleteContentItemAsync(It.IsAny<int>()));

var mockedPublishedStatusService = new Mock<IPublishStatusQueryService>();
mockedPublishedStatusService.Setup(x => x.IsDocumentPublishedInAnyCulture(It.IsAny<Guid>())).Returns(true);
mockedPublishedStatusService.Setup(x => x.HasPublishedAncestorPath(It.IsAny<Guid>())).Returns(true);

Check warning on line 103 in tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheMockTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ Getting worse: Large Method

SetUp increases from 72 to 75 lines of code, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.

_documentCacheService = new DocumentCacheService(
_mockDatabaseCacheRepository.Object,
Expand Down Expand Up @@ -235,6 +239,47 @@
_mockDatabaseCacheRepository.Verify(x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>()), Times.Exactly(1));
}

[Test]
public async Task RefreshMemoryCache_Fetches_Draft_And_Published()
{
// Arrange
var hybridCache = GetRequiredService<Microsoft.Extensions.Caching.Hybrid.HybridCache>();

// Clear both draft and published cache entries.
await hybridCache.RemoveAsync($"{Textpage.Key}+draft");
await hybridCache.RemoveAsync($"{Textpage.Key}");

// Act
await _documentCacheService.RefreshMemoryCacheAsync(Textpage.Key);

// Assert - verify only a single call was made to the combined method for retrieving both states.
_mockDatabaseCacheRepository.Verify(
x => x.GetContentSourceForPublishStatesAsync(Textpage.Key),
Times.Exactly(1));

// Verify individual GetContentSourceAsync was NOT called
_mockDatabaseCacheRepository.Verify(
x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>()),
Times.Never);

// Verify content is now cached - fetching should not hit the repository again.
var draftPage = await _mockedCache.GetByIdAsync(Textpage.Key, true);
var publishedPage = await _mockedCache.GetByIdAsync(Textpage.Key, false);

Assert.IsNotNull(draftPage);
Assert.IsNotNull(publishedPage);
Assert.AreEqual(Textpage.Name, draftPage.Name);
Assert.AreEqual(Textpage.Name, publishedPage.Name);

// Verify no additional repository calls were made (content served from cache).
_mockDatabaseCacheRepository.Verify(
x => x.GetContentSourceAsync(It.IsAny<Guid>(), It.IsAny<bool>()),
Times.Never);
_mockDatabaseCacheRepository.Verify(
x => x.GetContentSourceForPublishStatesAsync(It.IsAny<Guid>()),
Times.Exactly(1));
}

private void AssertTextPage(IPublishedContent textPage)
{
Assert.Multiple(() =>
Expand Down
Loading