diff --git a/ts/ConversationController.ts b/ts/ConversationController.ts index 506ed317a16..1f6b2af16b2 100644 --- a/ts/ConversationController.ts +++ b/ts/ConversationController.ts @@ -17,7 +17,7 @@ import type { ConversationModel } from './models/conversations'; import dataInterface from './sql/Client'; import * as log from './logging/log'; import * as Errors from './types/errors'; -import { getContactId } from './messages/helpers'; +import { getAuthorId } from './messages/helpers'; import { maybeDeriveGroupV2Id } from './groups'; import { assertDev, strictAssert } from './util/assert'; import { drop } from './util/drop'; @@ -1236,7 +1236,7 @@ export class ConversationController { targetTimestamp: number ): Promise { const messages = await getMessagesBySentAt(targetTimestamp); - const targetMessage = messages.find(m => getContactId(m) === targetFromId); + const targetMessage = messages.find(m => getAuthorId(m) === targetFromId); if (targetMessage) { return this.get(targetMessage.conversationId); diff --git a/ts/background.ts b/ts/background.ts index d4e1772f12b..6be63155ce4 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -59,7 +59,7 @@ import { RoutineProfileRefresher } from './routineProfileRefresh'; import { isOlderThan } from './util/timestamp'; import { isValidReactionEmoji } from './reactions/isValidReactionEmoji'; import type { ConversationModel } from './models/conversations'; -import { getContact, isIncoming } from './messages/helpers'; +import { getAuthor, isIncoming } from './messages/helpers'; import { migrateMessageData } from './messages/migrateMessageData'; import { createBatcher } from './util/batcher'; import { @@ -2329,7 +2329,7 @@ export async function startApp(): Promise { const message = initIncomingMessage(data, messageDescriptor); if (isIncoming(message.attributes)) { - const sender = getContact(message.attributes); + const sender = getAuthor(message.attributes); strictAssert(sender, 'MessageModel has no sender'); const serviceIdKind = window.textsecure.storage.user.getOurServiceIdKind( @@ -2395,7 +2395,7 @@ export async function startApp(): Promise { fromId: fromConversation.id, remove: reaction.remove, source: ReactionSource.FromSomeoneElse, - storyReactionMessage: message, + generatedMessageForStoryReaction: message, targetAuthorAci, targetTimestamp: reaction.targetTimestamp, receivedAtDate: data.receivedAtDate, @@ -2784,7 +2784,7 @@ export async function startApp(): Promise { fromId: window.ConversationController.getOurConversationIdOrThrow(), remove: reaction.remove, source: ReactionSource.FromSync, - storyReactionMessage: message, + generatedMessageForStoryReaction: message, targetAuthorAci, targetTimestamp: reaction.targetTimestamp, receivedAtDate: data.receivedAtDate, diff --git a/ts/messageModifiers/Deletes.ts b/ts/messageModifiers/Deletes.ts index f14a3c8e409..7f3da01dca5 100644 --- a/ts/messageModifiers/Deletes.ts +++ b/ts/messageModifiers/Deletes.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: AGPL-3.0-only import type { MessageAttributesType } from '../model-types.d'; -import { getContactId } from '../messages/helpers'; +import { getAuthorId } from '../messages/helpers'; import * as log from '../logging/log'; import * as Errors from '../types/errors'; import { deleteForEveryone } from '../util/deleteForEveryone'; @@ -32,7 +32,7 @@ export function forMessage( const matchingDeletes = deleteValues.filter(item => { return ( - item.fromId === getContactId(messageAttributes) && + item.fromId === getAuthorId(messageAttributes) && sentTimestamps.has(item.targetSentTimestamp) ); }); @@ -77,7 +77,7 @@ export async function onDelete(del: DeleteAttributesType): Promise { ); const targetMessage = messages.find( - m => del.fromId === getContactId(m) && !m.deletedForEveryone + m => del.fromId === getAuthorId(m) && !m.deletedForEveryone ); if (!targetMessage) { diff --git a/ts/messageModifiers/Edits.ts b/ts/messageModifiers/Edits.ts index e123078a890..dbd9d76d88d 100644 --- a/ts/messageModifiers/Edits.ts +++ b/ts/messageModifiers/Edits.ts @@ -5,7 +5,7 @@ import type { MessageAttributesType } from '../model-types.d'; import * as Errors from '../types/errors'; import * as log from '../logging/log'; import { drop } from '../util/drop'; -import { getContactId } from '../messages/helpers'; +import { getAuthorId } from '../messages/helpers'; import { handleEditMessage } from '../util/handleEditMessage'; import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp'; import { @@ -55,7 +55,7 @@ export function forMessage( const matchingEdits = editValues.filter(item => { return ( item.targetSentTimestamp === sentAt && - item.fromId === getContactId(messageAttributes) + item.fromId === getAuthorId(messageAttributes) ); }); @@ -125,7 +125,7 @@ export async function onEdit(edit: EditAttributesType): Promise { const targetMessage = messages.find( m => edit.conversationId === m.conversationId && - edit.fromId === getContactId(m) + edit.fromId === getAuthorId(m) ); if (!targetMessage) { diff --git a/ts/messageModifiers/Reactions.ts b/ts/messageModifiers/Reactions.ts index f1678772582..72eca4c3b77 100644 --- a/ts/messageModifiers/Reactions.ts +++ b/ts/messageModifiers/Reactions.ts @@ -2,17 +2,17 @@ // SPDX-License-Identifier: AGPL-3.0-only import type { AciString } from '../types/ServiceId'; -import type { ConversationModel } from '../models/conversations'; import type { MessageAttributesType } from '../model-types.d'; import type { MessageModel } from '../models/messages'; import type { ReactionSource } from '../reactions/ReactionSource'; import * as Errors from '../types/errors'; import * as log from '../logging/log'; -import { getContactId, getContact } from '../messages/helpers'; -import { getMessageIdForLogging } from '../util/idForLogging'; +import { getAuthor } from '../messages/helpers'; import { getMessageSentTimestampSet } from '../util/getMessageSentTimestampSet'; -import { isDirectConversation, isMe } from '../util/whatTypeOfConversation'; -import { isOutgoing, isStory } from '../state/selectors/message'; +import { isMe } from '../util/whatTypeOfConversation'; +import { isStory } from '../state/selectors/message'; +import { getPropForTimestamp } from '../util/editHelpers'; +import { isSent } from '../messages/MessageSendState'; import { strictAssert } from '../util/assert'; export type ReactionAttributesType = { @@ -22,9 +22,10 @@ export type ReactionAttributesType = { remove?: boolean; removeFromMessageReceiverCache: () => unknown; source: ReactionSource; - // Necessary to put 1:1 story replies into the right conversation - not the same - // conversation as the target message! - storyReactionMessage?: MessageModel; + // If this is a reaction to a 1:1 story, we can use this message, generated from the + // reaction message itself. Necessary to put 1:1 story replies into the right + // conversation - not the same conversation as the target message! + generatedMessageForStoryReaction?: MessageModel; targetAuthorAci: AciString; targetTimestamp: number; timestamp: number; @@ -38,70 +39,133 @@ function remove(reaction: ReactionAttributesType): void { reaction.removeFromMessageReceiverCache(); } -export function forMessage( +export function findReactionsForMessage( message: MessageModel ): Array { - const logId = `Reactions.forMessage(${getMessageIdForLogging( - message.attributes - )})`; - - const reactionValues = Array.from(reactions.values()); - const sentTimestamps = getMessageSentTimestampSet(message.attributes); - if (isOutgoing(message.attributes)) { - const outgoingReactions = reactionValues.filter(item => - sentTimestamps.has(item.targetTimestamp) - ); - - if (outgoingReactions.length > 0) { - log.info(`${logId}: Found early reaction for outgoing message`); - outgoingReactions.forEach(item => { - remove(item); - }); - return outgoingReactions; - } - } - - const senderId = getContactId(message.attributes); - const reactionsBySource = reactionValues.filter(re => { - const targetSender = window.ConversationController.lookupOrCreate({ - serviceId: re.targetAuthorAci, - reason: logId, + const matchingReactions = Array.from(reactions.values()).filter(reaction => { + return isMessageAMatchForReaction({ + message: message.attributes, + targetTimestamp: reaction.targetTimestamp, + targetAuthorAci: reaction.targetAuthorAci, + reactionSenderConversationId: reaction.fromId, }); - return ( - targetSender?.id === senderId && sentTimestamps.has(re.targetTimestamp) - ); }); - if (reactionsBySource.length > 0) { - log.info(`${logId}: Found early reaction for message`); - reactionsBySource.forEach(item => { - remove(item); - item.removeFromMessageReceiverCache(); - }); - return reactionsBySource; - } - - return []; + matchingReactions.forEach(reaction => remove(reaction)); + return matchingReactions; } -async function findMessage( - targetTimestamp: number, - targetConversationId: string -): Promise { +async function findMessageForReaction({ + targetTimestamp, + targetAuthorAci, + reactionSenderConversationId, + logId, +}: { + targetTimestamp: number; + targetAuthorAci: string; + reactionSenderConversationId: string; + logId: string; +}): Promise { const messages = await window.Signal.Data.getMessagesBySentAt( targetTimestamp ); - return messages.find(m => { - const contact = getContact(m); + const matchingMessages = messages.filter(message => + isMessageAMatchForReaction({ + message, + targetTimestamp, + targetAuthorAci, + reactionSenderConversationId, + }) + ); - if (!contact) { + if (!matchingMessages.length) { + return undefined; + } + + if (matchingMessages.length > 1) { + // This could theoretically happen given limitations in the reaction proto but + // is very unlikely + log.warn( + `${logId}/findMessageForReaction: found ${matchingMessages.length} matching messages for the reaction!` + ); + } + + return matchingMessages[0]; +} + +function isMessageAMatchForReaction({ + message, + targetTimestamp, + targetAuthorAci, + reactionSenderConversationId, +}: { + message: MessageAttributesType; + targetTimestamp: number; + targetAuthorAci: string; + reactionSenderConversationId: string; +}): boolean { + if (!getMessageSentTimestampSet(message).has(targetTimestamp)) { + return false; + } + + const targetAuthorConversation = + window.ConversationController.get(targetAuthorAci); + const reactionSenderConversation = window.ConversationController.get( + reactionSenderConversationId + ); + + if (!targetAuthorConversation || !reactionSenderConversation) { + return false; + } + + const author = getAuthor(message); + if (!author) { + return false; + } + + if (author.id !== targetAuthorConversation.id) { + return false; + } + + if (isMe(reactionSenderConversation.attributes)) { + // I am either the recipient or sender of all the messages I know about! + return true; + } + + if (message.type === 'outgoing') { + const sendStateByConversationId = getPropForTimestamp({ + log, + message, + prop: 'sendStateByConversationId', + targetTimestamp, + }); + + const sendState = + sendStateByConversationId?.[reactionSenderConversation.id]; + if (!sendState) { return false; } - const mcid = contact.get('id'); - return mcid === targetConversationId; - }); + return isSent(sendState.status); + } + + if (message.type === 'incoming') { + const messageConversation = window.ConversationController.get( + message.conversationId + ); + if (!messageConversation) { + return false; + } + + const reactionSenderServiceId = reactionSenderConversation.getServiceId(); + return ( + reactionSenderServiceId != null && + messageConversation.hasMember(reactionSenderServiceId) + ); + } + + return true; } export async function onReaction( @@ -112,36 +176,14 @@ export async function onReaction( const logId = `Reactions.onReaction(timestamp=${reaction.timestamp};target=${reaction.targetTimestamp})`; try { - // The conversation the target message was in; we have to find it in the database - // to to figure that out. - const targetAuthorConversation = - window.ConversationController.lookupOrCreate({ - serviceId: reaction.targetAuthorAci, - reason: logId, - }); - const targetConversationId = targetAuthorConversation?.id; - if (!targetConversationId) { - throw new Error( - `${logId} Error: No conversationId returned from lookupOrCreate!` - ); - } + const matchingMessage = await findMessageForReaction({ + targetTimestamp: reaction.targetTimestamp, + targetAuthorAci: reaction.targetAuthorAci, + reactionSenderConversationId: reaction.fromId, + logId, + }); - const generatedMessage = reaction.storyReactionMessage; - strictAssert( - generatedMessage, - `${logId} strictAssert: Story reactions must provide storyReactionMessage` - ); - const fromConversation = window.ConversationController.get( - generatedMessage.get('conversationId') - ); - - let targetConversation: ConversationModel | undefined | null; - - const targetMessageCheck = await findMessage( - reaction.targetTimestamp, - targetConversationId - ); - if (!targetMessageCheck) { + if (!matchingMessage) { log.info( `${logId}: No message for reaction`, 'targeting', @@ -150,22 +192,11 @@ export async function onReaction( return; } - if ( - fromConversation && - isStory(targetMessageCheck) && - isDirectConversation(fromConversation.attributes) && - !isMe(fromConversation.attributes) - ) { - targetConversation = fromConversation; - } else { - targetConversation = - await window.ConversationController.getConversationForTargetMessage( - targetConversationId, - reaction.targetTimestamp - ); - } + const matchingMessageConversation = window.ConversationController.get( + matchingMessage.conversationId + ); - if (!targetConversation) { + if (!matchingMessageConversation) { log.info( `${logId}: No target conversation for reaction`, reaction.targetAuthorAci, @@ -176,45 +207,52 @@ export async function onReaction( } // awaiting is safe since `onReaction` is never called from inside the queue - await targetConversation.queueJob('Reactions.onReaction', async () => { - log.info(`${logId}: handling`); + await matchingMessageConversation.queueJob( + 'Reactions.onReaction', + async () => { + log.info(`${logId}: handling`); - // Thanks TS. - if (!targetConversation) { - remove(reaction); - return; - } - - // Message is fetched inside the conversation queue so we have the - // most recent data - const targetMessage = await findMessage( - reaction.targetTimestamp, - targetConversationId - ); - - if (!targetMessage) { - remove(reaction); - return; - } - - const message = window.MessageCache.__DEPRECATED$register( - targetMessage.id, - targetMessage, - 'Reactions.onReaction' - ); - - // Use the generated message in ts/background.ts to create a message - // if the reaction is targeted at a story. - if (!isStory(targetMessage)) { - await message.handleReaction(reaction); - } else { - await generatedMessage.handleReaction(reaction, { - storyMessage: targetMessage, + // Message is fetched inside the conversation queue so we have the + // most recent data + const targetMessage = await findMessageForReaction({ + targetTimestamp: reaction.targetTimestamp, + targetAuthorAci: reaction.targetAuthorAci, + reactionSenderConversationId: reaction.fromId, + logId: `${logId}/conversationQueue`, }); - } - remove(reaction); - }); + if (!targetMessage || targetMessage.id !== matchingMessage.id) { + log.warn( + `${logId}: message no longer a match for reaction! Maybe it's been deleted?` + ); + remove(reaction); + return; + } + + const targetMessageModel = window.MessageCache.__DEPRECATED$register( + targetMessage.id, + targetMessage, + 'Reactions.onReaction' + ); + + // Use the generated message in ts/background.ts to create a message + // if the reaction is targeted at a story. + if (!isStory(targetMessage)) { + await targetMessageModel.handleReaction(reaction); + } else { + const generatedMessage = reaction.generatedMessageForStoryReaction; + strictAssert( + generatedMessage, + 'Generated message must exist for story reaction' + ); + await generatedMessage.handleReaction(reaction, { + storyMessage: targetMessage, + }); + } + + remove(reaction); + } + ); } catch (error) { remove(reaction); log.error(`${logId} error:`, Errors.toLogFormat(error)); diff --git a/ts/messages/helpers.ts b/ts/messages/helpers.ts index 4880b6c27c0..f667cdec883 100644 --- a/ts/messages/helpers.ts +++ b/ts/messages/helpers.ts @@ -132,11 +132,11 @@ export function isQuoteAMatch( return ( isSameTimestamp && message.conversationId === conversationId && - getContactId(message) === authorConversation?.id + getAuthorId(message) === authorConversation?.id ); } -export function getContactId( +export function getAuthorId( message: Pick ): string | undefined { const source = getSource(message); @@ -149,15 +149,15 @@ export function getContactId( const conversation = window.ConversationController.lookupOrCreate({ e164: source, serviceId: sourceServiceId, - reason: 'helpers.getContactId', + reason: 'helpers.getAuthorId', }); return conversation?.id; } -export function getContact( +export function getAuthor( message: MessageAttributesType ): ConversationModel | undefined { - const id = getContactId(message); + const id = getAuthorId(message); return window.ConversationController.get(id); } diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index a2639bfde75..909dce8d38a 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -51,7 +51,7 @@ import type { CustomColorType, } from '../types/Colors'; import type { MessageModel } from './messages'; -import { getContact } from '../messages/helpers'; +import { getAuthor } from '../messages/helpers'; import { strictAssert } from '../util/assert'; import { isConversationMuted } from '../util/isConversationMuted'; import { isConversationSMSOnly } from '../util/isConversationSMSOnly'; @@ -5153,7 +5153,7 @@ export class ConversationModel extends window.Backbone const sender = reaction ? window.ConversationController.get(reaction.fromId) - : getContact(message.attributes); + : getAuthor(message.attributes); const senderName = sender ? sender.getTitle() : window.i18n('icu:unknownContact'); diff --git a/ts/models/messages.ts b/ts/models/messages.ts index a747898ecaf..26a0bb1a664 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -113,12 +113,12 @@ import type { import * as log from '../logging/log'; import { cleanupMessage, deleteMessageData } from '../util/cleanup'; import { - getContact, getSource, getSourceServiceId, isCustomError, messageHasPaymentEvent, isQuoteAMatch, + getAuthor, } from '../messages/helpers'; import { viewOnceOpenJobQueue } from '../jobs/viewOnceOpenJobQueue'; import { getMessageIdForLogging } from '../util/idForLogging'; @@ -1625,7 +1625,7 @@ export class MessageModel extends window.Backbone.Model { const type = message.get('type'); const conversationId = message.get('conversationId'); - const fromContact = getContact(this.attributes); + const fromContact = getAuthor(this.attributes); if (fromContact) { fromContact.setRegistered(); } @@ -1751,6 +1751,8 @@ export class MessageModel extends window.Backbone.Model { return; } if (existingMessage) { + // TODO: (DESKTOP-7301): improve this check in case previous message is not yet + // registered in memory log.warn( `${idLog}: Received duplicate transcript for message ${message.idForLogging()}, but it was not an update transcript. Dropping.` ); @@ -2477,7 +2479,7 @@ export class MessageModel extends window.Backbone.Model { ); } - const generatedMessage = reaction.storyReactionMessage; + const generatedMessage = reaction.generatedMessageForStoryReaction; strictAssert( generatedMessage, 'Story reactions must provide storyReactionMessage' @@ -2668,7 +2670,7 @@ export class MessageModel extends window.Backbone.Model { 'New story reaction must have an emoji' ); - const generatedMessage = reaction.storyReactionMessage; + const generatedMessage = reaction.generatedMessageForStoryReaction; strictAssert( generatedMessage, 'Story reactions must provide storyReactionmessage' diff --git a/ts/reactions/enqueueReactionForSend.ts b/ts/reactions/enqueueReactionForSend.ts index 9ee9ff0abfa..44709e71fb2 100644 --- a/ts/reactions/enqueueReactionForSend.ts +++ b/ts/reactions/enqueueReactionForSend.ts @@ -121,7 +121,7 @@ export async function enqueueReactionForSend({ fromId: window.ConversationController.getOurConversationIdOrThrow(), remove, source: ReactionSource.FromThisDevice, - storyReactionMessage, + generatedMessageForStoryReaction: storyReactionMessage, targetAuthorAci, targetTimestamp, receivedAtDate: timestamp, diff --git a/ts/state/ducks/composer.ts b/ts/state/ducks/composer.ts index c615cdfc3e6..bd967abfdb9 100644 --- a/ts/state/ducks/composer.ts +++ b/ts/state/ducks/composer.ts @@ -70,7 +70,7 @@ import { shouldShowInvalidMessageToast } from '../../util/shouldShowInvalidMessa import { writeDraftAttachment } from '../../util/writeDraftAttachment'; import { __DEPRECATED$getMessageById } from '../../messages/getMessageById'; import { canReply, isNormalBubble } from '../selectors/message'; -import { getContactId } from '../../messages/helpers'; +import { getAuthorId } from '../../messages/helpers'; import { getConversationSelector } from '../selectors/conversations'; import { enqueueReactionForSend } from '../../reactions/enqueueReactionForSend'; import { useBoundActions } from '../../hooks/useBoundActions'; @@ -341,7 +341,7 @@ function scrollToQuotedMessage({ Boolean( item.conversationId === conversationId && authorId && - getContactId(item) === authorId + getAuthorId(item) === authorId ) ); diff --git a/ts/state/selectors/message.ts b/ts/state/selectors/message.ts index 8a00dd9b001..226305b5af0 100644 --- a/ts/state/selectors/message.ts +++ b/ts/state/selectors/message.ts @@ -244,7 +244,7 @@ export type GetContactOptions = Pick< 'conversationSelector' | 'ourConversationId' | 'ourNumber' | 'ourAci' >; -export function getContactId( +export function getAuthorId( message: MessageWithUIFieldsType, { conversationSelector, @@ -704,7 +704,7 @@ export const getPropsForMessage = ( (message.reactions || []).find(re => re.fromId === ourConversationId) || {} ).emoji; - const authorId = getContactId(message, { + const authorId = getAuthorId(message, { conversationSelector, ourConversationId, ourNumber, @@ -2096,7 +2096,7 @@ export const getMessageDetails = createSelector( let conversationIds: Array; if (isIncoming(message)) { conversationIds = [ - getContactId(message, { + getAuthorId(message, { conversationSelector, ourConversationId, ourNumber, diff --git a/ts/test-electron/models/messages_test.ts b/ts/test-electron/models/messages_test.ts index e8ca9bf5c5a..32012c92788 100644 --- a/ts/test-electron/models/messages_test.ts +++ b/ts/test-electron/models/messages_test.ts @@ -18,7 +18,7 @@ import enMessages from '../../../_locales/en/messages.json'; import { SendStatus } from '../../messages/MessageSendState'; import { SignalService as Proto } from '../../protobuf'; import { generateAci } from '../../types/ServiceId'; -import { getContact } from '../../messages/helpers'; +import { getAuthor } from '../../messages/helpers'; import { setupI18n } from '../../util/setupI18n'; import { APPLICATION_JSON, @@ -237,7 +237,7 @@ describe('Message', () => { describe('getContact', () => { it('gets outgoing contact', () => { const message = createMessage(attributes); - assert.exists(getContact(message.attributes)); + assert.exists(getAuthor(message.attributes)); }); it('gets incoming contact', () => { @@ -245,7 +245,7 @@ describe('Message', () => { type: 'incoming', source, }); - assert.exists(getContact(message.attributes)); + assert.exists(getAuthor(message.attributes)); }); }); diff --git a/ts/test-mock/helpers.ts b/ts/test-mock/helpers.ts index 408f6286829..2501586eaf9 100644 --- a/ts/test-mock/helpers.ts +++ b/ts/test-mock/helpers.ts @@ -1,7 +1,15 @@ // Copyright 2023 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only +import { + type Device, + type Group, + PrimaryDevice, + type Proto, + StorageState, +} from '@signalapp/mock-server'; import { assert } from 'chai'; +import Long from 'long'; import type { Locator, Page } from 'playwright'; import { expect } from 'playwright/test'; @@ -85,3 +93,187 @@ export async function expectSystemMessages( expected ); } + +function getDevice(author: PrimaryDevice | Device): Device { + return author instanceof PrimaryDevice ? author.device : author; +} + +type GroupInfo = { + group: Group; + members: Array; +}; + +function maybeWrapInSyncMessage({ + isSync, + to, + sentTo, + dataMessage, +}: { + isSync: boolean; + to: PrimaryDevice | Device; + sentTo?: Array; + dataMessage: Proto.IDataMessage; +}): Proto.IContent { + return isSync + ? { + syncMessage: { + sent: { + destinationServiceId: getDevice(to).aci, + message: dataMessage, + timestamp: dataMessage.timestamp, + unidentifiedStatus: (sentTo ?? [to]).map(contact => ({ + destinationServiceId: getDevice(contact).aci, + destination: getDevice(contact).number, + })), + }, + }, + } + : { dataMessage }; +} + +function isToGroup(to: Device | PrimaryDevice | GroupInfo): to is GroupInfo { + return 'group' in to; +} + +export function sendTextMessage({ + from, + to, + text, + desktop, + timestamp = Date.now(), +}: { + from: PrimaryDevice; + to: PrimaryDevice | Device | GroupInfo; + text: string; + desktop: Device; + timestamp?: number; +}): Promise { + const isSync = from.secondaryDevices.includes(desktop); + const toDevice = isSync || isToGroup(to) ? desktop : getDevice(to); + const groupInfo = isToGroup(to) ? to : undefined; + return from.sendRaw( + toDevice, + maybeWrapInSyncMessage({ + isSync, + to: to as PrimaryDevice, + dataMessage: { + body: text, + timestamp: Long.fromNumber(timestamp), + groupV2: groupInfo + ? { + masterKey: groupInfo.group.masterKey, + revision: groupInfo.group.revision, + } + : undefined, + }, + sentTo: groupInfo ? groupInfo.members : [to as PrimaryDevice | Device], + }), + { timestamp } + ); +} + +export function sendReaction({ + from, + to, + targetAuthor, + targetMessageTimestamp, + emoji = '👍', + reactionTimestamp = Date.now(), + desktop, +}: { + from: PrimaryDevice; + to: PrimaryDevice | Device; + targetAuthor: PrimaryDevice | Device; + targetMessageTimestamp: number; + emoji: string; + reactionTimestamp?: number; + desktop: Device; +}): Promise { + const isSync = from.secondaryDevices.includes(desktop); + return from.sendRaw( + isSync ? desktop : getDevice(to), + maybeWrapInSyncMessage({ + isSync, + to, + dataMessage: { + timestamp: Long.fromNumber(reactionTimestamp), + reaction: { + emoji, + targetAuthorAci: getDevice(targetAuthor).aci, + targetTimestamp: Long.fromNumber(targetMessageTimestamp), + }, + }, + }), + { + timestamp: reactionTimestamp, + } + ); +} + +async function getStorageState(phone: PrimaryDevice) { + return (await phone.getStorageState()) ?? StorageState.getEmpty(); +} + +export async function createGroup( + phone: PrimaryDevice, + otherMembers: Array, + groupTitle: string +): Promise { + const group = await phone.createGroup({ + title: groupTitle, + members: [phone, ...otherMembers], + }); + let state = await getStorageState(phone); + + state = state + .addGroup(group, { + whitelisted: true, + }) + .pinGroup(group); + + // Finally whitelist and pin contacts + for (const member of otherMembers) { + state = state.addContact(member, { + whitelisted: true, + serviceE164: member.device.number, + identityKey: member.publicKey.serialize(), + profileKey: member.profileKey.serialize(), + givenName: member.profileName, + }); + } + await phone.setStorageState(state); + return group; +} + +export async function clickOnConversation( + page: Page, + contact: PrimaryDevice +): Promise { + const leftPane = page.locator('#LeftPane'); + await leftPane.getByTestId(contact.device.aci).click(); +} +export async function pinContact( + phone: PrimaryDevice, + contact: PrimaryDevice +): Promise { + const state = await getStorageState(phone); + state.pin(contact); + await phone.setStorageState(state); +} + +export function acceptConversation(page: Page): Promise { + return page + .locator('.module-message-request-actions button >> "Accept"') + .click(); +} + +export function getTimeline(page: Page): Locator { + return page.locator('.module-timeline__messages__container'); +} + +export function getMessageInTimelineByTimestamp( + page: Page, + timestamp: number +): Locator { + return getTimeline(page).getByTestId(`${timestamp}`); +} diff --git a/ts/test-mock/messaging/reaction_test.ts b/ts/test-mock/messaging/reaction_test.ts new file mode 100644 index 00000000000..6219bd21c73 --- /dev/null +++ b/ts/test-mock/messaging/reaction_test.ts @@ -0,0 +1,353 @@ +// Copyright 2024 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import createDebug from 'debug'; +import { StorageState } from '@signalapp/mock-server'; +import { type Page } from 'playwright'; +import { expect } from 'playwright/test'; +import { assert } from 'chai'; + +import type { App } from '../playwright'; +import { Bootstrap } from '../bootstrap'; +import { MINUTE } from '../../util/durations'; +import { strictAssert } from '../../util/assert'; +import { + clickOnConversation, + getMessageInTimelineByTimestamp, + sendTextMessage, + sendReaction, + createGroup, +} from '../helpers'; + +export const debug = createDebug('mock:test:reactions'); + +async function getReactionsForMessage(page: Page, timestamp: number) { + const reactionsByEmoji: Record> = {}; + + try { + const message = await getMessageInTimelineByTimestamp(page, timestamp); + + await message.locator('.module-message__reactions').click(); + + const reactionRows = await page + .locator('.module-reaction-viewer__body__row') + .all(); + + for (const row of reactionRows) { + // eslint-disable-next-line no-await-in-loop + const emoji = await row.locator('img').getAttribute('title'); + // eslint-disable-next-line no-await-in-loop + const reactor = await row + .locator('.module-reaction-viewer__body__row__name') + .innerText(); + + strictAssert(emoji, 'emoji must exist'); + reactionsByEmoji[emoji] = (reactionsByEmoji[emoji] ?? []).concat([ + reactor, + ]); + } + // click away + await page.getByText("chat history isn't transferred").click(); + } catch { + // pass + } + return reactionsByEmoji; +} + +async function expectMessageToHaveReactions( + page: Page, + timestamp: number, + reactionsBySender: Record>, + options?: { timeout: number } +): Promise { + return expect(async () => { + assert.deepEqual( + await getReactionsForMessage(page, timestamp), + reactionsBySender + ); + }).toPass({ timeout: options?.timeout ?? 10000 }); +} + +describe('reactions', function (this: Mocha.Suite) { + let bootstrap: Bootstrap; + let app: App; + + this.timeout(MINUTE); + beforeEach(async () => { + bootstrap = new Bootstrap(); + await bootstrap.init(); + + const { phone, contacts } = bootstrap; + const [alice, bob, charlie] = contacts; + let state = StorageState.getEmpty(); + + state = state.addContact(alice, { + identityKey: alice.publicKey.serialize(), + profileKey: alice.profileKey.serialize(), + }); + state = state.addContact(bob, { + identityKey: bob.publicKey.serialize(), + profileKey: bob.profileKey.serialize(), + }); + state = state.addContact(charlie, { + identityKey: charlie.publicKey.serialize(), + profileKey: charlie.profileKey.serialize(), + }); + + await phone.setStorageState(state); + + app = await bootstrap.link(); + }); + + afterEach(async function (this: Mocha.Context) { + if (!bootstrap) { + return; + } + + await bootstrap.maybeSaveLogs(this.currentTest, app); + await app.close(); + await bootstrap.teardown(); + }); + + it('should correctly match on participant, timestamp, and author in 1:1 conversation', async () => { + this.timeout(10000); + const { contacts, phone, desktop } = bootstrap; + const [alice, bob, charlie] = contacts; + + const window = await app.getWindow(); + + const alice1on1Timestamp = Date.now(); + const outgoingTimestamp = alice1on1Timestamp; + + await sendTextMessage({ + from: alice, + to: desktop, + text: 'hi from alice', + timestamp: alice1on1Timestamp, + desktop, + }); + + // To test the case where we have different outgoing messages with the same + // timestamps, we need to send these without awaiting since otherwise desktop will + // drop them since they have the same timestamp (DESKTOP-7301) + await Promise.all([ + sendTextMessage({ + from: phone, + to: bob, + text: 'hi bob', + timestamp: outgoingTimestamp, + desktop, + }), + + sendTextMessage({ + from: phone, + to: charlie, + text: 'hi charlie', + timestamp: outgoingTimestamp, + desktop, + }), + ]); + + // [❌ invalid reaction] bob trying to trick us by reacting to a message in a + // conversation he's not a part of + await sendReaction({ + from: bob, + to: desktop, + emoji: '👻', + targetAuthor: alice, + targetMessageTimestamp: alice1on1Timestamp, + desktop, + }); + + // [❌ invalid reaction] phone sending message with wrong author but right timestamp + await sendReaction({ + from: phone, + to: desktop, + emoji: '💀', + targetAuthor: bob, + targetMessageTimestamp: alice1on1Timestamp, + desktop, + }); + + // [✅ incoming message] alice reacting to her own message + await sendReaction({ + from: alice, + to: desktop, + emoji: '👍', + targetAuthor: alice, + targetMessageTimestamp: alice1on1Timestamp, + desktop, + }); + + await clickOnConversation(window, alice); + await expectMessageToHaveReactions(window, alice1on1Timestamp, { + '👍': [alice.profileName], + }); + + // [✅ incoming message] phone sending message with right author + await sendReaction({ + from: phone, + to: alice, + emoji: '👋', + targetAuthor: alice, + targetMessageTimestamp: alice1on1Timestamp, + desktop, + }); + + await expectMessageToHaveReactions(window, alice1on1Timestamp, { + '👍': [alice.profileName], + '👋': ['You'], + }); + + // now, receive reactions from those messages with same timestamp + // [✅ outgoing message] bob reacting to our message + await sendReaction({ + from: bob, + to: desktop, + emoji: '👋', + targetAuthor: phone, + targetMessageTimestamp: outgoingTimestamp, + desktop, + }); + + // [✅ outgoing message] alice reacting to our message + await sendReaction({ + from: charlie, + to: desktop, + emoji: '👋', + targetAuthor: phone, + targetMessageTimestamp: outgoingTimestamp, + desktop, + }); + + await clickOnConversation(window, bob); + await expectMessageToHaveReactions(window, outgoingTimestamp, { + '👋': [bob.profileName], + }); + + await clickOnConversation(window, charlie); + await expectMessageToHaveReactions(window, outgoingTimestamp, { + '👋': [charlie.profileName], + }); + }); + + it('should correctly match on participant, timestamp, and author in group conversation', async () => { + this.timeout(10000); + + const { contacts, phone, desktop } = bootstrap; + const [alice, bob, charlie, danielle] = contacts; + + const groupMembers = [alice, bob, charlie]; + const groupForSending = { + group: await createGroup(phone, groupMembers, 'ReactionGroup'), + members: groupMembers, + }; + + const window = await app.getWindow(); + const leftPane = window.locator('#LeftPane'); + + const now = Date.now(); + const myGroupTimestamp = now; + const aliceGroupTimestamp = now + 1; + const bobGroupTimestamp = now + 2; + const charlieGroupTimestamp = now + 3; + + // [✅ outgoing message]: charlie reacting to bob's group message, early + await sendReaction({ + from: charlie, + to: desktop, + emoji: '👋', + targetAuthor: bob, + targetMessageTimestamp: bobGroupTimestamp, + desktop, + }); + + // Send a bunch of messages in the group + await sendTextMessage({ + from: phone, + to: groupForSending, + text: "hello group, it's me", + timestamp: myGroupTimestamp, + desktop, + }); + + await sendTextMessage({ + from: alice, + to: groupForSending, + text: "hello group, it's alice", + timestamp: aliceGroupTimestamp, + desktop, + }); + + await sendTextMessage({ + from: bob, + to: groupForSending, + text: "hello group, it's bob", + timestamp: bobGroupTimestamp, + desktop, + }); + + await sendTextMessage({ + from: charlie, + to: groupForSending, + text: "hello group, it's charlie", + timestamp: charlieGroupTimestamp, + desktop, + }); + + await leftPane.getByText('ReactionGroup').click(); + + // [❌ invalid reaction] danielle reacting to our group message, but she's not in the + // group! + await sendReaction({ + from: danielle, + to: desktop, + emoji: '👻', + targetAuthor: phone, + targetMessageTimestamp: myGroupTimestamp, + desktop, + }); + + // [✅ outgoing message]: alice reacting to our group message + await sendReaction({ + from: alice, + to: desktop, + emoji: '👍', + targetAuthor: phone, + targetMessageTimestamp: myGroupTimestamp, + desktop, + }); + + // [✅ outgoing message]: bob reacting to our group message + await sendReaction({ + from: bob, + to: desktop, + emoji: '👍', + targetAuthor: phone, + targetMessageTimestamp: myGroupTimestamp, + desktop, + }); + + // [✅ outgoing message]: charlie reacting to alice's group message + await sendReaction({ + from: charlie, + to: desktop, + emoji: '😛', + targetAuthor: alice, + targetMessageTimestamp: aliceGroupTimestamp, + desktop, + }); + + await expectMessageToHaveReactions(window, myGroupTimestamp, { + '👍': [bob.profileName, alice.profileName], + }); + + await expectMessageToHaveReactions(window, aliceGroupTimestamp, { + '😛': [charlie.profileName], + }); + + await expectMessageToHaveReactions(window, bobGroupTimestamp, { + '👋': [charlie.profileName], + }); + }); +}); diff --git a/ts/util/deleteForEveryone.ts b/ts/util/deleteForEveryone.ts index 796b18080c5..4241c72c5a5 100644 --- a/ts/util/deleteForEveryone.ts +++ b/ts/util/deleteForEveryone.ts @@ -5,7 +5,7 @@ import type { DeleteAttributesType } from '../messageModifiers/Deletes'; import type { MessageModel } from '../models/messages'; import * as log from '../logging/log'; import { isMe } from './whatTypeOfConversation'; -import { getContactId } from '../messages/helpers'; +import { getAuthorId } from '../messages/helpers'; import { isStory } from '../state/selectors/message'; import { isTooOldToModifyMessage } from './isTooOldToModifyMessage'; @@ -54,7 +54,7 @@ function isDeletionByMe( const ourConversationId = window.ConversationController.getOurConversationIdOrThrow(); return ( - getContactId(message.attributes) === ourConversationId && + getAuthorId(message.attributes) === ourConversationId && doe.fromId === ourConversationId ); } diff --git a/ts/util/findStoryMessage.ts b/ts/util/findStoryMessage.ts index 40191bdd24f..10e5137a639 100644 --- a/ts/util/findStoryMessage.ts +++ b/ts/util/findStoryMessage.ts @@ -8,7 +8,7 @@ import type { AciString } from '../types/ServiceId'; import * as log from '../logging/log'; import { normalizeAci } from './normalizeAci'; import { filter } from './iterables'; -import { getContactId } from '../messages/helpers'; +import { getAuthorId } from '../messages/helpers'; import { getTimestampFromLong } from './timestampLongUtils'; export async function findStoryMessages( @@ -89,7 +89,7 @@ function isStoryAMatch( return ( message.sent_at === sentTimestamp && - getContactId(message) === authorConversation?.id && + getAuthorId(message) === authorConversation?.id && (message.conversationId === conversationId || message.conversationId === ourConversationId) ); diff --git a/ts/util/getNotificationDataForMessage.ts b/ts/util/getNotificationDataForMessage.ts index e91961662ce..e6f5416045a 100644 --- a/ts/util/getNotificationDataForMessage.ts +++ b/ts/util/getNotificationDataForMessage.ts @@ -48,7 +48,7 @@ import { isMessageRequestResponse, } from '../state/selectors/message'; import { - getContact, + getAuthor, messageHasPaymentEvent, getPaymentEventNotificationText, } from '../messages/helpers'; @@ -260,7 +260,7 @@ export function getNotificationDataForMessage( if (isGroupUpdate(attributes)) { const { group_update: groupUpdate } = attributes; - const fromContact = getContact(attributes); + const fromContact = getAuthor(attributes); const messages = []; if (!groupUpdate) { throw new Error('getNotificationData: Missing group_update'); @@ -499,7 +499,7 @@ export function getNotificationDataForMessage( }; } - const fromContact = getContact(attributes); + const fromContact = getAuthor(attributes); const sender = fromContact?.getTitle() ?? window.i18n('icu:unknownContact'); return { emoji, diff --git a/ts/util/makeQuote.ts b/ts/util/makeQuote.ts index dd91198ae58..0c8a537ccb1 100644 --- a/ts/util/makeQuote.ts +++ b/ts/util/makeQuote.ts @@ -10,7 +10,7 @@ import type { MIMEType } from '../types/MIME'; import type { LinkPreviewType } from '../types/message/LinkPreviews'; import type { StickerType } from '../types/Stickers'; import { IMAGE_JPEG, IMAGE_GIF } from '../types/MIME'; -import { getContact } from '../messages/helpers'; +import { getAuthor } from '../messages/helpers'; import { getQuoteBodyText } from './getQuoteBodyText'; import { isGIF } from '../types/Attachment'; import { isGiftBadge, isTapToView } from '../state/selectors/message'; @@ -22,7 +22,7 @@ import { getMessageSentTimestamp } from './getMessageSentTimestamp'; export async function makeQuote( quotedMessage: MessageAttributesType ): Promise { - const contact = getContact(quotedMessage); + const contact = getAuthor(quotedMessage); strictAssert(contact, 'makeQuote: no contact'); diff --git a/ts/util/modifyTargetMessage.ts b/ts/util/modifyTargetMessage.ts index 67477eb2176..36724137ea9 100644 --- a/ts/util/modifyTargetMessage.ts +++ b/ts/util/modifyTargetMessage.ts @@ -220,12 +220,16 @@ export async function modifyTargetMessage( } // Does message message have any pending, previously-received associated reactions? - const reactions = Reactions.forMessage(message); + const reactions = Reactions.findReactionsForMessage(message); + + log.info( + `${logId}: Found ${reactions.length} early reaction(s) for ${message.attributes.type} message` + ); await Promise.all( reactions.map(async reaction => { if (isStory(message.attributes)) { // We don't set changed = true here, because we don't modify the original story - const generatedMessage = reaction.storyReactionMessage; + const generatedMessage = reaction.generatedMessageForStoryReaction; strictAssert( generatedMessage, 'Story reactions must provide storyReactionMessage'