Save restore storage only when it had backups

Incidentally this gets rid of the need to pass an implementation-specific Uri to the StoragePlugin.
This commit is contained in:
Torsten Grote 2022-03-31 11:20:44 -03:00 committed by Chirayu Desai
parent d7910a84b4
commit 81d5281a94
5 changed files with 27 additions and 18 deletions

View file

@ -74,8 +74,8 @@ class PluginTest : KoinComponent {
fun testInitializationAndRestoreSets() = runBlocking(Dispatchers.IO) { fun testInitializationAndRestoreSets() = runBlocking(Dispatchers.IO) {
// no backups available initially // no backups available initially
assertEquals(0, storagePlugin.getAvailableBackups()?.toList()?.size) assertEquals(0, storagePlugin.getAvailableBackups()?.toList()?.size)
val uri = settingsManager.getStorage()?.getDocumentFile(context)?.uri ?: error("no storage") val s = settingsManager.getStorage() ?: error("no storage")
assertFalse(storagePlugin.hasBackup(uri)) assertFalse(storagePlugin.hasBackup(s))
// prepare returned tokens requested when initializing device // prepare returned tokens requested when initializing device
every { mockedSettingsManager.getToken() } returnsMany listOf(token, token + 1, token + 1) every { mockedSettingsManager.getToken() } returnsMany listOf(token, token + 1, token + 1)
@ -90,7 +90,7 @@ class PluginTest : KoinComponent {
// one backup available now // one backup available now
assertEquals(1, storagePlugin.getAvailableBackups()?.toList()?.size) 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 // initializing again (with another restore set) does add a restore set
storagePlugin.startNewRestoreSet(token + 1) storagePlugin.startNewRestoreSet(token + 1)
@ -98,7 +98,7 @@ class PluginTest : KoinComponent {
storagePlugin.getOutputStream(token + 1, FILE_BACKUP_METADATA) storagePlugin.getOutputStream(token + 1, FILE_BACKUP_METADATA)
.writeAndClose(getRandomByteArray()) .writeAndClose(getRandomByteArray())
assertEquals(2, storagePlugin.getAvailableBackups()?.toList()?.size) 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 // initializing again (without new restore set) doesn't change number of restore sets
storagePlugin.initializeDevice() storagePlugin.initializeDevice()

View file

@ -1,8 +1,8 @@
package com.stevesoltys.seedvault.plugins package com.stevesoltys.seedvault.plugins
import android.app.backup.RestoreSet import android.app.backup.RestoreSet
import android.net.Uri
import androidx.annotation.WorkerThread import androidx.annotation.WorkerThread
import com.stevesoltys.seedvault.settings.Storage
import java.io.IOException import java.io.IOException
import java.io.InputStream import java.io.InputStream
import java.io.OutputStream import java.io.OutputStream
@ -48,14 +48,12 @@ interface StoragePlugin {
suspend fun removeData(token: Long, name: String) 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. * 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 @WorkerThread
@Throws(IOException::class) @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. * Get the set of all backups currently available for restore.

View file

@ -2,12 +2,12 @@ package com.stevesoltys.seedvault.plugins.saf
import android.content.Context import android.content.Context
import android.content.pm.PackageManager import android.content.pm.PackageManager
import android.net.Uri
import android.util.Log import android.util.Log
import androidx.documentfile.provider.DocumentFile import androidx.documentfile.provider.DocumentFile
import com.stevesoltys.seedvault.getSystemContext import com.stevesoltys.seedvault.getSystemContext
import com.stevesoltys.seedvault.plugins.EncryptedMetadata import com.stevesoltys.seedvault.plugins.EncryptedMetadata
import com.stevesoltys.seedvault.plugins.StoragePlugin import com.stevesoltys.seedvault.plugins.StoragePlugin
import com.stevesoltys.seedvault.settings.Storage
import java.io.FileNotFoundException import java.io.FileNotFoundException
import java.io.IOException import java.io.IOException
import java.io.InputStream import java.io.InputStream
@ -77,10 +77,12 @@ internal class DocumentsProviderStoragePlugin(
} }
@Throws(IOException::class) @Throws(IOException::class)
override suspend fun hasBackup(uri: Uri): Boolean { override suspend fun hasBackup(storage: Storage): Boolean {
val parent = DocumentFile.fromTreeUri(context, uri) ?: throw AssertionError() // potentially get system user context if needed here
val rootDir = parent.findFileBlocking(context, DIRECTORY_ROOT) ?: return false val c = appContext.getSystemContext { storage.isUsb }
val backupSets = getBackups(context, rootDir) 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() return backupSets.isNotEmpty()
} }

View file

@ -24,14 +24,15 @@ internal class RestoreStorageViewModel(
override fun onLocationSet(uri: Uri) { override fun onLocationSet(uri: Uri) {
viewModelScope.launch(Dispatchers.IO) { viewModelScope.launch(Dispatchers.IO) {
saveStorage(uri) val storage = createStorage(uri)
val hasBackup = try { val hasBackup = try {
storagePlugin.hasBackup(uri) storagePlugin.hasBackup(storage)
} catch (e: IOException) { } catch (e: IOException) {
Log.e(TAG, "Error reading URI: $uri", e) Log.e(TAG, "Error reading URI: $uri", e)
false false
} }
if (hasBackup) { if (hasBackup) {
saveStorage(storage)
mLocationChecked.postEvent(LocationResult()) mLocationChecked.postEvent(LocationResult())
} else { } else {
Log.w(TAG, "Location was rejected: $uri") Log.w(TAG, "Location was rejected: $uri")

View file

@ -98,13 +98,21 @@ internal abstract class StorageViewModel(
*/ */
protected fun saveStorage(uri: Uri): Boolean { protected fun saveStorage(uri: Uri): Boolean {
// store backup storage location in settings // 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 root = safOption ?: throw IllegalStateException("no storage root")
val name = if (root.isInternal()) { val name = if (root.isInternal()) {
"${root.title} (${app.getString(R.string.settings_backup_location_internal)})" "${root.title} (${app.getString(R.string.settings_backup_location_internal)})"
} else { } else {
root.title 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) settingsManager.setStorage(storage)
if (storage.isUsb) { if (storage.isUsb) {
@ -117,7 +125,7 @@ internal abstract class StorageViewModel(
} }
BackupManagerSettings.resetDefaults(app.contentResolver) BackupManagerSettings.resetDefaults(app.contentResolver)
Log.d(TAG, "New storage location saved: $uri") Log.d(TAG, "New storage location saved: ${storage.uri}")
return storage.isUsb return storage.isUsb
} }