From 1520c80013d8529a994f33ae60a10377ce8675c1 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Tue, 15 Jun 2021 17:44:14 -0700 Subject: [PATCH] Remove messageCollection from Conversation model --- js/delivery_receipts.js | 7 +- js/expiring_messages.js | 4 +- js/expiring_tap_to_view_messages.js | 1 - js/message_controller.js | 104 --------- js/modules/attachment_downloads.js | 8 - js/read_receipts.js | 7 +- test/_test.js | 2 + test/backup_test.js | 6 +- ts/background.ts | 4 +- .../conversation/MessageDetail.stories.tsx | 21 +- ts/components/conversation/MessageDetail.tsx | 12 +- ts/models/conversations.ts | 154 +++---------- ts/models/messages.ts | 42 ++-- ts/state/smart/MessageDetail.tsx | 9 + ts/test-electron/models/messages_test.ts | 36 --- ts/util/MessageController.ts | 113 +++++++++ ts/util/index.ts | 2 + ts/views/conversation_view.ts | 216 ++++++++++-------- ts/window.d.ts | 15 +- 19 files changed, 332 insertions(+), 431 deletions(-) delete mode 100644 js/message_controller.js create mode 100644 ts/util/MessageController.ts diff --git a/js/delivery_receipts.js b/js/delivery_receipts.js index bad8eb573..0335bfc6e 100644 --- a/js/delivery_receipts.js +++ b/js/delivery_receipts.js @@ -107,8 +107,11 @@ const conversation = ConversationController.get( message.get('conversationId') ); - if (conversation) { - conversation.trigger('delivered', message); + const updateLeftPane = conversation + ? conversation.debouncedUpdateLastMessage + : undefined; + if (updateLeftPane) { + updateLeftPane(); } this.remove(receipt); diff --git a/js/expiring_messages.js b/js/expiring_messages.js index 2d5b13551..caf9fe45b 100644 --- a/js/expiring_messages.js +++ b/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) { diff --git a/js/expiring_tap_to_view_messages.js b/js/expiring_tap_to_view_messages.js index 54d29bdfa..efdb68308 100644 --- a/js/expiring_tap_to_view_messages.js +++ b/js/expiring_tap_to_view_messages.js @@ -29,7 +29,6 @@ message.idForLogging() ); - message.trigger('erased'); await message.eraseContents(); }) ); diff --git a/js/message_controller.js b/js/message_controller.js deleted file mode 100644 index e43c3422e..000000000 --- a/js/message_controller.js +++ /dev/null @@ -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, - }; -})(); diff --git a/js/modules/attachment_downloads.js b/js/modules/attachment_downloads.js index 8600c0eb3..90bfe0b0a 100644 --- a/js/modules/attachment_downloads.js +++ b/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); diff --git a/js/read_receipts.js b/js/read_receipts.js index b9a727bd1..4582e84f1 100644 --- a/js/read_receipts.js +++ b/js/read_receipts.js @@ -108,8 +108,11 @@ const conversation = ConversationController.get( message.get('conversationId') ); - if (conversation) { - conversation.trigger('read', message); + const updateLeftPane = conversation + ? conversation.debouncedUpdateLastMessage + : undefined; + if (updateLeftPane) { + updateLeftPane(); } this.remove(receipt); diff --git a/test/_test.js b/test/_test.js index d412daf7a..fef1f84ce 100644 --- a/test/_test.js +++ b/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'); diff --git a/test/backup_test.js b/test/backup_test.js index 6e51b3d9f..ac9a4bb10 100644 --- a/test/backup_test.js +++ b/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); - const messageFromDB = removeId(messageCollection.at(0).attributes); + assert.strictEqual(messages.length, MESSAGE_COUNT); + const messageFromDB = removeId(messages.at(0).attributes); const expectedMessage = messageFromDB; console.log({ messageFromDB, expectedMessage }); assert.deepEqual(messageFromDB, expectedMessage); diff --git a/ts/background.ts b/ts/background.ts index 466984e56..e974feb63 100644 --- a/ts/background.ts +++ b/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 { } export async function startApp(): Promise { + window.Signal.Util.MessageController.install(); window.startupProcessingQueue = new window.Signal.Util.StartupQueue(); window.attachmentDownloadQueue = []; try { @@ -1712,7 +1714,7 @@ export async function startApp(): Promise { } window.Whisper.events.on('contactsync', () => { - if (window.reduxStore.getState().app.isShowingInstaller) { + if (window.reduxStore.getState().app.appView === AppViewType.Installer) { window.reduxActions.app.openInbox(); } }); diff --git a/ts/components/conversation/MessageDetail.stories.tsx b/ts/components/conversation/MessageDetail.stories.tsx index 10e5abbba..7e3384c03 100644 --- a/ts/components/conversation/MessageDetail.stories.tsx +++ b/ts/components/conversation/MessageDetail.stories.tsx @@ -47,8 +47,6 @@ const createProps = (overrideProps: Partial = {}): Props => ({ }), isOutgoingKeyError: false, isUnidentifiedDelivery: false, - onSendAnyway: action('onSendAnyway'), - onShowSafetyNumber: action('onShowSafetyNumber'), status: 'delivered', }, ], @@ -60,6 +58,9 @@ const createProps = (overrideProps: Partial = {}): 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', }, ], diff --git a/ts/components/conversation/MessageDetail.tsx b/ts/components/conversation/MessageDetail.tsx index 7da219548..9b6830d89 100644 --- a/ts/components/conversation/MessageDetail.tsx +++ b/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; - - 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 { } 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 { diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 444858e7a..60ef31c1f 100644 --- a/ts/models/conversations.ts +++ b/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 { 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 { const collection = new window.Backbone.Collection(); @@ -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 { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.debouncedUpdateLastMessage!(); - - 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 { - // 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 { - 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; + async onNewMessage(message: MessageModel): Promise { + const uuid = message.get('sourceUuid'); + const e164 = message.get('source'); + const sourceDevice = message.get('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 isNewMessage = true; - messagesAdded( - this.id, - [model.getReduxData()], - isNewMessage, - window.isActive() - ); - } + const { messagesAdded } = window.reduxActions.conversations; + const isNewMessage = true; + messagesAdded( + this.id, + [message.getReduxData()], + isNewMessage, + 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 { 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 { - // 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; - } - + async addMessageHistoryDisclaimer(): Promise { 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 { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.messageCollection!.reset([]); - this.set({ lastMessage: null, timestamp: null, diff --git a/ts/models/messages.ts b/ts/models/messages.ts index 401173e57..ebea782c7 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -284,7 +284,6 @@ export class MessageModel extends window.Backbone.Model { 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 { 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 { async cleanup(): Promise { 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 { 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 { async send( promise: Promise ): Promise> { - this.trigger('pending'); + const conversation = this.getConversation(); + const updateLeftPane = conversation?.debouncedUpdateLastMessage; + if (updateLeftPane) { + updateLeftPane(); + } + return (promise as Promise) .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 { }); } - 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 { ); } - 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 { 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 { await window.Signal.Data.saveMessage(this.attributes, { Message: window.Whisper.Message, }); - this.trigger('done'); - const errors = this.get('errors'); - if (errors) { - this.trigger('send-error', errors); - } else { - this.trigger('sent'); + if (updateLeftPane) { + updateLeftPane(); } } } diff --git a/ts/state/smart/MessageDetail.tsx b/ts/state/smart/MessageDetail.tsx index b41e270fa..1c17d80a5 100644 --- a/ts/state/smart/MessageDetail.tsx +++ b/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, diff --git a/ts/test-electron/models/messages_test.ts b/ts/test-electron/models/messages_test.ts index 8a6d57adb..ec05662be 100644 --- a/ts/test-electron/models/messages_test.ts +++ b/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 }); diff --git a/ts/util/MessageController.ts b/ts/util/MessageController.ts new file mode 100644 index 000000000..9c4a76647 --- /dev/null +++ b/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; + +export class MessageController { + private messageLookup: LookupType = Object.create(null); + + private msgIDsBySender = new Map(); + + private msgIDsBySentAt = new Map(); + + 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); + } +} diff --git a/ts/util/index.ts b/ts/util/index.ts index b1e9b68c3..c1da90c93 100644 --- a/ts/util/index.ts +++ b/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, diff --git a/ts/views/conversation_view.ts b/ts/views/conversation_view.ts index f156fefa2..4264feb92 100644 --- a/ts/views/conversation_view.ts +++ b/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(model, 'change:verified', this.onVerifiedChange); - this.listenTo(model, 'newmessage', this.addMessage); - this.listenTo(model, 'opened', this.onOpened); - this.listenTo(model, 'backgrounded', this.resetEmojiResults); - this.listenTo(model, 'scroll-to-message', this.scrollToMessage); - this.listenTo(model, 'unload', (reason: string) => + this.listenTo(this.model, 'destroy', this.stopListening); + this.listenTo(this.model, 'change:verified', this.onVerifiedChange); + this.listenTo(this.model, 'newmessage', this.lazyUpdateVerified); + + // These are triggered by InboxView + this.listenTo(this.model, 'opened', this.onOpened); + 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); - this.listenTo(model, 'begin-recording', this.captureAudio); - this.listenTo(model, 'attach-file', this.onChooseAttachment); - this.listenTo(model, 'escape-pressed', this.resetPanel); - this.listenTo(model, 'show-message-details', this.showMessageDetail); - this.listenTo(model, 'show-contact-modal', this.showContactModal); - this.listenTo(model, 'toggle-reply', (messageId: string | undefined) => { - const target = this.quote || !messageId ? null : messageId; - this.setQuoteMessage(target); - }); + + // These are triggered by background.ts for keyboard handling + this.listenTo(this.model, 'focus-composer', this.focusMessageField); + this.listenTo(this.model, 'open-all-media', this.showAllMedia); + this.listenTo(this.model, 'begin-recording', this.captureAudio); + this.listenTo(this.model, 'attach-file', this.onChooseAttachment); + this.listenTo(this.model, 'escape-pressed', this.resetPanel); + this.listenTo(this.model, 'show-message-details', this.showMessageDetail); + this.listenTo(this.model, 'show-contact-modal', this.showContactModal); + 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) { - throw new Error('Message collection does not exist'); + const message = window.MessageController.getById(options.messageId); + 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( - options.messageId - ); - assert(message, 'Message not found'); + const message = window.MessageController.getById(options.messageId); + if (!message) { + throw new Error( + `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 + ): Promise> { 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 = 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 = 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( - `showForwardMessageModal: Did not find message for id ${messageId}` - ); + throw new Error(`showForwardMessageModal: Message ${messageId} missing!`); } 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 | 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 | 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, 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( - `displayTapToViewMessage: Did not find message for id ${messageId}` - ); + throw new Error(`displayTapToViewMessage: Message ${messageId} missing!`); } 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( - `deleteMessage: Did not find message for id ${messageId}` - ); + throw new Error(`deleteMessage: Message ${messageId} missing!`); } window.showConfirmationDialog({ @@ -2934,8 +2954,7 @@ Whisper.ConversationView = Whisper.View.extend({ window.Signal.Data.removeMessage(message.id, { Message: Whisper.Message, }); - message.trigger('unload'); - this.model.messageCollection.remove(message.id); + message.cleanup(); 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 = model.messageCollection?.get(messageId); + const message = window.MessageController.getById(messageId); if (!message) { - throw new Error( - `showMessageDetail: Did not find message for id ${messageId}` - ); + throw new Error(`showMessageDetail: Message ${messageId} missing!`); } if (!message.isNormalBubble()) { diff --git a/ts/window.d.ts b/ts/window.d.ts index ba806e73a..9383cfc5f 100644 --- a/ts/window.d.ts +++ b/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; reduxActions: ReduxActions; - reduxStore: WhatIsThis; + reduxStore: Store; 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; }