Skip to content

Commit

Permalink
�Avoid scoping errors in our Dagger setup (#1597)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/414730916066338/1201415414578549/f

### Description
This PR harmonises the way we will scope dependencies in Dagger so that we avoid past mistakes mixing scope annotations resulting in unpexpected behavior.
We have also ditched the `ObjectGraph` naming and use `Scope` all over.

Recommended to see the [asana task](https://app.asana.com/0/414730916066338/1201415414578549/f) for more information about this.


With this PR we will have the following dagger scopes
* `AppScope` -> bound to the life of the application
* `ActivityScope` -> should be used for Activities
* `FragmentScope` -> should be used for Fragments
* `QuickSettingsScope` -> only use the quick tile setting for AppTP
* `ReceiverScope` -> should be used for broadcast receivers
* `VpnScope` -> only used for Vpn service


_Exceptions_
We currently do NOT support nesting dagger scopes, this means that the `AppScope` is the parent of ALL remaining scopes and, ALL remaining scopes are siblings, ie.
* children scopes have access to the dependencies in the parent scope
* sibling scopes do not have access to the dependencies in other siblins

Because we don't yet support nesting, some vpn-related activities that SHOULD use the `ActivityScope` are actually using the `VpnScope`, all because they need some dependencies that are contributed to the `VpnScope`.
To track those for future fixes, we have added an annotation dubbed `@WrongScope` that provide information as to what should be the appropriate scope.
In the future, when nesting is supported, we just need to find usages of that annotation and fix the scope.

Incidentally, and because we are heavy users of multi-bindings and unfortunatelly those require the `JvmSuppressWildcards` annotations, I have added a sugar over `Set` and `Map` multibindings to avoid mistakes.

### Steps to test this PR
This PR should not change any behavior, just fixes the usage of dagger annotations. If anything breaks, should now break at build time (this is why the `WrongScope` annotation exists now)

* Just perform regular smoke tests for the app and AppTP
  • Loading branch information
aitorvs authored Dec 10, 2021
1 parent f26a7d0 commit ae6bd28
Show file tree
Hide file tree
Showing 307 changed files with 1,666 additions and 1,502 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@
package com.duckduckgo.app.di

import com.duckduckgo.app.job.ConfigurationDownloader
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesTo
import dagger.Module
import dagger.Provides
import io.reactivex.Completable

@Module
@ContributesTo(
scope = AppObjectGraph::class,
scope = AppScope::class,
replaces = [AppConfigurationDownloaderModule::class]
)
class StubAppConfigurationDownloadModule {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,21 @@ package com.duckduckgo.app.di

import com.duckduckgo.app.CoroutineTestRule
import com.duckduckgo.app.global.DispatcherProvider
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesTo
import dagger.Module
import dagger.Provides
import javax.inject.Singleton
import dagger.SingleInstanceIn

@Module
@ContributesTo(
scope = AppObjectGraph::class,
scope = AppScope::class,
replaces = [CoroutinesModule::class]
)
class StubCoroutinesModule {

@Provides
@Singleton
@SingleInstanceIn(AppScope::class)
fun providesDispatcherProvider(): DispatcherProvider {
return CoroutineTestRule().testDispatcherProvider
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,27 @@ import android.content.Context
import android.webkit.WebViewDatabase
import androidx.room.Room
import com.duckduckgo.app.global.db.AppDatabase
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesTo
import dagger.Module
import dagger.Provides
import javax.inject.Singleton
import dagger.SingleInstanceIn

@Module(includes = [DaoModule::class])
@ContributesTo(
scope = AppObjectGraph::class,
scope = AppScope::class,
replaces = [DatabaseModule::class]
)
class StubDatabaseModule {

@Provides
@Singleton
@SingleInstanceIn(AppScope::class)
fun provideWebviewDatabase(context: Context): WebViewDatabase {
return WebViewDatabase.getInstance(context)
}

@Provides
@Singleton
@SingleInstanceIn(AppScope::class)
fun provideDatabase(context: Context): AppDatabase {
return Room.inMemoryDatabaseBuilder(context, AppDatabase::class.java)
.allowMainThreadQueries()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,20 @@ package com.duckduckgo.app.di
import android.app.job.JobInfo
import android.app.job.JobScheduler
import android.app.job.JobWorkItem
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesTo
import dagger.Module
import dagger.Provides
import javax.inject.Singleton
import dagger.SingleInstanceIn

@Module
@ContributesTo(
scope = AppObjectGraph::class,
scope = AppScope::class,
replaces = [JobsModule::class]
)
class StubJobSchedulerModule {

@Singleton
@SingleInstanceIn(AppScope::class)
@Provides
fun providesJobScheduler(): JobScheduler {
return object : JobScheduler() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,20 @@ import com.duckduckgo.app.statistics.api.StatisticsService
import com.duckduckgo.app.statistics.api.StatisticsUpdater
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.store.StatisticsDataStore
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.DaggerSet
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesTo
import dagger.Module
import dagger.Provides
import dagger.multibindings.IntoSet
import io.reactivex.Completable
import kotlinx.coroutines.CoroutineScope
import retrofit2.Retrofit
import javax.inject.Singleton
import dagger.SingleInstanceIn

@Module
@ContributesTo(
scope = AppObjectGraph::class,
scope = AppScope::class,
replaces = [StatisticsModule::class]
)
class StubStatisticsModule {
Expand Down Expand Up @@ -90,12 +91,12 @@ class StubStatisticsModule {

@Provides
@IntoSet
@Singleton
@SingleInstanceIn(AppScope::class)
fun atbInitializer(
@AppCoroutineScope appCoroutineScope: CoroutineScope,
statisticsDataStore: StatisticsDataStore,
statisticsUpdater: StatisticsUpdater,
listeners: Set<@JvmSuppressWildcards AtbInitializerListener>
listeners: DaggerSet<AtbInitializerListener>
): LifecycleObserver {
return AtbInitializer(appCoroutineScope, statisticsDataStore, statisticsUpdater, listeners)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ package com.duckduckgo.app.dev.settings
import android.content.Context
import com.duckduckgo.app.browser.R
import com.duckduckgo.app.settings.extension.InternalFeaturePlugin
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesMultibinding
import javax.inject.Inject

@ContributesMultibinding(AppObjectGraph::class)
@ContributesMultibinding(AppScope::class)
class DevSettingsFeature @Inject constructor(private val context: Context) : InternalFeaturePlugin {
override fun internalFeatureTitle(): String {
return context.getString(R.string.devSettingsTitle)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import com.duckduckgo.app.fire.FireAnimationLoader
import com.duckduckgo.app.global.plugins.view_model.ViewModelFactoryPlugin
import com.duckduckgo.app.statistics.VariantManager
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesMultibinding
import kotlinx.coroutines.channels.BufferOverflow
import kotlinx.coroutines.channels.Channel
Expand Down Expand Up @@ -96,7 +96,7 @@ class DevSettingsViewModel @Inject constructor(
}
}

@ContributesMultibinding(AppObjectGraph::class)
@ContributesMultibinding(AppScope::class)
class SettingsViewModelFactory @Inject constructor(
private val devSettingsDataStore: Provider<DevSettingsDataStore>,
private val defaultWebBrowserCapability: Provider<DefaultBrowserDetector>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ package com.duckduckgo.app.dev.settings.api

import com.duckduckgo.app.dev.settings.db.DevSettingsDataStore
import com.duckduckgo.app.global.api.ApiInterceptorPlugin
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesMultibinding
import okhttp3.Interceptor
import okhttp3.Response
import javax.inject.Inject

@ContributesMultibinding(
scope = AppObjectGraph::class,
scope = AppScope::class,
boundType = ApiInterceptorPlugin::class
)
class ApiDevTdsInterceptor @Inject constructor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ package com.duckduckgo.app.dev.settings.db
import android.content.Context
import android.content.SharedPreferences
import androidx.core.content.edit
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesBinding
import javax.inject.Inject

interface DevSettingsDataStore {
var nextTdsEnabled: Boolean
}

@ContributesBinding(AppObjectGraph::class)
@ContributesBinding(AppScope::class)
class DevSettingsSharedPreferences @Inject constructor(private val context: Context) : DevSettingsDataStore {
override var nextTdsEnabled: Boolean
get() = preferences.getBoolean(KEY_NEXT_TDS_ENABLED, false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import androidx.lifecycle.LifecycleObserver
import androidx.lifecycle.OnLifecycleEvent
import com.duckduckgo.app.browser.BuildConfig
import com.duckduckgo.app.trackerdetection.api.TrackerDataDownloader
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesMultibinding
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.schedulers.Schedulers
Expand All @@ -52,7 +52,7 @@ class TrackerDataDevReceiver(
}
}

@ContributesMultibinding(AppObjectGraph::class)
@ContributesMultibinding(AppScope::class)
class TrackerDataDevReceiverRegister @Inject constructor(
private val context: Context,
private val trackderDataDownloader: TrackerDataDownloader
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,38 +17,38 @@
package com.duckduckgo.app.di.component

import com.duckduckgo.app.dev.settings.DevSettingsActivity
import com.duckduckgo.app.di.ActivityScoped
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope

import com.duckduckgo.di.scopes.ActivityObjectGraph
import com.duckduckgo.di.scopes.ActivityScope
import com.squareup.anvil.annotations.ContributesTo
import com.squareup.anvil.annotations.MergeSubcomponent
import dagger.Binds
import dagger.Module
import dagger.SingleInstanceIn
import dagger.Subcomponent
import dagger.android.AndroidInjector
import dagger.multibindings.ClassKey
import dagger.multibindings.IntoMap

@ActivityScoped
@SingleInstanceIn(ActivityScope::class)
@MergeSubcomponent(
scope = ActivityObjectGraph::class
scope = ActivityScope::class
)
interface DevSettingsActivityComponent : AndroidInjector<DevSettingsActivity> {
@Subcomponent.Factory
interface Factory : AndroidInjector.Factory<DevSettingsActivity>
}

@ContributesTo(AppObjectGraph::class)
@ContributesTo(AppScope::class)
interface DevSettingsActivityComponentProvider {
fun provideDevSettingsActivityComponentFactory(): DevSettingsActivityComponent.Factory
}

@Module
@ContributesTo(AppObjectGraph::class)
@ContributesTo(AppScope::class)
abstract class DevSettingsActivityBindingModule {
@Binds
@IntoMap
@ClassKey(DevSettingsActivity::class)
abstract fun bindDevSettingsActivityComponentFactory(factory: DevSettingsActivityComponent.Factory): AndroidInjector.Factory<*>
abstract fun DevSettingsActivityComponent.Factory.bind(): AndroidInjector.Factory<*>
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,39 +16,39 @@

package com.duckduckgo.app.di.component

import com.duckduckgo.app.di.ActivityScoped
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope

import com.duckduckgo.di.scopes.ActivityObjectGraph
import com.duckduckgo.di.scopes.ActivityScope
import com.duckduckgo.vpn.internal.feature.rules.ExceptionRulesDebugActivity
import com.squareup.anvil.annotations.ContributesTo
import com.squareup.anvil.annotations.MergeSubcomponent
import dagger.Binds
import dagger.Module
import dagger.SingleInstanceIn
import dagger.Subcomponent
import dagger.android.AndroidInjector
import dagger.multibindings.ClassKey
import dagger.multibindings.IntoMap

@ActivityScoped
@SingleInstanceIn(ActivityScope::class)
@MergeSubcomponent(
scope = ActivityObjectGraph::class
scope = ActivityScope::class
)
interface ExceptionRulesDebugActivityComponent : AndroidInjector<ExceptionRulesDebugActivity> {
@Subcomponent.Factory
interface Factory : AndroidInjector.Factory<ExceptionRulesDebugActivity>
}

@ContributesTo(AppObjectGraph::class)
@ContributesTo(AppScope::class)
interface ExceptionRulesDebugActivityComponentProvider {
fun provideExceptionRulesDebugActivityComponentFactory(): ExceptionRulesDebugActivityComponent.Factory
}

@Module
@ContributesTo(AppObjectGraph::class)
@ContributesTo(AppScope::class)
abstract class ExceptionRulesDebugActivityBindingModule {
@Binds
@IntoMap
@ClassKey(ExceptionRulesDebugActivity::class)
abstract fun bindExceptionRulesDebugActivityComponentFactory(factory: ExceptionRulesDebugActivityComponent.Factory): AndroidInjector.Factory<*>
abstract fun ExceptionRulesDebugActivityComponent.Factory.bind(): AndroidInjector.Factory<*>
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,39 +16,40 @@

package com.duckduckgo.app.di.component

import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.VpnObjectGraph
import com.duckduckgo.mobile.android.vpn.di.VpnScope

import com.duckduckgo.di.scopes.ActivityScope
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.di.scopes.VpnScope
import com.duckduckgo.vpn.internal.feature.VpnInternalSettingsActivity
import com.squareup.anvil.annotations.ContributesTo
import com.squareup.anvil.annotations.MergeSubcomponent
import dagger.Binds
import dagger.Module
import dagger.Subcomponent
import dagger.*
import dagger.android.AndroidInjector
import dagger.multibindings.ClassKey
import dagger.multibindings.IntoMap

@VpnScope
@WrongScope(
comment = "To use the right scope we first need to enable dagger component nesting",
correctScope = ActivityScope::class,
)
@SingleInstanceIn(VpnScope::class)
@MergeSubcomponent(
scope = VpnObjectGraph::class
scope = VpnScope::class
)
interface VpnInternalSettingsActivityComponent : AndroidInjector<VpnInternalSettingsActivity> {
@Subcomponent.Factory
interface Factory : AndroidInjector.Factory<VpnInternalSettingsActivity>
}

@ContributesTo(AppObjectGraph::class)
@ContributesTo(AppScope::class)
interface VpnInternalSettingsActivityComponentProvider {
fun provideVpnInternalSettingsActivityComponentFactory(): VpnInternalSettingsActivityComponent.Factory
}

@Module
@ContributesTo(AppObjectGraph::class)
@ContributesTo(AppScope::class)
abstract class VpnInternalSettingsActivityBindingModule {
@Binds
@IntoMap
@ClassKey(VpnInternalSettingsActivity::class)
abstract fun bindVpnInternalSettingsActivityComponentFactory(factory: VpnInternalSettingsActivityComponent.Factory): AndroidInjector.Factory<*>
abstract fun VpnInternalSettingsActivityComponent.Factory.bind(): AndroidInjector.Factory<*>
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ package com.duckduckgo.app.feature

import android.content.Context
import com.duckduckgo.app.settings.extension.InternalFeaturePlugin
import com.duckduckgo.di.scopes.AppObjectGraph
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.mobile.android.themepreview.ui.AppComponentsActivity
import com.squareup.anvil.annotations.ContributesMultibinding
import javax.inject.Inject

@ContributesMultibinding(AppObjectGraph::class)
@ContributesMultibinding(AppScope::class)
class ThemesPreviewInternalFeature @Inject constructor() : InternalFeaturePlugin {
override fun internalFeatureTitle(): String {
return "App Components Design Preview"
Expand Down
Loading

0 comments on commit ae6bd28

Please sign in to comment.