From 4277c29bd87ea9c02725243e39daf1ab79ed1b4d Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Mon, 4 Oct 2021 16:13:21 +1100 Subject: [PATCH] make sure to refetch messageModel from db when saving attachments --- ts/receiver/attachments.ts | 33 +------------- ts/session/utils/AttachmentsDownload.ts | 59 +++++++++++-------------- 2 files changed, 26 insertions(+), 66 deletions(-) diff --git a/ts/receiver/attachments.ts b/ts/receiver/attachments.ts index a4b804fb0..b782bdc69 100644 --- a/ts/receiver/attachments.ts +++ b/ts/receiver/attachments.ts @@ -28,7 +28,7 @@ export async function downloadAttachment(attachment: any) { // try to get the fileId from the end of the URL attachmentId = attachment.url; } - window?.log?.info('Download v2 file server attachment'); + window?.log?.info('Download v2 file server attachment', attachmentId); res = await FSv2.downloadFileFromFSv2(attachmentId, defaultFsOldV2); } else { window.log.warn( @@ -229,32 +229,6 @@ async function processQuoteAttachments( return addedCount; } -async function processGroupAvatar( - message: MessageModel, - convo: ConversationModel -): Promise { - let group = message.get('group'); - - if (!group || !group.avatar) { - return false; - } - const isOpenGroupV2 = convo.isOpenGroupV2(); - - group = { - ...group, - avatar: await AttachmentDownloads.addJob(group.avatar, { - messageId: message.id, - type: 'group-avatar', - index: 0, - isOpenGroupV2, - }), - }; - - message.set({ group }); - - return true; -} - export async function queueAttachmentDownloads( message: MessageModel, conversation: ConversationModel @@ -267,11 +241,6 @@ export async function queueAttachmentDownloads( count += await processQuoteAttachments(message, conversation); - // I don 't think we rely on this for anything - if (await processGroupAvatar(message, conversation)) { - count += 1; - } - if (count > 0) { await saveMessage(message.attributes); } diff --git a/ts/session/utils/AttachmentsDownload.ts b/ts/session/utils/AttachmentsDownload.ts index df2c25e71..d99e0eaa6 100644 --- a/ts/session/utils/AttachmentsDownload.ts +++ b/ts/session/utils/AttachmentsDownload.ts @@ -1,4 +1,5 @@ import { isNumber, omit } from 'lodash'; +import _ from 'lodash'; // tslint:disable-next-line: no-submodule-imports import { default as getGuid } from 'uuid/v4'; import { @@ -13,12 +14,14 @@ import { import { MessageModel } from '../../models/message'; import { downloadAttachment, downloadAttachmentOpenGroupV2 } from '../../receiver/attachments'; +// this cause issues if we increment that value to > 1. const MAX_ATTACHMENT_JOB_PARALLELISM = 3; const SECOND = 1000; const MINUTE = SECOND * 60; const HOUR = MINUTE * 60; const TICK_INTERVAL = MINUTE; +// tslint:disable: function-name const RETRY_BACKOFF = { 1: SECOND * 30, @@ -110,6 +113,14 @@ async function _maybeStartJob() { return; } + const nextJobsWithoutCurrentlyRunning = _.filter( + nextJobs, + j => _activeAttachmentDownloadJobs[j.id] === undefined + ); + if (nextJobsWithoutCurrentlyRunning.length <= 0) { + return; + } + // To prevent the race condition caused by two parallel database calls, eached kicked // off because the jobCount wasn't at the max. const secondJobCount = getActiveJobCount(); @@ -118,7 +129,11 @@ async function _maybeStartJob() { return; } - const jobs = nextJobs.slice(0, Math.min(needed, nextJobs.length)); + const jobs = nextJobsWithoutCurrentlyRunning.slice( + 0, + Math.min(needed, nextJobsWithoutCurrentlyRunning.length) + ); + // tslint:disable: one-variable-per-declaration for (let i = 0, max = jobs.length; i < max; i += 1) { const job = jobs[i]; @@ -180,6 +195,8 @@ async function _runJob(job: any) { ); await _finishJob(found, id); + found = await getMessageById(messageId); + await _addAttachmentToMessage(found, _markAttachmentAsError(attachment), { type, index }); return; @@ -188,6 +205,7 @@ async function _runJob(job: any) { } const upgradedAttachment = await window.Signal.Migrations.processNewAttachment(downloaded); + found = await getMessageById(messageId); await _addAttachmentToMessage(found, upgradedAttachment, { type, index }); @@ -201,6 +219,7 @@ async function _runJob(job: any) { `_runJob: ${currentAttempt} failed attempts, marking attachment ${id} from message ${found?.idForLogging()} as permament error:`, error && error.stack ? error.stack : error ); + found = await getMessageById(messageId); await _finishJob(found || null, id); await _addAttachmentToMessage(found, _markAttachmentAsError(attachment), { type, index }); @@ -254,7 +273,11 @@ function _markAttachmentAsError(attachment: any) { } // tslint:disable-next-line: cyclomatic-complexity -async function _addAttachmentToMessage(message: any, attachment: any, { type, index }: any) { +async function _addAttachmentToMessage( + message: MessageModel | null | undefined, + attachment: any, + { type, index }: any +) { if (!message) { return; } @@ -285,23 +308,6 @@ async function _addAttachmentToMessage(message: any, attachment: any, { type, in return; } - if (type === 'contact') { - const contact = message.get('contact'); - if (!contact || contact.length <= index) { - throw new Error(`_addAttachmentToMessage: contact didn't exist or ${index} was too large`); - } - const item = contact[index]; - if (item && item.avatar && item.avatar.avatar) { - _replaceAttachment(item.avatar, 'avatar', attachment, logPrefix); - } else { - logger.warn( - `_addAttachmentToMessage: Couldn't update contact with avatar attachment for message ${message.idForLogging()}` - ); - } - - return; - } - if (type === 'quote') { const quote = message.get('quote'); if (!quote) { @@ -322,21 +328,6 @@ async function _addAttachmentToMessage(message: any, attachment: any, { type, in return; } - if (type === 'group-avatar') { - const group = message.get('group'); - if (!group) { - throw new Error("_addAttachmentToMessage: group didn't exist"); - } - - const existingAvatar = group.avatar; - if (existingAvatar && existingAvatar.path) { - await window.Signal.Migrations.deleteAttachmentData(existingAvatar.path); - } - - _replaceAttachment(group, 'avatar', attachment, logPrefix); - return; - } - throw new Error( `_addAttachmentToMessage: Unknown job type ${type} for message ${message.idForLogging()}` );