diff --git a/ts/jobs/helpers/sendNormalMessage.ts b/ts/jobs/helpers/sendNormalMessage.ts index d224fd990..37e9f58a2 100644 --- a/ts/jobs/helpers/sendNormalMessage.ts +++ b/ts/jobs/helpers/sendNormalMessage.ts @@ -56,7 +56,7 @@ import * as Bytes from '../../Bytes'; import { getPropForTimestamp, getTargetOfThisEditTimestamp, - setPropForTimestamp, + getChangesForPropAtTimestamp, } from '../../util/editHelpers'; import { getMessageSentTimestamp } from '../../util/getMessageSentTimestamp'; @@ -115,7 +115,7 @@ export async function sendNormalMessage( // 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, + message: message.attributes, targetTimestamp, }); @@ -486,7 +486,7 @@ function getMessageRecipients({ const sendStateByConversationId = getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'sendStateByConversationId', targetTimestamp, }) || {}; @@ -579,7 +579,7 @@ async function getMessageSendData({ // Figure out if we need to upload message body as an attachment. let body = getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'body', targetTimestamp, }); @@ -603,7 +603,7 @@ async function getMessageSendData({ const preUploadAttachments = getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'attachments', targetTimestamp, }) || []; @@ -677,7 +677,7 @@ async function getMessageSendData({ expireTimer: message.get('expireTimer'), bodyRanges: getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'bodyRanges', targetTimestamp, }), @@ -720,7 +720,7 @@ async function uploadSingleAttachment({ const logId = `uploadSingleAttachment(${message.idForLogging()}`; const oldAttachments = getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'attachments', targetTimestamp, }); @@ -742,13 +742,16 @@ async function uploadSingleAttachment({ ...copyCdnFields(uploaded), }; - setPropForTimestamp({ + const attributesToUpdate = getChangesForPropAtTimestamp({ log, - message, + message: message.attributes, prop: 'attachments', targetTimestamp, value: newAttachments, }); + if (attributesToUpdate) { + message.set(attributesToUpdate); + } return uploaded; } @@ -772,7 +775,7 @@ async function uploadMessageQuote({ // sends are failing, let's not add the complication of a cache. const startingQuote = getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'quote', targetTimestamp, }); @@ -808,7 +811,7 @@ async function uploadMessageQuote({ const logId = `uploadMessageQuote(${message.idForLogging()}`; const oldQuote = getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'quote', targetTimestamp, }); @@ -842,13 +845,16 @@ async function uploadMessageQuote({ }; }), }; - setPropForTimestamp({ + const attributesToUpdate = getChangesForPropAtTimestamp({ log, - message, + message: message.attributes, prop: 'quote', targetTimestamp, value: newQuote, }); + if (attributesToUpdate) { + message.set(attributesToUpdate); + } return { isGiftBadge: loadedQuote.isGiftBadge, @@ -879,7 +885,7 @@ async function uploadMessagePreviews({ // attachments. const startingPreview = getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'preview', targetTimestamp, }); @@ -919,7 +925,7 @@ async function uploadMessagePreviews({ const logId = `uploadMessagePreviews(${message.idForLogging()}`; const oldPreview = getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'preview', targetTimestamp, }); @@ -945,13 +951,16 @@ async function uploadMessagePreviews({ }; }); - setPropForTimestamp({ + const attributesToUpdate = getChangesForPropAtTimestamp({ log, - message, + message: message.attributes, prop: 'preview', targetTimestamp, value: newPreview, }); + if (attributesToUpdate) { + message.set(attributesToUpdate); + } return uploadedPreviews; } @@ -1119,7 +1128,7 @@ function didSendToEveryone({ const sendStateByConversationId = getPropForTimestamp({ log, - message, + message: message.attributes, prop: 'sendStateByConversationId', targetTimestamp, }) || {}; diff --git a/ts/models/messages.ts b/ts/models/messages.ts index 8a6772622..0a935e595 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -155,7 +155,10 @@ import { getSenderIdentifier } from '../util/getSenderIdentifier'; import { getNotificationDataForMessage } from '../util/getNotificationDataForMessage'; import { getNotificationTextForMessage } from '../util/getNotificationTextForMessage'; import { getMessageAuthorText } from '../util/getMessageAuthorText'; -import { getPropForTimestamp, setPropForTimestamp } from '../util/editHelpers'; +import { + getPropForTimestamp, + getChangesForPropAtTimestamp, +} from '../util/editHelpers'; import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp'; /* eslint-disable more/no-then */ @@ -838,7 +841,7 @@ export class MessageModel extends window.Backbone.Model { const targetTimestamp = editMessageTimestamp || this.get('timestamp'); const sendStateByConversationId = getPropForTimestamp({ log, - message: this, + message: this.attributes, prop: 'sendStateByConversationId', targetTimestamp, }); @@ -852,13 +855,16 @@ export class MessageModel extends window.Backbone.Model { }) ); - setPropForTimestamp({ + const attributesToUpdate = getChangesForPropAtTimestamp({ log, - message: this, + message: this.attributes, prop: 'sendStateByConversationId', targetTimestamp, value: newSendStateByConversationId, }); + if (attributesToUpdate) { + this.set(attributesToUpdate); + } // We aren't trying to send this message anymore, so we'll delete these caches delete this.cachedOutgoingContactData; @@ -956,7 +962,7 @@ export class MessageModel extends window.Backbone.Model { const sendStateByConversationId = { ...(getPropForTimestamp({ log, - message: this, + message: this.attributes, prop: 'sendStateByConversationId', targetTimestamp, }) || {}), @@ -1101,7 +1107,15 @@ export class MessageModel extends window.Backbone.Model { // We may overwrite this in the `saveErrors` call below. attributesToUpdate.errors = []; - this.set(attributesToUpdate); + const additionalProps = getChangesForPropAtTimestamp({ + log, + message: this.attributes, + prop: 'sendStateByConversationId', + targetTimestamp, + value: sendStateByConversationId, + }); + + this.set({ ...attributesToUpdate, ...additionalProps }); if (saveErrors) { saveErrors(errorsToSave); } else { @@ -1109,14 +1123,6 @@ 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(), @@ -1237,7 +1243,12 @@ export class MessageModel extends window.Backbone.Model { const conv = this.getConversation()!; const sendEntries = Object.entries( - this.get('sendStateByConversationId') || {} + getPropForTimestamp({ + log, + message: this.attributes, + prop: 'sendStateByConversationId', + targetTimestamp, + }) || {} ); const sentEntries = filter(sendEntries, ([_conversationId, { status }]) => isSent(status) @@ -1292,7 +1303,12 @@ export class MessageModel extends window.Backbone.Model { ).then(async result => { let newSendStateByConversationId: undefined | SendStateByConversationId; const sendStateByConversationId = - this.get('sendStateByConversationId') || {}; + getPropForTimestamp({ + log, + message: this.attributes, + prop: 'sendStateByConversationId', + targetTimestamp, + }) || {}; const ourOldSendState = getOwn( sendStateByConversationId, ourConversation.id @@ -1310,12 +1326,20 @@ export class MessageModel extends window.Backbone.Model { } } + const attributesForUpdate = newSendStateByConversationId + ? getChangesForPropAtTimestamp({ + log, + message: this.attributes, + prop: 'sendStateByConversationId', + value: newSendStateByConversationId, + targetTimestamp, + }) + : null; + this.set({ synced: true, dataMessage: null, - ...(newSendStateByConversationId - ? { sendStateByConversationId: newSendStateByConversationId } - : {}), + ...attributesForUpdate, }); // Return early, skip the save diff --git a/ts/util/editHelpers.ts b/ts/util/editHelpers.ts index 9cd1ecb23..3b9a169ec 100644 --- a/ts/util/editHelpers.ts +++ b/ts/util/editHelpers.ts @@ -7,8 +7,6 @@ import { strictAssert } from './assert'; import type { EditHistoryType, MessageAttributesType } from '../model-types'; import type { LoggerType } from '../types/Logging'; -import { getMessageIdForLogging } from './idForLogging'; -import type { MessageModel } from '../models/messages'; // 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. @@ -16,11 +14,13 @@ export function getTargetOfThisEditTimestamp({ message, targetTimestamp, }: { - message: MessageModel; + message: MessageAttributesType; targetTimestamp: number; }): number { - const originalTimestamp = message.get('timestamp'); - const editHistory = message.get('editHistory') || []; + const { timestamp: originalTimestamp, editHistory } = message; + if (!editHistory) { + return originalTimestamp; + } const sentItems = editHistory.filter(item => { return item.timestamp <= targetTimestamp; @@ -52,18 +52,13 @@ export function getPropForTimestamp({ targetTimestamp, }: { log: LoggerType; - message: MessageModel | MessageAttributesType; + message: MessageAttributesType; prop: T; targetTimestamp: number; }): EditHistoryType[T] { - const attributes = - message instanceof window.Whisper.Message ? message.attributes : message; + const logId = `getPropForTimestamp(${targetTimestamp}})`; - const logId = `getPropForTimestamp(${getMessageIdForLogging( - attributes - )}, target=${targetTimestamp}})`; - - const { editHistory } = attributes; + const { editHistory } = message; const targetEdit = editHistory?.find( item => item.timestamp === targetTimestamp ); @@ -71,13 +66,13 @@ export function getPropForTimestamp({ if (editHistory) { log.warn(`${logId}: No edit found, using top-level data`); } - return attributes[prop]; + return message[prop]; } return targetEdit[prop]; } -export function setPropForTimestamp({ +export function getChangesForPropAtTimestamp({ log, message, prop, @@ -85,48 +80,58 @@ export function setPropForTimestamp({ value, }: { log: LoggerType; - message: MessageModel; + message: MessageAttributesType; prop: T; targetTimestamp: number; value: EditHistoryType[T]; -}): void { - const logId = `setPropForTimestamp(${message.idForLogging()}, target=${targetTimestamp}})`; +}): Partial | undefined { + const logId = `getChangesForPropAtTimestamp(${targetTimestamp})`; - const editHistory = message.get('editHistory'); - const targetEditIndex = editHistory?.findIndex( - item => item.timestamp === targetTimestamp - ); - const targetEdit = - editHistory && isNumber(targetEditIndex) + const { editHistory } = message; + let partialProps: Partial | undefined; + + if (editHistory) { + const targetEditIndex = editHistory.findIndex( + item => item.timestamp === targetTimestamp + ); + const targetEdit = isNumber(targetEditIndex) ? editHistory[targetEditIndex] : undefined; - if (!targetEdit) { - if (editHistory) { - log.warn(`${logId}: No edit found, updating top-level data`); + if (!targetEdit) { + if (editHistory) { + log.warn(`${logId}: No edit found, updating top-level data`); + } + return { + [prop]: value, + }; } - message.set({ - [prop]: value, - }); - return; + + strictAssert( + isNumber(targetEditIndex), + 'Got targetEdit, but no targetEditIndex' + ); + + const newEditHistory = [...editHistory]; + newEditHistory[targetEditIndex] = { ...targetEdit, [prop]: value }; + + partialProps = { + editHistory: newEditHistory, + }; } - 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({ + const { editMessageTimestamp } = message; + if ( + !editHistory || + !editMessageTimestamp || + editMessageTimestamp === targetTimestamp + ) { + partialProps = { + ...partialProps, [prop]: value, - }); + }; } + + return partialProps; }