From 6edfcfdc4e5ffb56efbdb8b558c3d8e25e4398ad Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Fri, 6 Jun 2025 14:22:53 -0400 Subject: [PATCH] Properly order attachment archive copies. --- .../securesms/backup/v2/BackupRepository.kt | 2 +- .../helpers/SignalDatabaseMigrations.kt | 2 +- ...chiveGarbageCollectionPendingConstraint.kt | 61 +++++++++++++++++++ .../ArchiveAttachmentReconciliationJob.kt | 14 ++++- .../jobs/ArchiveCommitAttachmentDeletesJob.kt | 2 +- .../jobs/CopyAttachmentToArchiveJob.kt | 29 +++++++-- .../securesms/jobs/JobManagerFactories.java | 31 +++++----- .../jobs/UploadAttachmentToArchiveJob.kt | 1 - .../securesms/keyvalue/BackupValues.kt | 13 ++++ app/src/main/protowire/JobData.proto | 1 + 10 files changed, 131 insertions(+), 25 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/jobmanager/impl/NoRemoteArchiveGarbageCollectionPendingConstraint.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt index 813b5126e4..59465a557a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt @@ -1466,7 +1466,7 @@ object BackupRepository { ) } - private suspend fun getPaidType(): MessageBackupsType.Paid? { + suspend fun getPaidType(): MessageBackupsType.Paid? { val productPrice: FiatMoney? = if (SignalStore.backup.backupTierInternalOverride == MessageBackupTier.PAID) { Log.d(TAG, "Accessing price via mock subscription.") RecurringInAppPaymentRepository.getActiveSubscriptionSync(InAppPaymentSubscriberRecord.Type.BACKUP).getOrNull()?.activeSubscription?.let { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt index a0abd96cfd..2a1601d468 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt @@ -131,8 +131,8 @@ import org.thoughtcrime.securesms.database.helpers.migration.V273_FixUnreadOrigi import org.thoughtcrime.securesms.database.helpers.migration.V274_BackupMediaSnapshotLastSeenOnRemote import org.thoughtcrime.securesms.database.helpers.migration.V275_EnsureDefaultAllChatsFolder import org.thoughtcrime.securesms.database.helpers.migration.V276_AttachmentCdnDefaultValueMigration -import org.thoughtcrime.securesms.database.helpers.migration.V278_BackupSnapshotTableVersions import org.thoughtcrime.securesms.database.helpers.migration.V277_AddNotificationProfileStorageSync +import org.thoughtcrime.securesms.database.helpers.migration.V278_BackupSnapshotTableVersions import org.thoughtcrime.securesms.database.SQLiteDatabase as SignalSqliteDatabase /** diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/impl/NoRemoteArchiveGarbageCollectionPendingConstraint.kt b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/impl/NoRemoteArchiveGarbageCollectionPendingConstraint.kt new file mode 100644 index 0000000000..3412b12b5f --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/impl/NoRemoteArchiveGarbageCollectionPendingConstraint.kt @@ -0,0 +1,61 @@ +/* + * Copyright 2025 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.jobmanager.impl + +import android.app.job.JobInfo +import org.thoughtcrime.securesms.jobmanager.Constraint +import org.thoughtcrime.securesms.jobmanager.ConstraintObserver +import org.thoughtcrime.securesms.keyvalue.SignalStore + +/** + * A constraint that is met so long as there is no remote storage garbage collection pending. + * "Remote storage garbage collection" refers to the process of cleaning up unused or orphaned media files from the remote archive storage. + * We won't be put into garbage collection mode unless we've received some indication from the server that we've run out of space. + * + * Use this constraint to prevent jobs that require remote storage from running until we've done everything we can to free up space. + */ +class NoRemoteArchiveGarbageCollectionPendingConstraint : Constraint { + + companion object { + const val KEY = "NoRemoteArchiveGarbageCollectionPendingConstraint" + } + + override fun isMet(): Boolean { + if (!SignalStore.backup.areBackupsEnabled) { + return true + } + + if (!SignalStore.backup.backsUpMedia) { + return true + } + + return !SignalStore.backup.remoteStorageGarbageCollectionPending + } + + override fun getFactoryKey(): String = KEY + + override fun applyToJobInfo(jobInfoBuilder: JobInfo.Builder) = Unit + + object Observer : ConstraintObserver { + val listeners: MutableSet = mutableSetOf() + + override fun register(notifier: ConstraintObserver.Notifier) { + listeners += notifier + } + + fun notifyListeners() { + for (listener in listeners) { + listener.onConstraintMet(KEY) + } + } + } + + class Factory : Constraint.Factory { + override fun create(): NoRemoteArchiveGarbageCollectionPendingConstraint { + return NoRemoteArchiveGarbageCollectionPendingConstraint() + } + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveAttachmentReconciliationJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveAttachmentReconciliationJob.kt index 0c575aa6f5..dbc8a9cc16 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveAttachmentReconciliationJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveAttachmentReconciliationJob.kt @@ -36,6 +36,7 @@ import kotlin.time.Duration.Companion.days class ArchiveAttachmentReconciliationJob private constructor( private var snapshotVersion: Long?, private var serverCursor: String?, + private val forced: Boolean, parameters: Parameters ) : Job(parameters) { @@ -48,9 +49,10 @@ class ArchiveAttachmentReconciliationJob private constructor( private const val CDN_FETCH_LIMIT = 10_000 } - constructor() : this( + constructor(forced: Boolean = false) : this( snapshotVersion = null, serverCursor = null, + forced = forced, parameters = Parameters.Builder() .addConstraint(NetworkConstraint.KEY) .setQueue(ArchiveCommitAttachmentDeletesJob.ARCHIVE_ATTACHMENT_QUEUE) @@ -60,13 +62,17 @@ class ArchiveAttachmentReconciliationJob private constructor( .build() ) - override fun serialize(): ByteArray = ArchiveAttachmentReconciliationJobData(snapshotVersion, serverCursor ?: "").encode() + override fun serialize(): ByteArray = ArchiveAttachmentReconciliationJobData( + snapshot = snapshotVersion, + serverCursor = serverCursor ?: "", + forced = forced + ).encode() override fun getFactoryKey(): String = KEY override fun run(): Result { val timeSinceLastSync = System.currentTimeMillis() - SignalStore.backup.lastAttachmentReconciliationTime - if (serverCursor == null && timeSinceLastSync > 0 && timeSinceLastSync < RemoteConfig.archiveReconciliationSyncInterval.inWholeMilliseconds) { + if (!forced && serverCursor == null && timeSinceLastSync > 0 && timeSinceLastSync < RemoteConfig.archiveReconciliationSyncInterval.inWholeMilliseconds) { Log.d(TAG, "No need to do a remote sync yet. Time since last sync: $timeSinceLastSync ms") return Result.success() } @@ -130,6 +136,7 @@ class ArchiveAttachmentReconciliationJob private constructor( Log.d(TAG, "No attachments need to be repaired.") } + SignalStore.backup.remoteStorageGarbageCollectionPending = false SignalStore.backup.lastAttachmentReconciliationTime = System.currentTimeMillis() return null @@ -209,6 +216,7 @@ class ArchiveAttachmentReconciliationJob private constructor( return ArchiveAttachmentReconciliationJob( snapshotVersion = data.snapshot, serverCursor = data.serverCursor.nullIfBlank(), + forced = data.forced, parameters = parameters ) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveCommitAttachmentDeletesJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveCommitAttachmentDeletesJob.kt index 756b9b4c80..203d617bc7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveCommitAttachmentDeletesJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/ArchiveCommitAttachmentDeletesJob.kt @@ -45,7 +45,7 @@ class ArchiveCommitAttachmentDeletesJob private constructor(parameters: Paramete when (val result = BackupRepository.deleteAbandonedMediaObjects(chunk)) { is NetworkResult.Success -> { - Log.i(tag, "Successfully deleted ${chunk.size} attachments off of the CDN.") + Log.i(tag, "Successfully deleted ${chunk.size} attachments off of the CDN. (Note: Count includes thumbnails)") } is NetworkResult.NetworkError -> { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/CopyAttachmentToArchiveJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/CopyAttachmentToArchiveJob.kt index 66d3f15638..b4873d8bdd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/CopyAttachmentToArchiveJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/CopyAttachmentToArchiveJob.kt @@ -1,6 +1,10 @@ package org.thoughtcrime.securesms.jobs +import kotlinx.coroutines.runBlocking +import org.signal.core.util.ByteSize +import org.signal.core.util.bytes import org.signal.core.util.logging.Log +import org.signal.core.util.logging.logW import org.signal.libsignal.zkgroup.VerificationFailedException import org.thoughtcrime.securesms.attachments.AttachmentId import org.thoughtcrime.securesms.attachments.Cdn @@ -12,6 +16,7 @@ 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.NoRemoteArchiveGarbageCollectionPendingConstraint import org.thoughtcrime.securesms.jobs.protos.CopyAttachmentToArchiveJobData import org.thoughtcrime.securesms.keyvalue.SignalStore import org.whispersystems.signalservice.api.NetworkResult @@ -40,6 +45,7 @@ class CopyAttachmentToArchiveJob private constructor(private val attachmentId: A attachmentId = attachmentId, parameters = Parameters.Builder() .addConstraint(NetworkConstraint.KEY) + .addConstraint(NoRemoteArchiveGarbageCollectionPendingConstraint.KEY) .setLifespan(TimeUnit.DAYS.toMillis(1)) .setMaxAttempts(Parameters.UNLIMITED) .setQueue(UploadAttachmentToArchiveJob.buildQueueKey()) @@ -110,8 +116,7 @@ class CopyAttachmentToArchiveJob private constructor(private val attachmentId: A is NetworkResult.StatusCodeError -> { when (archiveResult.code) { 403 -> { - // TODO [backup] What is the best way to handle this UX-wise? - Log.w(TAG, "[$attachmentId] Insufficient permissions to upload. Is the user no longer on media tier?") + Log.w(TAG, "[$attachmentId] Insufficient permissions to upload. Handled in parent handler.") Result.success() } 410 -> { @@ -121,9 +126,19 @@ class CopyAttachmentToArchiveJob private constructor(private val attachmentId: A Result.success() } 413 -> { - // TODO [backup] What is the best way to handle this UX-wise? Log.w(TAG, "[$attachmentId] Insufficient storage space! Can't upload!") - Result.success() + val remoteStorageQuota = getServerQuota() ?: return Result.retry(defaultBackoff()).logW(TAG, "[$attachmentId] Failed to fetch server quota! Retrying.") + + if (SignalDatabase.attachments.getEstimatedArchiveMediaSize() > remoteStorageQuota.inWholeBytes) { + // [TODO] Handle too much data case + return Result.failure() + } + + Log.i(TAG, "[$attachmentId] Remote storage is full, but our local state indicates that once we reconcile our storage, we should have enough. Enqueuing the reconciliation job and retrying.") + SignalStore.backup.remoteStorageGarbageCollectionPending = true + AppDependencies.jobManager.add(ArchiveAttachmentReconciliationJob(forced = true)) + + Result.retry(defaultBackoff()) } else -> { Log.w(TAG, "[$attachmentId] Got back a non-2xx status code: ${archiveResult.code}. Retrying.") @@ -159,6 +174,12 @@ class CopyAttachmentToArchiveJob private constructor(private val attachmentId: A return result } + private fun getServerQuota(): ByteSize? { + return runBlocking { + BackupRepository.getPaidType()?.storageAllowanceBytes?.bytes + } + } + override fun onFailure() { if (this.isCanceled) { Log.w(TAG, "[$attachmentId] Job was canceled, updating archive transfer state to ${AttachmentTable.ArchiveTransferState.COPY_PENDING}.") diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java index b530c6739d..fa7b4c3c16 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java @@ -9,6 +9,7 @@ import org.thoughtcrime.securesms.jobmanager.Constraint; import org.thoughtcrime.securesms.jobmanager.ConstraintObserver; import org.thoughtcrime.securesms.jobmanager.Job; import org.thoughtcrime.securesms.jobmanager.JobMigration; +import org.thoughtcrime.securesms.jobmanager.impl.NoRemoteArchiveGarbageCollectionPendingConstraint; import org.thoughtcrime.securesms.jobmanager.impl.AutoDownloadEmojiConstraint; import org.thoughtcrime.securesms.jobmanager.impl.BatteryNotLowConstraint; import org.thoughtcrime.securesms.jobmanager.impl.CellServiceConstraintObserver; @@ -397,19 +398,20 @@ public final class JobManagerFactories { public static Map getConstraintFactories(@NonNull Application application) { return new HashMap() {{ - put(AutoDownloadEmojiConstraint.KEY, new AutoDownloadEmojiConstraint.Factory(application)); - put(BatteryNotLowConstraint.KEY, new BatteryNotLowConstraint.Factory()); - put(ChangeNumberConstraint.KEY, new ChangeNumberConstraint.Factory()); - put(ChargingConstraint.KEY, new ChargingConstraint.Factory()); - put(DataRestoreConstraint.KEY, new DataRestoreConstraint.Factory()); - put(DecryptionsDrainedConstraint.KEY, new DecryptionsDrainedConstraint.Factory()); - put(NetworkConstraint.KEY, new NetworkConstraint.Factory(application)); - put(NetworkOrCellServiceConstraint.KEY, new NetworkOrCellServiceConstraint.Factory(application)); - put(NetworkOrCellServiceConstraint.LEGACY_KEY, new NetworkOrCellServiceConstraint.Factory(application)); - put(NotInCallConstraint.KEY, new NotInCallConstraint.Factory()); - put(SqlCipherMigrationConstraint.KEY, new SqlCipherMigrationConstraint.Factory(application)); - put(WifiConstraint.KEY, new WifiConstraint.Factory(application)); - put(RestoreAttachmentConstraint.KEY, new RestoreAttachmentConstraint.Factory(application)); + put(NoRemoteArchiveGarbageCollectionPendingConstraint.KEY, new NoRemoteArchiveGarbageCollectionPendingConstraint.Factory()); + put(AutoDownloadEmojiConstraint.KEY, new AutoDownloadEmojiConstraint.Factory(application)); + put(BatteryNotLowConstraint.KEY, new BatteryNotLowConstraint.Factory()); + put(ChangeNumberConstraint.KEY, new ChangeNumberConstraint.Factory()); + put(ChargingConstraint.KEY, new ChargingConstraint.Factory()); + put(DataRestoreConstraint.KEY, new DataRestoreConstraint.Factory()); + put(DecryptionsDrainedConstraint.KEY, new DecryptionsDrainedConstraint.Factory()); + put(NetworkConstraint.KEY, new NetworkConstraint.Factory(application)); + put(NetworkOrCellServiceConstraint.KEY, new NetworkOrCellServiceConstraint.Factory(application)); + put(NetworkOrCellServiceConstraint.LEGACY_KEY, new NetworkOrCellServiceConstraint.Factory(application)); + put(NotInCallConstraint.KEY, new NotInCallConstraint.Factory()); + put(SqlCipherMigrationConstraint.KEY, new SqlCipherMigrationConstraint.Factory(application)); + put(WifiConstraint.KEY, new WifiConstraint.Factory(application)); + put(RestoreAttachmentConstraint.KEY, new RestoreAttachmentConstraint.Factory(application)); }}; } @@ -422,7 +424,8 @@ public final class JobManagerFactories { new NotInCallConstraintObserver(), ChangeNumberConstraintObserver.INSTANCE, DataRestoreConstraintObserver.INSTANCE, - RestoreAttachmentConstraintObserver.INSTANCE); + RestoreAttachmentConstraintObserver.INSTANCE, + NoRemoteArchiveGarbageCollectionPendingConstraint.Observer.INSTANCE); } public static List getJobMigrations(@NonNull Application application) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/UploadAttachmentToArchiveJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/UploadAttachmentToArchiveJob.kt index cf88a93c48..82ae35f189 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/UploadAttachmentToArchiveJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/UploadAttachmentToArchiveJob.kt @@ -32,7 +32,6 @@ import java.io.FileNotFoundException import java.io.IOException import java.net.ProtocolException import kotlin.random.Random -import kotlin.random.nextInt import kotlin.time.Duration.Companion.days import kotlin.time.Duration.Companion.milliseconds diff --git a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/BackupValues.kt b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/BackupValues.kt index 65ced3cb97..2da9685222 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/BackupValues.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/BackupValues.kt @@ -8,6 +8,7 @@ import org.thoughtcrime.securesms.backup.DeletionState import org.thoughtcrime.securesms.backup.RestoreState import org.thoughtcrime.securesms.backup.v2.BackupFrequency import org.thoughtcrime.securesms.backup.v2.MessageBackupTier +import org.thoughtcrime.securesms.jobmanager.impl.NoRemoteArchiveGarbageCollectionPendingConstraint import org.thoughtcrime.securesms.jobmanager.impl.RestoreAttachmentConstraintObserver import org.thoughtcrime.securesms.keyvalue.protos.ArchiveUploadProgressState import org.thoughtcrime.securesms.util.RemoteConfig @@ -72,6 +73,7 @@ class BackupValues(store: KeyValueStore) : SignalStoreValues(store) { private const val KEY_USER_MANUALLY_SKIPPED_MEDIA_RESTORE = "backup.user.manually.skipped.media.restore" private const val KEY_BACKUP_EXPIRED_AND_DOWNGRADED = "backup.expired.and.downgraded" private const val KEY_BACKUP_DELETION_STATE = "backup.deletion.state" + private const val KEY_REMOTE_STORAGE_GARBAGE_COLLECTION_PENDING = "backup.remoteStorageGarbageCollectionPending" private const val KEY_MEDIA_ROOT_BACKUP_KEY = "backup.mediaRootBackupKey" @@ -285,6 +287,17 @@ class BackupValues(store: KeyValueStore) : SignalStoreValues(store) { /** Store that lets you interact with media ZK credentials. */ val mediaCredentials = CredentialStore(KEY_MEDIA_CREDENTIALS, KEY_MEDIA_CDN_READ_CREDENTIALS, KEY_MEDIA_CDN_READ_CREDENTIALS_TIMESTAMP) + /** + * If true, it means we have been told that remote storage is full, but we have not yet run any of our "garbage collection" tasks, like committing deletes + * or pruning orphaned media. + */ + var remoteStorageGarbageCollectionPending + get() = store.getBoolean(KEY_REMOTE_STORAGE_GARBAGE_COLLECTION_PENDING, false) + set(value) { + store.beginWrite().putBoolean(KEY_REMOTE_STORAGE_GARBAGE_COLLECTION_PENDING, value) + NoRemoteArchiveGarbageCollectionPendingConstraint.Observer.notifyListeners() + } + fun markMessageBackupFailure() { store.beginWrite() .putBoolean(KEY_BACKUP_FAIL, true) diff --git a/app/src/main/protowire/JobData.proto b/app/src/main/protowire/JobData.proto index d8a133e735..8b24330794 100644 --- a/app/src/main/protowire/JobData.proto +++ b/app/src/main/protowire/JobData.proto @@ -144,6 +144,7 @@ message UploadAttachmentToArchiveJobData { message ArchiveAttachmentReconciliationJobData { optional uint64 snapshot = 1; string serverCursor = 2; + bool forced = 3; } message DeviceNameChangeJobData {