diff --git a/ts/jobs/helpers/sendNormalMessage.ts b/ts/jobs/helpers/sendNormalMessage.ts index 62cedf8ae93..be77c094d30 100644 --- a/ts/jobs/helpers/sendNormalMessage.ts +++ b/ts/jobs/helpers/sendNormalMessage.ts @@ -15,7 +15,6 @@ import { SignalService as Proto } from '../../protobuf'; import { handleMessageSend } from '../../util/handleMessageSend'; import { findAndFormatContact } from '../../util/findAndFormatContact'; import { uploadAttachment } from '../../util/uploadAttachment'; -import { getMessageSentTimestamp } from '../../util/getMessageSentTimestamp'; import type { CallbackResultType } from '../../textsecure/Types.d'; import { isSent } from '../../messages/MessageSendState'; import { isOutgoing, canReact } from '../../state/selectors/message'; @@ -54,6 +53,12 @@ import type { DurationInSeconds } from '../../util/durations'; import type { ServiceIdString } from '../../types/ServiceId'; import { normalizeAci } from '../../util/normalizeAci'; import * as Bytes from '../../Bytes'; +import { + getPropForTimestamp, + getTargetOfThisEditTimestamp, + setPropForTimestamp, +} from '../../util/editHelpers'; +import { getMessageSentTimestamp } from '../../util/getMessageSentTimestamp'; const LONG_ATTACHMENT_LIMIT = 2048; const MAX_CONCURRENT_ATTACHMENT_UPLOADS = 5; @@ -100,6 +105,20 @@ export async function sendNormalMessage( return; } + // The original timestamp for this message + const messageTimestamp = getMessageSentTimestamp(message.attributes, { + includeEdits: false, + log, + }); + // The timestamp for the thing we're sending now, whether a first send or an edit + const targetTimestamp = editedMessageTimestamp || messageTimestamp; + // The timestamp identifying the target of this edit; could be the original timestamp + // or the most recent edit prior to this one + const targetOfThisEditTimestamp = getTargetOfThisEditTimestamp({ + message, + targetTimestamp, + }); + let messageSendErrors: Array = []; // We don't want to save errors on messages unless we're giving up. If it's our @@ -118,9 +137,11 @@ export async function sendNormalMessage( if (!shouldContinue) { log.info(`message ${messageId} ran out of time. Giving up on sending it`); - await markMessageFailed(message, [ - new Error('Message send ran out of time'), - ]); + await markMessageFailed({ + message, + errors: [new Error('Message send ran out of time')], + targetTimestamp, + }); return; } @@ -141,6 +162,7 @@ export async function sendNormalMessage( log, message, conversation, + targetTimestamp, }); if (untrustedServiceIds.length) { @@ -169,14 +191,13 @@ export async function sendNormalMessage( deletedForEveryoneTimestamp, expireTimer, bodyRanges, - messageTimestamp, preview, quote, reaction, sticker, storyMessage, storyContext, - } = await getMessageSendData({ log, message }); + } = await getMessageSendData({ log, message, targetTimestamp }); if (reaction) { strictAssert( @@ -197,13 +218,21 @@ export async function sendNormalMessage( log.info( `could not react to ${messageId}. Removing this pending reaction` ); - await markMessageFailed(message, [ - new Error('Could not react to story'), - ]); + await markMessageFailed({ + message, + errors: [new Error('Could not react to story')], + targetTimestamp, + }); return; } } + log.info( + 'Sending normal message;', + `editedMessageTimestamp=${editedMessageTimestamp},`, + `storyMessage=${Boolean(storyMessage)}` + ); + let messageSendPromise: Promise; if (recipientServiceIdsWithoutMe.length === 0) { @@ -215,7 +244,11 @@ export async function sendNormalMessage( log.info( 'No recipients; not sending to ourselves or to group, and no successful sends. Failing job.' ); - void markMessageFailed(message, [new Error('No valid recipients')]); + void markMessageFailed({ + message, + errors: [new Error('No valid recipients')], + targetTimestamp, + }); return; } @@ -229,7 +262,6 @@ export async function sendNormalMessage( bodyRanges, contact, deletedForEveryoneTimestamp, - editedMessageTimestamp, expireTimer, groupV2: conversation.getGroupV2Info({ members: recipientServiceIdsWithoutMe, @@ -240,10 +272,17 @@ export async function sendNormalMessage( recipients: allRecipientServiceIds, sticker, storyContext, - timestamp: messageTimestamp, + targetTimestampForEdit: editedMessageTimestamp + ? targetOfThisEditTimestamp + : undefined, + timestamp: targetTimestamp, reaction, }); - messageSendPromise = message.sendSyncMessageOnly(dataMessage, saveErrors); + messageSendPromise = message.sendSyncMessageOnly({ + dataMessage, + saveErrors, + targetTimestamp, + }); } else { const conversationType = conversation.get('type'); const sendOptions = await getSendOptions(conversation.attributes); @@ -275,7 +314,6 @@ export async function sendNormalMessage( bodyRanges, contact, deletedForEveryoneTimestamp, - editedMessageTimestamp, expireTimer, groupV2: groupV2Info, messageText: body, @@ -285,7 +323,10 @@ export async function sendNormalMessage( sticker, storyContext, reaction, - timestamp: messageTimestamp, + targetTimestampForEdit: editedMessageTimestamp + ? targetOfThisEditTimestamp + : undefined, + timestamp: targetTimestamp, }, messageId, sendOptions, @@ -300,25 +341,33 @@ export async function sendNormalMessage( log.info( `conversation ${conversation.idForLogging()} is not accepted; refusing to send` ); - void markMessageFailed(message, [ - new Error('Message request was not accepted'), - ]); + void markMessageFailed({ + message, + errors: [new Error('Message request was not accepted')], + targetTimestamp, + }); return; } if (isConversationUnregistered(conversation.attributes)) { log.info( `conversation ${conversation.idForLogging()} is unregistered; refusing to send` ); - void markMessageFailed(message, [ - new Error('Contact no longer has a Signal account'), - ]); + void markMessageFailed({ + message, + errors: [new Error('Contact no longer has a Signal account')], + targetTimestamp, + }); return; } if (conversation.isBlocked()) { log.info( `conversation ${conversation.idForLogging()} is blocked; refusing to send` ); - void markMessageFailed(message, [new Error('Contact is blocked')]); + void markMessageFailed({ + message, + errors: [new Error('Contact is blocked')], + targetTimestamp, + }); return; } @@ -329,7 +378,6 @@ export async function sendNormalMessage( contact, contentHint: ContentHint.RESENDABLE, deletedForEveryoneTimestamp, - editedMessageTimestamp, expireTimer, groupId: undefined, serviceId: recipientServiceIdsWithoutMe[0], @@ -341,20 +389,24 @@ export async function sendNormalMessage( sticker, storyContext, reaction, - timestamp: messageTimestamp, + targetTimestampForEdit: editedMessageTimestamp + ? targetOfThisEditTimestamp + : undefined, + timestamp: targetTimestamp, // Note: 1:1 story replies should not set story=true - they aren't group sends urgent: true, includePniSignatureMessage: true, }); } - messageSendPromise = message.send( - handleMessageSend(innerPromise, { + messageSendPromise = message.send({ + promise: handleMessageSend(innerPromise, { messageIds: [messageId], sendType: 'message', }), - saveErrors - ); + saveErrors, + targetTimestamp, + }); // Because message.send swallows and processes errors, we'll await the inner promise // to get the SendMessageProtoError, which gives us information upstream @@ -377,7 +429,12 @@ export async function sendNormalMessage( await messageSendPromise; const didFullySend = - !messageSendErrors.length || didSendToEveryone(message); + !messageSendErrors.length || + didSendToEveryone({ + log, + message, + targetTimestamp: editedMessageTimestamp || messageTimestamp, + }); if (!didFullySend) { throw new Error('message did not fully send'); } @@ -387,7 +444,12 @@ export async function sendNormalMessage( errors, isFinalAttempt, log, - markFailed: () => markMessageFailed(message, messageSendErrors), + markFailed: () => + markMessageFailed({ + message, + errors: messageSendErrors, + targetTimestamp, + }), timeRemaining, // In the case of a failed group send thrownError will not be SentMessageProtoError, // but we should have been able to harvest the original error. In the Note to Self @@ -402,10 +464,12 @@ function getMessageRecipients({ log, conversation, message, + targetTimestamp, }: Readonly<{ log: LoggerType; conversation: ConversationModel; message: MessageModel; + targetTimestamp: number; }>): { allRecipientServiceIds: Array; recipientServiceIdsWithoutMe: Array; @@ -419,7 +483,15 @@ function getMessageRecipients({ const currentConversationRecipients = conversation.getMemberConversationIds(); - Object.entries(message.get('sendStateByConversationId') || {}).forEach( + const sendStateByConversationId = + getPropForTimestamp({ + log, + message, + prop: 'sendStateByConversationId', + targetTimestamp, + }) || {}; + + Object.entries(sendStateByConversationId).forEach( ([recipientConversationId, sendState]) => { const recipient = window.ConversationController.get( recipientConversationId @@ -483,9 +555,11 @@ function getMessageRecipients({ async function getMessageSendData({ log, message, + targetTimestamp, }: Readonly<{ log: LoggerType; message: MessageModel; + targetTimestamp: number; }>): Promise<{ attachments: Array; body: undefined | string; @@ -493,7 +567,6 @@ async function getMessageSendData({ deletedForEveryoneTimestamp: undefined | number; expireTimer: undefined | DurationInSeconds; bodyRanges: undefined | ReadonlyArray; - messageTimestamp: number; preview: Array | undefined; quote: OutgoingQuoteType | undefined; sticker: OutgoingStickerType | undefined; @@ -501,25 +574,22 @@ async function getMessageSendData({ storyMessage?: MessageModel; storyContext?: StoryContextType; }> { - const editMessageTimestamp = message.get('editMessageTimestamp'); - - const mainMessageTimestamp = getMessageSentTimestamp(message.attributes, { - includeEdits: false, - log, - }); - const messageTimestamp = editMessageTimestamp || mainMessageTimestamp; - const storyId = message.get('storyId'); // Figure out if we need to upload message body as an attachment. - let body = message.get('body'); + let body = getPropForTimestamp({ + log, + message, + prop: 'body', + targetTimestamp, + }); let maybeLongAttachment: AttachmentWithHydratedData | undefined; if (body && body.length > LONG_ATTACHMENT_LIMIT) { const data = Bytes.fromString(body); maybeLongAttachment = { contentType: LONG_MESSAGE, - fileName: `long-message-${messageTimestamp}.txt`, + fileName: `long-message-${targetTimestamp}.txt`, data, size: data.byteLength, }; @@ -530,6 +600,13 @@ async function getMessageSendData({ concurrency: MAX_CONCURRENT_ATTACHMENT_UPLOADS, }); + const preUploadAttachments = + getPropForTimestamp({ + log, + message, + prop: 'attachments', + targetTimestamp, + }) || []; const [ uploadedAttachments, maybeUploadedLongAttachment, @@ -540,16 +617,32 @@ async function getMessageSendData({ storyMessage, ] = await Promise.all([ uploadQueue.addAll( - (message.get('attachments') ?? []).map( - attachment => () => uploadSingleAttachment(message, attachment) + preUploadAttachments.map( + attachment => () => + uploadSingleAttachment({ + attachment, + log, + message, + targetTimestamp, + }) ) ), uploadQueue.add(async () => maybeLongAttachment ? uploadAttachment(maybeLongAttachment) : undefined ), uploadMessageContacts(message, uploadQueue), - uploadMessagePreviews(message, uploadQueue), - uploadMessageQuote(message, uploadQueue), + uploadMessagePreviews({ + log, + message, + targetTimestamp, + uploadQueue, + }), + uploadMessageQuote({ + log, + message, + targetTimestamp, + uploadQueue, + }), uploadMessageSticker(message, uploadQueue), storyId ? __DEPRECATED$getMessageById(storyId) : undefined, ]); @@ -582,9 +675,12 @@ async function getMessageSendData({ contact, deletedForEveryoneTimestamp: message.get('deletedForEveryoneTimestamp'), expireTimer: message.get('expireTimer'), - // TODO: we want filtration here if feature flag doesn't allow format/spoiler sends - bodyRanges: message.get('bodyRanges'), - messageTimestamp, + bodyRanges: getPropForTimestamp({ + log, + message, + prop: 'bodyRanges', + targetTimestamp, + }), preview, quote, reaction: reactionForSend, @@ -604,10 +700,17 @@ async function getMessageSendData({ }; } -async function uploadSingleAttachment( - message: MessageModel, - attachment: AttachmentType -): Promise { +async function uploadSingleAttachment({ + attachment, + log, + message, + targetTimestamp, +}: { + attachment: AttachmentType; + log: LoggerType; + message: MessageModel; + targetTimestamp: number; +}): Promise { const { loadAttachmentData } = window.Signal.Migrations; const withData = await loadAttachmentData(attachment); @@ -615,7 +718,12 @@ async function uploadSingleAttachment( // Add digest to the attachment const logId = `uploadSingleAttachment(${message.idForLogging()}`; - const oldAttachments = message.get('attachments'); + const oldAttachments = getPropForTimestamp({ + log, + message, + prop: 'attachments', + targetTimestamp, + }); strictAssert( oldAttachments !== undefined, `${logId}: Attachment was uploaded, but message doesn't ` + @@ -634,24 +742,42 @@ async function uploadSingleAttachment( ...copyCdnFields(uploaded), }; - message.set('attachments', newAttachments); + setPropForTimestamp({ + log, + message, + prop: 'attachments', + targetTimestamp, + value: newAttachments, + }); return uploaded; } -async function uploadMessageQuote( - message: MessageModel, - uploadQueue: PQueue -): Promise { +async function uploadMessageQuote({ + log, + message, + targetTimestamp, + uploadQueue, +}: { + log: LoggerType; + message: MessageModel; + targetTimestamp: number; + uploadQueue: PQueue; +}): Promise { const { loadQuoteData } = window.Signal.Migrations; // We don't update the caches here because (1) we expect the caches to be populated // on initial send, so they should be there in the 99% case (2) if you're retrying // a failed message across restarts, we don't touch the cache for simplicity. If // sends are failing, let's not add the complication of a cache. + const startingQuote = getPropForTimestamp({ + log, + message, + prop: 'quote', + targetTimestamp, + }); const loadedQuote = - message.cachedOutgoingQuoteData || - (await loadQuoteData(message.get('quote'))); + message.cachedOutgoingQuoteData || (await loadQuoteData(startingQuote)); if (!loadedQuote) { return undefined; @@ -680,7 +806,12 @@ async function uploadMessageQuote( // Update message with attachment digests const logId = `uploadMessageQuote(${message.idForLogging()}`; - const oldQuote = message.get('quote'); + const oldQuote = getPropForTimestamp({ + log, + message, + prop: 'quote', + targetTimestamp, + }); strictAssert(oldQuote, `${logId}: Quote is gone after upload`); const newQuote = { @@ -711,7 +842,13 @@ async function uploadMessageQuote( }; }), }; - message.set('quote', newQuote); + setPropForTimestamp({ + log, + message, + prop: 'quote', + targetTimestamp, + value: newQuote, + }); return { isGiftBadge: loadedQuote.isGiftBadge, @@ -725,17 +862,31 @@ async function uploadMessageQuote( }; } -async function uploadMessagePreviews( - message: MessageModel, - uploadQueue: PQueue -): Promise | undefined> { +async function uploadMessagePreviews({ + log, + message, + targetTimestamp, + uploadQueue, +}: { + log: LoggerType; + message: MessageModel; + targetTimestamp: number; + uploadQueue: PQueue; +}): Promise | undefined> { const { loadPreviewData } = window.Signal.Migrations; // See uploadMessageQuote for comment on how we do caching for these // attachments. + const startingPreview = getPropForTimestamp({ + log, + message, + prop: 'preview', + targetTimestamp, + }); + const loadedPreviews = message.cachedOutgoingPreviewData || - (await loadPreviewData(message.get('preview'))); + (await loadPreviewData(startingPreview)); if (!loadedPreviews) { return undefined; @@ -766,7 +917,12 @@ async function uploadMessagePreviews( // Update message with attachment digests const logId = `uploadMessagePreviews(${message.idForLogging()}`; - const oldPreview = message.get('preview'); + const oldPreview = getPropForTimestamp({ + log, + message, + prop: 'preview', + targetTimestamp, + }); strictAssert(oldPreview, `${logId}: Link preview is gone after upload`); const newPreview = oldPreview.map((preview, index) => { @@ -788,7 +944,14 @@ async function uploadMessagePreviews( }, }; }); - message.set('preview', newPreview); + + setPropForTimestamp({ + log, + message, + prop: 'preview', + targetTimestamp, + value: newPreview, + }); return uploadedPreviews; } @@ -928,20 +1091,38 @@ async function uploadMessageContacts( return uploadedContacts; } -async function markMessageFailed( - message: MessageModel, - errors: Array -): Promise { - message.markFailed(); +async function markMessageFailed({ + errors, + message, + targetTimestamp, +}: { + errors: Array; + message: MessageModel; + targetTimestamp: number; +}): Promise { + message.markFailed(targetTimestamp); void message.saveErrors(errors, { skipSave: true }); await window.Signal.Data.saveMessage(message.attributes, { ourAci: window.textsecure.storage.user.getCheckedAci(), }); } -function didSendToEveryone(message: Readonly): boolean { +function didSendToEveryone({ + log, + message, + targetTimestamp, +}: { + log: LoggerType; + message: MessageModel; + targetTimestamp: number; +}): boolean { const sendStateByConversationId = - message.get('sendStateByConversationId') || {}; + getPropForTimestamp({ + log, + message, + prop: 'sendStateByConversationId', + targetTimestamp, + }) || {}; return Object.values(sendStateByConversationId).every(sendState => isSent(sendState.status) ); diff --git a/ts/jobs/helpers/sendReaction.ts b/ts/jobs/helpers/sendReaction.ts index 0dc899b913b..e1f9829986c 100644 --- a/ts/jobs/helpers/sendReaction.ts +++ b/ts/jobs/helpers/sendReaction.ts @@ -198,10 +198,11 @@ export async function sendReaction( recipients: allRecipientServiceIds, timestamp: pendingReaction.timestamp, }); - await ephemeralMessageForReactionSend.sendSyncMessageOnly( + await ephemeralMessageForReactionSend.sendSyncMessageOnly({ dataMessage, - saveErrors - ); + saveErrors, + targetTimestamp: pendingReaction.timestamp, + }); didFullySend = true; successfulConversationIds.add(ourConversationId); @@ -289,13 +290,14 @@ export async function sendReaction( ); } - await ephemeralMessageForReactionSend.send( - handleMessageSend(promise, { + await ephemeralMessageForReactionSend.send({ + promise: handleMessageSend(promise, { messageIds: [messageId], sendType: 'reaction', }), - saveErrors - ); + saveErrors, + targetTimestamp: pendingReaction.timestamp, + }); // Because message.send swallows and processes errors, we'll await the inner promise // to get the SendMessageProtoError, which gives us information upstream diff --git a/ts/jobs/helpers/sendStory.ts b/ts/jobs/helpers/sendStory.ts index 1d134582b1c..466fa190b0f 100644 --- a/ts/jobs/helpers/sendStory.ts +++ b/ts/jobs/helpers/sendStory.ts @@ -365,13 +365,14 @@ export async function sendStory( // eslint-disable-next-line no-param-reassign message.doNotSendSyncMessage = true; - const messageSendPromise = message.send( - handleMessageSend(innerPromise, { + const messageSendPromise = message.send({ + promise: handleMessageSend(innerPromise, { messageIds: [message.id], sendType: 'story', }), - saveErrors - ); + saveErrors, + targetTimestamp: message.get('timestamp'), + }); // Because message.send swallows and processes errors, we'll await the // inner promise to get the SendMessageProtoError, which gives us diff --git a/ts/model-types.d.ts b/ts/model-types.d.ts index 007e5721738..10f90188c1c 100644 --- a/ts/model-types.d.ts +++ b/ts/model-types.d.ts @@ -113,6 +113,8 @@ export type MessageReactionType = { isSentByConversationId?: Record; }; +// Note: when adding to the set of things that can change via edits, sendNormalMessage.ts +// needs more usage of get/setPropForTimestamp. export type EditHistoryType = { attachments?: Array; body?: string; diff --git a/ts/models/messages.ts b/ts/models/messages.ts index 58c362488c7..3470d28c8c9 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -39,7 +39,6 @@ import type { } from '../textsecure/Types.d'; import { SendMessageProtoError } from '../textsecure/Errors'; import { getUserLanguages } from '../util/userLanguages'; -import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp'; import { copyCdnFields } from '../util/attachments'; import type { ReactionType } from '../types/Reactions'; @@ -156,6 +155,11 @@ import { getSenderIdentifier } from '../util/getSenderIdentifier'; import { getNotificationDataForMessage } from '../util/getNotificationDataForMessage'; import { getNotificationTextForMessage } from '../util/getNotificationTextForMessage'; import { getMessageAuthorText } from '../util/getMessageAuthorText'; +import { + getPropForTimestamp, + setPropForTimestamp, + hasEditBeenSent, +} from '../util/editHelpers'; /* eslint-disable more/no-then */ @@ -831,18 +835,40 @@ export class MessageModel extends window.Backbone.Model { * Change any Pending send state to Failed. Note that this will not mark successful * sends failed. */ - public markFailed(): void { + public markFailed(editMessageTimestamp?: number): void { const now = Date.now(); - this.set( - 'sendStateByConversationId', - mapValues(this.get('sendStateByConversationId') || {}, sendState => + + const targetTimestamp = editMessageTimestamp || this.get('timestamp'); + const sendStateByConversationId = getPropForTimestamp({ + log, + message: this, + prop: 'sendStateByConversationId', + targetTimestamp, + }); + + const newSendStateByConversationId = mapValues( + sendStateByConversationId || {}, + sendState => sendStateReducer(sendState, { type: SendActionType.Failed, updatedAt: now, }) - ) ); + setPropForTimestamp({ + log, + message: this, + prop: 'sendStateByConversationId', + targetTimestamp, + value: newSendStateByConversationId, + }); + + // We aren't trying to send this message anymore, so we'll delete these caches + delete this.cachedOutgoingContactData; + delete this.cachedOutgoingPreviewData; + delete this.cachedOutgoingQuoteData; + delete this.cachedOutgoingStickerData; + this.notifyStorySendFailed(); } @@ -886,10 +912,15 @@ export class MessageModel extends window.Backbone.Model { return errors[0][0]; } - async send( - promise: Promise, - saveErrors?: (errors: Array) => void - ): Promise { + async send({ + promise, + saveErrors, + targetTimestamp, + }: { + promise: Promise; + saveErrors?: (errors: Array) => void; + targetTimestamp: number; + }): Promise { const updateLeftPane = this.getConversation()?.debouncedUpdateLastMessage ?? noop; @@ -926,7 +957,12 @@ export class MessageModel extends window.Backbone.Model { } const sendStateByConversationId = { - ...(this.get('sendStateByConversationId') || {}), + ...(getPropForTimestamp({ + log, + message: this, + prop: 'sendStateByConversationId', + targetTimestamp, + }) || {}), }; const sendIsNotFinal = @@ -970,9 +1006,13 @@ export class MessageModel extends window.Backbone.Model { }); // Integrate sends via sealed sender + const latestEditTimestamp = this.get('editMessageTimestamp'); + const sendIsLatest = + !latestEditTimestamp || targetTimestamp === latestEditTimestamp; const previousUnidentifiedDeliveries = this.get('unidentifiedDeliveries') || []; const newUnidentifiedDeliveries = + sendIsLatest && sendIsFinal && 'unidentifiedDeliveries' in result.value && Array.isArray(result.value.unidentifiedDeliveries) @@ -1051,7 +1091,6 @@ export class MessageModel extends window.Backbone.Model { } }); - attributesToUpdate.sendStateByConversationId = sendStateByConversationId; // Only update the expirationStartTimestamp if we don't already have one set if (!this.get('expirationStartTimestamp')) { attributesToUpdate.expirationStartTimestamp = sentToAtLeastOneRecipient @@ -1073,6 +1112,14 @@ export class MessageModel extends window.Backbone.Model { void this.saveErrors(errorsToSave, { skipSave: true }); } + setPropForTimestamp({ + log, + message: this, + prop: 'sendStateByConversationId', + targetTimestamp, + value: sendStateByConversationId, + }); + if (!this.doNotSave) { await window.Signal.Data.saveMessage(this.attributes, { ourAci: window.textsecure.storage.user.getCheckedAci(), @@ -1082,13 +1129,14 @@ export class MessageModel extends window.Backbone.Model { updateLeftPane(); if (sentToAtLeastOneRecipient && !this.doNotSendSyncMessage) { - promises.push(this.sendSyncMessage()); + promises.push(this.sendSyncMessage(targetTimestamp)); } await Promise.all(promises); const isTotalSuccess: boolean = result.success && !this.get('errors')?.length; + if (isTotalSuccess) { delete this.cachedOutgoingContactData; delete this.cachedOutgoingPreviewData; @@ -1099,10 +1147,15 @@ export class MessageModel extends window.Backbone.Model { updateLeftPane(); } - async sendSyncMessageOnly( - dataMessage: Uint8Array, - saveErrors?: (errors: Array) => void - ): Promise { + async sendSyncMessageOnly({ + targetTimestamp, + dataMessage, + saveErrors, + }: { + targetTimestamp: number; + dataMessage: Uint8Array; + saveErrors?: (errors: Array) => void; + }): Promise { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const conv = this.getConversation()!; this.set({ dataMessage }); @@ -1115,7 +1168,7 @@ export class MessageModel extends window.Backbone.Model { expirationStartTimestamp: Date.now(), errors: [], }); - const result = await this.sendSyncMessage(); + const result = await this.sendSyncMessage(targetTimestamp); this.set({ // We have to do this afterward, since we didn't have a previous send! unidentifiedDeliveries: @@ -1147,7 +1200,9 @@ export class MessageModel extends window.Backbone.Model { } } - async sendSyncMessage(): Promise { + async sendSyncMessage( + targetTimestamp: number + ): Promise { const ourConversation = window.ConversationController.getOurConversationOrThrow(); const sendOptions = await getSendOptions(ourConversation.attributes, { @@ -1173,8 +1228,8 @@ export class MessageModel extends window.Backbone.Model { if (!dataMessage) { return; } - const isEditedMessage = Boolean(this.get('editHistory')); - const isUpdate = Boolean(this.get('synced')) && !isEditedMessage; + const wasEditSent = hasEditBeenSent(this); + const isUpdate = Boolean(this.get('synced')) && !wasEditSent; // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const conv = this.getConversation()!; @@ -1206,9 +1261,7 @@ export class MessageModel extends window.Backbone.Model { map(conversationsWithSealedSender, c => c.id) ); - const timestamp = getMessageSentTimestamp(this.attributes, { log }); - - const encodedContent = isEditedMessage + const encodedContent = wasEditSent ? { encodedEditMessage: dataMessage, } @@ -1219,7 +1272,7 @@ export class MessageModel extends window.Backbone.Model { return handleMessageSend( messaging.sendSyncMessage({ ...encodedContent, - timestamp, + timestamp: targetTimestamp, destination: conv.get('e164'), destinationServiceId: conv.getServiceId(), expirationStartTimestamp: diff --git a/ts/test-electron/models/messages_test.ts b/ts/test-electron/models/messages_test.ts index d62fe657885..e8ca9bf5c5a 100644 --- a/ts/test-electron/models/messages_test.ts +++ b/ts/test-electron/models/messages_test.ts @@ -183,7 +183,10 @@ describe('Message', () => { editMessage: undefined, }); - await message.send(promise); + await message.send({ + promise, + targetTimestamp: message.get('timestamp'), + }); const result = message.get('sendStateByConversationId') || {}; assert.hasAllKeys(result, [ @@ -203,7 +206,10 @@ describe('Message', () => { const message = createMessage({ type: 'outgoing', source }); const promise = Promise.reject(new Error('foo bar')); - await message.send(promise); + await message.send({ + promise, + targetTimestamp: message.get('timestamp'), + }); const errors = message.get('errors') || []; assert.lengthOf(errors, 1); @@ -217,7 +223,10 @@ describe('Message', () => { errors: [new Error('baz qux')], }; const promise = Promise.reject(result); - await message.send(promise); + await message.send({ + promise, + targetTimestamp: message.get('timestamp'), + }); const errors = message.get('errors') || []; assert.lengthOf(errors, 1); diff --git a/ts/test-mock/messaging/edit_test.ts b/ts/test-mock/messaging/edit_test.ts index c9bba290b1b..609bf9f5e4c 100644 --- a/ts/test-mock/messaging/edit_test.ts +++ b/ts/test-mock/messaging/edit_test.ts @@ -766,8 +766,8 @@ describe('editing', function (this: Mocha.Suite) { strictAssert(v2.sendStateByConversationId, 'v2 has send state'); assert.strictEqual( v2.sendStateByConversationId[conversationId].status, - SendStatus.Pending, // TODO (DESKTOP-6176) - this should be Sent! - 'send state for v2 message is pending' + SendStatus.Sent, + 'send state for v2 message is sent' ); strictAssert(v3.sendStateByConversationId, 'v3 has send state'); @@ -780,8 +780,8 @@ describe('editing', function (this: Mocha.Suite) { strictAssert(v4.sendStateByConversationId, 'v4 has send state'); assert.strictEqual( v4.sendStateByConversationId[conversationId].status, - SendStatus.Pending, // TODO (DESKTOP-6176) - this should be Sent! - 'send state for v4 message is pending' + SendStatus.Sent, + 'send state for v4 message is sent' ); assert.strictEqual( diff --git a/ts/textsecure/SendMessage.ts b/ts/textsecure/SendMessage.ts index b57d79bcac5..99f3dafc12c 100644 --- a/ts/textsecure/SendMessage.ts +++ b/ts/textsecure/SendMessage.ts @@ -164,7 +164,6 @@ export type MessageOptionsType = { body?: string; bodyRanges?: ReadonlyArray; contact?: ReadonlyArray; - editedMessageTimestamp?: number; expireTimer?: DurationInSeconds; flags?: number; group?: { @@ -180,6 +179,7 @@ export type MessageOptionsType = { sticker?: OutgoingStickerType; reaction?: ReactionType; deletedForEveryoneTimestamp?: number; + targetTimestampForEdit?: number; timestamp: number; groupCallUpdate?: GroupCallUpdateType; storyContext?: StoryContextType; @@ -189,7 +189,7 @@ export type GroupSendOptionsType = { bodyRanges?: ReadonlyArray; contact?: ReadonlyArray; deletedForEveryoneTimestamp?: number; - editedMessageTimestamp?: number; + targetTimestampForEdit?: number; expireTimer?: DurationInSeconds; flags?: number; groupCallUpdate?: GroupCallUpdateType; @@ -692,11 +692,11 @@ export default class MessageSender { const message = await this.getHydratedMessage(options); const dataMessage = message.toProto(); - if (options.editedMessageTimestamp) { + if (options.targetTimestampForEdit) { const editMessage = new Proto.EditMessage(); editMessage.dataMessage = dataMessage; editMessage.targetSentTimestamp = Long.fromNumber( - options.editedMessageTimestamp + options.targetTimestampForEdit ); return Proto.EditMessage.encode(editMessage).finish(); } @@ -768,11 +768,11 @@ export default class MessageSender { const dataMessage = message.toProto(); const contentMessage = new Proto.Content(); - if (options.editedMessageTimestamp) { + if (options.targetTimestampForEdit) { const editMessage = new Proto.EditMessage(); editMessage.dataMessage = dataMessage; editMessage.targetSentTimestamp = Long.fromNumber( - options.editedMessageTimestamp + options.targetTimestampForEdit ); contentMessage.editMessage = editMessage; } else { @@ -858,7 +858,6 @@ export default class MessageSender { bodyRanges, contact, deletedForEveryoneTimestamp, - editedMessageTimestamp, expireTimer, flags, groupCallUpdate, @@ -870,6 +869,7 @@ export default class MessageSender { reaction, sticker, storyContext, + targetTimestampForEdit, timestamp, } = options; @@ -900,7 +900,6 @@ export default class MessageSender { body: messageText, contact, deletedForEveryoneTimestamp, - editedMessageTimestamp, expireTimer, flags, groupCallUpdate, @@ -912,6 +911,7 @@ export default class MessageSender { recipients, sticker, storyContext, + targetTimestampForEdit, timestamp, }; } @@ -1133,7 +1133,6 @@ export default class MessageSender { contact, contentHint, deletedForEveryoneTimestamp, - editedMessageTimestamp, expireTimer, groupId, serviceId, @@ -1146,6 +1145,7 @@ export default class MessageSender { sticker, storyContext, story, + targetTimestampForEdit, timestamp, urgent, includePniSignatureMessage, @@ -1155,7 +1155,6 @@ export default class MessageSender { contact?: ReadonlyArray; contentHint: number; deletedForEveryoneTimestamp: number | undefined; - editedMessageTimestamp?: number; expireTimer: DurationInSeconds | undefined; groupId: string | undefined; serviceId: ServiceIdString; @@ -1168,6 +1167,7 @@ export default class MessageSender { sticker?: OutgoingStickerType; storyContext?: StoryContextType; story?: boolean; + targetTimestampForEdit?: number; timestamp: number; urgent: boolean; includePniSignatureMessage?: boolean; @@ -1179,7 +1179,6 @@ export default class MessageSender { body: messageText, contact, deletedForEveryoneTimestamp, - editedMessageTimestamp, expireTimer, preview, profileKey, @@ -1188,6 +1187,7 @@ export default class MessageSender { recipients: [serviceId], sticker, storyContext, + targetTimestampForEdit, timestamp, }, contentHint, diff --git a/ts/util/canEditMessage.ts b/ts/util/canEditMessage.ts index a54dcdc59ec..b4fc5525d43 100644 --- a/ts/util/canEditMessage.ts +++ b/ts/util/canEditMessage.ts @@ -6,7 +6,6 @@ import { DAY } from './durations'; import { canEditMessages } from './canEditMessages'; import { isMoreRecentThan } from './timestamp'; import { isOutgoing } from '../messages/helpers'; -import { isSent, someSendStatus } from '../messages/MessageSendState'; export const MESSAGE_MAX_EDIT_COUNT = 10; @@ -16,7 +15,6 @@ export function canEditMessage(message: MessageAttributesType): boolean { !message.deletedForEveryone && isOutgoing(message) && isMoreRecentThan(message.sent_at, DAY) && - someSendStatus(message.sendStateByConversationId, isSent) && Boolean(message.body); if (result) { diff --git a/ts/util/editHelpers.ts b/ts/util/editHelpers.ts new file mode 100644 index 00000000000..9d11bf63df5 --- /dev/null +++ b/ts/util/editHelpers.ts @@ -0,0 +1,143 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { isNumber, sortBy } from 'lodash'; + +import { strictAssert } from './assert'; +import { isSent } from '../messages/MessageSendState'; + +import type { EditHistoryType } from '../model-types'; +import type { MessageModel } from '../models/messages'; +import type { LoggerType } from '../types/Logging'; + +export function hasEditBeenSent(message: MessageModel): boolean { + const originalTimestamp = message.get('timestamp'); + const editHistory = message.get('editHistory') || []; + + return Boolean( + editHistory.find(item => { + if (item.timestamp === originalTimestamp) { + return false; + } + return Object.values(item.sendStateByConversationId || {}).some( + sendState => isSent(sendState.status) + ); + }) + ); +} + +// The tricky bit for this function is if we are on our second+ attempt to send a given +// edit, we're still sending that edit. +export function getTargetOfThisEditTimestamp({ + message, + targetTimestamp, +}: { + message: MessageModel; + targetTimestamp: number; +}): number { + const originalTimestamp = message.get('timestamp'); + const editHistory = message.get('editHistory') || []; + + const sentItems = editHistory.filter(item => { + return item.timestamp <= targetTimestamp; + }); + const mostRecent = sortBy( + sentItems, + (item: EditHistoryType) => item.timestamp + ); + + const { length } = mostRecent; + + // We want the second-to-last item, because we may have partially sent targetTimestamp + if (length > 1) { + return mostRecent[length - 2].timestamp; + } + // If there's only one item, we'll use it + if (length > 0) { + return mostRecent[length - 1].timestamp; + } + + // This is a good failover in case we ever stop duplicating data in editHistory + return originalTimestamp; +} + +export function getPropForTimestamp({ + log, + message, + prop, + targetTimestamp, +}: { + log: LoggerType; + message: MessageModel; + prop: T; + targetTimestamp: number; +}): EditHistoryType[T] { + const logId = `getPropForTimestamp(${message.idForLogging()}, target=${targetTimestamp}})`; + + const editHistory = message.get('editHistory'); + const targetEdit = editHistory?.find( + item => item.timestamp === targetTimestamp + ); + if (!targetEdit) { + if (editHistory) { + log.warn(`${logId}: No edit found, using top-level data`); + } + return message.get(prop); + } + + return targetEdit[prop]; +} + +export function setPropForTimestamp({ + log, + message, + prop, + targetTimestamp, + value, +}: { + log: LoggerType; + message: MessageModel; + prop: T; + targetTimestamp: number; + value: EditHistoryType[T]; +}): void { + const logId = `setPropForTimestamp(${message.idForLogging()}, target=${targetTimestamp}})`; + + const editHistory = message.get('editHistory'); + const targetEditIndex = editHistory?.findIndex( + item => item.timestamp === targetTimestamp + ); + const targetEdit = + editHistory && isNumber(targetEditIndex) + ? editHistory[targetEditIndex] + : undefined; + + if (!targetEdit) { + if (editHistory) { + log.warn(`${logId}: No edit found, updating top-level data`); + } + message.set({ + [prop]: value, + }); + return; + } + + strictAssert(editHistory, 'Got targetEdit, but no editHistory'); + strictAssert( + isNumber(targetEditIndex), + 'Got targetEdit, but no targetEditIndex' + ); + + const newEditHistory = [...editHistory]; + newEditHistory[targetEditIndex] = { ...targetEdit, [prop]: value }; + + message.set('editHistory', newEditHistory); + + // We always edit the top-level attribute if this is the most recent send + const editMessageTimestamp = message.get('editMessageTimestamp'); + if (!editMessageTimestamp || editMessageTimestamp === targetTimestamp) { + message.set({ + [prop]: value, + }); + } +} diff --git a/ts/util/sendEditedMessage.ts b/ts/util/sendEditedMessage.ts index 36fd0bfdd8d..8ce57311d10 100644 --- a/ts/util/sendEditedMessage.ts +++ b/ts/util/sendEditedMessage.ts @@ -218,7 +218,7 @@ export async function sendEditedMessage( conversationId, messageId: targetMessageId, revision: conversation.get('revision'), - editedMessageTimestamp: targetSentTimestamp, + editedMessageTimestamp: timestamp, }, async jobToInsert => { log.info( @@ -246,7 +246,7 @@ export async function sendEditedMessage( now: timestamp, }); }, - duration => `${idLog}: batchDisptach took ${duration}ms` + duration => `${idLog}: batchDispatch took ${duration}ms` ); window.Signal.Data.updateConversation(conversation.attributes); diff --git a/ts/util/sendToGroup.ts b/ts/util/sendToGroup.ts index 977632511f5..144c91ecc11 100644 --- a/ts/util/sendToGroup.ts +++ b/ts/util/sendToGroup.ts @@ -819,6 +819,11 @@ export function _shouldFailSend(error: unknown, logId: string): boolean { // SendMessageChallengeError // MessageError if (isRecord(error) && typeof error.code === 'number') { + if (error.code === -1) { + logError("We don't have connectivity. Failing."); + return true; + } + if (error.code === 400) { logError('Invalid request, failing.'); return true;