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

Swagger JAX-RS Reader and path building #3367

Open
maggu2810 opened this issue Nov 26, 2019 · 3 comments
Open

Swagger JAX-RS Reader and path building #3367

maggu2810 opened this issue Nov 26, 2019 · 3 comments

Comments

@maggu2810
Copy link
Contributor

@maggu2810 maggu2810 commented Nov 26, 2019

Hi,

I created two modules that allows me to generate the description using the OSGi JAX-RS Whiteboard mechanism to detect the resources. One module for Swagger 1.5 and one for the current OpenAPI one.

I realized that your readers (for 1.5 the one in the module swagger-jaxrs, for the master the one in the module swagger-jaxrs2) handles the path building differently.

As fair as I understand the @Path documentation it does not matter if a path starts with / or ends with /. It should be the same and if necessary the / are added on path building.

This is working for swagger-jaxrs2's Reader but not for swagger-jaxrs's one.

See: https://github.com/swagger-api/swagger-core/blob/01c4272/modules/swagger-jaxrs/src/main/java/io/swagger/jaxrs/Reader.java#L744-L756

If the parent path ends with '/' that character is removed. After that the class path is added. It does not check if the class path begins with a '/' and does not add it if missing. If the method's path is added after the class path has been added the check is done.
So, as long as the classes path annotation value does not begin with a / the concatenation between parent path and class path will be without an / as it is removed from the ending of the parent path (if present).
For my use case, I need to use the parent path for the application's path.

The swagger-jaxrs2 reader handles the path concatenation more generic and checks for missing / all the time.

Do you agree that this could be changed in the 1.5 branch? I can try to come up with a PR.

@frantuma

This comment has been minimized.

Copy link
Contributor

@frantuma frantuma commented Dec 9, 2019

Thanks for reporting and investigating this; and yes I agree that this should b changed in 1.5 branch, a PR is more than welcome!

maggu2810 added a commit to maggu2810/swagger-core that referenced this issue Dec 15, 2019
We use the same approach / code base as used by the current master
branch.

Fixes: swagger-api#3367

Signed-off-by: Markus Rathgeb <maggu2810@gmail.com>
@maggu2810

This comment has been minimized.

Copy link
Contributor Author

@maggu2810 maggu2810 commented Dec 15, 2019

A PR has been submitted (#3385).

@maggu2810

This comment has been minimized.

Copy link
Contributor Author

@maggu2810 maggu2810 commented Jan 16, 2020

@frantuma friendly ping 😉

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.