diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs index cb76720a75..8530cd8fa2 100644 --- a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs +++ b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs @@ -42,9 +42,9 @@ public class JsonApiOptions : IJsonApiOptions /// /// Whether or not ResourceHooks are enabled. /// - /// Default is set to for backward compatibility. + /// Default is set to /// - public bool EnableResourceHooks { get; set; } = false; + public bool EnableResourceHooks { get; set; } = true; /// /// Whether or not database values should be included by default diff --git a/src/JsonApiDotNetCore/Hooks/Discovery/HooksDiscovery.cs b/src/JsonApiDotNetCore/Hooks/Discovery/HooksDiscovery.cs index 9b95e24562..bfade1c50d 100644 --- a/src/JsonApiDotNetCore/Hooks/Discovery/HooksDiscovery.cs +++ b/src/JsonApiDotNetCore/Hooks/Discovery/HooksDiscovery.cs @@ -1,9 +1,9 @@ using System; using System.Collections.Generic; using System.Linq; -using JsonApiDotNetCore.Graph; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Models; +using Microsoft.Extensions.DependencyInjection; namespace JsonApiDotNetCore.Hooks { @@ -12,6 +12,7 @@ namespace JsonApiDotNetCore.Hooks /// public class HooksDiscovery : IHooksDiscovery where TResource : class, IIdentifiable { + private readonly Type _boundResourceDefinitionType = typeof(ResourceDefinition); private readonly ResourceHook[] _allHooks; private readonly ResourceHook[] _databaseValuesAttributeAllowed = { @@ -19,71 +20,66 @@ public class HooksDiscovery : IHooksDiscovery where TResou ResourceHook.BeforeUpdateRelationship, ResourceHook.BeforeDelete }; + /// public ResourceHook[] ImplementedHooks { get; private set; } public ResourceHook[] DatabaseValuesEnabledHooks { get; private set; } public ResourceHook[] DatabaseValuesDisabledHooks { get; private set; } - - public HooksDiscovery() + public HooksDiscovery(IServiceProvider provider) { _allHooks = Enum.GetValues(typeof(ResourceHook)) .Cast() .Where(h => h != ResourceHook.None) .ToArray(); - DiscoverImplementedHooksForModel(); + + Type containerType; + using (var scope = provider.CreateScope()) + { + containerType = scope.ServiceProvider.GetService(_boundResourceDefinitionType)?.GetType(); + } + + DiscoverImplementedHooks(containerType); } /// /// Discovers the implemented hooks for a model. /// /// The implemented hooks for model. - void DiscoverImplementedHooksForModel() + void DiscoverImplementedHooks(Type containerType) { - Type parameterizedResourceDefinition = typeof(ResourceDefinition); - var derivedTypes = TypeLocator.GetDerivedTypes(typeof(TResource).Assembly, parameterizedResourceDefinition).ToList(); - - - var implementedHooks = new List(); - var enabledHooks = new List() { ResourceHook.BeforeImplicitUpdateRelationship } ; - var disabledHooks = new List(); - Type targetType = null; - try + if (containerType == null || containerType == _boundResourceDefinitionType) { - targetType = derivedTypes.SingleOrDefault(); // multiple containers is not supported + return; } - catch - { - throw new JsonApiSetupException($"It is currently not supported to" + - "implement hooks across multiple implementations of ResourceDefinition"); - } - if (targetType != null) + + var implementedHooks = new List(); + // this hook can only be used with enabled database values + var databaseValuesEnabledHooks = new List { ResourceHook.BeforeImplicitUpdateRelationship }; + var databaseValuesDisabledHooks = new List(); + foreach (var hook in _allHooks) { - foreach (var hook in _allHooks) + var method = containerType.GetMethod(hook.ToString("G")); + if (method.DeclaringType == _boundResourceDefinitionType) + continue; + + implementedHooks.Add(hook); + var attr = method.GetCustomAttributes(true).OfType().SingleOrDefault(); + if (attr != null) { - var method = targetType.GetMethod(hook.ToString("G")); - if (method.DeclaringType != parameterizedResourceDefinition) + if (!_databaseValuesAttributeAllowed.Contains(hook)) { - implementedHooks.Add(hook); - var attr = method.GetCustomAttributes(true).OfType().SingleOrDefault(); - if (attr != null) - { - if (!_databaseValuesAttributeAllowed.Contains(hook)) - { - throw new JsonApiSetupException($"DatabaseValuesAttribute cannot be used on hook" + - $"{hook.ToString("G")} in resource definition {parameterizedResourceDefinition.Name}"); - } - var targetList = attr.value ? enabledHooks : disabledHooks; - targetList.Add(hook); - } - } + throw new JsonApiSetupException($"DatabaseValuesAttribute cannot be used on hook" + + $"{hook.ToString("G")} in resource definition {containerType.Name}"); + } + var targetList = attr.value ? databaseValuesEnabledHooks : databaseValuesDisabledHooks; + targetList.Add(hook); } - } - ImplementedHooks = implementedHooks.ToArray(); - DatabaseValuesDisabledHooks = disabledHooks.ToArray(); - DatabaseValuesEnabledHooks = enabledHooks.ToArray(); + ImplementedHooks = implementedHooks.ToArray(); + DatabaseValuesDisabledHooks = databaseValuesDisabledHooks.ToArray(); + DatabaseValuesEnabledHooks = databaseValuesEnabledHooks.ToArray(); } } -} +} \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Hooks/Discovery/LoadDatabaseValuesAttribute.cs b/src/JsonApiDotNetCore/Hooks/Discovery/LoadDatabaseValuesAttribute.cs index 1477cc0ec1..6a47e9d2a0 100644 --- a/src/JsonApiDotNetCore/Hooks/Discovery/LoadDatabaseValuesAttribute.cs +++ b/src/JsonApiDotNetCore/Hooks/Discovery/LoadDatabaseValuesAttribute.cs @@ -1,10 +1,10 @@ using System; namespace JsonApiDotNetCore.Hooks { - public class LoaDatabaseValues : Attribute + public class LoadDatabaseValues : Attribute { public readonly bool value; - public LoaDatabaseValues(bool mode = true) + public LoadDatabaseValues(bool mode = true) { value = mode; } diff --git a/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs index c43ae530c4..e0de8a11ff 100644 --- a/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs +++ b/src/JsonApiDotNetCore/Hooks/Execution/DiffableEntityHashSet.cs @@ -90,7 +90,7 @@ public IEnumerable> GetDiffs() private HashSet ThrowNoDbValuesError() { - throw new MemberAccessException("Cannot iterate over the diffs if the LoaDatabaseValues option is set to false"); + throw new MemberAccessException($"Cannot iterate over the diffs if the ${nameof(LoadDatabaseValues)} option is set to false"); } } diff --git a/test/UnitTests/Graph/TypeLocator_Tests.cs b/test/UnitTests/Graph/TypeLocator_Tests.cs index 502cfa1139..4c727b0253 100644 --- a/test/UnitTests/Graph/TypeLocator_Tests.cs +++ b/test/UnitTests/Graph/TypeLocator_Tests.cs @@ -1,5 +1,4 @@ using System; -using System.Reflection; using JsonApiDotNetCore.Graph; using JsonApiDotNetCore.Models; using Xunit; diff --git a/test/UnitTests/ResourceHooks/DiscoveryTests.cs b/test/UnitTests/ResourceHooks/DiscoveryTests.cs index 3a37184784..3c3dab0708 100644 --- a/test/UnitTests/ResourceHooks/DiscoveryTests.cs +++ b/test/UnitTests/ResourceHooks/DiscoveryTests.cs @@ -5,6 +5,8 @@ using JsonApiDotNetCore.Builders; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Internal.Contracts; +using System; +using Microsoft.Extensions.DependencyInjection; namespace UnitTests.ResourceHooks { @@ -19,41 +21,46 @@ public DummyResourceDefinition() : base(new ResourceGraphBuilder().AddResource entities, ResourcePipeline pipeline, bool succeeded) { } } + private IServiceProvider MockProvider(object service) where TResource : class, IIdentifiable + { + var services = new ServiceCollection(); + services.AddScoped((_) => (ResourceDefinition)service); + return services.BuildServiceProvider(); + } + [Fact] public void Hook_Discovery() { // Arrange & act - var hookConfig = new HooksDiscovery(); + var hookConfig = new HooksDiscovery(MockProvider(new DummyResourceDefinition())); // Assert Assert.Contains(ResourceHook.BeforeDelete, hookConfig.ImplementedHooks); Assert.Contains(ResourceHook.AfterDelete, hookConfig.ImplementedHooks); - } public class AnotherDummy : Identifiable { } public abstract class ResourceDefintionBase : ResourceDefinition where T : class, IIdentifiable { - protected ResourceDefintionBase(IResourceGraph resourceGraph) : base(resourceGraph) { } - + public ResourceDefintionBase(IResourceGraph resourceGraph) : base(resourceGraph) { } public override IEnumerable BeforeDelete(IEntityHashSet affected, ResourcePipeline pipeline) { return affected; } public override void AfterDelete(HashSet entities, ResourcePipeline pipeline, bool succeeded) { } } public class AnotherDummyResourceDefinition : ResourceDefintionBase { - public AnotherDummyResourceDefinition() : base(new ResourceGraphBuilder().AddResource().Build()) { } + public AnotherDummyResourceDefinition() : base(new ResourceGraphBuilder().AddResource().Build()) { } } + [Fact] public void Hook_Discovery_With_Inheritance() { // Arrange & act - var hookConfig = new HooksDiscovery(); + var hookConfig = new HooksDiscovery(MockProvider(new AnotherDummyResourceDefinition())); // Assert Assert.Contains(ResourceHook.BeforeDelete, hookConfig.ImplementedHooks); Assert.Contains(ResourceHook.AfterDelete, hookConfig.ImplementedHooks); } - public class YetAnotherDummy : Identifiable { } public class YetAnotherDummyResourceDefinition : ResourceDefinition { @@ -61,43 +68,20 @@ public YetAnotherDummyResourceDefinition() : base(new ResourceGraphBuilder().Add public override IEnumerable BeforeDelete(IEntityHashSet affected, ResourcePipeline pipeline) { return affected; } - [LoaDatabaseValues(false)] + [LoadDatabaseValues(false)] public override void AfterDelete(HashSet entities, ResourcePipeline pipeline, bool succeeded) { } } - [Fact] - public void LoaDatabaseValues_Attribute_Not_Allowed() - { - // assert - Assert.Throws(() => - { - // Arrange & act - var hookConfig = new HooksDiscovery(); - }); - - } - - public class DoubleDummy : Identifiable { } - public class DoubleDummyResourceDefinition1 : ResourceDefinition - { - public DoubleDummyResourceDefinition1() : base(new ResourceGraphBuilder().AddResource().Build()) { } - public override IEnumerable BeforeDelete(IEntityHashSet affected, ResourcePipeline pipeline) { return affected; } - } - public class DoubleDummyResourceDefinition2 : ResourceDefinition - { - public DoubleDummyResourceDefinition2() : base(new ResourceGraphBuilder().AddResource().Build()) { } - - public override void AfterDelete(HashSet entities, ResourcePipeline pipeline, bool succeeded) { } - } [Fact] - public void Multiple_Implementations_Of_ResourceDefinitions() + public void LoadDatabaseValues_Attribute_Not_Allowed() { // assert Assert.Throws(() => { // Arrange & act - var hookConfig = new HooksDiscovery(); + var hookConfig = new HooksDiscovery(MockProvider(new YetAnotherDummyResourceDefinition())); }); + } } }