Don't try to do backups if storage is not available

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)
     }

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)

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
+    }
+
 }

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 {

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 {

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
+        val parent = storage?.getDocumentFile(context) ?: return@lazy null
-        // [fromTreeUri] should only return null when SDK_INT < 21
-        val parent = DocumentFile.fromTreeUri(context, folderUri) ?: throw AssertionError()
         try {
             val rootDir = parent.createOrGetDirectory(DIRECTORY_ROOT)
             // create .nomedia file to prevent Android's MediaScanner from trying to index the backup

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 storage.getDocumentFile(context).isDirectory
-            return file.isDirectory
         }
     }
 

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<FileInputStream>()
 
-    @Test
-    fun `now is a good time for a backup`() {
-        assertEquals(0, backup.requestFullBackupTime())
-    }
-
     @Test
     fun `has no initial state`() {
         assertFalse(backup.hasState())

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())