Merge pull request #533 from json-api-dotnet/fix/hooks-hotfix

Hotfixes related to Resource Hooks

Author: maurei

src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs

@@ -58,15 +58,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                 .WithOne(t => t.ParentTodoItem)
                 .HasForeignKey(t => t.ParentTodoItemId);
 
-            modelBuilder.Entity<Person>()
-                .HasOne(p => p.Passport)
-                .WithOne(p => p.Person)
-                .HasForeignKey<Person>(p => p.PassportId);
-
             modelBuilder.Entity<Passport>()
                 .HasOne(p => p.Person)
                 .WithOne(p => p.Passport)
-                .HasForeignKey<Person>(p => p.PassportId);
+                .HasForeignKey<Person>(p => p.PassportId)
+                .OnDelete(DeleteBehavior.SetNull);
 
             modelBuilder.Entity<TodoItem>()
                 .HasOne(p => p.ToOnePerson)

src/Examples/JsonApiDotNetCoreExample/Resources/LockableResourceBase.cs renamed to src/Examples/JsonApiDotNetCoreExample/Resources/LockableResource.cs

@@ -7,9 +7,9 @@
 
 namespace JsonApiDotNetCoreExample.Resources
 {
-    public abstract class LockableResourceBase<T> : ResourceDefinition<T> where T : class, IIsLockable, IIdentifiable
+    public abstract class LockableResource<T> : ResourceDefinition<T> where T : class, IIsLockable, IIdentifiable
     {
-        protected LockableResourceBase(IResourceGraph graph) : base(graph) { }
+        protected LockableResource(IResourceGraph graph) : base(graph) { }
 
         protected void DisallowLocked(IEnumerable<T> entities)
         {

src/Examples/JsonApiDotNetCoreExample/Resources/PersonResource.cs

@@ -6,25 +6,19 @@
 
 namespace JsonApiDotNetCoreExample.Resources
 {
-    public class PersonResource : LockableResourceBase<Person>
+    public class PersonResource : LockableResource<Person>
     {
         public PersonResource(IResourceGraph graph) : base(graph) { }
 
-        public override IEnumerable<string> BeforeUpdateRelationship(HashSet<string> ids, IRelationshipsDictionary<Person> resourcesByRelationship, ResourcePipeline pipeline)
+        public override IEnumerable<string> BeforeUpdateRelationship(HashSet<string> ids, IRelationshipsDictionary<Person> entitiesByRelationship, ResourcePipeline pipeline)
         {
-            BeforeImplicitUpdateRelationship(resourcesByRelationship, pipeline);
+            BeforeImplicitUpdateRelationship(entitiesByRelationship, pipeline);
             return ids;
         }
 
-        //[LoadDatabaseValues(true)]
-        //public override IEnumerable<Person> BeforeUpdate(IResourceDiff<Person> entityDiff, ResourcePipeline pipeline)
-        //{
-        //    return entityDiff.Entities;
-        //}
-
-        public override void BeforeImplicitUpdateRelationship(IRelationshipsDictionary<Person> resourcesByRelationship, ResourcePipeline pipeline)
+        public override void BeforeImplicitUpdateRelationship(IRelationshipsDictionary<Person> entitiesByRelationship, ResourcePipeline pipeline)
         {
-            resourcesByRelationship.GetByRelationship<Passport>().ToList().ForEach(kvp => DisallowLocked(kvp.Value));
+            entitiesByRelationship.GetByRelationship<Passport>().ToList().ForEach(kvp => DisallowLocked(kvp.Value));
         }
     }
 }

src/Examples/JsonApiDotNetCoreExample/Resources/TodoResource.cs

@@ -7,7 +7,7 @@
 
 namespace JsonApiDotNetCoreExample.Resources
 {
-    public class TodoResource : LockableResourceBase<TodoItem>
+    public class TodoResource : LockableResource<TodoItem>
     {
         public TodoResource(IResourceGraph graph) : base(graph) { }
 

src/JsonApiDotNetCore/Hooks/Execution/EntityDiffs.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Linq;
 using JsonApiDotNetCore.Internal;
 using JsonApiDotNetCore.Models;
@@ -12,40 +13,38 @@ namespace JsonApiDotNetCore.Hooks
     /// Contains the resources from the request and the corresponding database values.
     /// 
     /// Also contains information about updated relationships through 
-    /// implementation of IRelationshipsDictionary<typeparamref name="TEntity"/>>
+    /// implementation of IRelationshipsDictionary<typeparamref name="TResource"/>>
     /// </summary>
-    public interface IEntityDiff<TEntity> : IRelationshipsDictionary<TEntity>, IEnumerable<EntityDiffPair<TEntity>> where TEntity : class, IIdentifiable
+    public interface IEntityDiffs<TResource> :  IEnumerable<EntityDiffPair<TResource>> where TResource : class, IIdentifiable
     {
         /// <summary>
         /// The database values of the resources affected by the request.
         /// </summary>
-        HashSet<TEntity> DatabaseValues { get; }
+        HashSet<TResource> DatabaseValues { get; }
 
         /// <summary>
         /// The resources that were affected by the request.
         /// </summary>
-        HashSet<TEntity> Entities { get; }
+        EntityHashSet<TResource> Entities { get; }
+
     }
 
     /// <inheritdoc />
-    public class EntityDiffs<TEntity> : IEntityDiff<TEntity> where TEntity : class, IIdentifiable
+    public class EntityDiffs<TResource> : IEntityDiffs<TResource> where TResource : class, IIdentifiable
     {
         /// <inheritdoc />
-        public HashSet<TEntity> DatabaseValues { get => _databaseValues ?? ThrowNoDbValuesError(); }
-        private readonly HashSet<TEntity> _databaseValues;
-        private readonly bool _databaseValuesLoaded;
-
-        /// <inheritdoc />
-        public HashSet<TEntity> Entities { get; private set; }
+        public HashSet<TResource> DatabaseValues { get => _databaseValues ?? ThrowNoDbValuesError(); }
         /// <inheritdoc />
-        public RelationshipsDictionary<TEntity> AffectedRelationships { get; private set; }
+        public EntityHashSet<TResource> Entities { get; private set; }
 
-        public EntityDiffs(HashSet<TEntity> requestEntities,
-                          HashSet<TEntity> databaseEntities,
-                          Dictionary<RelationshipAttribute, HashSet<TEntity>> relationships) 
+        private readonly HashSet<TResource> _databaseValues;
+        private readonly bool _databaseValuesLoaded;
+
+        public EntityDiffs(HashSet<TResource> requestEntities,
+                          HashSet<TResource> databaseEntities,
+                          Dictionary<RelationshipAttribute, HashSet<TResource>> relationships)
         {
-            Entities = requestEntities;
-            AffectedRelationships = new RelationshipsDictionary<TEntity>(relationships);
+            Entities = new EntityHashSet<TResource>(requestEntities, relationships);
             _databaseValues = databaseEntities;
             _databaseValuesLoaded |= _databaseValues != null;
         }
@@ -55,39 +54,27 @@ public EntityDiffs(HashSet<TEntity> requestEntities,
         /// </summary>
         internal EntityDiffs(IEnumerable requestEntities,
                   IEnumerable databaseEntities,
-                  Dictionary<RelationshipAttribute, IEnumerable> relationships) 
-            : this((HashSet<TEntity>)requestEntities, (HashSet<TEntity>)databaseEntities, TypeHelper.ConvertRelationshipDictionary<TEntity>(relationships)) { }
-
+                  Dictionary<RelationshipAttribute, IEnumerable> relationships)
+            : this((HashSet<TResource>)requestEntities, (HashSet<TResource>)databaseEntities, TypeHelper.ConvertRelationshipDictionary<TResource>(relationships)) { }
 
-        /// <inheritdoc />
-        public Dictionary<RelationshipAttribute, HashSet<TEntity>> GetByRelationship<TPrincipalResource>() where TPrincipalResource : class, IIdentifiable
-        {
-            return GetByRelationship(typeof(TPrincipalResource));
-        }
-
-        /// <inheritdoc />
-        public Dictionary<RelationshipAttribute, HashSet<TEntity>> GetByRelationship(Type principalType)
-        {
-            return AffectedRelationships.GetByRelationship(principalType);
-        }
 
         /// <inheritdoc />
-        public IEnumerator<EntityDiffPair<TEntity>> GetEnumerator()
+        public IEnumerator<EntityDiffPair<TResource>> GetEnumerator()
         {
             if (!_databaseValuesLoaded) ThrowNoDbValuesError();
 
             foreach (var entity in Entities)
             {
-                TEntity currentValueInDatabase = null;
+                TResource currentValueInDatabase = null;
                 currentValueInDatabase = _databaseValues.Single(e => entity.StringId == e.StringId);
-                yield return new EntityDiffPair<TEntity>(entity, currentValueInDatabase);
+                yield return new EntityDiffPair<TResource>(entity, currentValueInDatabase);
             }
         }
 
         /// <inheritdoc />
         IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
 
-        private HashSet<TEntity> ThrowNoDbValuesError()
+        private HashSet<TResource> ThrowNoDbValuesError()
         {
             throw new MemberAccessException("Cannot access database entities if the LoadDatabaseValues option is set to false");
         }
@@ -97,9 +84,9 @@ private HashSet<TEntity> ThrowNoDbValuesError()
     /// A wrapper that contains an entity that is affected by the request, 
     /// matched to its current database value
     /// </summary>
-    public class EntityDiffPair<TEntity> where TEntity : class, IIdentifiable
+    public class EntityDiffPair<TResource> where TResource : class, IIdentifiable
     {
-        public EntityDiffPair(TEntity entity, TEntity databaseValue)
+        public EntityDiffPair(TResource entity, TResource databaseValue)
         {
             Entity = entity;
             DatabaseValue = databaseValue;
@@ -108,10 +95,10 @@ public EntityDiffPair(TEntity entity, TEntity databaseValue)
         /// <summary>
         /// The resource from the request matching the resource from the database.
         /// </summary>
-        public TEntity Entity { get; private set; }
+        public TResource Entity { get; private set; }
         /// <summary>
         /// The resource from the database matching the resource from the request.
         /// </summary>
-        public TEntity DatabaseValue { get; private set; }
+        public TResource DatabaseValue { get; private set; }
     }
 }

src/JsonApiDotNetCore/Hooks/Execution/EntityHashSet.cs

@@ -3,6 +3,8 @@
 using System.Collections;
 using JsonApiDotNetCore.Internal;
 using System;
+using System.Collections.ObjectModel;
+using System.Collections.Immutable;
 
 namespace JsonApiDotNetCore.Hooks
 {
@@ -12,7 +14,7 @@ namespace JsonApiDotNetCore.Hooks
     /// Also contains information about updated relationships through 
     /// implementation of IAffectedRelationshipsDictionary<typeparamref name="TResource"/>>
     /// </summary>
-    public interface IEntityHashSet<TResource> : IRelationshipsDictionary<TResource>, IEnumerable<TResource> where TResource : class, IIdentifiable { }
+    public interface IEntityHashSet<TResource> : IByAffectedRelationships<TResource>, IReadOnlyCollection<TResource> where TResource : class, IIdentifiable { }
 
     /// <summary>
     /// Implementation of IResourceHashSet{TResource}.
@@ -24,13 +26,16 @@ public interface IEntityHashSet<TResource> : IRelationshipsDictionary<TResource>
     /// </summary>
     public class EntityHashSet<TResource> : HashSet<TResource>, IEntityHashSet<TResource> where TResource : class, IIdentifiable
     {
+    
+
         /// <inheritdoc />
-        public RelationshipsDictionary<TResource> AffectedRelationships { get; private set; }
+        public Dictionary<RelationshipAttribute, HashSet<TResource>> AffectedRelationships { get => _relationships; }
+        private readonly RelationshipsDictionary<TResource> _relationships;
 
         public EntityHashSet(HashSet<TResource> entities,
                         Dictionary<RelationshipAttribute, HashSet<TResource>> relationships) : base(entities)
         {
-            AffectedRelationships = new RelationshipsDictionary<TResource>(relationships);
+            _relationships = new RelationshipsDictionary<TResource>(relationships);
         }
 
         /// <summary>
@@ -44,13 +49,13 @@ internal EntityHashSet(IEnumerable entities,
         /// <inheritdoc />
         public Dictionary<RelationshipAttribute, HashSet<TResource>> GetByRelationship(Type principalType)
         {
-            return AffectedRelationships.GetByRelationship(principalType);
+            return _relationships.GetByRelationship(principalType);
         }
 
         /// <inheritdoc />
-        public Dictionary<RelationshipAttribute, HashSet<TResource>> GetByRelationship<TPrincipalResource>()  where TPrincipalResource : class, IIdentifiable
+        public Dictionary<RelationshipAttribute, HashSet<TResource>> GetByRelationship<TRelatedResource>()  where TRelatedResource : class, IIdentifiable
         {
-            return GetByRelationship<TPrincipalResource>();
+            return GetByRelationship(typeof(TRelatedResource));
         }
     }
 }

src/JsonApiDotNetCore/Hooks/Execution/HookExecutorHelper.cs

@@ -82,7 +82,7 @@ public IResourceHookContainer<TEntity> GetResourceHookContainer<TEntity>(Resourc
         public IEnumerable LoadDbValues(PrincipalType entityTypeForRepository, IEnumerable entities, ResourceHook hook, params RelationshipAttribute[] relationships)
         {
             var paths = relationships.Select(p => p.RelationshipPath).ToArray();
-            var idType = GetIdentifierType(entityTypeForRepository);
+            var idType = TypeHelper.GetIdentifierType(entityTypeForRepository);
             var parameterizedGetWhere = GetType()
                     .GetMethod(nameof(GetWhereAndInclude), BindingFlags.NonPublic | BindingFlags.Instance)
                     .MakeGenericMethod(entityTypeForRepository, idType);
@@ -144,11 +144,6 @@ IHooksDiscovery GetHookDiscovery(Type entityType)
             return discovery;
         }
 
-        Type GetIdentifierType(Type entityType)
-        {
-            return entityType.GetProperty("Id").PropertyType;
-        }
-
         IEnumerable<TEntity> GetWhereAndInclude<TEntity, TId>(IEnumerable<TId> ids, string[] relationshipPaths) where TEntity : class, IIdentifiable<TId>
         {
             var repo = GetRepository<TEntity, TId>();

src/JsonApiDotNetCore/Hooks/Execution/IHookExecutorHelper.cs

@@ -42,7 +42,20 @@ internal interface IHookExecutorHelper
         /// <summary>
         /// For a set of entities, loads current values from the database
         /// </summary>
+        /// <param name="repositoryEntityType">type of the entities to be loaded</param>
+        /// <param name="entities">The set of entities to load the db values for</param>
+        /// <param name="hook">The hook in which the db values will be displayed.</param>
+        /// <param name="relationships">Relationships that need to be included on entities.</param>
         IEnumerable LoadDbValues(Type repositoryEntityType, IEnumerable entities, ResourceHook hook, params RelationshipAttribute[] relationships);
-        bool ShouldLoadDbValues(Type containerEntityType, ResourceHook hook);
+
+        /// <summary>
+        /// Checks if the display database values option is allowed for the targetd hook, and for 
+        /// a given resource of type <paramref name="entityType"/> checks if this hook is implemented and if the
+        /// database values option is enabled.
+        /// </summary>
+        /// <returns><c>true</c>, if load db values was shoulded, <c>false</c> otherwise.</returns>
+        /// <param name="entityType">Container entity type.</param>
+        /// <param name="hook">Hook.</param>
+        bool ShouldLoadDbValues(Type entityType, ResourceHook hook);
     }
 }