paul/picpeak
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(images): fence the capture-date backfill on the file it read (#1201) (#1204)

Browse Source 
The capture-date backfill committed its result keyed on the row id alone. It
snapshots every candidate up front, then walks them one at a time reading
originals off S3 or a NAS mount — a pass that can run for many minutes.

replacePhoto, reachable from the replace_by_name upload path, swaps a NEW file
under an existing row and rewrites path/filename. A replacement landing inside
that window carries no date of its own, so captured_at was still NULL, the
whereNull guard passed, and the previous file's EXIF date was written onto the
new photo. Silent: nothing errored, the run reported it as a success, and the
gallery just sorted that photo to the wrong place.

Fenced on path and filename as well as the id — the same fence #1199 put on the
orientation backfill for the same reason — so a replaced row matches zero rows
and is skipped. The candidate query already selects both columns, so no query
change. Knex renders a null value in the object form as `is null` on both the pg
and sqlite3 clients, so a row with a NULL path still matches itself.

Those skipped candidates are now counted rather than dropped. replacePhoto is
not the only writer of path/filename — eventRenameService rewrites both on an
event rename, which is not a content change — and another writer filling
captured_at first lands in the same place. Without a counter they fell out of
the run's arithmetic entirely: success + noExif + failed no longer added up to
the count the operator was shown when they started the job, on the card as well
as in the log.

The card shows the count only when it is non-zero, the same shape the
orientation job uses for staleTiers. The wording states what is known — changed
by something else, not updated — rather than promising a retry: for the
already-dated case there is nothing to retry, and the Missing Capture Date
figure above is what says whether work is left. Locale coverage matches the
staleTiers key (en, de, fr, sl), with the defaultValue carrying the rest.

Regression test: a replacement landing mid-run leaves captured_at NULL and is
not counted as updated. Verified to fail against the unfenced code.
This commit is contained in:
Paul Nothaft
2026-08-27 08:44:19 +02:00
committed by GitHub
parent fb7af502ec
commit cec8eff70c
7 changed files with 74 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/routes/adminPhotoDimensions.js
+27 -5
View File
@@ -356,6 +356,7 @@ router.post('/repair-capture-dates', adminAuth, requirePermission('system.manage
let successCount = 0;
let missingCount = 0;
let errorCount = 0;
let skippedCount = 0;
let lostClaim = false;
// Same reasoning as the dimension repair: detached from the request, so
@@ -430,11 +431,29 @@ router.post('/repair-capture-dates', adminAuth, requirePermission('system.manage
// large library, and an import or a replacement finishing meanwhile
// has already written a date this pass would otherwise overwrite
// with the same-or-worse value.
//
// Fenced on path and filename as well as the id, for the same reason
// the orientation backfill below is (#1199): replacePhoto — reachable
// from the replace_by_name upload path (adminPhotos.js) — swaps a NEW
// file under an existing row and rewrites path/filename. That
// replacement carries no date of its own, so captured_at is still
// NULL and whereNull alone would let the previous file's EXIF date
// land on it. Matching the identity that was actually read means the
// update affects no rows and the row is simply skipped.
const updated = await db('photos')
.where({ id: photo.id })
.where({ id: photo.id, path: photo.path, filename: photo.filename })
.whereNull('captured_at')
.update({ captured_at: captured.toISOString() });
if (updated) successCount++;
// Counted, not dropped: without this a candidate that was read but
// not written falls out of the run's arithmetic entirely, and
// success + noExif + failed silently stops adding up to the count
// the operator was shown when they started it. Two ways to land
// here, both "another writer got there first" — the row was dated
// meanwhile (whereNull), or its file changed under us (the fence).
// Neither is an error and neither needs a retry: captured_at is
// still NULL for the fenced case, so the status endpoint keeps
// reporting it as backlog and the next run picks it up.
if (updated) successCount++; else skippedCount++;
if (successCount % 50 === 0 && successCount > 0) {
logger.info(`Capture date backfill progress: ${successCount} updated...`);
@@ -449,12 +468,15 @@ router.post('/repair-capture-dates', adminAuth, requirePermission('system.manage
logger.warn(`Capture date backfill stopped: claim taken over after ${successCount} updated, ${errorCount} errors`);
return;
}
await maintenanceJobs.release(JOB_CAPTURE_DATE_BACKFILL, token, { success: successCount, noExif: missingCount, failed: errorCount });
logger.info(`Capture date backfill complete: ${successCount} updated, ${missingCount} without EXIF, ${errorCount} errors`);
await maintenanceJobs.release(JOB_CAPTURE_DATE_BACKFILL, token, { success: successCount, noExif: missingCount, failed: errorCount, skipped: skippedCount });
logger.info(
`Capture date backfill complete: ${successCount} updated, ${missingCount} without EXIF, `
+ `${errorCount} errors, ${skippedCount} skipped (dated or replaced mid-run)`
);
} catch (err) {
logger.error('Capture date backfill aborted:', err);
await maintenanceJobs
.release(JOB_CAPTURE_DATE_BACKFILL, token, { success: successCount, noExif: missingCount, failed: errorCount, error: err.message })
.release(JOB_CAPTURE_DATE_BACKFILL, token, { success: successCount, noExif: missingCount, failed: errorCount, skipped: skippedCount, error: err.message })
.catch(() => {});
} finally {
lease.stop();