From c31d6835fdffec87ceb6aded6756b3f975fc059f Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Tue, 3 Oct 2023 15:29:47 -0700 Subject: [PATCH] Fixes #2754, enable key in parenthesis after composable function with parameters --- .../Parsers/FunctionParameterParser.cs | 76 +++++++++++++++++++ .../UriParser/Parsers/ODataPathParser.cs | 46 +++++++++-- .../UriParser/PathFunctionalTests.cs | 9 ++- .../UriParser/HardCodedTestModel.cs | 4 +- .../Parsers/FunctionParameterParserTests.cs | 13 ++++ .../UriParser/Parsers/ODataPathParserTests.cs | 24 ++++++ 6 files changed, 160 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/FunctionParameterParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/FunctionParameterParser.cs index 504ad0f129..b6dbbccff6 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/FunctionParameterParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/FunctionParameterParser.cs @@ -50,6 +50,82 @@ internal static bool TrySplitOperationParameters(string parenthesisExpression, O return ret; } + /// + /// Splits the parenthesis expression into two parts (if apply) + /// One is the function parameter, the other is key in parenthesis (if exists) + /// Be noted, the input expression doesn't contain the beginning "(" and the ending ")" + /// + /// the input expression + /// the output for parameter part + /// the output for key in parenthesis part + internal static void SplitOperationParametersAndParenthesisKey(string parenthesisExpression, out string parameters, out string parenthesisKey) + { + // Be noted, the input expression doesn't contain the first '(' and last ')'. + // for example + // "degree=')')('fawn'" + // ")('fawn'" ==> empty parameter with a key in parenthesis + parameters = parenthesisExpression; + parenthesisKey = null; + + if (string.IsNullOrEmpty(parenthesisExpression)) + { + return; + } + + Stack stack = new Stack(); + stack.Push(ExpressionTokenKind.OpenParen); + ExpressionLexer lexer = new ExpressionLexer(parenthesisExpression, true /*moveToFirstToken*/, false /*useSemicolonDelimiter*/, true /*parsingFunctionParameters*/); + bool paramertersFound = false; + bool parenthesisKeyFound = false; + int parenthesisKeyStartPosition = 0; + ExpressionToken currentToken = lexer.CurrentToken; + while (true) + { + if (currentToken.Kind == ExpressionTokenKind.OpenParen) + { + if (stack.Count == 0) + { + parenthesisKeyStartPosition = currentToken.Position; + } + + stack.Push(ExpressionTokenKind.OpenParen); + } + else if (currentToken.Kind == ExpressionTokenKind.CloseParen || currentToken.Kind == ExpressionTokenKind.End) + { + if (stack.Count == 1) // It's a top level + { + if (!paramertersFound) + { + parameters = parenthesisExpression.Substring(0, currentToken.Position); + paramertersFound = true; + } + else if (!parenthesisKeyFound) + { + parenthesisKeyFound = true; + } + else + { + throw new ODataException(ODataErrorStrings.ExpressionLexer_SyntaxError(currentToken.Position, parenthesisExpression)); + } + } + + stack.Pop(); // match an embeded '()' + } + + if (currentToken.Kind == ExpressionTokenKind.End) + { + break; + } + + currentToken = lexer.NextToken(); + } + + if (parenthesisKeyFound) + { + parenthesisKey = parenthesisExpression.Substring(parenthesisKeyStartPosition + 1);// +1 means to remove the leading '(' + } + } + /// /// Tries to parse a collection of function parameters. Allows path and filter to share the core algorithm while representing parameters differently. /// diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/ODataPathParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/ODataPathParser.cs index 0924034af2..cec4a909d8 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/ODataPathParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/ODataPathParser.cs @@ -1053,6 +1053,11 @@ private bool TryCreateSegmentForNavigationSource(string identifier, string paren [SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "IEdmModel", Justification = "The spelling is correct.")] private bool TryCreateSegmentForOperationImport(string identifier, string parenthesisExpression) { + FunctionParameterParser.SplitOperationParametersAndParenthesisKey(parenthesisExpression, + out string newParenthesisParameters, + out string parenthesisKey); + parenthesisExpression = newParenthesisParameters; + ICollection resolvedParameters; IEdmOperationImport singleImport; if (!TryBindingParametersAndMatchingOperationImport(identifier, parenthesisExpression, this.configuration, out resolvedParameters, out singleImport)) @@ -1072,7 +1077,14 @@ private bool TryCreateSegmentForOperationImport(string identifier, string parent this.parsedSegments.Add(segment); - this.TryBindKeySegmentIfNoResolvedParametersAndParenthesisValueExists(parenthesisExpression, returnType, resolvedParameters, segment); + // Be noted, it's back-compatibile since the function can be called without parameters but with keys + // for example: "~/GetCoolPeople(1)", where '1' is the key, not the parameters. + if (parenthesisKey == null && resolvedParameters == null) + { + parenthesisKey = parenthesisExpression; + } + + this.TryBindKeySegmentIfNoResolvedParametersAndParenthesisValueExists(parenthesisKey, returnType, segment); return true; } @@ -1082,12 +1094,11 @@ private bool TryCreateSegmentForOperationImport(string identifier, string parent /// /// The parenthesis expression. /// Type of the return. - /// The resolved parameters. /// The segment. - private void TryBindKeySegmentIfNoResolvedParametersAndParenthesisValueExists(string parenthesisExpression, IEdmTypeReference returnType, ICollection resolvedParameters, ODataPathSegment segment) + private void TryBindKeySegmentIfNoResolvedParametersAndParenthesisValueExists(string parenthesisExpression, IEdmTypeReference returnType, ODataPathSegment segment) { IEdmCollectionTypeReference collectionTypeReference = returnType as IEdmCollectionTypeReference; - if (collectionTypeReference != null && collectionTypeReference.ElementType().IsEntity() && resolvedParameters == null && parenthesisExpression != null) + if (collectionTypeReference != null && collectionTypeReference.ElementType().IsEntity() && parenthesisExpression != null) { // The parameters in the parenthesis is a key segment. if (this.TryBindKeyFromParentheses(parenthesisExpression)) @@ -1115,6 +1126,11 @@ private bool TryCreateSegmentForOperation(ODataPathSegment previousSegment, stri bindingType = (previousSegment is EachSegment) ? previousSegment.TargetEdmType : previousSegment.EdmType; } + FunctionParameterParser.SplitOperationParametersAndParenthesisKey(parenthesisExpression, + out string newParenthesisParameters, + out string parenthesisKey); + parenthesisExpression = newParenthesisParameters; + ICollection resolvedParameters; IEdmOperation singleOperation; if (!TryBindingParametersAndMatchingOperation(identifier, parenthesisExpression, bindingType, this.configuration, out resolvedParameters, out singleOperation)) @@ -1132,12 +1148,14 @@ private bool TryCreateSegmentForOperation(ODataPathSegment previousSegment, stri throw new ODataException(ODataErrorStrings.FunctionCallBinder_CallingFunctionOnOpenProperty(identifier)); } - CreateOperationSegment(previousSegment, singleOperation, resolvedParameters, identifier, parenthesisExpression); + CreateOperationSegment(previousSegment, singleOperation, resolvedParameters, identifier, parenthesisExpression, parenthesisKey); return true; } - private void CreateOperationSegment(ODataPathSegment previousSegment, IEdmOperation singleOperation, ICollection resolvedParameters, string identifier, string parenthesisExpression) + private void CreateOperationSegment(ODataPathSegment previousSegment, IEdmOperation singleOperation, + ICollection resolvedParameters, + string identifier, string parenthesisExpression, string parenthesisKey) { IEdmTypeReference returnType = singleOperation.ReturnType; IEdmEntitySetBase targetset = null; @@ -1162,7 +1180,15 @@ private void CreateOperationSegment(ODataPathSegment previousSegment, IEdmOperat }; this.parsedSegments.Add(segment); - this.TryBindKeySegmentIfNoResolvedParametersAndParenthesisValueExists(parenthesisExpression, returnType, resolvedParameters, segment); + + // Be noted, it's back-compatibile since the function can be called without parameters but with keys + // for example: "~/GetCoolPeople(1)", where '1' is the key, not the parameters. + if (parenthesisKey == null && resolvedParameters == null) + { + parenthesisKey = parenthesisExpression; + } + + this.TryBindKeySegmentIfNoResolvedParametersAndParenthesisValueExists(parenthesisKey, returnType, segment); return; } @@ -1342,10 +1368,14 @@ private bool TryBindEscapeFunction() IEdmFunction escapeFunction; if (this.TryResolveEscapeFunction(previous, out newIdentifier, out newParenthesisExpression, out anotherEscapeFunctionStarts, out escapeFunction)) { + FunctionParameterParser.SplitOperationParametersAndParenthesisKey(newParenthesisExpression, + out string newParenthesisParameters, out string parenthesisKey); + newParenthesisExpression = newParenthesisParameters; + ICollection splitParameters; FunctionParameterParser.TrySplitOperationParameters(newParenthesisExpression, configuration, out splitParameters); ICollection resolvedParameters = FunctionCallBinder.BindSegmentParameters(configuration, escapeFunction, splitParameters); - CreateOperationSegment(previous, escapeFunction, resolvedParameters, newIdentifier, newParenthesisExpression); + CreateOperationSegment(previous, escapeFunction, resolvedParameters, newIdentifier, newParenthesisExpression, parenthesisKey); if (anotherEscapeFunctionStarts) { // When we encounter an invalid escape function as a parameter, we should throw. diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/PathFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/PathFunctionalTests.cs index 8753bba147..30521fecc8 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/PathFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/PathFunctionalTests.cs @@ -1047,9 +1047,14 @@ public void KeyLookupCanAppearAfterComposableFunctionWithoutParameters() } [Fact] - public void KeyLookupCannotAppearAfterFunctionWithParameters() + public void KeyLookupCanAppearAfterFunctionWithParameters() { - PathFunctionalTestsUtil.RunParseErrorPath("People(1)/Fully.Qualified.Namespace.AllMyFriendsDogs(inOffice=true)(1)", ODataErrorStrings.ExpressionLexer_SyntaxError(14, "inOffice=true)(1")); + var path = PathFunctionalTestsUtil.RunParsePath("People(1)/Fully.Qualified.Namespace.AllMyFriendsDogs(inOffice=true)(42)"); + Assert.Equal(4, path.Count); + path.Segments[0].ShouldBeEntitySetSegment(HardCodedTestModel.GetPeopleSet()); + path.Segments[1].ShouldBeKeySegment(new KeyValuePair("ID", 1)); + path.Segments[2].ShouldBeOperationSegment(HardCodedTestModel.GetFunctionForAllMyFriendsDogs(2)); + path.Segments[3].ShouldBeKeySegment(new KeyValuePair("ID", 42)); } [Fact] diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/HardCodedTestModel.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/HardCodedTestModel.cs index e52f70e281..0614f9bd0e 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/HardCodedTestModel.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/HardCodedTestModel.cs @@ -2232,9 +2232,9 @@ public static IEdmFunctionImport GetFunctionImportIsAddressGood() return TestModel.EntityContainer.FindOperationImports("IsAddressGood").Single() as IEdmFunctionImport; } - public static IEdmFunction GetFunctionForAllMyFriendsDogs() + public static IEdmFunction GetFunctionForAllMyFriendsDogs(int parameterCount = 1) { - return TestModel.FindOperations("Fully.Qualified.Namespace.AllMyFriendsDogs").Single(f => f.Parameters.Count() == 1) as IEdmFunction; + return TestModel.FindOperations("Fully.Qualified.Namespace.AllMyFriendsDogs").Single(f => f.Parameters.Count() == parameterCount) as IEdmFunction; } public static IEdmOperationImport[] GetAllFunctionImportsForGetMostImportantPerson() diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/FunctionParameterParserTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/FunctionParameterParserTests.cs index 8708fe1a5b..673448f1fb 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/FunctionParameterParserTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/FunctionParameterParserTests.cs @@ -21,6 +21,19 @@ namespace Microsoft.OData.Tests.UriParser.Parsers /// public class FunctionParameterParserTests { + [Theory] + [InlineData("1", "1", null)] + [InlineData(")(1", "", "1")] + [InlineData("person=People(1)", "person=People(1)", null)] + [InlineData("person=People(1))('bca(aa('", "person=People(1)", "'bca(aa('")] + [InlineData("degree=1)('fawn'", "degree=1", "'fawn'")] + public void SplitOperationParametersAndParenthesisKey_WorksForInputExpression(string expression, string parameters, string keys) + { + FunctionParameterParser.SplitOperationParametersAndParenthesisKey(expression, out string acutalParams, out string actualKeys); + Assert.Equal(parameters, acutalParams); + Assert.Equal(keys, actualKeys); + } + [Fact] public void FunctionParameterParserShouldSupportUnresolvedAliasesInPath() { diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/ODataPathParserTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/ODataPathParserTests.cs index 71e2dad08a..e010ceb70c 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/ODataPathParserTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/ODataPathParserTests.cs @@ -800,6 +800,30 @@ public void ParseCountAfterServiceRootShouldFail() parsePath.Throws("The request URI is not valid, the segment $count cannot be applied to the root of the service."); } + [Fact] + public void ParseKeyInParenthesisAfterBoundFunctionReturnsCollectionOfEntitiesShouldWork() + { + // People(1)/Fully.Qualified.Namespace.AllMyFriendsDogs(inOffice = true)(42) + IList path = this.testSubject.ParsePath(new[] { "People(1)", "Fully.Qualified.Namespace.AllMyFriendsDogs(inOffice=true)(42)" }); + Assert.Equal(4, path.Count); + path[0].ShouldBeEntitySetSegment(HardCodedTestModel.GetPeopleSet()); + path[1].ShouldBeKeySegment(new KeyValuePair("ID", 1)); + path[2].ShouldBeOperationSegment(HardCodedTestModel.GetFunctionForAllMyFriendsDogs(2)); + path[3].ShouldBeKeySegment(new KeyValuePair("ID", 42)); + } + + [Fact] + public void ParseKeyAsSegmentAfterBoundFunctionReturnsCollectionOfEntitiesShouldWork() + { + // People(1)/Fully.Qualified.Namespace.AllMyFriendsDogs(inOffice = true)/42 + IList path = this.testSubject.ParsePath(new[] { "People(1)", "Fully.Qualified.Namespace.AllMyFriendsDogs(inOffice=true)", "42" }); + Assert.Equal(4, path.Count); + path[0].ShouldBeEntitySetSegment(HardCodedTestModel.GetPeopleSet()); + path[1].ShouldBeKeySegment(new KeyValuePair("ID", 1)); + path[2].ShouldBeOperationSegment(HardCodedTestModel.GetFunctionForAllMyFriendsDogs(2)); + path[3].ShouldBeKeySegment(new KeyValuePair("ID", 42)); + } + [Fact] public void ParseBoundFunctionWithTypeDefinitionAsParameterAndReturnTypeShouldWork() {