From 526798d03fd1408ef5e5eb7d596a40dec1bd5349 Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <darken@darken.eu>
Date: Fri, 18 Sep 2020 21:32:43 +0200
Subject: [PATCH] We only need to check that we have enough storage space for
what is missing. (#1180)
Co-authored-by: Matthias Urhahn <matthias.urhahn@sap.com>
---
.../diagnosiskeys/download/CountryData.kt | 22 +++++++++
.../download/KeyFileDownloader.kt | 23 ++++-----
.../coronawarnapp/storage/DeviceStorage.kt | 9 ++++
.../diagnosiskeys/download/CountryDataTest.kt | 32 +++++++++++++
.../download/KeyFileDownloaderTest.kt | 47 ++++++++++++++++---
.../storage/DeviceStorageTest.kt | 3 ++
6 files changed, 118 insertions(+), 18 deletions(-)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/CountryData.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/CountryData.kt
index f9b4abd94..6f58466ca 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/CountryData.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/CountryData.kt
@@ -8,6 +8,8 @@ import org.joda.time.LocalTime
sealed class CountryData {
abstract val country: LocationCode
+
+ abstract val approximateSizeInBytes: Long
}
internal data class CountryDays(
@@ -15,6 +17,10 @@ internal data class CountryDays(
val dayData: Collection<LocalDate>
) : CountryData() {
+ override val approximateSizeInBytes: Long by lazy {
+ dayData.size * APPROX_DAY_SIZE
+ }
+
/**
* Return a filtered list that contains all dates which are part of this wrapper, but not in the parameter.
*/
@@ -38,6 +44,11 @@ internal data class CountryDays(
return CountryDays(this.country, missingDays)
}
+
+ companion object {
+ // ~512KB
+ private const val APPROX_DAY_SIZE = 512 * 1024L
+ }
}
internal data class CountryHours(
@@ -45,6 +56,12 @@ internal data class CountryHours(
val hourData: Map<LocalDate, List<LocalTime>>
) : CountryData() {
+ override val approximateSizeInBytes: Long by lazy {
+ hourData.values.fold(0L) { acc, hoursForDay ->
+ acc + hoursForDay.size * APPROX_HOUR_SIZE
+ }
+ }
+
fun getMissingHours(cachedKeys: List<CachedKeyInfo>): Map<LocalDate, List<LocalTime>>? {
val cachedHours = cachedKeys
.filter { it.location == country }
@@ -63,4 +80,9 @@ internal data class CountryHours(
return CountryHours(this.country, missingHours)
}
+
+ companion object {
+ // ~22KB
+ private const val APPROX_HOUR_SIZE = 22 * 1024L
+ }
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloader.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloader.kt
index 686a07972..bf7accefe 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloader.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloader.kt
@@ -10,7 +10,6 @@ import de.rki.coronawarnapp.diagnosiskeys.storage.legacy.LegacyKeyCacheMigration
import de.rki.coronawarnapp.risk.TimeVariables
import de.rki.coronawarnapp.storage.AppSettings
import de.rki.coronawarnapp.storage.DeviceStorage
-import de.rki.coronawarnapp.storage.InsufficientStorageException
import de.rki.coronawarnapp.util.CWADebug
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
@@ -33,13 +32,14 @@ class KeyFileDownloader @Inject constructor(
private val settings: AppSettings
) {
- private suspend fun checkStorageSpace(countries: List<LocationCode>): DeviceStorage.CheckResult {
- val storageResult = deviceStorage.checkSpacePrivateStorage(
- // 512KB per day file, for 15 days, for each country ~ 65MB for 9 countries
- requiredBytes = countries.size * EXPECTED_STORAGE_PER_COUNTRY
- )
- Timber.tag(TAG).d("Storage check result: %s", storageResult)
- return storageResult
+ private suspend fun requireStorageSpace(data: List<CountryData>): DeviceStorage.CheckResult {
+ val requiredBytes = data.fold(0L) { acc, item ->
+ acc + item.approximateSizeInBytes
+ }
+ Timber.d("%dB are required for %s", requiredBytes, data)
+ return deviceStorage.requireSpacePrivateStorage(requiredBytes).also {
+ Timber.tag(TAG).d("Storage check result: %s", it)
+ }
}
private suspend fun getCompletedKeyFiles(type: CachedKeyInfo.Type): List<CachedKeyInfo> {
@@ -77,9 +77,6 @@ class KeyFileDownloader @Inject constructor(
availableCountries, wantedCountries, filteredCountries
)
- val storageResult = checkStorageSpace(filteredCountries)
- if (!storageResult.isSpaceAvailable) throw InsufficientStorageException(storageResult)
-
val availableKeys =
if (CWADebug.isDebugBuildOrMode && settings.isLast3HourModeEnabled) {
syncMissing3Hours(filteredCountries, DEBUG_HOUR_LIMIT)
@@ -132,6 +129,8 @@ class KeyFileDownloader @Inject constructor(
) = withContext(Dispatchers.IO) {
val countriesWithMissingDays = determineMissingDays(availableCountries)
+ requireStorageSpace(countriesWithMissingDays)
+
Timber.tag(TAG).d("Downloading missing days: %s", countriesWithMissingDays)
val batchDownloadStart = System.currentTimeMillis()
val dayDownloads = countriesWithMissingDays
@@ -266,6 +265,8 @@ class KeyFileDownloader @Inject constructor(
val missingHours = determineMissingHours(availableCountries, hourItemLimit)
Timber.tag(TAG).d("Downloading missing hours: %s", missingHours)
+ requireStorageSpace(missingHours)
+
val hourDownloads = missingHours.flatMap { country ->
country.hourData.flatMap { (day, missingHours) ->
missingHours.map { missingHour ->
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/DeviceStorage.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/DeviceStorage.kt
index 75caa3780..412d5900f 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/DeviceStorage.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/DeviceStorage.kt
@@ -115,6 +115,15 @@ class DeviceStorage @Inject constructor(
suspend fun checkSpacePrivateStorage(requiredBytes: Long = -1L): CheckResult =
checkSpace(privateStorage, requiredBytes)
+ /**
+ * Like **[checkSpacePrivateStorage]** but throws **[InsufficientStorageException]**
+ * if not enough is available
+ */
+ suspend fun requireSpacePrivateStorage(requiredBytes: Long = -1L): CheckResult =
+ checkSpace(privateStorage, requiredBytes).apply {
+ if (!isSpaceAvailable) throw InsufficientStorageException(this)
+ }
+
data class CheckResult(
val path: File,
val isSpaceAvailable: Boolean,
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/CountryDataTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/CountryDataTest.kt
index 199ddb20c..b936f5330 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/CountryDataTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/CountryDataTest.kt
@@ -238,4 +238,36 @@ class CountryDataTest : BaseTest() {
cd.toMissingHours(cachedHours) shouldBe null
}
+ @Test
+ fun `calculate approximate required space for day data`() {
+ CountryDays(LocationCode("DE"), emptyList()).approximateSizeInBytes shouldBe 0
+ CountryDays(
+ LocationCode("DE"),
+ listOf(LocalDate.parse("2222-12-30"))
+ ).approximateSizeInBytes shouldBe 512 * 1024L
+ CountryDays(
+ LocationCode("DE"),
+ listOf(LocalDate.parse("2222-12-30"), LocalDate.parse("2222-12-31"))
+ ).approximateSizeInBytes shouldBe 2 * 512 * 1024L
+ }
+
+ @Test
+ fun `calculate approximate required space for day hour`() {
+ CountryHours(LocationCode("DE"), emptyMap()).approximateSizeInBytes shouldBe 0
+ CountryHours(
+ LocationCode("DE"),
+ mapOf(LocalDate.parse("2222-12-30") to listOf(LocalTime.parse("23:00")))
+ ).approximateSizeInBytes shouldBe 22 * 1024L
+ CountryHours(
+ LocationCode("DE"),
+ mapOf(
+ LocalDate.parse("2222-12-30") to listOf(LocalTime.parse("23:00")),
+ LocalDate.parse("2222-12-31") to listOf(
+ LocalTime.parse("22:00"),
+ LocalTime.parse("23:00")
+ )
+
+ )
+ ).approximateSizeInBytes shouldBe 3 * 22 * 1024L
+ }
}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloaderTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloaderTest.kt
index f63d93257..95190ac1a 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloaderTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloaderTest.kt
@@ -70,7 +70,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
LocationCode("DE"),
LocationCode("NL")
)
- coEvery { deviceStorage.checkSpacePrivateStorage(any()) } returns mockk<DeviceStorage.CheckResult>().apply {
+ coEvery { deviceStorage.requireSpacePrivateStorage(any()) } returns mockk<DeviceStorage.CheckResult>().apply {
every { isSpaceAvailable } returns true
}
@@ -220,7 +220,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `storage is checked before fetching`() {
+ fun `wanted country list is empty, day mode`() {
val downloader = createDownloader()
runBlocking {
downloader.asyncFetchKeyFiles(emptyList()) shouldBe emptyList()
@@ -228,12 +228,39 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `fetching is aborted if not enough free storage`() {
- coEvery { deviceStorage.checkSpacePrivateStorage(any()) } returns mockk<DeviceStorage.CheckResult>().apply {
- every { isSpaceAvailable } returns false
- every { freeBytes } returns 1337L
- every { requiredBytes } returns 5000L
+ fun `wanted country list is empty, hour mode`() {
+ every { CWADebug.isDebugBuildOrMode } returns true
+ every { settings.isLast3HourModeEnabled } returns true
+
+ val downloader = createDownloader()
+ runBlocking {
+ downloader.asyncFetchKeyFiles(emptyList()) shouldBe emptyList()
}
+ }
+
+ @Test
+ fun `fetching is aborted in day if not enough free storage`() {
+ coEvery { deviceStorage.requireSpacePrivateStorage(1048576L) } throws InsufficientStorageException(
+ mockk(relaxed = true)
+ )
+
+ val downloader = createDownloader()
+
+ runBlocking {
+ shouldThrow<InsufficientStorageException> {
+ downloader.asyncFetchKeyFiles(listOf(LocationCode("DE")))
+ }
+ }
+ }
+
+ @Test
+ fun `fetching is aborted in hour if not enough free storage`() {
+ every { CWADebug.isDebugBuildOrMode } returns true
+ every { settings.isLast3HourModeEnabled } returns true
+
+ coEvery { deviceStorage.requireSpacePrivateStorage(67584L) } throws InsufficientStorageException(
+ mockk(relaxed = true)
+ )
val downloader = createDownloader()
@@ -295,6 +322,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
keyRepoData.size shouldBe 4
keyRepoData.values.forEach { it.isDownloadComplete shouldBe true }
+ coVerify { deviceStorage.requireSpacePrivateStorage(2097152L) }
}
@Test
@@ -340,6 +368,8 @@ class KeyFileDownloaderTest : BaseIOTest() {
coVerify(exactly = 2) { keyCache.createCacheEntry(any(), any(), any(), any()) }
coVerify(exactly = 2) { keyCache.markKeyComplete(any(), any()) }
+
+ coVerify { deviceStorage.requireSpacePrivateStorage(1048576L) }
}
@Test
@@ -473,6 +503,8 @@ class KeyFileDownloaderTest : BaseIOTest() {
keyRepoData.size shouldBe 6
keyRepoData.values.forEach { it.isDownloadComplete shouldBe true }
+
+ coVerify { deviceStorage.requireSpacePrivateStorage(135168L) }
}
@Test
@@ -539,6 +571,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
any()
)
}
+ coVerify { deviceStorage.requireSpacePrivateStorage(90112L) }
}
@Test
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/storage/DeviceStorageTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/storage/DeviceStorageTest.kt
index eed7854bf..2b26304d3 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/storage/DeviceStorageTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/storage/DeviceStorageTest.kt
@@ -171,6 +171,9 @@ class DeviceStorageTest : BaseIOTest() {
requiredBytes = Long.MAX_VALUE,
totalBytes = defaultTotalSpace
)
+ shouldThrow<InsufficientStorageException> {
+ deviceStorage.requireSpacePrivateStorage(Long.MAX_VALUE)
+ }
}
}
--
GitLab