From cd278162ec9991e3c35f59585acd6a784f8c247d Mon Sep 17 00:00:00 2001
From: Mohamed <mohamed.metwalli@sap.com>
Date: Tue, 2 Mar 2021 08:58:39 +0100
Subject: [PATCH] Fix race condition in test result donation (DEV) (#2489)
* Fix race condition in test result donation
* Make Klint happy
* Update docs
Co-authored-by: Ralf Gehrer <ralfgehrer@users.noreply.github.com>
---
.../registeredtest/TestResultDataCollector.kt | 8 +++++++
.../scan/SubmissionQRCodeScanViewModel.kt | 6 ++++--
.../TestResultDataCollectorTest.kt | 21 +++++++++++++++++++
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/registeredtest/TestResultDataCollector.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/registeredtest/TestResultDataCollector.kt
index 914bd7cc0..52b8be6d4 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/registeredtest/TestResultDataCollector.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/registeredtest/TestResultDataCollector.kt
@@ -19,6 +19,14 @@ class TestResultDataCollector @Inject constructor(
* exclude any registered test result before giving a consent
*/
suspend fun saveTestResultAnalyticsSettings(testResult: TestResult) {
+ val validTestResults = listOf(
+ TestResult.POSITIVE,
+ TestResult.PENDING,
+ TestResult.NEGATIVE
+ )
+
+ if (testResult !in validTestResults) return // Not interested in other values
+
if (analyticsSettings.analyticsEnabled.value) {
val lastRiskResult = riskLevelStorage
.latestAndLastSuccessful
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModel.kt
index e4087abac..250fa3998 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModel.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModel.kt
@@ -56,10 +56,12 @@ class SubmissionQRCodeScanViewModel @AssistedInject constructor(
try {
registrationState.postValue(RegistrationState(ApiRequestState.STARTED))
val testResult = submissionRepository.asyncRegisterDeviceViaGUID(scanResult.guid!!)
+ // Order here is important. When `registrationState.postValue` is called before
+ // `saveTestResultAnalyticsSettings`, this coroutine will get canceled due to the navigation
+ // to the next screen.
+ testResultDataCollector.saveTestResultAnalyticsSettings(testResult)
checkTestResult(testResult)
registrationState.postValue(RegistrationState(ApiRequestState.SUCCESS, testResult))
- // Order here is important. Save Analytics after SUCCESS
- testResultDataCollector.saveTestResultAnalyticsSettings(testResult)
} catch (err: CwaWebException) {
registrationState.postValue(RegistrationState(ApiRequestState.FAILED))
registrationError.postValue(err)
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/registeredtest/TestResultDataCollectorTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/registeredtest/TestResultDataCollectorTest.kt
index 104a2d06d..a0a8ba370 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/registeredtest/TestResultDataCollectorTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/registeredtest/TestResultDataCollectorTest.kt
@@ -5,6 +5,7 @@ import de.rki.coronawarnapp.datadonation.analytics.storage.TestResultDonorSettin
import de.rki.coronawarnapp.risk.RiskLevelResult
import de.rki.coronawarnapp.risk.storage.RiskLevelStorage
import de.rki.coronawarnapp.util.formatter.TestResult
+import io.mockk.Called
import io.mockk.MockKAnnotations
import io.mockk.Runs
import io.mockk.every
@@ -66,4 +67,24 @@ class TestResultDataCollectorTest : BaseTest() {
testResultDonorSettings.saveTestResultDonorDataAtRegistration(any(), any())
}
}
+
+ @Test
+ fun `saveTestResultAnalyticsSettings does not save data when TestResult is INVALID`() = runBlockingTest {
+ every { analyticsSettings.analyticsEnabled } returns mockFlowPreference(false)
+ testResultDataCollector.saveTestResultAnalyticsSettings(TestResult.INVALID)
+
+ verify {
+ analyticsSettings.analyticsEnabled wasNot Called
+ }
+ }
+
+ @Test
+ fun `saveTestResultAnalyticsSettings does not save data when TestResult is REDEEMED`() = runBlockingTest {
+ every { analyticsSettings.analyticsEnabled } returns mockFlowPreference(false)
+ testResultDataCollector.saveTestResultAnalyticsSettings(TestResult.REDEEMED)
+
+ verify {
+ analyticsSettings.analyticsEnabled wasNot Called
+ }
+ }
}
--
GitLab