From 854742c0b1d09dcf792ad06fe08b7ebcc4ec1760 Mon Sep 17 00:00:00 2001 From: William Grant Date: Mon, 3 Apr 2023 14:09:06 +0200 Subject: [PATCH] fix: improved logic for turning off disappearing messages --- ts/models/conversation.ts | 44 +++++++------------ ts/models/message.ts | 13 ++++-- ts/receiver/contentMessage.ts | 40 +++++++++-------- ts/receiver/dataMessage.ts | 32 +++++++------- ts/receiver/queuedJob.ts | 8 +++- .../messages/outgoing/ExpirableMessage.ts | 8 ++-- .../group/ClosedGroupMessage.ts | 5 +++ .../ClosedGroupVisibleMessage.ts | 5 --- .../outgoing/visibleMessage/VisibleMessage.ts | 5 --- 9 files changed, 78 insertions(+), 82 deletions(-) diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index bb2bbe274..2a0d0880b 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -95,10 +95,7 @@ import { import { sogsV3FetchPreviewAndSaveIt } from '../session/apis/open_group_api/sogsv3/sogsV3FetchFile'; import { Reaction } from '../types/Reaction'; import { Reactions } from '../util/reactions'; -import { - DisappearingMessageConversationType, - DisappearingMessageType, -} from '../util/expiringMessages'; +import { DisappearingMessageConversationType } from '../util/expiringMessages'; export class ConversationModel extends Backbone.Model { public updateLastMessage: () => any; @@ -635,8 +632,7 @@ export class ConversationModel extends Backbone.Model { if (this.isPrivate()) { if (this.isMe()) { - // TODO legacy messages support will be removed in a future release - if (!this.isDisappearingMode('legacy') && !this.isDisappearingMode('deleteAfterSend')) { + if (this.isDisappearingMode('deleteAfterRead')) { return; } chatMessageParams.syncTarget = this.id; @@ -668,8 +664,7 @@ export class ConversationModel extends Backbone.Model { } if (this.isMediumGroup()) { - // TODO legacy messages support will be removed in a future release - if (!this.isDisappearingMode('legacy') && !this.isDisappearingMode('disappearAfterSend')) { + if (this.isDisappearingMode('deleteAfterRead')) { return; } @@ -1082,7 +1077,7 @@ export class ConversationModel extends Backbone.Model { const lastDisappearingMessageChangeTimestamp = providedChangeTimestamp; let source = providedSource; - if (!expirationType || !expireTimer) { + if (expirationType === undefined || expireTimer === undefined) { expirationType = 'off'; expireTimer = 0; } @@ -1198,9 +1193,7 @@ export class ConversationModel extends Backbone.Model { const expirationTimerMessage = new ExpirationTimerUpdateMessage(expireUpdate); const pubkey = new PubKey(this.get('id')); await getMessageQueue().sendToPubKey(pubkey, expirationTimerMessage); - } else { - // Cannot be an open group - window?.log?.warn('TODO: Expiration update for closed groups are to be updated'); + } else if (this.isMediumGroup()) { const expireUpdateForGroup = { ...expireUpdate, groupId: this.get('id'), @@ -1209,8 +1202,9 @@ export class ConversationModel extends Backbone.Model { const expirationTimerMessage = new ExpirationTimerUpdateMessage(expireUpdateForGroup); await getMessageQueue().sendToGroup(expirationTimerMessage); + } else { + window?.log?.warn('Communities should not use disappearing messages'); } - return; } public triggerUIRefresh() { @@ -2237,24 +2231,18 @@ export class ConversationModel extends Backbone.Model { return []; } - // TODO I think this is flawed - private isDisappearingMode(mode: DisappearingMessageType) { + private isDisappearingMode(mode: DisappearingMessageConversationType) { // TODO legacy messages support will be removed in a future release const success = - this.get('expirationType') && this.get('expirationType') !== 'off' && mode === 'legacy' - ? this.get('expirationType') === 'legacy' - : mode === 'deleteAfterRead' + mode === 'deleteAfterRead' ? this.get('expirationType') === 'deleteAfterRead' - : this.get('expirationType') === 'deleteAfterSend'; - - if (!success) { - window.log.info( - `WIP: This message should be ${ - mode === 'legacy' ? ' a legacy disappearing message' : ` disappear after ${mode}` - }`, - this - ); - } + : mode === 'deleteAfterSend' + ? this.get('expirationType') === 'deleteAfterSend' + : mode === 'legacy' + ? this.get('expirationType') === 'legacy' + : mode === 'off' + ? this.get('expirationType') === 'off' + : false; return success; } diff --git a/ts/models/message.ts b/ts/models/message.ts index f12259db2..3fe396891 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -1086,7 +1086,6 @@ export class MessageModel extends Backbone.Model { const { dataMessage } = content; // TODO maybe we need to account for lastDisappearingMessageChangeTimestamp? - // if this message needs to be synced if ( dataMessage && (dataMessage.body?.length || @@ -1099,15 +1098,21 @@ export class MessageModel extends Backbone.Model { } // TODO legacy messages support will be removed in a future release + const isLegacyMessage = Boolean(dataMessage.expireTimer && dataMessage.expireTimer > -1); + const expirationType = content.expirationType ? DisappearingMessageConversationSetting[content.expirationType] - : DisappearingMessageConversationSetting[3]; - const expireTimer = content.expirationTimer || content?.dataMessage?.expireTimer || undefined; + : isLegacyMessage + ? DisappearingMessageConversationSetting[3] + : 'off'; + const expireTimer = isLegacyMessage + ? Number(dataMessage.expireTimer) + : content.expirationTimer; const lastDisappearingMessageChangeTimestamp = content.lastDisappearingMessageChangeTimestamp ? Number(content.lastDisappearingMessageChangeTimestamp) : undefined; - let expireUpdate: DisappearingMessageUpdate | null = null; + let expireUpdate: DisappearingMessageUpdate | null = null; if (expirationType && expireTimer !== undefined) { expireUpdate = { expirationType, diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index 7a1f1942c..4f5462a8c 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -408,13 +408,31 @@ export async function innerHandleSwarmContentMessage( perfStart(`handleSwarmDataMessage-${envelope.id}`); + // TODO legacy messages support will be removed in a future release // We will only support legacy disappearing messages for a short period before disappearing messages v2 is unlocked const isDisappearingMessagesV2Released = await checkIsFeatureReleased( 'Disappearing Messages V2' ); - const isLegacyMessage = Boolean(dataMessage.expireTimer && dataMessage.expireTimer > 0); + const isLegacyMessage = Boolean(dataMessage.expireTimer && dataMessage.expireTimer > -1); + + const expireUpdate: DisappearingMessageUpdate = { + expirationType: isDisappearingMessagesV2Released + ? DisappearingMessageConversationSetting[content.expirationType] + : isLegacyMessage + ? DisappearingMessageConversationSetting[3] + : 'off', + expireTimer: isDisappearingMessagesV2Released + ? content.expirationTimer + : isLegacyMessage + ? Number(dataMessage.expireTimer) + : 0, + lastDisappearingMessageChangeTimestamp: content.lastDisappearingMessageChangeTimestamp + ? Number(content.lastDisappearingMessageChangeTimestamp) + : undefined, + isLegacyMessage, + isDisappearingMessagesV2Released, + }; - // TODO account for outdated groups separately probably if (isLegacyMessage) { window.log.info( 'WIP: Received a legacy disappearing message', @@ -425,6 +443,7 @@ export async function innerHandleSwarmContentMessage( : '' }` ); + // trigger notice banner if (!conversationModel.get('hasOutdatedClient')) { conversationModel.set({ hasOutdatedClient: true }); @@ -435,23 +454,6 @@ export async function innerHandleSwarmContentMessage( } } - // TODO legacy messages support will be removed in a future release - const expireUpdate: DisappearingMessageUpdate = { - expirationType: - !isDisappearingMessagesV2Released && isLegacyMessage - ? DisappearingMessageConversationSetting[3] - : DisappearingMessageConversationSetting[content.expirationType] || 'off', - expireTimer: - !isDisappearingMessagesV2Released && isLegacyMessage - ? Number(dataMessage.expireTimer) - : content.expirationTimer, - lastDisappearingMessageChangeTimestamp: content.lastDisappearingMessageChangeTimestamp - ? Number(content.lastDisappearingMessageChangeTimestamp) - : undefined, - isLegacyMessage, - isDisappearingMessagesV2Released, - }; - await handleSwarmDataMessage( envelope, sentAtTimestamp, diff --git a/ts/receiver/dataMessage.ts b/ts/receiver/dataMessage.ts index 33426b685..1354d42b5 100644 --- a/ts/receiver/dataMessage.ts +++ b/ts/receiver/dataMessage.ts @@ -247,7 +247,7 @@ export async function handleSwarmDataMessage( // TODO handle sync messages separately window.log.info('WIP: Sync Message dropping'); } else { - const { + let { expirationType, expireTimer, lastDisappearingMessageChangeTimestamp, @@ -256,13 +256,19 @@ export async function handleSwarmDataMessage( } = expireUpdate; // TODO legacy messages support will be removed in a future release - msgModel.set({ - expirationType: isDisappearingMessagesV2Released + // if it is a legacy message and disappearing messages v2 is released then we ignore it and use the local client's conversation settings + expirationType = + isDisappearingMessagesV2Released && isLegacyMessage ? senderConversationModel.get('expirationType') - : expirationType, - expireTimer: isDisappearingMessagesV2Released + : expirationType; + expireTimer = + isDisappearingMessagesV2Released && isLegacyMessage ? senderConversationModel.get('expireTimer') - : expireTimer, + : expireTimer; + + msgModel.set({ + expirationType, + expireTimer, }); // TODO legacy messages support will be removed in a future release @@ -276,21 +282,13 @@ export async function handleSwarmDataMessage( expirationType, expireTimer, lastDisappearingMessageChangeTimestamp: isLegacyMessage - ? Date.now() + ? isDisappearingMessagesV2Released + ? senderConversationModel.get('lastDisappearingMessageChangeTimestamp') + : Date.now() : Number(lastDisappearingMessageChangeTimestamp), source: msgModel.get('source'), }; - if (isLegacyMessage && isDisappearingMessagesV2Released) { - // if it is a legacy message then we ignore it and use the local client's conversation settings - expirationTimerUpdate.expirationType = senderConversationModel.get('expirationType'); - expirationTimerUpdate.expireTimer = senderConversationModel.get('expireTimer'); - - expirationTimerUpdate.lastDisappearingMessageChangeTimestamp = senderConversationModel.get( - 'lastDisappearingMessageChangeTimestamp' - ); - } - msgModel.set({ expirationTimerUpdate, }); diff --git a/ts/receiver/queuedJob.ts b/ts/receiver/queuedJob.ts index ba11cb602..676df2fde 100644 --- a/ts/receiver/queuedJob.ts +++ b/ts/receiver/queuedJob.ts @@ -328,6 +328,8 @@ export async function handleMessageJob( ) || messageModel.get('timestamp')} in conversation ${conversation.idForLogging()}` ); + window.log.info(`WIP: handleMessageJob()`, messageModel, conversation, regularDataMessage); + const sendingDeviceConversation = await getConversationController().getOrCreateAndWait( source, ConversationTypeEnum.PRIVATE @@ -353,7 +355,11 @@ export async function handleMessageJob( if (messageModel.isExpirationTimerUpdate()) { const expirationTimerUpdate = messageModel.get('expirationTimerUpdate'); if (!expirationTimerUpdate || isEmpty(expirationTimerUpdate)) { - window.log.info(`WIP: There is a problem with the expiration timer update`, messageModel); + window.log.info( + `WIP: There is a problem with the expiration timer update`, + messageModel, + expirationTimerUpdate + ); return; } diff --git a/ts/session/messages/outgoing/ExpirableMessage.ts b/ts/session/messages/outgoing/ExpirableMessage.ts index c5a207e87..8865fe6de 100644 --- a/ts/session/messages/outgoing/ExpirableMessage.ts +++ b/ts/session/messages/outgoing/ExpirableMessage.ts @@ -24,8 +24,10 @@ export class ExpirableMessage extends ContentMessage { expirationType: this.expirationType === 'deleteAfterSend' ? SignalService.Content.ExpirationType.DELETE_AFTER_SEND - : SignalService.Content.ExpirationType.DELETE_AFTER_READ, - expirationTimer: this.expireTimer, + : this.expirationType === 'deleteAfterRead' + ? SignalService.Content.ExpirationType.DELETE_AFTER_READ + : undefined, + expirationTimer: this.expireTimer && this.expireTimer > -1 ? this.expireTimer : undefined, }); } @@ -33,7 +35,7 @@ export class ExpirableMessage extends ContentMessage { return this.expirationType; } - // TODO need to account for legacy messages here? + // TODO legacy messages? + update expire endpoint for message after read public ttl(): number { switch (this.expirationType) { case 'deleteAfterSend': diff --git a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts index 3210b0afb..50d7621e3 100644 --- a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts @@ -44,6 +44,11 @@ export abstract class ClosedGroupMessage extends ExpirableMessage { dataMessage.closedGroupControlMessage = new SignalService.DataMessage.ClosedGroupControlMessage(); + // TODO legacy messages support will be removed in a future release + if (this.expirationType === 'legacy' && this.expireTimer) { + dataMessage.expireTimer = this.expireTimer; + } + return dataMessage; } } diff --git a/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts b/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts index 98a2282b8..d40853112 100644 --- a/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts @@ -45,11 +45,6 @@ export class ClosedGroupVisibleMessage extends ClosedGroupMessage { dataProto.group = groupMessage; - // TODO legacy messages support will be removed in a future release - if (this.expirationType === 'legacy' && this.expireTimer) { - dataProto.expireTimer = this.expireTimer; - } - return dataProto; } } diff --git a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts index 87cd986a2..05782aa38 100644 --- a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts @@ -122,11 +122,6 @@ export class VisibleMessage extends ExpirableMessage { dataMessage.attachments = this.attachments || []; - // TODO should only happen in legacy mode and should be cancelled out once we have trigger the unix timestamp - // if (this.expireTimer) { - // dataMessage.expireTimer = this.expireTimer; - // } - if (this.preview) { dataMessage.preview = this.preview; }