Use OkHttp for network calls #431
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,166 +12,161 @@ | |
|
|
||
| package com.oktareactnative; | ||
|
|
||
| import static com.okta.oidc.net.ConnectionParameters.USER_AGENT; | ||
|
There was a problem hiding this comment. The new implementation is mostly taken from here: https://github.com/okta/okta-oidc-android/blob/ce298473dc2b1e2eaced212c98f3d3acee636f5c/app/src/main/java/com/okta/oidc/example/OkHttp.java |
||
|
|
||
| import android.net.Uri; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
|
|
||
| import com.okta.oidc.BuildConfig; | ||
| import com.okta.oidc.net.ConnectionParameters; | ||
| import com.okta.oidc.net.OktaHttpClient; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.concurrent.CountDownLatch; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| import okhttp3.Call; | ||
| import okhttp3.Callback; | ||
| import okhttp3.FormBody; | ||
| import okhttp3.OkHttpClient; | ||
| import okhttp3.Request; | ||
| import okhttp3.RequestBody; | ||
| import okhttp3.Response; | ||
|
|
||
| public class HttpClientImpl implements OktaHttpClient { | ||
| private final String userAgentTemplate; | ||
| private final int connectTimeoutMs; | ||
| private final int readTimeoutMs; | ||
| protected static OkHttpClient sOkHttpClient; | ||
| protected volatile Call mCall; | ||
| protected Response mResponse; | ||
| protected Exception mException; | ||
|
|
||
| HttpClientImpl(String userAgentTemplate, int connectTimeoutMs, int readTimeoutMs) { | ||
| this.userAgentTemplate = userAgentTemplate; | ||
| this.connectTimeoutMs = connectTimeoutMs; | ||
| this.readTimeoutMs = readTimeoutMs; | ||
| } | ||
|
|
||
| private String getUserAgent() { | ||
| String sdkVersion = "okta-oidc-android/" + BuildConfig.VERSION_NAME; | ||
| return userAgentTemplate.replace("$UPSTREAM_SDK", sdkVersion); | ||
| } | ||
|
|
||
| protected Request buildRequest(Uri uri, ConnectionParameters param) { | ||
| if (sOkHttpClient == null) { | ||
| sOkHttpClient = new OkHttpClient.Builder() | ||
| .connectTimeout(connectTimeoutMs, TimeUnit.MILLISECONDS) | ||
| .readTimeout(readTimeoutMs, TimeUnit.MILLISECONDS) | ||
| .build(); | ||
| } | ||
|
Comment on lines
+61
to
66
There was a problem hiding this comment. It's perfectly okay if this code gets called by multiple callers at the same time. okHttpClient doesn't need to be singleton for correctness, but only for performance reasons. |
||
| Request.Builder requestBuilder = new Request.Builder().url(uri.toString()); | ||
| requestBuilder.addHeader(USER_AGENT, getUserAgent()); | ||
| for (Map.Entry<String, String> headerEntry : param.requestProperties().entrySet()) { | ||
| String key = headerEntry.getKey(); | ||
| requestBuilder.addHeader(key, headerEntry.getValue()); | ||
| } | ||
| if (param.requestMethod() == ConnectionParameters.RequestMethod.GET) { | ||
| requestBuilder = requestBuilder.get(); | ||
| } else { | ||
| Map<String, String> postParameters = param.postParameters(); | ||
| if (postParameters != null) { | ||
| FormBody.Builder formBuilder = new FormBody.Builder(); | ||
| for (Map.Entry<String, String> postEntry : postParameters.entrySet()) { | ||
| String key = postEntry.getKey(); | ||
| formBuilder.add(key, postEntry.getValue()); | ||
| } | ||
| RequestBody formBody = formBuilder.build(); | ||
| requestBuilder.post(formBody); | ||
| } else { | ||
| requestBuilder.post(RequestBody.create(null, "")); | ||
| } | ||
| } | ||
| return requestBuilder.build(); | ||
| } | ||
|
|
||
| @Override | ||
| public InputStream connect(@NonNull Uri uri, @NonNull ConnectionParameters params) | ||
| throws Exception { | ||
| Request request = buildRequest(uri, params); | ||
| mCall = sOkHttpClient.newCall(request); | ||
|
There was a problem hiding this comment. mCall is a class level variable here. It's only useful to keep track of it in case of cancellation. We can only track and cancel one call at a time with this implementation, but this was also a limitation with the previous implementation. |
||
| final CountDownLatch latch = new CountDownLatch(1); | ||
| mCall.enqueue(new Callback() { | ||
| @Override | ||
| public void onFailure(Call call, IOException e) { | ||
| mException = e; | ||
| latch.countDown(); | ||
| } | ||
|
|
||
| @Override | ||
| public void onResponse(Call call, Response response) { | ||
| mResponse = response; | ||
| latch.countDown(); | ||
| } | ||
| }); | ||
| latch.await(); | ||
| if (mException != null) { | ||
| throw mException; | ||
| } | ||
| if (mResponse != null && mResponse.body() != null) { | ||
| return mResponse.body().byteStream(); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public void cleanUp() { | ||
| //NO-OP | ||
| } | ||
|
|
||
| @Override | ||
| public void cancel() { | ||
| if (mCall != null) { | ||
| mCall.cancel(); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, List<String>> getHeaderFields() { | ||
| if (mResponse != null) { | ||
| return mResponse.headers().toMultimap(); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public String getHeader(String header) { | ||
| if (mResponse != null) { | ||
| return mResponse.header(header); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public int getResponseCode() throws IOException { | ||
| if (mResponse != null) { | ||
| return mResponse.code(); | ||
| } | ||
| return -1; | ||
| } | ||
|
|
||
| @Override | ||
| public int getContentLength() { | ||
| if (mResponse != null && mResponse.body() != null) { | ||
| return (int) mResponse.body().contentLength(); | ||
| } | ||
| return -1; | ||
| } | ||
|
|
||
| @Override | ||
| public String getResponseMessage() throws IOException { | ||
| if (mResponse != null) { | ||
| return mResponse.message(); | ||
| } | ||
| return null; | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Despite my efforts here, CircleCI is still broken. I will address this in a separate commit.