diff --git a/test/models/messages_test.js b/test/models/messages_test.js index cff211938b..11fa5460ed 100644 --- a/test/models/messages_test.js +++ b/test/models/messages_test.js @@ -41,7 +41,6 @@ describe('Message', () => { const fakeDataMessage = new ArrayBuffer(0); const result = { dataMessage: fakeDataMessage, - discoveredIdentifierPairs: [], }; const promise = Promise.resolve(result); await message.send(promise); @@ -52,11 +51,7 @@ describe('Message', () => { it('updates the `sent` attribute', async () => { const message = createMessage({ type: 'outgoing', source, sent: false }); - await message.send( - Promise.resolve({ - discoveredIdentifierPairs: [], - }) - ); + await message.send(Promise.resolve({})); assert.isTrue(message.get('sent')); }); @@ -69,11 +64,7 @@ describe('Message', () => { callCount += 1; }); - await message.send( - Promise.resolve({ - discoveredIdentifierPairs: [], - }) - ); + await message.send(Promise.resolve({})); assert.strictEqual(callCount, 1); }); @@ -86,11 +77,7 @@ describe('Message', () => { calls.push(args); }); - await message.send( - Promise.resolve({ - discoveredIdentifierPairs: [], - }) - ); + await message.send(Promise.resolve({})); assert.lengthOf(calls, 1); assert.strictEqual(calls[0][0], message); @@ -125,7 +112,6 @@ describe('Message', () => { const result = { errors: [new Error('baz qux')], - discoveredIdentifierPairs: [], }; const promise = Promise.reject(result); await message.send(promise); diff --git a/ts/ConversationController.ts b/ts/ConversationController.ts index 117c7377d9..1a3a104e1b 100644 --- a/ts/ConversationController.ts +++ b/ts/ConversationController.ts @@ -655,7 +655,14 @@ export class ConversationController { const groups = await getAllGroupsInvolvingId(conversationId, { ConversationCollection: window.Whisper.ConversationCollection, }); - return groups.map(group => this._conversations.add(group)); + return groups.map(group => { + const existing = this.get(group.id); + if (existing) { + return existing; + } + + return this._conversations.add(group); + }); } async loadPromise(): Promise { diff --git a/ts/components/ConversationListItem.stories.tsx b/ts/components/ConversationListItem.stories.tsx index c14a35c08f..c7b811a34a 100644 --- a/ts/components/ConversationListItem.stories.tsx +++ b/ts/components/ConversationListItem.stories.tsx @@ -24,9 +24,11 @@ story.addDecorator(storyFn => ( const createProps = (overrideProps: Partial = {}): Props => ({ ...overrideProps, i18n, - isAccepted: boolean( - 'isAccepted', - overrideProps.isAccepted !== undefined ? overrideProps.isAccepted : true + acceptedMessageRequest: boolean( + 'acceptedMessageRequest', + overrideProps.acceptedMessageRequest !== undefined + ? overrideProps.acceptedMessageRequest + : true ), isMe: boolean('isMe', overrideProps.isMe || false), avatarPath: text('avatarPath', overrideProps.avatarPath || ''), @@ -103,7 +105,7 @@ story.add('Typing Status', () => { story.add('Message Request', () => { const props = createProps({ - isAccepted: false, + acceptedMessageRequest: false, lastMessage: { text: 'A Message', status: 'delivered', diff --git a/ts/components/ConversationListItem.tsx b/ts/components/ConversationListItem.tsx index 53c57e5407..19a7df4406 100644 --- a/ts/components/ConversationListItem.tsx +++ b/ts/components/ConversationListItem.tsx @@ -39,7 +39,7 @@ export type PropsData = { unreadCount?: number; isSelected: boolean; - isAccepted?: boolean; + acceptedMessageRequest?: boolean; draftPreview?: string; shouldShowDraft?: boolean; @@ -167,7 +167,7 @@ export class ConversationListItem extends React.PureComponent { const { draftPreview, i18n, - isAccepted, + acceptedMessageRequest, lastMessage, muteExpiresAt, shouldShowDraft, @@ -209,7 +209,7 @@ export class ConversationListItem extends React.PureComponent { {muteExpiresAt && Date.now() < muteExpiresAt && ( )} - {!isAccepted ? ( + {!acceptedMessageRequest ? ( {i18n('ConversationListItem--message-request')} diff --git a/ts/components/conversation/ConversationHeader.stories.tsx b/ts/components/conversation/ConversationHeader.stories.tsx index 2e0a26b6fb..0f63ac3c8e 100644 --- a/ts/components/conversation/ConversationHeader.stories.tsx +++ b/ts/components/conversation/ConversationHeader.stories.tsx @@ -70,7 +70,7 @@ const stories: Array = [ type: 'direct', id: '1', profileName: '🔥Flames🔥', - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -85,7 +85,7 @@ const stories: Array = [ phoneNumber: '(202) 555-0002', type: 'direct', id: '2', - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -100,7 +100,7 @@ const stories: Array = [ phoneNumber: '(202) 555-0002', type: 'direct', id: '2', - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -115,7 +115,7 @@ const stories: Array = [ id: '3', title: '🔥Flames🔥', profileName: '🔥Flames🔥', - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -127,7 +127,7 @@ const stories: Array = [ phoneNumber: '(202) 555-0011', type: 'direct', id: '11', - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -141,7 +141,7 @@ const stories: Array = [ title: '(202) 555-0004', type: 'direct', id: '4', - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -165,7 +165,7 @@ const stories: Array = [ value: 10, }, ], - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -179,7 +179,7 @@ const stories: Array = [ type: 'direct', id: '6', muteExpirationLabel: '10/18/3000, 11:11 AM', - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -211,7 +211,7 @@ const stories: Array = [ value: 10, }, ], - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -237,7 +237,7 @@ const stories: Array = [ value: 10, }, ], - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -257,7 +257,7 @@ const stories: Array = [ id: '7', type: 'direct', isMe: true, - isAccepted: true, + acceptedMessageRequest: true, ...actionProps, ...housekeepingProps, }, @@ -277,7 +277,7 @@ const stories: Array = [ id: '7', type: 'direct', isMe: false, - isAccepted: false, + acceptedMessageRequest: false, ...actionProps, ...housekeepingProps, }, diff --git a/ts/components/conversation/ConversationHeader.tsx b/ts/components/conversation/ConversationHeader.tsx index 4abb8d796f..38835156f1 100644 --- a/ts/components/conversation/ConversationHeader.tsx +++ b/ts/components/conversation/ConversationHeader.tsx @@ -31,7 +31,7 @@ export interface PropsDataType { type: 'direct' | 'group'; title: string; - isAccepted?: boolean; + acceptedMessageRequest?: boolean; isVerified?: boolean; isMe?: boolean; isArchived?: boolean; @@ -305,7 +305,7 @@ export class ConversationHeader extends React.Component { const { disableTimerChanges, i18n, - isAccepted, + acceptedMessageRequest, isMe, isPinned, type, @@ -388,7 +388,7 @@ export class ConversationHeader extends React.Component { {i18n('showSafetyNumber')} ) : null} - {!isGroup && isAccepted ? ( + {!isGroup && acceptedMessageRequest ? ( {i18n('resetSession')} ) : null} {isArchived ? ( diff --git a/ts/model-types.d.ts b/ts/model-types.d.ts index 71bbfeb5ae..32f0c3927d 100644 --- a/ts/model-types.d.ts +++ b/ts/model-types.d.ts @@ -147,7 +147,7 @@ export type ConversationAttributesType = { draftTimestamp: number | null; inbox_position: number; isPinned: boolean; - lastMessageDeletedForEveryone: unknown; + lastMessageDeletedForEveryone: boolean; lastMessageStatus: LastMessageStatus | null; messageCount: number; messageCountBeforeMessageRequests: number; diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index c5aff80cde..778101ecab 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -80,6 +80,8 @@ export class ConversationModel extends window.Backbone.Model< cachedProps?: ConversationType | null; + oldCachedProps?: ConversationType | null; + contactTypingTimers?: Record< string, { senderId: string; timer: NodeJS.Timer } @@ -235,6 +237,9 @@ export class ConversationModel extends window.Backbone.Model< // We clear our cached props whenever we change so that the next call to format() will // result in refresh via a getProps() call. See format() below. this.on('change', () => { + if (this.cachedProps) { + this.oldCachedProps = this.cachedProps; + } this.cachedProps = null; }); } @@ -1020,12 +1025,38 @@ export class ConversationModel extends window.Backbone.Model< } format(): ConversationType { - this.cachedProps = this.cachedProps || this.getProps(); + if (this.cachedProps) { + return this.cachedProps; + } + + const oldFormat = this.format; + // We don't want to crash or have an infinite loop if we loop back into this function + // again. We'll log a warning and returned old cached props or throw an error. + this.format = () => { + const { stack } = new Error('for stack'); + window.log.warn( + `Conversation.format()/${this.idForLogging()} reentrant call! ${stack}` + ); + if (!this.oldCachedProps) { + throw new Error( + `Conversation.format()/${this.idForLogging()} reentrant call, no old cached props!` + ); + } + return this.oldCachedProps; + }; + + this.cachedProps = this.getProps(); + + this.format = oldFormat; return this.cachedProps; } - getProps(): ConversationType { + // Note: this should never be called directly. Use conversation.format() instead, which + // maintains a cache, and protects against reentrant calls. + // Note: When writing code inside this function, do not call .format() on a conversation + // unless you are sure that it's not this very same conversation. + private getProps(): ConversationType { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const color = this.getColor()!; @@ -1060,7 +1091,7 @@ export class ConversationModel extends window.Backbone.Model< // TODO: DESKTOP-720 /* eslint-disable @typescript-eslint/no-non-null-assertion */ - const result = { + const result: ConversationType = { id: this.id, uuid: this.get('uuid'), e164: this.get('e164'), @@ -1077,7 +1108,6 @@ export class ConversationModel extends window.Backbone.Model< firstName: this.get('profileName')!, groupVersion, inboxPosition, - isAccepted: this.getAccepted(), isArchived: this.get('isArchived')!, isBlocked: this.isBlocked(), isMe: this.isMe(), @@ -1104,9 +1134,17 @@ export class ConversationModel extends window.Backbone.Model< timestamp, title: this.getTitle()!, type: (this.isPrivate() ? 'direct' : 'group') as ConversationTypeType, - typingContact: typingContact ? typingContact.format() : null, unreadCount: this.get('unreadCount')! || 0, }; + + if (typingContact) { + // We don't want to call .format() on our own conversation + if (typingContact.id === this.id) { + result.typingContact = result; + } else { + result.typingContact = typingContact.format(); + } + } /* eslint-enable @typescript-eslint/no-non-null-assertion */ return result; @@ -2710,8 +2748,7 @@ export class ConversationModel extends window.Backbone.Model< if (result) { await this.handleMessageSendResult( result.failoverIdentifiers, - result.unidentifiedDeliveries, - result.discoveredIdentifierPairs + result.unidentifiedDeliveries ); } return result; @@ -2721,8 +2758,7 @@ export class ConversationModel extends window.Backbone.Model< if (result) { await this.handleMessageSendResult( result.failoverIdentifiers, - result.unidentifiedDeliveries, - result.discoveredIdentifierPairs + result.unidentifiedDeliveries ); } throw result; @@ -2732,23 +2768,8 @@ export class ConversationModel extends window.Backbone.Model< async handleMessageSendResult( failoverIdentifiers: Array | undefined, - unidentifiedDeliveries: Array | undefined, - discoveredIdentifierPairs: - | Array<{ - uuid: string | null; - e164: string | null; - }> - | undefined + unidentifiedDeliveries: Array | undefined ): Promise { - (discoveredIdentifierPairs || []).forEach(item => { - const { uuid, e164 } = item; - window.ConversationController.ensureContactIds({ - uuid, - e164, - highTrust: true, - }); - }); - await Promise.all( (failoverIdentifiers || []).map(async identifier => { const conversation = window.ConversationController.get(identifier); @@ -4246,15 +4267,37 @@ window.Whisper.ConversationCollection = window.Backbone.Collection.extend({ this._byGroupId = Object.create(null); }, - add(...models: Array) { - const result = window.Backbone.Collection.prototype.add.apply( - this, - models as WhatIsThis + add(data: WhatIsThis | Array) { + let hydratedData; + + // First, we need to ensure that the data we're working with is Conversation models + if (Array.isArray(data)) { + hydratedData = []; + for (let i = 0, max = data.length; i < max; i += 1) { + const item = data[i]; + + // We create a new model if it's not already a model + if (!item.get) { + hydratedData.push(new Whisper.Conversation(item)); + } else { + hydratedData.push(item); + } + } + } else if (!data.get) { + hydratedData = new Whisper.Conversation(data); + } else { + hydratedData = data; + } + + // Next, we update our lookups first to prevent infinite loops on the 'add' event + this.generateLookups( + Array.isArray(hydratedData) ? hydratedData : [hydratedData] ); - this.generateLookups(Array.isArray(result) ? result.slice(0) : [result]); + // Lastly, we fire off the add events related to this change + window.Backbone.Collection.prototype.add.call(this, hydratedData); - return result; + return hydratedData; }, /** diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index 23e50c58da..5492598bee 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -54,6 +54,7 @@ export type ConversationType = { lastMessage?: { status: LastMessageStatus; text: string; + deletedForEveryone?: boolean; }; phoneNumber?: string; membersCount?: number; @@ -76,6 +77,7 @@ export type ConversationType = { draftText?: string | null; draftPreview?: string; + sharedGroupNames?: Array; groupVersion?: 1 | 2; isMissingMandatoryProfileSharing?: boolean; messageRequestsEnabled?: boolean; diff --git a/ts/textsecure/OutgoingMessage.ts b/ts/textsecure/OutgoingMessage.ts index beb0b3d6b7..0ad166c666 100644 --- a/ts/textsecure/OutgoingMessage.ts +++ b/ts/textsecure/OutgoingMessage.ts @@ -53,11 +53,6 @@ export default class OutgoingMessage { unidentifiedDeliveries: Array; - discoveredIdentifierPairs: Array<{ - e164: string; - uuid: string; - }>; - sendMetadata?: SendMetadataType; senderCertificate?: ArrayBuffer; @@ -93,7 +88,6 @@ export default class OutgoingMessage { this.successfulIdentifiers = []; this.failoverIdentifiers = []; this.unidentifiedDeliveries = []; - this.discoveredIdentifierPairs = []; const { sendMetadata, senderCertificate, online } = options; this.sendMetadata = sendMetadata; @@ -109,7 +103,6 @@ export default class OutgoingMessage { failoverIdentifiers: this.failoverIdentifiers, errors: this.errors, unidentifiedDeliveries: this.unidentifiedDeliveries, - discoveredIdentifierPairs: this.discoveredIdentifierPairs, }); } } @@ -621,9 +614,10 @@ export default class OutgoingMessage { ]); const uuid = lookup[identifier]; if (uuid) { - this.discoveredIdentifierPairs.push({ + window.ConversationController.ensureContactIds({ uuid, e164: identifier, + highTrust: true, }); identifier = uuid; } else { diff --git a/ts/textsecure/SendMessage.ts b/ts/textsecure/SendMessage.ts index c11d068c0a..0d9e9b9670 100644 --- a/ts/textsecure/SendMessage.ts +++ b/ts/textsecure/SendMessage.ts @@ -78,10 +78,6 @@ export type CallbackResultType = { errors?: Array; unidentifiedDeliveries?: Array; dataMessage?: ArrayBuffer; - discoveredIdentifierPairs: Array<{ - e164: string; - uuid: string | null; - }>; }; type PreviewType = { @@ -1383,7 +1379,6 @@ export default class MessageSender { if (identifiers.length === 0) { return Promise.resolve({ dataMessage: proto.toArrayBuffer(), - discoveredIdentifierPairs: [], errors: [], failoverIdentifiers: [], successfulIdentifiers: [], @@ -1657,7 +1652,6 @@ export default class MessageSender { errors: [], unidentifiedDeliveries: [], dataMessage: await this.getMessageProtoObj(attrs), - discoveredIdentifierPairs: [], }); } @@ -1731,7 +1725,6 @@ export default class MessageSender { errors: [], unidentifiedDeliveries: [], dataMessage: await this.getMessageProtoObj(attrs), - discoveredIdentifierPairs: [], }); }