diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/ContactJournalCheckInEntryCreator.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/ContactJournalCheckInEntryCreator.kt index e553c983471c6c5c523868b2bf95a80a760bf070..ed1f0b0c0eb042105bb16987f3e1f907a6b68528 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/ContactJournalCheckInEntryCreator.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/ContactJournalCheckInEntryCreator.kt @@ -12,7 +12,6 @@ import de.rki.coronawarnapp.eventregistration.checkins.split.splitByMidnightUTC import de.rki.coronawarnapp.util.TimeAndDateExtensions.toLocalDateUtc import de.rki.coronawarnapp.util.TimeAndDateExtensions.toUserTimeZone import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.firstOrNull import org.joda.time.Duration import org.joda.time.Seconds import org.joda.time.format.DateTimeFormat @@ -29,8 +28,7 @@ class ContactJournalCheckInEntryCreator @Inject constructor( Timber.d("Creating journal entry for %s", checkIn) // 1. Create location if missing - val location: ContactDiaryLocation = diaryRepository.locations.first() - .find { it.traceLocationID == checkIn.traceLocationId } ?: checkIn.toLocation() + val location = checkIn.createLocationIfMissing() // 2. Split CheckIn by Midnight UTC val splitCheckIns = checkIn.splitByMidnightUTC() @@ -42,17 +40,24 @@ class ContactJournalCheckInEntryCreator @Inject constructor( .forEach { diaryRepository.addLocationVisit(it) } } - private suspend fun CheckIn.toLocation(): ContactDiaryLocation { + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + suspend fun CheckIn.createLocationIfMissing(): ContactDiaryLocation = diaryRepository.locations.first() + .find { it.traceLocationID == traceLocationId } ?: createLocationEntry() + + private suspend fun CheckIn.createLocationEntry(): ContactDiaryLocation { + Timber.d("Creating new contact diary location from %s", this) val location = DefaultContactDiaryLocation( - locationName = locationName(), + locationName = toLocationName(), traceLocationID = traceLocationId ) - Timber.d("Created new location %s and adding it to contact journal db", location) - return diaryRepository.addLocation(location) // Get location from db cause we need the id autogenerated by db + + // Get location from db cause we need the id autogenerated by db + return diaryRepository.addLocation(location) + .also { Timber.d("Created %s and added it to contact journal db", it) } } @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - fun CheckIn.locationName(): String { + fun CheckIn.toLocationName(): String { val nameParts = mutableListOf(description, address) if (traceLocationStart != null && traceLocationEnd != null) { @@ -66,7 +71,10 @@ class ContactJournalCheckInEntryCreator @Inject constructor( return nameParts.joinToString(separator = ", ") } - private fun CheckIn.toLocationVisit(location: ContactDiaryLocation): ContactDiaryLocationVisit { + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + fun CheckIn.toLocationVisit(location: ContactDiaryLocation): ContactDiaryLocationVisit { + // Duration column is set by calculating the time difference in minutes between Check-in StartDate + // and Check-in EndDate and rounding it to the closest 15-minute duration // Use Seconds for more precision val durationInMinutes = Seconds.secondsBetween(checkInStart, checkInEnd).seconds / 60.0 val duration = (durationInMinutes / 15).roundToLong() * 15 @@ -75,17 +83,18 @@ class ContactJournalCheckInEntryCreator @Inject constructor( contactDiaryLocation = location, duration = Duration.standardMinutes(duration), checkInID = id - ) + ).also { Timber.d("Created %s for %s", it, this) } } - private suspend fun List<CheckIn>.createMissingLocationVisits(location: ContactDiaryLocation): + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + suspend fun List<CheckIn>.createMissingLocationVisits(location: ContactDiaryLocation): List<ContactDiaryLocationVisit> { Timber.d( "createMissingLocationVisits(location=%s) for %s", location, this.joinToString(prefix = System.lineSeparator(), separator = System.lineSeparator()) ) - val existingLocationVisits = diaryRepository.locationVisits.firstOrNull() ?: emptyList() + val existingLocationVisits = diaryRepository.locationVisits.first() // Existing location visits shall not be updated, so just drop them return filter { existingLocationVisits.none { visit -> @@ -96,7 +105,7 @@ class ContactJournalCheckInEntryCreator @Inject constructor( .map { it.toLocationVisit(location) } .also { Timber.d( - "Created locations visits: %s", + "Created location visits: %s", it.joinToString(prefix = System.lineSeparator(), separator = System.lineSeparator()) ) } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/CheckOutHandlerTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/CheckOutHandlerTest.kt index 07007ff99eeed66a36d4c651f91d3a51c8904be0..2c593184e9dc277b021427063294662dbe1a2a51 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/CheckOutHandlerTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/CheckOutHandlerTest.kt @@ -97,7 +97,7 @@ class CheckOutHandlerTest : BaseTest() { checkOut(42) } - updatedCheckIn?.createJournalEntry shouldBe true + updatedCheckIn!!.createJournalEntry shouldBe true coVerify(exactly = 1) { contactJournalCheckInEntryCreator.createEntry(any()) @@ -110,7 +110,7 @@ class CheckOutHandlerTest : BaseTest() { checkOut(43) } - updatedCheckIn?.createJournalEntry shouldBe false + updatedCheckIn!!.createJournalEntry shouldBe false coVerify(exactly = 0) { contactJournalCheckInEntryCreator.createEntry(any()) diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/ContactJournalCheckInEntryCreatorTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/ContactJournalCheckInEntryCreatorTest.kt index 58834e6c7b70de6ac2f0e5a7452baabbd0c29e86..0726f259a2e346abaf851c492c5ccc0e4fe2d9fd 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/ContactJournalCheckInEntryCreatorTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/presencetracing/checkins/checkout/ContactJournalCheckInEntryCreatorTest.kt @@ -1,8 +1,12 @@ package de.rki.coronawarnapp.presencetracing.checkins.checkout import de.rki.coronawarnapp.contactdiary.model.DefaultContactDiaryLocation +import de.rki.coronawarnapp.contactdiary.model.DefaultContactDiaryLocationVisit import de.rki.coronawarnapp.contactdiary.storage.repo.ContactDiaryRepository import de.rki.coronawarnapp.eventregistration.checkins.CheckIn +import de.rki.coronawarnapp.util.TimeAndDateExtensions.toLocalDateUtc +import de.rki.coronawarnapp.util.TimeAndDateExtensions.toUserTimeZone +import io.kotest.matchers.shouldBe import io.mockk.MockKAnnotations import io.mockk.coEvery import io.mockk.coVerify @@ -12,8 +16,12 @@ import io.mockk.just import io.mockk.runs import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runBlockingTest +import okio.ByteString.Companion.decodeBase64 import okio.ByteString.Companion.encode +import org.joda.time.Days import org.joda.time.Instant +import org.joda.time.Minutes +import org.joda.time.format.DateTimeFormat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import testhelpers.BaseTest @@ -24,7 +32,7 @@ class ContactJournalCheckInEntryCreatorTest : BaseTest() { private val testCheckIn = CheckIn( id = 42L, - traceLocationId = "traceLocationId1".encode(), + traceLocationId = "traceLocationId1".decodeBase64()!!, version = 1, type = 1, description = "Restaurant", @@ -42,10 +50,20 @@ class ContactJournalCheckInEntryCreatorTest : BaseTest() { private val testLocation = DefaultContactDiaryLocation( locationId = 123L, - locationName = "${testCheckIn.description}, ${testCheckIn.address}, ${testCheckIn.traceLocationStart} - ${testCheckIn.traceLocationEnd}", + locationName = "${testCheckIn.description}, ${testCheckIn.address}, ${testCheckIn.traceLocationStart?.toPrettyDate()} - ${testCheckIn.traceLocationEnd?.toPrettyDate()}", traceLocationID = testCheckIn.traceLocationId ) + private val testLocationVisit = DefaultContactDiaryLocationVisit( + id = 0, + date = testCheckIn.checkInStart.toLocalDateUtc(), + contactDiaryLocation = testLocation, + checkInID = testCheckIn.id, + duration = Minutes.minutes(60).toStandardDuration() + ) + + private fun Instant.toPrettyDate(): String = toUserTimeZone().toString(DateTimeFormat.shortDateTime()) + @BeforeEach fun setup() { MockKAnnotations.init(this) @@ -68,20 +86,136 @@ class ContactJournalCheckInEntryCreatorTest : BaseTest() { every { contactDiaryRepo.locations } returns flowOf(emptyList()) andThen flowOf(listOf(testLocation)) // Repo returns an empty list for the first call, so location is missing and a new location should be created and added - val instance = createInstance() - instance.createEntry(testCheckIn) + createInstance().apply { + testCheckIn.createLocationIfMissing() + + coVerify(exactly = 1) { + contactDiaryRepo.addLocation(any()) + } + + // Location with trace location id already exists, so that location will be used + testCheckIn.createLocationIfMissing() + testCheckIn.createLocationIfMissing() + testCheckIn.createLocationIfMissing() + testCheckIn.createLocationIfMissing() + + coVerify(exactly = 1) { + contactDiaryRepo.addLocation(any()) + } + + testCheckIn.copy(traceLocationId = "traceLocationId2".decodeBase64()!!).createLocationIfMissing() - coVerify(exactly = 1) { - contactDiaryRepo.addLocation(any()) + coVerify(exactly = 2) { + contactDiaryRepo.addLocation(any()) + } } + } - // Location with trace location id already exists, so that location will be used - instance.createEntry(testCheckIn) - instance.createEntry(testCheckIn) - instance.createEntry(testCheckIn) + @Test + fun `Location name concatenates description, address and if both are set trace location start and end date`() { + val testCheckInNoTraceLocationStartDate = testCheckIn.copy(traceLocationStart = null) + val testCheckInNoTraceLocationEndDate = testCheckIn.copy(traceLocationEnd = null) + val testCheckInNoTraceLocationStartAndEndDate = + testCheckIn.copy(traceLocationStart = null, traceLocationEnd = null) + + createInstance().apply { + testCheckIn.validateLocationName(testCheckIn.toLocationName()) + testCheckInNoTraceLocationStartDate.validateLocationName(testCheckInNoTraceLocationStartDate.toLocationName()) + testCheckInNoTraceLocationEndDate.validateLocationName(testCheckInNoTraceLocationEndDate.toLocationName()) + testCheckInNoTraceLocationStartAndEndDate.validateLocationName(testCheckInNoTraceLocationStartAndEndDate.toLocationName()) + } + } + + private fun CheckIn.validateLocationName(nameToValidate: String) { + nameToValidate shouldBe when (traceLocationStart != null && traceLocationEnd != null) { + true -> "$description, $address, ${traceLocationStart?.toPrettyDate()} - ${traceLocationEnd?.toPrettyDate()}" + else -> "$description, $address" + } + } - coVerify(exactly = 1) { - contactDiaryRepo.addLocation(any()) + @Test + fun `CheckIn to ContactDiaryLocationVisit is correct`() { + createInstance().apply { + testCheckIn.toLocationVisit(testLocation).also { + it.checkInID shouldBe testCheckIn.id + it.date shouldBe testCheckIn.checkInStart.toLocalDateUtc() + it.duration!!.toStandardMinutes() shouldBe Minutes.minutes(60) + it.contactDiaryLocation shouldBe testLocation + } + } + } + + @Test + fun `CheckIn to ContactDiaryLocationVisit duration mapping is correct`() { + createInstance().apply { + // Rounds duration to closest 15 minutes + testCheckIn.copy(checkInEnd = Instant.parse("2021-03-04T23:07:29+01:00")).toLocationVisit(testLocation) + .also { + it.duration!!.toStandardMinutes() shouldBe Minutes.minutes(60) + } + + testCheckIn.copy(checkInEnd = Instant.parse("2021-03-04T23:07:30+01:00")).toLocationVisit(testLocation) + .also { + it.duration!!.toStandardMinutes() shouldBe Minutes.minutes(75) + } + + testCheckIn.copy(checkInEnd = Instant.parse("2021-03-04T22:52:30+01:00")).toLocationVisit(testLocation) + .also { + it.duration!!.toStandardMinutes() shouldBe Minutes.minutes(60) + } + + testCheckIn.copy(checkInEnd = Instant.parse("2021-03-04T22:52:29+01:00")).toLocationVisit(testLocation) + .also { + it.duration!!.toStandardMinutes() shouldBe Minutes.minutes(45) + } + } + } + + @Test + fun `Creates location visits if missing`() = runBlockingTest { + every { contactDiaryRepo.locationVisits } returns flowOf(emptyList()) andThen flowOf(listOf(testLocationVisit)) + + createInstance().apply { + val checkins = mutableListOf(testCheckIn) + + checkins.createMissingLocationVisits(testLocation).also { + it[0] shouldBe testLocationVisit + } + + checkins.createMissingLocationVisits(testLocation).also { + it.isEmpty() shouldBe true + } + + // Create check in for next day which should also create a visit for the next day + val testCheckInNextDay = testCheckIn.copy( + checkInStart = testCheckIn.checkInStart.plus(Days.ONE.toStandardDuration()), + checkInEnd = testCheckIn.checkInEnd.plus(Days.ONE.toStandardDuration()) + ) + checkins.add(testCheckInNextDay) + + checkins.createMissingLocationVisits(testLocation).also { + it.size shouldBe 1 // and not 2 + it[0] shouldBe testLocationVisit.copy(date = testLocationVisit.date.plusDays(1)) + } + } + } + + @Test + fun `Creates 1 location and 2 visits for split check in`() = runBlockingTest { + val splitCheckIn = testCheckIn.copy( + checkInStart = Instant.parse("2021-03-04T22:00+01:00"), + checkInEnd = Instant.parse("2021-03-05T02:00+01:00") + ) + createInstance().apply { + createEntry(splitCheckIn) + + coVerify(exactly = 1) { + contactDiaryRepo.addLocation(any()) + } + + coVerify(exactly = 2) { + contactDiaryRepo.addLocationVisit(any()) + } } } }