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

Spring-MVC config "j8-async": Uses async servlet & Java 8 interface #1742

Merged
merged 4 commits into from Dec 30, 2015

Conversation

icha024
Copy link
Contributor

@icha024 icha024 commented Dec 19, 2015

default

This template is mainly for Maven code-gen plugin use-case - when
Swagger spec on existing project changes there is no need to manually
copy/paste the new functions from the generated client. This will
provide a default (empty) implementation to existing impl and user just
need to override the stub implementation.

Because it generates an interface instead of a concrete stub, an
implementation will be needed to actuate a service end-point. And don't
forget to put @controller on the implementation!

default

This template is mainly for Maven code-gen plugin use-case - when
Swagger spec on existing project changes there is no need to manually
copy/paste the new functions from the generated client. This will
provide a default (empty) implementation to existing impl and user just
need to override the stub implementation.

Because it generates an interface instead of a concrete stub, an
implementation will be needed to actuate a service end-point. And don't
forget to put @controller on the implementation!
@wing328
Copy link
Contributor

wing328 commented Dec 22, 2015

@icha024 thanks for the PR but the CI tests failed. Here is the log: https://api.travis-ci.org/jobs/97884873/log.txt?deansi=true

Please review and let us know if you need help addressing the issue reported by CI.

I would suggest you to add ./bin/spring-mvc-j8-async-petstore-server.sh to generate the output to samples/server/petstore/spring-mvc-j8-async using the new option so that we can use CI to test the output (as specified in .travis.yml)

The previous patch to allow sub options to spring-mvc had removed the
invalid Jersey2 option from spring-mvc codegen, this patch also fixes
the associated test.
@icha024
Copy link
Contributor Author

icha024 commented Dec 24, 2015

@wing328 All done

@wing328
Copy link
Contributor

wing328 commented Dec 24, 2015

@icha024 thanks but I don't see ./bin/spring-mvc-j8-async-petstore-server.sh and the sample. Did you do git add to those new files?

@wing328 wing328 added this to the v2.1.5 milestone Dec 24, 2015
Added the executable and config file for spring-mvc's j8-async target.
@icha024
Copy link
Contributor Author

icha024 commented Dec 25, 2015

@wing328 bin scripts are now added

@wing328
Copy link
Contributor

wing328 commented Dec 28, 2015

@icha024 when running 'mvn test` in the sample folder, I got the errors with the sample. Here is part of the error message:

[INFO] Compiling 16 source files to /private/var/tmp/pr/icha024/swagger-codegen/samples/server/petstore/spring-mvc/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /private/var/tmp/pr/icha024/swagger-codegen/samples/server/petstore/spring-mvc/src/main/java/io/swagger/api/UserApi.java:[48,3] illegal start of type
[ERROR] /private/var/tmp/pr/icha024/swagger-codegen/samples/server/petstore/spring-mvc/src/main/java/io/swagger/api/UserApi.java:[48,11] = expected
[ERROR] /private/var/tmp/pr/icha024/swagger-codegen/samples/server/petstore/spring-mvc/src/main/java/io/swagger/api/UserApi.java:[48,19] ';' expected
[ERROR] /private/var/tmp/pr/icha024/swagger-codegen/samples/server/petstore/spring-mvc/src/main/java/io/swagger/api/UserApi.java:[48,40] <identifier> expected
[ERROR] /private/var/tmp/pr/icha024/swagger-codegen/samples/server/petstore/spring-mvc/src/main/java/io/swagger/api/UserApi.java:[48,41] ';' expected
[ERROR] /private/var/tmp/pr/icha024/swagger-codegen/samples/server/petstore/spring-mvc/src/main/java/io/swagger/api/UserApi.java:[48,52] illegal start of type
[ERROR] /private/var/tmp/pr/icha024/swagger-codegen/samples/server/petstore/spring-mvc/src/main/java/io/swagger/api/UserApi.java:[48,53] <identifier> expected

Did you get similar error when running mvn test?

I would also suggest updating /bin/spring-mvc-petstore-j8-async-server.sh to output the generated code into samples/server/petstore/spring-mvc-j8-async instead

The spring-mvc j8-async code is now located in it's own sample directory
with Java 8 enforced via Maven plug-in.
@icha024
Copy link
Contributor Author

icha024 commented Dec 29, 2015

@wing328 The spring-mvc j8-async code is now located in it's own sample directory with Java 8 enforced via Maven plug-in.

wing328 added a commit that referenced this pull request Dec 30, 2015
Spring-MVC config "j8-async": Uses async servlet & Java 8 interface
@wing328 wing328 merged commit 96889d4 into swagger-api:master Dec 30, 2015
@cbornet
Copy link
Contributor

cbornet commented Jun 2, 2016

@icha024 If I understand correctly, a concrete class annotated by @controller needs to be added manually or the controller will not be created/scanned. Is that correct ?
Then I think the @controller on the interface is useless.

@icha024
Copy link
Contributor Author

icha024 commented Jun 2, 2016

@cbornet Yes that is correct, it's not needed. In hindsight it should've been removed and replaced with a comment instead.

BTW, if you are interested in using this with Maven, here's the plugin snippet that might be useful:
http://www.clianz.com/2016/05/29/java-mvc-swagger-gen/

@cbornet
Copy link
Contributor

cbornet commented Jun 3, 2016

In jaxrs generators the implementation class is generated if it doesn't exists. Would that work with your plugin ? It would be useful for the first generation.

@icha024
Copy link
Contributor Author

icha024 commented Jun 3, 2016

For the first generation that's ok, but the challenge we have is keeping the spec and code synced when spec changes (often pushed by a different team) - so an interface would work better for that. The 'default' implementation part is a nice to have.

@cbornet
Copy link
Contributor

cbornet commented Jun 4, 2016

I mean that in jaxrs, there is an interface and an implementation. When you regenerate, the implementation is not overridden.

@icha024
Copy link
Contributor Author

icha024 commented Jun 4, 2016

Cool, didn't know that. That should work

On Sat, Jun 4, 2016 at 7:53 AM, Christophe Bornet notifications@github.com
wrote:

I mean that in jaxrs, there is an interface and an implementation. When
you regenerate, the implementation is not overridden.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1742 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABptDrN4p_wNgH2rjpOPapxz-aEWHso2ks5qISDvgaJpZM4G4tcI
.

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.

None yet

3 participants