Override EXPECT_NEW_PACKAGES if we revoked key packages (EXPOSUREAPP-3455) (#1572)

* If we revoked any cached keys we need to force an index lookup.
+ Additional tests to check that forced index lookup is decided on correctly.

* Fix merge regressions in tests.

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/BaseKeyPackageSyncTool.kt

@@ -14,18 +14,28 @@ open class BaseKeyPackageSyncTool(
     private val tag: String
 ) {
 
-    internal suspend fun revokeCachedKeys(revokedKeyPackages: Collection<KeyDownloadConfig.RevokedKeyPackage>) {
+    /**
+     * Returns true if any of our cached keys were revoked
+     */
+    internal suspend fun revokeCachedKeys(
+        revokedKeyPackages: Collection<KeyDownloadConfig.RevokedKeyPackage>
+    ): Boolean {
         if (revokedKeyPackages.isEmpty()) {
             Timber.tag(tag).d("No revoked key packages to delete.")
-            return
+            return false
         }
 
         val badEtags = revokedKeyPackages.map { it.etag }
-        val toDelete = keyCache.getAllCachedKeys()
-            .filter { badEtags.contains(it.info.etag) }
+        val toDelete = keyCache.getAllCachedKeys().filter { badEtags.contains(it.info.etag) }
 
-        Timber.tag(tag).w("Deleting revoked cached keys: %s", toDelete.joinToString("\n"))
-        keyCache.delete(toDelete.map { it.info })
+        return if (toDelete.isEmpty()) {
+            Timber.tag(tag).d("No local cached keys matched the revoked ones.")
+            false
+        } else {
+            Timber.tag(tag).w("Deleting revoked cached keys: %s", toDelete.joinToString("\n"))
+            keyCache.delete(toDelete.map { it.info })
+            true
+        }
     }
 
     internal suspend fun requireStorageSpace(data: List<LocationData>): DeviceStorage.CheckResult {

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncTool.kt

@@ -44,10 +44,10 @@ class DayPackageSyncTool @Inject constructor(
         Timber.tag(TAG).v("syncMissingDays(targetLocations=%s)", targetLocations)
 
         val downloadConfig: KeyDownloadConfig = configProvider.getAppConfig()
-        revokeCachedKeys(downloadConfig.revokedDayPackages)
+        val keysWereRevoked = revokeCachedKeys(downloadConfig.revokedDayPackages)
 
         val missingDays = targetLocations.mapNotNull {
-            determineMissingDayPackages(it, forceIndexLookup)
+            determineMissingDayPackages(it, forceIndexLookup || keysWereRevoked)
         }
         if (missingDays.isEmpty()) {
             Timber.tag(TAG).i("There were no missing day packages.")
@@ -80,10 +80,16 @@ class DayPackageSyncTool @Inject constructor(
     }
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-    internal suspend fun determineMissingDayPackages(location: LocationCode, forceIndexLookup: Boolean): LocationDays? {
+    internal suspend fun determineMissingDayPackages(
+        location: LocationCode,
+        forceIndexLookup: Boolean
+    ): LocationDays? {
         val cachedDays = getDownloadedCachedKeys(location, Type.LOCATION_DAY)
 
-        if (!forceIndexLookup && !expectNewDayPackages(cachedDays)) return null
+        if (!forceIndexLookup && !expectNewDayPackages(cachedDays)) {
+            Timber.tag(TAG).d("We don't expect new day packages.")
+            return null
+        }
 
         val availableDays = LocationDays(location, keyServer.getDayIndex(location))
 

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncTool.kt

@@ -48,10 +48,10 @@ class HourPackageSyncTool @Inject constructor(
         Timber.tag(TAG).v("syncMissingHours(targetLocations=%s)", targetLocations)
 
         val downloadConfig: KeyDownloadConfig = configProvider.getAppConfig()
-        revokeCachedKeys(downloadConfig.revokedHourPackages)
+        val keysWereRevoked = revokeCachedKeys(downloadConfig.revokedHourPackages)
 
         val missingHours = targetLocations.mapNotNull {
-            determineMissingHours(it, forceIndexLookup)
+            determineMissingHours(it, forceIndexLookup || keysWereRevoked)
         }
         if (missingHours.isEmpty()) {
             Timber.tag(TAG).i("There were no missing hours.")
@@ -124,12 +124,18 @@ class HourPackageSyncTool @Inject constructor(
     }
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-    internal suspend fun determineMissingHours(location: LocationCode, forceIndexLookup: Boolean): LocationHours? {
+    internal suspend fun determineMissingHours(
+        location: LocationCode,
+        forceIndexLookup: Boolean
+    ): LocationHours? {
         val cachedHours = getDownloadedCachedKeys(location, Type.LOCATION_HOUR)
 
         val now = timeStamper.nowUTC
 
-        if (!forceIndexLookup && !expectNewHourPackages(cachedHours, now)) return null
+        if (!forceIndexLookup && !expectNewHourPackages(cachedHours, now)) {
+            Timber.tag(TAG).d("We don't expect new hour packages.")
+            return null
+        }
 
         val today = now.toLocalDate()
 

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/DayPackageSyncToolTest.kt

@@ -6,6 +6,7 @@ import de.rki.coronawarnapp.diagnosiskeys.storage.CachedKeyInfo
 import de.rki.coronawarnapp.diagnosiskeys.storage.CachedKeyInfo.Type
 import io.kotest.matchers.shouldBe
 import io.mockk.coEvery
+import io.mockk.coVerify
 import io.mockk.coVerifySequence
 import io.mockk.every
 import io.mockk.mockk
@@ -82,7 +83,7 @@ class DayPackageSyncToolTest : CommonSyncToolTest() {
     }
 
     @Test
-    fun `determine missing days forcesync ignores EXPECT NEW DAYS`() = runBlockingTest {
+    fun `determine missing days with forcesync ignores EXPECT NEW DAYS`() = runBlockingTest {
         mockCachedDay("EUR".loc, "2020-01-01".day)
         mockCachedDay("EUR".loc, "2020-01-02".day)
 
@@ -173,11 +174,52 @@ class DayPackageSyncToolTest : CommonSyncToolTest() {
             keyCache.delete(listOf(invalidDay.info))
 
             keyCache.getEntriesForType(Type.LOCATION_DAY)
-            timeStamper.nowUTC
             keyServer.getDayIndex("EUR".loc)
 
             keyCache.createCacheEntry(Type.LOCATION_DAY, "EUR".loc, "2020-01-03".day, null)
             downloadTool.downloadKeyFile(any(), downloadConfig)
         }
     }
+
+    @Test
+    fun `if keys were revoked skip the EXPECT packages check`() = runBlockingTest {
+        every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T12:12:12.000Z")
+        mockCachedDay("EUR".loc, "2020-01-01".day)
+        mockCachedDay("EUR".loc, "2020-01-02".day)
+        mockCachedDay("EUR".loc, "2020-01-03".day).apply {
+            every { downloadConfig.revokedDayPackages } returns listOf(
+                RevokedKeyPackage.Day(
+                    day = info.day,
+                    region = info.location,
+                    etag = info.etag!!
+                )
+            )
+        }
+
+        createInstance().syncMissingDayPackages(listOf("EUR".loc), false)
+
+        coVerify(exactly = 1) { keyServer.getDayIndex("EUR".loc) }
+    }
+
+    @Test
+    fun `if force-sync is set we skip the EXPECT packages check`() = runBlockingTest {
+        every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T12:12:12.000Z")
+        mockCachedDay("EUR".loc, "2020-01-01".day)
+        mockCachedDay("EUR".loc, "2020-01-02".day)
+        mockCachedDay("EUR".loc, "2020-01-03".day)
+        createInstance().syncMissingDayPackages(listOf("EUR".loc), true)
+
+        coVerify(exactly = 1) { keyServer.getDayIndex("EUR".loc) }
+    }
+
+    @Test
+    fun `if neither force-sync is set and keys were revoked we check EXPECT NEW PKGS`() = runBlockingTest {
+        every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T12:12:12.000Z")
+        mockCachedDay("EUR".loc, "2020-01-01".day)
+        mockCachedDay("EUR".loc, "2020-01-02".day)
+        mockCachedDay("EUR".loc, "2020-01-03".day)
+        createInstance().syncMissingDayPackages(listOf("EUR".loc), false)
+
+        coVerify(exactly = 0) { keyServer.getDayIndex("EUR".loc) }
+    }
 }

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/HourPackageSyncToolTest.kt

@@ -6,6 +6,7 @@ import de.rki.coronawarnapp.diagnosiskeys.storage.CachedKeyInfo
 import de.rki.coronawarnapp.diagnosiskeys.storage.CachedKeyInfo.Type
 import io.kotest.matchers.shouldBe
 import io.mockk.coEvery
+import io.mockk.coVerify
 import io.mockk.coVerifySequence
 import io.mockk.every
 import io.mockk.mockk
@@ -136,14 +137,14 @@ class HourPackageSyncToolTest : CommonSyncToolTest() {
     }
 
     @Test
-    fun `determine missing hours forcesync ignores EXPECT NEW HOURS`() = runBlockingTest {
+    fun `determine missing hours with forcesync ignores EXPECT NEW HOURS`() = runBlockingTest {
         mockCachedHour("EUR".loc, "2020-01-04".day, "00:00".hour)
         mockCachedHour("EUR".loc, "2020-01-04".day, "01:00".hour)
 
         val instance = createInstance()
 
         every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T02:00:00.000Z")
-        instance.determineMissingHours("EUR".loc, true) shouldBe LocationHours(
+        instance.determineMissingHours("EUR".loc, forceIndexLookup = true) shouldBe LocationHours(
             location = "EUR".loc,
             hourData = mapOf("2020-01-04".day to listOf("02:00".hour))
         )
@@ -202,4 +203,45 @@ class HourPackageSyncToolTest : CommonSyncToolTest() {
         now = Instant.parse("2020-01-01T03:00:03.000Z")
         instance.expectNewHourPackages(listOf(cachedKey1, cachedKey2), now) shouldBe true
     }
+
+    @Test
+    fun `if keys were revoked skip the EXPECT packages check`() = runBlockingTest {
+        every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T02:00:00.000Z")
+        mockCachedHour("EUR".loc, "2020-01-04".day, "00:00".hour)
+        mockCachedHour("EUR".loc, "2020-01-04".day, "01:00".hour)
+        mockCachedHour("EUR".loc, "2020-01-04".day, "02:00".hour).apply {
+            every { downloadConfig.revokedHourPackages } returns listOf(
+                RevokedKeyPackage.Hour(
+                    region = info.location,
+                    etag = info.etag!!,
+                    day = info.day,
+                    hour = info.hour!!
+                )
+            )
+        }
+
+        createInstance().syncMissingHourPackages(listOf("EUR".loc), false)
+
+        coVerify(exactly = 1) { keyServer.getHourIndex("EUR".loc, "2020-01-04".day) }
+    }
+
+    @Test
+    fun `if force-sync is set we skip the EXPECT packages check`() = runBlockingTest {
+        every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T02:00:00.000Z")
+        mockCachedHour("EUR".loc, "2020-01-04".day, "00:00".hour)
+        mockCachedHour("EUR".loc, "2020-01-04".day, "01:00".hour)
+        createInstance().syncMissingHourPackages(listOf("EUR".loc), true)
+
+        coVerify(exactly = 1) { keyServer.getHourIndex("EUR".loc, "2020-01-04".day) }
+    }
+
+    @Test
+    fun `if neither force-sync is set and keys were revoked we check EXPECT NEW PKGS`() = runBlockingTest {
+        every { timeStamper.nowUTC } returns Instant.parse("2020-01-04T02:00:00.000Z")
+        mockCachedHour("EUR".loc, "2020-01-04".day, "00:00".hour)
+        mockCachedHour("EUR".loc, "2020-01-04".day, "01:00".hour)
+        createInstance().syncMissingHourPackages(listOf("EUR".loc), false)
+
+        coVerify(exactly = 0) { keyServer.getHourIndex("EUR".loc, "2020-01-04".day) }
+    }
 }

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/main/home/HomeFragmentViewModelTest.kt

@@ -9,16 +9,13 @@ import de.rki.coronawarnapp.ui.main.home.HomeFragmentViewModel
 import de.rki.coronawarnapp.ui.main.home.SubmissionCardState
 import de.rki.coronawarnapp.ui.main.home.SubmissionCardsStateProvider
 import de.rki.coronawarnapp.ui.main.home.TracingHeaderState
-import de.rki.coronawarnapp.ui.submission.ApiRequestState
 import de.rki.coronawarnapp.ui.submission.ApiRequestState.SUCCESS
 import de.rki.coronawarnapp.ui.tracing.card.TracingCardState
 import de.rki.coronawarnapp.ui.tracing.card.TracingCardStateProvider
 import de.rki.coronawarnapp.ui.viewmodel.SettingsViewModel
-import de.rki.coronawarnapp.util.DeviceUIState
 import de.rki.coronawarnapp.util.DeviceUIState.PAIRED_POSITIVE
 import de.rki.coronawarnapp.util.DeviceUIState.PAIRED_POSITIVE_TELETAN
 import de.rki.coronawarnapp.util.security.EncryptionErrorResetTool
-import io.kotest.matchers.neverNullMatcher
 import io.kotest.matchers.shouldBe
 import io.mockk.MockKAnnotations
 import io.mockk.clearAllMocks
@@ -133,12 +130,12 @@ class HomeFragmentViewModelTest : BaseTest() {
     fun `positive test result notification is triggered on positive QR code result`() {
         val state = SubmissionCardState(PAIRED_POSITIVE, true, SUCCESS)
         every { submissionCardsStateProvider.state } returns flowOf(state)
-        every {testResultNotificationService.schedulePositiveTestResultReminder() } returns Unit
+        every { testResultNotificationService.schedulePositiveTestResultReminder() } returns Unit
 
         runBlocking {
             createInstance().apply {
                 observeTestResultToSchedulePositiveTestResultReminder()
-                verify { testResultNotificationService.schedulePositiveTestResultReminder()}
+                verify { testResultNotificationService.schedulePositiveTestResultReminder() }
             }
         }
     }
@@ -147,12 +144,12 @@ class HomeFragmentViewModelTest : BaseTest() {
     fun `positive test result notification is triggered on positive TeleTan code result`() {
         val state = SubmissionCardState(PAIRED_POSITIVE_TELETAN, true, SUCCESS)
         every { submissionCardsStateProvider.state } returns flowOf(state)
-        every {testResultNotificationService.schedulePositiveTestResultReminder() } returns Unit
+        every { testResultNotificationService.schedulePositiveTestResultReminder() } returns Unit
 
         runBlocking {
             createInstance().apply {
                 observeTestResultToSchedulePositiveTestResultReminder()
-                verify { testResultNotificationService.schedulePositiveTestResultReminder()}
+                verify { testResultNotificationService.schedulePositiveTestResultReminder() }
             }
         }
     }