From 7b2f75e6ae6ef3cf86782e56f3026ee6063e2e15 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sat, 2 May 2026 22:33:45 +0200 Subject: [PATCH 1/6] chore: bump @playwright/test to ^1.57.0; drop unused root dotenv The root devDependencies still pinned an older Playwright (1.48.2) plus a stray `dotenv` that nothing in the e2e suite or root scripts actually requires (verified via grep across tests/). Updates the Playwright version to match the current upstream stable and removes the unused dotenv to keep the root install lean. Originated from a local stash that picked up these changes; landing them as a small dedicated commit so they don't blend into the auth fix that follows. --- package-lock.json | 38 ++++++++++++-------------------------- package.json | 3 +-- 2 files changed, 13 insertions(+), 28 deletions(-) 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": { From f905f7e7336c756e73a8c650c9239b171697164a Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sat, 2 May 2026 22:42:59 +0200 Subject: [PATCH 2/6] fix(auth): /auth/session must reject tokens that adminAuth/galleryAuth would reject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second loop fix in the same /admin/login → /admin/dashboard → /admin/login pattern as #355. The frontend trusts /auth/session as the source of truth for "is the user authenticated?". When that endpoint is more lenient than the protected middleware, every admin endpoint 401s right after /auth/session said valid:true, the response interceptor hard-redirects to /admin/login, /auth/session says valid again, and the cycle closes — exactly the loop reported on v3.32.4-beta.0. #355 fixed the issuer-claim asymmetry. This commit fixes the remaining asymmetries: /auth/session was missing the admin-existence, admin-active, password-change-after-iat, and gallery-existence / gallery-archived / gallery-expired checks that adminAuth and galleryAuth perform on every protected request. The fix is to mirror those checks in /auth/session, scoped by token type, and degrade gracefully when the underlying tables aren't present (test fixtures, early bootstrap) so the endpoint never fails-closed because of a missing table. Reproducer that the new test covers: 1. Admin logs in (token issued at T). 2. Admin (or another admin) changes their own password at T+1. 3. Browser still has the cookie from T. 4. /auth/session says valid:true (no password-change check). 5. /admin/dashboard fires queries; adminAuth rejects with PASSWORD_CHANGED 401. 6. Frontend redirects to /admin/login. 7. /auth/session says valid:true again. → loop. Other surfaces this also covers: - admin user deactivated (admin_users.is_active = false) - admin user deleted - gallery token whose event is archived - gallery token whose event has expired Tests live in __tests__/routes/authSession.symmetry.test.js — 9 cases, mocking db / tokenRevocation / tokenUtils / recaptcha / sessionTimeout so the suite runs without a real database. --- .../routes/authSession.symmetry.test.js | 302 ++++++++++++++++++ backend/src/routes/auth.js | 59 +++- 2 files changed, 360 insertions(+), 1 deletion(-) create mode 100644 backend/__tests__/routes/authSession.symmetry.test.js 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/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); From 86dfcc4f116a394e7e092ab3e01f3f1d030bb367 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sat, 2 May 2026 00:15:20 +0200 Subject: [PATCH 3/6] feat(upload): two-state UI + temp dir cleanup (PR-A of async processing) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 of the upload-progress redesign. Two changes that ship UX wins without any architectural surgery — they're a stepping stone for the full async-processing rework that follows in subsequent commits. 1. Two-state progress bar (PhotoUpload.tsx, UserPhotoUpload.tsx) When axios.onUploadProgress reports loaded === total, the request is on the server and the bytes have left the browser. Today the bar sits at 100% for the chunk while the backend runs sharp/ffmpeg/EXIF (often minutes on NFS-backed storage) and users assume the upload froze. The component now distinguishes two phases: - 'transferring' — bytes-on-wire, determinate progress bar. - 'processing' — bytes done, waiting for response. Indeterminate spinner + an explanatory hint that the backend is generating thumbnails / reading metadata and the user can leave the page. Same pattern in UserPhotoUpload (gallery): the per-file checkmark icon is replaced by a Loader2 spinner while the request is in flight after bytes-on-wire finished. 2. Temp directory cleanup (adminPhotos.js) Multer creates temp/upload__/ per request. Files inside it are individually unlinked after they're moved to storage on the success path, but the empty directory was never removed. On error paths three different inline blocks each tried to clean up; the success path was missed entirely. Result: the orphan-empty-dirs accumulation reported in the issue (70+ on the affected instance). Replace the inline cleanup blocks with a single idempotent cleanupTempDir() registered on res.finish + res.close, so it fires exactly once on every exit path (validation 4xx, server 5xx, multer error, success). New translation keys (en/de): upload.transferring, upload.processing, upload.processingHint, upload.processingProgress, upload.processingFailed, upload.retryFailed. --- backend/src/routes/adminPhotos.js | 59 ++++------ frontend/src/components/admin/PhotoUpload.tsx | 109 ++++++++++++++---- .../components/gallery/UserPhotoUpload.tsx | 23 +++- frontend/src/i18n/locales/de.json | 6 + frontend/src/i18n/locales/en.json | 6 + 5 files changed, 141 insertions(+), 62 deletions(-) diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 46ec4661..1a577260 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -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' }); } @@ -597,17 +591,8 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r res.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' }); } }); diff --git a/frontend/src/components/admin/PhotoUpload.tsx b/frontend/src/components/admin/PhotoUpload.tsx index 326d8a2c..2d664cf6 100644 --- a/frontend/src/components/admin/PhotoUpload.tsx +++ b/frontend/src/components/admin/PhotoUpload.tsx @@ -1,5 +1,5 @@ import React, { useState, useRef, useMemo } from 'react'; -import { Upload, X, Image, Loader2 } from 'lucide-react'; +import { Upload, X, Image, Loader2, Cog } from 'lucide-react'; import { Button } from '../common'; import { clsx } from 'clsx'; import { api } from '../../config/api'; @@ -18,6 +18,15 @@ interface PhotoUploadProps { const DEFAULT_MAX_FILES_PER_UPLOAD = 500; const MAX_FILES_PER_UPLOAD_LIMIT = 2000; +// Upload phase machine. The user perceives "frozen" during 'processing' +// because the bytes are already on the server and we're waiting for +// thumbnail/EXIF/etc. work — the explicit phase + hint message kills +// that perception (#352 / contributor analysis on issue 357 review). +type UploadPhase = + | { kind: 'idle' } + | { kind: 'transferring'; chunkIndex: number; totalChunks: number; bytePct: number } + | { kind: 'processing'; chunkIndex: number; totalChunks: number; filesInChunk: number }; + export const PhotoUpload: React.FC = ({ eventId, onUploadComplete }) => { const { t } = useTranslation(); const [isUploading, setIsUploading] = useState(false); @@ -25,6 +34,7 @@ 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); const fileInputRef = useRef(null); @@ -144,11 +154,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,14 +166,44 @@ 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), + }); + } } }, }); @@ -173,7 +213,7 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl } 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; } @@ -210,6 +250,7 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl setUploadProgress(0); setCurrentChunk(0); setTotalChunks(0); + setPhase({ kind: 'idle' }); } }; @@ -344,26 +385,48 @@ 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')} + {phase.totalChunks > 1 && ` (${t('common.chunk')} ${phase.chunkIndex + 1}/${phase.totalChunks})`} +

+

+ {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/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", From 851744c3c4df7deba8d946b6592fdb5042c52a26 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sat, 2 May 2026 00:25:51 +0200 Subject: [PATCH 4/6] =?UTF-8?q?feat(upload):=20async=20photo=20processing?= =?UTF-8?q?=20=E2=80=94=20backend=20(PR-B=20part=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move thumbnail / EXIF / dimensions / watermark / webhook work off the upload request thread and into a background worker pool. Upload requests now return 202 in seconds even on NFS-backed storage; the worker(s) drain the pending queue independently and update each photo's processing_status to 'complete' or 'failed' on its own. Schema (migration 085_async_photo_processing.js): - photos.processing_status enum default 'complete' (existing rows are already done) - photos.processing_error populated on 'failed' - photos.processing_started_at timestamp for janitor recovery - photos.upload_id groups all photos from one upload request so the frontend can poll status by group - indexes on processing_status and upload_id for queue lookups services/photoProcessor.js - queueFilesForProcessing(files, options) — shared helper used by the admin and gallery upload routes. Moves files to final storage + inserts pending rows; returns { uploadId, photos, errors }. - processPhoto(photoId) — worker-mode: reads original from storage via withLocalCopy (transparent local/S3), generates thumbnail and EXIF/dimensions or video metadata, queues watermark, fires photo.uploaded webhook, marks 'complete'. Throws => caller marks 'failed' with the error message. - processUploadedPhotos kept untouched — chunkedUploadService still uses the synchronous path. services/backgroundProcessor.js (new) - N independent worker loops per backend instance (default 2, UPLOAD_PROCESSOR_CONCURRENCY env override). - Multi-pod safe: postgres SELECT FOR UPDATE SKIP LOCKED, sqlite UPDATE-with-status-guard. Pods race on rows, exactly one wins. - Janitor every minute resets photos stuck in 'processing' for >10 minutes (worker died, pod restarted) back to 'pending'. - UPLOAD_PROCESSOR_DISABLED=true opt-out for CI/test. - Started from server.js after the other long-running workers. routes/adminPhotos.js — POST /:eventId/upload - Replaced batch-of-25 sync processing loop with per-file move-to-storage + insert-pending. Response is now 202 with upload_id, count, photo_ids in addition to the legacy successCount / replacedCount fields the existing frontend reads. - Per-request temp directory cleanup is now a single idempotent handler on res.finish/res.close (was three inline blocks for error paths only, leaking dirs on success — original bug from contributor analysis). - GET /uploads/:upload_id/status — JSON snapshot of pending / processing / complete / failed counts plus per-photo state. - GET /uploads/:upload_id/stream — SSE upgrade. Polls internally every 1.5s, emits on snapshot change, ends when all photos reach a terminal state. - POST /photos/:photoId/retry — flips a 'failed' photo back to 'pending' so the worker picks it up again. - GET /:eventId/thumbnail/:photoId now returns 503 with Retry-After while the photo is still pending/processing, and 422 on 'failed'. The admin grid renders placeholders accordingly. routes/gallery.js — POST /:eventId/upload (guest) - Refactored to use queueFilesForProcessing instead of the synchronous processUploadedPhotos. Same 202 + upload_id shape. - GET /:slug/photos now filters processing_status to 'complete' (or NULL for pre-migration rows) so guests never see in-flight photos. Side-effect timing change: - photo.uploaded webhook now fires from the worker after the photo is actually processed (thumbnail + dimensions populated) instead of from inside the upload request. Same payload fields. Worth a one-line note in the changelog. --- .../core/085_async_photo_processing.js | 72 +++ backend/server.js | 10 +- backend/src/routes/adminPhotos.js | 564 ++++++++++-------- backend/src/routes/gallery.js | 44 +- backend/src/services/backgroundProcessor.js | 176 ++++++ backend/src/services/photoProcessor.js | 226 ++++++- 6 files changed, 815 insertions(+), 277 deletions(-) create mode 100644 backend/migrations/core/085_async_photo_processing.js create mode 100644 backend/src/services/backgroundProcessor.js 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 1a577260..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'); @@ -283,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 }); } } @@ -561,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, @@ -574,21 +417,22 @@ 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); // Temp directory cleanup is handled by the response finish/close @@ -597,6 +441,176 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r } }); +// 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 { @@ -1055,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); @@ -1106,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/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 }; From 3b827b80d51269e1a7b9c693396f3cb7a9a48ffc Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sat, 2 May 2026 00:29:37 +0200 Subject: [PATCH 5/6] =?UTF-8?q?feat(upload):=20async=20photo=20processing?= =?UTF-8?q?=20=E2=80=94=20frontend=20(PR-B=20part=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live processing-state UI that complements the backend async pipeline. Modal stays open through the processing phase and surfaces real progress (X of N photos processed); the admin grid renders placeholder cards for in-flight photos and auto-refreshes via polling until the queue drains. services/uploads.service.ts (new) - getStatus(uploadId) — JSON snapshot from /admin/uploads/:id/status - retryPhoto(photoId) — POST /admin/photos/:id/retry - streamUrl(uploadId) — SSE upgrade URL hooks/useUploadProgress.ts (new) - Tracks N concurrent upload IDs (one per chunk POST) and merges counters into a single aggregate. - Always polls every 1.5s; opportunistic SSE upgrade on top of that. SSE failure (proxy buffering, etc.) silently downgrades to polling only — no reconnect storms. - Auto-stops both channels when every tracked group is in a terminal (complete/failed) state. components/admin/PhotoUpload.tsx - Captures upload_id from each chunk's 202 response, feeds them into useUploadProgress. - Phase machine extended: stays in 'processing' until the worker drains the queue (not just until bytes-on-wire). Progress UI shows real "X of N done" with a determinate bar fed by the aggregate. - "You can leave this page" hint kept — closing the modal is now actually safe, work continues server-side. - Side-effect refactor: invokes onUploadComplete twice — once early so the user sees photos appearing immediately, once on terminal so the parent grid sees final state. components/admin/AdminPhotoGrid.tsx - Photos with processing_status pending/processing render an amber placeholder card with a spinning Cog instead of the missing thumbnail. - Photos with status='failed' render a red card with the error message and a "Retry" button that POSTs /admin/photos/:id/retry. pages/admin/EventDetailsPage.tsx - Photo list query gains refetchInterval that polls every 2s while any photo is non-terminal, then stops. Keeps the grid auto-fresh during ongoing processing. --- .../src/components/admin/AdminPhotoGrid.tsx | 42 ++++- frontend/src/components/admin/PhotoUpload.tsx | 105 ++++++++++-- frontend/src/hooks/useUploadProgress.ts | 159 ++++++++++++++++++ frontend/src/pages/admin/EventDetailsPage.tsx | 13 +- frontend/src/services/uploads.service.ts | 57 +++++++ 5 files changed, 359 insertions(+), 17 deletions(-) create mode 100644 frontend/src/hooks/useUploadProgress.ts create mode 100644 frontend/src/services/uploads.service.ts 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, onUploadCompl 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({ @@ -117,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 @@ -210,6 +220,12 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl 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)); @@ -219,41 +235,79 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl } } - // 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'; @@ -398,9 +452,34 @@ export const PhotoUpload: React.FC = ({ eventId, onUploadCompl

{t('upload.processing')} - {phase.totalChunks > 1 && ` (${t('common.chunk')} ${phase.chunkIndex + 1}/${phase.totalChunks})`}

-

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

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

+
+
+
+ + )} +

{t('upload.processingHint')}

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/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`; + }, +}; From b30010eb6fae0e3f577bf1786e5274af0b12f14e Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sat, 2 May 2026 00:47:46 +0200 Subject: [PATCH 6/6] test(upload): unit tests for backgroundProcessor + processPhoto MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover the two new pieces of the async pipeline: backgroundProcessor.claimNextPhoto - returns null when no pending rows - returns the row + flips status under postgres FOR UPDATE SKIP LOCKED - returns null when SQLite UPDATE-with-guard loses the race - returns the row when the SQLite guard wins photoProcessor.processPhoto - happy path: writes thumbnail / dimensions / EXIF capture date and marks 'complete'; fires watermark queue + photo.uploaded webhook with the right payload - video path: writes ffmpeg duration / codec / dimensions; does NOT queue watermark (image-only) - throws cleanly when the photo row no longer exists Mocks db / imageProcessor / videoProcessor / storage / sharp / watermarkGeneratorService / webhookService / logger so the tests run without a real DB or any image library calls — fast and deterministic. --- .../services/backgroundProcessor.test.js | 109 +++++++++ .../photoProcessor.processPhoto.test.js | 216 ++++++++++++++++++ 2 files changed, 325 insertions(+) create mode 100644 backend/__tests__/services/backgroundProcessor.test.js create mode 100644 backend/__tests__/services/photoProcessor.processPhoto.test.js 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