Fixes pinned conversations sync

ts/services/storage.ts

@@ -1,7 +1,7 @@
 // Copyright 2020-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import { debounce, isNumber, partition } from 'lodash';
+import { debounce, isNumber } from 'lodash';
 import pMap from 'p-map';
 
 import Crypto from '../textsecure/Crypto';
@@ -654,46 +654,60 @@ async function processManifest(
     );
   });
 
-  const localKeys = window
+  const remoteKeys = new Set(remoteKeysTypeMap.keys());
-    .getConversations()
+  const localKeys: Set<string> = new Set();
-    .map((conversation: ConversationModel) => conversation.get('storageID'))
+
-    .filter(Boolean);
+  const conversations = window.getConversations();
+  conversations.forEach((conversation: ConversationModel) => {
+    const storageID = conversation.get('storageID');
+    if (storageID) {
+      localKeys.add(storageID);
+    }
+  });
 
   const unknownRecordsArray: ReadonlyArray<UnknownRecord> =
     window.storage.get('storage-service-unknown-records') || [];
 
-  unknownRecordsArray.forEach((record: UnknownRecord) => {
+  const stillUnknown = unknownRecordsArray.filter((record: UnknownRecord) => {
     // Do not include any unknown records that we already support
     if (!validRecordTypes.has(record.itemType)) {
-      localKeys.push(record.storageID);
+      localKeys.add(record.storageID);
+      return false;
     }
+    return true;
   });
 
   window.log.info(
-    'storageService.processManifest: local keys:',
+    'storageService.processManifest: local records:',
-    localKeys.length
+    conversations.length
   );
-
   window.log.info(
-    'storageService.processManifest: incl. unknown records:',
+    'storageService.processManifest: local keys:',
-    unknownRecordsArray.length
+    localKeys.size
+  );
+  window.log.info(
+    'storageService.processManifest: unknown records:',
+    stillUnknown.length
+  );
+  window.log.info(
+    'storageService.processManifest: remote keys:',
+    remoteKeys.size
   );
-
-  const remoteKeys = Array.from(remoteKeysTypeMap.keys());
 
   const remoteOnlySet: Set<string> = new Set();
   remoteKeys.forEach((key: string) => {
-    if (!localKeys.includes(key)) {
+    if (!localKeys.has(key)) {
       remoteOnlySet.add(key);
     }
   });
 
+  window.log.info(
+    'storageService.processManifest: remote ids:',
+    Array.from(remoteOnlySet).map(redactStorageID).join(',')
+  );
+
   const remoteOnlyRecords = new Map<string, RemoteRecord>();
   remoteOnlySet.forEach(storageID => {
-    window.log.info(
-      'storageService.processManifest: remote key',
-      redactStorageID(storageID)
-    );
     remoteOnlyRecords.set(storageID, {
       storageID,
       itemType: remoteKeysTypeMap.get(storageID),
@@ -703,13 +717,36 @@ async function processManifest(
   // if the remote only keys are larger or equal to our local keys then it
   // was likely a forced push of storage service. We keep track of these
   // merges so that we can detect possible infinite loops
-  const isForcePushed = remoteOnlyRecords.size >= localKeys.length;
+  const isForcePushed = remoteOnlyRecords.size >= localKeys.size;
 
   const conflictCount = await processRemoteRecords(
     remoteOnlyRecords,
     isForcePushed
   );
-  const hasConflicts = conflictCount !== 0;
+
+  let hasConflicts = conflictCount !== 0;
+
+  // Post-merge, if our local records contain any storage IDs that were not
+  // present in the remote manifest then we'll need to clear it, generate a
+  // new storageID for that record, and upload.
+  // This might happen if a device pushes a manifest which doesn't contain
+  // the keys that we have in our local database.
+  window.getConversations().forEach((conversation: ConversationModel) => {
+    const storageID = conversation.get('storageID');
+    if (storageID && !remoteKeys.has(storageID)) {
+      window.log.info(
+        'storageService.processManifest: local key was not in remote manifest',
+        redactStorageID(storageID),
+        conversation.idForLogging()
+      );
+      conversation.set({
+        needsStorageServiceSync: true,
+        storageID: undefined,
+      });
+      updateConversation(conversation.attributes);
+      hasConflicts = true;
+    }
+  });
 
   return hasConflicts;
 }
@@ -722,7 +759,7 @@ async function processRemoteRecords(
   const storageKey = base64ToArrayBuffer(storageKeyBase64);
 
   window.log.info(
-    'storageService.processRemoteRecords: remote keys',
+    'storageService.processRemoteRecords: remote only keys',
     remoteOnlyRecords.size
   );
 
@@ -809,12 +846,12 @@ async function processRemoteRecords(
     { concurrency: 5 }
   );
 
-  // Merge Account records last
+  // Merge Account records last since it contains the pinned conversations
-  const sortedStorageItems = ([] as Array<MergeableItemType>).concat(
+  // and we need all other records merged first before we can find the pinned
-    ...partition(
+  // records in our db
-      decryptedStorageItems,
+  const ITEM_TYPE = window.textsecure.protobuf.ManifestRecord.Identifier.Type;
-      storageRecord => storageRecord.storageRecord.account === undefined
+  const sortedStorageItems = decryptedStorageItems.sort((_, b) =>
-    )
+    b.itemType === ITEM_TYPE.ACCOUNT ? -1 : 1
   );
 
   try {

ts/services/storageRecordOps.ts

@@ -32,8 +32,8 @@ import {
   PhoneNumberDiscoverability,
   parsePhoneNumberDiscoverability,
 } from '../util/phoneNumberDiscoverability';
+import { arePinnedConversationsEqual } from '../util/arePinnedConversationsEqual';
 import { ConversationModel } from '../models/conversations';
-import { ConversationAttributesTypeType } from '../model-types.d';
 
 const { updateConversation } = dataInterface;
 
@@ -364,6 +364,48 @@ function doRecordsConflict(
     const localValue = localRecord[key];
     const remoteValue = remoteRecord[key];
 
+    // Sometimes we have a ByteBuffer and an ArrayBuffer, this ensures that we
+    // are comparing them both equally by converting them into base64 string.
+    if (Object.prototype.toString.call(localValue) === '[object ArrayBuffer]') {
+      const areEqual =
+        arrayBufferToBase64(localValue) === arrayBufferToBase64(remoteValue);
+      if (!areEqual) {
+        window.log.info(
+          'storageService.doRecordsConflict: Conflict found for ArrayBuffer',
+          key,
+          idForLogging
+        );
+      }
+      return hasConflict || !areEqual;
+    }
+
+    // If both types are Long we can use Long's equals to compare them
+    if (
+      window.dcodeIO.Long.isLong(localValue) &&
+      window.dcodeIO.Long.isLong(remoteValue)
+    ) {
+      const areEqual = localValue.equals(remoteValue);
+      if (!areEqual) {
+        window.log.info(
+          'storageService.doRecordsConflict: Conflict found for Long',
+          key,
+          idForLogging
+        );
+      }
+      return hasConflict || !areEqual;
+    }
+
+    if (key === 'pinnedConversations') {
+      const areEqual = arePinnedConversationsEqual(localValue, remoteValue);
+      if (!areEqual) {
+        window.log.info(
+          'storageService.doRecordsConflict: Conflict found for pinnedConversations',
+          idForLogging
+        );
+      }
+      return hasConflict || !areEqual;
+    }
+
     if (localValue === remoteValue) {
       return hasConflict || false;
     }
@@ -373,7 +415,10 @@ function doRecordsConflict(
     // conflicting.
     if (
       remoteValue === null &&
-      (localValue === false || localValue === '' || localValue === 0)
+      (localValue === false ||
+        localValue === '' ||
+        localValue === 0 ||
+        (window.dcodeIO.Long.isLong(localValue) && localValue.toNumber() === 0))
     ) {
       return hasConflict || false;
     }
@@ -805,7 +850,6 @@ export async function mergeAccountRecord(
     const remotelyPinnedConversationPromises = pinnedConversations.map(
       async pinnedConversation => {
         let conversationId;
-        let conversationType: ConversationAttributesTypeType = 'private';
 
         switch (pinnedConversation.identifier) {
           case 'contact': {
@@ -815,7 +859,6 @@ export async function mergeAccountRecord(
             conversationId = window.ConversationController.ensureContactIds(
               pinnedConversation.contact
             );
-            conversationType = 'private';
             break;
           }
           case 'legacyGroupId': {
@@ -823,7 +866,6 @@ export async function mergeAccountRecord(
               throw new Error('mergeAccountRecord: no legacyGroupId found');
             }
             conversationId = pinnedConversation.legacyGroupId.toBinary();
-            conversationType = 'group';
             break;
           }
           case 'groupMasterKey': {
@@ -835,7 +877,6 @@ export async function mergeAccountRecord(
             const groupId = arrayBufferToBase64(groupFields.id);
 
             conversationId = groupId;
-            conversationType = 'group';
             break;
           }
           default: {
@@ -853,13 +894,6 @@ export async function mergeAccountRecord(
           return undefined;
         }
 
-        if (conversationType === 'private') {
-          return window.ConversationController.getOrCreateAndWait(
-            conversationId,
-            conversationType
-          );
-        }
-
         return window.ConversationController.get(conversationId);
       }
     );

ts/test-both/util/arePinnedConversationsEqual_test.ts

@@ -0,0 +1,120 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import { arePinnedConversationsEqual } from '../../util/arePinnedConversationsEqual';
+import { PinnedConversationClass } from '../../textsecure.d';
+
+describe('arePinnedConversationsEqual', () => {
+  it('is equal if both have same values at same indices', () => {
+    const localValue = [
+      {
+        identifier: 'contact' as const,
+        contact: {
+          uuid: '72313cde-2784-4a6f-a92a-abbe23763a60',
+          e164: '+13055551234',
+        },
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+      {
+        identifier: 'groupMasterKey' as const,
+        groupMasterKey: new ArrayBuffer(32),
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+    ];
+    const remoteValue = [
+      {
+        identifier: 'contact' as const,
+        contact: {
+          uuid: '72313cde-2784-4a6f-a92a-abbe23763a60',
+          e164: '+13055551234',
+        },
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+      {
+        identifier: 'groupMasterKey' as const,
+        groupMasterKey: new ArrayBuffer(32),
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+    ];
+
+    assert.isTrue(arePinnedConversationsEqual(localValue, remoteValue));
+  });
+
+  it('is not equal if values are mixed', () => {
+    const localValue = [
+      {
+        identifier: 'contact' as const,
+        contact: {
+          uuid: '72313cde-2784-4a6f-a92a-abbe23763a60',
+          e164: '+13055551234',
+        },
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+      {
+        identifier: 'contact' as const,
+        contact: {
+          uuid: 'f59a9fed-9e91-4bb4-a015-d49e58b47e25',
+          e164: '+17865554321',
+        },
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+    ];
+    const remoteValue = [
+      {
+        identifier: 'contact' as const,
+        contact: {
+          uuid: 'f59a9fed-9e91-4bb4-a015-d49e58b47e25',
+          e164: '+17865554321',
+        },
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+      {
+        identifier: 'contact' as const,
+        contact: {
+          uuid: '72313cde-2784-4a6f-a92a-abbe23763a60',
+          e164: '+13055551234',
+        },
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+    ];
+
+    assert.isFalse(arePinnedConversationsEqual(localValue, remoteValue));
+  });
+
+  it('is not equal if lengths are not same', () => {
+    const localValue = [
+      {
+        identifier: 'contact' as const,
+        contact: {
+          uuid: '72313cde-2784-4a6f-a92a-abbe23763a60',
+          e164: '+13055551234',
+        },
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+    ];
+    const remoteValue: Array<PinnedConversationClass> = [];
+    assert.isFalse(arePinnedConversationsEqual(localValue, remoteValue));
+  });
+
+  it('is not equal if content does not match', () => {
+    const localValue = [
+      {
+        identifier: 'contact' as const,
+        contact: {
+          uuid: '72313cde-2784-4a6f-a92a-abbe23763a60',
+          e164: '+13055551234',
+        },
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+    ];
+    const remoteValue = [
+      {
+        identifier: 'groupMasterKey' as const,
+        groupMasterKey: new ArrayBuffer(32),
+        toArrayBuffer: () => new ArrayBuffer(0),
+      },
+    ];
+    assert.isFalse(arePinnedConversationsEqual(localValue, remoteValue));
+  });
+});

ts/util/arePinnedConversationsEqual.ts

@@ -0,0 +1,46 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { arrayBufferToBase64 } from '../Crypto';
+import { PinnedConversationClass } from '../textsecure.d';
+
+export function arePinnedConversationsEqual(
+  localValue: Array<PinnedConversationClass>,
+  remoteValue: Array<PinnedConversationClass>
+): boolean {
+  if (localValue.length !== remoteValue.length) {
+    return false;
+  }
+  return localValue.every(
+    (localPinnedConversation: PinnedConversationClass, index: number) => {
+      const remotePinnedConversation = remoteValue[index];
+      if (
+        localPinnedConversation.identifier !==
+        remotePinnedConversation.identifier
+      ) {
+        return false;
+      }
+      switch (localPinnedConversation.identifier) {
+        case 'contact':
+          return (
+            localPinnedConversation.contact &&
+            remotePinnedConversation.contact &&
+            localPinnedConversation.contact.uuid ===
+              remotePinnedConversation.contact.uuid
+          );
+        case 'groupMasterKey':
+          return (
+            arrayBufferToBase64(localPinnedConversation.groupMasterKey) ===
+            arrayBufferToBase64(remotePinnedConversation.groupMasterKey)
+          );
+        case 'legacyGroupId':
+          return (
+            arrayBufferToBase64(localPinnedConversation.legacyGroupId) ===
+            arrayBufferToBase64(remotePinnedConversation.legacyGroupId)
+          );
+        default:
+          return false;
+      }
+    }
+  );
+}

ts/window.d.ts

@@ -572,8 +572,11 @@ export type DCodeIOType = {
     Long: DCodeIOType['Long'];
   };
   Long: Long & {
+    equals: (other: Long | number | string) => boolean;
     fromBits: (low: number, high: number, unsigned: boolean) => number;
+    fromNumber: (value: number, unsigned?: boolean) => Long;
     fromString: (str: string | null) => Long;
+    isLong: (obj: unknown) => obj is Long;
   };
 };