From 32101de1904642a617377a57e071003146039784 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Sun, 2 May 2021 22:03:44 +0200 Subject: [PATCH 1/5] Added support for passing a filter condition as second argument to the has() function. Example: GET /blogs?filter=has(posts,not(equals(url,null))) --- docs/usage/reading/filtering.md | 10 +++ .../CollectionNotEmptyExpression.cs | 27 ++++++-- .../Expressions/QueryExpressionRewriter.cs | 4 +- .../Queries/Internal/Parsing/FilterParser.cs | 26 ++++++- .../QueryableBuilding/QueryableBuilder.cs | 2 +- .../QueryableBuilding/WhereClauseBuilder.cs | 37 ++++++++-- .../Filtering/FilterDepthTests.cs | 67 +++++++++++++++++++ .../Filtering/FilterOperatorTests.cs | 47 +++++++++++++ .../QueryStringParameters/FilterParseTests.cs | 6 +- 9 files changed, 211 insertions(+), 15 deletions(-) diff --git a/docs/usage/reading/filtering.md b/docs/usage/reading/filtering.md index 90b03462d8..5d7cf8692f 100644 --- a/docs/usage/reading/filtering.md +++ b/docs/usage/reading/filtering.md @@ -74,6 +74,16 @@ Putting it all together, you can build quite complex filters, such as: GET /blogs?include=owner.articles.revisions&filter=and(or(equals(title,'Technology'),has(owner.articles)),not(equals(owner.lastName,null)))&filter[owner.articles]=equals(caption,'Two')&filter[owner.articles.revisions]=greaterThan(publishTime,'2005-05-05') HTTP/1.1 ``` +_since v4.2_ + +The `has` function takes an optional filter condition as second parameter, for example: + +```http +GET /customers?filter=has(orders,not(equals(status,'Paid'))) HTTP/1.1 +``` + +Which returns only customers that have at least one unpaid order. + # Legacy filters The next section describes how filtering worked in versions prior to v4.0. They are always applied on the set of resources being requested (no nesting). diff --git a/src/JsonApiDotNetCore/Queries/Expressions/CollectionNotEmptyExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/CollectionNotEmptyExpression.cs index 1d398192f8..5aaa958feb 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/CollectionNotEmptyExpression.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/CollectionNotEmptyExpression.cs @@ -1,21 +1,25 @@ +using System; +using System.Text; using JetBrains.Annotations; using JsonApiDotNetCore.Queries.Internal.Parsing; namespace JsonApiDotNetCore.Queries.Expressions { /// - /// Represents the "has" filter function, resulting from text such as: has(articles) + /// Represents the "has" filter function, resulting from text such as: has(articles) or has(articles,equals(isHidden,'false')) /// [PublicAPI] public class CollectionNotEmptyExpression : FilterExpression { public ResourceFieldChainExpression TargetCollection { get; } + public FilterExpression Filter { get; } - public CollectionNotEmptyExpression(ResourceFieldChainExpression targetCollection) + public CollectionNotEmptyExpression(ResourceFieldChainExpression targetCollection, FilterExpression filter) { ArgumentGuard.NotNull(targetCollection, nameof(targetCollection)); TargetCollection = targetCollection; + Filter = filter; } public override TResult Accept(QueryExpressionVisitor visitor, TArgument argument) @@ -25,7 +29,20 @@ public override TResult Accept(QueryExpressionVisitor _validateSingleFieldCallback; private ResourceContext _resourceContextInScope; @@ -22,8 +23,10 @@ public FilterParser(IResourceContextProvider resourceContextProvider, IResourceF Action validateSingleFieldCallback = null) : base(resourceContextProvider) { + ArgumentGuard.NotNull(resourceContextProvider, nameof(resourceContextProvider)); ArgumentGuard.NotNull(resourceFactory, nameof(resourceFactory)); + _resourceContextProvider = resourceContextProvider; _resourceFactory = resourceFactory; _validateSingleFieldCallback = validateSingleFieldCallback; } @@ -234,10 +237,31 @@ protected CollectionNotEmptyExpression ParseHas() EatSingleCharacterToken(TokenKind.OpenParen); ResourceFieldChainExpression targetCollection = ParseFieldChain(FieldChainRequirements.EndsInToMany, null); + FilterExpression filter = null; + + if (TokenStack.TryPeek(out Token nextToken) && nextToken.Kind == TokenKind.Comma) + { + EatSingleCharacterToken(TokenKind.Comma); + + filter = ParseFilterInHas((HasManyAttribute)targetCollection.Fields.Last()); + } EatSingleCharacterToken(TokenKind.CloseParen); - return new CollectionNotEmptyExpression(targetCollection); + return new CollectionNotEmptyExpression(targetCollection, filter); + } + + private FilterExpression ParseFilterInHas(HasManyAttribute hasManyRelationship) + { + ResourceContext outerScopeBackup = _resourceContextInScope; + + Type innerResourceType = hasManyRelationship.RightType; + _resourceContextInScope = _resourceContextProvider.GetResourceContext(innerResourceType); + + FilterExpression filter = ParseFilter(); + + _resourceContextInScope = outerScopeBackup; + return filter; } protected QueryExpression ParseCountOrField(FieldChainRequirements chainRequirements) diff --git a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/QueryableBuilder.cs b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/QueryableBuilder.cs index c85c223364..c49993e25c 100644 --- a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/QueryableBuilder.cs +++ b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/QueryableBuilder.cs @@ -93,7 +93,7 @@ protected virtual Expression ApplyFilter(Expression source, FilterExpression fil { using LambdaScope lambdaScope = _lambdaScopeFactory.CreateScope(_elementType); - var builder = new WhereClauseBuilder(source, lambdaScope, _extensionType); + var builder = new WhereClauseBuilder(source, lambdaScope, _extensionType, _nameFactory); return builder.ApplyWhere(filter); } diff --git a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/WhereClauseBuilder.cs b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/WhereClauseBuilder.cs index 823d532639..53fadd74da 100644 --- a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/WhereClauseBuilder.cs +++ b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/WhereClauseBuilder.cs @@ -23,27 +23,35 @@ public class WhereClauseBuilder : QueryClauseBuilder private readonly Expression _source; private readonly Type _extensionType; + private readonly LambdaParameterNameFactory _nameFactory; - public WhereClauseBuilder(Expression source, LambdaScope lambdaScope, Type extensionType) + public WhereClauseBuilder(Expression source, LambdaScope lambdaScope, Type extensionType, LambdaParameterNameFactory nameFactory) : base(lambdaScope) { ArgumentGuard.NotNull(source, nameof(source)); ArgumentGuard.NotNull(extensionType, nameof(extensionType)); + ArgumentGuard.NotNull(nameFactory, nameof(nameFactory)); _source = source; _extensionType = extensionType; + _nameFactory = nameFactory; } public Expression ApplyWhere(FilterExpression filter) { ArgumentGuard.NotNull(filter, nameof(filter)); - Expression body = Visit(filter, null); - LambdaExpression lambda = Expression.Lambda(body, LambdaScope.Parameter); + LambdaExpression lambda = GetPredicateLambda(filter); return WhereExtensionMethodCall(lambda); } + private LambdaExpression GetPredicateLambda(FilterExpression filter) + { + Expression body = Visit(filter, null); + return Expression.Lambda(body, LambdaScope.Parameter); + } + private Expression WhereExtensionMethodCall(LambdaExpression predicate) { return Expression.Call(_extensionType, "Where", LambdaScope.Parameter.Type.AsArray(), _source, predicate); @@ -60,11 +68,28 @@ public override Expression VisitCollectionNotEmpty(CollectionNotEmptyExpression throw new InvalidOperationException("Expression must be a collection."); } - return AnyExtensionMethodCall(elementType, property); + Expression predicate = null; + + if (expression.Filter != null) + { + var hasManyThrough = expression.TargetCollection.Fields.Last() as HasManyThroughAttribute; + var lambdaScopeFactory = new LambdaScopeFactory(_nameFactory, hasManyThrough); + using LambdaScope lambdaScope = lambdaScopeFactory.CreateScope(elementType); + + var builder = new WhereClauseBuilder(property, lambdaScope, typeof(Enumerable), _nameFactory); + predicate = builder.GetPredicateLambda(expression.Filter); + } + + return AnyExtensionMethodCall(elementType, property, predicate); } - private static MethodCallExpression AnyExtensionMethodCall(Type elementType, Expression source) + private static MethodCallExpression AnyExtensionMethodCall(Type elementType, Expression source, Expression predicate) { + if (predicate != null) + { + return Expression.Call(typeof(Enumerable), "Any", elementType.AsArray(), source, predicate); + } + return Expression.Call(typeof(Enumerable), "Any", elementType.AsArray(), source); } @@ -276,6 +301,8 @@ protected override MemberExpression CreatePropertyExpressionForFieldChain(IReadO private static string GetPropertyName(ResourceFieldAttribute field) { + // TODO: Is this still true when using has() with a filter? + // In case of a HasManyThrough access (from count() or has() function), we only need to look at the number of entries in the join table. return field is HasManyThroughAttribute hasManyThrough ? hasManyThrough.ThroughProperty.Name : field.Property.Name; } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/QueryStrings/Filtering/FilterDepthTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/QueryStrings/Filtering/FilterDepthTests.cs index 08eacbcde1..05ac775540 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/QueryStrings/Filtering/FilterDepthTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/QueryStrings/Filtering/FilterDepthTests.cs @@ -202,6 +202,35 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.ManyData[0].Id.Should().Be(blogs[1].StringId); } + [Fact] + public async Task Can_filter_on_HasMany_relationship_with_nested_condition() + { + // Arrange + List blogs = _fakers.Blog.Generate(2); + blogs[0].Posts = _fakers.BlogPost.Generate(1); + blogs[1].Posts = _fakers.BlogPost.Generate(1); + blogs[1].Posts[0].Comments = _fakers.Comment.Generate(1).ToHashSet(); + blogs[1].Posts[0].Comments.ElementAt(0).Text = "ABC"; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync(); + dbContext.Blogs.AddRange(blogs); + await dbContext.SaveChangesAsync(); + }); + + const string route = "/blogs?filter=has(posts,has(comments,startsWith(text,'A')))"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(1); + responseDocument.ManyData[0].Id.Should().Be(blogs[1].StringId); + } + [Fact] public async Task Can_filter_on_HasManyThrough_relationship() { @@ -235,6 +264,44 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.ManyData[0].Id.Should().Be(posts[1].StringId); } + [Fact] + public async Task Can_filter_on_HasManyThrough_relationship_with_nested_condition() + { + // Arrange + List blogs = _fakers.Blog.Generate(2); + blogs[0].Posts = _fakers.BlogPost.Generate(1); + blogs[1].Posts = _fakers.BlogPost.Generate(1); + + blogs[1].Posts[0].BlogPostLabels = new HashSet + { + new BlogPostLabel + { + Label = new Label + { + Color = LabelColor.Green + } + } + }; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync(); + dbContext.Blogs.AddRange(blogs); + await dbContext.SaveChangesAsync(); + }); + + const string route = "/blogs?filter=has(posts,has(labels,equals(color,'Green')))"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(1); + responseDocument.ManyData[0].Id.Should().Be(blogs[1].StringId); + } + [Fact] public async Task Can_filter_in_scope_of_HasMany_relationship() { diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/QueryStrings/Filtering/FilterOperatorTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/QueryStrings/Filtering/FilterOperatorTests.cs index a0979ff162..0a4f6d5283 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/QueryStrings/Filtering/FilterOperatorTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/QueryStrings/Filtering/FilterOperatorTests.cs @@ -489,6 +489,53 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.ManyData[0].Id.Should().Be(resource.StringId); } + [Fact] + public async Task Can_filter_on_has_with_nested_condition() + { + // Arrange + var resources = new List + { + new FilterableResource + { + Children = new List + { + new FilterableResource + { + SomeBoolean = false + } + } + }, + new FilterableResource + { + Children = new List + { + new FilterableResource + { + SomeBoolean = true + } + } + } + }; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync(); + dbContext.FilterableResources.AddRange(resources); + await dbContext.SaveChangesAsync(); + }); + + const string route = "/filterableResources?filter=has(children,equals(someBoolean,'true'))"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(1); + responseDocument.ManyData[0].Id.Should().Be(resources[1].StringId); + } + [Fact] public async Task Can_filter_on_count() { diff --git a/test/JsonApiDotNetCoreExampleTests/UnitTests/QueryStringParameters/FilterParseTests.cs b/test/JsonApiDotNetCoreExampleTests/UnitTests/QueryStringParameters/FilterParseTests.cs index edaa57e3ee..d4df940afa 100644 --- a/test/JsonApiDotNetCoreExampleTests/UnitTests/QueryStringParameters/FilterParseTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/UnitTests/QueryStringParameters/FilterParseTests.cs @@ -78,6 +78,7 @@ public void Reader_Is_Enabled(StandardQueryStringParameters parametersDisabled, [InlineData("filter", "equals(null", "Field 'null' does not exist on resource 'blogs'.")] [InlineData("filter", "equals(title,(", "Count function, value between quotes, null or field name expected.")] [InlineData("filter", "equals(has(posts),'true')", "Field 'has' does not exist on resource 'blogs'.")] + [InlineData("filter", "has(posts,", "Filter function expected.")] [InlineData("filter", "contains)", "( expected.")] [InlineData("filter", "contains(title,'a','b')", ") expected.")] [InlineData("filter", "contains(title,null)", "Value between quotes expected.")] @@ -125,14 +126,15 @@ public void Reader_Read_Fails(string parameterName, string parameterValue, strin [InlineData("filter[posts.comments]", "greaterThan(createdAt,'2000-01-01')", "posts.comments", "greaterThan(createdAt,'2000-01-01')")] [InlineData("filter[posts.comments]", "greaterOrEqual(createdAt,'2000-01-01')", "posts.comments", "greaterOrEqual(createdAt,'2000-01-01')")] [InlineData("filter", "has(posts)", null, "has(posts)")] + [InlineData("filter", "has(posts,not(equals(url,null)))", null, "has(posts,not(equals(url,null)))")] [InlineData("filter", "contains(title,'this')", null, "contains(title,'this')")] [InlineData("filter", "startsWith(title,'this')", null, "startsWith(title,'this')")] [InlineData("filter", "endsWith(title,'this')", null, "endsWith(title,'this')")] [InlineData("filter", "any(title,'this','that','there')", null, "any(title,'this','that','there')")] [InlineData("filter", "and(contains(title,'sales'),contains(title,'marketing'),contains(title,'advertising'))", null, "and(contains(title,'sales'),contains(title,'marketing'),contains(title,'advertising'))")] - [InlineData("filter[posts]", "or(and(not(equals(author.userName,null)),not(equals(author.displayName,null))),not(has(comments)))", "posts", - "or(and(not(equals(author.userName,null)),not(equals(author.displayName,null))),not(has(comments)))")] + [InlineData("filter[posts]", "or(and(not(equals(author.userName,null)),not(equals(author.displayName,null))),not(has(comments,startsWith(text,'A'))))", + "posts", "or(and(not(equals(author.userName,null)),not(equals(author.displayName,null))),not(has(comments,startsWith(text,'A'))))")] public void Reader_Read_Succeeds(string parameterName, string parameterValue, string scopeExpected, string valueExpected) { // Act From d8fb8c3a2bf363debb8199a1896a4edacd7eca9e Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Sun, 2 May 2021 22:47:23 +0200 Subject: [PATCH 2/5] Added filter parse test for special characters in literal text --- .../UnitTests/QueryStringParameters/FilterParseTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/JsonApiDotNetCoreExampleTests/UnitTests/QueryStringParameters/FilterParseTests.cs b/test/JsonApiDotNetCoreExampleTests/UnitTests/QueryStringParameters/FilterParseTests.cs index d4df940afa..2ebd5a05fd 100644 --- a/test/JsonApiDotNetCoreExampleTests/UnitTests/QueryStringParameters/FilterParseTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/UnitTests/QueryStringParameters/FilterParseTests.cs @@ -113,6 +113,7 @@ public void Reader_Read_Fails(string parameterName, string parameterValue, strin [Theory] [InlineData("filter", "equals(title,'Brian O''Quote')", null, "equals(title,'Brian O''Quote')")] + [InlineData("filter", "equals(title,'!@#$%^&*()-_=+\"''[]{}<>()/|\\:;.,`~')", null, "equals(title,'!@#$%^&*()-_=+\"''[]{}<>()/|\\:;.,`~')")] [InlineData("filter", "equals(title,'')", null, "equals(title,'')")] [InlineData("filter[posts]", "equals(caption,'this, that & more')", "posts", "equals(caption,'this, that & more')")] [InlineData("filter[owner.posts]", "equals(caption,'some')", "owner.posts", "equals(caption,'some')")] From 36b12fa0affe3ac17fb2634bfa0cac704f256d76 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Sun, 2 May 2021 22:53:40 +0200 Subject: [PATCH 3/5] Added filter grammar to documentation --- docs/usage/reading/filtering.md | 61 ++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/docs/usage/reading/filtering.md b/docs/usage/reading/filtering.md index 5d7cf8692f..bc24503db0 100644 --- a/docs/usage/reading/filtering.md +++ b/docs/usage/reading/filtering.md @@ -125,7 +125,7 @@ If you want to use the new filter notation in that case, prefix the parameter va GET /articles?filter[caption]=tech&filter=expr:equals(caption,'cooking')) HTTP/1.1 ``` -## Custom Filters +# Custom Filters There are multiple ways you can add custom filters: @@ -134,3 +134,62 @@ There are multiple ways you can add custom filters: 3. Add an implementation of `IQueryConstraintProvider` to supply additional `FilterExpression`s, which are combined with existing filters using AND operator 4. Override `EntityFrameworkCoreRepository.ApplyQueryLayer` to adapt the `IQueryable` expression just before execution 5. Take a deep dive and plug into reader/parser/tokenizer/visitor/builder for adding additional general-purpose filter operators + +# Filter syntax + +For reference, we provide the EBNF grammar for filter expressions below (in [ANTLR4](https://github.com/antlr/antlr4) style): + +```ebnf +grammar Filter; + +filterExpression: + notExpression + | logicalExpression + | comparisonExpression + | matchTextExpression + | anyExpression + | hasExpression; + +notExpression: + 'not' LPAREN filterExpression RPAREN; + +logicalExpression: + ( 'and' | 'or' ) LPAREN filterExpression ( COMMA filterExpression )* RPAREN; + +comparisonExpression: + ( 'equals' | 'greaterThan' | 'greaterOrEqual' | 'lessThan' | 'lessOrEqual' ) LPAREN ( + countExpression | fieldChain + ) COMMA ( + countExpression | literalConstant | 'null' | fieldChain + ) RPAREN; + +matchTextExpression: + ( 'contains' | 'startsWith' | 'endsWith' ) LPAREN fieldChain COMMA literalConstant RPAREN; + +anyExpression: + 'any' LPAREN fieldChain COMMA literalConstant ( COMMA literalConstant )+ RPAREN; + +hasExpression: + 'has' LPAREN fieldChain ( COMMA filterExpression )? RPAREN; + +countExpression: + 'count' LPAREN fieldChain RPAREN; + +fieldChain: + FIELD ( '.' FIELD )*; + +literalConstant: + ESCAPED_TEXT; + +LPAREN: '('; +RPAREN: ')'; +COMMA: ','; + +fragment OUTER_FIELD_CHARACTER: [A-Za-z0-9]; +fragment INNER_FIELD_CHARACTER: [A-Za-z0-9_-]; +FIELD: OUTER_FIELD_CHARACTER ( INNER_FIELD_CHARACTER* OUTER_FIELD_CHARACTER )?; + +ESCAPED_TEXT: '\'' ( ~['] | '\'\'' )* '\'' ; + +LINE_BREAKS: [\r\n]+ -> skip; +``` From ad2625d6332f2f26a13842bd57442c54be1d817b Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Sun, 2 May 2021 23:07:13 +0200 Subject: [PATCH 4/5] Narrowed some types in the filter parse tree that were too wide. The parsers already constrained the input, so API users should not observe any effects of this change. --- .../Queries/Expressions/FilterExpression.cs | 2 +- .../Queries/Expressions/FunctionExpression.cs | 2 +- .../Queries/Expressions/LogicalExpression.cs | 4 ++-- src/JsonApiDotNetCore/Queries/Expressions/NotExpression.cs | 4 ++-- .../Queries/Expressions/QueryExpressionRewriter.cs | 6 ++---- .../Queries/Internal/Parsing/FilterParser.cs | 2 +- .../IntegrationTests/CompositeKeys/CarExpressionRewriter.cs | 6 +++--- 7 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/JsonApiDotNetCore/Queries/Expressions/FilterExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/FilterExpression.cs index bdae74f21d..1ed0ed1de0 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/FilterExpression.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/FilterExpression.cs @@ -1,7 +1,7 @@ namespace JsonApiDotNetCore.Queries.Expressions { /// - /// Represents the base type for filter functions. + /// Represents the base type for filter functions that return a boolean value. /// public abstract class FilterExpression : FunctionExpression { diff --git a/src/JsonApiDotNetCore/Queries/Expressions/FunctionExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/FunctionExpression.cs index 4af5f1f4eb..6d7990a16a 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/FunctionExpression.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/FunctionExpression.cs @@ -1,7 +1,7 @@ namespace JsonApiDotNetCore.Queries.Expressions { /// - /// Represents the base type for functions. + /// Represents the base type for functions that return a value. /// public abstract class FunctionExpression : QueryExpression { diff --git a/src/JsonApiDotNetCore/Queries/Expressions/LogicalExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/LogicalExpression.cs index ee368235ce..ab991aad9a 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/LogicalExpression.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/LogicalExpression.cs @@ -14,9 +14,9 @@ namespace JsonApiDotNetCore.Queries.Expressions public class LogicalExpression : FilterExpression { public LogicalOperator Operator { get; } - public IReadOnlyCollection Terms { get; } + public IReadOnlyCollection Terms { get; } - public LogicalExpression(LogicalOperator @operator, IReadOnlyCollection terms) + public LogicalExpression(LogicalOperator @operator, IReadOnlyCollection terms) { ArgumentGuard.NotNull(terms, nameof(terms)); diff --git a/src/JsonApiDotNetCore/Queries/Expressions/NotExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/NotExpression.cs index da790bcfbe..3ac97b46bc 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/NotExpression.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/NotExpression.cs @@ -9,9 +9,9 @@ namespace JsonApiDotNetCore.Queries.Expressions [PublicAPI] public class NotExpression : FilterExpression { - public QueryExpression Child { get; } + public FilterExpression Child { get; } - public NotExpression(QueryExpression child) + public NotExpression(FilterExpression child) { ArgumentGuard.NotNull(child, nameof(child)); diff --git a/src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs b/src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs index c650992fe2..5def9a208a 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs @@ -54,7 +54,7 @@ public override QueryExpression VisitLogical(LogicalExpression expression, TArgu { if (expression != null) { - IReadOnlyCollection newTerms = VisitSequence(expression.Terms, argument); + IReadOnlyCollection newTerms = VisitSequence(expression.Terms, argument); if (newTerms.Count == 1) { @@ -75,9 +75,7 @@ public override QueryExpression VisitNot(NotExpression expression, TArgument arg { if (expression != null) { - QueryExpression newChild = Visit(expression.Child, argument); - - if (newChild != null) + if (Visit(expression.Child, argument) is FilterExpression newChild) { var newExpression = new NotExpression(newChild); return newExpression.Equals(expression) ? expression : newExpression; diff --git a/src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs b/src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs index 7f45adceb8..d4a7f04acd 100644 --- a/src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs +++ b/src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs @@ -106,7 +106,7 @@ protected LogicalExpression ParseLogical(string operatorName) EatText(operatorName); EatSingleCharacterToken(TokenKind.OpenParen); - var terms = new List(); + var terms = new List(); FilterExpression term = ParseFilter(); terms.Add(term); diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CarExpressionRewriter.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CarExpressionRewriter.cs index ee0ae50c6f..f25b890546 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CarExpressionRewriter.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CarExpressionRewriter.cs @@ -83,7 +83,7 @@ private static bool IsCarId(PropertyInfo property) private QueryExpression RewriteFilterOnCarStringIds(ResourceFieldChainExpression existingCarIdChain, IEnumerable carStringIds) { - var outerTerms = new List(); + var outerTerms = new List(); foreach (string carStringId in carStringIds) { @@ -92,14 +92,14 @@ private QueryExpression RewriteFilterOnCarStringIds(ResourceFieldChainExpression StringId = carStringId }; - QueryExpression keyComparison = CreateEqualityComparisonOnCompositeKey(existingCarIdChain, tempCar.RegionId, tempCar.LicensePlate); + FilterExpression keyComparison = CreateEqualityComparisonOnCompositeKey(existingCarIdChain, tempCar.RegionId, tempCar.LicensePlate); outerTerms.Add(keyComparison); } return outerTerms.Count == 1 ? outerTerms[0] : new LogicalExpression(LogicalOperator.Or, outerTerms); } - private QueryExpression CreateEqualityComparisonOnCompositeKey(ResourceFieldChainExpression existingCarIdChain, long regionIdValue, + private FilterExpression CreateEqualityComparisonOnCompositeKey(ResourceFieldChainExpression existingCarIdChain, long regionIdValue, string licensePlateValue) { ResourceFieldChainExpression regionIdChain = ReplaceLastAttributeInChain(existingCarIdChain, _regionIdAttribute); From f13af098dee3d850d8e3a817225f39db63457aa1 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Mon, 3 May 2021 10:11:20 +0200 Subject: [PATCH 5/5] Resolved TODO --- .../Queries/Internal/QueryableBuilding/WhereClauseBuilder.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/WhereClauseBuilder.cs b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/WhereClauseBuilder.cs index 53fadd74da..191e911cf9 100644 --- a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/WhereClauseBuilder.cs +++ b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/WhereClauseBuilder.cs @@ -301,9 +301,6 @@ protected override MemberExpression CreatePropertyExpressionForFieldChain(IReadO private static string GetPropertyName(ResourceFieldAttribute field) { - // TODO: Is this still true when using has() with a filter? - - // In case of a HasManyThrough access (from count() or has() function), we only need to look at the number of entries in the join table. return field is HasManyThroughAttribute hasManyThrough ? hasManyThrough.ThroughProperty.Name : field.Property.Name; } }