diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt index 62c21b6a..ef12f764 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt @@ -270,13 +270,7 @@ internal class BackupCoordinator( } val token = settingsManager.getToken() ?: error("no token in performFullBackup") val salt = metadataManager.salt - val result = kv.performBackup(packageInfo, data, flags, token, salt) - if (result == TRANSPORT_OK && packageName == MAGIC_PACKAGE_MANAGER) { - // TODO move to finish backup of @pm@ so we can upload the DB before - // hook in here to back up APKs of apps that are otherwise not allowed for backup - backUpApksOfNotBackedUpPackages() - } - return result + return kv.performBackup(packageInfo, data, flags, token, salt) } // ------------------------------------------------------------------------------------ @@ -392,10 +386,15 @@ internal class BackupCoordinator( "K/V backup has state, but full backup has dangling state as well" } // getCurrentPackage() not-null because we have state - onPackageBackedUp(kv.getCurrentPackage()!!, BackupType.KV) - val isPmBackup = kv.getCurrentPackage()!!.packageName == MAGIC_PACKAGE_MANAGER - kv.finishBackup() - // TODO move @pm@ backup hook here + val packageInfo = kv.getCurrentPackage()!! + onPackageBackedUp(packageInfo, BackupType.KV) + val isPmBackup = packageInfo.packageName == MAGIC_PACKAGE_MANAGER + val result = kv.finishBackup() + // hook in here to back up APKs of apps that are otherwise not allowed for backup + if (result == TRANSPORT_OK && isPmBackup) { + backUpApksOfNotBackedUpPackages() + } + result } full.hasState() -> { check(!kv.hasState()) { diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt index a4361257..9d32ca0a 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt @@ -186,20 +186,19 @@ internal class KVBackup( if (!dbManager.deleteDb(packageInfo.packageName)) throw IOException() } - @Throws(IOException::class) suspend fun finishBackup(): Int { val state = this.state ?: error("No state in finishBackup") val packageName = state.packageInfo.packageName Log.i(TAG, "Finish K/V Backup of $packageName") - try { + return try { if (state.needsUpload) uploadDb(state.token, state.name, packageName, state.db) + TRANSPORT_OK } catch (e: IOException) { - return TRANSPORT_ERROR + TRANSPORT_ERROR } finally { this.state = null } - return TRANSPORT_OK } /** @@ -235,11 +234,10 @@ internal class KVBackup( dbManager.getDbInputStream(packageName).use { inputStream -> inputStream.copyTo(gZipStream) } - // TODO remove log - Log.d(TAG, "=> Uploaded db file for $packageName") } } } + Log.d(TAG, "Uploaded db file for $packageName") } private class KVOperation( diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt index f4374c73..a169fe16 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt @@ -402,7 +402,7 @@ internal class BackupCoordinatorTest : BackupTest() { } @Test - fun `not allowed apps get their APKs backed up during @pm@ backup`() = runBlocking { + fun `not allowed apps get their APKs backed up after @pm@ backup`() = runBlocking { val packageInfo = PackageInfo().apply { packageName = MAGIC_PACKAGE_MANAGER } val notAllowedPackages = listOf( PackageInfo().apply { packageName = "org.example.1" }, @@ -422,6 +422,21 @@ internal class BackupCoordinatorTest : BackupTest() { coEvery { kv.performBackup(packageInfo, fileDescriptor, 0, token, salt) } returns TRANSPORT_OK + + assertEquals( + TRANSPORT_OK, + backup.performIncrementalBackup(packageInfo, fileDescriptor, 0) + ) + + // finish @pm@ backup + every { kv.hasState() } returns true + every { full.hasState() } returns false + every { kv.getCurrentPackage() } returns pmPackageInfo + every { + metadataManager.onPackageBackedUp(pmPackageInfo, BackupType.KV, metadataOutputStream) + } just Runs + coEvery { kv.finishBackup() } returns TRANSPORT_OK + // now check if we have opt-out apps that we need to back up APKs for every { packageService.notBackedUpPackages } returns notAllowedPackages // update notification @@ -465,7 +480,7 @@ internal class BackupCoordinatorTest : BackupTest() { } just Runs every { metadataOutputStream.close() } just Runs - assertEquals(TRANSPORT_OK, backup.performIncrementalBackup(packageInfo, fileDescriptor, 0)) + assertEquals(TRANSPORT_OK, backup.finishBackup()) coVerify { apkBackup.backupApkIfNecessary(notAllowedPackages[0], NOT_ALLOWED, any())