Allow derived types in the scope of sort/filter/page query string parameters

Author: bkoelman

src/JsonApiDotNetCore/Queries/Expressions/PaginationElementQueryStringValueExpression.cs

@@ -10,12 +10,12 @@ namespace JsonApiDotNetCore.Queries.Expressions;
 /// .
 /// </summary>
 [PublicAPI]
-public class PaginationElementQueryStringValueExpression(ResourceFieldChainExpression? scope, int value, int position) : QueryExpression
+public class PaginationElementQueryStringValueExpression(IncludeExpression? scope, int value, int position) : QueryExpression
 {
     /// <summary>
-    /// The relationship this pagination applies to. Chain format: zero or more relationships, followed by a to-many relationship.
+    /// The relationship this pagination applies to. Format: zero or more relationships, followed by a to-many relationship.
     /// </summary>
-    public ResourceFieldChainExpression? Scope { get; } = scope;
+    public IncludeExpression? Scope { get; } = scope;
 
     /// <summary>
     /// The numeric pagination value.

src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs

@@ -205,7 +205,7 @@ public override QueryExpression VisitSparseFieldSet(SparseFieldSetExpression exp
     public override QueryExpression? VisitQueryStringParameterScope(QueryStringParameterScopeExpression expression, TArgument argument)
     {
         var newParameterName = Visit(expression.ParameterName, argument) as LiteralConstantExpression;
-        ResourceFieldChainExpression? newScope = expression.Scope != null ? Visit(expression.Scope, argument) as ResourceFieldChainExpression : null;
+        IncludeExpression? newScope = expression.Scope != null ? Visit(expression.Scope, argument) as IncludeExpression : null;
 
         if (newParameterName != null)
         {
@@ -226,7 +226,7 @@ public override QueryExpression VisitPaginationQueryStringValue(PaginationQueryS
 
     public override QueryExpression VisitPaginationElementQueryStringValue(PaginationElementQueryStringValueExpression expression, TArgument argument)
     {
-        ResourceFieldChainExpression? newScope = expression.Scope != null ? Visit(expression.Scope, argument) as ResourceFieldChainExpression : null;
+        IncludeExpression? newScope = expression.Scope != null ? Visit(expression.Scope, argument) as IncludeExpression : null;
 
         var newExpression = new PaginationElementQueryStringValueExpression(newScope, expression.Value, expression.Position);
         return newExpression.Equals(expression) ? expression : newExpression;

src/JsonApiDotNetCore/Queries/Expressions/QueryStringParameterScopeExpression.cs

@@ -25,9 +25,9 @@ public class QueryStringParameterScopeExpression : QueryExpression
     /// The scope this parameter value applies to, or <c>null</c> for the URL endpoint scope. Chain format for the filter/sort parameters: an optional list
     /// of relationships, followed by a to-many relationship.
     /// </summary>
-    public ResourceFieldChainExpression? Scope { get; }
+    public IncludeExpression? Scope { get; }
 
-    public QueryStringParameterScopeExpression(LiteralConstantExpression parameterName, ResourceFieldChainExpression? scope)
+    public QueryStringParameterScopeExpression(LiteralConstantExpression parameterName, IncludeExpression? scope)
     {
         ArgumentNullException.ThrowIfNull(parameterName);
 

src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs

@@ -35,26 +35,7 @@ public IncludeExpression Parse(string source, ResourceType resourceType)
 
     protected virtual IncludeExpression ParseInclude(ResourceType resourceType)
     {
-        ArgumentNullException.ThrowIfNull(resourceType);
-
-        var treeRoot = IncludeTreeNode.CreateRoot(resourceType);
-        bool isAtStart = true;
-
-        while (TokenStack.Count > 0)
-        {
-            if (!isAtStart)
-            {
-                EatSingleCharacterToken(TokenKind.Comma);
-            }
-            else
-            {
-                isAtStart = false;
-            }
-
-            ParseRelationshipChain(treeRoot);
-        }
-
-        return treeRoot.ToExpression();
+        return ParseCommaSeparatedSequenceOfRelationshipChains(resourceType);
     }
 
     private void ValidateMaximumIncludeDepth(IncludeExpression include, int position)

src/JsonApiDotNetCore/Queries/Parsing/PaginationParser.cs

@@ -2,7 +2,6 @@
 using JetBrains.Annotations;
 using JsonApiDotNetCore.Configuration;
 using JsonApiDotNetCore.Queries.Expressions;
-using JsonApiDotNetCore.QueryStrings.FieldChains;
 
 namespace JsonApiDotNetCore.Queries.Parsing;
 
@@ -57,8 +56,7 @@ protected virtual PaginationElementQueryStringValueExpression ParsePaginationEle
             return new PaginationElementQueryStringValueExpression(null, number.Value, position);
         }
 
-        ResourceFieldChainExpression scope = ParseFieldChain(BuiltInPatterns.RelationshipChainEndingInToMany, FieldChainPatternMatchOptions.None, resourceType,
-            "Number or relationship name expected.");
+        IncludeExpression scope = ParseRelationshipChainEndingInToMany(resourceType, "Number or relationship name expected.");
 
         EatSingleCharacterToken(TokenKind.Colon);
 

src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs

@@ -193,7 +193,47 @@ protected void AssertTokenStackIsEmpty()
         }
     }
 
-    private protected void ParseRelationshipChain(IncludeTreeNode treeRoot)
+    /// <summary>
+    /// Parses a comma-separated sequence of relationship chains, taking relationships on derived types into account.
+    /// </summary>
+    protected IncludeExpression ParseCommaSeparatedSequenceOfRelationshipChains(ResourceType resourceType)
+    {
+        ArgumentNullException.ThrowIfNull(resourceType);
+
+        var treeRoot = IncludeTreeNode.CreateRoot(resourceType);
+        bool isAtStart = true;
+
+        while (TokenStack.Count > 0)
+        {
+            if (!isAtStart)
+            {
+                EatSingleCharacterToken(TokenKind.Comma);
+            }
+            else
+            {
+                isAtStart = false;
+            }
+
+            ParseRelationshipChain(treeRoot, false, null);
+        }
+
+        return treeRoot.ToExpression();
+    }
+
+    /// <summary>
+    /// Parses a relationship chain that ends in a to-many relationship, taking relationships on derived types into account.
+    /// </summary>
+    protected IncludeExpression ParseRelationshipChainEndingInToMany(ResourceType resourceType, string? alternativeErrorMessage)
+    {
+        ArgumentNullException.ThrowIfNull(resourceType);
+
+        var treeRoot = IncludeTreeNode.CreateRoot(resourceType);
+        ParseRelationshipChain(treeRoot, true, alternativeErrorMessage);
+
+        return treeRoot.ToExpression();
+    }
+
+    private void ParseRelationshipChain(IncludeTreeNode treeRoot, bool requireChainEndsInToMany, string? alternativeErrorMessage)
     {
         // A relationship name usually matches a single relationship, even when overridden in derived types.
         // But in the following case, two relationships are matched on GET /shoppingBaskets?include=items:
@@ -221,29 +261,35 @@ private protected void ParseRelationshipChain(IncludeTreeNode treeRoot)
         // that there's currently no way to include Products without Articles. We could add such optional upcast syntax
         // in the future, if desired.
 
-        ReadOnlyCollection<IncludeTreeNode> children = ParseRelationshipName([treeRoot]);
+        ReadOnlyCollection<IncludeTreeNode> children = ParseRelationshipName([treeRoot], requireChainEndsInToMany, alternativeErrorMessage);
 
         while (TokenStack.TryPeek(out Token? nextToken) && nextToken.Kind == TokenKind.Period)
         {
             EatSingleCharacterToken(TokenKind.Period);
 
-            children = ParseRelationshipName(children);
+            children = ParseRelationshipName(children, requireChainEndsInToMany, null);
         }
     }
 
-    private ReadOnlyCollection<IncludeTreeNode> ParseRelationshipName(IReadOnlyCollection<IncludeTreeNode> parents)
+    private ReadOnlyCollection<IncludeTreeNode> ParseRelationshipName(IReadOnlyCollection<IncludeTreeNode> parents, bool requireChainEndsInToMany,
+        string? alternativeErrorMessage)
     {
         int position = GetNextTokenPositionOrEnd();
 
         if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.Text)
         {
-            return LookupRelationshipName(token.Value!, parents, position);
+            bool isAtEndOfChain = !TokenStack.TryPeek(out Token? nextToken) || nextToken.Kind != TokenKind.Period;
+            bool requireToMany = requireChainEndsInToMany && isAtEndOfChain;
+
+            return LookupRelationshipName(token.Value!, parents, requireToMany, position);
         }
 
-        throw new QueryParseException("Relationship name expected.", position);
+        string message = alternativeErrorMessage ?? (requireChainEndsInToMany ? "To-many relationship name expected." : "Relationship name expected.");
+        throw new QueryParseException(message, position);
     }
 
-    private ReadOnlyCollection<IncludeTreeNode> LookupRelationshipName(string relationshipName, IReadOnlyCollection<IncludeTreeNode> parents, int position)
+    private ReadOnlyCollection<IncludeTreeNode> LookupRelationshipName(string relationshipName, IReadOnlyCollection<IncludeTreeNode> parents,
+        bool requireToMany, int position)
     {
         List<IncludeTreeNode> children = [];
         HashSet<RelationshipAttribute> relationshipsFound = [];
@@ -252,51 +298,58 @@ private ReadOnlyCollection<IncludeTreeNode> LookupRelationshipName(string relati
         {
             // 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.
-            HashSet<RelationshipAttribute> relationships = GetRelationshipsInConcreteTypes(parent.Relationship.RightType, relationshipName);
+            HashSet<RelationshipAttribute> relationships = GetRelationshipsInConcreteTypes(parent.Relationship.RightType, relationshipName, requireToMany);
 
             if (relationships.Count > 0)
             {
                 relationshipsFound.UnionWith(relationships);
 
                 RelationshipAttribute[] relationshipsToInclude = relationships.Where(relationship => !relationship.IsIncludeBlocked()).ToArray();
-                IReadOnlyCollection<IncludeTreeNode> affectedChildren = parent.EnsureChildren(relationshipsToInclude);
+                ReadOnlyCollection<IncludeTreeNode> affectedChildren = parent.EnsureChildren(relationshipsToInclude);
                 children.AddRange(affectedChildren);
             }
         }
 
-        AssertRelationshipsFound(relationshipsFound, relationshipName, parents, position);
+        AssertRelationshipsFound(relationshipsFound, relationshipName, requireToMany, parents, position);
         AssertAtLeastOneCanBeIncluded(relationshipsFound, relationshipName, position);
 
         return children.AsReadOnly();
     }
 
-    private static HashSet<RelationshipAttribute> GetRelationshipsInConcreteTypes(ResourceType resourceType, string relationshipName)
+    private static HashSet<RelationshipAttribute> GetRelationshipsInConcreteTypes(ResourceType resourceType, string relationshipName, bool requireToMany)
     {
         HashSet<RelationshipAttribute> relationshipsToInclude = [];
 
         foreach (RelationshipAttribute relationship in resourceType.GetRelationshipsInTypeOrDerived(relationshipName))
         {
-            if (!relationship.LeftType.ClrType.IsAbstract)
+            if (!requireToMany || relationship is HasManyAttribute)
             {
-                relationshipsToInclude.Add(relationship);
+                if (!relationship.LeftType.ClrType.IsAbstract)
+                {
+                    relationshipsToInclude.Add(relationship);
+                }
             }
 
-            IncludeRelationshipsFromConcreteDerivedTypes(relationship, relationshipsToInclude);
+            IncludeRelationshipsFromConcreteDerivedTypes(relationship, requireToMany, relationshipsToInclude);
         }
 
         return relationshipsToInclude;
     }
 
-    private static void IncludeRelationshipsFromConcreteDerivedTypes(RelationshipAttribute relationship, HashSet<RelationshipAttribute> relationshipsToInclude)
+    private static void IncludeRelationshipsFromConcreteDerivedTypes(RelationshipAttribute relationship, bool requireToMany,
+        HashSet<RelationshipAttribute> relationshipsToInclude)
     {
         foreach (ResourceType derivedType in relationship.LeftType.GetAllConcreteDerivedTypes())
         {
-            RelationshipAttribute relationshipInDerived = derivedType.GetRelationshipByPublicName(relationship.PublicName);
-            relationshipsToInclude.Add(relationshipInDerived);
+            if (!requireToMany || relationship is HasManyAttribute)
+            {
+                RelationshipAttribute relationshipInDerived = derivedType.GetRelationshipByPublicName(relationship.PublicName);
+                relationshipsToInclude.Add(relationshipInDerived);
+            }
         }
     }
 
-    private static void AssertRelationshipsFound(HashSet<RelationshipAttribute> relationshipsFound, string relationshipName,
+    private static void AssertRelationshipsFound(HashSet<RelationshipAttribute> relationshipsFound, string relationshipName, bool requireToMany,
         IReadOnlyCollection<IncludeTreeNode> parents, int position)
     {
         if (relationshipsFound.Count > 0)
@@ -308,13 +361,13 @@ private static void AssertRelationshipsFound(HashSet<RelationshipAttribute> rela
 
         bool hasDerivedTypes = parents.Any(parent => parent.Relationship.RightType.DirectlyDerivedTypes.Count > 0);
 
-        string message = GetErrorMessageForNoneFound(relationshipName, parentResourceTypes, hasDerivedTypes);
+        string message = GetErrorMessageForNoneFound(relationshipName, requireToMany, parentResourceTypes, hasDerivedTypes);
         throw new QueryParseException(message, position);
     }
 
-    private static string GetErrorMessageForNoneFound(string relationshipName, ResourceType[] parentResourceTypes, bool hasDerivedTypes)
+    private static string GetErrorMessageForNoneFound(string relationshipName, bool requireToMany, ResourceType[] parentResourceTypes, bool hasDerivedTypes)
     {
-        var builder = new StringBuilder($"Relationship '{relationshipName}'");
+        var builder = new StringBuilder($"{(requireToMany ? "To-many relationship" : "Relationship")} '{relationshipName}'");
 
         if (parentResourceTypes.Length == 1)
         {
@@ -345,7 +398,7 @@ private void AssertAtLeastOneCanBeIncluded(HashSet<RelationshipAttribute> relati
         }
     }
 
-    internal sealed class IncludeTreeNode
+    private sealed class IncludeTreeNode
     {
         private readonly Dictionary<RelationshipAttribute, IncludeTreeNode> _children = [];
 
@@ -362,7 +415,7 @@ public static IncludeTreeNode CreateRoot(ResourceType resourceType)
             return new IncludeTreeNode(relationship);
         }
 
-        public IReadOnlyCollection<IncludeTreeNode> EnsureChildren(RelationshipAttribute[] relationships)
+        public ReadOnlyCollection<IncludeTreeNode> EnsureChildren(RelationshipAttribute[] relationships)
         {
             foreach (RelationshipAttribute relationship in relationships)
             {

src/JsonApiDotNetCore/Queries/Parsing/QueryStringParameterScopeParser.cs

@@ -1,7 +1,6 @@
 using JetBrains.Annotations;
 using JsonApiDotNetCore.Configuration;
 using JsonApiDotNetCore.Queries.Expressions;
-using JsonApiDotNetCore.QueryStrings.FieldChains;
 
 namespace JsonApiDotNetCore.Queries.Parsing;
 
@@ -36,13 +35,13 @@ protected virtual QueryStringParameterScopeExpression ParseQueryStringParameterS
 
         var name = new LiteralConstantExpression(token.Value!);
 
-        ResourceFieldChainExpression? scope = null;
+        IncludeExpression? scope = null;
 
         if (TokenStack.TryPeek(out Token? nextToken) && nextToken.Kind == TokenKind.OpenBracket)
         {
             TokenStack.Pop();
 
-            scope = ParseFieldChain(BuiltInPatterns.RelationshipChainEndingInToMany, FieldChainPatternMatchOptions.None, resourceType, null);
+            scope = ParseRelationshipChainEndingInToMany(resourceType, null);
 
             EatSingleCharacterToken(TokenKind.CloseBracket);
         }

src/JsonApiDotNetCore/QueryStrings/FieldChains/BuiltInPatterns.cs

@@ -12,5 +12,7 @@ public static class BuiltInPatterns
     public static FieldChainPattern ToOneChainEndingInAttribute { get; } = FieldChainPattern.Parse("O*A");
     public static FieldChainPattern ToOneChainEndingInAttributeOrToOne { get; } = FieldChainPattern.Parse("O*[OA]");
     public static FieldChainPattern ToOneChainEndingInToMany { get; } = FieldChainPattern.Parse("O*M");
+
+    [Obsolete("""This is no longer used and will be removed in a future version. Instead, use: FieldChainPattern.Parse("R*M")""")]
     public static FieldChainPattern RelationshipChainEndingInToMany { get; } = FieldChainPattern.Parse("R*M");
 }

src/JsonApiDotNetCore/QueryStrings/FilterQueryStringParameterReader.cs

@@ -96,11 +96,16 @@ private void ReadSingleValue(string parameterName, string parameterValue)
                 (name, value) = LegacyConverter.Convert(name, value);
             }
 
-            ResourceFieldChainExpression? scope = GetScope(name);
+            IncludeExpression? scopeInclude = GetScope(name);
             parameterNameIsValid = true;
 
-            FilterExpression filter = GetFilter(value, scope);
-            StoreFilterInScope(filter, scope);
+            foreach (ResourceFieldChainExpression? scopeChain in scopeInclude == null
+                ? FieldChainInGlobalScope
+                : IncludeChainConverter.Instance.GetRelationshipChains(scopeInclude))
+            {
+                FilterExpression filter = GetFilter(value, scopeChain);
+                StoreFilterInScope(filter, scopeChain);
+            }
         }
         catch (QueryParseException exception)
         {
@@ -112,7 +117,7 @@ private void ReadSingleValue(string parameterName, string parameterValue)
         }
     }
 
-    private ResourceFieldChainExpression? GetScope(string parameterName)
+    private IncludeExpression? GetScope(string parameterName)
     {
         QueryStringParameterScopeExpression parameterScope = _scopeParser.Parse(parameterName, RequestResourceType);