Skip to content

Commit 09206a6

Browse files
authored
Database Cache: Fix full database cache rebuild dropping variant and composed property values (closes #21863, #21882) (#21890)
* Resolve full database cache rebuild dropping variant and composed property values * Addressed feedback from code review.
1 parent d3b3661 commit 09206a6

4 files changed

Lines changed: 240 additions & 6 deletions

File tree

src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -997,17 +997,28 @@ private void RebuildMediaDbCache(IContentCacheDataSerializer serializer, int gro
997997
/// </summary>
998998
private Dictionary<int, List<PropertyTypeInfo>> GetPropertyInfoByContentType(IReadOnlyCollection<int> contentTypeIds)
999999
{
1000+
// When contentTypeIds is empty (full rebuild), we need to fetch ALL content type IDs
1001+
// so that property variation info and compositions are correctly resolved.
1002+
IReadOnlyCollection<int> effectiveContentTypeIds = contentTypeIds;
10001003
if (contentTypeIds.Count == 0)
10011004
{
1002-
return new Dictionary<int, List<PropertyTypeInfo>>();
1005+
Sql<ISqlContext> allTypesSql = Sql()
1006+
.Select<ContentTypeDto>(x => x.NodeId)
1007+
.From<ContentTypeDto>();
1008+
effectiveContentTypeIds = Database.Fetch<int>(allTypesSql);
1009+
1010+
if (effectiveContentTypeIds.Count == 0)
1011+
{
1012+
return new Dictionary<int, List<PropertyTypeInfo>>();
1013+
}
10031014
}
10041015

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

10091020
// Build set of all content type IDs we need properties for (including all compositions).
1010-
var allContentTypeIds = new HashSet<int>(contentTypeIds);
1021+
var allContentTypeIds = new HashSet<int>(effectiveContentTypeIds);
10111022
foreach (HashSet<int> compositions in compositionsByChild.Values)
10121023
{
10131024
allContentTypeIds.UnionWith(compositions);
@@ -1033,7 +1044,7 @@ private Dictionary<int, List<PropertyTypeInfo>> GetPropertyInfoByContentType(IRe
10331044
// Build the final result: for each requested content type, include its own properties
10341045
// plus all properties from its compositions.
10351046
var result = new Dictionary<int, List<PropertyTypeInfo>>();
1036-
foreach (var contentTypeId in contentTypeIds)
1047+
foreach (var contentTypeId in effectiveContentTypeIds)
10371048
{
10381049
var allProperties = new Dictionary<string, PropertyTypeInfo>(StringComparer.OrdinalIgnoreCase);
10391050

@@ -1099,8 +1110,10 @@ private Dictionary<int, HashSet<int>> GetCompositionHierarchy(IReadOnlyCollectio
10991110
.ToDictionary(g => g.Key, g => g.Select(x => x.ParentId).ToHashSet());
11001111

11011112
// For each requested content type, recursively resolve all compositions.
1113+
// When contentTypeIds is empty (full rebuild), resolve for all child IDs in the composition table.
1114+
IEnumerable<int> idsToResolve = contentTypeIds.Count > 0 ? contentTypeIds : directParents.Keys;
11021115
var result = new Dictionary<int, HashSet<int>>();
1103-
foreach (var contentTypeId in contentTypeIds)
1116+
foreach (var contentTypeId in idsToResolve)
11041117
{
11051118
result[contentTypeId] = GetAllCompositionsRecursive(contentTypeId, directParents);
11061119
}

src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ public IEnumerable<IPublishedContent> GetByContentType(IPublishedContentType con
169169

170170
public async Task ClearMemoryCacheAsync(CancellationToken cancellationToken)
171171
{
172+
_publishedContentCache.Clear();
172173
await _hybridCache.RemoveByTagAsync(Constants.Cache.Tags.Content, cancellationToken);
173174

174175
// We have to run seeding again after the cache is cleared

src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ public async Task RefreshMemoryCacheAsync(Guid key)
245245

246246
public async Task ClearMemoryCacheAsync(CancellationToken cancellationToken)
247247
{
248+
_publishedContentCache.Clear();
248249
await _hybridCache.RemoveByTagAsync(Constants.Cache.Tags.Media, cancellationToken);
249250

250251
// We have to run seeding again after the cache is cleared

tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentCacheServiceTests.cs

Lines changed: 220 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ private static void RegisterServices(IUmbracoBuilder builder)
5151

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

54+
private IDatabaseCacheRebuilder DatabaseCacheRebuilder => GetRequiredService<IDatabaseCacheRebuilder>();
55+
5456
private ILanguageService LanguageService => GetRequiredService<ILanguageService>();
5557

5658
[Test]
@@ -94,7 +96,7 @@ public void Rebuild_Creates_Invariant_Document_Database_Cache_Records_For_Docume
9496

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

99101
// Verify cache data is not empty
100102
var textpageDto = draftDtos.Single(d => d.NodeId == Textpage.Id);
@@ -354,6 +356,223 @@ public async Task Rebuild_Includes_Composed_Properties_In_Cache()
354356
}
355357
}
356358

359+
[Test]
360+
public async Task FullRebuild_Creates_Invariant_Document_Database_Cache_Records()
361+
{
362+
// Arrange - Content is created in base class Setup()
363+
// The base class creates: Textpage, Subpage, Subpage2, Subpage3 (all using ContentType)
364+
365+
// - publish the root page to ensure we have published and draft content
366+
ContentService.Publish(Textpage, ["*"]);
367+
368+
// Act - Full rebuild (the "Rebuild Database Cache" dashboard button path)
369+
// This calls Rebuild([], [], []) internally — empty arrays meaning "rebuild all"
370+
await DatabaseCacheRebuilder.RebuildAsync(false);
371+
372+
// Assert - Verify cmsContentNu table has records for the content items
373+
using (var scope = ScopeProvider.CreateScope(autoComplete: true))
374+
{
375+
var selectSql = SqlContext.Sql()
376+
.Select<ContentNuDto>()
377+
.From<ContentNuDto>();
378+
379+
var dtos = ScopeAccessor.AmbientScope!.Database.Fetch<ContentNuDto>(selectSql);
380+
381+
var draftDtos = dtos.Where(d => !d.Published).ToList();
382+
var publishedDtos = dtos.Where(d => d.Published).ToList();
383+
384+
// Verify we have draft records for non-trashed content
385+
Assert.That(draftDtos, Has.Count.GreaterThanOrEqualTo(4), "Expected at least 4 draft cache records");
386+
387+
// Verify we have published records for published content
388+
Assert.AreEqual(1, publishedDtos.Count, "Expected 1 published cache record");
389+
390+
// Verify specific content items have cache entries
391+
var nodeIds = draftDtos.Select(d => d.NodeId).ToList();
392+
Assert.That(nodeIds, Does.Contain(Textpage.Id), "Textpage should have draft cache entry");
393+
Assert.That(nodeIds, Does.Contain(Subpage.Id), "Subpage should have draft cache entry");
394+
Assert.That(nodeIds, Does.Contain(Subpage2.Id), "Subpage2 should have draft cache entry");
395+
Assert.That(nodeIds, Does.Contain(Subpage3.Id), "Subpage3 should have draft cache entry");
396+
397+
nodeIds = [.. publishedDtos.Select(d => d.NodeId)];
398+
Assert.That(nodeIds, Does.Contain(Textpage.Id), "Textpage should have published cache entry");
399+
Assert.That(nodeIds, Has.No.Member(Subpage.Id), "Subpage should not have published cache entry");
400+
401+
// Verify cache data is not empty
402+
var textpageDto = draftDtos.Single(d => d.NodeId == Textpage.Id);
403+
Assert.That(textpageDto.Data, Is.Not.Null.And.Not.Empty, "Cache data should not be empty");
404+
405+
// Verify the cached data is correct (same expectation as the per-type rebuild test)
406+
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\"}";
407+
Assert.That(textpageDto.Data, Is.EqualTo(ExpectedJson), "Cache data does not match expected JSON");
408+
}
409+
}
410+
411+
[Test]
412+
public async Task FullRebuild_Preserves_Variant_Culture_Specific_Property_Values()
413+
{
414+
// Arrange - Create languages
415+
var langEn = new LanguageBuilder()
416+
.WithCultureInfo("en-US")
417+
.WithIsDefault(true)
418+
.Build();
419+
await LanguageService.CreateAsync(langEn, Constants.Security.SuperUserKey);
420+
421+
var langDa = new LanguageBuilder()
422+
.WithCultureInfo("da-DK")
423+
.Build();
424+
await LanguageService.CreateAsync(langDa, Constants.Security.SuperUserKey);
425+
426+
// Create a variant content type with a variant property
427+
var variantContentType = new ContentTypeBuilder()
428+
.WithAlias("variantPage")
429+
.WithName("Variant Page")
430+
.WithContentVariation(ContentVariation.Culture)
431+
.AddPropertyGroup()
432+
.WithName("Content")
433+
.WithAlias("content")
434+
.WithSortOrder(1)
435+
.AddPropertyType()
436+
.WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.TextBox)
437+
.WithValueStorageType(ValueStorageType.Nvarchar)
438+
.WithAlias("pageTitle")
439+
.WithName("Page Title")
440+
.WithVariations(ContentVariation.Culture)
441+
.WithSortOrder(1)
442+
.Done()
443+
.Done()
444+
.Build();
445+
variantContentType.AllowedAsRoot = true;
446+
await ContentTypeService.CreateAsync(variantContentType, Constants.Security.SuperUserKey);
447+
448+
// Create content with culture-specific values
449+
var variantContent = new ContentBuilder()
450+
.WithContentType(variantContentType)
451+
.WithCultureName(langEn.IsoCode, "English Page")
452+
.WithCultureName(langDa.IsoCode, "Danish Page")
453+
.Build();
454+
variantContent.SetValue("pageTitle", "English Title", culture: langEn.IsoCode);
455+
variantContent.SetValue("pageTitle", "Danish Title", culture: langDa.IsoCode);
456+
ContentService.Save(variantContent);
457+
458+
// Act - Full rebuild (the "Rebuild Database Cache" dashboard button path)
459+
await DatabaseCacheRebuilder.RebuildAsync(false);
460+
461+
// Assert - Verify cmsContentNu table has variant content with culture-specific values
462+
using (var scope = ScopeProvider.CreateScope(autoComplete: true))
463+
{
464+
var selectSql = SqlContext.Sql()
465+
.Select<ContentNuDto>()
466+
.From<ContentNuDto>()
467+
.Where<ContentNuDto>(x => x.NodeId == variantContent.Id && !x.Published);
468+
469+
var dto = ScopeAccessor.AmbientScope!.Database.Fetch<ContentNuDto>(selectSql).FirstOrDefault();
470+
Assert.That(dto, Is.Not.Null, "Variant content should have a cache entry");
471+
Assert.That(dto!.Data, Is.Not.Null.And.Not.Empty, "Cache data should not be empty");
472+
473+
// Verify the cached data includes the variant property with culture-specific values
474+
Assert.That(dto.Data, Does.Contain("\"pageTitle\":["), "Cache should include pageTitle property");
475+
Assert.That(dto.Data, Does.Contain("\"c\":\"en-US\""), "Cache should include English culture");
476+
Assert.That(dto.Data, Does.Contain("\"c\":\"da-DK\""), "Cache should include Danish culture");
477+
Assert.That(dto.Data, Does.Contain("\"v\":\"English Title\""), "Cache should include English title value");
478+
Assert.That(dto.Data, Does.Contain("\"v\":\"Danish Title\""), "Cache should include Danish title value");
479+
480+
// Verify the culture data section includes both cultures
481+
Assert.That(dto.Data, Does.Contain("\"cd\":{"), "Cache should include culture data section");
482+
Assert.That(dto.Data, Does.Contain("\"en-US\":{"), "Cache should include en-US culture data");
483+
Assert.That(dto.Data, Does.Contain("\"da-DK\":{"), "Cache should include da-DK culture data");
484+
485+
// Verify the cached data is correct (same expectation as the per-type rebuild test)
486+
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\"}";
487+
var actualJsonNormalized = RemoveDates(dto.Data!);
488+
489+
Assert.That(actualJsonNormalized, Is.EqualTo(ExpectedJson), "Cache data does not match expected JSON");
490+
}
491+
}
492+
493+
[Test]
494+
public async Task FullRebuild_Includes_Composed_Properties()
495+
{
496+
// Arrange - Create a composition content type with a custom property
497+
var compositionType = new ContentTypeBuilder()
498+
.WithAlias("documentComposition")
499+
.WithName("Document Composition")
500+
.AddPropertyGroup()
501+
.WithName("SEO")
502+
.WithAlias("seo")
503+
.WithSortOrder(1)
504+
.AddPropertyType()
505+
.WithPropertyEditorAlias(Cms.Core.Constants.PropertyEditors.Aliases.TextBox)
506+
.WithValueStorageType(ValueStorageType.Nvarchar)
507+
.WithAlias("metaDescription")
508+
.WithName("Meta Description")
509+
.WithSortOrder(1)
510+
.Done()
511+
.Done()
512+
.Build();
513+
await ContentTypeService.CreateAsync(compositionType, Constants.Security.SuperUserKey);
514+
515+
// Create a content type that uses the composition
516+
var composedContentType = new ContentTypeBuilder()
517+
.WithAlias("composedPage")
518+
.WithName("Composed Page")
519+
.AddPropertyGroup()
520+
.WithName("Content")
521+
.WithAlias("content")
522+
.WithSortOrder(1)
523+
.AddPropertyType()
524+
.WithPropertyEditorAlias(Cms.Core.Constants.PropertyEditors.Aliases.TextBox)
525+
.WithValueStorageType(ValueStorageType.Nvarchar)
526+
.WithAlias("pageTitle")
527+
.WithName("Page Title")
528+
.WithSortOrder(1)
529+
.Done()
530+
.Done()
531+
.Build();
532+
533+
// Add the composition to the content type
534+
composedContentType.AddContentType(compositionType);
535+
await ContentTypeService.CreateAsync(composedContentType, Constants.Security.SuperUserKey);
536+
537+
// Create content using the composed type
538+
var composedContent = new ContentBuilder()
539+
.WithName("Composed Content Item")
540+
.WithContentType(composedContentType)
541+
.WithPropertyValues(new
542+
{
543+
pageTitle = "Composed Page Title",
544+
metaDescription = "This is a meta description from the composition.",
545+
})
546+
.Build();
547+
ContentService.Save(composedContent);
548+
549+
// Act - Full rebuild (the "Rebuild Database Cache" dashboard button path)
550+
await DatabaseCacheRebuilder.RebuildAsync(false);
551+
552+
// Assert - Verify the cache includes properties from both the content type AND its composition
553+
using (var scope = ScopeProvider.CreateScope(autoComplete: true))
554+
{
555+
var selectSql = SqlContext.Sql()
556+
.Select<ContentNuDto>()
557+
.From<ContentNuDto>()
558+
.Where<ContentNuDto>(x => x.NodeId == composedContent.Id && !x.Published);
559+
560+
var dto = ScopeAccessor.AmbientScope!.Database.Fetch<ContentNuDto>(selectSql).FirstOrDefault();
561+
Assert.That(dto, Is.Not.Null, "Composed content should have a cache entry");
562+
Assert.That(dto!.Data, Is.Not.Null.And.Not.Empty, "Cache data should not be empty");
563+
564+
// Verify the cached data includes properties from the composition (metaDescription)
565+
Assert.That(dto.Data, Does.Contain("\"metaDescription\":["), "Cache should include metaDescription from composition");
566+
567+
// Verify the cached data includes direct properties (pageTitle)
568+
Assert.That(dto.Data, Does.Contain("\"pageTitle\":["), "Cache should include pageTitle from direct type");
569+
570+
// Verify the cached data is correct (same expectation as the per-type rebuild test)
571+
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\"}";
572+
Assert.That(dto.Data, Is.EqualTo(ExpectedJson), "Cache data does not match expected JSON");
573+
}
574+
}
575+
357576
[Test]
358577
public void Rebuild_Does_Not_Create_Untrusted_Foreign_Key_Constraints()
359578
{

0 commit comments

Comments
 (0)