From bf8e04741d9b9ac1939d4d7791f0d5975458b60a Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 1 Nov 2026 18:46:58 +0100 Subject: [PATCH] test(storage): follow the release queue in the tests that pinned an immediate delete Replacing a cover or an avatar and deleting a note file now hand the old bytes to the release queue instead of deleting them in the request; the integration tests check the queue row and that the bytes are still there. Release mocks answer with a count, the reclaim e2e sends its refused bodies one after another, and a stale-count reload that fails is told to the admin instead of swallowed. --- .../Admin/storage/useStorageAdmin.ts | 9 +++++- server/tests/e2e/storage-reclaim.e2e.test.ts | 29 ++++++++++++------- server/tests/integration/collab.test.ts | 18 +++++++++--- server/tests/integration/profile.test.ts | 9 ++++-- server/tests/integration/trips.test.ts | 7 +++-- .../tests/unit/nest/backup.controller.test.ts | 2 +- server/tests/unit/nest/collab.service.test.ts | 4 +-- server/tests/unit/nest/files.service.test.ts | 2 +- .../unit/nest/user-cleanup.service.test.ts | 2 +- 9 files changed, 56 insertions(+), 26 deletions(-) diff --git a/client/src/components/Admin/storage/useStorageAdmin.ts b/client/src/components/Admin/storage/useStorageAdmin.ts index 649e582e9..42bdf0452 100644 --- a/client/src/components/Admin/storage/useStorageAdmin.ts +++ b/client/src/components/Admin/storage/useStorageAdmin.ts @@ -432,7 +432,14 @@ export function useStorageAdmin(genericError: string, conflictError: string): St } catch (err: unknown) { const failure = reclaimFailureOf(err, genericError) // The count moved on since it was shown: show the current one before the admin confirms again. - if ('code' in failure && failure.code === 'reclaim_confirm_stale') void refreshState().catch(() => {}) + if ('code' in failure && failure.code === 'reclaim_confirm_stale') { + try { + await refreshState() + } catch (reloadErr: unknown) { + // Without the new figure there is nothing to confirm against: say why instead. + return reclaimFailureOf(reloadErr, genericError) + } + } return failure } }, diff --git a/server/tests/e2e/storage-reclaim.e2e.test.ts b/server/tests/e2e/storage-reclaim.e2e.test.ts index 7d899bc3c..053d77cd3 100644 --- a/server/tests/e2e/storage-reclaim.e2e.test.ts +++ b/server/tests/e2e/storage-reclaim.e2e.test.ts @@ -117,22 +117,29 @@ describe('Storage reclaim e2e (whole app: real auth + admin guard + managed guar }); it('RCLE-004: PUT and confirm refuse a body outside the contract', async () => { - const refused = await Promise.all( - [{}, { deleteEnabled: 'yes' }, { deleteEnabled: true, extra: 1 }].map((body) => - request(server).put('/api/admin/storage/reclaim').set('Cookie', adminCookie).send(body), - ), - ); - expect(refused.map((res) => res.status)).toEqual([400, 400, 400]); - const confirm = await Promise.all( - [ + // One request after the other: a burst of parallel requests on one supertest server resets connections. + const statusesOf = (method: 'put' | 'post', url: string, bodies: object[]) => + bodies.reduce>(async (previous, body) => { + const done = await previous; + const res = await request(server)[method](url).set('Cookie', adminCookie).send(body); + return [...done, res.status]; + }, Promise.resolve([])); + expect( + await statusesOf('put', '/api/admin/storage/reclaim', [ + {}, + { deleteEnabled: 'yes' }, + { deleteEnabled: true, extra: 1 }, + ]), + ).toEqual([400, 400, 400]); + expect( + await statusesOf('post', '/api/admin/storage/reclaim/confirm', [ {}, { category: 'nowhere', unreferenced: 3 }, { category: 'covers', count: 3 }, { category: 'covers' }, { category: 'covers', unreferenced: -1 }, - ].map((body) => request(server).post('/api/admin/storage/reclaim/confirm').set('Cookie', adminCookie).send(body)), - ); - expect(confirm.map((res) => res.status)).toEqual([400, 400, 400, 400, 400]); + ]), + ).toEqual([400, 400, 400, 400, 400]); expect(await countRows(orm, AuditLog)).toBe(0); }); diff --git a/server/tests/integration/collab.test.ts b/server/tests/integration/collab.test.ts index 151046e02..18c1750e1 100644 --- a/server/tests/integration/collab.test.ts +++ b/server/tests/integration/collab.test.ts @@ -7,10 +7,13 @@ */ import { buildApp } from '../../src/bootstrap'; import { db as testDb } from '../../src/db/database'; +import { StorageReleaseQueue } from '../../src/db/entities/StorageReleaseQueue.entity'; import { CollabService } from '../../src/nest/collab/collab.service'; import { authCookie, generateToken } from '../helpers/auth'; import { createUser, createTrip, addTripMember } from '../helpers/factories'; +import { findRow } from '../helpers/factories/rows'; import { resetTestDb, resetRateLimits } from '../helpers/test-db'; +import { MikroORM } from '@mikro-orm/core'; import type { INestApplication } from '@nestjs/common'; import type { Application } from 'express'; @@ -254,7 +257,7 @@ describe('Collab notes', () => { expect(fs.existsSync(path.join(uploadsDir, note.attachments[0].filename))).toBe(true); }); - it('COLLAB-P02 — deleting a note file removes its bytes from uploads/files', async () => { + it('COLLAB-P02 — deleting a note file hands its bytes to the release queue', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); @@ -276,10 +279,14 @@ describe('Collab notes', () => { .delete(`/api/trips/${trip.id}/collab/notes/${noteId}/files/${fileId}`) .set('Cookie', authCookie(user.id)); expect(del.status).toBe(200); - expect(fs.existsSync(path.join(uploadsDir, diskName))).toBe(false); + // The bytes stay until the release job, a day later, finds nothing naming them. + expect(fs.existsSync(path.join(uploadsDir, diskName))).toBe(true); + expect( + await findRow(nestApp.get(MikroORM), StorageReleaseQueue, { category: 'files', key: diskName }), + ).not.toBeNull(); }); - it('COLLAB-P03 — deleting a note removes its attachments’ bytes from uploads/files', async () => { + it('COLLAB-P03 — deleting a note hands its attachments’ bytes to the release queue', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); @@ -300,7 +307,10 @@ describe('Collab notes', () => { .delete(`/api/trips/${trip.id}/collab/notes/${noteId}`) .set('Cookie', authCookie(user.id)); expect(del.status).toBe(200); - expect(fs.existsSync(path.join(uploadsDir, diskName))).toBe(false); + expect(fs.existsSync(path.join(uploadsDir, diskName))).toBe(true); + expect( + await findRow(nestApp.get(MikroORM), StorageReleaseQueue, { category: 'files', key: diskName }), + ).not.toBeNull(); }); it('COLLAB-028 — uploaded note file URL uses authenticated download path, not /uploads/', async () => { diff --git a/server/tests/integration/profile.test.ts b/server/tests/integration/profile.test.ts index 66b4e7126..41d6208e4 100644 --- a/server/tests/integration/profile.test.ts +++ b/server/tests/integration/profile.test.ts @@ -4,10 +4,11 @@ */ import { buildApp } from '../../src/bootstrap'; import { db as testDb } from '../../src/db/database'; +import { StorageReleaseQueue } from '../../src/db/entities/StorageReleaseQueue.entity'; import { Users } from '../../src/db/entities/Users.entity'; import { authCookie } from '../helpers/auth'; import { createUser, createAdmin, createTrip } from '../helpers/factories'; -import { insertRow } from '../helpers/factories/rows'; +import { findRow, insertRow } from '../helpers/factories/rows'; import { readUser } from '../helpers/factories/users'; import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { MikroORM } from '@mikro-orm/core'; @@ -149,7 +150,7 @@ describe('Avatar', () => { expect(first).toMatch(/^[0-9a-f-]{36}\.jpg$/); expect(fsMod.existsSync(pathMod.join(avatarsDir, first))).toBe(true); - // Re-upload removes the previous file (saveAvatar's cleanup branch). + // Re-upload hands the previous file to the release queue (saveAvatar's cleanup branch). const second = await request(app) .post('/api/auth/avatar') .set('Cookie', authCookie(user.id)) @@ -157,7 +158,9 @@ describe('Avatar', () => { expect(second.status).toBe(200); const secondName = pathMod.basename(second.body.avatar_url); expect(fsMod.existsSync(pathMod.join(avatarsDir, secondName))).toBe(true); - expect(fsMod.existsSync(pathMod.join(avatarsDir, first))).toBe(false); + // The old bytes stay until the release job, a day later, finds nothing naming them. + expect(fsMod.existsSync(pathMod.join(avatarsDir, first))).toBe(true); + expect(await findRow(orm, StorageReleaseQueue, { category: 'avatars', key: first })).not.toBeNull(); }); it('PROFILE-003 — uploading non-image (PDF) is rejected', async () => { diff --git a/server/tests/integration/trips.test.ts b/server/tests/integration/trips.test.ts index fd2ff73f7..52cdd6499 100644 --- a/server/tests/integration/trips.test.ts +++ b/server/tests/integration/trips.test.ts @@ -14,6 +14,7 @@ import { Days } from '../../src/db/entities/Days.entity'; import { PackingItems } from '../../src/db/entities/PackingItems.entity'; import { Places } from '../../src/db/entities/Places.entity'; import { Reservations } from '../../src/db/entities/Reservations.entity'; +import { StorageReleaseQueue } from '../../src/db/entities/StorageReleaseQueue.entity'; import { Tags } from '../../src/db/entities/Tags.entity'; import { TodoItems } from '../../src/db/entities/TodoItems.entity'; import { TripMembers } from '../../src/db/entities/TripMembers.entity'; @@ -1301,7 +1302,7 @@ describe('Trip cover upload parity', () => { expect(fsMod.existsSync(pathMod.join(coversDir, diskName))).toBe(true); }); - it('TRIP-P02 — re-upload deletes the previous cover file', async () => { + it('TRIP-P02 — re-upload hands the previous cover to the release queue, which removes it once no undo can need it', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); @@ -1317,7 +1318,9 @@ describe('Trip cover upload parity', () => { .set('Cookie', authCookie(user.id)) .attach('cover', FIXTURE_IMG, 'two.jpg'); expect(second.status).toBe(201); - expect(fsMod.existsSync(pathMod.join(coversDir, firstName))).toBe(false); + // Not in the request any more: the bytes stay until the release job, a day later, finds nothing naming them. + expect(fsMod.existsSync(pathMod.join(coversDir, firstName))).toBe(true); + expect(await findRow(orm, StorageReleaseQueue, { category: 'covers', key: firstName })).not.toBeNull(); }); it('TRIP-P03 — no file → 400 "No image uploaded"', async () => { diff --git a/server/tests/unit/nest/backup.controller.test.ts b/server/tests/unit/nest/backup.controller.test.ts index 6488eeee4..facbf60c3 100644 --- a/server/tests/unit/nest/backup.controller.test.ts +++ b/server/tests/unit/nest/backup.controller.test.ts @@ -453,7 +453,7 @@ describe('BackupService (wrapper)', () => { let runningInside = false; vi.mocked(backupSvc.createBackup).mockImplementationOnce(async () => { runningInside = jobs.backupRunning; - return { filename: 'svc.zip', size: 5 }; + return { filename: 'svc.zip', size: 5, sizeText: '5 B', created_at: '2026-11-01T00:00:00.000Z' }; }); await wrapper.createBackup(); expect(runningInside).toBe(true); diff --git a/server/tests/unit/nest/collab.service.test.ts b/server/tests/unit/nest/collab.service.test.ts index 44a3f8d55..8ccddd411 100644 --- a/server/tests/unit/nest/collab.service.test.ts +++ b/server/tests/unit/nest/collab.service.test.ts @@ -996,7 +996,7 @@ describe('hardening', () => { collabRelease.releaseValues.mockClear(); collabRelease.releaseValues.mockImplementationOnce(async () => { expect(await countRows(orm, TripFiles, { id: attachment.id })).toBe(0); - return []; + return 0; }); expect(await svc.deleteNoteFile(trip.id, note.id, attachment.id)).toBe(true); expect(collabRelease.releaseValues).toHaveBeenLastCalledWith(['stuck.pdf'], { bare: 'files', basename: true }); @@ -1004,7 +1004,7 @@ describe('hardening', () => { await attachToNote(trip.id, note.id, 'files/legacy.pdf', 'legacy.pdf'); collabRelease.releaseValues.mockImplementationOnce(async () => { expect(await countRows(orm, CollabNotes, { id: note.id })).toBe(0); - return []; + return 0; }); expect(await svc.deleteNote(trip.id, note.id)).toBe(true); expect(collabRelease.releaseValues).toHaveBeenLastCalledWith(['files/legacy.pdf'], { diff --git a/server/tests/unit/nest/files.service.test.ts b/server/tests/unit/nest/files.service.test.ts index 9a6a57918..f895f074f 100644 --- a/server/tests/unit/nest/files.service.test.ts +++ b/server/tests/unit/nest/files.service.test.ts @@ -565,7 +565,7 @@ describe('permanentDeleteFile', () => { const file = await makeFile(trip.id, user.id, { filename: 'on-disk.pdf' }); release.releaseValues.mockImplementationOnce(async () => { expect(await svc.getFileById(file.id, trip.id)).toBeUndefined(); - return []; + return 0; }); await svc.permanentDeleteFile((await svc.getFileById(file.id, trip.id))!); expect(release.releaseValues).toHaveBeenCalledWith(['on-disk.pdf'], FILE_KEYS); diff --git a/server/tests/unit/nest/user-cleanup.service.test.ts b/server/tests/unit/nest/user-cleanup.service.test.ts index 4ae9177df..07e756848 100644 --- a/server/tests/unit/nest/user-cleanup.service.test.ts +++ b/server/tests/unit/nest/user-cleanup.service.test.ts @@ -454,7 +454,7 @@ describe('deleteUserCompletely', () => { await updateRows(orm, Users, { id: user.id }, { avatar: 'face.png' }); cleanupRelease.releaseValues.mockImplementationOnce(async () => { expect(await findRow(orm, Users, { id: user.id })).toBeNull(); - return []; + return 0; }); await svc.deleteUserCompletely(user.id); expect(cleanupRelease.releaseValues).toHaveBeenCalledWith(['face.png'], { bare: 'avatars' });