Invoke FlushAsync before disposing the HttpResponseWriter in JsonResultExecutor

Fixes #8486
This commit is contained in:
Pranav K 2018-09-24 10:46:54 -07:00
parent db7555b0ba
commit 50cef4822a
No known key found for this signature in database
GPG Key ID: 1963DA6D96C3057A
2 changed files with 67 additions and 3 deletions

View File

@ -86,7 +86,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Json.Internal
/// <param name="context">The <see cref="ActionContext"/>.</param>
/// <param name="result">The <see cref="JsonResult"/>.</param>
/// <returns>A <see cref="Task"/> which will complete when writing has completed.</returns>
public virtual Task ExecuteAsync(ActionContext context, JsonResult result)
public virtual async Task ExecuteAsync(ActionContext context, JsonResult result)
{
if (context == null)
{
@ -128,9 +128,11 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Json.Internal
var jsonSerializer = JsonSerializer.Create(serializerSettings);
jsonSerializer.Serialize(jsonWriter, result.Value);
}
}
return Task.CompletedTask;
// Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's
// buffers. This is better than just letting dispose handle it (which would result in a synchronous write).
await writer.FlushAsync();
}
}
}
}

View File

@ -5,6 +5,7 @@ using System;
using System.Buffers;
using System.IO;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Abstractions;
@ -14,6 +15,7 @@ using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using Microsoft.Net.Http.Headers;
using Moq;
using Newtonsoft.Json;
using Xunit;
@ -217,6 +219,66 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Json.Internal
Assert.Equal(expected, logger.MostRecentMessage);
}
[Fact]
public async Task ExecuteAsync_WritesToTheResponseStream_WhenContentIsLargerThanBuffer()
{
// Arrange
var writeLength = 2 * TestHttpResponseStreamWriterFactory.DefaultBufferSize + 4;
var text = new string('a', writeLength);
var expectedWriteCallCount = Math.Ceiling((double)writeLength / TestHttpResponseStreamWriterFactory.DefaultBufferSize);
var stream = new Mock<Stream>();
stream.SetupGet(s => s.CanWrite).Returns(true);
var httpContext = new DefaultHttpContext();
httpContext.Response.Body = stream.Object;
var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor());
var result = new JsonResult(text);
var executor = CreateExecutor();
// Act
await executor.ExecuteAsync(actionContext, result);
// Assert
// HttpResponseStreamWriter buffers content up to the buffer size (16k). When writes exceed the buffer size, it'll perform a synchronous
// write to the response stream.
stream.Verify(s => s.Write(It.IsAny<byte[]>(), It.IsAny<int>(), TestHttpResponseStreamWriterFactory.DefaultBufferSize), Times.Exactly(2));
// Remainder buffered content is written asynchronously as part of the FlushAsync.
stream.Verify(s => s.WriteAsync(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<CancellationToken>()), Times.Once());
// Dispose does not call Flush
stream.Verify(s => s.Flush(), Times.Never());
}
[Theory]
[InlineData(5)]
[InlineData(TestHttpResponseStreamWriterFactory.DefaultBufferSize - 30)]
public async Task ExecuteAsync_DoesNotWriteSynchronouslyToTheResponseBody_WhenContentIsSmallerThanBufferSize(int writeLength)
{
// Arrange
var text = new string('a', writeLength);
var stream = new Mock<Stream>();
stream.SetupGet(s => s.CanWrite).Returns(true);
var httpContext = new DefaultHttpContext();
httpContext.Response.Body = stream.Object;
var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor());
var result = new JsonResult(text);
var executor = CreateExecutor();
// Act
await executor.ExecuteAsync(actionContext, result);
// Assert
// HttpResponseStreamWriter buffers content up to the buffer size (16k) and will asynchronously write content to the response as part
// of the FlushAsync call if the content written to it is smaller than the buffer size.
// This test verifies that no synchronous writes are performed in this scenario.
stream.Verify(s => s.Flush(), Times.Never());
stream.Verify(s => s.Write(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>()), Times.Never());
}
private static JsonResultExecutor CreateExecutor(ILogger<JsonResultExecutor> logger = null)
{
return new JsonResultExecutor(