From 151cf446074dca22b66c287e1cc1d1d729753def Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 10 Aug 2017 19:06:44 -0700 Subject: [PATCH] Introduce ActionResult --- .../ActionResultOfT.cs | 61 +++++++++ .../Infrastructure/IConvertToActionResult.cs | 17 +++ .../Internal/ControllerActionInvoker.cs | 63 +++++++-- .../ActionResultOfTTest.cs | 68 ++++++++++ .../Internal/ControllerActionInvokerTest.cs | 121 +++++++++++++++++- .../BasicTests.cs | 46 ++++++- .../Controllers/ActionResultOfTController.cs | 31 +++++ 7 files changed, 393 insertions(+), 14 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/ActionResultOfT.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IConvertToActionResult.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/ActionResultOfTTest.cs create mode 100644 test/WebSites/BasicWebSite/Controllers/ActionResultOfTController.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ActionResultOfT.cs b/src/Microsoft.AspNetCore.Mvc.Core/ActionResultOfT.cs new file mode 100644 index 0000000000..0077a236d7 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ActionResultOfT.cs @@ -0,0 +1,61 @@ +// 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 +{ + /// + /// A type that wraps either an instance or an . + /// + /// The type of the result. + public class ActionResult : IConvertToActionResult + { + /// + /// Initializes a new instance of using the specified . + /// + /// The value. + public ActionResult(TValue value) + { + Value = value; + } + + /// + /// Intializes a new instance of using the specified . + /// + /// The . + public ActionResult(ActionResult result) + { + Result = result ?? throw new ArgumentNullException(nameof(result)); + } + + /// + /// Gets the . + /// + public ActionResult Result { get; } + + /// + /// Gets the value. + /// + public TValue Value { get; } + + public static implicit operator ActionResult(TValue value) + { + return new ActionResult(value); + } + + public static implicit operator ActionResult(ActionResult result) + { + return new ActionResult(result); + } + + IActionResult IConvertToActionResult.Convert() + { + return Result ?? new ObjectResult(Value) + { + DeclaredType = typeof(TValue), + }; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IConvertToActionResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IConvertToActionResult.cs new file mode 100644 index 0000000000..8b3d8345f2 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IConvertToActionResult.cs @@ -0,0 +1,17 @@ +// 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. + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// Defines the contract to convert a type to an during action invocation. + /// + public interface IConvertToActionResult + { + /// + /// Converts the current instance to an instance of . + /// + /// The converted . + IActionResult Convert(); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs index d495374695..14fdfc29b5 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; @@ -311,6 +312,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var diagnosticSource = _diagnosticSource; var logger = _logger; + var returnType = executor.MethodReturnType; IActionResult result = null; try @@ -321,7 +323,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal controller); logger.ActionMethodExecuting(controllerContext, orderedArguments); - var returnType = executor.MethodReturnType; if (returnType == typeof(void)) { // Sync method returning void @@ -346,6 +347,30 @@ namespace Microsoft.AspNetCore.Mvc.Internal Resources.FormatActionResult_ActionReturnValueCannotBeNull(typeof(IActionResult))); } } + else if (IsConvertibleToActionResult(executor)) + { + IConvertToActionResult convertToActionResult; + if (executor.IsMethodAsync) + { + // Async method returning awaitable-of-ActionResult (e.g., Task>) + // We have to use ExecuteAsync because we don't know the awaitable's type at compile time. + convertToActionResult = (IConvertToActionResult)await executor.ExecuteAsync(controller, orderedArguments); + } + else + { + // Sync method returning ActionResult + convertToActionResult = (IConvertToActionResult)executor.Execute(controller, orderedArguments); + } + + result = convertToActionResult.Convert(); + + if (result == null) + { + throw new InvalidOperationException( + Resources.FormatActionResult_ActionReturnValueCannotBeNull(typeof(IConvertToActionResult))); + } + + } else if (IsResultIActionResult(executor)) { if (executor.IsMethodAsync) @@ -370,10 +395,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal { // Sync method returning arbitrary object var resultAsObject = executor.Execute(controller, orderedArguments); - result = resultAsObject as IActionResult ?? new ObjectResult(resultAsObject) - { - DeclaredType = returnType, - }; + ConvertToActionResult(resultAsObject); + } else if (executor.AsyncResultType == typeof(void)) { @@ -385,10 +408,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { // Async method returning awaitable-of-nonvoid var resultAsObject = await executor.ExecuteAsync(controller, orderedArguments); - result = resultAsObject as IActionResult ?? new ObjectResult(resultAsObject) - { - DeclaredType = executor.AsyncResultType, - }; + ConvertToActionResult(resultAsObject); } _result = result; @@ -402,6 +422,25 @@ namespace Microsoft.AspNetCore.Mvc.Internal controllerContext, result); } + + void ConvertToActionResult(object resultAsObject) + { + if (resultAsObject is IActionResult actionResult) + { + result = actionResult; + } + else if (resultAsObject is IConvertToActionResult convertToActionResult) + { + result = convertToActionResult.Convert(); + } + else + { + result = new ObjectResult(resultAsObject) + { + DeclaredType = returnType, + }; + } + } } private static bool IsResultIActionResult(ObjectMethodExecutor executor) @@ -410,6 +449,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal 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/test/Microsoft.AspNetCore.Mvc.Core.Test/ActionResultOfTTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ActionResultOfTTest.cs new file mode 100644 index 0000000000..2445e84677 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ActionResultOfTTest.cs @@ -0,0 +1,68 @@ +// 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 Xunit; + +namespace Microsoft.AspNetCore.Mvc +{ + public class ActionResultOfTTest + { + [Fact] + public void Convert_ReturnsResultIfSet() + { + // Arrange + var expected = new OkResult(); + var actionResultOfT = new ActionResult(expected); + var convertToActionResult = (IConvertToActionResult)actionResultOfT; + + // Act + var result = convertToActionResult.Convert(); + + // Assert + Assert.Same(expected, result); + } + + [Fact] + public void Convert_ReturnsObjectResultWrappingValue() + { + // Arrange + var value = new BaseItem(); + var actionResultOfT = new ActionResult(value); + var convertToActionResult = (IConvertToActionResult)actionResultOfT; + + // Act + var result = convertToActionResult.Convert(); + + // Assert + var objectResult = Assert.IsType(result); + Assert.Same(value, objectResult.Value); + Assert.Equal(typeof(BaseItem), objectResult.DeclaredType); + } + + [Fact] + public void Convert_InfersDeclaredTypeFromActionResultTypeParameter() + { + // Arrange + var value = new DeriviedItem(); + var actionResultOfT = new ActionResult(value); + var convertToActionResult = (IConvertToActionResult)actionResultOfT; + + // Act + var result = convertToActionResult.Convert(); + + // Assert + var objectResult = Assert.IsType(result); + Assert.Same(value, objectResult.Value); + Assert.Equal(typeof(BaseItem), objectResult.DeclaredType); + } + + private class BaseItem + { + } + + private class DeriviedItem : BaseItem + { + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs index b2a007797a..96294a767c 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics; @@ -15,8 +14,8 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; @@ -1392,6 +1391,101 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(5, context.Object.Items["Result"]); } + [Fact] + public async Task InvokeAction_ConvertibleToActionResult() + { + // Arrange + var inputParam = 12; + var actionParameters = new Dictionary { { "input", 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 }, nameof(TestController.ActionReturningConvertibleToActionResult), actionParameters); + + // Act + await invoker.InvokeAsync(); + + // Assert + var testActionResult = Assert.IsType(result); + Assert.Equal(inputParam, testActionResult.Value); + } + + [Fact] + public async Task InvokeAction_AsyncAction_ConvertibleToActionResult() + { + // Arrange + var inputParam = 13; + var actionParameters = new Dictionary { { "input", 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 }, nameof(TestController.ActionReturningConvertibleToActionResultAsync), actionParameters); + + // Act + await invoker.InvokeAsync(); + + // Assert + var testActionResult = Assert.IsType(result); + Assert.Equal(inputParam, testActionResult.Value); + } + + [Fact] + public async Task InvokeAction_ConvertibleToActionResult_AsObject() + { + // Arrange + var actionParameters = new Dictionary(); + 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.ActionReturningConvertibleAsObject), actionParameters); + + // Act + await invoker.InvokeAsync(); + + // Assert + Assert.IsType(result); + } + + [Fact] + public async Task InvokeAction_ConvertibleToActionResult_ReturningNull_Throws() + { + // Arrange + var expectedMessage = @"Cannot return null from an action method with a return type of 'Microsoft.AspNetCore.Mvc.Infrastructure.IConvertToActionResult'."; + var actionParameters = new Dictionary(); + 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.ConvertibleToActionResultReturningNull), actionParameters); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => invoker.InvokeAsync()); + Assert.Equal(expectedMessage, exception.Message); + } + #endregion protected override ResourceInvoker CreateInvoker( @@ -1710,6 +1804,22 @@ namespace Microsoft.AspNetCore.Mvc.Internal return input; } + public ConvertibleToActionResult ActionReturningConvertibleToActionResult(int input) + => new ConvertibleToActionResult { Value = input }; + + public Task ActionReturningConvertibleToActionResultAsync(int input) + => Task.FromResult(new ConvertibleToActionResult { Value = input }); + + public object ActionReturningConvertibleAsObject() => new ConvertibleToActionResult(); + + public IConvertToActionResult ConvertibleToActionResultReturningNull() + { + var mock = new Mock(); + mock.Setup(m => m.Convert()).Returns((IActionResult)null); + + return mock.Object; + } + public class TaskDerivedType : Task { public TaskDerivedType() @@ -1745,5 +1855,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal actionDescriptor.ControllerTypeInfo, ParameterDefaultValues.GetParameterDefaultValues(actionDescriptor.MethodInfo)); } + + public class ConvertibleToActionResult : IConvertToActionResult + { + public int Value { get; set; } + + public IActionResult Convert() => new TestActionResult { Value = Value }; + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs index 7ce891e0d3..ed0a8c0164 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs @@ -8,9 +8,8 @@ using System.Net.Http; using System.Net.Http.Headers; using System.Reflection; using System.Threading.Tasks; -using Microsoft.AspNetCore.Mvc.Formatters; +using BasicWebSite.Models; using Newtonsoft.Json; -using Newtonsoft.Json.Serialization; using Xunit; namespace Microsoft.AspNetCore.Mvc.FunctionalTests @@ -402,5 +401,46 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert Assert.Equal(expected, response.Trim()); } + + [Fact] + public async Task ActionMethod_ReturningActionMethodOfT_WithBadRequest() + { + // Arrange + var url = "ActionResultOfT/GetProduct"; + + // Act + var response = await Client.GetAsync(url); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task ActionMethod_ReturningActionMethodOfT() + { + // Arrange + var url = "ActionResultOfT/GetProduct?productId=10"; + + // Act + var response = await Client.GetStringAsync(url); + + // Assert + var result = JsonConvert.DeserializeObject(response); + Assert.Equal(10, result.SampleInt); + } + + [Fact] + public async Task ActionMethod_ReturningSequenceOfObjectsWrappedInActionResultOfT() + { + // Arrange + var url = "ActionResultOfT/GetProductsAsync"; + + // Act + var response = await Client.GetStringAsync(url); + + // Assert + var result = JsonConvert.DeserializeObject(response); + Assert.Equal(2, result.Length); + } } -} \ No newline at end of file +} diff --git a/test/WebSites/BasicWebSite/Controllers/ActionResultOfTController.cs b/test/WebSites/BasicWebSite/Controllers/ActionResultOfTController.cs new file mode 100644 index 0000000000..d6fb9607ab --- /dev/null +++ b/test/WebSites/BasicWebSite/Controllers/ActionResultOfTController.cs @@ -0,0 +1,31 @@ +// 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.Collections.Generic; +using System.Threading.Tasks; +using BasicWebSite.Models; +using Microsoft.AspNetCore.Mvc; + +namespace BasicWebSite.Controllers +{ + public class ActionResultOfTController : Controller + { + [HttpGet] + public ActionResult GetProduct(int? productId) + { + if (productId == null) + { + return BadRequest(); + } + + return new Product { SampleInt = productId.Value, }; + } + + [HttpGet] + public async Task>> GetProductsAsync() + { + await Task.Delay(0); + return new[] { new Product(), new Product() }; + } + } +}