feat(modules): replay module schema fragments after core's
Phase 2, PR 3 of docs/website/MODULE_SYSTEM.md 2.7. ensureSchema() now replays every installed module's schema fragment immediately after core's schema.sql, per MODULE_API.md 2.6. The work splits across two files on the line of whether a database is needed to know the answer. loader.js VALIDATES a fragment at load time, before anything is mounted, because every rule 2.6 states about the SQL is knowable by reading it; a module that breaks one never mounts (4.4, left column). modules/schema.js EXECUTES it, so the only failures there are the ones the database alone could report, and those are post-mount and answer 503 (4.4, right column). Validation is a leading-verb allowlist -- CREATE, ALTER, INSERT, UPDATE, the four core's own schema.sql uses -- rather than the DROP denylist 2.6 words it as. A fragment is replayed on every boot, so TRUNCATE and DELETE would empty a table at each restart and RENAME would fail at the second one; a denylist only ever bans what somebody thought of. A CREATE TABLE missing IF NOT EXISTS is rejected for the same reason: it works once and fails every boot after, which presents to an operator as a module that broke on restart. The splitter moves to utils/sqlStatements.js so core's schema and a fragment are split by literally the same code, which is what 2.6 promises. It is its own file rather than an export of utils/db.js because the loader validates fragments at require time and must not drag the mariadb pool into app.js's require chain. The replay sits outside ensureSchema's wait-for-the-database retry loop: a fragment that throws is one module's failure, not a signal the database is coming up, and retrying core's whole schema nine more times over one module's bad SQL would turn a 503'd module into a two-minute boot. Found while wiring it: db/seed.js calls ensureSchema() standalone for `npm run seed`, without ever requiring app.js, so the loader has not scanned and fragments()'s 7.6 throw would have broken seeding outright. The replay asks isLoaded() and logs the skip rather than swallowing it -- a booting server quietly getting no module tables is the thing 7.6 exists to prevent. Verification: - 856 server tests pass, 14 new. moduleSchema.test.js injects the query fn, so the exact statements and their order are asserted with the pool at a dead port like every other suite. - routes.manifest.json and routes.guards.json diffs are zero lines, 229 routes -- the phase 2 exit criterion. swagger-output.json regenerates byte-identical. - Run for real against the local MariaDB with two fixture modules: a good fragment created its table, applied its ALTER and seeded its row; a fragment whose SQL passes validation but the server rejects (`id NOTATYPE`) marked only that module startup_failed, its route answering 503 while the other answered 200; a second ensureSchema on the same database was a clean no-op. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -4,6 +4,7 @@ const mariadb = require('mariadb')
|
||||
require('dotenv').config()
|
||||
|
||||
const log = require('./logger')('db')
|
||||
const { splitStatements } = require('./sqlStatements')
|
||||
|
||||
const pool = mariadb.createPool({
|
||||
host: process.env.DB_HOST || '127.0.0.1',
|
||||
@@ -44,28 +45,31 @@ const SCHEMA_PATH = path.join(__dirname, '..', '..', 'db', 'schema.sql')
|
||||
/**
|
||||
* Create tables if they do not exist. Idempotent. Retries while the DB is still
|
||||
* coming up (important under docker-compose even with a healthcheck).
|
||||
*
|
||||
* Once core's schema is in place, every installed module's schema fragment is
|
||||
* replayed after it (MODULE_API.md §2.6). That step is deliberately OUTSIDE the
|
||||
* retry loop: a fragment that throws is that module's failure, not a signal the
|
||||
* database is still coming up, and retrying core's whole schema nine more times
|
||||
* because one module shipped bad SQL would turn a 503'd module into a two-minute
|
||||
* boot. It is also why this file knows nothing about modules beyond the one call
|
||||
* below — the discovery, splitting and per-module failure handling all live in
|
||||
* modules/schema.js, required lazily so that requiring the pool never drags the
|
||||
* loader in with it.
|
||||
*/
|
||||
async function ensureSchema({ retries = 10, delayMs = 2000 } = {}) {
|
||||
await ensureCoreSchema({ retries, delayMs })
|
||||
// eslint-disable-next-line global-require
|
||||
await require('../modules/schema').replayFragments()
|
||||
}
|
||||
|
||||
/** Core's own schema.sql, with the wait-for-the-database retry. */
|
||||
async function ensureCoreSchema({ retries, delayMs }) {
|
||||
for (let attempt = 1; attempt <= retries; attempt++) {
|
||||
try {
|
||||
const conn = await pool.getConnection()
|
||||
try {
|
||||
const sql = fs.readFileSync(SCHEMA_PATH, 'utf8')
|
||||
// Strip `--` comments (full-line AND trailing) before splitting — so a
|
||||
// leading comment block doesn't get glued onto the statement that follows
|
||||
// it, and a `;` inside a trailing comment can't chop a statement in half.
|
||||
// Safe because the schema never puts `--` inside a string literal.
|
||||
const statements = sql
|
||||
.split('\n')
|
||||
.map((line) => {
|
||||
const i = line.indexOf('--')
|
||||
return i === -1 ? line : line.slice(0, i)
|
||||
})
|
||||
.join('\n')
|
||||
.split(';')
|
||||
.map((s) => s.trim())
|
||||
.filter((s) => s.length > 0)
|
||||
for (const statement of statements) {
|
||||
for (const statement of splitStatements(sql)) {
|
||||
await conn.query(statement)
|
||||
}
|
||||
log.info('schema ensured')
|
||||
|
||||
37
server/src/utils/sqlStatements.js
Normal file
37
server/src/utils/sqlStatements.js
Normal file
@@ -0,0 +1,37 @@
|
||||
// ── Splitting a .sql file into statements ──────────────────────────────────
|
||||
//
|
||||
// Extracted from utils/db.js so that core's schema.sql and a module's schema
|
||||
// fragment are split by literally the same code. MODULE_API.md §2.6 promises a
|
||||
// fragment is replayed "statement by statement, split the same way" — with two
|
||||
// copies of this that promise would hold only until one of them was edited.
|
||||
//
|
||||
// It lives in its own file rather than being exported from utils/db.js because
|
||||
// modules/loader.js validates fragments at require time and must not pull the
|
||||
// mariadb pool into app.js's require chain to do it.
|
||||
|
||||
/**
|
||||
* Split a .sql file into individual statements.
|
||||
*
|
||||
* Strips `--` comments (full-line AND trailing) before splitting — so a leading
|
||||
* comment block doesn't get glued onto the statement that follows it, and a `;`
|
||||
* inside a trailing comment can't chop a statement in half. Safe because neither
|
||||
* core's schema nor a conforming fragment puts `--` inside a string literal
|
||||
* (§2.6 states that as a rule a fragment must follow).
|
||||
*
|
||||
* @param {string} sql
|
||||
* @returns {string[]} non-empty, trimmed statements in file order
|
||||
*/
|
||||
function splitStatements(sql) {
|
||||
return sql
|
||||
.split('\n')
|
||||
.map((line) => {
|
||||
const i = line.indexOf('--')
|
||||
return i === -1 ? line : line.slice(0, i)
|
||||
})
|
||||
.join('\n')
|
||||
.split(';')
|
||||
.map((s) => s.trim())
|
||||
.filter((s) => s.length > 0)
|
||||
}
|
||||
|
||||
module.exports = { splitStatements }
|
||||
Reference in New Issue
Block a user