Remove messageCollection from Conversation model

js/delivery_receipts.js

@@ -107,8 +107,11 @@
         const conversation = ConversationController.get(
           message.get('conversationId')
         );
-        if (conversation) {
+        const updateLeftPane = conversation
-          conversation.trigger('delivered', message);
+          ? conversation.debouncedUpdateLastMessage
+          : undefined;
+        if (updateLeftPane) {
+          updateLeftPane();
         }
 
         this.remove(receipt);

js/expiring_messages.js

@@ -41,7 +41,9 @@
 
         const conversation = message.getConversation();
         if (conversation) {
-          conversation.trigger('expired', message);
+          // An expired message only counts as decrementing the message count, not
+          // the sent message count
+          conversation.decrementMessageCount();
         }
       });
     } catch (error) {

js/expiring_tap_to_view_messages.js

@@ -29,7 +29,6 @@
             message.idForLogging()
           );
 
-          message.trigger('erased');
           await message.eraseContents();
         })
       );

js/message_controller.js

@@ -1,104 +0,0 @@
-// Copyright 2019-2020 Signal Messenger, LLC
-// SPDX-License-Identifier: AGPL-3.0-only
-
-// eslint-disable-next-line func-names
-(function () {
-  window.Whisper = window.Whisper || {};
-
-  const messageLookup = Object.create(null);
-  const msgIDsBySender = new Map();
-  const msgIDsBySentAt = new Map();
-
-  const SECOND = 1000;
-  const MINUTE = SECOND * 60;
-  const FIVE_MINUTES = MINUTE * 5;
-  const HOUR = MINUTE * 60;
-
-  function register(id, message) {
-    if (!id || !message) {
-      return message;
-    }
-
-    const existing = messageLookup[id];
-    if (existing) {
-      messageLookup[id] = {
-        message: existing.message,
-        timestamp: Date.now(),
-      };
-      return existing.message;
-    }
-
-    messageLookup[id] = {
-      message,
-      timestamp: Date.now(),
-    };
-
-    msgIDsBySentAt.set(message.get('sent_at'), id);
-    msgIDsBySender.set(message.getSenderIdentifier(), id);
-
-    return message;
-  }
-
-  function unregister(id) {
-    const { message } = messageLookup[id] || {};
-    if (message) {
-      msgIDsBySender.delete(message.getSenderIdentifier());
-      msgIDsBySentAt.delete(message.get('sent_at'));
-    }
-    delete messageLookup[id];
-  }
-
-  function cleanup() {
-    const messages = Object.values(messageLookup);
-    const now = Date.now();
-
-    for (let i = 0, max = messages.length; i < max; i += 1) {
-      const { message, timestamp } = messages[i];
-      const conversation = message.getConversation();
-
-      if (
-        now - timestamp > FIVE_MINUTES &&
-        (!conversation || !conversation.messageCollection.length)
-      ) {
-        unregister(message.id);
-      }
-    }
-  }
-
-  function getById(id) {
-    const existing = messageLookup[id];
-    return existing && existing.message ? existing.message : undefined;
-  }
-
-  function findBySentAt(sentAt) {
-    const id = msgIDsBySentAt.get(sentAt);
-    if (!id) {
-      return null;
-    }
-    return getById(id);
-  }
-
-  function findBySender(sender) {
-    const id = msgIDsBySender.get(sender);
-    if (!id) {
-      return null;
-    }
-    return getById(id);
-  }
-
-  function _get() {
-    return messageLookup;
-  }
-
-  setInterval(cleanup, HOUR);
-
-  window.MessageController = {
-    register,
-    unregister,
-    cleanup,
-    findBySender,
-    findBySentAt,
-    getById,
-    _get,
-  };
-})();

js/modules/attachment_downloads.js

@@ -281,14 +281,6 @@ async function _finishJob(message, id) {
     await saveMessage(message.attributes, {
       Message: Whisper.Message,
     });
-    const conversation = message.getConversation();
-    if (conversation) {
-      const fromConversation = conversation.messageCollection.get(message.id);
-
-      if (fromConversation && message !== fromConversation) {
-        fromConversation.set(message.attributes);
-      }
-    }
   }
 
   await removeAttachmentDownloadJob(id);

js/read_receipts.js

@@ -108,8 +108,11 @@
         const conversation = ConversationController.get(
           message.get('conversationId')
         );
-        if (conversation) {
+        const updateLeftPane = conversation
-          conversation.trigger('read', message);
+          ? conversation.debouncedUpdateLastMessage
+          : undefined;
+        if (updateLeftPane) {
+          updateLeftPane();
         }
 
         this.remove(receipt);

test/_test.js

@@ -75,6 +75,8 @@ function deleteIndexedDB() {
 
 /* Delete the database before running any tests */
 before(async () => {
+  window.Signal.Util.MessageController.install();
+
   await deleteIndexedDB();
   try {
     window.log.info('Initializing SQL in renderer');

test/backup_test.js

@@ -615,11 +615,11 @@ describe('Backup', () => {
         );
 
         console.log('Backup test: Check messages');
-        const messageCollection = await window.Signal.Data._getAllMessages({
+        const messages = await window.Signal.Data._getAllMessages({
           MessageCollection: Whisper.MessageCollection,
         });
-        assert.strictEqual(messageCollection.length, MESSAGE_COUNT);
+        assert.strictEqual(messages.length, MESSAGE_COUNT);
-        const messageFromDB = removeId(messageCollection.at(0).attributes);
+        const messageFromDB = removeId(messages.at(0).attributes);
         const expectedMessage = messageFromDB;
         console.log({ messageFromDB, expectedMessage });
         assert.deepEqual(messageFromDB, expectedMessage);

ts/background.ts

@@ -43,6 +43,7 @@ import * as universalExpireTimer from './util/universalExpireTimer';
 import { isDirectConversation, isGroupV2 } from './util/whatTypeOfConversation';
 import { getSendOptions } from './util/getSendOptions';
 import { BackOff } from './util/BackOff';
+import { AppViewType } from './state/ducks/app';
 import { actionCreators } from './state/actions';
 
 const MAX_ATTACHMENT_DOWNLOAD_AGE = 3600 * 72 * 1000;
@@ -70,6 +71,7 @@ export async function cleanupSessionResets(): Promise<void> {
 }
 
 export async function startApp(): Promise<void> {
+  window.Signal.Util.MessageController.install();
   window.startupProcessingQueue = new window.Signal.Util.StartupQueue();
   window.attachmentDownloadQueue = [];
   try {
@@ -1712,7 +1714,7 @@ export async function startApp(): Promise<void> {
     }
 
     window.Whisper.events.on('contactsync', () => {
-      if (window.reduxStore.getState().app.isShowingInstaller) {
+      if (window.reduxStore.getState().app.appView === AppViewType.Installer) {
         window.reduxActions.app.openInbox();
       }
     });

ts/components/conversation/MessageDetail.stories.tsx

@@ -47,8 +47,6 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
       }),
       isOutgoingKeyError: false,
       isUnidentifiedDelivery: false,
-      onSendAnyway: action('onSendAnyway'),
-      onShowSafetyNumber: action('onShowSafetyNumber'),
       status: 'delivered',
     },
   ],
@@ -60,6 +58,9 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
   i18n,
   interactionMode: 'keyboard',
 
+  sendAnyway: action('onSendAnyway'),
+  showSafetyNumber: action('onShowSafetyNumber'),
+
   clearSelectedMessage: () => null,
   deleteMessage: action('deleteMessage'),
   deleteMessageForEveryone: action('deleteMessageForEveryone'),
@@ -108,8 +109,6 @@ story.add('Message Statuses', () => {
         }),
         isOutgoingKeyError: false,
         isUnidentifiedDelivery: false,
-        onSendAnyway: action('onSendAnyway'),
-        onShowSafetyNumber: action('onShowSafetyNumber'),
         status: 'sent',
       },
       {
@@ -119,8 +118,6 @@ story.add('Message Statuses', () => {
         }),
         isOutgoingKeyError: false,
         isUnidentifiedDelivery: false,
-        onSendAnyway: action('onSendAnyway'),
-        onShowSafetyNumber: action('onShowSafetyNumber'),
         status: 'sending',
       },
       {
@@ -130,8 +127,6 @@ story.add('Message Statuses', () => {
         }),
         isOutgoingKeyError: false,
         isUnidentifiedDelivery: false,
-        onSendAnyway: action('onSendAnyway'),
-        onShowSafetyNumber: action('onShowSafetyNumber'),
         status: 'partial-sent',
       },
       {
@@ -141,8 +136,6 @@ story.add('Message Statuses', () => {
         }),
         isOutgoingKeyError: false,
         isUnidentifiedDelivery: false,
-        onSendAnyway: action('onSendAnyway'),
-        onShowSafetyNumber: action('onShowSafetyNumber'),
         status: 'delivered',
       },
       {
@@ -152,8 +145,6 @@ story.add('Message Statuses', () => {
         }),
         isOutgoingKeyError: false,
         isUnidentifiedDelivery: false,
-        onSendAnyway: action('onSendAnyway'),
-        onShowSafetyNumber: action('onShowSafetyNumber'),
         status: 'read',
       },
     ],
@@ -211,8 +202,6 @@ story.add('All Errors', () => {
         }),
         isOutgoingKeyError: true,
         isUnidentifiedDelivery: false,
-        onSendAnyway: action('onSendAnyway'),
-        onShowSafetyNumber: action('onShowSafetyNumber'),
         status: 'error',
       },
       {
@@ -228,8 +217,6 @@ story.add('All Errors', () => {
         ],
         isOutgoingKeyError: false,
         isUnidentifiedDelivery: true,
-        onSendAnyway: action('onSendAnyway'),
-        onShowSafetyNumber: action('onShowSafetyNumber'),
         status: 'error',
       },
       {
@@ -239,8 +226,6 @@ story.add('All Errors', () => {
         }),
         isOutgoingKeyError: true,
         isUnidentifiedDelivery: true,
-        onSendAnyway: action('onSendAnyway'),
-        onShowSafetyNumber: action('onShowSafetyNumber'),
         status: 'error',
       },
     ],

ts/components/conversation/MessageDetail.tsx

@@ -24,6 +24,7 @@ export type Contact = Pick<
   | 'acceptedMessageRequest'
   | 'avatarPath'
   | 'color'
+  | 'id'
   | 'isMe'
   | 'name'
   | 'phoneNumber'
@@ -39,9 +40,6 @@ export type Contact = Pick<
   unblurredAvatarPath?: string;
 
   errors?: Array<Error>;
-
-  onSendAnyway: () => void;
-  onShowSafetyNumber: () => void;
 };
 
 export type Props = {
@@ -52,6 +50,8 @@ export type Props = {
   receivedAt: number;
   sentAt: number;
 
+  sendAnyway: (contactId: string, messageId: string) => unknown;
+  showSafetyNumber: (contactId: string) => void;
   i18n: LocalizerType;
 } & Pick<
   MessagePropsType,
@@ -148,7 +148,7 @@ export class MessageDetail extends React.Component<Props> {
   }
 
   public renderContact(contact: Contact): JSX.Element {
-    const { i18n } = this.props;
+    const { i18n, message, showSafetyNumber, sendAnyway } = this.props;
     const errors = contact.errors || [];
 
     const errorComponent = contact.isOutgoingKeyError ? (
@@ -156,14 +156,14 @@ export class MessageDetail extends React.Component<Props> {
         <button
           type="button"
           className="module-message-detail__contact__show-safety-number"
-          onClick={contact.onShowSafetyNumber}
+          onClick={() => showSafetyNumber(contact.id)}
         >
           {i18n('showSafetyNumber')}
         </button>
         <button
           type="button"
           className="module-message-detail__contact__send-anyway"
-          onClick={contact.onSendAnyway}
+          onClick={() => sendAnyway(contact.id, message.id)}
         >
           {i18n('sendAnyway')}
         </button>

ts/models/conversations.ts

@@ -136,8 +136,6 @@ export class ConversationModel extends window.Backbone
 
   jobQueue?: typeof window.PQueueType;
 
-  messageCollection?: MessageModelCollectionType;
-
   ourNumber?: string;
 
   ourUuid?: string;
@@ -168,6 +166,8 @@ export class ConversationModel extends window.Backbone
 
   private isFetchingUUID?: boolean;
 
+  private hasAddedHistoryDisclaimer?: boolean;
+
   // eslint-disable-next-line class-methods-use-this
   defaults(): Partial<ConversationAttributesType> {
     return {
@@ -198,10 +198,6 @@ export class ConversationModel extends window.Backbone
     return this.get('uuid') || this.get('e164');
   }
 
-  handleMessageError(message: unknown, errors: unknown): void {
-    this.trigger('messageError', message, errors);
-  }
-
   // eslint-disable-next-line class-methods-use-this
   getContactCollection(): Backbone.Collection<ConversationModel> {
     const collection = new window.Backbone.Collection<ConversationModel>();
@@ -252,30 +248,9 @@ export class ConversationModel extends window.Backbone
       );
     }
 
-    this.messageCollection = new window.Whisper.MessageCollection([], {
-      conversation: this,
-    });
-
-    this.messageCollection.on('change:errors', this.handleMessageError, this);
-    this.messageCollection.on('send-error', this.onMessageError, this);
-
-    this.listenTo(
-      this.messageCollection,
-      'add remove destroy content-changed',
-      this.debouncedUpdateLastMessage
-    );
-    this.listenTo(this.messageCollection, 'sent', this.updateLastMessage);
-    this.listenTo(this.messageCollection, 'send-error', this.updateLastMessage);
-
     this.on('newmessage', this.onNewMessage);
     this.on('change:profileKey', this.onChangeProfileKey);
 
-    // Listening for out-of-band data updates
-    this.on('delivered', this.updateAndMerge);
-    this.on('read', this.updateAndMerge);
-    this.on('expiration-change', this.updateAndMerge);
-    this.on('expired', this.onExpired);
-
     const sealedSender = this.get('sealedSender');
     if (sealedSender === undefined) {
       this.set({ sealedSender: SEALED_SENDER.UNKNOWN });
@@ -1238,64 +1213,10 @@ export class ConversationModel extends window.Backbone
     );
   }
 
-  async updateAndMerge(message: MessageModel): Promise<void> {
+  async onNewMessage(message: MessageModel): Promise<void> {
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    const uuid = message.get('sourceUuid');
-    this.debouncedUpdateLastMessage!();
+    const e164 = message.get('source');
-
+    const sourceDevice = message.get('sourceDevice');
-    const mergeMessage = () => {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      const existing = this.messageCollection!.get(message.id);
-      if (!existing) {
-        return;
-      }
-
-      existing.merge(message.attributes);
-    };
-
-    await this.inProgressFetch;
-    mergeMessage();
-  }
-
-  async onExpired(message: MessageModel): Promise<void> {
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    this.debouncedUpdateLastMessage!();
-
-    const removeMessage = () => {
-      const { id } = message;
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      const existing = this.messageCollection!.get(id);
-      if (!existing) {
-        return;
-      }
-
-      window.log.info('Remove expired message from collection', {
-        sentAt: existing.get('sent_at'),
-      });
-
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      this.messageCollection!.remove(id);
-      existing.trigger('expired');
-      existing.cleanup();
-
-      // An expired message only counts as decrementing the message count, not
-      // the sent message count
-      this.decrementMessageCount();
-    };
-
-    // If a fetch is in progress, then we need to wait until that's complete to
-    //   do this removal. Otherwise we could remove from messageCollection, then
-    //   the async database fetch could include the removed message.
-
-    await this.inProgressFetch;
-    removeMessage();
-  }
-
-  async onNewMessage(message: WhatIsThis): Promise<void> {
-    const uuid = message.get ? message.get('sourceUuid') : message.sourceUuid;
-    const e164 = message.get ? message.get('source') : message.source;
-    const sourceDevice = message.get
-      ? message.get('sourceDevice')
-      : message.sourceDevice;
 
     const sourceId = window.ConversationController.ensureContactIds({
       uuid,
@@ -1306,33 +1227,26 @@ export class ConversationModel extends window.Backbone
     // Clear typing indicator for a given contact if we receive a message from them
     this.clearContactTypingTimer(typingToken);
 
+    this.addSingleMessage(message);
+
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     this.debouncedUpdateLastMessage!();
   }
 
-  // For outgoing messages, we can call this directly. We're already loaded.
   addSingleMessage(message: MessageModel): MessageModel {
-    const { id } = message;
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    const existing = this.messageCollection!.get(id);
-
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    const model = this.messageCollection!.add(message, { merge: true });
     // TODO use MessageUpdater.setToExpire
-    model.setToExpire();
+    message.setToExpire();
 
-    if (!existing) {
+    const { messagesAdded } = window.reduxActions.conversations;
-      const { messagesAdded } = window.reduxActions.conversations;
+    const isNewMessage = true;
-      const isNewMessage = true;
+    messagesAdded(
-      messagesAdded(
+      this.id,
-        this.id,
+      [message.getReduxData()],
-        [model.getReduxData()],
+      isNewMessage,
-        isNewMessage,
+      window.isActive()
-        window.isActive()
+    );
-      );
-    }
 
-    return model;
+    return message;
   }
 
   // For incoming messages, they might arrive while we're in the middle of a bulk fetch
@@ -2119,10 +2033,6 @@ export class ConversationModel extends window.Backbone
     return true;
   }
 
-  onMessageError(): void {
-    this.updateVerified();
-  }
-
   async safeGetVerified(): Promise<number> {
     const promise = window.textsecure.storage.protocol.getVerified(this.id);
     return promise.catch(
@@ -3629,12 +3539,14 @@ export class ConversationModel extends window.Backbone
       if (isDirectConversation(this.attributes)) {
         messageWithSchema.destination = destination;
       }
-      const attributes: MessageModel = {
+      const attributes: MessageAttributesType = {
         ...messageWithSchema,
         id: window.getGuid(),
       };
 
-      const model = this.addSingleMessage(attributes);
+      const model = this.addSingleMessage(
+        new window.Whisper.Message(attributes)
+      );
       if (sticker) {
         await addStickerPackReference(model.id, sticker.packId);
       }
@@ -4205,16 +4117,17 @@ export class ConversationModel extends window.Backbone
     return message;
   }
 
-  async addMessageHistoryDisclaimer(): Promise<MessageModel> {
+  async addMessageHistoryDisclaimer(): Promise<void> {
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    const lastMessage = this.messageCollection!.last();
-    if (lastMessage && lastMessage.get('type') === 'message-history-unsynced') {
-      // We do not need another message history disclaimer
-      return lastMessage;
-    }
-
     const timestamp = Date.now();
 
+    if (this.hasAddedHistoryDisclaimer) {
+      window.log.warn(
+        `addMessageHistoryDisclaimer/${this.idForLogging()}: Refusing to add another this session`
+      );
+      return;
+    }
+    this.hasAddedHistoryDisclaimer = true;
+
     const model = new window.Whisper.Message(({
       type: 'message-history-unsynced',
       // Even though this isn't reflected to the user, we want to place the last seen
@@ -4238,8 +4151,6 @@ export class ConversationModel extends window.Backbone
 
     const message = window.MessageController.register(id, model);
     this.addSingleMessage(message);
-
-    return message;
   }
 
   isSearchable(): boolean {
@@ -4816,9 +4727,6 @@ export class ConversationModel extends window.Backbone
   }
 
   async destroyMessages(): Promise<void> {
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    this.messageCollection!.reset([]);
-
     this.set({
       lastMessage: null,
       timestamp: null,

ts/models/messages.ts

@@ -284,7 +284,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
     this.on('change:expirationStartTimestamp', this.setToExpire);
     this.on('change:expireTimer', this.setToExpire);
     this.on('unload', this.unload);
-    this.on('expired', this.onExpired);
     this.setToExpire();
 
     this.on('change', this.notifyRedux);
@@ -517,9 +516,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
           errors: errorsForContact,
           isOutgoingKeyError,
           isUnidentifiedDelivery,
-          onSendAnyway: () =>
-            this.trigger('force-send', { contactId: id, messageId: this.id }),
-          onShowSafetyNumber: () => this.trigger('show-identity', id),
         };
       }
     );
@@ -1797,6 +1793,9 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
   async cleanup(): Promise<void> {
     const { messageDeleted } = window.reduxActions.conversations;
     messageDeleted(this.id, this.get('conversationId'));
+
+    this.getConversation()?.debouncedUpdateLastMessage?.();
+
     window.MessageController.unregister(this.id);
     this.unload();
     await this.deleteData();
@@ -1953,7 +1952,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
       preview: [],
       ...additionalProperties,
     });
-    this.trigger('content-changed');
+    this.getConversation()?.debouncedUpdateLastMessage?.();
 
     if (shouldPersist) {
       await window.Signal.Data.saveMessage(this.attributes, {
@@ -2625,10 +2624,17 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
   async send(
     promise: Promise<CallbackResultType | void | null>
   ): Promise<void | Array<void>> {
-    this.trigger('pending');
+    const conversation = this.getConversation();
+    const updateLeftPane = conversation?.debouncedUpdateLastMessage;
+    if (updateLeftPane) {
+      updateLeftPane();
+    }
+
     return (promise as Promise<CallbackResultType>)
       .then(async result => {
-        this.trigger('done');
+        if (updateLeftPane) {
+          updateLeftPane();
+        }
 
         // This is used by sendSyncMessage, then set to null
         if (result.dataMessage) {
@@ -2652,11 +2658,15 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
           });
         }
 
-        this.trigger('sent', this);
+        if (updateLeftPane) {
+          updateLeftPane();
+        }
         this.sendSyncMessage();
       })
       .catch((result: CustomError | CallbackResultType) => {
-        this.trigger('done');
+        if (updateLeftPane) {
+          updateLeftPane();
+        }
 
         if ('dataMessage' in result && result.dataMessage) {
           this.set({ dataMessage: result.dataMessage });
@@ -2756,7 +2766,9 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
           );
         }
 
-        this.trigger('send-error', this.get('errors'));
+        if (updateLeftPane) {
+          updateLeftPane();
+        }
 
         return Promise.all(promises);
       });
@@ -2820,6 +2832,8 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
     const conv = this.getConversation()!;
     this.set({ dataMessage });
 
+    const updateLeftPane = conv?.debouncedUpdateLastMessage;
+
     try {
       this.set({
         // These are the same as a normal send()
@@ -2846,13 +2860,9 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
       await window.Signal.Data.saveMessage(this.attributes, {
         Message: window.Whisper.Message,
       });
-      this.trigger('done');
 
-      const errors = this.get('errors');
+      if (updateLeftPane) {
-      if (errors) {
+        updateLeftPane();
-        this.trigger('send-error', errors);
-      } else {
-        this.trigger('sent');
       }
     }
   }

ts/state/smart/MessageDetail.tsx

@@ -27,6 +27,9 @@ export type OwnProps = {
   message: MessagePropsDataType;
   receivedAt: number;
   sentAt: number;
+
+  sendAnyway: (contactId: string, messageId: string) => unknown;
+  showSafetyNumber: (contactId: string) => void;
 } & Pick<
   MessageDetailProps,
   | 'clearSelectedMessage'
@@ -60,6 +63,9 @@ const mapStateToProps = (
     receivedAt,
     sentAt,
 
+    sendAnyway,
+    showSafetyNumber,
+
     clearSelectedMessage,
     deleteMessage,
     deleteMessageForEveryone,
@@ -99,6 +105,9 @@ const mapStateToProps = (
     i18n: getIntl(state),
     interactionMode: getInteractionMode(state),
 
+    sendAnyway,
+    showSafetyNumber,
+
     clearSelectedMessage,
     deleteMessage,
     deleteMessageForEveryone,

ts/test-electron/models/messages_test.ts

@@ -75,42 +75,6 @@ describe('Message', () => {
       assert.isTrue(message.get('sent'));
     });
 
-    it("triggers the 'done' event on success", async () => {
-      const message = createMessage({ type: 'outgoing', source });
-
-      let callCount = 0;
-      message.on('done', () => {
-        callCount += 1;
-      });
-
-      await message.send(Promise.resolve({}));
-
-      assert.strictEqual(callCount, 1);
-    });
-
-    it("triggers the 'sent' event on success", async () => {
-      const message = createMessage({ type: 'outgoing', source });
-
-      const listener = sinon.spy();
-      message.on('sent', listener);
-
-      await message.send(Promise.resolve({}));
-
-      sinon.assert.calledOnce(listener);
-      sinon.assert.calledWith(listener, message);
-    });
-
-    it("triggers the 'done' event on failure", async () => {
-      const message = createMessage({ type: 'outgoing', source });
-
-      const listener = sinon.spy();
-      message.on('done', listener);
-
-      await message.send(Promise.reject(new Error('something went wrong!')));
-
-      sinon.assert.calledOnce(listener);
-    });
-
     it('saves errors from promise rejections with errors', async () => {
       const message = createMessage({ type: 'outgoing', source });
 

ts/util/MessageController.ts

@@ -0,0 +1,113 @@
+// Copyright 2019-2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { MessageModel } from '../models/messages';
+
+const SECOND = 1000;
+const MINUTE = SECOND * 60;
+const FIVE_MINUTES = MINUTE * 5;
+const HOUR = MINUTE * 60;
+
+type LookupItemType = {
+  timestamp: number;
+  message: MessageModel;
+};
+type LookupType = Record<string, LookupItemType>;
+
+export class MessageController {
+  private messageLookup: LookupType = Object.create(null);
+
+  private msgIDsBySender = new Map<string, string>();
+
+  private msgIDsBySentAt = new Map<number, string>();
+
+  static install(): MessageController {
+    const instance = new MessageController();
+    window.MessageController = instance;
+
+    instance.startCleanupInterval();
+    return instance;
+  }
+
+  register(id: string, message: MessageModel): MessageModel {
+    if (!id || !message) {
+      return message;
+    }
+
+    const existing = this.messageLookup[id];
+    if (existing) {
+      this.messageLookup[id] = {
+        message: existing.message,
+        timestamp: Date.now(),
+      };
+      return existing.message;
+    }
+
+    this.messageLookup[id] = {
+      message,
+      timestamp: Date.now(),
+    };
+
+    this.msgIDsBySentAt.set(message.get('sent_at'), id);
+    this.msgIDsBySender.set(message.getSenderIdentifier(), id);
+
+    return message;
+  }
+
+  unregister(id: string): void {
+    const { message } = this.messageLookup[id] || {};
+    if (message) {
+      this.msgIDsBySender.delete(message.getSenderIdentifier());
+      this.msgIDsBySentAt.delete(message.get('sent_at'));
+    }
+    delete this.messageLookup[id];
+  }
+
+  cleanup(): void {
+    const messages = Object.values(this.messageLookup);
+    const now = Date.now();
+
+    for (let i = 0, max = messages.length; i < max; i += 1) {
+      const { message, timestamp } = messages[i];
+      const conversation = message.getConversation();
+
+      const state = window.reduxStore.getState();
+      const selectedId = state?.conversations?.selectedConversationId;
+      const inActiveConversation =
+        conversation && selectedId && conversation.id === selectedId;
+
+      if (now - timestamp > FIVE_MINUTES && !inActiveConversation) {
+        this.unregister(message.id);
+      }
+    }
+  }
+
+  getById(id: string): MessageModel | undefined {
+    const existing = this.messageLookup[id];
+    return existing && existing.message ? existing.message : undefined;
+  }
+
+  findBySentAt(sentAt: number): MessageModel | undefined {
+    const id = this.msgIDsBySentAt.get(sentAt);
+    if (!id) {
+      return undefined;
+    }
+    return this.getById(id);
+  }
+
+  findBySender(sender: string): MessageModel | undefined {
+    const id = this.msgIDsBySender.get(sender);
+    if (!id) {
+      return undefined;
+    }
+    return this.getById(id);
+  }
+
+  _get(): LookupType {
+    return this.messageLookup;
+  }
+
+  startCleanupInterval(): NodeJS.Timeout | number {
+    return setInterval(this.cleanup.bind(this), HOUR);
+  }
+}

ts/util/index.ts

@@ -38,6 +38,7 @@ import { postLinkExperience } from './postLinkExperience';
 import { sendToGroup, sendContentMessageToGroup } from './sendToGroup';
 import { RetryPlaceholders } from './retryPlaceholders';
 import * as expirationTimer from './expirationTimer';
+import { MessageController } from './MessageController';
 
 export {
   GoogleChrome,
@@ -60,6 +61,7 @@ export {
   longRunningTaskWrapper,
   makeLookup,
   mapToSupportLocale,
+  MessageController,
   missingCaseError,
   parseRemoteClientExpiration,
   postLinkExperience,

ts/views/conversation_view.ts

@@ -5,7 +5,10 @@
 
 import { AttachmentType } from '../types/Attachment';
 import { ConversationModel } from '../models/conversations';
-import { GroupV2PendingMemberType } from '../model-types.d';
+import {
+  GroupV2PendingMemberType,
+  MessageModelCollectionType,
+} from '../model-types.d';
 import { LinkPreviewType } from '../types/message/LinkPreviews';
 import { MediaItemType } from '../components/LightboxGallery';
 import { MessageModel } from '../models/messages';
@@ -356,40 +359,38 @@ Whisper.ConversationView = Whisper.View.extend({
     const { model }: { model: ConversationModel } = this;
 
     // Events on Conversation model
-    this.listenTo(model, 'destroy', this.stopListening);
+    this.listenTo(this.model, 'destroy', this.stopListening);
-    this.listenTo(model, 'change:verified', this.onVerifiedChange);
+    this.listenTo(this.model, 'change:verified', this.onVerifiedChange);
-    this.listenTo(model, 'newmessage', this.addMessage);
+    this.listenTo(this.model, 'newmessage', this.lazyUpdateVerified);
-    this.listenTo(model, 'opened', this.onOpened);
+
-    this.listenTo(model, 'backgrounded', this.resetEmojiResults);
+    // These are triggered by InboxView
-    this.listenTo(model, 'scroll-to-message', this.scrollToMessage);
+    this.listenTo(this.model, 'opened', this.onOpened);
-    this.listenTo(model, 'unload', (reason: string) =>
+    this.listenTo(this.model, 'scroll-to-message', this.scrollToMessage);
+    this.listenTo(this.model, 'unload', (reason: string) =>
       this.unload(`model trigger - ${reason}`)
     );
-    this.listenTo(model, 'focus-composer', this.focusMessageField);
+
-    this.listenTo(model, 'open-all-media', this.showAllMedia);
+    // These are triggered by background.ts for keyboard handling
-    this.listenTo(model, 'begin-recording', this.captureAudio);
+    this.listenTo(this.model, 'focus-composer', this.focusMessageField);
-    this.listenTo(model, 'attach-file', this.onChooseAttachment);
+    this.listenTo(this.model, 'open-all-media', this.showAllMedia);
-    this.listenTo(model, 'escape-pressed', this.resetPanel);
+    this.listenTo(this.model, 'begin-recording', this.captureAudio);
-    this.listenTo(model, 'show-message-details', this.showMessageDetail);
+    this.listenTo(this.model, 'attach-file', this.onChooseAttachment);
-    this.listenTo(model, 'show-contact-modal', this.showContactModal);
+    this.listenTo(this.model, 'escape-pressed', this.resetPanel);
-    this.listenTo(model, 'toggle-reply', (messageId: string | undefined) => {
+    this.listenTo(this.model, 'show-message-details', this.showMessageDetail);
-      const target = this.quote || !messageId ? null : messageId;
+    this.listenTo(this.model, 'show-contact-modal', this.showContactModal);
-      this.setQuoteMessage(target);
+    this.listenTo(
-    });
+      this.model,
+      'toggle-reply',
+      (messageId: string | undefined) => {
+        const target = this.quote || !messageId ? null : messageId;
+        this.setQuoteMessage(target);
+      }
+    );
     this.listenTo(model, 'save-attachment', this.downloadAttachmentWrapper);
     this.listenTo(model, 'delete-message', this.deleteMessage);
     this.listenTo(model, 'remove-link-review', this.removeLinkPreview);
     this.listenTo(model, 'remove-all-draft-attachments', this.clearAttachments);
 
-    // Events on Message models - we still listen to these here because they
-    //   can be emitted by the non-reduxified MessageDetail pane
-    this.listenTo(
-      model.messageCollection,
-      'show-identity',
-      this.showSafetyNumber
-    );
-    this.listenTo(model.messageCollection, 'force-send', this.forceSend);
-
     this.lazyUpdateVerified = window._.debounce(
       model.updateVerified.bind(model),
       1000 // one second
@@ -718,7 +719,6 @@ Whisper.ConversationView = Whisper.View.extend({
   },
 
   getMessageActions() {
-    const { model }: { model: ConversationModel } = this;
     const reactToMessage = (
       messageId: string,
       reaction: { emoji: string; remove: boolean }
@@ -750,17 +750,21 @@ Whisper.ConversationView = Whisper.View.extend({
       this.showContactDetail(options);
     };
     const kickOffAttachmentDownload = async (options: any) => {
-      if (!model.messageCollection) {
+      const message = window.MessageController.getById(options.messageId);
-        throw new Error('Message collection does not exist');
+      if (!message) {
+        throw new Error(
+          `kickOffAttachmentDownload: Message ${options.messageId} missing!`
+        );
       }
-      const message = model.messageCollection.get(options.messageId);
       await message.queueAttachmentDownloads();
     };
     const markAttachmentAsCorrupted = (options: AttachmentOptions) => {
-      const message: MessageModel = this.model.messageCollection.get(
+      const message = window.MessageController.getById(options.messageId);
-        options.messageId
+      if (!message) {
-      );
+        throw new Error(
-      assert(message, 'Message not found');
+          `markAttachmentAsCorrupted: Message ${options.messageId} missing!`
+        );
+      }
       message.markAttachmentAsCorrupted(options.attachment);
     };
     const showVisualAttachment = (options: {
@@ -784,6 +788,12 @@ Whisper.ConversationView = Whisper.View.extend({
     const downloadNewVersion = () => {
       this.downloadNewVersion();
     };
+    const sendAnyway = (contactId: string, messageId: string) => {
+      this.forceSend(contactId, messageId);
+    };
+    const showSafetyNumber = (contactId: string) => {
+      this.showSafetyNumber(contactId);
+    };
     const showExpiredIncomingTapToViewToast = () => {
       this.showToast(Whisper.TapToViewExpiredIncomingToast);
     };
@@ -805,8 +815,10 @@ Whisper.ConversationView = Whisper.View.extend({
       reactToMessage,
       replyToMessage,
       retrySend,
+      sendAnyway,
       showContactDetail,
       showContactModal,
+      showSafetyNumber,
       showExpiredIncomingTapToViewToast,
       showExpiredOutgoingTapToViewToast,
       showForwardMessageModal,
@@ -895,12 +907,12 @@ Whisper.ConversationView = Whisper.View.extend({
         }
 
         const cleaned = await this.cleanModels(models);
-        this.model.messageCollection.add(cleaned);
-
         const isNewMessage = false;
         messagesAdded(
           id,
-          models.map(messageModel => messageModel.getReduxData()),
+          cleaned.map((messageModel: MessageModel) =>
+            messageModel.getReduxData()
+          ),
           isNewMessage,
           window.isActive()
         );
@@ -950,12 +962,12 @@ Whisper.ConversationView = Whisper.View.extend({
         }
 
         const cleaned = await this.cleanModels(models);
-        this.model.messageCollection.add(cleaned);
-
         const isNewMessage = false;
         messagesAdded(
           id,
-          models.map(messageModel => messageModel.getReduxData()),
+          cleaned.map((messageModel: MessageModel) =>
+            messageModel.getReduxData()
+          ),
           isNewMessage,
           window.isActive()
         );
@@ -1078,7 +1090,9 @@ Whisper.ConversationView = Whisper.View.extend({
     toast.render();
   },
 
-  async cleanModels(collection: any) {
+  async cleanModels(
+    collection: MessageModelCollectionType | Array<MessageModel>
+  ): Promise<Array<MessageModel>> {
     const result = collection
       .filter((message: any) => Boolean(message.id))
       .map((message: any) =>
@@ -1121,7 +1135,9 @@ Whisper.ConversationView = Whisper.View.extend({
       throw new Error(`scrollToMessage: failed to load message ${messageId}`);
     }
 
-    if (this.model.messageCollection.get(messageId)) {
+    const isInMemory = Boolean(window.MessageController.getById(messageId));
+
+    if (isInMemory) {
       const { scrollToMessage } = window.reduxActions.conversations;
       scrollToMessage(model.id, messageId);
       return;
@@ -1189,13 +1205,14 @@ Whisper.ConversationView = Whisper.View.extend({
       const all = [...older.models, message, ...newer.models];
 
       const cleaned: Array<MessageModel> = await this.cleanModels(all);
-      this.model.messageCollection.reset(cleaned);
       const scrollToMessageId =
         options && options.disableScroll ? undefined : messageId;
 
       messagesReset(
         conversationId,
-        cleaned.map(messageModel => messageModel.getReduxData()),
+        cleaned.map((messageModel: MessageModel) =>
+          messageModel.getReduxData()
+        ),
         metrics,
         scrollToMessageId
       );
@@ -1266,12 +1283,6 @@ Whisper.ConversationView = Whisper.View.extend({
       });
 
       const cleaned: Array<MessageModel> = await this.cleanModels(messages);
-      assert(
-        model.messageCollection,
-        'loadNewestMessages: model must have messageCollection'
-      );
-
-      model.messageCollection.reset(cleaned);
       const scrollToMessageId =
         setFocus && metrics.newest ? metrics.newest.id : undefined;
 
@@ -1283,7 +1294,9 @@ Whisper.ConversationView = Whisper.View.extend({
       const unboundedFetch = true;
       messagesReset(
         conversationId,
-        cleaned.map(messageModel => messageModel.getReduxData()),
+        cleaned.map((messageModel: MessageModel) =>
+          messageModel.getReduxData()
+        ),
         metrics,
         scrollToMessageId,
         unboundedFetch
@@ -1453,8 +1466,6 @@ Whisper.ConversationView = Whisper.View.extend({
     }
 
     this.remove();
-
-    this.model.messageCollection.reset([]);
   },
 
   navigateTo(url: any) {
@@ -2303,19 +2314,17 @@ Whisper.ConversationView = Whisper.View.extend({
   },
 
   async retrySend(messageId: string) {
-    const message = this.model.messageCollection.get(messageId);
+    const message = window.MessageController.getById(messageId);
     if (!message) {
-      throw new Error(`retrySend: Did not find message for id ${messageId}`);
+      throw new Error(`retrySend: Message ${messageId} missing!`);
     }
     await message.retrySend();
   },
 
   showForwardMessageModal(messageId: string) {
-    const message = this.model.messageCollection.get(messageId);
+    const message = window.MessageController.getById(messageId);
     if (!message) {
-      throw new Error(
+      throw new Error(`showForwardMessageModal: Message ${messageId} missing!`);
-        `showForwardMessageModal: Did not find message for id ${messageId}`
-      );
     }
 
     const attachments = message.getAttachmentsForMessage();
@@ -2658,7 +2667,7 @@ Whisper.ConversationView = Whisper.View.extend({
       Component: window.Signal.Components.MediaGallery,
       props: await getProps(),
       onClose: () => {
-        this.stopListening(model.messageCollection, 'remove', update);
+        unsubscribe();
       },
     });
     view.headerTitle = window.i18n('allMedia');
@@ -2667,7 +2676,28 @@ Whisper.ConversationView = Whisper.View.extend({
       view.update(await getProps());
     };
 
-    this.listenTo(model.messageCollection, 'remove', update);
+    function getMessageIds(): Array<string | undefined> | undefined {
+      const state = window.reduxStore.getState();
+      const byConversation = state?.conversations?.messagesByConversation;
+      const messages = byConversation && byConversation[conversationId];
+      if (!messages || !messages.messageIds) {
+        return undefined;
+      }
+
+      return messages.messageIds;
+    }
+
+    // Detect message changes in the current conversation
+    let previousMessageList: Array<string | undefined> | undefined;
+    previousMessageList = getMessageIds();
+
+    const unsubscribe = window.reduxStore.subscribe(() => {
+      const currentMessageList = getMessageIds();
+      if (currentMessageList !== previousMessageList) {
+        update();
+        previousMessageList = currentMessageList;
+      }
+    });
 
     this.listenBack(view);
   },
@@ -2696,25 +2726,12 @@ Whisper.ConversationView = Whisper.View.extend({
     this.compositionApi.current.resetEmojiResults(false);
   },
 
-  async addMessage(message: MessageModel) {
-    const { model }: { model: ConversationModel } = this;
-
-    // This is debounced, so it won't hit the database too often.
-    this.lazyUpdateVerified();
-
-    // We do this here because we don't want convo.messageCollection to have
-    //   anything in it unless it has an associated view. This is so, when we
-    //   fetch on open, it's clean.
-    model.addIncomingMessage(message);
-  },
-
   async showMembers(
     _e: unknown,
     providedMembers: void | Backbone.Collection<ConversationModel>,
     options: any = {}
   ) {
     const { model }: { model: ConversationModel } = this;
-
     window._.defaults(options, { needVerify: false });
 
     let contactCollection = providedMembers || model.contactCollection;
@@ -2746,9 +2763,9 @@ Whisper.ConversationView = Whisper.View.extend({
   }: Readonly<{ contactId: string; messageId: string }>) {
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     const contact = window.ConversationController.get(contactId)!;
-    const message = this.model.messageCollection.get(messageId);
+    const message = window.MessageController.getById(messageId);
     if (!message) {
-      throw new Error(`forceSend: Did not find message for id ${messageId}`);
+      throw new Error(`forceSend: Message ${messageId} missing!`);
     }
 
     window.showConfirmationDialog({
@@ -2770,7 +2787,14 @@ Whisper.ConversationView = Whisper.View.extend({
           contact.setApproved();
         }
 
-        message.resend(contact.getSendTarget());
+        const sendTarget = contact.getSendTarget();
+        if (!sendTarget) {
+          throw new Error(
+            `forceSend: Contact ${contact.idForLogging()} had no sendTarget!`
+          );
+        }
+
+        message.resend(sendTarget);
       },
     });
   },
@@ -2795,10 +2819,10 @@ Whisper.ConversationView = Whisper.View.extend({
   },
 
   downloadAttachmentWrapper(messageId: string) {
-    const message = this.model.messageCollection.get(messageId);
+    const message = window.MessageController.getById(messageId);
     if (!message) {
       throw new Error(
-        `downloadAttachmentWrapper: Did not find message for id ${messageId}`
+        `downloadAttachmentWrapper: Message ${messageId} missing!`
       );
     }
 
@@ -2842,11 +2866,9 @@ Whisper.ConversationView = Whisper.View.extend({
   },
 
   async displayTapToViewMessage(messageId: string) {
-    const message = this.model.messageCollection.get(messageId);
+    const message = window.MessageController.getById(messageId);
     if (!message) {
-      throw new Error(
+      throw new Error(`displayTapToViewMessage: Message ${messageId} missing!`);
-        `displayTapToViewMessage: Did not find message for id ${messageId}`
-      );
     }
 
     if (!message.isTapToView()) {
@@ -2919,11 +2941,9 @@ Whisper.ConversationView = Whisper.View.extend({
   },
 
   deleteMessage(messageId: string) {
-    const message = this.model.messageCollection.get(messageId);
+    const message = window.MessageController.getById(messageId);
     if (!message) {
-      throw new Error(
+      throw new Error(`deleteMessage: Message ${messageId} missing!`);
-        `deleteMessage: Did not find message for id ${messageId}`
-      );
     }
 
     window.showConfirmationDialog({
@@ -2934,8 +2954,7 @@ Whisper.ConversationView = Whisper.View.extend({
         window.Signal.Data.removeMessage(message.id, {
           Message: Whisper.Message,
         });
-        message.trigger('unload');
+        message.cleanup();
-        this.model.messageCollection.remove(message.id);
         if (message.isOutgoing()) {
           this.model.decrementSentMessageCount();
         } else {
@@ -2947,10 +2966,10 @@ Whisper.ConversationView = Whisper.View.extend({
   },
 
   deleteMessageForEveryone(messageId: string) {
-    const message = this.model.messageCollection.get(messageId);
+    const message = window.MessageController.getById(messageId);
     if (!message) {
       throw new Error(
-        `deleteMessageForEveryone: Did not find message for id ${messageId}`
+        `deleteMessageForEveryone: Message ${messageId} missing!`
       );
     }
 
@@ -3038,9 +3057,9 @@ Whisper.ConversationView = Whisper.View.extend({
     messageId: string;
     showSingle?: boolean;
   }) {
-    const message = this.model.messageCollection.get(messageId);
+    const message = window.MessageController.getById(messageId);
     if (!message) {
-      throw new Error(`showLightbox: did not find message for id ${messageId}`);
+      throw new Error(`showLightbox: Message ${messageId} missing!`);
     }
     const sticker = message.get('sticker');
     if (sticker) {
@@ -3365,12 +3384,9 @@ Whisper.ConversationView = Whisper.View.extend({
   },
 
   showMessageDetail(messageId: string) {
-    const { model }: { model: ConversationModel } = this;
+    const message = window.MessageController.getById(messageId);
-    const message = model.messageCollection?.get(messageId);
     if (!message) {
-      throw new Error(
+      throw new Error(`showMessageDetail: Message ${messageId} missing!`);
-        `showMessageDetail: Did not find message for id ${messageId}`
-      );
     }
 
     if (!message.isNormalBubble()) {

ts/window.d.ts

@@ -3,6 +3,7 @@
 
 // Captures the globals put in place by preload.js, background.js and others
 
+import { DeepPartial, Store } from 'redux';
 import * as Backbone from 'backbone';
 import * as Underscore from 'underscore';
 import moment from 'moment';
@@ -113,6 +114,8 @@ import * as synchronousCrypto from './util/synchronousCrypto';
 import { SocketStatus } from './types/SocketStatus';
 import SyncRequest from './textsecure/SyncRequest';
 import { ConversationColorType, CustomColorType } from './types/Colors';
+import { MessageController } from './util/MessageController';
+import { StateType } from './state/reducer';
 
 export { Long } from 'long';
 
@@ -235,7 +238,7 @@ declare global {
     platform: string;
     preloadedImages: Array<WhatIsThis>;
     reduxActions: ReduxActions;
-    reduxStore: WhatIsThis;
+    reduxStore: Store<StateType>;
     registerForActive: (handler: () => void) => void;
     restart: () => void;
     setImmediate: typeof setImmediate;
@@ -537,7 +540,7 @@ declare global {
 
     ConversationController: ConversationController;
     Events: WhatIsThis;
-    MessageController: MessageControllerType;
+    MessageController: MessageController;
     SignalProtocolStore: typeof SignalProtocolStore;
     WebAPI: WebAPIConnectType;
     Whisper: WhisperType;
@@ -604,14 +607,6 @@ export type DCodeIOType = {
   ProtoBuf: WhatIsThis;
 };
 
-type MessageControllerType = {
-  findBySender: (sender: string) => MessageModel | null;
-  findBySentAt: (sentAt: number) => MessageModel | null;
-  getById: (id: string) => MessageModel | undefined;
-  register: (id: string, model: MessageModel) => MessageModel;
-  unregister: (id: string) => void;
-};
-
 export class CertificateValidatorType {
   validate: (cerficate: any, certificateTime: number) => Promise<void>;
 }