Use OkHttp for network calls #431
Conversation
rajdeepnanua-okta
force-pushed
the
use_okhttp_networking
branch
from
9d79bd4
to
cfb0c33
Compare
rajdeepnanua-okta
force-pushed
the
use_okhttp_networking
branch
2 times, most recently
from
3adae66
to
172f537
Compare
rajdeepnanua-okta
force-pushed
the
use_okhttp_networking
branch
5 times, most recently
from
42e8450
to
c41f294
Compare
rajdeepnanua-okta
force-pushed
the
use_okhttp_networking
branch
from
c41f294
to
a086cf5
Compare
| @@ -1,5 +1,4 @@ | |||
| { | |||
| "private": true, | |||
There was a problem hiding this comment.
This isn't a private package, and CI publish script throws an error with the new yarn if this is still set to true. To match the previous behavior where CI does publish this package to our internal artifactory, I had to remove this.
| - run: brew install git-lfs | ||
| - run: git lfs install | ||
| - run: git lfs pull |
There was a problem hiding this comment.
Despite my efforts here, CircleCI is still broken. I will address this in a separate commit.
| @@ -12,166 +12,161 @@ | |||
|
|
|||
| package com.oktareactnative; | |||
|
|
|||
| import android.annotation.SuppressLint; | |||
| 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
There are a few changes to the reference implementation, namely customizable timeouts, and user-agent response headers as required by this SDK
| if (sOkHttpClient == null) { | ||
| sOkHttpClient = new OkHttpClient.Builder() | ||
| .connectTimeout(connectTimeoutMs, TimeUnit.MILLISECONDS) | ||
| .readTimeout(readTimeoutMs, TimeUnit.MILLISECONDS) | ||
| .build(); | ||
| } |
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.
| } | ||
|
|
||
| @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.
This SDK also doesn't make use of the cancel function
No description provided.