Improve handling of edited long message attachments

This commit is contained in:
trevor-signal
2025-02-25 14:23:36 -05:00
committed by GitHub
parent da2741ba31
commit 6f9438c74f
9 changed files with 357 additions and 77 deletions

View File

@@ -1,8 +1,7 @@
// Copyright 2020 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { partition } from 'lodash';
import * as logger from '../logging/log';
import * as defaultLogger from '../logging/log';
import { isLongMessage } from '../types/MIME';
import { getMessageIdForLogging } from './idForLogging';
import {
@@ -24,6 +23,7 @@ import {
getAttachmentSignatureSafe,
isDownloading,
isDownloaded,
partitionBodyAndNormalAttachments,
} from '../types/Attachment';
import type { StickerType } from '../types/Stickers';
import type { LinkPreviewType } from '../types/message/LinkPreviews';
@@ -43,6 +43,7 @@ import {
shouldUseAttachmentDownloadQueue,
} from './attachmentDownloadQueue';
import { queueUpdateMessage } from './messageBatcher';
import type { LoggerType } from '../types/Logging';
export type MessageAttachmentsDownloadedType = {
bodyAttachment?: AttachmentType;
@@ -56,7 +57,7 @@ export type MessageAttachmentsDownloadedType = {
function getLogger(source: AttachmentDownloadSource) {
const verbose = source !== AttachmentDownloadSource.BACKUP_IMPORT;
const log = verbose ? logger : { ...logger, info: () => null };
const log = verbose ? defaultLogger : { ...defaultLogger, info: () => null };
return log;
}
@@ -64,7 +65,9 @@ export async function handleAttachmentDownloadsForNewMessage(
message: MessageModel,
conversation: ConversationModel
): Promise<void> {
const idLog = `handleAttachmentDownloadsForNewMessage/${conversation.idForLogging()} ${getMessageIdForLogging(message.attributes)}`;
const logId =
`handleAttachmentDownloadsForNewMessage/${conversation.idForLogging()} ` +
`${getMessageIdForLogging(message.attributes)}`;
// Only queue attachments for downloads if this is a story (with additional logic), or
// if it's either an outgoing message or we've accepted the conversation
@@ -79,7 +82,7 @@ export async function handleAttachmentDownloadsForNewMessage(
if (shouldQueueForDownload) {
if (shouldUseAttachmentDownloadQueue()) {
addToAttachmentDownloadQueue(idLog, message);
addToAttachmentDownloadQueue(logId, message);
} else {
await queueAttachmentDownloadsForMessage(message);
}
@@ -117,28 +120,21 @@ export async function queueAttachmentDownloads(
attachmentDigestForImmediate?: string;
} = {}
): Promise<boolean> {
const attachmentsToQueue = message.get('attachments') || [];
const messageId = message.id;
const idForLogging = getMessageIdForLogging(message.attributes);
let count = 0;
const idLog = `queueAttachmentDownloads(${idForLogging}})`;
const logId = `queueAttachmentDownloads(${idForLogging}})`;
const log = getLogger(source);
const [longMessageAttachments, normalAttachments] = partition(
attachmentsToQueue,
attachment => isLongMessage(attachment.contentType)
message.set(
ensureBodyAttachmentsAreSeparated(message.attributes, {
logId,
logger: log,
})
);
if (longMessageAttachments.length > 1) {
log.error(`${idLog}: Received more than one long message attachment`);
}
if (longMessageAttachments.length > 0) {
message.set({ bodyAttachment: longMessageAttachments[0] });
}
const bodyAttachmentsToDownload = [
message.get('bodyAttachment'),
...(message
@@ -151,7 +147,7 @@ export async function queueAttachmentDownloads(
if (bodyAttachmentsToDownload.length) {
log.info(
`${idLog}: Queueing ${bodyAttachmentsToDownload.length} long message attachment download`
`${logId}: Queueing ${bodyAttachmentsToDownload.length} long message attachment download`
);
await Promise.all(
bodyAttachmentsToDownload.map(attachment =>
@@ -169,16 +165,11 @@ export async function queueAttachmentDownloads(
count += bodyAttachmentsToDownload.length;
}
if (normalAttachments.length > 0) {
log.info(
`${idLog}: Queueing ${normalAttachments.length} normal attachment downloads`
);
}
const { attachments, count: attachmentsCount } = await queueNormalAttachments(
{
idLog,
logId,
messageId,
attachments: normalAttachments,
attachments: message.get('attachments'),
otherAttachments: message
.get('editHistory')
?.flatMap(x => x.attachments ?? []),
@@ -189,19 +180,23 @@ export async function queueAttachmentDownloads(
attachmentDigestForImmediate,
}
);
if (attachmentsCount > 0) {
message.set({ attachments });
log.info(
`${logId}: Queueing ${attachmentsCount} normal attachment downloads`
);
}
count += attachmentsCount;
const previewsToQueue = message.get('preview') || [];
if (previewsToQueue.length > 0) {
log.info(
`${idLog}: Queueing ${previewsToQueue.length} preview attachment downloads`
`${logId}: Queueing ${previewsToQueue.length} preview attachment downloads`
);
}
const { preview, count: previewCount } = await queuePreviews({
idLog,
logId,
messageId,
previews: previewsToQueue,
otherPreviews: message.get('editHistory')?.flatMap(x => x.preview ?? []),
@@ -218,12 +213,12 @@ export async function queueAttachmentDownloads(
const numQuoteAttachments = message.get('quote')?.attachments?.length ?? 0;
if (numQuoteAttachments > 0) {
log.info(
`${idLog}: Queueing ${numQuoteAttachments} ` +
`${logId}: Queueing ${numQuoteAttachments} ` +
'quote attachment downloads'
);
}
const { quote, count: thumbnailCount } = await queueQuoteAttachments({
idLog,
logId,
messageId,
quote: message.get('quote'),
otherQuotes:
@@ -244,7 +239,7 @@ export async function queueAttachmentDownloads(
const contactsToQueue = message.get('contact') || [];
if (contactsToQueue.length > 0) {
log.info(
`${idLog}: Queueing ${contactsToQueue.length} contact attachment downloads`
`${logId}: Queueing ${contactsToQueue.length} contact attachment downloads`
);
}
const contact = await Promise.all(
@@ -254,7 +249,7 @@ export async function queueAttachmentDownloads(
}
// We've already downloaded this!
if (item.avatar.avatar.path) {
log.info(`${idLog}: Contact attachment already downloaded`);
log.info(`${logId}: Contact attachment already downloaded`);
return item;
}
@@ -280,9 +275,9 @@ export async function queueAttachmentDownloads(
let sticker = message.get('sticker');
if (sticker && sticker.data && sticker.data.path) {
log.info(`${idLog}: Sticker attachment already downloaded`);
log.info(`${logId}: Sticker attachment already downloaded`);
} else if (sticker) {
log.info(`${idLog}: Queueing sticker download`);
log.info(`${logId}: Queueing sticker download`);
count += 1;
const { packId, stickerId, packKey } = sticker;
@@ -294,7 +289,7 @@ export async function queueAttachmentDownloads(
data = await copyStickerToAttachments(packId, stickerId);
} catch (error) {
log.error(
`${idLog}: Problem copying sticker (${packId}, ${stickerId}) to attachments:`,
`${logId}: Problem copying sticker (${packId}, ${stickerId}) to attachments:`,
Errors.toLogFormat(error)
);
}
@@ -311,7 +306,7 @@ export async function queueAttachmentDownloads(
source,
});
} else {
log.error(`${idLog}: Sticker data was missing`);
log.error(`${logId}: Sticker data was missing`);
}
}
const stickerRef = {
@@ -341,12 +336,12 @@ export async function queueAttachmentDownloads(
let editHistory = message.get('editHistory');
if (editHistory) {
log.info(`${idLog}: Looping through ${editHistory.length} edits`);
log.info(`${logId}: Looping through ${editHistory.length} edits`);
editHistory = await Promise.all(
editHistory.map(async edit => {
const { attachments: editAttachments, count: editAttachmentsCount } =
await queueNormalAttachments({
idLog,
logId,
messageId,
attachments: edit.attachments,
otherAttachments: attachments,
@@ -358,14 +353,14 @@ export async function queueAttachmentDownloads(
count += editAttachmentsCount;
if (editAttachmentsCount !== 0) {
log.info(
`${idLog}: Queueing ${editAttachmentsCount} normal attachment ` +
`${logId}: Queueing ${editAttachmentsCount} normal attachment ` +
`downloads (edited:${edit.timestamp})`
);
}
const { preview: editPreview, count: editPreviewCount } =
await queuePreviews({
idLog,
logId,
messageId,
previews: edit.preview,
otherPreviews: preview,
@@ -377,7 +372,7 @@ export async function queueAttachmentDownloads(
count += editPreviewCount;
if (editPreviewCount !== 0) {
log.info(
`${idLog}: Queueing ${editPreviewCount} preview attachment ` +
`${logId}: Queueing ${editPreviewCount} preview attachment ` +
`downloads (edited:${edit.timestamp})`
);
}
@@ -396,13 +391,13 @@ export async function queueAttachmentDownloads(
return false;
}
log.info(`${idLog}: Queued ${count} total attachment downloads`);
log.info(`${logId}: Queued ${count} total attachment downloads`);
return true;
}
export async function queueNormalAttachments({
idLog,
logId,
messageId,
attachments = [],
otherAttachments,
@@ -412,7 +407,7 @@ export async function queueNormalAttachments({
source,
attachmentDigestForImmediate,
}: {
idLog: string;
logId: string;
messageId: string;
attachments: MessageAttributesType['attachments'];
otherAttachments: MessageAttributesType['attachments'];
@@ -446,9 +441,16 @@ export async function queueNormalAttachments({
if (!attachment) {
return attachment;
}
if (isLongMessage(attachment.contentType)) {
throw new Error(
`${logId}: queueNormalAttachments passed long-message attachment`
);
}
// We've already downloaded this!
if (isDownloaded(attachment)) {
log.info(`${idLog}: Normal attachment already downloaded`);
log.info(`${logId}: Normal attachment already downloaded`);
return attachment;
}
@@ -463,7 +465,7 @@ export async function queueNormalAttachments({
(isDownloading(existingAttachment) || isDownloaded(existingAttachment))
) {
log.info(
`${idLog}: Normal attachment already downloaded in other attachments. Replacing`
`${logId}: Normal attachment already downloaded in other attachments. Replacing`
);
// Incrementing count so that we update the message's fields downstream
count += 1;
@@ -511,7 +513,7 @@ function getLinkPreviewSignature(preview: LinkPreviewType): string | undefined {
}
async function queuePreviews({
idLog,
logId,
messageId,
previews = [],
otherPreviews,
@@ -520,7 +522,7 @@ async function queuePreviews({
urgency,
source,
}: {
idLog: string;
logId: string;
messageId: string;
previews: MessageAttributesType['preview'];
otherPreviews: MessageAttributesType['preview'];
@@ -550,7 +552,7 @@ async function queuePreviews({
}
// We've already downloaded this!
if (isDownloaded(item.image)) {
log.info(`${idLog}: Preview attachment already downloaded`);
log.info(`${logId}: Preview attachment already downloaded`);
return item;
}
const signature = getLinkPreviewSignature(item);
@@ -564,7 +566,7 @@ async function queuePreviews({
(isDownloading(existingPreview.image) ||
isDownloaded(existingPreview.image))
) {
log.info(`${idLog}: Preview already downloaded elsewhere. Replacing`);
log.info(`${logId}: Preview already downloaded elsewhere. Replacing`);
// Incrementing count so that we update the message's fields downstream
count += 1;
return existingPreview;
@@ -607,7 +609,7 @@ function getQuoteThumbnailSignature(
}
async function queueQuoteAttachments({
idLog,
logId,
messageId,
quote,
otherQuotes,
@@ -616,7 +618,7 @@ async function queueQuoteAttachments({
urgency,
source,
}: {
idLog: string;
logId: string;
messageId: string;
quote: QuotedMessageType | undefined;
otherQuotes: ReadonlyArray<QuotedMessageType>;
@@ -663,7 +665,7 @@ async function queueQuoteAttachments({
}
// We've already downloaded this!
if (isDownloaded(item.thumbnail)) {
log.info(`${idLog}: Quote attachment already downloaded`);
log.info(`${logId}: Quote attachment already downloaded`);
return item;
}
@@ -679,7 +681,7 @@ async function queueQuoteAttachments({
isDownloaded(existingThumbnail))
) {
log.info(
`${idLog}: Preview already downloaded elsewhere. Replacing`
`${logId}: Preview already downloaded elsewhere. Replacing`
);
// Incrementing count so that we update the message's fields downstream
count += 1;
@@ -708,3 +710,42 @@ async function queueQuoteAttachments({
count,
};
}
export function ensureBodyAttachmentsAreSeparated(
messageAttributes: MessageAttributesType,
{ logId, logger = defaultLogger }: { logId: string; logger?: LoggerType }
): {
bodyAttachment: AttachmentType | undefined;
attachments: Array<AttachmentType>;
editHistory: Array<EditHistoryType> | undefined;
} {
const { bodyAttachment, attachments } = partitionBodyAndNormalAttachments(
{
attachments: messageAttributes.attachments ?? [],
existingBodyAttachment: messageAttributes.bodyAttachment,
},
{ logId, logger }
);
const updatedEditHistory = messageAttributes.editHistory?.map(edit => {
return {
...edit,
...partitionBodyAndNormalAttachments(
{
attachments: edit.attachments ?? [],
existingBodyAttachment: edit.bodyAttachment,
},
{
logId: `${logId}/editHistory(${edit.timestamp})`,
logger,
}
),
};
});
return {
bodyAttachment: bodyAttachment ?? messageAttributes.bodyAttachment,
attachments,
editHistory: updatedEditHistory,
};
}