diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Properties/Resources.Designer.cs index f1dcaa3908..c43403c693 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Properties/Resources.Designer.cs @@ -858,6 +858,22 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures return string.Format(CultureInfo.CurrentCulture, GetString("Dictionary_DuplicateKey"), p0); } + /// + /// The view component method '{0}' cannot return a {1}. + /// + internal static string ViewComponent_SyncMethod_CannotReturnTask + { + get { return GetString("ViewComponent_SyncMethod_CannotReturnTask"); } + } + + /// + /// The view component method '{0}' cannot return a {1}. + /// + internal static string FormatViewComponent_SyncMethod_CannotReturnTask(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("ViewComponent_SyncMethod_CannotReturnTask"), p0, p1); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Resources.resx b/src/Microsoft.AspNet.Mvc.ViewFeatures/Resources.resx index eddad01662..cf06865c1a 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Resources.resx @@ -277,4 +277,7 @@ The collection already contains an entry with key '{0}'. + + The view component method '{0}' cannot return a {1}. + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewComponents/ViewComponentMethodSelector.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewComponents/ViewComponentMethodSelector.cs index 2d5e8cc1df..5a63795900 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewComponents/ViewComponentMethodSelector.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewComponents/ViewComponentMethodSelector.cs @@ -55,42 +55,44 @@ namespace Microsoft.AspNet.Mvc.ViewComponents throw new InvalidOperationException( Resources.FormatViewComponent_SyncMethod_ShouldReturnValue(SyncMethodName)); } + else if (method.ReturnType.IsAssignableFrom(typeof(Task))) + { + throw new InvalidOperationException( + Resources.FormatViewComponent_SyncMethod_CannotReturnTask(SyncMethodName, nameof(Task))); + } return method; } private static MethodInfo GetMethod(TypeInfo componentType, object[] args, string methodName) { - args = args ?? new object[0]; - var argumentExpressions = new Expression[args.Length]; - for (var i = 0; i < args.Length; i++) + Type[] types; + if (args == null || args.Length == 0) { - argumentExpressions[i] = Expression.Constant(args[i], args[i].GetType()); + types = Type.EmptyTypes; + } + else + { + types = new Type[args.Length]; + for (var i = 0; i < args.Length; i++) + { + types[i] = args[i]?.GetType() ?? typeof(object); + } } - try - { - // We're currently using this technique to make a call into a component method that looks like a - // regular method call. - // - // Ex: @Component.Invoke("hello", 5) => cart.Invoke("hello", 5) - // - // This approach has some drawbacks, namely it doesn't account for default parameters, and more - // noticably, it throws if the method is not found. - // - // Unfortunely the overload of Type.GetMethod that we would like to use is not present in CoreCLR. - // Item #160 in Jira tracks these issues. - var expression = Expression.Call( - Expression.Constant(null, componentType.AsType()), - methodName, - null, - argumentExpressions); - return expression.Method; - } - catch (InvalidOperationException) - { - return null; - } +#if DNX451 + return componentType.AsType().GetMethod( + methodName, + BindingFlags.Public | BindingFlags.Instance, + binder: null, + types: types, + modifiers: null); +#else + var method = componentType.AsType().GetMethod(methodName, types: types); + // At most one method (including static and instance methods) with the same parameter types can exist + // per type. + return method != null && method.IsStatic ? null : method; +#endif } } } diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewComponents/ViewComponentMethodSelectorTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewComponents/ViewComponentMethodSelectorTest.cs new file mode 100644 index 0000000000..c3344f5826 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewComponents/ViewComponentMethodSelectorTest.cs @@ -0,0 +1,282 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Reflection; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.AspNet.Mvc.ViewComponents +{ + public class ViewComponentMethodSelectorTest + { + [Theory] + [InlineData(typeof(ViewComponentWithSyncInvoke), new object[] { "" })] + [InlineData(typeof(ViewComponentWithAsyncInvoke), new object[] { 42, false })] + [InlineData(typeof(ViewComponentWithNonPublicNonInstanceInvokes), new object[] { })] + [InlineData(typeof(ViewComponentWithNonPublicNonInstanceInvokes), new object[] { "" })] + public void FindAsyncMethod_ReturnsNull_IfMatchCannotBeFound(Type type, object[] args) + { + // Arrange + var typeInfo = type.GetTypeInfo(); + + // Act + var method = ViewComponentMethodSelector.FindAsyncMethod(typeInfo, args); + + // Assert + Assert.Null(method); + } + + [Theory] + [InlineData(typeof(ViewComponentWithAsyncInvoke), new object[0])] + [InlineData(typeof(ViewComponentWithSyncInvoke), new object[] { "" })] + [InlineData(typeof(ViewComponentWithAsyncInvoke), new object[] { "" })] + [InlineData(typeof(ViewComponentWithSyncInvoke), new object[] { 42 })] + [InlineData(typeof(ViewComponentWithAsyncInvoke), new object[] { "", 42 })] + [InlineData(typeof(ViewComponentWithNonPublicNonInstanceInvokes), new object[] { })] + [InlineData(typeof(ViewComponentWithNonPublicNonInstanceInvokes), new object[] { "" })] + [InlineData(typeof(BaseClass), new object[] { })] + public void FindSyncMethod_ReturnsNull_IfMatchCannotBeFound(Type type, object[] args) + { + // Arrange + var typeInfo = type.GetTypeInfo(); + + // Act + var method = ViewComponentMethodSelector.FindSyncMethod(typeInfo, args); + + // Assert + Assert.Null(method); + } + + [Theory] + [InlineData(new object[] { new object[] { "Hello" } })] + [InlineData(new object[] { new object[] { 4 } })] + [InlineData(new object[] { new object[] { "", 5 } })] + public void FindAsyncMethod_ThrowsIfInvokeAsyncDoesNotHaveCorrectReturnType(object[] args) + { + // Arrange + var typeInfo = typeof(TypeWithInvalidInvokeAsync).GetTypeInfo(); + + // Act and Assert + var ex = Assert.Throws( + () => ViewComponentMethodSelector.FindAsyncMethod(typeInfo, args)); + Assert.Equal("The async view component method 'InvokeAsync' should be declared to return Task.", + ex.Message); + } + + [Fact] + public void FindSyncMethod_ThrowsIfInvokeSyncIsAVoidMethod() + { + // Arrange + var expectedMessage = "The view component method 'Invoke' should be declared to return a value."; + var typeInfo = typeof(TypeWithInvalidInvokeSync).GetTypeInfo(); + + // Act and Assert + var ex = Assert.Throws( + () => ViewComponentMethodSelector.FindSyncMethod(typeInfo, new object[] { 4 })); + Assert.Equal(expectedMessage, ex.Message); + } + + [Fact] + public void FindSyncMethod_ThrowsIfInvokeSyncReturnsTask() + { + // Arrange + var expectedMessage = "The view component method 'Invoke' cannot return a Task."; + var typeInfo = typeof(TypeWithInvalidInvokeSync).GetTypeInfo(); + + // Act and Assert + var ex = Assert.Throws( + () => ViewComponentMethodSelector.FindSyncMethod(typeInfo, new object[] { "" })); + Assert.Equal(expectedMessage, ex.Message); + } + + public static TheoryData FindAsyncMethod_ReturnsMethodMatchingParametersData + { + get + { + var derivedClass = new DerivedClass(); + + return new TheoryData + { + { typeof(ViewComponentWithAsyncInvoke), new object[] { "", }, "1" }, + { typeof(ViewComponentWithAsyncInvoke), new object[] { "", 2 }, "2" }, + { typeof(ViewComponentWithAsyncInvoke), new object[] { "", 0, 1 }, "3" }, + { typeof(ViewComponentWithAsyncInvoke), new object[] { 1, false, 1 }, "4" }, + { typeof(MethodsWithValueConversions), new object[] { 2, (byte)1, (byte)2 }, "2" }, + { typeof(MethodsWithValueConversions), new object[] { derivedClass, derivedClass }, "4" }, + { typeof(MethodsWithValueConversions), new object[] { CultureInfo.InvariantCulture }, "6" }, + }; + } + } + + [Theory] + [MemberData(nameof(FindAsyncMethod_ReturnsMethodMatchingParametersData))] + public void FindAsyncMethod_ReturnsMethodMatchingParameters(Type type, object[] args, string expectedId) + { + // Arrange + var typeInfo = type.GetTypeInfo(); + + // Act + var method = ViewComponentMethodSelector.FindAsyncMethod(typeInfo, args); + + // Assert + Assert.NotNull(method); + var data = method.GetCustomAttribute(); + Assert.Equal(expectedId, data.Data); + } + + public static TheoryData FindSyncMethod_ReturnsMethodMatchingParametersData + { + get + { + var derivedClass = new DerivedAgain(); + + return new TheoryData + { + { typeof(ViewComponentWithSyncInvoke), new object[] { }, "1" }, + { typeof(ViewComponentWithSyncInvoke), new object[] { 2, 3 }, "2" }, + { typeof(ViewComponentWithSyncInvoke), new object[] { "", 0, true }, "3" }, + { typeof(MethodsWithValueConversions), new object[] { 1, (byte)2, 3.0f }, "1" }, + { typeof(MethodsWithValueConversions), new object[] { derivedClass, derivedClass }, "3" }, + { typeof(MethodsWithValueConversions), new object[] { "Hello world" }, "5" }, + { typeof(DerivedClass), new object[] { }, "Derived1" }, +#if !DNXCORE50 + { typeof(DerivedAgain), new object[] { "" }, "Derived2" }, +#endif + }; + } + } + + [Theory] + [MemberData(nameof(FindSyncMethod_ReturnsMethodMatchingParametersData))] + public void FindSyncMethod_ReturnsMethodMatchingParameters(Type type, object[] args, string expectedId) + { + // Arrange + var typeInfo = type.GetTypeInfo(); + + // Act + var method = ViewComponentMethodSelector.FindSyncMethod(typeInfo, args); + + // Assert + Assert.NotNull(method); + var data = method.GetCustomAttribute(); + Assert.Equal(expectedId, data.Data); + } + + private class ViewComponentWithSyncInvoke + { + [MethodData("1")] + public int Invoke() => 3; + + [MethodData("2")] + public int Invoke(int a, int? b) => a + b.Value; + + [MethodData("3")] + public int Invoke(string a, int b, bool? c) => 3; + } + + private class ViewComponentWithAsyncInvoke + { + [MethodData("1")] + public Task InvokeAsync(string value) => Task.FromResult(value.ToUpperInvariant()); + + [MethodData("2")] + public Task InvokeAsync(string a, int b) => Task.FromResult(a + b); + + [MethodData("3")] + public Task InvokeAsync(string a, int? b, int c) => Task.FromResult(a + b + c); + + [MethodData("4")] + public Task InvokeAsync(int? a, bool? b, int c) => Task.FromResult(a.ToString() + b + c); + + [MethodData("5")] + public Task InvokeAsync(object value) => Task.FromResult(value.ToString()); + } + + public class MethodsWithValueConversions + { + [MethodData("1")] + public int Invoke(long a, char b, double c) => 1; + + [MethodData("2")] + public Task InvokeAsync(float a, float b, byte c) => Task.FromResult(1); + + [MethodData("3")] + public int Invoke(BaseClass a, DerivedClass b) => 1; + + [MethodData("4")] + public Task InvokeAsync(BaseClass a, DerivedClass b) => Task.FromResult(1); + + [MethodData("5")] + public int Invoke(IEnumerable value) => 1; + + [MethodData("6")] + public Task InvokeAsync(IFormatProvider formatProvider) => Task.FromResult(1); + } + + private class ViewComponentWithNonPublicNonInstanceInvokes + { + public static int Invoke() => 1; + + private int Invoke(string a) => 2; + + public static Task InvokeAsync() => Task.FromResult(3); + + protected Task InvokeAsync(string a) => Task.FromResult(a); + } + + public class BaseClass + { + [MethodData("Base")] + public static int Invoke() => 1; + } + + public class DerivedClass : BaseClass + { + [MethodData("Derived1")] + public new int Invoke() => 1; + + [MethodData("Derived2")] + public int Invoke(string x) => 2; + } + + public class DerivedAgain : DerivedClass + { + [MethodData("DerivedAgain")] + public new static int Invoke(string x) => 2; + } + + private class TypeWithInvalidInvokeAsync + { + public Task InvokeAsync(string value) => Task.FromResult(value); + + public void InvokeAsync(int value) + { + + } + + public long InvokeAsync(string a, int b) => b; + } + + private class TypeWithInvalidInvokeSync + { + public Task Invoke(string value) => Task.FromResult(value); + + public void Invoke(int value) + { + } + } + + private class MethodDataAttribute : Attribute + { + public MethodDataAttribute(string data) + { + Data = data; + } + + public string Data { get; } + } + } +}