Fixes #2754, enable key in parenthesis after composable function with…

… parameters

src/Microsoft.OData.Core/UriParser/Parsers/FunctionParameterParser.cs

@@ -50,6 +50,82 @@ internal static bool TrySplitOperationParameters(string parenthesisExpression, O
             return ret;
         }
 
+        /// <summary>
+        /// 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 ")"
+        /// </summary>
+        /// <param name="parenthesisExpression">the input expression</param>
+        /// <param name="parameters">the output for parameter part</param>
+        /// <param name="parenthesisKey">the output for key in parenthesis part</param>
+        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<ExpressionTokenKind> stack = new Stack<ExpressionTokenKind>();
+            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 '('
+            }
+        }
+
         /// <summary>
         /// Tries to parse a collection of function parameters. Allows path and filter to share the core algorithm while representing parameters differently.
         /// </summary>

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<OperationSegmentParameter> 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
         /// </summary>
         /// <param name="parenthesisExpression">The parenthesis expression.</param>
         /// <param name="returnType">Type of the return.</param>
-        /// <param name="resolvedParameters">The resolved parameters.</param>
         /// <param name="segment">The segment.</param>
-        private void TryBindKeySegmentIfNoResolvedParametersAndParenthesisValueExists(string parenthesisExpression, IEdmTypeReference returnType, ICollection<OperationSegmentParameter> 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<OperationSegmentParameter> 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<OperationSegmentParameter> resolvedParameters, string identifier, string parenthesisExpression)
+        private void CreateOperationSegment(ODataPathSegment previousSegment, IEdmOperation singleOperation,
+            ICollection<OperationSegmentParameter> 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<FunctionParameterToken> splitParameters;
                 FunctionParameterParser.TrySplitOperationParameters(newParenthesisExpression, configuration, out splitParameters);
                 ICollection<OperationSegmentParameter> 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.

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<string, object>("ID", 1));
+            path.Segments[2].ShouldBeOperationSegment(HardCodedTestModel.GetFunctionForAllMyFriendsDogs(2));
+            path.Segments[3].ShouldBeKeySegment(new KeyValuePair<string, object>("ID", 42));
         }
 
         [Fact]

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()

test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/FunctionParameterParserTests.cs

@@ -21,6 +21,19 @@ namespace Microsoft.OData.Tests.UriParser.Parsers
     /// </summary>
     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()
         {

test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/ODataPathParserTests.cs

@@ -800,6 +800,30 @@ public void ParseCountAfterServiceRootShouldFail()
             parsePath.Throws<ODataUnrecognizedPathException>("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<ODataPathSegment> 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<string, object>("ID", 1));
+            path[2].ShouldBeOperationSegment(HardCodedTestModel.GetFunctionForAllMyFriendsDogs(2));
+            path[3].ShouldBeKeySegment(new KeyValuePair<string, object>("ID", 42));
+        }
+
+        [Fact]
+        public void ParseKeyAsSegmentAfterBoundFunctionReturnsCollectionOfEntitiesShouldWork()
+        {
+            // People(1)/Fully.Qualified.Namespace.AllMyFriendsDogs(inOffice = true)/42
+            IList<ODataPathSegment> 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<string, object>("ID", 1));
+            path[2].ShouldBeOperationSegment(HardCodedTestModel.GetFunctionForAllMyFriendsDogs(2));
+            path[3].ShouldBeKeySegment(new KeyValuePair<string, object>("ID", 42));
+        }
+
         [Fact]
         public void ParseBoundFunctionWithTypeDefinitionAsParameterAndReturnTypeShouldWork()
         {