Let backup notification report more fine-grained progress

This adds @pm@ record backup and APK backup of opt-out apps to the
progress reporting since these two operations are slow when using a
cloud storage SAF backend.
This commit is contained in:
Torsten Grote 2020-08-26 17:28:03 -03:00 committed by Chirayu Desai
parent 740fe53a52
commit d2c426db93
11 changed files with 254 additions and 50 deletions

View file

@ -10,6 +10,7 @@ import android.app.PendingIntent.FLAG_UPDATE_CURRENT
import android.content.Context
import android.content.Intent
import android.content.pm.PackageManager.NameNotFoundException
import android.util.Log
import androidx.core.app.NotificationCompat.Action
import androidx.core.app.NotificationCompat.Builder
import androidx.core.app.NotificationCompat.PRIORITY_DEFAULT
@ -28,6 +29,8 @@ private const val NOTIFICATION_ID_OBSERVER = 1
private const val NOTIFICATION_ID_ERROR = 2
private const val NOTIFICATION_ID_RESTORE_ERROR = 3
private val TAG = BackupNotificationManager::class.java.simpleName
class BackupNotificationManager(private val context: Context) {
private val nm = context.getSystemService(NotificationManager::class.java)!!.apply {
@ -35,6 +38,9 @@ class BackupNotificationManager(private val context: Context) {
createNotificationChannel(getErrorChannel())
createNotificationChannel(getRestoreErrorChannel())
}
private var expectedApps: Int? = null
private var expectedOptOutApps: Int? = null
private var expectedPmRecords: Int? = null
private fun getObserverChannel(): NotificationChannel {
val title = context.getString(R.string.notification_channel_title)
@ -53,11 +59,92 @@ class BackupNotificationManager(private val context: Context) {
return NotificationChannel(CHANNEL_ID_RESTORE_ERROR, title, IMPORTANCE_HIGH)
}
fun onBackupUpdate(app: CharSequence, transferred: Int, expected: Int, userInitiated: Boolean) {
/**
* Call this right after starting a backup.
*
* We can not know [expectedPmRecords] here, because this number varies between backup runs
* and is only known when the system tells us to update [MAGIC_PACKAGE_MANAGER].
*/
fun onBackupStarted(
expectedPackages: Int,
expectedOptOutPackages: Int,
userInitiated: Boolean
) {
updateBackupNotification(
contentText = "", // This passes quickly, no need to show something here
transferred = 0,
expected = expectedPackages,
userInitiated = userInitiated
)
expectedApps = expectedPackages
expectedOptOutApps = expectedOptOutPackages
}
/**
* This is expected to get called before [onOptOutAppBackup] and [onBackupUpdate].
*/
fun onPmKvBackup(packageName: String, transferred: Int, expected: Int) {
if (expectedApps == null) {
Log.d(TAG, "Expected number of apps unknown. Not showing @pm@ notification.")
return
}
val appName = getAppName(context, packageName)
val contentText = context.getString(R.string.notification_content_package_manager, appName)
val addend = (expectedOptOutApps ?: 0) + (expectedApps ?: 0)
updateBackupNotification(
contentText = contentText,
transferred = transferred,
expected = expected + addend,
userInitiated = false
)
expectedPmRecords = expected
}
/**
* This should get called after [onPmKvBackup], but before [onBackupUpdate].
*/
fun onOptOutAppBackup(packageName: String, transferred: Int, expected: Int) {
if (expectedApps == null) {
Log.d(TAG, "Expected number of apps unknown. Not showing APK notification.")
return
}
val appName = getAppName(context, packageName)
val contentText = context.getString(R.string.notification_content_opt_out_app, appName)
updateBackupNotification(
contentText = contentText,
transferred = transferred + (expectedPmRecords ?: 0),
expected = expected + (expectedApps ?: 0) + (expectedPmRecords ?: 0),
userInitiated = false
)
expectedOptOutApps = expected
}
/**
* In the series of notification updates,
* this type is is expected to get called after [onOptOutAppBackup] and [onPmKvBackup].
*/
fun onBackupUpdate(app: CharSequence, transferred: Int, userInitiated: Boolean) {
val expected = expectedApps ?: error("expectedApps is null")
val addend = (expectedOptOutApps ?: 0) + (expectedPmRecords ?: 0)
updateBackupNotification(
contentText = app,
transferred = transferred + addend,
expected = expected + addend,
userInitiated = userInitiated
)
}
private fun updateBackupNotification(
contentText: CharSequence,
transferred: Int,
expected: Int,
userInitiated: Boolean
) {
Log.i(TAG, "$transferred/$expected $contentText")
val notification = Builder(context, CHANNEL_ID_OBSERVER).apply {
setSmallIcon(R.drawable.ic_cloud_upload)
setContentTitle(context.getString(R.string.notification_title))
setContentText(app)
setContentText(contentText)
setOngoing(true)
setShowWhen(false)
setWhen(System.currentTimeMillis())
@ -72,13 +159,14 @@ class BackupNotificationManager(private val context: Context) {
nm.cancel(NOTIFICATION_ID_OBSERVER)
return
}
val titleRes = if (success) R.string.notification_success_title else R.string.notification_failed_title
val titleRes =
if (success) R.string.notification_success_title else R.string.notification_failed_title
val contentText = if (notBackedUp == null) null else {
context.getString(R.string.notification_success_num_not_backed_up, notBackedUp)
}
val iconRes = if (success) R.drawable.ic_cloud_done else R.drawable.ic_cloud_error
val intent = Intent(context, SettingsActivity::class.java).apply {
action = ACTION_APP_STATUS_LIST
if (success) action = ACTION_APP_STATUS_LIST
}
val pendingIntent = PendingIntent.getActivity(context, 0, intent, 0)
val notification = Builder(context, CHANNEL_ID_OBSERVER).apply {
@ -94,6 +182,10 @@ class BackupNotificationManager(private val context: Context) {
priority = PRIORITY_LOW
}.build()
nm.notify(NOTIFICATION_ID_OBSERVER, notification)
// reset number of expected apps
expectedOptOutApps = null
expectedPmRecords = null
expectedApps = null
}
fun onBackupError() {
@ -128,7 +220,8 @@ class BackupNotificationManager(private val context: Context) {
setPackage(context.packageName)
putExtra(EXTRA_PACKAGE_NAME, packageName)
}
val pendingIntent = PendingIntent.getBroadcast(context, REQUEST_CODE_UNINSTALL, intent, FLAG_UPDATE_CURRENT)
val pendingIntent =
PendingIntent.getBroadcast(context, REQUEST_CODE_UNINSTALL, intent, FLAG_UPDATE_CURRENT)
val actionText = context.getString(R.string.notification_restore_error_action)
val action = Action(R.drawable.ic_warning, actionText, pendingIntent)
val notification = Builder(context, CHANNEL_ID_RESTORE_ERROR).apply {

View file

@ -16,6 +16,7 @@ private val TAG = NotificationBackupObserver::class.java.simpleName
class NotificationBackupObserver(
private val context: Context,
private val expectedPackages: Int,
expectedOptOutPackages: Int,
private val userInitiated: Boolean
) : IBackupObserver.Stub(), KoinComponent {
@ -25,20 +26,18 @@ class NotificationBackupObserver(
private var numPackages: Int = 0
init {
// we need to show this manually as [onUpdate] isn't called for first @pm@ package
// TODO consider showing something else for MAGIC_PACKAGE_MANAGER,
// because we also back up APKs at the beginning and this can take quite some time.
// Therefore, also consider showing a more fine-grained progress bar
// by (roughly) doubling the number [expectedPackages] (probably -3)
// and calling back here from KvBackup and ApkBackup to update progress.
// We will also need to take [PackageService#notAllowedPackages] into account.
nm.onBackupUpdate(getAppName(MAGIC_PACKAGE_MANAGER), 0, expectedPackages, userInitiated)
// Inform the notification manager that a backup has started
// and inform about the expected numbers, so it can compute a total.
nm.onBackupStarted(expectedPackages, expectedOptOutPackages, userInitiated)
}
/**
* This method could be called several times for packages with full data backup.
* It will tell how much of backup data is already saved and how much is expected.
*
* Note that this will not be called for [MAGIC_PACKAGE_MANAGER]
* which is usually the first package to get backed up.
*
* @param currentBackupPackage The name of the package that now being backed up.
* @param backupProgress Current progress of backup for the package.
*/
@ -91,7 +90,7 @@ class NotificationBackupObserver(
currentPackage = packageName
val app = getAppName(packageName)
numPackages += 1
nm.onBackupUpdate(app, numPackages, expectedPackages, userInitiated)
nm.onBackupUpdate(app, numPackages, userInitiated)
}
private fun getAppName(packageId: String): CharSequence = getAppName(context, packageId)
@ -99,7 +98,9 @@ class NotificationBackupObserver(
}
fun getAppName(context: Context, packageId: String): CharSequence {
if (packageId == MAGIC_PACKAGE_MANAGER) return context.getString(R.string.restore_magic_package)
if (packageId == MAGIC_PACKAGE_MANAGER || packageId.startsWith("@")) {
return context.getString(R.string.restore_magic_package)
}
return try {
val appInfo = context.packageManager.getApplicationInfo(packageId, 0)
context.packageManager.getApplicationLabel(appInfo) ?: packageId

View file

@ -53,8 +53,9 @@ class ConfigurableBackupTransportService : Service() {
fun requestBackup(context: Context) {
val packageService: PackageService = get().koin.get()
val packages = packageService.eligiblePackages
val optOutPackages = packageService.notAllowedPackages
val observer = NotificationBackupObserver(context, packages.size, true)
val observer = NotificationBackupObserver(context, packages.size, optOutPackages.size, true)
val result = try {
val backupManager: IBackupManager = get().koin.get()
backupManager.requestBackup(packages, observer, BackupMonitor(), FLAG_USER_INITIATED)

View file

@ -160,10 +160,13 @@ internal class BackupCoordinator(
if (getBackupBackoff() != 0L) {
return TRANSPORT_PACKAGE_REJECTED
}
}
val result = kv.performBackup(packageInfo, data, flags)
if (result == TRANSPORT_OK && packageName == MAGIC_PACKAGE_MANAGER) {
// hook in here to back up APKs of apps that are otherwise not allowed for backup
backUpNotAllowedPackages()
}
return kv.performBackup(packageInfo, data, flags)
return result
}
// ------------------------------------------------------------------------------------
@ -285,8 +288,10 @@ internal class BackupCoordinator(
private suspend fun backUpNotAllowedPackages() {
Log.d(TAG, "Checking if APKs of opt-out apps need backup...")
packageService.notAllowedPackages.forEach { optOutPackageInfo ->
val notAllowedPackages = packageService.notAllowedPackages
notAllowedPackages.forEachIndexed { i, optOutPackageInfo ->
try {
nm.onOptOutAppBackup(optOutPackageInfo.packageName, i + 1, notAllowedPackages.size)
backUpApk(optOutPackageInfo, NOT_ALLOWED)
} catch (e: IOException) {
Log.e(TAG, "Error backing up opt-out APK of ${optOutPackageInfo.packageName}", e)

View file

@ -7,7 +7,7 @@ val backupModule = module {
single { InputFactory() }
single { PackageService(androidContext().packageManager, get()) }
single { ApkBackup(androidContext().packageManager, get(), get()) }
single { KVBackup(get<BackupPlugin>().kvBackupPlugin, get(), get(), get()) }
single { KVBackup(get<BackupPlugin>().kvBackupPlugin, get(), get(), get(), get()) }
single { FullBackup(get<BackupPlugin>().fullBackupPlugin, get(), get(), get()) }
single { BackupCoordinator(androidContext(), get(), get(), get(), get(), get(), get(), get(), get(), get()) }
}

View file

@ -8,6 +8,8 @@ import android.app.backup.BackupTransport.TRANSPORT_OK
import android.content.pm.PackageInfo
import android.os.ParcelFileDescriptor
import android.util.Log
import com.stevesoltys.seedvault.BackupNotificationManager
import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
import com.stevesoltys.seedvault.crypto.Crypto
import com.stevesoltys.seedvault.encodeBase64
import com.stevesoltys.seedvault.header.HeaderWriter
@ -26,7 +28,8 @@ internal class KVBackup(
private val plugin: KVBackupPlugin,
private val inputFactory: InputFactory,
private val headerWriter: HeaderWriter,
private val crypto: Crypto
private val crypto: Crypto,
private val nm: BackupNotificationManager
) {
private var state: KVBackupState? = null
@ -101,32 +104,29 @@ internal class KVBackup(
}
private suspend fun storeRecords(packageInfo: PackageInfo, data: ParcelFileDescriptor): Int {
val backupSequence: Iterable<Result<KVOperation>>
val pmRecordNumber: Int?
if (packageInfo.packageName == MAGIC_PACKAGE_MANAGER) {
// Since the package manager has many small keys to store,
// and this can be slow, especially on cloud-based storage,
// we get the entire data set first, so we can show progress notifications.
val list = parseBackupStream(data).toList()
backupSequence = list
pmRecordNumber = list.size
} else {
backupSequence = parseBackupStream(data).asIterable()
pmRecordNumber = null
}
// apply the delta operations
for (result in parseBackupStream(data)) {
var i = 1
for (result in backupSequence) {
if (result is Result.Error) {
Log.e(TAG, "Exception reading backup input", result.exception)
return backupError(TRANSPORT_ERROR)
}
val op = (result as Result.Ok).result
try {
if (op.value == null) {
Log.e(TAG, "Deleting record with base64Key ${op.base64Key}")
plugin.deleteRecord(packageInfo, op.base64Key)
} else {
val outputStream = plugin.getOutputStreamForRecord(packageInfo, op.base64Key)
try {
val header = VersionHeader(
packageName = packageInfo.packageName,
key = op.key
)
headerWriter.writeVersion(outputStream, header)
crypto.encryptHeader(outputStream, header)
crypto.encryptMultipleSegments(outputStream, op.value)
outputStream.flush()
} finally {
closeQuietly(outputStream)
}
}
storeRecord(packageInfo, op, i++, pmRecordNumber)
} catch (e: IOException) {
Log.e(TAG, "Unable to update base64Key file for base64Key ${op.base64Key}", e)
// Returning something more forgiving such as TRANSPORT_PACKAGE_REJECTED
@ -140,6 +140,38 @@ internal class KVBackup(
return TRANSPORT_OK
}
@Throws(IOException::class)
private suspend fun storeRecord(
packageInfo: PackageInfo,
op: KVOperation,
currentNum: Int,
pmRecordNumber: Int?
) {
// update notification for package manager backup
if (pmRecordNumber != null) {
nm.onPmKvBackup(op.key, currentNum, pmRecordNumber)
}
// check if record should get deleted
if (op.value == null) {
Log.e(TAG, "Deleting record with base64Key ${op.base64Key}")
plugin.deleteRecord(packageInfo, op.base64Key)
} else {
val outputStream = plugin.getOutputStreamForRecord(packageInfo, op.base64Key)
try {
val header = VersionHeader(
packageName = packageInfo.packageName,
key = op.key
)
headerWriter.writeVersion(outputStream, header)
crypto.encryptHeader(outputStream, header)
crypto.encryptMultipleSegments(outputStream, op.value)
outputStream.flush()
} finally {
closeQuietly(outputStream)
}
}
}
/**
* Parses a backup stream into individual key/value operations
*/
@ -206,12 +238,12 @@ internal class KVBackup(
}
private class KVOperation(
internal val key: String,
internal val base64Key: String,
val key: String,
val base64Key: String,
/**
* value is null when this is a deletion operation
*/
internal val value: ByteArray?
val value: ByteArray?
)
private sealed class Result<out T> {

View file

@ -73,6 +73,10 @@
<!-- Notification -->
<string name="notification_channel_title">Backup notification</string>
<string name="notification_title">Backup running</string>
<!-- This is shown in a backup notification when metadata for an app is being backed up -->
<string name="notification_content_package_manager">Metadata for %s</string>
<!-- This is shown in a backup notification when *only* the APK of an app that opts out of backup gets backed up -->
<string name="notification_content_opt_out_app">Only app %s</string>
<string name="notification_backup_result_complete">Backup complete</string>
<string name="notification_backup_result_rejected">Not backed up</string>
<string name="notification_backup_result_error">Backup failed</string>

View file

@ -61,15 +61,15 @@ internal class CoordinatorIntegrationTest : TransportTest() {
private val headerReader = HeaderReaderImpl()
private val cryptoImpl = CryptoImpl(cipherFactory, headerWriter, headerReader)
private val metadataReader = MetadataReaderImpl(cryptoImpl)
private val notificationManager = mockk<BackupNotificationManager>()
private val backupPlugin = mockk<BackupPlugin>()
private val kvBackupPlugin = mockk<KVBackupPlugin>()
private val kvBackup = KVBackup(kvBackupPlugin, inputFactory, headerWriter, cryptoImpl)
private val kvBackup = KVBackup(kvBackupPlugin, inputFactory, headerWriter, cryptoImpl, notificationManager)
private val fullBackupPlugin = mockk<FullBackupPlugin>()
private val fullBackup = FullBackup(fullBackupPlugin, inputFactory, headerWriter, cryptoImpl)
private val apkBackup = mockk<ApkBackup>()
private val packageService: PackageService = mockk()
private val notificationManager = mockk<BackupNotificationManager>()
private val backup = BackupCoordinator(
context,
backupPlugin,

View file

@ -8,6 +8,7 @@ import android.content.pm.PackageInfo
import android.content.pm.SigningInfo
import android.util.Log
import com.stevesoltys.seedvault.Clock
import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
import com.stevesoltys.seedvault.crypto.Crypto
import com.stevesoltys.seedvault.metadata.MetadataManager
import com.stevesoltys.seedvault.settings.SettingsManager
@ -36,6 +37,9 @@ abstract class TransportTest {
}
signingInfo = sigInfo
}
protected val pmPackageInfo = PackageInfo().apply {
packageName = MAGIC_PACKAGE_MANAGER
}
init {
mockkStatic(Log::class)

View file

@ -315,7 +315,18 @@ internal class BackupCoordinatorTest : BackupTest() {
val packageMetadata: PackageMetadata = mockk()
every { settingsManager.getStorage() } returns storage // to check for removable storage
// do actual @pm@ backup
coEvery { kv.performBackup(packageInfo, fileDescriptor, 0) } returns TRANSPORT_OK
// now check if we have opt-out apps that we need to back up APKs for
every { packageService.notAllowedPackages } returns notAllowedPackages
// update notification
every {
notificationManager.onOptOutAppBackup(
notAllowedPackages[0].packageName,
1,
notAllowedPackages.size
)
} just Runs
// no backup needed
coEvery {
apkBackup.backupApkIfNecessary(
@ -324,6 +335,14 @@ internal class BackupCoordinatorTest : BackupTest() {
any()
)
} returns null
// update notification
every {
notificationManager.onOptOutAppBackup(
notAllowedPackages[1].packageName,
2,
notAllowedPackages.size
)
} just Runs
// was backed up, get new packageMetadata
coEvery {
apkBackup.backupApkIfNecessary(
@ -340,8 +359,6 @@ internal class BackupCoordinatorTest : BackupTest() {
metadataOutputStream
)
} just Runs
// do actual @pm@ backup
coEvery { kv.performBackup(packageInfo, fileDescriptor, 0) } returns TRANSPORT_OK
every { metadataOutputStream.close() } just Runs
assertEquals(

View file

@ -6,6 +6,8 @@ import android.app.backup.BackupTransport.FLAG_NON_INCREMENTAL
import android.app.backup.BackupTransport.TRANSPORT_ERROR
import android.app.backup.BackupTransport.TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED
import android.app.backup.BackupTransport.TRANSPORT_OK
import android.content.pm.PackageInfo
import com.stevesoltys.seedvault.BackupNotificationManager
import com.stevesoltys.seedvault.Utf8
import com.stevesoltys.seedvault.getRandomString
import com.stevesoltys.seedvault.header.MAX_KEY_LENGTH_SIZE
@ -16,6 +18,7 @@ import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.verify
import io.mockk.verifyOrder
import kotlinx.coroutines.runBlocking
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
@ -30,8 +33,9 @@ internal class KVBackupTest : BackupTest() {
private val plugin = mockk<KVBackupPlugin>()
private val dataInput = mockk<BackupDataInput>()
private val notificationManager = mockk<BackupNotificationManager>()
private val backup = KVBackup(plugin, inputFactory, headerWriter, crypto)
private val backup = KVBackup(plugin, inputFactory, headerWriter, crypto, notificationManager)
private val key = getRandomString(MAX_KEY_LENGTH_SIZE)
private val key64 = Base64.getEncoder().encodeToString(key.toByteArray(Utf8))
@ -53,6 +57,49 @@ internal class KVBackupTest : BackupTest() {
assertFalse(backup.hasState())
}
@Test
fun `@pm@ backup shows notification`() = runBlocking {
// init plugin and give back two keys
initPlugin(true, pmPackageInfo)
createBackupDataInput()
every { dataInput.readNextHeader() } returnsMany listOf(true, true, false)
every { dataInput.key } returnsMany listOf("key1", "key2")
// we don't care about values, so just use the same one always
every { dataInput.dataSize } returns value.size
every { dataInput.readEntityData(any(), 0, value.size) } returns value.size
// store first record and show notification for it
every { notificationManager.onPmKvBackup("key1", 1, 2) } just Runs
coEvery { plugin.getOutputStreamForRecord(pmPackageInfo, "a2V5MQ") } returns outputStream
val versionHeader1 = VersionHeader(packageName = pmPackageInfo.packageName, key = "key1")
every { headerWriter.writeVersion(outputStream, versionHeader1) } just Runs
every { crypto.encryptHeader(outputStream, versionHeader1) } just Runs
// store second record and show notification for it
every { notificationManager.onPmKvBackup("key2", 2, 2) } just Runs
coEvery { plugin.getOutputStreamForRecord(pmPackageInfo, "a2V5Mg") } returns outputStream
val versionHeader2 = VersionHeader(packageName = pmPackageInfo.packageName, key = "key2")
every { headerWriter.writeVersion(outputStream, versionHeader2) } just Runs
every { crypto.encryptHeader(outputStream, versionHeader2) } just Runs
// encrypt to and close output stream
every { crypto.encryptMultipleSegments(outputStream, any()) } just Runs
every { outputStream.write(value) } just Runs
every { outputStream.flush() } just Runs
every { outputStream.close() } just Runs
assertEquals(TRANSPORT_OK, backup.performBackup(pmPackageInfo, data, 0))
assertTrue(backup.hasState())
assertEquals(TRANSPORT_OK, backup.finishBackup())
assertFalse(backup.hasState())
// verify that notifications were shown
verifyOrder {
notificationManager.onPmKvBackup("key1", 1, 2)
notificationManager.onPmKvBackup("key2", 2, 2)
}
}
@Test
fun `incremental backup with no data gets rejected`() = runBlocking {
coEvery { plugin.hasDataForPackage(packageInfo) } returns false
@ -210,9 +257,9 @@ internal class KVBackupTest : BackupTest() {
every { outputStream.close() } just Runs
}
private fun initPlugin(hasDataForPackage: Boolean = false) {
coEvery { plugin.hasDataForPackage(packageInfo) } returns hasDataForPackage
coEvery { plugin.ensureRecordStorageForPackage(packageInfo) } just Runs
private fun initPlugin(hasDataForPackage: Boolean = false, pi: PackageInfo = packageInfo) {
coEvery { plugin.hasDataForPackage(pi) } returns hasDataForPackage
coEvery { plugin.ensureRecordStorageForPackage(pi) } just Runs
}
private fun createBackupDataInput() {