diff --git a/backend/__tests__/integration/previewTiers.test.js b/backend/__tests__/integration/previewTiers.test.js index 97bfb511..ff5c29e3 100644 --- a/backend/__tests__/integration/previewTiers.test.js +++ b/backend/__tests__/integration/previewTiers.test.js @@ -129,8 +129,15 @@ describe('preview tiers (#1095)', () => { it('derives every non-default tier key for cleanup', () => { // Tiers live outside preview_path, so delete/archive/regenerate have no // other way to find them. 1920 is excluded because that IS preview_path. + // Two candidates per width — the encoder picks `.jpg` or `.webp` and the + // cleanup list cannot know which without probing the source. const keys = imageProcessor.previewTierKeys({ id: 5, path: 'e/a.jpg', source_origin: 'managed' }); - expect(keys).toHaveLength(imageProcessor.PREVIEW_WIDTHS.length - 1); + const widths = imageProcessor.PREVIEW_WIDTHS.filter((w) => w !== 1920); + expect(keys).toHaveLength(widths.length * 2); + for (const w of widths) { + expect(keys).toContain(`previews/preview_w${w}_p5_a.jpg`); + expect(keys).toContain(`previews/preview_w${w}_p5_a.webp`); + } expect(keys.some((k) => k.includes('w1920'))).toBe(false); expect(keys.every((k) => k.includes('p5_'))).toBe(true); }); diff --git a/backend/__tests__/services/previewTierKeys.formats.test.js b/backend/__tests__/services/previewTierKeys.formats.test.js new file mode 100644 index 00000000..13d49de3 --- /dev/null +++ b/backend/__tests__/services/previewTierKeys.formats.test.js @@ -0,0 +1,148 @@ +/** + * Preview tier lookup and cleanup must agree with what the generator writes + * (#1020 follow-up). + * + * generatePreviewImage rewrites the extension to match the encoding (`.jpg`, + * or `.webp` for alpha / multi-frame sources). ensurePreviewImageAtWidth and + * previewTierKeys kept the SOURCE extension, so for a `.png`, `.JPG`, `.heic` + * or RAW source the tier was generated on every single request — the stat + * never matched — and cleanup never found the files, which piled up in + * storage for the life of the install. + * + * Driven against real Sharp output, because the whole question is which + * extension the encoder actually chose. + */ +const path = require('path'); +const fs = require('fs').promises; +const os = require('os'); +const sharp = require('sharp'); + +jest.mock('../../src/database/db', () => { + const state = { event: null }; + const api = (table) => { + if (table === 'events') return { where: () => ({ first: async () => state.event }) }; + if (table === 'photos') return { where: () => ({ update: async () => 1 }) }; + if (table === 'app_settings') return { whereIn: () => ({ select: async () => [] }) }; + throw new Error(`unexpected table in test: ${table}`); + }; + api.__state = state; + return { db: api }; +}); + +const LocalFsStorage = require('../../src/services/storage/LocalFsStorage'); +const storageModule = require('../../src/services/storage'); +const { db } = require('../../src/database/db'); + +const EVENT = { id: 21, slug: 'fmt-ev', source_mode: 'managed' }; +let nextId = 5000; + +describe('preview tier keys follow the encoded extension', () => { + let storage; let storageRoot; let imageProcessor; let puts; + + beforeAll(async () => { + storageRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'picpeak-tierkeys-')); + storage = new LocalFsStorage({ root: storageRoot }); + await storage.init(); + const origPut = storage.put.bind(storage); + storage.put = async (key, ...rest) => { puts.push(key); return origPut(key, ...rest); }; + storageModule.setStorageForTesting(storage); + delete require.cache[require.resolve('../../src/services/imageProcessor')]; + imageProcessor = require('../../src/services/imageProcessor'); + }, 30000); + + beforeEach(() => { puts = []; db.__state.event = EVENT; }); + + afterAll(async () => { + storageModule.resetStorage(); + await fs.rm(storageRoot, { recursive: true, force: true }).catch(() => {}); + }); + + async function photoWith(name, { alpha = false } = {}) { + const id = nextId++; + const rel = `${EVENT.slug}/${name}`; + const abs = path.join(storageRoot, 'events/active', rel); + await fs.mkdir(path.dirname(abs), { recursive: true }); + const pipeline = sharp({ + create: { + width: 2000, height: 1400, channels: alpha ? 4 : 3, + background: alpha ? { r: 10, g: 20, b: 30, alpha: 0.5 } : { r: 10, g: 20, b: 30 }, + }, + }); + if (/\.png$/i.test(name)) await pipeline.png().toFile(abs); + else if (/\.webp$/i.test(name)) await pipeline.webp().toFile(abs); + else await pipeline.jpeg().toFile(abs); + return { id, event_id: EVENT.id, source_origin: 'managed', path: rel, filename: name }; + } + + it.each([ + ['opaque PNG', 'photo.png', false, '.jpg'], + ['transparent PNG', 'photo-alpha.png', true, '.webp'], + ['uppercase JPG', 'IMG_0001.JPG', false, '.jpg'], + ['jpeg spelled out', 'photo.jpeg', false, '.jpg'], + ['lowercase jpg', 'photo.jpg', false, '.jpg'], + ])('%s: the second request is a cache hit, not a second generation', async (_label, name, alpha, ext) => { + const photo = await photoWith(name, { alpha }); + + const first = await imageProcessor.ensurePreviewImageAtWidth(photo, 640); + expect(first).toBe(`previews/preview_w640_p${photo.id}_${name.replace(/\.[^.]+$/, '')}${ext}`); + expect(puts).toEqual([first]); + + const second = await imageProcessor.ensurePreviewImageAtWidth(photo, 640); + expect(second).toBe(first); + expect(puts).toHaveLength(1); + }); + + it.each([ + ['opaque PNG', 'cleanup.png', false], + ['transparent PNG', 'cleanup-alpha.png', true], + ['uppercase JPG', 'CLEANUP.JPG', false], + ])('%s: previewTierKeys covers the generated key, so deletePreviewTiers removes it', async (_label, name, alpha) => { + const photo = await photoWith(name, { alpha }); + const k640 = await imageProcessor.ensurePreviewImageAtWidth(photo, 640); + const k1280 = await imageProcessor.ensurePreviewImageAtWidth(photo, 1280); + expect(await storage.stat(k640)).toBeTruthy(); + expect(await storage.stat(k1280)).toBeTruthy(); + + const keys = imageProcessor.previewTierKeys(photo); + expect(keys).toEqual(expect.arrayContaining([k640, k1280])); + + await imageProcessor.deletePreviewTiers(photo); + expect(await storage.stat(k640)).toBeNull(); + expect(await storage.stat(k1280)).toBeNull(); + }); + + it('a tier written before the extension rewrite is still found, by lookup and by cleanup', async () => { + // JPEG bytes under the source's `.png` name — what generatePreviewImage + // produced before it started rewriting the extension. Served as JPEG by + // the route (the Content-Type comes from the `.webp` suffix only). + const photo = await photoWith('legacy.png'); + const legacyKey = `previews/preview_w640_p${photo.id}_legacy.png`; + await storage.put(legacyKey, await sharp({ create: { width: 640, height: 448, channels: 3, background: '#123' } }).jpeg().toBuffer()); + puts = []; + + expect(await imageProcessor.ensurePreviewImageAtWidth(photo, 640)).toBe(legacyKey); + expect(puts).toHaveLength(0); + + expect(imageProcessor.previewTierKeys(photo)).toContain(legacyKey); + await imageProcessor.deletePreviewTiers(photo); + expect(await storage.stat(legacyKey)).toBeNull(); + }); + + it('never lists the canonical 1920 rendition, which preview_path owns', () => { + const keys = imageProcessor.previewTierKeys({ id: 9, path: 'e/a.png', source_origin: 'managed' }); + expect(keys.some((k) => k.includes('w1920'))).toBe(false); + expect(keys.every((k) => k.includes('p9_'))).toBe(true); + // Both encodings plus the legacy source-extension key, per width. + expect(keys).toEqual([ + 'previews/preview_w640_p9_a.jpg', 'previews/preview_w640_p9_a.webp', 'previews/preview_w640_p9_a.png', + 'previews/preview_w1280_p9_a.jpg', 'previews/preview_w1280_p9_a.webp', 'previews/preview_w1280_p9_a.png', + ]); + }); + + it('does not duplicate the legacy key when the source already is a lowercase .jpg', () => { + const keys = imageProcessor.previewTierKeys({ id: 9, path: 'e/a.jpg', source_origin: 'managed' }); + expect(keys.filter((k) => k.includes('w640'))).toEqual([ + 'previews/preview_w640_p9_a.jpg', 'previews/preview_w640_p9_a.webp', + ]); + }); +}); diff --git a/backend/src/services/imageProcessor.js b/backend/src/services/imageProcessor.js index 26a0039e..c7e462c6 100644 --- a/backend/src/services/imageProcessor.js +++ b/backend/src/services/imageProcessor.js @@ -1021,14 +1021,53 @@ async function isPreviewValid(previewPath) { */ function previewTierKeys(photo) { if (!photo) return []; + return PREVIEW_WIDTHS + .filter((w) => w !== DEFAULT_PREVIEW_LONG_EDGE) + .flatMap((w) => previewTierKeyCandidates(photo, w)); +} + +/** + * The output basename every preview tier of a photo is written under. + * + * ALWAYS scoped by photo id, managed rows included. Basenames are not unique + * across events — two galleries can each hold an IMG_0001.jpg — and because a + * tier is served straight from a cache hit without re-reading the source, a + * collision hands one gallery's photo to another. Scoping by id is what makes + * the cache safe to trust; it is not a tidiness choice. + */ +function previewTierBasename(photo) { const isExternal = photo.source_origin === 'external' || photo.source_origin === 'reference'; const sourceBasename = path.basename( (isExternal ? (photo.external_relpath || photo.filename) : photo.path) || `photo-${photo.id}` ); - const outputBasename = `p${photo.id}_${sourceBasename}`; - return PREVIEW_WIDTHS - .filter((w) => w !== DEFAULT_PREVIEW_LONG_EDGE) - .map((w) => path.posix.join('previews', `preview_w${w}_${outputBasename}`)); + return `p${photo.id}_${sourceBasename}`; +} + +/** + * Every storage key one preview tier of a photo can live under, most likely + * first. + * + * generatePreviewImage rewrites the extension to match the encoding it chose + * — `.jpg`, or `.webp` for a source with alpha or more than one frame — and + * which one that is cannot be known without probing the source, which is the + * work the cache exists to skip. The lookup used to probe a single key that + * kept the SOURCE extension, so for anything but a lowercase `.jpg` source + * (`.png`, `.JPG`, `.heic`, RAW) it never matched what had been written: every + * tier request re-ran Sharp, and cleanup, deriving the same key, never found + * the files it left behind. + * + * The source-extension key stays in the list, last: previews written before + * the extension rewrite carry it (JPEG bytes under a `.png` name, still + * served as JPEG), and they have to be found by cleanup as well as lookup. + */ +function previewTierKeyCandidates(photo, width) { + const outputBasename = previewTierBasename(photo); + const stem = `preview_w${width}_`; + const base = outputBasename.replace(/\.[^./\\]+$/, ''); + const keys = [`${stem}${base}.jpg`, `${stem}${base}.webp`]; + const legacy = `${stem}${outputBasename}`; + if (!keys.includes(legacy)) keys.push(legacy); + return keys.map((k) => path.posix.join('previews', k)); } /** Best-effort removal of every responsive tier for a photo. */ @@ -1196,23 +1235,17 @@ async function ensurePreviewImageAtWidthUnguarded(photo, width) { if (!event) return null; const isExternal = photo.source_origin === 'external' || photo.source_origin === 'reference'; - const sourceBasename = path.basename( - (isExternal ? (photo.external_relpath || photo.filename) : photo.path) || `photo-${photo.id}` - ); - // ALWAYS scoped by photo id, managed rows included. Basenames are not unique - // across events — two galleries can each hold an IMG_0001.jpg — and because a - // tier is served straight from a cache hit without re-reading the source, a - // collision hands one gallery's photo to another. Scoping by id is what makes - // the cache safe to trust; it is not a tidiness choice. - const outputBasename = `p${photo.id}_${sourceBasename}`; - const key = path.posix.join('previews', `preview_w${width}_${outputBasename}`); + const outputBasename = previewTierBasename(photo); // Cache hit: nothing to do. This is the common path once a gallery has been - // browsed at a given size. - try { - if (await storage.stat(key)) return key; - } catch (e) { - // fall through and regenerate + // browsed at a given size. Every key the tier can have been written under + // is probed — see previewTierKeyCandidates for why there is more than one. + for (const key of previewTierKeyCandidates(photo, width)) { + try { + if (await storage.stat(key)) return key; + } catch (e) { + // fall through to the next candidate, then regenerate + } } try {