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

Microsoft.AspNetCore.OpenApi: Support API versioning (required for Swashbuckle feature parity) #56314

Closed
1 task done
josundt opened this issue Jun 19, 2024 · 23 comments
Closed
1 task done
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. Status: No Recent Activity

Comments

@josundt
Copy link

josundt commented Jun 19, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

We currently use .NET 8.0/AspNetCore with Swashbuckle.AspNetCore in many APIs in our product ecosystem.
The current OpenApi documentations are very good with multiple API versions, good descriptions/comments, security defs/refs etc.

Swashbuckle.AspNetCore integrates with the package Asp.Versioning.Mvc.ApiExplorer to enable and simplify API versioning.
It augments SwashBuckle's capabilities (that we currently depend upon):

  • Tagging with Controllers/Action Methods with [ApiVersion("version")] attributes.
    These attributes add metadata describing in which version(s) the Controllers/ActonMethods are included.
  • Using special {version:apiVersion} route parameters that maps/has constraints to the tagged versions for Controller/ActionMethod.

Swashbuckle parses this metadata to enable splitting into multiple OpenApi documents - one JSON file per version.

I think the currently way of managing API versioning is simple and it works very well, and I hope the goal for Microsoft.AspNetCore.OpenApi is to get close to feature parity with Swashbuckle.AspNetCore and to make migration as easy as possible.

Describe the solution you'd like

This is a feature request to support API versioning, hopefully a similar way.
I don't know if you plan to integrate with Asp.Versioning.Mvc.ApiExplorer, but hope you can use similar concepts.

Can you ellaborate about the current plans to support API versioning?

Additional context

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jun 19, 2024
@captainsafia
Copy link
Member

@josundt Thanks for filling this issue!

Things actually work the other way around with respect to API versioning + OpenAPI in ASP.NET Core.

The Asp.Versioning.Mvc.ApiExplorer package that you referenced above integrates with ASP.NET Core's API explorer implementation. API explorer allows frameworks to expose an implementation of IApiDescriptionProvider that consumers can inspect to generate OpenAPI documentation.

All OpenAPI implementations in the ASP.NET ecosystem depend on API Explorer (NSwag, Swashbuckle, and our own Microsoft.AspNetCore.OpenApi) and can pick up the additional metadata that Asp.Versioning adds to the ApiExplorer interfaces in order to access versioning information.

With regard to how the new Microsoft.AspNetCore.OpenApi package integrates with Asp.Versioning, check out this PR on the eShop repo for an example of what the delta between the two implementations is.

Have you tried using Microsoft.AspNetCore.OpenApi in a project alongside Asp.Versioning and ran into issues?

@captainsafia captainsafia added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Jun 20, 2024
@captainsafia
Copy link
Member

Removing the No Recent Acitivity label to give @josundt a chance to comment on the question below before the bot auto-closes.

Have you tried using Microsoft.AspNetCore.OpenApi in a project alongside Asp.Versioning and ran into issues?

@josundt
Copy link
Author

josundt commented Jun 24, 2024

@captainsafia I'll try to perform a test migration of one of one of our APIs by looking at eShop repo today or tomorrow. I'll get back to you 👍

@dotnet-policy-service dotnet-policy-service bot added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Jun 24, 2024
@josundt
Copy link
Author

josundt commented Jun 26, 2024

@captainsafia
I have eventually been able to do a PoC of a .NET 9.0 (preview) API using the following NuGet packages:

  • Microsoft.AspNetCore.OpenApi
  • Asp.Versioning.Mvc.ApiExplorer
  • Microsoft.Extensions.ApiDescription.Server

After studying the eShop repo PR you mentioned, I managed to get a simple API running with two API versions (separate OpenAPI documents per version) and build-time OpenAPI documents generation.

You can see my small sample project here.

Findings/feedback:

  1. Schema duplication
    SwashBuckle added all complex type (and enum) schemas as single instance JSON objects in
    the OpenAPI document under #/components/schemas.
    HTTP operations that referred to the schemas through request parameters, requestBody or responses used JSON Pointer $ref references.
    Microsoft.AspNetCore.OpenApi currently serializes the same schema multiple times, thereby
    increasing the size of the OpenAPI document drastically. Even if the schema "multiplication" is
    not directly wrong logically speaking, the relationships between the shared
    Schemas and the different HTTP operations are no longer expressed. This is crucial for the various tools that
    generate code from OpenAPI documents. I hope there's a plan to support this on your roadmap.

  2. ProblemDetails Content-Type
    AspNetCore makes sure that non-success ProblemDetails responses at runtime get the
    response Content-Type header set to "application/problem+json".
    (You can test this through my sample api's ErrorDemoController using the
    AspNetCore.SampleOpenApi.http file).

    But: The swagger document reports "application/json" for such responses still, so there's
    inconsistency between OpenAPI documentation and the actual API behavior.

    FYI: This problem was the same with SwashBuckle, so with that package I needed to make a
    "OperationFilter" class to get the documentation right. I was able to do the same thing
    using an OperationTransformer in Microsoft.AspNetCore.OpenApi, but this should ideally be
    handled by the package out-of-the-box.

  3. Inclusion of XML comments for extended descriptions in OpenAPI documents
    SwashBuckle capabilities included extraction of XML comments from Controller action methods
    with parameters and returns and add this information to the OpenAPI documents. I can not see
    that this is a built-in feature of Microsoft.AspNetCore.OpenApi yet.

  4. Removing redundant response content-types
    In a standard AspNetCore API, OpenAPI documents' response status content will have 3 content-types;
    "text/plain", "application/json" and "text/json".
    In a well-designed JSON API, only "application/json" is desirable/required.
    I have added code to my repository to remove the redundant output formatters and media types
    here.
    I think this is typically what people want in their APIs, and I suggest that something similar
    is added to your documentation.

  5. Complexity of using OpenAPI with API versioning
    The code in my OpenApiOptionsExtensions.cs is more or less an exact copy of what I found in
    the eShop PR. This code is in my opinion quite low-level and it will probably be hard to get
    this right for consumers/customers. I spent quite some time myself before finding those magical
    lines that glues it all together. This needs be simplified somehow; what about including
    code in the package and calling the extension method
    UseDefaultVersioning(this OpenApiOptions options, ...) or similar?

  6. Documentation for API versioning
    The integration between Microsoft.AspNetCore.OpenApi and Asp.Versioning.Mvc.ApiExplorer
    needs better documentation on your documentation page
    before the package is made generally available for the .NET 9.0 release.

@josundt
Copy link
Author

josundt commented Jul 3, 2024

@captainsafia I have not received any feedback on this.
In the comment above I have described the currently missing features Microsoft.AspNetCore.OpenApi preventing a complete migration from SwashBuckle in our products.
Could you please share some information on whether or not each of these features are on the roadmap for the final .NET 9.0 release?

@martincostello
Copy link
Member

martincostello commented Jul 3, 2024

Schema duplication

This was implemented by #56175 and initial support will be in preview 6, with further changes in preview 7.

Other things you've mentioned such as XML documentation support and versioning are in the OpenAPI epic: #54598

@captainsafia
Copy link
Member

@josundt Apologies for the delay here! My focus has been on dev-related work over the past few weeks as we barrel closer to RC1. 😄

Schema duplication

I think @martincostello provided a great answer here. This space has been evolving in reaction to customer feedback. I'd recommend trying the latest nightly bits of the package to see where things currently are and sharing your feedback there.

ProblemDetails Content-Type

The issue that you're describing here is a limitation of the way that ProblemDetails responses are modeled in the ApiExplorer metadata. By default, if you are returning a ProblemDetails-type result from an API (either controller-based or minimal APIs), we will not add any metadata to the ApiExplorer that describes what that result would be.

This is primarily because we don't have a good way of inferring the HTTP status code that your error result is associated with, which is the primary key for response schemas in both ApiExplorer and OpenAPI. To fix this, you need to clue the famework in on what type of response your ProblemDetails is assocaited with by adding the metadata yourself (either via ProducesResponseType attributes or via a ProducesProblem method call on a minimal API).

As a converse to this, you'll observe that the ValidationProblem type in TypedResults does provide the correct metadata by default. That's because there, we can make an inference that your status code is 4xx based on the semantics of the platform.

TL;DR: The lack of specificity ProblemDetails here is intentional and is a deficiency in the underlying ApiExplorer layer and our ability to make automatic inferences about result types.

#52424 is another bug report around this in case you'd like to thumbs that up or chime in there.

Inclusion of XML comments for extended descriptions in OpenAPI documents

Yep, XML support isn't in yet. I've been playing around in this space though and hope to post an update on this issue (#39927) for it soon.

Removing redundant response content-types

I think the scenario that you are describing here is specific to controller-based APIs in MVC, where users have more flexibility to configure output formatters than they do in minimal APIs. In minimal APIs, the only content-types we'll automatically infer are application/json and text/plain.

In this case, ApiExplorer is doing its best to present an accurate representation of what outputs your API might return based on MVC's default output formatting configuration (which supports writing to multiple response content-types by default).

In this case, the action that you've taken is one way to clue in the system that you want to restrict the types of response content-types that are supported. Explicitly setting the Produces attribute on your handler is another way to do this.

With respect to documenting this officially, we do cover it in this section of the docs. I'm a little hesitant to include it in the top-level OpenAPI-docs given it is (a) MVC-specific and (b) more relevant to output formatting/content negotation than it is OpenAPI.

This needs be simplified somehow; what about including
code in the package and calling the extension method
UseDefaultVersioning(this OpenApiOptions options, ...) or similar?

Yep, this is a good observation. You might've noticed that the same transformation code is applied in the Asp.Versioning samples for both Swashbuckle and Microsoft.AspNetCore.OpenApi. I agree with you that it probably makes sense for the Asp.Versioning package to provide these as first-class APIs for both implementations so that users don't have to copy them into application code. Would you be able to file an issue on the Asp.Versioning repo for this?

@captainsafia captainsafia removed the Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. label Jul 10, 2024
@captainsafia captainsafia added this to the Discussions milestone Jul 10, 2024
@josundt
Copy link
Author

josundt commented Aug 14, 2024

Hello. I have just updated .NET and Microsoft.AspNetCore.OpenApi to 9.0 preview 7, and I see more improvements.
I have some new feedback related to the original findings/feedback comments (see list above):

  1. (Schema duplication)
    It looks like this is mostly sorted out now; schemas are defined under /#/components/schemas and all references under /#/paths are JSON pointer references to the definitions.

    Remarks: I noticed that .NET collection types (that end up as OpenAPI array types) are
    added as individual Schema definitions under /#/components/schemas.

    Link to schema in generated SampleAPI document

    I don't think it is desirable to have OpenAPI array types as individually defined schemas under /#/components/schemas.

    SwashBuckle did not add separate schema definitions for IEnumerable<T> (or IEnumerable) types, but did it for all
    other generic .NET types.

  2. (ProblemDetails Content-Type)
    I understood that my request to set the correct response content-type for ProblemDetails responses is not possible
    due to...

    [...] a limitation of the way that ProblemDetails responses are modeled in the ApiExplorer metadata.

    In my Sample API I have therefore added an IOperationTransformer that fixes this problem.

    Link to SampleAPI transformer

    FYI: I have earlier made a similar fix for SwashBuckle using their IOperationFilter API.

    Remarks: With the SwashBuckle "filters" API it was easy to find an OpenApiSchema's correlated .NET (CLR) type.
    With the Microsoft.AspNetCore.OpenApi "transformers" API I could not figure out a way to get the correlated .NET type.

    When writing code to detect responses that uses ProblemDetails in the content schema, I could with SwashBuckle simply
    check whether the response content schema's correlated .NET type is ProblemDetails.
    With Microsoft.AspNetCore.OpenApi, I need to check the OpenApiSchema's "shape" - check whether type == "object"
    and whether the properties has the expected property names/types (as you can see in the code).
    This is less precise, less performant and more complicated than with SwashBuckle.

    Would it be possible to add a CorrelatedType property (or similar) to the OpenApiSchema class to get the
    .NET CLR Type?
    Or is there already an existing way to get this correlated CLR type - it is just me not knowing how to get it?

    And: I think you should consider to include a transformer similar to the one I wrote with the NuGet package
    including an easy way of toggling it on/off. I believe most people would prefer if ProblemDetails responses had
    the correct content-type.

  3. (Inclusion of XML comments for extended descriptions in OpenAPI documents)
    Still waiting for this - required before I can migrate from SwashBuckle,

BTW: I have no new feedback for 4., 5. and 6.

@martincostello
Copy link
Member

Or is there already an existing way to get this correlated CLR type - it is just me not knowing how to get it?

In a schema transformer you access the OpenApiSchemaTransformerContext.JsonTypeInfo property. Here's an example.

@josundt
Copy link
Author

josundt commented Aug 14, 2024

@martincostello The example you sent me is from a schema transformer, not from an operation transformer.

I'm trying to change the content-type for an operation response*; that can only be done through operation or document transformers, correct?

@martincostello
Copy link
Member

Here's an operation transformer from the same repo - the information you're looking for can be found via OpenApiOperationTransformerContext.Description to get the ApiDescription instance, which will have lots of information in it. If you take a look at the repo you'll see I'm doing lots of type-based transforms.

@captainsafia captainsafia added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Aug 22, 2024
@josundt
Copy link
Author

josundt commented Aug 25, 2024

@martincostello I finally had time to have another look and this time figured it out. Thanks for guiding me in the right direction!👍

PS! I hope the way I map items in OpenApiOperationTransformerContext.Description.SupportedResponseTypes to responses in OpenApiOperation.Responses is correct link.

On another note; I'm awaiting feedback to my comment about the current behavior where individual schema definitions are created for collection types:

I don't think it is desirable to have OpenAPI array types as individually defined schemas under /#/components/schemas.
SwashBuckle did not add separate schema definitions for IEnumerable (or IEnumerable) types, but did it for all
other generic .NET types.

I assume there's a plan to do change this, right?

@martincostello
Copy link
Member

From what I've seen, arrays seem to be inlined if the elements aren't re-used anywhere else, but if there's commonality they're extracted out into their own schema reference so it goes from schema that is an array of items to array of schema reference.

@josundt
Copy link
Author

josundt commented Aug 28, 2024

@martincostello I think we are talking about two different things here, I am not talking about the array items, but the array itself.
In my test project, a schema called ArrayOfWeatherForecast is extracted as a schema definition (see this link).

Array schemas (the arrays, not the items) should in my opinion always be inlined even if there's "commonality".
Any .NET generic collection type (derived from IEnumerable<>) for the WeatherForecast class, I expect WeatherForecast to be extracted to /#/components/schemas, but the array schema for the IEnumerable<> should be inlined.
This is how it works with SwashBuckle.

PS! For other .NET generic types; types not derived from IEnumerable<T>, I do expect them to be extracted as schema definitions though. Example: .NET QueryResult<WeatherForecast> => OpenApi schema WeatherForecastQueryResult etc.

Swashbuckle consistently extracts the following .NET types as schema definitions regardless of "commonality"; -regardless of whether schemas are referenced single/multiple times:

  • Enums
  • Generic types not deriving from IEnumerable<>
  • Classes
  • Interfaces
  • (Potentially structs/records/similar constructs - have not tested/used it myself).

When considering which types should be "extracted" to /#/components/schemas and which should be inlined, the generated OpenAPI document becomes more well-structured, more optimized for human readability and document navigation when the decision is made by evaluating on the .NET type (see list above) rather than the references count ("commonality").

This is also how it works with SwashBuckle.

@martincostello
Copy link
Member

I think we are talking about the same thing. Here are some examples of the scenarios I was referring to from my snapshot tests.

A property which is an array of a schema: https://github.com/martincostello/aspnetcore-openapi/blob/365c761581473f47212d91bb85bfb51fc4993666/tests/TodoApp.Tests/OpenApiTests.Schema_Is_Correct_schemaUrl%3Dopenapi.verified.txt#L329-L335

A schema that is an array itself, whose items are a schema reference: https://github.com/martincostello/openapi-extensions/blob/3331a70264b302e855c42278dc699f4c370473e6/tests/OpenApi.Extensions.Tests/IntegrationTests.Schema_Is_Correct_For_Classes.verified.txt#L117-L122

A schema that is an array itself, whose items are defined inline and are not a reference: https://github.com/martincostello/openapi-extensions/blob/3331a70264b302e855c42278dc699f4c370473e6/tests/OpenApi.Extensions.Tests/IntegrationTests.Schema_Is_Correct_For_Records.verified.txt#L52-L73

I believe the difference in how they are rendered depends on whether the "shapes" are used more than once. When two identical schemas are found, they're extracted out to a common component for reference. You're right that it's different behaviour to Swashbuckle, as there as nothing it compared for equivalence with regard to the shape of the schemas - it's entirely driven by the runtime types used by the APIs implementation.

My understanding is that this is an intentional design decision, but the team can answer that.

@josundt
Copy link
Author

josundt commented Aug 28, 2024

@martincostello So to round things up, whether a schema should be extracted to /#/components/schemas is decided the following way:

  • Microsoft.AspNetCore.OpenApi: Whether the .NET type is referenced across more than one request param/body, response or other schema (latest .NET 9 preview release).

  • SwashBuckle.AspNetCore: Certain .NET types are extracted: .NET enums and complex types excl. collections (single vs multiple references not regarded).

I guess both solutions are technically correct, but while the Microsoft uses a traditional "circular reference handling" approach, SwashBuckle's generated documents become more deterministic and well-structured, with improved human readability and discoverability.

Example: I know that in a SwashBuckle generated document, the complete, alphabetically sorted list of complex type (+enum) schemas is found under /#/components/schemas.

These document characteristics are important for developers when they review validity/quality of OpenAPI documents generated for the AspNetCore APIs that they're writing, but also when they inspect OpenAPI documents for other APIs consumed by software that they're writing.

I hope you find my perspectives valuable and I'm hoping you may reconsider the approach.
I believe strongly that most developers will favor the structure of the documents created using SwashBuckle's approach.

@martincostello
Copy link
Member

I'm hoping you may reconsider the approach

This isn't something to pitch to me - I'm just a fellow member of the ASP.NET Core community and user-base.

@captainsafia Thoughts on the above?

@josundt
Copy link
Author

josundt commented Aug 30, 2024

@captainsafia Any feedback on my perspectives above?

I'd like to add one more argument: "Extracted" schemas are named which provides additional valuable information compared to "inlined" schemas. It makes it easier to correlate swagger schemas to their .NET complex/enum source types.

@sgluca
Copy link

sgluca commented Jan 14, 2025

I am migrating from Swashbuckle to the Microsoft.AspNetCore.OpenAPI package for generating API documentation.

In my project, I have an API structured with controllers, each assigned to a specific version using attributes. However, API versions are not explicitly defined in Program.cs. With Swashbuckle, the documentation for all versions was generated automatically at runtime.

When using Microsoft.AspNetCore.OpenAPI, it seems that I need to explicitly specify each version in the AddOpenAPI() method. Is there a way to configure the package to automatically detect and generate documentation for all versions in my project, without manually specifying them?

Thank you in advance for your support!

@mikekistler
Copy link
Contributor

@sgluca Are you using the Asp.Versioning package to implement API versioning? If so, then currently the answer to your question is no. There is no way (that I know of) for the built-in OpenAPI generation to detect the versions that are configured and generate the corresponding documents. I recently went through this process with the eShop demo app and I had to specify the API versions explicitly with separate calls to AddOpenApi.

It's possible this could change in the future but for right now that is the only solution available.

@mikekistler
Copy link
Contributor

@josundt Regarding the main subject of this issue, I think we have demonstrated with the eShop demo app that the .NET 9 OpenAPI generation does integrate with Asp.Versioning to support API versioning. Admittedly there as some aspects of this that could be improved but the basic functionality seems to be all there.

There were a number of other points raised here -- and thank you for all this feedback -- and most of these are either fixed before the 9.0 release (e.g. handling of collection schemas), will be fixed shortly (duplicate schemas), or tracked in other issues (content-types for problem details).

I'm going to mark this as "Needs Author Feedback" to give you an opportunity to point out anything that you think is not covered.

@mikekistler mikekistler added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Jan 25, 2025
Copy link
Contributor

Hi @josundt. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

Copy link
Contributor

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

@dotnet-policy-service dotnet-policy-service bot removed this from the Discussions milestone Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. Status: No Recent Activity
Projects
None yet
Development

No branches or pull requests

5 participants