Don't process edits until attachmentDownloadQueue finishes

ts/messageModifiers/Edits.ts

@@ -8,6 +8,10 @@ import { drop } from '../util/drop';
 import { getContactId } from '../messages/helpers';
 import { handleEditMessage } from '../util/handleEditMessage';
 import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp';
+import {
+  isAttachmentDownloadQueueEmpty,
+  registerQueueEmptyCallback,
+} from '../util/attachmentDownloadQueue';
 
 export type EditAttributesType = {
   conversationId: string;
@@ -40,6 +44,14 @@ export function forMessage(
   const sentAt = getMessageSentTimestamp(messageAttributes, { log });
   const editValues = Array.from(edits.values());
 
+  if (!isAttachmentDownloadQueueEmpty()) {
+    log.info(
+      'Edits.forMessage attachmentDownloadQueue not empty, not processing edits'
+    );
+    registerQueueEmptyCallback(flushEdits);
+    return [];
+  }
+
   const matchingEdits = editValues.filter(item => {
     return (
       item.targetSentTimestamp === sentAt &&
@@ -66,11 +78,26 @@ export function forMessage(
   return [];
 }
 
+export async function flushEdits(): Promise<void> {
+  log.info('Edits.flushEdits running');
+  return drop(
+    Promise.all(Array.from(edits.values()).map(edit => onEdit(edit)))
+  );
+}
+
 export async function onEdit(edit: EditAttributesType): Promise<void> {
   edits.set(edit.envelopeId, edit);
 
   const logId = `Edits.onEdit(timestamp=${edit.message.timestamp};target=${edit.targetSentTimestamp})`;
 
+  if (!isAttachmentDownloadQueueEmpty()) {
+    log.info(
+      `${logId}: attachmentDownloadQueue not empty, not processing edits`
+    );
+    registerQueueEmptyCallback(flushEdits);
+    return;
+  }
+
   try {
     // The conversation the edited message was in; we have to find it in the database
     //   to to figure that out.

ts/test-mock/messaging/edit_test.ts

@@ -11,11 +11,23 @@ import * as durations from '../../util/durations';
 import { Bootstrap } from '../bootstrap';
 import { ReceiptType } from '../../types/Receipt';
 import { SendStatus } from '../../messages/MessageSendState';
+import { drop } from '../../util/drop';
 import { sleep } from '../../util/sleep';
 import { strictAssert } from '../../util/assert';
+import { generateAci } from '../../types/ServiceId';
+import { IMAGE_GIF } from '../../types/MIME';
 
 export const debug = createDebug('mock:test:edit');
 
+const ACI_1 = generateAci();
+const UNPROCESSED_ATTACHMENT: Proto.IAttachmentPointer = {
+  cdnId: Long.fromNumber(123),
+  key: new Uint8Array([1, 2, 3]),
+  digest: new Uint8Array([4, 5, 6]),
+  contentType: IMAGE_GIF,
+  size: 34,
+};
+
 function wrap({
   dataMessage,
   editMessage,
@@ -37,6 +49,26 @@ function createMessage(body: string): Proto.IDataMessage {
   };
 }
 
+function createMessageWithQuote(body: string): Proto.IDataMessage {
+  return {
+    body,
+    quote: {
+      id: Long.fromNumber(1),
+      authorAci: ACI_1,
+      text: 'text',
+      attachments: [
+        {
+          contentType: 'image/jpeg',
+          fileName: 'image.jpg',
+          thumbnail: UNPROCESSED_ATTACHMENT,
+        },
+      ],
+    },
+    groupV2: undefined,
+    timestamp: Long.fromNumber(Date.now()),
+  };
+}
+
 function createEditedMessage(
   targetSentTimestamp: Long | null | undefined,
   body: string,
@@ -64,7 +96,6 @@ describe('editing', function (this: Mocha.Suite) {
   beforeEach(async () => {
     bootstrap = new Bootstrap();
     await bootstrap.init();
-    app = await bootstrap.link();
   });
 
   afterEach(async function (this: Mocha.Context) {
@@ -77,6 +108,11 @@ describe('editing', function (this: Mocha.Suite) {
     await bootstrap.teardown();
   });
 
+  describe('online', function (this: Mocha.Suite) {
+    beforeEach(async () => {
+      app = await bootstrap.link();
+    });
+
     it('handles incoming edited messages phone to desktop', async () => {
       const { phone, desktop } = bootstrap;
 
@@ -123,7 +159,9 @@ describe('editing', function (this: Mocha.Suite) {
         originalMessage.timestamp,
         editedMessageBody
       );
-    const editedMessageTimestamp = Number(editedMessage.dataMessage?.timestamp);
+      const editedMessageTimestamp = Number(
+        editedMessage.dataMessage?.timestamp
+      );
       {
         const sendOptions = {
           timestamp: editedMessageTimestamp,
@@ -199,7 +237,9 @@ describe('editing', function (this: Mocha.Suite) {
         originalMessage.timestamp,
         editedMessageBody
       );
-    const editedMessageTimestamp = Number(editedMessage.dataMessage?.timestamp);
+      const editedMessageTimestamp = Number(
+        editedMessage.dataMessage?.timestamp
+      );
       {
         const sendOptions = {
           timestamp: editedMessageTimestamp,
@@ -407,6 +447,53 @@ describe('editing', function (this: Mocha.Suite) {
       assert.strictEqual(await messages.count(), 1, 'message count');
     });
 
+    it('is fine with out of order edits with quotes removed', async () => {
+      const { phone, desktop } = bootstrap;
+
+      const originalMessage = createMessageWithQuote('v1');
+      const originalMessageTimestamp = Number(originalMessage.timestamp);
+
+      debug('sending edit');
+      const targetSentTimestamp = originalMessage.timestamp;
+      const editTimestamp = Date.now() + 1;
+      const editMessage: Proto.IEditMessage = createEditedMessage(
+        targetSentTimestamp,
+        'v2',
+        editTimestamp
+      );
+      const timestamp = Number(editMessage.dataMessage?.timestamp);
+      drop(phone.sendRaw(desktop, wrap({ editMessage }), { timestamp }));
+
+      debug('sending original message', originalMessageTimestamp);
+      const sendOptions = {
+        timestamp: originalMessageTimestamp,
+      };
+      drop(
+        phone.sendRaw(
+          desktop,
+          wrap({ dataMessage: originalMessage }),
+          sendOptions
+        )
+      );
+
+      const window = await app.getWindow();
+
+      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 >> "v2"').waitFor();
+      const messages = window.locator('.module-message__text');
+      assert.strictEqual(await messages.count(), 1, 'message count');
+      const quotes = window.locator('.module-quote');
+      assert.strictEqual(await quotes.count(), 0, 'quote count');
+    });
+
     it('tracks message send state for edits', async () => {
       const { contacts, desktop } = bootstrap;
 
@@ -550,7 +637,9 @@ describe('editing', function (this: Mocha.Suite) {
       assert.strictEqual(editMessageV3.dataMessage?.body, editMessageV3Text);
       strictAssert(editMessageV3.dataMessage?.timestamp, 'timestamp missing');
 
-    const editMessageV3Timestamp = Number(editMessageV3.dataMessage.timestamp);
+      const editMessageV3Timestamp = Number(
+        editMessageV3.dataMessage.timestamp
+      );
       debug('v3 message', { timestamp: editMessageV3Timestamp });
 
       // Sending a v4 edited message targetting v3
@@ -575,7 +664,9 @@ describe('editing', function (this: Mocha.Suite) {
       assert.strictEqual(editMessageV4.dataMessage?.body, editMessageV4Text);
       strictAssert(editMessageV4.dataMessage?.timestamp, 'timestamp missing');
 
-    const editMessageV4Timestamp = Number(editMessageV4.dataMessage.timestamp);
+      const editMessageV4Timestamp = Number(
+        editMessageV4.dataMessage.timestamp
+      );
       debug('v4 message', { timestamp: editMessageV4Timestamp });
 
       {
@@ -660,3 +751,129 @@ describe('editing', function (this: Mocha.Suite) {
       }
     });
   });
+
+  describe('offline', function (this: Mocha.Suite) {
+    beforeEach(async () => {
+      await bootstrap.linkAndClose();
+    });
+
+    it('is fine with out of order edits with quotes removed', async () => {
+      const { phone, desktop } = bootstrap;
+
+      const originalMessage = createMessageWithQuote('v1');
+      const originalMessageTimestamp = Number(originalMessage.timestamp);
+
+      debug('sending edit');
+      const targetSentTimestamp = originalMessage.timestamp;
+      const editTimestamp = Date.now() + 1;
+      const editMessage: Proto.IEditMessage = createEditedMessage(
+        targetSentTimestamp,
+        'v2',
+        editTimestamp
+      );
+      const timestamp = Number(editMessage.dataMessage?.timestamp);
+      drop(phone.sendRaw(desktop, wrap({ editMessage }), { timestamp }));
+
+      debug('sending original message', originalMessageTimestamp);
+      const sendOptions = {
+        timestamp: originalMessageTimestamp,
+      };
+      drop(
+        phone.sendRaw(
+          desktop,
+          wrap({ dataMessage: originalMessage }),
+          sendOptions
+        )
+      );
+
+      app = await bootstrap.startApp();
+      const window = await app.getWindow();
+
+      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 >> "v2"').waitFor();
+      const messages = window.locator('.module-message__text');
+      assert.strictEqual(await messages.count(), 1, 'message count');
+      const quotes = window.locator('.module-quote');
+      assert.strictEqual(await quotes.count(), 0, 'quote count');
+    });
+
+    it('is fine with out of order edit processing', async () => {
+      const { phone, desktop } = bootstrap;
+
+      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);
+          };
+        });
+        drop(Promise.all(sendEditMessages.reverse().map(f => f())));
+        drop(sendOriginalMessage());
+      }
+
+      app = await bootstrap.startApp();
+      const window = await app.getWindow();
+      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/attachmentDownloadQueue.ts

@@ -11,11 +11,20 @@ const MAX_ATTACHMENT_MSGS_TO_DOWNLOAD = 250;
 
 let isEnabled = true;
 let attachmentDownloadQueue: Array<MessageModel> | undefined = [];
+const queueEmptyCallbacks: Set<() => void> = new Set();
 
 export function shouldUseAttachmentDownloadQueue(): boolean {
   return isEnabled;
 }
 
+export function isAttachmentDownloadQueueEmpty(): boolean {
+  return !(attachmentDownloadQueue ?? []).length;
+}
+
+export function registerQueueEmptyCallback(callback: () => void): void {
+  queueEmptyCallbacks.add(callback);
+}
+
 export function addToAttachmentDownloadQueue(
   idLog: string,
   message: MessageModel
@@ -71,4 +80,6 @@ export async function flushAttachmentDownloadQueue(): Promise<void> {
   });
 
   attachmentDownloadQueue = undefined;
+  queueEmptyCallbacks.forEach(callback => callback());
+  queueEmptyCallbacks.clear();
 }