Merge pull request #613 from grote/backup-binder

Use BackupRequester to request backup in chunks

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

@@ -119,9 +119,21 @@ internal interface LargeBackupTestBase : LargeTestBase {
         coEvery {
             spyKVBackup.finishBackup()
         } answers {
+            val oldMap = HashMap<String, String>()
+            // @pm@ and android can get backed up multiple times (if we need more than one request)
+            // so we need to keep the data it backed up before
+            if (backupResult.kv.containsKey(packageName)) {
+                backupResult.kv[packageName]?.forEach { (key, value) ->
+                    // if a key existing in new data, we use its value from new data, don't override
+                    if (!data.containsKey(key)) oldMap[key] = value
+                }
+            }
             backupResult.kv[packageName!!] = data
                 .mapValues { entry -> entry.value.sha256() }
                 .toMutableMap()
+                .apply {
+                    putAll(oldMap)
+                }
 
             packageName = null
             data = mutableMapOf()

app/src/main/java/com/stevesoltys/seedvault/restore/RestoreViewModel.kt

@@ -39,6 +39,7 @@ import com.stevesoltys.seedvault.restore.install.isInstalled
 import com.stevesoltys.seedvault.settings.SettingsManager
 import com.stevesoltys.seedvault.storage.StorageRestoreService
 import com.stevesoltys.seedvault.transport.TRANSPORT_ID
+import com.stevesoltys.seedvault.transport.backup.NUM_PACKAGES_PER_TRANSACTION
 import com.stevesoltys.seedvault.transport.restore.RestoreCoordinator
 import com.stevesoltys.seedvault.ui.AppBackupState
 import com.stevesoltys.seedvault.ui.AppBackupState.FAILED
@@ -70,7 +71,7 @@ import java.util.LinkedList
 
 private val TAG = RestoreViewModel::class.java.simpleName
 
-internal const val PACKAGES_PER_CHUNK = 100
+internal const val PACKAGES_PER_CHUNK = NUM_PACKAGES_PER_TRANSACTION
 
 internal class RestoreViewModel(
     app: Application,

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

@@ -1,19 +1,16 @@
 package com.stevesoltys.seedvault.transport
 
 import android.app.Service
-import android.app.backup.BackupManager
 import android.app.backup.IBackupManager
 import android.content.Context
 import android.content.Intent
 import android.os.IBinder
-import android.os.RemoteException
 import android.util.Log
 import androidx.annotation.WorkerThread
-import com.stevesoltys.seedvault.BackupMonitor
 import com.stevesoltys.seedvault.crypto.KeyManager
+import com.stevesoltys.seedvault.transport.backup.BackupRequester
 import com.stevesoltys.seedvault.transport.backup.PackageService
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
-import com.stevesoltys.seedvault.ui.notification.NotificationBackupObserver
 import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
 import org.koin.core.context.GlobalContext.get
@@ -70,25 +67,10 @@ fun requestBackup(context: Context): Boolean {
     val backupManager: IBackupManager = get().get()
     return if (backupManager.isBackupEnabled) {
         val packageService: PackageService = get().get()
-        val packages = packageService.eligiblePackages
-        val appTotals = packageService.expectedAppTotals
 
-        val result = try {
+        Log.d(TAG, "Backup is enabled, request backup...")
-            Log.d(TAG, "Backup is enabled, request backup...")
+        val backupRequester = BackupRequester(context, backupManager, packageService)
-            val observer = NotificationBackupObserver(context, packages.size, appTotals)
+        return backupRequester.requestBackup()
-            backupManager.requestBackup(packages, observer, BackupMonitor(), 0)
-        } catch (e: RemoteException) {
-            Log.e(TAG, "Error during backup: ", e)
-            val nm: BackupNotificationManager = get().get()
-            nm.onBackupError()
-        }
-        if (result == BackupManager.SUCCESS) {
-            Log.i(TAG, "Backup request succeeded ")
-            true
-        } else {
-            Log.e(TAG, "Backup request failed: $result")
-            false
-        }
     } else {
         Log.i(TAG, "Backup is not enabled")
         true // this counts as success

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

@@ -158,7 +158,8 @@ internal class BackupCoordinator(
      */
     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 called
+            // 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))
         }
 
@@ -379,6 +380,7 @@ internal class BackupCoordinator(
                     }
                 }
                 // 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()
@@ -424,10 +426,12 @@ internal class BackupCoordinator(
             val packageName = packageInfo.packageName
             try {
                 nm.onOptOutAppBackup(packageName, i + 1, notBackedUpPackages.size)
-                val packageState =
+                val packageState = if (packageInfo.isStopped()) WAS_STOPPED else NOT_ALLOWED
-                    if (packageInfo.isStopped()) WAS_STOPPED else NOT_ALLOWED
                 val wasBackedUp = backUpApk(packageInfo, packageState)
-                if (!wasBackedUp) {
+                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

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

@@ -0,0 +1,108 @@
+/*
+ * SPDX-FileCopyrightText: 2024 The Calyx Institute
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.stevesoltys.seedvault.transport.backup
+
+import android.app.backup.BackupManager
+import android.app.backup.IBackupManager
+import android.content.Context
+import android.os.RemoteException
+import android.util.Log
+import androidx.annotation.WorkerThread
+import com.stevesoltys.seedvault.BackupMonitor
+import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
+import com.stevesoltys.seedvault.ui.notification.NotificationBackupObserver
+import org.koin.core.component.KoinComponent
+import org.koin.core.context.GlobalContext
+
+private val TAG = BackupRequester::class.java.simpleName
+internal const val NUM_PACKAGES_PER_TRANSACTION = 100
+
+/**
+ * Used for requesting a backup of all installed packages,
+ * in chunks if there are more than [NUM_PACKAGES_PER_TRANSACTION].
+ *
+ * Can only be used once for one backup.
+ * Make a new instance for subsequent backups.
+ */
+@WorkerThread
+internal class BackupRequester(
+    context: Context,
+    private val backupManager: IBackupManager,
+    val packageService: PackageService,
+) : KoinComponent {
+
+    private val packages = packageService.eligiblePackages
+    private val observer = NotificationBackupObserver(
+        context = context,
+        backupRequester = this,
+        requestedPackages = packages.size,
+        appTotals = packageService.expectedAppTotals,
+    )
+    private val monitor = BackupMonitor()
+
+    /**
+     * The current package index.
+     *
+     * Used for splitting the packages into chunks.
+     */
+    private var packageIndex: Int = 0
+
+    /**
+     * Request the backup to happen. Should be called short after constructing this object.
+     */
+    fun requestBackup(): Boolean {
+        if (packageIndex != 0) error("requestBackup() called more than once!")
+
+        return request(getNextChunk())
+    }
+
+    /**
+     * Backs up the next chunk of packages.
+     *
+     * @return true, if backup for all packages was already requested and false,
+     * if there are more packages that we just have requested backup for.
+     */
+    fun requestNext(): Boolean {
+        if (packageIndex <= 0) error("requestBackup() must be called first!")
+
+        // Backup next chunk if there are more packages to back up.
+        return if (packageIndex < packages.size) {
+            request(getNextChunk())
+            false
+        } else {
+            true
+        }
+    }
+
+    private fun request(chunk: Array<String>): Boolean {
+        Log.i(TAG, "${chunk.toList()}")
+        val result = try {
+            backupManager.requestBackup(chunk, observer, monitor, 0)
+        } catch (e: RemoteException) {
+            Log.e(TAG, "Error during backup: ", e)
+            val nm: BackupNotificationManager = GlobalContext.get().get()
+            nm.onBackupError()
+        }
+        return if (result == BackupManager.SUCCESS) {
+            Log.i(TAG, "Backup request succeeded")
+            true
+        } else {
+            Log.e(TAG, "Backup request failed: $result")
+            false
+        }
+    }
+
+    private fun getNextChunk(): Array<String> {
+        val nextChunkIndex =
+            (packageIndex + NUM_PACKAGES_PER_TRANSACTION).coerceAtMost(packages.size)
+        val packageChunk = packages.subList(packageIndex, nextChunkIndex).toTypedArray()
+        val numBackingUp = packageIndex + packageChunk.size
+        Log.i(TAG, "Requesting backup for $numBackingUp/${packages.size} packages...")
+        packageIndex += packageChunk.size
+        return packageChunk
+    }
+
+}

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

@@ -38,7 +38,7 @@ internal class PackageService(
     private val packageManager: PackageManager = context.packageManager
     private val myUserId = UserHandle.myUserId()
 
-    val eligiblePackages: Array<String>
+    val eligiblePackages: List<String>
         @WorkerThread
         @Throws(RemoteException::class)
         get() {
@@ -70,11 +70,12 @@ internal class PackageService(
             val packageArray = eligibleApps.toMutableList()
             packageArray.add(MAGIC_PACKAGE_MANAGER)
 
-            return packageArray.toTypedArray()
+            return packageArray
         }
 
     /**
-     * A list of packages that will not be backed up.
+     * A list of packages that will not be backed up,
+     * because they are currently force-stopped for example.
      */
     val notBackedUpPackages: List<PackageInfo>
         @WorkerThread
@@ -127,16 +128,16 @@ internal class PackageService(
         @WorkerThread
         get() {
             var appsTotal = 0
-            var appsOptOut = 0
+            var appsNotIncluded = 0
             packageManager.getInstalledPackages(GET_INSTRUMENTATION).forEach { packageInfo ->
                 if (packageInfo.isUserVisible(context)) {
                     appsTotal++
                     if (packageInfo.doesNotGetBackedUp()) {
-                        appsOptOut++
+                        appsNotIncluded++
                     }
                 }
             }
-            return ExpectedAppTotals(appsTotal, appsOptOut)
+            return ExpectedAppTotals(appsTotal, appsNotIncluded)
         }
 
     fun getVersionName(packageName: String): String? = try {
@@ -201,6 +202,7 @@ internal class PackageService(
      */
     private fun PackageInfo.doesNotGetBackedUp(): Boolean {
         if (packageName == MAGIC_PACKAGE_MANAGER || applicationInfo == null) return true
+        if (packageName == plugin.providerPackageName) return true
         return !allowsBackup() || isStopped()
     }
 }
@@ -211,9 +213,11 @@ internal data class ExpectedAppTotals(
      */
     val appsTotal: Int,
     /**
-     * The number of non-system apps that has opted-out of backup.
+     * 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 appsOptOut: Int,
+    val appsNotGettingBackedUp: Int,
 )
 
 internal fun PackageInfo.isUserVisible(context: Context): Boolean {

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

@@ -27,6 +27,7 @@ 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_SUCCESS = "NotificationBackupSuccess"
@@ -53,6 +54,11 @@ internal class BackupNotificationManager(private val context: Context) {
     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)
         return NotificationChannel(CHANNEL_ID_OBSERVER, title, IMPORTANCE_LOW).apply {
@@ -87,23 +93,34 @@ internal class BackupNotificationManager(private val context: Context) {
         updateBackupNotification(
             infoText = "", // This passes quickly, no need to show something here
             transferred = 0,
-            expected = expectedPackages
+            expected = appTotals.appsTotal
         )
         expectedApps = expectedPackages
-        expectedOptOutApps = appTotals.appsOptOut
+        expectedOptOutApps = appTotals.appsNotGettingBackedUp
         expectedAppTotals = appTotals
+        optOutAppsDone = false
+        Log.i(TAG, "onBackupStarted $expectedApps + $expectedOptOutApps = ${appTotals.appsTotal}")
     }
 
     /**
      * This should get called before [onBackupUpdate].
+     * 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) {
-        val text = "Opt-out APK for $packageName"
+        if (optOutAppsDone) return
+
+        val text = "APK for $packageName"
         if (expectedApps == null) {
             updateBackgroundBackupNotification(text)
         } else {
             updateBackupNotification(text, transferred, expected + (expectedApps ?: 0))
+            if (expectedOptOutApps != null && expectedOptOutApps != expected) {
+                Log.w(TAG, "Number of packages not getting backed up mismatch: " +
+                    "$expectedOptOutApps != $expected")
+            }
             expectedOptOutApps = expected
+            if (transferred == expected) optOutAppsDone = true
         }
     }
 
@@ -116,7 +133,7 @@ internal class BackupNotificationManager(private val context: Context) {
         val addend = expectedOptOutApps ?: 0
         updateBackupNotification(
             infoText = app,
-            transferred = transferred + addend,
+            transferred = min(transferred + addend, expected + addend),
             expected = expected + addend
         )
     }
@@ -167,13 +184,17 @@ internal class BackupNotificationManager(private val context: Context) {
         //
         // This won't bring back the expected finish notification in this case,
         // but at least we don't leave stuck notifications laying around.
-        nm.activeNotifications.forEach { notification ->
+        // FIXME the service gets destroyed for each chunk when requesting backup in chunks
-            // only consider ongoing notifications in our ID space (storage backup uses > 1000)
+        //  This leads to the cancellation of an ongoing backup notification.
-            if (notification.isOngoing && notification.id < 1000) {
+        //  So for now, we'll remove automatic notification clean-up
-                Log.w(TAG, "Needed to clean up notification with ID ${notification.id}")
+        //  and find out if it is still necessary. If not, this comment can be removed.
-                nm.cancel(notification.id)
+        // nm.activeNotifications.forEach { notification ->
-            }
+        //     // only consider ongoing notifications in our ID space (storage backup uses > 1000)
-        }
+        //     if (notification.isOngoing && notification.id < 1000) {
+        //         Log.w(TAG, "Needed to clean up notification with ID ${notification.id}")
+        //         nm.cancel(notification.id)
+        //     }
+        // }
     }
 
     fun onBackupFinished(success: Boolean, numBackedUp: Int?, size: Long) {

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

@@ -10,6 +10,7 @@ import android.util.Log.isLoggable
 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
@@ -18,7 +19,8 @@ private val TAG = NotificationBackupObserver::class.java.simpleName
 
 internal class NotificationBackupObserver(
     private val context: Context,
-    private val expectedPackages: Int,
+    private val backupRequester: BackupRequester,
+    private val requestedPackages: Int,
     appTotals: ExpectedAppTotals,
 ) : IBackupObserver.Stub(), KoinComponent {
 
@@ -30,7 +32,7 @@ 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.
-        nm.onBackupStarted(expectedPackages, appTotals)
+        nm.onBackupStarted(requestedPackages, appTotals)
     }
 
     /**
@@ -73,25 +75,31 @@ internal class NotificationBackupObserver(
      *   as a whole failed.
      */
     override fun backupFinished(status: Int) {
-        if (isLoggable(TAG, INFO)) {
+        if (backupRequester.requestNext()) {
-            Log.i(TAG, "Backup finished $numPackages/$expectedPackages. Status: $status")
+            if (isLoggable(TAG, INFO)) {
+                Log.i(TAG, "Backup finished $numPackages/$requestedPackages. Status: $status")
+            }
+            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)
         }
-        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)
     }
 
     private fun showProgressNotification(packageName: String?) {
         if (packageName == null || currentPackage == packageName) return
 
-        if (isLoggable(TAG, INFO)) {
+        if (isLoggable(TAG, INFO)) Log.i(
-            "Showing progress notification for $currentPackage $numPackages/$expectedPackages".let {
+            TAG, "Showing progress notification for " +
-                Log.i(TAG, it)
+                "$currentPackage $numPackages/$requestedPackages"
-            }
+        )
-        }
         currentPackage = packageName
-        val app = getAppName(packageName)
+        val appName = getAppName(packageName)
+        val app = if (appName != packageName) {
+            "${getAppName(packageName)} ($packageName)"
+        } else {
+            packageName
+        }
         numPackages += 1
         nm.onBackupUpdate(app, numPackages)
     }