From 440491425a8af04d50cb4924bfe96a0145a19f71 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 12 Dec 2019 16:27:57 -0300 Subject: [PATCH] Work around DocumentFile bug happening with cloud-based DocumentsProviders These might return outdated or now content when queried, then check their cloud storage and report back with up-to-date content. We now detect this (when looking for backups on newly setup storage) and wait until the content has been loaded before acting on the response. This is affecting and was tested with NextCloud. --- .../seedvault/transport/PluginManager.kt | 2 +- .../backup/plugins/DocumentsStorage.kt | 74 +++++++++++++++++++ .../transport/restore/RestoreCoordinator.kt | 4 +- .../transport/restore/RestorePlugin.kt | 3 +- .../plugins/DocumentsProviderRestorePlugin.kt | 27 ++++--- .../ui/storage/RestoreStorageViewModel.kt | 8 +- .../transport/CoordinatorIntegrationTest.kt | 2 +- .../restore/RestoreCoordinatorTest.kt | 4 +- 8 files changed, 104 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/PluginManager.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/PluginManager.kt index 7be8a57e..9f8b3563 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/PluginManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/PluginManager.kt @@ -49,6 +49,6 @@ class PluginManager(context: Context) { private val kvRestore = KVRestore(restorePlugin.kvRestorePlugin, outputFactory, headerReader, crypto) private val fullRestore = FullRestore(restorePlugin.fullRestorePlugin, outputFactory, headerReader, crypto) - internal val restoreCoordinator = RestoreCoordinator(settingsManager, restorePlugin, kvRestore, fullRestore, metadataReader) + internal val restoreCoordinator = RestoreCoordinator(context, settingsManager, restorePlugin, kvRestore, fullRestore, metadataReader) } diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/plugins/DocumentsStorage.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/plugins/DocumentsStorage.kt index 41d9d548..d401c179 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/plugins/DocumentsStorage.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/plugins/DocumentsStorage.kt @@ -1,14 +1,21 @@ package com.stevesoltys.seedvault.transport.backup.plugins +import android.annotation.SuppressLint import android.content.Context import android.content.pm.PackageInfo +import android.database.ContentObserver +import android.net.Uri +import android.provider.DocumentsContract.* +import android.provider.DocumentsContract.Document.* import android.util.Log import androidx.documentfile.provider.DocumentFile import com.stevesoltys.seedvault.settings.SettingsManager import com.stevesoltys.seedvault.settings.Storage +import libcore.io.IoUtils.closeQuietly import java.io.IOException import java.io.InputStream import java.io.OutputStream +import java.util.concurrent.TimeUnit.MINUTES const val DIRECTORY_ROOT = ".AndroidBackup" const val DIRECTORY_FULL_BACKUP = "full" @@ -126,3 +133,70 @@ fun DocumentFile.deleteContents() { fun DocumentFile.assertRightFile(packageInfo: PackageInfo) { if (name != packageInfo.packageName) throw AssertionError() } + +/** + * Works like [DocumentFile.listFiles] except that it waits until the DocumentProvider has a result. + * This prevents getting an empty list even though there are children to be listed. + */ +@Throws(IOException::class) +fun DocumentFile.listFilesBlocking(context: Context): ArrayList { + val resolver = context.contentResolver + val childrenUri = buildChildDocumentsUriUsingTree(uri, getDocumentId(uri)) + val projection = arrayOf(COLUMN_DOCUMENT_ID, COLUMN_MIME_TYPE) + val result = ArrayList() + + @SuppressLint("Recycle") // gets closed in with(), only earlier exit when null + var cursor = resolver.query(childrenUri, projection, null, null, null) + ?: throw IOException() + val loading = cursor.extras.getBoolean(EXTRA_LOADING, false) + if (loading) { + Log.d(TAG, "Wait for children to get loaded...") + var loaded = false + cursor.registerContentObserver(object : ContentObserver(null) { + override fun onChange(selfChange: Boolean, uri: Uri?) { + Log.d(TAG, "Children loaded. Continue...") + loaded = true + } + }) + val timeout = MINUTES.toMillis(2) + var time = 0 + while (!loaded && time < timeout) { + Thread.sleep(50) + time += 50 + } + if (time >= timeout) Log.w(TAG, "Timed out while waiting for children to load") + closeQuietly(cursor) + // do a new query after content was loaded + @SuppressLint("Recycle") // gets closed after with block + cursor = resolver.query(childrenUri, projection, null, null, null) + ?: throw IOException() + } + with(cursor) { + while (moveToNext()) { + val documentId = getString(0) + val isDirectory = getString(1) == MIME_TYPE_DIR + val file = if (isDirectory) { + val treeUri = buildTreeDocumentUri(uri.authority, documentId) + DocumentFile.fromTreeUri(context, treeUri)!! + } else { + val documentUri = buildDocumentUriUsingTree(uri, documentId) + DocumentFile.fromSingleUri(context, documentUri)!! + } + result.add(file) + } + } + return result +} + +fun DocumentFile.findFileBlocking(context: Context, displayName: String): DocumentFile? { + val files = try { + listFilesBlocking(context) + } catch (e: IOException) { + Log.e(TAG, "Error finding file blocking", e) + return null + } + for (doc in files) { + if (displayName == doc.name) return doc + } + return null +} diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt index 3812e588..e9fa2f14 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt @@ -5,6 +5,7 @@ import android.app.backup.BackupTransport.TRANSPORT_OK import android.app.backup.RestoreDescription import android.app.backup.RestoreDescription.* import android.app.backup.RestoreSet +import android.content.Context import android.content.pm.PackageInfo import android.os.ParcelFileDescriptor import android.util.Log @@ -22,6 +23,7 @@ private class RestoreCoordinatorState( private val TAG = RestoreCoordinator::class.java.simpleName internal class RestoreCoordinator( + private val context: Context, private val settingsManager: SettingsManager, private val plugin: RestorePlugin, private val kv: KVRestore, @@ -37,7 +39,7 @@ internal class RestoreCoordinator( * or null if an error occurred (the attempt should be rescheduled). **/ fun getAvailableRestoreSets(): Array? { - val availableBackups = plugin.getAvailableBackups() ?: return null + val availableBackups = plugin.getAvailableBackups(context) ?: return null val restoreSets = ArrayList() for (encryptedMetadata in availableBackups) { if (encryptedMetadata.error) continue diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorePlugin.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorePlugin.kt index f0e494eb..4a643438 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorePlugin.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorePlugin.kt @@ -1,5 +1,6 @@ package com.stevesoltys.seedvault.transport.restore +import android.content.Context import com.stevesoltys.seedvault.metadata.EncryptedBackupMetadata interface RestorePlugin { @@ -14,6 +15,6 @@ interface RestorePlugin { * @return metadata for the set of restore images available, * or null if an error occurred (the attempt should be rescheduled). **/ - fun getAvailableBackups(): Sequence? + fun getAvailableBackups(context: Context): Sequence? } diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/plugins/DocumentsProviderRestorePlugin.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/plugins/DocumentsProviderRestorePlugin.kt index 473066d0..ee451ecc 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/plugins/DocumentsProviderRestorePlugin.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/plugins/DocumentsProviderRestorePlugin.kt @@ -1,11 +1,11 @@ package com.stevesoltys.seedvault.transport.restore.plugins +import android.content.Context import android.util.Log +import androidx.annotation.WorkerThread import androidx.documentfile.provider.DocumentFile import com.stevesoltys.seedvault.metadata.EncryptedBackupMetadata -import com.stevesoltys.seedvault.transport.backup.plugins.DocumentsStorage -import com.stevesoltys.seedvault.transport.backup.plugins.FILE_BACKUP_METADATA -import com.stevesoltys.seedvault.transport.backup.plugins.FILE_NO_MEDIA +import com.stevesoltys.seedvault.transport.backup.plugins.* import com.stevesoltys.seedvault.transport.restore.FullRestorePlugin import com.stevesoltys.seedvault.transport.restore.KVRestorePlugin import com.stevesoltys.seedvault.transport.restore.RestorePlugin @@ -23,9 +23,9 @@ class DocumentsProviderRestorePlugin(private val storage: DocumentsStorage) : Re DocumentsProviderFullRestorePlugin(storage) } - override fun getAvailableBackups(): Sequence? { + override fun getAvailableBackups(context: Context): Sequence? { val rootDir = storage.rootBackupDir ?: return null - val backupSets = getBackups(rootDir) + val backupSets = getBackups(context, rootDir) val iterator = backupSets.iterator() return generateSequence { if (!iterator.hasNext()) return@generateSequence null // end sequence @@ -41,9 +41,17 @@ class DocumentsProviderRestorePlugin(private val storage: DocumentsStorage) : Re } companion object { - fun getBackups(rootDir: DocumentFile): List { + @WorkerThread + fun getBackups(context: Context, rootDir: DocumentFile): List { val backupSets = ArrayList() - for (set in rootDir.listFiles()) { + val files = try { + // block until the DocumentsProvider has results + rootDir.listFilesBlocking(context) + } catch (e: IOException) { + Log.e(TAG, "Error loading backups from storage", e) + return backupSets + } + for (set in files) { if (!set.isDirectory || set.name == null) { if (set.name != FILE_NO_MEDIA) { Log.w(TAG, "Found invalid backup set folder: ${set.name}") @@ -53,10 +61,11 @@ class DocumentsProviderRestorePlugin(private val storage: DocumentsStorage) : Re val token = try { set.name!!.toLong() } catch (e: NumberFormatException) { - Log.w(TAG, "Found invalid backup set folder: ${set.name}", e) + Log.w(TAG, "Found invalid backup set folder: ${set.name}") continue } - val metadata = set.findFile(FILE_BACKUP_METADATA) + // block until children of set are available + val metadata = set.findFileBlocking(context, FILE_BACKUP_METADATA) if (metadata == null) { Log.w(TAG, "Missing metadata file in backup set folder: ${set.name}") } else { 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 5183ad74..944e94b7 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 @@ -7,6 +7,7 @@ import androidx.annotation.WorkerThread import androidx.documentfile.provider.DocumentFile import com.stevesoltys.seedvault.R import com.stevesoltys.seedvault.transport.backup.plugins.DIRECTORY_ROOT +import com.stevesoltys.seedvault.transport.backup.plugins.findFileBlocking import com.stevesoltys.seedvault.transport.restore.plugins.DocumentsProviderRestorePlugin private val TAG = RestoreStorageViewModel::class.java.simpleName @@ -40,12 +41,9 @@ internal class RestoreStorageViewModel(private val app: Application) : StorageVi */ @WorkerThread private fun hasBackup(folderUri: Uri): Boolean { - // FIXME This currently fails for NextCloud's DocumentsProvider, - // if called right after setting up an account. - // It requires three attempts to finally find existing backups. val parent = DocumentFile.fromTreeUri(app, folderUri) ?: throw AssertionError() - val rootDir = parent.findFile(DIRECTORY_ROOT) ?: return false - val backupSets = DocumentsProviderRestorePlugin.getBackups(rootDir) + val rootDir = parent.findFileBlocking(app, DIRECTORY_ROOT) ?: return false + val backupSets = DocumentsProviderRestorePlugin.getBackups(app, rootDir) return backupSets.isNotEmpty() } diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/CoordinatorIntegrationTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/CoordinatorIntegrationTest.kt index 89904481..12331375 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/CoordinatorIntegrationTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/CoordinatorIntegrationTest.kt @@ -50,7 +50,7 @@ internal class CoordinatorIntegrationTest : TransportTest() { private val kvRestore = KVRestore(kvRestorePlugin, outputFactory, headerReader, cryptoImpl) private val fullRestorePlugin = mockk() private val fullRestore = FullRestore(fullRestorePlugin, outputFactory, headerReader, cryptoImpl) - private val restore = RestoreCoordinator(settingsManager, restorePlugin, kvRestore, fullRestore, metadataReader) + private val restore = RestoreCoordinator(context, settingsManager, restorePlugin, kvRestore, fullRestore, metadataReader) private val backupDataInput = mockk() private val fileDescriptor = mockk(relaxed = true) diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt index b857d785..46fd3ff0 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt @@ -27,7 +27,7 @@ internal class RestoreCoordinatorTest : TransportTest() { private val full = mockk() private val metadataReader = mockk() - private val restore = RestoreCoordinator(settingsManager, plugin, kv, full, metadataReader) + private val restore = RestoreCoordinator(context, settingsManager, plugin, kv, full, metadataReader) private val token = Random.nextLong() private val inputStream = mockk() @@ -43,7 +43,7 @@ internal class RestoreCoordinatorTest : TransportTest() { androidVersion = Random.nextInt(), deviceName = getRandomString()) - every { plugin.getAvailableBackups() } returns sequenceOf(encryptedMetadata, encryptedMetadata) + every { plugin.getAvailableBackups(context) } returns sequenceOf(encryptedMetadata, encryptedMetadata) every { metadataReader.readMetadata(inputStream, token) } returns metadata every { inputStream.close() } just Runs