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 @@ -997,43 +997,54 @@
/// </summary>
private Dictionary<int, List<PropertyTypeInfo>> GetPropertyInfoByContentType(IReadOnlyCollection<int> contentTypeIds)
{
// When contentTypeIds is empty (full rebuild), we need to fetch ALL content type IDs
// so that property variation info and compositions are correctly resolved.
IReadOnlyCollection<int> effectiveContentTypeIds = contentTypeIds;
if (contentTypeIds.Count == 0)
{
return new Dictionary<int, List<PropertyTypeInfo>>();
Sql<ISqlContext> allTypesSql = Sql()
.Select<ContentTypeDto>(x => x.NodeId)
.From<ContentTypeDto>();
effectiveContentTypeIds = Database.Fetch<int>(allTypesSql);

if (effectiveContentTypeIds.Count == 0)
{
return new Dictionary<int, List<PropertyTypeInfo>>();
}
}

// Get the composition hierarchy: which content types compose which other content types.
// The cmsContentType2ContentType table stores: ParentId = composed type, ChildId = composing type.
Dictionary<int, HashSet<int>> compositionsByChild = GetCompositionHierarchy(contentTypeIds);
Dictionary<int, HashSet<int>> compositionsByChild = GetCompositionHierarchy(effectiveContentTypeIds);

// Build set of all content type IDs we need properties for (including all compositions).
var allContentTypeIds = new HashSet<int>(contentTypeIds);
var allContentTypeIds = new HashSet<int>(effectiveContentTypeIds);
foreach (HashSet<int> compositions in compositionsByChild.Values)
{
allContentTypeIds.UnionWith(compositions);
}

// Get all property info (alias and variations) for all relevant content types.
Sql<ISqlContext> sql = Sql()
.Select<PropertyTypeDto>(x => x.Alias, x => x.Variations)
.AndSelect<ContentTypeDto>(x => x.NodeId)
.From<PropertyTypeDto>()
.InnerJoin<ContentTypeDto>().On<PropertyTypeDto, ContentTypeDto>((pt, ct) => pt.ContentTypeId == ct.NodeId)
.WhereIn<ContentTypeDto>(x => x.NodeId, allContentTypeIds);

List<PropertyTypeAliasDto> results = Database.Fetch<PropertyTypeAliasDto>(sql);

// Group properties by the content type that defines them.
var propertiesByDefiningType = results
.GroupBy(x => x.ContentTypeId)
.ToDictionary(
g => g.Key,
g => g.Select(x => new PropertyTypeInfo { Alias = x.Alias, Variations = x.Variations }).ToList());

// Build the final result: for each requested content type, include its own properties
// plus all properties from its compositions.
var result = new Dictionary<int, List<PropertyTypeInfo>>();
foreach (var contentTypeId in contentTypeIds)
foreach (var contentTypeId in effectiveContentTypeIds)

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

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ Getting worse: Complex Method

GetPropertyInfoByContentType increases in cyclomatic complexity from 10 to 11, 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.
{
var allProperties = new Dictionary<string, PropertyTypeInfo>(StringComparer.OrdinalIgnoreCase);

Expand Down Expand Up @@ -1099,8 +1110,10 @@
.ToDictionary(g => g.Key, g => g.Select(x => x.ParentId).ToHashSet());

// For each requested content type, recursively resolve all compositions.
// When contentTypeIds is empty (full rebuild), resolve for all child IDs in the composition table.
IEnumerable<int> idsToResolve = contentTypeIds.Count > 0 ? contentTypeIds : directParents.Keys;
var result = new Dictionary<int, HashSet<int>>();
foreach (var contentTypeId in contentTypeIds)
foreach (var contentTypeId in idsToResolve)
{
result[contentTypeId] = GetAllCompositionsRecursive(contentTypeId, directParents);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ public IEnumerable<IPublishedContent> GetByContentType(IPublishedContentType con

public async Task ClearMemoryCacheAsync(CancellationToken cancellationToken)
{
_publishedContentCache.Clear();
await _hybridCache.RemoveByTagAsync(Constants.Cache.Tags.Content, cancellationToken);

// We have to run seeding again after the cache is cleared
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ public async Task RefreshMemoryCacheAsync(Guid key)

public async Task ClearMemoryCacheAsync(CancellationToken cancellationToken)
{
_publishedContentCache.Clear();
await _hybridCache.RemoveByTagAsync(Constants.Cache.Tags.Media, cancellationToken);

// We have to run seeding again after the cache is cleared
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@

private IDocumentCacheService DocumentCacheService => GetRequiredService<IDocumentCacheService>();

private IDatabaseCacheRebuilder DatabaseCacheRebuilder => GetRequiredService<IDatabaseCacheRebuilder>();

private ILanguageService LanguageService => GetRequiredService<ILanguageService>();

[Test]
Expand Down Expand Up @@ -94,7 +96,7 @@

nodeIds = [.. publishedDtos.Select(d => d.NodeId)];
Assert.That(nodeIds, Does.Contain(Textpage.Id), "Textpage should have published cache entry");
Assert.That(nodeIds, Has.No.Member(Subpage.Id), "Subpage should have not have published cache entry");
Assert.That(nodeIds, Has.No.Member(Subpage.Id), "Subpage should not have published cache entry");

Check warning on line 99 in tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentCacheServiceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Code Duplication

The module contains 6 functions with similar structure: FullRebuild_Creates_Invariant_Document_Database_Cache_Records,FullRebuild_Includes_Composed_Properties,FullRebuild_Preserves_Variant_Culture_Specific_Property_Values,Rebuild_Creates_Invariant_Document_Database_Cache_Records_For_Document_Type and 2 more functions. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

Check warning on line 99 in tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentCacheServiceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Large Assertion Blocks

The test suite contains 6 assertion blocks with at least 4 assertions, threshold = 4. This test file has several blocks of large, consecutive assert statements. Avoid adding more.

// Verify cache data is not empty
var textpageDto = draftDtos.Single(d => d.NodeId == Textpage.Id);
Expand Down Expand Up @@ -354,6 +356,223 @@
}
}

[Test]
public async Task FullRebuild_Creates_Invariant_Document_Database_Cache_Records()
{
// Arrange - Content is created in base class Setup()
// The base class creates: Textpage, Subpage, Subpage2, Subpage3 (all using ContentType)

// - publish the root page to ensure we have published and draft content
ContentService.Publish(Textpage, ["*"]);

// Act - Full rebuild (the "Rebuild Database Cache" dashboard button path)
// This calls Rebuild([], [], []) internally — empty arrays meaning "rebuild all"
await DatabaseCacheRebuilder.RebuildAsync(false);

// Assert - Verify cmsContentNu table has records for the content items
using (var scope = ScopeProvider.CreateScope(autoComplete: true))
{
var selectSql = SqlContext.Sql()
.Select<ContentNuDto>()
.From<ContentNuDto>();

var dtos = ScopeAccessor.AmbientScope!.Database.Fetch<ContentNuDto>(selectSql);

var draftDtos = dtos.Where(d => !d.Published).ToList();
var publishedDtos = dtos.Where(d => d.Published).ToList();

// Verify we have draft records for non-trashed content
Assert.That(draftDtos, Has.Count.GreaterThanOrEqualTo(4), "Expected at least 4 draft cache records");

// Verify we have published records for published content
Assert.AreEqual(1, publishedDtos.Count, "Expected 1 published cache record");

// Verify specific content items have cache entries
var nodeIds = draftDtos.Select(d => d.NodeId).ToList();
Assert.That(nodeIds, Does.Contain(Textpage.Id), "Textpage should have draft cache entry");
Assert.That(nodeIds, Does.Contain(Subpage.Id), "Subpage should have draft cache entry");
Assert.That(nodeIds, Does.Contain(Subpage2.Id), "Subpage2 should have draft cache entry");
Assert.That(nodeIds, Does.Contain(Subpage3.Id), "Subpage3 should have draft cache entry");

nodeIds = [.. publishedDtos.Select(d => d.NodeId)];
Assert.That(nodeIds, Does.Contain(Textpage.Id), "Textpage should have published cache entry");
Assert.That(nodeIds, Has.No.Member(Subpage.Id), "Subpage should not have published cache entry");

// Verify cache data is not empty
var textpageDto = draftDtos.Single(d => d.NodeId == Textpage.Id);
Assert.That(textpageDto.Data, Is.Not.Null.And.Not.Empty, "Cache data should not be empty");

// Verify the cached data is correct (same expectation as the per-type rebuild test)
const string ExpectedJson = "{\"pd\":{\"title\":[{\"c\":\"\",\"s\":\"\",\"v\":\"Welcome to our Home page\"}],\"bodyText\":[{\"c\":\"\",\"s\":\"\",\"v\":\"This is the welcome message on the first page\"}],\"author\":[{\"c\":\"\",\"s\":\"\",\"v\":\"John Doe\"}]},\"cd\":{},\"us\":\"textpage\"}";
Assert.That(textpageDto.Data, Is.EqualTo(ExpectedJson), "Cache data does not match expected JSON");
}
}

[Test]
public async Task FullRebuild_Preserves_Variant_Culture_Specific_Property_Values()
{
// Arrange - Create languages
var langEn = new LanguageBuilder()
.WithCultureInfo("en-US")
.WithIsDefault(true)
.Build();
await LanguageService.CreateAsync(langEn, Constants.Security.SuperUserKey);

var langDa = new LanguageBuilder()
.WithCultureInfo("da-DK")
.Build();
await LanguageService.CreateAsync(langDa, Constants.Security.SuperUserKey);

// Create a variant content type with a variant property
var variantContentType = new ContentTypeBuilder()
.WithAlias("variantPage")
.WithName("Variant Page")
.WithContentVariation(ContentVariation.Culture)
.AddPropertyGroup()
.WithName("Content")
.WithAlias("content")
.WithSortOrder(1)
.AddPropertyType()
.WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.TextBox)
.WithValueStorageType(ValueStorageType.Nvarchar)
.WithAlias("pageTitle")
.WithName("Page Title")
.WithVariations(ContentVariation.Culture)
.WithSortOrder(1)
.Done()
.Done()
.Build();
variantContentType.AllowedAsRoot = true;
await ContentTypeService.CreateAsync(variantContentType, Constants.Security.SuperUserKey);

// Create content with culture-specific values
var variantContent = new ContentBuilder()
.WithContentType(variantContentType)
.WithCultureName(langEn.IsoCode, "English Page")
.WithCultureName(langDa.IsoCode, "Danish Page")
.Build();
variantContent.SetValue("pageTitle", "English Title", culture: langEn.IsoCode);
variantContent.SetValue("pageTitle", "Danish Title", culture: langDa.IsoCode);
ContentService.Save(variantContent);

// Act - Full rebuild (the "Rebuild Database Cache" dashboard button path)
await DatabaseCacheRebuilder.RebuildAsync(false);

// Assert - Verify cmsContentNu table has variant content with culture-specific values
using (var scope = ScopeProvider.CreateScope(autoComplete: true))
{
var selectSql = SqlContext.Sql()
.Select<ContentNuDto>()
.From<ContentNuDto>()
.Where<ContentNuDto>(x => x.NodeId == variantContent.Id && !x.Published);

var dto = ScopeAccessor.AmbientScope!.Database.Fetch<ContentNuDto>(selectSql).FirstOrDefault();
Assert.That(dto, Is.Not.Null, "Variant content should have a cache entry");
Assert.That(dto!.Data, Is.Not.Null.And.Not.Empty, "Cache data should not be empty");

// Verify the cached data includes the variant property with culture-specific values
Assert.That(dto.Data, Does.Contain("\"pageTitle\":["), "Cache should include pageTitle property");
Assert.That(dto.Data, Does.Contain("\"c\":\"en-US\""), "Cache should include English culture");
Assert.That(dto.Data, Does.Contain("\"c\":\"da-DK\""), "Cache should include Danish culture");
Assert.That(dto.Data, Does.Contain("\"v\":\"English Title\""), "Cache should include English title value");
Assert.That(dto.Data, Does.Contain("\"v\":\"Danish Title\""), "Cache should include Danish title value");

// Verify the culture data section includes both cultures
Assert.That(dto.Data, Does.Contain("\"cd\":{"), "Cache should include culture data section");
Assert.That(dto.Data, Does.Contain("\"en-US\":{"), "Cache should include en-US culture data");
Assert.That(dto.Data, Does.Contain("\"da-DK\":{"), "Cache should include da-DK culture data");

// Verify the cached data is correct (same expectation as the per-type rebuild test)
const string ExpectedJson = "{\"pd\":{\"pageTitle\":[{\"c\":\"da-DK\",\"s\":\"\",\"v\":\"Danish Title\"},{\"c\":\"en-US\",\"s\":\"\",\"v\":\"English Title\"}]},\"cd\":{\"en-US\":{\"nm\":\"English Page\",\"us\":\"english-page\",\"dt\":\"\",\"isd\":true},\"da-DK\":{\"nm\":\"Danish Page\",\"us\":\"danish-page\",\"dt\":\"\",\"isd\":true}},\"us\":\"english-page\"}";
var actualJsonNormalized = RemoveDates(dto.Data!);

Assert.That(actualJsonNormalized, Is.EqualTo(ExpectedJson), "Cache data does not match expected JSON");
}
}

[Test]
public async Task FullRebuild_Includes_Composed_Properties()
{
// Arrange - Create a composition content type with a custom property
var compositionType = new ContentTypeBuilder()
.WithAlias("documentComposition")
.WithName("Document Composition")
.AddPropertyGroup()
.WithName("SEO")
.WithAlias("seo")
.WithSortOrder(1)
.AddPropertyType()
.WithPropertyEditorAlias(Cms.Core.Constants.PropertyEditors.Aliases.TextBox)
.WithValueStorageType(ValueStorageType.Nvarchar)
.WithAlias("metaDescription")
.WithName("Meta Description")
.WithSortOrder(1)
.Done()
.Done()
.Build();
await ContentTypeService.CreateAsync(compositionType, Constants.Security.SuperUserKey);

// Create a content type that uses the composition
var composedContentType = new ContentTypeBuilder()
.WithAlias("composedPage")
.WithName("Composed Page")
.AddPropertyGroup()
.WithName("Content")
.WithAlias("content")
.WithSortOrder(1)
.AddPropertyType()
.WithPropertyEditorAlias(Cms.Core.Constants.PropertyEditors.Aliases.TextBox)
.WithValueStorageType(ValueStorageType.Nvarchar)
.WithAlias("pageTitle")
.WithName("Page Title")
.WithSortOrder(1)
.Done()
.Done()
.Build();

// Add the composition to the content type
composedContentType.AddContentType(compositionType);
await ContentTypeService.CreateAsync(composedContentType, Constants.Security.SuperUserKey);

// Create content using the composed type
var composedContent = new ContentBuilder()
.WithName("Composed Content Item")
.WithContentType(composedContentType)
.WithPropertyValues(new
{
pageTitle = "Composed Page Title",
metaDescription = "This is a meta description from the composition.",
})
.Build();
ContentService.Save(composedContent);

// Act - Full rebuild (the "Rebuild Database Cache" dashboard button path)
await DatabaseCacheRebuilder.RebuildAsync(false);

// Assert - Verify the cache includes properties from both the content type AND its composition
using (var scope = ScopeProvider.CreateScope(autoComplete: true))
{
var selectSql = SqlContext.Sql()
.Select<ContentNuDto>()
.From<ContentNuDto>()
.Where<ContentNuDto>(x => x.NodeId == composedContent.Id && !x.Published);

var dto = ScopeAccessor.AmbientScope!.Database.Fetch<ContentNuDto>(selectSql).FirstOrDefault();
Assert.That(dto, Is.Not.Null, "Composed content should have a cache entry");
Assert.That(dto!.Data, Is.Not.Null.And.Not.Empty, "Cache data should not be empty");

// Verify the cached data includes properties from the composition (metaDescription)
Assert.That(dto.Data, Does.Contain("\"metaDescription\":["), "Cache should include metaDescription from composition");

// Verify the cached data includes direct properties (pageTitle)
Assert.That(dto.Data, Does.Contain("\"pageTitle\":["), "Cache should include pageTitle from direct type");

// Verify the cached data is correct (same expectation as the per-type rebuild test)
const string ExpectedJson = "{\"pd\":{\"pageTitle\":[{\"c\":\"\",\"s\":\"\",\"v\":\"Composed Page Title\"}],\"metaDescription\":[{\"c\":\"\",\"s\":\"\",\"v\":\"This is a meta description from the composition.\"}]},\"cd\":{},\"us\":\"composed-content-item\"}";
Assert.That(dto.Data, Is.EqualTo(ExpectedJson), "Cache data does not match expected JSON");
}
}

[Test]
public void Rebuild_Does_Not_Create_Untrusted_Foreign_Key_Constraints()
{
Expand Down
Loading