Eagerly read IAsyncEnumerable{object} instances before formatting #11118
Conversation
analogrelay
added
the
area-mvc
pranavkm
force-pushed
the
prkrishn/asyncenumerable
branch
from
8e5e4ad to
b6a3036
Compare
| var type = value.GetType(); | ||
| if (!_asyncEnumerableConverters.TryGetValue(type, out var result)) | ||
| { | ||
| var enumerableType = ClosedGenericMatcher.ExtractGenericInterface(type, typeof(IAsyncEnumerable<>)); |
There was a problem hiding this comment.
What happens if you implement multiple IAsyncEnumerable<> interfaces? implementation-defined? Crash with a bad error?
|
|
||
| var converter = (ReadAsyncEnumerableDelegate)Converter | ||
| .MakeGenericMethod(enumeratedObjectType) | ||
| .CreateDelegate(typeof(ReadAsyncEnumerableDelegate), this); |
pranavkm
force-pushed
the
prkrishn/asyncenumerable
branch
2 times, most recently
from
b1f2a6b to
c2469ac
Compare
|
🆙 📅 |
| /// an <see cref="IAsyncEnumerable{T}"/>. An accurate <see cref="ICollection{T}"/> is required for XML formatters to | ||
| /// correctly serialize. | ||
| /// </remarks> | ||
| public sealed class AsyncEnumerableReader |
There was a problem hiding this comment.
Does this really need to be public? I don't have a strong objection, but it seems like infrastructure, rather than something we expect users to need.
There was a problem hiding this comment.
I could defer reading this from DI or have duplicated caching if your app uses a mix of ObjectResult and JsonResult returning IAsyncEnumerable. I guess the latter is rare and easily fixable if it turns out to be a problem in the future.
There was a problem hiding this comment.
I don't understand your answer. What's the reason why this type is public?
There was a problem hiding this comment.
ObjectResultExecutor is public. If we have to share the same singleton instance between the JsonResultExecutor(s) and ObjectResultExecutor, we either have to make the type public for ctor DI or read it in the body of ExecuteAsync.
| if (!_asyncEnumerableConverters.TryGetValue(type, out var result)) | ||
| { | ||
| var enumerableType = ClosedGenericMatcher.ExtractGenericInterface(type, typeof(IAsyncEnumerable<>)); | ||
| Debug.Assert(enumerableType != null); |
There was a problem hiding this comment.
This is a public API, so this should probably throw instead of assert. Someone could pass in any type here.
| private async Task ExecuteAsyncEnumerable(ActionContext context, ObjectResult result, IAsyncEnumerable<object> asyncEnumerable) | ||
| { | ||
| var enumerated = await _asyncEnumerableReader.ReadAsync(asyncEnumerable); | ||
| await ExecuteAsyncCore(context, result, enumerated.GetType(), enumerated); |
src/Mvc/Mvc.Core/src/Infrastructure/SystemTextJsonResultExecutor.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.Core/src/Infrastructure/SystemTextJsonResultExecutor.cs
Outdated
Show resolved
Hide resolved
pranavkm
force-pushed
the
prkrishn/asyncenumerable
branch
from
c2469ac to
ca281db
Compare
ghost
deleted the
Fixes #4833