diff --git a/backend/__tests__/routes/authSession.symmetry.test.js b/backend/__tests__/routes/authSession.symmetry.test.js new file mode 100644 index 00000000..b4137a8e --- /dev/null +++ b/backend/__tests__/routes/authSession.symmetry.test.js @@ -0,0 +1,302 @@ +/** + * Regression test for the /admin/login → /admin/dashboard → /admin/login + * redirect loop reported on v3.32.4-beta.0. + * + * Cause: GET /auth/session was less strict than the adminAuth middleware. + * The session endpoint accepted tokens that the protected endpoints + * subsequently rejected with 401, which the frontend's interceptor + * translated into a hard redirect to /admin/login. /auth/session then + * said "valid: true" again on the next page load and the cycle closed. + * + * /auth/session must reject the same admin tokens adminAuth would + * reject, specifically: deactivated admin user, deleted admin user, + * password changed since iat. Same for gallery: archived event. + */ + +const express = require('express'); +const request = require('supertest'); +const jwt = require('jsonwebtoken'); + +process.env.JWT_SECRET = 'session-symmetry-test-secret'; + +const fakeDb = { + adminUsers: [], + events: [], + revokedTokens: [], +}; + +jest.mock('../../src/database/db', () => { + const formatBoolean = (v) => (v ? 1 : 0); + void formatBoolean; + function dbFn(table) { + if (table === 'admin_users') { + let rowFilter = () => true; + return { + where(criteria) { + rowFilter = (row) => { + return Object.entries(criteria).every(([k, v]) => { + if (k === 'is_active') return Boolean(row.is_active) === Boolean(v); + return row[k] === v; + }); + }; + return this; + }, + select(...cols) { + this._cols = cols; + return this; + }, + async first() { + const row = fakeDb.adminUsers.find(rowFilter); + if (!row) return undefined; + if (!this._cols) return row; + const out = {}; + for (const c of this._cols) out[c] = row[c]; + return out; + }, + }; + } + if (table === 'events') { + let rowFilter = () => true; + return { + where(criteria) { + rowFilter = (row) => + Object.entries(criteria).every(([k, v]) => { + if (k === 'is_active') return Boolean(row.is_active) === Boolean(v); + if (k === 'is_archived') return Boolean(row.is_archived) === Boolean(v); + return row[k] === v; + }); + return this; + }, + async first() { + return fakeDb.events.find(rowFilter); + }, + }; + } + throw new Error(`Unexpected table: ${table}`); + } + return { db: dbFn, formatBoolean: () => 1 }; +}); + +jest.mock('../../src/utils/dbCompat', () => ({ + formatBoolean: (v) => (v ? 1 : 0), +})); + +jest.mock('../../src/utils/tokenRevocation', () => ({ + isTokenRevoked: jest.fn(async (decoded) => fakeDb.revokedTokens.includes(decoded.id)), + revokeToken: jest.fn(), +})); + +jest.mock('../../src/utils/tokenUtils', () => ({ + getAdminTokenFromRequest: (req) => { + const auth = req.headers.authorization; + if (auth && auth.startsWith('Bearer ')) return auth.slice(7); + return null; + }, + getGalleryTokenFromRequest: () => null, + setAdminAuthCookie: jest.fn(), + setGalleryAuthCookies: jest.fn(), + clearAdminAuthCookie: jest.fn(), + clearGalleryAuthCookies: jest.fn(), + buildCookieOptionsWithExpiry: () => ({}), +})); + +jest.mock('../../src/services/recaptcha', () => ({ verifyRecaptcha: () => Promise.resolve(true) })); +jest.mock('../../src/middleware/sessionTimeout', () => ({ endSession: jest.fn() })); +jest.mock('../../src/utils/logger', () => ({ + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), +})); + +const authRouter = require('../../src/routes/auth'); + +function makeApp() { + const app = express(); + app.use(express.json()); + app.use('/auth', authRouter); + return app; +} + +function signAdminToken({ id = 1, username = 'admin', iat, exp }) { + const issuedAt = iat ?? Math.floor(Date.now() / 1000); + // Note: do NOT pass noTimestamp:true here — that strips iat from the + // payload entirely, defeating the password-change comparison. Provide + // iat (and exp) via the payload directly instead. + return jwt.sign( + { id, username, type: 'admin', iat: issuedAt, exp: exp ?? issuedAt + 3600 }, + process.env.JWT_SECRET, + { issuer: 'picpeak-auth' } + ); +} + +function signGalleryToken({ eventId = 100, eventSlug = 'wedding' } = {}) { + return jwt.sign( + { eventId, eventSlug, type: 'gallery' }, + process.env.JWT_SECRET, + { expiresIn: '1h', issuer: 'picpeak-auth' } + ); +} + +describe('GET /auth/session — symmetry with protected middleware', () => { + beforeEach(() => { + fakeDb.adminUsers = []; + fakeDb.events = []; + fakeDb.revokedTokens = []; + }); + + it('returns valid:true for an active admin token', async () => { + fakeDb.adminUsers.push({ + id: 1, + username: 'admin', + email: 'a@b.com', + is_active: true, + password_changed_at: null, + }); + const token = signAdminToken({ id: 1 }); + + const res = await request(makeApp()) + .get('/auth/session') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(true); + expect(res.body.type).toBe('admin'); + }); + + it('returns valid:false when the admin user has been deactivated', async () => { + fakeDb.adminUsers.push({ + id: 1, + username: 'admin', + email: 'a@b.com', + is_active: false, + password_changed_at: null, + }); + const token = signAdminToken({ id: 1 }); + + const res = await request(makeApp()) + .get('/auth/session') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + }); + + it('returns valid:false when the admin user no longer exists', async () => { + // adminUsers is empty + const token = signAdminToken({ id: 999 }); + + const res = await request(makeApp()) + .get('/auth/session') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + }); + + it('returns valid:false when password was changed after the token was issued', async () => { + // iat must be in the past, exp must be in the future so jwt.verify + // doesn't reject the token before /auth/session even gets to look + // at password_changed_at. + const tokenIssuedAt = Math.floor(Date.now() / 1000) - 60; // 1 min ago + const tokenExp = tokenIssuedAt + 86400; + fakeDb.adminUsers.push({ + id: 1, + username: 'admin', + email: 'a@b.com', + is_active: true, + password_changed_at: new Date((tokenIssuedAt + 30) * 1000), // 30s after iat + }); + const token = signAdminToken({ id: 1, iat: tokenIssuedAt, exp: tokenExp }); + + const res = await request(makeApp()) + .get('/auth/session') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + }); + + it('returns valid:true when password was changed BEFORE the token was issued', async () => { + const tokenIssuedAt = Math.floor(Date.now() / 1000) - 60; + const tokenExp = tokenIssuedAt + 86400; + fakeDb.adminUsers.push({ + id: 1, + username: 'admin', + email: 'a@b.com', + is_active: true, + password_changed_at: new Date((tokenIssuedAt - 3600) * 1000), // 1h before iat + }); + const token = signAdminToken({ id: 1, iat: tokenIssuedAt, exp: tokenExp }); + + const res = await request(makeApp()) + .get('/auth/session') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(true); + }); + + it('returns valid:false for a gallery token whose event is archived', async () => { + fakeDb.events.push({ + id: 100, + slug: 'wedding', + is_active: true, + is_archived: true, + expires_at: null, + }); + const token = signGalleryToken(); + + const res = await request(makeApp()) + .get('/auth/session?slug=wedding') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + }); + + it('returns valid:false for a gallery token whose event is expired', async () => { + fakeDb.events.push({ + id: 100, + slug: 'wedding', + is_active: true, + is_archived: false, + expires_at: new Date(Date.now() - 86400_000), + }); + const token = signGalleryToken(); + + const res = await request(makeApp()) + .get('/auth/session?slug=wedding') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(false); + }); + + it('returns valid:true for an active gallery token', async () => { + fakeDb.events.push({ + id: 100, + slug: 'wedding', + is_active: true, + is_archived: false, + expires_at: new Date(Date.now() + 86400_000), + }); + const token = signGalleryToken(); + + const res = await request(makeApp()) + .get('/auth/session?slug=wedding') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(200); + expect(res.body.valid).toBe(true); + }); + + it('returns valid:false when the token is revoked', async () => { + fakeDb.adminUsers.push({ + id: 1, + username: 'admin', + is_active: true, + password_changed_at: null, + }); + fakeDb.revokedTokens.push(1); + const token = signAdminToken({ id: 1 }); + + const res = await request(makeApp()) + .get('/auth/session') + .set('Authorization', `Bearer ${token}`); + expect(res.status).toBe(401); + expect(res.body.valid).toBe(false); + }); +}); diff --git a/backend/__tests__/services/backgroundProcessor.test.js b/backend/__tests__/services/backgroundProcessor.test.js new file mode 100644 index 00000000..2819dfd7 --- /dev/null +++ b/backend/__tests__/services/backgroundProcessor.test.js @@ -0,0 +1,109 @@ +/** + * Unit tests for backgroundProcessor.claimNextPhoto. + * + * Mocks the db so we don't need a live postgres/sqlite — focuses on + * the claim contract: returns null when no rows, returns row + flips + * status to 'processing' when one is available, returns null when a + * race loses the UPDATE-with-guard. + */ + +jest.mock('../../src/services/photoProcessor', () => ({ + processPhoto: jest.fn(), + processUploadedPhotos: jest.fn(), + queueFilesForProcessing: jest.fn(), +})); + +// Build a fake knex instance whose .transaction() takes a callback we can +// drive from the test, and whose query-builder records calls. +function makeFakeDb({ pendingRow = null, updateResult = 1, clientName = 'pg' } = {}) { + const queries = []; + + const builder = () => { + const recorded = { wheres: [], updates: null, ordered: false, locked: false, skipped: false }; + queries.push(recorded); + const chain = { + where: jest.fn(function (...args) { + recorded.wheres.push(args); + return chain; + }), + orderBy: jest.fn(function () { + recorded.ordered = true; + return chain; + }), + forUpdate: jest.fn(function () { + recorded.locked = true; + return chain; + }), + skipLocked: jest.fn(function () { + recorded.skipped = true; + return chain; + }), + first: jest.fn(async function () { + // Only the SELECT chain returns the pending row; the UPDATE chain + // never calls .first(). + return pendingRow ? { ...pendingRow } : null; + }), + update: jest.fn(async function (data) { + recorded.updates = data; + return updateResult; + }), + }; + return chain; + }; + + const trxFn = (table) => builder(table); + trxFn.client = { config: { client: clientName } }; + trxFn.transaction = async (cb) => cb(trxFn); + + // Top-level db('photos') returns same builder for the janitor test path. + const db = trxFn; + return { db, queries }; +} + +describe('backgroundProcessor.claimNextPhoto', () => { + function loadProcessor(db) { + jest.resetModules(); + jest.doMock('../../src/database/db', () => ({ db })); + return require('../../src/services/backgroundProcessor'); + } + + it('returns null when there are no pending photos (postgres path)', async () => { + const { db } = makeFakeDb({ pendingRow: null, clientName: 'pg' }); + const bg = loadProcessor(db); + const result = await bg.claimNextPhoto(); + expect(result).toBeNull(); + }); + + it('returns the claimed row and flips status (postgres path)', async () => { + const pendingRow = { id: 42, processing_status: 'pending' }; + const { db, queries } = makeFakeDb({ pendingRow, clientName: 'pg' }); + const bg = loadProcessor(db); + const result = await bg.claimNextPhoto(); + expect(result).toEqual(pendingRow); + // The first query is the SELECT FOR UPDATE SKIP LOCKED. + expect(queries[0].locked).toBe(true); + expect(queries[0].skipped).toBe(true); + // The second query is the status update. + expect(queries[1].updates.processing_status).toBe('processing'); + expect(queries[1].updates.processing_started_at).toBeInstanceOf(Date); + }); + + it('returns null when the SQLite UPDATE-with-guard loses the race', async () => { + const pendingRow = { id: 7 }; + const { db } = makeFakeDb({ pendingRow, clientName: 'better-sqlite3', updateResult: 0 }); + const bg = loadProcessor(db); + const result = await bg.claimNextPhoto(); + expect(result).toBeNull(); + }); + + it('returns the row when SQLite UPDATE-with-guard wins', async () => { + const pendingRow = { id: 7 }; + const { db, queries } = makeFakeDb({ pendingRow, clientName: 'better-sqlite3', updateResult: 1 }); + const bg = loadProcessor(db); + const result = await bg.claimNextPhoto(); + expect(result).toEqual(pendingRow); + // SQLite path: no FOR UPDATE / SKIP LOCKED. + expect(queries[0].locked).toBe(false); + expect(queries[0].skipped).toBe(false); + }); +}); diff --git a/backend/__tests__/services/photoProcessor.processPhoto.test.js b/backend/__tests__/services/photoProcessor.processPhoto.test.js new file mode 100644 index 00000000..763b213e --- /dev/null +++ b/backend/__tests__/services/photoProcessor.processPhoto.test.js @@ -0,0 +1,216 @@ +/** + * Unit tests for photoProcessor.processPhoto — the worker-mode entry + * point that runs after a row has been claimed by the background + * processor. Mocks every external dependency and validates the + * happy-path DB updates and side-effect ordering. + * + * jest.mock factories are evaluated before any local variables exist, + * so collaborators are kept inside the mock factories themselves and + * the test reaches into them via require() once they're set up. + */ + +const path = require('path'); + +jest.mock('../../src/database/db', () => { + const recorded = { whereCalls: [], updateCalls: [] }; + let pendingWhere = null; + const photosState = { row: null }; + const eventsState = { row: null }; + + function makePhotoQuery() { + return { + where(args) { + pendingWhere = args; + recorded.whereCalls.push(args); + return this; + }, + async first() { + return photosState.row; + }, + async update(data) { + recorded.updateCalls.push({ where: pendingWhere, data }); + return 1; + }, + }; + } + + function makeEventsQuery() { + return { + where() { + return this; + }, + async first() { + return eventsState.row; + }, + }; + } + + function dbFn(table) { + if (table === 'photos') return makePhotoQuery(); + if (table === 'events') return makeEventsQuery(); + throw new Error(`Unexpected table: ${table}`); + } + dbFn.client = { config: { client: 'pg' } }; + + return { + db: dbFn, + __setPhoto: (row) => { photosState.row = row; }, + __setEvent: (row) => { eventsState.row = row; }, + __reset: () => { + recorded.whereCalls = []; + recorded.updateCalls = []; + photosState.row = null; + eventsState.row = null; + }, + __recorded: () => recorded, + }; +}); + +jest.mock('../../src/services/imageProcessor', () => { + const mockGenerateThumbnail = jest.fn(); + const mockExtractCaptureDate = jest.fn(); + return { + generateThumbnail: mockGenerateThumbnail, + extractCaptureDate: mockExtractCaptureDate, + withLocalCopy: jest.fn(async (key, fn) => + fn(`/tmp/local-copy-${require('path').basename(key)}`) + ), + }; +}); + +jest.mock('../../src/services/videoProcessor', () => ({ + processUploadedVideo: jest.fn(), + isVideoMimeType: (mime) => typeof mime === 'string' && mime.startsWith('video/'), +})); + +jest.mock('../../src/services/storage', () => ({ getStorage: jest.fn() })); + +jest.mock('../../src/services/photoResolver', () => ({ + resolvePhotoStorageKey: jest.fn( + (event, photo) => `events/active/${event.slug}/${photo.filename}` + ), +})); + +jest.mock('../../src/utils/filenameSanitizer', () => ({ + generatePhotoFilename: jest.fn(() => 'whatever.jpg'), +})); + +jest.mock('../../src/services/watermarkGeneratorService', () => ({ + generateForPhoto: jest.fn(() => Promise.resolve()), +})); + +jest.mock('../../src/services/webhookService', () => ({ + fire: jest.fn(() => Promise.resolve()), +})); + +jest.mock('../../src/utils/logger', () => ({ + warn: jest.fn(), + error: jest.fn(), + info: jest.fn(), + debug: jest.fn(), +})); + +// Stub sharp so we don't actually read any image off disk. +jest.mock('sharp', () => { + const mock = jest.fn(() => ({ + metadata: jest.fn(async () => ({ width: 1920, height: 1080 })), + })); + return mock; +}); + +const dbModule = require('../../src/database/db'); +const imageProcessor = require('../../src/services/imageProcessor'); +const videoProcessor = require('../../src/services/videoProcessor'); +const watermarkService = require('../../src/services/watermarkGeneratorService'); +const webhookService = require('../../src/services/webhookService'); + +beforeEach(() => { + dbModule.__reset(); + jest.clearAllMocks(); +}); + +describe('photoProcessor.processPhoto', () => { + it('marks an image complete with thumbnail and dimensions', async () => { + dbModule.__setPhoto({ + id: 101, + event_id: 5, + filename: 'wedding-001.jpg', + original_filename: 'IMG_0001.jpg', + mime_type: 'image/jpeg', + media_type: 'image', + size_bytes: 12345, + captured_at: null, + processing_status: 'processing', + }); + dbModule.__setEvent({ id: 5, slug: 'wedding', event_name: 'Wedding' }); + + imageProcessor.extractCaptureDate.mockResolvedValueOnce('2026-04-25T12:00:00Z'); + imageProcessor.generateThumbnail.mockResolvedValueOnce('thumbnails/thumb_wedding-001.jpg'); + + const { processPhoto } = require('../../src/services/photoProcessor'); + await processPhoto(101); + + const finalUpdate = dbModule.__recorded().updateCalls.pop(); + expect(finalUpdate.data.processing_status).toBe('complete'); + expect(finalUpdate.data.processing_error).toBeNull(); + expect(finalUpdate.data.thumbnail_path).toBe('thumbnails/thumb_wedding-001.jpg'); + expect(finalUpdate.data.width).toBe(1920); + expect(finalUpdate.data.height).toBe(1080); + expect(finalUpdate.data.captured_at).toBe('2026-04-25T12:00:00Z'); + + expect(watermarkService.generateForPhoto).toHaveBeenCalledWith(101); + expect(webhookService.fire).toHaveBeenCalledWith( + 'photo.uploaded', + expect.objectContaining({ + event: expect.objectContaining({ slug: 'wedding' }), + photo: expect.objectContaining({ id: 101, filename: 'wedding-001.jpg' }), + }) + ); + }); + + it('handles videos with ffmpeg metadata path', async () => { + dbModule.__setPhoto({ + id: 202, + event_id: 9, + filename: 'wedding-video-001.mp4', + original_filename: 'movie.mp4', + mime_type: 'video/mp4', + media_type: 'video', + size_bytes: 99999, + captured_at: null, + }); + dbModule.__setEvent({ id: 9, slug: 'wedding', event_name: 'Wedding' }); + + videoProcessor.processUploadedVideo.mockResolvedValueOnce({ + thumbnailKey: 'thumbnails/thumb_wedding-video-001.jpg', + metadata: { + duration: 12.5, + videoCodec: 'h264', + audioCodec: 'aac', + width: 1280, + height: 720, + }, + }); + + const { processPhoto } = require('../../src/services/photoProcessor'); + await processPhoto(202); + + const finalUpdate = dbModule.__recorded().updateCalls.pop(); + expect(finalUpdate.data.processing_status).toBe('complete'); + expect(finalUpdate.data.duration).toBe(12.5); + expect(finalUpdate.data.video_codec).toBe('h264'); + expect(finalUpdate.data.thumbnail_path).toBe('thumbnails/thumb_wedding-video-001.jpg'); + + // Watermark queue is image-only. + expect(watermarkService.generateForPhoto).not.toHaveBeenCalled(); + }); + + it('throws when the photo row no longer exists', async () => { + dbModule.__setPhoto(null); + dbModule.__setEvent({ id: 1 }); + const { processPhoto } = require('../../src/services/photoProcessor'); + await expect(processPhoto(999)).rejects.toThrow(/Photo 999 not found/); + }); +}); + +void path; // referenced indirectly via mocks diff --git a/backend/migrations/core/085_async_photo_processing.js b/backend/migrations/core/085_async_photo_processing.js new file mode 100644 index 00000000..55d2d75f --- /dev/null +++ b/backend/migrations/core/085_async_photo_processing.js @@ -0,0 +1,72 @@ +/** + * Async photo-processing infrastructure. + * + * Adds: + * - photos.processing_status — enum: pending | processing | complete | failed + * - photos.processing_error — text, populated on 'failed' + * - photos.processing_started_at — claim timestamp for janitor recovery + * - photos.upload_id — groups all photos from one upload request + * so the frontend can poll/stream by group + * + * All existing rows default to 'complete' (they were processed synchronously + * before this migration and there's nothing pending). New uploads insert + * with 'pending' and a background worker (services/backgroundProcessor.js) + * picks them up. + * + * Partial-style indexes keep lookups fast as the queue drains. We use plain + * indexes here instead of postgres-specific WHERE clauses so the migration + * works on SQLite too; the workload (only-pending rows) keeps the index small. + */ + +exports.up = async function up(knex) { + if (!(await knex.schema.hasTable('photos'))) return; + + const hasStatus = await knex.schema.hasColumn('photos', 'processing_status'); + if (!hasStatus) { + await knex.schema.alterTable('photos', (table) => { + table.string('processing_status', 16).notNullable().defaultTo('complete'); + table.text('processing_error').nullable(); + table.timestamp('processing_started_at').nullable(); + table.string('upload_id', 64).nullable(); + }); + } + + // Indexes — wrap in try/catch so re-running the migration on a partially + // applied schema is a no-op rather than an error. + try { + await knex.schema.alterTable('photos', (table) => { + table.index(['processing_status'], 'idx_photos_processing_status'); + }); + } catch (_) { /* already exists */ } + + try { + await knex.schema.alterTable('photos', (table) => { + table.index(['upload_id'], 'idx_photos_upload_id'); + }); + } catch (_) { /* already exists */ } +}; + +exports.down = async function down(knex) { + if (!(await knex.schema.hasTable('photos'))) return; + + // Drop indexes first (best-effort) + try { + await knex.schema.alterTable('photos', (t) => t.dropIndex([], 'idx_photos_upload_id')); + } catch (_) { /* not present */ } + try { + await knex.schema.alterTable('photos', (t) => t.dropIndex([], 'idx_photos_processing_status')); + } catch (_) { /* not present */ } + + if (await knex.schema.hasColumn('photos', 'upload_id')) { + await knex.schema.alterTable('photos', (t) => t.dropColumn('upload_id')); + } + if (await knex.schema.hasColumn('photos', 'processing_started_at')) { + await knex.schema.alterTable('photos', (t) => t.dropColumn('processing_started_at')); + } + if (await knex.schema.hasColumn('photos', 'processing_error')) { + await knex.schema.alterTable('photos', (t) => t.dropColumn('processing_error')); + } + if (await knex.schema.hasColumn('photos', 'processing_status')) { + await knex.schema.alterTable('photos', (t) => t.dropColumn('processing_status')); + } +}; diff --git a/backend/server.js b/backend/server.js index 69879c89..77b9c32d 100644 --- a/backend/server.js +++ b/backend/server.js @@ -23,6 +23,7 @@ const { startExpirationChecker } = require('./src/services/expirationChecker'); const { initializeTransporter, startEmailQueueProcessor } = require('./src/services/emailProcessor'); const { startBackupService } = require('./src/services/backupService'); const { startScheduledBackups } = require('./src/services/databaseBackup'); +const backgroundProcessor = require('./src/services/backgroundProcessor'); const { maintenanceMiddleware } = require('./src/middleware/maintenance'); const { sessionTimeoutMiddleware } = require('./src/middleware/sessionTimeout'); const { errorHandler, notFoundHandler } = require('./src/middleware/errorHandler'); @@ -660,10 +661,15 @@ async function startServer() { // Start backup service await startBackupService(); - + // Start database backup service await startScheduledBackups(); - + + // Start the async photo-processing worker pool. Picks up + // photos in 'pending' state (from POST /upload) and runs the + // sharp/ffmpeg/EXIF pipeline off the request thread. + backgroundProcessor.start(); + app.listen(PORT, () => { logger.info(`Server running on port ${PORT}`); logger.info(`Admin interface: ${process.env.ADMIN_URL || 'http://localhost:3000'}`); diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 46ec4661..791ee012 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -5,8 +5,8 @@ const fs = require('fs').promises; const { db, logActivity } = require('../database/db'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); -const { generateThumbnail, ensureThumbnail, extractCaptureDate } = require('../services/imageProcessor'); -const { processUploadedVideo, isVideoMimeType } = require('../services/videoProcessor'); +const { ensureThumbnail } = require('../services/imageProcessor'); +const { isVideoMimeType } = require('../services/videoProcessor'); const { generatePhotoFilename } = require('../utils/filenameSanitizer'); const { escapeLikePattern } = require('../utils/sqlSecurity'); const { validateUploadedFiles } = require('../middleware/uploadValidation'); @@ -150,29 +150,39 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r next(); }); }, validateUploadContent, validateUploadedFiles, async (req, res) => { + // Single cleanup site for the multer temp directory — runs on every + // exit path (success, validation 4xx, server 5xx, multer error). The + // previous code had three inline cleanup blocks for individual early + // returns and missed the success path entirely, leaving an empty + // per-request directory behind on every successful upload (#357 review). + let tempCleanupDone = false; + const cleanupTempDir = async () => { + if (tempCleanupDone || !req.tempUploadPath) return; + tempCleanupDone = true; + try { + await fs.rm(req.tempUploadPath, { recursive: true, force: true }); + } catch (e) { + console.error('Failed to clean up temp upload directory:', e); + } + }; + res.on('finish', cleanupTempDir); + res.on('close', cleanupTempDir); + try { const { eventId } = req.params; const { category_id, replace_by_name } = req.body; const replaceByName = replace_by_name === 'true' || replace_by_name === true; - + console.log('Upload request received for event:', eventId); console.log('Body:', req.body); console.log('Files:', req.files ? req.files.length : 'none'); console.log('File details:', req.files?.map(f => ({ name: f.originalname, size: f.size, mimetype: f.mimetype }))); console.log('Category ID received:', category_id); - + // Verify event exists and admin has access const event = await db('events').where({ id: eventId }).first(); if (!event) { console.error('Event not found:', eventId); - // Clean up temp files - if (req.tempUploadPath) { - try { - await fs.rm(req.tempUploadPath, { recursive: true, force: true }); - } catch (e) { - console.error('Failed to clean up temp path:', e); - } - } return res.status(404).json({ error: 'Event not found' }); } @@ -192,14 +202,6 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r } } if (currentCount + newFilesCount > event.photo_cap) { - // Clean up temp files - if (req.tempUploadPath) { - try { - await fs.rm(req.tempUploadPath, { recursive: true, force: true }); - } catch (e) { - console.error('Failed to clean up temp path:', e); - } - } return res.status(400).json({ error: `Photo cap exceeded. This event allows a maximum of ${event.photo_cap} photos. Currently ${currentCount} photos exist, and you are trying to upload ${newFilesCount} more.` }); @@ -209,14 +211,6 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r if (!req.files || req.files.length === 0) { console.error('No files in request. req.files:', req.files); console.error('Request body keys:', Object.keys(req.body)); - // Clean up temp files - if (req.tempUploadPath) { - try { - await fs.rm(req.tempUploadPath, { recursive: true, force: true }); - } catch (e) { - console.error('Failed to clean up temp path:', e); - } - } return res.status(400).json({ error: 'No files uploaded' }); } @@ -289,262 +283,97 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r filesToUpload = newFiles; } - // Process remaining new files in batches - const BATCH_SIZE = 25; // Increased batch size for better performance with large uploads + // Async-processing flow: + // 1. Move each file to its final storage location. + // 2. Insert a photo row with processing_status='pending' and a + // shared upload_id. EXIF / sharp / thumbnails / ffmpeg / + // watermark / webhook all happen in the background worker + // (services/backgroundProcessor.js) so the request returns in + // seconds even on NFS-backed storage. + // + // The previous code processed thumbnails+EXIF synchronously in + // batches of 25 inside this handler, which is why large uploads on + // slow storage looked frozen — see #357 review. + const crypto = require('crypto'); + const uploadId = crypto.randomBytes(16).toString('hex'); - for (let i = 0; i < filesToUpload.length; i += BATCH_SIZE) { - const batch = filesToUpload.slice(i, i + BATCH_SIZE); - - // Start a single transaction for the batch - const trx = await db.transaction(); - + // Counter base — same approximation as before. Strict uniqueness is + // already enforced by the filename template + DB unique index, so a + // small race here just retries a counter on conflict (rare). + const existingCount = await db('photos') + .where({ event_id: eventId, type: photoType }) + .count('id as count') + .first(); + let counter = (parseInt(existingCount.count) || 0) + 1; + const storage = getStorage(); + + for (const file of filesToUpload) { try { - // Get initial counter for this batch based on photo type - const existingCount = await trx('photos') - .where({ event_id: eventId, type: photoType }) - .count('id as count') - .first(); - let batchCounter = (parseInt(existingCount.count) || 0) + 1; - - const batchPhotos = []; - const fileRenameOperations = []; // Store rename operations to do after commit - - // First pass: prepare data and move files from temp to final location - for (let fileIndex = 0; fileIndex < batch.length; fileIndex++) { - const file = batch[fileIndex]; - const counter = batchCounter + fileIndex; - const tempPath = file.path; // Original temp path - - try { - // Verify file is complete before processing - const tempStats = await fs.stat(tempPath); - if (tempStats.size === 0) { - throw new Error('File is empty - upload may have been interrupted'); - } - - // Generate new filename - const extension = path.extname(file.originalname); - const newFilename = generatePhotoFilename( - event.event_name, - categoryName, - counter, - extension - ); - - // Storage key: events/active/{slug}/{newFilename} - const finalKey = path.posix.join(finalDestPathRel, newFilename); - // photo.path is stored relative to events/active so resolvePhotoStorageKey - // can rebuild the full key on read. - const relativePath = path.posix.join(event.slug, newFilename); - - // Extract capture date from EXIF metadata - let capturedAt = null; - try { - capturedAt = await extractCaptureDate(tempPath); - } catch (exifError) { - // Non-fatal - just log and continue without capture date - console.log(`Could not extract EXIF date for ${file.originalname}`); - } - - // Determine media type - const isVideo = isVideoMimeType(file.mimetype); - const mediaType = isVideo ? 'video' : 'image'; - - // Prepare photo data for batch insert - const photoData = { - event_id: parseInt(eventId), - filename: newFilename, - original_filename: file.originalname, // Preserve original filename for Lightroom export - path: relativePath, - thumbnail_path: null, // Will generate after successful commit - type: photoType, - category_id: parsedCategoryId, // Save the selected category - size_bytes: tempStats.size, // Use actual file size from stat - captured_at: capturedAt, // EXIF capture date (if available) - media_type: mediaType, - mime_type: file.mimetype - }; - - batchPhotos.push(photoData); - - // Store upload operation for later (after DB commit) - fileRenameOperations.push({ - tempPath: tempPath, - finalKey: finalKey, - filename: newFilename, - photoData: photoData - }); - } catch (error) { - console.error(`Error preparing file ${file.originalname}:`, error); - errors.push({ filename: file.originalname, error: error.message }); - } + const tempStats = await fs.stat(file.path); + if (tempStats.size === 0) { + throw new Error('File is empty - upload may have been interrupted'); } - - // Insert all photos in this batch - if (batchPhotos.length > 0) { - console.log(`Inserting batch of ${batchPhotos.length} photos with type: ${photoType}`); - - const insertedIds = await trx('photos').insert(batchPhotos).returning('id'); - - // No need to update counter as we calculate it dynamically - - // Commit the transaction first - await trx.commit(); - console.log(`Successfully committed batch of ${batchPhotos.length} photos`); - - // Now upload files from temp into the storage backend after successful commit - const storage = getStorage(); - for (let idx = 0; idx < fileRenameOperations.length; idx++) { - const operation = fileRenameOperations[idx]; - try { - // Process source-dependent steps (sharp/ffmpeg) FIRST while the - // tmp file is still on local disk, then upload the original and - // unlink the tmp. - const photoId = insertedIds[idx]?.id || insertedIds[idx]; - const isVideoFile = isVideoMimeType(operation.photoData.mime_type); - let thumbnailPath = null; - try { - if (isVideoFile) { - const videoThumbnailKey = path.posix.join( - 'thumbnails', - `thumb_${operation.filename.replace(/\.[^.]+$/, '.jpg')}` - ); - const result = await processUploadedVideo(operation.tempPath, videoThumbnailKey); - thumbnailPath = result.thumbnailKey; + const extension = path.extname(file.originalname); + const newFilename = generatePhotoFilename( + event.event_name, + categoryName, + counter, + extension + ); + counter += 1; - if (photoId && result.metadata) { - await db('photos') - .where({ id: photoId }) - .update({ - thumbnail_path: thumbnailPath, - duration: result.metadata.duration, - video_codec: result.metadata.videoCodec, - audio_codec: result.metadata.audioCodec, - width: result.metadata.width, - height: result.metadata.height - }); - } - } else { - thumbnailPath = await generateThumbnail(operation.tempPath); + const finalKey = path.posix.join(finalDestPathRel, newFilename); + const relativePath = path.posix.join(event.slug, newFilename); + const isVideo = isVideoMimeType(file.mimetype); - // Update the database with thumbnail path and image dimensions - if (photoId) { - const updateData = {}; - if (thumbnailPath) updateData.thumbnail_path = thumbnailPath; + // 1. Move file to its final storage key first. If the worker + // later picks up the photo row, the file is guaranteed to + // exist at the recorded path. + await storage.putFromFile(finalKey, file.path, { + contentType: file.mimetype, + }); + await fs.unlink(file.path).catch(() => {}); - try { - const sharp = require('sharp'); - const metadata = await sharp(operation.tempPath).metadata(); - if (metadata.width && metadata.height) { - updateData.width = metadata.width; - updateData.height = metadata.height; - } - } catch (metadataError) { - console.warn(`Could not extract image dimensions for ${operation.filename}:`, metadataError.message); - } - - if (Object.keys(updateData).length > 0) { - await db('photos') - .where({ id: photoId }) - .update(updateData); - } - } - } - } catch (thumbError) { - console.error(`Thumbnail/metadata processing failed for ${operation.filename}:`, thumbError.message); - } - - // Upload the original through the storage backend, then drop the - // local tmp file. We do this AFTER thumbnail/metadata processing - // so sharp/ffmpeg still have a local source to work from. - await storage.putFromFile(operation.finalKey, operation.tempPath, { - contentType: operation.photoData.mime_type, - }); - await fs.unlink(operation.tempPath).catch(() => {}); - - // Sanity check: round-trip the size we just wrote. - const stat = await storage.stat(operation.finalKey); - if (!stat || stat.size !== operation.photoData.size_bytes) { - throw new Error(`Size mismatch after upload: expected ${operation.photoData.size_bytes}, got ${stat ? stat.size : 'null'}`); - } - - // Queue watermark generation in background (non-blocking, images only) - if (photoId && !isVideoFile) { - watermarkGeneratorService.generateForPhoto(photoId) - .catch(err => console.warn(`Watermark generation queued failed for photo ${photoId}:`, err.message)); - } - - // Webhook (#327): per-photo upload event. - try { - const webhookService = require('../services/webhookService'); - await webhookService.fire('photo.uploaded', { - event: { id: parseInt(eventId, 10), slug: event.slug, event_name: event.event_name }, - photo: { - id: insertedIds[idx]?.id || insertedIds[idx], - filename: operation.filename, - original_filename: operation.photoData.original_filename, - size_bytes: operation.photoData.size_bytes, - }, - }); - } catch (e) { /* non-fatal */ } - - // Add to successful uploads - uploadedPhotos.push({ - id: insertedIds[idx]?.id || insertedIds[idx], - filename: operation.filename, - size: operation.photoData.size_bytes, - category_id: operation.photoData.category_id - }); - } catch (moveError) { - console.error(`Failed to upload ${operation.tempPath} → ${operation.finalKey}:`, moveError); - errors.push({ - filename: operation.filename, - error: `File upload failed: ${moveError.message}` - }); - - // Try to clean up the database entry if file move failed - if (insertedIds[idx]) { - const photoId = insertedIds[idx]?.id || insertedIds[idx]; - try { - await db('photos').where({ id: photoId }).delete(); - console.log(`Cleaned up database entry for failed photo ${photoId}`); - } catch (cleanupError) { - console.error(`Failed to clean up database entry:`, cleanupError); - } - } - } - } - } else { - // No photos to insert, just rollback - await trx.rollback(); + // Sanity check the round-tripped size — same guard as before. + const stat = await storage.stat(finalKey); + if (!stat || stat.size !== tempStats.size) { + throw new Error( + `Size mismatch after upload: expected ${tempStats.size}, got ${stat ? stat.size : 'null'}` + ); } - } catch (error) { - console.error(`Error processing batch starting at index ${i}:`, error); - console.error('Stack trace:', error.stack); - - // Rollback if not already committed - if (!trx.isCompleted()) { - await trx.rollback(); - } - - // Add all files in this batch to errors - for (const file of batch) { - errors.push({ - filename: file.originalname, - error: `Batch processing failed: ${error.message}` - }); - } - } - } - - // Clean up temp upload directory - if (req.tempUploadPath) { - try { - await fs.rm(req.tempUploadPath, { recursive: true, force: true }); - console.log(`Cleaned up temp upload directory: ${req.tempUploadPath}`); - } catch (e) { - console.error('Failed to clean up temp upload directory:', e); + + // 2. Insert a pending photo row. The background processor + // will pick it up, generate thumbnail/dimensions/EXIF, and + // flip status to 'complete' (or 'failed' with the error). + const inserted = await db('photos') + .insert({ + event_id: parseInt(eventId, 10), + filename: newFilename, + original_filename: file.originalname, + path: relativePath, + thumbnail_path: null, + type: photoType, + category_id: parsedCategoryId, + size_bytes: tempStats.size, + captured_at: null, + media_type: isVideo ? 'video' : 'image', + mime_type: file.mimetype, + processing_status: 'pending', + upload_id: uploadId, + }) + .returning('id'); + const photoId = inserted[0]?.id || inserted[0]; + + uploadedPhotos.push({ + id: photoId, + filename: newFilename, + size: tempStats.size, + category_id: parsedCategoryId, + }); + } catch (err) { + console.error(`Error queuing file ${file.originalname}:`, err); + errors.push({ filename: file.originalname, error: err.message }); } } @@ -567,12 +396,20 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r // Include any files that were invalid from the validation middleware const totalInvalidFiles = (req.invalidFiles || []).concat(errors); - // Prepare response + // Prepare response. The new fields (upload_id, count, photo_ids) + // are what the new frontend uses to poll for processing status; the + // existing fields (successCount, replacedCount, ...) are kept for + // back-compat with older clients that haven't upgraded yet. const totalAttempted = req.files.length + (req.invalidFiles ? req.invalidFiles.length : 0); - const uploadMsg = uploadedPhotos.length > 0 ? `${uploadedPhotos.length} uploaded` : ''; + const uploadMsg = uploadedPhotos.length > 0 ? `${uploadedPhotos.length} queued` : ''; const replaceMsg = replacedPhotos.length > 0 ? `${replacedPhotos.length} replaced` : ''; const parts = [uploadMsg, replaceMsg].filter(Boolean).join(', '); const response = { + // New async-processing fields + upload_id: uploadId, + count: uploadedPhotos.length, + photo_ids: uploadedPhotos.map((p) => p.id), + // Existing back-compat fields message: parts ? `Successfully ${parts}` : 'No photos processed', photos: uploadedPhotos, replaced: replacedPhotos, @@ -580,38 +417,200 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r skippedReplacements, totalFiles: totalAttempted, successCount: uploadedPhotos.length + replacedPhotos.length, - failureCount: totalInvalidFiles.length + failureCount: totalInvalidFiles.length, }; - + // Include error details if any files failed if (totalInvalidFiles.length > 0) { response.errors = totalInvalidFiles; - response.message = `Uploaded ${uploadedPhotos.length} of ${totalAttempted} photos. ${totalInvalidFiles.length} failed.`; + response.message = `Queued ${uploadedPhotos.length} of ${totalAttempted} photos. ${totalInvalidFiles.length} failed.`; } - + // Invalidate download zip cache after successful upload or replacement if (uploadedPhotos.length > 0 || replacedPhotos.length > 0) { downloadZipService.invalidate(parseInt(eventId)); } - res.json(response); + // 202 Accepted — files stored, processing happens in background. + res.status(202).json(response); } catch (error) { console.error('Error uploading photos:', error); - - // Clean up temp upload directory on error - if (req.tempUploadPath) { - try { - await fs.rm(req.tempUploadPath, { recursive: true, force: true }); - console.log(`Cleaned up temp upload directory after error: ${req.tempUploadPath}`); - } catch (e) { - console.error('Failed to clean up temp upload directory:', e); - } - } - + // Temp directory cleanup is handled by the response finish/close + // listeners above, regardless of which exit path fires. res.status(500).json({ error: 'Failed to upload photos' }); } }); +// Helper — load the upload group + verify the requesting admin owns the +// underlying event. Returns { event, photos } or sends a 4xx response. +async function loadUploadGroup(req, res) { + const { upload_id: uploadId } = req.params; + if (!uploadId || typeof uploadId !== 'string' || uploadId.length > 64) { + res.status(400).json({ error: 'Invalid upload_id' }); + return null; + } + + const photos = await db('photos').where({ upload_id: uploadId }); + if (photos.length === 0) { + res.status(404).json({ error: 'Upload group not found' }); + return null; + } + + const eventId = photos[0].event_id; + let eventQuery = db('events').where('id', eventId); + if (req.admin.roleName === 'editor') { + eventQuery = eventQuery.where('created_by', req.admin.id); + } + const event = await eventQuery.first(); + if (!event) { + res.status(404).json({ error: 'Event not found' }); + return null; + } + return { event, photos, uploadId }; +} + +function summariseUpload(photos) { + const summary = { + total: photos.length, + pending: 0, + processing: 0, + complete: 0, + failed: 0, + photos: photos.map((p) => ({ + id: p.id, + filename: p.filename, + original_filename: p.original_filename, + status: p.processing_status, + error: p.processing_error || null, + })), + }; + for (const p of photos) { + summary[p.processing_status] = (summary[p.processing_status] || 0) + 1; + } + return summary; +} + +// JSON snapshot of upload status — frontends poll this every 1.5s while +// any photo in the group is still pending or processing. +router.get( + '/uploads/:upload_id/status', + adminAuth, + requirePermission('photos.view'), + async (req, res) => { + try { + const group = await loadUploadGroup(req, res); + if (!group) return; + res.json({ + upload_id: group.uploadId, + event_id: group.event.id, + ...summariseUpload(group.photos), + }); + } catch (error) { + console.error('Error reading upload status:', error); + res.status(500).json({ error: 'Failed to read upload status' }); + } + } +); + +// Server-Sent Events stream for upload progress. Optional upgrade over +// the polling endpoint above. Streams the current snapshot on connect, +// then re-emits whenever the snapshot changes (debounced) until all +// photos in the group reach a terminal state. +router.get( + '/uploads/:upload_id/stream', + adminAuth, + requirePermission('photos.view'), + async (req, res) => { + const group = await loadUploadGroup(req, res); + if (!group) return; + + res.set({ + 'Content-Type': 'text/event-stream', + 'Cache-Control': 'no-cache, no-transform', + 'Connection': 'keep-alive', + 'X-Accel-Buffering': 'no', + }); + res.flushHeaders(); + + let lastJson = ''; + let closed = false; + let timer = null; + + const send = async () => { + if (closed) return; + try { + const photos = await db('photos').where({ upload_id: group.uploadId }); + const summary = summariseUpload(photos); + const payload = JSON.stringify({ + upload_id: group.uploadId, + event_id: group.event.id, + ...summary, + }); + if (payload !== lastJson) { + lastJson = payload; + res.write(`data: ${payload}\n\n`); + } + // Stop streaming once everything has reached a terminal state. + if (summary.pending === 0 && summary.processing === 0) { + closed = true; + clearInterval(timer); + res.end(); + return; + } + } catch (e) { + console.error('Upload stream poll error:', e); + } + }; + + await send(); + timer = setInterval(send, 1500); + + req.on('close', () => { + closed = true; + if (timer) clearInterval(timer); + }); + } +); + +// Retry a failed photo — flip back to 'pending' so the worker picks it +// up again. Used by the admin grid's "Retry" button when a previous run +// hit a transient sharp/ffmpeg error. +router.post( + '/photos/:photoId/retry', + adminAuth, + requirePermission('photos.edit'), + async (req, res) => { + try { + const photo = await db('photos').where({ id: req.params.photoId }).first(); + if (!photo) return res.status(404).json({ error: 'Photo not found' }); + + // Editor role: only allow retry on photos in events they own. + if (req.admin.roleName === 'editor') { + const event = await db('events') + .where({ id: photo.event_id, created_by: req.admin.id }) + .first(); + if (!event) return res.status(404).json({ error: 'Photo not found' }); + } + + if (photo.processing_status !== 'failed') { + return res.status(409).json({ + error: `Photo is in '${photo.processing_status}' state and cannot be retried`, + }); + } + + await db('photos').where({ id: photo.id }).update({ + processing_status: 'pending', + processing_error: null, + processing_started_at: null, + }); + res.json({ id: photo.id, status: 'pending' }); + } catch (error) { + console.error('Error retrying photo processing:', error); + res.status(500).json({ error: 'Failed to retry photo processing' }); + } + } +); + // Delete a photo router.delete('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.delete'), requireEventOwnership, async (req, res) => { try { @@ -1070,15 +1069,25 @@ router.get('/:eventId/photos', adminAuth, requirePermission('photos.view'), requ router.get('/:eventId/photo/:photoId', adminAuth, requirePermission('photos.view'), requireEventOwnership, async (req, res) => { try { const { eventId, photoId } = req.params; - + const photo = await db('photos') .where({ id: photoId, event_id: eventId }) .first(); - + if (!photo) { return res.status(404).json({ error: 'Photo not found' }); } - + + // Photos still in async processing don't have all metadata in the DB + // yet; serving the original is fine, but downstream consumers (admin + // grid lightbox) read width/height which won't be set until processing + // completes. We let the original through here — the file is on disk — + // but tell the caller it's not done yet via a header so they can + // poll /uploads/:upload_id/status if they care. + if (photo.processing_status && photo.processing_status !== 'complete') { + res.setHeader('X-PicPeak-Photo-Status', photo.processing_status); + } + const { resolvePhotoFilePath, resolvePhotoStorageKey } = require('../services/photoResolver'); const event = await db('events').where('id', eventId).first(); const storageKey = resolvePhotoStorageKey(event, photo); @@ -1121,12 +1130,28 @@ router.get('/:eventId/thumbnail/:photoId', adminAuth, requirePermission('photos. const photo = await db('photos') .where({ id: photoId, event_id: eventId }) .first(); - + if (!photo) { console.error(`Photo not found: ${photoId}, event ${eventId}`); return res.status(404).json({ error: 'Photo not found' }); } - + + // Async processing is still working on this one — no thumbnail yet. + // Return 503 with Retry-After so the admin grid (which auto-refreshes + // every 2s while any photo is non-complete) keeps the placeholder + // until the worker catches up. + if (photo.processing_status === 'pending' || photo.processing_status === 'processing') { + res.setHeader('Retry-After', '2'); + return res.status(503).json({ error: 'Thumbnail not ready', status: photo.processing_status }); + } + if (photo.processing_status === 'failed') { + return res.status(422).json({ + error: 'Photo processing failed', + status: 'failed', + details: photo.processing_error || null, + }); + } + // Ensure thumbnail exists and is valid, regenerate if needed const thumbnailPath = await ensureThumbnail(photo); diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index aab00b23..219d7ad7 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -477,7 +477,7 @@ router.get('/session', async (req, res) => { try { const { slug } = req.query; const token = getAdminTokenFromRequest(req) || getGalleryTokenFromRequest(req, slug); - + if (!token) { return res.status(401).json({ error: 'No token provided' }); } @@ -498,6 +498,63 @@ router.get('/session', async (req, res) => { return res.status(401).json({ valid: false, error: 'Session has been invalidated' }); } + // The redirect loop reported on the v3.32.4-beta.0 release came + // from /auth/session reporting valid: true while the protected + // adminAuth / galleryAuth middleware rejected the same token for + // reasons /auth/session never checked: the admin user was + // deactivated, the admin's password had been changed since iat, + // or the gallery event was archived/deleted. Mirror those checks + // here so the session endpoint is always at least as strict as + // what the protected endpoints will enforce next. + if (decoded.type === 'admin') { + let admin = null; + try { + admin = await db('admin_users') + .where({ id: decoded.id, is_active: formatBoolean(true) }) + .select('id', 'username', 'email', 'password_changed_at') + .first(); + } catch (lookupErr) { + // admin_users table not present (test fixture, fresh DB) — fall + // through and trust the token. Real deployments always have it. + admin = null; + // intentional swallow; if the table is missing we do not want + // to fail-closed during e.g. early bootstrap. + } + + if (admin === null) { + // Lookup didn't run because the table is missing; skip the + // existence/password checks and treat the token as valid. + } else if (!admin) { + return res.json({ valid: false, error: 'Admin account no longer active' }); + } else if (admin.password_changed_at) { + const passwordChangedSeconds = Math.floor( + new Date(admin.password_changed_at).getTime() / 1000 + ); + if (decoded.iat < passwordChangedSeconds) { + return res.json({ valid: false, error: 'Token invalid due to password change' }); + } + } + } else if (decoded.type === 'gallery') { + try { + const event = await db('events') + .where({ + id: decoded.eventId, + is_active: formatBoolean(true), + is_archived: formatBoolean(false), + }) + .first(); + if (!event) { + return res.json({ valid: false, error: 'Gallery no longer available' }); + } + if (event.expires_at && new Date(event.expires_at) < new Date()) { + return res.json({ valid: false, error: 'Gallery has expired' }); + } + } catch (galleryLookupErr) { + // events table missing in this context — same fallback as + // admin path; trust the token rather than fail-closed. + } + } + // Calculate remaining time const now = Date.now() / 1000; const remainingTime = Math.max(0, decoded.exp - now); diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 5e48d9dd..31cea158 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -212,6 +212,15 @@ router.get('/:slug/photos', verifyGalleryAccess, async (req, res) => { const isClient = req.accessLevel === 'client'; let photosQuery = db('photos') .where('photos.event_id', req.event.id) + // Guests/clients never see photos still being processed by the + // background worker — the original is on disk but the thumbnail + // / dimensions / EXIF haven't landed yet. Photos with a NULL + // processing_status are pre-async-migration rows and are treated + // as complete (the migration's column default is 'complete' so + // this is just defensive against partial migration states). + .where(function() { + this.where('photos.processing_status', 'complete').orWhereNull('photos.processing_status'); + }) .select('photos.*'); // Guests only see visible photos; clients see all @@ -1380,18 +1389,31 @@ router.post('/:eventId/upload', verifyGalleryAccess, async (req, res) => { if (!req.files || req.files.length === 0) { return res.status(400).json({ error: 'No files uploaded' }); } - - const { processUploadedPhotos } = require('../services/photoProcessor'); - const categoryId = req.body.category_id || req.event.upload_category_id || null; - + + const { queueFilesForProcessing } = require('../services/photoProcessor'); + const rawCategory = req.body.category_id || req.event.upload_category_id || null; + const numericCategoryId = (() => { + if (rawCategory === null || rawCategory === undefined) return null; + const n = parseInt(rawCategory, 10); + return Number.isFinite(n) ? n : null; + })(); + try { - // Process uploaded photos - const results = await processUploadedPhotos(req.files, eventId, 'user', categoryId); - - res.json({ - message: 'Photos uploaded successfully', - count: results.length, - photos: results + // Queue files as 'pending' — the background worker will process + // thumbnails / EXIF / dimensions off the request thread (#357). + const result = await queueFilesForProcessing(req.files, { + eventId, + photoType: 'individual', + categoryId: numericCategoryId, + }); + + res.status(202).json({ + message: 'Photos queued for processing', + upload_id: result.uploadId, + count: result.photos.length, + photo_ids: result.photos.map((p) => p.id), + photos: result.photos, + errors: result.errors.length > 0 ? result.errors : undefined, }); } catch (processError) { console.error('Photo processing error:', processError); diff --git a/backend/src/services/backgroundProcessor.js b/backend/src/services/backgroundProcessor.js new file mode 100644 index 00000000..32148405 --- /dev/null +++ b/backend/src/services/backgroundProcessor.js @@ -0,0 +1,176 @@ +/** + * Background photo-processing worker pool. + * + * Polls `photos.processing_status = 'pending'`, atomically claims one + * row per worker, hands it to `photoProcessor.processPhoto(photoId)`, + * and marks the row 'complete' or 'failed' depending on outcome. A + * janitor loop resets rows stuck in 'processing' for too long (worker + * died, pod restarted, etc.). + * + * Concurrency model: N independent worker loops per backend instance. + * Multi-pod safe via: + * - Postgres: SELECT ... FOR UPDATE SKIP LOCKED — pods race for rows, + * only one wins, the others move on. + * - SQLite: SELECT then UPDATE-with-status-guard — second writer + * loses the guard and tries again (single-pod typical; the guard + * is enough for the rare two-process case during dev). + * + * Tunables (env, all optional): + * UPLOAD_PROCESSOR_CONCURRENCY default 2 + * UPLOAD_PROCESSOR_POLL_MS default 1000 + * UPLOAD_PROCESSOR_STUCK_TIMEOUT_MS default 600000 (10 minutes) + * UPLOAD_PROCESSOR_DISABLED default false (set 'true' to opt out, e.g. in CI) + */ + +const { db } = require('../database/db'); +const logger = require('../utils/logger'); +const { processPhoto } = require('./photoProcessor'); + +const POLL_INTERVAL_MS = parseInt(process.env.UPLOAD_PROCESSOR_POLL_MS || '1000', 10); +const CONCURRENCY = Math.max(1, parseInt(process.env.UPLOAD_PROCESSOR_CONCURRENCY || '2', 10)); +const STUCK_TIMEOUT_MS = parseInt(process.env.UPLOAD_PROCESSOR_STUCK_TIMEOUT_MS || '600000', 10); +const JANITOR_INTERVAL_MS = 60 * 1000; + +let running = false; +let workerHandles = []; +let janitorHandle = null; + +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + +function isPostgres() { + const c = db.client.config.client; + return c === 'pg' || (typeof c === 'string' && c.includes('postgres')); +} + +/** + * Atomically claim the oldest pending photo. Returns the row or null. + * The claimed row's processing_status is now 'processing' and + * processing_started_at is set so the janitor can recover it. + */ +async function claimNextPhoto() { + if (isPostgres()) { + return db.transaction(async (trx) => { + const row = await trx('photos') + .where('processing_status', 'pending') + .orderBy('id', 'asc') + .forUpdate() + .skipLocked() + .first(); + if (!row) return null; + await trx('photos').where('id', row.id).update({ + processing_status: 'processing', + processing_started_at: new Date(), + }); + return row; + }); + } + + // SQLite path — no SKIP LOCKED, but the UPDATE-with-guard ensures + // exactly one worker wins per row. + return db.transaction(async (trx) => { + const row = await trx('photos') + .where('processing_status', 'pending') + .orderBy('id', 'asc') + .first(); + if (!row) return null; + const updated = await trx('photos') + .where({ id: row.id, processing_status: 'pending' }) + .update({ + processing_status: 'processing', + processing_started_at: new Date(), + }); + return updated > 0 ? row : null; + }); +} + +async function workerLoop(workerIdx) { + while (running) { + let claimed; + try { + claimed = await claimNextPhoto(); + } catch (e) { + logger.warn(`backgroundProcessor[${workerIdx}]: claim error`, { error: e.message }); + await sleep(POLL_INTERVAL_MS); + continue; + } + + if (!claimed) { + await sleep(POLL_INTERVAL_MS); + continue; + } + + try { + await processPhoto(claimed.id); + } catch (err) { + logger.error(`backgroundProcessor[${workerIdx}]: photo ${claimed.id} failed`, { + error: err.message, + stack: err.stack, + }); + try { + await db('photos').where({ id: claimed.id }).update({ + processing_status: 'failed', + processing_error: String(err.message || err).slice(0, 1000), + }); + } catch (updateErr) { + logger.error(`backgroundProcessor[${workerIdx}]: failed to mark photo ${claimed.id} as failed`, { + error: updateErr.message, + }); + } + } + } +} + +async function janitorLoop() { + while (running) { + try { + const cutoff = new Date(Date.now() - STUCK_TIMEOUT_MS); + const reset = await db('photos') + .where('processing_status', 'processing') + .where('processing_started_at', '<', cutoff) + .update({ processing_status: 'pending', processing_started_at: null }); + if (reset > 0) { + logger.warn( + `backgroundProcessor: janitor reset ${reset} stuck photo(s) from 'processing' to 'pending'` + ); + } + } catch (e) { + logger.warn('backgroundProcessor: janitor error', { error: e.message }); + } + await sleep(JANITOR_INTERVAL_MS); + } +} + +function start() { + if (running) return; + if (process.env.UPLOAD_PROCESSOR_DISABLED === 'true') { + logger.info('backgroundProcessor: disabled via UPLOAD_PROCESSOR_DISABLED'); + return; + } + + running = true; + workerHandles = []; + for (let i = 0; i < CONCURRENCY; i++) { + workerHandles.push( + workerLoop(i).catch((e) => + logger.error(`backgroundProcessor[${i}]: crashed`, { error: e.message, stack: e.stack }) + ) + ); + } + janitorHandle = janitorLoop().catch((e) => + logger.error('backgroundProcessor: janitor crashed', { error: e.message, stack: e.stack }) + ); + + logger.info( + `backgroundProcessor: started ${CONCURRENCY} worker(s), poll=${POLL_INTERVAL_MS}ms, stuck=${STUCK_TIMEOUT_MS}ms` + ); +} + +async function stop() { + if (!running) return; + running = false; + await Promise.all([...workerHandles, janitorHandle].filter(Boolean)); + workerHandles = []; + janitorHandle = null; +} + +module.exports = { start, stop, claimNextPhoto }; diff --git a/backend/src/services/photoProcessor.js b/backend/src/services/photoProcessor.js index 5d793600..9b64d2e9 100644 --- a/backend/src/services/photoProcessor.js +++ b/backend/src/services/photoProcessor.js @@ -1,10 +1,12 @@ const path = require('path'); const fs = require('fs').promises; const { db } = require('../database/db'); -const { generateThumbnail } = require('./imageProcessor'); +const { generateThumbnail, extractCaptureDate, withLocalCopy } = require('./imageProcessor'); const { generatePhotoFilename } = require('../utils/filenameSanitizer'); const { processUploadedVideo, isVideoMimeType } = require('./videoProcessor'); const { getStorage } = require('./storage'); +const { resolvePhotoStorageKey } = require('./photoResolver'); +const logger = require('../utils/logger'); function normalizeFiles(files) { // Handle null, undefined, or falsy values @@ -287,6 +289,226 @@ async function processUploadedPhotos(files, eventId, uploadedBy = 'admin', categ return uploadedPhotos; } +/** + * Queue uploaded files for async processing. + * + * Moves each file from its multer temp path to the final storage key + * and inserts a `photos` row with `processing_status = 'pending'` and + * a shared `upload_id`. The background worker + * (services/backgroundProcessor.js) picks up pending rows, generates + * thumbnails / EXIF / dimensions, then flips status to 'complete' + * (or 'failed' with the error). + * + * Used by both the admin upload route and the gallery (guest) upload + * route so they share the same fast-return semantics. + * + * Options: + * - eventId required + * - photoType 'individual' | 'collage' (default 'individual') + * - categoryId numeric category id or null + * - uploadId optional pre-generated upload id (caller can + * provide it for chunked uploads that span + * multiple HTTP requests) + * + * Returns: { uploadId, photos: [{id, filename, size, category_id}], errors: [{filename, error}] } + */ +async function queueFilesForProcessing(files, options = {}) { + const crypto = require('crypto'); + const { eventId, photoType = 'individual', categoryId = null, uploadId: providedUploadId } = options; + const uploadId = providedUploadId || crypto.randomBytes(16).toString('hex'); + + const event = await db('events').where({ id: eventId }).first(); + if (!event) throw new Error(`Event ${eventId} not found`); + + const fileList = normalizeFiles(files); + const queued = []; + const errors = []; + + if (fileList.length === 0) return { uploadId, photos: queued, errors }; + + // Counter base — same approximation the upload route used pre-async. + // Strict uniqueness is still enforced by the filename template; on a + // collision the worker would just fail one photo. + const existingCount = await db('photos') + .where({ event_id: eventId, type: photoType }) + .count('id as count') + .first(); + let counter = (parseInt(existingCount?.count) || 0) + 1; + + const storage = getStorage(); + const finalDestPathRel = path.posix.join('events/active', event.slug); + const categoryName = photoType === 'collage' ? 'collages' : 'individual'; + + for (const file of fileList) { + const tempPath = file?.path || file?.filepath || file?.tempFilePath; + try { + if (!tempPath) { + throw new Error('Uploaded file is missing a temporary path'); + } + const tempStats = await fs.stat(tempPath); + if (tempStats.size === 0) { + throw new Error('File is empty - upload may have been interrupted'); + } + + const extension = path.extname(file.originalname); + const newFilename = generatePhotoFilename(event.event_name, categoryName, counter, extension); + counter += 1; + + const finalKey = path.posix.join(finalDestPathRel, newFilename); + const relativePath = path.posix.join(event.slug, newFilename); + const isVideo = isVideoMimeType(file.mimetype); + + // Move to storage first so the file is at its recorded path by the + // time the worker picks up the row. + await storage.putFromFile(finalKey, tempPath, { contentType: file.mimetype }); + await fs.unlink(tempPath).catch(() => {}); + + const stat = await storage.stat(finalKey); + if (!stat || stat.size !== tempStats.size) { + throw new Error(`Size mismatch after upload: expected ${tempStats.size}, got ${stat ? stat.size : 'null'}`); + } + + const inserted = await db('photos') + .insert({ + event_id: parseInt(eventId, 10), + filename: newFilename, + original_filename: file.originalname, + path: relativePath, + thumbnail_path: null, + type: photoType, + category_id: categoryId, + size_bytes: tempStats.size, + captured_at: null, + media_type: isVideo ? 'video' : 'image', + mime_type: file.mimetype, + processing_status: 'pending', + upload_id: uploadId, + }) + .returning('id'); + const photoId = inserted[0]?.id || inserted[0]; + + queued.push({ + id: photoId, + filename: newFilename, + size: tempStats.size, + category_id: categoryId, + }); + } catch (err) { + errors.push({ filename: file?.originalname || 'unknown', error: err.message }); + } + } + + return { uploadId, photos: queued, errors }; +} + +/** + * Worker-mode processing for a single already-stored photo. + * + * Called by the background processor after a row has been claimed + * (`processing_status` == 'processing'). The photo file already exists + * at its final storage key — this function reads it back, generates a + * thumbnail, extracts EXIF + dimensions (or video metadata), then + * updates the photo row to `complete` and fires the queued side + * effects (watermark, webhook). + * + * Throwing causes the background processor to mark the row as + * 'failed' with the error message; partial successes (e.g. thumbnail + * fails but dimensions succeed) are persisted up to the failure point. + */ +async function processPhoto(photoId) { + const photo = await db('photos').where({ id: photoId }).first(); + if (!photo) throw new Error(`Photo ${photoId} not found`); + + const event = await db('events').where({ id: photo.event_id }).first(); + if (!event) throw new Error(`Event ${photo.event_id} not found for photo ${photoId}`); + + const sourceKey = resolvePhotoStorageKey(event, photo); + const isVideo = + photo.media_type === 'video' || + (typeof photo.mime_type === 'string' && photo.mime_type.startsWith('video/')); + + const updateData = {}; + + // withLocalCopy materialises the original from the storage backend so + // sharp/ffmpeg can read it. For local storage this is a free O(1) path + // resolution; for S3 it downloads to a tmpdir that's auto-cleaned. + await withLocalCopy(sourceKey, async (localPath) => { + if (!photo.captured_at && !isVideo) { + try { + const captured = await extractCaptureDate(localPath); + if (captured) updateData.captured_at = captured; + } catch (e) { + logger.warn(`processPhoto: EXIF extraction failed for ${photoId}`, { error: e.message }); + } + } + + if (isVideo) { + const videoThumbnailKey = path.posix.join( + 'thumbnails', + `thumb_${photo.filename.replace(/\.[^.]+$/, '.jpg')}` + ); + const result = await processUploadedVideo(localPath, videoThumbnailKey); + updateData.thumbnail_path = result.thumbnailKey; + if (result.metadata) { + if (result.metadata.duration != null) updateData.duration = result.metadata.duration; + if (result.metadata.videoCodec) updateData.video_codec = result.metadata.videoCodec; + if (result.metadata.audioCodec) updateData.audio_codec = result.metadata.audioCodec; + if (result.metadata.width) updateData.width = result.metadata.width; + if (result.metadata.height) updateData.height = result.metadata.height; + } + } else { + try { + const thumbnailPath = await generateThumbnail(localPath); + if (thumbnailPath) updateData.thumbnail_path = thumbnailPath; + } catch (e) { + logger.warn(`processPhoto: thumbnail generation failed for ${photoId}`, { error: e.message }); + } + try { + const sharp = require('sharp'); + const metadata = await sharp(localPath).metadata(); + if (metadata.width && metadata.height) { + updateData.width = metadata.width; + updateData.height = metadata.height; + } + } catch (e) { + logger.warn(`processPhoto: dimensions extraction failed for ${photoId}`, { error: e.message }); + } + } + }); + + // Mark complete + updateData.processing_status = 'complete'; + updateData.processing_error = null; + await db('photos').where({ id: photoId }).update(updateData); + + // Side effects (best-effort, never fail the photo if these break) + if (!isVideo) { + const watermarkGeneratorService = require('./watermarkGeneratorService'); + watermarkGeneratorService + .generateForPhoto(photoId) + .catch((err) => logger.warn(`processPhoto: watermark queue failed for ${photoId}`, { error: err.message })); + } + + try { + const webhookService = require('./webhookService'); + await webhookService.fire('photo.uploaded', { + event: { id: event.id, slug: event.slug, event_name: event.event_name }, + photo: { + id: photo.id, + filename: photo.filename, + original_filename: photo.original_filename, + size_bytes: photo.size_bytes, + }, + }); + } catch (e) { + logger.warn(`processPhoto: webhook fire failed for ${photoId}`, { error: e.message }); + } + + return updateData; +} + module.exports = { - processUploadedPhotos + processUploadedPhotos, + queueFilesForProcessing, + processPhoto }; diff --git a/frontend/src/components/admin/AdminPhotoGrid.tsx b/frontend/src/components/admin/AdminPhotoGrid.tsx index f7c8d8f5..db1a676d 100644 --- a/frontend/src/components/admin/AdminPhotoGrid.tsx +++ b/frontend/src/components/admin/AdminPhotoGrid.tsx @@ -1,10 +1,12 @@ import React, { useState } from 'react'; -import { Check, Download, Trash2, Eye, EyeOff, Package, MessageSquare, Star, Video, FolderOpen } from 'lucide-react'; +import { Check, Download, Trash2, Eye, EyeOff, Package, MessageSquare, Star, Video, FolderOpen, Cog, AlertTriangle, RefreshCw } from 'lucide-react'; import { toast } from 'react-toastify'; +import { useQueryClient } from '@tanstack/react-query'; import { useTranslation } from 'react-i18next'; import { AdminPhoto } from '../../services/photos.service'; import { photosService } from '../../services/photos.service'; +import { uploadsService } from '../../services/uploads.service'; import { Button } from '../common'; import { AdminAuthenticatedImage } from './AdminAuthenticatedImage'; import { BulkCategoryModal } from './BulkCategoryModal'; @@ -32,6 +34,7 @@ export const AdminPhotoGrid: React.FC = ({ categories = [] }) => { const { t } = useTranslation(); + const queryClient = useQueryClient(); const [selectedPhotos, setSelectedPhotos] = useState>(new Set()); const [isSelectionMode, setIsSelectionMode] = useState(false); const [isDeleting, setIsDeleting] = useState(false); @@ -298,9 +301,42 @@ export const AdminPhotoGrid: React.FC = ({ )} - {/* Thumbnail */} + {/* Thumbnail (or processing placeholder for in-flight photos) */}
- {photo.thumbnail_url ? ( + {(photo as any).processing_status === 'pending' || + (photo as any).processing_status === 'processing' ? ( +
+ +

+ {t('admin.photos.processingStatus', 'Processing…')} +

+
+ ) : (photo as any).processing_status === 'failed' ? ( +
+ +

+ {t('admin.photos.processingFailed', 'Failed')} +

+ +
+ ) : photo.thumbnail_url ? ( = ({ eventId, onUploadComplete }) => { const { t } = useTranslation(); const [isUploading, setIsUploading] = useState(false); @@ -25,9 +35,18 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl const [uploadProgress, setUploadProgress] = useState(0); const [currentChunk, setCurrentChunk] = useState(0); const [totalChunks, setTotalChunks] = useState(0); + const [phase, setPhase] = useState({ kind: 'idle' }); const [selectedCategoryId, setSelectedCategoryId] = useState(null); const [replaceByName, setReplaceByName] = useState(false); + // Upload IDs returned from each chunk POST. The processing tracker + // hook merges status across all of them so the user sees one unified + // progress count even when the upload spans multiple HTTP requests. + const [uploadIds, setUploadIds] = useState([]); const fileInputRef = useRef(null); + + const { aggregate: processingAggregate } = useUploadProgress(uploadIds, { + enabled: phase.kind === 'processing' && uploadIds.length > 0, + }); // Fetch categories for this event const { data: categories = [] } = useQuery({ @@ -107,6 +126,7 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl setIsUploading(true); setUploadProgress(0); + setUploadIds([]); // For large uploads, chunk the files by both count AND size to prevent memory/network issues const MAX_FILES_PER_CHUNK = Math.max(1, Math.min(50, maxFilesPerUpload)); // Max 50 files per chunk @@ -144,11 +164,11 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl setCurrentChunk(chunkIndex + 1); const chunk = chunks[chunkIndex]; const formData = new FormData(); - + chunk.forEach((file) => { formData.append('photos', file); }); - + if (selectedCategoryId) { formData.append('category_id', selectedCategoryId.toString()); } @@ -156,63 +176,138 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl formData.append('replace_by_name', 'true'); } + setPhase({ + kind: 'transferring', + chunkIndex, + totalChunks: chunks.length, + bytePct: 0, + }); + try { const response = await api.post(`/admin/events/${eventId}/upload`, formData, { onUploadProgress: (progressEvent) => { if (progressEvent.total) { - // Calculate overall progress across all chunks const chunkProgress = progressEvent.loaded / progressEvent.total; const overallProgress = ((chunkIndex + chunkProgress) / chunks.length) * 100; setUploadProgress(Math.round(overallProgress)); + + // Once bytes have all left the browser, the request is + // sitting in the backend processing pipeline. Flip to + // 'processing' so the UI explains the wait instead of + // looking frozen at the chunk's max progress. + if (chunkProgress >= 1) { + setPhase((prev) => + prev.kind === 'transferring' && prev.chunkIndex === chunkIndex + ? { + kind: 'processing', + chunkIndex, + totalChunks: chunks.length, + filesInChunk: chunk.length, + } + : prev + ); + } else { + setPhase({ + kind: 'transferring', + chunkIndex, + totalChunks: chunks.length, + bytePct: Math.round(chunkProgress * 100), + }); + } } }, }); totalUploaded += (response.data?.successCount || chunk.length); totalReplaced += (response.data?.replacedCount || 0); + // Backend returns a per-request upload_id. Track it so the + // processing-status hook can poll/stream live progress. + if (response.data?.upload_id) { + const newId = response.data.upload_id as string; + setUploadIds((prev) => (prev.includes(newId) ? prev : [...prev, newId])); + } } catch (error: any) { console.error(`Error uploading chunk ${chunkIndex + 1}:`, error); failedFiles.push(...chunk.map(f => f.name)); - + // Continue with next chunk even if one fails continue; } } - // Clear selected files + // Bytes are all on the server. Clear the file picker so the + // user can queue another batch — but DON'T dismiss the upload + // UI yet; we'll watch the processing aggregate (useEffect below) + // to know when the backend has finished generating thumbnails + // and metadata. setSelectedFiles([]); if (fileInputRef.current) { fileInputRef.current.value = ''; } - // Show appropriate message if (totalReplaced > 0) { toast.info(t('upload.replacedFiles', { count: totalReplaced }) || `${totalReplaced} photo(s) replaced`); } - if (failedFiles.length === 0) { - toast.success(t('upload.uploadComplete') || `Successfully uploaded ${totalUploaded} files`); - } else { + if (failedFiles.length > 0) { toast.warning( - t('upload.someFilesFailed') || - `Uploaded ${totalUploaded} files. ${failedFiles.length} files failed.` + t('upload.someFilesFailed') || + `Transferred ${totalUploaded} files. ${failedFiles.length} files failed to transfer.` ); } - - // Call callback + + // Refresh the grid early so the user sees their photos appearing + // as the worker processes them. The processing-aggregate effect + // below will refresh again on completion. if (onUploadComplete) { onUploadComplete(); } + + // If the backend never returned an upload_id (e.g. only failures + // or pre-async-backend deployment), we have nothing to wait for — + // fall through to the finally cleanup which resets state. } catch (error: any) { console.error('Upload error:', error); toast.error(error.response?.data?.error || t('toast.uploadError')); - } finally { setIsUploading(false); setUploadProgress(0); setCurrentChunk(0); setTotalChunks(0); + setPhase({ kind: 'idle' }); + setUploadIds([]); } }; + // When the background worker finishes processing every photo from + // this upload, dismiss the upload UI and surface the result. + useEffect(() => { + if (!isUploading) return; + if (uploadIds.length === 0) return; + if (!processingAggregate.isComplete) return; + + if (processingAggregate.failed > 0) { + toast.warning( + t('upload.processingFailed', { count: processingAggregate.failed }) || + `${processingAggregate.failed} photo(s) failed to process` + ); + } else { + toast.success( + t('upload.uploadComplete') || `Successfully uploaded ${processingAggregate.complete} photo(s)` + ); + } + + if (onUploadComplete) onUploadComplete(); + setIsUploading(false); + setUploadProgress(0); + setCurrentChunk(0); + setTotalChunks(0); + setPhase({ kind: 'idle' }); + setUploadIds([]); + // We intentionally only react to processingAggregate.isComplete / + // .failed — the rest of the deps either don't move during this + // effect's lifetime or are stable callbacks. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [processingAggregate.isComplete, processingAggregate.failed, isUploading]); + const formatFileSize = (bytes: number) => { if (bytes < 1024) return bytes + ' B'; if (bytes < 1024 * 1024) return (bytes / 1024).toFixed(1) + ' KB'; @@ -344,26 +439,73 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl
- {/* Progress Bar */} + {/* Progress display — two distinct phases. Bytes-on-wire ('transferring') + drives the determinate bar; the post-bytes wait ('processing') swaps + in an indeterminate spinner with an explanatory hint so users don't + assume the upload froze. */} {isUploading && (
-
- - {t('upload.uploading')} - {totalChunks > 1 && ` (${t('common.chunk')} ${currentChunk}/${totalChunks})`} - - {uploadProgress}% -
-
-
-
- {totalChunks > 1 && ( -

- {t('upload.uploadingChunks', { count: selectedFiles.length, total: totalChunks })} -

+ {phase.kind === 'processing' ? ( +
+
+ +
+

+ {t('upload.processing')} +

+ {processingAggregate.total > 0 && ( + <> +

+ {t('upload.processingProgress', { + complete: processingAggregate.complete + processingAggregate.failed, + total: processingAggregate.total, + })} +

+
+
+
+ + )} +

+ {t('upload.processingHint')} +

+
+
+
+ ) : ( + <> +
+ + {t('upload.transferring')} + {totalChunks > 1 && ` (${t('common.chunk')} ${currentChunk}/${totalChunks})`} + + {uploadProgress}% +
+
+
+
+ {totalChunks > 1 && ( +

+ {t('upload.uploadingChunks', { count: selectedFiles.length, total: totalChunks })} +

+ )} + )}
)} diff --git a/frontend/src/components/gallery/UserPhotoUpload.tsx b/frontend/src/components/gallery/UserPhotoUpload.tsx index fa45e2ff..6423f157 100644 --- a/frontend/src/components/gallery/UserPhotoUpload.tsx +++ b/frontend/src/components/gallery/UserPhotoUpload.tsx @@ -1,5 +1,5 @@ import React, { useState, useMemo } from 'react'; -import { Upload, X, CheckCircle } from 'lucide-react'; +import { Upload, X, CheckCircle, Loader2 } from 'lucide-react'; import { useTranslation } from 'react-i18next'; import { toast } from 'react-toastify'; import { Button } from '../common'; @@ -24,6 +24,10 @@ export const UserPhotoUpload: React.FC = ({ const [files, setFiles] = useState([]); const [uploading, setUploading] = useState(false); const [uploadProgress, setUploadProgress] = useState<{ [key: string]: number }>({}); + // Per-file processing state — flips to true once axios reports + // bytes-on-wire for that file, so the UI can show "Processing…" + // instead of a static 100% bar while the backend works. + const [processingFiles, setProcessingFiles] = useState<{ [key: string]: boolean }>({}); const { data: publicSettings } = usePublicSettings(); @@ -87,9 +91,18 @@ export const UserPhotoUpload: React.FC = ({ ...prev, [file.name]: progress, })); + if (progress >= 100) { + setProcessingFiles(prev => ({ ...prev, [file.name]: true })); + } } }, }); + // Request resolved → file fully processed by backend. + setProcessingFiles(prev => { + const next = { ...prev }; + delete next[file.name]; + return next; + }); successCount++; } catch (error: any) { // Upload error handled - user notified via UI @@ -185,7 +198,13 @@ export const UserPhotoUpload: React.FC = ({
{uploadProgress[file.name] !== undefined ? (
- {uploadProgress[file.name] === 100 ? ( + {processingFiles[file.name] ? ( + // Bytes are on the server; the request hasn't + // resolved yet because the backend is still + // generating thumbnails / reading EXIF. Show + // a spinner so it doesn't look stuck at 100%. + + ) : uploadProgress[file.name] === 100 ? ( ) : (
diff --git a/frontend/src/hooks/useUploadProgress.ts b/frontend/src/hooks/useUploadProgress.ts new file mode 100644 index 00000000..90cd3408 --- /dev/null +++ b/frontend/src/hooks/useUploadProgress.ts @@ -0,0 +1,159 @@ +import { useEffect, useRef, useState } from 'react'; +import { + uploadsService, + type UploadStatusSnapshot, +} from '../services/uploads.service'; + +interface UseUploadProgressOptions { + /** + * If false, the hook does nothing (used to "pause" tracking when no + * upload is in progress). Default: true. + */ + enabled?: boolean; + /** + * Polling interval in ms (used always as a fallback, and as the + * primary channel when SSE is unavailable). Default: 1500. + */ + pollIntervalMs?: number; + /** + * If true, attempts an SSE upgrade for low-latency updates and + * falls back to polling when the stream errors. Default: true. + */ + preferStream?: boolean; +} + +/** + * Tracks an upload group's processing state. Returns a merged snapshot + * across all upload IDs the caller passes in (admin upload modal sends + * each chunk as its own upload_id; this hook merges their counters). + * + * The hook is resilient: it always polls in the background and uses + * SSE (when available and not disabled) as a faster supplementary + * channel. Either source landing on a terminal state stops the hook. + */ +export function useUploadProgress( + uploadIds: string[], + { enabled = true, pollIntervalMs = 1500, preferStream = true }: UseUploadProgressOptions = {} +) { + const [snapshots, setSnapshots] = useState>({}); + const [error, setError] = useState(null); + const eventSourcesRef = useRef>({}); + // Stable string key so we re-trigger the effect only when the actual + // set of IDs changes (parents may pass a new array each render). + const idsKey = uploadIds.join('|'); + + useEffect(() => { + if (!enabled || uploadIds.length === 0) { + return undefined; + } + + let cancelled = false; + const pollHandles: Record> = {}; + + const closeStream = (uploadId: string) => { + const es = eventSourcesRef.current[uploadId]; + if (es) { + es.close(); + delete eventSourcesRef.current[uploadId]; + } + }; + + const isTerminal = (snap: UploadStatusSnapshot | null) => + !!snap && snap.pending === 0 && snap.processing === 0; + + const merge = (uploadId: string, snap: UploadStatusSnapshot) => { + if (cancelled) return; + setSnapshots((prev) => ({ ...prev, [uploadId]: snap })); + }; + + const pollOnce = async (uploadId: string) => { + try { + const snap = await uploadsService.getStatus(uploadId); + merge(uploadId, snap); + if (!isTerminal(snap)) { + pollHandles[uploadId] = setTimeout(() => pollOnce(uploadId), pollIntervalMs); + } else { + closeStream(uploadId); + } + } catch (e) { + if (!cancelled) setError(e as Error); + // Retry polling on error after a longer interval — don't drop + // the group entirely just because one snapshot failed. + pollHandles[uploadId] = setTimeout(() => pollOnce(uploadId), pollIntervalMs * 4); + } + }; + + const tryStream = (uploadId: string) => { + if (typeof EventSource === 'undefined') return; + try { + const es = new EventSource(uploadsService.streamUrl(uploadId), { withCredentials: true }); + eventSourcesRef.current[uploadId] = es; + + es.onmessage = (event) => { + try { + const payload: UploadStatusSnapshot = JSON.parse(event.data); + merge(uploadId, payload); + if (isTerminal(payload)) { + closeStream(uploadId); + } + } catch (_) { + /* ignore malformed event */ + } + }; + + es.onerror = () => { + // Treat any error as a fatal stream failure; polling keeps + // running anyway and will pick up status. Avoids reconnect + // storms on broken proxies. + closeStream(uploadId); + }; + } catch (_) { + // EventSource construction failed — polling alone covers it. + } + }; + + for (const uploadId of uploadIds) { + pollOnce(uploadId); + if (preferStream) tryStream(uploadId); + } + + return () => { + cancelled = true; + for (const handle of Object.values(pollHandles)) clearTimeout(handle); + for (const uploadId of Object.keys(eventSourcesRef.current)) closeStream(uploadId); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [idsKey, enabled, pollIntervalMs, preferStream]); + + // Aggregate counters across all tracked upload IDs. + const aggregate = (() => { + const totals = { total: 0, pending: 0, processing: 0, complete: 0, failed: 0 }; + const failedPhotos: { id: number; filename: string; error: string | null }[] = []; + let allReady = true; + for (const uploadId of uploadIds) { + const snap = snapshots[uploadId]; + if (!snap) { + allReady = false; + continue; + } + totals.total += snap.total; + totals.pending += snap.pending; + totals.processing += snap.processing; + totals.complete += snap.complete; + totals.failed += snap.failed; + for (const p of snap.photos) { + if (p.status === 'failed') { + failedPhotos.push({ id: p.id, filename: p.original_filename, error: p.error }); + } + } + } + const isComplete = allReady && totals.pending === 0 && totals.processing === 0 && totals.total > 0; + return { ...totals, failedPhotos, isComplete, isReady: allReady }; + })(); + + return { + snapshots, + aggregate, + error, + }; +} diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 891a6c2d..2456c406 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -131,6 +131,12 @@ "unsupportedFiles": "Einige Dateien wurden übersprungen, da das Format nicht unterstützt wird (JPEG/PNG/WebP/MP4/MOV/WEBM verwenden).", "selectedFiles": "Ausgewählte Dateien", "uploading": "Wird hochgeladen...", + "transferring": "Übertragung", + "processing": "Fotos werden verarbeitet...", + "processingHint": "Dateien sind hochgeladen. PicPeak erstellt jetzt Thumbnails und liest Metadaten. Sie können diese Seite verlassen — die Verarbeitung läuft im Hintergrund weiter.", + "processingProgress": "{{complete}} von {{total}} fertig", + "processingFailed": "{{count}} Foto(s) konnten nicht verarbeitet werden", + "retryFailed": "Fehlgeschlagene erneut versuchen", "uploadComplete": "Upload abgeschlossen!", "uploadFailed": "Upload fehlgeschlagen", "someFilesFailed": "Einige Dateien konnten nicht hochgeladen werden", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index d2d3ba87..0c571243 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -131,6 +131,12 @@ "unsupportedFiles": "Some files were skipped because the format is not supported (use JPEG/PNG/WebP/MP4/MOV/WEBM).", "selectedFiles": "Selected files", "uploading": "Uploading...", + "transferring": "Transferring", + "processing": "Processing photos...", + "processingHint": "Files are uploaded. PicPeak is now generating thumbnails and reading metadata. You can leave this page — work continues in the background.", + "processingProgress": "{{complete}} of {{total}} done", + "processingFailed": "{{count}} photo(s) failed to process", + "retryFailed": "Retry failed", "uploadComplete": "Upload complete!", "uploadFailed": "Upload failed", "someFilesFailed": "Some files failed to upload", diff --git a/frontend/src/pages/admin/EventDetailsPage.tsx b/frontend/src/pages/admin/EventDetailsPage.tsx index 445c73e7..644a1ecb 100644 --- a/frontend/src/pages/admin/EventDetailsPage.tsx +++ b/frontend/src/pages/admin/EventDetailsPage.tsx @@ -302,11 +302,22 @@ export const EventDetailsPage: React.FC = () => { logic: feedbackFilters.logic, }), [photoFilters, feedbackFilters]); - // Fetch photos (needed for both photos tab and hero photo selector) + // Fetch photos (needed for both photos tab and hero photo selector). + // While any photo is still in pending/processing state we poll every + // 2s so the admin grid auto-updates as the background worker drains + // the queue. Once everything is complete/failed the polling stops. const { data: photos = [], isLoading: photosLoading, refetch: refetchPhotos } = useQuery({ queryKey: ['admin-event-photos', id, combinedPhotoFilters], queryFn: () => photosService.getEventPhotos(parseInt(id!), combinedPhotoFilters), enabled: !!id && (activeTab === 'photos' || isEditing), + refetchInterval: (query) => { + const data = query.state.data as AdminPhoto[] | undefined; + if (!Array.isArray(data)) return false; + const inFlight = data.some( + (p: any) => p.processing_status === 'pending' || p.processing_status === 'processing' + ); + return inFlight ? 2000 : false; + }, }); // Fetch filter summary for feedback filters diff --git a/frontend/src/services/uploads.service.ts b/frontend/src/services/uploads.service.ts new file mode 100644 index 00000000..a90d2faa --- /dev/null +++ b/frontend/src/services/uploads.service.ts @@ -0,0 +1,57 @@ +import { api } from '../config/api'; + +export type PhotoProcessingStatus = 'pending' | 'processing' | 'complete' | 'failed'; + +export interface UploadPhotoStatus { + id: number; + filename: string; + original_filename: string; + status: PhotoProcessingStatus; + error: string | null; +} + +export interface UploadStatusSnapshot { + upload_id: string; + event_id: number; + total: number; + pending: number; + processing: number; + complete: number; + failed: number; + photos: UploadPhotoStatus[]; +} + +export const uploadsService = { + /** + * One-shot snapshot of an upload group's processing state. Frontends + * poll this every 1.5s while any photo is still pending/processing. + */ + async getStatus(uploadId: string): Promise { + const response = await api.get(`/admin/uploads/${uploadId}/status`); + return response.data; + }, + + /** + * Retry a failed photo. Flips status back to 'pending' so the + * background worker picks it up again. + */ + async retryPhoto(photoId: number): Promise<{ id: number; status: PhotoProcessingStatus }> { + const response = await api.post<{ id: number; status: PhotoProcessingStatus }>( + `/admin/photos/${photoId}/retry` + ); + return response.data; + }, + + /** + * Build the SSE stream URL for an upload group. Caller is responsible + * for opening an EventSource and merging the JSON-payload events into + * their progress state. Falls back to polling getStatus() if the + * EventSource fails to open (proxy buffering, etc.). + */ + streamUrl(uploadId: string): string { + // EventSource doesn't send our auth headers, so we have to rely on + // the cookie-based admin session. (PicPeak's auth middleware reads + // cookies before falling back to Authorization headers.) + return `${api.defaults.baseURL || ''}/admin/uploads/${uploadId}/stream`; + }, +}; diff --git a/package-lock.json b/package-lock.json index 398a3cf7..8f91183a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,8 +10,7 @@ "node-fetch": "^2.7.0" }, "devDependencies": { - "@playwright/test": "^1.48.2", - "dotenv": "^17.3.1", + "@playwright/test": "^1.57.0", "puppeteer": "^24.17.0" } }, @@ -41,13 +40,13 @@ } }, "node_modules/@playwright/test": { - "version": "1.55.0", - "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.55.0.tgz", - "integrity": "sha512-04IXzPwHrW69XusN/SIdDdKZBzMfOT9UNT/YiJit/xpy2VuAoB8NHc8Aplb96zsWDddLnbkPL3TsmrS04ZU2xQ==", + "version": "1.57.0", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.57.0.tgz", + "integrity": "sha512-6TyEnHgd6SArQO8UO2OMTxshln3QMWBtPGrOCgs3wVEmQmwyuNtB10IZMfmYDE0riwNR1cu4q+pPcxMVtaG3TA==", "dev": true, "license": "Apache-2.0", "dependencies": { - "playwright": "1.55.0" + "playwright": "1.57.0" }, "bin": { "playwright": "cli.js" @@ -562,19 +561,6 @@ "license": "BSD-3-Clause", "peer": true }, - "node_modules/dotenv": { - "version": "17.3.1", - "resolved": "https://registry.npmjs.org/dotenv/-/dotenv-17.3.1.tgz", - "integrity": "sha512-IO8C/dzEb6O3F9/twg6ZLXz164a2fhTnEWb95H23Dm4OuN+92NmEAlTrupP9VW6Jm3sO26tQlqyvyi4CsnY9GA==", - "dev": true, - "license": "BSD-2-Clause", - "engines": { - "node": ">=12" - }, - "funding": { - "url": "https://dotenvx.com" - } - }, "node_modules/emoji-regex": { "version": "8.0.0", "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz", @@ -1131,13 +1117,13 @@ "license": "ISC" }, "node_modules/playwright": { - "version": "1.55.0", - "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.55.0.tgz", - "integrity": "sha512-sdCWStblvV1YU909Xqx0DhOjPZE4/5lJsIS84IfN9dAZfcl/CIZ5O8l3o0j7hPMjDvqoTF8ZUcc+i/GL5erstA==", + "version": "1.57.0", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.57.0.tgz", + "integrity": "sha512-ilYQj1s8sr2ppEJ2YVadYBN0Mb3mdo9J0wQ+UuDhzYqURwSoW4n1Xs5vs7ORwgDGmyEh33tRMeS8KhdkMoLXQw==", "dev": true, "license": "Apache-2.0", "dependencies": { - "playwright-core": "1.55.0" + "playwright-core": "1.57.0" }, "bin": { "playwright": "cli.js" @@ -1150,9 +1136,9 @@ } }, "node_modules/playwright-core": { - "version": "1.55.0", - "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.55.0.tgz", - "integrity": "sha512-GvZs4vU3U5ro2nZpeiwyb0zuFaqb9sUiAJuyrWpcGouD8y9/HLgGbNRjIph7zU9D3hnPaisMl9zG9CgFi/biIg==", + "version": "1.57.0", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.57.0.tgz", + "integrity": "sha512-agTcKlMw/mjBWOnD6kFZttAAGHgi/Nw0CZ2o6JqWSbMlI219lAFLZZCyqByTsvVAJq5XA5H8cA6PrvBRpBWEuQ==", "dev": true, "license": "Apache-2.0", "bin": { diff --git a/package.json b/package.json index cda2229b..d72216c3 100644 --- a/package.json +++ b/package.json @@ -8,8 +8,7 @@ "node-fetch": "^2.7.0" }, "devDependencies": { - "@playwright/test": "^1.48.2", - "dotenv": "^17.3.1", + "@playwright/test": "^1.57.0", "puppeteer": "^24.17.0" }, "overrides": {