From 839a5b13b0da9069cb06e48f3ba40e3c7cda2d98 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 3 Feb 2015 20:23:05 +1100 Subject: [PATCH] updates extension method finder with tests to better support finding generic equivialents, adds tests for TypeHelper as well. --- .../Dynamics/ExtensionMethodFinder.cs | 111 ++++++------------ src/Umbraco.Core/TypeHelper.cs | 77 +++++++++++- .../ExtensionMethodFinderTests.cs | 25 ++++ src/Umbraco.Tests/Plugins/TypeHelperTests.cs | 72 ++++++++++++ 4 files changed, 212 insertions(+), 73 deletions(-) diff --git a/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs b/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs index fbfa933e83..78bf778fba 100644 --- a/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs +++ b/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs @@ -82,75 +82,34 @@ namespace Umbraco.Core.Dynamics var methodsByName = candidates.Where(m => m.Name == name); //ensure we add + 1 to the arg count because the 'this' arg is not included in the count above! - var isGenericAndRightParamCount = methodsByName.Where(m => m.GetParameters().Length == argumentCount + 1); + var byParamCount = methodsByName.Where(m => m.GetParameters().Length == argumentCount + 1); - //find the right overload that can take genericParameterType - //which will be either DynamicNodeList or List which is IEnumerable` + var methodsWithFirstParamType = byParamCount.Select(m => new {m, t = FirstParameterType(m)}) + .ToArray(); // get the array so we don't keep enumerating. - var withGenericParameterType = isGenericAndRightParamCount.Select(m => new { m, t = FirstParameterType(m) }); + //Find the method with an assignable first parameter type + var methodsWhereArgZeroIsTargetType = (from method in methodsWithFirstParamType + where + method.t != null && TypeHelper.IsTypeAssignableFrom(method.t, thisType) + select method).ToArray(); + + //found some so return this + if (methodsWhereArgZeroIsTargetType.Any()) return methodsWhereArgZeroIsTargetType.Select(mt => mt.m).ToArray(); - var methodsWhereArgZeroIsTargetType = (from method in withGenericParameterType - where - method.t != null && MethodArgZeroHasCorrectTargetType(method.m, method.t, thisType) - select method); + //this is where it gets tricky, none of the first parameters were assignable to our type which means that + // if they are assignable they are generic arguments - return methodsWhereArgZeroIsTargetType.Select(mt => mt.m).ToArray(); + methodsWhereArgZeroIsTargetType = (from method in methodsWithFirstParamType + where + method.t != null && TypeHelper.IsAssignableToGenericType(method.t, thisType) + select method).ToArray(); + + return methodsWhereArgZeroIsTargetType.Select(mt => mt.m).ToArray(); }); } - - 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 - - return - - // is it defined on me? - MethodArgZeroHasCorrectTargetTypeTypeMatchesExactly(method, firstArgumentType, thisType) || - - // or on any of my interfaces? - MethodArgZeroHasCorrectTargetTypeAnInterfaceMatches(method, firstArgumentType, thisType) || - - // or on any of my base types? - MethodArgZeroHasCorrectTargetTypeIsASubclassOf(method, firstArgumentType, thisType) || - - //share a common interface (e.g. IEnumerable) - MethodArgZeroHasCorrectTargetTypeShareACommonInterface(method, firstArgumentType, thisType); - - - } - - private static bool MethodArgZeroHasCorrectTargetTypeShareACommonInterface(MethodInfo method, Type firstArgumentType, Type thisType) - { - var interfaces = firstArgumentType.GetInterfaces(); - if (interfaces.Length == 0) - { - return false; - } - var result = interfaces.All(i => thisType.GetInterfaces().Contains(i)); - return result; - } - - private static bool MethodArgZeroHasCorrectTargetTypeIsASubclassOf(MethodInfo method, Type firstArgumentType, Type thisType) - { - var result = thisType.IsSubclassOf(firstArgumentType); - return result; - } - - private static bool MethodArgZeroHasCorrectTargetTypeAnInterfaceMatches(MethodInfo method, Type firstArgumentType, Type thisType) - { - var result = thisType.GetInterfaces().Contains(firstArgumentType); - return result; - } - - private static bool MethodArgZeroHasCorrectTargetTypeTypeMatchesExactly(MethodInfo method, Type firstArgumentType, Type thisType) - { - var result = (thisType == firstArgumentType); - return result; - } - - private static Type FirstParameterType(MethodInfo m) + + private static Type FirstParameterType(MethodInfo m) { var p = m.GetParameters(); if (p.Any()) @@ -160,7 +119,7 @@ namespace Umbraco.Core.Dynamics return null; } - private static MethodInfo DetermineMethodFromParams(IEnumerable methods, Type genericType, IEnumerable args) + private static MethodInfo DetermineMethodFromParams(IEnumerable methods, Type thisType, IEnumerable args) { MethodInfo methodToExecute = null; @@ -194,19 +153,27 @@ namespace Umbraco.Core.Dynamics if (firstMatchingOverload != null) { methodToExecute = firstMatchingOverload.method; + + var extensionParam = methodToExecute.GetParameters()[0].ParameterType; + + //We've already done this check before in the GetAllExtensionMethods method, but need to do it here + // again because in order to use this found generic method we need to create a real generic method + // with the generic type parameters + var baseCompareTypeAttempt = TypeHelper.IsAssignableToGenericType(extensionParam, thisType); + //This should never throw + if (baseCompareTypeAttempt.Success == false) throw new InvalidOperationException("No base compare type could be resolved"); + + if (methodToExecute.IsGenericMethodDefinition && baseCompareTypeAttempt.Result.IsGenericType) + { + methodToExecute = methodToExecute.MakeGenericMethod(baseCompareTypeAttempt.Result.GetGenericArguments()); + } } - return methodToExecute; + return methodToExecute; } public static MethodInfo FindExtensionMethod(IRuntimeCacheProvider runtimeCache, Type thisType, object[] args, string name, bool argsContainsThis) { - Type genericType = null; - if (thisType.IsGenericType) - { - genericType = thisType.GetGenericArguments()[0]; - } - args = args //if the args contains 'this', remove the first one since that is 'this' and we don't want to use //that in the method searching @@ -214,8 +181,8 @@ namespace Umbraco.Core.Dynamics .ToArray(); var methods = GetAllExtensionMethods(runtimeCache, thisType, name, args.Length).ToArray(); - - return DetermineMethodFromParams(methods, genericType, args); + + return DetermineMethodFromParams(methods, thisType, args); } } } diff --git a/src/Umbraco.Core/TypeHelper.cs b/src/Umbraco.Core/TypeHelper.cs index 6a35c92f08..03e5769c1c 100644 --- a/src/Umbraco.Core/TypeHelper.cs +++ b/src/Umbraco.Core/TypeHelper.cs @@ -131,7 +131,82 @@ namespace Umbraco.Core return Attempt.Fail(); } - /// + /// + /// Checks if the generic type passed in can be assigned from the given type + /// + /// + /// + /// + /// Returns an Attempt{Type} which if true will include the actual type that matched the genericType + /// being compared. + /// + /// + /// First we need to check a special case, if the generic type is a generic definition but has not FullName, then + /// we cannot compare it with traditional means because the types will never match. + /// Generic types will not have a FullName in these cases: http://blogs.msdn.com/b/haibo_luo/archive/2006/02/17/534480.aspx + /// or apparently when you retrieve a generic method parameter using reflection. + /// + /// This is using a version modified from: http://stackoverflow.com/a/1075059/1968 + /// + public static Attempt IsAssignableToGenericType(Type genericType, Type givenType) + { + if (genericType.IsGenericTypeDefinition == false && genericType.FullName.IsNullOrWhiteSpace()) + { + return IsAssignableToReflectedGenericType(genericType, givenType); + } + + var genericTypeDef = givenType.IsGenericType ? givenType.GetGenericTypeDefinition() : null; + + if (genericTypeDef != null && genericTypeDef == genericType) + return Attempt.Succeed(givenType); + + var its = givenType.GetInterfaces(); + + foreach (var it in its) + { + genericTypeDef = it.IsGenericType ? it.GetGenericTypeDefinition() : null; + + if (genericTypeDef != null && genericTypeDef == genericType) + return Attempt.Succeed(it); + } + + var baseType = givenType.BaseType; + return baseType != null + ? IsAssignableToGenericType(genericType, baseType) + : Attempt.Fail(); + } + + /// + /// This is used in IsAssignableToGenericType + /// + /// + /// + /// + /// Returns an Attempt{Type} which if true will include the actual type that matched the genericType + /// being compared. + /// + private static Attempt IsAssignableToReflectedGenericType(Type genericType, Type givenType) + { + if (givenType.IsGenericType && givenType.GetGenericTypeDefinition().Name == genericType.Name && givenType.GenericTypeArguments.Length == genericType.GenericTypeArguments.Length) + return Attempt.Succeed(givenType); + + var its = givenType.GetInterfaces(); + + foreach (var it in its) + { + if (it.IsGenericType && it.GetGenericTypeDefinition().Name == genericType.Name && givenType.GenericTypeArguments.Length == genericType.GenericTypeArguments.Length) + { + return Attempt.Succeed(it); + } + } + + var baseType = givenType.BaseType; + return baseType != null + ? IsAssignableToReflectedGenericType(genericType, baseType) + : Attempt.Fail(); + } + + /// /// Determines whether the type is assignable from the specified implementation , /// and caches the result across the application using a . /// diff --git a/src/Umbraco.Tests/DynamicsAndReflection/ExtensionMethodFinderTests.cs b/src/Umbraco.Tests/DynamicsAndReflection/ExtensionMethodFinderTests.cs index 37b0a63866..15844e4245 100644 --- a/src/Umbraco.Tests/DynamicsAndReflection/ExtensionMethodFinderTests.cs +++ b/src/Umbraco.Tests/DynamicsAndReflection/ExtensionMethodFinderTests.cs @@ -166,6 +166,11 @@ namespace Umbraco.Tests.DynamicsAndReflection public class TestClass : TestClass { } + public class TestClassCollection : List + { + + } + [Test] public void Find_Non_Overloaded_Method() { @@ -253,10 +258,30 @@ namespace Umbraco.Tests.DynamicsAndReflection method = ExtensionMethodFinder.FindExtensionMethod(new NullCacheProvider(), typeof(TestClass), new object[] { nonGenericTestClass }, "GenericParameterMethod", false); Assert.IsNotNull(method); } + + [Test] + public void Find_Generic_Enumerable_Method() + { + MethodInfo method; + var class1 = Enumerable.Empty(); + + method = ExtensionMethodFinder.FindExtensionMethod(new NullCacheProvider(), typeof(IEnumerable), new object[] {}, "GenericMethod", false); + Assert.IsNotNull(method); + method.Invoke(null, new object[] { class1 }); + + var class2 = new TestClassCollection(); + + method = ExtensionMethodFinder.FindExtensionMethod(new NullCacheProvider(), typeof(TestClassCollection), new object[] { }, "GenericMethod", false); + Assert.IsNotNull(method); + method.Invoke(null, new object[] { class2 }); + } } static class ExtensionMethodFinderTestsExtensions { + public static void GenericMethod(this IEnumerable source) + { } + public static void SimpleMethod(this ExtensionMethodFinderTests.TestClass source, int value) { } diff --git a/src/Umbraco.Tests/Plugins/TypeHelperTests.cs b/src/Umbraco.Tests/Plugins/TypeHelperTests.cs index 404dc00eaa..d66c06efda 100644 --- a/src/Umbraco.Tests/Plugins/TypeHelperTests.cs +++ b/src/Umbraco.Tests/Plugins/TypeHelperTests.cs @@ -1,11 +1,17 @@ using System; +using System.Collections; +using System.Collections.Generic; using System.ComponentModel; using System.Data.Odbc; using System.Data.OleDb; using System.Data.SqlClient; +using System.Linq; +using System.Reflection; using NUnit.Framework; using Umbraco.Core; +using Umbraco.Core.Models; using Umbraco.Web.Cache; +using Umbraco.Web.Models; using Umbraco.Web.Scheduling; using UmbracoExamine.DataServices; @@ -18,6 +24,72 @@ namespace Umbraco.Tests.Plugins public class TypeHelperTests { + [Test] + public void Is_Generic_Assignable() + { + var type1 = typeof (DynamicPublishedContentList); + var type2 = typeof (IEnumerable); + var type3 = typeof(IQueryable); + var type4 = typeof(List); + var type5 = typeof(IEnumerable<>); + + Assert.IsTrue(TypeHelper.IsTypeAssignableFrom(type2, type1)); + Assert.IsFalse(TypeHelper.IsTypeAssignableFrom(type3, type1)); + Assert.IsTrue(TypeHelper.IsTypeAssignableFrom(type2, type4)); + + //Will always fail which is correct, you cannot 'assign' IEnumerable simply to IEnumerable<> + //Assert.IsTrue(TypeHelper.IsTypeAssignableFrom(type5, type2)); + + Assert.IsTrue(TypeHelper.IsAssignableToGenericType(type5, type2)); + } + + [Test] + public void Is_Assignable_To_Generic_Type() + { + //modified from: https://gist.github.com/klmr/4174727 + //using a version modified from: http://stackoverflow.com/a/1075059/1968 + + Assert.IsTrue(TypeHelper.IsAssignableToGenericType(typeof(Base<>), typeof(Derived))); + Assert.IsTrue(TypeHelper.IsAssignableToGenericType(typeof(IEnumerable<>), typeof(List))); + Assert.IsTrue(TypeHelper.IsAssignableToGenericType(typeof(Derived<>), typeof(Derived))); + Assert.IsTrue(TypeHelper.IsAssignableToGenericType(typeof(Base<>), typeof(Derived2))); + Assert.IsTrue(TypeHelper.IsAssignableToGenericType(typeof(IBase<>), typeof(DerivedI))); + Assert.IsTrue(TypeHelper.IsAssignableToGenericType(typeof(IBase<>), typeof(Derived2))); + Assert.IsTrue(TypeHelper.IsAssignableToGenericType(typeof(Nullable<>), typeof(int?))); + + Assert.IsFalse(TypeHelper.IsAssignableToGenericType(typeof(Object), typeof(Derived))); + Assert.IsFalse(TypeHelper.IsAssignableToGenericType(typeof(List<>), typeof(Derived))); + Assert.IsFalse(TypeHelper.IsAssignableToGenericType(typeof(IEnumerable<>), typeof(Derived))); + Assert.IsFalse(TypeHelper.IsAssignableToGenericType(typeof(Base), typeof(Derived))); + Assert.IsFalse(TypeHelper.IsAssignableToGenericType(typeof(IEnumerable), typeof(List))); + Assert.IsFalse(TypeHelper.IsAssignableToGenericType(typeof(Nullable<>), typeof(int))); + + //This get's the "Type" from the Count extension method on IEnumerable, however the type IEnumerable isn't + // IEnumerable<> and it is not a generic definition, this attempts to explain that: + // http://blogs.msdn.com/b/haibo_luo/archive/2006/02/17/534480.aspx + + var genericEnumerableNonGenericDefinition = typeof (Enumerable).GetMethods(BindingFlags.Static | BindingFlags.Public) + .Single(x => x.Name == "Count" && x.GetParameters().Count() == 1) + .GetParameters() + .Single() + .ParameterType; + + Assert.IsTrue(TypeHelper.IsAssignableToGenericType(genericEnumerableNonGenericDefinition, typeof(List))); + + } + + class Base { } + + interface IBase { } + + interface IDerived : IBase { } + + class Derived : Base, IBase { } + + class Derived2 : Derived { } + + class DerivedI : IDerived { } + [Test] public void Is_Static_Class() {