From 86c603e2d2b9c7c266a2d03634f6f099f89c2770 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 5 Feb 2024 17:29:38 -0300 Subject: [PATCH 1/4] Use BackupRequester to request backup in chunks Otherwise users with lots of installed apps with request a lot of packages causing binder transactions to reach their size limit and crash. --- .../seedvault/restore/RestoreViewModel.kt | 3 +- .../ConfigurableBackupTransportService.kt | 26 +---- .../transport/backup/BackupCoordinator.kt | 4 +- .../transport/backup/BackupRequester.kt | 105 ++++++++++++++++++ .../transport/backup/PackageService.kt | 7 +- .../notification/BackupNotificationManager.kt | 8 ++ .../NotificationBackupObserver.kt | 16 ++- 7 files changed, 136 insertions(+), 33 deletions(-) create mode 100644 app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupRequester.kt diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreViewModel.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreViewModel.kt index 9992a15c..e03e55e8 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreViewModel.kt +++ b/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, diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt index 443badd4..1b9fe3b6 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt +++ b/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...") - val observer = NotificationBackupObserver(context, packages.size, appTotals) - 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 - } + Log.d(TAG, "Backup is enabled, request backup...") + val backupRequester = BackupRequester(context, backupManager, packageService) + return backupRequester.requestBackup() } else { Log.i(TAG, "Backup is not enabled") true // this counts as success diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt index c6f9e1c6..2f088671 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt @@ -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() diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupRequester.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupRequester.kt new file mode 100644 index 00000000..d1e195fb --- /dev/null +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupRequester.kt @@ -0,0 +1,105 @@ +/* + * 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, + expectedPackages = 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 + + 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): 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 { + 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 + } + +} diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt index 7d16e173..a8ed0d3a 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt +++ b/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 + val eligiblePackages: List @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 @WorkerThread diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt index 8308fc2a..2b9a0e5b 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt @@ -53,6 +53,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 { @@ -98,6 +103,8 @@ internal class BackupNotificationManager(private val context: Context) { * This should get called before [onBackupUpdate]. */ fun onOptOutAppBackup(packageName: String, transferred: Int, expected: Int) { + if (optOutAppsDone) return + val text = "Opt-out APK for $packageName" if (expectedApps == null) { updateBackgroundBackupNotification(text) @@ -112,6 +119,7 @@ internal class BackupNotificationManager(private val context: Context) { * this type is is expected to get called after [onOptOutAppBackup]. */ fun onBackupUpdate(app: CharSequence, transferred: Int) { + optOutAppsDone = true val expected = expectedApps ?: error("expectedApps is null") val addend = expectedOptOutApps ?: 0 updateBackupNotification( diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt index d959dd05..ea7bc4a2 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt +++ b/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,6 +19,7 @@ private val TAG = NotificationBackupObserver::class.java.simpleName internal class NotificationBackupObserver( private val context: Context, + private val backupRequester: BackupRequester, private val expectedPackages: Int, appTotals: ExpectedAppTotals, ) : IBackupObserver.Stub(), KoinComponent { @@ -73,13 +75,15 @@ internal class NotificationBackupObserver( * as a whole failed. */ override fun backupFinished(status: Int) { - if (isLoggable(TAG, INFO)) { - Log.i(TAG, "Backup finished $numPackages/$expectedPackages. Status: $status") + if (backupRequester.requestNext()) { + if (isLoggable(TAG, INFO)) { + Log.i(TAG, "Backup finished $numPackages/$expectedPackages. 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?) { From 4014666c15ef7b6d24978a429349562ee7b6341b Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 6 Feb 2024 13:45:55 -0300 Subject: [PATCH 2/4] Improve notification progress reporting It is still somewhat buggy when d2d is on, but this is easier to resolve when moving everything to own scheduling. --- .../transport/backup/BackupCoordinator.kt | 8 ++++--- .../transport/backup/BackupRequester.kt | 5 ++++- .../transport/backup/PackageService.kt | 13 ++++++----- .../notification/BackupNotificationManager.kt | 19 +++++++++++----- .../NotificationBackupObserver.kt | 22 +++++++++++-------- 5 files changed, 44 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt index 2f088671..f1dde3b1 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt @@ -426,10 +426,12 @@ internal class BackupCoordinator( val packageName = packageInfo.packageName try { nm.onOptOutAppBackup(packageName, i + 1, notBackedUpPackages.size) - val packageState = - if (packageInfo.isStopped()) WAS_STOPPED else NOT_ALLOWED + val packageState = 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 diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupRequester.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupRequester.kt index d1e195fb..79c79ba9 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupRequester.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupRequester.kt @@ -38,7 +38,7 @@ internal class BackupRequester( private val observer = NotificationBackupObserver( context = context, backupRequester = this, - expectedPackages = packages.size, + requestedPackages = packages.size, appTotals = packageService.expectedAppTotals, ) private val monitor = BackupMonitor() @@ -50,6 +50,9 @@ internal class BackupRequester( */ 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!") diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt index a8ed0d3a..7a59437f 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt @@ -128,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 { @@ -202,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() } } @@ -212,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 { diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt index 2b9a0e5b..f2cbd003 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt +++ b/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" @@ -92,25 +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) { if (optOutAppsDone) return - val text = "Opt-out APK for $packageName" + 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 } } @@ -119,12 +129,11 @@ internal class BackupNotificationManager(private val context: Context) { * this type is is expected to get called after [onOptOutAppBackup]. */ fun onBackupUpdate(app: CharSequence, transferred: Int) { - optOutAppsDone = true val expected = expectedApps ?: error("expectedApps is null") val addend = expectedOptOutApps ?: 0 updateBackupNotification( infoText = app, - transferred = transferred + addend, + transferred = min(transferred + addend, expected + addend), expected = expected + addend ) } diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt index ea7bc4a2..47235213 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt @@ -20,7 +20,7 @@ private val TAG = NotificationBackupObserver::class.java.simpleName internal class NotificationBackupObserver( private val context: Context, private val backupRequester: BackupRequester, - private val expectedPackages: Int, + private val requestedPackages: Int, appTotals: ExpectedAppTotals, ) : IBackupObserver.Stub(), KoinComponent { @@ -32,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) } /** @@ -77,7 +77,7 @@ internal class NotificationBackupObserver( override fun backupFinished(status: Int) { if (backupRequester.requestNext()) { if (isLoggable(TAG, INFO)) { - Log.i(TAG, "Backup finished $numPackages/$expectedPackages. Status: $status") + Log.i(TAG, "Backup finished $numPackages/$requestedPackages. Status: $status") } val success = status == 0 val numBackedUp = if (success) metadataManager.getPackagesNumBackedUp() else null @@ -89,13 +89,17 @@ internal class NotificationBackupObserver( private fun showProgressNotification(packageName: String?) { if (packageName == null || currentPackage == packageName) return - if (isLoggable(TAG, INFO)) { - "Showing progress notification for $currentPackage $numPackages/$expectedPackages".let { - Log.i(TAG, it) - } - } + if (isLoggable(TAG, INFO)) Log.i( + TAG, "Showing progress notification for " + + "$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) } From b47b4ade1efdbe4f683fa0eebd381ae461bba50f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 6 Feb 2024 13:50:28 -0300 Subject: [PATCH 3/4] Stop cleaning up notification when service is destroyed. --- .../notification/BackupNotificationManager.kt | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt index f2cbd003..3ba30dae 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt @@ -184,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 -> - // 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) - } - } + // FIXME the service gets destroyed for each chunk when requesting backup in chunks + // This leads to the cancellation of an ongoing backup notification. + // So for now, we'll remove automatic notification clean-up + // and find out if it is still necessary. If not, this comment can be removed. + // 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) { From a586ee6b14b6a0b33f26fa40eb21f7d548ed9cf0 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 14 Feb 2024 10:40:01 -0300 Subject: [PATCH 4/4] In instrumentation tests, keep incremental backups If we request backup in several chunks, packages like 'pm@' or 'android' get backed up for each chunk, so due to incremental backups, we need to keep old data when comparing. --- .../stevesoltys/seedvault/e2e/LargeBackupTestBase.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/app/src/androidTest/java/com/stevesoltys/seedvault/e2e/LargeBackupTestBase.kt b/app/src/androidTest/java/com/stevesoltys/seedvault/e2e/LargeBackupTestBase.kt index f4537cd2..82d2e492 100644 --- a/app/src/androidTest/java/com/stevesoltys/seedvault/e2e/LargeBackupTestBase.kt +++ b/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() + // @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()