commit f783f1dcec5450b70f219b590f4f6657d88afd5a
parent f4e33f3904fab77b66325d970f2b953f26232855
Author: Florian Dold <dold@taler.net>
Date: Mon, 20 Jul 2026 00:09:05 +0200
db: exercise and validate the sqlite schema migration path
schemaMigrations was empty, so nothing had run this code.
initSqliteWalletDb takes the migration list as a parameter, so tests can
drive it with a synthetic one. A version that doesn't strictly increase
is now rejected instead of skipped.
Diffstat:
2 files changed, 283 insertions(+), 11 deletions(-)
diff --git a/packages/taler-wallet-core/src/db-sqlite-migrations.test.ts b/packages/taler-wallet-core/src/db-sqlite-migrations.test.ts
@@ -0,0 +1,236 @@
+/*
+ This file is part of GNU Taler
+ (C) 2026 Taler Systems S.A.
+
+ GNU Taler is free software; you can redistribute it and/or modify it under the
+ terms of the GNU General Public License as published by the Free Software
+ Foundation; either version 3, or (at your option) any later version.
+
+ GNU Taler is distributed in the hope that it will be useful, but WITHOUT ANY
+ WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+ A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along with
+ GNU Taler; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
+ */
+
+/**
+ * Tests for the sqlite schema migration mechanism.
+ *
+ * schemaMigrations is empty, so nothing in the wallet exercises this path:
+ * the first real migration would otherwise be both the first use of the
+ * mechanism and a change to a user's database at the same time. These tests
+ * drive it with synthetic migrations instead.
+ */
+
+import { createNodeHelperSqlite3Impl } from "@gnu-taler/idb-bridge/node-helper-sqlite3-impl";
+import assert from "node:assert";
+import { mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { test } from "node:test";
+
+import { SchemaMigration } from "./db-sqlite-schema.js";
+import { initSqliteWalletDb } from "./dbtx-sqlite.js";
+
+/**
+ * A migration must survive the database being closed and reopened, so these
+ * run against a file rather than :memory:.
+ */
+function withTempDb(): { path: string; cleanup: () => void } {
+ const dir = mkdtempSync(join(tmpdir(), "wallet-migration-test-"));
+ return {
+ path: join(dir, "wallet.sqlite3"),
+ cleanup: () => rmSync(dir, { recursive: true, force: true }),
+ };
+}
+
+async function openRaw(path: string) {
+ const impl = await createNodeHelperSqlite3Impl({ enableTracing: false });
+ return await impl.open(path);
+}
+
+async function queryAll(db: any, sql: string): Promise<any[]> {
+ return await (await db.prepare(sql)).getAll();
+}
+
+const addColumn: SchemaMigration = {
+ version: 2,
+ name: "add-tombstone-note",
+ statements: [
+ "ALTER TABLE tombstones ADD COLUMN note TEXT",
+ // A migration is DDL *and* the backfill that makes the new column true of
+ // rows written before it existed.
+ "UPDATE tombstones SET note = 'backfilled' WHERE note IS NULL",
+ ],
+};
+
+test("migration applies DDL and backfills existing rows", async () => {
+ const { path, cleanup } = withTempDb();
+ try {
+ // A database at baseline, with a row written before the new column exists.
+ let db = await openRaw(path);
+ await initSqliteWalletDb(db);
+ await (
+ await db.prepare("INSERT INTO tombstones (id) VALUES ($id)")
+ ).run({ id: "pre-existing" });
+ await db.close();
+
+ // Reopened by a build that has the migration.
+ db = await openRaw(path);
+ await initSqliteWalletDb(db, [addColumn]);
+
+ const rows = await queryAll(db, "SELECT id, note FROM tombstones");
+ assert.strictEqual(rows.length, 1);
+ assert.strictEqual(
+ rows[0].note,
+ "backfilled",
+ "the row written before the migration must be backfilled",
+ );
+
+ const applied = await queryAll(
+ db,
+ "SELECT version, name, applied_at FROM schema_migrations ORDER BY version",
+ );
+ assert.deepStrictEqual(
+ applied.map((r) => Number(r.version)),
+ [1, 2],
+ "the baseline and the migration must both be recorded",
+ );
+ assert.strictEqual(applied[1].name, "add-tombstone-note");
+ // Microseconds, per the schema's convention for INTEGER timestamps. A
+ // millisecond value would be ~1000x too small and still look plausible.
+ const appliedAt = Number(applied[1].applied_at);
+ const nowMicros = Date.now() * 1000;
+ assert.ok(
+ appliedAt > nowMicros - 60_000_000 && appliedAt <= nowMicros + 1_000_000,
+ `applied_at ${appliedAt} is not a plausible microsecond timestamp`,
+ );
+
+ await db.close();
+ } finally {
+ cleanup();
+ }
+});
+
+test("a migration already recorded is not applied twice", async () => {
+ const { path, cleanup } = withTempDb();
+ try {
+ let db = await openRaw(path);
+ await initSqliteWalletDb(db, [addColumn]);
+ const first = await queryAll(
+ db,
+ "SELECT applied_at FROM schema_migrations WHERE version = 2",
+ );
+ await (
+ await db.prepare("INSERT INTO tombstones (id, note) VALUES ($id, $n)")
+ ).run({ id: "later", n: "written-by-hand" });
+ await db.close();
+
+ // Opening again must not re-run the migration: the second statement of
+ // this one would overwrite the note of any row where it is NULL, but more
+ // importantly re-running arbitrary DDL fails outright (the column already
+ // exists), so a backend that ignored the log could not open at all.
+ db = await openRaw(path);
+ await initSqliteWalletDb(db, [addColumn]);
+
+ const second = await queryAll(
+ db,
+ "SELECT applied_at FROM schema_migrations WHERE version = 2",
+ );
+ assert.strictEqual(
+ Number(second[0].applied_at),
+ Number(first[0].applied_at),
+ "applied_at must not change: the migration should not have re-run",
+ );
+ const rows = await queryAll(
+ db,
+ "SELECT note FROM tombstones WHERE id = 'later'",
+ );
+ assert.strictEqual(rows[0].note, "written-by-hand");
+ await db.close();
+ } finally {
+ cleanup();
+ }
+});
+
+test("a failing migration rolls back and is not recorded", async () => {
+ const { path, cleanup } = withTempDb();
+ try {
+ const broken: SchemaMigration = {
+ version: 2,
+ name: "half-broken",
+ statements: [
+ "ALTER TABLE tombstones ADD COLUMN note TEXT",
+ "UPDATE tombstones SET note = 'x'",
+ "THIS IS NOT SQL",
+ ],
+ };
+
+ let db = await openRaw(path);
+ await initSqliteWalletDb(db);
+ await (
+ await db.prepare("INSERT INTO tombstones (id) VALUES ($id)")
+ ).run({ id: "row" });
+ await db.close();
+
+ db = await openRaw(path);
+ await assert.rejects(
+ async () => await initSqliteWalletDb(db, [broken]),
+ "opening must fail rather than continue with a half-applied migration",
+ );
+
+ // The DDL must have rolled back too, not just the bookkeeping. Checking
+ // only schema_migrations would pass even if the transaction were
+ // committed on failure, because the row is inserted after the statements
+ // and so is never written either way -- sqlite makes ALTER TABLE
+ // transactional, and this is the assertion that depends on it.
+ const cols = await queryAll(db, "PRAGMA table_info(tombstones)");
+ assert.ok(
+ !cols.some((c) => c.name === "note"),
+ "the column added by the failed migration must not survive",
+ );
+
+ // Nothing recorded, so the next attempt starts from a known state rather
+ // than skipping the migration as done.
+ const applied = await queryAll(
+ db,
+ "SELECT version FROM schema_migrations ORDER BY version",
+ );
+ assert.deepStrictEqual(
+ applied.map((r) => Number(r.version)),
+ [1],
+ "a failed migration must not be recorded as applied",
+ );
+ await db.close();
+ } finally {
+ cleanup();
+ }
+});
+
+test("migration versions must strictly increase", async () => {
+ const { path, cleanup } = withTempDb();
+ try {
+ const db = await openRaw(path);
+ for (const bad of [
+ [
+ { version: 3, name: "c", statements: [] },
+ { version: 2, name: "b", statements: [] },
+ ],
+ [
+ { version: 2, name: "a", statements: [] },
+ { version: 2, name: "b", statements: [] },
+ ],
+ // 1 is the baseline; reusing it would make the migration a silent no-op.
+ [{ version: 1, name: "clashes-with-baseline", statements: [] }],
+ ] as SchemaMigration[][]) {
+ await assert.rejects(
+ async () => await initSqliteWalletDb(db, bad),
+ `must reject ${JSON.stringify(bad.map((m) => m.version))}`,
+ );
+ }
+ await db.close();
+ } finally {
+ cleanup();
+ }
+});
diff --git a/packages/taler-wallet-core/src/dbtx-sqlite.ts b/packages/taler-wallet-core/src/dbtx-sqlite.ts
@@ -118,6 +118,7 @@ import {
} from "./db-common.js";
import {
SQLITE_BASELINE_SCHEMA,
+ SchemaMigration,
SQLITE_SCHEMA_VERSION,
schemaMigrations,
} from "./db-sqlite-schema.js";
@@ -131,13 +132,6 @@ export class NotImplementedError extends Error {
}
/**
- * Opens the database and brings its schema up to date.
- *
- * Each migration runs in its own transaction, and the schema_migrations row is
- * written inside that transaction, so a crash part-way through cannot leave a
- * half-applied migration recorded as done.
- */
-/**
* Transaction control for the helper protocol.
*
* These must go through prepared statements. `exec` commits implicitly, which
@@ -190,7 +184,49 @@ export class SqliteTxControl {
*/
const SQLITE_BUSY_TIMEOUT_MS = 5000;
-export async function initSqliteWalletDb(db: Sqlite3Database): Promise<void> {
+/** Current time in the microseconds the schema's INTEGER timestamps use. */
+function nowMicros(): number {
+ return Date.now() * 1000;
+}
+
+/**
+ * Check the migration list before running any of it.
+ *
+ * A duplicate or out-of-order version does not fail on its own: migrations are
+ * skipped by looking up the version in schema_migrations, so a reused version
+ * silently never runs, and the database ends up missing a change while
+ * claiming to have applied it. Better to refuse to open.
+ */
+function validateSchemaMigrations(migrations: SchemaMigration[]): void {
+ let prev = 1; // the baseline occupies version 1
+ for (const mig of migrations) {
+ if (mig.version <= prev) {
+ throw Error(
+ `schema migration ${mig.version} (${mig.name}) is not greater than` +
+ ` the preceding version ${prev}: versions must strictly increase` +
+ ` and may not be reused`,
+ );
+ }
+ prev = mig.version;
+ }
+}
+
+/**
+ * Open the database and bring its schema up to date.
+ *
+ * Each migration runs in its own transaction with its schema_migrations row
+ * written inside that transaction, so a crash part-way through cannot leave a
+ * half-applied migration recorded as done.
+ *
+ * The migration list is a parameter so that tests can exercise the path with
+ * a synthetic migration. Until a real one exists this is the only thing that
+ * runs it at all.
+ */
+export async function initSqliteWalletDb(
+ db: Sqlite3Database,
+ migrations: SchemaMigration[] = schemaMigrations,
+): Promise<void> {
+ validateSchemaMigrations(migrations);
await db.exec("PRAGMA foreign_keys = ON");
await db.exec(`PRAGMA busy_timeout = ${SQLITE_BUSY_TIMEOUT_MS}`);
// WAL: readers do not block the writer, and a commit appends to the log
@@ -226,7 +262,7 @@ export async function initSqliteWalletDb(db: Sqlite3Database): Promise<void> {
await baselineStmt.run({
version: 1,
name: "baseline",
- applied_at: Date.now(),
+ applied_at: nowMicros(),
});
const applied = await (
@@ -234,7 +270,7 @@ export async function initSqliteWalletDb(db: Sqlite3Database): Promise<void> {
).getAll();
const have = new Set(applied.map((r) => Number(r.version)));
- for (const mig of schemaMigrations) {
+ for (const mig of migrations) {
if (have.has(mig.version)) {
continue;
}
@@ -251,7 +287,7 @@ export async function initSqliteWalletDb(db: Sqlite3Database): Promise<void> {
await stmt.run({
version: mig.version,
name: mig.name,
- applied_at: Date.now(),
+ applied_at: nowMicros(),
});
await txc.commit();
} catch (e) {
