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.19k stars 9.93k forks source link

Automatically set OpenAPI endpoint metadata for minimal APIs #34544

Open DamianEdwards opened 3 years ago

DamianEdwards commented 3 years ago

Rather than having to manually specify the endpoint metadata associated with describing the response types a minimal API produces, it would be preferable to have these details automatically added based on what the minimal API actually does (its implemention) and/or by some convention based on how it's declared.

This is scoped to post-.NET 6 at this time, but could potentially be delivered before .NET 7 if it only involves an update in the tooling/SDK rather than an update in the ASP.NET Core shared framework.

ghost commented 3 years ago

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.

captainsafia commented 2 years ago

For a given endpoint, we need to be able describe the following attributes via OpenAPI:

Deriving request parameters (route params, query params, header params) is already done, given that these parameter types are easy to infer since they are all parseable from strings. We run into some issues with representing things like DateOnly and new primitive types from time to time, but there's no foundational issue here.

Deriving the request body is marginally harder than describing request parameters because we need to be able to determine the schema for a given property.

Deriving the response body is harder still since there can be multiple responses from a single endpoint and there are multiple facets to annotate as mentioned above (status code, content type, response schema).

For both request body and response body, the IEndpointMetadataProvider and IEndpointParameterMetadataProvider API provide vectors for setting this metadata, however, it's not automatic and requires user intervention.

Deriving authentication config will likely depend on the work done in https://github.com/dotnet/aspnetcore/issues/39761. IMO, this is probably the most important one for us to figure out at the moment given that there's really no way to configure this in the framework at the moment. We don't have a Produces or Accepts for authentication info.

With that in mind, we can either solve the problem for automatically setting authentication config separately from setting request/response body information or we can do user the same hammer for both problems.

https://github.com/dotnet/aspnetcore/issues/34543 is related to this work and lends itself to a "static analysis"-based approach for automatically determine the right annotations to put in for requests/responses. https://github.com/dotnet/aspnetcore/issues/39761 identifies the challenge with automatically generating auth annotations.

With the landscape laid out this way, I'm inclined to shake out the static analysis approach (analyzer/source generator/etc.) as a dual-purpose solution for both the authentication problem and the request/response problem. Also, the static analysis approach would provide a vector for solving https://github.com/dotnet/aspnetcore/issues/39927 (which has quite a few thumbs up!)

Oh, the 'automatically generating OperationId" problem is certainly a valid one to solve although, IMO, it's not as important as solving the other two until we make considerable headway in the client generation area.

DamianEdwards commented 2 years ago

Worth noting that we added TypedResults and union results via Results<TResult1, TResultN, ...> in .NET 7 to enable automatic setting of OpenAPI related metadata for response codes/bodies which at least partially addresses this issue (albeit with a required app code change).

captainsafia commented 2 years ago

Worth noting that we added TypedResults and union results via Results<TResult1, TResultN, ...> in .NET 7 to enable automatic setting of OpenAPI related metadata for response codes/bodies which at least partially addresses this issue (albeit with a required app code change).

Ah, good point! I had captured the metadata provider strategy above but not this one.

And this thread has made me realize that we have quite a few different ways to annotate responses. I've captured an item to make sure we document these in https://github.com/dotnet/aspnetcore/issues/43145.

captainsafia commented 2 years ago

As I've been playing around with the static analysis for OpenAPI generation strategy here, I'm noting the challenge of converging schema that can be deduced by static analysis and schema that must be derived by examining metadata.

Currently, we can derive annotation responses in the following ways (from lowest precedence to highest precedence):

Assuming we had a static analysis phase, we would be able to derive the following at compile-time:

And the following would be derived at runtime:

This flow accounts for premise, so we consider annotations from XML documents to be lower precedence than annotations from IEndpointMetadataProviders. Note: although the above is in reference to responses, the same hierarchy exists for requests.

DamianEdwards commented 2 years ago

Crazy thought, although not sure how valuable, perhaps annotations from IEndpointMetadataProvider could actually be captured at compile/build-time, similar to how the EF Core and scaffolding CLIs captures app/CI details. Not sure it gains anything in this scenario but .NET has a history of this style of extensibility in the past.

captainsafia commented 2 years ago

@DamianEdwards has been working on a scenario that leverages the new ProblemDetailsService to configure response types that are applied to all endpoints in an application or all endpoints in a group.

This is an inference problem that is a little closer to the "introspect auth" problem than it is the "introspect an endpoint" problem since it requires taking knowledge derived at the app-level and including it in each's groups annotation. OpenAPI uses the components property as a category in which these cross-cutting and reusable objects are stored (like response schemas, authentication schemes, etc).

This behavior exposes another notable gap in our OpenAPI-implementations which is that they are highly operation specific, we don't rely expose any primitives that allow the user to provide app-level annotations and generally rely on external dependencies for that. This has generally been a fine design choice, but the ProblemDetails case and the "introspect auth" scenario indicate that we might be outgrowing that restriction.

zyofeng commented 3 months ago

Hi guys

Not sure if this is the right place to report the issue, but in .net 8 minimal api, it seems the metadata provider is not working with FileContentHttpResult


        group.MapGet("test",  () =>
                TypedResults.File([])
            )

The following is produced in NSwag

"responses": {
      "200": {
          "description": ""
      }
  }

and I have to explicitly add the Produces() to make it work.

        group.MapPost("test",  () =>
                TypedResults.File([])
            )
            .Produces<FileContentResult>()
halter73 commented 3 months ago

FileContentHttpResult is not intended to add any metadata to your response. It does not implement IEndpointMetadataProvider because we don't know just based on the type whether the default content-type of application/octet-stream was overridden or even if the status code was changed from 200 before the FileContentHttpResult was returned. We decided to be very conservative and not infer more than we really know.

.Produces<FileContentResult>()

I don't think there's a reason to ever do that. Unless you want the OpenAPI doc to say that will return an application/json response with a "FileContents" property. I think you would want something like the following instead:

.Produces(200, contentType: "application/octet-stream")

zyofeng commented 3 months ago

According to Mozilla, "application/octet-stream" is for generic binary data whose true type is unknown, I feel like it's safe to infer it as a fallback since we still have the ability to overwrite it with a specific content type using Produces(200, xxxx) https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types

.Produces<FileContentResult>()

Generates the correct metadata as far as I can see. image