From 3db8f5a2af27e2ecf63417ccf3615ee126e7f4f5 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 23 Dec 2025 09:42:43 -0500 Subject: [PATCH] Ensure that attachments received during calls are autodownloaded afterwards. The easiest way to do this is to add the constraint to the job itself, when appropriate, so that we don't even run the job until you're done with the call. --- .../securesms/jobs/AttachmentDownloadJob.kt | 34 ++++++++++++------- .../messages/DataMessageProcessor.kt | 4 +-- .../messages/SyncMessageProcessor.kt | 16 ++++----- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt index 50bb8d12fd..75c27f3d45 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt @@ -23,9 +23,11 @@ import org.thoughtcrime.securesms.database.SignalDatabase import org.thoughtcrime.securesms.dependencies.AppDependencies import org.thoughtcrime.securesms.events.PartProgressEvent import org.thoughtcrime.securesms.jobmanager.Job +import org.thoughtcrime.securesms.jobmanager.Job.Parameters import org.thoughtcrime.securesms.jobmanager.JobLogger.format import org.thoughtcrime.securesms.jobmanager.JsonJobData import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint +import org.thoughtcrime.securesms.jobmanager.impl.NotInCallConstraint import org.thoughtcrime.securesms.jobmanager.persistence.JobSpec import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.mms.MmsException @@ -56,7 +58,7 @@ class AttachmentDownloadJob private constructor( parameters: Parameters, private val messageId: Long, private val attachmentId: AttachmentId, - private val manual: Boolean + private val forceDownload: Boolean ) : BaseJob(parameters) { companion object { @@ -65,7 +67,7 @@ class AttachmentDownloadJob private constructor( private const val KEY_MESSAGE_ID = "message_id" private const val KEY_ATTACHMENT_ID = "part_row_id" - private const val KEY_MANUAL = "part_manual" + private const val KEY_FORCE_DOWNLOAD = "part_manual" @JvmStatic fun constructQueueString(attachmentId: AttachmentId): String { @@ -107,7 +109,7 @@ class AttachmentDownloadJob private constructor( val downloadJob = AttachmentDownloadJob( messageId = databaseAttachment.mmsId, attachmentId = databaseAttachment.attachmentId, - manual = true + forceDownload = true ) AppDependencies.jobManager.add(downloadJob) downloadJob.id @@ -128,23 +130,24 @@ class AttachmentDownloadJob private constructor( } } - constructor(messageId: Long, attachmentId: AttachmentId, manual: Boolean) : this( + constructor(messageId: Long, attachmentId: AttachmentId, forceDownload: Boolean) : this( Parameters.Builder() .setQueue(constructQueueString(attachmentId)) .addConstraint(NetworkConstraint.KEY) + .maybeApplyNotInCallConstraint(forceDownload) .setLifespan(TimeUnit.DAYS.toMillis(1)) .setMaxAttempts(Parameters.UNLIMITED) .build(), messageId, attachmentId, - manual + forceDownload ) override fun serialize(): ByteArray? { return JsonJobData.Builder() .putLong(KEY_MESSAGE_ID, messageId) .putLong(KEY_ATTACHMENT_ID, attachmentId.id) - .putBoolean(KEY_MANUAL, manual) + .putBoolean(KEY_FORCE_DOWNLOAD, forceDownload) .serialize() } @@ -153,12 +156,12 @@ class AttachmentDownloadJob private constructor( } override fun onAdded() { - Log.i(TAG, "onAdded() messageId: $messageId attachmentId: $attachmentId manual: $manual") + Log.i(TAG, "onAdded() messageId: $messageId attachmentId: $attachmentId manual: $forceDownload") val attachment = SignalDatabase.attachments.getAttachment(attachmentId) val pending = attachment != null && attachment.transferState != AttachmentTable.TRANSFER_PROGRESS_DONE && attachment.transferState != AttachmentTable.TRANSFER_PROGRESS_PERMANENT_FAILURE - if (pending && (manual || AttachmentUtil.isAutoDownloadPermitted(context, attachment))) { + if (pending && (forceDownload || AttachmentUtil.isAutoDownloadPermitted(context, attachment))) { Log.i(TAG, "onAdded() Marking attachment progress as 'started'") SignalDatabase.attachments.setTransferState(messageId, attachmentId, AttachmentTable.TRANSFER_PROGRESS_STARTED) } @@ -175,7 +178,7 @@ class AttachmentDownloadJob private constructor( @Throws(IOException::class, RetryLaterException::class) fun doWork() { - Log.i(TAG, "onRun() messageId: $messageId attachmentId: $attachmentId manual: $manual") + Log.i(TAG, "onRun() messageId: $messageId attachmentId: $attachmentId manual: $forceDownload") val attachment = SignalDatabase.attachments.getAttachment(attachmentId) @@ -194,7 +197,7 @@ class AttachmentDownloadJob private constructor( return } - if (!manual && !AttachmentUtil.isAutoDownloadPermitted(context, attachment)) { + if (!forceDownload && !AttachmentUtil.isAutoDownloadPermitted(context, attachment)) { Log.w(TAG, "Attachment can't be auto downloaded...") SignalDatabase.attachments.setTransferState(messageId, attachmentId, AttachmentTable.TRANSFER_PROGRESS_PENDING) return @@ -256,7 +259,7 @@ class AttachmentDownloadJob private constructor( } override fun onFailure() { - Log.w(TAG, format(this, "onFailure() messageId: $messageId attachmentId: $attachmentId manual: $manual")) + Log.w(TAG, format(this, "onFailure() messageId: $messageId attachmentId: $attachmentId manual: $forceDownload")) markFailed(messageId, attachmentId) } @@ -450,8 +453,15 @@ class AttachmentDownloadJob private constructor( parameters = parameters, messageId = data.getLong(KEY_MESSAGE_ID), attachmentId = AttachmentId(data.getLong(KEY_ATTACHMENT_ID)), - manual = data.getBoolean(KEY_MANUAL) + forceDownload = data.getBoolean(KEY_FORCE_DOWNLOAD) ) } } } + +private fun Parameters.Builder.maybeApplyNotInCallConstraint(forceDownload: Boolean): Parameters.Builder { + if (forceDownload) { + return this + } + return this.addConstraint(NotInCallConstraint.KEY) +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt index a786c511b8..1ef6ae4404 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt @@ -961,12 +961,12 @@ object DataMessageProcessor { val downloadJobs: List = insertResult.insertedAttachments.mapNotNull { (attachment, attachmentId) -> if (attachment.isSticker) { if (attachment.transferState != AttachmentTable.TRANSFER_PROGRESS_DONE) { - AttachmentDownloadJob(insertResult.messageId, attachmentId, true) + AttachmentDownloadJob(messageId = insertResult.messageId, attachmentId = attachmentId, forceDownload = true) } else { null } } else { - AttachmentDownloadJob(insertResult.messageId, attachmentId, false) + AttachmentDownloadJob(messageId = insertResult.messageId, attachmentId = attachmentId, forceDownload = false) } } AppDependencies.jobManager.addAll(downloadJobs) diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/SyncMessageProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/SyncMessageProcessor.kt index c3429485f7..a1d13e83c7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/SyncMessageProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/SyncMessageProcessor.kt @@ -496,9 +496,8 @@ object SyncMessageProcessor { if (syncAttachments.isNotEmpty()) { SignalDatabase.runPostSuccessfulTransaction { - for (attachment in attachments) { - AppDependencies.jobManager.add(AttachmentDownloadJob(messageId, attachment.attachmentId, false)) - } + val downloadJobs: List = attachments.map { AttachmentDownloadJob(messageId = messageId, attachmentId = it.attachmentId, forceDownload = it.isSticker) } + AppDependencies.jobManager.addAll(downloadJobs) } } } @@ -596,9 +595,8 @@ object SyncMessageProcessor { } SignalDatabase.runPostSuccessfulTransaction { - for (attachment in attachments) { - AppDependencies.jobManager.add(AttachmentDownloadJob(messageId, attachment.attachmentId, false)) - } + val downloadJobs: List = attachments.map { AttachmentDownloadJob(messageId = messageId, attachmentId = it.attachmentId, forceDownload = false) } + AppDependencies.jobManager.addAll(downloadJobs) } } @@ -893,10 +891,8 @@ object SyncMessageProcessor { } SignalDatabase.runPostSuccessfulTransaction { - val downloadJobs: List = attachments.map { AttachmentDownloadJob(messageId, it.attachmentId, false) } - for (attachment in attachments) { - AppDependencies.jobManager.addAll(downloadJobs) - } + val downloadJobs: List = attachments.map { AttachmentDownloadJob(messageId = messageId, attachmentId = it.attachmentId, forceDownload = it.isSticker) } + AppDependencies.jobManager.addAll(downloadJobs) } return threadId