From 46778838380bb3dd4c1f5cd31c5592194111b85a Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Fri, 2 Jul 2021 17:27:19 -0400 Subject: [PATCH] Improve mapping SignalServiceAddresses to Recipients. --- .../database/MessageSendLogDatabase.kt | 13 ++---- .../jobs/GroupCallUpdateSendJob.java | 2 +- .../securesms/jobs/GroupSendJobHelper.java | 8 ++-- .../securesms/jobs/ProfileKeySendJob.java | 2 +- .../securesms/jobs/PushGroupSendJob.java | 23 +++-------- .../jobs/PushGroupSilentUpdateSendJob.java | 2 +- .../securesms/jobs/ReactionSendJob.java | 2 +- .../securesms/jobs/RemoteDeleteSendJob.java | 2 +- .../securesms/messages/GroupSendUtil.java | 24 ++--------- .../securesms/util/RecipientAccessList.kt | 40 +++++++++++++++++++ 10 files changed, 62 insertions(+), 56 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/util/RecipientAccessList.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessageSendLogDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/MessageSendLogDatabase.kt index f72d670910..d1d1ab4307 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageSendLogDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageSendLogDatabase.kt @@ -9,11 +9,11 @@ import org.thoughtcrime.securesms.recipients.Recipient import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.util.CursorUtil import org.thoughtcrime.securesms.util.FeatureFlags +import org.thoughtcrime.securesms.util.RecipientAccessList import org.thoughtcrime.securesms.util.SqlUtil import org.whispersystems.signalservice.api.crypto.ContentHint import org.whispersystems.signalservice.api.messages.SendMessageResult import org.whispersystems.signalservice.internal.push.SignalServiceProtos -import java.util.UUID /** * Stores a 24-hr buffer of all outgoing messages. Used for the retry logic required for sender key. @@ -176,19 +176,12 @@ class MessageSendLogDatabase constructor(context: Context?, databaseHelper: SQLC fun insertIfPossible(sentTimestamp: Long, possibleRecipients: List, results: List, contentHint: ContentHint, messageId: MessageId): Long { if (!FeatureFlags.senderKey()) return -1 - val recipientsByUuid: Map = possibleRecipients.filter(Recipient::hasUuid).associateBy(Recipient::requireUuid, { it }) - val recipientsByE164: Map = possibleRecipients.filter(Recipient::hasE164).associateBy(Recipient::requireE164, { it }) + val accessList = RecipientAccessList(possibleRecipients) val recipientDevices: List = results .filter { it.isSuccess && it.success.content.isPresent } .map { result -> - val recipient: Recipient = - if (result.address.uuid.isPresent) { - recipientsByUuid[result.address.uuid.get()]!! - } else { - recipientsByE164[result.address.number.get()]!! - } - + val recipient: Recipient = accessList.requireByAddress(result.address) RecipientDevice(recipient.id, result.success.devices) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupCallUpdateSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupCallUpdateSendJob.java index 682838011d..d2dbd6642b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupCallUpdateSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupCallUpdateSendJob.java @@ -166,7 +166,7 @@ public class GroupCallUpdateSendJob extends BaseJob { ContentHint.DEFAULT, dataMessage.build()); - return GroupSendJobHelper.getCompletedSends(context, results); + return GroupSendJobHelper.getCompletedSends(destinations, results); } public static class Factory implements Job.Factory { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupSendJobHelper.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupSendJobHelper.java index 7eeeb75775..b6d4e3aff6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupSendJobHelper.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupSendJobHelper.java @@ -6,6 +6,7 @@ import androidx.annotation.NonNull; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.recipients.Recipient; +import org.thoughtcrime.securesms.util.RecipientAccessList; import org.whispersystems.signalservice.api.messages.SendMessageResult; import java.util.ArrayList; @@ -19,11 +20,12 @@ final class GroupSendJobHelper { private GroupSendJobHelper() { } - static List getCompletedSends(@NonNull Context context, @NonNull Collection results) { - List completions = new ArrayList<>(results.size()); + static List getCompletedSends(@NonNull List possibleRecipients, @NonNull Collection results) { + RecipientAccessList accessList = new RecipientAccessList(possibleRecipients); + List completions = new ArrayList<>(results.size()); for (SendMessageResult sendMessageResult : results) { - Recipient recipient = Recipient.externalPush(context, sendMessageResult.getAddress()); + Recipient recipient = accessList.requireByAddress(sendMessageResult.getAddress()); if (sendMessageResult.getIdentityFailure() != null) { Log.w(TAG, "Identity failure for " + recipient.getId()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ProfileKeySendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/ProfileKeySendJob.java index 31b03f9fe0..0c6669c060 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/ProfileKeySendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/ProfileKeySendJob.java @@ -156,7 +156,7 @@ public class ProfileKeySendJob extends BaseJob { List results = GroupSendUtil.sendUnresendableDataMessage(context, null, destinations, false, ContentHint.IMPLICIT, dataMessage.build()); - return GroupSendJobHelper.getCompletedSends(context, results); + return GroupSendJobHelper.getCompletedSends(destinations, results); } public static class Factory implements Job.Factory { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java index 2424f0b188..b0d35245c2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java @@ -41,6 +41,7 @@ import org.thoughtcrime.securesms.recipients.RecipientUtil; import org.thoughtcrime.securesms.transport.RetryLaterException; import org.thoughtcrime.securesms.transport.UndeliverableMessageException; import org.thoughtcrime.securesms.util.GroupUtil; +import org.thoughtcrime.securesms.util.RecipientAccessList; import org.whispersystems.libsignal.util.Pair; import org.whispersystems.libsignal.util.guava.Optional; import org.whispersystems.signalservice.api.SignalServiceMessageSender; @@ -191,17 +192,16 @@ public final class PushGroupSendJob extends PushSendJob { else if (!existingNetworkFailures.isEmpty()) target = Stream.of(existingNetworkFailures).map(nf -> Recipient.resolved(nf.getRecipientId(context))).toList(); else target = getGroupMessageRecipients(groupRecipient.requireGroupId(), messageId); - Map idByE164 = Stream.of(target).filter(Recipient::hasE164).collect(Collectors.toMap(Recipient::requireE164, r -> r)); - Map idByUuid = Stream.of(target).filter(Recipient::hasUuid).collect(Collectors.toMap(Recipient::requireUuid, r -> r)); + RecipientAccessList accessList = new RecipientAccessList(target); List results = deliver(message, groupRecipient, target); Log.i(TAG, JobLogger.format(this, "Finished send.")); - List networkFailures = Stream.of(results).filter(SendMessageResult::isNetworkFailure).map(result -> new NetworkFailure(findId(result.getAddress(), idByE164, idByUuid))).toList(); - List identityMismatches = Stream.of(results).filter(result -> result.getIdentityFailure() != null).map(result -> new IdentityKeyMismatch(findId(result.getAddress(), idByE164, idByUuid), result.getIdentityFailure().getIdentityKey())).toList(); + List networkFailures = Stream.of(results).filter(SendMessageResult::isNetworkFailure).map(result -> new NetworkFailure(accessList.requireIdByAddress(result.getAddress()))).toList(); + List identityMismatches = Stream.of(results).filter(result -> result.getIdentityFailure() != null).map(result -> new IdentityKeyMismatch(accessList.requireIdByAddress(result.getAddress()), result.getIdentityFailure().getIdentityKey())).toList(); ProofRequiredException proofRequired = Stream.of(results).filter(r -> r.getProofRequiredFailure() != null).findLast().map(SendMessageResult::getProofRequiredFailure).orElse(null); List successes = Stream.of(results).filter(result -> result.getSuccess() != null).toList(); - List> successUnidentifiedStatus = Stream.of(successes).map(result -> new Pair<>(findId(result.getAddress(), idByE164, idByUuid), result.getSuccess().isUnidentified())).toList(); + List> successUnidentifiedStatus = Stream.of(successes).map(result -> new Pair<>(accessList.requireIdByAddress(result.getAddress()), result.getSuccess().isUnidentified())).toList(); Set successIds = Stream.of(successUnidentifiedStatus).map(Pair::first).collect(Collectors.toSet()); List resolvedNetworkFailures = Stream.of(existingNetworkFailures).filter(failure -> successIds.contains(failure.getRecipientId(context))).toList(); List resolvedIdentityFailures = Stream.of(existingIdentityMismatches).filter(failure -> successIds.contains(failure.getRecipientId(context))).toList(); @@ -274,19 +274,6 @@ public final class PushGroupSendJob extends PushSendJob { DatabaseFactory.getMmsDatabase(context).markAsSentFailed(messageId); } - private static @NonNull RecipientId findId(@NonNull SignalServiceAddress address, - @NonNull Map byE164, - @NonNull Map byUuid) - { - if (address.getNumber().isPresent() && byE164.containsKey(address.getNumber().get())) { - return Objects.requireNonNull(byE164.get(address.getNumber().get())).getId(); - } else if (address.getUuid().isPresent() && byUuid.containsKey(address.getUuid().get())) { - return Objects.requireNonNull(byUuid.get(address.getUuid().get())).getId(); - } else { - throw new IllegalStateException("Found an address that was never provided!"); - } - } - private List deliver(OutgoingMediaMessage message, @NonNull Recipient groupRecipient, @NonNull List destinations) throws IOException, UntrustedIdentityException, UndeliverableMessageException { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSilentUpdateSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSilentUpdateSendJob.java index b08c00dcc4..8a3eff2807 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSilentUpdateSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSilentUpdateSendJob.java @@ -169,7 +169,7 @@ public final class PushGroupSilentUpdateSendJob extends BaseJob { List results = GroupSendUtil.sendUnresendableDataMessage(context, groupId, destinations, false, ContentHint.IMPLICIT, groupDataMessage); - return GroupSendJobHelper.getCompletedSends(context, results); + return GroupSendJobHelper.getCompletedSends(destinations, results); } public static class Factory implements Job.Factory { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ReactionSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/ReactionSendJob.java index bfaa86e3e5..8345db03ff 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/ReactionSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/ReactionSendJob.java @@ -242,7 +242,7 @@ public class ReactionSendJob extends BaseJob { new MessageId(messageId, isMms), dataMessage); - return GroupSendJobHelper.getCompletedSends(context, results); + return GroupSendJobHelper.getCompletedSends(destinations, results); } private static SignalServiceDataMessage.Reaction buildReaction(@NonNull Context context, diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java index 695869cd24..0b320a30d8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java @@ -193,7 +193,7 @@ public class RemoteDeleteSendJob extends BaseJob { new MessageId(messageId, isMms), dataMessage); - return GroupSendJobHelper.getCompletedSends(context, results); + return GroupSendJobHelper.getCompletedSends(destinations, results); } public static class Factory implements Job.Factory { diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/GroupSendUtil.java b/app/src/main/java/org/thoughtcrime/securesms/messages/GroupSendUtil.java index 1c63a96e86..5568544eea 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/GroupSendUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/GroupSendUtil.java @@ -19,6 +19,7 @@ import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; import org.thoughtcrime.securesms.recipients.RecipientUtil; import org.thoughtcrime.securesms.util.FeatureFlags; +import org.thoughtcrime.securesms.util.RecipientAccessList; import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.whispersystems.libsignal.InvalidKeyException; import org.whispersystems.libsignal.NoSessionException; @@ -410,23 +411,12 @@ public final class GroupSendUtil { private final Map> accessById; private final Map addressById; - private final Map idByUuid; - private final Map idByE164; + private final RecipientAccessList accessList; RecipientData(@NonNull Context context, @NonNull List recipients) throws IOException { this.accessById = UnidentifiedAccessUtil.getAccessMapFor(context, recipients); this.addressById = mapAddresses(context, recipients); - this.idByUuid = new HashMap<>(recipients.size()); - this.idByE164 = new HashMap<>(recipients.size()); - - for (Recipient recipient : recipients) { - if (recipient.hasUuid()) { - idByUuid.put(recipient.requireUuid(), recipient.getId()); - } - if (recipient.hasE164()) { - idByE164.put(recipient.requireE164(), recipient.getId()); - } - } + this.accessList = new RecipientAccessList(recipients); } @NonNull SignalServiceAddress getAddress(@NonNull RecipientId id) { @@ -442,13 +432,7 @@ public final class GroupSendUtil { } @NonNull RecipientId requireRecipientId(@NonNull SignalServiceAddress address) { - if (address.getUuid().isPresent() && idByUuid.containsKey(address.getUuid().get())) { - return Objects.requireNonNull(idByUuid.get(address.getUuid().get())); - } else if (address.getNumber().isPresent() && idByE164.containsKey(address.getNumber().get())) { - return Objects.requireNonNull(idByE164.get(address.getNumber().get())); - } else { - throw new IllegalStateException(); - } + return accessList.requireIdByAddress(address); } private static @NonNull Map mapAddresses(@NonNull Context context, @NonNull List recipients) throws IOException { diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/RecipientAccessList.kt b/app/src/main/java/org/thoughtcrime/securesms/util/RecipientAccessList.kt new file mode 100644 index 0000000000..8e82a8ee35 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/util/RecipientAccessList.kt @@ -0,0 +1,40 @@ +package org.thoughtcrime.securesms.util + +import org.thoughtcrime.securesms.recipients.Recipient +import org.thoughtcrime.securesms.recipients.RecipientId +import org.whispersystems.signalservice.api.push.SignalServiceAddress +import java.lang.IllegalArgumentException +import java.util.UUID + +/** + * A list of Recipients, but with some helpful methods for retrieving them by various properties. Uses lazy properties to ensure that it will be as performant + * as a regular list if you don't call any of the extra methods. + */ +class RecipientAccessList(private val recipients: List) : List by recipients { + + private val byUuid: Map by lazy { + recipients + .filter { it.hasUuid() } + .associateBy { it.requireUuid() } + } + + private val byE164: Map by lazy { + recipients + .filter { it.hasE164() } + .associateBy { it.requireE164() } + } + + fun requireByAddress(address: SignalServiceAddress): Recipient { + if (address.uuid.isPresent && byUuid.containsKey(address.uuid.get())) { + return byUuid.get(address.uuid.get())!! + } else if (address.number.isPresent && byE164.containsKey(address.number.get())) { + return byE164.get(address.number.get())!! + } else { + throw IllegalArgumentException("Could not find a matching recipient!") + } + } + + fun requireIdByAddress(address: SignalServiceAddress): RecipientId { + return requireByAddress(address).id + } +}