From 918327811bf2e8dd6d6b9b7f34e65229452542c9 Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Thu, 7 Aug 2025 01:19:29 +0200 Subject: [PATCH 1/4] Simplify IQueryStringParameterScopeParser --- .../Queries/Expressions/IncludeChainConverter.cs | 6 ++++++ .../Queries/Expressions/IncludeExpression.cs | 4 +--- .../Parsing/IQueryStringParameterScopeParser.cs | 13 +++---------- .../Parsing/QueryStringParameterScopeParser.cs | 11 ++++------- .../QueryableBuilding/IncludeClauseBuilder.cs | 4 +--- .../FilterQueryStringParameterReader.cs | 4 +--- .../QueryStrings/SortQueryStringParameterReader.cs | 4 +--- .../Queries/QueryExpressionRewriterTests.cs | 4 +--- 8 files changed, 18 insertions(+), 32 deletions(-) diff --git a/src/JsonApiDotNetCore/Queries/Expressions/IncludeChainConverter.cs b/src/JsonApiDotNetCore/Queries/Expressions/IncludeChainConverter.cs index c729c692b9..615d0608cd 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/IncludeChainConverter.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/IncludeChainConverter.cs @@ -8,6 +8,12 @@ namespace JsonApiDotNetCore.Queries.Expressions; /// internal sealed class IncludeChainConverter { + public static IncludeChainConverter Instance { get; } = new(); + + private IncludeChainConverter() + { + } + /// /// Converts a tree of inclusions into a set of relationship chains. /// diff --git a/src/JsonApiDotNetCore/Queries/Expressions/IncludeExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/IncludeExpression.cs index b1d5bd00fb..50b152caf7 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/IncludeExpression.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/IncludeExpression.cs @@ -13,8 +13,6 @@ namespace JsonApiDotNetCore.Queries.Expressions; [PublicAPI] public class IncludeExpression : QueryExpression { - private static readonly IncludeChainConverter IncludeChainConverter = new(); - public static readonly IncludeExpression Empty = new(); /// @@ -51,7 +49,7 @@ public override string ToFullString() private string InnerToString(bool toFullString) { - IReadOnlyCollection chains = IncludeChainConverter.GetRelationshipChains(this); + IReadOnlyCollection chains = IncludeChainConverter.Instance.GetRelationshipChains(this); return string.Join(",", chains.Select(field => toFullString ? field.ToFullString() : field.ToString()).Distinct().OrderBy(name => name)); } diff --git a/src/JsonApiDotNetCore/Queries/Parsing/IQueryStringParameterScopeParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/IQueryStringParameterScopeParser.cs index 22cd2b1426..99637fcfbd 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/IQueryStringParameterScopeParser.cs +++ b/src/JsonApiDotNetCore/Queries/Parsing/IQueryStringParameterScopeParser.cs @@ -1,12 +1,11 @@ using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Queries.Expressions; -using JsonApiDotNetCore.QueryStrings.FieldChains; namespace JsonApiDotNetCore.Queries.Parsing; /// -/// Parses the JSON:API 'sort' and 'filter' query string parameter names, which contain a resource field chain that indicates the scope its query string -/// parameter value applies to. +/// Parser for the JSON:API 'sort' and 'filter' query string parameter names, which indicate the scope their query string parameter value applies to. The +/// value consists of an optional relationship chain ending in a to-many relationship, surrounded by brackets. /// public interface IQueryStringParameterScopeParser { @@ -20,11 +19,5 @@ public interface IQueryStringParameterScopeParser /// /// The resource type used to lookup JSON:API fields that are referenced in . /// - /// - /// The pattern that the field chain in must match. - /// - /// - /// The match options for . - /// - QueryStringParameterScopeExpression Parse(string source, ResourceType resourceType, FieldChainPattern pattern, FieldChainPatternMatchOptions options); + QueryStringParameterScopeExpression Parse(string source, ResourceType resourceType); } diff --git a/src/JsonApiDotNetCore/Queries/Parsing/QueryStringParameterScopeParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/QueryStringParameterScopeParser.cs index dfb98dc612..fefc85a0d5 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/QueryStringParameterScopeParser.cs +++ b/src/JsonApiDotNetCore/Queries/Parsing/QueryStringParameterScopeParser.cs @@ -10,25 +10,22 @@ namespace JsonApiDotNetCore.Queries.Parsing; public class QueryStringParameterScopeParser : QueryExpressionParser, IQueryStringParameterScopeParser { /// - public QueryStringParameterScopeExpression Parse(string source, ResourceType resourceType, FieldChainPattern pattern, FieldChainPatternMatchOptions options) + public QueryStringParameterScopeExpression Parse(string source, ResourceType resourceType) { ArgumentNullException.ThrowIfNull(resourceType); - ArgumentNullException.ThrowIfNull(pattern); Tokenize(source); - QueryStringParameterScopeExpression expression = ParseQueryStringParameterScope(resourceType, pattern, options); + QueryStringParameterScopeExpression expression = ParseQueryStringParameterScope(resourceType); AssertTokenStackIsEmpty(); return expression; } - protected virtual QueryStringParameterScopeExpression ParseQueryStringParameterScope(ResourceType resourceType, FieldChainPattern pattern, - FieldChainPatternMatchOptions options) + protected virtual QueryStringParameterScopeExpression ParseQueryStringParameterScope(ResourceType resourceType) { ArgumentNullException.ThrowIfNull(resourceType); - ArgumentNullException.ThrowIfNull(pattern); int position = GetNextTokenPositionOrEnd(); @@ -45,7 +42,7 @@ protected virtual QueryStringParameterScopeExpression ParseQueryStringParameterS { TokenStack.Pop(); - scope = ParseFieldChain(pattern, options, resourceType, null); + scope = ParseFieldChain(BuiltInPatterns.RelationshipChainEndingInToMany, FieldChainPatternMatchOptions.None, resourceType, null); EatSingleCharacterToken(TokenKind.CloseBracket); } diff --git a/src/JsonApiDotNetCore/Queries/QueryableBuilding/IncludeClauseBuilder.cs b/src/JsonApiDotNetCore/Queries/QueryableBuilding/IncludeClauseBuilder.cs index 115202b138..e46f1249ab 100644 --- a/src/JsonApiDotNetCore/Queries/QueryableBuilding/IncludeClauseBuilder.cs +++ b/src/JsonApiDotNetCore/Queries/QueryableBuilding/IncludeClauseBuilder.cs @@ -10,8 +10,6 @@ namespace JsonApiDotNetCore.Queries.QueryableBuilding; [PublicAPI] public class IncludeClauseBuilder : QueryClauseBuilder, IIncludeClauseBuilder { - private static readonly IncludeChainConverter IncludeChainConverter = new(); - public virtual Expression ApplyInclude(IncludeExpression include, QueryClauseBuilderContext context) { ArgumentNullException.ThrowIfNull(include); @@ -26,7 +24,7 @@ public override Expression VisitInclude(IncludeExpression expression, QueryClaus ApplyEagerLoads(context.ResourceType.EagerLoads, null, propertyPaths); - foreach (ResourceFieldChainExpression chain in IncludeChainConverter.GetRelationshipChains(expression)) + foreach (ResourceFieldChainExpression chain in IncludeChainConverter.Instance.GetRelationshipChains(expression)) { ProcessRelationshipChain(chain, propertyPaths); } diff --git a/src/JsonApiDotNetCore/QueryStrings/FilterQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/FilterQueryStringParameterReader.cs index 6b85273ddb..d93002ba2b 100644 --- a/src/JsonApiDotNetCore/QueryStrings/FilterQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/FilterQueryStringParameterReader.cs @@ -7,7 +7,6 @@ using JsonApiDotNetCore.Queries; using JsonApiDotNetCore.Queries.Expressions; using JsonApiDotNetCore.Queries.Parsing; -using JsonApiDotNetCore.QueryStrings.FieldChains; using Microsoft.Extensions.Primitives; namespace JsonApiDotNetCore.QueryStrings; @@ -115,8 +114,7 @@ private void ReadSingleValue(string parameterName, string parameterValue) private ResourceFieldChainExpression? GetScope(string parameterName) { - QueryStringParameterScopeExpression parameterScope = _scopeParser.Parse(parameterName, RequestResourceType, - BuiltInPatterns.RelationshipChainEndingInToMany, FieldChainPatternMatchOptions.None); + QueryStringParameterScopeExpression parameterScope = _scopeParser.Parse(parameterName, RequestResourceType); if (parameterScope.Scope == null) { diff --git a/src/JsonApiDotNetCore/QueryStrings/SortQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/SortQueryStringParameterReader.cs index 203ee70235..82324a30e9 100644 --- a/src/JsonApiDotNetCore/QueryStrings/SortQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/SortQueryStringParameterReader.cs @@ -6,7 +6,6 @@ using JsonApiDotNetCore.Queries; using JsonApiDotNetCore.Queries.Expressions; using JsonApiDotNetCore.Queries.Parsing; -using JsonApiDotNetCore.QueryStrings.FieldChains; using Microsoft.Extensions.Primitives; namespace JsonApiDotNetCore.QueryStrings; @@ -72,8 +71,7 @@ public virtual void Read(string parameterName, StringValues parameterValue) private ResourceFieldChainExpression? GetScope(string parameterName) { - QueryStringParameterScopeExpression parameterScope = _scopeParser.Parse(parameterName, RequestResourceType, - BuiltInPatterns.RelationshipChainEndingInToMany, FieldChainPatternMatchOptions.None); + QueryStringParameterScopeExpression parameterScope = _scopeParser.Parse(parameterName, RequestResourceType); if (parameterScope.Scope == null) { diff --git a/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionRewriterTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionRewriterTests.cs index 7f18610f89..38811a78e1 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionRewriterTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionRewriterTests.cs @@ -4,7 +4,6 @@ using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Queries.Expressions; using JsonApiDotNetCore.Queries.Parsing; -using JsonApiDotNetCore.QueryStrings.FieldChains; using JsonApiDotNetCore.Resources; using JsonApiDotNetCoreTests.IntegrationTests.QueryStrings; using Microsoft.Extensions.Logging.Abstractions; @@ -198,8 +197,7 @@ public void VisitParameterScope(string expressionText, string expectedTypes) var parser = new QueryStringParameterScopeParser(); ResourceType blogType = ResourceGraph.GetResourceType(); - QueryExpression expression = - parser.Parse(expressionText, blogType, BuiltInPatterns.RelationshipChainEndingInToMany, FieldChainPatternMatchOptions.None); + QueryExpression expression = parser.Parse(expressionText, blogType); var rewriter = new TestableQueryExpressionRewriter(); From 6376f4cbded521148c1a4b2cf54140e8c7c79f0a Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Thu, 7 Aug 2025 02:55:58 +0200 Subject: [PATCH 2/4] Expose source text on parser base class --- .../Queries/Parsing/IncludeParser.cs | 26 +++++++++---------- .../Queries/Parsing/QueryExpressionParser.cs | 12 ++++++--- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs index eb328f3183..4ca397bc4a 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs +++ b/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs @@ -29,7 +29,7 @@ public IncludeExpression Parse(string source, ResourceType resourceType) Tokenize(source); - IncludeExpression expression = ParseInclude(source, resourceType); + IncludeExpression expression = ParseInclude(resourceType); AssertTokenStackIsEmpty(); ValidateMaximumIncludeDepth(expression, 0); @@ -37,9 +37,8 @@ public IncludeExpression Parse(string source, ResourceType resourceType) return expression; } - protected virtual IncludeExpression ParseInclude(string source, ResourceType resourceType) + protected virtual IncludeExpression ParseInclude(ResourceType resourceType) { - ArgumentNullException.ThrowIfNull(source); ArgumentNullException.ThrowIfNull(resourceType); var treeRoot = IncludeTreeNode.CreateRoot(resourceType); @@ -56,13 +55,13 @@ protected virtual IncludeExpression ParseInclude(string source, ResourceType res isAtStart = false; } - ParseRelationshipChain(source, treeRoot); + ParseRelationshipChain(treeRoot); } return treeRoot.ToExpression(); } - private void ParseRelationshipChain(string source, IncludeTreeNode treeRoot) + private void ParseRelationshipChain(IncludeTreeNode treeRoot) { // 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: @@ -90,30 +89,29 @@ private void ParseRelationshipChain(string source, 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 children = ParseRelationshipName(source, [treeRoot]); + ReadOnlyCollection children = ParseRelationshipName([treeRoot]); while (TokenStack.TryPeek(out Token? nextToken) && nextToken.Kind == TokenKind.Period) { EatSingleCharacterToken(TokenKind.Period); - children = ParseRelationshipName(source, children); + children = ParseRelationshipName(children); } } - private ReadOnlyCollection ParseRelationshipName(string source, IReadOnlyCollection parents) + private ReadOnlyCollection ParseRelationshipName(IReadOnlyCollection parents) { int position = GetNextTokenPositionOrEnd(); if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.Text) { - return LookupRelationshipName(token.Value!, parents, source, position); + return LookupRelationshipName(token.Value!, parents, position); } throw new QueryParseException("Relationship name expected.", position); } - private static ReadOnlyCollection LookupRelationshipName(string relationshipName, IReadOnlyCollection parents, - string source, int position) + private ReadOnlyCollection LookupRelationshipName(string relationshipName, IReadOnlyCollection parents, int position) { List children = []; HashSet relationshipsFound = []; @@ -135,7 +133,7 @@ private static ReadOnlyCollection LookupRelationshipName(string } AssertRelationshipsFound(relationshipsFound, relationshipName, parents, position); - AssertAtLeastOneCanBeIncluded(relationshipsFound, relationshipName, source, position); + AssertAtLeastOneCanBeIncluded(relationshipsFound, relationshipName, position); return children.AsReadOnly(); } @@ -201,7 +199,7 @@ private static string GetErrorMessageForNoneFound(string relationshipName, Resou return builder.ToString(); } - private static void AssertAtLeastOneCanBeIncluded(HashSet relationshipsFound, string relationshipName, string source, int position) + private void AssertAtLeastOneCanBeIncluded(HashSet relationshipsFound, string relationshipName, int position) { if (relationshipsFound.All(relationship => relationship.IsIncludeBlocked())) { @@ -209,7 +207,7 @@ private static void AssertAtLeastOneCanBeIncluded(HashSet string message = $"Including the relationship '{relationshipName}' on '{resourceType}' is not allowed."; var exception = new QueryParseException(message, position); - string specificMessage = exception.GetMessageWithPosition(source); + string specificMessage = exception.GetMessageWithPosition(Source); throw new InvalidQueryStringParameterException("include", "The specified include is invalid.", specificMessage); } diff --git a/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs index 772be38530..c1c9be49b8 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs +++ b/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs @@ -18,8 +18,6 @@ namespace JsonApiDotNetCore.Queries.Parsing; [PublicAPI] public abstract class QueryExpressionParser { - private int _endOfSourcePosition; - /// /// Contains the tokens produced from the source text, after has been called. /// @@ -28,6 +26,11 @@ public abstract class QueryExpressionParser /// protected Stack TokenStack { get; private set; } = new(); + /// + /// Contains the source text that tokens were produced from, after has been called. + /// + protected string Source { get; private set; } = string.Empty; + /// /// Enables derived types to throw a when usage of a JSON:API field inside a field chain is not permitted. /// @@ -45,9 +48,10 @@ protected virtual void Tokenize(string source) { ArgumentNullException.ThrowIfNull(source); + Source = source; + var tokenizer = new QueryTokenizer(source); TokenStack = new Stack(tokenizer.EnumerateTokens().Reverse()); - _endOfSourcePosition = source.Length; } /// @@ -154,7 +158,7 @@ protected int GetNextTokenPositionOrEnd() return nextToken.Position; } - return _endOfSourcePosition; + return Source.Length; } /// From d90416ab42b78b4708282779b5517e5985914506 Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Thu, 7 Aug 2025 03:37:05 +0200 Subject: [PATCH 3/4] Move parsing of include chains into base class --- .../Queries/Parsing/IncludeParser.cs | 223 ----------------- .../Queries/Parsing/QueryExpressionParser.cs | 225 +++++++++++++++++- 2 files changed, 223 insertions(+), 225 deletions(-) diff --git a/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs index 4ca397bc4a..6c8e3af5e0 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs +++ b/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs @@ -1,9 +1,5 @@ -using System.Collections.Immutable; -using System.Collections.ObjectModel; -using System.Text; using JetBrains.Annotations; using JsonApiDotNetCore.Configuration; -using JsonApiDotNetCore.Errors; using JsonApiDotNetCore.Queries.Expressions; using JsonApiDotNetCore.Resources.Annotations; @@ -61,158 +57,6 @@ protected virtual IncludeExpression ParseInclude(ResourceType resourceType) return treeRoot.ToExpression(); } - private void ParseRelationshipChain(IncludeTreeNode treeRoot) - { - // 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: - // - // public abstract class ShoppingBasket : Identifiable - // { - // } - // - // public sealed class SilverShoppingBasket : ShoppingBasket - // { - // [HasMany] - // public ISet
Items { get; get; } - // } - // - // public sealed class PlatinumShoppingBasket : ShoppingBasket - // { - // [HasMany] - // public ISet Items { get; get; } - // } - // - // Now if the include chain has subsequent relationships, we need to scan both Items relationships for matches, - // which is why ParseRelationshipName returns a collection. - // - // The advantage of this unfolding is we don't require callers to upcast in relationship chains. The downside is - // that there's currently no way to include Products without Articles. We could add such optional upcast syntax - // in the future, if desired. - - ReadOnlyCollection children = ParseRelationshipName([treeRoot]); - - while (TokenStack.TryPeek(out Token? nextToken) && nextToken.Kind == TokenKind.Period) - { - EatSingleCharacterToken(TokenKind.Period); - - children = ParseRelationshipName(children); - } - } - - private ReadOnlyCollection ParseRelationshipName(IReadOnlyCollection parents) - { - int position = GetNextTokenPositionOrEnd(); - - if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.Text) - { - return LookupRelationshipName(token.Value!, parents, position); - } - - throw new QueryParseException("Relationship name expected.", position); - } - - private ReadOnlyCollection LookupRelationshipName(string relationshipName, IReadOnlyCollection parents, int position) - { - List children = []; - HashSet relationshipsFound = []; - - foreach (IncludeTreeNode parent in parents) - { - // 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 relationships = GetRelationshipsInConcreteTypes(parent.Relationship.RightType, relationshipName); - - if (relationships.Count > 0) - { - relationshipsFound.UnionWith(relationships); - - RelationshipAttribute[] relationshipsToInclude = relationships.Where(relationship => !relationship.IsIncludeBlocked()).ToArray(); - ReadOnlyCollection affectedChildren = parent.EnsureChildren(relationshipsToInclude); - children.AddRange(affectedChildren); - } - } - - AssertRelationshipsFound(relationshipsFound, relationshipName, parents, position); - AssertAtLeastOneCanBeIncluded(relationshipsFound, relationshipName, position); - - return children.AsReadOnly(); - } - - private static HashSet GetRelationshipsInConcreteTypes(ResourceType resourceType, string relationshipName) - { - HashSet 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 relationshipsToInclude) - { - foreach (ResourceType derivedType in relationship.LeftType.GetAllConcreteDerivedTypes()) - { - RelationshipAttribute relationshipInDerived = derivedType.GetRelationshipByPublicName(relationship.PublicName); - relationshipsToInclude.Add(relationshipInDerived); - } - } - - private static void AssertRelationshipsFound(HashSet relationshipsFound, string relationshipName, - IReadOnlyCollection parents, int position) - { - if (relationshipsFound.Count > 0) - { - return; - } - - ResourceType[] parentResourceTypes = parents.Select(parent => parent.Relationship.RightType).Distinct().ToArray(); - - bool hasDerivedTypes = parents.Any(parent => parent.Relationship.RightType.DirectlyDerivedTypes.Count > 0); - - string message = GetErrorMessageForNoneFound(relationshipName, parentResourceTypes, hasDerivedTypes); - throw new QueryParseException(message, position); - } - - private static string GetErrorMessageForNoneFound(string relationshipName, ResourceType[] parentResourceTypes, bool hasDerivedTypes) - { - var builder = new StringBuilder($"Relationship '{relationshipName}'"); - - if (parentResourceTypes.Length == 1) - { - builder.Append($" does not exist on resource type '{parentResourceTypes.First().PublicName}'"); - } - else - { - string typeNames = string.Join(", ", parentResourceTypes.Select(type => $"'{type.PublicName}'")); - builder.Append($" does not exist on any of the resource types {typeNames}"); - } - - builder.Append(hasDerivedTypes ? " or any of its derived types." : "."); - - return builder.ToString(); - } - - private void AssertAtLeastOneCanBeIncluded(HashSet relationshipsFound, string relationshipName, int position) - { - if (relationshipsFound.All(relationship => relationship.IsIncludeBlocked())) - { - ResourceType resourceType = relationshipsFound.First().LeftType; - string message = $"Including the relationship '{relationshipName}' on '{resourceType}' is not allowed."; - - var exception = new QueryParseException(message, position); - string specificMessage = exception.GetMessageWithPosition(Source); - - throw new InvalidQueryStringParameterException("include", "The specified include is invalid.", specificMessage); - } - } - private void ValidateMaximumIncludeDepth(IncludeExpression include, int position) { if (_options.MaximumIncludeDepth != null) @@ -245,71 +89,4 @@ private static void ThrowIfMaximumDepthExceeded(IncludeElementExpression include parentChain.Pop(); } - - private sealed class IncludeTreeNode - { - private readonly Dictionary _children = []; - - public RelationshipAttribute Relationship { get; } - - private IncludeTreeNode(RelationshipAttribute relationship) - { - Relationship = relationship; - } - - public static IncludeTreeNode CreateRoot(ResourceType resourceType) - { - var relationship = new HiddenRootRelationshipAttribute(resourceType); - return new IncludeTreeNode(relationship); - } - - public ReadOnlyCollection EnsureChildren(RelationshipAttribute[] relationships) - { - foreach (RelationshipAttribute relationship in relationships) - { - if (!_children.ContainsKey(relationship)) - { - var newChild = new IncludeTreeNode(relationship); - _children.Add(relationship, newChild); - } - } - - return _children.Where(pair => relationships.Contains(pair.Key)).Select(pair => pair.Value).ToArray().AsReadOnly(); - } - - public IncludeExpression ToExpression() - { - IncludeElementExpression element = ToElementExpression(); - - if (element.Relationship is HiddenRootRelationshipAttribute) - { - return element.Children.Count > 0 ? new IncludeExpression(element.Children) : IncludeExpression.Empty; - } - - return new IncludeExpression(ImmutableHashSet.Create(element)); - } - - private IncludeElementExpression ToElementExpression() - { - IImmutableSet elementChildren = _children.Values.Select(child => child.ToElementExpression()).ToImmutableHashSet(); - return new IncludeElementExpression(Relationship, elementChildren); - } - - public override string ToString() - { - IncludeExpression include = ToExpression(); - return include.ToFullString(); - } - - private sealed class HiddenRootRelationshipAttribute : RelationshipAttribute - { - public HiddenRootRelationshipAttribute(ResourceType rightType) - { - ArgumentNullException.ThrowIfNull(rightType); - - RightType = rightType; - PublicName = "<>"; - } - } - } } diff --git a/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs index c1c9be49b8..60d02dd4f4 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs +++ b/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs @@ -1,7 +1,9 @@ using System.Collections.Immutable; +using System.Collections.ObjectModel; using System.Text; using JetBrains.Annotations; using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Errors; using JsonApiDotNetCore.Queries.Expressions; using JsonApiDotNetCore.QueryStrings.FieldChains; using JsonApiDotNetCore.Resources.Annotations; @@ -179,8 +181,8 @@ protected int GetRelativePositionOfLastFieldInChain(ResourceFieldChainExpression } /// - /// Throws a when isn't empty. Derived types should call this when parsing has completed, to - /// ensure all input has been processed. + /// Throws a when is not empty. Derived types should call this when parsing has completed, + /// to ensure all input has been processed. /// protected void AssertTokenStackIsEmpty() { @@ -190,4 +192,223 @@ protected void AssertTokenStackIsEmpty() throw new QueryParseException("End of expression expected.", position); } } + + private protected void ParseRelationshipChain(IncludeTreeNode treeRoot) + { + // 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: + // + // public abstract class ShoppingBasket : Identifiable + // { + // } + // + // public sealed class SilverShoppingBasket : ShoppingBasket + // { + // [HasMany] + // public ISet
Items { get; get; } + // } + // + // public sealed class PlatinumShoppingBasket : ShoppingBasket + // { + // [HasMany] + // public ISet Items { get; get; } + // } + // + // Now if the include chain has subsequent relationships, we need to scan both Items relationships for matches, + // which is why ParseRelationshipName returns a collection. + // + // The advantage of this unfolding is we don't require callers to upcast in relationship chains. The downside is + // that there's currently no way to include Products without Articles. We could add such optional upcast syntax + // in the future, if desired. + + ReadOnlyCollection children = ParseRelationshipName([treeRoot]); + + while (TokenStack.TryPeek(out Token? nextToken) && nextToken.Kind == TokenKind.Period) + { + EatSingleCharacterToken(TokenKind.Period); + + children = ParseRelationshipName(children); + } + } + + private ReadOnlyCollection ParseRelationshipName(IReadOnlyCollection parents) + { + int position = GetNextTokenPositionOrEnd(); + + if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.Text) + { + return LookupRelationshipName(token.Value!, parents, position); + } + + throw new QueryParseException("Relationship name expected.", position); + } + + private ReadOnlyCollection LookupRelationshipName(string relationshipName, IReadOnlyCollection parents, int position) + { + List children = []; + HashSet relationshipsFound = []; + + foreach (IncludeTreeNode parent in parents) + { + // 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 relationships = GetRelationshipsInConcreteTypes(parent.Relationship.RightType, relationshipName); + + if (relationships.Count > 0) + { + relationshipsFound.UnionWith(relationships); + + RelationshipAttribute[] relationshipsToInclude = relationships.Where(relationship => !relationship.IsIncludeBlocked()).ToArray(); + IReadOnlyCollection affectedChildren = parent.EnsureChildren(relationshipsToInclude); + children.AddRange(affectedChildren); + } + } + + AssertRelationshipsFound(relationshipsFound, relationshipName, parents, position); + AssertAtLeastOneCanBeIncluded(relationshipsFound, relationshipName, position); + + return children.AsReadOnly(); + } + + private static HashSet GetRelationshipsInConcreteTypes(ResourceType resourceType, string relationshipName) + { + HashSet 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 relationshipsToInclude) + { + foreach (ResourceType derivedType in relationship.LeftType.GetAllConcreteDerivedTypes()) + { + RelationshipAttribute relationshipInDerived = derivedType.GetRelationshipByPublicName(relationship.PublicName); + relationshipsToInclude.Add(relationshipInDerived); + } + } + + private static void AssertRelationshipsFound(HashSet relationshipsFound, string relationshipName, + IReadOnlyCollection parents, int position) + { + if (relationshipsFound.Count > 0) + { + return; + } + + ResourceType[] parentResourceTypes = parents.Select(parent => parent.Relationship.RightType).Distinct().ToArray(); + + bool hasDerivedTypes = parents.Any(parent => parent.Relationship.RightType.DirectlyDerivedTypes.Count > 0); + + string message = GetErrorMessageForNoneFound(relationshipName, parentResourceTypes, hasDerivedTypes); + throw new QueryParseException(message, position); + } + + private static string GetErrorMessageForNoneFound(string relationshipName, ResourceType[] parentResourceTypes, bool hasDerivedTypes) + { + var builder = new StringBuilder($"Relationship '{relationshipName}'"); + + if (parentResourceTypes.Length == 1) + { + builder.Append($" does not exist on resource type '{parentResourceTypes.First().PublicName}'"); + } + else + { + string typeNames = string.Join(", ", parentResourceTypes.Select(type => $"'{type.PublicName}'")); + builder.Append($" does not exist on any of the resource types {typeNames}"); + } + + builder.Append(hasDerivedTypes ? " or any of its derived types." : "."); + + return builder.ToString(); + } + + private void AssertAtLeastOneCanBeIncluded(HashSet relationshipsFound, string relationshipName, int position) + { + if (relationshipsFound.All(relationship => relationship.IsIncludeBlocked())) + { + ResourceType resourceType = relationshipsFound.First().LeftType; + string message = $"Including the relationship '{relationshipName}' on '{resourceType}' is not allowed."; + + var exception = new QueryParseException(message, position); + string specificMessage = exception.GetMessageWithPosition(Source); + + throw new InvalidQueryStringParameterException("include", "The specified include is invalid.", specificMessage); + } + } + + internal sealed class IncludeTreeNode + { + private readonly Dictionary _children = []; + + public RelationshipAttribute Relationship { get; } + + private IncludeTreeNode(RelationshipAttribute relationship) + { + Relationship = relationship; + } + + public static IncludeTreeNode CreateRoot(ResourceType resourceType) + { + var relationship = new HiddenRootRelationshipAttribute(resourceType); + return new IncludeTreeNode(relationship); + } + + public IReadOnlyCollection EnsureChildren(RelationshipAttribute[] relationships) + { + foreach (RelationshipAttribute relationship in relationships) + { + if (!_children.ContainsKey(relationship)) + { + var newChild = new IncludeTreeNode(relationship); + _children.Add(relationship, newChild); + } + } + + return _children.Where(pair => relationships.Contains(pair.Key)).Select(pair => pair.Value).ToArray().AsReadOnly(); + } + + public IncludeExpression ToExpression() + { + IncludeElementExpression element = ToElementExpression(); + + if (element.Relationship is HiddenRootRelationshipAttribute) + { + return element.Children.Count > 0 ? new IncludeExpression(element.Children) : IncludeExpression.Empty; + } + + return new IncludeExpression(ImmutableHashSet.Create(element)); + } + + private IncludeElementExpression ToElementExpression() + { + IImmutableSet elementChildren = _children.Values.Select(child => child.ToElementExpression()).ToImmutableHashSet(); + return new IncludeElementExpression(Relationship, elementChildren); + } + + public override string ToString() + { + IncludeExpression include = ToExpression(); + return include.ToFullString(); + } + + private sealed class HiddenRootRelationshipAttribute : RelationshipAttribute + { + public HiddenRootRelationshipAttribute(ResourceType rightType) + { + ArgumentNullException.ThrowIfNull(rightType); + + RightType = rightType; + PublicName = "<>"; + } + } + } } From fb826150a409b7a15b9c2f87299bc90bc09fe8e0 Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Thu, 7 Aug 2025 03:42:24 +0200 Subject: [PATCH 4/4] Allow derived types in the scope of sort/filter/page query string parameters --- ...nationElementQueryStringValueExpression.cs | 6 +- .../Expressions/QueryExpressionRewriter.cs | 4 +- .../QueryStringParameterScopeExpression.cs | 4 +- .../Queries/Parsing/IncludeParser.cs | 21 +- .../Queries/Parsing/PaginationParser.cs | 4 +- .../Queries/Parsing/QueryExpressionParser.cs | 99 +++++-- .../QueryStringParameterScopeParser.cs | 5 +- .../FieldChains/BuiltInPatterns.cs | 2 + .../FilterQueryStringParameterReader.cs | 13 +- .../PaginationQueryStringParameterReader.cs | 37 ++- .../QueryStringParameterReader.cs | 5 + .../SortQueryStringParameterReader.cs | 15 +- .../QueryStrings/Filtering/FilterTests.cs | 4 +- .../PaginationWithTotalCountTests.cs | 4 +- .../QueryStrings/Sorting/SortTests.cs | 4 +- .../ResourceInheritanceReadTests.cs | 252 ++++++++++++++++++ .../Queries/QueryExpressionRewriterTests.cs | 4 +- .../UnitTests/Queries/QueryExpressionTests.cs | 4 +- .../QueryStringParameters/FilterParseTests.cs | 27 +- .../PaginationParseTests.cs | 49 ++-- .../QueryStringParameters/SortParseTests.cs | 24 +- test/TestBuildingBlocks/FluentExtensions.cs | 3 +- 22 files changed, 464 insertions(+), 126 deletions(-) diff --git a/src/JsonApiDotNetCore/Queries/Expressions/PaginationElementQueryStringValueExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/PaginationElementQueryStringValueExpression.cs index b6e182a714..0b068f8144 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/PaginationElementQueryStringValueExpression.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/PaginationElementQueryStringValueExpression.cs @@ -10,12 +10,12 @@ namespace JsonApiDotNetCore.Queries.Expressions; /// . ///
[PublicAPI] -public class PaginationElementQueryStringValueExpression(ResourceFieldChainExpression? scope, int value, int position) : QueryExpression +public class PaginationElementQueryStringValueExpression(IncludeExpression? scope, int value, int position) : QueryExpression { /// - /// 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. /// - public ResourceFieldChainExpression? Scope { get; } = scope; + public IncludeExpression? Scope { get; } = scope; /// /// The numeric pagination value. diff --git a/src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs b/src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs index 48613d6f60..071491b8a7 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs +++ b/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; diff --git a/src/JsonApiDotNetCore/Queries/Expressions/QueryStringParameterScopeExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/QueryStringParameterScopeExpression.cs index 4063199dd2..02abe732cd 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/QueryStringParameterScopeExpression.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/QueryStringParameterScopeExpression.cs @@ -25,9 +25,9 @@ public class QueryStringParameterScopeExpression : QueryExpression /// The scope this parameter value applies to, or null for the URL endpoint scope. Chain format for the filter/sort parameters: an optional list /// of relationships, followed by a to-many relationship. /// - public ResourceFieldChainExpression? Scope { get; } + public IncludeExpression? Scope { get; } - public QueryStringParameterScopeExpression(LiteralConstantExpression parameterName, ResourceFieldChainExpression? scope) + public QueryStringParameterScopeExpression(LiteralConstantExpression parameterName, IncludeExpression? scope) { ArgumentNullException.ThrowIfNull(parameterName); diff --git a/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs index 6c8e3af5e0..f8338c93c2 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/IncludeParser.cs +++ b/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) diff --git a/src/JsonApiDotNetCore/Queries/Parsing/PaginationParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/PaginationParser.cs index 669fba7b9c..d6199dade7 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/PaginationParser.cs +++ b/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); diff --git a/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs index 60d02dd4f4..d324ffc11b 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs +++ b/src/JsonApiDotNetCore/Queries/Parsing/QueryExpressionParser.cs @@ -193,7 +193,47 @@ protected void AssertTokenStackIsEmpty() } } - private protected void ParseRelationshipChain(IncludeTreeNode treeRoot) + /// + /// Parses a comma-separated sequence of relationship chains, taking relationships on derived types into account. + /// + 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(); + } + + /// + /// Parses a relationship chain that ends in a to-many relationship, taking relationships on derived types into account. + /// + 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 children = ParseRelationshipName([treeRoot]); + ReadOnlyCollection 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 ParseRelationshipName(IReadOnlyCollection parents) + private ReadOnlyCollection ParseRelationshipName(IReadOnlyCollection 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 LookupRelationshipName(string relationshipName, IReadOnlyCollection parents, int position) + private ReadOnlyCollection LookupRelationshipName(string relationshipName, IReadOnlyCollection parents, + bool requireToMany, int position) { List children = []; HashSet relationshipsFound = []; @@ -252,51 +298,58 @@ private ReadOnlyCollection 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 relationships = GetRelationshipsInConcreteTypes(parent.Relationship.RightType, relationshipName); + HashSet relationships = GetRelationshipsInConcreteTypes(parent.Relationship.RightType, relationshipName, requireToMany); if (relationships.Count > 0) { relationshipsFound.UnionWith(relationships); RelationshipAttribute[] relationshipsToInclude = relationships.Where(relationship => !relationship.IsIncludeBlocked()).ToArray(); - IReadOnlyCollection affectedChildren = parent.EnsureChildren(relationshipsToInclude); + ReadOnlyCollection 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 GetRelationshipsInConcreteTypes(ResourceType resourceType, string relationshipName) + private static HashSet GetRelationshipsInConcreteTypes(ResourceType resourceType, string relationshipName, bool requireToMany) { HashSet 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 relationshipsToInclude) + private static void IncludeRelationshipsFromConcreteDerivedTypes(RelationshipAttribute relationship, bool requireToMany, + HashSet 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 relationshipsFound, string relationshipName, + private static void AssertRelationshipsFound(HashSet relationshipsFound, string relationshipName, bool requireToMany, IReadOnlyCollection parents, int position) { if (relationshipsFound.Count > 0) @@ -308,13 +361,13 @@ private static void AssertRelationshipsFound(HashSet 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 relati } } - internal sealed class IncludeTreeNode + private sealed class IncludeTreeNode { private readonly Dictionary _children = []; @@ -362,7 +415,7 @@ public static IncludeTreeNode CreateRoot(ResourceType resourceType) return new IncludeTreeNode(relationship); } - public IReadOnlyCollection EnsureChildren(RelationshipAttribute[] relationships) + public ReadOnlyCollection EnsureChildren(RelationshipAttribute[] relationships) { foreach (RelationshipAttribute relationship in relationships) { diff --git a/src/JsonApiDotNetCore/Queries/Parsing/QueryStringParameterScopeParser.cs b/src/JsonApiDotNetCore/Queries/Parsing/QueryStringParameterScopeParser.cs index fefc85a0d5..5ba95f3d98 100644 --- a/src/JsonApiDotNetCore/Queries/Parsing/QueryStringParameterScopeParser.cs +++ b/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); } diff --git a/src/JsonApiDotNetCore/QueryStrings/FieldChains/BuiltInPatterns.cs b/src/JsonApiDotNetCore/QueryStrings/FieldChains/BuiltInPatterns.cs index 52e31c8dc7..a1d7f1b62b 100644 --- a/src/JsonApiDotNetCore/QueryStrings/FieldChains/BuiltInPatterns.cs +++ b/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"); } diff --git a/src/JsonApiDotNetCore/QueryStrings/FilterQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/FilterQueryStringParameterReader.cs index d93002ba2b..5c1fffb8b5 100644 --- a/src/JsonApiDotNetCore/QueryStrings/FilterQueryStringParameterReader.cs +++ b/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); diff --git a/src/JsonApiDotNetCore/QueryStrings/PaginationQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/PaginationQueryStringParameterReader.cs index 353204eee2..7f4012478f 100644 --- a/src/JsonApiDotNetCore/QueryStrings/PaginationQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/PaginationQueryStringParameterReader.cs @@ -1,4 +1,3 @@ -using System.Collections.Immutable; using System.Collections.ObjectModel; using JetBrains.Annotations; using JsonApiDotNetCore.Configuration; @@ -145,19 +144,14 @@ public virtual IReadOnlyCollection GetConstraints() { var paginationState = new PaginationState(); - foreach (PaginationElementQueryStringValueExpression element in _pageSizeConstraint?.Elements ?? - ImmutableArray.Empty) + foreach (PaginationElementQueryStringValueExpression element in _pageSizeConstraint?.Elements ?? []) { - MutablePaginationEntry entry = paginationState.ResolveEntryInScope(element.Scope); - entry.PageSize = element.Value == 0 ? null : new PageSize(element.Value); - entry.HasSetPageSize = true; + UpdatePageSize(element, paginationState); } - foreach (PaginationElementQueryStringValueExpression element in _pageNumberConstraint?.Elements ?? - ImmutableArray.Empty) + foreach (PaginationElementQueryStringValueExpression element in _pageNumberConstraint?.Elements ?? []) { - MutablePaginationEntry entry = paginationState.ResolveEntryInScope(element.Scope); - entry.PageNumber = new PageNumber(element.Value); + UpdatePageNumber(element, paginationState); } paginationState.ApplyOptions(_options); @@ -165,6 +159,29 @@ public virtual IReadOnlyCollection GetConstraints() return paginationState.GetExpressionsInScope(); } + private static void UpdatePageSize(PaginationElementQueryStringValueExpression element, PaginationState paginationState) + { + foreach (ResourceFieldChainExpression? scopeChain in element.Scope == null + ? FieldChainInGlobalScope + : IncludeChainConverter.Instance.GetRelationshipChains(element.Scope)) + { + MutablePaginationEntry entry = paginationState.ResolveEntryInScope(scopeChain); + entry.PageSize = element.Value == 0 ? null : new PageSize(element.Value); + entry.HasSetPageSize = true; + } + } + + private static void UpdatePageNumber(PaginationElementQueryStringValueExpression element, PaginationState paginationState) + { + foreach (ResourceFieldChainExpression? scopeChain in element.Scope == null + ? FieldChainInGlobalScope + : IncludeChainConverter.Instance.GetRelationshipChains(element.Scope)) + { + MutablePaginationEntry entry = paginationState.ResolveEntryInScope(scopeChain); + entry.PageNumber = new PageNumber(element.Value); + } + } + private sealed class PaginationState { private readonly MutablePaginationEntry _globalScope = new(); diff --git a/src/JsonApiDotNetCore/QueryStrings/QueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/QueryStringParameterReader.cs index 1b59eed083..e51cb2a5a2 100644 --- a/src/JsonApiDotNetCore/QueryStrings/QueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/QueryStringParameterReader.cs @@ -11,6 +11,11 @@ public abstract class QueryStringParameterReader private readonly IResourceGraph _resourceGraph; private readonly bool _isCollectionRequest; + protected static IReadOnlyCollection FieldChainInGlobalScope { get; } = new[] + { + (ResourceFieldChainExpression?)null + }.AsReadOnly(); + protected ResourceType RequestResourceType { get; } protected bool IsAtomicOperationsRequest { get; } diff --git a/src/JsonApiDotNetCore/QueryStrings/SortQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/SortQueryStringParameterReader.cs index 82324a30e9..8bfe85a9e0 100644 --- a/src/JsonApiDotNetCore/QueryStrings/SortQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/SortQueryStringParameterReader.cs @@ -55,12 +55,17 @@ public virtual void Read(string parameterName, StringValues parameterValue) try { - ResourceFieldChainExpression? scope = GetScope(parameterName); + IncludeExpression? scopeInclude = GetScope(parameterName); parameterNameIsValid = true; - SortExpression sort = GetSort(parameterValue.ToString(), scope); - var expressionInScope = new ExpressionInScope(scope, sort); - _constraints.Add(expressionInScope); + foreach (ResourceFieldChainExpression? scopeChain in scopeInclude == null + ? FieldChainInGlobalScope + : IncludeChainConverter.Instance.GetRelationshipChains(scopeInclude)) + { + SortExpression sort = GetSort(parameterValue.ToString(), scopeChain); + var expressionInScope = new ExpressionInScope(scopeChain, sort); + _constraints.Add(expressionInScope); + } } catch (QueryParseException exception) { @@ -69,7 +74,7 @@ public virtual void Read(string parameterName, StringValues parameterValue) } } - private ResourceFieldChainExpression? GetScope(string parameterName) + private IncludeExpression? GetScope(string parameterName) { QueryStringParameterScopeExpression parameterScope = _scopeParser.Parse(parameterName, RequestResourceType); diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Filtering/FilterTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Filtering/FilterTests.cs index 2f6c468f08..76157aae37 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Filtering/FilterTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Filtering/FilterTests.cs @@ -42,7 +42,7 @@ public async Task Cannot_filter_in_unknown_scope() ErrorObject error = responseDocument.Errors[0]; error.StatusCode.Should().Be(HttpStatusCode.BadRequest); error.Title.Should().Be("The specified filter is invalid."); - error.Detail.Should().Be($"Field '{Unknown.Relationship}' does not exist on resource type 'webAccounts'. {parameterName}"); + error.Detail.Should().Be($"To-many relationship '{Unknown.Relationship}' does not exist on resource type 'webAccounts'. {parameterName}"); error.Source.Should().NotBeNull(); error.Source.Parameter.Should().Be(parameterName.Text); } @@ -65,7 +65,7 @@ public async Task Cannot_filter_in_unknown_nested_scope() ErrorObject error = responseDocument.Errors[0]; error.StatusCode.Should().Be(HttpStatusCode.BadRequest); error.Title.Should().Be("The specified filter is invalid."); - error.Detail.Should().Be($"Field '{Unknown.Relationship}' does not exist on resource type 'blogPosts'. {parameterName}"); + error.Detail.Should().Be($"To-many relationship '{Unknown.Relationship}' does not exist on resource type 'blogPosts'. {parameterName}"); error.Source.Should().NotBeNull(); error.Source.Parameter.Should().Be(parameterName.Text); } diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Pagination/PaginationWithTotalCountTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Pagination/PaginationWithTotalCountTests.cs index 3fb7714c3f..fa02575227 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Pagination/PaginationWithTotalCountTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Pagination/PaginationWithTotalCountTests.cs @@ -466,7 +466,7 @@ public async Task Cannot_paginate_in_unknown_scope() ErrorObject error = responseDocument.Errors[0]; error.StatusCode.Should().Be(HttpStatusCode.BadRequest); error.Title.Should().Be("The specified pagination is invalid."); - error.Detail.Should().Be($"Field '{Unknown.Relationship}' does not exist on resource type 'webAccounts'. {parameterValue}"); + error.Detail.Should().Be($"To-many relationship '{Unknown.Relationship}' does not exist on resource type 'webAccounts'. {parameterValue}"); error.Source.Should().NotBeNull(); error.Source.Parameter.Should().Be("page[number]"); } @@ -489,7 +489,7 @@ public async Task Cannot_paginate_in_unknown_nested_scope() ErrorObject error = responseDocument.Errors[0]; error.StatusCode.Should().Be(HttpStatusCode.BadRequest); error.Title.Should().Be("The specified pagination is invalid."); - error.Detail.Should().Be($"Field '{Unknown.Relationship}' does not exist on resource type 'blogPosts'. {parameterValue}"); + error.Detail.Should().Be($"To-many relationship '{Unknown.Relationship}' does not exist on resource type 'blogPosts'. {parameterValue}"); error.Source.Should().NotBeNull(); error.Source.Parameter.Should().Be("page[size]"); } diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Sorting/SortTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Sorting/SortTests.cs index 45cd010379..413f18b91e 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Sorting/SortTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Sorting/SortTests.cs @@ -468,7 +468,7 @@ public async Task Cannot_sort_in_unknown_scope() ErrorObject error = responseDocument.Errors[0]; error.StatusCode.Should().Be(HttpStatusCode.BadRequest); error.Title.Should().Be("The specified sort is invalid."); - error.Detail.Should().Be($"Field '{Unknown.Relationship}' does not exist on resource type 'webAccounts'. {parameterName}"); + error.Detail.Should().Be($"To-many relationship '{Unknown.Relationship}' does not exist on resource type 'webAccounts'. {parameterName}"); error.Source.Should().NotBeNull(); error.Source.Parameter.Should().Be(parameterName.Text); } @@ -491,7 +491,7 @@ public async Task Cannot_sort_in_unknown_nested_scope() ErrorObject error = responseDocument.Errors[0]; error.StatusCode.Should().Be(HttpStatusCode.BadRequest); error.Title.Should().Be("The specified sort is invalid."); - error.Detail.Should().Be($"Field '{Unknown.Relationship}' does not exist on resource type 'blogPosts'. {parameterName}"); + error.Detail.Should().Be($"To-many relationship '{Unknown.Relationship}' does not exist on resource type 'blogPosts'. {parameterName}"); error.Source.Should().NotBeNull(); error.Source.Parameter.Should().Be(parameterName.Text); } diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceReadTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceReadTests.cs index 04ee189d8a..17bf6f55d9 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceReadTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceReadTests.cs @@ -2323,6 +2323,132 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.Data.ManyValue.Should().ContainSingle(resource => resource.Type == "cars" && resource.Id == car3.StringId); } + [Fact] + public async Task Can_filter_in_scope_of_derived_include_at_abstract_endpoint() + { + // Arrange + Bike bike = _fakers.Bike.GenerateOne(); + + GasolineEngine carEngine = _fakers.GasolineEngine.GenerateOne(); + carEngine.Cylinders = _fakers.Cylinder.GenerateSet(2); + carEngine.Cylinders.ElementAt(0).SparkPlugCount = 10; + + Car car = _fakers.Car.GenerateOne(); + car.Engine = carEngine; + + GasolineEngine truckEngine = _fakers.GasolineEngine.GenerateOne(); + truckEngine.Cylinders = _fakers.Cylinder.GenerateSet(2); + truckEngine.Cylinders.ElementAt(0).SparkPlugCount = 12; + + Truck truck = _fakers.Truck.GenerateOne(); + truck.Engine = truckEngine; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync(); + dbContext.Vehicles.AddRange(bike, car, truck); + await dbContext.SaveChangesAsync(); + }); + + const string route = "/vehicles?include=engine.cylinders&filter[engine.cylinders]=greaterThan(sparkPlugCount,'8')"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.ShouldHaveStatusCode(HttpStatusCode.OK); + + responseDocument.Data.ManyValue.Should().HaveCount(3); + + responseDocument.Included.Should().HaveCount(4); + responseDocument.Included.Where(resource => resource.Type == "gasolineEngines").Should().HaveCount(2); + responseDocument.Included.Where(resource => resource.Type == "cylinders").Should().HaveCount(2); + } + + [Fact] + public async Task Can_filter_in_scope_of_derived_include_with_type_checks_at_abstract_endpoint() + { + // Arrange + GenericFeature numberFeature = _fakers.GenericFeature.GenerateOne().With(feature => + { + NumberProperty numberProperty = _fakers.NumberProperty.GenerateOne(); + numberProperty.Value = _fakers.NumberValue.GenerateOne(); + numberProperty.Value.Content = 999; + + feature.Properties.Add(numberProperty); + }); + + GenericFeature otherNumberFeature = _fakers.GenericFeature.GenerateOne().With(feature => + { + NumberProperty numberProperty = _fakers.NumberProperty.GenerateOne(); + numberProperty.Value = _fakers.NumberValue.GenerateOne(); + + feature.Properties.Add(numberProperty); + }); + + GenericFeature stringFeature = _fakers.GenericFeature.GenerateOne().With(feature => + { + StringProperty stringProperty = _fakers.StringProperty.GenerateOne(); + stringProperty.Value = _fakers.StringValue.GenerateOne(); + stringProperty.Value.Content = "XXX"; + + feature.Properties.Add(stringProperty); + }); + + GenericFeature otherStringFeature = _fakers.GenericFeature.GenerateOne().With(feature => + { + StringProperty stringProperty = _fakers.StringProperty.GenerateOne(); + stringProperty.Value = _fakers.StringValue.GenerateOne(); + + feature.Properties.Add(stringProperty); + }); + + Bike bike = _fakers.Bike.GenerateOne(); + + Tandem tandem = _fakers.Tandem.GenerateOne(); + tandem.Features.Add(otherNumberFeature); + tandem.Features.Add(stringFeature); + + Car car = _fakers.Car.GenerateOne(); + car.Engine = _fakers.GasolineEngine.GenerateOne(); + car.Features.Add(numberFeature); + car.Features.Add(otherStringFeature); + + Truck truck = _fakers.Truck.GenerateOne(); + truck.Engine = _fakers.DieselEngine.GenerateOne(); + truck.Features.Add(otherNumberFeature); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync(); + dbContext.Vehicles.AddRange(bike, tandem, car, truck); + await dbContext.SaveChangesAsync(); + }); + + const string route = "/vehicles?include=features.properties.value&filter[features.properties]=" + + "or(isType(,stringProperties,equals(value.content,'XXX')),isType(,numberProperties,equals(value.content,'999')))"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.ShouldHaveStatusCode(HttpStatusCode.OK); + + responseDocument.Data.ManyValue.Should().HaveCount(4); + + responseDocument.Included.Should().HaveCount(8); + + responseDocument.Included.Should().ContainSingle(resource => resource.Type == "numberValues").Subject.With(resource => + { + resource.Attributes.Should().ContainKey("content").WhoseValue.Should().Be(999); + }); + + responseDocument.Included.Should().ContainSingle(resource => resource.Type == "stringValues").Subject.With(resource => + { + resource.Attributes.Should().ContainKey("content").WhoseValue.Should().Be("XXX"); + }); + } + [Fact] public async Task Can_sort_on_derived_attribute_at_abstract_endpoint() { @@ -2450,6 +2576,76 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.Data.ManyValue[2].Id.Should().Be(car2.StringId); } + [Fact] + public async Task Can_sort_in_scope_of_derived_include_at_abstract_endpoint() + { + // Arrange + Bike bike = _fakers.Bike.GenerateOne(); + + GasolineEngine carEngine = _fakers.GasolineEngine.GenerateOne(); + carEngine.Cylinders = _fakers.Cylinder.GenerateSet(3); + carEngine.Cylinders.ElementAt(0).SparkPlugCount = 10; + carEngine.Cylinders.ElementAt(1).SparkPlugCount = 3; + carEngine.Cylinders.ElementAt(2).SparkPlugCount = 8; + + Car car = _fakers.Car.GenerateOne(); + car.Engine = carEngine; + + GasolineEngine truckEngine = _fakers.GasolineEngine.GenerateOne(); + truckEngine.Cylinders = _fakers.Cylinder.GenerateSet(3); + truckEngine.Cylinders.ElementAt(0).SparkPlugCount = 12; + truckEngine.Cylinders.ElementAt(1).SparkPlugCount = 5; + truckEngine.Cylinders.ElementAt(2).SparkPlugCount = 18; + + Truck truck = _fakers.Truck.GenerateOne(); + truck.Engine = truckEngine; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync(); + dbContext.Vehicles.AddRange(bike, car, truck); + await dbContext.SaveChangesAsync(); + }); + + const string route = "/vehicles?include=engine.cylinders&sort[engine.cylinders]=sparkPlugCount"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.ShouldHaveStatusCode(HttpStatusCode.OK); + + responseDocument.Data.ManyValue.Should().HaveCount(3); + + responseDocument.Included.Should().HaveCount(8); + + responseDocument.Included.Should().ContainSingle(resource => resource.Type == "gasolineEngines" && resource.Id == carEngine.StringId).Subject + .With(resource => + { + resource.Should().NotBeNull(); + + RelationshipObject relationship = resource.Relationships.Should().ContainKey("cylinders").WhoseValue.RefShould().NotBeNull().And.Subject; + relationship.Data.ManyValue.Should().HaveCount(3); + + relationship.Data.ManyValue[0].Id.Should().Be(carEngine.Cylinders.ElementAt(1).StringId); + relationship.Data.ManyValue[1].Id.Should().Be(carEngine.Cylinders.ElementAt(2).StringId); + relationship.Data.ManyValue[2].Id.Should().Be(carEngine.Cylinders.ElementAt(0).StringId); + }); + + responseDocument.Included.Should().ContainSingle(resource => resource.Type == "gasolineEngines" && resource.Id == truckEngine.StringId).Subject + .With(resource => + { + resource.Should().NotBeNull(); + + RelationshipObject relationship = resource.Relationships.Should().ContainKey("cylinders").WhoseValue.RefShould().NotBeNull().And.Subject; + relationship.Data.ManyValue.Should().HaveCount(3); + + relationship.Data.ManyValue[0].Id.Should().Be(truckEngine.Cylinders.ElementAt(1).StringId); + relationship.Data.ManyValue[1].Id.Should().Be(truckEngine.Cylinders.ElementAt(0).StringId); + relationship.Data.ManyValue[2].Id.Should().Be(truckEngine.Cylinders.ElementAt(2).StringId); + }); + } + [Fact] public async Task Cannot_sort_on_ambiguous_derived_attribute() { @@ -2611,4 +2807,60 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.Data.ManyValue[4].Type.Should().Be("carbonWheels"); responseDocument.Data.ManyValue[4].Id.Should().Be(carbonWheel1.StringId); } + + [Fact] + public async Task Can_paginate_in_scope_of_derived_include_at_abstract_endpoint() + { + // Arrange + Bike bike = _fakers.Bike.GenerateOne(); + + GasolineEngine carEngine = _fakers.GasolineEngine.GenerateOne(); + carEngine.Cylinders = _fakers.Cylinder.GenerateSet(3); + + Car car = _fakers.Car.GenerateOne(); + car.Engine = carEngine; + + GasolineEngine truckEngine = _fakers.GasolineEngine.GenerateOne(); + truckEngine.Cylinders = _fakers.Cylinder.GenerateSet(3); + + Truck truck = _fakers.Truck.GenerateOne(); + truck.Engine = truckEngine; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync(); + dbContext.Vehicles.AddRange(bike, car, truck); + await dbContext.SaveChangesAsync(); + }); + + const string route = "/vehicles?include=engine.cylinders&page[size]=engine.cylinders:2"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.ShouldHaveStatusCode(HttpStatusCode.OK); + + responseDocument.Data.ManyValue.Should().HaveCount(3); + + responseDocument.Included.Should().HaveCount(6); + + responseDocument.Included.Should().ContainSingle(resource => resource.Type == "gasolineEngines" && resource.Id == carEngine.StringId).Subject + .With(resource => + { + resource.Should().NotBeNull(); + + RelationshipObject relationship = resource.Relationships.Should().ContainKey("cylinders").WhoseValue.RefShould().NotBeNull().And.Subject; + relationship.Data.ManyValue.Should().HaveCount(2); + }); + + responseDocument.Included.Should().ContainSingle(resource => resource.Type == "gasolineEngines" && resource.Id == truckEngine.StringId).Subject + .With(resource => + { + resource.Should().NotBeNull(); + + RelationshipObject relationship = resource.Relationships.Should().ContainKey("cylinders").WhoseValue.RefShould().NotBeNull().And.Subject; + relationship.Data.ManyValue.Should().HaveCount(2); + }); + } } diff --git a/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionRewriterTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionRewriterTests.cs index 38811a78e1..a691706c15 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionRewriterTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionRewriterTests.cs @@ -167,7 +167,7 @@ public void VisitSort(string expressionText, string expectedTypes) [Theory] [InlineData("2", "PaginationQueryStringValue,PaginationElementQueryStringValue")] - [InlineData("posts:3,2", "PaginationQueryStringValue,PaginationElementQueryStringValue,ResourceFieldChain,PaginationElementQueryStringValue")] + [InlineData("posts:3,2", "PaginationQueryStringValue,PaginationElementQueryStringValue,Include,IncludeElement,PaginationElementQueryStringValue")] public void VisitPagination(string expressionText, string expectedTypes) { // Arrange @@ -190,7 +190,7 @@ public void VisitPagination(string expressionText, string expectedTypes) [Theory] [InlineData("filter", "QueryStringParameterScope,LiteralConstant")] - [InlineData("filter[posts.comments]", "QueryStringParameterScope,LiteralConstant,ResourceFieldChain")] + [InlineData("filter[posts.comments]", "QueryStringParameterScope,LiteralConstant,Include,IncludeElement,IncludeElement")] public void VisitParameterScope(string expressionText, string expectedTypes) { // Arrange diff --git a/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionTests.cs index ad2d4a8d89..c8ed8ec3f9 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/Queries/QueryExpressionTests.cs @@ -282,7 +282,7 @@ public NullConstantExpression NullConstant() public PaginationElementQueryStringValueExpression PaginationElementQueryStringValue() { - return new PaginationElementQueryStringValueExpression(ResourceFieldChainForChildren(), 5, 8); + return new PaginationElementQueryStringValueExpression(Include(), 5, 8); } public PaginationExpression Pagination() @@ -305,7 +305,7 @@ public QueryableHandlerExpression QueryableHandler() public QueryStringParameterScopeExpression QueryStringParameterScope() { - return new QueryStringParameterScopeExpression(LiteralConstant(), ResourceFieldChainForChildren()); + return new QueryStringParameterScopeExpression(LiteralConstant(), Include()); } public ResourceFieldChainExpression ResourceFieldChainForText() diff --git a/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/FilterParseTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/FilterParseTests.cs index c9de843e56..fbfd7aaecb 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/FilterParseTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/FilterParseTests.cs @@ -59,17 +59,21 @@ public void Reader_Is_Enabled(JsonApiQueryStringParameters parametersDisabled, b } [Theory] - [InlineData("filter[^", "Field name expected.")] - [InlineData("filter[^caption]", "Field 'caption' does not exist on resource type 'blogs'.")] - [InlineData("filter[posts.^caption]", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'blogPosts' expected.")] - [InlineData("filter[posts.author^]", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'webAccounts' expected.")] - [InlineData("filter[posts.comments.^text]", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'comments' expected.")] + [InlineData("filter[^", "To-many relationship name expected.")] + [InlineData("filter[^.", "To-many relationship name expected.")] + [InlineData("filter[posts.^]", "To-many relationship name expected.")] + [InlineData("filter[posts.author.^]", "To-many relationship name expected.")] + [InlineData("filter[^unknown]", "To-many relationship 'unknown' does not exist on resource type 'blogs'.")] + [InlineData("filter[^unknown.other]", "Relationship 'unknown' does not exist on resource type 'blogs'.")] + [InlineData("filter[posts.^caption]", "To-many relationship 'caption' does not exist on resource type 'blogPosts'.")] + [InlineData("filter[posts.^author]", "To-many relationship 'author' does not exist on resource type 'blogPosts'.")] + [InlineData("filter[posts.comments.^unknown]", "To-many relationship 'unknown' does not exist on resource type 'comments'.")] + [InlineData("filter[posts.comments.^text]", "To-many relationship 'text' does not exist on resource type 'comments'.")] + [InlineData("filter[posts.comments.^parent]", "To-many relationship 'parent' does not exist on resource type 'comments'.")] + [InlineData("filter[owner.person.^unknown]", "To-many relationship 'unknown' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("filter[owner.person.^unknown.other]", "Relationship 'unknown' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("filter[owner.person.^hasBeard]", "To-many relationship 'hasBeard' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("filter[owner.person.^wife]", "To-many relationship 'wife' does not exist on resource type 'humans' or any of its derived types.")] public void Reader_Read_ParameterName_Fails(string parameterName, string errorMessage) { // Arrange @@ -179,6 +183,7 @@ public void Reader_Read_ParameterValue_Fails(string parameterName, string parame [InlineData("filter[posts]", "equals(caption,'this, that & more')", "posts")] [InlineData("filter[owner.posts]", "equals(caption,'some')", "owner.posts")] [InlineData("filter[posts.comments]", "equals(createdAt,'2000-01-01')", "posts.comments")] + [InlineData("filter[owner.person.wife.children]", "not(equals(mother,null))", "owner.person.wife.children")] [InlineData("filter", "equals(count(posts),'1')", null)] [InlineData("filter", "equals(count(posts),count(owner.posts))", null)] [InlineData("filter", "equals(has(posts),'true')", null)] diff --git a/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/PaginationParseTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/PaginationParseTests.cs index 8020bbb01c..9c102f5d15 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/PaginationParseTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/PaginationParseTests.cs @@ -55,6 +55,7 @@ public void Reader_Is_Enabled(JsonApiQueryStringParameters parametersDisabled, b [Theory] [InlineData("^", "Number or relationship name expected.")] + [InlineData("^.", "Number or relationship name expected.")] [InlineData("1,^", "Number or relationship name expected.")] [InlineData("^(", "Number or relationship name expected.")] [InlineData("^ ", "Unexpected whitespace.")] @@ -66,17 +67,18 @@ public void Reader_Is_Enabled(JsonApiQueryStringParameters parametersDisabled, b [InlineData("1^(", ", expected.")] [InlineData("posts:-^abc", "Digits expected.")] [InlineData("posts:^-1", "Page number cannot be negative or zero.")] - [InlineData("posts.^some", "Field 'some' does not exist on resource type 'blogPosts'.")] - [InlineData("posts.^id", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'blogPosts' expected.")] - [InlineData("posts.comments.^id", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'comments' expected.")] - [InlineData("posts.author^", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'webAccounts' expected.")] - [InlineData("^some", "Field 'some' does not exist on resource type 'blogs'.")] + [InlineData("^unknown", "To-many relationship 'unknown' does not exist on resource type 'blogs'.")] + [InlineData("^unknown.other", "Relationship 'unknown' does not exist on resource type 'blogs'.")] + [InlineData("posts.^", "To-many relationship name expected.")] + [InlineData("posts.^unknown", "To-many relationship 'unknown' does not exist on resource type 'blogPosts'.")] + [InlineData("posts.^id", "To-many relationship 'id' does not exist on resource type 'blogPosts'.")] + [InlineData("posts.^author", "To-many relationship 'author' does not exist on resource type 'blogPosts'.")] + [InlineData("posts.author.^", "To-many relationship name expected.")] + [InlineData("posts.comments.^id", "To-many relationship 'id' does not exist on resource type 'comments'.")] + [InlineData("owner.person.^unknown", "To-many relationship 'unknown' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("owner.person.^unknown.other", "Relationship 'unknown' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("owner.person.^hasBeard", "To-many relationship 'hasBeard' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("owner.person.^wife", "To-many relationship 'wife' does not exist on resource type 'humans' or any of its derived types.")] public void Reader_Read_Page_Number_Fails(string parameterValue, string errorMessage) { // Arrange @@ -101,6 +103,7 @@ public void Reader_Read_Page_Number_Fails(string parameterValue, string errorMes [Theory] [InlineData("^", "Number or relationship name expected.")] + [InlineData("^.", "Number or relationship name expected.")] [InlineData("1,^", "Number or relationship name expected.")] [InlineData("^(", "Number or relationship name expected.")] [InlineData("^ ", "Unexpected whitespace.")] @@ -112,16 +115,18 @@ public void Reader_Read_Page_Number_Fails(string parameterValue, string errorMes [InlineData("1^(", ", expected.")] [InlineData("posts:-^abc", "Digits expected.")] [InlineData("posts:^-1", "Page size cannot be negative.")] - [InlineData("posts.^id", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'blogPosts' expected.")] - [InlineData("posts.comments.^id", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'comments' expected.")] - [InlineData("posts.author^", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'webAccounts' expected.")] - [InlineData("^some", "Field 'some' does not exist on resource type 'blogs'.")] + [InlineData("^unknown", "To-many relationship 'unknown' does not exist on resource type 'blogs'.")] + [InlineData("^unknown.other", "Relationship 'unknown' does not exist on resource type 'blogs'.")] + [InlineData("posts.^", "To-many relationship name expected.")] + [InlineData("posts.^unknown", "To-many relationship 'unknown' does not exist on resource type 'blogPosts'.")] + [InlineData("posts.^id", "To-many relationship 'id' does not exist on resource type 'blogPosts'.")] + [InlineData("posts.^author", "To-many relationship 'author' does not exist on resource type 'blogPosts'.")] + [InlineData("posts.author.^", "To-many relationship name expected.")] + [InlineData("posts.comments.^id", "To-many relationship 'id' does not exist on resource type 'comments'.")] + [InlineData("owner.person.^unknown", "To-many relationship 'unknown' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("owner.person.^unknown.other", "Relationship 'unknown' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("owner.person.^hasBeard", "To-many relationship 'hasBeard' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("owner.person.^wife", "To-many relationship 'wife' does not exist on resource type 'humans' or any of its derived types.")] public void Reader_Read_Page_Size_Fails(string parameterValue, string errorMessage) { // Arrange @@ -155,6 +160,8 @@ public void Reader_Read_Page_Size_Fails(string parameterValue, string errorMessa [InlineData("posts:4,3", "posts:10,20", "|posts", "Page number: 3, size: 20|Page number: 4, size: 10")] [InlineData("posts:4,posts.comments:5,3", "posts:10,posts.comments:15,20", "|posts|posts.comments", "Page number: 3, size: 20|Page number: 4, size: 10|Page number: 5, size: 15")] + [InlineData("owner.person.wife.children:5,2", "owner.person.husband.children:8,3", "|owner.person.husband.children|owner.person.wife.children", + "Page number: 2, size: 3|Page number: 1, size: 8|Page number: 5, size: 25")] public void Reader_Read_Pagination_Succeeds(string? pageNumber, string? pageSize, string scopeTreesExpected, string valueTreesExpected) { // Act diff --git a/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/SortParseTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/SortParseTests.cs index 24832cbc7d..0fb39c44d6 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/SortParseTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/SortParseTests.cs @@ -54,14 +54,21 @@ public void Reader_Is_Enabled(JsonApiQueryStringParameters parametersDisabled, b } [Theory] - [InlineData("sort[^", "Field name expected.")] - [InlineData("sort[^abc.def]", "Field 'abc' does not exist on resource type 'blogs'.")] - [InlineData("sort[posts.^caption]", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'blogPosts' expected.")] - [InlineData("sort[posts.author^]", - "Field chain on resource type 'blogs' failed to match the pattern: zero or more relationships, followed by a to-many relationship. " + - "Relationship on resource type 'webAccounts' expected.")] + [InlineData("sort[^", "To-many relationship name expected.")] + [InlineData("sort[^.", "To-many relationship name expected.")] + [InlineData("sort[posts.^]", "To-many relationship name expected.")] + [InlineData("sort[posts.author.^]", "To-many relationship name expected.")] + [InlineData("sort[^unknown]", "To-many relationship 'unknown' does not exist on resource type 'blogs'.")] + [InlineData("sort[^unknown.other]", "Relationship 'unknown' does not exist on resource type 'blogs'.")] + [InlineData("sort[posts.^caption]", "To-many relationship 'caption' does not exist on resource type 'blogPosts'.")] + [InlineData("sort[posts.^author]", "To-many relationship 'author' does not exist on resource type 'blogPosts'.")] + [InlineData("sort[posts.comments.^unknown]", "To-many relationship 'unknown' does not exist on resource type 'comments'.")] + [InlineData("sort[posts.comments.^text]", "To-many relationship 'text' does not exist on resource type 'comments'.")] + [InlineData("sort[posts.comments.^parent]", "To-many relationship 'parent' does not exist on resource type 'comments'.")] + [InlineData("sort[owner.person.^unknown]", "To-many relationship 'unknown' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("sort[owner.person.^unknown.other]", "Relationship 'unknown' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("sort[owner.person.^hasBeard]", "To-many relationship 'hasBeard' does not exist on resource type 'humans' or any of its derived types.")] + [InlineData("sort[owner.person.^wife]", "To-many relationship 'wife' does not exist on resource type 'humans' or any of its derived types.")] public void Reader_Read_ParameterName_Fails(string parameterName, string errorMessage) { // Arrange @@ -142,6 +149,7 @@ public void Reader_Read_ParameterValue_Fails(string parameterName, string parame [InlineData("sort", "-count(posts),id", null)] [InlineData("sort[posts]", "count(comments),-id", "posts")] [InlineData("sort[owner.posts]", "-caption", "owner.posts")] + [InlineData("sort[owner.person.wife.children]", "-name", "owner.person.wife.children")] [InlineData("sort[posts]", "author.userName", "posts")] [InlineData("sort[posts]", "-caption,-author.userName", "posts")] [InlineData("sort[posts]", "caption,author.userName,-id", "posts")] diff --git a/test/TestBuildingBlocks/FluentExtensions.cs b/test/TestBuildingBlocks/FluentExtensions.cs index 1061a71765..28b414f0ef 100644 --- a/test/TestBuildingBlocks/FluentExtensions.cs +++ b/test/TestBuildingBlocks/FluentExtensions.cs @@ -97,9 +97,10 @@ public static WhoseValueConstraint Conta return source.ContainKey(expected); } - public static void With(this T subject, [InstantHandle] Action continuation) + public static T With(this T subject, [InstantHandle] Action continuation) { continuation(subject); + return subject; } public sealed class StrongReferenceTypeAssertions(TReference subject)