[Java][Spring][Server] fix spring-mvc path enum issue #6890

MZinchenko · 2017-11-05T13:49:02Z

PR checklist

Read the contribution guidelines.
Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
Copied the technical committee to review the pull request if your PR is targeting a particular programming langauge.

Description of the PR

Issue #4680 fixed in proper way

…otes

wing328 · 2017-11-09T09:53:58Z

@MZinchenko thanks for the PR. Please run ./bin/spring-all-pestore.sh to update Spring Petstore samples so that CI can verify the change.

MZinchenko · 2017-11-13T11:42:52Z

Sorry for long time reply.
I found mistake in my merge and fix it.

Also when I update java samples, my samples\server\petstore\spring-mvc\pom.xml always going wrong.
Artefact and module name changes to swagger-spring (must be swagger-spring-mvc-server).
Am I doing smth wrong?

wing328 · 2017-11-14T04:51:12Z

Artefact and module name changes to swagger-spring (must be swagger-spring-mvc-server).

I'll take a look...

wing328 · 2017-11-14T13:41:25Z

I'm doing some tests as well...

cc @cbornet

wing328 · 2017-11-14T14:35:44Z

Tests passed via https://circleci.com/gh/swagger-api/swagger-codegen/2594 (after updating Spring Petstore samples)

cbornet · 2017-11-15T15:04:18Z

The samples of this PR are not OK so it's hard to review...

MZinchenko · 2017-11-15T16:10:41Z

What's wrong in samples? 2017-11-15 18:13 GMT+03:00 Christophe Bornet <notifications@github.com>:

…

The samples of this PR are not OK so it's hard to review... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6890 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AfX4bA9CjiFqIFP7kSElRat1ZvB3Rrvsks5s2v16gaJpZM4QSYYw> .

cbornet · 2017-11-15T20:49:04Z

There are @Generated annotations everywhere instead of being hidden, the examples have been removed from spring controllers, etc...

MZinchenko · 2017-11-16T15:01:34Z

I simply ran scripts: java-petstore.bat java-petstore-okhttp-gson.bat java-play-framework-petstore-server.bat java-vertx-async-petstore-server.bat java-vertx-rx-petstore-server.bat spring-mvc-petstore-j8-async-server.bat spring-mvc-petstore-server.bat springboot-petstore-server.bat What can I do to solve this problems? Annotation @generated <https://github.com/generated> created by default, as I can see (in my swagger classes it exists too) 2017-11-15 23:49 GMT+03:00 Christophe Bornet <notifications@github.com>:

…

There are @generated <https://github.com/generated> annotations everywhere instead of being hidden, the examples have been removed from spring controllers, etc... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6890 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AfX4bK9clxAQc5vnBopQ6mtcea7qMISYks5s205JgaJpZM4QSYYw> .

cbornet · 2017-11-16T22:56:24Z

The windows scripts are incorrect...

MZinchenko · 2017-11-17T15:39:12Z

Ok. I fix windows scripts and push it in this PR. But as I can see there are lot of changes is samples. There are comment changes and "get => is" at most. As I can see this is result of changing version from 2.2.3 to 2.3.0. 2017-11-17 1:56 GMT+03:00 Christophe Bornet <notifications@github.com>:

…

The windows scripts are incorrect... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6890 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AfX4bNqyXDW_8wYWopXL2S6X7Ned_uZxks5s3L2jgaJpZM4QSYYw> .

cbornet · 2017-11-17T16:05:50Z

samples/server/petstore/spring-mvc-j8-async/src/main/java/io/swagger/api/PetApi.java

        method = RequestMethod.POST)
-    default CompletableFuture<ResponseEntity<Void>> addPet(@ApiParam(value = "Pet object that needs to be added to the store" ,required=true )  @Valid @RequestBody Pet body) {
+    default CompletableFuture<ResponseEntity<Void>> addPet(
+@ApiParam(value = "Pet object that needs to be added to the store" ,required=true )  @Valid @RequestBody Pet body) {


Alignment is wrong

cbornet · 2017-11-17T16:07:59Z

samples/server/petstore/spring-mvc-j8-async/src/main/java/io/swagger/api/PetApi.java

        method = RequestMethod.DELETE)
-    default CompletableFuture<ResponseEntity<Void>> deletePet(@ApiParam(value = "Pet id to delete",required=true) @PathVariable("petId") Long petId,@ApiParam(value = "" ) @RequestHeader(value="api_key", required=false) String apiKey) {
+    default CompletableFuture<ResponseEntity<Void>> deletePet(@ApiParam(value = "Pet id to delete",required=true) @PathVariable("petId") Long petId
+,


This comma should not be here

cbornet · 2017-11-17T16:09:31Z

samples/server/petstore/spring-mvc-j8-async/src/main/java/io/swagger/api/PetApi.java

        method = RequestMethod.GET)
-    default CompletableFuture<ResponseEntity<List<Pet>>> findPetsByStatus(@NotNull @ApiParam(value = "Status values that need to be considered for filter", required = true, allowableValues = "available, pending, sold") @Valid @RequestParam(value = "status", required = true) List<String> status) {
+    default CompletableFuture<ResponseEntity<List<Pet>>> findPetsByStatus(@NotNull @ApiParam(value = "Status values that need to be considered for filter", required = true, allowableValues = "available, pending, sold") @Valid @RequestParam(value = "status", required = true) List<String> status
+) {


This should not be on this line

cbornet · 2017-11-17T16:12:25Z

...e/spring-mvc-j8-async/src/main/java/io/swagger/configuration/SwaggerDocumentationConfig.java

        return new ApiInfoBuilder()
            .title("Swagger Petstore")
-            .description("This spec is mainly for testing Petstore server and contains fake endpoints, models. Please do not use this for any other purpose. Special characters: \" \\")
+            .description("This is a sample server Petstore server.  You can find out more about Swagger at [http://swagger.io](http://swagger.io) or on [irc.freenode.net, #swagger](http://swagger.io/irc/).  For this sample, you can use the api key `special-key` to test the authorization filters.")


The swagger spec used seems different than before. Still an issue with Windows script ?

cbornet · 2017-11-17T16:14:02Z

samples/server/petstore/spring-mvc/src/main/java/io/swagger/api/StoreApi.java

        method = RequestMethod.GET)
-    ResponseEntity<Order> getOrderById(@Min(1) @Max(5) @ApiParam(value = "ID of pet that needs to be fetched",required=true) @PathVariable("order_id") Long orderId);
+    ResponseEntity<Order> getOrderById(@Min(1) @Max(5) @ApiParam(value = "ID of pet that needs to be fetched",required=true) @PathVariable("orderId") Long orderId
+);


Should not be on this line

MZinchenko · 2017-11-20T13:42:47Z

Summary, there are some problems: 1) spring-mvc-petstore-server.sh, springboot-petstore-server.sh and other java shell scripts using "petstore-with-fake-endpoints-models-for-testing.yaml" but "petstore.yaml". That's very strange (windows scripts using "petstore.yaml"). 2) windows scripts are very outdated. I think, they must be removed or updated. I can't run shell scripts nativly, but I'll try 3) suddenly new-line problem. I can't understand why new-lines appears before some params. I revert my branch changes, but they are still here. I think this problem is elder or environment aware. 2017-11-17 19:15 GMT+03:00 Christophe Bornet <notifications@github.com>:

…

***@***.**** commented on this pull request. ------------------------------ In samples/server/petstore/spring-mvc-j8-async/src/main/ java/io/swagger/api/PetApi.java <#6890 (comment)> : > @@ -61,7 +61,8 @@ produces = { "application/xml", "application/json" }, consumes = { "application/json", "application/xml" }, method = RequestMethod.POST) - default CompletableFuture<ResponseEntity<Void>> ***@***.***(value = "Pet object that needs to be added to the store" ,required=true ) @Valid @RequestBody Pet body) { + default CompletableFuture<ResponseEntity<Void>> addPet( ***@***.***(value = "Pet object that needs to be added to the store" ,required=true ) @Valid @RequestBody Pet body) { Alignment is wrong ------------------------------ In samples/server/petstore/spring-mvc-j8-async/src/main/ java/io/swagger/api/PetApi.java <#6890 (comment)> : > @@ -81,7 +82,9 @@ @RequestMapping(value = "/pet/{petId}", produces = { "application/xml", "application/json" }, method = RequestMethod.DELETE) - default CompletableFuture<ResponseEntity<Void>> ***@***.***(value = "Pet id to delete",required=true) @PathVariable("petId") Long ***@***.***(value = "" ) @RequestHeader(value="api_key", required=false) String apiKey) { + default CompletableFuture<ResponseEntity<Void>> ***@***.***(value = "Pet id to delete",required=true) @PathVariable("petId") Long petId +, This comma should not be here ------------------------------ In samples/server/petstore/spring-mvc-j8-async/src/main/ java/io/swagger/api/PetApi.java <#6890 (comment)> : > @@ -102,7 +105,8 @@ @RequestMapping(value = "/pet/findByStatus", produces = { "application/xml", "application/json" }, method = RequestMethod.GET) - default CompletableFuture<ResponseEntity<List<Pet>>> ***@***.*** @ApiParam(value = "Status values that need to be considered for filter", required = true, allowableValues = "available, pending, sold") @Valid @RequestParam(value = "status", required = true) List<String> status) { + default CompletableFuture<ResponseEntity<List<Pet>>> ***@***.*** @ApiParam(value = "Status values that need to be considered for filter", required = true, allowableValues = "available, pending, sold") @Valid @RequestParam(value = "status", required = true) List<String> status +) { This should not be on this line ------------------------------ In samples/server/petstore/spring-mvc-j8-async/src/main/ java/io/swagger/configuration/SwaggerDocumentationConfig.java <#6890 (comment)> : > @@ -17,7 +17,7 @@ ApiInfo apiInfo() { return new ApiInfoBuilder() .title("Swagger Petstore") - .description("This spec is mainly for testing Petstore server and contains fake endpoints, models. Please do not use this for any other purpose. Special characters: \" \\") + .description("This is a sample server Petstore server. You can find out more about Swagger at [http://swagger.io](http://swagger.io) or on [irc.freenode.net, #swagger](http://swagger.io/irc/). For this sample, you can use the api key `special-key` to test the authorization filters.") The swagger spec used seems different than before. Still an issue with Windows script ? ------------------------------ In samples/server/petstore/spring-mvc/src/main/java/io/ swagger/api/StoreApi.java <#6890 (comment)> : > produces = { "application/xml", "application/json" }, method = RequestMethod.GET) - ResponseEntity<Order> ***@***.***(1) @max(5) @ApiParam(value = "ID of pet that needs to be fetched",required=true) @PathVariable("order_id") Long orderId); + ResponseEntity<Order> ***@***.***(1) @max(5) @ApiParam(value = "ID of pet that needs to be fetched",required=true) @PathVariable("orderId") Long orderId +); Should not be on this line — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6890 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AfX4bKYozLSOPXr10Dt7S2w8FWXvQTwEks5s3bEmgaJpZM4QSYYw> .

cbornet · 2017-11-20T22:41:46Z

Correct and most used scripts are Unix ones. I think the windows scripts have been added as a block without checking each particular languages so that's why there are differences.

cbornet · 2017-11-20T22:43:37Z

modules/swagger-codegen/src/main/resources/JavaSpring/pathParams.mustache

@@ -1 +1 @@
-{{#isPathParam}}{{#useBeanValidation}}{{>beanValidationPathParams}}{{/useBeanValidation}}@ApiParam(value = "{{{description}}}"{{#required}},required=true{{/required}}{{#allowableValues}}, allowableValues = "{{#values}}{{{.}}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/values}}"{{/allowableValues}}{{#defaultValue}}, defaultValue="{{{defaultValue}}}"{{/defaultValue}}) @PathVariable("{{baseName}}") {{>optionalDataType}} {{paramName}}{{/isPathParam}}
+{{#isPathParam}}{{#useBeanValidation}}{{>beanValidationPathParams}}{{/useBeanValidation}}@ApiParam(value = "{{{description}}}"{{#required}},required=true{{/required}}{{#allowableValues}}, allowableValues = "{{#enumVars}}{{#lambdaEscapeDoubleQuote}}{{{value}}}{{/lambdaEscapeDoubleQuote}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/enumVars}}"{{/allowableValues}}{{#defaultValue}}, defaultValue="{{{defaultValue}}}"{{/defaultValue}}) @PathVariable("{{baseName}}") {{>optionalDataType}} {{paramName}}{{/isPathParam}}


This is were the extra line has been added. Remove it and regenerate the samples.

You means new line after {{/isPathParam}}?
I already done that and regenerate the samples, but new lines not removed.
Also I revert all changes in my branch and regenerate the samples, but new lines not removed.
As you can see, new lines appears in blocks generated by other templates, not pathParams.mustache only. First of all PetApi.java using bodyParams.mustache for addPet method, but I don't change bodyParams.mustache

I pushed this change and some windows scripts changes, but samples still have some differences

MZinchenko · 2017-11-21T13:27:48Z

Ok. I done update samples under CentOS inside Docker.
New lines vanished, so that's some type of environment bug.
That's no good, some users may use codegen under Windows with default setting and they are suffering.

wing328 · 2017-11-27T12:43:22Z

@MZinchenko PR merged into master. Thanks for your contribution.

When you've time, I wonder if you can contact me via the email address on my Github profile. Thanks.

Issue swagger-api#4680 correct values for enum with escaped double qu…

a10fbe8

…otes

MZinchenko mentioned this pull request Nov 5, 2017

[Java][Spring][Server] Issue #4680 and objectMapper issue #6732

Closed

4 tasks

wing328 added Server: Java Server: Spring labels Nov 6, 2017

wing328 added this to the v2.3.0 milestone Nov 6, 2017

wing328 added the Issue: Bug label Nov 6, 2017

cbornet reviewed Nov 17, 2017

View reviewed changes

cbornet reviewed Nov 20, 2017

View reviewed changes

MZinchenko added 3 commits November 21, 2017 15:51

Issue swagger-api#4680 regenerate samples

80b1f76

Issue swagger-api#4680 remove new lines

86a3531

Issue swagger-api#4680 fixed scripts

363e500

MZinchenko force-pushed the 4680 branch from 43dd074 to 363e500 Compare November 21, 2017 12:51

Issue swagger-api#4680 update samples under CentOS

5d9bacb

wing328 merged commit a8642db into swagger-api:master Nov 27, 2017

wing328 changed the title ~~[Java][Spring][Server] Issue #4680~~ [Java][Spring][Server] fix spring-mvc path enum issue Nov 27, 2017

		@@ -1 +1 @@
		{{#isPathParam}}{{#useBeanValidation}}{{>beanValidationPathParams}}{{/useBeanValidation}}@ApiParam(value = "{{{description}}}"{{#required}},required=true{{/required}}{{#allowableValues}}, allowableValues = "{{#values}}{{{.}}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/values}}"{{/allowableValues}}{{#defaultValue}}, defaultValue="{{{defaultValue}}}"{{/defaultValue}}) @PathVariable("{{baseName}}") {{>optionalDataType}} {{paramName}}{{/isPathParam}}
		{{#isPathParam}}{{#useBeanValidation}}{{>beanValidationPathParams}}{{/useBeanValidation}}@ApiParam(value = "{{{description}}}"{{#required}},required=true{{/required}}{{#allowableValues}}, allowableValues = "{{#enumVars}}{{#lambdaEscapeDoubleQuote}}{{{value}}}{{/lambdaEscapeDoubleQuote}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/enumVars}}"{{/allowableValues}}{{#defaultValue}}, defaultValue="{{{defaultValue}}}"{{/defaultValue}}) @PathVariable("{{baseName}}") {{>optionalDataType}} {{paramName}}{{/isPathParam}}

[Java][Spring][Server] fix spring-mvc path enum issue #6890

[Java][Spring][Server] fix spring-mvc path enum issue #6890

Uh oh!

Conversation

MZinchenko commented Nov 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Description of the PR

Uh oh!

wing328 commented Nov 9, 2017

Uh oh!

MZinchenko commented Nov 13, 2017

Uh oh!

wing328 commented Nov 14, 2017

Uh oh!

wing328 commented Nov 14, 2017

Uh oh!

wing328 commented Nov 14, 2017

Uh oh!

cbornet commented Nov 15, 2017

Uh oh!

MZinchenko commented Nov 15, 2017 via email

Uh oh!

cbornet commented Nov 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MZinchenko commented Nov 16, 2017 via email

Uh oh!

cbornet commented Nov 16, 2017

Uh oh!

MZinchenko commented Nov 17, 2017 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MZinchenko commented Nov 20, 2017 via email

Uh oh!

cbornet commented Nov 20, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MZinchenko commented Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wing328 commented Nov 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MZinchenko commented Nov 5, 2017 •

edited

Loading

cbornet commented Nov 15, 2017 •

edited

Loading

MZinchenko commented Nov 21, 2017 •

edited

Loading