Skip to content

[Jaxrs-spec] fix usage of Jersey templates in shellscript #4722

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.

Sign up for GitHub

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

Jump to bottom
Merged
wing328 merged 2 commits into from
Feb 7, 2017
Merged

[Jaxrs-spec] fix usage of Jersey templates in shellscript #4722

wing328 merged 2 commits into from
Feb 7, 2017

Conversation

@jfiala
Copy link
Contributor

@jfiala jfiala commented Feb 5, 2017

PR checklist

  • Read the contribution guildelines.
  • Ran the shell/batch 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)
  • Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

see #4720

@jfiala
Copy link
Contributor Author

jfiala commented Feb 6, 2017

@wing328 pls merge this when you have time, this fixes the currently wrong samples for jaxrs-spec and the corresponding shellscript.

@wing328
Copy link
Contributor

wing328 commented Feb 6, 2017

@jfiala sure. Let me review later ...

@wing328 wing328 added this to the v2.2.2 milestone Feb 6, 2017
wing328
wing328 reviewed Feb 6, 2017
View reviewed changes
samples/server/petstore/jaxrs-spec/swagger.json
"required" : false,
"type" : "number",
"maximum" : 987.6,
"maximum" : 987.6000000000000227373675443232059478759765625,
Copy link
Contributor

@wing328 wing328 Feb 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfiala this is weird. Can you rebase this PR on the latest master and generate the sample again to see if the issue (long decimal) goes away?

Copy link
Contributor Author

@jfiala jfiala Feb 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rebased and regenerated, but nothing changed, it shows still the float value (and it should have nothing to do with my change...).

Copy link
Contributor

@wing328 wing328 Feb 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the latest master to run ./bin/jaxrs-spec-petstore-server.sh but couldn't repeat the issue.

Copy link
Contributor Author

@jfiala jfiala Feb 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently jaxrs-spec-petstore-server.sh uses the wrong templates (Jaxrs-Jersey!), you have to remove the "-t modules/swagger-codegen/src/main/resources/JavaJaxRS" flag in the shellscript, otherwise you are generating using the Jersey templates...

Copy link
Contributor

@wing328 wing328 Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a test with this PR and I got different results when running ./bin/jaxrs-spec-petstore-server.sh: faa453d, which has new issues (e.g. integer becomes decimal...)

Copy link
Contributor Author

@jfiala jfiala Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on windows so I had to reassmble the shellscript, but in fact that shouldn't change the outcome. Anyway, please correct the shellscript and the samples, so we have a green state here again.

Copy link
Contributor

@wing328 wing328 Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear. I'm not saying your PR causes the issue we're seeing.

Looks like we discovered some new issues, probably due to Swagger Parser. I'm actually performing more tests to confirm the "behaviour".

Copy link
Contributor

@wing328 wing328 Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've merged the PR into master and updated the sample.

Can you pull the latest to give it another try to see if the issue (long decimal) is repeatable?

Copy link
Contributor Author

@jfiala jfiala Feb 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled the master and now everything is fine, thanks (issue does not occur, float looks fine)!

Copy link
Contributor

@wing328 wing328 Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfiala glad to hear that the issue has gone.

@wing328 wing328 merged commit e9823a8 into swagger-api:master Feb 7, 2017
Stwissel added a commit to Stwissel/swagger-codegen that referenced this pull request Feb 9, 2017
@Stwissel
Merge branch 'master' of github.com:swagger-api/swagger-codegen
0d045f5 
* 'master' of github.com:swagger-api/swagger-codegen: (40 commits)
  remove default temp folder during initalization (swagger-api#4749)
  [Java-retrofit] Fix for swagger-api#4750 String comparison with equals (swagger-api#4751)
  update java server stub samples with new uuid mapping
  update java petstore with new uuid mapping
  [Java-Feign] Fixed String comparison using equals instead of == operator (swagger-api#4740)
  add SPINEN
  update jaxrs spec petstore sample (mac)
  [Jaxrs-spec] fix usage of Jersey templates in shellscript (swagger-api#4722)
  [Bash] Bash generator improvements (swagger-api#4730)
  [Java][Issue swagger-api#1806] generate using java.util.UUID for UUIDs
  Revert "rx2 support" (swagger-api#4737)
  rx2 support (swagger-api#4708)
  add https and change order for HPE
  add Hewlett Packard Enterprise (hpe.com)
  Add "Simpfony" to list of companies using Swagger (swagger-api#4726)
  add https://www.slamby.com/
  [csharp] Fix enum default value (swagger-api#4681)
  fix issue swagger-api#4672 - XmlExampleGenerator does not properly handle properties of several numeric types (swagger-api#4673)
  [JAXRS-CXF] Issue 4569 - Re-added usage of contextPath in api.mustache (basePath) (swagger-api#4580)
  [Jaxrs-cxf-cdi] merge beanvalidation templates to single one swagger-api#4719 (swagger-api#4723)
  ...
Stwissel added a commit to Stwissel/swagger-codegen that referenced this pull request Feb 9, 2017
@Stwissel
Merge branch 'master' into feature/domino-server
d944e03 
* master: (40 commits)
  remove default temp folder during initalization (swagger-api#4749)
  [Java-retrofit] Fix for swagger-api#4750 String comparison with equals (swagger-api#4751)
  update java server stub samples with new uuid mapping
  update java petstore with new uuid mapping
  [Java-Feign] Fixed String comparison using equals instead of == operator (swagger-api#4740)
  add SPINEN
  update jaxrs spec petstore sample (mac)
  [Jaxrs-spec] fix usage of Jersey templates in shellscript (swagger-api#4722)
  [Bash] Bash generator improvements (swagger-api#4730)
  [Java][Issue swagger-api#1806] generate using java.util.UUID for UUIDs
  Revert "rx2 support" (swagger-api#4737)
  rx2 support (swagger-api#4708)
  add https and change order for HPE
  add Hewlett Packard Enterprise (hpe.com)
  Add "Simpfony" to list of companies using Swagger (swagger-api#4726)
  add https://www.slamby.com/
  [csharp] Fix enum default value (swagger-api#4681)
  fix issue swagger-api#4672 - XmlExampleGenerator does not properly handle properties of several numeric types (swagger-api#4673)
  [JAXRS-CXF] Issue 4569 - Re-added usage of contextPath in api.mustache (basePath) (swagger-api#4580)
  [Jaxrs-cxf-cdi] merge beanvalidation templates to single one swagger-api#4719 (swagger-api#4723)
  ...
@wing328 wing328 mentioned this pull request Feb 23, 2017
Closed
@jfiala jfiala deleted the jaxrs_sh_4720 branch March 9, 2017 21:02
davidgri pushed a commit to davidgri/swagger-codegen that referenced this pull request May 11, 2017
@jfiala @wing328
[Jaxrs-spec] fix usage of Jersey templates in shellscript (swagger-ap…
aef27d3 
…i#4722)

* remove jersey templates swagger-api#4720

* regenerate sample with jaxrs-spec templates swagger-api#4720
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@wing328 wing328 wing328 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Issue: Bug Server: Java

Projects

None yet

Milestone

v2.2.2

Development

Successfully merging this pull request may close these issues.

2 participants

@jfiala @wing328