-
Notifications
You must be signed in to change notification settings - Fork 6k
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] [okhttp-gson] Update OkHttp to version 3.x #9632
Conversation
The current implementation of the okhttp-gson client uses OkHttp 2.x as the underlying client. This library was superseded by version 3.x in early 2016. Version 3.x of the library is still under active development. Update codegen templates for okhttp-gson to use version 3.x of the OkHttp client library. Signed-off-by: Nick Travers <n.e.travers@gmail.com>
cc: @frantuma |
This is a first pass at porting to OkHttp 3.x. Please let me know if it makes sense to have this in a completely separate package. The changes aren't too invasive (only one new public method was added), but as mentioned on #9606, this is technically a breaking change as the underlying okhttp package name changed to There were some seemingly unrelated changes to some files after running the Unit tests in |
@@ -2,10 +2,20 @@ | |||
|
|||
package {{invokerPackage}}; | |||
|
|||
import com.squareup.okhttp.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Google styleguide discourages wildcards:
https://google.github.io/styleguide/javaguide.html#s3.3.1-wildcard-imports
If it's preferable to use wildcards from a codegen perspective, I can revert.
@@ -454,7 +468,9 @@ public class ApiClient { | |||
* @return Api client | |||
*/ | |||
public ApiClient setConnectTimeout(int connectionTimeout) { | |||
httpClient.setConnectTimeout(connectionTimeout, TimeUnit.MILLISECONDS); | |||
httpClient = httpClient.newBuilder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One notable change in version 3.x is that a client cannot be modified after it has been created. The alternative is to create a new client by converting back to a builder, modifying the relevant fields and the instantiating a new client. I've adopted this pattern throughout, where client mutations need to take place.
compile 'com.google.code.gson:gson:2.8.1' | ||
compile 'io.gsonfire:gson-fire:1.8.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some additional changes in petstore-security-test
that don't seem related to the OkHttp 3.x change. I'm guessing something wasn't updated in a previous commit.
Signed-off-by: Nick Travers <n.e.travers@gmail.com>
37a4604
to
b483ef0
Compare
Bumping this. Would someone be able to help me out with finding a reviewer? cc: @wing328 |
You might try submitting this here: |
Could someone point out to me how you generate okhttp v3? I always get 2.7.5? |
PR checklist
./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). Windows batch files can be found in.\bin\windows\
.3.0.0
branch for changes related to OpenAPI spec 3.0. Default:master
.Description of the PR
The current implementation of the okhttp-gson client uses OkHttp 2.x as
the underlying client. This library was superseded by version 3.x in
early 2016. Version 3.x of the library is still under active
development.
Update codegen templates for okhttp-gson to use version 3.x of the
OkHttp client library.
Closes #9606.