Hybrid Cache: Clear published content cache on content type change#21225
Hybrid Cache: Clear published content cache on content type change#21225
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where saving a document type causes documents based on that type to show null model errors in Razor rendering. The root cause is that the _publishedContentCache caches IPublishedContent objects which hold references to IPublishedContentType. When a content type is modified, these cached objects become stale and must be cleared.
Key Changes
- Adds
_publishedContentCache.Clear()at the end of theRebuildmethod inDocumentCacheServiceto clear stale published content when document types change - Adds the same cache clearing logic to
MediaCacheServiceto handle media type changes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs | Clears the entire published content cache after rebuilding when document types are modified |
| src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs | Clears the entire published content cache after rebuilding when media types are modified |
src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Zeegaan
left a comment
There was a problem hiding this comment.
This makes sense to me, we should really be looking into not having the _publishedContentCache, and instead entirely caching IPublishContent via the hybrid cache instead, this might cause som breaking changes, but effectively we're double caching published content, just to avoid having to rebuild from the factory every time, which is not ideal 🙈
…1225) * Clear the cacje level published content cache on content type change. * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]> (cherry picked from commit 396ebdd)
|
Cherry picked for 17.1 🍒 |
Issue
I found this issue when testing the results of performance improvements for save of content types in #21207, but as it's a clear regression in 17 we should look to review and release it earlier.
To reproduce:
InMemoryAutofor models builder.Cause
_publishedContentCache(introduced in Performance: Cache published content instances at cache service level #20681) cachesIPublishedContentobjects which hold references toIPublishedContentType. When a content type is modified, these cached objects become stale._publishedContentCacheat the end of theRebuildmethod in bothDocumentCacheServiceandMediaCacheServiceTechnical Details
The
_publishedContentCacheis aConcurrentDictionary<string, IPublishedContent>that caches fully-constructedIPublishedContentobjects for performance. However, these objects contain references to theirIPublishedContentType, which becomes stale when the content type is modified.A filtered/targeted clear approach was attempted but has inherent race conditions due to the non-atomic check-then-act pattern. I was also concerned that content types can be used in various places in a document (elements, compositions etc.). So the atomic
Clear()looks to be the only reliable solution.Test plan
🤖 Generated with Claude Code with human updates.