From b8f5078135c9d77d8842bf8ca9364f5b6326becf Mon Sep 17 00:00:00 2001
From: Ronald Barendse <ronald@barend.se>
Date: Fri, 10 Nov 2023 09:07:10 +0100
Subject: [PATCH] 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
---
 .../Implement/ContentRepositoryBase.cs        | 72 ++++++++++---------
 1 file changed, 40 insertions(+), 32 deletions(-)

diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs
index fde1c6ca05..6df4d66ab5 100644
--- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs
+++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs
@@ -1084,50 +1084,58 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
         {
             // 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
-            var trackedRelations = _dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors);
+            ISet<UmbracoEntityReference> references = _dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors);
 
             // First delete all auto-relations for this entity
-            var relationTypeAliases = _dataValueReferenceFactories.GetAutomaticRelationTypesAliases(entity.Properties, PropertyEditors).ToArray();
-            RelationRepository.DeleteByParent(entity.Id, relationTypeAliases);
+            ISet<string> automaticRelationTypeAliases = _dataValueReferenceFactories.GetAutomaticRelationTypesAliases(entity.Properties, PropertyEditors);
+            RelationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray());
 
-            if (trackedRelations.Count == 0)
+            if (references.Count == 0)
             {
+                // No need to add new references/relations
                 return;
             }
 
-            var udiToGuids = trackedRelations.Select(x => x.Udi as GuidUdi).WhereNotNull().ToDictionary(x => (Udi)x, x => x.Guid);
+            // Lookup node IDs for all GUID based UDIs
+            IEnumerable<Guid> keys = references.Select(x => x.Udi).OfType<GuidUdi>().Select(x => x.Guid);
+            var keysLookup = Database.FetchByGroups<NodeIdKey, Guid>(keys, Constants.Sql.MaxParameterCount, guids =>
+            {
+                return Sql()
+                    .Select<NodeDto>(x => x.NodeId, x => x.UniqueId)
+                    .From<NodeDto>()
+                    .WhereIn<NodeDto>(x => x.UniqueId, guids);
+            }).ToDictionary(x => x.UniqueId, x => x.NodeId);
 
-            // lookup in the DB all INT ids for the GUIDs and chuck into a dictionary
-            var keyToIds = Database.FetchByGroups<NodeIdKey, Guid>(udiToGuids.Values, Constants.Sql.MaxParameterCount, guids => Sql()
-                .Select<NodeDto>(x => x.NodeId, x => x.UniqueId)
-                .From<NodeDto>()
-                .WhereIn<NodeDto>(x => x.UniqueId, guids))
-                .ToDictionary(x => x.UniqueId, x => x.NodeId);
+            // Lookup all relation type IDs
+            var relationTypeLookup = RelationTypeRepository.GetMany(Array.Empty<int>()).ToDictionary(x => x.Alias, x => x.Id);
 
-            var allRelationTypes = RelationTypeRepository.GetMany(Array.Empty<int>()).ToDictionary(x => x.Alias, x => x);
-
-            IEnumerable<ReadOnlyRelation> toSave = trackedRelations.Select(rel =>
+            // Get all valid relations
+            var relations = new List<ReadOnlyRelation>(references.Count);
+            foreach (UmbracoEntityReference reference in references)
+            {
+                if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias))
                 {
-                    if (!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();
+                    // 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));
+                }
+            }
 
             // Save bulk relations
-            RelationRepository.SaveBulk(toSave);
+            RelationRepository.SaveBulk(relations);
         }
 
         /// <summary>