Skip to content

Amending Path annotation#3553

Merged
wing328 merged 2 commits intoswagger-api:masterfrom
rynger:rynger-patch-1
Aug 9, 2016
Merged

Amending Path annotation#3553
wing328 merged 2 commits intoswagger-api:masterfrom
rynger:rynger-patch-1

Conversation

@rynger
Copy link
Copy Markdown
Contributor

@rynger rynger commented Aug 8, 2016

The code generated for CXF was always referring to to @path("/") in the API.
Changed the templated Path annotation value from "/" to "/{{baseName}}".

Changed the Path annotation value from "/" to "/{{baseName}}"
@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Aug 8, 2016

@rynger thanks for the PR. Please run ./bin/jaxrs-cxf-petstore-server.sh to update the Petstore sample so that the CI can tests the change.

I still believe it should be {{basePath}} as {{baseName}} refers to the original parameter name (header, form, query)

@rynger
Copy link
Copy Markdown
Contributor Author

rynger commented Aug 8, 2016

Ah yes, looking at other templates, they refer to the {{basePath}}, in which case all the api.mustache files under the JavaJaxRS resources folder have @path annotations which should use {{basePath}} and not {{baseName}}. (A seperate issue?)

I will amend the CXF template, run the scripts and try the built snapshot with my code first before attempting another commit.

@wing328 wing328 modified the milestones: v2.2.1, v2.3.0 Aug 8, 2016
@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Aug 8, 2016

@rynger it's worth sharing with you that there's another variable {{contextPath}} which stores the base path without the host name (e.g. /v2 for Petstore while an endpoint for Petstore looks like this: https://petstore.swagger.io/v2/pet/1)

@rynger
Copy link
Copy Markdown
Contributor Author

rynger commented Aug 8, 2016

Thanks for that, additionally from inspecting the DefaultGenerator class source (~Line 399) i see there's a {{basePathWithoutHost}} as well and would seem to contain the same value. I will err in favour of your suggested {{contextPath}} as it seems to have a null check in its assignment.

…rectory. Included generated files following script run. Have amended Path annotation value to {{contextPath}} and removed public access modifier from template as this is redundant for Java interface definition.
@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Aug 9, 2016

@rynger thanks for the contribution. The change looks good to me. When you've time, I wonder if you can add a auto-generated pom.xml similar to this one: https://github.com/swagger-api/swagger-codegen/blob/master/samples/server/petstore/jaxrs/jersey1/pom.xml (with another PR)

@wing328 wing328 merged commit 3faee1f into swagger-api:master Aug 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants