Simplify copyQuote logic

ts/messages/copyQuote.ts

@@ -4,16 +4,14 @@
 import { omit } from 'lodash';
 
 import * as log from '../logging/log';
-import { DataReader, DataWriter } from '../sql/Client';
 import type { QuotedMessageType } from '../model-types';
-import type { MessageModel } from '../models/messages';
+import type { MessageAttributesType } from '../model-types.d';
 import { SignalService } from '../protobuf';
 import { isGiftBadge, isTapToView } from '../state/selectors/message';
 import type { ProcessedQuote } from '../textsecure/Types';
 import { IMAGE_JPEG } from '../types/MIME';
 import { strictAssert } from '../util/assert';
 import { getQuoteBodyText } from '../util/getQuoteBodyText';
-import { find } from '../util/iterables';
 import { isQuoteAMatch, messageHasPaymentEvent } from './helpers';
 import * as Errors from '../types/errors';
 import { isDownloadable } from '../types/Attachment';
@@ -40,32 +38,13 @@ export const copyFromQuotedMessage = async (
     messageId: '',
   };
 
-  const inMemoryMessages = window.MessageCache.__DEPRECATED$filterBySentAt(id);
+  const queryMessage = await window.MessageCache.findBySentAt(id, attributes =>
-  const matchingMessage = find(inMemoryMessages, item =>
+    isQuoteAMatch(attributes, conversationId, result)
-    isQuoteAMatch(item.attributes, conversationId, result)
   );
 
-  let queryMessage: undefined | MessageModel;
+  if (queryMessage == null) {
-
+    result.referencedMessageNotFound = true;
-  if (matchingMessage) {
+    return result;
-    queryMessage = matchingMessage;
-  } else {
-    log.info('copyFromQuotedMessage: db lookup needed', id);
-    const messages = await DataReader.getMessagesBySentAt(id);
-    const found = messages.find(item =>
-      isQuoteAMatch(item, conversationId, result)
-    );
-
-    if (!found) {
-      result.referencedMessageNotFound = true;
-      return result;
-    }
-
-    queryMessage = window.MessageCache.__DEPRECATED$register(
-      found.id,
-      found,
-      'copyFromQuotedMessage'
-    );
   }
 
   if (queryMessage) {
@@ -76,18 +55,20 @@ export const copyFromQuotedMessage = async (
 };
 
 export const copyQuoteContentFromOriginal = async (
-  originalMessage: MessageModel,
+  providedOriginalMessage: MessageAttributesType,
   quote: QuotedMessageType
 ): Promise<void> => {
+  let originalMessage = providedOriginalMessage;
+
   const { attachments } = quote;
   const firstAttachment = attachments ? attachments[0] : undefined;
 
-  if (messageHasPaymentEvent(originalMessage.attributes)) {
+  if (messageHasPaymentEvent(originalMessage)) {
     // eslint-disable-next-line no-param-reassign
-    quote.payment = originalMessage.get('payment');
+    quote.payment = originalMessage.payment;
   }
 
-  if (isTapToView(originalMessage.attributes)) {
+  if (isTapToView(originalMessage)) {
     // eslint-disable-next-line no-param-reassign
     quote.text = undefined;
     // eslint-disable-next-line no-param-reassign
@@ -102,7 +83,7 @@ export const copyQuoteContentFromOriginal = async (
     return;
   }
 
-  const isMessageAGiftBadge = isGiftBadge(originalMessage.attributes);
+  const isMessageAGiftBadge = isGiftBadge(originalMessage);
   if (isMessageAGiftBadge !== quote.isGiftBadge) {
     log.warn(
       `copyQuoteContentFromOriginal: Quote.isGiftBadge: ${quote.isGiftBadge}, isGiftBadge(message): ${isMessageAGiftBadge}`
@@ -123,30 +104,20 @@ export const copyQuoteContentFromOriginal = async (
   quote.isViewOnce = false;
 
   // eslint-disable-next-line no-param-reassign
-  quote.text = getQuoteBodyText(originalMessage.attributes, quote.id);
+  quote.text = getQuoteBodyText(originalMessage, quote.id);
 
   // eslint-disable-next-line no-param-reassign
-  quote.bodyRanges = originalMessage.attributes.bodyRanges;
+  quote.bodyRanges = originalMessage.bodyRanges;
 
   if (!firstAttachment || !firstAttachment.contentType) {
     return;
   }
 
   try {
-    const schemaVersion = originalMessage.get('schemaVersion');
+    originalMessage = await window.MessageCache.upgradeSchema(
-    if (
+      originalMessage,
-      schemaVersion &&
+      window.Signal.Types.Message.VERSION_NEEDED_FOR_DISPLAY
-      schemaVersion < window.Signal.Types.Message.VERSION_NEEDED_FOR_DISPLAY
+    );
-    ) {
-      const upgradedMessage =
-        await window.Signal.Migrations.upgradeMessageSchema(
-          originalMessage.attributes
-        );
-      originalMessage.set(upgradedMessage);
-      await DataWriter.saveMessage(upgradedMessage, {
-        ourAci: window.textsecure.storage.user.getCheckedAci(),
-      });
-    }
   } catch (error) {
     log.error(
       'Problem upgrading message quoted message from database',
@@ -155,7 +126,12 @@ export const copyQuoteContentFromOriginal = async (
     return;
   }
 
-  const queryAttachments = originalMessage.get('attachments') || [];
+  const {
+    attachments: queryAttachments = [],
+    preview: queryPreview = [],
+    sticker,
+  } = originalMessage;
+
   if (queryAttachments.length > 0) {
     const queryFirst = queryAttachments[0];
     const { thumbnail } = queryFirst;
@@ -175,7 +151,6 @@ export const copyQuoteContentFromOriginal = async (
     }
   }
 
-  const queryPreview = originalMessage.get('preview') || [];
   if (queryPreview.length > 0) {
     const queryFirst = queryPreview[0];
     const { image } = queryFirst;
@@ -188,7 +163,6 @@ export const copyQuoteContentFromOriginal = async (
     }
   }
 
-  const sticker = originalMessage.get('sticker');
   if (sticker && sticker.data && sticker.data.path) {
     firstAttachment.thumbnail = {
       ...sticker.data,

ts/models/conversations.ts

@@ -1932,25 +1932,15 @@ export class ConversationModel extends window.Backbone
         `cleanAttributes: Eliminated ${eliminated} messages without an id`
       );
     }
-    const ourAci = window.textsecure.storage.user.getCheckedAci();
 
     let upgraded = 0;
     const hydrated = await Promise.all(
       present.map(async message => {
-        const { schemaVersion } = message;
+        const upgradedMessage = await window.MessageCache.upgradeSchema(
-
-        const model = window.MessageCache.__DEPRECATED$register(
-          message.id,
           message,
-          'cleanAttributes'
+          Message.VERSION_NEEDED_FOR_DISPLAY
         );
-
+        if (upgradedMessage !== message) {
-        let upgradedMessage = message;
-        if ((schemaVersion || 0) < Message.VERSION_NEEDED_FOR_DISPLAY) {
-          // Yep, we really do want to wait for each of these
-          upgradedMessage = await upgradeMessageSchema(model.attributes);
-          model.set(upgradedMessage);
-          await DataWriter.saveMessage(upgradedMessage, { ourAci });
           upgraded += 1;
         }
 

ts/models/messages.ts

@@ -18,7 +18,7 @@ import type {
   MessageAttributesType,
   MessageReactionType,
 } from '../model-types.d';
-import { filter, find, map, repeat, zipObject } from '../util/iterables';
+import { filter, map, repeat, zipObject } from '../util/iterables';
 import * as GoogleChrome from '../util/GoogleChrome';
 import type { DeleteAttributesType } from '../messageModifiers/Deletes';
 import type { SentEventData } from '../textsecure/messageReceiverEvents';
@@ -454,25 +454,11 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
         quoteAttachment: quote.attachments.at(0),
       })
     ) {
-      const inMemoryMessages = window.MessageCache.__DEPRECATED$filterBySentAt(
+      const matchingMessage = await window.MessageCache.findBySentAt(
-        Number(sentAt)
+        Number(sentAt),
+        attributes =>
+          isQuoteAMatch(attributes, this.get('conversationId'), quote)
       );
-      let matchingMessage = find(inMemoryMessages, message =>
-        isQuoteAMatch(message.attributes, this.get('conversationId'), quote)
-      );
-      if (!matchingMessage) {
-        const messages = await DataReader.getMessagesBySentAt(Number(sentAt));
-        const found = messages.find(item =>
-          isQuoteAMatch(item, this.get('conversationId'), quote)
-        );
-        if (found) {
-          matchingMessage = window.MessageCache.__DEPRECATED$register(
-            found.id,
-            found,
-            'doubleCheckMissingQuoteReference'
-          );
-        }
-      }
 
       if (!matchingMessage) {
         log.info(
@@ -1737,11 +1723,11 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
 
       const storyQuote = storyQuotes.find(candidateQuote => {
         const sendStateByConversationId =
-          candidateQuote.get('sendStateByConversationId') || {};
+          candidateQuote.sendStateByConversationId || {};
         const sendState = sendStateByConversationId[sender.id];
 
         const storyQuoteIsFromSelf =
-          candidateQuote.get('sourceServiceId') ===
+          candidateQuote.sourceServiceId ===
           window.storage.user.getCheckedAci();
 
         if (!storyQuoteIsFromSelf) {
@@ -1776,9 +1762,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
       }
 
       if (storyQuote) {
-        const storyDistributionListId = storyQuote.get(
+        const { storyDistributionListId } = storyQuote;
-          'storyDistributionListId'
-        );
 
         if (storyDistributionListId) {
           const storyDistribution =
@@ -1904,7 +1888,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
         });
 
         if (storyQuote) {
-          await this.hydrateStoryContext(storyQuote.attributes, {
+          await this.hydrateStoryContext(storyQuote, {
             shouldSave: true,
           });
         }
@@ -1940,10 +1924,8 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
           // expiration timer
           if (isGroupStoryReply && storyQuote) {
             message.set({
-              expireTimer: storyQuote.get('expireTimer'),
+              expireTimer: storyQuote.expireTimer,
-              expirationStartTimestamp: storyQuote.get(
+              expirationStartTimestamp: storyQuote.expirationStartTimestamp,
-                'expirationStartTimestamp'
-              ),
             });
           }
 

ts/services/MessageCache.ts

@@ -4,7 +4,10 @@
 import cloneDeep from 'lodash/cloneDeep';
 import { throttle } from 'lodash';
 import LRU from 'lru-cache';
-import type { MessageAttributesType } from '../model-types.d';
+import type {
+  MessageAttributesType,
+  ReadonlyMessageAttributesType,
+} from '../model-types.d';
 import type { MessageModel } from '../models/messages';
 import { DataReader, DataWriter } from '../sql/Client';
 import * as Errors from '../types/errors';
@@ -14,7 +17,6 @@ import { getMessageConversation } from '../util/getMessageConversation';
 import { getMessageModelLogger } from '../util/MessageModelLogger';
 import { getSenderIdentifier } from '../util/getSenderIdentifier';
 import { isNotNil } from '../util/isNotNil';
-import { map } from '../util/iterables';
 import { softAssert, strictAssert } from '../util/assert';
 import { isStory } from '../messages/helpers';
 import type { SendStateByConversationId } from '../messages/MessageSendState';
@@ -451,11 +453,47 @@ export class MessageCache {
     return this.toModel(data);
   }
 
+  public async upgradeSchema(
+    attributes: MessageAttributesType,
+    minSchemaVersion: number
+  ): Promise<MessageAttributesType> {
+    const { schemaVersion } = attributes;
+    if (!schemaVersion || schemaVersion >= minSchemaVersion) {
+      return attributes;
+    }
+    const upgradedAttributes =
+      await window.Signal.Migrations.upgradeMessageSchema(attributes);
+    await this.setAttributes({
+      messageId: upgradedAttributes.id,
+      messageAttributes: upgradedAttributes,
+      skipSaveToDatabase: false,
+    });
+    return upgradedAttributes;
+  }
+
   // Finds a message in the cache by sentAt/timestamp
-  public __DEPRECATED$filterBySentAt(sentAt: number): Iterable<MessageModel> {
+  public async findBySentAt(
+    sentAt: number,
+    predicate: (attributes: ReadonlyMessageAttributesType) => boolean
+  ): Promise<MessageAttributesType | undefined> {
     const items = this.state.messageIdsBySentAt.get(sentAt) ?? [];
-    const attrs = items.map(id => this.accessAttributes(id)).filter(isNotNil);
+    const inMemory = items
-    return map(attrs, data => this.toModel(data));
+      .map(id => this.accessAttributes(id))
+      .filter(isNotNil)
+      .find(predicate);
+
+    if (inMemory != null) {
+      return inMemory;
+    }
+
+    log.info(`findBySentAt(${sentAt}): db lookup needed`);
+    const allOnDisk = await DataReader.getMessagesBySentAt(sentAt);
+    const onDisk = allOnDisk.find(predicate);
+
+    if (onDisk != null) {
+      this.addMessageToCache(onDisk);
+    }
+    return onDisk;
   }
 
   // Marks cached model as "should be stale" to discourage continued use.

ts/state/ducks/mediaGallery.ts

@@ -6,7 +6,7 @@ import type { ReadonlyDeep } from 'type-fest';
 
 import * as log from '../../logging/log';
 import * as Errors from '../../types/errors';
-import { DataReader, DataWriter } from '../../sql/Client';
+import { DataReader } from '../../sql/Client';
 import {
   CONVERSATION_UNLOADED,
   MESSAGE_CHANGED,
@@ -17,6 +17,7 @@ import { VERSION_NEEDED_FOR_DISPLAY } from '../../types/Message2';
 import { isDownloading, hasFailed } from '../../types/Attachment';
 import { isNotNil } from '../../util/isNotNil';
 import { getLocalAttachmentUrl } from '../../util/getLocalAttachmentUrl';
+import { getMessageIdForLogging } from '../../util/idForLogging';
 import { useBoundActions } from '../../hooks/useBoundActions';
 
 import type { AttachmentType } from '../../types/Attachment';
@@ -191,34 +192,21 @@ function _cleanFileAttachments(
 async function _upgradeMessages(
   messages: ReadonlyArray<MessageAttributesType>
 ): Promise<ReadonlyArray<MessageAttributesType>> {
-  const { upgradeMessageSchema } = window.Signal.Migrations;
-  const ourAci = window.textsecure.storage.user.getCheckedAci();
-
   // We upgrade these messages so they are sure to have thumbnails
   const upgraded = await Promise.all(
     messages.map(async message => {
-      const { schemaVersion } = message;
-      const model = window.MessageCache.__DEPRECATED$register(
-        message.id,
-        message,
-        'loadMediaItems'
-      );
-
       try {
-        if (schemaVersion && schemaVersion < VERSION_NEEDED_FOR_DISPLAY) {
+        return await window.MessageCache.upgradeSchema(
-          const upgradedMsgAttributes = await upgradeMessageSchema(message);
+          message,
-          model.set(upgradedMsgAttributes);
+          VERSION_NEEDED_FOR_DISPLAY
-
+        );
-          await DataWriter.saveMessage(upgradedMsgAttributes, { ourAci });
-        }
       } catch (error) {
         log.warn(
-          `_upgradeMessages: Failed to upgrade message ${model.idForLogging()}: ${Errors.toLogFormat(error)}`
+          '_upgradeMessages: Failed to upgrade message ' +
+            `${getMessageIdForLogging(message)}: ${Errors.toLogFormat(error)}`
         );
         return undefined;
       }
-
-      return model.attributes;
     })
   );
 

ts/test-electron/services/MessageCache_test.ts

@@ -18,14 +18,14 @@ describe('MessageCache', () => {
     await window.ConversationController.load();
   });
 
-  describe('filterBySentAt', () => {
+  describe('findBySentAt', () => {
-    it('returns an empty iterable if no messages match', () => {
+    it('returns an empty iterable if no messages match', async () => {
       const mc = new MessageCache();
 
-      assert.isEmpty([...mc.__DEPRECATED$filterBySentAt(123)]);
+      assert.isUndefined(await mc.findBySentAt(123, () => true));
     });
 
-    it('returns all messages that match the timestamp', () => {
+    it('returns all messages that match the timestamp', async () => {
       const mc = new MessageCache();
 
       let message1 = new MessageModel({
@@ -62,23 +62,15 @@ describe('MessageCache', () => {
       message2 = mc.__DEPRECATED$register(message2.id, message2, 'test');
       mc.__DEPRECATED$register(message3.id, message3, 'test');
 
-      const filteredMessages = Array.from(
+      const filteredMessage = await mc.findBySentAt(1234, () => true);
-        mc.__DEPRECATED$filterBySentAt(1234)
-      ).map(x => x.attributes);
 
-      assert.deepEqual(
+      assert.deepEqual(filteredMessage, message1.attributes, 'first');
-        filteredMessages,
-        [message1.attributes, message2.attributes],
-        'first'
-      );
 
-      mc.__DEPRECATED$unregister(message2.id);
+      mc.__DEPRECATED$unregister(message1.id);
 
-      const filteredMessages2 = Array.from(
+      const filteredMessage2 = await mc.findBySentAt(1234, () => true);
-        mc.__DEPRECATED$filterBySentAt(1234)
-      ).map(x => x.attributes);
 
-      assert.deepEqual(filteredMessages2, [message1.attributes], 'second');
+      assert.deepEqual(filteredMessage2, message2.attributes, 'second');
     });
   });
 

ts/util/findStoryMessage.ts

@@ -1,21 +1,22 @@
 // Copyright 2022 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import type { ReadonlyMessageAttributesType } from '../model-types.d';
+import type {
-import type { MessageModel } from '../models/messages';
+  ReadonlyMessageAttributesType,
+  MessageAttributesType,
+} from '../model-types.d';
 import type { SignalService as Proto } from '../protobuf';
 import type { AciString } from '../types/ServiceId';
 import { DataReader } from '../sql/Client';
 import * as log from '../logging/log';
 import { normalizeAci } from './normalizeAci';
-import { filter } from './iterables';
 import { getAuthorId } from '../messages/helpers';
 import { getTimestampFromLong } from './timestampLongUtils';
 
 export async function findStoryMessages(
   conversationId: string,
   storyContext?: Proto.DataMessage.IStoryContext
-): Promise<Array<MessageModel>> {
+): Promise<Array<MessageAttributesType>> {
   if (!storyContext) {
     return [];
   }
@@ -32,25 +33,6 @@ export async function findStoryMessages(
   const ourConversationId =
     window.ConversationController.getOurConversationIdOrThrow();
 
-  const inMemoryMessages =
-    window.MessageCache.__DEPRECATED$filterBySentAt(sentAt);
-  const matchingMessages = [
-    ...filter(inMemoryMessages, item =>
-      isStoryAMatch(
-        item.attributes,
-        conversationId,
-        ourConversationId,
-        authorAci,
-        sentAt
-      )
-    ),
-  ];
-
-  if (matchingMessages.length > 0) {
-    return matchingMessages;
-  }
-
-  log.info('findStoryMessages: db lookup needed', sentAt);
   const messages = await DataReader.getMessagesBySentAt(sentAt);
   const found = messages.filter(item =>
     isStoryAMatch(item, conversationId, ourConversationId, authorAci, sentAt)
@@ -61,14 +43,7 @@ export async function findStoryMessages(
     return [];
   }
 
-  const result = found.map(attributes =>
+  return found;
-    window.MessageCache.__DEPRECATED$register(
-      attributes.id,
-      attributes,
-      'findStoryMessages'
-    )
-  );
-  return result;
 }
 
 function isStoryAMatch(
@@ -77,7 +52,7 @@ function isStoryAMatch(
   ourConversationId: string,
   authorAci: AciString,
   sentTimestamp: number
-): message is ReadonlyMessageAttributesType {
+): boolean {
   if (!message) {
     return false;
   }