From b59da2a805802e71dc9017a0dcfc70b82ba147ac Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 28 Aug 2024 15:46:16 -0300 Subject: [PATCH] Ensure ChunkWriter uses current backend When changing backends, the ChunkWriter could still use the old one causing data loss, because chunks assumed to exist on new backend, were written to old one. --- .../backup/storage/SnapshotRetriever.kt | 4 +-- .../calyxos/backup/storage/backup/Backup.kt | 3 +- .../backup/storage/backup/ChunkWriter.kt | 3 +- .../backup/storage/BackupRestoreTest.kt | 34 +++++++++++++++++++ .../backup/storage/backup/ChunkWriterTest.kt | 4 ++- .../backup/SmallFileBackupIntegrationTest.kt | 4 ++- 6 files changed, 46 insertions(+), 6 deletions(-) diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/SnapshotRetriever.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/SnapshotRetriever.kt index e1934204..31442596 100644 --- a/storage/lib/src/main/java/org/calyxos/backup/storage/SnapshotRetriever.kt +++ b/storage/lib/src/main/java/org/calyxos/backup/storage/SnapshotRetriever.kt @@ -17,7 +17,7 @@ import java.io.IOException import java.security.GeneralSecurityException internal class SnapshotRetriever( - private val storagePlugin: () -> Backend, + private val backendGetter: () -> Backend, private val streamCrypto: StreamCrypto = StreamCrypto, ) { @@ -27,7 +27,7 @@ internal class SnapshotRetriever( InvalidProtocolBufferException::class, ) suspend fun getSnapshot(streamKey: ByteArray, storedSnapshot: StoredSnapshot): BackupSnapshot { - return storagePlugin().load(storedSnapshot.snapshotHandle).use { inputStream -> + return backendGetter().load(storedSnapshot.snapshotHandle).use { inputStream -> val version = inputStream.readVersion() val timestamp = storedSnapshot.timestamp val ad = streamCrypto.getAssociatedDataForSnapshot(timestamp, version.toByte()) diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/Backup.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/Backup.kt index 6c78fa71..ad8548b4 100644 --- a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/Backup.kt +++ b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/Backup.kt @@ -74,7 +74,8 @@ internal class Backup( } catch (e: GeneralSecurityException) { throw AssertionError(e) } - private val chunkWriter = ChunkWriter(streamCrypto, streamKey, chunksCache, backend, androidId) + private val chunkWriter = + ChunkWriter(streamCrypto, streamKey, chunksCache, backendGetter, androidId) private val hasMediaAccessPerm = context.checkSelfPermission(ACCESS_MEDIA_LOCATION) == PERMISSION_GRANTED private val fileBackup = FileBackup( diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ChunkWriter.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ChunkWriter.kt index f146d328..1424ed30 100644 --- a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ChunkWriter.kt +++ b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ChunkWriter.kt @@ -31,11 +31,12 @@ internal class ChunkWriter( private val streamCrypto: StreamCrypto, private val streamKey: ByteArray, private val chunksCache: ChunksCache, - private val backend: Backend, + private val backendGetter: () -> Backend, private val androidId: String, private val bufferSize: Int = DEFAULT_BUFFER_SIZE, ) { + private val backend get() = backendGetter() private val buffer = ByteArray(bufferSize) @Throws(IOException::class, GeneralSecurityException::class) diff --git a/storage/lib/src/test/java/org/calyxos/backup/storage/BackupRestoreTest.kt b/storage/lib/src/test/java/org/calyxos/backup/storage/BackupRestoreTest.kt index f8092d39..f5310b19 100644 --- a/storage/lib/src/test/java/org/calyxos/backup/storage/BackupRestoreTest.kt +++ b/storage/lib/src/test/java/org/calyxos/backup/storage/BackupRestoreTest.kt @@ -14,6 +14,7 @@ import android.text.format.Formatter import io.mockk.Runs import io.mockk.coEvery import io.mockk.coVerify +import io.mockk.coVerifyOrder import io.mockk.every import io.mockk.just import io.mockk.mockk @@ -475,6 +476,39 @@ internal class BackupRestoreTest { } } + @Test + fun testBackupUpdatesBackend(): Unit = runBlocking { + val backendGetterNew: () -> Backend = mockk() + val backend1: Backend = mockk() + val backend2: Backend = mockk() + val backup = Backup( + context = context, + db = db, + fileScanner = fileScanner, + backendGetter = backendGetterNew, + androidId = androidId, + keyManager = keyManager, + cacheRepopulater = cacheRepopulater, + ) + every { backendGetterNew() } returnsMany listOf(backend1, backend2) + + coEvery { backend1.list(any(), Blob::class, callback = any()) } just Runs + every { chunksCache.areAllAvailableChunksCached(db, emptySet()) } returns true + every { fileScanner.getFiles() } returns FileScannerResult(emptyList(), emptyList()) + every { filesCache.getByUri(any()) } returns null // nothing is cached, all is new + + backup.runBackup(null) + + // second run uses new backend + coEvery { backend2.list(any(), Blob::class, callback = any()) } just Runs + backup.runBackup(null) + + coVerifyOrder { + backend1.list(any(), Blob::class, callback = any()) + backend2.list(any(), Blob::class, callback = any()) + } + } + private fun getRandomMediaFile(size: Int) = MediaFile( uri = mockk(), dir = getRandomString(), diff --git a/storage/lib/src/test/java/org/calyxos/backup/storage/backup/ChunkWriterTest.kt b/storage/lib/src/test/java/org/calyxos/backup/storage/backup/ChunkWriterTest.kt index 034c25f6..8dd886ac 100644 --- a/storage/lib/src/test/java/org/calyxos/backup/storage/backup/ChunkWriterTest.kt +++ b/storage/lib/src/test/java/org/calyxos/backup/storage/backup/ChunkWriterTest.kt @@ -32,6 +32,7 @@ internal class ChunkWriterTest { private val streamCrypto: StreamCrypto = mockk() private val chunksCache: ChunksCache = mockk() + private val backendGetter: () -> Backend = mockk() private val backend: Backend = mockk() private val androidId: String = getRandomString() private val streamKey: ByteArray = Random.nextBytes(KEY_SIZE_BYTES) @@ -42,7 +43,7 @@ internal class ChunkWriterTest { streamCrypto = streamCrypto, streamKey = streamKey, chunksCache = chunksCache, - backend = backend, + backendGetter = backendGetter, androidId = androidId, bufferSize = Random.nextInt(1, 42), ) @@ -53,6 +54,7 @@ internal class ChunkWriterTest { init { mockLog() + every { backendGetter() } returns backend } @Test diff --git a/storage/lib/src/test/java/org/calyxos/backup/storage/backup/SmallFileBackupIntegrationTest.kt b/storage/lib/src/test/java/org/calyxos/backup/storage/backup/SmallFileBackupIntegrationTest.kt index aad3f5b2..3c46f07c 100644 --- a/storage/lib/src/test/java/org/calyxos/backup/storage/backup/SmallFileBackupIntegrationTest.kt +++ b/storage/lib/src/test/java/org/calyxos/backup/storage/backup/SmallFileBackupIntegrationTest.kt @@ -39,6 +39,7 @@ internal class SmallFileBackupIntegrationTest { private val filesCache: FilesCache = mockk() private val mac: Mac = mockk() private val chunksCache: ChunksCache = mockk() + private val backendGetter: () -> Backend = mockk() private val backend: Backend = mockk() private val androidId: String = getRandomString() @@ -46,7 +47,7 @@ internal class SmallFileBackupIntegrationTest { streamCrypto = StreamCrypto, streamKey = Random.nextBytes(KEY_SIZE_BYTES), chunksCache = chunksCache, - backend = backend, + backendGetter = backendGetter, androidId = androidId, ) private val zipChunker = ZipChunker( @@ -58,6 +59,7 @@ internal class SmallFileBackupIntegrationTest { init { mockLog() + every { backendGetter() } returns backend } /**