Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add killswitch, support incremental rollouts, and force update when downloading privacy config #5683

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,11 @@ interface MaliciousSiteProtectionFeature {
@Toggle.InternalAlwaysEnabled
@Toggle.DefaultValue(false)
fun self(): Toggle

@Toggle.InternalAlwaysEnabled
@Toggle.DefaultValue(false)
fun visibleAndOnByDefault(): Toggle

@Toggle.DefaultValue(true)
fun canUpdateDatasets(): Toggle
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import com.duckduckgo.app.lifecycle.MainProcessLifecycleObserver
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.malicioussiteprotection.impl.data.MaliciousSiteRepository
import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin
import com.squareup.anvil.annotations.ContributesMultibinding
import dagger.SingleInstanceIn
import java.util.concurrent.TimeUnit
Expand All @@ -51,7 +52,7 @@ class MaliciousSiteProtectionFiltersUpdateWorker(

override suspend fun doWork(): Result {
return withContext(dispatcherProvider.io()) {
if (maliciousSiteProtectionFeature.isFeatureEnabled().not()) {
if (maliciousSiteProtectionFeature.isFeatureEnabled().not() || maliciousSiteProtectionFeature.canUpdateDatasets().not()) {
return@withContext Result.success()
}
return@withContext if (maliciousSiteRepository.loadFilters().isSuccess) {
Expand All @@ -63,6 +64,10 @@ class MaliciousSiteProtectionFiltersUpdateWorker(
}
}

@ContributesMultibinding(
scope = AppScope::class,
boundType = PrivacyConfigCallbackPlugin::class,
)
@ContributesMultibinding(
scope = AppScope::class,
boundType = MainProcessLifecycleObserver::class,
Expand All @@ -72,17 +77,33 @@ class MaliciousSiteProtectionFiltersUpdateWorkerScheduler @Inject constructor(
private val workManager: WorkManager,
private val maliciousSiteProtectionFeature: MaliciousSiteProtectionRCFeature,

) : MainProcessLifecycleObserver {
) : PrivacyConfigCallbackPlugin, MainProcessLifecycleObserver {

override fun onCreate(owner: LifecycleOwner) {
enqueuePeriodicWork()
}

override fun onPrivacyConfigDownloaded() {
enqueuePeriodicWork()
}

private fun enqueuePeriodicWork() {
if (maliciousSiteProtectionFeature.isFeatureEnabled().not() || maliciousSiteProtectionFeature.canUpdateDatasets().not()) {
workManager.cancelUniqueWork(MALICIOUS_SITE_PROTECTION_FILTERS_UPDATE_WORKER_TAG)
return
}
val workerRequest = PeriodicWorkRequestBuilder<MaliciousSiteProtectionFiltersUpdateWorker>(
maliciousSiteProtectionFeature.getFilterSetUpdateFrequency(),
TimeUnit.MINUTES,
)
.addTag(MALICIOUS_SITE_PROTECTION_FILTERS_UPDATE_WORKER_TAG)
.setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 1, TimeUnit.MINUTES)
.build()
workManager.enqueueUniquePeriodicWork(MALICIOUS_SITE_PROTECTION_FILTERS_UPDATE_WORKER_TAG, ExistingPeriodicWorkPolicy.UPDATE, workerRequest)
workManager.enqueueUniquePeriodicWork(
MALICIOUS_SITE_PROTECTION_FILTERS_UPDATE_WORKER_TAG,
ExistingPeriodicWorkPolicy.UPDATE,
workerRequest,
)
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import com.duckduckgo.app.lifecycle.MainProcessLifecycleObserver
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.malicioussiteprotection.impl.data.MaliciousSiteRepository
import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin
import com.squareup.anvil.annotations.ContributesMultibinding
import dagger.SingleInstanceIn
import java.util.concurrent.TimeUnit
Expand All @@ -51,7 +52,7 @@ class MaliciousSiteProtectionHashPrefixesUpdateWorker(

override suspend fun doWork(): Result {
return withContext(dispatcherProvider.io()) {
if (maliciousSiteProtectionFeature.isFeatureEnabled().not()) {
if (maliciousSiteProtectionFeature.isFeatureEnabled().not() || maliciousSiteProtectionFeature.canUpdateDatasets().not()) {
return@withContext Result.success()
}
return@withContext if (maliciousSiteRepository.loadHashPrefixes().isSuccess) {
Expand All @@ -63,6 +64,10 @@ class MaliciousSiteProtectionHashPrefixesUpdateWorker(
}
}

@ContributesMultibinding(
scope = AppScope::class,
boundType = PrivacyConfigCallbackPlugin::class,
)
@ContributesMultibinding(
scope = AppScope::class,
boundType = MainProcessLifecycleObserver::class,
Expand All @@ -72,9 +77,21 @@ class MaliciousSiteProtectionHashPrefixesUpdateWorkerScheduler @Inject construct
private val workManager: WorkManager,
private val maliciousSiteProtectionFeature: MaliciousSiteProtectionRCFeature,

) : MainProcessLifecycleObserver {
) : PrivacyConfigCallbackPlugin, MainProcessLifecycleObserver {

override fun onCreate(owner: LifecycleOwner) {
enqueuePeriodicWork()
}

override fun onPrivacyConfigDownloaded() {
enqueuePeriodicWork()
}

private fun enqueuePeriodicWork() {
if (maliciousSiteProtectionFeature.isFeatureEnabled().not() || maliciousSiteProtectionFeature.canUpdateDatasets().not()) {
workManager.cancelUniqueWork(MALICIOUS_SITE_PROTECTION_HASH_PREFIXES_UPDATE_WORKER_TAG)
return
}
val workerRequest = PeriodicWorkRequestBuilder<MaliciousSiteProtectionHashPrefixesUpdateWorker>(
maliciousSiteProtectionFeature.getHashPrefixUpdateFrequency(),
TimeUnit.MINUTES,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import org.json.JSONObject

interface MaliciousSiteProtectionRCFeature {
fun isFeatureEnabled(): Boolean
fun canUpdateDatasets(): Boolean
fun getHashPrefixUpdateFrequency(): Long
fun getFilterSetUpdateFrequency(): Long
}
Expand All @@ -45,6 +46,7 @@ class RealMaliciousSiteProtectionRCFeature @Inject constructor(
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
) : MaliciousSiteProtectionRCFeature, PrivacyConfigCallbackPlugin {
private var isFeatureEnabled = false
private var canUpdateDatasets = false

private var hashPrefixUpdateFrequency = 20L
private var filterSetUpdateFrequency = 720L
Expand All @@ -71,9 +73,15 @@ class RealMaliciousSiteProtectionRCFeature @Inject constructor(
return isFeatureEnabled
}

override fun canUpdateDatasets(): Boolean {
return canUpdateDatasets
}

private fun loadToMemory() {
appCoroutineScope.launch(dispatchers.io()) {
isFeatureEnabled = maliciousSiteProtectionFeature.self().isEnabled()
isFeatureEnabled = maliciousSiteProtectionFeature.self().isEnabled() &&
maliciousSiteProtectionFeature.visibleAndOnByDefault().isEnabled()
canUpdateDatasets = maliciousSiteProtectionFeature.canUpdateDatasets().isEnabled()
maliciousSiteProtectionFeature.self().getSettings()?.let {
JSONObject(it).let { settings ->
hashPrefixUpdateFrequency = settings.getLong("hashPrefixUpdateFrequency")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.mockito.Mockito.mock
import org.mockito.Mockito.verify
import org.mockito.kotlin.capture
import org.mockito.kotlin.eq
import org.mockito.kotlin.never
import org.mockito.kotlin.whenever

@RunWith(AndroidJUnit4::class)
Expand All @@ -41,6 +42,7 @@ class MaliciousSiteProtectionFiltersUpdateWorkerTest {
worker.maliciousSiteRepository = maliciousSiteRepository
worker.dispatcherProvider = dispatcherProvider
worker.maliciousSiteProtectionFeature = maliciousSiteProtectionFeature
whenever(maliciousSiteProtectionFeature.canUpdateDatasets()).thenReturn(true)
}

@Test
Expand All @@ -49,6 +51,17 @@ class MaliciousSiteProtectionFiltersUpdateWorkerTest {

val result = worker.doWork()

verify(maliciousSiteRepository, never()).loadFilters()
assertEquals(success(), result)
}

@Test
fun doWork_returnsSuccessWhenCanUpdateDatasetsIsDisabled() = runTest {
whenever(maliciousSiteProtectionFeature.canUpdateDatasets()).thenReturn(false)

val result = worker.doWork()

verify(maliciousSiteRepository, never()).loadFilters()
assertEquals(success(), result)
}

Expand Down Expand Up @@ -80,12 +93,12 @@ class MaliciousSiteProtectionFiltersUpdateWorkerSchedulerTest {
private val scheduler = MaliciousSiteProtectionFiltersUpdateWorkerScheduler(workManager, maliciousSiteProtectionFeature)

@Test
fun onCreate_schedulesWorkerWithUpdateFrequencyFromRCFlag() {
fun onPrivacyConfigDownloaded_schedulesWorkerWithUpdateFrequencyFromRCFlagAndUpdatePolicy() {
val updateFrequencyMinutes = 15L

whenever(maliciousSiteProtectionFeature.getFilterSetUpdateFrequency()).thenReturn(updateFrequencyMinutes)

scheduler.onCreate(mock())
scheduler.onPrivacyConfigDownloaded()

val workRequestCaptor = ArgumentCaptor.forClass(PeriodicWorkRequest::class.java)
verify(workManager).enqueueUniquePeriodicWork(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.mockito.Mockito.mock
import org.mockito.Mockito.verify
import org.mockito.kotlin.capture
import org.mockito.kotlin.eq
import org.mockito.kotlin.never
import org.mockito.kotlin.whenever

@RunWith(AndroidJUnit4::class)
Expand All @@ -41,6 +42,7 @@ class MaliciousSiteProtectionHashPrefixesUpdateWorkerTest {
worker.maliciousSiteRepository = maliciousSiteRepository
worker.dispatcherProvider = dispatcherProvider
worker.maliciousSiteProtectionFeature = maliciousSiteProtectionFeature
whenever(maliciousSiteProtectionFeature.canUpdateDatasets()).thenReturn(true)
}

@Test
Expand All @@ -49,6 +51,17 @@ class MaliciousSiteProtectionHashPrefixesUpdateWorkerTest {

val result = worker.doWork()

verify(maliciousSiteRepository, never()).loadHashPrefixes()
assertEquals(success(), result)
}

@Test
fun doWork_returnsSuccessWhenUpdateDatasetsIsDisabled() = runTest {
whenever(maliciousSiteProtectionFeature.canUpdateDatasets()).thenReturn(false)

val result = worker.doWork()

verify(maliciousSiteRepository, never()).loadHashPrefixes()
assertEquals(success(), result)
}

Expand Down Expand Up @@ -80,12 +93,12 @@ class MaliciousSiteProtectionHashPrefixesUpdateWorkerSchedulerTest {
private val scheduler = MaliciousSiteProtectionHashPrefixesUpdateWorkerScheduler(workManager, maliciousSiteProtectionFeature)

@Test
fun onCreate_schedulesWorkerWithUpdateFrequencyFromRCFlag() {
fun onPrivacyConfigDownloaded_schedulesWorkerWithUpdateFrequencyFromRCFlag() {
val updateFrequencyMinutes = 15L

whenever(maliciousSiteProtectionFeature.getHashPrefixUpdateFrequency()).thenReturn(updateFrequencyMinutes)

scheduler.onCreate(mock())
scheduler.onPrivacyConfigDownloaded()

val workRequestCaptor = ArgumentCaptor.forClass(PeriodicWorkRequest::class.java)
verify(workManager).enqueueUniquePeriodicWork(
Expand Down
Loading