Fix RiskLevelCalculationTask not being executed if no new match was found...

Fix RiskLevelCalculationTask not being executed if no new match was found (EXPOSUREAPP-3849) (#1657)

* Execute risk calculation even if no match.

* Pass an "originTag" with each started task to support debugging efforts.
Fix unit test regression.

* Re-add missing download task arguments.

* Fix misleading log statement.

Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/RiskLevelAndKeyRetrievalBenchmark.kt

-package de.rki.coronawarnapp.test
-
-import android.content.Context
-import android.text.format.Formatter
-import de.rki.coronawarnapp.diagnosiskeys.download.DownloadDiagnosisKeysTask
-import de.rki.coronawarnapp.diagnosiskeys.download.DownloadDiagnosisKeysTask.Progress.ApiSubmissionFinished
-import de.rki.coronawarnapp.diagnosiskeys.download.DownloadDiagnosisKeysTask.Progress.ApiSubmissionStarted
-import de.rki.coronawarnapp.diagnosiskeys.download.DownloadDiagnosisKeysTask.Progress.KeyFilesDownloadFinished
-import de.rki.coronawarnapp.diagnosiskeys.download.DownloadDiagnosisKeysTask.Progress.KeyFilesDownloadStarted
-import de.rki.coronawarnapp.risk.RiskLevelTask
-import de.rki.coronawarnapp.task.Task
-import de.rki.coronawarnapp.task.common.DefaultTaskRequest
-import de.rki.coronawarnapp.task.submitAndListen
-import de.rki.coronawarnapp.util.di.AppInjector
-import kotlinx.coroutines.flow.collect
-import kotlinx.coroutines.flow.map
-import timber.log.Timber
-import java.util.UUID
-
-class RiskLevelAndKeyRetrievalBenchmark(
-    private val context: Context,
-    private val countries: List<String>
-) {
-
-    /**
-     * the key cache instance used to store queried dates and hours
-     */
-    private val keyCache = AppInjector.component.keyCacheRepository
-
-    /**
-     * Calls the RetrieveDiagnosisKeysTransaction and RiskLevelTransaction and measures them.
-     * Results are displayed using a label
-     * @param callCount defines how often the transactions should be called (each call will be
-     * measured separately)
-     */
-    suspend fun start(
-        callCount: Int,
-        onBenchmarkCompletedListener: OnBenchmarkCompletedListener
-    ) {
-
-        var resultInfo = StringBuilder()
-            .append(
-                "MEASUREMENT Running for Countries:\n " +
-                    "${countries.joinToString(", ")}\n\n"
-            )
-            .append("Result: \n\n")
-            .append("#\t Combined \t Download \t Sub \t Risk \t File # \t  F. size\n")
-
-        onBenchmarkCompletedListener(resultInfo.toString())
-
-        repeat(callCount) { index ->
-
-            keyCache.clear()
-
-            var keyRetrievalError = ""
-            var keyFileCount: Int = -1
-            var keyFileDownloadDuration: Long = -1
-            var keyFilesSize: Long = -1
-            var apiSubmissionDuration: Long = -1
-
-            measureDiagnosticKeyRetrieval(
-                label = "#$index",
-                countries = countries,
-                downloadFinished = { duration, keyCount, totalFileSize ->
-                    keyFileCount = keyCount
-                    keyFileDownloadDuration = duration
-                    keyFilesSize = totalFileSize
-                }, apiSubmissionFinished = { duration ->
-                    apiSubmissionDuration = duration
-                })
-
-            var calculationDuration: Long = -1
-            var calculationError = ""
-
-            measureKeyCalculation("#$index") {
-                if (it != null) calculationDuration = it
-
-                // build result entry for current iteration with all gathered data
-                resultInfo.append(
-                    "${index + 1}. \t ${calculationDuration + keyFileDownloadDuration + apiSubmissionDuration} ms \t " +
-                        "$keyFileDownloadDuration ms " + "\t $apiSubmissionDuration ms" +
-                        "\t $calculationDuration ms \t $keyFileCount \t " +
-                        "${Formatter.formatFileSize(context, keyFilesSize)}\n"
-                )
-
-                if (keyRetrievalError.isNotEmpty()) {
-                    resultInfo.append("Key Retrieval Error: $keyRetrievalError\n")
-                }
-
-                if (calculationError.isNotEmpty()) {
-                    resultInfo.append("Calculation Error: $calculationError\n")
-                }
-
-                onBenchmarkCompletedListener(resultInfo.toString())
-            }
-        }
-    }
-
-    private suspend fun measureKeyCalculation(label: String, callback: (Long?) -> Unit) {
-        val uuid = UUID.randomUUID()
-        val t0 = System.currentTimeMillis()
-        AppInjector.component.taskController.tasks
-            .map {
-                it
-                    .map { taskInfo -> taskInfo.taskState }
-                    .filter { taskState -> taskState.request.id == uuid && taskState.isFinished }
-            }
-            .collect {
-                it.firstOrNull()?.also { state ->
-                    Timber.v("MEASURE [Risk Level Calculation] $label finished")
-                    callback.invoke(
-                        if (state.error != null)
-                            null
-                        else
-                            System.currentTimeMillis() - t0
-                    )
-                }
-            }
-        Timber.v("MEASURE [Risk Level Calculation] $label started")
-        AppInjector.component.taskController.submit(
-            DefaultTaskRequest(
-                RiskLevelTask::class,
-                object : Task.Arguments {},
-                uuid
-            )
-        )
-    }
-
-    private suspend fun measureDiagnosticKeyRetrieval(
-        label: String,
-        countries: List<String>,
-        downloadFinished: (duration: Long, keyCount: Int, fileSize: Long) -> Unit,
-        apiSubmissionFinished: (duration: Long) -> Unit
-    ) {
-        var keyFileDownloadStart: Long = -1
-        var apiSubmissionStarted: Long = -1
-
-        AppInjector.component.taskController.submitAndListen(
-            DefaultTaskRequest(DownloadDiagnosisKeysTask::class, DownloadDiagnosisKeysTask.Arguments(countries))
-        ).collect { progress: Task.Progress ->
-            when (progress) {
-                is KeyFilesDownloadStarted -> {
-                    Timber.v("MEASURE [Diagnostic Key Files] $label started")
-                    keyFileDownloadStart = System.currentTimeMillis()
-                }
-                is KeyFilesDownloadFinished -> {
-                    Timber.v("MEASURE [Diagnostic Key Files] $label finished")
-                    val duration = System.currentTimeMillis() - keyFileDownloadStart
-                    downloadFinished(duration, progress.keyCount, progress.fileSize)
-                }
-                is ApiSubmissionStarted -> {
-                    apiSubmissionStarted = System.currentTimeMillis()
-                }
-                is ApiSubmissionFinished -> {
-                    val duration = System.currentTimeMillis() - apiSubmissionStarted
-                    apiSubmissionFinished(duration)
-                }
-            }
-        }
-    }
-}
-
-typealias OnBenchmarkCompletedListener = (resultInfo: String) -> Unit

Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/api/ui/TestForApiFragmentViewModel.kt

@@ -18,7 +18,7 @@ class TestForApiFragmentViewModel @AssistedInject constructor(
 ) : CWAViewModel() {
 
     fun calculateRiskLevelClicked() {
-        taskController.submit(DefaultTaskRequest(RiskLevelTask::class))
+        taskController.submit(DefaultTaskRequest(RiskLevelTask::class, originTag = "TestForApiFragmentViewModel"))
     }
 
     val gmsState by smartLiveData {

Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/risklevel/ui/TestRiskLevelCalculationFragmentCWAViewModel.kt

@@ -83,14 +83,23 @@ class TestRiskLevelCalculationFragmentCWAViewModel @AssistedInject constructor(
     fun retrieveDiagnosisKeys() {
         launch {
             taskController.submitBlocking(
-                DefaultTaskRequest(DownloadDiagnosisKeysTask::class, DownloadDiagnosisKeysTask.Arguments())
+                DefaultTaskRequest(
+                    DownloadDiagnosisKeysTask::class,
+                    DownloadDiagnosisKeysTask.Arguments(),
+                    originTag = "TestRiskLevelCalculationFragmentCWAViewModel.retrieveDiagnosisKeys()"
+                )
             )
             calculateRiskLevel()
         }
     }
 
     fun calculateRiskLevel() {
-        taskController.submit(DefaultTaskRequest(RiskLevelTask::class))
+        taskController.submit(
+            DefaultTaskRequest(
+                RiskLevelTask::class,
+                originTag = "TestRiskLevelCalculationFragmentCWAViewModel.calculateRiskLevel()"
+            )
+        )
     }
 
     fun resetRiskLevel() {

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

@@ -47,7 +47,7 @@ class ConfigChangeDetector @Inject constructor(
         if (newIdentifier != oldConfigId) {
             Timber.i("New config id ($newIdentifier) differs from last one ($oldConfigId), resetting.")
             RiskLevelRepositoryDeferrer.resetRiskLevel()
-            taskController.submit(DefaultTaskRequest(RiskLevelTask::class))
+            taskController.submit(DefaultTaskRequest(RiskLevelTask::class, originTag = "ConfigChangeDetector"))
         } else {
             Timber.v("Config identifier ($oldConfigId) didn't change, NOOP.")
         }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTask.kt

@@ -223,8 +223,7 @@ class DownloadDiagnosisKeysTask @Inject constructor(
     }
 
     class Arguments(
-        val requestedCountries: List<String>? = null,
-        val withConstraints: Boolean = false
+        val requestedCountries: List<String>? = null
     ) : Task.Arguments
 
     data class Config(

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/reporting/ExceptionReporter.kt

@@ -16,17 +16,19 @@ import de.rki.coronawarnapp.util.tryHumanReadableError
 import java.io.PrintWriter
 import java.io.StringWriter
 
-fun Throwable.report(exceptionCategory: ExceptionCategory) =
+fun Throwable.report(exceptionCategory: ExceptionCategory) {
     this.report(exceptionCategory, null, null)
+}
 
 fun Throwable.report(
     exceptionCategory: ExceptionCategory,
     prefix: String?,
     suffix: String?
 ) {
+    reportProblem(tag = prefix, info = suffix)
+
     if (CWADebug.isAUnitTest) return
 
-    reportProblem(tag = prefix, info = suffix)
     val context = CoronaWarnApplication.getAppContext()
 
     val intent = Intent(ReportingConstants.ERROR_REPORT_LOCAL_BROADCAST_CHANNEL)

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

@@ -36,7 +36,9 @@ class ExposureStateUpdateWorker @AssistedInject constructor(
                     Timber.v("exposure summary state updated: $it")
                 }
 
-            taskController.submit(DefaultTaskRequest(RiskLevelTask::class))
+            taskController.submit(
+                DefaultTaskRequest(RiskLevelTask::class, originTag = "ExposureStateUpdateWorker")
+            )
             Timber.v("risk level calculation triggered")
         } catch (e: ApiException) {
             e.report(ExceptionCategory.EXPOSURENOTIFICATION)

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

@@ -16,7 +16,7 @@ import de.rki.coronawarnapp.exception.UnknownBroadcastException
 import de.rki.coronawarnapp.exception.reporting.report
 import de.rki.coronawarnapp.nearby.ExposureStateUpdateWorker
 import de.rki.coronawarnapp.nearby.modules.detectiontracker.ExposureDetectionTracker
-import de.rki.coronawarnapp.nearby.modules.detectiontracker.TrackedExposureDetection
+import de.rki.coronawarnapp.nearby.modules.detectiontracker.TrackedExposureDetection.Result
 import de.rki.coronawarnapp.util.coroutine.AppScope
 import de.rki.coronawarnapp.util.coroutine.DispatcherProvider
 import kotlinx.coroutines.CoroutineScope
@@ -42,25 +42,35 @@ class ExposureStateUpdateReceiver : BroadcastReceiver() {
     @Inject @AppScope lateinit var scope: CoroutineScope
     @Inject lateinit var dispatcherProvider: DispatcherProvider
     @Inject lateinit var exposureDetectionTracker: ExposureDetectionTracker
-    lateinit var context: Context
+    @Inject lateinit var workManager: WorkManager
 
     override fun onReceive(context: Context, intent: Intent) {
         Timber.tag(TAG).d("onReceive(context=%s, intent=%s)", context, intent)
         AndroidInjection.inject(this, context)
-        this.context = context
 
         val action = intent.action
         Timber.tag(TAG).v("Looking up action: %s", action)
 
         val async = goAsync()
-        scope.launch(context = dispatcherProvider.Default) {
+
+        scope.launch(context = scope.coroutineContext) {
             try {
-                when (action) {
-                    ACTION_EXPOSURE_STATE_UPDATED -> processStateUpdates(intent)
-                    ACTION_EXPOSURE_NOT_FOUND -> processNotFound(intent)
-                    else -> throw UnknownBroadcastException(action)
-                }
+                val token = intent.requireToken()
+
+                trackDetection(token, action)
+
+                val data = Data
+                    .Builder()
+                    .putString(EXTRA_TOKEN, token)
+                    .build()
+
+                OneTimeWorkRequest
+                    .Builder(ExposureStateUpdateWorker::class.java)
+                    .setInputData(data)
+                    .build()
+                    .let { workManager.enqueue(it) }
             } catch (e: Exception) {
+                Timber.e(e, "Failed to process intent.")
                 e.report(INTERNAL)
             } finally {
                 Timber.tag(TAG).i("Finished processing broadcast.")
@@ -69,39 +79,16 @@ class ExposureStateUpdateReceiver : BroadcastReceiver() {
         }
     }
 
-    private fun processStateUpdates(intent: Intent) {
-        Timber.tag(TAG).i("Processing ACTION_EXPOSURE_STATE_UPDATED")
-
-        val workManager = WorkManager.getInstance(context)
-
-        val token = intent.requireToken()
-
-        val data = Data
-            .Builder()
-            .putString(EXTRA_TOKEN, token)
-            .build()
-
-        OneTimeWorkRequest
-            .Builder(ExposureStateUpdateWorker::class.java)
-            .setInputData(data)
-            .build()
-            .let { workManager.enqueue(it) }
-
-        exposureDetectionTracker.finishExposureDetection(
-            token,
-            TrackedExposureDetection.Result.UPDATED_STATE
-        )
-    }
-
-    private fun processNotFound(intent: Intent) {
-        Timber.tag(TAG).i("Processing ACTION_EXPOSURE_NOT_FOUND")
-
-        val token = intent.requireToken()
-
-        exposureDetectionTracker.finishExposureDetection(
-            token,
-            TrackedExposureDetection.Result.NO_MATCHES
-        )
+    private fun trackDetection(token: String, action: String?) {
+        when (action) {
+            ACTION_EXPOSURE_STATE_UPDATED -> {
+                exposureDetectionTracker.finishExposureDetection(token, Result.UPDATED_STATE)
+            }
+            ACTION_EXPOSURE_NOT_FOUND -> {
+                exposureDetectionTracker.finishExposureDetection(token, Result.NO_MATCHES)
+            }
+            else -> throw UnknownBroadcastException(action)
+        }
     }
 
     private fun Intent.requireToken(): String = getStringExtra(EXTRA_TOKEN).also {

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

@@ -71,6 +71,7 @@ class TracingRepository @Inject constructor(
     private fun List<TaskInfo>.isRiskLevelTaskRunning() = any {
         it.taskState.isActive && it.taskState.request.type == RiskLevelTask::class
     }
+
     private fun List<TaskInfo>.isDownloadDiagnosisKeysTaskRunning() = any {
         it.taskState.isActive && it.taskState.request.type == DownloadDiagnosisKeysTask::class
     }
@@ -89,10 +90,15 @@ class TracingRepository @Inject constructor(
             taskController.submitBlocking(
                 DefaultTaskRequest(
                     DownloadDiagnosisKeysTask::class,
-                    DownloadDiagnosisKeysTask.Arguments()
+                    DownloadDiagnosisKeysTask.Arguments(),
+                    originTag = "TracingRepository.refreshDiagnosisKeys()"
+                )
+            )
+            taskController.submit(
+                DefaultTaskRequest(
+                    RiskLevelTask::class, originTag = "TracingRepository.refreshDiagnosisKeys()"
                 )
             )
-            taskController.submit(DefaultTaskRequest(RiskLevelTask::class))
             TimerHelper.startManualKeyRetrievalTimer()
         }
     }
@@ -137,12 +143,15 @@ class TracingRepository @Inject constructor(
                     taskController.submitBlocking(
                         DefaultTaskRequest(
                             DownloadDiagnosisKeysTask::class,
-                            DownloadDiagnosisKeysTask.Arguments()
+                            DownloadDiagnosisKeysTask.Arguments(),
+                            originTag = "TracingRepository.refreshRisklevel()"
                         )
                     )
                     TimerHelper.checkManualKeyRetrievalTimer()
 
-                    taskController.submit(DefaultTaskRequest(RiskLevelTask::class))
+                    taskController.submit(
+                        DefaultTaskRequest(RiskLevelTask::class, originTag = "TracingRepository.refreshRiskLevel()")
+                    )
                 }
             }
         }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/task/common/DefaultTaskRequest.kt

@@ -8,7 +8,8 @@ import kotlin.reflect.KClass
 data class DefaultTaskRequest(
     override val type: KClass<out Task<Task.Progress, Task.Result>>,
     override val arguments: Task.Arguments = object : Task.Arguments {},
-    override val id: UUID = UUID.randomUUID()
+    override val id: UUID = UUID.randomUUID(),
+    val originTag: String? = null
 ) : TaskRequest {
 
     fun toNewTask(): DefaultTaskRequest = copy(id = UUID.randomUUID())

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

@@ -51,7 +51,8 @@ class WatchdogService @Inject constructor(
             val state = taskController.submitBlocking(
                 DefaultTaskRequest(
                     DownloadDiagnosisKeysTask::class,
-                    DownloadDiagnosisKeysTask.Arguments(null, true)
+                    DownloadDiagnosisKeysTask.Arguments(),
+                    originTag = "WatchdogService"
                 )
             )
             if (state.isFailed) {

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

@@ -40,12 +40,11 @@ class DiagnosisKeyRetrievalOneTimeWorker @AssistedInject constructor(
         taskController.submitBlocking(
             DefaultTaskRequest(
                 DownloadDiagnosisKeysTask::class,
-                DownloadDiagnosisKeysTask.Arguments(null, true)
+                DownloadDiagnosisKeysTask.Arguments(),
+                originTag = "DiagnosisKeyRetrievalOneTimeWorker"
             )
         ).error?.also { error: Throwable ->
-            Timber.w(
-                error, "$id: Error during startWithConstraints()."
-            )
+            Timber.w(error, "$id: Error when submitting DownloadDiagnosisKeysTask.")
 
             if (runAttemptCount > BackgroundConstants.WORKER_RETRY_COUNT_THRESHOLD) {
                 Timber.w(error, "$id: Retry attempts exceeded.")

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/receiver/ExposureStateUpdateReceiverTest.kt

@@ -12,12 +12,13 @@ import de.rki.coronawarnapp.nearby.modules.detectiontracker.ExposureDetectionTra
 import de.rki.coronawarnapp.nearby.modules.detectiontracker.TrackedExposureDetection
 import de.rki.coronawarnapp.util.di.AppInjector
 import io.mockk.MockKAnnotations
+import io.mockk.Runs
 import io.mockk.clearAllMocks
 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.mockkStatic
 import io.mockk.verifySequence
 import kotlinx.coroutines.test.TestCoroutineScope
 import org.junit.jupiter.api.AfterEach
@@ -36,6 +37,7 @@ class ExposureStateUpdateReceiverTest : BaseTest() {
     @MockK private lateinit var intent: Intent
     @MockK private lateinit var workManager: WorkManager
     @MockK private lateinit var exposureDetectionTracker: ExposureDetectionTracker
+
     private val scope = TestCoroutineScope()
 
     class TestApp : Application(), HasAndroidInjector {
@@ -48,23 +50,25 @@ class ExposureStateUpdateReceiverTest : BaseTest() {
     @BeforeEach
     fun setUp() {
         MockKAnnotations.init(this)
-        mockkStatic(WorkManager::class)
         every { intent.getStringExtra(ExposureNotificationClient.EXTRA_TOKEN) } returns "token"
 
         mockkObject(AppInjector)
 
+        every { workManager.enqueue(any<WorkRequest>()) } answers { mockk() }
+
         val application = mockk<TestApp>()
         every { context.applicationContext } returns application
+
         val broadcastReceiverInjector = AndroidInjector<Any> {
             it as ExposureStateUpdateReceiver
             it.exposureDetectionTracker = exposureDetectionTracker
             it.dispatcherProvider = TestDispatcherProvider
             it.scope = scope
+            it.workManager = workManager
         }
         every { application.androidInjector() } returns broadcastReceiverInjector
 
-        every { WorkManager.getInstance(context) } returns workManager
-        every { workManager.enqueue(any<WorkRequest>()) } answers { mockk() }
+        every { exposureDetectionTracker.finishExposureDetection(any(), any()) } just Runs
     }
 
     @AfterEach
@@ -77,9 +81,11 @@ class ExposureStateUpdateReceiverTest : BaseTest() {
         every { intent.action } returns ExposureNotificationClient.ACTION_EXPOSURE_STATE_UPDATED
         ExposureStateUpdateReceiver().onReceive(context, intent)
 
+        scope.advanceUntilIdle()
+
         verifySequence {
-            workManager.enqueue(any<WorkRequest>())
             exposureDetectionTracker.finishExposureDetection("token", TrackedExposureDetection.Result.UPDATED_STATE)
+            workManager.enqueue(any<WorkRequest>())
         }
     }
 
@@ -88,8 +94,11 @@ class ExposureStateUpdateReceiverTest : BaseTest() {
         every { intent.action } returns ExposureNotificationClient.ACTION_EXPOSURE_NOT_FOUND
         ExposureStateUpdateReceiver().onReceive(context, intent)
 
+        scope.advanceUntilIdle()
+
         verifySequence {
             exposureDetectionTracker.finishExposureDetection("token", TrackedExposureDetection.Result.NO_MATCHES)
+            workManager.enqueue(any<WorkRequest>())
         }
     }
 }