From 6f551c3d28d59e654e1ea7704d256f868ed40e1f Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 5 Jul 2019 17:54:10 +0200 Subject: [PATCH 1/5] feat: introduced EntityHashSetDiff instead of EntityDiff --- .../{EntityDiffs.cs => EntityHashSetDiff.cs} | 42 +++++++------------ .../Hooks/IResourceHookContainer.cs | 4 +- .../Hooks/ResourceHookExecutor.cs | 2 +- .../Models/ResourceDefinition.cs | 2 +- .../AffectedEntitiesHelperTests.cs | 4 +- .../Update/BeforeUpdateTests.cs | 4 +- .../Update/BeforeUpdate_WithDbValues_Tests.cs | 12 +++--- .../ResourceHooks/ResourceHooksTestsSetup.cs | 4 +- 8 files changed, 30 insertions(+), 44 deletions(-) rename src/JsonApiDotNetCore/Hooks/Execution/{EntityDiffs.cs => EntityHashSetDiff.cs} (63%) diff --git a/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs b/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSetDiff.cs similarity index 63% rename from src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs rename to src/JsonApiDotNetCore/Hooks/Execution/EntityHashSetDiff.cs index fb30c26c7d..25ea63892f 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSetDiff.cs @@ -1,7 +1,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Linq; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Models; @@ -15,36 +14,27 @@ namespace JsonApiDotNetCore.Hooks /// Also contains information about updated relationships through /// implementation of IRelationshipsDictionary> /// - public interface IEntityDiffs : IEnumerable> where TResource : class, IIdentifiable + public interface IEntityHashSetDiff : IEntityHashSet where TResource : class, IIdentifiable { /// - /// The database values of the resources affected by the request. + /// Iterates over diffs, which is the affected entity from the request + /// with their associated current value from the database. /// - HashSet DatabaseValues { get; } - - /// - /// The resources that were affected by the request. - /// - EntityHashSet Entities { get; } - + IEnumerator> GetDiffs(); + } /// - public class EntityDiffs : IEntityDiffs where TResource : class, IIdentifiable + public class EntityHashSetDiff : EntityHashSet, IEntityHashSetDiff where TResource : class, IIdentifiable { - /// - public HashSet DatabaseValues { get => _databaseValues ?? ThrowNoDbValuesError(); } - /// - public EntityHashSet Entities { get; private set; } - private readonly HashSet _databaseValues; private readonly bool _databaseValuesLoaded; - public EntityDiffs(HashSet requestEntities, + public EntityHashSetDiff(HashSet requestEntities, HashSet databaseEntities, - Dictionary> relationships) + Dictionary> relationships) + : base(requestEntities, relationships) { - Entities = new EntityHashSet(requestEntities, relationships); _databaseValues = databaseEntities; _databaseValuesLoaded |= _databaseValues != null; } @@ -52,31 +42,27 @@ public EntityDiffs(HashSet requestEntities, /// /// Used internally by the ResourceHookExecutor to make live a bit easier with generics /// - internal EntityDiffs(IEnumerable requestEntities, + internal EntityHashSetDiff(IEnumerable requestEntities, IEnumerable databaseEntities, Dictionary relationships) : this((HashSet)requestEntities, (HashSet)databaseEntities, TypeHelper.ConvertRelationshipDictionary(relationships)) { } /// - public IEnumerator> GetEnumerator() + public IEnumerator> GetDiffs() { if (!_databaseValuesLoaded) ThrowNoDbValuesError(); - foreach (var entity in Entities) + foreach (var entity in this) { - TResource currentValueInDatabase = null; - currentValueInDatabase = _databaseValues.Single(e => entity.StringId == e.StringId); + TResource currentValueInDatabase = _databaseValues.Single(e => entity.StringId == e.StringId); yield return new EntityDiffPair(entity, currentValueInDatabase); } } - /// - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - private HashSet ThrowNoDbValuesError() { - throw new MemberAccessException("Cannot access database entities if the LoadDatabaseValues option is set to false"); + throw new MemberAccessException("Cannot iterate over the diffs if the LoadDatabaseValues option is set to false"); } } diff --git a/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs b/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs index 75b52cbddd..c6ae73776a 100644 --- a/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs +++ b/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs @@ -59,7 +59,7 @@ public interface IBeforeHooks where TResource : class, IIdentifiable /// multiple entities. /// /// The returned may be a subset - /// of the property in parameter , + /// of the property in parameter , /// in which case the operation of the pipeline will not be executed /// for the omitted entities. The returned set may also contain custom /// changes of the properties on the entities. @@ -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(IEntityDiffs entityDiff, ResourcePipeline pipeline); + IEnumerable BeforeUpdate(IEntityHashSetDiff entityDiff, ResourcePipeline pipeline); /// /// Implement this hook to run custom logic in the diff --git a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs index 30bb51560d..e0cddb1a03 100644 --- a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs +++ b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs @@ -49,7 +49,7 @@ public virtual IEnumerable BeforeUpdate(IEnumerable e { var relationships = node.RelationshipsToNextLayer.Select(p => p.Attribute).ToArray(); var dbValues = LoadDbValues(typeof(TEntity), (IEnumerable)node.UniqueEntities, ResourceHook.BeforeUpdate, relationships); - var diff = new EntityDiffs(node.UniqueEntities, dbValues, node.PrincipalsToNextLayer()); + var diff = new EntityHashSetDiff(node.UniqueEntities, dbValues, node.PrincipalsToNextLayer()); IEnumerable updated = container.BeforeUpdate(diff, pipeline); node.UpdateUnique(updated); node.Reassign(entities); diff --git a/src/JsonApiDotNetCore/Models/ResourceDefinition.cs b/src/JsonApiDotNetCore/Models/ResourceDefinition.cs index 0a6b0ac091..9e308de55a 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(IEntityDiffs entityDiff, ResourcePipeline pipeline) { return entityDiff.Entities; } + public virtual IEnumerable BeforeUpdate(IEntityHashSetDiff 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 index 39513d05b3..a941e72d5f 100644 --- a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs +++ b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs @@ -87,7 +87,7 @@ 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); + EntityHashSetDiff diffs = new EntityHashSetDiff(AllEntities, dbEntities, Relationships); // act Dictionary> toOnes = diffs.Entities.GetByRelationship(); @@ -120,7 +120,7 @@ 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); + EntityHashSetDiff diffs = new EntityHashSetDiff(AllEntities, dbEntities, Relationships); // Assert & act foreach (EntityDiffPair diff in diffs) diff --git a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdateTests.cs b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdateTests.cs index f02acda264..6f015aa3fc 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 fca4d9c480..4f76eb1c54 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) => TodoCheckDiff(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) => TodoCheckDiff(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) => TodoCheckDiff(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) => TodoCheckDiff(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,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) => TodoCheckDiff(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 TodoCheckDiff(IEntityDiffs diff, string checksum) + private bool TodoCheckDiff(IEntityHashSetDiff 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 17065d3bec..318ea89441 100644 --- a/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs +++ b/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs @@ -260,8 +260,8 @@ void MockHooks(Mock> resourceDefinition) .Setup(rd => rd.BeforeRead(It.IsAny(), It.IsAny(), It.IsAny())) .Verifiable(); resourceDefinition - .Setup(rd => rd.BeforeUpdate(It.IsAny>(), It.IsAny())) - .Returns, ResourcePipeline>((entityDiff, context) => entityDiff.Entities) + .Setup(rd => rd.BeforeUpdate(It.IsAny>(), It.IsAny())) + .Returns, ResourcePipeline>((entityDiff, context) => entityDiff.Entities) .Verifiable(); resourceDefinition .Setup(rd => rd.BeforeDelete(It.IsAny>(), It.IsAny())) From 8648fe54024f2c4018fedafdf11e194418b04509 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 5 Jul 2019 18:05:33 +0200 Subject: [PATCH 2/5] chore: fixed test --- ...HashSetDiff.cs => DiffableEntityHashSet.cs} | 12 ++++++------ .../Hooks/IResourceHookContainer.cs | 8 ++++---- .../Hooks/ResourceHookExecutor.cs | 2 +- .../Models/ResourceDefinition.cs | 2 +- .../AffectedEntitiesHelperTests.cs | 18 +++++++++--------- .../Update/BeforeUpdateTests.cs | 4 ++-- .../Update/BeforeUpdate_WithDbValues_Tests.cs | 16 ++++++++-------- .../ResourceHooks/ResourceHooksTestsSetup.cs | 4 ++-- 8 files changed, 33 insertions(+), 33 deletions(-) rename src/JsonApiDotNetCore/Hooks/Execution/{EntityHashSetDiff.cs => DiffableEntityHashSet.cs} (85%) diff --git a/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSetDiff.cs b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs similarity index 85% rename from src/JsonApiDotNetCore/Hooks/Execution/EntityHashSetDiff.cs rename to src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs index 25ea63892f..9aac4b1fd9 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSetDiff.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs @@ -14,23 +14,23 @@ namespace JsonApiDotNetCore.Hooks /// Also contains information about updated relationships through /// implementation of IRelationshipsDictionary> /// - public interface IEntityHashSetDiff : IEntityHashSet where TResource : class, IIdentifiable + public interface IDiffableEntityHashSet : IEntityHashSet where TResource : class, IIdentifiable { /// /// Iterates over diffs, which is the affected entity from the request /// with their associated current value from the database. /// - IEnumerator> GetDiffs(); + IEnumerable> GetDiffs(); } /// - public class EntityHashSetDiff : EntityHashSet, IEntityHashSetDiff where TResource : class, IIdentifiable + public class DiffableEntityHashSet : EntityHashSet, IDiffableEntityHashSet where TResource : class, IIdentifiable { private readonly HashSet _databaseValues; private readonly bool _databaseValuesLoaded; - public EntityHashSetDiff(HashSet requestEntities, + public DiffableEntityHashSet(HashSet requestEntities, HashSet databaseEntities, Dictionary> relationships) : base(requestEntities, relationships) @@ -42,14 +42,14 @@ public EntityHashSetDiff(HashSet requestEntities, /// /// Used internally by the ResourceHookExecutor to make live a bit easier with generics /// - internal EntityHashSetDiff(IEnumerable requestEntities, + internal DiffableEntityHashSet(IEnumerable requestEntities, IEnumerable databaseEntities, Dictionary relationships) : this((HashSet)requestEntities, (HashSet)databaseEntities, TypeHelper.ConvertRelationshipDictionary(relationships)) { } /// - public IEnumerator> GetDiffs() + public IEnumerable> GetDiffs() { if (!_databaseValuesLoaded) ThrowNoDbValuesError(); diff --git a/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs b/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs index c6ae73776a..38d296f8f0 100644 --- a/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs +++ b/src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs @@ -54,12 +54,12 @@ public interface IBeforeHooks where TResource : class, IIdentifiable /// layer just before updating entities of type . /// /// For the pipeline, the - /// will typically contain one entity. + /// will typically contain one entity. /// For , this it may contain /// multiple entities. /// /// The returned may be a subset - /// of the property in parameter , + /// of the property in parameter , /// in which case the operation of the pipeline will not be executed /// for the omitted entities. The returned set may also contain custom /// changes of the properties on the entities. @@ -75,9 +75,9 @@ public interface IBeforeHooks where TResource : class, IIdentifiable /// hook is fired for these. /// /// The transformed entity set - /// The entity diff. + /// The affected entities. /// An enum indicating from where the hook was triggered. - IEnumerable BeforeUpdate(IEntityHashSetDiff entityDiff, ResourcePipeline pipeline); + IEnumerable BeforeUpdate(IDiffableEntityHashSet entities, ResourcePipeline pipeline); /// /// Implement this hook to run custom logic in the diff --git a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs index e0cddb1a03..1b5c4fa4ef 100644 --- a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs +++ b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs @@ -49,7 +49,7 @@ public virtual IEnumerable BeforeUpdate(IEnumerable e { var relationships = node.RelationshipsToNextLayer.Select(p => p.Attribute).ToArray(); var dbValues = LoadDbValues(typeof(TEntity), (IEnumerable)node.UniqueEntities, ResourceHook.BeforeUpdate, relationships); - var diff = new EntityHashSetDiff(node.UniqueEntities, dbValues, node.PrincipalsToNextLayer()); + var diff = new DiffableEntityHashSet(node.UniqueEntities, dbValues, node.PrincipalsToNextLayer()); IEnumerable updated = container.BeforeUpdate(diff, pipeline); node.UpdateUnique(updated); node.Reassign(entities); diff --git a/src/JsonApiDotNetCore/Models/ResourceDefinition.cs b/src/JsonApiDotNetCore/Models/ResourceDefinition.cs index 9e308de55a..2939bc40d1 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(IEntityHashSetDiff entityDiff, ResourcePipeline pipeline) { return entityDiff.Entities; } + public virtual IEnumerable BeforeUpdate(IDiffableEntityHashSet entities, ResourcePipeline pipeline) { return 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 index a941e72d5f..75fce33390 100644 --- a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs +++ b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs @@ -87,13 +87,13 @@ public void EntityDiff_GetByRelationships() { // arrange var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id }).ToList()); - EntityHashSetDiff diffs = new EntityHashSetDiff(AllEntities, dbEntities, Relationships); + DiffableEntityHashSet diffs = new DiffableEntityHashSet(AllEntities, dbEntities, Relationships); // act - Dictionary> toOnes = diffs.Entities.GetByRelationship(); - Dictionary> toManies = diffs.Entities.GetByRelationship(); - Dictionary> notTargeted = diffs.Entities.GetByRelationship(); - Dictionary> allRelationships = diffs.Entities.AffectedRelationships; + Dictionary> toOnes = diffs.GetByRelationship(); + Dictionary> toManies = diffs.GetByRelationship(); + Dictionary> notTargeted = diffs.GetByRelationship(); + Dictionary> allRelationships = diffs.AffectedRelationships; // Assert AssertRelationshipDictionaryGetters(allRelationships, toOnes, toManies, notTargeted); @@ -103,12 +103,12 @@ public void EntityDiff_GetByRelationships() Assert.DoesNotContain(e, allEntitiesWithAffectedRelationships); }); - var requestEntitiesFromDiff = diffs.Entities; + var requestEntitiesFromDiff = diffs; requestEntitiesFromDiff.ToList().ForEach(e => { Assert.Contains(e, AllEntities); }); - var databaseEntitiesFromDiff = diffs.DatabaseValues; + var databaseEntitiesFromDiff = diffs.GetDiffs().Select( d => d.DatabaseValue); databaseEntitiesFromDiff.ToList().ForEach(e => { Assert.Contains(e, dbEntities); @@ -120,10 +120,10 @@ public void EntityDiff_Loops_Over_Diffs() { // arrange var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id })); - EntityHashSetDiff diffs = new EntityHashSetDiff(AllEntities, dbEntities, Relationships); + DiffableEntityHashSet diffs = new DiffableEntityHashSet(AllEntities, dbEntities, Relationships); // Assert & act - foreach (EntityDiffPair diff in diffs) + foreach (EntityDiffPair diff in diffs.GetDiffs()) { Assert.Equal(diff.Entity.Id, diff.DatabaseValue.Id); Assert.NotEqual(diff.Entity, diff.DatabaseValue); diff --git a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdateTests.cs b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdateTests.cs index 6f015aa3fc..807dc38b18 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 4f76eb1c54..d383a2694b 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) => TodoCheckDiff(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) => TodoCheckDiff(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) => TodoCheckDiff(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) => TodoCheckDiff(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,18 +204,18 @@ public void BeforeUpdate_NoImplicit_Without_Child_Hook_Implemented() hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch); // assert - todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is>((diff) => TodoCheckDiff(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 TodoCheckDiff(IEntityHashSetDiff diff, string checksum) + private bool TodoCheckDiff(IDiffableEntityHashSet entities, string checksum) { - var diffPair = diff.Single(); + var diffPair = entities.GetDiffs().Single(); var dbCheck = diffPair.DatabaseValue.Description == checksum; var reqCheck = diffPair.Entity.Description == null; var diffPairCheck = (dbCheck && reqCheck); - var updatedRelationship = diff.Entities.GetByRelationship().Single(); + var updatedRelationship = entities.GetByRelationship().Single(); var diffcheck = updatedRelationship.Key.PublicRelationshipName == "one-to-one-person"; return (dbCheck && reqCheck && diffcheck); diff --git a/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs b/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs index 318ea89441..454a2e225f 100644 --- a/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs +++ b/test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs @@ -260,8 +260,8 @@ void MockHooks(Mock> resourceDefinition) .Setup(rd => rd.BeforeRead(It.IsAny(), It.IsAny(), It.IsAny())) .Verifiable(); resourceDefinition - .Setup(rd => rd.BeforeUpdate(It.IsAny>(), It.IsAny())) - .Returns, ResourcePipeline>((entityDiff, context) => entityDiff.Entities) + .Setup(rd => rd.BeforeUpdate(It.IsAny>(), It.IsAny())) + .Returns, ResourcePipeline>((entities, context) => entities) .Verifiable(); resourceDefinition .Setup(rd => rd.BeforeDelete(It.IsAny>(), It.IsAny())) From 6ee367903632eb4aa18c004f82bab792721b2833 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Sun, 14 Jul 2019 19:06:03 +0200 Subject: [PATCH 3/5] feat: GetAffected syntax in hook helpers --- .../Properties/launchSettings.json | 5 +- .../Properties/launchSettings.json | 2 +- .../Properties/launchSettings.json | 3 +- .../Properties/launchSettings.json | 41 ++++----- .../IServiceCollectionExtensions.cs | 23 ++--- .../Hooks/Execution/DiffableEntityHashSet.cs | 36 +++++++- .../Hooks/Execution/EntityHashSet.cs | 11 ++- .../Execution/RelationshipsDictionary.cs | 36 +++++--- .../Hooks/ResourceHookExecutor.cs | 2 +- src/JsonApiDotNetCore/Internal/TypeHelper.cs | 47 +++++++++- .../AffectedEntitiesHelperTests.cs | 89 +++++++++++++++++-- .../Update/BeforeUpdate_WithDbValues_Tests.cs | 12 +-- 12 files changed, 236 insertions(+), 71 deletions(-) diff --git a/src/Examples/GettingStarted/Properties/launchSettings.json b/src/Examples/GettingStarted/Properties/launchSettings.json index 7dc07f5b4f..a0f317edf0 100644 --- a/src/Examples/GettingStarted/Properties/launchSettings.json +++ b/src/Examples/GettingStarted/Properties/launchSettings.json @@ -17,7 +17,8 @@ }, "GettingStarted": { "commandName": "Project", - "launchBrowser": true + "launchBrowser": true, + "environmentVariables": {} } } -} +} \ No newline at end of file diff --git a/src/Examples/JsonApiDotNetCoreExample/Properties/launchSettings.json b/src/Examples/JsonApiDotNetCoreExample/Properties/launchSettings.json index 6d991610f0..0daa3352d1 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Properties/launchSettings.json +++ b/src/Examples/JsonApiDotNetCoreExample/Properties/launchSettings.json @@ -25,4 +25,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Examples/NoEntityFrameworkExample/Properties/launchSettings.json b/src/Examples/NoEntityFrameworkExample/Properties/launchSettings.json index b78d7494b8..310f04da95 100644 --- a/src/Examples/NoEntityFrameworkExample/Properties/launchSettings.json +++ b/src/Examples/NoEntityFrameworkExample/Properties/launchSettings.json @@ -16,7 +16,8 @@ } }, "NoEntityFrameworkExample": { - "commandName": "Project" + "commandName": "Project", + "environmentVariables": {} } } } \ No newline at end of file diff --git a/src/Examples/ResourceEntitySeparationExample/Properties/launchSettings.json b/src/Examples/ResourceEntitySeparationExample/Properties/launchSettings.json index 49b260ed41..a51fc0dc79 100644 --- a/src/Examples/ResourceEntitySeparationExample/Properties/launchSettings.json +++ b/src/Examples/ResourceEntitySeparationExample/Properties/launchSettings.json @@ -1,23 +1,24 @@ { - "iisSettings": { - "windowsAuthentication": false, - "anonymousAuthentication": true, - "iisExpress": { - "applicationUrl": "http://localhost:57181/", - "sslPort": 0 - } + "iisSettings": { + "windowsAuthentication": false, + "anonymousAuthentication": true, + "iisExpress": { + "applicationUrl": "http://localhost:57181/", + "sslPort": 0 + } + }, + "profiles": { + "IIS Express": { + "commandName": "IISExpress", + "launchBrowser": true, + "launchUrl": "api/v1/students", + "environmentVariables": { + "ASPNETCORE_ENVIRONMENT": "Development" + } }, - "profiles": { - "IIS Express": { - "commandName": "IISExpress", - "launchBrowser": true, - "launchUrl": "api/v1/students", - "environmentVariables": { - "ASPNETCORE_ENVIRONMENT": "Development" - } - }, - "ResourceEntitySeparationExample": { - "commandName": "Project" - } + "ResourceEntitySeparationExample": { + "commandName": "Project", + "environmentVariables": {} } -} + } +} \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs b/src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs index 6c3779cbb3..cb1e635c7a 100644 --- a/src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs @@ -64,7 +64,7 @@ public static IServiceCollection AddJsonApi( var config = new JsonApiOptions(); configureOptions(config); - if(autoDiscover != null) + if (autoDiscover != null) { var facade = new ServiceDiscoveryFacade(services, config.ResourceGraphBuilder); autoDiscover(facade); @@ -159,7 +159,7 @@ public static void AddJsonApiInternals( services.AddScoped(); services.AddScoped(); - + if (jsonApiOptions.EnableResourceHooks) { services.AddSingleton(typeof(IHooksDiscovery<>), typeof(HooksDiscovery<>)); @@ -204,7 +204,7 @@ public static void SerializeAsJsonApi(this MvcOptions options, JsonApiOptions js /// Adds all required registrations for the service to the container /// /// - public static IServiceCollection AddResourceService(this IServiceCollection services) + public static IServiceCollection AddResourceService(this IServiceCollection services) { var typeImplementsAnExpectedInterface = false; @@ -213,28 +213,29 @@ public static IServiceCollection AddResourceService(this IServiceCollection s // it is _possible_ that a single concrete type could be used for multiple resources... var resourceDescriptors = GetResourceTypesFromServiceImplementation(serviceImplementationType); - foreach(var resourceDescriptor in resourceDescriptors) + foreach (var resourceDescriptor in resourceDescriptors) { - foreach(var openGenericType in ServiceDiscoveryFacade.ServiceInterfaces) + foreach (var openGenericType in ServiceDiscoveryFacade.ServiceInterfaces) { // A shorthand interface is one where the id type is ommitted // e.g. IResourceService is the shorthand for IResourceService var isShorthandInterface = (openGenericType.GetTypeInfo().GenericTypeParameters.Length == 1); - if(isShorthandInterface && resourceDescriptor.IdType != typeof(int)) + if (isShorthandInterface && resourceDescriptor.IdType != typeof(int)) continue; // we can't create a shorthand for id types other than int var concreteGenericType = isShorthandInterface ? openGenericType.MakeGenericType(resourceDescriptor.ResourceType) : openGenericType.MakeGenericType(resourceDescriptor.ResourceType, resourceDescriptor.IdType); - if(concreteGenericType.IsAssignableFrom(serviceImplementationType)) { + if (concreteGenericType.IsAssignableFrom(serviceImplementationType)) + { services.AddScoped(concreteGenericType, serviceImplementationType); typeImplementsAnExpectedInterface = true; } } } - if(typeImplementsAnExpectedInterface == false) + if (typeImplementsAnExpectedInterface == false) throw new JsonApiSetupException($"{serviceImplementationType} does not implement any of the expected JsonApiDotNetCore interfaces."); return services; @@ -244,12 +245,12 @@ private static HashSet GetResourceTypesFromServiceImplementa { var resourceDecriptors = new HashSet(); var interfaces = type.GetInterfaces(); - foreach(var i in interfaces) + foreach (var i in interfaces) { - if(i.IsGenericType) + if (i.IsGenericType) { var firstGenericArgument = i.GenericTypeArguments.FirstOrDefault(); - if(TypeLocator.TryGetResourceDescriptor(firstGenericArgument, out var resourceDescriptor) == true) + if (TypeLocator.TryGetResourceDescriptor(firstGenericArgument, out var resourceDescriptor) == true) { resourceDecriptors.Add(resourceDescriptor); } diff --git a/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs index 9aac4b1fd9..160617dd92 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs @@ -2,8 +2,12 @@ using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Linq.Expressions; +using System.Reflection; +using JsonApiDotNetCore.Extensions; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Models; +using JsonApiDotNetCore.Services; namespace JsonApiDotNetCore.Hooks { @@ -21,7 +25,7 @@ public interface IDiffableEntityHashSet : IEntityHashSet w /// with their associated current value from the database. /// IEnumerable> GetDiffs(); - + } /// @@ -29,14 +33,17 @@ public class DiffableEntityHashSet : EntityHashSet, IDiffa { private readonly HashSet _databaseValues; private readonly bool _databaseValuesLoaded; + private Dictionary> _updatedAttributes; public DiffableEntityHashSet(HashSet requestEntities, HashSet databaseEntities, - Dictionary> relationships) + Dictionary> relationships, + Dictionary> updatedAttributes) : base(requestEntities, relationships) { _databaseValues = databaseEntities; _databaseValuesLoaded |= _databaseValues != null; + _updatedAttributes = updatedAttributes; } /// @@ -44,8 +51,11 @@ public DiffableEntityHashSet(HashSet requestEntities, /// internal DiffableEntityHashSet(IEnumerable requestEntities, IEnumerable databaseEntities, - Dictionary relationships) - : this((HashSet)requestEntities, (HashSet)databaseEntities, TypeHelper.ConvertRelationshipDictionary(relationships)) { } + Dictionary relationships, + IJsonApiContext jsonApiContext) + : this((HashSet)requestEntities, (HashSet)databaseEntities, TypeHelper.ConvertRelationshipDictionary(relationships), + TypeHelper.ConvertAttributesToUpdate(jsonApiContext.AttributesToUpdate, (HashSet)requestEntities)) + { } /// @@ -60,6 +70,24 @@ public IEnumerable> GetDiffs() } } + /// + public new HashSet GetAffected(Expression> NavigationAction) + { + var propertyInfo = TypeHelper.ParseNavigationExpression(NavigationAction); + var propertyType = propertyInfo.PropertyType; + if (propertyType.Inherits(typeof(IEnumerable))) propertyType = TypeHelper.GetTypeOfList(propertyType); + if (propertyType.Implements()) + { + // the navigation action references a relationship. Redirect the call to the relationship dictionary. + return base.GetAffected(NavigationAction); + } + else if (_updatedAttributes.TryGetValue(propertyInfo, out HashSet entities)) + { + return entities; + } + return new HashSet(); + } + private HashSet ThrowNoDbValuesError() { throw new MemberAccessException("Cannot iterate over the diffs if the LoadDatabaseValues option is set to false"); diff --git a/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs b/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs index 8a97705c39..a6302a40c7 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs @@ -5,6 +5,7 @@ using System; using System.Collections.ObjectModel; using System.Collections.Immutable; +using System.Linq.Expressions; namespace JsonApiDotNetCore.Hooks { @@ -26,7 +27,7 @@ public interface IEntityHashSet : IByAffectedRelationships /// public class EntityHashSet : HashSet, IEntityHashSet where TResource : class, IIdentifiable { - + /// public Dictionary> AffectedRelationships { get => _relationships; } @@ -53,9 +54,15 @@ public Dictionary> GetByRelationship(T } /// - public Dictionary> GetByRelationship() where TRelatedResource : class, IIdentifiable + public Dictionary> GetByRelationship() where TRelatedResource : class, IIdentifiable { return GetByRelationship(typeof(TRelatedResource)); } + + /// + public HashSet GetAffected(Expression> NavigationAction) + { + return _relationships.GetAffected(NavigationAction); + } } } \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs index b6125b40d3..b967e31464 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/RelationshipsDictionary.cs @@ -2,6 +2,8 @@ using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Linq.Expressions; +using System.Reflection; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Models; @@ -15,13 +17,9 @@ public interface IRelationshipsDictionary { } /// /// An interface that is implemented to expose a relationship dictionary on another class. /// - public interface IByAffectedRelationships : + 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. /// @@ -31,10 +29,11 @@ public interface IByAffectedRelationships : /// /// A helper class that provides insights in which relationships have been updated for which entities. /// - public interface IRelationshipsDictionary : - IRelationshipGetters, - IReadOnlyDictionary>, - IRelationshipsDictionary where TDependentResource : class, IIdentifiable { } + public interface IRelationshipsDictionary : + IRelationshipGetters, + IReadOnlyDictionary>, + IRelationshipsDictionary where TDependentResource : class, IIdentifiable + { } /// /// A helper class that provides insights in which relationships have been updated for which entities. @@ -49,8 +48,16 @@ public interface IRelationshipGetters where TResource : class, IIdent /// Gets a dictionary of all entities that have an affected relationship to type /// Dictionary> GetByRelationship(Type relatedResourceType); + + /// + /// Gets a collection of all the entities for the property within + /// has been affected by the request + /// + /// + HashSet GetAffected(Expression> NavigationAction); } + /// /// Implementation of IAffectedRelationships{TDependentResource} /// @@ -58,7 +65,7 @@ public interface IRelationshipGetters where TResource : class, IIdent /// with the two helper methods defined on IAffectedRelationships{TDependentResource}. /// public class RelationshipsDictionary : - Dictionary>, + Dictionary>, IRelationshipsDictionary where TResource : class, IIdentifiable { /// @@ -70,7 +77,7 @@ public RelationshipsDictionary(Dictionary /// Used internally by the ResourceHookExecutor to make live a bit easier with generics /// - internal RelationshipsDictionary(Dictionary relationships) + internal RelationshipsDictionary(Dictionary relationships) : this(TypeHelper.ConvertRelationshipDictionary(relationships)) { } /// @@ -84,5 +91,12 @@ public Dictionary> GetByRelationship(T { return this.Where(p => p.Key.DependentType == relatedType).ToDictionary(p => p.Key, p => p.Value); } + + /// + public HashSet GetAffected(Expression> NavigationAction) + { + var property = TypeHelper.ParseNavigationExpression(NavigationAction); + return this.Where(p => p.Key.InternalRelationshipName == property.Name).Select(p => p.Value).SingleOrDefault(); + } } } diff --git a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs index 1b5c4fa4ef..338fa6ee6b 100644 --- a/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs +++ b/src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs @@ -49,7 +49,7 @@ public virtual IEnumerable BeforeUpdate(IEnumerable e { var relationships = node.RelationshipsToNextLayer.Select(p => p.Attribute).ToArray(); var dbValues = LoadDbValues(typeof(TEntity), (IEnumerable)node.UniqueEntities, ResourceHook.BeforeUpdate, relationships); - var diff = new DiffableEntityHashSet(node.UniqueEntities, dbValues, node.PrincipalsToNextLayer()); + var diff = new DiffableEntityHashSet(node.UniqueEntities, dbValues, node.PrincipalsToNextLayer(), _context); IEnumerable updated = container.BeforeUpdate(diff, pipeline); node.UpdateUnique(updated); node.Reassign(entities); diff --git a/src/JsonApiDotNetCore/Internal/TypeHelper.cs b/src/JsonApiDotNetCore/Internal/TypeHelper.cs index 7326377b3d..4971da18d2 100644 --- a/src/JsonApiDotNetCore/Internal/TypeHelper.cs +++ b/src/JsonApiDotNetCore/Internal/TypeHelper.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Linq.Expressions; using System.Reflection; using JsonApiDotNetCore.Hooks; using JsonApiDotNetCore.Models; @@ -13,7 +14,7 @@ internal static class TypeHelper public static IList ConvertCollection(IEnumerable collection, Type targetType) { var list = Activator.CreateInstance(typeof(List<>).MakeGenericType(targetType)) as IList; - foreach(var item in collection) + foreach (var item in collection) list.Add(ConvertType(item, targetType)); return list; } @@ -43,7 +44,7 @@ public static object ConvertType(object value, Type type) if (type == typeof(DateTimeOffset)) return DateTimeOffset.Parse(stringValue); - if(type == typeof(TimeSpan)) + if (type == typeof(TimeSpan)) return TimeSpan.Parse(stringValue); if (type.GetTypeInfo().IsEnum) @@ -75,11 +76,44 @@ public static Type GetTypeOfList(Type type) { if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(List<>)) { - return type.GetGenericArguments()[0]; + return type.GetGenericArguments()[0]; } return null; } + + + /// + /// Gets the property info that is referenced in the NavigatioAction expression. + /// Credits: https://stackoverflow.com/a/17116267/4441216 + /// + public static PropertyInfo ParseNavigationExpression(Expression> NavigationExpression) + { + MemberExpression Exp = null; + + //this line is necessary, because sometimes the expression comes in as Convert(originalexpression) + if (NavigationExpression.Body is UnaryExpression) + { + var UnExp = (UnaryExpression)NavigationExpression.Body; + if (UnExp.Operand is MemberExpression) + { + Exp = (MemberExpression)UnExp.Operand; + } + else + throw new ArgumentException(); + } + else if (NavigationExpression.Body is MemberExpression) + { + Exp = (MemberExpression)NavigationExpression.Body; + } + else + { + throw new ArgumentException(); + } + + return (PropertyInfo)Exp.Member; + } + /// /// Convert collection of query string params to Collection of concrete Type /// @@ -114,11 +148,16 @@ 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); } + public static Dictionary> ConvertAttributesToUpdate(Dictionary attributes, HashSet entities) + { + return attributes?.ToDictionary(p => p.Key.PropertyInfo, p => entities); + } + /// /// Use this overload if you need to instantiate a type that has a internal constructor /// diff --git a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs index 75fce33390..5f138d2092 100644 --- a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs +++ b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs @@ -3,15 +3,28 @@ using System.Collections.Generic; using Xunit; using System.Linq; +using System.Reflection; namespace UnitTests.ResourceHooks.AffectedEntities { - public class Dummy : Identifiable { } + public class Dummy : Identifiable + { + public string SomeUpdatedProperty { get; set; } + public string SomeNotUpdatedProperty { get; set; } + + + [HasOne("first-to-one")] + public ToOne FirstToOne { get; set; } + [HasOne("second-to-one")] + public ToOne SecondToOne { get; set; } + [HasMany("to-manies")] + public List ToManies { get; set; } + } public class NotTargeted : Identifiable { } public class ToMany : Identifiable { } public class ToOne : Identifiable { } - public class AffectedEntitiesHelperTests + public class RelationshipDictionaryTests { public readonly HasOneAttribute FirstToOneAttr; public readonly HasOneAttribute SecondToOneAttr; @@ -23,22 +36,25 @@ public class AffectedEntitiesHelperTests 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() + public RelationshipDictionaryTests() { FirstToOneAttr = new HasOneAttribute("first-to-one") { PrincipalType = typeof(Dummy), - DependentType = typeof(ToOne) + DependentType = typeof(ToOne), + InternalRelationshipName = "FirstToOne" }; SecondToOneAttr = new HasOneAttribute("second-to-one") { PrincipalType = typeof(Dummy), - DependentType = typeof(ToOne) + DependentType = typeof(ToOne), + InternalRelationshipName = "SecondToOne" }; ToManyAttr = new HasManyAttribute("to-manies") { PrincipalType = typeof(Dummy), - DependentType = typeof(ToMany) + DependentType = typeof(ToMany), + InternalRelationshipName = "ToManies" }; Relationships.Add(FirstToOneAttr, FirstToOnesEntities); Relationships.Add(SecondToOneAttr, SecondToOnesEntities); @@ -61,6 +77,23 @@ public void RelationshipsDictionary_GetByRelationships() AssertRelationshipDictionaryGetters(relationshipsDictionary, toOnes, toManies, notTargeted); } + [Fact] + public void RelationshipsDictionary_GetAffected() + { + // arrange + RelationshipsDictionary relationshipsDictionary = new RelationshipsDictionary(Relationships); + + // act + var affectedThroughFirstToOne = relationshipsDictionary.GetAffected(d => d.FirstToOne).ToList(); + var affectedThroughSecondToOne = relationshipsDictionary.GetAffected(d => d.SecondToOne).ToList(); + var affectedThroughToMany = relationshipsDictionary.GetAffected(d => d.ToManies).ToList(); + + // assert + affectedThroughFirstToOne.ForEach((entitiy) => Assert.Contains(entitiy, FirstToOnesEntities)); + affectedThroughSecondToOne.ForEach((entitiy) => Assert.Contains(entitiy, SecondToOnesEntities)); + affectedThroughToMany.ForEach((entitiy) => Assert.Contains(entitiy, ToManiesEntities)); + } + [Fact] public void EntityHashSet_GetByRelationships() { @@ -87,7 +120,7 @@ public void EntityDiff_GetByRelationships() { // arrange var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id }).ToList()); - DiffableEntityHashSet diffs = new DiffableEntityHashSet(AllEntities, dbEntities, Relationships); + DiffableEntityHashSet diffs = new DiffableEntityHashSet(AllEntities, dbEntities, Relationships, null); // act Dictionary> toOnes = diffs.GetByRelationship(); @@ -108,7 +141,7 @@ public void EntityDiff_GetByRelationships() { Assert.Contains(e, AllEntities); }); - var databaseEntitiesFromDiff = diffs.GetDiffs().Select( d => d.DatabaseValue); + var databaseEntitiesFromDiff = diffs.GetDiffs().Select(d => d.DatabaseValue); databaseEntitiesFromDiff.ToList().ForEach(e => { Assert.Contains(e, dbEntities); @@ -120,7 +153,7 @@ public void EntityDiff_Loops_Over_Diffs() { // arrange var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id })); - DiffableEntityHashSet diffs = new DiffableEntityHashSet(AllEntities, dbEntities, Relationships); + DiffableEntityHashSet diffs = new DiffableEntityHashSet(AllEntities, dbEntities, Relationships, null); // Assert & act foreach (EntityDiffPair diff in diffs.GetDiffs()) @@ -132,6 +165,44 @@ public void EntityDiff_Loops_Over_Diffs() } } + [Fact] + public void EntityDiff_GetAffected_Relationships() + { + // arrange + var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id })); + DiffableEntityHashSet diffs = new DiffableEntityHashSet(AllEntities, dbEntities, Relationships, null); + + // act + var affectedThroughFirstToOne = diffs.GetAffected(d => d.FirstToOne).ToList(); + var affectedThroughSecondToOne = diffs.GetAffected(d => d.SecondToOne).ToList(); + var affectedThroughToMany = diffs.GetAffected(d => d.ToManies).ToList(); + + // assert + affectedThroughFirstToOne.ForEach((entitiy) => Assert.Contains(entitiy, FirstToOnesEntities)); + affectedThroughSecondToOne.ForEach((entitiy) => Assert.Contains(entitiy, SecondToOnesEntities)); + affectedThroughToMany.ForEach((entitiy) => Assert.Contains(entitiy, ToManiesEntities)); + } + + [Fact] + public void EntityDiff_GetAffected_Attributes() + { + // arrange + var dbEntities = new HashSet(AllEntities.Select(e => new Dummy { Id = e.Id })); + var updatedAttributes = new Dictionary> + { + { typeof(Dummy).GetProperty("SomeUpdatedProperty"), AllEntities } + }; + DiffableEntityHashSet diffs = new DiffableEntityHashSet(AllEntities, dbEntities, Relationships, updatedAttributes); + + // act + var affectedThroughSomeUpdatedProperty = diffs.GetAffected(d => d.SomeUpdatedProperty).ToList(); + var affectedThroughSomeNotUpdatedProperty = diffs.GetAffected(d => d.SomeNotUpdatedProperty).ToList(); + + // assert + Assert.NotEmpty(affectedThroughSomeUpdatedProperty); + Assert.Empty(affectedThroughSomeNotUpdatedProperty); + } + private void AssertRelationshipDictionaryGetters(Dictionary> relationshipsDictionary, Dictionary> toOnes, Dictionary> toManies, diff --git a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs index d383a2694b..a13de78f94 100644 --- a/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs +++ b/test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs @@ -70,7 +70,7 @@ public void BeforeUpdate() ResourcePipeline.Patch), Times.Once()); todoResourceMock.Verify(rd => rd.BeforeImplicitUpdateRelationship( - It.Is>( rh => TodoCheck(rh, description + description)), + It.Is>(rh => TodoCheck(rh, description + description)), ResourcePipeline.Patch), Times.Once()); VerifyNoOtherCalls(todoResourceMock, ownerResourceMock); @@ -152,8 +152,8 @@ public void BeforeUpdate_Without_Child_Hook_Implemented() public void BeforeUpdate_NoImplicit() { // arrange - var todoDiscovery = SetDiscoverableHooks(targetHooksNoImplicit, ResourceHook.BeforeUpdate ); - var personDiscovery = SetDiscoverableHooks(targetHooksNoImplicit, ResourceHook.BeforeUpdateRelationship ); + var todoDiscovery = SetDiscoverableHooks(targetHooksNoImplicit, ResourceHook.BeforeUpdate); + var personDiscovery = SetDiscoverableHooks(targetHooksNoImplicit, ResourceHook.BeforeUpdateRelationship); (var contextMock, var hookExecutor, var todoResourceMock, var ownerResourceMock) = CreateTestObjects(todoDiscovery, personDiscovery, repoDbContextOptions: options); @@ -175,7 +175,7 @@ public void BeforeUpdate_NoImplicit_Without_Parent_Hook_Implemented() { // arrange var todoDiscovery = SetDiscoverableHooks(NoHooks, DisableDbValues); - var personDiscovery = SetDiscoverableHooks(targetHooksNoImplicit, ResourceHook.BeforeUpdateRelationship ); + var personDiscovery = SetDiscoverableHooks(targetHooksNoImplicit, ResourceHook.BeforeUpdateRelationship); (var contextMock, var hookExecutor, var todoResourceMock, var ownerResourceMock) = CreateTestObjects(todoDiscovery, personDiscovery, repoDbContextOptions: options); @@ -218,7 +218,9 @@ private bool TodoCheckDiff(IDiffableEntityHashSet entities, string che var updatedRelationship = entities.GetByRelationship().Single(); var diffcheck = updatedRelationship.Key.PublicRelationshipName == "one-to-one-person"; - return (dbCheck && reqCheck && diffcheck); + var getAffectedCheck = entities.GetAffected(e => e.ToOnePerson).Any(); + + return (dbCheck && reqCheck && diffcheck && getAffectedCheck); } private bool TodoCheck(IRelationshipsDictionary rh, string checksum) From 95974a6f064c895e94d97cf9207c278568c31c3e Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Thu, 18 Jul 2019 06:56:59 +0200 Subject: [PATCH 4/5] chore: styling --- .../Hooks/Execution/DiffableEntityHashSet.cs | 2 +- src/JsonApiDotNetCore/Internal/TypeHelper.cs | 6 ++---- test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs index 160617dd92..6c658fe3ba 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs @@ -54,7 +54,7 @@ internal DiffableEntityHashSet(IEnumerable requestEntities, Dictionary relationships, IJsonApiContext jsonApiContext) : this((HashSet)requestEntities, (HashSet)databaseEntities, TypeHelper.ConvertRelationshipDictionary(relationships), - TypeHelper.ConvertAttributesToUpdate(jsonApiContext.AttributesToUpdate, (HashSet)requestEntities)) + TypeHelper.ConvertAttributeDictionary(jsonApiContext.AttributesToUpdate, (HashSet)requestEntities)) { } diff --git a/src/JsonApiDotNetCore/Internal/TypeHelper.cs b/src/JsonApiDotNetCore/Internal/TypeHelper.cs index 4971da18d2..d0d8294682 100644 --- a/src/JsonApiDotNetCore/Internal/TypeHelper.cs +++ b/src/JsonApiDotNetCore/Internal/TypeHelper.cs @@ -4,7 +4,6 @@ using System.Linq; using System.Linq.Expressions; using System.Reflection; -using JsonApiDotNetCore.Hooks; using JsonApiDotNetCore.Models; namespace JsonApiDotNetCore.Internal @@ -81,8 +80,6 @@ public static Type GetTypeOfList(Type type) return null; } - - /// /// Gets the property info that is referenced in the NavigatioAction expression. /// Credits: https://stackoverflow.com/a/17116267/4441216 @@ -100,7 +97,9 @@ public static PropertyInfo ParseNavigationExpression(Expression /// Helper method that "unboxes" the TValue from the relationship dictionary into /// diff --git a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs index 5f138d2092..d4f579d121 100644 --- a/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs +++ b/test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs @@ -12,7 +12,6 @@ public class Dummy : Identifiable public string SomeUpdatedProperty { get; set; } public string SomeNotUpdatedProperty { get; set; } - [HasOne("first-to-one")] public ToOne FirstToOne { get; set; } [HasOne("second-to-one")] @@ -20,6 +19,7 @@ public class Dummy : Identifiable [HasMany("to-manies")] public List ToManies { get; set; } } + public class NotTargeted : Identifiable { } public class ToMany : Identifiable { } public class ToOne : Identifiable { } From 3148d9fa87f5b43a6e732bbc7a088b1bbf80349f Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Thu, 18 Jul 2019 06:58:54 +0200 Subject: [PATCH 5/5] chore: styling --- src/JsonApiDotNetCore/Internal/TypeHelper.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/JsonApiDotNetCore/Internal/TypeHelper.cs b/src/JsonApiDotNetCore/Internal/TypeHelper.cs index d0d8294682..fb2c7df973 100644 --- a/src/JsonApiDotNetCore/Internal/TypeHelper.cs +++ b/src/JsonApiDotNetCore/Internal/TypeHelper.cs @@ -151,7 +151,12 @@ public static Dictionary> ConvertRelat return relationships.ToDictionary(pair => pair.Key, pair => (HashSet)pair.Value); } - public static Dictionary> ConvertAttributesToUpdate(Dictionary attributes, HashSet entities) + /// + /// Converts a dictionary of AttrAttributes to the underlying PropertyInfo that is referenced + /// + /// + /// + public static Dictionary> ConvertAttributeDictionary(Dictionary attributes, HashSet entities) { return attributes?.ToDictionary(p => p.Key.PropertyInfo, p => entities); }