Move backup of APKs of opt-out apps to after uploading @pm@ DB

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()) {

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(

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())