From 5b567c79a20e3a302882eb9c479efdc63626f483 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 11 Sep 2024 17:38:20 -0300 Subject: [PATCH] Also snapshot unchanged APKs --- .../stevesoltys/seedvault/worker/ApkBackup.kt | 31 +++--- .../seedvault/worker/ApkBackupManager.kt | 4 +- .../seedvault/worker/WorkerModule.kt | 2 +- .../restore/install/ApkBackupRestoreTest.kt | 5 +- .../transport/backup/BackupCoordinatorTest.kt | 4 +- .../seedvault/worker/ApkBackupManagerTest.kt | 12 ++- .../seedvault/worker/ApkBackupTest.kt | 95 +++++++++++++++---- 7 files changed, 111 insertions(+), 42 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackup.kt b/app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackup.kt index 1d282982..003ded5d 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackup.kt +++ b/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, - // but we rely on the app themselves to re-download those, if needed after restore. + // build up chunkMap from old snapshot + 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, ): 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, + chunkMap: MutableMap, ): List { check(packageInfo.splitNames != null) // attention: though not documented, splitSourceDirs can be null diff --git a/app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackupManager.kt b/app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackupManager.kt index 5d772333..ef5ea76d 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackupManager.kt +++ b/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() diff --git a/app/src/main/java/com/stevesoltys/seedvault/worker/WorkerModule.kt b/app/src/main/java/com/stevesoltys/seedvault/worker/WorkerModule.kt index 0546c3b6..45f7e6dc 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/worker/WorkerModule.kt +++ b/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(), diff --git a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt index f613fbf8..83ee8864 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt +++ b/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 = - ApkBackup(pm, backupReceiver, appBackupManager, snapshotManager, settingsManager) + private val apkBackup = ApkBackup(pm, backupReceiver, appBackupManager, settingsManager) private val apkRestore: ApkRestore = ApkRestore( context = strictContext, backupManager = backupManager, @@ -153,7 +152,7 @@ internal class ApkBackupRestoreTest : TransportTest() { snapshotCreator.onApkBackedUp(packageInfo, any(), chunkMap) } just Runs - apkBackup.backupApkIfNecessary(packageInfo) + apkBackup.backupApkIfNecessary(packageInfo, snapshot) assertArrayEquals(apkBytes, outputStream.toByteArray()) assertArrayEquals(splitBytes, splitOutputStream.toByteArray()) diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt index 35bf022d..3424355e 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt +++ b/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 } diff --git a/app/src/test/java/com/stevesoltys/seedvault/worker/ApkBackupManagerTest.kt b/app/src/test/java/com/stevesoltys/seedvault/worker/ApkBackupManagerTest.kt index 82ef99c4..5d9773bb 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/worker/ApkBackupManagerTest.kt +++ b/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[1]) + apkBackup.backupApkIfNecessary(notAllowedPackages[0], snapshot) + apkBackup.backupApkIfNecessary(notAllowedPackages[1], snapshot) } } diff --git a/app/src/test/java/com/stevesoltys/seedvault/worker/ApkBackupTest.kt b/app/src/test/java/com/stevesoltys/seedvault/worker/ApkBackupTest.kt index a2c012dc..588ef1e0 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/worker/ApkBackupTest.kt +++ b/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 = - ApkBackup(pm, backupReceiver, appBackupManager, snapshotManager, settingsManager) + private val apkBackup = ApkBackup(pm, backupReceiver, appBackupManager, 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 { + it.signaturesList != apk.signaturesList + }, apkBackupData.chunkMap) + } just Runs + + apkBackup.backupApkIfNecessary(packageInfo, s) + + coVerify { + backupReceiver.readFromStream(any()) + backupReceiver.finalize() + snapshotCreator.onApkBackedUp(packageInfo, match { + 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