* fix(preview): generate lightbox previews for external/reference photos (#1078) Stable twin of the main-line fix. ensurePreviewImage() resolved its source only via resolvePhotoStorageKey(), which returns null for external/reference photos by design — those live on a media mount outside the managed storage tree. The null went straight into withLocalCopy(), which throws, so the preview route fell back to redirecting at the full-size original. Galleries whose photos are all external got no benefit from the preview tier (#492): guests paid 5-12 MB on every lightbox open. Add the external branch ensureThumbnail() already has: resolve via resolvePhotoFilePath() and feed the mount path to generatePreviewImage() directly, with an ext<id>_ output basename. generatePreviewImage() on this branch hardcoded path.basename(imagePath) and ignored options.outputBasename, so it needs the same one-line honouring that generateThumbnail() already does — without it two events referencing the same NAS basename collide on one preview key. Also return null rather than throwing for a row with no source_origin in a reference-mode event, whose mode falls back to the event's. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc * fix(preview): select the columns the external branch needs on bulk regenerate POST /api/admin/thumbnails/regenerate-previews selected only id, event_id, path, media_type, mime_type and preview_path, so photo.source_origin was undefined by the time ensurePreviewImage branched on it. Every external row in a reference gallery took the managed path, resolvePhotoStorageKey returned null for it, and the endpoint reported success while generating nothing. Add source_origin, external_relpath and filename to the select, plus a source-inspection test pinning the caller contract and a service-level test showing a column-starved row is indistinguishable from a managed one. Claude-Session: https://claude.ai/code/session_01Ra4hcsYiKuQLbbRsg6EjAc --------- Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
co-authored by
Paul Nothaft
parent
45ffe64b7c
commit
6df42ab22c
@@ -201,7 +201,13 @@ router.post('/regenerate-previews', adminAuth, requirePermission('photos.edit'),
|
||||
try {
|
||||
const { eventId } = req.body;
|
||||
|
||||
let query = db('photos').select('id', 'event_id', 'path', 'media_type', 'mime_type', 'preview_path');
|
||||
// source_origin/external_relpath/filename are what ensurePreviewImage
|
||||
// branches on for external/reference rows (#1078) — without them every
|
||||
// external photo looks managed here and generation is skipped.
|
||||
let query = db('photos').select(
|
||||
'id', 'event_id', 'path', 'media_type', 'mime_type', 'preview_path',
|
||||
'source_origin', 'external_relpath', 'filename'
|
||||
);
|
||||
if (eventId) query = query.where('event_id', eventId);
|
||||
// Skip videos — preview tier is image-only.
|
||||
query = query.where(function() {
|
||||
|
||||
@@ -498,7 +498,10 @@ async function ensureHeroImage(photo) {
|
||||
* thumbnails or heroes.
|
||||
*/
|
||||
async function generatePreviewImage(imagePath, options = {}) {
|
||||
const filename = path.basename(imagePath);
|
||||
// outputBasename lets callers disambiguate sources that share a basename
|
||||
// (external mounts, see ensurePreviewImage) — same contract as
|
||||
// generateThumbnail.
|
||||
const filename = options.outputBasename || path.basename(imagePath);
|
||||
const previewFilename = `preview_${filename}`;
|
||||
const previewRelKey = path.posix.join('previews', previewFilename);
|
||||
const storage = getStorage();
|
||||
@@ -579,17 +582,27 @@ async function isPreviewValid(previewPath) {
|
||||
* Lazy-generate the preview image for a photo if missing or invalid.
|
||||
* Returns the storage key or null on failure (callers fall back to
|
||||
* the original URL so the lightbox never shows a broken image).
|
||||
*
|
||||
* Handles both managed photos (via the storage backend, possibly S3) and
|
||||
* external/reference photos (#1078 — sourced from a local mount outside the
|
||||
* managed storage tree). Externals used to have no branch here at all:
|
||||
* resolvePhotoStorageKey returns null for them by design, that null reached
|
||||
* withLocalCopy, and the throw put every lightbox open back on the full-size
|
||||
* original — the exact cost the preview tier (#492) exists to avoid.
|
||||
*/
|
||||
async function ensurePreviewImage(photo) {
|
||||
const { resolvePhotoStorageKey } = require('./photoResolver');
|
||||
const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('./photoResolver');
|
||||
|
||||
let sourceKey;
|
||||
let event;
|
||||
try {
|
||||
const event = await db('events').where('id', photo.event_id).first();
|
||||
sourceKey = resolvePhotoStorageKey(event, photo);
|
||||
event = await db('events').where('id', photo.event_id).first();
|
||||
} catch (e) {
|
||||
const msg = (e && e.message) ? e.message : String(e);
|
||||
logger.error(`Failed to resolve original key for preview (photo ${photo.id}): ${msg}`);
|
||||
logger.error(`Failed to load event for preview (photo ${photo.id}): ${msg}`);
|
||||
return null;
|
||||
}
|
||||
if (!event) {
|
||||
logger.error(`ensurePreviewImage: event ${photo.event_id} not found for photo ${photo.id}`);
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -599,9 +612,46 @@ async function ensurePreviewImage(photo) {
|
||||
logger.warn(`Invalid preview detected for photo ${photo.id}, regenerating…`);
|
||||
}
|
||||
|
||||
const newPreviewPath = await withLocalCopy(sourceKey, (localPath) =>
|
||||
generatePreviewImage(localPath, { regenerate: true })
|
||||
);
|
||||
const isExternal = photo.source_origin === 'external' || photo.source_origin === 'reference';
|
||||
|
||||
let newPreviewPath;
|
||||
if (isExternal) {
|
||||
// Mirrors ensureThumbnail's external branch: the source is a direct fs
|
||||
// read off the mount, so no withLocalCopy. The per-photo outputBasename
|
||||
// keeps two events that reference the same NAS basename from clobbering
|
||||
// each other's preview.
|
||||
let localPath;
|
||||
try {
|
||||
localPath = resolvePhotoFilePath(event, photo);
|
||||
} catch (e) {
|
||||
logger.error(`Failed to resolve external file for preview (photo ${photo.id}): ${e.message}`);
|
||||
return null;
|
||||
}
|
||||
const sourceBasename = path.basename(photo.external_relpath || photo.filename || `photo-${photo.id}`);
|
||||
const outputBasename = `ext${photo.id}_${sourceBasename}`;
|
||||
logger.info(`Ensuring preview for external photo ${photo.id} from ${localPath}`);
|
||||
newPreviewPath = await generatePreviewImage(localPath, { regenerate: true, outputBasename });
|
||||
} else {
|
||||
let sourceKey;
|
||||
try {
|
||||
sourceKey = resolvePhotoStorageKey(event, photo);
|
||||
} catch (e) {
|
||||
const msg = (e && e.message) ? e.message : String(e);
|
||||
logger.error(`Failed to resolve original key for preview (photo ${photo.id}): ${msg}`);
|
||||
return null;
|
||||
}
|
||||
if (!sourceKey) {
|
||||
// Reference-mode event holding a row with no source_origin: the mode
|
||||
// falls back to the event's and resolvePhotoStorageKey returns null.
|
||||
// Honour the documented null-on-failure contract instead of feeding
|
||||
// null into withLocalCopy, which throws out of this function.
|
||||
logger.warn(`No managed storage key for preview (photo ${photo.id}); skipping preview generation`);
|
||||
return null;
|
||||
}
|
||||
newPreviewPath = await withLocalCopy(sourceKey, (localPath) =>
|
||||
generatePreviewImage(localPath, { regenerate: true })
|
||||
);
|
||||
}
|
||||
|
||||
if (newPreviewPath) {
|
||||
await db('photos').where({ id: photo.id }).update({ preview_path: newPreviewPath });
|
||||
|
||||
Reference in New Issue
Block a user