From f56c598a42f3c240a28ed8a5f09fdde5b8905344 Mon Sep 17 00:00:00 2001 From: Matthias Urhahn <matthias.urhahn@sap.com> Date: Fri, 2 Oct 2020 14:15:27 +0200 Subject: [PATCH] If there is no fallback appconfig yet, abort early to show a more accurate error (download error vs "no app config" error). (#1293) Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com> --- .../coronawarnapp/appconfig/AppConfigProvider.kt | 7 ++++++- .../coronawarnapp/appconfig/AppConfigStorage.kt | 8 ++++++++ .../appconfig/AppConfigProviderTest.kt | 1 + .../appconfig/AppConfigStorageTest.kt | 15 +++++++++++++++ 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/AppConfigProvider.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/AppConfigProvider.kt index 796f19a12..47810d450 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/AppConfigProvider.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/AppConfigProvider.kt @@ -58,7 +58,12 @@ class AppConfigProvider @Inject constructor( downloadAppConfig() } catch (e: Exception) { Timber.w(e, "Failed to download latest AppConfig.") - null + if (configStorage.isAppConfigAvailable) { + null + } else { + Timber.e("No fallback available, rethrowing!") + throw e + } } val newConfigParsed = try { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/AppConfigStorage.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/AppConfigStorage.kt index 49bbca096..44edccd9c 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/AppConfigStorage.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/AppConfigStorage.kt @@ -13,6 +13,9 @@ class AppConfigStorage @Inject constructor( private val configDir = File(context.filesDir, "appconfig_storage") private val configFile = File(configDir, "appconfig") + val isAppConfigAvailable: Boolean + get() = configFile.exists() && configFile.length() > MIN_VALID_CONFIG_BYTES + var appConfigRaw: ByteArray? get() { Timber.v("get() AppConfig") @@ -36,4 +39,9 @@ class AppConfigStorage @Inject constructor( configFile.delete() } } + + companion object { + // The normal config is ~512B+, we just need to check for a non 0 value, 128 is fine. + private const val MIN_VALID_CONFIG_BYTES = 128 + } } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/AppConfigProviderTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/AppConfigProviderTest.kt index b61f67757..e744b2fc3 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/AppConfigProviderTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/AppConfigProviderTest.kt @@ -38,6 +38,7 @@ class AppConfigProviderTest : BaseIOTest() { testDir.mkdirs() testDir.exists() shouldBe true + every { appConfigStorage.isAppConfigAvailable } answers { mockConfigStorage != null } every { appConfigStorage.appConfigRaw } answers { mockConfigStorage } every { appConfigStorage.appConfigRaw = any() } answers { mockConfigStorage = arg(0) } } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/AppConfigStorageTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/AppConfigStorageTest.kt index aaecb0b01..dd637fbb7 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/AppConfigStorageTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/appconfig/AppConfigStorageTest.kt @@ -36,6 +36,21 @@ class AppConfigStorageTest : BaseIOTest() { private fun createStorage() = AppConfigStorage(context) + @Test + fun `config availability is determined by file existence and min size`() { + storageDir.mkdirs() + val storage = createStorage() + storage.isAppConfigAvailable shouldBe false + configPath.createNewFile() + storage.isAppConfigAvailable shouldBe false + + configPath.writeBytes(ByteArray(128) { 1 }) + storage.isAppConfigAvailable shouldBe false + + configPath.writeBytes(ByteArray(129) { 1 }) + storage.isAppConfigAvailable shouldBe true + } + @Test fun `simple read and write config`() { configPath.exists() shouldBe false -- GitLab