From 62040d06b40ebcc2a0fc01eaafee1ff7e7dd1fe6 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Thu, 1 Jul 2021 12:56:25 -0400 Subject: [PATCH] Create a write-through cache for PendingRetryReceiptDatabase. --- .../securesms/database/GroupDatabase.java | 14 ++++ .../database/PendingRetryReceiptCache.kt | 78 +++++++++++++++++++ .../database/PendingRetryReceiptDatabase.java | 40 ++++------ .../dependencies/ApplicationDependencies.java | 17 +++- .../ApplicationDependencyProvider.java | 6 ++ .../securesms/jobs/ResendMessageJob.java | 13 ++++ .../messages/MessageContentProcessor.java | 4 +- .../messages/MessageDecryptionUtil.java | 2 +- .../service/PendingRetryReceiptManager.java | 12 +-- 9 files changed, 152 insertions(+), 34 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/database/PendingRetryReceiptCache.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java index 06bcc49be..99fccef88 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java @@ -172,6 +172,20 @@ private static final String[] GROUP_PROJECTION = { } } + public Optional getGroupByDistributionId(@NonNull DistributionId distributionId) { + SQLiteDatabase db = databaseHelper.getReadableDatabase(); + String query = DISTRIBUTION_ID + " = ?"; + String[] args = SqlUtil.buildArgs(distributionId); + + try (Cursor cursor = db.query(TABLE_NAME, null, query, args, null, null, null)) { + if (cursor.moveToFirst()) { + return getGroup(cursor); + } else { + return Optional.absent(); + } + } + } + /** * Removes the specified members from the list of 'unmigrated V1 members' -- the list of members * that were either dropped or had to be invited when migrating the group from V1->V2. diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/PendingRetryReceiptCache.kt b/app/src/main/java/org/thoughtcrime/securesms/database/PendingRetryReceiptCache.kt new file mode 100644 index 000000000..d91dce0ca --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/database/PendingRetryReceiptCache.kt @@ -0,0 +1,78 @@ +package org.thoughtcrime.securesms.database + +import android.content.Context +import androidx.annotation.VisibleForTesting +import org.thoughtcrime.securesms.database.model.PendingRetryReceiptModel +import org.thoughtcrime.securesms.recipients.RecipientId +import org.thoughtcrime.securesms.util.FeatureFlags + +/** + * A write-through cache for [PendingRetryReceiptDatabase]. + * + * We have to read from this cache every time we process an incoming message. As a result, it's a very performance-sensitive operation. + * + * This cache is very similar to our job storage cache or our key-value store, in the sense that the first access of it will fetch all data from disk so all + * future reads can happen in memory. + */ +class PendingRetryReceiptCache @VisibleForTesting constructor( + private val database: PendingRetryReceiptDatabase +) { + constructor(context: Context) : this(DatabaseFactory.getPendingRetryReceiptDatabase(context)) + + private val pendingRetries: MutableMap = HashMap() + private var populated: Boolean = false + + fun insert(author: RecipientId, authorDevice: Int, sentTimestamp: Long, receivedTimestamp: Long, threadId: Long) { + if (!FeatureFlags.senderKey()) return + ensurePopulated() + + synchronized(pendingRetries) { + val model: PendingRetryReceiptModel = database.insert(author, authorDevice, sentTimestamp, receivedTimestamp, threadId) + pendingRetries[RemoteMessageId(author, sentTimestamp)] = model + } + } + + fun get(author: RecipientId, sentTimestamp: Long): PendingRetryReceiptModel? { + if (!FeatureFlags.senderKey()) return null + ensurePopulated() + + synchronized(pendingRetries) { + return pendingRetries[RemoteMessageId(author, sentTimestamp)] + } + } + + fun getOldest(): PendingRetryReceiptModel? { + if (!FeatureFlags.senderKey()) return null + ensurePopulated() + + synchronized(pendingRetries) { + return pendingRetries.values.minByOrNull { it.receivedTimestamp } + } + } + + fun delete(model: PendingRetryReceiptModel) { + if (!FeatureFlags.senderKey()) return + ensurePopulated() + + synchronized(pendingRetries) { + pendingRetries.remove(RemoteMessageId(model.author, model.sentTimestamp)) + database.delete(model) + } + } + + private fun ensurePopulated() { + if (!populated) { + synchronized(pendingRetries) { + if (!populated) { + database.all.forEach { model -> + pendingRetries[RemoteMessageId(model.author, model.sentTimestamp)] = model + } + + populated = true + } + } + } + } + + data class RemoteMessageId(val author: RecipientId, val sentTimestamp: Long) +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/PendingRetryReceiptDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/PendingRetryReceiptDatabase.java index 1630e8cca..f5e0de0e7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/PendingRetryReceiptDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/PendingRetryReceiptDatabase.java @@ -5,7 +5,6 @@ import android.content.Context; import android.database.Cursor; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import org.thoughtcrime.securesms.database.helpers.SQLCipherOpenHelper; import org.thoughtcrime.securesms.database.model.PendingRetryReceiptModel; @@ -13,8 +12,13 @@ import org.thoughtcrime.securesms.recipients.RecipientId; import org.thoughtcrime.securesms.util.CursorUtil; import org.thoughtcrime.securesms.util.SqlUtil; +import java.util.LinkedList; +import java.util.List; + /** * Holds information about messages we've sent out retry receipts for. + * + * Do not use directly! The only class that should be accessing this is {@link PendingRetryReceiptCache} */ public final class PendingRetryReceiptDatabase extends Database { @@ -39,7 +43,7 @@ public final class PendingRetryReceiptDatabase extends Database { super(context, databaseHelper); } - public void insert(@NonNull RecipientId author, int authorDevice, long sentTimestamp, long receivedTimestamp, long threadId) { + @NonNull PendingRetryReceiptModel insert(@NonNull RecipientId author, int authorDevice, long sentTimestamp, long receivedTimestamp, long threadId) { ContentValues values = new ContentValues(); values.put(AUTHOR, author.serialize()); values.put(DEVICE, authorDevice); @@ -47,37 +51,27 @@ public final class PendingRetryReceiptDatabase extends Database { values.put(RECEIVED_TIMESTAMP, receivedTimestamp); values.put(THREAD_ID, threadId); - databaseHelper.getWritableDatabase().insertWithOnConflict(TABLE_NAME, null, values, SQLiteDatabase.CONFLICT_REPLACE); + long id = databaseHelper.getWritableDatabase().insertWithOnConflict(TABLE_NAME, null, values, SQLiteDatabase.CONFLICT_REPLACE); + + return new PendingRetryReceiptModel(id, author, authorDevice, sentTimestamp, receivedTimestamp, threadId); } - public @Nullable PendingRetryReceiptModel get(@NonNull RecipientId author, long sentTimestamp) { - String query = AUTHOR + " = ? AND " + SENT_TIMESTAMP + " = ?"; - String[] args = SqlUtil.buildArgs(author, sentTimestamp); + @NonNull List getAll() { + List models = new LinkedList<>(); - try (Cursor cursor = databaseHelper.getReadableDatabase().query(TABLE_NAME, null, query, args, null, null, null)) { - if (cursor.moveToFirst()) { - return fromCursor(cursor); + try (Cursor cursor = databaseHelper.getReadableDatabase().query(TABLE_NAME, null, null, null, null, null, null)) { + while (cursor.moveToNext()) { + models.add(fromCursor(cursor)); } } - return null; + return models; } - public @Nullable PendingRetryReceiptModel getOldest() { - try (Cursor cursor = databaseHelper.getReadableDatabase().query(TABLE_NAME, null, null, null, null, null, RECEIVED_TIMESTAMP + " ASC", "1")) { - if (cursor.moveToFirst()) { - return fromCursor(cursor); - } - } - - return null; + void delete(@NonNull PendingRetryReceiptModel model) { + databaseHelper.getWritableDatabase().delete(TABLE_NAME, ID_WHERE, SqlUtil.buildArgs(model.getId())); } - public void delete(long id) { - databaseHelper.getWritableDatabase().delete(TABLE_NAME, ID_WHERE, SqlUtil.buildArgs(id)); - } - - private static @NonNull PendingRetryReceiptModel fromCursor(@NonNull Cursor cursor) { return new PendingRetryReceiptModel(CursorUtil.requireLong(cursor, ID), RecipientId.from(CursorUtil.requireString(cursor, AUTHOR)), 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 2023975bb..2210b83d4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java +++ b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java @@ -9,7 +9,6 @@ import org.thoughtcrime.securesms.KbsEnclave; import org.thoughtcrime.securesms.components.TypingStatusRepository; import org.thoughtcrime.securesms.components.TypingStatusSender; import org.thoughtcrime.securesms.database.DatabaseObserver; -import org.thoughtcrime.securesms.database.model.PendingRetryReceiptModel; import org.thoughtcrime.securesms.groups.GroupsV2Authorization; import org.thoughtcrime.securesms.groups.GroupsV2AuthorizationMemoryValueCache; import org.thoughtcrime.securesms.groups.v2.processing.GroupsV2StateProcessor; @@ -19,7 +18,7 @@ import org.thoughtcrime.securesms.megaphone.MegaphoneRepository; import org.thoughtcrime.securesms.messages.BackgroundMessageRetriever; import org.thoughtcrime.securesms.messages.IncomingMessageObserver; import org.thoughtcrime.securesms.messages.IncomingMessageProcessor; -import org.thoughtcrime.securesms.net.ContentProxySelector; +import org.thoughtcrime.securesms.database.PendingRetryReceiptCache; import org.thoughtcrime.securesms.net.PipeConnectivityListener; import org.thoughtcrime.securesms.net.StandardUserAgentInterceptor; import org.thoughtcrime.securesms.notifications.MessageNotifier; @@ -90,6 +89,7 @@ public class ApplicationDependencies { private static volatile ShakeToReport shakeToReport; private static volatile OkHttpClient okHttpClient; private static volatile PendingRetryReceiptManager pendingRetryReceiptManager; + private static volatile PendingRetryReceiptCache pendingRetryReceiptCache; @MainThread public static void init(@NonNull Application application, @NonNull Provider provider) { @@ -480,6 +480,18 @@ public class ApplicationDependencies { return appForegroundObserver; } + public static @NonNull PendingRetryReceiptCache getPendingRetryReceiptCache() { + if (pendingRetryReceiptCache == null) { + synchronized (LOCK) { + if (pendingRetryReceiptCache == null) { + pendingRetryReceiptCache = provider.providePendingRetryReceiptCache(); + } + } + } + + return pendingRetryReceiptCache; + } + public interface Provider { @NonNull PipeConnectivityListener providePipeListener(); @@ -508,5 +520,6 @@ public class ApplicationDependencies { @NonNull AppForegroundObserver provideAppForegroundObserver(); @NonNull SignalCallManager provideSignalCallManager(); @NonNull PendingRetryReceiptManager providePendingRetryReceiptManager(); + @NonNull PendingRetryReceiptCache providePendingRetryReceiptCache(); } } 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 23368b242..49764d9ed 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java +++ b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java @@ -33,6 +33,7 @@ import org.thoughtcrime.securesms.megaphone.MegaphoneRepository; import org.thoughtcrime.securesms.messages.BackgroundMessageRetriever; import org.thoughtcrime.securesms.messages.IncomingMessageObserver; import org.thoughtcrime.securesms.messages.IncomingMessageProcessor; +import org.thoughtcrime.securesms.database.PendingRetryReceiptCache; import org.thoughtcrime.securesms.net.PipeConnectivityListener; import org.thoughtcrime.securesms.notifications.MessageNotifier; import org.thoughtcrime.securesms.notifications.OptimizedMessageNotifier; @@ -255,6 +256,11 @@ public class ApplicationDependencyProvider implements ApplicationDependencies.Pr return new PendingRetryReceiptManager(context); } + @Override + public @NonNull PendingRetryReceiptCache providePendingRetryReceiptCache() { + return new PendingRetryReceiptCache(context); + } + private static class DynamicCredentialsProvider implements CredentialsProvider { private final Context context; diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ResendMessageJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/ResendMessageJob.java index 4a08428f4..d8042f84c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/ResendMessageJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/ResendMessageJob.java @@ -10,6 +10,9 @@ import org.signal.core.util.ThreadUtil; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.crypto.UnidentifiedAccessUtil; import org.thoughtcrime.securesms.crypto.storage.SignalSenderKeyStore; +import org.thoughtcrime.securesms.database.DatabaseFactory; +import org.thoughtcrime.securesms.database.GroupDatabase; +import org.thoughtcrime.securesms.database.GroupDatabase.GroupRecord; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.groups.GroupId; import org.thoughtcrime.securesms.jobmanager.Data; @@ -129,6 +132,16 @@ public class ResendMessageJob extends BaseJob { Content contentToSend = content; if (distributionId != null) { + Optional groupRecord = DatabaseFactory.getGroupDatabase(context).getGroupByDistributionId(distributionId); + + if (!groupRecord.isPresent()) { + Log.w(TAG, "Could not find a matching group for the distributionId! Skipping message send."); + return; + } else if (!groupRecord.get().getMembers().contains(recipientId)) { + Log.w(TAG, "The target user is no longer in the group! Skipping message send."); + return; + } + SenderKeyDistributionMessage senderKeyDistributionMessage = messageSender.getOrCreateNewGroupSession(distributionId); ByteString distributionBytes = ByteString.copyFrom(senderKeyDistributionMessage.serialize()); 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 f261ff876..db2e5c53d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.java +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.java @@ -232,7 +232,7 @@ public final class MessageContentProcessor { } RecipientId senderId = RecipientId.fromHighTrust(content.getSender()); - PendingRetryReceiptModel pending = DatabaseFactory.getPendingRetryReceiptDatabase(context).get(senderId, content.getTimestamp()); + PendingRetryReceiptModel pending = ApplicationDependencies.getPendingRetryReceiptCache().get(senderId, content.getTimestamp()); long receivedTime = handlePendingRetry(pending, content); log(String.valueOf(content.getTimestamp()), "Beginning message processing."); @@ -350,7 +350,7 @@ public final class MessageContentProcessor { if (pending != null) { warn(content.getTimestamp(), "Pending retry was processed. Deleting."); - DatabaseFactory.getPendingRetryReceiptDatabase(context).delete(pending.getId()); + ApplicationDependencies.getPendingRetryReceiptCache().delete(pending); } } catch (StorageFailedException e) { warn(String.valueOf(content.getTimestamp()), e); diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/MessageDecryptionUtil.java b/app/src/main/java/org/thoughtcrime/securesms/messages/MessageDecryptionUtil.java index ccdc2aad8..b440de59d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/MessageDecryptionUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/MessageDecryptionUtil.java @@ -147,7 +147,7 @@ public final class MessageDecryptionUtil { break; case RESENDABLE: Log.w(TAG, "[" + envelope.getTimestamp() + "] Inserting into pending retries store because it's " + contentHint); - DatabaseFactory.getPendingRetryReceiptDatabase(context).insert(sender.getId(), senderDevice, envelope.getTimestamp(), receivedTimestamp, threadId); + ApplicationDependencies.getPendingRetryReceiptCache().insert(sender.getId(), senderDevice, envelope.getTimestamp(), receivedTimestamp, threadId); ApplicationDependencies.getPendingRetryReceiptManager().scheduleIfNecessary(); break; case IMPLICIT: diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/PendingRetryReceiptManager.java b/app/src/main/java/org/thoughtcrime/securesms/service/PendingRetryReceiptManager.java index bd0e01b51..53e40bed6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/PendingRetryReceiptManager.java +++ b/app/src/main/java/org/thoughtcrime/securesms/service/PendingRetryReceiptManager.java @@ -13,9 +13,9 @@ import androidx.annotation.WorkerThread; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.database.DatabaseFactory; import org.thoughtcrime.securesms.database.MessageDatabase; -import org.thoughtcrime.securesms.database.PendingRetryReceiptDatabase; import org.thoughtcrime.securesms.database.model.PendingRetryReceiptModel; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; +import org.thoughtcrime.securesms.database.PendingRetryReceiptCache; import org.thoughtcrime.securesms.util.FeatureFlags; @@ -26,13 +26,13 @@ public final class PendingRetryReceiptManager extends TimedEventManager