feat(digital-archive): digital-asset manifest schema + watermark trigger - #2323
Merged
Conversation
…trigger Declares digital_asset_store, digital_asset, digital_asset_file, and catalog_export_flag_state in wxyc_schema (migration 0158) — the manifest substrate for the digital archive (epic WXYC/wxyc-dj-ios#135): which library.id has audio, in which store, under what object key, with what codec and checksums. Generalized so hand-uploaded auto-DJ MP3s and future verified CD rips share the same tables rather than two schemas; the rip-evidence columns on digital_asset are all nullable now so the CD-rip phase adds rows, not columns. A second migration (0159) attaches library_watermark's statement trigger to digital_asset, narrowed to UPDATE OF status, library_id (mirroring 0142's narrowing of the library trigger — those are the two columns that affect export eligibility), and adds a callable touch_library_watermark_now() wrapper reproducing 0104's monotonic GREATEST(now(), last_modified_at) formula exactly, since the existing trigger function is RETURNS trigger and can't be invoked from application code. Both migrations are additive DDL only, with no data backfill. Unit tests pin the generated DDL by regex, following the schema.library-watermark.test.ts / schema.concerts.test.ts pattern. Verified by hand against a disposable PostgreSQL 14 container (matching prod): the full migration chain applies cleanly, touch_library_watermark_now() advances the watermark, and the trigger correctly narrows — an INSERT or a status-column UPDATE advances the watermark, an unrelated-column UPDATE does not, and DELETE still advances it.
Schema constraint shape reportProbed: This PR adds:
one or more probes failed; see above. The check status is non-blocking. |
`digital_asset.library_id` is a real FK at `library.id`, so the dedup job's enforced-fk-actions invariant test correctly refused a schema that added it without a matching `FK_TARGETS` entry: on a merge the loser's assets would have been left pointing at a row the merge deletes. `(library_id, provenance, disc_number)` is UNIQUE with all three NOT NULL (`disc_number` defaults to 1), so the key is total and needs no `uniqueWhenNull`. Where both sides of a merge hold the same (provenance, disc_number), the loser's binding row is dropped as a collision; the audio objects themselves hang off `digital_asset_file` under the store and are untouched, so nothing in object storage is orphaned.
The FK_TARGETS comment claimed the collision-delete "drops only the binding row" and that "nothing in object storage is orphaned". Both halves are false: `digital_asset_file.asset_id` cascades off `digital_asset(id)`, so dropping a colliding binding row takes every file row under it — object_key, the digests, bitrate, duration, tags — and `fillNullsFromLoser` carries none of it across first. The objects survive in the store with nothing pointing at them, and a comment asserting otherwise is exactly what would stop someone building the reclamation path. The delete stays, because BS#2319's bind job discovers assets by scanning the store, so an orphan is re-bindable rather than lost. What was missing is that nothing said so: `repointTarget` now names the orphaned object keys before the cascade removes the rows that identify them. Preferring the servable asset over the survivor's arbitrary one is the correct behaviour and is deliberately left to its own change rather than smuggled into a schema PR. Also pins the new FK's enforced delete action in the integration spec's EXPECTED map — NO ACTION is deliberate here, so an incomplete repoint fails loudly instead of cascading — corrects the now-stale "three of thirteen sites" counts in the header and README, and widens the rip-evidence nullability regex to end-of-line: it stopped at the first whitespace, so for a multi-word type like `timestamp with time zone` it captured only `"ripped_at" timestamp` and could never observe a NOT NULL.
…atalog The previous commit incremented a stale count rather than correcting one. "Fourteen sites" matches no inventory in the repo: FK_TARGETS holds 15 entries, the database enforces 13 FKs on `library.id`, and the README table lists 16 rows. The meaningful denominator for "still raise" is the 13 real FK constraints, since the two FK-less entries in FK_TARGETS cannot raise either. The adjacent "six of the sites cascade" was stale the same way — the enforced count is seven — and the README total predates `digital_asset`. The comment claiming this is "the only collision-delete that reaches beyond its own row" was also literally false: `flowsheet.rotation_id` references `rotation`, so a rotation collision-delete reaches beyond too. That FK is ON DELETE set null, so it loses a link and never a row — which is the distinction the warning exists for, and the claim is now narrowed to destroying rows rather than merely reaching. The EXPECTED omission this PR already fixed could recur, because the test written to catch omissions compares the catalog against FK_TARGETS while its sibling iterates EXPECTED — so an FK added with its FK_TARGETS entry but no EXPECTED entry passes both, which is exactly what happened. The catalog-to-code test now asserts against both maps, closing the hole rather than the instance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
digital_asset_store,digital_asset,digital_asset_file, andcatalog_export_flag_stateinwxyc_schema(migration0158_digital-asset-manifest) — the manifest substrate for the digital archive: whichlibrary.ids have audio, in which store, under what object keys, with what codec and checksums. Generalized so today's hand-uploaded auto-DJ MP3s (provenance = 'rotation_upload') and tomorrow's verified CD rips ('cd_rip') share the same tables; the rip-evidence columns ondigital_assetare all nullable now so the CD-rip phase adds rows, not columns.library_watermarkstatement trigger todigital_asset, narrowed toUPDATE OF status, library_id(mirroring migration0142's narrowing of thelibrarytrigger — those are the two columns that affect export eligibility), and adds a callabletouch_library_watermark_now()wrapper reproducing migration0104's monotonicGREATEST(now(), last_modified_at)formula exactly, since the existing trigger function isRETURNS triggerand can't be invoked from application code (migration0159_digital-asset-watermark).Closes #2318
Test plan
npm run lint:migrations,scripts/check-precondition-guards.sh,node scripts/check-bulk-update-analyze.mjs --strictall greennpm run typecheck,npm run lint,npm run format:checkall greennpm run test:unit— 503 suites / 8887 tests pass, including the newtests/unit/database/schema.digital-asset.test.tsSELECT wxyc_schema.touch_library_watermark_now()advanceslibrary_watermark.last_modified_at; anINSERTor astatus-columnUPDATEondigital_assetadvances the watermark, an unrelated-columnUPDATEdoes not, andDELETEstill advances it