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.
This commit is contained in:
Greyson Parrelli
2025-12-23 09:42:43 -05:00
committed by jeffrey-signal
parent 0b5fa18504
commit 3db8f5a2af
3 changed files with 30 additions and 24 deletions

View File

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

View File

@@ -961,12 +961,12 @@ object DataMessageProcessor {
val downloadJobs: List<AttachmentDownloadJob> = 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)

View File

@@ -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<AttachmentDownloadJob> = 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<AttachmentDownloadJob> = 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<AttachmentDownloadJob> = attachments.map { AttachmentDownloadJob(messageId, it.attachmentId, false) }
for (attachment in attachments) {
AppDependencies.jobManager.addAll(downloadJobs)
}
val downloadJobs: List<AttachmentDownloadJob> = attachments.map { AttachmentDownloadJob(messageId = messageId, attachmentId = it.attachmentId, forceDownload = it.isSticker) }
AppDependencies.jobManager.addAll(downloadJobs)
}
return threadId