6

I'm creating a pet project with Hilt, and perhaps I'm having this issue because I'm installing everything in SingletonComponent::class, and perhaps I should create components for each one.

The pet project has a NetworkModule, UserPrefsModule, and the problem appeared when I was trying to create an Authenticator for OkHttp3.

This is my network module

@Module
@InstallIn(SingletonComponent::class)
object NetworkModule {

    @Singleton
    @Provides
    fun providesHttpLoggingInterceptor() = HttpLoggingInterceptor()
        .apply {
            if (BuildConfig.DEBUG) level = HttpLoggingInterceptor.Level.BODY
        }

    @Singleton
    @Provides
    fun providesErrorInterceptor(): Interceptor {
        return ErrorInterceptor()
    }

    @Singleton
    @Provides
    fun providesAccessTokenAuthenticator(
        accessTokenRefreshDataSource: AccessTokenRefreshDataSource,
        userPrefsDataSource: UserPrefsDataSource,
    ): Authenticator = AccessTokenAuthenticator(
        accessTokenRefreshDataSource,
        userPrefsDataSource,
    )

    @Singleton
    @Provides
    fun providesOkHttpClient(
        httpLoggingInterceptor: HttpLoggingInterceptor,
        errorInterceptor: ErrorInterceptor,
        authenticator: Authenticator,
    ): OkHttpClient =
        OkHttpClient
            .Builder()
            .authenticator(authenticator)
            .addInterceptor(httpLoggingInterceptor)
            .addInterceptor(errorInterceptor)
            .build()

}

Then my UserPrefsModule is :

@Module
@InstallIn(SingletonComponent::class)
object UserPrefsModule {

    @Singleton
    @Provides
    fun provideSharedPreference(@ApplicationContext context: Context): SharedPreferences {
        return context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE)
    }

    @Singleton
    @Provides
    fun provideUserPrefsDataSource(sharedPreferences: SharedPreferences): UserPrefsDataSource {
        return UserPrefsDataSourceImpl(sharedPreferences)
    }
}

Then I have an AuthenticatorModule

@Module
@InstallIn(SingletonComponent::class)
object AuthenticationModule {

    private const val BASE_URL = "http://10.0.2.2:8080/"

    @Singleton
    @Provides
    fun provideRetrofit(okHttpClient: OkHttpClient): Retrofit = Retrofit.Builder()
        .addConverterFactory(MoshiConverterFactory.create())
        .baseUrl(BASE_URL)
        .client(okHttpClient)
        .build()

    @Singleton
    @Provides
    fun provideApiService(retrofit: Retrofit): AuthenticationService =
        retrofit.create(AuthenticationService::class.java)


    @Singleton
    @Provides
    fun providesAccessTokenRefreshDataSource(
        userPrefsDataSource: UserPrefsDataSource,
        authenticationService: AuthenticationService,
    ): AccessTokenRefreshDataSource = AccessTokenRefreshDataSourceImpl(
        authenticationService, userPrefsDataSource
    )
}

The problem started to happen when I created the AccessTokenRefreshDataSourceImpl that I need the AuthenticationService and UserPrefsDataSource, and I'm getting this error :

error: [Dagger/DependencyCycle] Found a dependency cycle: public abstract static class SingletonC implements App_GeneratedInjector,

For each feature like Login, SignIn, Verification, etc.. I was creating a new @Module as this :

@Module
@InstallIn(SingletonComponent::class)
interface SignInModule {

    @Binds
    fun bindIsValidPasswordUseCase(
        isValidPasswordUseCaseImpl: IsValidPasswordUseCaseImpl,
    ): IsValidPasswordUseCase

    @Binds
    fun bindIsValidEmailUseCase(
        isValidEmailUseCase: IsValidEmailUseCaseImpl,
    ): IsValidEmailUseCase

     //Here in that Datasource I'm using the AuthenticationService from AuthenticationModule and it works
    @Binds
    fun bindSignInDataSource(
        signInDataSourceImpl: SignInDataSourceImpl
    ): SignInDataSource
}

Constructor of AccessTokenAutenticator

class AccessTokenAuthenticator @Inject constructor(
    private val accessTokenRefreshDataSource: AccessTokenRefreshDataSource,
    private val userPrefsDataSource: UserPrefsDataSource,
) : Authenticator {

Constructor of AccessTokenRefreshDatasource

class AccessTokenRefreshDataSourceImpl @Inject constructor(
    private val authenticationService: AuthenticationService,
    private val userPrefsDataSource: UserPrefsDataSource,
) : AccessTokenRefreshDataSource {

Note I have everything in a @Module separated by features for a future be able to modularise the app.

Marcin Orlowski
  • 72,056
  • 11
  • 123
  • 141
Skizo-ozᴉʞS ツ
  • 19,464
  • 18
  • 81
  • 148
  • The cyclic dependency is there, but without seeing the code of the implementations for `AccessTokenAuthenticator` and `AccessTokenRefreshDataSourceImpl` it is hard to say what needs to be done to break the cyclic dependency. – Ma3x Jan 08 '22 at 17:37
  • @Ma3x edited the question, take a look please. – Skizo-ozᴉʞS ツ Jan 08 '22 at 19:27

1 Answers1

13

In most programming languages, if you require an instance of B to construct A and an instance of A to construct B, then you won't be able to construct either.

Here:

  • AccessTokenRefreshDataSource requires AuthenticationService
  • AuthenticationService requires Retrofit
  • Retrofit requires OkHttpClient
  • OkHttpClient requires Authenticator
  • Authenticator requires AccessTokenRefreshDataSource

...and consequently, regardless of your module or component structure, Dagger can't create any of those instances first.

However, if your AccessTokenRefreshDataSourceImpl does not need its AuthenticationService instance within the constructor itself, you can replace it with Provider<AuthenticationService>: Dagger automatically lets you inject Provider<T> for any T in the graph, among other useful bindings. This allows Dagger to create your AccessTokenRefreshDataSource without first creating an AuthenticationService, with the promise that once the object graph is created your AccessTokenRefreshDataSource can receive the singleton AuthenticationService instance it needs. After you inject the provider, just call authenticationServiceProvider.get() to get the instance wherever you need it (presumably outside the constructor).

Of course, you can solve your problem with the same refactor anywhere else in your graph you control. AccessTokenAuthenticator is also a reasonable refactor point, assuming you've written it yourself and thus can modify its constructor.


Points discussed in the comments:

  • You can always inject a Provider<T> instead of any binding T in your graph. In addition to being valuable for breaking dependency cycles, it can also be handy if your dependency-injected class needs to instantiate an arbitrary number of that object, or if creating the object takes a lot of memory or classloading and you want to delay it until later. Of course, if the object is cheap to construct without dependency cycles and you expect to call get() exactly once, then you can skip that and directly inject T as you've done here.
    • Provider<T> is a single-method object. Calling get() on it is the same as calling a getter of type T on the Component itself. If the object is unscoped, you get a new one; if the object is scoped, you get the one that Dagger has stored in the Component.

    • Generally speaking you can just inject the Provider and call get on it directly:

      class AccessTokenRefreshDataSourceImpl @Inject constructor(
        private val authenticationServiceProvider:
          Provider<AuthenticationService>,
        private val userPrefsDataSource: UserPrefsDataSource,
      ) : AccessTokenRefreshDataSource {
      

      ... and then rather than using this.authenticationService.someMethod() directly, use this.authenticationServiceProvider.get().someMethod(). Ma3x pointed out in the comments that if you declare val authenticationService get() = authenticationServiceProvider.get() as a class field, Kotlin can abstract away the fact that there's a call to get() involved and you won't need to make any other changes to AccessTokenRefreshDataSourceImpl.

    • You will also need to change the @Provides method in your Module, but only because you're not taking full advantage of the @Inject annotation on your AccessTokenRefreshDataSourceImpl as below.

      @Singleton
      @Provides
      fun providesAccessTokenRefreshDataSource(
          userPrefsDataSource: UserPrefsDataSource,
          authenticationServiceProvider: Provider<AuthenticationService>,  // here
      ): AccessTokenRefreshDataSource = AccessTokenRefreshDataSourceImpl(
          authenticationServiceProvider /* and here */, userPrefsDataSource
      )
      
  • It is generally not necessary to use @Provides to refer to an @Inject-annotated constructor. @Provides is useful when you can't change the constructor to make it @Inject. @Inject can be less maintenance because then you don't need to copy @Provides method arguments to your constructor; Dagger will do that for you. Read more here.
    • If you do use @Inject and delete your @Provides method, you might still want to use @Binds to indicate that your AccessTokenRefreshDataSource should be bound to AccessTokenRefreshDataSourceImpl, though then you'll need to decide how to put @Binds and @Provides in the same Module. In Java 8 you can do this by making your @Provides methods static and putting them on an interface, but in Kotlin it might be easier to create a nested interface and install that using @Module(includes = ...). Read more here.
Jeff Bowman
  • 90,959
  • 16
  • 217
  • 251
  • Thanks for your answer @Jeff Bowman, I can change it, I can refactor it, so could you please provide to me a scalable or better way than this please? – Skizo-ozᴉʞS ツ Jan 08 '22 at 19:28
  • The thing is `AccessTokenRefreshDataSourceImpl ` needs the `AuthenticationService` to get the call of refresh token. So you mean I have to provides a `Provider` and inside `AccessTokenRefreshDataSourceImpl` have this in the constructor and before use it do a `.get()`? Is there any other way of doing this? – Skizo-ozᴉʞS ツ Jan 08 '22 at 19:32
  • @Skizo-ozᴉʞS Yes, inject the Provider rather than the AuthenticationService. It's fine that AccessTokenRefreshDataSourceImpl needs AuthenticationService; the Provider is just a way to briefly _delay_ the injection to avoid the cycle. There is no scalability problem with this solution, and it only involves changing a couple of lines; all other ways I can think of doing this are more intrusive or verbose. Why are you looking for a better or different way? – Jeff Bowman Jan 08 '22 at 19:42
  • 1
    @Skizo-ozᴉʞS this is a perfectly valid way, especially since you don't need these dependecies in the constructor. You can change the property in the constructor to `private val authenticationServiceProvider: Provider` and add a non constructor property `val authenticationService get() = authenticationServiceProvider.get()` into the class body and all existing code should just work without other modifications. – Ma3x Jan 08 '22 at 19:54
  • @Ma3x Yes, that's the only change needed, other than plumbing `Provider` through in the `@Provides` method. Skizo note that you are declaring `@Inject` constructors but not making use of them because you are using `@Provides` methods. It doesn't hurt anything to have both, but it's slightly confusing and not necessary. – Jeff Bowman Jan 08 '22 at 19:58
  • @JeffBowman can you explain this to me please? Having `@Inject constructor` and don't having Provides? I thought that I had to put `@Inject constructor` always even if there's nothing so I can create then the `@Provides/@Binds`, is it ok? – Skizo-ozᴉʞS ツ Jan 08 '22 at 20:23
  • @Ma3x and do I have to change something from the module? Or I can leave as it is? – Skizo-ozᴉʞS ツ Jan 08 '22 at 20:27
  • @Skizo-ozᴉʞS A `@Provides` method lets you write arbitrary code that can use any constructor, static method, constant, anything to get to the instance you want. With an `@Inject` constructor annotations like `@Singleton` directly on the class itself, you don't need `@Provides`. Then when you change constructor arguments, you wouldn't have to change the Module to match, which is what you'd have to do here. Of course, this only works for the exact class (AccessTokenRefreshDataSourceImpl): You'd need `@Binds` to make AccessTokenRefreshDataSource injections return AccessTokenRefreshDataSourceImpl. – Jeff Bowman Jan 08 '22 at 20:33
  • I do have an answer here about [Provides vs Inject](https://stackoverflow.com/q/39207845/1426891). It's in Java, but the concept still applies. – Jeff Bowman Jan 08 '22 at 20:59
  • @JeffBowman so since I'm using an object on Network module I can not add a `@Binds` there, because you mean this, right? I need to change `@Provides` to `@Binds` – Skizo-ozᴉʞS ツ Jan 09 '22 at 09:33
  • @Skizo-ozᴉʞS Sure, but you could always put a nested or adjacent interface to contain the `@Binds` and other abstract modules, and then install one from the other with `@Module(includes=NetworkBindsModule::class)`. – Jeff Bowman Jan 09 '22 at 16:28
  • I see, like I can create an interface inside the object with `@Module` and then in the object one add what you said in the last comment? – Skizo-ozᴉʞS ツ Jan 09 '22 at 19:38
  • I'm going to accept the answer since it solved my problem, but could you try to explain again why the `Provides` and when is ok to use it and when not? And why use `@Binds` in this case instead of `@Provides`? Edit your answer please I'm going to accept it anyways but I'd love to understand this – Skizo-ozᴉʞS ツ Jan 09 '22 at 19:39
  • @Skizo-ozᴉʞS Yes, that's right. See https://stackoverflow.com/a/53635909/1426891 . – Jeff Bowman Jan 09 '22 at 19:40
  • 1
    @Skizo-ozᴉʞS I've edited the comments into the answer. In the future, your follow-up questions would make good top-level questions, and I recommend asking separately so future readers can find these follow-up answers individually. – Jeff Bowman Jan 09 '22 at 20:03