stable
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Paul NothaftandPaul Nothaft
|
2b1c3588ae |
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]> |
||
|
Paul NothaftandPaul Nothaft
|
e9fcf4960e |
fix(external-media): one row per external file per event (#1162) (#1173)
* fix(external-media): one row per external file per event (#1162) Stable twin of #1167. Two overlapping import-external runs against the same event inserted every file twice. The route checked for an existing external_relpath and then inserted, with an fs.stat and a sharp().metadata() read sitting in between — a window wide enough for both runs to see "not there". A reporter's event held 8004 rows for 6012 distinct paths. Nothing at the storage layer stopped it: migration 041 created only a NON-unique (event_id, source_origin) index. - migration 176 removes the existing duplicates and adds a partial unique index on (event_id, external_relpath), verified against the catalog afterwards — a failed CREATE INDEX raises 23505 on Postgres, which run-migrations-safe treats as "schema already exists" and would record as applied on an install that never got the index. - dependent rows are removed explicitly rather than by cascade: PicPeak never sets `PRAGMA foreign_keys = ON`, so on SQLite the declared CASCADE is inert and a bare delete strands feedback and access-log rows. Guest feedback moves to the survivor instead of being discarded, keyed on guest identity the way feedbackService defines it, and the survivor's denormalized counters are recomputed. - the route treats a unique violation as a skip, so a writer this process cannot see converges instead of duplicating, and a second import while one is running gets a 409. - a .picpeak taken before migration 176 carries exactly these duplicates, and suspending FK enforcement does not suspend a unique index — so the restore drops the index for the load and rebuilds it after running the same dedupe. Divergences from the main twin, both because the feature is absent here: faces (no faceProcessor, so no purgePhotoFaces reconciliation — the rows are still deleted so nothing dangles), admin marks, transfer membership, and photos.view_count/download_count. The service guards each on hasTable / hasColumn, so those branches simply do not fire. Verified on this branch: 36 new tests pass; full suite leaves the same 5 pre-existing failures as origin/stable, unchanged. * fix(external-media): invalidate the download zip when duplicates are removed (#1162) External review. Same fix as the main twin. The pre-built "download everything" archive still contained the duplicate rows the dedupe had just deleted, so guests kept receiving them. Every ordinary photo-deletion path calls downloadZipService.invalidate for exactly this reason. The columns are cleared rather than the service being called: that service carries debounce timers and a regeneration queue, which a migration should not start. getZipInfo already treats a cleared record as a cache miss and rebuilds on the next request. The stale object is left in storage, as elsewhere. --------- Co-authored-by: Paul Nothaft <[email protected]> |