Skip to content

Commit

Permalink
Eagerly read IAsyncEnumerable{object} instances before formatting
Browse files Browse the repository at this point in the history
Fixes #4833
  • Loading branch information
pranavkm committed Jun 11, 2019
1 parent f8bd011 commit 8e5e4ad
Show file tree
Hide file tree
Showing 20 changed files with 430 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<Microsoft.AspNetCore.Mvc.Formatters.IInputFormatter> 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 { } }
Expand Down Expand Up @@ -2084,7 +2085,9 @@ public MvcCompatibilityOptions() { }
}
public partial class ObjectResultExecutor : Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultExecutor<Microsoft.AspNetCore.Mvc.ObjectResult>
{
[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<Microsoft.AspNetCore.Mvc.MvcOptions> 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<System.IO.Stream, System.Text.Encoding, System.IO.TextWriter> WriterFactory { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
Expand Down
99 changes: 98 additions & 1 deletion src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -18,16 +23,43 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
/// </summary>
public class ObjectResultExecutor : IActionResultExecutor<ObjectResult>
{
private delegate Task<object> ReadAsyncEnumerableDelegate(object value);

private readonly MethodInfo Converter = typeof(ObjectResultExecutor).GetMethod(
nameof(ReadAsyncEnumerable),
BindingFlags.NonPublic | BindingFlags.Instance);

private readonly ConcurrentDictionary<Type, ReadAsyncEnumerableDelegate> _asyncEnumerableConverters =
new ConcurrentDictionary<Type, ReadAsyncEnumerableDelegate>();
private readonly MvcOptions _mvcOptions;

/// <summary>
/// Creates a new <see cref="ObjectResultExecutor"/>.
/// </summary>
/// <param name="formatterSelector">The <see cref="OutputFormatterSelector"/>.</param>
/// <param name="writerFactory">The <see cref="IHttpResponseStreamWriterFactory"/>.</param>
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
[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)
{
}

/// <summary>
/// Creates a new <see cref="ObjectResultExecutor"/>.
/// </summary>
/// <param name="formatterSelector">The <see cref="OutputFormatterSelector"/>.</param>
/// <param name="writerFactory">The <see cref="IHttpResponseStreamWriterFactory"/>.</param>
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
/// <param name="mvcOptions">Accessor to <see cref="MvcOptions"/>.</param>
public ObjectResultExecutor(
OutputFormatterSelector formatterSelector,
IHttpResponseStreamWriterFactory writerFactory,
ILoggerFactory loggerFactory,
IOptions<MvcOptions> mvcOptions)
{
if (formatterSelector == null)
{
Expand All @@ -47,6 +79,7 @@ public ObjectResultExecutor(
FormatterSelector = formatterSelector;
WriterFactory = writerFactory.CreateWriter;
Logger = loggerFactory.CreateLogger<ObjectResultExecutor>();
_mvcOptions = mvcOptions?.Value ?? throw new ArgumentNullException(nameof(mvcOptions));
}

/// <summary>
Expand Down Expand Up @@ -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<object> asyncEnumerable)
{
return ExecuteAsyncEnumerable(context, result, asyncEnumerable);
}

return ExecuteAsyncCore(context, result, objectType, value);
}

private async Task ExecuteAsyncEnumerable(ActionContext context, ObjectResult result, IAsyncEnumerable<object> 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,
Expand Down Expand Up @@ -138,5 +190,50 @@ private static void InferContentTypes(ActionContext context, ObjectResult result
result.ContentTypes.Add("application/problem+xml");
}
}

private Task<object> EnumerateAsyncEnumerable(IAsyncEnumerable<object> 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<object> ReadAsyncEnumerable<T>(object value)
{
var asyncEnumerable = (IAsyncEnumerable<T>)value;
var result = new List<T>();
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;
}
}
}
13 changes: 13 additions & 0 deletions src/Mvc/Mvc.Core/src/MvcOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,19 @@ public int MaxModelBindingRecursionDepth
}
}

/// <summary>
/// Gets or sets the most number of entries of an <see cref="IAsyncEnumerable{T}"/> that
/// that <see cref="ObjectResultExecutor"/> will buffer.
/// <para>
/// When <see cref="ObjectResult.Value" /> is an instance of <see cref="IAsyncEnumerable{T}"/>,
/// <see cref="ObjectResultExecutor"/> 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.
/// </para>
/// </summary>
/// <value>Defaults to <c>8192</c>.</value>
public int MaxIAsyncEnumerableBufferLimit { get; set; } = 8192;

IEnumerator<ICompatibilitySwitch> IEnumerable<ICompatibilitySwitch>.GetEnumerator() => _switches.GetEnumerator();

IEnumerator IEnumerable.GetEnumerator() => _switches.GetEnumerator();
Expand Down
14 changes: 14 additions & 0 deletions src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions src/Mvc/Mvc.Core/src/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -503,5 +503,8 @@
</data>
<data name="Property_MustBeInstanceOfType" xml:space="preserve">
<value>Property '{0}.{1}' must be an instance of type '{2}'.</value>
</data>
</root>
</data>
<data name="ObjectResultExecutor_MaxEnumerationExceeded" xml:space="preserve">
<value>{0} execeeded the maximum configured enumeration limit '{1}' when enumerating async enumerable type '{2}'.</value>
</data>
</root>
3 changes: 2 additions & 1 deletion src/Mvc/Mvc.Core/test/AcceptedAtActionResultTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ private static IServiceProvider CreateServices(Mock<IOutputFormatter> formatter)
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
new TestHttpResponseStreamWriterFactory(),
NullLoggerFactory.Instance));
NullLoggerFactory.Instance,
Options.Create(new MvcOptions())));

return services.BuildServiceProvider();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Mvc/Mvc.Core/test/AcceptedAtRouteResultTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ private static IServiceProvider CreateServices(Mock<IOutputFormatter> formatter)
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
new TestHttpResponseStreamWriterFactory(),
NullLoggerFactory.Instance));
NullLoggerFactory.Instance,
Options.Create(new MvcOptions())));

return services.BuildServiceProvider();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Mvc/Mvc.Core/test/AcceptedResultTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ private static IServiceProvider CreateServices(Mock<IOutputFormatter> formatter)
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
new TestHttpResponseStreamWriterFactory(),
NullLoggerFactory.Instance));
NullLoggerFactory.Instance,
Options.Create(new MvcOptions())));

return services.BuildServiceProvider();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ private static IServiceProvider CreateServices()
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
new TestHttpResponseStreamWriterFactory(),
NullLoggerFactory.Instance));
NullLoggerFactory.Instance,
Options.Create(new MvcOptions())));

return services.BuildServiceProvider();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ private static IServiceProvider CreateServices()
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
new TestHttpResponseStreamWriterFactory(),
NullLoggerFactory.Instance));
NullLoggerFactory.Instance,
Options.Create(new MvcOptions())));

return services.BuildServiceProvider();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Mvc/Mvc.Core/test/CreatedResultTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ private static IServiceProvider CreateServices()
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
new TestHttpResponseStreamWriterFactory(),
NullLoggerFactory.Instance));
NullLoggerFactory.Instance,
Options.Create(new MvcOptions())));

return services.BuildServiceProvider();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ private static IServiceProvider CreateServices()
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
new TestHttpResponseStreamWriterFactory(),
NullLoggerFactory.Instance));
NullLoggerFactory.Instance,
Options.Create(new MvcOptions())));

return services.BuildServiceProvider();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ private static IServiceProvider CreateServices()
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
new TestHttpResponseStreamWriterFactory(),
NullLoggerFactory.Instance));
NullLoggerFactory.Instance,
Options.Create(new MvcOptions())));

return services.BuildServiceProvider();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1567,7 +1567,8 @@ private ControllerActionInvoker CreateInvoker(
services.AddSingleton<IActionResultExecutor<ObjectResult>>(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();
Expand Down
Loading

0 comments on commit 8e5e4ad

Please sign in to comment.