Fix backup message job cancel and start bugs.

2025-12-22 20:18:36 +00:00 · 2025-07-16 14:21:59 -04:00
parent 141faf3fb6
commit 8ee80b0d27
11 changed files with 135 additions and 17 deletions
--- a/app/src/main/java/org/thoughtcrime/securesms/backup/ArchiveUploadProgress.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/backup/ArchiveUploadProgress.kt
@@ -13,6 +13,7 @@ import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.flow.onStart
 import kotlinx.coroutines.withContext
 import org.signal.core.util.logging.Log
 import org.signal.core.util.throttleLatest
@@ -22,6 +23,7 @@ import org.thoughtcrime.securesms.database.SignalDatabase
 import org.thoughtcrime.securesms.dependencies.AppDependencies
 import org.thoughtcrime.securesms.jobs.ArchiveCommitAttachmentDeletesJob
 import org.thoughtcrime.securesms.jobs.ArchiveThumbnailUploadJob
+import org.thoughtcrime.securesms.jobs.BackupMessagesJob
 import org.thoughtcrime.securesms.jobs.UploadAttachmentToArchiveJob
 import org.thoughtcrime.securesms.keyvalue.SignalStore
 import org.thoughtcrime.securesms.keyvalue.protos.ArchiveUploadProgressState
@@ -88,6 +90,7 @@ object ArchiveUploadProgress {
    .onEach { updated ->
      updateState(notify = false) { updated }
    }
+    .onStart { emit(uploadProgress) }
    .flowOn(Dispatchers.IO)

  val inProgress
@@ -108,6 +111,8 @@ object ArchiveUploadProgress {
      )
    }

+    BackupMessagesJob.cancel()
+
    AppDependencies.jobManager.cancelAllInQueue(ArchiveCommitAttachmentDeletesJob.ARCHIVE_ATTACHMENT_QUEUE)
    UploadAttachmentToArchiveJob.getAllQueueKeys().forEach {
      AppDependencies.jobManager.cancelAllInQueue(it)
--- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/backups/remote/RemoteBackupsSettingsFragment.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/backups/remote/RemoteBackupsSettingsFragment.kt
@@ -511,6 +511,7 @@ private fun RemoteBackupsSettingsContent(
          canViewBackupKey = state.canViewBackupKey,
          backupRestoreState = backupRestoreState,
          backupProgress = backupProgress,
+          canBackupMessagesRun = state.canBackupMessagesJobRun,
          lastBackupTimestamp = state.lastBackupTimestamp,
          backupMediaSize = state.backupMediaSize,
          backupsFrequency = state.backupsFrequency,
@@ -815,6 +816,7 @@ private fun LazyListScope.appendBackupDetailsItems(
  canViewBackupKey: Boolean,
  backupRestoreState: BackupRestoreState,
  backupProgress: ArchiveUploadProgressState?,
+  canBackupMessagesRun: Boolean,
  lastBackupTimestamp: Long,
  backupMediaSize: Long,
  backupsFrequency: BackupFrequency,
@@ -868,6 +870,8 @@ private fun LazyListScope.appendBackupDetailsItems(
    item {
      InProgressBackupRow(
        archiveUploadProgressState = backupProgress,
+        canBackupMessagesRun = canBackupMessagesRun,
+        canBackupUsingCellular = canBackUpUsingCellular,
        cancelArchiveUpload = contentCallbacks::onCancelUploadClick
      )
    }
@@ -1326,6 +1330,8 @@ private fun SubscriptionMismatchMissingGooglePlayCard(
@Composable
 private fun InProgressBackupRow(
  archiveUploadProgressState: ArchiveUploadProgressState,
+  canBackupMessagesRun: Boolean = true,
+  canBackupUsingCellular: Boolean = true,
  cancelArchiveUpload: () -> Unit = {}
 ) {
  Row(
@@ -1361,7 +1367,7 @@ private fun InProgressBackupRow(
      }

      Text(
-        text = getProgressStateMessage(archiveUploadProgressState),
+        text = getProgressStateMessage(archiveUploadProgressState, canBackupMessagesRun, canBackupUsingCellular),
        style = MaterialTheme.typography.bodyMedium,
        color = MaterialTheme.colorScheme.onSurfaceVariant
      )
@@ -1399,18 +1405,26 @@ private fun ArchiveProgressIndicator(
 }

@Composable
-private fun getProgressStateMessage(archiveUploadProgressState: ArchiveUploadProgressState): String {
+private fun getProgressStateMessage(archiveUploadProgressState: ArchiveUploadProgressState, canBackupMessagesRun: Boolean, canBackupUsingCellular: Boolean): String {
  return when (archiveUploadProgressState.state) {
    ArchiveUploadProgressState.State.None, ArchiveUploadProgressState.State.UserCanceled -> stringResource(R.string.RemoteBackupsSettingsFragment__processing_backup)
-    ArchiveUploadProgressState.State.Export -> getBackupExportPhaseProgressString(archiveUploadProgressState)
+    ArchiveUploadProgressState.State.Export -> getBackupExportPhaseProgressString(archiveUploadProgressState, canBackupMessagesRun, canBackupUsingCellular)
    ArchiveUploadProgressState.State.UploadBackupFile, ArchiveUploadProgressState.State.UploadMedia -> getBackupUploadPhaseProgressString(archiveUploadProgressState)
  }
 }

@Composable
-private fun getBackupExportPhaseProgressString(state: ArchiveUploadProgressState): String {
+private fun getBackupExportPhaseProgressString(state: ArchiveUploadProgressState, canBackupMessagesRun: Boolean, canBackupUsingCellular: Boolean): String {
  return when (state.backupPhase) {
-    ArchiveUploadProgressState.BackupPhase.BackupPhaseNone -> stringResource(R.string.RemoteBackupsSettingsFragment__processing_backup)
+    ArchiveUploadProgressState.BackupPhase.BackupPhaseNone -> {
+      if (canBackupMessagesRun) {
+        stringResource(R.string.RemoteBackupsSettingsFragment__processing_backup)
+      } else if (canBackupUsingCellular) {
+        stringResource(R.string.RemoteBackupsSettingsFragment__Waiting_for_internet_connection)
+      } else {
+        stringResource(R.string.RemoteBackupsSettingsFragment__Waiting_for_Wifi)
+      }
+    }
    ArchiveUploadProgressState.BackupPhase.Message -> {
      pluralStringResource(
        R.plurals.RemoteBackupsSettingsFragment__processing_messages_progress_text,
--- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/backups/remote/RemoteBackupsSettingsState.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/backups/remote/RemoteBackupsSettingsState.kt
@@ -27,7 +27,8 @@ data class RemoteBackupsSettingsState(
  val lastBackupTimestamp: Long = 0,
  val dialog: Dialog = Dialog.NONE,
  val snackbar: Snackbar = Snackbar.NONE,
-  val includeDebuglog: Boolean? = null
+  val includeDebuglog: Boolean? = null,
+  val canBackupMessagesJobRun: Boolean = false
 ) {

  enum class Dialog {
--- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/backups/remote/RemoteBackupsSettingsViewModel.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/backups/remote/RemoteBackupsSettingsViewModel.kt
@@ -39,6 +39,7 @@ import org.thoughtcrime.securesms.database.InAppPaymentTable
 import org.thoughtcrime.securesms.database.SignalDatabase
 import org.thoughtcrime.securesms.database.attachmentUpdates
 import org.thoughtcrime.securesms.dependencies.AppDependencies
+import org.thoughtcrime.securesms.jobmanager.impl.BackupMessagesConstraint
 import org.thoughtcrime.securesms.jobs.BackupMessagesJob
 import org.thoughtcrime.securesms.keyvalue.SignalStore
 import org.thoughtcrime.securesms.keyvalue.protos.ArchiveUploadProgressState
@@ -62,6 +63,7 @@ class RemoteBackupsSettingsViewModel : ViewModel() {
    RemoteBackupsSettingsState(
      tier = SignalStore.backup.backupTier,
      backupsEnabled = SignalStore.backup.areBackupsEnabled,
+      canBackupMessagesJobRun = BackupMessagesConstraint.isMet(AppDependencies.application),
      canViewBackupKey = !TextSecurePreferences.isUnauthorizedReceived(AppDependencies.application),
      lastBackupTimestamp = SignalStore.backup.lastBackupTime,
      backupsFrequency = SignalStore.backup.backupFrequency,
@@ -152,7 +154,12 @@ class RemoteBackupsSettingsViewModel : ViewModel() {

  fun setCanBackUpUsingCellular(canBackUpUsingCellular: Boolean) {
    SignalStore.backup.backupWithCellular = canBackUpUsingCellular
-    _state.update { it.copy(canBackUpUsingCellular = canBackUpUsingCellular) }
+    _state.update {
+      it.copy(
+        canBackupMessagesJobRun = BackupMessagesConstraint.isMet(AppDependencies.application),
+        canBackUpUsingCellular = canBackUpUsingCellular
+      )
+    }
  }

  fun setCanRestoreUsingCellular() {
@@ -257,6 +264,7 @@ class RemoteBackupsSettingsViewModel : ViewModel() {
        tier = SignalStore.backup.backupTier,
        backupsEnabled = SignalStore.backup.areBackupsEnabled,
        lastBackupTimestamp = SignalStore.backup.lastBackupTime,
+        canBackupMessagesJobRun = BackupMessagesConstraint.isMet(AppDependencies.application),
        backupMediaSize = SignalDatabase.attachments.getEstimatedArchiveMediaSize(),
        backupsFrequency = SignalStore.backup.backupFrequency,
        canBackUpUsingCellular = SignalStore.backup.backupWithCellular,
--- a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/impl/BackupMessagesConstraint.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/impl/BackupMessagesConstraint.kt
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2024 Signal Messenger, LLC
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+package org.thoughtcrime.securesms.jobmanager.impl
+
+import android.app.Application
+import android.app.job.JobInfo
+import android.content.Context
+import org.thoughtcrime.securesms.jobmanager.Constraint
+import org.thoughtcrime.securesms.keyvalue.SignalStore
+
+/**
+ * Constraint that, when added, means that a job cannot be performed unless the user either has Wifi or, if they enabled it, cellular
+ */
+class BackupMessagesConstraint(private val application: Application) : Constraint {
+
+  companion object {
+    const val KEY = "BackupMessagesConstraint"
+
+    fun isMet(context: Context): Boolean {
+      if (SignalStore.backup.backupWithCellular) {
+        return NetworkConstraint.isMet(context)
+      }
+      return WifiConstraint.isMet(context)
+    }
+  }
+
+  override fun isMet(): Boolean {
+    return isMet(application)
+  }
+
+  override fun getFactoryKey(): String = KEY
+
+  override fun applyToJobInfo(jobInfoBuilder: JobInfo.Builder) = Unit
+
+  class Factory(val application: Application) : Constraint.Factory<BackupMessagesConstraint> {
+    override fun create(): BackupMessagesConstraint {
+      return BackupMessagesConstraint(application)
+    }
+  }
+}
--- a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/impl/BackupMessagesConstraintObserver.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/impl/BackupMessagesConstraintObserver.kt
@@ -0,0 +1,27 @@
+package org.thoughtcrime.securesms.jobmanager.impl
+
+import org.thoughtcrime.securesms.dependencies.AppDependencies
+import org.thoughtcrime.securesms.jobmanager.ConstraintObserver
+
+/**
+ * An observer for the [BackupMessagesConstraint]. This is called when users change whether or not backup is allowed via cellular
+ */
+object BackupMessagesConstraintObserver : ConstraintObserver {
+
+  private const val REASON = "BackupMessagesConstraint"
+
+  private var notifier: ConstraintObserver.Notifier? = null
+
+  override fun register(notifier: ConstraintObserver.Notifier) {
+    this.notifier = notifier
+  }
+
+  /**
+   * Let the observer know that the backup using cellular flag has changed.
+   */
+  fun onChange() {
+    if (BackupMessagesConstraint.isMet(AppDependencies.application)) {
+      notifier?.onConstraintMet(REASON)
+    }
+  }
+}
--- a/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt
@@ -18,8 +18,7 @@ import org.thoughtcrime.securesms.backup.v2.ResumableMessagesBackupUploadSpec
 import org.thoughtcrime.securesms.database.SignalDatabase
 import org.thoughtcrime.securesms.dependencies.AppDependencies
 import org.thoughtcrime.securesms.jobmanager.Job
-import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint
-import org.thoughtcrime.securesms.jobmanager.impl.WifiConstraint
+import org.thoughtcrime.securesms.jobmanager.impl.BackupMessagesConstraint
 import org.thoughtcrime.securesms.jobs.protos.BackupMessagesJobData
 import org.thoughtcrime.securesms.keyvalue.SignalStore
 import org.thoughtcrime.securesms.keyvalue.isDecisionPending
@@ -96,6 +95,10 @@ class BackupMessagesJob private constructor(

      chain.enqueue()
    }
+
+    fun cancel() {
+      AppDependencies.jobManager.find { it.factoryKey == KEY }.forEach { AppDependencies.jobManager.cancel(it.id) }
+    }
  }

  constructor() : this(
@@ -103,7 +106,7 @@ class BackupMessagesJob private constructor(
    dataFile = "",
    resumableMessagesBackupUploadSpec = null,
    parameters = Parameters.Builder()
-      .addConstraint(if (SignalStore.backup.backupWithCellular) NetworkConstraint.KEY else WifiConstraint.KEY)
+      .addConstraint(BackupMessagesConstraint.KEY)
      .setMaxAttempts(3)
      .setMaxInstancesForFactory(1)
      .build()
--- a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java
@@ -10,6 +10,8 @@ import org.thoughtcrime.securesms.jobmanager.ConstraintObserver;
 import org.thoughtcrime.securesms.jobmanager.Job;
 import org.thoughtcrime.securesms.jobmanager.JobMigration;
 import org.thoughtcrime.securesms.jobmanager.impl.AutoDownloadEmojiConstraint;
+import org.thoughtcrime.securesms.jobmanager.impl.BackupMessagesConstraint;
+import org.thoughtcrime.securesms.jobmanager.impl.BackupMessagesConstraintObserver;
 import org.thoughtcrime.securesms.jobmanager.impl.BatteryNotLowConstraint;
 import org.thoughtcrime.securesms.jobmanager.impl.CellServiceConstraintObserver;
 import org.thoughtcrime.securesms.jobmanager.impl.ChangeNumberConstraint;
@@ -404,6 +406,7 @@ public final class JobManagerFactories {
    return new HashMap<String, Constraint.Factory>() {{
      put(NoRemoteArchiveGarbageCollectionPendingConstraint.KEY, new NoRemoteArchiveGarbageCollectionPendingConstraint.Factory());
      put(AutoDownloadEmojiConstraint.KEY,                       new AutoDownloadEmojiConstraint.Factory(application));
+      put(BackupMessagesConstraint.KEY,                          new BackupMessagesConstraint.Factory(application));
      put(BatteryNotLowConstraint.KEY,                           new BatteryNotLowConstraint.Factory());
      put(ChangeNumberConstraint.KEY,                            new ChangeNumberConstraint.Factory());
      put(ChargingConstraint.KEY,                                new ChargingConstraint.Factory());
@@ -431,7 +434,8 @@ public final class JobManagerFactories {
                         DataRestoreConstraintObserver.INSTANCE,
                         RestoreAttachmentConstraintObserver.INSTANCE,
                         NoRemoteArchiveGarbageCollectionPendingConstraint.Observer.INSTANCE,
-                         RegisteredConstraint.Observer.INSTANCE);
+                         RegisteredConstraint.Observer.INSTANCE,
+                         BackupMessagesConstraintObserver.INSTANCE);
  }

  public static List<JobMigration> getJobMigrations(@NonNull Application application) {
--- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/BackupValues.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/BackupValues.kt
@@ -9,6 +9,7 @@ import org.thoughtcrime.securesms.backup.RestoreState
 import org.thoughtcrime.securesms.backup.v2.BackupFrequency
 import org.thoughtcrime.securesms.backup.v2.BackupRepository
 import org.thoughtcrime.securesms.backup.v2.MessageBackupTier
+import org.thoughtcrime.securesms.jobmanager.impl.BackupMessagesConstraintObserver
 import org.thoughtcrime.securesms.jobmanager.impl.NoRemoteArchiveGarbageCollectionPendingConstraint
 import org.thoughtcrime.securesms.jobmanager.impl.RestoreAttachmentConstraintObserver
 import org.thoughtcrime.securesms.keyvalue.protos.ArchiveUploadProgressState
@@ -60,7 +61,7 @@ class BackupValues(store: KeyValueStore) : SignalStoreValues(store) {
    private const val KEY_OPTIMIZE_STORAGE = "backup.optimizeStorage"
    private const val KEY_BACKUPS_INITIALIZED = "backup.initialized"

-    private const val KEY_ARCHIVE_UPLOAD_STATE = "backup.archiveUploadState"
+    const val KEY_ARCHIVE_UPLOAD_STATE = "backup.archiveUploadState"

    private const val KEY_BACKUP_UPLOADED = "backup.backupUploaded"
    private const val KEY_SUBSCRIPTION_STATE_MISMATCH = "backup.subscriptionStateMismatch"
@@ -104,7 +105,12 @@ class BackupValues(store: KeyValueStore) : SignalStoreValues(store) {

  var restoreState: RestoreState by enumValue(KEY_RESTORE_STATE, RestoreState.NONE, RestoreState.serializer)
  var optimizeStorage: Boolean by booleanValue(KEY_OPTIMIZE_STORAGE, false)
-  var backupWithCellular: Boolean by booleanValue(KEY_BACKUP_OVER_CELLULAR, false)
+  var backupWithCellular: Boolean
+    get() = getBoolean(KEY_BACKUP_OVER_CELLULAR, false)
+    set(value) {
+      putBoolean(KEY_BACKUP_OVER_CELLULAR, value)
+      BackupMessagesConstraintObserver.onChange()
+    }

  var backupDownloadNotifierState: BackupDownloadNotifierState? by protoValue(KEY_BACKUP_DOWNLOAD_NOTIFIER_STATE, BackupDownloadNotifierState.ADAPTER)
    private set
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -8249,6 +8249,10 @@
    <string name="RemoteBackupsSettingsFragment__processing_backup">Processing backup…</string>
    <!-- Linear progress dialog text shown when preparing a backup -->
    <string name="RemoteBackupsSettingsFragment__preparing_backup">Preparing backup…</string>
+    <!-- Linear progress dialog text shown when backup is paused because unmetered connectivity, such as WiFi, is unavailable. -->
+    <string name="RemoteBackupsSettingsFragment__Waiting_for_Wifi">Waiting for WiFi…</string>
+    <!-- Linear progress dialog text shown when backup is paused because internet is unavailable. -->
+    <string name="RemoteBackupsSettingsFragment__Waiting_for_internet_connection">Waiting for Internet connection…</string>
    <!-- Linear progress dialog text shown when processing messages for backup. First placeholder is completed count, second is approximate total count, third is percent completed. -->
    <plurals name="RemoteBackupsSettingsFragment__processing_messages_progress_text">
        <item quantity="one">Processing %1$s of %2$s message (%3$d%%)</item>
--- a/app/src/spinner/java/org/thoughtcrime/securesms/database/SignalStoreTransformer.kt
+++ b/app/src/spinner/java/org/thoughtcrime/securesms/database/SignalStoreTransformer.kt
@@ -6,12 +6,15 @@
 package org.thoughtcrime.securesms.database

 import android.database.Cursor
+import com.squareup.wire.ProtoAdapter
 import org.signal.core.util.requireBlob
 import org.signal.core.util.requireString
 import org.signal.spinner.ColumnTransformer
 import org.signal.spinner.DefaultColumnTransformer
 import org.thoughtcrime.securesms.database.model.databaseprotos.RestoreDecisionState
+import org.thoughtcrime.securesms.keyvalue.BackupValues
 import org.thoughtcrime.securesms.keyvalue.RegistrationValues
+import org.thoughtcrime.securesms.keyvalue.protos.ArchiveUploadProgressState

 /**
 * Transform non-user friendly store values into less-non-user friendly representations.
@@ -23,13 +26,13 @@ object SignalStoreTransformer : ColumnTransformer {

  override fun transform(tableName: String?, columnName: String, cursor: Cursor): String? {
    return when (cursor.requireString(KeyValueDatabase.KEY)) {
-      RegistrationValues.RESTORE_DECISION_STATE -> transformRestoreDecisionState(cursor)
+      RegistrationValues.RESTORE_DECISION_STATE -> decodeProto(cursor, RestoreDecisionState.ADAPTER)
+      BackupValues.KEY_ARCHIVE_UPLOAD_STATE -> decodeProto(cursor, ArchiveUploadProgressState.ADAPTER)
      else -> DefaultColumnTransformer.transform(tableName, columnName, cursor)
    }
  }

-  private fun transformRestoreDecisionState(cursor: Cursor): String? {
-    val restoreDecisionState = cursor.requireBlob(KeyValueDatabase.VALUE)?.let { RestoreDecisionState.ADAPTER.decode(it) }
-    return restoreDecisionState.toString()
+  private fun decodeProto(cursor: Cursor, adapter: ProtoAdapter<*>): String? {
+    return cursor.requireBlob(KeyValueDatabase.VALUE)?.let { adapter.decode(it) }?.toString()
  }
 }