Storage Service: Fetch updates on any group record merge

Co-authored-by: Scott Nonnenberg <scott@signal.org>

ts/models/conversations.ts

@@ -2366,9 +2366,9 @@ export class ConversationModel extends window.Backbone
 
       if (didResponseChange) {
         if (response === messageRequestEnum.ACCEPT) {
-          // Only add a message when the user took an explicit action to accept
+          // Only add a message if the user unblocked this conversation, or took an
-          // the message request on one of their devices
+          // explicit action to accept the message request on one of their devices
-          if (!viaStorageServiceSync) {
+          if (!viaStorageServiceSync || didUnblock) {
             drop(
               this.addMessageRequestResponseEventMessage(
                 didUnblock

ts/services/storageRecordOps.ts

@@ -1,7 +1,7 @@
 // Copyright 2020 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import { isEqual, isNumber } from 'lodash';
+import { isEqual } from 'lodash';
 import Long from 'long';
 
 import { CallLinkRootKey } from '@signalapp/ringrtc';
@@ -1019,32 +1019,31 @@ export async function mergeGroupV2Record(
 
   details = details.concat(extraDetails);
 
-  const isGroupNewToUs = !isNumber(conversation.get('revision'));
-  const isFirstSync = !window.storage.get('storageFetchComplete');
-  const dropInitialJoinMessage = isFirstSync;
-
   if (isGroupV1(conversation.attributes)) {
     // If we found a GroupV1 conversation from this incoming GroupV2 record, we need to
     //   migrate it!
 
     // We don't await this because this could take a very long time, waiting for queues to
     //   empty, etc.
-    void waitThenRespondToGroupV2Migration({
+    drop(
-      conversation,
+      waitThenRespondToGroupV2Migration({
-    });
+        conversation,
-  } else if (isGroupNewToUs) {
+      })
-    // We don't need to update GroupV2 groups all the time. We fetch group state the first
+    );
-    //   time we hear about these groups, from then on we rely on incoming messages or
+  } else {
-    //   the user opening that conversation.
+    const isFirstSync = !window.storage.get('storageFetchComplete');
+    const dropInitialJoinMessage = isFirstSync;
 
     // We don't await this because this could take a very long time, waiting for queues to
     //   empty, etc.
-    void waitThenMaybeUpdateGroup(
+    drop(
-      {
+      waitThenMaybeUpdateGroup(
-        conversation,
+        {
-        dropInitialJoinMessage,
+          conversation,
-      },
+          dropInitialJoinMessage,
-      { viaFirstStorageSync: isFirstSync }
+        },
+        { viaFirstStorageSync: isFirstSync }
+      )
     );
   }
 

ts/test-both/util/diffArraysAsSets_test.ts

@@ -3,55 +3,71 @@
 
 import { assert } from 'chai';
 
-import { areArraysMatchingSets } from '../../util/areArraysMatchingSets';
+import { diffArraysAsSets } from '../../util/diffArraysAsSets';
 
-describe('areArraysMatchingSets', () => {
+function assertMatch<T>({
+  added,
+  removed,
+}: {
+  added: Array<T>;
+  removed: Array<T>;
+}) {
+  return added.length === 0 && removed.length === 0;
+}
+
+describe('diffArraysAsSets', () => {
   it('returns true if arrays are both empty', () => {
     const left: Array<string> = [];
     const right: Array<string> = [];
 
-    assert.isTrue(areArraysMatchingSets(left, right));
+    assertMatch(diffArraysAsSets(left, right));
   });
 
   it('returns true if arrays are equal', () => {
     const left = [1, 2, 3];
     const right = [1, 2, 3];
 
-    assert.isTrue(areArraysMatchingSets(left, right));
+    assertMatch(diffArraysAsSets(left, right));
   });
 
   it('returns true if arrays are equal but out of order', () => {
     const left = [1, 2, 3];
     const right = [3, 1, 2];
 
-    assert.isTrue(areArraysMatchingSets(left, right));
+    assertMatch(diffArraysAsSets(left, right));
   });
 
   it('returns true if arrays are equal but one has duplicates', () => {
     const left = [1, 2, 3, 1];
     const right = [1, 2, 3];
 
-    assert.isTrue(areArraysMatchingSets(left, right));
+    assertMatch(diffArraysAsSets(left, right));
   });
 
   it('returns false if first array has missing elements', () => {
     const left = [1, 2];
     const right = [1, 2, 3];
 
-    assert.isFalse(areArraysMatchingSets(left, right));
+    const { added, removed } = diffArraysAsSets(left, right);
+    assert.deepEqual(added, [3]);
+    assert.deepEqual(removed, []);
   });
 
   it('returns false if second array has missing elements', () => {
     const left = [1, 2, 3];
     const right = [1, 2];
 
-    assert.isFalse(areArraysMatchingSets(left, right));
+    const { added, removed } = diffArraysAsSets(left, right);
+    assert.deepEqual(added, []);
+    assert.deepEqual(removed, [3]);
   });
 
   it('returns false if second array is empty', () => {
     const left = [1, 2, 3];
     const right: Array<number> = [];
 
-    assert.isFalse(areArraysMatchingSets(left, right));
+    const { added, removed } = diffArraysAsSets(left, right);
+    assert.deepEqual(added, []);
+    assert.deepEqual(removed, [1, 2, 3]);
   });
 });

ts/textsecure/MessageReceiver.ts

@@ -137,7 +137,7 @@ import {
   ViewSyncEvent,
 } from './messageReceiverEvents';
 import * as log from '../logging/log';
-import { areArraysMatchingSets } from '../util/areArraysMatchingSets';
+import { diffArraysAsSets } from '../util/diffArraysAsSets';
 import { generateBlurHash } from '../util/generateBlurHash';
 import { TEXT_ATTACHMENT } from '../types/MIME';
 import type { SendTypesType } from '../util/handleMessageSend';
@@ -2187,16 +2187,6 @@ export default class MessageReceiver
     }
 
     const message = this.processDecrypted(envelope, msg);
-    const groupId = this.getProcessedGroupId(message);
-    const isBlocked = groupId ? this.isGroupBlocked(groupId) : false;
-
-    if (groupId && isBlocked) {
-      log.warn(
-        `Message ${getEnvelopeId(envelope)} ignored; destined for blocked group`
-      );
-      this.removeFromCache(envelope);
-      return undefined;
-    }
 
     const ev = new SentEvent(
       {
@@ -3849,42 +3839,63 @@ export default class MessageReceiver
     await this.dispatchAndWait(logId, contactSync);
   }
 
+  // This function calls applyMessageRequestResponse before setting window.storage so
+  // proper before/after logic can be applied within that function.
   private async handleBlocked(
     envelope: ProcessedEnvelope,
     blocked: Proto.SyncMessage.IBlocked
   ): Promise<void> {
-    const allIdentifiers = [];
-    let changed = false;
-
     const logId = `handleBlocked(${getEnvelopeId(envelope)})`;
+    const messageRequestEnum = Proto.SyncMessage.MessageRequestResponse.Type;
 
     logUnexpectedUrgentValue(envelope, 'blockSync');
 
+    function getAndApply(
+      type: Proto.SyncMessage.MessageRequestResponse.Type
+    ): (value: string) => Promise<void> {
+      return async item => {
+        const conversation = window.ConversationController.getOrCreate(
+          item,
+          'private'
+        );
+        await conversation.applyMessageRequestResponse(type, {
+          fromSync: true,
+        });
+      };
+    }
+
     if (blocked.numbers) {
       const previous = this.storage.get('blocked', []);
 
-      log.info(`${logId}: Blocking these numbers:`, blocked.numbers);
+      const { added, removed } = diffArraysAsSets(previous, blocked.numbers);
-      await this.storage.put('blocked', blocked.numbers);
+      if (added.length) {
-
+        await Promise.all(added.map(getAndApply(messageRequestEnum.BLOCK)));
-      if (!areArraysMatchingSets(previous, blocked.numbers)) {
-        changed = true;
-        allIdentifiers.push(...previous);
-        allIdentifiers.push(...blocked.numbers);
       }
+      if (removed.length) {
+        await Promise.all(removed.map(getAndApply(messageRequestEnum.ACCEPT)));
+      }
+
+      log.info(`${logId}: New e164 blocks:`, added);
+      log.info(`${logId}: New e164 unblocks:`, removed);
+      await this.storage.put('blocked', blocked.numbers);
     }
     if (blocked.acis) {
       const previous = this.storage.get('blocked-uuids', []);
       const acis = blocked.acis.map((aci, index) => {
         return normalizeAci(aci, `handleBlocked.acis.${index}`);
       });
-      log.info(`${logId}: Blocking these acis:`, acis);
-      await this.storage.put('blocked-uuids', acis);
 
-      if (!areArraysMatchingSets(previous, acis)) {
+      const { added, removed } = diffArraysAsSets(previous, acis);
-        changed = true;
+      if (added.length) {
-        allIdentifiers.push(...previous);
+        await Promise.all(added.map(getAndApply(messageRequestEnum.BLOCK)));
-        allIdentifiers.push(...blocked.acis);
       }
+      if (removed.length) {
+        await Promise.all(removed.map(getAndApply(messageRequestEnum.ACCEPT)));
+      }
+
+      log.info(`${logId}: New aci blocks:`, added);
+      log.info(`${logId}: New aci unblocks:`, removed);
+      await this.storage.put('blocked-uuids', acis);
     }
 
     if (blocked.groupIds) {
@@ -3898,27 +3909,55 @@ export default class MessageReceiver
           log.error(`${logId}: Received invalid groupId value`);
         }
       });
-      log.info(
-        `${logId}: Blocking these groups - v2:`,
-        groupIds.map(groupId => `groupv2(${groupId})`)
-      );
 
-      await this.storage.put('blocked-groups', groupIds);
+      const { added, removed } = diffArraysAsSets(previous, groupIds);
-
+      if (added.length) {
-      if (!areArraysMatchingSets(previous, groupIds)) {
+        await Promise.all(
-        changed = true;
+          added.map(async item => {
-        allIdentifiers.push(...previous);
+            const conversation = window.ConversationController.get(item);
-        allIdentifiers.push(...groupIds);
+            if (!conversation) {
+              log.warn(`${logId}: Group groupv2(${item}) not found!`);
+              return;
+            }
+            await conversation.applyMessageRequestResponse(
+              messageRequestEnum.BLOCK,
+              {
+                fromSync: true,
+              }
+            );
+          })
+        );
       }
+      if (removed.length) {
+        await Promise.all(
+          removed.map(async item => {
+            const conversation = window.ConversationController.get(item);
+            if (!conversation) {
+              log.warn(`${logId}: Group groupv2(${item}) not found!`);
+              return;
+            }
+            await conversation.applyMessageRequestResponse(
+              messageRequestEnum.ACCEPT,
+              {
+                fromSync: true,
+              }
+            );
+          })
+        );
+      }
+
+      log.info(
+        `${logId}: New groupId blocks:`,
+        added.map(groupId => `groupv2(${groupId})`)
+      );
+      log.info(
+        `${logId}: New groupId unblocks:`,
+        removed.map(groupId => `groupv2(${groupId})`)
+      );
+      await this.storage.put('blocked-groups', groupIds);
     }
 
     this.removeFromCache(envelope);
-
-    if (changed) {
-      log.info(`${logId}: Block list changed, forcing re-render.`);
-      const uniqueIdentifiers = Array.from(new Set(allIdentifiers));
-      void window.ConversationController.forceRerender(uniqueIdentifiers);
-    }
   }
 
   private isBlocked(number: string): boolean {

ts/util/areArraysMatchingSets.ts

@@ -1,24 +0,0 @@
-// Copyright 2020 Signal Messenger, LLC
-// SPDX-License-Identifier: AGPL-3.0-only
-
-export function areArraysMatchingSets<T>(
-  left: Array<T>,
-  right: Array<T>
-): boolean {
-  const leftSet = new Set(left);
-  const rightSet = new Set(right);
-
-  for (const item of leftSet) {
-    if (!rightSet.has(item)) {
-      return false;
-    }
-  }
-
-  for (const item of rightSet) {
-    if (!leftSet.has(item)) {
-      return false;
-    }
-  }
-
-  return true;
-}

ts/util/diffArraysAsSets.ts

@@ -0,0 +1,26 @@
+// Copyright 2020 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+export function diffArraysAsSets<T>(
+  starting: Array<T>,
+  current: Array<T>
+): { added: Array<T>; removed: Array<T> } {
+  const startingSet = new Set(starting);
+  const currentSet = new Set(current);
+
+  const removed = [];
+  for (const item of startingSet) {
+    if (!currentSet.has(item)) {
+      removed.push(item);
+    }
+  }
+
+  const added = [];
+  for (const item of currentSet) {
+    if (!startingSet.has(item)) {
+      added.push(item);
+    }
+  }
+
+  return { added, removed };
+}