On send, pull data from target edit if sending edit

This commit is contained in:
Scott Nonnenberg 2023-11-17 10:16:48 -08:00 committed by GitHub
parent 146b562c91
commit 48245eeea6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 529 additions and 135 deletions

View file

@ -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<Error> = [];
// 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<CallbackResultType | void>;
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<ServiceIdString>;
recipientServiceIdsWithoutMe: Array<ServiceIdString>;
@ -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<UploadedAttachmentType>;
body: undefined | string;
@ -493,7 +567,6 @@ async function getMessageSendData({
deletedForEveryoneTimestamp: undefined | number;
expireTimer: undefined | DurationInSeconds;
bodyRanges: undefined | ReadonlyArray<RawBodyRange>;
messageTimestamp: number;
preview: Array<OutgoingLinkPreviewType> | 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<UploadedAttachmentType> {
async function uploadSingleAttachment({
attachment,
log,
message,
targetTimestamp,
}: {
attachment: AttachmentType;
log: LoggerType;
message: MessageModel;
targetTimestamp: number;
}): Promise<UploadedAttachmentType> {
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<OutgoingQuoteType | undefined> {
async function uploadMessageQuote({
log,
message,
targetTimestamp,
uploadQueue,
}: {
log: LoggerType;
message: MessageModel;
targetTimestamp: number;
uploadQueue: PQueue;
}): Promise<OutgoingQuoteType | undefined> {
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<Array<OutgoingLinkPreviewType> | undefined> {
async function uploadMessagePreviews({
log,
message,
targetTimestamp,
uploadQueue,
}: {
log: LoggerType;
message: MessageModel;
targetTimestamp: number;
uploadQueue: PQueue;
}): Promise<Array<OutgoingLinkPreviewType> | 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<Error>
): Promise<void> {
message.markFailed();
async function markMessageFailed({
errors,
message,
targetTimestamp,
}: {
errors: Array<Error>;
message: MessageModel;
targetTimestamp: number;
}): Promise<void> {
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<MessageModel>): 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)
);

View file

@ -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

View file

@ -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

2
ts/model-types.d.ts vendored
View file

@ -113,6 +113,8 @@ export type MessageReactionType = {
isSentByConversationId?: Record<string, boolean>;
};
// 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<AttachmentType>;
body?: string;

View file

@ -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<MessageAttributesType> {
* 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<MessageAttributesType> {
return errors[0][0];
}
async send(
promise: Promise<CallbackResultType | void | null>,
saveErrors?: (errors: Array<Error>) => void
): Promise<void> {
async send({
promise,
saveErrors,
targetTimestamp,
}: {
promise: Promise<CallbackResultType | void | null>;
saveErrors?: (errors: Array<Error>) => void;
targetTimestamp: number;
}): Promise<void> {
const updateLeftPane =
this.getConversation()?.debouncedUpdateLastMessage ?? noop;
@ -926,7 +957,12 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
}
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<MessageAttributesType> {
});
// 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<MessageAttributesType> {
}
});
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<MessageAttributesType> {
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<MessageAttributesType> {
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<MessageAttributesType> {
updateLeftPane();
}
async sendSyncMessageOnly(
dataMessage: Uint8Array,
saveErrors?: (errors: Array<Error>) => void
): Promise<CallbackResultType | void> {
async sendSyncMessageOnly({
targetTimestamp,
dataMessage,
saveErrors,
}: {
targetTimestamp: number;
dataMessage: Uint8Array;
saveErrors?: (errors: Array<Error>) => void;
}): Promise<CallbackResultType | void> {
// 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<MessageAttributesType> {
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<MessageAttributesType> {
}
}
async sendSyncMessage(): Promise<CallbackResultType | void> {
async sendSyncMessage(
targetTimestamp: number
): Promise<CallbackResultType | void> {
const ourConversation =
window.ConversationController.getOurConversationOrThrow();
const sendOptions = await getSendOptions(ourConversation.attributes, {
@ -1173,8 +1228,8 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
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<MessageAttributesType> {
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<MessageAttributesType> {
return handleMessageSend(
messaging.sendSyncMessage({
...encodedContent,
timestamp,
timestamp: targetTimestamp,
destination: conv.get('e164'),
destinationServiceId: conv.getServiceId(),
expirationStartTimestamp:

View file

@ -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);

View file

@ -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(

View file

@ -164,7 +164,6 @@ export type MessageOptionsType = {
body?: string;
bodyRanges?: ReadonlyArray<RawBodyRange>;
contact?: ReadonlyArray<EmbeddedContactWithUploadedAvatar>;
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<RawBodyRange>;
contact?: ReadonlyArray<EmbeddedContactWithUploadedAvatar>;
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<EmbeddedContactWithUploadedAvatar>;
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,

View file

@ -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) {

143
ts/util/editHelpers.ts Normal file
View file

@ -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<T extends keyof EditHistoryType>({
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<T extends keyof EditHistoryType>({
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,
});
}
}

View file

@ -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);

View file

@ -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;