diff --git a/website/MODULE_API.md b/website/MODULE_API.md index 450a99b..25b3010 100644 --- a/website/MODULE_API.md +++ b/website/MODULE_API.md @@ -237,6 +237,40 @@ immediately after it, statement by statement, split the same way. It is subject core's file already follows: `CREATE TABLE IF NOT EXISTS`, `ALTER TABLE … ADD COLUMN IF NOT EXISTS`, no `--` inside a string literal, no `DROP`. +"Split the same way" is shared code, not a shared description: `utils/sqlStatements.js` holds the +splitter and both callers use it. It is its own file rather than an export of `utils/db.js` because +the loader validates fragments at require time and must not pull the mariadb pool into `app.js`'s +require chain to do it. + +**The rules above are enforced at LOAD time, not at replay time** (PR 3). Everything §2.6 states +about the SQL is knowable by reading the file, so a fragment that breaks a rule costs the module its +mount entirely (§4.4's left-hand column) rather than mounting and then 503ing with tables half +created. What is left for the replay is the class of failure only the database can report — an +unknown column type, a bad foreign key — and those are post-mount and answer 503. + +**The check is a leading-verb allowlist: `CREATE`, `ALTER`, `INSERT`, `UPDATE`.** Those are the four +core's own `schema.sql` uses. It is an allowlist rather than the `DROP` denylist this section words +it as because a fragment is **replayed on every boot**: `TRUNCATE` and `DELETE` would empty a table +at every restart, `RENAME` would fail at the second one, and `GRANT`/`SET`/`USE` are core's business. +A denylist only ever bans what somebody thought of. It is a leading-verb check and claims no more: +`ALTER TABLE x DROP COLUMN y` passes it, and catching that needs a SQL parser — a large dependency +for a rule whose job is stopping the obvious foot-gun early. A `CREATE TABLE` missing `IF NOT EXISTS` +is rejected on the same grounds: it succeeds exactly once and fails every boot after, presenting to +an operator as a module that broke on restart. + +**The replay is outside `ensureSchema()`'s retry loop.** Core's schema is retried ten times while the +database comes up; a fragment that throws is one module's failure, not a signal the database is not +ready, and retrying core's whole schema over one module's bad SQL would turn a 503'd module into a +two-minute boot. Partial application is accepted rather than compensated for — MariaDB self-commits +each DDL statement, so no transaction could roll back the tables created before the failing one, and +the idempotence rule is what makes re-running a corrected fragment safe. + +**One caller replays nothing, deliberately.** `db/seed.js` (`npm run seed`) calls `ensureSchema()` +standalone without requiring `app.js`, so no scan has happened and `fragments()`'s §7.6 throw would +break seeding outright. The replay asks `isLoaded()` and logs the skip. That is the only sanctioned +use of that predicate: everywhere else, reading the module list before `load()` still throws, because +a booting server quietly getting no module tables is precisely what §7.6 exists to prevent. + **Table names are namespaced and collision-checked.** New tables must be prefixed `_`. The loader extracts every `CREATE TABLE IF NOT EXISTS ` from the fragment and rejects the module if a name collides with a core table or with another module's — a wrong `DROP`-free fragment can diff --git a/website/MODULE_SYSTEM.md b/website/MODULE_SYSTEM.md index d2f0d5b..33d7194 100644 --- a/website/MODULE_SYSTEM.md +++ b/website/MODULE_SYSTEM.md @@ -453,7 +453,7 @@ too (API §7.2). Exit criterion: `routes.manifest.json` diff is zero lines and every existing test passes. If Phase 2 changes one URL, it is wrong. -**Progress: PRs 1-2 done.** +**Progress: PRs 1-3 done.** - **PR 1** — `installed_modules` and the state machine, with the stored shape and the boot rules settled in §2.4 above. @@ -466,10 +466,18 @@ changes one URL, it is wrong. de-entanglement registries and the two lifecycle hooks throw `not available until phase 2 PR 4/5` rather than no-op — an accepting stub would let a module believe it had registered something. 28 tests, all on the failure paths. +- **PR 3** — schema fragment replay. `ensureSchema()` replays each installed module's fragment after + core's, with the statement splitter extracted to `utils/sqlStatements.js` so both are split by the + same code. The decision that shaped it, recorded in [`MODULE_API.md`](MODULE_API.md) §2.6: the + fragment is **validated at load time and executed later**, split on whether a database is needed to + know the answer — a fragment breaking a stated rule never mounts, while a failure only the server + could report (a bad column type) is post-mount and 503s. The rules are enforced as a **leading-verb + allowlist** (`CREATE`, `ALTER`, `INSERT`, `UPDATE`) rather than the `DROP` denylist §2.6 words them + as, because the file is replayed on **every boot**. Found while wiring it: `npm run seed` calls + `ensureSchema()` without ever requiring `app.js`, so the replay has to tolerate an unscanned loader. -There is still no module on the volume, no schema replay and no boot wiring, so this changes nothing -an operator or a client can see: 842 tests pass and `routes.manifest.json` is unchanged at 229 -routes. +There is still no module on the volume and no boot wiring, so this changes nothing an operator or a +client can see: 856 tests pass and `routes.manifest.json` is unchanged at 229 routes. **Phase 3 — Extract `module-uo`.** Moves out of `website/`: the 8 model directories and their 25 tables; the nine UO `utils/` files plus `newsGump.js`; the 13 router/controller files;