refactor: move HasManyThrough GetValue and SetValue logic to HasManyThroughAttribute

Author: maurei

src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs

@@ -24,26 +24,26 @@ public class DefaultEntityRepository<TEntity, TId> : IEntityRepository<TEntity,
  private readonly ITargetedFields _targetedFields;
  private readonly DbContext _context;
  private readonly DbSet<TEntity> _dbSet;
- private readonly IResourceGraph _resourceGraph;
+ private readonly IContextEntityProvider _provider;
  private readonly IGenericProcessorFactory _genericProcessorFactory;
 
  public DefaultEntityRepository(
  ITargetedFields targetedFields,
  IDbContextResolver contextResolver,
- IResourceGraph resourceGraph,
+ IContextEntityProvider provider,
  IGenericProcessorFactory genericProcessorFactory)
- : this(targetedFields, contextResolver, resourceGraph, genericProcessorFactory, null)
+ : this(targetedFields, contextResolver, provider, genericProcessorFactory, null)
  { }
 
  public DefaultEntityRepository(
  ITargetedFields targetedFields,
  IDbContextResolver contextResolver,
- IResourceGraph resourceGraph,
+ IContextEntityProvider provider,
  IGenericProcessorFactory genericProcessorFactory,
  ILoggerFactory loggerFactory = null)
  {
  _targetedFields = targetedFields;
- _resourceGraph = resourceGraph;
+ _provider = provider;
  _genericProcessorFactory = genericProcessorFactory;
  _context = contextResolver.GetContext();
  _dbSet = _context.Set<TEntity>();
@@ -85,17 +85,15 @@ public virtual async Task<TEntity> CreateAsync(TEntity entity)
  {
  foreach (var relationshipAttr in _targetedFields.Relationships)
  {
- object trackedRelationshipValue = GetTrackedRelationshipValue(relationshipAttr, entity, out bool wasAlreadyTracked);
+ object trackedRelationshipValue = GetTrackedRelationshipValue(relationshipAttr, entity, out bool relationshipWasAlreadyTracked);
  LoadInverseRelationships(trackedRelationshipValue, relationshipAttr);
- if (wasAlreadyTracked)
+ if (relationshipWasAlreadyTracked || relationshipAttr is HasManyThroughAttribute)
  /// We only need to reassign the relationship value to the to-be-added
- /// entity when we're using a different instance (because this different one
+ /// entity when we're using a different instance of the relationship (because this different one
  /// was already tracked) than the one assigned to the to-be-created entity.
- AssignRelationshipValue(entity, trackedRelationshipValue, relationshipAttr);
- else if (relationshipAttr is HasManyThroughAttribute throughAttr)
- /// even if we don't have to reassign anything because of already tracked 
+ /// Alternatively, even if we don't have to reassign anything because of already tracked 
  /// entities, we still need to assign the "through" entities in the case of many-to-many.
- AssignHasManyThrough(entity, throughAttr, (IList)trackedRelationshipValue);
+ relationshipAttr.SetValue(entity, trackedRelationshipValue);
  }
  _dbSet.Add(entity);
  await _context.SaveChangesAsync();
@@ -139,7 +137,7 @@ private void LoadInverseRelationships(object trackedRelationshipValue, Relations
 
  private bool IsHasOneRelationship(string internalRelationshipName, Type type)
  {
- var relationshipAttr = _resourceGraph.GetContextEntity(type).Relationships.FirstOrDefault(r => r.InternalRelationshipName == internalRelationshipName);
+ var relationshipAttr = _provider.GetContextEntity(type).Relationships.FirstOrDefault(r => r.InternalRelationshipName == internalRelationshipName);
  if (relationshipAttr != null)
  {
  if (relationshipAttr is HasOneAttribute)
@@ -149,7 +147,7 @@ private bool IsHasOneRelationship(string internalRelationshipName, Type type)
  }
  // relationshipAttr is null when we don't put a [RelationshipAttribute] on the inverse navigation property.
  // In this case we use relfection to figure out what kind of relationship is pointing back.
- return !(type.GetProperty(internalRelationshipName).PropertyType.Inherits(typeof(IEnumerable)));
+ return !type.GetProperty(internalRelationshipName).PropertyType.Inherits(typeof(IEnumerable));
  }
 
  private void DetachRelationships(TEntity entity)
@@ -199,7 +197,8 @@ public virtual async Task<TEntity> UpdateAsync(TEntity updatedEntity)
  /// to the todoItems in trackedRelationshipValue
  LoadInverseRelationships(trackedRelationshipValue, relationshipAttr);
  /// assigns the updated relationship to the database entity
- AssignRelationshipValue(databaseEntity, trackedRelationshipValue, relationshipAttr);
+ //AssignRelationshipValue(databaseEntity, trackedRelationshipValue, relationshipAttr);
+ relationshipAttr.SetValue(databaseEntity, trackedRelationshipValue);
  }
 
  await _context.SaveChangesAsync();
@@ -357,30 +356,13 @@ protected void LoadCurrentRelationships(TEntity oldEntity, RelationshipAttribute
  if (relationshipAttribute is HasManyThroughAttribute throughAttribute)
  {
  _context.Entry(oldEntity).Collection(throughAttribute.InternalThroughName).Load();
-
  }
  else if (relationshipAttribute is HasManyAttribute hasManyAttribute)
  {
  _context.Entry(oldEntity).Collection(hasManyAttribute.InternalRelationshipName).Load();
  }
  }
 
- /// <summary>
- /// Assigns the <paramref name="relationshipValue"/> to <paramref name="targetEntity"/>
- /// </summary>
- private void AssignRelationshipValue(TEntity targetEntity, object relationshipValue, RelationshipAttribute relationshipAttribute)
- {
- if (relationshipAttribute is HasManyThroughAttribute throughAttribute)
- {
- // todo: this logic should be put in the HasManyThrough attribute
- AssignHasManyThrough(targetEntity, throughAttribute, (IList)relationshipValue);
- }
- else
- {
- relationshipAttribute.SetValue(targetEntity, relationshipValue);
- }
- }
-
  /// <summary>
  /// The relationshipValue parameter contains the dependent side of the relationship (Tags).
  /// We can't directly add them to the principal entity (Article): we need to 

src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs

@@ -44,7 +44,7 @@ public static IServiceCollection AddJsonApi<TContext>(this IServiceCollection se
  /// </summary>
  /// <typeparam name="TContext"></typeparam>
  /// <param name="services"></param>
- /// <param name="configureAction"></param>
+ /// <param name="configureOptions"></param>
  /// <returns></returns>
  public static IServiceCollection AddJsonApi<TContext>(this IServiceCollection services,
  Action<JsonApiOptions> configureOptions,

src/JsonApiDotNetCore/Internal/Contracts/IResourceGraph.cs

@@ -11,38 +11,6 @@ namespace JsonApiDotNetCore.Internal.Contracts
  public interface IResourceGraph : IContextEntityProvider
  {
  RelationshipAttribute GetInverseRelationship(RelationshipAttribute relationship);
- /// <summary>
- /// Gets the value of the navigation property, defined by the relationshipName,
- /// on the provided instance.
- /// </summary>
- /// <param name="resource">The resource instance</param>
- /// <param name="propertyName">The navigation property name.</param>
- /// <example>
- /// <code>
- /// _graph.GetRelationship(todoItem, nameof(TodoItem.Owner));
- /// </code>
- /// </example>
- /// <remarks>
- /// In the case of a `HasManyThrough` relationship, it will not traverse the relationship 
- /// and will instead return the value of the shadow property (e.g. Articles.Tags).
- /// If you want to traverse the relationship, you should use <see cref="GetRelationshipValue" />.
- /// </remarks>
- object GetRelationship<TParent>(TParent resource, string propertyName);
-
- /// <summary>
- /// Gets the value of the navigation property (defined by the <see cref="RelationshipAttribute" />)
- /// on the provided instance.
- /// In the case of `HasManyThrough` relationships, it will traverse the through entity and return the 
- /// value of the relationship on the other side of a join entity (e.g. Articles.ArticleTags.Tag).
- /// </summary>
- /// <param name="resource">The resource instance</param>
- /// <param name="relationship">The attribute used to define the relationship.</param>
- /// <example>
- /// <code>
- /// _graph.GetRelationshipValue(todoItem, nameof(TodoItem.Owner));
- /// </code>
- /// </example>
- object GetRelationshipValue<TParent>(TParent resource, RelationshipAttribute relationship) where TParent : IIdentifiable;
 
  /// <summary>
  /// Get the internal navigation property name for the specified public

src/JsonApiDotNetCore/Internal/ResourceGraph.cs

@@ -52,59 +52,14 @@ internal ResourceGraph(List<ContextEntity> entities, bool usesDbContext, List<Va
  /// <inheritdoc />
  public bool UsesDbContext { get; }
 
- /// <inheritdoc />
- public object GetRelationship<TParent>(TParent entity, string relationshipName)
- {
- var parentEntityType = entity.GetType();
-
- var navigationProperty = parentEntityType
- .GetProperties()
- .SingleOrDefault(p => string.Equals(p.Name, relationshipName, StringComparison.OrdinalIgnoreCase));
-
- if (navigationProperty == null)
- throw new JsonApiException(400, $"{parentEntityType} does not contain a relationship named {relationshipName}");
-
- return navigationProperty.GetValue(entity);
- }
-
- public object GetRelationshipValue<TParent>(TParent resource, RelationshipAttribute relationship) where TParent : IIdentifiable
- {
- if (relationship is HasManyThroughAttribute hasManyThroughRelationship)
- {
- return GetHasManyThrough(resource, hasManyThroughRelationship);
- }
-
- return GetRelationship(resource, relationship.InternalRelationshipName);
- }
-
- private IEnumerable<IIdentifiable> GetHasManyThrough(IIdentifiable parent, HasManyThroughAttribute hasManyThrough)
- {
- var throughProperty = GetRelationship(parent, hasManyThrough.InternalThroughName);
- if (throughProperty is IEnumerable hasManyNavigationEntity)
- {
- // wrap "yield return" in a sub-function so we can correctly return null if the property is null.
- return GetHasManyThroughIter(hasManyThrough, hasManyNavigationEntity);
- }
- return null;
- }
-
- private IEnumerable<IIdentifiable> GetHasManyThroughIter(HasManyThroughAttribute hasManyThrough, IEnumerable hasManyNavigationEntity)
- {
- foreach (var includedEntity in hasManyNavigationEntity)
- {
- var targetValue = hasManyThrough.RightProperty.GetValue(includedEntity) as IIdentifiable;
- yield return targetValue;
- }
- }
-
  /// <inheritdoc />
  public string GetRelationshipName<TParent>(string relationshipName)
  {
  var entityType = typeof(TParent);
  return Entities
  .SingleOrDefault(e => e.EntityType == entityType)
  ?.Relationships
- .SingleOrDefault(r => r.Is(relationshipName))
+  .SingleOrDefault(r => r.Is(relationshipName))
  ?.InternalRelationshipName;
  }
 

src/JsonApiDotNetCore/Internal/TypeHelper.cs

@@ -80,7 +80,7 @@ public static T ConvertType<T>(object value)
 
  public static Type GetTypeOfList(Type type)
  {
- if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(List<>))
+ if (type != null && type.IsGenericType && type.GetGenericTypeDefinition() == typeof(List<>))
  {
  return type.GetGenericArguments()[0];
  }

src/JsonApiDotNetCore/Models/Annotation/HasManyAttribute.cs

@@ -31,18 +31,31 @@ public HasManyAttribute(string publicName = null, Link relationshipLinks = Link.
  InverseNavigation = inverseNavigationProperty;
  }
 
+ /// <summary>
+ /// Gets the value of the navigation property, defined by the relationshipName,
+ /// on the provided instance.
+ /// </summary>
+
+ public override object GetValue(object entity)
+ {
+ return entity?.GetType()?
+ .GetProperty(InternalRelationshipName)?
+ .GetValue(entity);
+ }
+
+
  /// <summary>
  /// Sets the value of the property identified by this attribute
  /// </summary>
- /// <param name="resource">The target object</param>
+ /// <param name="entity">The target object</param>
  /// <param name="newValue">The new property value</param>
- public override void SetValue(object resource, object newValue)
+ public override void SetValue(object entity, object newValue)
  {
- var propertyInfo = resource
+ var propertyInfo = entity
  .GetType()
  .GetProperty(InternalRelationshipName);
 
- propertyInfo.SetValue(resource, newValue);
+ propertyInfo.SetValue(entity, newValue);
  }
  }
 }

src/JsonApiDotNetCore/Models/Annotation/HasManyThroughAttribute.cs

@@ -1,5 +1,10 @@
 using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
 using System.Reflection;
+using JsonApiDotNetCore.Extensions;
+using JsonApiDotNetCore.Internal;
 using JsonApiDotNetCore.Models.Links;
 
 namespace JsonApiDotNetCore.Models
@@ -65,6 +70,64 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li
  InternalThroughName = internalThroughName;
  }
 
+ /// <summary>
+ /// Traverses the through the provided entity and returns the 
+ /// value of the relationship on the other side of a join entity
+ /// (e.g. Articles.ArticleTags.Tag).
+ /// </summary>
+ public override object GetValue(object entity)
+ {
+ var throughNavigationProperty = entity.GetType()
+ .GetProperties()
+ .SingleOrDefault(p => string.Equals(p.Name, InternalThroughName, StringComparison.OrdinalIgnoreCase));
+
+ var throughEntities = throughNavigationProperty.GetValue(entity);
+
+ if (throughEntities == null)
+ // return an empty list for the right-type of the property.
+ return TypeHelper.CreateListFor(DependentType);
+
+ // the right entities are included on the navigation/through entities. Extract and return them.
+ var rightEntities = new List<IIdentifiable>();
+ foreach (var rightEntity in (IList)throughEntities)
+ rightEntities.Add((IIdentifiable)RightProperty.GetValue(rightEntity));
+
+ return rightEntities.Cast(DependentType);
+ }
+
+
+ /// <summary>
+ /// Sets the value of the property identified by this attribute
+ /// </summary>
+ /// <param name="entity">The target object</param>
+ /// <param name="newValue">The new property value</param>
+ public override void SetValue(object entity, object newValue)
+ {
+
+ var propertyInfo = entity
+ .GetType()
+ .GetProperty(InternalRelationshipName);
+ propertyInfo.SetValue(entity, newValue);
+
+ if (newValue == null)
+ {
+ ThroughProperty.SetValue(entity, null);
+ }
+ else
+ {
+ var throughRelationshipCollection = (IList)Activator.CreateInstance(ThroughProperty.PropertyType);
+ ThroughProperty.SetValue(entity, throughRelationshipCollection);
+
+ foreach (IIdentifiable pointer in (IList)newValue)
+ {
+ var throughInstance = Activator.CreateInstance(ThroughType);
+ LeftProperty.SetValue(throughInstance, entity);
+ RightProperty.SetValue(throughInstance, pointer);
+ throughRelationshipCollection.Add(throughInstance);
+ }
+ }
+ }
+
  /// <summary>
  /// The name of the join property on the parent resource.
  /// </summary>

src/JsonApiDotNetCore/Models/Annotation/HasOneAttribute.cs

@@ -37,6 +37,14 @@ public HasOneAttribute(string publicName = null, Link links = Link.NotConfigured
  InverseNavigation = inverseNavigationProperty;
  }
 
+
+ public override object GetValue(object entity)
+ {
+ return entity?.GetType()?
+ .GetProperty(InternalRelationshipName)?
+ .GetValue(entity);
+ }
+
  private readonly string _explicitIdentifiablePropertyName;
 
  /// <summary>
@@ -49,23 +57,23 @@ public HasOneAttribute(string publicName = null, Link links = Link.NotConfigured
  /// <summary>
  /// Sets the value of the property identified by this attribute
  /// </summary>
- /// <param name="resource">The target object</param>
+ /// <param name="entity">The target object</param>
  /// <param name="newValue">The new property value</param>
- public override void SetValue(object resource, object newValue)
+ public override void SetValue(object entity, object newValue)
  {
  string propertyName = InternalRelationshipName;
  // if we're deleting the relationship (setting it to null),
  // we set the foreignKey to null. We could also set the actual property to null,
  // but then we would first need to load the current relationship, which requires an extra query.
  if (newValue == null) propertyName = IdentifiablePropertyName;
- var resourceType = resource.GetType();
+ var resourceType = entity.GetType();
  var propertyInfo = resourceType.GetProperty(propertyName);
  if (propertyInfo == null)
  {
  // we can't set the FK to null because there isn't any.
  propertyInfo = resourceType.GetProperty(RelationshipPath);
  }
- propertyInfo.SetValue(resource, newValue);
+ propertyInfo.SetValue(entity, newValue);
  }
 
  // HACK: this will likely require boxing