buvinghausen / Swashbuckle.NodaTime.AspNetCore

Easily configure Swashbuckle to generate correct documentation for NodaTime types.
MIT License
3 stars 7 forks source link

Consider redesigning the library to use System.Text.Json and IOptions<JsonOptions> #5

Open nathan-alden-sr opened 2 years ago

nathan-alden-sr commented 2 years ago

I'm using the new ASP.NET 6 template for an API. ConfigureForNodaTime is throwing here:

builder.Services.AddSwaggerGen(
    options =>
    {
        options.SwaggerDoc(
            "v1",
            new OpenApiInfo
            {
                Title = "Web API",
                Version = "v1"
            });

        options.ConfigureForNodaTime();
    });

I did some digging and it appears that this library uses Newtonsoft.Json. I do not want to use Newtonsoft.Json as System.Text.Json is the idiomatic way to do JSON serialization in .NET 6. Also, the fact that ConfigureForNodaTime requires me to pass an instance of a serializer is not idiomatic; optimally, your internal code should allow for injection of IOptions<JsonOptions>. For example, here's how I'm configuring JsonOptions in my app:

builder.Services.Configure<JsonOptions>(options => ConfigureJsonSerializerOptions(options.SerializerOptions));

Annoyingly, Microsoft did not unify JSON settings between the host builder APIs and ASP.NET. With ASP.NET, I have to configure JSON options in an additional spot:

builder.Services
    .AddControllers()
    .AddJsonOptions(options => ConfigureJsonSerializerOptions(options.JsonSerializerOptions))

If you do tackle this change, feel free to use my ASP.NET 6 bootstrap.

nathan-alden-sr commented 2 years ago

A note about JsonOptions: there are actually two: Microsoft.AspNetCore.Http.Json.JsonOptions and Microsoft.AspNetCore.Mvc.JsonOptions. Don't ask me why Microsoft did this. 😛 The Mvc one is apparently only used with controllers, so IMO you should use the Http one due to the new minimal APIs not using controllers. Or maybe support both, if that's possible.

buvinghausen commented 2 years ago

@nathan-alden-sr unfortunately the .NET team keeps being a PITA about System.Text.Json features to the point that I have not converted any of my existing apps over because every time I go look at Github issues on the subject they are still usually gnashing teeth on stupid things like supporting snake_case, kebab-case, etc even though those APIs are ubiquitous despite JSON using any naming convention than camelCase seems bad in my opinion.

With that said I don't have the time to invest in this project currently. I would say check out MicroElements as an alternative.