Skip to content

Commit 86dfd8d

Browse files
maureimaurei
committed
refactor: AffectedResourcesDiff
1 parent 0290ae4 commit 86dfd8d

File tree

7 files changed

+43
-61
lines changed

7 files changed

+43
-61
lines changed

‎src/JsonApiDotNetCore/Hooks/Execution/EntityDiff.cs renamed to ‎src/JsonApiDotNetCore/Hooks/Execution/AffectedResourceDiff.cs

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,49 +8,50 @@ namespace JsonApiDotNetCore.Hooks
88
{
99
/// <summary>
1010
/// A helper class that provides insight in what is to be updated. The
11-
/// <see cref="IResourceDiff{TEntity}.RequestEntities"/> property reflects what was parsed from the incoming request,
12-
/// where the <see cref="IResourceDiff{TEntity}.DatabaseValues"/> reflects what is the current state in the database.
11+
/// <see cref="IAffectedResourcesDiff{TEntity}.RequestEntities"/> property reflects what was parsed from the incoming request,
12+
/// where the <see cref="IAffectedResourcesDiff{TEntity}.DatabaseValues"/> reflects what is the current state in the database.
1313
///
1414
/// Any relationships that are updated can be retrieved via the methods implemented on
1515
/// <see cref="IAffectedRelationships{TDependent}"/>.
1616
/// </summary>
17-
public interface IResourceDiff<TEntity> : IEnumerable<ResourceDiffPair<TEntity>>, IAffectedResourcesBase<TEntity> where TEntity : class, IIdentifiable
17+
public interface IAffectedResourcesDiff<TEntity> : IAffectedResources<TEntity> where TEntity : class, IIdentifiable
1818
{
1919
HashSet<TEntity> DatabaseValues { get; }
20+
IEnumerable<ResourceDiffPair<TEntity>> GetDiff();
2021
}
2122

22-
public class ResourceDiff<TEntity> : AffectedResourcesBase<TEntity>, IResourceDiff<TEntity> where TEntity : class, IIdentifiable
23+
public class ResourceDiff<TEntity> : AffectedResources<TEntity>, IAffectedResourcesDiff<TEntity> where TEntity : class, IIdentifiable
2324
{
24-
private readonly HashSet<TEntity> _databaseEntities;
25+
26+
private readonly HashSet<TEntity> _databaseValues;
2527
private readonly bool _databaseValuesLoaded;
26-
public HashSet<TEntity> DatabaseValues { get => _databaseEntities ?? ThrowNoDbValuesError(); }
28+
29+
/// <summary>
30+
/// the current database values of the affected resources collection.
31+
/// </summary>
32+
public HashSet<TEntity> DatabaseValues { get => _databaseValues ?? ThrowNoDbValuesError(); }
2733

2834
internal ResourceDiff(IEnumerable requestEntities,
2935
IEnumerable databaseEntities,
3036
Dictionary<RelationshipProxy, IEnumerable> relationships) : base(requestEntities, relationships)
3137
{
32-
_databaseEntities = (HashSet<TEntity>)databaseEntities;
33-
_databaseValuesLoaded |= _databaseEntities != null;
38+
_databaseValues = (HashSet<TEntity>)databaseEntities;
39+
_databaseValuesLoaded |= _databaseValues != null;
3440
}
3541

36-
private HashSet<TEntity> ThrowNoDbValuesError()
37-
{
38-
throw new MemberAccessException("Cannot access database entities if the LoadDatabaseValues option is set to false");
39-
}
40-
41-
public IEnumerator<ResourceDiffPair<TEntity>> GetEnumerator()
42+
public IEnumerable<ResourceDiffPair<TEntity>> GetDiff()
4243
{
4344
foreach (var entity in Entities)
4445
{
4546
TEntity currentValueInDatabase = null;
46-
if (_databaseValuesLoaded) currentValueInDatabase = _databaseEntities.Single(e => entity.StringId == e.StringId);
47+
if (_databaseValuesLoaded) currentValueInDatabase = _databaseValues.Single(e => entity.StringId == e.StringId);
4748
yield return new ResourceDiffPair<TEntity>(entity, currentValueInDatabase);
4849
}
4950
}
5051

51-
IEnumerator IEnumerable.GetEnumerator()
52+
private HashSet<TEntity> ThrowNoDbValuesError()
5253
{
53-
return GetEnumerator();
54+
throw new MemberAccessException("Cannot access database entities if the LoadDatabaseValues option is set to false");
5455
}
5556
}
5657

‎src/JsonApiDotNetCore/Hooks/Execution/AffectedResources.cs

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,32 +6,23 @@
66
namespace JsonApiDotNetCore.Hooks
77
{
88

9-
public interface IAffectedResources<TEntity> : IAffectedResourcesBase<TEntity>, IEnumerable<TEntity> where TEntity : class, IIdentifiable
10-
{
11-
12-
}
13-
14-
/// NOTE: you might wonder why there is a separate AffectedResourceBase and AffectedResource.
15-
/// If we merge them together, ie get rid of the base and just let the AffectedResource directly implement IEnumerable{TEntity},
16-
/// we will run in to problems with the following:
17-
/// ResourceDiff{<typeparam name="TEntity"/>} inherits from AffectedResource{TEntity},
18-
/// but ResourceDiff also implements IEnumerable{ResourceDiffPair{TEntity}}. This means that
19-
/// ResourceDiff will implement two IEnumerable{x} where (x1 = TEntity and x2 = ResourceDiffPair{TEntity} )
20-
/// The problem with this is that when you then try to do a simple foreach loop over
21-
/// a ResourceDiff instance, it will throw an error, because it does not know which of the two enumerators to pick.
22-
/// We want ResourceDiff to only loop over the ResourceDiffPair, so we can do that by making sure
23-
/// it doesn't inherit the IEnumerable{TEntity} part from AffectedResources.
24-
public interface IAffectedResourcesBase<TEntity> where TEntity : class, IIdentifiable
9+
public interface IAffectedResources<TEntity> : IEnumerable<TEntity> where TEntity : class, IIdentifiable
2510
{
2611
HashSet<TEntity> Entities { get; }
2712
}
2813

29-
public class AffectedResources<TEntity> : AffectedResourcesBase<TEntity>, IAffectedResources<TEntity> where TEntity : class, IIdentifiable
14+
public class AffectedResources<TEntity> : AffectedRelationships<TEntity>, IAffectedResources<TEntity> where TEntity : class, IIdentifiable
3015
{
31-
internal AffectedResources(IEnumerable entities,
32-
Dictionary<RelationshipProxy, IEnumerable> relationships)
33-
: base(entities, relationships) { }
16+
/// <summary>
17+
/// The entities that are affected by the request.
18+
/// </summary>
19+
public HashSet<TEntity> Entities { get; }
3420

21+
internal AffectedResources(IEnumerable entities,
22+
Dictionary<RelationshipProxy, IEnumerable> relationships) : base(relationships)
23+
{
24+
Entities = new HashSet<TEntity>(entities.Cast<TEntity>());
25+
}
3526
public IEnumerator<TEntity> GetEnumerator()
3627
{
3728
return Entities.GetEnumerator();
@@ -43,15 +34,4 @@ IEnumerator IEnumerable.GetEnumerator()
4334
}
4435
}
4536

46-
public abstract class AffectedResourcesBase<TEntity> : AffectedRelationships<TEntity>, IAffectedResourcesBase<TEntity> where TEntity : class, IIdentifiable
47-
{
48-
public HashSet<TEntity> Entities { get; }
49-
50-
internal protected AffectedResourcesBase(IEnumerable entities,
51-
Dictionary<RelationshipProxy, IEnumerable> relationships) : base(relationships)
52-
{
53-
Entities = new HashSet<TEntity>(entities.Cast<TEntity>());
54-
}
55-
}
56-
5737
}

‎src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public interface IBeforeHooks<TEntity> where TEntity : class, IIdentifiable
133133
/// <returns>The transformed entity set</returns>
134134
/// <param name="ResourceDiff">The entity diff.</param>
135135
/// <param name="pipeline">An enum indicating from where the hook was triggered.</param>
136-
IEnumerable<TEntity> BeforeUpdate(IResourceDiff<TEntity> ResourceDiff, ResourcePipeline pipeline);
136+
IEnumerable<TEntity> BeforeUpdate(IAffectedResourcesDiff<TEntity> ResourceDiff, ResourcePipeline pipeline);
137137
/// <summary>
138138
/// Implement this hook to run custom logic in the <see cref=" EntityResourceService{T}"/>
139139
/// layer just before deleting entities of type <typeparamref name="TEntity"/>.

‎src/JsonApiDotNetCore/Models/ResourceDefinition.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ public virtual void AfterUpdateRelationship(IAffectedRelationships<T> resourcesB
179179
/// <inheritdoc/>
180180
public virtual void BeforeRead(ResourcePipeline pipeline, bool isIncluded = false, string stringId = null) { }
181181
/// <inheritdoc/>
182-
public virtual IEnumerable<T> BeforeUpdate(IResourceDiff<T> ResourceDiff, ResourcePipeline pipeline) { return ResourceDiff.Entities; }
182+
public virtual IEnumerable<T> BeforeUpdate(IAffectedResourcesDiff<T> ResourceDiff, ResourcePipeline pipeline) { return ResourceDiff; }
183183
/// <inheritdoc/>
184184
public virtual IEnumerable<T> BeforeDelete(IAffectedResources<T> affected, ResourcePipeline pipeline) { return affected; }
185185
/// <inheritdoc/>

‎test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdateTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public void BeforeUpdate()
2424
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);
2525

2626
// assert
27-
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny<IResourceDiff<TodoItem>>(), ResourcePipeline.Patch), Times.Once());
27+
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny<IAffectedResourcesDiff<TodoItem>>(), ResourcePipeline.Patch), Times.Once());
2828
ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship(It.IsAny<HashSet<string>>(), It.IsAny<IAffectedRelationships<Person>>(), ResourcePipeline.Patch), Times.Once());
2929
VerifyNoOtherCalls(todoResourceMock, ownerResourceMock);
3030
}
@@ -62,7 +62,7 @@ public void BeforeUpdate_Without_Child_Hook_Implemented()
6262
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);
6363

6464
// assert
65-
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny<IResourceDiff<TodoItem>>(), ResourcePipeline.Patch), Times.Once());
65+
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.IsAny<IAffectedResourcesDiff<TodoItem>>(), ResourcePipeline.Patch), Times.Once());
6666
VerifyNoOtherCalls(todoResourceMock, ownerResourceMock);
6767
}
6868

‎test/UnitTests/ResourceHooks/ResourceHookExecutor/Update/BeforeUpdate_WithDbValues_Tests.cs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public void BeforeUpdate()
5959
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);
6060

6161
// assert
62-
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IResourceDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
62+
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IAffectedResourcesDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
6363
ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship(
6464
It.Is<HashSet<string>>(ids => PersonIdCheck(ids, personId)),
6565
It.Is<IAffectedRelationships<Person>>(rh => PersonCheck(lastName, rh)),
@@ -93,7 +93,7 @@ public void BeforeUpdate_Deleting_Relationship()
9393
hookExecutor.BeforeUpdate(_todoList, ResourcePipeline.Patch);
9494

9595
// assert
96-
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IResourceDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
96+
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IAffectedResourcesDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
9797
ownerResourceMock.Verify(rd => rd.BeforeImplicitUpdateRelationship(
9898
It.Is<IAffectedRelationships<Person>>(rh => PersonCheck(lastName + lastName, rh)),
9999
ResourcePipeline.Patch),
@@ -140,7 +140,7 @@ public void BeforeUpdate_Without_Child_Hook_Implemented()
140140
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);
141141

142142
// assert
143-
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IResourceDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
143+
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IAffectedResourcesDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
144144
todoResourceMock.Verify(rd => rd.BeforeImplicitUpdateRelationship(
145145
It.Is<IAffectedRelationships<TodoItem>>(rh => TodoCheck(rh, description + description)),
146146
ResourcePipeline.Patch),
@@ -161,7 +161,7 @@ public void BeforeUpdate_NoImplicit()
161161
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);
162162

163163
// assert
164-
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IResourceDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
164+
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IAffectedResourcesDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
165165
ownerResourceMock.Verify(rd => rd.BeforeUpdateRelationship(
166166
It.Is<HashSet<string>>(ids => PersonIdCheck(ids, personId)),
167167
It.IsAny<IAffectedRelationships<Person>>(),
@@ -204,14 +204,15 @@ public void BeforeUpdate_NoImplicit_Without_Child_Hook_Implemented()
204204
hookExecutor.BeforeUpdate(todoList, ResourcePipeline.Patch);
205205

206206
// assert
207-
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IResourceDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
207+
todoResourceMock.Verify(rd => rd.BeforeUpdate(It.Is<IAffectedResourcesDiff<TodoItem>>((diff) => TodoCheck(diff, description)), ResourcePipeline.Patch), Times.Once());
208208
VerifyNoOtherCalls(todoResourceMock, ownerResourceMock);
209209
}
210210

211-
private bool TodoCheck(IResourceDiff<TodoItem> diff, string checksum)
211+
private bool TodoCheck(IAffectedResourcesDiff<TodoItem> diff, string checksum)
212212
{
213-
var dbCheck = diff.DatabaseValues.Single().Description == checksum;
214-
var reqCheck = diff.Single().Entity.Description == null;
213+
var diffPair = diff.GetDiff().Single();
214+
var dbCheck = diffPair.DatabaseValue.Description == checksum;
215+
var reqCheck = diffPair.Entity.Description == null;
215216
return (dbCheck && reqCheck);
216217
}
217218

‎test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ void MockHooks<TModel>(Mock<IResourceHookContainer<TModel>> resourceDefinition)
261261
.Setup(rd => rd.BeforeRead(It.IsAny<ResourcePipeline>(), It.IsAny<bool>(), It.IsAny<string>()))
262262
.Verifiable();
263263
resourceDefinition
264-
.Setup(rd => rd.BeforeUpdate(It.IsAny<IResourceDiff<TModel>>(), It.IsAny<ResourcePipeline>()))
264+
.Setup(rd => rd.BeforeUpdate(It.IsAny<IAffectedResourcesDiff<TModel>>(), It.IsAny<ResourcePipeline>()))
265265
.Returns<ResourceDiff<TModel>, ResourcePipeline>((entityDiff, context) => entityDiff.Entities)
266266
.Verifiable();
267267
resourceDefinition

0 commit comments

Comments
 (0)