From 5c75574f6539e7a66c96cc7d0b992190446a7d8a Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Mon, 23 Sep 2024 16:09:05 -0300
Subject: [PATCH] Use cached snapshots for auto-restore to save time
All snapshots we wrote out should be cached locally. Auto-restore is holding up app installs, so we should be as fast as possible.
---
.../seedvault/repo/SnapshotManager.kt | 19 +++++++++++++-
.../transport/restore/RestorableBackup.kt | 1 +
.../transport/restore/RestoreCoordinator.kt | 17 ++++++++----
.../seedvault/repo/SnapshotManagerTest.kt | 26 +++++++++++++++----
.../restore/RestoreCoordinatorTest.kt | 22 +++++-----------
5 files changed, 59 insertions(+), 26 deletions(-)
diff --git a/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotManager.kt b/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotManager.kt
index f3946045..6d3bde26 100644
--- a/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotManager.kt
+++ b/app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotManager.kt
@@ -12,6 +12,7 @@ import com.stevesoltys.seedvault.header.VERSION
import com.stevesoltys.seedvault.proto.Snapshot
import io.github.oshai.kotlinlogging.KotlinLogging
import org.calyxos.seedvault.core.backends.AppBackupFileType
+import org.calyxos.seedvault.core.backends.Constants.appSnapshotRegex
import org.calyxos.seedvault.core.toHexString
import java.io.ByteArrayOutputStream
import java.io.File
@@ -25,13 +26,14 @@ internal const val FOLDER_SNAPSHOTS = "snapshots"
* Also keeps a reference to the [latestSnapshot] that holds important re-usable data.
*/
internal class SnapshotManager(
- private val snapshotFolder: File,
+ private val snapshotFolderRoot: File,
private val crypto: Crypto,
private val loader: Loader,
private val backendManager: BackendManager,
) {
private val log = KotlinLogging.logger {}
+ private val snapshotFolder: File get() = File(snapshotFolderRoot, crypto.repoId)
/**
* The latest [Snapshot]. May be stale if [onSnapshotsLoaded] has not returned
@@ -123,6 +125,7 @@ internal class SnapshotManager(
@Throws(IOException::class)
suspend fun loadSnapshot(snapshotHandle: AppBackupFileType.Snapshot): Snapshot {
val file = File(snapshotFolder, snapshotHandle.name)
+ snapshotFolder.mkdirs()
val inputStream = if (file.isFile) {
loader.loadFile(file, snapshotHandle.hash)
} else {
@@ -131,4 +134,18 @@ internal class SnapshotManager(
return inputStream.use { Snapshot.parseFrom(it) }
}
+ @Throws(IOException::class)
+ fun loadCachedSnapshots(): List<Snapshot> {
+ if (!snapshotFolder.isDirectory) return emptyList()
+ return snapshotFolder.listFiles()?.mapNotNull { file ->
+ val match = appSnapshotRegex.matchEntire(file.name)
+ if (match == null) {
+ log.error { "Unexpected file found: $file" }
+ null
+ } else {
+ loader.loadFile(file, match.groupValues[1]).use { Snapshot.parseFrom(it) }
+ }
+ } ?: throw IOException("Could not access snapshotFolder")
+ }
+
}
diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorableBackup.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorableBackup.kt
index 9ff14b29..cbadca53 100644
--- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorableBackup.kt
+++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorableBackup.kt
@@ -20,6 +20,7 @@ data class RestorableBackup(
val snapshot: Snapshot? = null,
) {
+ // FIXME creating this mapping is expensive, a single call can take several seconds to complete
constructor(repoId: String, snapshot: Snapshot) : this(
backupMetadata = BackupMetadata.fromSnapshot(snapshot),
repoId = repoId,
diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt
index 15fd465f..4958aa32 100644
--- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt
+++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt
@@ -216,13 +216,20 @@ internal class RestoreCoordinator(
?: return TRANSPORT_ERROR
backup.backups.find { it.token == token } ?: return TRANSPORT_ERROR
} else {
- // this is auto-restore, so we try harder to find a working restore set
+ // this is auto-restore, so we use cache and try hard to find a working restore set
Log.i(TAG, "No cached backups, loading all and look for $token")
- // TODO may be cold start and need snapshot loading (ideally from cache only?)
- val backup = getAvailableBackups() as? RestorableBackupResult.SuccessResult
- ?: return TRANSPORT_ERROR
+ val backups = try {
+ snapshotManager.loadCachedSnapshots().map { snapshot ->
+ RestorableBackup(crypto.repoId, snapshot)
+ }
+ } catch (e: Exception) {
+ Log.e(TAG, "Error loading cached snapshots: ", e)
+ (getAvailableBackups() as? RestorableBackupResult.SuccessResult)?.backups
+ ?: return TRANSPORT_ERROR
+ }
+ Log.i(TAG, "Found ${backups.size} snapshots.")
val autoRestorePackageName = autoRestorePackageInfo.packageName
- val sortedBackups = backup.backups.sortedByDescending { it.token }
+ val sortedBackups = backups.sortedByDescending { it.token } // latest first
sortedBackups.find { it.token == token } ?: sortedBackups.find {
val chunkIds = it.packageMetadataMap[autoRestorePackageName]?.chunkIds
// try a backup where our auto restore package has data
diff --git a/app/src/test/java/com/stevesoltys/seedvault/repo/SnapshotManagerTest.kt b/app/src/test/java/com/stevesoltys/seedvault/repo/SnapshotManagerTest.kt
index 960f9125..ee712c7f 100644
--- a/app/src/test/java/com/stevesoltys/seedvault/repo/SnapshotManagerTest.kt
+++ b/app/src/test/java/com/stevesoltys/seedvault/repo/SnapshotManagerTest.kt
@@ -52,6 +52,11 @@ internal class SnapshotManagerTest : TransportTest() {
every { backendManager.backend } returns backend
}
+ private fun getSnapshotFolder(tmpDir: Path, hash: String): File {
+ val repoFolder = File(tmpDir.toString(), repoId)
+ return File(repoFolder, hash)
+ }
+
@Test
fun `test onSnapshotsLoaded sets latestSnapshot`(@TempDir tmpDir: Path) = runBlocking {
val snapshotManager = getSnapshotManager(File(tmpDir.toString()))
@@ -63,6 +68,7 @@ internal class SnapshotManagerTest : TransportTest() {
val snapshotHandle1 = AppBackupFileType.Snapshot(repoId, chunkId1)
val snapshotHandle2 = AppBackupFileType.Snapshot(repoId, chunkId2)
+ every { crypto.repoId } returns repoId
coEvery { loader.loadFile(snapshotHandle1, any()) } returns inputStream1
coEvery { loader.loadFile(snapshotHandle2, any()) } returns inputStream2
snapshotManager.onSnapshotsLoaded(listOf(snapshotHandle1, snapshotHandle2))
@@ -87,7 +93,7 @@ internal class SnapshotManagerTest : TransportTest() {
snapshotManager.saveSnapshot(snapshot)
- val snapshotFile = File(tmpDir.toString(), snapshotHandle.name)
+ val snapshotFile = getSnapshotFolder(tmpDir, snapshotHandle.name)
assertTrue(snapshotFile.isFile)
assertTrue(outputStream.size() > 0)
val cachedBytes = snapshotFile.inputStream().use { it.readAllBytes() }
@@ -97,21 +103,28 @@ internal class SnapshotManagerTest : TransportTest() {
@Test
fun `snapshot loads from cache without backend`(@TempDir tmpDir: Path) = runBlocking {
val snapshotManager = getSnapshotManager(File(tmpDir.toString()))
- val snapshotData = snapshot { token = 1337 }.toByteArray()
+ val snapshot = snapshot { token = 1337 }
+ val snapshotData = snapshot.toByteArray()
val inputStream = ByteArrayInputStream(snapshotData)
val snapshotHandle = AppBackupFileType.Snapshot(repoId, chunkId1)
// create cached file
- val file = File(tmpDir.toString(), snapshotHandle.name)
+ val file = getSnapshotFolder(tmpDir, snapshotHandle.name)
+ file.parentFile?.mkdirs()
file.outputStream().use { it.write(snapshotData) }
+ every { crypto.repoId } returns repoId
coEvery { loader.loadFile(file, snapshotHandle.hash) } returns inputStream
- snapshotManager.onSnapshotsLoaded(listOf(snapshotHandle))
+ assertEquals(listOf(snapshot), snapshotManager.onSnapshotsLoaded(listOf(snapshotHandle)))
coVerify(exactly = 0) { // did not load from backend
loader.loadFile(snapshotHandle, any())
}
+
+ // now load all snapshots from cache
+ inputStream.reset()
+ assertEquals(listOf(snapshot), snapshotManager.loadCachedSnapshots())
}
@Test
@@ -124,6 +137,7 @@ internal class SnapshotManagerTest : TransportTest() {
val snapshotHandle1 = AppBackupFileType.Snapshot(repoId, chunkId1)
val snapshotHandle2 = AppBackupFileType.Snapshot(repoId, chunkId2)
+ every { crypto.repoId } returns repoId
coEvery { loader.loadFile(snapshotHandle1, any()) } returns inputStream
coEvery { loader.loadFile(snapshotHandle2, any()) } throws IOException()
snapshotManager.onSnapshotsLoaded(listOf(snapshotHandle1, snapshotHandle2))
@@ -179,10 +193,12 @@ internal class SnapshotManagerTest : TransportTest() {
val snapshotManager = getSnapshotManager(File(tmpDir.toString()))
val snapshotHandle = AppBackupFileType.Snapshot(repoId, chunkId1)
- val file = File(tmpDir.toString(), snapshotHandle.name)
+ val file = getSnapshotFolder(tmpDir, snapshotHandle.name)
+ file.parentFile?.mkdirs()
file.createNewFile()
assertTrue(file.isFile)
+ every { crypto.repoId } returns repoId
coEvery { backend.remove(snapshotHandle) } just Runs
snapshotManager.removeSnapshot(snapshotHandle)
diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt
index 76bd9da1..fd468f51 100644
--- a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt
+++ b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt
@@ -256,26 +256,18 @@ internal class RestoreCoordinatorTest : TransportTest() {
}
@Test
- fun `startRestore() loads snapshots for auto-restore`() = runBlocking {
- val handle = AppBackupFileType.Snapshot(repoId, getRandomByteArray(32).toHexString())
- val info = FileInfo(handle, 1)
-
+ fun `startRestore() loads snapshots for auto-restore from local cache`() = runBlocking {
every { backendManager.backendProperties } returns safStorage
every { safStorage.isUnavailableUsb(context) } returns false
- coEvery {
- backend.list(
- topLevelFolder = null,
- AppBackupFileType.Snapshot::class, LegacyAppBackupFile.Metadata::class,
- callback = captureLambda<(FileInfo) -> Unit>()
- )
- } answers {
- val callback = lambda<(FileInfo) -> Unit>().captured
- callback(info)
- }
- coEvery { snapshotManager.loadSnapshot(handle) } returns snapshot
+ every { crypto.repoId } returns repoId
+ every { snapshotManager.loadCachedSnapshots() } returns listOf(snapshot)
assertEquals(TRANSPORT_OK, restore.startRestore(token, pmPackageInfoArray))
+
+ verify {
+ snapshotManager.loadCachedSnapshots()
+ }
}
@Test