From 5e953124e6071e10fd71834afc1e45d6e9a04b97 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Sun, 21 Apr 2019 09:13:30 -0700 Subject: [PATCH] Add support for System.Text.Json based JsonResult executor (#9558) Fixes https://github.com/aspnet/AspNetCore/issues/9554 --- .../src/Formatters/TranscodingReadStream.cs | 19 ++- .../src/Formatters/TranscodingWriteStream.cs | 7 +- .../SystemTextJsonResultExecutor.cs | 133 ++++++++++++++++ src/Mvc/Mvc.Core/src/JsonResult.cs | 12 +- .../Mvc.Core/src/Properties/AssemblyInfo.cs | 1 + .../src/Properties/Resources.Designer.cs | 18 ++- src/Mvc/Mvc.Core/src/Resources.resx | 5 +- .../JsonResultExecutorTestBase.cs} | 144 ++++++++++++------ .../SystemTextJsonResultExecutorTest.cs | 22 +++ src/Mvc/Mvc.Core/test/JsonResultTest.cs | 32 ---- .../NewtonsoftJsonMvcCoreBuilderExtensions.cs | 21 ++- ...tor.cs => NewtonsoftJsonResultExecutor.cs} | 10 +- ...tonsoftJsonMvcCoreBuilderExtensionsTest.cs | 16 ++ .../Mvc.NewtonsoftJson/test/JsonResultTest.cs | 6 +- ....AspNetCore.Mvc.NewtonsoftJson.Test.csproj | 2 + .../test/NewtonsoftJsonResultExecutorTest.cs | 29 ++++ 16 files changed, 353 insertions(+), 124 deletions(-) create mode 100644 src/Mvc/Mvc.Core/src/Infrastructure/SystemTextJsonResultExecutor.cs rename src/Mvc/{Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs => Mvc.Core/test/Infrastructure/JsonResultExecutorTestBase.cs} (66%) create mode 100644 src/Mvc/Mvc.Core/test/Infrastructure/SystemTextJsonResultExecutorTest.cs delete mode 100644 src/Mvc/Mvc.Core/test/JsonResultTest.cs rename src/Mvc/Mvc.NewtonsoftJson/src/{JsonResultExecutor.cs => NewtonsoftJsonResultExecutor.cs} (94%) create mode 100644 src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonResultExecutorTest.cs diff --git a/src/Mvc/Mvc.Core/src/Formatters/TranscodingReadStream.cs b/src/Mvc/Mvc.Core/src/Formatters/TranscodingReadStream.cs index 297ad3cc33..9657f5947e 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/TranscodingReadStream.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/TranscodingReadStream.cs @@ -24,6 +24,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Json private ArraySegment _byteBuffer; private ArraySegment _charBuffer; private ArraySegment _overflowBuffer; + private bool _disposed; public TranscodingReadStream(Stream input, Encoding sourceEncoding) { @@ -150,9 +151,9 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Json readBuffer = readBuffer.Slice(bytesEncoded); } - // We need to exit in one of the 2 conditions: - // * encoderCompleted will return false if "buffer" was too small for all the chars to be encoded. - // * no bytes were converted in an iteration. This can occur if there wasn't any input. + // We need to exit in one of the 2 conditions: + // * encoderCompleted will return false if "buffer" was too small for all the chars to be encoded. + // * no bytes were converted in an iteration. This can occur if there wasn't any input. } while (encoderCompleted && bytesEncoded > 0); return totalBytes; @@ -224,11 +225,13 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Json protected override void Dispose(bool disposing) { - ArrayPool.Shared.Return(_charBuffer.Array); - ArrayPool.Shared.Return(_byteBuffer.Array); - ArrayPool.Shared.Return(_overflowBuffer.Array); - - base.Dispose(disposing); + if (!_disposed) + { + _disposed = true; + ArrayPool.Shared.Return(_charBuffer.Array); + ArrayPool.Shared.Return(_byteBuffer.Array); + ArrayPool.Shared.Return(_overflowBuffer.Array); + } } } } diff --git a/src/Mvc/Mvc.Core/src/Formatters/TranscodingWriteStream.cs b/src/Mvc/Mvc.Core/src/Formatters/TranscodingWriteStream.cs index 5062778d89..07a975c9e0 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/TranscodingWriteStream.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/TranscodingWriteStream.cs @@ -21,6 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Json private readonly Encoder _encoder; private readonly char[] _charBuffer; private int _charsDecoded; + private bool _disposed; public TranscodingWriteStream(Stream stream, Encoding targetEncoding) { @@ -154,13 +155,11 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Json protected override void Dispose(bool disposing) { - if (disposing) + if (!_disposed) { + _disposed = true; ArrayPool.Shared.Return(_charBuffer); } - - - base.Dispose(disposing); } } } diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/SystemTextJsonResultExecutor.cs b/src/Mvc/Mvc.Core/src/Infrastructure/SystemTextJsonResultExecutor.cs new file mode 100644 index 0000000000..c3e0e5f9dd --- /dev/null +++ b/src/Mvc/Mvc.Core/src/Infrastructure/SystemTextJsonResultExecutor.cs @@ -0,0 +1,133 @@ +// 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.Text; +using System.Text.Json.Serialization; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Core; +using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.AspNetCore.Mvc.Formatters.Json; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Microsoft.Net.Http.Headers; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + internal sealed class SystemTextJsonResultExecutor : IActionResultExecutor + { + private static readonly string DefaultContentType = new MediaTypeHeaderValue("application/json") + { + Encoding = Encoding.UTF8 + }.ToString(); + + private readonly MvcOptions _mvcOptions; + private readonly ILogger _logger; + + public SystemTextJsonResultExecutor( + IOptions mvcOptions, + ILogger logger) + { + _mvcOptions = mvcOptions.Value; + _logger = logger; + } + + public async Task ExecuteAsync(ActionContext context, JsonResult result) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (result == null) + { + throw new ArgumentNullException(nameof(result)); + } + + var jsonSerializerOptions = GetSerializerOptions(result); + + var response = context.HttpContext.Response; + + ResponseContentTypeHelper.ResolveContentTypeAndEncoding( + result.ContentType, + response.ContentType, + DefaultContentType, + out var resolvedContentType, + out var resolvedContentTypeEncoding); + + response.ContentType = resolvedContentType; + + if (result.StatusCode != null) + { + response.StatusCode = result.StatusCode.Value; + } + + Log.JsonResultExecuting(_logger, result.Value); + + // Keep this code in sync with SystemTextJsonOutputFormatter + var writeStream = GetWriteStream(context.HttpContext, resolvedContentTypeEncoding); + try + { + var type = result.Value?.GetType() ?? typeof(object); + await JsonSerializer.WriteAsync(result.Value, type, writeStream, jsonSerializerOptions); + await writeStream.FlushAsync(); + } + finally + { + if (writeStream is TranscodingWriteStream transcoding) + { + await transcoding.DisposeAsync(); + } + } + } + + private Stream GetWriteStream(HttpContext httpContext, Encoding selectedEncoding) + { + if (selectedEncoding.CodePage == Encoding.UTF8.CodePage) + { + // JsonSerializer does not write a BOM. Therefore we do not have to handle it + // in any special way. + return httpContext.Response.Body; + } + + return new TranscodingWriteStream(httpContext.Response.Body, selectedEncoding); + } + + private JsonSerializerOptions GetSerializerOptions(JsonResult result) + { + var serializerSettings = result.SerializerSettings; + if (serializerSettings == null) + { + return _mvcOptions.SerializerOptions; + } + else + { + if (!(serializerSettings is JsonSerializerOptions settingsFromResult)) + { + throw new InvalidOperationException(Resources.FormatProperty_MustBeInstanceOfType( + nameof(JsonResult), + nameof(JsonResult.SerializerSettings), + typeof(JsonSerializerOptions))); + } + + return settingsFromResult; + } + } + + private static class Log + { + private static readonly Action _jsonResultExecuting = LoggerMessage.Define( + LogLevel.Information, + new EventId(1, "JsonResultExecuting"), + "Executing JsonResult, writing value of type '{Type}'."); + + public static void JsonResultExecuting(ILogger logger, object value) + { + var type = value == null ? "null" : value.GetType().FullName; + _jsonResultExecuting(logger, type, null); + } + } + } +} diff --git a/src/Mvc/Mvc.Core/src/JsonResult.cs b/src/Mvc/Mvc.Core/src/JsonResult.cs index ac0107311b..c62cf5e49e 100644 --- a/src/Mvc/Mvc.Core/src/JsonResult.cs +++ b/src/Mvc/Mvc.Core/src/JsonResult.cs @@ -68,17 +68,7 @@ namespace Microsoft.AspNetCore.Mvc } var services = context.HttpContext.RequestServices; - var executor = services.GetService>(); - if (executor == null) - { - throw new InvalidOperationException(Resources.FormatReferenceToNewtonsoftJsonRequired( - $"{nameof(JsonResult)}.{nameof(ExecuteResultAsync)}", - "Microsoft.AspNetCore.Mvc.NewtonsoftJson", - nameof(IMvcBuilder), - "AddNewtonsoftJson", - "ConfigureServices(...)")); - } - + var executor = services.GetRequiredService>(); return executor.ExecuteAsync(context, this); } } diff --git a/src/Mvc/Mvc.Core/src/Properties/AssemblyInfo.cs b/src/Mvc/Mvc.Core/src/Properties/AssemblyInfo.cs index 526d0b26aa..c217d6411a 100644 --- a/src/Mvc/Mvc.Core/src/Properties/AssemblyInfo.cs +++ b/src/Mvc/Mvc.Core/src/Properties/AssemblyInfo.cs @@ -27,6 +27,7 @@ using Microsoft.AspNetCore.Mvc.Formatters; [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.Formatters.Xml.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.IntegrationTests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.Localization.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.NewtonsoftJson.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.Razor.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.RazorPages.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.TagHelpers.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] diff --git a/src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs b/src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs index 11a2b28a4e..2878cafd4c 100644 --- a/src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs +++ b/src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs @@ -1705,7 +1705,7 @@ namespace Microsoft.AspNetCore.Mvc.Core => string.Format(CultureInfo.CurrentCulture, GetString("ReferenceToNewtonsoftJsonRequired"), p0, p1, p2, p3, p4); /// - /// Collection bound to '{0}' exceeded {1}.{2} ({3}). This limit is a safeguard against incorrect model binders and models. Address issues in '{4}'. For example, this type may have a property with a model binder that always succeeds. See the {0}.{1} documentation for more information. + /// Collection bound to '{0}' exceeded {1}.{2} ({3}). This limit is a safeguard against incorrect model binders and models. Address issues in '{4}'. For example, this type may have a property with a model binder that always succeeds. See the {1}.{2} documentation for more information. /// internal static string ModelBinding_ExceededMaxModelBindingCollectionSize { @@ -1713,7 +1713,7 @@ namespace Microsoft.AspNetCore.Mvc.Core } /// - /// Collection bound to '{0}' exceeded {1}.{2} ({3}). This limit is a safeguard against incorrect model binders and models. Address issues in '{4}'. For example, this type may have a property with a model binder that always succeeds. See the {0}.{1} documentation for more information. + /// Collection bound to '{0}' exceeded {1}.{2} ({3}). This limit is a safeguard against incorrect model binders and models. Address issues in '{4}'. For example, this type may have a property with a model binder that always succeeds. See the {1}.{2} documentation for more information. /// internal static string FormatModelBinding_ExceededMaxModelBindingCollectionSize(object p0, object p1, object p2, object p3, object p4) => string.Format(CultureInfo.CurrentCulture, GetString("ModelBinding_ExceededMaxModelBindingCollectionSize"), p0, p1, p2, p3, p4); @@ -1732,6 +1732,20 @@ namespace Microsoft.AspNetCore.Mvc.Core internal static string FormatModelBinding_ExceededMaxModelBindingRecursionDepth(object p0, object p1, object p2, object p3) => string.Format(CultureInfo.CurrentCulture, GetString("ModelBinding_ExceededMaxModelBindingRecursionDepth"), p0, p1, p2, p3); + /// + /// Property '{0}.{1}' must be an instance of type '{2}'. + /// + internal static string Property_MustBeInstanceOfType + { + get => GetString("Property_MustBeInstanceOfType"); + } + + /// + /// Property '{0}.{1}' must be an instance of type '{2}'. + /// + internal static string FormatProperty_MustBeInstanceOfType(object p0, object p1, object p2) + => string.Format(CultureInfo.CurrentCulture, GetString("Property_MustBeInstanceOfType"), p0, p1, p2); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Mvc/Mvc.Core/src/Resources.resx b/src/Mvc/Mvc.Core/src/Resources.resx index 181d9aa923..bbf596d558 100644 --- a/src/Mvc/Mvc.Core/src/Resources.resx +++ b/src/Mvc/Mvc.Core/src/Resources.resx @@ -501,4 +501,7 @@ Model binding system exceeded {0}.{1} ({2}). Reduce the potential nesting of '{3}'. For example, this type may have a property with a model binder that always succeeds. See the {0}.{1} documentation for more information. {0} is MvcOptions, {1} is MaxModelBindingRecursionDepth, {2} is option value, {3} is (loopy or deeply nested) top-level model type. - \ No newline at end of file + + Property '{0}.{1}' must be an instance of type '{2}'. + + diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/JsonResultExecutorTestBase.cs similarity index 66% rename from src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs rename to src/Mvc/Mvc.Core/test/Infrastructure/JsonResultExecutorTestBase.cs index 72e030ba6c..44092a4761 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs +++ b/src/Mvc/Mvc.Core/test/Infrastructure/JsonResultExecutorTestBase.cs @@ -2,11 +2,10 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; -using System.Collections.Generic; using System.IO; using System.Linq; using System.Text; +using System.Text.Json.Serialization; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -15,21 +14,20 @@ using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Extensions.Options; +using Microsoft.Extensions.Logging.Testing; using Microsoft.Net.Http.Headers; using Moq; -using Newtonsoft.Json; using Xunit; -namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson +namespace Microsoft.AspNetCore.Mvc.Infrastructure { - public class JsonResultExecutorTest + public abstract class JsonResultExecutorTestBase { [Fact] public async Task ExecuteAsync_UsesDefaultContentType_IfNoContentTypeSpecified() { // Arrange - var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + var expected = Encoding.UTF8.GetBytes(JsonSerializer.ToString(new { foo = "abcd" })); var context = GetActionContext(); @@ -49,7 +47,7 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson public async Task ExecuteAsync_NullEncoding_DoesNotSetCharsetOnContentType() { // Arrange - var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + var expected = Encoding.UTF8.GetBytes(JsonSerializer.ToString(new { foo = "abcd" })); var context = GetActionContext(); @@ -66,11 +64,55 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson Assert.Equal("text/json", context.HttpContext.Response.ContentType); } + [Fact] + public async Task ExecuteAsync_UsesEncodingSpecifiedInContentType() + { + // Arrange + var expected = Encoding.Unicode.GetBytes(JsonSerializer.ToString(new { foo = "abcd" })); + + var context = GetActionContext(); + context.HttpContext.Response.ContentType = "text/json; charset=utf-8"; + + var result = new JsonResult(new { foo = "abcd" }) + { + ContentType = "text/json; charset=utf-16", + }; + var executor = CreateExecutor(); + + // Act + await executor.ExecuteAsync(context, result); + + // Assert + var written = GetWrittenBytes(context.HttpContext); + Assert.Equal(expected, written); + Assert.Equal("text/json; charset=utf-16", context.HttpContext.Response.ContentType); + } + + [Fact] + public async Task ExecuteAsync_UsesEncodingSpecifiedInResponseContentType() + { + // Arrange + var expected = Encoding.Unicode.GetBytes(JsonSerializer.ToString(new { foo = "abcd" })); + + var context = GetActionContext(); + context.HttpContext.Response.ContentType = "text/json; charset=utf-16"; + var result = new JsonResult(new { foo = "abcd" }); + var executor = CreateExecutor(); + + // Act + await executor.ExecuteAsync(context, result); + + // Assert + var written = GetWrittenBytes(context.HttpContext); + Assert.Equal(expected, written); + Assert.Equal("text/json; charset=utf-16", context.HttpContext.Response.ContentType); + } + [Fact] public async Task ExecuteAsync_SetsContentTypeAndEncoding() { // Arrange - var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + var expected = Encoding.UTF8.GetBytes(JsonSerializer.ToString(new { foo = "abcd" })); var context = GetActionContext(); @@ -94,7 +136,7 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson public async Task ExecuteAsync_NoResultContentTypeSet_UsesResponseContentType_AndSuppliedEncoding() { // Arrange - var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + var expected = Encoding.UTF8.GetBytes(JsonSerializer.ToString(new { foo = "abcd" })); var expectedContentType = "text/foo; p1=p1-value; charset=us-ascii"; var context = GetActionContext(); @@ -120,7 +162,7 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson string expectedContentType) { // Arrange - var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { foo = "abcd" })); + var expected = Encoding.UTF8.GetBytes(JsonSerializer.ToString(new { foo = "abcd" })); var context = GetActionContext(); context.HttpContext.Response.ContentType = responseContentType; @@ -141,14 +183,13 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson public async Task ExecuteAsync_UsesPassedInSerializerSettings() { // Arrange - var expected = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject( + var expected = Encoding.UTF8.GetBytes(JsonSerializer.ToString( new { foo = "abcd" }, - Formatting.Indented)); + new JsonSerializerOptions { WriteIndented = true })); var context = GetActionContext(); - var serializerSettings = new JsonSerializerSettings(); - serializerSettings.Formatting = Formatting.Indented; + var serializerSettings = GetIndentedSettings(); var result = new JsonResult(new { foo = "abcd" }, serializerSettings); var executor = CreateExecutor(); @@ -162,6 +203,8 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson Assert.Equal("application/json; charset=utf-8", context.HttpContext.Response.ContentType); } + protected abstract object GetIndentedSettings(); + [Fact] public async Task ExecuteAsync_ErrorDuringSerialization_DoesNotWriteContent() { @@ -175,7 +218,11 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson { await executor.ExecuteAsync(context, result); } - catch (JsonSerializationException serializerException) + catch (NotImplementedException ex) + { + Assert.Equal("Property Age has not been implemented", ex.Message); + } + catch (Exception serializerException) { var expectedException = Assert.IsType(serializerException.InnerException); Assert.Equal("Property Age has not been implemented", expectedException.Message); @@ -192,15 +239,16 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson // Arrange var expected = "Executing JsonResult, writing value of type 'System.String'."; var context = GetActionContext(); - var logger = new StubLogger(); - var executer = CreateExecutor(logger); + var sink = new TestSink(); + var executor = CreateExecutor(new TestLoggerFactory(sink, enabled: true)); var result = new JsonResult("result_value"); // Act - await executer.ExecuteAsync(context, result); + await executor.ExecuteAsync(context, result); // Assert - Assert.Equal(expected, logger.MostRecentMessage); + var write = Assert.Single(sink.Writes); + Assert.Equal(expected, write.State.ToString()); } [Fact] @@ -209,26 +257,28 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson // Arrange var expected = "Executing JsonResult, writing value of type 'null'."; var context = GetActionContext(); - var logger = new StubLogger(); - var executer = CreateExecutor(logger); + var sink = new TestSink(); + var executor = CreateExecutor(new TestLoggerFactory(sink, enabled: true)); var result = new JsonResult(null); // Act - await executer.ExecuteAsync(context, result); + await executor.ExecuteAsync(context, result); // Assert - Assert.Equal(expected, logger.MostRecentMessage); + var write = Assert.Single(sink.Writes); + Assert.Equal(expected, write.State.ToString()); } [Fact] public async Task ExecuteAsync_LargePayload_DoesNotPerformSynchronousWrites() { // Arrange - var model = Enumerable.Range(0, 1000).Select(p => new TestModel { Property = new string('a', 5000)}); + var model = Enumerable.Range(0, 1000).Select(p => new TestModel { Property = new string('a', 5000) }).ToArray(); - var stream = new Mock { CallBase = true }; + var stream = new Mock(); stream.Setup(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(Task.CompletedTask); + .Returns(Task.CompletedTask) + .Verifiable(); stream.SetupGet(s => s.CanWrite).Returns(true); var context = GetActionContext(); context.HttpContext.Response.Body = stream.Object; @@ -240,22 +290,30 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson await executor.ExecuteAsync(context, result); // Assert - stream.Verify(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.AtLeastOnce()); - stream.Verify(v => v.Write(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); stream.Verify(v => v.Flush(), Times.Never()); } - private static JsonResultExecutor CreateExecutor(ILogger logger = null) + [Fact] + public async Task ExecuteAsync_ThrowsIfSerializerSettingIsNotTheCorrectType() { - return new JsonResultExecutor( - new TestHttpResponseStreamWriterFactory(), - logger ?? NullLogger.Instance, - Options.Create(new MvcOptions()), - Options.Create(new MvcNewtonsoftJsonOptions()), - ArrayPool.Shared); + // Arrange + var context = GetActionContext(); + + var result = new JsonResult(new { foo = "abcd" }, new object()); + var executor = CreateExecutor(); + + // Act & Assert + var ex = await Assert.ThrowsAsync(() => executor.ExecuteAsync(context, result)); + + // Assert + Assert.StartsWith("Property 'JsonResult.SerializerSettings' must be an instance of type", ex.Message); } + protected IActionResultExecutor CreateExecutor() => CreateExecutor(NullLoggerFactory.Instance); + + protected abstract IActionResultExecutor CreateExecutor(ILoggerFactory loggerFactory); + private static HttpContext GetHttpContext() { var httpContext = new DefaultHttpContext(); @@ -292,20 +350,6 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson } } - private class StubLogger : ILogger - { - public string MostRecentMessage { get; private set; } - - public IDisposable BeginScope(TState state) => throw new NotImplementedException(); - - public bool IsEnabled(LogLevel logLevel) => true; - - public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) - { - MostRecentMessage = formatter(state, exception); - } - } - private class TestModel { public string Property { get; set; } diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/SystemTextJsonResultExecutorTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/SystemTextJsonResultExecutorTest.cs new file mode 100644 index 0000000000..3467dc72a6 --- /dev/null +++ b/src/Mvc/Mvc.Core/test/Infrastructure/SystemTextJsonResultExecutorTest.cs @@ -0,0 +1,22 @@ +// 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.Text.Json.Serialization; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + public class SystemTextJsonResultExecutorTest : JsonResultExecutorTestBase + { + protected override IActionResultExecutor CreateExecutor(ILoggerFactory loggerFactory) + { + return new SystemTextJsonResultExecutor(Options.Create(new MvcOptions()), loggerFactory.CreateLogger()); + } + + protected override object GetIndentedSettings() + { + return new JsonSerializerOptions { WriteIndented = true }; + } + } +} diff --git a/src/Mvc/Mvc.Core/test/JsonResultTest.cs b/src/Mvc/Mvc.Core/test/JsonResultTest.cs deleted file mode 100644 index a89c95e919..0000000000 --- a/src/Mvc/Mvc.Core/test/JsonResultTest.cs +++ /dev/null @@ -1,32 +0,0 @@ -// 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.Threading.Tasks; -using Microsoft.AspNetCore.Http; -using Moq; -using Xunit; - -namespace Microsoft.AspNetCore.Mvc.Core -{ - public class JsonResultTest - { - [Fact] - public async Task ExecuteResultAsync_ThrowsIfExecutorIsNotAvailableInServices() - { - // Arrange - var jsonResult = new JsonResult("Hello"); - var message = "'JsonResult.ExecuteResultAsync' requires a reference to 'Microsoft.AspNetCore.Mvc.NewtonsoftJson'. " + - "Configure your application by adding a reference to the 'Microsoft.AspNetCore.Mvc.NewtonsoftJson' package and calling 'IMvcBuilder.AddNewtonsoftJson' " + - "inside the call to 'ConfigureServices(...)' in the application startup code."; - var actionContext = new ActionContext - { - HttpContext = new DefaultHttpContext { RequestServices = Mock.Of() } - }; - - // Act & Assert - var ex = await Assert.ThrowsAsync(() => jsonResult.ExecuteResultAsync(actionContext)); - Assert.Equal(message, ex.Message); - } - } -} diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs index d39ce087cd..617307004c 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs @@ -2,11 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; using System.Linq; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ApiExplorer; -using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Mvc.Rendering; @@ -73,14 +71,22 @@ namespace Microsoft.Extensions.DependencyInjection ServiceDescriptor.Transient, NewtonsoftJsonMvcOptionsSetup>()); services.TryAddEnumerable( ServiceDescriptor.Transient()); - services.TryAddSingleton, JsonResultExecutor>(); + + + var jsonResultExecutor = services.FirstOrDefault(f => + f.ServiceType == typeof(IActionResultExecutor) && + f.ImplementationType?.Assembly == typeof(JsonResult).Assembly); + + if (jsonResultExecutor != null) + { + services.Remove(jsonResultExecutor); + } + services.TryAddSingleton, NewtonsoftJsonResultExecutor>(); var viewFeaturesAssembly = typeof(IHtmlHelper).Assembly; - var tempDataSerializer = services.FirstOrDefault(f => f.ServiceType == typeof(TempDataSerializer) && - f.ImplementationType?.Assembly == viewFeaturesAssembly && - f.ImplementationType.FullName == "Microsoft.AspNetCore.Mvc.ViewFeatures.Infrastructure.DefaultTempDataSerializer"); + f.ImplementationType?.Assembly == viewFeaturesAssembly); if (tempDataSerializer != null) { @@ -94,8 +100,7 @@ namespace Microsoft.Extensions.DependencyInjection // var jsonHelper = services.FirstOrDefault( f => f.ServiceType == typeof(IJsonHelper) && - f.ImplementationType?.Assembly == viewFeaturesAssembly && - f.ImplementationType.FullName == "Microsoft.AspNetCore.Mvc.Rendering.DefaultJsonHelper"); + f.ImplementationType?.Assembly == viewFeaturesAssembly); if (jsonHelper != null) { services.Remove(jsonHelper); diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonResultExecutor.cs similarity index 94% rename from src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs rename to src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonResultExecutor.cs index 9bff0e56df..1dc0b26fab 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonResultExecutor.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson /// /// Executes a to write to the response. /// - internal class JsonResultExecutor : IActionResultExecutor + internal class NewtonsoftJsonResultExecutor : IActionResultExecutor { private static readonly string DefaultContentType = new MediaTypeHeaderValue("application/json") { @@ -34,16 +34,16 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson private readonly IArrayPool _charPool; /// - /// Creates a new . + /// Creates a new . /// /// The . - /// The . + /// The . /// Accessor to . /// Accessor to . /// The for creating buffers. - public JsonResultExecutor( + public NewtonsoftJsonResultExecutor( IHttpResponseStreamWriterFactory writerFactory, - ILogger logger, + ILogger logger, IOptions mvcOptions, IOptions jsonOptions, ArrayPool charPool) diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensionsTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensionsTest.cs index 84635d9ca6..a1bb5cf55f 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensionsTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensionsTest.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.ViewFeatures.Infrastructure; @@ -59,5 +60,20 @@ namespace Microsoft.Extensions.DependencyInjection var tempDataSerializer = Assert.Single(services, d => d.ServiceType == typeof(TempDataSerializer)); Assert.Same(typeof(BsonTempDataSerializer), tempDataSerializer.ImplementationType); } + + [Fact] + public void AddServicesCore_ReplacesDefaultJsonResultExecutor() + { + // Arrange + var services = new ServiceCollection() + .AddSingleton, SystemTextJsonResultExecutor>(); + + // Act + NewtonsoftJsonMvcCoreBuilderExtensions.AddServicesCore(services); + + // Assert + var jsonResultExecutor = Assert.Single(services, d => d.ServiceType == typeof(IActionResultExecutor)); + Assert.Same(typeof(NewtonsoftJsonResultExecutor), jsonResultExecutor.ImplementationType); + } } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs index bc1c0869ee..7e8310ae06 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs @@ -44,9 +44,9 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson var httpContext = new DefaultHttpContext(); httpContext.Response.Body = new MemoryStream(); - var executor = new JsonResultExecutor( + var executor = new NewtonsoftJsonResultExecutor( new TestHttpResponseStreamWriterFactory(), - NullLogger.Instance, + NullLogger.Instance, Options.Create(new MvcOptions()), Options.Create(new MvcNewtonsoftJsonOptions()), ArrayPool.Shared); @@ -69,4 +69,4 @@ namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson return Assert.IsType(context.Response.Body).ToArray(); } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/Microsoft.AspNetCore.Mvc.NewtonsoftJson.Test.csproj b/src/Mvc/Mvc.NewtonsoftJson/test/Microsoft.AspNetCore.Mvc.NewtonsoftJson.Test.csproj index c843466c52..b97b8afbe3 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/Microsoft.AspNetCore.Mvc.NewtonsoftJson.Test.csproj +++ b/src/Mvc/Mvc.NewtonsoftJson/test/Microsoft.AspNetCore.Mvc.NewtonsoftJson.Test.csproj @@ -8,11 +8,13 @@ + + diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonResultExecutorTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonResultExecutorTest.cs new file mode 100644 index 0000000000..c2aab7fcff --- /dev/null +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonResultExecutorTest.cs @@ -0,0 +1,29 @@ +// 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.Buffers; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Newtonsoft.Json; + +namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson +{ + public class NewtonsoftJsonResultExecutorTest : JsonResultExecutorTestBase + { + protected override IActionResultExecutor CreateExecutor(ILoggerFactory loggerFactory) + { + return new NewtonsoftJsonResultExecutor( + new TestHttpResponseStreamWriterFactory(), + loggerFactory.CreateLogger< NewtonsoftJsonResultExecutor>(), + Options.Create(new MvcOptions()), + Options.Create(new MvcNewtonsoftJsonOptions()), + ArrayPool.Shared); + } + + protected override object GetIndentedSettings() + { + return new JsonSerializerSettings { Formatting = Formatting.Indented }; + } + } +}