Skip to content

Hooks discovery detection of implemented hooks issue #617

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 10 commits into from
Nov 6, 2019
4 changes: 2 additions & 2 deletions src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ public class JsonApiOptions : IJsonApiOptions
/// <summary>
/// Whether or not ResourceHooks are enabled.
///
/// Default is set to <see langword="false"/> for backward compatibility.
/// Default is set to <see langword="true"/>
/// </summary>
public bool EnableResourceHooks { get; set; } = false;
public bool EnableResourceHooks { get; set; } = true;

/// <summary>
/// Whether or not database values should be included by default
Expand Down
80 changes: 38 additions & 42 deletions src/JsonApiDotNetCore/Hooks/Discovery/HooksDiscovery.cs
Original file line number Diff line number Diff line change
@@ -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
{
Expand All @@ -12,78 +12,74 @@ namespace JsonApiDotNetCore.Hooks
/// </summary>
public class HooksDiscovery<TResource> : IHooksDiscovery<TResource> where TResource : class, IIdentifiable
{
private readonly Type _boundResourceDefinitionType = typeof(ResourceDefinition<TResource>);
private readonly ResourceHook[] _allHooks;
private readonly ResourceHook[] _databaseValuesAttributeAllowed =
{
ResourceHook.BeforeUpdate,
ResourceHook.BeforeUpdateRelationship,
ResourceHook.BeforeDelete
};

/// <inheritdoc/>
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<ResourceHook>()
.Where(h => h != ResourceHook.None)
.ToArray();
DiscoverImplementedHooksForModel();

Type containerType;
using (var scope = provider.CreateScope())
{
containerType = scope.ServiceProvider.GetService(_boundResourceDefinitionType)?.GetType();
}

DiscoverImplementedHooks(containerType);
}

/// <summary>
/// Discovers the implemented hooks for a model.
/// </summary>
/// <returns>The implemented hooks for model.</returns>
void DiscoverImplementedHooksForModel()
void DiscoverImplementedHooks(Type containerType)
{
Type parameterizedResourceDefinition = typeof(ResourceDefinition<TResource>);
var derivedTypes = TypeLocator.GetDerivedTypes(typeof(TResource).Assembly, parameterizedResourceDefinition).ToList();


var implementedHooks = new List<ResourceHook>();
var enabledHooks = new List<ResourceHook>() { ResourceHook.BeforeImplicitUpdateRelationship } ;
var disabledHooks = new List<ResourceHook>();
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<T>");
}
if (targetType != null)

var implementedHooks = new List<ResourceHook>();
// this hook can only be used with enabled database values
var databaseValuesEnabledHooks = new List<ResourceHook> { ResourceHook.BeforeImplicitUpdateRelationship };
var databaseValuesDisabledHooks = new List<ResourceHook>();
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<LoadDatabaseValues>().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<LoaDatabaseValues>().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();
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public IEnumerable<EntityDiffPair<TResource>> GetDiffs()

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

Expand Down
1 change: 0 additions & 1 deletion test/UnitTests/Graph/TypeLocator_Tests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Reflection;
using JsonApiDotNetCore.Graph;
using JsonApiDotNetCore.Models;
using Xunit;
Expand Down
52 changes: 18 additions & 34 deletions test/UnitTests/ResourceHooks/DiscoveryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using JsonApiDotNetCore.Builders;
using JsonApiDotNetCore.Internal;
using JsonApiDotNetCore.Internal.Contracts;
using System;
using Microsoft.Extensions.DependencyInjection;

namespace UnitTests.ResourceHooks
{
Expand All @@ -19,85 +21,67 @@ public DummyResourceDefinition() : base(new ResourceGraphBuilder().AddResource<D
public override void AfterDelete(HashSet<Dummy> entities, ResourcePipeline pipeline, bool succeeded) { }
}

private IServiceProvider MockProvider<TResource>(object service) where TResource : class, IIdentifiable
{
var services = new ServiceCollection();
services.AddScoped((_) => (ResourceDefinition<TResource>)service);
return services.BuildServiceProvider();
}

[Fact]
public void Hook_Discovery()
{
// Arrange & act
var hookConfig = new HooksDiscovery<Dummy>();
var hookConfig = new HooksDiscovery<Dummy>(MockProvider<Dummy>(new DummyResourceDefinition()));
// Assert
Assert.Contains(ResourceHook.BeforeDelete, hookConfig.ImplementedHooks);
Assert.Contains(ResourceHook.AfterDelete, hookConfig.ImplementedHooks);

}

public class AnotherDummy : Identifiable { }
public abstract class ResourceDefintionBase<T> : ResourceDefinition<T> where T : class, IIdentifiable
{
protected ResourceDefintionBase(IResourceGraph resourceGraph) : base(resourceGraph) { }

public ResourceDefintionBase(IResourceGraph resourceGraph) : base(resourceGraph) { }
public override IEnumerable<T> BeforeDelete(IEntityHashSet<T> affected, ResourcePipeline pipeline) { return affected; }
public override void AfterDelete(HashSet<T> entities, ResourcePipeline pipeline, bool succeeded) { }
}

public class AnotherDummyResourceDefinition : ResourceDefintionBase<AnotherDummy>
{
public AnotherDummyResourceDefinition() : base(new ResourceGraphBuilder().AddResource<Dummy>().Build()) { }
public AnotherDummyResourceDefinition() : base(new ResourceGraphBuilder().AddResource<AnotherDummy>().Build()) { }
}

[Fact]
public void Hook_Discovery_With_Inheritance()
{
// Arrange & act
var hookConfig = new HooksDiscovery<AnotherDummy>();
var hookConfig = new HooksDiscovery<AnotherDummy>(MockProvider<AnotherDummy>(new AnotherDummyResourceDefinition()));
// Assert
Assert.Contains(ResourceHook.BeforeDelete, hookConfig.ImplementedHooks);
Assert.Contains(ResourceHook.AfterDelete, hookConfig.ImplementedHooks);
}


public class YetAnotherDummy : Identifiable { }
public class YetAnotherDummyResourceDefinition : ResourceDefinition<YetAnotherDummy>
{
public YetAnotherDummyResourceDefinition() : base(new ResourceGraphBuilder().AddResource<YetAnotherDummy>().Build()) { }

public override IEnumerable<YetAnotherDummy> BeforeDelete(IEntityHashSet<YetAnotherDummy> affected, ResourcePipeline pipeline) { return affected; }

[LoaDatabaseValues(false)]
[LoadDatabaseValues(false)]
public override void AfterDelete(HashSet<YetAnotherDummy> entities, ResourcePipeline pipeline, bool succeeded) { }
}
[Fact]
public void LoaDatabaseValues_Attribute_Not_Allowed()
{
// assert
Assert.Throws<JsonApiSetupException>(() =>
{
// Arrange & act
var hookConfig = new HooksDiscovery<YetAnotherDummy>();
});

}

public class DoubleDummy : Identifiable { }
public class DoubleDummyResourceDefinition1 : ResourceDefinition<DoubleDummy>
{
public DoubleDummyResourceDefinition1() : base(new ResourceGraphBuilder().AddResource<DoubleDummy>().Build()) { }

public override IEnumerable<DoubleDummy> BeforeDelete(IEntityHashSet<DoubleDummy> affected, ResourcePipeline pipeline) { return affected; }
}
public class DoubleDummyResourceDefinition2 : ResourceDefinition<DoubleDummy>
{
public DoubleDummyResourceDefinition2() : base(new ResourceGraphBuilder().AddResource<DoubleDummy>().Build()) { }

public override void AfterDelete(HashSet<DoubleDummy> entities, ResourcePipeline pipeline, bool succeeded) { }
}
[Fact]
public void Multiple_Implementations_Of_ResourceDefinitions()
public void LoadDatabaseValues_Attribute_Not_Allowed()
{
// assert
Assert.Throws<JsonApiSetupException>(() =>
{
// Arrange & act
var hookConfig = new HooksDiscovery<DoubleDummy>();
var hookConfig = new HooksDiscovery<YetAnotherDummy>(MockProvider<YetAnotherDummy>(new YetAnotherDummyResourceDefinition()));
});

}
}
}