From cdbbd6a921084128d6789beb057104dac6493402 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Mon, 22 Jan 2024 14:53:20 +0100 Subject: [PATCH] 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; }