Skip to content

Commit b8df749

Browse files
maureimaurei
committed
feat: improved loaddbvalues attribute error handling, refactored a bit
1 parent a1f5d12 commit b8df749

File tree

11 files changed

+129
-76
lines changed

11 files changed

+129
-76
lines changed

‎src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
using JsonApiDotNetCore.Models;
1111
using JsonApiDotNetCore.Services;
1212
using Microsoft.EntityFrameworkCore;
13-
using Microsoft.EntityFrameworkCore.Metadata;
1413
using Microsoft.Extensions.Logging;
1514
namespace JsonApiDotNetCore.Data
1615
{
@@ -378,22 +377,14 @@ public async Task UpdateRelationshipsAsync(object parent, RelationshipAttribute
378377
await genericProcessor.UpdateRelationshipsAsync(parent, relationship, relationshipIds);
379378
}
380379

380+
381381
/// <inheritdoc />
382382
public virtual async Task<bool> DeleteAsync(TId id)
383383
{
384-
return await DeleteAsync(await GetAsync(id));
385-
386-
}
387-
388-
public virtual async Task<bool> DeleteAsync(TEntity entity)
389-
{
390-
if (entity == null)
391-
return false;
392-
393-
384+
var entity = await GetAsync(id);
385+
if (entity == null) return false;
394386
_dbSet.Remove(entity);
395387
await _context.SaveChangesAsync();
396-
397388
return true;
398389
}
399390

‎src/JsonApiDotNetCore/Data/IEntityWriteRepository.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ public interface IEntityWriteRepository<TEntity, in TId>
1919

2020
Task UpdateRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable<string> relationshipIds);
2121

22-
Task<bool> DeleteAsync(TEntity entity);
23-
2422
Task<bool> DeleteAsync(TId id);
2523
}
2624
}

‎src/JsonApiDotNetCore/Hooks/Discovery/HooksDiscovery.cs

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,16 @@ namespace JsonApiDotNetCore.Hooks
1414
public class HooksDiscovery<TEntity> : IHooksDiscovery<TEntity> where TEntity : class, IIdentifiable
1515
{
1616
private readonly ResourceHook[] _allHooks;
17-
17+
private readonly ResourceHook[] _databaseValuesAttributeAllowed =
18+
{
19+
ResourceHook.BeforeUpdate,
20+
ResourceHook.BeforeUpdateRelationship,
21+
ResourceHook.BeforeDelete
22+
};
1823
/// <inheritdoc/>
1924
public ResourceHook[] ImplementedHooks { get; private set; }
20-
public ResourceHook[] DatabaseDiffEnabledHooks { get; private set; }
21-
public ResourceHook[] DatabaseDiffDisabledHooks { get; private set; }
25+
public ResourceHook[] DatabaseValuesEnabledHooks { get; private set; }
26+
public ResourceHook[] DatabaseValuesDisabledHooks { get; private set; }
2227

2328

2429
public HooksDiscovery()
@@ -38,43 +43,47 @@ void DiscoverImplementedHooksForModel()
3843
{
3944
Type parameterizedResourceDefinition = typeof(ResourceDefinition<TEntity>);
4045
var derivedTypes = TypeLocator.GetDerivedTypes(typeof(TEntity).Assembly, parameterizedResourceDefinition).ToList();
46+
47+
48+
var implementedHooks = new List<ResourceHook>();
49+
var enabledHooks = new List<ResourceHook>() { ResourceHook.BeforeImplicitUpdateRelationship } ;
50+
var disabledHooks = new List<ResourceHook>();
51+
Type targetType = null;
4152
try
4253
{
43-
var implementedHooks = new List<ResourceHook>();
44-
var diffEnabledHooks = new List<ResourceHook>();
45-
var diffDisabledHooks = new List<ResourceHook>();
46-
Type targetType = derivedTypes.SingleOrDefault(); // multiple containers is not supported
47-
if (targetType != null)
54+
targetType = derivedTypes.SingleOrDefault(); // multiple containers is not supported
55+
}
56+
catch
57+
{
58+
throw new JsonApiSetupException($"It is currently not supported to" +
59+
"implement hooks across multiple implementations of ResourceDefinition<T>");
60+
}
61+
if (targetType != null)
62+
{
63+
foreach (var hook in _allHooks)
4864
{
49-
foreach (var hook in _allHooks)
65+
var method = targetType.GetMethod(hook.ToString("G"));
66+
if (method.DeclaringType != parameterizedResourceDefinition)
5067
{
51-
var method = targetType.GetMethod(hook.ToString("G"));
52-
if (method.DeclaringType != parameterizedResourceDefinition)
68+
implementedHooks.Add(hook);
69+
var attr = method.GetCustomAttributes(true).OfType<LoadDatabaseValues>().SingleOrDefault();
70+
if (attr != null)
5371
{
54-
implementedHooks.Add(hook);
55-
if (hook == ResourceHook.BeforeImplicitUpdateRelationship)
56-
{
57-
diffEnabledHooks.Add(hook);
58-
continue;
59-
}
60-
var attr = method.GetCustomAttributes(true).OfType<LoadDatabaseValues>().SingleOrDefault();
61-
if (attr != null)
72+
if (!_databaseValuesAttributeAllowed.Contains(hook))
6273
{
63-
var targetList = attr.value ? diffEnabledHooks : diffDisabledHooks;
64-
targetList.Add(hook);
74+
throw new JsonApiSetupException($"DatabaseValuesAttribute cannot be used on hook" +
75+
$"{hook.ToString("G")} in resource definition {parameterizedResourceDefinition.Name}");
6576
}
66-
}
67-
}
68-
77+
var targetList = attr.value ? enabledHooks : disabledHooks;
78+
targetList.Add(hook);
79+
}
80+
}
6981
}
70-
ImplementedHooks = implementedHooks.ToArray();
71-
DatabaseDiffDisabledHooks = diffDisabledHooks.ToArray();
72-
DatabaseDiffEnabledHooks = diffEnabledHooks.ToArray();
73-
} catch (Exception e)
74-
{
75-
throw new JsonApiSetupException($@"Incorrect resource hook setup. For a given model of type TEntity,
76-
only one class may implement IResourceHookContainer<TEntity>");
82+
7783
}
84+
ImplementedHooks = implementedHooks.ToArray();
85+
DatabaseValuesDisabledHooks = disabledHooks.ToArray();
86+
DatabaseValuesEnabledHooks = enabledHooks.ToArray();
7887

7988
}
8089
}

‎src/JsonApiDotNetCore/Hooks/Discovery/IHooksDiscovery.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ public interface IHooksDiscovery
2121
/// </summary>
2222
/// <value>The implemented hooks.</value>
2323
ResourceHook[] ImplementedHooks { get; }
24-
ResourceHook[] DatabaseDiffEnabledHooks { get; }
25-
ResourceHook[] DatabaseDiffDisabledHooks { get; }
24+
ResourceHook[] DatabaseValuesEnabledHooks { get; }
25+
ResourceHook[] DatabaseValuesDisabledHooks { get; }
2626
}
2727

2828
}

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ internal class HookExecutorHelper : IHookExecutorHelper
2929
public HookExecutorHelper(
3030
IGenericProcessorFactory genericProcessorFactory,
3131
IResourceGraph graph,
32-
IJsonApiContext context = null
32+
IJsonApiContext context
3333
)
3434
{
3535
_genericProcessorFactory = genericProcessorFactory;
@@ -81,39 +81,38 @@ public IResourceHookContainer<TEntity> GetResourceHookContainer<TEntity>(Resourc
8181
return (IResourceHookContainer<TEntity>)GetResourceHookContainer(typeof(TEntity), hook);
8282
}
8383

84-
public IEnumerable LoadDbValues(PrincipalType repositoryEntityType, Type affectedHookEntityType, IEnumerable entities, ResourceHook hook, params RelationshipProxy[] relationships)
84+
public IEnumerable LoadDbValues(PrincipalType entityTypeForRepository, IEnumerable entities, ResourceHook hook, params RelationshipProxy[] relationships)
8585
{
86-
if (!ShouldLoadDbValues(affectedHookEntityType, hook)) return null;
87-
8886
var paths = relationships.Select(p => p.Attribute.RelationshipPath).ToArray();
89-
var idType = GetIdentifierType(repositoryEntityType);
87+
var idType = GetIdentifierType(entityTypeForRepository);
9088
var parameterizedGetWhere = GetType()
9189
.GetMethod(nameof(GetWhereAndInclude), BindingFlags.NonPublic | BindingFlags.Instance)
92-
.MakeGenericMethod(repositoryEntityType, idType);
90+
.MakeGenericMethod(entityTypeForRepository, idType);
9391
var casted = ((IEnumerable<object>)entities).Cast<IIdentifiable>();
9492
var ids = casted.Select(e => e.StringId).Cast(idType);
9593
var values = (IEnumerable)parameterizedGetWhere.Invoke(this, new object[] { ids, paths });
96-
return values;
94+
if (values == null) return null;
95+
return (IEnumerable)Activator.CreateInstance(typeof(HashSet<>).MakeGenericType(entityTypeForRepository), values.Cast(entityTypeForRepository));
9796
}
9897

9998
public HashSet<TEntity> LoadDbValues<TEntity>(IEnumerable<TEntity> entities, ResourceHook hook, params RelationshipProxy[] relationships) where TEntity : class, IIdentifiable
10099
{
101100
var entityType = typeof(TEntity);
102-
var dbValues = LoadDbValues(entityType, entityType, entities, hook, relationships)?.Cast<TEntity>();
101+
var dbValues = LoadDbValues(entityType, entities, hook, relationships)?.Cast<TEntity>();
103102
if (dbValues == null) return null;
104103
return new HashSet<TEntity>(dbValues);
105104
}
106105

107106

108-
bool ShouldLoadDbValues(DependentType entityType, ResourceHook hook)
107+
public bool ShouldLoadDbValues(Type entityType, ResourceHook hook)
109108
{
110109
var discovery = GetHookDiscovery(entityType);
111110

112-
if (discovery.DatabaseDiffDisabledHooks.Contains(hook))
111+
if (discovery.DatabaseValuesDisabledHooks.Contains(hook))
113112
{
114113
return false;
115114
}
116-
else if (discovery.DatabaseDiffEnabledHooks.Contains(hook))
115+
else if (discovery.DatabaseValuesEnabledHooks.Contains(hook))
117116
{
118117
return true;
119118
}
@@ -177,7 +176,9 @@ public Dictionary<RelationshipProxy, IEnumerable> LoadImplicitlyAffected(
177176
foreach (var kvp in principalEntitiesByRelation)
178177
{
179178
if (IsHasManyThrough(kvp, out var principals, out var relationship)) continue;
180-
var includedPrincipals = LoadDbValues(relationship.PrincipalType, relationship.DependentType, principals, ResourceHook.BeforeImplicitUpdateRelationship, relationship);
179+
180+
// note that we dont't have to check if BeforeImplicitUpdate hook is implemented. If not, it wont ever get here.
181+
var includedPrincipals = LoadDbValues(relationship.PrincipalType, principals, ResourceHook.BeforeImplicitUpdateRelationship, relationship);
181182

182183
foreach (IIdentifiable ip in includedPrincipals)
183184
{

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,7 @@ internal interface IHookExecutorHelper
4242
/// <summary>
4343
/// For a set of entities, loads current values from the database
4444
/// </summary>
45-
IEnumerable LoadDbValues(Type repositoryEntityType, Type affectedHookEntityType, IEnumerable entities, ResourceHook hook, params RelationshipProxy[] relationships);
46-
/// <summary>
47-
/// For a set of entities, loads current values from the database
48-
/// </summary>
49-
HashSet<TEntity> LoadDbValues<TEntity>(IEnumerable<TEntity> entities, ResourceHook hook, params RelationshipProxy[] relationships) where TEntity : class, IIdentifiable;
45+
IEnumerable LoadDbValues(Type repositoryEntityType, IEnumerable entities, ResourceHook hook, params RelationshipProxy[] relationships);
46+
bool ShouldLoadDbValues(Type containerEntityType, ResourceHook hook);
5047
}
5148
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ public interface IAffectedRelationships<TDependent> : IAffectedRelationships whe
1616
/// <summary>
1717
/// Gets a dictionary of all entities grouped by affected relationship.
1818
/// </summary>
19-
Dictionary<RelationshipAttribute, HashSet<TDependent>> All { get; }
19+
Dictionary<RelationshipAttribute, HashSet<TDependent>> AllByRelationships();
20+
2021
/// <summary>
2122
/// Gets a dictionary of all entities that have an affected relationship to type <typeparamref name="TPrincipal"/>
2223
/// </summary>
@@ -31,9 +32,9 @@ public class UpdatedRelationshipHelper<TDependent> : IAffectedRelationships<TDep
3132
{
3233
private readonly Dictionary<RelationshipProxy, HashSet<TDependent>> _groups;
3334

34-
public Dictionary<RelationshipAttribute, HashSet<TDependent>> All
35+
public Dictionary<RelationshipAttribute, HashSet<TDependent>> AllByRelationships()
3536
{
36-
get { return _groups?.ToDictionary(p => p.Key.Attribute, p => p.Value); }
37+
return _groups?.ToDictionary(p => p.Key.Attribute, p => p.Value);
3738
}
3839
public UpdatedRelationshipHelper(Dictionary<RelationshipProxy, IEnumerable> relationships)
3940
{

‎src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public virtual IEnumerable<TEntity> BeforeUpdate<TEntity>(IEnumerable<TEntity> e
4747
{
4848
if (GetHook(ResourceHook.BeforeUpdate, entities, out var container, out var node))
4949
{
50-
var dbValues = _executorHelper.LoadDbValues((IEnumerable<TEntity>)node.UniqueEntities, ResourceHook.BeforeUpdate, node.RelationshipsToNextLayer);
50+
var dbValues = LoadDbValues(typeof(TEntity), (IEnumerable<TEntity>)node.UniqueEntities, ResourceHook.BeforeUpdate, node.RelationshipsToNextLayer);
5151
var diff = new EntityDiff<TEntity>(node.UniqueEntities, dbValues, node.PrincipalsToNextLayer());
5252
IEnumerable<TEntity> updated = container.BeforeUpdate(diff, pipeline);
5353
node.UpdateUnique(updated);
@@ -58,6 +58,7 @@ public virtual IEnumerable<TEntity> BeforeUpdate<TEntity>(IEnumerable<TEntity> e
5858
return entities;
5959
}
6060

61+
6162
/// <inheritdoc/>
6263
public virtual IEnumerable<TEntity> BeforeCreate<TEntity>(IEnumerable<TEntity> entities, ResourcePipeline pipeline) where TEntity : class, IIdentifiable
6364
{
@@ -77,7 +78,8 @@ public virtual IEnumerable<TEntity> BeforeDelete<TEntity>(IEnumerable<TEntity> e
7778
{
7879
if (GetHook(ResourceHook.BeforeDelete, entities, out var container, out var node))
7980
{
80-
IEnumerable<TEntity> updated = container.BeforeDelete((HashSet<TEntity>)node.UniqueEntities, pipeline);
81+
var targetEntities = (LoadDbValues(typeof(TEntity), (IEnumerable<TEntity>)node.UniqueEntities, ResourceHook.BeforeDelete, node.RelationshipsToNextLayer) ?? node.UniqueEntities);
82+
IEnumerable<TEntity> updated = container.BeforeDelete((HashSet<TEntity>)targetEntities, pipeline);
8183
node.UpdateUnique(updated);
8284
node.Reassign(entities);
8385
}
@@ -246,7 +248,7 @@ void FireNestedBeforeUpdateHooks(ResourcePipeline pipeline, EntityChildLayer lay
246248
{
247249
if (uniqueEntities.Cast<IIdentifiable>().Any())
248250
{
249-
var dbValues = _executorHelper.LoadDbValues(entityType, entityType, uniqueEntities, ResourceHook.BeforeUpdateRelationship, node.RelationshipsToNextLayer);
251+
var dbValues = LoadDbValues(entityType, uniqueEntities, ResourceHook.BeforeUpdateRelationship, node.RelationshipsToNextLayer);
250252
var resourcesByRelationship = CreateRelationshipHelper(entityType, node.RelationshipsFromPreviousLayer.GetDependentEntities(), dbValues);
251253
var allowedIds = CallHook(nestedHookcontainer, ResourceHook.BeforeUpdateRelationship, new object[] { GetIds(uniqueEntities), resourcesByRelationship, pipeline }).Cast<string>();
252254
var updated = GetAllowedEntities(uniqueEntities, allowedIds);
@@ -381,6 +383,12 @@ RelationshipProxy GetInverseRelationship(RelationshipProxy proxy)
381383
return new RelationshipProxy(_graph.GetInverseRelationship(proxy.Attribute), proxy.PrincipalType, false);
382384
}
383385

386+
IEnumerable LoadDbValues(Type containerEntityType, IEnumerable uniqueEntities, ResourceHook targetHook, RelationshipProxy[] relationshipsToNextLayer)
387+
{
388+
if (!_executorHelper.ShouldLoadDbValues(containerEntityType, targetHook)) return null;
389+
return _executorHelper.LoadDbValues(containerEntityType, uniqueEntities, targetHook, relationshipsToNextLayer);
390+
}
391+
384392
void FireAfterUpdateRelationship(IResourceHookContainer container, IEntityNode node, ResourcePipeline pipeline)
385393
{
386394
var resourcesByRelationship = CreateRelationshipHelper(node.EntityType, node.RelationshipsFromPreviousLayer.GetDependentEntities());

‎src/JsonApiDotNetCore/Services/EntityResourceService.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,10 @@ public virtual async Task<TResource> CreateAsync(TResource resource)
116116
}
117117
public virtual async Task<bool> DeleteAsync(TId id)
118118
{
119-
var entity = await _entities.GetAsync(id);
119+
var entity = (TEntity)Activator.CreateInstance(typeof(TEntity));
120+
entity.Id = id;
120121
if (!IsNull(_hookExecutor, entity)) _hookExecutor.BeforeDelete(AsList(entity), ResourcePipeline.Delete);
121-
var succeeded = await _entities.DeleteAsync(entity);
122+
var succeeded = await _entities.DeleteAsync(entity.Id);
122123
if (!IsNull(_hookExecutor, entity)) _hookExecutor.AfterDelete(AsList(entity), ResourcePipeline.Delete, succeeded);
123124
return succeeded;
124125
}

‎test/UnitTests/ResourceHooks/DiscoveryTests.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,52 @@ public void Hook_Discovery_With_Inheritance()
5252
Assert.Contains(ResourceHook.BeforeDelete, hookConfig.ImplementedHooks);
5353
Assert.Contains(ResourceHook.AfterDelete, hookConfig.ImplementedHooks);
5454
}
55+
56+
57+
public class YetAnotherDummy : Identifiable { }
58+
public class YetAnotherDummyResourceDefinition : ResourceDefinition<YetAnotherDummy>
59+
{
60+
public YetAnotherDummyResourceDefinition() : base(new ResourceGraphBuilder().AddResource<YetAnotherDummy>().Build()) { }
61+
62+
public override IEnumerable<YetAnotherDummy> BeforeDelete(HashSet<YetAnotherDummy> entities, ResourcePipeline pipeline) { return entities; }
63+
64+
[LoadDatabaseValues(false)]
65+
public override void AfterDelete(HashSet<YetAnotherDummy> entities, ResourcePipeline pipeline, bool succeeded) { }
66+
}
67+
[Fact]
68+
public void LoadDatabaseValues_Attribute_Not_Allowed()
69+
{
70+
// assert
71+
Assert.Throws<JsonApiSetupException>(() =>
72+
{
73+
// arrange & act
74+
var hookConfig = new HooksDiscovery<YetAnotherDummy>();
75+
});
76+
77+
}
78+
79+
public class DoubleDummy : Identifiable { }
80+
public class DoubleDummyResourceDefinition1 : ResourceDefinition<DoubleDummy>
81+
{
82+
public DoubleDummyResourceDefinition1() : base(new ResourceGraphBuilder().AddResource<DoubleDummy>().Build()) { }
83+
84+
public override IEnumerable<DoubleDummy> BeforeDelete(HashSet<DoubleDummy> entities, ResourcePipeline pipeline) { return entities; }
85+
}
86+
public class DoubleDummyResourceDefinition2 : ResourceDefinition<DoubleDummy>
87+
{
88+
public DoubleDummyResourceDefinition2() : base(new ResourceGraphBuilder().AddResource<DoubleDummy>().Build()) { }
89+
90+
public override void AfterDelete(HashSet<DoubleDummy> entities, ResourcePipeline pipeline, bool succeeded) { }
91+
}
92+
[Fact]
93+
public void Multiple_Implementations_Of_ResourceDefinitions()
94+
{
95+
// assert
96+
Assert.Throws<JsonApiSetupException>(() =>
97+
{
98+
// arrange & act
99+
var hookConfig = new HooksDiscovery<DoubleDummy>();
100+
});
101+
}
55102
}
56103
}

0 commit comments

Comments
 (0)