From eb276128c251a84d99d0152925c831112ed67c8e Mon Sep 17 00:00:00 2001
From: Tau <tau@nowhere>
Date: Sat, 26 Sep 2020 23:58:41 -0400
Subject: [PATCH] sql: Add "maintenance transaction" system

SQLite's ALTER TABLE functionality leaves a lot to be desired, so
in practice the way you alter SQLite schema is to create a new
table, migrate the data into it, then drop the old table. However,
this is potentially complicated by the presence of foreign key
constraints.

In order to resolve the foreign key constraint issue we add a
`maintenance()` method to the `DataSource` abstraction layer. This
operates similarly to the existing `transaction()` method, but the
SQLite implementation of this method defers enforcement of foreign
keys until the transaction is about to commit.
---
 src/checkdb.ts         |  2 +-
 src/sql/api.ts         | 10 ++++++++++
 src/sql/sqlite.test.ts | 33 +++++++++++++++++++++++++++++++
 src/sql/sqlite.ts      | 44 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 src/sql/sqlite.test.ts

diff --git a/src/checkdb.ts b/src/checkdb.ts
index e6b4ac3..1d054f1 100644
--- a/src/checkdb.ts
+++ b/src/checkdb.ts
@@ -106,7 +106,7 @@ async function migratedb(
 export default async function checkdb(db: DataSource): Promise<void> {
   let schemaver: number | undefined;
 
-  await db.transaction(async txn => {
+  await db.maintenance(async txn => {
     const stmt = sql.select("schemaver").from("meta");
 
     try {
diff --git a/src/sql/api.ts b/src/sql/api.ts
index ad79a0f..ab0b703 100644
--- a/src/sql/api.ts
+++ b/src/sql/api.ts
@@ -45,6 +45,16 @@ export interface DataSource {
    */
   transaction<T>(callback: (txn: Transaction) => Promise<T>): Promise<T>;
 
+  /**
+   * Execute an async callback in the context of a "maintenance" transaction.
+   *
+   * This method should be used when altering the data store's schema. On
+   * SQLite it will temporarily disable foreign key constraints (and then
+   * re-enable and validate them immediately prior to committing the
+   * transaction) so that tables may be dropped and re-created.
+   */
+  maintenance<T>(callback: (txn: Transaction) => Promise<T>): Promise<T>;
+
   /**
    * Perform housekeeping operations on the database's on-disk data structures.
    *
diff --git a/src/sql/sqlite.test.ts b/src/sql/sqlite.test.ts
new file mode 100644
index 0000000..c44c292
--- /dev/null
+++ b/src/sql/sqlite.test.ts
@@ -0,0 +1,33 @@
+import { Transaction } from "./api";
+import { openSqlite } from "./sqlite";
+
+async function violateFk(txn: Transaction) {
+  await txn.raw("create table foo (id number primary key)");
+  await txn.raw("create table bar (foo_id integer references foo(id))");
+  await txn.raw("insert into bar (foo_id) values (1)");
+}
+
+test("FK enforcement", async () => {
+  const db = openSqlite(":memory:");
+  const promise = db.transaction(violateFk);
+
+  expect(promise).rejects.toMatchObject({
+    message: "FOREIGN KEY constraint failed",
+  });
+});
+
+test("FK violation detection", async () => {
+  const db = openSqlite(":memory:");
+  const promise = db.maintenance(violateFk);
+
+  expect(promise).rejects.toMatchObject({
+    fkViolations: [
+      {
+        fkid: BigInt(0),
+        parent: "foo",
+        rowid: BigInt(1),
+        table: "bar",
+      },
+    ],
+  });
+});
diff --git a/src/sql/sqlite.ts b/src/sql/sqlite.ts
index 98fc480..d1bdcf9 100644
--- a/src/sql/sqlite.ts
+++ b/src/sql/sqlite.ts
@@ -1,10 +1,13 @@
 import Database, { ResultRow } from "@decafcode/sqlite";
 import { randomBytes } from "crypto";
+import logger from "debug";
 import * as sql from "sql-bricks-postgres";
 
 import { DataSource, Row, Transaction } from "./api";
 import { Id } from "../model";
 
+const debug = logger("app:sqlite");
+
 // bless me father for i have sinned
 const fuFixup = new RegExp(" FOR UPDATE$");
 
@@ -53,6 +56,12 @@ function _postprocess(obj: ResultRow): Row {
   return result;
 }
 
+export class MaintenanceError extends Error {
+  constructor(msg: string, public readonly fkViolations: any[]) {
+    super(msg);
+  }
+}
+
 class SqliteTransaction implements Transaction {
   constructor(private readonly _db: Database) {}
 
@@ -141,6 +150,41 @@ class SqliteDataSource implements DataSource {
     }
   }
 
+  async maintenance<T>(
+    callback: (txn: Transaction) => Promise<T>
+  ): Promise<T> {
+    const db = new Database(this._path);
+
+    try {
+      db.exec("pragma foreign_keys=off");
+      db.exec("begin");
+
+      const txn = new SqliteTransaction(db);
+      const result = await callback(txn);
+
+      const fkViolations = db.prepare("pragma foreign_key_check").all();
+
+      if (fkViolations.length > 0) {
+        debug("Foreign key violations: %O", fkViolations);
+
+        throw new MaintenanceError(
+          "Maintenance failed, db unchanged. Consult 'app:sqlite' debug log.",
+          fkViolations
+        );
+      }
+
+      db.exec("commit");
+
+      return result;
+    } catch (e) {
+      db.exec("rollback");
+
+      throw e;
+    } finally {
+      db.close();
+    }
+  }
+
   vacuum(): Promise<void> {
     const db = new Database(this._path);