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

Add support for listing available OpenAPI documents #3263

Merged

Conversation

rassilon
Copy link
Contributor

@rassilon rassilon commented Feb 16, 2025

Pull Request

  • ISwaggerProvider/IAsyncSwaggerProvider: Add GetSwaggerDocumentNames method and implementation (and test coverage). This fixes Expose a way to get the list of documents on ISwaggerProvider #472.
  • SwaggerUi: Add options to control the behavior of a new /swagger/documentUrls route that just returns the JSON version of ConfigOption.Urls. You can see this working in the MultipleVersions website.

Details on the issue fix or feature implementation

ISwaggerProvider.cs: Add GetSwaggerDocumentNames method.
IAsyncSwaggerProvider.cs: ditto
Inclucled new public methods in PublicAP.Unshipped.txt
SwaggerGenerator.cs: Implement GetSwaggerDocumentNames.
SwaggerUIJsonSerializationContext: Added List due to various issues with just using IEnumerable.
SwaggerUIMiddleware.cs: Implement /swagger/documentUrls route.
SwaggerUIOptions.cs: Add a new bool property to turn on the documentUrls route and the path to the documentUrls route.
SwaggerUIOptionsExtensions.cs: Add an extension method to set the new bool property on SwaggerUIOptions.
SwaggerGeneratorTests.cs: Add GetSwaggerDocumentNames testing coverage to a pre-existing test case.
MultipleVersions/Startup.cs: Enable the documentUrls route.

I need to automate broad security testing (both positive and negative) across all REST APIs our multiple microservice product utilizes. However, there wasn't a way to ask (in an automated fashion) what all of the swagger document URLs are...

Therefore, this PR. I also decided to fix #472 while I was at it.

I'm happy to address any changes you'd like to have me make, so I can get this into a public release so that my product team can upgrade to this new version.

Additionally, all tests passed successfully and I made sure I could successfully retrieve the expected JSON from the MultipleVersions website.

Thanks!

@martincostello
Copy link
Collaborator

Thanks for your PR.

I've only skim read it for now, but I see that it adds members to an interface. This would be a breaking change, so even if we wanted to take that, it would require a major version bump, so isn't something that's going to happen quickly (certainly not as quickly as you seem to want it).

I would suggest looking into adding a new interface, which can be implemented by the same types as the interface you modified, then we could release that in a minor version as it would just be incremental new functionality.

@rassilon
Copy link
Contributor Author

Thanks for your PR.

I've only skim read it for now, but I see that it adds members to an interface. This would be a breaking change, so even if we wanted to take that, it would require a major version bump, so isn't something that's going to happen quickly (certainly not as quickly as you seem to want it).

I would suggest looking into adding a new interface, which can be implemented by the same types as the interface you modified, then we could release that in a minor version as it would just be incremental new functionality.

I'm happy do to that. Any preferences on the naming of the extra interface? It could be ISwaggerProvider2/IAsyncSwaggerProvider2 in a homage to all of the fun COM interface naming practices of old, or it could be something simple like ISwaggerDocumentInformation or something like that with the method name: "GetDocumentNames" or something?

Thanks for your quick response!

@rassilon
Copy link
Contributor Author

@martincostello , I've updated the PR by moving the method into a new ISwaggerDocumentInformation interface instead.
If you'd like a different name, please don't hesitate to let me know.

Thanks!

@rassilon
Copy link
Contributor Author

rassilon commented Feb 19, 2025

@martincostello, for giggles I also locally added Cli support for a new 'list' command. I tried to see if that would work with the TestFirst website, but since that website doesn't reference Swagger/SwaggerGen that idea didn't pan out since it uses pre-generated json or imported json swagger documents.

I also thought about adding a test for the documentUrls route to TesFirst/TestFirst.IntegrationTests but it looks like there's not any easy helpers in this area atm. I can add that if you'd like as well. Just use HttpClient directly maybe?

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 44.44444% with 10 lines in your changes missing coverage. Please review.

Project coverage is 83.52%. Comparing base (19c9dd2) to head (efbc135).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
...buckle.AspNetCore.SwaggerUI/SwaggerUIMiddleware.cs 30.76% 9 Missing ⚠️
...re.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3263      +/-   ##
==========================================
- Coverage   83.74%   83.52%   -0.23%     
==========================================
  Files          76       76              
  Lines        3192     3210      +18     
  Branches      553      556       +3     
==========================================
+ Hits         2673     2681       +8     
- Misses        519      529      +10     
Flag Coverage Δ
Linux 83.52% <44.44%> (-0.23%) ⬇️
Windows 83.52% <44.44%> (-0.23%) ⬇️
macOS 83.52% <44.44%> (-0.23%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* ISwaggerProvider/IAsyncSwaggerProvider: Add GetSwaggerDocumentNames method and implementation (and test coverage). This fixes domaindrivendev#472.
* SwaggerUi: Add options to control the behavior of a new /swagger/documentUrls route that just returns the JSON version of ConfigOption.Urls.
  You can see this working in the MultipleVersions website.
* Renamed new interface to ISwaggerDocumentMetadataProvider.cs
* SwaggerGeneerator: return IList<string> from new method, and simplify to expression.
* SwaggerUIJsonSerliazerContext.cs: Put List<UrlDescriptor> after InterceptorFunctions.
* SwaggerUIOptions(Extensions): Tried to clarify documentation and update naming based on review.
* MultipleVersions\Startup.cs: Applied extension method rename.
@rassilon rassilon force-pushed the minor-swaggerImprovements branch from 6135649 to 7ac0fa9 Compare February 20, 2025 00:07
@martincostello martincostello added this to the v7.3.0 milestone Feb 26, 2025
Rename method missed from previous commit.
@martincostello martincostello changed the title Add 2 new bits of functionality: Add support for listing available OpenAPI documents Feb 26, 2025
@martincostello martincostello enabled auto-merge (squash) February 26, 2025 11:00
@martincostello martincostello added the version-minor A change suitable for release in a minor version label Feb 26, 2025
@martincostello martincostello merged commit b84ea3b into domaindrivendev:master Feb 26, 2025
9 checks passed
@martincostello
Copy link
Collaborator

Thanks for you contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version-minor A change suitable for release in a minor version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose a way to get the list of documents on ISwaggerProvider
3 participants