Skip to content

EntityDiff replaced by IDiffableEntityHashSet #539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -15,68 +14,55 @@ namespace JsonApiDotNetCore.Hooks
/// Also contains information about updated relationships through
/// implementation of IRelationshipsDictionary<typeparamref name="TResource"/>>
/// </summary>
public interface IEntityDiffs<TResource> : IEnumerable<EntityDiffPair<TResource>> where TResource : class, IIdentifiable
public interface IDiffableEntityHashSet<TResource> : IEntityHashSet<TResource> where TResource : class, IIdentifiable
{
/// <summary>
/// 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.
/// </summary>
HashSet<TResource> DatabaseValues { get; }

/// <summary>
/// The resources that were affected by the request.
/// </summary>
EntityHashSet<TResource> Entities { get; }

IEnumerable<EntityDiffPair<TResource>> GetDiffs();

}

/// <inheritdoc />
public class EntityDiffs<TResource> : IEntityDiffs<TResource> where TResource : class, IIdentifiable
public class DiffableEntityHashSet<TResource> : EntityHashSet<TResource>, IDiffableEntityHashSet<TResource> where TResource : class, IIdentifiable
{
/// <inheritdoc />
public HashSet<TResource> DatabaseValues { get => _databaseValues ?? ThrowNoDbValuesError(); }
/// <inheritdoc />
public EntityHashSet<TResource> Entities { get; private set; }

private readonly HashSet<TResource> _databaseValues;
private readonly bool _databaseValuesLoaded;

public EntityDiffs(HashSet<TResource> requestEntities,
public DiffableEntityHashSet(HashSet<TResource> requestEntities,
HashSet<TResource> databaseEntities,
Dictionary<RelationshipAttribute, HashSet<TResource>> relationships)
Dictionary<RelationshipAttribute, HashSet<TResource>> relationships)
: base(requestEntities, relationships)
{
Entities = new EntityHashSet<TResource>(requestEntities, relationships);
_databaseValues = databaseEntities;
_databaseValuesLoaded |= _databaseValues != null;
}

/// <summary>
/// Used internally by the ResourceHookExecutor to make live a bit easier with generics
/// </summary>
internal EntityDiffs(IEnumerable requestEntities,
internal DiffableEntityHashSet(IEnumerable requestEntities,
IEnumerable databaseEntities,
Dictionary<RelationshipAttribute, IEnumerable> relationships)
: this((HashSet<TResource>)requestEntities, (HashSet<TResource>)databaseEntities, TypeHelper.ConvertRelationshipDictionary<TResource>(relationships)) { }


/// <inheritdoc />
public IEnumerator<EntityDiffPair<TResource>> GetEnumerator()
public IEnumerable<EntityDiffPair<TResource>> 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<TResource>(entity, currentValueInDatabase);
}
}

/// <inheritdoc />
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

private HashSet<TResource> 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");
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ public interface IBeforeHooks<TResource> where TResource : class, IIdentifiable
/// layer just before updating entities of type <typeparamref name="TResource"/>.
/// <para />
/// For the <see cref="ResourcePipeline.Patch"/> pipeline, the
/// <paramref name="entityDiff" /> will typically contain one entity.
/// <paramref name="entities" /> will typically contain one entity.
/// For <see cref="ResourcePipeline.BulkPatch"/>, this it may contain
/// multiple entities.
/// <para />
/// The returned <see cref="IEnumerable{TEntity}"/> may be a subset
/// of the <see cref="EntityDiffs{TEntity}.RequestEntities"/> property in parameter <paramref name="entityDiff"/>,
/// of the <see cref="DiffableEntityHashSet{TEntity}"/> property in parameter <paramref name="entities"/>,
/// 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.
Expand All @@ -75,9 +75,9 @@ public interface IBeforeHooks<TResource> where TResource : class, IIdentifiable
/// hook is fired for these.
/// </summary>
/// <returns>The transformed entity set</returns>
/// <param name="entityDiff">The entity diff.</param>
/// <param name="entities">The affected entities.</param>
/// <param name="pipeline">An enum indicating from where the hook was triggered.</param>
IEnumerable<TResource> BeforeUpdate(IEntityDiffs<TResource> entityDiff, ResourcePipeline pipeline);
IEnumerable<TResource> BeforeUpdate(IDiffableEntityHashSet<TResource> entities, ResourcePipeline pipeline);

/// <summary>
/// Implement this hook to run custom logic in the <see cref=" EntityResourceService{T}"/>
Expand Down
2 changes: 1 addition & 1 deletion src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public virtual IEnumerable<TEntity> BeforeUpdate<TEntity>(IEnumerable<TEntity> e
{
var relationships = node.RelationshipsToNextLayer.Select(p => p.Attribute).ToArray();
var dbValues = LoadDbValues(typeof(TEntity), (IEnumerable<TEntity>)node.UniqueEntities, ResourceHook.BeforeUpdate, relationships);
var diff = new EntityDiffs<TEntity>(node.UniqueEntities, dbValues, node.PrincipalsToNextLayer());
var diff = new DiffableEntityHashSet<TEntity>(node.UniqueEntities, dbValues, node.PrincipalsToNextLayer());
IEnumerable<TEntity> updated = container.BeforeUpdate(diff, pipeline);
node.UpdateUnique(updated);
node.Reassign(entities);
Expand Down
2 changes: 1 addition & 1 deletion src/JsonApiDotNetCore/Models/ResourceDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public virtual void AfterUpdateRelationship(IRelationshipsDictionary<T> entities
/// <inheritdoc/>
public virtual void BeforeRead(ResourcePipeline pipeline, bool isIncluded = false, string stringId = null) { }
/// <inheritdoc/>
public virtual IEnumerable<T> BeforeUpdate(IEntityDiffs<T> entityDiff, ResourcePipeline pipeline) { return entityDiff.Entities; }
public virtual IEnumerable<T> BeforeUpdate(IDiffableEntityHashSet<T> entities, ResourcePipeline pipeline) { return entities; }
/// <inheritdoc/>
public virtual IEnumerable<T> BeforeDelete(IEntityHashSet<T> entities, ResourcePipeline pipeline) { return entities; }
/// <inheritdoc/>
Expand Down
18 changes: 9 additions & 9 deletions test/UnitTests/ResourceHooks/AffectedEntitiesHelperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,13 @@ public void EntityDiff_GetByRelationships()
{
// arrange
var dbEntities = new HashSet<Dummy>(AllEntities.Select(e => new Dummy { Id = e.Id }).ToList());
EntityDiffs<Dummy> diffs = new EntityDiffs<Dummy>(AllEntities, dbEntities, Relationships);
DiffableEntityHashSet<Dummy> diffs = new DiffableEntityHashSet<Dummy>(AllEntities, dbEntities, Relationships);

// act
Dictionary<RelationshipAttribute, HashSet<Dummy>> toOnes = diffs.Entities.GetByRelationship<ToOne>();
Dictionary<RelationshipAttribute, HashSet<Dummy>> toManies = diffs.Entities.GetByRelationship<ToMany>();
Dictionary<RelationshipAttribute, HashSet<Dummy>> notTargeted = diffs.Entities.GetByRelationship<NotTargeted>();
Dictionary<RelationshipAttribute, HashSet<Dummy>> allRelationships = diffs.Entities.AffectedRelationships;
Dictionary<RelationshipAttribute, HashSet<Dummy>> toOnes = diffs.GetByRelationship<ToOne>();
Dictionary<RelationshipAttribute, HashSet<Dummy>> toManies = diffs.GetByRelationship<ToMany>();
Dictionary<RelationshipAttribute, HashSet<Dummy>> notTargeted = diffs.GetByRelationship<NotTargeted>();
Dictionary<RelationshipAttribute, HashSet<Dummy>> allRelationships = diffs.AffectedRelationships;

// Assert
AssertRelationshipDictionaryGetters(allRelationships, toOnes, toManies, notTargeted);
Expand All @@ -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);
Expand All @@ -120,10 +120,10 @@ public void EntityDiff_Loops_Over_Diffs()
{
// arrange
var dbEntities = new HashSet<Dummy>(AllEntities.Select(e => new Dummy { Id = e.Id }));
EntityDiffs<Dummy> diffs = new EntityDiffs<Dummy>(AllEntities, dbEntities, Relationships);
DiffableEntityHashSet<Dummy> diffs = new DiffableEntityHashSet<Dummy>(AllEntities, dbEntities, Relationships);

// Assert & act
foreach (EntityDiffPair<Dummy> diff in diffs)
foreach (EntityDiffPair<Dummy> diff in diffs.GetDiffs())
{
Assert.Equal(diff.Entity.Id, diff.DatabaseValue.Id);
Assert.NotEqual(diff.Entity, diff.DatabaseValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public void BeforeUpdate()
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);

// assert
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny<IEntityDiffs<TodoItem>>(), ResourcePipeline.Patch), Times.Once());
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny<IDiffableEntityHashSet<TodoItem>>(), ResourcePipeline.Patch), Times.Once());
ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship(It.IsAny<HashSet<string>>(), It.IsAny<IRelationshipsDictionary<Person>>(), ResourcePipeline.Patch), Times.Once());
VerifyNoOtherCalls(todoResourceMock, ownerResourceMock);
}
Expand Down Expand Up @@ -62,7 +62,7 @@ public void BeforeUpdate_Without_Child_Hook_Implemented()
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);

// assert
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny<IEntityDiffs<TodoItem>>(), ResourcePipeline.Patch), Times.Once());
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny<IDiffableEntityHashSet<TodoItem>>(), ResourcePipeline.Patch), Times.Once());
VerifyNoOtherCalls(todoResourceMock, ownerResourceMock);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void BeforeUpdate()
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);

// assert
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IEntityDiffs<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IDiffableEntityHashSet<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship(
It.Is<HashSet<string>>(ids => PersonIdCheck(ids, personId)),
It.Is<IRelationshipsDictionary<Person>>(rh => PersonCheck(lastName, rh)),
Expand Down Expand Up @@ -93,7 +93,7 @@ public void BeforeUpdate_Deleting_Relationship()
hookExecutor.BeforeUpdate(_todoList, ResourcePipeline.Patch);

// assert
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IEntityDiffs<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IDiffableEntityHashSet<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
ownerResourceMock.Verify(rd => rd.BeforeImplicitUpdateRelationship(
It.Is<IRelationshipsDictionary<Person>>(rh => PersonCheck(lastName + lastName, rh)),
ResourcePipeline.Patch),
Expand Down Expand Up @@ -140,7 +140,7 @@ public void BeforeUpdate_Without_Child_Hook_Implemented()
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);

// assert
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IEntityDiffs<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IDiffableEntityHashSet<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
todoResourceMock.Verify(rd => rd.BeforeImplicitUpdateRelationship(
It.Is<IRelationshipsDictionary<TodoItem>>(rh => TodoCheck(rh, description + description)),
ResourcePipeline.Patch),
Expand All @@ -161,7 +161,7 @@ public void BeforeUpdate_NoImplicit()
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);

// assert
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IEntityDiffs<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IDiffableEntityHashSet<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship(
It.Is<HashSet<string>>(ids => PersonIdCheck(ids, personId)),
It.IsAny<IRelationshipsDictionary<Person>>(),
Expand Down Expand Up @@ -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<IEntityDiffs<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IDiffableEntityHashSet<TodoItem>>((diff) => TodoCheckDiff(diff, description)), ResourcePipeline.Patch), Times.Once());
VerifyNoOtherCalls(todoResourceMock, ownerResourceMock);
}

private bool TodoCheckDiff(IEntityDiffs<TodoItem> diff, string checksum)
private bool TodoCheckDiff(IDiffableEntityHashSet<TodoItem> 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<Person>().Single();
var updatedRelationship = entities.GetByRelationship<Person>().Single();
var diffcheck = updatedRelationship.Key.PublicRelationshipName == "one-to-one-person";

return (dbCheck && reqCheck && diffcheck);
Expand Down
4 changes: 2 additions & 2 deletions test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ void MockHooks<TModel>(Mock<IResourceHookContainer<TModel>> resourceDefinition)
.Setup(rd => rd.BeforeRead(It.IsAny<ResourcePipeline>(), It.IsAny<bool>(), It.IsAny<string>()))
.Verifiable();
resourceDefinition
.Setup(rd => rd.BeforeUpdate(It.IsAny<IEntityDiffs<TModel>>(), It.IsAny<ResourcePipeline>()))
.Returns<EntityDiffs<TModel>, ResourcePipeline>((entityDiff, context) => entityDiff.Entities)
.Setup(rd => rd.BeforeUpdate(It.IsAny<IDiffableEntityHashSet<TModel>>(), It.IsAny<ResourcePipeline>()))
.Returns<DiffableEntityHashSet<TModel>, ResourcePipeline>((entities, context) => entities)
.Verifiable();
resourceDefinition
.Setup(rd => rd.BeforeDelete(It.IsAny<IEntityHashSet<TModel>>(), It.IsAny<ResourcePipeline>()))
Expand Down