Polish AppBackupManager and write tests

app/src/main/java/com/stevesoltys/seedvault/transport/backup/AppBackupManager.kt

@@ -17,7 +17,13 @@ import org.calyxos.seedvault.core.backends.AppBackupFileType.Blob
 import org.calyxos.seedvault.core.backends.AppBackupFileType.Snapshot
 import org.calyxos.seedvault.core.backends.FileInfo
 import org.calyxos.seedvault.core.backends.TopLevelFolder
+import java.io.IOException
 
+/**
+ * Manages the process of app data backups, especially related to work that needs to happen
+ * before and after a backup run.
+ * See [beforeBackup] and [afterBackupFinished].
+ */
 internal class AppBackupManager(
     private val crypto: Crypto,
     private val blobCache: BlobCache,
@@ -28,10 +34,25 @@ internal class AppBackupManager(
 ) {
 
     private val log = KotlinLogging.logger {}
+
+    /**
+     * A temporary [SnapshotCreator] that has a lifetime only valid during the backup run.
+     */
     var snapshotCreator: SnapshotCreator? = null
         private set
 
+    /**
+     * Call this method before doing any kind of backup work.
+     * It will
+     * * download the blobs available on the backend,
+     * * assemble the chunk ID to blob mapping from previous snapshots and
+     * * create a new instance of a [SnapshotCreator].
+     *
+     * @throws IOException or other exceptions.
+     * These should be caught by the caller who may retry us on transient errors.
+     */
     @WorkerThread
+    @Throws(IOException::class)
     suspend fun beforeBackup() {
         log.info { "Loading existing snapshots and blobs..." }
         val blobInfos = mutableListOf<FileInfo>()
@@ -46,11 +67,21 @@ internal class AppBackupManager(
                 else -> error("Unexpected FileHandle: $fileInfo")
             }
         }
-        snapshotCreator = snapshotCreatorFactory.createSnapshotCreator()
+        log.info { "Found ${snapshotHandles.size} existing snapshots." }
         val snapshots = snapshotManager.onSnapshotsLoaded(snapshotHandles)
         blobCache.populateCache(blobInfos, snapshots)
+        snapshotCreator = snapshotCreatorFactory.createSnapshotCreator()
     }
 
+    /**
+     * This must be called after the backup run has been completed.
+     * It finalized the current snapshot and saves it to the backend.
+     * Then, it clears up the [BlobCache] and the [SnapshotCreator].
+     *
+     * @param success true if the backup run was successful, false otherwise.
+     *
+     * @return the snapshot saved to the backend or null if there was an error saving it.
+     */
     @WorkerThread
     suspend fun afterBackupFinished(success: Boolean): com.stevesoltys.seedvault.proto.Snapshot? {
         MemoryLogger.log()
@@ -59,11 +90,15 @@ internal class AppBackupManager(
         blobCache.clear()
         return try {
             if (success) {
-                val snapshot =
+                // only save snapshot when backup was successful,
-                    snapshotCreator?.finalizeSnapshot() ?: error("Had no snapshotCreator")
+                // otherwise we'd have partial snapshots
-                keepTrying { // saving this is so important, we even keep trying
+                val snapshot = snapshotCreator?.finalizeSnapshot()
+                    ?: error("Had no snapshotCreator")
+                keepTrying { // TODO remove when we have auto-retrying backends
+                    // saving this is so important, we even keep trying
                     snapshotManager.saveSnapshot(snapshot)
                 }
+                // save token and time of last backup
                 settingsManager.onSuccessfulBackupCompleted(snapshot.token)
                 // after snapshot was written, we can clear local cache as its info is in snapshot
                 blobCache.clearLocalCache()
@@ -86,7 +121,7 @@ internal class AppBackupManager(
             } catch (e: Exception) {
                 if (i == n) throw e
                 log.error(e) { "Error (#$i), we'll keep trying" }
-                delay(1000)
+                delay(1000 * i.toLong())
             }
         }
     }

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

@@ -145,7 +145,7 @@ class AppBackupWorker(
             try {
                 appBackupManager.beforeBackup()
             } catch (e: Exception) {
-                Log.e(TAG, "Error populating blobs cache: ", e)
+                Log.e(TAG, "Error during 'beforeBackup': ", e)
                 return Result.retry()
             }
         }

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

@@ -0,0 +1,141 @@
+/*
+ * SPDX-FileCopyrightText: 2024 The Calyx Institute
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.stevesoltys.seedvault.transport.backup
+
+import com.stevesoltys.seedvault.backend.BackendManager
+import com.stevesoltys.seedvault.transport.SnapshotManager
+import com.stevesoltys.seedvault.transport.TransportTest
+import io.mockk.Runs
+import io.mockk.andThenJust
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import kotlinx.coroutines.runBlocking
+import org.calyxos.seedvault.core.backends.AppBackupFileType.Blob
+import org.calyxos.seedvault.core.backends.AppBackupFileType.Snapshot
+import org.calyxos.seedvault.core.backends.Backend
+import org.calyxos.seedvault.core.backends.FileInfo
+import org.calyxos.seedvault.core.backends.TopLevelFolder
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertNull
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+import java.io.IOException
+import kotlin.random.Random
+
+internal class AppBackupManagerTest : TransportTest() {
+
+    private val blobCache: BlobCache = mockk()
+    private val backendManager: BackendManager = mockk()
+    private val backend: Backend = mockk()
+    private val snapshotManager: SnapshotManager = mockk()
+    private val snapshotCreatorFactory: SnapshotCreatorFactory = mockk()
+    private val appBackupManager = AppBackupManager(
+        crypto = crypto,
+        blobCache = blobCache,
+        backendManager = backendManager,
+        settingsManager = settingsManager,
+        snapshotManager = snapshotManager,
+        snapshotCreatorFactory = snapshotCreatorFactory,
+    )
+
+    private val snapshotCreator: SnapshotCreator = mockk()
+
+    @Test
+    fun `beforeBackup passes exception on`() = runBlocking {
+        every { backendManager.backend } returns backend
+        every { crypto.repoId } returns repoId
+        coEvery {
+            backend.list(TopLevelFolder(repoId), Blob::class, Snapshot::class, callback = any())
+        } throws IOException()
+
+        assertThrows<IOException> {
+            appBackupManager.beforeBackup()
+        }
+        Unit
+    }
+
+    @Test
+    fun `beforeBackup passes on blobs, snapshots and initializes SnapshotCreator`() = runBlocking {
+        val snapshotHandle = Snapshot(repoId, "foo bar")
+        val snapshotInfo = FileInfo(snapshotHandle, Random.nextLong())
+        val top = TopLevelFolder(repoId)
+        every { backendManager.backend } returns backend
+        every { crypto.repoId } returns repoId
+        coEvery {
+            backend.list(top, Blob::class, Snapshot::class, callback = captureLambda())
+        } answers {
+            lambda<(FileInfo) -> Unit>().captured.invoke(fileInfo1)
+            lambda<(FileInfo) -> Unit>().captured.invoke(fileInfo2)
+            lambda<(FileInfo) -> Unit>().captured.invoke(snapshotInfo)
+        }
+        coEvery {
+            snapshotManager.onSnapshotsLoaded(listOf(snapshotHandle))
+        } returns listOf(snapshot)
+        every { blobCache.populateCache(listOf(fileInfo1, fileInfo2), listOf(snapshot)) } just Runs
+        every { snapshotCreatorFactory.createSnapshotCreator() } returns mockk()
+
+        appBackupManager.beforeBackup()
+
+        coVerify {
+            snapshotManager.onSnapshotsLoaded(listOf(snapshotHandle))
+            blobCache.populateCache(listOf(fileInfo1, fileInfo2), listOf(snapshot))
+            snapshotCreatorFactory.createSnapshotCreator()
+        }
+    }
+
+    @Test
+    fun `afterBackupFinished doesn't save snapshot on failure`() = runBlocking {
+        every { blobCache.clear() } just Runs
+
+        assertNull(appBackupManager.afterBackupFinished(false))
+    }
+
+    @Test
+    fun `afterBackupFinished doesn't throw exception`() = runBlocking {
+        // need to run beforeBackup to get a snapshotCreator
+        minimalBeforeBackup()
+
+        every { blobCache.clear() } just Runs
+        every { snapshotCreator.finalizeSnapshot() } returns snapshot
+        coEvery { snapshotManager.saveSnapshot(snapshot) } throws IOException()
+
+        assertNull(appBackupManager.afterBackupFinished(true))
+    }
+
+    @Test
+    fun `afterBackupFinished retries saving snapshot`() = runBlocking {
+        // need to run beforeBackup to get a snapshotCreator
+        minimalBeforeBackup()
+
+        every { blobCache.clear() } just Runs
+        every { snapshotCreator.finalizeSnapshot() } returns snapshot
+        coEvery {
+            snapshotManager.saveSnapshot(snapshot) // works only at third attempt
+        } throws IOException() andThenThrows IOException() andThenJust Runs
+        every { settingsManager.onSuccessfulBackupCompleted(snapshot.token) } just Runs
+        every { blobCache.clearLocalCache() } just Runs
+
+        assertEquals(snapshot, appBackupManager.afterBackupFinished(true))
+    }
+
+    private suspend fun minimalBeforeBackup() {
+        every { backendManager.backend } returns backend
+        every { crypto.repoId } returns repoId
+        coEvery {
+            backend.list(any(), Blob::class, Snapshot::class, callback = any())
+        } just Runs
+        coEvery {
+            snapshotManager.onSnapshotsLoaded(emptyList())
+        } returns emptyList()
+        every { blobCache.populateCache(emptyList(), emptyList()) } just Runs
+        every { snapshotCreatorFactory.createSnapshotCreator() } returns snapshotCreator
+
+        appBackupManager.beforeBackup()
+    }
+}