mirrors/signal-desktop
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 2 Wiki Activity Actions

Simplify edit-related send functionality

Browse source
This commit is contained in:
Scott Nonnenberg 2023-12-15 17:43:31 -08:00 • committed by GitHub
parent 183619ad90
commit 0918b3da7f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 121 additions and 83 deletions
Download patch file Download diff file Expand all files Collapse all files

45
ts/jobs/helpers/sendNormalMessage.ts
View file

@ -56,7 +56,7 @@ import * as Bytes from '../../Bytes';
import { import {
getPropForTimestamp, getPropForTimestamp,
getTargetOfThisEditTimestamp, getTargetOfThisEditTimestamp,
setPropForTimestamp, getChangesForPropAtTimestamp,
} from '../../util/editHelpers'; } from '../../util/editHelpers';
import { getMessageSentTimestamp } from '../../util/getMessageSentTimestamp'; 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 // The timestamp identifying the target of this edit; could be the original timestamp
// or the most recent edit prior to this one // or the most recent edit prior to this one
const targetOfThisEditTimestamp = getTargetOfThisEditTimestamp({ const targetOfThisEditTimestamp = getTargetOfThisEditTimestamp({
message, message: message.attributes,
targetTimestamp, targetTimestamp,
}); });
@ -486,7 +486,7 @@ function getMessageRecipients({
const sendStateByConversationId = const sendStateByConversationId =
getPropForTimestamp({ getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'sendStateByConversationId', prop: 'sendStateByConversationId',
targetTimestamp, targetTimestamp,
}) || {}; }) || {};
@ -579,7 +579,7 @@ async function getMessageSendData({
// Figure out if we need to upload message body as an attachment. // Figure out if we need to upload message body as an attachment.
let body = getPropForTimestamp({ let body = getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'body', prop: 'body',
targetTimestamp, targetTimestamp,
}); });
@ -603,7 +603,7 @@ async function getMessageSendData({
const preUploadAttachments = const preUploadAttachments =
getPropForTimestamp({ getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'attachments', prop: 'attachments',
targetTimestamp, targetTimestamp,
}) || []; }) || [];
@ -677,7 +677,7 @@ async function getMessageSendData({
expireTimer: message.get('expireTimer'), expireTimer: message.get('expireTimer'),
bodyRanges: getPropForTimestamp({ bodyRanges: getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'bodyRanges', prop: 'bodyRanges',
targetTimestamp, targetTimestamp,
}), }),
@ -720,7 +720,7 @@ async function uploadSingleAttachment({
const logId = `uploadSingleAttachment(${message.idForLogging()}`; const logId = `uploadSingleAttachment(${message.idForLogging()}`;
const oldAttachments = getPropForTimestamp({ const oldAttachments = getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'attachments', prop: 'attachments',
targetTimestamp, targetTimestamp,
}); });
@ -742,13 +742,16 @@ async function uploadSingleAttachment({
...copyCdnFields(uploaded), ...copyCdnFields(uploaded),
}; };
setPropForTimestamp({ const attributesToUpdate = getChangesForPropAtTimestamp({
log, log,
message, message: message.attributes,
prop: 'attachments', prop: 'attachments',
targetTimestamp, targetTimestamp,
value: newAttachments, value: newAttachments,
}); });
if (attributesToUpdate) {
message.set(attributesToUpdate);
}
return uploaded; return uploaded;
} }
@ -772,7 +775,7 @@ async function uploadMessageQuote({
// sends are failing, let's not add the complication of a cache. // sends are failing, let's not add the complication of a cache.
const startingQuote = getPropForTimestamp({ const startingQuote = getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'quote', prop: 'quote',
targetTimestamp, targetTimestamp,
}); });
@ -808,7 +811,7 @@ async function uploadMessageQuote({
const logId = `uploadMessageQuote(${message.idForLogging()}`; const logId = `uploadMessageQuote(${message.idForLogging()}`;
const oldQuote = getPropForTimestamp({ const oldQuote = getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'quote', prop: 'quote',
targetTimestamp, targetTimestamp,
}); });
@ -842,13 +845,16 @@ async function uploadMessageQuote({
}; };
}), }),
}; };
setPropForTimestamp({ const attributesToUpdate = getChangesForPropAtTimestamp({
log, log,
message, message: message.attributes,
prop: 'quote', prop: 'quote',
targetTimestamp, targetTimestamp,
value: newQuote, value: newQuote,
}); });
if (attributesToUpdate) {
message.set(attributesToUpdate);
}
return { return {
isGiftBadge: loadedQuote.isGiftBadge, isGiftBadge: loadedQuote.isGiftBadge,
@ -879,7 +885,7 @@ async function uploadMessagePreviews({
// attachments. // attachments.
const startingPreview = getPropForTimestamp({ const startingPreview = getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'preview', prop: 'preview',
targetTimestamp, targetTimestamp,
}); });
@ -919,7 +925,7 @@ async function uploadMessagePreviews({
const logId = `uploadMessagePreviews(${message.idForLogging()}`; const logId = `uploadMessagePreviews(${message.idForLogging()}`;
const oldPreview = getPropForTimestamp({ const oldPreview = getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'preview', prop: 'preview',
targetTimestamp, targetTimestamp,
}); });
@ -945,13 +951,16 @@ async function uploadMessagePreviews({
}; };
}); });
setPropForTimestamp({ const attributesToUpdate = getChangesForPropAtTimestamp({
log, log,
message, message: message.attributes,
prop: 'preview', prop: 'preview',
targetTimestamp, targetTimestamp,
value: newPreview, value: newPreview,
}); });
if (attributesToUpdate) {
message.set(attributesToUpdate);
}
return uploadedPreviews; return uploadedPreviews;
} }
@ -1119,7 +1128,7 @@ function didSendToEveryone({
const sendStateByConversationId = const sendStateByConversationId =
getPropForTimestamp({ getPropForTimestamp({
log, log,
message, message: message.attributes,
prop: 'sendStateByConversationId', prop: 'sendStateByConversationId',
targetTimestamp, targetTimestamp,
}) || {}; }) || {};

62
ts/models/messages.ts
View file

@ -155,7 +155,10 @@ import { getSenderIdentifier } from '../util/getSenderIdentifier';
import { getNotificationDataForMessage } from '../util/getNotificationDataForMessage'; import { getNotificationDataForMessage } from '../util/getNotificationDataForMessage';
import { getNotificationTextForMessage } from '../util/getNotificationTextForMessage'; import { getNotificationTextForMessage } from '../util/getNotificationTextForMessage';
import { getMessageAuthorText } from '../util/getMessageAuthorText'; import { getMessageAuthorText } from '../util/getMessageAuthorText';
import { getPropForTimestamp, setPropForTimestamp } from '../util/editHelpers'; import {
getPropForTimestamp,
getChangesForPropAtTimestamp,
} from '../util/editHelpers';
import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp'; import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp';
/* eslint-disable more/no-then */ /* eslint-disable more/no-then */
@ -838,7 +841,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
const targetTimestamp = editMessageTimestamp || this.get('timestamp'); const targetTimestamp = editMessageTimestamp || this.get('timestamp');
const sendStateByConversationId = getPropForTimestamp({ const sendStateByConversationId = getPropForTimestamp({
log, log,
message: this, message: this.attributes,
prop: 'sendStateByConversationId', prop: 'sendStateByConversationId',
targetTimestamp, targetTimestamp,
}); });
@ -852,13 +855,16 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
}) })
); );
setPropForTimestamp({ const attributesToUpdate = getChangesForPropAtTimestamp({
log, log,
message: this, message: this.attributes,
prop: 'sendStateByConversationId', prop: 'sendStateByConversationId',
targetTimestamp, targetTimestamp,
value: newSendStateByConversationId, value: newSendStateByConversationId,
}); });
if (attributesToUpdate) {
this.set(attributesToUpdate);
}
// We aren't trying to send this message anymore, so we'll delete these caches // We aren't trying to send this message anymore, so we'll delete these caches
delete this.cachedOutgoingContactData; delete this.cachedOutgoingContactData;
@ -956,7 +962,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
const sendStateByConversationId = { const sendStateByConversationId = {
...(getPropForTimestamp({ ...(getPropForTimestamp({
log, log,
message: this, message: this.attributes,
prop: 'sendStateByConversationId', prop: 'sendStateByConversationId',
targetTimestamp, targetTimestamp,
}) || {}), }) || {}),
@ -1101,7 +1107,15 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
// We may overwrite this in the `saveErrors` call below. // We may overwrite this in the `saveErrors` call below.
attributesToUpdate.errors = []; attributesToUpdate.errors = [];
this.set(attributesToUpdate); const additionalProps = getChangesForPropAtTimestamp({
log,
message: this.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
value: sendStateByConversationId,
});
this.set({ ...attributesToUpdate, ...additionalProps });
if (saveErrors) { if (saveErrors) {
saveErrors(errorsToSave); saveErrors(errorsToSave);
} else { } else {
@ -1109,14 +1123,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
void this.saveErrors(errorsToSave, { skipSave: true }); void this.saveErrors(errorsToSave, { skipSave: true });
} }
setPropForTimestamp({
log,
message: this,
prop: 'sendStateByConversationId',
targetTimestamp,
value: sendStateByConversationId,
});
if (!this.doNotSave) { if (!this.doNotSave) {
await window.Signal.Data.saveMessage(this.attributes, { await window.Signal.Data.saveMessage(this.attributes, {
ourAci: window.textsecure.storage.user.getCheckedAci(), ourAci: window.textsecure.storage.user.getCheckedAci(),
@ -1237,7 +1243,12 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
const conv = this.getConversation()!; const conv = this.getConversation()!;
const sendEntries = Object.entries( const sendEntries = Object.entries(
this.get('sendStateByConversationId') || {} getPropForTimestamp({
log,
message: this.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
}) || {}
); );
const sentEntries = filter(sendEntries, ([_conversationId, { status }]) => const sentEntries = filter(sendEntries, ([_conversationId, { status }]) =>
isSent(status) isSent(status)
@ -1292,7 +1303,12 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
).then(async result => { ).then(async result => {
let newSendStateByConversationId: undefined | SendStateByConversationId; let newSendStateByConversationId: undefined | SendStateByConversationId;
const sendStateByConversationId = const sendStateByConversationId =
this.get('sendStateByConversationId') || {}; getPropForTimestamp({
log,
message: this.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
}) || {};
const ourOldSendState = getOwn( const ourOldSendState = getOwn(
sendStateByConversationId, sendStateByConversationId,
ourConversation.id ourConversation.id
@ -1310,12 +1326,20 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
} }
} }
const attributesForUpdate = newSendStateByConversationId
? getChangesForPropAtTimestamp({
log,
message: this.attributes,
prop: 'sendStateByConversationId',
value: newSendStateByConversationId,
targetTimestamp,
})
: null;
this.set({ this.set({
synced: true, synced: true,
dataMessage: null, dataMessage: null,
...(newSendStateByConversationId ...attributesForUpdate,
? { sendStateByConversationId: newSendStateByConversationId }
: {}),
}); });
// Return early, skip the save // Return early, skip the save

97
ts/util/editHelpers.ts
View file

@ -7,8 +7,6 @@ import { strictAssert } from './assert';
import type { EditHistoryType, MessageAttributesType } from '../model-types'; import type { EditHistoryType, MessageAttributesType } from '../model-types';
import type { LoggerType } from '../types/Logging'; 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 // 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. // edit, we're still sending that edit.
@ -16,11 +14,13 @@ export function getTargetOfThisEditTimestamp({
message, message,
targetTimestamp, targetTimestamp,
}: { }: {
message: MessageModel; message: MessageAttributesType;
targetTimestamp: number; targetTimestamp: number;
}): number { }): number {
const originalTimestamp = message.get('timestamp'); const { timestamp: originalTimestamp, editHistory } = message;
const editHistory = message.get('editHistory') || []; if (!editHistory) {
return originalTimestamp;
}
const sentItems = editHistory.filter(item => { const sentItems = editHistory.filter(item => {
return item.timestamp <= targetTimestamp; return item.timestamp <= targetTimestamp;
@ -52,18 +52,13 @@ export function getPropForTimestamp<T extends keyof EditHistoryType>({
targetTimestamp, targetTimestamp,
}: { }: {
log: LoggerType; log: LoggerType;
message: MessageModel | MessageAttributesType; message: MessageAttributesType;
prop: T; prop: T;
targetTimestamp: number; targetTimestamp: number;
}): EditHistoryType[T] { }): EditHistoryType[T] {
const attributes = const logId = `getPropForTimestamp(${targetTimestamp}})`;
message instanceof window.Whisper.Message ? message.attributes : message;
const logId = `getPropForTimestamp(${getMessageIdForLogging( const { editHistory } = message;
attributes
)}, target=${targetTimestamp}})`;
const { editHistory } = attributes;
const targetEdit = editHistory?.find( const targetEdit = editHistory?.find(
item => item.timestamp === targetTimestamp item => item.timestamp === targetTimestamp
); );
@ -71,13 +66,13 @@ export function getPropForTimestamp<T extends keyof EditHistoryType>({
if (editHistory) { if (editHistory) {
log.warn(`${logId}: No edit found, using top-level data`); log.warn(`${logId}: No edit found, using top-level data`);
} }
return attributes[prop]; return message[prop];
} }
return targetEdit[prop]; return targetEdit[prop];
} }
export function setPropForTimestamp<T extends keyof EditHistoryType>({ export function getChangesForPropAtTimestamp<T extends keyof EditHistoryType>({
log, log,
message, message,
prop, prop,
@ -85,48 +80,58 @@ export function setPropForTimestamp<T extends keyof EditHistoryType>({
value, value,
}: { }: {
log: LoggerType; log: LoggerType;
message: MessageModel; message: MessageAttributesType;
prop: T; prop: T;
targetTimestamp: number; targetTimestamp: number;
value: EditHistoryType[T]; value: EditHistoryType[T];
}): void { }): Partial<MessageAttributesType> | undefined {
const logId = `setPropForTimestamp(${message.idForLogging()}, target=${targetTimestamp}})`; const logId = `getChangesForPropAtTimestamp(${targetTimestamp})`;
const editHistory = message.get('editHistory'); const { editHistory } = message;
const targetEditIndex = editHistory?.findIndex( let partialProps: Partial<MessageAttributesType> | undefined;
item => item.timestamp === targetTimestamp
); if (editHistory) {
const targetEdit = const targetEditIndex = editHistory.findIndex(
editHistory && isNumber(targetEditIndex) item => item.timestamp === targetTimestamp
);
const targetEdit = isNumber(targetEditIndex)
? editHistory[targetEditIndex] ? editHistory[targetEditIndex]
: undefined; : undefined;
if (!targetEdit) { if (!targetEdit) {
if (editHistory) { if (editHistory) {
log.warn(`${logId}: No edit found, updating top-level data`); log.warn(`${logId}: No edit found, updating top-level data`);
}
return {
[prop]: value,
};
} }
message.set({
[prop]: value, strictAssert(
}); isNumber(targetEditIndex),
return; '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 // We always edit the top-level attribute if this is the most recent send
const editMessageTimestamp = message.get('editMessageTimestamp'); const { editMessageTimestamp } = message;
if (!editMessageTimestamp || editMessageTimestamp === targetTimestamp) { if (
message.set({ !editHistory ||
!editMessageTimestamp ||
editMessageTimestamp === targetTimestamp
) {
partialProps = {
...partialProps,
[prop]: value, [prop]: value,
}); };
} }
return partialProps;
} }