Move code to get available backups from RestorePlugin to BackupPlugin

app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt

@@ -10,6 +10,7 @@ import com.stevesoltys.seedvault.plugins.saf.DocumentsProviderKVBackup
 import com.stevesoltys.seedvault.plugins.saf.DocumentsProviderKVRestorePlugin
 import com.stevesoltys.seedvault.plugins.saf.DocumentsProviderRestorePlugin
 import com.stevesoltys.seedvault.plugins.saf.DocumentsStorage
+import com.stevesoltys.seedvault.plugins.saf.FILE_BACKUP_METADATA
 import com.stevesoltys.seedvault.plugins.saf.MAX_KEY_LENGTH
 import com.stevesoltys.seedvault.plugins.saf.MAX_KEY_LENGTH_NEXTCLOUD
 import com.stevesoltys.seedvault.plugins.saf.deleteContents
@@ -94,9 +95,9 @@ class PluginTest : KoinComponent {
     @Test
     fun testInitializationAndRestoreSets() = runBlocking(Dispatchers.IO) {
         // no backups available initially
-        assertEquals(0, restorePlugin.getAvailableBackups()?.toList()?.size)
+        assertEquals(0, backupPlugin.getAvailableBackups()?.toList()?.size)
         val uri = settingsManager.getStorage()?.getDocumentFile(context)?.uri ?: error("no storage")
-        assertFalse(restorePlugin.hasBackup(uri))
+        assertFalse(backupPlugin.hasBackup(uri))
 
         // prepare returned tokens requested when initializing device
         every { mockedSettingsManager.getToken() } returnsMany listOf(token, token + 1, token + 1)
@@ -106,23 +107,26 @@ class PluginTest : KoinComponent {
         backupPlugin.initializeDevice()
 
         // write metadata (needed for backup to be recognized)
-        backupPlugin.getMetadataOutputStream().writeAndClose(getRandomByteArray())
+        backupPlugin.getOutputStream(token, FILE_BACKUP_METADATA)
+            .writeAndClose(getRandomByteArray())
 
         // one backup available now
-        assertEquals(1, restorePlugin.getAvailableBackups()?.toList()?.size)
+        assertEquals(1, backupPlugin.getAvailableBackups()?.toList()?.size)
-        assertTrue(restorePlugin.hasBackup(uri))
+        assertTrue(backupPlugin.hasBackup(uri))
 
         // initializing again (with another restore set) does add a restore set
         backupPlugin.startNewRestoreSet(token + 1)
         backupPlugin.initializeDevice()
-        backupPlugin.getMetadataOutputStream().writeAndClose(getRandomByteArray())
+        backupPlugin.getOutputStream(token, FILE_BACKUP_METADATA)
-        assertEquals(2, restorePlugin.getAvailableBackups()?.toList()?.size)
+            .writeAndClose(getRandomByteArray())
-        assertTrue(restorePlugin.hasBackup(uri))
+        assertEquals(2, backupPlugin.getAvailableBackups()?.toList()?.size)
+        assertTrue(backupPlugin.hasBackup(uri))
 
         // initializing again (without new restore set) doesn't change number of restore sets
         backupPlugin.initializeDevice()
-        backupPlugin.getMetadataOutputStream().writeAndClose(getRandomByteArray())
+        backupPlugin.getOutputStream(token, FILE_BACKUP_METADATA)
-        assertEquals(2, restorePlugin.getAvailableBackups()?.toList()?.size)
+            .writeAndClose(getRandomByteArray())
+        assertEquals(2, backupPlugin.getAvailableBackups()?.toList()?.size)
 
         // ensure that the new backup dirs exist
         assertTrue(storage.currentKvBackupDir!!.exists())
@@ -138,29 +142,27 @@ class PluginTest : KoinComponent {
 
         // write metadata
         val metadata = getRandomByteArray()
-        backupPlugin.getMetadataOutputStream().writeAndClose(metadata)
+        backupPlugin.getOutputStream(token, FILE_BACKUP_METADATA).writeAndClose(metadata)
 
         // get available backups, expect only one with our token and no error
-        var availableBackups = restorePlugin.getAvailableBackups()?.toList()
+        var availableBackups = backupPlugin.getAvailableBackups()?.toList()
         check(availableBackups != null)
         assertEquals(1, availableBackups.size)
         assertEquals(token, availableBackups[0].token)
-        assertFalse(availableBackups[0].error)
 
         // read metadata matches what was written earlier
-        assertReadEquals(metadata, availableBackups[0].inputStream)
+        assertReadEquals(metadata, availableBackups[0].inputStreamRetriever())
 
         // initializing again (without changing storage) keeps restore set with same token
         backupPlugin.initializeDevice()
-        backupPlugin.getMetadataOutputStream().writeAndClose(metadata)
+        backupPlugin.getOutputStream(token, FILE_BACKUP_METADATA).writeAndClose(metadata)
-        availableBackups = restorePlugin.getAvailableBackups()?.toList()
+        availableBackups = backupPlugin.getAvailableBackups()?.toList()
         check(availableBackups != null)
         assertEquals(1, availableBackups.size)
         assertEquals(token, availableBackups[0].token)
-        assertFalse(availableBackups[0].error)
 
         // metadata hasn't changed
-        assertReadEquals(metadata, availableBackups[0].inputStream)
+        assertReadEquals(metadata, availableBackups[0].inputStreamRetriever())
     }
 
     @Test

app/src/main/java/com/stevesoltys/seedvault/metadata/Metadata.kt

@@ -6,7 +6,6 @@ import com.stevesoltys.seedvault.crypto.TYPE_METADATA
 import com.stevesoltys.seedvault.header.VERSION
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
 import org.calyxos.backup.storage.crypto.StreamCrypto.toByteArray
-import java.io.InputStream
 import java.nio.ByteBuffer
 
 typealias PackageMetadataMap = HashMap<String, PackageMetadata>
@@ -100,20 +99,6 @@ internal const val JSON_PACKAGE_SIGNATURES = "signatures"
 
 internal class DecryptionFailedException(cause: Throwable) : Exception(cause)
 
-class EncryptedBackupMetadata private constructor(
-    val token: Long,
-    val inputStream: InputStream?,
-    val error: Boolean
-) {
-
-    constructor(token: Long, inputStream: InputStream) : this(token, inputStream, false)
-
-    /**
-     * Indicates that there was an error retrieving the encrypted backup metadata.
-     */
-    constructor(token: Long) : this(token, null, true)
-}
-
 internal fun getAD(version: Byte, token: Long) = ByteBuffer.allocate(2 + 8)
     .put(version)
     .put(TYPE_METADATA)

app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderRestorePlugin.kt

@@ -1,11 +1,7 @@
 package com.stevesoltys.seedvault.plugins.saf
 
 import android.content.Context
-import android.net.Uri
-import android.util.Log
 import androidx.annotation.WorkerThread
-import androidx.documentfile.provider.DocumentFile
-import com.stevesoltys.seedvault.metadata.EncryptedBackupMetadata
 import com.stevesoltys.seedvault.transport.restore.FullRestorePlugin
 import com.stevesoltys.seedvault.transport.restore.KVRestorePlugin
 import com.stevesoltys.seedvault.transport.restore.RestorePlugin
@@ -13,8 +9,6 @@ import java.io.FileNotFoundException
 import java.io.IOException
 import java.io.InputStream
 
-private val TAG = DocumentsProviderRestorePlugin::class.java.simpleName
-
 @WorkerThread
 @Suppress("BlockingMethodInNonBlockingContext") // all methods do I/O
 internal class DocumentsProviderRestorePlugin(
@@ -24,31 +18,6 @@ internal class DocumentsProviderRestorePlugin(
     override val fullRestorePlugin: FullRestorePlugin
 ) : RestorePlugin {
 
-    @Throws(IOException::class)
-    override suspend fun hasBackup(uri: Uri): Boolean {
-        val parent = DocumentFile.fromTreeUri(context, uri) ?: throw AssertionError()
-        val rootDir = parent.findFileBlocking(context, DIRECTORY_ROOT) ?: return false
-        val backupSets = getBackups(context, rootDir)
-        return backupSets.isNotEmpty()
-    }
-
-    override suspend fun getAvailableBackups(): Sequence<EncryptedBackupMetadata>? {
-        val rootDir = storage.rootBackupDir ?: return null
-        val backupSets = getBackups(context, rootDir)
-        val iterator = backupSets.iterator()
-        return generateSequence {
-            if (!iterator.hasNext()) return@generateSequence null // end sequence
-            val backupSet = iterator.next()
-            try {
-                val stream = storage.getInputStream(backupSet.metadataFile)
-                EncryptedBackupMetadata(backupSet.token, stream)
-            } catch (e: IOException) {
-                Log.e(TAG, "Error getting InputStream for backup metadata.", e)
-                EncryptedBackupMetadata(backupSet.token)
-            }
-        }
-    }
-
     @Throws(IOException::class)
     override suspend fun getApkInputStream(
         token: Long,

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

@@ -58,7 +58,7 @@ interface BackupPlugin {
      * Returns an [OutputStream] for writing backup metadata.
      */
     @Throws(IOException::class)
-    @Deprecated("use getOutputStream() instead")
+    @Deprecated("use getOutputStream(token, FILE_BACKUP_METADATA) instead")
     suspend fun getMetadataOutputStream(token: Long): OutputStream
 
     /**

app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt

@@ -19,8 +19,8 @@ import com.stevesoltys.seedvault.metadata.DecryptionFailedException
 import com.stevesoltys.seedvault.metadata.MetadataManager
 import com.stevesoltys.seedvault.metadata.MetadataReader
 import com.stevesoltys.seedvault.settings.SettingsManager
+import com.stevesoltys.seedvault.transport.backup.BackupPlugin
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
-import libcore.io.IoUtils.closeQuietly
 import java.io.IOException
 
 private data class RestoreCoordinatorState(
@@ -43,7 +43,7 @@ internal class RestoreCoordinator(
     private val settingsManager: SettingsManager,
     private val metadataManager: MetadataManager,
     private val notificationManager: BackupNotificationManager,
-    private val plugin: RestorePlugin,
+    private val plugin: BackupPlugin,
     private val kv: KVRestore,
     private val full: FullRestore,
     private val metadataReader: MetadataReader
@@ -57,15 +57,10 @@ internal class RestoreCoordinator(
         val availableBackups = plugin.getAvailableBackups() ?: return null
         val metadataMap = HashMap<Long, BackupMetadata>()
         for (encryptedMetadata in availableBackups) {
-            if (encryptedMetadata.error) continue
-            check(encryptedMetadata.inputStream != null) {
-                "No error when getting encrypted metadata, but stream is still missing."
-            }
             try {
-                val metadata = metadataReader.readMetadata(
+                val metadata = encryptedMetadata.inputStreamRetriever().use { inputStream ->
-                    encryptedMetadata.inputStream,
+                    metadataReader.readMetadata(inputStream, encryptedMetadata.token)
-                    encryptedMetadata.token
+                }
-                )
                 metadataMap[encryptedMetadata.token] = metadata
             } catch (e: IOException) {
                 Log.e(TAG, "Error while getting restore set ${encryptedMetadata.token}", e)
@@ -79,8 +74,6 @@ internal class RestoreCoordinator(
             } catch (e: UnsupportedVersionException) {
                 Log.w(TAG, "Backup with unsupported version read", e)
                 continue
-            } finally {
-                closeQuietly(encryptedMetadata.inputStream)
             }
         }
         Log.i(TAG, "Got available metadata for tokens: ${metadataMap.keys}")

app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorePlugin.kt

@@ -1,8 +1,5 @@
 package com.stevesoltys.seedvault.transport.restore
 
-import android.net.Uri
-import androidx.annotation.WorkerThread
-import com.stevesoltys.seedvault.metadata.EncryptedBackupMetadata
 import java.io.IOException
 import java.io.InputStream
 
@@ -12,24 +9,6 @@ interface RestorePlugin {
 
     val fullRestorePlugin: FullRestorePlugin
 
-    /**
-     * Get the set of all backups currently available for restore.
-     *
-     * @return metadata for the set of restore images available,
-     * or null if an error occurred (the attempt should be rescheduled).
-     **/
-    suspend fun getAvailableBackups(): Sequence<EncryptedBackupMetadata>?
-
-    /**
-     * Searches if there's really a backup available in the given location.
-     * Returns true if at least one was found and false otherwise.
-     *
-     * FIXME: Passing a Uri is maybe too plugin-specific?
-     */
-    @WorkerThread
-    @Throws(IOException::class)
-    suspend fun hasBackup(uri: Uri): Boolean
-
     /**
      * Returns an [InputStream] for the given token, for reading an APK that is to be restored.
      */

app/src/main/java/com/stevesoltys/seedvault/ui/storage/RestoreStorageViewModel.kt

@@ -7,7 +7,7 @@ import androidx.lifecycle.viewModelScope
 import com.stevesoltys.seedvault.R
 import com.stevesoltys.seedvault.plugins.saf.DIRECTORY_ROOT
 import com.stevesoltys.seedvault.settings.SettingsManager
-import com.stevesoltys.seedvault.transport.restore.RestorePlugin
+import com.stevesoltys.seedvault.transport.backup.BackupPlugin
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
 import java.io.IOException
@@ -16,7 +16,7 @@ private val TAG = RestoreStorageViewModel::class.java.simpleName
 
 internal class RestoreStorageViewModel(
     private val app: Application,
-    private val restorePlugin: RestorePlugin,
+    private val backupPlugin: BackupPlugin,
     settingsManager: SettingsManager
 ) : StorageViewModel(app, settingsManager) {
 
@@ -25,7 +25,7 @@ internal class RestoreStorageViewModel(
     override fun onLocationSet(uri: Uri) {
         viewModelScope.launch(Dispatchers.IO) {
             val hasBackup = try {
-                restorePlugin.hasBackup(uri)
+                backupPlugin.hasBackup(uri)
             } catch (e: IOException) {
                 Log.e(TAG, "Error reading URI: $uri", e)
                 false

app/src/test/java/com/stevesoltys/seedvault/transport/CoordinatorIntegrationTest.kt

@@ -104,7 +104,7 @@ internal class CoordinatorIntegrationTest : TransportTest() {
         settingsManager,
         metadataManager,
         notificationManager,
-        restorePlugin,
+        backupPlugin,
         kvRestore,
         fullRestore,
         metadataReader

app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt

@@ -11,11 +11,12 @@ import android.os.ParcelFileDescriptor
 import com.stevesoltys.seedvault.coAssertThrows
 import com.stevesoltys.seedvault.getRandomString
 import com.stevesoltys.seedvault.header.VERSION
-import com.stevesoltys.seedvault.metadata.EncryptedBackupMetadata
 import com.stevesoltys.seedvault.metadata.MetadataReader
 import com.stevesoltys.seedvault.metadata.PackageMetadata
 import com.stevesoltys.seedvault.settings.Storage
 import com.stevesoltys.seedvault.transport.TransportTest
+import com.stevesoltys.seedvault.transport.backup.BackupPlugin
+import com.stevesoltys.seedvault.transport.backup.EncryptedMetadata
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
 import io.mockk.Runs
 import io.mockk.coEvery
@@ -37,7 +38,7 @@ import kotlin.random.Random
 internal class RestoreCoordinatorTest : TransportTest() {
 
     private val notificationManager: BackupNotificationManager = mockk()
-    private val plugin = mockk<RestorePlugin>()
+    private val plugin = mockk<BackupPlugin>()
     private val kv = mockk<KVRestore>()
     private val full = mockk<FullRestore>()
     private val metadataReader = mockk<MetadataReader>()
@@ -67,11 +68,11 @@ internal class RestoreCoordinatorTest : TransportTest() {
 
     @Test
     fun `getAvailableRestoreSets() builds set from plugin response`() = runBlocking {
-        val encryptedMetadata = EncryptedBackupMetadata(token, inputStream)
+        val encryptedMetadata = EncryptedMetadata(token) { inputStream }
 
         coEvery { plugin.getAvailableBackups() } returns sequenceOf(
             encryptedMetadata,
-            EncryptedBackupMetadata(token + 1, inputStream)
+            EncryptedMetadata(token + 1) { inputStream }
         )
         every { metadataReader.readMetadata(inputStream, token) } returns metadata
         every { metadataReader.readMetadata(inputStream, token + 1) } returns metadata
@@ -99,8 +100,8 @@ internal class RestoreCoordinatorTest : TransportTest() {
     @Test
     fun `startRestore() fetches metadata if missing`() = runBlocking {
         coEvery { plugin.getAvailableBackups() } returns sequenceOf(
-            EncryptedBackupMetadata(token, inputStream),
+            EncryptedMetadata(token) { inputStream },
-            EncryptedBackupMetadata(token + 1, inputStream)
+            EncryptedMetadata(token + 1) { inputStream }
         )
         every { metadataReader.readMetadata(inputStream, token) } returns metadata
         every { metadataReader.readMetadata(inputStream, token + 1) } returns metadata
@@ -112,7 +113,7 @@ internal class RestoreCoordinatorTest : TransportTest() {
     @Test
     fun `startRestore() errors if metadata is not matching token`() = runBlocking {
         coEvery { plugin.getAvailableBackups() } returns sequenceOf(
-            EncryptedBackupMetadata(token + 42, inputStream)
+            EncryptedMetadata(token + 42) { inputStream }
         )
         every { metadataReader.readMetadata(inputStream, token + 42) } returns metadata
         every { inputStream.close() } just Runs

app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreV0IntegrationTest.kt

@@ -15,6 +15,7 @@ import com.stevesoltys.seedvault.header.HeaderReaderImpl
 import com.stevesoltys.seedvault.metadata.MetadataReaderImpl
 import com.stevesoltys.seedvault.toByteArrayFromHex
 import com.stevesoltys.seedvault.transport.TransportTest
+import com.stevesoltys.seedvault.transport.backup.BackupPlugin
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
 import io.mockk.coEvery
 import io.mockk.every
@@ -46,7 +47,7 @@ internal class RestoreV0IntegrationTest : TransportTest() {
     private val metadataReader = MetadataReaderImpl(cryptoImpl)
     private val notificationManager = mockk<BackupNotificationManager>()
 
-    private val restorePlugin = mockk<RestorePlugin>()
+    private val backupPlugin = mockk<BackupPlugin>()
     private val kvRestorePlugin = mockk<KVRestorePlugin>()
     private val kvRestore = KVRestore(kvRestorePlugin, outputFactory, headerReader, cryptoImpl)
     private val fullRestorePlugin = mockk<FullRestorePlugin>()
@@ -57,7 +58,7 @@ internal class RestoreV0IntegrationTest : TransportTest() {
         settingsManager,
         metadataManager,
         notificationManager,
-        restorePlugin,
+        backupPlugin,
         kvRestore,
         fullRestore,
         metadataReader