Fix ID remapping issues when getting group membership.

app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java

@@ -581,7 +581,7 @@ private static final String[] GROUP_PROJECTION = {
         throw new AssertionError("V2 master key but no group state");
       }
       groupId.requireV2();
-      groupMembers = getV2GroupMembers(context, groupState, true);
+      groupMembers = getV2GroupMembers(groupState, true);
       contentValues.put(V2_MASTER_KEY, groupMasterKey.serialize());
       contentValues.put(V2_REVISION, groupState.getRevision());
       contentValues.put(V2_DECRYPTED_GROUP, groupState.toByteArray());
@@ -720,7 +720,7 @@ private static final String[] GROUP_PROJECTION = {
       contentValues.put(UNMIGRATED_V1_MEMBERS, unmigratedV1Members.isEmpty() ? null : RecipientId.toSerializedList(unmigratedV1Members));
     }
 
-    List<RecipientId> groupMembers = getV2GroupMembers(context, decryptedGroup, true);
+    List<RecipientId> groupMembers = getV2GroupMembers(decryptedGroup, true);
     contentValues.put(TITLE, title);
     contentValues.put(V2_REVISION, decryptedGroup.getRevision());
     contentValues.put(V2_DECRYPTED_GROUP, decryptedGroup.toByteArray());
@@ -885,7 +885,15 @@ private static final String[] GROUP_PROJECTION = {
       if (UuidUtil.UNKNOWN_UUID.equals(uuid)) {
         Log.w(TAG, "Seen unknown UUID in members list");
       } else {
-        groupMembers.add(RecipientId.from(ACI.from(uuid), null));
+        RecipientId           id       = RecipientId.from(ACI.from(uuid), null);
+        Optional<RecipientId> remapped = RemappedRecords.getInstance().getRecipient(id);
+
+        if (remapped.isPresent()) {
+          Log.w(TAG, "Saw that " + id + " remapped to " + remapped + ". Using the mapping.");
+          groupMembers.add(remapped.get());
+        } else {
+          groupMembers.add(id);
+        }
       }
     }
 
@@ -894,7 +902,7 @@ private static final String[] GROUP_PROJECTION = {
     return groupMembers;
   }
 
-  private static @NonNull List<RecipientId> getV2GroupMembers(@NonNull Context context, @NonNull DecryptedGroup decryptedGroup, boolean shouldRetry) {
+  private static @NonNull List<RecipientId> getV2GroupMembers(@NonNull DecryptedGroup decryptedGroup, boolean shouldRetry) {
     List<UUID>        uuids = DecryptedGroupUtil.membersToUuidList(decryptedGroup.getMembersList());
     List<RecipientId> ids   = uuidsToRecipientIds(uuids);
 
@@ -902,7 +910,8 @@ private static final String[] GROUP_PROJECTION = {
       if (shouldRetry) {
         Log.w(TAG, "Found remapped records where we shouldn't. Clearing cache and trying again.");
         RecipientId.clearCache();
-        return getV2GroupMembers(context, decryptedGroup, false);
+        RemappedRecords.getInstance().resetCache();
+        return getV2GroupMembers(decryptedGroup, false);
       } else {
         throw new IllegalStateException("Remapped records in group membership!");
       }

app/src/main/java/org/thoughtcrime/securesms/database/RemappedRecords.java

@@ -1,12 +1,11 @@
 package org.thoughtcrime.securesms.database;
 
-import android.content.Context;
-
 import androidx.annotation.NonNull;
 
 import org.signal.core.util.logging.Log;
 import org.thoughtcrime.securesms.recipients.RecipientId;
 import org.whispersystems.libsignal.util.guava.Optional;
+import org.whispersystems.libsignal.util.guava.Preconditions;
 
 import java.util.Collection;
 import java.util.LinkedHashSet;
@@ -88,6 +87,7 @@ class RemappedRecords {
    */
   void addRecipient(@NonNull RecipientId oldId, @NonNull RecipientId newId) {
     Log.w(TAG, "[Recipient] Remapping " + oldId + " to " + newId);
+    Preconditions.checkArgument(!oldId.equals(newId), "Cannot remap an ID to the same thing!");
     ensureInTransaction();
     ensureRecipientMapIsPopulated();
     recipientMap.put(oldId, newId);
@@ -99,12 +99,20 @@ class RemappedRecords {
    */
   void addThread(long oldId, long newId) {
     Log.w(TAG, "[Thread] Remapping " + oldId + " to " + newId);
+    Preconditions.checkArgument(oldId != newId, "Cannot remap an ID to the same thing!");
     ensureInTransaction();
     ensureThreadMapIsPopulated();
     threadMap.put(oldId, newId);
     SignalDatabase.remappedRecords().addThreadMapping(oldId, newId);
   }
 
+  /**
+   * Clears out the memory cache. The next read will pull values from disk.
+   */
+  void resetCache() {
+    recipientMap = null;
+  }
+
   private void ensureRecipientMapIsPopulated() {
     if (recipientMap == null) {
       recipientMap = SignalDatabase.remappedRecords().getAllRecipientMappings();

app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java

@@ -211,20 +211,20 @@ public class Recipient {
   }
 
   /**
-   * Returns a fully-populated {@link Recipient} based off of a UUID and phone number, creating one
+   * Returns a fully-populated {@link Recipient} based off of an ACI and phone number, creating one
    * in the database if necessary. We want both piece of information so we're able to associate them
    * both together, depending on which are available.
    *
-   * In particular, while we'll eventually get the UUID of a user created via a phone number
+   * In particular, while we'll eventually get the ACI of a user created via a phone number
    * (through a directory sync), the only way we can store the phone number is by retrieving it from
    * sent messages and whatnot. So we should store it when available.
    *
-   * @param highTrust This should only be set to true if the source of the E164-UUID pairing is one
+   * @param highTrust This should only be set to true if the source of the E164-ACI pairing is one
    *                  that can be trusted as accurate (like an envelope).
    */
   @WorkerThread
   public static @NonNull Recipient externalPush(@NonNull Context context, @Nullable ACI aci, @Nullable String e164, boolean highTrust) {
-    if (UuidUtil.UNKNOWN_UUID.equals(aci)) {
+    if (ACI.UNKNOWN.equals(aci)) {
       throw new AssertionError();
     }
 
@@ -233,11 +233,15 @@ public class Recipient {
 
     Recipient resolved = resolved(recipientId);
 
+    if (!resolved.getId().equals(recipientId)) {
+      Log.w(TAG, "Resolved " + recipientId + ", but got back a recipient with " + resolved.getId());
+    }
+
     if (highTrust && !resolved.isRegistered() && aci != null) {
       Log.w(TAG, "External high-trust push was locally marked unregistered. Marking as registered.");
       db.markRegistered(recipientId, aci);
     } else if (highTrust && !resolved.isRegistered()) {
-      Log.w(TAG, "External high-trust push was locally marked unregistered, but we don't have a UUID, so we can't do anything.", new Throwable());
+      Log.w(TAG, "External high-trust push was locally marked unregistered, but we don't have an ACI, so we can't do anything.", new Throwable());
     }
 
     return resolved;