Detect changes to app config (EXPOSUREAPP-2718) (#1583)

* + config changed detector

* tests

* ConfigChangeDetector.kt improvements.
* Fix config collecting scoping
* Move the identifier update into the RiskLevelTask
* Add test coverage

* Logs NO-OPs too.

* Filter emissions early if the value didn't change.

* extracted constant

Co-authored-by: Matthias Urhahn <matthias.urhahn@sap.com>

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt

@@ -11,6 +11,7 @@ import androidx.work.WorkManager
 import dagger.android.AndroidInjector
 import dagger.android.DispatchingAndroidInjector
 import dagger.android.HasAndroidInjector
+import de.rki.coronawarnapp.appconfig.ConfigChangeDetector
 import de.rki.coronawarnapp.bugreporting.loghistory.LogHistoryTree
 import de.rki.coronawarnapp.deadman.DeadmanNotificationScheduler
 import de.rki.coronawarnapp.exception.reporting.ErrorReportReceiver
@@ -44,6 +45,7 @@ class CoronaWarnApplication : Application(), HasAndroidInjector {
     @Inject lateinit var taskController: TaskController
     @Inject lateinit var foregroundState: ForegroundState
     @Inject lateinit var workManager: WorkManager
+    @Inject lateinit var configChangeDetector: ConfigChangeDetector
     @Inject lateinit var deadmanNotificationScheduler: DeadmanNotificationScheduler
     @LogHistoryTree @Inject lateinit var rollingLogHistory: Timber.Tree
 
@@ -79,6 +81,8 @@ class CoronaWarnApplication : Application(), HasAndroidInjector {
         if (LocalData.onboardingCompletedTimestamp() != null) {
             deadmanNotificationScheduler.schedulePeriodic()
         }
+
+        configChangeDetector.launch()
     }
 
     private val activityLifecycleCallback = object : ActivityLifecycleCallbacks {

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/ConfigChangeDetector.kt

+package de.rki.coronawarnapp.appconfig
+
+import androidx.annotation.VisibleForTesting
+import de.rki.coronawarnapp.risk.RiskLevel
+import de.rki.coronawarnapp.risk.RiskLevelData
+import de.rki.coronawarnapp.risk.RiskLevelTask
+import de.rki.coronawarnapp.storage.RiskLevelRepository
+import de.rki.coronawarnapp.task.TaskController
+import de.rki.coronawarnapp.task.common.DefaultTaskRequest
+import de.rki.coronawarnapp.util.coroutine.AppScope
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.flow.catch
+import kotlinx.coroutines.flow.distinctUntilChangedBy
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.onEach
+import timber.log.Timber
+import javax.inject.Inject
+
+class ConfigChangeDetector @Inject constructor(
+    private val appConfigProvider: AppConfigProvider,
+    private val taskController: TaskController,
+    @AppScope private val appScope: CoroutineScope,
+    private val riskLevelData: RiskLevelData
+) {
+
+    fun launch() {
+        Timber.v("Monitoring config changes.")
+        appConfigProvider.currentConfig
+            .distinctUntilChangedBy { it.identifier }
+            .onEach {
+                Timber.v("Running app config change checks.")
+                check(it.identifier)
+            }
+            .catch { Timber.e(it, "App config change checks failed.") }
+            .launchIn(appScope)
+    }
+
+    @VisibleForTesting
+    internal fun check(newIdentifier: String) {
+        if (riskLevelData.lastUsedConfigIdentifier == null) {
+            // No need to reset anything if we didn't calculate a risklevel yet.
+            Timber.d("Config changed, but no previous identifier is available.")
+            return
+        }
+
+        val oldConfigId = riskLevelData.lastUsedConfigIdentifier
+        if (newIdentifier != oldConfigId) {
+            Timber.i("New config id ($newIdentifier) differs from last one ($oldConfigId), resetting.")
+            RiskLevelRepositoryDeferrer.resetRiskLevel()
+            taskController.submit(DefaultTaskRequest(RiskLevelTask::class))
+        } else {
+            Timber.v("Config identifier ($oldConfigId) didn't change, NOOP.")
+        }
+    }
+
+    @VisibleForTesting
+    internal object RiskLevelRepositoryDeferrer {
+
+        fun resetRiskLevel() {
+            RiskLevelRepository.setRiskLevelScore(RiskLevel.UNDETERMINED)
+        }
+    }
+}

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/DefaultRiskLevels.kt

@@ -5,7 +5,7 @@ import androidx.core.app.NotificationManagerCompat
 import com.google.android.gms.nearby.exposurenotification.ExposureSummary
 import de.rki.coronawarnapp.CoronaWarnApplication
 import de.rki.coronawarnapp.R
-import de.rki.coronawarnapp.appconfig.AppConfigProvider
+import de.rki.coronawarnapp.appconfig.RiskCalculationConfig
 import de.rki.coronawarnapp.exception.RiskLevelCalculationException
 import de.rki.coronawarnapp.notification.NotificationHelper
 import de.rki.coronawarnapp.risk.RiskLevel.UNKNOWN_RISK_INITIAL
@@ -20,9 +20,7 @@ import javax.inject.Singleton
 import kotlin.math.round
 
 @Singleton
-class DefaultRiskLevels @Inject constructor(
-    private val appConfigProvider: AppConfigProvider
-) : RiskLevels {
+class DefaultRiskLevels @Inject constructor() : RiskLevels {
 
     override fun updateRepository(riskLevel: RiskLevel, time: Long) {
         val rollbackItems = mutableListOf<RollbackItem>()
@@ -78,8 +76,10 @@ class DefaultRiskLevels @Inject constructor(
             }
         }
 
-    override suspend fun isIncreasedRisk(lastExposureSummary: ExposureSummary): Boolean {
-        val appConfiguration = appConfigProvider.getAppConfig()
+    override suspend fun isIncreasedRisk(
+        lastExposureSummary: ExposureSummary,
+        appConfiguration: RiskCalculationConfig
+    ): Boolean {
         Timber.tag(TAG).v("Retrieved configuration from backend")
         // custom attenuation parameters to weigh the attenuation
         // values provided by the Google API

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/RiskLevelData.kt

+package de.rki.coronawarnapp.risk
+
+import android.content.Context
+import androidx.core.content.edit
+import de.rki.coronawarnapp.util.di.AppContext
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Singleton
+class RiskLevelData @Inject constructor(
+    @AppContext private val context: Context
+) {
+
+    private val prefs by lazy {
+        context.getSharedPreferences(NAME_SHARED_PREFS, Context.MODE_PRIVATE)
+    }
+
+    /**
+     * The identifier of the config used during the last risklevel calculation
+     */
+    var lastUsedConfigIdentifier: String?
+        get() = prefs.getString(PKEY_RISKLEVEL_CALC_LAST_CONFIG_ID, null)
+        set(value) = prefs.edit(true) {
+            putString(PKEY_RISKLEVEL_CALC_LAST_CONFIG_ID, value)
+        }
+
+    companion object {
+        private const val NAME_SHARED_PREFS = "risklevel_localdata"
+        private const val PKEY_RISKLEVEL_CALC_LAST_CONFIG_ID = "risklevel.config.identifier.last"
+    }
+}

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/RiskLevelTask.kt

@@ -2,6 +2,8 @@ package de.rki.coronawarnapp.risk
 
 import android.content.Context
 import com.google.android.gms.nearby.exposurenotification.ExposureSummary
+import de.rki.coronawarnapp.appconfig.AppConfigProvider
+import de.rki.coronawarnapp.appconfig.ConfigData
 import de.rki.coronawarnapp.exception.ExceptionCategory
 import de.rki.coronawarnapp.exception.RiskLevelCalculationException
 import de.rki.coronawarnapp.exception.reporting.report
@@ -38,7 +40,9 @@ class RiskLevelTask @Inject constructor(
     @AppContext private val context: Context,
     private val enfClient: ENFClient,
     private val timeStamper: TimeStamper,
-    private val backgroundModeStatus: BackgroundModeStatus
+    private val backgroundModeStatus: BackgroundModeStatus,
+    private val riskLevelData: RiskLevelData,
+    private val appConfigProvider: AppConfigProvider
 ) : Task<DefaultProgress, RiskLevelTask.Result> {
 
     private val internalProgress = ConflatedBroadcastChannel<DefaultProgress>()
@@ -60,6 +64,8 @@ class RiskLevelTask @Inject constructor(
                 return Result(NO_CALCULATION_POSSIBLE_TRACING_OFF)
             }
 
+            val configData: ConfigData = appConfigProvider.getAppConfig()
+
             with(riskLevels) {
                 return Result(
                     when {
@@ -75,7 +81,7 @@ class RiskLevelTask @Inject constructor(
                             UNKNOWN_RISK_OUTDATED_RESULTS_MANUAL
                         }
 
-                        isIncreasedRisk(getNewExposureSummary()).also {
+                        isIncreasedRisk(getNewExposureSummary(), configData).also {
                             checkCancel()
                         } -> INCREASED_RISK
 
@@ -87,6 +93,7 @@ class RiskLevelTask @Inject constructor(
                     }.also {
                         checkCancel()
                         updateRepository(it, timeStamper.nowUTC.millis)
+                        riskLevelData.lastUsedConfigIdentifier = configData.identifier
                     }
                 )
             }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/RiskLevels.kt

 package de.rki.coronawarnapp.risk
 
 import com.google.android.gms.nearby.exposurenotification.ExposureSummary
+import de.rki.coronawarnapp.appconfig.RiskCalculationConfig
 import de.rki.coronawarnapp.server.protocols.internal.AttenuationDurationOuterClass
 
 interface RiskLevels {
@@ -15,7 +16,10 @@ interface RiskLevels {
      */
     fun isActiveTracingTimeAboveThreshold(): Boolean
 
-    suspend fun isIncreasedRisk(lastExposureSummary: ExposureSummary): Boolean
+    suspend fun isIncreasedRisk(
+        lastExposureSummary: ExposureSummary,
+        appConfiguration: RiskCalculationConfig
+    ): Boolean
 
     fun updateRepository(
         riskLevel: RiskLevel,

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/ConfigChangeDetectorTest.kt

+package de.rki.coronawarnapp.appconfig
+
+import de.rki.coronawarnapp.risk.RiskLevelData
+import de.rki.coronawarnapp.task.TaskController
+import io.mockk.MockKAnnotations
+import io.mockk.Runs
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import io.mockk.just
+import io.mockk.mockk
+import io.mockk.mockkObject
+import io.mockk.verify
+import io.mockk.verifySequence
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.test.TestCoroutineScope
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import testhelpers.BaseTest
+
+class ConfigChangeDetectorTest : BaseTest() {
+
+    @MockK lateinit var appConfigProvider: AppConfigProvider
+    @MockK lateinit var taskController: TaskController
+    @MockK lateinit var riskLevelData: RiskLevelData
+
+    private val currentConfigFake = MutableStateFlow(mockConfigId("initial"))
+
+    @BeforeEach
+    fun setup() {
+        MockKAnnotations.init(this)
+
+        mockkObject(ConfigChangeDetector.RiskLevelRepositoryDeferrer)
+        every { ConfigChangeDetector.RiskLevelRepositoryDeferrer.resetRiskLevel() } just Runs
+
+        every { taskController.submit(any()) } just Runs
+        every { appConfigProvider.currentConfig } returns currentConfigFake
+    }
+
+    private fun mockConfigId(id: String): ConfigData {
+        return mockk<ConfigData>().apply {
+            every { identifier } returns id
+        }
+    }
+
+    private fun createInstance() = ConfigChangeDetector(
+        appConfigProvider = appConfigProvider,
+        taskController = taskController,
+        appScope = TestCoroutineScope(),
+        riskLevelData = riskLevelData
+    )
+
+    @Test
+    fun `new identifier without previous one is ignored`() {
+
+        every { riskLevelData.lastUsedConfigIdentifier } returns null
+
+        createInstance().launch()
+
+        verify(exactly = 0) {
+            taskController.submit(any())
+            ConfigChangeDetector.RiskLevelRepositoryDeferrer.resetRiskLevel()
+        }
+    }
+
+    @Test
+    fun `new identifier results in new risk level calculation`() {
+        every { riskLevelData.lastUsedConfigIdentifier } returns "I'm a new identifier"
+
+        createInstance().launch()
+
+        verifySequence {
+            ConfigChangeDetector.RiskLevelRepositoryDeferrer.resetRiskLevel()
+            taskController.submit(any())
+        }
+    }
+
+    @Test
+    fun `same idetifier results in no op`() {
+        every { riskLevelData.lastUsedConfigIdentifier } returns "initial"
+
+        createInstance().launch()
+
+        verify(exactly = 0) {
+            taskController.submit(any())
+            ConfigChangeDetector.RiskLevelRepositoryDeferrer.resetRiskLevel()
+        }
+    }
+
+    @Test
+    fun `new emissions keep triggering the check`() {
+        every { riskLevelData.lastUsedConfigIdentifier } returns "initial"
+
+        createInstance().launch()
+        currentConfigFake.value = mockConfigId("Straw")
+        currentConfigFake.value = mockConfigId("berry")
+
+        verifySequence {
+            ConfigChangeDetector.RiskLevelRepositoryDeferrer.resetRiskLevel()
+            taskController.submit(any())
+            ConfigChangeDetector.RiskLevelRepositoryDeferrer.resetRiskLevel()
+            taskController.submit(any())
+        }
+    }
+}

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/risk/RiskLevelDataTest.kt

+package de.rki.coronawarnapp.risk
+
+import android.content.Context
+import io.kotest.matchers.shouldBe
+import io.mockk.MockKAnnotations
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import testhelpers.BaseTest
+import testhelpers.preferences.MockSharedPreferences
+
+class RiskLevelDataTest : BaseTest() {
+
+    @MockK lateinit var context: Context
+    lateinit var preferences: MockSharedPreferences
+
+    @BeforeEach
+    fun setup() {
+        MockKAnnotations.init(this)
+        preferences = MockSharedPreferences()
+        every { context.getSharedPreferences("risklevel_localdata", Context.MODE_PRIVATE) } returns preferences
+    }
+
+    fun createInstance() = RiskLevelData(context = context)
+
+    @Test
+    fun `update last used config identifier`() {
+        createInstance().apply {
+            lastUsedConfigIdentifier shouldBe null
+            lastUsedConfigIdentifier = "Banana"
+            lastUsedConfigIdentifier shouldBe "Banana"
+            preferences.dataMapPeek.containsValue("Banana") shouldBe true
+
+            lastUsedConfigIdentifier = null
+            lastUsedConfigIdentifier shouldBe null
+            preferences.dataMapPeek.isEmpty() shouldBe true
+        }
+    }
+}

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/risk/RiskLevelTaskTest.kt

+package de.rki.coronawarnapp.risk
+
+import android.content.Context
+import android.net.ConnectivityManager
+import android.net.Network
+import android.net.NetworkCapabilities
+import de.rki.coronawarnapp.appconfig.AppConfigProvider
+import de.rki.coronawarnapp.appconfig.ConfigData
+import de.rki.coronawarnapp.nearby.ENFClient
+import de.rki.coronawarnapp.task.Task
+import de.rki.coronawarnapp.util.BackgroundModeStatus
+import de.rki.coronawarnapp.util.TimeStamper
+import io.mockk.MockKAnnotations
+import io.mockk.Runs
+import io.mockk.coEvery
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import io.mockk.just
+import io.mockk.mockk
+import io.mockk.verify
+import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.test.runBlockingTest
+import org.joda.time.Instant
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import testhelpers.BaseTest
+
+class RiskLevelTaskTest : BaseTest() {
+
+    @MockK lateinit var riskLevels: RiskLevels
+    @MockK lateinit var context: Context
+    @MockK lateinit var enfClient: ENFClient
+    @MockK lateinit var timeStamper: TimeStamper
+    @MockK lateinit var backgroundModeStatus: BackgroundModeStatus
+    @MockK lateinit var riskLevelData: RiskLevelData
+    @MockK lateinit var configData: ConfigData
+    @MockK lateinit var appConfigProvider: AppConfigProvider
+
+    private val arguments: Task.Arguments = object : Task.Arguments {}
+
+    private fun createTask() = RiskLevelTask(
+        riskLevels = riskLevels,
+        context = context,
+        enfClient = enfClient,
+        timeStamper = timeStamper,
+        backgroundModeStatus = backgroundModeStatus,
+        riskLevelData = riskLevelData,
+        appConfigProvider = appConfigProvider
+    )
+
+    @BeforeEach
+    fun setup() {
+        MockKAnnotations.init(this)
+        coEvery { appConfigProvider.getAppConfig() } returns configData
+        every { configData.identifier } returns "config-identifier"
+
+        every { context.getSystemService(Context.CONNECTIVITY_SERVICE) } returns mockk<ConnectivityManager>().apply {
+            every { activeNetwork } returns mockk<Network>().apply {
+                every { getNetworkCapabilities(any()) } returns mockk<NetworkCapabilities>().apply {
+                    every { hasCapability(any()) } returns true
+                }
+            }
+        }
+
+        every { enfClient.isTracingEnabled } returns flowOf(true)
+        every { timeStamper.nowUTC } returns Instant.EPOCH
+        every { riskLevels.updateRepository(any(), any()) } just Runs
+
+        every { riskLevelData.lastUsedConfigIdentifier = any() } just Runs
+    }
+
+    @Test
+    fun `last used config ID is set after calculation`() = runBlockingTest {
+        every { riskLevels.calculationNotPossibleBecauseOfNoKeys() } returns true
+        val task = createTask()
+        task.run(arguments)
+
+        verify { riskLevelData.lastUsedConfigIdentifier = "config-identifier" }
+    }
+}

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/risk/RiskLevelsTest.kt

 package de.rki.coronawarnapp.risk
 
 import com.google.android.gms.nearby.exposurenotification.ExposureSummary
-import de.rki.coronawarnapp.appconfig.AppConfigProvider
 import de.rki.coronawarnapp.server.protocols.internal.AttenuationDurationOuterClass
 import io.kotest.matchers.shouldBe
 import io.mockk.MockKAnnotations
-import io.mockk.impl.annotations.MockK
 import junit.framework.TestCase.assertEquals
 import org.junit.Before
 import org.junit.Test
@@ -13,13 +11,12 @@ import testhelpers.BaseTest
 
 class RiskLevelsTest : BaseTest() {
 
-    @MockK lateinit var appConfigProvider: AppConfigProvider
     private lateinit var riskLevels: DefaultRiskLevels
 
     @Before
     fun setUp() {
         MockKAnnotations.init(this)
-        riskLevels = DefaultRiskLevels(appConfigProvider)
+        riskLevels = DefaultRiskLevels()
     }
 
     @Test