WIP: Use BackupManagerMonitor to inform transport of no data changed

The fake package manager package is essential for the backup, but when its data doesn't change and we request a normal incremental backup, it doesn't get included, because our transport doesn't even get called for it. Only the BackupMonitor gets a hint that it had no (new?) data via LOG_EVENT_ID_NO_DATA_TO_SEND. This behavior started with Android 15 that fixed a bug that caused @pm@ to always backup. However, other K/V apps were probably affected before.
seedvault-app · Oct 2, 2024 · 88ec3c1 · 88ec3c1
1 parent 1dee14f
commit 88ec3c1
Show file tree

Hide file tree

Showing 9 changed files with 90 additions and 12 deletions.
diff --git a/app/src/main/java/com/stevesoltys/seedvault/BackupMonitor.kt b/app/src/main/java/com/stevesoltys/seedvault/BackupMonitor.kt
@@ -17,18 +17,26 @@ import android.util.Log.DEBUG
 
 private val TAG = BackupMonitor::class.java.name
 
-class BackupMonitor : IBackupManagerMonitor.Stub() {
+open class BackupMonitor : IBackupManagerMonitor.Stub() {
 
     override fun onEvent(bundle: Bundle) {
-        val id = bundle.getInt(EXTRA_LOG_EVENT_ID)
-        val packageName = bundle.getString(EXTRA_LOG_EVENT_PACKAGE_NAME, "?")
+        onEvent(
+            id = bundle.getInt(EXTRA_LOG_EVENT_ID),
+            category = bundle.getInt(EXTRA_LOG_EVENT_CATEGORY),
+            packageName = bundle.getString(EXTRA_LOG_EVENT_PACKAGE_NAME)
+                ?: error("no package name for $bundle"),
+            bundle = bundle,
+        )
+    }
+
+    open fun onEvent(id: Int, category: Int, packageName: String, bundle: Bundle) {
         if (id == LOG_EVENT_ID_ERROR_PREFLIGHT) {
             val preflightResult = bundle.getLong(EXTRA_LOG_PREFLIGHT_ERROR, -1)
             Log.w(TAG, "Pre-flight error from $packageName: $preflightResult")
         }
         if (!Log.isLoggable(TAG, DEBUG)) return
         Log.d(TAG, "ID: $id")
-        Log.d(TAG, "CATEGORY: " + bundle.getInt(EXTRA_LOG_EVENT_CATEGORY, -1))
+        Log.d(TAG, "CATEGORY: $category")
         Log.d(TAG, "PACKAGE: $packageName")
     }
 

diff --git a/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotCreator.kt b/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotCreator.kt
@@ -16,6 +16,7 @@ import android.provider.Settings
 import android.provider.Settings.Secure.ANDROID_ID
 import com.google.protobuf.ByteString
 import com.stevesoltys.seedvault.Clock
+import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.header.VERSION
 import com.stevesoltys.seedvault.metadata.BackupType
 import com.stevesoltys.seedvault.metadata.MetadataManager
@@ -134,6 +135,8 @@ internal class SnapshotCreator(
             putAllApps(appBuilderMap.mapValues { it.value.build() })
             putAllBlobs(this@SnapshotCreator.blobsMap)
         }.build()
+        // may as well fail the backup, if @pm@ isn't in it
+        check(MAGIC_PACKAGE_MANAGER in snapshot.appsMap) { "No metadata for @pm@" }
         appBuilderMap.clear()
         snapshotBuilder.clear()
         blobsMap.clear()

diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt
@@ -9,6 +9,7 @@ import org.koin.android.ext.koin.androidContext
 import org.koin.dsl.module
 
 val backupModule = module {
+    factory { BackupTransportMonitor(get()) }
     single { BackupInitializer(get()) }
     single { InputFactory() }
     single {

diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupTransportMonitor.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupTransportMonitor.kt
@@ -0,0 +1,52 @@
+/*
+ * SPDX-FileCopyrightText: 2024 The Calyx Institute
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.stevesoltys.seedvault.transport.backup
+
+import android.app.backup.BackupManagerMonitor.LOG_EVENT_CATEGORY_BACKUP_MANAGER_POLICY
+import android.app.backup.BackupManagerMonitor.LOG_EVENT_ID_NO_DATA_TO_SEND
+import android.app.backup.BackupTransport.FLAG_DATA_NOT_CHANGED
+import android.app.backup.BackupTransport.TRANSPORT_OK
+import android.content.pm.PackageInfo
+import android.os.Bundle
+import android.os.ParcelFileDescriptor
+import android.os.ParcelFileDescriptor.MODE_READ_ONLY
+import com.stevesoltys.seedvault.BackupMonitor
+import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
+import io.github.oshai.kotlinlogging.KotlinLogging
+import kotlinx.coroutines.runBlocking
+import java.io.File
+
+internal class BackupTransportMonitor(
+    private val backupCoordinator: BackupCoordinator,
+) : BackupMonitor() {
+
+    private val log = KotlinLogging.logger { }
+
+    private var backedUpPm = false
+
+    override fun onEvent(id: Int, category: Int, packageName: String, bundle: Bundle) {
+        super.onEvent(id, category, packageName, bundle)
+        if (id == LOG_EVENT_ID_NO_DATA_TO_SEND &&
+            category == LOG_EVENT_CATEGORY_BACKUP_MANAGER_POLICY
+        ) {
+            sendNoDataChanged(packageName)
+        }
+    }
+
+    private fun sendNoDataChanged(packageName: String) {
+        log.info { "sendNoDataChanged($packageName) $backedUpPm" }
+        val packageInfo = PackageInfo().apply { this.packageName = packageName }
+        val data = ParcelFileDescriptor.open(File("/dev/null"), MODE_READ_ONLY)
+        val result =
+            backupCoordinator.performIncrementalBackup(packageInfo, data, FLAG_DATA_NOT_CHANGED)
+        if (result == TRANSPORT_OK) runBlocking {
+            // FIXME where are on a different thread than normal backup here,
+            //  so we compete for BackupReceiver which isn't thread safe
+            backupCoordinator.finishBackup()
+            if (packageName == MAGIC_PACKAGE_MANAGER) backedUpPm = true
+        }
+    }
+}
diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt
@@ -14,6 +14,7 @@ import android.app.backup.BackupTransport.TRANSPORT_OK
 import android.content.pm.PackageInfo
 import android.os.ParcelFileDescriptor
 import android.util.Log
+import com.stevesoltys.seedvault.NO_DATA_END_SENTINEL
 import com.stevesoltys.seedvault.repo.BackupData
 import com.stevesoltys.seedvault.repo.BackupReceiver
 import java.io.IOException
@@ -52,6 +53,8 @@ internal class KVBackup(
             else -> Log.i(TAG, "Performing K/V backup for $packageName")
         }
         check(state == null) { "Have unexpected state for ${state?.packageInfo?.packageName}" }
+        // This fake package name just signals that we've seen all packages without new data
+        if (packageName == NO_DATA_END_SENTINEL) return TRANSPORT_OK
 
         // initialize state
         state = KVBackupState(packageInfo = packageInfo, db = dbManager.getDb(packageName))

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
@@ -59,18 +59,17 @@ internal class PackageService(
                 logPackages(packages)
             }
 
-            val eligibleApps = packages.filter(::shouldIncludeAppInBackup).toTypedArray()
+            val eligibleApps = packages.filter(::shouldIncludeAppInBackup).toMutableList()
             // log eligible packages
             if (Log.isLoggable(TAG, INFO)) {
                 Log.i(TAG, "Filtering left ${eligibleApps.size} eligible packages:")
-                logPackages(eligibleApps.toList())
+                logPackages(eligibleApps)
             }
 
             // add magic @pm@ package (PACKAGE_MANAGER_SENTINEL) which holds package manager data
-            val packageArray = eligibleApps.toMutableList()
-            packageArray.add(MAGIC_PACKAGE_MANAGER)
+            eligibleApps.add(0, MAGIC_PACKAGE_MANAGER)
 
-            return packageArray
+            return eligibleApps
         }
 
     /**

diff --git a/app/src/main/java/com/stevesoltys/seedvault/worker/BackupRequester.kt b/app/src/main/java/com/stevesoltys/seedvault/worker/BackupRequester.kt
@@ -11,7 +11,7 @@ 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.transport.backup.BackupTransportMonitor
 import com.stevesoltys.seedvault.transport.backup.PackageService
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
 import com.stevesoltys.seedvault.ui.notification.NotificationBackupObserver
@@ -32,6 +32,7 @@ internal const val NUM_PACKAGES_PER_TRANSACTION = 100
 internal class BackupRequester(
     context: Context,
     private val backupManager: IBackupManager,
+    private val monitor: BackupTransportMonitor,
     val packageService: PackageService,
 ) : KoinComponent {
 
@@ -43,7 +44,6 @@ internal class BackupRequester(
         backupRequester = this,
         requestedPackages = packages.size,
     )
-    private val monitor = BackupMonitor()
 
     /**
      * The current package index.

diff --git a/app/src/main/java/com/stevesoltys/seedvault/worker/WorkerModule.kt b/app/src/main/java/com/stevesoltys/seedvault/worker/WorkerModule.kt
@@ -13,6 +13,7 @@ val workerModule = module {
         BackupRequester(
             context = androidContext(),
             backupManager = get(),
+            monitor = get(),
             packageService = get(),
         )
     }

diff --git a/app/src/test/java/com/stevesoltys/seedvault/repo/SnapshotCreatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/repo/SnapshotCreatorTest.kt
@@ -15,6 +15,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.stevesoltys.seedvault.TestApp
 import com.stevesoltys.seedvault.header.VERSION
 import com.stevesoltys.seedvault.metadata.BackupType
+import com.stevesoltys.seedvault.metadata.BackupType.KV
 import com.stevesoltys.seedvault.proto.Snapshot
 import com.stevesoltys.seedvault.transport.TransportTest
 import com.stevesoltys.seedvault.transport.backup.PackageService
@@ -40,12 +41,18 @@ internal class SnapshotCreatorTest : TransportTest() {
     private val packageService: PackageService = mockk()
     private val snapshotCreator = SnapshotCreator(ctx, clock, packageService, metadataManager)
 
+    init {
+        every { packageService.launchableSystemApps } returns emptyList()
+        every { metadataManager.onPackageBackedUp(pmPackageInfo, any(), any()) } just Runs
+    }
+
     @Test
     fun `test onApkBackedUp`() {
         every { applicationInfo.loadLabel(any()) } returns name
         every { clock.time() } returns token
 
         snapshotCreator.onApkBackedUp(packageInfo, apk, blobMap)
+        snapshotCreator.onPackageBackedUp(pmPackageInfo, KV, BackupData(emptyList(), emptyMap()))
         val s = snapshotCreator.finalizeSnapshot()
 
         assertEquals(apk, s.appsMap[packageName]?.apk)
@@ -72,6 +79,7 @@ internal class SnapshotCreatorTest : TransportTest() {
         every { packageService.launchableSystemApps } returns listOf(resolveInfo)
 
         snapshotCreator.onPackageBackedUp(packageInfo, BackupType.FULL, apkBackupData)
+        snapshotCreator.onPackageBackedUp(pmPackageInfo, KV, BackupData(emptyList(), emptyMap()))
         val s = snapshotCreator.finalizeSnapshot()
 
         assertEquals(name, s.appsMap[packageName]?.name)
@@ -94,6 +102,7 @@ internal class SnapshotCreatorTest : TransportTest() {
         every { packageService.launchableSystemApps } returns emptyList()
 
         snapshotCreator.onPackageBackedUp(packageInfo, BackupType.FULL, apkBackupData)
+        snapshotCreator.onPackageBackedUp(pmPackageInfo, KV, BackupData(emptyList(), emptyMap()))
         snapshotCreator.finalizeSnapshot()
     }
 
@@ -102,6 +111,7 @@ internal class SnapshotCreatorTest : TransportTest() {
         every { clock.time() } returns token andThen token + 1
 
         snapshotCreator.onIconsBackedUp(apkBackupData)
+        snapshotCreator.onPackageBackedUp(pmPackageInfo, KV, BackupData(emptyList(), emptyMap()))
         val s = snapshotCreator.finalizeSnapshot()
 
         assertEquals(apkBackupData.chunkIds.forProto(), s.iconChunkIdsList)
@@ -112,6 +122,7 @@ internal class SnapshotCreatorTest : TransportTest() {
     fun `test finalize`() {
         every { clock.time() } returns token
 
+        snapshotCreator.onPackageBackedUp(pmPackageInfo, KV, BackupData(emptyList(), emptyMap()))
         val s = snapshotCreator.finalizeSnapshot()
 
         assertEquals(VERSION, s.version.toByte())
@@ -122,7 +133,7 @@ internal class SnapshotCreatorTest : TransportTest() {
         assertEquals(34, s.sdkInt) // as per config above, needs bump once possible
         assertEquals("unknown", s.androidIncremental)
         assertTrue(s.d2D)
-        assertEquals(0, s.appsCount)
+        assertEquals(1, s.appsCount)
         assertEquals(0, s.iconChunkIdsCount)
         assertEquals(emptyMap<String, Snapshot.Blob>(), s.blobsMap)
     }