Skip to content

Add JSON Serialize API to support ASP.NET polymorphic serialization #47973

New issue

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

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

Already on GitHub? Sign in to your account

Open
eerhardt opened this issue Apr 28, 2023 · 3 comments
Open

Add JSON Serialize API to support ASP.NET polymorphic serialization #47973

eerhardt opened this issue Apr 28, 2023 · 3 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc

Comments

@eerhardt
Copy link
Member

Background and Motivation

As described in #47548, ASP.NET has some internal logic around how it serializes objects to JSON for Minimal APIs and MVC. When fixing this issue in #47859, I noticed that this logic is spread out between:

  • MVC's SystemTextJsonOutputFormatter
  • HttpResultsHelper
  • ExecuteHandlerHelper (shared between Http.Extensions and Routing)
  • RequestDelegateGenerator generated code

The logic for all 4 looks like:

public static Task WriteJsonResponseAsync<T>(HttpResponse response, T? value, JsonTypeInfo<T> jsonTypeInfo)
{
var runtimeType = value?.GetType();
if (jsonTypeInfo.ShouldUseWith(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);
}
// Since we don't know the type's polymorphic characteristics
// our best option is to serialize the value as 'object'.
// call WriteAsJsonAsync<object>() 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
return response.WriteAsJsonAsync<object?>(value, jsonTypeInfo.Options);
}

with the "ShouldUseWith" logic:

public static bool HasKnownPolymorphism(this JsonTypeInfo jsonTypeInfo)
=> jsonTypeInfo.Type.IsSealed || jsonTypeInfo.Type.IsValueType || jsonTypeInfo.PolymorphismOptions is not null;
public static bool ShouldUseWith(this JsonTypeInfo jsonTypeInfo, [NotNullWhen(false)] Type? runtimeType)
=> runtimeType is null || jsonTypeInfo.Type == runtimeType || jsonTypeInfo.HasKnownPolymorphism();

If users want to have this same serialization behavior, they would have to write that same logic in their app/library. Also, we need to encode this logic in the source generator, which means it isn't as serviceable because if we need to fix a bug in it, the dev needs to rebuild their app to get the fix.

We should come up with an API that we can shared code between these 4 places, and allow customers to serialize objects with the same behavior as how MVC and Minimal APIs does.

Proposed API

TBD

Usage Examples

TBD

Alternative Designs

Risks

The name of the API is a risk at confusing people what the difference between our existing APIs and this one do.


cc @halter73 @captainsafia @mitchdenny @eiriktsarpalis

@eerhardt eerhardt added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Apr 28, 2023
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Apr 28, 2023
@eerhardt eerhardt added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Apr 28, 2023
@mitchdenny
Copy link
Member

Interesting comment about serviceability. If someone happens to be using RDG without native AOT I guess they could run into that issue. but in the case of RDG with native AOT the serviceability problem is inevitable.

@shivangnayar-dev
Copy link

can i be assigned this issue

@captainsafia
Copy link
Member

captainsafia commented Jun 27, 2023

@shivangnayar-dev At the moment, the issue isn't ready for implementation. The next step is to polish up the requirements here into an API proposal (fill in all the TBD sections) and take it through API review. You're welcome to help with that process but it would be helpful to have context into the background here...

I'd recommend checking out the help-wanted label for issues that are more shovel-ready.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Projects
None yet
Development

No branches or pull requests

5 participants