Stop Calling JSON GetTypeInfo with the runtime type

[eerhardt]

In both the RDF and RDG we have code like the following:

aspnetcore/src/Shared/RouteHandlers/ExecuteHandlerHelper.cs

Lines 36 to 53 in 0b10e13

	public static Task WriteJsonResponseAsync<T>(HttpResponse response, T? value, JsonSerializerOptions options, JsonTypeInfo<T> jsonTypeInfo)
	{
	var runtimeType = value?.GetType();
	if (jsonTypeInfo.IsValid(runtimeType))
	{
	// 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);
	}
	// 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://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism
	var runtimeTypeInfo = options.GetTypeInfo(runtimeType);
	return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, runtimeTypeInfo, default);
	}

This doesn't work with the unspeakable types behavior that was added to System.Text.Json - dotnet/runtime#83631. In particular, GetTypeInfo(runtimeType) doesn't work when using unspeakable types.

We started using the "runtimeType" back in .NET Core 3.0 (bed3542) when we switched from Json.Net to System.Text.Json and wanted to maintain the polymorphic serialization behavior of Json.Net (ex. if the "declared type" was a base type, and the endpoint returned a derived type, we want to serialize all the derived properties). This strategy was adopted by Minimal APIs as well, when they were introduced.

This approach was then changed early in .NET 8 in order to support "System.Text.Json Polymorphism" features that were added in .NET 7. (See #45405). The strategy used by that PR was: "when a JsonPolymorphismOptions is detected, uses the declared type's JsonTypeInfo to call the serializer." When it isn't detected, use the "runtimeType"'s JsonTypeInfo to maintain the above polymorphic serialization behavior to not break backwards compatibility.

In order to support all these scenarios, we need to tweak our JSON serialization strategy. After consulting with @brunolins16 and @eiriktsarpalis, the strategy we have settled on is:

1. When a JsonPolymorphismOptions is detected (or when polymorphism is not possible - ex. ValueType, sealed, etc), use the declared type's JsonTypeInfo to call the serializer.
   • This is no change to what we do today in .NET 8.
2. When the above rule doesn't work, instead of using the "runtimeType", we will serialize the value "as object" - i.e. JsonSerializer.Serialize<object>(value, options).
   • This will allow System.Text.Json to control the behavior of serialization. It will allow to fix issues like:
     • "unspeakable type support" Add source gen support for unspeakable types runtime#82457
     • Polymorphic Type Resolving System.Text.Json Polymorphic Type Resolving Issue runtime#77532
     • Along with keeping the above behavior consistent with Json.Net before .NET Core 3.0.

A drawback to this strategy is that JsonSerializer.Serialize<object>(value, options) is not guaranteed to be trim/AOT-compatible, and thus it is annotated as incompatible. We will need to suppress these warnings in ASP.NET, which is OK because of the other mechanisms we are adding to support trimming/AOT - dotnet/runtime#83279.

However, we can't do this until the JSON Trimming feature switch is merged, or we will regress the AOT size and warnings. We may also need to wait for System.Text.Json Polymorphic Type Resolving Issue.

[brunolins16]

If the polymorphism behavior is now cover inside STJ and the call to the GetTypeInfo is not needed anymore, should this also be removed?

aspnetcore/src/Http/Http.Extensions/src/JsonOptions.cs

Line 28 in c6c2d95

TypeInfoResolver = TrimmingAppContextSwitches.EnsureJsonTrimmability ? null : CreateDefaultTypeResolver()

[eerhardt]

If the polymorphism behavior is now cover inside STJ and the call to the GetTypeInfo is not needed anymore, should this also be removed?

We will still have other calls to GetTypeInfo, which I believe are still needed. For example:

aspnetcore/src/Http/Http.Extensions/src/RequestDelegateFactory.cs

Line 1141 in 0b10e13

var jsonTypeInfo = factoryContext.JsonSerializerOptions.GetReadOnlyTypeInfo(returnType);

[captainsafia]

I can take a look at this and also make sure that we emit the same invocations between RDF and RDG. Right now they are functionally the same, but syntactically different.

[eerhardt]

@captainsafia - I can take this. I have a handful (3 maybe?) JSON changes in the works:

1. react to Add a JsonSerializerOptions.TryGetTypeInfo method. runtime#84411 (comment)
2. Remove our feature switch and instead use Implement the JsonSerializer.IsReflectionEnabledByDefault feature switch runtime#83844 (comment)
3. This change.

[captainsafia]

@eerhardt Awesome! Is it OK to put this in the preview4 milestone?

[eerhardt]

Done.