From 0708c1ca5c9df9c09ddaf9a2aeb9ba201bf70a21 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 21 Jun 2019 14:53:11 +0200 Subject: [PATCH 01/12] fix: IRelationshipDictionary implements IReadOnlyDictionary --- .../Hooks/Execution/EntityDiffs.cs | 6 +++--- .../Hooks/Execution/EntityHashSet.cs | 2 +- .../Hooks/Execution/RelationshipsDictionary.cs | 18 ++++++++++++++---- .../Hooks/ResourceHookExecutor.cs | 1 - .../Models/ResourceDefinition.cs | 12 ++++++------ 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs b/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs index f0379636b5..d98a299d9e 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs @@ -14,7 +14,7 @@ namespace JsonApiDotNetCore.Hooks /// Also contains information about updated relationships through /// implementation of IRelationshipsDictionary> /// - public interface IEntityDiff : IRelationshipsDictionary, IEnumerable> where TEntity : class, IIdentifiable + public interface IEntityDiff : IExposeRelationshipsDictionary, IEnumerable> where TEntity : class, IIdentifiable { /// /// The database values of the resources affected by the request. @@ -42,7 +42,7 @@ public class EntityDiffs : IEntityDiff where TEntity : class, public EntityDiffs(HashSet requestEntities, HashSet databaseEntities, - Dictionary> relationships) + Dictionary> relationships) { Entities = requestEntities; AffectedRelationships = new RelationshipsDictionary(relationships); @@ -55,7 +55,7 @@ public EntityDiffs(HashSet requestEntities, /// internal EntityDiffs(IEnumerable requestEntities, IEnumerable databaseEntities, - Dictionary relationships) + Dictionary relationships) : this((HashSet)requestEntities, (HashSet)databaseEntities, TypeHelper.ConvertRelationshipDictionary(relationships)) { } diff --git a/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs b/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs index 98d7393796..642a5b56af 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs @@ -12,7 +12,7 @@ namespace JsonApiDotNetCore.Hooks /// Also contains information about updated relationships through /// implementation of IAffectedRelationshipsDictionary> /// - public interface IEntityHashSet : IRelationshipsDictionary, IEnumerable where TResource : class, IIdentifiable { } + public interface IEntityHashSet : IExposeRelationshipsDictionary, IEnumerable where TResource : class, IIdentifiable { } /// /// Implementation of IResourceHashSet{TResource}. diff --git a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs index 77248f5169..a31274f315 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs @@ -13,7 +13,8 @@ public interface IRelationshipsDictionary { } /// /// An interface that is implemented to expose a relationship dictionary on another class. /// - public interface IExposeRelationshipsDictionary : IRelationshipsDictionary where TDependentResource : class, IIdentifiable + public interface IExposeRelationshipsDictionary : + IRelationshipsDictionaryGetters where TDependentResource : class, IIdentifiable { /// /// Gets a dictionary of affected resources grouped by affected relationships. @@ -24,7 +25,15 @@ public interface IExposeRelationshipsDictionary : IRelations /// /// A helper class that provides insights in which relationships have been updated for which entities. /// - public interface IRelationshipsDictionary : IRelationshipsDictionary where TDependentResource : class, IIdentifiable + public interface IRelationshipsDictionary : + IRelationshipsDictionaryGetters, + IReadOnlyDictionary>, + IRelationshipsDictionary where TDependentResource : class, IIdentifiable { } + + /// + /// A helper class that provides insights in which relationships have been updated for which entities. + /// + public interface IRelationshipsDictionaryGetters where TDependentResource : class, IIdentifiable { /// /// Gets a dictionary of all entities that have an affected relationship to type @@ -42,7 +51,9 @@ public interface IRelationshipsDictionary : IRelationshipsDi /// It is practically a ReadOnlyDictionary{RelationshipAttribute, HashSet{TDependentResource}} dictionary /// with the two helper methods defined on IAffectedRelationships{TDependentResource}. /// - public class RelationshipsDictionary : ReadOnlyDictionary>, IRelationshipsDictionary where TDependentResource : class, IIdentifiable + public class RelationshipsDictionary : + ReadOnlyDictionary>, + IRelationshipsDictionary where TDependentResource : class, IIdentifiable { /// /// a dictionary with affected relationships as keys and values being the corresponding resources @@ -62,7 +73,6 @@ public RelationshipsDictionary(Dictionary relationships) : this(TypeHelper.ConvertRelationshipDictionary(relationships)) { } - /// public Dictionary> GetByRelationship() where TPrincipalResource : class, IIdentifiable { diff --git a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs index 7fccbbba7f..758b8c32d4 100644 --- a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs +++ b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs @@ -318,7 +318,6 @@ void ValidateHookResponse(IEnumerable returnedList, ResourcePipeline pipel PrincipalType principalType = dependentEntities.First().Key.PrincipalType; var byInverseRelationship = dependentEntities.Where(kvp => kvp.Key.InverseNavigation != null).ToDictionary(kvp => GetInverseRelationship(kvp.Key), kvp => kvp.Value); return (byInverseRelationship, principalType); - } /// diff --git a/src/JsonApiDotNetCore/Models/ResourceDefinition.cs b/src/JsonApiDotNetCore/Models/ResourceDefinition.cs index e615de6538..aea224812b 100644 --- a/src/JsonApiDotNetCore/Models/ResourceDefinition.cs +++ b/src/JsonApiDotNetCore/Models/ResourceDefinition.cs @@ -173,19 +173,19 @@ public virtual void AfterUpdate(HashSet entities, ResourcePipeline pipeline) /// public virtual void AfterDelete(HashSet entities, ResourcePipeline pipeline, bool succeeded) { } /// - public virtual void AfterUpdateRelationship(IRelationshipsDictionary resourcesByRelationship, ResourcePipeline pipeline) { } + public virtual void AfterUpdateRelationship(IRelationshipsDictionary entitiesByRelationship, ResourcePipeline pipeline) { } /// - public virtual IEnumerable BeforeCreate(IEntityHashSet affected, ResourcePipeline pipeline) { return affected; } + public virtual IEnumerable BeforeCreate(IEntityHashSet entities, ResourcePipeline pipeline) { return entities; } /// public virtual void BeforeRead(ResourcePipeline pipeline, bool isIncluded = false, string stringId = null) { } /// - public virtual IEnumerable BeforeUpdate(IEntityDiff ResourceDiff, ResourcePipeline pipeline) { return ResourceDiff.Entities; } + public virtual IEnumerable BeforeUpdate(IEntityDiff entityDiff, ResourcePipeline pipeline) { return entityDiff.Entities; } /// - public virtual IEnumerable BeforeDelete(IEntityHashSet affected, ResourcePipeline pipeline) { return affected; } + public virtual IEnumerable BeforeDelete(IEntityHashSet entities, ResourcePipeline pipeline) { return entities; } /// - public virtual IEnumerable BeforeUpdateRelationship(HashSet ids, IRelationshipsDictionary resourcesByRelationship, ResourcePipeline pipeline) { return ids; } + public virtual IEnumerable BeforeUpdateRelationship(HashSet ids, IRelationshipsDictionary entitiesByRelationship, ResourcePipeline pipeline) { return ids; } /// - public virtual void BeforeImplicitUpdateRelationship(IRelationshipsDictionary resourcesByRelationship, ResourcePipeline pipeline) { } + public virtual void BeforeImplicitUpdateRelationship(IRelationshipsDictionary entitiesByRelationship, ResourcePipeline pipeline) { } /// public virtual IEnumerable OnReturn(HashSet entities, ResourcePipeline pipeline) { return entities; } From a11c9e9e58a70038c26f528f4bd6922d424d2c1f Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Mon, 24 Jun 2019 15:19:14 +0200 Subject: [PATCH 02/12] chore: updated entity helpers --- .../Hooks/Execution/EntityDiffs.cs | 63 +++---- .../Hooks/Execution/EntityHashSet.cs | 17 +- .../Execution/RelationshipsDictionary.cs | 46 ++--- .../Hooks/IResourceHookContainer.cs | 2 +- src/JsonApiDotNetCore/Internal/TypeHelper.cs | 5 +- .../Models/ResourceDefinition.cs | 2 +- .../AffectedEntitiesHelperTests.cs | 166 ++++++++++++++++++ .../Update/BeforeUpdateTests.cs | 4 +- .../Update/BeforeUpdate_WithDbValues_Tests.cs | 12 +- .../ResourceHooks/ResourceHooksTestsSetup.cs | 2 +- 10 files changed, 239 insertions(+), 80 deletions(-) create mode 100644 test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs diff --git a/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs b/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs index d98a299d9e..fb30c26c7d 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Linq; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Models; @@ -12,40 +13,38 @@ namespace JsonApiDotNetCore.Hooks /// Contains the resources from the request and the corresponding database values. /// /// Also contains information about updated relationships through - /// implementation of IRelationshipsDictionary> + /// implementation of IRelationshipsDictionary> /// - public interface IEntityDiff : IExposeRelationshipsDictionary, IEnumerable> where TEntity : class, IIdentifiable + public interface IEntityDiffs : IEnumerable> where TResource : class, IIdentifiable { /// /// The database values of the resources affected by the request. /// - HashSet DatabaseValues { get; } + HashSet DatabaseValues { get; } /// /// The resources that were affected by the request. /// - HashSet Entities { get; } + EntityHashSet Entities { get; } + } /// - public class EntityDiffs : IEntityDiff where TEntity : class, IIdentifiable + public class EntityDiffs : IEntityDiffs where TResource : class, IIdentifiable { /// - public HashSet DatabaseValues { get => _databaseValues ?? ThrowNoDbValuesError(); } - private readonly HashSet _databaseValues; - private readonly bool _databaseValuesLoaded; - - /// - public HashSet Entities { get; private set; } + public HashSet DatabaseValues { get => _databaseValues ?? ThrowNoDbValuesError(); } /// - public RelationshipsDictionary AffectedRelationships { get; private set; } + public EntityHashSet Entities { get; private set; } - public EntityDiffs(HashSet requestEntities, - HashSet databaseEntities, - Dictionary> relationships) + private readonly HashSet _databaseValues; + private readonly bool _databaseValuesLoaded; + + public EntityDiffs(HashSet requestEntities, + HashSet databaseEntities, + Dictionary> relationships) { - Entities = requestEntities; - AffectedRelationships = new RelationshipsDictionary(relationships); + Entities = new EntityHashSet(requestEntities, relationships); _databaseValues = databaseEntities; _databaseValuesLoaded |= _databaseValues != null; } @@ -56,38 +55,26 @@ public EntityDiffs(HashSet requestEntities, internal EntityDiffs(IEnumerable requestEntities, IEnumerable databaseEntities, Dictionary relationships) - : this((HashSet)requestEntities, (HashSet)databaseEntities, TypeHelper.ConvertRelationshipDictionary(relationships)) { } - + : this((HashSet)requestEntities, (HashSet)databaseEntities, TypeHelper.ConvertRelationshipDictionary(relationships)) { } - /// - public Dictionary> GetByRelationship() where TPrincipalResource : class, IIdentifiable - { - return GetByRelationship(typeof(TPrincipalResource)); - } - - /// - public Dictionary> GetByRelationship(Type principalType) - { - return AffectedRelationships.GetByRelationship(principalType); - } /// - public IEnumerator> GetEnumerator() + public IEnumerator> GetEnumerator() { if (!_databaseValuesLoaded) ThrowNoDbValuesError(); foreach (var entity in Entities) { - TEntity currentValueInDatabase = null; + TResource currentValueInDatabase = null; currentValueInDatabase = _databaseValues.Single(e => entity.StringId == e.StringId); - yield return new EntityDiffPair(entity, currentValueInDatabase); + yield return new EntityDiffPair(entity, currentValueInDatabase); } } /// IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - private HashSet ThrowNoDbValuesError() + private HashSet ThrowNoDbValuesError() { throw new MemberAccessException("Cannot access database entities if the LoadDatabaseValues option is set to false"); } @@ -97,9 +84,9 @@ private HashSet ThrowNoDbValuesError() /// A wrapper that contains an entity that is affected by the request, /// matched to its current database value /// - public class EntityDiffPair where TEntity : class, IIdentifiable + public class EntityDiffPair where TResource : class, IIdentifiable { - public EntityDiffPair(TEntity entity, TEntity databaseValue) + public EntityDiffPair(TResource entity, TResource databaseValue) { Entity = entity; DatabaseValue = databaseValue; @@ -108,10 +95,10 @@ public EntityDiffPair(TEntity entity, TEntity databaseValue) /// /// The resource from the request matching the resource from the database. /// - public TEntity Entity { get; private set; } + public TResource Entity { get; private set; } /// /// The resource from the database matching the resource from the request. /// - public TEntity DatabaseValue { get; private set; } + public TResource DatabaseValue { get; private set; } } } diff --git a/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs b/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs index 642a5b56af..8a97705c39 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs @@ -3,6 +3,8 @@ using System.Collections; using JsonApiDotNetCore.Internal; using System; +using System.Collections.ObjectModel; +using System.Collections.Immutable; namespace JsonApiDotNetCore.Hooks { @@ -12,7 +14,7 @@ namespace JsonApiDotNetCore.Hooks /// Also contains information about updated relationships through /// implementation of IAffectedRelationshipsDictionary> /// - public interface IEntityHashSet : IExposeRelationshipsDictionary, IEnumerable where TResource : class, IIdentifiable { } + public interface IEntityHashSet : IByAffectedRelationships, IReadOnlyCollection where TResource : class, IIdentifiable { } /// /// Implementation of IResourceHashSet{TResource}. @@ -24,13 +26,16 @@ public interface IEntityHashSet : IExposeRelationshipsDictionary public class EntityHashSet : HashSet, IEntityHashSet where TResource : class, IIdentifiable { + + /// - public RelationshipsDictionary AffectedRelationships { get; private set; } + public Dictionary> AffectedRelationships { get => _relationships; } + private readonly RelationshipsDictionary _relationships; public EntityHashSet(HashSet entities, Dictionary> relationships) : base(entities) { - AffectedRelationships = new RelationshipsDictionary(relationships); + _relationships = new RelationshipsDictionary(relationships); } /// @@ -44,13 +49,13 @@ internal EntityHashSet(IEnumerable entities, /// public Dictionary> GetByRelationship(Type principalType) { - return AffectedRelationships.GetByRelationship(principalType); + return _relationships.GetByRelationship(principalType); } /// - public Dictionary> GetByRelationship() where TPrincipalResource : class, IIdentifiable + public Dictionary> GetByRelationship() where TRelatedResource : class, IIdentifiable { - return GetByRelationship(); + return GetByRelationship(typeof(TRelatedResource)); } } } \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs index a31274f315..0871facf12 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs @@ -1,39 +1,45 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Linq; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Models; namespace JsonApiDotNetCore.Hooks { + + + + /// + /// A dummy interface used internally by the hook executor. + /// public interface IRelationshipsDictionary { } + /// /// An interface that is implemented to expose a relationship dictionary on another class. /// - public interface IExposeRelationshipsDictionary : - IRelationshipsDictionaryGetters where TDependentResource : class, IIdentifiable + public interface IByAffectedRelationships : + IRelationshipGetters where TDependentResource : class, IIdentifiable { /// /// Gets a dictionary of affected resources grouped by affected relationships. /// - RelationshipsDictionary AffectedRelationships { get; } + Dictionary> AffectedRelationships { get; } } /// /// A helper class that provides insights in which relationships have been updated for which entities. /// public interface IRelationshipsDictionary : - IRelationshipsDictionaryGetters, + IRelationshipGetters, IReadOnlyDictionary>, IRelationshipsDictionary where TDependentResource : class, IIdentifiable { } /// /// A helper class that provides insights in which relationships have been updated for which entities. /// - public interface IRelationshipsDictionaryGetters where TDependentResource : class, IIdentifiable + public interface IRelationshipGetters where TDependentResource : class, IIdentifiable { /// /// Gets a dictionary of all entities that have an affected relationship to type @@ -51,38 +57,32 @@ public interface IRelationshipsDictionaryGetters where TDepe /// It is practically a ReadOnlyDictionary{RelationshipAttribute, HashSet{TDependentResource}} dictionary /// with the two helper methods defined on IAffectedRelationships{TDependentResource}. /// - public class RelationshipsDictionary : - ReadOnlyDictionary>, - IRelationshipsDictionary where TDependentResource : class, IIdentifiable + public class RelationshipsDictionary : + Dictionary>, + IRelationshipsDictionary where TResource : class, IIdentifiable { /// - /// a dictionary with affected relationships as keys and values being the corresponding resources - /// that were affected + /// Initializes a new instance of the class. /// - private readonly Dictionary> _groups; - - /// - public RelationshipsDictionary(Dictionary> relationships) : base(relationships) - { - _groups = relationships; - } + /// Relationships. + public RelationshipsDictionary(Dictionary> relationships) : base(relationships) { } /// /// Used internally by the ResourceHookExecutor to make live a bit easier with generics /// internal RelationshipsDictionary(Dictionary relationships) - : this(TypeHelper.ConvertRelationshipDictionary(relationships)) { } + : this(TypeHelper.ConvertRelationshipDictionary(relationships)) { } /// - public Dictionary> GetByRelationship() where TPrincipalResource : class, IIdentifiable + public Dictionary> GetByRelationship() where TRelatedResource : class, IIdentifiable { - return GetByRelationship(typeof(TPrincipalResource)); + return GetByRelationship(typeof(TRelatedResource)); } /// - public Dictionary> GetByRelationship(Type principalType) + public Dictionary> GetByRelationship(Type relatedType) { - return this.Where(p => p.Key.PrincipalType == principalType).ToDictionary(p => p.Key, p => p.Value); + return this.Where(p => p.Key.PrincipalType == relatedType).ToDictionary(p => p.Key, p => p.Value); } } } diff --git a/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs b/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs index cadb96a4fa..75b52cbddd 100644 --- a/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs +++ b/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs @@ -77,7 +77,7 @@ public interface IBeforeHooks where TResource : class, IIdentifiable /// The transformed entity set /// The entity diff. /// An enum indicating from where the hook was triggered. - IEnumerable BeforeUpdate(IEntityDiff entityDiff, ResourcePipeline pipeline); + IEnumerable BeforeUpdate(IEntityDiffs entityDiff, ResourcePipeline pipeline); /// /// Implement this hook to run custom logic in the diff --git a/src/JsonApiDotNetCore/Internal/TypeHelper.cs b/src/JsonApiDotNetCore/Internal/TypeHelper.cs index cf642aa395..cd5932d59b 100644 --- a/src/JsonApiDotNetCore/Internal/TypeHelper.cs +++ b/src/JsonApiDotNetCore/Internal/TypeHelper.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; +using JsonApiDotNetCore.Hooks; using JsonApiDotNetCore.Models; namespace JsonApiDotNetCore.Internal @@ -113,9 +114,9 @@ public static object CreateInstanceOfOpenType(Type openType, Type[] parameters, /// /// Helper method that "unboxes" the TValue from the relationship dictionary into /// - public static Dictionary> ConvertRelationshipDictionary(Dictionary relationships) + public static Dictionary> ConvertRelationshipDictionary(Dictionary relationships) { - return relationships.ToDictionary(pair => pair.Key, pair => (HashSet)pair.Value); + return relationships.ToDictionary(pair => pair.Key, pair => (HashSet)pair.Value); } /// diff --git a/src/JsonApiDotNetCore/Models/ResourceDefinition.cs b/src/JsonApiDotNetCore/Models/ResourceDefinition.cs index aea224812b..0a6b0ac091 100644 --- a/src/JsonApiDotNetCore/Models/ResourceDefinition.cs +++ b/src/JsonApiDotNetCore/Models/ResourceDefinition.cs @@ -179,7 +179,7 @@ public virtual void AfterUpdateRelationship(IRelationshipsDictionary entities /// public virtual void BeforeRead(ResourcePipeline pipeline, bool isIncluded = false, string stringId = null) { } /// - public virtual IEnumerable BeforeUpdate(IEntityDiff entityDiff, ResourcePipeline pipeline) { return entityDiff.Entities; } + public virtual IEnumerable BeforeUpdate(IEntityDiffs entityDiff, ResourcePipeline pipeline) { return entityDiff.Entities; } /// public virtual IEnumerable BeforeDelete(IEntityHashSet entities, ResourcePipeline pipeline) { return entities; } /// diff --git a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs new file mode 100644 index 0000000000..8e497d9f9d --- /dev/null +++ b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs @@ -0,0 +1,166 @@ +//using JsonApiDotNetCore.Models; +//using JsonApiDotNetCore.Hooks; +//using System.Collections.Generic; +//using Xunit; +//using System.Linq; + +//namespace UnitTests.ResourceHooks.AffectedEntities +//{ +// public class Dummy : Identifiable { } +// public class NotTargeted : Identifiable { } +// public class ToMany : Identifiable { } +// public class ToOne : Identifiable { } + +// public class AffectedEntitiesHelperTests +// { + +// public readonly HasOneAttribute FirstToOneAttr; +// public readonly HasOneAttribute SecondToOneAttr; +// public readonly HasManyAttribute ToManyAttr; + + +// public readonly Dictionary> Relationships = new Dictionary>(); +// public readonly HashSet FirstToOnesEntities = new HashSet { new Dummy() { Id = 1 }, new Dummy() { Id = 2 }, new Dummy() { Id = 3 } }; +// public readonly HashSet SecondToOnesEntities = new HashSet { new Dummy() { Id = 4 }, new Dummy() { Id = 5 }, new Dummy() { Id = 6 } }; +// public readonly HashSet ToManiesEntities = new HashSet { new Dummy() { Id = 7 }, new Dummy() { Id = 8 }, new Dummy() { Id = 9 } }; +// public readonly HashSet NoRelationshipsEntities = new HashSet { new Dummy() { Id = 10 }, new Dummy() { Id = 11 }, new Dummy() { Id = 12 } }; +// public readonly HashSet AllEntities; +// public AffectedEntitiesHelperTests() +// { +// FirstToOneAttr = new HasOneAttribute("first-to-one") +// { +// PrincipalType = typeof(Dummy), +// DependentType = typeof(ToOne) +// }; +// SecondToOneAttr = new HasOneAttribute("second-to-one") +// { +// PrincipalType = typeof(Dummy), +// DependentType = typeof(ToOne) +// }; +// ToManyAttr = new HasManyAttribute("to-manies") +// { +// PrincipalType = typeof(Dummy), +// DependentType = typeof(ToMany) +// }; +// Relationships.Add(FirstToOneAttr, FirstToOnesEntities); +// Relationships.Add(SecondToOneAttr, SecondToOnesEntities); +// Relationships.Add(ToManyAttr, ToManiesEntities); +// AllEntities = new HashSet(FirstToOnesEntities.Union(SecondToOnesEntities).Union(ToManiesEntities).Union(NoRelationshipsEntities)); +// } + +// [Fact] +// public void RelationshipsDictionary_GetByRelationships() +// { +// // arrange +// RelationshipsDictionary relationshipsDictionary = new RelationshipsDictionary(Relationships); + +// // act +// Dictionary> toOnes = relationshipsDictionary.GetByRelationship(); +// Dictionary> toManies = relationshipsDictionary.GetByRelationship(); +// Dictionary> notTargeted = relationshipsDictionary.GetByRelationship(); + +// // assert +// AssertRelationshipDictionaryGetters(relationshipsDictionary, toOnes, toManies, notTargeted); +// } + +// [Fact] +// public void EntityHashSet_GetByRelationships() +// { +// // arrange +// EntityHashSet entities = new EntityHashSet(AllEntities, Relationships); + +// // act +// Dictionary> toOnes = entities.GetByRelationship(); +// Dictionary> toManies = entities.GetByRelationship(); +// Dictionary> notTargeted = entities.GetByRelationship(); +// Dictionary> allRelationships = entities.AffectedRelationships; + +// // Assert +// AssertRelationshipDictionaryGetters(allRelationships, toOnes, toManies, notTargeted); +// var allEntitiesWithAffectedRelationships = allRelationships.SelectMany(kvp => kvp.Value).ToList(); +// NoRelationshipsEntities.ToList().ForEach(e => +// { +// Assert.DoesNotContain(e, allEntitiesWithAffectedRelationships); +// }); +// } + +// [Fact] +// public void EntityDiff_GetByRelationships() +// { +// // arrange +// var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id }).ToList()); +// EntityDiffs diffs = new EntityDiffs(AllEntities, dbEntities, Relationships); + +// // act +// Dictionary> toOnes = diffs.Entities.GetByRelationship(); +// Dictionary> toManies = diffs.Entities.GetByRelationship(); +// Dictionary> notTargeted = diffs.Entities.GetByRelationship(); +// Dictionary> allRelationships = diffs.Entities.AffectedRelationships; + +// // Assert +// AssertRelationshipDictionaryGetters(allRelationships, toOnes, toManies, notTargeted); +// var allEntitiesWithAffectedRelationships = allRelationships.SelectMany(kvp => kvp.Value).ToList(); +// NoRelationshipsEntities.ToList().ForEach(e => +// { +// Assert.DoesNotContain(e, allEntitiesWithAffectedRelationships); +// }); + +// var requestEntitiesFromDiff = diffs.Entities; +// requestEntitiesFromDiff.ToList().ForEach(e => +// { +// Assert.Contains(e, AllEntities); +// }); +// var databaseEntitiesFromDiff = diffs.DatabaseValues; +// databaseEntitiesFromDiff.ToList().ForEach(e => +// { +// Assert.Contains(e, dbEntities); +// }); +// } + +// [Fact] +// public void EntityDiff_Loops_Over_Diffs() +// { +// // arrange +// var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id })); +// EntityDiffs diffs = new EntityDiffs(AllEntities, dbEntities, Relationships); + +// // Assert & act +// foreach (EntityDiffPair diff in diffs) +// { +// Assert.Equal(diff.Entity.Id, diff.DatabaseValue.Id); +// Assert.NotEqual(diff.Entity, diff.DatabaseValue); +// Assert.Contains(diff.Entity, AllEntities); +// Assert.Contains(diff.DatabaseValue, dbEntities); +// } + +// } + +// private void AssertRelationshipDictionaryGetters(Dictionary> relationshipsDictionary, +// Dictionary> toOnes, +// Dictionary> toManies, +// Dictionary> notTargeted) +// { +// Assert.Contains(FirstToOneAttr, toOnes.Keys); +// Assert.Contains(SecondToOneAttr, toOnes.Keys); +// Assert.Contains(ToManyAttr, toManies.Keys); +// Assert.Equal(relationshipsDictionary.Keys.Count, toOnes.Keys.Count + toManies.Keys.Count + notTargeted.Keys.Count); + +// toOnes[FirstToOneAttr].ToList().ForEach((entitiy) => +// { +// Assert.Contains(entitiy, FirstToOnesEntities); +// }); + +// toOnes[SecondToOneAttr].ToList().ForEach((entity) => +// { +// Assert.Contains(entity, SecondToOnesEntities); +// }); + +// toManies[ToManyAttr].ToList().ForEach((entitiy) => +// { +// Assert.Contains(entitiy, ToManiesEntities); +// }); +// Assert.Empty(notTargeted); +// } + +// } +//} \ No newline at end of file diff --git a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdateTests.cs b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdateTests.cs index e27f7f8caa..f02acda264 100644 --- a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdateTests.cs +++ b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdateTests.cs @@ -24,7 +24,7 @@ public void BeforeUpdate() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny>(), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny>(), ResourcePipeline.Patch), Times.Once()); ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship(It.IsAny>(), It.IsAny>(), ResourcePipeline.Patch), Times.Once()); VerifyNoOtherCalls(todoResourceMock, ownerResourceMock); } @@ -62,7 +62,7 @@ public void BeforeUpdate_Without_Child_Hook_Implemented() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny>(), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny>(), ResourcePipeline.Patch), Times.Once()); VerifyNoOtherCalls(todoResourceMock, ownerResourceMock); } diff --git a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs index dd1eeea5e1..c22c5fb443 100644 --- a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs +++ b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs @@ -59,7 +59,7 @@ public void BeforeUpdate() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship( It.Is>(ids => PersonIdCheck(ids, personId)), It.Is>(rh => PersonCheck(lastName, rh)), @@ -93,7 +93,7 @@ public void BeforeUpdate_Deleting_Relationship() hookExecutor.BeforeUpdate(_todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); ownerResourceMock.Verify(rd => rd.BeforeImplicitUpdateRelationship( It.Is>(rh => PersonCheck(lastName + lastName, rh)), ResourcePipeline.Patch), @@ -140,7 +140,7 @@ public void BeforeUpdate_Without_Child_Hook_Implemented() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); todoResourceMock.Verify(rd => rd.BeforeImplicitUpdateRelationship( It.Is>(rh => TodoCheck(rh, description + description)), ResourcePipeline.Patch), @@ -161,7 +161,7 @@ public void BeforeUpdate_NoImplicit() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship( It.Is>(ids => PersonIdCheck(ids, personId)), It.IsAny>(), @@ -204,11 +204,11 @@ public void BeforeUpdate_NoImplicit_Without_Child_Hook_Implemented() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); VerifyNoOtherCalls(todoResourceMock, ownerResourceMock); } - private bool TodoCheck(IEntityDiff diff, string checksum) + private bool TodoCheck(IEntityDiffs diff, string checksum) { var diffPair = diff.Single(); var dbCheck = diffPair.DatabaseValue.Description == checksum; diff --git a/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs b/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs index 369e46b26d..45d105c1d6 100644 --- a/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs +++ b/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs @@ -261,7 +261,7 @@ void MockHooks(Mock> resourceDefinition) .Setup(rd => rd.BeforeRead(It.IsAny(), It.IsAny(), It.IsAny())) .Verifiable(); resourceDefinition - .Setup(rd => rd.BeforeUpdate(It.IsAny>(), It.IsAny())) + .Setup(rd => rd.BeforeUpdate(It.IsAny>(), It.IsAny())) .Returns, ResourcePipeline>((entityDiff, context) => entityDiff.Entities) .Verifiable(); resourceDefinition From a23998f6a55eb1d646033b7c6d936c7a267e4da7 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Mon, 24 Jun 2019 18:13:49 +0200 Subject: [PATCH 03/12] fix: construction of relationship dictionaries for root layer --- .../Execution/RelationshipsDictionary.cs | 12 +- .../Hooks/Traversal/TraversalHelper.cs | 59 ++-- .../AffectedEntitiesHelperTests.cs | 330 +++++++++--------- .../Update/BeforeUpdate_WithDbValues_Tests.cs | 19 +- 4 files changed, 217 insertions(+), 203 deletions(-) diff --git a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs index 0871facf12..966d0ab867 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs @@ -22,6 +22,10 @@ public interface IRelationshipsDictionary { } public interface IByAffectedRelationships : IRelationshipGetters where TDependentResource : class, IIdentifiable { + /// todo: expose getters that behave something like this: + /// relationshipDictionary.GetAffected( entity => entity.NavigationProperty ). + /// see https://stackoverflow.com/a/17116267/4441216 + /// /// Gets a dictionary of affected resources grouped by affected relationships. /// @@ -39,16 +43,16 @@ public interface IRelationshipsDictionary : /// /// A helper class that provides insights in which relationships have been updated for which entities. /// - public interface IRelationshipGetters where TDependentResource : class, IIdentifiable + public interface IRelationshipGetters where TResource : class, IIdentifiable { /// /// Gets a dictionary of all entities that have an affected relationship to type /// - Dictionary> GetByRelationship() where TPrincipalResource : class, IIdentifiable; + Dictionary> GetByRelationship() where TRelatedResource : class, IIdentifiable; /// /// Gets a dictionary of all entities that have an affected relationship to type /// - Dictionary> GetByRelationship(Type principalType); + Dictionary> GetByRelationship(Type relatedResourceType); } /// @@ -82,7 +86,7 @@ public Dictionary> GetByRelationship public Dictionary> 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); } } } diff --git a/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs b/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs index 50b155daff..0fc575f6f2 100644 --- a/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs +++ b/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs @@ -57,7 +57,7 @@ public RootNode CreateRootNode(IEnumerable rootEntiti _processedEntities = new Dictionary>(); RegisterRelationshipProxies(typeof(TEntity)); var uniqueEntities = ProcessEntities(rootEntities); - var relationshipsToNextLayer = GetRelationships(typeof(TEntity)); + var relationshipsToNextLayer = GetPopulatedRelationships(typeof(TEntity), uniqueEntities.Cast()); return new RootNode(uniqueEntities, relationshipsToNextLayer); } @@ -80,23 +80,27 @@ public EntityChildLayer CreateNextLayer(IEnumerable 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(); 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 @@ -104,6 +108,10 @@ public EntityChildLayer CreateNextLayer(IEnumerable nodes) } + /// + /// iterates throug the dictinary and groups the values + /// by matching dependent type of the keys (which are relationshipattributes) + /// Dictionary>>> GroupByDependentTypeOfRelationship(Dictionary> relationships) { return relationships.GroupBy(kvp => kvp.Key.DependentType).ToDictionary(gdc => gdc.Key, gdc => gdc.ToList()); @@ -115,11 +123,8 @@ Dictionary (Dictionary>, Dictionary>) ExtractEntities(IEnumerable principalNodes) { - var currentLayerEntities = new List(); - var principalsGrouped = new Dictionary>(); - var dependentsGrouped = new Dictionary>(); - - principalNodes.ForEach(n => RegisterRelationshipProxies(n.EntityType)); + var principalsEntitiesGrouped = new Dictionary>(); // RelationshipAttr_prevlayer->currentlayer => prevLayerEntities + var dependentsEntitiesGrouped = new Dictionary>(); // RelationshipAttr_prevlayer->currentlayer => currentLayerEntities foreach (var node in principalNodes) { @@ -141,36 +146,36 @@ Dictionary(), proxy.DependentType); - if (proxy.IsContextRelation || newDependentEntities.Any()) + var uniqueDependentEntities = UniqueInTree(dependentEntities.Cast(), 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); } /// - /// 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 /// /// The relationships. - RelationshipProxy[] GetRelationships(PrincipalType principal) + RelationshipProxy[] GetPopulatedRelationships(PrincipalType principalType, IEnumerable 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(); } /// @@ -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 /// /// Reflective helper method to create an instance of ; /// - IRelationshipGroup CreateRelationsipGroupInstance(Type thisLayerType, RelationshipProxy proxy, List principalEntities, List dependentEntities) + IRelationshipGroup CreateRelationshipGroupInstance(Type thisLayerType, RelationshipProxy proxy, List principalEntities, List dependentEntities) { var dependentEntitiesHashed = TypeHelper.CreateInstanceOfOpenType(typeof(HashSet<>), thisLayerType, dependentEntities.Cast(thisLayerType)); return (IRelationshipGroup)TypeHelper.CreateInstanceOfOpenType(typeof(RelationshipGroup<>), diff --git a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs index 8e497d9f9d..36dd8c7fbf 100644 --- a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs +++ b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs @@ -1,166 +1,166 @@ -//using JsonApiDotNetCore.Models; -//using JsonApiDotNetCore.Hooks; -//using System.Collections.Generic; -//using Xunit; -//using System.Linq; - -//namespace UnitTests.ResourceHooks.AffectedEntities -//{ -// public class Dummy : Identifiable { } -// public class NotTargeted : Identifiable { } -// public class ToMany : Identifiable { } -// public class ToOne : Identifiable { } - -// public class AffectedEntitiesHelperTests -// { - -// public readonly HasOneAttribute FirstToOneAttr; -// public readonly HasOneAttribute SecondToOneAttr; -// public readonly HasManyAttribute ToManyAttr; - - -// public readonly Dictionary> Relationships = new Dictionary>(); -// public readonly HashSet FirstToOnesEntities = new HashSet { new Dummy() { Id = 1 }, new Dummy() { Id = 2 }, new Dummy() { Id = 3 } }; -// public readonly HashSet SecondToOnesEntities = new HashSet { new Dummy() { Id = 4 }, new Dummy() { Id = 5 }, new Dummy() { Id = 6 } }; -// public readonly HashSet ToManiesEntities = new HashSet { new Dummy() { Id = 7 }, new Dummy() { Id = 8 }, new Dummy() { Id = 9 } }; -// public readonly HashSet NoRelationshipsEntities = new HashSet { new Dummy() { Id = 10 }, new Dummy() { Id = 11 }, new Dummy() { Id = 12 } }; -// public readonly HashSet AllEntities; -// public AffectedEntitiesHelperTests() -// { -// FirstToOneAttr = new HasOneAttribute("first-to-one") -// { -// PrincipalType = typeof(Dummy), -// DependentType = typeof(ToOne) -// }; -// SecondToOneAttr = new HasOneAttribute("second-to-one") -// { -// PrincipalType = typeof(Dummy), -// DependentType = typeof(ToOne) -// }; -// ToManyAttr = new HasManyAttribute("to-manies") -// { -// PrincipalType = typeof(Dummy), -// DependentType = typeof(ToMany) -// }; -// Relationships.Add(FirstToOneAttr, FirstToOnesEntities); -// Relationships.Add(SecondToOneAttr, SecondToOnesEntities); -// Relationships.Add(ToManyAttr, ToManiesEntities); -// AllEntities = new HashSet(FirstToOnesEntities.Union(SecondToOnesEntities).Union(ToManiesEntities).Union(NoRelationshipsEntities)); -// } - -// [Fact] -// public void RelationshipsDictionary_GetByRelationships() -// { -// // arrange -// RelationshipsDictionary relationshipsDictionary = new RelationshipsDictionary(Relationships); - -// // act -// Dictionary> toOnes = relationshipsDictionary.GetByRelationship(); -// Dictionary> toManies = relationshipsDictionary.GetByRelationship(); -// Dictionary> notTargeted = relationshipsDictionary.GetByRelationship(); - -// // assert -// AssertRelationshipDictionaryGetters(relationshipsDictionary, toOnes, toManies, notTargeted); -// } - -// [Fact] -// public void EntityHashSet_GetByRelationships() -// { -// // arrange -// EntityHashSet entities = new EntityHashSet(AllEntities, Relationships); - -// // act -// Dictionary> toOnes = entities.GetByRelationship(); -// Dictionary> toManies = entities.GetByRelationship(); -// Dictionary> notTargeted = entities.GetByRelationship(); -// Dictionary> allRelationships = entities.AffectedRelationships; - -// // Assert -// AssertRelationshipDictionaryGetters(allRelationships, toOnes, toManies, notTargeted); -// var allEntitiesWithAffectedRelationships = allRelationships.SelectMany(kvp => kvp.Value).ToList(); -// NoRelationshipsEntities.ToList().ForEach(e => -// { -// Assert.DoesNotContain(e, allEntitiesWithAffectedRelationships); -// }); -// } - -// [Fact] -// public void EntityDiff_GetByRelationships() -// { -// // arrange -// var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id }).ToList()); -// EntityDiffs diffs = new EntityDiffs(AllEntities, dbEntities, Relationships); - -// // act -// Dictionary> toOnes = diffs.Entities.GetByRelationship(); -// Dictionary> toManies = diffs.Entities.GetByRelationship(); -// Dictionary> notTargeted = diffs.Entities.GetByRelationship(); -// Dictionary> allRelationships = diffs.Entities.AffectedRelationships; - -// // Assert -// AssertRelationshipDictionaryGetters(allRelationships, toOnes, toManies, notTargeted); -// var allEntitiesWithAffectedRelationships = allRelationships.SelectMany(kvp => kvp.Value).ToList(); -// NoRelationshipsEntities.ToList().ForEach(e => -// { -// Assert.DoesNotContain(e, allEntitiesWithAffectedRelationships); -// }); - -// var requestEntitiesFromDiff = diffs.Entities; -// requestEntitiesFromDiff.ToList().ForEach(e => -// { -// Assert.Contains(e, AllEntities); -// }); -// var databaseEntitiesFromDiff = diffs.DatabaseValues; -// databaseEntitiesFromDiff.ToList().ForEach(e => -// { -// Assert.Contains(e, dbEntities); -// }); -// } - -// [Fact] -// public void EntityDiff_Loops_Over_Diffs() -// { -// // arrange -// var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id })); -// EntityDiffs diffs = new EntityDiffs(AllEntities, dbEntities, Relationships); - -// // Assert & act -// foreach (EntityDiffPair diff in diffs) -// { -// Assert.Equal(diff.Entity.Id, diff.DatabaseValue.Id); -// Assert.NotEqual(diff.Entity, diff.DatabaseValue); -// Assert.Contains(diff.Entity, AllEntities); -// Assert.Contains(diff.DatabaseValue, dbEntities); -// } +using JsonApiDotNetCore.Models; +using JsonApiDotNetCore.Hooks; +using System.Collections.Generic; +using Xunit; +using System.Linq; + +namespace UnitTests.ResourceHooks.AffectedEntities +{ + public class Dummy : Identifiable { } + public class NotTargeted : Identifiable { } + public class ToMany : Identifiable { } + public class ToOne : Identifiable { } + + public class AffectedEntitiesHelperTests + { + + public readonly HasOneAttribute FirstToOneAttr; + public readonly HasOneAttribute SecondToOneAttr; + public readonly HasManyAttribute ToManyAttr; + + + public readonly Dictionary> Relationships = new Dictionary>(); + public readonly HashSet FirstToOnesEntities = new HashSet { new Dummy() { Id = 1 }, new Dummy() { Id = 2 }, new Dummy() { Id = 3 } }; + public readonly HashSet SecondToOnesEntities = new HashSet { new Dummy() { Id = 4 }, new Dummy() { Id = 5 }, new Dummy() { Id = 6 } }; + public readonly HashSet ToManiesEntities = new HashSet { new Dummy() { Id = 7 }, new Dummy() { Id = 8 }, new Dummy() { Id = 9 } }; + public readonly HashSet NoRelationshipsEntities = new HashSet { new Dummy() { Id = 10 }, new Dummy() { Id = 11 }, new Dummy() { Id = 12 } }; + public readonly HashSet AllEntities; + public AffectedEntitiesHelperTests() + { + FirstToOneAttr = new HasOneAttribute("first-to-one") + { + PrincipalType = typeof(Dummy), + DependentType = typeof(ToOne) + }; + SecondToOneAttr = new HasOneAttribute("second-to-one") + { + PrincipalType = typeof(Dummy), + DependentType = typeof(ToOne) + }; + ToManyAttr = new HasManyAttribute("to-manies") + { + PrincipalType = typeof(Dummy), + DependentType = typeof(ToMany) + }; + Relationships.Add(FirstToOneAttr, FirstToOnesEntities); + Relationships.Add(SecondToOneAttr, SecondToOnesEntities); + Relationships.Add(ToManyAttr, ToManiesEntities); + AllEntities = new HashSet(FirstToOnesEntities.Union(SecondToOnesEntities).Union(ToManiesEntities).Union(NoRelationshipsEntities)); + } + + [Fact] + public void RelationshipsDictionary_GetByRelationships() + { + // arrange + RelationshipsDictionary relationshipsDictionary = new RelationshipsDictionary(Relationships); + + // act + Dictionary> toOnes = relationshipsDictionary.GetByRelationship(); + Dictionary> toManies = relationshipsDictionary.GetByRelationship(); + Dictionary> notTargeted = relationshipsDictionary.GetByRelationship(); + + // assert + AssertRelationshipDictionaryGetters(relationshipsDictionary, toOnes, toManies, notTargeted); + } + + [Fact] + public void EntityHashSet_GetByRelationships() + { + // arrange + EntityHashSet entities = new EntityHashSet(AllEntities, Relationships); + + // act + Dictionary> toOnes = entities.GetByRelationship(); + Dictionary> toManies = entities.GetByRelationship(); + Dictionary> notTargeted = entities.GetByRelationship(); + Dictionary> allRelationships = entities.AffectedRelationships; + + // Assert + AssertRelationshipDictionaryGetters(allRelationships, toOnes, toManies, notTargeted); + var allEntitiesWithAffectedRelationships = allRelationships.SelectMany(kvp => kvp.Value).ToList(); + NoRelationshipsEntities.ToList().ForEach(e => + { + Assert.DoesNotContain(e, allEntitiesWithAffectedRelationships); + }); + } + + [Fact] + public void EntityDiff_GetByRelationships() + { + // arrange + var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id }).ToList()); + EntityDiffs diffs = new EntityDiffs(AllEntities, dbEntities, Relationships); + + // act + Dictionary> toOnes = diffs.Entities.GetByRelationship(); + Dictionary> toManies = diffs.Entities.GetByRelationship(); + Dictionary> notTargeted = diffs.Entities.GetByRelationship(); + Dictionary> allRelationships = diffs.Entities.AffectedRelationships; + + // Assert + AssertRelationshipDictionaryGetters(allRelationships, toOnes, toManies, notTargeted); + var allEntitiesWithAffectedRelationships = allRelationships.SelectMany(kvp => kvp.Value).ToList(); + NoRelationshipsEntities.ToList().ForEach(e => + { + Assert.DoesNotContain(e, allEntitiesWithAffectedRelationships); + }); + + var requestEntitiesFromDiff = diffs.Entities; + requestEntitiesFromDiff.ToList().ForEach(e => + { + Assert.Contains(e, AllEntities); + }); + var databaseEntitiesFromDiff = diffs.DatabaseValues; + databaseEntitiesFromDiff.ToList().ForEach(e => + { + Assert.Contains(e, dbEntities); + }); + } + + [Fact] + public void EntityDiff_Loops_Over_Diffs() + { + // arrange + var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id })); + EntityDiffs diffs = new EntityDiffs(AllEntities, dbEntities, Relationships); + + // Assert & act + foreach (EntityDiffPair diff in diffs) + { + Assert.Equal(diff.Entity.Id, diff.DatabaseValue.Id); + Assert.NotEqual(diff.Entity, diff.DatabaseValue); + Assert.Contains(diff.Entity, AllEntities); + Assert.Contains(diff.DatabaseValue, dbEntities); + } -// } - -// private void AssertRelationshipDictionaryGetters(Dictionary> relationshipsDictionary, -// Dictionary> toOnes, -// Dictionary> toManies, -// Dictionary> notTargeted) -// { -// Assert.Contains(FirstToOneAttr, toOnes.Keys); -// Assert.Contains(SecondToOneAttr, toOnes.Keys); -// Assert.Contains(ToManyAttr, toManies.Keys); -// Assert.Equal(relationshipsDictionary.Keys.Count, toOnes.Keys.Count + toManies.Keys.Count + notTargeted.Keys.Count); - -// toOnes[FirstToOneAttr].ToList().ForEach((entitiy) => -// { -// Assert.Contains(entitiy, FirstToOnesEntities); -// }); - -// toOnes[SecondToOneAttr].ToList().ForEach((entity) => -// { -// Assert.Contains(entity, SecondToOnesEntities); -// }); - -// toManies[ToManyAttr].ToList().ForEach((entitiy) => -// { -// Assert.Contains(entitiy, ToManiesEntities); -// }); -// Assert.Empty(notTargeted); -// } - -// } -//} \ No newline at end of file + } + + private void AssertRelationshipDictionaryGetters(Dictionary> relationshipsDictionary, + Dictionary> toOnes, + Dictionary> toManies, + Dictionary> notTargeted) + { + Assert.Contains(FirstToOneAttr, toOnes.Keys); + Assert.Contains(SecondToOneAttr, toOnes.Keys); + Assert.Contains(ToManyAttr, toManies.Keys); + Assert.Equal(relationshipsDictionary.Keys.Count, toOnes.Keys.Count + toManies.Keys.Count + notTargeted.Keys.Count); + + toOnes[FirstToOneAttr].ToList().ForEach((entitiy) => + { + Assert.Contains(entitiy, FirstToOnesEntities); + }); + + toOnes[SecondToOneAttr].ToList().ForEach((entity) => + { + Assert.Contains(entity, SecondToOnesEntities); + }); + + toManies[ToManyAttr].ToList().ForEach((entitiy) => + { + Assert.Contains(entitiy, ToManiesEntities); + }); + Assert.Empty(notTargeted); + } + + } +} \ No newline at end of file diff --git a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs index c22c5fb443..fca4d9c480 100644 --- a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs +++ b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs @@ -59,7 +59,7 @@ public void BeforeUpdate() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once()); ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship( It.Is>(ids => PersonIdCheck(ids, personId)), It.Is>(rh => PersonCheck(lastName, rh)), @@ -93,7 +93,7 @@ public void BeforeUpdate_Deleting_Relationship() hookExecutor.BeforeUpdate(_todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once()); ownerResourceMock.Verify(rd => rd.BeforeImplicitUpdateRelationship( It.Is>(rh => PersonCheck(lastName + lastName, rh)), ResourcePipeline.Patch), @@ -140,7 +140,7 @@ public void BeforeUpdate_Without_Child_Hook_Implemented() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once()); todoResourceMock.Verify(rd => rd.BeforeImplicitUpdateRelationship( It.Is>(rh => TodoCheck(rh, description + description)), ResourcePipeline.Patch), @@ -161,7 +161,7 @@ public void BeforeUpdate_NoImplicit() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once()); ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship( It.Is>(ids => PersonIdCheck(ids, personId)), It.IsAny>(), @@ -204,16 +204,21 @@ public void BeforeUpdate_NoImplicit_Without_Child_Hook_Implemented() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once()); + todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once()); VerifyNoOtherCalls(todoResourceMock, ownerResourceMock); } - private bool TodoCheck(IEntityDiffs diff, string checksum) + private bool TodoCheckDiff(IEntityDiffs diff, string checksum) { var diffPair = diff.Single(); var dbCheck = diffPair.DatabaseValue.Description == checksum; var reqCheck = diffPair.Entity.Description == null; - return (dbCheck && reqCheck); + var diffPairCheck = (dbCheck && reqCheck); + + var updatedRelationship = diff.Entities.GetByRelationship().Single(); + var diffcheck = updatedRelationship.Key.PublicRelationshipName == "one-to-one-person"; + + return (dbCheck && reqCheck && diffcheck); } private bool TodoCheck(IRelationshipsDictionary rh, string checksum) From db857e51d20738d541600a6ef18596d1fa04f54c Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Mon, 24 Jun 2019 18:46:46 +0200 Subject: [PATCH 04/12] fix: before update pipeline --- .../Hooks/ResourceHookExecutor.cs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs index 758b8c32d4..050679c03b 100644 --- a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs +++ b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs @@ -248,14 +248,18 @@ void FireNestedBeforeUpdateHooks(ResourcePipeline pipeline, EntityChildLayer lay IEnumerable uniqueEntities = node.UniqueEntities; DependentType entityType = node.EntityType; - // fire the BeforeUpdateRelationship hook for o1 + // fire the BeforeUpdateRelationship hook for o2 if (nestedHookcontainer != null) { if (uniqueEntities.Cast().Any()) { var relationships = node.RelationshipsToNextLayer.Select(p => p.Attribute).ToArray(); var dbValues = LoadDbValues(entityType, uniqueEntities, ResourceHook.BeforeUpdateRelationship, relationships); - var resourcesByRelationship = CreateRelationshipHelper(entityType, node.RelationshipsFromPreviousLayer.GetDependentEntities(), dbValues); + + var dependentByPrevLayerRelationships = node.RelationshipsFromPreviousLayer.GetDependentEntities(); + var principalsByCurrentLayerRelationships = dependentByPrevLayerRelationships.ToDictionary(kvp => _graph.GetInverseRelationship(kvp.Key), kvp => kvp.Value); + + var resourcesByRelationship = CreateRelationshipHelper(entityType, principalsByCurrentLayerRelationships, dbValues); var allowedIds = CallHook(nestedHookcontainer, ResourceHook.BeforeUpdateRelationship, new object[] { GetIds(uniqueEntities), resourcesByRelationship, pipeline }).Cast(); var updated = GetAllowedEntities(uniqueEntities, allowedIds); node.UpdateUnique(updated); @@ -263,14 +267,17 @@ void FireNestedBeforeUpdateHooks(ResourcePipeline pipeline, EntityChildLayer lay } } - // fire the BeforeImplicitUpdateRelationship hook for o1 + // fire the BeforeImplicitUpdateRelationship hook for o1 var implicitPrincipalTargets = node.RelationshipsFromPreviousLayer.GetPrincipalEntities(); if (pipeline != ResourcePipeline.Post && implicitPrincipalTargets.Any()) { + // value in implicitPrincipalTargets is a1 here. + // we need to load the current value in db, which is o1. + // then we need to inverse the relationship attribute FireForAffectedImplicits(entityType, implicitPrincipalTargets, pipeline, uniqueEntities); } - // fire the BeforeImplicitUpdateRelationship hook for a2 + // fire the BeforeImplicitUpdateRelationship hook for a2 var dependentEntities = node.RelationshipsFromPreviousLayer.GetDependentEntities(); if (dependentEntities.Any()) { @@ -290,7 +297,8 @@ void FireForAffectedImplicits(Type entityTypeToInclude, Dictionary _graph.GetInverseRelationship(kvp.Key), kvp => kvp.Value); + var resourcesByRelationship = CreateRelationshipHelper(entityTypeToInclude, inverse); CallHook(container, ResourceHook.BeforeImplicitUpdateRelationship, new object[] { resourcesByRelationship, pipeline, }); } @@ -366,8 +374,8 @@ Dictionary ReplaceWithDbValues(Dictionary().Select(entity => dbValues.Single(dbEntity => dbEntity.StringId == entity.StringId)).Cast(key.DependentType); - prevLayerRelationships[key] = TypeHelper.CreateHashSetFor(key.DependentType, replaced); + var replaced = prevLayerRelationships[key].Cast().Select(entity => dbValues.Single(dbEntity => dbEntity.StringId == entity.StringId)).Cast(key.PrincipalType); + prevLayerRelationships[key] = TypeHelper.CreateHashSetFor(key.PrincipalType, replaced); } return prevLayerRelationships; } From b53a03e9173466de5d13d633fdd2d442914e9432 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 25 Jun 2019 10:11:43 +0200 Subject: [PATCH 05/12] fix: error when exposing relationship information between layers in hooks, added comments to code --- .../Hooks/Execution/HookExecutorHelper.cs | 7 +- .../Hooks/Execution/IHookExecutorHelper.cs | 15 +- .../Hooks/ResourceHookExecutor.cs | 139 ++++++++++++------ .../Hooks/Traversal/TraversalHelper.cs | 6 +- src/JsonApiDotNetCore/Internal/TypeHelper.cs | 18 +++ .../ResourceHooks/ResourceHooksTestsSetup.cs | 13 +- 6 files changed, 145 insertions(+), 53 deletions(-) diff --git a/src/JsonApiDotNetCore/Hooks/Execution/HookExecutorHelper.cs b/src/JsonApiDotNetCore/Hooks/Execution/HookExecutorHelper.cs index 6a2c2767bf..df7f4daa0c 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/HookExecutorHelper.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/HookExecutorHelper.cs @@ -82,7 +82,7 @@ public IResourceHookContainer GetResourceHookContainer(Resourc public IEnumerable LoadDbValues(PrincipalType entityTypeForRepository, IEnumerable entities, ResourceHook hook, params RelationshipAttribute[] relationships) { var paths = relationships.Select(p => p.RelationshipPath).ToArray(); - var idType = GetIdentifierType(entityTypeForRepository); + var idType = TypeHelper.GetIdentifierType(entityTypeForRepository); var parameterizedGetWhere = GetType() .GetMethod(nameof(GetWhereAndInclude), BindingFlags.NonPublic | BindingFlags.Instance) .MakeGenericMethod(entityTypeForRepository, idType); @@ -144,11 +144,6 @@ IHooksDiscovery GetHookDiscovery(Type entityType) return discovery; } - Type GetIdentifierType(Type entityType) - { - return entityType.GetProperty("Id").PropertyType; - } - IEnumerable GetWhereAndInclude(IEnumerable ids, string[] relationshipPaths) where TEntity : class, IIdentifiable { var repo = GetRepository(); diff --git a/src/JsonApiDotNetCore/Hooks/Execution/IHookExecutorHelper.cs b/src/JsonApiDotNetCore/Hooks/Execution/IHookExecutorHelper.cs index 63b3621a34..98de544f0a 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/IHookExecutorHelper.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/IHookExecutorHelper.cs @@ -42,7 +42,20 @@ internal interface IHookExecutorHelper /// /// For a set of entities, loads current values from the database /// + /// type of the entities to be loaded + /// The set of entities to load the db values for + /// The hook in which the db values will be displayed. + /// Relationships that need to be included on entities. IEnumerable LoadDbValues(Type repositoryEntityType, IEnumerable entities, ResourceHook hook, params RelationshipAttribute[] relationships); - bool ShouldLoadDbValues(Type containerEntityType, ResourceHook hook); + + /// + /// Checks if the display database values option is allowed for the targetd hook, and for + /// a given resource of type checks if this hook is implemented and if the + /// database values option is enabled. + /// + /// true, if load db values was shoulded, false otherwise. + /// Container entity type. + /// Hook. + bool ShouldLoadDbValues(Type entityType, ResourceHook hook); } } \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs index 050679c03b..9a30bf41e5 100644 --- a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs +++ b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs @@ -70,7 +70,6 @@ public virtual IEnumerable BeforeCreate(IEnumerable e node.UpdateUnique(updated); node.Reassign(entities); } - FireNestedBeforeUpdateHooks(pipeline, _traversalHelper.CreateNextLayer(node)); return entities; } @@ -234,12 +233,15 @@ void RecursiveBeforeRead(ContextEntity contextEntity, List relationshipC } /// - /// Fires the nested before hooks. For example consider the case when - /// the owner of an article a1 (one-to-one) was updated from o1 to o2, where o2 - /// was already related to a2. Then, the BeforeUpdateRelationship should be - /// fired for o2, and the BeforeImplicitUpdateRelationship hook should be fired for - /// o2 and then too for a2. + /// Fires the nested before hooks for entities in the current /// + /// + /// For example: consider the case when the owner of article1 (one-to-one) + /// is being updated from owner_old to owner_new, where owner_new is currently already + /// related to article2. Then, the following nested hooks need to be fired in the following order. + /// First the BeforeUpdateRelationship should be for owner1, then the + /// BeforeImplicitUpdateRelationship hook should be fired for + /// owner2, and lastely the BeforeImplicitUpdateRelationship for article2. void FireNestedBeforeUpdateHooks(ResourcePipeline pipeline, EntityChildLayer layer) { foreach (IEntityNode node in layer) @@ -247,8 +249,10 @@ void FireNestedBeforeUpdateHooks(ResourcePipeline pipeline, EntityChildLayer lay var nestedHookcontainer = _executorHelper.GetResourceHookContainer(node.EntityType, ResourceHook.BeforeUpdateRelationship); IEnumerable uniqueEntities = node.UniqueEntities; DependentType entityType = node.EntityType; + Dictionary currenEntitiesGrouped; + Dictionary currentEntitiesGroupedInverse; - // fire the BeforeUpdateRelationship hook for o2 + // fire the BeforeUpdateRelationship hook for owner_new if (nestedHookcontainer != null) { if (uniqueEntities.Cast().Any()) @@ -256,10 +260,17 @@ void FireNestedBeforeUpdateHooks(ResourcePipeline pipeline, EntityChildLayer lay var relationships = node.RelationshipsToNextLayer.Select(p => p.Attribute).ToArray(); var dbValues = LoadDbValues(entityType, uniqueEntities, ResourceHook.BeforeUpdateRelationship, relationships); - var dependentByPrevLayerRelationships = node.RelationshipsFromPreviousLayer.GetDependentEntities(); - var principalsByCurrentLayerRelationships = dependentByPrevLayerRelationships.ToDictionary(kvp => _graph.GetInverseRelationship(kvp.Key), kvp => kvp.Value); - - var resourcesByRelationship = CreateRelationshipHelper(entityType, principalsByCurrentLayerRelationships, dbValues); + /// these are the entities of the current node grouped by + /// RelationshipAttributes that occured in the previous layer + /// so it looks like { HasOneAttribute:owner => owner_new }. + /// Note that in the BeforeUpdateRelationship hook of Person, + /// we want want inverse relationship attribute: + /// we now have the one pointing from article -> person, ] + /// but we require the the one that points from person -> article + currenEntitiesGrouped = node.RelationshipsFromPreviousLayer.GetDependentEntities(); + currentEntitiesGroupedInverse = ReplaceKeysWithInverseRelationships(currenEntitiesGrouped); + + var resourcesByRelationship = CreateRelationshipHelper(entityType, currentEntitiesGroupedInverse, dbValues); var allowedIds = CallHook(nestedHookcontainer, ResourceHook.BeforeUpdateRelationship, new object[] { GetIds(uniqueEntities), resourcesByRelationship, pipeline }).Cast(); var updated = GetAllowedEntities(uniqueEntities, allowedIds); node.UpdateUnique(updated); @@ -267,26 +278,60 @@ void FireNestedBeforeUpdateHooks(ResourcePipeline pipeline, EntityChildLayer lay } } - // fire the BeforeImplicitUpdateRelationship hook for o1 - var implicitPrincipalTargets = node.RelationshipsFromPreviousLayer.GetPrincipalEntities(); - if (pipeline != ResourcePipeline.Post && implicitPrincipalTargets.Any()) + /// Fire the BeforeImplicitUpdateRelationship hook for owner_old. + /// Note: if the pipeline is Post it means we just created article1, + /// which means we are sure that it isn't related to any other entities yet. + if (pipeline != ResourcePipeline.Post) { - // value in implicitPrincipalTargets is a1 here. - // we need to load the current value in db, which is o1. - // then we need to inverse the relationship attribute - FireForAffectedImplicits(entityType, implicitPrincipalTargets, pipeline, uniqueEntities); + /// To fire a hook for owner_old, we need to first get a reference to it. + /// For this, we need to query the database for the HasOneAttribute:owner + /// relationship of article1, which is referred to as the + /// principal side of the HasOneAttribute:owner relationship. + var principalEntities = node.RelationshipsFromPreviousLayer.GetPrincipalEntities(); + if (principalEntities.Any()) + { + /// owner_old is loaded, which is an "implicitly affected entity" + FireForAffectedImplicits(entityType, principalEntities, pipeline, uniqueEntities); + } } - // fire the BeforeImplicitUpdateRelationship hook for a2 - var dependentEntities = node.RelationshipsFromPreviousLayer.GetDependentEntities(); - if (dependentEntities.Any()) + /// Fire the BeforeImplicitUpdateRelationship hook for article2 + /// For this, we need to query the database for the current owner + /// relationship value of owner_new. + currenEntitiesGrouped = node.RelationshipsFromPreviousLayer.GetDependentEntities(); + if (currenEntitiesGrouped.Any()) { - (var implicitDependentTargets, var principalEntityType) = GetDependentImplicitsTargets(dependentEntities); - FireForAffectedImplicits(principalEntityType, implicitDependentTargets, pipeline); + /// dependentEntities is grouped by relationships from previous + /// layer, ie { HasOneAttribute:owner => owner_new }. But + /// to load article2 onto owner_new, we need to have the + /// RelationshipAttribute from owner to article, which is the + /// inverse of HasOneAttribute:owner + currentEntitiesGroupedInverse = ReplaceKeysWithInverseRelationships(currenEntitiesGrouped); + /// Note that currently in the JADNC implementation of hooks, + /// the root layer is ALWAYS homogenous, so we safely assume + /// that for every relationship to the previous layer, the + /// principal type is the same. + PrincipalType principalEntityType = currenEntitiesGrouped.First().Key.PrincipalType; + FireForAffectedImplicits(principalEntityType, currentEntitiesGroupedInverse, pipeline); } } } + /// + /// replaces the keys of the dictionary + /// with its inverse relationship attribute. + /// + /// Entities grouped by relationship attribute + Dictionary ReplaceKeysWithInverseRelationships(Dictionary entitiesByRelationship) + { + /// when Article has one Owner (HasOneAttribute:owner) is set, there is no guarantee + /// that the inverse attribute was also set (Owner has one Article: HasOneAttr:article). + /// If it isn't, JADNC currently knows nothing about this relationship pointing back, and it + /// currently cannot fire hooks for entities resolved through inverse relationships. + var inversableRelationshipAttributes = entitiesByRelationship.Where(kvp => kvp.Key.InverseNavigation != null); + return inversableRelationshipAttributes.ToDictionary(kvp => _graph.GetInverseRelationship(kvp.Key), kvp => kvp.Value); + } + /// /// Given a source of entities, gets the implicitly affected entities /// from the database and calls the BeforeImplicitUpdateRelationship hook. @@ -317,16 +362,6 @@ void ValidateHookResponse(IEnumerable returnedList, ResourcePipeline pipel } } - /// - /// NOTE: in JADNC usage, the root layer is ALWAYS homogenous, so we can be sure that for every - /// relationship to the previous layer, the principal type is the same. - /// - (Dictionary, PrincipalType) GetDependentImplicitsTargets(Dictionary dependentEntities) - { - PrincipalType principalType = dependentEntities.First().Key.PrincipalType; - var byInverseRelationship = dependentEntities.Where(kvp => kvp.Key.InverseNavigation != null).ToDictionary(kvp => GetInverseRelationship(kvp.Key), kvp => kvp.Value); - return (byInverseRelationship, principalType); - } /// /// A helper method to call a hook on reflectively. @@ -389,26 +424,46 @@ HashSet GetAllowedEntities(IEnumerable source, IEnumerable(source.Cast().Where(ue => allowedIds.Contains(ue.StringId))); } + /// - /// Gets the inverse for + /// given the set of , it will load all the + /// values from the database of these entites. /// - RelationshipAttribute GetInverseRelationship(RelationshipAttribute attribute) + /// The db values. + /// type of the entities to be loaded + /// The set of entities to load the db values for + /// The hook in which the db values will be displayed. + /// Relationships from to the next layer: + /// this indicates which relationships will be included on . + IEnumerable LoadDbValues(Type entityType, IEnumerable uniqueEntities, ResourceHook targetHook, RelationshipAttribute[] relationshipsToNextLayer) { - return _graph.GetInverseRelationship(attribute); + /// We only need to load database values if the target hook of this hook execution + /// cycle is compatible with displaying database values and has this option enabled. + if (!_executorHelper.ShouldLoadDbValues(entityType, targetHook)) return null; + return _executorHelper.LoadDbValues(entityType, uniqueEntities, targetHook, relationshipsToNextLayer); } - IEnumerable LoadDbValues(Type containerEntityType, IEnumerable uniqueEntities, ResourceHook targetHook, RelationshipAttribute[] relationshipsToNextLayer) - { - if (!_executorHelper.ShouldLoadDbValues(containerEntityType, targetHook)) return null; - return _executorHelper.LoadDbValues(containerEntityType, uniqueEntities, targetHook, relationshipsToNextLayer); - } + /// + /// Fires the AfterUpdateRelationship hook + /// void FireAfterUpdateRelationship(IResourceHookContainer container, IEntityNode node, ResourcePipeline pipeline) { - var resourcesByRelationship = CreateRelationshipHelper(node.EntityType, node.RelationshipsFromPreviousLayer.GetDependentEntities()); + + Dictionary currenEntitiesGrouped = node.RelationshipsFromPreviousLayer.GetDependentEntities(); + /// the relationships attributes in currenEntitiesGrouped will be pointing from a + /// resource in the previouslayer to a resource in the current (nested) layer. + /// For the nested hook we need to replace these attributes with their inverse. + /// See the FireNestedBeforeUpdateHooks method for a more detailed example. + var resourcesByRelationship = CreateRelationshipHelper(node.EntityType, ReplaceKeysWithInverseRelationships(currenEntitiesGrouped)); CallHook(container, ResourceHook.AfterUpdateRelationship, new object[] { resourcesByRelationship, pipeline }); } + /// + /// Returns a list of StringIds from a list of IIdentifiables (). + /// + /// The ids. + /// iidentifiable entities. HashSet GetIds(IEnumerable entities) { return new HashSet(entities.Cast().Select(e => e.StringId)); diff --git a/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs b/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs index 0fc575f6f2..89304235d5 100644 --- a/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs +++ b/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs @@ -91,6 +91,8 @@ public EntityChildLayer CreateNextLayer(IEnumerable nodes) var nextNodes = principalsGrouped.Select(entry => { var nextNodeType = entry.Key; + RegisterRelationshipProxies(nextNodeType); + var populatedRelationships = new List(); var relationshipsToPreviousLayer = entry.Value.Select(grouped => { @@ -99,7 +101,6 @@ public EntityChildLayer CreateNextLayer(IEnumerable nodes) return CreateRelationshipGroupInstance(nextNodeType, proxy, grouped.Value, dependents[proxy]); }).ToList(); - RegisterRelationshipProxies(nextNodeType); return CreateNodeInstance(nextNodeType, populatedRelationships.ToArray(), relationshipsToPreviousLayer); }).ToList(); @@ -201,7 +202,8 @@ void RegisterRelationshipProxies(DependentType type) if (!RelationshipProxies.TryGetValue(attr, out RelationshipProxy proxies)) { DependentType dependentType = GetDependentTypeFromRelationship(attr); - var isContextRelation = (_context.RelationshipsToUpdate?.ContainsKey(attr)) != null; + bool isContextRelation = false; + if (_context.RelationshipsToUpdate != null) isContextRelation = _context.RelationshipsToUpdate.ContainsKey(attr); var proxy = new RelationshipProxy(attr, dependentType, isContextRelation); RelationshipProxies[attr] = proxy; } diff --git a/src/JsonApiDotNetCore/Internal/TypeHelper.cs b/src/JsonApiDotNetCore/Internal/TypeHelper.cs index cd5932d59b..7326377b3d 100644 --- a/src/JsonApiDotNetCore/Internal/TypeHelper.cs +++ b/src/JsonApiDotNetCore/Internal/TypeHelper.cs @@ -181,5 +181,23 @@ public static Type GetListInnerType(IEnumerable list) { return list.GetType().GetGenericArguments()[0]; } + + /// + /// Gets the type (Guid or int) of the Id of a type that implements IIdentifiable + /// + public static Type GetIdentifierType(Type entityType) + { + var property = entityType.GetProperty("Id"); + if (property == null) throw new ArgumentException("Type does not have a property Id"); + return entityType.GetProperty("Id").PropertyType; + } + + /// + /// Gets the type (Guid or int) of the Id of a type that implements IIdentifiable + /// + public static Type GetIdentifierType() where T : IIdentifiable + { + return typeof(T).GetProperty("Id").PropertyType; + } } } diff --git a/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs b/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs index 45d105c1d6..27e413de4c 100644 --- a/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs +++ b/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs @@ -39,6 +39,7 @@ public HooksDummyData() .AddResource
() .AddResource() .AddResource() + .AddResource() .Build(); _todoFaker = new Faker().Rules((f, i) => i.Id = f.UniqueIndex + 1); @@ -325,8 +326,16 @@ void SetupProcessorFactoryForResourceDefinition( if (dbContext != null) { - IEntityReadRepository repo = CreateTestRepository(dbContext, apiContext); - processorFactory.Setup(c => c.GetProcessor>(typeof(IEntityReadRepository<,>), typeof(TModel), typeof(int))).Returns(repo); + var idType = TypeHelper.GetIdentifierType(); + if (idType == typeof(int)) + { + IEntityReadRepository repo = CreateTestRepository(dbContext, apiContext); + processorFactory.Setup(c => c.GetProcessor>(typeof(IEntityReadRepository<,>), typeof(TModel), typeof(int))).Returns(repo); + } else + { + throw new TypeLoadException("Test not set up properly"); + } + } } From c179310415c0cc7975e6e2060d7057c1a9cd4857 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 25 Jun 2019 10:13:28 +0200 Subject: [PATCH 06/12] chore: more comments --- src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs b/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs index 89304235d5..fcffedf16b 100644 --- a/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs +++ b/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs @@ -193,6 +193,11 @@ HashSet ProcessEntities(IEnumerable incomingEntities) return newEntities; } + /// + /// Parses all relationships from to + /// other models in the resource graphs by constructing RelationshipProxies . + /// + /// The type to parse void RegisterRelationshipProxies(DependentType type) { var contextEntity = _graph.GetContextEntity(type); From 30565eb89bbf0903a0d8c277b1041e3570880268 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 25 Jun 2019 10:58:51 +0200 Subject: [PATCH 07/12] fix: BeforeImplicitUpdate hook through the Delete pipeline, added a bunch of comments --- .../Data/AppDbContext.cs | 8 ++------ ...ckableResourceBase.cs => LockableResource.cs} | 4 ++-- .../Resources/PersonResource.cs | 16 +++++----------- .../Resources/TodoResource.cs | 2 +- .../Data/DefaultEntityRepository.cs | 2 +- .../Hooks/ResourceHookExecutor.cs | 6 +++++- .../Hooks/Traversal/RootNode.cs | 13 ++++++++----- .../Hooks/Traversal/TraversalHelper.cs | 6 +++--- 8 files changed, 27 insertions(+), 30 deletions(-) rename src/Examples/JsonApiDotNetCoreExample/Resources/{LockableResourceBase.cs => LockableResource.cs} (76%) diff --git a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs index b333e88536..d7147123f6 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs @@ -58,15 +58,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) .WithOne(t => t.ParentTodoItem) .HasForeignKey(t => t.ParentTodoItemId); - modelBuilder.Entity() - .HasOne(p => p.Passport) - .WithOne(p => p.Person) - .HasForeignKey(p => p.PassportId); - modelBuilder.Entity() .HasOne(p => p.Person) .WithOne(p => p.Passport) - .HasForeignKey(p => p.PassportId); + .HasForeignKey(p => p.PassportId) + .OnDelete(DeleteBehavior.SetNull); modelBuilder.Entity() .HasOne(p => p.ToOnePerson) diff --git a/src/Examples/JsonApiDotNetCoreExample/Resources/LockableResourceBase.cs b/src/Examples/JsonApiDotNetCoreExample/Resources/LockableResource.cs similarity index 76% rename from src/Examples/JsonApiDotNetCoreExample/Resources/LockableResourceBase.cs rename to src/Examples/JsonApiDotNetCoreExample/Resources/LockableResource.cs index 7e2ce4f658..7ad9659f18 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Resources/LockableResourceBase.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Resources/LockableResource.cs @@ -7,9 +7,9 @@ namespace JsonApiDotNetCoreExample.Resources { - public abstract class LockableResourceBase : ResourceDefinition where T : class, IIsLockable, IIdentifiable + public abstract class LockableResource : ResourceDefinition where T : class, IIsLockable, IIdentifiable { - protected LockableResourceBase(IResourceGraph graph) : base(graph) { } + protected LockableResource(IResourceGraph graph) : base(graph) { } protected void DisallowLocked(IEnumerable entities) { diff --git a/src/Examples/JsonApiDotNetCoreExample/Resources/PersonResource.cs b/src/Examples/JsonApiDotNetCoreExample/Resources/PersonResource.cs index 032c9acb4b..4887414e73 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Resources/PersonResource.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Resources/PersonResource.cs @@ -6,25 +6,19 @@ namespace JsonApiDotNetCoreExample.Resources { - public class PersonResource : LockableResourceBase + public class PersonResource : LockableResource { public PersonResource(IResourceGraph graph) : base(graph) { } - public override IEnumerable BeforeUpdateRelationship(HashSet ids, IRelationshipsDictionary resourcesByRelationship, ResourcePipeline pipeline) + public override IEnumerable BeforeUpdateRelationship(HashSet ids, IRelationshipsDictionary entitiesByRelationship, ResourcePipeline pipeline) { - BeforeImplicitUpdateRelationship(resourcesByRelationship, pipeline); + BeforeImplicitUpdateRelationship(entitiesByRelationship, pipeline); return ids; } - //[LoadDatabaseValues(true)] - //public override IEnumerable BeforeUpdate(IResourceDiff entityDiff, ResourcePipeline pipeline) - //{ - // return entityDiff.Entities; - //} - - public override void BeforeImplicitUpdateRelationship(IRelationshipsDictionary resourcesByRelationship, ResourcePipeline pipeline) + public override void BeforeImplicitUpdateRelationship(IRelationshipsDictionary entitiesByRelationship, ResourcePipeline pipeline) { - resourcesByRelationship.GetByRelationship().ToList().ForEach(kvp => DisallowLocked(kvp.Value)); + entitiesByRelationship.GetByRelationship().ToList().ForEach(kvp => DisallowLocked(kvp.Value)); } } } diff --git a/src/Examples/JsonApiDotNetCoreExample/Resources/TodoResource.cs b/src/Examples/JsonApiDotNetCoreExample/Resources/TodoResource.cs index 02a7ba6f15..cfba9855d3 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Resources/TodoResource.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Resources/TodoResource.cs @@ -7,7 +7,7 @@ namespace JsonApiDotNetCoreExample.Resources { - public class TodoResource : LockableResourceBase + public class TodoResource : LockableResource { public TodoResource(IResourceGraph graph) : base(graph) { } diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index 889e2f7198..3b37b8d748 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -376,7 +376,7 @@ public virtual async Task DeleteAsync(TId id) var entity = await GetAsync(id); if (entity == null) return false; _dbSet.Remove(entity); - await _context.SaveChangesAsync(); + await _context.SaveChangesAsync(); return true; } diff --git a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs index 9a30bf41e5..ca7e1aab83 100644 --- a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs +++ b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs @@ -88,7 +88,11 @@ public virtual IEnumerable BeforeDelete(IEnumerable e node.Reassign(entities); } - foreach (var entry in node.PrincipalsToNextLayerByType()) + /// If we're deleting an article, we're implicitly affected any owners related to it. + /// Here we're loading all relations onto the to-be-deleted article + /// if for that relation the BeforeImplicitUpdateHook is implemented, + /// and this hook is then executed + foreach (var entry in node.PrincipalsToNextLayerByRelationships()) { var dependentType = entry.Key; var implicitTargets = entry.Value; diff --git a/src/JsonApiDotNetCore/Hooks/Traversal/RootNode.cs b/src/JsonApiDotNetCore/Hooks/Traversal/RootNode.cs index a7965f1ec9..7783d041e1 100644 --- a/src/JsonApiDotNetCore/Hooks/Traversal/RootNode.cs +++ b/src/JsonApiDotNetCore/Hooks/Traversal/RootNode.cs @@ -13,13 +13,15 @@ namespace JsonApiDotNetCore.Hooks ///
internal class RootNode : IEntityNode where TEntity : class, IIdentifiable { + private readonly RelationshipProxy[] _allRelationshipsToNextLayer; private HashSet _uniqueEntities; public Type EntityType { get; internal set; } public IEnumerable UniqueEntities { get { return _uniqueEntities; } } - public RelationshipProxy[] RelationshipsToNextLayer { get; private set; } - public Dictionary> PrincipalsToNextLayerByType() + public RelationshipProxy[] RelationshipsToNextLayer { get; } + + public Dictionary> PrincipalsToNextLayerByRelationships() { - return RelationshipsToNextLayer + return _allRelationshipsToNextLayer .GroupBy(proxy => proxy.DependentType) .ToDictionary(gdc => gdc.Key, gdc => gdc.ToDictionary(p => p.Attribute, p => UniqueEntities)); } @@ -37,11 +39,12 @@ public Dictionary PrincipalsToNextLayer() /// public IRelationshipsFromPreviousLayer RelationshipsFromPreviousLayer { get { return null; } } - public RootNode(IEnumerable uniqueEntities, RelationshipProxy[] relationships) + public RootNode(IEnumerable uniqueEntities, RelationshipProxy[] poplatedRelationships, RelationshipProxy[] allRelationships) { EntityType = typeof(TEntity); _uniqueEntities = new HashSet(uniqueEntities); - RelationshipsToNextLayer = relationships; + RelationshipsToNextLayer = poplatedRelationships; + _allRelationshipsToNextLayer = allRelationships; } /// diff --git a/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs b/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs index fcffedf16b..5061cce9bf 100644 --- a/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs +++ b/src/JsonApiDotNetCore/Hooks/Traversal/TraversalHelper.cs @@ -57,8 +57,9 @@ public RootNode CreateRootNode(IEnumerable rootEntiti _processedEntities = new Dictionary>(); RegisterRelationshipProxies(typeof(TEntity)); var uniqueEntities = ProcessEntities(rootEntities); - var relationshipsToNextLayer = GetPopulatedRelationships(typeof(TEntity), uniqueEntities.Cast()); - return new RootNode(uniqueEntities, relationshipsToNextLayer); + var populatedRelationshipsToNextLayer = GetPopulatedRelationships(typeof(TEntity), uniqueEntities.Cast()); + var allRelationshipsFromType = RelationshipProxies.Select(entry => entry.Value).Where(proxy => proxy.PrincipalType == typeof(TEntity)).ToArray(); + return new RootNode(uniqueEntities, populatedRelationshipsToNextLayer, allRelationshipsFromType); } /// @@ -108,7 +109,6 @@ public EntityChildLayer CreateNextLayer(IEnumerable nodes) return new EntityChildLayer(nextNodes); } - /// /// iterates throug the dictinary and groups the values /// by matching dependent type of the keys (which are relationshipattributes) From 582bb244d04e0c0e7cd32e2309694da588ef3a18 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 25 Jun 2019 11:00:50 +0200 Subject: [PATCH 08/12] chore: fix spacing in test file --- src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs index ca7e1aab83..30bb51560d 100644 --- a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs +++ b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs @@ -59,7 +59,6 @@ public virtual IEnumerable BeforeUpdate(IEnumerable e return entities; } - /// public virtual IEnumerable BeforeCreate(IEnumerable entities, ResourcePipeline pipeline) where TEntity : class, IIdentifiable { @@ -366,7 +365,6 @@ void ValidateHookResponse(IEnumerable returnedList, ResourcePipeline pipel } } - /// /// A helper method to call a hook on reflectively. /// @@ -428,7 +426,6 @@ HashSet GetAllowedEntities(IEnumerable source, IEnumerable(source.Cast().Where(ue => allowedIds.Contains(ue.StringId))); } - /// /// given the set of , it will load all the /// values from the database of these entites. @@ -447,7 +444,6 @@ IEnumerable LoadDbValues(Type entityType, IEnumerable uniqueEntities, ResourceHo return _executorHelper.LoadDbValues(entityType, uniqueEntities, targetHook, relationshipsToNextLayer); } - /// /// Fires the AfterUpdateRelationship hook /// From 273cb2bc87569a23434c33ac896afcec2de5d384 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 25 Jun 2019 11:10:36 +0200 Subject: [PATCH 09/12] Update DefaultEntityRepository.cs --- src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index 3b37b8d748..889e2f7198 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -376,7 +376,7 @@ public virtual async Task DeleteAsync(TId id) var entity = await GetAsync(id); if (entity == null) return false; _dbSet.Remove(entity); - await _context.SaveChangesAsync(); + await _context.SaveChangesAsync(); return true; } From e144365be833d36e587f9fbf77bad4e78ed435c0 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 25 Jun 2019 11:11:30 +0200 Subject: [PATCH 10/12] fix: spacing --- .../Hooks/Execution/RelationshipsDictionary.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs index 966d0ab867..b6125b40d3 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs @@ -7,15 +7,11 @@ namespace JsonApiDotNetCore.Hooks { - - - /// /// A dummy interface used internally by the hook executor. /// public interface IRelationshipsDictionary { } - /// /// An interface that is implemented to expose a relationship dictionary on another class. /// From 0cf7f9ea8129d4710260a8e9e30d3e7dfa0d5ac0 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 25 Jun 2019 11:12:29 +0200 Subject: [PATCH 11/12] fix: more spacing --- test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs index 36dd8c7fbf..39513d05b3 100644 --- a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs +++ b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs @@ -13,12 +13,10 @@ public class ToOne : Identifiable { } public class AffectedEntitiesHelperTests { - public readonly HasOneAttribute FirstToOneAttr; public readonly HasOneAttribute SecondToOneAttr; public readonly HasManyAttribute ToManyAttr; - public readonly Dictionary> Relationships = new Dictionary>(); public readonly HashSet FirstToOnesEntities = new HashSet { new Dummy() { Id = 1 }, new Dummy() { Id = 2 }, new Dummy() { Id = 3 } }; public readonly HashSet SecondToOnesEntities = new HashSet { new Dummy() { Id = 4 }, new Dummy() { Id = 5 }, new Dummy() { Id = 6 } }; @@ -132,7 +130,6 @@ public void EntityDiff_Loops_Over_Diffs() Assert.Contains(diff.Entity, AllEntities); Assert.Contains(diff.DatabaseValue, dbEntities); } - } private void AssertRelationshipDictionaryGetters(Dictionary> relationshipsDictionary, @@ -161,6 +158,5 @@ private void AssertRelationshipDictionaryGetters(Dictionary Date: Tue, 25 Jun 2019 11:14:16 +0200 Subject: [PATCH 12/12] fix: more spacing --- test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs b/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs index 27e413de4c..17065d3bec 100644 --- a/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs +++ b/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs @@ -130,7 +130,6 @@ protected List CreateTodoWithOwner() var articles = new List
() { articleTagsSubset, articleWithAllTags }; return (articles, allJoins, allTags); } - } public class HooksTestsSetup : HooksDummyData @@ -169,7 +168,6 @@ public class HooksTestsSetup : HooksDummyData // mocking the GenericProcessorFactory and JsonApiContext and wiring them up. (var context, var processorFactory) = CreateContextAndProcessorMocks(); - var dbContext = repoDbContextOptions != null ? new AppDbContext(repoDbContextOptions) : null; SetupProcessorFactoryForResourceDefinition(processorFactory, mainResource.Object, mainDiscovery, context.Object, dbContext); @@ -276,12 +274,10 @@ void MockHooks(Mock> resourceDefinition) resourceDefinition .Setup(rd => rd.BeforeImplicitUpdateRelationship(It.IsAny>(), It.IsAny())) .Verifiable(); - resourceDefinition .Setup(rd => rd.OnReturn(It.IsAny>(), It.IsAny())) .Returns, ResourcePipeline>((entities, context) => entities) .Verifiable(); - resourceDefinition .Setup(rd => rd.AfterCreate(It.IsAny>(), It.IsAny())) .Verifiable(); @@ -294,8 +290,6 @@ void MockHooks(Mock> resourceDefinition) resourceDefinition .Setup(rd => rd.AfterDelete(It.IsAny>(), It.IsAny(), It.IsAny())) .Verifiable(); - - } (Mock, Mock) CreateContextAndProcessorMocks()