From 8c31c83fc59155dd6e53ae25cd9f9474514da1a1 Mon Sep 17 00:00:00 2001 From: alansley Date: Wed, 15 May 2024 13:09:18 +1000 Subject: [PATCH] Fixes #1483 --- .../conversation/v2/ConversationActivityV2.kt | 23 +++++++++++++++---- .../securesms/database/MmsSmsDatabase.java | 7 +++++- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt index 84f43e014b..71ada31293 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt @@ -298,6 +298,13 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe private val reverseMessageList = false private val adapter by lazy { + + // To prevent repeated attachment download jobs being spawned we'll keep a set of what + // attachmentId / mmsId pairs we've already attempted to download and only spawn the job + // if we haven't already done so. Without this then when the retry limit for a failed job + // hits another job is immediately spawned (endlessly). + var alreadyAttemptedAttachmentDownloadPairs = mutableSetOf>() + val cursor = mmsSmsDb.getConversation(viewModel.threadId, reverseMessageList) val adapter = ConversationAdapter( this, @@ -325,9 +332,15 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe } }, onAttachmentNeedsDownload = { attachmentId, mmsId -> - // Start download (on IO thread) - lifecycleScope.launch(Dispatchers.IO) { - JobQueue.shared.add(AttachmentDownloadJob(attachmentId, mmsId)) + // Keep track of this specific attachment so we don't download it again + val pair = Pair(attachmentId, mmsId) + if (!alreadyAttemptedAttachmentDownloadPairs.contains(pair)) { + alreadyAttemptedAttachmentDownloadPairs.add(pair) + + // Start download (on IO thread) + lifecycleScope.launch(Dispatchers.IO) { + JobQueue.shared.add(AttachmentDownloadJob(attachmentId, mmsId)) + } } }, glide = glide, @@ -335,8 +348,8 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe ) adapter.visibleMessageViewDelegate = this - // Register an AdapterDataObserver to scroll us to the bottom of the RecyclerView if we're - // already near the the bottom and the data changes. + // Register an AdapterDataObserver to scroll us to the bottom of the RecyclerView for if + // we're already near the the bottom and the data changes. adapter.registerAdapterDataObserver(ConversationAdapterDataObserver(binding?.conversationRecyclerView!!, adapter)) adapter diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java index da4f39f0c1..e6685f3ec0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java @@ -305,7 +305,12 @@ public class MmsSmsDatabase extends Database { } String order = MmsSmsColumns.NORMALIZED_DATE_SENT + " DESC"; - String selection = MmsSmsColumns.THREAD_ID + " = " + threadId; + + // As the MmsSmsDatabase.ADDRESS column never contains the sender address we have to get creative to filter down all the + // messages that have been sent without interrogating each MessageRecord returned by the cursor. One way to do this is + // via the fact that the `ADDRESS_DEVICE_ID` is always null for incoming messages, but always has a value (such as 1) for + // outgoing messages - so we'll filter our query for only records with non-null ADDRESS_DEVICE_IDs in the current thread. + String selection = MmsSmsColumns.THREAD_ID + " = " + threadId + " AND " + MmsSmsColumns.ADDRESS_DEVICE_ID + " IS NOT NULL"; // Try everything with resources so that they auto-close on end of scope try (Cursor cursor = queryTables(PROJECTION, selection, order, null)) {