Only use check-ins of last 15 days (EXPOSUREAPP-5910)

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepository.kt

@@ -3,6 +3,7 @@ package de.rki.coronawarnapp.eventregistration.checkins
 import de.rki.coronawarnapp.eventregistration.storage.TraceLocationDatabase
 import de.rki.coronawarnapp.eventregistration.storage.dao.CheckInDao
 import de.rki.coronawarnapp.eventregistration.storage.entity.toCheckIn
+import de.rki.coronawarnapp.util.TimeStamper
 import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.map
@@ -13,7 +14,8 @@ import javax.inject.Singleton
 
 @Singleton
 class CheckInRepository @Inject constructor(
-    traceLocationDatabaseFactory: TraceLocationDatabase.Factory
+    traceLocationDatabaseFactory: TraceLocationDatabase.Factory,
+    private val timeStamper: TimeStamper
 ) {
 
     private val traceLocationDatabase: TraceLocationDatabase by lazy {
@@ -24,10 +26,27 @@ class CheckInRepository @Inject constructor(
         traceLocationDatabase.eventCheckInDao()
     }
 
+    /**
+     * Returns all stored check-ins
+     *
+     * Attention: this could also include check-ins that are older than
+     * the retention period. Therefore, you should probably use [checkInsWithinRetention]
+     */
     val allCheckIns: Flow<List<CheckIn>> = checkInDao
         .allEntries()
         .map { list -> list.map { it.toCheckIn() } }
 
+    /**
+     * Returns check-ins that are within the retention period. Even though we have a worker that deletes all stale
+     * check-ins it's still possible to have stale check-ins in the database because the worker only runs once a day.
+     */
+    val checkInsWithinRetention: Flow<List<CheckIn>> = allCheckIns.map { checkInList ->
+        val now = timeStamper.nowUTC
+        checkInList.filter { checkIn ->
+            checkIn.isWithinRetention(now)
+        }
+    }
+
     suspend fun getCheckInById(checkInId: Long): CheckIn? {
         Timber.d("getCheckInById(checkInId=$checkInId)")
         return checkInDao.entryForId(checkInId)?.toCheckIn()

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRetention.kt

@@ -0,0 +1,21 @@
+package de.rki.coronawarnapp.eventregistration.checkins
+
+import de.rki.coronawarnapp.util.TimeAndDateExtensions.seconds
+import org.joda.time.Instant
+import java.util.concurrent.TimeUnit
+
+private const val CHECK_IN_RETENTION_DAYS = 15
+private val CHECK_IN_RETENTION_SECONDS = TimeUnit.DAYS.toSeconds(CHECK_IN_RETENTION_DAYS.toLong())
+
+/**
+ * returns true if the end date of the check-in isn't older than [CHECK_IN_RETENTION_DAYS], otherwise false
+ */
+fun CheckIn.isWithinRetention(now: Instant): Boolean {
+    val retentionThreshold = (now.seconds - CHECK_IN_RETENTION_SECONDS)
+    return checkInEnd.seconds >= retentionThreshold
+}
+
+/**
+ * Returns true if a check-in is stale and therefore can be deleted, otherwise false
+ */
+fun CheckIn.isOutOfRetention(now: Instant) = !isWithinRetention(now)

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/repo/DefaultTraceLocationRepository.kt

@@ -6,6 +6,8 @@ import de.rki.coronawarnapp.eventregistration.checkins.qrcode.toTraceLocations
 import de.rki.coronawarnapp.eventregistration.storage.TraceLocationDatabase
 import de.rki.coronawarnapp.eventregistration.storage.dao.TraceLocationDao
 import de.rki.coronawarnapp.eventregistration.storage.entity.toTraceLocationEntity
+import de.rki.coronawarnapp.eventregistration.storage.retention.isWithinRetention
+import de.rki.coronawarnapp.util.TimeStamper
 import de.rki.coronawarnapp.util.coroutine.AppScope
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.Flow
@@ -18,7 +20,8 @@ import javax.inject.Singleton
 @Singleton
 class DefaultTraceLocationRepository @Inject constructor(
     traceLocationDatabaseFactory: TraceLocationDatabase.Factory,
-    @AppScope private val appScope: CoroutineScope
+    @AppScope private val appScope: CoroutineScope,
+    private val timeStamper: TimeStamper
 ) : TraceLocationRepository {
 
     private val traceLocationDatabase: TraceLocationDatabase by lazy {
@@ -36,9 +39,28 @@ class DefaultTraceLocationRepository @Inject constructor(
         return checkIn.toTraceLocation()
     }
 
+    /**
+     * Returns all stored trace locations
+     *
+     * Attention: this could also include trace locations that are older than
+     * the retention period. Therefore, you should probably use [traceLocationsWithinRetention]
+     */
     override val allTraceLocations: Flow<List<TraceLocation>>
         get() = traceLocationDao.allEntries().map { it.toTraceLocations() }
 
+    /**
+     * Returns trace locations that are within the retention period. Even though we have a worker that deletes all stale
+     * trace locations it's still possible to have stale trace-locations in the database because the worker only runs
+     * once a day.
+     */
+    override val traceLocationsWithinRetention: Flow<List<TraceLocation>>
+        get() = allTraceLocations.map { traceLocationList ->
+            val now = timeStamper.nowUTC
+            traceLocationList.filter { traceLocation ->
+                traceLocation.isWithinRetention(now)
+            }
+        }
+
     override suspend fun addTraceLocation(traceLocation: TraceLocation): TraceLocation {
         Timber.d("Add trace location: %s", traceLocation)
         val traceLocationEntity = traceLocation.toTraceLocationEntity()

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/repo/TraceLocationRepository.kt

@@ -5,8 +5,21 @@ import kotlinx.coroutines.flow.Flow
 
 interface TraceLocationRepository {
 
+    /**
+     * Returns all stored trace locations
+     *
+     * Attention: this could also include trace locations that are older than
+     * the retention period. Therefore, you should probably use [traceLocationsWithinRetention]
+     */
     val allTraceLocations: Flow<List<TraceLocation>>
 
+    /**
+     * Returns trace locations that are within the retention period. Even though we have a worker that deletes all stale
+     * trace locations it's still possible to have stale trace-locations in the database because the worker only runs
+     * once a day.
+     */
+    val traceLocationsWithinRetention: Flow<List<TraceLocation>>
+
     suspend fun traceLocationForId(id: Long): TraceLocation
 
     suspend fun addTraceLocation(traceLocation: TraceLocation): TraceLocation

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/retention/CheckInCleaner.kt

@@ -2,11 +2,10 @@ package de.rki.coronawarnapp.eventregistration.storage.retention
 
 import dagger.Reusable
 import de.rki.coronawarnapp.eventregistration.checkins.CheckInRepository
-import de.rki.coronawarnapp.util.TimeAndDateExtensions.seconds
+import de.rki.coronawarnapp.eventregistration.checkins.isOutOfRetention
 import de.rki.coronawarnapp.util.TimeStamper
 import kotlinx.coroutines.flow.first
 import timber.log.Timber
-import java.util.concurrent.TimeUnit
 import javax.inject.Inject
 
 @Reusable
@@ -17,18 +16,15 @@ class CheckInCleaner @Inject constructor(
 
     suspend fun cleanUp() {
         Timber.d("Starting to clean up stale check-ins.")
-        val retentionThreshold = (timeStamper.nowUTC.seconds - RETENTION_SECONDS)
+
+        val now = timeStamper.nowUTC
         val checkInsToDelete = checkInRepository.allCheckIns.first()
-            .filter {
-                it.checkInEnd.seconds < retentionThreshold
-            }
+            .filter { checkIn -> checkIn.isOutOfRetention(now) }
+
         Timber.d("Cleaning up ${checkInsToDelete.size} stale check-ins.")
+
         checkInRepository.deleteCheckIns(checkInsToDelete)
-        Timber.d("Clean up of stale check-ins completed.")
-    }
 
-    companion object {
-        private const val RETENTION_DAYS = 15
-        private val RETENTION_SECONDS = TimeUnit.DAYS.toSeconds(RETENTION_DAYS.toLong())
+        Timber.d("Clean up of stale check-ins completed.")
     }
 }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/retention/TraceLocationCleaner.kt

@@ -2,11 +2,9 @@ package de.rki.coronawarnapp.eventregistration.storage.retention
 
 import dagger.Reusable
 import de.rki.coronawarnapp.eventregistration.storage.repo.TraceLocationRepository
-import de.rki.coronawarnapp.util.TimeAndDateExtensions.seconds
 import de.rki.coronawarnapp.util.TimeStamper
 import kotlinx.coroutines.flow.first
 import timber.log.Timber
-import java.util.concurrent.TimeUnit
 import javax.inject.Inject
 
 @Reusable
@@ -17,19 +15,16 @@ class TraceLocationCleaner @Inject constructor(
 
     suspend fun cleanUp() {
         Timber.d("Starting to clean up stale trace locations.")
-        val retentionThreshold = (timeStamper.nowUTC.seconds - RETENTION_SECONDS)
+
+        val now = timeStamper.nowUTC
         traceLocationRepository.allTraceLocations.first()
-            .filter {
-                it.endDate != null && it.endDate.seconds < retentionThreshold
+            .filter { traceLocation ->
+                traceLocation.isOutOfRetention(now)
             }.forEach {
                 Timber.d("Cleaning up stale trace location: %s", it)
                 traceLocationRepository.deleteTraceLocation(it)
             }
-        Timber.d("Clean up of stale trace locations completed.")
-    }
 
-    companion object {
-        private const val RETENTION_DAYS = 15
-        private val RETENTION_SECONDS = TimeUnit.DAYS.toSeconds(RETENTION_DAYS.toLong())
+        Timber.d("Clean up of stale trace locations completed.")
     }
 }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/eventregistration/storage/retention/TraceLocationRetention.kt

@@ -0,0 +1,27 @@
+package de.rki.coronawarnapp.eventregistration.storage.retention
+
+import de.rki.coronawarnapp.eventregistration.checkins.qrcode.TraceLocation
+import de.rki.coronawarnapp.util.TimeAndDateExtensions.seconds
+import org.joda.time.Instant
+import java.util.concurrent.TimeUnit
+
+private const val TRACE_LOCATION_RETENTION_DAYS = 15
+private val TRACE_LOCATION_RETENTION_SECONDS = TimeUnit.DAYS.toSeconds(TRACE_LOCATION_RETENTION_DAYS.toLong())
+
+/**
+ * returns true if a trace location either has no end date or has an end date that isn't older than
+ * [TRACE_LOCATION_RETENTION_DAYS], otherwise false
+ */
+fun TraceLocation.isWithinRetention(now: Instant): Boolean {
+    val retentionThreshold = (now.seconds - TRACE_LOCATION_RETENTION_SECONDS)
+    return if (endDate == null) {
+        true
+    } else {
+        endDate.seconds >= retentionThreshold
+    }
+}
+
+/**
+ * Returns true if a trace location is stale and therefore can be deleted, otherwise false
+ */
+fun TraceLocation.isOutOfRetention(now: Instant) = !isWithinRetention(now)

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/risk/CheckInWarningMatcher.kt

@@ -31,7 +31,7 @@ class CheckInWarningMatcher @Inject constructor(
 
         presenceTracingRiskRepository.deleteStaleData()
 
-        val checkIns = checkInsRepository.allCheckIns.firstOrNull()
+        val checkIns = checkInsRepository.checkInsWithinRetention.firstOrNull()
         if (checkIns.isNullOrEmpty()) {
             Timber.i("No check-ins available. Deleting all matches.")
             presenceTracingRiskRepository.deleteAllMatches()

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

@@ -141,7 +141,7 @@ class SubmissionTask @Inject constructor(
         )
         Timber.tag(TAG).d("Transformed keys with symptoms %s from %s to %s", symptoms, keys, transformedKeys)
 
-        val checkIns = checkInsRepository.allCheckIns.first()
+        val checkIns = checkInsRepository.checkInsWithinRetention.first()
         val transformedCheckIns = checkInsTransformer.transform(checkIns, symptoms)
 
         Timber.tag(TAG).d("Transformed CheckIns from: %s to: %s", checkIns, transformedCheckIns)

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/eventregistration/attendee/checkins/CheckInsViewModel.kt

@@ -56,7 +56,7 @@ class CheckInsViewModel @AssistedInject constructor(
 
     val checkins: LiveData<List<CheckInsItem>> = combine(
         intervalFlow(1000),
-        checkInsRepository.allCheckIns,
+        checkInsRepository.checkInsWithinRetention,
         cameraPermissionProvider.deniedPermanently
     ) { _, checkIns, denied ->
         mutableListOf<CheckInsItem>().apply {

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/eventregistration/organizer/list/TraceLocationsViewModel.kt

@@ -8,7 +8,6 @@ import de.rki.coronawarnapp.eventregistration.checkins.qrcode.TraceLocation
 import de.rki.coronawarnapp.eventregistration.storage.repo.TraceLocationRepository
 import de.rki.coronawarnapp.ui.eventregistration.organizer.list.items.TraceLocationItem
 import de.rki.coronawarnapp.ui.eventregistration.organizer.list.items.TraceLocationVH
-import de.rki.coronawarnapp.util.TimeStamper
 import de.rki.coronawarnapp.util.coroutine.DispatcherProvider
 import de.rki.coronawarnapp.util.ui.SingleLiveEvent
 import de.rki.coronawarnapp.util.viewmodel.CWAViewModel
@@ -17,17 +16,14 @@ import kotlinx.coroutines.flow.map
 
 class TraceLocationsViewModel @AssistedInject constructor(
     dispatcherProvider: DispatcherProvider,
-    private val traceLocationRepository: TraceLocationRepository,
-    private val timeStamper: TimeStamper
+    private val traceLocationRepository: TraceLocationRepository
 ) : CWAViewModel(dispatcherProvider = dispatcherProvider) {
 
     val events = SingleLiveEvent<TraceLocationEvent>()
 
-    val traceLocations: LiveData<List<TraceLocationItem>> = traceLocationRepository.allTraceLocations
+    val traceLocations: LiveData<List<TraceLocationItem>> = traceLocationRepository.traceLocationsWithinRetention
         .map { traceLocations ->
-            traceLocations
-                .filter { it.endDate == null || it.endDate.isAfter(timeStamper.nowUTC.toDateTime().minusDays(15)) }
-                .sortedBy { it.description }
+            traceLocations.sortedBy { it.description }
         }
         .map { traceLocations ->
             traceLocations.map { traceLocation ->

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivityViewModel.kt

@@ -42,7 +42,7 @@ class MainActivityViewModel @AssistedInject constructor(
     private val mutableIsTraceLocationOnboardingDone = MutableLiveData<Boolean>()
     val isTraceLocationOnboardingDone: LiveData<Boolean> = mutableIsTraceLocationOnboardingDone
 
-    val activeCheckIns = checkInRepository.allCheckIns
+    val activeCheckIns = checkInRepository.checkInsWithinRetention
         .map { checkins -> checkins.filter { !it.completed }.size }
         .asLiveData(context = dispatcherProvider.Default)
 

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/eventregistration/checkins/CheckInRepositoryTest.kt

@@ -3,18 +3,21 @@ package de.rki.coronawarnapp.eventregistration.checkins
 import de.rki.coronawarnapp.eventregistration.storage.TraceLocationDatabase
 import de.rki.coronawarnapp.eventregistration.storage.dao.CheckInDao
 import de.rki.coronawarnapp.eventregistration.storage.entity.TraceLocationCheckInEntity
+import de.rki.coronawarnapp.util.TimeStamper
 import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.matchers.shouldBe
 import io.mockk.MockKAnnotations
 import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.impl.annotations.MockK
+import io.mockk.impl.annotations.RelaxedMockK
 import io.mockk.mockk
 import io.mockk.slot
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.runBlockingTest
 import okio.ByteString.Companion.encode
 import org.joda.time.Instant
@@ -27,6 +30,7 @@ class CheckInRepositoryTest : BaseTest() {
     @MockK lateinit var factory: TraceLocationDatabase.Factory
     @MockK lateinit var database: TraceLocationDatabase
     @MockK lateinit var checkInDao: CheckInDao
+    @RelaxedMockK lateinit var timeStamper: TimeStamper
     private val allEntriesFlow = MutableStateFlow(emptyList<TraceLocationCheckInEntity>())
 
     @BeforeEach
@@ -40,7 +44,7 @@ class CheckInRepositoryTest : BaseTest() {
         }
     }
 
-    private fun createInstance(scope: CoroutineScope) = CheckInRepository(factory)
+    private fun createInstance(scope: CoroutineScope) = CheckInRepository(factory, timeStamper)
 
     @Test
     fun `new entities should have ID 0`() = runBlockingTest {
@@ -182,4 +186,46 @@ class CheckInRepositoryTest : BaseTest() {
             )
         }
     }
+
+    @Test
+    fun `checkInsWithinRetention() should filter out stale check-ins`() = runBlockingTest {
+
+        // Now = Jan 16th 2020, 00:00
+        // CheckIns should be kept for 15 days, so every check-in with an end date before
+        // Jan 1st 2020, 00:00 should get deleted
+        every { timeStamper.nowUTC } returns Instant.parse("2020-01-16T00:00:00.000Z")
+
+        val checkInWithinRetention = createCheckIn(Instant.parse("2020-01-01T00:00:00.000Z"))
+
+        // should be filtered out
+        val staleCheckIn = createCheckIn(Instant.parse("2019-12-31T23:59:59.000Z"))
+
+        every { checkInDao.allEntries() } returns flowOf(
+            listOf(
+                staleCheckIn.toEntity(),
+                checkInWithinRetention.toEntity()
+            )
+        )
+
+        createInstance(scope = this).checkInsWithinRetention.first() shouldBe
+            listOf(checkInWithinRetention)
+    }
+
+    private fun createCheckIn(checkOutDate: Instant) = CheckIn(
+        traceLocationId = "traceLocationId1".encode(),
+        traceLocationIdHash = "traceLocationIdHash1".encode(),
+        version = 1,
+        type = 1,
+        description = "",
+        address = "",
+        traceLocationStart = null,
+        traceLocationEnd = null,
+        defaultCheckInLengthInMinutes = 30,
+        cryptographicSeed = "cryptographicSeed".encode(),
+        cnPublicKey = "cnPublicKey",
+        checkInStart = Instant.parse("1970-01-01T00:00:00.000Z"),
+        checkInEnd = checkOutDate,
+        completed = true,
+        createJournalEntry = true
+    )
 }