diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResultFactory.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResultFactory.cs deleted file mode 100644 index 1618fa9f5b..0000000000 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResultFactory.cs +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; - -namespace Microsoft.AspNet.Mvc -{ - public class ActionResultFactory : IActionResultFactory - { - private readonly IActionResultHelper _result; - - public ActionResultFactory(IActionResultHelper result) - { - _result = result; - } - - public IActionResult CreateActionResult(Type declaredReturnType, object actionReturnValue, ActionContext actionContext) - { - // optimize common path - var actionResult = actionReturnValue as IActionResult; - - if (actionResult != null) - { - return actionResult; - } - - if (declaredReturnType == null) - { - throw new InvalidOperationException("Declared type must be passed"); - } - - if (typeof(IActionResult).IsAssignableFrom(declaredReturnType) && actionReturnValue == null) - { - throw new InvalidOperationException("Cannot return null from an action method declaring IActionResult or HttpResponseMessage"); - } - - if (declaredReturnType.IsGenericParameter) - { - // This can happen if somebody declares an action method as: - // public T Get() { } - throw new InvalidOperationException("HttpActionDescriptor_NoConverterForGenericParamterTypeExists"); - } - - if (declaredReturnType == typeof(void) || actionReturnValue == null) - { - return new NoContentResult(); - } - - var actionReturnString = actionReturnValue as string; - - if (actionReturnString != null) - { - return new ContentResult - { - ContentType = "text/plain", - Content = actionReturnString, - }; - } - - return _result.Json(actionReturnValue); - } - } -} diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectContentResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectContentResult.cs new file mode 100644 index 0000000000..eb95a3d317 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectContentResult.cs @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; + +namespace Microsoft.AspNet.Mvc +{ + public class ObjectContentResult : ActionResult + { + public object Value { get; set; } + + public ObjectContentResult(object value) + { + Value = value; + } + + public override async Task ExecuteResultAsync(ActionContext context) + { + ActionResult result; + var actionReturnString = Value as string; + if (actionReturnString != null) + { + result = new ContentResult + { + ContentType = "text/plain", + Content = actionReturnString, + }; + } + else + { + result = new JsonResult(Value); + } + + await result.ExecuteResultAsync(context); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/IActionResultFactory.cs b/src/Microsoft.AspNet.Mvc.Core/IActionResultFactory.cs deleted file mode 100644 index 2a5553f8d2..0000000000 --- a/src/Microsoft.AspNet.Mvc.Core/IActionResultFactory.cs +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; - -namespace Microsoft.AspNet.Mvc -{ - public interface IActionResultFactory - { - IActionResult CreateActionResult(Type declaredReturnType, object actionReturnValue, ActionContext actionContext); - } -} diff --git a/src/Microsoft.AspNet.Mvc.Core/Microsoft.AspNet.Mvc.Core.kproj b/src/Microsoft.AspNet.Mvc.Core/Microsoft.AspNet.Mvc.Core.kproj index 4fbac71c8e..eac12b055f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Microsoft.AspNet.Mvc.Core.kproj +++ b/src/Microsoft.AspNet.Mvc.Core/Microsoft.AspNet.Mvc.Core.kproj @@ -31,13 +31,13 @@ - + @@ -126,7 +126,6 @@ - diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index ad77550a67..0ee16ae538 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -1002,6 +1002,22 @@ namespace Microsoft.AspNet.Mvc.Core return string.Format(CultureInfo.CurrentCulture, GetString("UnobtrusiveJavascript_ValidationTypeMustBeUnique"), p0); } + /// + /// Cannot return null from an action method with a return type of '{0}'. + /// + internal static string ActionResult_ActionReturnValueCannotBeNull + { + get { return GetString("ActionResult_ActionReturnValueCannotBeNull"); } + } + + /// + /// Cannot return null from an action method with a return type of '{0}'. + /// + internal static string FormatActionResult_ActionReturnValueCannotBeNull(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("ActionResult_ActionReturnValueCannotBeNull"), p0); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs index 0a0955f4ea..82c9573abe 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs @@ -17,7 +17,6 @@ namespace Microsoft.AspNet.Mvc { private readonly ActionContext _actionContext; private readonly ReflectedActionDescriptor _descriptor; - private readonly IActionResultFactory _actionResultFactory; private readonly IControllerFactory _controllerFactory; private readonly IActionBindingContextProvider _bindingProvider; private readonly INestedProviderManager _filterProvider; @@ -37,14 +36,12 @@ namespace Microsoft.AspNet.Mvc public ReflectedActionInvoker([NotNull] ActionContext actionContext, [NotNull] ReflectedActionDescriptor descriptor, - [NotNull] IActionResultFactory actionResultFactory, [NotNull] IControllerFactory controllerFactory, [NotNull] IActionBindingContextProvider bindingContextProvider, [NotNull] INestedProviderManager filterProvider) { _actionContext = actionContext; _descriptor = descriptor; - _actionResultFactory = actionResultFactory; _controllerFactory = controllerFactory; _bindingProvider = bindingContextProvider; _filterProvider = filterProvider; @@ -99,6 +96,31 @@ namespace Microsoft.AspNet.Mvc } } + // Marking as internal for Unit Testing purposes. + internal static IActionResult CreateActionResult([NotNull] Type declaredReturnType, object actionReturnValue) + { + // optimize common path + var actionResult = actionReturnValue as IActionResult; + + if (actionResult != null) + { + return actionResult; + } + + if (actionReturnValue == null && typeof(IActionResult).IsAssignableFrom(declaredReturnType)) + { + throw new InvalidOperationException( + Resources.FormatActionResult_ActionReturnValueCannotBeNull(declaredReturnType)); + } + + if (declaredReturnType == typeof(void)) + { + return new NoContentResult(); + } + + return new ObjectContentResult(actionReturnValue); + } + private IFilter[] GetFilters() { var filterProviderContext = new FilterProviderContext( @@ -372,10 +394,9 @@ namespace Microsoft.AspNet.Mvc _actionExecutingContext.ActionArguments); var underlyingReturnType = TypeHelper.GetTaskInnerTypeOrNull(actionMethodInfo.ReturnType) ?? actionMethodInfo.ReturnType; - var actionResult = _actionResultFactory.CreateActionResult( + var actionResult = CreateActionResult( underlyingReturnType, - actionReturnValue, - _actionContext); + actionReturnValue); return actionResult; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvokerProvider.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvokerProvider.cs index 72e1705894..76854d6477 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvokerProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvokerProvider.cs @@ -8,19 +8,16 @@ namespace Microsoft.AspNet.Mvc { public class ReflectedActionInvokerProvider : IActionInvokerProvider { - private readonly IActionResultFactory _actionResultFactory; private readonly IServiceProvider _serviceProvider; private readonly IControllerFactory _controllerFactory; private readonly IActionBindingContextProvider _bindingProvider; private readonly INestedProviderManager _filterProvider; - public ReflectedActionInvokerProvider(IActionResultFactory actionResultFactory, - IControllerFactory controllerFactory, + public ReflectedActionInvokerProvider(IControllerFactory controllerFactory, IActionBindingContextProvider bindingProvider, INestedProviderManager filterProvider, IServiceProvider serviceProvider) { - _actionResultFactory = actionResultFactory; _controllerFactory = controllerFactory; _bindingProvider = bindingProvider; _filterProvider = filterProvider; @@ -41,7 +38,6 @@ namespace Microsoft.AspNet.Mvc context.Result = new ReflectedActionInvoker( context.ActionContext, actionDescriptor, - _actionResultFactory, _controllerFactory, _bindingProvider, _filterProvider); diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index a0a50aef3b..8997a1daf1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -303,4 +303,7 @@ Validation type names in unobtrusive client validation rules must be unique. The following validation type was seen more than once: {0} + + Cannot return null from an action method with a return type of '{0}'. + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc/MvcServices.cs b/src/Microsoft.AspNet.Mvc/MvcServices.cs index 0acb28e310..27784799cb 100644 --- a/src/Microsoft.AspNet.Mvc/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc/MvcServices.cs @@ -30,7 +30,6 @@ namespace Microsoft.AspNet.Mvc yield return describe.Transient(); yield return describe.Transient(); yield return describe.Transient(); - yield return describe.Transient(); yield return describe.Transient(); yield return describe.Transient(); yield return describe.Transient(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectContentResultTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectContentResultTests.cs new file mode 100644 index 0000000000..e5fb96dcbf --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectContentResultTests.cs @@ -0,0 +1,92 @@ +// Copyright (c) Microsoft Open Technologies, Inc. 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.IO; +using System.Threading.Tasks; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Mvc.Internal; +using Microsoft.AspNet.Routing; +using Moq; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Core.Test.ActionResults +{ + public class ObjectContentResultTests + { + [Fact] + public void ObjectContentResult_Create_CallsContentResult_InitializesValue() + { + // Arrange + var input = "testInput"; + var actionContext = CreateMockActionContext(); + + // Act + var result = new ObjectContentResult(input); + + // Assert + Assert.Equal(input, result.Value); + } + + [Fact] + public async Task ObjectContentResult_Execute_CallsContentResult_SetsContent() + { + // Arrange + var expectedContentType = "text/plain"; + var input = "testInput"; + + var httpResponse = new Mock(); + httpResponse.SetupSet(r => r.ContentType = expectedContentType).Verifiable(); + httpResponse.Object.Body = new MemoryStream(); + + var actionContext = CreateMockActionContext(httpResponse.Object); + + // Act + var result = new ObjectContentResult(input); + await result.ExecuteResultAsync(actionContext); + + // Assert + httpResponse.VerifySet(r => r.ContentType = expectedContentType); + // The following verifies the correct Content was written to Body + httpResponse.Verify(o => o.WriteAsync(input), Times.Exactly(1)); + } + + [Fact] + public async Task ObjectContentResult_Execute_CallsJsonResult_SetsContent() + { + // Arrange + var expectedContentType = "application/json"; + var nonStringValue = new { x1 = 10, y1 = "Hello" }; + var httpResponse = Mock.Of(); + httpResponse.Body = new MemoryStream(); + var actionContext = CreateMockActionContext(httpResponse); + + var tempStream = new MemoryStream(); + using (var writer = new StreamWriter(tempStream, UTF8EncodingWithoutBOM.Encoding, 1024, leaveOpen: true)) + { + var formatter = new JsonOutputFormatter(JsonOutputFormatter.CreateDefaultSettings(), false); + formatter.WriteObject(writer, nonStringValue); + } + + // Act + var result = new ObjectContentResult(nonStringValue); + await result.ExecuteResultAsync(actionContext); + + // Assert + Assert.Equal(expectedContentType, httpResponse.ContentType); + Assert.Equal(tempStream.ToArray(), ((MemoryStream)actionContext.HttpContext.Response.Body).ToArray()); + } + + private static ActionContext CreateMockActionContext(HttpResponse response = null) + { + var httpContext = new Mock(); + if (response != null) + { + httpContext.Setup(o => o.Response).Returns(response); + } + + return new ActionContext(httpContext.Object, Mock.Of(), new Dictionary(), + new ActionDescriptor()); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj index f150153062..8480330b3c 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj @@ -23,6 +23,7 @@ + diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs index 15a5d77741..ec3f81742e 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs @@ -21,6 +21,27 @@ namespace Microsoft.AspNet.Mvc private readonly JsonResult _result = new JsonResult(new { message = "Hello, world!" }); + private struct SampleStruct + { + public int x; + } + + public static IEnumerable CreateActionResultData + { + get + { + yield return new object[] { new { x1 = 10, y1 = "Hello" } }; + yield return new object[] { 5 }; + yield return new object[] { "sample input" }; + + SampleStruct test; + test.x = 10; + yield return new object[] { test }; + + yield return new object[] { new Task(() => Console.WriteLine("Test task")) }; + } + } + [Fact] public async Task InvokeAction_DoesNotInvokeExceptionFilter_WhenActionDoesNotThrow() { @@ -1190,6 +1211,57 @@ namespace Microsoft.AspNet.Mvc resultFilter2.Verify(f => f.OnResultExecuting(It.IsAny()), Times.Once()); } + [Fact] + public void CreateActionResult_ReturnsSameActionResult() + { + // Arrange + var mockActionResult = new Mock(); + + // Assert + var result = ReflectedActionInvoker.CreateActionResult( + mockActionResult.Object.GetType(), mockActionResult.Object); + + // Act + Assert.Same(mockActionResult.Object, result); + } + + [Fact] + [ReplaceCulture] + public void CreateActionResult_NullActionResultReturnValueThrows() + { + // Arrange, Act & Assert + ExceptionAssert.Throws( + () => ReflectedActionInvoker.CreateActionResult(typeof(IActionResult), null), + "Cannot return null from an action method with a return type of '" + + typeof(IActionResult) + + "'."); + } + + [Theory] + [InlineData(typeof(void), typeof(NoContentResult))] + [InlineData(typeof(string), typeof(ObjectContentResult))] + public void CreateActionResult_Types_ReturnsAppropriateResults(Type type, Type returnType) + { + // Arrange & Act + var result = ReflectedActionInvoker.CreateActionResult(type, null).GetType(); + + // Assert + Assert.Equal(returnType, result); + } + + [Theory] + [MemberData("CreateActionResultData")] + public void CreateActionResult_ReturnsObjectContentResult(object input) + { + // Arrange & Act + var actualResult = ReflectedActionInvoker.CreateActionResult(input.GetType(), input) + as ObjectContentResult; + + // Assert + Assert.NotNull(actualResult); + Assert.Equal(input, actualResult.Value); + } + private ReflectedActionInvoker CreateInvoker(IFilter filter, bool actionThrows = false) { return CreateInvoker(new[] { filter }, actionThrows); @@ -1223,11 +1295,6 @@ namespace Microsoft.AspNet.Mvc routeValues: null, actionDescriptor: actionDescriptor); - var actionResultFactory = new Mock(MockBehavior.Strict); - actionResultFactory - .Setup(arf => arf.CreateActionResult(It.IsAny(), It.IsAny(), It.IsAny())) - .Returns((t, o, ac) => (IActionResult)o); - var controllerFactory = new Mock(MockBehavior.Strict); controllerFactory.Setup(c => c.CreateController(It.IsAny())).Returns(this); @@ -1245,7 +1312,6 @@ namespace Microsoft.AspNet.Mvc var invoker = new ReflectedActionInvoker( actionContext, actionDescriptor, - actionResultFactory.Object, controllerFactory.Object, actionBindingContextProvider.Object, filterProvider.Object);