From ece6ecde45f6d5c12cdc2317ec3d3ebc60a6a317 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Mon, 9 Nov 2015 12:05:38 -0800 Subject: [PATCH] Add buffer pooling to JsonResult --- .../MvcJsonMvcCoreBuilderExtensions.cs | 2 + .../Infrastructure/JsonResultExecutor.cs | 125 ++++++++++++ .../JsonResult.cs | 67 +----- ... => JsonResultExecutorLoggerExtensions.cs} | 4 +- .../Infrastructure/JsonResultExecutorTest.cs | 193 ++++++++++++++++++ .../JsonResultTest.cs | 165 ++------------- 6 files changed, 352 insertions(+), 204 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.Formatters.Json/Infrastructure/JsonResultExecutor.cs rename src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/{JsonResultLoggerExtensions.cs => JsonResultExecutorLoggerExtensions.cs} (86%) create mode 100644 test/Microsoft.AspNet.Mvc.Formatters.Json.Test/Infrastructure/JsonResultExecutorTest.cs diff --git a/src/Microsoft.AspNet.Mvc.Formatters.Json/DependencyInjection/MvcJsonMvcCoreBuilderExtensions.cs b/src/Microsoft.AspNet.Mvc.Formatters.Json/DependencyInjection/MvcJsonMvcCoreBuilderExtensions.cs index 66cedeed09..26a87d0d80 100644 --- a/src/Microsoft.AspNet.Mvc.Formatters.Json/DependencyInjection/MvcJsonMvcCoreBuilderExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Formatters.Json/DependencyInjection/MvcJsonMvcCoreBuilderExtensions.cs @@ -4,6 +4,7 @@ using System; using Microsoft.AspNet.Mvc; using Microsoft.AspNet.Mvc.Formatters.Json.Internal; +using Microsoft.AspNet.Mvc.Infrastructure; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.OptionsModel; using Newtonsoft.Json; @@ -52,6 +53,7 @@ namespace Microsoft.Extensions.DependencyInjection { services.TryAddEnumerable( ServiceDescriptor.Transient, MvcJsonMvcOptionsSetup>()); + services.TryAddSingleton(); } } } diff --git a/src/Microsoft.AspNet.Mvc.Formatters.Json/Infrastructure/JsonResultExecutor.cs b/src/Microsoft.AspNet.Mvc.Formatters.Json/Infrastructure/JsonResultExecutor.cs new file mode 100644 index 0000000000..6cd36012eb --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Formatters.Json/Infrastructure/JsonResultExecutor.cs @@ -0,0 +1,125 @@ +// 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 System.Text; +using System.Threading.Tasks; +using Microsoft.AspNet.Mvc.Internal; +using Microsoft.AspNet.Mvc.Logging; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.OptionsModel; +using Microsoft.Net.Http.Headers; +using Newtonsoft.Json; + +namespace Microsoft.AspNet.Mvc.Infrastructure +{ + /// + /// Executes a to write to the response. + /// + public class JsonResultExecutor + { + private static readonly MediaTypeHeaderValue DefaultContentType = new MediaTypeHeaderValue("application/json") + { + Encoding = Encoding.UTF8 + }.CopyAsReadOnly(); + + /// + /// Creates a new . + /// + /// The . + /// The . + /// The . + public JsonResultExecutor( + IHttpResponseStreamWriterFactory writerFactory, + ILogger logger, + IOptions options) + { + if (writerFactory == null) + { + throw new ArgumentNullException(nameof(writerFactory)); + } + + if (logger == null) + { + throw new ArgumentNullException(nameof(logger)); + } + + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + WriterFactory = writerFactory; + Logger = logger; + Options = options.Value; + } + + /// + /// Gets the . + /// + protected ILogger Logger { get; } + + /// + /// Gets the . + /// + protected MvcJsonOptions Options { get; } + + /// + /// Gets the . + /// + protected IHttpResponseStreamWriterFactory WriterFactory { get; } + + /// + /// Executes the and writes the response. + /// + /// The . + /// The . + /// A which will complete when writing has completed. + public Task ExecuteAsync(ActionContext context, JsonResult result) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (result == null) + { + throw new ArgumentNullException(nameof(result)); + } + + var response = context.HttpContext.Response; + + string resolvedContentType = null; + Encoding resolvedContentTypeEncoding = null; + ResponseContentTypeHelper.ResolveContentTypeAndEncoding( + result.ContentType, + response.ContentType, + DefaultContentType, + out resolvedContentType, + out resolvedContentTypeEncoding); + + response.ContentType = resolvedContentType; + + if (result.StatusCode != null) + { + response.StatusCode = result.StatusCode.Value; + } + + var serializerSettings = result.SerializerSettings ?? Options.SerializerSettings; + + Logger.JsonResultExecuting(result.Value); + using (var writer = WriterFactory.CreateWriter(response.Body, resolvedContentTypeEncoding)) + { + using (var jsonWriter = new JsonTextWriter(writer)) + { + jsonWriter.CloseOutput = false; + + var jsonSerializer = JsonSerializer.Create(serializerSettings); + jsonSerializer.Serialize(jsonWriter, result.Value); + } + } + + return TaskCache.CompletedTask; + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Formatters.Json/JsonResult.cs b/src/Microsoft.AspNet.Mvc.Formatters.Json/JsonResult.cs index 9273b70af3..861335d8e6 100644 --- a/src/Microsoft.AspNet.Mvc.Formatters.Json/JsonResult.cs +++ b/src/Microsoft.AspNet.Mvc.Formatters.Json/JsonResult.cs @@ -2,15 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Text; using System.Threading.Tasks; -using Microsoft.AspNet.Mvc.Internal; +using Microsoft.AspNet.Mvc.Infrastructure; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.OptionsModel; using Microsoft.Net.Http.Headers; using Newtonsoft.Json; -using Microsoft.AspNet.Mvc.Logging; namespace Microsoft.AspNet.Mvc { @@ -19,13 +15,6 @@ namespace Microsoft.AspNet.Mvc /// public class JsonResult : ActionResult { - private readonly JsonSerializerSettings _serializerSettings; - - private static readonly MediaTypeHeaderValue DefaultContentType = new MediaTypeHeaderValue("application/json") - { - Encoding = Encoding.UTF8 - }; - /// /// Creates a new with the given . /// @@ -49,7 +38,7 @@ namespace Microsoft.AspNet.Mvc } Value = value; - _serializerSettings = serializerSettings; + SerializerSettings = serializerSettings; } /// @@ -57,6 +46,11 @@ namespace Microsoft.AspNet.Mvc /// public MediaTypeHeaderValue ContentType { get; set; } + /// + /// Gets or sets the . + /// + public JsonSerializerSettings SerializerSettings { get; set; } + /// /// Gets or sets the HTTP status code. /// @@ -75,50 +69,9 @@ namespace Microsoft.AspNet.Mvc throw new ArgumentNullException(nameof(context)); } - var loggerFactory = context.HttpContext.RequestServices.GetRequiredService(); - var logger = loggerFactory.CreateLogger(); - - var response = context.HttpContext.Response; - - string resolvedContentType = null; - Encoding resolvedContentTypeEncoding = null; - ResponseContentTypeHelper.ResolveContentTypeAndEncoding( - ContentType, - response.ContentType, - DefaultContentType, - out resolvedContentType, - out resolvedContentTypeEncoding); - - response.ContentType = resolvedContentType; - - if (StatusCode != null) - { - response.StatusCode = StatusCode.Value; - } - - var serializerSettings = _serializerSettings; - if (serializerSettings == null) - { - serializerSettings = context - .HttpContext - .RequestServices - .GetRequiredService>() - .Value - .SerializerSettings; - } - - logger.JsonResultExecuting(Value); - using (var writer = new HttpResponseStreamWriter(response.Body, resolvedContentTypeEncoding)) - { - using (var jsonWriter = new JsonTextWriter(writer)) - { - jsonWriter.CloseOutput = false; - var jsonSerializer = JsonSerializer.Create(serializerSettings); - jsonSerializer.Serialize(jsonWriter, Value); - } - } - - return Task.FromResult(true); + var services = context.HttpContext.RequestServices; + var executor = services.GetRequiredService(); + return executor.ExecuteAsync(context, this); } } } diff --git a/src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonResultExecutorLoggerExtensions.cs similarity index 86% rename from src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonResultLoggerExtensions.cs rename to src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonResultExecutorLoggerExtensions.cs index 095f1ccaa9..964a3661c6 100644 --- a/src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonResultLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonResultExecutorLoggerExtensions.cs @@ -6,11 +6,11 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - internal static class JsonResultLoggerExtensions + internal static class JsonResultExecutorLoggerExtensions { private static readonly Action _jsonResultExecuting; - static JsonResultLoggerExtensions() + static JsonResultExecutorLoggerExtensions() { _jsonResultExecuting = LoggerMessage.Define( LogLevel.Information, diff --git a/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/Infrastructure/JsonResultExecutorTest.cs b/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/Infrastructure/JsonResultExecutorTest.cs new file mode 100644 index 0000000000..e64aa2d57e --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/Infrastructure/JsonResultExecutorTest.cs @@ -0,0 +1,193 @@ +// 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 System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Internal; +using Microsoft.AspNet.Mvc.Abstractions; +using Microsoft.AspNet.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; +using Microsoft.Net.Http.Headers; +using Newtonsoft.Json; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Infrastructure +{ + public class JsonResultExecutorTest + { + [Fact] + public async Task ExecuteAsync_UsesDefaultContentType_IfNoContentTypeSpecified() + { + // Arrange + var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + + var context = GetActionContext(); + + var result = new JsonResult(new { foo = "abcd" }); + var executor = CreateExcutor(); + + // Act + await executor.ExecuteAsync(context, result); + + // Assert + var written = GetWrittenBytes(context.HttpContext); + Assert.Equal(expected, written); + Assert.Equal("application/json; charset=utf-8", context.HttpContext.Response.ContentType); + } + + [Fact] + public async Task ExecuteAsync_NullEncoding_DoesNotSetCharsetOnContentType() + { + // Arrange + var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + + var context = GetActionContext(); + + var result = new JsonResult(new { foo = "abcd" }); + result.ContentType = new MediaTypeHeaderValue("text/json"); + var executor = CreateExcutor(); + + // Act + await executor.ExecuteAsync(context, result); + + // Assert + var written = GetWrittenBytes(context.HttpContext); + Assert.Equal(expected, written); + Assert.Equal("text/json", context.HttpContext.Response.ContentType); + } + + [Fact] + public async Task ExecuteAsync_SetsContentTypeAndEncoding() + { + // Arrange + var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + + var context = GetActionContext(); + + var result = new JsonResult(new { foo = "abcd" }); + result.ContentType = new MediaTypeHeaderValue("text/json") + { + Encoding = Encoding.ASCII + }; + var executor = CreateExcutor(); + + // Act + await executor.ExecuteAsync(context, result); + + // Assert + var written = GetWrittenBytes(context.HttpContext); + Assert.Equal(expected, written); + Assert.Equal("text/json; charset=us-ascii", context.HttpContext.Response.ContentType); + } + + [Fact] + public async Task ExecuteAsync_NoResultContentTypeSet_UsesResponseContentType_AndSuppliedEncoding() + { + // Arrange + var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + var expectedContentType = "text/foo; p1=p1-value; charset=us-ascii"; + + var context = GetActionContext(); + context.HttpContext.Response.ContentType = expectedContentType; + + var result = new JsonResult(new { foo = "abcd" }); + var executor = CreateExcutor(); + + // Act + await executor.ExecuteAsync(context, result); + + // Assert + var written = GetWrittenBytes(context.HttpContext); + Assert.Equal(expected, written); + Assert.Equal(expectedContentType, context.HttpContext.Response.ContentType); + } + + [Theory] + [InlineData("text/foo", "text/foo")] + [InlineData("text/foo; p1=p1-value", "text/foo; p1=p1-value")] + public async Task ExecuteAsync_NoResultContentTypeSet_UsesDefaultEncoding_DoesNotSetCharset( + string responseContentType, + string expectedContentType) + { + // Arrange + var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + + var context = GetActionContext(); + context.HttpContext.Response.ContentType = responseContentType; + + var result = new JsonResult(new { foo = "abcd" }); + var executor = CreateExcutor(); + + // Act + await executor.ExecuteAsync(context, result); + + // Assert + var written = GetWrittenBytes(context.HttpContext); + Assert.Equal(expected, written); + Assert.Equal(expectedContentType, context.HttpContext.Response.ContentType); + } + + [Fact] + public async Task ExecuteAsync_UsesPassedInSerializerSettings() + { + // Arrange + var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject( + new { foo = "abcd" }, + Formatting.Indented)); + + var context = GetActionContext(); + + var serializerSettings = new JsonSerializerSettings(); + serializerSettings.Formatting = Formatting.Indented; + + var result = new JsonResult(new { foo = "abcd" }, serializerSettings); + var executor = CreateExcutor(); + + // Act + await executor.ExecuteAsync(context, result); + + // Assert + var written = GetWrittenBytes(context.HttpContext); + Assert.Equal(expected, written); + Assert.Equal("application/json; charset=utf-8", context.HttpContext.Response.ContentType); + } + + private static JsonResultExecutor CreateExcutor() + { + return new JsonResultExecutor( + new TestHttpResponseStreamWriterFactory(), + NullLogger.Instance, + new TestOptionsManager()); + } + + private static HttpContext GetHttpContext() + { + var httpContext = new DefaultHttpContext(); + httpContext.Response.Body = new MemoryStream(); + + var services = new ServiceCollection(); + services.AddOptions(); + services.AddInstance(NullLoggerFactory.Instance); + httpContext.RequestServices = services.BuildServiceProvider(); + + return httpContext; + } + + private static ActionContext GetActionContext() + { + return new ActionContext(GetHttpContext(), new RouteData(), new ActionDescriptor()); + } + + private static byte[] GetWrittenBytes(HttpContext context) + { + context.Response.Body.Seek(0, SeekOrigin.Begin); + return Assert.IsType(context.Response.Body).ToArray(); + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonResultTest.cs b/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonResultTest.cs index 946b660a82..ae5ee4ed9d 100644 --- a/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonResultTest.cs @@ -1,19 +1,16 @@ // 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 System.Collections.Generic; using System.IO; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc.Abstractions; +using Microsoft.AspNet.Mvc.Infrastructure; using Microsoft.AspNet.Routing; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; -using Microsoft.Net.Http.Headers; using Newtonsoft.Json; using Xunit; @@ -21,155 +18,24 @@ namespace Microsoft.AspNet.Mvc { public class JsonResultTest { - private static readonly byte[] _abcdUTF8Bytes - = new byte[] { 123, 34, 102, 111, 111, 34, 58, 34, 97, 98, 99, 100, 34, 125 }; - [Fact] - public async Task ExecuteResultAsync_UsesDefaultContentType_IfNoContentTypeSpecified() + public async Task ExecuteAsync_WritesJsonContent() { // Arrange - var expected = _abcdUTF8Bytes; + var value = new { foo = "abcd" }; + var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(value)); - var context = GetHttpContext(); - var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); + var context = GetActionContext(); - var result = new JsonResult(new { foo = "abcd" }); + var result = new JsonResult(value); // Act - await result.ExecuteResultAsync(actionContext); - var written = GetWrittenBytes(context); + await result.ExecuteResultAsync(context); // Assert + var written = GetWrittenBytes(context.HttpContext); Assert.Equal(expected, written); - Assert.Equal("application/json; charset=utf-8", context.Response.ContentType); - } - - [Fact] - public async Task ExecuteResultAsync_NullEncoding_DoesNotSetCharsetOnContentType() - { - // Arrange - var expected = _abcdUTF8Bytes; - - var context = GetHttpContext(); - var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); - - var result = new JsonResult(new { foo = "abcd" }); - result.ContentType = new MediaTypeHeaderValue("text/json"); - - // Act - await result.ExecuteResultAsync(actionContext); - var written = GetWrittenBytes(context); - - // Assert - Assert.Equal(expected, written); - Assert.Equal("text/json", context.Response.ContentType); - } - - [Fact] - public async Task ExecuteResultAsync_SetsContentTypeAndEncoding() - { - // Arrange - var expected = _abcdUTF8Bytes; - - var context = GetHttpContext(); - var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); - - var result = new JsonResult(new { foo = "abcd" }); - result.ContentType = new MediaTypeHeaderValue("text/json") - { - Encoding = Encoding.ASCII - }; - - // Act - await result.ExecuteResultAsync(actionContext); - var written = GetWrittenBytes(context); - - // Assert - Assert.Equal(expected, written); - Assert.Equal("text/json; charset=us-ascii", context.Response.ContentType); - } - - [Fact] - public async Task NoResultContentTypeSet_UsesResponseContentType_AndSuppliedEncoding() - { - // Arrange - var expectedData = Encoding.ASCII.GetBytes("{\"foo\":\"abcd\"}"); - var expectedContentType = "text/foo; p1=p1-value; charset=us-ascii"; - var context = GetHttpContext(); - context.Response.ContentType = expectedContentType; - var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); - - var result = new JsonResult(new { foo = "abcd" }); - - // Act - await result.ExecuteResultAsync(actionContext); - var written = GetWrittenBytes(context); - - // Assert - Assert.Equal(expectedData, written); - Assert.Equal(expectedContentType, context.Response.ContentType); - } - - [Theory] - [InlineData("text/foo", "text/foo")] - [InlineData("text/foo; p1=p1-value", "text/foo; p1=p1-value")] - public async Task NoResultContentTypeSet_UsesResponseContentTypeAndDefaultEncoding_DoesNotSetCharset( - string responseContentType, - string expectedContentType) - { - // Arrange - var expected = _abcdUTF8Bytes; - - var context = GetHttpContext(); - context.Response.ContentType = responseContentType; - var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); - - var result = new JsonResult(new { foo = "abcd" }); - - // Act - await result.ExecuteResultAsync(actionContext); - var written = GetWrittenBytes(context); - - // Assert - Assert.Equal(expected, written); - Assert.Equal(expectedContentType, context.Response.ContentType); - } - - private static List AbcdIndentedUTF8Bytes - { - get - { - var bytes = new List(); - bytes.Add(123); - bytes.AddRange(Encoding.UTF8.GetBytes(Environment.NewLine)); - bytes.AddRange(new byte[] { 32, 32, 34, 102, 111, 111, 34, 58, 32, 34, 97, 98, 99, 100, 34 }); - bytes.AddRange(Encoding.UTF8.GetBytes(Environment.NewLine)); - bytes.Add(125); - return bytes; - } - } - - [Fact] - public async Task ExecuteResultAsync_UsesPassedInSerializerSettings() - { - // Arrange - var expected = AbcdIndentedUTF8Bytes; - - var context = GetHttpContext(); - var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); - - var serializerSettings = new JsonSerializerSettings(); - serializerSettings.Formatting = Formatting.Indented; - - var result = new JsonResult(new { foo = "abcd" }, serializerSettings); - - // Act - await result.ExecuteResultAsync(actionContext); - var written = GetWrittenBytes(context); - - // Assert - Assert.Equal(expected, written); - Assert.Equal("application/json; charset=utf-8", context.Response.ContentType); + Assert.Equal("application/json; charset=utf-8", context.HttpContext.Response.ContentType); } private static HttpContext GetHttpContext() @@ -177,14 +43,23 @@ namespace Microsoft.AspNet.Mvc var httpContext = new DefaultHttpContext(); httpContext.Response.Body = new MemoryStream(); + var executor = new JsonResultExecutor( + new TestHttpResponseStreamWriterFactory(), + NullLogger.Instance, + new TestOptionsManager()); + var services = new ServiceCollection(); - services.AddOptions(); - services.AddInstance(NullLoggerFactory.Instance); + services.AddInstance(executor); httpContext.RequestServices = services.BuildServiceProvider(); return httpContext; } + private static ActionContext GetActionContext() + { + return new ActionContext(GetHttpContext(), new RouteData(), new ActionDescriptor()); + } + private static byte[] GetWrittenBytes(HttpContext context) { context.Response.Body.Seek(0, SeekOrigin.Begin);