Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop Calling JSON GetTypeInfo with the runtime type #47859

Merged
merged 6 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,18 @@ private static Func<HttpContext, StringValues> ResolveFromRouteOrQuery(string pa
""";

public static string WriteToResponseAsyncMethod => """
private static Task WriteToResponseAsync<T>(T? value, HttpContext httpContext, JsonTypeInfo<T> jsonTypeInfo, JsonSerializerOptions options)
[UnconditionalSuppressMessage("Trimming", "IL2026:RequiresUnreferencedCode",
Justification = "The 'JsonSerializer.IsReflectionEnabledByDefault' feature switch, which is set to false by default for trimmed ASP.NET apps, ensures the JsonSerializer doesn't use Reflection.")]
[UnconditionalSuppressMessage("AOT", "IL3050:RequiresDynamicCode", Justification = "See above.")]
private static Task WriteToResponseAsync<T>(T? value, HttpContext httpContext, JsonTypeInfo<T> jsonTypeInfo)
{
var runtimeType = value?.GetType();
if (runtimeType is null || jsonTypeInfo.Type == runtimeType || jsonTypeInfo.PolymorphismOptions is not null)
{
return httpContext.Response.WriteAsJsonAsync(value!, jsonTypeInfo);
}

return httpContext.Response.WriteAsJsonAsync(value!, options.GetTypeInfo(runtimeType));
return httpContext.Response.WriteAsJsonAsync<object?>(value, jsonTypeInfo.Options);
}
""";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ internal static string EmitJsonResponse(this EndpointResponse endpointResponse)
{
return $"httpContext.Response.WriteAsJsonAsync(result, jsonTypeInfo);";
}
return $"GeneratedRouteBuilderExtensionsCore.WriteToResponseAsync(result, httpContext, jsonTypeInfo, serializerOptions);";
return $"GeneratedRouteBuilderExtensionsCore.WriteToResponseAsync(result, httpContext, jsonTypeInfo);";
}
}
14 changes: 6 additions & 8 deletions src/Http/Http.Extensions/src/HttpResponseJsonExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@ public static partial class HttpResponseJsonExtensions
/// <param name="value">The value to write as JSON.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken"/> used to cancel the operation.</param>
/// <returns>The task object representing the asynchronous operation.</returns>
[RequiresUnreferencedCode(RequiresUnreferencedCodeMessage)]
[RequiresDynamicCode(RequiresDynamicCodeMessage)]
public static Task WriteAsJsonAsync<TValue>(
this HttpResponse response,
TValue value,
CancellationToken cancellationToken = default)
{
ArgumentNullException.ThrowIfNull(response);

var options = ResolveSerializerOptions(response.HttpContext);
return response.WriteAsJsonAsync(value, jsonTypeInfo: (JsonTypeInfo<TValue>)options.GetTypeInfo(typeof(TValue)), contentType: null, cancellationToken);
return response.WriteAsJsonAsync(value, options: null, contentType: null, cancellationToken);
}

/// <summary>
Expand Down Expand Up @@ -108,9 +109,7 @@ public static Task WriteAsJsonAsync<TValue>(
/// <param name="contentType">The content-type to set on the response.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken"/> used to cancel the operation.</param>
/// <returns>The task object representing the asynchronous operation.</returns>
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do these no longer apply?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK. It builds without them... 🤷

public static Task WriteAsJsonAsync<TValue>(
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
this HttpResponse response,
TValue value,
JsonTypeInfo<TValue> jsonTypeInfo,
Expand Down Expand Up @@ -204,6 +203,8 @@ private static async Task WriteAsJsonAsyncSlow<TValue>(
/// <param name="type">The type of object to write.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken"/> used to cancel the operation.</param>
/// <returns>The task object representing the asynchronous operation.</returns>
[RequiresUnreferencedCode(RequiresUnreferencedCodeMessage)]
[RequiresDynamicCode(RequiresDynamicCodeMessage)]
public static Task WriteAsJsonAsync(
this HttpResponse response,
object? value,
Expand All @@ -212,8 +213,7 @@ public static Task WriteAsJsonAsync(
{
ArgumentNullException.ThrowIfNull(response);

var options = ResolveSerializerOptions(response.HttpContext);
return response.WriteAsJsonAsync(value, jsonTypeInfo: options.GetTypeInfo(type), contentType: null, cancellationToken);
return response.WriteAsJsonAsync(value, type, options: null, contentType: null, cancellationToken);
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
Expand Down Expand Up @@ -302,9 +302,7 @@ private static async Task WriteAsJsonAsyncSlow(
/// <param name="contentType">The content-type to set on the response.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken"/> used to cancel the operation.</param>
/// <returns>The task object representing the asynchronous operation.</returns>
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
public static Task WriteAsJsonAsync(
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
this HttpResponse response,
object? value,
Type type,
Expand Down
55 changes: 24 additions & 31 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat
EndpointBuilder = endpointBuilder,
MetadataAlreadyInferred = metadataResult is not null,
JsonSerializerOptions = jsonSerializerOptions,
JsonSerializerOptionsExpression = Expression.Constant(jsonSerializerOptions, typeof(JsonSerializerOptions)),
};

return factoryContext;
Expand Down Expand Up @@ -1000,23 +999,20 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
ExecuteAwaitedReturnMethod,
methodCall,
HttpContextExpr,
factoryContext.JsonSerializerOptionsExpression,
Expression.Constant(factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo<object>)));
}
else if (returnType == typeof(ValueTask<object>))
{
return Expression.Call(ExecuteValueTaskOfObjectMethod,
methodCall,
HttpContextExpr,
factoryContext.JsonSerializerOptionsExpression,
Expression.Constant(factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo<object>)));
}
else if (returnType == typeof(Task<object>))
{
return Expression.Call(ExecuteTaskOfObjectMethod,
methodCall,
HttpContextExpr,
factoryContext.JsonSerializerOptionsExpression,
Expression.Constant(factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo<object>)));
}
else if (AwaitableInfo.IsTypeAwaitable(returnType, out _))
Expand Down Expand Up @@ -1068,7 +1064,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
ExecuteTaskOfTMethod.MakeGenericMethod(typeArg),
methodCall,
HttpContextExpr,
factoryContext.JsonSerializerOptionsExpression,
Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg)));
}
}
Expand Down Expand Up @@ -1109,7 +1104,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
ExecuteValueTaskOfTMethod.MakeGenericMethod(typeArg),
methodCall,
HttpContextExpr,
factoryContext.JsonSerializerOptionsExpression,
Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg)));
}
}
Expand Down Expand Up @@ -1154,7 +1148,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
JsonResultWriteResponseOfTAsyncMethod.MakeGenericMethod(returnType),
HttpResponseExpr,
methodCall,
factoryContext.JsonSerializerOptionsExpression,
Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(returnType)));
}
}
Expand Down Expand Up @@ -2107,39 +2100,39 @@ private static MemberInfo GetMemberInfo<T>(Expression<T> expr)
// if necessary and restart the cycle until we've reached a terminal state (unknown type).
// We currently don't handle Task<unknown> or ValueTask<unknown>. We can support this later if this
// ends up being a common scenario.
private static Task ExecuteValueTaskOfObject(ValueTask<object> valueTask, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<object> jsonTypeInfo)
private static Task ExecuteValueTaskOfObject(ValueTask<object> valueTask, HttpContext httpContext, JsonTypeInfo<object> jsonTypeInfo)
{
static async Task ExecuteAwaited(ValueTask<object> valueTask, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<object> jsonTypeInfo)
static async Task ExecuteAwaited(ValueTask<object> valueTask, HttpContext httpContext, JsonTypeInfo<object> jsonTypeInfo)
{
await ExecuteAwaitedReturn(await valueTask, httpContext, options, jsonTypeInfo);
await ExecuteAwaitedReturn(await valueTask, httpContext, jsonTypeInfo);
}

if (valueTask.IsCompletedSuccessfully)
{
return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, options, jsonTypeInfo);
return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, jsonTypeInfo);
}

return ExecuteAwaited(valueTask, httpContext, options, jsonTypeInfo);
return ExecuteAwaited(valueTask, httpContext, jsonTypeInfo);
}

private static Task ExecuteTaskOfObject(Task<object> task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<object> jsonTypeInfo)
private static Task ExecuteTaskOfObject(Task<object> task, HttpContext httpContext, JsonTypeInfo<object> jsonTypeInfo)
{
static async Task ExecuteAwaited(Task<object> task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<object> jsonTypeInfo)
static async Task ExecuteAwaited(Task<object> task, HttpContext httpContext, JsonTypeInfo<object> jsonTypeInfo)
{
await ExecuteAwaitedReturn(await task, httpContext, options, jsonTypeInfo);
await ExecuteAwaitedReturn(await task, httpContext, jsonTypeInfo);
}

if (task.IsCompletedSuccessfully)
{
return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, options, jsonTypeInfo);
return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, jsonTypeInfo);
}

return ExecuteAwaited(task, httpContext, options, jsonTypeInfo);
return ExecuteAwaited(task, httpContext, jsonTypeInfo);
}

private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<object> jsonTypeInfo)
private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonTypeInfo<object> jsonTypeInfo)
{
return ExecuteHandlerHelper.ExecuteReturnAsync(obj, httpContext, options, jsonTypeInfo);
return ExecuteHandlerHelper.ExecuteReturnAsync(obj, httpContext, jsonTypeInfo);
}

private static Task ExecuteTaskOfTFast<T>(Task<T> task, HttpContext httpContext, JsonTypeInfo<T> jsonTypeInfo)
Expand All @@ -2159,21 +2152,21 @@ static async Task ExecuteAwaited(Task<T> task, HttpContext httpContext, JsonType
return ExecuteAwaited(task, httpContext, jsonTypeInfo);
}

private static Task ExecuteTaskOfT<T>(Task<T> task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<T> jsonTypeInfo)
private static Task ExecuteTaskOfT<T>(Task<T> task, HttpContext httpContext, JsonTypeInfo<T> jsonTypeInfo)
{
EnsureRequestTaskNotNull(task);

static async Task ExecuteAwaited(Task<T> task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<T> jsonTypeInfo)
static async Task ExecuteAwaited(Task<T> task, HttpContext httpContext, JsonTypeInfo<T> jsonTypeInfo)
{
await WriteJsonResponse(httpContext.Response, await task, options, jsonTypeInfo);
await WriteJsonResponse(httpContext.Response, await task, jsonTypeInfo);
}

if (task.IsCompletedSuccessfully)
{
return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options, jsonTypeInfo);
return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), jsonTypeInfo);
}

return ExecuteAwaited(task, httpContext, options, jsonTypeInfo);
return ExecuteAwaited(task, httpContext, jsonTypeInfo);
}

private static Task ExecuteTaskOfString(Task<string?> task, HttpContext httpContext)
Expand Down Expand Up @@ -2264,19 +2257,19 @@ static async Task ExecuteAwaited(ValueTask<T> task, HttpContext httpContext, Jso
return ExecuteAwaited(task, httpContext, jsonTypeInfo);
}

private static Task ExecuteValueTaskOfT<T>(ValueTask<T> task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<T> jsonTypeInfo)
private static Task ExecuteValueTaskOfT<T>(ValueTask<T> task, HttpContext httpContext, JsonTypeInfo<T> jsonTypeInfo)
{
static async Task ExecuteAwaited(ValueTask<T> task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<T> jsonTypeInfo)
static async Task ExecuteAwaited(ValueTask<T> task, HttpContext httpContext, JsonTypeInfo<T> jsonTypeInfo)
{
await WriteJsonResponse(httpContext.Response, await task, options, jsonTypeInfo);
await WriteJsonResponse(httpContext.Response, await task, jsonTypeInfo);
}

if (task.IsCompletedSuccessfully)
{
return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options, jsonTypeInfo);
return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), jsonTypeInfo);
}

return ExecuteAwaited(task, httpContext, options, jsonTypeInfo);
return ExecuteAwaited(task, httpContext, jsonTypeInfo);
}

private static Task ExecuteValueTaskOfString(ValueTask<string?> task, HttpContext httpContext)
Expand Down Expand Up @@ -2328,9 +2321,9 @@ private static async Task ExecuteResultWriteResponse(IResult? result, HttpContex
private static Task WriteJsonResponseFast<T>(HttpResponse response, T value, JsonTypeInfo<T> jsonTypeInfo)
=> HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, jsonTypeInfo, default);

private static Task WriteJsonResponse<T>(HttpResponse response, T? value, JsonSerializerOptions options, JsonTypeInfo<T> jsonTypeInfo)
private static Task WriteJsonResponse<T>(HttpResponse response, T? value, JsonTypeInfo<T> jsonTypeInfo)
{
return ExecuteHandlerHelper.WriteJsonResponseAsync(response, value, options, jsonTypeInfo);
return ExecuteHandlerHelper.WriteJsonResponseAsync(response, value, jsonTypeInfo);
}

private static NotSupportedException GetUnsupportedReturnTypeException(Type returnType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,4 @@ internal sealed class RequestDelegateFactoryContext

// Grab these options upfront to avoid the per request DI scope that would be made otherwise to get the options when writing Json
public required JsonSerializerOptions JsonSerializerOptions { get; set; }
public required Expression JsonSerializerOptionsExpression { get; set; }
}
35 changes: 0 additions & 35 deletions src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2033,46 +2033,11 @@ public async Task RequestDelegateWritesJsonTypeDiscriminatorToJsonResponseBody_W
Assert.Equal(nameof(JsonTodoChild), deserializedResponseBody["$type"]!.GetValue<string>());
}

public static IEnumerable<object[]> JsonContextActions
{
get
{
return ComplexResult.Concat(ChildResult);
}
}

[JsonSerializable(typeof(Todo))]
[JsonSerializable(typeof(TodoChild))]
private partial class TestJsonContext : JsonSerializerContext
{ }

[Theory]
[MemberData(nameof(JsonContextActions))]
public async Task RequestDelegateWritesAsJsonResponseBody_WithJsonSerializerContext(Delegate @delegate)
{
var httpContext = CreateHttpContext();
httpContext.RequestServices = new ServiceCollection()
.AddSingleton(LoggerFactory)
.ConfigureHttpJsonOptions(o => o.SerializerOptions.TypeInfoResolver = TestJsonContext.Default)
.BuildServiceProvider();

var responseBodyStream = new MemoryStream();
httpContext.Response.Body = responseBodyStream;

var factoryResult = RequestDelegateFactory.Create(@delegate);
var requestDelegate = factoryResult.RequestDelegate;

await requestDelegate(httpContext);

var deserializedResponseBody = JsonSerializer.Deserialize<Todo>(responseBodyStream.ToArray(), new JsonSerializerOptions
{
PropertyNameCaseInsensitive = true
});

Assert.NotNull(deserializedResponseBody);
Assert.Equal("Write even more tests!", deserializedResponseBody!.Name);
}

[Fact]
public void CreateDelegateThrows_WhenGetJsonTypeInfoFail()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,6 @@ public ServiceProvider CreateServiceProvider(Action<IServiceCollection> configur
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddSingleton(LoggerFactory);
var jsonOptions = new JsonOptions();
serviceCollection.AddSingleton(Options.Create(jsonOptions));
if (configureServices is not null)
{
configureServices(serviceCollection);
Expand Down
Loading