feat: remove GetInverseRelationship from ResourceGraph

Author: maurei

src/JsonApiDotNetCore/Hooks/Execution/HookExecutorHelper.cs

@@ -22,20 +22,15 @@ internal class HookExecutorHelper : IHookExecutorHelper
  private readonly IdentifiableComparer _comparer = new IdentifiableComparer();
  private readonly IJsonApiOptions _options;
  protected readonly IGenericProcessorFactory _genericProcessorFactory;
- protected readonly IResourceGraph _graph;
  protected readonly Dictionary<DependentType, IResourceHookContainer> _hookContainers;
  protected readonly Dictionary<DependentType, IHooksDiscovery> _hookDiscoveries;
  protected readonly List<ResourceHook> _targetedHooksForRelatedEntities;
 
- public HookExecutorHelper(
- IGenericProcessorFactory genericProcessorFactory,
- IResourceGraph graph,
- IJsonApiOptions options
- )
+ public HookExecutorHelper(IGenericProcessorFactory genericProcessorFactory,
+ IJsonApiOptions options)
  {
  _options = options;
  _genericProcessorFactory = genericProcessorFactory;
- _graph = graph;
  _hookContainers = new Dictionary<DependentType, IResourceHookContainer>();
  _hookDiscoveries = new Dictionary<DependentType, IHooksDiscovery>();
  _targetedHooksForRelatedEntities = new List<ResourceHook>();

src/JsonApiDotNetCore/Hooks/ResourceHookExecutor.cs

@@ -21,19 +21,19 @@ internal class ResourceHookExecutor : IResourceHookExecutor
  private readonly ITraversalHelper _traversalHelper;
  private readonly IIncludeService _includeService;
  private readonly ITargetedFields _targetedFields;
- private readonly IResourceGraph _graph;
+ private readonly IInverseRelationships _inverseRelationships;
  public ResourceHookExecutor(
  IHookExecutorHelper executorHelper,
  ITraversalHelper traversalHelper,
  ITargetedFields targetedFields,
  IIncludeService includedRelationships,
- IResourceGraph resourceGraph)
+ IInverseRelationships inverseRelationships)
  {
  _executorHelper = executorHelper;
  _traversalHelper = traversalHelper;
  _targetedFields = targetedFields;
  _includeService = includedRelationships;
- _graph = resourceGraph;
+ _inverseRelationships = inverseRelationships;
  }
 
  /// <inheritdoc/>
@@ -324,7 +324,7 @@ Dictionary<RelationshipAttribute, IEnumerable> ReplaceKeysWithInverseRelationshi
  /// If it isn't, JADNC currently knows nothing about this relationship pointing back, and it 
  /// currently cannot fire hooks for entities resolved through inverse relationships.
  var inversableRelationshipAttributes = entitiesByRelationship.Where(kvp => kvp.Key.InverseNavigation != null);
- return inversableRelationshipAttributes.ToDictionary(kvp => _graph.GetInverseRelationship(kvp.Key), kvp => kvp.Value);
+ return inversableRelationshipAttributes.ToDictionary(kvp => _inverseRelationships.GetInverse(kvp.Key), kvp => kvp.Value);
  }
 
  /// <summary>
@@ -337,7 +337,7 @@ void FireForAffectedImplicits(Type entityTypeToInclude, Dictionary<RelationshipA
  if (container == null) return;
  var implicitAffected = _executorHelper.LoadImplicitlyAffected(implicitsTarget, existingImplicitEntities);
  if (!implicitAffected.Any()) return;
- var inverse = implicitAffected.ToDictionary(kvp => _graph.GetInverseRelationship(kvp.Key), kvp => kvp.Value);
+ var inverse = implicitAffected.ToDictionary(kvp => _inverseRelationships.GetInverse(kvp.Key), kvp => kvp.Value);
  var resourcesByRelationship = CreateRelationshipHelper(entityTypeToInclude, inverse);
  CallHook(container, ResourceHook.BeforeImplicitUpdateRelationship, new object[] { resourcesByRelationship, pipeline, });
  }

src/JsonApiDotNetCore/Internal/Contracts/IContextEntityProvider.cs

@@ -8,6 +8,11 @@ namespace JsonApiDotNetCore.Internal.Contracts
  /// </summary>
  public interface IContextEntityProvider
  {
+ /// <summary>
+ /// Gets all registered context entities
+ /// </summary>
+ ContextEntity[] GetContextEntities();
+
  /// <summary>
  /// Get the resource metadata by the DbSet property name
  /// </summary>

src/JsonApiDotNetCore/Internal/Contracts/IResourceGraph.cs

@@ -4,14 +4,9 @@ namespace JsonApiDotNetCore.Internal.Contracts
 {
  /// <summary>
  /// A cache for the models in entity core
- /// TODO: separate context entity getting part from relationship resolving part.
- /// These are two deviating responsibilities that often do not need to be exposed
- /// at the same time.
  /// </summary>
  public interface IResourceGraph : IContextEntityProvider
  {
- RelationshipAttribute GetInverseRelationship(RelationshipAttribute relationship);
-
  /// <summary>
  /// Was built against an EntityFrameworkCore DbContext ?
  /// </summary>

src/JsonApiDotNetCore/Internal/InverseRelationships.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using JsonApiDotNetCore.Data;
 using JsonApiDotNetCore.Internal.Contracts;
 using JsonApiDotNetCore.Models;
@@ -24,28 +25,45 @@ public interface IInverseRelationships
  /// deal with resolving the inverse relationships. 
  /// </summary>
  void Resolve();
+
+
+ /// <summary>
+ /// Traverses the resource graph for the inverse relationship of the provided
+ /// <paramref name="relationship"/>;
+ /// </summary>
+ /// <param name="relationship"></param>
+ RelationshipAttribute GetInverse(RelationshipAttribute relationship);
  }
 
  /// <inheritdoc />
  public class InverseRelationships : IInverseRelationships
  {
- private readonly ResourceGraph _graph;
+ private readonly IContextEntityProvider _provider;
  private readonly IDbContextResolver _resolver;
 
- public InverseRelationships(IResourceGraph graph, IDbContextResolver resolver = null)
+ public InverseRelationships(IContextEntityProvider provider, IDbContextResolver resolver = null)
  {
- _graph = (ResourceGraph)graph;
+ _provider = (ResourceGraph)provider;
  _resolver = resolver;
  }
 
+ /// <inheritdoc />
+ public RelationshipAttribute GetInverse(RelationshipAttribute relationship)
+ {
+ if (relationship.InverseNavigation == null) return null;
+ return _provider.GetContextEntity(relationship.DependentType)
+ .Relationships
+ .SingleOrDefault(r => r.InternalRelationshipName == relationship.InverseNavigation);
+ }
+
  /// <inheritdoc />
  public void Resolve()
  {
  if (EntityFrameworkCoreIsEnabled())
  {
  DbContext context = _resolver.GetContext();
 
- foreach (ContextEntity ce in _graph.Entities)
+ foreach (ContextEntity ce in _provider.GetContextEntities())
  {
  IEntityType meta = context.Model.FindEntityType(ce.EntityType);
  if (meta == null) continue;
@@ -63,7 +81,6 @@ public void Resolve()
  /// If EF Core is not being used, we're expecting the resolver to not be registered.
  /// </summary>
  /// <returns><c>true</c>, if entity framework core was enabled, <c>false</c> otherwise.</returns>
- /// <param name="resolver">Resolver.</param>
  private bool EntityFrameworkCoreIsEnabled()
  {
  return _resolver != null;

src/JsonApiDotNetCore/Internal/ResourceGraph.cs

@@ -47,11 +47,8 @@ internal ResourceGraph(List<ContextEntity> entities, bool usesDbContext, List<Va
  /// <inheritdoc />
  public bool UsesDbContext { get; }
 
- public RelationshipAttribute GetInverseRelationship(RelationshipAttribute relationship)
- {
- if (relationship.InverseNavigation == null) return null;
- return GetContextEntity(relationship.DependentType).Relationships.SingleOrDefault(r => r.InternalRelationshipName == relationship.InverseNavigation);
- }
+ /// <inheritdoc />
+ public ContextEntity[] GetContextEntities() => Entities.ToArray();
 
  /// <inheritdoc />
  public ContextEntity GetContextEntity(string entityName)

test/UnitTests/ResourceHooks/ResourceHooksTestsSetup.cs

@@ -142,14 +142,13 @@ protected List<TodoItem> CreateTodoWithOwner()
 
  public class HooksTestsSetup : HooksDummyData
  {
- (IResourceGraph, Mock<ITargetedFields>, Mock<IIncludeService>, Mock<IGenericProcessorFactory>, IJsonApiOptions) CreateMocks()
+ (IInverseRelationships, Mock<ITargetedFields>, Mock<IIncludeService>, Mock<IGenericProcessorFactory>, IJsonApiOptions) CreateMocks()
  {
  var pfMock = new Mock<IGenericProcessorFactory>();
- var graph = _graph;
  var ufMock = new Mock<ITargetedFields>();
  var iqsMock = new Mock<IIncludeService>();
  var optionsMock = new JsonApiOptions { LoaDatabaseValues = false };
- return (graph, ufMock, iqsMock, pfMock, optionsMock);
+ return (new InverseRelationships(_graph), ufMock, iqsMock, pfMock, optionsMock);
  }
 
  internal (Mock<IIncludeService>, ResourceHookExecutor, Mock<IResourceHookContainer<TMain>>) CreateTestObjects<TMain>(IHooksDiscovery<TMain> mainDiscovery = null)
@@ -159,13 +158,13 @@ public class HooksTestsSetup : HooksDummyData
  var mainResource = CreateResourceDefinition(mainDiscovery);
 
  // mocking the GenericProcessorFactory and JsonApiContext and wiring them up.
- var (graph, ufMock, iqMock, gpfMock, options) = CreateMocks();
+ var (inverse, ufMock, iqMock, gpfMock, options) = CreateMocks();
 
  SetupProcessorFactoryForResourceDefinition(gpfMock, mainResource.Object, mainDiscovery, null);
 
- var execHelper = new HookExecutorHelper(gpfMock.Object, graph, options);
- var traversalHelper = new TraversalHelper(graph, ufMock.Object);
- var hookExecutor = new ResourceHookExecutor(execHelper, traversalHelper, ufMock.Object, iqMock.Object, graph);
+ var execHelper = new HookExecutorHelper(gpfMock.Object, options);
+ var traversalHelper = new TraversalHelper(_graph, ufMock.Object);
+ var hookExecutor = new ResourceHookExecutor(execHelper, traversalHelper, ufMock.Object, iqMock.Object, inverse);
 
  return (iqMock, hookExecutor, mainResource);
  }
@@ -184,16 +183,16 @@ public class HooksTestsSetup : HooksDummyData
  var nestedResource = CreateResourceDefinition(nestedDiscovery);
 
  // mocking the GenericProcessorFactory and JsonApiContext and wiring them up.
- var (graph, ufMock, iqMock, gpfMock, options) = CreateMocks();
+ var (inverse, ufMock, iqMock, gpfMock, options) = CreateMocks();
 
  var dbContext = repoDbContextOptions != null ? new AppDbContext(repoDbContextOptions) : null;
 
  SetupProcessorFactoryForResourceDefinition(gpfMock, mainResource.Object, mainDiscovery, dbContext);
  SetupProcessorFactoryForResourceDefinition(gpfMock, nestedResource.Object, nestedDiscovery, dbContext);
 
- var execHelper = new HookExecutorHelper(gpfMock.Object, graph, options);
- var traversalHelper = new TraversalHelper(graph, ufMock.Object);
- var hookExecutor = new ResourceHookExecutor(execHelper, traversalHelper, ufMock.Object, iqMock.Object, graph);
+ var execHelper = new HookExecutorHelper(gpfMock.Object, options);
+ var traversalHelper = new TraversalHelper(_graph, ufMock.Object);
+ var hookExecutor = new ResourceHookExecutor(execHelper, traversalHelper, ufMock.Object, iqMock.Object, inverse);
 
  return (iqMock, ufMock, hookExecutor, mainResource, nestedResource);
  }
@@ -215,17 +214,17 @@ public class HooksTestsSetup : HooksDummyData
  var secondNestedResource = CreateResourceDefinition(secondNestedDiscovery);
 
  // mocking the GenericProcessorFactory and JsonApiContext and wiring them up.
- var (graph, ufMock, iqMock, gpfMock, options) = CreateMocks();
+ var (inverse, ufMock, iqMock, gpfMock, options) = CreateMocks();
 
  var dbContext = repoDbContextOptions != null ? new AppDbContext(repoDbContextOptions) : null;
 
  SetupProcessorFactoryForResourceDefinition(gpfMock, mainResource.Object, mainDiscovery, dbContext);
  SetupProcessorFactoryForResourceDefinition(gpfMock, firstNestedResource.Object, firstNestedDiscovery, dbContext);
  SetupProcessorFactoryForResourceDefinition(gpfMock, secondNestedResource.Object, secondNestedDiscovery, dbContext);
 
- var execHelper = new HookExecutorHelper(gpfMock.Object, graph, options);
- var traversalHelper = new TraversalHelper(graph, ufMock.Object);
- var hookExecutor = new ResourceHookExecutor(execHelper, traversalHelper, ufMock.Object, iqMock.Object, graph);
+ var execHelper = new HookExecutorHelper(gpfMock.Object, options);
+ var traversalHelper = new TraversalHelper(_graph, ufMock.Object);
+ var hookExecutor = new ResourceHookExecutor(execHelper, traversalHelper, ufMock.Object, iqMock.Object, inverse);
 
  return (iqMock, hookExecutor, mainResource, firstNestedResource, secondNestedResource);
  }