From 16106acfd13ed8b74c2fcc361872b5e2843639ac Mon Sep 17 00:00:00 2001 From: ditadi Date: Mon, 12 Oct 2026 13:20:49 +0100 Subject: [PATCH 1/2] feat(appkit): apply committed database migrations on deploy start A deployed DatabasePlugin app now applies the migrations committed under config/database/migrations before its startup checks, as its own service principal and under a per-schema advisory lock. Pending migrations run in one transaction, and a table lock held for over 30s fails the migration instead of stalling the table. A 0000_baseline whose tables already exist is recorded, in its own transaction, instead of run. Any failure stops startup, and the server log names it: the tags and SQLSTATE, or the missing migration file. Local runs never migrate on start. Signed-off-by: ditadi --- .../appkit/src/database/migrations/apply.ts | 469 ++++++++++ .../src/database/migrations/ownership.ts | 18 + .../appkit/src/database/migrations/paths.ts | 72 ++ .../database/migrations/tests/apply.test.ts | 832 ++++++++++++++++++ .../migrations/tests/ownership.test.ts | 21 + .../appkit/src/plugins/database/lifecycle.ts | 8 +- .../src/plugins/database/migrate-on-start.ts | 78 ++ .../src/plugins/database/schema-check.ts | 10 +- .../plugins/database/tests/lifecycle.test.ts | 97 +- .../database/tests/migrate-on-start.test.ts | 187 ++++ 10 files changed, 1784 insertions(+), 8 deletions(-) create mode 100644 packages/appkit/src/database/migrations/apply.ts create mode 100644 packages/appkit/src/database/migrations/ownership.ts create mode 100644 packages/appkit/src/database/migrations/paths.ts create mode 100644 packages/appkit/src/database/migrations/tests/apply.test.ts create mode 100644 packages/appkit/src/database/migrations/tests/ownership.test.ts create mode 100644 packages/appkit/src/plugins/database/migrate-on-start.ts create mode 100644 packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts diff --git a/packages/appkit/src/database/migrations/apply.ts b/packages/appkit/src/database/migrations/apply.ts new file mode 100644 index 000000000..a4e6dd704 --- /dev/null +++ b/packages/appkit/src/database/migrations/apply.ts @@ -0,0 +1,469 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import path from "node:path"; + +import { readMigrationFiles } from "drizzle-orm/migrator"; +import { drizzle } from "drizzle-orm/node-postgres"; +import { migrate } from "drizzle-orm/node-postgres/migrator"; +import type { Pool, PoolClient } from "pg"; + +import { createLogger } from "../../logging/logger"; +import { ownershipHint } from "./ownership"; +import { + BASELINE_TAG, + type Journal, + type JournalEntry, + MIGRATIONS_TABLE, + MigrationFilesError, + readJournal, + readJsonFile, +} from "./paths"; + +const logger = createLogger("database"); + +interface ApplyMigrationsOptions { + /** Absolute path of the committed migrations folder. */ + readonly migrationsFolder: string; + /** Journal schema = app schema. */ + readonly schemaName: string; +} + +interface ApplyMigrationsResult { + /** Tags applied by this call, in order. */ + readonly applied: readonly string[]; + /** Tags recorded without running, because the catalog already held them (the baseline). */ + readonly adopted: readonly string[]; +} + +/** lock_not_available: a table lock was not granted within LOCK_TIMEOUT. */ +const LOCK_NOT_AVAILABLE = "55P03"; +/** + * Longest wait for a table lock while migrating. Past it the migration fails; + * waiting longer would queue every other query on that table behind it. + */ +const LOCK_TIMEOUT = "30s"; + +function failureHint(code: string | undefined): string | undefined { + if (code === LOCK_NOT_AVAILABLE) { + return `Another session held a lock on a table this migration changes for over ${LOCK_TIMEOUT}; deploy again once it finishes.`; + } + return ownershipHint(code); +} + +/** + * A statement failed while migrating. Pending migrations run in one drizzle-orm + * migrator transaction, so none of them were applied; a baseline adopted just + * before commits on its own and stays recorded. + */ +export class MigrationApplyError extends Error { + /** Tags that were being applied; empty when reading the recorded migrations failed. */ + readonly pending: readonly string[]; + /** SQLSTATE of the failing statement, when the driver reported one. */ + readonly code?: string; + /** Tags this run recorded as applied before the failure. */ + readonly adopted: readonly string[]; + + constructor( + pending: readonly string[], + code: string | undefined, + options: { cause?: unknown; adopted?: readonly string[] } = {}, + ) { + const adopted = options.adopted ?? []; + const sqlState = code ? ` (SQLSTATE ${code})` : ""; + const failure = + pending.length > 0 + ? `Applying migration${pending.length === 1 ? "" : "s"} ${pending.join(", ")} failed${sqlState}; none were applied.` + : `Reading the recorded migrations in ${MIGRATIONS_TABLE} failed${sqlState}; none were applied.`; + const kept = + adopted.length > 0 + ? ` ${adopted.join(", ")} stays recorded as applied: its tables already existed.` + : ""; + const hint = failureHint(code); + super(`${failure}${kept}${hint ? ` ${hint}` : ""}`, { + cause: options.cause, + }); + this.name = "MigrationApplyError"; + this.pending = Object.freeze([...pending]); + this.code = code; + this.adopted = Object.freeze([...adopted]); + } +} + +/** + * Nothing is recorded yet and the database holds only part of what + * 0000_baseline declares: neither running nor adopting the baseline would be safe. + */ +export class MigrationBaselineMismatchError extends Error { + readonly problems: readonly string[]; + + constructor(schemaName: string, problems: readonly string[]) { + super( + `Cannot adopt ${BASELINE_TAG}: schema "${schemaName}" has no recorded migrations and only part of the baseline: ${problems.join("; ")}. ` + + "Create the missing tables and columns (or drop the partial ones), or regenerate the baseline against this database.", + ); + this.name = "MigrationBaselineMismatchError"; + this.problems = Object.freeze([...problems]); + } +} + +const SQLSTATE = /^[0-9A-Z]{5}$/; + +/** drizzle wraps driver errors; the SQLSTATE lives somewhere on the cause chain. */ +function sqlStateOf(error: unknown): string | undefined { + let current = error; + for (let depth = 0; depth < 5 && current; depth++) { + const code = (current as { code?: unknown }).code; + // No SQLSTATE class starts with "E"; Node system errors (EPIPE, EPERM, …) all do. + if (typeof code === "string" && SQLSTATE.test(code) && code[0] !== "E") { + return code; + } + current = (current as { cause?: unknown }).cause; + } + return undefined; +} + +function quoteIdentifier(name: string): string { + return `"${name.replace(/"/g, '""')}"`; +} + +/** One lock per app schema, so concurrent instances apply in turn. */ +export function migrationLockKey(schemaName: string): string { + return createHash("sha256") + .update(`appkit:migrations:${schemaName}`) + .digest() + .readBigInt64BE(0) + .toString(); +} + +type QueryClient = Pick; + +/** + * `created_at` of the last migration the database recorded; null when none is. + * An empty journal table counts as nothing recorded: drizzle-orm creates it + * before its transaction, so a failed first deploy leaves one behind. + */ +async function lastRecordedMigration( + client: QueryClient, + schemaName: string, +): Promise { + // pg_catalog needs no privileges, so a missing grant is not mistaken for a missing table. + const table = await client.query( + `select 1 from pg_catalog.pg_class c + join pg_catalog.pg_namespace n on n.oid = c.relnamespace + where n.nspname = $1 and c.relname = $2`, + [schemaName, MIGRATIONS_TABLE], + ); + if (table.rows.length === 0) return null; + const last = await client.query<{ last: string | number | null }>( + `select max(created_at) as last from ${quoteIdentifier(schemaName)}.${quoteIdentifier(MIGRATIONS_TABLE)}`, + ); + const value = last.rows[0]?.last; + return value === null || value === undefined ? null : Number(value); +} + +/** + * Journal entries newer than the last recorded migration: drizzle-orm's own + * rule, so this agrees with what its migrator runs. + * + * Known gap, shared with drizzle-orm: an entry older than the last recorded one + * (e.g. merged from a parallel branch) is never applied, and an applied + * migration edited later goes unnoticed. Comparing the recorded hashes with the + * journal would catch both. + */ +function pendingEntries( + journal: Journal, + lastRecorded: number | null, +): JournalEntry[] { + if (lastRecorded === null) return journal.entries; + return journal.entries.filter((entry) => entry.when > lastRecorded); +} + +/** Fail before connecting, naming the file, rather than inside drizzle-orm's migrator. */ +function assertMigrationFiles(folder: string, journal: Journal): void { + for (const entry of journal.entries) { + const file = path.join(folder, `${entry.tag}.sql`); + if (!fs.existsSync(file)) { + throw new MigrationFilesError( + `Missing ${file}, listed in meta/_journal.json. Restore it from version control.`, + ); + } + } +} + +/** + * Run `work` on one dedicated connection under the per-schema advisory lock, + * with no statement timeout and at most LOCK_TIMEOUT per table lock. The + * connection is always unlocked, reset and released; a connection whose + * cleanup fails is destroyed, not reused. + */ +async function withMigrationLock( + pool: Pick, + schemaName: string, + work: (client: PoolClient) => Promise, +): Promise { + const lockKey = migrationLockKey(schemaName); + const client = await pool.connect(); + let locked = false; + try { + await client.query("SET statement_timeout = 0"); + const attempt = await client.query<{ locked: boolean }>( + "select pg_catalog.pg_try_advisory_lock($1::bigint) as locked", + [lockKey], + ); + if (!attempt.rows[0]?.locked) { + logger.info( + "Waiting for another instance to finish migrating schema %s", + schemaName, + ); + await client.query("select pg_catalog.pg_advisory_lock($1::bigint)", [ + lockKey, + ]); + } + locked = true; + // Only now: lock_timeout would also cut short the wait for the advisory lock. + await client.query(`SET lock_timeout = '${LOCK_TIMEOUT}'`); + return await work(client); + } finally { + const cleanup: Array<() => Promise> = []; + if (locked) { + cleanup.push(() => + client.query("select pg_catalog.pg_advisory_unlock($1::bigint)", [ + lockKey, + ]), + ); + } + cleanup.push(() => client.query("RESET lock_timeout")); + cleanup.push(() => client.query("RESET statement_timeout")); + + let cleanupError: Error | undefined; + for (const step of cleanup) { + try { + await step(); + } catch (error) { + cleanupError ??= + error instanceof Error ? error : new Error(String(error)); + } + } + // A connection that may still hold the lock or the session settings must not be reused. + client.release(cleanupError); + } +} + +async function inTransaction( + client: QueryClient, + work: () => Promise, +): Promise { + await client.query("begin"); + try { + await work(); + await client.query("commit"); + } catch (error) { + await client.query("rollback").catch(() => undefined); + throw error; + } +} + +/** The tables and columns a migration snapshot (meta/NNNN_snapshot.json) declares in one schema. */ +function snapshotTables( + folder: string, + idx: number, + schemaName: string, +): Map { + const file = path.join( + folder, + "meta", + `${String(idx).padStart(4, "0")}_snapshot.json`, + ); + if (!fs.existsSync(file)) { + throw new MigrationFilesError( + `Missing ${file} for ${BASELINE_TAG}. Restore it from version control; it lists the tables the baseline adopts.`, + ); + } + const snapshot = readJsonFile<{ + tables?: Record< + string, + { name: string; schema?: string; columns: Record } + >; + }>(file); + const tables = new Map(); + for (const table of Object.values(snapshot.tables ?? {})) { + if ((table.schema || "public") !== schemaName) continue; + tables.set(table.name, Object.keys(table.columns)); + } + return tables; +} + +type BaselineState = + | { readonly kind: "absent" } + | { readonly kind: "present" } + | { readonly kind: "partial"; readonly problems: readonly string[] }; + +/** Compare the baseline's tables and columns with pg_catalog. */ +async function baselineState( + client: QueryClient, + options: ApplyMigrationsOptions, + baseline: JournalEntry, +): Promise { + const declared = snapshotTables( + options.migrationsFolder, + baseline.idx, + options.schemaName, + ); + if (declared.size === 0) return { kind: "absent" }; + const { rows } = await client.query<{ + table_name: string; + column_name: string | null; + }>( + `select c.relname::text as table_name, a.attname::text as column_name + from pg_catalog.pg_class c + join pg_catalog.pg_namespace n on n.oid = c.relnamespace + left join pg_catalog.pg_attribute a + on a.attrelid = c.oid and a.attnum > 0 and not a.attisdropped + where n.nspname = $1 and c.relkind in ('r', 'p') + and c.relname = any($2::text[])`, + [options.schemaName, [...declared.keys()]], + ); + const found = new Map>(); + for (const row of rows) { + const columns = found.get(row.table_name) ?? new Set(); + if (row.column_name) columns.add(row.column_name); + found.set(row.table_name, columns); + } + if (found.size === 0) return { kind: "absent" }; + + const problems: string[] = []; + for (const [table, columns] of declared) { + const qualified = `${options.schemaName}.${table}`; + const present = found.get(table); + if (!present) { + problems.push(`table ${qualified} does not exist`); + continue; + } + const missing = columns.filter((column) => !present.has(column)); + if (missing.length > 0) { + problems.push( + `table ${qualified} is missing ${missing.length === 1 ? "column" : "columns"} ${missing.join(", ")}`, + ); + } + } + return problems.length > 0 + ? { kind: "partial", problems } + : { kind: "present" }; +} + +/** + * Record migrations as applied without running them: create the journal table + * as drizzle-orm's migrator does, then insert one row per tag with the + * migrator's own hash (sha256 of the .sql file) and `created_at` (the journal's + * `when`). Runs on the caller's connection; wrap it in a transaction. + */ +async function recordMigrationsApplied( + client: QueryClient, + options: ApplyMigrationsOptions, + journal: Journal, + tags: readonly string[], +): Promise { + // One file per journal entry, in journal order. + const files = readMigrationFiles({ + migrationsFolder: options.migrationsFolder, + }); + const schema = quoteIdentifier(options.schemaName); + const table = `${schema}.${quoteIdentifier(MIGRATIONS_TABLE)}`; + await client.query(`CREATE SCHEMA IF NOT EXISTS ${schema}`); + // drizzle-orm's own definition, so its migrator later reads and extends this table. + await client.query( + `CREATE TABLE IF NOT EXISTS ${table} ( + id SERIAL PRIMARY KEY, + hash text NOT NULL, + created_at bigint + )`, + ); + for (const tag of tags) { + const index = journal.entries.findIndex((entry) => entry.tag === tag); + const file = files[index]; + if (index < 0 || !file) { + throw new Error(`Migration ${tag} is not in the journal.`); + } + await client.query( + `insert into ${table} ("hash", "created_at") values($1, $2)`, + [file.hash, file.folderMillis], + ); + } +} + +/** + * Apply every committed migration the database has not recorded yet. + * + * Runs on one dedicated connection as the connecting identity, which owns what + * it creates (see ownership.ts), under a per-schema advisory lock; drizzle-orm's + * migrator runs only when something is pending. + * + * Baseline adoption: when nothing is recorded yet and `0000_baseline` comes + * first, the baseline's tables are looked up in pg_catalog. None present → it + * runs like any migration. All present → it is recorded as applied, in its own + * transaction, and only later migrations run. Anything in between → + * MigrationBaselineMismatchError, nothing changes. + */ +export async function applyPendingMigrations( + pool: Pick, + options: ApplyMigrationsOptions, +): Promise { + const journal = readJournal(options.migrationsFolder); + if (!journal || journal.entries.length === 0) { + return { applied: [], adopted: [] }; + } + assertMigrationFiles(options.migrationsFolder, journal); + + return withMigrationLock(pool, options.schemaName, async (client) => { + const adopted: string[] = []; + // What the failing step was applying, for the error. + let applying: readonly string[] = []; + try { + const lastRecorded = await lastRecordedMigration( + client, + options.schemaName, + ); + let pending = pendingEntries(journal, lastRecorded); + if (pending.length === 0) return { applied: [], adopted }; + + const first = journal.entries[0]; + if (lastRecorded === null && first?.tag === BASELINE_TAG) { + applying = [BASELINE_TAG]; + const state = await baselineState(client, options, first); + if (state.kind === "partial") { + throw new MigrationBaselineMismatchError( + options.schemaName, + state.problems, + ); + } + if (state.kind === "present") { + await inTransaction(client, () => + recordMigrationsApplied(client, options, journal, [BASELINE_TAG]), + ); + adopted.push(BASELINE_TAG); + pending = pending.filter((entry) => entry.tag !== BASELINE_TAG); + if (pending.length === 0) return { applied: [], adopted }; + } + } + + applying = pending.map((entry) => entry.tag); + await migrate(drizzle({ client }), { + migrationsFolder: options.migrationsFolder, + migrationsSchema: options.schemaName, + migrationsTable: MIGRATIONS_TABLE, + }); + return { applied: applying, adopted }; + } catch (error) { + if ( + error instanceof MigrationBaselineMismatchError || + error instanceof MigrationFilesError + ) { + throw error; + } + throw new MigrationApplyError(applying, sqlStateOf(error), { + cause: error, + adopted, + }); + } + }); +} diff --git a/packages/appkit/src/database/migrations/ownership.ts b/packages/appkit/src/database/migrations/ownership.ts new file mode 100644 index 000000000..606efb1d1 --- /dev/null +++ b/packages/appkit/src/database/migrations/ownership.ts @@ -0,0 +1,18 @@ +// Ownership model: each branch has one owner, the identity its migrations run +// as, and that identity owns the app's objects there. On the deploy branch it +// is the app's service principal, which creates the app schema on first start. +// There is no shared owner role and no SET ROLE. A shared owner role is not +// possible: Lakebase rejects `GRANT … TO databricks_superuser`. + +/** insufficient_privilege: "must be owner of …", "permission denied for …". */ +const INSUFFICIENT_PRIVILEGE = "42501"; + +/** How to fix a migration that failed for lack of privileges; undefined for any other SQLSTATE. */ +export function ownershipHint(code: string | undefined): string | undefined { + if (code !== INSUFFICIENT_PRIVILEGE) return undefined; + return ( + "The connecting role lacks a privilege this migration needs: CREATE on the database, or ownership of the app schema and its tables. " + + "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE, " + + "and if another role created the schema, use another schemaName in schema.ts or have that role transfer the schema and its objects." + ); +} diff --git a/packages/appkit/src/database/migrations/paths.ts b/packages/appkit/src/database/migrations/paths.ts new file mode 100644 index 000000000..e331080b0 --- /dev/null +++ b/packages/appkit/src/database/migrations/paths.ts @@ -0,0 +1,72 @@ +import fs from "node:fs"; +import path from "node:path"; + +/** Committed migration history, relative to the application root. */ +const MIGRATIONS_DIR = "config/database/migrations"; +/** Journal table drizzle-orm's migrator keeps in the app schema. */ +export const MIGRATIONS_TABLE = "__drizzle_migrations"; +/** + * First migration `appkit db introspect` writes for tables that already exist. + * The migrator adopts it (records it without running it) when the catalog + * already holds every one of its tables and columns. + */ +export const BASELINE_TAG = "0000_baseline"; + +/** One entry of drizzle-kit's v7 `meta/_journal.json`. */ +export interface JournalEntry { + idx: number; + version: "7"; + when: number; + tag: string; + breakpoints: true; +} + +/** drizzle-kit's v7 `meta/_journal.json`. */ +export interface Journal { + version: "7"; + dialect: "postgresql"; + entries: JournalEntry[]; +} + +/** Absolute migrations folder for an application root. */ +export function migrationsFolder(root: string): string { + return path.resolve(root, MIGRATIONS_DIR); +} + +/** Path of the journal inside a migrations folder. */ +function journalFile(folder: string): string { + return path.join(folder, "meta", "_journal.json"); +} + +/** A committed migration file is missing or unreadable; the message names the file. */ +export class MigrationFilesError extends Error { + constructor(message: string, options?: { cause?: unknown }) { + super(message, options); + this.name = "MigrationFilesError"; + } +} + +/** Parse a committed JSON file of the migration history. */ +export function readJsonFile(file: string): T { + try { + return JSON.parse(fs.readFileSync(file, "utf-8")) as T; + } catch (error) { + throw new MigrationFilesError( + `Cannot read ${file}: ${error instanceof Error ? error.message : String(error)}. Restore it from version control.`, + { cause: error }, + ); + } +} + +/** null when meta/_journal.json is absent (nothing committed yet). */ +export function readJournal(folder: string): Journal | null { + const file = journalFile(folder); + if (!fs.existsSync(file)) return null; + const parsed = readJsonFile>(file); + if (parsed.dialect !== "postgresql" || !Array.isArray(parsed.entries)) { + throw new MigrationFilesError( + `${file} is not a drizzle-kit PostgreSQL migration journal`, + ); + } + return parsed as Journal; +} diff --git a/packages/appkit/src/database/migrations/tests/apply.test.ts b/packages/appkit/src/database/migrations/tests/apply.test.ts new file mode 100644 index 000000000..4fc178d14 --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/apply.test.ts @@ -0,0 +1,832 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, test, vi } from "vitest"; + +import { + applyPendingMigrations, + MigrationApplyError, + MigrationBaselineMismatchError, + migrationLockKey, +} from "../apply"; +import { type Journal, MigrationFilesError } from "../paths"; + +const folders: string[] = []; + +/** Write a v7 journal with one SQL file per entry, the way drizzle-kit lays them out. */ +function migrations( + entries: Array<{ + tag: string; + when: number; + sql: string[]; + /** Tables (name → columns) for meta/NNNN_snapshot.json. */ + snapshot?: Record; + }>, +) { + const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); + folders.push(folder); + fs.mkdirSync(path.join(folder, "meta")); + const journal: Journal = { + version: "7", + dialect: "postgresql", + entries: entries.map((entry, idx) => ({ + idx, + version: "7", + when: entry.when, + tag: entry.tag, + breakpoints: true, + })), + }; + fs.writeFileSync( + path.join(folder, "meta/_journal.json"), + JSON.stringify(journal), + ); + entries.forEach((entry, idx) => { + fs.writeFileSync( + path.join(folder, `${entry.tag}.sql`), + entry.sql.join("--> statement-breakpoint\n"), + ); + if (entry.snapshot) { + const tables = Object.fromEntries( + Object.entries(entry.snapshot).map(([name, columns]) => [ + `app.${name}`, + { + name, + schema: "app", + columns: Object.fromEntries(columns.map((c) => [c, { name: c }])), + }, + ]), + ); + fs.writeFileSync( + path.join( + folder, + "meta", + `${String(idx).padStart(4, "0")}_snapshot.json`, + ), + JSON.stringify({ + version: "7", + dialect: "postgresql", + tables, + enums: {}, + }), + ); + } + }); + return folder; +} + +afterEach(() => { + vi.restoreAllMocks(); + for (const folder of folders.splice(0)) { + fs.rmSync(folder, { recursive: true, force: true }); + } +}); + +interface FakeState { + journalTableExists: boolean; + lastCreatedAt: number | null; + failOn?: RegExp; + /** SQLSTATE of the failOn error; defaults to 42P07 (duplicate_table). */ + failCode?: string; + failCleanupOn?: RegExp; + /** pg_catalog (table, column) pairs the baseline check sees. */ + catalog?: Array<[string, string]>; + /** Another instance holds the migration lock, so the first try fails. */ + lockHeld?: boolean; +} + +/** + * A pg PoolClient that records every statement. Its class name must not contain + * "Pool": drizzle-orm would take it for a pool and call connect() on it. + */ +class RecordingClient { + readonly statements: string[] = []; + /** Parameters of each journal insert. */ + readonly recorded: unknown[][] = []; + released: { error: unknown } | undefined; + + constructor(private readonly state: FakeState) {} + + async query( + input: string | { text: string; values?: unknown[] }, + values?: unknown[], + ) { + const text = (typeof input === "string" ? input : input.text).trim(); + const params = values ?? (typeof input === "string" ? [] : input.values); + this.statements.push(text); + if (this.state.failOn?.test(text)) { + throw Object.assign(new Error("relation already exists"), { + code: this.state.failCode ?? "42P07", + }); + } + if (this.state.failCleanupOn?.test(text)) { + throw new Error("connection lost"); + } + if (text.includes("pg_try_advisory_lock(")) { + return { rows: [{ locked: !this.state.lockHeld }] }; + } + if (text.includes("pg_catalog.pg_attribute")) { + return { + rows: (this.state.catalog ?? []).map(([table_name, column_name]) => ({ + table_name, + column_name, + })), + }; + } + if (text.includes("pg_catalog.pg_class")) { + return { rows: this.state.journalTableExists ? [{ one: 1 }] : [] }; + } + if (text.startsWith(`insert into "app"."__drizzle_migrations"`)) { + this.recorded.push(params ?? []); + this.state.journalTableExists = true; + this.state.lastCreatedAt = Number(params?.[1]); + } + if (text.startsWith("select max(created_at)")) { + return { rows: [{ last: this.state.lastCreatedAt }] }; + } + if (text.includes("order by created_at desc limit 1")) { + return { + rows: + this.state.lastCreatedAt === null + ? [] + : [{ id: 1, hash: "h", created_at: this.state.lastCreatedAt }], + }; + } + return { rows: [] }; + } + + release(error?: unknown) { + this.released = { error }; + } +} + +function fakePool(state: FakeState) { + const client = new RecordingClient(state); + let connects = 0; + return { + client, + connects: () => connects, + pool: { + connect: async () => { + connects++; + return client as never; + }, + }, + }; +} + +const twoMigrations = () => + migrations([ + { + tag: "0000_migration", + when: 1000, + sql: [`CREATE TABLE "app"."notes" ("id" integer);`], + }, + { + tag: "0001_migration", + when: 2000, + sql: [ + `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + ], + }, + ]); + +/** Collapse the recorded statements into readable steps. */ +function steps(statements: readonly string[]): string[] { + return statements.map((statement) => { + if (statement.includes("pg_try_advisory_lock(")) return "try lock"; + if (statement.includes("pg_advisory_lock(")) return "lock"; + if (statement.includes("pg_advisory_unlock(")) return "unlock"; + if (statement.includes("pg_catalog.pg_attribute")) + return "baseline tables?"; + if (statement.includes("pg_catalog.pg_class")) return "journal table?"; + if (statement.startsWith("select max(created_at)")) return "last applied?"; + return statement.replace(/\s+/g, " "); + }); +} + +describe("applyPendingMigrations", () => { + test("applies every pending migration as the connecting identity under the advisory lock, then unlocks and releases", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result.applied).toEqual(["0000_migration", "0001_migration"]); + expect(steps(client.statements)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "SET lock_timeout = '30s'", + "journal table?", + // drizzle-orm's migrator as the connecting identity (no SET ROLE), journal in the app schema: + `CREATE SCHEMA IF NOT EXISTS "app"`, + `CREATE TABLE IF NOT EXISTS "app"."__drizzle_migrations" ( id SERIAL PRIMARY KEY, hash text NOT NULL, created_at bigint )`, + `select id, hash, created_at from "app"."__drizzle_migrations" order by created_at desc limit 1`, + "begin", + `CREATE TABLE "app"."notes" ("id" integer);`, + `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + "commit", + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("applies only entries newer than the last recorded migration", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result.applied).toEqual(["0001_migration"]); + expect(client.statements).not.toContain( + `CREATE TABLE "app"."notes" ("id" integer);`, + ); + expect(client.statements).toContain( + `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, + ); + }); + + test("with nothing pending, issues no migration SQL", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 2000, + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result.applied).toEqual([]); + expect(steps(client.statements)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "SET lock_timeout = '30s'", + "journal table?", + "last applied?", + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("without a journal it never connects", async () => { + const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); + folders.push(folder); + const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); + + await expect( + applyPendingMigrations(fake.pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).resolves.toEqual({ applied: [], adopted: [] }); + expect(fake.connects()).toBe(0); + }); + + test("a failing statement rolls back all pending migrations and still unlocks and releases", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + failOn: /ADD COLUMN "title"/, + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ + pending: ["0000_migration", "0001_migration"], + code: "42P07", + }); + expect((error as Error).message).toBe( + "Applying migrations 0000_migration, 0001_migration failed (SQLSTATE 42P07); none were applied.", + ); + expect(steps(client.statements).slice(-5)).toEqual([ + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + "rollback", + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("an ownership failure (42501) names the branch-owner rule and the fix", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + failOn: /ADD COLUMN "body"/, + failCode: "42501", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toMatchObject({ code: "42501" }); + expect((error as Error).message).toMatch( + /^Applying migrations 0000_migration, 0001_migration failed \(SQLSTATE 42501\); none were applied\. The connecting role lacks a privilege this migration needs: /, + ); + }); + + test("a failed unlock destroys the connection instead of returning it to the pool", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + failCleanupOn: /pg_advisory_unlock/, + }); + + await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + // The remaining cleanup still runs. + expect(steps(client.statements).slice(-3)).toEqual([ + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released?.error).toBeInstanceOf(Error); + }); + + test("a failure before the lock is held releases without unlocking", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + failOn: /pg_try_advisory_lock/, + }); + + await expect( + applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).rejects.toThrow("relation already exists"); + expect(steps(client.statements)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); +}); + +describe("migrationLockKey", () => { + test("is a stable signed 64-bit key per schema", () => { + const key = migrationLockKey("app"); + expect(key).toBe(migrationLockKey("app")); + expect(key).not.toBe(migrationLockKey("public")); + const value = BigInt(key); + expect(value).toBeGreaterThanOrEqual(-(2n ** 63n)); + expect(value).toBeLessThan(2n ** 63n); + }); +}); + +const BASELINE_SQL = [ + `CREATE TABLE "app"."notes" ("id" integer PRIMARY KEY, "body" text);`, + `CREATE TABLE "app"."tags" ("id" integer PRIMARY KEY);`, +]; + +/** 0000_baseline (notes, tags) followed by one ordinary migration. */ +const baselineThenMigration = () => + migrations([ + { + tag: "0000_baseline", + when: 1000, + sql: BASELINE_SQL, + snapshot: { notes: ["id", "body"], tags: ["id"] }, + }, + { + tag: "0001_migration", + when: 2000, + sql: [`ALTER TABLE "app"."notes" ADD COLUMN "title" text;`], + snapshot: { notes: ["id", "body", "title"], tags: ["id"] }, + }, + ]); + +describe("applyPendingMigrations — baseline adoption", () => { + test("none of the baseline's tables exist: the baseline runs like any migration", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [], + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result).toEqual({ + applied: ["0000_baseline", "0001_migration"], + adopted: [], + }); + expect(client.statements).toContain(BASELINE_SQL[0]); + }); + + test("every baseline table and column exists: the baseline is recorded, not run, and later migrations apply", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + }); + + const result = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(result).toEqual({ + applied: ["0001_migration"], + adopted: ["0000_baseline"], + }); + expect(client.statements).not.toContain(BASELINE_SQL[0]); + expect(client.statements).toContain( + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + ); + // The adopted row carries exactly what drizzle-orm's migrator would have written. + const baselineFile = fs.readFileSync( + path.join(folder, "0000_baseline.sql"), + "utf-8", + ); + expect(client.recorded[0]).toEqual([ + createHash("sha256").update(baselineFile).digest("hex"), + 1000, + ]); + expect(steps(client.statements).slice(0, 10)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "SET lock_timeout = '30s'", + "journal table?", + "baseline tables?", + "begin", + `CREATE SCHEMA IF NOT EXISTS "app"`, + `CREATE TABLE IF NOT EXISTS "app"."__drizzle_migrations" ( id SERIAL PRIMARY KEY, hash text NOT NULL, created_at bigint )`, + `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + "commit", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("adopting a lone baseline applies nothing else", async () => { + const folder = migrations([ + { + tag: "0000_baseline", + when: 1000, + sql: BASELINE_SQL, + snapshot: { notes: ["id", "body"], tags: ["id"] }, + }, + ]); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + }); + + await expect( + applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).resolves.toEqual({ applied: [], adopted: ["0000_baseline"] }); + expect(client.statements.slice(-3)).toEqual([ + expect.stringContaining("pg_advisory_unlock"), + "RESET lock_timeout", + "RESET statement_timeout", + ]); + }); + + test("only part of the baseline exists: MigrationBaselineMismatchError, nothing recorded or run", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [["notes", "id"]], + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationBaselineMismatchError); + expect((error as MigrationBaselineMismatchError).problems).toEqual([ + "table app.notes is missing column body", + "table app.tags does not exist", + ]); + expect((error as Error).message).toMatch( + /^Cannot adopt 0000_baseline: schema "app" has no recorded migrations and only part of the baseline: /, + ); + expect(client.recorded).toEqual([]); + expect(client.statements).not.toContain("begin"); + expect(steps(client.statements).slice(-3)).toEqual([ + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + expect(client.released).toEqual({ error: undefined }); + }); + + test("recording the adopted baseline fails: MigrationApplyError with the SQLSTATE and the ownership hint", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + // The schema belongs to another role, so the journal table cannot be created in it. + failOn: /^CREATE TABLE IF NOT EXISTS "app"\."__drizzle_migrations"/, + failCode: "42501", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ pending: ["0000_baseline"], code: "42501" }); + expect((error as Error).message).toContain( + "On the deploy branch that role is the app's service principal", + ); + expect(client.statements).toContain("rollback"); + expect(client.statements).not.toContain( + `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + ); + }); + + test("with a recorded migration, the baseline is never adopted", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + catalog: [["notes", "id"]], + }); + + await expect( + applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).resolves.toEqual({ applied: ["0001_migration"], adopted: [] }); + expect( + client.statements.some((s) => s.includes("pg_catalog.pg_attribute")), + ).toBe(false); + }); + + test("an empty journal table, left by a failed first deploy, still adopts the baseline", async () => { + const folder = baselineThenMigration(); + const { pool } = fakePool({ + journalTableExists: true, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + }); + + await expect( + applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }), + ).resolves.toEqual({ + applied: ["0001_migration"], + adopted: ["0000_baseline"], + }); + }); + + test("a migration failing after the adoption says the baseline stays recorded", async () => { + const folder = baselineThenMigration(); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + catalog: [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], + ], + failOn: /ADD COLUMN "title"/, + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ + pending: ["0001_migration"], + adopted: ["0000_baseline"], + }); + expect((error as Error).message).toBe( + "Applying migration 0001_migration failed (SQLSTATE 42P07); none were applied. 0000_baseline stays recorded as applied: its tables already existed.", + ); + expect(client.recorded).toHaveLength(1); + }); + + test("a missing baseline snapshot names the file", async () => { + const folder = baselineThenMigration(); + fs.rmSync(path.join(folder, "meta/0000_snapshot.json")); + const { pool, client } = fakePool({ + journalTableExists: false, + lastCreatedAt: null, + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch( + /^Missing .*meta\/0000_snapshot\.json for 0000_baseline\. Restore it from version control/, + ); + expect(client.released).toEqual({ error: undefined }); + }); +}); + +describe("applyPendingMigrations — failures name their cause", () => { + test("reading the recorded migrations fails (42501): the hint, and nothing applied", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + // The journal table belongs to another role. + failOn: /^select max\(created_at\)/, + failCode: "42501", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ pending: [], code: "42501" }); + expect((error as Error).message).toMatch( + /^Reading the recorded migrations in __drizzle_migrations failed \(SQLSTATE 42501\); none were applied\. The connecting role lacks a privilege/, + ); + expect(steps(client.statements).slice(-3)).toEqual([ + "unlock", + "RESET lock_timeout", + "RESET statement_timeout", + ]); + }); + + test("a table lock held past the lock timeout says to deploy again", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + failOn: /ADD COLUMN "body"/, + failCode: "55P03", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect((error as Error).message).toBe( + "Applying migration 0001_migration failed (SQLSTATE 55P03); none were applied. Another session held a lock on a table this migration changes for over 30s; deploy again once it finishes.", + ); + }); + + test("a Node system error is not reported as a SQLSTATE", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + journalTableExists: true, + lastCreatedAt: 1000, + failOn: /ADD COLUMN "body"/, + failCode: "EPIPE", + }); + + const error = await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toMatchObject({ code: undefined }); + expect((error as Error).message).toBe( + "Applying migration 0001_migration failed; none were applied.", + ); + }); + + test("a missing .sql file fails before connecting, naming the file", async () => { + const folder = twoMigrations(); + fs.rmSync(path.join(folder, "0001_migration.sql")); + const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); + + const error = await applyPendingMigrations(fake.pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch( + /^Missing .*0001_migration\.sql, listed in meta\/_journal\.json\. Restore it from version control\.$/, + ); + expect(fake.connects()).toBe(0); + }); + + test.each([ + [ + "not JSON", + "{", + /^Cannot read .*_journal\.json: .*Restore it from version control\.$/, + ], + [ + "another dialect", + JSON.stringify({ version: "7", dialect: "mysql", entries: [] }), + /_journal\.json is not a drizzle-kit PostgreSQL migration journal$/, + ], + ])( + "a journal that is %s fails before connecting, naming the file", + async (_label, contents, message) => { + const folder = twoMigrations(); + fs.writeFileSync(path.join(folder, "meta/_journal.json"), contents); + const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); + + const error = await applyPendingMigrations(fake.pool, { + migrationsFolder: folder, + schemaName: "app", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch(message); + expect(fake.connects()).toBe(0); + }, + ); + + test("waits for another instance's lock, and says so", async () => { + const folder = twoMigrations(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const { pool, client } = fakePool({ + journalTableExists: true, + lastCreatedAt: 2000, + lockHeld: true, + }); + + await applyPendingMigrations(pool, { + migrationsFolder: folder, + schemaName: "app", + }); + + expect(steps(client.statements).slice(0, 4)).toEqual([ + "SET statement_timeout = 0", + "try lock", + "lock", + "SET lock_timeout = '30s'", + ]); + expect(log.mock.calls.flat().map(String).join(" ")).toContain( + "Waiting for another instance to finish migrating schema app", + ); + }); +}); diff --git a/packages/appkit/src/database/migrations/tests/ownership.test.ts b/packages/appkit/src/database/migrations/tests/ownership.test.ts new file mode 100644 index 000000000..56ea47022 --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/ownership.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, test } from "vitest"; + +import { ownershipHint } from "../ownership"; + +describe("ownership: one owner per branch", () => { + test("insufficient_privilege gets the branch-owner rule and the fix", () => { + const hint = ownershipHint("42501"); + expect(hint).toContain( + "lacks a privilege this migration needs: CREATE on the database, or ownership of the app schema and its tables", + ); + expect(hint).toContain( + "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE", + ); + expect(hint).toContain("use another schemaName in schema.ts"); + }); + + test("any other failure gets no ownership hint", () => { + expect(ownershipHint("42P07")).toBeUndefined(); + expect(ownershipHint(undefined)).toBeUndefined(); + }); +}); diff --git a/packages/appkit/src/plugins/database/lifecycle.ts b/packages/appkit/src/plugins/database/lifecycle.ts index c50badf6f..aff2abc08 100644 --- a/packages/appkit/src/plugins/database/lifecycle.ts +++ b/packages/appkit/src/plugins/database/lifecycle.ts @@ -23,6 +23,7 @@ import type { SqlTag, TransactionClient, } from "./entity-types"; +import { migrateOnStart, shouldMigrateOnStart } from "./migrate-on-start"; import { assertSchemaMatchesDatabase } from "./schema-check"; import { createMutationScope, type MutationScope } from "./scope"; import type { DatabaseHooks } from "./types"; @@ -163,8 +164,9 @@ function buildDatabaseExports(context: ExportContext): DatabaseExports { } /** - * Validate the schema, create one pool-backed API, and verify connectivity and - * that every declared table and column exists. + * Validate the schema, create one pool-backed API, apply committed migrations + * when deployed, and verify connectivity and that every declared table and + * column exists. */ export async function createDatabaseState( schema: TSchema, @@ -188,6 +190,8 @@ export async function createDatabaseState( statement_timeout: STATEMENT_TIMEOUT_MS, idle_in_transaction_session_timeout: IDLE_IN_TRANSACTION_TIMEOUT_MS, }); + // A deployed app applies committed migrations as its own identity before the checks below. + if (shouldMigrateOnStart()) await migrateOnStart(pool, schema); const db = createDrizzleDb(pool, schema); const dataPath = createDrizzleDataPath(db, schema, { columnAccess: "trusted", diff --git a/packages/appkit/src/plugins/database/migrate-on-start.ts b/packages/appkit/src/plugins/database/migrate-on-start.ts new file mode 100644 index 000000000..8061c1124 --- /dev/null +++ b/packages/appkit/src/plugins/database/migrate-on-start.ts @@ -0,0 +1,78 @@ +import type { Pool } from "pg"; + +import { databaseSetupFailed } from "../../database/errors"; +import { + applyPendingMigrations, + MigrationApplyError, + MigrationBaselineMismatchError, +} from "../../database/migrations/apply"; +import { + MigrationFilesError, + migrationsFolder, +} from "../../database/migrations/paths"; +import type { Schema } from "../../database/schema-builder"; +import { createLogger } from "../../logging/logger"; + +const logger = createLogger("database"); + +/** + * True only inside a deployed Databricks App: not local development, and the + * runtime has set the app's name and its service principal's credentials, as + * Databricks Apps does for every app. `databricks apps run-local` sets the + * name but not the credentials. Locally, `appkit db migrate` applies + * migrations instead. + */ +export function shouldMigrateOnStart( + env: NodeJS.ProcessEnv = process.env, +): boolean { + return ( + env.NODE_ENV !== "development" && + Boolean(env.DATABRICKS_APP_NAME) && + Boolean(env.DATABRICKS_CLIENT_ID) && + Boolean(env.DATABRICKS_CLIENT_SECRET) + ); +} + +/** + * Apply pending committed migrations from /config/database/migrations as + * the app's identity (see database/migrations/ownership.ts). No journal → + * no-op. A failed or unadoptable migration, or a missing or unreadable + * migration file, becomes a setup failure that names it; any other error + * (e.g. no connection) is left to lifecycle's generic sanitizer. + */ +export async function migrateOnStart( + pool: Pick, + schema: Schema, + root: string = process.cwd(), +): Promise { + let result: Awaited>; + try { + result = await applyPendingMigrations(pool, { + migrationsFolder: migrationsFolder(root), + schemaName: schema.$schemaName, + }); + } catch (error) { + if ( + error instanceof MigrationApplyError || + error instanceof MigrationBaselineMismatchError || + error instanceof MigrationFilesError + ) { + throw databaseSetupFailed(error.message); + } + throw error; + } + if (result.adopted.length > 0) { + logger.info( + "Adopted %s in schema %s: its tables already existed", + result.adopted.join(", "), + schema.$schemaName, + ); + } + if (result.applied.length > 0) { + logger.info( + "Applied migrations %s to schema %s", + result.applied.join(", "), + schema.$schemaName, + ); + } +} diff --git a/packages/appkit/src/plugins/database/schema-check.ts b/packages/appkit/src/plugins/database/schema-check.ts index f1334ecda..9b6f7176a 100644 --- a/packages/appkit/src/plugins/database/schema-check.ts +++ b/packages/appkit/src/plugins/database/schema-check.ts @@ -10,10 +10,10 @@ interface CatalogColumn { /** * Confirm every declared table and column exists before routes are published. - * The plugin never migrates, so drift would otherwise surface on each request - * as an undefined-table or undefined-column failure. pg_catalog is read rather - * than information_schema, which hides objects the role cannot access and - * would report a privilege gap as a missing column. + * Even after a deployed app applies its migrations, drift would surface on each + * request as an undefined-table or undefined-column failure, so check up front. + * pg_catalog is read rather than information_schema, which hides objects the + * role cannot access and would report a privilege gap as a missing column. */ export async function assertSchemaMatchesDatabase( dataPath: DataPath, @@ -76,7 +76,7 @@ export async function assertSchemaMatchesDatabase( } if (problems.length > 0) { throw databaseSetupFailed( - `the declared schema does not match the database: ${problems.join("; ")}. Create or migrate these before starting the app; the plugin does not.`, + `the declared schema does not match the database: ${problems.join("; ")}. Create them, or commit migrations under config/database/migrations so the deployed app applies them on start.`, ); } } diff --git a/packages/appkit/src/plugins/database/tests/lifecycle.test.ts b/packages/appkit/src/plugins/database/tests/lifecycle.test.ts index aae7c04d2..0401912f6 100644 --- a/packages/appkit/src/plugins/database/tests/lifecycle.test.ts +++ b/packages/appkit/src/plugins/database/tests/lifecycle.test.ts @@ -9,6 +9,8 @@ const mocks = vi.hoisted(() => ({ createDrizzleDb: vi.fn(), createDrizzleDataPath: vi.fn(), assertSchemaMatchesDatabase: vi.fn(), + shouldMigrateOnStart: vi.fn(() => false), + migrateOnStart: vi.fn(async () => undefined), })); vi.mock("../../../connectors/lakebase", () => ({ initializeLakebasePool: mocks.initializeLakebasePool, @@ -21,6 +23,11 @@ vi.mock("../../../database/runtime/engine/drizzle-data-path", () => ({ vi.mock("../schema-check", () => ({ assertSchemaMatchesDatabase: mocks.assertSchemaMatchesDatabase, })); +// The migrator has its own suite; here only its place in setup matters. +vi.mock("../migrate-on-start", () => ({ + shouldMigrateOnStart: mocks.shouldMigrateOnStart, + migrateOnStart: mocks.migrateOnStart, +})); import { IDLE_IN_TRANSACTION_TIMEOUT_MS, @@ -94,7 +101,11 @@ function arrange(path = fakePath()) { return { pool, db, path, execute }; } -beforeEach(() => vi.clearAllMocks()); +beforeEach(() => { + vi.clearAllMocks(); + // clearAllMocks keeps implementations; a test that turns migration on must not leak it. + mocks.shouldMigrateOnStart.mockReturnValue(false); +}); afterEach(() => vi.restoreAllMocks()); describe("createDatabaseState", () => { @@ -216,6 +227,90 @@ describe("createDatabaseState", () => { expect(pool.end).toHaveBeenCalledTimes(1); }); + test("a deployed app applies migrations after the pool opens, before readiness and the catalog check", async () => { + const order: string[] = []; + const path = fakePath({ + raw: vi.fn(async () => { + order.push("select 1"); + return []; + }) as unknown as DataPath["raw"], + }); + const { pool, execute } = arrange(path); + mocks.initializeLakebasePool.mockImplementationOnce(async () => { + order.push("pool"); + return pool; + }); + mocks.shouldMigrateOnStart.mockReturnValue(true); + mocks.migrateOnStart.mockImplementationOnce(async () => { + order.push("migrate"); + }); + mocks.assertSchemaMatchesDatabase.mockImplementationOnce(async () => { + order.push("catalog"); + }); + + await createDatabaseState(schema, execute); + + expect(order).toEqual(["pool", "migrate", "select 1", "catalog"]); + expect(mocks.migrateOnStart).toHaveBeenCalledWith(pool, schema); + }); + + test("local runs never call the migrator", async () => { + const { execute } = arrange(); + mocks.shouldMigrateOnStart.mockReturnValue(false); + + await createDatabaseState(schema, execute); + + expect(mocks.migrateOnStart).not.toHaveBeenCalled(); + }); + + test("a migration failure keeps its reason, ends the pool and publishes nothing", async () => { + const { pool, path, execute } = arrange(); + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + mocks.shouldMigrateOnStart.mockReturnValue(true); + const failure = new DatabasePluginError( + "SETUP_FAILED", + "setup", + "Database setup failed: Applying migration 0001_migration failed (SQLSTATE 42501); none were applied.", + ); + mocks.migrateOnStart.mockRejectedValueOnce(failure); + + const error = await createDatabaseState(schema, execute).catch( + (caught) => caught, + ); + + expect(error).toBe(failure); + expect(errorLog.mock.calls.flat().map(String).join(" ")).toContain( + "0001_migration failed (SQLSTATE 42501)", + ); + expect(pool.end).toHaveBeenCalledTimes(1); + expect(path.raw).not.toHaveBeenCalled(); + expect(mocks.assertSchemaMatchesDatabase).not.toHaveBeenCalled(); + }); + + test("an unexpected migrator error is sanitized like any driver error", async () => { + const { pool, execute } = arrange(); + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + mocks.shouldMigrateOnStart.mockReturnValue(true); + mocks.migrateOnStart.mockRejectedValueOnce( + new Error("secret connection string"), + ); + + const error = await createDatabaseState(schema, execute).catch( + (caught) => caught, + ); + + expect(error).toMatchObject({ category: "SETUP_FAILED", phase: "setup" }); + expect(error.message).toBe("Database setup failed"); + expect(errorLog.mock.calls.flat().map(String).join(" ")).not.toContain( + "secret connection string", + ); + expect(pool.end).toHaveBeenCalledTimes(1); + }); + test("sanitizes connector initialization failures", async () => { const { execute } = arrange(); const errorLog = vi diff --git a/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts new file mode 100644 index 000000000..cf453a32d --- /dev/null +++ b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts @@ -0,0 +1,187 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +const { applyPendingMigrations } = vi.hoisted(() => ({ + applyPendingMigrations: vi.fn(), +})); +vi.mock("../../../database/migrations/apply", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../database/migrations/apply") + >()), + applyPendingMigrations, +})); + +import { DatabasePluginError } from "../../../database/errors"; +import * as apply from "../../../database/migrations/apply"; +import { MigrationFilesError } from "../../../database/migrations/paths"; +import { defineSchema, id } from "../../../database/schema-builder"; +import { migrateOnStart, shouldMigrateOnStart } from "../migrate-on-start"; + +const schema = defineSchema( + ({ table }) => ({ notes: table("notes", { id: id() }) }), + { schemaName: "app" }, +); +const roots: string[] = []; +const pool = { connect: vi.fn() }; + +function appRoot(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-migrate-start-")); + roots.push(root); + return root; +} + +beforeEach(async () => { + const actual = await vi.importActual< + typeof import("../../../database/migrations/apply") + >("../../../database/migrations/apply"); + applyPendingMigrations.mockReset(); + applyPendingMigrations.mockImplementation(actual.applyPendingMigrations); + pool.connect.mockReset(); +}); + +afterEach(() => { + vi.restoreAllMocks(); + for (const root of roots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +describe("shouldMigrateOnStart", () => { + const credentials = { + DATABRICKS_CLIENT_ID: "sp-client-id", + DATABRICKS_CLIENT_SECRET: "secret", + }; + const app = { DATABRICKS_APP_NAME: "my-app", ...credentials }; + + test.each([ + ["a deployed app", { ...app, NODE_ENV: "production" }, true], + ["a deployed app without NODE_ENV", app, true], + [ + "local development, even with app credentials", + { ...app, NODE_ENV: "development" }, + false, + ], + [ + "`databricks apps run-local`: the app name without its credentials", + { DATABRICKS_APP_NAME: "my-app", NODE_ENV: "production" }, + false, + ], + [ + "a local production start with app credentials in .env but no app name", + { ...credentials, NODE_ENV: "production" }, + false, + ], + [ + "no client secret (a developer's own credentials)", + { + DATABRICKS_APP_NAME: "my-app", + DATABRICKS_CLIENT_ID: "x", + NODE_ENV: "production", + }, + false, + ], + [ + "no client id", + { + DATABRICKS_APP_NAME: "my-app", + DATABRICKS_CLIENT_SECRET: "x", + NODE_ENV: "production", + }, + false, + ], + ["nothing set", {}, false], + ] as const)("%s → %s", (_label, env, expected) => { + expect(shouldMigrateOnStart(env)).toBe(expected); + }); +}); + +describe("migrateOnStart", () => { + test("no committed journal: a no-op that never connects", async () => { + await migrateOnStart(pool, schema, appRoot()); + + expect(pool.connect).not.toHaveBeenCalled(); + }); + + test("applies from /config/database/migrations into the schema's own journal, and logs what it did", async () => { + const root = appRoot(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + applyPendingMigrations.mockResolvedValueOnce({ + applied: ["0001_migration"], + adopted: ["0000_baseline"], + }); + + await migrateOnStart(pool, schema, root); + + expect(applyPendingMigrations).toHaveBeenCalledWith(pool, { + migrationsFolder: path.join(root, "config/database/migrations"), + schemaName: "app", + }); + const logged = log.mock.calls.map((call) => call.map(String).join(" ")); + expect(logged).toEqual([ + expect.stringContaining( + "Adopted 0000_baseline in schema app: its tables already existed", + ), + expect.stringContaining( + "Applied migrations 0001_migration to schema app", + ), + ]); + }); + + test("nothing pending logs nothing", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + applyPendingMigrations.mockResolvedValueOnce({ applied: [], adopted: [] }); + + await migrateOnStart(pool, schema, appRoot()); + + expect(log).not.toHaveBeenCalled(); + }); + + test.each([ + [ + "a failed migration", + new apply.MigrationApplyError(["0001_migration"], "42501"), + "Applying migration 0001_migration failed (SQLSTATE 42501); none were applied.", + ], + [ + "a baseline that only partly matches", + new apply.MigrationBaselineMismatchError("app", [ + "table app.tags does not exist", + ]), + "Cannot adopt 0000_baseline", + ], + [ + "a missing migration file", + new MigrationFilesError( + "Missing /app/config/database/migrations/0001_migration.sql, listed in meta/_journal.json. Restore it from version control.", + ), + "Missing /app/config/database/migrations/0001_migration.sql", + ], + ])( + "%s becomes a setup failure that names it", + async (_label, failure, reason) => { + applyPendingMigrations.mockRejectedValueOnce(failure); + + const error = await migrateOnStart(pool, schema, appRoot()).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(DatabasePluginError); + expect(error).toMatchObject({ category: "SETUP_FAILED", phase: "setup" }); + expect((error as Error).message).toContain(reason); + // Only the stable message crosses a request boundary. + expect((error as DatabasePluginError).clientMessage).toBe( + "Database setup failed", + ); + }, + ); + + test("any other error is left to lifecycle's sanitizer", async () => { + const raw = new Error("connection reset"); + applyPendingMigrations.mockRejectedValueOnce(raw); + + await expect(migrateOnStart(pool, schema, appRoot())).rejects.toBe(raw); + }); +}); From 19adf06c5f575f1917401926c7cc2733c81ac31b Mon Sep 17 00:00:00 2001 From: ditadi Date: Mon, 19 Oct 2026 14:07:23 +0200 Subject: [PATCH 2/2] fix(appkit): apply each migration in its own transaction and fail closed - Run and record each pending migration in its own transaction. An enum value added by one migration can now be used by the next (55P04), and a failure keeps the migrations before it. - Compare the committed journal with the recorded rows by hash and created_at. Fail closed on a migration older than the last recorded one; warn on edited migrations and on recorded rows with no committed file. - Read the history without the lock first. Take the lock only when something is pending, then read again. - Bound the wait for the migration lock to 5 minutes and name its holder. Set application_name and client_connection_check_interval. Lower lock_timeout to 5s with three tries per migration. - Migrate on start only with APPKIT_DB_MIGRATE_ON_START=true and service principal credentials. Skip dev- and session- branches; refuse a -pooler host. - Name the failed step and the SQLSTATE or system code. On 42501, name the schema owner and the connecting role. - Add real-Postgres tests gated on APPKIT_TEST_PG_URL; CI runs them against a postgres:17 service. Signed-off-by: ditadi --- .github/workflows/ci.yml | 14 + .../appkit/src/database/migrations/apply.ts | 723 +++++++++---- .../appkit/src/database/migrations/history.ts | 64 ++ .../src/database/migrations/ownership.ts | 58 +- .../appkit/src/database/migrations/paths.ts | 2 +- .../migrations/tests/apply.pg.test.ts | 408 ++++++++ .../database/migrations/tests/apply.test.ts | 984 +++++++++--------- .../src/database/migrations/tests/fixtures.ts | 86 ++ .../database/migrations/tests/history.test.ts | 75 ++ .../migrations/tests/ownership.test.ts | 28 +- .../src/plugins/database/migrate-on-start.ts | 79 +- .../src/plugins/database/schema-check.ts | 2 +- .../database/tests/migrate-on-start.test.ts | 160 ++- 13 files changed, 1909 insertions(+), 774 deletions(-) create mode 100644 packages/appkit/src/database/migrations/history.ts create mode 100644 packages/appkit/src/database/migrations/tests/apply.pg.test.ts create mode 100644 packages/appkit/src/database/migrations/tests/fixtures.ts create mode 100644 packages/appkit/src/database/migrations/tests/history.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23bb21c94..8a2cea71f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -117,6 +117,18 @@ jobs: runs-on: group: databricks-ghec-untrusted-runner-group labels: linux-ubuntu-latest-untrusted-2core-8gb + services: + postgres: + image: postgres:17@sha256:2d2b8998d31037bf721cfdf764d76ba74171b4fab3431b7f72c27c56ddbdf9e3 + env: + POSTGRES_PASSWORD: postgres + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U postgres" + --health-interval 5s + --health-timeout 5s + --health-retries 10 steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -131,6 +143,8 @@ jobs: run: pnpm install --frozen-lockfile - name: Run Tests run: pnpm test + env: + APPKIT_TEST_PG_URL: postgres://postgres:postgres@localhost:5432/postgres playground-integration-test: name: Playground Integration Tests diff --git a/packages/appkit/src/database/migrations/apply.ts b/packages/appkit/src/database/migrations/apply.ts index a4e6dd704..bc9a1f99d 100644 --- a/packages/appkit/src/database/migrations/apply.ts +++ b/packages/appkit/src/database/migrations/apply.ts @@ -1,18 +1,25 @@ import { createHash } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; +import { setTimeout as sleep } from "node:timers/promises"; import { readMigrationFiles } from "drizzle-orm/migrator"; -import { drizzle } from "drizzle-orm/node-postgres"; -import { migrate } from "drizzle-orm/node-postgres/migrator"; import type { Pool, PoolClient } from "pg"; import { createLogger } from "../../logging/logger"; -import { ownershipHint } from "./ownership"; +import { + type MigrationFile, + type RecordedMigration, + reconcileMigrations, +} from "./history"; +import { + INSUFFICIENT_PRIVILEGE, + ownershipHint, + schemaOwnership, +} from "./ownership"; import { BASELINE_TAG, type Journal, - type JournalEntry, MIGRATIONS_TABLE, MigrationFilesError, readJournal, @@ -26,6 +33,10 @@ interface ApplyMigrationsOptions { readonly migrationsFolder: string; /** Journal schema = app schema. */ readonly schemaName: string; + /** Longest wait for another session's migration lock, in milliseconds. */ + readonly lockWaitMs?: number; + /** Longest wait for each table lock a migration takes, in milliseconds. */ + readonly lockTimeoutMs?: number; } interface ApplyMigrationsResult { @@ -33,59 +44,95 @@ interface ApplyMigrationsResult { readonly applied: readonly string[]; /** Tags recorded without running, because the catalog already held them (the baseline). */ readonly adopted: readonly string[]; + /** Recorded migrations whose committed file changed since; the change never runs. */ + readonly edited: readonly string[]; + /** Recorded migrations that are not committed here. */ + readonly unknown: number; + /** Time spent waiting for another session's migration lock. */ + readonly lockWaitMs: number; } -/** lock_not_available: a table lock was not granted within LOCK_TIMEOUT. */ -const LOCK_NOT_AVAILABLE = "55P03"; +/** A deploy waits this long for another instance to finish migrating, then fails. */ +const LOCK_WAIT_MS = 5 * 60_000; /** - * Longest wait for a table lock while migrating. Past it the migration fails; - * waiting longer would queue every other query on that table behind it. + * Longest wait for one table lock. Past it the migration rolls back and is + * retried: a longer wait would queue every other query on that table behind it. */ -const LOCK_TIMEOUT = "30s"; +const LOCK_TIMEOUT_MS = 5_000; +/** Tries per migration when a table lock is not granted within the lock timeout. */ +const LOCK_ATTEMPTS = 3; +const LOCK_POLL_MS = 1_000; +const LOCK_REPORT_MS = 30_000; +/** Shown in pg_stat_activity, so a waiting instance can name the lock holder. */ +const APPLICATION_NAME = "appkit-migrate"; +/** Lets the server cancel a migration whose instance died. Linux servers only. */ +const CONNECTION_CHECK_INTERVAL = "10s"; -function failureHint(code: string | undefined): string | undefined { - if (code === LOCK_NOT_AVAILABLE) { - return `Another session held a lock on a table this migration changes for over ${LOCK_TIMEOUT}; deploy again once it finishes.`; - } - return ownershipHint(code); -} +/** lock_not_available: a table lock was not granted within the lock timeout. */ +const LOCK_NOT_AVAILABLE = "55P03"; /** - * A statement failed while migrating. Pending migrations run in one drizzle-orm - * migrator transaction, so none of them were applied; a baseline adopted just - * before commits on its own and stays recorded. + * A step of applying migrations failed. Each migration commits in its own + * transaction: the failing one rolled back, the ones before it stay applied. */ export class MigrationApplyError extends Error { - /** Tags that were being applied; empty when reading the recorded migrations failed. */ + /** The migration that failed and rolled back; undefined when another step failed. */ + readonly failed?: string; + /** Committed migrations still not applied, the failed one first. */ readonly pending: readonly string[]; + /** Tags this run applied before the failure. */ + readonly applied: readonly string[]; + /** Tags this run recorded as applied (adopted) before the failure. */ + readonly adopted: readonly string[]; /** SQLSTATE of the failing statement, when the driver reported one. */ readonly code?: string; - /** Tags this run recorded as applied before the failure. */ - readonly adopted: readonly string[]; - constructor( - pending: readonly string[], - code: string | undefined, - options: { cause?: unknown; adopted?: readonly string[] } = {}, - ) { - const adopted = options.adopted ?? []; - const sqlState = code ? ` (SQLSTATE ${code})` : ""; - const failure = - pending.length > 0 - ? `Applying migration${pending.length === 1 ? "" : "s"} ${pending.join(", ")} failed${sqlState}; none were applied.` - : `Reading the recorded migrations in ${MIGRATIONS_TABLE} failed${sqlState}; none were applied.`; - const kept = - adopted.length > 0 - ? ` ${adopted.join(", ")} stays recorded as applied: its tables already existed.` + constructor(details: { + /** What failed when no migration did, e.g. "Connecting to the database". */ + readonly step: string; + readonly failed?: string; + readonly pending?: readonly string[]; + readonly applied?: readonly string[]; + readonly adopted?: readonly string[]; + readonly hint?: string; + readonly cause?: unknown; + }) { + const pending = details.pending ?? []; + const applied = details.applied ?? []; + const adopted = details.adopted ?? []; + const code = sqlStateOf(details.cause); + const systemCode = code ? undefined : systemCodeOf(details.cause); + const reason = code + ? ` (SQLSTATE ${code})` + : systemCode + ? ` (${systemCode})` : ""; - const hint = failureHint(code); - super(`${failure}${kept}${hint ? ` ${hint}` : ""}`, { - cause: options.cause, - }); + const parts = [ + details.failed + ? `Migration ${details.failed} failed${reason} and was rolled back.` + : `${details.step} failed${reason}.`, + ]; + if (adopted.length > 0) { + parts.push( + `${adopted.join(", ")} stays recorded as applied: its tables already existed.`, + ); + } + if (applied.length > 0) { + parts.push(`Applied before the failure: ${applied.join(", ")}.`); + } + parts.push( + pending.length > 0 + ? `Not applied: ${pending.join(", ")}.` + : "No migration was applied.", + ); + if (details.hint) parts.push(details.hint); + super(parts.join(" "), { cause: details.cause }); this.name = "MigrationApplyError"; + this.failed = details.failed; this.pending = Object.freeze([...pending]); - this.code = code; + this.applied = Object.freeze([...applied]); this.adopted = Object.freeze([...adopted]); + this.code = code; } } @@ -106,26 +153,67 @@ export class MigrationBaselineMismatchError extends Error { } } +/** + * Committed migrations are older than the last one the database recorded and + * were never applied, so they would never run. Nothing was applied. + */ +export class MigrationHistoryError extends Error { + readonly stranded: readonly string[]; + + constructor(schemaName: string, stranded: readonly string[], last: string) { + const many = stranded.length > 1; + super( + `Migration${many ? "s" : ""} ${stranded.join(", ")} ${many ? "are" : "is"} older than ${last}, which schema "${schemaName}" already recorded, ` + + `and ${many ? "were" : "was"} never applied, so ${many ? "they" : "it"} would never run. This happens when migrations from parallel branches merge. ` + + `Delete ${many ? "their" : "its"} .sql file, snapshot and journal entry, generate the migration again from schema.ts, and deploy again. ` + + "No migration was applied.", + ); + this.name = "MigrationHistoryError"; + this.stranded = Object.freeze([...stranded]); + } +} + +/** The migration lock stayed taken past the wait limit. */ +class MigrationLockWaitError extends Error { + constructor(readonly holder: string | undefined) { + super("migration lock wait timed out"); + } +} + const SQLSTATE = /^[0-9A-Z]{5}$/; +const SYSTEM_CODE = /^E[A-Z0-9_]+$/; -/** drizzle wraps driver errors; the SQLSTATE lives somewhere on the cause chain. */ -function sqlStateOf(error: unknown): string | undefined { +/** drizzle and the connector wrap driver errors; the code lives somewhere on the cause chain. */ +function findCode( + error: unknown, + matches: (code: string) => boolean, +): string | undefined { let current = error; for (let depth = 0; depth < 5 && current; depth++) { const code = (current as { code?: unknown }).code; - // No SQLSTATE class starts with "E"; Node system errors (EPIPE, EPERM, …) all do. - if (typeof code === "string" && SQLSTATE.test(code) && code[0] !== "E") { - return code; - } + if (typeof code === "string" && matches(code)) return code; current = (current as { cause?: unknown }).cause; } return undefined; } +// No SQLSTATE class starts with "E"; Node system errors (EPIPE, ECONNREFUSED, …) all do. +function sqlStateOf(error: unknown): string | undefined { + return findCode(error, (code) => SQLSTATE.test(code) && code[0] !== "E"); +} + +function systemCodeOf(error: unknown): string | undefined { + return findCode(error, (code) => SYSTEM_CODE.test(code)); +} + function quoteIdentifier(name: string): string { return `"${name.replace(/"/g, '""')}"`; } +function journalTable(schemaName: string): string { + return `${quoteIdentifier(schemaName)}.${quoteIdentifier(MIGRATIONS_TABLE)}`; +} + /** One lock per app schema, so concurrent instances apply in turn. */ export function migrationLockKey(schemaName: string): string { return createHash("sha256") @@ -137,15 +225,40 @@ export function migrationLockKey(schemaName: string): string { type QueryClient = Pick; -/** - * `created_at` of the last migration the database recorded; null when none is. - * An empty journal table counts as nothing recorded: drizzle-orm creates it - * before its transaction, so a failed first deploy leaves one behind. - */ -async function lastRecordedMigration( +/** Every committed migration, in journal order. Fails on a missing file before connecting. */ +function readMigrations(folder: string, journal: Journal): MigrationFile[] { + for (const entry of journal.entries) { + const file = path.join(folder, `${entry.tag}.sql`); + if (!fs.existsSync(file)) { + throw new MigrationFilesError( + `Missing ${file}, listed in meta/_journal.json. Restore it from version control.`, + ); + } + } + // drizzle-orm's reader, so `hash` matches what drizzle-kit records. + const files = readMigrationFiles({ migrationsFolder: folder }); + return journal.entries.map((entry, index) => { + const file = files[index]; + if (!file) { + throw new MigrationFilesError( + `Cannot read ${entry.tag}.sql in ${folder}. Restore it from version control.`, + ); + } + return { + tag: entry.tag, + idx: entry.idx, + when: entry.when, + hash: file.hash, + statements: file.sql, + }; + }); +} + +/** The journal table's rows; none when the table does not exist yet. */ +async function readRecordedMigrations( client: QueryClient, schemaName: string, -): Promise { +): Promise { // pg_catalog needs no privileges, so a missing grant is not mistaken for a missing table. const table = await client.query( `select 1 from pg_catalog.pg_class c @@ -153,99 +266,103 @@ async function lastRecordedMigration( where n.nspname = $1 and c.relname = $2`, [schemaName, MIGRATIONS_TABLE], ); - if (table.rows.length === 0) return null; - const last = await client.query<{ last: string | number | null }>( - `select max(created_at) as last from ${quoteIdentifier(schemaName)}.${quoteIdentifier(MIGRATIONS_TABLE)}`, - ); - const value = last.rows[0]?.last; - return value === null || value === undefined ? null : Number(value); + if (table.rows.length === 0) return []; + const { rows } = await client.query<{ + hash: string; + created_at: string | number | null; + }>(`select hash, created_at from ${journalTable(schemaName)} order by id`); + return rows.map((row) => ({ + hash: row.hash, + createdAt: row.created_at === null ? null : Number(row.created_at), + })); } -/** - * Journal entries newer than the last recorded migration: drizzle-orm's own - * rule, so this agrees with what its migrator runs. - * - * Known gap, shared with drizzle-orm: an entry older than the last recorded one - * (e.g. merged from a parallel branch) is never applied, and an applied - * migration edited later goes unnoticed. Comparing the recorded hashes with the - * journal would catch both. - */ -function pendingEntries( - journal: Journal, - lastRecorded: number | null, -): JournalEntry[] { - if (lastRecorded === null) return journal.entries; - return journal.entries.filter((entry) => entry.when > lastRecorded); +/** Session settings for migrating, each added to `reset` once set. */ +async function prepareMigrationSession( + client: QueryClient, + reset: string[], +): Promise { + await client.query("SET statement_timeout = 0"); + reset.push("statement_timeout"); + await client.query(`SET application_name = '${APPLICATION_NAME}'`); + reset.push("application_name"); + try { + await client.query( + `SET client_connection_check_interval = '${CONNECTION_CHECK_INTERVAL}'`, + ); + reset.push("client_connection_check_interval"); + } catch (error) { + // Postgres before 14, or a server that cannot poll sockets. + logger.debug( + "client_connection_check_interval not set: %s", + error instanceof Error ? error.message : String(error), + ); + } } -/** Fail before connecting, naming the file, rather than inside drizzle-orm's migrator. */ -function assertMigrationFiles(folder: string, journal: Journal): void { - for (const entry of journal.entries) { - const file = path.join(folder, `${entry.tag}.sql`); - if (!fs.existsSync(file)) { - throw new MigrationFilesError( - `Missing ${file}, listed in meta/_journal.json. Restore it from version control.`, - ); - } - } +/** pg_locks splits a bigint advisory key into classid (high 32 bits) and objid (low 32 bits). */ +function lockKeyParts(key: string): [string, string] { + const unsigned = BigInt.asUintN(64, BigInt(key)); + return [(unsigned >> 32n).toString(), (unsigned & 0xffffffffn).toString()]; +} + +/** "pid 123 (appkit-migrate, active)" for the session holding the migration lock. */ +async function migrationLockHolder( + client: QueryClient, + key: string, +): Promise { + const [classid, objid] = lockKeyParts(key); + const { rows } = await client.query<{ + pid: number; + application_name: string | null; + state: string | null; + }>( + `select a.pid, a.application_name, a.state + from pg_catalog.pg_locks l + join pg_catalog.pg_stat_activity a on a.pid = l.pid + where l.locktype = 'advisory' and l.granted + and l.database = (select d.oid from pg_catalog.pg_database d where d.datname = current_database()) + and l.classid = $1::oid and l.objid = $2::oid and l.objsubid = 1`, + [classid, objid], + ); + const holder = rows[0]; + if (!holder) return undefined; + const details = [holder.application_name, holder.state].filter(Boolean); + return `pid ${holder.pid}${details.length > 0 ? ` (${details.join(", ")})` : ""}`; } /** - * Run `work` on one dedicated connection under the per-schema advisory lock, - * with no statement timeout and at most LOCK_TIMEOUT per table lock. The - * connection is always unlocked, reset and released; a connection whose - * cleanup fails is destroyed, not reused. + * Take the per-schema advisory lock, polling so the wait has a limit and the + * log names the holder. Returns how long it waited. */ -async function withMigrationLock( - pool: Pick, +async function acquireMigrationLock( + client: QueryClient, schemaName: string, - work: (client: PoolClient) => Promise, -): Promise { - const lockKey = migrationLockKey(schemaName); - const client = await pool.connect(); - let locked = false; - try { - await client.query("SET statement_timeout = 0"); - const attempt = await client.query<{ locked: boolean }>( + waitMs: number, +): Promise { + const key = migrationLockKey(schemaName); + const started = Date.now(); + let reportAt = started; + for (;;) { + const { rows } = await client.query<{ locked: boolean }>( "select pg_catalog.pg_try_advisory_lock($1::bigint) as locked", - [lockKey], + [key], ); - if (!attempt.rows[0]?.locked) { + if (rows[0]?.locked) return Date.now() - started; + const waited = Date.now() - started; + if (waited >= waitMs || Date.now() >= reportAt) { + const holder = await migrationLockHolder(client, key).catch( + () => undefined, + ); + if (waited >= waitMs) throw new MigrationLockWaitError(holder); logger.info( - "Waiting for another instance to finish migrating schema %s", + "Waiting for another session to finish migrating schema %s (lock held by %s)", schemaName, + holder ?? "an unknown session", ); - await client.query("select pg_catalog.pg_advisory_lock($1::bigint)", [ - lockKey, - ]); + reportAt = Date.now() + LOCK_REPORT_MS; } - locked = true; - // Only now: lock_timeout would also cut short the wait for the advisory lock. - await client.query(`SET lock_timeout = '${LOCK_TIMEOUT}'`); - return await work(client); - } finally { - const cleanup: Array<() => Promise> = []; - if (locked) { - cleanup.push(() => - client.query("select pg_catalog.pg_advisory_unlock($1::bigint)", [ - lockKey, - ]), - ); - } - cleanup.push(() => client.query("RESET lock_timeout")); - cleanup.push(() => client.query("RESET statement_timeout")); - - let cleanupError: Error | undefined; - for (const step of cleanup) { - try { - await step(); - } catch (error) { - cleanupError ??= - error instanceof Error ? error : new Error(String(error)); - } - } - // A connection that may still hold the lock or the session settings must not be reused. - client.release(cleanupError); + await sleep(Math.min(LOCK_POLL_MS, waitMs - waited)); } } @@ -263,6 +380,74 @@ async function inTransaction( } } +/** drizzle-orm's own journal table, so drizzle-kit reads and extends it too. */ +async function ensureJournalTable( + client: QueryClient, + schemaName: string, +): Promise { + await client.query( + `CREATE SCHEMA IF NOT EXISTS ${quoteIdentifier(schemaName)}`, + ); + await client.query( + `CREATE TABLE IF NOT EXISTS ${journalTable(schemaName)} ( + id SERIAL PRIMARY KEY, + hash text NOT NULL, + created_at bigint + )`, + ); +} + +/** Record a migration the way drizzle-orm's migrator does: its file hash and the journal's `when`. */ +async function recordMigration( + client: QueryClient, + schemaName: string, + file: MigrationFile, +): Promise { + await client.query( + `insert into ${journalTable(schemaName)} ("hash", "created_at") values($1, $2)`, + [file.hash, file.when], + ); +} + +/** + * Run one migration and record it in one transaction. A table lock not granted + * within the lock timeout rolls it back; it is retried LOCK_ATTEMPTS times. + */ +async function applyMigration( + client: QueryClient, + schemaName: string, + file: MigrationFile, + lockTimeoutMs: number, +): Promise { + for (let attempt = 1; ; attempt++) { + try { + await inTransaction(client, async () => { + await ensureJournalTable(client, schemaName); + for (const statement of file.statements) { + if (statement.trim()) await client.query(statement); + } + await recordMigration(client, schemaName, file); + }); + return; + } catch (error) { + if ( + attempt >= LOCK_ATTEMPTS || + sqlStateOf(error) !== LOCK_NOT_AVAILABLE + ) { + throw error; + } + logger.warn( + "Migration %s waited over %dms for a table lock (try %d of %d); retrying", + file.tag, + lockTimeoutMs, + attempt, + LOCK_ATTEMPTS, + ); + await sleep(Math.min(attempt * LOCK_POLL_MS, lockTimeoutMs)); + } + } +} + /** The tables and columns a migration snapshot (meta/NNNN_snapshot.json) declares in one schema. */ function snapshotTables( folder: string, @@ -302,7 +487,7 @@ type BaselineState = async function baselineState( client: QueryClient, options: ApplyMigrationsOptions, - baseline: JournalEntry, + baseline: MigrationFile, ): Promise { const declared = snapshotTables( options.migrationsFolder, @@ -351,52 +536,60 @@ async function baselineState( : { kind: "present" }; } -/** - * Record migrations as applied without running them: create the journal table - * as drizzle-orm's migrator does, then insert one row per tag with the - * migrator's own hash (sha256 of the .sql file) and `created_at` (the journal's - * `when`). Runs on the caller's connection; wrap it in a transaction. - */ -async function recordMigrationsApplied( - client: QueryClient, - options: ApplyMigrationsOptions, - journal: Journal, - tags: readonly string[], +/** Unlock and reset the session. A connection whose cleanup fails is destroyed, not reused. */ +async function releaseMigrationClient( + client: PoolClient, + lockKey: string | undefined, + reset: readonly string[], ): Promise { - // One file per journal entry, in journal order. - const files = readMigrationFiles({ - migrationsFolder: options.migrationsFolder, - }); - const schema = quoteIdentifier(options.schemaName); - const table = `${schema}.${quoteIdentifier(MIGRATIONS_TABLE)}`; - await client.query(`CREATE SCHEMA IF NOT EXISTS ${schema}`); - // drizzle-orm's own definition, so its migrator later reads and extends this table. - await client.query( - `CREATE TABLE IF NOT EXISTS ${table} ( - id SERIAL PRIMARY KEY, - hash text NOT NULL, - created_at bigint - )`, - ); - for (const tag of tags) { - const index = journal.entries.findIndex((entry) => entry.tag === tag); - const file = files[index]; - if (index < 0 || !file) { - throw new Error(`Migration ${tag} is not in the journal.`); - } - await client.query( - `insert into ${table} ("hash", "created_at") values($1, $2)`, - [file.hash, file.folderMillis], + const cleanup: Array<() => Promise> = []; + if (lockKey) { + cleanup.push(() => + client.query("select pg_catalog.pg_advisory_unlock($1::bigint)", [ + lockKey, + ]), ); } + for (const name of [...reset].reverse()) { + cleanup.push(() => client.query(`RESET ${name}`)); + } + let cleanupError: Error | undefined; + for (const step of cleanup) { + try { + await step(); + } catch (error) { + cleanupError ??= + error instanceof Error ? error : new Error(String(error)); + } + } + client.release(cleanupError); +} + +function tags(files: readonly MigrationFile[]): string[] { + return files.map((file) => file.tag); +} + +function duration(ms: number): string { + if (ms >= 60_000) { + const minutes = Math.round(ms / 60_000); + return `${minutes} minute${minutes === 1 ? "" : "s"}`; + } + if (ms >= 1_000) return `${Math.round(ms / 1_000)}s`; + return `${ms}ms`; } /** * Apply every committed migration the database has not recorded yet. * - * Runs on one dedicated connection as the connecting identity, which owns what - * it creates (see ownership.ts), under a per-schema advisory lock; drizzle-orm's - * migrator runs only when something is pending. + * Runs on one connection as the connecting identity, which owns what it + * creates (see ownership.ts). The recorded history is read first without a + * lock; only when something is pending does it take the per-schema advisory + * lock and read again. Each migration then runs and is recorded in its own + * transaction, so a failure keeps the ones before it. + * + * Fails closed, changing nothing, when a committed migration is older than the + * last recorded one but was never applied. An applied migration whose file + * changed, and recorded migrations that are not committed, are logged. * * Baseline adoption: when nothing is recorded yet and `0000_baseline` comes * first, the baseline's tables are looked up in pg_catalog. None present → it @@ -410,60 +603,150 @@ export async function applyPendingMigrations( ): Promise { const journal = readJournal(options.migrationsFolder); if (!journal || journal.entries.length === 0) { - return { applied: [], adopted: [] }; + return { applied: [], adopted: [], edited: [], unknown: 0, lockWaitMs: 0 }; } - assertMigrationFiles(options.migrationsFolder, journal); + const files = readMigrations(options.migrationsFolder, journal); + const { schemaName } = options; + const lockWaitMs = options.lockWaitMs ?? LOCK_WAIT_MS; + const lockTimeoutMs = options.lockTimeoutMs ?? LOCK_TIMEOUT_MS; - return withMigrationLock(pool, options.schemaName, async (client) => { - const adopted: string[] = []; - // What the failing step was applying, for the error. - let applying: readonly string[] = []; - try { - const lastRecorded = await lastRecordedMigration( - client, - options.schemaName, + let client: PoolClient; + try { + client = await pool.connect(); + } catch (error) { + throw new MigrationApplyError({ + step: "Connecting to the database", + cause: error, + }); + } + + const reset: string[] = []; + let locked = false; + const applied: string[] = []; + const adopted: string[] = []; + let pending: string[] = []; + let step = "Reading the recorded migrations"; + let failed: string | undefined; + try { + let history = reconcileMigrations( + files, + await readRecordedMigrations(client, schemaName), + ); + let waitedMs = 0; + if (history.pending.length > 0 && history.stranded.length === 0) { + pending = tags(history.pending); + step = "Waiting for the migration lock"; + await prepareMigrationSession(client, reset); + waitedMs = await acquireMigrationLock(client, schemaName, lockWaitMs); + locked = true; + await client.query(`SET lock_timeout = '${lockTimeoutMs}ms'`); + reset.push("lock_timeout"); + step = "Reading the recorded migrations"; + // Another instance may have applied them while this one waited. + history = reconcileMigrations( + files, + await readRecordedMigrations(client, schemaName), ); - let pending = pendingEntries(journal, lastRecorded); - if (pending.length === 0) return { applied: [], adopted }; - - const first = journal.entries[0]; - if (lastRecorded === null && first?.tag === BASELINE_TAG) { - applying = [BASELINE_TAG]; - const state = await baselineState(client, options, first); - if (state.kind === "partial") { - throw new MigrationBaselineMismatchError( - options.schemaName, - state.problems, - ); - } - if (state.kind === "present") { - await inTransaction(client, () => - recordMigrationsApplied(client, options, journal, [BASELINE_TAG]), - ); - adopted.push(BASELINE_TAG); - pending = pending.filter((entry) => entry.tag !== BASELINE_TAG); - if (pending.length === 0) return { applied: [], adopted }; - } - } + } - applying = pending.map((entry) => entry.tag); - await migrate(drizzle({ client }), { - migrationsFolder: options.migrationsFolder, - migrationsSchema: options.schemaName, - migrationsTable: MIGRATIONS_TABLE, - }); - return { applied: applying, adopted }; - } catch (error) { - if ( - error instanceof MigrationBaselineMismatchError || - error instanceof MigrationFilesError - ) { - throw error; + if (history.stranded.length > 0) { + const last = + files.find((file) => file.when === history.lastRecorded)?.tag ?? + `the migration recorded at ${new Date(history.lastRecorded ?? 0).toISOString()}`; + throw new MigrationHistoryError(schemaName, tags(history.stranded), last); + } + if (history.edited.length > 0) { + logger.warn( + "Migrations %s changed after schema %s applied them; the changes never run. Restore the files and put the change in a new migration.", + tags(history.edited).join(", "), + schemaName, + ); + } + if (history.unknown.length > 0) { + logger.warn( + "Schema %s recorded %d migration(s) that are not committed here. A newer release applied them, or their files were deleted.", + schemaName, + history.unknown.length, + ); + } + + const toApply = [...history.pending]; + pending = tags(toApply); + const baseline = toApply[0]; + if (history.lastRecorded === null && baseline?.tag === BASELINE_TAG) { + failed = BASELINE_TAG; + const state = await baselineState(client, options, baseline); + if (state.kind === "partial") { + throw new MigrationBaselineMismatchError(schemaName, state.problems); + } + if (state.kind === "present") { + await inTransaction(client, async () => { + await ensureJournalTable(client, schemaName); + await recordMigration(client, schemaName, baseline); + }); + adopted.push(BASELINE_TAG); + toApply.shift(); + pending.shift(); } - throw new MigrationApplyError(applying, sqlStateOf(error), { - cause: error, - adopted, - }); } - }); + + for (const file of toApply) { + failed = file.tag; + const started = Date.now(); + await applyMigration(client, schemaName, file, lockTimeoutMs); + applied.push(file.tag); + pending.shift(); + logger.info( + "Applied migration %s to schema %s in %dms", + file.tag, + schemaName, + Date.now() - started, + ); + } + return { + applied, + adopted, + edited: tags(history.edited), + unknown: history.unknown.length, + lockWaitMs: waitedMs, + }; + } catch (error) { + if ( + error instanceof MigrationBaselineMismatchError || + error instanceof MigrationFilesError || + error instanceof MigrationHistoryError + ) { + throw error; + } + const code = sqlStateOf(error); + let hint: string | undefined; + if (error instanceof MigrationLockWaitError) { + const holder = error.holder + ? `Session ${error.holder}` + : "Another session"; + hint = `${holder} held the migration lock for over ${duration(lockWaitMs)}. Deploy again once it finishes; if it is stuck, end it with pg_terminate_backend.`; + } else if (code === LOCK_NOT_AVAILABLE) { + hint = `Another session held a lock on a table this migration changes for over ${duration(lockTimeoutMs)}, ${LOCK_ATTEMPTS} times. Deploy again once the long-running query or transaction finishes.`; + } else if (code === INSUFFICIENT_PRIVILEGE) { + const ownership = await schemaOwnership(client, schemaName).catch( + () => undefined, + ); + hint = ownershipHint(code, ownership); + } + throw new MigrationApplyError({ + step, + failed, + pending, + applied, + adopted, + hint, + cause: error, + }); + } finally { + await releaseMigrationClient( + client, + locked ? migrationLockKey(schemaName) : undefined, + reset, + ); + } } diff --git a/packages/appkit/src/database/migrations/history.ts b/packages/appkit/src/database/migrations/history.ts new file mode 100644 index 000000000..c7ca35cda --- /dev/null +++ b/packages/appkit/src/database/migrations/history.ts @@ -0,0 +1,64 @@ +/** A committed migration: its journal entry plus what drizzle-orm reads from its .sql file. */ +export interface MigrationFile { + readonly tag: string; + readonly idx: number; + /** The journal's `when`, recorded as `created_at`. */ + readonly when: number; + /** sha256 of the .sql file, recorded as `hash`. */ + readonly hash: string; + /** The file split on `--> statement-breakpoint`. */ + readonly statements: readonly string[]; +} + +/** One row of the journal table. */ +export interface RecordedMigration { + readonly hash: string; + readonly createdAt: number | null; +} + +/** Committed migrations compared with what the database recorded. */ +interface MigrationHistory { + /** Not recorded and newer than every recorded migration: applied next, oldest first. */ + readonly pending: readonly MigrationFile[]; + /** Not recorded but older than the last recorded migration, so a later one ran first. */ + readonly stranded: readonly MigrationFile[]; + /** Recorded, but the committed file changed since; the change never runs. */ + readonly edited: readonly MigrationFile[]; + /** Recorded rows no committed migration matches: a newer release ran, or files were deleted. */ + readonly unknown: readonly RecordedMigration[]; + /** `created_at` of the newest recorded migration; null when none is. */ + readonly lastRecorded: number | null; +} + +/** + * Match each committed migration with a recorded row by `when` = `created_at`, + * the pair drizzle-orm's migrator writes, then compare hashes. + */ +export function reconcileMigrations( + files: readonly MigrationFile[], + recorded: readonly RecordedMigration[], +): MigrationHistory { + const hashAt = new Map(); + for (const row of recorded) { + if (row.createdAt !== null) hashAt.set(row.createdAt, row.hash); + } + const lastRecorded = hashAt.size > 0 ? Math.max(...hashAt.keys()) : null; + + const pending: MigrationFile[] = []; + const stranded: MigrationFile[] = []; + const edited: MigrationFile[] = []; + for (const file of files) { + const hash = hashAt.get(file.when); + if (hash === undefined) { + if (lastRecorded === null || file.when > lastRecorded) pending.push(file); + else stranded.push(file); + } else if (hash !== file.hash) { + edited.push(file); + } + } + const committed = new Set(files.map((file) => file.when)); + const unknown = recorded.filter( + (row) => row.createdAt === null || !committed.has(row.createdAt), + ); + return { pending, stranded, edited, unknown, lastRecorded }; +} diff --git a/packages/appkit/src/database/migrations/ownership.ts b/packages/appkit/src/database/migrations/ownership.ts index 606efb1d1..ec2dd75ac 100644 --- a/packages/appkit/src/database/migrations/ownership.ts +++ b/packages/appkit/src/database/migrations/ownership.ts @@ -1,18 +1,66 @@ // Ownership model: each branch has one owner, the identity its migrations run // as, and that identity owns the app's objects there. On the deploy branch it // is the app's service principal, which creates the app schema on first start. -// There is no shared owner role and no SET ROLE. A shared owner role is not -// possible: Lakebase rejects `GRANT … TO databricks_superuser`. +// There is no shared owner role and no SET ROLE: a shared role needs someone +// holding ADMIN OPTION on it to grant it to every new app, which a deployed +// app cannot do for itself. + +import type { PoolClient } from "pg"; /** insufficient_privilege: "must be owner of …", "permission denied for …". */ -const INSUFFICIENT_PRIVILEGE = "42501"; +export const INSUFFICIENT_PRIVILEGE = "42501"; + +/** Who owns the app schema, next to who is connected; owner is null when the schema does not exist. */ +interface SchemaOwnership { + readonly schemaName: string; + readonly owner: string | null; + readonly currentUser: string; +} + +/** Read the app schema's owner. pg_catalog needs no privileges. */ +export async function schemaOwnership( + client: Pick, + schemaName: string, +): Promise { + const { rows } = await client.query<{ + owner: string | null; + current_user: string; + }>( + `select (select pg_catalog.pg_get_userbyid(n.nspowner) + from pg_catalog.pg_namespace n where n.nspname = $1) as owner, + current_user::text as current_user`, + [schemaName], + ); + return { + schemaName, + owner: rows[0]?.owner ?? null, + currentUser: rows[0]?.current_user ?? "", + }; +} /** How to fix a migration that failed for lack of privileges; undefined for any other SQLSTATE. */ -export function ownershipHint(code: string | undefined): string | undefined { +export function ownershipHint( + code: string | undefined, + ownership?: SchemaOwnership, +): string | undefined { if (code !== INSUFFICIENT_PRIVILEGE) return undefined; + if (ownership && ownership.owner === null) { + return ( + `Role "${ownership.currentUser}" cannot create schema "${ownership.schemaName}". ` + + "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE." + ); + } + if (ownership && ownership.owner !== ownership.currentUser) { + return ( + `Schema "${ownership.schemaName}" is owned by "${ownership.owner}", but the app connects as "${ownership.currentUser}". ` + + "This happens when the app's database resource was removed and added again, or the app was recreated with a new service principal: " + + `Lakebase hands the objects to another role. Have "${ownership.owner}" transfer the schema and its tables to "${ownership.currentUser}" ` + + "(ALTER … OWNER TO), or set another schemaName in schema.ts." + ); + } return ( "The connecting role lacks a privilege this migration needs: CREATE on the database, or ownership of the app schema and its tables. " + "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE, " + - "and if another role created the schema, use another schemaName in schema.ts or have that role transfer the schema and its objects." + "and if another role created an object the migration changes, have that role transfer it to the app's service principal." ); } diff --git a/packages/appkit/src/database/migrations/paths.ts b/packages/appkit/src/database/migrations/paths.ts index e331080b0..8d5126076 100644 --- a/packages/appkit/src/database/migrations/paths.ts +++ b/packages/appkit/src/database/migrations/paths.ts @@ -34,7 +34,7 @@ export function migrationsFolder(root: string): string { } /** Path of the journal inside a migrations folder. */ -function journalFile(folder: string): string { +export function journalFile(folder: string): string { return path.join(folder, "meta", "_journal.json"); } diff --git a/packages/appkit/src/database/migrations/tests/apply.pg.test.ts b/packages/appkit/src/database/migrations/tests/apply.pg.test.ts new file mode 100644 index 000000000..7e4875576 --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/apply.pg.test.ts @@ -0,0 +1,408 @@ +import { randomUUID } from "node:crypto"; + +import { Pool, type PoolClient } from "pg"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + test, + vi, +} from "vitest"; + +import { + applyPendingMigrations, + MigrationApplyError, + MigrationHistoryError, + migrationLockKey, +} from "../apply"; +import { hashOf, removeMigrationFolders, writeMigrations } from "./fixtures"; + +// A real server, for what a fake cannot show: transactions, enum rules, locks +// and catalogs. Point it at a scratch database, e.g. +// APPKIT_TEST_PG_URL=postgres://localhost:5432/postgres +const url = process.env.APPKIT_TEST_PG_URL; +const ROLE_PASSWORD = "appkit-test"; + +describe.skipIf(!url)("applyPendingMigrations against Postgres", () => { + let admin: Pool; + let schema: string; + const pools: Pool[] = []; + const held: PoolClient[] = []; + + const newPool = (config: { user?: string; max?: number } = {}) => { + const target = new URL(url ?? ""); + if (config.user) { + target.username = config.user; + target.password = ROLE_PASSWORD; + } + const pool = new Pool({ + connectionString: target.toString(), + max: config.max ?? 2, + }); + pools.push(pool); + return pool; + }; + const q = (name: string) => `"${schema}"."${name}"`; + const options = (folder: string) => ({ + migrationsFolder: folder, + schemaName: schema, + }); + const recorded = async () => + ( + await admin.query<{ hash: string; created_at: string }>( + `select hash, created_at from ${q("__drizzle_migrations")} order by id`, + ) + ).rows.map((row) => ({ + hash: row.hash, + createdAt: Number(row.created_at), + })); + const columns = async (table: string) => + ( + await admin.query<{ column_name: string }>( + `select column_name from information_schema.columns + where table_schema = $1 and table_name = $2 order by ordinal_position`, + [schema, table], + ) + ).rows.map((row) => row.column_name); + + beforeAll(() => { + admin = new Pool({ connectionString: url, max: 4 }); + }); + afterAll(async () => { + await admin.end(); + }); + beforeEach(() => { + schema = `appkit_test_${randomUUID().slice(0, 8)}`; + vi.spyOn(console, "log").mockImplementation(() => undefined); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + }); + afterEach(async () => { + for (const client of held.splice(0)) client.release(true); + await Promise.all(pools.splice(0).map((pool) => pool.end())); + await admin.query(`drop schema if exists "${schema}" cascade`); + removeMigrationFolders(); + vi.restoreAllMocks(); + }); + + const notes = () => + writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer PRIMARY KEY);`], + }, + { + tag: "0001_body", + when: 2000, + sql: [ + `ALTER TABLE ${q("notes")} ADD COLUMN "body" text;`, + `ALTER TABLE ${q("notes")} ADD COLUMN "title" text;`, + ], + }, + ]); + + test("applies each migration once and records what drizzle-kit records", async () => { + const folder = notes(); + const pool = newPool(); + + const first = await applyPendingMigrations(pool, options(folder)); + const second = await applyPendingMigrations(pool, options(folder)); + + expect(first.applied).toEqual(["0000_notes", "0001_body"]); + expect(second.applied).toEqual([]); + expect(await columns("notes")).toEqual(["id", "body", "title"]); + expect(await recorded()).toEqual([ + { hash: hashOf(folder, "0000_notes"), createdAt: 1000 }, + { hash: hashOf(folder, "0001_body"), createdAt: 2000 }, + ]); + }); + + test("an enum value one migration adds can be used by the next one", async () => { + // In one transaction for all, Postgres rejects this with 55P04. + const folder = writeMigrations([ + { + tag: "0000_mood", + when: 1000, + sql: [ + `CREATE TYPE ${q("mood")} AS ENUM ('ok');`, + `CREATE TABLE ${q("notes")} ("id" integer, "mood" ${q("mood")});`, + ], + }, + { + tag: "0001_great", + when: 2000, + sql: [`ALTER TYPE ${q("mood")} ADD VALUE 'great';`], + }, + { + tag: "0002_default", + when: 3000, + sql: [ + `ALTER TABLE ${q("notes")} ALTER COLUMN "mood" SET DEFAULT 'great';`, + ], + }, + ]); + + const result = await applyPendingMigrations(newPool(), options(folder)); + + expect(result.applied).toEqual(["0000_mood", "0001_great", "0002_default"]); + }); + + test("a failing migration keeps the ones before it, and runs once it is fixed", async () => { + const folder = writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer);`], + }, + { + tag: "0001_broken", + when: 2000, + sql: [`ALTER TABLE ${q("missing")} ADD COLUMN "x" integer;`], + }, + ]); + const pool = newPool(); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ + failed: "0001_broken", + applied: ["0000_notes"], + code: "42P01", + }); + expect(await columns("notes")).toEqual(["id"]); + expect(await recorded()).toHaveLength(1); + + const fixed = writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer);`], + }, + { + tag: "0001_broken", + when: 2000, + sql: [`ALTER TABLE ${q("notes")} ADD COLUMN "x" integer;`], + }, + ]); + const result = await applyPendingMigrations(pool, options(fixed)); + expect(result.applied).toEqual(["0001_broken"]); + }); + + test("a migration merged behind the last recorded one fails closed and changes nothing", async () => { + const a = { + tag: "0000_a", + when: 1000, + sql: [`CREATE TABLE ${q("a")} ("id" integer);`], + }; + const c = { + tag: "0002_c", + when: 3000, + sql: [`CREATE TABLE ${q("c")} ("id" integer);`], + }; + const pool = newPool(); + await applyPendingMigrations(pool, options(writeMigrations([a, c]))); + + const merged = writeMigrations([ + a, + { + tag: "0001_b", + when: 2000, + sql: [`CREATE TABLE ${q("b")} ("id" integer);`], + }, + c, + { + tag: "0003_d", + when: 4000, + sql: [`CREATE TABLE ${q("d")} ("id" integer);`], + }, + ]); + const error = await applyPendingMigrations(pool, options(merged)).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(MigrationHistoryError); + expect(error).toMatchObject({ stranded: ["0001_b"] }); + expect(await recorded()).toHaveLength(2); + expect(await columns("d")).toEqual([]); + }); + + test("instances starting together apply each migration once", async () => { + const folder = notes(); + + const results = await Promise.all([ + applyPendingMigrations(newPool(), options(folder)), + applyPendingMigrations(newPool(), options(folder)), + applyPendingMigrations(newPool(), options(folder)), + ]); + + expect(results.flatMap((result) => result.applied).sort()).toEqual([ + "0000_notes", + "0001_body", + ]); + expect(await recorded()).toHaveLength(2); + }); + + test("the wait for another session's migration lock has a limit, and the error names the holder", async () => { + const holder = await admin.connect(); + held.push(holder); + await holder.query("set application_name = 'another-deploy'"); + await holder.query("select pg_advisory_lock($1::bigint)", [ + migrationLockKey(schema), + ]); + const pid = ( + await holder.query<{ pid: number }>("select pg_backend_pid() as pid") + ).rows[0]?.pid; + + const error = await applyPendingMigrations(newPool(), { + ...options(notes()), + lockWaitMs: 300, + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect((error as Error).message).toContain( + `Session pid ${pid} (another-deploy, idle) held the migration lock for over 300ms.`, + ); + }); + + test("a table lock held elsewhere is retried, and the migration applies once it is released", async () => { + const folder = notes(); + const pool = newPool(); + await applyPendingMigrations(pool, { + ...options( + writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer PRIMARY KEY);`], + }, + ]), + ), + }); + const holder = await admin.connect(); + held.push(holder); + await holder.query("begin"); + await holder.query(`lock table ${q("notes")} in access share mode`); + setTimeout(() => void holder.query("commit"), 300); + + const result = await applyPendingMigrations(pool, { + ...options(folder), + lockTimeoutMs: 200, + }); + + expect(result.applied).toEqual(["0001_body"]); + expect(console.warn).toHaveBeenCalled(); + }); + + test("a table lock held through every try fails with 55P03 and leaves the migration unapplied", async () => { + const pool = newPool(); + await applyPendingMigrations(pool, { + ...options( + writeMigrations([ + { + tag: "0000_notes", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer PRIMARY KEY);`], + }, + ]), + ), + }); + const holder = await admin.connect(); + held.push(holder); + await holder.query("begin"); + await holder.query(`lock table ${q("notes")} in access share mode`); + + const error = await applyPendingMigrations(pool, { + ...options(notes()), + lockTimeoutMs: 100, + }).catch((caught: unknown) => caught); + await holder.query("rollback"); + + expect(error).toMatchObject({ failed: "0001_body", code: "55P03" }); + expect(await columns("notes")).toEqual(["id"]); + }); + + test("the session goes back to the pool reset and unlocked", async () => { + const pool = newPool({ max: 1 }); + const before = await pool.query<{ application_name: string }>( + "select current_setting('application_name') as application_name", + ); + + await applyPendingMigrations(pool, options(notes())); + + const after = await pool.query<{ + application_name: string; + lock_timeout: string; + locks: string; + }>( + `select current_setting('application_name') as application_name, + current_setting('lock_timeout') as lock_timeout, + (select count(*) from pg_locks where locktype = 'advisory' and pid = pg_backend_pid())::text as locks`, + ); + expect(after.rows[0]).toEqual({ + application_name: before.rows[0]?.application_name, + lock_timeout: "0", + locks: "0", + }); + }); + + test("an existing schema owned by another role: the error names both roles", async () => { + const suffix = schema.slice(-8); + const owner = `appkit_owner_${suffix}`; + const app = `appkit_app_${suffix}`; + await admin.query(`create role "${owner}" nologin`); + await admin.query(`create role "${app}" login password '${ROLE_PASSWORD}'`); + await admin.query(`create schema "${schema}" authorization "${owner}"`); + try { + const error = await applyPendingMigrations( + newPool({ user: app }), + options(notes()), + ).catch((caught: unknown) => caught); + + expect(error).toMatchObject({ code: "42501" }); + expect((error as Error).message).toContain( + `Schema "${schema}" is owned by "${owner}", but the app connects as "${app}".`, + ); + } finally { + await Promise.all(pools.splice(0).map((pool) => pool.end())); + await admin.query(`drop schema if exists "${schema}" cascade`); + await admin.query(`drop owned by "${app}"`); + await admin.query(`drop role "${app}"`); + await admin.query(`drop role "${owner}"`); + } + }); + + test("a baseline whose tables already exist is adopted, and later migrations apply", async () => { + await admin.query(`create schema "${schema}"`); + await admin.query(`create table ${q("notes")} ("id" integer primary key)`); + const folder = writeMigrations( + [ + { + tag: "0000_baseline", + when: 1000, + sql: [`CREATE TABLE ${q("notes")} ("id" integer PRIMARY KEY);`], + snapshot: { notes: ["id"] }, + }, + { + tag: "0001_body", + when: 2000, + sql: [`ALTER TABLE ${q("notes")} ADD COLUMN "body" text;`], + }, + ], + schema, + ); + + const result = await applyPendingMigrations(newPool(), options(folder)); + + expect(result).toMatchObject({ + adopted: ["0000_baseline"], + applied: ["0001_body"], + }); + expect(await columns("notes")).toEqual(["id", "body"]); + }); +}); diff --git a/packages/appkit/src/database/migrations/tests/apply.test.ts b/packages/appkit/src/database/migrations/tests/apply.test.ts index 4fc178d14..6892e5df0 100644 --- a/packages/appkit/src/database/migrations/tests/apply.test.ts +++ b/packages/appkit/src/database/migrations/tests/apply.test.ts @@ -1,6 +1,4 @@ -import { createHash } from "node:crypto"; import fs from "node:fs"; -import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, test, vi } from "vitest"; @@ -9,103 +7,52 @@ import { applyPendingMigrations, MigrationApplyError, MigrationBaselineMismatchError, + MigrationHistoryError, migrationLockKey, } from "../apply"; -import { type Journal, MigrationFilesError } from "../paths"; - -const folders: string[] = []; - -/** Write a v7 journal with one SQL file per entry, the way drizzle-kit lays them out. */ -function migrations( - entries: Array<{ - tag: string; - when: number; - sql: string[]; - /** Tables (name → columns) for meta/NNNN_snapshot.json. */ - snapshot?: Record; - }>, -) { - const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); - folders.push(folder); - fs.mkdirSync(path.join(folder, "meta")); - const journal: Journal = { - version: "7", - dialect: "postgresql", - entries: entries.map((entry, idx) => ({ - idx, - version: "7", - when: entry.when, - tag: entry.tag, - breakpoints: true, - })), - }; - fs.writeFileSync( - path.join(folder, "meta/_journal.json"), - JSON.stringify(journal), - ); - entries.forEach((entry, idx) => { - fs.writeFileSync( - path.join(folder, `${entry.tag}.sql`), - entry.sql.join("--> statement-breakpoint\n"), - ); - if (entry.snapshot) { - const tables = Object.fromEntries( - Object.entries(entry.snapshot).map(([name, columns]) => [ - `app.${name}`, - { - name, - schema: "app", - columns: Object.fromEntries(columns.map((c) => [c, { name: c }])), - }, - ]), - ); - fs.writeFileSync( - path.join( - folder, - "meta", - `${String(idx).padStart(4, "0")}_snapshot.json`, - ), - JSON.stringify({ - version: "7", - dialect: "postgresql", - tables, - enums: {}, - }), - ); - } - }); - return folder; -} +import { MigrationFilesError } from "../paths"; +import { + hashOf, + removeMigrationFolders, + writeMigrations as migrations, +} from "./fixtures"; afterEach(() => { vi.restoreAllMocks(); - for (const folder of folders.splice(0)) { - fs.rmSync(folder, { recursive: true, force: true }); - } + removeMigrationFolders(); }); +interface JournalRow { + hash: string; + created_at: number | null; +} + interface FakeState { - journalTableExists: boolean; - lastCreatedAt: number | null; + /** Rows of the journal table; null while the table does not exist. */ + recorded: JournalRow[] | null; failOn?: RegExp; /** SQLSTATE of the failOn error; defaults to 42P07 (duplicate_table). */ failCode?: string; + /** How many failOn matches fail before one succeeds; all of them when unset. */ + failTimes?: number; failCleanupOn?: RegExp; /** pg_catalog (table, column) pairs the baseline check sees. */ catalog?: Array<[string, string]>; - /** Another instance holds the migration lock, so the first try fails. */ + /** Another session holds the migration lock. */ lockHeld?: boolean; + lockHolder?: { pid: number; application_name: string; state: string }; + ownership?: { owner: string | null; current_user: string }; } /** - * A pg PoolClient that records every statement. Its class name must not contain - * "Pool": drizzle-orm would take it for a pool and call connect() on it. + * A pg PoolClient that records every statement and keeps journal inserts + * only when their transaction commits. */ class RecordingClient { readonly statements: string[] = []; - /** Parameters of each journal insert. */ - readonly recorded: unknown[][] = []; released: { error: unknown } | undefined; + private uncommitted: JournalRow[] | undefined; + private failures = 0; constructor(private readonly state: FakeState) {} @@ -116,17 +63,43 @@ class RecordingClient { const text = (typeof input === "string" ? input : input.text).trim(); const params = values ?? (typeof input === "string" ? [] : input.values); this.statements.push(text); - if (this.state.failOn?.test(text)) { - throw Object.assign(new Error("relation already exists"), { + if ( + this.state.failOn?.test(text) && + (this.state.failTimes === undefined || + this.failures < this.state.failTimes) + ) { + this.failures++; + throw Object.assign(new Error("statement failed"), { code: this.state.failCode ?? "42P07", }); } if (this.state.failCleanupOn?.test(text)) { throw new Error("connection lost"); } + if (text === "begin") this.uncommitted = []; + if (text === "commit") { + if (this.uncommitted?.length) { + this.state.recorded = [ + ...(this.state.recorded ?? []), + ...this.uncommitted, + ]; + } + this.uncommitted = undefined; + } + if (text === "rollback") this.uncommitted = undefined; if (text.includes("pg_try_advisory_lock(")) { return { rows: [{ locked: !this.state.lockHeld }] }; } + if (text.includes("pg_catalog.pg_locks")) { + return { rows: this.state.lockHolder ? [this.state.lockHolder] : [] }; + } + if (text.includes("pg_get_userbyid")) { + return { + rows: [ + this.state.ownership ?? { owner: "app-sp", current_user: "app-sp" }, + ], + }; + } if (text.includes("pg_catalog.pg_attribute")) { return { rows: (this.state.catalog ?? []).map(([table_name, column_name]) => ({ @@ -136,23 +109,16 @@ class RecordingClient { }; } if (text.includes("pg_catalog.pg_class")) { - return { rows: this.state.journalTableExists ? [{ one: 1 }] : [] }; - } - if (text.startsWith(`insert into "app"."__drizzle_migrations"`)) { - this.recorded.push(params ?? []); - this.state.journalTableExists = true; - this.state.lastCreatedAt = Number(params?.[1]); + return { rows: this.state.recorded ? [{ one: 1 }] : [] }; } - if (text.startsWith("select max(created_at)")) { - return { rows: [{ last: this.state.lastCreatedAt }] }; + if (text.startsWith("select hash, created_at")) { + return { rows: this.state.recorded ?? [] }; } - if (text.includes("order by created_at desc limit 1")) { - return { - rows: - this.state.lastCreatedAt === null - ? [] - : [{ id: 1, hash: "h", created_at: this.state.lastCreatedAt }], - }; + if (text.startsWith(`insert into "app"."__drizzle_migrations"`)) { + this.uncommitted?.push({ + hash: String(params?.[0]), + created_at: Number(params?.[1]), + }); } return { rows: [] }; } @@ -167,6 +133,7 @@ function fakePool(state: FakeState) { let connects = 0; return { client, + state, connects: () => connects, pool: { connect: async () => { @@ -194,213 +161,490 @@ const twoMigrations = () => }, ]); +const options = (folder: string) => ({ + migrationsFolder: folder, + schemaName: "app", +}); + /** Collapse the recorded statements into readable steps. */ function steps(statements: readonly string[]): string[] { return statements.map((statement) => { if (statement.includes("pg_try_advisory_lock(")) return "try lock"; - if (statement.includes("pg_advisory_lock(")) return "lock"; if (statement.includes("pg_advisory_unlock(")) return "unlock"; + if (statement.includes("pg_catalog.pg_locks")) return "lock holder?"; + if (statement.includes("pg_get_userbyid")) return "schema owner?"; if (statement.includes("pg_catalog.pg_attribute")) return "baseline tables?"; if (statement.includes("pg_catalog.pg_class")) return "journal table?"; - if (statement.startsWith("select max(created_at)")) return "last applied?"; + if (statement.startsWith("select hash, created_at")) return "recorded?"; + if (statement.startsWith(`CREATE SCHEMA IF NOT EXISTS`)) + return "create schema"; + if (statement.startsWith(`CREATE TABLE IF NOT EXISTS "app"."__drizzle`)) + return "create journal"; + if (statement.startsWith(`insert into "app"."__drizzle_migrations"`)) + return "record"; return statement.replace(/\s+/g, " "); }); } +const SESSION_SETUP = [ + "SET statement_timeout = 0", + "SET application_name = 'appkit-migrate'", + "SET client_connection_check_interval = '10s'", + "try lock", + "SET lock_timeout = '5000ms'", +]; + +const SESSION_CLEANUP = [ + "unlock", + "RESET lock_timeout", + "RESET client_connection_check_interval", + "RESET application_name", + "RESET statement_timeout", +]; + describe("applyPendingMigrations", () => { - test("applies every pending migration as the connecting identity under the advisory lock, then unlocks and releases", async () => { + test("reads without the lock, then applies each pending migration in its own transaction under it", async () => { const folder = twoMigrations(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - }); + const { pool, client, state } = fakePool({ recorded: null }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); - expect(result.applied).toEqual(["0000_migration", "0001_migration"]); + expect(result).toEqual({ + applied: ["0000_migration", "0001_migration"], + adopted: [], + edited: [], + unknown: 0, + lockWaitMs: expect.any(Number), + }); expect(steps(client.statements)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "SET lock_timeout = '30s'", "journal table?", - // drizzle-orm's migrator as the connecting identity (no SET ROLE), journal in the app schema: - `CREATE SCHEMA IF NOT EXISTS "app"`, - `CREATE TABLE IF NOT EXISTS "app"."__drizzle_migrations" ( id SERIAL PRIMARY KEY, hash text NOT NULL, created_at bigint )`, - `select id, hash, created_at from "app"."__drizzle_migrations" order by created_at desc limit 1`, + ...SESSION_SETUP, + "journal table?", "begin", + "create schema", + "create journal", `CREATE TABLE "app"."notes" ("id" integer);`, - `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + "record", + "commit", + "begin", + "create schema", + "create journal", `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, - `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, + "record", "commit", - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", + ...SESSION_CLEANUP, + ]); + // The rows drizzle-orm's migrator would have written. + expect(state.recorded).toEqual([ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, ]); expect(client.released).toEqual({ error: undefined }); }); - test("applies only entries newer than the last recorded migration", async () => { + test("applies only migrations newer than the last recorded one", async () => { const folder = twoMigrations(); const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, + recorded: [{ hash: hashOf(folder, "0000_migration"), created_at: 1000 }], }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); expect(result.applied).toEqual(["0001_migration"]); expect(client.statements).not.toContain( `CREATE TABLE "app"."notes" ("id" integer);`, ); - expect(client.statements).toContain( - `ALTER TABLE "app"."notes" ADD COLUMN "body" text;`, - ); }); - test("with nothing pending, issues no migration SQL", async () => { + test("with nothing pending, it only reads: no lock, no session settings", async () => { const folder = twoMigrations(); const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 2000, + recorded: [ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, + ], }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); expect(result.applied).toEqual([]); - expect(steps(client.statements)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "SET lock_timeout = '30s'", - "journal table?", - "last applied?", - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); + expect(steps(client.statements)).toEqual(["journal table?", "recorded?"]); expect(client.released).toEqual({ error: undefined }); }); + test("another instance applied everything while this one waited: nothing runs twice", async () => { + const folder = twoMigrations(); + const fake = fakePool({ recorded: null }); + const query = fake.client.query.bind(fake.client); + vi.spyOn(fake.client, "query").mockImplementation(async (input, values) => { + const result = await query(input, values); + if (String(input).includes("pg_try_advisory_lock(")) { + fake.state.recorded = [ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, + ]; + } + return result; + }); + + const result = await applyPendingMigrations(fake.pool, options(folder)); + + expect(result.applied).toEqual([]); + expect(fake.client.statements).not.toContain("begin"); + }); + test("without a journal it never connects", async () => { - const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); - folders.push(folder); - const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); + const folder = migrations([]); + fs.rmSync(path.join(folder, "meta"), { recursive: true }); + const fake = fakePool({ recorded: null }); await expect( - applyPendingMigrations(fake.pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).resolves.toEqual({ applied: [], adopted: [] }); + applyPendingMigrations(fake.pool, options(folder)), + ).resolves.toEqual({ + applied: [], + adopted: [], + edited: [], + unknown: 0, + lockWaitMs: 0, + }); expect(fake.connects()).toBe(0); }); - test("a failing statement rolls back all pending migrations and still unlocks and releases", async () => { + test("a failing migration rolls back alone: the ones before it stay applied", async () => { const folder = twoMigrations(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, + const { pool, client, state } = fakePool({ + recorded: null, failOn: /ADD COLUMN "title"/, }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationApplyError); expect(error).toMatchObject({ - pending: ["0000_migration", "0001_migration"], + failed: "0001_migration", + applied: ["0000_migration"], + pending: ["0001_migration"], code: "42P07", }); expect((error as Error).message).toBe( - "Applying migrations 0000_migration, 0001_migration failed (SQLSTATE 42P07); none were applied.", + "Migration 0001_migration failed (SQLSTATE 42P07) and was rolled back. Applied before the failure: 0000_migration. Not applied: 0001_migration.", ); - expect(steps(client.statements).slice(-5)).toEqual([ - `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, + expect(state.recorded).toEqual([ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + ]); + expect(steps(client.statements).slice(-6)).toEqual([ "rollback", - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", + ...SESSION_CLEANUP, ]); expect(client.released).toEqual({ error: undefined }); }); - test("an ownership failure (42501) names the branch-owner rule and the fix", async () => { + test("a migration older than the last recorded one, never applied, fails closed before taking the lock", async () => { + const folder = migrations([ + { tag: "0000_a", when: 1000, sql: [`SELECT 1;`] }, + { tag: "0001_parallel", when: 1500, sql: [`SELECT 2;`] }, + { tag: "0002_b", when: 2000, sql: [`SELECT 3;`] }, + ]); + const { pool, client } = fakePool({ + recorded: [ + { hash: hashOf(folder, "0000_a"), created_at: 1000 }, + { hash: hashOf(folder, "0002_b"), created_at: 2000 }, + ], + }); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(MigrationHistoryError); + expect(error).toMatchObject({ stranded: ["0001_parallel"] }); + expect((error as Error).message).toMatch( + /^Migration 0001_parallel is older than 0002_b, which schema "app" already recorded, and was never applied/, + ); + expect(steps(client.statements)).toEqual(["journal table?", "recorded?"]); + }); + + test("a stranded migration also stops the pending ones after it", async () => { + const folder = migrations([ + { tag: "0000_a", when: 1000, sql: [`SELECT 1;`] }, + { tag: "0001_parallel", when: 1500, sql: [`SELECT 2;`] }, + { tag: "0002_b", when: 2000, sql: [`SELECT 3;`] }, + { tag: "0003_c", when: 3000, sql: [`SELECT 4;`] }, + ]); + const { pool, client } = fakePool({ + recorded: [ + { hash: hashOf(folder, "0000_a"), created_at: 1000 }, + { hash: hashOf(folder, "0002_b"), created_at: 2000 }, + ], + }); + + await expect( + applyPendingMigrations(pool, options(folder)), + ).rejects.toBeInstanceOf(MigrationHistoryError); + expect(client.statements).not.toContain("SELECT 4;"); + }); + + test("an applied migration whose file changed is logged, and its change does not run", async () => { + const folder = twoMigrations(); + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); + const { pool } = fakePool({ + recorded: [ + { hash: "the-hash-before-the-edit", created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, + ], + }); + + const result = await applyPendingMigrations(pool, options(folder)); + + expect(result).toMatchObject({ applied: [], edited: ["0000_migration"] }); + expect(warn.mock.calls.flat().map(String).join(" ")).toContain( + "Migrations 0000_migration changed after schema app applied them", + ); + }); + + test("recorded migrations that are not committed here are logged", async () => { const folder = twoMigrations(); + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); const { pool } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, + recorded: [ + { hash: hashOf(folder, "0000_migration"), created_at: 1000 }, + { hash: hashOf(folder, "0001_migration"), created_at: 2000 }, + { hash: "from-a-newer-release", created_at: 3000 }, + ], + }); + + const result = await applyPendingMigrations(pool, options(folder)); + + expect(result).toMatchObject({ applied: [], unknown: 1 }); + expect(warn.mock.calls.flat().map(String).join(" ")).toContain( + "Schema app recorded 1 migration(s) that are not committed here", + ); + }); + + test("a table lock not granted in time is retried, then fails with the hint", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + recorded: [{ hash: hashOf(folder, "0000_migration"), created_at: 1000 }], failOn: /ADD COLUMN "body"/, - failCode: "42501", + failCode: "55P03", }); const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", + ...options(folder), + lockTimeoutMs: 1, }).catch((caught: unknown) => caught); + expect(client.statements.filter((s) => s === "rollback")).toHaveLength(3); + expect((error as Error).message).toBe( + "Migration 0001_migration failed (SQLSTATE 55P03) and was rolled back. Not applied: 0001_migration. " + + "Another session held a lock on a table this migration changes for over 1ms, 3 times. Deploy again once the long-running query or transaction finishes.", + ); + }); + + test("a table lock granted on the retry applies the migration", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + recorded: [{ hash: hashOf(folder, "0000_migration"), created_at: 1000 }], + failOn: /ADD COLUMN "body"/, + failCode: "55P03", + failTimes: 1, + }); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + + const result = await applyPendingMigrations(pool, { + ...options(folder), + lockTimeoutMs: 1, + }); + + expect(result.applied).toEqual(["0001_migration"]); + }); + + test("the wait for another session's migration lock has a limit, and the error names the holder", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + recorded: null, + lockHeld: true, + lockHolder: { + pid: 42, + application_name: "appkit-migrate", + state: "active", + }, + }); + + const error = await applyPendingMigrations(pool, { + ...options(folder), + lockWaitMs: 0, + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect((error as Error).message).toBe( + "Waiting for the migration lock failed. Not applied: 0000_migration, 0001_migration. " + + "Session pid 42 (appkit-migrate, active) held the migration lock for over 0ms. Deploy again once it finishes; if it is stuck, end it with pg_terminate_backend.", + ); + // Never locked, so never unlocked; the session settings are still reset. + expect(steps(client.statements).slice(-3)).toEqual([ + "RESET client_connection_check_interval", + "RESET application_name", + "RESET statement_timeout", + ]); + }); + + test("a server that cannot check client connections still migrates", async () => { + const folder = twoMigrations(); + const { pool, client } = fakePool({ + recorded: null, + failOn: /client_connection_check_interval/, + failCode: "22023", + }); + + const result = await applyPendingMigrations(pool, options(folder)); + + expect(result.applied).toHaveLength(2); + expect(client.statements).not.toContain( + "RESET client_connection_check_interval", + ); + }); + + test("an ownership failure (42501) names the schema owner and the connecting role", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + recorded: null, + failOn: /CREATE TABLE "app"."notes"/, + failCode: "42501", + ownership: { owner: "old-app-sp", current_user: "new-app-sp" }, + }); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + expect(error).toMatchObject({ code: "42501" }); + expect((error as Error).message).toContain( + `Schema "app" is owned by "old-app-sp", but the app connects as "new-app-sp".`, + ); + }); + + test("reading the recorded migrations fails (42501): the hint, and nothing applied", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + recorded: [], + failOn: /^select hash, created_at/, + failCode: "42501", + ownership: { owner: "someone-else", current_user: "app-sp" }, + }); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toMatchObject({ pending: [], code: "42501" }); expect((error as Error).message).toMatch( - /^Applying migrations 0000_migration, 0001_migration failed \(SQLSTATE 42501\); none were applied\. The connecting role lacks a privilege this migration needs: /, + /^Reading the recorded migrations failed \(SQLSTATE 42501\)\. No migration was applied\. Schema "app" is owned by "someone-else"/, + ); + }); + + test("a connection failure carries the system error code", async () => { + const folder = twoMigrations(); + const pool = { + connect: async () => { + throw Object.assign(new Error("connect ECONNREFUSED"), { + code: "ECONNREFUSED", + }); + }, + }; + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(MigrationApplyError); + expect(error).toMatchObject({ code: undefined }); + expect((error as Error).message).toBe( + "Connecting to the database failed (ECONNREFUSED). No migration was applied.", + ); + }); + + test("a Node system error is not reported as a SQLSTATE", async () => { + const folder = twoMigrations(); + const { pool } = fakePool({ + recorded: [{ hash: hashOf(folder, "0000_migration"), created_at: 1000 }], + failOn: /ADD COLUMN "body"/, + failCode: "EPIPE", + }); + + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); + + expect(error).toMatchObject({ code: undefined }); + expect((error as Error).message).toBe( + "Migration 0001_migration failed (EPIPE) and was rolled back. Not applied: 0001_migration.", ); }); test("a failed unlock destroys the connection instead of returning it to the pool", async () => { const folder = twoMigrations(); const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, + recorded: null, failCleanupOn: /pg_advisory_unlock/, }); - await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + await applyPendingMigrations(pool, options(folder)); // The remaining cleanup still runs. - expect(steps(client.statements).slice(-3)).toEqual([ - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); + expect(steps(client.statements).slice(-5)).toEqual(SESSION_CLEANUP); expect(client.released?.error).toBeInstanceOf(Error); }); - test("a failure before the lock is held releases without unlocking", async () => { + test("a missing .sql file fails before connecting, naming the file", async () => { const folder = twoMigrations(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - failOn: /pg_try_advisory_lock/, - }); + fs.rmSync(path.join(folder, "0001_migration.sql")); + const fake = fakePool({ recorded: null }); - await expect( - applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).rejects.toThrow("relation already exists"); - expect(steps(client.statements)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); - expect(client.released).toEqual({ error: undefined }); + const error = await applyPendingMigrations( + fake.pool, + options(folder), + ).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch( + /^Missing .*0001_migration\.sql, listed in meta\/_journal\.json\. Restore it from version control\.$/, + ); + expect(fake.connects()).toBe(0); }); + + test.each([ + [ + "not JSON", + "{", + /^Cannot read .*_journal\.json: .*Restore it from version control\.$/, + ], + [ + "another dialect", + JSON.stringify({ version: "7", dialect: "mysql", entries: [] }), + /_journal\.json is not a drizzle-kit PostgreSQL migration journal$/, + ], + ])( + "a journal that is %s fails before connecting, naming the file", + async (_label, contents, message) => { + const folder = twoMigrations(); + fs.writeFileSync(path.join(folder, "meta/_journal.json"), contents); + const fake = fakePool({ recorded: null }); + + const error = await applyPendingMigrations( + fake.pool, + options(folder), + ).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(MigrationFilesError); + expect((error as Error).message).toMatch(message); + expect(fake.connects()).toBe(0); + }, + ); }); describe("migrationLockKey", () => { @@ -436,21 +680,20 @@ const baselineThenMigration = () => }, ]); +const BASELINE_CATALOG: Array<[string, string]> = [ + ["notes", "id"], + ["notes", "body"], + ["tags", "id"], +]; + describe("applyPendingMigrations — baseline adoption", () => { test("none of the baseline's tables exist: the baseline runs like any migration", async () => { const folder = baselineThenMigration(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [], - }); + const { pool, client } = fakePool({ recorded: null, catalog: [] }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); - expect(result).toEqual({ + expect(result).toMatchObject({ applied: ["0000_baseline", "0001_migration"], adopted: [], }); @@ -459,22 +702,14 @@ describe("applyPendingMigrations — baseline adoption", () => { test("every baseline table and column exists: the baseline is recorded, not run, and later migrations apply", async () => { const folder = baselineThenMigration(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], + const { pool, client, state } = fakePool({ + recorded: null, + catalog: BASELINE_CATALOG, }); - const result = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); + const result = await applyPendingMigrations(pool, options(folder)); - expect(result).toEqual({ + expect(result).toMatchObject({ applied: ["0001_migration"], adopted: ["0000_baseline"], }); @@ -483,27 +718,10 @@ describe("applyPendingMigrations — baseline adoption", () => { `ALTER TABLE "app"."notes" ADD COLUMN "title" text;`, ); // The adopted row carries exactly what drizzle-orm's migrator would have written. - const baselineFile = fs.readFileSync( - path.join(folder, "0000_baseline.sql"), - "utf-8", - ); - expect(client.recorded[0]).toEqual([ - createHash("sha256").update(baselineFile).digest("hex"), - 1000, - ]); - expect(steps(client.statements).slice(0, 10)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "SET lock_timeout = '30s'", - "journal table?", - "baseline tables?", - "begin", - `CREATE SCHEMA IF NOT EXISTS "app"`, - `CREATE TABLE IF NOT EXISTS "app"."__drizzle_migrations" ( id SERIAL PRIMARY KEY, hash text NOT NULL, created_at bigint )`, - `insert into "app"."__drizzle_migrations" ("hash", "created_at") values($1, $2)`, - "commit", - ]); - expect(client.released).toEqual({ error: undefined }); + expect(state.recorded?.[0]).toEqual({ + hash: hashOf(folder, "0000_baseline"), + created_at: 1000, + }); }); test("adopting a lone baseline applies nothing else", async () => { @@ -515,41 +733,23 @@ describe("applyPendingMigrations — baseline adoption", () => { snapshot: { notes: ["id", "body"], tags: ["id"] }, }, ]); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], - }); + const { pool } = fakePool({ recorded: null, catalog: BASELINE_CATALOG }); await expect( - applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).resolves.toEqual({ applied: [], adopted: ["0000_baseline"] }); - expect(client.statements.slice(-3)).toEqual([ - expect.stringContaining("pg_advisory_unlock"), - "RESET lock_timeout", - "RESET statement_timeout", - ]); + applyPendingMigrations(pool, options(folder)), + ).resolves.toMatchObject({ applied: [], adopted: ["0000_baseline"] }); }); test("only part of the baseline exists: MigrationBaselineMismatchError, nothing recorded or run", async () => { const folder = baselineThenMigration(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, + const { pool, client, state } = fakePool({ + recorded: null, catalog: [["notes", "id"]], }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationBaselineMismatchError); expect((error as MigrationBaselineMismatchError).problems).toEqual([ @@ -559,40 +759,34 @@ describe("applyPendingMigrations — baseline adoption", () => { expect((error as Error).message).toMatch( /^Cannot adopt 0000_baseline: schema "app" has no recorded migrations and only part of the baseline: /, ); - expect(client.recorded).toEqual([]); + expect(state.recorded).toBeNull(); expect(client.statements).not.toContain("begin"); - expect(steps(client.statements).slice(-3)).toEqual([ - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); expect(client.released).toEqual({ error: undefined }); }); test("recording the adopted baseline fails: MigrationApplyError with the SQLSTATE and the ownership hint", async () => { const folder = baselineThenMigration(); const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], + recorded: null, + catalog: BASELINE_CATALOG, // The schema belongs to another role, so the journal table cannot be created in it. failOn: /^CREATE TABLE IF NOT EXISTS "app"\."__drizzle_migrations"/, failCode: "42501", + ownership: { owner: "someone-else", current_user: "app-sp" }, }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationApplyError); - expect(error).toMatchObject({ pending: ["0000_baseline"], code: "42501" }); + expect(error).toMatchObject({ + failed: "0000_baseline", + pending: ["0000_baseline", "0001_migration"], + code: "42501", + }); expect((error as Error).message).toContain( - "On the deploy branch that role is the app's service principal", + `Schema "app" is owned by "someone-else"`, ); expect(client.statements).toContain("rollback"); expect(client.statements).not.toContain( @@ -603,40 +797,25 @@ describe("applyPendingMigrations — baseline adoption", () => { test("with a recorded migration, the baseline is never adopted", async () => { const folder = baselineThenMigration(); const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, + recorded: [{ hash: hashOf(folder, "0000_baseline"), created_at: 1000 }], catalog: [["notes", "id"]], }); await expect( - applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).resolves.toEqual({ applied: ["0001_migration"], adopted: [] }); + applyPendingMigrations(pool, options(folder)), + ).resolves.toMatchObject({ applied: ["0001_migration"], adopted: [] }); expect( client.statements.some((s) => s.includes("pg_catalog.pg_attribute")), ).toBe(false); }); - test("an empty journal table, left by a failed first deploy, still adopts the baseline", async () => { + test("an empty journal table still adopts the baseline", async () => { const folder = baselineThenMigration(); - const { pool } = fakePool({ - journalTableExists: true, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], - }); + const { pool } = fakePool({ recorded: [], catalog: BASELINE_CATALOG }); await expect( - applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }), - ).resolves.toEqual({ + applyPendingMigrations(pool, options(folder)), + ).resolves.toMatchObject({ applied: ["0001_migration"], adopted: ["0000_baseline"], }); @@ -644,21 +823,15 @@ describe("applyPendingMigrations — baseline adoption", () => { test("a migration failing after the adoption says the baseline stays recorded", async () => { const folder = baselineThenMigration(); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - catalog: [ - ["notes", "id"], - ["notes", "body"], - ["tags", "id"], - ], + const { pool, state } = fakePool({ + recorded: null, + catalog: BASELINE_CATALOG, failOn: /ADD COLUMN "title"/, }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationApplyError); expect(error).toMatchObject({ @@ -666,23 +839,19 @@ describe("applyPendingMigrations — baseline adoption", () => { adopted: ["0000_baseline"], }); expect((error as Error).message).toBe( - "Applying migration 0001_migration failed (SQLSTATE 42P07); none were applied. 0000_baseline stays recorded as applied: its tables already existed.", + "Migration 0001_migration failed (SQLSTATE 42P07) and was rolled back. 0000_baseline stays recorded as applied: its tables already existed. Not applied: 0001_migration.", ); - expect(client.recorded).toHaveLength(1); + expect(state.recorded).toHaveLength(1); }); test("a missing baseline snapshot names the file", async () => { const folder = baselineThenMigration(); fs.rmSync(path.join(folder, "meta/0000_snapshot.json")); - const { pool, client } = fakePool({ - journalTableExists: false, - lastCreatedAt: null, - }); + const { pool, client } = fakePool({ recorded: null }); - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); + const error = await applyPendingMigrations(pool, options(folder)).catch( + (caught: unknown) => caught, + ); expect(error).toBeInstanceOf(MigrationFilesError); expect((error as Error).message).toMatch( @@ -691,142 +860,3 @@ describe("applyPendingMigrations — baseline adoption", () => { expect(client.released).toEqual({ error: undefined }); }); }); - -describe("applyPendingMigrations — failures name their cause", () => { - test("reading the recorded migrations fails (42501): the hint, and nothing applied", async () => { - const folder = twoMigrations(); - const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, - // The journal table belongs to another role. - failOn: /^select max\(created_at\)/, - failCode: "42501", - }); - - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(MigrationApplyError); - expect(error).toMatchObject({ pending: [], code: "42501" }); - expect((error as Error).message).toMatch( - /^Reading the recorded migrations in __drizzle_migrations failed \(SQLSTATE 42501\); none were applied\. The connecting role lacks a privilege/, - ); - expect(steps(client.statements).slice(-3)).toEqual([ - "unlock", - "RESET lock_timeout", - "RESET statement_timeout", - ]); - }); - - test("a table lock held past the lock timeout says to deploy again", async () => { - const folder = twoMigrations(); - const { pool } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, - failOn: /ADD COLUMN "body"/, - failCode: "55P03", - }); - - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect((error as Error).message).toBe( - "Applying migration 0001_migration failed (SQLSTATE 55P03); none were applied. Another session held a lock on a table this migration changes for over 30s; deploy again once it finishes.", - ); - }); - - test("a Node system error is not reported as a SQLSTATE", async () => { - const folder = twoMigrations(); - const { pool } = fakePool({ - journalTableExists: true, - lastCreatedAt: 1000, - failOn: /ADD COLUMN "body"/, - failCode: "EPIPE", - }); - - const error = await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect(error).toMatchObject({ code: undefined }); - expect((error as Error).message).toBe( - "Applying migration 0001_migration failed; none were applied.", - ); - }); - - test("a missing .sql file fails before connecting, naming the file", async () => { - const folder = twoMigrations(); - fs.rmSync(path.join(folder, "0001_migration.sql")); - const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); - - const error = await applyPendingMigrations(fake.pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(MigrationFilesError); - expect((error as Error).message).toMatch( - /^Missing .*0001_migration\.sql, listed in meta\/_journal\.json\. Restore it from version control\.$/, - ); - expect(fake.connects()).toBe(0); - }); - - test.each([ - [ - "not JSON", - "{", - /^Cannot read .*_journal\.json: .*Restore it from version control\.$/, - ], - [ - "another dialect", - JSON.stringify({ version: "7", dialect: "mysql", entries: [] }), - /_journal\.json is not a drizzle-kit PostgreSQL migration journal$/, - ], - ])( - "a journal that is %s fails before connecting, naming the file", - async (_label, contents, message) => { - const folder = twoMigrations(); - fs.writeFileSync(path.join(folder, "meta/_journal.json"), contents); - const fake = fakePool({ journalTableExists: false, lastCreatedAt: null }); - - const error = await applyPendingMigrations(fake.pool, { - migrationsFolder: folder, - schemaName: "app", - }).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(MigrationFilesError); - expect((error as Error).message).toMatch(message); - expect(fake.connects()).toBe(0); - }, - ); - - test("waits for another instance's lock, and says so", async () => { - const folder = twoMigrations(); - const log = vi.spyOn(console, "log").mockImplementation(() => undefined); - const { pool, client } = fakePool({ - journalTableExists: true, - lastCreatedAt: 2000, - lockHeld: true, - }); - - await applyPendingMigrations(pool, { - migrationsFolder: folder, - schemaName: "app", - }); - - expect(steps(client.statements).slice(0, 4)).toEqual([ - "SET statement_timeout = 0", - "try lock", - "lock", - "SET lock_timeout = '30s'", - ]); - expect(log.mock.calls.flat().map(String).join(" ")).toContain( - "Waiting for another instance to finish migrating schema app", - ); - }); -}); diff --git a/packages/appkit/src/database/migrations/tests/fixtures.ts b/packages/appkit/src/database/migrations/tests/fixtures.ts new file mode 100644 index 000000000..33d5d1baa --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/fixtures.ts @@ -0,0 +1,86 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import type { Journal } from "../paths"; + +interface MigrationFixture { + tag: string; + when: number; + sql: string[]; + /** Tables (name → columns) for meta/NNNN_snapshot.json. */ + snapshot?: Record; +} + +const folders: string[] = []; + +/** Write a v7 journal with one SQL file per entry, the way drizzle-kit lays them out. */ +export function writeMigrations( + entries: MigrationFixture[], + schemaName = "app", +): string { + const folder = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-apply-")); + folders.push(folder); + fs.mkdirSync(path.join(folder, "meta")); + const journal: Journal = { + version: "7", + dialect: "postgresql", + entries: entries.map((entry, idx) => ({ + idx, + version: "7", + when: entry.when, + tag: entry.tag, + breakpoints: true, + })), + }; + fs.writeFileSync( + path.join(folder, "meta/_journal.json"), + JSON.stringify(journal), + ); + entries.forEach((entry, idx) => { + fs.writeFileSync( + path.join(folder, `${entry.tag}.sql`), + entry.sql.join("--> statement-breakpoint\n"), + ); + if (entry.snapshot) { + const tables = Object.fromEntries( + Object.entries(entry.snapshot).map(([name, columns]) => [ + `${schemaName}.${name}`, + { + name, + schema: schemaName, + columns: Object.fromEntries(columns.map((c) => [c, { name: c }])), + }, + ]), + ); + fs.writeFileSync( + path.join( + folder, + "meta", + `${String(idx).padStart(4, "0")}_snapshot.json`, + ), + JSON.stringify({ + version: "7", + dialect: "postgresql", + tables, + enums: {}, + }), + ); + } + }); + return folder; +} + +/** The hash drizzle-orm's migrator records for a migration file. */ +export function hashOf(folder: string, tag: string): string { + return createHash("sha256") + .update(fs.readFileSync(path.join(folder, `${tag}.sql`), "utf-8")) + .digest("hex"); +} + +export function removeMigrationFolders(): void { + for (const folder of folders.splice(0)) { + fs.rmSync(folder, { recursive: true, force: true }); + } +} diff --git a/packages/appkit/src/database/migrations/tests/history.test.ts b/packages/appkit/src/database/migrations/tests/history.test.ts new file mode 100644 index 000000000..04da4ac6e --- /dev/null +++ b/packages/appkit/src/database/migrations/tests/history.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, test } from "vitest"; + +import { type MigrationFile, reconcileMigrations } from "../history"; + +const file = ( + tag: string, + when: number, + hash = `hash-${tag}`, +): MigrationFile => ({ + tag, + idx: 0, + when, + hash, + statements: [], +}); + +const tags = (files: readonly MigrationFile[]) => files.map((f) => f.tag); + +describe("reconcileMigrations", () => { + const files = [ + file("0000_a", 1000), + file("0001_b", 2000), + file("0002_c", 3000), + ]; + + test("nothing recorded: every migration is pending", () => { + const history = reconcileMigrations(files, []); + expect(tags(history.pending)).toEqual(["0000_a", "0001_b", "0002_c"]); + expect(history.lastRecorded).toBeNull(); + }); + + test("newer than the last recorded migration: pending", () => { + const history = reconcileMigrations(files, [ + { hash: "hash-0000_a", createdAt: 1000 }, + ]); + expect(tags(history.pending)).toEqual(["0001_b", "0002_c"]); + expect(history.stranded).toEqual([]); + expect(history.lastRecorded).toBe(1000); + }); + + test("older than the last recorded migration and never recorded: stranded", () => { + const history = reconcileMigrations(files, [ + { hash: "hash-0000_a", createdAt: 1000 }, + { hash: "hash-0002_c", createdAt: 3000 }, + ]); + expect(tags(history.stranded)).toEqual(["0001_b"]); + expect(history.pending).toEqual([]); + }); + + test("recorded with another hash: edited", () => { + const history = reconcileMigrations(files, [ + { hash: "before-the-edit", createdAt: 1000 }, + { hash: "hash-0001_b", createdAt: 2000 }, + { hash: "hash-0002_c", createdAt: 3000 }, + ]); + expect(tags(history.edited)).toEqual(["0000_a"]); + expect(history.pending).toEqual([]); + }); + + test("recorded rows no committed migration matches: unknown, and they still set the last recorded", () => { + const unknownRow = { hash: "newer-release", createdAt: 4000 }; + const nullRow = { hash: "no-timestamp", createdAt: null }; + const history = reconcileMigrations(files, [ + { hash: "hash-0000_a", createdAt: 1000 }, + { hash: "hash-0001_b", createdAt: 2000 }, + { hash: "hash-0002_c", createdAt: 3000 }, + unknownRow, + nullRow, + ]); + expect(history.unknown).toEqual([unknownRow, nullRow]); + expect(history.lastRecorded).toBe(4000); + expect(history.pending).toEqual([]); + expect(history.stranded).toEqual([]); + }); +}); diff --git a/packages/appkit/src/database/migrations/tests/ownership.test.ts b/packages/appkit/src/database/migrations/tests/ownership.test.ts index 56ea47022..4f93e59d7 100644 --- a/packages/appkit/src/database/migrations/tests/ownership.test.ts +++ b/packages/appkit/src/database/migrations/tests/ownership.test.ts @@ -11,7 +11,33 @@ describe("ownership: one owner per branch", () => { expect(hint).toContain( "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE", ); - expect(hint).toContain("use another schemaName in schema.ts"); + }); + + test("a schema owned by another role names both roles and how that happens", () => { + const hint = ownershipHint("42501", { + schemaName: "app", + owner: "old-app-sp", + currentUser: "new-app-sp", + }); + expect(hint).toContain( + `Schema "app" is owned by "old-app-sp", but the app connects as "new-app-sp".`, + ); + expect(hint).toContain( + "the app's database resource was removed and added again, or the app was recreated", + ); + expect(hint).toContain("set another schemaName in schema.ts"); + }); + + test("a schema that does not exist yet points at CREATE on the database", () => { + expect( + ownershipHint("42501", { + schemaName: "app", + owner: null, + currentUser: "app-sp", + }), + ).toBe( + `Role "app-sp" cannot create schema "app". On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE.`, + ); }); test("any other failure gets no ownership hint", () => { diff --git a/packages/appkit/src/plugins/database/migrate-on-start.ts b/packages/appkit/src/plugins/database/migrate-on-start.ts index 8061c1124..a350b3537 100644 --- a/packages/appkit/src/plugins/database/migrate-on-start.ts +++ b/packages/appkit/src/plugins/database/migrate-on-start.ts @@ -1,3 +1,5 @@ +import fs from "node:fs"; + import type { Pool } from "pg"; import { databaseSetupFailed } from "../../database/errors"; @@ -5,8 +7,10 @@ import { applyPendingMigrations, MigrationApplyError, MigrationBaselineMismatchError, + MigrationHistoryError, } from "../../database/migrations/apply"; import { + journalFile, MigrationFilesError, migrationsFolder, } from "../../database/migrations/paths"; @@ -15,64 +19,89 @@ import { createLogger } from "../../logging/logger"; const logger = createLogger("database"); +/** Branches `appkit db` creates for one developer or one agent session; only they migrate them. */ +const DEVELOPER_BRANCH = /^(dev-[a-z0-9-]+-[0-9a-f]{6}|session-[a-z0-9-]+)$/; + /** - * True only inside a deployed Databricks App: not local development, and the - * runtime has set the app's name and its service principal's credentials, as - * Databricks Apps does for every app. `databricks apps run-local` sets the - * name but not the credentials. Locally, `appkit db migrate` applies - * migrations instead. + * True when the app opts in with APPKIT_DB_MIGRATE_ON_START=true (app.yaml) + * and runs with service principal credentials, as Databricks Apps sets for + * every app. `.env` never carries the flag, so a local start does not migrate; + * `databricks apps run-local` reads app.yaml but sets no credentials. */ export function shouldMigrateOnStart( env: NodeJS.ProcessEnv = process.env, ): boolean { return ( - env.NODE_ENV !== "development" && - Boolean(env.DATABRICKS_APP_NAME) && + env.APPKIT_DB_MIGRATE_ON_START === "true" && Boolean(env.DATABRICKS_CLIENT_ID) && Boolean(env.DATABRICKS_CLIENT_SECRET) ); } +/** The branch id in LAKEBASE_ENDPOINT (projects/

/branches//endpoints/). */ +function endpointBranch(endpoint: string | undefined): string | undefined { + return endpoint?.match(/\/branches\/([^/]+)\//)?.[1]; +} + /** * Apply pending committed migrations from /config/database/migrations as * the app's identity (see database/migrations/ownership.ts). No journal → - * no-op. A failed or unadoptable migration, or a missing or unreadable - * migration file, becomes a setup failure that names it; any other error - * (e.g. no connection) is left to lifecycle's generic sanitizer. + * no-op. Every migration failure becomes a setup failure that names it. */ export async function migrateOnStart( pool: Pick, schema: Schema, root: string = process.cwd(), + env: NodeJS.ProcessEnv = process.env, ): Promise { + const folder = migrationsFolder(root); + const branch = endpointBranch(env.LAKEBASE_ENDPOINT); + if (branch && DEVELOPER_BRANCH.test(branch)) { + logger.warn( + "Not migrating branch %s on start: it belongs to a developer or agent session, which migrates it with `appkit db migrate`.", + branch, + ); + return; + } + if (!fs.existsSync(journalFile(folder))) { + logger.info("No committed migrations at %s; nothing to apply", folder); + return; + } + // A transaction-mode pooler hands each statement to any server session, so + // the session-level migration lock would not exclude other instances. + if (env.PGHOST?.includes("-pooler.")) { + throw databaseSetupFailed( + `PGHOST ${env.PGHOST} is a pooled endpoint, and migrations need a direct connection. Use the host without "-pooler".`, + ); + } + + const started = Date.now(); let result: Awaited>; try { result = await applyPendingMigrations(pool, { - migrationsFolder: migrationsFolder(root), + migrationsFolder: folder, schemaName: schema.$schemaName, }); } catch (error) { if ( error instanceof MigrationApplyError || error instanceof MigrationBaselineMismatchError || + error instanceof MigrationHistoryError || error instanceof MigrationFilesError ) { throw databaseSetupFailed(error.message); } throw error; } - if (result.adopted.length > 0) { - logger.info( - "Adopted %s in schema %s: its tables already existed", - result.adopted.join(", "), - schema.$schemaName, - ); - } - if (result.applied.length > 0) { - logger.info( - "Applied migrations %s to schema %s", - result.applied.join(", "), - schema.$schemaName, - ); - } + logger.info( + "Migrations for schema %s from %s: applied=%d adopted=%d edited=%d unknown=%d lock_wait_ms=%d duration_ms=%d", + schema.$schemaName, + folder, + result.applied.length, + result.adopted.length, + result.edited.length, + result.unknown, + result.lockWaitMs, + Date.now() - started, + ); } diff --git a/packages/appkit/src/plugins/database/schema-check.ts b/packages/appkit/src/plugins/database/schema-check.ts index 9b6f7176a..bf206c427 100644 --- a/packages/appkit/src/plugins/database/schema-check.ts +++ b/packages/appkit/src/plugins/database/schema-check.ts @@ -76,7 +76,7 @@ export async function assertSchemaMatchesDatabase( } if (problems.length > 0) { throw databaseSetupFailed( - `the declared schema does not match the database: ${problems.join("; ")}. Create them, or commit migrations under config/database/migrations so the deployed app applies them on start.`, + `the declared schema does not match the database: ${problems.join("; ")}. Create them, or commit migrations under config/database/migrations and set APPKIT_DB_MIGRATE_ON_START=true in app.yaml so the deployed app applies them on start.`, ); } } diff --git a/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts index cf453a32d..6d53bc5c4 100644 --- a/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts +++ b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts @@ -26,6 +26,10 @@ const schema = defineSchema( ); const roots: string[] = []; const pool = { connect: vi.fn() }; +const deployEnv = { + LAKEBASE_ENDPOINT: "projects/p/branches/production/endpoints/primary", + PGHOST: "ep-abc.database.cloud.databricks.com", +}; function appRoot(): string { const root = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-migrate-start-")); @@ -33,6 +37,26 @@ function appRoot(): string { return root; } +/** An app root with a committed (empty) journal. */ +function appRootWithMigrations(): string { + const root = appRoot(); + const meta = path.join(root, "config/database/migrations/meta"); + fs.mkdirSync(meta, { recursive: true }); + fs.writeFileSync( + path.join(meta, "_journal.json"), + JSON.stringify({ version: "7", dialect: "postgresql", entries: [] }), + ); + return root; +} + +const nothingDone = { + applied: [], + adopted: [], + edited: [], + unknown: 0, + lockWaitMs: 0, +}; + beforeEach(async () => { const actual = await vi.importActual< typeof import("../../../database/migrations/apply") @@ -54,44 +78,31 @@ describe("shouldMigrateOnStart", () => { DATABRICKS_CLIENT_ID: "sp-client-id", DATABRICKS_CLIENT_SECRET: "secret", }; - const app = { DATABRICKS_APP_NAME: "my-app", ...credentials }; + const optIn = { APPKIT_DB_MIGRATE_ON_START: "true" }; test.each([ - ["a deployed app", { ...app, NODE_ENV: "production" }, true], - ["a deployed app without NODE_ENV", app, true], + ["a deployed app that opts in", { ...optIn, ...credentials }, true], [ - "local development, even with app credentials", - { ...app, NODE_ENV: "development" }, + "a deployed app without the flag", + { DATABRICKS_APP_NAME: "my-app", ...credentials }, false, ], [ - "`databricks apps run-local`: the app name without its credentials", - { DATABRICKS_APP_NAME: "my-app", NODE_ENV: "production" }, + "a flag other than true", + { APPKIT_DB_MIGRATE_ON_START: "1", ...credentials }, false, ], [ - "a local production start with app credentials in .env but no app name", - { ...credentials, NODE_ENV: "production" }, + "`databricks apps run-local`: the flag from app.yaml, no credentials", + { ...optIn, DATABRICKS_APP_NAME: "my-app" }, false, ], [ "no client secret (a developer's own credentials)", - { - DATABRICKS_APP_NAME: "my-app", - DATABRICKS_CLIENT_ID: "x", - NODE_ENV: "production", - }, - false, - ], - [ - "no client id", - { - DATABRICKS_APP_NAME: "my-app", - DATABRICKS_CLIENT_SECRET: "x", - NODE_ENV: "production", - }, + { ...optIn, DATABRICKS_CLIENT_ID: "x" }, false, ], + ["no client id", { ...optIn, DATABRICKS_CLIENT_SECRET: "x" }, false], ["nothing set", {}, false], ] as const)("%s → %s", (_label, env, expected) => { expect(shouldMigrateOnStart(env)).toBe(expected); @@ -99,21 +110,29 @@ describe("shouldMigrateOnStart", () => { }); describe("migrateOnStart", () => { - test("no committed journal: a no-op that never connects", async () => { - await migrateOnStart(pool, schema, appRoot()); + test("no committed journal: says so and never connects", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const root = appRoot(); + + await migrateOnStart(pool, schema, root, deployEnv); expect(pool.connect).not.toHaveBeenCalled(); + expect(log.mock.calls.flat().map(String).join(" ")).toContain( + `No committed migrations at ${path.join(root, "config/database/migrations")}`, + ); }); - test("applies from /config/database/migrations into the schema's own journal, and logs what it did", async () => { - const root = appRoot(); + test("applies from /config/database/migrations into the schema's own journal, and logs one summary", async () => { + const root = appRootWithMigrations(); const log = vi.spyOn(console, "log").mockImplementation(() => undefined); applyPendingMigrations.mockResolvedValueOnce({ + ...nothingDone, applied: ["0001_migration"], adopted: ["0000_baseline"], + lockWaitMs: 12, }); - await migrateOnStart(pool, schema, root); + await migrateOnStart(pool, schema, root, deployEnv); expect(applyPendingMigrations).toHaveBeenCalledWith(pool, { migrationsFolder: path.join(root, "config/database/migrations"), @@ -121,29 +140,77 @@ describe("migrateOnStart", () => { }); const logged = log.mock.calls.map((call) => call.map(String).join(" ")); expect(logged).toEqual([ - expect.stringContaining( - "Adopted 0000_baseline in schema app: its tables already existed", - ), - expect.stringContaining( - "Applied migrations 0001_migration to schema app", + expect.stringMatching( + /Migrations for schema app from .*: applied=1 adopted=1 edited=0 unknown=0 lock_wait_ms=12 duration_ms=\d+/, ), ]); }); - test("nothing pending logs nothing", async () => { - const log = vi.spyOn(console, "log").mockImplementation(() => undefined); - applyPendingMigrations.mockResolvedValueOnce({ applied: [], adopted: [] }); + test.each([ + ["a developer branch", "dev-jane-a1b2c3"], + ["an agent session branch", "session-k3x9q2m7"], + ])( + "%s is never migrated on start, even with committed migrations", + async (_label, branch) => { + const warn = vi + .spyOn(console, "warn") + .mockImplementation(() => undefined); + + await migrateOnStart(pool, schema, appRootWithMigrations(), { + ...deployEnv, + LAKEBASE_ENDPOINT: `projects/p/branches/${branch}/endpoints/primary`, + }); + + expect(applyPendingMigrations).not.toHaveBeenCalled(); + expect(warn.mock.calls.flat().map(String).join(" ")).toContain( + `Not migrating branch ${branch} on start`, + ); + }, + ); - await migrateOnStart(pool, schema, appRoot()); + test.each([ + ["a branch that only starts with dev-", "dev-environment"], + ["the production branch", "production"], + ])("%s is migrated", async (_label, branch) => { + vi.spyOn(console, "log").mockImplementation(() => undefined); + applyPendingMigrations.mockResolvedValueOnce(nothingDone); - expect(log).not.toHaveBeenCalled(); + await migrateOnStart(pool, schema, appRootWithMigrations(), { + ...deployEnv, + LAKEBASE_ENDPOINT: `projects/p/branches/${branch}/endpoints/primary`, + }); + + expect(applyPendingMigrations).toHaveBeenCalled(); + }); + + test("a pooled endpoint is refused: the migration lock would not hold", async () => { + const error = await migrateOnStart(pool, schema, appRootWithMigrations(), { + ...deployEnv, + PGHOST: "ep-abc-pooler.database.cloud.databricks.com", + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(DatabasePluginError); + expect((error as Error).message).toContain( + "is a pooled endpoint, and migrations need a direct connection", + ); + expect(applyPendingMigrations).not.toHaveBeenCalled(); }); test.each([ [ "a failed migration", - new apply.MigrationApplyError(["0001_migration"], "42501"), - "Applying migration 0001_migration failed (SQLSTATE 42501); none were applied.", + new apply.MigrationApplyError({ + step: "Applying migrations", + failed: "0001_migration", + pending: ["0001_migration"], + cause: Object.assign(new Error("denied"), { code: "42501" }), + }), + "Migration 0001_migration failed (SQLSTATE 42501) and was rolled back.", + ], + [ + "a stranded migration", + new apply.MigrationHistoryError("app", ["0001_parallel"], "0002_b"), + "Migration 0001_parallel is older than 0002_b", ], [ "a baseline that only partly matches", @@ -164,9 +231,12 @@ describe("migrateOnStart", () => { async (_label, failure, reason) => { applyPendingMigrations.mockRejectedValueOnce(failure); - const error = await migrateOnStart(pool, schema, appRoot()).catch( - (caught: unknown) => caught, - ); + const error = await migrateOnStart( + pool, + schema, + appRootWithMigrations(), + deployEnv, + ).catch((caught: unknown) => caught); expect(error).toBeInstanceOf(DatabasePluginError); expect(error).toMatchObject({ category: "SETUP_FAILED", phase: "setup" }); @@ -182,6 +252,8 @@ describe("migrateOnStart", () => { const raw = new Error("connection reset"); applyPendingMigrations.mockRejectedValueOnce(raw); - await expect(migrateOnStart(pool, schema, appRoot())).rejects.toBe(raw); + await expect( + migrateOnStart(pool, schema, appRootWithMigrations(), deployEnv), + ).rejects.toBe(raw); }); });