Signal/Desktop
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Desktop.git synced 2025-12-24 20:26:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

Skip queueing old attachments on non-media-enabled backup import

Browse Source 
Co-authored-by: Scott Nonnenberg <scott@signal.org>
This commit is contained in:
trevor-signal
2025-07-31 16:21:59 -04:00
committed by GitHub
parent 0b598bef96
commit a7cd27f3cf
2 changed files with 115 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
ts/jobs/AttachmentDownloadManager.ts
View File

@@ -65,6 +65,8 @@ import { formatCountForLogging } from '../logging/formatCountForLogging';
import { strictAssert } from '../util/assert';
import { updateBackupMediaDownloadProgress } from '../util/updateBackupMediaDownloadProgress';
import { HTTPError } from '../textsecure/Errors';
import { isOlderThan } from '../util/timestamp';
import { getMessageQueueTime as doGetMessageQueueTime } from '../util/getMessageQueueTime';
const log = createLogger('AttachmentDownloadManager');
@@ -103,6 +105,7 @@ type RunDownloadAttachmentJobOptions = {
isForCurrentlyVisibleMessage: boolean;
maxAttachmentSizeInKib: number;
maxTextAttachmentSizeInKib: number;
hasMediaBackups: boolean;
};
type AttachmentDownloadManagerParamsType = Omit<
@@ -122,6 +125,8 @@ type AttachmentDownloadManagerParamsType = Omit<
dependencies?: DependenciesType;
}) => Promise<JobManagerJobResultType<CoreAttachmentDownloadJobType>>;
onLowDiskSpaceBackupImport: (bytesNeeded: number) => Promise<void>;
hasMediaBackups: () => boolean;
getMessageQueueTime: () => number;
statfs: typeof statfs;
};
@@ -149,6 +154,8 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
},
});
#onLowDiskSpaceBackupImport: (bytesNeeded: number) => Promise<void>;
#getMessageQueueTime: () => number;
#hasMediaBackups: () => boolean;
#statfs: typeof statfs;
#maxAttachmentSizeInKib = getMaximumIncomingAttachmentSizeInKb(getValue);
#maxTextAttachmentSizeInKib =
@@ -204,6 +211,8 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
bytesNeeded
);
},
hasMediaBackups: () => window.Signal.Services.backups.hasMediaBackups(),
getMessageQueueTime: () => doGetMessageQueueTime(),
statfs,
};
@@ -244,6 +253,7 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
isLastAttempt,
options: {
abortSignal,
hasMediaBackups: this.#hasMediaBackups(),
isForCurrentlyVisibleMessage,
maxAttachmentSizeInKib: this.#maxAttachmentSizeInKib,
maxTextAttachmentSizeInKib: this.#maxTextAttachmentSizeInKib,
@@ -252,6 +262,8 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
},
});
this.#onLowDiskSpaceBackupImport = params.onLowDiskSpaceBackupImport;
this.#getMessageQueueTime = params.getMessageQueueTime;
this.#hasMediaBackups = params.hasMediaBackups;
this.#statfs = params.statfs;
}
@@ -272,8 +284,25 @@ export class AttachmentDownloadManager extends JobManager<CoreAttachmentDownload
const logId = `AttachmentDownloadManager/addJob(${sentAt}.${attachmentType})`;
if (attachment.error && source === AttachmentDownloadSource.BACKUP_IMPORT) {
return attachment;
if (source === AttachmentDownloadSource.BACKUP_IMPORT) {
if (attachment.error) {
return attachment;
}
// For non-media-enabled backups, we will skip queueing download for old attachments
// that cannot still be on the transit tier
if (!this.#hasMediaBackups() && !wasImportedFromLocalBackup(attachment)) {
const attachmentUploadedAt = attachment.uploadTimestamp || sentAt;
// Skip queueing download if attachment is older than twice the message queue time
// (a generous buffer that ensures we download anything that could still exist on
// the transit tier)
if (
isOlderThan(attachmentUploadedAt, this.#getMessageQueueTime() * 2)
) {
return attachment;
}
}
}
const parseResult = safeParsePartial(coreAttachmentDownloadJobSchema, {
@@ -458,6 +487,7 @@ async function runDownloadAttachmentJob({
const result = await runDownloadAttachmentJobInner({
job,
abortSignal: options.abortSignal,
hasMediaBackups: options.hasMediaBackups,
isForCurrentlyVisibleMessage:
options?.isForCurrentlyVisibleMessage ?? false,
maxAttachmentSizeInKib: options.maxAttachmentSizeInKib,
@@ -582,6 +612,7 @@ export async function runDownloadAttachmentJobInner({
isForCurrentlyVisibleMessage,
maxAttachmentSizeInKib,
maxTextAttachmentSizeInKib,
hasMediaBackups,
dependencies,
}: {
job: AttachmentDownloadJobType;
@@ -616,7 +647,6 @@ export async function runDownloadAttachmentJobInner({
`${logId}: Text attachment was ${sizeInKib}kib, max is ${maxTextAttachmentSizeInKib}kib`
);
}
const hasMediaBackups = window.Signal.Services.backups.hasMediaBackups();
const mightBeInRemoteBackup = shouldAttachmentEndUpInRemoteBackup({
attachment,
hasMediaBackups,

87
ts/test-electron/services/AttachmentDownloadManager_test.ts
View File

@@ -19,7 +19,7 @@ import {
AttachmentDownloadUrgency,
} from '../../types/AttachmentDownload';
import { DataReader, DataWriter } from '../../sql/Client';
import { MINUTE } from '../../util/durations';
import { DAY, MINUTE, MONTH } from '../../util/durations';
import { type AttachmentType, AttachmentVariant } from '../../types/Attachment';
import { strictAssert } from '../../util/assert';
import type { downloadAttachment as downloadAttachmentUtil } from '../../util/downloadAttachment';
@@ -78,6 +78,7 @@ describe('AttachmentDownloadManager/JobManager', () => {
let runJob: sinon.SinonStub;
let sandbox: sinon.SinonSandbox;
let clock: sinon.SinonFakeTimers;
let hasMediaBackups: sinon.SinonStub;
let isInCall: sinon.SinonStub;
let onLowDiskSpaceBackupImport: sinon.SinonStub;
let statfs: sinon.SinonStub;
@@ -89,6 +90,7 @@ describe('AttachmentDownloadManager/JobManager', () => {
sandbox = sinon.createSandbox();
clock = sandbox.useFakeTimers();
hasMediaBackups = sandbox.stub().returns(true);
isInCall = sandbox.stub().returns(false);
onLowDiskSpaceBackupImport = sandbox
.stub()
@@ -123,6 +125,8 @@ describe('AttachmentDownloadManager/JobManager', () => {
},
}),
onLowDiskSpaceBackupImport,
hasMediaBackups,
getMessageQueueTime: () => 45 * DAY,
statfs,
});
});
@@ -162,7 +166,8 @@ describe('AttachmentDownloadManager/JobManager', () => {
num: number,
jobOverrides?:
| Partial<AttachmentDownloadJobType>
| ((idx: number) => Partial<AttachmentDownloadJobType>)
| ((idx: number) => Partial<AttachmentDownloadJobType>),
attachmentOverrides?: Partial<AttachmentType>
): Promise<Array<AttachmentDownloadJobType>> {
const jobs = new Array(num).fill(null).map((_, idx) =>
composeJob({
@@ -170,6 +175,7 @@ describe('AttachmentDownloadManager/JobManager', () => {
receivedAt: idx,
jobOverrides:
typeof jobOverrides === 'function' ? jobOverrides(idx) : jobOverrides,
attachmentOverrides,
})
);
for (const job of jobs) {
@@ -496,6 +502,73 @@ describe('AttachmentDownloadManager/JobManager', () => {
jobs[2],
]);
});
describe('will drop jobs from non-media backup imports that are old', () => {
it('will not queue attachments older than 90 days (2 * message queue time)', async () => {
hasMediaBackups.returns(false);
await addJobs(
1,
{
source: AttachmentDownloadSource.BACKUP_IMPORT,
},
{ uploadTimestamp: Date.now() - 4 * MONTH }
);
const savedJobs = await DataWriter.getNextAttachmentDownloadJobs({
limit: 100,
});
assert.strictEqual(savedJobs.length, 0);
});
it('will queue old attachments with media backups on', async () => {
hasMediaBackups.returns(true);
await addJobs(
1,
{
source: AttachmentDownloadSource.BACKUP_IMPORT,
},
{ uploadTimestamp: Date.now() - 4 * MONTH }
);
const savedJobs = await DataWriter.getNextAttachmentDownloadJobs({
limit: 100,
});
assert.strictEqual(savedJobs.length, 1);
});
it('will queue old local backup attachments', async () => {
hasMediaBackups.returns(false);
await addJobs(
1,
{
source: AttachmentDownloadSource.BACKUP_IMPORT,
},
{
uploadTimestamp: Date.now() - 4 * MONTH,
localBackupPath: 'localBackupPath',
localKey: toBase64(generateAttachmentKeys()),
}
);
const savedJobs = await DataWriter.getNextAttachmentDownloadJobs({
limit: 100,
});
assert.strictEqual(savedJobs.length, 1);
});
it('will fallback to sentAt if uploadTimestamp is falsy', async () => {
hasMediaBackups.returns(false);
await addJobs(
1,
{
source: AttachmentDownloadSource.BACKUP_IMPORT,
sentAt: Date.now() - 4 * MONTH,
},
{ uploadTimestamp: 0 }
);
const savedJobs = await DataWriter.getNextAttachmentDownloadJobs({
limit: 100,
});
assert.strictEqual(savedJobs.length, 0);
});
});
});
describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
@@ -521,9 +594,6 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
downloadAttachment = sandbox
.stub()
.returns(Promise.resolve(downloadedAttachment));
sandbox
.stub(window.Signal.Services.backups, 'hasMediaBackups')
.returns(true);
processNewAttachment = sandbox.stub().callsFake(attachment => attachment);
});
@@ -544,6 +614,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
const result = await runDownloadAttachmentJobInner({
job,
isForCurrentlyVisibleMessage: true,
hasMediaBackups: true,
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -573,6 +644,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
const result = await runDownloadAttachmentJobInner({
job,
isForCurrentlyVisibleMessage: true,
hasMediaBackups: true,
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -620,6 +692,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
const result = await runDownloadAttachmentJobInner({
job,
isForCurrentlyVisibleMessage: true,
hasMediaBackups: true,
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -654,6 +727,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
runDownloadAttachmentJobInner({
job,
isForCurrentlyVisibleMessage: true,
hasMediaBackups: true,
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -692,6 +766,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
const result = await runDownloadAttachmentJobInner({
job,
isForCurrentlyVisibleMessage: false,
hasMediaBackups: true,
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -727,6 +802,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
const result = await runDownloadAttachmentJobInner({
job,
isForCurrentlyVisibleMessage: false,
hasMediaBackups: true,
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
@@ -781,6 +857,7 @@ describe('AttachmentDownloadManager/runDownloadAttachmentJob', () => {
runDownloadAttachmentJobInner({
job,
isForCurrentlyVisibleMessage: false,
hasMediaBackups: true,
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,