Move APK backup from BackupCoordinator to new ApkBackupManager

This is a preparation for doing APK backup ourselves in a worker and not hacked into the backup transport. The latter was prone to timeouts by the AOSP backup API. With a worker, we have a bit more control and can schedule backups ourselves.

app/src/androidTest/java/com/stevesoltys/seedvault/e2e/LargeBackupTestBase.kt

@@ -179,7 +179,7 @@ internal interface LargeBackupTestBase : LargeTestBase {
         clearMocks(spyBackupNotificationManager)
 
         every {
-            spyBackupNotificationManager.onBackupFinished(any(), any(), any())
+            spyBackupNotificationManager.onBackupFinished(any(), any(), any(), any())
         } answers {
             val success = firstArg<Boolean>()
             assert(success) { "Backup failed." }

app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt

@@ -14,9 +14,7 @@ import com.stevesoltys.seedvault.crypto.Crypto
 import com.stevesoltys.seedvault.encodeBase64
 import com.stevesoltys.seedvault.header.VERSION
 import com.stevesoltys.seedvault.metadata.PackageState.APK_AND_DATA
-import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.NO_DATA
-import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import com.stevesoltys.seedvault.settings.SettingsManager
 import com.stevesoltys.seedvault.transport.backup.isSystemApp
 import java.io.FileNotFoundException
@@ -36,7 +34,7 @@ internal class MetadataManager(
     private val crypto: Crypto,
     private val metadataWriter: MetadataWriter,
     private val metadataReader: MetadataReader,
-    private val settingsManager: SettingsManager
+    private val settingsManager: SettingsManager,
 ) {
 
     private val uninitializedMetadata = BackupMetadata(token = 0L, salt = "")
@@ -76,42 +74,25 @@ internal class MetadataManager(
     /**
      * Call this after a package's APK has been backed up successfully.
      *
-     * It updates the packages' metadata
-     * and writes it encrypted to the given [OutputStream] as well as the internal cache.
-     *
-     * Closing the [OutputStream] is the responsibility of the caller.
+     * It updates the packages' metadata to the internal cache.
+     * You still need to call [uploadMetadata] to persist all local modifications.
      */
     @Synchronized
     @Throws(IOException::class)
     fun onApkBackedUp(
         packageInfo: PackageInfo,
         packageMetadata: PackageMetadata,
-        metadataOutputStream: OutputStream,
     ) {
         val packageName = packageInfo.packageName
         metadata.packageMetadataMap[packageName]?.let {
             check(packageMetadata.version != null) {
                 "APK backup returned version null"
             }
-            check(it.version == null || it.version < packageMetadata.version) {
-                "APK backup backed up the same or a smaller version:" +
-                    "was ${it.version} is ${packageMetadata.version}"
-            }
         }
         val oldPackageMetadata = metadata.packageMetadataMap[packageName]
             ?: PackageMetadata()
-        // only allow state change if backup of this package is not allowed,
-        // because we need to change from the default of UNKNOWN_ERROR here,
-        // but otherwise don't want to modify the state since set elsewhere.
-        val newState =
-            if (packageMetadata.state == NOT_ALLOWED || packageMetadata.state == WAS_STOPPED) {
-                packageMetadata.state
-            } else {
-                oldPackageMetadata.state
-            }
-        modifyMetadata(metadataOutputStream) {
+        modifyCachedMetadata {
             metadata.packageMetadataMap[packageName] = oldPackageMetadata.copy(
-                state = newState,
                 system = packageInfo.isSystemApp(),
                 version = packageMetadata.version,
                 installer = packageMetadata.installer,
@@ -192,6 +173,61 @@ internal class MetadataManager(
         }
     }
 
+    /**
+     * Call this for all packages we can not back up for some reason.
+     *
+     * It updates the packages' local metadata.
+     * You still need to call [uploadMetadata] to persist all local modifications.
+     */
+    @Synchronized
+    @Throws(IOException::class)
+    internal fun onPackageDoesNotGetBackedUp(
+        packageInfo: PackageInfo,
+        packageState: PackageState,
+    ) = modifyCachedMetadata {
+        val packageName = packageInfo.packageName
+        if (metadata.packageMetadataMap.containsKey(packageName)) {
+            metadata.packageMetadataMap[packageName]!!.state = packageState
+        } else {
+            metadata.packageMetadataMap[packageName] = PackageMetadata(
+                time = 0L,
+                state = packageState,
+                system = packageInfo.isSystemApp(),
+            )
+        }
+    }
+
+    /**
+     * Uploads metadata to given [metadataOutputStream] after performing local modifications.
+     */
+    @Synchronized
+    @Throws(IOException::class)
+    fun uploadMetadata(metadataOutputStream: OutputStream) {
+        val oldMetadata = metadata.copy()
+        try {
+            metadataWriter.write(metadata, metadataOutputStream)
+        } catch (e: IOException) {
+            Log.w(TAG, "Error writing metadata to storage", e)
+            // revert metadata and do not write it to cache
+            metadata = oldMetadata
+            throw IOException(e)
+        }
+    }
+
+    @Throws(IOException::class)
+    private fun modifyCachedMetadata(modFun: () -> Unit) {
+        val oldMetadata = metadata.copy()
+        try {
+            modFun.invoke()
+            writeMetadataToCache()
+        } catch (e: IOException) {
+            Log.w(TAG, "Error writing metadata to storage", e)
+            // revert metadata and do not write it to cache
+            metadata = oldMetadata
+            throw IOException(e)
+        }
+    }
+
     @Throws(IOException::class)
     private fun modifyMetadata(metadataOutputStream: OutputStream, modFun: () -> Unit) {
         val oldMetadata = metadata.copy()

app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt

@@ -15,9 +15,9 @@ import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED_SYSTEM_A
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.IN_PROGRESS
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.QUEUED
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.SUCCEEDED
-import com.stevesoltys.seedvault.transport.backup.copyStreamsAndGetHash
-import com.stevesoltys.seedvault.transport.backup.getSignatures
 import com.stevesoltys.seedvault.transport.backup.isSystemApp
+import com.stevesoltys.seedvault.worker.copyStreamsAndGetHash
+import com.stevesoltys.seedvault.worker.getSignatures
 import kotlinx.coroutines.TimeoutCancellationException
 import kotlinx.coroutines.flow.FlowCollector
 import kotlinx.coroutines.flow.flow

app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransport.kt

@@ -130,8 +130,8 @@ class ConfigurableBackupTransport internal constructor(private val context: Cont
         return backupCoordinator.isAppEligibleForBackup(targetPackage, isFullBackup)
     }
 
-    override fun getBackupQuota(packageName: String, isFullBackup: Boolean): Long = runBlocking {
-        backupCoordinator.getBackupQuota(packageName, isFullBackup)
+    override fun getBackupQuota(packageName: String, isFullBackup: Boolean): Long {
+        return backupCoordinator.getBackupQuota(packageName, isFullBackup)
     }
 
     override fun clearBackupData(packageInfo: PackageInfo): Int = runBlocking {

app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt

@@ -13,21 +13,17 @@ import android.app.backup.BackupTransport.TRANSPORT_QUOTA_EXCEEDED
 import android.app.backup.RestoreSet
 import android.content.Context
 import android.content.pm.PackageInfo
-import android.content.pm.PackageManager.GET_SIGNING_CERTIFICATES
 import android.os.ParcelFileDescriptor
 import android.util.Log
-import androidx.annotation.VisibleForTesting
 import androidx.annotation.WorkerThread
 import com.stevesoltys.seedvault.Clock
 import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.metadata.BackupType
 import com.stevesoltys.seedvault.metadata.MetadataManager
 import com.stevesoltys.seedvault.metadata.PackageState
-import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.NO_DATA
 import com.stevesoltys.seedvault.metadata.PackageState.QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
-import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import com.stevesoltys.seedvault.plugins.StoragePlugin
 import com.stevesoltys.seedvault.plugins.saf.FILE_BACKUP_METADATA
 import com.stevesoltys.seedvault.settings.SettingsManager
@@ -65,7 +61,6 @@ internal class BackupCoordinator(
     private val plugin: StoragePlugin,
     private val kv: KVBackup,
     private val full: FullBackup,
-    private val apkBackup: ApkBackup,
     private val clock: Clock,
     private val packageService: PackageService,
     private val metadataManager: MetadataManager,
@@ -156,13 +151,7 @@ internal class BackupCoordinator(
      *                      otherwise for key-value backup.
      * @return Current limit on backup size in bytes.
      */
-    suspend fun getBackupQuota(packageName: String, isFullBackup: Boolean): Long {
-        if (packageName != MAGIC_PACKAGE_MANAGER) {
-            // try to back up APK here as later methods are sometimes not called
-            // TODO move this into BackupWorker
-            backUpApk(context.packageManager.getPackageInfo(packageName, GET_SIGNING_CERTIFICATES))
-        }
-
+    fun getBackupQuota(packageName: String, isFullBackup: Boolean): Long {
         // report back quota
         Log.i(TAG, "Get backup quota for $packageName. Is full backup: $isFullBackup.")
         val quota = if (isFullBackup) full.getQuota() else kv.getQuota()
@@ -369,27 +358,16 @@ internal class BackupCoordinator(
             // tell K/V backup to finish
             var result = kv.finishBackup()
             if (result == TRANSPORT_OK) {
-                val isPmBackup = packageName == MAGIC_PACKAGE_MANAGER
+                val isNormalBackup = packageName != MAGIC_PACKAGE_MANAGER
                 // call onPackageBackedUp for @pm@ only if we can do backups right now
-                if (!isPmBackup || settingsManager.canDoBackupNow()) {
+                if (isNormalBackup || settingsManager.canDoBackupNow()) {
                     try {
                         onPackageBackedUp(packageInfo, BackupType.KV, size)
                     } catch (e: Exception) {
                         Log.e(TAG, "Error calling onPackageBackedUp for $packageName", e)
                         result = TRANSPORT_PACKAGE_REJECTED
                     }
                 }
-                // hook in here to back up APKs of apps that are otherwise not allowed for backup
-                // TODO move this into BackupWorker
-                if (isPmBackup && settingsManager.canDoBackupNow()) {
-                    try {
-                        backUpApksOfNotBackedUpPackages()
-                    } catch (e: Exception) {
-                        Log.e(TAG, "Error backing up APKs of opt-out apps: ", e)
-                        // We are re-throwing this, because we want to know about problems here
-                        throw e
-                    }
-                }
             }
             result
         }
@@ -418,65 +396,6 @@ internal class BackupCoordinator(
         else -> throw IllegalStateException("Unexpected state in finishBackup()")
     }
 
-    @VisibleForTesting
-    internal suspend fun backUpApksOfNotBackedUpPackages() {
-        Log.d(TAG, "Checking if APKs of opt-out apps need backup...")
-        val notBackedUpPackages = packageService.notBackedUpPackages
-        notBackedUpPackages.forEachIndexed { i, packageInfo ->
-            val packageName = packageInfo.packageName
-            try {
-                nm.onOptOutAppBackup(packageName, i + 1, notBackedUpPackages.size)
-                val packageState = if (packageInfo.isStopped()) WAS_STOPPED else NOT_ALLOWED
-                val wasBackedUp = backUpApk(packageInfo, packageState)
-                if (wasBackedUp) {
-                    Log.d(TAG, "Was backed up: $packageName")
-                } else {
-                    Log.d(TAG, "Not backed up: $packageName - ${packageState.name}")
-                    val packageMetadata =
-                        metadataManager.getPackageMetadata(packageName)
-                    val oldPackageState = packageMetadata?.state
-                    if (oldPackageState != packageState) {
-                        Log.i(
-                            TAG, "Package $packageName was in $oldPackageState" +
-                                ", update to $packageState"
-                        )
-                        plugin.getMetadataOutputStream().use {
-                            metadataManager.onPackageBackupError(packageInfo, packageState, it)
-                        }
-                    }
-                }
-            } catch (e: IOException) {
-                Log.e(TAG, "Error backing up opt-out APK of $packageName", e)
-            }
-        }
-    }
-
-    /**
-     * Backs up an APK for the given [PackageInfo].
-     *
-     * @return true if a backup was performed and false if no backup was needed or it failed.
-     */
-    private suspend fun backUpApk(
-        packageInfo: PackageInfo,
-        packageState: PackageState = UNKNOWN_ERROR,
-    ): Boolean {
-        val packageName = packageInfo.packageName
-        return try {
-            apkBackup.backupApkIfNecessary(packageInfo, packageState) { name ->
-                val token = settingsManager.getToken() ?: throw IOException("no current token")
-                plugin.getOutputStream(token, name)
-            }?.let { packageMetadata ->
-                plugin.getMetadataOutputStream().use {
-                    metadataManager.onApkBackedUp(packageInfo, packageMetadata, it)
-                }
-                true
-            } ?: false
-        } catch (e: IOException) {
-            Log.e(TAG, "Error while writing APK or metadata for $packageName", e)
-            false
-        }
-    }
-
     private suspend fun onPackageBackedUp(packageInfo: PackageInfo, type: BackupType, size: Long?) {
         plugin.getMetadataOutputStream().use {
             metadataManager.onPackageBackedUp(packageInfo, type, size, it)

app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt

@@ -1,5 +1,6 @@
 package com.stevesoltys.seedvault.transport.backup
 
+import com.stevesoltys.seedvault.worker.ApkBackup
 import org.koin.android.ext.koin.androidContext
 import org.koin.dsl.module
 
@@ -45,7 +46,6 @@ val backupModule = module {
             plugin = get(),
             kv = get(),
             full = get(),
-            apkBackup = get(),
             clock = get(),
             packageService = get(),
             metadataManager = get(),

app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupRequester.kt

@@ -39,7 +39,6 @@ internal class BackupRequester(
         context = context,
         backupRequester = this,
         requestedPackages = packages.size,
-        appTotals = packageService.expectedAppTotals,
     )
     private val monitor = BackupMonitor()