openapi-tools / swagger-maven-plugin

Maven plugin to activate the Swagger Core library to generate OpenAPI documentation.
MIT License
70 stars 45 forks source link

Introducing the possibility to cut the basePath from Paths #79

Closed TheSesam closed 3 years ago

langecode commented 3 years ago

Thanks for the contribution. I'll take a look as soon as possible.

langecode commented 3 years ago

@TheSesam I have looked through your pull-request and quality wise I think it looks good. However, I am a little bit at ease about the use case here.

I surely understand you want the version (if you want to do URL based versioning - that is a whole discussion in itself) on the server path instead of the individual path of the endpoints. I am just wondering why you do not remove it from the @Path annotation on the class level?

It is also possible to add the server section to configuration generating the OpenAPI document - you have take a look at generate-mojo-full.xml which shows pretty much the complete configuration you can do. Alternative you can also add class or interface with the @OpenAPIDefinition somewhere among the scanned packages to do this configuration.

We intentionally wanted to keep this plugin as thin a wrapper around the Swagger classes as possible. While I do see some value in doing a generic "post processing" step I am also worried we might find a lot of other relevant cases and thus expand the plugin a lot.

If you cannot alter the @Path annotations on your classes or use the sort of "global" Swagger configuration there is also one other option for doing a post processing. You can create a ReaderListener implementation and put this into your scannes classpath. I created an example for you doing the exact same thing - manipulating the generated path see PathManipulator. The output from the test there should be almost the same as your test for this PR. The condition in the PathManipulator is just to make sure it is not applied to other tests.

TheSesam commented 3 years ago

Hallo Thor,

Thanks for your reply!

I have created this solution since I have not seen a better solution for our use case. I totally understand that you want to keep the plugin simple and clean! I think this is the correct approach.

Unfortunately we need the Path annotation on class level for other use cases. Your Tip with Readerlistener is great! This is exactly what we need and does the trick perfectly! I always struggle with learning new Plugins and tools and sometimes I miss such things.

I think you are completely right we should not add to the plugin and use the ReaderListener instead!

Thanks for your help here, we can now proceed 😊

Best regards Josef

Von: J H @.> Gesendet: Montag, 24. Mai 2021 20:26 An: Hochwind Josef-Michael, DE-820 @.> Betreff: [EXTERNAL] Fwd: [openapi-tools/swagger-maven-plugin] Introducing the possibility to cut the basePath from Paths (#79)

EXTERNAL SENDER - be CAUTIOUS, particularly with links and attachments.

EXTERNER Absender - Bitte VORSICHT beim Öffnen von Links und Anhängen.


-------- Weitergeleitete Nachricht -------- Betreff:

Re: [openapi-tools/swagger-maven-plugin] Introducing the possibility to cut the basePath from Paths (#79)

Datum:

Mon, 24 May 2021 02:49:39 -0700

Von:

Thor Anker Kvisgård Lange @.**@.>

Antwort an:

openapi-tools/swagger-maven-plugin @.**@.>

An:

openapi-tools/swagger-maven-plugin @.**@.>

Kopie (CC):

TheSesam @.**@.>, Mention @.**@.>

@TheSesamhttps://github.com/TheSesam I have looked through your pull-request and quality wise I think it looks good. However, I am a little bit at ease about the use case here.

I surely understand you want the version (if you want to do URL based versioning - that is a whole discussion in itself) on the server path instead of the individual path of the endpoints. I am just wondering why you do not remove it from the @Path annotation on the class level?

It is also possible to add the server section to configuration generating the OpenAPI document - you have take a look at generate-mojo-full.xmlhttps://github.com/openapi-tools/swagger-maven-plugin/blob/master/src/test/resources/generate-mojo-full-pom.xml#L38 which shows pretty much the complete configuration you can do. Alternative you can also add class or interface with the @OpenAPIDefinition somewhere among the scanned packages to do this configuration.

We intentionally wanted to keep this plugin as thin a wrapper around the Swagger classes as possible. While I do see some value in doing a generic "post processing" step I am also worried we might find a lot of other relevant cases and thus expand the plugin a lot.

If you cannot alter the @Path annotations on your classes or use the sort of "global" Swagger configuration there is also one other option for doing a post processing. You can create a ReaderListener implementation and put this into your scannes classpath. I created an example for you doing the exact same thing - manipulating the generated path see PathManipulatorhttps://github.com/openapi-tools/swagger-maven-plugin/blob/feature/path-manipulation/src/test/java/io/openapitools/swagger/example/manipulate/PathManipulator.java. The output from the test there should be almost the same as your test for this PR.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/openapi-tools/swagger-maven-plugin/pull/79#issuecomment-846924922, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOEJWJEKISPYHIVUSEKAVWTTPIOLHANCNFSM45ISM2AQ.

langecode commented 3 years ago

I'll close this PR then.

ReaderListener is part of the Swagger framework but it also took a while before I discovered this feature. The support for this feature was actually one of the reasons for creating this plugin because it was not being picked up by the original Kong plugin. But it is a quite nice feature doing something like this. Glad you found a solution :-)