fix: construction of relationship dictionaries for root layer

Author: maurei

src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs

@@ -22,6 +22,10 @@ public interface IRelationshipsDictionary { }
     public interface IByAffectedRelationships<TDependentResource> : 
         IRelationshipGetters<TDependentResource> where TDependentResource : class, IIdentifiable
     {
+        /// todo: expose getters that behave something like this:
+        /// relationshipDictionary.GetAffected( entity => entity.NavigationProperty ).
+        /// see https://stackoverflow.com/a/17116267/4441216
+
         /// <summary>
         /// Gets a dictionary of affected resources grouped by affected relationships.
         /// </summary>
@@ -39,16 +43,16 @@ public interface IRelationshipsDictionary<TDependentResource> :
     /// <summary>
     /// A helper class that provides insights in which relationships have been updated for which entities.
     /// </summary>
-    public interface IRelationshipGetters<TDependentResource> where TDependentResource : class, IIdentifiable
+    public interface IRelationshipGetters<TResource> where TResource : class, IIdentifiable
     {
         /// <summary>
         /// Gets a dictionary of all entities that have an affected relationship to type <typeparamref name="TPrincipalResource"/>
         /// </summary>
-        Dictionary<RelationshipAttribute, HashSet<TDependentResource>> GetByRelationship<TPrincipalResource>() where TPrincipalResource : class, IIdentifiable;
+        Dictionary<RelationshipAttribute, HashSet<TResource>> GetByRelationship<TRelatedResource>() where TRelatedResource : class, IIdentifiable;
         /// <summary>
         /// Gets a dictionary of all entities that have an affected relationship to type <paramref name="principalType"/>
         /// </summary>
-        Dictionary<RelationshipAttribute, HashSet<TDependentResource>> GetByRelationship(Type principalType);
+        Dictionary<RelationshipAttribute, HashSet<TResource>> GetByRelationship(Type relatedResourceType);
     }
 
     /// <summary>
@@ -82,7 +86,7 @@ public Dictionary<RelationshipAttribute, HashSet<TResource>> GetByRelationship<T
         /// <inheritdoc />
         public Dictionary<RelationshipAttribute, HashSet<TResource>> GetByRelationship(Type relatedType)
         {
-            return this.Where(p => p.Key.PrincipalType == relatedType).ToDictionary(p => p.Key, p => p.Value);
+            return this.Where(p => p.Key.DependentType == relatedType).ToDictionary(p => p.Key, p => p.Value);
         }
     }
 }

src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs

@@ -57,7 +57,7 @@ public RootNode<TEntity> CreateRootNode<TEntity>(IEnumerable<TEntity> rootEntiti
             _processedEntities = new Dictionary<DependentType, HashSet<IIdentifiable>>();
             RegisterRelationshipProxies(typeof(TEntity));
             var uniqueEntities = ProcessEntities(rootEntities);
-            var relationshipsToNextLayer = GetRelationships(typeof(TEntity));
+            var relationshipsToNextLayer = GetPopulatedRelationships(typeof(TEntity), uniqueEntities.Cast<IIdentifiable>());
             return new RootNode<TEntity>(uniqueEntities, relationshipsToNextLayer);
         }
 
@@ -80,30 +80,38 @@ public EntityChildLayer CreateNextLayer(IEnumerable<IEntityNode> nodes)
         {
             /// first extract entities by parsing populated relationships in the entities
             /// of previous layer
-            (var dependents, var principals) = ExtractEntities(nodes);
+            (var principals, var dependents) = ExtractEntities(nodes);
 
-            /// group them conveniently so we can make ChildNodes of them
-            var dependentsGrouped = GroupByDependentTypeOfRelationship(dependents);
+            /// group them conveniently so we can make ChildNodes of them:
+            /// there might be several relationship attributes in dependents dictionary
+            /// that point to the same dependent type. 
+            var principalsGrouped = GroupByDependentTypeOfRelationship(principals);
 
             /// convert the groups into child nodes
-            var nextNodes = dependentsGrouped.Select(entry =>
+            var nextNodes = principalsGrouped.Select(entry =>
             {
                 var nextNodeType = entry.Key;
+                var populatedRelationships = new List<RelationshipProxy>();
                 var relationshipsToPreviousLayer = entry.Value.Select(grouped =>
                 {
                     var proxy = grouped.Key;
-                    return CreateRelationsipGroupInstance(nextNodeType, proxy, grouped.Value, principals[proxy]);
+                    populatedRelationships.AddRange(GetPopulatedRelationships(nextNodeType, dependents[proxy]));
+                    return CreateRelationshipGroupInstance(nextNodeType, proxy, grouped.Value, dependents[proxy]);
                 }).ToList();
 
                 RegisterRelationshipProxies(nextNodeType);
-                return CreateNodeInstance(nextNodeType, GetRelationships(nextNodeType), relationshipsToPreviousLayer);
+                return CreateNodeInstance(nextNodeType, populatedRelationships.ToArray(), relationshipsToPreviousLayer);
             }).ToList();
 
             /// wrap the child nodes in a EntityChildLayer
             return new EntityChildLayer(nextNodes);
         }
 
 
+        /// <summary>
+        /// iterates throug the <paramref name="relationships"/> dictinary and groups the values 
+        /// by matching dependent type of the keys (which are relationshipattributes)
+        /// </summary>
         Dictionary<DependentType, List<KeyValuePair<RelationshipProxy, List<IIdentifiable>>>> GroupByDependentTypeOfRelationship(Dictionary<RelationshipProxy, List<IIdentifiable>> relationships)
         {
             return relationships.GroupBy(kvp => kvp.Key.DependentType).ToDictionary(gdc => gdc.Key, gdc => gdc.ToList());
@@ -115,11 +123,8 @@ Dictionary<DependentType, List<KeyValuePair<RelationshipProxy, List<IIdentifiabl
         /// </summary>
         (Dictionary<RelationshipProxy, List<IIdentifiable>>, Dictionary<RelationshipProxy, List<IIdentifiable>>) ExtractEntities(IEnumerable<IEntityNode> principalNodes)
         {
-            var currentLayerEntities = new List<IIdentifiable>();
-            var principalsGrouped = new Dictionary<RelationshipProxy, List<IIdentifiable>>();
-            var dependentsGrouped = new Dictionary<RelationshipProxy, List<IIdentifiable>>();
-
-            principalNodes.ForEach(n => RegisterRelationshipProxies(n.EntityType));
+            var principalsEntitiesGrouped = new Dictionary<RelationshipProxy, List<IIdentifiable>>();  // RelationshipAttr_prevlayer->currentlayer  => prevLayerEntities
+            var dependentsEntitiesGrouped = new Dictionary<RelationshipProxy, List<IIdentifiable>>(); // RelationshipAttr_prevlayer->currentlayer   => currentLayerEntities
 
             foreach (var node in principalNodes)
             {
@@ -141,36 +146,36 @@ Dictionary<DependentType, List<KeyValuePair<RelationshipProxy, List<IIdentifiabl
                             dependentEntities = list;
                         }
 
-                        var newDependentEntities = UniqueInTree(dependentEntities.Cast<IIdentifiable>(), proxy.DependentType);
-                        if (proxy.IsContextRelation || newDependentEntities.Any())
+                        var uniqueDependentEntities = UniqueInTree(dependentEntities.Cast<IIdentifiable>(), proxy.DependentType);
+                        if (proxy.IsContextRelation || uniqueDependentEntities.Any())
                         {
-                            currentLayerEntities.AddRange(newDependentEntities);
-                            AddToRelationshipGroup(dependentsGrouped, proxy, newDependentEntities); // TODO check if this needs to be newDependentEntities or just dependentEntities
-                            AddToRelationshipGroup(principalsGrouped, proxy, new IIdentifiable[] { principalEntity });
+                            AddToRelationshipGroup(dependentsEntitiesGrouped, proxy, uniqueDependentEntities);
+                            AddToRelationshipGroup(principalsEntitiesGrouped, proxy, new IIdentifiable[] { principalEntity });
                         }
                     }
                 }
             }
 
-            var processEntities = GetType().GetMethod(nameof(ProcessEntities), BindingFlags.NonPublic | BindingFlags.Instance);
-            foreach (var kvp in dependentsGrouped)
+            var processEntitiesMethod = GetType().GetMethod(nameof(ProcessEntities), BindingFlags.NonPublic | BindingFlags.Instance);
+            foreach (var kvp in dependentsEntitiesGrouped)
             {
                 var type = kvp.Key.DependentType;
                 var list = kvp.Value.Cast(type);
-                processEntities.MakeGenericMethod(type).Invoke(this, new object[] { list });
+                processEntitiesMethod.MakeGenericMethod(type).Invoke(this, new object[] { list });
             }
 
-            return (principalsGrouped, dependentsGrouped);
+            return (principalsEntitiesGrouped, dependentsEntitiesGrouped);
         }
 
         /// <summary>
-        /// Get all relationships known in the current tree traversal from a 
+        /// Get all populated relationships known in the current tree traversal from a 
         /// principal type to any dependent type
         /// </summary>
         /// <returns>The relationships.</returns>
-        RelationshipProxy[] GetRelationships(PrincipalType principal)
+        RelationshipProxy[] GetPopulatedRelationships(PrincipalType principalType, IEnumerable<IIdentifiable> principals)
         {
-            return RelationshipProxies.Select(entry => entry.Value).Where(proxy => proxy.PrincipalType == principal).ToArray();
+            var relationshipsFromPrincipalToDependent = RelationshipProxies.Select(entry => entry.Value).Where(proxy => proxy.PrincipalType == principalType);
+            return relationshipsFromPrincipalToDependent.Where(proxy => proxy.IsContextRelation || principals.Any(p => proxy.GetValue(p) != null)).ToArray();
         }
 
         /// <summary>
@@ -196,8 +201,8 @@ void RegisterRelationshipProxies(DependentType type)
                 if (!RelationshipProxies.TryGetValue(attr, out RelationshipProxy proxies))
                 {
                     DependentType dependentType = GetDependentTypeFromRelationship(attr);
-                    var isContextRelation = _context.RelationshipsToUpdate?.ContainsKey(attr);
-                    var proxy = new RelationshipProxy(attr, dependentType, isContextRelation != null && (bool)isContextRelation);
+                    var isContextRelation = (_context.RelationshipsToUpdate?.ContainsKey(attr)) != null;
+                    var proxy = new RelationshipProxy(attr, dependentType, isContextRelation);
                     RelationshipProxies[attr] = proxy;
                 }
             }
@@ -291,7 +296,7 @@ IRelationshipsFromPreviousLayer CreateRelationshipsFromInstance(DependentType no
         /// <summary>
         /// Reflective helper method to create an instance of <see cref="RelationshipGroup{TDependent}"/>;
         /// </summary>
-        IRelationshipGroup CreateRelationsipGroupInstance(Type thisLayerType, RelationshipProxy proxy, List<IIdentifiable> principalEntities, List<IIdentifiable> dependentEntities)
+        IRelationshipGroup CreateRelationshipGroupInstance(Type thisLayerType, RelationshipProxy proxy, List<IIdentifiable> principalEntities, List<IIdentifiable> dependentEntities)
         {
             var dependentEntitiesHashed = TypeHelper.CreateInstanceOfOpenType(typeof(HashSet<>), thisLayerType, dependentEntities.Cast(thisLayerType));
             return (IRelationshipGroup)TypeHelper.CreateInstanceOfOpenType(typeof(RelationshipGroup<>),