From e2dfb4268730db73c3815799006b09df92ff3949 Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Mon, 4 Jan 2021 16:58:20 +0100
Subject: [PATCH] Better handling of issues caused by time traveling
(DownloadTask) (DEV) #2018
* If maxExposureDetectionsPerDay is 0, return a saner value than 99 days.
maxExposureDetectionsPerDay is checked separately and will abort the download task if necessary.
* When checking whether the last detection was recent, add a sanity check that causes it to not skip, if the last detection was in our future.
Co-authored-by: Ralf Gehrer <ralfgehrer@users.noreply.github.com>
---
.../mapping/ExposureDetectionConfigMapper.kt | 2 +-
.../download/DownloadDiagnosisKeysTask.kt | 18 ++++++++++++++++--
.../ExposureDetectionConfigMapperTest.kt | 4 ++--
.../download/DownloadDiagnosisKeysTaskTest.kt | 14 +++++++++++++-
4 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapper.kt
index 5110334a3..900ce5bef 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapper.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapper.kt
@@ -54,7 +54,7 @@ fun ExposureDetectionParametersAndroid?.maxExposureDetectionsPerDay(): Int =
fun ExposureDetectionParametersAndroid?.minTimeBetweenExposureDetections(): Duration {
val detectionsPerDay = this.maxExposureDetectionsPerDay()
return if (detectionsPerDay == 0) {
- Duration.standardDays(99)
+ Duration.standardDays(1)
} else {
(24 / detectionsPerDay).let { Duration.standardHours(it.toLong()) }
}
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 7be1a88e9..6a7414214 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
@@ -140,9 +140,23 @@ class DownloadDiagnosisKeysTask @Inject constructor(
trackedDetections: Collection<TrackedExposureDetection>
): Boolean {
val lastDetection = trackedDetections.maxByOrNull { it.startedAt }
- val nextDetectionAt = lastDetection?.startedAt?.plus(exposureConfig.minTimeBetweenDetections)
+ if (lastDetection == null) {
+ Timber.tag(TAG).d("No previous detections exist, don't abort.")
+ return false
+ }
+
+ if (lastDetection.startedAt.isAfter(now.plus(Duration.standardHours(1)))) {
+ Timber.tag(TAG).w("Last detection happened in our future? Don't abort as precaution.")
+ return false
+ }
+
+ val nextDetectionAt = lastDetection.startedAt.plus(exposureConfig.minTimeBetweenDetections)
+
+ Duration(now, nextDetectionAt).also {
+ Timber.tag(TAG).d("Next detection is available in %d min", it.standardMinutes)
+ }
- return (nextDetectionAt != null && now.isBefore(nextDetectionAt)).also {
+ return (now.isBefore(nextDetectionAt)).also {
if (it) Timber.tag(TAG).w("Aborting. Last detection is recent: %s (now=%s)", lastDetection, now)
}
}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapperTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapperTest.kt
index f39e4f9de..7a6e080ae 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapperTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapperTest.kt
@@ -21,13 +21,13 @@ class ExposureDetectionConfigMapperTest : BaseTest() {
}
@Test
- fun `detection interval 0 defaults to almost infinite delay`() {
+ fun `detection interval 0 defaults to sane delay`() {
val exposureDetectionParameters = ExposureDetectionParametersAndroid.newBuilder()
val rawConfig = AppConfigAndroid.ApplicationConfigurationAndroid.newBuilder()
.setExposureDetectionParameters(exposureDetectionParameters)
.build()
createInstance().map(rawConfig).apply {
- minTimeBetweenDetections shouldBe Duration.standardDays(99)
+ minTimeBetweenDetections shouldBe Duration.standardDays(1)
maxExposureDetectionsPerUTCDay shouldBe 0
}
}
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 f85ce9af7..c22593689 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
@@ -141,7 +141,7 @@ class DownloadDiagnosisKeysTaskTest : BaseTest() {
}
@Test
- fun `execution is skipped if last detection was recent via`() = runBlockingTest {
+ fun `execution is skipped if last detection was recent`() = runBlockingTest {
// Last detection was at T+2h
every { timeStamper.nowUTC } returns Instant.EPOCH.plus(Duration.standardHours(2))
@@ -157,6 +157,18 @@ class DownloadDiagnosisKeysTaskTest : BaseTest() {
}
}
+ @Test
+ fun `execution is NOT skipped if last detection is in our future`() = runBlockingTest {
+ // Last detection was at T, i.e. our time is now T-1h, so it was in our future.
+ every { timeStamper.nowUTC } returns Instant.EPOCH.minus(Duration.standardHours(1).plus(1))
+
+ createInstance().run(DownloadDiagnosisKeysTask.Arguments())
+
+ coVerify {
+ enfClient.provideDiagnosisKeys(any(), any())
+ }
+ }
+
@Test
fun `wasLastDetectionPerformedRecently honors paramters from config`() = runBlockingTest {
every { timeStamper.nowUTC } returns Instant.EPOCH.plus(Duration.standardHours(4))
--
GitLab