From bce133ac2884dac337f3cfb02913ef18307ebe70 Mon Sep 17 00:00:00 2001 From: Nicholas Tinsley Date: Mon, 25 Sep 2023 12:20:12 -0400 Subject: [PATCH] Add more logging around missing RecipientId. --- .../thoughtcrime/securesms/database/RecipientTable.kt | 6 +++++- .../securesms/jobs/PushProcessMessageJob.kt | 11 ++++------- .../securesms/messages/MessageContentProcessor.kt | 6 +++--- .../thoughtcrime/securesms/recipients/Recipient.java | 9 ++++++++- .../securesms/recipients/RecipientId.java | 7 +++++-- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt index f6bda3112d..73c278484b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -601,7 +601,11 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da } val recipientId = RecipientId.from(id) - update(recipientId, groupUpdates) + val updateSuccess = update(recipientId, groupUpdates) + + if (!updateSuccess) { + Log.w(TAG, "Failed to update newly-created record for $recipientId") + } return recipientId } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.kt index 2a8ab2fd2f..d183ecd189 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.kt @@ -114,15 +114,11 @@ class PushProcessMessageJob private constructor( } fun processOrDefer(messageProcessor: MessageContentProcessor, result: MessageDecryptor.Result.Success, localReceiveMetric: SignalLocalMetrics.MessageReceive): PushProcessMessageJob? { - val queueName: String - val groupContext = GroupUtil.getGroupContextIfPresent(result.content) val groupId = groupContext?.groupId var requireNetwork = false - if (groupId != null) { - queueName = getQueueName(RecipientId.from(groupId)) - + val queueName: String = if (groupId != null) { if (groupId.isV2) { val localRevision = groups.getGroupV2Revision(groupId.requireV2()) @@ -131,10 +127,11 @@ class PushProcessMessageJob private constructor( requireNetwork = true } } + getQueueName(RecipientId.from(groupId)) } else if (result.content.syncMessage != null && result.content.syncMessage!!.sent != null && result.content.syncMessage!!.sent!!.destinationServiceId != null) { - queueName = getQueueName(RecipientId.from(ServiceId.parseOrThrow(result.content.syncMessage!!.sent!!.destinationServiceId!!))) + getQueueName(RecipientId.from(ServiceId.parseOrThrow(result.content.syncMessage!!.sent!!.destinationServiceId!!))) } else { - queueName = getQueueName(RecipientId.from(result.metadata.sourceServiceId)) + getQueueName(RecipientId.from(result.metadata.sourceServiceId)) } return if (requireNetwork || !isQueueEmpty(queueName = queueName, isGroup = groupId != null)) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.kt index a98510b403..738dd23cdd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/MessageContentProcessor.kt @@ -125,11 +125,11 @@ open class MessageContentProcessor(private val context: Context) { @Throws(BadGroupIdException::class) private fun getMessageDestination(content: Content, sender: Recipient): Recipient { return if (content.storyMessage != null && content.storyMessage!!.group.isValid) { - getGroupRecipient(content.storyMessage!!.group, sender) + getGroupRecipient(content.storyMessage?.group, sender) } else if (content.dataMessage.hasGroupContext) { - getGroupRecipient(content.dataMessage!!.groupV2, sender) + getGroupRecipient(content.dataMessage?.groupV2, sender) } else if (content.editMessage?.dataMessage.hasGroupContext) { - getGroupRecipient(content.editMessage!!.dataMessage!!.groupV2, sender) + getGroupRecipient(content.editMessage?.dataMessage?.groupV2, sender) } else { sender } diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java index 719399e5b2..be8aecc437 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java @@ -320,7 +320,14 @@ public class Recipient { */ @WorkerThread public static @NonNull Recipient externalPossiblyMigratedGroup(@NonNull GroupId groupId) { - return Recipient.resolved(RecipientId.from(groupId)); + RecipientId id = RecipientId.from(groupId); + try { + return Recipient.resolved(id); + } catch (RecipientTable.MissingRecipientException ex) { + Log.w(TAG, "Could not find recipient (" + id + ") for group " + groupId + ". Clearing RecipientId cache and trying again.", ex); + RecipientId.clearCache(); + return Recipient.resolved(SignalDatabase.recipients().getOrInsertFromPossiblyMigratedGroupId(groupId)); + } } /** diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientId.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientId.java index d6eeb3cf1a..f36e200a46 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientId.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientId.java @@ -12,6 +12,7 @@ import com.annimon.stream.Stream; import org.signal.core.util.DatabaseId; import org.signal.core.util.LongSerializer; +import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.database.SignalDatabase; import org.thoughtcrime.securesms.groups.GroupId; import org.thoughtcrime.securesms.util.DelimiterUtil; @@ -27,8 +28,9 @@ import java.util.regex.Pattern; public class RecipientId implements Parcelable, Comparable, DatabaseId { - private static final long UNKNOWN_ID = -1; - private static final char DELIMITER = ','; + private static final String TAG = "RecipientId"; + private static final long UNKNOWN_ID = -1; + private static final char DELIMITER = ','; public static final RecipientId UNKNOWN = RecipientId.from(UNKNOWN_ID); public static final LongSerializer SERIALIZER = new Serializer(); @@ -73,6 +75,7 @@ public class RecipientId implements Parcelable, Comparable, Databas public static @NonNull RecipientId from(@NonNull GroupId groupId) { RecipientId recipientId = RecipientIdCache.INSTANCE.get(groupId); if (recipientId == null) { + Log.d(TAG, "RecipientId cache miss for " + groupId); recipientId = SignalDatabase.recipients().getOrInsertFromPossiblyMigratedGroupId(groupId); if (groupId.isV2()) { RecipientIdCache.INSTANCE.put(groupId, recipientId);