Don't expose domains

Author: CrisBarreiro

ad-blocking/ad-blocking-impl/src/main/java/com/duckduckgo/adblocking/impl/di/AdBlockingExtensionModule.kt

@@ -17,12 +17,10 @@
 package com.duckduckgo.adblocking.impl.di
 
 import com.duckduckgo.adblocking.impl.remoteconfig.AdBlockingExtensionSettings
-import com.duckduckgo.adblocking.impl.remoteconfig.DomainJsonAdapter
 import com.duckduckgo.adblocking.impl.remoteconfig.ScriptletsSettings
 import com.duckduckgo.adblocking.impl.store.AD_BLOCKING_EXTENSION_MIGRATIONS
 import com.duckduckgo.adblocking.impl.store.AdBlockingExtensionDao
 import com.duckduckgo.adblocking.impl.store.AdBlockingExtensionDatabase
-import com.duckduckgo.app.browser.Domain
 import com.duckduckgo.data.store.api.DatabaseProvider
 import com.duckduckgo.data.store.api.RoomDatabaseConfig
 import com.duckduckgo.di.scopes.AppScope
@@ -59,7 +57,6 @@ object AdBlockingExtensionModule {
     @Provides
     fun provideAdBlockingExtensionSettingsAdapter(): JsonAdapter<AdBlockingExtensionSettings> =
         Moshi.Builder()
-            .add(Domain::class.java, DomainJsonAdapter().nullSafe())
             .add(KotlinJsonAdapterFactory())
             .build()
             .adapter(AdBlockingExtensionSettings::class.java)

ad-blocking/ad-blocking-impl/src/main/java/com/duckduckgo/adblocking/impl/remoteconfig/AdBlockingExtensionConfigProvider.kt

@@ -16,14 +16,11 @@
 
 package com.duckduckgo.adblocking.impl.remoteconfig
 
-import com.duckduckgo.app.browser.Domain
 import com.duckduckgo.di.scopes.AppScope
 import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin
 import com.squareup.anvil.annotations.ContributesBinding
 import com.squareup.anvil.annotations.ContributesMultibinding
 import com.squareup.moshi.JsonAdapter
-import com.squareup.moshi.JsonReader
-import com.squareup.moshi.JsonWriter
 import dagger.SingleInstanceIn
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
@@ -36,7 +33,6 @@ import javax.inject.Inject
 data class AdBlockingExtensionSettings(
     val version: String,
     val scriptlets: Map<String, ScriptletEntry>,
-    val domains: List<Domain> = emptyList(),
 )
 
 data class ScriptletEntry(
@@ -49,13 +45,8 @@ data class ScriptletsSettings(
     val scriptlets: Map<String, ScriptletEntry>,
 )
 
-data class DomainsSettings(
-    val domains: List<Domain>,
-)
-
 interface AdBlockingExtensionConfigProvider {
     val scriptletsSettings: StateFlow<ScriptletsSettings?>
-    val domainsSettings: StateFlow<DomainsSettings?>
 }
 
 @SingleInstanceIn(AppScope::class)
@@ -67,14 +58,12 @@ class RealAdBlockingExtensionConfigProvider @Inject constructor(
 ) : AdBlockingExtensionConfigProvider, PrivacyConfigCallbackPlugin {
 
     private val scriptletsFlow = MutableStateFlow<ScriptletsSettings?>(null)
-    private val domainsFlow = MutableStateFlow<DomainsSettings?>(null)
 
     init {
         refresh()
     }
 
     override val scriptletsSettings: StateFlow<ScriptletsSettings?> = scriptletsFlow.asStateFlow()
-    override val domainsSettings: StateFlow<DomainsSettings?> = domainsFlow.asStateFlow()
 
     override fun onPrivacyConfigDownloaded() {
         logcat { "onPrivacyConfigDownloaded" }
@@ -84,7 +73,6 @@ class RealAdBlockingExtensionConfigProvider @Inject constructor(
     private fun refresh() {
         val raw = parseSettings()
         scriptletsFlow.value = raw?.let { ScriptletsSettings(it.version, it.scriptlets) }
-        domainsFlow.value = raw?.let { DomainsSettings(it.domains) }
     }
 
     private fun parseSettings(): AdBlockingExtensionSettings? {
@@ -95,10 +83,3 @@ class RealAdBlockingExtensionConfigProvider @Inject constructor(
             ?.takeIf { it.version.isNotEmpty() }
     }
 }
-
-internal class DomainJsonAdapter : JsonAdapter<Domain>() {
-    override fun fromJson(reader: JsonReader): Domain = Domain(reader.nextString())
-    override fun toJson(writer: JsonWriter, value: Domain?) {
-        writer.value(value?.value)
-    }
-}

ad-blocking/ad-blocking-impl/src/test/java/com/duckduckgo/adblocking/impl/AdBlockingExtensionConfigProviderTest.kt

@@ -21,10 +21,8 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
 import app.cash.turbine.test
 import com.duckduckgo.adblocking.impl.remoteconfig.AdBlockingExtensionFeature
 import com.duckduckgo.adblocking.impl.remoteconfig.AdBlockingExtensionSettings
-import com.duckduckgo.adblocking.impl.remoteconfig.DomainJsonAdapter
 import com.duckduckgo.adblocking.impl.remoteconfig.RealAdBlockingExtensionConfigProvider
 import com.duckduckgo.adblocking.impl.remoteconfig.ScriptletEntry
-import com.duckduckgo.app.browser.Domain
 import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
 import com.duckduckgo.feature.toggles.api.Toggle
 import com.squareup.moshi.Moshi
@@ -42,7 +40,6 @@ class AdBlockingExtensionConfigProviderTest {
 
     private val feature = FakeFeatureToggleFactory.create(AdBlockingExtensionFeature::class.java)
     private val settingsAdapter = Moshi.Builder()
-        .add(Domain::class.java, DomainJsonAdapter().nullSafe())
         .add(KotlinJsonAdapterFactory())
         .build()
         .adapter(AdBlockingExtensionSettings::class.java)
@@ -54,8 +51,7 @@ class AdBlockingExtensionConfigProviderTest {
             "scriptlets": {
                 "scriptlets/isolated/ublock-filters.js": { "url": "https://cdn.example/isolated.js", "signature": "iso-sig" },
                 "scriptlets/main/ublock-filters.js": { "url": "https://cdn.example/main.js", "signature": "main-sig" }
-            },
-            "domains": ["youtube.com", "m.youtube.com"]
+            }
         }
     """.trimIndent()
 
@@ -65,7 +61,6 @@ class AdBlockingExtensionConfigProviderTest {
         provider.onPrivacyConfigDownloaded()
 
         assertNull(provider.scriptletsSettings.value)
-        assertNull(provider.domainsSettings.value)
     }
 
     @Test
@@ -74,29 +69,26 @@ class AdBlockingExtensionConfigProviderTest {
         provider.onPrivacyConfigDownloaded()
 
         assertNull(provider.scriptletsSettings.value)
-        assertNull(provider.domainsSettings.value)
     }
 
     @Test
     fun whenVersionIsMissingThenSettingsAreNull() = runTest {
         feature.self().setRawStoredState(
-            Toggle.State(remoteEnableState = true, settings = """{"scriptlets": {}, "domains": []}"""),
+            Toggle.State(remoteEnableState = true, settings = """{"scriptlets": {}}"""),
         )
         provider.onPrivacyConfigDownloaded()
 
         assertNull(provider.scriptletsSettings.value)
-        assertNull(provider.domainsSettings.value)
     }
 
     @Test
     fun whenScriptletsIsMissingThenSettingsAreNull() = runTest {
         feature.self().setRawStoredState(
-            Toggle.State(remoteEnableState = true, settings = """{"version": "1.0", "domains": []}"""),
+            Toggle.State(remoteEnableState = true, settings = """{"version": "1.0"}"""),
         )
         provider.onPrivacyConfigDownloaded()
 
         assertNull(provider.scriptletsSettings.value)
-        assertNull(provider.domainsSettings.value)
     }
 
     @Test
@@ -117,30 +109,6 @@ class AdBlockingExtensionConfigProviderTest {
         )
     }
 
-    @Test
-    fun whenSettingsAreValidThenDomainsSettingsReflectsParsedConfig() = runTest {
-        feature.self().setRawStoredState(Toggle.State(remoteEnableState = true, settings = validSettingsJson))
-        provider.onPrivacyConfigDownloaded()
-
-        assertEquals(
-            listOf(Domain("youtube.com"), Domain("m.youtube.com")),
-            provider.domainsSettings.value?.domains,
-        )
-    }
-
-    @Test
-    fun whenDomainsFieldIsAbsentThenDomainsSettingsHasEmptyDomains() = runTest {
-        feature.self().setRawStoredState(
-            Toggle.State(
-                remoteEnableState = true,
-                settings = """{"version": "1.0", "scriptlets": {}}""",
-            ),
-        )
-        provider.onPrivacyConfigDownloaded()
-
-        assertEquals(emptyList<Domain>(), provider.domainsSettings.value?.domains)
-    }
-
     @Test
     fun whenSettingsAreValidAtConstructionThenScriptletsSettingsEmitsInitialValue() = runTest {
         feature.self().setRawStoredState(Toggle.State(remoteEnableState = true, settings = validSettingsJson))