Refactor Message Request logic to fix some GV1->GV2 bugs.

app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java

@@ -190,6 +190,7 @@
 import org.thoughtcrime.securesms.mediasend.MediaSendActivity;
 import org.thoughtcrime.securesms.mediasend.MediaSendActivityResult;
 import org.thoughtcrime.securesms.messagedetails.MessageDetailsActivity;
+import org.thoughtcrime.securesms.messagerequests.MessageRequestState;
 import org.thoughtcrime.securesms.messagerequests.MessageRequestViewModel;
 import org.thoughtcrime.securesms.messagerequests.MessageRequestsBottomView;
 import org.thoughtcrime.securesms.mms.AttachmentManager;
@@ -3140,8 +3141,7 @@ public void onMessageRequest(@NonNull MessageRequestViewModel viewModel) {
     messageRequestBottomView.setGroupV1MigrationContinueListener(v -> GroupsV1MigrationInitiationBottomSheetDialogFragment.showForInitiation(getSupportFragmentManager(), recipient.getId()));
 
     viewModel.getRequestReviewDisplayState().observe(this, this::presentRequestReviewBanner);
-    viewModel.getMessageData().observe(this, this::presentMessageRequestBottomViewTo);
-    viewModel.getMessageRequestDisplayState().observe(this, this::presentMessageRequestDisplayState);
+    viewModel.getMessageData().observe(this, this::presentMessageRequestState);
     viewModel.getFailures().observe(this, this::showGroupChangeErrorToast);
     viewModel.getMessageRequestStatus().observe(this, status -> {
       switch (status) {
@@ -3155,7 +3155,6 @@ public void onMessageRequest(@NonNull MessageRequestViewModel viewModel) {
           break;
         case ACCEPTED:
           hideMessageRequestBusy();
-          messageRequestBottomView.setVisibility(View.GONE);
           break;
         case DELETED:
         case BLOCKED:
@@ -3426,31 +3425,6 @@ private void onMessageRequestUnblockClicked(@NonNull MessageRequestViewModel req
     BlockUnblockDialog.showUnblockFor(this, getLifecycle(), recipient, requestModel::onUnblock);
   }
 
-  private void presentMessageRequestDisplayState(@NonNull MessageRequestViewModel.DisplayState displayState) {
-    if ((getIntent().hasExtra(TEXT_EXTRA) && !Util.isEmpty(getIntent().getStringExtra(TEXT_EXTRA))) ||
-         getIntent().hasExtra(MEDIA_EXTRA)                                                          ||
-         getIntent().hasExtra(STICKER_EXTRA))
-    {
-      Log.d(TAG, "[presentMessageRequestDisplayState] Have extra, so ignoring provided state.");
-      messageRequestBottomView.setVisibility(View.GONE);
-    } else if (isPushGroupV1Conversation() && !isActiveGroup()) {
-      Log.d(TAG, "[presentMessageRequestDisplayState] Inactive push group V1, so ignoring provided state.");
-      messageRequestBottomView.setVisibility(View.GONE);
-    } else {
-      Log.d(TAG, "[presentMessageRequestDisplayState] " + displayState);
-      switch (displayState) {
-        case DISPLAY_MESSAGE_REQUEST:
-          messageRequestBottomView.setVisibility(View.VISIBLE);
-          break;
-        case DISPLAY_NONE:
-          messageRequestBottomView.setVisibility(View.GONE);
-          break;
-      }
-    }
-
-    invalidateOptionsMenu();
-  }
-
   private static void hideMenuItem(@NonNull Menu menu, @IdRes int menuItem) {
     if (menu.findItem(menuItem) != null) {
       menu.findItem(menuItem).setVisible(false);
@@ -3597,10 +3571,28 @@ protected void onPostExecute(ConversationMessage conversationMessage) {
     }
   }
 
-  private void presentMessageRequestBottomViewTo(@Nullable MessageRequestViewModel.MessageData messageData) {
-    if (messageData == null) return;
+  private void presentMessageRequestState(@Nullable MessageRequestViewModel.MessageData messageData) {
+    if ((getIntent().hasExtra(TEXT_EXTRA) && !Util.isEmpty(getIntent().getStringExtra(TEXT_EXTRA))) ||
+        getIntent().hasExtra(MEDIA_EXTRA)                                                           ||
+        getIntent().hasExtra(STICKER_EXTRA))
+    {
+      Log.d(TAG, "[presentMessageRequestState] Have extra, so ignoring provided state.");
+      messageRequestBottomView.setVisibility(View.GONE);
+    } else if (isPushGroupV1Conversation() && !isActiveGroup()) {
+      Log.d(TAG, "[presentMessageRequestState] Inactive push group V1, so ignoring provided state.");
+      messageRequestBottomView.setVisibility(View.GONE);
+    } else if (messageData == null) {
+      Log.d(TAG, "[presentMessageRequestState] Null messageData. Ignoring.");
+    } else if (messageData.getMessageState() == MessageRequestState.NONE) {
+      Log.d(TAG, "[presentMessageRequestState] No message request necessary.");
+      messageRequestBottomView.setVisibility(View.GONE);
+    } else {
+      Log.d(TAG, "[presentMessageRequestState] " + messageData.getMessageState());
+      messageRequestBottomView.setMessageData(messageData);
+      messageRequestBottomView.setVisibility(View.VISIBLE);
+    }
 
-    messageRequestBottomView.setMessageData(messageData);
+    invalidateOptionsMenu();
   }
 
   private static class KeyboardImageDetails {

app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationGroupViewModel.java

@@ -170,6 +170,14 @@ private static List<RecipientId> mapToGroupV1MigrationSuggestions(@Nullable Grou
       return Collections.emptyList();
     }
 
+    if (!record.isV2Group()) {
+      return Collections.emptyList();
+    }
+
+    if (!record.isActive() || record.isPendingMember(Recipient.self())) {
+      return Collections.emptyList();
+    }
+
     Set<RecipientId> difference = SetUtil.difference(record.getFormerV1Members(), record.getMembers());
 
     return Stream.of(Recipient.resolvedList(difference))
@@ -180,7 +188,13 @@ private static List<RecipientId> mapToGroupV1MigrationSuggestions(@Nullable Grou
 
   @WorkerThread
   private static boolean mapToGroupV1MigrationReminder(@Nullable GroupRecord record) {
-    if (record == null || !record.isV1Group() || !record.isActive() || !FeatureFlags.groupsV1ManualMigration()) {
+    if (record == null                          ||
+        !record.isV1Group()                     ||
+        !record.isActive()                      ||
+        !FeatureFlags.groupsV1ManualMigration() ||
+        FeatureFlags.groupsV1ForcedMigration()  ||
+        !Recipient.resolved(record.getRecipientId()).isProfileSharing())
+    {
       return false;
     }
 

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

@@ -966,7 +966,10 @@ public GroupAccessControl getAttributesAccessControl() {
       }
     }
 
-    boolean isPendingMember(@NonNull Recipient recipient) {
+    /**
+     * Whether or not the recipient is a pending member.
+     */
+    public boolean isPendingMember(@NonNull Recipient recipient) {
       if (isV2Group()) {
         Optional<UUID> uuid = recipient.getUuid();
         if (uuid.isPresent()) {

app/src/main/java/org/thoughtcrime/securesms/groups/GroupsV1MigrationUtil.java

@@ -140,17 +140,25 @@ public static void migrate(@NonNull Context context, @NonNull RecipientId recipi
     DecryptedGroup decryptedGroup = performLocalMigration(context, gv1Id, threadId, groupRecipient);
 
     if (newlyCreated && decryptedGroup != null && !SignalStore.internalValues().disableGv1AutoMigrateNotification()) {
+      Log.i(TAG, "Sending no-op update to notify others.");
       GroupManager.sendNoopUpdate(context, gv2MasterKey, decryptedGroup);
     }
   }
 
   public static void performLocalMigration(@NonNull Context context, @NonNull GroupId.V1 gv1Id) throws IOException
   {
+    Log.i(TAG, "Beginning local migration!", new Throwable());
     try (Closeable ignored = GroupsV2ProcessingLock.acquireGroupProcessingLock()) {
+      if (DatabaseFactory.getGroupDatabase(context).groupExists(gv1Id.deriveV2MigrationGroupId())) {
+        Log.w(TAG, "Group was already migrated! Could have been waiting for the lock.", new Throwable());
+        return;
+      }
+
       Recipient recipient = Recipient.externalGroupExact(context, gv1Id);
       long      threadId  = DatabaseFactory.getThreadDatabase(context).getThreadIdFor(recipient);
 
       performLocalMigration(context, gv1Id, threadId, recipient);
+      Log.i(TAG, "Migration complete!", new Throwable());
     } catch (GroupChangeBusyException e) {
       throw new IOException(e);
     }

app/src/main/java/org/thoughtcrime/securesms/messagerequests/MessageRequestRepository.java

@@ -72,44 +72,57 @@ void getMemberCount(@NonNull RecipientId recipientId, @NonNull Consumer<GroupMem
     });
   }
 
-  void getMessageRequestState(@NonNull Recipient recipient, long threadId, @NonNull Consumer<MessageRequestState> state) {
-    executor.execute(() -> state.accept(findMessageRequestState(recipient, threadId)));
-  }
-
   @WorkerThread
-  private MessageRequestState findMessageRequestState(@NonNull Recipient recipient, long threadId) {
-    if (recipient.isGroup() && recipient.isPushV2Group()) {
-      GroupDatabase.MemberLevel memberLevel = DatabaseFactory.getGroupDatabase(context)
-                                                             .getGroup(recipient.getId())
-                                                             .transform(g -> g.memberLevel(Recipient.self()))
-                                                             .or(GroupDatabase.MemberLevel.NOT_A_MEMBER);
-
-      if (memberLevel == GroupDatabase.MemberLevel.PENDING_MEMBER) {
-        return MessageRequestState.REQUIRED;
+  @NonNull MessageRequestState getMessageRequestState(@NonNull Recipient recipient, long threadId) {
+    if (recipient.isBlocked()) {
+      if (recipient.isGroup()) {
+        return MessageRequestState.BLOCKED_GROUP;
+      } else {
+        return MessageRequestState.BLOCKED_INDIVIDUAL;
       }
-    }
-
-    if (recipient.isPushV1Group() && FeatureFlags.groupsV1ForcedMigration()) {
-      return MessageRequestState.REQUIRED;
-    }
-
-    if (!RecipientUtil.isMessageRequestAccepted(context, threadId)) {
+    } else if (threadId <= 0) {
+      return MessageRequestState.NONE;
+    } else if (recipient.isPushV2Group()) {
+      switch (getGroupMemberLevel(recipient.getId())) {
+        case NOT_A_MEMBER:
+          return MessageRequestState.NONE;
+        case PENDING_MEMBER:
+          return MessageRequestState.GROUP_V2_INVITE;
+        default:
+          if (RecipientUtil.isMessageRequestAccepted(context, threadId)) {
+            return MessageRequestState.NONE;
+          } else {
+            return MessageRequestState.GROUP_V2_ADD;
+          }
+      }
+    } else if (!RecipientUtil.isLegacyProfileSharingAccepted(recipient) && isLegacyThread(recipient)) {
       if (recipient.isGroup()) {
-        GroupDatabase.MemberLevel memberLevel = DatabaseFactory.getGroupDatabase(context)
-                                                               .getGroup(recipient.getId())
-                                                               .transform(g -> g.memberLevel(Recipient.self()))
-                                                               .or(GroupDatabase.MemberLevel.NOT_A_MEMBER);
-
-        if (memberLevel == GroupDatabase.MemberLevel.NOT_A_MEMBER) {
-          return MessageRequestState.NOT_REQUIRED;
+        return MessageRequestState.LEGACY_GROUP_V1;
+      } else {
+        return MessageRequestState.LEGACY_INDIVIDUAL;
+      }
+    } else if (recipient.isPushV1Group()) {
+      if (RecipientUtil.isMessageRequestAccepted(context, threadId)) {
+        if (FeatureFlags.groupsV1ForcedMigration()) {
+          if (recipient.getParticipants().size() > FeatureFlags.groupLimits().getHardLimit()) {
+            return MessageRequestState.DEPRECATED_GROUP_V1_TOO_LARGE;
+          } else {
+            return MessageRequestState.DEPRECATED_GROUP_V1;
+          }
+        } else {
+          return MessageRequestState.NONE;
         }
+      } else if (!recipient.isActiveGroup()) {
+        return MessageRequestState.NONE;
+      } else {
+        return MessageRequestState.GROUP_V1;
       }
-
-      return MessageRequestState.REQUIRED;
-    } else if (!RecipientUtil.isLegacyProfileSharingAccepted(recipient) && threadId > 0) {
-      return MessageRequestState.REQUIRED;
     } else {
-      return MessageRequestState.NOT_REQUIRED;
+      if (RecipientUtil.isMessageRequestAccepted(context, threadId)) {
+        return MessageRequestState.NONE;
+      } else {
+        return MessageRequestState.INDIVIDUAL;
+      }
     }
   }
 
@@ -259,23 +272,20 @@ void unblockAndAccept(@NonNull LiveRecipient liveRecipient, long threadId, @NonN
     });
   }
 
-  @WorkerThread
-  boolean isPendingMember(@NonNull GroupId.V2 groupId) {
-    return DatabaseFactory.getGroupDatabase(context).isPendingMember(groupId, Recipient.self());
+  private GroupDatabase.MemberLevel getGroupMemberLevel(@NonNull RecipientId recipientId) {
+    return DatabaseFactory.getGroupDatabase(context)
+                          .getGroup(recipientId)
+                          .transform(g -> g.memberLevel(Recipient.self()))
+                          .or(GroupDatabase.MemberLevel.NOT_A_MEMBER);
   }
 
-  enum MessageRequestState {
-    /**
-     * Message request permission does not need to be gained at this time.
-     * <p>
-     * Either:
-     * - Explicit message request has been accepted, or;
-     * - Did not need to be shown because they are a contact etc, or;
-     * - It's a group that they are no longer in or invited to.
-     */
-    NOT_REQUIRED,
-
-    /** Explicit message request permission is required. */
-    REQUIRED
+
+  @WorkerThread
+  private boolean isLegacyThread(@NonNull Recipient recipient) {
+    Context context  = ApplicationDependencies.getApplication();
+    Long    threadId = DatabaseFactory.getThreadDatabase(context).getThreadIdFor(recipient.getId());
+
+    return threadId != null &&
+        (RecipientUtil.hasSentMessageInThread(context, threadId) || RecipientUtil.isPreMessageRequestThread(context, threadId));
   }
 }

app/src/main/java/org/thoughtcrime/securesms/messagerequests/MessageRequestState.java

@@ -0,0 +1,39 @@
+package org.thoughtcrime.securesms.messagerequests;
+
+/**
+ * An enum representing the possible message request states a user can be in.
+ */
+public enum MessageRequestState {
+  /** No message request necessary */
+  NONE,
+
+  /** A user is blocked */
+  BLOCKED_INDIVIDUAL,
+
+  /** A group is blocked */
+  BLOCKED_GROUP,
+
+  /** An individual conversation that existed pre-message-requests but doesn't have profile sharing enabled */
+  LEGACY_INDIVIDUAL,
+
+  /** A V1 group conversation that existed pre-message-requests but doesn't have profile sharing enabled */
+  LEGACY_GROUP_V1,
+
+  /** A V1 group conversation that is no longer allowed, because we've forced GV2 on. */
+  DEPRECATED_GROUP_V1,
+
+  /** A V1 group conversation that is no longer allowed, because we've forced GV2 on, but it's also too large to migrate. Nothing we can do. */
+  DEPRECATED_GROUP_V1_TOO_LARGE,
+
+  /** A message request is needed for a V1 group */
+  GROUP_V1,
+
+  /** An invite response is needed for a V2 group */
+  GROUP_V2_INVITE,
+
+  /** A message request is needed for a V2 group */
+  GROUP_V2_ADD,
+
+  /** A message request is needed for an individual */
+  INDIVIDUAL
+}