From c93c168df307230e8f0f0cfc520d4980bbcc16fd Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 21 Mar 2018 22:22:58 -0700 Subject: [PATCH] Add mapping service for action results This allows the use of custom 'envelope' types like ActionResult<> with a corresponding API Explorer implementation. Basically this PR services to decouple a bunch of infrastructure from ActionResult<>. --- .../DefaultApiDescriptionProvider.cs | 48 +++++++---- .../MvcCoreServiceCollectionExtensions.cs | 1 + .../Infrastructure/IActionResultTypeMapper.cs | 48 +++++++++++ .../Internal/ActionMethodExecutor.cs | 82 ++++++++++++------- .../Internal/ActionResultTypeMapper.cs | 45 ++++++++++ .../Internal/ControllerActionInvoker.cs | 17 +--- .../ControllerActionInvokerProvider.cs | 7 +- .../Internal/ResourceInvoker.cs | 4 + .../Internal/PageActionInvoker.cs | 3 + .../Internal/PageActionInvokerProvider.cs | 6 +- .../DefaultApiDescriptionProviderTest.cs | 3 +- .../Internal/ActionMethodExecutorTest.cs | 45 ++++++---- .../Internal/ActionResultTypeMapperTest.cs | 75 +++++++++++++++++ .../Internal/ControllerActionInvokerTest.cs | 2 + .../Internal/MiddlewareFilterTest.cs | 4 + .../Internal/PageActionInvokerProviderTest.cs | 4 +- .../Internal/PageActionInvokerTest.cs | 1 + 17 files changed, 317 insertions(+), 78 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionResultTypeMapper.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionResultTypeMapper.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionResultTypeMapperTest.cs diff --git a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs index 56f340b9d0..4dda71ab01 100644 --- a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing; @@ -25,6 +26,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer public class DefaultApiDescriptionProvider : IApiDescriptionProvider { private readonly MvcOptions _mvcOptions; + private readonly IActionResultTypeMapper _mapper; private readonly IInlineConstraintResolver _constraintResolver; private readonly IModelMetadataProvider _modelMetadataProvider; @@ -35,6 +37,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer /// The used for resolving inline /// constraints. /// The . + [Obsolete("This constructor is obsolete and will be removed in a future release.")] public DefaultApiDescriptionProvider( IOptions optionsAccessor, IInlineConstraintResolver constraintResolver, @@ -45,6 +48,26 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer _modelMetadataProvider = modelMetadataProvider; } + /// + /// Creates a new instance of . + /// + /// The accessor for . + /// The used for resolving inline + /// constraints. + /// The . + /// The . + public DefaultApiDescriptionProvider( + IOptions optionsAccessor, + IInlineConstraintResolver constraintResolver, + IModelMetadataProvider modelMetadataProvider, + IActionResultTypeMapper mapper) + { + _mvcOptions = optionsAccessor.Value; + _constraintResolver = constraintResolver; + _modelMetadataProvider = modelMetadataProvider; + _mapper = mapper; + } + /// public int Order => -1000; @@ -481,12 +504,14 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer { return typeof(void); } - + // Unwrap the type if it's a Task. The Task (non-generic) case was already handled. - var unwrappedType = UnwrapGenericType(declaredReturnType, typeof(Task<>)); - - // Unwrap the type if it's ActionResult or Task>. - unwrappedType = UnwrapGenericType(unwrappedType, typeof(ActionResult<>)); + Type unwrappedType = declaredReturnType; + if (declaredReturnType.IsGenericType && + declaredReturnType.GetGenericTypeDefinition() == typeof(Task<>)) + { + unwrappedType = declaredReturnType.GetGenericArguments()[0]; + } // If the method is declared to return IActionResult or a derived class, that information // isn't valuable to the formatter. @@ -494,16 +519,11 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer { return null; } - else - { - return unwrappedType; - } - Type UnwrapGenericType(Type type, Type queryType) - { - var genericType = ClosedGenericMatcher.ExtractGenericInterface(type, queryType); - return genericType?.GenericTypeArguments[0] ?? type; - } + // If we get here, the type should be a user-defined data type or an envelope type + // like ActionResult. The mapper service will unwrap envelopes. + unwrappedType = _mapper.GetResultDataType(unwrappedType); + return unwrappedType; } private Type GetRuntimeReturnType(Type declaredReturnType) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index b0a9a9f47a..c0bfe2039e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -202,6 +202,7 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton(); services.TryAddEnumerable( ServiceDescriptor.Singleton()); + services.TryAddSingleton(); // // Request body limit filters diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionResultTypeMapper.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionResultTypeMapper.cs new file mode 100644 index 0000000000..1f45bfbb5a --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionResultTypeMapper.cs @@ -0,0 +1,48 @@ +// 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.Threading.Tasks; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// Provides a mapping from the return value of an action to an + /// for request processing. + /// + /// + /// The default implementation of this service handles the conversion of + /// to an during request + /// processing as well as the mapping of to TValue + /// during API Explorer processing. + /// + public interface IActionResultTypeMapper + { + /// + /// Gets the result data type that corresponds to . This + /// method will not be called for actions that return void or an + /// type. + /// + /// The declared return type of an action. + /// A that represents the response data. + /// + /// Prior to calling this method, the infrastructure will unwrap or + /// other task-like types. + /// + Type GetResultDataType(Type returnType); + + /// + /// Converts the result of an action to an for response processing. + /// This method will be not be called when a method returns void or an + /// value. + /// + /// The action return value. May be null. + /// The declared return type. + /// An for response processing. + /// + /// Prior to calling this method, the infrastructure will unwrap or + /// other task-like types. + /// + IActionResult Convert(object value, Type returnType); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionMethodExecutor.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionMethodExecutor.cs index d0e3991d3c..92b6335887 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionMethodExecutor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionMethodExecutor.cs @@ -27,7 +27,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal new AwaitableObjectResultExecutor(), }; - public abstract ValueTask Execute(ObjectMethodExecutor executor, object controller, object[] arguments); + public abstract ValueTask Execute( + IActionResultTypeMapper mapper, + ObjectMethodExecutor executor, + object controller, + object[] arguments); protected abstract bool CanExecute(ObjectMethodExecutor executor); @@ -48,7 +52,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal // void LogMessage(..) private class VoidResultExecutor : ActionMethodExecutor { - public override ValueTask Execute(ObjectMethodExecutor executor, object controller, object[] arguments) + public override ValueTask Execute( + IActionResultTypeMapper mapper, + ObjectMethodExecutor executor, + object controller, + object[] arguments) { executor.Execute(controller, arguments); return new ValueTask(new EmptyResult()); @@ -62,7 +70,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal // CreatedAtResult Put(..) private class SyncActionResultExecutor : ActionMethodExecutor { - public override ValueTask Execute(ObjectMethodExecutor executor, object controller, object[] arguments) + public override ValueTask Execute( + IActionResultTypeMapper mapper, + ObjectMethodExecutor executor, + object controller, + object[] arguments) { var actionResult = (IActionResult)executor.Execute(controller, arguments); EnsureActionResultNotNull(executor, actionResult); @@ -78,11 +90,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal // object Index(..) private class SyncObjectResultExecutor : ActionMethodExecutor { - public override ValueTask Execute(ObjectMethodExecutor executor, object controller, object[] arguments) + public override ValueTask Execute( + IActionResultTypeMapper mapper, + ObjectMethodExecutor executor, + object controller, + object[] arguments) { // Sync method returning arbitrary object var returnValue = executor.Execute(controller, arguments); - var actionResult = ConvertToActionResult(returnValue, executor.MethodReturnType); + var actionResult = ConvertToActionResult(mapper, returnValue, executor.MethodReturnType); return new ValueTask(actionResult); } @@ -93,7 +109,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Task SaveState(..) private class TaskResultExecutor : ActionMethodExecutor { - public override async ValueTask Execute(ObjectMethodExecutor executor, object controller, object[] arguments) + public override async ValueTask Execute( + IActionResultTypeMapper mapper, + ObjectMethodExecutor executor, + object controller, + object[] arguments) { await (Task)executor.Execute(controller, arguments); return new EmptyResult(); @@ -106,7 +126,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Custom task-like type with no return value. private class AwaitableResultExecutor : ActionMethodExecutor { - public override async ValueTask Execute(ObjectMethodExecutor executor, object controller, object[] arguments) + public override async ValueTask Execute( + IActionResultTypeMapper mapper, + ObjectMethodExecutor executor, + object controller, + object[] arguments) { await executor.ExecuteAsync(controller, arguments); return new EmptyResult(); @@ -122,7 +146,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Task Post(..) private class TaskOfIActionResultExecutor : ActionMethodExecutor { - public override async ValueTask Execute(ObjectMethodExecutor executor, object controller, object[] arguments) + public override async ValueTask Execute( + IActionResultTypeMapper mapper, + ObjectMethodExecutor executor, + object controller, + object[] arguments) { // Async method returning Task // Avoid extra allocations by calling Execute rather than ExecuteAsync and casting to Task. @@ -141,7 +169,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal // ValueTask GetViewsAsync(..) private class TaskOfActionResultExecutor : ActionMethodExecutor { - public override async ValueTask Execute(ObjectMethodExecutor executor, object controller, object[] arguments) + public override async ValueTask Execute( + IActionResultTypeMapper mapper, + ObjectMethodExecutor executor, + object controller, + object[] arguments) { // Async method returning awaitable-of-IActionResult (e.g., Task) // We have to use ExecuteAsync because we don't know the awaitable's type at compile time. @@ -161,11 +193,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Task GetCustomerAsync(..) private class AwaitableObjectResultExecutor : ActionMethodExecutor { - public override async ValueTask Execute(ObjectMethodExecutor executor, object controller, object[] arguments) + public override async ValueTask Execute( + IActionResultTypeMapper mapper, + ObjectMethodExecutor executor, + object controller, + object[] arguments) { // Async method returning awaitable-of-nonvoid var returnValue = await executor.ExecuteAsync(controller, arguments); - var actionResult = ConvertToActionResult(returnValue, executor.MethodReturnType); + var actionResult = ConvertToActionResult(mapper, returnValue, executor.MethodReturnType); return actionResult; } @@ -176,30 +212,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal { if (actionResult == null) { - throw new InvalidOperationException( - Resources.FormatActionResult_ActionReturnValueCannotBeNull(executor.AsyncResultType ?? executor.MethodReturnType)); + var type = executor.AsyncResultType ?? executor.MethodReturnType; + throw new InvalidOperationException(Resources.FormatActionResult_ActionReturnValueCannotBeNull(type)); } } - private static IActionResult ConvertToActionResult(object returnValue, Type declaredType) + private IActionResult ConvertToActionResult(IActionResultTypeMapper mapper, object returnValue, Type declaredType) { - IActionResult result; - switch (returnValue) - { - case IActionResult actionResult: - result = actionResult; - break; - case IConvertToActionResult convertToActionResult: - result = convertToActionResult.Convert(); - break; - default: - result = new ObjectResult(returnValue) - { - DeclaredType = declaredType, - }; - break; - } - + var result = (returnValue as IActionResult) ?? mapper.Convert(returnValue, declaredType); if (result == null) { throw new InvalidOperationException(Resources.FormatActionResult_ActionReturnValueCannotBeNull(declaredType)); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionResultTypeMapper.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionResultTypeMapper.cs new file mode 100644 index 0000000000..82f1d02998 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionResultTypeMapper.cs @@ -0,0 +1,45 @@ +// 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 Microsoft.AspNetCore.Mvc.Infrastructure; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class ActionResultTypeMapper : IActionResultTypeMapper + { + public Type GetResultDataType(Type returnType) + { + if (returnType == null) + { + throw new ArgumentNullException(nameof(returnType)); + } + + if (returnType.IsGenericType && + returnType.GetGenericTypeDefinition() == typeof(ActionResult<>)) + { + return returnType.GetGenericArguments()[0]; + } + + return returnType; + } + + public IActionResult Convert(object value, Type returnType) + { + if (returnType == null) + { + throw new ArgumentNullException(nameof(returnType)); + } + + if (value is IConvertToActionResult converter) + { + return converter.Convert(); + } + + return new ObjectResult(value) + { + DeclaredType = returnType, + }; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs index b068cdd6da..f020c86bf3 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs @@ -28,10 +28,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal internal ControllerActionInvoker( ILogger logger, DiagnosticSource diagnosticSource, + IActionResultTypeMapper mapper, ControllerContext controllerContext, ControllerActionInvokerCacheEntry cacheEntry, IFilterMetadata[] filters) - : base(diagnosticSource, logger, controllerContext, filters, controllerContext.ValueProviderFactories) + : base(diagnosticSource, logger, mapper, controllerContext, filters, controllerContext.ValueProviderFactories) { if (cacheEntry == null) { @@ -347,7 +348,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal controller); logger.ActionMethodExecuting(controllerContext, orderedArguments); var stopwatch = ValueStopwatch.StartNew(); - var actionResultValueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, orderedArguments); + var actionResultValueTask = actionMethodExecutor.Execute(_mapper, objectMethodExecutor, controller, orderedArguments); if (actionResultValueTask.IsCompletedSuccessfully) { result = actionResultValueTask.Result; @@ -370,18 +371,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } - private static bool IsResultIActionResult(ObjectMethodExecutor executor) - { - var resultType = executor.AsyncResultType ?? executor.MethodReturnType; - return typeof(IActionResult).IsAssignableFrom(resultType); - } - - private bool IsConvertibleToActionResult(ObjectMethodExecutor executor) - { - var resultType = executor.AsyncResultType ?? executor.MethodReturnType; - return typeof(IConvertToActionResult).IsAssignableFrom(resultType); - } - /// for details on what the /// variables in this method represent. protected override async Task InvokeInnerFilterAsync() diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerProvider.cs index bbd9c34854..b9b135ff52 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerProvider.cs @@ -7,6 +7,7 @@ using System.Diagnostics; using System.Linq; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -20,18 +21,21 @@ namespace Microsoft.AspNetCore.Mvc.Internal private readonly int _maxModelValidationErrors; private readonly ILogger _logger; private readonly DiagnosticSource _diagnosticSource; + private readonly IActionResultTypeMapper _mapper; public ControllerActionInvokerProvider( ControllerActionInvokerCache controllerActionInvokerCache, IOptions optionsAccessor, ILoggerFactory loggerFactory, - DiagnosticSource diagnosticSource) + DiagnosticSource diagnosticSource, + IActionResultTypeMapper mapper) { _controllerActionInvokerCache = controllerActionInvokerCache; _valueProviderFactories = optionsAccessor.Value.ValueProviderFactories.ToArray(); _maxModelValidationErrors = optionsAccessor.Value.MaxModelValidationErrors; _logger = loggerFactory.CreateLogger(); _diagnosticSource = diagnosticSource; + _mapper = mapper; } public int Order => -1000; @@ -56,6 +60,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var invoker = new ControllerActionInvoker( _logger, _diagnosticSource, + _mapper, controllerContext, cacheResult.cacheEntry, cacheResult.filters); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs index 8cf4a65ea2..8da6e6012e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs @@ -8,6 +8,7 @@ using System.Runtime.ExceptionServices; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; @@ -18,6 +19,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { protected readonly DiagnosticSource _diagnosticSource; protected readonly ILogger _logger; + protected readonly IActionResultTypeMapper _mapper; protected readonly ActionContext _actionContext; protected readonly IFilterMetadata[] _filters; protected readonly IList _valueProviderFactories; @@ -38,12 +40,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal public ResourceInvoker( DiagnosticSource diagnosticSource, ILogger logger, + IActionResultTypeMapper mapper, ActionContext actionContext, IFilterMetadata[] filters, IList valueProviderFactories) { _diagnosticSource = diagnosticSource ?? throw new ArgumentNullException(nameof(diagnosticSource)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _mapper = mapper ?? throw new ArgumentNullException(nameof(mapper)); _actionContext = actionContext ?? throw new ArgumentNullException(nameof(actionContext)); _filters = filters ?? throw new ArgumentNullException(nameof(filters)); diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs index a6a0de16fe..de4536d275 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs @@ -9,6 +9,7 @@ using System.Runtime.ExceptionServices; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; @@ -43,6 +44,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal IPageHandlerMethodSelector handlerMethodSelector, DiagnosticSource diagnosticSource, ILogger logger, + IActionResultTypeMapper mapper, PageContext pageContext, IFilterMetadata[] filterMetadata, PageActionInvokerCacheEntry cacheEntry, @@ -52,6 +54,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal : base( diagnosticSource, logger, + mapper, pageContext, filterMetadata, pageContext.ValueProviderFactories) diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs index 9a296ce7c4..007a2f9b14 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs @@ -42,6 +42,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal private readonly RazorProjectFileSystem _razorFileSystem; private readonly DiagnosticSource _diagnosticSource; private readonly ILogger _logger; + private readonly IActionResultTypeMapper _mapper; private volatile InnerCache _currentCache; public PageActionInvokerProvider( @@ -60,7 +61,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal IPageHandlerMethodSelector selector, RazorProjectFileSystem razorFileSystem, DiagnosticSource diagnosticSource, - ILoggerFactory loggerFactory) + ILoggerFactory loggerFactory, + IActionResultTypeMapper mapper) { _loader = loader; _pageFactoryProvider = pageFactoryProvider; @@ -79,6 +81,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal _razorFileSystem = razorFileSystem; _diagnosticSource = diagnosticSource; _logger = loggerFactory.CreateLogger(); + _mapper = mapper; } public int Order { get; } = -1000; @@ -158,6 +161,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal _selector, _diagnosticSource, _logger, + _mapper, pageContext, filters, cacheEntry, diff --git a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs index faecd9554c..d3f0110711 100644 --- a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs @@ -1449,7 +1449,8 @@ namespace Microsoft.AspNetCore.Mvc.Description var provider = new DefaultApiDescriptionProvider( optionsAccessor, constraintResolver.Object, - modelMetadataProvider); + modelMetadataProvider, + new ActionResultTypeMapper()); provider.OnProvidersExecuting(context); provider.OnProvidersExecuted(context); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionMethodExecutorTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionMethodExecutorTest.cs index e29e74dc10..b79a948e7c 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionMethodExecutorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionMethodExecutorTest.cs @@ -17,12 +17,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesVoidActions() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.VoidAction)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert Assert.True(controller.Executed); @@ -33,12 +34,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsReturningIActionResult() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnIActionResult)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert Assert.True(valueTask.IsCompleted); @@ -49,12 +51,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsReturningSubTypeOfActionResult() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnsIActionResultSubType)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert Assert.IsType(valueTask.Result); @@ -64,12 +67,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsReturningActionResultOfT() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnsActionResultOfT)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert var result = Assert.IsType(valueTask.Result); @@ -81,12 +85,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsReturningModelAsModel() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnsModelAsModel)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert var result = Assert.IsType(valueTask.Result); @@ -98,12 +103,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsReturningModelAsObject() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnModelAsObject)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert var result = Assert.IsType(valueTask.Result); @@ -115,12 +121,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsReturningActionResultAsObject() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnsIActionResultSubType)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert Assert.IsType(valueTask.Result); @@ -130,12 +137,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsReturnTask() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnsTask)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert Assert.True(controller.Executed); @@ -146,12 +154,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutorExecutesActionsAsynchronouslyReturningIActionResult() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnIActionResultAsync)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert Assert.IsType(valueTask.Result); @@ -161,12 +170,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public async Task ActionMethodExecutor_ExecutesActionsAsynchronouslyReturningActionResultSubType() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnIActionResultAsync)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert await valueTask; @@ -177,12 +187,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsAsynchronouslyReturningModel() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnsModelAsModelAsync)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert var result = Assert.IsType(valueTask.Result); @@ -194,12 +205,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsAsynchronouslyReturningModelAsObject() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnsModelAsObjectAsync)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert var result = Assert.IsType(valueTask.Result); @@ -211,12 +223,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsAsynchronouslyReturningIActionResultAsObject() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnIActionResultAsObjectAsync)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert Assert.IsType(valueTask.Result); @@ -226,12 +239,13 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ExecutesActionsAsynchronouslyReturningActionResultOfT() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnActionResultOFTAsync)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act - var valueTask = actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty()); + var valueTask = actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty()); // Assert var result = Assert.IsType(valueTask.Result); @@ -243,13 +257,14 @@ namespace Microsoft.AspNetCore.Mvc.Core.Internal public void ActionMethodExecutor_ThrowsIfIConvertFromIActionResult_ReturnsNull() { // Arrange + var mapper = new ActionResultTypeMapper(); var controller = new TestController(); var objectMethodExecutor = GetExecutor(nameof(TestController.ReturnsCustomConvertibleFromIActionResult)); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); // Act & Assert var ex = Assert.Throws( - () => actionMethodExecutor.Execute(objectMethodExecutor, controller, Array.Empty())); + () => actionMethodExecutor.Execute(mapper, objectMethodExecutor, controller, Array.Empty())); Assert.Equal($"Cannot return null from an action method with a return type of '{typeof(CustomConvertibleFromAction)}'.", ex.Message); } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionResultTypeMapperTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionResultTypeMapperTest.cs new file mode 100644 index 0000000000..3b4d3a0f4d --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionResultTypeMapperTest.cs @@ -0,0 +1,75 @@ +// 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 Microsoft.AspNetCore.Mvc.Infrastructure; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class ActionResultTypeMapperTest + { + [Fact] + public void Convert_WithIConvertToActionResult_DelegatesToInterface() + { + // Arrange + var mapper = new ActionResultTypeMapper(); + + var expected = new EmptyResult(); + var returnValue = Mock.Of(r => r.Convert() == expected); + + // Act + var result = mapper.Convert(returnValue, typeof(string)); + + // Assert + Assert.Same(expected, result); + } + + [Fact] + public void Convert_WithRegularType_CreatesObjectResult() + { + // Arrange + var mapper = new ActionResultTypeMapper(); + + var returnValue = "hello"; + + // Act + var result = mapper.Convert(returnValue, typeof(string)); + + // Assert + var objectResult = Assert.IsType(result); + Assert.Same(returnValue, objectResult.Value); + Assert.Equal(typeof(string), objectResult.DeclaredType); + } + + [Fact] + public void GetResultDataType_WithActionResultOfT_UnwrapsType() + { + // Arrange + var mapper = new ActionResultTypeMapper(); + + var returnType = typeof(ActionResult); + + // Act + var result = mapper.GetResultDataType(returnType); + + // Assert + Assert.Equal(typeof(string), result); + } + + [Fact] + public void GetResultDataType_WithRegularType_ReturnsType() + { + // Arrange + var mapper = new ActionResultTypeMapper(); + + var returnType = typeof(string); + + // Act + var result = mapper.GetResultDataType(returnType); + + // Assert + Assert.Equal(typeof(string), result); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs index 0d93254154..9d5c551ce1 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs @@ -1345,6 +1345,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var invoker = new ControllerActionInvoker( new NullLoggerFactory().CreateLogger(), new DiagnosticListener("Microsoft.AspNetCore"), + new ActionResultTypeMapper(), controllerContext, cacheEntry, new IFilterMetadata[0]); @@ -1624,6 +1625,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var invoker = new ControllerActionInvoker( logger, diagnosticSource, + new ActionResultTypeMapper(), controllerContext, cacheEntry, filters); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs index 9c6a474f3e..e9c770e27a 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs @@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.AspNetCore.Routing; @@ -285,6 +286,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new MockControllerFactory(controller ?? this), new NullLoggerFactory().CreateLogger(), diagnosticSource, + new ActionResultTypeMapper(), actionContext, new List(), maxAllowedErrorsInModelState: 200); @@ -389,12 +391,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal MockControllerFactory controllerFactory, ILogger logger, DiagnosticSource diagnosticSource, + IActionResultTypeMapper mapper, ActionContext actionContext, IReadOnlyList valueProviderFactories, int maxAllowedErrorsInModelState) : base( logger, diagnosticSource, + mapper, CreatControllerContext(actionContext, valueProviderFactories, maxAllowedErrorsInModelState), CreateCacheEntry((ControllerActionDescriptor)actionContext.ActionDescriptor, controllerFactory), filters) diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs index f41c4db6e5..4d5c14dbd1 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.AspNetCore.Mvc.Razor; @@ -515,7 +516,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal Mock.Of(), fileSystem, new DiagnosticListener("Microsoft.AspNetCore"), - NullLoggerFactory.Instance); + NullLoggerFactory.Instance, + new ActionResultTypeMapper()); } private IActionDescriptorCollectionProvider CreateActionDescriptorCollection(PageActionDescriptor descriptor) diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs index e1e7abf2ff..126f03bd4d 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs @@ -1217,6 +1217,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal selector.Object, diagnosticListener ?? new DiagnosticListener("Microsoft.AspNetCore"), logger ?? NullLogger.Instance, + new ActionResultTypeMapper(), pageContext, filters ?? Array.Empty(), cacheEntry,