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

Stable twin of #1204.

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, 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.

The card shows the count only when it is non-zero, and 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: 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 on this
branch.
This commit is contained in:
Paul Nothaft
2026-08-27 08:44:32 +02:00
committed by GitHub
parent 292dd4fa09
commit 74ff236b51
7 changed files with 73 additions and 5 deletions
@@ -198,5 +198,30 @@ describe('capture date backfill (#1172)', () => {
expect(new Date((await db('photos').where({ id: photoId }).first()).captured_at).toISOString()).toBe(claimed);
expect(done.body.lastResult.success).toBe(0);
// Read but not written, so it is accounted for rather than dropped.
expect(done.body.lastResult.skipped).toBe(1);
});
it('does not date a row whose file was replaced while it was reading (#1201)', async () => {
// replacePhoto swaps a NEW file under an existing row and rewrites
// path/filename (reachable from replace_by_name). The replacement carries
// no date of its own, so captured_at is still NULL and the whereNull guard
// alone would let the previous file's EXIF date land on it. The write is
// fenced on the identity that was read, so the row is skipped instead —
// and not counted as updated either.
const { photoId } = await seed({ relpath: 'orig.jpg', exifIso: '2026-06-03T09:00:00Z' });
const res = await request(app).post('/api/admin/photos/repair-capture-dates');
expect(res.body.count).toBe(1);
// Simulate the replacement landing before the loop writes.
await db('photos').where({ id: photoId })
.update({ path: 'capfill/replaced.jpg', filename: 'replaced.jpg' });
const done = await settle();
expect((await db('photos').where({ id: photoId }).first()).captured_at).toBeNull();
expect(done.body.lastResult.success).toBe(0);
// Not an error and not "no EXIF" — the date was found, another writer just
// got there first. It stays in the backlog for the next run.
expect(done.body.lastResult).toMatchObject({ noExif: 0, failed: 0, skipped: 1 });
});
});
+27 -5
View File
@@ -350,6 +350,7 @@ router.post('/repair-capture-dates', adminAuth, requirePermission('settings.edit
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
@@ -424,11 +425,29 @@ router.post('/repair-capture-dates', adminAuth, requirePermission('settings.edit
// 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 (#1201):
// 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...`);
@@ -443,12 +462,15 @@ router.post('/repair-capture-dates', adminAuth, requirePermission('settings.edit
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();