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][*] Annotate deprecated operations and schemas #9478

Merged

Conversation

jenseng
Copy link
Contributor

@jenseng jenseng commented May 14, 2021

Refs #3358

Ensure deprecated operations are annotated/documented as such on the
generated methods. Libraries updated:

  • [feign]
  • [google-api-client]
  • [microprofile]
  • [okhttp-gson]
  • [resttemplate]
  • [retrofit]
  • [retrofit/play*]
  • [webclient]
  • [vertx]

Ensure deprecated schemas are annotated/documented as such on the
generated classes/fields. Libraries updated:

  • [feign]
  • [google-api-client]
  • [jersey2]
  • [microprofile]
  • [native]
  • [okhttp-gson]
  • [rest-assured]
  • [resteasy]
  • [resttemplate]
  • [retrofit*]
  • [webclient]
  • [vertx]

Also fix two minor bugs to get the java sample tests working:

  • Fix an invalid jackson-datatype-threetenbp version number in vertx/pom.mustache
  • Fix a bad return type in webclient/api_test.mustache when uniqueItems=true

Since this commit updates petstore-with-fake-endpoints-models-for-testing.yaml,
several other samples were updated, but it's just new files to reflect the
deprecated schemas, so there should be no consequential differences. 🤞

Relevant bits of the spec:

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/utils/export_docs_generators.sh
    
    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*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master, 5.1.x, 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request. CC @bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @nmuesch

@jenseng jenseng force-pushed the improve-deprecated-support-in-java branch 3 times, most recently from 15fcec4 to a68965c Compare May 14, 2021 20:02
@wing328 wing328 added this to the 5.2.0 milestone May 31, 2021
@cljk
Copy link

cljk commented Jun 9, 2021

AFAIK marking Operations as deprecated is working - marking model-fields as deprecated is not

@jenseng
Copy link
Contributor Author

jenseng commented Jun 9, 2021

@cljk can you elaborate a bit? AFAICT it's working correctly with this commit... see ObjectWithDeprecatedFields in the schema and in all the newly generated samples, for example this one

@jenseng
Copy link
Contributor Author

jenseng commented Jun 9, 2021

And to clarify, prior to this commit "marking Operations as deprecated" was only supported for a few Java clients (jersey2, resteasy, rest-assured, some retrofit) ... this commit brings along all the other Java clients

@cljk
Copy link

cljk commented Jun 9, 2021

Oh, I´m sorry. I completely missed out your commit / pull request. I thought this was just an open issue description.

I checked out your pull request for 5.2.0-SNAPSHOT, regenerated my client code and can confirm that it works as it should with deprecation marking for fields AND types.
Checked: jersey, native, resteasy, retrofit

I would by glad if this would be merged and released soon.

@cljk
Copy link

cljk commented Jun 9, 2021

I had an uneducated look in the circleci output and tried to figure out why it failed.
Perhaps I´m wrong but it looks to me like the test env. is not set up correctly - which should not have anything to do with your commit?! Missing go reference?!

  232 passing (355ms)

go: downloading github.com/gorilla/mux v1.7.3
?   	github.com/GIT_USER_ID/GIT_REPO_ID	[no test files]
?   	_/home/circleci/OpenAPITools/openapi-generator/samples/server/petstore/go-gin-api-server	[no test files]
go go: unknown command
Run 'go help' for usage.
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (go-get) on project GoEchoServer: Command execution failed. Process exited with an error: 2 (Exit value: 2) -> [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/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :GoEchoServer
2021-05-31 16:56:34.514:INFO::main: Logging initialized @136ms
2021-05-31 16:56:34.532:INFO:oejr.Runner:main: Runner
2021-05-31 16:56:34.683:INFO:oejs.Server:main: jetty-9.2.9.v20150224
May 31, 2021 4:56:38 PM com.sun.jersey.api.core.PackagesResourceConfig init

@jenseng
Copy link
Contributor Author

jenseng commented Jun 9, 2021

I'll rebase and see if it behaves better now, at the time I opened the PR it seems that most builds were failing the same way, perhaps the issue has been fixed on master?

@jenseng jenseng force-pushed the improve-deprecated-support-in-java branch from a68965c to 0d55c7e Compare June 9, 2021 23:14
@cljk
Copy link

cljk commented Jun 10, 2021

This time travis, also an error and also an error of the test setup - not code modifications?!

Perhaps the maintainers can tell something about that 🤷🏼‍♂️

docker pull swaggerapi/petstore
Using default tag: latest
Error response from daemon: toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit
The command "docker pull swaggerapi/petstore" failed and exited with 1 during .
Your build has been stopped.

@cljk
Copy link

cljk commented Jun 16, 2021

@wing328
Could you (or any other) please have a look into this pull request or say if it has good chances to be merged to 5.2.0 ?
I would really like to have this fix...

Refs OpenAPITools#3358

Ensure `deprecated` operations are annotated/documented as such on the
generated methods. Libraries updated:
  * [feign]
  * [google-api-client]
  * [microprofile]
  * [okhttp-gson]
  * [resttemplate]
  * [retrofit]
  * [retrofit/play*]
  * [webclient]
  * [vertx]

Ensure `deprecated` schemas are annotated/documented as such on the
generated classes/fields. Libraries updated:
  * [feign]
  * [google-api-client]
  * [jersey2]
  * [microprofile]
  * [native]
  * [okhttp-gson]
  * [rest-assured]
  * [resteasy]
  * [resttemplate]
  * [retrofit*]
  * [webclient]
  * [vertx]

Also fix two minor bugs to get the java sample tests working:

* Fix an invalid jackson-datatype-threetenbp version number in vertx/pom.mustache
* Fix a bad return type in webclient/api_test.mustache when uniqueItems=true

Since this commit updates petstore-with-fake-endpoints-models-for-testing.yaml,
several other samples were updated, but it's just new files to reflect the
deprecated schemas, so there should be no consequential differences.

Relevant bits of the spec:

* https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.2.md#user-content-operationdeprecated
* https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.2.md#user-content-schemadeprecated
@jenseng jenseng force-pushed the improve-deprecated-support-in-java branch from 0d55c7e to fd99bf5 Compare June 29, 2021 17:10
@jenseng
Copy link
Contributor Author

jenseng commented Jun 29, 2021

Rebased and resolved conflicts around 95a11a7. Also regenerated all samples, which brought along fake/body-with-binary stuff from f7b93eb.

@wing328 wing328 modified the milestones: 5.2.0, 5.2.1 Jul 13, 2021
@wing328
Copy link
Member

wing328 commented Jul 19, 2021

Sorry for the delay in reviewing as there are too many PRs to review.

I tested it locally and the result is good. We may remove some newly added samples introduced in this PR while we're consolidating the test samples.

@wing328 wing328 merged commit bd07030 into OpenAPITools:master Jul 19, 2021
@jenseng
Copy link
Contributor Author

jenseng commented Jul 19, 2021

Thanks @wing328!

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