Fix data donation key submission (EXPOSUREAPP-5814) (#2626)

* move logic to collector

* reset before new registration

* more tests

* fix test

* fix calculation for missing values and more tests

* fix test

* remove max

* use -1 as default if value cannot be calculated

* set default to -1

* change date in tests

* fix reset

Co-authored-by: Lukas Lechner <lukas.lechner@sap.com>

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/common/Calculations.kt

@@ -8,8 +8,9 @@ fun calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration(
     lastChangeCheckedRiskLevelTimestamp: Instant?,
     testRegisteredAt: Instant?
 ): Int {
-    val lastChangeCheckedRiskLevelDate = lastChangeCheckedRiskLevelTimestamp?.toLocalDate() ?: return 0
-    val testRegisteredAtDate = testRegisteredAt?.toLocalDate() ?: return 0
+    val lastChangeCheckedRiskLevelDate = lastChangeCheckedRiskLevelTimestamp?.toLocalDate() ?: return -1
+    val testRegisteredAtDate = testRegisteredAt?.toLocalDate() ?: return -1
+    if (lastChangeCheckedRiskLevelDate.isAfter(testRegisteredAtDate)) return -1
     return Days.daysBetween(
         lastChangeCheckedRiskLevelDate,
         testRegisteredAtDate

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/keysubmission/AnalyticsKeySubmissionCollector.kt

 package de.rki.coronawarnapp.datadonation.analytics.modules.keysubmission
 
+import de.rki.coronawarnapp.datadonation.analytics.common.calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration
 import de.rki.coronawarnapp.datadonation.analytics.common.toMetadataRiskLevel
 import de.rki.coronawarnapp.datadonation.analytics.storage.AnalyticsSettings
 import de.rki.coronawarnapp.risk.RiskLevelSettings
@@ -54,6 +55,13 @@ class AnalyticsKeySubmissionCollector @Inject constructor(
                 }
             }
         }
+
+        analyticsKeySubmissionStorage.daysSinceMostRecentDateAtRiskLevelAtTestRegistration.update {
+            calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration(
+                riskLevelSettings.lastChangeCheckedRiskLevelTimestamp,
+                testRegisteredAt
+            )
+        }
     }
 
     fun reportSubmitted() {

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/keysubmission/AnalyticsKeySubmissionRepository.kt

 package de.rki.coronawarnapp.datadonation.analytics.modules.keysubmission
 
-import de.rki.coronawarnapp.datadonation.analytics.common.calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration
-import de.rki.coronawarnapp.risk.RiskLevelSettings
 import org.joda.time.Duration
-import org.joda.time.Instant
 import javax.inject.Inject
-import kotlin.math.max
 
 class AnalyticsKeySubmissionRepository @Inject constructor(
-    private val storage: AnalyticsKeySubmissionStorage,
-    private val riskLevelSettings: RiskLevelSettings
+    private val storage: AnalyticsKeySubmissionStorage
 ) {
     val testResultReceivedAt: Long
         get() = storage.testResultReceivedAt.value
@@ -42,16 +37,23 @@ class AnalyticsKeySubmissionRepository @Inject constructor(
         get() = storage.advancedConsentGiven.value
 
     val hoursSinceTestResult: Int
-        get() = Duration.millis(max(submittedAt - testResultReceivedAt, 0)).toStandardHours().hours
+        get() {
+            if (submittedAt <= 0) return -1
+            if (testResultReceivedAt <= 0) return -1
+            if (submittedAt < testResultReceivedAt) return -1
+            return Duration.millis(submittedAt - testResultReceivedAt).toStandardHours().hours
+        }
 
     val hoursSinceTestRegistration: Int
-        get() = Duration.millis(max(submittedAt - testRegisteredAt, 0L)).toStandardHours().hours
+        get() {
+            if (submittedAt <= 0) return -1
+            if (testRegisteredAt <= 0) return -1
+            if (submittedAt < testRegisteredAt) return -1
+            return Duration.millis(submittedAt - testRegisteredAt).toStandardHours().hours
+        }
 
     val daysSinceMostRecentDateAtRiskLevelAtTestRegistration: Int
-        get() = calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration(
-            riskLevelSettings.lastChangeCheckedRiskLevelTimestamp,
-            Instant.ofEpochMilli(testRegisteredAt)
-        )
+        get() = storage.daysSinceMostRecentDateAtRiskLevelAtTestRegistration.value
 
     val hoursSinceHighRiskWarningAtTestRegistration: Int
         get() = storage.hoursSinceHighRiskWarningAtTestRegistration.value

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/keysubmission/AnalyticsKeySubmissionStorage.kt

@@ -73,6 +73,11 @@ class AnalyticsKeySubmissionStorage @Inject constructor(
         defaultValue = -1
     )
 
+    val daysSinceMostRecentDateAtRiskLevelAtTestRegistration = prefs.createFlowPreference(
+        key = "analytics_key_submission_daysSinceMostRecentDateAtRiskLevelAtTestRegistration",
+        defaultValue = -1
+    )
+
     fun clear() {
         prefs.clearAndNotify()
     }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/submission/SubmissionRepository.kt

@@ -124,6 +124,7 @@ class SubmissionRepository @Inject constructor(
     }
 
     suspend fun asyncRegisterDeviceViaTAN(tan: String) {
+        analyticsKeySubmissionCollector.reset()
         val registrationData = submissionService.asyncRegisterDeviceViaTAN(tan)
         submissionSettings.registrationToken.update {
             registrationData.registrationToken
@@ -136,6 +137,7 @@ class SubmissionRepository @Inject constructor(
     }
 
     suspend fun asyncRegisterDeviceViaGUID(guid: String): TestResult {
+        analyticsKeySubmissionCollector.reset()
         val registrationData = submissionService.asyncRegisterDeviceViaGUID(guid)
         submissionSettings.registrationToken.update {
             registrationData.registrationToken
@@ -188,7 +190,6 @@ class SubmissionRepository @Inject constructor(
     fun removeTestFromDevice() {
         submissionSettings.hasViewedTestResult.update { false }
         submissionSettings.hasGivenConsent.update { false }
-        analyticsKeySubmissionCollector.reset()
         revokeConsentToSubmission()
         submissionSettings.registrationToken.update { null }
         submissionSettings.devicePairingSuccessfulAt = null

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/common/PpaDataExtensionsTest.kt

@@ -2,6 +2,7 @@ package de.rki.coronawarnapp.datadonation.analytics.common
 
 import de.rki.coronawarnapp.server.protocols.internal.ppdd.PpaData
 import io.kotest.matchers.shouldBe
+import org.joda.time.Instant
 import org.junit.jupiter.api.Test
 import testhelpers.BaseTest
 
@@ -26,4 +27,60 @@ class PpaDataExtensionsTest : BaseTest() {
         PpaData.PPAFederalState.FEDERAL_STATE_SH.federalStateShortName shouldBe "SH"
         PpaData.PPAFederalState.FEDERAL_STATE_TH.federalStateShortName shouldBe "TH"
     }
+
+    @Test
+    fun `days since most recent date at risk level at test registration are calculated correctly`() {
+        val march15At2200 = Instant.parse("2021-03-15T22:00:00.000Z")
+        val march17At0500 = Instant.parse("2021-03-17T05:00:00.000Z")
+        calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration(
+            lastChangeCheckedRiskLevelTimestamp = march15At2200,
+            testRegisteredAt = march17At0500
+        ) shouldBe 2
+    }
+
+    @Test
+    fun `days between most recent risk level change and test registration should be 0 if on same day`() {
+        val march15At0500 = Instant.parse("2021-03-15T05:00:00.000Z")
+        val march15At2200 = Instant.parse("2021-03-15T22:00:00.000Z")
+        calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration(
+            lastChangeCheckedRiskLevelTimestamp = march15At0500,
+            testRegisteredAt = march15At2200
+        ) shouldBe 0
+    }
+
+    @Test
+    fun `days should be -1 if lastChangeCheckedRiskLevelTimestamp is null`() {
+        val march15At0500 = Instant.parse("2021-03-15T05:00:00.000Z")
+        calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration(
+            lastChangeCheckedRiskLevelTimestamp = null,
+            testRegisteredAt = march15At0500
+        ) shouldBe -1
+    }
+
+    @Test
+    fun `days should be -1 if testRegisteredAt is null`() {
+        val march15At0500 = Instant.parse("2021-03-15T05:00:00.000Z")
+        calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration(
+            lastChangeCheckedRiskLevelTimestamp = march15At0500,
+            testRegisteredAt = null
+        ) shouldBe -1
+    }
+
+    @Test
+    fun `days should be -1 if both are null`() {
+        calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration(
+            lastChangeCheckedRiskLevelTimestamp = null,
+            testRegisteredAt = null
+        ) shouldBe -1
+    }
+
+    @Test
+    fun `days should be -1 if order is reversed`() {
+        val march20At2200 = Instant.parse("2021-03-20T22:00:00.000Z")
+        val march10At0500 = Instant.parse("2021-03-10T05:00:00.000Z")
+        calculateDaysSinceMostRecentDateAtRiskLevelAtTestRegistration(
+            lastChangeCheckedRiskLevelTimestamp = march20At2200,
+            testRegisteredAt = march10At0500
+        ) shouldBe -1
+    }
 }

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/keysubmission/AnalyticsKeySubmissionCollectorTest.kt

@@ -7,14 +7,18 @@ import de.rki.coronawarnapp.risk.RiskState
 import de.rki.coronawarnapp.risk.storage.RiskLevelStorage
 import de.rki.coronawarnapp.util.TimeStamper
 import io.mockk.MockKAnnotations
+import io.mockk.Runs
 import io.mockk.coEvery
 import io.mockk.every
 import io.mockk.impl.annotations.MockK
+import io.mockk.just
 import io.mockk.verify
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.runBlockingTest
+import org.joda.time.Days
 import org.joda.time.Hours
 import org.joda.time.Instant
+import org.joda.time.LocalTime
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import testhelpers.BaseTest
@@ -53,13 +57,24 @@ class AnalyticsKeySubmissionCollectorTest : BaseTest() {
         val riskLevelAtTestRegistration = mockFlowPreference(-1)
         every { analyticsKeySubmissionStorage.riskLevelAtTestRegistration } returns riskLevelAtTestRegistration
         val hoursSinceHighRiskWarningAtTestRegistration = mockFlowPreference(-1)
-        every { analyticsKeySubmissionStorage.hoursSinceHighRiskWarningAtTestRegistration } returns hoursSinceHighRiskWarningAtTestRegistration
+        every { analyticsKeySubmissionStorage.hoursSinceHighRiskWarningAtTestRegistration } returns
+            hoursSinceHighRiskWarningAtTestRegistration
+        coEvery {
+            riskLevelSettings.lastChangeCheckedRiskLevelTimestamp
+        } returns now
+            .minus(Days.days(2).toStandardDuration()).toDateTime().toLocalDate()
+            .toDateTime(LocalTime(22, 0)).toInstant()
+        val daysSinceMostRecentDateAtRiskLevelAtTestRegistration = mockFlowPreference(0)
+        every { analyticsKeySubmissionStorage.daysSinceMostRecentDateAtRiskLevelAtTestRegistration } returns
+            daysSinceMostRecentDateAtRiskLevelAtTestRegistration
+        every { analyticsKeySubmissionStorage.clear() } just Runs
         runBlockingTest {
             val collector = createInstance()
             collector.reportTestRegistered()
             verify { testRegisteredAt.update(any()) }
             verify { riskLevelAtTestRegistration.update(any()) }
             verify { hoursSinceHighRiskWarningAtTestRegistration.update(any()) }
+            verify { daysSinceMostRecentDateAtRiskLevelAtTestRegistration.update(any()) }
         }
     }
 
@@ -119,6 +134,7 @@ class AnalyticsKeySubmissionCollectorTest : BaseTest() {
         coEvery { analyticsSettings.analyticsEnabled.value } returns true
         val flow = mockFlowPreference(now.millis)
         every { analyticsKeySubmissionStorage.testResultReceivedAt } returns flow
+
         runBlockingTest {
             val collector = createInstance()
             collector.reportPositiveTestResultReceived()

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/keysubmission/AnalyticsKeySubmissionRepositoryTest.kt

 package de.rki.coronawarnapp.datadonation.analytics.modules.keysubmission
 
-import de.rki.coronawarnapp.risk.RiskLevelSettings
 import io.kotest.matchers.shouldBe
 import io.mockk.MockKAnnotations
 import io.mockk.coEvery
 import io.mockk.impl.annotations.MockK
-import org.joda.time.Days
 import org.joda.time.Hours
 import org.joda.time.Instant
-import org.joda.time.LocalTime
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import testhelpers.BaseTest
@@ -16,7 +13,6 @@ import testhelpers.BaseTest
 class AnalyticsKeySubmissionRepositoryTest : BaseTest() {
 
     @MockK lateinit var storage: AnalyticsKeySubmissionStorage
-    @MockK lateinit var riskLevelSettings: RiskLevelSettings
 
     private val now = Instant.now()
 
@@ -26,8 +22,7 @@ class AnalyticsKeySubmissionRepositoryTest : BaseTest() {
     }
 
     private fun createInstance() = AnalyticsKeySubmissionRepository(
-        storage,
-        riskLevelSettings
+        storage
     )
 
     @Test
@@ -39,27 +34,35 @@ class AnalyticsKeySubmissionRepositoryTest : BaseTest() {
     }
 
     @Test
-    fun `hours since test result when not submitted should be 0`() {
+    fun `hours since test result when not submitted should be -1`() {
         coEvery { storage.submittedAt.value } returns -1
         coEvery { storage.testResultReceivedAt.value } returns now.minus(Hours.hours(5).toStandardDuration()).millis
         val repository = createInstance()
-        repository.hoursSinceTestResult shouldBe 0
+        repository.hoursSinceTestResult shouldBe -1
     }
 
     @Test
-    fun `hours since test result when not received or submitted should be 0`() {
+    fun `hours since test result should be -1 when testResultReceivedAt is missing`() {
+        coEvery { storage.submittedAt.value } returns now.minus(Hours.hours(5).toStandardDuration()).millis
+        coEvery { storage.testResultReceivedAt.value } returns -1
+        val repository = createInstance()
+        repository.hoursSinceTestResult shouldBe -1
+    }
+
+    @Test
+    fun `hours since test result when not received or submitted should be -1`() {
         coEvery { storage.submittedAt.value } returns -1
         coEvery { storage.testResultReceivedAt.value } returns -1
         val repository = createInstance()
-        repository.hoursSinceTestResult shouldBe 0
+        repository.hoursSinceTestResult shouldBe -1
     }
 
     @Test
-    fun `hours since test result should be 0 when dates have been manipulated`() {
+    fun `hours since test result should be -1 when dates have been manipulated`() {
         coEvery { storage.submittedAt.value } returns now.minus(Hours.hours(5).toStandardDuration()).millis
         coEvery { storage.testResultReceivedAt.value } returns now.millis
         val repository = createInstance()
-        repository.hoursSinceTestResult shouldBe 0
+        repository.hoursSinceTestResult shouldBe -1
     }
 
     @Test
@@ -71,47 +74,18 @@ class AnalyticsKeySubmissionRepositoryTest : BaseTest() {
     }
 
     @Test
-    fun `hours since test registration should be 0 if not submitted`() {
+    fun `hours since test registration should be -1 if not submitted`() {
         coEvery { storage.submittedAt.value } returns -1
         coEvery { storage.testRegisteredAt.value } returns now.minus(Hours.hours(5).toStandardDuration()).millis
         val repository = createInstance()
-        repository.hoursSinceTestRegistration shouldBe 0
+        repository.hoursSinceTestRegistration shouldBe -1
     }
 
     @Test
-    fun `days since most recent date at risk level at test registration are calculated correctly`() {
-        coEvery {
-            riskLevelSettings.lastChangeCheckedRiskLevelTimestamp
-        } returns now
-            .minus(Days.days(2).toStandardDuration()).toDateTime().toLocalDate()
-            .toDateTime(LocalTime(22, 0)).toInstant()
-        coEvery { storage.testRegisteredAt.value } returns
-            now.toDateTime().toLocalDate().toDateTime(LocalTime(5, 0)).millis
-        val repository = createInstance()
-        repository.daysSinceMostRecentDateAtRiskLevelAtTestRegistration shouldBe 2
-    }
-
-    @Test
-    fun `days between most recent risk level change and test registration should be 0 if on same day`() {
-        coEvery {
-            riskLevelSettings.lastChangeCheckedRiskLevelTimestamp
-        } returns now
-            .toDateTime().toLocalDate()
-            .toDateTime(LocalTime(13, 0)).toInstant()
-        coEvery { storage.testRegisteredAt.value } returns
-            now.toDateTime().toLocalDate().toDateTime(LocalTime(14, 0)).millis
-        val repository = createInstance()
-        repository.daysSinceMostRecentDateAtRiskLevelAtTestRegistration shouldBe 0
-    }
-
-    @Test
-    fun `days should be 0 if lastChangeCheckedRiskLevelTimestamp is null`() {
-        coEvery {
-            riskLevelSettings.lastChangeCheckedRiskLevelTimestamp
-        } returns null
-        coEvery { storage.testRegisteredAt.value } returns
-            now.toDateTime().toLocalDate().toDateTime(LocalTime(14, 0)).millis
+    fun `hours since test registration should be -1 if testRegisteredAt is missing`() {
+        coEvery { storage.submittedAt.value } returns now.minus(Hours.hours(5).toStandardDuration()).millis
+        coEvery { storage.testRegisteredAt.value } returns -1
         val repository = createInstance()
-        repository.daysSinceMostRecentDateAtRiskLevelAtTestRegistration shouldBe 0
+        repository.hoursSinceTestRegistration shouldBe -1
     }
 }

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/storage/SubmissionRepositoryTest.kt

@@ -121,7 +121,6 @@ class SubmissionRepositoryTest : BaseTest() {
         every { submissionSettings.initialTestResultReceivedAt = any() } just Runs
         every { submissionSettings.isAllowedToSubmitKeys = any() } just Runs
         every { submissionSettings.isSubmissionSuccessful = any() } just Runs
-        every { analyticsKeySubmissionCollector.reset() } just Runs
 
         submissionRepository.removeTestFromDevice()
 
@@ -141,6 +140,7 @@ class SubmissionRepositoryTest : BaseTest() {
     fun registrationWithGUIDSucceeds() = runBlockingTest {
         coEvery { submissionService.asyncRegisterDeviceViaGUID(guid) } returns registrationData
         coEvery { analyticsKeySubmissionCollector.reportTestRegistered() } just Runs
+        every { analyticsKeySubmissionCollector.reset() } just Runs
 
         val submissionRepository = createInstance(scope = this)
 
@@ -161,6 +161,7 @@ class SubmissionRepositoryTest : BaseTest() {
         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)