From 93156bdef2e94a15b7be1156e9e272873c7d2548 Mon Sep 17 00:00:00 2001 From: Matthias Urhahn <matthias.urhahn@sap.com> Date: Wed, 28 Apr 2021 16:12:08 +0200 Subject: [PATCH] Improve error handling during CoronaTest result retrieval (EXPOSUREAPP-6784) (#2980) Only HTTP400 errors are mapped to TestInvalid state. Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com> --- .../type/pcr/PCRCoronaTestExtensions.kt | 4 +- .../RapidAntigenCoronaTestExtensions.kt | 4 +- .../type/pcr/PCRCoronaTestExtensionsTest.kt | 225 +++--------------- .../RapidAntigenCoronaTestExtensionsTest.kt | 44 ++++ 4 files changed, 78 insertions(+), 199 deletions(-) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTestExtensions.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTestExtensions.kt index 7ea0ce6d7..9d7c69f37 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTestExtensions.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTestExtensions.kt @@ -13,13 +13,13 @@ import de.rki.coronawarnapp.coronatest.type.pcr.SubmissionStatePCR.TestNegative import de.rki.coronawarnapp.coronatest.type.pcr.SubmissionStatePCR.TestPending import de.rki.coronawarnapp.coronatest.type.pcr.SubmissionStatePCR.TestPositive import de.rki.coronawarnapp.coronatest.type.pcr.SubmissionStatePCR.TestResultReady -import de.rki.coronawarnapp.exception.http.CwaServerError +import de.rki.coronawarnapp.exception.http.BadRequestException fun PCRCoronaTest?.toSubmissionState() = when { this == null -> NoTest isSubmitted -> SubmissionStatePCR.SubmissionDone(testRegisteredAt = registeredAt) isProcessing -> FetchingResult - lastError != null -> if (lastError is CwaServerError) TestPending else TestInvalid + lastError is BadRequestException -> TestInvalid else -> when (state) { INVALID -> TestError POSITIVE -> when { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenCoronaTestExtensions.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenCoronaTestExtensions.kt index 523b6f2a0..faedc26b6 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenCoronaTestExtensions.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenCoronaTestExtensions.kt @@ -17,14 +17,14 @@ import de.rki.coronawarnapp.coronatest.type.rapidantigen.SubmissionStateRAT.Test import de.rki.coronawarnapp.coronatest.type.rapidantigen.SubmissionStateRAT.TestPending import de.rki.coronawarnapp.coronatest.type.rapidantigen.SubmissionStateRAT.TestPositive import de.rki.coronawarnapp.coronatest.type.rapidantigen.SubmissionStateRAT.TestResultReady -import de.rki.coronawarnapp.exception.http.CwaServerError +import de.rki.coronawarnapp.exception.http.BadRequestException import org.joda.time.Instant fun RACoronaTest?.toSubmissionState(nowUTC: Instant = Instant.now(), coronaTestConfig: CoronaTestConfig) = when { this == null -> NoTest isSubmitted -> SubmissionDone(testRegisteredAt = registeredAt) isProcessing -> FetchingResult - lastError != null -> if (lastError is CwaServerError) TestPending else TestInvalid + lastError is BadRequestException -> TestInvalid else -> when (getState(nowUTC, coronaTestConfig)) { INVALID -> TestError POSITIVE -> { diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTestExtensionsTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTestExtensionsTest.kt index f1ea75919..8b3f39956 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTestExtensionsTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTestExtensionsTest.kt @@ -1,9 +1,14 @@ package de.rki.coronawarnapp.coronatest.type.pcr +import de.rki.coronawarnapp.coronatest.server.CoronaTestResult +import de.rki.coronawarnapp.exception.http.BadRequestException import io.kotest.matchers.shouldBe +import io.kotest.matchers.types.instanceOf import kotlinx.coroutines.test.runBlockingTest +import org.joda.time.Instant import org.junit.jupiter.api.Test import testhelpers.BaseTest +import java.net.SocketException class PCRCoronaTestExtensionsTest : BaseTest() { @@ -13,200 +18,30 @@ class PCRCoronaTestExtensionsTest : BaseTest() { test.toSubmissionState() shouldBe SubmissionStatePCR.NoTest } -// @Test -// fun removeTestFromDeviceSucceeds() = runBlockingTest { -// val submissionRepository = createInstance(scope = this) -// -// val initialPollingForTestResultTimeStampSlot = slot<Long>() -// val isTestResultAvailableNotificationSent = slot<Boolean>() -// every { -// tracingSettings.initialPollingForTestResultTimeStamp = capture(initialPollingForTestResultTimeStampSlot) -// } answers {} -// every { -// tracingSettings.isTestResultAvailableNotificationSent = capture(isTestResultAvailableNotificationSent) -// } answers {} -// -// every { submissionSettings.isAllowedToSubmitKeys = any() } just Runs -// every { submissionSettings.isSubmissionSuccessful = any() } just Runs -// -// submissionRepository.removeTestFromDevice() -// -// verify(exactly = 1) { -// testResultDataCollector.clear() -// registrationTokenPreference.update(any()) -// submissionSettings.devicePairingSuccessfulAt = null -// submissionSettings.initialTestResultReceivedAt = null -// submissionSettings.isAllowedToSubmitKeys = false -// submissionSettings.isSubmissionSuccessful = false -// } -// -// initialPollingForTestResultTimeStampSlot.captured shouldBe 0L -// isTestResultAvailableNotificationSent.captured shouldBe false -// } -// -// @Test -// fun registrationWithGUIDSucceeds() = runBlockingTest { -// coEvery { submissionService.asyncRegisterDeviceViaGUID(guid) } returns registrationData -// coEvery { analyticsKeySubmissionCollector.reportTestRegistered() } just Runs -// every { analyticsKeySubmissionCollector.reset() } just Runs -// -// val submissionRepository = createInstance(scope = this) -// -// submissionRepository.asyncRegisterDeviceViaGUID(guid) -// -// registrationTokenPreference.value shouldBe registrationToken -// submissionRepository.testResultReceivedDateFlow.first() shouldBe resultReceivedTimeStamp.toDate() -// -// verify(exactly = 1) { -// registrationTokenPreference.update(any()) -// submissionSettings.devicePairingSuccessfulAt = any() -// backgroundNoise.scheduleDummyPattern() -// } -// -// coVerify { testResultDataCollector.saveTestResultAnalyticsSettings(any()) } -// } -// -// @Test -// fun registrationWithTeleTANSucceeds() = runBlockingTest { -// coEvery { submissionService.asyncRegisterDeviceViaTAN(tan) } returns registrationData -// coEvery { analyticsKeySubmissionCollector.reportTestRegistered() } just Runs -// every { analyticsKeySubmissionCollector.reportRegisteredWithTeleTAN() } just Runs -// every { analyticsKeySubmissionCollector.reset() } just Runs -// -// val submissionRepository = createInstance(scope = this) -// -// submissionRepository.asyncRegisterDeviceViaTAN(tan) -// -// registrationTokenPreference.value shouldBe registrationToken -// submissionRepository.testResultReceivedDateFlow.first() shouldBe resultReceivedTimeStamp.toDate() -// -// verify(exactly = 1) { -// registrationTokenPreference.update(any()) -// submissionSettings.devicePairingSuccessfulAt = any() -// backgroundNoise.scheduleDummyPattern() -// } -// -// coVerify(exactly = 0) { -// testResultDataCollector.saveTestResultAnalyticsSettings(any()) -// } -// } -// -// @Test -// fun `reset clears tek history and settings`() = runBlockingTest { -// val instance = createInstance(this) -// instance.reset() -// -// instance.deviceUIStateFlow.first() shouldBe NetworkRequestWrapper.RequestIdle -// -// coVerifyOrder { -// tekHistoryStorage.clear() -// submissionSettings.clear() -// } -// } -// -// @Test -// fun `ui state is SUBMITTED_FINAL when submission was done`() = runBlockingTest { -// every { submissionSettings.isSubmissionSuccessful } returns true -// -// val submissionRepository = createInstance(scope = this) -// -// submissionRepository.refreshTest() -// submissionRepository.deviceUIStateFlow.first() shouldBe -// NetworkRequestWrapper.RequestSuccessful(DeviceUIState.SUBMITTED_FINAL) -// } -// -// @Test -// fun `ui state is UNPAIRED when no token is present`() = runBlockingTest { -// every { submissionSettings.isSubmissionSuccessful } returns false -// every { submissionSettings.registrationToken } returns mockFlowPreference(null) -// -// val submissionRepository = createInstance(scope = this) -// -// submissionRepository.refreshTest() -// submissionRepository.deviceUIStateFlow.first() shouldBe -// NetworkRequestWrapper.RequestSuccessful(DeviceUIState.UNPAIRED) -// } -// -// @Test -// fun `ui state is PAIRED_POSITIVE when allowed to submit`() = runBlockingTest { -// every { submissionSettings.isSubmissionSuccessful } returns false -// every { submissionSettings.registrationToken } returns mockFlowPreference("token") -// coEvery { submissionSettings.isAllowedToSubmitKeys } returns true -// -// val submissionRepository = createInstance(scope = this) -// -// submissionRepository.refreshTest() -// submissionRepository.deviceUIStateFlow.first() shouldBe -// NetworkRequestWrapper.RequestSuccessful(DeviceUIState.PAIRED_POSITIVE) -// } -// -// @Test -// fun `refresh when state is PAIRED_NO_RESULT`() = runBlockingTest { -// every { submissionSettings.isSubmissionSuccessful } returns false -// every { submissionSettings.registrationToken } returns mockFlowPreference("token") -// coEvery { submissionSettings.isAllowedToSubmitKeys } returns false -// coEvery { submissionService.asyncRequestTestResult(any()) } returns CoronaTestResult.PCR_OR_RAT_PENDING -// -// val submissionRepository = createInstance(scope = this) -// -// submissionRepository.refreshTest() -// submissionRepository.deviceUIStateFlow.first() shouldBe -// NetworkRequestWrapper.RequestSuccessful(DeviceUIState.PAIRED_NO_RESULT) -// -// coVerify(exactly = 1) { submissionService.asyncRequestTestResult(any()) } -// } -// -// @Test -// fun `refresh when state is UNPAIRED`() = runBlockingTest { -// every { submissionSettings.isSubmissionSuccessful } returns false -// every { submissionSettings.registrationToken } returns mockFlowPreference(null) -// coEvery { submissionSettings.isAllowedToSubmitKeys } returns false -// coEvery { submissionService.asyncRequestTestResult(any()) } returns CoronaTestResult.PCR_OR_RAT_PENDING -// -// val submissionRepository = createInstance(scope = this) -// -// submissionRepository.refreshTest() -// submissionRepository.deviceUIStateFlow.first() shouldBe -// NetworkRequestWrapper.RequestSuccessful(DeviceUIState.UNPAIRED) -// -// every { submissionSettings.registrationToken } returns mockFlowPreference("token") -// -// submissionRepository.refreshTest() -// -// coVerify(exactly = 1) { submissionService.asyncRequestTestResult(any()) } -// } -// -// @Test -// fun `no refresh when state is SUBMITTED_FINAL`() = runBlockingTest { -// every { submissionSettings.isSubmissionSuccessful } returns true -// -// val submissionRepository = createInstance(scope = this) -// -// submissionRepository.refreshTest() -// -// submissionRepository.deviceUIStateFlow.first() shouldBe -// NetworkRequestWrapper.RequestSuccessful(DeviceUIState.SUBMITTED_FINAL) -// -// submissionRepository.refreshTest() -// -// coVerify(exactly = 0) { submissionService.asyncRequestTestResult(any()) } -// } -// @Test -// fun `updateTestResult updates test result donor data`() = runBlockingTest { -// val submissionRepository = createInstance(scope = this) -// submissionRepository.updateTestResult(CoronaTestResult.PCR_NEGATIVE) -// -// verify { testResultDataCollector.updatePendingTestResultReceivedTime(any()) } -// } + // EXPOSUREAPP-6784 / https://github.com/corona-warn-app/cwa-app-android/issues/2953 + @Test + fun `non http 400 errors do not affect result state`() = runBlockingTest { + val test = PCRCoronaTest( + identifier = "identifier", + registeredAt = Instant.ofEpochMilli(123), + registrationToken = "regtoken", + testResult = CoronaTestResult.PCR_POSITIVE, + lastUpdatedAt = Instant.EPOCH, + lastError = SocketException("Connection reset") + ) + test.toSubmissionState() shouldBe instanceOf(SubmissionStatePCR.TestResultReady::class) + } -// @Test -// fun `doDeviceRegistration calls TestResultDataCollector`() { -// val viewModel = createViewModel() -// val mockResult = mockk<QRScanResult>().apply { -// every { guid } returns "guid" -// } -// -// coEvery { submissionRepository.asyncRegisterDeviceViaGUID(any()) } returns CoronaTestResult.PCR_POSITIVE -// viewModel.doDeviceRegistration(mockResult) -// } + @Test + fun `client HTTP400 errors result in invalid test state`() = runBlockingTest { + val test = PCRCoronaTest( + identifier = "identifier", + registeredAt = Instant.ofEpochMilli(123), + registrationToken = "regtoken", + testResult = CoronaTestResult.PCR_POSITIVE, + lastUpdatedAt = Instant.EPOCH, + lastError = BadRequestException("") + ) + test.toSubmissionState() shouldBe instanceOf(SubmissionStatePCR.TestInvalid::class) + } } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenCoronaTestExtensionsTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenCoronaTestExtensionsTest.kt index 9588466ba..e4ce30b84 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenCoronaTestExtensionsTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RapidAntigenCoronaTestExtensionsTest.kt @@ -2,8 +2,10 @@ package de.rki.coronawarnapp.coronatest.type.rapidantigen import de.rki.coronawarnapp.appconfig.CoronaTestConfig import de.rki.coronawarnapp.coronatest.server.CoronaTestResult +import de.rki.coronawarnapp.exception.http.BadRequestException import de.rki.coronawarnapp.util.TimeStamper import io.kotest.matchers.shouldBe +import io.kotest.matchers.types.instanceOf import io.mockk.MockKAnnotations import io.mockk.every import io.mockk.impl.annotations.MockK @@ -13,6 +15,7 @@ import org.joda.time.Instant import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import testhelpers.BaseTest +import java.net.SocketException class RapidAntigenCoronaTestExtensionsTest : BaseTest() { @MockK lateinit var coronaTestConfig: CoronaTestConfig @@ -51,4 +54,45 @@ class RapidAntigenCoronaTestExtensionsTest : BaseTest() { testRegisteredAt = Instant.ofEpochMilli(123) ) } + + // EXPOSUREAPP-6784 / https://github.com/corona-warn-app/cwa-app-android/issues/2953 + @Test + fun `errors that are not http 400 do not affect result state`() = runBlockingTest { + val test = RACoronaTest( + identifier = "identifier", + registeredAt = Instant.ofEpochMilli(123), + registrationToken = "regtoken", + testResult = CoronaTestResult.RAT_POSITIVE, + testedAt = Instant.EPOCH, + dateOfBirth = null, + firstName = null, + lastName = null, + lastUpdatedAt = Instant.EPOCH, + lastError = SocketException("Connection reset") + ) + test.toSubmissionState( + timeStamper.nowUTC, + coronaTestConfig + ) shouldBe instanceOf(SubmissionStateRAT.TestResultReady::class) + } + + @Test + fun `client http 400 errors result in invalid test state`() = runBlockingTest { + val test = RACoronaTest( + identifier = "identifier", + registeredAt = Instant.ofEpochMilli(123), + registrationToken = "regtoken", + testResult = CoronaTestResult.RAT_POSITIVE, + testedAt = Instant.EPOCH, + dateOfBirth = null, + firstName = null, + lastName = null, + lastUpdatedAt = Instant.EPOCH, + lastError = BadRequestException("") + ) + test.toSubmissionState( + timeStamper.nowUTC, + coronaTestConfig + ) shouldBe instanceOf(SubmissionStateRAT.TestInvalid::class) + } } -- GitLab