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

New WithOpenApi() methods breaks ASP.NET API Versioning #920

Closed
1 task done
joaofbantunes opened this issue Nov 20, 2022 · 5 comments
Closed
1 task done

New WithOpenApi() methods breaks ASP.NET API Versioning #920

joaofbantunes opened this issue Nov 20, 2022 · 5 comments
Assignees
Labels

Comments

@joaofbantunes
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

When using the new WithOpenApi() methods, from the Microsoft.AspNetCore.OpenApi package, ASP.NET API Versioning seems to be affected and some features misbehave.

A couple of examples I've noticed:

  1. The name passed in MapApiGroup isn't used to group the endpoints in the Swagger UI
  2. Non-deprecated version is shown as deprecated

A couple of visual examples:

image
image

Expected Behavior

ASP.NET API Versioning features working the same, regardless of WithOpenApi usage.

Steps To Reproduce

To reproduce, all I did was use the MinimalOpenApiExample available in the examples, then:

  • Added the Microsoft.AspNetCore.OpenApi package:
<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="7.0.0" />
  • Called WithOpenApi in one of the endpoints:
ordersV1.MapGet( "/{id:int}", ( int id ) => new OrderV1() { Id = id, Customer = "John Doe" } )
    .WithOpenApi()
    .Produces<OrderV1>()
    .Produces( 404 );

Complete repro example: https://github.com/joaofbantunes/AspNetApiVersioningWithOpenApiRepro

Exceptions (if any)

No response

.NET Version

7.0.100

Anything else?

Apologies if it's "too soon", as I'm reporting this while testing with ASP.NET API Versioning 7.0.0-preview.2, and maybe you're aware of the issue just haven't got to it yet. Just in case, as I was testing, thought of reporting.

Also, not sure if this is something fixable here, or misbehavior of Microsoft.AspNetCore.OpenApi?

@commonsensesoftware
Copy link
Collaborator

It's definitely not "too early". This is exactly the type of feedback I'm looking for and is why there is a preview. Honestly, this is the ammunition I'm looking for. The ASP.NET team has been trying to say the API Explorer isn't important anymore and everything will focus directly on OpenAPI 😞 . API Versioning doesn't directly care about OpenAPI or use any of its libraries. It provides exploration extensions that allow it to be used in OpenAPI and many other places.

This is happening for two reasons:

  1. WithOpenApi writes a tag which will be picked up with higher precedence by Swashbuckle
    a. Changing setup to .WithOpenApi(o => {o.Tags[0].Name = "Orders"; return o;}) will produce the right result
  2. The reason that you see Orders deprecated in both version is because the OpenAPI extension adds a single instance and value, but API Versioning varies it by API version as each document is generated
    a. Inside SwaggerDefaultValues.cs the bridge to Deprecated honors any existing value with |=; however, since there is a single instance it will be true in 0.9 and stay true in 1.0.
    b. The following change will produce the correct result operation.Deprecated = apiDescription.IsDeprecated(); (e.g. no |=)

I'd also be curious what else you want or need from WithOpenApi. I would expect most of the stuff to be there. Any details and scenarios you can provided will help my partner with the ASP.NET team to provide a better experience when mixing API Versioning with OpenAPI.

@joaofbantunes
Copy link
Author

I think my needs aren't particularly complex (though, sadly, I don't have a real world scenario to use at this moment, so I'm talking from memory about past projects).

We heavily relied on OpenAPI for API reviews (contract first), so we provided details about as many things as possible (i.e. summaries, descriptions samples and so on of operations, parameters and models).

In terms of versioning, we used the URL segment approach, and having the ability to switch between versions with that top right dropdown in the UI is nice.

@commonsensesoftware
Copy link
Collaborator

Just a quick update. I did find a way to retain the name specified in MapApiGroup (if provided) and you subsequently call WithOpenApi. That will make it into the final release. Thank you for trying it out and reporting this.

As for the issue with deprecation, I don't think there is good answer beyond what has already been shared as a workaround. The OpenAPI extensions are paired to an endpoint 1:1. This is problematic if you interleave two different versions on the same endpoint. The endpoint will appear in multiple groups. In most cases, they will be exactly the same, but deprecation is an edge case where it's supported in one version and deprecated in another. If you don't interleave versions that way (e.g. break them apart), it's not a problem.

@joaofbantunes
Copy link
Author

Just a quick update. I did find a way to retain the name specified in MapApiGroup (if provided) and you subsequently call WithOpenApi. That will make it into the final release. Thank you for trying it out and reporting this.

Great to hear 🙂.

As for the issue with deprecation, I don't think there is good answer beyond what has already been shared as a workaround. The OpenAPI extensions are paired to an endpoint 1:1. This is problematic if you interleave two different versions on the same endpoint. The endpoint will appear in multiple groups. In most cases, they will be exactly the same, but deprecation is an edge case where it's supported in one version and deprecated in another. If you don't interleave versions that way (e.g. break them apart), it's not a problem.

If I'm honest, the deprecation bits wasn't something I had considered before, just tried them because I noticed the library supported them (and seems useful). The scenario I reported is possibly not so common, so I guess the workaround is fine for folks (at least it is for me).

@commonsensesoftware
Copy link
Collaborator

The name issue is fixed in 7.0.0-rc.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants