Process out of order edits

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<string, EditAttributesType>();
 
-export function forMessage(message: MessageModel): Array<EditAttributesType> {
+export function forMessage(
-  const sentAt = getMessageSentTimestamp(message.attributes, { log });
+  messageAttributes: Pick<
+    MessageAttributesType,
+    | 'editMessageTimestamp'
+    | 'sent_at'
+    | 'source'
+    | 'sourceUuid'
+    | 'timestamp'
+    | 'type'
+  >
+): Array<EditAttributesType> {
+  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<EditAttributesType> {
     });
 
     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<void> {
       );
 
     if (!targetConversation) {
-      log.info(`${logId}: No target conversation`);
+      log.info(`${logId}: No message found`);
-
       return;
     }
 

ts/messages/helpers.ts

@@ -137,7 +137,7 @@ export function isQuoteAMatch(
 }
 
 export function getContactId(
-  message: MessageAttributesType
+  message: Pick<MessageAttributesType, 'type' | 'source' | 'sourceUuid'>
 ): string | undefined {
   const source = getSource(message);
   const sourceUuid = getSourceUuid(message);

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<Proto.IEditMessage> = [
+      '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');
+  });
 });

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<void> {
-  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)
+    )
+  );
 }

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 =>