Swashbuckle.AspNetCore icon indicating copy to clipboard operation
Swashbuckle.AspNetCore copied to clipboard

Add 2 new bits of functionality:

Open rassilon opened this issue 9 months ago • 5 comments

Pull Request

  • ISwaggerProvider/IAsyncSwaggerProvider: Add GetSwaggerDocumentNames method and implementation (and test coverage). This fixes #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<UrlDescriptor> due to various issues with just using IEnumerable<UrlDescriptor>. 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!

rassilon avatar Feb 16 '25 02:02 rassilon

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.

martincostello avatar Feb 16 '25 07:02 martincostello

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 avatar Feb 16 '25 14:02 rassilon

@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 avatar Feb 16 '25 15:02 rassilon

@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?

rassilon avatar Feb 19 '25 19:02 rassilon

:warning: 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 :warning:
...re.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs 0.00% 1 Missing :warning:

:exclamation: 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%) :arrow_down:
Windows 83.52% <44.44%> (-0.23%) :arrow_down:
macOS 83.52% <44.44%> (-0.23%) :arrow_down:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Feb 19 '25 22:02 codecov-commenter

Thanks for you contribution!

martincostello avatar Feb 26 '25 11:02 martincostello

Thanks for you contribution!

Thanks for merging them in!

rassilon avatar Mar 04 '25 19:03 rassilon