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
+     * It updates the packages' metadata to the internal cache.
-     * and writes it encrypted to the given [OutputStream] as well as the internal cache.
+     * You still need to call [uploadMetadata] to persist all local modifications.
-     *
-     * Closing the [OutputStream] is the responsibility of the caller.
      */
     @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,
+        modifyCachedMetadata {
-        // 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) {
             metadata.packageMetadataMap[packageName] = oldPackageMetadata.copy(
-                state = newState,
                 system = packageInfo.isSystemApp(),
                 version = packageMetadata.version,
                 installer = packageMetadata.installer,
@@ -192,9 +173,60 @@ 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) {
+        metadataWriter.write(metadata, metadataOutputStream)
+    }
+
+    @Throws(IOException::class)
+    private fun modifyCachedMetadata(modFun: () -> Unit) {
+        val oldMetadata = metadata.copy( // copy map, otherwise it will re-use same reference
+            packageMetadataMap = PackageMetadataMap(metadata.packageMetadataMap),
+        )
+        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()
+        val oldMetadata = metadata.copy( // copy map, otherwise it will re-use same reference
+            packageMetadataMap = PackageMetadataMap(metadata.packageMetadataMap),
+        )
         try {
             modFun.invoke()
             metadataWriter.write(metadata, metadataOutputStream)

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 {
+    override fun getBackupQuota(packageName: String, isFullBackup: Boolean): Long {
-        backupCoordinator.getBackupQuota(packageName, isFullBackup)
+        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 {
+    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))
-        }
-
         // 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,9 +358,9 @@ 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) {
@@ -379,17 +368,6 @@ internal class BackupCoordinator(
                         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

@@ -13,14 +13,6 @@ val backupModule = module {
             plugin = get()
         )
     }
-    single {
-        ApkBackup(
-            pm = androidContext().packageManager,
-            crypto = get(),
-            settingsManager = get(),
-            metadataManager = get()
-        )
-    }
     single<KvDbManager> { KvDbManagerImpl(androidContext()) }
     single {
         KVBackup(
@@ -45,7 +37,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()
 

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

@@ -73,6 +73,22 @@ internal class PackageService(
             return packageArray
         }
 
+    /**
+     * A list of packages that is installed and that we need to re-install for restore,
+     * such as user-installed packages or updated system apps.
+     */
+    val allUserPackages: List<PackageInfo>
+        @WorkerThread
+        get() {
+            // We need the GET_SIGNING_CERTIFICATES flag here,
+            // because the package info is used by [ApkBackup] which needs signing info.
+            return packageManager.getInstalledPackages(GET_SIGNING_CERTIFICATES)
+                .filter { packageInfo -> // only apps that are:
+                    !packageInfo.isNotUpdatedSystemApp() && // not vanilla system apps
+                        packageInfo.packageName != context.packageName // not this app
+                }
+        }
+
     /**
      * A list of packages that will not be backed up,
      * because they are currently force-stopped for example.
@@ -90,9 +106,9 @@ internal class PackageService(
                 }.sortedBy { packageInfo ->
                     packageInfo.packageName
                 }.also { notAllowed ->
-                    // log eligible packages
+                    // log packages that don't get backed up
                     if (Log.isLoggable(TAG, INFO)) {
-                        Log.i(TAG, "${notAllowed.size} apps do not allow backup:")
+                        Log.i(TAG, "${notAllowed.size} apps do not get backed up:")
                         logPackages(notAllowed.map { it.packageName })
                     }
                 }
@@ -124,22 +140,6 @@ internal class PackageService(
             }
         }
 
-    val expectedAppTotals: ExpectedAppTotals
-        @WorkerThread
-        get() {
-            var appsTotal = 0
-            var appsNotIncluded = 0
-            packageManager.getInstalledPackages(GET_INSTRUMENTATION).forEach { packageInfo ->
-                if (packageInfo.isUserVisible(context)) {
-                    appsTotal++
-                    if (packageInfo.doesNotGetBackedUp()) {
-                        appsNotIncluded++
-                    }
-                }
-            }
-            return ExpectedAppTotals(appsTotal, appsNotIncluded)
-        }
-
     fun getVersionName(packageName: String): String? = try {
         packageManager.getPackageInfo(packageName, 0).versionName
     } catch (e: PackageManager.NameNotFoundException) {
@@ -208,19 +208,6 @@ internal class PackageService(
     }
 }
 
-internal data class ExpectedAppTotals(
-    /**
-     * The total number of non-system apps eligible for backup.
-     */
-    val appsTotal: Int,
-    /**
-     * The number of non-system apps that do not get backed up.
-     * These are included here, because we'll at least back up their APKs,
-     * so at least the app itself does get restored.
-     */
-    val appsNotGettingBackedUp: Int,
-)
-
 internal fun PackageInfo.isUserVisible(context: Context): Boolean {
     if (packageName == MAGIC_PACKAGE_MANAGER || applicationInfo == null) return false
     return !isNotUpdatedSystemApp() && instrumentation == null && packageName != context.packageName

app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt

@@ -1,6 +1,7 @@
 package com.stevesoltys.seedvault.ui.notification
 
 import android.annotation.SuppressLint
+import android.app.Notification
 import android.app.NotificationChannel
 import android.app.NotificationManager
 import android.app.NotificationManager.IMPORTANCE_DEFAULT
@@ -26,19 +27,20 @@ import com.stevesoltys.seedvault.restore.EXTRA_PACKAGE_NAME
 import com.stevesoltys.seedvault.restore.REQUEST_CODE_UNINSTALL
 import com.stevesoltys.seedvault.settings.ACTION_APP_STATUS_LIST
 import com.stevesoltys.seedvault.settings.SettingsActivity
-import com.stevesoltys.seedvault.transport.backup.ExpectedAppTotals
 import kotlin.math.min
 
 private const val CHANNEL_ID_OBSERVER = "NotificationBackupObserver"
+private const val CHANNEL_ID_APK = "NotificationApkBackup"
 private const val CHANNEL_ID_SUCCESS = "NotificationBackupSuccess"
 private const val CHANNEL_ID_ERROR = "NotificationError"
 private const val CHANNEL_ID_RESTORE_ERROR = "NotificationRestoreError"
 private const val NOTIFICATION_ID_OBSERVER = 1
-private const val NOTIFICATION_ID_SUCCESS = 2
+internal const val NOTIFICATION_ID_APK = 2
-private const val NOTIFICATION_ID_ERROR = 3
+private const val NOTIFICATION_ID_SUCCESS = 3
-private const val NOTIFICATION_ID_RESTORE_ERROR = 4
+private const val NOTIFICATION_ID_ERROR = 4
-private const val NOTIFICATION_ID_BACKGROUND = 5
+private const val NOTIFICATION_ID_RESTORE_ERROR = 5
-private const val NOTIFICATION_ID_NO_MAIN_KEY_ERROR = 6
+private const val NOTIFICATION_ID_BACKGROUND = 6
+private const val NOTIFICATION_ID_NO_MAIN_KEY_ERROR = 7
 
 private val TAG = BackupNotificationManager::class.java.simpleName
 
@@ -46,18 +48,11 @@ internal class BackupNotificationManager(private val context: Context) {
 
     private val nm = context.getSystemService(NotificationManager::class.java)!!.apply {
         createNotificationChannel(getObserverChannel())
+        createNotificationChannel(getApkChannel())
         createNotificationChannel(getSuccessChannel())
         createNotificationChannel(getErrorChannel())
         createNotificationChannel(getRestoreErrorChannel())
     }
-    private var expectedApps: Int? = null
-    private var expectedOptOutApps: Int? = null
-    private var expectedAppTotals: ExpectedAppTotals? = null
-
-    /**
-     * Used as a (temporary) hack to fix progress reporting when fake d2d is enabled.
-     */
-    private var optOutAppsDone = false
 
     private fun getObserverChannel(): NotificationChannel {
         val title = context.getString(R.string.notification_channel_title)
@@ -66,6 +61,13 @@ internal class BackupNotificationManager(private val context: Context) {
         }
     }
 
+    private fun getApkChannel(): NotificationChannel {
+        val title = context.getString(R.string.notification_apk_channel_title)
+        return NotificationChannel(CHANNEL_ID_APK, title, IMPORTANCE_LOW).apply {
+            enableVibration(false)
+        }
+    }
+
     private fun getSuccessChannel(): NotificationChannel {
         val title = context.getString(R.string.notification_success_channel_title)
         return NotificationChannel(CHANNEL_ID_SUCCESS, title, IMPORTANCE_LOW).apply {
@@ -84,57 +86,69 @@ internal class BackupNotificationManager(private val context: Context) {
     }
 
     /**
-     * Call this right after starting a backup.
+     * This should get called for each APK we are backing up.
      */
-    fun onBackupStarted(
+    fun onApkBackup(packageName: String, name: CharSequence, transferred: Int, expected: Int) {
-        expectedPackages: Int,
+        Log.i(TAG, "$transferred/$expected - $name ($packageName)")
-        appTotals: ExpectedAppTotals,
+        val text = context.getString(R.string.notification_apk_text, name)
-    ) {
+        val notification = getApkBackupNotification(text, transferred, expected)
-        updateBackupNotification(
+        nm.notify(NOTIFICATION_ID_APK, notification)
-            infoText = "", // This passes quickly, no need to show something here
-            transferred = 0,
-            expected = appTotals.appsTotal
-        )
-        expectedApps = expectedPackages
-        expectedOptOutApps = appTotals.appsNotGettingBackedUp
-        expectedAppTotals = appTotals
-        optOutAppsDone = false
-        Log.i(TAG, "onBackupStarted $expectedApps + $expectedOptOutApps = ${appTotals.appsTotal}")
     }
 
     /**
-     * This should get called before [onBackupUpdate].
+     * This should get called for recording apps we don't back up.
-     * In case of d2d backups, this actually gets called some time after
-     * some apps were already backed up, so [onBackupUpdate] was called several times.
      */
-    fun onOptOutAppBackup(packageName: String, transferred: Int, expected: Int) {
+    fun onAppsNotBackedUp() {
-        if (optOutAppsDone) return
+        Log.i(TAG, "onAppsNotBackedUp")
+        val notification =
+            getApkBackupNotification(context.getString(R.string.notification_apk_not_backed_up))
+        nm.notify(NOTIFICATION_ID_APK, notification)
+    }
 
-        val text = "APK for $packageName"
+    fun getApkBackupNotification(
-        if (expectedApps == null) {
+        text: String?,
-            updateBackgroundBackupNotification(text)
+        expected: Int = 0,
-        } else {
+        transferred: Int = 0,
-            updateBackupNotification(text, transferred, expected + (expectedApps ?: 0))
+    ): Notification = Builder(context, CHANNEL_ID_APK).apply {
-            if (expectedOptOutApps != null && expectedOptOutApps != expected) {
+        setSmallIcon(R.drawable.ic_cloud_upload)
-                Log.w(TAG, "Number of packages not getting backed up mismatch: " +
+        setContentTitle(context.getString(R.string.notification_title))
-                    "$expectedOptOutApps != $expected")
+        setContentText(text)
-            }
+        setOngoing(true)
-            expectedOptOutApps = expected
+        setShowWhen(false)
-            if (transferred == expected) optOutAppsDone = true
+        setWhen(System.currentTimeMillis())
-        }
+        setProgress(expected, transferred, false)
+        priority = PRIORITY_DEFAULT
+        foregroundServiceBehavior = FOREGROUND_SERVICE_IMMEDIATE
+    }.build()
+
+    /**
+     * Call after [onApkBackup] or [onAppsNotBackedUp] were called.
+     */
+    fun onApkBackupDone() {
+        nm.cancel(NOTIFICATION_ID_APK)
+    }
+
+    /**
+     * Call this right after starting a backup.
+     */
+    fun onBackupStarted(expectedPackages: Int) {
+        updateBackupNotification(
+            infoText = "", // This passes quickly, no need to show something here
+            transferred = 0,
+            expected = expectedPackages
+        )
+        Log.i(TAG, "onBackupStarted - Expecting $expectedPackages apps")
     }
 
     /**
      * In the series of notification updates,
-     * this type is is expected to get called after [onOptOutAppBackup].
+     * this type is is expected to get called after [onApkBackup].
      */
-    fun onBackupUpdate(app: CharSequence, transferred: Int) {
+    fun onBackupUpdate(app: CharSequence, transferred: Int, total: Int) {
-        val expected = expectedApps ?: error("expectedApps is null")
-        val addend = expectedOptOutApps ?: 0
         updateBackupNotification(
             infoText = app,
-            transferred = min(transferred + addend, expected + addend),
+            transferred = min(transferred, total),
-            expected = expected + addend
+            expected = total
         )
     }
 
@@ -197,11 +211,10 @@ internal class BackupNotificationManager(private val context: Context) {
         // }
     }
 
-    fun onBackupFinished(success: Boolean, numBackedUp: Int?, size: Long) {
+    fun onBackupFinished(success: Boolean, numBackedUp: Int?, total: Int, size: Long) {
         val titleRes =
             if (success) R.string.notification_success_title else R.string.notification_failed_title
-        val total = expectedAppTotals?.appsTotal
+        val contentText = if (numBackedUp == null) null else {
-        val contentText = if (numBackedUp == null || total == null) null else {
             val sizeStr = Formatter.formatShortFileSize(context, size)
             context.getString(R.string.notification_success_text, numBackedUp, total, sizeStr)
         }
@@ -224,10 +237,6 @@ internal class BackupNotificationManager(private val context: Context) {
         }.build()
         nm.cancel(NOTIFICATION_ID_OBSERVER)
         nm.notify(NOTIFICATION_ID_SUCCESS, notification)
-        // reset number of expected apps
-        expectedOptOutApps = null
-        expectedApps = null
-        expectedAppTotals = null
     }
 
     fun hasActiveBackupNotifications(): Boolean {

app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt

@@ -11,7 +11,6 @@ import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.R
 import com.stevesoltys.seedvault.metadata.MetadataManager
 import com.stevesoltys.seedvault.transport.backup.BackupRequester
-import com.stevesoltys.seedvault.transport.backup.ExpectedAppTotals
 import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
 
@@ -21,7 +20,6 @@ internal class NotificationBackupObserver(
     private val context: Context,
     private val backupRequester: BackupRequester,
     private val requestedPackages: Int,
-    appTotals: ExpectedAppTotals,
 ) : IBackupObserver.Stub(), KoinComponent {
 
     private val nm: BackupNotificationManager by inject()
@@ -31,8 +29,8 @@ internal class NotificationBackupObserver(
 
     init {
         // Inform the notification manager that a backup has started
-        // and inform about the expected numbers, so it can compute a total.
+        // and inform about the expected numbers of apps.
-        nm.onBackupStarted(requestedPackages, appTotals)
+        nm.onBackupStarted(requestedPackages)
     }
 
     /**
@@ -82,7 +80,7 @@ internal class NotificationBackupObserver(
             val success = status == 0
             val numBackedUp = if (success) metadataManager.getPackagesNumBackedUp() else null
             val size = if (success) metadataManager.getPackagesBackupSize() else 0L
-            nm.onBackupFinished(success, numBackedUp, size)
+            nm.onBackupFinished(success, numBackedUp, requestedPackages, size)
         }
     }
 
@@ -101,7 +99,7 @@ internal class NotificationBackupObserver(
             packageName
         }
         numPackages += 1
-        nm.onBackupUpdate(app, numPackages)
+        nm.onBackupUpdate(app, numPackages, requestedPackages)
     }
 
     private fun getAppName(packageId: String): CharSequence = getAppName(context, packageId)

app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackup.kt

@@ -1,4 +1,9 @@
-package com.stevesoltys.seedvault.transport.backup
+/*
+ * SPDX-FileCopyrightText: 2024 The Calyx Institute
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.stevesoltys.seedvault.worker
 
 import android.annotation.SuppressLint
 import android.content.pm.PackageInfo
@@ -13,8 +18,9 @@ import com.stevesoltys.seedvault.encodeBase64
 import com.stevesoltys.seedvault.metadata.ApkSplit
 import com.stevesoltys.seedvault.metadata.MetadataManager
 import com.stevesoltys.seedvault.metadata.PackageMetadata
-import com.stevesoltys.seedvault.metadata.PackageState
 import com.stevesoltys.seedvault.settings.SettingsManager
+import com.stevesoltys.seedvault.transport.backup.isNotUpdatedSystemApp
+import com.stevesoltys.seedvault.transport.backup.isTestOnly
 import java.io.File
 import java.io.FileInputStream
 import java.io.IOException
@@ -44,7 +50,6 @@ internal class ApkBackup(
     @SuppressLint("NewApi") // can be removed when minSdk is set to 30
     suspend fun backupApkIfNecessary(
         packageInfo: PackageInfo,
-        packageState: PackageState,
         streamGetter: suspend (name: String) -> OutputStream,
     ): PackageMetadata? {
         // do not back up @pm@
@@ -118,11 +123,10 @@ internal class ApkBackup(
         val splits =
             if (packageInfo.splitNames == null) null else backupSplitApks(packageInfo, streamGetter)
 
-        Log.d(TAG, "Backed up new APK of $packageName with version $version.")
+        Log.d(TAG, "Backed up new APK of $packageName with version ${packageInfo.versionName}.")
 
         // return updated metadata
-        return PackageMetadata(
+        return packageMetadata.copy(
-            state = packageState,
             version = version,
             installer = pm.getInstallSourceInfo(packageName).installingPackageName,
             splits = splits,

app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackupManager.kt

@@ -0,0 +1,116 @@
+/*
+ * SPDX-FileCopyrightText: 2024 The Calyx Institute
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.stevesoltys.seedvault.worker
+
+import android.content.Context
+import android.content.pm.PackageInfo
+import android.util.Log
+import com.stevesoltys.seedvault.metadata.MetadataManager
+import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
+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
+import com.stevesoltys.seedvault.transport.backup.PackageService
+import com.stevesoltys.seedvault.transport.backup.isStopped
+import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
+import com.stevesoltys.seedvault.ui.notification.getAppName
+import java.io.IOException
+import java.io.OutputStream
+
+internal class ApkBackupManager(
+    private val context: Context,
+    private val settingsManager: SettingsManager,
+    private val metadataManager: MetadataManager,
+    private val packageService: PackageService,
+    private val apkBackup: ApkBackup,
+    private val plugin: StoragePlugin,
+    private val nm: BackupNotificationManager,
+) {
+
+    companion object {
+        private val TAG = ApkBackupManager::class.simpleName
+    }
+
+    suspend fun backup() {
+        try {
+            // We may be backing up APKs of packages that don't get their data backed up.
+            // Since an APK backup does not change the [packageState], we first record it for all
+            // packages that don't get backed up.
+            recordNotBackedUpPackages()
+            // Now, if APK backups are enabled by the user, we back those up.
+            if (settingsManager.backupApks()) {
+                backUpApks()
+            }
+        } finally {
+            // upload all local changes only at the end, so we don't have to re-upload the metadata
+            plugin.getMetadataOutputStream().use { outputStream ->
+                metadataManager.uploadMetadata(outputStream)
+            }
+            nm.onApkBackupDone()
+        }
+    }
+
+    /**
+     * Goes through the list of all apps and uploads their APK, if needed.
+     */
+    private suspend fun backUpApks() {
+        val apps = packageService.allUserPackages
+        apps.forEachIndexed { i, packageInfo ->
+            val packageName = packageInfo.packageName
+            val name = getAppName(context, packageName)
+            nm.onApkBackup(packageName, name, i, apps.size)
+            backUpApk(packageInfo)
+        }
+    }
+
+    private fun recordNotBackedUpPackages() {
+        nm.onAppsNotBackedUp()
+        packageService.notBackedUpPackages.forEach { packageInfo ->
+            val packageName = packageInfo.packageName
+            try {
+                val packageState = if (packageInfo.isStopped()) WAS_STOPPED else NOT_ALLOWED
+                val packageMetadata = metadataManager.getPackageMetadata(packageName)
+                val oldPackageState = packageMetadata?.state
+                if (oldPackageState != packageState) {
+                    Log.i(
+                        TAG, "Package $packageName was in $oldPackageState" +
+                            ", update to $packageState"
+                    )
+                    metadataManager.onPackageDoesNotGetBackedUp(packageInfo, packageState)
+                }
+            } catch (e: IOException) {
+                Log.e(TAG, "Error storing new metadata for $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): Boolean {
+        val packageName = packageInfo.packageName
+        return try {
+            apkBackup.backupApkIfNecessary(packageInfo) { name ->
+                val token = settingsManager.getToken() ?: throw IOException("no current token")
+                plugin.getOutputStream(token, name)
+            }?.let { packageMetadata ->
+                metadataManager.onApkBackedUp(packageInfo, packageMetadata)
+                true
+            } ?: false
+        } catch (e: IOException) {
+            Log.e(TAG, "Error while writing APK for $packageName", e)
+            false
+        }
+    }
+
+    private suspend fun StoragePlugin.getMetadataOutputStream(token: Long? = null): OutputStream {
+        val t = token ?: settingsManager.getToken() ?: throw IOException("no current token")
+        return getOutputStream(t, FILE_BACKUP_METADATA)
+    }
+}

app/src/main/java/com/stevesoltys/seedvault/worker/WorkerModule.kt

@@ -0,0 +1,31 @@
+/*
+ * SPDX-FileCopyrightText: 2024 The Calyx Institute
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.stevesoltys.seedvault.worker
+
+import org.koin.android.ext.koin.androidContext
+import org.koin.dsl.module
+
+val workerModule = module {
+    single {
+        ApkBackup(
+            pm = androidContext().packageManager,
+            crypto = get(),
+            settingsManager = get(),
+            metadataManager = get()
+        )
+    }
+    single {
+        ApkBackupManager(
+            context = androidContext(),
+            settingsManager = get(),
+            metadataManager = get(),
+            packageService = get(),
+            apkBackup = get(),
+            plugin = get(),
+            nm = get()
+        )
+    }
+}

app/src/main/res/values/strings.xml

@@ -119,8 +119,11 @@
 
     <!-- Notification -->
     <string name="notification_channel_title">Backup notification</string>
+    <string name="notification_apk_channel_title">APK backup notification</string>
     <string name="notification_success_channel_title">Success notification</string>
     <string name="notification_title">Backup running</string>
+    <string name="notification_apk_text">Backing up APK of %s</string>
+    <string name="notification_apk_not_backed_up">Saving list of apps we can not back up.</string>
     <string name="notification_backup_already_running">Backup already in progress</string>
     <string name="notification_backup_disabled">Backup not enabled</string>
 

app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt

@@ -28,10 +28,12 @@ import io.mockk.verify
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Assert.fail
 import org.junit.Before
 import org.junit.Test
+import org.junit.jupiter.api.assertThrows
 import org.junit.runner.RunWith
 import org.koin.core.context.stopKoin
 import org.robolectric.annotation.Config
@@ -121,7 +123,7 @@ class MetadataManagerTest {
         expectReadFromCache()
         expectModifyMetadata(initialMetadata)
 
-        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata)
 
         assertEquals(packageMetadata, manager.getPackageMetadata(packageName))
 
@@ -144,7 +146,7 @@ class MetadataManagerTest {
         expectReadFromCache()
         expectModifyMetadata(initialMetadata)
 
-        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata)
 
         assertEquals(packageMetadata.copy(system = true), manager.getPackageMetadata(packageName))
 
@@ -171,9 +173,9 @@ class MetadataManagerTest {
         )
 
         expectReadFromCache()
-        expectModifyMetadata(initialMetadata)
+        expectWriteToCache(initialMetadata)
 
-        manager.onApkBackedUp(packageInfo, updatedPackageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, updatedPackageMetadata)
 
         assertEquals(updatedPackageMetadata, manager.getPackageMetadata(packageName))
 
@@ -184,7 +186,7 @@ class MetadataManagerTest {
     }
 
     @Test
-    fun `test onApkBackedUp() limits state changes`() {
+    fun `test onApkBackedUp() does not change package state`() {
         var version = Random.nextLong(Long.MAX_VALUE)
         var packageMetadata = PackageMetadata(
             version = version,
@@ -193,12 +195,12 @@ class MetadataManagerTest {
         )
 
         expectReadFromCache()
-        expectModifyMetadata(initialMetadata)
+        expectWriteToCache(initialMetadata)
         val oldState = UNKNOWN_ERROR
 
         // state doesn't change for APK_AND_DATA
         packageMetadata = packageMetadata.copy(version = ++version, state = APK_AND_DATA)
-        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata)
         assertEquals(
             packageMetadata.copy(state = oldState),
             manager.getPackageMetadata(packageName)
@@ -206,7 +208,7 @@ class MetadataManagerTest {
 
         // state doesn't change for QUOTA_EXCEEDED
         packageMetadata = packageMetadata.copy(version = ++version, state = QUOTA_EXCEEDED)
-        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata)
         assertEquals(
             packageMetadata.copy(state = oldState),
             manager.getPackageMetadata(packageName)
@@ -214,25 +216,25 @@ class MetadataManagerTest {
 
         // state doesn't change for NO_DATA
         packageMetadata = packageMetadata.copy(version = ++version, state = NO_DATA)
-        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata)
         assertEquals(
             packageMetadata.copy(state = oldState),
             manager.getPackageMetadata(packageName)
         )
 
-        // state DOES change for NOT_ALLOWED
+        // state doesn't change for NOT_ALLOWED
         packageMetadata = packageMetadata.copy(version = ++version, state = NOT_ALLOWED)
-        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata)
         assertEquals(
-            packageMetadata.copy(state = NOT_ALLOWED),
+            packageMetadata.copy(state = oldState),
             manager.getPackageMetadata(packageName)
         )
 
-        // state DOES change for WAS_STOPPED
+        // state doesn't change for WAS_STOPPED
         packageMetadata = packageMetadata.copy(version = ++version, state = WAS_STOPPED)
-        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata)
         assertEquals(
-            packageMetadata.copy(state = WAS_STOPPED),
+            packageMetadata.copy(state = oldState),
             manager.getPackageMetadata(packageName)
         )
 
@@ -242,6 +244,39 @@ class MetadataManagerTest {
         }
     }
 
+    @Test
+    fun `test onApkBackedUp() throws while writing local cache`() {
+        val packageMetadata = PackageMetadata(
+            time = 0L,
+            version = Random.nextLong(Long.MAX_VALUE),
+            installer = getRandomString(),
+            signatures = listOf("sig")
+        )
+
+        expectReadFromCache()
+
+        assertNull(manager.getPackageMetadata(packageName))
+
+        every { metadataWriter.encode(initialMetadata) } returns encodedMetadata
+        every {
+            context.openFileOutput(
+                METADATA_CACHE_FILE,
+                MODE_PRIVATE
+            )
+        } throws FileNotFoundException()
+
+        assertThrows<IOException> {
+            manager.onApkBackedUp(packageInfo, packageMetadata)
+        }
+
+        // metadata change got reverted
+        assertNull(manager.getPackageMetadata(packageName))
+
+        verify {
+            cacheInputStream.close()
+        }
+    }
+
     @Test
     fun `test onPackageBackedUp()`() {
         packageInfo.applicationInfo.flags = FLAG_SYSTEM
@@ -317,10 +352,7 @@ class MetadataManagerTest {
         }
 
         assertEquals(0L, manager.getLastBackupTime()) // time was reverted
-        assertEquals(
+        assertNull(manager.getPackageMetadata(packageName)) // no package metadata got added
-            initialMetadata.packageMetadataMap[packageName],
-            manager.getPackageMetadata(packageName)
-        )
 
         verify { cacheInputStream.close() }
     }
@@ -358,6 +390,70 @@ class MetadataManagerTest {
         }
     }
 
+    @Test
+    fun `test onPackageDoesNotGetBackedUp() updates state`() {
+        val updatedMetadata = initialMetadata.copy()
+        updatedMetadata.packageMetadataMap[packageName] = PackageMetadata(state = NOT_ALLOWED)
+
+        expectReadFromCache()
+        expectWriteToCache(updatedMetadata)
+
+        manager.onPackageDoesNotGetBackedUp(packageInfo, NOT_ALLOWED)
+
+        assertEquals(
+            updatedMetadata.packageMetadataMap[packageName],
+            manager.getPackageMetadata(packageName),
+        )
+    }
+
+    @Test
+    fun `test onPackageDoesNotGetBackedUp() creates new state`() {
+        val updatedMetadata = initialMetadata.copy()
+        updatedMetadata.packageMetadataMap[packageName] = PackageMetadata(state = WAS_STOPPED)
+        initialMetadata.packageMetadataMap.remove(packageName)
+
+        expectReadFromCache()
+        expectWriteToCache(updatedMetadata)
+
+        manager.onPackageDoesNotGetBackedUp(packageInfo, WAS_STOPPED)
+
+        assertEquals(
+            updatedMetadata.packageMetadataMap[packageName],
+            manager.getPackageMetadata(packageName),
+        )
+    }
+
+    @Test
+    fun `test onPackageBackupError() updates state`() {
+        val updatedMetadata = initialMetadata.copy()
+        updatedMetadata.packageMetadataMap[packageName] = PackageMetadata(state = NO_DATA)
+
+        expectReadFromCache()
+        expectModifyMetadata(updatedMetadata)
+
+        manager.onPackageBackupError(packageInfo, NO_DATA, storageOutputStream, BackupType.KV)
+    }
+
+    @Test
+    fun `test onPackageBackupError() inserts new package`() {
+        val updatedMetadata = initialMetadata.copy()
+        updatedMetadata.packageMetadataMap[packageName] = PackageMetadata(state = WAS_STOPPED)
+        initialMetadata.packageMetadataMap.remove(packageName)
+
+        expectReadFromCache()
+        expectModifyMetadata(updatedMetadata)
+
+        manager.onPackageBackupError(packageInfo, WAS_STOPPED, storageOutputStream)
+    }
+
+    @Test
+    fun `test uploadMetadata() uploads`() {
+        expectReadFromCache()
+        every { metadataWriter.write(initialMetadata, storageOutputStream) } just Runs
+
+        manager.uploadMetadata(storageOutputStream)
+    }
+
     @Test
     fun `test getBackupToken() on first run`() {
         every { context.openFileInput(METADATA_CACHE_FILE) } throws FileNotFoundException()
@@ -386,15 +482,7 @@ class MetadataManagerTest {
 
     private fun expectModifyMetadata(metadata: BackupMetadata) {
         every { metadataWriter.write(metadata, storageOutputStream) } just Runs
-        every { metadataWriter.encode(metadata) } returns encodedMetadata
+        expectWriteToCache(metadata)
-        every {
-            context.openFileOutput(
-                METADATA_CACHE_FILE,
-                MODE_PRIVATE
-            )
-        } returns cacheOutputStream
-        every { cacheOutputStream.write(encodedMetadata) } just Runs
-        every { cacheOutputStream.close() } just Runs
     }
 
     private fun expectReadFromCache() {
@@ -406,4 +494,16 @@ class MetadataManagerTest {
         every { cacheInputStream.close() } just Runs
     }
 
+    private fun expectWriteToCache(metadata: BackupMetadata) {
+        every { metadataWriter.encode(metadata) } returns encodedMetadata
+        every {
+            context.openFileOutput(
+                METADATA_CACHE_FILE,
+                MODE_PRIVATE
+            )
+        } returns cacheOutputStream
+        every { cacheOutputStream.write(encodedMetadata) } just Runs
+        every { cacheOutputStream.close() } just Runs
+    }
+
 }

app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt

@@ -10,12 +10,11 @@ import com.stevesoltys.seedvault.getRandomString
 import com.stevesoltys.seedvault.metadata.ApkSplit
 import com.stevesoltys.seedvault.metadata.PackageMetadata
 import com.stevesoltys.seedvault.metadata.PackageMetadataMap
-import com.stevesoltys.seedvault.metadata.PackageState
 import com.stevesoltys.seedvault.plugins.LegacyStoragePlugin
 import com.stevesoltys.seedvault.plugins.StoragePlugin
 import com.stevesoltys.seedvault.restore.RestorableBackup
 import com.stevesoltys.seedvault.transport.TransportTest
-import com.stevesoltys.seedvault.transport.backup.ApkBackup
+import com.stevesoltys.seedvault.worker.ApkBackup
 import io.mockk.coEvery
 import io.mockk.every
 import io.mockk.mockk
@@ -121,7 +120,7 @@ internal class ApkBackupRestoreTest : TransportTest() {
         every { crypto.getNameForApk(salt, packageName, splitName) } returns suffixName
         every { storagePlugin.providerPackageName } returns storageProviderPackageName
 
-        apkBackup.backupApkIfNecessary(packageInfo, PackageState.APK_AND_DATA, outputStreamGetter)
+        apkBackup.backupApkIfNecessary(packageInfo, outputStreamGetter)
 
         assertArrayEquals(apkBytes, outputStream.toByteArray())
         assertArrayEquals(splitBytes, splitOutputStream.toByteArray())

app/src/test/java/com/stevesoltys/seedvault/transport/CoordinatorIntegrationTest.kt

@@ -15,11 +15,9 @@ import com.stevesoltys.seedvault.header.MAX_SEGMENT_CLEARTEXT_LENGTH
 import com.stevesoltys.seedvault.metadata.BackupType
 import com.stevesoltys.seedvault.metadata.MetadataReaderImpl
 import com.stevesoltys.seedvault.metadata.PackageMetadata
-import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
 import com.stevesoltys.seedvault.plugins.LegacyStoragePlugin
 import com.stevesoltys.seedvault.plugins.StoragePlugin
 import com.stevesoltys.seedvault.plugins.saf.FILE_BACKUP_METADATA
-import com.stevesoltys.seedvault.transport.backup.ApkBackup
 import com.stevesoltys.seedvault.transport.backup.BackupCoordinator
 import com.stevesoltys.seedvault.transport.backup.FullBackup
 import com.stevesoltys.seedvault.transport.backup.InputFactory
@@ -31,6 +29,7 @@ import com.stevesoltys.seedvault.transport.restore.KVRestore
 import com.stevesoltys.seedvault.transport.restore.OutputFactory
 import com.stevesoltys.seedvault.transport.restore.RestoreCoordinator
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
+import com.stevesoltys.seedvault.worker.ApkBackup
 import io.mockk.CapturingSlot
 import io.mockk.Runs
 import io.mockk.coEvery
@@ -73,7 +72,6 @@ internal class CoordinatorIntegrationTest : TransportTest() {
         backupPlugin,
         kvBackup,
         fullBackup,
-        apkBackup,
         clock,
         packageService,
         metadataManager,
@@ -138,13 +136,13 @@ internal class CoordinatorIntegrationTest : TransportTest() {
             appData2.size
         }
         coEvery {
-            apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, any())
+            apkBackup.backupApkIfNecessary(packageInfo, any())
         } returns packageMetadata
         coEvery {
             backupPlugin.getOutputStream(token, FILE_BACKUP_METADATA)
         } returns metadataOutputStream
         every {
-            metadataManager.onApkBackedUp(packageInfo, packageMetadata, metadataOutputStream)
+            metadataManager.onApkBackedUp(packageInfo, packageMetadata)
         } just Runs
         every {
             metadataManager.onPackageBackedUp(
@@ -215,7 +213,7 @@ internal class CoordinatorIntegrationTest : TransportTest() {
             appData.copyInto(value.captured) // write the app data into the passed ByteArray
             appData.size
         }
-        coEvery { apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, any()) } returns null
+        coEvery { apkBackup.backupApkIfNecessary(packageInfo, any()) } returns null
         every { settingsManager.getToken() } returns token
         coEvery {
             backupPlugin.getOutputStream(token, FILE_BACKUP_METADATA)
@@ -279,25 +277,13 @@ internal class CoordinatorIntegrationTest : TransportTest() {
         coEvery { backupPlugin.getOutputStream(token, realName) } returns bOutputStream
         every { inputFactory.getInputStream(fileDescriptor) } returns bInputStream
         every { settingsManager.isQuotaUnlimited() } returns false
-        coEvery {
+        coEvery { apkBackup.backupApkIfNecessary(packageInfo, any()) } returns packageMetadata
-            apkBackup.backupApkIfNecessary(
-                packageInfo,
-                UNKNOWN_ERROR,
-                any()
-            )
-        } returns packageMetadata
         every { settingsManager.getToken() } returns token
         every { metadataManager.salt } returns salt
         coEvery {
             backupPlugin.getOutputStream(token, FILE_BACKUP_METADATA)
         } returns metadataOutputStream
-        every {
+        every { metadataManager.onApkBackedUp(packageInfo, packageMetadata) } just Runs
-            metadataManager.onApkBackedUp(
-                packageInfo,
-                packageMetadata,
-                metadataOutputStream
-            )
-        } just Runs
         every {
             metadataManager.onPackageBackedUp(
                 packageInfo = packageInfo,

app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt

@@ -5,7 +5,6 @@ import android.app.backup.BackupTransport.TRANSPORT_NOT_INITIALIZED
 import android.app.backup.BackupTransport.TRANSPORT_OK
 import android.app.backup.BackupTransport.TRANSPORT_PACKAGE_REJECTED
 import android.app.backup.BackupTransport.TRANSPORT_QUOTA_EXCEEDED
-import android.content.pm.ApplicationInfo.FLAG_STOPPED
 import android.content.pm.PackageInfo
 import android.net.Uri
 import android.os.ParcelFileDescriptor
@@ -14,18 +13,15 @@ import com.stevesoltys.seedvault.coAssertThrows
 import com.stevesoltys.seedvault.getRandomString
 import com.stevesoltys.seedvault.metadata.BackupType
 import com.stevesoltys.seedvault.metadata.PackageMetadata
-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.Storage
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
+import com.stevesoltys.seedvault.worker.ApkBackup
 import io.mockk.Runs
 import io.mockk.coEvery
-import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk
@@ -36,7 +32,6 @@ import org.junit.jupiter.api.Test
 import java.io.IOException
 import java.io.OutputStream
 import kotlin.random.Random
-import kotlin.random.nextLong
 
 @Suppress("BlockingMethodInNonBlockingContext")
 internal class BackupCoordinatorTest : BackupTest() {
@@ -53,7 +48,6 @@ internal class BackupCoordinatorTest : BackupTest() {
         plugin,
         kv,
         full,
-        apkBackup,
         clock,
         packageService,
         metadataManager,
@@ -157,16 +151,12 @@ internal class BackupCoordinatorTest : BackupTest() {
         val isFullBackup = Random.nextBoolean()
         val quota = Random.nextLong()
 
-        expectApkBackupAndMetadataWrite()
         if (isFullBackup) {
             every { full.getQuota() } returns quota
         } else {
             every { kv.getQuota() } returns quota
         }
-        every { metadataOutputStream.close() } just Runs
         assertEquals(quota, backup.getBackupQuota(packageInfo.packageName, isFullBackup))
-
-        verify { metadataOutputStream.close() }
     }
 
     @Test
@@ -276,7 +266,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         coEvery {
             full.performFullBackup(packageInfo, fileDescriptor, 0, token, salt)
         } returns TRANSPORT_OK
-        coEvery { apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, any()) } returns null
+        coEvery { apkBackup.backupApkIfNecessary(packageInfo, any()) } returns null
 
         assertEquals(TRANSPORT_OK, backup.performFullBackup(packageInfo, fileDescriptor, 0))
     }
@@ -380,180 +370,13 @@ internal class BackupCoordinatorTest : BackupTest() {
 
     @Test
     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" },
-            PackageInfo().apply {
-                packageName = "org.example.2"
-                // the second package does not get backed up, because it is stopped
-                applicationInfo = mockk {
-                    flags = FLAG_STOPPED
-                }
-            }
-        )
-        val packageMetadata: PackageMetadata = mockk()
-        val size = Random.nextLong(1L..Long.MAX_VALUE)
-
-        every { settingsManager.canDoBackupNow() } returns true
-        every { metadataManager.requiresInit } returns false
-        every { settingsManager.getToken() } returns token
-        every { metadataManager.salt } returns salt
-        // do actual @pm@ backup
-        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 { kv.getCurrentSize() } returns size
-        every {
-            metadataManager.onPackageBackedUp(
-                pmPackageInfo,
-                BackupType.KV,
-                size,
-                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
-        every {
-            notificationManager.onOptOutAppBackup(
-                notAllowedPackages[0].packageName,
-                1,
-                notAllowedPackages.size
-            )
-        } just Runs
-        // no backup needed
-        coEvery {
-            apkBackup.backupApkIfNecessary(notAllowedPackages[0], NOT_ALLOWED, any())
-        } returns null
-        // check old metadata for state changes, because we won't update it otherwise
-        every {
-            metadataManager.getPackageMetadata(notAllowedPackages[0].packageName)
-        } returns packageMetadata
-        every { packageMetadata.state } returns NOT_ALLOWED // no change
-
-        // update notification for second package
-        every {
-            notificationManager.onOptOutAppBackup(
-                notAllowedPackages[1].packageName,
-                2,
-                notAllowedPackages.size
-            )
-        } just Runs
-        // was backed up, get new packageMetadata
-        coEvery {
-            apkBackup.backupApkIfNecessary(notAllowedPackages[1], WAS_STOPPED, any())
-        } returns packageMetadata
-        every { settingsManager.getToken() } returns token
-        coEvery { plugin.getOutputStream(token, FILE_BACKUP_METADATA) } returns metadataOutputStream
-        every {
-            metadataManager.onApkBackedUp(
-                notAllowedPackages[1],
-                packageMetadata,
-                metadataOutputStream
-            )
-        } just Runs
-        every { metadataOutputStream.close() } just Runs
-
-        assertEquals(TRANSPORT_OK, backup.finishBackup())
-
-        coVerify {
-            apkBackup.backupApkIfNecessary(notAllowedPackages[0], NOT_ALLOWED, any())
-            apkBackup.backupApkIfNecessary(notAllowedPackages[1], WAS_STOPPED, any())
-            metadataOutputStream.close()
-        }
-    }
-
-    @Test
-    fun `APK backup of not allowed apps updates state even without new APK`() = runBlocking {
-        val oldPackageMetadata: PackageMetadata = mockk()
-
-        every { packageService.notBackedUpPackages } returns listOf(packageInfo)
-        every {
-            notificationManager.onOptOutAppBackup(packageInfo.packageName, 1, 1)
-        } just Runs
-        coEvery { apkBackup.backupApkIfNecessary(packageInfo, NOT_ALLOWED, any()) } returns null
-        every {
-            metadataManager.getPackageMetadata(packageInfo.packageName)
-        } returns oldPackageMetadata
-        // state differs now, was stopped before
-        every { oldPackageMetadata.state } returns WAS_STOPPED
-        every { settingsManager.getToken() } returns token
-        coEvery { plugin.getOutputStream(token, FILE_BACKUP_METADATA) } returns metadataOutputStream
-        every {
-            metadataManager.onPackageBackupError(
-                packageInfo,
-                NOT_ALLOWED,
-                metadataOutputStream
-            )
-        } just Runs
-        every { metadataOutputStream.close() } just Runs
-
-        backup.backUpApksOfNotBackedUpPackages()
-
-        verify {
-            metadataManager.onPackageBackupError(packageInfo, NOT_ALLOWED, metadataOutputStream)
-            metadataOutputStream.close()
-        }
-    }
-
-    @Test
-    fun `APK backup of not allowed apps updates state even without old state`() = runBlocking {
-        every { packageService.notBackedUpPackages } returns listOf(packageInfo)
-        every {
-            notificationManager.onOptOutAppBackup(packageInfo.packageName, 1, 1)
-        } just Runs
-        coEvery { apkBackup.backupApkIfNecessary(packageInfo, NOT_ALLOWED, any()) } returns null
-        every {
-            metadataManager.getPackageMetadata(packageInfo.packageName)
-        } returns null
-        every { settingsManager.getToken() } returns token
-        coEvery { plugin.getOutputStream(token, FILE_BACKUP_METADATA) } returns metadataOutputStream
-        every {
-            metadataManager.onPackageBackupError(
-                packageInfo,
-                NOT_ALLOWED,
-                metadataOutputStream
-            )
-        } just Runs
-        every { metadataOutputStream.close() } just Runs
-
-        backup.backUpApksOfNotBackedUpPackages()
-
-        verify {
-            metadataManager.onPackageBackupError(packageInfo, NOT_ALLOWED, metadataOutputStream)
-            metadataOutputStream.close()
-        }
     }
 
     private fun expectApkBackupAndMetadataWrite() {
-        coEvery {
+        coEvery { apkBackup.backupApkIfNecessary(any(), any()) } returns packageMetadata
-            apkBackup.backupApkIfNecessary(
-                any(),
-                UNKNOWN_ERROR,
-                any()
-            )
-        } returns packageMetadata
         every { settingsManager.getToken() } returns token
         coEvery { plugin.getOutputStream(token, FILE_BACKUP_METADATA) } returns metadataOutputStream
-        every {
+        every { metadataManager.onApkBackedUp(any(), packageMetadata) } just Runs
-            metadataManager.onApkBackedUp(
-                any(),
-                packageMetadata,
-                metadataOutputStream
-            )
-        } just Runs
     }
 
 }

app/src/test/java/com/stevesoltys/seedvault/worker/ApkBackupManagerTest.kt

@@ -0,0 +1,204 @@
+package com.stevesoltys.seedvault.worker
+
+import android.content.pm.ApplicationInfo
+import android.content.pm.ApplicationInfo.FLAG_ALLOW_BACKUP
+import android.content.pm.ApplicationInfo.FLAG_INSTALLED
+import android.content.pm.ApplicationInfo.FLAG_STOPPED
+import android.content.pm.PackageInfo
+import com.stevesoltys.seedvault.metadata.PackageMetadata
+import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
+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.transport.TransportTest
+import com.stevesoltys.seedvault.transport.backup.PackageService
+import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
+import io.mockk.Runs
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import io.mockk.verify
+import io.mockk.verifyAll
+import kotlinx.coroutines.runBlocking
+import org.junit.jupiter.api.Test
+import java.io.OutputStream
+
+internal class ApkBackupManagerTest : TransportTest() {
+
+    private val packageService: PackageService = mockk()
+    private val apkBackup: ApkBackup = mockk()
+    private val plugin: StoragePlugin = mockk()
+    private val nm: BackupNotificationManager = mockk()
+
+    private val apkBackupManager = ApkBackupManager(
+        context = context,
+        settingsManager = settingsManager,
+        metadataManager = metadataManager,
+        packageService = packageService,
+        apkBackup = apkBackup,
+        plugin = plugin,
+        nm = nm,
+    )
+
+    private val metadataOutputStream = mockk<OutputStream>()
+    private val packageMetadata: PackageMetadata = mockk()
+
+    @Test
+    fun `Package state of app that is not stopped gets recorded as not-allowed`() = runBlocking {
+        every { nm.onAppsNotBackedUp() } just Runs
+        every { packageService.notBackedUpPackages } returns listOf(packageInfo)
+
+        every {
+            metadataManager.getPackageMetadata(packageInfo.packageName)
+        } returns packageMetadata
+        every { packageMetadata.state } returns UNKNOWN_ERROR
+        every { metadataManager.onPackageDoesNotGetBackedUp(packageInfo, NOT_ALLOWED) } just Runs
+
+        every { settingsManager.backupApks() } returns false
+        expectFinalUpload()
+        every { nm.onApkBackupDone() } just Runs
+
+        apkBackupManager.backup()
+
+        verify {
+            metadataManager.onPackageDoesNotGetBackedUp(packageInfo, NOT_ALLOWED)
+            metadataOutputStream.close()
+        }
+    }
+
+    @Test
+    fun `Package state of app gets recorded even if no previous state`() = runBlocking {
+        every { nm.onAppsNotBackedUp() } just Runs
+        every { packageService.notBackedUpPackages } returns listOf(packageInfo)
+
+        every {
+            metadataManager.getPackageMetadata(packageInfo.packageName)
+        } returns null
+        every { metadataManager.onPackageDoesNotGetBackedUp(packageInfo, NOT_ALLOWED) } just Runs
+
+        every { settingsManager.backupApks() } returns false
+        expectFinalUpload()
+        every { nm.onApkBackupDone() } just Runs
+
+        apkBackupManager.backup()
+
+        verify {
+            metadataManager.onPackageDoesNotGetBackedUp(packageInfo, NOT_ALLOWED)
+            metadataOutputStream.close()
+        }
+    }
+
+    @Test
+    fun `Package state of app that is stopped gets recorded`() = runBlocking {
+        val packageInfo = PackageInfo().apply {
+            packageName = "org.example"
+            applicationInfo = mockk<ApplicationInfo> {
+                flags = FLAG_ALLOW_BACKUP or FLAG_INSTALLED or FLAG_STOPPED
+            }
+        }
+
+        every { nm.onAppsNotBackedUp() } just Runs
+        every { packageService.notBackedUpPackages } returns listOf(packageInfo)
+
+        every {
+            metadataManager.getPackageMetadata(packageInfo.packageName)
+        } returns packageMetadata
+        every { packageMetadata.state } returns UNKNOWN_ERROR
+        every { metadataManager.onPackageDoesNotGetBackedUp(packageInfo, WAS_STOPPED) } just Runs
+
+        every { settingsManager.backupApks() } returns false
+        expectFinalUpload()
+        every { nm.onApkBackupDone() } just Runs
+
+        apkBackupManager.backup()
+
+        verify {
+            metadataManager.onPackageDoesNotGetBackedUp(packageInfo, WAS_STOPPED)
+            metadataOutputStream.close()
+        }
+    }
+
+    @Test
+    fun `Package state only updated when changed`() = runBlocking {
+        every { nm.onAppsNotBackedUp() } just Runs
+        every { packageService.notBackedUpPackages } returns listOf(packageInfo)
+
+        every {
+            metadataManager.getPackageMetadata(packageInfo.packageName)
+        } returns packageMetadata
+        every { packageMetadata.state } returns NOT_ALLOWED
+
+        every { settingsManager.backupApks() } returns false
+        expectFinalUpload()
+        every { nm.onApkBackupDone() } just Runs
+
+        apkBackupManager.backup()
+
+        verifyAll(inverse = true) {
+            metadataManager.onPackageDoesNotGetBackedUp(packageInfo, NOT_ALLOWED)
+        }
+    }
+
+    @Test
+    fun `two packages get backed up, one their APK uploaded`() = runBlocking {
+        val notAllowedPackages = listOf(
+            PackageInfo().apply { packageName = "org.example.1" },
+            PackageInfo().apply {
+                packageName = "org.example.2"
+                // the second package does not get backed up, because it is stopped
+                applicationInfo = mockk {
+                    flags = FLAG_STOPPED
+                }
+            }
+        )
+
+        expectAllAppsWillGetBackedUp()
+        every { settingsManager.backupApks() } returns true
+
+        every { packageService.allUserPackages } returns notAllowedPackages
+        // update notification
+        every {
+            nm.onApkBackup(notAllowedPackages[0].packageName, any(), 0, notAllowedPackages.size)
+        } just Runs
+        // no backup needed
+        coEvery {
+            apkBackup.backupApkIfNecessary(notAllowedPackages[0], any())
+        } returns null
+        // update notification for second package
+        every {
+            nm.onApkBackup(notAllowedPackages[1].packageName, any(), 1, notAllowedPackages.size)
+        } just Runs
+        // was backed up, get new packageMetadata
+        coEvery {
+            apkBackup.backupApkIfNecessary(notAllowedPackages[1], any())
+        } returns packageMetadata
+        every { metadataManager.onApkBackedUp(notAllowedPackages[1], packageMetadata) } just Runs
+
+        expectFinalUpload()
+        every { nm.onApkBackupDone() } just Runs
+
+        apkBackupManager.backup()
+
+        coVerify {
+            apkBackup.backupApkIfNecessary(notAllowedPackages[0], any())
+            apkBackup.backupApkIfNecessary(notAllowedPackages[1], any())
+            metadataOutputStream.close()
+        }
+    }
+
+    private fun expectAllAppsWillGetBackedUp() {
+        every { nm.onAppsNotBackedUp() } just Runs
+        every { packageService.notBackedUpPackages } returns emptyList()
+    }
+
+    private fun expectFinalUpload() {
+        every { settingsManager.getToken() } returns token
+        coEvery { plugin.getOutputStream(token, FILE_BACKUP_METADATA) } returns metadataOutputStream
+        every { metadataManager.uploadMetadata(metadataOutputStream) } just Runs
+        every { metadataOutputStream.close() } just Runs
+    }
+
+}

app/src/test/java/com/stevesoltys/seedvault/worker/ApkBackupTest.kt

@@ -1,4 +1,9 @@
-package com.stevesoltys.seedvault.transport.backup
+/*
+ * SPDX-FileCopyrightText: 2024 The Calyx Institute
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.stevesoltys.seedvault.worker
 
 import android.content.pm.ApplicationInfo.FLAG_SYSTEM
 import android.content.pm.ApplicationInfo.FLAG_TEST_ONLY
@@ -13,6 +18,7 @@ import com.stevesoltys.seedvault.getRandomString
 import com.stevesoltys.seedvault.metadata.ApkSplit
 import com.stevesoltys.seedvault.metadata.PackageMetadata
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
+import com.stevesoltys.seedvault.transport.backup.BackupTest
 import io.mockk.coEvery
 import io.mockk.every
 import io.mockk.mockk
@@ -56,7 +62,7 @@ internal class ApkBackupTest : BackupTest() {
     @Test
     fun `does not back up @pm@`() = runBlocking {
         val packageInfo = PackageInfo().apply { packageName = MAGIC_PACKAGE_MANAGER }
-        assertNull(apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter))
+        assertNull(apkBackup.backupApkIfNecessary(packageInfo, streamGetter))
     }
 
     @Test
@@ -64,7 +70,7 @@ internal class ApkBackupTest : BackupTest() {
         every { settingsManager.backupApks() } returns false
         every { settingsManager.isBackupEnabled(any()) } returns true
 
-        assertNull(apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter))
+        assertNull(apkBackup.backupApkIfNecessary(packageInfo, streamGetter))
     }
 
     @Test
@@ -72,7 +78,7 @@ internal class ApkBackupTest : BackupTest() {
         every { settingsManager.backupApks() } returns true
         every { settingsManager.isBackupEnabled(any()) } returns false
 
-        assertNull(apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter))
+        assertNull(apkBackup.backupApkIfNecessary(packageInfo, streamGetter))
     }
 
     @Test
@@ -81,7 +87,7 @@ internal class ApkBackupTest : BackupTest() {
 
         every { settingsManager.isBackupEnabled(any()) } returns true
         every { settingsManager.backupApks() } returns true
-        assertNull(apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter))
+        assertNull(apkBackup.backupApkIfNecessary(packageInfo, streamGetter))
     }
 
     @Test
@@ -90,7 +96,7 @@ internal class ApkBackupTest : BackupTest() {
 
         every { settingsManager.isBackupEnabled(any()) } returns true
         every { settingsManager.backupApks() } returns true
-        assertNull(apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter))
+        assertNull(apkBackup.backupApkIfNecessary(packageInfo, streamGetter))
     }
 
     @Test
@@ -102,7 +108,7 @@ internal class ApkBackupTest : BackupTest() {
 
         expectChecks(packageMetadata)
 
-        assertNull(apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter))
+        assertNull(apkBackup.backupApkIfNecessary(packageInfo, streamGetter))
     }
 
     @Test
@@ -113,7 +119,7 @@ internal class ApkBackupTest : BackupTest() {
 
         assertThrows(IOException::class.java) {
             runBlocking {
-                assertNull(apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter))
+                assertNull(apkBackup.backupApkIfNecessary(packageInfo, streamGetter))
             }
         }
     }
@@ -128,7 +134,7 @@ internal class ApkBackupTest : BackupTest() {
         every { sigInfo.hasMultipleSigners() } returns false
         every { sigInfo.signingCertificateHistory } returns emptyArray()
 
-        assertNull(apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter))
+        assertNull(apkBackup.backupApkIfNecessary(packageInfo, streamGetter))
     }
 
     @Test
@@ -141,7 +147,7 @@ internal class ApkBackupTest : BackupTest() {
         }.absolutePath
         val apkOutputStream = ByteArrayOutputStream()
         val updatedMetadata = PackageMetadata(
-            time = 0L,
+            time = packageMetadata.time,
             state = UNKNOWN_ERROR,
             version = packageInfo.longVersionCode,
             installer = getRandomString(),
@@ -159,7 +165,7 @@ internal class ApkBackupTest : BackupTest() {
 
         assertEquals(
             updatedMetadata,
-            apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter)
+            apkBackup.backupApkIfNecessary(packageInfo, streamGetter)
         )
         assertArrayEquals(apkBytes, apkOutputStream.toByteArray())
     }
@@ -198,7 +204,7 @@ internal class ApkBackupTest : BackupTest() {
         val split2OutputStream = ByteArrayOutputStream()
         // expected new metadata for package
         val updatedMetadata = PackageMetadata(
-            time = 0L,
+            time = packageMetadata.time,
             state = UNKNOWN_ERROR,
             version = packageInfo.longVersionCode,
             installer = getRandomString(),
@@ -231,7 +237,7 @@ internal class ApkBackupTest : BackupTest() {
 
         assertEquals(
             updatedMetadata,
-            apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter)
+            apkBackup.backupApkIfNecessary(packageInfo, streamGetter)
         )
         assertArrayEquals(apkBytes, apkOutputStream.toByteArray())
         assertArrayEquals(split1Bytes, split1OutputStream.toByteArray())