From 7cc182df4ffd927fd3e0ab3c920c02e1d809f43c Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 29 Nov 2022 14:39:00 -0800 Subject: [PATCH 01/26] Initial GetTypeInfo usage --- .../src/RequestDelegateFactory.cs | 36 ++++++++++++++----- .../SystemTextJsonOutputFormatter.cs | 9 +++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 1eac1fb019eb..9c05b5489bb1 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -12,13 +12,16 @@ 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; using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Http; @@ -63,7 +66,7 @@ public static partial class RequestDelegateFactory private static readonly PropertyInfo FormFilesIndexerProperty = typeof(IFormFileCollection).GetProperty("Item")!; private static readonly PropertyInfo FormIndexerProperty = typeof(IFormCollection).GetProperty("Item")!; - private static readonly MethodInfo JsonResultWriteResponseAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(WriteJsonResponse), BindingFlags.NonPublic | BindingFlags.Static)!; + private static readonly MethodInfo JsonResultOfTWriteResponseAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(WriteJsonResponse), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo LogParameterBindingFailedMethod = GetMethodInfo>((httpContext, parameterType, parameterName, sourceValue, shouldThrow) => Log.ParameterBindingFailed(httpContext, parameterType, parameterName, sourceValue, shouldThrow)); @@ -1102,14 +1105,13 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, { throw GetUnsupportedReturnTypeException(returnType); } - else if (returnType.IsValueType) - { - var box = Expression.TypeAs(methodCall, typeof(object)); - return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, box); - } + //else if (returnType.IsValueType) + //{ + // return Expression.Call(JsonResultOfTWriteResponseAsyncMethod.MakeGenericMethod(returnType), HttpResponseExpr, methodCall); + //} else { - return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, methodCall); + return Expression.Call(JsonResultOfTWriteResponseAsyncMethod.MakeGenericMethod(returnType), HttpResponseExpr, methodCall); } } @@ -2241,13 +2243,29 @@ private static async Task ExecuteResultWriteResponse(IResult? result, HttpContex await EnsureRequestResultNotNull(result).ExecuteAsync(httpContext); } - private static Task WriteJsonResponse(HttpResponse response, object? value) + private static Task WriteJsonResponse(HttpResponse response, T? value) { + // waiting for https://github.com/dotnet/runtime/issues/77051 + + if (value is null) + { + return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, typeof(object), default); + } + + var declaredType = typeof(T); + var options = response.HttpContext.RequestServices?.GetService>()?.Value?.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; + var jsonTypeInfo = (JsonTypeInfo)options.GetTypeInfo(declaredType); + + if (declaredType.IsValueType || jsonTypeInfo.PolymorphismOptions is not null) + { + return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, jsonTypeInfo, default); + } + // Call WriteAsJsonAsync() with the runtime type to serialize the runtime type rather than the declared type // and avoid source generators issues. // https://github.com/dotnet/aspnetcore/issues/43894 // https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism - return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, value is null ? typeof(object) : value.GetType(), default); + return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, value.GetType(), default); } diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 54e14704e7e9..8a279035e125 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -5,6 +5,7 @@ using System.Text; using System.Text.Encodings.Web; using System.Text.Json; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Formatters; @@ -68,12 +69,20 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon var httpContext = context.HttpContext; + // waiting for https://github.com/dotnet/runtime/issues/77051 + // context.ObjectType reflects the declared model type when specified. // For polymorphic scenarios where the user declares a return type, but returns a derived type, // we want to serialize all the properties on the derived type. This keeps parity with // the behavior you get when the user does not declare the return type and with Json.Net at least at the top level. var objectType = context.Object?.GetType() ?? context.ObjectType ?? typeof(object); + if (context.ObjectType is not null && SerializerOptions.GetTypeInfo(context.ObjectType).PolymorphismOptions is not null) + { + // type is polymorphic, so, we should use the declared type + objectType = context.ObjectType; + } + var responseStream = httpContext.Response.Body; if (selectedEncoding.CodePage == Encoding.UTF8.CodePage) { From 69ecc803f54a363c98747fdf6bf7472b24296f5e Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 30 Nov 2022 14:58:53 -0800 Subject: [PATCH 02/26] Merging 45359 --- .../src/RequestDelegateFactory.cs | 93 ++++++++++--------- 1 file changed, 51 insertions(+), 42 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 9c05b5489bb1..b3a2a04ae732 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -271,6 +271,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 factoryContext = new RequestDelegateFactoryContext { @@ -282,6 +283,8 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat DisableInferredFromBody = options?.DisableInferBodyFromParameters ?? false, EndpointBuilder = endpointBuilder, MetadataAlreadyInferred = metadataResult is not null, + JsonSerializerOptions = jsonSerializerOptions, + JsonSerializerOptionsExpression = Expression.Constant(jsonSerializerOptions, typeof(JsonSerializerOptions)), }; return factoryContext; @@ -364,7 +367,7 @@ private static IReadOnlyList AsReadOnlyList(IList metadata) var responseWritingMethodCall = factoryContext.ParamCheckExpressions.Count > 0 ? CreateParamCheckingResponseWritingMethodCall(returnType, factoryContext) : - AddResponseWritingToMethodCall(factoryContext.MethodCall, returnType); + AddResponseWritingToMethodCall(factoryContext.MethodCall, returnType, factoryContext); if (factoryContext.UsingTempSourceString) { @@ -925,7 +928,7 @@ private static Expression CreateParamCheckingResponseWritingMethodCall(Type retu Expression.IfThen( WasParamCheckFailureExpr, Expression.Assign(StatusCodeExpr, Expression.Constant(400))), - AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType) + AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType, factoryContext) ); checkParamAndCallMethod[factoryContext.ParamCheckExpressions.Count] = checkWasParamCheckFailureWithFilters; @@ -943,7 +946,7 @@ private static Expression CreateParamCheckingResponseWritingMethodCall(Type retu Expression.Block( Expression.Assign(StatusCodeExpr, Expression.Constant(400)), CompletedTaskExpr), - AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType)); + AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType, factoryContext)); checkParamAndCallMethod[factoryContext.ParamCheckExpressions.Count] = checkWasParamCheckFailure; } @@ -991,7 +994,7 @@ private static void PopulateBuiltInResponseTypeMetadata(Type returnType, Endpoin } } - private static Expression AddResponseWritingToMethodCall(Expression methodCall, Type returnType) + private static Expression AddResponseWritingToMethodCall(Expression methodCall, Type returnType, RequestDelegateFactoryContext factoryContext) { // Exact request delegate match if (returnType == typeof(void)) @@ -1000,19 +1003,19 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else if (returnType == typeof(object)) { - return Expression.Call(ExecuteAwaitedReturnMethod, methodCall, HttpContextExpr); + return Expression.Call(ExecuteAwaitedReturnMethod, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); } else if (returnType == typeof(ValueTask)) { return Expression.Call(ExecuteValueTaskOfObjectMethod, methodCall, - HttpContextExpr); + HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); } else if (returnType == typeof(Task)) { return Expression.Call(ExecuteTaskOfObjectMethod, methodCall, - HttpContextExpr); + HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); } else if (AwaitableInfo.IsTypeAwaitable(returnType, out _)) { @@ -1051,7 +1054,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, return Expression.Call( ExecuteTaskOfTMethod.MakeGenericMethod(typeArg), methodCall, - HttpContextExpr); + HttpContextExpr, + factoryContext.JsonSerializerOptionsExpression); } } else if (returnType.IsGenericType && @@ -1079,7 +1083,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, return Expression.Call( ExecuteValueTaskOfTMethod.MakeGenericMethod(typeArg), methodCall, - HttpContextExpr); + HttpContextExpr, + factoryContext.JsonSerializerOptionsExpression); } } else @@ -1105,13 +1110,14 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, { throw GetUnsupportedReturnTypeException(returnType); } - //else if (returnType.IsValueType) - //{ - // return Expression.Call(JsonResultOfTWriteResponseAsyncMethod.MakeGenericMethod(returnType), HttpResponseExpr, methodCall); - //} + else if (returnType.IsValueType) + { + var box = Expression.TypeAs(methodCall, typeof(object)); + return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, box, factoryContext.JsonSerializerOptionsExpression); + } else { - return Expression.Call(JsonResultOfTWriteResponseAsyncMethod.MakeGenericMethod(returnType), HttpResponseExpr, methodCall); + return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, methodCall, factoryContext.JsonSerializerOptionsExpression); } } @@ -1192,7 +1198,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, parameterTypeName, parameterName, factoryContext.AllowEmptyRequestBody, - factoryContext.ThrowOnBadRequest); + factoryContext.ThrowOnBadRequest, + factoryContext.JsonSerializerOptions); if (!successful) { @@ -1216,7 +1223,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, parameterTypeName, parameterName, factoryContext.AllowEmptyRequestBody, - factoryContext.ThrowOnBadRequest); + factoryContext.ThrowOnBadRequest, + factoryContext.JsonSerializerOptions); if (!successful) { @@ -1233,7 +1241,8 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, string parameterTypeName, string parameterName, bool allowEmptyRequestBody, - bool throwOnBadRequest) + bool throwOnBadRequest, + JsonSerializerOptions? jsonSerializerOptions) { object? defaultBodyValue = null; @@ -1255,7 +1264,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } try { - bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType); + bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType, jsonSerializerOptions); } catch (IOException ex) { @@ -2045,37 +2054,37 @@ 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) + private static Task ExecuteValueTaskOfObject(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options) { - static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext) + static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options) { - await ExecuteAwaitedReturn(await valueTask, httpContext); + await ExecuteAwaitedReturn(await valueTask, httpContext, options); } if (valueTask.IsCompletedSuccessfully) { - return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext); + return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, options); } - return ExecuteAwaited(valueTask, httpContext); + return ExecuteAwaited(valueTask, httpContext, options); } - private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext) + private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext, JsonSerializerOptions? options) { - static async Task ExecuteAwaited(Task task, HttpContext httpContext) + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options) { - await ExecuteAwaitedReturn(await task, httpContext); + await ExecuteAwaitedReturn(await task, httpContext, options); } if (task.IsCompletedSuccessfully) { - return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext); + return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, options); } - return ExecuteAwaited(task, httpContext); + return ExecuteAwaited(task, httpContext, options); } - private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext) + private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions? options) { // Terminal built ins if (obj is IResult result) @@ -2090,25 +2099,25 @@ private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext) else { // Otherwise, we JSON serialize when we reach the terminal state - return WriteJsonResponse(httpContext.Response, obj); + return WriteJsonResponse(httpContext.Response, obj, options); } } - private static Task ExecuteTaskOfT(Task task, HttpContext httpContext) + private static Task ExecuteTaskOfT(Task task, HttpContext httpContext, JsonSerializerOptions? options) { EnsureRequestTaskNotNull(task); - static async Task ExecuteAwaited(Task task, HttpContext httpContext) + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options) { - await WriteJsonResponse(httpContext.Response, await task); + await WriteJsonResponse(httpContext.Response, await task, options); } if (task.IsCompletedSuccessfully) { - return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult()); + return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options); } - return ExecuteAwaited(task, httpContext); + return ExecuteAwaited(task, httpContext, options); } private static Task ExecuteTaskOfString(Task task, HttpContext httpContext) @@ -2184,19 +2193,19 @@ static async Task ExecuteAwaited(ValueTask task) return ExecuteAwaited(valueTask); } - private static Task ExecuteValueTaskOfT(ValueTask task, HttpContext httpContext) + private static Task ExecuteValueTaskOfT(ValueTask task, HttpContext httpContext, JsonSerializerOptions? options) { - static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext) + static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext, JsonSerializerOptions? options) { - await WriteJsonResponse(httpContext.Response, await task); + await WriteJsonResponse(httpContext.Response, await task, options); } if (task.IsCompletedSuccessfully) { - return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult()); + return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options); } - return ExecuteAwaited(task, httpContext); + return ExecuteAwaited(task, httpContext, options); } private static Task ExecuteValueTaskOfString(ValueTask task, HttpContext httpContext) @@ -2243,7 +2252,7 @@ private static async Task ExecuteResultWriteResponse(IResult? result, HttpContex await EnsureRequestResultNotNull(result).ExecuteAsync(httpContext); } - private static Task WriteJsonResponse(HttpResponse response, T? value) + private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSerializerOptions? options) { // waiting for https://github.com/dotnet/runtime/issues/77051 @@ -2265,7 +2274,7 @@ private static Task WriteJsonResponse(HttpResponse response, T? value) // and avoid source generators issues. // https://github.com/dotnet/aspnetcore/issues/43894 // https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism - return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, value.GetType(), default); + return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, value.GetType(), options, default); } From fb1fabb334b134d0294fb5c05c23357544b9254e Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 30 Nov 2022 17:01:40 -0800 Subject: [PATCH 03/26] Using TypeInfoAPI --- .../src/RequestDelegateFactory.cs | 104 ++++++++++-------- .../src/RequestDelegateFactoryContext.cs | 5 + .../SystemTextJsonOutputFormatter.cs | 4 + 3 files changed, 65 insertions(+), 48 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index b3a2a04ae732..68392b397a07 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -273,6 +273,13 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat var endpointBuilder = options?.EndpointBuilder ?? new RDFEndpointBuilder(serviceProvider); var jsonSerializerOptions = serviceProvider.GetService>()?.Value.SerializerOptions; + // TODO: Verify + if (jsonSerializerOptions is not null) + { + jsonSerializerOptions.TypeInfoResolver ??= new DefaultJsonTypeInfoResolver(); + jsonSerializerOptions.MakeReadOnly(); + } + var factoryContext = new RequestDelegateFactoryContext { Handler = handler, @@ -1003,19 +1010,19 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else if (returnType == typeof(object)) { - return Expression.Call(ExecuteAwaitedReturnMethod, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); + return Expression.Call(ExecuteAwaitedReturnMethod, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, Expression.Constant(null, typeof(JsonTypeInfo))); } else if (returnType == typeof(ValueTask)) { return Expression.Call(ExecuteValueTaskOfObjectMethod, methodCall, - HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); + HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, Expression.Constant(null, typeof(JsonTypeInfo))); } else if (returnType == typeof(Task)) { return Expression.Call(ExecuteTaskOfObjectMethod, methodCall, - HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); + HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, Expression.Constant(null, typeof(JsonTypeInfo))); } else if (AwaitableInfo.IsTypeAwaitable(returnType, out _)) { @@ -1051,11 +1058,14 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { + var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(typeArg); + return Expression.Call( ExecuteTaskOfTMethod.MakeGenericMethod(typeArg), methodCall, HttpContextExpr, - factoryContext.JsonSerializerOptionsExpression); + factoryContext.JsonSerializerOptionsExpression, + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); } } else if (returnType.IsGenericType && @@ -1080,11 +1090,13 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { + var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(typeArg); return Expression.Call( ExecuteValueTaskOfTMethod.MakeGenericMethod(typeArg), methodCall, HttpContextExpr, - factoryContext.JsonSerializerOptionsExpression); + factoryContext.JsonSerializerOptionsExpression, + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); } } else @@ -1110,14 +1122,15 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, { throw GetUnsupportedReturnTypeException(returnType); } - else if (returnType.IsValueType) - { - var box = Expression.TypeAs(methodCall, typeof(object)); - return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, box, factoryContext.JsonSerializerOptionsExpression); - } + //else if (returnType.IsValueType) + //{ + // var box = Expression.TypeAs(methodCall, typeof(object)); + // return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, box, factoryContext.JsonSerializerOptionsExpression); + //} else { - return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, methodCall, factoryContext.JsonSerializerOptionsExpression); + var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(returnType); + return Expression.Call(JsonResultOfTWriteResponseAsyncMethod.MakeGenericMethod(returnType), HttpResponseExpr, methodCall, factoryContext.JsonSerializerOptionsExpression, Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); } } @@ -2054,37 +2067,37 @@ 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) + private static Task ExecuteValueTaskOfObject(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options) + static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - await ExecuteAwaitedReturn(await valueTask, httpContext, options); + await ExecuteAwaitedReturn(await valueTask, httpContext, options, jsonTypeInfo); } if (valueTask.IsCompletedSuccessfully) { - return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, options); + return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, options, jsonTypeInfo); } - return ExecuteAwaited(valueTask, httpContext, options); + return ExecuteAwaited(valueTask, httpContext, options, jsonTypeInfo); } - private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext, JsonSerializerOptions? options) + private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options) + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - await ExecuteAwaitedReturn(await task, httpContext, options); + await ExecuteAwaitedReturn(await task, httpContext, options, jsonTypeInfo); } if (task.IsCompletedSuccessfully) { - return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, options); + return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, options, jsonTypeInfo); } - return ExecuteAwaited(task, httpContext, options); + return ExecuteAwaited(task, httpContext, options, jsonTypeInfo); } - private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions? options) + private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { // Terminal built ins if (obj is IResult result) @@ -2099,25 +2112,25 @@ private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, Js else { // Otherwise, we JSON serialize when we reach the terminal state - return WriteJsonResponse(httpContext.Response, obj, options); + return WriteJsonResponse(httpContext.Response, obj, options, jsonTypeInfo); } } - private static Task ExecuteTaskOfT(Task task, HttpContext httpContext, JsonSerializerOptions? options) + private static Task ExecuteTaskOfT(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { EnsureRequestTaskNotNull(task); - static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options) + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - await WriteJsonResponse(httpContext.Response, await task, options); + await WriteJsonResponse(httpContext.Response, await task, options, jsonTypeInfo); } if (task.IsCompletedSuccessfully) { - return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options); + return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options, jsonTypeInfo); } - return ExecuteAwaited(task, httpContext, options); + return ExecuteAwaited(task, httpContext, options, jsonTypeInfo); } private static Task ExecuteTaskOfString(Task task, HttpContext httpContext) @@ -2193,19 +2206,19 @@ static async Task ExecuteAwaited(ValueTask task) return ExecuteAwaited(valueTask); } - private static Task ExecuteValueTaskOfT(ValueTask task, HttpContext httpContext, JsonSerializerOptions? options) + private static Task ExecuteValueTaskOfT(ValueTask task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext, JsonSerializerOptions? options) + static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - await WriteJsonResponse(httpContext.Response, await task, options); + await WriteJsonResponse(httpContext.Response, await task, options, jsonTypeInfo); } if (task.IsCompletedSuccessfully) { - return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options); + return WriteJsonResponse(httpContext.Response, task.GetAwaiter().GetResult(), options, jsonTypeInfo); } - return ExecuteAwaited(task, httpContext, options); + return ExecuteAwaited(task, httpContext, options, jsonTypeInfo); } private static Task ExecuteValueTaskOfString(ValueTask task, HttpContext httpContext) @@ -2252,30 +2265,25 @@ private static async Task ExecuteResultWriteResponse(IResult? result, HttpContex await EnsureRequestResultNotNull(result).ExecuteAsync(httpContext); } - private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSerializerOptions? options) + private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - // waiting for https://github.com/dotnet/runtime/issues/77051 - - if (value is null) + if (jsonTypeInfo is not null && + (jsonTypeInfo.PolymorphismOptions is not null || + jsonTypeInfo.Type.IsValueType || + jsonTypeInfo.Type == value?.GetType())) { - return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, typeof(object), default); - } - - var declaredType = typeof(T); - var options = response.HttpContext.RequestServices?.GetService>()?.Value?.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; - var jsonTypeInfo = (JsonTypeInfo)options.GetTypeInfo(declaredType); + // waiting for https://github.com/dotnet/runtime/issues/77051 - if (declaredType.IsValueType || jsonTypeInfo.PolymorphismOptions is not null) - { - return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, jsonTypeInfo, default); + // In this case the polymorphism is not + // relevant for us and will be handled by STJ. + return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, (JsonTypeInfo)jsonTypeInfo, default); } // Call WriteAsJsonAsync() with the runtime type to serialize the runtime type rather than the declared type // and avoid source generators issues. // https://github.com/dotnet/aspnetcore/issues/43894 // https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism - return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, value.GetType(), options, default); - + return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, value?.GetType() ?? typeof(object), options, default); } private static NotSupportedException GetUnsupportedReturnTypeException(Type returnType) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs b/src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs index 1f0c0963ae19..a19dbe24090e 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs @@ -3,6 +3,7 @@ using System.Linq.Expressions; using System.Reflection; +using System.Text.Json; using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.DependencyInjection; @@ -56,4 +57,8 @@ internal sealed class RequestDelegateFactoryContext public bool FilterFactoriesHaveRunWithoutModifyingPerRequestBehavior { get; set; } 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 Expression JsonSerializerOptionsExpression { get; set; } } diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 8a279035e125..4089ce22c08c 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -5,6 +5,7 @@ using System.Text; using System.Text.Encodings.Web; using System.Text.Json; +using System.Text.Json.Serialization.Metadata; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Formatters; @@ -42,6 +43,9 @@ internal static SystemTextJsonOutputFormatter CreateFormatter(JsonOptions jsonOp }; } + jsonSerializerOptions.TypeInfoResolver ??= new DefaultJsonTypeInfoResolver(); + jsonSerializerOptions.MakeReadOnly(); + return new SystemTextJsonOutputFormatter(jsonSerializerOptions); } From 2eac02f43d7b120478d0214136ffabfb6f3a1f96 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 1 Dec 2022 13:55:34 -0800 Subject: [PATCH 04/26] Updating comments --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 6 +++--- .../src/Formatters/SystemTextJsonOutputFormatter.cs | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index fdd994a01697..6d8ed0603f70 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -2272,13 +2272,13 @@ private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSe jsonTypeInfo.Type.IsValueType || jsonTypeInfo.Type == value?.GetType())) { - // waiting for https://github.com/dotnet/runtime/issues/77051 - // In this case the polymorphism is not - // relevant for us and will be handled by STJ. + // relevant for us and will be handled by STJ, if needed. return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, (JsonTypeInfo)jsonTypeInfo, default); } + // We cannot use JsonTypeInfo here, waiting for https://github.com/dotnet/runtime/issues/77051 + // Call WriteAsJsonAsync() with the runtime type to serialize the runtime type rather than the declared type // and avoid source generators issues. // https://github.com/dotnet/aspnetcore/issues/43894 diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 4089ce22c08c..3699551a2a71 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -43,6 +43,7 @@ internal static SystemTextJsonOutputFormatter CreateFormatter(JsonOptions jsonOp }; } + // TODO: Verify jsonSerializerOptions.TypeInfoResolver ??= new DefaultJsonTypeInfoResolver(); jsonSerializerOptions.MakeReadOnly(); @@ -73,6 +74,7 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon var httpContext = context.HttpContext; + // Maybe we could use the jsontypeinfo overload but we need the untyped, // waiting for https://github.com/dotnet/runtime/issues/77051 // context.ObjectType reflects the declared model type when specified. From 59e9b6e647697a62e3fcd491cdbdef8065b8ce8e Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 1 Dec 2022 14:01:48 -0800 Subject: [PATCH 05/26] Remove using --- src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 3699551a2a71..f85d52b32820 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -6,7 +6,6 @@ using System.Text.Encodings.Web; using System.Text.Json; using System.Text.Json.Serialization.Metadata; -using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Formatters; From 1e05325370bd735db13886c09440c2ebfd8d5b9b Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 1 Dec 2022 16:25:50 -0800 Subject: [PATCH 06/26] Updating MVC support --- src/Mvc/Mvc.Core/src/ActionResultOfT.cs | 5 +++++ .../src/Formatters/SystemTextJsonOutputFormatter.cs | 9 +++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ActionResultOfT.cs b/src/Mvc/Mvc.Core/src/ActionResultOfT.cs index 8086cadfd0ae..9ad739e2bfae 100644 --- a/src/Mvc/Mvc.Core/src/ActionResultOfT.cs +++ b/src/Mvc/Mvc.Core/src/ActionResultOfT.cs @@ -79,6 +79,11 @@ IActionResult IConvertToActionResult.Convert() { if (Result != null) { + if (Result is ObjectResult objectResult) + { + objectResult.DeclaredType ??= typeof(TValue); + return objectResult; + } return Result; } diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index f85d52b32820..275a753d834b 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -76,13 +76,18 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon // Maybe we could use the jsontypeinfo overload but we need the untyped, // waiting for https://github.com/dotnet/runtime/issues/77051 + var runtimeType = context.Object?.GetType(); + // context.ObjectType reflects the declared model type when specified. // For polymorphic scenarios where the user declares a return type, but returns a derived type, // we want to serialize all the properties on the derived type. This keeps parity with // the behavior you get when the user does not declare the return type and with Json.Net at least at the top level. - var objectType = context.Object?.GetType() ?? context.ObjectType ?? typeof(object); + var objectType = runtimeType ?? context.ObjectType ?? typeof(object); - if (context.ObjectType is not null && SerializerOptions.GetTypeInfo(context.ObjectType).PolymorphismOptions is not null) + if (runtimeType is not null && + context.ObjectType is not null && + runtimeType != context.ObjectType && + SerializerOptions.GetTypeInfo(context.ObjectType).PolymorphismOptions is not null) { // type is polymorphic, so, we should use the declared type objectType = context.ObjectType; From 044d2196112abf8c59b1b4e355f894a059829a36 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 2 Dec 2022 13:31:32 -0800 Subject: [PATCH 07/26] Adding fastpath --- .../src/RequestDelegateFactory.cs | 106 ++++++++++++++---- 1 file changed, 85 insertions(+), 21 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 6d8ed0603f70..f9bb90463e3f 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -41,10 +41,12 @@ public static partial class RequestDelegateFactory private static readonly MethodInfo ExecuteTaskWithEmptyResultMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteTaskWithEmptyResult), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteValueTaskWithEmptyResultMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteValueTaskWithEmptyResult), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteTaskOfTMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteTaskOfT), BindingFlags.NonPublic | BindingFlags.Static)!; + private static readonly MethodInfo ExecuteTaskOfTFastMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteTaskOfTFast), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteTaskOfObjectMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteTaskOfObject), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteValueTaskOfObjectMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteValueTaskOfObject), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteTaskOfStringMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteTaskOfString), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteValueTaskOfTMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteValueTaskOfT), BindingFlags.NonPublic | BindingFlags.Static)!; + private static readonly MethodInfo ExecuteValueTaskOfTFastMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteValueTaskOfTFast), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteValueTaskMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteValueTask), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteValueTaskOfStringMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteValueTaskOfString), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteTaskResultOfTMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteTaskResult), BindingFlags.NonPublic | BindingFlags.Static)!; @@ -67,6 +69,7 @@ public static partial class RequestDelegateFactory private static readonly PropertyInfo FormIndexerProperty = typeof(IFormCollection).GetProperty("Item")!; private static readonly MethodInfo JsonResultOfTWriteResponseAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(WriteJsonResponse), BindingFlags.NonPublic | BindingFlags.Static)!; + private static readonly MethodInfo JsonResultOfTWriteResponseFastAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(WriteJsonResponseFast), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo LogParameterBindingFailedMethod = GetMethodInfo>((httpContext, parameterType, parameterName, sourceValue, shouldThrow) => Log.ParameterBindingFailed(httpContext, parameterType, parameterName, sourceValue, shouldThrow)); @@ -1010,19 +1013,19 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else if (returnType == typeof(object)) { - return Expression.Call(ExecuteAwaitedReturnMethod, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, Expression.Constant(null, typeof(JsonTypeInfo))); + return Expression.Call(ExecuteAwaitedReturnMethod, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); } else if (returnType == typeof(ValueTask)) { return Expression.Call(ExecuteValueTaskOfObjectMethod, methodCall, - HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, Expression.Constant(null, typeof(JsonTypeInfo))); + HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); } else if (returnType == typeof(Task)) { return Expression.Call(ExecuteTaskOfObjectMethod, methodCall, - HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, Expression.Constant(null, typeof(JsonTypeInfo))); + HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); } else if (AwaitableInfo.IsTypeAwaitable(returnType, out _)) { @@ -1060,6 +1063,16 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, { var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(typeArg); + if (jsonTypeInfo is not null && + (typeArg.IsSealed || typeArg.IsValueType)) + { + return Expression.Call( + ExecuteTaskOfTFastMethod.MakeGenericMethod(typeArg), + methodCall, + HttpContextExpr, + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); + } + return Expression.Call( ExecuteTaskOfTMethod.MakeGenericMethod(typeArg), methodCall, @@ -1091,6 +1104,17 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, else { var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(typeArg); + + if (jsonTypeInfo is not null && + (typeArg.IsSealed || typeArg.IsValueType)) + { + return Expression.Call( + ExecuteValueTaskOfTFastMethod.MakeGenericMethod(typeArg), + methodCall, + HttpContextExpr, + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); + } + return Expression.Call( ExecuteValueTaskOfTMethod.MakeGenericMethod(typeArg), methodCall, @@ -1122,14 +1146,20 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, { throw GetUnsupportedReturnTypeException(returnType); } - //else if (returnType.IsValueType) - //{ - // var box = Expression.TypeAs(methodCall, typeof(object)); - // return Expression.Call(JsonResultWriteResponseAsyncMethod, HttpResponseExpr, box, factoryContext.JsonSerializerOptionsExpression); - //} else { var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(returnType); + + if (jsonTypeInfo is not null && + (returnType.IsSealed || returnType.IsValueType)) + { + return Expression.Call( + JsonResultOfTWriteResponseFastAsyncMethod.MakeGenericMethod(returnType), + HttpResponseExpr, + methodCall, + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); + } + return Expression.Call(JsonResultOfTWriteResponseAsyncMethod.MakeGenericMethod(returnType), HttpResponseExpr, methodCall, factoryContext.JsonSerializerOptionsExpression, Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); } } @@ -2067,37 +2097,37 @@ 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) { - static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options) { - await ExecuteAwaitedReturn(await valueTask, httpContext, options, jsonTypeInfo); + await ExecuteAwaitedReturn(await valueTask, httpContext, options); } if (valueTask.IsCompletedSuccessfully) { - return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, options, jsonTypeInfo); + return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, options); } - return ExecuteAwaited(valueTask, httpContext, options, jsonTypeInfo); + return ExecuteAwaited(valueTask, httpContext, options); } - private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext, JsonSerializerOptions? options) { - static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options) { - await ExecuteAwaitedReturn(await task, httpContext, options, jsonTypeInfo); + await ExecuteAwaitedReturn(await task, httpContext, options); } if (task.IsCompletedSuccessfully) { - return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, options, jsonTypeInfo); + return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, options); } - return ExecuteAwaited(task, httpContext, options, jsonTypeInfo); + return ExecuteAwaited(task, httpContext, options); } - private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) + private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions? options) { // Terminal built ins if (obj is IResult result) @@ -2112,8 +2142,25 @@ private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, Js else { // Otherwise, we JSON serialize when we reach the terminal state - return WriteJsonResponse(httpContext.Response, obj, options, jsonTypeInfo); + return WriteJsonResponse(httpContext.Response, obj, options, null); + } + } + + private static Task ExecuteTaskOfTFast(Task task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) + { + EnsureRequestTaskNotNull(task); + + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) + { + await WriteJsonResponseFast(httpContext.Response, await task, jsonTypeInfo); } + + if (task.IsCompletedSuccessfully) + { + return WriteJsonResponseFast(httpContext.Response, task.GetAwaiter().GetResult(), jsonTypeInfo); + } + + return ExecuteAwaited(task, httpContext, jsonTypeInfo); } private static Task ExecuteTaskOfT(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) @@ -2206,6 +2253,21 @@ static async Task ExecuteAwaited(ValueTask task) return ExecuteAwaited(valueTask); } + private static Task ExecuteValueTaskOfTFast(ValueTask task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) + { + static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) + { + await WriteJsonResponseFast(httpContext.Response, await task, jsonTypeInfo); + } + + if (task.IsCompletedSuccessfully) + { + return WriteJsonResponseFast(httpContext.Response, task.GetAwaiter().GetResult(), jsonTypeInfo); + } + + return ExecuteAwaited(task, httpContext, 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) @@ -2265,11 +2327,13 @@ private static async Task ExecuteResultWriteResponse(IResult? result, HttpContex await EnsureRequestResultNotNull(result).ExecuteAsync(httpContext); } + private static Task WriteJsonResponseFast(HttpResponse response, T value, JsonTypeInfo jsonTypeInfo) + => HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, (JsonTypeInfo)jsonTypeInfo, default); + private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { if (jsonTypeInfo is not null && (jsonTypeInfo.PolymorphismOptions is not null || - jsonTypeInfo.Type.IsValueType || jsonTypeInfo.Type == value?.GetType())) { // In this case the polymorphism is not From 0b7940f88403da0145cd1eeb9ed50cb9cf0ef4a8 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 2 Dec 2022 13:31:44 -0800 Subject: [PATCH 08/26] Trying to fix mvc --- .../SystemTextJsonOutputFormatter.cs | 38 ++++++++++++------- .../Infrastructure/ObjectResultExecutor.cs | 4 +- .../SystemTextJsonOutputFormatterTest.cs | 6 ++- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 275a753d834b..cfa3bd1cf0a3 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -76,23 +76,33 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon // Maybe we could use the jsontypeinfo overload but we need the untyped, // waiting for https://github.com/dotnet/runtime/issues/77051 - var runtimeType = context.Object?.GetType(); - - // context.ObjectType reflects the declared model type when specified. - // For polymorphic scenarios where the user declares a return type, but returns a derived type, - // we want to serialize all the properties on the derived type. This keeps parity with - // the behavior you get when the user does not declare the return type and with Json.Net at least at the top level. - var objectType = runtimeType ?? context.ObjectType ?? typeof(object); - - if (runtimeType is not null && - context.ObjectType is not null && - runtimeType != context.ObjectType && - SerializerOptions.GetTypeInfo(context.ObjectType).PolymorphismOptions is not null) + Type GetSerializationType(object? content, Type? declaredType) { - // type is polymorphic, so, we should use the declared type - objectType = context.ObjectType; + // context.ObjectType reflects the declared model type when specified. + // For polymorphic scenarios where the user declares a return type, but returns a derived type, + // we want to serialize all the properties on the derived type. This keeps parity with + // the behavior you get when the user does not declare the return type and with Json.Net at least at the top level. + + if (content is null) + { + return declaredType ?? typeof(object); + } + + var runtimeType = content.GetType(); + + if (declaredType is not null && + runtimeType != declaredType && + SerializerOptions.GetTypeInfo(declaredType).PolymorphismOptions is not null) + { + // Using declared type in this case. The polymorphism is not + // relevant for us and will be handled by STJ, if needed. + return declaredType; + } + return runtimeType; } + var objectType = GetSerializationType(context.Object, context.ObjectType); + var responseStream = httpContext.Response.Body; if (selectedEncoding.CodePage == Encoding.UTF8.CodePage) { diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs index d8bab1ab8ef6..015b04db18a4 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs @@ -89,7 +89,9 @@ public virtual Task ExecuteAsync(ActionContext context, ObjectResult result) var objectType = result.DeclaredType; - if (objectType == null || objectType == typeof(object)) + if (objectType == null || + objectType == typeof(object) || + result.Value?.GetType() is { } runtimeType && !objectType.IsAssignableFrom(runtimeType)) { objectType = result.Value?.GetType(); } diff --git a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs index 71fd6d97ac52..a133e3957542 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs @@ -21,8 +21,10 @@ protected override TextOutputFormatter GetOutputFormatter() public async Task WriteResponseBodyAsync_AllowsConfiguringPreserveReferenceHandling() { // Arrange - var formatter = GetOutputFormatter(); - ((SystemTextJsonOutputFormatter)formatter).SerializerOptions.ReferenceHandler = ReferenceHandler.Preserve; + var jsonOptions = new JsonOptions(); + jsonOptions.JsonSerializerOptions.ReferenceHandler = ReferenceHandler.Preserve; + + var formatter = SystemTextJsonOutputFormatter.CreateFormatter(jsonOptions); var expectedContent = "{\"$id\":\"1\",\"name\":\"Person\",\"child\":{\"$id\":\"2\",\"name\":\"Child\",\"child\":null,\"parent\":{\"$ref\":\"1\"}},\"parent\":null}"; var person = new Person { From 32d7cf4f7fe57670a60f3bcc9a1a52f35d942fb2 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 2 Dec 2022 14:30:33 -0800 Subject: [PATCH 09/26] Updating mvc support --- src/Mvc/Mvc.Core/src/ActionResultOfT.cs | 5 ----- .../src/Formatters/SystemTextJsonOutputFormatter.cs | 6 +++--- src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs | 4 +--- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ActionResultOfT.cs b/src/Mvc/Mvc.Core/src/ActionResultOfT.cs index 9ad739e2bfae..8086cadfd0ae 100644 --- a/src/Mvc/Mvc.Core/src/ActionResultOfT.cs +++ b/src/Mvc/Mvc.Core/src/ActionResultOfT.cs @@ -79,11 +79,6 @@ IActionResult IConvertToActionResult.Convert() { if (Result != null) { - if (Result is ObjectResult objectResult) - { - objectResult.DeclaredType ??= typeof(TValue); - return objectResult; - } return Result; } diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index cfa3bd1cf0a3..7a2182a44ef6 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -76,7 +76,7 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon // Maybe we could use the jsontypeinfo overload but we need the untyped, // waiting for https://github.com/dotnet/runtime/issues/77051 - Type GetSerializationType(object? content, Type? declaredType) + static Type GetSerializationType(object? content, Type? declaredType, JsonSerializerOptions options) { // context.ObjectType reflects the declared model type when specified. // For polymorphic scenarios where the user declares a return type, but returns a derived type, @@ -92,7 +92,7 @@ Type GetSerializationType(object? content, Type? declaredType) if (declaredType is not null && runtimeType != declaredType && - SerializerOptions.GetTypeInfo(declaredType).PolymorphismOptions is not null) + options.GetTypeInfo(declaredType).PolymorphismOptions is not null) { // Using declared type in this case. The polymorphism is not // relevant for us and will be handled by STJ, if needed. @@ -101,7 +101,7 @@ Type GetSerializationType(object? content, Type? declaredType) return runtimeType; } - var objectType = GetSerializationType(context.Object, context.ObjectType); + var objectType = GetSerializationType(context.Object, context.ObjectType, SerializerOptions); var responseStream = httpContext.Response.Body; if (selectedEncoding.CodePage == Encoding.UTF8.CodePage) diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs index 015b04db18a4..d8bab1ab8ef6 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs @@ -89,9 +89,7 @@ public virtual Task ExecuteAsync(ActionContext context, ObjectResult result) var objectType = result.DeclaredType; - if (objectType == null || - objectType == typeof(object) || - result.Value?.GetType() is { } runtimeType && !objectType.IsAssignableFrom(runtimeType)) + if (objectType == null || objectType == typeof(object)) { objectType = result.Value?.GetType(); } From 16f37f9550cb58b565e633fbf01a4063f3d77f95 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 2 Dec 2022 14:59:00 -0800 Subject: [PATCH 10/26] Simplify MVC --- .../SystemTextJsonOutputFormatter.cs | 34 ++++++------------- 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 7a2182a44ef6..6e710158cc34 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -76,33 +76,19 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon // Maybe we could use the jsontypeinfo overload but we need the untyped, // waiting for https://github.com/dotnet/runtime/issues/77051 - static Type GetSerializationType(object? content, Type? declaredType, JsonSerializerOptions options) - { - // context.ObjectType reflects the declared model type when specified. - // For polymorphic scenarios where the user declares a return type, but returns a derived type, - // we want to serialize all the properties on the derived type. This keeps parity with - // the behavior you get when the user does not declare the return type and with Json.Net at least at the top level. - - if (content is null) - { - return declaredType ?? typeof(object); - } - - var runtimeType = content.GetType(); + var declaredType = context.ObjectType; + var runtimeType = context.Object?.GetType(); + var objectType = runtimeType ?? declaredType ?? typeof(object); - if (declaredType is not null && - runtimeType != declaredType && - options.GetTypeInfo(declaredType).PolymorphismOptions is not null) - { - // Using declared type in this case. The polymorphism is not - // relevant for us and will be handled by STJ, if needed. - return declaredType; - } - return runtimeType; + if (declaredType is not null && + runtimeType != declaredType && + SerializerOptions.GetTypeInfo(declaredType).PolymorphismOptions is not null) + { + // Using declared type in this case. The polymorphism is not + // relevant for us and will be handled by STJ, if needed. + objectType = declaredType; } - var objectType = GetSerializationType(context.Object, context.ObjectType, SerializerOptions); - var responseStream = httpContext.Response.Body; if (selectedEncoding.CodePage == Encoding.UTF8.CodePage) { From 784d3645e918444a9cbf27de7196e13556733f78 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 2 Dec 2022 16:26:57 -0800 Subject: [PATCH 11/26] Moving more to fastpath --- .../Http.Extensions/src/RequestDelegateFactory.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index f9bb90463e3f..dca1c8baf325 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1064,7 +1064,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(typeArg); if (jsonTypeInfo is not null && - (typeArg.IsSealed || typeArg.IsValueType)) + (typeArg.IsSealed || typeArg.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) { return Expression.Call( ExecuteTaskOfTFastMethod.MakeGenericMethod(typeArg), @@ -1106,7 +1106,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(typeArg); if (jsonTypeInfo is not null && - (typeArg.IsSealed || typeArg.IsValueType)) + (typeArg.IsSealed || typeArg.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) { return Expression.Call( ExecuteValueTaskOfTFastMethod.MakeGenericMethod(typeArg), @@ -1151,7 +1151,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(returnType); if (jsonTypeInfo is not null && - (returnType.IsSealed || returnType.IsValueType)) + (returnType.IsSealed || returnType.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) { return Expression.Call( JsonResultOfTWriteResponseFastAsyncMethod.MakeGenericMethod(returnType), @@ -2332,9 +2332,9 @@ private static Task WriteJsonResponseFast(HttpResponse response, T value, Jso private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - if (jsonTypeInfo is not null && - (jsonTypeInfo.PolymorphismOptions is not null || - jsonTypeInfo.Type == value?.GetType())) + var runtimeType = value is null ? typeof(object) : value.GetType(); + + if (jsonTypeInfo is not null && jsonTypeInfo.Type == runtimeType) { // In this case the polymorphism is not // relevant for us and will be handled by STJ, if needed. @@ -2347,7 +2347,7 @@ private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSe // and avoid source generators issues. // https://github.com/dotnet/aspnetcore/issues/43894 // https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism - return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, value is null ? typeof(object) : value.GetType(), options, default); + return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, runtimeType, options, default); } From 62edd21e554af4e6f868cbd7653e86af03b0f7c3 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 8 Dec 2022 16:48:19 -0800 Subject: [PATCH 12/26] Adding JsonOptions setup --- src/DefaultBuilder/src/WebHost.cs | 4 ++++ .../src/HttpJsonOptionsSetup.cs | 24 +++++++++++++++++++ .../src/RequestDelegateFactory.cs | 12 ++++------ 3 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 src/Http/Http.Extensions/src/HttpJsonOptionsSetup.cs diff --git a/src/DefaultBuilder/src/WebHost.cs b/src/DefaultBuilder/src/WebHost.cs index 156efd0e326e..ed5b3318aab7 100644 --- a/src/DefaultBuilder/src/WebHost.cs +++ b/src/DefaultBuilder/src/WebHost.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Hosting.StaticWebAssets; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Json; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -247,6 +248,9 @@ internal static void ConfigureWebDefaults(IWebHostBuilder builder) services.AddTransient(); services.AddTransient, ForwardedHeadersOptionsSetup>(); + // Json options + services.AddTransient, HttpJsonOptionsSetup>(); + services.AddRouting(); }) .UseIIS() diff --git a/src/Http/Http.Extensions/src/HttpJsonOptionsSetup.cs b/src/Http/Http.Extensions/src/HttpJsonOptionsSetup.cs new file mode 100644 index 000000000000..5e872b97ebe9 --- /dev/null +++ b/src/Http/Http.Extensions/src/HttpJsonOptionsSetup.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics.CodeAnalysis; +using System.Text.Json.Serialization.Metadata; +using Microsoft.AspNetCore.Http.Json; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Http; + +public sealed class HttpJsonOptionsSetup : IConfigureOptions +{ + [UnconditionalSuppressMessage("Trimmer", "IL2026", Justification = "Calls System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.DefaultJsonTypeInfoResolver().")] + public void Configure(JsonOptions options) + { + if (options.SerializerOptions.TypeInfoResolver is null) + { + options.SerializerOptions.TypeInfoResolver = new DefaultJsonTypeInfoResolver(); + + // TODO: Should we make it readonly???? + options.SerializerOptions.MakeReadOnly(); + } + } +} diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index dca1c8baf325..e031638431f1 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -276,13 +276,6 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat var endpointBuilder = options?.EndpointBuilder ?? new RDFEndpointBuilder(serviceProvider); var jsonSerializerOptions = serviceProvider.GetService>()?.Value.SerializerOptions; - // TODO: Verify - if (jsonSerializerOptions is not null) - { - jsonSerializerOptions.TypeInfoResolver ??= new DefaultJsonTypeInfoResolver(); - jsonSerializerOptions.MakeReadOnly(); - } - var factoryContext = new RequestDelegateFactoryContext { Handler = handler, @@ -2341,7 +2334,10 @@ private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSe return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, (JsonTypeInfo)jsonTypeInfo, default); } - // We cannot use JsonTypeInfo here, waiting for https://github.com/dotnet/runtime/issues/77051 + // We cannot use JsonTypeInfo here yet, waiting for https://github.com/dotnet/runtime/issues/77051 + // What should happens if options is null? + // var runtimeTypeInfo = options!.GetTypeInfo(runtimeType); + // return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, runtimeTypeInfo, default); // Call WriteAsJsonAsync() with the runtime type to serialize the runtime type rather than the declared type // and avoid source generators issues. From 5586708778964aa0364e61a05079daec921e4cc0 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 9 Dec 2022 14:22:02 -0800 Subject: [PATCH 13/26] Moving DefaultJsonTypeInfoResolver instance creation --- src/DefaultBuilder/src/WebHost.cs | 3 --- .../src/HttpJsonOptionsSetup.cs | 24 ------------------- src/Http/Http.Extensions/src/JsonOptions.cs | 8 +++++++ .../src/RequestDelegateFactory.cs | 24 +++++++++++++++---- .../SystemTextJsonOutputFormatter.cs | 6 +---- src/Mvc/Mvc.Core/src/JsonOptions.cs | 6 +++++ 6 files changed, 34 insertions(+), 37 deletions(-) delete mode 100644 src/Http/Http.Extensions/src/HttpJsonOptionsSetup.cs diff --git a/src/DefaultBuilder/src/WebHost.cs b/src/DefaultBuilder/src/WebHost.cs index ed5b3318aab7..0a0ab5a1481c 100644 --- a/src/DefaultBuilder/src/WebHost.cs +++ b/src/DefaultBuilder/src/WebHost.cs @@ -248,9 +248,6 @@ internal static void ConfigureWebDefaults(IWebHostBuilder builder) services.AddTransient(); services.AddTransient, ForwardedHeadersOptionsSetup>(); - // Json options - services.AddTransient, HttpJsonOptionsSetup>(); - services.AddRouting(); }) .UseIIS() diff --git a/src/Http/Http.Extensions/src/HttpJsonOptionsSetup.cs b/src/Http/Http.Extensions/src/HttpJsonOptionsSetup.cs deleted file mode 100644 index 5e872b97ebe9..000000000000 --- a/src/Http/Http.Extensions/src/HttpJsonOptionsSetup.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics.CodeAnalysis; -using System.Text.Json.Serialization.Metadata; -using Microsoft.AspNetCore.Http.Json; -using Microsoft.Extensions.Options; - -namespace Microsoft.AspNetCore.Http; - -public sealed class HttpJsonOptionsSetup : IConfigureOptions -{ - [UnconditionalSuppressMessage("Trimmer", "IL2026", Justification = "Calls System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.DefaultJsonTypeInfoResolver().")] - public void Configure(JsonOptions options) - { - if (options.SerializerOptions.TypeInfoResolver is null) - { - options.SerializerOptions.TypeInfoResolver = new DefaultJsonTypeInfoResolver(); - - // TODO: Should we make it readonly???? - options.SerializerOptions.MakeReadOnly(); - } - } -} diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index 220ad7938f53..7b0d6bd17c34 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -1,8 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; using System.Text.Encodings.Web; using System.Text.Json; +using System.Text.Json.Serialization.Metadata; #nullable enable @@ -14,6 +16,7 @@ namespace Microsoft.AspNetCore.Http.Json; /// public class JsonOptions { + [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "")] internal static readonly JsonSerializerOptions DefaultSerializerOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web) { // Web defaults don't use the relex JSON escaping encoder. @@ -21,6 +24,11 @@ public class JsonOptions // Because these options are for producing content that is written directly to the request // (and not embedded in an HTML page for example), we can use UnsafeRelaxedJsonEscaping. Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, + + // The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver + // setting the default resolver (reflection-based) but the user can overwrite it directly or calling + // .AddContext() + TypeInfoResolver = new DefaultJsonTypeInfoResolver() }; // Use a copy so the defaults are not modified. diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index e031638431f1..e1afcd85fb8f 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1054,7 +1054,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { - var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(typeArg); + var jsonTypeInfo = GetJsonTypeInfo(factoryContext.JsonSerializerOptions, typeArg); if (jsonTypeInfo is not null && (typeArg.IsSealed || typeArg.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) @@ -1096,7 +1096,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { - var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(typeArg); + var jsonTypeInfo = GetJsonTypeInfo(factoryContext.JsonSerializerOptions, typeArg); if (jsonTypeInfo is not null && (typeArg.IsSealed || typeArg.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) @@ -1141,7 +1141,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { - var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetTypeInfo(returnType); + var jsonTypeInfo = GetJsonTypeInfo(factoryContext.JsonSerializerOptions, returnType); if (jsonTypeInfo is not null && (returnType.IsSealed || returnType.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) @@ -2335,8 +2335,8 @@ private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSe } // We cannot use JsonTypeInfo here yet, waiting for https://github.com/dotnet/runtime/issues/77051 - // What should happens if options is null? - // var runtimeTypeInfo = options!.GetTypeInfo(runtimeType); + // What should happens if options or type info is null? + // var runtimeTypeInfo = GetJsonTypeInfo(options, runtimeType); // return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, runtimeTypeInfo, default); // Call WriteAsJsonAsync() with the runtime type to serialize the runtime type rather than the declared type @@ -2346,6 +2346,20 @@ private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSe return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, runtimeType, options, default); } + private static JsonTypeInfo? GetJsonTypeInfo(JsonSerializerOptions? jsonSerializerOptions, Type type) + { + if (jsonSerializerOptions?.TypeInfoResolver != null) + { + if (!jsonSerializerOptions.IsReadOnly) + { + jsonSerializerOptions.MakeReadOnly(); + } + + return jsonSerializerOptions.GetTypeInfo(type); + } + + return null; + } private static NotSupportedException GetUnsupportedReturnTypeException(Type returnType) { diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 6e710158cc34..65231aa3c2f3 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -5,7 +5,6 @@ using System.Text; using System.Text.Encodings.Web; using System.Text.Json; -using System.Text.Json.Serialization.Metadata; namespace Microsoft.AspNetCore.Mvc.Formatters; @@ -42,10 +41,6 @@ internal static SystemTextJsonOutputFormatter CreateFormatter(JsonOptions jsonOp }; } - // TODO: Verify - jsonSerializerOptions.TypeInfoResolver ??= new DefaultJsonTypeInfoResolver(); - jsonSerializerOptions.MakeReadOnly(); - return new SystemTextJsonOutputFormatter(jsonSerializerOptions); } @@ -82,6 +77,7 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon if (declaredType is not null && runtimeType != declaredType && + SerializerOptions.TypeInfoResolver != null && SerializerOptions.GetTypeInfo(declaredType).PolymorphismOptions is not null) { // Using declared type in this case. The polymorphism is not diff --git a/src/Mvc/Mvc.Core/src/JsonOptions.cs b/src/Mvc/Mvc.Core/src/JsonOptions.cs index d3966142f027..bbfcc75890d7 100644 --- a/src/Mvc/Mvc.Core/src/JsonOptions.cs +++ b/src/Mvc/Mvc.Core/src/JsonOptions.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Text.Json; +using System.Text.Json.Serialization.Metadata; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -37,5 +38,10 @@ public class JsonOptions // from deserialization errors that might occur from deeply nested objects. // This value is the same for model binding and Json.Net's serialization. MaxDepth = MvcOptions.DefaultMaxModelBindingRecursionDepth, + + // The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver + // setting the default resolver (reflection-based) but the user can overwrite it directly or calling + // .AddContext() + TypeInfoResolver = new DefaultJsonTypeInfoResolver() }; } From 19b54b62ffae49e2b94bad35a2c744088d614d8a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 9 Dec 2022 14:30:42 -0800 Subject: [PATCH 14/26] Fix bad merge --- .../Http.Extensions/src/RequestDelegateFactory.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index f8eb5376bc22..265f31891495 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -69,7 +69,7 @@ public static partial class RequestDelegateFactory private static readonly PropertyInfo FormIndexerProperty = typeof(IFormCollection).GetProperty("Item")!; private static readonly MethodInfo JsonResultWriteResponseOfTFastAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(WriteJsonResponseFast), BindingFlags.NonPublic | BindingFlags.Static)!; - private static readonly MethodInfo JsonResultWriteResponseOfTAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(WriteJsonResponseOfT), BindingFlags.NonPublic | BindingFlags.Static)!; + private static readonly MethodInfo JsonResultWriteResponseOfTAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(WriteJsonResponse), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo LogParameterBindingFailedMethod = GetMethodInfo>((httpContext, parameterType, parameterName, sourceValue, shouldThrow) => Log.ParameterBindingFailed(httpContext, parameterType, parameterName, sourceValue, shouldThrow)); @@ -2320,6 +2320,8 @@ private static async Task ExecuteResultWriteResponse(IResult? result, HttpContex await EnsureRequestResultNotNull(result).ExecuteAsync(httpContext); } + // This method will not check for polymorphism and will + // leverage the STJ polymorphism support. private static Task WriteJsonResponseFast(HttpResponse response, T value, JsonTypeInfo jsonTypeInfo) => HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, (JsonTypeInfo)jsonTypeInfo, default); @@ -2344,13 +2346,8 @@ private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSe // https://github.com/dotnet/aspnetcore/issues/43894 // https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, runtimeType, options, default); - - // Only for use with structs, use WriteJsonResponse for classes to preserve polymorphism - private static Task WriteJsonResponseOfT(HttpResponse response, T value, JsonSerializerOptions? options) - { - Debug.Assert(typeof(T).IsValueType); - return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, options, default); } + private static JsonTypeInfo? GetJsonTypeInfo(JsonSerializerOptions? jsonSerializerOptions, Type type) { if (jsonSerializerOptions?.TypeInfoResolver != null) From 5f76c1ba035d87fc2ac1a73f36417e305f567f43 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 9 Dec 2022 16:42:49 -0800 Subject: [PATCH 15/26] Adding RDF unit tests --- src/DefaultBuilder/src/WebHost.cs | 1 - .../test/RequestDelegateFactoryTests.cs | 99 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/src/DefaultBuilder/src/WebHost.cs b/src/DefaultBuilder/src/WebHost.cs index 0a0ab5a1481c..156efd0e326e 100644 --- a/src/DefaultBuilder/src/WebHost.cs +++ b/src/DefaultBuilder/src/WebHost.cs @@ -8,7 +8,6 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Hosting.StaticWebAssets; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Json; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index b1f173440043..145fa734ce20 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -17,7 +17,9 @@ using System.Security.Cryptography.X509Certificates; using System.Text; using System.Text.Json; +using System.Text.Json.Nodes; using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -31,6 +33,7 @@ using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Moq; +using Xunit.Abstractions; namespace Microsoft.AspNetCore.Routing.Internal; @@ -3070,6 +3073,95 @@ public async Task RequestDelegateWritesMembersFromChildTypesToJsonResponseBody(D Assert.Equal("With type hierarchies!", deserializedResponseBody!.Child); } + public static IEnumerable PolymorphicResult + { + get + { + JsonTodoChild originalTodo = new() + { + Name = "Write even more tests!", + Child = "With type hierarchies!", + }; + + JsonTodo TestAction() => originalTodo; + + Task TaskTestAction() => Task.FromResult(originalTodo); + async Task TaskTestActionAwaited() + { + await Task.Yield(); + return originalTodo; + } + + ValueTask ValueTaskTestAction() => ValueTask.FromResult(originalTodo); + async ValueTask ValueTaskTestActionAwaited() + { + await Task.Yield(); + return originalTodo; + } + + return new List + { + new object[] { (Func)TestAction }, + new object[] { (Func>)TaskTestAction}, + new object[] { (Func>)TaskTestActionAwaited}, + new object[] { (Func>)ValueTaskTestAction}, + new object[] { (Func>)ValueTaskTestActionAwaited}, + }; + } + } + + [Theory] + [MemberData(nameof(PolymorphicResult))] + public async Task RequestDelegateWritesMembersFromChildTypesToJsonResponseBody_WithJsonPolymorphicOptions(Delegate @delegate) + { + var httpContext = CreateHttpContext(); + httpContext.RequestServices = new ServiceCollection() + .AddSingleton(LoggerFactory) + .AddSingleton(Options.Create(new JsonOptions())) + .BuildServiceProvider(); + var responseBodyStream = new MemoryStream(); + httpContext.Response.Body = responseBodyStream; + + var factoryResult = RequestDelegateFactory.Create(@delegate, new() { ServiceProvider = httpContext.RequestServices }); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + var deserializedResponseBody = JsonSerializer.Deserialize(responseBodyStream.ToArray(), new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true + }); + + Assert.NotNull(deserializedResponseBody); + Assert.Equal("Write even more tests!", deserializedResponseBody!.Name); + Assert.Equal("With type hierarchies!", deserializedResponseBody!.Child); + } + + [Theory] + [MemberData(nameof(PolymorphicResult))] + public async Task RequestDelegateWritesJsonTypeDistinguisherToJsonResponseBody_WithJsonPolymorphicOptions(Delegate @delegate) + { + var httpContext = CreateHttpContext(); + httpContext.RequestServices = new ServiceCollection() + .AddSingleton(LoggerFactory) + .AddSingleton(Options.Create(new JsonOptions())) + .BuildServiceProvider(); + + var responseBodyStream = new MemoryStream(); + httpContext.Response.Body = responseBodyStream; + + var factoryResult = RequestDelegateFactory.Create(@delegate, new() { ServiceProvider = httpContext.RequestServices }); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + var deserializedResponseBody = JsonNode.Parse(responseBodyStream.ToArray()); + + Assert.NotNull(deserializedResponseBody); + Assert.NotNull(deserializedResponseBody["$type"]); + Assert.Equal(nameof(JsonTodoChild), deserializedResponseBody["$type"]!.GetValue()); + } + public static IEnumerable JsonContextActions { get @@ -7357,6 +7449,11 @@ private class TodoChild : Todo public string? Child { get; set; } } + private class JsonTodoChild : JsonTodo + { + public string? Child { get; set; } + } + private class CustomTodo : Todo { public static async ValueTask BindAsync(HttpContext context, ParameterInfo parameter) @@ -7370,6 +7467,8 @@ private class CustomTodo : Todo } } + [JsonPolymorphic] + [JsonDerivedType(typeof(JsonTodoChild), nameof(JsonTodoChild))] private class JsonTodo : Todo { public static async ValueTask BindAsync(HttpContext context, ParameterInfo parameter) From 7657b254e8692cb7cb8a915dfa5898a89fa00f31 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 9 Dec 2022 17:04:35 -0800 Subject: [PATCH 16/26] Adding more RDF tests --- .../test/RequestDelegateFactoryTests.cs | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 145fa734ce20..e4a77afabb1a 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -3139,7 +3139,7 @@ public async Task RequestDelegateWritesMembersFromChildTypesToJsonResponseBody_W [Theory] [MemberData(nameof(PolymorphicResult))] - public async Task RequestDelegateWritesJsonTypeDistinguisherToJsonResponseBody_WithJsonPolymorphicOptions(Delegate @delegate) + public async Task RequestDelegateWritesJsonTypeDiscriminatorToJsonResponseBody_WithJsonPolymorphicOptions(Delegate @delegate) { var httpContext = CreateHttpContext(); httpContext.RequestServices = new ServiceCollection() @@ -3188,7 +3188,7 @@ public async Task RequestDelegateWritesAsJsonResponseBody_WithJsonSerializerCont var responseBodyStream = new MemoryStream(); httpContext.Response.Body = responseBodyStream; - var factoryResult = RequestDelegateFactory.Create(@delegate); + var factoryResult = RequestDelegateFactory.Create(@delegate, new() { ServiceProvider = httpContext.RequestServices }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -3202,6 +3202,22 @@ public async Task RequestDelegateWritesAsJsonResponseBody_WithJsonSerializerCont Assert.Equal("Write even more tests!", deserializedResponseBody!.Name); } + [Fact] + public void CreateDelegateThrows_WhenGetJsonTypeInfoFail() + { + var httpContext = CreateHttpContext(); + httpContext.RequestServices = new ServiceCollection() + .AddSingleton(LoggerFactory) + .ConfigureHttpJsonOptions(o => o.SerializerOptions.AddContext()) + .BuildServiceProvider(); + + var responseBodyStream = new MemoryStream(); + httpContext.Response.Body = responseBodyStream; + + TodoStruct TestAction() => new TodoStruct(42, "Bob", true); + Assert.Throws(() => RequestDelegateFactory.Create(TestAction, new() { ServiceProvider = httpContext.RequestServices })); + } + public static IEnumerable CustomResults { get From c3f98bff2f1fd93e4d853dfd6abd6884ae6f42b3 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 12 Dec 2022 13:22:27 -0800 Subject: [PATCH 17/26] Adding MVC unit tests --- .../SystemTextJsonOutputFormatter.cs | 6 ++ .../SystemTextJsonOutputFormatterTest.cs | 87 +++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 65231aa3c2f3..9aa64594cf8a 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -41,6 +41,12 @@ internal static SystemTextJsonOutputFormatter CreateFormatter(JsonOptions jsonOp }; } + if (!jsonSerializerOptions.IsReadOnly && + jsonSerializerOptions.TypeInfoResolver != null) + { + jsonSerializerOptions.MakeReadOnly(); + } + return new SystemTextJsonOutputFormatter(jsonSerializerOptions); } diff --git a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs index a133e3957542..318da2b617c6 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs @@ -4,7 +4,9 @@ using System.Runtime.CompilerServices; using System.Text; using System.Text.Json; +using System.Text.Json.Nodes; using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; @@ -158,6 +160,81 @@ async IAsyncEnumerable AsyncEnumerableClosedConnection([EnumeratorCancellat } } + [Fact] + public async Task WriteResponseBodyAsync_UsesJsonPolymorphismOptions() + { + // Arrange + var jsonOptions = new JsonOptions(); + + var formatter = SystemTextJsonOutputFormatter.CreateFormatter(jsonOptions); + var expectedContent = "{\"$type\":\"JsonPersonExtended\",\"age\":99,\"name\":\"Person\",\"child\":null,\"parent\":null}"; + JsonPerson todo = new JsonPersonExtended() + { + Name = "Person", + Age = 99, + }; + + var mediaType = MediaTypeHeaderValue.Parse("application/json; charset=utf-8"); + var encoding = CreateOrGetSupportedEncoding(formatter, "utf-8", isDefaultEncoding: true); + + var body = new MemoryStream(); + var actionContext = GetActionContext(mediaType, body); + + var outputFormatterContext = new OutputFormatterWriteContext( + actionContext.HttpContext, + new TestHttpResponseStreamWriterFactory().CreateWriter, + typeof(JsonPerson), + todo) + { + ContentType = new StringSegment(mediaType.ToString()), + }; + + // Act + await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding("utf-8")); + + // Assert + var actualContent = encoding.GetString(body.ToArray()); + Assert.Equal(expectedContent, actualContent); + } + + [Fact] + public async Task WriteResponseBodyAsync_UsesRuntimeType_WhenTypeResolverIsNull() + { + // Arrange + var jsonOptions = new JsonOptions(); + jsonOptions.JsonSerializerOptions.TypeInfoResolver = null; + + var formatter = SystemTextJsonOutputFormatter.CreateFormatter(jsonOptions); + var expectedContent = "{\"age\":99,\"name\":\"Person\",\"child\":null,\"parent\":null}"; + JsonPerson todo = new JsonPersonExtended() + { + Name = "Person", + Age = 99, + }; + + var mediaType = MediaTypeHeaderValue.Parse("application/json; charset=utf-8"); + var encoding = CreateOrGetSupportedEncoding(formatter, "utf-8", isDefaultEncoding: true); + + var body = new MemoryStream(); + var actionContext = GetActionContext(mediaType, body); + + var outputFormatterContext = new OutputFormatterWriteContext( + actionContext.HttpContext, + new TestHttpResponseStreamWriterFactory().CreateWriter, + typeof(JsonPerson), + todo) + { + ContentType = new StringSegment(mediaType.ToString()), + }; + + // Act + await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding("utf-8")); + + // Assert + var actualContent = encoding.GetString(body.ToArray()); + Assert.Equal(expectedContent, actualContent); + } + private class Person { public string Name { get; set; } @@ -167,6 +244,16 @@ private class Person public Person Parent { get; set; } } + [JsonPolymorphic] + [JsonDerivedType(typeof(JsonPersonExtended), nameof(JsonPersonExtended))] + private class JsonPerson : Person + {} + + private class JsonPersonExtended : JsonPerson + { + public int Age { get; set; } + } + [JsonConverter(typeof(ThrowingFormatterPersonConverter))] private class ThrowingFormatterModel { From e4eb4bd872ee2455f950b1675d27eca8e800795a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 12 Dec 2022 13:27:49 -0800 Subject: [PATCH 18/26] Updating IL2026 warnings --- src/Http/Http.Extensions/src/JsonOptions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index 7b0d6bd17c34..1e2daf952de4 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -16,7 +16,8 @@ namespace Microsoft.AspNetCore.Http.Json; /// public class JsonOptions { - [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "")] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", + Justification = "The default type resolver will be set as DefaultJsonTypeInfoResolver (reflection-based) that has the same runtime behavior when TypeResolver is null.")] internal static readonly JsonSerializerOptions DefaultSerializerOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web) { // Web defaults don't use the relex JSON escaping encoder. From 84b7eb58d36f7ce0a0e2441f09f093955d1024dd Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 12 Dec 2022 13:31:42 -0800 Subject: [PATCH 19/26] Scoping trimming warning --- src/Http/Http.Extensions/src/JsonOptions.cs | 9 ++++++--- src/Mvc/Mvc.Core/src/JsonOptions.cs | 8 +++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index 1e2daf952de4..0a47e6631280 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -16,8 +16,6 @@ namespace Microsoft.AspNetCore.Http.Json; /// public class JsonOptions { - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", - Justification = "The default type resolver will be set as DefaultJsonTypeInfoResolver (reflection-based) that has the same runtime behavior when TypeResolver is null.")] internal static readonly JsonSerializerOptions DefaultSerializerOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web) { // Web defaults don't use the relex JSON escaping encoder. @@ -29,7 +27,7 @@ public class JsonOptions // The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver // setting the default resolver (reflection-based) but the user can overwrite it directly or calling // .AddContext() - TypeInfoResolver = new DefaultJsonTypeInfoResolver() + TypeInfoResolver = CreateDefaultTypeResolver() }; // Use a copy so the defaults are not modified. @@ -37,4 +35,9 @@ public class JsonOptions /// Gets the . /// public JsonSerializerOptions SerializerOptions { get; } = new JsonSerializerOptions(DefaultSerializerOptions); + + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", + Justification = "The default type resolver will be set as DefaultJsonTypeInfoResolver (reflection-based) that has the same runtime behavior when TypeResolver is null.")] + private static IJsonTypeInfoResolver? CreateDefaultTypeResolver() + => new DefaultJsonTypeInfoResolver(); } diff --git a/src/Mvc/Mvc.Core/src/JsonOptions.cs b/src/Mvc/Mvc.Core/src/JsonOptions.cs index bbfcc75890d7..44aad84caa57 100644 --- a/src/Mvc/Mvc.Core/src/JsonOptions.cs +++ b/src/Mvc/Mvc.Core/src/JsonOptions.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Serialization.Metadata; using Microsoft.AspNetCore.Mvc.Formatters; @@ -42,6 +43,11 @@ public class JsonOptions // The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver // setting the default resolver (reflection-based) but the user can overwrite it directly or calling // .AddContext() - TypeInfoResolver = new DefaultJsonTypeInfoResolver() + TypeInfoResolver = CreateDefaultTypeResolver() }; + + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", + Justification = "The default type resolver will be set as DefaultJsonTypeInfoResolver (reflection-based) that has the same runtime behavior when TypeResolver is null.")] + private static IJsonTypeInfoResolver? CreateDefaultTypeResolver() + => new DefaultJsonTypeInfoResolver(); } From ca3809f34342002eb3b0d1602ae257939b6c911d Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 15 Dec 2022 13:49:29 -0800 Subject: [PATCH 20/26] Updates --- src/Http/Http.Extensions/src/JsonOptions.cs | 8 ++-- .../src/RequestDelegateFactory.cs | 44 +++++++++++-------- .../SystemTextJsonOutputFormatter.cs | 6 --- src/Mvc/Mvc.Core/src/JsonOptions.cs | 7 +-- 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index 0a47e6631280..93577c3fd1c2 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -27,7 +27,10 @@ public class JsonOptions // The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver // setting the default resolver (reflection-based) but the user can overwrite it directly or calling // .AddContext() +#pragma warning disable IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code TypeInfoResolver = CreateDefaultTypeResolver() +#pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code + }; // Use a copy so the defaults are not modified. @@ -36,8 +39,7 @@ public class JsonOptions /// public JsonSerializerOptions SerializerOptions { get; } = new JsonSerializerOptions(DefaultSerializerOptions); - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", - Justification = "The default type resolver will be set as DefaultJsonTypeInfoResolver (reflection-based) that has the same runtime behavior when TypeResolver is null.")] - private static IJsonTypeInfoResolver? CreateDefaultTypeResolver() + [RequiresUnreferencedCode("Calls System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.DefaultJsonTypeInfoResolver()")] + private static IJsonTypeInfoResolver CreateDefaultTypeResolver() => new DefaultJsonTypeInfoResolver(); } diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 265f31891495..347c4c11b7b3 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1055,6 +1055,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, else { var jsonTypeInfo = GetJsonTypeInfo(factoryContext.JsonSerializerOptions, typeArg); + var jsonTypeInfoConstant = Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg)); if (jsonTypeInfo is not null && (typeArg.IsSealed || typeArg.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) @@ -1063,7 +1064,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, ExecuteTaskOfTFastMethod.MakeGenericMethod(typeArg), methodCall, HttpContextExpr, - Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); + jsonTypeInfoConstant); } return Expression.Call( @@ -1071,7 +1072,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, - Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); + jsonTypeInfoConstant); } } else if (returnType.IsGenericType && @@ -1097,6 +1098,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, else { var jsonTypeInfo = GetJsonTypeInfo(factoryContext.JsonSerializerOptions, typeArg); + var jsonTypeInfoConstant = Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg)); if (jsonTypeInfo is not null && (typeArg.IsSealed || typeArg.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) @@ -1105,7 +1107,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, ExecuteValueTaskOfTFastMethod.MakeGenericMethod(typeArg), methodCall, HttpContextExpr, - Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); + jsonTypeInfoConstant); } return Expression.Call( @@ -1113,7 +1115,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, - Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); + jsonTypeInfoConstant); } } else @@ -1142,6 +1144,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, else { var jsonTypeInfo = GetJsonTypeInfo(factoryContext.JsonSerializerOptions, returnType); + var jsonTypeInfoConstant = Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(returnType)); if (jsonTypeInfo is not null && (returnType.IsSealed || returnType.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) @@ -1150,10 +1153,15 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, JsonResultWriteResponseOfTFastAsyncMethod.MakeGenericMethod(returnType), HttpResponseExpr, methodCall, - Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); + jsonTypeInfoConstant); } - return Expression.Call(JsonResultWriteResponseOfTAsyncMethod.MakeGenericMethod(returnType), HttpResponseExpr, methodCall, factoryContext.JsonSerializerOptionsExpression, Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo))); + return Expression.Call( + JsonResultWriteResponseOfTAsyncMethod.MakeGenericMethod(returnType), + HttpResponseExpr, + methodCall, + factoryContext.JsonSerializerOptionsExpression, + jsonTypeInfoConstant); } } @@ -2139,11 +2147,11 @@ private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, Js } } - private static Task ExecuteTaskOfTFast(Task task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) + private static Task ExecuteTaskOfTFast(Task task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) { EnsureRequestTaskNotNull(task); - static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) { await WriteJsonResponseFast(httpContext.Response, await task, jsonTypeInfo); } @@ -2156,11 +2164,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); } @@ -2246,9 +2254,9 @@ static async Task ExecuteAwaited(ValueTask task) return ExecuteAwaited(valueTask); } - private static Task ExecuteValueTaskOfTFast(ValueTask task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) + private static Task ExecuteValueTaskOfTFast(ValueTask task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) { - static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) + static async Task ExecuteAwaited(ValueTask task, HttpContext httpContext, JsonTypeInfo jsonTypeInfo) { await WriteJsonResponseFast(httpContext.Response, await task, jsonTypeInfo); } @@ -2261,9 +2269,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); } @@ -2322,10 +2330,10 @@ private static async Task ExecuteResultWriteResponse(IResult? result, HttpContex // This method will not check for polymorphism and will // leverage the STJ polymorphism support. - private static Task WriteJsonResponseFast(HttpResponse response, T value, JsonTypeInfo jsonTypeInfo) - => HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, (JsonTypeInfo)jsonTypeInfo, default); + 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 is null ? typeof(object) : value.GetType(); @@ -2333,7 +2341,7 @@ private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSe { // In this case the polymorphism is not // relevant for us and will be handled by STJ, if needed. - return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, (JsonTypeInfo)jsonTypeInfo, default); + return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, jsonTypeInfo, default); } // We cannot use JsonTypeInfo here yet, waiting for https://github.com/dotnet/runtime/issues/77051 diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 9aa64594cf8a..65231aa3c2f3 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -41,12 +41,6 @@ internal static SystemTextJsonOutputFormatter CreateFormatter(JsonOptions jsonOp }; } - if (!jsonSerializerOptions.IsReadOnly && - jsonSerializerOptions.TypeInfoResolver != null) - { - jsonSerializerOptions.MakeReadOnly(); - } - return new SystemTextJsonOutputFormatter(jsonSerializerOptions); } diff --git a/src/Mvc/Mvc.Core/src/JsonOptions.cs b/src/Mvc/Mvc.Core/src/JsonOptions.cs index 44aad84caa57..087b74849b0d 100644 --- a/src/Mvc/Mvc.Core/src/JsonOptions.cs +++ b/src/Mvc/Mvc.Core/src/JsonOptions.cs @@ -43,11 +43,12 @@ public class JsonOptions // The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver // setting the default resolver (reflection-based) but the user can overwrite it directly or calling // .AddContext() +#pragma warning disable IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code TypeInfoResolver = CreateDefaultTypeResolver() +#pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code }; - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", - Justification = "The default type resolver will be set as DefaultJsonTypeInfoResolver (reflection-based) that has the same runtime behavior when TypeResolver is null.")] - private static IJsonTypeInfoResolver? CreateDefaultTypeResolver() + [RequiresUnreferencedCode("Calls System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.DefaultJsonTypeInfoResolver()")] + private static IJsonTypeInfoResolver CreateDefaultTypeResolver() => new DefaultJsonTypeInfoResolver(); } From 6e8783069f3ef996cfb6d276a4455855717fb993 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 16 Dec 2022 14:55:09 -0800 Subject: [PATCH 21/26] Changing to always JsonTypeInfo --- .../src/HttpResponseJsonExtensions.cs | 44 +++++++ src/Http/Http.Extensions/src/JsonOptions.cs | 3 +- .../src/JsonSerializerExtensions.cs | 19 +++ .../src/RequestDelegateFactory.cs | 111 ++++++++---------- .../test/RequestDelegateFactoryTests.cs | 33 +++++- .../SystemTextJsonOutputFormatter.cs | 53 ++++++--- src/Mvc/Mvc.Core/src/JsonOptions.cs | 3 +- .../Formatters/JsonOutputFormatterTestBase.cs | 2 +- .../SystemTextJsonOutputFormatterTest.cs | 32 +---- .../SystemTextJsonOutputFormatterTest.cs | 14 +++ ...SystemTextJsonOutputFormatterController.cs | 37 ++++++ 11 files changed, 241 insertions(+), 110 deletions(-) create mode 100644 src/Http/Http.Extensions/src/JsonSerializerExtensions.cs create mode 100644 src/Mvc/test/WebSites/FormatterWebSite/Controllers/SystemTextJsonOutputFormatterController.cs diff --git a/src/Http/Http.Extensions/src/HttpResponseJsonExtensions.cs b/src/Http/Http.Extensions/src/HttpResponseJsonExtensions.cs index 300b836a7d34..e43197ec9afb 100644 --- a/src/Http/Http.Extensions/src/HttpResponseJsonExtensions.cs +++ b/src/Http/Http.Extensions/src/HttpResponseJsonExtensions.cs @@ -139,6 +139,50 @@ static async Task WriteAsJsonAsyncSlow(HttpResponse response, TValue value, Json } } + /// + /// Write the specified value as JSON to the response body. The response content-type will be set to + /// the specified content-type. + /// + /// The response to write JSON to. + /// The value to write as JSON. + /// Metadata about the type to convert. + /// The content-type to set on the response. + /// A used to cancel the operation. + /// The task object representing the asynchronous operation. +#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters + internal static Task WriteAsJsonAsync( +#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters + this HttpResponse response, + object? value, + JsonTypeInfo jsonTypeInfo, + string? contentType = default, + CancellationToken cancellationToken = default) + { + if (response == null) + { + throw new ArgumentNullException(nameof(response)); + } + + response.ContentType = contentType ?? JsonConstants.JsonContentTypeWithCharset; + + // if no user provided token, pass the RequestAborted token and ignore OperationCanceledException + if (!cancellationToken.CanBeCanceled) + { + return WriteAsJsonAsyncSlow(response, value, jsonTypeInfo); + } + + return JsonSerializer.SerializeAsync(response.Body, value, jsonTypeInfo, cancellationToken); + + static async Task WriteAsJsonAsyncSlow(HttpResponse response, object? value, JsonTypeInfo jsonTypeInfo) + { + try + { + await JsonSerializer.SerializeAsync(response.Body, value, jsonTypeInfo, response.HttpContext.RequestAborted); + } + catch (OperationCanceledException) { } + } + } + [RequiresUnreferencedCode(RequiresUnreferencedCodeMessage)] private static async Task WriteAsJsonAsyncSlow( Stream body, diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index 93577c3fd1c2..09649a3a2764 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -39,7 +39,8 @@ public class JsonOptions /// public JsonSerializerOptions SerializerOptions { get; } = new JsonSerializerOptions(DefaultSerializerOptions); - [RequiresUnreferencedCode("Calls System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.DefaultJsonTypeInfoResolver()")] + [RequiresUnreferencedCode("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver might require types that cannot be statically analyzed and might need runtime code generation.")] + [RequiresDynamicCode("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver might require types that cannot be statically analyzed and might need runtime code generation. Enable EnsureJsonTrimmability feature switch for native AOT applications.")] private static IJsonTypeInfoResolver CreateDefaultTypeResolver() => new DefaultJsonTypeInfoResolver(); } diff --git a/src/Http/Http.Extensions/src/JsonSerializerExtensions.cs b/src/Http/Http.Extensions/src/JsonSerializerExtensions.cs new file mode 100644 index 000000000000..bf53e0f21993 --- /dev/null +++ b/src/Http/Http.Extensions/src/JsonSerializerExtensions.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text.Json; +using System.Text.Json.Serialization.Metadata; + +namespace Microsoft.AspNetCore.Http; + +internal static class JsonSerializerExtensions +{ + public static bool IsPolymorphicSafe(this JsonTypeInfo jsonTypeInfo) + => jsonTypeInfo.Type.IsSealed || jsonTypeInfo.Type.IsValueType || jsonTypeInfo.PolymorphismOptions is not null; + + public static JsonTypeInfo GetReadOnlyTypeInfo(this JsonSerializerOptions options, Type type) + { + options.MakeReadOnly(); + return options.GetTypeInfo(type); + } +} diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 347c4c11b7b3..63090ae7a793 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1006,19 +1006,28 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else if (returnType == typeof(object)) { - return Expression.Call(ExecuteAwaitedReturnMethod, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); + return Expression.Call( + ExecuteAwaitedReturnMethod, + methodCall, + HttpContextExpr, + factoryContext.JsonSerializerOptionsExpression, + Expression.Constant(factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo))); } else if (returnType == typeof(ValueTask)) { return Expression.Call(ExecuteValueTaskOfObjectMethod, methodCall, - HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); + HttpContextExpr, + factoryContext.JsonSerializerOptionsExpression, + Expression.Constant(factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo))); } else if (returnType == typeof(Task)) { return Expression.Call(ExecuteTaskOfObjectMethod, methodCall, - HttpContextExpr, factoryContext.JsonSerializerOptionsExpression); + HttpContextExpr, + factoryContext.JsonSerializerOptionsExpression, + Expression.Constant(factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeof(object)), typeof(JsonTypeInfo))); } else if (AwaitableInfo.IsTypeAwaitable(returnType, out _)) { @@ -1054,17 +1063,15 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { - var jsonTypeInfo = GetJsonTypeInfo(factoryContext.JsonSerializerOptions, typeArg); - var jsonTypeInfoConstant = Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg)); + var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeArg); - if (jsonTypeInfo is not null && - (typeArg.IsSealed || typeArg.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) + if (jsonTypeInfo?.IsPolymorphicSafe() == true) { return Expression.Call( ExecuteTaskOfTFastMethod.MakeGenericMethod(typeArg), methodCall, HttpContextExpr, - jsonTypeInfoConstant); + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg))); } return Expression.Call( @@ -1072,7 +1079,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, - jsonTypeInfoConstant); + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg))); } } else if (returnType.IsGenericType && @@ -1097,17 +1104,15 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { - var jsonTypeInfo = GetJsonTypeInfo(factoryContext.JsonSerializerOptions, typeArg); - var jsonTypeInfoConstant = Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg)); + var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(typeArg); - if (jsonTypeInfo is not null && - (typeArg.IsSealed || typeArg.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) + if (jsonTypeInfo?.IsPolymorphicSafe() == true) { return Expression.Call( ExecuteValueTaskOfTFastMethod.MakeGenericMethod(typeArg), methodCall, HttpContextExpr, - jsonTypeInfoConstant); + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg))); } return Expression.Call( @@ -1115,7 +1120,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, methodCall, HttpContextExpr, factoryContext.JsonSerializerOptionsExpression, - jsonTypeInfoConstant); + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(typeArg))); } } else @@ -1143,17 +1148,16 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } else { - var jsonTypeInfo = GetJsonTypeInfo(factoryContext.JsonSerializerOptions, returnType); - var jsonTypeInfoConstant = Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(returnType)); + var jsonTypeInfo = factoryContext.JsonSerializerOptions?.GetReadOnlyTypeInfo(returnType); - if (jsonTypeInfo is not null && - (returnType.IsSealed || returnType.IsValueType || jsonTypeInfo.PolymorphismOptions is not null)) + if (jsonTypeInfo?.IsPolymorphicSafe() == true) { return Expression.Call( JsonResultWriteResponseOfTFastAsyncMethod.MakeGenericMethod(returnType), HttpResponseExpr, methodCall, - jsonTypeInfoConstant); + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(returnType))); + } return Expression.Call( @@ -1161,7 +1165,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, HttpResponseExpr, methodCall, factoryContext.JsonSerializerOptionsExpression, - jsonTypeInfoConstant); + Expression.Constant(jsonTypeInfo, typeof(JsonTypeInfo<>).MakeGenericType(returnType))); } } @@ -2098,37 +2102,37 @@ 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) + private static Task ExecuteValueTaskOfObject(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options) + static async Task ExecuteAwaited(ValueTask valueTask, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - await ExecuteAwaitedReturn(await valueTask, httpContext, options); + await ExecuteAwaitedReturn(await valueTask, httpContext, options, jsonTypeInfo); } if (valueTask.IsCompletedSuccessfully) { - return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, options); + return ExecuteAwaitedReturn(valueTask.GetAwaiter().GetResult(), httpContext, options, jsonTypeInfo); } - return ExecuteAwaited(valueTask, httpContext, options); + return ExecuteAwaited(valueTask, httpContext, options, jsonTypeInfo); } - private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext, JsonSerializerOptions? options) + private static Task ExecuteTaskOfObject(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options) + static async Task ExecuteAwaited(Task task, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - await ExecuteAwaitedReturn(await task, httpContext, options); + await ExecuteAwaitedReturn(await task, httpContext, options, jsonTypeInfo); } if (task.IsCompletedSuccessfully) { - return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, options); + return ExecuteAwaitedReturn(task.GetAwaiter().GetResult(), httpContext, options, jsonTypeInfo); } - return ExecuteAwaited(task, httpContext, options); + return ExecuteAwaited(task, httpContext, options, jsonTypeInfo); } - private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions? options) + private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { // Terminal built ins if (obj is IResult result) @@ -2143,7 +2147,7 @@ private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, Js else { // Otherwise, we JSON serialize when we reach the terminal state - return WriteJsonResponse(httpContext.Response, obj, options, null); + return WriteJsonResponse(httpContext.Response, obj, options, jsonTypeInfo); } } @@ -2164,11 +2168,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); } @@ -2269,9 +2273,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); } @@ -2335,40 +2339,27 @@ private static Task WriteJsonResponseFast(HttpResponse response, T value, Jso private static Task WriteJsonResponse(HttpResponse response, T? value, JsonSerializerOptions? options, JsonTypeInfo? jsonTypeInfo) { - var runtimeType = value is null ? typeof(object) : value.GetType(); + var runtimeType = value?.GetType(); - if (jsonTypeInfo is not null && jsonTypeInfo.Type == runtimeType) + // 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 // relevant for us and will be handled by STJ, if needed. return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, jsonTypeInfo, default); } - // We cannot use JsonTypeInfo here yet, waiting for https://github.com/dotnet/runtime/issues/77051 - // What should happens if options or type info is null? - // var runtimeTypeInfo = GetJsonTypeInfo(options, runtimeType); - // return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, runtimeTypeInfo, default); - // Call WriteAsJsonAsync() with the runtime type to serialize the runtime type rather than the declared type // and avoid source generators issues. // https://github.com/dotnet/aspnetcore/issues/43894 // https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism - return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, runtimeType, options, default); - } - - private static JsonTypeInfo? GetJsonTypeInfo(JsonSerializerOptions? jsonSerializerOptions, Type type) - { - if (jsonSerializerOptions?.TypeInfoResolver != null) - { - if (!jsonSerializerOptions.IsReadOnly) - { - jsonSerializerOptions.MakeReadOnly(); - } - - return jsonSerializerOptions.GetTypeInfo(type); - } - - return null; + var runtimeTypeInfo = options.GetTypeInfo(runtimeType); + return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, runtimeTypeInfo, default); } private static NotSupportedException GetUnsupportedReturnTypeException(Type returnType) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index e4a77afabb1a..e646da615a7b 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -3122,7 +3122,34 @@ public async Task RequestDelegateWritesMembersFromChildTypesToJsonResponseBody_W var responseBodyStream = new MemoryStream(); httpContext.Response.Body = responseBodyStream; - var factoryResult = RequestDelegateFactory.Create(@delegate, new() { ServiceProvider = httpContext.RequestServices }); + var factoryResult = RequestDelegateFactory.Create(@delegate); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + var deserializedResponseBody = JsonSerializer.Deserialize(responseBodyStream.ToArray(), new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true + }); + + Assert.NotNull(deserializedResponseBody); + Assert.Equal("Write even more tests!", deserializedResponseBody!.Name); + Assert.Equal("With type hierarchies!", deserializedResponseBody!.Child); + } + + [Theory] + [MemberData(nameof(PolymorphicResult))] + public async Task RequestDelegateWritesMembersFromChildTypesToJsonResponseBody_WithJsonPolymorphicOptionsAndConfiguredJsonOptions(Delegate @delegate) + { + var httpContext = CreateHttpContext(); + httpContext.RequestServices = new ServiceCollection() + .AddSingleton(LoggerFactory) + .AddSingleton(Options.Create(new JsonOptions())) + .BuildServiceProvider(); + var responseBodyStream = new MemoryStream(); + httpContext.Response.Body = responseBodyStream; + + var factoryResult = RequestDelegateFactory.Create(@delegate, new RequestDelegateFactoryOptions { ServiceProvider = httpContext.RequestServices }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -3150,7 +3177,7 @@ public async Task RequestDelegateWritesJsonTypeDiscriminatorToJsonResponseBody_W var responseBodyStream = new MemoryStream(); httpContext.Response.Body = responseBodyStream; - var factoryResult = RequestDelegateFactory.Create(@delegate, new() { ServiceProvider = httpContext.RequestServices }); + var factoryResult = RequestDelegateFactory.Create(@delegate); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -3188,7 +3215,7 @@ public async Task RequestDelegateWritesAsJsonResponseBody_WithJsonSerializerCont var responseBodyStream = new MemoryStream(); httpContext.Response.Body = responseBodyStream; - var factoryResult = RequestDelegateFactory.Create(@delegate, new() { ServiceProvider = httpContext.RequestServices }); + var factoryResult = RequestDelegateFactory.Create(@delegate); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 65231aa3c2f3..6996e9f2a7ab 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -1,10 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; using System.Runtime.ExceptionServices; using System.Text; using System.Text.Encodings.Web; using System.Text.Json; +using System.Text.Json.Serialization.Metadata; namespace Microsoft.AspNetCore.Mvc.Formatters; @@ -21,6 +23,8 @@ public SystemTextJsonOutputFormatter(JsonSerializerOptions jsonSerializerOptions { SerializerOptions = jsonSerializerOptions; + jsonSerializerOptions.MakeReadOnly(); + SupportedEncodings.Add(Encoding.UTF8); SupportedEncodings.Add(Encoding.Unicode); SupportedMediaTypes.Add(MediaTypeHeaderValues.ApplicationJson); @@ -68,29 +72,27 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon var httpContext = context.HttpContext; - // Maybe we could use the jsontypeinfo overload but we need the untyped, - // waiting for https://github.com/dotnet/runtime/issues/77051 - - var declaredType = context.ObjectType; var runtimeType = context.Object?.GetType(); - var objectType = runtimeType ?? declaredType ?? typeof(object); + JsonTypeInfo? jsonTypeInfo = null; - if (declaredType is not null && - runtimeType != declaredType && - SerializerOptions.TypeInfoResolver != null && - SerializerOptions.GetTypeInfo(declaredType).PolymorphismOptions is not null) + if (context.ObjectType is not null) { - // Using declared type in this case. The polymorphism is not - // relevant for us and will be handled by STJ, if needed. - objectType = declaredType; + var declaredTypeJsonInfo = SerializerOptions.GetTypeInfo(context.ObjectType); + + if (declaredTypeJsonInfo.IsPolymorphicSafe() || context.Object is null || runtimeType == declaredTypeJsonInfo.Type) + { + jsonTypeInfo = declaredTypeJsonInfo; + } } + jsonTypeInfo ??= SerializerOptions.GetTypeInfo(runtimeType ?? typeof(object)); + var responseStream = httpContext.Response.Body; if (selectedEncoding.CodePage == Encoding.UTF8.CodePage) { try { - await JsonSerializer.SerializeAsync(responseStream, context.Object, objectType, SerializerOptions, httpContext.RequestAborted); + await JsonSerializer.SerializeAsync(responseStream, context.Object, jsonTypeInfo, httpContext.RequestAborted); await responseStream.FlushAsync(httpContext.RequestAborted); } catch (OperationCanceledException) when (context.HttpContext.RequestAborted.IsCancellationRequested) { } @@ -104,7 +106,7 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon ExceptionDispatchInfo? exceptionDispatchInfo = null; try { - await JsonSerializer.SerializeAsync(transcodingStream, context.Object, objectType, SerializerOptions); + await JsonSerializer.SerializeAsync(transcodingStream, context.Object, jsonTypeInfo); await transcodingStream.FlushAsync(); } catch (Exception ex) @@ -129,3 +131,26 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon } } } + +internal static class JsonSerializerExtensions +{ + public static bool IsPolymorphicSafe(this JsonTypeInfo jsonTypeInfo) + => jsonTypeInfo.Type.IsSealed || jsonTypeInfo.Type.IsValueType || jsonTypeInfo.PolymorphismOptions is not null; + + public static bool TryGetTypeInfo(this JsonSerializerOptions options, Type type, [NotNullWhen(true)] out JsonTypeInfo? jsonTypeInfo) + { + // This shouldn't happen when in AOT and will be controlled + // by a future feature switch. + if (options.TypeInfoResolver == null) + { + jsonTypeInfo = null; + return false; + } + + options.MakeReadOnly(); + + jsonTypeInfo = options.GetTypeInfo(type); + return true; + } +} + diff --git a/src/Mvc/Mvc.Core/src/JsonOptions.cs b/src/Mvc/Mvc.Core/src/JsonOptions.cs index 087b74849b0d..52a717e9b396 100644 --- a/src/Mvc/Mvc.Core/src/JsonOptions.cs +++ b/src/Mvc/Mvc.Core/src/JsonOptions.cs @@ -48,7 +48,8 @@ public class JsonOptions #pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code }; - [RequiresUnreferencedCode("Calls System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.DefaultJsonTypeInfoResolver()")] + [RequiresUnreferencedCode("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver might require types that cannot be statically analyzed and might need runtime code generation.")] + [RequiresDynamicCode("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver might require types that cannot be statically analyzed and might need runtime code generation. Enable EnsureJsonTrimmability feature switch for native AOT applications.")] private static IJsonTypeInfoResolver CreateDefaultTypeResolver() => new DefaultJsonTypeInfoResolver(); } diff --git a/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs b/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs index 1e8a0a0c666a..5fee454c939f 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs @@ -123,7 +123,7 @@ public async Task WriteResponseBodyAsync_Encodes() var outputFormatterContext = new OutputFormatterWriteContext( actionContext.HttpContext, new TestHttpResponseStreamWriterFactory().CreateWriter, - typeof(string), + typeof(object), content) { ContentType = new StringSegment(mediaType.ToString()), diff --git a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs index 2852a4991ca5..1d5d81c91093 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs @@ -199,41 +199,13 @@ public async Task WriteResponseBodyAsync_UsesJsonPolymorphismOptions() } [Fact] - public async Task WriteResponseBodyAsync_UsesRuntimeType_WhenTypeResolverIsNull() + public async Task WriteResponseBodyAsync_Throws_WhenTypeResolverIsNull() { // Arrange var jsonOptions = new JsonOptions(); jsonOptions.JsonSerializerOptions.TypeInfoResolver = null; - var formatter = SystemTextJsonOutputFormatter.CreateFormatter(jsonOptions); - var expectedContent = "{\"age\":99,\"name\":\"Person\",\"child\":null,\"parent\":null}"; - JsonPerson todo = new JsonPersonExtended() - { - Name = "Person", - Age = 99, - }; - - var mediaType = MediaTypeHeaderValue.Parse("application/json; charset=utf-8"); - var encoding = CreateOrGetSupportedEncoding(formatter, "utf-8", isDefaultEncoding: true); - - var body = new MemoryStream(); - var actionContext = GetActionContext(mediaType, body); - - var outputFormatterContext = new OutputFormatterWriteContext( - actionContext.HttpContext, - new TestHttpResponseStreamWriterFactory().CreateWriter, - typeof(JsonPerson), - todo) - { - ContentType = new StringSegment(mediaType.ToString()), - }; - - // Act - await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.GetEncoding("utf-8")); - - // Assert - var actualContent = encoding.GetString(body.ToArray()); - Assert.Equal(expectedContent, actualContent); + Assert.Throws(() => SystemTextJsonOutputFormatter.CreateFormatter(jsonOptions)); } private class Person diff --git a/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs index 9d333536a7d5..2067351335cd 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs @@ -56,4 +56,18 @@ static void ConfigureServices(IServiceCollection serviceCollection) [Fact] public override Task Formatting_PolymorphicModel() => base.Formatting_PolymorphicModel(); + + [Fact] + public async Task Formatting_PolymorphicModel_WithJsonPolymorphism() + { + // Arrange + var expected = "{\"$type\":\"DerivedModel\",\"address\":\"Some address\",\"id\":10,\"name\":\"test\",\"streetName\":null}"; + + // Act + var response = await Client.GetAsync($"/SystemTextJsonOutputFormatter/{nameof(SystemTextJsonOutputFormatterController.PolymorphicResult)}"); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + Assert.Equal(expected, await response.Content.ReadAsStringAsync()); + } } diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/SystemTextJsonOutputFormatterController.cs b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/SystemTextJsonOutputFormatterController.cs new file mode 100644 index 000000000000..287ffa90fd91 --- /dev/null +++ b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/SystemTextJsonOutputFormatterController.cs @@ -0,0 +1,37 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text.Json.Serialization; +using Microsoft.AspNetCore.Mvc; + +namespace FormatterWebSite.Controllers; + +[ApiController] +[Route("[controller]/[action]")] +[Produces("application/json")] +public class SystemTextJsonOutputFormatterController : ControllerBase +{ + [HttpGet] + public ActionResult PolymorphicResult() => new DerivedModel + { + Id = 10, + Name = "test", + Address = "Some address", + }; + + [JsonPolymorphic] + [JsonDerivedType(typeof(DerivedModel), nameof(DerivedModel))] + public class SimpleModel + { + public int Id { get; set; } + + public string Name { get; set; } + + public string StreetName { get; set; } + } + + public class DerivedModel : SimpleModel + { + public string Address { get; set; } + } +} From 3e136b4689dcc9097aa1b5606b2af69ac77606f1 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 3 Jan 2023 11:25:54 -0800 Subject: [PATCH 22/26] Adding JsonSerializerExtensions --- ...icrosoft.AspNetCore.Http.Extensions.csproj | 3 ++- .../SystemTextJsonOutputFormatter.cs | 25 +------------------ .../src/Microsoft.AspNetCore.Mvc.Core.csproj | 1 + .../Json}/JsonSerializerExtensions.cs | 0 4 files changed, 4 insertions(+), 25 deletions(-) rename src/{Http/Http.Extensions/src => Shared/Json}/JsonSerializerExtensions.cs (100%) diff --git a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj index 6c7b5a47df8a..748965a360a6 100644 --- a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj +++ b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj @@ -21,7 +21,8 @@ - + + diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index 6996e9f2a7ab..ef9308e5ff70 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -1,12 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics.CodeAnalysis; using System.Runtime.ExceptionServices; using System.Text; using System.Text.Encodings.Web; using System.Text.Json; using System.Text.Json.Serialization.Metadata; +using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Mvc.Formatters; @@ -131,26 +131,3 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon } } } - -internal static class JsonSerializerExtensions -{ - public static bool IsPolymorphicSafe(this JsonTypeInfo jsonTypeInfo) - => jsonTypeInfo.Type.IsSealed || jsonTypeInfo.Type.IsValueType || jsonTypeInfo.PolymorphismOptions is not null; - - public static bool TryGetTypeInfo(this JsonSerializerOptions options, Type type, [NotNullWhen(true)] out JsonTypeInfo? jsonTypeInfo) - { - // This shouldn't happen when in AOT and will be controlled - // by a future feature switch. - if (options.TypeInfoResolver == null) - { - jsonTypeInfo = null; - return false; - } - - options.MakeReadOnly(); - - jsonTypeInfo = options.GetTypeInfo(type); - return true; - } -} - diff --git a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj index 36a376a97f50..477e4fb86aa1 100644 --- a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj +++ b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj @@ -33,6 +33,7 @@ Microsoft.AspNetCore.Mvc.RouteAttribute + diff --git a/src/Http/Http.Extensions/src/JsonSerializerExtensions.cs b/src/Shared/Json/JsonSerializerExtensions.cs similarity index 100% rename from src/Http/Http.Extensions/src/JsonSerializerExtensions.cs rename to src/Shared/Json/JsonSerializerExtensions.cs From bc162e6f5f9544f8b3e1f80ce9ff61cc97fcf401 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 3 Jan 2023 13:09:24 -0800 Subject: [PATCH 23/26] Fixing warnings --- src/Http/Http.Extensions/src/JsonOptions.cs | 4 ++-- ...spNetCore.Http.Extensions.WarningSuppressions.xml | 12 ++++++++++++ .../Formatters/SystemTextJsonOutputFormatterTest.cs | 2 +- 3 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index 09649a3a2764..e7710b04777b 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -27,9 +27,9 @@ public class JsonOptions // The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver // setting the default resolver (reflection-based) but the user can overwrite it directly or calling // .AddContext() -#pragma warning disable IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code +#pragma warning disable IL2026 // Suppressed in ILLink.Suppressions.LibraryBuild.xml TypeInfoResolver = CreateDefaultTypeResolver() -#pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code +#pragma warning restore IL2026 // Suppressed in ILLink.Suppressions.LibraryBuild.xml }; diff --git a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml new file mode 100644 index 000000000000..a8107b1cfb45 --- /dev/null +++ b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml @@ -0,0 +1,12 @@ + + + + + ILLink + IL2026 + member + M:Microsoft.AspNetCore.Http.Json.JsonOptions.#cctor + This warning is left in the product so developers get an ILLink warning when trimming an app, in future, only when Microsoft.AspNetCore.EnsureJsonTrimmability=false. + + + diff --git a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs index 1d5d81c91093..d7fc88d828e9 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/SystemTextJsonOutputFormatterTest.cs @@ -199,7 +199,7 @@ public async Task WriteResponseBodyAsync_UsesJsonPolymorphismOptions() } [Fact] - public async Task WriteResponseBodyAsync_Throws_WhenTypeResolverIsNull() + public void WriteResponseBodyAsync_Throws_WhenTypeResolverIsNull() { // Arrange var jsonOptions = new JsonOptions(); From 0bdfb96b515f50882bf4d4a90203af3fb779c328 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 5 Jan 2023 10:38:46 -0800 Subject: [PATCH 24/26] Apply suggestions from code review --- src/Http/Http.Extensions/src/JsonOptions.cs | 2 -- src/Mvc/Mvc.Core/src/JsonOptions.cs | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index e7710b04777b..b411a3196d94 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -39,8 +39,6 @@ public class JsonOptions /// public JsonSerializerOptions SerializerOptions { get; } = new JsonSerializerOptions(DefaultSerializerOptions); - [RequiresUnreferencedCode("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver might require types that cannot be statically analyzed and might need runtime code generation.")] - [RequiresDynamicCode("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver might require types that cannot be statically analyzed and might need runtime code generation. Enable EnsureJsonTrimmability feature switch for native AOT applications.")] private static IJsonTypeInfoResolver CreateDefaultTypeResolver() => new DefaultJsonTypeInfoResolver(); } diff --git a/src/Mvc/Mvc.Core/src/JsonOptions.cs b/src/Mvc/Mvc.Core/src/JsonOptions.cs index 52a717e9b396..344c3e641856 100644 --- a/src/Mvc/Mvc.Core/src/JsonOptions.cs +++ b/src/Mvc/Mvc.Core/src/JsonOptions.cs @@ -48,8 +48,6 @@ public class JsonOptions #pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code }; - [RequiresUnreferencedCode("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver might require types that cannot be statically analyzed and might need runtime code generation.")] - [RequiresDynamicCode("System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver might require types that cannot be statically analyzed and might need runtime code generation. Enable EnsureJsonTrimmability feature switch for native AOT applications.")] private static IJsonTypeInfoResolver CreateDefaultTypeResolver() => new DefaultJsonTypeInfoResolver(); } From 71bb5b65c46b6433f9e85dc44829a4be2d4477c4 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 5 Jan 2023 10:47:49 -0800 Subject: [PATCH 25/26] Updating suppression --- src/Http/Http.Extensions/src/JsonOptions.cs | 6 ++---- ...osoft.AspNetCore.Http.Extensions.WarningSuppressions.xml | 4 ++-- src/Mvc/Mvc.Core/src/JsonOptions.cs | 3 --- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index b411a3196d94..77489f9ffef5 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics.CodeAnalysis; using System.Text.Encodings.Web; using System.Text.Json; using System.Text.Json.Serialization.Metadata; @@ -27,10 +26,7 @@ public class JsonOptions // The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver // setting the default resolver (reflection-based) but the user can overwrite it directly or calling // .AddContext() -#pragma warning disable IL2026 // Suppressed in ILLink.Suppressions.LibraryBuild.xml TypeInfoResolver = CreateDefaultTypeResolver() -#pragma warning restore IL2026 // Suppressed in ILLink.Suppressions.LibraryBuild.xml - }; // Use a copy so the defaults are not modified. @@ -39,6 +35,8 @@ public class JsonOptions /// public JsonSerializerOptions SerializerOptions { get; } = new JsonSerializerOptions(DefaultSerializerOptions); +#pragma warning disable IL2026 // Suppressed in Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml private static IJsonTypeInfoResolver CreateDefaultTypeResolver() => new DefaultJsonTypeInfoResolver(); +#pragma warning restore IL2026 // Suppressed in Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml } diff --git a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml index a8107b1cfb45..70f9e571648c 100644 --- a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml +++ b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml @@ -1,11 +1,11 @@ - + ILLink IL2026 member - M:Microsoft.AspNetCore.Http.Json.JsonOptions.#cctor + M:Microsoft.AspNetCore.Http.Json.JsonOptions.CreateDefaultTypeResolver This warning is left in the product so developers get an ILLink warning when trimming an app, in future, only when Microsoft.AspNetCore.EnsureJsonTrimmability=false. diff --git a/src/Mvc/Mvc.Core/src/JsonOptions.cs b/src/Mvc/Mvc.Core/src/JsonOptions.cs index 344c3e641856..834c758bb9dc 100644 --- a/src/Mvc/Mvc.Core/src/JsonOptions.cs +++ b/src/Mvc/Mvc.Core/src/JsonOptions.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Serialization.Metadata; using Microsoft.AspNetCore.Mvc.Formatters; @@ -43,9 +42,7 @@ public class JsonOptions // The JsonSerializerOptions.GetTypeInfo method is called directly and needs a defined resolver // setting the default resolver (reflection-based) but the user can overwrite it directly or calling // .AddContext() -#pragma warning disable IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code TypeInfoResolver = CreateDefaultTypeResolver() -#pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code }; private static IJsonTypeInfoResolver CreateDefaultTypeResolver() From 534c62cdb82a71147742b15a83da0025c45b16ed Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 5 Jan 2023 13:00:20 -0800 Subject: [PATCH 26/26] Adding IL3050 --- src/Http/Http.Extensions/src/JsonOptions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Http/Http.Extensions/src/JsonOptions.cs b/src/Http/Http.Extensions/src/JsonOptions.cs index 77489f9ffef5..ad718418c09e 100644 --- a/src/Http/Http.Extensions/src/JsonOptions.cs +++ b/src/Http/Http.Extensions/src/JsonOptions.cs @@ -36,7 +36,9 @@ public class JsonOptions public JsonSerializerOptions SerializerOptions { get; } = new JsonSerializerOptions(DefaultSerializerOptions); #pragma warning disable IL2026 // Suppressed in Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml +#pragma warning disable IL3050 // Calling members annotated with 'RequiresDynamicCodeAttribute' may break functionality when AOT compiling. private static IJsonTypeInfoResolver CreateDefaultTypeResolver() => new DefaultJsonTypeInfoResolver(); +#pragma warning restore IL3050 // Calling members annotated with 'RequiresDynamicCodeAttribute' may break functionality when AOT compiling. #pragma warning restore IL2026 // Suppressed in Microsoft.AspNetCore.Http.Extensions.WarningSuppressions.xml }