diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/HttpNoContentOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/HttpNoContentOutputFormatter.cs index f3b7f693a6..8e3c92e767 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/HttpNoContentOutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/HttpNoContentOutputFormatter.cs @@ -13,11 +13,23 @@ namespace Microsoft.AspNet.Mvc /// public class HttpNoContentOutputFormatter : IOutputFormatter { + /// + /// Indicates whether to select this formatter if the returned value from the action + /// is null. + /// + public bool TreatNullValueAsNoContent { get; set; } = true; + public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType) { // ignore the contentType and just look at the content. // This formatter will be selected if the content is null. - return context.Object == null; + // We check for Task as a user can directly create an ObjectContentResult with the unwrapped type. + if(context.DeclaredType == typeof(void) || context.DeclaredType == typeof(Task)) + { + return true; + } + + return TreatNullValueAsNoContent && context.Object == null; } public IReadOnlyList GetSupportedContentTypes( diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs index 178010cc95..491c938ec2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs @@ -80,7 +80,11 @@ namespace Microsoft.AspNet.Mvc if (declaredReturnType == typeof(void) || declaredReturnType == typeof(Task)) { - return new NoContentResult(); + return new ObjectResult(null) + { + // Treat the declared type as void, which is the unwrapped type for Task. + DeclaredType = typeof(void) + }; } // Unwrap potential Task types. diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/NoContentFormatterTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/NoContentFormatterTests.cs index 0210e3fb55..808bd22fc7 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/NoContentFormatterTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/NoContentFormatterTests.cs @@ -37,10 +37,10 @@ namespace Microsoft.AspNet.Mvc.Test [Theory] [MemberData(nameof(OutputFormatterContextValues_CanWriteType))] - public void CanWriteResult_ReturnsTrueOnlyIfTheValueIsNull(object value, - bool declaredTypeAsString, - bool expectedCanwriteResult, - bool useNonNullContentType) + public void CanWriteResult_ByDefault_ReturnsTrue_IfTheValueIsNull(object value, + bool declaredTypeAsString, + bool expectedCanwriteResult, + bool useNonNullContentType) { // Arrange var typeToUse = declaredTypeAsString ? typeof(string) : typeof(object); @@ -60,6 +60,58 @@ namespace Microsoft.AspNet.Mvc.Test Assert.Equal(expectedCanwriteResult, actualCanWriteResult); } + [Theory] + [InlineData(typeof(void))] + [InlineData(typeof(Task))] + public void CanWriteResult_ReturnsTrue_IfReturnTypeIsVoidOrTask(Type declaredType) + { + // Arrange + var formatterContext = new OutputFormatterContext() + { + Object = "Something non null.", + DeclaredType = declaredType, + ActionContext = null, + }; + var contetType = MediaTypeHeaderValue.Parse("text/plain"); + var formatter = new HttpNoContentOutputFormatter(); + + // Act + var actualCanWriteResult = formatter.CanWriteResult(formatterContext, contetType); + + // Assert + Assert.True(actualCanWriteResult); + } + + [Theory] + [InlineData(null, true, true)] + [InlineData(null, false, false)] + [InlineData("some value", true, false)] + public void + CanWriteResult_ReturnsTrue_IfReturnValueIsNullAndTreatNullValueAsNoContentIsNotSet(string value, + bool treatNullValueAsNoContent, + bool expectedCanwriteResult) + { + // Arrange + var formatterContext = new OutputFormatterContext() + { + Object = value, + DeclaredType = typeof(string), + ActionContext = null, + }; + + var contetType = MediaTypeHeaderValue.Parse("text/plain"); + var formatter = new HttpNoContentOutputFormatter() + { + TreatNullValueAsNoContent = treatNullValueAsNoContent + }; + + // Act + var actualCanWriteResult = formatter.CanWriteResult(formatterContext, contetType); + + // Assert + Assert.Equal(expectedCanwriteResult, actualCanWriteResult); + } + [Fact] public async Task WriteAsync_WritesTheStatusCode204() { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs index 70d85031c3..1211895697 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionInvokerTest.cs @@ -1234,13 +1234,17 @@ namespace Microsoft.AspNet.Mvc [Theory] [InlineData(typeof(void))] [InlineData(typeof(Task))] - public void CreateActionResult_Types_ReturnsNoContentResultForTaskAndVoidReturnTypes(Type type) + public void CreateActionResult_Types_ReturnsObjectResultForTaskAndVoidReturnTypes(Type type) { // Arrange & Act - var result = ReflectedActionInvoker.CreateActionResult(type, null).GetType(); + var result = ReflectedActionInvoker.CreateActionResult(type, null); // Assert - Assert.Equal(typeof(NoContentResult), (result)); + var objectResult = Assert.IsType(result); + + // Since we unwrap the Task type to void, the expected type will always be void. + Assert.Equal(typeof(void), objectResult.DeclaredType); + Assert.Null(objectResult.Value); } public static IEnumerable CreateActionResult_ReturnsObjectContentResultData diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/OutputFormattterTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/OutputFormattterTests.cs index 9511454900..4af5bff60b 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/OutputFormattterTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/OutputFormattterTests.cs @@ -57,10 +57,9 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests } [Theory] - [InlineData("ReturnTaskOfString_NullValue")] - [InlineData("ReturnTaskOfObject_NullValue")] - [InlineData("ReturnObject_NullValue")] - public async Task NoContentFormatter_ForNullValue_GetsSelectedAndWritesResponse(string actionName) + [InlineData("ReturnTask")] + [InlineData("ReturnVoid")] + public async Task NoContentFormatter_ForVoidAndTaskReturnType_GetsSelectedAndWritesResponse(string actionName) { // Arrange var server = TestServer.Create(_provider, _app); @@ -77,5 +76,50 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); Assert.Equal(0, response.Content.Headers.ContentLength); } + + [Theory] + [InlineData("ReturnTaskOfString_NullValue")] + [InlineData("ReturnTaskOfObject_NullValue")] + [InlineData("ReturnObject_NullValue")] + public async Task NoContentFormatter_ForNullValue_ByDefault_GetsSelectedAndWritesResponse(string actionName) + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/NoContent/" + actionName); + + // Assert + Assert.Null(response.Content.Headers.ContentType); + var body = await response.Content.ReadAsStringAsync(); + // Response body is empty instead of null. + Assert.Empty(body); + Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); + Assert.Equal(0, response.Content.Headers.ContentLength); + } + + [Theory] + [InlineData("ReturnTaskOfString_NullValue")] + [InlineData("ReturnTaskOfObject_NullValue")] + [InlineData("ReturnObject_NullValue")] + public async Task + NoContentFormatter_ForNullValue_AndTreatNullAsNoContentFlagSetToFalse_DoesNotGetSelected(string actionName) + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync("http://localhost/NoContentDoNotTreatNullValueAsNoContent/" + + actionName); + + // Assert + Assert.Null(response.Content.Headers.ContentType); + var body = await response.Content.ReadAsStringAsync(); + // Response body is empty instead of null. + Assert.Empty(body); + Assert.Equal(HttpStatusCode.NotAcceptable, response.StatusCode); + } } } \ No newline at end of file diff --git a/test/WebSites/BasicWebSite/Controllers/HomeController.cs b/test/WebSites/BasicWebSite/Controllers/HomeController.cs index 4d2a414cf3..fbc8be7e55 100644 --- a/test/WebSites/BasicWebSite/Controllers/HomeController.cs +++ b/test/WebSites/BasicWebSite/Controllers/HomeController.cs @@ -34,7 +34,7 @@ namespace BasicWebSite.Controllers public async Task ActionReturningTask() { // TODO: #1077. With HttpResponseMessage, there seems to be a race between the write operation setting the - // header to 200 and NoContentResult returned by the action invoker setting it to 204. + // header to 200 and HttpNoContentOutputFormatter which sets the header to 204. Context.Response.StatusCode = 204; await Context.Response.WriteAsync("Hello world"); } diff --git a/test/WebSites/ConnegWebSite/Controllers/NoContentController.cs b/test/WebSites/ConnegWebSite/Controllers/NoContentController.cs index 6805cf195d..b2019243b8 100644 --- a/test/WebSites/ConnegWebSite/Controllers/NoContentController.cs +++ b/test/WebSites/ConnegWebSite/Controllers/NoContentController.cs @@ -27,5 +27,14 @@ namespace ConnegWebsite { return null; } + + public Task ReturnTask() + { + return Task.FromResult(true); + } + + public void ReturnVoid() + { + } } } \ No newline at end of file diff --git a/test/WebSites/ConnegWebSite/Controllers/NoContentDoNotTreatNullValueAsNoContentController.cs b/test/WebSites/ConnegWebSite/Controllers/NoContentDoNotTreatNullValueAsNoContentController.cs new file mode 100644 index 0000000000..a218236535 --- /dev/null +++ b/test/WebSites/ConnegWebSite/Controllers/NoContentDoNotTreatNullValueAsNoContentController.cs @@ -0,0 +1,44 @@ +// 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.Linq; +using System.Threading.Tasks; +using Microsoft.AspNet.Mvc; + +namespace ConnegWebsite +{ + public class NoContentDoNotTreatNullValueAsNoContentController : Controller + { + public override void OnActionExecuted(ActionExecutedContext context) + { + var result = context.Result as ObjectResult; + if (result != null) + { + var noContentFormatter = new HttpNoContentOutputFormatter() { TreatNullValueAsNoContent = false }; + result.Formatters.Add(noContentFormatter); + } + + base.OnActionExecuted(context); + } + + public Task ReturnTaskOfString_NullValue() + { + return Task.FromResult(null); + } + + public Task ReturnTaskOfObject_NullValue() + { + return Task.FromResult(null); + } + + public object ReturnObject_NullValue() + { + return null; + } + + public string ReturnString_NullValue() + { + return null; + } + } +} \ No newline at end of file