Also snapshot unchanged APKs

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

@@ -14,9 +14,10 @@ import android.util.PackageUtils.computeSha256DigestBytes
 import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.metadata.PackageMetadata
 import com.stevesoltys.seedvault.proto.Snapshot
+import com.stevesoltys.seedvault.proto.Snapshot.Apk
+import com.stevesoltys.seedvault.proto.Snapshot.Blob
 import com.stevesoltys.seedvault.proto.SnapshotKt.split
 import com.stevesoltys.seedvault.settings.SettingsManager
-import com.stevesoltys.seedvault.transport.SnapshotManager
 import com.stevesoltys.seedvault.transport.backup.AppBackupManager
 import com.stevesoltys.seedvault.transport.backup.BackupReceiver
 import com.stevesoltys.seedvault.transport.backup.forProto
@@ -35,7 +36,6 @@ internal class ApkBackup(
     private val pm: PackageManager,
     private val backupReceiver: BackupReceiver,
     private val appBackupManager: AppBackupManager,
-    private val snapshotManager: SnapshotManager,
     private val settingsManager: SettingsManager,
 ) {
 
@@ -50,7 +50,7 @@ internal class ApkBackup(
      * @return new [PackageMetadata] if an APK backup was made or null if no backup was made.
      */
     @Throws(IOException::class)
-    suspend fun backupApkIfNecessary(packageInfo: PackageInfo) {
+    suspend fun backupApkIfNecessary(packageInfo: PackageInfo, latestSnapshot: Snapshot?) {
         // do not back up @pm@
         val packageName = packageInfo.packageName
         if (packageName == MAGIC_PACKAGE_MANAGER) return
@@ -93,23 +93,33 @@ internal class ApkBackup(
 
         // get info from latest snapshot
         val version = packageInfo.longVersionCode
-        val oldApk = snapshotManager.latestSnapshot?.appsMap?.get(packageName)?.apk
+        val oldApk = latestSnapshot?.appsMap?.get(packageName)?.apk
         val backedUpVersion = oldApk?.versionCode ?: 0L // no version will cause backup
 
         // do not backup if we have the version already and signatures did not change
-        if (version <= backedUpVersion && !signaturesChanged(oldApk, signatures)) {
+        val needsBackup = version > backedUpVersion || signaturesChanged(oldApk, signatures)
+        if (!needsBackup && oldApk != null) {
+            // We could also check if there are new feature module splits to back up,
+            // but we rely on the app themselves to re-download those, if needed after restore.
             Log.d(
                 TAG, "Package $packageName with version $version" +
                     " already has a backup ($backedUpVersion)" +
                     " with the same signature. Not backing it up."
             )
-            // We could also check if there are new feature module splits to back up,
+            // build up chunkMap from old snapshot
-            // but we rely on the app themselves to re-download those, if needed after restore.
+            val chunkIds = oldApk.splitsList.flatMap {
+                it.chunkIdsList.map { chunkId -> chunkId.hexFromProto() }
+            }
+            val chunkMap = chunkIds.associateWith { chunkId ->
+                latestSnapshot.blobsMap[chunkId] ?: error("Missing blob for $chunkId")
+            }
+            // important: add old APK to snapshot or it wouldn't be part of backup
+            snapshotCreator.onApkBackedUp(packageInfo, oldApk, chunkMap)
             return
         }
 
         // builder for Apk object
-        val apkBuilder = Snapshot.Apk.newBuilder().apply {
+        val apkBuilder = Apk.newBuilder().apply {
             versionCode = version
             pm.getInstallSourceInfo(packageName).installingPackageName?.let {
                 // protobuf doesn't support null values
@@ -142,12 +152,11 @@ internal class ApkBackup(
         }
         val apk = apkBuilder.addAllSplits(splits).build()
         snapshotCreator.onApkBackedUp(packageInfo, apk, chunkMap)
-
         Log.d(TAG, "Backed up new APK of $packageName with version ${packageInfo.versionName}.")
     }
 
     private fun signaturesChanged(
-        apk: Snapshot.Apk?,
+        apk: Apk?,
         signatures: List<String>,
     ): Boolean {
         // no signatures counts as them not having changed
@@ -172,7 +181,7 @@ internal class ApkBackup(
     @Throws(IOException::class)
     private suspend fun backupSplitApks(
         packageInfo: PackageInfo,
-        chunkMap: MutableMap<String, Snapshot.Blob>,
+        chunkMap: MutableMap<String, Blob>,
     ): List<Snapshot.Split> {
         check(packageInfo.splitNames != null)
         // attention: though not documented, splitSourceDirs can be null

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

@@ -13,6 +13,7 @@ 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.settings.SettingsManager
+import com.stevesoltys.seedvault.transport.SnapshotManager
 import com.stevesoltys.seedvault.transport.backup.PackageService
 import com.stevesoltys.seedvault.transport.backup.isStopped
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
@@ -22,6 +23,7 @@ import java.io.IOException
 internal class ApkBackupManager(
     private val context: Context,
     private val settingsManager: SettingsManager,
+    private val snapshotManager: SnapshotManager,
     private val metadataManager: MetadataManager,
     private val packageService: PackageService,
     private val iconManager: IconManager,
@@ -99,7 +101,7 @@ internal class ApkBackupManager(
     private suspend fun backUpApk(packageInfo: PackageInfo) {
         val packageName = packageInfo.packageName
         try {
-            apkBackup.backupApkIfNecessary(packageInfo)
+            apkBackup.backupApkIfNecessary(packageInfo, snapshotManager.latestSnapshot)
         } catch (e: IOException) {
             Log.e(TAG, "Error while writing APK for $packageName", e)
             if (e.isOutOfSpace()) nm.onInsufficientSpaceError()

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

@@ -33,7 +33,6 @@ val workerModule = module {
             pm = androidContext().packageManager,
             backupReceiver = get(),
             appBackupManager = get(),
-            snapshotManager = get(),
             settingsManager = get(),
         )
     }
@@ -41,6 +40,7 @@ val workerModule = module {
         ApkBackupManager(
             context = androidContext(),
             settingsManager = get(),
+            snapshotManager = get(),
             metadataManager = get(),
             packageService = get(),
             apkBackup = get(),

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

@@ -82,8 +82,7 @@ internal class ApkBackupRestoreTest : TransportTest() {
     private val apkInstaller: ApkInstaller = mockk()
     private val installRestriction: InstallRestriction = mockk()
 
-    private val apkBackup =
+    private val apkBackup = ApkBackup(pm, backupReceiver, appBackupManager, settingsManager)
-        ApkBackup(pm, backupReceiver, appBackupManager, snapshotManager, settingsManager)
     private val apkRestore: ApkRestore = ApkRestore(
         context = strictContext,
         backupManager = backupManager,
@@ -153,7 +152,7 @@ internal class ApkBackupRestoreTest : TransportTest() {
             snapshotCreator.onApkBackedUp(packageInfo, any<Snapshot.Apk>(), chunkMap)
         } just Runs
 
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, snapshot)
 
         assertArrayEquals(apkBytes, outputStream.toByteArray())
         assertArrayEquals(splitBytes, splitOutputStream.toByteArray())

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

@@ -189,7 +189,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         coEvery {
             full.performFullBackup(packageInfo, fileDescriptor, 0)
         } returns TRANSPORT_OK
-        coEvery { apkBackup.backupApkIfNecessary(packageInfo) } just Runs
+        coEvery { apkBackup.backupApkIfNecessary(packageInfo, snapshot) } just Runs
 
         assertEquals(TRANSPORT_OK, backup.performFullBackup(packageInfo, fileDescriptor, 0))
     }
@@ -286,7 +286,7 @@ internal class BackupCoordinatorTest : BackupTest() {
     }
 
     private fun expectApkBackupAndMetadataWrite() {
-        coEvery { apkBackup.backupApkIfNecessary(packageInfo) } just Runs
+        coEvery { apkBackup.backupApkIfNecessary(packageInfo, snapshot) } just Runs
         every { metadataManager.onApkBackedUp(any(), packageMetadata) } just Runs
     }
 

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

@@ -15,6 +15,7 @@ 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.transport.SnapshotManager
 import com.stevesoltys.seedvault.transport.TransportTest
 import com.stevesoltys.seedvault.transport.backup.PackageService
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
@@ -32,6 +33,7 @@ import org.junit.jupiter.api.Test
 
 internal class ApkBackupManagerTest : TransportTest() {
 
+    private val snapshotManager: SnapshotManager = mockk()
     private val packageService: PackageService = mockk()
     private val apkBackup: ApkBackup = mockk()
     private val iconManager: IconManager = mockk()
@@ -42,6 +44,7 @@ internal class ApkBackupManagerTest : TransportTest() {
     private val apkBackupManager = ApkBackupManager(
         context = context,
         settingsManager = settingsManager,
+        snapshotManager = snapshotManager,
         metadataManager = metadataManager,
         packageService = packageService,
         iconManager = iconManager,
@@ -195,14 +198,15 @@ internal class ApkBackupManagerTest : TransportTest() {
         every {
             nm.onApkBackup(notAllowedPackages[0].packageName, any(), 0, notAllowedPackages.size)
         } just Runs
+        every { snapshotManager.latestSnapshot } returns snapshot
         // no backup needed
-        coEvery { apkBackup.backupApkIfNecessary(notAllowedPackages[0]) } just Runs
+        coEvery { apkBackup.backupApkIfNecessary(notAllowedPackages[0], snapshot) } just Runs
         // 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]) } just Runs
+        coEvery { apkBackup.backupApkIfNecessary(notAllowedPackages[1], snapshot) } just Runs
         every { metadataManager.onApkBackedUp(notAllowedPackages[1], packageMetadata) } just Runs
 
         every { nm.onApkBackupDone() } just Runs
@@ -210,8 +214,8 @@ internal class ApkBackupManagerTest : TransportTest() {
         apkBackupManager.backup()
 
         coVerify {
-            apkBackup.backupApkIfNecessary(notAllowedPackages[0])
+            apkBackup.backupApkIfNecessary(notAllowedPackages[0], snapshot)
-            apkBackup.backupApkIfNecessary(notAllowedPackages[1])
+            apkBackup.backupApkIfNecessary(notAllowedPackages[1], snapshot)
         }
     }
 

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

@@ -15,11 +15,11 @@ import android.content.pm.Signature
 import android.util.PackageUtils
 import com.google.protobuf.ByteString.copyFromUtf8
 import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
+import com.stevesoltys.seedvault.decodeBase64
 import com.stevesoltys.seedvault.getRandomString
 import com.stevesoltys.seedvault.proto.Snapshot
 import com.stevesoltys.seedvault.proto.SnapshotKt.app
 import com.stevesoltys.seedvault.proto.copy
-import com.stevesoltys.seedvault.transport.SnapshotManager
 import com.stevesoltys.seedvault.transport.backup.AppBackupManager
 import com.stevesoltys.seedvault.transport.backup.BackupData
 import com.stevesoltys.seedvault.transport.backup.BackupReceiver
@@ -27,11 +27,13 @@ import com.stevesoltys.seedvault.transport.backup.BackupTest
 import com.stevesoltys.seedvault.transport.backup.SnapshotCreator
 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.mockkStatic
 import io.mockk.slot
+import io.mockk.verify
 import kotlinx.coroutines.runBlocking
 import org.junit.jupiter.api.Assertions.assertArrayEquals
 import org.junit.jupiter.api.Assertions.assertThrows
@@ -49,11 +51,9 @@ internal class ApkBackupTest : BackupTest() {
     private val pm: PackageManager = mockk()
     private val backupReceiver: BackupReceiver = mockk()
     private val appBackupManager: AppBackupManager = mockk()
-    private val snapshotManager: SnapshotManager = mockk()
     private val snapshotCreator: SnapshotCreator = mockk()
 
-    private val apkBackup =
+    private val apkBackup = ApkBackup(pm, backupReceiver, appBackupManager, settingsManager)
-        ApkBackup(pm, backupReceiver, appBackupManager, snapshotManager, settingsManager)
 
     private val signatureBytes = byteArrayOf(0x01, 0x02, 0x03)
     private val signatureHash = byteArrayOf(0x03, 0x02, 0x01)
@@ -67,7 +67,7 @@ internal class ApkBackupTest : BackupTest() {
     @Test
     fun `does not back up @pm@`() = runBlocking {
         val packageInfo = PackageInfo().apply { packageName = MAGIC_PACKAGE_MANAGER }
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, null)
     }
 
     @Test
@@ -75,7 +75,7 @@ internal class ApkBackupTest : BackupTest() {
         every { settingsManager.backupApks() } returns false
         every { settingsManager.isBackupEnabled(any()) } returns true
 
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, null)
     }
 
     @Test
@@ -83,7 +83,7 @@ internal class ApkBackupTest : BackupTest() {
         every { settingsManager.backupApks() } returns true
         every { settingsManager.isBackupEnabled(any()) } returns false
 
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, null)
     }
 
     @Test
@@ -92,7 +92,7 @@ internal class ApkBackupTest : BackupTest() {
 
         every { settingsManager.isBackupEnabled(any()) } returns true
         every { settingsManager.backupApks() } returns true
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, null)
     }
 
     @Test
@@ -101,7 +101,7 @@ internal class ApkBackupTest : BackupTest() {
 
         every { settingsManager.isBackupEnabled(any()) } returns true
         every { settingsManager.backupApks() } returns true
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, null)
     }
 
     @Test
@@ -109,13 +109,61 @@ internal class ApkBackupTest : BackupTest() {
         packageInfo.applicationInfo!!.flags = FLAG_UPDATED_SYSTEM_APP
         val apk = apk.copy { versionCode = packageInfo.longVersionCode }
         val app = app { this.apk = apk }
-        expectChecks(snapshot.toBuilder().putApps(packageInfo.packageName, app).build())
+        val s = snapshot.copy { apps.put(packageName, app) }
+        expectChecks()
+        every {
+            snapshotCreator.onApkBackedUp(packageInfo, apk, chunkMap)
+        } just Runs
 
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, s)
+
+        // ensure we are still snapshotting this version
+        verify {
+            snapshotCreator.onApkBackedUp(packageInfo, apk, chunkMap)
+        }
     }
 
     @Test
-    fun `does back up the same version when signatures changes`() {
+    fun `does back up the same version when signatures changes`(@TempDir tmpDir: Path) =
+        runBlocking {
+            val tmpFile = File(tmpDir.toAbsolutePath().toString())
+            packageInfo.applicationInfo!!.sourceDir = File(tmpFile, "test.apk").apply {
+                assertTrue(createNewFile())
+            }.absolutePath
+            val apk = apk.copy {
+                versionCode = packageInfo.longVersionCode
+                signatures[0] = copyFromUtf8("AwIX".decodeBase64())
+                splits.clear()
+                splits.add(baseSplit)
+            }
+            val app = app { this.apk = apk }
+            val s = snapshot.copy { apps.put(packageName, app) }
+            expectChecks()
+            every {
+                pm.getInstallSourceInfo(packageInfo.packageName)
+            } returns InstallSourceInfo(null, null, null, apk.installer)
+            coEvery { backupReceiver.readFromStream(any()) } just Runs
+            coEvery { backupReceiver.finalize() } returns apkBackupData
+
+            every {
+                snapshotCreator.onApkBackedUp(packageInfo, match<Snapshot.Apk> {
+                    it.signaturesList != apk.signaturesList
+                }, apkBackupData.chunkMap)
+            } just Runs
+
+            apkBackup.backupApkIfNecessary(packageInfo, s)
+
+            coVerify {
+                backupReceiver.readFromStream(any())
+                backupReceiver.finalize()
+                snapshotCreator.onApkBackedUp(packageInfo, match<Snapshot.Apk> {
+                    it.signaturesList != apk.signaturesList
+                }, apkBackupData.chunkMap)
+            }
+        }
+
+    @Test
+    fun `throws exception when APK doesn't exist`() {
         packageInfo.applicationInfo!!.sourceDir = "/tmp/doesNotExist"
         val apk = apk.copy {
             signatures.clear()
@@ -123,14 +171,15 @@ internal class ApkBackupTest : BackupTest() {
             versionCode = packageInfo.longVersionCode
         }
         val app = app { this.apk = apk }
-        expectChecks(snapshot.toBuilder().putApps(packageInfo.packageName, app).build())
+        val s = snapshot.copy { apps.put(packageName, app) }
+        expectChecks()
         every {
             pm.getInstallSourceInfo(packageInfo.packageName)
         } returns InstallSourceInfo(null, null, null, getRandomString())
 
         assertThrows(IOException::class.java) {
             runBlocking {
-                apkBackup.backupApkIfNecessary(packageInfo)
+                apkBackup.backupApkIfNecessary(packageInfo, s)
             }
         }
         Unit
@@ -140,11 +189,10 @@ internal class ApkBackupTest : BackupTest() {
     fun `do not accept empty signature`() = runBlocking {
         every { settingsManager.backupApks() } returns true
         every { settingsManager.isBackupEnabled(any()) } returns true
-        every { snapshotManager.latestSnapshot } returns snapshot
         every { sigInfo.hasMultipleSigners() } returns false
         every { sigInfo.signingCertificateHistory } returns emptyArray()
 
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, snapshot)
     }
 
     @Test
@@ -173,8 +221,12 @@ internal class ApkBackupTest : BackupTest() {
             }, emptyMap())
         } just Runs
 
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, snapshot)
         assertArrayEquals(apkBytes, apkOutputStream.toByteArray())
+
+        coVerify {
+            backupReceiver.finalize()
+        }
     }
 
     @Test
@@ -230,16 +282,19 @@ internal class ApkBackupTest : BackupTest() {
             }, emptyMap())
         } just Runs
 
-        apkBackup.backupApkIfNecessary(packageInfo)
+        apkBackup.backupApkIfNecessary(packageInfo, snapshot)
         assertArrayEquals(apkBytes, apkOutputStream.toByteArray())
         assertArrayEquals(split1Bytes, split1OutputStream.toByteArray())
         assertArrayEquals(split2Bytes, split2OutputStream.toByteArray())
+
+        coVerify {
+            backupReceiver.finalize()
+        }
     }
 
-    private fun expectChecks(snapshot: Snapshot = this.snapshot) {
+    private fun expectChecks() {
         every { settingsManager.isBackupEnabled(any()) } returns true
         every { settingsManager.backupApks() } returns true
-        every { snapshotManager.latestSnapshot } returns snapshot
         every { PackageUtils.computeSha256DigestBytes(signatureBytes) } returns signatureHash
         every { sigInfo.hasMultipleSigners() } returns false
         every { sigInfo.signingCertificateHistory } returns sigs