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.55k stars 10.05k forks source link

Enhance OpenAPI support in Minimal API with ProducesDefault for default responses #58723

Open sander1095 opened 3 weeks ago

sander1095 commented 3 weeks ago

Summary

Allow developers to set the default response for OpenAPI operations with the Produces() family of extensions methods from Microsoft.AspNetCore.OpenApi to achieve parity with MVC's [ProducesResponseType].

Motivation and goals

Being able to set the default response for an operation is useful as it demonstrates the default response for unexpected/undocumented operation responses. A common response type for default is ProblemDetails, as this usually is the standard type for API errors.

Proper use of default is also very important for API client generation, because API Client generators can then ensure that any other errors would always be typed to a specific type like ProblemDetails, which is a common pattern for error responses in REST APIs. I generate API clients quite often, and often run into issues where undocumented/unexpected error codes aren't deserialized into a correct type because of a missing default response type.

Setting default is easy to do with the MVC approach:

Click here to see the MVC approach ```csharp [HttpGet("{id:int:min(1)}")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] [ProducesDefaultResponseType)] public ActionResult GetTalk(int id) { // Code here } ``` which could result in the following OpenAPI document (Parts like problem details and other irrelevant code has been removed for brevity): ```yaml paths: /talks/{id}: get: operationId: "GetTalk" parameters: - name: "id" in: "path" required: true schema: type: "integer" responses: '200': content: application/json: schema: $ref: "#/components/schemas/Talk" '403': # stuff '404': # stuff 'default': # stuff ```

You can already use [ProducesDefaultResponseType] with Minimal API's, but it's not as discoverable, nor as much as a first-class citizen, as the Produces family of methods from the Microsoft.AspNetCore.OpenApi package:

Attributes with Minimal API:

app.MapGet("/todos/{id}",
[ProducesResponseType<Talk>(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status403Forbidden)]
[ProducesResponseType(StatusCodes.Status404NotFound)]
[ProducesDefaultResponseType]
async (TodoDb db, int id) => /* Code here */)

Extension methods with Minimal API and default:

app.MapGet("/todos/{id}", async (TodoDb db, int id) => /* Code here */)
  .Produces<Todo>(StatusCodes.Status200OK)
  .ProducesProblem(StatusCodes.Status403Forbidden)
  .ProducesProblem(StatusCodes.Status404NotFound)

There is no way to set the default response for an operation using extension methods. For this, you'd need an Operation Filter or an attribute, which doesn't mix very well with the extension methods. Also, ProducesDefaultResponseType is of the MVC namespace, which might be considered odd when using the Minimal API approach.

Extension methods with Minimal API and attribute for default:

app.MapGet("/todos/{id}", 
  [ProducesDefaultResponseType]
  async (TodoDb db, int id) => /* Code here */)
  .Produces<Todo>(StatusCodes.Status200OK)
  .ProducesProblem(StatusCodes.Status403Forbidden)
  .ProducesProblem(StatusCodes.Status404NotFound)

Therefore, I'd like to add something like ProducesDefault to the Microsoft.AspNetCore.OpenApi package to allow developers to set the default response for an operation using extension methods.

In scope

  1. Add a ProducesDefault extension method to the Microsoft.AspNetCore.OpenApi package.
    1. Perhaps this defaults to ProblemDetails?
    2. It should also allow for a content type and schema to be set.
  2. This would work on specific endpoints.
  3. This could work on endpoint groups, too.
  4. Perhaps this should be enabled by default for all applications, as you usually want this enabled for every endpoint, decreasing the amount of boilerplate code needed to get this to work.
    1. Consider allowing a developer to configure the default settings in AddOpenAPI, like setting the content type and/or schema (like problem details or something else) globally.

Out of scope

None come to mind right now.

Risks / unknowns

One risk is considering my other issue of adding Description to the Produces family of methods: #57963, as this would be more ergonomic than current options.

I've created a design proposal for this here: #58724 , but there is an issue of source incompatibility as a string description would clash with the string contentTypes, params string[] additionalContentTypes parameters of the Produces() family of methods.

If that design proposal is accepted, we should consider how to make ProducesDefault work with Description as well.

This is important, as I recently implemented setting Description on ProducesResponseType and that family of attributes for MVC (#55656), so we should try to decrease the amount of friction for adding better Description support to Minimal API's, too.

Examples

Setting the default response for an operation

app.MapGet("/todos/{id}", 
  async (TodoDb db, int id) => /* Code here */)
  .Produces<Todo>(StatusCodes.Status200OK)
  .ProducesProblem(StatusCodes.Status403Forbidden)
  .ProducesProblem(StatusCodes.Status404NotFound)
  .ProducesDefault();
  //.ProducesDefault<CustomApiErrorType>();
  //.ProducesDefault<CustomApiErrorType>("application/xml");

Setting the default response for an endpoint group

app.MapGroup("/todos").MapTodosApi().ProducesDefault();

Customizing global default response

builder.Services.AddOpenApi(x => x.DefaultResponseType = typeof(CustomApiErrorClass));
sander1095 commented 3 weeks ago

Hi @martincostello ! How nice to see you here :)

martincostello commented 3 weeks ago

Why was the design proposal label removed?

Mistake - I meant to remove the deprecated webframeworks label.

What's the next step?

The team will look at it.