111 fixed, updated OkHttp 3.2.0->3.9.0 #236
Conversation
Codecov Report
@@ Coverage Diff @@
## master #236 +/- ##
===========================================
- Coverage 36.98% 35.18% -1.8%
Complexity 45 45
===========================================
Files 39 39
Lines 530 540 +10
Branches 27 28 +1
===========================================
- Hits 196 190 -6
- Misses 325 341 +16
Partials 9 9
Continue to review full report at Codecov.
|
| */ | ||
| public final class HostSelectionInterceptor implements Interceptor { | ||
| /** | ||
| * Using static variable in order to avoid adding this interceptor to ApplicationComponent |
There was a problem hiding this comment.
This comment seems to be out of sync with code
| } | ||
|
|
||
| @Override | ||
| public okhttp3.Response intercept(@NonNull Chain chain) throws IOException { |
| HttpUrl newUrl = request.url().newBuilder() | ||
| .host(uri.getHost()) | ||
| .port(uri.getPort()) | ||
| .scheme("http") //in order to avoid SSL Handshake failure |
There was a problem hiding this comment.
wow wow, this is pretty dangerous code in case if someone will copy-paste it and use to switch to lets say "staging/prod" environment in the app
I'd prefer to store schema in base url that we want to swap to, maybe use HttpUrl class instead of String and get basic info like host, schema and port
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class ChangeableBaseUrlTest { |
There was a problem hiding this comment.
Ideally we should have test for interceptor then
| public List<Interceptor> provideOkHttpInterceptors(@NonNull HttpLoggingInterceptor httpLoggingInterceptor) { | ||
| return singletonList(httpLoggingInterceptor); | ||
| public List<Interceptor> provideOkHttpInterceptors(@NonNull HttpLoggingInterceptor httpLoggingInterceptor, | ||
| @NonNull HostSelectionInterceptor hostSelectionInterceptor) { |
There was a problem hiding this comment.
is that the proper indentation for this project?
| @@ -0,0 +1,2 @@ | |||
| # OkHttp3 | |||
| -dontwarn okhttp3.** | |||
There was a problem hiding this comment.
we'll need more rules. there's one more for the publicsuffixdatabase thing. they've got it documented.
No description provided.