From 93d5587ae20302d8ec27b40262886a20130b6c9e Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Fri, 30 Apr 2021 08:17:29 +0200
Subject: [PATCH] Adjust test polling behavior 21 days after registration
(EXPOSUREAPP-6882) (#3016)
* Map HTTP 400 errors to *_REDEEMED after 21 days (server has deleted data set).
Stop test polling after 21 days if we are in a final state (*_REDEEMED).
* LINTs
* Improve comments.
---
.../coronatest/type/CoronaTestExtensions.kt | 8 +++
.../coronatest/type/pcr/PCRProcessor.kt | 40 ++++++++++----
.../rapidantigen/RapidAntigenProcessor.kt | 38 +++++++++----
.../type/CoronaTestExtensionsTest.kt | 34 ++++++++++++
.../coronatest/type/pcr/PCRProcessorTest.kt | 52 ++++++++++++++++++
.../rapidantigen/RapidAntigenProcessorTest.kt | 54 +++++++++++++++++++
6 files changed, 207 insertions(+), 19 deletions(-)
create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/CoronaTestExtensions.kt
create mode 100644 Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/CoronaTestExtensionsTest.kt
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/CoronaTestExtensions.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/CoronaTestExtensions.kt
new file mode 100644
index 000000000..a666b118c
--- /dev/null
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/CoronaTestExtensions.kt
@@ -0,0 +1,8 @@
+package de.rki.coronawarnapp.coronatest.type
+
+import org.joda.time.Duration
+import org.joda.time.Instant
+
+fun CoronaTest.isOlderThan21Days(nowUTC: Instant): Boolean {
+ return Duration(registeredAt, nowUTC).standardDays > 21
+}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessor.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessor.kt
index 9c01a64e9..a5d44f688 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessor.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessor.kt
@@ -18,9 +18,11 @@ import de.rki.coronawarnapp.coronatest.tan.CoronaTestTAN
import de.rki.coronawarnapp.coronatest.type.CoronaTest
import de.rki.coronawarnapp.coronatest.type.CoronaTestProcessor
import de.rki.coronawarnapp.coronatest.type.CoronaTestService
+import de.rki.coronawarnapp.coronatest.type.isOlderThan21Days
import de.rki.coronawarnapp.datadonation.analytics.modules.keysubmission.AnalyticsKeySubmissionCollector
import de.rki.coronawarnapp.datadonation.analytics.modules.registeredtest.TestResultDataCollector
import de.rki.coronawarnapp.exception.ExceptionCategory
+import de.rki.coronawarnapp.exception.http.BadRequestException
import de.rki.coronawarnapp.exception.http.CwaWebException
import de.rki.coronawarnapp.exception.reporting.report
import de.rki.coronawarnapp.util.TimeStamper
@@ -103,15 +105,33 @@ class PCRProcessor @Inject constructor(
test as PCRCoronaTest
if (test.isSubmitted) {
- Timber.tag(TAG).w("Not refreshing, we have already submitted.")
+ Timber.tag(TAG).w("Not polling, we have already submitted.")
return test
}
- val newTestResult = submissionService.asyncRequestTestResult(test.registrationToken).let {
- Timber.tag(TAG).d("Raw test result was %s", it)
- testResultDataCollector.updatePendingTestResultReceivedTime(it)
+ val nowUTC = timeStamper.nowUTC
+ val isOlderThan21Days = test.isOlderThan21Days(nowUTC)
- it.toValidatedResult()
+ if (isOlderThan21Days && test.testResult == PCR_REDEEMED) {
+ Timber.tag(TAG).w("Not polling, test is older than 21 days.")
+ return test
+ }
+
+ val newTestResult = try {
+ submissionService.asyncRequestTestResult(test.registrationToken).let {
+ Timber.tag(TAG).d("Raw test result was %s", it)
+ testResultDataCollector.updatePendingTestResultReceivedTime(it)
+
+ it.toValidatedResult()
+ }
+ } catch (e: BadRequestException) {
+ if (isOlderThan21Days) {
+ Timber.tag(TAG).w("HTTP 400 error after 21 days, remapping to PCR_REDEEMED.")
+ PCR_REDEEMED
+ } else {
+ Timber.tag(TAG).v("Unexpected HTTP 400 error, rethrowing...")
+ throw e
+ }
}
if (newTestResult == PCR_POSITIVE) {
@@ -119,9 +139,9 @@ class PCRProcessor @Inject constructor(
}
test.copy(
- testResult = check60PlusDays(test, newTestResult),
+ testResult = check21PlusDays(test, newTestResult),
testResultReceivedAt = determineReceivedDate(test, newTestResult),
- lastUpdatedAt = timeStamper.nowUTC,
+ lastUpdatedAt = nowUTC,
lastError = null
)
} catch (e: Exception) {
@@ -133,10 +153,10 @@ class PCRProcessor @Inject constructor(
}
}
- // After 60 days, the previously EXPIRED test is deleted from the server, and it will return pending again.
- private fun check60PlusDays(test: CoronaTest, newResult: CoronaTestResult): CoronaTestResult {
+ // After 21 days, the previously EXPIRED test is deleted from the server, and it may return pending again.
+ private fun check21PlusDays(test: CoronaTest, newResult: CoronaTestResult): CoronaTestResult {
val calculateDays = Duration(test.registeredAt, timeStamper.nowUTC).standardDays
- Timber.tag(TAG).d("Calculated test age: %d days", calculateDays)
+ Timber.tag(TAG).d("Calculated test age: %d days, newResult=%s", calculateDays, newResult)
return if (newResult == PCR_OR_RAT_PENDING && calculateDays >= BackgroundConstants.POLLING_VALIDITY_MAX_DAYS) {
Timber.tag(TAG).d("$calculateDays is exceeding the maximum polling duration")
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenProcessor.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenProcessor.kt
index 2a9d412af..964bb01bd 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenProcessor.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenProcessor.kt
@@ -17,7 +17,9 @@ import de.rki.coronawarnapp.coronatest.tan.CoronaTestTAN
import de.rki.coronawarnapp.coronatest.type.CoronaTest
import de.rki.coronawarnapp.coronatest.type.CoronaTestProcessor
import de.rki.coronawarnapp.coronatest.type.CoronaTestService
+import de.rki.coronawarnapp.coronatest.type.isOlderThan21Days
import de.rki.coronawarnapp.exception.ExceptionCategory
+import de.rki.coronawarnapp.exception.http.BadRequestException
import de.rki.coronawarnapp.exception.http.CwaWebException
import de.rki.coronawarnapp.exception.reporting.report
import de.rki.coronawarnapp.util.TimeStamper
@@ -82,19 +84,37 @@ class RapidAntigenProcessor @Inject constructor(
test as RACoronaTest
if (test.isSubmitted) {
- Timber.tag(TAG).w("Not refreshing, we have already submitted.")
+ Timber.tag(TAG).w("Not polling, we have already submitted.")
return test
}
- val newTestResult = submissionService.asyncRequestTestResult(test.registrationToken).let {
- Timber.tag(TAG).v("Raw test result was %s", it)
- it.toValidatedResult()
+ val nowUTC = timeStamper.nowUTC
+ val isOlderThan21Days = test.isOlderThan21Days(nowUTC)
+
+ if (isOlderThan21Days && test.testResult == RAT_REDEEMED) {
+ Timber.tag(TAG).w("Not polling, test is older than 21 days.")
+ return test
+ }
+
+ val newTestResult = try {
+ submissionService.asyncRequestTestResult(test.registrationToken).let {
+ Timber.tag(TAG).v("Raw test result was %s", it)
+ it.toValidatedResult()
+ }
+ } catch (e: BadRequestException) {
+ if (isOlderThan21Days) {
+ Timber.tag(TAG).w("HTTP 400 error after 21 days, remapping to RAT_REDEEMED.")
+ RAT_REDEEMED
+ } else {
+ Timber.tag(TAG).v("Unexpected HTTP 400 error, rethrowing...")
+ throw e
+ }
}
test.copy(
- testResult = check60PlusDays(test, newTestResult),
+ testResult = check21PlusDays(test, newTestResult),
testResultReceivedAt = determineReceivedDate(test, newTestResult),
- lastUpdatedAt = timeStamper.nowUTC,
+ lastUpdatedAt = nowUTC,
lastError = null
)
} catch (e: Exception) {
@@ -106,10 +126,10 @@ class RapidAntigenProcessor @Inject constructor(
}
}
- // After 60 days, the previously EXPIRED test is deleted from the server, and it will return pending again.
- private fun check60PlusDays(test: CoronaTest, newResult: CoronaTestResult): CoronaTestResult {
+ // After 21 days, the previously EXPIRED test is deleted from the server, and it may return pending again.
+ private fun check21PlusDays(test: CoronaTest, newResult: CoronaTestResult): CoronaTestResult {
val calculateDays = Duration(test.registeredAt, timeStamper.nowUTC).standardDays
- Timber.tag(TAG).d("Calculated test age: %d days", calculateDays)
+ Timber.tag(TAG).d("Calculated test age: %d days, newResult=%s", calculateDays, newResult)
return if (
(newResult == PCR_OR_RAT_PENDING || newResult == RAT_PENDING) &&
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/CoronaTestExtensionsTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/CoronaTestExtensionsTest.kt
new file mode 100644
index 000000000..aba14190d
--- /dev/null
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/CoronaTestExtensionsTest.kt
@@ -0,0 +1,34 @@
+package de.rki.coronawarnapp.coronatest.type
+
+import io.kotest.matchers.shouldBe
+import io.mockk.every
+import io.mockk.mockk
+import org.joda.time.Duration
+import org.joda.time.Instant
+import org.junit.jupiter.api.Test
+import testhelpers.BaseTest
+
+class CoronaTestExtensionsTest : BaseTest() {
+
+ @Test
+ fun `is test older than 21 days - time changes`() {
+ val test = mockk<CoronaTest>().apply {
+ every { registeredAt } returns Instant.EPOCH
+ }
+
+ test.isOlderThan21Days(Instant.EPOCH.plus(Duration.standardDays(21))) shouldBe false
+ test.isOlderThan21Days(Instant.EPOCH.plus(Duration.standardDays(22))) shouldBe true
+ }
+
+ @Test
+ fun `is test older than 21 days - test changes`() {
+ val nowUTC = Instant.EPOCH.plus(Duration.standardDays(22))
+ mockk<CoronaTest>().apply {
+ every { registeredAt } returns Instant.EPOCH
+ }.isOlderThan21Days(nowUTC) shouldBe true
+
+ mockk<CoronaTest>().apply {
+ every { registeredAt } returns Instant.EPOCH.plus(Duration.standardDays(1))
+ }.isOlderThan21Days(nowUTC) shouldBe false
+ }
+}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessorTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessorTest.kt
index 7a2c51d7d..ab6c4b178 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessorTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessorTest.kt
@@ -17,6 +17,7 @@ import de.rki.coronawarnapp.coronatest.tan.CoronaTestTAN
import de.rki.coronawarnapp.coronatest.type.CoronaTestService
import de.rki.coronawarnapp.datadonation.analytics.modules.keysubmission.AnalyticsKeySubmissionCollector
import de.rki.coronawarnapp.datadonation.analytics.modules.registeredtest.TestResultDataCollector
+import de.rki.coronawarnapp.exception.http.BadRequestException
import de.rki.coronawarnapp.util.TimeStamper
import io.kotest.matchers.shouldBe
import io.mockk.MockKAnnotations
@@ -181,4 +182,55 @@ class PCRProcessorTest : BaseTest() {
isResultAvailableNotificationSent shouldBe false
}
}
+
+ @Test
+ fun `polling is skipped if test is older than 21 days and state was already REDEEMED`() = runBlockingTest {
+ coEvery { submissionService.asyncRequestTestResult(any()) } answers { PCR_POSITIVE }
+
+ val instance = createInstance()
+
+ val pcrTest = PCRCoronaTest(
+ identifier = "identifier",
+ lastUpdatedAt = Instant.EPOCH,
+ registeredAt = nowUTC.minus(Duration.standardDays(22)),
+ registrationToken = "regtoken",
+ testResult = PCR_REDEEMED
+ )
+
+ // Older than 21 days and already redeemed
+ instance.pollServer(pcrTest) shouldBe pcrTest
+
+ // Older than 21 days but not in final state, we take value from server
+ instance.pollServer(
+ pcrTest.copy(testResult = PCR_NEGATIVE)
+ ).testResult shouldBe PCR_POSITIVE
+ }
+
+ @Test
+ fun `http 400 errors map to REDEEMED (EXPIRED) state after 21 days`() = runBlockingTest {
+ val ourBadRequest = BadRequestException("Who?")
+ coEvery { submissionService.asyncRequestTestResult(any()) } throws ourBadRequest
+
+ val instance = createInstance()
+
+ val pcrTest = PCRCoronaTest(
+ identifier = "identifier",
+ lastUpdatedAt = Instant.EPOCH,
+ registeredAt = nowUTC,
+ registrationToken = "regtoken",
+ testResult = PCR_POSITIVE
+ )
+
+ // Test is not older than 21 days, we want the error!
+ instance.pollServer(pcrTest).apply {
+ testResult shouldBe PCR_POSITIVE
+ lastError shouldBe ourBadRequest
+ }
+
+ // Test IS older than 21 days, we expected the error, and map it to REDEEMED (expired)
+ instance.pollServer(pcrTest.copy(registeredAt = nowUTC.minus(Duration.standardDays(22)))).apply {
+ testResult shouldBe PCR_REDEEMED
+ lastError shouldBe null
+ }
+ }
}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenProcessorTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenProcessorTest.kt
index 76573d88b..3cd6913d9 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenProcessorTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenProcessorTest.kt
@@ -14,6 +14,7 @@ import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_POSITIVE
import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_REDEEMED
import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.values
import de.rki.coronawarnapp.coronatest.type.CoronaTestService
+import de.rki.coronawarnapp.exception.http.BadRequestException
import de.rki.coronawarnapp.util.TimeStamper
import io.kotest.matchers.shouldBe
import io.mockk.MockKAnnotations
@@ -146,4 +147,57 @@ class RapidAntigenProcessorTest : BaseTest() {
}
}
}
+
+ @Test
+ fun `polling is skipped if test is older than 21 days and state was already REDEEMED`() = runBlockingTest {
+ coEvery { submissionService.asyncRequestTestResult(any()) } answers { RAT_POSITIVE }
+
+ val instance = createInstance()
+
+ val raTest = RACoronaTest(
+ identifier = "identifier",
+ lastUpdatedAt = Instant.EPOCH,
+ registeredAt = nowUTC.minus(Duration.standardDays(22)),
+ registrationToken = "regtoken",
+ testResult = RAT_REDEEMED,
+ testedAt = Instant.EPOCH,
+ )
+
+ // Older than 21 days and already redeemed
+ instance.pollServer(raTest) shouldBe raTest
+
+ // Older than 21 days but not in final state, we take value from server
+ instance.pollServer(
+ raTest.copy(testResult = RAT_NEGATIVE)
+ ).testResult shouldBe RAT_POSITIVE
+ }
+
+ @Test
+ fun `http 400 errors map to REDEEMED (EXPIRED) state after 21 days`() = runBlockingTest {
+ val ourBadRequest = BadRequestException("Who?")
+ coEvery { submissionService.asyncRequestTestResult(any()) } throws ourBadRequest
+
+ val instance = createInstance()
+
+ val raTest = RACoronaTest(
+ identifier = "identifier",
+ lastUpdatedAt = Instant.EPOCH,
+ registeredAt = nowUTC,
+ registrationToken = "regtoken",
+ testResult = RAT_POSITIVE,
+ testedAt = Instant.EPOCH,
+ )
+
+ // Test is not older than 21 days, we want the error!
+ instance.pollServer(raTest).apply {
+ testResult shouldBe RAT_POSITIVE
+ lastError shouldBe ourBadRequest
+ }
+
+ // Test IS older than 21 days, we expected the error, and map it to REDEEMED (expired)
+ instance.pollServer(raTest.copy(registeredAt = nowUTC.minus(Duration.standardDays(22)))).apply {
+ testResult shouldBe RAT_REDEEMED
+ lastError shouldBe null
+ }
+ }
}
--
GitLab