Skip to content

Commit

Permalink
Stop Calling JSON GetTypeInfo with the runtime type (dotnet#47859)
Browse files Browse the repository at this point in the history 
* Stop Calling JSON GetTypeInfo with the runtime type

Calling GetTypeInfo with the object's runtime type doesn't work when using unspeakable types and using JSON source generation. There is no way to mark the unspeakable type (like a C# compiler implemented IAsyncEnumerable iterator) as JsonSerializable.

Instead, when the "declared type"'s JsonTypeInfo isn't compatible with the runtime type w.r.t. polymorphism, serialize the value "as object", letting System.Text.Json's serialization take over to serialize the value.

Fix dotnet#47548

* Move RequestDelegateFactory tests to be shared with RDG

* Add justifications for suppressing the warnings from JsonSerializer.

* Convert RequestDelegateWritesAsJsonResponseBody_WithJsonSerializerContext to shared test between RDF and RDG.

* Remove redundant JsonSerializerOptions.

* Rename JsonTypeInfo extension method IsValid to ShouldUseWith.
  • Loading branch information
@eerhardt
eerhardt authored Apr 25, 2023
1 parent 47d0784 commit 08f6eea
Show file tree
Hide file tree
Showing 17 changed files with 321 additions and 119 deletions.
7 changes: 5 additions & 2 deletions src/Http/Http.Extensions/gen/RequestDelegateGeneratorSources.cs
View file
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
2 changes: 1 addition & 1 deletion src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Emitters/EndpointJsonResponseEmitter.cs
View file
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
View file
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
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);
}

/// <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
View file
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
1 change: 0 additions & 1 deletion src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs
View file
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
View file
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
2 changes: 0 additions & 2 deletions src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTestBase.cs
View file
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

0 comments on commit 08f6eea

Please sign in to comment.