Addressed PR feedback

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt

@@ -299,11 +299,11 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
 
     private val adapter by lazy {
 
-        // To prevent repeated attachment download jobs being spawned we'll keep a set of what
+        // To prevent repeated attachment download jobs being spawned we'll keep track of the
-        // attachmentId / mmsId pairs we've already attempted to download and only spawn the job
+        // attachment Ids we've attempted to download, and only spawn job if we haven't already
-        // if we haven't already done so. Without this then when the retry limit for a failed job
+        // tried. Without this then when the retry limit for a failed job hits another job is
-        // hits another job is immediately spawned (endlessly).
+        // immediately spawned (endlessly).
-        var alreadyAttemptedAttachmentDownloadPairs = mutableSetOf<Pair<Long, Long>>()
+        val alreadyAttemptedAttachmentDownloads = mutableSetOf<Long>()
 
         val cursor = mmsSmsDb.getConversation(viewModel.threadId, reverseMessageList)
         val adapter = ConversationAdapter(
@@ -332,12 +332,9 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
                 }
             },
             onAttachmentNeedsDownload = { 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)
+                alreadyAttemptedAttachmentDownloads.takeUnless { attachmentId in alreadyAttemptedAttachmentDownloads }.let {
+                    alreadyAttemptedAttachmentDownloads += attachmentId
                     lifecycleScope.launch(Dispatchers.IO) {
                         JobQueue.shared.add(AttachmentDownloadJob(attachmentId, mmsId))
                     }

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationAdapter.kt

@@ -139,8 +139,7 @@ class ConversationAdapter(
                         senderId,
                         lastSeen.get(),
                         visibleMessageViewDelegate,
-                        onAttachmentNeedsDownload,
+                        onAttachmentNeedsDownload
-                        lastSentMessageId
                 )
 
                 if (!message.isDeleted) {
@@ -216,8 +215,7 @@ class ConversationAdapter(
         if (cursorHasContent) {
             val thisThreadId = cursor.getLong(4) // Column index 4 is "thread_id"
             if (thisThreadId != -1L) {
-                val thisUsersSessionId = TextSecurePreferences.getLocalNumber(context)
+                return messageDB.getLastOutgoingMessage(thisThreadId)
-                return messageDB.getLastSentMessageFromSender(thisThreadId, thisUsersSessionId)
             }
         }
         return -1L

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt

@@ -134,8 +134,7 @@ class VisibleMessageView : LinearLayout {
         senderSessionID: String,
         lastSeen: Long,
         delegate: VisibleMessageViewDelegate? = null,
-        onAttachmentNeedsDownload: (Long, Long) -> Unit,
+        onAttachmentNeedsDownload: (Long, Long) -> Unit
-        lastSentMessageId: Long
     ) {
         replyDisabled = message.isOpenGroupInvitation
         val threadID = message.threadId
@@ -303,8 +302,7 @@ class VisibleMessageView : LinearLayout {
 
         // --- If we got here then we know the message is outgoing ---
 
-        val thisUsersSessionId = TextSecurePreferences.getLocalNumber(context)
+        val lastSentMessageId = mmsSmsDb.getLastOutgoingMessage(message.threadId)
-        val lastSentMessageId = mmsSmsDb.getLastSentMessageFromSender(message.threadId, thisUsersSessionId)
         val isLastSentMessage = lastSentMessageId == message.id
 
         // ----- Case ii.) Message is outgoing but NOT scheduled to disappear -----

app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsColumns.java

@@ -9,7 +9,11 @@ public interface MmsSmsColumns {
   public static final String THREAD_ID                = "thread_id";
   public static final String READ                     = "read";
   public static final String BODY                     = "body";
+
+  // This is the address of the message recipient, which may be a single user, a group, or a community!
+  // It is NOT the address of the sender of any given message!
   public static final String ADDRESS                  = "address";
+
   public static final String ADDRESS_DEVICE_ID        = "address_device_id";
   public static final String DELIVERY_RECEIPT_COUNT   = "delivery_receipt_count";
   public static final String READ_RECEIPT_COUNT       = "read_receipt_count";

app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java

@@ -295,29 +295,19 @@ public class MmsSmsDatabase extends Database {
     return identifiedMessages;
   }
 
-  public long getLastSentMessageFromSender(long threadId, String serializedAuthor) {
+  public long getLastOutgoingMessage(long threadId) {
-
-    // Early exit
-    boolean isOwnNumber = Util.isOwnNumber(context, serializedAuthor);
-    if (!isOwnNumber) {
-      Log.i(TAG, "Asked to find last sent message but sender isn't us - returning null.");
-      return -1;
-    }
-
     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
+    // Try everything with resources so that they auto-close on end of scope.
-    // messages that have been sent without interrogating each MessageRecord returned by the cursor. One way to do this is
+    // Note: Do NOT call cursor.moveToFirst() once we have it, for reasons unknown to me it breaks the functionality. -AL
-    // 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)) {
       try (MmsSmsDatabase.Reader reader = readerFor(cursor)) {
         MessageRecord messageRecord;
         while ((messageRecord = reader.getNext()) != null) {
-          if (messageRecord.isOutgoing()) { return messageRecord.id; }
+          // Note: We rely on the message order to get us the most recent outgoing message - so we
+          // take the first outgoing message we find.
+          if (messageRecord.isOutgoing()) return messageRecord.id;
         }
       }
     }