From d38c1067186426684057d2a1243fb6c8f70a21b9 Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Wed, 18 Nov 2020 14:44:42 +0100
Subject: [PATCH] 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>
---
.../appconfig/ExposureDetectionConfig.kt | 2 +-
.../appconfig/internal/AppConfigSource.kt | 2 +
.../mapping/ExposureDetectionConfigMapper.kt | 34 ++++++++-------
.../mapping/KeyDownloadParametersMapper.kt | 42 ++++++++++++-------
.../coronawarnapp/risk/DefaultRiskLevels.kt | 3 --
.../mapping/DownloadConfigMapperTest.kt | 14 +++++++
.../ExposureDetectionConfigMapperTest.kt | 15 ++++++-
7 files changed, 78 insertions(+), 34 deletions(-)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/ExposureDetectionConfig.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/ExposureDetectionConfig.kt
index be47c2f17..7f44f8f89 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/ExposureDetectionConfig.kt
+++ b/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>
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/internal/AppConfigSource.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/internal/AppConfigSource.kt
index 4ea21f387..1e5cc4959 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/internal/AppConfigSource.kt
+++ b/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()
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapper.kt
index 92473af00..841ecadb2 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapper.kt
+++ b/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 {
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/KeyDownloadParametersMapper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/KeyDownloadParametersMapper.kt
index 4c55393ec..80a6ddc68 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/mapping/KeyDownloadParametersMapper.kt
+++ b/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,
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/DefaultRiskLevels.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/DefaultRiskLevels.kt
index d0ca47e3b..37bb5130c 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/DefaultRiskLevels.kt
+++ b/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")
- }
}
}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/DownloadConfigMapperTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/DownloadConfigMapperTest.kt
index 2d0ab66e1..7de0f9e96 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/DownloadConfigMapperTest.kt
+++ b/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)
+ }
+ }
}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapperTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapperTest.kt
index 7812c23ab..96fcb7387 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/mapping/ExposureDetectionConfigMapperTest.kt
+++ b/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
+ }
+ }
}
--
GitLab