diff --git a/backend/__tests__/integration/regenerateThumbnailsScript.test.js b/backend/__tests__/integration/regenerateThumbnailsScript.test.js new file mode 100644 index 00000000..7e435568 --- /dev/null +++ b/backend/__tests__/integration/regenerateThumbnailsScript.test.js @@ -0,0 +1,297 @@ +/** + * scripts/regenerate-thumbnails.js against external photos (#1148). + * + * The same defect #1129 fixed in the admin route, still standing in the CLI + * fallback: the script resolved every source as + * `storage/events/active/` and fs.access'd it. External and + * reference rows do not live there — their originals sit under + * `events.external_path` — so every one failed the check and was counted as an + * error. On an install where all photos are external the script did nothing at + * all, while reporting one error per photo. + * + * Driven against a REAL file on a REAL external mount with the real + * imageProcessor, not a mock: the whole point is that the source resolves off + * the mount, and a mocked ensureThumbnail would assert nothing about that. + */ + +const fs = require('fs'); +const path = require('path'); +const os = require('os'); +const sharp = require('sharp'); +const { execFile } = require('child_process'); + +describe('regenerate-thumbnails script (#1148)', () => { + let tmpDir; let db; let cleanup; let regenerateThumbnails; + let eventId; let externalPhotoId; let videoPhotoId; let watcherVideoId; let repairPhotoId; + let vanishingPhotoId; + let externalRoot; + + beforeAll(async () => { + tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'picpeak-regen-script-')); + process.env.NODE_ENV = 'test'; + process.env.TEST_DATABASE_PATH = path.join(tmpDir, 'data', 'test.db'); + process.env.STORAGE_PATH = path.join(tmpDir, 'storage'); + // External sources are sandboxed under EXTERNAL_MEDIA_ROOT; event + // external_path is relative to it, exactly as on a real install. + process.env.EXTERNAL_MEDIA_ROOT = path.join(tmpDir, 'media'); + externalRoot = path.join(process.env.EXTERNAL_MEDIA_ROOT, 'wedding'); + + await fs.promises.mkdir(path.dirname(process.env.TEST_DATABASE_PATH), { recursive: true }); + await fs.promises.mkdir(process.env.STORAGE_PATH, { recursive: true }); + await fs.promises.mkdir(externalRoot, { recursive: true }); + + jest.resetModules(); + ({ db, cleanup } = await require('./helpers/crmDb').bootCrmDb()); + + // A real image on the external mount — never under events/active. + await sharp({ + create: { width: 1200, height: 800, channels: 3, background: { r: 10, g: 90, b: 160 } }, + }).jpeg().toFile(path.join(externalRoot, 'shot.jpg')); + + const [ev] = await db('events').insert({ + slug: 'regen-script-event', + event_type: 'wedding', + event_name: 'Regen Script', + event_date: '2026-08-01', + host_email: 'h@example.com', + admin_email: 'a@example.com', + password_hash: 'x', + share_link: '/gallery/regen-script-event/share', + expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(), + source_mode: 'reference', + external_path: 'wedding', + created_at: new Date().toISOString(), + }).returning('id'); + eventId = typeof ev === 'object' ? ev.id : ev; + + const [p] = await db('photos').insert({ + event_id: eventId, + filename: 'shot.jpg', + // `path` is what the old script joined onto events/active. Left + // populated on purpose: the fix must ignore it for an external row. + path: 'regen-script-event/shot.jpg', + type: 'individual', + source_origin: 'external', + external_relpath: 'shot.jpg', + uploaded_at: new Date().toISOString(), + }).returning('id'); + externalPhotoId = typeof p === 'object' ? p.id : p; + + const [v] = await db('photos').insert({ + event_id: eventId, + filename: 'clip.mp4', + path: 'regen-script-event/clip.mp4', + type: 'individual', + media_type: 'video', + mime_type: 'video/mp4', + source_origin: 'external', + external_relpath: 'clip.mp4', + uploaded_at: new Date().toISOString(), + }).returning('id'); + videoPhotoId = typeof v === 'object' ? v.id : v; + + // How fileWatcher.processNewPhoto actually writes a video: `type` and + // `mime_type` set, media_type left to its 'image' default. A media_type-only + // filter lets this through and hands the container to Sharp. + // + // The file has to EXIST, otherwise the row fails resolution and looks + // skipped for the wrong reason — the bug is Sharp being handed a video, not + // a missing source. Real MP4 header bytes, no image in sight. + await fs.promises.writeFile( + path.join(externalRoot, 'watched.mp4'), + Buffer.from('00000018667479706d70343200000000', 'hex') + ); + const [wv] = await db('photos').insert({ + event_id: eventId, + filename: 'watched.mp4', + path: 'regen-script-event/watched.mp4', + type: 'video', + mime_type: 'video/mp4', + source_origin: 'external', + external_relpath: 'watched.mp4', + uploaded_at: new Date().toISOString(), + }).returning('id'); + watcherVideoId = typeof wv === 'object' ? wv.id : wv; + expect((await db('photos').where('id', watcherVideoId).first()).media_type).not.toBe('video'); + + // A photo whose thumbnail_path points at something that is no longer there. + await sharp({ + create: { width: 900, height: 600, channels: 3, background: { r: 200, g: 40, b: 40 } }, + }).jpeg().toFile(path.join(externalRoot, 'repair.jpg')); + const [rp] = await db('photos').insert({ + event_id: eventId, + filename: 'repair.jpg', + path: 'regen-script-event/repair.jpg', + type: 'individual', + thumbnail_path: 'thumbnails/thumb_ext_missing_repair.jpg', + source_origin: 'external', + external_relpath: 'repair.jpg', + uploaded_at: new Date().toISOString(), + }).returning('id'); + repairPhotoId = typeof rp === 'object' ? rp.id : rp; + + // A photo whose source will be removed after its canonical thumbnail is + // cached — the "mount went away" case, where the canonical rendition is + // served from cache but a tier still needs to read the original. + await sharp({ + create: { width: 1000, height: 700, channels: 3, background: { r: 30, g: 140, b: 60 } }, + }).jpeg().toFile(path.join(externalRoot, 'vanishing.jpg')); + const [vp] = await db('photos').insert({ + event_id: eventId, + filename: 'vanishing.jpg', + path: 'regen-script-event/vanishing.jpg', + type: 'individual', + source_origin: 'external', + external_relpath: 'vanishing.jpg', + uploaded_at: new Date().toISOString(), + }).returning('id'); + vanishingPhotoId = typeof vp === 'object' ? vp.id : vp; + + ({ regenerateThumbnails } = require('../../scripts/regenerate-thumbnails')); + }, 180000); + + afterAll(async () => { + if (cleanup) await cleanup(); + await fs.promises.rm(tmpDir, { recursive: true, force: true }).catch(() => {}); + }); + + it('builds a thumbnail for an external photo instead of erroring on events/active', async () => { + // The location the old script computed and fs.access'd. Nothing is there, + // which is the whole defect — it is not where an external original lives. + // (The old script cannot be driven from a test directly: it had no export + // and ran on require, calling process.exit. Making it importable is part + // of this fix.) + const legacyPath = path.join(process.env.STORAGE_PATH, 'events/active', 'regen-script-event/shot.jpg'); + expect(fs.existsSync(legacyPath)).toBe(false); + + const result = await regenerateThumbnails(eventId, { tiers: false }); + + // The old script reported an error for this photo and wrote nothing. + expect(result.errorCount).toBe(0); + // The external photo, the repair row and the vanishing one; no video. + expect(result.successCount).toBe(3); + + const row = await db('photos').where('id', externalPhotoId).first(); + expect(row.thumbnail_path).toBeTruthy(); + const onDisk = path.join(process.env.STORAGE_PATH, row.thumbnail_path); + expect(fs.existsSync(onDisk)).toBe(true); + + // Named per-photo so two events referencing one NAS basename cannot + // clobber each other — the property ensureThumbnail owns and the reason + // the script must not build this name itself. + expect(path.basename(row.thumbnail_path)).toContain(`ext${externalPhotoId}_`); + }); + + it('leaves videos alone', async () => { + // A video thumbnail is a poster frame from videoProcessor; handing the + // container to Sharp produced one error per video row. + const row = await db('photos').where('id', videoPhotoId).first(); + expect(row.thumbnail_path).toBeFalsy(); + }); + + it('leaves a watcher-imported video alone, which carries no media_type', async () => { + // fileWatcher writes type + mime_type and lets media_type default to + // 'image', so filtering on media_type alone still fed these to Sharp. The + // signal is errorCount: the images are already done by now, so the only + // thing that can fail this run is a video reaching Sharp. + const result = await regenerateThumbnails(eventId, { tiers: false }); + + expect(result.errorCount).toBe(0); + const row = await db('photos').where('id', watcherVideoId).first(); + expect(row.thumbnail_path).toBeFalsy(); + }); + + it('is idempotent — a second run skips instead of rebuilding', async () => { + const before = await db('photos').where('id', externalPhotoId).first(); + const result = await regenerateThumbnails(eventId, { tiers: false }); + + expect(result.errorCount).toBe(0); + expect(result.successCount).toBe(0); + expect(result.skipCount).toBe(3); + + const after = await db('photos').where('id', externalPhotoId).first(); + expect(after.thumbnail_path).toBe(before.thumbnail_path); + }); + + it('counts a repaired thumbnail as generated, not skipped', async () => { + // Both images are valid at this point. Destroy ONE thumbnail object while + // leaving thumbnail_path pointing at it — the corrupt/missing case. + const row = await db('photos').where('id', repairPhotoId).first(); + const onDisk = path.join(process.env.STORAGE_PATH, row.thumbnail_path); + await fs.promises.rm(onDisk); + + const result = await regenerateThumbnails(eventId, { tiers: false }); + + // On local and external storage the rebuilt key is identical, so inferring + // "skipped" from an unchanged path reports this repair as already valid — + // the one number an operator running this is actually reading. + expect(result.successCount).toBe(1); + expect(result.skipCount).toBe(2); + expect(result.errorCount).toBe(0); + expect(fs.existsSync(onDisk)).toBe(true); + }); + + it('backfills the responsive tiers, which is what a backfill is for', async () => { + // The tiers (#1095/#1109) are cached separately from thumbnail_path, so a + // gallery can hold every canonical rendition and still serve phones the + // full-size image. The old script only ever produced `thumb_` at + // a hard-coded 300px and could not backfill them at all. + const { THUMBNAIL_WIDTHS } = require('../../src/services/imageProcessor'); + const imageRows = 3; // external, repaired and vanishing; videos excluded + const result = await regenerateThumbnails(eventId, { tiers: true }); + + expect(result.errorCount).toBe(0); + expect(result.tierCount).toBe(THUMBNAIL_WIDTHS.length * imageRows); + expect(result.tierFailures).toBe(0); + }); + + it('reports tiers it could not build instead of claiming success', async () => { + // ensureThumbnailAtWidth handles the expected failures itself and returns + // NULL rather than throwing — an unreachable mount, a storage write that + // did not land. A try/catch alone never sees those, so the run counted + // zero errors and printed a clean summary after backfilling nothing. + // + // Reproduced the honest way: cache the canonical rendition, then take the + // source away. The canonical is served from cache; the tiers still need + // the original. + const row = await db('photos').where('id', vanishingPhotoId).first(); + expect(row.thumbnail_path).toBeTruthy(); + + const { deleteThumbnailTiers } = require('../../src/services/imageProcessor'); + await deleteThumbnailTiers(row).catch(() => {}); + await fs.promises.rm(path.join(externalRoot, 'vanishing.jpg')); + + const result = await regenerateThumbnails(eventId, { tiers: true }); + + expect(result.tierFailures).toBeGreaterThan(0); + // Still not an error against the photo: the canonical rendition is intact + // and the gallery falls back to it. + expect(result.errorCount).toBe(0); + }); + + /** Run the CLI the way cron does, and hand back its exit status. */ + const runCli = (args = []) => new Promise((resolve) => { + execFile( + process.execPath, + [path.join(__dirname, '..', '..', 'scripts', 'regenerate-thumbnails.js'), ...args], + { env: { ...process.env }, cwd: path.join(__dirname, '..', '..') }, + (error, stdout, stderr) => resolve({ code: error?.code ?? 0, stdout, stderr }) + ); + }); + + it('exits nonzero when work was left unfinished', async () => { + // Exit status is the only thing a cron job reads. `vanishing.jpg` still + // has no source, so its tiers cannot be built. + const failed = await runCli([String(eventId)]); + expect(failed.code).toBe(1); + expect(failed.stderr).toContain('completed with failures'); + }, 120000); + + it('exits zero when there is nothing left to do', async () => { + // Same event with tiers switched off: every canonical rendition is already + // valid, so a clean run must not cry wolf at automation. + const ok = await runCli([String(eventId), '--no-tiers']); + expect(ok.code).toBe(0); + expect(ok.stdout).toContain('Script completed successfully'); + }, 120000); +}); diff --git a/backend/scripts/regenerate-thumbnails.js b/backend/scripts/regenerate-thumbnails.js index 3797c556..2848334e 100644 --- a/backend/scripts/regenerate-thumbnails.js +++ b/backend/scripts/regenerate-thumbnails.js @@ -1,141 +1,196 @@ #!/usr/bin/env node /** - * Script to regenerate missing thumbnails for photos in the database - * Usage: node scripts/regenerate-thumbnails.js [eventId] + * Fill in missing thumbnails for photos already in the database. + * + * The CLI fallback for when the admin UI is not reachable. It is deliberately + * "missing only": ensureThumbnail short-circuits on a thumbnail that is + * already present and valid, so re-running this is cheap and safe. To REBUILD + * everything after a settings change, use POST /api/admin/thumbnails/regenerate + * — that path drops the existing renditions first, which this one must not do. + * + * Resolution goes through ensureThumbnail rather than a hand-built path + * (#1148, same defect as #1129). This script used to compute + * `storage/events/active/` and fs.access it, a location that does + * not exist for `external` or `reference` rows — their originals live under + * the mount in events.external_path. Every such photo failed the check and was + * counted as an error, so on an external-media install the script was inert + * while reporting one error per photo. + * + * ensureThumbnail already branches on source_origin, resolves both kinds via + * photoResolver, uses the per-photo `ext_` output name so two events + * referencing one NAS basename cannot clobber each other, and writes + * thumbnail_path back itself. Sharing it is what stops the script and the + * route drifting apart again. + * + * Usage: + * node scripts/regenerate-thumbnails.js [eventId] [--no-tiers] */ -const path = require('path'); -const fs = require('fs').promises; -const sharp = require('sharp'); const { db } = require('../src/database/db'); +const { + ensureThumbnail, + ensureThumbnailAtWidth, + isThumbnailValid, + THUMBNAIL_WIDTHS, +} = require('../src/services/imageProcessor'); -// Configuration -const THUMBNAIL_SIZE = 300; -const STORAGE_PATH = process.env.STORAGE_PATH || path.join(__dirname, '../../storage'); -const THUMBNAILS_DIR = path.join(STORAGE_PATH, 'thumbnails'); +async function regenerateThumbnails(eventId = null, { tiers = true } = {}) { + console.log('Starting thumbnail regeneration...'); -async function ensureDirectoryExists(dirPath) { - try { - await fs.access(dirPath); - } catch { - await fs.mkdir(dirPath, { recursive: true }); - console.log(`Created directory: ${dirPath}`); + // These columns are what ensureThumbnail and ensureThumbnailAtWidth branch + // on to resolve a source and name their output. Selecting a subset that + // misses source_origin/external_relpath is how the old path bug would come + // back — an external row would look managed and resolve under events/active. + let query = db('photos').select( + 'id', 'event_id', 'path', 'filename', 'thumbnail_path', + 'media_type', 'mime_type', 'source_origin', 'external_relpath' + ); + + if (eventId) { + query = query.where('event_id', eventId); + console.log(`Filtering for event ID: ${eventId}`); } -} -async function generateThumbnail(photoPath, thumbnailPath) { - try { - await sharp(photoPath) - .resize(THUMBNAIL_SIZE, THUMBNAIL_SIZE, { - fit: 'cover', - position: 'center' - }) - .jpeg({ quality: 80 }) - .toFile(thumbnailPath); - - return true; - } catch (error) { - console.error(`Failed to generate thumbnail for ${photoPath}:`, error.message); - return false; - } -} + // Skip videos. A video's thumbnail is a poster frame produced by + // videoProcessor, not a resize of the stored file, so handing the container + // to Sharp here only ever produced one error per row. + // + // Tested on every marker a video row can carry, not media_type alone: + // fileWatcher.processNewPhoto writes `type` and `mime_type` but never + // media_type, which defaults to 'image' — so an auto-imported video passes a + // media_type-only filter. Each clause is null-safe on its own so a row that + // simply has no mime_type is not swept up with them. + query = query + .where(function () { + this.whereNull('media_type').orWhere('media_type', '!=', 'video'); + }) + .where(function () { + this.whereNull('type').orWhere('type', '!=', 'video'); + }) + .where(function () { + this.whereNull('mime_type').orWhereNot('mime_type', 'like', 'video/%'); + }); -async function regenerateThumbnails(eventId = null) { - try { - console.log('Starting thumbnail regeneration...'); - console.log(`Storage path: ${STORAGE_PATH}`); - console.log(`Thumbnails directory: ${THUMBNAILS_DIR}`); - - // Ensure thumbnails directory exists - await ensureDirectoryExists(THUMBNAILS_DIR); - - // Build query - let query = db('photos') - .join('events', 'photos.event_id', 'events.id') - .select( - 'photos.id', - 'photos.filename', - 'photos.path', - 'photos.thumbnail_path', - 'events.slug as event_slug' - ); - - if (eventId) { - query = query.where('photos.event_id', eventId); - console.log(`Filtering for event ID: ${eventId}`); - } - - const photos = await query; - console.log(`Found ${photos.length} photos to process`); - - let successCount = 0; - let skipCount = 0; - let errorCount = 0; - - for (const photo of photos) { - const photoPath = path.join(STORAGE_PATH, 'events/active', photo.path); - const thumbnailFilename = `thumb_${photo.filename}`; - const thumbnailPath = path.join(THUMBNAILS_DIR, thumbnailFilename); - - try { - // Check if photo file exists - await fs.access(photoPath); - - // Check if thumbnail already exists - try { - await fs.access(thumbnailPath); - console.log(`Thumbnail already exists for ${photo.filename}, skipping...`); - skipCount++; - continue; - } catch { - // Thumbnail doesn't exist, generate it - } - - console.log(`Generating thumbnail for ${photo.filename}...`); - const success = await generateThumbnail(photoPath, thumbnailPath); - - if (success) { - // Update database with thumbnail path - await db('photos') - .where('id', photo.id) - .update({ - thumbnail_path: `thumbnails/${thumbnailFilename}` - }); - - successCount++; - console.log(`✓ Generated thumbnail for ${photo.filename}`); - } else { - errorCount++; - } - } catch (error) { - console.error(`✗ Photo file not found: ${photoPath}`); + const photos = await query; + console.log(`Found ${photos.length} photos to process`); + + let successCount = 0; + let skipCount = 0; + let errorCount = 0; + let tierCount = 0; + let tierFailures = 0; + + for (const photo of photos) { + const label = photo.filename || `photo ${photo.id}`; + try { + const existing = photo.thumbnail_path; + // Asked BEFORE the call, not inferred from the returned path afterwards. + // On local and external storage the key is deterministic, so repairing a + // missing or corrupt thumbnail hands back the identical string — and + // comparing paths would report that repair as "already valid", which is + // the one number an operator running this is actually reading. + const wasValid = existing ? await isThumbnailValid(existing) : false; + const thumbnailPath = await ensureThumbnail(photo); + + if (!thumbnailPath) { + console.error(`✗ Could not generate thumbnail for ${label}`); errorCount++; + continue; } + + if (wasValid && thumbnailPath === existing) { + skipCount++; + } else { + successCount++; + console.log(`✓ Generated thumbnail for ${label}`); + } + + // The responsive tiers (#1095/#1109) are cached separately from + // thumbnail_path, so a gallery can have every canonical rendition and + // still serve phones the full-size image. Backfilling them is the most + // likely reason to reach for this script at all, so it is the default. + // Each call is a no-op when the tier is already stored. + if (tiers) { + for (const width of THUMBNAIL_WIDTHS) { + // Two ways this fails and both have to be reported. It THROWS on an + // unexpected error, and it RETURNS NULL on the expected ones it + // handles itself — an unreachable mount, a storage operation that + // did not land. Ignoring the null said "complete" after backfilling + // nothing, which is worse than the error it was hiding. + let built = null; + try { + built = await ensureThumbnailAtWidth({ ...photo, thumbnail_path: thumbnailPath }, width); + } catch (error) { + console.warn(` ! tier ${width}px failed for ${label}: ${error.message}`); + } + + // One missing tier is not a failed photo — the canonical rendition + // above is what the gallery falls back to — so this is counted + // separately rather than as an error against the photo. + if (built) tierCount++; + else { + tierFailures++; + console.warn(` ! tier ${width}px not built for ${label}`); + } + } + } + } catch (error) { + console.error(`✗ Failed for ${label}: ${error.message}`); + errorCount++; } - - console.log('\nThumbnail regeneration complete!'); - console.log(`- Successfully generated: ${successCount}`); - console.log(`- Skipped (already exist): ${skipCount}`); - console.log(`- Errors: ${errorCount}`); - console.log(`- Total processed: ${photos.length}`); - - } catch (error) { - console.error('Error during thumbnail regeneration:', error); - process.exit(1); - } finally { - await db.destroy(); } + + console.log('\nThumbnail regeneration complete!'); + console.log(`- Generated: ${successCount}`); + console.log(`- Skipped (already valid): ${skipCount}`); + console.log(`- Errors: ${errorCount}`); + if (tiers) { + console.log(`- Responsive tiers present: ${tierCount}`); + if (tierFailures) console.log(`- Responsive tiers NOT built: ${tierFailures}`); + } + console.log(`- Total processed: ${photos.length}`); + + return { successCount, skipCount, errorCount, tierCount, tierFailures }; } -// Parse command line arguments -const eventId = process.argv[2] ? parseInt(process.argv[2]) : null; +if (require.main === module) { + const args = process.argv.slice(2); + const tiers = !args.includes('--no-tiers'); + const eventArg = args.find((a) => !a.startsWith('--')); + const eventId = eventArg ? parseInt(eventArg, 10) : null; -// Run the script -regenerateThumbnails(eventId).then(() => { - console.log('Script completed successfully'); - process.exit(0); -}).catch(error => { - console.error('Script failed:', error); - process.exit(1); -}); \ No newline at end of file + if (eventArg && !Number.isInteger(eventId)) { + console.error(`Not an event id: ${eventArg}`); + process.exit(1); + } + + regenerateThumbnails(eventId, { tiers }) + .then(async (result) => { + await db.destroy(); + // Exit status is the only thing a cron job reads. Resolving with a + // nonzero errorCount and still exiting 0 told automation the backfill + // was done when it had failed — which is how an unavailable mount stays + // unnoticed until someone opens a gallery. + // + // tierFailures counts too: a run that was asked for tiers and could not + // build them is incomplete, even though the canonical renditions are + // intact and the gallery still works. + if (result.errorCount || result.tierFailures) { + console.error( + `Script completed with failures: ${result.errorCount} photo(s), ` + + `${result.tierFailures} tier(s)` + ); + process.exit(1); + } + console.log('Script completed successfully'); + process.exit(0); + }) + .catch(async (error) => { + console.error('Script failed:', error); + await db.destroy().catch(() => {}); + process.exit(1); + }); +} + +module.exports = { regenerateThumbnails };