From 610ebdd1e30d3c5a4863f365ef7f2fa8660e9ec7 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Tue, 12 Apr 2022 17:50:17 -0700 Subject: [PATCH] Batch storage item read requests --- package.json | 2 +- ts/services/storage.ts | 43 +++++++---- ts/services/storageConstants.ts | 5 ++ ts/services/storageRecordOps.ts | 6 +- ts/test-mock/bootstrap.ts | 10 ++- ts/test-mock/storage/fixtures.ts | 9 ++- ts/test-mock/storage/max_read_keys_test.ts | 90 ++++++++++++++++++++++ yarn.lock | 8 +- 8 files changed, 144 insertions(+), 29 deletions(-) create mode 100644 ts/services/storageConstants.ts create mode 100644 ts/test-mock/storage/max_read_keys_test.ts diff --git a/package.json b/package.json index d992c44db..1ea399b4b 100644 --- a/package.json +++ b/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", diff --git a/ts/services/storage.ts b/ts/services/storage.ts index a9281ce04..08c5de203 100644 --- a/ts/services/storage.ts +++ b/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 + ): Promise> => { + 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(remoteOnlyRecords.keys()); - const storageItems = Proto.StorageItems.decode(storageItemsBuffer); - const decryptedStorageItems = await pMap( - storageItems.items, + storageItems, async ( storageRecordWrapper: Proto.IStorageItem ): Promise => { diff --git a/ts/services/storageConstants.ts b/ts/services/storageConstants.ts new file mode 100644 index 000000000..81cd2a1c8 --- /dev/null +++ b/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; diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index 3fbdefa3a..e96b27349 100644 --- a/ts/services/storageRecordOps.ts +++ b/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: diff --git a/ts/test-mock/bootstrap.ts b/ts/test-mock/bootstrap.ts index 87314d7bb..259a09e96 100644 --- a/ts/test-mock/bootstrap.ts +++ b/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 & // 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; @@ -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, diff --git a/ts/test-mock/storage/fixtures.ts b/ts/test-mock/storage/fixtures.ts index f01956b37..1badb6396 100644 --- a/ts/test-mock/storage/fixtures.ts +++ b/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; + members: ReadonlyArray; }>; // @@ -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 { +export async function initStorage( + options?: BootstrapOptions +): Promise { // Creates primary device, contacts - const bootstrap = new Bootstrap(); + const bootstrap = new Bootstrap(options); await bootstrap.init(); diff --git a/ts/test-mock/storage/max_read_keys_test.ts b/ts/test-mock/storage/max_read_keys_test.ts new file mode 100644 index 000000000..d790c9e37 --- /dev/null +++ b/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); + }); +}); diff --git a/yarn.lock b/yarn.lock index 7a9dcb8f6..9d8f3fec0 100644 --- a/yarn.lock +++ b/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"