From dff90c6ec07d986f4b11b1f0e4c1f203b65d604d Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Wed, 10 Jan 2024 12:22:36 +0100 Subject: [PATCH 1/6] Run the same cleanup with scaffolding content as when copying. (#15541) * Run the same cleanup with scaffolding content as when copying. - Added a new ContentScaffoldedNotification - Published the notification when a new scaffold has been created from a blueprint (content template) - Linked up the ComplextPEContent handler to do the same cleanup for the new notification as when copying. - registered handlers to the event for blocklist, blockgrid and nested content * PR pattern matching suggestion Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --------- Co-authored-by: Sven Geusens Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --- .../ContentScaffoldedNotification.cs | 18 +++++++++++ .../Notifications/ScaffoldedNotification.cs | 23 ++++++++++++++ .../UmbracoBuilder.CoreServices.cs | 3 ++ ...ropertyEditorContentNotificationHandler.cs | 15 ++++++++- .../Controllers/ContentController.cs | 31 ++++++++++++++----- 5 files changed, 82 insertions(+), 8 deletions(-) create mode 100644 src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs create mode 100644 src/Umbraco.Core/Notifications/ScaffoldedNotification.cs diff --git a/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs b/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs new file mode 100644 index 0000000000..47eda5468d --- /dev/null +++ b/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs @@ -0,0 +1,18 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Core.Notifications; + +/// +/// Notification that is send out when a Content item has been scaffolded from an original item and basic cleaning has been performed +/// +public sealed class ContentScaffoldedNotification : ScaffoldedNotification +{ + public ContentScaffoldedNotification(IContent original, IContent scaffold, int parentId, EventMessages messages) + : base(original, scaffold, parentId, messages) + { + } +} diff --git a/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs b/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs new file mode 100644 index 0000000000..f64bfd3933 --- /dev/null +++ b/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs @@ -0,0 +1,23 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Cms.Core.Events; + +namespace Umbraco.Cms.Core.Notifications; + +public abstract class ScaffoldedNotification : CancelableObjectNotification + where T : class +{ + protected ScaffoldedNotification(T original, T scaffold, int parentId, EventMessages messages) + : base(original, messages) + { + Scaffold = scaffold; + ParentId = parentId; + } + + public T Original => Target; + + public T Scaffold { get; } + + public int ParentId { get; } +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index c65e50024c..bd6f2e9b38 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -358,10 +358,13 @@ public static partial class UmbracoBuilderExtensions builder .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs b/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs index 2b4ac75042..ce867d29e4 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs @@ -10,9 +10,16 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Core.PropertyEditors; +/// +/// Handles nested Udi keys when +/// - saving: Empty keys get generated +/// - copy: keys get replaced by new ones while keeping references intact +/// - scaffolding: keys get replaced by new ones while keeping references intact +/// public abstract class ComplexPropertyEditorContentNotificationHandler : INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationHandler { protected abstract string EditorAlias { get; } @@ -31,6 +38,12 @@ public abstract class ComplexPropertyEditorContentNotificationHandler : } } + public void Handle(ContentScaffoldedNotification notification) + { + IEnumerable props = notification.Scaffold.GetPropertiesByEditor(EditorAlias); + UpdatePropertyValues(props, false); + } + protected abstract string FormatPropertyValue(string rawJson, bool onlyMissingKeys); private void UpdatePropertyValues(IEnumerable props, bool onlyMissingKeys) diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs index 8beec57812..0a1a82b3eb 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs @@ -16,6 +16,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Models.Validation; +using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Routing; @@ -623,17 +624,33 @@ public class ContentController : ContentControllerBase [OutgoingEditorModelEvent] public ActionResult GetEmptyBlueprint(int blueprintId, int parentId) { - IContent? blueprint = _contentService.GetBlueprintById(blueprintId); - if (blueprint == null) + IContent? scaffold; + using (ICoreScope scope = _scopeProvider.CreateCoreScope()) { - return NotFound(); + IContent? blueprint = _contentService.GetBlueprintById(blueprintId); + if (blueprint is null) + { + return NotFound(); + } + scaffold = (IContent)blueprint.DeepClone(); + + scaffold.Id = 0; + scaffold.Name = string.Empty; + scaffold.ParentId = parentId; + + var scaffoldedNotification = new ContentScaffoldedNotification(blueprint, scaffold, parentId, new EventMessages()); + if (scope.Notifications.PublishCancelable(scaffoldedNotification)) + { + scope.Complete(); + return Problem("Scaffolding was cancelled"); + } + + scope.Complete(); } - blueprint.Id = 0; - blueprint.Name = string.Empty; - blueprint.ParentId = parentId; - ContentItemDisplay? mapped = _umbracoMapper.Map(blueprint); + + ContentItemDisplay? mapped = _umbracoMapper.Map(scaffold); if (mapped is not null) { From 57b3a196bf920e380b274746fbb4c17f10a55b30 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> Date: Thu, 11 Jan 2024 12:46:31 +0100 Subject: [PATCH 2/6] V10: Pass in variation context to published cache (#15563) * Make sure that we always have variation context * Fix references --- .../PublishedCache/PublishedCacheBase.cs | 22 ++++++++++++++++--- .../ContentCache.cs | 2 +- .../MediaCache.cs | 2 +- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs b/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs index 3e961ce434..ffb2ef4278 100644 --- a/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs +++ b/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs @@ -1,6 +1,8 @@ using System.Xml.XPath; +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.Xml; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; namespace Umbraco.Cms.Core.PublishedCache; @@ -9,10 +11,24 @@ public abstract class PublishedCacheBase : IPublishedCache { private readonly IVariationContextAccessor? _variationContextAccessor; - public PublishedCacheBase(IVariationContextAccessor variationContextAccessor) => _variationContextAccessor = - variationContextAccessor ?? throw new ArgumentNullException(nameof(variationContextAccessor)); - protected PublishedCacheBase(bool previewDefault) => PreviewDefault = previewDefault; + [Obsolete("Use ctor with all parameters. This will be removed in V15")] + public PublishedCacheBase(IVariationContextAccessor variationContextAccessor) + : this(variationContextAccessor, false) + { + } + + [Obsolete("Use ctor with all parameters. This will be removed in V15")] + protected PublishedCacheBase(bool previewDefault) + : this(StaticServiceProvider.Instance.GetRequiredService(), previewDefault) + { + } + + public PublishedCacheBase(IVariationContextAccessor variationContextAccessor, bool previewDefault) + { + _variationContextAccessor = variationContextAccessor; + PreviewDefault = previewDefault; + } public bool PreviewDefault { get; } diff --git a/src/Umbraco.PublishedCache.NuCache/ContentCache.cs b/src/Umbraco.PublishedCache.NuCache/ContentCache.cs index d8a5c0bc04..1686f6aacf 100644 --- a/src/Umbraco.PublishedCache.NuCache/ContentCache.cs +++ b/src/Umbraco.PublishedCache.NuCache/ContentCache.cs @@ -36,7 +36,7 @@ public class ContentCache : PublishedCacheBase, IPublishedContentCache, INavigab IDomainCache domainCache, IOptions globalSettings, IVariationContextAccessor variationContextAccessor) - : base(previewDefault) + : base(variationContextAccessor, previewDefault) { _snapshot = snapshot; _snapshotCache = snapshotCache; diff --git a/src/Umbraco.PublishedCache.NuCache/MediaCache.cs b/src/Umbraco.PublishedCache.NuCache/MediaCache.cs index 626e2fe36c..68c23c5a34 100644 --- a/src/Umbraco.PublishedCache.NuCache/MediaCache.cs +++ b/src/Umbraco.PublishedCache.NuCache/MediaCache.cs @@ -17,7 +17,7 @@ public class MediaCache : PublishedCacheBase, IPublishedMediaCache, INavigableDa #region Constructors public MediaCache(bool previewDefault, ContentStore.Snapshot snapshot, IVariationContextAccessor variationContextAccessor) - : base(previewDefault) + : base(variationContextAccessor, previewDefault) { _snapshot = snapshot; _variationContextAccessor = variationContextAccessor; From 49f5d2e2d42e71838ca6d055ced6d53a4df41ffe Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Fri, 12 Jan 2024 13:13:20 +0100 Subject: [PATCH 3/6] Updates JSON schema for Umbraco 10 to include details of additional configuration introduced in Forms and Deploy. (#15566) --- src/JsonSchema/JsonSchema.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/JsonSchema/JsonSchema.csproj b/src/JsonSchema/JsonSchema.csproj index 662c35ecb5..9ef072ccdf 100644 --- a/src/JsonSchema/JsonSchema.csproj +++ b/src/JsonSchema/JsonSchema.csproj @@ -13,7 +13,7 @@ - - + + From 5198e7c52d2a9fafb321ee4b8e79783580c0635b Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Fri, 19 Jan 2024 20:02:57 +0100 Subject: [PATCH 4/6] Backport relation tracking fixes and get references from recursive (nested/block) properties (#15593) * Include automatic relation type aliases from factory and fix SQL parameter overflow (#15141) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations (#15160) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations * Always get all automatic relation type aliases * Do not set relation type alias for unknown entity types * Get references from recursive (nested/block) properties --- .../Composing/BuilderCollectionBase.cs | 18 +- .../Composing/IBuilderCollection.cs | 7 +- .../Models/Editors/UmbracoEntityReference.cs | 57 +++++- .../DataValueReferenceFactoryCollection.cs | 174 +++++++++++++----- .../Implement/ContentRepositoryBase.cs | 102 ++++------ .../BlockEditorPropertyValueEditor.cs | 85 ++++----- .../BlockGridPropertyEditorBase.cs | 3 +- .../BlockListPropertyEditorBase.cs | 3 +- .../NestedContentPropertyEditor.cs | 72 +++----- .../DataValueEditorReuseTests.cs | 8 +- ...ataValueReferenceFactoryCollectionTests.cs | 36 +++- 11 files changed, 337 insertions(+), 228 deletions(-) diff --git a/src/Umbraco.Core/Composing/BuilderCollectionBase.cs b/src/Umbraco.Core/Composing/BuilderCollectionBase.cs index ffacd89cff..e92790ab4e 100644 --- a/src/Umbraco.Core/Composing/BuilderCollectionBase.cs +++ b/src/Umbraco.Core/Composing/BuilderCollectionBase.cs @@ -3,30 +3,26 @@ using System.Collections; namespace Umbraco.Cms.Core.Composing; /// -/// Provides a base class for builder collections. +/// Provides a base class for builder collections. /// /// The type of the items. public abstract class BuilderCollectionBase : IBuilderCollection { private readonly LazyReadOnlyCollection _items; - /// Initializes a new instance of the - /// - /// with items. + /// + /// Initializes a new instance of the with items. /// /// The items. - public BuilderCollectionBase(Func> items) => _items = new LazyReadOnlyCollection(items); + public BuilderCollectionBase(Func> items) + => _items = new LazyReadOnlyCollection(items); /// public int Count => _items.Count; - /// - /// Gets an enumerator. - /// + /// public IEnumerator GetEnumerator() => _items.GetEnumerator(); - /// - /// Gets an enumerator. - /// + /// IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } diff --git a/src/Umbraco.Core/Composing/IBuilderCollection.cs b/src/Umbraco.Core/Composing/IBuilderCollection.cs index 56036997bc..c8920149c5 100644 --- a/src/Umbraco.Core/Composing/IBuilderCollection.cs +++ b/src/Umbraco.Core/Composing/IBuilderCollection.cs @@ -1,13 +1,16 @@ namespace Umbraco.Cms.Core.Composing; /// -/// Represents a builder collection, ie an immutable enumeration of items. +/// Represents a builder collection, ie an immutable enumeration of items. /// /// The type of the items. public interface IBuilderCollection : IEnumerable { /// - /// Gets the number of items in the collection. + /// Gets the number of items in the collection. /// + /// + /// The count. + /// int Count { get; } } diff --git a/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs b/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs index c093962408..3f02be10b1 100644 --- a/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs +++ b/src/Umbraco.Core/Models/Editors/UmbracoEntityReference.cs @@ -1,49 +1,88 @@ namespace Umbraco.Cms.Core.Models.Editors; /// -/// Used to track reference to other entities in a property value +/// Used to track a reference to another entity in a property value. /// public struct UmbracoEntityReference : IEquatable { private static readonly UmbracoEntityReference _empty = new(UnknownTypeUdi.Instance, string.Empty); + /// + /// Initializes a new instance of the struct. + /// + /// The UDI. + /// The relation type alias. public UmbracoEntityReference(Udi udi, string relationTypeAlias) { Udi = udi ?? throw new ArgumentNullException(nameof(udi)); RelationTypeAlias = relationTypeAlias ?? throw new ArgumentNullException(nameof(relationTypeAlias)); } + /// + /// Initializes a new instance of the struct for a document or media item. + /// + /// The UDI. public UmbracoEntityReference(Udi udi) { Udi = udi ?? throw new ArgumentNullException(nameof(udi)); switch (udi.EntityType) { + case Constants.UdiEntityType.Document: + RelationTypeAlias = Constants.Conventions.RelationTypes.RelatedDocumentAlias; + break; case Constants.UdiEntityType.Media: RelationTypeAlias = Constants.Conventions.RelationTypes.RelatedMediaAlias; break; default: - RelationTypeAlias = Constants.Conventions.RelationTypes.RelatedDocumentAlias; + // No relation type alias convention for this entity type, so leave it empty + RelationTypeAlias = string.Empty; break; } } + /// + /// Gets the UDI. + /// + /// + /// The UDI. + /// public Udi Udi { get; } - public static UmbracoEntityReference Empty() => _empty; - - public static bool IsEmpty(UmbracoEntityReference reference) => reference == Empty(); - + /// + /// Gets the relation type alias. + /// + /// + /// The relation type alias. + /// public string RelationTypeAlias { get; } - public static bool operator ==(UmbracoEntityReference left, UmbracoEntityReference right) => left.Equals(right); + /// + /// Gets an empty reference. + /// + /// + /// An empty reference. + /// + public static UmbracoEntityReference Empty() => _empty; + /// + /// Determines whether the specified reference is empty. + /// + /// The reference. + /// + /// true if the specified reference is empty; otherwise, false. + /// + public static bool IsEmpty(UmbracoEntityReference reference) => reference == Empty(); + + /// public override bool Equals(object? obj) => obj is UmbracoEntityReference reference && Equals(reference); + /// public bool Equals(UmbracoEntityReference other) => EqualityComparer.Default.Equals(Udi, other.Udi) && RelationTypeAlias == other.RelationTypeAlias; + /// public override int GetHashCode() { var hashCode = -487348478; @@ -52,5 +91,9 @@ public struct UmbracoEntityReference : IEquatable return hashCode; } + /// + public static bool operator ==(UmbracoEntityReference left, UmbracoEntityReference right) => left.Equals(right); + + /// public static bool operator !=(UmbracoEntityReference left, UmbracoEntityReference right) => !(left == right); } diff --git a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs index 24d6f17eb0..c173e766e0 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -4,64 +4,148 @@ using Umbraco.Cms.Core.Models.Editors; namespace Umbraco.Cms.Core.PropertyEditors; +/// +/// Provides a builder collection for items. +/// public class DataValueReferenceFactoryCollection : BuilderCollectionBase { - public DataValueReferenceFactoryCollection(Func> items) - : base(items) - { - } - // TODO: We could further reduce circular dependencies with PropertyEditorCollection by not having IDataValueReference implemented // by property editors and instead just use the already built in IDataValueReferenceFactory and/or refactor that into a more normal collection - public IEnumerable GetAllReferences( - IPropertyCollection properties, - PropertyEditorCollection propertyEditors) - { - var trackedRelations = new HashSet(); - foreach (IProperty p in properties) + /// + /// Initializes a new instance of the class. + /// + /// The items. + public DataValueReferenceFactoryCollection(Func> items) + : base(items) + { } + + /// + /// Gets all unique references from the specified properties. + /// + /// The properties. + /// The property editors. + /// + /// The unique references from the specified properties. + /// + public ISet GetAllReferences(IPropertyCollection properties, PropertyEditorCollection propertyEditors) + { + var references = new HashSet(); + + foreach (IProperty property in properties) { - if (!propertyEditors.TryGet(p.PropertyType.PropertyEditorAlias, out IDataEditor? editor)) + if (!propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) { continue; } - // TODO: We will need to change this once we support tracking via variants/segments - // for now, we are tracking values from ALL variants - foreach (IPropertyValue propertyVal in p.Values) + // Only use edited value for now + references.UnionWith(GetReferences(dataEditor, property.Values.Select(x => x.EditedValue))); + } + + return references; + } + + /// + /// Gets the references. + /// + /// The data editor. + /// The values. + /// + /// The references. + /// + public IEnumerable GetReferences(IDataEditor dataEditor, params object?[] values) + => GetReferences(dataEditor, (IEnumerable)values); + + /// + /// Gets the references. + /// + /// The data editor. + /// The values. + /// + /// The references. + /// + public IEnumerable GetReferences(IDataEditor dataEditor, IEnumerable values) + { + // TODO: We will need to change this once we support tracking via variants/segments + // for now, we are tracking values from ALL variants + if (dataEditor.GetValueEditor() is IDataValueReference dataValueReference) + { + foreach (UmbracoEntityReference reference in values.SelectMany(dataValueReference.GetReferences)) { - var val = propertyVal.EditedValue; - - IDataValueEditor? valueEditor = editor?.GetValueEditor(); - if (valueEditor is IDataValueReference reference) - { - IEnumerable refs = reference.GetReferences(val); - foreach (UmbracoEntityReference r in refs) - { - trackedRelations.Add(r); - } - } - - // Loop over collection that may be add to existing property editors - // implementation of GetReferences in IDataValueReference. - // Allows developers to add support for references by a - // package /property editor that did not implement IDataValueReference themselves - foreach (IDataValueReferenceFactory item in this) - { - // Check if this value reference is for this datatype/editor - // Then call it's GetReferences method - to see if the value stored - // in the dataeditor/property has referecnes to media/content items - if (item.IsForEditor(editor)) - { - foreach (UmbracoEntityReference r in item.GetDataValueReference().GetReferences(val)) - { - trackedRelations.Add(r); - } - } - } + yield return reference; } } - return trackedRelations; + // Loop over collection that may be add to existing property editors + // implementation of GetReferences in IDataValueReference. + // Allows developers to add support for references by a + // package /property editor that did not implement IDataValueReference themselves + foreach (IDataValueReferenceFactory dataValueReferenceFactory in this) + { + // Check if this value reference is for this datatype/editor + // Then call it's GetReferences method - to see if the value stored + // in the dataeditor/property has references to media/content items + if (dataValueReferenceFactory.IsForEditor(dataEditor)) + { + IDataValueReference factoryDataValueReference = dataValueReferenceFactory.GetDataValueReference(); + foreach (UmbracoEntityReference reference in values.SelectMany(factoryDataValueReference.GetReferences)) + { + yield return reference; + } + } + } + } + + /// + /// Gets all relation type aliases that are automatically tracked. + /// + /// The property editors. + /// + /// All relation type aliases that are automatically tracked. + /// + public ISet GetAllAutomaticRelationTypesAliases(PropertyEditorCollection propertyEditors) + { + // Always add default automatic relation types + var automaticRelationTypeAliases = new HashSet(Constants.Conventions.RelationTypes.AutomaticRelationTypes); + + // Add relation types for all property editors + foreach (IDataEditor dataEditor in propertyEditors) + { + automaticRelationTypeAliases.UnionWith(GetAutomaticRelationTypesAliases(dataEditor)); + } + + return automaticRelationTypeAliases; + } + + /// + /// Gets the automatic relation types aliases. + /// + /// The data editor. + /// + /// The automatic relation types aliases. + /// + public IEnumerable GetAutomaticRelationTypesAliases(IDataEditor dataEditor) + { + if (dataEditor.GetValueEditor() is IDataValueReference dataValueReference) + { + // Return custom relation types from value editor implementation + foreach (var alias in dataValueReference.GetAutomaticRelationTypesAliases()) + { + yield return alias; + } + } + + foreach (IDataValueReferenceFactory dataValueReferenceFactory in this) + { + if (dataValueReferenceFactory.IsForEditor(dataEditor)) + { + // Return custom relation types from factory + foreach (var alias in dataValueReferenceFactory.GetDataValueReference().GetAutomaticRelationTypesAliases()) + { + yield return alias; + } + } + } } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs index de247a6120..6569ad4d89 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -1083,81 +1083,59 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement protected void PersistRelations(TEntity entity) { // Get all references from our core built in DataEditors/Property Editors - // Along with seeing if deverlopers want to collect additional references from the DataValueReferenceFactories collection - var trackedRelations = new List(); - trackedRelations.AddRange(_dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors)); - - var relationTypeAliases = GetAutomaticRelationTypesAliases(entity.Properties, PropertyEditors).ToArray(); + // Along with seeing if developers want to collect additional references from the DataValueReferenceFactories collection + ISet references = _dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors); // First delete all auto-relations for this entity - RelationRepository.DeleteByParent(entity.Id, relationTypeAliases); + ISet automaticRelationTypeAliases = _dataValueReferenceFactories.GetAllAutomaticRelationTypesAliases(PropertyEditors); + RelationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray()); - if (trackedRelations.Count == 0) + if (references.Count == 0) { + // No need to add new references/relations return; } - trackedRelations = trackedRelations.Distinct().ToList(); - var udiToGuids = trackedRelations.Select(x => x.Udi as GuidUdi) - .ToDictionary(x => (Udi)x!, x => x!.Guid); - - // lookup in the DB all INT ids for the GUIDs and chuck into a dictionary - var keyToIds = Database.Fetch(Sql() - .Select(x => x.NodeId, x => x.UniqueId) - .From() - .WhereIn(x => x.UniqueId, udiToGuids.Values)) - .ToDictionary(x => x.UniqueId, x => x.NodeId); - - var allRelationTypes = RelationTypeRepository.GetMany(Array.Empty())? - .ToDictionary(x => x.Alias, x => x); - - IEnumerable toSave = trackedRelations.Select(rel => - { - if (allRelationTypes is null || !allRelationTypes.TryGetValue(rel.RelationTypeAlias, out IRelationType? relationType)) - { - throw new InvalidOperationException($"The relation type {rel.RelationTypeAlias} does not exist"); - } - - if (!udiToGuids.TryGetValue(rel.Udi, out Guid guid)) - { - return null; // This shouldn't happen! - } - - if (!keyToIds.TryGetValue(guid, out var id)) - { - return null; // This shouldn't happen! - } - - return new ReadOnlyRelation(entity.Id, id, relationType.Id); - }).WhereNotNull(); - - // Save bulk relations - RelationRepository.SaveBulk(toSave); - } - - private IEnumerable GetAutomaticRelationTypesAliases( - IPropertyCollection properties, - PropertyEditorCollection propertyEditors) - { - var automaticRelationTypesAliases = new HashSet(Constants.Conventions.RelationTypes.AutomaticRelationTypes); - - foreach (IProperty property in properties) + // Lookup node IDs for all GUID based UDIs + IEnumerable keys = references.Select(x => x.Udi).OfType().Select(x => x.Guid); + var keysLookup = Database.FetchByGroups(keys, Constants.Sql.MaxParameterCount, guids => { - if (propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out IDataEditor? editor) is false ) - { - continue; - } + return Sql() + .Select(x => x.NodeId, x => x.UniqueId) + .From() + .WhereIn(x => x.UniqueId, guids); + }).ToDictionary(x => x.UniqueId, x => x.NodeId); - if (editor.GetValueEditor() is IDataValueReference reference) + // Lookup all relation type IDs + var relationTypeLookup = RelationTypeRepository.GetMany(Array.Empty()).ToDictionary(x => x.Alias, x => x.Id); + + // Get all valid relations + var relations = new List(references.Count); + foreach (UmbracoEntityReference reference in references) + { + if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias)) { - foreach (var alias in reference.GetAutomaticRelationTypesAliases()) - { - automaticRelationTypesAliases.Add(alias); - } + // Returning a reference that doesn't use an automatic relation type is an issue that should be fixed in code + Logger.LogError("The reference to {Udi} uses a relation type {RelationTypeAlias} that is not an automatic relation type.", reference.Udi, reference.RelationTypeAlias); + } + else if (!relationTypeLookup.TryGetValue(reference.RelationTypeAlias, out int relationTypeId)) + { + // A non-existent relation type could be caused by an environment issue (e.g. it was manually removed) + Logger.LogWarning("The reference to {Udi} uses a relation type {RelationTypeAlias} that does not exist.", reference.Udi, reference.RelationTypeAlias); + } + else if (reference.Udi is not GuidUdi udi || !keysLookup.TryGetValue(udi.Guid, out var id)) + { + // Relations only support references to items that are stored in the NodeDto table (because of foreign key constraints) + Logger.LogInformation("The reference to {Udi} can not be saved as relation, because doesn't have a node ID.", reference.Udi); + } + else + { + relations.Add(new ReadOnlyRelation(entity.Id, id, relationTypeId)); } } - return automaticRelationTypesAliases; + // Save bulk relations + RelationRepository.SaveBulk(relations); } /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs index fbf2239828..9739553b99 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs @@ -17,12 +17,14 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV { private BlockEditorValues? _blockEditorValues; private readonly IDataTypeService _dataTypeService; - private readonly ILogger _logger; private readonly PropertyEditorCollection _propertyEditors; + private readonly DataValueReferenceFactoryCollection _dataValueReferenceFactories; + private readonly ILogger _logger; protected BlockEditorPropertyValueEditor( DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, ILocalizedTextService textService, ILogger logger, @@ -32,6 +34,7 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV : base(textService, shortStringHelper, jsonSerializer, ioHelper, attribute) { _propertyEditors = propertyEditors; + _dataValueReferenceFactories = dataValueReferenceFactories; _dataTypeService = dataTypeService; _logger = logger; } @@ -42,73 +45,58 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV set => _blockEditorValues = value; } + /// public IEnumerable GetReferences(object? value) { - var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); - - var result = new List(); - BlockEditorData? blockEditorData = BlockEditorValues.DeserializeAndClean(rawJson); - if (blockEditorData == null) + foreach (BlockItemData.BlockPropertyValue propertyValue in GetAllPropertyValues(value)) { - return Enumerable.Empty(); - } - - // loop through all content and settings data - foreach (BlockItemData row in blockEditorData.BlockValue.ContentData.Concat(blockEditorData.BlockValue.SettingsData)) - { - foreach (KeyValuePair prop in row.PropertyValues) + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + continue; + } - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (!(valueEditor is IDataValueReference reference)) - { - continue; - } - - var val = prop.Value.Value?.ToString(); - - IEnumerable refs = reference.GetReferences(val); - - result.AddRange(refs); + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + { + yield return reference; } } - - return result; } /// public IEnumerable GetTags(object? value, object? dataTypeConfiguration, int? languageId) + { + foreach (BlockItemData.BlockPropertyValue propertyValue in GetAllPropertyValues(value)) + { + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor) || + dataEditor.GetValueEditor() is not IDataValueTags dataValueTags) + { + continue; + } + + object? configuration = _dataTypeService.GetDataType(propertyValue.PropertyType.DataTypeKey)?.Configuration; + foreach (ITag tag in dataValueTags.GetTags(propertyValue.Value, configuration, languageId)) + { + yield return tag; + } + } + } + + private IEnumerable GetAllPropertyValues(object? value) { var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); BlockEditorData? blockEditorData = BlockEditorValues.DeserializeAndClean(rawJson); - if (blockEditorData == null) + if (blockEditorData is null) { - return Enumerable.Empty(); + yield break; } - var result = new List(); - // loop through all content and settings data - foreach (BlockItemData row in blockEditorData.BlockValue.ContentData.Concat(blockEditorData.BlockValue.SettingsData)) + // Return all property values from the content and settings data + IEnumerable data = blockEditorData.BlockValue.ContentData.Concat(blockEditorData.BlockValue.SettingsData); + foreach (BlockItemData.BlockPropertyValue propertyValue in data.SelectMany(x => x.PropertyValues.Select(x => x.Value))) { - foreach (KeyValuePair prop in row.PropertyValues) - { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; - - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (valueEditor is not IDataValueTags tagsProvider) - { - continue; - } - - object? configuration = _dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeKey)?.Configuration; - - result.AddRange(tagsProvider.GetTags(prop.Value.Value, configuration, languageId)); - } + yield return propertyValue; } - - return result; } #region Convert database // editor @@ -119,7 +107,6 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV /// Ensure that sub-editor values are translated through their ToEditor methods /// /// - /// /// /// /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs index 73b767bd4f..7e67c2d8a7 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs @@ -50,6 +50,7 @@ public abstract class BlockGridPropertyEditorBase : DataEditor public BlockGridEditorPropertyValueEditor( DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, ILocalizedTextService textService, ILogger logger, @@ -58,7 +59,7 @@ public abstract class BlockGridPropertyEditorBase : DataEditor IIOHelper ioHelper, IContentTypeService contentTypeService, IPropertyValidationService propertyValidationService) - : base(attribute, propertyEditors, dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) + : base(attribute, propertyEditors, dataValueReferenceFactories, dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) { BlockEditorValues = new BlockEditorValues(new BlockGridEditorDataConverter(jsonSerializer), contentTypeService, logger); Validators.Add(new BlockEditorValidator(propertyValidationService, BlockEditorValues, contentTypeService)); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs index 194383560e..d59af3817c 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs @@ -46,6 +46,7 @@ public abstract class BlockListPropertyEditorBase : DataEditor public BlockListEditorPropertyValueEditor( DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, IContentTypeService contentTypeService, ILocalizedTextService textService, @@ -54,7 +55,7 @@ public abstract class BlockListPropertyEditorBase : DataEditor IJsonSerializer jsonSerializer, IIOHelper ioHelper, IPropertyValidationService propertyValidationService) : - base(attribute, propertyEditors, dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) + base(attribute, propertyEditors, dataValueReferenceFactories,dataTypeService, textService, logger, shortStringHelper, jsonSerializer, ioHelper) { BlockEditorValues = new BlockEditorValues(new BlockListEditorDataConverter(), contentTypeService, logger); Validators.Add(new BlockEditorValidator(propertyValidationService, BlockEditorValues, contentTypeService)); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs index d64df34aa4..e34de1a3ad 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs @@ -89,9 +89,10 @@ public class NestedContentPropertyEditor : DataEditor internal class NestedContentPropertyValueEditor : DataValueEditor, IDataValueReference, IDataValueTags { private readonly IDataTypeService _dataTypeService; + private readonly PropertyEditorCollection _propertyEditors; + private readonly DataValueReferenceFactoryCollection _dataValueReferenceFactories; private readonly ILogger _logger; private readonly NestedContentValues _nestedContentValues; - private readonly PropertyEditorCollection _propertyEditors; public NestedContentPropertyValueEditor( IDataTypeService dataTypeService, @@ -100,16 +101,19 @@ public class NestedContentPropertyEditor : DataEditor IShortStringHelper shortStringHelper, DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, + DataValueReferenceFactoryCollection dataValueReferenceFactories, ILogger logger, IJsonSerializer jsonSerializer, IIOHelper ioHelper, IPropertyValidationService propertyValidationService) : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) { - _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; + _propertyEditors = propertyEditors; + _dataValueReferenceFactories = dataValueReferenceFactories; _logger = logger; _nestedContentValues = new NestedContentValues(contentTypeService); + Validators.Add(new NestedContentValidator(propertyValidationService, _nestedContentValues, contentTypeService)); } @@ -137,66 +141,45 @@ public class NestedContentPropertyEditor : DataEditor } } + /// public IEnumerable GetReferences(object? value) { - var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); - - var result = new List(); - - foreach (NestedContentValues.NestedContentRowValue row in _nestedContentValues.GetPropertyValues(rawJson)) + foreach (NestedContentValues.NestedContentPropertyValue propertyValue in GetAllPropertyValues(value)) { - foreach (KeyValuePair prop in - row.PropertyValues) + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + continue; + } - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (!(valueEditor is IDataValueReference reference)) - { - continue; - } - - var val = prop.Value.Value?.ToString(); - - IEnumerable refs = reference.GetReferences(val); - - result.AddRange(refs); + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + { + yield return reference; } } - - return result; } /// public IEnumerable GetTags(object? value, object? dataTypeConfiguration, int? languageId) { - IReadOnlyList rows = - _nestedContentValues.GetPropertyValues(value); - - var result = new List(); - - foreach (NestedContentValues.NestedContentRowValue row in rows.ToList()) + foreach (NestedContentValues.NestedContentPropertyValue propertyValue in GetAllPropertyValues(value)) { - foreach (KeyValuePair prop in row.PropertyValues - .ToList()) + if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor) || + dataEditor.GetValueEditor() is not IDataValueTags dataValueTags) { - IDataEditor? propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; + continue; + } - IDataValueEditor? valueEditor = propEditor?.GetValueEditor(); - if (valueEditor is not IDataValueTags tagsProvider) - { - continue; - } - - object? configuration = _dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeKey)?.Configuration; - - result.AddRange(tagsProvider.GetTags(prop.Value.Value, configuration, languageId)); + object? configuration = _dataTypeService.GetDataType(propertyValue.PropertyType.DataTypeKey)?.Configuration; + foreach (ITag tag in dataValueTags.GetTags(propertyValue.Value, configuration, languageId)) + { + yield return tag; } } - - return result; } + private IEnumerable GetAllPropertyValues(object? value) + => _nestedContentValues.GetPropertyValues(value).SelectMany(x => x.PropertyValues.Values); + #region DB to String public override string ConvertDbToString(IPropertyType propertyType, object? propertyValue) @@ -422,7 +405,8 @@ public class NestedContentPropertyEditor : DataEditor // set values to null row.PropertyValues[elementTypeProp.Alias] = new NestedContentValues.NestedContentPropertyValue { - PropertyType = elementTypeProp, Value = null, + PropertyType = elementTypeProp, + Value = null, }; row.RawPropertyValues[elementTypeProp.Alias] = null; } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs index d88a9689ab..75d4f0d509 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs @@ -1,4 +1,3 @@ -using System.Linq; using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; @@ -15,6 +14,7 @@ public class DataValueEditorReuseTests { private Mock _dataValueEditorFactoryMock; private PropertyEditorCollection _propertyEditorCollection; + private DataValueReferenceFactoryCollection _dataValueReferenceFactories; [SetUp] public void SetUp() @@ -31,6 +31,7 @@ public class DataValueEditorReuseTests Mock.Of())); _propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)); + _dataValueReferenceFactories = new DataValueReferenceFactoryCollection(Enumerable.Empty); _dataValueEditorFactoryMock .Setup(m => @@ -38,6 +39,7 @@ public class DataValueEditorReuseTests .Returns(() => new BlockListPropertyEditorBase.BlockListEditorPropertyValueEditor( new DataEditorAttribute("a", "b", "c"), _propertyEditorCollection, + _dataValueReferenceFactories, Mock.Of(), Mock.Of(), Mock.Of(), @@ -93,7 +95,7 @@ public class DataValueEditorReuseTests { var blockListPropertyEditor = new BlockListPropertyEditor( _dataValueEditorFactoryMock.Object, - new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)), + _propertyEditorCollection, Mock.Of(), Mock.Of(), Mock.Of()); @@ -114,7 +116,7 @@ public class DataValueEditorReuseTests { var blockListPropertyEditor = new BlockListPropertyEditor( _dataValueEditorFactoryMock.Object, - new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)), + _propertyEditorCollection, Mock.Of(), Mock.Of(), Mock.Of()); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs index 46ed8af979..38fc5125dc 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs @@ -1,9 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System; -using System.Collections.Generic; -using System.Linq; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; @@ -174,6 +171,33 @@ public class DataValueReferenceFactoryCollectionTests Assert.AreEqual(trackedUdi4, result.ElementAt(1).Udi.ToString()); } + [Test] + public void GetAutomaticRelationTypesAliases_ContainsDefault() + { + var collection = new DataValueReferenceFactoryCollection(Enumerable.Empty); + var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)); + + var result = collection.GetAllAutomaticRelationTypesAliases(propertyEditors).ToArray(); + + var expected = Constants.Conventions.RelationTypes.AutomaticRelationTypes; + CollectionAssert.AreEquivalent(expected, result, "Result does not contain the expected relation type aliases."); + } + + [Test] + public void GetAutomaticRelationTypesAliases_ContainsCustom() + { + var collection = new DataValueReferenceFactoryCollection(() => new TestDataValueReferenceFactory().Yield()); + + var labelPropertyEditor = new LabelPropertyEditor(DataValueEditorFactory, IOHelper, EditorConfigurationParser); + var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(() => labelPropertyEditor.Yield())); + var serializer = new ConfigurationEditorJsonSerializer(); + + var result = collection.GetAllAutomaticRelationTypesAliases(propertyEditors).ToArray(); + + var expected = Constants.Conventions.RelationTypes.AutomaticRelationTypes.Append("umbTest"); + CollectionAssert.AreEquivalent(expected, result, "Result does not contain the expected relation type aliases."); + } + private class TestDataValueReferenceFactory : IDataValueReferenceFactory { public IDataValueReference GetDataValueReference() => new TestMediaDataValueReference(); @@ -197,6 +221,12 @@ public class DataValueReferenceFactoryCollectionTests yield return new UmbracoEntityReference(udi); } } + + public IEnumerable GetAutomaticRelationTypesAliases() => new[] + { + "umbTest", + "umbTest", // Duplicate on purpose to test distinct aliases + }; } } } From bee20b525fd6fa9311f7994a41db33420c85ffb9 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Sat, 20 Jan 2024 11:26:40 +0100 Subject: [PATCH 5/6] Post merge fix --- tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj b/tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj index 9e70ea8382..8b66986898 100644 --- a/tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj +++ b/tools/Umbraco.JsonSchema/Umbraco.JsonSchema.csproj @@ -13,8 +13,5 @@ - - - From cdbbd6a921084128d6789beb057104dac6493402 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Mon, 22 Jan 2024 14:53:20 +0100 Subject: [PATCH 6/6] Optimize relation tracking for adding new and keeping existing relations (#15596) * Include automatic relation type aliases from factory and fix SQL parameter overflow (#15141) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations (#15160) * Include automatic relation type aliases from factory * Remove unnessecary distinct and fix SQL parameter overflow issue * Fixed assertions and test distinct aliases * Simplified collection assertions * Improve logging of invalid reference relations * Always get all automatic relation type aliases * Do not set relation type alias for unknown entity types * Get references from recursive (nested/block) properties * Optimize relation tracking for adding new and keeping existing relations * Optimize getting references by grouping by property editor alias and avoiding duplicate parsing of the same value --- .../DataValueReferenceFactoryCollection.cs | 16 +++++-- .../Implement/ContentRepositoryBase.cs | 44 +++++++++++++------ .../BlockEditorPropertyValueEditor.cs | 8 ++-- .../NestedContentPropertyEditor.cs | 8 ++-- 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs index c173e766e0..e15f581809 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -32,15 +32,23 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase(); - foreach (IProperty property in properties) + // Group by property editor alias to avoid duplicate lookups and optimize value parsing + foreach (var propertyValuesByPropertyEditorAlias in properties.GroupBy(x => x.PropertyType.PropertyEditorAlias, x => x.Values)) { - if (!propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) + if (!propertyEditors.TryGet(propertyValuesByPropertyEditorAlias.Key, out IDataEditor? dataEditor)) { continue; } - // Only use edited value for now - references.UnionWith(GetReferences(dataEditor, property.Values.Select(x => x.EditedValue))); + // Use distinct values to avoid duplicate parsing of the same value + var values = new HashSet(properties.Count); + foreach (IPropertyValue propertyValue in propertyValuesByPropertyEditorAlias.SelectMany(x => x)) + { + values.Add(propertyValue.EditedValue); + values.Add(propertyValue.PublishedValue); + } + + references.UnionWith(GetReferences(dataEditor, values)); } return references; diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 6569ad4d89..56c356e7b4 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -1082,20 +1082,24 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement protected void PersistRelations(TEntity entity) { - // Get all references from our core built in DataEditors/Property Editors - // Along with seeing if developers want to collect additional references from the DataValueReferenceFactories collection + // Get all references and automatic relation type aliases ISet references = _dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors); - - // First delete all auto-relations for this entity ISet automaticRelationTypeAliases = _dataValueReferenceFactories.GetAllAutomaticRelationTypesAliases(PropertyEditors); - RelationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray()); if (references.Count == 0) { + // Delete all relations using the automatic relation type aliases + RelationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray()); + // No need to add new references/relations return; } + // Lookup all relation type IDs + var relationTypeLookup = RelationTypeRepository.GetMany(Array.Empty()) + .Where(x => automaticRelationTypeAliases.Contains(x.Alias)) + .ToDictionary(x => x.Alias, x => x.Id); + // Lookup node IDs for all GUID based UDIs IEnumerable keys = references.Select(x => x.Udi).OfType().Select(x => x.Guid); var keysLookup = Database.FetchByGroups(keys, Constants.Sql.MaxParameterCount, guids => @@ -1106,14 +1110,16 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement .WhereIn(x => x.UniqueId, guids); }).ToDictionary(x => x.UniqueId, x => x.NodeId); - // Lookup all relation type IDs - var relationTypeLookup = RelationTypeRepository.GetMany(Array.Empty()).ToDictionary(x => x.Alias, x => x.Id); - // Get all valid relations - var relations = new List(references.Count); + var relations = new List<(int ChildId, int RelationTypeId)>(references.Count); foreach (UmbracoEntityReference reference in references) { - if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias)) + if (string.IsNullOrEmpty(reference.RelationTypeAlias)) + { + // Reference does not specify a relation type alias, so skip adding a relation + Logger.LogDebug("The reference to {Udi} does not specify a relation type alias, so it will not be saved as relation.", reference.Udi); + } + else if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias)) { // Returning a reference that doesn't use an automatic relation type is an issue that should be fixed in code Logger.LogError("The reference to {Udi} uses a relation type {RelationTypeAlias} that is not an automatic relation type.", reference.Udi, reference.RelationTypeAlias); @@ -1130,12 +1136,24 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement } else { - relations.Add(new ReadOnlyRelation(entity.Id, id, relationTypeId)); + relations.Add((id, relationTypeId)); } } - // Save bulk relations - RelationRepository.SaveBulk(relations); + // Get all existing relations (optimize for adding new and keeping existing relations) + var query = Query().Where(x => x.ParentId == entity.Id).WhereIn(x => x.RelationTypeId, relationTypeLookup.Values); + var existingRelations = RelationRepository.GetPagedRelationsByQuery(query, 0, int.MaxValue, out _, null) + .ToDictionary(x => (x.ChildId, x.RelationTypeId)); // Relations are unique by parent ID, child ID and relation type ID + + // Add relations that don't exist yet + var relationsToAdd = relations.Except(existingRelations.Keys).Select(x => new ReadOnlyRelation(entity.Id, x.ChildId, x.RelationTypeId)); + RelationRepository.SaveBulk(relationsToAdd); + + // Delete relations that don't exist anymore + foreach (IRelation relation in existingRelations.Where(x => !relations.Contains(x.Key)).Select(x => x.Value)) + { + RelationRepository.Delete(relation); + } } /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs index 9739553b99..c281b0758e 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs @@ -48,14 +48,16 @@ internal abstract class BlockEditorPropertyValueEditor : DataValueEditor, IDataV /// public IEnumerable GetReferences(object? value) { - foreach (BlockItemData.BlockPropertyValue propertyValue in GetAllPropertyValues(value)) + // Group by property editor alias to avoid duplicate lookups and optimize value parsing + foreach (var valuesByPropertyEditorAlias in GetAllPropertyValues(value).GroupBy(x => x.PropertyType.PropertyEditorAlias, x => x.Value)) { - if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) + if (!_propertyEditors.TryGet(valuesByPropertyEditorAlias.Key, out IDataEditor? dataEditor)) { continue; } - foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + // Use distinct values to avoid duplicate parsing of the same value + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, valuesByPropertyEditorAlias.Distinct())) { yield return reference; } diff --git a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs index e34de1a3ad..6f30605c20 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/NestedContentPropertyEditor.cs @@ -144,14 +144,16 @@ public class NestedContentPropertyEditor : DataEditor /// public IEnumerable GetReferences(object? value) { - foreach (NestedContentValues.NestedContentPropertyValue propertyValue in GetAllPropertyValues(value)) + // Group by property editor alias to avoid duplicate lookups and optimize value parsing + foreach (var valuesByPropertyEditorAlias in GetAllPropertyValues(value).GroupBy(x => x.PropertyType.PropertyEditorAlias, x => x.Value)) { - if (!_propertyEditors.TryGet(propertyValue.PropertyType.PropertyEditorAlias, out IDataEditor? dataEditor)) + if (!_propertyEditors.TryGet(valuesByPropertyEditorAlias.Key, out IDataEditor? dataEditor)) { continue; } - foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, propertyValue.Value)) + // Use distinct values to avoid duplicate parsing of the same value + foreach (UmbracoEntityReference reference in _dataValueReferenceFactories.GetReferences(dataEditor, valuesByPropertyEditorAlias.Distinct())) { yield return reference; }