20

Hey there I am using Dagger2, Retrofit and OkHttp and I am facing dependency cycle issue.

When providing OkHttp :

@Provides
@ApplicationScope
OkHttpClient provideOkHttpClient(TokenAuthenticator auth,Dispatcher dispatcher){
    return new OkHttpClient.Builder()
            .connectTimeout(Constants.CONNECT_TIMEOUT, TimeUnit.SECONDS)
            .readTimeout(Constants.READ_TIMEOUT,TimeUnit.SECONDS)
            .writeTimeout(Constants.WRITE_TIMEOUT,TimeUnit.SECONDS)
            .authenticator(auth)
            .dispatcher(dispatcher)
            .build();
}

When providing Retrofit :

@Provides
@ApplicationScope
Retrofit provideRetrofit(Resources resources,Gson gson, OkHttpClient okHttpClient){
    return new Retrofit.Builder()
            .baseUrl(resources.getString(R.string.base_api_url))
            .addConverterFactory(GsonConverterFactory.create(gson))
            .addCallAdapterFactory(RxJava2CallAdapterFactory.create())
            .client(okHttpClient)
            .build();
}

When providing APIService :

@Provides
@ApplicationScope
APIService provideAPI(Retrofit retrofit) {
    return retrofit.create(APIService.class);
}

My APIService interface :

public interface  APIService {
@FormUrlEncoded
@POST("token")
Observable<Response<UserTokenResponse>> refreshUserToken();

--- other methods like login, register ---

}

My TokenAuthenticator class :

@Inject
public TokenAuthenticator(APIService mApi,@NonNull ImmediateSchedulerProvider mSchedulerProvider) {
    this.mApi= mApi;
    this.mSchedulerProvider=mSchedulerProvider;
    mDisposables=new CompositeDisposable();
}

@Override
public  Request authenticate(Route route, Response response) throws IOException {

    request = null;

    mApi.refreshUserToken(...)
            .subscribeOn(mSchedulerProvider.io())
            .observeOn(mSchedulerProvider.ui())
            .doOnSubscribe(d -> mDisposables.add(d))
            .subscribe(tokenResponse -> {
                if(tokenResponse.isSuccessful()) {
                    saveUserToken(tokenResponse.body());
                    request = response.request().newBuilder()
                            .header("Authorization", getUserAccessToken())
                            .build();
                } else {
                    logoutUser();
                }
            },error -> {

            },() -> {});

    mDisposables.clear();
    stop();
    return request;

}

My logcat :

Error:(55, 16) error: Found a dependency cycle:
com.yasinkacmaz.myapp.service.APIService is injected at com.yasinkacmaz.myapp.darkvane.modules.NetworkModule.provideTokenAuthenticator(…, mApi, …)
com.yasinkacmaz.myapp.service.token.TokenAuthenticator is injected at
com.yasinkacmaz.myapp.darkvane.modules.NetworkModule.provideOkHttpClient(…, tokenAuthenticator, …)
okhttp3.OkHttpClient is injected at
com.yasinkacmaz.myapp.darkvane.modules.NetworkModule.provideRetrofit(…, okHttpClient)
retrofit2.Retrofit is injected at
com.yasinkacmaz.myapp.darkvane.modules.NetworkModule.provideAPI(retrofit)
com.yasinkacmaz.myapp.service.APIService is provided at
com.yasinkacmaz.myapp.darkvane.components.ApplicationComponent.exposeAPI()

So my question: My TokenAuthenticator class is depends on APIService but I need to provide TokenAuthenticator when creating APIService. This causes dependency cycle error. How do I beat this , is there anyone facing this issue ? Thanks in advance.

Prokash Sarkar
  • 11,723
  • 1
  • 37
  • 50
Yasin Kaçmaz
  • 6,573
  • 5
  • 40
  • 58
  • because it doesn't make sens ... OkHttpClient with TokenAuthenticator for getting auth token needed by TokenAuthenticator ... it has "circular dependency" even "on paper" ... create another service for getting auth token with another instance of http client without authenticator – Selvin May 11 '17 at 12:01
  • TokenAuthenticator is for refreshing user token and I want to use same OkHttp instance for every network call. Because of managing user token. I have dispatcher at that OkHttp instance – Yasin Kaçmaz May 11 '17 at 12:02
  • 1
    Again, it doesn't make sens ... even if you fix it, it will cause stackoverflow ... you are calling `refreshUserToken` from `authenticate` ... but `refreshUserToken` needs to call `authenticate` – Selvin May 11 '17 at 12:26
  • @Selvin Authenticate works when I got 401 error code and then I refresh my token using refreshUserToken then continue my work. refreshUserToken method do not need to call authenticate – Yasin Kaçmaz May 11 '17 at 12:35
  • 1
    refreshUserToken is using OkHttpClient which is setup to use TokenAuthenticator which is using refreshUserToken ... how *refreshUserToken method do not need to call authenticate* can be true? – Selvin May 11 '17 at 12:37
  • @Selvin I think your sentence couple hours and ended up with this solution : Use another instance of `OkHttp` for `TokenAuthenticator` and `TokenInterceptor` classes because of they only trigger when our general `OkHttp` instance makes requests. So they are not bound. – Yasin Kaçmaz May 11 '17 at 20:42
  • @Yasin, I'm facing the same problem, how did you get the solution, please help me. – Bajrang Hudda Jan 19 '19 at 08:00
  • Dear @BajrangHudda I have an answer below this page (it was deleted somehow I undeleted it) based on using Qualifiers. Was written long time ago but can you read and try yo implement that. – Yasin Kaçmaz Jan 19 '19 at 08:56

4 Answers4

31

Your problem is:

  1. Your OKHttpClient depends on your Authenticator
  2. Your Authenticator depends on a Retrofit Service
  3. Retrofit depends on an OKHttpClient (as in point 1)

Hence the circular dependency.

One possible solution here is for your TokenAuthenticator to depend on an APIServiceHolder rather than a APIService. Then your TokenAuthenticator can be provided as a dependency when configuring OKHttpClient regardless of whether the APIService (further down the object graph) has been instantiated or not.

A very simple APIServiceHolder:

public class APIServiceHolder {

    private APIService apiService;

    @Nullable
    APIService apiService() {
        return apiService;
    }

    void setAPIService(APIService apiService) {
        this.apiService = apiService;
    }
}

Then refactor your TokenAuthenticator:

@Inject
public TokenAuthenticator(@NonNull APIServiceHolder apiServiceHolder, @NonNull ImmediateSchedulerProvider schedulerProvider) {
    this.apiServiceHolder = apiServiceHolder;
    this.schedulerProvider = schedulerProvider;
    this.disposables = new CompositeDisposable();
}

@Override
public  Request authenticate(Route route, Response response) throws IOException {

    if (apiServiceHolder.get() == null) {
         //we cannot answer the challenge as no token service is available

         return null //as per contract of Retrofit Authenticator interface for when unable to contest a challenge
    }    

    request = null;            

    TokenResponse tokenResponse = apiServiceHolder.get().blockingGet()

    if (tokenResponse.isSuccessful()) {
        saveUserToken(tokenResponse.body());
        request = response.request().newBuilder()
                     .header("Authorization", getUserAccessToken())
                     .build();
    } else {
       logoutUser();
    }

    return request;
}

Note that the code to retrieve the token should be synchronous. This is part of the contract of Authenticator. The code inside the Authenticator will run off the main thread.

Of course you will need to write the @Provides methods for the same:

@Provides
@ApplicationScope
apiServiceHolder() {
    return new APIServiceHolder();
}

And refactor the provider methods:

@Provides
@ApplicationScope
APIService provideAPI(Retrofit retrofit, APIServiceHolder apiServiceHolder) {
    APIService apiService = retrofit.create(APIService.class);
    apiServiceHolder.setAPIService(apiService);
    return apiService;
}

Note that mutable global state is not usually a good idea. However, if you have your packages organised well you may be able to use access modifiers appropriately to avoid unintended usages of the holder.

David Rawson
  • 20,912
  • 7
  • 88
  • 124
  • I have two ways. One of them is this and the other one is: Use another instance of OkHttp for TokenAuthenticator and TokenInterceptor classes because of they only trigger when any APIService request call so they are not bound. Also in this way I will seperate token process from my other requests so I can easily maintain. Which one you suggest ? – Yasin Kaçmaz May 12 '17 at 12:36
  • @Yasin I would personally prefer my solution to having two instances of OkHttpClient. – David Rawson May 12 '17 at 20:33
  • @Yasin if you look at the docs for Authenticator it seems it is designed to work with a single OkHttpClient. The code in authenticate is executed on the same thread as the original request. – David Rawson May 12 '17 at 20:38
  • thanks for reply. I just want to make sure this: since it is on the same thread I can use whatever I want. I mean `OkHttp`, `Retrofit` or even `HttpUrlConnection` because of I am making synchronous requests at `Authenticator` class which means they block that thread until job done. So seperating `OkHttp` instances for Token and regular api calls is good approach or maybe not? – Yasin Kaçmaz May 12 '17 at 20:46
  • @Yasin either approach will do – David Rawson May 12 '17 at 20:50
  • @Yasin and yes the call to the token service inside the authenticate method should be synchronous – David Rawson May 12 '17 at 20:51
  • I should post an answer too, future viewers can see difference and they can select whichever they like. Again thank you so much. – Yasin Kaçmaz May 12 '17 at 20:52
  • @Yasin yes you can post your solution as well – David Rawson May 12 '17 at 21:08
  • would be nice to mark this as the accepted answer. the answer provides clear explanation along with complete code that solves the issue. – Steve Yohanan Jun 07 '18 at 18:26
  • 4
    APIServiceHolder always return null in my case. – thalissonestrela Jun 21 '18 at 16:54
  • in the code above "request" is the same as "response.request()" ? response in that case is the parameter of the Authentificator – BMU Jun 27 '18 at 19:27
  • I follow your suggestion but in my case, the request = null always return first and then the refreshToken got a response, so it always response 401 although after that the new refreshToken got successfully, how can I deal with it? – Khang Tran Aug 23 '18 at 09:33
  • @khangtran maybe make the call to the refresh token API block – David Rawson Aug 23 '18 at 09:35
  • @David : sorry I don't understand yours mean? – Khang Tran Aug 23 '18 at 09:39
  • @khangtran can you try the solution here : https://stackoverflow.com/a/43227410/5241933 – David Rawson Aug 23 '18 at 09:43
  • those who are getting a null value for apiService, please make sure apiServiceHolder provider written before provideAPI & marked singleton – Rax Jul 25 '19 at 10:52
  • How can we logout user (I mean call logout screen)? Is there a listener for situation when authenticate returns null so we can respond to it and logout user? – Zoran Jan 20 '20 at 12:28
  • 1
    Isn't this still a circular dependency? You are hiding the service in a wrapper and kind of applying a lazy initialization but the TokenAuthenticator is still dependant on the service. – Tartar Feb 06 '20 at 14:33
  • I copied your code but I can't call apiServiceHolder.get() .. and get is undefined can you please help me @DavidRawson?? – Anice Jahanjoo Feb 16 '20 at 07:24
  • furthermore when you set apiService ? it always is null :( – Anice Jahanjoo Feb 16 '20 at 07:31
  • How we can add TokenAuthenticator in okHttpClientBuilder, because it has two parameters now authenticator(TokenAuthenticator(...)) – Shivang Apr 24 '20 at 10:28
9

Using the Lazy interface of Dagger 2 is the solution here. In your TokenAuthenticator replace APIService mApi with Lazy<APIService> mApiLazyWrapper

@Inject
public TokenAuthenticator(Lazy<APIService> mApiLazyWrapper,@NonNull ImmediateSchedulerProvider mSchedulerProvider) {
    this.mApiLazyWrapper= mApiLazyWrapper;
    this.mSchedulerProvider=mSchedulerProvider;
    mDisposables=new CompositeDisposable();
}

And to get the APIService instance from wrapper use mApiLazyWrapper.get()

In case mApiLazyWrapper.get() returns null, return null from the authenticate method of TokenAuthenticator as well.

Tartar
  • 5,149
  • 16
  • 63
  • 104
  • 1
    Dear @Tartar this question is from 2017 which is roughly 3 years old. This may work as well but our APIService still holds TokenAuthenticator. The only difference is that we delay injecting it. So IMHO I prefer having a simple client for authentication flow, may be isolated from couple interceptors. – Yasin Kaçmaz Feb 09 '20 at 20:10
  • 4
    This is the Dagger way of solving the cyclic dependency error so the answer would be useful for those who want to solve the issue in that way instead of having multiple HTTP clients. – Tartar Feb 09 '20 at 20:29
  • Yes, this would be a guide for new viewers. Also, @max has an answer about Lazy too. There are several ways to deal with dependency cycle problem, they can choose the appropriate solution for their app – Yasin Kaçmaz Feb 09 '20 at 20:46
  • I think this is the best solution now. I don't believe Lazy was around when I wrote the initial answer – David Rawson Jul 24 '20 at 09:38
  • How to do this with Hilt? – Torima Jun 03 '22 at 16:05
  • you're a life-saver! thanks, this works. – mrzbn Aug 18 '23 at 20:52
1

Big thanks to @Selvin and @David. I have two approach, one of them is David's answer and the other one is :

Creating another OkHttp or Retrofit or another library which will handle our operations inside TokenAuthenticator class.

If you want to use another OkHttp or Retrofit instance you must use Qualifier annotation.

For example :

@Qualifier
public @interface ApiClient {}


@Qualifier
public @interface RefreshTokenClient {}

then provide :

@Provides
@ApplicationScope
@ApiClient
OkHttpClient provideOkHttpClientForApi(TokenAuthenticator tokenAuthenticator, TokenInterceptor tokenInterceptor, Dispatcher dispatcher){
    return new OkHttpClient.Builder()
            .connectTimeout(Constants.CONNECT_TIMEOUT, TimeUnit.SECONDS)
            .readTimeout(Constants.READ_TIMEOUT,TimeUnit.SECONDS)
            .writeTimeout(Constants.WRITE_TIMEOUT,TimeUnit.SECONDS)
            .authenticator(tokenAuthenticator)
            .addInterceptor(tokenInterceptor)
            .dispatcher(dispatcher)
            .build();
}

@Provides
@ApplicationScope
@RefreshTokenClient
OkHttpClient provideOkHttpClientForRefreshToken(Dispatcher dispatcher){
    return new OkHttpClient.Builder()
            .connectTimeout(Constants.CONNECT_TIMEOUT, TimeUnit.SECONDS)
            .readTimeout(Constants.READ_TIMEOUT,TimeUnit.SECONDS)
            .writeTimeout(Constants.WRITE_TIMEOUT,TimeUnit.SECONDS)
            .dispatcher(dispatcher)
            .build();
}

@Provides
@ApplicationScope
@ApiClient
Retrofit provideRetrofitForApi(Resources resources, Gson gson,@ApiClient OkHttpClient okHttpClient){
    return new Retrofit.Builder()
            .baseUrl(resources.getString(R.string.base_api_url))
            .addConverterFactory(GsonConverterFactory.create(gson))
            .addCallAdapterFactory(RxJava2CallAdapterFactory.create())
            .client(okHttpClient)
            .build();
}

@Provides
@ApplicationScope
@RefreshTokenClient
Retrofit provideRetrofitForRefreshToken(Resources resources, Gson gson,@RefreshTokenClient OkHttpClient okHttpClient){
    return new Retrofit.Builder()
            .baseUrl(resources.getString(R.string.base_api_url))
            .addConverterFactory(GsonConverterFactory.create(gson))
            .addCallAdapterFactory(RxJava2CallAdapterFactory.create())
            .client(okHttpClient)
            .build();
}

Then we can provide our seperated interfaces :

@Provides
@ApplicationScope
public APIService provideApi(@ApiClient Retrofit retrofit) {
    return retrofit.create(APIService.class);
}

@Provides
@ApplicationScope
public RefreshTokenApi provideRefreshApi(@RefreshTokenClient Retrofit retrofit) {
    return retrofit.create(RefreshTokenApi.class);
}

When providing our TokenAuthenticator :

@Provides
@ApplicationScope
TokenAuthenticator provideTokenAuthenticator(RefreshTokenApi mApi){
    return new TokenAuthenticator(mApi);
}

Advantages : You have two seperated api interfaces which means you can maintain them independently. Also you can use plain OkHttp or HttpUrlConnection or another library.

Disadvantages : You will have two different OkHttp and Retrofit instance.

P.S : Make sure you make syncronous calls inside Authenticator class.

Community
  • 1
  • 1
Yasin Kaçmaz
  • 6,573
  • 5
  • 40
  • 58
  • I am beginner for dagger 2, when I used your approach I start getting error in my previous apiService ApiService cannot be provided without an @Provides-annotated method. What I am missing? – Bajrang Hudda Jan 23 '19 at 12:33
  • Dear @BajrangHudda I cannot help you if you don't show your code. I mean your components, modules. Also if you are beginner with dagger I highly recommend you to read Dagger samples and try to understand what dependency injection is. – Yasin Kaçmaz Jan 23 '19 at 12:51
  • I used David's answer, is there anything drawback? – Bajrang Hudda Jan 25 '19 at 05:33
  • 1
    @BajrangHudda I saw no drawback on David's answer; but you know in software sometimes there is multiple solutions available for you. And I personally do not like nullable things inside dependency injection modules. Also this question is old tho and I am using `Kotlin` and `Dagger` right now. `Dagger` has been updated a lot and let me look at this question and see if any other workaround. – Yasin Kaçmaz Jan 25 '19 at 07:26
  • I think that's why I'm getting - HTTP FAILED: java.lang.IllegalArgumentException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkParameterIsNotNull, parameter route – Bajrang Hudda Jan 25 '19 at 07:29
  • @BajrangHudda I guess we can use lateinit instead of null since you are using kotlin too. Also in my answer I will update qualifier please try it like that too – Yasin Kaçmaz Jan 25 '19 at 07:30
  • 1
    @BajrangHudda also there is [Lazy](https://google.github.io/dagger/api/2.10/dagger/Lazy.html) at Dagger too. Did not used it but you can take a look at it. – Yasin Kaçmaz Jan 25 '19 at 07:37
  • @YasinKaçmaz why we have to make synchronous calls inside the Authenticator class? – Tartar Feb 06 '20 at 12:04
  • @Tartar We must return request from `Authenticator#authenticate` method and `OkHttp` will retry with that request. So in order to refresh token header of existing request before retry we must block that thread until refresh token process finish. – Yasin Kaçmaz Feb 06 '20 at 13:17
  • @YasinKaçmaz I assume it's not Android's main thread and OkHttp has its own thread for that? – Tartar Feb 06 '20 at 14:03
  • @Tartar I really don't know your thread management, which thread you are using is up to you. Also this question about Dagger graph. You can try and debug your code to see thread it may help you to see. – Yasin Kaçmaz Feb 06 '20 at 14:15
0

You can inject the service dependency into your authenticator via the Lazy type. This way you will avoid the cyclic dependency on instantiation.

Check this link on how Lazy works.

max
  • 138
  • 7