From d90f5a4554f83105fb9cf2a5ce605f0893400ef2 Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Fri, 30 Apr 2021 12:33:21 +0200
Subject: [PATCH] Fix positive test abort condition for ENS key provision
(EXPOSUREAPP-6879) (#3014)
* Fix positive test abort condition for ENS submission.
We want to abort if there is a positive test, and not just when we are allowed to submit (which is transient).
* Also abort PresenceTracing risk calculation on positive tests.
Co-authored-by: Mohamed <mohamed.metwalli@sap.com>
---
.../download/DownloadDiagnosisKeysTask.kt | 26 +++++-----
.../execution/PresenceTracingWarningTask.kt | 25 ++++++----
.../download/DownloadDiagnosisKeysTaskTest.kt | 4 +-
.../PresenceTracingWarningTaskTest.kt | 47 ++++++++++++++++++-
4 files changed, 77 insertions(+), 25 deletions(-)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTask.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTask.kt
index 9c415cdc5..ba1448d41 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTask.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTask.kt
@@ -35,14 +35,14 @@ class DownloadDiagnosisKeysTask @Inject constructor(
private val timeStamper: TimeStamper,
private val settings: DownloadDiagnosisKeysSettings,
private val coronaTestRepository: CoronaTestRepository,
-) : Task<DownloadDiagnosisKeysTask.Progress, Task.Result> {
+) : Task<DownloadDiagnosisKeysTask.Progress, DownloadDiagnosisKeysTask.Result> {
private val internalProgress = ConflatedBroadcastChannel<Progress>()
override val progress: Flow<Progress> = internalProgress.asFlow()
private var isCanceled = false
- override suspend fun run(arguments: Task.Arguments): Task.Result {
+ override suspend fun run(arguments: Task.Arguments): Result {
val rollbackItems = mutableListOf<RollbackItem>()
try {
Timber.d("Running with arguments=%s", arguments)
@@ -55,7 +55,7 @@ class DownloadDiagnosisKeysTask @Inject constructor(
*/
if (!enfClient.isTracingEnabled.first()) {
Timber.tag(TAG).w("EN is not enabled, skipping RetrieveDiagnosisKeys")
- return object : Task.Result {}
+ return Result()
}
throwIfCancelled()
@@ -76,14 +76,14 @@ class DownloadDiagnosisKeysTask @Inject constructor(
if (!exposureConfig.isDeviceTimeCorrect) {
Timber.tag(TAG).w("Aborting, Device time is incorrect, offset=%s", exposureConfig.localOffset)
- return object : Task.Result {}
+ return Result()
}
val now = timeStamper.nowUTC
if (exposureConfig.maxExposureDetectionsPerUTCDay == 0) {
Timber.tag(TAG).w("Exposure detections are disabled! maxExposureDetectionsPerUTCDay=0")
- return object : Task.Result {}
+ return Result()
}
val trackedExposureDetections = enfClient.latestTrackedExposureDetection().first()
@@ -93,12 +93,12 @@ class DownloadDiagnosisKeysTask @Inject constructor(
if (!isUpdateToEnfV2 && wasLastDetectionPerformedRecently(now, exposureConfig, trackedExposureDetections)) {
// At most one detection every 6h
Timber.tag(TAG).i("task aborted, because detection was performed recently")
- return object : Task.Result {}
+ return Result()
}
if (!isUpdateToEnfV2 && hasRecentDetectionAndNoNewFiles(now, keySyncResult, trackedExposureDetections)) {
Timber.tag(TAG).i("task aborted, last check was within 24h, and there are no new files")
- return object : Task.Result {}
+ return Result()
}
val availableKeyFiles = keySyncResult.availableKeys.map { it.path }
@@ -114,10 +114,10 @@ class DownloadDiagnosisKeysTask @Inject constructor(
// remember version code of this execution for next time
settings.updateLastVersionCodeToCurrent()
- val isAllowedToSubmitKeys = coronaTestRepository.coronaTests.first().any { it.isSubmissionAllowed }
- if (isAllowedToSubmitKeys) {
- Timber.tag(TAG).i("task aborted, positive test result")
- return object : Task.Result {}
+ val isPositive = coronaTestRepository.coronaTests.first().any { it.isPositive }
+ if (isPositive) {
+ Timber.tag(TAG).i("EW risk calculation aborted, positive test result available.")
+ return Result()
}
Timber.tag(TAG).d("Attempting submission to ENF")
@@ -129,7 +129,7 @@ class DownloadDiagnosisKeysTask @Inject constructor(
internalProgress.send(Progress.ApiSubmissionFinished)
- return object : Task.Result {}
+ return Result()
} catch (error: Exception) {
Timber.tag(TAG).e(error)
@@ -215,6 +215,8 @@ class DownloadDiagnosisKeysTask @Inject constructor(
isCanceled = true
}
+ class Result : Task.Result
+
sealed class Progress : Task.Progress {
object ApiSubmissionStarted : Progress()
object ApiSubmissionFinished : Progress()
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/risk/execution/PresenceTracingWarningTask.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/risk/execution/PresenceTracingWarningTask.kt
index b5c6b195d..b8b64db0f 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/risk/execution/PresenceTracingWarningTask.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/risk/execution/PresenceTracingWarningTask.kt
@@ -2,6 +2,7 @@ package de.rki.coronawarnapp.presencetracing.risk.execution
import de.rki.coronawarnapp.appconfig.AppConfigProvider
import de.rki.coronawarnapp.bugreporting.reportProblem
+import de.rki.coronawarnapp.coronatest.CoronaTestRepository
import de.rki.coronawarnapp.exception.ExceptionCategory
import de.rki.coronawarnapp.exception.reporting.report
import de.rki.coronawarnapp.presencetracing.checkins.CheckInRepository
@@ -17,9 +18,9 @@ import de.rki.coronawarnapp.util.TimeStamper
import kotlinx.coroutines.channels.ConflatedBroadcastChannel
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.asFlow
+import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.firstOrNull
import org.joda.time.Duration
-import org.joda.time.Instant
import timber.log.Timber
import javax.inject.Inject
import javax.inject.Provider
@@ -31,7 +32,8 @@ class PresenceTracingWarningTask @Inject constructor(
private val presenceTracingRiskRepository: PresenceTracingRiskRepository,
private val traceWarningRepository: TraceWarningRepository,
private val checkInsRepository: CheckInRepository,
- private val presenceTracingRiskMapper: PresenceTracingRiskMapper
+ private val presenceTracingRiskMapper: PresenceTracingRiskMapper,
+ private val coronaTestRepository: CoronaTestRepository,
) : Task<PresenceTracingWarningTaskProgress, PresenceTracingWarningTask.Result> {
private val internalProgress = ConflatedBroadcastChannel<PresenceTracingWarningTaskProgress>()
@@ -59,7 +61,6 @@ class PresenceTracingWarningTask @Inject constructor(
}
private suspend fun doWork(): Result {
- val nowUTC = timeStamper.nowUTC
checkCancel()
Timber.tag(TAG).d("Resetting config to make sure latest changes are considered.")
@@ -75,7 +76,7 @@ class PresenceTracingWarningTask @Inject constructor(
} else {
Timber.tag(TAG).w("WarningPackage sync failed: %s", syncResult)
presenceTracingRiskRepository.reportCalculation(successful = false)
- return Result(calculatedAt = nowUTC)
+ return Result()
}
presenceTracingRiskRepository.deleteStaleData()
@@ -89,7 +90,13 @@ class PresenceTracingWarningTask @Inject constructor(
presenceTracingRiskRepository.reportCalculation(successful = true)
- return Result(calculatedAt = nowUTC)
+ return Result()
+ }
+
+ val isPositive = coronaTestRepository.coronaTests.first().any { it.isPositive }
+ if (isPositive) {
+ Timber.tag(TAG).i("PT risk calculation aborted, positive test result available.")
+ return Result()
}
val unprocessedPackages = traceWarningRepository.unprocessedWarningPackages.firstOrNull() ?: emptyList()
@@ -100,7 +107,7 @@ class PresenceTracingWarningTask @Inject constructor(
presenceTracingRiskRepository.reportCalculation(successful = true)
- return Result(calculatedAt = nowUTC)
+ return Result()
}
Timber.tag(TAG).d("Running check-in matcher.")
@@ -131,7 +138,7 @@ class PresenceTracingWarningTask @Inject constructor(
matcherResult.processedPackages.map { it.warningPackage.packageId }
)
- return Result(calculatedAt = nowUTC)
+ return Result()
}
private fun checkCancel() {
@@ -143,9 +150,7 @@ class PresenceTracingWarningTask @Inject constructor(
isCanceled = true
}
- data class Result(
- val calculatedAt: Instant
- ) : Task.Result
+ class Result : Task.Result
data class Config(
override val executionTimeout: Duration = Duration.standardMinutes(9),
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTaskTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTaskTest.kt
index 3ad5c90bc..55891e4f3 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTaskTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/DownloadDiagnosisKeysTaskTest.kt
@@ -53,7 +53,7 @@ class DownloadDiagnosisKeysTaskTest : BaseTest() {
private val coronaTests: MutableStateFlow<Set<CoronaTest>> = MutableStateFlow(
setOf(
- mockk<CoronaTest>().apply { every { isSubmissionAllowed } returns false }
+ mockk<CoronaTest>().apply { every { isPositive } returns false }
)
)
@@ -241,7 +241,7 @@ class DownloadDiagnosisKeysTaskTest : BaseTest() {
@Test
fun `we do not submit keys if user got positive test results`() = runBlockingTest {
coronaTests.value = setOf(
- mockk<CoronaTest>().apply { every { isSubmissionAllowed } returns true }
+ mockk<CoronaTest>().apply { every { isPositive } returns true }
)
createInstance().run(DownloadDiagnosisKeysTask.Arguments())
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/execution/PresenceTracingWarningTaskTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/execution/PresenceTracingWarningTaskTest.kt
index 57b4cfb40..79a5518b6 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/execution/PresenceTracingWarningTaskTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/risk/execution/PresenceTracingWarningTaskTest.kt
@@ -1,5 +1,7 @@
package de.rki.coronawarnapp.presencetracing.risk.execution
+import de.rki.coronawarnapp.coronatest.CoronaTestRepository
+import de.rki.coronawarnapp.coronatest.type.CoronaTest
import de.rki.coronawarnapp.presencetracing.checkins.CheckInRepository
import de.rki.coronawarnapp.presencetracing.risk.calculation.CheckInWarningMatcher
import de.rki.coronawarnapp.presencetracing.risk.calculation.PresenceTracingRiskMapper
@@ -24,6 +26,7 @@ import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.just
import io.mockk.mockk
+import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.runBlockingTest
import org.joda.time.Duration
@@ -42,11 +45,20 @@ class PresenceTracingWarningTaskTest : BaseTest() {
@MockK lateinit var traceWarningRepository: TraceWarningRepository
@MockK lateinit var checkInsRepository: CheckInRepository
@MockK lateinit var presenceTracingRiskMapper: PresenceTracingRiskMapper
+ @MockK lateinit var coronaTestRepository: CoronaTestRepository
+
+ private val coronaTests: MutableStateFlow<Set<CoronaTest>> = MutableStateFlow(
+ setOf(
+ mockk<CoronaTest>().apply { every { isPositive } returns false }
+ )
+ )
@BeforeEach
fun setup() {
MockKAnnotations.init(this)
+ every { coronaTestRepository.coronaTests } returns coronaTests
+
every { timeStamper.nowUTC } returns Instant.ofEpochMilli(9000)
coEvery { syncTool.syncPackages() } returns TraceWarningPackageSyncTool.SyncResult(successful = true)
coEvery { checkInWarningMatcher.process(any(), any()) } answers {
@@ -84,7 +96,8 @@ class PresenceTracingWarningTaskTest : BaseTest() {
presenceTracingRiskRepository = presenceTracingRiskRepository,
traceWarningRepository = traceWarningRepository,
checkInsRepository = checkInsRepository,
- presenceTracingRiskMapper = presenceTracingRiskMapper
+ presenceTracingRiskMapper = presenceTracingRiskMapper,
+ coronaTestRepository = coronaTestRepository,
)
@Test
@@ -95,6 +108,9 @@ class PresenceTracingWarningTaskTest : BaseTest() {
syncTool.syncPackages()
presenceTracingRiskRepository.deleteStaleData()
checkInsRepository.checkInsWithinRetention
+
+ coronaTestRepository.coronaTests
+
traceWarningRepository.unprocessedWarningPackages
checkInWarningMatcher.process(any(), any())
@@ -229,6 +245,35 @@ class PresenceTracingWarningTaskTest : BaseTest() {
PresenceTracingWarningTask.Config().executionTimeout shouldBeLessThan maxDuration
}
+ @Test
+ fun `we do not submit keys if user got positive test results`() = runBlockingTest {
+ coronaTests.value = setOf(
+ mockk<CoronaTest>().apply { every { isPositive } returns true }
+ )
+
+ createInstance().run(mockk()) shouldNotBe null
+
+ coVerifySequence {
+ syncTool.syncPackages()
+ presenceTracingRiskRepository.deleteStaleData()
+ checkInsRepository.checkInsWithinRetention
+
+ coronaTestRepository.coronaTests
+ }
+
+ coVerify(exactly = 0) {
+ traceWarningRepository.unprocessedWarningPackages
+
+ checkInWarningMatcher.process(any(), any())
+
+ presenceTracingRiskRepository.reportCalculation(
+ successful = any(),
+ overlaps = any()
+ )
+ traceWarningRepository.markPackagesProcessed(any())
+ }
+ }
+
companion object {
val CHECKIN_1 = createCheckIn(
id = 2L,
--
GitLab