Content Types: Allow editing compositions on document types that are inherited from (closes #23103)

src/Umbraco.Cms.Api.Management/Controllers/DocumentType/CompositionReferenceDocumentTypeController.cs

@@ -8,6 +8,7 @@
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.OperationStatus;
 using Umbraco.Cms.Web.Common.Authorization;
+using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Api.Management.Controllers.DocumentType;
 
@@ -50,14 +51,14 @@ public CompositionReferenceDocumentTypeController(IContentTypeService contentTyp
     [EndpointDescription("Gets a collection of document types that reference the specified document type as a composition.")]
     public async Task<IActionResult> CompositionReferences(CancellationToken cancellationToken, Guid id)
     {
-        var contentType = await _contentTypeService.GetAsync(id);
+        IContentType? contentType = await _contentTypeService.GetAsync(id);
 
         if (contentType is null)
         {
             return OperationStatusResult(ContentTypeOperationStatus.NotFound);
         }
 
-        IEnumerable<IContentType> composedOf = _contentTypeService.GetComposedOf(contentType.Id);
+        IEnumerable<IContentType> composedOf = _contentTypeService.GetComposedOf(contentType.Id, ComposedOfType.Composition);
         List<DocumentTypeCompositionResponseModel> responseModels = _umbracoMapper.MapEnumerable<IContentType, DocumentTypeCompositionResponseModel>(composedOf);
 
         return Ok(responseModels);

src/Umbraco.Cms.Api.Management/Controllers/MediaType/CompositionReferenceMediaTypeController.cs

@@ -6,6 +6,7 @@
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.OperationStatus;
+using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Api.Management.Controllers.MediaType;
 
@@ -38,14 +39,14 @@ public CompositionReferenceMediaTypeController(IMediaTypeService mediaTypeServic
     [EndpointDescription("Gets a collection of media types that reference the specified media type as a composition.")]
     public async Task<IActionResult> CompositionReferences(CancellationToken cancellationToken, Guid id)
     {
-        var mediaType = await _mediaTypeService.GetAsync(id);
+        IMediaType? mediaType = await _mediaTypeService.GetAsync(id);
 
         if (mediaType is null)
         {
             return OperationStatusResult(ContentTypeOperationStatus.NotFound);
         }
 
-        IEnumerable<IMediaType> composedOf = _mediaTypeService.GetComposedOf(mediaType.Id);
+        IEnumerable<IMediaType> composedOf = _mediaTypeService.GetComposedOf(mediaType.Id, ComposedOfType.Composition);
         List<MediaTypeCompositionResponseModel> responseModels = _umbracoMapper.MapEnumerable<IMediaType, MediaTypeCompositionResponseModel>(composedOf);
 
         return Ok(responseModels);

src/Umbraco.Cms.Api.Management/Controllers/MemberType/CompositionReferenceMemberTypeController.cs

@@ -8,6 +8,7 @@
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.OperationStatus;
 using Umbraco.Cms.Web.Common.Authorization;
+using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Api.Management.Controllers.MemberType;
 
@@ -47,14 +48,14 @@ public CompositionReferenceMemberTypeController(IMemberTypeService memberTypeSer
     [EndpointDescription("Gets a collection of member types that reference the specified member type as a composition.")]
     public async Task<IActionResult> CompositionReferences(CancellationToken cancellationToken, Guid id)
     {
-        var memberType = await _memberTypeService.GetAsync(id);
+        IMemberType? memberType = await _memberTypeService.GetAsync(id);
 
         if (memberType is null)
         {
             return OperationStatusResult(ContentTypeOperationStatus.NotFound);
         }
 
-        IEnumerable<IMemberType> composedOf = _memberTypeService.GetComposedOf(memberType.Id);
+        IEnumerable<IMemberType> composedOf = _memberTypeService.GetComposedOf(memberType.Id, ComposedOfType.Composition);
         List<MemberTypeCompositionResponseModel> responseModels = _umbracoMapper.MapEnumerable<IMemberType, MemberTypeCompositionResponseModel>(composedOf);
 
         return Ok(responseModels);

src/Umbraco.Core/Services/ComposedOfType.cs

@@ -0,0 +1,28 @@
+namespace Umbraco.Cms.Core.Services;
+
+/// <summary>
+///     Specifies which "composed of" relationships to include when resolving the content types that build upon a
+///     given content type.
+/// </summary>
+/// <remarks>
+///     Inheritance and composition are stored in the same underlying collection and are distinguished only by whether
+///     the referencing content type's tree parent is the content type in question. This lets callers ask for a single
+///     axis or both.
+/// </remarks>
+public enum ComposedOfType
+{
+    /// <summary>
+    ///     Content types that use the content type as a true composition (i.e. not tree inheritance).
+    /// </summary>
+    Composition,
+
+    /// <summary>
+    ///     Content types that inherit from the content type in the tree.
+    /// </summary>
+    Inheritance,
+
+    /// <summary>
+    ///     Both composition users and inheritance children.
+    /// </summary>
+    All,
+}

src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingHelper.cs

@@ -0,0 +1,67 @@
+using Umbraco.Cms.Core.Models;
+
+namespace Umbraco.Cms.Core.Services.ContentTypeEditing;
+
+/// <summary>
+///     Shared helper logic for content type editing.
+/// </summary>
+internal static class ContentTypeEditingHelper
+{
+    /// <summary>
+    ///     Gets the combined, full effective property alias set (own properties plus everything each one gets from
+    ///     its own compositions) of every content type descending from <paramref name="source"/>, across both the
+    ///     tree inheritance and composition axes - excluding whatever already flows into those descendants through
+    ///     <paramref name="source"/>'s own current state.
+    /// </summary>
+    /// <param name="source">The content type to find descendants of, or <c>null</c> when there are none (e.g. creating a new content type).</param>
+    /// <param name="allContentTypes">All existing content type compositions, used to resolve the descendant tree.</param>
+    /// <returns>The property aliases (compared case-insensitively) already effective on every descendant of <paramref name="source"/>, excluding aliases <paramref name="source"/> itself already contributes.</returns>
+    /// <remarks>
+    ///     Inheritance children hold their parent in their own <see cref="IContentTypeComposition.ContentTypeComposition"/>,
+    ///     so walking the composition graph downward captures inheritance and composition descendants alike.
+    /// </remarks>
+    internal static HashSet<string> GetPropertyAliasesReservedByDescendants(this IContentTypeComposition? source, IContentTypeComposition[] allContentTypes)
+    {
+        if (source is null)
+        {
+            return [];
+        }
+
+        // build a "referenced id -> types that directly reference it" lookup once, so the traversal is O(n + d)
+        // rather than rescanning every content type for each descendant
+        ILookup<int, IContentTypeComposition> directReferencingTypes = allContentTypes
+            .SelectMany(
+                contentType => contentType.ContentTypeComposition,
+                (contentType, referenced) => (ReferencedId: referenced.Id, ContentType: contentType))
+            .ToLookup(x => x.ReferencedId, x => x.ContentType);
+
+        var descendantIds = new HashSet<int>();
+        var descendantPropertyAliases = new HashSet<string>(StringComparer.InvariantCultureIgnoreCase);
+        var stack = new Stack<int>();
+        stack.Push(source.Id);
+        while (stack.Count > 0)
+        {
+            var currentId = stack.Pop();
+            foreach (IContentTypeComposition descendant in directReferencingTypes[currentId])
+            {
+                if (!descendantIds.Add(descendant.Id))
+                {
+                    continue;
+                }
+
+                IEnumerable<IPropertyType> allProperties = descendant.PropertyTypes
+                    .Concat(descendant.CompositionPropertyTypes);
+                foreach (IPropertyType descendantPropertyType in allProperties)
+                {
+                    descendantPropertyAliases.Add(descendantPropertyType.Alias);
+                }
+
+                stack.Push(descendant.Id);
+            }
+        }
+
+        descendantPropertyAliases.ExceptWith(source.CompositionPropertyTypes.Select(pt => pt.Alias));
+
+        return descendantPropertyAliases;
+    }
+}

src/Umbraco.Core/Services/ContentTypeEditing/ContentTypeEditingServiceBase.cs

@@ -271,6 +271,13 @@
             return operationStatus;
         }
 
+        // verify that no newly introduced property/composition alias collides with a property already effective on a descendant
+        operationStatus = ValidateDescendantPropertyAliases(contentType, model, allContentTypeCompositions);
+        if (operationStatus is not ContentTypeOperationStatus.Success)
+        {
+            return operationStatus;
+        }
+
         // verify that all property/container relationships (groups/tabs) are valid
         operationStatus = ValidateContainers(model, allContentTypeCompositions);
         if (operationStatus is not ContentTypeOperationStatus.Success)
@@ -279,7 +286,7 @@
         }
 
         // element types cannot vary by segment
-        if (model.IsElement && model.VariesBySegment)
+        if (model is { IsElement: true, VariesBySegment: true })
         {
             return ContentTypeOperationStatus.InvalidSegmentVariationForElementType;
         }
@@ -432,33 +439,25 @@
     /// <param name="model">The model to validate.</param>
     /// <param name="allContentTypeCompositions">All existing content type compositions.</param>
     /// <returns>The validation status.</returns>
-    private ContentTypeOperationStatus ValidateCompositions(TContentType? contentType, ContentTypeEditingModelBase<TPropertyTypeModel, TPropertyTypeContainer> model, IContentTypeComposition[] allContentTypeCompositions)
+    private ContentTypeOperationStatus ValidateCompositions(
+        TContentType? contentType,
+        ContentTypeEditingModelBase<TPropertyTypeModel, TPropertyTypeContainer> model,
+        IContentTypeComposition[] allContentTypeCompositions)
     {
-        // get the content type keys we want to use for compositions
         Guid[] compositionKeys = KeysForCompositionTypes(model, CompositionType.Composition);
 
-        // if the content type keys are already set as compositions, don't perform any additional validation
-        // - this covers an edge case where compositions are configured for a content type before child content types are created
-        if (contentType is not null && contentType.ContentTypeComposition
-            .Select(c => c.Key)
-            .ContainsAll(compositionKeys))
-        {
-            return ContentTypeOperationStatus.Success;
-        }
-
-        // verify that all compositions keys are allowed
-        Guid[] allowedCompositionKeys = _contentTypeService.GetAvailableCompositeContentTypes(contentType, allContentTypeCompositions, isElement: model.IsElement)
+        Guid[] allowedCompositionKeys = _contentTypeService.GetAvailableCompositeContentTypes(
+                contentType,
+                allContentTypeCompositions,
+                isElement: model.IsElement)
             .Results
             .Where(x => x.Allowed)
             .Select(x => x.Composition.Key)
             .ToArray();
 
-        if (allowedCompositionKeys.ContainsAll(compositionKeys) is false)
-        {
-            return ContentTypeOperationStatus.InvalidComposition;
-        }
-
-        return ContentTypeOperationStatus.Success;
+        return allowedCompositionKeys.ContainsAll(compositionKeys) is false
+            ? ContentTypeOperationStatus.InvalidComposition
+            : ContentTypeOperationStatus.Success;
     }
 
     /// <summary>
@@ -486,6 +485,31 @@
         return ContentTypeOperationStatus.Success;
     }
 
+    /// <summary>
+    ///     Validates that no property alias on this content type's own properties collides with a property alias
+    ///     already effective on one of its descendants.
+    /// </summary>
+    /// <remarks>
+    ///     Descendants inherit this content type's properties (through tree inheritance and/or composition), so an
+    ///     alias that a descendant already defines would produce a duplicate effective alias on that descendant.
+    ///     <see cref="ValidateProperties"/> only guards the edited type's own model, so this handles the case where
+    ///     the edited type is itself inherited from or composed by others. Collisions caused by a newly selected
+    ///     composition are caught separately by <see cref="ValidateCompositions"/>.
+    /// </remarks>
+    private static ContentTypeOperationStatus ValidateDescendantPropertyAliases(
+        TContentType? contentType,
+        ContentTypeEditingModelBase<TPropertyTypeModel, TPropertyTypeContainer> model,
+        IContentTypeComposition[] allContentTypeCompositions)
+    {
+        HashSet<string> descendantPropertyAliases = contentType.GetPropertyAliasesReservedByDescendants(allContentTypeCompositions);
+
+        var collidesWithDescendants = model.Properties.Select(p => p.Alias).Any(descendantPropertyAliases.InvariantContains);
+
+        return collidesWithDescendants
+            ? ContentTypeOperationStatus.DuplicatePropertyTypeAlias
+            : ContentTypeOperationStatus.Success;
+    }
+
     /// <summary>
     ///     Validates that all containers (property groups/tabs) are properly configured.
     /// </summary>

src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs

@@ -408,30 +408,38 @@
             // property variation change?
             var hasAnyPropertyVariationChanged = contentType.WasPropertyTypeVariationChanged();
 
+            // A composition change dirties the composition collection (add or remove). Both directions change the
+            // set of properties this content type - and every type composed of or inheriting from it - exposes, so
+            // it is a main-impact change that must refresh this type and its descendants. Whether it also needs a
+            // raw cmsContentNu rebuild is decided separately below: an addition on its own does not (its aliases
+            // have no stored values yet), but a removal - or an addition that reintroduces a just-removed alias -
+            // does. See the rawDataAffected calculation.
+            var hasCompositionChanged = dirty.WasPropertyDirty("ContentTypeComposition");
+
             // 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");
+                                                                       || hasAnyCompositionBeenRemoved || hasAnyPropertyBeenRemoved || hasAnyPropertyChangedAlias
+                                                                       || hasCompositionChanged;
 
             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;
+                // The raw cmsContentNu blob is keyed by property alias, so a structural change only requires a
+                // rebuild when it re-keys existing stored values or lets a stale value resolve to a different
+                // property. It does NOT when the only structural change is:
+                //  - a property removal - the orphaned value simply stops resolving and is never read again, or
+                //  - a composition being added - its new aliases have no stored value yet.
+                // In those cases clearing the converted content cache is enough (the content type cache is
+                // refreshed regardless). An alias or variation change, a composition removal, or a property
+                // removal combined with a composition change (which can reintroduce the removed alias behind a
+                // different property type) all re-key or revive stored values and therefore need a rebuild.
+                var rawDataAffected =
+                    hasAliasChanged ||
+                    hasAnyPropertyChangedAlias ||
+                    hasContentTypeVariationChanged ||
+                    hasAnyPropertyVariationChanged ||
+                    hasAnyCompositionBeenRemoved ||
+                    (hasAnyPropertyBeenRemoved && hasCompositionChanged);
+                var rawDataUnaffected = rawDataAffected is false;
 
                 // add that one, as a main change
                 AddChange(changes, contentType, ContentTypeChangeTypes.RefreshMain);