From 220281ca149bf00467ba765e051cbea7c8afda0a Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Wed, 14 Apr 2021 13:57:04 +0300 Subject: [PATCH 01/19] Add CountSegmentToken --- .../SyntacticAst/CountSegmentToken.cs | 122 ++++++++++++++++++ .../UriParser/SyntacticAst/QueryTokenKind.cs | 7 +- .../Visitors/ISyntacticTreeVisitor.cs | 7 + 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 src/Microsoft.OData.Core/UriParser/SyntacticAst/CountSegmentToken.cs diff --git a/src/Microsoft.OData.Core/UriParser/SyntacticAst/CountSegmentToken.cs b/src/Microsoft.OData.Core/UriParser/SyntacticAst/CountSegmentToken.cs new file mode 100644 index 0000000000..e99f4f19d3 --- /dev/null +++ b/src/Microsoft.OData.Core/UriParser/SyntacticAst/CountSegmentToken.cs @@ -0,0 +1,122 @@ +//--------------------------------------------------------------------- +// +// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information. +// +//--------------------------------------------------------------------- + +#if ODATA_CLIENT +namespace Microsoft.OData.Client.ALinq.UriParser +#else +namespace Microsoft.OData.UriParser +#endif +{ + /// + /// Lexical token representing the $count segment in a path. + /// + public sealed class CountSegmentToken : PathToken + { + /// + /// The name of this token, which in this case is always "$count". + /// + private readonly string identifier; + + /// + /// The instance to count on. + /// + private QueryToken nextToken; + + /// + /// The token representing $filter. + /// + private QueryToken filterOption; + + /// + /// The token representing $search. + /// + private QueryToken searchOption; + + /// + /// Create a CountSegmentToken given the Identifier and the NextToken. + /// + /// The name of this token, which in this case is always "$count". + /// The instance to count on. + public CountSegmentToken(string identifier, QueryToken nextToken) + :this(identifier, nextToken, null, null) + { + } + + /// + /// Create a CountSegmentToken given the Identifier, NextToken, FilterOption (if any) and SearchOption (if any). + /// + /// The name of this token, which in this case is always "$count". + /// The instance to count on. + /// The representing $filter. + /// The representing $search. + public CountSegmentToken(string identifier, QueryToken nextToken, QueryToken filterOption, QueryToken searchOption) + { + ExceptionUtils.CheckArgumentStringNotNullOrEmpty(identifier, "identifier"); + ExceptionUtils.CheckArgumentNotNull(nextToken, "nextToken"); + + this.identifier = identifier; + this.nextToken = nextToken; + this.filterOption = filterOption; + this.searchOption = searchOption; + } + + /// + /// The kind of the query token. + /// + public override QueryTokenKind Kind + { + get { return QueryTokenKind.CountSegment; } + } + + /// + /// The name of this token, which in this case is always "$count". + /// + public override string Identifier + { + get { return this.identifier; } + } + + /// + /// The instance to count on. + /// + public override QueryToken NextToken + { + get { return this.nextToken; } + set { this.nextToken = value; } + } + + /// + /// The representing $filter. + /// If this is null, then the $count segment does not have a filter query option. + /// + public QueryToken FilterOption + { + get { return this.filterOption; } + set { this.filterOption = value; } + } + + /// + /// The representing $search. + /// If this is null, then the $count segment does not have a search query option. + /// + public QueryToken SearchOption + { + get { return this.searchOption; } + set { this.searchOption = value; } + } + + /// + /// Accept a to walk a tree of s. + /// + /// Type that the visitor will return after visiting this token. + /// An implementation of the visitor interface. + /// An object whose type is determined by the type parameter of the visitor. + public override T Accept(ISyntacticTreeVisitor visitor) + { + return visitor.Visit(this); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.OData.Core/UriParser/SyntacticAst/QueryTokenKind.cs b/src/Microsoft.OData.Core/UriParser/SyntacticAst/QueryTokenKind.cs index 97fedd3b21..56d1b2bc6b 100644 --- a/src/Microsoft.OData.Core/UriParser/SyntacticAst/QueryTokenKind.cs +++ b/src/Microsoft.OData.Core/UriParser/SyntacticAst/QueryTokenKind.cs @@ -154,6 +154,11 @@ public enum QueryTokenKind /// /// SelectTerm Token /// - SelectTerm = 31 + SelectTerm = 31, + + /// + /// $count segment + /// + CountSegment = 32 } } \ No newline at end of file diff --git a/src/Microsoft.OData.Core/UriParser/Visitors/ISyntacticTreeVisitor.cs b/src/Microsoft.OData.Core/UriParser/Visitors/ISyntacticTreeVisitor.cs index 596bac38d5..29ef90e9b3 100644 --- a/src/Microsoft.OData.Core/UriParser/Visitors/ISyntacticTreeVisitor.cs +++ b/src/Microsoft.OData.Core/UriParser/Visitors/ISyntacticTreeVisitor.cs @@ -39,6 +39,13 @@ public interface ISyntacticTreeVisitor /// A BinaryOperatorNode that's bound to this token T Visit(BinaryOperatorToken tokenIn); + /// + /// Visits a CountSegmentToken + /// + /// The Count segment token to visit. + /// A CountNode that's bound to this token + T Visit(CountSegmentToken tokenIn); + /// /// Visits an InToken /// From 0adb2c8955993a9e6c1f79782999714b269a8024 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Thu, 15 Apr 2021 10:22:22 +0300 Subject: [PATCH 02/19] Add implementation and reference to CountSegment token --- .../Microsoft.OData.Client.csproj | 1 + .../UriParser/Visitors/SyntacticTreeVisitor.cs | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/src/Microsoft.OData.Client/Microsoft.OData.Client.csproj b/src/Microsoft.OData.Client/Microsoft.OData.Client.csproj index c09d3055a1..047d1a3d8e 100644 --- a/src/Microsoft.OData.Client/Microsoft.OData.Client.csproj +++ b/src/Microsoft.OData.Client/Microsoft.OData.Client.csproj @@ -96,6 +96,7 @@ + diff --git a/src/Microsoft.OData.Core/UriParser/Visitors/SyntacticTreeVisitor.cs b/src/Microsoft.OData.Core/UriParser/Visitors/SyntacticTreeVisitor.cs index 0cd583b895..98215f4cb7 100644 --- a/src/Microsoft.OData.Core/UriParser/Visitors/SyntacticTreeVisitor.cs +++ b/src/Microsoft.OData.Core/UriParser/Visitors/SyntacticTreeVisitor.cs @@ -49,6 +49,16 @@ public virtual T Visit(BinaryOperatorToken tokenIn) throw new NotImplementedException(); } + /// + /// Visits a CountSegmentToken + /// + /// The Count segment token to visit. + /// A CountNode that's bound to this token + public virtual T Visit(CountSegmentToken tokenIn) + { + throw new NotImplementedException(); + } + /// /// Visits an InToken /// From 611d93b9ddfe7e992569eb232591749c7e7fe0cf Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Mon, 19 Apr 2021 10:27:12 +0300 Subject: [PATCH 03/19] Add CountSegmentParser --- .../UriParser/Parsers/CountSegmentParser.cs | 76 +++++++++++++++++++ .../UriParser/Parsers/IdentifierTokenizer.cs | 8 ++ 2 files changed, 84 insertions(+) create mode 100644 src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs new file mode 100644 index 0000000000..157c62f433 --- /dev/null +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -0,0 +1,76 @@ +//--------------------------------------------------------------------- +// +// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information. +// +//--------------------------------------------------------------------- + +namespace Microsoft.OData.UriParser +{ + using System; + using System.Collections.Generic; + using System.Diagnostics; + using System.Linq; + using ODataErrorStrings = Microsoft.OData.Strings; + + /// + /// Syntactic parser for the $count segment. + /// + internal sealed class CountSegmentParser + { + /// + /// Reference to the lexer. + /// + private readonly ExpressionLexer lexer; + + /// + /// Method used to parse arguments. + /// + private readonly UriQueryExpressionParser parser; + + /// + /// If set to true, catches any ODataException thrown while trying to parse $count segment query options. + /// + private readonly bool restoreStateIfFail; + + /// + /// Create a new CountSegmentParser. + /// + /// Lexer positioned at a function identifier. + /// The UriQueryExpressionParser. + public CountSegmentParser(ExpressionLexer lexer, UriQueryExpressionParser parser) + : this(lexer, parser, false /* restoreStateIfFail */) + { + } + + /// + /// Create a new CountSegmentParser. + /// + /// Lexer positioned at a function identifier. + /// The UriQueryExpressionParser. + /// If set to true, catches any ODataException thrown while trying to parse function arguments. + public CountSegmentParser(ExpressionLexer lexer, UriQueryExpressionParser parser, bool restoreStateIfFail) + { + ExceptionUtils.CheckArgumentNotNull(lexer, "lexer"); + ExceptionUtils.CheckArgumentNotNull(parser, "parser"); + this.lexer = lexer; + this.parser = parser; + this.restoreStateIfFail = restoreStateIfFail; + } + + /// + /// Reference to the underlying UriQueryExpressionParser. + /// + public UriQueryExpressionParser UriQueryExpressionParser + { + get { return this.parser; } + } + + /// + /// Reference to the lexer. + /// + public ExpressionLexer Lexer + { + get { return this.lexer; } + } + } +} diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs index 193fded08a..a148c31565 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs @@ -10,6 +10,7 @@ namespace Microsoft.OData.UriParser using System; using System.Collections.Generic; + using System.Globalization; using ODataErrorStrings = Microsoft.OData.Strings; #endregion Namespaces @@ -60,6 +61,13 @@ public QueryToken ParseIdentifier(QueryToken parent) // An open paren here would indicate calling a method in regular C# syntax. // TODO: Make this more generalized to work with any kind of function. bool identifierIsFunction = this.lexer.ExpandIdentifierAsFunction(); + string textWithinParenthesis = this.lexer.AdvanceThroughBalancedParentheticalExpression(); + + if (textWithinParenthesis.StartsWith(UriQueryConstants.FilterQueryOption, StringComparison.InvariantCulture) || + textWithinParenthesis.StartsWith(UriQueryConstants.FilterQueryOption, StringComparison.InvariantCulture)) + { + + } QueryToken functionCallToken; if (identifierIsFunction && this.functionCallParser.TryParseIdentifierAsFunction(parent, out functionCallToken)) { From 96db6d7c1311953bbd717b13e6363e980b80897f Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Fri, 23 Apr 2021 15:32:37 +0300 Subject: [PATCH 04/19] Create the CountSegmentToken --- .../UriParser/Parsers/CountSegmentParser.cs | 89 +++++++++++++++---- .../UriParser/Parsers/IdentifierTokenizer.cs | 34 +++++-- .../Parsers/UriQueryExpressionParser.cs | 14 ++- .../FilterAndOrderByFunctionalTests.cs | 10 +++ 4 files changed, 121 insertions(+), 26 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index 157c62f433..c9eb5f780b 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -27,34 +27,17 @@ internal sealed class CountSegmentParser /// private readonly UriQueryExpressionParser parser; - /// - /// If set to true, catches any ODataException thrown while trying to parse $count segment query options. - /// - private readonly bool restoreStateIfFail; - /// /// Create a new CountSegmentParser. /// - /// Lexer positioned at a function identifier. + /// Lexer positioned at a $count identifier. /// The UriQueryExpressionParser. public CountSegmentParser(ExpressionLexer lexer, UriQueryExpressionParser parser) - : this(lexer, parser, false /* restoreStateIfFail */) - { - } - - /// - /// Create a new CountSegmentParser. - /// - /// Lexer positioned at a function identifier. - /// The UriQueryExpressionParser. - /// If set to true, catches any ODataException thrown while trying to parse function arguments. - public CountSegmentParser(ExpressionLexer lexer, UriQueryExpressionParser parser, bool restoreStateIfFail) { ExceptionUtils.CheckArgumentNotNull(lexer, "lexer"); ExceptionUtils.CheckArgumentNotNull(parser, "parser"); this.lexer = lexer; this.parser = parser; - this.restoreStateIfFail = restoreStateIfFail; } /// @@ -72,5 +55,75 @@ public ExpressionLexer Lexer { get { return this.lexer; } } + + /// + /// Create a . + /// + /// The name of this token, which in this case is always "$count".The name of this token, which in this case is always "$count". + /// The instance to count on. + public CountSegmentToken CreateCountSegmentToken(string identifier, QueryToken countedInstance) + { + QueryToken filterToken = null; + QueryToken searchToken = null; + string textWithinParenthesis = null; + + // We need to maintain the recursionDepth of the outer $filter query since calling + // ParseFilter or ParseSearch below will reset it to 0. + int outerRecursiveDepth = this.UriQueryExpressionParser.recursionDepth; + + if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.OpenParen) + { + return new CountSegmentToken(identifier, countedInstance); + } + else + { + textWithinParenthesis = this.lexer.AdvanceThroughExpandOption(); + } + + if (textWithinParenthesis.StartsWith(ExpressionConstants.QueryOptionFilter, StringComparison.OrdinalIgnoreCase)) + { + string filterQuery = TryGetQueryOption(ExpressionConstants.QueryOptionFilter, textWithinParenthesis); + filterToken = filterQuery == null ? null: this.UriQueryExpressionParser.ParseFilter(filterQuery); + } + else if (textWithinParenthesis.StartsWith(ExpressionConstants.QueryOptionSearch, StringComparison.OrdinalIgnoreCase)) + { + string searchQuery = TryGetQueryOption(ExpressionConstants.QueryOptionSearch, textWithinParenthesis); + searchToken = searchQuery == null ? null : this.UriQueryExpressionParser.ParseSearch(searchQuery); + } + else + { + // Error + } + + if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.CloseParen) + { + // Error + } + + this.lexer.NextToken(); + this.UriQueryExpressionParser.recursionDepth = outerRecursiveDepth; + return new CountSegmentToken(identifier, countedInstance, filterToken, searchToken); + } + + /// + /// + /// + /// + /// + /// + private static string TryGetQueryOption(string queryOptionName, string query) + { + if (queryOptionName == null) + { + return null; + } + + // Concat $filter or $search with an = symbol to get $filter= or $search= + string queryOption = string.Concat(queryOptionName, ExpressionConstants.SymbolEqual); + + char[] trimmingChars = queryOption.ToCharArray(); + + return query.TrimStart(trimmingChars); + } } } diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs index a148c31565..135d8ee01f 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs @@ -35,18 +35,35 @@ internal sealed class IdentifierTokenizer /// private readonly IFunctionCallParser functionCallParser; + /// + /// Parser which consumes the query expressions. + /// + private readonly UriQueryExpressionParser uriQueryExpressionParser; + /// /// Parse an Identifier into the right QueryToken /// /// parameters passed in to the UriQueryExpressionParser /// Object to use to handle parsing function calls. public IdentifierTokenizer(HashSet parameters, IFunctionCallParser functionCallParser) + :this(parameters, functionCallParser, null) + { + } + + /// + /// Parse an Identifier into the right QueryToken + /// + /// parameters passed in to the UriQueryExpressionParser + /// Object to use to handle parsing function calls. + /// Instance of the UriQueryExpressionParser. + public IdentifierTokenizer(HashSet parameters, IFunctionCallParser functionCallParser, UriQueryExpressionParser uriQueryExpressionParser) { ExceptionUtils.CheckArgumentNotNull(parameters, "parameters"); ExceptionUtils.CheckArgumentNotNull(functionCallParser, "functionCallParser"); this.lexer = functionCallParser.Lexer; this.parameters = parameters; this.functionCallParser = functionCallParser; + this.uriQueryExpressionParser = uriQueryExpressionParser; } /// @@ -58,16 +75,21 @@ public QueryToken ParseIdentifier(QueryToken parent) { this.lexer.ValidateToken(ExpressionTokenKind.Identifier); - // An open paren here would indicate calling a method in regular C# syntax. - // TODO: Make this more generalized to work with any kind of function. - bool identifierIsFunction = this.lexer.ExpandIdentifierAsFunction(); - string textWithinParenthesis = this.lexer.AdvanceThroughBalancedParentheticalExpression(); + string identifier = this.lexer.CurrentToken.GetIdentifier(); - if (textWithinParenthesis.StartsWith(UriQueryConstants.FilterQueryOption, StringComparison.InvariantCulture) || - textWithinParenthesis.StartsWith(UriQueryConstants.FilterQueryOption, StringComparison.InvariantCulture)) + // If identifier is $count, we need to return a CountSegmentToken. + if (identifier.Equals(UriQueryConstants.CountSegment, StringComparison.OrdinalIgnoreCase)) { + this.lexer.NextToken(); + CountSegmentParser countSegmentParser = new CountSegmentParser(this.lexer, this.uriQueryExpressionParser); + return countSegmentParser.CreateCountSegmentToken(identifier, parent); } + + // An open paren here would indicate calling a method in regular C# syntax. + // TODO: Make this more generalized to work with any kind of function. + bool identifierIsFunction = this.lexer.ExpandIdentifierAsFunction(); + QueryToken functionCallToken; if (identifierIsFunction && this.functionCallParser.TryParseIdentifierAsFunction(parent, out functionCallToken)) { diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index af332069a6..b2770142af 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -44,7 +44,7 @@ public sealed class UriQueryExpressionParser /// /// The current recursion depth. /// - private int recursionDepth; + internal int recursionDepth; /// /// The lexer being used for the parsing. @@ -134,6 +134,16 @@ public QueryToken ParseFilter(string filter) return this.ParseExpressionText(filter); } + /// + /// Parses the $search expression. + /// + /// The $search expression string to parse. + /// The lexical token representing the filter. + public QueryToken ParseSearch(string search) + { + return this.ParseExpressionText(search); + } + /// /// Parses a literal. /// @@ -1046,7 +1056,7 @@ private QueryToken ParsePrimary() } else { - IdentifierTokenizer identifierTokenizer = new IdentifierTokenizer(this.parameters, new FunctionCallParser(this.lexer, this, this.IsInAggregateExpression)); + IdentifierTokenizer identifierTokenizer = new IdentifierTokenizer(this.parameters, new FunctionCallParser(this.lexer, this, this.IsInAggregateExpression), this); expr = identifierTokenizer.ParseIdentifier(expr); } } diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index 32e8cfafb1..c54d44f359 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -400,6 +400,16 @@ public void ParseFilterWithEntityCollectionCount() Source.ShouldBeCollectionNavigationNode(HardCodedTestModel.GetPersonMyFriendsDogsProp()); } + [Fact] + public void ParseFilterWithEntityCollectionCountWithFilterOption() + { + var filterQueryNode = ParseFilter("MyFriendsDogs/$count($filter=Color eq 'Brown') gt 1", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + + filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.Equal). + Left.ShouldBeCountNode(). + Source.ShouldBeCollectionNavigationNode(HardCodedTestModel.GetPersonMyFriendsDogsProp()); + } + [Fact] public void CompareComplexWithNull() { From 454434e3ddf2bc0c2af09da1ec21e105ff6f5f8b Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Mon, 26 Apr 2021 10:08:58 +0300 Subject: [PATCH 05/19] Improve error handling --- src/Microsoft.OData.Core/Microsoft.OData.Core.cs | 2 ++ .../Microsoft.OData.Core.txt | 2 ++ .../Parameterized.Microsoft.OData.Core.cs | 16 ++++++++++++++++ .../UriParser/Parsers/CountSegmentParser.cs | 14 ++++++++++++-- .../UriParser/Parsers/IdentifierTokenizer.cs | 4 +++- .../Parsers/UriQueryExpressionParser.cs | 2 +- .../UriParser/FilterAndOrderByFunctionalTests.cs | 14 ++++++++++++++ 7 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs index d5d2e0dd79..2e9b84ee65 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs @@ -600,6 +600,8 @@ internal sealed class TextRes { internal const string UriQueryExpressionParser_OpenParenExpected = "UriQueryExpressionParser_OpenParenExpected"; internal const string UriQueryExpressionParser_CloseParenOrCommaExpected = "UriQueryExpressionParser_CloseParenOrCommaExpected"; internal const string UriQueryExpressionParser_CloseParenOrOperatorExpected = "UriQueryExpressionParser_CloseParenOrOperatorExpected"; + internal const string UriQueryExpressionParser_CloseParenExpected = "UriQueryExpressionParser_CloseParenExpected"; + internal const string UriQueryExpressionParser_IllegalQueryOptioninDollarCount = "UriQueryExpressionParser_IllegalQueryOptioninDollarCount"; internal const string UriQueryExpressionParser_CannotCreateStarTokenFromNonStar = "UriQueryExpressionParser_CannotCreateStarTokenFromNonStar"; internal const string UriQueryExpressionParser_RangeVariableAlreadyDeclared = "UriQueryExpressionParser_RangeVariableAlreadyDeclared"; internal const string UriQueryExpressionParser_AsExpected = "UriQueryExpressionParser_AsExpected"; diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt index 38fac3c22e..969feea802 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt @@ -692,6 +692,8 @@ UriQueryExpressionParser_ExpressionExpected=Expression expected at position {0} UriQueryExpressionParser_OpenParenExpected='(' expected at position {0} in '{1}'. UriQueryExpressionParser_CloseParenOrCommaExpected=')' or ',' expected at position {0} in '{1}'. UriQueryExpressionParser_CloseParenOrOperatorExpected=')' or operator expected at position {0} in '{1}'. +UriQueryExpressionParser_CloseParenExpected=')' expected at position {0} in '{1}'. +UriQueryExpressionParser_IllegalQueryOptioninDollarCount=Only $filter and $search query options are allowed within $count. UriQueryExpressionParser_CannotCreateStarTokenFromNonStar=Expecting a Star token but got: '{0}'. UriQueryExpressionParser_RangeVariableAlreadyDeclared=The range variable '{0}' has already been declared. UriQueryExpressionParser_AsExpected='as' expected at position {0} in '{1}'. diff --git a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs index e6816eb148..50fbb87747 100644 --- a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs @@ -5212,6 +5212,22 @@ internal static string UriQueryExpressionParser_CloseParenOrOperatorExpected(obj return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.UriQueryExpressionParser_CloseParenOrOperatorExpected, p0, p1); } + /// + /// A string like "')' expected at position {0} in '{1}'." + /// + internal static string UriQueryExpressionParser_CloseParenExpected(object p0, object p1) + { + return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.UriQueryExpressionParser_CloseParenExpected, p0, p1); + } + + /// + /// A string like "Only $filter and $search query options are allowed within $count." + /// + internal static string UriQueryExpressionParser_IllegalQueryOptioninDollarCount() + { + return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.UriQueryExpressionParser_IllegalQueryOptioninDollarCount); + } + /// /// A string like "Expecting a Star token but got: '{0}'." /// diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index c9eb5f780b..c22d3f4e26 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -80,6 +80,7 @@ public CountSegmentToken CreateCountSegmentToken(string identifier, QueryToken c textWithinParenthesis = this.lexer.AdvanceThroughExpandOption(); } + // TODO: check for NoDollarQueryOptions if (textWithinParenthesis.StartsWith(ExpressionConstants.QueryOptionFilter, StringComparison.OrdinalIgnoreCase)) { string filterQuery = TryGetQueryOption(ExpressionConstants.QueryOptionFilter, textWithinParenthesis); @@ -92,12 +93,13 @@ public CountSegmentToken CreateCountSegmentToken(string identifier, QueryToken c } else { - // Error + // Only a $filter and $search can be inside a $count() + throw ParseError(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); } if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.CloseParen) { - // Error + throw ParseError(ODataErrorStrings.UriQueryExpressionParser_CloseParenExpected(this.lexer.CurrentToken.Position, this.lexer.ExpressionText)); } this.lexer.NextToken(); @@ -125,5 +127,13 @@ private static string TryGetQueryOption(string queryOptionName, string query) return query.TrimStart(trimmingChars); } + + /// Creates an exception for a parse error. + /// Message text. + /// A new Exception. + private static Exception ParseError(string message) + { + return new ODataException(message); + } } } diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs index 135d8ee01f..d7d4039123 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs @@ -77,8 +77,10 @@ public QueryToken ParseIdentifier(QueryToken parent) string identifier = this.lexer.CurrentToken.GetIdentifier(); + bool enableCaseInsensitive = uriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier; + // If identifier is $count, we need to return a CountSegmentToken. - if (identifier.Equals(UriQueryConstants.CountSegment, StringComparison.OrdinalIgnoreCase)) + if (identifier.Equals(UriQueryConstants.CountSegment, enableCaseInsensitive ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal)) { this.lexer.NextToken(); diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index b2770142af..2fe2af870a 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -54,7 +54,7 @@ public sealed class UriQueryExpressionParser /// /// Whether to allow case insensitive for builtin identifier. /// - private bool enableCaseInsensitiveBuiltinIdentifier = false; + internal bool enableCaseInsensitiveBuiltinIdentifier = false; /// /// Tracks the depth of aggregate expression recursion. diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index c54d44f359..f679e834c7 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -410,6 +410,20 @@ public void ParseFilterWithEntityCollectionCountWithFilterOption() Source.ShouldBeCollectionNavigationNode(HardCodedTestModel.GetPersonMyFriendsDogsProp()); } + [Fact] + public void ParseFilterWithEntityCollectionCountWithUnbalanceParenthesisThrows() + { + Action parse = () => ParseFilter("MyFriendsDogs/$count($filter=Color eq 'Brown' gt 1", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + parse.Throws(ODataErrorStrings.UriQueryExpressionParser_CloseParenExpected(50, "MyFriendsDogs/$count($filter=Color eq 'Brown' gt 1")); + } + + [Fact] + public void ParseFilterWithEntityCollectionCountWithIllegalQueryOptionThrows() + { + Action parse = () => ParseFilter("MyFriendsDogs/$count($orderby=Color) gt 1", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + parse.Throws(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); + } + [Fact] public void CompareComplexWithNull() { From 070f498b9c4ee3dd4840d32e187a710f737397a7 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Mon, 26 Apr 2021 11:02:09 +0300 Subject: [PATCH 06/19] Null check --- .../UriParser/Parsers/IdentifierTokenizer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs index d7d4039123..7d66bd8872 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs @@ -77,7 +77,7 @@ public QueryToken ParseIdentifier(QueryToken parent) string identifier = this.lexer.CurrentToken.GetIdentifier(); - bool enableCaseInsensitive = uriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier; + bool enableCaseInsensitive = uriQueryExpressionParser != null ? uriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier : false; // If identifier is $count, we need to return a CountSegmentToken. if (identifier.Equals(UriQueryConstants.CountSegment, enableCaseInsensitive ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal)) From a0029e6a0900fa68d2fa9734f9840e6649da51a5 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Mon, 26 Apr 2021 11:43:49 +0300 Subject: [PATCH 07/19] Move CountSegmentParser logic --- .../UriParser/Parsers/IdentifierTokenizer.cs | 30 ------------------- .../Parsers/UriQueryExpressionParser.cs | 22 ++++++++++++-- 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs index 7d66bd8872..cf603821db 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs @@ -35,35 +35,18 @@ internal sealed class IdentifierTokenizer /// private readonly IFunctionCallParser functionCallParser; - /// - /// Parser which consumes the query expressions. - /// - private readonly UriQueryExpressionParser uriQueryExpressionParser; - /// /// Parse an Identifier into the right QueryToken /// /// parameters passed in to the UriQueryExpressionParser /// Object to use to handle parsing function calls. public IdentifierTokenizer(HashSet parameters, IFunctionCallParser functionCallParser) - :this(parameters, functionCallParser, null) - { - } - - /// - /// Parse an Identifier into the right QueryToken - /// - /// parameters passed in to the UriQueryExpressionParser - /// Object to use to handle parsing function calls. - /// Instance of the UriQueryExpressionParser. - public IdentifierTokenizer(HashSet parameters, IFunctionCallParser functionCallParser, UriQueryExpressionParser uriQueryExpressionParser) { ExceptionUtils.CheckArgumentNotNull(parameters, "parameters"); ExceptionUtils.CheckArgumentNotNull(functionCallParser, "functionCallParser"); this.lexer = functionCallParser.Lexer; this.parameters = parameters; this.functionCallParser = functionCallParser; - this.uriQueryExpressionParser = uriQueryExpressionParser; } /// @@ -75,19 +58,6 @@ public QueryToken ParseIdentifier(QueryToken parent) { this.lexer.ValidateToken(ExpressionTokenKind.Identifier); - string identifier = this.lexer.CurrentToken.GetIdentifier(); - - bool enableCaseInsensitive = uriQueryExpressionParser != null ? uriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier : false; - - // If identifier is $count, we need to return a CountSegmentToken. - if (identifier.Equals(UriQueryConstants.CountSegment, enableCaseInsensitive ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal)) - { - this.lexer.NextToken(); - - CountSegmentParser countSegmentParser = new CountSegmentParser(this.lexer, this.uriQueryExpressionParser); - return countSegmentParser.CreateCountSegmentToken(identifier, parent); - } - // An open paren here would indicate calling a method in regular C# syntax. // TODO: Make this more generalized to work with any kind of function. bool identifierIsFunction = this.lexer.ExpandIdentifierAsFunction(); diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index 2fe2af870a..fa219098be 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -54,7 +54,7 @@ public sealed class UriQueryExpressionParser /// /// Whether to allow case insensitive for builtin identifier. /// - internal bool enableCaseInsensitiveBuiltinIdentifier = false; + private bool enableCaseInsensitiveBuiltinIdentifier = false; /// /// Tracks the depth of aggregate expression recursion. @@ -1050,13 +1050,17 @@ private QueryToken ParsePrimary() { expr = this.ParseAll(expr); } + else if (this.TokenIdentifierIs(ExpressionConstants.QueryOptionCount)) + { + expr = this.ParseCountSegment(expr); + } else if (this.lexer.PeekNextToken().Kind == ExpressionTokenKind.Slash) { expr = this.ParseSegment(expr); } else { - IdentifierTokenizer identifierTokenizer = new IdentifierTokenizer(this.parameters, new FunctionCallParser(this.lexer, this, this.IsInAggregateExpression), this); + IdentifierTokenizer identifierTokenizer = new IdentifierTokenizer(this.parameters, new FunctionCallParser(this.lexer, this, this.IsInAggregateExpression)); expr = identifierTokenizer.ParseIdentifier(expr); } } @@ -1229,6 +1233,20 @@ private QueryToken ParseSegment(QueryToken parent) return new InnerPathToken(propertyName, parent, null); } + /// + /// Parses a $count segment. + /// + /// The parent of the segment node. + /// The lexical token representing the $count segment. + private QueryToken ParseCountSegment(QueryToken parent) + { + string identifier = this.lexer.CurrentToken.GetIdentifier(); + this.lexer.NextToken(); + + CountSegmentParser countSegmentParser = new CountSegmentParser(this.lexer, this); + return countSegmentParser.CreateCountSegmentToken(identifier, parent); + } + private AggregationMethodDefinition ParseAggregateWith() { if (!TokenIdentifierIs(ExpressionConstants.KeywordWith)) From 986a79248b123e9fc3a15771dece85e67ef2b7a3 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Tue, 27 Apr 2021 08:56:03 +0300 Subject: [PATCH 08/19] Initial commit for semantic binding --- .../UriParser/Binders/CountSegmentBinder.cs | 45 +++++++++++++++++++ .../UriParser/Binders/MetadataBinder.cs | 14 ++++++ .../Parsers/UriQueryExpressionParser.cs | 4 +- 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs diff --git a/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs new file mode 100644 index 0000000000..a4d732dd8b --- /dev/null +++ b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs @@ -0,0 +1,45 @@ +//--------------------------------------------------------------------- +// +// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information. +// +//--------------------------------------------------------------------- + +namespace Microsoft.OData.UriParser +{ + using System.Linq; + using Microsoft.OData.Edm; + using Microsoft.OData.Metadata; + using ODataErrorStrings = Microsoft.OData.Strings; + + /// + /// Class that knows how to bind an end path token, which could be several things. + /// + internal sealed class CountSegmentBinder + { + /// + /// Constructs a CountSegmentBinder object using the given function to bind parent token. + /// + /// Method to bind the EndPathToken's parent, if there is one. + /// State of the metadata binding.am> + /// + /// Make bindMethod of return type SingleValueQueryNode. + /// + internal CountSegmentBinder(MetadataBinder.QueryTokenVisitor bindMethod) + { + } + + /// + /// Binds an In operator token. + /// + /// The In operator token to bind. + /// State of the metadata binding. + /// The bound In operator token. + internal QueryNode BindCountSegment(CountSegmentToken countSegmentToken) + { + ExceptionUtils.CheckArgumentNotNull(countSegmentToken, "countSegmentToken"); + + CollectionNode node = null; + return new CountNode(node); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.OData.Core/UriParser/Binders/MetadataBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/MetadataBinder.cs index 119e5f4d3e..480e8a9acd 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/MetadataBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/MetadataBinder.cs @@ -198,6 +198,9 @@ protected internal QueryNode Bind(QueryToken token) case QueryTokenKind.In: result = this.BindIn((InToken)token); break; + case QueryTokenKind.CountSegment: + result = this.BindCountSegment((CountSegmentToken)token); + break; default: throw new ODataException(ODataErrorStrings.MetadataBinder_UnsupportedQueryTokenKind(token.Kind)); } @@ -370,5 +373,16 @@ protected virtual QueryNode BindIn(InToken inToken) InBinder inBinder = new InBinder(InBinderMethod); return inBinder.BindInOperator(inToken, this.BindingState); } + + /// + /// Binds a CountSegment token. + /// + /// The CountSegment token to bind. + /// The bound CountSegment token. + protected virtual QueryNode BindCountSegment(CountSegmentToken countSegmentToken) + { + CountSegmentBinder countSegmentBinder = new CountSegmentBinder(this.Bind); + return countSegmentBinder.BindCountSegment(countSegmentToken); + } } } \ No newline at end of file diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index fa219098be..d417e5fc56 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -1050,10 +1050,10 @@ private QueryToken ParsePrimary() { expr = this.ParseAll(expr); } - else if (this.TokenIdentifierIs(ExpressionConstants.QueryOptionCount)) + /*else if (this.TokenIdentifierIs(ExpressionConstants.QueryOptionCount)) { expr = this.ParseCountSegment(expr); - } + }*/ else if (this.lexer.PeekNextToken().Kind == ExpressionTokenKind.Slash) { expr = this.ParseSegment(expr); From 68c610d3e7348459c11944a9159e98a8eec953a4 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Tue, 27 Apr 2021 10:36:23 +0300 Subject: [PATCH 09/19] Fix bug on the lexer --- .../UriParser/Parsers/CountSegmentParser.cs | 2 ++ .../UriParser/Parsers/UriQueryExpressionParser.cs | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index c22d3f4e26..1e64e3af17 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -70,6 +70,7 @@ public CountSegmentToken CreateCountSegmentToken(string identifier, QueryToken c // We need to maintain the recursionDepth of the outer $filter query since calling // ParseFilter or ParseSearch below will reset it to 0. int outerRecursiveDepth = this.UriQueryExpressionParser.recursionDepth; + ExpressionLexer outerLexer = this.UriQueryExpressionParser.Lexer; if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.OpenParen) { @@ -104,6 +105,7 @@ public CountSegmentToken CreateCountSegmentToken(string identifier, QueryToken c this.lexer.NextToken(); this.UriQueryExpressionParser.recursionDepth = outerRecursiveDepth; + this.UriQueryExpressionParser.Lexer = outerLexer; return new CountSegmentToken(identifier, countedInstance, filterToken, searchToken); } diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index d417e5fc56..6544a3df1f 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -111,6 +111,7 @@ internal UriQueryExpressionParser(int maxDepth, ExpressionLexer lexer) : this(ma internal ExpressionLexer Lexer { get { return this.lexer; } + set { this.lexer = value; } } /// @@ -1050,10 +1051,10 @@ private QueryToken ParsePrimary() { expr = this.ParseAll(expr); } - /*else if (this.TokenIdentifierIs(ExpressionConstants.QueryOptionCount)) + else if (this.TokenIdentifierIs(ExpressionConstants.QueryOptionCount)) { expr = this.ParseCountSegment(expr); - }*/ + } else if (this.lexer.PeekNextToken().Kind == ExpressionTokenKind.Slash) { expr = this.ParseSegment(expr); From 0b121fcdf23cb9baa1c256f7502b661cb1e64da6 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Tue, 27 Apr 2021 17:21:28 +0300 Subject: [PATCH 10/19] Update semantic binding --- .../UriParser/Binders/CountSegmentBinder.cs | 59 ++++++++++++++----- .../UriParser/Binders/MetadataBinder.cs | 4 +- .../UriParser/Parsers/CountSegmentParser.cs | 3 - .../UriParser/SemanticAst/CountNode.cs | 43 ++++++++++++++ .../FilterAndOrderByFunctionalTests.cs | 2 +- 5 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs index a4d732dd8b..a60c5d6402 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs @@ -6,40 +6,67 @@ namespace Microsoft.OData.UriParser { - using System.Linq; - using Microsoft.OData.Edm; - using Microsoft.OData.Metadata; using ODataErrorStrings = Microsoft.OData.Strings; /// - /// Class that knows how to bind an end path token, which could be several things. + /// Class that knows how to bind a Count segment token. /// internal sealed class CountSegmentBinder { /// - /// Constructs a CountSegmentBinder object using the given function to bind parent token. + /// Method to use to visit the token tree and bind the tokens recursively. /// - /// Method to bind the EndPathToken's parent, if there is one. - /// State of the metadata binding.am> - /// - /// Make bindMethod of return type SingleValueQueryNode. - /// - internal CountSegmentBinder(MetadataBinder.QueryTokenVisitor bindMethod) + private readonly MetadataBinder.QueryTokenVisitor bindMethod; + + /// + /// State to use for binding. + /// + private readonly BindingState state; + + /// + /// Constructs a CountSegmentBinder object. + /// + /// Method to use to visit the token tree and bind the tokens recursively. + /// State of the metadata binding. + internal CountSegmentBinder(MetadataBinder.QueryTokenVisitor bindMethod, BindingState state) { + this.bindMethod = bindMethod; + this.state = state; } /// - /// Binds an In operator token. + /// Binds an Count segment token. /// - /// The In operator token to bind. + /// The Count segment token to bind. /// State of the metadata binding. - /// The bound In operator token. + /// The bound Count segment token. internal QueryNode BindCountSegment(CountSegmentToken countSegmentToken) { ExceptionUtils.CheckArgumentNotNull(countSegmentToken, "countSegmentToken"); - CollectionNode node = null; - return new CountNode(node); + QueryNode source = this.bindMethod(countSegmentToken.NextToken); + CollectionNode node = source as CollectionNode; + + FilterClause filterClause = null; + SearchClause searchClause = null; + + BindingState innerBindingState = new BindingState(state.Configuration); + MetadataBinder binder = new MetadataBinder(innerBindingState); + innerBindingState.ImplicitRangeVariable = NodeFactory.CreateParameterNode(ExpressionConstants.It, node); + + if (countSegmentToken.FilterOption != null) + { + FilterBinder filterBinder = new FilterBinder(binder.Bind, innerBindingState); + filterClause = filterBinder.BindFilter(countSegmentToken.FilterOption); + } + + if(countSegmentToken.SearchOption != null) + { + SearchBinder searchBinder = new SearchBinder(bindMethod); + searchClause = searchBinder.BindSearch(countSegmentToken.SearchOption); + } + + return new CountNode(node, filterClause, searchClause); } } } \ No newline at end of file diff --git a/src/Microsoft.OData.Core/UriParser/Binders/MetadataBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/MetadataBinder.cs index 480e8a9acd..bac1e17bf9 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/MetadataBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/MetadataBinder.cs @@ -377,11 +377,11 @@ protected virtual QueryNode BindIn(InToken inToken) /// /// Binds a CountSegment token. /// - /// The CountSegment token to bind. + /// The CountSegment token to bind. /// The bound CountSegment token. protected virtual QueryNode BindCountSegment(CountSegmentToken countSegmentToken) { - CountSegmentBinder countSegmentBinder = new CountSegmentBinder(this.Bind); + CountSegmentBinder countSegmentBinder = new CountSegmentBinder(this.Bind, this.BindingState); return countSegmentBinder.BindCountSegment(countSegmentToken); } } diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index 1e64e3af17..0f6004fdd1 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -7,9 +7,6 @@ namespace Microsoft.OData.UriParser { using System; - using System.Collections.Generic; - using System.Diagnostics; - using System.Linq; using ODataErrorStrings = Microsoft.OData.Strings; /// diff --git a/src/Microsoft.OData.Core/UriParser/SemanticAst/CountNode.cs b/src/Microsoft.OData.Core/UriParser/SemanticAst/CountNode.cs index d3754e43fc..91ac11437c 100644 --- a/src/Microsoft.OData.Core/UriParser/SemanticAst/CountNode.cs +++ b/src/Microsoft.OData.Core/UriParser/SemanticAst/CountNode.cs @@ -23,18 +23,45 @@ public sealed class CountNode : SingleValueNode /// private readonly CollectionNode source; + /// + /// The filter clause to be evaluated first before count is applied. + /// + private readonly FilterClause filterClause; + + /// + /// The search clause to be evaluated first before count is applied. + /// + private readonly SearchClause searchClause; + /// /// Constructs a new . /// /// The value containing the property. /// Throws if the input source is null. public CountNode(CollectionNode source) + :this(source, null, null) { ExceptionUtils.CheckArgumentNotNull(source, "source"); this.source = source; } + /// + /// Constructs a new . + /// + /// The value containing the property. + /// The in the count node. + /// The in the count node. + /// Throws if the input source is null. + public CountNode(CollectionNode source, FilterClause filterClause, SearchClause searchClause) + { + ExceptionUtils.CheckArgumentNotNull(source, "source"); + + this.source = source; + this.filterClause = filterClause; + this.searchClause = searchClause; + } + /// /// Gets the collection property node to be counted. /// @@ -43,6 +70,22 @@ public CollectionNode Source get { return this.source; } } + /// + /// Gets the filter node. + /// + public FilterClause FilterClause + { + get { return this.filterClause; } + } + + /// + /// Gets the search node. + /// + public SearchClause SearchClause + { + get { return this.searchClause; } + } + /// /// Gets the value type this node represents. /// diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index f679e834c7..53aa5becfc 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -405,7 +405,7 @@ public void ParseFilterWithEntityCollectionCountWithFilterOption() { var filterQueryNode = ParseFilter("MyFriendsDogs/$count($filter=Color eq 'Brown') gt 1", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); - filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.Equal). + filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.GreaterThan). Left.ShouldBeCountNode(). Source.ShouldBeCollectionNavigationNode(HardCodedTestModel.GetPersonMyFriendsDogsProp()); } From 0328130490fc35d9a179816c657a1092d3921574 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Tue, 27 Apr 2021 17:31:19 +0300 Subject: [PATCH 11/19] code cleanup --- .../UriParser/Binders/CountSegmentBinder.cs | 4 ++-- .../UriParser/Parsers/IdentifierTokenizer.cs | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs index a60c5d6402..e6805ac8d7 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs @@ -50,9 +50,9 @@ internal QueryNode BindCountSegment(CountSegmentToken countSegmentToken) FilterClause filterClause = null; SearchClause searchClause = null; - BindingState innerBindingState = new BindingState(state.Configuration); - MetadataBinder binder = new MetadataBinder(innerBindingState); + BindingState innerBindingState = new BindingState(state.Configuration); innerBindingState.ImplicitRangeVariable = NodeFactory.CreateParameterNode(ExpressionConstants.It, node); + MetadataBinder binder = new MetadataBinder(innerBindingState); if (countSegmentToken.FilterOption != null) { diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs index cf603821db..193fded08a 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/IdentifierTokenizer.cs @@ -10,7 +10,6 @@ namespace Microsoft.OData.UriParser using System; using System.Collections.Generic; - using System.Globalization; using ODataErrorStrings = Microsoft.OData.Strings; #endregion Namespaces @@ -61,7 +60,6 @@ public QueryToken ParseIdentifier(QueryToken parent) // An open paren here would indicate calling a method in regular C# syntax. // TODO: Make this more generalized to work with any kind of function. bool identifierIsFunction = this.lexer.ExpandIdentifierAsFunction(); - QueryToken functionCallToken; if (identifierIsFunction && this.functionCallParser.TryParseIdentifierAsFunction(parent, out functionCallToken)) { From 7d98b9b3b33deef373031b0494e5d2e351d5b853 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Thu, 6 May 2021 11:05:35 +0300 Subject: [PATCH 12/19] Fix bug on --- .../UriParser/Binders/CountSegmentBinder.cs | 2 +- .../UriParser/Parsers/CountSegmentParser.cs | 3 ++- .../UriParser/FilterAndOrderByFunctionalTests.cs | 12 ++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs index e6805ac8d7..96f30ac2e3 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs @@ -62,7 +62,7 @@ internal QueryNode BindCountSegment(CountSegmentToken countSegmentToken) if(countSegmentToken.SearchOption != null) { - SearchBinder searchBinder = new SearchBinder(bindMethod); + SearchBinder searchBinder = new SearchBinder(binder.Bind); searchClause = searchBinder.BindSearch(countSegmentToken.SearchOption); } diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index 0f6004fdd1..4b5b6aaa31 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -87,7 +87,8 @@ public CountSegmentToken CreateCountSegmentToken(string identifier, QueryToken c else if (textWithinParenthesis.StartsWith(ExpressionConstants.QueryOptionSearch, StringComparison.OrdinalIgnoreCase)) { string searchQuery = TryGetQueryOption(ExpressionConstants.QueryOptionSearch, textWithinParenthesis); - searchToken = searchQuery == null ? null : this.UriQueryExpressionParser.ParseSearch(searchQuery); + SearchParser searchParser = new SearchParser(ODataUriParserSettings.DefaultSearchLimit); + searchToken = searchQuery == null ? null : searchParser.ParseSearch(searchQuery); } else { diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index 53aa5becfc..31b2659c70 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -424,6 +424,18 @@ public void ParseFilterWithEntityCollectionCountWithIllegalQueryOptionThrows() parse.Throws(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); } + [Fact] + public void ParseFilterWithEntityCollectionCountWithSearchOption() + { + var filterQueryNode = ParseFilter("MyFriendsDogs/$count($search=brown) gt 1", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + + BinaryOperatorNode binaryNode = filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.GreaterThan); + CountNode countNode = binaryNode.Left.ShouldBeCountNode(); + + Assert.Null(countNode.FilterClause); + countNode.SearchClause.Expression.ShouldBeSearchTermNode("brown"); + } + [Fact] public void CompareComplexWithNull() { From 0f13d3ec1feeb4b88f2d639a394ff50f4e6aa500 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Thu, 6 May 2021 12:15:32 +0300 Subject: [PATCH 13/19] Fix based on review comments --- .../Microsoft.OData.Core.cs | 1 - .../Microsoft.OData.Core.txt | 1 - .../Parameterized.Microsoft.OData.Core.cs | 8 ------ .../UriParser/Parsers/CountSegmentParser.cs | 12 +++------ .../Parsers/UriQueryExpressionParser.cs | 3 +-- .../SyntacticAst/CountSegmentToken.cs | 25 ++++++++----------- .../FilterAndOrderByFunctionalTests.cs | 2 +- 7 files changed, 16 insertions(+), 36 deletions(-) diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs index 2e9b84ee65..246fcd78bb 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs @@ -600,7 +600,6 @@ internal sealed class TextRes { internal const string UriQueryExpressionParser_OpenParenExpected = "UriQueryExpressionParser_OpenParenExpected"; internal const string UriQueryExpressionParser_CloseParenOrCommaExpected = "UriQueryExpressionParser_CloseParenOrCommaExpected"; internal const string UriQueryExpressionParser_CloseParenOrOperatorExpected = "UriQueryExpressionParser_CloseParenOrOperatorExpected"; - internal const string UriQueryExpressionParser_CloseParenExpected = "UriQueryExpressionParser_CloseParenExpected"; internal const string UriQueryExpressionParser_IllegalQueryOptioninDollarCount = "UriQueryExpressionParser_IllegalQueryOptioninDollarCount"; internal const string UriQueryExpressionParser_CannotCreateStarTokenFromNonStar = "UriQueryExpressionParser_CannotCreateStarTokenFromNonStar"; internal const string UriQueryExpressionParser_RangeVariableAlreadyDeclared = "UriQueryExpressionParser_RangeVariableAlreadyDeclared"; diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt index 969feea802..465026c4c1 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt @@ -692,7 +692,6 @@ UriQueryExpressionParser_ExpressionExpected=Expression expected at position {0} UriQueryExpressionParser_OpenParenExpected='(' expected at position {0} in '{1}'. UriQueryExpressionParser_CloseParenOrCommaExpected=')' or ',' expected at position {0} in '{1}'. UriQueryExpressionParser_CloseParenOrOperatorExpected=')' or operator expected at position {0} in '{1}'. -UriQueryExpressionParser_CloseParenExpected=')' expected at position {0} in '{1}'. UriQueryExpressionParser_IllegalQueryOptioninDollarCount=Only $filter and $search query options are allowed within $count. UriQueryExpressionParser_CannotCreateStarTokenFromNonStar=Expecting a Star token but got: '{0}'. UriQueryExpressionParser_RangeVariableAlreadyDeclared=The range variable '{0}' has already been declared. diff --git a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs index 50fbb87747..0ccc7118b1 100644 --- a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs @@ -5212,14 +5212,6 @@ internal static string UriQueryExpressionParser_CloseParenOrOperatorExpected(obj return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.UriQueryExpressionParser_CloseParenOrOperatorExpected, p0, p1); } - /// - /// A string like "')' expected at position {0} in '{1}'." - /// - internal static string UriQueryExpressionParser_CloseParenExpected(object p0, object p1) - { - return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.UriQueryExpressionParser_CloseParenExpected, p0, p1); - } - /// /// A string like "Only $filter and $search query options are allowed within $count." /// diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index 4b5b6aaa31..a926806695 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -56,9 +56,8 @@ public ExpressionLexer Lexer /// /// Create a . /// - /// The name of this token, which in this case is always "$count".The name of this token, which in this case is always "$count". /// The instance to count on. - public CountSegmentToken CreateCountSegmentToken(string identifier, QueryToken countedInstance) + public CountSegmentToken CreateCountSegmentToken(QueryToken countedInstance) { QueryToken filterToken = null; QueryToken searchToken = null; @@ -71,7 +70,7 @@ public CountSegmentToken CreateCountSegmentToken(string identifier, QueryToken c if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.OpenParen) { - return new CountSegmentToken(identifier, countedInstance); + return new CountSegmentToken(countedInstance); } else { @@ -96,15 +95,12 @@ public CountSegmentToken CreateCountSegmentToken(string identifier, QueryToken c throw ParseError(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); } - if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.CloseParen) - { - throw ParseError(ODataErrorStrings.UriQueryExpressionParser_CloseParenExpected(this.lexer.CurrentToken.Position, this.lexer.ExpressionText)); - } + this.lexer.ValidateToken(ExpressionTokenKind.CloseParen); this.lexer.NextToken(); this.UriQueryExpressionParser.recursionDepth = outerRecursiveDepth; this.UriQueryExpressionParser.Lexer = outerLexer; - return new CountSegmentToken(identifier, countedInstance, filterToken, searchToken); + return new CountSegmentToken(countedInstance, filterToken, searchToken); } /// diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index 6544a3df1f..748d665750 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -1241,11 +1241,10 @@ private QueryToken ParseSegment(QueryToken parent) /// The lexical token representing the $count segment. private QueryToken ParseCountSegment(QueryToken parent) { - string identifier = this.lexer.CurrentToken.GetIdentifier(); this.lexer.NextToken(); CountSegmentParser countSegmentParser = new CountSegmentParser(this.lexer, this); - return countSegmentParser.CreateCountSegmentToken(identifier, parent); + return countSegmentParser.CreateCountSegmentToken(parent); } private AggregationMethodDefinition ParseAggregateWith() diff --git a/src/Microsoft.OData.Core/UriParser/SyntacticAst/CountSegmentToken.cs b/src/Microsoft.OData.Core/UriParser/SyntacticAst/CountSegmentToken.cs index e99f4f19d3..92ccdda54a 100644 --- a/src/Microsoft.OData.Core/UriParser/SyntacticAst/CountSegmentToken.cs +++ b/src/Microsoft.OData.Core/UriParser/SyntacticAst/CountSegmentToken.cs @@ -15,11 +15,6 @@ namespace Microsoft.OData.UriParser /// public sealed class CountSegmentToken : PathToken { - /// - /// The name of this token, which in this case is always "$count". - /// - private readonly string identifier; - /// /// The instance to count on. /// @@ -36,28 +31,24 @@ public sealed class CountSegmentToken : PathToken private QueryToken searchOption; /// - /// Create a CountSegmentToken given the Identifier and the NextToken. + /// Create a CountSegmentToken given the NextToken. /// - /// The name of this token, which in this case is always "$count". /// The instance to count on. - public CountSegmentToken(string identifier, QueryToken nextToken) - :this(identifier, nextToken, null, null) + public CountSegmentToken(QueryToken nextToken) + :this(nextToken, null, null) { } /// - /// Create a CountSegmentToken given the Identifier, NextToken, FilterOption (if any) and SearchOption (if any). + /// Create a CountSegmentToken given the NextToken, FilterOption (if any) and SearchOption (if any). /// - /// The name of this token, which in this case is always "$count". /// The instance to count on. /// The representing $filter. /// The representing $search. - public CountSegmentToken(string identifier, QueryToken nextToken, QueryToken filterOption, QueryToken searchOption) + public CountSegmentToken(QueryToken nextToken, QueryToken filterOption, QueryToken searchOption) { - ExceptionUtils.CheckArgumentStringNotNullOrEmpty(identifier, "identifier"); ExceptionUtils.CheckArgumentNotNull(nextToken, "nextToken"); - this.identifier = identifier; this.nextToken = nextToken; this.filterOption = filterOption; this.searchOption = searchOption; @@ -76,7 +67,11 @@ public override QueryTokenKind Kind /// public override string Identifier { - get { return this.identifier; } +#if ODATA_CLIENT + get { return UriHelper.VIRTUALPROPERTYCOUNT; } +#else + get { return ExpressionConstants.QueryOptionCount; } +#endif } /// diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index 31b2659c70..367294d94f 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -414,7 +414,7 @@ public void ParseFilterWithEntityCollectionCountWithFilterOption() public void ParseFilterWithEntityCollectionCountWithUnbalanceParenthesisThrows() { Action parse = () => ParseFilter("MyFriendsDogs/$count($filter=Color eq 'Brown' gt 1", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); - parse.Throws(ODataErrorStrings.UriQueryExpressionParser_CloseParenExpected(50, "MyFriendsDogs/$count($filter=Color eq 'Brown' gt 1")); + parse.Throws(ODataErrorStrings.ExpressionLexer_SyntaxError(50, "MyFriendsDogs/$count($filter=Color eq 'Brown' gt 1")); } [Fact] From 97053b25cc1db26734deeae5d7d44f3d8c498231 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Thu, 6 May 2021 20:55:56 +0300 Subject: [PATCH 14/19] Fix search and update tests --- .../UriParser/Parsers/CountSegmentParser.cs | 69 ++++++++++++++----- .../Parsers/UriQueryExpressionParser.cs | 4 +- .../FilterAndOrderByFunctionalTests.cs | 24 +++++++ 3 files changed, 78 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index a926806695..4925e33f76 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -61,7 +61,12 @@ public CountSegmentToken CreateCountSegmentToken(QueryToken countedInstance) { QueryToken filterToken = null; QueryToken searchToken = null; - string textWithinParenthesis = null; + + bool isFilterOption = false; + bool isSearchOption = false; + + string filterText = string.Empty; + string searchText = string.Empty; // We need to maintain the recursionDepth of the outer $filter query since calling // ParseFilter or ParseSearch below will reset it to 0. @@ -74,27 +79,57 @@ public CountSegmentToken CreateCountSegmentToken(QueryToken countedInstance) } else { - textWithinParenthesis = this.lexer.AdvanceThroughExpandOption(); + this.lexer.NextToken(); + isFilterOption = this.lexer.CurrentToken.IdentifierIs(ExpressionConstants.QueryOptionFilter, this.UriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier); + isSearchOption = this.lexer.CurrentToken.IdentifierIs(ExpressionConstants.QueryOptionSearch, this.UriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier); + + this.lexer.NextToken(); + + if (isFilterOption) + { + filterText = this.lexer.AdvanceThroughExpandOption(); + filterToken = filterText == null ? null : this.UriQueryExpressionParser.ParseFilter(filterText); + } + else if(isSearchOption) + { + searchText = this.lexer.AdvanceThroughExpandOption(); + SearchParser searchParser = new SearchParser(ODataUriParserSettings.DefaultSearchLimit); + searchToken = searchText == null ? null : searchParser.ParseSearch(searchText); + } + else + { + // Only a $filter and $search can be inside a $count() + throw ParseError(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); + } } - // TODO: check for NoDollarQueryOptions - if (textWithinParenthesis.StartsWith(ExpressionConstants.QueryOptionFilter, StringComparison.OrdinalIgnoreCase)) - { - string filterQuery = TryGetQueryOption(ExpressionConstants.QueryOptionFilter, textWithinParenthesis); - filterToken = filterQuery == null ? null: this.UriQueryExpressionParser.ParseFilter(filterQuery); - } - else if (textWithinParenthesis.StartsWith(ExpressionConstants.QueryOptionSearch, StringComparison.OrdinalIgnoreCase)) + if (this.lexer.CurrentToken.Kind == ExpressionTokenKind.SemiColon) { - string searchQuery = TryGetQueryOption(ExpressionConstants.QueryOptionSearch, textWithinParenthesis); - SearchParser searchParser = new SearchParser(ODataUriParserSettings.DefaultSearchLimit); - searchToken = searchQuery == null ? null : searchParser.ParseSearch(searchQuery); - } - else - { - // Only a $filter and $search can be inside a $count() - throw ParseError(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); + this.lexer.NextToken(); + isFilterOption = this.lexer.CurrentToken.IdentifierIs(ExpressionConstants.QueryOptionFilter, this.UriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier); + isSearchOption = this.lexer.CurrentToken.IdentifierIs(ExpressionConstants.QueryOptionSearch, this.UriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier); + + this.lexer.NextToken(); + + if (isFilterOption) + { + filterText = this.lexer.AdvanceThroughExpandOption(); + filterToken = filterText == null ? null : this.UriQueryExpressionParser.ParseFilter(filterText); + } + else if (isSearchOption) + { + searchText = this.lexer.AdvanceThroughExpandOption(); + SearchParser searchParser = new SearchParser(ODataUriParserSettings.DefaultSearchLimit); + searchToken = searchText == null ? null : searchParser.ParseSearch(searchText); + } + else + { + // Only a $filter and $search can be inside a $count() + throw ParseError(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); + } } + this.lexer.ValidateToken(ExpressionTokenKind.CloseParen); this.lexer.NextToken(); diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index 748d665750..5e3f3299c9 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -54,7 +54,7 @@ public sealed class UriQueryExpressionParser /// /// Whether to allow case insensitive for builtin identifier. /// - private bool enableCaseInsensitiveBuiltinIdentifier = false; + internal bool enableCaseInsensitiveBuiltinIdentifier = false; /// /// Tracks the depth of aggregate expression recursion. @@ -700,7 +700,7 @@ internal QueryToken ParseExpression() /// The lexer for the expression, which will have already moved to the first token. private static ExpressionLexer CreateLexerForFilterOrOrderByOrApplyExpression(string expression) { - return new ExpressionLexer(expression, true /*moveToFirstToken*/, false /*useSemicolonDelimiter*/, true /*parsingFunctionParameters*/); + return new ExpressionLexer(expression, true /*moveToFirstToken*/, true /*useSemicolonDelimiter*/, true /*parsingFunctionParameters*/); } /// Creates an exception for a parse error. diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index 367294d94f..cb48b2ed52 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -408,6 +408,15 @@ public void ParseFilterWithEntityCollectionCountWithFilterOption() filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.GreaterThan). Left.ShouldBeCountNode(). Source.ShouldBeCollectionNavigationNode(HardCodedTestModel.GetPersonMyFriendsDogsProp()); + + BinaryOperatorNode binaryNode = filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.GreaterThan); + CountNode countNode = binaryNode.Left.ShouldBeCountNode(); + countNode.Source.ShouldBeCollectionNavigationNode(HardCodedTestModel.GetPersonMyFriendsDogsProp()); + + Assert.Null(countNode.SearchClause); + BinaryOperatorNode innerBinaryNode = countNode.FilterClause.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.Equal); + Assert.Equal("Color", Assert.IsType(innerBinaryNode.Left).Property.Name); + Assert.Equal("Brown", Assert.IsType(innerBinaryNode.Right).Value); } [Fact] @@ -431,11 +440,26 @@ public void ParseFilterWithEntityCollectionCountWithSearchOption() BinaryOperatorNode binaryNode = filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.GreaterThan); CountNode countNode = binaryNode.Left.ShouldBeCountNode(); + countNode.Source.ShouldBeCollectionNavigationNode(HardCodedTestModel.GetPersonMyFriendsDogsProp()); Assert.Null(countNode.FilterClause); countNode.SearchClause.Expression.ShouldBeSearchTermNode("brown"); } + [Fact] + public void ParseFilterWithEntityCollectionCountWithFilterAndSearchOptions() + { + var filterQueryNode = ParseFilter("MyFriendsDogs/$count($filter=Color eq 'Brown';$search=brown) gt 1", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + + BinaryOperatorNode binaryNode = filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.GreaterThan); + CountNode countNode = binaryNode.Left.ShouldBeCountNode(); + countNode.SearchClause.Expression.ShouldBeSearchTermNode("brown"); + + BinaryOperatorNode innerBinaryNode = countNode.FilterClause.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.Equal); + Assert.Equal("Color", Assert.IsType(innerBinaryNode.Left).Property.Name); + Assert.Equal("Brown", Assert.IsType(innerBinaryNode.Right).Value); + } + [Fact] public void CompareComplexWithNull() { From 7b8b7cd7acf35e6435287622b54fa8da264f97cb Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Fri, 7 May 2021 00:05:18 +0300 Subject: [PATCH 15/19] Refractor code --- .../GlobalSuppressions.cs | 1 + .../Microsoft.OData.Core.cs | 1 + .../Microsoft.OData.Core.txt | 1 + .../Parameterized.Microsoft.OData.Core.cs | 8 + .../UriParser/Parsers/CountSegmentParser.cs | 145 +++++++++--------- .../Parsers/UriQueryExpressionParser.cs | 12 +- .../FilterAndOrderByFunctionalTests.cs | 7 + 7 files changed, 98 insertions(+), 77 deletions(-) diff --git a/src/Microsoft.OData.Core/GlobalSuppressions.cs b/src/Microsoft.OData.Core/GlobalSuppressions.cs index c9b4478326..94f4249174 100644 --- a/src/Microsoft.OData.Core/GlobalSuppressions.cs +++ b/src/Microsoft.OData.Core/GlobalSuppressions.cs @@ -52,5 +52,6 @@ [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1800:DoNotCastUnnecessarily", Scope = "member", Target = "Microsoft.OData.UriParser.InnerPathTokenBinder.#BindInnerPathSegment(Microsoft.OData.UriParser.InnerPathToken)")] [assembly: SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "System.String.Format(System.String,System.Object[])", Scope = "member", Target = "Microsoft.OData.PropertyCacheHandler.#GetProperty(System.String,Microsoft.OData.Edm.IEdmStructuredType)")] [assembly: SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Utils", Scope = "type", Target = "Microsoft.OData.Buffers.BufferUtils")] +[assembly: SuppressMessage("Microsoft.Globalization", "CA1308:NormalizeStringsToUppercase", Scope = "member", Target = "Microsoft.OData.UriParser.CountSegmentParser.#CreateCountSegmentToken(Microsoft.OData.UriParser.QueryToken)")] [assembly: SuppressMessage("Microsoft.Globalization", "CA1308:NormalizeStringsToUppercase", Scope = "member", Target = "Microsoft.OData.UriParser.SelectExpandOptionParser.#BuildSelectTermToken(Microsoft.OData.UriParser.PathSegmentToken,System.String)")] diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs index 246fcd78bb..67b1393b42 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs @@ -712,6 +712,7 @@ internal sealed class TextRes { internal const string UriSelectParser_InvalidLevelsOption = "UriSelectParser_InvalidLevelsOption"; internal const string UriSelectParser_SystemTokenInSelectExpand = "UriSelectParser_SystemTokenInSelectExpand"; internal const string UriParser_MissingExpandOption = "UriParser_MissingExpandOption"; + internal const string UriParser_EmptyParenthesis = "UriParser_EmptyParenthesis"; internal const string UriParser_MissingSelectOption = "UriParser_MissingSelectOption"; internal const string UriParser_RelativeUriMustBeRelative = "UriParser_RelativeUriMustBeRelative"; internal const string UriParser_NeedServiceRootForThisOverload = "UriParser_NeedServiceRootForThisOverload"; diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt index 465026c4c1..9e574af303 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt @@ -820,6 +820,7 @@ UriSelectParser_InvalidCountOption=Count option must be a boolean value, it is s UriSelectParser_InvalidLevelsOption=Levels option must be a non-negative integer or 'max', it is set to '{0}' instead. UriSelectParser_SystemTokenInSelectExpand=Found system token '{0}' in select or expand clause '{1}'. UriParser_MissingExpandOption=Missing expand option on navigation property '{0}'. If a parenthesis expression follows an expanded navigation property, then at least one expand option must be provided. +UriParser_EmptyParenthesis=Empty parenthesis not allowed. UriParser_MissingSelectOption=Missing select option on property '{0}'. If a parenthesis expression follows a selected property, then at least one query option must be provided. UriParser_RelativeUriMustBeRelative=Parameter 'relativeUri' must be a relative Uri if serviceRoot is not specified. UriParser_NeedServiceRootForThisOverload=A service root URI must be provided to the ODataUriParser in order to use this method. diff --git a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs index 0ccc7118b1..40d801684e 100644 --- a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs @@ -6204,6 +6204,14 @@ internal static string UriParser_MissingExpandOption(object p0) return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.UriParser_MissingExpandOption, p0); } + /// + /// A string like "Empty parenthesis not allowed." + /// + internal static string UriParser_EmptyParenthesis() + { + return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.UriParser_EmptyParenthesis); + } + /// /// A string like "Missing select option on property '{0}'. If a parenthesis expression follows a selected property, then at least one query option must be provided." /// diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index 4925e33f76..f688b21045 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -62,109 +62,104 @@ public CountSegmentToken CreateCountSegmentToken(QueryToken countedInstance) QueryToken filterToken = null; QueryToken searchToken = null; - bool isFilterOption = false; - bool isSearchOption = false; - - string filterText = string.Empty; - string searchText = string.Empty; - - // We need to maintain the recursionDepth of the outer $filter query since calling - // ParseFilter or ParseSearch below will reset it to 0. - int outerRecursiveDepth = this.UriQueryExpressionParser.recursionDepth; - ExpressionLexer outerLexer = this.UriQueryExpressionParser.Lexer; - if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.OpenParen) { return new CountSegmentToken(countedInstance); } else { + // advance past the '(' this.lexer.NextToken(); - isFilterOption = this.lexer.CurrentToken.IdentifierIs(ExpressionConstants.QueryOptionFilter, this.UriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier); - isSearchOption = this.lexer.CurrentToken.IdentifierIs(ExpressionConstants.QueryOptionSearch, this.UriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier); - this.lexer.NextToken(); - - if (isFilterOption) + // Check for (), which is not allowed. + if (this.lexer.CurrentToken.Kind == ExpressionTokenKind.CloseParen) { - filterText = this.lexer.AdvanceThroughExpandOption(); - filterToken = filterText == null ? null : this.UriQueryExpressionParser.ParseFilter(filterText); + throw new ODataException(ODataErrorStrings.UriParser_EmptyParenthesis()); } - else if(isSearchOption) - { - searchText = this.lexer.AdvanceThroughExpandOption(); - SearchParser searchParser = new SearchParser(ODataUriParserSettings.DefaultSearchLimit); - searchToken = searchText == null ? null : searchParser.ParseSearch(searchText); - } - else - { - // Only a $filter and $search can be inside a $count() - throw ParseError(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); - } - } - - if (this.lexer.CurrentToken.Kind == ExpressionTokenKind.SemiColon) - { - this.lexer.NextToken(); - isFilterOption = this.lexer.CurrentToken.IdentifierIs(ExpressionConstants.QueryOptionFilter, this.UriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier); - isSearchOption = this.lexer.CurrentToken.IdentifierIs(ExpressionConstants.QueryOptionSearch, this.UriQueryExpressionParser.enableCaseInsensitiveBuiltinIdentifier); - this.lexer.NextToken(); - - if (isFilterOption) + // Look for all the supported query options + while (this.lexer.CurrentToken.Kind != ExpressionTokenKind.CloseParen) { - filterText = this.lexer.AdvanceThroughExpandOption(); - filterToken = filterText == null ? null : this.UriQueryExpressionParser.ParseFilter(filterText); - } - else if (isSearchOption) - { - searchText = this.lexer.AdvanceThroughExpandOption(); - SearchParser searchParser = new SearchParser(ODataUriParserSettings.DefaultSearchLimit); - searchToken = searchText == null ? null : searchParser.ParseSearch(searchText); - } - else - { - // Only a $filter and $search can be inside a $count() - throw ParseError(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); + string text = this.UriQueryExpressionParser.EnableCaseInsensitiveBuiltinIdentifier + ? this.lexer.CurrentToken.Text.ToLowerInvariant() + : this.lexer.CurrentToken.Text; + + switch (text) + { + case ExpressionConstants.QueryOptionFilter: // $filter within $count segment + filterToken = ParseInnerFilter(); + break; + + case ExpressionConstants.QueryOptionSearch: // $search within $count segment + searchToken = ParseInnerSearch(); + break; + + default: + { + throw new ODataException(ODataErrorStrings.UriQueryExpressionParser_IllegalQueryOptioninDollarCount()); + } + } } } - - this.lexer.ValidateToken(ExpressionTokenKind.CloseParen); - + // Move past the ')' this.lexer.NextToken(); - this.UriQueryExpressionParser.recursionDepth = outerRecursiveDepth; - this.UriQueryExpressionParser.Lexer = outerLexer; return new CountSegmentToken(countedInstance, filterToken, searchToken); } /// - /// + /// Parse the filter option in the $count segment. /// - /// - /// - /// - private static string TryGetQueryOption(string queryOptionName, string query) + /// The filter option for the $count segment. + private QueryToken ParseInnerFilter() { - if (queryOptionName == null) - { - return null; - } + // advance to the equal sign + this.lexer.NextToken(); + string filterText = this.ReadQueryOption(); - // Concat $filter or $search with an = symbol to get $filter= or $search= - string queryOption = string.Concat(queryOptionName, ExpressionConstants.SymbolEqual); + UriQueryExpressionParser filterParser = new UriQueryExpressionParser(ODataUriParserSettings.DefaultFilterLimit, this.UriQueryExpressionParser.EnableCaseInsensitiveBuiltinIdentifier); + return filterParser.ParseFilter(filterText); + } - char[] trimmingChars = queryOption.ToCharArray(); + /// + /// Parse the search option in the $count segment. + /// + /// The search option for the $count segment. + private QueryToken ParseInnerSearch() + { + // advance to the equal sign + this.lexer.NextToken(); + string searchText = this.ReadQueryOption(); - return query.TrimStart(trimmingChars); + SearchParser searchParser = new SearchParser(ODataUriParserSettings.DefaultSearchLimit); + return searchParser.ParseSearch(searchText); } - /// Creates an exception for a parse error. - /// Message text. - /// A new Exception. - private static Exception ParseError(string message) + /// + /// Read a query option within the $count brackets. + /// + /// The query option as a string. + private string ReadQueryOption() { - return new ODataException(message); + // $filter or $search should be followed by an equal sign. + // e.g $filter= or $search= + if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.Equal) + { + throw new ODataException(ODataErrorStrings.ExpressionLexer_SyntaxError(this.lexer.Position, this.lexer.ExpressionText)); + } + + string expressionText = this.lexer.AdvanceThroughExpandOption(); + + if (this.lexer.CurrentToken.Kind == ExpressionTokenKind.SemiColon) + { + // Move over the ';' separator + this.lexer.NextToken(); + return expressionText; + } + + // If there wasn't a semicolon, it MUST be the last option. We must be at ')' in this case + this.lexer.ValidateToken(ExpressionTokenKind.CloseParen); + return expressionText; } } } diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index 5e3f3299c9..59c7511e40 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -44,7 +44,7 @@ public sealed class UriQueryExpressionParser /// /// The current recursion depth. /// - internal int recursionDepth; + private int recursionDepth; /// /// The lexer being used for the parsing. @@ -54,7 +54,7 @@ public sealed class UriQueryExpressionParser /// /// Whether to allow case insensitive for builtin identifier. /// - internal bool enableCaseInsensitiveBuiltinIdentifier = false; + private bool enableCaseInsensitiveBuiltinIdentifier = false; /// /// Tracks the depth of aggregate expression recursion. @@ -114,6 +114,14 @@ internal ExpressionLexer Lexer set { this.lexer = value; } } + /// + /// Reference to the lexer. + /// + internal bool EnableCaseInsensitiveBuiltinIdentifier + { + get { return this.enableCaseInsensitiveBuiltinIdentifier; } + } + /// /// Gets if this parser is currently within an aggregate expression parsing stack. /// diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index cb48b2ed52..7477f086b5 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -426,6 +426,13 @@ public void ParseFilterWithEntityCollectionCountWithUnbalanceParenthesisThrows() parse.Throws(ODataErrorStrings.ExpressionLexer_SyntaxError(50, "MyFriendsDogs/$count($filter=Color eq 'Brown' gt 1")); } + [Fact] + public void ParseFilterWithEntityCollectionCountWithEmptyParenthesisThrows() + { + Action parse = () => ParseFilter("MyFriendsDogs/$count()", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + parse.Throws(ODataErrorStrings.UriParser_EmptyParenthesis()); + } + [Fact] public void ParseFilterWithEntityCollectionCountWithIllegalQueryOptionThrows() { From 5f437ca32e488a02d934831c850e2f25adf3f9e4 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Fri, 7 May 2021 00:11:34 +0300 Subject: [PATCH 16/19] Remove unnecessary code --- .../UriParser/Parsers/UriQueryExpressionParser.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index 59c7511e40..5cabfd48ed 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -111,7 +111,6 @@ internal UriQueryExpressionParser(int maxDepth, ExpressionLexer lexer) : this(ma internal ExpressionLexer Lexer { get { return this.lexer; } - set { this.lexer = value; } } /// @@ -143,16 +142,6 @@ public QueryToken ParseFilter(string filter) return this.ParseExpressionText(filter); } - /// - /// Parses the $search expression. - /// - /// The $search expression string to parse. - /// The lexical token representing the filter. - public QueryToken ParseSearch(string search) - { - return this.ParseExpressionText(search); - } - /// /// Parses a literal. /// From 673ec5f95e8eaa65d5b449fe37b840ea4edd4ca4 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Fri, 7 May 2021 01:05:59 +0300 Subject: [PATCH 17/19] Throw exception when NextToken is not a collection --- src/Microsoft.OData.Core/Microsoft.OData.Core.cs | 1 + src/Microsoft.OData.Core/Microsoft.OData.Core.txt | 1 + .../Parameterized.Microsoft.OData.Core.cs | 8 ++++++++ .../UriParser/Binders/CountSegmentBinder.cs | 5 +++++ .../UriParser/FilterAndOrderByFunctionalTests.cs | 7 +++++++ 5 files changed, 22 insertions(+) diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs index 67b1393b42..c1176419e1 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs @@ -633,6 +633,7 @@ internal sealed class TextRes { internal const string MetadataBinder_LeftOperandNotSingleValue = "MetadataBinder_LeftOperandNotSingleValue"; internal const string MetadataBinder_RightOperandNotCollectionValue = "MetadataBinder_RightOperandNotCollectionValue"; internal const string MetadataBinder_PropertyAccessSourceNotSingleValue = "MetadataBinder_PropertyAccessSourceNotSingleValue"; + internal const string MetadataBinder_CountSegmentNextTokenNotCollectionValue = "MetadataBinder_CountSegmentNextTokenNotCollectionValue"; internal const string MetadataBinder_IncompatibleOperandsError = "MetadataBinder_IncompatibleOperandsError"; internal const string MetadataBinder_IncompatibleOperandError = "MetadataBinder_IncompatibleOperandError"; internal const string MetadataBinder_UnknownFunction = "MetadataBinder_UnknownFunction"; diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt index 9e574af303..61d00987a5 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt @@ -729,6 +729,7 @@ MetadataBinder_UnaryOperatorOperandNotSingleValue=The operand for a unary operat MetadataBinder_LeftOperandNotSingleValue=The left operand for the IN operation is not a single value. IN operations require the left operand to be a single value and the right operand to be a collection value. MetadataBinder_RightOperandNotCollectionValue=The right operand for the IN operation is not a collection value. IN operations require the left operand to be a single value and the right operand to be a collection value. MetadataBinder_PropertyAccessSourceNotSingleValue=The parent value for a property access of a property '{0}' is not a single value. Property access can only be applied to a single value. +MetadataBinder_CountSegmentNextTokenNotCollectionValue=The next token in a CountSegmentNode must be a collection. MetadataBinder_IncompatibleOperandsError=A binary operator with incompatible types was detected. Found operand types '{0}' and '{1}' for operator kind '{2}'. MetadataBinder_IncompatibleOperandError=A unary operator with an incompatible type was detected. Found operand type '{0}' for operator kind '{1}'. MetadataBinder_UnknownFunction=An unknown function with name '{0}' was found. This may also be a function import or a key lookup on a navigation property, which is not allowed. diff --git a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs index 40d801684e..acc2b304f7 100644 --- a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs @@ -5503,6 +5503,14 @@ internal static string MetadataBinder_PropertyAccessSourceNotSingleValue(object return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.MetadataBinder_PropertyAccessSourceNotSingleValue, p0); } + /// + /// A string like "The next token in a CountSegmentNode must be a collection." + /// + internal static string MetadataBinder_CountSegmentNextTokenNotCollectionValue() + { + return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.MetadataBinder_CountSegmentNextTokenNotCollectionValue); + } + /// /// A string like "A binary operator with incompatible types was detected. Found operand types '{0}' and '{1}' for operator kind '{2}'." /// diff --git a/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs index 96f30ac2e3..d7f128beb5 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/CountSegmentBinder.cs @@ -47,6 +47,11 @@ internal QueryNode BindCountSegment(CountSegmentToken countSegmentToken) QueryNode source = this.bindMethod(countSegmentToken.NextToken); CollectionNode node = source as CollectionNode; + if(node == null) + { + throw new ODataException(ODataErrorStrings.MetadataBinder_CountSegmentNextTokenNotCollectionValue()); + } + FilterClause filterClause = null; SearchClause searchClause = null; diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index 7477f086b5..276455fdb7 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -467,6 +467,13 @@ public void ParseFilterWithEntityCollectionCountWithFilterAndSearchOptions() Assert.Equal("Brown", Assert.IsType(innerBinaryNode.Right).Value); } + [Fact] + public void ParseFilterWithSingleValueCountWithFilterAndSearchOptionsThrows() + { + Action parse = () => ParseFilter("ID/$count($filter=Color eq 'Brown';$search=brown) gt 1", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + parse.Throws(ODataErrorStrings.MetadataBinder_CountSegmentNextTokenNotCollectionValue()); + } + [Fact] public void CompareComplexWithNull() { From 4853d1739f0ab4c00ed318be62f6cb8a4cc923af Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Fri, 7 May 2021 10:16:16 +0300 Subject: [PATCH 18/19] Enable nodollarquery options --- .../UriParser/ODataQueryOptionParser.cs | 2 +- .../UriParser/Parsers/CountSegmentParser.cs | 7 +++++ .../Parsers/UriQueryExpressionParser.cs | 27 ++++++++++++++++++- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/ODataQueryOptionParser.cs b/src/Microsoft.OData.Core/UriParser/ODataQueryOptionParser.cs index 8d1675e125..9b1033eba8 100644 --- a/src/Microsoft.OData.Core/UriParser/ODataQueryOptionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/ODataQueryOptionParser.cs @@ -384,7 +384,7 @@ private FilterClause ParseFilterImplementation(string filter, ODataUriParserConf ExceptionUtils.CheckArgumentNotNull(filter, "filter"); // Get the syntactic representation of the filter expression - UriQueryExpressionParser expressionParser = new UriQueryExpressionParser(configuration.Settings.FilterLimit, configuration.EnableCaseInsensitiveUriFunctionIdentifier); + UriQueryExpressionParser expressionParser = new UriQueryExpressionParser(configuration.Settings.FilterLimit, configuration.EnableCaseInsensitiveUriFunctionIdentifier, configuration.Resolver.EnableNoDollarQueryOptions); QueryToken filterToken = expressionParser.ParseFilter(filter); // Bind it to metadata diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index f688b21045..fa17535281 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -7,6 +7,7 @@ namespace Microsoft.OData.UriParser { using System; + using System.Globalization; using ODataErrorStrings = Microsoft.OData.Strings; /// @@ -84,6 +85,12 @@ public CountSegmentToken CreateCountSegmentToken(QueryToken countedInstance) ? this.lexer.CurrentToken.Text.ToLowerInvariant() : this.lexer.CurrentToken.Text; + // Prepend '$' prefix if needed. + if (this.UriQueryExpressionParser.EnableNoDollarQueryOptions && !text.StartsWith(UriQueryConstants.DollarSign, StringComparison.Ordinal)) + { + text = string.Format(CultureInfo.InvariantCulture, "{0}{1}", UriQueryConstants.DollarSign, text); + } + switch (text) { case ExpressionConstants.QueryOptionFilter: // $filter within $count segment diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs index 5cabfd48ed..a70fc65914 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs @@ -56,6 +56,11 @@ public sealed class UriQueryExpressionParser /// private bool enableCaseInsensitiveBuiltinIdentifier = false; + /// + /// Whether to allow no-dollar query options. + /// + private bool enableNoDollarQueryOptions = false; + /// /// Tracks the depth of aggregate expression recursion. /// @@ -81,11 +86,23 @@ public UriQueryExpressionParser(int maxDepth) /// The maximum depth of each part of the query - a recursion limit. /// Whether to allow case insensitive for builtin identifier. internal UriQueryExpressionParser(int maxDepth, bool enableCaseInsensitiveBuiltinIdentifier = false) + : this(maxDepth, enableCaseInsensitiveBuiltinIdentifier, false) + { + } + + /// + /// Constructor. + /// + /// The maximum depth of each part of the query - a recursion limit. + /// Whether to allow case insensitive for builtin identifier. + /// Whether to allow no-dollar query options. + internal UriQueryExpressionParser(int maxDepth, bool enableCaseInsensitiveBuiltinIdentifier = false, bool enableNoDollarQueryOptions = false) { Debug.Assert(maxDepth >= 0, "maxDepth >= 0"); this.maxDepth = maxDepth; this.enableCaseInsensitiveBuiltinIdentifier = enableCaseInsensitiveBuiltinIdentifier; + this.enableNoDollarQueryOptions = enableNoDollarQueryOptions; } /// @@ -114,13 +131,21 @@ internal ExpressionLexer Lexer } /// - /// Reference to the lexer. + /// Reference to the enableCaseInsensitiveBuiltinIdentifier. /// internal bool EnableCaseInsensitiveBuiltinIdentifier { get { return this.enableCaseInsensitiveBuiltinIdentifier; } } + /// + /// Reference to the enableNoDollarQueryOptions. + /// + internal bool EnableNoDollarQueryOptions + { + get { return this.enableNoDollarQueryOptions; } + } + /// /// Gets if this parser is currently within an aggregate expression parsing stack. /// From 5f1dff25453a1bb4711d1f24c723cb607ce71d74 Mon Sep 17 00:00:00 2001 From: Kennedy Kangethe Date: Mon, 10 May 2021 10:10:37 +0300 Subject: [PATCH 19/19] Move ReadQueryOption to UriParserHelper --- .../UriParser/Parsers/CountSegmentParser.cs | 31 +----------- .../Parsers/SelectExpandOptionParser.cs | 50 ++++--------------- .../UriParser/Parsers/UriParserHelper.cs | 28 +++++++++++ 3 files changed, 41 insertions(+), 68 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs index fa17535281..b67049603d 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs @@ -122,7 +122,7 @@ private QueryToken ParseInnerFilter() { // advance to the equal sign this.lexer.NextToken(); - string filterText = this.ReadQueryOption(); + string filterText = UriParserHelper.ReadQueryOption(this.lexer); UriQueryExpressionParser filterParser = new UriQueryExpressionParser(ODataUriParserSettings.DefaultFilterLimit, this.UriQueryExpressionParser.EnableCaseInsensitiveBuiltinIdentifier); return filterParser.ParseFilter(filterText); @@ -136,37 +136,10 @@ private QueryToken ParseInnerSearch() { // advance to the equal sign this.lexer.NextToken(); - string searchText = this.ReadQueryOption(); + string searchText = UriParserHelper.ReadQueryOption(this.lexer); SearchParser searchParser = new SearchParser(ODataUriParserSettings.DefaultSearchLimit); return searchParser.ParseSearch(searchText); } - - /// - /// Read a query option within the $count brackets. - /// - /// The query option as a string. - private string ReadQueryOption() - { - // $filter or $search should be followed by an equal sign. - // e.g $filter= or $search= - if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.Equal) - { - throw new ODataException(ODataErrorStrings.ExpressionLexer_SyntaxError(this.lexer.Position, this.lexer.ExpressionText)); - } - - string expressionText = this.lexer.AdvanceThroughExpandOption(); - - if (this.lexer.CurrentToken.Kind == ExpressionTokenKind.SemiColon) - { - // Move over the ';' separator - this.lexer.NextToken(); - return expressionText; - } - - // If there wasn't a semicolon, it MUST be the last option. We must be at ')' in this case - this.lexer.ValidateToken(ExpressionTokenKind.CloseParen); - return expressionText; - } } } diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/SelectExpandOptionParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/SelectExpandOptionParser.cs index d7ad4ff6c7..9139c23806 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/SelectExpandOptionParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/SelectExpandOptionParser.cs @@ -438,7 +438,7 @@ private QueryToken ParseInnerFilter() { // advance to the equal sign this.lexer.NextToken(); - string filterText = this.ReadQueryOption(); + string filterText = UriParserHelper.ReadQueryOption(this.lexer); UriQueryExpressionParser filterParser = new UriQueryExpressionParser(this.MaxFilterDepth, enableCaseInsensitiveBuiltinIdentifier); return filterParser.ParseFilter(filterText); @@ -452,7 +452,7 @@ private IEnumerable ParseInnerOrderBy() { // advance to the equal sign this.lexer.NextToken(); - string orderByText = this.ReadQueryOption(); + string orderByText = UriParserHelper.ReadQueryOption(this.lexer); UriQueryExpressionParser orderbyParser = new UriQueryExpressionParser(this.MaxOrderByDepth, enableCaseInsensitiveBuiltinIdentifier); return orderbyParser.ParseOrderBy(orderByText); @@ -466,7 +466,7 @@ private IEnumerable ParseInnerOrderBy() { // advance to the equal sign this.lexer.NextToken(); - string topText = this.ReadQueryOption(); + string topText = UriParserHelper.ReadQueryOption(this.lexer); // TryParse requires a non-nullable non-negative long. long top; @@ -486,7 +486,7 @@ private IEnumerable ParseInnerOrderBy() { // advance to the equal sign this.lexer.NextToken(); - string skipText = this.ReadQueryOption(); + string skipText = UriParserHelper.ReadQueryOption(this.lexer); // TryParse requires a non-nullable non-negative long. long skip; @@ -506,7 +506,7 @@ private IEnumerable ParseInnerOrderBy() { // advance to the equal sign this.lexer.NextToken(); - string countText = this.ReadQueryOption(); + string countText = UriParserHelper.ReadQueryOption(this.lexer); switch (countText) { case ExpressionConstants.KeywordTrue: @@ -528,7 +528,7 @@ private QueryToken ParseInnerSearch() { // advance to the equal sign this.lexer.NextToken(); - string searchText = this.ReadQueryOption(); + string searchText = UriParserHelper.ReadQueryOption(this.lexer); SearchParser searchParser = new SearchParser(this.MaxSearchDepth); return searchParser.ParseSearch(searchText); @@ -543,7 +543,7 @@ private SelectToken ParseInnerSelect(PathSegmentToken pathToken) { // advance to the equal sign this.lexer.NextToken(); - string selectText = this.ReadQueryOption(); + string selectText = UriParserHelper.ReadQueryOption(this.lexer); IEdmStructuredType targetStructuredType = null; if (this.resolver != null && this.parentStructuredType != null) @@ -579,7 +579,7 @@ private ExpandToken ParseInnerExpand(PathSegmentToken pathToken) // advance to the equal sign this.lexer.NextToken(); - string expandText = this.ReadQueryOption(); + string expandText = UriParserHelper.ReadQueryOption(this.lexer); IEdmStructuredType targetStructuredType = null; if (this.resolver != null && this.parentStructuredType != null) @@ -617,7 +617,7 @@ private ExpandToken ParseInnerExpand(PathSegmentToken pathToken) // advance to the equal sign this.lexer.NextToken(); - string levelsText = this.ReadQueryOption(); + string levelsText = UriParserHelper.ReadQueryOption(this.lexer); long level; if (string.Equals( @@ -646,7 +646,7 @@ private ExpandToken ParseInnerExpand(PathSegmentToken pathToken) private ComputeToken ParseInnerCompute() { this.lexer.NextToken(); - string computeText = this.ReadQueryOption(); + string computeText = UriParserHelper.ReadQueryOption(this.lexer); UriQueryExpressionParser computeParser = new UriQueryExpressionParser(this.MaxOrderByDepth, enableCaseInsensitiveBuiltinIdentifier); return computeParser.ParseCompute(computeText); @@ -659,38 +659,10 @@ private ComputeToken ParseInnerCompute() private IEnumerable ParseInnerApply() { this.lexer.NextToken(); - string applyText = this.ReadQueryOption(); + string applyText = UriParserHelper.ReadQueryOption(this.lexer); UriQueryExpressionParser applyParser = new UriQueryExpressionParser(this.MaxOrderByDepth, enableCaseInsensitiveBuiltinIdentifier); return applyParser.ParseApply(applyText); } - - /// - /// Read a query option from the lexer. - /// - /// The query option as a string. - private string ReadQueryOption() - { - if (this.lexer.CurrentToken.Kind != ExpressionTokenKind.Equal) - { - throw new ODataException(ODataErrorStrings.UriSelectParser_TermIsNotValid(this.lexer.ExpressionText)); - } - - // get the full text from the current location onward - // there could be literals like 'A string literal; tricky!' in there, so we need to be careful. - // Also there could be more nested (...) expressions that we ignore until we recurse enough times to get there. - string expressionText = this.lexer.AdvanceThroughExpandOption(); - - if (this.lexer.CurrentToken.Kind == ExpressionTokenKind.SemiColon) - { - // Move over the ';' separator - this.lexer.NextToken(); - return expressionText; - } - - // If there wasn't a semicolon, it MUST be the last option. We must be at ')' in this case - this.lexer.ValidateToken(ExpressionTokenKind.CloseParen); - return expressionText; - } } } \ No newline at end of file diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/UriParserHelper.cs b/src/Microsoft.OData.Core/UriParser/Parsers/UriParserHelper.cs index c26d8b2b00..5eb73718af 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/UriParserHelper.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/UriParserHelper.cs @@ -271,6 +271,34 @@ internal static bool IsAnnotation(string identifier) return !string.IsNullOrEmpty(identifier) && identifier[0] == UriQueryConstants.AnnotationPrefix && identifier.Contains("."); } + /// + /// Read a query option from the lexer. + /// + /// The query option as a string. + internal static string ReadQueryOption(ExpressionLexer lexer) + { + if (lexer.CurrentToken.Kind != ExpressionTokenKind.Equal) + { + throw new ODataException(ODataErrorStrings.UriSelectParser_TermIsNotValid(lexer.ExpressionText)); + } + + // get the full text from the current location onward + // there could be literals like 'A string literal; tricky!' in there, so we need to be careful. + // Also there could be more nested (...) expressions that we ignore until we recurse enough times to get there. + string expressionText = lexer.AdvanceThroughExpandOption(); + + if (lexer.CurrentToken.Kind == ExpressionTokenKind.SemiColon) + { + // Move over the ';' separator + lexer.NextToken(); + return expressionText; + } + + // If there wasn't a semicolon, it MUST be the last option. We must be at ')' in this case + lexer.ValidateToken(ExpressionTokenKind.CloseParen); + return expressionText; + } + #endregion #region Private Methods