Skip to content
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

Allow Custom JsonSerializerOptions for Minimal Actions #35904

Open
Kahbazi opened this issue Aug 29, 2021 · 8 comments
Open

Allow Custom JsonSerializerOptions for Minimal Actions #35904

Kahbazi opened this issue Aug 29, 2021 · 8 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 feature-minimal-actions Controller-like actions for endpoint routing
Milestone

Comments

@Kahbazi
Copy link
Member

Kahbazi commented Aug 29, 2021

Background and Motivation

Add ability to have custom JsonSerializerOptions for serializing and deserializing in minimal actions.

Proposed API

namespace Microsoft.AspNetCore.Builder
{
     public static class DelegateEndpointRouteBuilderExtensions
     {
+        public static DelegateEndpointConventionBuilder Map(
+            this IEndpointRouteBuilder endpoints,
+            RoutePattern pattern,
+            Delegate handler,
+            JsonSerializerOptions options)
    }
}

namespace Microsoft.AspNetCore.Http
{
     public sealed class RequestDelegateFactoryOptions
     {
+      public JsonSerializerOptions JsonSerializerOptions { get; init; }
     }
}

Usage Examples

var jsonOptions = new JsonSerializerOptions
{
    PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
};

app.Map("\action", () => MyAction(), jsonOptions);

Alternative Designs

This could also be added via Endpoint metadata in order to prevent all extra overloads (MapGet, MapPost, ...), but the downside is that it would add an extra fetch from Endpoint.Metadata for serializing and deserializing.

public static class DelegateEndpointRouteBuilderExtensions
{
+   public static DelegateEndpointConventionBuilder WithJsonOptions(
+       this DelegateEndpointConventionBuilder builder,
+       JsonSerializerOptions options)
}

Risks

@Kahbazi Kahbazi added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Aug 29, 2021
@davidfowl
Copy link
Member

davidfowl commented Aug 29, 2021

No I don’t think we should do this. The options are:

  • configure JsonOptions globally
  • Use Results.Json and pass the options that way
  • Call WriteAsJsonAsync and pass the options that way

@Kahbazi
Copy link
Member Author

Kahbazi commented Aug 29, 2021

  • configure JsonOptions globally

I thought there were no static global options for System.Text.Json.

  • Use Results.Json and pass the options that way
  • Call WriteAsJsonAsync and pass the options that way

Fair enough. The only thing left is for serializing from body which I guess BindAsync could be used for that. Although the code would be simpler if JsonOption could be set via the proposed API.

@davidfowl I have nothing more to add, the issue could be closed. 🙂

@davidfowl
Copy link
Member

We should add a first class API to configure the JsonOptions. This isn't global for all calls, it's for calls to ReadAsJsonAsync and WriteAsJsonAsync

@Kahbazi
Copy link
Member Author

Kahbazi commented Aug 30, 2021

Ah I see what you're talking about.

private static JsonSerializerOptions ResolveSerializerOptions(HttpContext httpContext)
{
// Attempt to resolve options from DI then fallback to default options
return httpContext.RequestServices?.GetService<IOptions<JsonOptions>>()?.Value?.SerializerOptions ?? JsonOptions.DefaultSerializerOptions;
}

So should I update the proposal to this one?

public static class ServiceCollectionExtensions
{
    /// <summary>
    /// Configures JsonOptions for WriteAsJsonAsync and ReadFromJsonAsync.
    /// </summary>
    public static IServiceCollection ConfigureJsonOptions(this IServiceCollection services, Action<Microsoft.AspNetCore.Http.Json.JsonOptions> action)
    {
        return services.Configure(action);
    }
}

@ghost
Copy link

ghost commented Sep 1, 2021

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@rafikiassumani-msft rafikiassumani-msft added the feature-minimal-actions Controller-like actions for endpoint routing label Sep 1, 2021
@ghost
Copy link

ghost commented Dec 16, 2021

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@rafikiassumani-msft
Copy link
Contributor

rafikiassumani-msft commented Dec 28, 2021

@Kahbazi and @davidfowl We also have a namespace issue that we may need to address if we were to provide a first-class API to globally configure JsonSerializerOptions. Notice that the following Microsoft.AspNetCore.Http.Json.JsonOptions in your updated proposal is different from Microsoft.AspNetCore.Mvc.JsonOptions . This causes confusion to users who want to configure JsonOptions as the Http namespace one usually gets imported by the editor but may not achieve the intended user configuration objective.

@davidfowl Given the namespace issue, do we need to provide a first-class API to hide the code below and help avoid confusion?

  builder.Services.Configure<Microsoft.AspNetCore.Mvc.JsonOptions>(opts =>
  {
     //opts.JsonSerializerOptions.PropertyNamingPolicy = null;
     //More Configs if needed.
  
  });

@ghost
Copy link

ghost commented Oct 11, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@amcasey amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023
@captainsafia captainsafia modified the milestones: .NET 8 Planning, Backlog Mar 1, 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 feature-minimal-actions Controller-like actions for endpoint routing
Projects
No open projects
Status: No status
Development

No branches or pull requests

6 participants