Content Types: Refresh composing types when a property is added to a composition (closes #24117)

[AndyButland]

Description

Adding a property to a content type that is used as a composition left every document type composed of (or inheriting from) it serving a published content type built before the property existed. content.Value("newAlias") returned null on the front end and in Preview even after the value was filled in and published. Reload Memory Cache and Rebuild Database Cache didn't help — only an application restart did.

Fixes #24117.

The issue report diagnoses this accurately; this PR implements the suggested fix plus three related gaps found while verifying it.

Root cause

ContentTypeServiceBase.ComposeContentTypeChanges only propagated a change to the types that derive from the edited one when the change was in hasPropertyMainImpact:

var hasPropertyMainImpact = hasContentTypeVariationChanged || hasAnyPropertyVariationChanged
                            || hasAnyCompositionBeenRemoved || hasAnyPropertyBeenRemoved
                            || hasAnyPropertyChangedAlias || hasCompositionChanged;

A property addition is not in that list, so it fell into the else branch — RefreshOther for the edited type alone — and the GetComposedOf propagation loop was skipped entirely. The composing types got no change entry at all, so neither ContentTypeCacheRefresher.Refresh nor CacheRefreshingNotificationHandler ever cleared their published content type or their converted content cache.

Adding a property directly to a document type works, because that type's own id is in the payload.

It can be masked because ContentTypeCacheRefresher.Refresh clears the whole converted cache when the model factory is an IAutoPublishedModelFactory. With a live ModelsBuilder that blanket clear hides the bug, so it passes then but not with ModelsMode = Nothing.

Three further gaps found while verifying

The propagation was never transitive. GetComposedOf(id) returns direct consumers only (x.ContentTypeComposition.Any(y => y.Id == id)), but CompositionPropertyTypes recurses. Inheritance is stored as composition, so for Base ← Middle ← Leaf a change on Base reached Middle and never Leaf. This affected the existing RefreshMain propagation too, not just the new path.

There was no recovery short of a restart. IPublishedContentTypeCache.ClearAll() is declared and implemented but not called by Reload Memory Cache.

A content type alias change didn't propagate either (raised in review). hasAliasChanged puts the edited type into RefreshMain but was never part of the propagation condition. PublishedContentType snapshots contentType.CompositionAliases() at construction, and that recurses through ContentTypeComposition — so after renaming a composition, every deriving type kept reporting the old alias. Observable through the public IPublishedContent.IsComposedOf template API (also behind DescendantsOrSelfOfType) and through ContentTypeSchemaService.CompositionSchemaIds for the Delivery API.

Changes

1. Propagate a property addition to the deriving types. rawDataAffected is hoisted out of the main-impact if and the propagation loop lifted alongside it. The loop now runs for hasPropertyMainImpact || hasAnyPropertyBeenAdded || hasAliasChanged. A pure addition emits RefreshOther for the deriving types — which satisfies RequiresConvertedCacheClearOnly() and so clears both the published content type cache and the converted content cache, without a cmsContentNu rebuild. The added alias has no stored value to re-key, so no rebuild is warranted.

2. Propagate a content type alias change too, on the same RefreshOther path. The stored blob is keyed by property alias, not content type alias, so nothing re-keys there either — the deriving types just need their published content type rebuilt so their composition aliases are resolved afresh.

3. Make the propagation transitive. A private GetComposedOfTransitive walks the composition graph and is used for both the RefreshMain and the RefreshOther propagation.

4. Clear the published content type cache on a reload. ContentCacheRefresher.Refresh calls _publishedContentTypeCache.ClearAll() when a TreeChangeTypes.RefreshAll payload arrives, so Reload Memory Cache can recover a site left in this state.

Testing

Five new tests, each verified to fail before the corresponding production change and pass after.

Manual verification

Run every scenario on this branch, then repeat on v17/dev to confirm the old behaviour.

Prerequisites

• Set ModelsMode to Nothing in src/Umbraco.Web.UI/appsettings.Development.json, and confirm it on Settings → Models Builder. Under the default InMemoryAuto the whole converted cache is cleared on any content type change, which hides the bug.

  { "Umbraco": { "CMS": { "ModelsBuilder": { "ModelsMode": "Nothing" } } } }

Setup

1. Settings → Document Types → create Hero Tab with one Textstring property, alias heading. Save.
2. Create Home, allow as root, give it a template, add Hero Tab via the Compositions picker. Save.
3. Set the Home template to:

   @inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage
   @{ Layout = null; }
   <h1>@Model.Name</h1>
   <p>heading: @(Model.Value<string>("heading"))</p>
   <p>videoId  HasProperty=@Model.HasProperty("videoId")  value=@(Model.Value<string>("videoId"))</p>

4. Create and publish a Home node, filling in heading.
5. Confirm the page routes.

Verification: a property added to a composition is visible immediately

1. Restart the site, then view the Home page: videoId HasProperty=False.
2. Add a Textstring property videoId to Hero Tab and save. Do not restart.
3. View the page again.
   • v17/dev: videoId HasProperty=False.
   • This branch: videoId HasProperty=True.
4. For the full reported symptom, fill in videoId on the Home node, Save and publish, then view the page and Preview.
   • v17/dev: value= is empty, although the value is stored and displays correctly in the back office, which reads IContentType. It appears on the front end only after a restart.
   • This branch: the value renders immediately.

[claude]

Claude finished @AndyButland's task in 5m 20s —— View job

---

PR Review

Target: origin/v17/dev · Based on commit: b71fcec4

Fixes a silent stale-cache bug where adding a property to a content type used as a composition left all derived types serving a published content type built before the property existed, with no recovery short of an app restart. Three related gaps are addressed: propagating property additions to derived types, making that propagation transitive, and clearing published content type cache on a memory reload.

• Modified public API: ContentCacheRefresher — new constructor overload added; previous 14-param canonical constructor obsoleted (delegates to new 15-param one). ContentTypeServiceBase<TRepository,TItem> — no public surface changed (new GetComposedOfTransitive is private).
• Breaking changes: None. The obsolete-constructor + StaticServiceProvider pattern is applied correctly; removal is scheduled for Umbraco 19 (current+2). ✓

---

Critical

None

Important

None

Suggestions

• src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs:85: The [Obsolete] message on the newly-obsoleted 14-param constructor is identical to the message already on the 13-param constructor. Both say "Please use the constructor taking all parameters." When there are two obsolete constructors, this wording is ambiguous — consumers can't tell which constructor to migrate to. A concrete parameter-count hint ("the 16-parameter constructor" or naming the new IPublishedContentTypeCache param) would make the migration path unambiguous. Minor, but may cause confusion in dotnet tooling warnings.

• tests/Umbraco.Tests.Integration/Umbraco.Core/Cache/ReloadPublishedCacheTests.cs:43: Assert.IsFalse(ReferenceEquals(primed, after), ...) correctly tests that the cache was cleared (since a cache hit returns the same instance). If a future implementation of IPublishedContentTypeCache returns new instances on every Get() call regardless of caching (e.g. for test isolation), this assertion would pass vacuously. A stronger assertion — e.g. that after reflects a re-fetched definition (comparing property counts, or checking a captured BuildCount if the implementation exposes one) — would be more robust. That said, for the current HybridPublishedContentTypeCache implementation this is perfectly adequate.

---

Approved with Suggestions for improvement

Good to go. The three root causes are correctly diagnosed and patched, the obsolete-constructor pattern is applied cleanly, GetComposedOfTransitive is a correct BFS traversal with cycle-guard, and the four new tests are well-targeted. The suggestions above are genuinely minor — the core change is solid.

[Copilot]

Copilot review overview

🟡 Changes recommended

Alias changes still leave descendant published types stale, and the new traversal scales quadratically.

Review effort: Balanced
Findings: 2

Open (2)

• Propagate composition alias changes to descendant type caches · New
• Avoid rescanning content types for each composition descendant · New

What changed in this PR

Fixes stale published content types when compositions gain properties, including transitive descendants and manual cache reload recovery.

Changes:

• Propagates property-set changes transitively without unnecessary database-cache rebuilds.
• Clears published content-type definitions during full published-cache reloads.
• Adds integration coverage for composition, inheritance, and reload scenarios.

File	Description
ContentCacheRefresherIdKeyMapTests.cs	Updates refresher construction.
DocumentHybridCacheDocumentTypeTests.cs	Tests composed property additions without blob rebuilds.
ContentTypeEditingServiceTests.Update.cs	Verifies refresh payload propagation.
ReloadPublishedCacheTests.cs	Tests full-reload content-type invalidation.
PublishedContentTypeCacheTests.cs	Tests direct and transitive invalidation.
ContentTypeServiceBase{TRepository,TItem}.cs	Adds transitive change propagation.
ContentCacheRefresher.cs	Clears published content types on full reload.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[kjac]

@AndyButland this looks good. I have pushed a few updates:

• A few more test scenarios covering chained inheritance.
• An optimization for batch saves. While this is not the norm, batch saves can happen; the API permits them (albeit obsoleted), and the package installer (still) performs them.

The optimization in a nutshell:

1. Fetches all content types once instead of per content type in the batch. Minor optimization, but it prevents excessive deep cloning from the repo (cached at repo level).
2. Ensures only a single change is effectuated for any content type (in)directly affected by the batch. This is more important, as it potentially prevents consumers from duplicate work as a result of content type change(s).

Neither of these changes are decisively critical, particularly not for "normal operations", but please do have a look and see if you agree. You're more than welcome to roll them back if you disagree.

[AndyButland]

Thanks @kjac, the updates look good to me.

[sonarqubecloud]

Quality Gate passed

Issues
23 New issues
0 Accepted issues

Measures
0 Security Hotspots
2.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[kjac]

Good to go then 👍