From 83f6640bd3caa9d6335bcb1fe2e8a8528537df81 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Thu, 16 Apr 2020 16:09:49 -0400 Subject: [PATCH] Add a more generic system for handling early messages. --- .../securesms/database/MmsDatabase.java | 14 ++--- .../securesms/database/MmsSmsDatabase.java | 14 +++-- .../securesms/database/SmsDatabase.java | 12 ++-- .../dependencies/ApplicationDependencies.java | 13 ++++ .../ApplicationDependencyProvider.java | 6 ++ .../securesms/jobs/PushProcessMessageJob.java | 28 ++++++--- .../securesms/jobs/PushTextSendJob.java | 3 + .../securesms/util/EarlyMessageCache.java | 60 +++++++++++++++++++ 8 files changed, 123 insertions(+), 27 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/util/EarlyMessageCache.java 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 54756b9b6d..b42b6e25f4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java @@ -231,7 +231,6 @@ public class MmsDatabase extends MessagingDatabase { private static final String OUTGOING_SECURE_MESSAGES_CLAUSE = "(" + MESSAGE_BOX + " & " + Types.BASE_TYPE_MASK + ") = " + Types.BASE_SENT_TYPE + " AND (" + MESSAGE_BOX + " & " + (Types.SECURE_MESSAGE_BIT | Types.PUSH_MESSAGE_BIT) + ")"; private final EarlyReceiptCache earlyDeliveryReceiptCache = new EarlyReceiptCache("MmsDelivery"); - private final EarlyReceiptCache earlyReadReceiptCache = new EarlyReceiptCache("MmsRead"); public MmsDatabase(Context context, SQLCipherOpenHelper databaseHelper) { super(context, databaseHelper); @@ -336,7 +335,7 @@ public class MmsDatabase extends MessagingDatabase { } } - public void incrementReceiptCount(SyncMessageId messageId, long timestamp, boolean deliveryReceipt, boolean readReceipt) { + public boolean incrementReceiptCount(SyncMessageId messageId, long timestamp, boolean deliveryReceipt) { SQLiteDatabase database = databaseHelper.getWritableDatabase(); Cursor cursor = null; boolean found = false; @@ -368,10 +367,12 @@ public class MmsDatabase extends MessagingDatabase { } } - if (!found) { - if (deliveryReceipt) earlyDeliveryReceiptCache.increment(messageId.getTimetamp(), messageId.getRecipientId()); - if (readReceipt) earlyReadReceiptCache.increment(messageId.getTimetamp(), messageId.getRecipientId()); + if (!found && deliveryReceipt) { + earlyDeliveryReceiptCache.increment(messageId.getTimetamp(), messageId.getRecipientId()); + return true; } + + return found; } finally { if (cursor != null) cursor.close(); @@ -1097,7 +1098,6 @@ public class MmsDatabase extends MessagingDatabase { } Map earlyDeliveryReceipts = earlyDeliveryReceiptCache.remove(message.getSentTimeMillis()); - Map earlyReadReceipts = earlyReadReceiptCache.remove(message.getSentTimeMillis()); ContentValues contentValues = new ContentValues(); contentValues.put(DATE_SENT, message.getSentTimeMillis()); @@ -1112,7 +1112,6 @@ public class MmsDatabase extends MessagingDatabase { contentValues.put(VIEW_ONCE, message.isViewOnce()); contentValues.put(RECIPIENT_ID, message.getRecipient().getId().serialize()); contentValues.put(DELIVERY_RECEIPT_COUNT, Stream.of(earlyDeliveryReceipts.values()).mapToLong(Long::longValue).sum()); - contentValues.put(READ_RECEIPT_COUNT, Stream.of(earlyReadReceipts.values()).mapToLong(Long::longValue).sum()); List quoteAttachments = new LinkedList<>(); @@ -1148,7 +1147,6 @@ public class MmsDatabase extends MessagingDatabase { receiptDatabase.insert(members, messageId, defaultReceiptStatus, message.getSentTimeMillis()); for (RecipientId recipientId : earlyDeliveryReceipts.keySet()) receiptDatabase.update(recipientId, messageId, GroupReceiptDatabase.STATUS_DELIVERED, -1); - for (RecipientId recipientId : earlyReadReceipts.keySet()) receiptDatabase.update(recipientId, messageId, GroupReceiptDatabase.STATUS_READ, -1); } DatabaseFactory.getThreadDatabase(context).setLastSeen(threadId); 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 0569106af7..8e16aadbe3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java @@ -269,13 +269,17 @@ public class MmsSmsDatabase extends Database { } public void incrementDeliveryReceiptCount(SyncMessageId syncMessageId, long timestamp) { - DatabaseFactory.getSmsDatabase(context).incrementReceiptCount(syncMessageId, true, false); - DatabaseFactory.getMmsDatabase(context).incrementReceiptCount(syncMessageId, timestamp, true, false); + DatabaseFactory.getSmsDatabase(context).incrementReceiptCount(syncMessageId, true); + DatabaseFactory.getMmsDatabase(context).incrementReceiptCount(syncMessageId, timestamp, true); } - public void incrementReadReceiptCount(SyncMessageId syncMessageId, long timestamp) { - DatabaseFactory.getSmsDatabase(context).incrementReceiptCount(syncMessageId, false, true); - DatabaseFactory.getMmsDatabase(context).incrementReceiptCount(syncMessageId, timestamp, false, true); + public boolean incrementReadReceiptCount(SyncMessageId syncMessageId, long timestamp) { + boolean handled = false; + + handled |= DatabaseFactory.getSmsDatabase(context).incrementReceiptCount(syncMessageId, false); + handled |= DatabaseFactory.getMmsDatabase(context).incrementReceiptCount(syncMessageId, timestamp, false); + + return handled; } public int getQuotedMessagePosition(long threadId, long quoteId, @NonNull RecipientId recipientId) { 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 74a6106aa9..2ebd763b10 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java @@ -134,7 +134,6 @@ public class SmsDatabase extends MessagingDatabase { private final String OUTGOING_SECURE_MESSAGE_CLAUSE = "(" + TYPE + " & " + Types.BASE_TYPE_MASK + ") = " + Types.BASE_SENT_TYPE + " AND (" + TYPE + " & " + (Types.SECURE_MESSAGE_BIT | Types.PUSH_MESSAGE_BIT) + ")"; private static final EarlyReceiptCache earlyDeliveryReceiptCache = new EarlyReceiptCache("SmsDelivery"); - private static final EarlyReceiptCache earlyReadReceiptCache = new EarlyReceiptCache("SmsRead"); public SmsDatabase(Context context, SQLCipherOpenHelper databaseHelper) { super(context, databaseHelper); @@ -395,7 +394,7 @@ public class SmsDatabase extends MessagingDatabase { database.update(TABLE_NAME, contentValues, ID_WHERE, new String[] {String.valueOf(id)}); } - public void incrementReceiptCount(SyncMessageId messageId, boolean deliveryReceipt, boolean readReceipt) { + public boolean incrementReceiptCount(SyncMessageId messageId, boolean deliveryReceipt) { SQLiteDatabase database = databaseHelper.getWritableDatabase(); Cursor cursor = null; boolean foundMessage = false; @@ -426,11 +425,12 @@ public class SmsDatabase extends MessagingDatabase { } } - if (!foundMessage) { - if (deliveryReceipt) earlyDeliveryReceiptCache.increment(messageId.getTimetamp(), messageId.getRecipientId()); - if (readReceipt) earlyReadReceiptCache.increment(messageId.getTimetamp(), messageId.getRecipientId()); + if (!foundMessage && deliveryReceipt) { + earlyDeliveryReceiptCache.increment(messageId.getTimetamp(), messageId.getRecipientId()); + return true; } + return foundMessage; } finally { if (cursor != null) cursor.close(); @@ -721,7 +721,6 @@ public class SmsDatabase extends MessagingDatabase { RecipientId recipientId = message.getRecipient().getId(); Map earlyDeliveryReceipts = earlyDeliveryReceiptCache.remove(date); - Map earlyReadReceipts = earlyReadReceiptCache.remove(date); ContentValues contentValues = new ContentValues(6); contentValues.put(RECIPIENT_ID, recipientId.serialize()); @@ -734,7 +733,6 @@ public class SmsDatabase extends MessagingDatabase { contentValues.put(SUBSCRIPTION_ID, message.getSubscriptionId()); contentValues.put(EXPIRES_IN, message.getExpiresIn()); contentValues.put(DELIVERY_RECEIPT_COUNT, Stream.of(earlyDeliveryReceipts.values()).mapToLong(Long::longValue).sum()); - contentValues.put(READ_RECEIPT_COUNT, Stream.of(earlyReadReceipts.values()).mapToLong(Long::longValue).sum()); SQLiteDatabase db = databaseHelper.getWritableDatabase(); long messageId = db.insert(TABLE_NAME, null, contentValues); diff --git a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java index 9f39970a13..aa820ecf18 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java +++ b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java @@ -13,6 +13,7 @@ import org.thoughtcrime.securesms.megaphone.MegaphoneRepository; import org.thoughtcrime.securesms.push.SignalServiceNetworkAccess; import org.thoughtcrime.securesms.recipients.LiveRecipientCache; import org.thoughtcrime.securesms.service.IncomingMessageObserver; +import org.thoughtcrime.securesms.util.EarlyMessageCache; import org.thoughtcrime.securesms.util.FeatureFlags; import org.thoughtcrime.securesms.util.FrameRateTracker; import org.thoughtcrime.securesms.util.IasKeyStore; @@ -47,6 +48,7 @@ public class ApplicationDependencies { private static KeyValueStore keyValueStore; private static MegaphoneRepository megaphoneRepository; private static GroupsV2Operations groupsV2Operations; + private static EarlyMessageCache earlyMessageCache; public static synchronized void init(@NonNull Application application, @NonNull Provider provider) { if (ApplicationDependencies.application != null || ApplicationDependencies.provider != null) { @@ -195,6 +197,16 @@ public class ApplicationDependencies { return megaphoneRepository; } + public static synchronized @NonNull EarlyMessageCache getEarlyMessageCache() { + assertInitialization(); + + if (earlyMessageCache == null) { + earlyMessageCache = provider.provideEarlyMessageCache(); + } + + return earlyMessageCache; + } + private static void assertInitialization() { if (application == null || provider == null) { throw new UninitializedException(); @@ -214,6 +226,7 @@ public class ApplicationDependencies { @NonNull FrameRateTracker provideFrameRateTracker(); @NonNull KeyValueStore provideKeyValueStore(); @NonNull MegaphoneRepository provideMegaphoneRepository(); + @NonNull EarlyMessageCache provideEarlyMessageCache(); } private static class UninitializedException extends IllegalStateException { diff --git a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java index deac052f4c..20210a72be 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java +++ b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java @@ -25,6 +25,7 @@ import org.thoughtcrime.securesms.push.SignalServiceNetworkAccess; import org.thoughtcrime.securesms.recipients.LiveRecipientCache; import org.thoughtcrime.securesms.service.IncomingMessageObserver; import org.thoughtcrime.securesms.util.AlarmSleepTimer; +import org.thoughtcrime.securesms.util.EarlyMessageCache; import org.thoughtcrime.securesms.util.FeatureFlags; import org.thoughtcrime.securesms.util.FrameRateTracker; import org.thoughtcrime.securesms.util.TextSecurePreferences; @@ -146,6 +147,11 @@ public class ApplicationDependencyProvider implements ApplicationDependencies.Pr return new MegaphoneRepository(context); } + @Override + public @NonNull EarlyMessageCache provideEarlyMessageCache() { + return new EarlyMessageCache(); + } + private static class DynamicCredentialsProvider implements CredentialsProvider { private final Context context; diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.java index 2246742e58..9ece2d59b8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.java @@ -237,8 +237,8 @@ public final class PushProcessMessageJob extends BaseJob { Optional optionalSmsMessageId = smsMessageId > 0 ? Optional.of(smsMessageId) : Optional.absent(); if (messageState == MessageState.DECRYPTED_OK) { - //noinspection ConstantConditions - handleMessage(serializedPlaintextContent, optionalSmsMessageId); + SignalServiceContent content = SignalServiceContent.deserialize(serializedPlaintextContent); + handleMessage(content, optionalSmsMessageId); } else { //noinspection ConstantConditions handleExceptionMessage(exceptionMetadata, optionalSmsMessageId); @@ -254,10 +254,9 @@ public final class PushProcessMessageJob extends BaseJob { public void onFailure() { } - private void handleMessage(@NonNull byte[] plaintextDataBuffer, @NonNull Optional smsMessageId) { + private void handleMessage(@Nullable SignalServiceContent content, @NonNull Optional smsMessageId) { try { - GroupDatabase groupDatabase = DatabaseFactory.getGroupDatabase(context); - SignalServiceContent content = SignalServiceContent.deserialize(plaintextDataBuffer); + GroupDatabase groupDatabase = DatabaseFactory.getGroupDatabase(context); if (content == null || shouldIgnore(content)) { Log.i(TAG, "Ignoring message."); @@ -333,6 +332,13 @@ public final class PushProcessMessageJob extends BaseJob { resetRecipientToPush(Recipient.externalPush(context, content.getSender())); + Optional earlyContent = ApplicationDependencies.getEarlyMessageCache() + .retrieve(Recipient.externalPush(context, content.getSender()).getId(), + content.getTimestamp()); + if (earlyContent.isPresent()) { + Log.i(TAG, "Found dependent content that was retrieved earlier. Processing."); + handleMessage(earlyContent.get(), Optional.absent()); + } } catch (StorageFailedException e) { Log.w(TAG, e); handleCorruptMessage(e.getSender(), e.getSenderDevice(), timestamp, smsMessageId); @@ -625,6 +631,7 @@ public final class PushProcessMessageJob extends BaseJob { Log.w(TAG, "[handleReaction] Found a matching message, but it's flagged as remotely deleted. timestamp: " + reaction.getTargetSentTimestamp() + " author: " + targetAuthor.getId()); } else { Log.w(TAG, "[handleReaction] Could not find matching message! timestamp: " + reaction.getTargetSentTimestamp() + " author: " + targetAuthor.getId()); + ApplicationDependencies.getEarlyMessageCache().store(targetAuthor.getId(), reaction.getTargetSentTimestamp(), content); } } @@ -640,6 +647,7 @@ public final class PushProcessMessageJob extends BaseJob { MessageNotifier.updateNotification(context, targetMessage.getThreadId(), false); } else if (targetMessage == null) { Log.w(TAG, "[handleRemoteDelete] Could not find matching message! timestamp: " + delete.getTargetSentTimestamp() + " author: " + sender.getId()); + ApplicationDependencies.getEarlyMessageCache().store(sender.getId(), delete.getTargetSentTimestamp(), content); } else { Log.w(TAG, String.format(Locale.ENGLISH, "[handleRemoteDelete] Invalid remote delete! deleteTime: %d, targetTime: %d, deleteAuthor: %s, targetAuthor: %s", content.getServerTimestamp(), targetMessage.getServerTimestamp(), sender.getId(), targetMessage.getRecipient().getId())); @@ -1339,8 +1347,14 @@ public final class PushProcessMessageJob extends BaseJob { for (long timestamp : message.getTimestamps()) { Log.i(TAG, String.format("Received encrypted read receipt: (XXXXX, %d)", timestamp)); - DatabaseFactory.getMmsSmsDatabase(context) - .incrementReadReceiptCount(new SyncMessageId(Recipient.externalPush(context, content.getSender()).getId(), timestamp), content.getTimestamp()); + Recipient sender = Recipient.externalPush(context, content.getSender()); + SyncMessageId id = new SyncMessageId(sender.getId(), timestamp); + boolean handled = DatabaseFactory.getMmsSmsDatabase(context) + .incrementReadReceiptCount(id, content.getTimestamp()); + + if (!handled) { + ApplicationDependencies.getEarlyMessageCache().store(sender.getId(), timestamp, content); + } } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushTextSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushTextSendJob.java index 0b3bc26c9e..6cdcf66d0c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushTextSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushTextSendJob.java @@ -26,12 +26,15 @@ import org.whispersystems.libsignal.util.guava.Optional; import org.whispersystems.signalservice.api.SignalServiceMessageSender; import org.whispersystems.signalservice.api.crypto.UnidentifiedAccessPair; import org.whispersystems.signalservice.api.crypto.UntrustedIdentityException; +import org.whispersystems.signalservice.api.messages.SendMessageResult; import org.whispersystems.signalservice.api.messages.SignalServiceDataMessage; +import org.whispersystems.signalservice.api.messages.SignalServiceGroup; import org.whispersystems.signalservice.api.messages.multidevice.SignalServiceSyncMessage; import org.whispersystems.signalservice.api.push.SignalServiceAddress; import org.whispersystems.signalservice.api.push.exceptions.UnregisteredUserException; import java.io.IOException; +import java.util.List; public class PushTextSendJob extends PushSendJob { diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/EarlyMessageCache.java b/app/src/main/java/org/thoughtcrime/securesms/util/EarlyMessageCache.java new file mode 100644 index 0000000000..db68d72f85 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/util/EarlyMessageCache.java @@ -0,0 +1,60 @@ +package org.thoughtcrime.securesms.util; + +import androidx.annotation.NonNull; + +import org.thoughtcrime.securesms.recipients.RecipientId; +import org.whispersystems.libsignal.util.guava.Optional; +import org.whispersystems.signalservice.api.messages.SignalServiceContent; + +import java.util.Objects; + +/** + * Sometimes a message that is referencing another message can arrive out of order. In these cases, + * we want to temporarily hold on (i.e. keep a memory cache) to these messages and apply them after + * we receive the referenced message. + */ +public final class EarlyMessageCache { + + private final LRUCache cache = new LRUCache<>(100); + + /** + * @param targetSender The sender of the message this message depends on. + * @param targetSentTimestamp The sent timestamp of the message this message depends on. + */ + public void store(@NonNull RecipientId targetSender, long targetSentTimestamp, @NonNull SignalServiceContent content) { + cache.put(new MessageId(targetSender, targetSentTimestamp), content); + } + + /** + * Returns and removes any content that is dependent on the provided message id. + * @param sender The sender of the message in question. + * @param sentTimestamp The sent timestamp of the message in question. + */ + public Optional retrieve(@NonNull RecipientId sender, long sentTimestamp) { + return Optional.fromNullable(cache.remove(new MessageId(sender, sentTimestamp))); + } + + private static final class MessageId { + private final RecipientId sender; + private final long sentTimestamp; + + private MessageId(@NonNull RecipientId sender, long sentTimestamp) { + this.sender = sender; + this.sentTimestamp = sentTimestamp; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + MessageId messageId = (MessageId) o; + return sentTimestamp == messageId.sentTimestamp && + Objects.equals(sender, messageId.sender); + } + + @Override + public int hashCode() { + return Objects.hash(sentTimestamp, sender); + } + } +}