Feature/success code responses #7674

ignaciomolina · 2018-02-16T10:44:00Z

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 language.

@bbdouglas @JFCote @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger

Description of the PR

Add attribute to CodegenOperations to obtain the response code
Add response code to Java Play Framework Controller template

tzimisce012 · 2018-02-16T10:51:01Z

#7583 @JFCote

JFCote

It look good and with proper review from the core team, I think it will be a very good addition to swagger-codegen. We need to make sure to generate the samples of every generator affected by this and make sure they still compile and run test properly.

JFCote · 2018-02-16T12:03:08Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/PistacheServerCodegen.java

@@ -158,14 +158,21 @@ public CodegenOperation fromOperation(String path, String httpMethod, Operation
        CodegenOperation op = super.fromOperation(path, httpMethod, operation, definitions, swagger);

        if (operation.getResponses() != null && !operation.getResponses().isEmpty()) {
-            Response methodResponse = findMethodResponse(operation.getResponses());
+            Map.Entry<String, Response> methodResponse = findMethodResponse(operation.getResponses());


Since your are changing stuff in many other generators, did you make sure to generate all theses generators? Does it create any changes to the samples?

this change affects to other generators but only changes code generated for java play framework, because successCode is used nowhere else

JFCote · 2018-02-16T12:11:12Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultCodegen.java

@@ -1975,20 +1975,20 @@ protected void setNonArrayMapProperty(CodegenProperty property, String type) {
     * @param responses Swagger Operation's responses
     * @return default method response or <tt>null</tt> if not found
     */
-    protected Response findMethodResponse(Map<String, Response> responses) {
+    protected Entry<String, Response> findMethodResponse(Map<String, Response> responses) {


If I understand your code correctly, this seems to be a change to the CORE of swagger codegen since it change the way we handle return status. At the beginning, I though it was only Play Framework generator that was missing feature to return the good code but since you needed to add stuff in the DefaultCodegen, I guess it was missing from all codegen.

That is making this change request more important and I think that core members should review it too.

@wing328 Can you check this and also tag some core members to review this.

Thanks!

JFCote · 2018-02-16T12:14:00Z

samples/server/petstore/java-play-framework-controller-only/public/swagger.json

@@ -307,7 +306,7 @@
          "type" : "file"
        } ],
        "responses" : {
-          "200" : {
+          "201" : {


I'm surprised the json changed... If I'm not mistaken, this is simply a dump from swagger-parser in json format. Why is it now using the 201 code instead of 200. Did I miss something?

my mistake, I did a few tests and I add the file to the commit without notice it

ignaciomolina · 2018-02-27T15:03:48Z

How is it going? @wing328 😅

wing328 · 2018-02-27T15:06:23Z

@ignaciomolina let me review tomorrow. Sorry for the delay.

wing328 · 2018-02-28T08:50:04Z

@ignaciomolina the change breaks the Pistache generator:

[main] INFO io.swagger.parser.Swagger20Parser - reading from modules/swagger-codegen/src/test/resources/2_0/petstore.yaml
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/ApiResponse.h
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/ApiResponse.cpp
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/Category.h
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/Category.cpp
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/Order.h
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/Order.cpp
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/Pet.h
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/Pet.cpp
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/Tag.h
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/Tag.cpp
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/User.h
[main] INFO io.swagger.codegen.AbstractGenerator - writing file /root/src/github.com/swagger-api/swagger-codegen/samples/server/petstore/pistache-server/model/User.cpp
Exception in thread "main" java.lang.RuntimeException: Could not process operation:
  Tag: Tag {
	name: user
	description: Operations about user
	externalDocs: io.swagger.models.ExternalDocs@e1e57bab
	extensions:{}}
  Operation: createUser
  Resource: post /user
  Definitions: {Order=io.swagger.models.ModelImpl@6b904c51, Category=io.swagger.models.ModelImpl@ba13e696, User=io.swagger.models.ModelImpl@9db581f0, Tag=io.swagger.models.ModelImpl@d651f3db, Pet=io.swagger.models.ModelImpl@c9d5d174, ApiResponse=io.swagger.models.ModelImpl@16a9e5a9}
  Exception: For input string: "default"
	at io.swagger.codegen.DefaultGenerator.processOperation(DefaultGenerator.java:935)
	at io.swagger.codegen.DefaultGenerator.processPaths(DefaultGenerator.java:814)
	at io.swagger.codegen.DefaultGenerator.generateApis(DefaultGenerator.java:434)
	at io.swagger.codegen.DefaultGenerator.generate(DefaultGenerator.java:749)
	at io.swagger.codegen.cmd.Generate.run(Generate.java:285)
	at io.swagger.codegen.SwaggerCodegen.main(SwaggerCodegen.java:35)
Caused by: java.lang.NumberFormatException: For input string: "default"
	at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
	at java.lang.Integer.parseInt(Integer.java:580)
	at java.lang.Integer.parseInt(Integer.java:615)
	at io.swagger.codegen.languages.PistacheServerCodegen.fromOperation(PistacheServerCodegen.java:166)
	at io.swagger.codegen.DefaultGenerator.processOperation(DefaultGenerator.java:884)
	... 5 more

For the full log, please click on the link to the Shippable CI job.

wing328 · 2018-02-28T08:50:45Z

I was able to repeat the issue locally by running ./bin/pistache-server-petstore.sh.

ignaciomolina · 2018-02-28T08:52:55Z

Shippable doesn't show me the logs, it says that I have to download it but I don't see any download button

wing328 · 2018-02-28T08:56:46Z

@ignaciomolina can you try https://app.shippable.com/download/jobConsoles?jobId=5a965e1b9e550508008cef87? You will need to register an account at shippable and log in to obtain a valid token.

ignaciomolina · 2018-03-01T11:28:54Z

@wing328 solved!

wing328 · 2018-03-01T11:39:55Z

Thanks. I will review tonight.

wing328 · 2018-03-01T16:19:18Z

cc the following technical committees as the change affects multiple generators:

Ada: @stcarrez @micheleISEP
Apex: @asnelling
C++: @ravinikam @stkrwork @fvarose
Pistache: @sebymiano

wing328 · 2018-03-08T13:22:47Z

If no further question/feedback, I"ll merge this PR on coming Friday.

edrevo · 2018-04-26T08:30:34Z

@wing328 , it would be great if we could merge this PR :)

@ignaciomolina there is a merge conflict.

ignaciomolina added 3 commits February 16, 2018 11:36

Add success code response property

d69c934

Add success code response support to play framework controller

0c5929c

Update samples for java play framework

5124bd3

Fix default response condition bug

bf8ee90

JFCote reviewed Feb 16, 2018

View reviewed changes

Regenerate samples for java play framework

177b0d2

wing328 added the Server: Java label Feb 27, 2018

wing328 added this to the v2.4.0 milestone Feb 27, 2018

wing328 added the Enhancement: Feature label Feb 27, 2018

Fix bug in PistacheServerCodegen

9e384ab

Merge branch 'master' into feature/successCodeResponses

65446af

frantuma added the Language: java label Jun 27, 2018

frantuma force-pushed the master branch from 4ba5164 to 949f0e6 Compare May 20, 2025 09:13

Feature/success code responses #7674

Are you sure you want to change the base?

Feature/success code responses #7674

Uh oh!

Conversation

ignaciomolina commented Feb 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Description of the PR

Uh oh!

tzimisce012 commented Feb 16, 2018

Uh oh!

JFCote left a comment

Choose a reason for hiding this comment

Uh oh!

JFCote Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

ignaciomolina Feb 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JFCote Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

JFCote Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

tzimisce012 Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

ignaciomolina Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

ignaciomolina commented Feb 27, 2018

Uh oh!

wing328 commented Feb 27, 2018

Uh oh!

wing328 commented Feb 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wing328 commented Feb 28, 2018

Uh oh!

ignaciomolina commented Feb 28, 2018

Uh oh!

wing328 commented Feb 28, 2018

Uh oh!

ignaciomolina commented Mar 1, 2018

Uh oh!

wing328 commented Mar 1, 2018 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wing328 commented Mar 1, 2018

Uh oh!

wing328 commented Mar 8, 2018

Uh oh!

edrevo commented Apr 26, 2018

Uh oh!

Uh oh!

ignaciomolina commented Feb 16, 2018 •

edited

Loading

ignaciomolina Feb 16, 2018 •

edited

Loading

wing328 commented Feb 28, 2018 •

edited

Loading

wing328 commented Mar 1, 2018 via email •

edited

Loading