On message delete, ensure that all external files are deleted

js/background.js

@@ -880,7 +880,7 @@
       messageDescriptor.id,
       messageDescriptor.type
     );
-    await conversation.save({ profileSharing: true });
+    await wrapDeferred(conversation.save({ profileSharing: true }));
     return confirm();
   }
 

js/models/conversations.js

@@ -1769,7 +1769,6 @@
           Whisper.Notifications.add({
             conversationId,
             iconUrl,
-            imageUrl: message.getImageUrl(),
             isExpiringMessage,
             message: message.getNotificationText(),
             messageId,

js/models/messages.js

@@ -20,8 +20,7 @@
 
   const { Message: TypedMessage, Contact, PhoneNumber } = Signal.Types;
   const {
-    // loadAttachmentData,
+    deleteExternalMessageFiles,
-    deleteAttachmentData,
     getAbsoluteAttachmentPath,
   } = Signal.Migrations;
 
@@ -69,7 +68,6 @@
 
       this.OUR_NUMBER = textsecure.storage.user.getNumber();
 
-      this.on('change:attachments', this.updateImageUrl);
       this.on('destroy', this.onDestroy);
       this.on('change:expirationStartTimestamp', this.setToExpire);
       this.on('change:expireTimer', this.setToExpire);
@@ -223,54 +221,15 @@
 
       return '';
     },
-    async onDestroy() {
+    onDestroy() {
-      this.revokeImageUrl();
+      this.unload();
-      const attachments = this.get('attachments');
+
-      await Promise.all(attachments.map(deleteAttachmentData));
+      return deleteExternalMessageFiles(this.attributes);
-    },
-    updateImageUrl() {
-      this.revokeImageUrl();
-      const attachment = this.get('attachments')[0];
-      if (attachment) {
-        const blob = new Blob([attachment.data], {
-          type: attachment.contentType,
-        });
-        this.imageUrl = URL.createObjectURL(blob);
-      } else {
-        this.imageUrl = null;
-      }
     },
     unload() {
-      if (this.quoteThumbnail) {
-        URL.revokeObjectURL(this.quoteThumbnail.objectUrl);
-        this.quoteThumbnail = null;
-      }
       if (this.quotedMessage) {
         this.quotedMessage = null;
       }
-      const quote = this.get('quote');
-      const attachments = (quote && quote.attachments) || [];
-      attachments.forEach(attachment => {
-        if (attachment.thumbnail && attachment.thumbnail.objectUrl) {
-          URL.revokeObjectURL(attachment.thumbnail.objectUrl);
-          // eslint-disable-next-line no-param-reassign
-          attachment.thumbnail.objectUrl = null;
-        }
-      });
-
-      this.revokeImageUrl();
-    },
-    revokeImageUrl() {
-      if (this.imageUrl) {
-        URL.revokeObjectURL(this.imageUrl);
-        this.imageUrl = null;
-      }
-    },
-    getImageUrl() {
-      if (this.imageUrl === undefined) {
-        this.updateImageUrl();
-      }
-      return this.imageUrl;
     },
     getQuoteObjectUrl() {
       const thumbnail = this.quoteThumbnail;

js/modules/signal.js

@@ -110,12 +110,14 @@ function initializeMigrations({
   const readAttachmentData = createReader(attachmentsPath);
   const loadAttachmentData = Type.loadData(readAttachmentData);
   const getAbsoluteAttachmentPath = createAbsolutePathGetter(attachmentsPath);
+  const deleteOnDisk = Attachments.createDeleter(attachmentsPath);
 
   return {
     attachmentsPath,
-    deleteAttachmentData: Type.deleteData(
+    deleteExternalMessageFiles: MessageType.deleteAllExternalFiles({
-      Attachments.createDeleter(attachmentsPath)
+      deleteAttachmentData: Type.deleteData(deleteOnDisk),
-    ),
+      deleteOnDisk,
+    }),
     getAbsoluteAttachmentPath,
     getPlaceholderMigrations,
     loadAttachmentData,

js/modules/types/attachment.js

@@ -158,26 +158,28 @@ exports.loadData = readAttachmentData => {
 //      deleteData :: (RelativePath -> IO Unit)
 //                    Attachment ->
 //                    IO Unit
-exports.deleteData = deleteAttachmentData => {
+exports.deleteData = deleteOnDisk => {
-  if (!is.function(deleteAttachmentData)) {
+  if (!is.function(deleteOnDisk)) {
-    throw new TypeError("'deleteAttachmentData' must be a function");
+    throw new TypeError('deleteData: deleteOnDisk must be a function');
   }
 
   return async attachment => {
     if (!exports.isValid(attachment)) {
-      throw new TypeError("'attachment' is not valid");
+      throw new TypeError('deleteData: attachment is not valid');
     }
 
-    const hasDataInMemory = exports.hasData(attachment);
+    const { path, thumbnail, screenshot } = attachment;
-    if (hasDataInMemory) {
+    if (is.string(path)) {
-      return;
+      await deleteOnDisk(path);
     }
 
-    if (!is.string(attachment.path)) {
+    if (thumbnail && is.string(thumbnail.path)) {
-      throw new TypeError("'attachment.path' is required");
+      await deleteOnDisk(thumbnail.path);
     }
 
-    await deleteAttachmentData(attachment.path);
+    if (screenshot && is.string(screenshot.path)) {
+      await deleteOnDisk(screenshot.path);
+    }
   };
 };
 

js/modules/types/message.js

@@ -361,6 +361,52 @@ exports.createAttachmentLoader = loadAttachmentData => {
     });
 };
 
+exports.deleteAllExternalFiles = ({ deleteAttachmentData, deleteOnDisk }) => {
+  if (!isFunction(deleteAttachmentData)) {
+    throw new TypeError(
+      'deleteAllExternalFiles: deleteAttachmentData must be a function'
+    );
+  }
+
+  if (!isFunction(deleteOnDisk)) {
+    throw new TypeError(
+      'deleteAllExternalFiles: deleteOnDisk must be a function'
+    );
+  }
+
+  return async message => {
+    const { attachments, quote, contact } = message;
+
+    if (attachments && attachments.length) {
+      await Promise.all(attachments.map(deleteAttachmentData));
+    }
+
+    if (quote && quote.attachments && quote.attachments.length) {
+      await Promise.all(
+        quote.attachments.map(async attachment => {
+          const { thumbnail } = attachment;
+
+          if (thumbnail.path) {
+            await deleteOnDisk(thumbnail.path);
+          }
+        })
+      );
+    }
+
+    if (contact && contact.length) {
+      await Promise.all(
+        contact.map(async item => {
+          const { avatar } = item;
+
+          if (avatar && avatar.avatar && avatar.avatar.path) {
+            await deleteOnDisk(avatar.avatar.path);
+          }
+        })
+      );
+    }
+  };
+};
+
 //      createAttachmentDataWriter :: (RelativePath -> IO Unit)
 //                                    Message ->
 //                                    IO (Promise Message)

test/models/messages_test.js

@@ -27,29 +27,15 @@
   var source = '+14155555555';
 
   describe('MessageCollection', function() {
-    before(function() {
+    before(async function() {
-      return Promise.all([deleteAllMessages(), ConversationController.load()]);
+      await deleteAllMessages();
+      ConversationController.reset();
+      await ConversationController.load();
     });
     after(function() {
       return deleteAllMessages();
     });
 
-    it('has no image url', function() {
-      var messages = new Whisper.MessageCollection();
-      var message = messages.add(attributes);
-      assert.isNull(message.getImageUrl());
-    });
-
-    it('updates image url', function() {
-      var messages = new Whisper.MessageCollection();
-      var message = messages.add({ attachments: [attachment] });
-
-      var firstUrl = message.getImageUrl();
-      message.updateImageUrl();
-      var secondUrl = message.getImageUrl();
-      assert.notEqual(secondUrl, firstUrl);
-    });
-
     it('gets outgoing contact', function() {
       var messages = new Whisper.MessageCollection();
       var message = messages.add(attributes);