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 11 native HTTP client library #3434

Merged
merged 3 commits into from
Jul 26, 2019

Conversation

bbdouglas
Copy link
Contributor

@bbdouglas bbdouglas commented Jul 24, 2019

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

This is a set of mustache templates to create a new Java library called "native" that is backed by the standard Java 11 HTTPClient class.

The majority of files are only lightly edited from the default Java templates. The model classes, for example, are entirely untouched. The main two edits are:

  • APIClient.mustache
  • api.mustache

It follows the pattern used by Jersey, where the APIClient acts as a mechanism for configuring the clients along with a small repository of utility functions. The individual api classes are created from the settings and helper classes held by the APIClient, and are immutable and thread-safe. Object serialization/deserialization is handled by Jackson, as is common in the other libraries.

We at Yelp currently use these templates to generate clientlibs used in our production microservices architecture. This library lacks some of the features seen in other openapi-generator libraries, but represents a fully functional subset for our use case. Additional features could be added on as needed. Here is a summary of the limitations of the current clientlibs:

  • No Validation
  • No Auth
  • No HTML Forms
  • Only native Java dates
  • Only application/json mimetype

resolves #3433

@bbdouglas
Copy link
Contributor Author

This is intended to resolve #3433

Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks a lot!
I haven't tested it, though.
Might it be possible to add a small integration test to avoid breaking this code in the future?

@macjohnny
Copy link
Member

@bbdouglas can you please run bin/utils/export_generators_readme.sh and commit the updated docs file docs/generators/java.md?
moreover, please also add samples/client/petstore/java/native/gradlew.bat

@macjohnny macjohnny added this to the 4.1.0 milestone Jul 24, 2019
@bbdouglas
Copy link
Contributor Author

@macjohnny I went ahead and completed the tasks you suggested:

  • Added an integration test for ApiClient
  • Added back gradlew.bat, but without Windows newlines this time (which Travis had complained about before)
  • Updated docs/generators/java.md

Note, I ran bin/utils/export_generators_readme.sh, but it didn't have any effect on the repo. I believe that script changes docs/generators.md, but seeing as I'm only adding a library to the existing java generator, it didn't have any effect on that file.

@macjohnny
Copy link
Member

@bbdouglas thanks for the changes!
one last thing: in order to have your tests run in CI, please add them to

<module>samples/client/petstore/java/rest-assured</module>

and add a pom.xml that references your tests, see e.g.
https://github.com/OpenAPITools/openapi-generator/blob/master/samples/client/petstore/java/rest-assured/pom.xml

@bbdouglas bbdouglas force-pushed the native_httpclient branch from 0076062 to a0753cd Compare July 25, 2019 18:52
@bbdouglas
Copy link
Contributor Author

Hi @macjohnny , thanks for the heads up about #3455 . I went ahead and rebased and added a JSR305 dependency to the pom/gradle templates to support the new @Nullable annotation.

I have a pom in samples/client/petstore/java/native that runs the tests, but I'm not sure where to reference it in the top-level pom because it needs Java 11 to run. I see profiles for Java 7, 8 and 9, but nothing for Java 11. It looks like we use a combination of Circle CI and Shippable CI, but I don't have much experience with either to know how to configure them.

@wing328
Copy link
Member

wing328 commented Jul 26, 2019

I have a pom in samples/client/petstore/java/native that runs the tests, but I'm not sure where to reference it in the top-level pom because it needs Java 11 to run. I see profiles for Java 7, 8 and 9, but nothing for Java 11. It looks like we use a combination of Circle CI and Shippable CI, but I don't have much experience with either to know how to configure them.

Very good point. We may need to test it in Shippable or another CI running JDK11+.

I'm in the process to setup another CI (free) so maybe we can use that one to test this new client. More on this later.

Copy link
Member

@wing328 wing328 left a comment

Choose a reason for hiding this comment

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

LGTM

@wing328 wing328 merged commit 4359058 into OpenAPITools:master Jul 26, 2019
@bbdouglas bbdouglas deleted the native_httpclient branch July 26, 2019 03:23
@wing328
Copy link
Member

wing328 commented Jul 26, 2019

FYI. Setup drone.io (CI) to cover the Java native client: #3472

@wing328
Copy link
Member

wing328 commented Jul 27, 2019

Added @bbdouglas as the template creator via b9e863a

@wing328
Copy link
Member

wing328 commented Aug 10, 2019

@bbdouglas thanks for the PR, which has been included in the 4.1.0 release: https://twitter.com/oas_generator/status/1160000504455319553

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.

[REQ] [JAVA] Java 11 Native HTTP client library
3 participants