Fixes to LaylaLiu, Issue OData#378 OData#430

- Usage of 'ExceptionUtils' - Typo fixes - lock 'TryGetCustomFunction' method
LaylaLiu · Jan 13, 2016 · 2d69557 · 2d69557
1 parent afd7a99
commit 2d69557
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 34 deletions.
diff --git a/src/Microsoft.OData.Core/UriParser/CustomUriFunctions.cs b/src/Microsoft.OData.Core/UriParser/CustomUriFunctions.cs
@@ -59,15 +59,8 @@ static CustomUriFunctions()
         public static void AddCustomUriFunction(string customFunctionName, FunctionSignatureWithReturnType newCustomFunctionSignature, bool overrideBuiltInFunction = false)
         {
             // Parameters validation
-            if (string.IsNullOrEmpty(customFunctionName))
-            {
-                throw Error.ArgumentNull("customFunctionName");
-            }
-
-            if (newCustomFunctionSignature == null)
-            {
-                throw Error.ArgumentNull("newCustomFunctionSignature");
-            }
+            ExceptionUtils.CheckArgumentStringNotNullOrEmpty(customFunctionName, "customFunctionName");
+            ExceptionUtils.CheckArgumentNotNull(newCustomFunctionSignature, "newCustomFunctionSignature");
 
             ValidateFunctionWithReturnType(newCustomFunctionSignature);
 
@@ -160,10 +153,7 @@ public static bool RemoveCustomUriFunction(string customFunctionName, FunctionSi
         /// <exception cref="ArgumentNullException">Arguments are null, or function signature return type is null</exception>
         public static bool RemoveCustomUriFunction(string customFunctionName)
         {
-            if (string.IsNullOrEmpty(customFunctionName))
-            {
-                throw Error.ArgumentNull("customFunctionName");
-            }
+            ExceptionUtils.CheckArgumentStringNotNullOrEmpty(customFunctionName, "customFunctionName");
 
             lock (Locker)
             {
@@ -185,7 +175,10 @@ internal static bool TryGetCustomFunction(string name, out FunctionSignatureWith
         {
             Debug.Assert(name != null, "name != null");
 
-            return CustomFunctions.TryGetValue(name, out signatures);
+            lock (Locker)
+            {
+                return CustomFunctions.TryGetValue(name, out signatures);
+            }
         }
 
         #endregion
@@ -215,7 +208,7 @@ private static void AddCustomFunction(string customFunctionName, FunctionSignatu
                 }
 
                 // Add the custom function as an overload to the same function name
-                // It overrides the BuiltIn function because search is first on CustomFunctions and the on BuiltInFunctions. Another option is to remove the BuiltIn function - seems like a worse option.
+                // It overrides the BuiltIn function because search is first on CustomFunctions and then on BuiltInFunctions. Another option is to remove the BuiltIn function - seems like a worse option.
                 CustomFunctions[customFunctionName] = 
                     existingCustomFunctionOverloads.Concat(new FunctionSignatureWithReturnType[] { newCustomFunctionSignature }).ToArray();
             }
@@ -237,9 +230,9 @@ private static bool AreFunctionsSignatureEqual(FunctionSignatureWithReturnType f
             }
 
             // Check if the arguments are equal
-            for (int argumnetIndex = 0; argumnetIndex < functionOne.ArgumentTypes.Length; argumnetIndex++)
+            for (int argumentIndex = 0; argumentIndex < functionOne.ArgumentTypes.Length; argumentIndex++)
             {
-                if (!functionOne.ArgumentTypes[argumnetIndex].IsEquivalentTo(functionTwo.ArgumentTypes[argumnetIndex]))
+                if (!functionOne.ArgumentTypes[argumentIndex].IsEquivalentTo(functionTwo.ArgumentTypes[argumentIndex]))
                 {
                     return false;
                 }
@@ -260,10 +253,7 @@ private static void ValidateFunctionWithReturnType(FunctionSignatureWithReturnTy
                 return;
             }
 
-            if (functionSignature.ReturnType == null)
-            {
-                throw new ArgumentNullException("FunctionSignatureWithReturnType must contain a reuturn type.");
-            }
+            ExceptionUtils.CheckArgumentNotNull(functionSignature.ReturnType, "unctionSignatureWithReturnType must contain a reuturn type");
         }
 
         #endregion

diff --git a/...tionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/CustomUriFunctionsTests.cs b/...tionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/CustomUriFunctionsTests.cs
@@ -163,7 +163,7 @@ public void AddCustomFunction_CannotAddFunctionWithFullSignatureExistsAsCustomFu
             string customFunctionName = "my.ExistingCustomFunction";
             try
             {
-                // Preaper
+                // Prepare
                 var existingCustomFunctionSignature = new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetDouble(false), EdmCoreModel.Instance.GetBoolean(false));
                 CustomUriFunctions.AddCustomUriFunction(customFunctionName, existingCustomFunctionSignature);
 
@@ -190,7 +190,7 @@ public void AddCustomFunction_CannotAddFunctionWithFullSignatureExistsAsCustomFu
             string customFunctionName = "my.ExistingCustomFunction";
             try
             {
-                // Preaper
+                // Prepare
                 var existingCustomFunctionSignature = new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetDouble(false), EdmCoreModel.Instance.GetBoolean(false));
                 CustomUriFunctions.AddCustomUriFunction(customFunctionName, existingCustomFunctionSignature);
 
@@ -269,14 +269,12 @@ public void AddCustomFunction_CustomFunctionNameExistsButNotFullSignature_Should
             string customFunctionName = "my.ExistingCustomFunction";
             try
             {
-                // Preaper
+                // Prepare
                 FunctionSignatureWithReturnType existingCustomFunctionSignature =
                     new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetDouble(false), EdmCoreModel.Instance.GetBoolean(false));
                 CustomUriFunctions.AddCustomUriFunction(customFunctionName, existingCustomFunctionSignature);
 
                 //Test
-
-
                 // Same name, but different signature
                 var newCustomFunctionSignature =
                     new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetDouble(false), EdmCoreModel.Instance.GetInt32(false), EdmCoreModel.Instance.GetBoolean(false));
@@ -360,7 +358,7 @@ public void RemoveCustomFunction_ShouldRemoveAnExistingFunction_ByName()
         {
             string customFunctionName = "my.ExistingCustomFunction";
 
-            // Preaper
+            // Prepare
             FunctionSignatureWithReturnType existingCustomFunctionSignature =
                     new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetDouble(false), EdmCoreModel.Instance.GetBoolean(false));
             CustomUriFunctions.AddCustomUriFunction(customFunctionName, existingCustomFunctionSignature);
@@ -414,15 +412,13 @@ public void RemoveCustomFunction_CannotRemoveFunctionWithSameNameAndDifferentSig
 
             try
             {
-                // Preaper
+                // Prepare
                 FunctionSignatureWithReturnType existingCustomFunctionSignature =
                         new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetDouble(false), EdmCoreModel.Instance.GetBoolean(false));
                 CustomUriFunctions.AddCustomUriFunction(customFunctionName, existingCustomFunctionSignature);
 
                 GetCustomFunctionSignaturesOrNull(customFunctionName)[0].Equals(existingCustomFunctionSignature).Should().BeTrue();
 
-                // Preaper
-
                 // Function with different siganture
                 FunctionSignatureWithReturnType customFunctionSignatureToRemove =
                         new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetInt16(false), EdmCoreModel.Instance.GetBoolean(false));
@@ -451,7 +447,7 @@ public void RemoveCustomFunction_RemoveFunctionWithSameNameAndSignature()
 
             try
             {
-                // Preaper
+                // Prepare
                 FunctionSignatureWithReturnType existingCustomFunctionSignature =
                         new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetDouble(false), EdmCoreModel.Instance.GetBoolean(false));
                 CustomUriFunctions.AddCustomUriFunction(customFunctionName, existingCustomFunctionSignature);
@@ -480,7 +476,7 @@ public void RemoveCustomFunction_RemoveFunctionWithSameNameAndSignature_OtherOve
 
             try
             {
-                // Preaper
+                // Prepare
                 FunctionSignatureWithReturnType existingCustomFunctionSignature =
                         new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetDouble(false), EdmCoreModel.Instance.GetBoolean(false));
                 CustomUriFunctions.AddCustomUriFunction(customFunctionName, existingCustomFunctionSignature);
@@ -536,10 +532,36 @@ FunctionSignatureWithReturnType myStringFunction
             }
             finally
             {
-                CustomUriFunctions.RemoveCustomUriFunction("mystringfunction");
+                CustomUriFunctions.RemoveCustomUriFunction("mystringfunction").Should().BeTrue();
             }
         }
 
+        [Fact]
+        public void ParseWithCustomUriFunction_OverrideBuiltInFunction()
+        {
+            FunctionSignatureWithReturnType customStartWithFunctionSignature = 
+                new FunctionSignatureWithReturnType(EdmCoreModel.Instance.GetBoolean(true),
+                                                    EdmCoreModel.Instance.GetString(true),
+                                                    EdmCoreModel.Instance.GetInt32(true));
+            try
+            {
+                // Add with override 'true'
+                CustomUriFunctions.AddCustomUriFunction("startswith", customStartWithFunctionSignature, true);
+
+                var fullUri = new Uri("http://www.odata.com/OData/People" + "?$filter=startswith(Name, 66)");
+                ODataUriParser parser = new ODataUriParser(HardCodedTestModel.TestModel, new Uri("http://www.odata.com/OData/"), fullUri);
+
+                var startsWithArgs = parser.ParseFilter().Expression.ShouldBeSingleValueFunctionCallQueryNode("startswith").And.Parameters.ToList();
+                startsWithArgs[0].ShouldBeSingleValuePropertyAccessQueryNode(HardCodedTestModel.GetPersonNameProp());
+                startsWithArgs[1].ShouldBeConstantQueryNode(66);
+            }
+            finally
+            {
+                CustomUriFunctions.RemoveCustomUriFunction("startswith").Should().BeTrue();
+            }
+        }
+
+
         #endregion
 
         #region Private Methods

diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Binders/FunctionCallBinderTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Binders/FunctionCallBinderTests.cs
@@ -1281,7 +1281,7 @@ public void GetUriFunction_SearchForCustomFunctionBeforeBuiltIn()
             }
             finally
             {
-                // Clean uo CustomFunctions cache
+                // Clean from CustomFunctions cache
                 CustomUriFunctions.RemoveCustomUriFunction(BUILT_IN_GEODISTANCE_FUNCTION_NAME);
             }
         }
-Original file line number
+Diff line change
@@ Expand Up @@
                 }
                 finally
                 {
-                    // Clean uo CustomFunctions cache
+                    // Clean from CustomFunctions cache
                     CustomUriFunctions.RemoveCustomUriFunction(BUILT_IN_GEODISTANCE_FUNCTION_NAME);
                 }
             }
@@ Expand Down @@