From b743ea0398c7ec0178cf29107629a7877442c494 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Fri, 17 Jul 2026 22:35:11 +0200 Subject: [PATCH] fix(uploads): apply RAW extraction in the actual async ingest path (codex review of #833) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RAW/DNG extraction was only wired into processUploadedPhotos() (the synchronous path), but real uploads queue to 'pending' and are handled by the background worker → processPhoto(), which generated the thumbnail + dimensions directly from the DNG (both fail) and then marked the photo 'complete' — success with no thumbnail. Wire withProcessableImage() into processPhoto() (the live path) and into photoReplacementService.replacePhoto() (replace-by-name), so all three ingest paths extract the embedded JPEG preview for RAW. Updates the processPhoto test's imageProcessor mock with the new withProcessableImage dependency (pass-through for ordinary images). --- .../photoProcessor.processPhoto.test.js | 7 ++++ backend/src/services/photoProcessor.js | 36 ++++++++++++------- .../src/services/photoReplacementService.js | 36 +++++++++++-------- 3 files changed, 51 insertions(+), 28 deletions(-) diff --git a/backend/__tests__/services/photoProcessor.processPhoto.test.js b/backend/__tests__/services/photoProcessor.processPhoto.test.js index 763b213e..d5935300 100644 --- a/backend/__tests__/services/photoProcessor.processPhoto.test.js +++ b/backend/__tests__/services/photoProcessor.processPhoto.test.js @@ -75,6 +75,13 @@ jest.mock('../../src/services/imageProcessor', () => { withLocalCopy: jest.fn(async (key, fn) => fn(`/tmp/local-copy-${require('path').basename(key)}`) ), + // Pass-through for ordinary (non-RAW) images: returns the path unchanged + // with a no-op cleanup, matching the real helper's behaviour for jpg/png. + withProcessableImage: jest.fn(async (localPath) => ({ + path: localPath, + outputBasename: undefined, + cleanup: () => {}, + })), }; }); diff --git a/backend/src/services/photoProcessor.js b/backend/src/services/photoProcessor.js index eec86982..986df36a 100644 --- a/backend/src/services/photoProcessor.js +++ b/backend/src/services/photoProcessor.js @@ -465,21 +465,31 @@ async function processPhoto(photoId) { if (result.metadata.height) updateData.height = result.metadata.height; } } else { + // RAW/DNG can't be sharp-decoded directly — extract the embedded JPEG + // preview and thumbnail/measure that. Pass-through for ordinary images. + // This is the ASYNC worker path (backgroundProcessor → processPhoto), the + // one real uploads actually take; the synchronous processUploadedPhotos() + // has the same handling. + const proc = await withProcessableImage(localPath, photo.filename); 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; + try { + const thumbnailPath = await generateThumbnail(proc.path, { outputBasename: proc.outputBasename }); + if (thumbnailPath) updateData.thumbnail_path = thumbnailPath; + } catch (e) { + logger.warn(`processPhoto: thumbnail generation failed for ${photoId}`, { error: e.message }); } - } catch (e) { - logger.warn(`processPhoto: dimensions extraction failed for ${photoId}`, { error: e.message }); + try { + const sharp = require('sharp'); + const metadata = await sharp(proc.path).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 }); + } + } finally { + await proc.cleanup(); } } }); diff --git a/backend/src/services/photoReplacementService.js b/backend/src/services/photoReplacementService.js index 3680cbbd..8799825c 100644 --- a/backend/src/services/photoReplacementService.js +++ b/backend/src/services/photoReplacementService.js @@ -10,7 +10,7 @@ const path = require('path'); const fsp = require('fs/promises'); const sharp = require('sharp'); const { db } = require('../database/db'); -const { generateThumbnail, extractCaptureDate } = require('./imageProcessor'); +const { generateThumbnail, extractCaptureDate, withProcessableImage } = require('./imageProcessor'); const { generatePhotoFilename } = require('../utils/filenameSanitizer'); const watermarkGeneratorService = require('./watermarkGeneratorService'); const { getStorage } = require('./storage'); @@ -61,24 +61,30 @@ async function replacePhoto(existingPhoto, newFileTempPath, { originalFilename, // No EXIF — keep null } - let width = null; - let height = null; - try { - const metadata = await sharp(newFileTempPath).metadata(); - width = metadata.width || null; - height = metadata.height || null; - } catch { - // Non-image or corrupt - } - const stats = await fsp.stat(newFileTempPath); - // Generate new thumbnail FROM the local temp before uploading the original. + // RAW/DNG isn't sharp-decodable — extract the embedded JPEG preview first + // (pass-through for ordinary images), then measure + thumbnail that. Mirrors + // the ingest paths (processPhoto / processUploadedPhotos). + let width = null; + let height = null; let thumbnailPath = null; + const proc = await withProcessableImage(newFileTempPath, originalFilename); try { - thumbnailPath = await generateThumbnail(newFileTempPath); - } catch { - logger.warn('Failed to generate thumbnail for replaced photo', { photoId: existingPhoto.id }); + try { + const metadata = await sharp(proc.path).metadata(); + width = metadata.width || null; + height = metadata.height || null; + } catch { + // Non-image or corrupt + } + try { + thumbnailPath = await generateThumbnail(proc.path, { outputBasename: proc.outputBasename }); + } catch { + logger.warn('Failed to generate thumbnail for replaced photo', { photoId: existingPhoto.id }); + } + } finally { + await proc.cleanup(); } // Delete old assets BEFORE uploading the new key — if they share the path