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

java jax rs code generation fix and formatting tidy #1093

Closed
wants to merge 16 commits into from

Conversation

ahgittin
Copy link

introduction of skipOverwrite in #831 / #859 caused Factory and Impl classes never to be written for jax-rs. this fixes that.

also i noticed the old jax-rs templates have bad spacing, e.g.

   public method(...) {
   // body
}

this aligns things and generally improves whitespace (superseding #1092 as per @wing328 's comment)

@ahgittin
Copy link
Author

@wing328
Copy link
Contributor

wing328 commented Aug 20, 2015

@ahgittin thanks! will review and let you know if I've any questions.

@ahgittin
Copy link
Author

@wing328 noticed the generated code did not compile in some cases due to supportsInheritance now defaulting to false; have pushed one more one-liner to fix that. can unwind and open separate PR if you prefer (didn't expect you to get to it so quickly!).

@ahgittin ahgittin force-pushed the jax-rs-format-fixes branch from c3f5992 to ac6a051 Compare August 20, 2015 11:16
@ahgittin
Copy link
Author

@wing328 actually having seen #1050 i've reverted that so nvm. i will open a separate PR.

@wing328
Copy link
Contributor

wing328 commented Aug 21, 2015

@ahgittin under samples/server/petstore/jaxrs I ran mvn test but got the following:

[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /private/var/tmp/ahgittin/swagger-codegen/samples/server/petstore/jaxrs/src/gen/java/io/swagger/api/StoreApi.java:[32,153] cannot find symbol
  symbol:   variable String
  location: class io.swagger.api.StoreApi
[ERROR] /private/var/tmp/ahgittin/swagger-codegen/samples/server/petstore/jaxrs/src/gen/java/io/swagger/api/StoreApi.java:[32,168] annotation values must be of the form 'name=value'
[INFO] 2 errors 
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.005 s
[INFO] Finished at: 2015-08-21T21:58:41+08:00
[INFO] Final Memory: 15M/245M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project swagger-jaxrs-server: Compilation failure: Compilation failure:
[ERROR] /private/var/tmp/ahgittin/swagger-codegen/samples/server/petstore/jaxrs/src/gen/java/io/swagger/api/StoreApi.java:[32,153] cannot find symbol
[ERROR] symbol:   variable String
[ERROR] location: class io.swagger.api.StoreApi
[ERROR] /private/var/tmp/ahgittin/swagger-codegen/samples/server/petstore/jaxrs/src/gen/java/io/swagger/api/StoreApi.java:[32,168] annotation values must be of the form 'name=value'
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
jaxrs|jax-rs-format-fixes ⇒ 
jaxrs|jax-rs-format-fixes ⇒ pwd
/var/tmp/ahgittin/swagger-codegen/samples/server/petstore/jaxrs

Is this PR supposed to address that or the error is already there before you submitted the fix ?
(I've run mvn clean && ./bin/jaxrs-petstore-server.sh to update the sample before running mvn test)

@webron
Copy link
Contributor

webron commented Aug 24, 2015

@ahgittin - due to a development procedure change, you'll have to resubmit the PR against the master branch.

… `Impl` weren't written

in eca8276 when `skipOverwrite` was introduced,
a file exists check was moved into `DefaultCodegen.shouldOverwrite` effectively changing
its semantics from "is overwriting allowed for this file" to "should we write this file".

the method is overridden in `JaxRSServerCodegen` to specify that `Factory` and `Impl`
files should always return false, because we never want to *overwrite* them.
but the above change in semantics meant we would never *write* these files.

this reverts the semantics of that method, in line with both the method name and the jax-rs interpretation,
and introduces a new `shouldSkipOverwrite` which adds the file exists check
and is used in places where the above commit expected the latter semantics.
alignment and line spaces were odd and inconsistent;
this tries to respect the coding conventions which it seems were intended,
but make them consistent (i'm not trying to bike-shed how code should be formatted,
honest, just to make the formatting which it seems was intended line up!)
@ahgittin ahgittin force-pushed the jax-rs-format-fixes branch from ac6a051 to 0fc2532 Compare August 24, 2015 09:59
@ahgittin
Copy link
Author

@wing328 I've rebased on master and will submit PR shortly. I've no idea why that error appeared, it is supposed to build, and when I do this (after the rebase and tidies) all works well:

rm -rf samples/server/petstore/jaxrs/
bin/jaxrs-petstore-server.sh
cd samples/server/petstore/jaxrs/
mvn clean install

@ahgittin
Copy link
Author

Closing in preference to #1119

@ahgittin ahgittin closed this Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants