diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index f0f168c8..e20cd87f 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -32,7 +32,7 @@ Paths are relative to `packages/api/`. - Return the full `SpecimenSource` shape to every caller, not a per-endpoint subset (ADR 0007). - Load a Container's read view (placement, identity, Tags, unit, location, Specimen, Source) with `loadContainerReadViews` or `loadContainerReadViewsByIds` from `src/lib/container-read-view.ts`, not with per-endpoint queries (ADR 0009). - Map the read view to its read path's type: `EnrichedContainerApi` for list, derivations and PATCH, `EnrichedStorageContainer` for collection detail, and CSV row fields for Container export (ADR 0009). -- To choose which Containers an export or statistic covers, call `selectContainers` from `src/lib/container-selection/`, and filter a paginated Container query by Tags with `containerHasAllTags` (ADR 0011). +- To choose which Containers an export or statistic covers, call `selectContainers` from `src/lib/container-selection/`, and filter a paginated Container query by Tags with `containerHasAllTags` and by Location with `containerAtLocation` (ADR 0011). ### Routes diff --git a/CONTEXT.md b/CONTEXT.md index 99674c4f..617fc7a3 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -14,6 +14,38 @@ _Avoid_: End user, operator (without context) Lab staff responsible for long-term specimen storage, inventory accuracy, and sample retrieval. _Avoid_: Curator (unless that is your org's term) +### Roles + +Every account has one role. The API enforces it on each route, and the web app hides actions the role cannot take. + +**Admin**: +Can do everything a **Member** can, plus manage users, settings, **Reference data**, **Locations**, and every **Destructive action**. + +**Member**: +Lab staff who create and change **Operational data**: register, store, move, derive, and import. A member cannot take a **Destructive action**, except deleting a tutorial **Study** (short code starting with TUT). +_Avoid_: User (every role is a user) + +**Viewer**: +Can read data and run exports, but not create or change anything. + +**Destructive action**: +An action that removes or rewrites recorded **Operational data** or configuration in bulk, or that the app cannot undo. Admin-only. Today these are: + +- Deleting a **Collection** with its contents, and its preflight (removes every **Container** in it, orphaned **Specimens**, and optionally emptied **Subjects**) +- Deleting an empty **Collection** (Data Integrity) +- Deleting a **Control batch** (removes its **Specimens** and their **Containers**) +- Editing or deleting a **Derivation** record +- Deleting a non-tutorial **Study** (removes its **Subjects**, **Specimens**, and **Containers**) +- Deleting **Reference data**, a **Location**, or a user account + +Members may also take these actions, which cannot be undone but are part of everyday lab work, so they are not destructive actions: + +- Merging two **Subjects** +- Deleting a qPCR experiment with its plate layout and results +- Deleting one **Specimen** and its **Containers** from a **Control batch** + +_Avoid_: Calling an ordinary edit (renaming a **Collection**, changing a **Container**'s quantity, removing a **Tag** from a **Container**) a destructive action. + ### Provenance hierarchy **Study → Subject → Specimen** is the primary provenance chain. Most specimens trace back to a study participant or sample source through this path. @@ -25,11 +57,15 @@ The top-level organizational unit for a research project, clinical trial, or oth _Avoid_: Project (unless referring to non-study work), trial (as a synonym for study) **Subject**: -A participant or sample source within a study, identified by a unique name within that study. +A participant or sample source within a study, identified by a unique name within that study. Names compare ignoring case (ASCII letters, as SQLite's `NOCASE` does): a study can't have both `ABC-1` and `abc-1`, and creating, renaming or importing a Subject whose name differs from an existing one only by case is refused. The write paths enforce it; there is no database index for it, because a database from before the rule may already hold such names, and an index would stop it from starting. The integrity report lists those pairs for an operator to merge or rename. Decided in issue #124 (decision 6). _Avoid_: Patient (unless the study is clinical and that is your org's term), donor (unless that is your org's term) **Specimen**: The collected unit — a registered biological material from a specific collection event. One collection event, one specimen record. + +**Specimen identity**: +A Subject's Specimen is identified by its Subject, Specimen type and collection date: **Specimen registration** adds Containers to the Specimen with that combination instead of creating a second one. Existing duplicates stay as they are (issue #124, decision 7): no migration merges them or adds a unique index to an upgraded database, and the integrity report lists them. Migration 008 can create dated duplicates by converting legacy `M/D/YY` dates, and clearing a placeholder collection date can make a Specimen an undated duplicate of another. Known difference: fresh installs have the unique index `idx_specimen_study_subject_type_date` on (`study_subject_id`, `specimen_type_id`, `collection_date`) and upgraded databases do not. That index treats an empty collection date as distinct, so fresh installs also allow undated duplicates. +_Avoid_: Assuming the database refuses a duplicate Specimen; on an upgraded database only registration's lookup prevents one. _Avoid_: Sample (as the domain term; see **Sample** below) **Sample**: @@ -132,9 +168,13 @@ Where a **Container** sits in the physical storage hierarchy — its **grid posi _Avoid_: Enrichment (implementation term); conflating placement with specimen or source lookups; placing paper directly on a box or bag without a sheet **Grid position**: -The well or slot of a tube or well **Container** in its plate or box (e.g. A01). Optional: a tube or well may belong to a **Collection** with no grid position (legacy rows). At most one Container occupies a given grid position in a Collection; a missing position occupies no cell. Not applicable to **Paper**. +The well or slot of a tube or well **Container** in its plate or box, stored as a row letter plus a two-digit column (A01). One rule, the Grid position rule, says which positions a **Collection** has (micronix plate A01–H12, cryovial box A01–J10) and how a typed position maps onto one: `a1`, `A1` and `a01` all mean A01, and anything else, including a cell outside the plate or box, is not a Grid position. Every write that sets a position applies the rule and rejects a position it does not accept; every read that lays Containers out on the grid applies the same rule, so a position a write would reject places no Container (see **Unplaced container**). Migration 010 repairs the legacy malformed positions it can and leaves the rest as they were; the integrity report flags any stored position that fails the rule. Optional: a tube or well may belong to a **Collection** with no grid position (legacy rows). At most one Container occupies a given grid position in a Collection; a missing position occupies no cell. Not applicable to **Paper**. _Avoid_: Well (as the domain term); treating a missing position as occupying a slot; requiring every tube to have a position before it can exist in a Collection +**Unplaced container**: +A tube or well **Container** that belongs to a plate or box but occupies no cell: it has no **Grid position**, or its stored position fails the Grid position rule for that Collection. It still counts as part of the Collection. Plate and box pages list unplaced Containers in their own section, labelled with the stored value, and plate scan validation expects them rather than treating them as extras. +_Avoid_: Empty (an empty cell holds no Container); treating an unplaced Container as missing from its Collection + ### Export **Container selection**: @@ -218,6 +258,10 @@ _Avoid_: Stuffing paper spot ids into `collection.barcode`; sending JSON `null` The application — a specimen inventory and workflow system for research and clinical labs. _Avoid_: LIMS (unless describing the broader category externally), ELN, SRA submission system (not available today) +**Data fix**: +A one-time SQL change that one install's operator runs by hand against that install's database with `bun run data-fix` in `packages/api`, to carry out decisions about that install's own data. Unlike a numbered migration, it ships to no other install, so it may name that install's rows by id. The runner copies the database first, refuses a database below the current schema version, runs the fix in one transaction, and records it in `data_fix_log` so it runs only once. The EPPIcenter lab's fixes live in `ops/data-fixes/`. +_Avoid_: Migration (for a change to one install's data); a migration changes every install's schema. + ## Scope What SampleDB is and is not — for product descriptions and documentation. diff --git a/docs/adr/0005-wire-dto-omit-on-wire.md b/docs/adr/0005-wire-dto-omit-on-wire.md index 487687fa..3d2eed2b 100644 --- a/docs/adr/0005-wire-dto-omit-on-wire.md +++ b/docs/adr/0005-wire-dto-omit-on-wire.md @@ -14,7 +14,7 @@ SampleDB API responses use a **wire DTO** layer between persistence (Drizzle/SQL | **Wire** (HTTP JSON) | Key **omitted** when unset | | **Client** (web parse / UI) | TypeScript optional / `undefined` | -Persistence-shaped rows must not be spread directly into JSON responses. Route handlers return wire-shaped payloads via `c.json(...)`; **global `omitOnWireMiddleware`** applies the null-omission policy to every successful JSON response. +Persistence-shaped rows must not be spread directly into JSON responses. Route handlers return wire-shaped payloads via `c.json(...)`; **global `omitOnWireMiddleware`** applies the null-omission policy to every successful JSON response, except a JSON Container export (see Consequences). ### Two responsibilities (do not conflate) @@ -53,6 +53,7 @@ Middleware is **permanent infrastructure** for null omission — not a substitut - Expand `@sampledb/contract/wire` schemas incrementally where discriminated shape matters; do not wire-schema every CRUD route. - Web parsers import `@sampledb/contract/wire` for container responses; optional fields use `.optional()` in Zod, not `.nullish()`. - Inbound container writes use the unified `ContainerWriteInput` shape ([ADR 0006](./0006-unified-container-inbound-write.md)) — same placement/identity split as outbound wire, not flat legacy fields. +- A JSON Container export keeps its nulls: the handler calls `keepNullsOnWire` from `packages/api/src/middleware/omit-on-wire.ts`, and every row carries every export column, null when empty. An export is a file, and a row missing a key reads as a missing column (#122). No other response opts out. - Export columns split accordingly (`barcode`, `sublabel`, `sheet_name`). Stored export configurations migrate retired keys on read (`label` → `sheet_name`; append `sublabel` when `barcode` or `sheet_name` is present). ## Related ADRs diff --git a/ops/data-fixes/2026-11-01-eppicenter-legacy-cleanup.sql b/ops/data-fixes/2026-11-01-eppicenter-legacy-cleanup.sql new file mode 100644 index 00000000..d717d6a3 --- /dev/null +++ b/ops/data-fixes/2026-11-01-eppicenter-legacy-cleanup.sql @@ -0,0 +1,211 @@ +-- One-time data fix for the EPPIcenter lab's SampleDB database ONLY. +-- +-- DO NOT RUN THIS ON ANY OTHER DATABASE. It names rows by the ids they have in the EPPIcenter +-- production database (snapshot arwen_2026-10-31) and carries out decisions that lab made about +-- its own legacy data. On another install it would abort or, worse, change unrelated rows. +-- +-- Decisions: eppicenter/sampledb3 issue #124, comment "Decisions (2026-11-01)"; work in #126. +-- Run with `bun run data-fix` (packages/api/src/db/README.md, "One-time data fixes"). Every +-- step is guarded: if the database holds rows the lab did not review, the fix aborts and +-- changes nothing, so they can be reviewed first. +-- +-- 1. Placeholder collection dates (1970-01-01, 1999-09-09, 1899-12-30, 1111-01-01) become unknown. +-- 3. Containers commented "exhausted" with remaining quantity 1 get remaining quantity 0; the +-- comment stays. +-- 4. NA / N/A-style cryovial tube barcodes become empty (NULL, as for any tube without a barcode). +-- 5. Container 34513 (no tube or Paper row; Specimen 31863 keeps its other Container) is deleted. +-- 6. CTRL Subjects 3d7_0.05 (31047) and 3d7_10k (31049) are merged into 3D7_0.05 (66537) and +-- 3D7_10K (66543) the way the app's Subject merge does it. The app refuses new case-variant +-- Subject names; there is no database index for it. +-- 8. Empty plate 1760 `EXVIVO-27 ` (trailing space) is deleted; plate 1761 `EXVIVO-27` stays. +-- 9. Specimen type Polyclonal Ab (47) allows micronix tubes, so the 8 micronix tubes holding +-- Specimen 141173 are in an allowed Container type (decided 2026-11-02, follow-up on #124). +-- Decisions 2 (future dates) and 7 (duplicate Specimen identity) change no data; the integrity +-- report lists them. + +-- Setup: list the exhausted Containers the lab reviewed (decision 3) +CREATE TEMP TABLE __fix_exhausted (id INTEGER PRIMARY KEY);--> statement-breakpoint +-- Setup: reviewed exhausted Container ids +INSERT INTO __fix_exhausted (id) VALUES + (50465), (87730), (87732), (87735), (87736), (87737), (87738), (87739), (87741), (87742), + (87743), (87744), (87747), (87748), (87749), (87750), (87751), (87752), (87753), (87754), + (87756), (87757), (87759), (87762), (87763), (87765), (87766), (87768), (87769), (124552), + (124564), (124565), (124568), (124570), (124598), (124645), (124646), (136248), (140979), (143331), + (143335), (143339), (143343), (143347), (144277), (144318);--> statement-breakpoint +-- Setup: list the placeholder dates and the counts the lab reviewed (decision 1) +CREATE TEMP TABLE __fix_placeholder_dates (collection_date TEXT PRIMARY KEY, reviewed INTEGER NOT NULL);--> statement-breakpoint +-- Setup: placeholder dates and reviewed counts +INSERT INTO __fix_placeholder_dates (collection_date, reviewed) VALUES + ('1970-01-01', 17043), ('1999-09-09', 245), ('1899-12-30', 17), ('1111-01-01', 1);--> statement-breakpoint +-- Setup: list the Subject pairs to merge (decision 6) +CREATE TEMP TABLE __fix_subject_merge ( + source_id INTEGER PRIMARY KEY, + source_name TEXT NOT NULL, + target_id INTEGER NOT NULL, + target_name TEXT NOT NULL +);--> statement-breakpoint +-- Setup: Subject pairs to merge +INSERT INTO __fix_subject_merge (source_id, source_name, target_id, target_name) VALUES + (31047, '3d7_0.05', 66537, '3D7_0.05'), (31049, '3d7_10k', 66543, '3D7_10K');--> statement-breakpoint +-- Setup: drop pairs whose source Subject is gone +-- A pair is merged only while its source Subject still exists. +DELETE FROM __fix_subject_merge WHERE source_id NOT IN (SELECT id FROM study_subject);--> statement-breakpoint +-- Guard: table the guard triggers fire on +CREATE TEMP TABLE IF NOT EXISTS __fix_guard (id INTEGER PRIMARY KEY);--> statement-breakpoint +-- Guard: placeholder date counts (decision 1) +CREATE TRIGGER __fix_dates_tr BEFORE INSERT ON __fix_guard +BEGIN + SELECT RAISE(ABORT, 'Cannot clear placeholder collection dates: more Specimens are dated 1970-01-01, 1999-09-09, 1899-12-30 or 1111-01-01 than the lab reviewed (17043, 245, 17 and 1). Review the newer ones first: SELECT id, collection_date, created FROM specimen WHERE collection_date IN (''1970-01-01'', ''1999-09-09'', ''1899-12-30'', ''1111-01-01'') ORDER BY created DESC.') + WHERE EXISTS ( + SELECT 1 FROM specimen s JOIN __fix_placeholder_dates p ON p.collection_date = s.collection_date + GROUP BY p.collection_date HAVING COUNT(*) > MAX(p.reviewed) + ); +END;--> statement-breakpoint +-- Guard: exhausted Containers (decision 3) +CREATE TRIGGER __fix_exhausted_tr BEFORE INSERT ON __fix_guard +BEGIN + SELECT RAISE(ABORT, 'Cannot set exhausted Containers to remaining quantity 0: a Container commented exhausted has remaining quantity above 0 and is not one of the 46 the lab reviewed, or its remaining quantity is not 1. Review it first: SELECT id, comment, remaining_quantity FROM storage_container WHERE comment LIKE ''%exhausted%'' AND remaining_quantity > 0.') + WHERE EXISTS ( + SELECT 1 FROM storage_container + WHERE comment LIKE '%exhausted%' AND remaining_quantity > 0 + AND (id NOT IN (SELECT id FROM __fix_exhausted) OR remaining_quantity <> 1) + ); +END;--> statement-breakpoint +-- Guard: placeholder barcodes (decision 4) +CREATE TRIGGER __fix_barcodes_tr BEFORE INSERT ON __fix_guard +BEGIN + SELECT RAISE(ABORT, 'Cannot empty placeholder barcodes: a micronix tube has an NA-style placeholder barcode (micronix tubes must have one), or more cryovial tubes have one than the 285 the lab reviewed. Review them first.') + WHERE EXISTS ( + SELECT 1 FROM micronix_tube WHERE upper(replace(replace(trim(barcode), '/', ''), '.', '')) = 'NA' + ) OR ( + SELECT COUNT(*) FROM cryovial_tube WHERE upper(replace(replace(trim(barcode), '/', ''), '.', '')) = 'NA' + ) > 285; +END;--> statement-breakpoint +-- Guard: Container 34513 (decision 5) +CREATE TRIGGER __fix_container_tr BEFORE INSERT ON __fix_guard +BEGIN + SELECT RAISE(ABORT, 'Cannot delete Container 34513: it no longer matches what the lab reviewed (Specimen 31863, no tube, Paper or well row, no derivation, Tag or qPCR well, and its Specimen has another Container). Review it first.') + WHERE EXISTS (SELECT 1 FROM storage_container WHERE id = 34513) + AND NOT ( + (SELECT specimen_id FROM storage_container WHERE id = 34513) = 31863 + AND NOT EXISTS (SELECT 1 FROM micronix_tube WHERE id = 34513) + AND NOT EXISTS (SELECT 1 FROM cryovial_tube WHERE id = 34513) + AND NOT EXISTS (SELECT 1 FROM paper WHERE id = 34513) + AND NOT EXISTS (SELECT 1 FROM static_well WHERE id = 34513) + AND NOT EXISTS (SELECT 1 FROM container_derivation WHERE parent_container_id = 34513 OR child_container_id = 34513) + AND NOT EXISTS (SELECT 1 FROM storage_container_tag WHERE storage_container_id = 34513) + AND NOT EXISTS (SELECT 1 FROM qpcr_experiment_well WHERE storage_container_id = 34513) + AND EXISTS (SELECT 1 FROM storage_container WHERE specimen_id = 31863 AND id <> 34513) + ); +END;--> statement-breakpoint +-- Guard: plate 1760 (decision 8) +CREATE TRIGGER __fix_plate_tr BEFORE INSERT ON __fix_guard +BEGIN + SELECT RAISE(ABORT, 'Cannot delete plate 1760: it no longer matches what the lab reviewed (named "EXVIVO-27 " with a trailing space, holding no tubes or wells, beside plate 1761 "EXVIVO-27"). Review it first.') + WHERE EXISTS (SELECT 1 FROM micronix_plate WHERE id = 1760) + AND NOT ( + (SELECT name FROM micronix_plate WHERE id = 1760) = 'EXVIVO-27 ' + AND NOT EXISTS (SELECT 1 FROM micronix_tube WHERE collection_id = 1760) + AND NOT EXISTS (SELECT 1 FROM static_well WHERE collection_id = 1760) + AND EXISTS (SELECT 1 FROM micronix_plate WHERE id = 1761 AND name = 'EXVIVO-27') + ); +END;--> statement-breakpoint +-- Guard: Subject pairs to merge (decision 6) +CREATE TRIGGER __fix_merge_tr BEFORE INSERT ON __fix_guard +BEGIN + SELECT RAISE(ABORT, 'Cannot merge the CTRL Subjects 3d7_0.05 into 3D7_0.05 and 3d7_10k into 3D7_10K: one of them no longer has the name or Study the lab reviewed, or its target Subject is missing. Review them first.') + WHERE EXISTS ( + SELECT 1 FROM __fix_subject_merge m + JOIN study_subject src ON src.id = m.source_id + LEFT JOIN study_subject tgt ON tgt.id = m.target_id + WHERE src.name IS NOT m.source_name + OR tgt.name IS NOT m.target_name + OR tgt.study_id IS NOT src.study_id + OR src.study_id IS NOT (SELECT id FROM study WHERE short_code = 'CTRL') + ); +END;--> statement-breakpoint +-- Guard: no other case-variant Subjects (decision 6) +CREATE TRIGGER __fix_subject_case_tr BEFORE INSERT ON __fix_guard +BEGIN + SELECT RAISE(ABORT, 'Cannot merge the CTRL case-variant Subjects: a Study has Subjects whose names differ only by case beyond the two CTRL pairs the lab reviewed. Review them first. SELECT study_id, group_concat(id || '':'' || name, '', '') FROM study_subject GROUP BY study_id, name COLLATE NOCASE HAVING COUNT(*) > 1 lists them.') + WHERE EXISTS ( + SELECT 1 FROM study_subject + WHERE id NOT IN (SELECT source_id FROM __fix_subject_merge) + GROUP BY study_id, name COLLATE NOCASE HAVING COUNT(*) > 1 + ); +END;--> statement-breakpoint +-- Guard: specimen type 47 is Polyclonal Ab (decision 9) +CREATE TRIGGER __fix_polyclonal_tr BEFORE INSERT ON __fix_guard +BEGIN + SELECT RAISE(ABORT, 'Cannot allow micronix tubes for Polyclonal Ab: specimen type 47 is no longer named Polyclonal Ab. Review it first: SELECT id, name FROM specimen_type WHERE id = 47.') + WHERE EXISTS (SELECT 1 FROM specimen_type WHERE id = 47 AND name IS NOT 'Polyclonal Ab'); +END;--> statement-breakpoint +-- Guard: run every check; aborts the fix on unreviewed rows +INSERT INTO __fix_guard DEFAULT VALUES;--> statement-breakpoint +DROP TRIGGER __fix_dates_tr;--> statement-breakpoint +DROP TRIGGER __fix_exhausted_tr;--> statement-breakpoint +DROP TRIGGER __fix_barcodes_tr;--> statement-breakpoint +DROP TRIGGER __fix_container_tr;--> statement-breakpoint +DROP TRIGGER __fix_plate_tr;--> statement-breakpoint +DROP TRIGGER __fix_merge_tr;--> statement-breakpoint +DROP TRIGGER __fix_subject_case_tr;--> statement-breakpoint +DROP TRIGGER __fix_polyclonal_tr;--> statement-breakpoint +DROP TABLE __fix_guard;--> statement-breakpoint +-- Decision 1: clear placeholder collection dates +UPDATE specimen SET collection_date = NULL +WHERE collection_date IN (SELECT collection_date FROM __fix_placeholder_dates);--> statement-breakpoint +-- Decision 3: exhausted Containers get remaining quantity 0 +UPDATE storage_container SET remaining_quantity = 0 +WHERE id IN (SELECT id FROM __fix_exhausted) AND comment LIKE '%exhausted%' AND remaining_quantity = 1;--> statement-breakpoint +-- Decision 4: empty placeholder cryovial barcodes +UPDATE cryovial_tube SET barcode = NULL +WHERE upper(replace(replace(trim(barcode), '/', ''), '.', '')) = 'NA';--> statement-breakpoint +-- Decision 5: delete Container 34513 +DELETE FROM storage_container WHERE id = 34513;--> statement-breakpoint +-- Decision 8: delete empty plate 1760 +DELETE FROM micronix_plate WHERE id = 1760;--> statement-breakpoint +-- Decision 9: Polyclonal Ab allows micronix tubes +INSERT OR IGNORE INTO specimen_type_container_type (specimen_type_id, container_type) +SELECT id, 'micronix_tube' FROM specimen_type WHERE id = 47 AND name = 'Polyclonal Ab';--> statement-breakpoint +-- Decision 6: pair each source Specimen with its match +-- As mergeSubjects in src/lib/subjects/merge.ts does it, each source Specimen in turn +-- (lowest id first) is merged into the first target Specimen with the same Specimen type and +-- collection date, moving its Containers there; a source Specimen with no such match moves to +-- the target Subject, where a later source Specimen can match it. Runs after decision 1, so it +-- compares the cleaned dates. +CREATE TEMP TABLE __fix_specimen_merge AS +SELECT s.id AS specimen_id, m.target_id, + COALESCE( + (SELECT MIN(t.id) FROM specimen t + WHERE t.study_subject_id = m.target_id AND t.specimen_type_id = s.specimen_type_id AND t.collection_date IS s.collection_date), + (SELECT MIN(e.id) FROM specimen e + WHERE e.study_subject_id = m.source_id AND e.id < s.id AND e.specimen_type_id = s.specimen_type_id AND e.collection_date IS s.collection_date) + ) AS match_id +FROM specimen s +JOIN __fix_subject_merge m ON m.source_id = s.study_subject_id;--> statement-breakpoint +-- Decision 6: move Containers to the matching target Specimen +UPDATE storage_container +SET specimen_id = (SELECT match_id FROM __fix_specimen_merge WHERE specimen_id = storage_container.specimen_id), + last_updated = datetime('now') +WHERE specimen_id IN (SELECT specimen_id FROM __fix_specimen_merge WHERE match_id IS NOT NULL);--> statement-breakpoint +-- Decision 6: move qPCR wells to the matching target Specimen +UPDATE qpcr_experiment_well +SET specimen_id = (SELECT match_id FROM __fix_specimen_merge WHERE specimen_id = qpcr_experiment_well.specimen_id) +WHERE specimen_id IN (SELECT specimen_id FROM __fix_specimen_merge WHERE match_id IS NOT NULL);--> statement-breakpoint +-- Decision 6: delete the merged source Specimens +DELETE FROM specimen +WHERE id IN (SELECT specimen_id FROM __fix_specimen_merge WHERE match_id IS NOT NULL);--> statement-breakpoint +-- Decision 6: move unmatched source Specimens to the target Subject +UPDATE specimen +SET study_subject_id = (SELECT target_id FROM __fix_specimen_merge WHERE specimen_id = specimen.id), + last_updated = datetime('now') +WHERE id IN (SELECT specimen_id FROM __fix_specimen_merge WHERE match_id IS NULL);--> statement-breakpoint +-- Decision 6: mark the target Subjects updated +UPDATE study_subject SET last_updated = datetime('now') +WHERE id IN (SELECT target_id FROM __fix_subject_merge);--> statement-breakpoint +-- Decision 6: delete the merged source Subjects +DELETE FROM study_subject WHERE id IN (SELECT source_id FROM __fix_subject_merge);--> statement-breakpoint +DROP TABLE __fix_specimen_merge;--> statement-breakpoint +DROP TABLE __fix_subject_merge;--> statement-breakpoint +DROP TABLE __fix_placeholder_dates;--> statement-breakpoint +DROP TABLE __fix_exhausted; diff --git a/packages/api/initial_schema.sql b/packages/api/initial_schema.sql index ce2b56e2..ea80d50a 100644 --- a/packages/api/initial_schema.sql +++ b/packages/api/initial_schema.sql @@ -13,9 +13,12 @@ CREATE TABLE IF NOT EXISTS users ( );--> statement-breakpoint CREATE TABLE IF NOT EXISTS sessions ( id TEXT PRIMARY KEY, + token_hash TEXT NOT NULL, user_id INTEGER NOT NULL REFERENCES users(id), expires_at INTEGER NOT NULL );--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS sessions_token_hash_idx ON sessions(token_hash);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS sessions_user_id_idx ON sessions(user_id);--> statement-breakpoint CREATE TABLE IF NOT EXISTS tag ( id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL UNIQUE @@ -172,6 +175,7 @@ CREATE TABLE IF NOT EXISTS storage_container_tag ( tag_id INTEGER NOT NULL REFERENCES tag(id), PRIMARY KEY (storage_container_id, tag_id) );--> statement-breakpoint +CREATE INDEX IF NOT EXISTS storage_container_tag_tag_id_idx ON storage_container_tag(tag_id);--> statement-breakpoint CREATE TABLE IF NOT EXISTS location ( id INTEGER PRIMARY KEY, parent_id INTEGER REFERENCES location(id), @@ -208,6 +212,8 @@ CREATE TABLE IF NOT EXISTS micronix_tube ( position TEXT );--> statement-breakpoint CREATE UNIQUE INDEX IF NOT EXISTS micronix_tube_collection_position_idx ON micronix_tube(collection_id, position) WHERE position IS NOT NULL;--> statement-breakpoint +CREATE INDEX IF NOT EXISTS micronix_tube_collection_id_idx ON micronix_tube(collection_id);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS micronix_tube_barcode_folded_idx ON micronix_tube(lower(trim(barcode)));--> statement-breakpoint CREATE TABLE IF NOT EXISTS cryovial_box ( id INTEGER PRIMARY KEY, location_id INTEGER NOT NULL REFERENCES location(id), @@ -226,6 +232,9 @@ CREATE TABLE IF NOT EXISTS cryovial_tube ( position TEXT );--> statement-breakpoint CREATE UNIQUE INDEX IF NOT EXISTS cryovial_tube_collection_position_idx ON cryovial_tube(collection_id, position) WHERE position IS NOT NULL;--> statement-breakpoint +CREATE INDEX IF NOT EXISTS cryovial_tube_collection_id_idx ON cryovial_tube(collection_id);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS cryovial_tube_barcode_idx ON cryovial_tube(barcode);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS cryovial_tube_barcode_folded_idx ON cryovial_tube(lower(trim(barcode)));--> statement-breakpoint CREATE TABLE IF NOT EXISTS box ( id INTEGER PRIMARY KEY, location_id INTEGER NOT NULL REFERENCES location(id), @@ -260,17 +269,21 @@ CREATE TABLE IF NOT EXISTS sheet ( (box_id IS NULL AND bag_id IS NULL)) );--> statement-breakpoint CREATE UNIQUE INDEX IF NOT EXISTS sheet_box_name_idx ON sheet(box_id, name) WHERE box_id IS NOT NULL;--> statement-breakpoint +CREATE INDEX IF NOT EXISTS sheet_bag_id_idx ON sheet(bag_id);--> statement-breakpoint CREATE TABLE IF NOT EXISTS paper ( id INTEGER PRIMARY KEY REFERENCES storage_container(id), sheet_id INTEGER NOT NULL REFERENCES sheet(id), sublabel TEXT );--> statement-breakpoint +CREATE INDEX IF NOT EXISTS paper_sheet_id_sublabel_idx ON paper(sheet_id, sublabel);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS paper_sublabel_idx ON paper(sublabel);--> statement-breakpoint CREATE TABLE IF NOT EXISTS static_well ( id INTEGER PRIMARY KEY REFERENCES storage_container(id), collection_id INTEGER NOT NULL REFERENCES micronix_plate(id), position TEXT );--> statement-breakpoint CREATE UNIQUE INDEX IF NOT EXISTS static_well_collection_position_idx ON static_well(collection_id, position) WHERE position IS NOT NULL;--> statement-breakpoint +CREATE INDEX IF NOT EXISTS static_well_collection_id_idx ON static_well(collection_id);--> statement-breakpoint CREATE TABLE IF NOT EXISTS strain ( id INTEGER PRIMARY KEY, name TEXT NOT NULL UNIQUE, @@ -284,7 +297,7 @@ CREATE TABLE IF NOT EXISTS storage_type ( CREATE TABLE IF NOT EXISTS schema_version ( version INTEGER NOT NULL );--> statement-breakpoint -INSERT INTO schema_version (version) VALUES (7);--> statement-breakpoint +INSERT INTO schema_version (version) VALUES (12);--> statement-breakpoint CREATE TABLE IF NOT EXISTS settings ( key TEXT NOT NULL, user_id INTEGER REFERENCES users(id), diff --git a/packages/api/package.json b/packages/api/package.json index 5ae1e026..8a9551d4 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -13,7 +13,8 @@ "test:watch": "NODE_ENV=test bun test --watch src eslint-rules", "test:ui": "vitest --ui", "test:coverage": "NODE_ENV=test bun test src eslint-rules --coverage", - "demo:seed": "bun src/lib/demo-seed.ts" + "demo:seed": "bun src/lib/demo-seed.ts", + "data-fix": "bun src/scripts/data-fix.ts" }, "dependencies": { "@sampledb/contract": "workspace:*", diff --git a/packages/api/src/__tests__/helpers/auth-helpers.ts b/packages/api/src/__tests__/helpers/auth-helpers.ts index f46ffd54..99e3c549 100644 --- a/packages/api/src/__tests__/helpers/auth-helpers.ts +++ b/packages/api/src/__tests__/helpers/auth-helpers.ts @@ -1,7 +1,7 @@ import type { Database } from '../../db/client' import { users, sessions, settings } from '../../db/schema' import bcrypt from 'bcryptjs' -import { BCRYPT_COST } from '../../lib/auth/auth-service' +import { BCRYPT_COST, hashSessionToken } from '../../lib/auth/auth-service' import { nanoid } from 'nanoid' import type { Hono } from 'hono' import { testClient } from 'hono/testing' @@ -150,11 +150,14 @@ export async function createAuthenticatedSession( userId: number, maxAgeSeconds: number = 604800 ) { - const sessionId = nanoid() + // sessionId is the cookie token; the row stores only its hash, under a separate public id. + const sessionId = nanoid(43) + const publicId = nanoid() const expiresAt = Math.floor(Date.now() / 1000) + maxAgeSeconds await db.insert(sessions).values({ - id: sessionId, + id: publicId, + tokenHash: hashSessionToken(sessionId), userId, expiresAt, }) @@ -162,6 +165,7 @@ export async function createAuthenticatedSession( // Return session cookie in format expected by Hono test client return { sessionId, + publicId, cookie: `session_id=${sessionId}`, cookieHeader: `session_id=${sessionId}; Path=/; HttpOnly; SameSite=Lax`, } diff --git a/packages/api/src/__tests__/helpers/factories.ts b/packages/api/src/__tests__/helpers/factories.ts index b39f0915..28e0ce2d 100644 --- a/packages/api/src/__tests__/helpers/factories.ts +++ b/packages/api/src/__tests__/helpers/factories.ts @@ -354,13 +354,16 @@ export async function createTestControlDefinition(db: Database, data?: { /** * Create a test study */ -export async function createTestStudy(db: Database, data: { title: string, shortCode: string, leadPerson?: string }) { +export async function createTestStudy( + db: Database, + data: { title: string, shortCode: string, leadPerson?: string, description?: string, created?: string }, +) { const now = utcNow() const [result] = await db.insert(study).values({ ...data, isLongitudinal: false, leadPerson: data.leadPerson ?? 'Test Person', - created: now, + created: data.created ?? now, lastUpdated: now, }).returning() return result @@ -369,16 +372,21 @@ export async function createTestStudy(db: Database, data: { title: string, short /** * Create a test study subject */ -export async function createTestStudySubject(db: Database, data: { studyId: number, name: string }) { +export async function createTestStudySubject( + db: Database, + data: { studyId: number, name: string, created?: string }, +) { const now = utcNow() const [result] = await db.insert(studySubject).values({ ...data, - created: now, + created: data.created ?? now, lastUpdated: now, }).returning() return result } +let defaultSpecimenStudyCount = 0 + /** * Create a test specimen (for testing specimen type "in use") */ @@ -392,9 +400,12 @@ export async function createTestSpecimen( // Default to study subject if neither provided if (!studySubjectId && !controlBatchId) { + // A counter, not Date.now() plus a random number: tests create dozens of these within one + // millisecond, and random suffixes then collide on study's unique title and short_code. + defaultSpecimenStudyCount += 1 const testStudy = await createTestStudy(db, { - title: `Study ${Date.now()}-${Math.floor(Math.random() * 1000)}`, - shortCode: `ST${Date.now()}-${Math.floor(Math.random() * 1000)}` + title: `Study ${Date.now()}-${defaultSpecimenStudyCount}`, + shortCode: `ST${Date.now()}-${defaultSpecimenStudyCount}` }); const testSubject = await createTestStudySubject(db, { studyId: testStudy.id, diff --git a/packages/api/src/db/README.md b/packages/api/src/db/README.md index b40792b2..53d03d14 100644 --- a/packages/api/src/db/README.md +++ b/packages/api/src/db/README.md @@ -14,6 +14,28 @@ See [ADR 0003: SQLite schema evolution](../../../docs/adr/0003-sqlite-schema-evo 6. Refresh `initial_schema.sql` so **new** empty databases match the latest shape (merge generated SQL; keep `INSERT INTO schema_version` at `CURRENT_SCHEMA_VERSION`). `__tests__/initial-schema-snapshot.test.ts` fails until the snapshot matches a database built by the migrations: columns, types, NOT NULL, defaults, indexes, and foreign keys. 7. Extend the legacy fixtures if the migration touches a table they don't create. Older-version tests in `__tests__/schema-evolution.test.ts` build minimal databases through `createTablesForLaterMigrations`, and `__tests__/client.test.ts` creates its own tables inline; both must contain every table and column a later migration alters. +## One-time data fixes + +A migration ships to every install and runs at startup, so it must be correct on any database: it may not name rows by id or carry out one lab's decisions about its own data, and it must not abort on data another install could legitimately hold. A change to one install's data is a **data fix** (CONTEXT.md) instead: a SQL file an operator runs by hand. + +```bash +bun run data-fix [--db=path/to/database.sqlite] [--dry-run] +``` + +`src/scripts/data-fix.ts` is the CLI; `runDataFix` in `data-fix.ts` does the work. It opens the file directly, not through `openOperationalDatabase`, because a data fix must not migrate the database, and it: + +1. refuses a database whose `schema_version` is not `CURRENT_SCHEMA_VERSION`; +2. does nothing, and says so, if `data_fix_log` already records the fix's name with the same checksum, and refuses if the checksum differs; +3. copies the database with `VACUUM INTO` (as `migration-backup.ts` does before migrations) to `.pre-data-fix--.bak`; +4. runs the statements (split on `--> statement-breakpoint`) in one `BEGIN IMMEDIATE` transaction, with foreign keys on, and records `(name, checksum, applied_at)` in `data_fix_log` in the same transaction; +5. prints the rows each statement changed (`total_changes()`, so trigger and foreign-key changes count), labelled by the `--` comment just above it. + +`--dry-run` does steps 1, 2 and 4 in a transaction it rolls back, and writes no copy and no record. + +`data_fix_log` belongs to the runner, not the operational schema: it is created on the first fix and is absent from `schema.ts`, `initial_schema.sql`, and databases no fix ran on. + +**Fix files are site-specific.** `ops/data-fixes/` holds the EPPIcenter lab's fixes for its own database; other installs must not run them. Start each file with a header naming the database it is for, the decision it carries out, and a warning not to run it elsewhere. Guard it: check that the rows it changes are the ones that were reviewed, and abort (for example with a temp trigger that `RAISE(ABORT, …)`s, as `2026-11-01-eppicenter-legacy-cleanup.sql` does) when they are not. A test under `__tests__/data-fixes/` runs the lab's fix against a database seeded in production's shape. The operator steps are in the deployment guide in `packages/docs` (One-time data fixes). + ## How evolution works | Database state | Behaviour | @@ -32,7 +54,7 @@ Migration `001` is **only** for legacy files; fresh installs from the snapshot s ## Opening a database -Use `openOperationalDatabase(optionalPath?)` from the client module. It resolves the file path, enables WAL, turns on foreign keys, runs schema evolution, and returns Drizzle + raw `bun:sqlite` handles. Importing the client module does **not** open a connection. +Use `openOperationalDatabase(optionalPath?)` from the client module. It resolves the file path, enables WAL, turns on foreign keys, runs schema evolution (backing up the file first when a migration is pending, see Failures), and returns Drizzle + raw `bun:sqlite` handles. Importing the client module does **not** open a connection. The rules for transactions, DDL, and which entry point to open a database with are under Database in `CODING_STANDARDS.md`. @@ -66,4 +88,21 @@ When you add a multi-statement write, use `withWriteTransaction` instead of addi ## Failures -Each migration runs in a transaction. On failure, the transaction rolls back, `schema_version` is unchanged, and startup throws `SchemaMigrationError` with the migration number. Restore from backup or fix the database manually; there are no down-migrations. +Each migration runs in a transaction. On failure, the transaction rolls back, `schema_version` is unchanged, and startup throws `SchemaMigrationError` with the migration number. There are no down-migrations, so a migration that succeeds but leaves bad data can only be undone from a copy. + +### Backup before migrating + +Before the first migration changes an existing file (a numbered migration, or the legacy baseline `001`), `openOperationalDatabase` writes a copy next to it with `VACUUM INTO`, which includes pages still in the WAL: + +``` +.pre-v-.bak +e.g. sampledb.sqlite.pre-v11-20261101T143005123Z.bak +``` + +The version in the name is the one the migrations move to; the copy itself holds the version it had before. `migration-backup.ts` writes it, and `evolveOperationalSchema` calls it when passed `{ backupBeforeMigrating: true }`. + +- No copy when nothing is pending, when the file was empty (the snapshot path), or for `:memory:`. +- `openOperationalDatabase` skips the copy under `NODE_ENV=test`; pass `{ backupBeforeMigrating: true }` to force it. +- If the copy can't be written (read-only directory, full disk), startup throws `MigrationBackupError` before any migration runs, and the database is unchanged. + +Copies are never deleted automatically. The deployment guide in `packages/docs` says when to remove them. diff --git a/packages/api/src/db/__tests__/client.test.ts b/packages/api/src/db/__tests__/client.test.ts index c08ee650..3ae770fa 100644 --- a/packages/api/src/db/__tests__/client.test.ts +++ b/packages/api/src/db/__tests__/client.test.ts @@ -27,13 +27,22 @@ describe('Database Client', () => { it('creates error_logs table when missing even if study and settings exist', () => { // Simulate database created before error_logs was added to schema const db = new Database(testDbPath) - db.exec('CREATE TABLE study (id INTEGER PRIMARY KEY)') + db.exec('CREATE TABLE study (id INTEGER PRIMARY KEY, title TEXT, short_code TEXT)') db.exec('CREATE TABLE settings (key TEXT, user_id INTEGER, value TEXT, PRIMARY KEY (key, user_id))') db.exec(`CREATE TABLE micronix_tube (id INTEGER PRIMARY KEY, collection_id INTEGER NOT NULL, barcode TEXT, position TEXT)`) db.exec(`CREATE TABLE cryovial_tube (id INTEGER PRIMARY KEY, collection_id INTEGER NOT NULL, barcode TEXT, position TEXT)`) db.exec(`CREATE TABLE static_well (id INTEGER PRIMARY KEY, collection_id INTEGER NOT NULL, position TEXT)`) db.exec(`CREATE TABLE study_subject (id INTEGER PRIMARY KEY, study_id INTEGER NOT NULL, name TEXT NOT NULL)`) db.exec(`CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT NOT NULL, box_id INTEGER, bag_id INTEGER)`) + db.exec(`CREATE TABLE specimen (id INTEGER PRIMARY KEY, study_subject_id INTEGER, control_batch_id INTEGER, specimen_type_id INTEGER NOT NULL, collection_date DATE)`) + // Tables migration 011 indexes + db.exec(`CREATE TABLE storage_container (id INTEGER PRIMARY KEY, specimen_id INTEGER)`) + db.exec(`CREATE TABLE storage_container_tag (storage_container_id INTEGER, tag_id INTEGER)`) + db.exec(`CREATE TABLE control_batch (id INTEGER PRIMARY KEY, control_definition_id INTEGER, properties TEXT)`) + db.exec(`CREATE TABLE location (id INTEGER PRIMARY KEY, parent_id INTEGER, path TEXT)`) + db.exec(`CREATE TABLE micronix_plate (id INTEGER PRIMARY KEY, location_id INTEGER)`) + db.exec(`CREATE TABLE cryovial_box (id INTEGER PRIMARY KEY, location_id INTEGER)`) + db.exec(`CREATE TABLE bag (id INTEGER PRIMARY KEY, location_id INTEGER)`) // Intentionally omit paper — migration 003 preflight creates legacy stub // Intentionally do NOT create error_logs const before = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name='error_logs'").get() diff --git a/packages/api/src/db/__tests__/data-fix.test.ts b/packages/api/src/db/__tests__/data-fix.test.ts new file mode 100644 index 00000000..e14ce2a5 --- /dev/null +++ b/packages/api/src/db/__tests__/data-fix.test.ts @@ -0,0 +1,127 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { Database } from 'bun:sqlite' +import { mkdtempSync, readdirSync, rmSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' +import { applyInitialSchema } from '../apply-initial-schema' +import { CURRENT_SCHEMA_VERSION } from '../schema-evolution' +import { DataFixError, runDataFix, type DataFix } from '../data-fix' + +const NOW = '2024-01-01 00:00:00' + +const renameStudies: DataFix = { + name: '2026-11-01-rename-studies', + sql: [ + '-- Header comment', + "-- Rename the pilot study\nUPDATE study SET title = 'Pilot (renamed)' WHERE short_code = 'PIL'", + "UPDATE study SET lead_person = 'Lab' WHERE short_code IN ('PIL', 'MAIN')", + "CREATE TEMP TABLE __fix_scratch (id INTEGER);\n", + 'DROP TABLE __fix_scratch', + ].join(';--> statement-breakpoint\n'), +} + +function titles(sqlite: Database): string[] { + return (sqlite.prepare('SELECT title FROM study ORDER BY id').all() as Array<{ title: string }>).map((r) => r.title) +} + +describe('runDataFix', () => { + let dir: string + let sqlite: Database + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'data-fix-')) + sqlite = new Database(join(dir, 'lab.sqlite')) + sqlite.exec('PRAGMA foreign_keys = ON') + applyInitialSchema(sqlite) + sqlite.exec(`INSERT INTO study (id, title, short_code, is_longitudinal, lead_person, created, last_updated) VALUES + (1, 'Pilot', 'PIL', 0, 'A', '${NOW}', '${NOW}'), (2, 'Main', 'MAIN', 0, 'B', '${NOW}', '${NOW}')`) + }) + + afterEach(() => { + sqlite.close() + rmSync(dir, { recursive: true, force: true }) + }) + + it('applies the fix, reports the rows each statement changed, and records it', () => { + const result = runDataFix(sqlite, renameStudies, { now: new Date('2026-11-01T12:00:00Z') }) + + expect(result.status).toBe('applied') + expect(result.statements).toEqual([ + { label: 'Rename the pilot study', changes: 1 }, + { label: "UPDATE study SET lead_person = 'Lab' WHERE short_code IN ('PIL', 'MAIN')", changes: 2 }, + { label: 'CREATE TEMP TABLE __fix_scratch (id INTEGER)', changes: 0 }, + { label: 'DROP TABLE __fix_scratch', changes: 0 }, + ]) + expect(titles(sqlite)).toEqual(['Pilot (renamed)', 'Main']) + expect(sqlite.prepare('SELECT name, applied_at, checksum FROM data_fix_log').all()).toEqual([ + { name: '2026-11-01-rename-studies', applied_at: '2026-11-01T12:00:00.000Z', checksum: expect.stringMatching(/^[0-9a-f]{64}$/) }, + ]) + }) + + it('does nothing the second time and says when it ran', () => { + runDataFix(sqlite, renameStudies, { now: new Date('2026-11-01T12:00:00Z') }) + sqlite.exec(`UPDATE study SET title = 'Pilot' WHERE id = 1`) + + const again = runDataFix(sqlite, renameStudies) + + expect(again).toEqual({ status: 'already-applied', name: renameStudies.name, appliedAt: '2026-11-01T12:00:00.000Z' }) + expect(titles(sqlite)).toEqual(['Pilot', 'Main']) + }) + + it('refuses a fix whose file changed since it ran under the same name', () => { + runDataFix(sqlite, renameStudies) + + expect(() => runDataFix(sqlite, { ...renameStudies, sql: `${renameStudies.sql};\nSELECT 1` })).toThrow( + /2026-11-01-rename-studies already ran .* with different contents/, + ) + }) + + it('copies the database before changing it, naming the copy for the fix', () => { + const result = runDataFix(sqlite, renameStudies, { now: new Date('2026-11-01T12:00:00Z') }) + + expect(result.status === 'applied' && result.backupPath).toBe( + join(dir, 'lab.sqlite.pre-data-fix-2026-11-01-rename-studies-20261101T120000000Z.bak'), + ) + const copy = new Database(join(dir, 'lab.sqlite.pre-data-fix-2026-11-01-rename-studies-20261101T120000000Z.bak'), { readonly: true }) + expect(titles(copy)).toEqual(['Pilot', 'Main']) + copy.close() + }) + + it('in a dry run reports the counts, rolls everything back and records nothing', () => { + const result = runDataFix(sqlite, renameStudies, { dryRun: true }) + + expect(result.status).toBe('dry-run') + expect(result.status === 'dry-run' && result.statements.map((s) => s.changes)).toEqual([1, 2, 0, 0]) + expect(titles(sqlite)).toEqual(['Pilot', 'Main']) + expect(sqlite.prepare(`SELECT name FROM sqlite_master WHERE name = 'data_fix_log'`).all()).toEqual([]) + expect(readdirSync(dir).filter((f) => f.endsWith('.bak'))).toEqual([]) + }) + + it('in a dry run of a fix that already ran says so', () => { + runDataFix(sqlite, renameStudies, { now: new Date('2026-11-01T12:00:00Z') }) + + expect(runDataFix(sqlite, renameStudies, { dryRun: true }).status).toBe('already-applied') + }) + + it('refuses a database that is not at the current schema version, changing nothing', () => { + sqlite.exec(`UPDATE schema_version SET version = ${CURRENT_SCHEMA_VERSION - 1}`) + + expect(() => runDataFix(sqlite, renameStudies)).toThrow( + new RegExp(`schema version ${CURRENT_SCHEMA_VERSION - 1}.*needs ${CURRENT_SCHEMA_VERSION}`), + ) + expect(titles(sqlite)).toEqual(['Pilot', 'Main']) + expect(readdirSync(dir).filter((f) => f.endsWith('.bak'))).toEqual([]) + }) + + it('rolls back every statement when one fails, names it, and records nothing', () => { + const failing: DataFix = { + name: 'failing', + sql: `UPDATE study SET title = title || ' (changed)';--> statement-breakpoint\nUPDATE study SET short_code = 'MAIN' WHERE id = 1`, + } + + expect(() => runDataFix(sqlite, failing)).toThrow(DataFixError) + expect(() => runDataFix(sqlite, failing)).toThrow(/statement 2 .*UNIQUE constraint failed/) + expect(titles(sqlite)).toEqual(['Pilot', 'Main']) + expect(sqlite.prepare(`SELECT name FROM sqlite_master WHERE name = 'data_fix_log'`).all()).toEqual([]) + }) +}) diff --git a/packages/api/src/db/__tests__/data-fixes/eppicenter-legacy-cleanup.test.ts b/packages/api/src/db/__tests__/data-fixes/eppicenter-legacy-cleanup.test.ts new file mode 100644 index 00000000..073b85a1 --- /dev/null +++ b/packages/api/src/db/__tests__/data-fixes/eppicenter-legacy-cleanup.test.ts @@ -0,0 +1,319 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { Database } from 'bun:sqlite' +import { resolve } from 'path' +import { applyInitialSchema } from '../../apply-initial-schema' +import { loadDataFix, runDataFix } from '../../data-fix' +import { getMonorepoRoot } from '../../open' + +/** + * The EPPIcenter lab's one-time legacy data cleanup (issues #124 and #126), a site-specific data + * fix in ops/data-fixes that only the lab runs. Each test builds a database in production's + * shape: the current snapshot, without the index production lacks, seeded with the production + * ids each decision names. + */ + +const FIX = loadDataFix(resolve(getMonorepoRoot(), 'ops/data-fixes/2026-11-01-eppicenter-legacy-cleanup.sql')) + +function applyFix(sqlite: Database): void { + runDataFix(sqlite, FIX) +} + +const NOW = '2024-01-01 00:00:00' + +function productionShapedDatabase(): Database { + const sqlite = new Database(':memory:') + sqlite.exec('PRAGMA foreign_keys=ON') + applyInitialSchema(sqlite) + // Production has no Specimen identity index (decision 7). + sqlite.exec('DROP INDEX IF EXISTS idx_specimen_study_subject_type_date') + sqlite.exec(`INSERT INTO study (id, title, short_code, is_longitudinal, lead_person, created, last_updated) VALUES + (3, 'Controls', 'CTRL', 0, 'Lab', '${NOW}', '${NOW}'), (8, 'Jiangsu', 'JIANGSU17', 0, 'Lab', '${NOW}', '${NOW}')`) + sqlite.exec(`INSERT INTO specimen_type (id, name, created, last_updated) VALUES (2, 'DBS', '${NOW}', '${NOW}'), (21, 'WB', '${NOW}', '${NOW}')`) + sqlite.exec(`INSERT INTO unit (id, symbol, name, category) VALUES (5, 'uL', 'microliter', 'volume')`) + sqlite.exec(`INSERT INTO storage_type (id, name) VALUES (1, 'Freezer')`) + sqlite.exec(`INSERT INTO location (id, name, storage_type_id) VALUES (107, 'Freezer A', 1)`) + sqlite.exec(`INSERT INTO cryovial_box (id, location_id, name, created, last_updated) VALUES (900, 107, 'Box 900', '${NOW}', '${NOW}')`) + return sqlite +} + +function addSubject(sqlite: Database, id: number, studyId: number, name: string): void { + sqlite.prepare(`INSERT INTO study_subject (id, study_id, name, created, last_updated) VALUES (?, ?, ?, ?, ?)`).run(id, studyId, name, NOW, NOW) +} + +function addSpecimen(sqlite: Database, id: number, subjectId: number, typeId: number, date: string | null): void { + sqlite + .prepare(`INSERT INTO specimen (id, study_subject_id, specimen_type_id, collection_date, created, last_updated) VALUES (?, ?, ?, ?, ?, ?)`) + .run(id, subjectId, typeId, date, NOW, NOW) +} + +function addContainer(sqlite: Database, id: number, specimenId: number, options: { comment?: string; remaining?: number } = {}): void { + sqlite + .prepare(`INSERT INTO storage_container (id, specimen_id, comment, total_quantity, remaining_quantity, unit_id, created, last_updated) VALUES (?, ?, ?, 1.0, ?, 5, ?, ?)`) + .run(id, specimenId, options.comment ?? null, options.remaining ?? 1.0, NOW, NOW) +} + +function addCryovialTube(sqlite: Database, id: number, barcode: string | null): void { + sqlite.prepare(`INSERT INTO cryovial_tube (id, collection_id, barcode) VALUES (?, 900, ?)`).run(id, barcode) +} + +function addMicronixTube(sqlite: Database, id: number, plateId: number, barcode: string): void { + sqlite.prepare(`INSERT INTO micronix_tube (id, collection_id, barcode) VALUES (?, ?, ?)`).run(id, plateId, barcode) +} + +/** Production's rows for every decision, plus neighbours each step must leave alone. */ +function seedProductionRows(sqlite: Database): void { + // Polyclonal Ab: no allowed Container types, though Specimen 141173's tubes are micronix. + sqlite.exec(`INSERT INTO specimen_type (id, name, created, last_updated) VALUES (47, 'Polyclonal Ab', '${NOW}', '${NOW}')`) + sqlite.exec(`INSERT INTO specimen_type_container_type (specimen_type_id, container_type) VALUES (2, 'paper')`) + addSubject(sqlite, 31047, 3, '3d7_0.05') + addSubject(sqlite, 66537, 3, '3D7_0.05') + addSubject(sqlite, 31049, 3, '3d7_10k') + addSubject(sqlite, 66543, 3, '3D7_10K') + addSubject(sqlite, 25613, 8, 'JS087') + addSubject(sqlite, 500, 8, 'S500') + + // Decision 6: Specimen 43644 has no match on the target and moves to it; Specimen 43645 + // matches target Specimen 120524 (same type and date) and is merged into it. + addSpecimen(sqlite, 43644, 31047, 21, '2018-03-15') + addSpecimen(sqlite, 43645, 31047, 2, '2021-06-07') + addSpecimen(sqlite, 120523, 66537, 2, '2021-06-08') + addSpecimen(sqlite, 120524, 66537, 2, '2021-06-07') + addSpecimen(sqlite, 43646, 31049, 21, '2018-03-15') + addSpecimen(sqlite, 120529, 66543, 2, '2021-06-07') + addContainer(sqlite, 70001, 43644) + addContainer(sqlite, 70002, 43645) + addContainer(sqlite, 70003, 43645) + addContainer(sqlite, 70004, 120524) + addContainer(sqlite, 70005, 43646) + + // Decision 1: placeholder dates, and dates that stay (including a future one, decision 2). + addSpecimen(sqlite, 1, 500, 2, '1970-01-01') + addSpecimen(sqlite, 2, 500, 2, '1999-09-09') + addSpecimen(sqlite, 3, 500, 2, '1899-12-30') + addSpecimen(sqlite, 4, 500, 2, '1111-01-01') + addSpecimen(sqlite, 5, 500, 2, '1970-01-02') + addSpecimen(sqlite, 6, 500, 2, '2073-02-08') + addSpecimen(sqlite, 7, 500, 21, '1970-01-01') + + // Decision 3: exhausted Containers with remaining 1, and ones that stay. + addSpecimen(sqlite, 10, 500, 21, '2020-01-01') + addContainer(sqlite, 50465, 10, { comment: 'exhausted' }) + addContainer(sqlite, 124552, 10, { comment: 'Z32592; exhausted' }) + addContainer(sqlite, 143335, 10, { comment: 'Z20759; Zymo cleaned; exhausted' }) + addContainer(sqlite, 74034, 10, { comment: 'Reuploaded & Set to Exhausted', remaining: 0 }) + addContainer(sqlite, 74035, 10, { comment: 'half used', remaining: 0.5 }) + + // Decision 4: placeholder tube barcodes, and real barcodes that contain NA. + for (const [id, barcode] of [ + [80001, 'NA'], + [80002, 'N/A'], + [80003, 'n/a'], + [80004, ' NA '], + [80005, 'NA98'], + [80006, 'DFNA'], + [80007, null], + ] as const) { + addContainer(sqlite, id, 10) + addCryovialTube(sqlite, id, barcode) + } + + // Decision 5: Container 34513 has no tube or Paper row; its Specimen keeps its other Container. + addSpecimen(sqlite, 31863, 25613, 21, '2018-07-24') + addContainer(sqlite, 34513, 31863) + addContainer(sqlite, 34514, 31863) + addCryovialTube(sqlite, 34514, 'JS087-1') + + // Decision 8: plate 1760 is empty; plate 1761 keeps its tubes. + sqlite.exec(`INSERT INTO micronix_plate (id, location_id, name, created, last_updated) VALUES + (1760, 107, 'EXVIVO-27 ', '${NOW}', '${NOW}'), (1761, 107, 'EXVIVO-27', '${NOW}', '${NOW}')`) + addContainer(sqlite, 90001, 10) + addMicronixTube(sqlite, 90001, 1761, '1000000001') +} + +function column(sqlite: Database, sql: string): T[] { + return (sqlite.prepare(sql).all() as Array>).map((row) => Object.values(row)[0] as T) +} + +function rows(sqlite: Database, sql: string): unknown[] { + return sqlite.prepare(sql).all() +} + +describe('data fix 2026-11-01-eppicenter-legacy-cleanup', () => { + let sqlite: Database + + beforeEach(() => { + sqlite = productionShapedDatabase() + }) + + afterEach(() => { + sqlite.close() + }) + + it('applies each decision to the rows it names and leaves every other row alone', () => { + seedProductionRows(sqlite) + + applyFix(sqlite) + + // Decision 1 + expect(rows(sqlite, 'SELECT id, collection_date FROM specimen WHERE id <= 7 ORDER BY id')).toEqual([ + { id: 1, collection_date: null }, + { id: 2, collection_date: null }, + { id: 3, collection_date: null }, + { id: 4, collection_date: null }, + { id: 5, collection_date: '1970-01-02' }, + { id: 6, collection_date: '2073-02-08' }, + { id: 7, collection_date: null }, + ]) + // Decision 3 + expect(rows(sqlite, 'SELECT id, comment, remaining_quantity FROM storage_container WHERE specimen_id = 10 AND comment IS NOT NULL ORDER BY id')).toEqual([ + { id: 50465, comment: 'exhausted', remaining_quantity: 0 }, + { id: 74034, comment: 'Reuploaded & Set to Exhausted', remaining_quantity: 0 }, + { id: 74035, comment: 'half used', remaining_quantity: 0.5 }, + { id: 124552, comment: 'Z32592; exhausted', remaining_quantity: 0 }, + { id: 143335, comment: 'Z20759; Zymo cleaned; exhausted', remaining_quantity: 0 }, + ]) + // Decision 4 + expect(rows(sqlite, 'SELECT id, barcode FROM cryovial_tube WHERE id BETWEEN 80001 AND 80007 ORDER BY id')).toEqual([ + { id: 80001, barcode: null }, + { id: 80002, barcode: null }, + { id: 80003, barcode: null }, + { id: 80004, barcode: null }, + { id: 80005, barcode: 'NA98' }, + { id: 80006, barcode: 'DFNA' }, + { id: 80007, barcode: null }, + ]) + // Decision 5 + expect(column(sqlite, 'SELECT id FROM storage_container WHERE specimen_id = 31863')).toEqual([34514]) + // Decision 8 + expect(column(sqlite, `SELECT id FROM micronix_plate WHERE trim(name) = 'EXVIVO-27'`)).toEqual([1761]) + expect(column(sqlite, 'SELECT COUNT(*) FROM micronix_tube WHERE collection_id = 1761')).toEqual([1]) + // Polyclonal Ab allows micronix tubes; other types' allowed types are unchanged. + expect(rows(sqlite, 'SELECT specimen_type_id, container_type FROM specimen_type_container_type ORDER BY specimen_type_id')).toEqual([ + { specimen_type_id: 2, container_type: 'paper' }, + { specimen_type_id: 47, container_type: 'micronix_tube' }, + ]) + // Decision 6: the app's Subject merge, run for both pairs. + expect(rows(sqlite, 'SELECT id, name FROM study_subject WHERE study_id = 3 ORDER BY id')).toEqual([ + { id: 66537, name: '3D7_0.05' }, + { id: 66543, name: '3D7_10K' }, + ]) + expect(rows(sqlite, 'SELECT id, study_subject_id FROM specimen WHERE id IN (43644, 43645, 43646, 120523, 120524, 120529) ORDER BY id')).toEqual([ + { id: 43644, study_subject_id: 66537 }, + { id: 43646, study_subject_id: 66543 }, + { id: 120523, study_subject_id: 66537 }, + { id: 120524, study_subject_id: 66537 }, + { id: 120529, study_subject_id: 66543 }, + ]) + expect(rows(sqlite, 'SELECT id, specimen_id FROM storage_container WHERE id BETWEEN 70001 AND 70005 ORDER BY id')).toEqual([ + { id: 70001, specimen_id: 43644 }, + { id: 70002, specimen_id: 120524 }, + { id: 70003, specimen_id: 120524 }, + { id: 70004, specimen_id: 120524 }, + { id: 70005, specimen_id: 43646 }, + ]) + expect(column(sqlite, 'SELECT last_updated FROM study_subject WHERE id IN (66537, 66543)').every((t) => t !== NOW)).toBe(true) + expect(column(sqlite, 'SELECT last_updated FROM specimen WHERE id = 43644')).not.toEqual([NOW]) + expect(column(sqlite, 'SELECT last_updated FROM storage_container WHERE id = 70002')).not.toEqual([NOW]) + expect(column(sqlite, `SELECT name FROM sqlite_temp_master WHERE name LIKE '\\_\\_fix\\_%' ESCAPE '\\'`)).toEqual([]) + expect(column(sqlite, 'PRAGMA foreign_key_check')).toEqual([]) + }) + + it('changes nothing on a database without the legacy rows', () => { + addSubject(sqlite, 1, 3, 'A') + addSpecimen(sqlite, 1, 1, 2, '2020-01-01') + addContainer(sqlite, 1, 1, { comment: 'exhausted', remaining: 0 }) + addCryovialTube(sqlite, 1, 'CV1') + + applyFix(sqlite) + + expect(rows(sqlite, 'SELECT id, study_subject_id, collection_date FROM specimen')).toEqual([{ id: 1, study_subject_id: 1, collection_date: '2020-01-01' }]) + expect(rows(sqlite, 'SELECT id, remaining_quantity FROM storage_container')).toEqual([{ id: 1, remaining_quantity: 0 }]) + expect(rows(sqlite, 'SELECT id, barcode FROM cryovial_tube')).toEqual([{ id: 1, barcode: 'CV1' }]) + expect(column(sqlite, 'SELECT COUNT(*) FROM study_subject')).toEqual([1]) + expect(column(sqlite, 'SELECT COUNT(*) FROM specimen_type_container_type')).toEqual([0]) + }) + + const aborts: Array<[string, (sqlite: Database) => void, RegExp]> = [ + [ + 'an exhausted Container the lab did not review still has quantity', + (db) => addContainer(db, 99999, 10, { comment: 'exhausted' }), + /exhausted/, + ], + [ + 'a reviewed exhausted Container has a remaining quantity other than 1', + (db) => db.exec('UPDATE storage_container SET remaining_quantity = 0.5 WHERE id = 50465'), + /exhausted/, + ], + [ + 'there are more Specimens on a placeholder date than the lab reviewed', + (db) => addSpecimen(db, 99999, 500, 21, '1111-01-01'), + /placeholder collection date/, + ], + [ + 'a micronix tube has a placeholder barcode, which cannot be emptied', + (db) => { + addContainer(db, 99999, 10) + addMicronixTube(db, 99999, 1761, 'N/A') + }, + /placeholder barcode/, + ], + [ + 'Container 34513 has a tube row', + (db) => addCryovialTube(db, 34513, 'X34513'), + /Container 34513/, + ], + [ + 'Container 34513 is the only Container of its Specimen', + (db) => db.exec('DELETE FROM cryovial_tube WHERE id = 34514; DELETE FROM storage_container WHERE id = 34514'), + /Container 34513/, + ], + [ + 'plate 1760 holds a tube', + (db) => { + addContainer(db, 99999, 10) + addMicronixTube(db, 99999, 1760, '1000000002') + }, + /plate 1760/, + ], + [ + 'plate 1760 has been renamed', + (db) => db.exec(`UPDATE micronix_plate SET name = 'EXVIVO-28' WHERE id = 1760`), + /plate 1760/, + ], + [ + 'a Subject to merge has been renamed', + (db) => db.exec(`UPDATE study_subject SET name = '3d7_10k_b' WHERE id = 31049`), + /merge/, + ], + [ + 'a Subject to merge has lost its target', + (db) => db.exec(`UPDATE specimen SET study_subject_id = 31049 WHERE study_subject_id = 66543; DELETE FROM study_subject WHERE id = 66543`), + /merge/, + ], + [ + 'specimen type 47 is no longer Polyclonal Ab', + (db) => db.exec(`UPDATE specimen_type SET name = 'Monoclonal Ab' WHERE id = 47`), + /Polyclonal Ab/, + ], + [ + 'another Study has Subjects whose names differ only by case', + (db) => { + addSubject(db, 600, 8, 'abc') + addSubject(db, 601, 8, 'ABC') + }, + /differ only by case/, + ], + ] + + for (const [situation, change, message] of aborts) { + it(`aborts, changing nothing, when ${situation}`, () => { + seedProductionRows(sqlite) + change(sqlite) + const before = sqlite.serialize() + + expect(() => applyFix(sqlite)).toThrow(message) + + expect(Buffer.from(sqlite.serialize()).equals(Buffer.from(before))).toBe(true) + }) + } +}) diff --git a/packages/api/src/db/__tests__/migration-backup.test.ts b/packages/api/src/db/__tests__/migration-backup.test.ts new file mode 100644 index 00000000..175ff995 --- /dev/null +++ b/packages/api/src/db/__tests__/migration-backup.test.ts @@ -0,0 +1,166 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { Database } from 'bun:sqlite' +import { chmodSync, mkdtempSync, readdirSync, rmSync } from 'fs' +import { join } from 'path' +import { tmpdir } from 'os' +import { CURRENT_SCHEMA_VERSION, evolveOperationalSchema, getRecordedSchemaVersion } from '../schema-evolution' +import { openOperationalDatabase } from '../open' +import { MigrationBackupError } from '../migration-backup' + +const PREVIOUS_VERSION = CURRENT_SCHEMA_VERSION - 1 + +/** A database one version behind current, holding three studies. */ +function createDatabaseOneVersionBehind(path: string): void { + const sqlite = new Database(path) + evolveOperationalSchema(sqlite) + for (const code of ['A', 'B', 'C']) { + sqlite + .prepare( + `INSERT INTO study (title, short_code, is_longitudinal, lead_person, created, last_updated) + VALUES (?, ?, 0, 'Lead', '2026-01-01', '2026-01-01')`, + ) + .run(`Study ${code}`, code) + } + sqlite.prepare('UPDATE schema_version SET version = ?').run(PREVIOUS_VERSION) + sqlite.close() +} + +function backupFiles(dir: string): string[] { + return readdirSync(dir).filter((name) => name.endsWith('.bak')) +} + +describe('backup before startup migrations', () => { + let dir: string + let dbPath: string + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'migration-backup-')) + dbPath = join(dir, 'sampledb.sqlite') + }) + + afterEach(() => { + chmodSync(dir, 0o755) + rmSync(dir, { recursive: true, force: true }) + }) + + it('writes a copy with the pre-migration version and rows before migrating', () => { + createDatabaseOneVersionBehind(dbPath) + + const { sqlite } = openOperationalDatabase(dbPath, { backupBeforeMigrating: true }) + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + sqlite.close() + + const backups = backupFiles(dir) + expect(backups).toHaveLength(1) + expect(backups[0]).toMatch( + new RegExp(`^sampledb\\.sqlite\\.pre-v${CURRENT_SCHEMA_VERSION}-\\d{8}T\\d{6}\\d{3}Z\\.bak$`), + ) + + // Read-only: inspecting the copy must not migrate it. + const copy = new Database(join(dir, backups[0]), { readonly: true }) + expect(getRecordedSchemaVersion(copy)).toBe(PREVIOUS_VERSION) + expect(copy.prepare('SELECT COUNT(*) AS n FROM study').get()).toEqual({ n: 3 }) + copy.close() + }) + + it('includes rows another connection left in the WAL', () => { + createDatabaseOneVersionBehind(dbPath) + const writer = new Database(dbPath) + writer.exec('PRAGMA journal_mode = WAL') + writer.exec('PRAGMA wal_autocheckpoint = 0') + writer + .prepare( + `INSERT INTO study (title, short_code, is_longitudinal, lead_person, created, last_updated) + VALUES ('Study D', 'D', 0, 'Lead', '2026-01-01', '2026-01-01')`, + ) + .run() + + const { sqlite } = openOperationalDatabase(dbPath, { backupBeforeMigrating: true }) + sqlite.close() + writer.close() + + const [backup] = backupFiles(dir) + const copy = new Database(join(dir, backup), { readonly: true }) + expect(copy.prepare('SELECT COUNT(*) AS n FROM study').get()).toEqual({ n: 4 }) + copy.close() + }) + + it('backs up an unversioned legacy database before its baseline migration', () => { + const legacy = new Database(dbPath) + legacy.exec('CREATE TABLE study (id INTEGER PRIMARY KEY, title TEXT, short_code TEXT)') + legacy.exec("INSERT INTO study (title, short_code) VALUES ('Legacy', 'L')") + legacy.close() + + // Only the copy matters here; the bare legacy file can't finish migrating. + try { + openOperationalDatabase(dbPath, { backupBeforeMigrating: true }).sqlite.close() + } catch { + // ignore + } + + const [backup] = backupFiles(dir) + const copy = new Database(join(dir, backup), { readonly: true }) + expect(getRecordedSchemaVersion(copy)).toBeNull() + expect(copy.prepare('SELECT COUNT(*) AS n FROM study').get()).toEqual({ n: 1 }) + copy.close() + }) + + it('writes no copy when no migration is pending', () => { + openOperationalDatabase(dbPath, { backupBeforeMigrating: true }).sqlite.close() + openOperationalDatabase(dbPath, { backupBeforeMigrating: true }).sqlite.close() + + expect(backupFiles(dir)).toEqual([]) + }) + + it('writes no copy of a test database by default', () => { + expect(process.env.NODE_ENV).toBe('test') + createDatabaseOneVersionBehind(dbPath) + + const { sqlite } = openOperationalDatabase(dbPath) + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + sqlite.close() + + expect(backupFiles(dir)).toEqual([]) + }) + + it('migrates an in-memory database without a copy', () => { + // A copy of ':memory:' would land relative to the working directory, so run from `dir`. + const previousCwd = process.cwd() + process.chdir(dir) + try { + const { sqlite } = openOperationalDatabase(':memory:', { backupBeforeMigrating: true }) + sqlite.prepare('UPDATE schema_version SET version = ?').run(PREVIOUS_VERSION) + + evolveOperationalSchema(sqlite, { backupBeforeMigrating: true }) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + sqlite.close() + } finally { + process.chdir(previousCwd) + } + expect(backupFiles(dir)).toEqual([]) + }) + + it('stops without migrating when the copy cannot be written', () => { + createDatabaseOneVersionBehind(dbPath) + const sqlite = new Database(dbPath) + sqlite.exec('PRAGMA journal_mode = WAL') + expect(getRecordedSchemaVersion(sqlite)).toBe(PREVIOUS_VERSION) + chmodSync(dir, 0o555) + + let thrown: unknown + try { + evolveOperationalSchema(sqlite, { backupBeforeMigrating: true }) + } catch (error: unknown) { + thrown = error + } + + expect(thrown).toBeInstanceOf(MigrationBackupError) + expect((thrown as Error).message).toContain(`Could not back up ${dbPath}`) + expect((thrown as Error).message).toContain('No migrations were run') + expect(getRecordedSchemaVersion(sqlite)).toBe(PREVIOUS_VERSION) + sqlite.close() + chmodSync(dir, 0o755) + expect(backupFiles(dir)).toEqual([]) + }) +}) diff --git a/packages/api/src/db/__tests__/schema-evolution.test.ts b/packages/api/src/db/__tests__/schema-evolution.test.ts index 65caa5ae..0cf32f2f 100644 --- a/packages/api/src/db/__tests__/schema-evolution.test.ts +++ b/packages/api/src/db/__tests__/schema-evolution.test.ts @@ -7,10 +7,14 @@ import { CURRENT_SCHEMA_VERSION, evolveOperationalSchema, getRecordedSchemaVersion, + hasSchemaVersionTable, isEmptyOperationalDatabase, } from '../schema-evolution' import { openOperationalDatabase } from '../open' -import { SchemaMigrationError, runSqlMigrationStatements } from '../migration-runner' +import { SchemaMigrationError, listNumberedMigrations, runSqlMigration, runSqlMigrationStatements } from '../migration-runner' +import { drizzle } from 'drizzle-orm/bun-sqlite' +import * as schema from '../schema' +import { listContainersWithInvalidGridPosition, listSpecimensWithNonIsoCollectionDate } from '../../lib/data-audit' /** Tables that migrations 004+ alter, so older-version fixtures can migrate to current. */ function createTablesForLaterMigrations(sqlite: Database): void { @@ -42,6 +46,46 @@ function createTablesForLaterMigrations(sqlite: Database): void { box_id INTEGER, bag_id INTEGER )`) + sqlite.exec(`CREATE TABLE IF NOT EXISTS specimen ( + id INTEGER PRIMARY KEY, + study_subject_id INTEGER, + specimen_type_id INTEGER NOT NULL, + collection_date DATE + )`) + sqlite.exec(`CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY, email TEXT NOT NULL, created TEXT, approved_at TEXT)`) + sqlite.exec(`CREATE TABLE IF NOT EXISTS sessions ( + id TEXT PRIMARY KEY, + user_id INTEGER NOT NULL REFERENCES users(id), + expires_at INTEGER NOT NULL + )`) + // Migration 011 indexes these columns. Some tests create a bare `study (id)` first. + ensureColumns(sqlite, 'study', ['title TEXT', 'short_code TEXT']) + ensureColumns(sqlite, 'specimen', ['control_batch_id INTEGER']) + ensureColumns(sqlite, 'storage_container', ['specimen_id INTEGER']) + ensureColumns(sqlite, 'storage_container_tag', ['storage_container_id INTEGER', 'tag_id INTEGER']) + ensureColumns(sqlite, 'control_batch', ['control_definition_id INTEGER', 'properties TEXT']) + ensureColumns(sqlite, 'location', ['parent_id INTEGER', 'path TEXT']) + ensureColumns(sqlite, 'micronix_plate', ['location_id INTEGER']) + ensureColumns(sqlite, 'cryovial_box', ['location_id INTEGER']) + ensureColumns(sqlite, 'bag', ['location_id INTEGER']) + // Before version 3, migration 003 renames barcode to sublabel (and its preflight creates the + // table if missing), so only a database already past 003 gets paper in its later shape. + const version = hasSchemaVersionTable(sqlite) ? getRecordedSchemaVersion(sqlite) : null + if (version != null && version >= 3) ensureColumns(sqlite, 'paper', ['sheet_id INTEGER', 'sublabel TEXT']) +} + +/** Create `table` with `columns`, or add the ones an existing `table` lacks. */ +function ensureColumns(sqlite: Database, table: string, columns: string[]): void { + sqlite.exec(`CREATE TABLE IF NOT EXISTS ${table} (id INTEGER PRIMARY KEY)`) + const existing = new Set( + sqlite + .query<{ name: string }, []>(`SELECT name FROM pragma_table_info('${table}')`) + .all() + .map((row) => row.name), + ) + for (const column of columns) { + if (!existing.has(column.split(' ')[0])) sqlite.exec(`ALTER TABLE ${table} ADD COLUMN ${column}`) + } } describe('schema evolution', () => { @@ -293,6 +337,66 @@ describe('schema evolution', () => { sqlite.close() }) + it('migration 008 rewrites M/D/YY and M/D/YYYY collection dates as YYYY-MM-DD', () => { + const sqlite = new Database(testDbPath) + sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') + sqlite.exec('INSERT INTO schema_version (version) VALUES (7)') + createTablesForLaterMigrations(sqlite) + const values: Array<[number, string | null]> = [ + [1, '5/7/26'], + [2, '9/14/20'], + [3, '12/24/2024'], + [4, '07/04/2024'], + [5, '2023-08-21'], + [6, null], + [7, ' '], + [8, ' 3/17/22 '], + [9, '1970-01-01'], + ] + const insert = sqlite.prepare('INSERT INTO specimen (id, specimen_type_id, collection_date) VALUES (?, 1, ?)') + for (const [id, date] of values) insert.run(id, date) + + evolveOperationalSchema(sqlite) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + const rows = sqlite.prepare('SELECT collection_date AS date FROM specimen ORDER BY id').all() as Array<{ date: string | null }> + expect(rows.map((r) => r.date)).toEqual([ + '2026-05-07', + '2020-09-14', + '2024-12-24', + '2024-07-04', + '2023-08-21', + null, + null, + '2022-03-17', + '1970-01-01', + ]) + sqlite.close() + }) + + it('migration 008 leaves a collection date it cannot read as it is, converts the rest, and the integrity report lists it', async () => { + const sqlite = new Database(testDbPath) + sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') + sqlite.exec('INSERT INTO schema_version (version) VALUES (7)') + createTablesForLaterMigrations(sqlite) + const unreadable = ['13/2/2024', '2/30/2024', '2024-02-30', '5/7', '7 Apr 2024', '2024/05/07', '5/7/124', ' 2/30/24 '] + const insert = sqlite.prepare('INSERT INTO specimen (id, specimen_type_id, collection_date) VALUES (?, 1, ?)') + insert.run(1, '5/7/26') + unreadable.forEach((value, i) => insert.run(i + 2, value)) + insert.run(100, ' 12/24/2024 ') + + evolveOperationalSchema(sqlite) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + const rows = sqlite.prepare('SELECT collection_date AS date FROM specimen ORDER BY id').all() as Array<{ date: string }> + expect(rows.map((r) => r.date)).toEqual(['2026-05-07', ...unreadable, '2024-12-24']) + const temp = sqlite.prepare(`SELECT name FROM sqlite_temp_master WHERE name LIKE '__migration_008%'`).all() + expect(temp).toEqual([]) + const leftovers = await listSpecimensWithNonIsoCollectionDate(drizzle(sqlite, { schema })) + expect(leftovers).toEqual(unreadable.map((collectionDate, i) => ({ id: i + 2, collectionDate }))) + sqlite.close() + }) + /** The CREATE TABLE text SQLite stored for qpcr_well_result. */ function qpcrWellResultDdl(sqlite: Database): string { const row = sqlite.prepare("SELECT sql FROM sqlite_master WHERE type='table' AND name='qpcr_well_result'").get() as @@ -311,6 +415,7 @@ describe('schema evolution', () => { const migrated = new Database(testDbPath) migrated.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') migrated.exec('INSERT INTO schema_version (version) VALUES (6)') + createTablesForLaterMigrations(migrated) evolveOperationalSchema(migrated) expect(getRecordedSchemaVersion(migrated)).toBe(CURRENT_SCHEMA_VERSION) expect(qpcrWellResultDdl(migrated)).toContain('CONSTRAINT qpcr_well_result_run_well_target_unique UNIQUE') @@ -329,6 +434,7 @@ describe('schema evolution', () => { UNIQUE (qpcr_run_id, well_position, target_name) )` sqlite.exec(existing) + createTablesForLaterMigrations(sqlite) evolveOperationalSchema(sqlite) @@ -340,6 +446,197 @@ describe('schema evolution', () => { sqlite.close() }) + it('migration 009 drops sessions stored by cookie value and stores sessions by token hash', () => { + const sqlite = new Database(testDbPath) + sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') + sqlite.exec('INSERT INTO schema_version (version) VALUES (7)') + createTablesForLaterMigrations(sqlite) + sqlite.exec(`INSERT INTO users (id, email) VALUES (1, 'a@test.com')`) + sqlite.exec(`INSERT INTO sessions (id, user_id, expires_at) VALUES ('raw-cookie-value', 1, 9999999999)`) + + evolveOperationalSchema(sqlite) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(sqlite.prepare('SELECT COUNT(*) AS n FROM sessions').get()).toEqual({ n: 0 }) + const columns = (sqlite.prepare('PRAGMA table_info(sessions)').all() as Array<{ name: string; notnull: number }>) + .map((column) => `${column.name}${column.notnull ? ' NOT NULL' : ''}`) + expect(columns).toEqual(['id', 'token_hash NOT NULL', 'user_id NOT NULL', 'expires_at NOT NULL']) + sqlite.exec(`INSERT INTO sessions (id, token_hash, user_id, expires_at) VALUES ('p1', 'h1', 1, 1)`) + expect(() => + sqlite.exec(`INSERT INTO sessions (id, token_hash, user_id, expires_at) VALUES ('p2', 'h1', 1, 1)`), + ).toThrow(/UNIQUE/) + const plan = sqlite + .prepare(`EXPLAIN QUERY PLAN SELECT id FROM sessions WHERE token_hash = 'h1'`) + .all() as Array<{ detail: string }> + expect(plan.map((row) => row.detail).join(' ')).toContain('sessions_token_hash_idx') + sqlite.close() + }) + + /** A database at version 9 with grid tables, ready for migration 010. */ + function databaseBeforeMigration010(): Database { + const sqlite = new Database(testDbPath) + sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') + sqlite.exec('INSERT INTO schema_version (version) VALUES (9)') + createTablesForLaterMigrations(sqlite) + return sqlite + } + + function positions(sqlite: Database, table: string): Array<[number, string | null]> { + const rows = sqlite.prepare(`SELECT id, position FROM ${table} ORDER BY id`).all() as Array<{ id: number; position: string | null }> + return rows.map((r) => [r.id, r.position]) + } + + it('migration 010 strips dashes, clears barcode-shaped and NA positions, and leaves valid ones', () => { + const sqlite = databaseBeforeMigration010() + sqlite.exec(`INSERT INTO cryovial_tube (id, collection_id, barcode, position) VALUES + (1, 790, 'C1', '--A01'), (2, 790, 'C2', '-F02'), (3, 790, 'C3', 'G01'), (4, 84, 'C4', 'NA01'), + (5, 84, 'C5', 'na05'), (6, 84, NULL, NULL), (7, 84, 'C7', 'b3'), (8, 84, 'C8', ' J10 ')`) + sqlite.exec(`INSERT INTO micronix_tube (id, collection_id, barcode, position) VALUES + (11, 254, '1145854300', 'A1145854300'), (12, 254, '1106665111', 'A1106665111'), (13, 254, 'MX3', NULL), (14, 1, 'MX4', 'H12')`) + sqlite.exec(`INSERT INTO static_well (id, collection_id, position) VALUES (21, 1, '-a1')`) + + evolveOperationalSchema(sqlite) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(positions(sqlite, 'cryovial_tube')).toEqual([ + [1, 'A01'], [2, 'F02'], [3, 'G01'], [4, null], [5, null], [6, null], [7, 'B03'], [8, 'J10'], + ]) + expect(positions(sqlite, 'micronix_tube')).toEqual([[11, null], [12, null], [13, null], [14, 'H12']]) + expect(positions(sqlite, 'static_well')).toEqual([[21, 'A01']]) + const temp = sqlite.prepare(`SELECT name FROM sqlite_temp_master WHERE name LIKE '__migration_010%'`).all() + expect(temp).toEqual([]) + sqlite.close() + }) + + it('migration 010 leaves a repair that would collide with a stored position as it is, and repairs the rest', () => { + const sqlite = databaseBeforeMigration010() + sqlite.exec(`INSERT INTO cryovial_tube (id, collection_id, barcode, position) VALUES (1, 790, 'C1', '--A01'), (2, 790, 'C2', 'A01'), (3, 790, 'C3', '-B01')`) + + evolveOperationalSchema(sqlite) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(positions(sqlite, 'cryovial_tube')).toEqual([[1, '--A01'], [2, 'A01'], [3, 'B01']]) + sqlite.close() + }) + + it('migration 010 leaves every repair that lands in one position as it is, and repairs the rest', () => { + const sqlite = databaseBeforeMigration010() + sqlite.exec(`INSERT INTO micronix_tube (id, collection_id, barcode, position) VALUES (1, 5, 'M1', '--C04'), (3, 5, 'M3', '-d5')`) + sqlite.exec(`INSERT INTO static_well (id, collection_id, position) VALUES (2, 5, 'c4')`) + + evolveOperationalSchema(sqlite) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(positions(sqlite, 'micronix_tube')).toEqual([[1, '--C04'], [3, 'D05']]) + expect(positions(sqlite, 'static_well')).toEqual([[2, 'c4']]) + sqlite.close() + }) + + it('migration 010 allows a collision of the same position in different Collections', () => { + const sqlite = databaseBeforeMigration010() + sqlite.exec(`INSERT INTO cryovial_tube (id, collection_id, barcode, position) VALUES (1, 790, 'C1', '--A01'), (2, 794, 'C2', 'A01')`) + + evolveOperationalSchema(sqlite) + + expect(positions(sqlite, 'cryovial_tube')).toEqual([[1, 'A01'], [2, 'A01']]) + sqlite.close() + }) + + it('migration 010 leaves a position it cannot repair as it is, repairs the rest, and the integrity report lists what is left', async () => { + const sqlite = databaseBeforeMigration010() + sqlite.exec(`INSERT INTO cryovial_tube (id, collection_id, barcode, position) VALUES + (1, 1, 'X1', '--A02'), (2, 1, 'X2', 'K01'), (3, 1, 'X3', '--A11'), (4, 1, 'X4', ' NA01 '), (5, 1, 'X5', '--A03'), (6, 1, 'X6', 'A03')`) + sqlite.exec(`INSERT INTO micronix_tube (id, collection_id, barcode, position) VALUES + (11, 2, 'M1', 'I01'), (12, 2, 'M2', 'A001'), (13, 2, 'M3', 'hello'), (14, 2, 'M4', 'b7')`) + + evolveOperationalSchema(sqlite) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(positions(sqlite, 'cryovial_tube')).toEqual([[1, 'A02'], [2, 'K01'], [3, '--A11'], [4, null], [5, '--A03'], [6, 'A03']]) + expect(positions(sqlite, 'micronix_tube')).toEqual([[11, 'I01'], [12, 'A001'], [13, 'hello'], [14, 'B07']]) + const temp = sqlite.prepare(`SELECT name FROM sqlite_temp_master WHERE name LIKE '__migration_010%'`).all() + expect(temp).toEqual([]) + const leftovers = await listContainersWithInvalidGridPosition(drizzle(sqlite, { schema })) + expect(leftovers).toEqual([ + { id: 11, containerType: 'micronix_tube', collectionId: 2, position: 'I01' }, + { id: 12, containerType: 'micronix_tube', collectionId: 2, position: 'A001' }, + { id: 13, containerType: 'micronix_tube', collectionId: 2, position: 'hello' }, + { id: 2, containerType: 'cryovial_tube', collectionId: 1, position: 'K01' }, + { id: 3, containerType: 'cryovial_tube', collectionId: 1, position: '--A11' }, + { id: 5, containerType: 'cryovial_tube', collectionId: 1, position: '--A03' }, + ]) + sqlite.close() + }) + + /** A database at version 11 with control batches, ready for migration 012. */ + function databaseBeforeMigration012(): Database { + const sqlite = new Database(testDbPath) + sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') + sqlite.exec('INSERT INTO schema_version (version) VALUES (11)') + createTablesForLaterMigrations(sqlite) + const insert = sqlite.prepare('INSERT INTO control_batch (id, control_definition_id, properties) VALUES (?, 1, ?)') + const object = { legacy_study_id: 94, lead_person: 'Ann', nested: { strains: [1, 2] } } + insert.run(1, JSON.stringify(object)) + insert.run(2, JSON.stringify(JSON.stringify(object))) + insert.run(3, null) + insert.run(4, JSON.stringify(JSON.stringify({}))) + insert.run(5, '{}') + return sqlite + } + + function storedProperties(sqlite: Database): Array<[number, string | null, string | null]> { + const rows = sqlite + .prepare(`SELECT id, properties, json_type(properties) AS type FROM control_batch ORDER BY id`) + .all() as Array<{ id: number; properties: string | null; type: string | null }> + return rows.map((r) => [r.id, r.properties, r.type]) + } + + const MIGRATED_PROPERTIES: Array<[number, string | null, string | null]> = [ + [1, '{"legacy_study_id":94,"lead_person":"Ann","nested":{"strains":[1,2]}}', 'object'], + [2, '{"legacy_study_id":94,"lead_person":"Ann","nested":{"strains":[1,2]}}', 'object'], + [3, null, null], + [4, '{}', 'object'], + [5, '{}', 'object'], + ] + + it('migration 012 stores double-encoded control batch properties once and leaves single-encoded ones alone', () => { + const sqlite = databaseBeforeMigration012() + + evolveOperationalSchema(sqlite) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(storedProperties(sqlite)).toEqual(MIGRATED_PROPERTIES) + sqlite.close() + }) + + it('migration 012 changes nothing when run again', () => { + const sqlite = databaseBeforeMigration012() + evolveOperationalSchema(sqlite) + const migration = listNumberedMigrations().find((m) => m.version === 12) + expect(migration).toBeDefined() + + runSqlMigration(sqlite, 12, migration!.path) + + expect(storedProperties(sqlite)).toEqual(MIGRATED_PROPERTIES) + sqlite.close() + }) + + it('migration 012 leaves a JSON string property that does not hold an object as it is', () => { + const sqlite = databaseBeforeMigration012() + sqlite.prepare('INSERT INTO control_batch (id, control_definition_id, properties) VALUES (6, 1, ?)').run(JSON.stringify('not json {')) + sqlite.prepare('INSERT INTO control_batch (id, control_definition_id, properties) VALUES (7, 1, ?)').run(JSON.stringify('[1,2]')) + + evolveOperationalSchema(sqlite) + + const rows = storedProperties(sqlite) + expect(rows.slice(0, 5)).toEqual(MIGRATED_PROPERTIES) + expect(rows.slice(5)).toEqual([ + [6, '"not json {"', 'text'], + [7, '"[1,2]"', 'text'], + ]) + sqlite.close() + }) + it('fail-hard: invalid migration leaves schema_version unchanged', () => { const sqlite = new Database(testDbPath) sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') diff --git a/packages/api/src/db/__tests__/schema-indexes.test.ts b/packages/api/src/db/__tests__/schema-indexes.test.ts new file mode 100644 index 00000000..073fb471 --- /dev/null +++ b/packages/api/src/db/__tests__/schema-indexes.test.ts @@ -0,0 +1,249 @@ +import { describe, it, expect } from 'vitest' +import { Database } from 'bun:sqlite' +import { readFileSync } from 'fs' +import { dirname, join } from 'path' +import { fileURLToPath } from 'url' +import { SQL, is } from 'drizzle-orm' +import { SQLiteSyncDialect, SQLiteTable, getTableConfig } from 'drizzle-orm/sqlite-core' +import * as schema from '../schema' +import { applyInitialSchema } from '../apply-initial-schema' +import { splitSqlStatements } from '../migration-runner' +import { CURRENT_SCHEMA_VERSION, evolveOperationalSchema, getRecordedSchemaVersion } from '../schema-evolution' + +/** + * Index definitions compared by name: table, uniqueness, key columns or expressions in order, + * and the partial-index WHERE. Text is normalised so that Drizzle's rendering + * (`lower(trim("micronix_tube"."barcode"))`) and SQLite's stored DDL + * (`lower(trim(barcode))`) compare equal. + */ +type IndexDefinition = { table: string; unique: boolean; keys: string[]; where: string | null } + +const dialect = new SQLiteSyncDialect() + +function normalise(text: string, table: string): string { + return text + .replace(/["`[\]]/g, '') + .replace(new RegExp(`\\b${table}\\.`, 'gi'), '') + .replace(/\s+/g, ' ') + .replace(/\s*([(),])\s*/g, '$1') + .trim() + .toLowerCase() +} + +/** Every index schema.ts declares, by name. Unique column constraints are not indexes here. */ +function declaredIndexes(): Map { + const declared = new Map() + for (const value of Object.values(schema)) { + if (!is(value, SQLiteTable)) continue + const { name: table, indexes } = getTableConfig(value) + for (const { config } of indexes) { + if (declared.has(config.name)) throw new Error(`index ${config.name} declared twice`) + declared.set(config.name, { + table, + unique: config.unique, + keys: config.columns.map((key) => normalise(is(key, SQL) ? dialect.sqlToQuery(key).sql : key.name, table)), + where: config.where ? normalise(dialect.sqlToQuery(config.where).sql, table) : null, + }) + } + } + return declared +} + +/** Split `a, f(b, c), d` on top-level commas. */ +function splitTopLevel(list: string): string[] { + const parts: string[] = [] + let depth = 0 + let start = 0 + for (let i = 0; i < list.length; i++) { + if (list[i] === '(') depth++ + else if (list[i] === ')') depth-- + else if (list[i] === ',' && depth === 0) { + parts.push(list.slice(start, i)) + start = i + 1 + } + } + parts.push(list.slice(start)) + return parts +} + +/** + * Every index a CREATE INDEX statement made, by name. Indexes SQLite creates for UNIQUE and + * PRIMARY KEY constraints have no SQL and are left out, as they are of declaredIndexes. + */ +function databaseIndexes(sqlite: Database): Map { + const rows = sqlite + .query<{ name: string; table: string; sql: string }, []>( + `SELECT name, tbl_name AS "table", sql FROM sqlite_master WHERE type = 'index' AND sql IS NOT NULL`, + ) + .all() + const found = new Map() + for (const { name, table, sql } of rows) { + const open = sql.indexOf('(', sql.search(/\bON\b/i)) + let depth = 0 + let close = open + for (; close < sql.length; close++) { + if (sql[close] === '(') depth++ + else if (sql[close] === ')' && --depth === 0) break + } + const where = sql.slice(close + 1).match(/^\s*WHERE\s+([\s\S]*?)\s*;?\s*$/i)?.[1] ?? null + const unique = sqlite + .query<{ unique: number }, []>(`SELECT "unique" FROM pragma_index_list('${table}') WHERE name = '${name}'`) + .get()!.unique + found.set(name, { + table, + unique: unique === 1, + keys: splitTopLevel(sql.slice(open + 1, close)).map((key) => normalise(key, table)), + where: where == null ? null : normalise(where, table), + }) + } + return found +} + +/** The differences between two index maps, one line per index, empty when they agree. */ +function indexDifferences(expected: Map, actual: Map): string[] { + const differences: string[] = [] + for (const [name, definition] of expected) { + const found = actual.get(name) + if (!found) differences.push(`missing ${name}`) + else if (JSON.stringify(found) !== JSON.stringify(definition)) { + differences.push(`${name}: expected ${JSON.stringify(definition)}, found ${JSON.stringify(found)}`) + } + } + for (const name of actual.keys()) { + if (!expected.has(name)) differences.push(`undeclared ${name}`) + } + return differences.sort() +} + +const VERSION_1_SNAPSHOT = join(dirname(fileURLToPath(import.meta.url)), 'fixtures', 'initial_schema_v1.sql') + +function openVersion1Install(): Database { + const sqlite = new Database(':memory:') + for (const statement of splitSqlStatements(readFileSync(VERSION_1_SNAPSHOT, 'utf-8'))) { + sqlite.exec(statement) + } + return sqlite +} + +/** + * Production files that predate the versioned snapshot hold some schema.ts indexes under other + * names, lack others, and have none of the indexes added later. Migration 011 renames the first + * and adds the second. Each entry: production name (null when production lacks the index), + * schema.ts name, and what the production index is on. + */ +const PRODUCTION_INDEX_NAMES: Array<[string | null, string, string]> = [ + ['idx_specimen_study_subject_id', 'specimen_study_subject_id_idx', 'specimen(study_subject_id)'], + ['idx_specimen_control_batch_id', 'specimen_control_batch_id_idx', 'specimen(control_batch_id)'], + ['idx_storage_container_specimen_id', 'storage_container_specimen_id_idx', 'storage_container(specimen_id)'], + ['idx_study_short_code', 'study_short_code_idx', 'study(short_code)'], + ['idx_study_subject_study_id', 'study_subject_study_id_idx', 'study_subject(study_id)'], + ['idx_study_subject_name', 'study_subject_name_idx', 'study_subject(name)'], + ['idx_location_new_parent_id', 'idx_location_parent_id', 'location(parent_id)'], + ['idx_location_new_path', 'idx_location_path', 'location(path)'], + [null, 'study_title_idx', ''], + [null, 'control_batch_control_definition_id_idx', ''], + [null, 'specimen_collection_date_idx', ''], + [null, 'specimen_specimen_type_id_idx', ''], + [null, 'micronix_plate_location_id_idx', ''], + [null, 'cryovial_box_location_id_idx', ''], + [null, 'bag_location_id_idx', ''], +] + +/** + * Indexes schema.ts declares that migrations leave off production files, because their data + * breaks them: those files hold more than one Specimen for some collection events. + */ +const NOT_ADDED_TO_PRODUCTION = ['idx_specimen_study_subject_type_date', 'idx_specimen_control_batch_type_date'] + +/** A version 1 install with production's index names, at the version the production snapshot had. */ +function openProductionShapedInstall(): Database { + const sqlite = openVersion1Install() + for (const [legacyName, schemaName, on] of PRODUCTION_INDEX_NAMES) { + sqlite.exec(`DROP INDEX ${schemaName}`) + if (legacyName) sqlite.exec(`CREATE INDEX ${legacyName} ON ${on}`) + } + for (const name of NOT_ADDED_TO_PRODUCTION) sqlite.exec(`DROP INDEX ${name}`) + // Redundant with the unique study_subject_study_name_idx that migration 005 adds. + sqlite.exec('CREATE INDEX idx_study_subject_study_name ON study_subject(study_id, name)') + return sqlite +} + +describe('indexes declared in schema.ts', () => { + const declared = declaredIndexes() + + it('finds the declarations, including expression and partial indexes', () => { + expect(declared.size).toBeGreaterThan(40) + expect(declared.get('micronix_tube_barcode_folded_idx')).toEqual({ + table: 'micronix_tube', + unique: false, + keys: ['lower(trim(barcode))'], + where: null, + }) + expect(declared.get('sheet_box_name_idx')).toEqual({ + table: 'sheet', + unique: true, + keys: ['box_id', 'name'], + where: 'box_id is not null', + }) + }) + + it('match a new install from initial_schema.sql', () => { + const sqlite = new Database(':memory:') + try { + applyInitialSchema(sqlite) + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(indexDifferences(declared, databaseIndexes(sqlite))).toEqual([]) + } finally { + sqlite.close() + } + }) + + it('match a version 1 install after the migrations', () => { + const sqlite = openVersion1Install() + try { + evolveOperationalSchema(sqlite) + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(indexDifferences(declared, databaseIndexes(sqlite))).toEqual([]) + } finally { + sqlite.close() + } + }) + + it('match a production file after the migrations, except the Specimen collection event indexes', () => { + const sqlite = openProductionShapedInstall() + try { + expect(indexDifferences(declared, databaseIndexes(sqlite))).not.toEqual([]) + evolveOperationalSchema(sqlite) + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(indexDifferences(declared, databaseIndexes(sqlite))).toEqual( + NOT_ADDED_TO_PRODUCTION.map((name) => `missing ${name}`).sort(), + ) + } finally { + sqlite.close() + } + }) + + it('notice a missing, undeclared or changed index', () => { + const sqlite = new Database(':memory:') + try { + applyInitialSchema(sqlite) + sqlite.exec('DROP INDEX paper_sublabel_idx') + sqlite.exec('CREATE INDEX paper_extra_idx ON paper(sublabel)') + sqlite.exec('DROP INDEX cryovial_tube_barcode_folded_idx') + sqlite.exec('CREATE INDEX cryovial_tube_barcode_folded_idx ON cryovial_tube(lower(barcode))') + sqlite.exec('DROP INDEX sheet_box_name_idx') + sqlite.exec('CREATE UNIQUE INDEX sheet_box_name_idx ON sheet(box_id, name)') + sqlite.exec('DROP INDEX static_well_collection_id_idx') + sqlite.exec('CREATE UNIQUE INDEX static_well_collection_id_idx ON static_well(collection_id)') + expect(indexDifferences(declared, databaseIndexes(sqlite)).map((line) => line.split(':')[0])).toEqual([ + 'cryovial_tube_barcode_folded_idx', + 'missing paper_sublabel_idx', + 'sheet_box_name_idx', + 'static_well_collection_id_idx', + 'undeclared paper_extra_idx', + ]) + } finally { + sqlite.close() + } + }) +}) diff --git a/packages/api/src/db/data-fix.ts b/packages/api/src/db/data-fix.ts new file mode 100644 index 00000000..4263f597 --- /dev/null +++ b/packages/api/src/db/data-fix.ts @@ -0,0 +1,180 @@ +import type { Database } from 'bun:sqlite' +import { createHash } from 'crypto' +import { readFileSync } from 'fs' +import { basename } from 'path' +import { databaseCopyPath, writeDatabaseCopy } from './migration-backup' +import { splitSqlStatements } from './migration-runner' +import { CURRENT_SCHEMA_VERSION, getRecordedSchemaVersion } from './schema-evolution' + +/** + * A one-time data fix: SQL one site runs by hand against its own database (README, "One-time + * data fixes"). Unlike a numbered migration it ships to no other install, so it may name that + * site's rows. Statements are separated with `--> statement-breakpoint`, as in migrations. + */ +export interface DataFix { + /** Recorded in `data_fix_log`; the file name without `.sql`. */ + name: string + sql: string +} + +export interface DataFixStatementResult { + /** The first line of the comment block just above the statement, or the statement's first line. */ + label: string + /** Rows inserted, updated or deleted, including by triggers and foreign key actions. */ + changes: number +} + +export type DataFixResult = + | { status: 'applied'; name: string; backupPath: string | null; statements: DataFixStatementResult[] } + | { status: 'dry-run'; name: string; statements: DataFixStatementResult[] } + | { status: 'already-applied'; name: string; appliedAt: string } + +export interface RunDataFixOptions { + /** Run the fix in a transaction that is rolled back, and report what it would change. */ + dryRun?: boolean + now?: Date +} + +export class DataFixError extends Error { + constructor(message: string, cause?: unknown) { + super(message) + this.name = 'DataFixError' + if (cause instanceof Error) this.cause = cause + } +} + +const LOG_TABLE = 'data_fix_log' + +export function loadDataFix(path: string): DataFix { + return { name: basename(path).replace(/\.sql$/i, ''), sql: readFileSync(path, 'utf-8') } +} + +function checksum(sql: string): string { + return createHash('sha256').update(sql).digest('hex') +} + +interface Statement { + sql: string + label: string +} + +/** The fix's statements; a chunk holding only comments (such as the file header) is skipped. */ +function statementsOf(sql: string): Statement[] { + const statements: Statement[] = [] + for (const chunk of splitSqlStatements(sql)) { + const lines = chunk.split('\n').map((line) => line.trim()) + const firstSql = lines.findIndex((line) => line !== '' && !line.startsWith('--')) + if (firstSql === -1 || lines.slice(firstSql).join('').replace(/;/g, '').trim() === '') continue + let commentStart = firstSql + while (commentStart > 0 && lines[commentStart - 1]!.startsWith('--')) commentStart-- + const comment = commentStart < firstSql ? lines[commentStart]!.replace(/^--\s*/, '') : undefined + const label = comment || lines[firstSql]!.replace(/;$/, '') + statements.push({ sql: chunk, label: label.length > 120 ? `${label.slice(0, 117)}...` : label }) + } + return statements +} + +function loggedRun(sqlite: Database, name: string): { checksum: string; applied_at: string } | null { + const table = sqlite.prepare(`SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = ?`).get(LOG_TABLE) + if (!table) return null + return sqlite.prepare(`SELECT checksum, applied_at FROM ${LOG_TABLE} WHERE name = ?`).get(name) as { + checksum: string + applied_at: string + } | null +} + +function totalChanges(sqlite: Database): number { + return (sqlite.prepare('SELECT total_changes() AS n').get() as { n: number }).n +} + +/** Run every statement inside an open transaction, counting the rows each one changed. */ +function runStatements(sqlite: Database, fix: DataFix): DataFixStatementResult[] { + const results: DataFixStatementResult[] = [] + statementsOf(fix.sql).forEach((statement, index) => { + const before = totalChanges(sqlite) + try { + sqlite.exec(statement.sql) + } catch (error: unknown) { + const detail = error instanceof Error ? error.message : String(error) + throw new DataFixError( + `Data fix ${fix.name} failed at statement ${index + 1} (${statement.label}): ${detail}. ` + + 'Every statement was rolled back; the database is unchanged.', + error, + ) + } + results.push({ label: statement.label, changes: totalChanges(sqlite) - before }) + }) + return results +} + +/** + * Run a one-time data fix against a database at CURRENT_SCHEMA_VERSION, in one write + * transaction, and record it in `data_fix_log` so a second run does nothing. A real run first + * copies the database next to its file. Throws DataFixError, with the database unchanged, when + * the schema version differs, the same name ran with different contents, the copy fails, or a + * statement fails. + */ +export function runDataFix(sqlite: Database, fix: DataFix, options: RunDataFixOptions = {}): DataFixResult { + const now = options.now ?? new Date() + const version = getRecordedSchemaVersion(sqlite) + if (version !== CURRENT_SCHEMA_VERSION) { + throw new DataFixError( + `The database is at schema version ${version ?? 'none'}, and data fix ${fix.name} needs ${CURRENT_SCHEMA_VERSION}. ` + + 'Start this version of SampleDB on the database once so it migrates, stop it, then run the fix.', + ) + } + + const sum = checksum(fix.sql) + const previous = loggedRun(sqlite, fix.name) + if (previous) { + if (previous.checksum !== sum) { + throw new DataFixError( + `Data fix ${fix.name} already ran on ${previous.applied_at} with different contents ` + + `(checksum ${previous.checksum}, the file now has ${sum}). Nothing was run. ` + + 'Put further changes in a new data fix with its own name.', + ) + } + return { status: 'already-applied', name: fix.name, appliedAt: previous.applied_at } + } + + if (options.dryRun) { + sqlite.exec('BEGIN IMMEDIATE') + try { + return { status: 'dry-run', name: fix.name, statements: runStatements(sqlite, fix) } + } finally { + sqlite.exec('ROLLBACK') + } + } + + const backupPath = databaseCopyPath(sqlite, `pre-data-fix-${fix.name}`, now) + if (backupPath != null) { + try { + writeDatabaseCopy(sqlite, backupPath) + } catch (error: unknown) { + const detail = error instanceof Error ? error.message : String(error) + throw new DataFixError( + `Could not copy ${sqlite.filename} to ${backupPath} before data fix ${fix.name}: ${detail}. ` + + 'Nothing was run. Make the directory writable (and check free space), then run it again.', + error, + ) + } + } + + sqlite.exec('BEGIN IMMEDIATE') + try { + const statements = runStatements(sqlite, fix) + // The runner's own record, not part of the operational schema: only databases a fix ran on + // have it. Created in the fix's transaction, so a failed first fix leaves no trace. + sqlite.exec(`CREATE TABLE IF NOT EXISTS ${LOG_TABLE} ( + name TEXT PRIMARY KEY NOT NULL, + checksum TEXT NOT NULL, + applied_at TEXT NOT NULL + )`) + sqlite.prepare(`INSERT INTO ${LOG_TABLE} (name, checksum, applied_at) VALUES (?, ?, ?)`).run(fix.name, sum, now.toISOString()) + sqlite.exec('COMMIT') + return { status: 'applied', name: fix.name, backupPath, statements } + } catch (error: unknown) { + sqlite.exec('ROLLBACK') + throw error + } +} diff --git a/packages/api/src/db/migration-backup.ts b/packages/api/src/db/migration-backup.ts new file mode 100644 index 00000000..4075e9cb --- /dev/null +++ b/packages/api/src/db/migration-backup.ts @@ -0,0 +1,68 @@ +import { Database } from 'bun:sqlite' + +export class MigrationBackupError extends Error { + readonly databasePath: string + readonly backupPath: string + + constructor(databasePath: string, backupPath: string, cause: unknown) { + const detail = cause instanceof Error ? cause.message : String(cause) + super( + `Could not back up ${databasePath} to ${backupPath} before running schema migrations: ${detail}. ` + + 'No migrations were run and the database is unchanged. Make the directory writable ' + + '(and check free space), then start again.', + ) + this.name = 'MigrationBackupError' + this.databasePath = databasePath + this.backupPath = backupPath + if (cause instanceof Error) { + this.cause = cause + } + } +} + +/** `20261101T143005123Z`: UTC, sortable, and safe in a file name. */ +function backupTimestamp(now: Date): string { + return now.toISOString().replace(/[-:.]/g, '') +} + +/** + * Where to copy an on-disk database before changing it: next to its file, named + * `.