To render selectors for derived types, a selector for each non-abstract derived type must be available in QueryLayer.Selection, which wasn't the case.

bkoelman · bkoelman · commit 7bf6be825b48 · 2024-11-26T16:54:21.000+01:00
Parsing of include chains is changed to include relationships on all concrete derived types. For example, given an inclusion chain of "wheels" at `/vehicles`, the parser now returns `Car.Wheels` and `Truck.Wheels`. This used to be `Vehicle.Wheels`. There's no point in adding abstract types, because they can't be instantiated in selectors. `QueryLayerComposer` was fixed to generate selectors for the left-type of these relationships, instead of for the `QueryLayer` resource type (so `Car.Wheels` and `Truck.Wheels` instead of `Vehicle.Wheels`). `SelectClauseBuilder` was missing a cast to the derived type when descending into relationship selectors, which is fixed now. Being unable to include relationships of sibling types after a POST/PATCH resource request at a base endpoint was because a `QueryLayer` for fetch-after-post was built against the accurized resource type, and then sent to the original (non-accurized) repository. For example, `POST /vehicles?include=TruckRelationship,CarRelationship` only works if the query executes against the non-accurized table (so `Vehicle` instead of `Car`, because `Car` doesn't contain `TruckRelationship`). The fix is to discard the accurized resource type and use the original `TResource`. Other improvements: - During the process of building expression trees, consistency checks have been added to prevent downstream crashes that are difficult to diagnose. - `SelectClauseBuilder` internally passed along a `LambdaScope` that overruled the one present in context, so care had to be taken to use the right one. To eliminate this pitfall, now a new context is forked which contains the appropriate `LambdaScope`, so the overruling parameter is no longer needed. Fixes #1639, fixes #1640.
diff --git a/JsonApiDotNetCore.sln.DotSettings b/JsonApiDotNetCore.sln.DotSettings
@@ -668,6 +668,7 @@ $left$ = $right$;</s:String>
 	<s:String x:Key="/Default/PatternsAndTemplates/StructuralSearch/Pattern/=D29C3A091CD9E74BBFDF1B8974F5A977/SearchPattern/@EntryValue">if ($argument$ is null) throw new ArgumentNullException(nameof($argument$));</s:String>
 	<s:String x:Key="/Default/PatternsAndTemplates/StructuralSearch/Pattern/=D29C3A091CD9E74BBFDF1B8974F5A977/Severity/@EntryValue">WARNING</s:String>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Accurize/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=accurized/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=appsettings/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Assignee/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Contoso/@EntryIndexedValue">True</s:Boolean>
diff --git a/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs
@@ -122,7 +122,7 @@ private static ReadOnlyCollection<IncludeTreeNode> LookupRelationshipName(string
         {
             // Depending on the left side of the include chain, we may match relationships anywhere in the resource type hierarchy.
             // This is compensated for when rendering the response, which substitutes relationships on base types with the derived ones.
-            IReadOnlySet<RelationshipAttribute> relationships = parent.Relationship.RightType.GetRelationshipsInTypeOrDerived(relationshipName);
+            HashSet<RelationshipAttribute> relationships = GetRelationshipsInConcreteTypes(parent.Relationship.RightType, relationshipName);
 
             if (relationships.Count > 0)
             {
@@ -140,6 +140,32 @@ private static ReadOnlyCollection<IncludeTreeNode> LookupRelationshipName(string
         return children.AsReadOnly();
     }
 
+    private static HashSet<RelationshipAttribute> GetRelationshipsInConcreteTypes(ResourceType resourceType, string relationshipName)
+    {
+        HashSet<RelationshipAttribute> relationshipsToInclude = [];
+
+        foreach (RelationshipAttribute relationship in resourceType.GetRelationshipsInTypeOrDerived(relationshipName))
+        {
+            if (!relationship.LeftType.ClrType.IsAbstract)
+            {
+                relationshipsToInclude.Add(relationship);
+            }
+
+            IncludeRelationshipsFromConcreteDerivedTypes(relationship, relationshipsToInclude);
+        }
+
+        return relationshipsToInclude;
+    }
+
+    private static void IncludeRelationshipsFromConcreteDerivedTypes(RelationshipAttribute relationship, HashSet<RelationshipAttribute> relationshipsToInclude)
+    {
+        foreach (ResourceType derivedType in relationship.LeftType.GetAllConcreteDerivedTypes())
+        {
+            RelationshipAttribute relationshipInDerived = derivedType.GetRelationshipByPublicName(relationship.PublicName);
+            relationshipsToInclude.Add(relationshipInDerived);
+        }
+    }
+
     private static void AssertRelationshipsFound(HashSet<RelationshipAttribute> relationshipsFound, string relationshipName,
         IReadOnlyCollection<IncludeTreeNode> parents, int position)
     {
diff --git a/src/JsonApiDotNetCore/Queries/QueryLayerComposer.cs b/src/JsonApiDotNetCore/Queries/QueryLayerComposer.cs
@@ -212,7 +212,7 @@ private IImmutableSet<IncludeElementExpression> ProcessIncludeSet(IImmutableSet<
         foreach (IncludeElementExpression includeElement in includeElementsEvaluated)
         {
             parentLayer.Selection ??= new FieldSelection();
-            FieldSelectors selectors = parentLayer.Selection.GetOrCreateSelectors(parentLayer.ResourceType);
+            FieldSelectors selectors = parentLayer.Selection.GetOrCreateSelectors(includeElement.Relationship.LeftType);
 
             if (!selectors.ContainsField(includeElement.Relationship))
             {
diff --git a/src/JsonApiDotNetCore/Queries/QueryableBuilding/QueryClauseBuilderContext.cs b/src/JsonApiDotNetCore/Queries/QueryableBuilding/QueryClauseBuilderContext.cs
@@ -61,6 +61,7 @@ public QueryClauseBuilderContext(Expression source, ResourceType resourceType, T
         ArgumentGuard.NotNull(lambdaScopeFactory);
         ArgumentGuard.NotNull(lambdaScope);
         ArgumentGuard.NotNull(queryableBuilder);
+        AssertSameType(source.Type, resourceType);
 
         Source = source;
         ResourceType = resourceType;
@@ -72,6 +73,17 @@ public QueryClauseBuilderContext(Expression source, ResourceType resourceType, T
         State = state;
     }
 
+    private static void AssertSameType(Type sourceType, ResourceType resourceType)
+    {
+        Type? sourceElementType = CollectionConverter.Instance.FindCollectionElementType(sourceType);
+
+        if (sourceElementType != resourceType.ClrType)
+        {
+            throw new InvalidOperationException(
+                $"Internal error: Mismatch between expression type '{sourceElementType?.Name}' and resource type '{resourceType.ClrType.Name}'.");
+        }
+    }
+
     public QueryClauseBuilderContext WithSource(Expression source)
     {
         ArgumentGuard.NotNull(source);
diff --git a/src/JsonApiDotNetCore/Queries/QueryableBuilding/QueryableBuilder.cs b/src/JsonApiDotNetCore/Queries/QueryableBuilding/QueryableBuilder.cs
@@ -35,6 +35,7 @@ public virtual Expression ApplyQuery(QueryLayer layer, QueryableBuilderContext c
     {
         ArgumentGuard.NotNull(layer);
         ArgumentGuard.NotNull(context);
+        AssertSameType(layer.ResourceType, context.ElementType);
 
         Expression expression = context.Source;
 
@@ -66,6 +67,15 @@ public virtual Expression ApplyQuery(QueryLayer layer, QueryableBuilderContext c
         return expression;
     }
 
+    private static void AssertSameType(ResourceType resourceType, Type elementType)
+    {
+        if (elementType != resourceType.ClrType)
+        {
+            throw new InvalidOperationException(
+                $"Internal error: Mismatch between query layer type '{resourceType.ClrType.Name}' and query element type '{elementType.Name}'.");
+        }
+    }
+
     protected virtual Expression ApplyInclude(Expression source, IncludeExpression include, ResourceType resourceType, QueryableBuilderContext context)
     {
         ArgumentGuard.NotNull(source);
diff --git a/src/JsonApiDotNetCore/Queries/QueryableBuilding/SelectClauseBuilder.cs b/src/JsonApiDotNetCore/Queries/QueryableBuilding/SelectClauseBuilder.cs
@@ -29,36 +29,47 @@ public virtual Expression ApplySelect(FieldSelection selection, QueryClauseBuild
     {
         ArgumentGuard.NotNull(selection);
 
-        Expression bodyInitializer = CreateLambdaBodyInitializer(selection, context.ResourceType, context.LambdaScope, false, context);
+        Expression bodyInitializer = CreateLambdaBodyInitializer(selection, context.ResourceType, false, context);
 
         LambdaExpression lambda = Expression.Lambda(bodyInitializer, context.LambdaScope.Parameter);
 
         return SelectExtensionMethodCall(context.ExtensionType, context.Source, context.LambdaScope.Parameter.Type, lambda);
     }
 
-    private Expression CreateLambdaBodyInitializer(FieldSelection selection, ResourceType resourceType, LambdaScope lambdaScope,
-        bool lambdaAccessorRequiresTestForNull, QueryClauseBuilderContext context)
+    private Expression CreateLambdaBodyInitializer(FieldSelection selection, ResourceType resourceType, bool lambdaAccessorRequiresTestForNull,
+        QueryClauseBuilderContext context)
     {
+        AssertSameType(context.LambdaScope.Accessor.Type, resourceType);
+
         IReadOnlyEntityType entityType = context.EntityModel.FindEntityType(resourceType.ClrType)!;
         IReadOnlyEntityType[] concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToArray();
 
         Expression bodyInitializer = concreteEntityTypes.Length > 1
-            ? CreateLambdaBodyInitializerForTypeHierarchy(selection, resourceType, concreteEntityTypes, lambdaScope, context)
-            : CreateLambdaBodyInitializerForSingleType(selection, resourceType, lambdaScope, context);
+            ? CreateLambdaBodyInitializerForTypeHierarchy(selection, resourceType, concreteEntityTypes, context)
+            : CreateLambdaBodyInitializerForSingleType(selection, resourceType, context);
 
         if (!lambdaAccessorRequiresTestForNull)
         {
             return bodyInitializer;
         }
 
-        return TestForNull(lambdaScope.Accessor, bodyInitializer);
+        return TestForNull(context.LambdaScope.Accessor, bodyInitializer);
+    }
+
+    private static void AssertSameType(Type lambdaAccessorType, ResourceType resourceType)
+    {
+        if (lambdaAccessorType != resourceType.ClrType)
+        {
+            throw new InvalidOperationException(
+                $"Internal error: Mismatch between lambda accessor type '{lambdaAccessorType.Name}' and resource type '{resourceType.ClrType.Name}'.");
+        }
     }
 
     private Expression CreateLambdaBodyInitializerForTypeHierarchy(FieldSelection selection, ResourceType baseResourceType,
-        IEnumerable<IReadOnlyEntityType> concreteEntityTypes, LambdaScope lambdaScope, QueryClauseBuilderContext context)
+        IEnumerable<IReadOnlyEntityType> concreteEntityTypes, QueryClauseBuilderContext context)
     {
         IReadOnlySet<ResourceType> resourceTypes = selection.GetResourceTypes();
-        Expression rootCondition = lambdaScope.Accessor;
+        Expression rootCondition = context.LambdaScope.Accessor;
 
         foreach (IReadOnlyEntityType entityType in concreteEntityTypes)
         {
@@ -73,14 +84,14 @@ private Expression CreateLambdaBodyInitializerForTypeHierarchy(FieldSelection se
                     Dictionary<PropertyInfo, PropertySelector>.ValueCollection propertySelectors =
                         ToPropertySelectors(fieldSelectors, resourceType, entityType.ClrType, context.EntityModel);
 
-                    MemberBinding[] propertyAssignments = propertySelectors.Select(selector => CreatePropertyAssignment(selector, lambdaScope, context))
+                    MemberBinding[] propertyAssignments = propertySelectors.Select(selector => CreatePropertyAssignment(selector, context))
                         .Cast<MemberBinding>().ToArray();
 
                     NewExpression createInstance = _resourceFactory.CreateNewExpression(entityType.ClrType);
                     MemberInitExpression memberInit = Expression.MemberInit(createInstance, propertyAssignments);
                     UnaryExpression castToBaseType = Expression.Convert(memberInit, baseResourceType.ClrType);
 
-                    BinaryExpression typeCheck = CreateRuntimeTypeCheck(lambdaScope, entityType.ClrType);
+                    BinaryExpression typeCheck = CreateRuntimeTypeCheck(context.LambdaScope, entityType.ClrType);
                     rootCondition = Expression.Condition(typeCheck, castToBaseType, rootCondition);
                 }
             }
@@ -100,18 +111,16 @@ private static BinaryExpression CreateRuntimeTypeCheck(LambdaScope lambdaScope,
         return Expression.MakeBinary(ExpressionType.Equal, getTypeCall, concreteTypeConstant, false, TypeOpEqualityMethod);
     }
 
-    private MemberInitExpression CreateLambdaBodyInitializerForSingleType(FieldSelection selection, ResourceType resourceType, LambdaScope lambdaScope,
+    private MemberInitExpression CreateLambdaBodyInitializerForSingleType(FieldSelection selection, ResourceType resourceType,
         QueryClauseBuilderContext context)
     {
         FieldSelectors fieldSelectors = selection.GetOrCreateSelectors(resourceType);
 
         Dictionary<PropertyInfo, PropertySelector>.ValueCollection propertySelectors =
-            ToPropertySelectors(fieldSelectors, resourceType, lambdaScope.Accessor.Type, context.EntityModel);
-
-        MemberBinding[] propertyAssignments = propertySelectors.Select(selector => CreatePropertyAssignment(selector, lambdaScope, context))
-            .Cast<MemberBinding>().ToArray();
+            ToPropertySelectors(fieldSelectors, resourceType, context.LambdaScope.Accessor.Type, context.EntityModel);
 
-        NewExpression createInstance = _resourceFactory.CreateNewExpression(lambdaScope.Accessor.Type);
+        MemberBinding[] propertyAssignments = propertySelectors.Select(selector => CreatePropertyAssignment(selector, context)).Cast<MemberBinding>().ToArray();
+        NewExpression createInstance = _resourceFactory.CreateNewExpression(context.LambdaScope.Accessor.Type);
         return Expression.MemberInit(createInstance, propertyAssignments);
     }
 
@@ -182,50 +191,56 @@ private static void IncludeEagerLoads(ResourceType resourceType, Dictionary<Prop
         }
     }
 
-    private MemberAssignment CreatePropertyAssignment(PropertySelector propertySelector, LambdaScope lambdaScope, QueryClauseBuilderContext context)
+    private MemberAssignment CreatePropertyAssignment(PropertySelector propertySelector, QueryClauseBuilderContext context)
     {
-        bool requiresUpCast = lambdaScope.Accessor.Type != propertySelector.Property.DeclaringType &&
-            lambdaScope.Accessor.Type.IsAssignableFrom(propertySelector.Property.DeclaringType);
+        bool requiresUpCast = context.LambdaScope.Accessor.Type != propertySelector.Property.DeclaringType &&
+            context.LambdaScope.Accessor.Type.IsAssignableFrom(propertySelector.Property.DeclaringType);
+
+        UnaryExpression? derivedAccessor = requiresUpCast ? Expression.Convert(context.LambdaScope.Accessor, propertySelector.Property.DeclaringType!) : null;
 
-        MemberExpression propertyAccess = requiresUpCast
-            ? Expression.MakeMemberAccess(Expression.Convert(lambdaScope.Accessor, propertySelector.Property.DeclaringType!), propertySelector.Property)
-            : Expression.Property(lambdaScope.Accessor, propertySelector.Property);
+        MemberExpression propertyAccess = derivedAccessor != null
+            ? Expression.MakeMemberAccess(derivedAccessor, propertySelector.Property)
+            : Expression.Property(context.LambdaScope.Accessor, propertySelector.Property);
 
         Expression assignmentRightHandSide = propertyAccess;
 
         if (propertySelector.NextLayer != null)
         {
-            assignmentRightHandSide =
-                CreateAssignmentRightHandSideForLayer(propertySelector.NextLayer, lambdaScope, propertyAccess, propertySelector.Property, context);
+            QueryClauseBuilderContext rightHandSideContext =
+                derivedAccessor != null ? context.WithLambdaScope(context.LambdaScope.WithAccessor(derivedAccessor)) : context;
+
+            assignmentRightHandSide = CreateAssignmentRightHandSideForLayer(propertySelector.NextLayer, propertyAccess,
+                propertySelector.Property, rightHandSideContext);
         }
 
         return Expression.Bind(propertySelector.Property, assignmentRightHandSide);
     }
 
-    private Expression CreateAssignmentRightHandSideForLayer(QueryLayer layer, LambdaScope outerLambdaScope, MemberExpression propertyAccess,
-        PropertyInfo selectorPropertyInfo, QueryClauseBuilderContext context)
+    private Expression CreateAssignmentRightHandSideForLayer(QueryLayer layer, MemberExpression propertyAccess, PropertyInfo selectorPropertyInfo,
+        QueryClauseBuilderContext context)
     {
         Type? collectionElementType = CollectionConverter.Instance.FindCollectionElementType(selectorPropertyInfo.PropertyType);
         Type bodyElementType = collectionElementType ?? selectorPropertyInfo.PropertyType;
 
         if (collectionElementType != null)
         {
-            return CreateCollectionInitializer(outerLambdaScope, selectorPropertyInfo, bodyElementType, layer, context);
+            return CreateCollectionInitializer(selectorPropertyInfo, bodyElementType, layer, context);
         }
 
         if (layer.Selection == null || layer.Selection.IsEmpty)
         {
             return propertyAccess;
         }
 
-        using LambdaScope scope = context.LambdaScopeFactory.CreateScope(bodyElementType, propertyAccess);
-        return CreateLambdaBodyInitializer(layer.Selection, layer.ResourceType, scope, true, context);
+        using LambdaScope initializerScope = context.LambdaScopeFactory.CreateScope(bodyElementType, propertyAccess);
+        QueryClauseBuilderContext initializerContext = context.WithLambdaScope(initializerScope);
+        return CreateLambdaBodyInitializer(layer.Selection, layer.ResourceType, true, initializerContext);
     }
 
-    private static MethodCallExpression CreateCollectionInitializer(LambdaScope lambdaScope, PropertyInfo collectionProperty, Type elementType,
-        QueryLayer layer, QueryClauseBuilderContext context)
+    private static MethodCallExpression CreateCollectionInitializer(PropertyInfo collectionProperty, Type elementType, QueryLayer layer,
+        QueryClauseBuilderContext context)
     {
-        MemberExpression propertyExpression = Expression.Property(lambdaScope.Accessor, collectionProperty);
+        MemberExpression propertyExpression = Expression.Property(context.LambdaScope.Accessor, collectionProperty);
 
         var nestedContext = new QueryableBuilderContext(propertyExpression, elementType, typeof(Enumerable), context.EntityModel, context.LambdaScopeFactory,
             context.State);
diff --git a/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs b/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs
@@ -11,6 +11,11 @@ namespace JsonApiDotNetCore.Repositories;
 /// </summary>
 public interface IResourceRepositoryAccessor
 {
+    /// <summary>
+    /// Uses the <see cref="IResourceGraph" /> to lookup the corresponding <see cref="ResourceType" /> for the specified CLR type.
+    /// </summary>
+    ResourceType LookupResourceType(Type resourceClrType);
+
     /// <summary>
     /// Invokes <see cref="IResourceReadRepository{TResource,TId}.GetAsync" /> for the specified resource type.
     /// </summary>
diff --git a/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs b/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs
@@ -29,6 +29,14 @@ public ResourceRepositoryAccessor(IServiceProvider serviceProvider, IResourceGra
         _request = request;
     }
 
+    /// <inheritdoc />
+    public ResourceType LookupResourceType(Type resourceClrType)
+    {
+        ArgumentGuard.NotNull(resourceClrType);
+
+        return _resourceGraph.GetResourceType(resourceClrType);
+    }
+
     /// <inheritdoc />
     public async Task<IReadOnlyCollection<TResource>> GetAsync<TResource>(QueryLayer queryLayer, CancellationToken cancellationToken)
         where TResource : class, IIdentifiable
diff --git a/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs b/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs
diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceReadTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceReadTests.cs
diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceWriteTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceWriteTests.cs
diff --git a/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/IncludeParseTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/IncludeParseTests.cs

Original file line number	Diff line number	Diff line change
`@@ -212,7 +212,7 @@ private IImmutableSet<IncludeElementExpression> ProcessIncludeSet(IImmutableSet<`
`212`	`212`	`foreach (IncludeElementExpression includeElement in includeElementsEvaluated)`
`213`	`213`	`{`
`214`	`214`	`parentLayer.Selection ??= new FieldSelection();`
`215`		`- FieldSelectors selectors = parentLayer.Selection.GetOrCreateSelectors(parentLayer.ResourceType);`
	`215`	`+ FieldSelectors selectors = parentLayer.Selection.GetOrCreateSelectors(includeElement.Relationship.LeftType);`
`216`	`216`
`217`	`217`	`if (!selectors.ContainsField(includeElement.Relationship))`
`218`	`218`	`{`