diff --git a/backend/__tests__/routes/adminArchivesQuery.test.js b/backend/__tests__/routes/adminArchivesQuery.test.js index 1904f53f..267e8b4e 100644 --- a/backend/__tests__/routes/adminArchivesQuery.test.js +++ b/backend/__tests__/routes/adminArchivesQuery.test.js @@ -29,15 +29,32 @@ const { bootCrmDb, seedMinimal, buildRouteApp } = require('../integration/helper describe('GET /admin/archives query params (#I.01)', () => { let db; let cleanup; let app; let token; - // name, type, archived_at, photo sizes + // name, type, archived_at, photo sizes, archive_size (the zip's own bytes) + // + // The zip sizes are deliberately in a DIFFERENT order from the summed photo + // bytes, because that is the only way a test can tell which of the two the + // route sorted by. Echo's zip size is null — an archive whose file could not + // be measured. const fixtures = [ - ['Alpha Wedding', 'wedding', '2026-01-05T10:00:00.000Z', [300]], - ['Bravo Birthday', 'birthday', '2026-02-05T10:00:00.000Z', [100]], - ['Charlie Wedding', 'wedding', '2026-03-05T10:00:00.000Z', [500, 400]], - ['Delta Corporate', 'corporate', '2026-04-05T10:00:00.000Z', [200]], - ['Echo WEDDING Gala', 'wedding', '2026-05-05T10:00:00.000Z', [50]], + ['Alpha Wedding', 'wedding', '2026-01-05T10:00:00.000Z', [300], 100], + ['Bravo Birthday', 'birthday', '2026-02-05T10:00:00.000Z', [100], 900], + ['Charlie Wedding', 'wedding', '2026-03-05T10:00:00.000Z', [500, 400], 50], + ['Delta Corporate', 'corporate', '2026-04-05T10:00:00.000Z', [200], 400], + ['Echo WEDDING Gala', 'wedding', '2026-05-05T10:00:00.000Z', [50], null], + // Four rows that exist purely to pin LIKE-metacharacter handling: each + // metacharacter name is paired with a name that a wildcard reading of it + // would also match. Dated in 2025 so they sit at the tail of the default + // newest-first ordering. None of them contains "wedding". + ['Summer 100% Sale', 'party', '2025-01-05T10:00:00.000Z', [10], 1], + ['Summer 100X Sale', 'party', '2025-02-05T10:00:00.000Z', [10], 2], + ['Gala_Night', 'party', '2025-03-05T10:00:00.000Z', [10], 3], + ['GalaXNight', 'party', '2025-04-05T10:00:00.000Z', [10], 4], ]; + // Every fixture's photo bytes and zip bytes, for the aggregate assertions. + const ALL_PHOTOS = 10; + const ALL_ARCHIVE_BYTES = 100 + 900 + 50 + 400 + 0 + 1 + 2 + 3 + 4; + const list = async (query) => { const res = await request(app) .get('/admin/archives') @@ -71,7 +88,7 @@ describe('GET /admin/archives query params (#I.01)', () => { ); let i = 0; - for (const [eventName, eventType, archivedAt, sizes] of fixtures) { + for (const [eventName, eventType, archivedAt, sizes, archiveSize] of fixtures) { const slug = `arch-${i++}`; const ev = await db('events').insert({ slug, @@ -88,6 +105,8 @@ describe('GET /admin/archives query params (#I.01)', () => { is_archived: 1, is_draft: 0, archived_at: archivedAt, + archive_path: `events/archived/${slug}.zip`, + archive_size: archiveSize, created_at: new Date().toISOString(), }).returning('id'); const eventId = ev[0]?.id ?? ev[0]; @@ -132,8 +151,9 @@ describe('GET /admin/archives query params (#I.01)', () => { const body = await list({}); expect(names(body)).toEqual([ 'Echo WEDDING Gala', 'Delta Corporate', 'Charlie Wedding', 'Bravo Birthday', 'Alpha Wedding', + 'GalaXNight', 'Gala_Night', 'Summer 100X Sale', 'Summer 100% Sale', ]); - expect(body.pagination.total).toBe(5); + expect(body.pagination.total).toBe(9); }); test('search filters in SQL and the total describes the filtered set', async () => { @@ -166,7 +186,7 @@ describe('GET /admin/archives query params (#I.01)', () => { expect(filtered.pagination.total).toBe(3); const all = await list({ type: 'all' }); - expect(all.pagination.total).toBe(5); + expect(all.pagination.total).toBe(9); }); test('search and type filter combine', async () => { @@ -180,30 +200,165 @@ describe('GET /admin/archives query params (#I.01)', () => { expect(names(page1)).toEqual(['Alpha Wedding', 'Bravo Birthday']); const page3 = await list({ sortBy: 'name', limit: 2, page: 3 }); - expect(names(page3)).toEqual(['Echo WEDDING Gala']); + expect(names(page3)).toEqual(['Echo WEDDING Gala', 'GalaXNight']); }); - test('sortBy=size orders by archived content size, largest first', async () => { + test('sortBy=size orders by the zip size — the number the Size column shows', async () => { const body = await list({ sortBy: 'size' }); - expect(names(body)).toEqual([ - 'Charlie Wedding', // 900 - 'Alpha Wedding', // 300 - 'Delta Corporate', // 200 - 'Bravo Birthday', // 100 - 'Echo WEDDING Gala' // 50 + // Ordering by the summed photo bytes (what this did before + // events.archive_size existed) would have produced Charlie / Alpha / + // Delta / Bravo / Echo — a different list from the one on screen. + expect(names(body).slice(0, 5)).toEqual([ + 'Bravo Birthday', // zip 900, photos 100 + 'Delta Corporate', // zip 400, photos 200 + 'Alpha Wedding', // zip 100, photos 300 + 'Charlie Wedding', // zip 50, photos 900 + 'GalaXNight', // zip 4 ]); + // The unmeasured zip sorts last, as the 0 it displays. + expect(names(body).at(-1)).toBe('Echo WEDDING Gala'); + }); + + test('archiveSize is the stored zip size, and 0 when it was never measured', async () => { + const body = await list({ search: 'wedding' }); + const bySlug = Object.fromEntries(body.archives.map((a) => [a.eventName, a.archiveSize])); + expect(bySlug['Alpha Wedding']).toBe(100); + expect(bySlug['Charlie Wedding']).toBe(50); + expect(bySlug['Echo WEDDING Gala']).toBe(0); }); test('an unknown sortBy falls back to the date ordering', async () => { const body = await list({ sortBy: 'events.id; drop table events' }); expect(names(body)[0]).toBe('Echo WEDDING Gala'); - expect(body.pagination.total).toBe(5); + expect(body.pagination.total).toBe(9); }); test('quotes in the search are bound as a value, not injected as SQL', async () => { const body = await list({ search: '\'; DROP TABLE events; --' }); expect(body.archives).toEqual([]); // The table is still there. - expect((await list({})).pagination.total).toBe(5); + expect((await list({})).pagination.total).toBe(9); + }); + + // --- LIKE metacharacters (C2) ------------------------------------------- + // `%` and `_` are wildcards in a LIKE pattern. An admin typing them into a + // search box means them literally, so they are escaped in the bound value + // and the pattern carries an explicit ESCAPE clause. + + test('a literal % matches only a literal %', async () => { + const body = await list({ search: '100%' }); + expect(names(body)).toEqual(['Summer 100% Sale']); + // Unescaped, "100%" is the prefix wildcard "100" and would also have + // matched "Summer 100X Sale". + expect(body.pagination.total).toBe(1); + }); + + test('a bare % is not a match-everything wildcard', async () => { + const body = await list({ search: '%' }); + expect(names(body)).toEqual(['Summer 100% Sale']); + expect(body.pagination.total).toBe(1); + }); + + test('a literal _ matches only a literal _', async () => { + const body = await list({ search: 'gala_night' }); + expect(names(body)).toEqual(['Gala_Night']); + // Unescaped, "_" is any single character and would also have matched + // "GalaXNight". + expect(body.pagination.total).toBe(1); + }); + + test('a backslash in the search is matched literally, not eaten as an escape', async () => { + // Nothing in the fixtures contains a backslash. Without escaping the + // escape character itself, "\%" would reach SQL as an escaped percent and + // match every row on the engine that honours it. + const body = await list({ search: '\\%' }); + expect(body.archives).toEqual([]); + expect(body.pagination.total).toBe(0); + }); + + // --- Aggregates (C3) ----------------------------------------------------- + + test('totals describe the whole filtered set, not the page', async () => { + const body = await list({ limit: 2, page: 1 }); + expect(body.archives).toHaveLength(2); + expect(body.totals).toEqual({ + archives: 9, + photos: ALL_PHOTOS, + archiveSize: ALL_ARCHIVE_BYTES, + }); + + // Same numbers from the last page — the stat cards must not move as the + // admin pages through the list. + const last = await list({ limit: 2, page: 5 }); + expect(last.totals).toEqual(body.totals); + }); + + test('totals respect the active search and type filter', async () => { + const body = await list({ type: 'wedding' }); + expect(body.totals).toEqual({ + archives: 3, + photos: 4, // Alpha 1 + Charlie 2 + Echo 1 + archiveSize: 100 + 50 + 0, // Echo's zip was never measured + }); + + const none = await list({ search: 'zzz-nothing' }); + expect(none.totals).toEqual({ archives: 0, photos: 0, archiveSize: 0 }); + }); + + // --- Migration backfill (C1) --------------------------------------------- + + test('migration 197 backfills archive_size from the zip on disk', async () => { + const migration = require('../../migrations/core/197_add_event_archive_size'); + const storagePath = process.env.STORAGE_PATH; + await fs.promises.mkdir(path.join(storagePath, 'events/archived'), { recursive: true }); + await fs.promises.writeFile(path.join(storagePath, 'events/archived/backfill.zip'), Buffer.alloc(4096)); + + const base = { + event_type: 'wedding', + event_name: 'Backfill Me', + event_date: '2026-08-01', + host_email: 'h@example.com', + admin_email: 'a@example.com', + password_hash: 'x', + expires_at: new Date(Date.now() + 7 * 864e5).toISOString(), + is_active: 0, + is_archived: 1, + is_draft: 0, + archived_at: '2026-07-05T10:00:00.000Z', + archive_size: null, + created_at: new Date().toISOString(), + }; + await db('events').insert([ + { + ...base, + slug: 'backfill-present', + share_token: 'tok-backfill-present', + share_link: '/gallery/backfill-present/tok-backfill-present', + archive_path: 'events/archived/backfill.zip', + }, + { + ...base, + slug: 'backfill-missing', + share_token: 'tok-backfill-missing', + share_link: '/gallery/backfill-missing/tok-backfill-missing', + archive_path: 'events/archived/gone.zip', + }, + ]); + + // Re-runnable by design: the column already exists, only the empty rows + // are touched. + await migration.up(db); + + const present = await db('events').where('slug', 'backfill-present').first(); + const missing = await db('events').where('slug', 'backfill-missing').first(); + expect(Number(present.archive_size)).toBe(4096); + // A zip that cannot be statted stays NULL rather than claiming 0 bytes. + expect(missing.archive_size).toBeNull(); + + // A second run must not disturb the value it already wrote. + await migration.up(db); + expect(Number((await db('events').where('slug', 'backfill-present').first()).archive_size)).toBe(4096); + + await db('events').whereIn('slug', ['backfill-present', 'backfill-missing']).del(); }); }); diff --git a/backend/migrations/core/197_add_event_archive_size.js b/backend/migrations/core/197_add_event_archive_size.js new file mode 100644 index 00000000..e4447861 --- /dev/null +++ b/backend/migrations/core/197_add_event_archive_size.js @@ -0,0 +1,63 @@ +/** + * `events.archive_size` — the archive zip's own byte size, recorded once at + * archive time. + * + * The archives list showed a Size column produced by a per-row `fs.stat` run + * AFTER pagination, so the number was only ever known for the 20 rows on + * screen. That made a server-side "sort by size" impossible without statting + * every archive on every request, and the sort fell back to + * `SUM(photos.size_bytes)` — the archived *content* size, a different number + * from the one in the column. Rows whose zip is missing or whose compression + * ratio differs sorted by a value the admin was not looking at. + * + * With the size on the row, the sorted number and the displayed number are the + * same number, and the list stops touching the filesystem at all. + * + * bigInteger, not integer: a real wedding archive crosses int4's 2.1 GB + * ceiling routinely — that limit is why the restore path had to move off + * adm-zip. Read it back through `Number()`, the pg driver hands bigints out as + * strings. + */ + +const fs = require('fs').promises; +const path = require('path'); + +exports.up = async function (knex) { + if (!(await knex.schema.hasColumn('events', 'archive_size'))) { + await knex.schema.alterTable('events', (table) => { + table.bigInteger('archive_size'); + }); + } + + // Backfill deliberately OUTSIDE the column guard: a run that died after the + // alterTable but partway through the stats would leave the column present + // and half the rows empty, and the re-run would skip both. `whereNull` makes + // this idempotent and self-healing. + // + // Same storage-root resolution the routes use, so the migration reads the + // files the app writes. + const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage'); + const rows = await knex('events') + .whereNotNull('archive_path') + .whereNull('archive_size') + .select('id', 'archive_path'); + + for (const row of rows) { + try { + const stats = await fs.stat(path.join(storagePath, row.archive_path)); + await knex('events').where('id', row.id).update({ archive_size: stats.size }); + } catch (_) { + // Zip gone, or on a storage backend this process cannot stat (S3). Left + // NULL, which the list renders and orders as 0 — exactly what the per-row + // fs.stat this replaces already did for a file it could not read. + } + } +}; + +exports.down = async function (knex) { + if (await knex.schema.hasColumn('events', 'archive_size')) { + await knex.schema.alterTable('events', (table) => { + table.dropColumn('archive_size'); + }); + } +}; diff --git a/backend/src/routes/adminArchives.js b/backend/src/routes/adminArchives.js index 8fefff4c..51003cc2 100644 --- a/backend/src/routes/adminArchives.js +++ b/backend/src/routes/adminArchives.js @@ -22,18 +22,23 @@ router.get('/', adminAuth, requirePermission('archives.view'), async (req, res) const type = typeof req.query.type === 'string' ? req.query.type.trim() : ''; const sortBy = ['date', 'name', 'size'].includes(req.query.sortBy) ? req.query.sortBy : 'date'; + // A literal % or _ typed into the search box has to match itself rather + // than act as a wildcard. Escape the escape character first, then the two + // wildcards. utils/sqlSecurity.js's escapeLikePattern() is not usable + // here: it also doubles single quotes, which corrupts a BOUND value — + // "Sarah's Birthday" would be searched for as "Sarah''s Birthday". + const escapeLike = (value) => value.replace(/[\\%_]/g, '\\$&'); + // Search and type filtering run in SQL so both the returned rows and // the total count cover the whole archive table, not just the page the // client happens to be on. Values are bound, never interpolated. - // % and _ are wildcards to LIKE but literal characters to the client-side - // `includes()` this replaced, so searching for "100%" would otherwise match - // every archive and report a nonsense total. The ESCAPE clause is - // load-bearing rather than decorative: SQLite has no default LIKE escape - // character, so without it the escaped pattern matches literal backslashes - // there while working on Postgres. - const escapeLike = (value) => value.replace(/[\\%_]/g, '\\$&'); const applyFilters = (query) => { if (search) { + // The ESCAPE clause is explicit because the two engines disagree + // without it: Postgres treats a backslash in a LIKE pattern as an + // escape by default, SQLite has no default escape character at all and + // would match the backslash literally. Naming it makes the escaping + // above mean the same thing on both. query.whereRaw( 'LOWER(events.event_name) LIKE ? ESCAPE \'\\\'', [`%${escapeLike(search.toLowerCase())}%`] @@ -45,11 +50,24 @@ router.get('/', adminAuth, requirePermission('archives.view'), async (req, res) return query; }; - // Get total count (of the filtered set, so pagination stays truthful) + // Totals for the filtered set, so pagination AND the stat cards describe + // the whole result rather than the page. Unjoined: joining photos here + // would multiply archive_size by the event's photo count. const totalCount = await applyFilters( db('events').where('events.is_archived', formatBoolean(true)) ) .count('events.id as count') + .sum({ archive_size: 'events.archive_size' }) + .first(); + + // Photo total needs the join, so it is its own query for the same reason. + // count(photos.id) rather than count(*): a left-joined event with no + // photos must contribute 0, not 1. + const photoTotal = await applyFilters( + db('events').where('events.is_archived', formatBoolean(true)) + ) + .leftJoin('photos', 'events.id', 'photos.event_id') + .count('photos.id as count') .first(); // Get archived events @@ -67,29 +85,20 @@ router.get('/', adminAuth, requirePermission('archives.view'), async (req, res) if (sortBy === 'name') { archivesQuery.orderBy('events.event_name', 'asc'); } else if (sortBy === 'size') { - // The zip's on-disk size is only known after the per-row fs.stat below, - // so a global size sort has to use the archived content size instead. - archivesQuery.orderByRaw('COALESCE(SUM(photos.size_bytes), 0) desc'); + // events.archive_size — the same number the Size column renders. It was + // the archived *content* size (SUM(photos.size_bytes)) until the column + // existed, which meant the list could be ordered by a value that is not + // on screen. NULL is an archive whose zip could not be measured (missing + // file, or a storage backend the backfill could not stat); it sorts as 0, + // i.e. last, which is also what it displays as. + archivesQuery.orderByRaw('COALESCE(events.archive_size, 0) desc'); } else { archivesQuery.orderBy('events.archived_at', 'desc'); } const archives = await archivesQuery.limit(limit).offset(offset); - // Check if archive files exist and get their sizes - const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage'); - const archivesWithFileInfo = await Promise.all(archives.map(async (archive) => { - let archiveFileSize = 0; - if (archive.archive_path) { - try { - const fullArchivePath = path.join(storagePath, archive.archive_path); - const stats = await fs.stat(fullArchivePath); - archiveFileSize = stats.size; - } catch (error) { - logger.error(`Archive file not found: ${archive.archive_path}`); - } - } - + const archivesWithFileInfo = archives.map((archive) => { return { id: archive.id, slug: archive.slug, @@ -101,10 +110,11 @@ router.get('/', adminAuth, requirePermission('archives.view'), async (req, res) expiresAt: archive.expires_at ? new Date(archive.expires_at).toISOString() : null, photoCount: archive.photo_count || 0, originalSize: archive.total_size || 0, - archiveSize: archiveFileSize, + // Number(): bigInteger comes back from the pg driver as a string. + archiveSize: Number(archive.archive_size) || 0, archivePath: archive.archive_path }; - })); + }); res.json({ archives: archivesWithFileInfo, @@ -113,6 +123,14 @@ router.get('/', adminAuth, requirePermission('archives.view'), async (req, res) limit, total: totalCount.count, totalPages: Math.ceil(totalCount.count / limit) + }, + // Aggregates over the FILTERED set — the stat cards used to sum the rows + // the client could see, so every figure was page-scoped while the footer + // beside them reported the real total. + totals: { + archives: Number(totalCount.count) || 0, + photos: Number(photoTotal.count) || 0, + archiveSize: Number(totalCount.archive_size) || 0 } }); } catch (error) { @@ -533,6 +551,10 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re is_archived: false, is_active: true, archive_path: null, + // Cleared with the path it measures — a restored event has no zip, and + // a stale size would be re-shown verbatim if it is archived again + // before the new archive finishes writing. + archive_size: null, archived_at: null, expires_at: thirtyDaysFromNow.toISOString() // Reset expiration - works on both DBs }); diff --git a/backend/src/services/archiveService.js b/backend/src/services/archiveService.js index 8fb3c7da..7c255ea7 100644 --- a/backend/src/services/archiveService.js +++ b/backend/src/services/archiveService.js @@ -166,6 +166,10 @@ async function archiveEvent(event) { await db('events').where('id', event.id).update({ is_archived: true, archive_path: archiveRelKey, + // The zip's own byte size — the same number the completion email + // reports below. Persisted so the archives list can sort and display it + // without statting every archive on every request. + archive_size: totalBytes, archived_at: new Date(), }); diff --git a/frontend/src/pages/admin/ArchivesPage.tsx b/frontend/src/pages/admin/ArchivesPage.tsx index 31352b97..a7a27744 100644 --- a/frontend/src/pages/admin/ArchivesPage.tsx +++ b/frontend/src/pages/admin/ArchivesPage.tsx @@ -68,9 +68,11 @@ export const ArchivesPage: React.FC = () => { const archives = archivesData?.archives || []; - const getTotalSize = () => { - return archives.reduce((sum, archive) => sum + archive.archiveSize, 0); - }; + // Server-side aggregates over the whole filtered set. Summing `archives` + // here only ever described the 20 rows of the current page, so "Storage + // used" on an 802-archive install was off by roughly 40x while the footer + // right below it reported the real total. + const totals = archivesData?.totals ?? { archives: 0, photos: 0, archiveSize: 0 }; // Mutations const restoreMutation = useMutationWithToast({ @@ -136,7 +138,7 @@ export const ArchivesPage: React.FC = () => {
{t('archives.totalArchives')}
-{archives.length}
+{totals.archives}
{t('archives.storageUsed')}
-{archiveService.formatBytes(getTotalSize())}
+{archiveService.formatBytes(totals.archiveSize)}
{t('archives.totalPhotos')}
- {(() => { - const total = archives.reduce((sum, a) => sum + (parseInt(String(a.photoCount)) || 0), 0); - return total === 0 ? '0' : total.toLocaleString(); - })()} + {totals.photos === 0 ? '0' : totals.photos.toLocaleString()}
{t('archives.avgArchiveSize')}
- {archives.length > 0 - ? archiveService.formatBytes(getTotalSize() / archives.length) + {totals.archives > 0 + ? archiveService.formatBytes(totals.archiveSize / totals.archives) : '0 Bytes' }
@@ -340,39 +339,45 @@ export const ArchivesPage: React.FC = () => {