From 650642068e3285c959fd75d2a13f2c93edd0170a Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 17 Sep 2019 15:45:14 -0300 Subject: [PATCH] Don't try to do backups if storage is not available --- .../backup/BackupNotificationManager.kt | 2 +- .../backup/settings/SettingsManager.kt | 7 ++- .../transport/backup/BackupCoordinator.kt | 46 ++++++++++++++++++- .../backup/transport/backup/FullBackup.kt | 5 -- .../backup/transport/backup/KVBackup.kt | 5 -- .../backup/plugins/DocumentsStorage.kt | 4 +- .../backup/ui/storage/StorageViewModel.kt | 4 +- .../backup/transport/backup/FullBackupTest.kt | 5 -- .../backup/transport/backup/KVBackupTest.kt | 5 -- 9 files changed, 52 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/backup/BackupNotificationManager.kt b/app/src/main/java/com/stevesoltys/backup/BackupNotificationManager.kt index 51b9317e..d8df735a 100644 --- a/app/src/main/java/com/stevesoltys/backup/BackupNotificationManager.kt +++ b/app/src/main/java/com/stevesoltys/backup/BackupNotificationManager.kt @@ -79,9 +79,9 @@ class BackupNotificationManager(private val context: Context) { val notification = errorBuilder.apply { setContentTitle(context.getString(R.string.notification_error_title)) setContentText(context.getString(R.string.notification_error_text)) - addAction(action) setOnlyAlertOnce(true) setAutoCancel(true) + mActions = arrayListOf(action) }.build() nm.notify(NOTIFICATION_ID_ERROR, notification) } diff --git a/app/src/main/java/com/stevesoltys/backup/settings/SettingsManager.kt b/app/src/main/java/com/stevesoltys/backup/settings/SettingsManager.kt index 70ec8db5..404efae9 100644 --- a/app/src/main/java/com/stevesoltys/backup/settings/SettingsManager.kt +++ b/app/src/main/java/com/stevesoltys/backup/settings/SettingsManager.kt @@ -3,6 +3,7 @@ package com.stevesoltys.backup.settings import android.content.Context import android.net.Uri import android.preference.PreferenceManager.getDefaultSharedPreferences +import androidx.documentfile.provider.DocumentFile import java.util.* private const val PREF_KEY_STORAGE_URI = "storageUri" @@ -14,8 +15,10 @@ private const val PREF_KEY_BACKUP_PASSWORD = "backupLegacyPassword" data class Storage( val uri: Uri, val name: String, - val ejectable: Boolean -) + val ejectable: Boolean) { + fun getDocumentFile(context: Context) = DocumentFile.fromTreeUri(context, uri) + ?: throw AssertionError("Should only happen on API < 21.") +} fun setStorage(context: Context, storage: Storage) { getDefaultSharedPreferences(context) diff --git a/app/src/main/java/com/stevesoltys/backup/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/backup/transport/backup/BackupCoordinator.kt index c244e611..44eef03c 100644 --- a/app/src/main/java/com/stevesoltys/backup/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/backup/transport/backup/BackupCoordinator.kt @@ -9,7 +9,9 @@ import android.util.Log import com.stevesoltys.backup.BackupNotificationManager import com.stevesoltys.backup.metadata.MetadataWriter import com.stevesoltys.backup.settings.getBackupToken +import com.stevesoltys.backup.settings.getStorage import java.io.IOException +import java.util.concurrent.TimeUnit.MINUTES private val TAG = BackupCoordinator::class.java.simpleName @@ -83,7 +85,20 @@ class BackupCoordinator( // Key/value incremental backup support // - fun requestBackupTime() = kv.requestBackupTime() + /** + * Verify that this is a suitable time for a key/value backup pass. + * This should return zero if a backup is reasonable right now, some positive value otherwise. + * This method will be called outside of the [performIncrementalBackup]/[finishBackup] pair. + * + * If this is not a suitable time for a backup, the transport should return a backoff delay, + * in milliseconds, after which the Backup Manager should try again. + * + * @return Zero if this is a suitable time for a backup pass, or a positive time delay + * in milliseconds to suggest deferring the backup pass for a while. + */ + fun requestBackupTime(): Long = getBackupBackoff().apply { + Log.i(TAG, "Request incremental backup time. Returned $this") + } fun performIncrementalBackup(packageInfo: PackageInfo, data: ParcelFileDescriptor, flags: Int) = kv.performBackup(packageInfo, data, flags) @@ -92,7 +107,22 @@ class BackupCoordinator( // Full backup // - fun requestFullBackupTime() = full.requestFullBackupTime() + /** + * Verify that this is a suitable time for a full-data backup pass. + * This should return zero if a backup is reasonable right now, some positive value otherwise. + * This method will be called outside of the [performFullBackup]/[finishBackup] pair. + * + * If this is not a suitable time for a backup, the transport should return a backoff delay, + * in milliseconds, after which the Backup Manager should try again. + * + * @return Zero if this is a suitable time for a backup pass, or a positive time delay + * in milliseconds to suggest deferring the backup pass for a while. + * + * @see [requestBackupTime] + */ + fun requestFullBackupTime(): Long = getBackupBackoff().apply { + Log.i(TAG, "Request full backup time. Returned $this") + } fun checkFullBackupSize(size: Long) = full.checkFullBackupSize(size) @@ -156,4 +186,16 @@ class BackupCoordinator( metadataWriter.write(outputStream, token) } + private fun getBackupBackoff(): Long { + val noBackoff = 0L + val defaultBackoff = MINUTES.toMillis(10) + + // back off if there's no storage set + val storage = getStorage(context) ?: return defaultBackoff + // don't back off if storage is not ejectable or available right now + return if (!storage.ejectable || storage.getDocumentFile(context).isDirectory) noBackoff + // otherwise back off + else defaultBackoff + } + } diff --git a/app/src/main/java/com/stevesoltys/backup/transport/backup/FullBackup.kt b/app/src/main/java/com/stevesoltys/backup/transport/backup/FullBackup.kt index ec2746d5..37639176 100644 --- a/app/src/main/java/com/stevesoltys/backup/transport/backup/FullBackup.kt +++ b/app/src/main/java/com/stevesoltys/backup/transport/backup/FullBackup.kt @@ -36,11 +36,6 @@ class FullBackup( fun hasState() = state != null - fun requestFullBackupTime(): Long { - Log.i(TAG, "Request full backup time") - return 0 - } - fun getQuota(): Long = plugin.getQuota() fun checkFullBackupSize(size: Long): Int { diff --git a/app/src/main/java/com/stevesoltys/backup/transport/backup/KVBackup.kt b/app/src/main/java/com/stevesoltys/backup/transport/backup/KVBackup.kt index 05138e0a..7e3de67a 100644 --- a/app/src/main/java/com/stevesoltys/backup/transport/backup/KVBackup.kt +++ b/app/src/main/java/com/stevesoltys/backup/transport/backup/KVBackup.kt @@ -27,11 +27,6 @@ class KVBackup( fun hasState() = state != null - fun requestBackupTime(): Long { - Log.i(TAG, "Request K/V backup time") - return 0 - } - fun getQuota(): Long = plugin.getQuota() fun performBackup(packageInfo: PackageInfo, data: ParcelFileDescriptor, flags: Int): Int { diff --git a/app/src/main/java/com/stevesoltys/backup/transport/backup/plugins/DocumentsStorage.kt b/app/src/main/java/com/stevesoltys/backup/transport/backup/plugins/DocumentsStorage.kt index 505dd86c..93865ee8 100644 --- a/app/src/main/java/com/stevesoltys/backup/transport/backup/plugins/DocumentsStorage.kt +++ b/app/src/main/java/com/stevesoltys/backup/transport/backup/plugins/DocumentsStorage.kt @@ -22,9 +22,7 @@ private val TAG = DocumentsStorage::class.java.simpleName class DocumentsStorage(private val context: Context, storage: Storage?, token: Long) { internal val rootBackupDir: DocumentFile? by lazy { - val folderUri = storage?.uri ?: return@lazy null - // [fromTreeUri] should only return null when SDK_INT < 21 - val parent = DocumentFile.fromTreeUri(context, folderUri) ?: throw AssertionError() + val parent = storage?.getDocumentFile(context) ?: return@lazy null try { val rootDir = parent.createOrGetDirectory(DIRECTORY_ROOT) // create .nomedia file to prevent Android's MediaScanner from trying to index the backup diff --git a/app/src/main/java/com/stevesoltys/backup/ui/storage/StorageViewModel.kt b/app/src/main/java/com/stevesoltys/backup/ui/storage/StorageViewModel.kt index 1868f25b..6d0872a3 100644 --- a/app/src/main/java/com/stevesoltys/backup/ui/storage/StorageViewModel.kt +++ b/app/src/main/java/com/stevesoltys/backup/ui/storage/StorageViewModel.kt @@ -7,7 +7,6 @@ import android.content.Intent.FLAG_GRANT_READ_URI_PERMISSION import android.content.Intent.FLAG_GRANT_WRITE_URI_PERMISSION import android.net.Uri import android.util.Log -import androidx.documentfile.provider.DocumentFile import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData @@ -41,8 +40,7 @@ internal abstract class StorageViewModel(private val app: Application) : Android internal fun validLocationIsSet(context: Context): Boolean { val storage = getStorage(context) ?: return false if (storage.ejectable) return true - val file = DocumentFile.fromTreeUri(context, storage.uri) ?: return false - return file.isDirectory + return storage.getDocumentFile(context).isDirectory } } diff --git a/app/src/test/java/com/stevesoltys/backup/transport/backup/FullBackupTest.kt b/app/src/test/java/com/stevesoltys/backup/transport/backup/FullBackupTest.kt index af66c82b..b8b79cf7 100644 --- a/app/src/test/java/com/stevesoltys/backup/transport/backup/FullBackupTest.kt +++ b/app/src/test/java/com/stevesoltys/backup/transport/backup/FullBackupTest.kt @@ -20,11 +20,6 @@ internal class FullBackupTest : BackupTest() { private val closeBytes = ByteArray(42).apply { Random.nextBytes(this) } private val inputStream = mockk() - @Test - fun `now is a good time for a backup`() { - assertEquals(0, backup.requestFullBackupTime()) - } - @Test fun `has no initial state`() { assertFalse(backup.hasState()) diff --git a/app/src/test/java/com/stevesoltys/backup/transport/backup/KVBackupTest.kt b/app/src/test/java/com/stevesoltys/backup/transport/backup/KVBackupTest.kt index 422c3aab..59955663 100644 --- a/app/src/test/java/com/stevesoltys/backup/transport/backup/KVBackupTest.kt +++ b/app/src/test/java/com/stevesoltys/backup/transport/backup/KVBackupTest.kt @@ -28,11 +28,6 @@ internal class KVBackupTest : BackupTest() { private val value = ByteArray(23).apply { Random.nextBytes(this) } private val versionHeader = VersionHeader(packageName = packageInfo.packageName, key = key) - @Test - fun `now is a good time for a backup`() { - assertEquals(0, backup.requestBackupTime()) - } - @Test fun `has no initial state`() { assertFalse(backup.hasState())