New MessageController as the single place for in-memory messages

background.html

@@ -492,6 +492,7 @@
   <script type='text/javascript' src='js/registration.js'></script>
   <script type='text/javascript' src='js/expire.js'></script>
   <script type='text/javascript' src='js/conversation_controller.js'></script>
+  <script type='text/javascript' src='js/message_controller.js'></script>
 
   <script type='text/javascript' src='js/views/react_wrapper_view.js'></script>
   <script type='text/javascript' src='js/views/whisper_view.js'></script>

js/delivery_receipts.js

@@ -1,7 +1,10 @@
-/* global Backbone: false */
+/* global
-/* global Whisper: false */
+  Backbone,
-/* global ConversationController: false */
+  Whisper,
-/* global _: false */
+  ConversationController,
+  MessageController,
+  _
+*/
 
 /* eslint-disable more/no-then */
 
@@ -45,10 +48,15 @@
       const ids = groups.pluck('id');
       ids.push(source);
 
-      return messages.find(
+      const target = messages.find(
         item =>
           !item.isIncoming() && _.contains(ids, item.get('conversationId'))
       );
+      if (!target) {
+        return null;
+      }
+
+      return MessageController.register(target.id, target);
     },
     async onReceipt(receipt) {
       try {

js/expiring_messages.js

@@ -1,8 +1,11 @@
-/* global _: false */
+/* global
-/* global Backbone: false */
+  _,
-/* global i18n: false */
+  Backbone,
-/* global moment: false */
+  i18n,
-/* global Whisper: false */
+  MessageController,
+  moment,
+  Whisper
+*/
 
 // eslint-disable-next-line func-names
 (function() {
@@ -18,7 +21,9 @@
       });
 
       await Promise.all(
-        messages.map(async message => {
+        messages.map(async fromDB => {
+          const message = MessageController.register(fromDB.id, fromDB);
+
           window.log.info('Message expired', {
             sentAt: message.get('sent_at'),
           });

js/message_controller.js

@@ -0,0 +1,65 @@
+// eslint-disable-next-line func-names
+(function() {
+  'use strict';
+
+  window.Whisper = window.Whisper || {};
+
+  const messageLookup = Object.create(null);
+
+  const SECOND = 1000;
+  const MINUTE = SECOND * 60;
+  const FIVE_MINUTES = MINUTE * 5;
+  const HOUR = MINUTE * 60;
+
+  function register(id, message) {
+    const existing = messageLookup[id];
+    if (existing) {
+      messageLookup[id] = {
+        message: existing.message,
+        timestamp: Date.now(),
+      };
+      return existing.message;
+    }
+
+    messageLookup[id] = {
+      message,
+      timestamp: Date.now(),
+    };
+
+    return message;
+  }
+
+  function unregister(id) {
+    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)
+      ) {
+        delete messageLookup[message.id];
+      }
+    }
+  }
+
+  function _get() {
+    return messageLookup;
+  }
+
+  setInterval(cleanup, HOUR);
+
+  window.MessageController = {
+    register,
+    unregister,
+    cleanup,
+    _get,
+  };
+})();

js/models/conversations.js

@@ -4,6 +4,7 @@
   Backbone,
   libphonenumber,
   ConversationController,
+  MessageController,
   libsignal,
   storage,
   textsecure,
@@ -673,14 +674,6 @@
     },
 
     async onReadMessage(message, readAt) {
-      const existing = this.messageCollection.get(message.id);
-      if (existing) {
-        const fetched = await window.Signal.Data.getMessageById(existing.id, {
-          Message: Whisper.Message,
-        });
-        existing.merge(fetched);
-      }
-
       // We mark as read everything older than this message - to clean up old stuff
       //   still marked unread in the database. If the user generally doesn't read in
       //   the desktop app, so the desktop app only gets read syncs, we can very
@@ -883,10 +876,23 @@
           recipients,
         });
 
-        const message = this.addSingleMessage(messageWithSchema);
+        if (this.isPrivate()) {
+          messageWithSchema.destination = destination;
+        }
+        const attributes = {
+          ...messageWithSchema,
+          id: window.getGuid(),
+        };
+
+        const model = this.addSingleMessage(attributes);
+        const message = MessageController.register(model.id, model);
+        await window.Signal.Data.saveMessage(message.attributes, {
+          forceSave: true,
+          Message: Whisper.Message,
+        });
 
         this.set({
-          lastMessage: message.getNotificationText(),
+          lastMessage: model.getNotificationText(),
           lastMessageStatus: 'sending',
           active_at: now,
           timestamp: now,
@@ -896,15 +902,6 @@
           Conversation: Whisper.Conversation,
         });
 
-        if (this.isPrivate()) {
-          message.set({ destination });
-        }
-
-        const id = await window.Signal.Data.saveMessage(message.attributes, {
-          Message: Whisper.Message,
-        });
-        message.set({ id });
-
         // We're offline!
         if (!textsecure.messaging) {
           const errors = this.contactCollection.map(contact => {
@@ -1424,11 +1421,9 @@
 
       let read = await Promise.all(
         _.map(oldUnread, async providedM => {
-          let m = providedM;
+          const m = MessageController.register(providedM.id, providedM);
 
-          if (this.messageCollection.get(m.id)) {
+          if (!this.messageCollection.get(m.id)) {
-            m = this.messageCollection.get(m.id);
-          } else {
             window.log.warn(
               'Marked a message as read in the database, but ' +
                 'it was not in messageCollection.'

js/models/messages.js

@@ -1,13 +1,16 @@
-/* global _: false */
+/* global
-/* global Backbone: false */
+  _,
-/* global storage: false */
+  Backbone,
-/* global filesize: false */
+  storage,
-/* global ConversationController: false */
+  filesize,
-/* global getAccountManager: false */
+  ConversationController,
-/* global i18n: false */
+  MessageController,
-/* global Signal: false */
+  getAccountManager,
-/* global textsecure: false */
+  i18n,
-/* global Whisper: false */
+  Signal,
+  textsecure,
+  Whisper
+*/
 
 /* eslint-disable more/no-then */
 
@@ -261,6 +264,7 @@
       this.cleanup();
     },
     async cleanup() {
+      MessageController.unregister(this.id);
       this.unload();
       await deleteExternalMessageFiles(this.attributes);
     },
@@ -1394,17 +1398,18 @@
       const collection = await window.Signal.Data.getMessagesBySentAt(id, {
         MessageCollection: Whisper.MessageCollection,
       });
-      const queryMessage = collection.find(item => {
+      const found = collection.find(item => {
         const messageAuthor = item.getContact();
 
         return messageAuthor && author === messageAuthor.id;
       });
 
-      if (!queryMessage) {
+      if (!found) {
         quote.referencedMessageNotFound = true;
         return message;
       }
 
+      const queryMessage = MessageController.register(found.id, found);
       quote.text = queryMessage.get('body');
       if (firstAttachment) {
         firstAttachment.thumbnail = null;
@@ -1730,6 +1735,7 @@
             Message: Whisper.Message,
           });
           message.set({ id });
+          MessageController.register(message.id, message);
 
           // Note that this can save the message again, if jobs were queued. We need to
           //   call it after we have an id for this message, because the jobs refer back
@@ -1933,7 +1939,9 @@
         }
       );
 
-      const models = messages.filter(message => Boolean(message.id));
+      const models = messages
+        .filter(message => Boolean(message.id))
+        .map(message => MessageController.register(message.id, message));
       const eliminated = messages.length - models.length;
       if (eliminated > 0) {
         window.log.warn(

js/modules/attachment_downloads.js

@@ -1,4 +1,4 @@
-/* global Whisper, Signal, setTimeout, clearTimeout */
+/* global Whisper, Signal, setTimeout, clearTimeout, MessageController */
 
 const { isFunction, isNumber, omit } = require('lodash');
 const getGuid = require('uuid/v4');
@@ -37,7 +37,6 @@ let timeout;
 let getMessageReceiver;
 let logger;
 const _activeAttachmentDownloadJobs = {};
-const _messageCache = {};
 
 async function start(options = {}) {
   ({ getMessageReceiver, logger } = options);
@@ -149,12 +148,15 @@ async function _runJob(job) {
       );
     }
 
-    message = await _getMessage(messageId);
+    const found = await getMessageById(messageId, {
-    if (!message) {
+      Message: Whisper.Message,
+    });
+    if (!found) {
       logger.error('_runJob: Source message not found, deleting job');
-      await _finishJob(message, id);
+      await _finishJob(null, id);
       return;
     }
+    message = MessageController.register(found.id, found);
 
     const pending = true;
     await setAttachmentDownloadJobPending(id, pending);
@@ -232,46 +234,6 @@ async function _runJob(job) {
   }
 }
 
-async function _getMessage(id) {
-  let item = _messageCache[id];
-  if (item) {
-    const fiveMinutesAgo = Date.now() - 5 * MINUTE;
-    if (item.timestamp >= fiveMinutesAgo) {
-      return item.message;
-    }
-
-    delete _messageCache[id];
-  }
-
-  let message = await getMessageById(id, {
-    Message: Whisper.Message,
-  });
-  if (!message) {
-    return message;
-  }
-
-  // Once more, checking for race conditions
-  item = _messageCache[id];
-  if (item) {
-    const fiveMinutesAgo = Date.now() - 5 * MINUTE;
-    if (item.timestamp >= fiveMinutesAgo) {
-      return item.message;
-    }
-  }
-
-  const conversation = message.getConversation();
-  if (conversation && conversation.messageCollection.get(id)) {
-    message = conversation.get(id);
-  }
-
-  _messageCache[id] = {
-    timestamp: Date.now(),
-    message,
-  };
-
-  return message;
-}
-
 async function _finishJob(message, id) {
   if (message) {
     await saveMessage(message.attributes, {

js/read_receipts.js

@@ -1,4 +1,11 @@
-/* global Whisper, Backbone, _, ConversationController, window */
+/* global
+  Whisper,
+  Backbone,
+  _,
+  ConversationController,
+  MessageController,
+  window
+*/
 
 /* eslint-disable more/no-then */
 
@@ -46,9 +53,14 @@
       const ids = groups.pluck('id');
       ids.push(reader);
 
-      return messages.find(
+      const target = messages.find(
         item => item.isOutgoing() && _.contains(ids, item.get('conversationId'))
       );
+      if (!target) {
+        return null;
+      }
+
+      return MessageController.register(target.id, target);
     },
     async onReceipt(receipt) {
       try {

js/read_syncs.js

@@ -1,4 +1,8 @@
-/* global Backbone, Whisper */
+/* global
+  Backbone,
+  Whisper,
+  MessageController
+*/
 
 /* eslint-disable more/no-then */
 
@@ -30,19 +34,19 @@
           }
         );
 
-        const message = messages.find(
+        const found = messages.find(
           item =>
             item.isIncoming() && item.get('source') === receipt.get('sender')
         );
-        const notificationForMessage = message
+        const notificationForMessage = found
-          ? Whisper.Notifications.findWhere({ messageId: message.id })
+          ? Whisper.Notifications.findWhere({ messageId: found.id })
           : null;
         const removedNotification = Whisper.Notifications.remove(
           notificationForMessage
         );
         const receiptSender = receipt.get('sender');
         const receiptTimestamp = receipt.get('timestamp');
-        const wasMessageFound = Boolean(message);
+        const wasMessageFound = Boolean(found);
         const wasNotificationFound = Boolean(notificationForMessage);
         const wasNotificationRemoved = Boolean(removedNotification);
         window.log.info('Receive read sync:', {
@@ -53,10 +57,11 @@
           wasNotificationRemoved,
         });
 
-        if (!message) {
+        if (!found) {
           return;
         }
 
+        const message = MessageController.register(found.id, found);
         const readAt = receipt.get('read_at');
 
         // If message is unread, we mark it read. Otherwise, we update the expiration

js/views/conversation_view.js

@@ -729,13 +729,15 @@
         const collection = await window.Signal.Data.getMessagesBySentAt(id, {
           MessageCollection: Whisper.MessageCollection,
         });
-        const messageFromDatabase = collection.find(item => {
+        const found = Boolean(
-          const messageAuthor = item.getContact();
+          collection.find(item => {
+            const messageAuthor = item.getContact();
 
-          return messageAuthor && author === messageAuthor.id;
+            return messageAuthor && author === messageAuthor.id;
-        });
+          })
+        );
 
-        if (messageFromDatabase) {
+        if (found) {
           const toast = new Whisper.FoundButNotLoadedToast();
           toast.$el.appendTo(this.$el);
           toast.render();

test/index.html

@@ -478,6 +478,7 @@
   <script type="text/javascript" src="../js/models/conversations.js" data-cover></script>
   <script type="text/javascript" src="../js/models/blockedNumbers.js" data-cover></script>
   <script type="text/javascript" src="../js/conversation_controller.js" data-cover></script>
+  <script type="text/javascript" src="../js/message_controller.js" data-cover></script>
   <script type="text/javascript" src="../js/keychange_listener.js" data-cover></script>
   <script type='text/javascript' src='../js/expiring_messages.js' data-cover></script>
   <script type='text/javascript' src='../js/notifications.js' data-cover></script>