From a42c3d7ce837516acda6c104f0aca3036f7b2272 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 2 Aug 2022 10:46:45 -0400 Subject: [PATCH] Fix handling of early receipts. We were storing the early content under the wrong recipient. --- .../securesms/database/MessageDatabase.java | 47 ++++++++++++++++++- .../securesms/database/MmsDatabase.java | 45 ------------------ .../securesms/database/MmsSmsDatabase.java | 18 +++---- .../securesms/database/SmsDatabase.java | 45 ------------------ .../messages/MessageContentProcessor.java | 31 +++++++----- 5 files changed, 74 insertions(+), 112 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessageDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/MessageDatabase.java index c9ead39ffd..0e0c1577d3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageDatabase.java @@ -45,6 +45,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Locale; import java.util.Map; @@ -128,7 +129,7 @@ public abstract class MessageDatabase extends Database implements MmsSmsColumns public abstract void markGiftRedemptionFailed(long messageId); public abstract Set incrementReceiptCount(SyncMessageId messageId, long timestamp, @NonNull ReceiptType receiptType, boolean storiesOnly); - abstract @NonNull MmsSmsDatabase.TimestampReadResult setTimestampRead(SyncMessageId messageId, long proposedExpireStarted, @NonNull Map threadToLatestRead); + public abstract List setEntireThreadRead(long threadId); public abstract List setMessagesReadSince(long threadId, long timestamp); public abstract List setAllMessagesRead(); @@ -276,6 +277,50 @@ public abstract class MessageDatabase extends Database implements MmsSmsColumns } } + /** + * Handles a synchronized read message. + * @param messageId An id representing the author-timestamp pair of the message that was read on a linked device. Note that the author could be self when + * syncing read receipts for reactions. + */ + final @NonNull MmsSmsDatabase.TimestampReadResult setTimestampReadFromSyncMessage(SyncMessageId messageId, long proposedExpireStarted, @NonNull Map threadToLatestRead) { + SQLiteDatabase database = databaseHelper.getSignalWritableDatabase(); + List> expiring = new LinkedList<>(); + String[] projection = new String[] { ID, THREAD_ID, EXPIRES_IN, EXPIRE_STARTED }; + String query = getDateSentColumnName() + " = ? AND (" + RECIPIENT_ID + " = ? OR (" + RECIPIENT_ID + " = ? AND " + getOutgoingTypeClause() + "))"; + String[] args = SqlUtil.buildArgs(messageId.getTimetamp(), messageId.getRecipientId(), Recipient.self().getId()); + List threads = new LinkedList<>(); + + try (Cursor cursor = database.query(getTableName(), projection, query, args, null, null, null)) { + while (cursor.moveToNext()) { + long id = cursor.getLong(cursor.getColumnIndexOrThrow(ID)); + long threadId = cursor.getLong(cursor.getColumnIndexOrThrow(THREAD_ID)); + long expiresIn = cursor.getLong(cursor.getColumnIndexOrThrow(EXPIRES_IN)); + long expireStarted = cursor.getLong(cursor.getColumnIndexOrThrow(EXPIRE_STARTED)); + + expireStarted = expireStarted > 0 ? Math.min(proposedExpireStarted, expireStarted) : proposedExpireStarted; + + ContentValues values = new ContentValues(); + values.put(READ, 1); + values.put(REACTIONS_UNREAD, 0); + values.put(REACTIONS_LAST_SEEN, System.currentTimeMillis()); + + if (expiresIn > 0) { + values.put(EXPIRE_STARTED, expireStarted); + expiring.add(new Pair<>(id, expiresIn)); + } + + database.update(getTableName(), values, ID_WHERE, SqlUtil.buildArgs(id)); + + threads.add(threadId); + + Long latest = threadToLatestRead.get(threadId); + threadToLatestRead.put(threadId, (latest != null) ? Math.max(latest, messageId.getTimetamp()) : messageId.getTimetamp()); + } + } + + return new MmsSmsDatabase.TimestampReadResult(expiring, threads); + } + private int getMessageCountForRecipientsAndType(String typeClause) { SQLiteDatabase db = databaseHelper.getSignalReadableDatabase(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java index aad20590d0..c9ebd6a808 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java @@ -1538,51 +1538,6 @@ public class MmsDatabase extends MessageDatabase { return result; } - @Override - @NonNull MmsSmsDatabase.TimestampReadResult setTimestampRead(SyncMessageId messageId, long proposedExpireStarted, @NonNull Map threadToLatestRead) { - SQLiteDatabase database = databaseHelper.getSignalWritableDatabase(); - List> expiring = new LinkedList<>(); - String[] projection = new String[] { ID, THREAD_ID, MESSAGE_BOX, EXPIRES_IN, EXPIRE_STARTED, RECIPIENT_ID }; - String query = DATE_SENT + " = ?"; - String[] args = SqlUtil.buildArgs(messageId.getTimetamp()); - List threads = new LinkedList<>(); - - try (Cursor cursor = database.query(TABLE_NAME, projection, query, args, null, null, null)) { - while (cursor.moveToNext()) { - RecipientId theirRecipientId = RecipientId.from(cursor.getLong(cursor.getColumnIndexOrThrow(RECIPIENT_ID))); - RecipientId ourRecipientId = messageId.getRecipientId(); - - if (ourRecipientId.equals(theirRecipientId) || Recipient.resolved(theirRecipientId).isGroup() || ourRecipientId.equals(Recipient.self().getId())) { - long id = cursor.getLong(cursor.getColumnIndexOrThrow(ID)); - long threadId = cursor.getLong(cursor.getColumnIndexOrThrow(THREAD_ID)); - long expiresIn = cursor.getLong(cursor.getColumnIndexOrThrow(EXPIRES_IN)); - long expireStarted = cursor.getLong(cursor.getColumnIndexOrThrow(EXPIRE_STARTED)); - - expireStarted = expireStarted > 0 ? Math.min(proposedExpireStarted, expireStarted) : proposedExpireStarted; - - ContentValues values = new ContentValues(); - values.put(READ, 1); - values.put(REACTIONS_UNREAD, 0); - values.put(REACTIONS_LAST_SEEN, System.currentTimeMillis()); - - if (expiresIn > 0) { - values.put(EXPIRE_STARTED, expireStarted); - expiring.add(new Pair<>(id, expiresIn)); - } - - database.update(TABLE_NAME, values, ID_WHERE, SqlUtil.buildArgs(id)); - - threads.add(threadId); - - Long latest = threadToLatestRead.get(threadId); - threadToLatestRead.put(threadId, (latest != null) ? Math.max(latest, messageId.getTimetamp()) : messageId.getTimetamp()); - } - } - } - - return new MmsSmsDatabase.TimestampReadResult(expiring, threads); - } - @Override public @Nullable Pair getOldestUnreadMentionDetails(long threadId) { SQLiteDatabase database = databaseHelper.getSignalReadableDatabase(); 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 6006ede7b0..29090adbaa 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java @@ -45,7 +45,6 @@ import java.io.Closeable; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -617,7 +616,7 @@ public class MmsSmsDatabase extends Database { /** * @return Unhandled ids */ - public Collection setTimestampRead(@NonNull Recipient senderRecipient, @NonNull List readMessages, long proposedExpireStarted, @NonNull Map threadToLatestRead) { + public Collection setTimestampReadFromSyncMessage(@NonNull List readMessages, long proposedExpireStarted, @NonNull Map threadToLatestRead) { SQLiteDatabase db = getWritableDatabase(); List> expiringText = new LinkedList<>(); @@ -628,12 +627,13 @@ public class MmsSmsDatabase extends Database { db.beginTransaction(); try { for (ReadMessage readMessage : readMessages) { - TimestampReadResult textResult = SignalDatabase.sms().setTimestampRead(new SyncMessageId(senderRecipient.getId(), readMessage.getTimestamp()), - proposedExpireStarted, - threadToLatestRead); - TimestampReadResult mediaResult = SignalDatabase.mms().setTimestampRead(new SyncMessageId(senderRecipient.getId(), readMessage.getTimestamp()), - proposedExpireStarted, - threadToLatestRead); + RecipientId authorId = Recipient.externalPush(readMessage.getSender()).getId(); + TimestampReadResult textResult = SignalDatabase.sms().setTimestampReadFromSyncMessage(new SyncMessageId(authorId, readMessage.getTimestamp()), + proposedExpireStarted, + threadToLatestRead); + TimestampReadResult mediaResult = SignalDatabase.mms().setTimestampReadFromSyncMessage(new SyncMessageId(authorId, readMessage.getTimestamp()), + proposedExpireStarted, + threadToLatestRead); expiringText.addAll(textResult.expiring); expiringMedia.addAll(mediaResult.expiring); @@ -642,7 +642,7 @@ public class MmsSmsDatabase extends Database { updatedThreads.addAll(mediaResult.threads); if (textResult.threads.isEmpty() && mediaResult.threads.isEmpty()) { - unhandled.add(new SyncMessageId(senderRecipient.getId(), readMessage.getTimestamp())); + unhandled.add(new SyncMessageId(authorId, readMessage.getTimestamp())); } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java index 42a329e0e3..5b49381a58 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java @@ -548,51 +548,6 @@ public class SmsDatabase extends MessageDatabase { } } - @Override - @NonNull MmsSmsDatabase.TimestampReadResult setTimestampRead(SyncMessageId messageId, long proposedExpireStarted, @NonNull Map threadToLatestRead) { - SQLiteDatabase database = databaseHelper.getSignalWritableDatabase(); - List> expiring = new LinkedList<>(); - String[] projection = new String[] {ID, THREAD_ID, RECIPIENT_ID, TYPE, EXPIRES_IN, EXPIRE_STARTED}; - String query = DATE_SENT + " = ?"; - String[] args = SqlUtil.buildArgs(messageId.getTimetamp()); - List threads = new LinkedList<>(); - - try (Cursor cursor = database.query(TABLE_NAME, projection, query, args, null, null, null)) { - while (cursor.moveToNext()) { - RecipientId theirRecipientId = messageId.getRecipientId(); - RecipientId outRecipientId = RecipientId.from(cursor.getLong(cursor.getColumnIndexOrThrow(RECIPIENT_ID))); - - if (outRecipientId.equals(theirRecipientId) || theirRecipientId.equals(Recipient.self().getId())) { - long id = cursor.getLong(cursor.getColumnIndexOrThrow(ID)); - long threadId = cursor.getLong(cursor.getColumnIndexOrThrow(THREAD_ID)); - long expiresIn = cursor.getLong(cursor.getColumnIndexOrThrow(EXPIRES_IN)); - long expireStarted = cursor.getLong(cursor.getColumnIndexOrThrow(EXPIRE_STARTED)); - - expireStarted = expireStarted > 0 ? Math.min(proposedExpireStarted, expireStarted) : proposedExpireStarted; - - ContentValues contentValues = new ContentValues(); - contentValues.put(READ, 1); - contentValues.put(REACTIONS_UNREAD, 0); - contentValues.put(REACTIONS_LAST_SEEN, System.currentTimeMillis()); - - if (expiresIn > 0) { - contentValues.put(EXPIRE_STARTED, expireStarted); - expiring.add(new Pair<>(id, expiresIn)); - } - - database.update(TABLE_NAME, contentValues, ID_WHERE, SqlUtil.buildArgs(id)); - - threads.add(threadId); - - Long latest = threadToLatestRead.get(threadId); - threadToLatestRead.put(threadId, (latest != null) ? Math.max(latest, messageId.getTimetamp()) : messageId.getTimetamp()); - } - } - } - - return new MmsSmsDatabase.TimestampReadResult(expiring, threads); - } - @Override public List setEntireThreadRead(long threadId) { return setMessagesRead(THREAD_ID + " = ?", new String[] {String.valueOf(threadId)}); diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.java b/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.java index 658e91fa8e..db84a6473d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.java +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.java @@ -338,7 +338,7 @@ public final class MessageContentProcessor { if (syncMessage.getSent().isPresent()) handleSynchronizeSentMessage(content, syncMessage.getSent().get(), senderRecipient); else if (syncMessage.getRequest().isPresent()) handleSynchronizeRequestMessage(syncMessage.getRequest().get(), content.getTimestamp()); - else if (syncMessage.getRead().isPresent()) handleSynchronizeReadMessage(content, syncMessage.getRead().get(), content.getTimestamp(), senderRecipient); + else if (syncMessage.getRead().isPresent()) handleSynchronizeReadMessage(content, syncMessage.getRead().get(), content.getTimestamp()); else if (syncMessage.getViewed().isPresent()) handleSynchronizeViewedMessage(syncMessage.getViewed().get(), content.getTimestamp()); else if (syncMessage.getViewOnceOpen().isPresent()) handleSynchronizeViewOnceOpenMessage(content, syncMessage.getViewOnceOpen().get(), content.getTimestamp()); else if (syncMessage.getVerified().isPresent()) handleSynchronizeVerifiedMessage(syncMessage.getVerified().get()); @@ -1279,14 +1279,13 @@ public final class MessageContentProcessor { private void handleSynchronizeReadMessage(@NonNull SignalServiceContent content, @NonNull List readMessages, - long envelopeTimestamp, - @NonNull Recipient senderRecipient) + long envelopeTimestamp) { log(envelopeTimestamp, "Synchronize read message. Count: " + readMessages.size() + ", Timestamps: " + Stream.of(readMessages).map(ReadMessage::getTimestamp).toList()); Map threadToLatestRead = new HashMap<>(); - Collection unhandled = SignalDatabase.mmsSms().setTimestampRead(senderRecipient, readMessages, envelopeTimestamp, threadToLatestRead); + Collection unhandled = SignalDatabase.mmsSms().setTimestampReadFromSyncMessage(readMessages, envelopeTimestamp, threadToLatestRead); List markedMessages = SignalDatabase.threads().setReadSince(threadToLatestRead, false); @@ -2521,10 +2520,14 @@ public final class MessageContentProcessor { SignalDatabase.mmsSms().updateViewedStories(handled); - for (SyncMessageId id : unhandled) { - warn(String.valueOf(content.getTimestamp()), "[handleViewedReceipt] Could not find matching message! timestamp: " + id.getTimetamp() + " author: " + id.getRecipientId()); - if (!processingEarlyContent) { - ApplicationDependencies.getEarlyMessageCache().store(id.getRecipientId(), id.getTimetamp(), content); + if (unhandled.size() > 0) { + RecipientId selfId = Recipient.self().getId(); + + for (SyncMessageId id : unhandled) { + warn(String.valueOf(content.getTimestamp()), "[handleViewedReceipt] Could not find matching message! timestamp: " + id.getTimetamp() + ", author: " + id.getRecipientId() + " | Receipt so associating with message from self (" + selfId + ")"); + if (!processingEarlyContent) { + ApplicationDependencies.getEarlyMessageCache().store(selfId, id.getTimetamp(), content); + } } } @@ -2576,10 +2579,14 @@ public final class MessageContentProcessor { Collection unhandled = SignalDatabase.mmsSms().incrementReadReceiptCounts(ids, content.getTimestamp()); - for (SyncMessageId id : unhandled) { - warn(String.valueOf(content.getTimestamp()), "[handleReadReceipt] Could not find matching message! timestamp: " + id.getTimetamp() + " author: " + id.getRecipientId()); - if (!processingEarlyContent) { - ApplicationDependencies.getEarlyMessageCache().store(id.getRecipientId(), id.getTimetamp(), content); + if (unhandled.size() > 0) { + RecipientId selfId = Recipient.self().getId(); + + for (SyncMessageId id : unhandled) { + warn(String.valueOf(content.getTimestamp()), "[handleReadReceipt] Could not find matching message! timestamp: " + id.getTimetamp() + ", author: " + id.getRecipientId() + " | Receipt, so associating with message from self (" + selfId + ")"); + if (!processingEarlyContent) { + ApplicationDependencies.getEarlyMessageCache().store(selfId, id.getTimetamp(), content); + } } }