From b5c762174c67324a9ed10d98227dce231d268c95 Mon Sep 17 00:00:00 2001 From: Matthias Urhahn <matthias.urhahn@sap.com> Date: Fri, 18 Jun 2021 11:32:53 +0200 Subject: [PATCH] Fix `labId` not being stored, when retrieved during polling. (#3490) (It may be null during initial registration) Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com> Co-authored-by: Juraj Kusnier <jurajkusnier@users.noreply.github.com> --- .../coronatest/server/CoronaTestResult.kt | 4 +- .../coronatest/type/pcr/PCRTestProcessor.kt | 31 ++++--- .../type/rapidantigen/RATestProcessor.kt | 21 ++--- .../coronatest/type/pcr/PCRProcessorTest.kt | 58 ++++++++++++- .../type/rapidantigen/RAProcessorTest.kt | 82 +++++++++++++------ 5 files changed, 142 insertions(+), 54 deletions(-) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/server/CoronaTestResult.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/server/CoronaTestResult.kt index 09c931f12..0beee6acd 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/server/CoronaTestResult.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/server/CoronaTestResult.kt @@ -8,8 +8,8 @@ import org.json.JSONObject data class CoronaTestResultResponse( val coronaTestResult: CoronaTestResult, - val sampleCollectedAt: Instant?, - val labId: String? + val sampleCollectedAt: Instant? = null, // Only used for RA tests + val labId: String? = null, // May initially be null while test is pending ) { companion object { fun fromResponse(response: VerificationApiV1.TestResultResponse) = diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRTestProcessor.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRTestProcessor.kt index ce35035b4..7a0457077 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRTestProcessor.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRTestProcessor.kt @@ -14,6 +14,7 @@ import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_NEGATIVE import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_PENDING import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_POSITIVE import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_REDEEMED +import de.rki.coronawarnapp.coronatest.server.CoronaTestResultResponse import de.rki.coronawarnapp.coronatest.server.RegistrationData import de.rki.coronawarnapp.coronatest.server.RegistrationRequest import de.rki.coronawarnapp.coronatest.server.VerificationKeyType @@ -152,32 +153,36 @@ class PCRTestProcessor @Inject constructor( return test } - val newTestResult = try { - submissionService.checkTestResult(test.registrationToken).coronaTestResult.let { - Timber.tag(TAG).d("Raw test result was %s", it) - analyticsTestResultCollector.reportTestResultReceived(it, type) - - it.toValidatedResult() - } + val response = try { + submissionService.checkTestResult(test.registrationToken) + .also { + Timber.tag(TAG).d("Raw test result was %s", it) + // TODO Should this be called here? Compare with RA + analyticsTestResultCollector.reportTestResultReceived(it.coronaTestResult, type) + } + .let { orig -> + orig.copy(coronaTestResult = orig.coronaTestResult.toValidatedResult()) + } } catch (e: BadRequestException) { if (isOlderThan21Days) { Timber.tag(TAG).w("HTTP 400 error after 21 days, remapping to PCR_REDEEMED.") - PCR_REDEEMED + CoronaTestResultResponse(coronaTestResult = PCR_REDEEMED) } else { Timber.tag(TAG).v("Unexpected HTTP 400 error, rethrowing...") throw e } } - if (newTestResult == PCR_POSITIVE) { + if (response.coronaTestResult == PCR_POSITIVE) { analyticsKeySubmissionCollector.reportPositiveTestResultReceived(type) } test.copy( - testResult = check60Days(test, newTestResult), - testResultReceivedAt = determineReceivedDate(test, newTestResult), + testResult = check60Days(test, response.coronaTestResult), + testResultReceivedAt = determineReceivedDate(test, response.coronaTestResult), lastUpdatedAt = nowUTC, - lastError = null + labId = response.labId ?: test.labId, + lastError = null, ) } catch (e: Exception) { Timber.tag(TAG).e(e, "Failed to poll server for %s", test) @@ -256,7 +261,7 @@ class PCRTestProcessor @Inject constructor( companion object { private val FINAL_STATES = setOf(PCR_POSITIVE, PCR_NEGATIVE, PCR_REDEEMED) - internal const val TAG = "PCRProcessor" + internal const val TAG = "PCRTestProcessor" } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RATestProcessor.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RATestProcessor.kt index e119cba0f..6cf5d6044 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RATestProcessor.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RATestProcessor.kt @@ -126,7 +126,7 @@ class RATestProcessor @Inject constructor( return test } - val newTestResult = try { + val response = try { submissionService.checkTestResult(test.registrationToken).let { Timber.tag(TAG).v("Raw test result was %s", it) it.copy( @@ -136,27 +136,24 @@ class RATestProcessor @Inject constructor( } catch (e: BadRequestException) { if (isOlderThan21Days) { Timber.tag(TAG).w("HTTP 400 error after 21 days, remapping to RAT_REDEEMED.") - CoronaTestResultResponse( - coronaTestResult = RAT_REDEEMED, - sampleCollectedAt = null, - labId = null - ) + CoronaTestResultResponse(coronaTestResult = RAT_REDEEMED) } else { Timber.tag(TAG).v("Unexpected HTTP 400 error, rethrowing...") throw e } } - if (newTestResult.coronaTestResult == RAT_POSITIVE) { + if (response.coronaTestResult == RAT_POSITIVE) { analyticsKeySubmissionCollector.reportPositiveTestResultReceived(type) } test.copy( - testResult = check60Days(test, newTestResult.coronaTestResult), - testResultReceivedAt = determineReceivedDate(test, newTestResult.coronaTestResult), + testResult = check60Days(test, response.coronaTestResult), + testResultReceivedAt = determineReceivedDate(test, response.coronaTestResult), lastUpdatedAt = nowUTC, - lastError = null, - sampleCollectedAt = newTestResult.sampleCollectedAt + sampleCollectedAt = response.sampleCollectedAt ?: test.sampleCollectedAt, + labId = response.labId ?: test.labId, + lastError = null ) } catch (e: Exception) { Timber.tag(TAG).e(e, "Failed to poll server for %s", test) @@ -231,7 +228,7 @@ class RATestProcessor @Inject constructor( companion object { private val FINAL_STATES = setOf(RAT_POSITIVE, RAT_NEGATIVE, RAT_REDEEMED) - internal const val TAG = "RapidAntigenProcessor" + internal const val TAG = "RATestProcessor" } } 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 d11874da7..d3de32a9c 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 @@ -161,8 +161,7 @@ class PCRProcessorTest : BaseTest() { RAT_NEGATIVE, RAT_POSITIVE, RAT_INVALID, - RAT_REDEEMED -> - instance.create(request).testResult shouldBe PCR_INVALID + RAT_REDEEMED -> instance.create(request).testResult shouldBe PCR_INVALID } } } @@ -290,7 +289,7 @@ class PCRProcessorTest : BaseTest() { testResultResponse = CoronaTestResultResponse( coronaTestResult = PCR_OR_RAT_PENDING, sampleCollectedAt = null, - labId = null, + labId = "labId", ) ) coEvery { submissionService.registerTest(any()) } answers { registrationData } @@ -305,6 +304,7 @@ class PCRProcessorTest : BaseTest() { isDccConsentGiven shouldBe true isDccDataSetCreated shouldBe false isDccSupportedByPoc shouldBe true + labId shouldBe "labId" } createInstance().create( @@ -316,6 +316,7 @@ class PCRProcessorTest : BaseTest() { isDccConsentGiven shouldBe false isDccDataSetCreated shouldBe false isDccSupportedByPoc shouldBe true + labId shouldBe "labId" } } @@ -330,4 +331,55 @@ class PCRProcessorTest : BaseTest() { isDccDataSetCreated = false ) } + + @Test + fun `response parameters are stored during initial registration`() = runBlockingTest { + val registrationData = RegistrationData( + registrationToken = "regtoken", + testResultResponse = CoronaTestResultResponse( + coronaTestResult = PCR_NEGATIVE, + labId = "labId", + ) + ) + coEvery { submissionService.registerTest(any()) } answers { registrationData } + + createInstance().create( + CoronaTestQRCode.PCR( + qrCodeGUID = "guid", + isDccConsentGiven = true, + dateOfBirth = LocalDate.parse("2021-06-02"), + ) + ).apply { + testResult shouldBe PCR_NEGATIVE + labId shouldBe "labId" + isDccDataSetCreated shouldBe false + isDccSupportedByPoc shouldBe true + } + } + + @Test + fun `new data received during polling is stored in the test`() = runBlockingTest { + val instance = createInstance() + val pcrTest = PCRCoronaTest( + identifier = "identifier", + lastUpdatedAt = Instant.EPOCH, + registeredAt = nowUTC, + registrationToken = "regtoken", + testResult = PCR_POSITIVE, + ) + + (instance.pollServer(pcrTest) as PCRCoronaTest).apply { + labId shouldBe null + } + + coEvery { submissionService.checkTestResult(any()) } returns CoronaTestResultResponse( + coronaTestResult = PCR_OR_RAT_PENDING, + sampleCollectedAt = nowUTC, + labId = "labId", + ) + + (instance.pollServer(pcrTest) as PCRCoronaTest).apply { + labId shouldBe "labId" + } + } } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RAProcessorTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RAProcessorTest.kt index c9c60d347..9a8114f0b 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RAProcessorTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RAProcessorTest.kt @@ -106,29 +106,6 @@ class RAProcessorTest : BaseTest() { analyticsTestResultCollector = analyticsTestResultCollector, ) - @Test - fun `if a test result poll returns a sc set it on the test`() = runBlockingTest { - val instance = createInstance() - val raTest = RACoronaTest( - identifier = "identifier", - lastUpdatedAt = Instant.EPOCH, - registeredAt = nowUTC, - registrationToken = "regtoken", - testResult = RAT_POSITIVE, - testedAt = Instant.EPOCH, - ) - - (instance.pollServer(raTest) as RACoronaTest).sampleCollectedAt shouldBe null - - coEvery { submissionService.checkTestResult(any()) } returns CoronaTestResultResponse( - coronaTestResult = PCR_OR_RAT_PENDING, - sampleCollectedAt = nowUTC, - labId = null, - ) - - (instance.pollServer(raTest) as RACoronaTest).sampleCollectedAt shouldBe nowUTC - } - @Test fun `if we receive a pending result 60 days after registration, we map to REDEEMED`() = runBlockingTest { val instance = createInstance() @@ -319,7 +296,7 @@ class RAProcessorTest : BaseTest() { testResultResponse = CoronaTestResultResponse( coronaTestResult = PCR_OR_RAT_PENDING, sampleCollectedAt = null, - labId = null, + labId = "labId", ) ) coEvery { submissionService.registerTest(any()) } answers { registrationData } @@ -336,6 +313,7 @@ class RAProcessorTest : BaseTest() { isDccConsentGiven shouldBe false isDccDataSetCreated shouldBe false isDccSupportedByPoc shouldBe false + labId shouldBe "labId" } createInstance().create( @@ -350,6 +328,7 @@ class RAProcessorTest : BaseTest() { isDccConsentGiven shouldBe true isDccDataSetCreated shouldBe false isDccSupportedByPoc shouldBe true + labId shouldBe "labId" } } @@ -364,4 +343,59 @@ class RAProcessorTest : BaseTest() { isDccDataSetCreated = false ) } + + @Test + fun `response parameters are stored during initial registration`() = runBlockingTest { + val registrationData = RegistrationData( + registrationToken = "regtoken", + testResultResponse = CoronaTestResultResponse( + coronaTestResult = RAT_NEGATIVE, + sampleCollectedAt = Instant.ofEpochSecond(123), + labId = "labId", + ) + ) + coEvery { submissionService.registerTest(any()) } answers { registrationData } + + createInstance().create( + CoronaTestQRCode.RapidAntigen( + hash = "hash", + createdAt = Instant.EPOCH, + dateOfBirth = LocalDate.parse("2021-06-02"), + ) + ).apply { + this as RACoronaTest + testResult shouldBe RAT_NEGATIVE + sampleCollectedAt shouldBe Instant.ofEpochSecond(123) + labId shouldBe "labId" + } + } + + @Test + fun `new data received during polling is stored in the test`() = runBlockingTest { + val instance = createInstance() + val raTest = RACoronaTest( + identifier = "identifier", + lastUpdatedAt = Instant.EPOCH, + registeredAt = nowUTC, + registrationToken = "regtoken", + testResult = RAT_POSITIVE, + testedAt = Instant.EPOCH, + ) + + (instance.pollServer(raTest) as RACoronaTest).apply { + sampleCollectedAt shouldBe null + labId shouldBe null + } + + coEvery { submissionService.checkTestResult(any()) } returns CoronaTestResultResponse( + coronaTestResult = PCR_OR_RAT_PENDING, + sampleCollectedAt = nowUTC, + labId = "labId", + ) + + (instance.pollServer(raTest) as RACoronaTest).apply { + sampleCollectedAt shouldBe nowUTC + labId shouldBe "labId" + } + } } -- GitLab