diff --git a/ts/messageModifiers/Edits.ts b/ts/messageModifiers/Edits.ts index df2d49a3f8c8..f08872823051 100644 --- a/ts/messageModifiers/Edits.ts +++ b/ts/messageModifiers/Edits.ts @@ -2,7 +2,6 @@ // SPDX-License-Identifier: AGPL-3.0-only import type { MessageAttributesType } from '../model-types.d'; -import type { MessageModel } from '../models/messages'; import * as Errors from '../types/errors'; import * as log from '../logging/log'; import { drop } from '../util/drop'; @@ -23,12 +22,22 @@ export type EditAttributesType = { const edits = new Map(); -export function forMessage(message: MessageModel): Array { - const sentAt = getMessageSentTimestamp(message.attributes, { log }); +export function forMessage( + messageAttributes: Pick< + MessageAttributesType, + | 'editMessageTimestamp' + | 'sent_at' + | 'source' + | 'sourceUuid' + | 'timestamp' + | 'type' + > +): Array { + const sentAt = getMessageSentTimestamp(messageAttributes, { log }); const matchingEdits = filter(edits, ([_envelopeId, item]) => { return ( item.targetSentTimestamp === sentAt && - item.fromId === getContactId(message.attributes) + item.fromId === getContactId(messageAttributes) ); }); @@ -44,7 +53,7 @@ export function forMessage(message: MessageModel): Array { }); log.info( - `Edits.forMessage(${message.get('sent_at')}): ` + + `Edits.forMessage(${messageAttributes.sent_at}): ` + `Found early edits for message ${editsLogIds.join(', ')}` ); return result; @@ -68,8 +77,7 @@ export async function onEdit(edit: EditAttributesType): Promise { ); if (!targetConversation) { - log.info(`${logId}: No target conversation`); - + log.info(`${logId}: No message found`); return; } diff --git a/ts/messages/helpers.ts b/ts/messages/helpers.ts index 5ef8f3a02cb0..1573e677b9fb 100644 --- a/ts/messages/helpers.ts +++ b/ts/messages/helpers.ts @@ -137,7 +137,7 @@ export function isQuoteAMatch( } export function getContactId( - message: MessageAttributesType + message: Pick ): string | undefined { const source = getSource(message); const sourceUuid = getSourceUuid(message); diff --git a/ts/test-mock/messaging/edit_test.ts b/ts/test-mock/messaging/edit_test.ts index 791ee5ca0352..ae78c4b39d3a 100644 --- a/ts/test-mock/messaging/edit_test.ts +++ b/ts/test-mock/messaging/edit_test.ts @@ -27,25 +27,27 @@ function wrap({ }; } -function createMessage(): Proto.IDataMessage { +function createMessage(body: string): Proto.IDataMessage { return { - body: 'hey yhere', + body, groupV2: undefined, timestamp: Long.fromNumber(Date.now()), }; } function createEditedMessage( - targetMessage: Proto.IDataMessage + targetSentTimestamp: Long | null | undefined, + body: string, + timestamp = Date.now() ): Proto.IEditMessage { - strictAssert(targetMessage.timestamp, 'timestamp missing'); + strictAssert(targetSentTimestamp, 'timestamp missing'); return { - targetSentTimestamp: targetMessage.timestamp, + targetSentTimestamp, dataMessage: { - body: 'hey there', + body, groupV2: undefined, - timestamp: Long.fromNumber(Date.now()), + timestamp: Long.fromNumber(timestamp), }, }; } @@ -77,7 +79,8 @@ describe('editing', function needsName() { const window = await app.getWindow(); - const originalMessage = createMessage(); + const initialMessageBody = 'hey yhere'; + const originalMessage = createMessage(initialMessageBody); const originalMessageTimestamp = Number(originalMessage.timestamp); debug('sending message'); @@ -101,7 +104,9 @@ describe('editing', function needsName() { await window.locator('.module-conversation-hero').waitFor(); debug('checking for message'); - await window.locator('.module-message__text >> "hey yhere"').waitFor(); + await window + .locator(`.module-message__text >> "${initialMessageBody}"`) + .waitFor(); debug('waiting for receipts for original message'); const receipts = await app.waitForReceipts(); @@ -110,7 +115,11 @@ describe('editing', function needsName() { assert.strictEqual(receipts.timestamps[0], originalMessageTimestamp); debug('sending edited message'); - const editedMessage = createEditedMessage(originalMessage); + const editedMessageBody = 'hey there'; + const editedMessage = createEditedMessage( + originalMessage.timestamp, + editedMessageBody + ); const editedMessageTimestamp = Number(editedMessage.dataMessage?.timestamp); { const sendOptions = { @@ -124,7 +133,9 @@ describe('editing', function needsName() { } debug('checking for edited message'); - await window.locator('.module-message__text >> "hey there"').waitFor(); + await window + .locator(`.module-message__text >> "${editedMessageBody}"`) + .waitFor(); const messages = window.locator('.module-message__text'); assert.strictEqual(await messages.count(), 1, 'message count'); @@ -137,7 +148,8 @@ describe('editing', function needsName() { const [friend] = contacts; - const originalMessage = createMessage(); + const initialMessageBody = 'hey yhere'; + const originalMessage = createMessage(initialMessageBody); const originalMessageTimestamp = Number(originalMessage.timestamp); debug('incoming message'); @@ -161,7 +173,9 @@ describe('editing', function needsName() { await window.locator('.module-conversation-hero').waitFor(); debug('checking for message'); - await window.locator('.module-message__text >> "hey yhere"').waitFor(); + await window + .locator(`.module-message__text >> "${initialMessageBody}"`) + .waitFor(); debug('waiting for original receipt'); const originalReceipt = await friend.waitForReceipt(); @@ -177,7 +191,11 @@ describe('editing', function needsName() { } debug('sending edited message'); - const editedMessage = createEditedMessage(originalMessage); + const editedMessageBody = 'hey there'; + const editedMessage = createEditedMessage( + originalMessage.timestamp, + editedMessageBody + ); const editedMessageTimestamp = Number(editedMessage.dataMessage?.timestamp); { const sendOptions = { @@ -191,7 +209,9 @@ describe('editing', function needsName() { } debug('checking for edited message'); - await window.locator('.module-message__text >> "hey there"').waitFor(); + await window + .locator(`.module-message__text >> "${editedMessageBody}"`) + .waitFor(); const messages = window.locator('.module-message__text'); assert.strictEqual(await messages.count(), 1, 'message count'); @@ -264,6 +284,7 @@ describe('editing', function needsName() { await input.press('Enter'); } + debug("waiting for friend's edit message"); const { editMessage: firstEdit } = await friend.waitForEditMessage(); assert.strictEqual( firstEdit.targetSentTimestamp?.toNumber(), @@ -311,4 +332,75 @@ describe('editing', function needsName() { assert.isTrue(await history.locator('"edit message 2"').isVisible()); assert.isTrue(await history.locator('"edit message 3"').isVisible()); }); + + it('is fine with out of order edit processing', async () => { + const { phone, desktop } = bootstrap; + + const window = await app.getWindow(); + + const originalMessage = createMessage('v1'); + const originalMessageTimestamp = Number(originalMessage.timestamp); + + const sendOriginalMessage = async () => { + debug('sending original message', originalMessageTimestamp); + const sendOptions = { + timestamp: originalMessageTimestamp, + }; + await phone.sendRaw( + desktop, + wrap({ dataMessage: originalMessage }), + sendOptions + ); + }; + + debug('sending all messages + edits'); + let targetSentTimestamp = originalMessage.timestamp; + let editTimestamp = Date.now() + 1; + const editedMessages: Array = [ + 'v2', + 'v3', + 'v4', + 'v5', + ].map(body => { + const message = createEditedMessage( + targetSentTimestamp, + body, + editTimestamp + ); + targetSentTimestamp = Long.fromNumber(editTimestamp); + editTimestamp += 1; + return message; + }); + { + const sendEditMessages = editedMessages.map(editMessage => { + const timestamp = Number(editMessage.dataMessage?.timestamp); + const sendOptions = { + timestamp, + }; + return () => { + debug( + `sending edit timestamp=${timestamp}, target=${editMessage.targetSentTimestamp}` + ); + + return phone.sendRaw(desktop, wrap({ editMessage }), sendOptions); + }; + }); + await Promise.all(sendEditMessages.reverse().map(f => f())); + await sendOriginalMessage(); + } + + debug('opening conversation'); + const leftPane = window.locator('#LeftPane'); + await leftPane + .locator('.module-conversation-list__item--contact-or-conversation') + .first() + .click(); + await window.locator('.module-conversation-hero').waitFor(); + + debug('checking for latest message'); + await window.locator('.module-message__text >> "v5"').waitFor(); + + const messages = window.locator('.module-message__text'); + assert.strictEqual(await messages.count(), 1, 'message count'); + }); }); diff --git a/ts/util/handleEditMessage.ts b/ts/util/handleEditMessage.ts index a49c209cb962..76e0e2285ec2 100644 --- a/ts/util/handleEditMessage.ts +++ b/ts/util/handleEditMessage.ts @@ -9,6 +9,7 @@ import type { QuotedMessageType, } from '../model-types.d'; import type { LinkPreviewType } from '../types/message/LinkPreviews'; +import * as Edits from '../messageModifiers/Edits'; import * as durations from './durations'; import * as log from '../logging/log'; import { ReadStatus } from '../messages/MessageReadStatus'; @@ -23,14 +24,26 @@ import { isDirectConversation } from './whatTypeOfConversation'; import { queueAttachmentDownloads } from './queueAttachmentDownloads'; import { modifyTargetMessage } from './modifyTargetMessage'; +const RECURSION_LIMIT = 15; + export async function handleEditMessage( mainMessage: MessageAttributesType, editAttributes: Pick< EditAttributesType, 'message' | 'conversationId' | 'fromDevice' | 'fromId' - > + >, + recursionCount = 0 ): Promise { - const idLog = `handleEditMessage(${getMessageIdForLogging(mainMessage)})`; + const idLog = `handleEditMessage(edit=${ + editAttributes.message.timestamp + },original=${getMessageIdForLogging(mainMessage)})`; + + if (recursionCount >= RECURSION_LIMIT) { + log.warn(`${idLog}: Too much recursion`); + return; + } + + log.info(idLog); // Verify that we can safely apply an edit to this type of message if (mainMessage.deletedForEveryone) { @@ -298,9 +311,23 @@ export async function handleEditMessage( const mainMessageConversation = mainMessageModel.getConversation(); if (mainMessageConversation) { drop(mainMessageConversation.updateLastMessage()); + // Apply any other operations, excluding edits that target this message await modifyTargetMessage(mainMessageModel, mainMessageConversation, { isFirstRun: true, skipEdits: true, }); } + + // Apply any other pending edits that target this message + const edits = Edits.forMessage({ + ...mainMessage, + sent_at: editedMessage.timestamp, + timestamp: editedMessage.timestamp, + }); + log.info(`${idLog}: ${edits.length} edits`); + await Promise.all( + edits.map(edit => + handleEditMessage(mainMessageModel.attributes, edit, recursionCount + 1) + ) + ); } diff --git a/ts/util/modifyTargetMessage.ts b/ts/util/modifyTargetMessage.ts index a1555bd9f682..6aeea0bd2e78 100644 --- a/ts/util/modifyTargetMessage.ts +++ b/ts/util/modifyTargetMessage.ts @@ -252,7 +252,7 @@ export async function modifyTargetMessage( // We want to make sure the message is saved first before applying any edits if (!isFirstRun && !skipEdits) { - const edits = Edits.forMessage(message); + const edits = Edits.forMessage(message.attributes); log.info(`${logId}: ${edits.length} edits in second run`); await Promise.all( edits.map(editAttributes =>