Fix config default values not being used if protobuf is missing the data...

Fix config default values not being used if protobuf is missing the data structures (EXPOSUREAPP-3856) (#1649)

* Improve logging.

* Fix exposure detection config not using default values if protobuf structures are null.

* Fix key download config not using default values if protobuf structures are null.

Co-authored-by: Ralf Gehrer <ralfgehrer@users.noreply.github.com>

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/ExposureDetectionConfig.kt

@@ -12,7 +12,7 @@ interface ExposureDetectionConfig {
     val overallDetectionTimeout: Duration
 
     val exposureDetectionConfiguration: ExposureConfiguration
-    val exposureDetectionParameters: ExposureDetectionParameters.ExposureDetectionParametersAndroid
+    val exposureDetectionParameters: ExposureDetectionParameters.ExposureDetectionParametersAndroid?
 
     interface Mapper : ConfigMapper<ExposureDetectionConfig>
 }

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/internal/AppConfigSource.kt

@@ -24,6 +24,8 @@ class AppConfigSource @Inject constructor(
         if (localConfig != null && localConfig.isValid(timeStamper.nowUTC)) {
             Timber.tag(TAG).d("Returning local config, still valid.")
             return localConfig
+        } else {
+            Timber.tag(TAG).d("Local app config was unavailable(${localConfig == null} or invalid.")
         }
 
         val remoteConfig = remoteAppConfigSource.getConfigData()

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapper.kt

@@ -12,7 +12,11 @@ import javax.inject.Inject
 @Reusable
 class ExposureDetectionConfigMapper @Inject constructor() : ExposureDetectionConfig.Mapper {
     override fun map(rawConfig: AppConfig.ApplicationConfiguration): ExposureDetectionConfig {
-        val exposureParams = rawConfig.androidExposureDetectionParameters
+        val exposureParams = if (rawConfig.hasAndroidExposureDetectionParameters()) {
+            rawConfig.androidExposureDetectionParameters
+        } else {
+            null
+        }
         return ExposureDetectionConfigContainer(
             exposureDetectionConfiguration = rawConfig.mapRiskScoreToExposureConfiguration(),
             exposureDetectionParameters = exposureParams,
@@ -24,7 +28,7 @@ class ExposureDetectionConfigMapper @Inject constructor() : ExposureDetectionCon
 
     data class ExposureDetectionConfigContainer(
         override val exposureDetectionConfiguration: ExposureConfiguration,
-        override val exposureDetectionParameters: ExposureDetectionParametersAndroid,
+        override val exposureDetectionParameters: ExposureDetectionParametersAndroid?,
         override val maxExposureDetectionsPerUTCDay: Int,
         override val minTimeBetweenDetections: Duration,
         override val overallDetectionTimeout: Duration
@@ -33,23 +37,25 @@ class ExposureDetectionConfigMapper @Inject constructor() : ExposureDetectionCon
 
 // If we are outside the valid data range, fallback to default value.
 @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-fun ExposureDetectionParametersAndroid.overAllDetectionTimeout(): Duration = when {
-    overallTimeoutInSeconds > 3600 -> Duration.standardMinutes(15)
-    overallTimeoutInSeconds <= 0 -> Duration.standardMinutes(15)
-    else -> Duration.standardSeconds(overallTimeoutInSeconds.toLong())
-}
+fun ExposureDetectionParametersAndroid?.overAllDetectionTimeout(): Duration =
+    if (this == null || overallTimeoutInSeconds > 3600 || overallTimeoutInSeconds <= 0) {
+        Duration.standardMinutes(15)
+    } else {
+        Duration.standardSeconds(overallTimeoutInSeconds.toLong())
+    }
 
 // If we are outside the valid data range, fallback to default value.
 @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-fun ExposureDetectionParametersAndroid.maxExposureDetectionsPerDay(): Int = when {
-    maxExposureDetectionsPerInterval > 6 -> 6
-    maxExposureDetectionsPerInterval < 0 -> 6
-    else -> maxExposureDetectionsPerInterval
-}
+fun ExposureDetectionParametersAndroid?.maxExposureDetectionsPerDay(): Int =
+    if (this == null || maxExposureDetectionsPerInterval > 6 || maxExposureDetectionsPerInterval < 0) {
+        6
+    } else {
+        maxExposureDetectionsPerInterval
+    }
 
 @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-fun ExposureDetectionParametersAndroid.minTimeBetweenExposureDetections(): Duration {
-    val detectionsPerDay = maxExposureDetectionsPerDay()
+fun ExposureDetectionParametersAndroid?.minTimeBetweenExposureDetections(): Duration {
+    val detectionsPerDay = this.maxExposureDetectionsPerDay()
     return if (detectionsPerDay == 0) {
         Duration.standardDays(99)
     } else {

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/KeyDownloadParametersMapper.kt

@@ -16,7 +16,11 @@ import javax.inject.Inject
 @Reusable
 class KeyDownloadParametersMapper @Inject constructor() : KeyDownloadConfig.Mapper {
     override fun map(rawConfig: AppConfig.ApplicationConfiguration): KeyDownloadConfig {
-        val rawParameters = rawConfig.androidKeyDownloadParameters
+        val rawParameters = if (rawConfig.hasAndroidKeyDownloadParameters()) {
+            rawConfig.androidKeyDownloadParameters
+        } else {
+            null
+        }
 
         return KeyDownloadConfigContainer(
             individualDownloadTimeout = rawParameters.individualTimeout(),
@@ -27,21 +31,25 @@ class KeyDownloadParametersMapper @Inject constructor() : KeyDownloadConfig.Mapp
     }
 
     // If we are outside the valid data range, fallback to default value.
-    private fun KeyDownloadParametersAndroid.individualTimeout(): Duration = when {
-        downloadTimeoutInSeconds > 1800 -> Duration.standardSeconds(60)
-        downloadTimeoutInSeconds <= 0 -> Duration.standardSeconds(60)
-        else -> Duration.standardSeconds(downloadTimeoutInSeconds.toLong())
-    }
+    private fun KeyDownloadParametersAndroid?.individualTimeout(): Duration =
+        if (this == null || downloadTimeoutInSeconds > 1800 || downloadTimeoutInSeconds <= 0) {
+            Duration.standardSeconds(60)
+        } else {
+            Duration.standardSeconds(downloadTimeoutInSeconds.toLong())
+        }
 
     // If we are outside the valid data range, fallback to default value.
-    private fun KeyDownloadParametersAndroid.overAllTimeout(): Duration = when {
-        overallTimeoutInSeconds > 1800 -> Duration.standardMinutes(8)
-        overallTimeoutInSeconds <= 0 -> Duration.standardMinutes(8)
-        else -> Duration.standardSeconds(overallTimeoutInSeconds.toLong())
-    }
+    private fun KeyDownloadParametersAndroid?.overAllTimeout(): Duration =
+        if (this == null || overallTimeoutInSeconds > 1800 || overallTimeoutInSeconds <= 0) {
+            Duration.standardMinutes(8)
+        } else {
+            Duration.standardSeconds(overallTimeoutInSeconds.toLong())
+        }
 
-    private fun KeyDownloadParametersAndroid.mapDayEtags(): List<RevokedKeyPackage.Day> =
-        this.revokedDayPackagesList.mapNotNull {
+    private fun KeyDownloadParametersAndroid?.mapDayEtags(): List<RevokedKeyPackage.Day> {
+        if (this == null) return emptyList()
+
+        return this.revokedDayPackagesList.mapNotNull {
             try {
                 RevokedKeyPackage.Day(
                     etag = it.etag,
@@ -53,9 +61,12 @@ class KeyDownloadParametersMapper @Inject constructor() : KeyDownloadConfig.Mapp
                 null
             }
         }
+    }
 
-    private fun KeyDownloadParametersAndroid.mapHourEtags(): List<RevokedKeyPackage.Hour> =
-        this.revokedHourPackagesList.mapNotNull {
+    private fun KeyDownloadParametersAndroid?.mapHourEtags(): List<RevokedKeyPackage.Hour> {
+        if (this == null) return emptyList()
+
+        return this.revokedHourPackagesList.mapNotNull {
             try {
                 RevokedKeyPackage.Hour(
                     etag = it.etag,
@@ -68,6 +79,7 @@ class KeyDownloadParametersMapper @Inject constructor() : KeyDownloadConfig.Mapp
                 null
             }
         }
+    }
 
     data class KeyDownloadConfigContainer(
         override val individualDownloadTimeout: Duration,

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

@@ -132,9 +132,6 @@ class DefaultRiskLevels @Inject constructor() : RiskLevels {
                 "Active tracing time ($activeTracingDurationInHours h) is above threshold " +
                     "($durationTracingIsActiveThreshold h): $it"
             )
-            if (it) {
-                Timber.tag(TAG).v("Active tracing time is not enough")
-            }
         }
     }
 

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/DownloadConfigMapperTest.kt

@@ -4,6 +4,7 @@ import de.rki.coronawarnapp.diagnosiskeys.server.LocationCode
 import de.rki.coronawarnapp.server.protocols.internal.AppConfig
 import de.rki.coronawarnapp.server.protocols.internal.KeyDownloadParameters
 import io.kotest.matchers.shouldBe
+import org.joda.time.Duration
 import org.joda.time.LocalDate
 import org.joda.time.LocalTime
 import org.junit.jupiter.api.Test
@@ -59,4 +60,17 @@ class DownloadConfigMapperTest : BaseTest() {
             }
         }
     }
+
+    @Test
+    fun `if the protobuf data structures are null we return defaults`() {
+        val rawConfig = AppConfig.ApplicationConfiguration.newBuilder()
+            .build()
+
+        createInstance().map(rawConfig).apply {
+            revokedDayPackages shouldBe emptyList()
+            revokedHourPackages shouldBe emptyList()
+            overallDownloadTimeout shouldBe Duration.standardMinutes(8)
+            individualDownloadTimeout shouldBe Duration.standardSeconds(60)
+        }
+    }
 }

Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapperTest.kt

@@ -17,8 +17,9 @@ class ExposureDetectionConfigMapperTest : BaseTest() {
             .setMinRiskScore(1)
             .build()
         createInstance().map(rawConfig).apply {
+            // This is basically the old legacy config without the new hourly related data structures
             exposureDetectionConfiguration shouldBe rawConfig.mapRiskScoreToExposureConfiguration()
-            exposureDetectionParameters shouldBe rawConfig.androidExposureDetectionParameters
+            exposureDetectionParameters shouldBe null
         }
     }
 
@@ -77,4 +78,16 @@ class ExposureDetectionConfigMapperTest : BaseTest() {
             overallDetectionTimeout shouldBe Duration.standardMinutes(15)
         }
     }
+
+    @Test
+    fun `if protobuf is missing the datastructure we return defaults`() {
+        val rawConfig = AppConfig.ApplicationConfiguration.newBuilder()
+            .setMinRiskScore(1)
+            .build()
+        createInstance().map(rawConfig).apply {
+            overallDetectionTimeout shouldBe Duration.standardMinutes(15)
+            minTimeBetweenDetections shouldBe Duration.standardHours(24 / 6)
+            maxExposureDetectionsPerUTCDay shouldBe 6
+        }
+    }
 }