dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.45k stars 10.03k forks source link

JsonSerializerContext not working with minimalApi #52088

Open dany28 opened 12 months ago

dany28 commented 12 months ago

Is there an existing issue for this?

Describe the bug

After configure Web API service to use generated version of JsonSerializerContext for serializing JSON, still using default serialization

Expected Behavior

Object returned by endpoint should be serialized using configured class derived from JsonSerializerContext with his configuration.

Steps To Reproduce

using System.Text.Json;
using System.Text.Json.Serialization;

var builder = WebApplication.CreateSlimBuilder(args);

builder.Services.ConfigureHttpJsonOptions(options =>
    options.SerializerOptions.TypeInfoResolverChain.Insert(0, AppJsonSerializerContext.Default));

var app = builder.Build();

app.MapGet("/json", () => new Person("Alex"));
app.MapGet("/string", () => JsonSerializer.Serialize(new Person("Alex"),AppJsonSerializerContext.Default.Person));

app.Run();

record Person(string FirstName)
{
}

[JsonSerializable(typeof(Person))]
[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.KebabCaseUpper)]
internal partial class AppJsonSerializerContext : JsonSerializerContext
{

}

endpoint /string (manually using AppJsonSerializerContext) returns:

{"FIRST-NAME":"Alex"}

but endpoint /json returns default serialized JSON, without AppJsonSerializerContext:

{
  "firstName": "Alex"
}

Exceptions (if any)

No response

.NET Version

8.0.100

Anything else?

Project configuration .csproj

<Project Sdk="Microsoft.NET.Sdk.Web">
    <PropertyGroup>
        <TargetFramework>net8.0</TargetFramework>
        <Nullable>enable</Nullable>
        <ImplicitUsings>enable</ImplicitUsings>
        <InvariantGlobalization>true</InvariantGlobalization>
    </PropertyGroup>
</Project>

I've tried debugging generated code, endpoint /string goes into generated implementation, but /json not

ChaosEngine commented 11 months ago

I am seeing the same when NOT providing any PropertyNamingPolicy attribute and default json generation (in MVC and minimal API) is misbehaving giving different results. Only proper when explicitly providing JsonSerializer.Serialize(new MyType() MyTypeContext.Default.MyType)

Same repro as above except commented out //[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.KebabCaseUpper)]

using System.Text.Json;
using System.Text.Json.Serialization;

var builder = WebApplication.CreateSlimBuilder(args);

builder.Services.ConfigureHttpJsonOptions(options =>
    options.SerializerOptions.TypeInfoResolverChain.Insert(0, AppJsonSerializerContext.Default));

var app = builder.Build();

app.MapGet("/json", () => new Person("Alex"));
app.MapGet("/string", () => JsonSerializer.Serialize(new Person("Alex"), AppJsonSerializerContext.Default.Person));

app.Run();

record Person(string FirstName)
{
}

[JsonSerializable(typeof(Person))]
//[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.KebabCaseUpper)]
internal partial class AppJsonSerializerContext : JsonSerializerContext
{

}

expecteed output would be {"FirstName":"Alex"} while it is {"firstName":"Alex"} (camelCase instead PascalCase)

Long story short: keeping previous code with many methods reading:

[Route(@"/getPosts/{blogId}")]
public async Task<ActionResult> GetPosts(int blogId)
{
    var lst = await _repo.GetPostsFromBlogAsync(blogId);

    return Json(lst);
}

would not work with added TypeInfoResolverChain.Insert(0, MyTypeContext.Default)); only chanign those metods to

[Route(@"/getPosts/{blogId}")]
public async Task<ActionResult> GetPosts(int blogId)
{
    var lst = await _repo.GetPostsFromBlogAsync(blogId);

    return Json(lst, ListPost_Context.Default.Options);
}

would bring behavior to old behavior but defeats the purpose of having single defined place for generated serialization contexts

dany28 commented 11 months ago

Yes, setting PropertyNamingPolicy is not necessary to how JsonSerializerContext is NOT working for minimalApi and MVC (different code for setting)

Kumima commented 11 months ago

Same here, when using parameter-binding if you provide the JsonSerializerContext, it will also cause binding fail, which means it does not even use the JsonSourceGenerationOptions. This is really serious, why is there no official feedback?

eiriktsarpalis commented 11 months ago

The JsonSourceGenerationOptions attribute maps configuration to the AppJsonSerializerContext .Default property added by the source generator. The configuration code in the repro is only copying the TypeInfoResolver component of the serialization configuration, in order to also get the desired behavior you would need to write something like

builder.Services.ConfigureHttpJsonOptions(options
{
    options.SerializerOptions.TypeInfoResolverChain.Insert(0, AppJsonSerializerContext.Default);
    options.SerializerOptions.PropertyNamingPolicy = AppJsonSerializerContext.Default.Options.PropertyNamingPolicy;
});

I discussed this with @captainsafia and it seems to me that an accelerator method that copies all configuration from a generated JsonSerializerContext to a JsonOptions might be warranted (although, N.B. this could create conflicts in applications where more than one generated JsonSerializerContext is being consumed).

Kumima commented 11 months ago

@eiriktsarpalis So if we have different PropertyNamingPolicy for different JsonSerializerContext, is there a way to apply multiple Naming Policies automatically based on Type. It will be more convenient, since we've defined the Policy through the context.

eiriktsarpalis commented 11 months ago

PropertyNamingPolicy is a global setting. If you're looking to apply naming policy that is scoped to a particular type or property you should consider using the JsonPropertyName attribute instead.

Kumima commented 11 months ago

@eiriktsarpalis

JsonSerializer.Serialize(new Person("Alex"), AppJsonSerializerContext.Default.Person)

Code like above will automaticly resolve the JsonSourceGenerationOptions defined through context. Can we have a way to resolve the JsonSerializerOptions automatically for type? It's just weird that we must configure a global Options although we've defined that through context. JsonSourceGenerationOptions becomes useless for this scenorio. Some time we don't want a global, and JsonPropertyNameAttribute is more verbose than JsonSourceGenerationOptions.

eiriktsarpalis commented 11 months ago

Can we have a way to resolve the JsonSerializerOptions automatically for type? It's just weird that we must configure a global Options although we've defined that through context.

But wouldn't you still need to pass a particular context to aspnetcore somehow? These are not tied to the type definition and are not necessarily unique for each type. There's been proposals for type-directed serialization contracts (see https://github.com/dotnet/runtime/issues/90787) but this is more of a long-term vision.

Kumima commented 11 months ago

The purpose to provide JsonSourceGenerationOptions to context is to set options for group-related objects. They can be grouped by library, functionality, or other purpose. Groups may differ, and global is not appropriate. Adjustment for individual type works, but group configuration brings convenient. I'm not totally sure why we may separate the JsonSerializerContext, isn't it that we need to group related objects? Configuration like [JsonSourceGenerationOptions(UseStringEnumConverter = true)] configures the enum policy for group.

So, I don't think it's totally related to type-directed serialization. And passing a particular context to aspnetcore is like we pass the group configuration, neither global nor individual type.

Anyway, thanks for your @eiriktsarpalis explanation, I can understand the work around for now. I just think if JsonSourceGenerationOptions works as I expect, it will be more convenient.

Kumima commented 11 months ago

@eiriktsarpalis I just remember my main purpose for this issue. The overload method like httpClient.PostAsJsonAsync("uri", requestObject, AppJsonSerializerContext.Default.RequestObject) will apply the JsonSourceGenerationOptions while asp.net core does not. This brings a huge inconsistency.

eiriktsarpalis commented 11 months ago

Fundamentally the issue stems from the fact that minimal APIs can't accept JsonTypeInfo parameters on a per-response basis. Instead this needs to be configured centrally. It is conceivable that this could be worked around with a JsonResult-like wrapper, e.g.

app.MapGet("/json", () => JsonResult.Create(new Person("Alex"), AppJsonSerializerContext.Default.Person));

Assuming we had an IJsonSerializable<T> interface with a static abstract factory returning a JsonTypeInfo<T> then the second argument could be elided:

app.MapGet("/json", () => JsonResult.Create(new Person("Alex")));
Kumima commented 11 months ago

@eiriktsarpalis It's not quite inconvenient for response, since there are overload methods as you mentioned. For auto parameter binding, I have to do things like this:

//Must read from request to apply JsonSourceGenerationOptions from JsonSerializerContext
app.MapPost("/json", () => 
{
    request = await context.Request.ReadFromJsonAsync(AppJsonSerializerContext.Default.Request);
    //manually handle bad request. e.g., return TypedResults.BadRequest() when deserializing failed` 
}

//expected: auto apply JsonSourceGenerationOptions from JsonSerializerContext
//asp.net core take over the BadRequest task during binding as default behavior
//for now, asp.net core always provide BadRequest since it does not use JsonSourceGenerationOptions. The binding failed.
app.MapPost("/json", ([FromBody] Request request)) => Handle(request))

//Another work around: Implement this for Object, but this introduces verbose.
public static ValueTask<T?> BindAsync(HttpContext context);
Kumima commented 2 months ago

https://github.com/dotnet/runtime/issues/107535 This one provides a explanation for this.