fix(external-media): store external paths from the media root (#1163) (#1174)

* fix(external-media): store external paths from the media root (#1163)

Stable twin of #1168. Stacked on the #1162 twin, which supplies
deleteDuplicatePhotos.

Importing a second folder into an event silently invalidated every photo
already in it. external_relpath was stored relative to events.external_path,
and every import overwrites that column, so the older rows were rebased onto
the new folder. Nothing errored and the grid still rendered — thumbnails are
written to local storage during the import while the base path is still
correct — so only the things that need the original broke. The reporter had
7547 of 8004 rows pointing into the void.

- external_relpath is now relative to EXTERNAL_MEDIA_ROOT, so a row is
  self-describing and nothing an admin does to the event can move it.
- migration 177 folds each event's base into its rows. Where the current
  resolution is missing it walks up for an ancestor holding a file of the same
  name AND the size the import recorded — existence alone would let a deleted
  file adopt an unrelated namesake and serve the wrong original. Rows it
  cannot place keep resolving where they resolve today, and the probe is
  skipped entirely when the mount is unreachable.
- probing is read-only and runs first; the rewrites and the marker commit
  together, so an interrupted fold cannot be folded twice.
- rewrites are staged through a per-row parking value, because a final path
  can equal another row's current one; and migration 177 re-throws without the
  driver's error code, which run-migrations-safe would otherwise read as
  "schema already exists".
- the fold also runs after a .picpeak restore, since knex_migrations is
  excluded from the archive, and a failure there is reported rather than
  presented as a clean restore.
- drops the duplicate-leaf-segment guess in photoResolver, which papered over
  this same double-prefixing.

Divergence from the main twin: no face-scan requeue reordering. Face
recognition is main-only, so the hazard of queueing rows against unconverted
paths does not exist on this branch — in picpeakImportService or in
restoreService.

Verified on this branch: 23 new tests pass, and the four suites carrying
base-relative fixtures were updated. Full suite leaves the same 5 pre-existing
failures as origin/stable, unchanged.

* fix(external-media): the fold's staging value must be storable on Postgres (#1163)

External review found this on this branch first; it was on both.

The two-pass rewrite parks each row on a temporary value, and that value was
written with a leading NUL. SQLite stores NUL in TEXT without complaint;
Postgres rejects it outright — "invalid byte sequence for encoding UTF8: 0x00"
— so migration 177 rolled back on exactly the installs that need the two-pass
repair, and only on the engine most of them run. Restores hit the same wall
and reported the conversion as failed.

The prefix is ordinary text now. Adds a gated Postgres test alongside the
existing picpeakRestorePg one, because a SQLite-only suite structurally cannot
catch this class: restoring the NUL makes exactly the two-pass repair case
fail with that error, and nothing else.

---------

Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Paul Nothaft
2026-08-26 09:00:05 +02:00
committed by GitHub
co-authored by Paul Nothaft
parent e9fcf4960e
commit 2b1c3588ae
15 changed files with 1187 additions and 56 deletions
+5
View File
@@ -240,6 +240,11 @@ router.post('/picpeak/import', adminAuth, requirePermission('backup.restore'), p
tables: result.tables,
filesRestored: result.filesRestored,
usesExternalMedia: result.usesExternalMedia,
// False when the pre-#1163 external-path conversion failed. The rows and
// files are in place, but no external original resolves until it is
// retried — the UI must say so rather than showing a plain success.
externalPathsConverted: result.externalPathsConverted !== false,
externalPathError: result.externalPathError || null,
crossEngine: result.crossEngine,
sessionInvalidated: true,
});
+18 -3
View File
@@ -81,6 +81,14 @@ router.post('/events/:id/import-external', adminAuth, requirePermission('photos.
if (!event) return res.status(404).json({ error: 'Event not found' });
const baseAbs = resolveExternalPath({ external_path }, '');
// What gets STORED on the row (#1163). `f.rel` stays relative to the
// imported folder because the type inference below reads its first segment
// ('individual' / 'collages'); external_relpath is written relative to
// EXTERNAL_MEDIA_ROOT so the row does not depend on a column this very
// handler is about to overwrite.
const basePrefix = String(external_path).replace(/^\/+|\/+$/g, '');
const toRootRelative = (rel) => (basePrefix ? path.join(basePrefix, rel) : rel);
// Collect files
const files = recursive ? await walkDir(baseAbs, baseAbs) : (await fs.readdir(baseAbs, { withFileTypes: true }))
.filter(e => e.isFile())
@@ -127,6 +135,8 @@ router.post('/events/:id/import-external', adminAuth, requirePermission('photos.
if (segs[0] === map.collages) type = 'collage';
if (segs[0] === map.individual) type = 'individual';
const relFromRoot = toRootRelative(f.rel);
try {
// Fast path only. This SELECT settles the common case — a re-import of
// a folder already in the event — without paying for a stat and a
@@ -136,7 +146,7 @@ router.post('/events/:id/import-external', adminAuth, requirePermission('photos.
// index from migration 176 is the guard, and the catch below is how
// this loop converges when it fires.
const exists = await db('photos')
.where({ event_id: eventId, external_relpath: f.rel })
.where({ event_id: eventId, external_relpath: relFromRoot })
.first();
if (exists) { skipped++; continue; }
const stats = await fs.stat(f.full);
@@ -166,7 +176,7 @@ router.post('/events/:id/import-external', adminAuth, requirePermission('photos.
width,
height,
source_origin: 'external',
external_relpath: f.rel
external_relpath: relFromRoot
})
.returning('id');
} catch (insertErr) {
@@ -214,7 +224,12 @@ router.post('/events/:id/import-external', adminAuth, requirePermission('photos.
}
}
// Update event fields
// Safe for existing EXTERNAL rows as of #1163. It was not: relpaths were
// stored relative to external_path, so overwriting the column here rebased
// every row already in the event onto the new folder — quietly, because
// their thumbnails were already on local disk and the grid carried on
// rendering. Rows now carry a root-relative path and this write cannot
// reach them.
await db('events').where('id', eventId).update({ source_mode: 'reference', external_path });
await logActivity(