Address review comments from Marcos

app/src/main/java/com/duckduckgo/app/brokensite/api/BrokenSiteSender.kt

@@ -50,7 +50,7 @@ class BrokenSiteSubmitter(
 ) : BrokenSiteSender {
 
     override fun submitBrokenSiteFeedback(brokenSite: BrokenSite) {
-        val isGpcEnabled = (featureToggle.isFeatureEnabled(PrivacyFeatureName.GpcFeatureName) == true && gpc.isEnabled()).toString()
+        val isGpcEnabled = (featureToggle.isFeatureEnabled(PrivacyFeatureName.GpcFeatureName) && gpc.isEnabled()).toString()
         val absoluteUrl = Uri.parse(brokenSite.siteUrl).absoluteString
 
         appCoroutineScope.launch(dispatcherProvider.io()) {

app/src/main/java/com/duckduckgo/app/globalprivacycontrol/ui/GlobalPrivacyControlViewModel.kt

@@ -53,7 +53,7 @@ class GlobalPrivacyControlViewModel(
     init {
         _viewState.value = ViewState(
             globalPrivacyControlEnabled = gpc.isEnabled(),
-            globalPrivacyControlFeatureEnabled = featureToggle.isFeatureEnabled(PrivacyFeatureName.GpcFeatureName, true) == true
+            globalPrivacyControlFeatureEnabled = featureToggle.isFeatureEnabled(PrivacyFeatureName.GpcFeatureName, true)
         )
         pixel.fire(SETTINGS_DO_NOT_SELL_SHOWN)
     }

app/src/main/java/com/duckduckgo/app/httpsupgrade/HttpsUpgrader.kt

@@ -78,7 +78,7 @@ class HttpsUpgraderImpl @Inject constructor(
     override fun shouldUpgrade(uri: Uri): Boolean {
         val host = uri.host ?: return false
 
-        if (toggle.isFeatureEnabled(PrivacyFeatureName.HttpsFeatureName) != true) {
+        if (!toggle.isFeatureEnabled(PrivacyFeatureName.HttpsFeatureName)) {
             Timber.d("https is disabled in the remote config and so $host is not upgradable")
             return false
         }

feature-toggles/feature-toggles-impl/src/test/java/com/duckduckgo/feature/toggles/impl/RealFeatureToggleImplTest.kt

@@ -35,9 +35,8 @@ class RealFeatureToggleImplTest {
     }
 
     @Test(expected = IllegalArgumentException::class)
-    fun whenFeatureNameCannotBeHandledByAnyPluginThenReturnNull() {
-        val result = testee.isFeatureEnabled(NullFeatureName(), false)
-        assertNull(result)
+    fun whenFeatureNameCannotBeHandledByAnyPluginThenThrowException() {
+        testee.isFeatureEnabled(NullFeatureName(), false)
     }
 
     class FakeTruePlugin : FeatureTogglesPlugin {

privacy-config/privacy-config-impl/src/main/java/com/duckduckgo/privacy/config/impl/features/amplinks/RealAmpLinks.kt

@@ -50,7 +50,7 @@ class RealAmpLinks @Inject constructor(
     }
 
     override fun extractCanonicalFromAmpLink(url: String): AmpLinkType? {
-        if (featureToggle.isFeatureEnabled(PrivacyFeatureName.AmpLinksFeatureName) == false) return null
+        if (!featureToggle.isFeatureEnabled(PrivacyFeatureName.AmpLinksFeatureName)) return null
         if (url == lastExtractedUrl) return null
 
         val extractedUrl = extractCanonical(url)

privacy-config/privacy-config-impl/src/main/java/com/duckduckgo/privacy/config/impl/features/gpc/RealGpc.kt

@@ -80,7 +80,7 @@ class RealGpc @Inject constructor(
     }
 
     private fun isFeatureEnabled(): Boolean {
-        return featureToggle.isFeatureEnabled(PrivacyFeatureName.GpcFeatureName) == true
+        return featureToggle.isFeatureEnabled(PrivacyFeatureName.GpcFeatureName)
     }
 
     private fun containsGpcHeader(headers: Map<String, String>): Boolean {

privacy-config/privacy-config-impl/src/main/java/com/duckduckgo/privacy/config/impl/features/trackerallowlist/RealTrackerAllowlist.kt

@@ -40,7 +40,7 @@ class RealTrackerAllowlist @Inject constructor(
         documentURL: String,
         url: String
     ): Boolean {
-        return if (featureToggle.isFeatureEnabled(PrivacyFeatureName.TrackerAllowlistFeatureName, true) == true) {
+        return if (featureToggle.isFeatureEnabled(PrivacyFeatureName.TrackerAllowlistFeatureName, true)) {
             trackerAllowlistRepository.exceptions
                 .filter { UriString.sameOrSubdomain(url, it.domain) }
                 .map { matches(url, documentURL, it) }

privacy-config/privacy-config-impl/src/main/java/com/duckduckgo/privacy/config/impl/features/trackingparameters/RealTrackingParameters.kt

@@ -59,7 +59,7 @@ class RealTrackingParameters @Inject constructor(
     }
 
     override fun cleanTrackingParameters(url: String): String? {
-        if (featureToggle.isFeatureEnabled(PrivacyFeatureName.TrackingParametersFeatureName) == false) return null
+        if (!featureToggle.isFeatureEnabled(PrivacyFeatureName.TrackingParametersFeatureName)) return null
         if (isAnException(url)) return null
 
         val trackingParameters = trackingParametersRepository.parameters

privacy-config/privacy-config-store/src/main/java/com/duckduckgo/privacy/config/store/PrivacyFeatureTogglesRepository.kt

@@ -16,18 +16,8 @@
 
 package com.duckduckgo.privacy.config.store
 
-import com.duckduckgo.privacy.config.api.PrivacyFeatureName
-
 interface PrivacyFeatureTogglesRepository : PrivacyFeatureTogglesDataStore
 
 class RealPrivacyFeatureTogglesRepository(
-    private val privacyFeatureTogglesDataStore: PrivacyFeatureTogglesDataStore
-) : PrivacyFeatureTogglesRepository, PrivacyFeatureTogglesDataStore by privacyFeatureTogglesDataStore {
-
-    override fun get(
-        featureName: PrivacyFeatureName,
-        defaultValue: Boolean
-    ): Boolean {
-        return privacyFeatureTogglesDataStore.get(featureName, defaultValue)
-    }
-}
+    privacyFeatureTogglesDataStore: PrivacyFeatureTogglesDataStore
+) : PrivacyFeatureTogglesRepository, PrivacyFeatureTogglesDataStore by privacyFeatureTogglesDataStore

remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/di/AndroidAppAttrMatcher.kt

@@ -18,7 +18,7 @@ package com.duckduckgo.remote.messaging.impl.di
 
 import javax.inject.Qualifier
 
-/** Identifies a coroutine scope type that is scope to the app lifecycle */
+/** Marks the Android matcher implementation for remote-messaging */
 @Qualifier
 @Retention(AnnotationRetention.RUNTIME)
 annotation class AndroidAppAttrMatcher

remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/di/DeviceAttrMatcher.kt

@@ -18,7 +18,7 @@ package com.duckduckgo.remote.messaging.impl.di
 
 import javax.inject.Qualifier
 
-/** Identifies a coroutine scope type that is scope to the app lifecycle */
+/** Marks the device matcher implementation for remote-messaging */
 @Qualifier
 @Retention(AnnotationRetention.RUNTIME)
 annotation class DeviceAttrMatcher

remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/di/UserAttrMatcher.kt

@@ -18,7 +18,7 @@ package com.duckduckgo.remote.messaging.impl.di
 
 import javax.inject.Qualifier
 
-/** Identifies a coroutine scope type that is scope to the app lifecycle */
+/** Marks the Android matcher implementation for remote-messaging */
 @Qualifier
 @Retention(AnnotationRetention.RUNTIME)
 annotation class UserAttrMatcher

vpn-internal/src/main/java/com/duckduckgo/vpn/internal/feature/VpnInternalSettingsActivity.kt

@@ -22,11 +22,12 @@ import android.os.Bundle
 import android.widget.CompoundButton
 import androidx.lifecycle.lifecycleScope
 import com.duckduckgo.app.global.DuckDuckGoActivity
+import com.duckduckgo.feature.toggles.api.FeatureToggle
 import com.duckduckgo.mobile.android.ui.viewbinding.viewBinding
 import com.duckduckgo.mobile.android.vpn.feature.AppTpFeatureName
 import com.duckduckgo.mobile.android.vpn.health.AppHealthMonitor
 import com.duckduckgo.mobile.android.vpn.health.BadHealthMitigationFeature
-import com.duckduckgo.mobile.android.vpn.store.RealVpnFeatureToggleStore
+import com.duckduckgo.mobile.android.vpn.store.AppTpFeatureToggleRepository
 import com.duckduckgo.mobile.android.vpn.store.VpnFeatureToggles
 import com.duckduckgo.vpn.internal.databinding.ActivityVpnInternalSettingsBinding
 import com.duckduckgo.vpn.internal.feature.bugreport.VpnBugReporter
@@ -50,7 +51,11 @@ class VpnInternalSettingsActivity : DuckDuckGoActivity() {
     @Inject
     lateinit var badHealthMitigationFeature: BadHealthMitigationFeature
 
-    private val vpnFeatureToggleStore = RealVpnFeatureToggleStore(this)
+    @Inject
+    lateinit var featureToggle: FeatureToggle
+
+    @Inject
+    lateinit var ppTpFeatureToggleRepository: AppTpFeatureToggleRepository
 
     private val binding: ActivityVpnInternalSettingsBinding by viewBinding()
     private var transparencyModeDebugReceiver: TransparencyModeDebugReceiver? = null
@@ -174,20 +179,19 @@ class VpnInternalSettingsActivity : DuckDuckGoActivity() {
     }
 
     private fun setupConfigSection() {
-        binding.ipv6SupportToggle.isChecked = vpnFeatureToggleStore.get(AppTpFeatureName.Ipv6Support, false) ?: false
+        binding.ipv6SupportToggle.isChecked = featureToggle.isFeatureEnabled(AppTpFeatureName.Ipv6Support, false)
         binding.ipv6SupportToggle.setOnCheckedChangeListener { _, isChecked ->
-            vpnFeatureToggleStore.insert(VpnFeatureToggles(AppTpFeatureName.Ipv6Support, isChecked))
+            ppTpFeatureToggleRepository.insert(VpnFeatureToggles(AppTpFeatureName.Ipv6Support, isChecked))
         }
 
-        binding.privateDnsToggle.isChecked = vpnFeatureToggleStore.get(AppTpFeatureName.PrivateDnsSupport, false) ?: false
+        binding.privateDnsToggle.isChecked = featureToggle.isFeatureEnabled(AppTpFeatureName.PrivateDnsSupport, false)
         binding.privateDnsToggle.setOnCheckedChangeListener { _, isChecked ->
-            vpnFeatureToggleStore.insert(VpnFeatureToggles(AppTpFeatureName.PrivateDnsSupport, isChecked))
+            ppTpFeatureToggleRepository.insert(VpnFeatureToggles(AppTpFeatureName.PrivateDnsSupport, isChecked))
         }
 
-        binding.vpnUnderlyingNetworksToggle.isChecked = vpnFeatureToggleStore.get(AppTpFeatureName.NetworkSwitchingHandling, false)
-            ?: false
+        binding.vpnUnderlyingNetworksToggle.isChecked = featureToggle.isFeatureEnabled(AppTpFeatureName.NetworkSwitchingHandling, false)
         binding.vpnUnderlyingNetworksToggle.setOnCheckedChangeListener { _, isChecked ->
-            vpnFeatureToggleStore.insert(VpnFeatureToggles(AppTpFeatureName.NetworkSwitchingHandling, isChecked))
+            ppTpFeatureToggleRepository.insert(VpnFeatureToggles(AppTpFeatureName.NetworkSwitchingHandling, isChecked))
         }
     }
 

vpn-store/build.gradle

@@ -39,6 +39,7 @@ dependencies {
 
     implementation project(path: ':common')
     implementation project(path: ':vpn-api')
+    implementation project(path: ':appbuildconfig-api')
 
     implementation Kotlin.stdlib.jdk7
     implementation AndroidX.core.ktx
@@ -62,6 +63,7 @@ dependencies {
 
     testImplementation "junit:junit:_"
     testImplementation project(path: ':common-test')
+    testImplementation "org.mockito.kotlin:mockito-kotlin:_"
 
     androidTestImplementation AndroidX.test.runner
     androidTestImplementation AndroidX.test.rules

vpn/src/main/java/com/duckduckgo/mobile/android/vpn/feature/AppTpFeatureToggleRepository.kt → vpn-store/src/main/java/com/duckduckgo/mobile/android/vpn/store/AppTpFeatureToggleRepository.kt

@@ -14,18 +14,12 @@
  * limitations under the License.
  */
 
-package com.duckduckgo.mobile.android.vpn.feature
+package com.duckduckgo.mobile.android.vpn.store
+
+import com.duckduckgo.mobile.android.vpn.feature.AppTpFeatureName
 
-import android.content.Context
 import com.duckduckgo.appbuildconfig.api.AppBuildConfig
 import com.duckduckgo.appbuildconfig.api.BuildFlavor
-import com.duckduckgo.di.scopes.AppScope
-import com.duckduckgo.mobile.android.vpn.store.RealVpnFeatureToggleStore
-import com.duckduckgo.mobile.android.vpn.store.VpnFeatureToggleStore
-import com.squareup.anvil.annotations.ContributesTo
-import dagger.Module
-import dagger.Provides
-import dagger.SingleInstanceIn
 
 // marker interface to use delegate pattern
 interface AppTpFeatureToggleRepository : VpnFeatureToggleStore
@@ -42,14 +36,3 @@ class RealAppTpFeatureToggleRepository constructor(
         return (appBuildConfig.flavor == BuildFlavor.INTERNAL) && delegateValue
     }
 }
-
-@ContributesTo(AppScope::class)
-@Module
-class AppTpFeatureToggleRepositoryModule {
-    @SingleInstanceIn(AppScope::class)
-    @Provides
-    fun provideAppTpFeatureToggleRepository(context: Context, appBuildConfig: AppBuildConfig): AppTpFeatureToggleRepository {
-        val store = RealVpnFeatureToggleStore(context)
-        return RealAppTpFeatureToggleRepository(store, appBuildConfig)
-    }
-}

vpn/src/test/java/com/duckduckgo/mobile/android/vpn/feature/RealAppTpFeatureToggleRepositoryTest.kt → vpn-store/src/test/java/com/duckduckgo/mobile/android/vpn/store/RealAppTpFeatureToggleRepositoryTest.kt

@@ -14,12 +14,11 @@
  * limitations under the License.
  */
 
-package com.duckduckgo.mobile.android.vpn.feature
+package com.duckduckgo.mobile.android.vpn.store
 
 import com.duckduckgo.appbuildconfig.api.AppBuildConfig
 import com.duckduckgo.appbuildconfig.api.BuildFlavor
-import com.duckduckgo.mobile.android.vpn.store.VpnFeatureToggleStore
-import com.duckduckgo.mobile.android.vpn.store.VpnFeatureToggles
+import com.duckduckgo.mobile.android.vpn.feature.AppTpFeatureName
 import org.junit.Assert.*
 import org.junit.Before
 import org.junit.Test

vpn/src/main/java/com/duckduckgo/mobile/android/vpn/feature/AppTpFeatureToggleRepositoryModule.kt

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2022 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.mobile.android.vpn.feature
+
+import android.content.Context
+import com.duckduckgo.appbuildconfig.api.AppBuildConfig
+import com.duckduckgo.di.scopes.AppScope
+import com.duckduckgo.mobile.android.vpn.store.AppTpFeatureToggleRepository
+import com.duckduckgo.mobile.android.vpn.store.RealAppTpFeatureToggleRepository
+import com.duckduckgo.mobile.android.vpn.store.RealVpnFeatureToggleStore
+import com.squareup.anvil.annotations.ContributesTo
+import dagger.Module
+import dagger.Provides
+import dagger.SingleInstanceIn
+
+@ContributesTo(AppScope::class)
+@Module
+class AppTpFeatureToggleRepositoryModule {
+    @SingleInstanceIn(AppScope::class)
+    @Provides
+    fun provideAppTpFeatureToggleRepository(context: Context, appBuildConfig: AppBuildConfig): AppTpFeatureToggleRepository {
+        val store = RealVpnFeatureToggleStore(context)
+        return RealAppTpFeatureToggleRepository(store, appBuildConfig)
+    }
+}

vpn/src/main/java/com/duckduckgo/mobile/android/vpn/feature/AppTpFeatureTogglesPlugin.kt

@@ -19,6 +19,7 @@ package com.duckduckgo.mobile.android.vpn.feature
 import com.duckduckgo.di.scopes.AppScope
 import com.duckduckgo.feature.toggles.api.FeatureName
 import com.duckduckgo.feature.toggles.api.FeatureTogglesPlugin
+import com.duckduckgo.mobile.android.vpn.store.AppTpFeatureToggleRepository
 import com.squareup.anvil.annotations.ContributesMultibinding
 import javax.inject.Inject
 

vpn/src/test/java/com/duckduckgo/mobile/android/vpn/feature/AppTpFeatureTogglesPluginTest.kt

@@ -17,6 +17,7 @@
 package com.duckduckgo.mobile.android.vpn.feature
 
 import com.duckduckgo.feature.toggles.api.FeatureName
+import com.duckduckgo.mobile.android.vpn.store.AppTpFeatureToggleRepository
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
 import org.junit.Before