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][jaxrs-spec] add fromString() method to generated enums as required by JAX RS spec #7494

Conversation

upachler
Copy link
Contributor

@upachler upachler commented Sep 23, 2020

I'd like to fix issue #3995 with this PR (see my comments there).

The fix creates an additional static method fromString() as required by the JAX RS spec, so that compliant implementations can correctly parse enum arguments whoose wire string representation differs from the Java enum field name (e.g. MyEnum.FOO vs. "foo"). As it stands right now, a JAX RS implementation would expect "FOO" as a value passed as parameter, so parsing "foo" will fail.

I've coded this fix against v4.3.1, as I ran into other issues with 5.0.0 in my project and did use 4.3.1 there; however I assume this can be merged into both the 4.3.x and 5.0.x lines. I couldn't find a release branch for 4.3.x, so this PR is made against master in the hope that it will also be patched into the 4.3.x line if accepted.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
  • script does not exist in 4.3.1 Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@upachler upachler changed the title add fromString() method to generated enums as required by JAX RS spec [Java][jaxrs-spec] add fromString() method to generated enums as required by JAX RS spec Sep 24, 2020
@wing328
Copy link
Member

wing328 commented Sep 26, 2020

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

@wing328
Copy link
Member

wing328 commented Sep 26, 2020

@upachler the current master is for the upcoming v5.0.0 release.

@upachler upachler force-pushed the issue-3996-jaxrs-cxf-enum-defined-in-lowercase-only-accepts-uppercase branch from 6aa402f to ee4cf62 Compare September 27, 2020 20:37
@upachler
Copy link
Contributor Author

Hi @wing328,
Sorry for the confusion, I've got to many laptops I work on :) - I fixed up the author on the commit, should point to my account now.
About 4.3.2 vs. 5.0.0 - I've tried v5.0.0 available via Maven, and my project fell apart :) - so I was hoping that my fix would make it into 4.3.2, if there will be one.

@wing328 wing328 added this to the 5.1.0 milestone Mar 6, 2021
@wing328 wing328 removed this from the 5.1.0 milestone Mar 19, 2021
@hermes85pl
Copy link

Is there any hope that you guys will proceed with such solution for this long-standing issue anytime soon?

@wing328
Copy link
Member

wing328 commented Feb 9, 2022

@hermes85pl sorry that this PR has not been reviewed yet....

May I know if you've tested the change? Does it work for you? If yes, we can definitely include it in the upcoming v6.0.0 release due in a month or 2.

@wing328 wing328 added this to the 6.0.0 milestone Feb 9, 2022
@wing328
Copy link
Member

wing328 commented Feb 14, 2022

Tested locally with updated samples and the result is ok:

[INFO] --- maven-jar-plugin:2.2:jar (default-jar) @ jaxrs-spec-interface-petstore-server ---
[INFO] Building jar: /Users/williamcheng/Code/openapi-generator/samples/server/petstore/jaxrs-spec-interface/target/jaxrs-spec-interface-petstore-server-1.0.0.jar
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  4.513 s
[INFO] Finished at: 2022-02-14T10:53:40+08:00
[INFO] ------------------------------------------------------------------------

Thanks for the fix and sorry for the delay in merging the PR.

@wing328 wing328 merged commit 2584c9d into OpenAPITools:master Feb 14, 2022
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.

3 participants