Halt file backup when switched to metered network

grote · grote · commit cb6667c79a80 · 2025-01-21T14:25:39.000-03:00
diff --git a/app/src/main/java/com/stevesoltys/seedvault/backend/BackendManager.kt b/app/src/main/java/com/stevesoltys/seedvault/backend/BackendManager.kt
@@ -110,7 +110,7 @@ class BackendManager(
      * @return true if a backup is possible, false if not.
      */
     @WorkerThread
-    fun canDoBackupNow(): Boolean {
+    override fun canDoBackupNow(): Boolean {
         val storage = backendProperties ?: return false
         return !isOnUnavailableUsb() &&
             !storage.isUnavailableNetwork(context, settingsManager.useMeteredNetwork)
diff --git a/core/src/main/java/org/calyxos/seedvault/core/backends/IBackendManager.kt b/core/src/main/java/org/calyxos/seedvault/core/backends/IBackendManager.kt
@@ -9,6 +9,7 @@ public interface IBackendManager {
     public val backend: Backend
     public val isOnRemovableDrive: Boolean
     public val requiresNetwork: Boolean
+    public fun canDoBackupNow(): Boolean
 }
 
 public enum class BackendId {
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
@@ -100,6 +100,10 @@ internal class Backup(
 
     @Throws(IOException::class, GeneralSecurityException::class)
     suspend fun runBackup(backupObserver: BackupObserver?) {
+        if (!backendManager.canDoBackupNow()) {
+            Log.w(TAG, "runBackup(): Can't do backup right now, aborting...")
+            throw IOException("Metered Network")
+        }
         backupObserver?.onStartScanning()
         var duration: Duration? = null
         try {
@@ -142,15 +146,29 @@ internal class Backup(
         availableChunkIds: Set<String>,
         backupObserver: BackupObserver?,
     ) {
+        if (!backendManager.canDoBackupNow()) {
+            Log.w(TAG, "backupFiles(): Can't do backup right now, aborting...")
+            throw IOException("Metered Network")
+        }
+        val wasAborted = { !backendManager.canDoBackupNow() }
         val startTime = System.currentTimeMillis()
         val numSmallFiles = filesResult.smallFiles.size
         val smallResult = measure("Backing up $numSmallFiles small files") {
-            smallFileBackup.backupFiles(filesResult.smallFiles, availableChunkIds, backupObserver)
+            smallFileBackup.backupFiles(
+                files = filesResult.smallFiles,
+                availableChunkIds = availableChunkIds,
+                wasAborted = wasAborted,
+                backupObserver = backupObserver,
+            )
+        }
+        if (!backendManager.canDoBackupNow()) {
+            Log.w(TAG, "backupFiles(): Can't do backup right now, aborting...")
+            throw IOException("Metered Network")
         }
         MemoryLogger.log()
         val numLargeFiles = filesResult.files.size
         val largeResult = measure("Backing up $numLargeFiles files") {
-            fileBackup.backupFiles(filesResult.files, availableChunkIds, backupObserver)
+            fileBackup.backupFiles(filesResult.files, availableChunkIds, wasAborted, backupObserver)
         }
         MemoryLogger.log()
         val result = largeResult + smallResult
diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/FileBackup.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/FileBackup.kt
@@ -34,13 +34,15 @@ internal class FileBackup(
     suspend fun backupFiles(
         files: List<ContentFile>,
         availableChunkIds: Set<String>,
+        wasAborted: () -> Boolean,
         backupObserver: BackupObserver?,
     ): BackupResult {
         val chunkIds = HashSet<String>()
         val backupMediaFiles = ArrayList<BackupMediaFile>()
         val backupDocumentFiles = ArrayList<BackupDocumentFile>()
         var bytesWritten = 0L
         files.forEach { file ->
+            if (wasAborted()) throw IOException("Metered Network")
             val result = try {
                 backupFile(file, availableChunkIds)
             } catch (e: IOException) {
diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/SmallFileBackup.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/SmallFileBackup.kt
@@ -33,6 +33,7 @@ internal class SmallFileBackup(
     suspend fun backupFiles(
         files: List<ContentFile>,
         availableChunkIds: Set<String>,
+        wasAborted: () -> Boolean,
         backupObserver: BackupObserver?,
     ): BackupResult {
         val chunkIds = HashSet<String>()
@@ -66,6 +67,7 @@ internal class SmallFileBackup(
             } else true
         }
         changedFiles.windowed(2, 1, true).forEach { window ->
+            if (wasAborted()) throw IOException("Metered Network")
             val file = window[0]
             val result = try {
                 makeZipChunk(window, missingChunkIds)
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
@@ -92,6 +92,7 @@ internal class BackupRestoreTest {
         mockkStatic("org.calyxos.backup.storage.UriUtilsKt")
 
         every { backendManager.backend } returns backend
+        every { backendManager.canDoBackupNow() } returns true
         every { db.getFilesCache() } returns filesCache
         every { db.getChunksCache() } returns chunksCache
         every { keyManager.getMainKey() } returns SecretKeySpec(
@@ -526,6 +527,7 @@ internal class BackupRestoreTest {
             keyManager = keyManager,
             cacheRepopulater = cacheRepopulater,
         )
+        every { backendManagerNew.canDoBackupNow() } returns true
         every { backendManagerNew.backend } returnsMany listOf(backend1, backend2)
 
         coEvery { backend1.list(any(), Blob::class, callback = any()) } just Runs
diff --git a/storage/lib/src/test/java/org/calyxos/backup/storage/backup/BackupTest.kt b/storage/lib/src/test/java/org/calyxos/backup/storage/backup/BackupTest.kt
@@ -17,6 +17,7 @@ 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.calyxos.backup.storage.backup.Backup.Companion.CHUNK_SIZE_MAX
 import org.calyxos.backup.storage.db.CachedChunk
@@ -37,13 +38,15 @@ import org.calyxos.seedvault.core.crypto.CoreCrypto.ALGORITHM_HMAC
 import org.calyxos.seedvault.core.crypto.CoreCrypto.KEY_SIZE_BYTES
 import org.calyxos.seedvault.core.crypto.KeyManager
 import org.junit.Assert.assertArrayEquals
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertThrows
 import org.junit.Assert.assertTrue
 import org.junit.Test
 import java.io.ByteArrayInputStream
 import java.io.ByteArrayOutputStream
+import java.io.IOException
 import javax.crypto.spec.SecretKeySpec
 import kotlin.random.Random
-import kotlin.test.assertEquals
 
 internal class BackupTest {
 
@@ -97,12 +100,7 @@ internal class BackupTest {
         )
 
         // preliminaries find the file above
-        coEvery { backend.list(any(), Blob::class, callback = any()) } just Runs
-        every { chunksCache.areAllAvailableChunksCached(emptySet()) } returns true
-        every { fileScanner.getFiles() } returns scannedFiles
-        every { filesCache.getByUri(any()) } returns null // nothing is cached, all is new
-        every { chunksCache.get(any()) } returns null // no chunks are cached, all are new
-        every { chunksCache.hasCorruptedChunks(any()) } returns false // no chunks corrupted
+        prepareBackup(scannedFiles)
 
         // backup file and save its blob
         every {
@@ -143,4 +141,80 @@ internal class BackupTest {
         assertEquals(size2, outputStream2.size().toLong())
     }
 
+    @Test
+    fun testAbortBackupEarly() {
+        every { backendManager.canDoBackupNow() } returns false
+
+        val e = assertThrows(IOException::class.java) {
+            runBlocking {
+                backup.runBackup(null)
+            }
+        }
+        assertEquals("Metered Network", e.message)
+    }
+
+    @Test
+    fun testAbortBackupBeforeSmallFiles() {
+        // define one file in backup
+        val fileMBytes = Random.nextBytes(Random.nextInt(1, CHUNK_SIZE_MAX))
+        val fileM = getRandomDocFile(fileMBytes.size)
+        val scannedFiles = FileScannerResult(
+            smallFiles = listOf(fileM),
+            files = listOf(fileM),
+        )
+
+        prepareBackup(scannedFiles)
+        every { backendManager.canDoBackupNow() } returns true andThen false
+        every {
+            contentResolver.openInputStream(fileM.uri)
+        } returns ByteArrayInputStream(fileMBytes)
+
+        val e = assertThrows(IOException::class.java) {
+            runBlocking {
+                backup.runBackup(null)
+            }
+        }
+        assertEquals("Metered Network", e.message)
+    }
+
+    @Test
+    fun testAbortBackupBeforeLargeFiles() {
+        // define one file in backup
+        val fileMBytes = Random.nextBytes(Random.nextInt(1, CHUNK_SIZE_MAX))
+        val fileM = getRandomDocFile(fileMBytes.size)
+        val scannedFiles = FileScannerResult(
+            smallFiles = listOf(fileM),
+            files = listOf(fileM),
+        )
+
+        prepareBackup(scannedFiles)
+        every { backendManager.canDoBackupNow() } returnsMany listOf(true, true, true, false)
+        every {
+            contentResolver.openInputStream(fileM.uri)
+        } returns ByteArrayInputStream(fileMBytes)
+        coEvery { backend.save(match { it is Blob }, any()) } returns 42L
+        every { chunksCache.insert(any<CachedChunk>()) } just Runs
+        every { filesCache.upsert(any()) } just Runs
+
+        val e = assertThrows(IOException::class.java) {
+            runBlocking {
+                backup.runBackup(null)
+            }
+        }
+        assertEquals("Metered Network", e.message)
+
+        verify {
+            filesCache.upsert(any()) // small file got backed up
+        }
+    }
+
+    private fun prepareBackup(scannedFiles: FileScannerResult) {
+        every { backendManager.canDoBackupNow() } returns true
+        coEvery { backend.list(any(), Blob::class, callback = any()) } just Runs
+        every { chunksCache.areAllAvailableChunksCached(emptySet()) } returns true
+        every { fileScanner.getFiles() } returns scannedFiles
+        every { filesCache.getByUri(any()) } returns null // nothing is cached, all is new
+        every { chunksCache.get(any()) } returns null // no chunks are cached, all are new
+        every { chunksCache.hasCorruptedChunks(any()) } returns false // no chunks corrupted
+    }
 }
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
@@ -121,7 +121,7 @@ internal class SmallFileBackupIntegrationTest {
             observer.onFileBackedUp(file2, true, 0, match<Long> { it <= outputStream2.size() }, "S")
         } just Runs
 
-        val result = smallFileBackup.backupFiles(files, availableChunkIds, observer)
+        val result = smallFileBackup.backupFiles(files, availableChunkIds, { false }, observer)
         assertEquals(setOf(chunkId.toHexString()), result.chunkIds)
         assertEquals(1, result.backupDocumentFiles.size)
         assertEquals(backupFile, result.backupDocumentFiles[0])
diff --git a/storage/lib/src/test/java/org/calyxos/backup/storage/backup/SmallFileBackupTest.kt b/storage/lib/src/test/java/org/calyxos/backup/storage/backup/SmallFileBackupTest.kt
@@ -22,6 +22,7 @@ import org.calyxos.backup.storage.getRandomString
 import org.calyxos.backup.storage.mockLog
 import org.calyxos.backup.storage.sameCachedFile
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertThrows
 import org.junit.Test
 import java.io.IOException
 import java.io.InputStream
@@ -53,7 +54,7 @@ internal class SmallFileBackupTest {
         every { filesCache.getByUri(files[0].uri) } returns cachedFile
         every { chunksCache.hasCorruptedChunks(cachedFile.chunks) } returns false
 
-        val result = smallFileBackup.backupFiles(files, availableChunkIds, null)
+        val result = smallFileBackup.backupFiles(files, availableChunkIds, { false }, null)
         assertEquals(cachedFile.chunks.toSet(), result.chunkIds)
         assertEquals(1, result.backupDocumentFiles.size)
         assertEquals(backupFile, result.backupDocumentFiles[0])
@@ -102,6 +103,22 @@ internal class SmallFileBackupTest {
         singleFileBackup(files, cachedFile, availableChunkIds, corrupted = true)
     }
 
+    @Test
+    fun `no file gets backed up if we aborted`() {
+        val file = getRandomDocFile()
+        val files = listOf(file)
+        val availableChunkIds = hashSetOf(getRandomString(6))
+
+        addFile(file)
+
+        val e = assertThrows(IOException::class.java) {
+            runBlocking {
+                smallFileBackup.backupFiles(files, availableChunkIds, { true }, null)
+            }
+        }
+        assertEquals("Metered Network", e.message)
+    }
+
     private suspend fun singleFileBackup(
         files: List<DocFile>,
         cachedFile: CachedFile?,
@@ -119,7 +136,7 @@ internal class SmallFileBackupTest {
         coEvery { zipChunker.finalizeAndReset(missingChunks) } returns zipChunk
         every { filesCache.upsert(sameCachedFile(newCachedFile)) } just Runs
 
-        val result = smallFileBackup.backupFiles(files, availableChunkIds, null)
+        val result = smallFileBackup.backupFiles(files, availableChunkIds, { false }, null)
         assertEquals(newCachedFile.chunks.toSet(), result.chunkIds)
         assertEquals(1, result.backupDocumentFiles.size)
         assertEquals(backupFile, result.backupDocumentFiles[0])
@@ -143,7 +160,7 @@ internal class SmallFileBackupTest {
         coEvery { zipChunker.finalizeAndReset(emptyList()) } returns zipChunk
         every { filesCache.upsert(sameCachedFile(cachedFile2)) } just Runs
 
-        val result = smallFileBackup.backupFiles(files, availableChunkIds, null)
+        val result = smallFileBackup.backupFiles(files, availableChunkIds, { false }, null)
         assertEquals(cachedFile2.chunks.toSet(), result.chunkIds)
         assertEquals(1, result.backupDocumentFiles.size)
         assertEquals(backupFile, result.backupDocumentFiles[0])
@@ -171,7 +188,7 @@ internal class SmallFileBackupTest {
         // zipChunker.finalizeAndReset defined above for both files
         every { filesCache.upsert(sameCachedFile(cachedFile2)) } just Runs
 
-        val result = smallFileBackup.backupFiles(files, hashSetOf(), null)
+        val result = smallFileBackup.backupFiles(files, hashSetOf(), { false }, null)
         assertEquals(listOf(zipChunk1.id, zipChunk2.id).toSet(), result.chunkIds)
         assertEquals(
             listOf(backupFile1, backupFile2).sortedBy { it.name },
@@ -199,7 +216,7 @@ internal class SmallFileBackupTest {
         every { filesCache.upsert(sameCachedFile(cachedFile1)) } just Runs
         every { filesCache.upsert(sameCachedFile(cachedFile2)) } just Runs
 
-        val result = smallFileBackup.backupFiles(files, hashSetOf(), null)
+        val result = smallFileBackup.backupFiles(files, hashSetOf(), { false }, null)
         assertEquals(listOf(zipChunk.id).toSet(), result.chunkIds)
         assertEquals(
             listOf(backupFile1, backupFile2).sortedBy { it.name },
@@ -219,7 +236,7 @@ internal class SmallFileBackupTest {
         addFile(file2)
         coEvery { zipChunker.finalizeAndReset(emptyList()) } throws IOException()
 
-        val result = smallFileBackup.backupFiles(files, hashSetOf(), null)
+        val result = smallFileBackup.backupFiles(files, hashSetOf(), { false }, null)
         assertEquals(emptySet<String>(), result.chunkIds)
         assertEquals(0, result.backupDocumentFiles.size)
         assertEquals(0, result.backupMediaFiles.size)

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@ public interface IBackendManager {`
`9`	`9`	`public val backend: Backend`
`10`	`10`	`public val isOnRemovableDrive: Boolean`
`11`	`11`	`public val requiresNetwork: Boolean`
	`12`	`+ public fun canDoBackupNow(): Boolean`
`12`	`13`	`}`
`13`	`14`
`14`	`15`	`public enum class BackendId {`