Fix for 1071: Output Formatters should be invoked for writing out the response.
This commit is contained in:
parent
f076fc7dcf
commit
572ec0175c
|
|
@ -13,11 +13,23 @@ namespace Microsoft.AspNet.Mvc
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public class HttpNoContentOutputFormatter : IOutputFormatter
|
public class HttpNoContentOutputFormatter : IOutputFormatter
|
||||||
{
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// Indicates whether to select this formatter if the returned value from the action
|
||||||
|
/// is null.
|
||||||
|
/// </summary>
|
||||||
|
public bool TreatNullValueAsNoContent { get; set; } = true;
|
||||||
|
|
||||||
public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType)
|
public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType)
|
||||||
{
|
{
|
||||||
// ignore the contentType and just look at the content.
|
// ignore the contentType and just look at the content.
|
||||||
// This formatter will be selected if the content is null.
|
// 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<MediaTypeHeaderValue> GetSupportedContentTypes(
|
public IReadOnlyList<MediaTypeHeaderValue> GetSupportedContentTypes(
|
||||||
|
|
|
||||||
|
|
@ -80,7 +80,11 @@ namespace Microsoft.AspNet.Mvc
|
||||||
if (declaredReturnType == typeof(void) ||
|
if (declaredReturnType == typeof(void) ||
|
||||||
declaredReturnType == typeof(Task))
|
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<T> types.
|
// Unwrap potential Task<T> types.
|
||||||
|
|
|
||||||
|
|
@ -37,10 +37,10 @@ namespace Microsoft.AspNet.Mvc.Test
|
||||||
|
|
||||||
[Theory]
|
[Theory]
|
||||||
[MemberData(nameof(OutputFormatterContextValues_CanWriteType))]
|
[MemberData(nameof(OutputFormatterContextValues_CanWriteType))]
|
||||||
public void CanWriteResult_ReturnsTrueOnlyIfTheValueIsNull(object value,
|
public void CanWriteResult_ByDefault_ReturnsTrue_IfTheValueIsNull(object value,
|
||||||
bool declaredTypeAsString,
|
bool declaredTypeAsString,
|
||||||
bool expectedCanwriteResult,
|
bool expectedCanwriteResult,
|
||||||
bool useNonNullContentType)
|
bool useNonNullContentType)
|
||||||
{
|
{
|
||||||
// Arrange
|
// Arrange
|
||||||
var typeToUse = declaredTypeAsString ? typeof(string) : typeof(object);
|
var typeToUse = declaredTypeAsString ? typeof(string) : typeof(object);
|
||||||
|
|
@ -60,6 +60,58 @@ namespace Microsoft.AspNet.Mvc.Test
|
||||||
Assert.Equal(expectedCanwriteResult, actualCanWriteResult);
|
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]
|
[Fact]
|
||||||
public async Task WriteAsync_WritesTheStatusCode204()
|
public async Task WriteAsync_WritesTheStatusCode204()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -1234,13 +1234,17 @@ namespace Microsoft.AspNet.Mvc
|
||||||
[Theory]
|
[Theory]
|
||||||
[InlineData(typeof(void))]
|
[InlineData(typeof(void))]
|
||||||
[InlineData(typeof(Task))]
|
[InlineData(typeof(Task))]
|
||||||
public void CreateActionResult_Types_ReturnsNoContentResultForTaskAndVoidReturnTypes(Type type)
|
public void CreateActionResult_Types_ReturnsObjectResultForTaskAndVoidReturnTypes(Type type)
|
||||||
{
|
{
|
||||||
// Arrange & Act
|
// Arrange & Act
|
||||||
var result = ReflectedActionInvoker.CreateActionResult(type, null).GetType();
|
var result = ReflectedActionInvoker.CreateActionResult(type, null);
|
||||||
|
|
||||||
// Assert
|
// Assert
|
||||||
Assert.Equal(typeof(NoContentResult), (result));
|
var objectResult = Assert.IsType<ObjectResult>(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<object[]> CreateActionResult_ReturnsObjectContentResultData
|
public static IEnumerable<object[]> CreateActionResult_ReturnsObjectContentResultData
|
||||||
|
|
|
||||||
|
|
@ -57,10 +57,9 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
|
||||||
}
|
}
|
||||||
|
|
||||||
[Theory]
|
[Theory]
|
||||||
[InlineData("ReturnTaskOfString_NullValue")]
|
[InlineData("ReturnTask")]
|
||||||
[InlineData("ReturnTaskOfObject_NullValue")]
|
[InlineData("ReturnVoid")]
|
||||||
[InlineData("ReturnObject_NullValue")]
|
public async Task NoContentFormatter_ForVoidAndTaskReturnType_GetsSelectedAndWritesResponse(string actionName)
|
||||||
public async Task NoContentFormatter_ForNullValue_GetsSelectedAndWritesResponse(string actionName)
|
|
||||||
{
|
{
|
||||||
// Arrange
|
// Arrange
|
||||||
var server = TestServer.Create(_provider, _app);
|
var server = TestServer.Create(_provider, _app);
|
||||||
|
|
@ -77,5 +76,50 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
|
||||||
Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);
|
Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);
|
||||||
Assert.Equal(0, response.Content.Headers.ContentLength);
|
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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -34,7 +34,7 @@ namespace BasicWebSite.Controllers
|
||||||
public async Task ActionReturningTask()
|
public async Task ActionReturningTask()
|
||||||
{
|
{
|
||||||
// TODO: #1077. With HttpResponseMessage, there seems to be a race between the write operation setting the
|
// 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;
|
Context.Response.StatusCode = 204;
|
||||||
await Context.Response.WriteAsync("Hello world");
|
await Context.Response.WriteAsync("Hello world");
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -27,5 +27,14 @@ namespace ConnegWebsite
|
||||||
{
|
{
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public Task ReturnTask()
|
||||||
|
{
|
||||||
|
return Task.FromResult<bool>(true);
|
||||||
|
}
|
||||||
|
|
||||||
|
public void ReturnVoid()
|
||||||
|
{
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -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<string> ReturnTaskOfString_NullValue()
|
||||||
|
{
|
||||||
|
return Task.FromResult<string>(null);
|
||||||
|
}
|
||||||
|
|
||||||
|
public Task<object> ReturnTaskOfObject_NullValue()
|
||||||
|
{
|
||||||
|
return Task.FromResult<object>(null);
|
||||||
|
}
|
||||||
|
|
||||||
|
public object ReturnObject_NullValue()
|
||||||
|
{
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
public string ReturnString_NullValue()
|
||||||
|
{
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue