From 5fc3d8e248e79557129ff0857ae58f2ab8bfa074 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Mon, 8 Oct 2012 00:09:44 +0500 Subject: [PATCH] Fixes threading issue with Dynamicexpression... would have caused some strange behavior in the past! Fixes: U4-995 - most methods will now work in dynamic expressions --- .../Dynamics/ExtensionMethodFinder.cs | 134 +++++++++--------- .../DynamicDocumentTestsBase.cs | 91 +++++++++++- src/Umbraco.Web/Dynamics/DynamicExpression.cs | 11 +- src/Umbraco.Web/Dynamics/DynamicQueryable.cs | 2 +- src/Umbraco.Web/Dynamics/ExpressionParser.cs | 94 ++++++++---- .../RazorDynamicNode/DynamicExpression.cs | 8 +- .../RazorDynamicNode/ExpressionParser.cs | 4 +- 7 files changed, 230 insertions(+), 114 deletions(-) diff --git a/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs b/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs index fdfa8e35f7..6dd8a1ad12 100644 --- a/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs +++ b/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs @@ -9,10 +9,11 @@ using System.Linq.Expressions; namespace Umbraco.Core.Dynamics { + /// + /// Utility class for finding extension methods on a type to execute + /// internal static class ExtensionMethodFinder - { - - + { /// /// Returns all extension methods found matching the definition /// @@ -22,7 +23,7 @@ namespace Umbraco.Core.Dynamics /// /// /// - /// NOTE: This will be an intensive method to call!! Results should be cached! + /// TODO: NOTE: This will be an intensive method to call!! Results should be cached! /// private static IEnumerable GetAllExtensionMethods(Type thisType, string name, int argumentCount, bool argsContainsThis) { @@ -62,7 +63,8 @@ namespace Umbraco.Core.Dynamics return methodsWhereArgZeroIsTargetType.Select(mt => mt.m); } - private static bool MethodArgZeroHasCorrectTargetType(MethodInfo method, Type firstArgumentType, Type thisType) + + private static bool MethodArgZeroHasCorrectTargetType(MethodInfo method, Type firstArgumentType, Type thisType) { //This is done with seperate method calls because you can't debug/watch lamdas - if you're trying to figure //out why the wrong method is returned, it helps to be able to see each boolean result @@ -112,7 +114,8 @@ namespace Umbraco.Core.Dynamics bool result = (thisType == firstArgumentType); return result; } - private static Type FirstParameterType(MethodInfo m) + + private static Type FirstParameterType(MethodInfo m) { ParameterInfo[] p = m.GetParameters(); if (p.Any()) @@ -122,71 +125,74 @@ namespace Umbraco.Core.Dynamics return null; } + private static MethodInfo DetermineMethodFromParams(IEnumerable methods, Type genericType, IEnumerable args) + { + if (!methods.Any()) + { + return null; + } + MethodInfo methodToExecute = null; + if (methods.Count() > 1) + { + //Given the args, lets get the types and compare the type sequence to try and find the correct overload + var argTypes = args.ToList().ConvertAll(o => + { + var oe = (o as Expression); + if (oe != null) + { + return oe.Type.FullName; + } + return o.GetType().FullName; + }); + var methodsWithArgTypes = methods.Select(method => new { method, types = method.GetParameters().Select(pi => pi.ParameterType.FullName) }); + var firstMatchingOverload = methodsWithArgTypes.FirstOrDefault(m => m.types.SequenceEqual(argTypes)); + if (firstMatchingOverload != null) + { + methodToExecute = firstMatchingOverload.method; + } + } + + if (methodToExecute == null) + { + var firstMethod = methods.FirstOrDefault(); + // NH: this is to ensure that it's always the correct one being chosen when using the LINQ extension methods + if (methods.Count() > 1) + { + var firstGenericMethod = methods.FirstOrDefault(x => x.IsGenericMethodDefinition); + if (firstGenericMethod != null) + { + firstMethod = firstGenericMethod; + } + } + + if (firstMethod != null) + { + if (firstMethod.IsGenericMethodDefinition) + { + if (genericType != null) + { + methodToExecute = firstMethod.MakeGenericMethod(genericType); + } + } + else + { + methodToExecute = firstMethod; + } + } + } + return methodToExecute; + } + public static MethodInfo FindExtensionMethod(Type thisType, object[] args, string name, bool argsContainsThis) { Type genericType = null; if (thisType.IsGenericType) { genericType = thisType.GetGenericArguments()[0]; - } + } - var methods = GetAllExtensionMethods(thisType, name, args.Length, argsContainsThis) - .ToArray(); - - if (!methods.Any()) - { - return null; - } - MethodInfo methodToExecute = null; - if (methods.Count() > 1) - { - //Given the args, lets get the types and compare the type sequence to try and find the correct overload - var argTypes = args.ToList().ConvertAll(o => - { - var oe = (o as Expression); - if (oe != null) - { - return oe.Type.FullName; - } - return o.GetType().FullName; - }); - var methodsWithArgTypes = methods.Select(method => new {method, types = method.GetParameters().Select(pi => pi.ParameterType.FullName) }); - var firstMatchingOverload = methodsWithArgTypes.FirstOrDefault(m => m.types.SequenceEqual(argTypes)); - if (firstMatchingOverload != null) - { - methodToExecute = firstMatchingOverload.method; - } - } - - if (methodToExecute == null) - { - var firstMethod = methods.FirstOrDefault(); - // NH: this is to ensure that it's always the correct one being chosen when using the LINQ extension methods - if (methods.Count() > 1) - { - var firstGenericMethod = methods.FirstOrDefault(x => x.IsGenericMethodDefinition); - if (firstGenericMethod != null) - { - firstMethod = firstGenericMethod; - } - } - - if (firstMethod != null) - { - if (firstMethod.IsGenericMethodDefinition) - { - if (genericType != null) - { - methodToExecute = firstMethod.MakeGenericMethod(genericType); - } - } - else - { - methodToExecute = firstMethod; - } - } - } - return methodToExecute; + var methods = GetAllExtensionMethods(thisType, name, args.Length, argsContainsThis).ToArray(); + return DetermineMethodFromParams(methods, genericType, args); } } } diff --git a/src/Umbraco.Tests/DynamicDocument/DynamicDocumentTestsBase.cs b/src/Umbraco.Tests/DynamicDocument/DynamicDocumentTestsBase.cs index 523befc611..f5db4adc33 100644 --- a/src/Umbraco.Tests/DynamicDocument/DynamicDocumentTestsBase.cs +++ b/src/Umbraco.Tests/DynamicDocument/DynamicDocumentTestsBase.cs @@ -135,18 +135,85 @@ namespace Umbraco.Tests.DynamicDocument } [Test] - public void Complex_Linq() + public void String_ContainsValue_Extension_Method() { - var doc = GetDynamicNode(1173); - + var doc = GetDynamicNode(1046); - var result = doc.Ancestors().OrderBy("level") - .Single() - .Descendants() - .Where("selectedNodes != null && selectedNodes != \"\" && selectedNodes.Split(new char[] {','}).Contains(\"1173\")") + var paramVals = new Dictionary { { "searchId", 1173 } }; //this is an integer value + var result = doc.Children() + .Where("selectedNodes.ContainsValue(searchId)", paramVals) //call an extension method .FirstOrDefault(); Assert.IsNotNull(result); + Assert.AreEqual(4444, result.Id); + + //don't find! + paramVals = new Dictionary { { "searchId", 1111777 } }; + result = doc.Children() + .Where("selectedNodes.ContainsValue(searchId)", paramVals) + .FirstOrDefault(); + + Assert.IsNotNull(result); + Assert.IsTrue(result.GetType() == typeof(DynamicNull) || result.GetType() == typeof(umbraco.MacroEngines.DynamicNull)); + //Assert.AreEqual(typeof(DynamicNull), result.GetType()); + } + + [Test] + public void String_Contains_Method() + { + var doc = GetDynamicNode(1046); + + var paramVals = new Dictionary { { "searchId", "1173" } }; + var result = doc.Children() + .Where("selectedNodes.Contains(searchId)", paramVals) + .FirstOrDefault(); + + Assert.IsNotNull(result); + Assert.AreEqual(4444, result.Id); + + //don't find! + paramVals = new Dictionary { { "searchId", "1aaa173" } }; + result = doc.Children() + .Where("selectedNodes.Contains(searchId)", paramVals) + .FirstOrDefault(); + + Assert.IsNotNull(result); + Assert.IsTrue(result.GetType() == typeof (DynamicNull) || result.GetType() == typeof (umbraco.MacroEngines.DynamicNull)); + //Assert.AreEqual(typeof (DynamicNull), result.GetType()); + } + + [Test] + public void String_Split_Method() + { + var doc = GetDynamicNode(1046); + + var paramVals = new Dictionary + { + { "splitTerm", new char[] { ',' } }, + { "splitOptions", StringSplitOptions.RemoveEmptyEntries } + }; + var result = doc.Children() + .Where("selectedNodes.Split(splitTerm, splitOptions).Length == 3", paramVals) + .FirstOrDefault(); + + Assert.IsNotNull(result); + Assert.AreEqual(4444, result.Id); + } + + [Test] + public void Complex_Linq() + { + var doc = GetDynamicNode(1173); + + var paramVals = new Dictionary {{"splitTerm", new char[] {','}}, {"searchId", "1173"}}; + var result = doc.Ancestors().OrderBy("level") + .Single() + .Descendants() + .Where("selectedNodes != null && selectedNodes != String.Empty && selectedNodes.Split(splitTerm).Contains(searchId)", paramVals) + .FirstOrDefault(); + + Assert.IsNotNull(result); + Assert.AreEqual(4444, result.Id); } [Test] @@ -557,6 +624,16 @@ namespace Umbraco.Tests.DynamicDocument Assert.AreEqual((int) 1174, (int) result.Id); } + } + /// + /// Extension methods used in tests + /// + public static class TestExtensionMethods + { + public static bool ContainsValue(this string s, int val) + { + return s.Contains(val.ToString()); + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Dynamics/DynamicExpression.cs b/src/Umbraco.Web/Dynamics/DynamicExpression.cs index 0b84c97ec7..ae0befbfe8 100644 --- a/src/Umbraco.Web/Dynamics/DynamicExpression.cs +++ b/src/Umbraco.Web/Dynamics/DynamicExpression.cs @@ -7,11 +7,12 @@ namespace Umbraco.Web.Dynamics { internal static class DynamicExpression { - public static bool ConvertDynamicNullToBooleanFalse = false; + //public static bool ConvertDynamicNullToBooleanFalse = false; + public static Expression Parse(Type resultType, string expression, bool convertDynamicNullToBooleanFalse, params object[] values) { - ConvertDynamicNullToBooleanFalse = convertDynamicNullToBooleanFalse; - var parser = new ExpressionParser(null, expression, values); + //ConvertDynamicNullToBooleanFalse = convertDynamicNullToBooleanFalse; + var parser = new ExpressionParser(null, expression, values, convertDynamicNullToBooleanFalse); return parser.Parse(resultType); } @@ -22,8 +23,8 @@ namespace Umbraco.Web.Dynamics public static LambdaExpression ParseLambda(ParameterExpression[] parameters, Type resultType, string expression, bool convertDynamicNullToBooleanFalse, params object[] values) { - ConvertDynamicNullToBooleanFalse = convertDynamicNullToBooleanFalse; - var parser = new ExpressionParser(parameters, expression, values); + //ConvertDynamicNullToBooleanFalse = convertDynamicNullToBooleanFalse; + var parser = new ExpressionParser(parameters, expression, values, convertDynamicNullToBooleanFalse); return Expression.Lambda(parser.Parse(resultType), parameters); } diff --git a/src/Umbraco.Web/Dynamics/DynamicQueryable.cs b/src/Umbraco.Web/Dynamics/DynamicQueryable.cs index 817e58063e..f4650af6d9 100644 --- a/src/Umbraco.Web/Dynamics/DynamicQueryable.cs +++ b/src/Umbraco.Web/Dynamics/DynamicQueryable.cs @@ -197,7 +197,7 @@ namespace Umbraco.Web.Dynamics ParameterExpression[] parameters = new ParameterExpression[] { Expression.Parameter(source.ElementType, "") }; - var parser = new ExpressionParser(parameters, ordering, values); + var parser = new ExpressionParser(parameters, ordering, values, false); IEnumerable orderings = parser.ParseOrdering(); Expression queryExpr = source.Expression; string methodAsc = "OrderBy"; diff --git a/src/Umbraco.Web/Dynamics/ExpressionParser.cs b/src/Umbraco.Web/Dynamics/ExpressionParser.cs index 46e49b38e0..7d57352b44 100644 --- a/src/Umbraco.Web/Dynamics/ExpressionParser.cs +++ b/src/Umbraco.Web/Dynamics/ExpressionParser.cs @@ -200,12 +200,13 @@ namespace Umbraco.Web.Dynamics Dictionary literals; ParameterExpression it; string text; + private readonly bool _flagConvertDynamicNullToBooleanFalse; int textPos; int textLen; char ch; Token token; - public ExpressionParser(ParameterExpression[] parameters, string expression, object[] values) + public ExpressionParser(ParameterExpression[] parameters, string expression, object[] values, bool flagConvertDynamicNullToBooleanFalse) { if (expression == null) throw new ArgumentNullException("expression"); if (keywords == null) keywords = CreateKeywords(); @@ -214,6 +215,7 @@ namespace Umbraco.Web.Dynamics if (parameters != null) ProcessParameters(parameters); if (values != null) ProcessValues(values); text = expression; + _flagConvertDynamicNullToBooleanFalse = flagConvertDynamicNullToBooleanFalse; textLen = text.Length; SetTextPos(0); NextToken(); @@ -839,6 +841,11 @@ namespace Umbraco.Web.Dynamics Expression ParseMemberAccess(Type type, Expression instance) { + //NOTE: SD: There is a lot of string checking going on here and I'm 99% sure this can all be done better + // in a more generic sense to support any types with any extension methods, etc... + // Too bad whoever wrote this decided not to put any code comments in :( + // This is how to support method calls, etc... in dynamic statements. + if (instance != null) type = instance.Type; int errorPos = token.Pos; string id = GetIdentifier(); @@ -864,17 +871,26 @@ namespace Umbraco.Web.Dynamics if (typeArgs[0] == typeof(T)) { if (instance != null && instance is LambdaExpression) - { + { + //not sure why this is object or why we need to do this but if we change it, things die... + //also not sure why it is changed to string, i think this might be to ensure string methods are supported + //but seems to me that we then won't support other types of methods? if (typeArgs[1] == typeof(object)) { instanceAsString = StringFormat(instance as LambdaExpression, instanceExpression); type = typeof(string); } - if (typeArgs[1] == typeof(string)) + else if (typeArgs[1] == typeof(string)) { instanceAsString = instance as LambdaExpression; type = typeof(string); } + //else + //{ + // instanceAsString = instance as LambdaExpression; + // type = typeArgs[1]; + //} + } } } @@ -882,6 +898,10 @@ namespace Umbraco.Web.Dynamics { case 0: //not found + + //SD: I have yet to see extension methods actually being called in the dynamic parsing... need to unit test these + // scenarios and figure out why all this type checking occurs. + if (type == typeof(string) && instanceAsString != null) { Expression[] newArgs = (new List() { Expression.Invoke(instanceAsString, instanceExpression) }).Concat(args).ToArray(); @@ -944,7 +964,7 @@ namespace Umbraco.Web.Dynamics BlockExpression block = Expression.Block( typeof(object), new[] { ignoreCase, binder, result, convertDynamicNullToBooleanFalse }, - Expression.Assign(convertDynamicNullToBooleanFalse, Expression.Constant(DynamicExpression.ConvertDynamicNullToBooleanFalse, typeof(bool))), + Expression.Assign(convertDynamicNullToBooleanFalse, Expression.Constant(_flagConvertDynamicNullToBooleanFalse, typeof(bool))), Expression.Assign(ignoreCase, Expression.Constant(false, typeof(bool))), Expression.Assign(binder, Expression.New(getMemberBinderConstructor, Expression.Constant(id, typeof(string)), ignoreCase)), Expression.Assign(result, Expression.Constant(null)), @@ -981,7 +1001,7 @@ namespace Umbraco.Web.Dynamics BlockExpression block = Expression.Block( typeof(object), new[] { lambdaResult, result, idParam, convertDynamicNullToBooleanFalse }, - Expression.Assign(convertDynamicNullToBooleanFalse, Expression.Constant(DynamicExpression.ConvertDynamicNullToBooleanFalse, typeof(bool))), + Expression.Assign(convertDynamicNullToBooleanFalse, Expression.Constant(_flagConvertDynamicNullToBooleanFalse, typeof(bool))), Expression.Assign(lambdaResult, Expression.Invoke(instance, lambdaInstanceExpression)), Expression.Assign(result, Expression.Call(ReflectPropertyValue, lambdaResult, Expression.Constant(id))), Expression.IfThen( @@ -1020,21 +1040,11 @@ namespace Umbraco.Web.Dynamics return null; } private static Expression CallMethodOnDynamicNode(Expression instance, Expression[] args, LambdaExpression instanceAsString, ParameterExpression instanceExpression, MethodInfo method, bool isStatic) - { - ConstantExpression defaultReturnValue = Expression.Constant(null, typeof(object)); + { Type methodReturnType = method.ReturnType; - switch (methodReturnType.Name) - { - case "String": - defaultReturnValue = Expression.Constant(null, typeof(string)); - break; - case "Int32": - defaultReturnValue = Expression.Constant(0, typeof(int)); - break; - case "Boolean": - defaultReturnValue = Expression.Constant(false, typeof(bool)); - break; - } + + var defaultReturnValue = Expression.Constant(methodReturnType.GetDefaultValue(), methodReturnType); + ParameterExpression result = Expression.Parameter(method.ReturnType, "result"); LabelTarget blockReturnLabel = Expression.Label(method.ReturnType); BlockExpression block = Expression.Block( @@ -1050,16 +1060,24 @@ namespace Umbraco.Web.Dynamics Expression.Label(blockReturnLabel, defaultReturnValue) ); - switch (methodReturnType.Name) - { - case "String": - return Expression.Lambda>(block, instanceExpression); - case "Int32": - return Expression.Lambda>(block, instanceExpression); - case "Boolean": - return Expression.Lambda>(block, instanceExpression); - } - return Expression.Call(instance, (MethodInfo)method, args); + Type func = typeof(Func<,>); + Type generic = func.MakeGenericType(typeof(T), methodReturnType); + return Expression.Lambda(generic, block, instanceExpression); + + //if (methodReturnType == typeof(string)) + // return Expression.Lambda>(block, instanceExpression); + //if (methodReturnType == typeof(int)) + // return Expression.Lambda>(block, instanceExpression); + //if (methodReturnType == typeof(bool)) + // return Expression.Lambda>(block, instanceExpression); + //if (methodReturnType == typeof(string[])) + //return Expression.Lambda>(block, instanceExpression); + + //return Expression.Call(instance, (MethodInfo)method, args); + + //return Expression.Lambda>( + // Expression.Convert(block, typeof(object)), instanceExpression); + } static Type FindGenericType(Type generic, Type type) @@ -1398,7 +1416,23 @@ namespace Umbraco.Web.Dynamics { ParameterInfo pi = method.Parameters[i]; if (pi.IsOut) return false; - Expression promoted = PromoteExpression(args[i], pi.ParameterType, false); + Expression promoted; + + //TODO: Turns out this is real difficult to parse and don't really have time to figure this out at the moment + // to parse params parameter arrays. + + ////here we need to check if it is a params array parameter + //if (pi.ParameterType.IsArray + // && pi.ParameterType.GetElementType() != null + // && pi.GetCustomAttributes(typeof(ParamArrayAttribute), false).Any()) + //{ + // //it is a params parameter so convert the value to an array + // promoted = PromoteExpression(args[i], pi.ParameterType.GetElementType(), false); + //} + //else + //{ + promoted = PromoteExpression(args[i], pi.ParameterType, false); + //} if (promoted == null) return false; promotedArgs[i] = promoted; } diff --git a/src/umbraco.MacroEngines/RazorDynamicNode/DynamicExpression.cs b/src/umbraco.MacroEngines/RazorDynamicNode/DynamicExpression.cs index db823e1b69..8dac737778 100644 --- a/src/umbraco.MacroEngines/RazorDynamicNode/DynamicExpression.cs +++ b/src/umbraco.MacroEngines/RazorDynamicNode/DynamicExpression.cs @@ -8,11 +8,9 @@ namespace System.Linq.Dynamic [Obsolete("This class has been superceded by Umbraco.Core.Dynamics.DynamicExpression")] public static class DynamicExpression { - public static bool ConvertDynamicNullToBooleanFalse - { - get { return Umbraco.Web.Dynamics.DynamicExpression.ConvertDynamicNullToBooleanFalse; } - set { Umbraco.Web.Dynamics.DynamicExpression.ConvertDynamicNullToBooleanFalse = value; } - } + [Obsolete("This property is no longer used and had caused concurrency issues.")] + public static bool ConvertDynamicNullToBooleanFalse { get; set; } + public static Expression Parse(Type resultType, string expression, bool convertDynamicNullToBooleanFalse, params object[] values) { return Umbraco.Web.Dynamics.DynamicExpression.Parse(resultType, expression, convertDynamicNullToBooleanFalse, values); diff --git a/src/umbraco.MacroEngines/RazorDynamicNode/ExpressionParser.cs b/src/umbraco.MacroEngines/RazorDynamicNode/ExpressionParser.cs index c9001e9495..cb836100ce 100644 --- a/src/umbraco.MacroEngines/RazorDynamicNode/ExpressionParser.cs +++ b/src/umbraco.MacroEngines/RazorDynamicNode/ExpressionParser.cs @@ -9,8 +9,8 @@ namespace System.Linq.Dynamic [Obsolete("This class is no longer used, use Umbraco.Web.Dynamics.ExpressionParser instead")] internal class ExpressionParser : Umbraco.Web.Dynamics.ExpressionParser { - public ExpressionParser(ParameterExpression[] parameters, string expression, object[] values) - : base(parameters, expression, values) + public ExpressionParser(ParameterExpression[] parameters, string expression, object[] values, bool flagConvertDynamicNullToBooleanFalse) + : base(parameters, expression, values, flagConvertDynamicNullToBooleanFalse) { } }