feat(retrofit2): Improvise OkHttpClientProvider to accept interceptors #1220
Conversation
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
dbyron-sf
reviewed
Jan 8, 2025
| ObjectMapper objectMapper, | ||
| List<Interceptor> interceptors) { | ||
| // retrofit1 doesn't support okhttp3.Interceptor. This method override is just to fulfill the | ||
| // contract. |
There was a problem hiding this comment.
Safer to throw an exception here if interceptors isn't empty?
dbyron-sf
reviewed
Jan 8, 2025
| Class<T> type, | ||
| ServiceEndpoint serviceEndpoint, | ||
| ObjectMapper objectMapper, | ||
| List<Interceptor> interceptors); // This is supported only by Retrofit2ServiceFactory |
There was a problem hiding this comment.
I suspect this comment will get stale over time.
|
We could use some test coverage for the new methods. |
…tProvider to support list of interceptors.
kirangodishala
force-pushed
the
improvise-okhttpclientprovider
branch
from
78b1111 to
8e409c1
Compare
dbyron-sf
approved these changes
Jan 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As an alternate to using
OkHttp3ClientConfigurationfor creating an OkHttpClient,OkHttpClientProviderwas created through#639. But there are many existing retrofit clients that are passing Interceptor(s) which is not supported by
OkHttpClientProvider. This PR allowsOkHttpClientProviderto acceptokhttp3.Interceptorlist.As we are slowly moving to retrofit2(rosco completed, echo under review), the real consumer of OkHttpClientProvider will be
Retrofit2ServiceFactory(as opposed toRetrofitServiceFactory) but since it's an implementation ofServiceClientFactory, I had to add a method that takesList<Interceptor>which is specific toRetrofit2ServiceFactoryand hence the method has no significance wrtRetrofitServiceFactory.The idea is to use
OkHttpClientProviderinstead ofOkHttp3ClientConfigurationwhile upgrading the http clients from retrofit1 to retrofit2.