From d7ccea17ce00711d9b3a882e7ef61842a5bc33a6 Mon Sep 17 00:00:00 2001 From: mnltejaswini Date: Mon, 16 May 2016 13:15:19 -0700 Subject: [PATCH] [Perf] Refactoring ControllerActionInvoker to avoid coercing action method's return type to Task for simple cases. Fixes #4539 --- .../Internal/ControllerActionExecutor.cs | 17 - .../Internal/ControllerActionInvoker.cs | 108 +-- .../Internal/ObjectMethodExecutor.cs | 131 +-- .../DefaultViewComponentInvoker.cs | 58 +- .../ViewComponentInvokerCache.cs | 9 + .../Internal/ControllerActionExecutorTests.cs | 509 ----------- .../Internal/ControllerActionInvokerTest.cs | 792 ++++++++++++++++-- .../Internal/ObjectMethodExecutorTest.cs | 94 +-- 8 files changed, 859 insertions(+), 859 deletions(-) delete mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionExecutorTests.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionExecutor.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionExecutor.cs index 965d0f8022..9f7b4b5808 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionExecutor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionExecutor.cs @@ -11,23 +11,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal { public static class ControllerActionExecutor { - public static Task ExecuteAsync( - ObjectMethodExecutor actionMethodExecutor, - object instance, - IDictionary actionArguments) - { - var orderedArguments = PrepareArguments(actionArguments, actionMethodExecutor); - return ExecuteAsync(actionMethodExecutor, instance, orderedArguments); - } - - public static Task ExecuteAsync( - ObjectMethodExecutor actionMethodExecutor, - object instance, - object[] orderedActionArguments) - { - return actionMethodExecutor.ExecuteAsync(instance, orderedActionArguments); - } - public static object[] PrepareArguments( IDictionary actionParameters, ObjectMethodExecutor actionMethodExecutor) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs index d699c6f6cf..367b17a061 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs @@ -573,12 +573,67 @@ namespace Microsoft.AspNetCore.Mvc.Internal _logger.ActionMethodExecuting(_actionExecutingContext, arguments); - var actionReturnValue = await ControllerActionExecutor.ExecuteAsync( - _executor, - _controller, - arguments); + var returnType = _executor.MethodReturnType; - result = CreateActionResult( actionMethodInfo.ReturnType, actionReturnValue); + if (returnType == typeof(void)) + { + _executor.Execute(_controller, arguments); + result = new EmptyResult(); + } + else if (returnType == typeof(Task)) + { + await (Task)_executor.Execute(_controller, arguments); + result = new EmptyResult(); + } + else if (_executor.TaskGenericType == typeof(IActionResult)) + { + result = await (Task)_executor.Execute(_controller, arguments); + if (result == null) + { + throw new InvalidOperationException( + Resources.FormatActionResult_ActionReturnValueCannotBeNull(typeof(IActionResult))); + } + } + else if (_executor.IsTypeAssignableFromIActionResult) + { + if (_executor.IsMethodAsync) + { + result = (IActionResult)await _executor.ExecuteAsync(_controller, arguments); + } + else + { + result = (IActionResult)_executor.Execute(_controller, arguments); + } + + if (result == null) + { + throw new InvalidOperationException( + Resources.FormatActionResult_ActionReturnValueCannotBeNull(_executor.TaskGenericType ?? returnType)); + } + } + else if (!_executor.IsMethodAsync) + { + var resultAsObject = _executor.Execute(_controller, arguments); + result = new ObjectResult(resultAsObject) + { + DeclaredType = returnType, + }; + } + else if (_executor.TaskGenericType != null) + { + var resultAsObject = await _executor.ExecuteAsync(_controller, arguments); + result = new ObjectResult(resultAsObject) + { + DeclaredType = _executor.TaskGenericType, + }; + } + else + { + // This will be the case for types which have derived from Task and Task or non Task types. + throw new InvalidOperationException(Resources.FormatActionExecutor_UnexpectedTaskInstance( + _executor.MethodInfo.Name, + _executor.MethodInfo.DeclaringType)); + } _logger.ActionMethodExecuted(_actionExecutingContext, result); } @@ -773,49 +828,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } - // Marking as internal for Unit Testing purposes. - internal static IActionResult CreateActionResult(Type declaredReturnType, object actionReturnValue) - { - if (declaredReturnType == null) - { - throw new ArgumentNullException(nameof(declaredReturnType)); - } - - // optimize common path - var actionResult = actionReturnValue as IActionResult; - if (actionResult != null) - { - return actionResult; - } - - if (declaredReturnType == typeof(void) || - declaredReturnType == typeof(Task)) - { - return new EmptyResult(); - } - - // Unwrap potential Task types. - var actualReturnType = GetTaskInnerTypeOrNull(declaredReturnType) ?? declaredReturnType; - if (actionReturnValue == null && - typeof(IActionResult).IsAssignableFrom(actualReturnType)) - { - throw new InvalidOperationException( - Resources.FormatActionResult_ActionReturnValueCannotBeNull(actualReturnType)); - } - - return new ObjectResult(actionReturnValue) - { - DeclaredType = actualReturnType - }; - } - - private static Type GetTaskInnerTypeOrNull(Type type) - { - var genericType = ClosedGenericMatcher.ExtractGenericInterface(type, typeof(Task<>)); - - return genericType?.GenericTypeArguments[0]; - } - /// /// A one-way cursor for filters. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectMethodExecutor.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectMethodExecutor.cs index 2c91270b1a..39990ab8bf 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectMethodExecutor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectMethodExecutor.cs @@ -22,25 +22,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal private static readonly MethodInfo _convertOfTMethod = typeof(ObjectMethodExecutor).GetRuntimeMethods().Single(methodInfo => methodInfo.Name == nameof(ObjectMethodExecutor.Convert)); - private static readonly Expression>> _createTaskFromResultExpression = - ((result) => Task.FromResult(result)); - - private static readonly MethodInfo _createTaskFromResultMethod = - ((MethodCallExpression)_createTaskFromResultExpression.Body).Method; - - private static readonly Expression>> _coerceTaskExpression = - ((result, methodName, declaringType) => ObjectMethodExecutor.CoerceTaskType(result, methodName, declaringType)); - - private static readonly MethodInfo _coerceMethod = ((MethodCallExpression)_coerceTaskExpression.Body).Method; - - private ObjectMethodExecutor(MethodInfo methodInfo) + private ObjectMethodExecutor(MethodInfo methodInfo, TypeInfo targetTypeInfo) { if (methodInfo == null) { throw new ArgumentNullException(nameof(methodInfo)); } MethodInfo = methodInfo; + TargetTypeInfo = targetTypeInfo; ActionParameters = methodInfo.GetParameters(); + MethodReturnType = methodInfo.ReturnType; + IsMethodAsync = typeof(Task).IsAssignableFrom(MethodReturnType); + TaskGenericType = IsMethodAsync ? GetTaskInnerTypeOrNull(MethodReturnType) : null; + IsTypeAssignableFromIActionResult = typeof(IActionResult).IsAssignableFrom(TaskGenericType ?? MethodReturnType); } private delegate Task ActionExecutorAsync(object target, object[] parameters); @@ -53,17 +47,40 @@ namespace Microsoft.AspNetCore.Mvc.Internal public ParameterInfo[] ActionParameters { get; } + public TypeInfo TargetTypeInfo { get; } + + public Type TaskGenericType { get; } + + // This field is made internal set because it is set in unit tests. + public Type MethodReturnType { get; internal set; } + + public bool IsMethodAsync { get; } + + public bool IsTypeAssignableFromIActionResult { get; } + + private ActionExecutorAsync TaskOfTActionExecutorAsync + { + get + { + if (_executorAsync == null) + { + _executorAsync = GetExecutorAsync(TaskGenericType, MethodInfo, TargetTypeInfo); + } + + return _executorAsync; + } + } + public static ObjectMethodExecutor Create(MethodInfo methodInfo, TypeInfo targetTypeInfo) { - var executor = new ObjectMethodExecutor(methodInfo); + var executor = new ObjectMethodExecutor(methodInfo, targetTypeInfo); executor._executor = GetExecutor(methodInfo, targetTypeInfo); - executor._executorAsync = GetExecutorAsync(methodInfo, targetTypeInfo); return executor; } public Task ExecuteAsync(object target, object[] parameters) { - return _executorAsync(target, parameters); + return TaskOfTActionExecutorAsync(target, parameters); } public object Execute(object target, object[] parameters) @@ -132,7 +149,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal }; } - private static ActionExecutorAsync GetExecutorAsync(MethodInfo methodInfo, TypeInfo targetTypeInfo) + private static ActionExecutorAsync GetExecutorAsync(Type taskInnerType, MethodInfo methodInfo, TypeInfo targetTypeInfo) { // Parameters to executor var targetParameter = Expression.Parameter(typeof(object), "target"); @@ -155,83 +172,27 @@ namespace Microsoft.AspNetCore.Mvc.Internal var instanceCast = Expression.Convert(targetParameter, targetTypeInfo.AsType()); var methodCall = Expression.Call(instanceCast, methodInfo, parameters); - // methodCall is "((Ttarget) target) method((T0) parameters[0], (T1) parameters[1], ...)" - // Create function - if (methodCall.Type == typeof(void)) - { - var lambda = Expression.Lambda(methodCall, targetParameter, parametersParameter); - var voidExecutor = lambda.Compile(); - return WrapVoidActionAsync(voidExecutor); - } - else - { - // must coerce methodCall to match ActionExecutorAsync signature - var coerceMethodCall = GetCoerceMethodCallExpression(methodCall, methodInfo); - var lambda = Expression.Lambda(coerceMethodCall, targetParameter, parametersParameter); - return lambda.Compile(); - } + var coerceMethodCall = GetCoerceMethodCallExpression(taskInnerType, methodCall, methodInfo); + var lambda = Expression.Lambda(coerceMethodCall, targetParameter, parametersParameter); + return lambda.Compile(); } // We need to CoerceResult as the object value returned from methodInfo.Invoke has to be cast to a Task. // This is necessary to enable calling await on the returned task. // i.e we need to write the following var result = await (Task)mInfo.Invoke. // Returning Task enables us to await on the result. - private static Expression GetCoerceMethodCallExpression(MethodCallExpression methodCall, MethodInfo methodInfo) + private static Expression GetCoerceMethodCallExpression( + Type taskValueType, + MethodCallExpression methodCall, + MethodInfo methodInfo) { var castMethodCall = Expression.Convert(methodCall, typeof(object)); - var returnType = methodCall.Type; - - if (typeof(Task).IsAssignableFrom(returnType)) - { - if (returnType == typeof(Task)) - { - var stringExpression = Expression.Constant(methodInfo.Name); - var typeExpression = Expression.Constant(methodInfo.DeclaringType); - return Expression.Call(null, _coerceMethod, castMethodCall, stringExpression, typeExpression); - } - - var taskValueType = GetTaskInnerTypeOrNull(returnType); - if (taskValueType != null) - { - // for: public Task Action() - // constructs: return (Task)Convert((Task)result) - var genericMethodInfo = _convertOfTMethod.MakeGenericMethod(taskValueType); - var genericMethodCall = Expression.Call(null, genericMethodInfo, castMethodCall); - var convertedResult = Expression.Convert(genericMethodCall, typeof(Task)); - return convertedResult; - } - - // This will be the case for types which have derived from Task and Task - throw new InvalidOperationException(Resources.FormatActionExecutor_UnexpectedTaskInstance( - methodInfo.Name, - methodInfo.DeclaringType)); - } - - return Expression.Call(null, _createTaskFromResultMethod, castMethodCall); - } - - private static ActionExecutorAsync WrapVoidActionAsync(VoidActionExecutor executor) - { - return delegate (object target, object[] parameters) - { - executor(target, parameters); - return Task.FromResult(null); - }; - } - - private static Task CoerceTaskType(object result, string methodName, Type declaringType) - { - var resultAsTask = (Task)result; - return CastToObject(resultAsTask); - } - - /// - /// Cast Task to Task of object - /// - private static async Task CastToObject(Task task) - { - await task; - return null; + // for: public Task Action() + // constructs: return (Task)Convert((Task)result) + var genericMethodInfo = _convertOfTMethod.MakeGenericMethod(taskValueType); + var genericMethodCall = Expression.Call(null, genericMethodInfo, castMethodCall); + var convertedResult = Expression.Convert(genericMethodCall, typeof(Task)); + return convertedResult; } /// diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/DefaultViewComponentInvoker.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/DefaultViewComponentInvoker.cs index 274fd9defb..e086c3a633 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/DefaultViewComponentInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/DefaultViewComponentInvoker.cs @@ -71,48 +71,64 @@ namespace Microsoft.AspNetCore.Mvc.ViewComponents throw new ArgumentNullException(nameof(context)); } - var methodInfo = context.ViewComponentDescriptor?.MethodInfo; - if (methodInfo == null) + var executor = _viewComponentInvokerCache.GetViewComponentMethodExecutor(context); + + var returnType = executor.MethodReturnType; + + if (returnType == typeof(void) || returnType == typeof(Task)) { - throw new InvalidOperationException(Resources.FormatPropertyOfTypeCannotBeNull( - nameof(ViewComponentDescriptor.MethodInfo), - nameof(ViewComponentDescriptor))); + throw new InvalidOperationException(Resources.ViewComponent_MustReturnValue); } - var isAsync = typeof(Task).IsAssignableFrom(methodInfo.ReturnType); IViewComponentResult result; - if (isAsync) + if (executor.IsMethodAsync) { - result = await InvokeAsyncCore(context); + result = await InvokeAsyncCore(executor, context); } else { // We support falling back to synchronous if there is no InvokeAsync method, in this case we'll still // execute the IViewResult asynchronously. - result = InvokeSyncCore(context); + result = InvokeSyncCore(executor, context); } await result.ExecuteAsync(context); } - private async Task InvokeAsyncCore(ViewComponentContext context) + private async Task InvokeAsyncCore(ObjectMethodExecutor executor, ViewComponentContext context) { var component = _viewComponentFactory.CreateViewComponent(context); using (_logger.ViewComponentScope(context)) { - var method = context.ViewComponentDescriptor.MethodInfo; - var methodExecutor = _viewComponentInvokerCache.GetViewComponentMethodExecutor(context); - - var arguments = ControllerActionExecutor.PrepareArguments(context.Arguments, methodExecutor); + var arguments = ControllerActionExecutor.PrepareArguments(context.Arguments, executor); _diagnosticSource.BeforeViewComponent(context, component); _logger.ViewComponentExecuting(context, arguments); var startTimestamp = _logger.IsEnabled(LogLevel.Debug) ? Stopwatch.GetTimestamp() : 0; - var result = await ControllerActionExecutor.ExecuteAsync(methodExecutor, component, arguments); - var viewComponentResult = CoerceToViewComponentResult(result); + object resultAsObject = null; + var taskGenericType = executor.TaskGenericType; + + if (taskGenericType == typeof(IViewComponentResult)) + { + resultAsObject = await (Task)executor.Execute(component, arguments); + } + else if (taskGenericType == typeof(string)) + { + resultAsObject = await (Task)executor.Execute(component, arguments); + } + else if (taskGenericType == typeof(IHtmlContent)) + { + resultAsObject = await (Task)executor.Execute(component, arguments); + } + else + { + resultAsObject = await executor.ExecuteAsync(component, arguments); + } + + var viewComponentResult = CoerceToViewComponentResult(resultAsObject); _logger.ViewComponentExecuted(context, startTimestamp, viewComponentResult); _diagnosticSource.AfterViewComponent(context, viewComponentResult, component); @@ -122,27 +138,25 @@ namespace Microsoft.AspNetCore.Mvc.ViewComponents } } - private IViewComponentResult InvokeSyncCore(ViewComponentContext context) + private IViewComponentResult InvokeSyncCore(ObjectMethodExecutor executor, ViewComponentContext context) { var component = _viewComponentFactory.CreateViewComponent(context); using (_logger.ViewComponentScope(context)) { - var method = context.ViewComponentDescriptor.MethodInfo; - var methodExecutor = _viewComponentInvokerCache.GetViewComponentMethodExecutor(context); - var arguments = ControllerActionExecutor.PrepareArguments( context.Arguments, - methodExecutor); + executor); _diagnosticSource.BeforeViewComponent(context, component); _logger.ViewComponentExecuting(context, arguments); var startTimestamp = _logger.IsEnabled(LogLevel.Debug) ? Stopwatch.GetTimestamp() : 0; object result; + try { - result = methodExecutor.Execute(component, arguments); + result = executor.Execute(component, arguments); } finally { diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/ViewComponentInvokerCache.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/ViewComponentInvokerCache.cs index 285312c5bc..c3d57f846d 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/ViewComponentInvokerCache.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/ViewComponentInvokerCache.cs @@ -1,6 +1,7 @@ // 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.Concurrent; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ViewComponents; @@ -46,6 +47,14 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal return executor; } + var methodInfo = viewComponentContext.ViewComponentDescriptor?.MethodInfo; + if (methodInfo == null) + { + throw new InvalidOperationException(Resources.FormatPropertyOfTypeCannotBeNull( + nameof(ViewComponentDescriptor.MethodInfo), + nameof(ViewComponentDescriptor))); + } + executor = ObjectMethodExecutor.Create(viewComponentDescriptor.MethodInfo, viewComponentDescriptor.TypeInfo); cache.Entries.TryAdd(viewComponentDescriptor, executor); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionExecutorTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionExecutorTests.cs deleted file mode 100644 index 38b962d130..0000000000 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionExecutorTests.cs +++ /dev/null @@ -1,509 +0,0 @@ -// 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.ComponentModel; -using System.Globalization; -using System.Reflection; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Testing; -using Xunit; - -namespace Microsoft.AspNetCore.Mvc.Internal -{ - public class ControllerActionExecutorTests - { - private TestController _controller = new TestController(); - - private delegate void MethodWithVoidReturnType(); - - private delegate string SyncMethod(string s); - - private delegate Task MethodWithTaskReturnType(int i, string s); - - private delegate Task MethodWithTaskOfIntReturnType(int i, string s); - - private delegate Task> MethodWithTaskOfTaskOfIntReturnType(int i, string s); - - public delegate TestController.TaskDerivedType MethodWithCustomTaskReturnType(int i, string s); - - private delegate TestController.TaskOfTDerivedType MethodWithCustomTaskOfTReturnType(int i, string s); - - private delegate dynamic ReturnTaskAsDynamicValue(int i, string s); - - [Fact] - public async Task AsyncAction_TaskReturnType() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - var methodWithTaskReturnType = new MethodWithTaskReturnType(_controller.TaskAction); - var result = await ExecuteAction( - methodWithTaskReturnType, - _controller, - actionParameters); - - // Assert - Assert.Same(null, result); - } - - [Fact] - public async Task AsyncAction_TaskOfValueReturnType() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - var methodWithTaskOfIntReturnType = new MethodWithTaskOfIntReturnType(_controller.TaskValueTypeAction); - - // Act - var result = await ExecuteAction( - methodWithTaskOfIntReturnType, - _controller, - actionParameters); - // Assert - Assert.Equal(inputParam1, result); - } - - [Fact] - public async Task AsyncAction_TaskOfTaskOfValueReturnType() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - var methodWithTaskOfTaskOfIntReturnType = new MethodWithTaskOfTaskOfIntReturnType(_controller.TaskOfTaskAction); - - // Act - var result = await (Task)( await ExecuteAction( - methodWithTaskOfTaskOfIntReturnType, - _controller, - actionParameters)); - - // Assert - Assert.Equal(inputParam1, result); - } - - [Fact] - public async Task AsyncAction_WithAsyncKeywordThrows() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - var methodWithTaskOfIntReturnType = new MethodWithTaskOfIntReturnType(_controller.TaskActionWithException); - - // Act and Assert - await Assert.ThrowsAsync( - () => ExecuteAction( - methodWithTaskOfIntReturnType, - _controller, - actionParameters)); - } - - [Fact] - public async Task AsyncAction_WithoutAsyncThrows() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - var methodWithTaskOfIntReturnType = new MethodWithTaskOfIntReturnType(_controller.TaskActionWithExceptionWithoutAsync); - - // Act & Assert - await Assert.ThrowsAsync( - () => ExecuteAction( - methodWithTaskOfIntReturnType, - _controller, - actionParameters)); - } - - [Fact] - public async Task AsyncAction_WithExceptionsAfterAwait() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - var methodWithTaskOfIntReturnType = new MethodWithTaskOfIntReturnType(_controller.TaskActionThrowAfterAwait); - var expectedException = "Argument Exception"; - - // Act & Assert - var ex = await Assert.ThrowsAsync( - () => ExecuteAction( - methodWithTaskOfIntReturnType, - _controller, - actionParameters)); - Assert.Equal(expectedException, ex.Message); - } - - [Fact] - public async Task SyncAction() - { - // Arrange - var inputString = "hello"; - var syncMethod = new SyncMethod(_controller.Echo); - - // Act - var result = await ExecuteAction( - syncMethod, - _controller, - new Dictionary() { { "input", inputString } }); - // Assert - Assert.Equal(inputString, result); - } - - [Fact] - public async Task SyncAction_WithException() - { - // Arrange - var inputString = "hello"; - var syncMethod = new SyncMethod(_controller.EchoWithException); - - // Act & Assert - await Assert.ThrowsAsync( - () => ExecuteAction( - syncMethod, - _controller, - new Dictionary() { { "input", inputString } })); - } - - [Fact] - public async Task ExecuteAsync_WithArgumentDictionary_DefaultValueAttributeUsed() - { - // Arrange - var syncMethod = new SyncMethod(_controller.EchoWithDefaultValue); - - // Act - var result = await ExecuteAction( - syncMethod, - _controller, - new Dictionary()); - - // Assert - Assert.Equal("hello", result); - } - - [Fact] - public async Task ExecuteAsync_WithArgumentArray_DefaultValueAttributeIgnored() - { - // Arrange - var syncMethod = new SyncMethod(_controller.EchoWithDefaultValue); - - // Act - var result = await ExecuteAction( - syncMethod, - _controller, - new object[] { null, }); - - // Assert - Assert.Null(result); - } - - [Fact] - public async Task ExecuteAsync_WithArgumentDictionary_DefaultParameterValueUsed() - { - // Arrange - var syncMethod = new SyncMethod(_controller.EchoWithDefaultValueAndAttribute); - - // Act - var result = await ExecuteAction( - syncMethod, - _controller, - new Dictionary()); - - // Assert - Assert.Equal("world", result); - } - - [Fact] - public async Task ExecuteAsync_WithArgumentDictionary_AnyValue_HasPrecedenceOverDefaults() - { - // Arrange - var syncMethod = new SyncMethod(_controller.EchoWithDefaultValueAndAttribute); - - // Act - var result = await ExecuteAction( - syncMethod, - _controller, - new Dictionary() { { "input", null } }); - - // Assert - Assert.Null(result); - } - - [Fact] - public async Task AsyncAction_WithCustomTaskReturnTypeThrows() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - // If it is an unrecognized derived type we throw an InvalidOperationException. - var methodWithCutomTaskReturnType = new MethodWithCustomTaskReturnType(_controller.TaskActionWithCustomTaskReturnType); - - var expectedException = string.Format( - CultureInfo.CurrentCulture, - "The method 'TaskActionWithCustomTaskReturnType' on type '{0}' returned a Task instance even though it is not an asynchronous method.", - typeof(TestController)); - - // Act & Assert - var ex = await Assert.ThrowsAsync( - () => ExecuteAction( - methodWithCutomTaskReturnType, - _controller, - actionParameters)); - Assert.Equal(expectedException, ex.Message); - } - - [Fact] - public async Task AsyncAction_WithCustomTaskOfTReturnTypeThrows() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - var methodWithCutomTaskOfTReturnType = new MethodWithCustomTaskOfTReturnType(_controller.TaskActionWithCustomTaskOfTReturnType); - var expectedException = string.Format( - CultureInfo.CurrentCulture, - "The method 'TaskActionWithCustomTaskOfTReturnType' on type '{0}' returned a Task instance even though it is not an asynchronous method.", - typeof(TestController)); - - // Act & Assert - var ex = await Assert.ThrowsAsync( - () => ExecuteAction( - methodWithCutomTaskOfTReturnType, - _controller, - actionParameters)); - Assert.Equal(expectedException, ex.Message); - } - - // Async actions that declares a return type of Task and returns an instance of Task or Task> - // are not handled in any special way as they are not supported. They are considered as methods that return Task type. - [Fact] - public async Task AsyncAction_ReturningUnwrappedTask() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - var methodWithUnwrappedTask = new MethodWithTaskReturnType(_controller.UnwrappedTask); - - // Act - var result = await ExecuteAction( - methodWithUnwrappedTask, - _controller, - actionParameters); - - // Assert - Assert.Same(null, result); - } - - [Fact] - // Async actions that has dynamic return type but return Task or Task are not handled in any special way as, - // they are not supported. They are considered as regular methods that return object type. - public async Task AsyncAction_WithDynamicReturnType() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; - - var dynamicTaskMethod = new ReturnTaskAsDynamicValue(_controller.ReturnTaskAsDynamicValue); - - // Act - var result = await (Task)(await ExecuteAction( - dynamicTaskMethod, - _controller, - actionParameters)); - - // Assert - Assert.Equal(inputParam1, result); - } - - [Fact] - public async Task ParametersInRandomOrder() - { - // Arrange - var inputParam1 = 1; - var inputParam2 = "Second Parameter"; - - // Note that the order of parameters is reversed - var actionParameters = new Dictionary { { "s", inputParam2 }, { "i", inputParam1 } }; - var methodWithTaskOfIntReturnType = new MethodWithTaskOfIntReturnType(_controller.TaskValueTypeAction); - - // Act - var result = await ExecuteAction( - methodWithTaskOfIntReturnType, - _controller, - actionParameters); - - // Assert - Assert.Equal(inputParam1, result); - } - - [Fact] - public async Task InvalidParameterValueThrows() - { - // Arrange - var inputParam2 = "Second Parameter"; - - var actionParameters = new Dictionary { { "i", "Some Invalid Value" }, { "s", inputParam2 } }; - var methodWithTaskOfIntReturnType = new MethodWithTaskOfIntReturnType(_controller.TaskValueTypeAction); - - // Act & Assert - // If it is an unrecognized derived type we throw an InvalidOperationException. - var ex = await Assert.ThrowsAsync( - () => ExecuteAction( - methodWithTaskOfIntReturnType, - _controller, - actionParameters)); - } - - private async Task ExecuteAction( - Delegate methodDelegate, - TestController controller, - IDictionary actionParameters) - { - var executor = ObjectMethodExecutor.Create(methodDelegate.GetMethodInfo(), _controller.GetType().GetTypeInfo()); - - var result = await ControllerActionExecutor.ExecuteAsync( - executor, - controller, - actionParameters); - - return result; - } - - private async Task ExecuteAction( - Delegate methodDelegate, - TestController controller, - object[] actionParameters) - { - var executor = ObjectMethodExecutor.Create(methodDelegate.GetMethodInfo(), _controller.GetType().GetTypeInfo()); - - var result = await ControllerActionExecutor.ExecuteAsync( - executor, - controller, - actionParameters); - return result; - } - - public class TestController - { -#pragma warning disable 1998 - public async Task TaskAction(int i, string s) - { - return; - } -#pragma warning restore 1998 - -#pragma warning disable 1998 - public async Task TaskValueTypeAction(int i, string s) - { - return i; - } -#pragma warning restore 1998 - -#pragma warning disable 1998 - public async Task> TaskOfTaskAction(int i, string s) - { - return TaskValueTypeAction(i, s); - } -#pragma warning restore 1998 - - public Task TaskValueTypeActionWithoutAsync(int i, string s) - { - return TaskValueTypeAction(i, s); - } - -#pragma warning disable 1998 - public async Task TaskActionWithException(int i, string s) - { - throw new NotImplementedException("Not Implemented Exception"); - } -#pragma warning restore 1998 - - public Task TaskActionWithExceptionWithoutAsync(int i, string s) - { - throw new NotImplementedException("Not Implemented Exception"); - } - - public async Task TaskActionThrowAfterAwait(int i, string s) - { - await Task.Delay(500); - throw new ArgumentException("Argument Exception"); - } - - public TaskDerivedType TaskActionWithCustomTaskReturnType(int i, string s) - { - return new TaskDerivedType(); - } - - public TaskOfTDerivedType TaskActionWithCustomTaskOfTReturnType(int i, string s) - { - return new TaskOfTDerivedType(1); - } - - /// - /// Returns a instead of a . - /// - public Task UnwrappedTask(int i, string s) - { - return Task.Factory.StartNew(async () => await Task.Factory.StartNew(() => i)); - } - - public string Echo(string input) - { - return input; - } - - public string EchoWithException(string input) - { - throw new NotImplementedException(); - } - - public string EchoWithDefaultValue([DefaultValue("hello")] string input) - { - return input; - } - - public string EchoWithDefaultValueAndAttribute([DefaultValue("hello")] string input = "world") - { - return input; - } - - public dynamic ReturnTaskAsDynamicValue(int i, string s) - { - return Task.Factory.StartNew(() => i); - } - - public class TaskDerivedType : Task - { - public TaskDerivedType() - : base(() => Console.WriteLine("In The Constructor")) - { - } - } - - public class TaskOfTDerivedType : Task - { - public TaskOfTDerivedType(T input) - : base(() => input) - { - } - } - } - } -} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs index 1e0833c753..9224dec9f7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs @@ -3,7 +3,9 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Diagnostics; +using System.Globalization; using System.IO; using System.Linq; using System.Reflection; @@ -33,10 +35,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal private readonly ContentResult _result = new ContentResult() { Content = "Hello, world!" }; - private struct SampleStruct - { - public int x; - } + private readonly TestController _controller = new TestController(); [Fact] public async Task InvokeAction_DoesNotInvokeExceptionFilter_WhenActionDoesNotThrow() @@ -1863,73 +1862,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.False(invoker.ControllerFactory.CreateCalled); } - [Fact] - public void CreateActionResult_ReturnsSameActionResult() - { - // Arrange - var mockActionResult = new Mock(); - - // Assert - var result = ControllerActionInvoker.CreateActionResult( - mockActionResult.Object.GetType(), mockActionResult.Object); - - // Act - Assert.Same(mockActionResult.Object, result); - } - - [Fact] - [ReplaceCulture] - public void CreateActionResult_NullActionResultReturnValueThrows() - { - // Arrange, Act & Assert - ExceptionAssert.Throws( - () => ControllerActionInvoker.CreateActionResult(typeof(IActionResult), null), - "Cannot return null from an action method with a return type of '" - + typeof(IActionResult) - + "'."); - } - - [Theory] - [InlineData(typeof(void))] - [InlineData(typeof(Task))] - public void CreateActionResult_Types_ReturnsEmptyResultForTaskAndVoidReturnTypes(Type type) - { - // Arrange & Act - var result = ControllerActionInvoker.CreateActionResult(type, null); - - // Assert - Assert.IsType(result); - } - - public static IEnumerable CreateActionResult_ReturnsObjectContentResultData - { - get - { - var anonymousObject = new { x1 = 10, y1 = "Hello" }; - yield return new object[] { anonymousObject.GetType(), anonymousObject, }; - yield return new object[] { typeof(int), 5 }; - yield return new object[] { typeof(string), "sample input" }; - - SampleStruct test; - test.x = 10; - yield return new object[] { test.GetType(), test }; - yield return new object[] { typeof(Task), 5 }; - yield return new object[] { typeof(Task), "Hello world" }; - } - } - - [Theory] - [MemberData(nameof(CreateActionResult_ReturnsObjectContentResultData))] - public void CreateActionResult_ReturnsObjectContentResult(Type type, object input) - { - // Arrange & Act - var actualResult = ControllerActionInvoker.CreateActionResult(type, input); - - // Assert - var contentResult = Assert.IsType(actualResult); - Assert.Same(input, contentResult.Value); - } - [Fact] public async Task MaxAllowedErrorsIsSet_BeforeCallingAuthorizationFilter() { @@ -1946,6 +1878,488 @@ namespace Microsoft.AspNetCore.Mvc.Internal await invoker.InvokeAsync(); } + [Fact] + public async Task InvokeAction_AsyncAction_TaskReturnType() + { + // Arrange + var inputParam1 = 1; + var inputParam2 = "Second Parameter"; + var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker(new[] { filter.Object }, nameof(TestController.TaskAction), actionParameters); + + // Act + await invoker.InvokeAsync(); + + // Assert + Assert.IsType(result); + } + + [Fact] + public async Task InvokeAction_AsyncAction_TaskOfValueReturnType() + { + // Arrange + var inputParam1 = 1; + var inputParam2 = "Second Parameter"; + var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker(new[] { filter.Object }, nameof(TestController.TaskValueTypeAction), actionParameters); + + // Act + await invoker.InvokeAsync(); + + // Assert + var contentResult = Assert.IsType(result); + Assert.Equal(inputParam1, contentResult.Value); + } + + [Fact] + public async Task InvokeAction_AsyncAction_WithAsyncKeywordThrows() + { + // Arrange + var inputParam1 = 1; + var inputParam2 = "Second Parameter"; + var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker(new[] { filter.Object }, nameof(TestController.TaskActionWithException), actionParameters); + + // Act and Assert + await Assert.ThrowsAsync( + () => invoker.InvokeAsync()); + } + + [Fact] + public async Task InvokeAction_AsyncAction_WithoutAsyncThrows() + { + // Arrange + var inputParam1 = 1; + var inputParam2 = "Second Parameter"; + var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker(new[] { filter.Object }, nameof(TestController.TaskActionWithExceptionWithoutAsync), actionParameters); + + // Act and Assert + await Assert.ThrowsAsync( + () => invoker.InvokeAsync()); + } + + public async Task InvokeAction_AsyncAction_WithExceptionsAfterAwait() + { + // Arrange + var inputParam1 = 1; + var inputParam2 = "Second Parameter"; + var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker(new[] { filter.Object }, nameof(TestController.TaskActionThrowAfterAwait), actionParameters); + var expectedException = "Argument Exception"; + + // Act and Assert + var ex = await Assert.ThrowsAsync( + () => invoker.InvokeAsync()); + Assert.Equal(expectedException, ex.Message); + } + + [Fact] + public async Task InvokeAction_SyncAction() + { + // Arrange + var inputString = "hello"; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker(new[] { filter.Object }, nameof(TestController.Echo), new Dictionary() { { "input", inputString } }); + + // Act + await invoker.InvokeAsync(); + + // Assert + var contentResult = Assert.IsType(result); + Assert.Equal(inputString, contentResult.Value); + } + + [Fact] + public async Task InvokeAction_SyncAction_WithException() + { + // Arrange + var inputString = "hello"; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + nameof(TestController.EchoWithException), + new Dictionary() { { "input", inputString } }); + + // Act & Assert + await Assert.ThrowsAsync( + () => invoker.InvokeAsync()); + } + + [Fact] + public async Task InvokeAction_SyncMethod_WithArgumentDictionary_DefaultValueAttributeUsed() + { + // Arrange + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + nameof(TestController.EchoWithDefaultValue), + new Dictionary()); + + // Act + await invoker.InvokeAsync(); + + // Assert + var contentResult = Assert.IsType(result); + Assert.Equal("hello", contentResult.Value); + } + + [Fact] + public async Task InvokeAction_SyncMethod_WithArgumentArray_DefaultValueAttributeIgnored() + { + // Arrange + var inputString = "test"; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + nameof(TestController.EchoWithDefaultValue), + new Dictionary() { { "input", inputString } }); + + // Act + await invoker.InvokeAsync(); + + // Assert + var contentResult = Assert.IsType(result); + Assert.Equal(inputString, contentResult.Value); + } + + [Fact] + public async Task InvokeAction_SyncMethod_WithArgumentDictionary_DefaultParameterValueUsed() + { + // Arrange + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + nameof(TestController.EchoWithDefaultValueAndAttribute), + new Dictionary()); + + // Act + await invoker.InvokeAsync(); + + // Assert + var contentResult = Assert.IsType(result); + Assert.Equal("world", contentResult.Value); + } + + [Fact] + public async Task InvokeAction_SyncMethod_WithArgumentDictionary_AnyValue_HasPrecedenceOverDefaults() + { + // Arrange + var inputString = "test"; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + nameof(TestController.EchoWithDefaultValueAndAttribute), + new Dictionary() { { "input", inputString } }); + + // Act + await invoker.InvokeAsync(); + + // Assert + var contentResult = Assert.IsType(result); + Assert.Equal(inputString, contentResult.Value); + } + + [Fact] + public async Task InvokeAction_AsyncAction_WithCustomTaskReturnTypeThrows() + { + // Arrange + var inputParam1 = 1; + var inputParam2 = "Second Parameter"; + var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + nameof(TestController.TaskActionWithCustomTaskReturnType), + actionParameters); + + var expectedException = string.Format( + CultureInfo.CurrentCulture, + "The method 'TaskActionWithCustomTaskReturnType' on type '{0}' returned a Task instance even though it is not an asynchronous method.", + typeof(TestController)); + + // Act & Assert + var ex = await Assert.ThrowsAsync( + () => invoker.InvokeAsync()); + Assert.Equal(expectedException, ex.Message); + } + + [Fact] + public async Task InvokeAction_AsyncAction_WithCustomTaskOfTReturnTypeThrows() + { + // Arrange + var inputParam1 = 1; + var inputParam2 = "Second Parameter"; + var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + nameof(TestController.TaskActionWithCustomTaskOfTReturnType), + actionParameters); + + var expectedException = string.Format( + CultureInfo.CurrentCulture, + "The method 'TaskActionWithCustomTaskOfTReturnType' on type '{0}' returned a Task instance even though it is not an asynchronous method.", + typeof(TestController)); + + // Act & Assert + var ex = await Assert.ThrowsAsync( + () => invoker.InvokeAsync()); + Assert.Equal(expectedException, ex.Message); + } + + [Fact] + public async Task InvokeAction_AsyncAction_ReturningUnwrappedTask() + { + // Arrange + var inputParam1 = 1; + var inputParam2 = "Second Parameter"; + var actionParameters = new Dictionary { { "i", inputParam1 }, { "s", inputParam2 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker(new[] { filter.Object }, nameof(TestController.UnwrappedTask), actionParameters); + + // Act + await invoker.InvokeAsync(); + + // Assert + Assert.IsType(result); + } + + [Fact] + public async Task InvokeAction_AsyncMethod_ParametersInRandomOrder() + { + //Arrange + var inputParam1 = 1; + var inputParam2 = "Second Parameter"; + + // Note that the order of parameters is reversed + var actionParameters = new Dictionary { { "s", inputParam2 }, { "i", inputParam1 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + nameof(TestController.TaskValueTypeAction), + actionParameters); + + // Act + await invoker.InvokeAsync(); + + // Assert + var contentResult = Assert.IsType(result); + Assert.Equal(inputParam1, contentResult.Value); + } + + [Theory] + [InlineData(nameof(TestController.AsynActionMethodWithTestActionResult))] + [InlineData(nameof(TestController.ActionMethodWithTestActionResult))] + public async Task InvokeAction_ReturnTypeAsIActionResult_ReturnsExpected(string methodName) + { + //Arrange + var inputParam = 1; + var actionParameters = new Dictionary { { "value", inputParam } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + methodName, + actionParameters); + + // Act + await invoker.InvokeAsync(); + + // Assert + var contentResult = Assert.IsType(result); + Assert.Equal(inputParam, contentResult.Value); + } + + [Fact] + public async Task InvokeAction_AsyncMethod_InvalidParameterValueThrows() + { + //Arrange + var inputParam2 = "Second Parameter"; + + var actionParameters = new Dictionary { { "i", "Some Invalid Value" }, { "s", inputParam2 } }; + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + nameof(TestController.TaskValueTypeAction), + actionParameters); + + // Act & Assert + await Assert.ThrowsAsync( + () => invoker.InvokeAsync()); + } + + [Theory] + [InlineData(nameof(TestController.ActionMethodWithNullActionResult), typeof(IActionResult))] + [InlineData(nameof(TestController.TestActionMethodWithNullActionResult), typeof(TestActionResult))] + [InlineData(nameof(TestController.AsyncActionMethodWithNullActionResult), typeof(IActionResult))] + [InlineData(nameof(TestController.AsyncActionMethodWithNullTestActionResult), typeof(TestActionResult))] + [ReplaceCulture] + public async Task InvokeAction_WithNullActionResultThrows(string methodName, Type resultType) + { + // Arrange + IActionResult result = null; + + var filter = new Mock(MockBehavior.Strict); + filter.Setup(f => f.OnActionExecuting(It.IsAny())).Verifiable(); + filter + .Setup(f => f.OnActionExecuted(It.IsAny())) + .Callback(c => result = c.Result) + .Verifiable(); + + var invoker = CreateInvoker( + new[] { filter.Object }, + methodName, + new Dictionary()); + + // Act & Assert + await ExceptionAssert.ThrowsAsync( + () => invoker.InvokeAsync(), + "Cannot return null from an action method with a return type of '" + + resultType + + "'."); + } + private TestControllerActionInvoker CreateInvoker( IFilterMetadata filter, bool actionThrows = false, @@ -1967,14 +2381,44 @@ namespace Microsoft.AspNetCore.Mvc.Internal if (actionThrows) { - actionDescriptor.MethodInfo = typeof(ControllerActionInvokerTest).GetMethod("ThrowingActionMethod"); + actionDescriptor.MethodInfo = typeof(ControllerActionInvokerTest).GetMethod(nameof(ControllerActionInvokerTest.ThrowingActionMethod)); } else { - actionDescriptor.MethodInfo = typeof(ControllerActionInvokerTest).GetMethod("ActionMethod"); + actionDescriptor.MethodInfo = typeof(ControllerActionInvokerTest).GetMethod(nameof(ControllerActionInvokerTest.ActionMethod)); } actionDescriptor.ControllerTypeInfo = typeof(ControllerActionInvokerTest).GetTypeInfo(); + return CreateInvoker(filters, actionDescriptor, null, null, maxAllowedErrorsInModelState); + } + + private TestControllerActionInvoker CreateInvoker( + IFilterMetadata[] filters, + string methodName, + IDictionary arguments, + int maxAllowedErrorsInModelState = 200) + { + var actionDescriptor = new ControllerActionDescriptor() + { + FilterDescriptors = new List(), + Parameters = new List(), + }; + + actionDescriptor.MethodInfo = typeof(TestController).GetMethod(methodName); + actionDescriptor.ControllerTypeInfo = typeof(TestController).GetTypeInfo(); + + var argumentBinder = new TestControllerArgumentBinder(arguments); + + return CreateInvoker(filters, actionDescriptor, argumentBinder, _controller, maxAllowedErrorsInModelState); + } + + private TestControllerActionInvoker CreateInvoker( + IFilterMetadata[] filters, + ControllerActionDescriptor actionDescriptor, + IControllerArgumentBinder controllerArgumentBinder, + object controller, + int maxAllowedErrorsInModelState = 200) + { var httpContext = new Mock(MockBehavior.Loose); var http = GetHttpContext(); @@ -2013,6 +2457,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal httpContext .Setup(o => o.RequestServices.GetService(typeof(IOptions))) .Returns(optionsAccessor.Object); + httpContext.SetupGet(c => c.Items) + .Returns(new Dictionary()); + + httpContext + .Setup(o => o.RequestServices.GetService(typeof(ObjectResultExecutor))) + .Returns(new ObjectResultExecutor( + optionsAccessor.Object, + new TestHttpResponseStreamWriterFactory(), + NullLoggerFactory.Instance)); var actionContext = new ActionContext( httpContext: httpContext.Object, @@ -2023,27 +2476,37 @@ namespace Microsoft.AspNetCore.Mvc.Internal filterProvider .Setup(fp => fp.OnProvidersExecuting(It.IsAny())) .Callback(context => + { + foreach (var filterMetadata in filters) { - foreach (var filterMetadata in filters) + context.Results.Add(new FilterItem(new FilterDescriptor(filterMetadata, FilterScope.Action)) { - context.Results.Add(new FilterItem(new FilterDescriptor(filterMetadata, FilterScope.Action)) - { - Filter = filterMetadata, - }); - } - }); + Filter = filterMetadata, + }); + } + }); filterProvider .Setup(fp => fp.OnProvidersExecuted(It.IsAny())) .Verifiable(); - var argumentBinder = new Mock(); - argumentBinder - .Setup(b => b.BindArgumentsAsync( - It.IsAny(), - It.IsAny(), - It.IsAny>())) - .Returns(TaskCache.CompletedTask); + IControllerArgumentBinder argumentBinder = null; + + if (controllerArgumentBinder == null) + { + var mockBinder = new Mock(); + mockBinder + .Setup(b => b.BindArgumentsAsync( + It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Returns(TaskCache.CompletedTask); + argumentBinder = mockBinder.Object; + } + else + { + argumentBinder = controllerArgumentBinder; + } filterProvider .SetupGet(fp => fp.Order) @@ -2051,8 +2514,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal var invoker = new TestControllerActionInvoker( new[] { filterProvider.Object }, - new MockControllerFactory(this), - argumentBinder.Object, + new MockControllerFactory(controller ?? this), + argumentBinder, new NullLoggerFactory().CreateLogger(), new DiagnosticListener("Microsoft.AspNetCore"), actionContext, @@ -2061,6 +2524,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal return invoker; } + [Fact] public async Task Invoke_UsesDefaultValuesIfNotBound() { @@ -2168,6 +2632,133 @@ namespace Microsoft.AspNetCore.Mvc.Internal { return new TestActionResult { Value = value }; } + + public TestActionResult ActionMethodWithTestActionResult(int value) + { + return new TestActionResult { Value = value }; + } + + public async Task AsynActionMethodWithTestActionResult(int value) + { + return await Task.FromResult(new TestActionResult { Value = value }); + } + + public IActionResult ActionMethodWithNullActionResult() + { + return null; + } + + public TestActionResult TestActionMethodWithNullActionResult() + { + return null; + } + + public async Task AsyncActionMethodWithNullActionResult() + { + return await Task.FromResult(null); + } + + public async Task AsyncActionMethodWithNullTestActionResult() + { + return await Task.FromResult(null); + } +#pragma warning disable 1998 + public async Task TaskAction(int i, string s) + { + return; + } +#pragma warning restore 1998 + +#pragma warning disable 1998 + public async Task TaskValueTypeAction(int i, string s) + { + return i; + } +#pragma warning restore 1998 + +#pragma warning disable 1998 + public async Task> TaskOfTaskAction(int i, string s) + { + return TaskValueTypeAction(i, s); + } +#pragma warning restore 1998 + + public Task TaskValueTypeActionWithoutAsync(int i, string s) + { + return TaskValueTypeAction(i, s); + } + +#pragma warning disable 1998 + public async Task TaskActionWithException(int i, string s) + { + throw new NotImplementedException("Not Implemented Exception"); + } +#pragma warning restore 1998 + + public Task TaskActionWithExceptionWithoutAsync(int i, string s) + { + throw new NotImplementedException("Not Implemented Exception"); + } + + public async Task TaskActionThrowAfterAwait(int i, string s) + { + await Task.Delay(500); + throw new ArgumentException("Argument Exception"); + } + + public TaskDerivedType TaskActionWithCustomTaskReturnType(int i, string s) + { + return new TaskDerivedType(); + } + + public TaskOfTDerivedType TaskActionWithCustomTaskOfTReturnType(int i, string s) + { + return new TaskOfTDerivedType(1); + } + + /// + /// Returns a instead of a . + /// + public Task UnwrappedTask(int i, string s) + { + return Task.Factory.StartNew(async () => await Task.Factory.StartNew(() => i)); + } + + public string Echo(string input) + { + return input; + } + + public string EchoWithException(string input) + { + throw new NotImplementedException(); + } + + public string EchoWithDefaultValue([DefaultValue("hello")] string input) + { + return input; + } + + public string EchoWithDefaultValueAndAttribute([DefaultValue("hello")] string input = "world") + { + return input; + } + + public class TaskDerivedType : Task + { + public TaskDerivedType() + : base(() => Console.WriteLine("In The Constructor")) + { + } + } + + public class TaskOfTDerivedType : Task + { + public TaskOfTDerivedType(T input) + : base(() => input) + { + } + } } private sealed class TestActionResult : IActionResult @@ -2273,5 +2864,28 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(_expectedMaxAllowedErrors, context.ModelState.MaxAllowedErrors); } } + + private class TestControllerArgumentBinder : IControllerArgumentBinder + { + private readonly IDictionary _actionParameters; + public TestControllerArgumentBinder(IDictionary actionParameters) + { + _actionParameters = actionParameters; + } + + public Task BindArgumentsAsync( + ControllerContext controllerContext, + object controller, + IDictionary arguments) + { + foreach (var entry in _actionParameters) + { + arguments.Add(entry.Key, entry.Value); + } + + return TaskCache.CompletedTask; + } + } + } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ObjectMethodExecutorTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ObjectMethodExecutorTest.cs index 1115462364..91ac23cc00 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ObjectMethodExecutorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ObjectMethodExecutorTest.cs @@ -69,26 +69,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal new object[] { parameter })); } - [Fact] - public async void ExecuteValueMethodUsingAsyncMethod() - { - var executor = GetExecutorForMethod("ValueMethod"); - var result = await executor.ExecuteAsync( - _targetObject, - new object[] { 10, 20 }); - Assert.Equal(30, (int)result); - } - - [Fact] - public async void ExecuteVoidValueMethodUsingAsyncMethod() - { - var executor = GetExecutorForMethod("VoidValueMethod"); - var result = await executor.ExecuteAsync( - _targetObject, - new object[] { 10 }); - Assert.Same(null, result); - } - [Fact] public async void ExecuteValueMethodAsync() { @@ -99,16 +79,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(30, (int)result); } - [Fact] - public async void ExecuteVoidValueMethodAsync() - { - var executor = GetExecutorForMethod("VoidValueMethodAsync"); - var result = await executor.ExecuteAsync( - _targetObject, - new object[] { 10 }); - Assert.Same(null, result); - } - [Fact] public async void ExecuteValueMethodWithReturnTypeAsync() { @@ -143,33 +113,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal new object[] { parameter })); } - [Fact] - public void ExecuteMethodOfTaskDerivedTypeReturnTypeThrowsException() - { - var expectedException = string.Format( - CultureInfo.CurrentCulture, - "The method 'TaskActionWithCustomTaskReturnType' on type '{0}' returned a Task instance even though it is not an asynchronous method.", - typeof(TestObject)); - - var ex = Assert.Throws( - () => GetExecutorForMethod("TaskActionWithCustomTaskReturnType")); - Assert.Equal(expectedException, ex.Message); - } - - [Fact] - public void ExecuteMethodOfTaskDerivedTypeOfTReturnTypeThrowsException() - { - var expectedException = string.Format( - CultureInfo.CurrentCulture, - "The method 'TaskActionWithCustomTaskOfTReturnType' on type '{0}' returned a Task instance even though it is not an asynchronous method.", - typeof(TestObject)); - - var ex = Assert.Throws( - () => GetExecutorForMethod("TaskActionWithCustomTaskOfTReturnType")); - - Assert.Equal(expectedException, ex.Message); - } - [Theory] [InlineData("EchoWithDefaultAttributes", new object[] { "hello", true, 10 })] [InlineData("EchoWithDefaultValues", new object[] { "hello", true, 20 })] @@ -186,7 +129,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal } Assert.Equal(expectedValues, defaultValues); - } private ObjectMethodExecutor GetExecutorForMethod(string methodName) @@ -197,16 +139,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal } public class TestObject - { + { public string value; public int ValueMethod(int i, int j) { - return i+j; + return i + j; } public void VoidValueMethod(int i) { - + } public TestObject ValueMethodWithReturnType(int i) { @@ -249,16 +191,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal return Task.FromResult(parameter); } - public TaskDerivedType TaskActionWithCustomTaskReturnType(int i, string s) - { - return new TaskDerivedType(); - } - - public TaskOfTDerivedType TaskActionWithCustomTaskOfTReturnType(int i, string s) - { - return new TaskOfTDerivedType(1); - } - public string EchoWithDefaultAttributes( [DefaultValue("hello")] string input1, [DefaultValue(true)] bool input2, @@ -283,29 +215,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal } public string EchoWithNoDefaultAttributesAndValues( - string input1, - int input2, + string input1, + int input2, bool input3, TestObject input4) { return input1; } - - public class TaskDerivedType : Task - { - public TaskDerivedType() - : base(() => Console.WriteLine("In The Constructor")) - { - } - } - - public class TaskOfTDerivedType : Task - { - public TaskOfTDerivedType(T input) - : base(() => input) - { - } - } } } }