Refactor ENF version check (#1680)

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/diagnosiskeyprovider/DefaultDiagnosisKeyProvider.kt

@@ -25,13 +25,9 @@ class DefaultDiagnosisKeyProvider @Inject constructor(
             return true
         }
 
-        // Check version of ENF
-        try {
-            enfVersion.requireAtLeast(ENFVersion.V16)
-        } catch (e: Exception) {
-            Timber.e(e)
-            throw e
-        }
+        // Check version of ENF, WindowMode since v1.5, but version check since v1.6
+        // Will throw if requirement is not satisfied
+        enfVersion.requireMinimumVersion(ENFVersion.V1_6)
 
         if (!submissionQuota.consumeQuota(1)) {
             Timber.w("No key files submitted because not enough quota available.")

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/version/DefaultENFVersion.kt

@@ -9,7 +9,6 @@ import javax.inject.Singleton
 import kotlin.coroutines.resume
 import kotlin.coroutines.resumeWithException
 import kotlin.coroutines.suspendCoroutine
-import kotlin.math.abs
 
 @Singleton
 class DefaultENFVersion @Inject constructor(
@@ -23,22 +22,19 @@ class DefaultENFVersion @Inject constructor(
         null
     }
 
-    override suspend fun requireAtLeast(compareVersion: Long) {
-        if (!isAtLeast(compareVersion)) {
-            throw ENFVersion.Companion.UnsupportedENFVersionException()
-        }
-    }
-
-    override suspend fun isAtLeast(compareVersion: Long): Boolean {
-        if (!compareVersion.isCorrectVersionLength) throw IllegalArgumentException("given version has incorrect length")
-
-        return try {
-            internalGetENFClientVersion() >= compareVersion
+    override suspend fun requireMinimumVersion(required: Long) {
+        try {
+            val currentVersion = internalGetENFClientVersion()
+            if (currentVersion < required) {
+                val error = OutdatedENFVersionException(current = currentVersion, required = required)
+                Timber.e(error, "Version requirement not satisfied.")
+                throw error
+            } else {
+                Timber.d("Version requirement satisfied: current=$currentVersion, required=$required")
+            }
         } catch (apiException: ApiException) {
             if (apiException.statusCode != CommonStatusCodes.API_NOT_CONNECTED) {
                 throw apiException
-            } else {
-                return false
             }
         }
     }
@@ -48,12 +44,4 @@ class DefaultENFVersion @Inject constructor(
             .addOnSuccessListener { cont.resume(it) }
             .addOnFailureListener { cont.resumeWithException(it) }
     }
-
-    // check if a raw long has the correct length to be considered an API version
-    private val Long.isCorrectVersionLength
-        get(): Boolean = abs(this).toString().length == GOOGLE_API_VERSION_FIELD_LENGTH
-
-    companion object {
-        private const val GOOGLE_API_VERSION_FIELD_LENGTH = 8
-    }
 }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/version/ENFVersion.kt

 package de.rki.coronawarnapp.nearby.modules.version
 
 interface ENFVersion {
-    suspend fun getENFClientVersion(): Long?
-
     /**
-     * Indicates if the client runs above a certain version
-     *
-     * @return isAboveVersion, if connected to an old unsupported version, return false
+     * May return null if the API is currently not connected.
      */
-    suspend fun isAtLeast(compareVersion: Long): Boolean
+    suspend fun getENFClientVersion(): Long?
 
     /**
-     * Throws an [UnsupportedENFVersionException] if the client runs an old unsupported version of the ENF
+     * Throws an [OutdatedENFVersionException] if the client runs an old unsupported version of the ENF
+     * If the API is currently not connected, no exception will be thrown, we expect this to only be a temporary state
      */
-    suspend fun requireAtLeast(compareVersion: Long)
+    suspend fun requireMinimumVersion(required: Long)
 
     companion object {
-        const val V16 = 16000000L
-        const val V15 = 15000000L
-
-        class UnsupportedENFVersionException : Exception("The client runs an old unsupported version of the ENF")
+        const val V1_6 = 16000000L
     }
 }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/version/OutdatedENFVersionException.kt

+package de.rki.coronawarnapp.nearby.modules.version
+
+class OutdatedENFVersionException(
+    val current: Long,
+    val required: Long
+) : Exception("Client is using an outdated ENF version: current=$current, required=$required")

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/diagnosiskeyprovider/DefaultDiagnosisKeyProviderTest.kt

@@ -2,6 +2,7 @@ package de.rki.coronawarnapp.nearby.modules.diagnosiskeyprovider
 
 import com.google.android.gms.nearby.exposurenotification.ExposureNotificationClient
 import de.rki.coronawarnapp.nearby.modules.version.ENFVersion
+import de.rki.coronawarnapp.nearby.modules.version.OutdatedENFVersionException
 import io.kotest.matchers.shouldBe
 import io.mockk.MockKAnnotations
 import io.mockk.clearAllMocks
@@ -33,7 +34,7 @@ class DefaultDiagnosisKeyProviderTest : BaseTest() {
 
         coEvery { googleENFClient.provideDiagnosisKeys(any<List<File>>()) } returns MockGMSTask.forValue(null)
 
-        coEvery { enfVersion.requireAtLeast(any()) } returns Unit
+        coEvery { enfVersion.requireMinimumVersion(any()) } returns Unit
     }
 
     @AfterEach
@@ -49,11 +50,14 @@ class DefaultDiagnosisKeyProviderTest : BaseTest() {
 
     @Test
     fun `provide diagnosis keys with outdated ENF versions`() {
-        coEvery { enfVersion.requireAtLeast(any()) } throws ENFVersion.Companion.UnsupportedENFVersionException()
+        coEvery { enfVersion.requireMinimumVersion(any()) } throws OutdatedENFVersionException(
+            current = 9000,
+            required = 5000
+        )
 
         val provider = createProvider()
 
-        assertThrows<ENFVersion.Companion.UnsupportedENFVersionException> {
+        assertThrows<OutdatedENFVersionException> {
             runBlockingTest { provider.provideDiagnosisKeys(exampleKeyFiles) } shouldBe false
         }
 

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/version/DefaultENFVersionTest.kt

@@ -2,21 +2,21 @@ package de.rki.coronawarnapp.nearby.modules.version
 
 import com.google.android.gms.common.api.ApiException
 import com.google.android.gms.common.api.CommonStatusCodes.API_NOT_CONNECTED
+import com.google.android.gms.common.api.CommonStatusCodes.INTERNAL_ERROR
 import com.google.android.gms.common.api.Status
 import com.google.android.gms.nearby.exposurenotification.ExposureNotificationClient
+import io.kotest.assertions.throwables.shouldNotThrowAny
+import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.matchers.shouldBe
-import io.mockk.Called
 import io.mockk.MockKAnnotations
 import io.mockk.clearAllMocks
 import io.mockk.every
 import io.mockk.impl.annotations.MockK
-import io.mockk.verify
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.runBlockingTest
 import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
-import org.junit.jupiter.api.assertThrows
 import testhelpers.gms.MockGMSTask
 
 @ExperimentalCoroutinesApi
@@ -39,61 +39,74 @@ internal class DefaultENFVersionTest {
     )
 
     @Test
-    fun `isAbove API v16 is true for v17`() {
+    fun `current version is newer than the required version`() {
         every { client.version } returns MockGMSTask.forValue(17000000L)
 
         runBlockingTest {
-            createInstance().isAtLeast(ENFVersion.V16) shouldBe true
+            createInstance().apply {
+                getENFClientVersion() shouldBe 17000000L
+                shouldNotThrowAny {
+                    requireMinimumVersion(ENFVersion.V1_6)
+                }
+            }
         }
     }
 
     @Test
-    fun `isAbove API v16 is false for v15`() {
+    fun `current version is older than the required version`() {
         every { client.version } returns MockGMSTask.forValue(15000000L)
 
         runBlockingTest {
-            createInstance().isAtLeast(ENFVersion.V16) shouldBe false
-        }
-    }
+            createInstance().apply {
+                getENFClientVersion() shouldBe 15000000L
 
-    @Test
-    fun `isAbove API v16 throws IllegalArgument for invalid version`() {
-        assertThrows<IllegalArgumentException> {
-            runBlockingTest {
-                createInstance().isAtLeast(1L)
+                shouldThrow<OutdatedENFVersionException> {
+                    requireMinimumVersion(ENFVersion.V1_6)
+                }
             }
-            verify { client.version wasNot Called }
         }
     }
 
     @Test
-    fun `isAbove API v16 false when APIException for too low version`() {
-        every { client.version } returns MockGMSTask.forError(ApiException(Status(API_NOT_CONNECTED)))
+    fun `current version is equal to the required version`() {
+        every { client.version } returns MockGMSTask.forValue(16000000L)
 
         runBlockingTest {
-            createInstance().isAtLeast(ENFVersion.V16) shouldBe false
+            createInstance().apply {
+                getENFClientVersion() shouldBe ENFVersion.V1_6
+                shouldNotThrowAny {
+                    requireMinimumVersion(ENFVersion.V1_6)
+                }
+            }
         }
     }
 
     @Test
-    fun `require API v16 throws UnsupportedENFVersionException for v15`() {
-        every { client.version } returns MockGMSTask.forValue(ENFVersion.V15)
+    fun `API_NOT_CONNECTED exceptions are not treated as failures`() {
+        every { client.version } returns MockGMSTask.forError(ApiException(Status(API_NOT_CONNECTED)))
 
-        assertThrows<ENFVersion.Companion.UnsupportedENFVersionException> {
-            runBlockingTest {
-                createInstance().requireAtLeast(ENFVersion.V16)
+        runBlockingTest {
+            createInstance().apply {
+                getENFClientVersion() shouldBe null
+                shouldNotThrowAny {
+                    requireMinimumVersion(ENFVersion.V1_6)
+                }
             }
         }
     }
 
     @Test
-    fun `require API v15 does not throw for v16`() {
-        every { client.version } returns MockGMSTask.forValue(ENFVersion.V16)
+    fun `rethrows unexpected exceptions`() {
+        every { client.version } returns MockGMSTask.forError(ApiException(Status(INTERNAL_ERROR)))
 
         runBlockingTest {
-            createInstance().requireAtLeast(ENFVersion.V15)
-        }
+            createInstance().apply {
+                getENFClientVersion() shouldBe null
 
-        verify { client.version }
+                shouldThrow<ApiException> {
+                    requireMinimumVersion(ENFVersion.V1_6)
+                }
+            }
+        }
     }
 }