Batch storage item read requests

package.json

@@ -189,7 +189,7 @@
     "@chanzuckerberg/axe-storybook-testing": "3.0.2",
     "@electron/fuses": "1.5.0",
     "@mixer/parallel-prettier": "2.0.1",
-    "@signalapp/mock-server": "1.2.1",
+    "@signalapp/mock-server": "1.3.0",
     "@storybook/addon-actions": "5.1.11",
     "@storybook/addon-knobs": "5.1.11",
     "@storybook/addons": "5.1.11",

ts/services/storage.ts

@@ -1,7 +1,7 @@
 // Copyright 2020-2022 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import { debounce, isNumber } from 'lodash';
+import { debounce, isNumber, chunk } from 'lodash';
 import pMap from 'p-map';
 import Long from 'long';
 
@@ -26,6 +26,7 @@ import {
   toGroupV2Record,
 } from './storageRecordOps';
 import type { MergeResultType } from './storageRecordOps';
+import { MAX_READ_KEYS } from './storageConstants';
 import type { ConversationModel } from '../models/conversations';
 import { strictAssert } from '../util/assert';
 import { dropNull } from '../util/dropNull';
@@ -979,26 +980,36 @@ async function processRemoteRecords(
       `count=${remoteOnlyRecords.size}`
   );
 
-  const readOperation = new Proto.ReadOperation();
-  readOperation.readKey = Array.from(remoteOnlyRecords.keys()).map(
-    Bytes.fromBase64
-  );
-
   const credentials = window.storage.get('storageCredentials');
-  const storageItemsBuffer =
-    await window.textsecure.messaging.getStorageRecords(
-      Proto.ReadOperation.encode(readOperation).finish(),
-      {
-        credentials,
-      }
-    );
+  const batches = chunk(Array.from(remoteOnlyRecords.keys()), MAX_READ_KEYS);
+
+  const storageItems = (
+    await pMap(
+      batches,
+      async (
+        batch: ReadonlyArray<string>
+      ): Promise<Array<Proto.IStorageItem>> => {
+        const readOperation = new Proto.ReadOperation();
+        readOperation.readKey = batch.map(Bytes.fromBase64);
+
+        const storageItemsBuffer =
+          await window.textsecure.messaging.getStorageRecords(
+            Proto.ReadOperation.encode(readOperation).finish(),
+            {
+              credentials,
+            }
+          );
+
+        return Proto.StorageItems.decode(storageItemsBuffer).items ?? [];
+      },
+      { concurrency: 5 }
+    )
+  ).flat();
 
   const missingKeys = new Set<string>(remoteOnlyRecords.keys());
 
-  const storageItems = Proto.StorageItems.decode(storageItemsBuffer);
-
   const decryptedStorageItems = await pMap(
-    storageItems.items,
+    storageItems,
     async (
       storageRecordWrapper: Proto.IStorageItem
     ): Promise<MergeableItemType> => {

ts/services/storageConstants.ts

@@ -0,0 +1,5 @@
+// Copyright 2022 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+// Server limit is 5120, but we set this to a safer lower amount
+export const MAX_READ_KEYS = 2500;

ts/services/storageRecordOps.ts

@@ -832,9 +832,7 @@ export async function mergeContactRecord(
     }
   }
 
-  // Update verified status unconditionally to make sure we will take the
-  // latest identity key from the manifest.
-  {
+  if (contactRecord.identityKey) {
     const verified = await conversation.safeGetVerified();
     const storageServiceVerified = contactRecord.identityState || 0;
     const verifiedOptions = {
@@ -847,6 +845,8 @@ export async function mergeContactRecord(
       details.push(`updating verified state to=${verified}`);
     }
 
+    // Update verified status unconditionally to make sure we will take the
+    // latest identity key from the manifest.
     let keyChange: boolean;
     switch (storageServiceVerified) {
       case STATE_ENUM.VERIFIED:

ts/test-mock/bootstrap.ts

@@ -9,8 +9,9 @@ import createDebug from 'debug';
 
 import type { Device, PrimaryDevice } from '@signalapp/mock-server';
 import { Server, loadCertificates } from '@signalapp/mock-server';
-import { App } from './playwright';
+import { MAX_READ_KEYS as MAX_STORAGE_READ_KEYS } from '../services/storageConstants';
 import * as durations from '../util/durations';
+import { App } from './playwright';
 
 const debug = createDebug('mock:bootstrap');
 
@@ -97,7 +98,7 @@ type BootstrapInternalOptions = Pick<BootstrapOptions, 'extraConfig'> &
 // the same between different test runs.
 //
 export class Bootstrap {
-  public readonly server = new Server();
+  public readonly server: Server;
 
   private readonly options: BootstrapInternalOptions;
   private privContacts?: ReadonlyArray<PrimaryDevice>;
@@ -107,6 +108,11 @@ export class Bootstrap {
   private timestamp: number = Date.now() - durations.MONTH;
 
   constructor(options: BootstrapOptions = {}) {
+    this.server = new Server({
+      // Limit number of storage read keys for easier testing
+      maxStorageReadKeys: MAX_STORAGE_READ_KEYS,
+    });
+
     this.options = {
       linkedDevices: 5,
       contactCount: MAX_CONTACTS,

ts/test-mock/storage/fixtures.ts

@@ -6,6 +6,7 @@ import type { Group, PrimaryDevice } from '@signalapp/mock-server';
 import { StorageState, Proto } from '@signalapp/mock-server';
 import { App } from '../playwright';
 import { Bootstrap } from '../bootstrap';
+import type { BootstrapOptions } from '../bootstrap';
 
 export const debug = createDebug('mock:test-storage');
 
@@ -17,7 +18,7 @@ export type InitStorageResultType = Readonly<{
   bootstrap: Bootstrap;
   app: App;
   group: Group;
-  members: Array<PrimaryDevice>;
+  members: ReadonlyArray<PrimaryDevice>;
 }>;
 
 //
@@ -30,9 +31,11 @@ export type InitStorageResultType = Readonly<{
 // In addition to above, this function will queue one incoming message in the
 // group, and one for the first contact (so that both will appear in the left
 // pane).
-export async function initStorage(): Promise<InitStorageResultType> {
+export async function initStorage(
+  options?: BootstrapOptions
+): Promise<InitStorageResultType> {
   // Creates primary device, contacts
-  const bootstrap = new Bootstrap();
+  const bootstrap = new Bootstrap(options);
 
   await bootstrap.init();
 

ts/test-mock/storage/max_read_keys_test.ts

@@ -0,0 +1,90 @@
+// Copyright 2022 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import { Proto } from '@signalapp/mock-server';
+
+import * as durations from '../../util/durations';
+import { UUID } from '../../types/UUID';
+import { MAX_READ_KEYS } from '../../services/storageConstants';
+import type { App, Bootstrap } from './fixtures';
+import { initStorage, debug } from './fixtures';
+
+const IdentifierType = Proto.ManifestRecord.Identifier.Type;
+
+describe('storage service', function needsName() {
+  this.timeout(durations.MINUTE);
+
+  let bootstrap: Bootstrap;
+  let app: App;
+
+  beforeEach(async () => {
+    ({ bootstrap, app } = await initStorage());
+  });
+
+  afterEach(async () => {
+    await app.close();
+    await bootstrap.teardown();
+  });
+
+  it('should receive all contacts despite low read keys limit', async () => {
+    debug('prepare for a slow test');
+
+    const { phone, contacts } = bootstrap;
+    const firstContact = contacts[0];
+    const lastContact = contacts[contacts.length - 1];
+
+    const window = await app.getWindow();
+
+    const leftPane = window.locator('.left-pane-wrapper');
+
+    debug('wait for first contact to be pinned in the left pane');
+    await leftPane
+      .locator(
+        '_react=ConversationListItem' +
+          '[isPinned = true] ' +
+          `[title = ${JSON.stringify(firstContact.profileName)}]`
+      )
+      .waitFor();
+
+    {
+      let state = await phone.expectStorageState('consistency check');
+
+      debug('generating a lot of fake contacts');
+      for (let i = 0; i < MAX_READ_KEYS + 1; i += 1) {
+        state = state.addRecord({
+          type: IdentifierType.CONTACT,
+          record: {
+            contact: {
+              serviceUuid: UUID.generate().toString(),
+            },
+          },
+        });
+      }
+
+      debug('pinning last contact');
+      state = state.pin(lastContact);
+
+      await phone.setStorageState(state);
+
+      debug('sending fetch storage');
+      await phone.sendFetchStorage({
+        timestamp: bootstrap.getTimestamp(),
+      });
+    }
+
+    debug('wait for last contact to be pinned in the left pane');
+    await leftPane
+      .locator(
+        '_react=ConversationListItem' +
+          '[isPinned = true] ' +
+          `[title = ${JSON.stringify(lastContact.profileName)}]`
+      )
+      .waitFor({ timeout: durations.MINUTE });
+
+    debug('Verifying the final manifest version');
+    const finalState = await phone.expectStorageState('consistency check');
+
+    assert.strictEqual(finalState.version, 2);
+  });
+});

yarn.lock

@@ -1372,10 +1372,10 @@
     node-gyp-build "^4.2.3"
     uuid "^8.3.0"
 
-"@signalapp/mock-server@1.2.1":
-  version "1.2.1"
-  resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-1.2.1.tgz#20fd9f1efded52155ad3d55b7e739d4bfcf1953f"
-  integrity sha512-TR2l3+6rSQ3+jXGhrPTQ/QIk1ygKro5CrEg4X8A8j68V/uPxoa1b8a4EGBS6swHxw26Wh1l0DZUPoOGXhdM9Qg==
+"@signalapp/mock-server@1.3.0":
+  version "1.3.0"
+  resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-1.3.0.tgz#288a994c4f5c26c4c2680289af471e08746ac353"
+  integrity sha512-ix3GO0lytE02nWLj1fKY3UhKM3lCynhvF2LVNHEiMen9wurVyb8mVcmBDb9zRBi63tZmFLAq/IQEYrc1OK3ZJQ==
   dependencies:
     "@signalapp/libsignal-client" "0.15.0"
     debug "^4.3.2"