Update quote behavior in backups

ts/jobs/helpers/sendNormalMessage.ts

@@ -905,7 +905,7 @@ async function uploadMessageQuote({
 
   return {
     isGiftBadge: loadedQuote.isGiftBadge,
-    id: loadedQuote.id,
+    id: loadedQuote.id ?? undefined,
     authorAci: loadedQuote.authorAci
       ? normalizeAci(loadedQuote.authorAci, 'sendNormalMessage.quote.authorAci')
       : undefined,

ts/messages/copyQuote.ts

@@ -55,7 +55,6 @@ export const copyFromQuotedMessage = async (
     referencedMessageNotFound: false,
     isGiftBadge: quote.type === SignalService.DataMessage.Quote.Type.GIFT_BADGE,
     isViewOnce: false,
-    messageId: '',
   };
 
   const queryMessage = await messageCache.findBySentAt(id, attributes =>

ts/messages/helpers.ts

@@ -117,7 +117,7 @@ export function getPaymentEventDescription(
 export function isQuoteAMatch(
   message: ReadonlyMessageAttributesType | null | undefined,
   conversationId: string,
-  quote: ReadonlyDeep<Pick<QuotedMessageType, 'id' | 'authorAci' | 'author'>>
+  quote: ReadonlyDeep<Pick<QuotedMessageType, 'id' | 'authorAci'>>
 ): message is ReadonlyMessageAttributesType {
   if (!message) {
     return false;

ts/model-types.d.ts

@@ -85,7 +85,6 @@ export type QuotedAttachmentType = {
 };
 
 export type QuotedMessageType = {
-  // TODO DESKTOP-3826
   attachments: ReadonlyArray<QuotedAttachmentType>;
   payment?: AnyPaymentEvent;
   // `author` is an old attribute that holds the author's E164. We shouldn't use it for
@@ -93,10 +92,13 @@ export type QuotedMessageType = {
   author?: string;
   authorAci?: AciString;
   bodyRanges?: ReadonlyArray<RawBodyRange>;
-  id: number;
+  // id can be null if the referenced message was not found and we imported this quote
+  // from backup
+  id: number | null;
   isGiftBadge?: boolean;
   isViewOnce: boolean;
-  messageId: string;
+  // `messageId` is deprecated
+  messageId?: string;
   referencedMessageNotFound: boolean;
   text?: string;
 };

ts/services/backups/export.ts

@@ -2126,7 +2126,10 @@ export class BackupExportStream extends Readable {
     }
 
     return {
-      targetSentTimestamp: Long.fromNumber(quote.id),
+      targetSentTimestamp:
+        quote.referencedMessageNotFound || quote.id == null
+          ? null
+          : Long.fromNumber(quote.id),
       authorId,
       text:
         quote.text != null

ts/services/backups/import.ts

@@ -89,10 +89,8 @@ import {
   convertBackupMessageAttachmentToAttachment,
   convertFilePointerToAttachment,
 } from './util/filePointers';
-import { CircularMessageCache } from './util/CircularMessageCache';
 import { filterAndClean } from '../../types/BodyRange';
 import { APPLICATION_OCTET_STREAM, stringToMIMEType } from '../../types/MIME';
-import { copyFromQuotedMessage } from '../../messages/copyQuote';
 import { groupAvatarJobQueue } from '../../jobs/groupAvatarJobQueue';
 import { AttachmentDownloadManager } from '../../jobs/AttachmentDownloadManager';
 import {
@@ -119,9 +117,6 @@ const MAX_CONCURRENCY = 10;
 
 const SAVE_MESSAGE_BATCH_SIZE = 10000;
 
-// Keep 1000 recent messages in memory to speed up quote lookup.
-const RECENT_MESSAGES_CACHE_SIZE = 1000;
-
 type ChatItemParseResult = {
   message: Partial<MessageAttributesType>;
   additionalMessages: Array<Partial<MessageAttributesType>>;
@@ -219,10 +214,6 @@ export class BackupImportStream extends Writable {
   private releaseNotesRecipientId: Long | undefined;
   private releaseNotesChatId: Long | undefined;
   private pendingGroupAvatars = new Map<string, string>();
-  private recentMessages = new CircularMessageCache({
-    size: RECENT_MESSAGES_CACHE_SIZE,
-    flush: () => this.flushMessages(),
-  });
 
   private constructor(private readonly backupType: BackupType) {
     super({ objectMode: true });
@@ -470,7 +461,6 @@ export class BackupImportStream extends Writable {
   }
 
   private async saveMessage(attributes: MessageAttributesType): Promise<void> {
-    this.recentMessages.push(attributes);
     this.saveMessageBatch.add(attributes);
     if (this.saveMessageBatch.size >= SAVE_MESSAGE_BATCH_SIZE) {
       return this.flushMessages();
@@ -1327,7 +1317,7 @@ export class BackupImportStream extends Writable {
     if (item.standardMessage) {
       attributes = {
         ...attributes,
-        ...(await this.fromStandardMessage(item.standardMessage, chatConvo.id)),
+        ...(await this.fromStandardMessage(item.standardMessage)),
       };
     } else if (item.viewOnceMessage) {
       attributes = {
@@ -1561,8 +1551,7 @@ export class BackupImportStream extends Writable {
   }
 
   private async fromStandardMessage(
-    data: Backups.IStandardMessage,
+    data: Backups.IStandardMessage
-    conversationId: string
   ): Promise<Partial<MessageAttributesType>> {
     return {
       body: data.text?.body || undefined,
@@ -1591,9 +1580,7 @@ export class BackupImportStream extends Writable {
           })
         : undefined,
       reactions: this.fromReactions(data.reactions),
-      quote: data.quote
+      quote: data.quote ? await this.fromQuote(data.quote) : undefined,
-        ? await this.fromQuote(data.quote, conversationId)
-        : undefined,
     };
   }
 
@@ -1644,10 +1631,7 @@ export class BackupImportStream extends Writable {
           } = this.fromDirectionDetails(rev, timestamp);
 
           return {
-            ...(await this.fromStandardMessage(
+            ...(await this.fromStandardMessage(rev.standardMessage)),
-              rev.standardMessage,
-              mainMessage.conversationId
-            )),
             timestamp,
             received_at: incrementMessageCounter(),
             sendStateByConversationId,
@@ -1685,29 +1669,7 @@ export class BackupImportStream extends Writable {
     return result;
   }
 
-  private convertQuoteType(
+  private async fromQuote(quote: Backups.IQuote): Promise<QuotedMessageType> {
-    type: Backups.Quote.Type | null | undefined
-  ): SignalService.DataMessage.Quote.Type {
-    switch (type) {
-      case Backups.Quote.Type.GIFT_BADGE:
-        return SignalService.DataMessage.Quote.Type.GIFT_BADGE;
-      case Backups.Quote.Type.VIEW_ONCE:
-        // No special treatment, we'll compute it once we find the message
-        return SignalService.DataMessage.Quote.Type.NORMAL;
-      case Backups.Quote.Type.NORMAL:
-      case Backups.Quote.Type.UNKNOWN:
-      case null:
-      case undefined:
-        return SignalService.DataMessage.Quote.Type.NORMAL;
-      default:
-        throw missingCaseError(type);
-    }
-  }
-
-  private async fromQuote(
-    quote: Backups.IQuote,
-    conversationId: string
-  ): Promise<QuotedMessageType> {
     strictAssert(quote.authorId != null, 'quote must have an authorId');
 
     const authorConvo = this.recipientIdToConvo.get(quote.authorId.toNumber());
@@ -1717,12 +1679,14 @@ export class BackupImportStream extends Writable {
       'must have ACI for authorId in quote'
     );
 
-    return copyFromQuotedMessage(
+    return {
-      {
+      id: getTimestampFromLong(quote.targetSentTimestamp) || null,
-        id: getTimestampFromLong(quote.targetSentTimestamp),
+      referencedMessageNotFound: quote.targetSentTimestamp == null,
       authorAci: authorConvo.serviceId,
       text: dropNull(quote.text?.body),
       bodyRanges: this.fromBodyRanges(quote.text),
+      isGiftBadge: quote.type === Backups.Quote.Type.GIFT_BADGE,
+      isViewOnce: quote.type === Backups.Quote.Type.VIEW_ONCE,
       attachments:
         quote.attachments?.map(quotedAttachment => {
           const { fileName, contentType, thumbnail } = quotedAttachment;
@@ -1736,13 +1700,7 @@ export class BackupImportStream extends Writable {
               : undefined,
           };
         }) ?? [],
-        type: this.convertQuoteType(quote.type),
+    };
-      },
-      conversationId,
-      {
-        messageCache: this.recentMessages,
-      }
-    );
   }
 
   private fromBodyRanges(

ts/services/backups/util/CircularMessageCache.ts

@@ -1,80 +0,0 @@
-// Copyright 2024 Signal Messenger, LLC
-// SPDX-License-Identifier: AGPL-3.0-only
-
-import type {
-  ReadonlyMessageAttributesType,
-  MessageAttributesType,
-} from '../../../model-types.d';
-import { find } from '../../../util/iterables';
-import { DataReader } from '../../../sql/Client';
-
-export type CircularMessageCacheOptionsType = Readonly<{
-  size: number;
-  flush: () => Promise<void>;
-}>;
-
-export class CircularMessageCache {
-  private readonly flush: () => Promise<void>;
-  private readonly buffer: Array<MessageAttributesType | undefined>;
-  private readonly sentAtToMessages = new Map<
-    number,
-    Set<MessageAttributesType>
-  >();
-  private offset = 0;
-
-  constructor({ size, flush }: CircularMessageCacheOptionsType) {
-    this.flush = flush;
-    this.buffer = new Array(size);
-  }
-
-  public push(attributes: MessageAttributesType): void {
-    const stale = this.buffer[this.offset];
-    this.buffer[this.offset] = attributes;
-    this.offset = (this.offset + 1) % this.buffer.length;
-
-    let addedSet = this.sentAtToMessages.get(attributes.sent_at);
-    if (addedSet === undefined) {
-      addedSet = new Set();
-      this.sentAtToMessages.set(attributes.sent_at, addedSet);
-    }
-    addedSet.add(attributes);
-
-    if (stale === undefined) {
-      return;
-    }
-
-    const staleSet = this.sentAtToMessages.get(stale.sent_at);
-    if (staleSet === undefined) {
-      return;
-    }
-    staleSet.delete(stale);
-    if (staleSet.size === 0) {
-      this.sentAtToMessages.delete(stale.sent_at);
-    }
-  }
-
-  public async findBySentAt(
-    sentAt: number,
-    predicate: (attributes: ReadonlyMessageAttributesType) => boolean
-  ): Promise<MessageAttributesType | undefined> {
-    const set = this.sentAtToMessages.get(sentAt);
-    if (set !== undefined) {
-      const cached = find(set.values(), predicate);
-      if (cached != null) {
-        return cached;
-      }
-    }
-
-    await this.flush();
-
-    const onDisk = await DataReader.getMessagesBySentAt(sentAt);
-    return onDisk.find(predicate);
-  }
-
-  // Just a stub to conform with the interface
-  public async upgradeSchema(
-    attributes: MessageAttributesType
-  ): Promise<MessageAttributesType> {
-    return attributes;
-  }
-}

ts/test-electron/backup/CircularMessageCache_test.ts

@@ -1,74 +0,0 @@
-// Copyright 2024 Signal Messenger, LLC
-// SPDX-License-Identifier: AGPL-3.0-only
-
-import { assert } from 'chai';
-import * as sinon from 'sinon';
-
-import { generateAci } from '../../types/ServiceId';
-import { type MessageAttributesType } from '../../model-types.d';
-import { CircularMessageCache } from '../../services/backups/util/CircularMessageCache';
-import { DataWriter } from '../../sql/Client';
-
-const OUR_ACI = generateAci();
-
-function createMessage(sentAt: number): MessageAttributesType {
-  return {
-    sent_at: sentAt,
-    received_at: sentAt,
-    timestamp: sentAt,
-
-    id: 'abc',
-    type: 'incoming' as const,
-    conversationId: 'cid',
-  };
-}
-
-describe('backup/attachments', () => {
-  let messageCache: CircularMessageCache;
-  let flush: sinon.SinonStub;
-
-  beforeEach(async () => {
-    await DataWriter.removeAll();
-    flush = sinon.stub();
-    messageCache = new CircularMessageCache({
-      size: 2,
-      flush,
-    });
-  });
-
-  afterEach(async () => {
-    await DataWriter.removeAll();
-  });
-
-  it('should return a cached message', async () => {
-    const message = createMessage(123);
-    messageCache.push(message);
-
-    const found = await messageCache.findBySentAt(123, () => true);
-    sinon.assert.notCalled(flush);
-    assert.strictEqual(found, message);
-  });
-
-  it('should purge message from cache on overflow', async () => {
-    messageCache.push(createMessage(123));
-    messageCache.push(createMessage(124));
-    messageCache.push(createMessage(125));
-
-    const found = await messageCache.findBySentAt(123, () => true);
-    sinon.assert.calledOnce(flush);
-    assert.isUndefined(found);
-  });
-
-  it('should find message in the database', async () => {
-    const message = createMessage(123);
-
-    await DataWriter.saveMessage(message, {
-      ourAci: OUR_ACI,
-      forceSave: true,
-    });
-
-    const found = await messageCache.findBySentAt(123, () => true);
-    sinon.assert.calledOnce(flush);
-    assert.deepStrictEqual(found, message);
-  });
-});

ts/test-electron/backup/attachments_test.ts

@@ -486,7 +486,6 @@ describe('backup/attachments', () => {
         isViewOnce: false,
         id: Date.now(),
         referencedMessageNotFound: false,
-        messageId: '',
         isGiftBadge: true,
         attachments: [{ thumbnail: attachment, contentType: VIDEO_MP4 }],
       };
@@ -502,7 +501,6 @@ describe('backup/attachments', () => {
           composeMessage(1, {
             quote: {
               ...quotedMessage,
-              referencedMessageNotFound: true,
               attachments: [
                 {
                   thumbnail: omit(attachment, NON_ROUNDTRIPPED_FIELDS),
@@ -524,7 +522,6 @@ describe('backup/attachments', () => {
         isViewOnce: false,
         id: Date.now(),
         referencedMessageNotFound: false,
-        messageId: '',
         isGiftBadge: true,
         attachments: [{ thumbnail: attachment, contentType: VIDEO_MP4 }],
       };
@@ -539,7 +536,6 @@ describe('backup/attachments', () => {
           composeMessage(1, {
             quote: {
               ...quotedMessage,
-              referencedMessageNotFound: true,
               attachments: [
                 {
                   thumbnail: {
@@ -575,7 +571,6 @@ describe('backup/attachments', () => {
         isViewOnce: false,
         id: existingMessageTimestamp,
         referencedMessageNotFound: false,
-        messageId: '',
         isGiftBadge: false,
         attachments: [{ thumbnail: quoteAttachment, contentType: VIDEO_MP4 }],
       };
@@ -636,7 +631,6 @@ describe('backup/attachments', () => {
         isViewOnce: false,
         id: originalMessage.timestamp,
         referencedMessageNotFound: false,
-        messageId: '',
         isGiftBadge: false,
         attachments: [
           {

ts/test-electron/backup/bubble_test.ts

@@ -229,6 +229,7 @@ describe('backup/bubble messages', () => {
     ]);
   });
 
+  describe('quotes', () => {
     it('roundtrips gift badge quote', async () => {
       await symmetricRoundtripHarness([
         {
@@ -269,12 +270,77 @@ describe('backup/bubble messages', () => {
             id: 3,
             isViewOnce: false,
             isGiftBadge: true,
-          messageId: '',
             referencedMessageNotFound: false,
           },
         },
       ]);
     });
+    it('roundtrips quote with referenced message found', async () => {
+      await symmetricRoundtripHarness([
+        {
+          conversationId: contactA.id,
+          id: generateGuid(),
+          type: 'incoming',
+          received_at: 3,
+          received_at_ms: 3,
+          sent_at: 3,
+          sourceServiceId: CONTACT_A,
+          readStatus: ReadStatus.Unread,
+          seenStatus: SeenStatus.Unseen,
+          unidentifiedDeliveryReceived: true,
+          timestamp: 3,
+          quote: {
+            authorAci: CONTACT_A,
+            attachments: [],
+            id: 42,
+            isGiftBadge: false,
+            text: 'quote text',
+            isViewOnce: false,
+            referencedMessageNotFound: false,
+          },
+        },
+      ]);
+    });
+    it('roundtrips quote without referenced message found', async () => {
+      const message = {
+        conversationId: contactA.id,
+        id: generateGuid(),
+        type: 'incoming',
+        received_at: 3,
+        received_at_ms: 3,
+        sent_at: 3,
+        sourceServiceId: CONTACT_A,
+        readStatus: ReadStatus.Unread,
+        seenStatus: SeenStatus.Unseen,
+        unidentifiedDeliveryReceived: true,
+        timestamp: 3,
+        quote: {
+          authorAci: CONTACT_A,
+          attachments: [],
+          id: 42,
+          text: 'quote text',
+          isViewOnce: false,
+          isGiftBadge: false,
+          referencedMessageNotFound: true,
+        },
+      } as const;
+
+      await asymmetricRoundtripHarness(
+        [message],
+        [
+          {
+            ...message,
+            quote: {
+              ...message.quote,
+              // id is removed during roundtrip
+              id: null,
+            },
+          },
+        ]
+      );
+    });
+    // TODO (DESKTOP-7899): Roundtrip view-once quotes
+  });
 
   it('roundtrips sealed/unsealed incoming message', async () => {
     await symmetricRoundtripHarness([

ts/util/getQuoteBodyText.ts

@@ -6,10 +6,11 @@ import * as EmbeddedContact from '../types/EmbeddedContact';
 
 export function getQuoteBodyText(
   messageAttributes: ReadonlyMessageAttributesType,
-  id: number
+  id: number | null
 ): string | undefined {
   const storyReactionEmoji = messageAttributes.storyReaction?.emoji;
 
+  if (id != null) {
     const { editHistory } = messageAttributes;
     const editedMessage =
       editHistory && editHistory.find(edit => edit.timestamp === id);
@@ -17,6 +18,7 @@ export function getQuoteBodyText(
     if (editedMessage && editedMessage.body) {
       return editedMessage.body;
     }
+  }
 
   const { body, contact: embeddedContact } = messageAttributes;
   const embeddedContactName =

ts/util/makeQuote.ts

@@ -27,14 +27,7 @@ export async function makeQuote(
 
   strictAssert(contact, 'makeQuote: no contact');
 
-  const {
+  const { attachments, bodyRanges, payment, preview, sticker } = quotedMessage;
-    attachments,
-    bodyRanges,
-    id: messageId,
-    payment,
-    preview,
-    sticker,
-  } = quotedMessage;
 
   const quoteId = getMessageSentTimestamp(quotedMessage, { log });
 
@@ -48,7 +41,6 @@ export async function makeQuote(
     id: quoteId,
     isViewOnce: isTapToView(quotedMessage),
     isGiftBadge: isGiftBadge(quotedMessage),
-    messageId,
     referencedMessageNotFound: false,
     text: getQuoteBodyText(quotedMessage, quoteId),
   };