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 @@ -175,68 +175,69 @@ 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<T>()) from being returned. In non-auto modes, only affected types need clearing.
var isAutoFactory = _publishedModelFactory is IAutoPublishedModelFactory;

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);
}
}
});
Expand Down
23 changes: 23 additions & 0 deletions src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,27 @@ public static bool IsStructuralChange(this ContentTypeChangeTypes change) =>
/// <returns><c>true</c> if the change has non-structural impact; otherwise, <c>false</c>.</returns>
public static bool IsNonStructuralChange(this ContentTypeChangeTypes change) =>
change.HasType(ContentTypeChangeTypes.RefreshOther) && !change.HasType(ContentTypeChangeTypes.RefreshMain);

/// <summary>
/// Determines whether the change requires the raw database cache (<c>cmsContentNu</c>) to be rebuilt.
/// </summary>
/// <param name="change">The change to check.</param>
/// <returns>
/// <c>true</c> for a structural change unless it is flagged <see cref="ContentTypeChangeTypes.RawDataUnaffected"/>
/// (e.g. a property removal), in which case the stored blob stays valid and only the converted cache needs clearing.
/// </returns>
public static bool RequiresRawDataRebuild(this ContentTypeChangeTypes change) =>
change.IsStructuralChange() && !change.HasType(ContentTypeChangeTypes.RawDataUnaffected);

/// <summary>
/// Determines whether the change only requires the converted (in-memory) content cache to be cleared,
/// leaving the stored database cache (<c>cmsContentNu</c>) and HybridCache entries valid.
/// </summary>
/// <param name="change">The change to check.</param>
/// <returns>
/// <c>true</c> for a non-structural change, or a structural change flagged
/// <see cref="ContentTypeChangeTypes.RawDataUnaffected"/> (e.g. a property removal).
/// </returns>
public static bool RequiresConvertedCacheClearOnly(this ContentTypeChangeTypes change) =>
change.IsNonStructuralChange() || (change.IsStructuralChange() && change.HasType(ContentTypeChangeTypes.RawDataUnaffected));
}
14 changes: 14 additions & 0 deletions src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
/// </summary>
VariationChanged = 16,

/// <summary>
/// Supplements <see cref="RefreshMain"/> to indicate that, although the change is structural, the raw
/// content data stored in the database cache (<c>cmsContentNu</c>) does not need rebuilding.
/// </summary>
/// <remarks>
/// 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 <see cref="RefreshMain"/> handling (e.g. search
/// re-indexing, published content type cache clearing) still runs — only the expensive
/// <c>cmsContentNu</c> rebuild is skipped.
/// </remarks>
RawDataUnaffected = 32,
}
Original file line number Diff line number Diff line change
Expand Up @@ -357,57 +357,85 @@

var changes = new List<ContentTypeChange<TItem>>();

// 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<int>();
var rawDataUnaffectedCandidateIds = new HashSet<int>();

foreach (TItem contentType in contentTypes)
{
var dirty = (IRememberBeingDirty)contentType;

// skip new content types
// TODO: This used to be WasPropertyDirty("HasIdentity") but i don't think that actually worked for detecting new entities this does seem to work properly
var isNewContentType = dirty.WasPropertyDirty("Id");
if (isNewContentType)
{
AddChange(changes, contentType, ContentTypeChangeTypes.Create);
continue;
}

// alias change?
var hasAliasChanged = dirty.WasPropertyDirty("Alias");

// existing property alias change?
var hasAnyPropertyChangedAlias = contentType.PropertyTypes.Any(propertyType =>
{
// skip new properties
// TODO: This used to be WasPropertyDirty("HasIdentity") but i don't think that actually worked for detecting new entities this does seem to work properly
var isNewProperty = propertyType.WasPropertyDirty("Id");
if (isNewProperty)
{
return false;
}

// alias change?
return propertyType.WasPropertyDirty("Alias");
});

// removed properties?
var hasAnyPropertyBeenRemoved = dirty.WasPropertyDirty("HasPropertyTypeBeenRemoved");

// removed compositions?
var hasAnyCompositionBeenRemoved = dirty.WasPropertyDirty("HasCompositionTypeBeenRemoved");

// variation changed?
var hasContentTypeVariationChanged = dirty.WasPropertyDirty("Variations");

// property variation change?
var hasAnyPropertyVariationChanged = contentType.WasPropertyTypeVariationChanged();

// main impact on properties?
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 &&

Check warning on line 429 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the unnecessary Boolean literal(s).

See more on https://sonarcloud.io/project/issues?id=umbraco_Umbraco-CMS&issues=AZ9CVdbLrkAjOSPQQ79S&open=AZ9CVdbLrkAjOSPQQ79S&pullRequest=23330
hasAnyPropertyChangedAlias is false &&

Check warning on line 430 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the unnecessary Boolean literal(s).

See more on https://sonarcloud.io/project/issues?id=umbraco_Umbraco-CMS&issues=AZ9CVdbLrkAjOSPQQ79T&open=AZ9CVdbLrkAjOSPQQ79T&pullRequest=23330
hasContentTypeVariationChanged is false &&

Check warning on line 431 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the unnecessary Boolean literal(s).

See more on https://sonarcloud.io/project/issues?id=umbraco_Umbraco-CMS&issues=AZ9CVdbLrkAjOSPQQ79U&open=AZ9CVdbLrkAjOSPQQ79U&pullRequest=23330
hasAnyPropertyVariationChanged is false &&

Check warning on line 432 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the unnecessary Boolean literal(s).

See more on https://sonarcloud.io/project/issues?id=umbraco_Umbraco-CMS&issues=AZ9CVdbLrkAjOSPQQ79V&open=AZ9CVdbLrkAjOSPQQ79V&pullRequest=23330
hasAnyCompositionBeenRemoved is false &&

Check warning on line 433 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the unnecessary Boolean literal(s).

See more on https://sonarcloud.io/project/issues?id=umbraco_Umbraco-CMS&issues=AZ9GQQRApqNN6u-X8gHN&open=AZ9GQQRApqNN6u-X8gHN&pullRequest=23330
hasCompositionChanged is false;

Check warning on line 434 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the unnecessary Boolean literal(s).

See more on https://sonarcloud.io/project/issues?id=umbraco_Umbraco-CMS&issues=AZ9GQQRApqNN6u-X8gHO&open=AZ9GQQRApqNN6u-X8gHO&pullRequest=23330

// 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.
Expand All @@ -420,7 +448,9 @@
{
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);
}
}
}
Expand All @@ -431,6 +461,20 @@
}
}

// 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<TItem> change in changes)

Check warning on line 468 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Loops should be simplified using the "Where" LINQ method

See more on https://sonarcloud.io/project/issues?id=umbraco_Umbraco-CMS&issues=AZ9CVdbLrkAjOSPQQ79Y&open=AZ9CVdbLrkAjOSPQQ79Y&pullRequest=23330
{
if (change.ChangeTypes.HasType(ContentTypeChangeTypes.RefreshMain)
&& rawDataUnaffectedCandidateIds.Contains(change.Item.Id)
&& rebuildRequiredIds.Contains(change.Item.Id) is false)

Check warning on line 472 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

❌ New issue: Complex Conditional

ComposeContentTypeChanges has 1 complex conditionals with 2 branches, threshold = 2 A complex conditional is an expression inside a branch (e.g. if, for, while) which consists of multiple, logical operators such as AND/OR. The more logical operators in an expression, the more severe the code smell.

Check warning on line 472 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the unnecessary Boolean literal(s).

See more on https://sonarcloud.io/project/issues?id=umbraco_Umbraco-CMS&issues=AZ9CVdbLrkAjOSPQQ79X&open=AZ9CVdbLrkAjOSPQQ79X&pullRequest=23330
{
change.ChangeTypes |= ContentTypeChangeTypes.RawDataUnaffected;
}
}

Check warning on line 477 in src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

❌ Getting worse: Complex Method

ComposeContentTypeChanges increases in cyclomatic complexity from 14 to 26, 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.
return changes;
}

Expand Down
Loading
Loading