From 8f6b7ab4bfe1cb80666747942c07249158b64257 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 6 Jan 2023 14:30:23 -0800 Subject: [PATCH 1/3] Using TypeInfo to read request body in RDF --- .../src/RequestDelegateFactory.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 079d432979b0..995ded99de72 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -12,6 +12,7 @@ using System.Security.Claims; using System.Text; using System.Text.Json; +using System.Text.Json.Serialization.Metadata; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Json; @@ -1159,6 +1160,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, Debug.Assert(factoryContext.JsonRequestBodyParameter is not null, "factoryContext.JsonRequestBodyParameter is null for a JSON body."); var bodyType = factoryContext.JsonRequestBodyParameter.ParameterType; + var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(bodyType); var parameterTypeName = TypeNameHelper.GetTypeDisplayName(factoryContext.JsonRequestBodyParameter.ParameterType, fullName: false); var parameterName = factoryContext.JsonRequestBodyParameter.Name; @@ -1191,7 +1193,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, parameterName, factoryContext.AllowEmptyRequestBody, factoryContext.ThrowOnBadRequest, - factoryContext.JsonSerializerOptions); + factoryContext.JsonSerializerOptions, + jsonTypeInfo); if (!successful) { @@ -1216,7 +1219,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, parameterName, factoryContext.AllowEmptyRequestBody, factoryContext.ThrowOnBadRequest, - factoryContext.JsonSerializerOptions); + factoryContext.JsonSerializerOptions, + jsonTypeInfo); if (!successful) { @@ -1234,7 +1238,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, string parameterName, bool allowEmptyRequestBody, bool throwOnBadRequest, - JsonSerializerOptions? jsonSerializerOptions) + JsonSerializerOptions? jsonSerializerOptions, + JsonTypeInfo? jsonTypeInfo) { object? defaultBodyValue = null; @@ -1256,7 +1261,13 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } try { - bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType, jsonSerializerOptions); + // Edge case but possible if the RequestDelegateFactoryOptions.ServiceProvider and + // RequestDelegateFactoryOptions.EndpointBuilder.ServiceProvider are null + // In this situation both options and jsonTypeInfo are null. + jsonSerializerOptions ??= httpContext.RequestServices.GetService>()?.Value.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; + jsonTypeInfo ??= jsonSerializerOptions.GetTypeInfo(bodyType); + + bodyValue = await httpContext.Request.ReadFromJsonAsync(jsonTypeInfo); } catch (IOException ex) { From 04313eb14abd270345713a8eaaaaeea0768539a1 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 10 Jan 2023 10:00:24 -0800 Subject: [PATCH 2/3] Changing to non-nullable options --- .../Http.Extensions/src/RequestDelegateFactory.cs | 15 +++------------ .../src/RequestDelegateFactoryContext.cs | 2 +- .../test/RequestDelegateFactoryTests.cs | 2 +- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 76e2713de00d..7229547b12d3 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -265,7 +265,7 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat var serviceProvider = options?.ServiceProvider ?? options?.EndpointBuilder?.ApplicationServices ?? EmptyServiceProvider.Instance; var endpointBuilder = options?.EndpointBuilder ?? new RdfEndpointBuilder(serviceProvider); - var jsonSerializerOptions = serviceProvider.GetService>()?.Value.SerializerOptions; + var jsonSerializerOptions = serviceProvider.GetService>()?.Value.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; var factoryContext = new RequestDelegateFactoryContext { @@ -1163,7 +1163,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, Debug.Assert(factoryContext.JsonRequestBodyParameter is not null, "factoryContext.JsonRequestBodyParameter is null for a JSON body."); var bodyType = factoryContext.JsonRequestBodyParameter.ParameterType; - var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(bodyType); + var jsonTypeInfo = factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(bodyType); var parameterTypeName = TypeNameHelper.GetTypeDisplayName(factoryContext.JsonRequestBodyParameter.ParameterType, fullName: false); var parameterName = factoryContext.JsonRequestBodyParameter.Name; @@ -1196,7 +1196,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, parameterName, factoryContext.AllowEmptyRequestBody, factoryContext.ThrowOnBadRequest, - factoryContext.JsonSerializerOptions, jsonTypeInfo); if (!successful) @@ -1222,7 +1221,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, parameterName, factoryContext.AllowEmptyRequestBody, factoryContext.ThrowOnBadRequest, - factoryContext.JsonSerializerOptions, jsonTypeInfo); if (!successful) @@ -1241,8 +1239,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, string parameterName, bool allowEmptyRequestBody, bool throwOnBadRequest, - JsonSerializerOptions? jsonSerializerOptions, - JsonTypeInfo? jsonTypeInfo) + JsonTypeInfo jsonTypeInfo) { object? defaultBodyValue = null; @@ -1264,12 +1261,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } try { - // Edge case but possible if the RequestDelegateFactoryOptions.ServiceProvider and - // RequestDelegateFactoryOptions.EndpointBuilder.ServiceProvider are null - // In this situation both options and jsonTypeInfo are null. - jsonSerializerOptions ??= httpContext.RequestServices.GetService>()?.Value.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; - jsonTypeInfo ??= jsonSerializerOptions.GetTypeInfo(bodyType); - bodyValue = await httpContext.Request.ReadFromJsonAsync(jsonTypeInfo); } catch (IOException ex) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs b/src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs index c2b486515b93..4067434bef1e 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs @@ -58,6 +58,6 @@ internal sealed class RequestDelegateFactoryContext public List Parameters { get; set; } = new(); // Grab these options upfront to avoid the per request DI scope that would be made otherwise to get the options when writing Json - public JsonSerializerOptions? JsonSerializerOptions { get; set; } + public required JsonSerializerOptions JsonSerializerOptions { get; set; } public required Expression JsonSerializerOptionsExpression { get; set; } } diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index ba833e414546..c5871aa6610e 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -1878,7 +1878,7 @@ public async Task RequestDelegatePopulatesFromBodyParameter(Delegate action) }); httpContext.RequestServices = mock.Object; - var factoryResult = RequestDelegateFactory.Create(action); + var factoryResult = RequestDelegateFactory.Create(action, new RequestDelegateFactoryOptions() { ServiceProvider = mock.Object }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); From cbe986f7d4a2f8574b02884c3d199387885f4454 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 10 Jan 2023 10:10:35 -0800 Subject: [PATCH 3/3] Changing to a non-nullable option --- .../src/RequestDelegateFactory.cs | 44 ++++++++----------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 6ea3c306c379..c78390966dae 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1000,7 +1000,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, - Expression.Constant(factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo))); + Expression.Constant(factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo))); } else if (returnType == typeof(ValueTask)) { @@ -1008,7 +1008,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, - Expression.Constant(factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo))); + Expression.Constant(factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo))); } else if (returnType == typeof(Task)) { @@ -1016,7 +1016,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, - Expression.Constant(factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo))); + Expression.Constant(factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo))); } else if (AwaitableInfo.IsTypeAwaitable(returnType, out _)) { @@ -1052,9 +1052,9 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { - var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeArg); + var jsonTypeInfo = factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(typeArg); - if (jsonTypeInfo?.IsPolymorphicSafe() == true) + if (jsonTypeInfo.IsPolymorphicSafe() == true) { return Expression.Call( ExecuteTaskOfTFastMethod.MakeGenericMethod(typeArg), @@ -1093,9 +1093,9 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { - var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeArg); + var jsonTypeInfo = factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(typeArg); - if (jsonTypeInfo?.IsPolymorphicSafe() == true) + if (jsonTypeInfo.IsPolymorphicSafe() == true) { return Expression.Call( ExecuteValueTaskOfTFastMethod.MakeGenericMethod(typeArg), @@ -1137,9 +1137,9 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { - var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(returnType); + var jsonTypeInfo = factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(returnType); - if (jsonTypeInfo?.IsPolymorphicSafe() == true) + if (jsonTypeInfo.IsPolymorphicSafe() == true) { return Expression.Call( JsonResultWriteResponseOfTFastAsyncMethod.MakeGenericMethod(returnType), @@ -2092,9 +2092,9 @@ private static MemberInfo GetMemberInfo(Expression expr) // if necessary and restart the cycle until we've reached a terminal state (unknown type). // We currently don't handle Task or ValueTask. We can support this later if this // ends up being a common scenario. - private static Task ExecuteValueTaskOfObject(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + private static Task ExecuteValueTaskOfObject(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { - static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { await ExecuteAwaitedReturn(await valueTask, httpContext, options, jsonTypeInfo); } @@ -2107,9 +2107,9 @@ static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpCo return ExecuteAwaited(valueTask, httpContext, options, jsonTypeInfo); } - private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { - static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { await ExecuteAwaitedReturn(await task, httpContext, options, jsonTypeInfo); } @@ -2122,7 +2122,7 @@ static async Task ExecuteAwaited(Task task, HttpContext httpContext, Jso return ExecuteAwaited(task, httpContext, options, jsonTypeInfo); } - private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { // Terminal built ins if (obj is IResult result) @@ -2158,11 +2158,11 @@ static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonType return ExecuteAwaited(task, httpContext, jsonTypeInfo); } - private static Task ExecuteTaskOfT(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo jsonTypeInfo) + private static Task ExecuteTaskOfT(Task task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { EnsureRequestTaskNotNull(task); - static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo jsonTypeInfo) + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { await WriteJsonResponse(httpContext.Response, await task, options, jsonTypeInfo); } @@ -2263,9 +2263,9 @@ static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext, Jso return ExecuteAwaited(task, httpContext, jsonTypeInfo); } - private static Task ExecuteValueTaskOfT(ValueTask task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo jsonTypeInfo) + private static Task ExecuteValueTaskOfT(ValueTask task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { - static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo jsonTypeInfo) + static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { await WriteJsonResponse(httpContext.Response, await task, options, jsonTypeInfo); } @@ -2327,16 +2327,10 @@ private static async Task ExecuteResultWriteResponse(IResult? result, HttpContex private static Task WriteJsonResponseFast(HttpResponse response, T value, JsonTypeInfo jsonTypeInfo) => HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, jsonTypeInfo, default); - private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSerializerOptions options, JsonTypeInfo jsonTypeInfo) { var runtimeType = value?.GetType(); - // Edge case but possible if the RequestDelegateFactoryOptions.ServiceProvider and - // RequestDelegateFactoryOptions.EndpointBuilder.ServiceProvider are null - // In this situation both options and jsonTypeInfo are null. - options ??= response.HttpContext.RequestServices.GetService>()?.Value.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; - jsonTypeInfo ??= (JsonTypeInfo)options.GetTypeInfo(typeof(T)); - if (runtimeType is null || jsonTypeInfo.Type == runtimeType || jsonTypeInfo.IsPolymorphicSafe()) { // In this case the polymorphism is not