From e90d987e2762f36e14a6c0cf4b31db0bbce9cc3d Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Wed, 2 Oct 2024 10:29:26 -0500 Subject: [PATCH] Storage Service: Fetch updates on any group record merge Co-authored-by: Scott Nonnenberg --- ts/models/conversations.ts | 6 +- ts/services/storageRecordOps.ts | 35 +++-- ...gSets_test.ts => diffArraysAsSets_test.ts} | 34 +++-- ts/textsecure/MessageReceiver.ts | 125 ++++++++++++------ ts/util/areArraysMatchingSets.ts | 24 ---- ts/util/diffArraysAsSets.ts | 26 ++++ 6 files changed, 153 insertions(+), 97 deletions(-) rename ts/test-both/util/{areArraysMatchingSets_test.ts => diffArraysAsSets_test.ts} (54%) delete mode 100644 ts/util/areArraysMatchingSets.ts create mode 100644 ts/util/diffArraysAsSets.ts diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index eb2916a95..9f3568db4 100644 --- a/ts/models/conversations.ts +++ b/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 - // the message request on one of their devices - if (!viaStorageServiceSync) { + // Only add a message if the user unblocked this conversation, or took an + // explicit action to accept the message request on one of their devices + if (!viaStorageServiceSync || didUnblock) { drop( this.addMessageRequestResponseEventMessage( didUnblock diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index dee4348c5..6a48dda24 100644 --- a/ts/services/storageRecordOps.ts +++ b/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({ - 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 - // the user opening that conversation. + drop( + waitThenRespondToGroupV2Migration({ + conversation, + }) + ); + } else { + 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( - { - conversation, - dropInitialJoinMessage, - }, - { viaFirstStorageSync: isFirstSync } + drop( + waitThenMaybeUpdateGroup( + { + conversation, + dropInitialJoinMessage, + }, + { viaFirstStorageSync: isFirstSync } + ) ); } diff --git a/ts/test-both/util/areArraysMatchingSets_test.ts b/ts/test-both/util/diffArraysAsSets_test.ts similarity index 54% rename from ts/test-both/util/areArraysMatchingSets_test.ts rename to ts/test-both/util/diffArraysAsSets_test.ts index e832d2fc5..52d0918d9 100644 --- a/ts/test-both/util/areArraysMatchingSets_test.ts +++ b/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({ + added, + removed, +}: { + added: Array; + removed: Array; +}) { + return added.length === 0 && removed.length === 0; +} + +describe('diffArraysAsSets', () => { it('returns true if arrays are both empty', () => { const left: Array = []; const right: Array = []; - 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 = []; - assert.isFalse(areArraysMatchingSets(left, right)); + const { added, removed } = diffArraysAsSets(left, right); + assert.deepEqual(added, []); + assert.deepEqual(removed, [1, 2, 3]); }); }); diff --git a/ts/textsecure/MessageReceiver.ts b/ts/textsecure/MessageReceiver.ts index 0c24896be..f39ff5cd7 100644 --- a/ts/textsecure/MessageReceiver.ts +++ b/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 { - 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 { + 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); - await this.storage.put('blocked', blocked.numbers); - - if (!areArraysMatchingSets(previous, blocked.numbers)) { - changed = true; - allIdentifiers.push(...previous); - allIdentifiers.push(...blocked.numbers); + const { added, removed } = diffArraysAsSets(previous, blocked.numbers); + if (added.length) { + await Promise.all(added.map(getAndApply(messageRequestEnum.BLOCK))); } + 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)) { - changed = true; - allIdentifiers.push(...previous); - allIdentifiers.push(...blocked.acis); + const { added, removed } = diffArraysAsSets(previous, acis); + if (added.length) { + await Promise.all(added.map(getAndApply(messageRequestEnum.BLOCK))); } + 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); - - if (!areArraysMatchingSets(previous, groupIds)) { - changed = true; - allIdentifiers.push(...previous); - allIdentifiers.push(...groupIds); + const { added, removed } = diffArraysAsSets(previous, groupIds); + if (added.length) { + await Promise.all( + added.map(async item => { + const conversation = window.ConversationController.get(item); + 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 { diff --git a/ts/util/areArraysMatchingSets.ts b/ts/util/areArraysMatchingSets.ts deleted file mode 100644 index a6168dc30..000000000 --- a/ts/util/areArraysMatchingSets.ts +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2020 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only - -export function areArraysMatchingSets( - left: Array, - right: Array -): 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; -} diff --git a/ts/util/diffArraysAsSets.ts b/ts/util/diffArraysAsSets.ts new file mode 100644 index 000000000..c7b2ccfe2 --- /dev/null +++ b/ts/util/diffArraysAsSets.ts @@ -0,0 +1,26 @@ +// Copyright 2020 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +export function diffArraysAsSets( + starting: Array, + current: Array +): { added: Array; removed: Array } { + 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 }; +}