Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
535e954
Only use check-ins of last 15 days
LukasLechnerDev Apr 6, 2021
66c085b
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 6, 2021
c0295a7
Refactor TraceLocation Retention
LukasLechnerDev Apr 6, 2021
6c37b5a
Fix SubmissionTaskTest.kt
LukasLechnerDev Apr 6, 2021
ffdf0d7
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 6, 2021
858d48a
Fix MainActivityViewModelTest.kt
LukasLechnerDev Apr 6, 2021
2a314d1
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
harambasicluka Apr 6, 2021
d780e25
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 7, 2021
1f40374
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
harambasicluka Apr 7, 2021
b672a83
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
mtwalli Apr 7, 2021
a9908ef
Address PR feedback
LukasLechnerDev Apr 7, 2021
7de7a6f
Add CheckInRetentionTest.kt and TraceLocationRetentionTest.kt
LukasLechnerDev Apr 7, 2021
c52b3f4
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 7, 2021
728ccc6
Add some comments
LukasLechnerDev Apr 7, 2021
c158c1a
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 7, 2021
4030702
Add more comments
LukasLechnerDev Apr 7, 2021
8c386c0
Merge remote-tracking branch 'origin/feature/5910-Only-use-checkins-o…
LukasLechnerDev Apr 7, 2021
19c5614
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 7, 2021
3cd9608
Fix tests
LukasLechnerDev Apr 7, 2021
b8bf44b
Load checkInsWithinRetention instead of allCheckIns in ContactDiary, …
LukasLechnerDev Apr 7, 2021
d476087
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 7, 2021
e347478
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 7, 2021
5ceecd3
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 8, 2021
b81ca2b
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 8, 2021
1035662
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 8, 2021
80564a7
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
harambasicluka Apr 8, 2021
830c234
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 8, 2021
576b1ec
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
LukasLechnerDev Apr 8, 2021
1895018
Merge branch 'release/2.0.x' into feature/5910-Only-use-checkins-of-l…
mtwalli Apr 8, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ class ContactDiaryOverviewViewModel @AssistedInject constructor(

private val riskLevelPerDateFlow = riskLevelStorage.ewDayRiskStates
private val traceLocationCheckInRiskFlow = riskLevelStorage.traceLocationCheckInRiskStates
private val allCheckInsFlow = checkInRepository.allCheckIns
private val checkInsWithinRetentionFlow = checkInRepository.checkInsWithinRetention

val listItems = combine(
flowOf(dates),
locationVisitsFlow,
personEncountersFlow,
riskLevelPerDateFlow,
traceLocationCheckInRiskFlow,
allCheckInsFlow
checkInsWithinRetentionFlow
) { dateList, locationVisists, personEncounters, riskLevelPerDateList, traceLocationCheckInRiskList, checkInList ->
mutableListOf<DiaryOverviewItem>().apply {
add(OverviewSubHeaderItem)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.")
}
}
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class PresenceTracingWarningTask @Inject constructor(

presenceTracingRiskRepository.deleteStaleData()

val checkIns = checkInsRepository.allCheckIns.firstOrNull() ?: emptyList()
val checkIns = checkInsRepository.checkInsWithinRetention.firstOrNull() ?: emptyList()
Timber.tag(TAG).d("There are %d check-ins to match against.", checkIns.size)

if (checkIns.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class TraceWarningPackageSyncTool @Inject constructor(
internal suspend fun syncPackagesForLocation(location: LocationCode): SyncResult {
Timber.tag(TAG).d("syncTraceWarningPackages(location=%s)", location)

val oldestCheckIn = checkInRepository.allCheckIns.first().minByOrNull { it.checkInStart }.also {
val oldestCheckIn = checkInRepository.checkInsWithinRetention.first().minByOrNull { it.checkInStart }.also {
Timber.tag(TAG).d("Our oldest check-in is %s", it)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,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
Expand All @@ -21,16 +20,13 @@ class TraceLocationsViewModel @AssistedInject constructor(
dispatcherProvider: DispatcherProvider,
checkInsRepository: CheckInRepository,
private val traceLocationRepository: TraceLocationRepository,
private val timeStamper: TimeStamper
) : 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 }
}
.combine(checkInsRepository.allCheckIns) { traceLocations, checkIns ->
traceLocations.map { traceLocation ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ open class ContactDiaryOverviewViewModelTest {
every { contactDiaryRepository.personEncounters } returns flowOf(emptyList())
every { riskLevelStorage.ewDayRiskStates } returns flowOf(emptyList())
every { riskLevelStorage.traceLocationCheckInRiskStates } returns flowOf(emptyList())
every { checkInRepository.allCheckIns } returns flowOf(emptyList())
every { checkInRepository.checkInsWithinRetention } returns flowOf(emptyList())

mockStringsForContactDiaryExporterTests(context)
every { timeStamper.nowUTC } returns Instant.ofEpochMilli(dateMillis)
Expand Down Expand Up @@ -361,7 +361,7 @@ open class ContactDiaryOverviewViewModelTest {
every { riskLevelStorage.ewDayRiskStates } returns flowOf(listOf(aggregatedRiskPerDateResultLowRisk))
every { riskLevelStorage.traceLocationCheckInRiskStates } returns flowOf(listOf(traceLocationCheckInRiskHigh))
every { contactDiaryRepository.locationVisits } returns flowOf(listOf(locationEventHighRiskVisit))
every { checkInRepository.allCheckIns } returns flowOf(listOf(checkInHigh))
every { checkInRepository.checkInsWithinRetention } returns flowOf(listOf(checkInHigh))

var item = createInstance().listItems.getOrAwaitValue().first {
it is DayOverviewItem && it.date == date
Expand All @@ -384,7 +384,7 @@ open class ContactDiaryOverviewViewModelTest {
)
every { riskLevelStorage.traceLocationCheckInRiskStates } returns flowOf(listOf(traceLocationCheckInRiskLow))
every { contactDiaryRepository.locationVisits } returns flowOf(listOf(locationEventLowRiskVisit))
every { checkInRepository.allCheckIns } returns flowOf(listOf(checkInLow))
every { checkInRepository.checkInsWithinRetention } returns flowOf(listOf(checkInLow))

item = createInstance().listItems.getOrAwaitValue().first {
it is DayOverviewItem && it.date == date
Expand Down Expand Up @@ -414,7 +414,7 @@ open class ContactDiaryOverviewViewModelTest {
fun `low risk event by attending event with low risk`() {
every { contactDiaryRepository.locationVisits } returns flowOf(listOf(locationEventLowRiskVisit))
every { riskLevelStorage.traceLocationCheckInRiskStates } returns flowOf(listOf(traceLocationCheckInRiskLow))
every { checkInRepository.allCheckIns } returns flowOf(listOf(checkInLow))
every { checkInRepository.checkInsWithinRetention } returns flowOf(listOf(checkInLow))

val item = createInstance().listItems.getOrAwaitValue().first {
it is DayOverviewItem && it.date == date
Expand All @@ -436,7 +436,7 @@ open class ContactDiaryOverviewViewModelTest {
fun `high risk event by attending event with high risk`() {
every { contactDiaryRepository.locationVisits } returns flowOf(listOf(locationEventHighRiskVisit))
every { riskLevelStorage.traceLocationCheckInRiskStates } returns flowOf(listOf(traceLocationCheckInRiskHigh))
every { checkInRepository.allCheckIns } returns flowOf(listOf(checkInHigh))
every { checkInRepository.checkInsWithinRetention } returns flowOf(listOf(checkInHigh))

val item = createInstance().listItems.getOrAwaitValue().first {
it is DayOverviewItem && it.date == date
Expand Down Expand Up @@ -469,7 +469,7 @@ open class ContactDiaryOverviewViewModelTest {
)
)

every { checkInRepository.allCheckIns } returns flowOf(listOf(checkInHigh, checkInLow))
every { checkInRepository.checkInsWithinRetention } returns flowOf(listOf(checkInHigh, checkInLow))

val item = createInstance().listItems.getOrAwaitValue().first {
it is DayOverviewItem && it.date == date
Expand Down
Loading