[go: nahoru, domu]

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

Fix duplicate import javax.valid.Valid in resteasy generator #19055

Merged
merged 2 commits into from
Jul 7, 2024

Conversation

hamburml
Copy link
Contributor
@hamburml hamburml commented Jul 2, 2024

Fixes #17069

The resteasy generator generates

import javax.validation.Valid;

twice when useBeanValidation is enabled.
It is already added here https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/JavaJaxRS/resteasy/model.mustache#L12

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.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh ./bin/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    
    (For Windows users, please run the script in Git BASH)
    Commit all changed files.
    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*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.6.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@hamburml hamburml changed the title remove javax.valid.Valid import Fix duplicate import javax.valid.Valid Jul 2, 2024
@hamburml
Copy link
Contributor Author
hamburml commented Jul 2, 2024

mention
@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @martin-mfg for review. Sorry, did not know which one I should use from the technical committee.

@hamburml hamburml changed the title Fix duplicate import javax.valid.Valid Fix duplicate import javax.valid.Valid in resteasy generator Jul 2, 2024
@hamburml
Copy link
Contributor Author
hamburml commented Jul 7, 2024

@wing328 could you pls review? Thx

@wing328
Copy link
Member
wing328 commented Jul 7, 2024

thanks for the PR but I couldn't repeat the issue with the latest master using the follow command/test:

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g jaxrs-resteasy -i modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o /tmp/resteasy --additional-properties useBeanValidation=true

If I add useJakartaEe=true but then the output won't compile.

Can you share with me a way to reproduce the issue?

@hamburml
Copy link
Contributor Author
hamburml commented Jul 7, 2024

Sure! I have a reproducer here

https://github.com/hamburml/quarkus-openapi-generator

Just clone main, run mvn package and check the Order.java or Pet.java class. You should see one jakarta and one javax import.
image

I am sorry, I haven't used the cli command, always the maven plugin. I tried your command but I do not know where the files are generated.

@hamburml
Copy link
Contributor Author
hamburml commented Jul 7, 2024

Sorry, I found it :)
I used

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g jaxrs-resteasy -i modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o /tmp/resteasy --additional-properties useBeanValidation=true --additional-properties useJakartaEe=true

And the generated classes have also both imports (one jakarta and one javax).
image

The example doesnt work on my end but it looks like my gradle doesnt work. I normally use maven.

@wing328
Copy link
Member
wing328 commented Jul 7, 2024

tested locally to confirm there's n duplicate import of javax.valid.Valid

@wing328 wing328 merged commit 8f7cce7 into OpenAPITools:master Jul 7, 2024
36 checks passed
@hamburml
Copy link
Contributor Author
hamburml commented Jul 7, 2024

Thanks <3 I am not that versed with gradle but I could try to fix the example. The current example would always only work with javax and never with jakarta annotations because it forces java ee 8.

@wing328
Copy link
Member
wing328 commented Jul 7, 2024

javax and never with jakarta annotations because it forces java ee 8.

Shall we set the source and target to JDK 11 if useJakrataEe is enabled?

@wing328
Copy link
Member
wing328 commented Jul 7, 2024

just tried it. no luck

seems like something is fundamentally wrong, e.g.

[ERROR] /C:/Users/wing3/AppData/Local/Temp/resteasy234/src/main/java/org/openapitools/api/impl/UserApiServiceImpl.java:[15,34] package jakarta.enterprise.context does not exist

missing import in pom.xml?

@hamburml
Copy link
Contributor Author
hamburml commented Jul 7, 2024

Shall we set the source and target to JDK 11 if useJakrataEe is enabled?

Yeah, I think would be good. Jakarta EE 10 needs JDK 11 minimum.

@wing328
Copy link
Member
wing328 commented Jul 7, 2024

https://mvnrepository.com/artifact/jakarta.servlet/jakarta.servlet-api/6.1.0

that's the latest version but we're still using 4.x

@hamburml
Copy link
Contributor Author
hamburml commented Jul 7, 2024

seems like something is fundamentally wrong, e.g.

In the build.gradle of the generated example you need
implementation 'jakarta.platform:jakarta.jakartaee-api:10.0.0' instead of javax:javaee-api:8.0

And I think some other dependencies are outdated.

@wing328
Copy link
Member
wing328 commented Jul 7, 2024

I ran gradle build but no luck either:

$ gradle build
Starting a Gradle Daemon (subsequent builds will be faster)

FAILURE: Build failed with an exception.

* Where:
Build file 'C:\Users\wing3\AppData\Local\Temp\resteasy234\build.gradle' line: 20

* What went wrong:
A problem occurred evaluating root project 'openapi-jaxrs-resteasy-server'.
> Could not find method compile() for arguments [io.swagger:swagger-annotations:1.5.22] on object of type org.gradle.api.internal.artifacts.dsl.dependencies.DefaultDependencyHandler.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.

@wing328
Copy link
Member
wing328 commented Jul 7, 2024

And I think some other dependencies are outdated.

I wonder if you can help file a PR to update these dependencies 🙏

@hamburml
Copy link
Contributor Author
hamburml commented Jul 7, 2024

Could not find method compile() for arguments [io.swagger:swagger-annotations:1.5.22] on object of type org.gradle.api.internal.artifacts.dsl.dependencies.DefaultDependencyHandler.

Had the same. Gradle v7 removed compile command. It is now implementation.

@hamburml
Copy link
Contributor Author
hamburml commented Jul 7, 2024

I will try :)
@wing328 this could take some time, sorry. I need to get gradle working :D Do you have a documentation on your examples? It is kinda strange that the example has a pom.xml and a gradle file.

@wing328
Copy link
Member
wing328 commented Jul 7, 2024

kinda strange that the example has a pom.xml and a gradle file.

The reason is that some users prefer pom/maven while others prefer gradle.

one can manually delete the one they don't need or ignore it in .openapi-generator-ignore: https://github.com/openapitools/openapi-generator/blob/master/docs/customization.md#ignore-file-format

@wing328
Copy link
Member
wing328 commented Jul 7, 2024

to reproduce the gradle issue, please run

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g jaxrs-resteasy -i modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o /tmp/resteasy234 --additional-properties useBeanValidation=false --additional-properties useJakartaEe=true
cd /tmp/resteasy234
gradle build

please take your time. appreciate your help to improve the jaxrs-resteasy generator

@wing328 wing328 added this to the 7.8.0 milestone Jul 7, 2024
@hamburml hamburml deleted the FixResteasyGenerator branch July 7, 2024 13:00
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.

[BUG][JAVA] jaxrs-resteasy Generator and useJakartaEe still imports packages from javax instead of jakarta
2 participants