diff --git a/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs similarity index 63% rename from src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs rename to src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs index fb30c26c7d..9aac4b1fd9 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.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 IDiffableEntityHashSet : 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; } - + IEnumerable> GetDiffs(); + } /// - public class EntityDiffs : IEntityDiffs where TResource : class, IIdentifiable + public class DiffableEntityHashSet : EntityHashSet, IDiffableEntityHashSet 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 DiffableEntityHashSet(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 DiffableEntityHashSet(IEnumerable requestEntities, IEnumerable databaseEntities, Dictionary relationships) : this((HashSet)requestEntities, (HashSet)databaseEntities, TypeHelper.ConvertRelationshipDictionary(relationships)) { } /// - public IEnumerator> GetEnumerator() + public IEnumerable> 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..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(IEntityDiffs 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 30bb51560d..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 EntityDiffs(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 0a6b0ac091..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(IEntityDiffs 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 39513d05b3..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()); - EntityDiffs diffs = new EntityDiffs(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 })); - EntityDiffs diffs = new EntityDiffs(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 f02acda264..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 fca4d9c480..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(IEntityDiffs 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 17065d3bec..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()))