diff --git a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentTypeCacheRefresher.cs b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentTypeCacheRefresher.cs index 08f90cf18fdd..a86029c7157f 100644 --- a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentTypeCacheRefresher.cs @@ -175,41 +175,42 @@ public override void Refresh(JsonPayload[] payloads) _publishedContentTypeFactory.NotifyDataTypeChanges(); _publishedModelFactory.WithSafeLiveFactoryReset(() => { - // Separate structural changes (RefreshMain) from non-structural changes (RefreshOther). - // Structural changes require a full memory cache rebuild, while non-structural changes - // only need the converted content cache cleared since ContentCacheNode only stores ContentTypeId. - var structuralDocumentTypeIds = payloads - .Where(x => x.ItemType == nameof(IContentType) && x.ChangeTypes.IsStructuralChange()) + // Split changes into those that need the in-memory cache rebuilding (evicting the HybridCache + // entries by content type tag) and those that only need the converted content cache cleared. + // A structural change flagged RawDataUnaffected (a property removal) keeps its stored blob valid, + // so it belongs with the non-structural changes here — clearing the converted cache is enough. + var rebuildDocumentTypeIds = payloads + .Where(x => x.ItemType == nameof(IContentType) && x.ChangeTypes.RequiresRawDataRebuild()) .Select(x => x.Id) .ToArray(); - var nonStructuralDocumentTypeIds = payloads - .Where(x => x.ItemType == nameof(IContentType) && x.ChangeTypes.IsNonStructuralChange()) + var convertedOnlyDocumentTypeIds = payloads + .Where(x => x.ItemType == nameof(IContentType) && x.ChangeTypes.RequiresConvertedCacheClearOnly()) .Select(x => x.Id) .ToArray(); - var structuralMediaTypeIds = payloads - .Where(x => x.ItemType == nameof(IMediaType) && x.ChangeTypes.IsStructuralChange()) + var rebuildMediaTypeIds = payloads + .Where(x => x.ItemType == nameof(IMediaType) && x.ChangeTypes.RequiresRawDataRebuild()) .Select(x => x.Id) .ToArray(); - var nonStructuralMediaTypeIds = payloads - .Where(x => x.ItemType == nameof(IMediaType) && x.ChangeTypes.IsNonStructuralChange()) + var convertedOnlyMediaTypeIds = payloads + .Where(x => x.ItemType == nameof(IMediaType) && x.ChangeTypes.RequiresConvertedCacheClearOnly()) .Select(x => x.Id) .ToArray(); - // Full memory cache rebuild only for structural changes - if (structuralDocumentTypeIds.Length > 0) + // Full memory cache rebuild only for changes that affect the stored data + if (rebuildDocumentTypeIds.Length > 0) { - _documentCacheService.RebuildMemoryCacheByContentTypeAsync(structuralDocumentTypeIds).GetAwaiter().GetResult(); + _documentCacheService.RebuildMemoryCacheByContentTypeAsync(rebuildDocumentTypeIds).GetAwaiter().GetResult(); } - if (structuralMediaTypeIds.Length > 0) + if (rebuildMediaTypeIds.Length > 0) { - _mediaCacheService.RebuildMemoryCacheByContentTypeAsync(structuralMediaTypeIds).GetAwaiter().GetResult(); + _mediaCacheService.RebuildMemoryCacheByContentTypeAsync(rebuildMediaTypeIds).GetAwaiter().GetResult(); } - // Clear the converted content cache for non-structural changes (HybridCache entries remain valid). + // Clear the converted content cache for the remaining changes (HybridCache entries remain valid). // In auto models builder mode (InMemoryAuto), the factory reset above invalidates ALL compiled // model types, so we must clear all entries to prevent stale instances of other types // (e.g. Model.Parent()) from being returned. In non-auto modes, only affected types need clearing. @@ -217,26 +218,26 @@ public override void Refresh(JsonPayload[] payloads) if (isAutoFactory) { - if (structuralDocumentTypeIds.Length > 0 || nonStructuralDocumentTypeIds.Length > 0) + if (rebuildDocumentTypeIds.Length > 0 || convertedOnlyDocumentTypeIds.Length > 0) { _documentCacheService.ClearConvertedContentCache(); } - if (structuralMediaTypeIds.Length > 0 || nonStructuralMediaTypeIds.Length > 0) + if (rebuildMediaTypeIds.Length > 0 || convertedOnlyMediaTypeIds.Length > 0) { _mediaCacheService.ClearConvertedContentCache(); } } else { - if (nonStructuralDocumentTypeIds.Length > 0) + if (convertedOnlyDocumentTypeIds.Length > 0) { - _documentCacheService.ClearConvertedContentCache(nonStructuralDocumentTypeIds); + _documentCacheService.ClearConvertedContentCache(convertedOnlyDocumentTypeIds); } - if (nonStructuralMediaTypeIds.Length > 0) + if (convertedOnlyMediaTypeIds.Length > 0) { - _mediaCacheService.ClearConvertedContentCache(nonStructuralMediaTypeIds); + _mediaCacheService.ClearConvertedContentCache(convertedOnlyMediaTypeIds); } } }); diff --git a/src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs b/src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs index d36d73378356..672717d41b51 100644 --- a/src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs +++ b/src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs @@ -61,4 +61,27 @@ public static bool IsStructuralChange(this ContentTypeChangeTypes change) => /// true if the change has non-structural impact; otherwise, false. public static bool IsNonStructuralChange(this ContentTypeChangeTypes change) => change.HasType(ContentTypeChangeTypes.RefreshOther) && !change.HasType(ContentTypeChangeTypes.RefreshMain); + + /// + /// Determines whether the change requires the raw database cache (cmsContentNu) to be rebuilt. + /// + /// The change to check. + /// + /// true for a structural change unless it is flagged + /// (e.g. a property removal), in which case the stored blob stays valid and only the converted cache needs clearing. + /// + public static bool RequiresRawDataRebuild(this ContentTypeChangeTypes change) => + change.IsStructuralChange() && !change.HasType(ContentTypeChangeTypes.RawDataUnaffected); + + /// + /// Determines whether the change only requires the converted (in-memory) content cache to be cleared, + /// leaving the stored database cache (cmsContentNu) and HybridCache entries valid. + /// + /// The change to check. + /// + /// true for a non-structural change, or a structural change flagged + /// (e.g. a property removal). + /// + public static bool RequiresConvertedCacheClearOnly(this ContentTypeChangeTypes change) => + change.IsNonStructuralChange() || (change.IsStructuralChange() && change.HasType(ContentTypeChangeTypes.RawDataUnaffected)); } diff --git a/src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs b/src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs index 7f804b4ab0ec..7fd3cd0136a1 100644 --- a/src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs +++ b/src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs @@ -51,4 +51,18 @@ public enum ContentTypeChangeTypes : byte /// This impacts how URL segments and aliases are stored (NULL languageId for invariant, specific ID for variant). /// VariationChanged = 16, + + /// + /// Supplements to indicate that, although the change is structural, the raw + /// content data stored in the database cache (cmsContentNu) does not need rebuilding. + /// + /// + /// Set for a change whose only structural impact is the removal of a property type: the stored blob keeps + /// the removed property's data, but it is never read because published content is always resolved against + /// the current content type (a removed alias simply no longer maps to a property type). The converted + /// in-memory cache is still cleared, and all other handling (e.g. search + /// re-indexing, published content type cache clearing) still runs — only the expensive + /// cmsContentNu rebuild is skipped. + /// + RawDataUnaffected = 32, } diff --git a/src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs b/src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs index 8942e0708a7a..1151e9a790ec 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs @@ -357,6 +357,14 @@ internal IEnumerable> ComposeContentTypeChanges(params var changes = new List>(); + // Track which types genuinely need a raw cmsContentNu rebuild vs. which only had a property removed. + // A type can appear via more than one path (e.g. a batch save touching a composition, where the same + // type is both saved directly and returned by GetComposedOf as a different instance), so we key these + // by Id — not entity reference — and resolve the RawDataUnaffected flag once at the end: it is only + // safe when *nothing* required a rebuild for that Id. + var rebuildRequiredIds = new HashSet(); + var rawDataUnaffectedCandidateIds = new HashSet(); + foreach (TItem contentType in contentTypes) { var dirty = (IRememberBeingDirty)contentType; @@ -404,10 +412,30 @@ internal IEnumerable> ComposeContentTypeChanges(params var hasPropertyMainImpact = hasContentTypeVariationChanged || hasAnyPropertyVariationChanged || hasAnyCompositionBeenRemoved || hasAnyPropertyBeenRemoved || hasAnyPropertyChangedAlias; + // A composition change dirties the composition collection (add or remove). Adding a composition can + // reintroduce a just-removed alias behind a different property type, and the cmsContentNu blob is + // keyed by alias — so the stale value would resolve to the new property. Treat any composition + // change as requiring a rebuild. + var hasCompositionChanged = dirty.WasPropertyDirty("ContentTypeComposition"); + if (hasAliasChanged || hasPropertyMainImpact) { + // A property removal is the only structural change that does not require a raw cmsContentNu + // rebuild: the removed alias simply stops resolving against the content type, so the stored + // blob's orphaned value is never read. This holds only when nothing in the same change + // reintroduces that alias (e.g. an added composition bringing it back), so any composition + // change disqualifies it. Any other structural cause also needs a rebuild. + var rawDataUnaffected = hasAnyPropertyBeenRemoved && + hasAliasChanged is false && + hasAnyPropertyChangedAlias is false && + hasContentTypeVariationChanged is false && + hasAnyPropertyVariationChanged is false && + hasAnyCompositionBeenRemoved is false && + hasCompositionChanged is false; + // add that one, as a main change AddChange(changes, contentType, ContentTypeChangeTypes.RefreshMain); + (rawDataUnaffected ? rawDataUnaffectedCandidateIds : rebuildRequiredIds).Add(contentType.Id); // Add VariationChanged flag if content type variation changed. // This is used by DocumentUrlService to rebuild URL cache with correct languageId. @@ -420,7 +448,9 @@ internal IEnumerable> ComposeContentTypeChanges(params { foreach (TItem c in GetComposedOf(contentType.Id)) { + // Composing types inherit the same property change, so they share its rebuild requirement. AddChange(changes, c, ContentTypeChangeTypes.RefreshMain); + (rawDataUnaffected ? rawDataUnaffectedCandidateIds : rebuildRequiredIds).Add(c.Id); } } } @@ -431,6 +461,20 @@ internal IEnumerable> ComposeContentTypeChanges(params } } + // Flag the raw data as unaffected only for types that were never independently marked as needing a + // rebuild. RawDataUnaffected supplements RefreshMain, so only apply it to entries that already carry + // RefreshMain — a batch save can emit a separate RefreshOther-only entry for the same Id, which must + // not be flagged. + foreach (ContentTypeChange change in changes) + { + if (change.ChangeTypes.HasType(ContentTypeChangeTypes.RefreshMain) + && rawDataUnaffectedCandidateIds.Contains(change.Item.Id) + && rebuildRequiredIds.Contains(change.Item.Id) is false) + { + change.ChangeTypes |= ContentTypeChangeTypes.RawDataUnaffected; + } + } + return changes; } diff --git a/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs b/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs index 0b5104f13100..cd542294d12c 100644 --- a/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs +++ b/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; @@ -31,6 +32,7 @@ internal sealed class CacheRefreshingNotificationHandler : private readonly IMediaCacheService _mediaCacheService; private readonly IPublishedContentTypeCache _publishedContentTypeCache; private readonly CacheSettings _cacheSettings; + private readonly ILogger _logger; /// /// Initializes a new instance of the class. @@ -39,16 +41,19 @@ internal sealed class CacheRefreshingNotificationHandler : /// The media cache service. /// The published content type cache. /// The cache settings. + /// The logger. public CacheRefreshingNotificationHandler( IDocumentCacheService documentCacheService, IMediaCacheService mediaCacheService, IPublishedContentTypeCache publishedContentTypeCache, - IOptions cacheSettings) + IOptions cacheSettings, + ILogger logger) { _documentCacheService = documentCacheService; _mediaCacheService = mediaCacheService; _publishedContentTypeCache = publishedContentTypeCache; _cacheSettings = cacheSettings.Value; + _logger = logger; } /// @@ -86,9 +91,10 @@ public async Task HandleAsync(MediaDeletedNotification notification, Cancellatio public Task HandleAsync(ContentTypeRefreshedNotification notification, CancellationToken cancellationToken) #pragma warning restore CS0618 // Type or member is obsolete { - // Separate structural changes (RefreshMain) from non-structural changes (RefreshOther). - // Structural changes require a full rebuild, while non-structural changes only need - // the converted content cache cleared since ContentCacheNode only stores ContentTypeId. + // These two sets identify every refreshed content type (structural + non-structural) purely so their + // content type cache is cleared below. The rebuild-vs-clear-converted decision is made separately + // further down via RequiresRawDataRebuild()/RequiresConvertedCacheClearOnly(), because a structural + // change flagged RawDataUnaffected (a property removal) needs the converted clear, not a rebuild. var structuralChangeIds = notification.Changes .Where(x => x.ChangeTypes.IsStructuralChange()) .Select(x => x.Item.Id) @@ -105,21 +111,51 @@ public Task HandleAsync(ContentTypeRefreshedNotification notification, Cancellat _publishedContentTypeCache.ClearContentType(contentTypeId); } - // Full rebuild only for structural changes (property removed, alias changed, variation changed, etc.) + // Rebuild only for structural changes that actually affect the stored data (alias/variation changes, etc.). // In deferred mode, the rebuild is queued from the ContentTypeChangedNotification handler instead, // which fires after the scope is disposed — avoiding database lock contention between the // deferred rebuild's transaction and the original save transaction. - if (structuralChangeIds.Length > 0 && _cacheSettings.ContentTypeRebuildMode != ContentTypeRebuildMode.Deferred) + var rebuildIds = notification.Changes + .Where(x => x.ChangeTypes.RequiresRawDataRebuild()) + .Select(x => x.Item.Id) + .ToArray(); + + if (rebuildIds.Length > 0) { - _documentCacheService.Rebuild(structuralChangeIds); + if (_cacheSettings.ContentTypeRebuildMode != ContentTypeRebuildMode.Deferred) + { + if (_logger.IsEnabled(LogLevel.Debug)) + { + _logger.LogDebug("Content type change: rebuilding the document database cache for content type(s) {ContentTypeIds}.", rebuildIds); + } + + _documentCacheService.Rebuild(rebuildIds); + } + else if (_logger.IsEnabled(LogLevel.Debug)) + { + // In deferred mode this handler does nothing here; DeferredCacheRebuildNotificationHandler + // performs the rebuild in response to ContentTypeChangedNotification after the scope commits. + _logger.LogDebug("Content type change: document database cache rebuild for content type(s) {ContentTypeIds} left to the deferred rebuild (ContentTypeRebuildMode.Deferred).", rebuildIds); + } } - // For non-structural changes (name, icon, description, new property added), - // just clear the converted content cache - HybridCache entries remain valid. - // Selective clearing is safe here because no model factory reset occurs in this handler. - if (nonStructuralChangeIds.Length > 0) + // Non-structural changes (name, icon, description, new property added), plus structural changes whose + // raw data is unaffected (a property removal): the stored cmsContentNu blob stays valid, so only the + // converted content cache needs clearing. Selective clearing is safe here because no model factory + // reset occurs in this handler. + var clearConvertedCacheIds = notification.Changes + .Where(x => x.ChangeTypes.RequiresConvertedCacheClearOnly()) + .Select(x => x.Item.Id) + .ToArray(); + + if (clearConvertedCacheIds.Length > 0) { - _documentCacheService.ClearConvertedContentCache(nonStructuralChangeIds); + if (_logger.IsEnabled(LogLevel.Debug)) + { + _logger.LogDebug("Content type change: clearing the converted document cache only (no database rebuild) for content type(s) {ContentTypeIds}.", clearConvertedCacheIds); + } + + _documentCacheService.ClearConvertedContentCache(clearConvertedCacheIds); } return Task.CompletedTask; @@ -141,9 +177,10 @@ public Task HandleAsync(ContentTypeDeletedNotification notification, Cancellatio public Task HandleAsync(MediaTypeRefreshedNotification notification, CancellationToken cancellationToken) #pragma warning restore CS0618 // Type or member is obsolete { - // Separate structural changes (RefreshMain) from non-structural changes (RefreshOther). - // Structural changes require a full rebuild, while non-structural changes only need - // the converted content cache cleared since ContentCacheNode only stores ContentTypeId. + // These two sets identify every refreshed media type (structural + non-structural) purely so their + // content type cache is cleared below. The rebuild-vs-clear-converted decision is made separately + // further down via RequiresRawDataRebuild()/RequiresConvertedCacheClearOnly(), because a structural + // change flagged RawDataUnaffected (a property removal) needs the converted clear, not a rebuild. var structuralChangeIds = notification.Changes .Where(x => x.ChangeTypes.IsStructuralChange()) .Select(x => x.Item.Id) @@ -160,21 +197,51 @@ public Task HandleAsync(MediaTypeRefreshedNotification notification, Cancellatio _publishedContentTypeCache.ClearContentType(mediaTypeId); } - // Full rebuild only for structural changes (property removed, alias changed, variation changed, etc.) + // Rebuild only for structural changes that actually affect the stored data (alias/variation changes, etc.). // In deferred mode, the rebuild is queued from the MediaTypeChangedNotification handler instead, // which fires after the scope is disposed — avoiding database lock contention between the // deferred rebuild's transaction and the original save transaction. - if (structuralChangeIds.Length > 0 && _cacheSettings.ContentTypeRebuildMode != ContentTypeRebuildMode.Deferred) + var rebuildIds = notification.Changes + .Where(x => x.ChangeTypes.RequiresRawDataRebuild()) + .Select(x => x.Item.Id) + .ToArray(); + + if (rebuildIds.Length > 0) { - _mediaCacheService.Rebuild(structuralChangeIds); + if (_cacheSettings.ContentTypeRebuildMode != ContentTypeRebuildMode.Deferred) + { + if (_logger.IsEnabled(LogLevel.Debug)) + { + _logger.LogDebug("Media type change: rebuilding the media database cache for media type(s) {MediaTypeIds}.", rebuildIds); + } + + _mediaCacheService.Rebuild(rebuildIds); + } + else if (_logger.IsEnabled(LogLevel.Debug)) + { + // In deferred mode this handler does nothing here; DeferredCacheRebuildNotificationHandler + // performs the rebuild in response to MediaTypeChangedNotification after the scope commits. + _logger.LogDebug("Media type change: media database cache rebuild for media type(s) {MediaTypeIds} left to the deferred rebuild (ContentTypeRebuildMode.Deferred).", rebuildIds); + } } - // For non-structural changes (name, icon, description, new property added), - // just clear the converted content cache - HybridCache entries remain valid. - // Selective clearing is safe here because no model factory reset occurs in this handler. - if (nonStructuralChangeIds.Length > 0) + // Non-structural changes (name, icon, description, new property added), plus structural changes whose + // raw data is unaffected (a property removal): the stored cmsContentNu blob stays valid, so only the + // converted content cache needs clearing. Selective clearing is safe here because no model factory + // reset occurs in this handler. + var clearConvertedCacheIds = notification.Changes + .Where(x => x.ChangeTypes.RequiresConvertedCacheClearOnly()) + .Select(x => x.Item.Id) + .ToArray(); + + if (clearConvertedCacheIds.Length > 0) { - _mediaCacheService.ClearConvertedContentCache(nonStructuralChangeIds); + if (_logger.IsEnabled(LogLevel.Debug)) + { + _logger.LogDebug("Media type change: clearing the converted media cache only (no database rebuild) for media type(s) {MediaTypeIds}.", clearConvertedCacheIds); + } + + _mediaCacheService.ClearConvertedContentCache(clearConvertedCacheIds); } return Task.CompletedTask; diff --git a/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/DeferredCacheRebuildNotificationHandler.cs b/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/DeferredCacheRebuildNotificationHandler.cs index f895bc080655..2ad7f1473b91 100644 --- a/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/DeferredCacheRebuildNotificationHandler.cs +++ b/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/DeferredCacheRebuildNotificationHandler.cs @@ -43,14 +43,14 @@ public void Handle(ContentTypeChangedNotification notification) return; } - var structuralChangeIds = notification.Changes - .Where(x => x.ChangeTypes.IsStructuralChange()) + var rebuildIds = notification.Changes + .Where(x => x.ChangeTypes.RequiresRawDataRebuild()) .Select(x => x.Item.Id) .ToArray(); - if (structuralChangeIds.Length > 0) + if (rebuildIds.Length > 0) { - _deferredCacheRebuildService.QueueContentTypeRebuild(structuralChangeIds); + _deferredCacheRebuildService.QueueContentTypeRebuild(rebuildIds); } } @@ -62,14 +62,14 @@ public void Handle(MediaTypeChangedNotification notification) return; } - var structuralChangeIds = notification.Changes - .Where(x => x.ChangeTypes.IsStructuralChange()) + var rebuildIds = notification.Changes + .Where(x => x.ChangeTypes.RequiresRawDataRebuild()) .Select(x => x.Item.Id) .ToArray(); - if (structuralChangeIds.Length > 0) + if (rebuildIds.Length > 0) { - _deferredCacheRebuildService.QueueMediaTypeRebuild(structuralChangeIds); + _deferredCacheRebuildService.QueueMediaTypeRebuild(rebuildIds); } } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentTypeEditingServiceTests.Update.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentTypeEditingServiceTests.Update.cs index 05df46bfbd24..a357adbc5b1b 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentTypeEditingServiceTests.Update.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentTypeEditingServiceTests.Update.cs @@ -416,7 +416,7 @@ public async Task Can_Remove_Properties(bool isElement) Assert.AreEqual(0, contentType.NoGroupPropertyTypes.Count()); // expect RefreshMain when removing properties - AssertContentTypeRefreshPayload(refreshedPayloads, contentType.Id, ContentTypeChangeTypes.RefreshMain); + AssertContentTypeRefreshPayload(refreshedPayloads, contentType.Id, ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected); } [TestCase(false)] @@ -464,7 +464,7 @@ public async Task Can_Remove_Single_Property_From_Container(bool isElement) Assert.AreEqual(0, contentType.NoGroupPropertyTypes.Count()); // expect RefreshMain when removing properties - AssertContentTypeRefreshPayload(refreshedPayloads, contentType.Id, ContentTypeChangeTypes.RefreshMain); + AssertContentTypeRefreshPayload(refreshedPayloads, contentType.Id, ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected); } [Test] @@ -507,7 +507,7 @@ public async Task Can_Remove_Properties_Without_Container() Assert.AreEqual(0, contentType.NoGroupPropertyTypes.Count()); // expect RefreshMain when removing properties - AssertContentTypeRefreshPayload(refreshedPayloads, contentType.Id, ContentTypeChangeTypes.RefreshMain); + AssertContentTypeRefreshPayload(refreshedPayloads, contentType.Id, ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected); } [TestCase(false)] @@ -1512,7 +1512,8 @@ public async Task Can_Add_Composition_With_Conflicting_Property_Type_Alias_When_ Assert.AreEqual("Same Test Property Alias", compositionProperty.Name); }); - // expect RefreshMain, because a property was removed to "make room" for the new compositions + // Expect a full rebuild (RefreshMain, not RawDataUnaffected): the removed alias is reintroduced by the + // added composition behind a different property type, so the alias-keyed cache blob must be rebuilt. AssertContentTypeRefreshPayload(refreshedPayloads, targetContentType.Id, ContentTypeChangeTypes.RefreshMain); } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheDocumentTypeTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheDocumentTypeTests.cs index 0bc36b51c4ba..da53df780757 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheDocumentTypeTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheDocumentTypeTests.cs @@ -1,10 +1,17 @@ +using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.Changes; using Umbraco.Cms.Core.Sync; +using Umbraco.Cms.Infrastructure.Persistence; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; using Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; @@ -15,17 +22,182 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.PublishedCache.HybridCache; [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] internal sealed class DocumentHybridCacheDocumentTypeTests : UmbracoIntegrationTestWithContentEditing { + // Per-test (the fixture is instantiated per test) capture of content type changes, so nothing leaks between + // tests. The instance is registered below so the capture handler writes into it. + private readonly List> _capturedContentTypeChanges = []; + protected override void CustomTestSetup(IUmbracoBuilder builder) { builder.AddNotificationHandler(); builder.AddNotificationHandler(); + builder.AddNotificationHandler(); + builder.Services.AddSingleton(_capturedContentTypeChanges); builder.Services.AddUnique(); } + [SetUp] + public void ResetCapturedContentTypeChanges() => _capturedContentTypeChanges.Clear(); + private IPublishedContentCache PublishedContentHybridCache => GetRequiredService(); private IContentTypeService ContentTypeService => GetRequiredService(); + private ISqlContext SqlContext => GetRequiredService(); + + /// + /// Proves that removing a property does not rebuild the stored database cache, yet the removed property + /// is no longer exposed by any published read path (template Value/GetProperty/Properties, + /// which is also the source the Delivery API maps from). Correctness comes from resolving against the + /// current content type, not from regenerating the stored blob. + /// + [Test] + public async Task Removing_Property_Hides_Value_Without_Rebuilding_Stored_Cache() + { + // Arrange - ensure the draft blob is populated, then snapshot the stored cmsContentNu row. + var before = await PublishedContentHybridCache.GetByIdAsync(TextpageId, true); + Assert.That(before!.Value("title"), Is.Not.Null); + var blobBefore = ReadDraftBlobSignature(TextpageId); + + // Act - a pure property removal (a RawDataUnaffected structural change). + ContentType.RemovePropertyType("title"); + await ContentTypeService.UpdateAsync(ContentType, Constants.Security.SuperUserKey); + + // Assert - the property is gone from every published read path. + var after = await PublishedContentHybridCache.GetByIdAsync(TextpageId, true); + Assert.Multiple(() => + { + Assert.That(after!.Value("title"), Is.Null, "Templates must not render the removed property's old value."); + Assert.That(after!.GetProperty("title"), Is.Null); + Assert.That(after!.Properties.Any(p => p.Alias == "title"), Is.False, "Delivery API maps from Properties, so it must not expose the removed property."); + }); + + // Assert - the stored cmsContentNu blob was NOT rebuilt. A rebuild would have regenerated it from the + // now-deleted property data (dropping the orphaned value); an unchanged blob proves the rebuild was skipped. + var blobAfter = ReadDraftBlobSignature(TextpageId); + Assert.That(blobAfter, Is.EqualTo(blobBefore), "The stored blob should be untouched — the rebuild must be skipped for a property removal."); + } + + /// + /// Guards against over-broadening the skip: a property alias change is structural but NOT + /// RawDataUnaffected, so it must still rebuild the stored cache (the blob is re-keyed to the new alias). + /// + [Test] + public async Task Renaming_Property_Alias_Still_Rebuilds_Stored_Cache() + { + // Arrange + var before = await PublishedContentHybridCache.GetByIdAsync(TextpageId, true); + Assert.That(before!.Value("title"), Is.Not.Null); + var blobBefore = ReadDraftBlobSignature(TextpageId); + + // Act - rename an existing property's alias (a structural change that DOES affect the stored data). + var titleProperty = ContentType.PropertyTypes.First(x => x.Alias == "title"); + titleProperty.Alias = "newTitle"; + await ContentTypeService.UpdateAsync(ContentType, Constants.Security.SuperUserKey); + + // Assert - the stored blob was rebuilt (re-keyed under the new alias), proving the rebuild was not skipped. + var blobAfter = ReadDraftBlobSignature(TextpageId); + Assert.That(blobAfter, Is.Not.EqualTo(blobBefore), "An alias change must still rebuild the stored blob."); + } + + /// + /// A composition removal is structural but is deliberately NOT treated as RawDataUnaffected (kept as a + /// full rebuild), so the change must not carry the flag. + /// + [Test] + public async Task Removing_A_Composition_Still_Requires_A_Rebuild() + { + var composition = await CreateContentType("compositionType", "compProp"); + var composing = await CreateContentType("composingType", "ownProp", composition); + + _capturedContentTypeChanges.Clear(); + + // Act + composing.RemoveContentType("compositionType"); + await ContentTypeService.UpdateAsync(composing, Constants.Security.SuperUserKey); + + // Assert + var changes = ChangeTypesFor(composing.Id); + Assert.Multiple(() => + { + Assert.That(changes.Any(c => c.RequiresRawDataRebuild()), Is.True, "Composition removal must still require a rebuild."); + Assert.That(changes.Any(c => c.HasType(ContentTypeChangeTypes.RawDataUnaffected)), Is.False); + }); + } + + /// + /// Exercises the multi-type guard: in a batch save, one type only has a property removed (a + /// RawDataUnaffected candidate) while a type it composes has a property alias change (rebuild-required, + /// which propagates to the composing type). The composing type must NOT end up flagged RawDataUnaffected, + /// because its inherited renamed property means the stored blob is genuinely stale. + /// + [Test] + public async Task Batch_Save_Does_Not_Flag_A_Type_That_Independently_Requires_A_Rebuild() + { + var composition = await CreateContentType("compositionType", "compProp"); + var composing = await CreateContentType("composingType", "ownProp", composition); + + _capturedContentTypeChanges.Clear(); + + // Act - composing removes its own property (candidate); composition renames its property alias + // (rebuild-required, propagates to composing). Saved together as a single batch so both changes are + // classified in one ComposeContentTypeChanges call — which is what exercises the multi-type guard. + composing.RemovePropertyType("ownProp"); + composition.PropertyTypes.First(p => p.Alias == "compProp").Alias = "compPropRenamed"; +#pragma warning disable CS0618 // Type or member is obsolete + ContentTypeService.Save([composing, composition]); +#pragma warning restore CS0618 // Type or member is obsolete + + // Assert - the composing type keeps a full rebuild; the guard prevents the removal-only flag. + // (It can surface as more than one change entry — the guard must leave none of them flagged.) + var composingChanges = ChangeTypesFor(composing.Id); + Assert.Multiple(() => + { + Assert.That(composingChanges.Any(c => c.RequiresRawDataRebuild()), Is.True, "A type that inherits a renamed property must still be rebuilt."); + Assert.That(composingChanges.Any(c => c.HasType(ContentTypeChangeTypes.RawDataUnaffected)), Is.False); + }); + } + + private IReadOnlyList ChangeTypesFor(int contentTypeId) => + _capturedContentTypeChanges.Where(c => c.Item.Id == contentTypeId).Select(c => c.ChangeTypes).ToList(); + + private async Task CreateContentType(string alias, string propertyAlias, IContentType? composition = null) + { + ContentType contentType = ContentTypeBuilder.CreateBasicContentType(alias, alias); + contentType.AddPropertyType( + new PropertyType(ShortStringHelper, Constants.PropertyEditors.Aliases.TextBox, ValueStorageType.Ntext, propertyAlias) + { + Name = propertyAlias, + DataTypeId = Constants.DataTypes.Textbox, + }, + "content", + "Content"); + + if (composition is not null) + { + contentType.AddContentType(composition); + } + + await ContentTypeService.CreateAsync(contentType, Constants.Security.SuperUserKey); + return contentType; + } + + private string ReadDraftBlobSignature(int nodeId) + { + using var scope = ScopeProvider.CreateScope(autoComplete: true); + var sql = SqlContext.Sql() + .Select() + .From() + .Where(x => x.NodeId == nodeId); + ContentNuDto draft = ScopeAccessor.AmbientScope!.Database.Fetch(sql).Single(x => !x.Published); + return (draft.Data ?? string.Empty) + "|" + Convert.ToBase64String(draft.RawData ?? Array.Empty()); + } + + private sealed class ContentTypeChangeCapture(List> capturedChanges) + : INotificationHandler + { + public void Handle(ContentTypeChangedNotification notification) => capturedChanges.AddRange(notification.Changes); + } + [Test] public async Task Structural_Update_Removes_Property_From_Draft_Content_By_Id() { @@ -50,7 +222,7 @@ public async Task Structural_Update_Removes_Property_From_Draft_Content_By_Key() ContentType.RemovePropertyType("title"); await ContentTypeService.UpdateAsync(ContentType, Constants.Security.SuperUserKey); - //Assert + // Assert var newTextPage = await PublishedContentHybridCache.GetByIdAsync(Textpage.Key.Value, true); Assert.IsNull(newTextPage.Value("title")); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/ContentTypeCacheRefresherTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/ContentTypeCacheRefresherTests.cs index 68110e5df373..03ac835e1218 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/ContentTypeCacheRefresherTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/ContentTypeCacheRefresherTests.cs @@ -168,6 +168,35 @@ public void Non_Structural_Media_Change_Selectively_Clears_Converted_Cache() Times.Never); } + [Test] + public void RawDataUnaffected_Document_Change_Selectively_Clears_Converted_Cache_Without_Memory_Rebuild() + { + // Arrange — a property removal is structural but RawDataUnaffected, so on other servers the stored blob + // and HybridCache entries stay valid; only the converted cache needs clearing (no memory rebuild/tag eviction). + var refresher = CreateRefresher(Mock.Of()); + var payloads = new[] + { + new ContentTypeCacheRefresher.JsonPayload(nameof(IContentType), 100, ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected), + }; + + _documentCacheService + .Setup(x => x.ClearConvertedContentCache(It.Is>(ids => ids.Count == 1 && ids.Contains(100)))); + + // Act + refresher.Refresh(payloads); + + // Assert — selective converted-cache clear, no memory rebuild, no full clear. + _documentCacheService.Verify( + x => x.ClearConvertedContentCache(It.Is>(ids => ids.Count == 1 && ids.Contains(100))), + Times.Once); + _documentCacheService.Verify( + x => x.RebuildMemoryCacheByContentTypeAsync(It.IsAny>()), + Times.Never); + _documentCacheService.Verify( + x => x.ClearConvertedContentCache(), + Times.Never); + } + [Test] public void Combined_Structural_And_Non_Structural_Change_Uses_Rebuild_Not_Clear() { diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ContentTypeChangeExtensionsTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ContentTypeChangeExtensionsTests.cs index 596b97044760..e626413df74a 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ContentTypeChangeExtensionsTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ContentTypeChangeExtensionsTests.cs @@ -31,4 +31,24 @@ public void IsStructuralChange(ContentTypeChangeTypes change, bool expected) => [TestCase(ContentTypeChangeTypes.Remove, false)] public void IsNonStructuralChange(ContentTypeChangeTypes change, bool expected) => Assert.AreEqual(expected, change.IsNonStructuralChange()); + + [TestCase(ContentTypeChangeTypes.RefreshMain, true)] + [TestCase(ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.VariationChanged, true)] + [TestCase(ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected, false)] + [TestCase(ContentTypeChangeTypes.RefreshOther, false)] + [TestCase(ContentTypeChangeTypes.RawDataUnaffected, false)] // never set without RefreshMain, but guard anyway + [TestCase(ContentTypeChangeTypes.None, false)] + [TestCase(ContentTypeChangeTypes.Create, false)] + public void RequiresRawDataRebuild(ContentTypeChangeTypes change, bool expected) => + Assert.AreEqual(expected, change.RequiresRawDataRebuild()); + + [TestCase(ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected, true)] + [TestCase(ContentTypeChangeTypes.RefreshOther, true)] + [TestCase(ContentTypeChangeTypes.RefreshMain, false)] + [TestCase(ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.VariationChanged, false)] + [TestCase(ContentTypeChangeTypes.None, false)] + [TestCase(ContentTypeChangeTypes.Create, false)] + [TestCase(ContentTypeChangeTypes.Remove, false)] + public void RequiresConvertedCacheClearOnly(ContentTypeChangeTypes change, bool expected) => + Assert.AreEqual(expected, change.RequiresConvertedCacheClearOnly()); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/CacheRefreshingNotificationHandlerTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/CacheRefreshingNotificationHandlerTests.cs index e81563286dbe..25b1276ca3ef 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/CacheRefreshingNotificationHandlerTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/CacheRefreshingNotificationHandlerTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; @@ -36,7 +37,8 @@ private CacheRefreshingNotificationHandler CreateHandler(ContentTypeRebuildMode _documentCacheService.Object, _mediaCacheService.Object, _publishedContentTypeCache.Object, - Options.Create(new CacheSettings { ContentTypeRebuildMode = mode })); + Options.Create(new CacheSettings { ContentTypeRebuildMode = mode }), + NullLogger.Instance); /// /// Verifies that a structural content type change in immediate mode triggers a synchronous database cache rebuild. @@ -133,6 +135,68 @@ public async Task Non_Structural_Content_Type_Change_Never_Does_Full_Clear() Times.Never); } + /// + /// Verifies that a property-removal (RawDataUnaffected) content type change skips the database rebuild and + /// only clears the converted cache, since the stored cmsContentNu blob stays valid. + /// + [Test] + public async Task RawDataUnaffected_Content_Type_Change_Skips_Rebuild_And_Clears_Converted_Cache() + { + // Arrange + var contentType = CreateContentType(100); +#pragma warning disable CS0618 // Type or member is obsolete + var notification = new ContentTypeRefreshedNotification( + new ContentTypeChange(contentType, ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected), + new EventMessages()); +#pragma warning restore CS0618 // Type or member is obsolete + + _documentCacheService + .Setup(x => x.ClearConvertedContentCache(It.Is>(ids => ids.Count == 1 && ids.Contains(100)))); + + // Act + await _handler.HandleAsync(notification, CancellationToken.None); + + // Assert — no raw database rebuild for a property removal. + _documentCacheService.Verify( + x => x.Rebuild(It.IsAny>()), + Times.Never); + + // Assert — converted cache is cleared selectively by content type ID. + _documentCacheService.Verify( + x => x.ClearConvertedContentCache(It.Is>(ids => ids.Count == 1 && ids.Contains(100))), + Times.Once); + } + + /// + /// Verifies that a property-removal (RawDataUnaffected) media type change skips the database rebuild and + /// only clears the converted cache. + /// + [Test] + public async Task RawDataUnaffected_Media_Type_Change_Skips_Rebuild_And_Clears_Converted_Cache() + { + // Arrange + var mediaType = CreateMediaType(200); +#pragma warning disable CS0618 // Type or member is obsolete + var notification = new MediaTypeRefreshedNotification( + new ContentTypeChange(mediaType, ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected), + new EventMessages()); +#pragma warning restore CS0618 // Type or member is obsolete + + _mediaCacheService + .Setup(x => x.ClearConvertedContentCache(It.Is>(ids => ids.Count == 1 && ids.Contains(200)))); + + // Act + await _handler.HandleAsync(notification, CancellationToken.None); + + // Assert + _mediaCacheService.Verify( + x => x.Rebuild(It.IsAny>()), + Times.Never); + _mediaCacheService.Verify( + x => x.ClearConvertedContentCache(It.Is>(ids => ids.Count == 1 && ids.Contains(200))), + Times.Once); + } + /// /// Verifies that a structural media type change in immediate mode triggers a synchronous database cache rebuild. /// @@ -274,6 +338,38 @@ public async Task Deferred_Mode_Non_Structural_Change_Still_Clears_Converted_Cac Times.Once); } + /// + /// Verifies the deferred + RawDataUnaffected combination: a property removal in deferred mode still + /// clears the converted cache here (the stored blob stays valid) and never rebuilds. The paired + /// "must not queue a deferred rebuild" behaviour is covered by DeferredCacheRebuildNotificationHandlerTests. + /// + [Test] + public async Task Deferred_Mode_RawDataUnaffected_Change_Clears_Converted_Cache_And_Skips_Rebuild() + { + // Arrange + var handler = CreateHandler(ContentTypeRebuildMode.Deferred); + var contentType = CreateContentType(100); +#pragma warning disable CS0618 // Type or member is obsolete + var notification = new ContentTypeRefreshedNotification( + new ContentTypeChange(contentType, ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected), + new EventMessages()); +#pragma warning restore CS0618 // Type or member is obsolete + + _documentCacheService + .Setup(x => x.ClearConvertedContentCache(It.Is>(ids => ids.Count == 1 && ids.Contains(100)))); + + // Act + await handler.HandleAsync(notification, CancellationToken.None); + + // Assert — converted cache is cleared (never deferred), and no rebuild happens in either mode. + _documentCacheService.Verify( + x => x.ClearConvertedContentCache(It.Is>(ids => ids.Count == 1 && ids.Contains(100))), + Times.Once); + _documentCacheService.Verify( + x => x.Rebuild(It.IsAny>()), + Times.Never); + } + private static IContentType CreateContentType(int id) { var mock = new Mock(); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/DeferredCacheRebuildNotificationHandlerTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/DeferredCacheRebuildNotificationHandlerTests.cs index 7bb35d333f6b..cab6126129d3 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/DeferredCacheRebuildNotificationHandlerTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/DeferredCacheRebuildNotificationHandlerTests.cs @@ -123,6 +123,29 @@ public void Non_Structural_Change_Does_Not_Queue() Times.Never); } + /// + /// Verifies that a property-removal (RawDataUnaffected) change does not queue a deferred rebuild — the + /// stored blob stays valid, so only the converted cache is cleared (by the refreshers, not here). + /// + [Test] + public void RawDataUnaffected_Change_Does_Not_Queue() + { + // Arrange + var handler = CreateHandler(ContentTypeRebuildMode.Deferred); + var contentType = CreateContentType(100); + var notification = new ContentTypeChangedNotification( + new ContentTypeChange(contentType, ContentTypeChangeTypes.RefreshMain | ContentTypeChangeTypes.RawDataUnaffected), + new EventMessages()); + + // Act + handler.Handle(notification); + + // Assert + _deferredCacheRebuildService.Verify( + x => x.QueueContentTypeRebuild(It.IsAny>()), + Times.Never); + } + /// /// Verifies the handler still queues a rebuild when the notification is dispatched through a /// distributed-cache-only publisher (e.g. the publisher Umbraco Deploy installs on restore/import