From 81d5281a943191a8bb4c7420adb249b90073179e Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 31 Mar 2022 11:20:44 -0300 Subject: [PATCH] Save restore storage only when it had backups Incidentally this gets rid of the need to pass an implementation-specific Uri to the StoragePlugin. --- .../java/com/stevesoltys/seedvault/PluginTest.kt | 8 ++++---- .../stevesoltys/seedvault/plugins/StoragePlugin.kt | 8 +++----- .../plugins/saf/DocumentsProviderStoragePlugin.kt | 12 +++++++----- .../seedvault/ui/storage/RestoreStorageViewModel.kt | 5 +++-- .../seedvault/ui/storage/StorageViewModel.kt | 12 ++++++++++-- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt b/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt index ba7d444c..3188686b 100644 --- a/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt +++ b/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt @@ -74,8 +74,8 @@ class PluginTest : KoinComponent { fun testInitializationAndRestoreSets() = runBlocking(Dispatchers.IO) { // no backups available initially assertEquals(0, storagePlugin.getAvailableBackups()?.toList()?.size) - val uri = settingsManager.getStorage()?.getDocumentFile(context)?.uri ?: error("no storage") - assertFalse(storagePlugin.hasBackup(uri)) + val s = settingsManager.getStorage() ?: error("no storage") + assertFalse(storagePlugin.hasBackup(s)) // prepare returned tokens requested when initializing device every { mockedSettingsManager.getToken() } returnsMany listOf(token, token + 1, token + 1) @@ -90,7 +90,7 @@ class PluginTest : KoinComponent { // one backup available now assertEquals(1, storagePlugin.getAvailableBackups()?.toList()?.size) - assertTrue(storagePlugin.hasBackup(uri)) + assertTrue(storagePlugin.hasBackup(s)) // initializing again (with another restore set) does add a restore set storagePlugin.startNewRestoreSet(token + 1) @@ -98,7 +98,7 @@ class PluginTest : KoinComponent { storagePlugin.getOutputStream(token + 1, FILE_BACKUP_METADATA) .writeAndClose(getRandomByteArray()) assertEquals(2, storagePlugin.getAvailableBackups()?.toList()?.size) - assertTrue(storagePlugin.hasBackup(uri)) + assertTrue(storagePlugin.hasBackup(s)) // initializing again (without new restore set) doesn't change number of restore sets storagePlugin.initializeDevice() diff --git a/app/src/main/java/com/stevesoltys/seedvault/plugins/StoragePlugin.kt b/app/src/main/java/com/stevesoltys/seedvault/plugins/StoragePlugin.kt index 5ba57345..53becfac 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/plugins/StoragePlugin.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/plugins/StoragePlugin.kt @@ -1,8 +1,8 @@ package com.stevesoltys.seedvault.plugins import android.app.backup.RestoreSet -import android.net.Uri import androidx.annotation.WorkerThread +import com.stevesoltys.seedvault.settings.Storage import java.io.IOException import java.io.InputStream import java.io.OutputStream @@ -48,14 +48,12 @@ interface StoragePlugin { suspend fun removeData(token: Long, name: String) /** - * Searches if there's really a backup available in the given location. + * Searches if there's really a backup available in the given storage location. * Returns true if at least one was found and false otherwise. - * - * FIXME: Passing a Uri is too plugin-specific and should be handled differently */ @WorkerThread @Throws(IOException::class) - suspend fun hasBackup(uri: Uri): Boolean + suspend fun hasBackup(storage: Storage): Boolean /** * Get the set of all backups currently available for restore. diff --git a/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderStoragePlugin.kt b/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderStoragePlugin.kt index 04a3cb00..55dbd98e 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderStoragePlugin.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderStoragePlugin.kt @@ -2,12 +2,12 @@ package com.stevesoltys.seedvault.plugins.saf import android.content.Context import android.content.pm.PackageManager -import android.net.Uri import android.util.Log import androidx.documentfile.provider.DocumentFile import com.stevesoltys.seedvault.getSystemContext import com.stevesoltys.seedvault.plugins.EncryptedMetadata import com.stevesoltys.seedvault.plugins.StoragePlugin +import com.stevesoltys.seedvault.settings.Storage import java.io.FileNotFoundException import java.io.IOException import java.io.InputStream @@ -77,10 +77,12 @@ internal class DocumentsProviderStoragePlugin( } @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) + override suspend fun hasBackup(storage: Storage): Boolean { + // potentially get system user context if needed here + val c = appContext.getSystemContext { storage.isUsb } + val parent = DocumentFile.fromTreeUri(c, storage.uri) ?: throw AssertionError() + val rootDir = parent.findFileBlocking(c, DIRECTORY_ROOT) ?: return false + val backupSets = getBackups(c, rootDir) return backupSets.isNotEmpty() } diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/storage/RestoreStorageViewModel.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/storage/RestoreStorageViewModel.kt index b42473f1..916ca3e0 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/storage/RestoreStorageViewModel.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/storage/RestoreStorageViewModel.kt @@ -24,14 +24,15 @@ internal class RestoreStorageViewModel( override fun onLocationSet(uri: Uri) { viewModelScope.launch(Dispatchers.IO) { - saveStorage(uri) + val storage = createStorage(uri) val hasBackup = try { - storagePlugin.hasBackup(uri) + storagePlugin.hasBackup(storage) } catch (e: IOException) { Log.e(TAG, "Error reading URI: $uri", e) false } if (hasBackup) { + saveStorage(storage) mLocationChecked.postEvent(LocationResult()) } else { Log.w(TAG, "Location was rejected: $uri") diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageViewModel.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageViewModel.kt index b94757b8..0d0e5f11 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageViewModel.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageViewModel.kt @@ -98,13 +98,21 @@ internal abstract class StorageViewModel( */ protected fun saveStorage(uri: Uri): Boolean { // store backup storage location in settings + val storage = createStorage(uri) + return saveStorage(storage) + } + + protected fun createStorage(uri: Uri): Storage { val root = safOption ?: throw IllegalStateException("no storage root") val name = if (root.isInternal()) { "${root.title} (${app.getString(R.string.settings_backup_location_internal)})" } else { root.title } - val storage = Storage(uri, name, root.isUsb, root.requiresNetwork) + return Storage(uri, name, root.isUsb, root.requiresNetwork) + } + + protected fun saveStorage(storage: Storage): Boolean { settingsManager.setStorage(storage) if (storage.isUsb) { @@ -117,7 +125,7 @@ internal abstract class StorageViewModel( } BackupManagerSettings.resetDefaults(app.contentResolver) - Log.d(TAG, "New storage location saved: $uri") + Log.d(TAG, "New storage location saved: ${storage.uri}") return storage.isUsb }