Try everything to abort backups on metered network (when not allowed)

app/src/main/AndroidManifest.xml

@@ -159,6 +159,14 @@
                 android:resource="@xml/device_filter" />
         </receiver>
 
+        <receiver
+            android:name=".settings.TryAgainBroadcastReceiver"
+            android:exported="false">
+            <intent-filter>
+                <action android:name="com.stevesoltys.seedvault.action.TRY_AGAIN" />
+            </intent-filter>
+        </receiver>
+
         <receiver
             android:name=".restore.RestoreErrorBroadcastReceiver"
             android:exported="false">

app/src/main/java/com/stevesoltys/seedvault/backend/BackendManager.kt

@@ -110,7 +110,7 @@ class BackendManager(
      * @return true if a backup is possible, false if not.
      */
     @WorkerThread
-    fun canDoBackupNow(): Boolean {
+    override fun canDoBackupNow(): Boolean {
         val storage = backendProperties ?: return false
         return !isOnUnavailableUsb() &&
             !storage.isUnavailableNetwork(context, settingsManager.useMeteredNetwork)

app/src/main/java/com/stevesoltys/seedvault/settings/SettingsFragment.kt

@@ -200,6 +200,17 @@ class SettingsFragment : PreferenceFragmentCompat() {
     private fun onMenuItemSelected(item: MenuItem): Boolean = when (item.itemId) {
         R.id.action_backup -> {
             viewModel.backupNow()
+            if (!backendManager.canDoBackupNow()) {
+                // if USB isn't plugged in, this action shouldn't be enabled,
+                // so this leaves only that we are on a metered network
+                MaterialAlertDialogBuilder(requireContext())
+                    .setTitle(getString(R.string.settings_backup_metered_title))
+                    .setMessage(getString(R.string.settings_backup_metered_text))
+                    .setNeutralButton(getString(R.string.restore_storage_got_it)) { dialog, _ ->
+                        dialog.dismiss()
+                    }
+                    .show()
+            }
             true
         }
         R.id.action_restore -> {

app/src/main/java/com/stevesoltys/seedvault/settings/TryAgainBroadcastReceiver.kt

@@ -0,0 +1,36 @@
+/*
+ * SPDX-FileCopyrightText: 2020 The Calyx Institute
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.stevesoltys.seedvault.settings
+
+import android.app.backup.IBackupManager
+import android.content.BroadcastReceiver
+import android.content.Context
+import android.content.Intent
+import com.stevesoltys.seedvault.backend.BackendManager
+import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
+import com.stevesoltys.seedvault.worker.BackupRequester.Companion.requestFilesAndAppBackup
+import org.koin.core.context.GlobalContext.get
+
+internal const val ACTION_TRY_AGAIN = "com.stevesoltys.seedvault.action.TRY_AGAIN"
+
+class TryAgainBroadcastReceiver : BroadcastReceiver() {
+
+    // using KoinComponent would crash robolectric tests :(
+    private val notificationManager: BackupNotificationManager by lazy { get().get() }
+    private val backendManager: BackendManager by lazy { get().get() }
+    private val settingsManager: SettingsManager by lazy { get().get() }
+    private val backupManager: IBackupManager by lazy { get().get() }
+
+    override fun onReceive(context: Context, intent: Intent) {
+        if (intent.action != ACTION_TRY_AGAIN) return
+
+        notificationManager.onBackupErrorSeen()
+
+        val reschedule = !backendManager.isOnRemovableDrive
+        requestFilesAndAppBackup(context, settingsManager, backupManager, reschedule)
+    }
+
+}

app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt

@@ -204,6 +204,10 @@ internal class BackupCoordinator(
         flags: Int,
     ): Int {
         state.cancelReason = UNKNOWN_ERROR
+        if (!backendManager.canDoBackupNow()) {
+            Log.w(TAG, "performIncrementalBackup(): Can't do backup now, rejecting...")
+            return TRANSPORT_PACKAGE_REJECTED
+        }
         return kv.performBackup(packageInfo, data, flags)
     }
 
@@ -229,6 +233,10 @@ internal class BackupCoordinator(
     }
 
     fun checkFullBackupSize(size: Long): Int {
+        if (!backendManager.canDoBackupNow()) {
+            Log.w(TAG, "checkFullBackupSize(): Can't do backup now, rejecting...")
+            return TRANSPORT_PACKAGE_REJECTED
+        }
         val result = full.checkFullBackupSize(size)
         if (result == TRANSPORT_PACKAGE_REJECTED) state.cancelReason = NO_DATA
         else if (result == TRANSPORT_QUOTA_EXCEEDED) state.cancelReason = QUOTA_EXCEEDED
@@ -241,6 +249,10 @@ internal class BackupCoordinator(
         flags: Int,
     ): Int {
         state.cancelReason = UNKNOWN_ERROR
+        if (!backendManager.canDoBackupNow()) {
+            Log.w(TAG, "performFullBackup(): Can't do backup now, rejecting...")
+            return TRANSPORT_PACKAGE_REJECTED
+        }
         return full.performFullBackup(targetPackage, fileDescriptor, flags)
     }
 

app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt

@@ -14,10 +14,10 @@ import android.app.NotificationManager
 import android.app.NotificationManager.IMPORTANCE_DEFAULT
 import android.app.NotificationManager.IMPORTANCE_HIGH
 import android.app.NotificationManager.IMPORTANCE_LOW
-import android.app.PendingIntent
 import android.app.PendingIntent.FLAG_IMMUTABLE
 import android.app.PendingIntent.FLAG_UPDATE_CURRENT
 import android.app.PendingIntent.getActivity
+import android.app.PendingIntent.getBroadcast
 import android.content.Context
 import android.content.Intent
 import android.content.Intent.FLAG_ACTIVITY_NEW_TASK
@@ -38,6 +38,7 @@ import com.stevesoltys.seedvault.restore.EXTRA_PACKAGE_NAME
 import com.stevesoltys.seedvault.restore.REQUEST_CODE_UNINSTALL
 import com.stevesoltys.seedvault.restore.RestoreActivity
 import com.stevesoltys.seedvault.settings.ACTION_APP_STATUS_LIST
+import com.stevesoltys.seedvault.settings.ACTION_TRY_AGAIN
 import com.stevesoltys.seedvault.settings.SettingsActivity
 import com.stevesoltys.seedvault.ui.check.ACTION_FINISHED
 import com.stevesoltys.seedvault.ui.check.ACTION_SHOW
@@ -228,19 +229,30 @@ internal class BackupNotificationManager(private val context: Context) {
         nm.notify(NOTIFICATION_ID_SUCCESS, notification)
     }
 
-    fun onBackupError() {
+    fun onBackupError(meteredNetwork: Boolean = false) {
         val intent = Intent(context, SettingsActivity::class.java)
         val pendingIntent = getActivity(context, 0, intent, FLAG_IMMUTABLE)
+        val actionIntent = Intent(ACTION_TRY_AGAIN).apply { setPackage(context.packageName) }
+        val flags = FLAG_UPDATE_CURRENT or FLAG_IMMUTABLE
+        val actionPendingIntent = getBroadcast(context, REQUEST_CODE_UNINSTALL, actionIntent, flags)
+        val actionText = context.getString(R.string.recovery_code_verification_try_again)
+        val action = Action(null, actionText, actionPendingIntent)
+        val text = if (meteredNetwork) {
+            context.getString(R.string.notification_failed_metered_text)
+        } else {
+            context.getString(R.string.notification_failed_text)
+        }
         val notification = Builder(context, CHANNEL_ID_ERROR).apply {
             setSmallIcon(R.drawable.ic_cloud_error)
             setContentTitle(context.getString(R.string.notification_failed_title))
-            setContentText(context.getString(R.string.notification_failed_text))
+            setContentText(text)
             setOngoing(false)
             setShowWhen(true)
             setAutoCancel(true)
             setContentIntent(pendingIntent)
             setWhen(System.currentTimeMillis())
             setProgress(0, 0, false)
+            addAction(action)
             priority = PRIORITY_LOW
         }.build()
         nm.cancel(NOTIFICATION_ID_OBSERVER)
@@ -312,7 +324,7 @@ internal class BackupNotificationManager(private val context: Context) {
         }
         val flags = FLAG_UPDATE_CURRENT or FLAG_IMMUTABLE
         val pendingIntent =
-            PendingIntent.getBroadcast(context, REQUEST_CODE_UNINSTALL, intent, flags)
+            getBroadcast(context, REQUEST_CODE_UNINSTALL, intent, flags)
         val actionText = context.getString(R.string.notification_restore_error_action)
         val action = Action(R.drawable.ic_warning, actionText, pendingIntent)
         val notification = Builder(context, CHANNEL_ID_RESTORE_ERROR).apply {

app/src/main/java/com/stevesoltys/seedvault/ui/notification/NotificationBackupObserver.kt

@@ -19,6 +19,7 @@ import com.stevesoltys.seedvault.ERROR_BACKUP_CANCELLED
 import com.stevesoltys.seedvault.ERROR_BACKUP_NOT_ALLOWED
 import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.R
+import com.stevesoltys.seedvault.backend.BackendManager
 import com.stevesoltys.seedvault.metadata.MetadataManager
 import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.repo.AppBackupManager
@@ -44,6 +45,7 @@ internal class NotificationBackupObserver(
     private val settingsManager: SettingsManager by inject()
     private val metadataManager: MetadataManager by inject()
     private val appBackupManager: AppBackupManager by inject()
+    private val backendManager: BackendManager by inject()
     private var currentPackage: String? = null
     private var numPackages: Int = 0
     private var numPackagesToReport: Int = 0
@@ -148,16 +150,17 @@ internal class NotificationBackupObserver(
                 settingsManager.disableBackup(packageName)
             }
         }
-        // FIXME we should consider not requesting backup of more chunks of packages,
-        //  if the backup has already failed for this chunk,
-        //  because it will result in incomplete snapshots
-        //  since the rest of packages from the failed chunk won't get backed up.
-        //  So we either re-include those packages somehow (may fail again in a loop!)
-        //  or we simply fail the entire backup which may cause more failures for users :(
-        if (backupRequester.requestNext()) {
-            if (isLoggable(TAG, INFO)) {
-                Log.i(TAG, "Backup finished $numPackages/$requestedPackages. Status: $status")
-            }
+        Log.i(TAG, "Backup finished $numPackages/$requestedPackages. Status: $status")
+        if (backupRequester.hasNext && !backendManager.canDoBackupNow()) {
+            Log.w(TAG, "Not requesting another backup, likely on metered network. ")
+            nm.onBackupError(true)
+        } else if (backupRequester.requestNext()) {
+            // FIXME we should consider not requesting backup of more chunks of packages,
+            //  if the backup has already failed for this chunk,
+            //  because it will result in incomplete snapshots
+            //  since the rest of packages from the failed chunk won't get backed up.
+            //  So we either re-include those packages somehow (may fail again in a loop!)
+            //  or we simply fail the entire backup which may cause more failures for users :(
             var success = status == 0
             val total = try {
                 packageService.allUserPackages.size

app/src/main/java/com/stevesoltys/seedvault/worker/ApkBackupManager.kt

@@ -8,6 +8,7 @@ package com.stevesoltys.seedvault.worker
 import android.content.Context
 import android.content.pm.PackageInfo
 import android.util.Log
+import com.stevesoltys.seedvault.backend.BackendManager
 import com.stevesoltys.seedvault.metadata.MetadataManager
 import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
@@ -24,6 +25,7 @@ import java.io.IOException
 internal class ApkBackupManager(
     private val context: Context,
     private val appBackupManager: AppBackupManager,
+    private val backendManager: BackendManager,
     private val settingsManager: SettingsManager,
     private val snapshotManager: SnapshotManager,
     private val metadataManager: MetadataManager,
@@ -43,6 +45,7 @@ internal class ApkBackupManager(
             // Since an APK backup does not change the [packageState], we first record it for all
             // packages that don't get backed up.
             recordNotBackedUpPackages()
+            if (!backendManager.canDoBackupNow()) throw IllegalStateException("can't do backup now")
             // Upload current icons, so we can show them to user before restore
             uploadIcons()
             // Now, if APK backups are enabled by the user, we back those up.
@@ -60,6 +63,9 @@ internal class ApkBackupManager(
     private suspend fun backUpApks() {
         val apps = packageService.allUserPackages
         apps.forEachIndexed { i, packageInfo ->
+            // the situation may change, so stop backup when it does by throwing exception
+            if (!backendManager.canDoBackupNow()) throw IllegalStateException("can't do backup now")
+
             val packageName = packageInfo.packageName
             val name = getAppName(context, packageName)
             nm.onApkBackup(packageName, name, i, apps.size)
@@ -68,6 +74,7 @@ internal class ApkBackupManager(
     }
 
     // TODO we could use BackupMonitor for this. It emits LOG_EVENT_ID_PACKAGE_STOPPED
+    //  need to check if it has something for NOT_ALLOWED as well
     private fun recordNotBackedUpPackages() {
         nm.onAppsNotBackedUp()
         packageService.notBackedUpPackages.forEach { packageInfo ->

app/src/main/java/com/stevesoltys/seedvault/worker/AppBackupWorker.kt

@@ -121,19 +121,21 @@ class AppBackupWorker(
         } catch (e: Exception) {
             Log.e(TAG, "Error while running setForeground: ", e)
         }
-        val freeSpace = backendManager.getFreeSpace()
-        Log.i(TAG, "freeSpace: $freeSpace")
-        if (freeSpace != null && freeSpace < MIN_FREE_SPACE) {
-            nm.onInsufficientSpaceError()
-            return Result.failure()
-        }
         return try {
-            if (isStopped) {
+            if (isStopped || !backendManager.canDoBackupNow()) {
                 Result.retry()
             } else {
+                val freeSpace = backendManager.getFreeSpace()
+                Log.i(TAG, "freeSpace: $freeSpace")
+                if (freeSpace != null && freeSpace < MIN_FREE_SPACE) {
+                    nm.onInsufficientSpaceError()
+                    return Result.failure()
+                }
                 val result = doBackup()
-                // show error notification if backup wasn't successful (maybe only when no retry?)
-                if (result != Result.success()) nm.onBackupError()
+                // show error notification if backup wasn't successful
+                if (result != Result.success()) {
+                    nm.onBackupError(meteredNetwork = !backendManager.canDoBackupNow())
+                }
                 // only allow retrying if rescheduling is allowed
                 if (tags.contains(TAG_RESCHEDULE)) return result
                 else Result.success()
@@ -149,32 +151,36 @@ class AppBackupWorker(
     }
 
     private suspend fun doBackup(): Result {
-        var result: Result = Result.success()
-        if (!isStopped) {
+        if (!isStopped && backendManager.canDoBackupNow()) {
             Log.i(TAG, "Initializing backup info...")
             try {
                 appBackupManager.beforeBackup()
             } catch (e: Exception) {
                 Log.e(TAG, "Error during 'beforeBackup': ", e)
                 return Result.retry()
             }
+        } else {
+            Log.i(TAG, "Stopping, because s:$isStopped c:${backendManager.canDoBackupNow()}")
         }
         try {
-            Log.i(TAG, "Starting APK backup... (stopped: $isStopped)")
-            if (!isStopped) apkBackupManager.backup()
+            if (!isStopped && backendManager.canDoBackupNow()) {
+                Log.i(TAG, "Starting APK backup...")
+                apkBackupManager.backup()
+            } else {
+                Log.i(TAG, "Stopping, because s:$isStopped c:${backendManager.canDoBackupNow()}")
+            }
         } catch (e: Exception) {
             Log.e(TAG, "Error backing up APKs: ", e)
-            result = Result.retry()
-        } finally {
-            Log.i(TAG, "Requesting app data backup... (stopped: $isStopped)")
-            val requestSuccess = if (!isStopped && backupRequester.isBackupEnabled) {
-                Log.d(TAG, "Backup is enabled, request backup...")
-                backupRequester.requestBackup()
-            } else true
-            Log.d(TAG, "Have requested backup.")
-            if (!requestSuccess) result = Result.retry()
+            return Result.retry()
+        }
+        Log.i(TAG, "Requesting app data backup... (stopped: $isStopped)")
+        if (!isStopped && backupRequester.isBackupEnabled && backendManager.canDoBackupNow()) {
+            Log.i(TAG, "Backup is enabled, request backup...")
+            if (!backupRequester.requestBackup()) return Result.retry()
+        } else {
+            Log.i(TAG, "Stopping, because s:$isStopped c:${backendManager.canDoBackupNow()}")
         }
-        return result
+        return Result.success()
     }
 
     private fun createForegroundInfo() = ForegroundInfo(

app/src/main/java/com/stevesoltys/seedvault/worker/BackupRequester.kt

@@ -70,12 +70,10 @@ internal class BackupRequester(
 
     val isBackupEnabled: Boolean get() = backupManager.isBackupEnabled
 
-    private val packages = packageService.eligiblePackages
-    private val observer = NotificationBackupObserver(
-        context = context,
-        backupRequester = this,
-        requestedPackages = packages.size,
-    )
+    private val packages by lazy { packageService.eligiblePackages }
+    private val observer by lazy {
+        NotificationBackupObserver(context, this, packages.size)
+    }
 
     /**
      * The current package index.
@@ -93,6 +91,11 @@ internal class BackupRequester(
         return request(getNextChunk())
     }
 
+    /**
+     * Returns true, if there are more packages waiting to get backed up by calling [requestNext].
+     */
+    val hasNext: Boolean get() = packageIndex < packages.size
+
     /**
      * Backs up the next chunk of packages.
      *

app/src/main/java/com/stevesoltys/seedvault/worker/WorkerModule.kt

@@ -40,6 +40,7 @@ val workerModule = module {
         ApkBackupManager(
             context = androidContext(),
             appBackupManager = get(),
+            backendManager = get(),
             settingsManager = get(),
             snapshotManager = get(),
             metadataManager = get(),

app/src/main/res/values/strings.xml

@@ -60,6 +60,9 @@
     <string name="settings_backup_new_code_dialog_title">New recovery code required</string>
     <string name="settings_backup_new_code_dialog_message">To continue using app backups, you need to generate a new recovery code.\n\nWe are sorry for the inconvenience.</string>
     <string name="settings_backup_new_code_code_dialog_ok">New code</string>
+    <string name="settings_backup_metered_title">Backup stopped</string>
+    <string name="settings_backup_metered_text">The backup won\'t proceed because your device is using mobile data.\n\nYou can enable backups on mobile data under \"Backup scheduling\".</string>
+
 
     <string name="settings_scheduling_frequency_title">Backup frequency</string>
     <string name="settings_scheduling_frequency_12_hours">Every 12 hours</string>
@@ -169,6 +172,7 @@
     <string name="notification_success_text">%1$d of %2$d apps backed up (%3$s). Tap to learn more.</string>
     <string name="notification_failed_title">Backup failed</string>
     <string name="notification_failed_text">An error occurred while running the backup.</string>
+    <string name="notification_failed_metered_text">The backup has been aborted. Are you using mobile data? Connect to Wi-Fi to continue.</string>
 
     <string name="notification_error_channel_title">Error notification</string>
     <string name="notification_error_title">Backup error</string>