From 8e5e4ad641a400ef03f65cfc86e74921962e1d52 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 7 Jun 2019 15:36:37 -0700 Subject: [PATCH] Eagerly read IAsyncEnumerable{object} instances before formatting Fixes https://github.com/aspnet/AspNetCore/issues/4833 --- ...osoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs | 3 + .../Infrastructure/ObjectResultExecutor.cs | 99 +++++++++++++- src/Mvc/Mvc.Core/src/MvcOptions.cs | 13 ++ .../src/Properties/Resources.Designer.cs | 14 ++ src/Mvc/Mvc.Core/src/Resources.resx | 7 +- .../test/AcceptedAtActionResultTests.cs | 3 +- .../test/AcceptedAtRouteResultTests.cs | 3 +- src/Mvc/Mvc.Core/test/AcceptedResultTests.cs | 3 +- .../test/CreatedAtActionResultTests.cs | 3 +- .../test/CreatedAtRouteResultTests.cs | 3 +- src/Mvc/Mvc.Core/test/CreatedResultTests.cs | 3 +- .../test/HttpNotFoundObjectResultTest.cs | 3 +- .../Mvc.Core/test/HttpOkObjectResultTest.cs | 3 +- .../ControllerActionInvokerTest.cs | 3 +- .../ObjectResultExecutorTest.cs | 124 ++++++++++++++++-- src/Mvc/Mvc.Core/test/ObjectResultTests.cs | 3 +- .../AsyncEnumerableTestBase.cs | 97 ++++++++++++++ .../TestJsonSerializationOptionsProvider.cs | 15 +++ .../Controllers/AsyncEnumerableController.cs | 48 +++++++ .../test/WebSites/FormatterWebSite/Startup.cs | 2 + 20 files changed, 430 insertions(+), 22 deletions(-) create mode 100644 src/Mvc/test/Mvc.FunctionalTests/AsyncEnumerableTestBase.cs create mode 100644 src/Mvc/test/Mvc.FunctionalTests/TestJsonSerializationOptionsProvider.cs create mode 100644 src/Mvc/test/WebSites/FormatterWebSite/Controllers/AsyncEnumerableController.cs diff --git a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs index bf33019b86b0..bf70107dd6e0 100644 --- a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs +++ b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs @@ -874,6 +874,7 @@ public MvcOptions() { } public Microsoft.AspNetCore.Mvc.Filters.FilterCollection Filters { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public Microsoft.AspNetCore.Mvc.Formatters.FormatterMappings FormatterMappings { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public Microsoft.AspNetCore.Mvc.Formatters.FormatterCollection InputFormatters { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + public int MaxIAsyncEnumerableBufferLimit { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public int MaxModelBindingCollectionSize { get { throw null; } set { } } public int MaxModelBindingRecursionDepth { get { throw null; } set { } } public int MaxModelValidationErrors { get { throw null; } set { } } @@ -2084,7 +2085,9 @@ public MvcCompatibilityOptions() { } } public partial class ObjectResultExecutor : Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultExecutor { + [System.ObsoleteAttribute("This constructor is obsolete and will be removed in a future release.")] public ObjectResultExecutor(Microsoft.AspNetCore.Mvc.Infrastructure.OutputFormatterSelector formatterSelector, Microsoft.AspNetCore.Mvc.Infrastructure.IHttpResponseStreamWriterFactory writerFactory, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) { } + public ObjectResultExecutor(Microsoft.AspNetCore.Mvc.Infrastructure.OutputFormatterSelector formatterSelector, Microsoft.AspNetCore.Mvc.Infrastructure.IHttpResponseStreamWriterFactory writerFactory, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, Microsoft.Extensions.Options.IOptions mvcOptions) { } protected Microsoft.AspNetCore.Mvc.Infrastructure.OutputFormatterSelector FormatterSelector { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } protected Microsoft.Extensions.Logging.ILogger Logger { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } protected System.Func WriterFactory { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs index 6c5179d222b4..9f5c15cf40e5 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs @@ -2,14 +2,19 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Reflection; using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Formatters; +using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Infrastructure { @@ -18,16 +23,43 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure /// public class ObjectResultExecutor : IActionResultExecutor { + private delegate Task ReadAsyncEnumerableDelegate(object value); + + private readonly MethodInfo Converter = typeof(ObjectResultExecutor).GetMethod( + nameof(ReadAsyncEnumerable), + BindingFlags.NonPublic | BindingFlags.Instance); + + private readonly ConcurrentDictionary _asyncEnumerableConverters = + new ConcurrentDictionary(); + private readonly MvcOptions _mvcOptions; + /// /// Creates a new . /// /// The . /// The . /// The . + [Obsolete("This constructor is obsolete and will be removed in a future release.")] public ObjectResultExecutor( OutputFormatterSelector formatterSelector, IHttpResponseStreamWriterFactory writerFactory, ILoggerFactory loggerFactory) + : this(formatterSelector, writerFactory, loggerFactory, mvcOptions: null) + { + } + + /// + /// Creates a new . + /// + /// The . + /// The . + /// The . + /// Accessor to . + public ObjectResultExecutor( + OutputFormatterSelector formatterSelector, + IHttpResponseStreamWriterFactory writerFactory, + ILoggerFactory loggerFactory, + IOptions mvcOptions) { if (formatterSelector == null) { @@ -47,6 +79,7 @@ public ObjectResultExecutor( FormatterSelector = formatterSelector; WriterFactory = writerFactory.CreateWriter; Logger = loggerFactory.CreateLogger(); + _mvcOptions = mvcOptions?.Value ?? throw new ArgumentNullException(nameof(mvcOptions)); } /// @@ -87,16 +120,35 @@ public virtual Task ExecuteAsync(ActionContext context, ObjectResult result) InferContentTypes(context, result); var objectType = result.DeclaredType; + if (objectType == null || objectType == typeof(object)) { objectType = result.Value?.GetType(); } + var value = result.Value; + + if (value is IAsyncEnumerable asyncEnumerable) + { + return ExecuteAsyncEnumerable(context, result, asyncEnumerable); + } + + return ExecuteAsyncCore(context, result, objectType, value); + } + + private async Task ExecuteAsyncEnumerable(ActionContext context, ObjectResult result, IAsyncEnumerable asyncEnumerable) + { + var enumerated = await EnumerateAsyncEnumerable(asyncEnumerable); + await ExecuteAsyncCore(context, result, enumerated.GetType(), enumerated); + } + + private Task ExecuteAsyncCore(ActionContext context, ObjectResult result, Type objectType, object value) + { var formatterContext = new OutputFormatterWriteContext( context.HttpContext, WriterFactory, objectType, - result.Value); + value); var selectedFormatter = FormatterSelector.SelectFormatter( formatterContext, @@ -138,5 +190,50 @@ private static void InferContentTypes(ActionContext context, ObjectResult result result.ContentTypes.Add("application/problem+xml"); } } + + private Task EnumerateAsyncEnumerable(IAsyncEnumerable value) + { + var type = value.GetType(); + if (!_asyncEnumerableConverters.TryGetValue(type, out var result)) + { + var enumerableType = ClosedGenericMatcher.ExtractGenericInterface(type, typeof(IAsyncEnumerable<>)); + result = null; + if (enumerableType != null) + { + var enumeratedObjectType = enumerableType.GetGenericArguments()[0]; + + var converter = (ReadAsyncEnumerableDelegate)Converter + .MakeGenericMethod(enumeratedObjectType) + .CreateDelegate(typeof(ReadAsyncEnumerableDelegate), this); + + _asyncEnumerableConverters.TryAdd(type, converter); + result = converter; + } + } + + return result(value); + } + + private async Task ReadAsyncEnumerable(object value) + { + var asyncEnumerable = (IAsyncEnumerable)value; + var result = new List(); + var count = 0; + + await foreach (var item in asyncEnumerable) + { + if (count++ >= _mvcOptions.MaxIAsyncEnumerableBufferLimit) + { + throw new InvalidOperationException(Resources.FormatObjectResultExecutor_MaxEnumerationExceeded( + nameof(ObjectResultExecutor), + _mvcOptions.MaxIAsyncEnumerableBufferLimit, + value.GetType())); + } + + result.Add(item); + } + + return result; + } } } diff --git a/src/Mvc/Mvc.Core/src/MvcOptions.cs b/src/Mvc/Mvc.Core/src/MvcOptions.cs index 2cb5f9fa7216..46cf9c63a341 100644 --- a/src/Mvc/Mvc.Core/src/MvcOptions.cs +++ b/src/Mvc/Mvc.Core/src/MvcOptions.cs @@ -359,6 +359,19 @@ public int MaxModelBindingRecursionDepth } } + /// + /// Gets or sets the most number of entries of an that + /// that will buffer. + /// + /// When is an instance of , + /// will eagerly read the enumeration and add to a synchronous collection + /// prior to invoking the selected formatter. + /// This property determines the most number of entries that the executor is allowed to buffer. + /// + /// + /// Defaults to 8192. + public int MaxIAsyncEnumerableBufferLimit { get; set; } = 8192; + IEnumerator IEnumerable.GetEnumerator() => _switches.GetEnumerator(); IEnumerator IEnumerable.GetEnumerator() => _switches.GetEnumerator(); diff --git a/src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs b/src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs index 2878cafd4ca3..a4d64c752b05 100644 --- a/src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs +++ b/src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs @@ -1746,6 +1746,20 @@ internal static string Property_MustBeInstanceOfType internal static string FormatProperty_MustBeInstanceOfType(object p0, object p1, object p2) => string.Format(CultureInfo.CurrentCulture, GetString("Property_MustBeInstanceOfType"), p0, p1, p2); + /// + /// {0} execeeded the maximum configured enumeration limit '{1}' when enumerating async enumerable type '{2}'. + /// + internal static string ObjectResultExecutor_MaxEnumerationExceeded + { + get => GetString("ObjectResultExecutor_MaxEnumerationExceeded"); + } + + /// + /// {0} execeeded the maximum configured enumeration limit '{1}' when enumerating async enumerable type '{2}'. + /// + internal static string FormatObjectResultExecutor_MaxEnumerationExceeded(object p0, object p1, object p2) + => string.Format(CultureInfo.CurrentCulture, GetString("ObjectResultExecutor_MaxEnumerationExceeded"), 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 bbf596d558eb..3860106b63fb 100644 --- a/src/Mvc/Mvc.Core/src/Resources.resx +++ b/src/Mvc/Mvc.Core/src/Resources.resx @@ -503,5 +503,8 @@ Property '{0}.{1}' must be an instance of type '{2}'. - - + + + {0} execeeded the maximum configured enumeration limit '{1}' when enumerating async enumerable type '{2}'. + + \ No newline at end of file diff --git a/src/Mvc/Mvc.Core/test/AcceptedAtActionResultTests.cs b/src/Mvc/Mvc.Core/test/AcceptedAtActionResultTests.cs index 721b58ca9b58..ee57bdbd8e03 100644 --- a/src/Mvc/Mvc.Core/test/AcceptedAtActionResultTests.cs +++ b/src/Mvc/Mvc.Core/test/AcceptedAtActionResultTests.cs @@ -275,7 +275,8 @@ private static IServiceProvider CreateServices(Mock formatter) services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); return services.BuildServiceProvider(); } diff --git a/src/Mvc/Mvc.Core/test/AcceptedAtRouteResultTests.cs b/src/Mvc/Mvc.Core/test/AcceptedAtRouteResultTests.cs index 220013197113..edcd21ef5c52 100644 --- a/src/Mvc/Mvc.Core/test/AcceptedAtRouteResultTests.cs +++ b/src/Mvc/Mvc.Core/test/AcceptedAtRouteResultTests.cs @@ -183,7 +183,8 @@ private static IServiceProvider CreateServices(Mock formatter) services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); return services.BuildServiceProvider(); } diff --git a/src/Mvc/Mvc.Core/test/AcceptedResultTests.cs b/src/Mvc/Mvc.Core/test/AcceptedResultTests.cs index 07ca76072e64..cae823f94de6 100644 --- a/src/Mvc/Mvc.Core/test/AcceptedResultTests.cs +++ b/src/Mvc/Mvc.Core/test/AcceptedResultTests.cs @@ -139,7 +139,8 @@ private static IServiceProvider CreateServices(Mock formatter) services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); return services.BuildServiceProvider(); } diff --git a/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs b/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs index cad29a30437d..8295216ae6f1 100644 --- a/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs +++ b/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs @@ -97,7 +97,8 @@ private static IServiceProvider CreateServices() services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); return services.BuildServiceProvider(); } diff --git a/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs b/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs index 65410977a06c..e3655e36b56c 100644 --- a/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs +++ b/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs @@ -110,7 +110,8 @@ private static IServiceProvider CreateServices() services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); return services.BuildServiceProvider(); } diff --git a/src/Mvc/Mvc.Core/test/CreatedResultTests.cs b/src/Mvc/Mvc.Core/test/CreatedResultTests.cs index 6d3eefc85bde..cc9011ecedb5 100644 --- a/src/Mvc/Mvc.Core/test/CreatedResultTests.cs +++ b/src/Mvc/Mvc.Core/test/CreatedResultTests.cs @@ -98,7 +98,8 @@ private static IServiceProvider CreateServices() services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); return services.BuildServiceProvider(); } diff --git a/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs b/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs index 498a12c57ac4..00cf34431a11 100644 --- a/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs @@ -75,7 +75,8 @@ private static IServiceProvider CreateServices() services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); return services.BuildServiceProvider(); } diff --git a/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs b/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs index 13f5572c4e53..6dafb12543ab 100644 --- a/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs @@ -76,7 +76,8 @@ private static IServiceProvider CreateServices() services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); return services.BuildServiceProvider(); } diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/ControllerActionInvokerTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ControllerActionInvokerTest.cs index 70ce4124e1cf..39c0a6b25149 100644 --- a/src/Mvc/Mvc.Core/test/Infrastructure/ControllerActionInvokerTest.cs +++ b/src/Mvc/Mvc.Core/test/Infrastructure/ControllerActionInvokerTest.cs @@ -1567,7 +1567,8 @@ private ControllerActionInvoker CreateInvoker( services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); httpContext.Response.Body = new MemoryStream(); httpContext.RequestServices = services.BuildServiceProvider(); diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/ObjectResultExecutorTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ObjectResultExecutorTest.cs index 207d04161f2d..8ab2d7667848 100644 --- a/src/Mvc/Mvc.Core/test/Infrastructure/ObjectResultExecutorTest.cs +++ b/src/Mvc/Mvc.Core/test/Infrastructure/ObjectResultExecutorTest.cs @@ -2,6 +2,7 @@ // 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.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -209,8 +210,8 @@ public async Task ExecuteAsync_NoFormatterFound_Returns406() public async Task ExecuteAsync_FallsBackOnFormattersInOptions() { // Arrange - var options = Options.Create(new MvcOptions()); - options.Value.OutputFormatters.Add(new TestJsonOutputFormatter()); + var options = new MvcOptions(); + options.OutputFormatters.Add(new TestJsonOutputFormatter()); var executor = CreateExecutor(options: options); @@ -300,8 +301,8 @@ public async Task ExecuteAsync_SelectDefaultFormatter_OnAllMediaRangeAcceptHeade string expectedContentType) { // Arrange - var options = Options.Create(new MvcOptions()); - options.Value.RespectBrowserAcceptHeader = false; + var options = new MvcOptions(); + options.RespectBrowserAcceptHeader = false; var executor = CreateExecutor(options: options); @@ -337,8 +338,8 @@ public async Task ObjectResult_PerformsContentNegotiation_OnAllMediaRangeAcceptH string expectedContentType) { // Arrange - var options = Options.Create(new MvcOptions()); - options.Value.RespectBrowserAcceptHeader = true; + var options = new MvcOptions(); + options.RespectBrowserAcceptHeader = true; var executor = CreateExecutor(options: options); @@ -360,6 +361,106 @@ public async Task ObjectResult_PerformsContentNegotiation_OnAllMediaRangeAcceptH MediaTypeAssert.Equal(expectedContentType, responseContentType); } + [Fact] + public async Task ObjectResult_ReadsAsyncEnumerables() + { + // Arrange + var executor = CreateExecutor(); + var result = new ObjectResult(AsyncEnumerable()); + var formatter = new TestJsonOutputFormatter(); + result.Formatters.Add(formatter); + + var actionContext = new ActionContext() + { + HttpContext = GetHttpContext(), + }; + + // Act + await executor.ExecuteAsync(actionContext, result); + + // Assert + var formatterContext = formatter.LastOutputFormatterContext; + Assert.Equal(typeof(List), formatterContext.ObjectType); + var value = Assert.IsType>(formatterContext.Object); + Assert.Equal(new[] { "Hello 0", "Hello 1", "Hello 2", "Hello 3", }, value); + } + + [Fact] + public async Task ObjectResult_Throws_IfEnumerableThrows() + { + // Arrange + var executor = CreateExecutor(); + var result = new ObjectResult(AsyncEnumerable(throwError: true)); + var formatter = new TestJsonOutputFormatter(); + result.Formatters.Add(formatter); + + var actionContext = new ActionContext() + { + HttpContext = GetHttpContext(), + }; + + // Act & Assert + await Assert.ThrowsAsync(() => executor.ExecuteAsync(actionContext, result)); + } + + [Fact] + public async Task ObjectResult_AsyncEnumeration_AtLimit() + { + // Arrange + var count = 24; + var executor = CreateExecutor(options: new MvcOptions { MaxIAsyncEnumerableBufferLimit = count }); + var result = new ObjectResult(AsyncEnumerable(count: count)); + var formatter = new TestJsonOutputFormatter(); + result.Formatters.Add(formatter); + + var actionContext = new ActionContext() + { + HttpContext = GetHttpContext(), + }; + + // Act + await executor.ExecuteAsync(actionContext, result); + + // Assert + var formatterContext = formatter.LastOutputFormatterContext; + var value = Assert.IsType>(formatterContext.Object); + Assert.Equal(24, value.Count); + } + + [Theory] + [InlineData(25)] + [InlineData(1024)] + public async Task ObjectResult_Throws_IfEnumerationExceedsLimit(int count) + { + // Arrange + var executor = CreateExecutor(options: new MvcOptions { MaxIAsyncEnumerableBufferLimit = 24 }); + var result = new ObjectResult(AsyncEnumerable(count: count)); + var formatter = new TestJsonOutputFormatter(); + result.Formatters.Add(formatter); + + var actionContext = new ActionContext() + { + HttpContext = GetHttpContext(), + }; + + // Act & Assert + var ex = await Assert.ThrowsAsync(() => executor.ExecuteAsync(actionContext, result)); + } + + private static async IAsyncEnumerable AsyncEnumerable(int count = 4, bool throwError = false) + { + await Task.Yield(); + for (var i = 0; i < count; i++) + { + yield return $"Hello {i}"; + } + + if (throwError) + { + throw new TimeZoneNotFoundException(); + } + } + private static IServiceCollection CreateServices() { var services = new ServiceCollection(); @@ -379,10 +480,12 @@ private static HttpContext GetHttpContext() return httpContext; } - private static ObjectResultExecutor CreateExecutor(IOptions options = null) + private static ObjectResultExecutor CreateExecutor(MvcOptions options = null) { - var selector = new DefaultOutputFormatterSelector(options ?? Options.Create(new MvcOptions()), NullLoggerFactory.Instance); - return new ObjectResultExecutor(selector, new TestHttpResponseStreamWriterFactory(), NullLoggerFactory.Instance); + options ??= new MvcOptions(); + var optionsAccessor = Options.Create(options); + var selector = new DefaultOutputFormatterSelector(optionsAccessor, NullLoggerFactory.Instance); + return new ObjectResultExecutor(selector, new TestHttpResponseStreamWriterFactory(), NullLoggerFactory.Instance, optionsAccessor); } private class CannotWriteFormatter : IOutputFormatter @@ -409,8 +512,11 @@ public TestJsonOutputFormatter() SupportedEncodings.Add(Encoding.UTF8); } + public OutputFormatterWriteContext LastOutputFormatterContext { get; private set; } + public override Task WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding selectedEncoding) { + LastOutputFormatterContext = context; return Task.FromResult(0); } } diff --git a/src/Mvc/Mvc.Core/test/ObjectResultTests.cs b/src/Mvc/Mvc.Core/test/ObjectResultTests.cs index 7b818ad97b65..b1a1eb81c7fe 100644 --- a/src/Mvc/Mvc.Core/test/ObjectResultTests.cs +++ b/src/Mvc/Mvc.Core/test/ObjectResultTests.cs @@ -100,7 +100,8 @@ private static IServiceProvider CreateServices() services.AddSingleton>(new ObjectResultExecutor( new DefaultOutputFormatterSelector(Options.Create(new MvcOptions()), NullLoggerFactory.Instance), new TestHttpResponseStreamWriterFactory(), - NullLoggerFactory.Instance)); + NullLoggerFactory.Instance, + Options.Create(new MvcOptions()))); services.AddSingleton(NullLoggerFactory.Instance); return services.BuildServiceProvider(); diff --git a/src/Mvc/test/Mvc.FunctionalTests/AsyncEnumerableTestBase.cs b/src/Mvc/test/Mvc.FunctionalTests/AsyncEnumerableTestBase.cs new file mode 100644 index 000000000000..75160338f05a --- /dev/null +++ b/src/Mvc/test/Mvc.FunctionalTests/AsyncEnumerableTestBase.cs @@ -0,0 +1,97 @@ +// 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.Linq; +using System.Net; +using System.Net.Http; +using System.Text.Json; +using System.Threading.Tasks; +using System.Xml.Linq; +using FormatterWebSite; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests +{ + public class AsyncEnumerableTestBase : IClassFixture> + { + public AsyncEnumerableTestBase(MvcTestFixture fixture) + { + Client = fixture.CreateDefaultClient(); + } + + public HttpClient Client { get; } + + [Fact] + public Task AsyncEnumerableReturnedWorks() => AsyncEnumerableWorks(); + + [Fact] + public Task AsyncEnumerableWrappedInTask() => AsyncEnumerableWorks(); + + private async Task AsyncEnumerableWorks() + { + // Act + var response = await Client.GetAsync("asyncenumerable/getallprojects"); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + + var content = await response.Content.ReadAsStringAsync(); + + // Some sanity tests to verify things serialized correctly. + var projects = JsonSerializer.Parse>(content, TestJsonSerializerOptionsProvider.Options); + Assert.Equal(10, projects.Count); + Assert.Equal("Project0", projects[0].Name); + Assert.Equal("Project9", projects[9].Name); + } + + [Fact] + public async Task AsyncEnumerableExceptionsAreThrown() + { + // Act + var response = await Client.GetAsync("asyncenumerable/GetAllProjectsWithError"); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.InternalServerError); + + var content = await response.Content.ReadAsStringAsync(); + + // Verify that the exception shows up in the callstack + Assert.Contains(nameof(InvalidTimeZoneException), content); + } + + [Fact] + public async Task AsyncEnumerableWithXmlFormatterWorks() + { + // Arrange + var request = new HttpRequestMessage(HttpMethod.Get, "asyncenumerable/getallprojects"); + request.Headers.Add("Accept", "application/xml"); + + // Act + var response = await Client.SendAsync(request); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + + var content = await response.Content.ReadAsStringAsync(); + + // Some sanity tests to verify things serialized correctly. + var xml = XDocument.Parse(content); + var @namespace = xml.Root.Name.NamespaceName; + var projects = xml.Root.Elements(XName.Get("Project", @namespace)); + Assert.Equal(10, projects.Count()); + + Assert.Equal("Project0", GetName(projects.ElementAt(0))); + Assert.Equal("Project9", GetName(projects.ElementAt(9))); + + string GetName(XElement element) + { + var name = element.Element(XName.Get("Name", @namespace)); + Assert.NotNull(name); + + return name.Value; + } + } + } +} diff --git a/src/Mvc/test/Mvc.FunctionalTests/TestJsonSerializationOptionsProvider.cs b/src/Mvc/test/Mvc.FunctionalTests/TestJsonSerializationOptionsProvider.cs new file mode 100644 index 000000000000..4a418cb0f6f8 --- /dev/null +++ b/src/Mvc/test/Mvc.FunctionalTests/TestJsonSerializationOptionsProvider.cs @@ -0,0 +1,15 @@ +// 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; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests +{ + internal static class TestJsonSerializerOptionsProvider + { + public static JsonSerializerOptions Options = new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }; + } +} diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/AsyncEnumerableController.cs b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/AsyncEnumerableController.cs new file mode 100644 index 000000000000..864e385c7638 --- /dev/null +++ b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/AsyncEnumerableController.cs @@ -0,0 +1,48 @@ +// 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.Threading.Tasks; +using Microsoft.AspNetCore.Mvc; + +namespace FormatterWebSite.Controllers +{ + [ApiController] + [Route("{controller}/{action}")] + public class AsyncEnumerableController : ControllerBase + { + [HttpGet] + public IAsyncEnumerable GetAllProjects() + => GetAllProjectsCore(); + + [HttpGet] + public async Task> GetAllProjectsAsTask() + { + await Task.Yield(); + return GetAllProjectsCore(); + } + + [HttpGet] + public IAsyncEnumerable GetAllProjectsWithError() + => GetAllProjectsCore(true); + + public async IAsyncEnumerable GetAllProjectsCore(bool throwError = false) + { + await Task.Delay(5); + for (var i = 0; i < 10; i++) + { + if (throwError && i == 5) + { + throw new InvalidTimeZoneException(); + } + + yield return new Project + { + Id = i, + Name = $"Project{i}", + }; + } + } + } +} diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Startup.cs b/src/Mvc/test/WebSites/FormatterWebSite/Startup.cs index ba036d9a081e..ac3d04966f8f 100644 --- a/src/Mvc/test/WebSites/FormatterWebSite/Startup.cs +++ b/src/Mvc/test/WebSites/FormatterWebSite/Startup.cs @@ -26,6 +26,8 @@ public void ConfigureServices(IServiceCollection services) public void Configure(IApplicationBuilder app) { + app.UseDeveloperExceptionPage(); + app.UseRouting(); app.UseEndpoints(endpoints => {