From 4e47bd8ce033873ea821a92ca0690cd45c319838 Mon Sep 17 00:00:00 2001 From: Benjamin Toby Date: Wed, 14 Oct 2026 03:00:29 +0100 Subject: [PATCH] Port bun-mariadb schema guard, targetId fix, lazy init and tests - schema: add --dry-run (sync in a rolled-back transaction) and require confirmation (or --yes) before dropping tables or columns; unapproved destructive changes are refused - only rebuild tables whose CREATE TABLE statement changed - always AND targetId with the query, regardless of searchOperator - empty IN / NOT IN lists become 1 = 0 / 1 = 1 - initialize lazily on first call so importing never throws - read CLI version from package.json, chmod CLI on compile, pin @types/bun - add SQL generator, value and schema guard tests Co-Authored-By: Claude Opus 5.5 --- README.md | 18 +++ bun.lock | 6 +- package.json | 6 +- src/commands/index.ts | 8 +- src/commands/schema.ts | 103 +++++++++++++++-- src/index.ts | 23 ++-- src/lib/schema/build-create-table-sql.ts | 54 +++++++++ src/lib/schema/create-db-schema.test.ts | 54 ++++++++- src/lib/schema/create-db-schema.ts | 27 ++--- src/lib/schema/create-table.ts | 52 ++------- src/lib/schema/drop-removed-tables.ts | 28 +++-- src/lib/schema/handle-db-schema-table.ts | 9 +- src/lib/schema/record-schema-change.ts | 35 ++++++ src/lib/schema/sync-indexes.ts | 22 ++-- src/lib/schema/update-table.ts | 64 +++++++--- src/lib/sqlite/db-delete.ts | 17 +-- src/lib/sqlite/db-select.ts | 19 +-- src/lib/sqlite/db-update.ts | 17 +-- src/types/index.ts | 22 ++++ src/utils/sql-generator-gen-search-str.ts | 5 +- src/utils/sql-generator.ts | 36 +++++- tests/sql-generator.test.ts | 135 ++++++++++++++++++++++ tests/sql-values.test.ts | 61 ++++++++++ 23 files changed, 649 insertions(+), 172 deletions(-) create mode 100644 src/lib/schema/build-create-table-sql.ts create mode 100644 src/lib/schema/record-schema-change.ts create mode 100644 tests/sql-generator.test.ts create mode 100644 tests/sql-values.test.ts diff --git a/README.md b/README.md index 0edc5d3..69ce130 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ A schema-driven SQLite manager for [Bun](https://bun.sh), featuring automatic sc - [TypeScript Type Generation](#typescript-type-generation) - [Default Fields](#default-fields) - [Project Structure](#project-structure) +- [Testing](#testing) --- @@ -272,6 +273,8 @@ bunx bun-sqlite schema [options] | ----------------- | ---------------------------------------------------------- | | `-v`, `--vector` | Drop and recreate all vector (`sqlite-vec`) virtual tables | | `-t`, `--typedef` | Also generate TypeScript type definitions after syncing | +| `--dry-run` | Print the planned schema changes without applying them | +| `-y`, `--yes` | Apply destructive changes (dropped tables / columns) without confirmation | **Examples:** @@ -284,8 +287,13 @@ bunx bun-sqlite schema --typedef # Sync schema, recreate vector tables, and regenerate types bunx bun-sqlite schema --vector --typedef + +# Review planned changes first +bunx bun-sqlite schema --dry-run ``` +Only tables whose definition changed are rebuilt. Changes that discard data (dropping a table, or a column removed from the schema) are listed and need confirmation before they run. Non-interactive runs (CI, scripts) refuse them unless `--yes` is passed. The plan is exact: `--dry-run` runs the whole sync in a transaction and rolls it back. + ### `typedef` — Generate TypeScript types only ```bash @@ -844,6 +852,16 @@ bun-sqlite/ --- +## Testing + +```bash +bun run test +``` + +Runs the unit tests (`tests/`) and the schema sync tests (`src/lib/schema/*.test.ts`) against in-memory SQLite databases. No setup needed. + +--- + ## License MIT diff --git a/bun.lock b/bun.lock index 26ff395..4a614f1 100644 --- a/bun.lock +++ b/bun.lock @@ -13,7 +13,7 @@ "sqlite-vec": "^0.1.7-alpha.2", }, "devDependencies": { - "@types/bun": "latest", + "@types/bun": "1.3.14", "@types/inquirer": "^9.0.9", "@types/lodash": "^4.17.24", "@types/node": "^25.3.3", @@ -56,7 +56,7 @@ "@inquirer/type": ["@inquirer/type@4.0.4", "", { "peerDependencies": { "@types/node": ">=18" }, "optionalPeers": ["@types/node"] }, "sha512-PamArxO3cFJZoOzspzo6cxVlLeIftyBsZw/S9bKY5DzxqJVZgjoj1oP8d0rskKtp7sZxBycsoer1g6UeJV1BBA=="], - "@types/bun": ["@types/bun@1.3.9", "", { "dependencies": { "bun-types": "1.3.9" } }, "sha512-KQ571yULOdWJiMH+RIWIOZ7B2RXQGpL1YQrBtLIV3FqDcCu6FsbFUBwhdKUlCKUpS3PJDsHlJ1QKlpxoVR+xtw=="], + "@types/bun": ["@types/bun@1.3.14", "", { "dependencies": { "bun-types": "1.3.14" } }, "sha512-h1hFqFVcvAvD9j9K7ZW7vd82aSA+rTdznZa+5bwvCwqSB1jmmfLcbIWhOLx1/+boy/xmjgCs/OMUL8hRJSmnPw=="], "@types/inquirer": ["@types/inquirer@9.0.9", "", { "dependencies": { "@types/through": "*", "rxjs": "^7.2.0" } }, "sha512-/mWx5136gts2Z2e5izdoRCo46lPp5TMs9R15GTSsgg/XnZyxDWVqoVU3R9lWnccKpqwsJLvRoxbCjoJtZB7DSw=="], @@ -66,7 +66,7 @@ "@types/through": ["@types/through@0.0.33", "", { "dependencies": { "@types/node": "*" } }, "sha512-HsJ+z3QuETzP3cswwtzt2vEIiHBk/dCcHGhbmG5X3ecnwFD/lPrMpliGXxSCg03L9AhrdwA4Oz/qfspkDW+xGQ=="], - "bun-types": ["bun-types@1.3.9", "", { "dependencies": { "@types/node": "*" } }, "sha512-+UBWWOakIP4Tswh0Bt0QD0alpTY8cb5hvgiYeWCMet9YukHbzuruIEeXC2D7nMJPB12kbh8C7XJykSexEqGKJg=="], + "bun-types": ["bun-types@1.3.14", "", { "dependencies": { "@types/node": "*" } }, "sha512-4N0ig0fEomHt5R0KCFWjovxow98rIoRwKolrYdCcknNwMekCXRnWEUvgu5soYV8QXtVsrUD8B95MBOZGPvr6KQ=="], "chalk": ["chalk@5.6.2", "", {}, "sha512-7NzBL0rN6fMUW+f7A6Io4h40qQlG+xGmtMxfbnH/K7TAtt8JQWVQK+6g0UXKMeVJoyV5EkkNsErQ8pVD3bLHbA=="], diff --git a/package.json b/package.json index 9206470..2ec20b8 100644 --- a/package.json +++ b/package.json @@ -26,13 +26,13 @@ }, "scripts": { "dev": "tsc --watch", - "compile": "rm -rf dist tsconfig.tsbuildinfo && tsc -p tsconfig.json", + "compile": "rm -rf dist tsconfig.tsbuildinfo && tsc -p tsconfig.json && chmod +x dist/commands/index.js", "typecheck": "tsc -p tsconfig.json --noEmit", - "test": "bun test src", + "test": "bun test src tests", "prepublishOnly": "bun run compile" }, "devDependencies": { - "@types/bun": "latest", + "@types/bun": "1.3.14", "@types/inquirer": "^9.0.9", "@types/lodash": "^4.17.24", "@types/node": "^25.3.3" diff --git a/src/commands/index.ts b/src/commands/index.ts index a3f3d2b..bd97d6d 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -9,6 +9,7 @@ import exportCmd from "./export"; import importCmd from "./import"; import admin from "./admin"; import init from "../functions/init"; +import { readFileSync } from "fs"; try { init(); @@ -17,13 +18,18 @@ try { process.exit(1); } +/** Resolves to the package root from both src/commands and dist/commands */ +const { version } = JSON.parse( + readFileSync(new URL("../../package.json", import.meta.url), "utf-8"), +); + /** * # Describe Program */ program .name(`bun-sqlite`) .description(`SQLite manager for Bun`) - .version(`1.0.0`); + .version(version); /** * # Declare Commands diff --git a/src/commands/schema.ts b/src/commands/schema.ts index b761be8..1d21a38 100644 --- a/src/commands/schema.ts +++ b/src/commands/schema.ts @@ -9,6 +9,8 @@ import { writeLiveSchema } from "../functions/live-schema"; import grabDBDir from "../utils/grab-db-dir"; import snapshotDb from "../utils/snapshot-db"; import replaceDbFile from "../utils/replace-db-file"; +import { confirm } from "@inquirer/prompts"; +import type { SchemaChange } from "../types"; export default function () { return new Command("schema") @@ -18,6 +20,14 @@ export default function () { "Recreate Vector Tables. This will drop and rebuild all vector tables", ) .option("-t, --typedef", "Generate typescript type definitions") + .option( + "--dry-run", + "Print the schema changes that would be applied, without applying them", + ) + .option( + "-y, --yes", + "Apply destructive changes (dropped tables / columns) without confirmation", + ) .action(async (opts) => { console.log(`Starting process ...`); @@ -27,20 +37,93 @@ export default function () { const { ROOT_DIR, BUN_SQLITE_TEMP_DB_FILE_PATH } = grabDirNames(); const { db_file_path } = grabDBDir({ config }); + const isVector = Boolean(opts.vector || opts.v); + const isTypeDef = Boolean(opts.typedef || opts.t); + + const finaldbSchema = appendDefaultFieldsToDbSchema({ + dbSchema, + }); + + const params = { + db: DbClient, + db_schema: finaldbSchema, + recreate_vector_table: isVector, + }; + + /** + * Plan: run the sync in a transaction that is rolled back + */ + const plan: SchemaChange[] = []; + + try { + createDBSchema({ ...params, dry_run: true, changes: plan }); + } catch (error: any) { + console.error(chalk.red(`Schema sync ERROR => ${error.message}`)); + process.exit(1); + } + + const destructive = plan.filter((change) => change.destructive); + + if (opts.dryRun) { + if (!plan.length) { + console.log(chalk.green(`Schema is up to date.`)); + } else { + console.log(chalk.bold(`Planned schema changes:\n`)); + for (const change of plan) { + console.log( + change.destructive + ? chalk.red(` ${change.description}`) + : ` ${change.description}`, + ); + } + if (destructive.length) { + console.log( + chalk.red( + `\n${destructive.length} destructive change(s) (in red) would discard data.`, + ), + ); + } + } + process.exit(0); + } + + if (destructive.length && !opts.yes) { + console.log( + chalk.yellow(`\nThese changes will permanently discard data:\n`), + ); + for (const change of destructive) { + console.log(chalk.red(` ${change.description}`)); + } + console.log(); + + if (!process.stdin.isTTY) { + console.error( + chalk.red( + `Refusing to apply destructive changes non-interactively. Re-run with --yes to apply them.`, + ), + ); + process.exit(1); + } + + const proceed = await confirm({ + message: `Apply ${destructive.length} destructive change(s)?`, + default: false, + }); + + if (!proceed) { + console.log(chalk.dim(`Schema sync cancelled.`)); + process.exit(1); + } + } + snapshotDb({ db: DbClient, dst: BUN_SQLITE_TEMP_DB_FILE_PATH }); try { - const isVector = Boolean(opts.vector || opts.v); - const isTypeDef = Boolean(opts.typedef || opts.t); - - const finaldbSchema = appendDefaultFieldsToDbSchema({ - dbSchema, - }); - createDBSchema({ - db: DbClient, - db_schema: finaldbSchema, - recreate_vector_table: isVector, + ...params, + allow_destructive: opts.yes + ? true + : new Set(destructive.map((c) => c.description)), }); DbClient.close(); diff --git a/src/index.ts b/src/index.ts index f6ad547..b7a3e39 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,16 +7,25 @@ import DbUpdate from "./lib/sqlite/db-update"; import grabDbSchema from "./utils/grab-db-schema"; import grabJoinFieldsFromQueryObject from "./utils/grab-join-fields-from-query-object"; -init(); +/** + * Initialize lazily on first call, so importing the library never throws. + * Setup errors (missing config / schema) reject the call instead. + */ +function withInit Promise>(fn: F): F { + return (async (...args: Parameters) => { + init(); + return fn(...args); + }) as F; +} const BunSQLite = { - select: DbSelect, - insert: DbInsert, - update: DbUpdate, - delete: DbDelete, - sql: DbSQL, + select: withInit(DbSelect), + insert: withInit(DbInsert), + update: withInit(DbUpdate), + delete: withInit(DbDelete), + sql: withInit(DbSQL), utils: { - grab_db_schema: grabDbSchema, + grab_db_schema: withInit(grabDbSchema), grab_join_fields_from_query_object: grabJoinFieldsFromQueryObject, }, } as const; diff --git a/src/lib/schema/build-create-table-sql.ts b/src/lib/schema/build-create-table-sql.ts new file mode 100644 index 0000000..167459b --- /dev/null +++ b/src/lib/schema/build-create-table-sql.ts @@ -0,0 +1,54 @@ +import type { + BUN_SQLITE_DatabaseSchemaType, + BUN_SQLITE_TableSchemaType, +} from "../../types"; +import { sqlQuoteIdent as q } from "../../utils/sql-safe"; +import buildColumnDefinition from "./build-column-definition"; +import buildForeignKeyConstraint from "./build-foreign-key-constraint"; +import resolveTable from "./resolve-table"; + +type Params = { + db_schema: BUN_SQLITE_DatabaseSchemaType; + table: BUN_SQLITE_TableSchemaType; +}; + +/** + * Build the `CREATE TABLE` (or `CREATE VIRTUAL TABLE`) statement for a table + */ +export default function buildCreateTableSql({ db_schema, table }: Params) { + const new_table = resolveTable({ table, db_schema }); + + const columns: string[] = []; + const foreignKeys: string[] = []; + + for (const field of new_table.fields) { + columns.push(buildColumnDefinition(field, new_table.isVector)); + + if (field.foreignKey && !new_table.isVector) { + foreignKeys.push(buildForeignKeyConstraint(field)); + } + } + + // Add unique constraints + for (const constraint of new_table.uniqueConstraints || []) { + if (!constraint.constraintTableFields?.length) continue; + + const fieldNames = constraint.constraintTableFields.map((f) => + String(f.value), + ); + const constraintName = + constraint.constraintName || `unique_${fieldNames.join(", ")}`; + + columns.push( + `CONSTRAINT ${q(constraintName)} UNIQUE (${fieldNames + .map(q) + .join(", ")})`, + ); + } + + const allConstraints = [...columns, ...foreignKeys]; + + return new_table.isVector + ? `CREATE VIRTUAL TABLE ${q(new_table.tableName)} USING ${new_table.vectorType || "vec0"}(${allConstraints.join(", ")})` + : `CREATE TABLE ${q(new_table.tableName)} (${allConstraints.join(", ")})`; +} diff --git a/src/lib/schema/create-db-schema.test.ts b/src/lib/schema/create-db-schema.test.ts index e1fb65d..507d9e3 100644 --- a/src/lib/schema/create-db-schema.test.ts +++ b/src/lib/schema/create-db-schema.test.ts @@ -3,6 +3,7 @@ import { Database } from "bun:sqlite"; import type { BUN_SQLITE_FieldSchemaType, BUN_SQLITE_TableSchemaType, + SchemaChange, } from "../../types"; import createDBSchema from "./create-db-schema"; @@ -38,8 +39,13 @@ const posts: BUN_SQLITE_TableSchemaType = { let db: Database; -function sync(tables: BUN_SQLITE_TableSchemaType[]) { - createDBSchema({ db, db_schema: { tables } }); +function sync( + tables: BUN_SQLITE_TableSchemaType[], + options: { dry_run?: boolean; allow_destructive?: true | Set } = {}, +) { + const changes: SchemaChange[] = []; + createDBSchema({ db, db_schema: { tables }, changes, ...options }); + return changes.map((c) => (c.destructive ? `! ${c.description}` : c.description)); } function tableNames() { @@ -143,4 +149,48 @@ describe("createDBSchema", () => { ), ).toEqual(["id", "name"]); }); + + test("leaves unchanged tables alone", () => { + sync([users, posts]); + + expect(sync([users, posts])).toEqual([]); + }); + + test("records changes, marking data loss as destructive", () => { + sync([users, posts]); + + expect( + sync([{ ...users, fields: [id, { fieldName: "email", dataType: "TEXT" }] }]), + ).toEqual([ + `Rebuild table "users"`, + `! Drop column "users"."name"`, + `! Drop table "posts"`, + ]); + }); + + test("dry run changes nothing", () => { + sync([users, posts]); + db.run(`INSERT INTO users (name) VALUES ('ada')`); + + expect(sync([], { dry_run: true })).toEqual([ + `! Drop table "users"`, + `! Drop table "posts"`, + ]); + expect(tableNames()).toEqual(["posts", "users"]); + expect(db.query(`SELECT name FROM users`).all()).toEqual([ + { name: "ada" }, + ]); + }); + + test("refuses unapproved destructive changes and rolls back", () => { + sync([users, posts]); + + expect(() => + sync([users], { allow_destructive: new Set() }), + ).toThrow(/Refusing unapproved destructive change/); + expect(tableNames()).toEqual(["posts", "users"]); + + sync([users], { allow_destructive: new Set([`Drop table "posts"`]) }); + expect(tableNames()).toEqual(["users"]); + }); }); diff --git a/src/lib/schema/create-db-schema.ts b/src/lib/schema/create-db-schema.ts index fefc7f6..77619ea 100644 --- a/src/lib/schema/create-db-schema.ts +++ b/src/lib/schema/create-db-schema.ts @@ -8,21 +8,15 @@ import orderDBSchema from "./order-db-schema"; /** * # Sync the database to the schema - * @description Tables are rebuilt by copying into a new table and dropping - * the old one. With foreign keys ON, `DROP TABLE` runs an implicit + * @description Changed tables are rebuilt by copying into a new table and + * dropping the old one. With foreign keys ON, `DROP TABLE` runs an implicit * `DELETE FROM` that fires `ON DELETE CASCADE` on child tables, so keys are * disabled for the sync and checked before committing. (`PRAGMA - * foreign_keys` is a no-op inside a transaction.) + * foreign_keys` is a no-op inside a transaction.) With `dry_run` the + * transaction is rolled back, so `changes` is the exact plan. */ export default function createDBSchema(params: CreateDBSchemaParams) { - const { db } = params; - - console.log("Starting schema synchronization..."); - - /** - * Create Schema Manager Table - */ - createDBManagerTable({ db }); + const { db, dry_run } = params; /** * Reorder Tables (parents before children with FKs) @@ -33,6 +27,11 @@ export default function createDBSchema(params: CreateDBSchemaParams) { db.run("BEGIN"); try { + /** + * Create Schema Manager Table + */ + createDBManagerTable({ db }); + const existing_tables = getExistingTablesFromTablesManagerTable({ db, }); @@ -53,7 +52,7 @@ export default function createDBSchema(params: CreateDBSchemaParams) { * Drop tables that no longer exist in schema */ dropRemovedTables({ - db, + ...params, existing_tables, schema_tables: db_schema.tables.map((t) => t.tableName), }); @@ -72,13 +71,11 @@ export default function createDBSchema(params: CreateDBSchemaParams) { ); } - db.run("COMMIT"); + db.run(dry_run ? "ROLLBACK" : "COMMIT"); } catch (error) { if (db.inTransaction) db.run("ROLLBACK"); throw error; } finally { db.run("PRAGMA foreign_keys = ON;"); } - - console.log("Schema synchronization complete!"); } diff --git a/src/lib/schema/create-table.ts b/src/lib/schema/create-table.ts index 723144e..bf2a213 100644 --- a/src/lib/schema/create-table.ts +++ b/src/lib/schema/create-table.ts @@ -2,10 +2,8 @@ import type { BUN_SQLITE_TableSchemaType, CreateDBSchemaParams, } from "../../types"; -import { sqlQuoteIdent as q } from "../../utils/sql-safe"; -import buildColumnDefinition from "./build-column-definition"; -import buildForeignKeyConstraint from "./build-foreign-key-constraint"; -import resolveTable from "./resolve-table"; +import buildCreateTableSql from "./build-create-table-sql"; +import recordSchemaChange from "./record-schema-change"; type Params = CreateDBSchemaParams & { table: BUN_SQLITE_TableSchemaType; @@ -14,46 +12,12 @@ type Params = CreateDBSchemaParams & { /** * Create a new table */ -export default function createTable({ db, db_schema, table }: Params) { - if (!table.tableName.match(/_temp_\d+$/)) { - console.log(`Creating table: ${table.tableName}`); - } +export default function createTable(params: Params) { + const { db, db_schema, table } = params; - const new_table = resolveTable({ table, db_schema }); + recordSchemaChange(params, { + description: `Create table "${table.tableName}"`, + }); - const columns: string[] = []; - const foreignKeys: string[] = []; - - for (const field of new_table.fields) { - columns.push(buildColumnDefinition(field, new_table.isVector)); - - if (field.foreignKey && !new_table.isVector) { - foreignKeys.push(buildForeignKeyConstraint(field)); - } - } - - // Add unique constraints - for (const constraint of new_table.uniqueConstraints || []) { - if (!constraint.constraintTableFields?.length) continue; - - const fieldNames = constraint.constraintTableFields.map((f) => - String(f.value), - ); - const constraintName = - constraint.constraintName || `unique_${fieldNames.join(", ")}`; - - columns.push( - `CONSTRAINT ${q(constraintName)} UNIQUE (${fieldNames - .map(q) - .join(", ")})`, - ); - } - - const allConstraints = [...columns, ...foreignKeys]; - - const sql = new_table.isVector - ? `CREATE VIRTUAL TABLE ${q(new_table.tableName)} USING ${new_table.vectorType || "vec0"}(${allConstraints.join(", ")})` - : `CREATE TABLE ${q(new_table.tableName)} (${allConstraints.join(", ")})`; - - db.run(sql); + db.run(buildCreateTableSql({ db_schema, table })); } diff --git a/src/lib/schema/drop-removed-tables.ts b/src/lib/schema/drop-removed-tables.ts index aa30092..34f96de 100644 --- a/src/lib/schema/drop-removed-tables.ts +++ b/src/lib/schema/drop-removed-tables.ts @@ -1,10 +1,12 @@ -import type { Database } from "bun:sqlite"; +import _ from "lodash"; +import type { CreateDBSchemaParams } from "../../types"; import { readLiveSchema } from "../../functions/live-schema"; import { sqlQuoteIdent as q } from "../../utils/sql-safe"; import removeDBManagerTable from "./remove-db-manager-table"; +import recordSchemaChange from "./record-schema-change"; +import checkIfTableExists from "./check-if-table-exists"; -type Params = { - db: Database; +type Params = CreateDBSchemaParams & { existing_tables: string[]; schema_tables: string[]; }; @@ -12,12 +14,8 @@ type Params = { /** * Drop tables that are no longer in the schema */ -export default function dropRemovedTables({ - db, - existing_tables, - schema_tables, -}: Params) { - console.log(`Cleaning up tables ...`); +export default function dropRemovedTables(params: Params) { + const { db, existing_tables, schema_tables } = params; const tablesToDrop = existing_tables.filter( (t) => @@ -35,8 +33,16 @@ export default function dropRemovedTables({ } } - for (const tableName of tablesToDrop) { - console.log(`Dropping table: ${tableName}`); + for (const tableName of _.uniq(tablesToDrop)) { + if (!checkIfTableExists({ db, table_name: tableName })) { + removeDBManagerTable({ db, table_name: tableName }); + continue; + } + + recordSchemaChange(params, { + description: `Drop table "${tableName}"`, + destructive: true, + }); db.run(`DROP TABLE IF EXISTS ${q(tableName)}`); removeDBManagerTable({ db, table_name: tableName }); } diff --git a/src/lib/schema/handle-db-schema-table.ts b/src/lib/schema/handle-db-schema-table.ts index 636c1ff..cf3c012 100644 --- a/src/lib/schema/handle-db-schema-table.ts +++ b/src/lib/schema/handle-db-schema-table.ts @@ -5,6 +5,7 @@ import type { import { sqlQuoteIdent as q } from "../../utils/sql-safe"; import createTable from "./create-table"; import insertDBManagerTable from "./insert-db-manager-table"; +import recordSchemaChange from "./record-schema-change"; import removeDBManagerTable from "./remove-db-manager-table"; import syncIndexes from "./sync-indexes"; import updateTable from "./update-table"; @@ -28,9 +29,9 @@ export default function handleDBSchemaTable(params: Params) { table.tableNameOld !== table.tableName && existing_tables.includes(table.tableNameOld) ) { - console.log( - `Renaming table: ${table.tableNameOld} -> ${table.tableName}`, - ); + recordSchemaChange(params, { + description: `Rename table "${table.tableNameOld}" -> "${table.tableName}"`, + }); db.run( `ALTER TABLE ${q(table.tableNameOld)} RENAME TO ${q(table.tableName)}`, ); @@ -46,5 +47,5 @@ export default function handleDBSchemaTable(params: Params) { updateTable(params); } - syncIndexes({ db, table }); + syncIndexes(params); } diff --git a/src/lib/schema/record-schema-change.ts b/src/lib/schema/record-schema-change.ts new file mode 100644 index 0000000..6924dba --- /dev/null +++ b/src/lib/schema/record-schema-change.ts @@ -0,0 +1,35 @@ +import type { CreateDBSchemaParams, SchemaChange } from "../../types"; + +/** + * # Record a schema change before it runs + * @description Logs the change (unless this is a dry run) and refuses + * destructive changes that were not approved. + */ +export default function recordSchemaChange( + params: Pick< + CreateDBSchemaParams, + "dry_run" | "changes" | "allow_destructive" + >, + change: SchemaChange, +) { + params.changes?.push(change); + + if (params.dry_run) { + return; + } + + const allowed = params.allow_destructive; + + if ( + change.destructive && + allowed !== undefined && + allowed !== true && + !allowed.has(change.description) + ) { + throw new Error( + `Refusing unapproved destructive change: ${change.description}. Re-run with --dry-run to review, or --yes to allow it.`, + ); + } + + console.log(change.description); +} diff --git a/src/lib/schema/sync-indexes.ts b/src/lib/schema/sync-indexes.ts index 05b43a1..e9953db 100644 --- a/src/lib/schema/sync-indexes.ts +++ b/src/lib/schema/sync-indexes.ts @@ -1,16 +1,20 @@ -import type { Database } from "bun:sqlite"; -import type { BUN_SQLITE_TableSchemaType } from "../../types"; +import type { + BUN_SQLITE_TableSchemaType, + CreateDBSchemaParams, +} from "../../types"; import { sqlQuoteIdent as q } from "../../utils/sql-safe"; +import recordSchemaChange from "./record-schema-change"; -type Params = { - db: Database; +type Params = CreateDBSchemaParams & { table: BUN_SQLITE_TableSchemaType; }; /** * Sync indexes for a table */ -export default function syncIndexes({ db, table }: Params) { +export default function syncIndexes(params: Params) { + const { db, table } = params; + if (!table.indexes || table.indexes.length === 0) { return; } @@ -29,7 +33,9 @@ export default function syncIndexes({ db, table }: Params) { (idx) => idx.indexName === indexName, ); if (!stillExists) { - console.log(`Dropping index: ${indexName}`); + recordSchemaChange(params, { + description: `Drop index "${indexName}"`, + }); db.run(`DROP INDEX IF EXISTS ${q(indexName)}`); } } @@ -41,7 +47,9 @@ export default function syncIndexes({ db, table }: Params) { } if (!existingIndexes.includes(index.indexName)) { - console.log(`Creating index: ${index.indexName}`); + recordSchemaChange(params, { + description: `Create index "${index.indexName}" on "${table.tableName}"`, + }); const fields = index.indexTableFields .map((f) => q(String(f))) .join(", "); diff --git a/src/lib/schema/update-table.ts b/src/lib/schema/update-table.ts index 1c31893..9f0a39a 100644 --- a/src/lib/schema/update-table.ts +++ b/src/lib/schema/update-table.ts @@ -3,9 +3,10 @@ import type { CreateDBSchemaParams, } from "../../types"; import { sqlQuoteIdent as q } from "../../utils/sql-safe"; +import buildCreateTableSql from "./build-create-table-sql"; import checkIfTableExists from "./check-if-table-exists"; -import createTable from "./create-table"; import getTableColumns from "./get-table-columns"; +import recordSchemaChange from "./record-schema-change"; import resolveTable from "./resolve-table"; type Params = CreateDBSchemaParams & { @@ -14,36 +15,65 @@ type Params = CreateDBSchemaParams & { /** * # Update an existing table - * @description SQLite has limited ALTER TABLE support, so the table is + * @description SQLite has limited ALTER TABLE support, so a changed table is * rebuilt: create a temp table with the new schema, copy the data over, - * drop the old table and rename the temp table. + * drop the old table and rename the temp table. Unchanged tables are left + * alone. */ export default function updateTable(params: Params) { const { db, db_schema, table } = params; - console.log(`Updating table: ${table.tableName}`); - if (table.isVector) { recreateVectorTable(params); return; } + /** + * SQLite keeps the `CREATE TABLE` text (with the renamed table name), so + * an identical statement means the table already matches the schema. + */ + const current = db + .query(`SELECT sql FROM sqlite_master WHERE type='table' AND name=?`) + .get(table.tableName) as { sql: string } | null; + + if (current?.sql === buildCreateTableSql({ db_schema, table })) { + return; + } + const tempTableName = `${table.tableName}_temp_${Date.now()}`; // Columns are matched by name only: SQLite converts values to the new // column affinity on copy, so a type change keeps the data instead of // silently dropping the column. const existingColumns = getTableColumns({ db, table_name: table.tableName }); - const columnsToKeep = resolveTable({ table, db_schema }) - .fields.filter( - (f) => - f.fieldName && - existingColumns.find((c) => c.name == f.fieldName), - ) - .map((f) => f.fieldName as string); + const schemaColumns = resolveTable({ table, db_schema }).fields.map( + (f) => f.fieldName, + ); + const columnsToKeep = existingColumns + .map((c) => c.name) + .filter((name) => schemaColumns.includes(name)); + const columnsToDrop = existingColumns + .map((c) => c.name) + .filter((name) => !schemaColumns.includes(name)); + + recordSchemaChange(params, { + description: `Rebuild table "${table.tableName}"`, + }); + + for (const column of columnsToDrop) { + recordSchemaChange(params, { + description: `Drop column "${table.tableName}"."${column}"`, + destructive: true, + }); + } // Create temp table with new schema - createTable({ ...params, table: { ...table, tableName: tempTableName } }); + db.run( + buildCreateTableSql({ + db_schema, + table: { ...table, tableName: tempTableName }, + }), + ); // Copy data if there are common columns if (columnsToKeep.length > 0) { @@ -61,12 +91,16 @@ export default function updateTable(params: Params) { * Vector tables are only rebuilt when `recreate_vector_table` is set. */ function recreateVectorTable(params: Params) { - const { db, table, recreate_vector_table } = params; + const { db, db_schema, table, recreate_vector_table } = params; if (!recreate_vector_table) { return; } + recordSchemaChange(params, { + description: `Recreate vector table "${table.tableName}"`, + }); + let existingRows: { [k: string]: any }[] = []; if (checkIfTableExists({ db, table_name: table.tableName })) { @@ -77,7 +111,7 @@ function recreateVectorTable(params: Params) { db.run(`DROP TABLE ${q(table.tableName)}`); } - createTable(params); + db.run(buildCreateTableSql({ db_schema, table })); for (const row of existingRows) { const columns = Object.keys(row); diff --git a/src/lib/sqlite/db-delete.ts b/src/lib/sqlite/db-delete.ts index a8fffd1..5a60a65 100644 --- a/src/lib/sqlite/db-delete.ts +++ b/src/lib/sqlite/db-delete.ts @@ -30,25 +30,12 @@ export default async function DbDelete< let sqlObj: ReturnType | null = null; try { - let finalQuery = query || {}; - - if (targetId !== undefined && targetId !== null && targetId !== "") { - finalQuery = _.merge< - ServerQueryParam, - ServerQueryParam, - ServerQueryParam - >({}, finalQuery, { - query: { - id: { - value: String(targetId), - }, - }, - }); - } + const finalQuery = query || {}; sqlObj = sqlGenerator({ tableName: table, genObject: _.pick(finalQuery, ["query", "searchOperator"]), + targetId, }); const whereClause = sqlObj.where.string; diff --git a/src/lib/sqlite/db-select.ts b/src/lib/sqlite/db-select.ts index 0f44f3d..07f44d2 100644 --- a/src/lib/sqlite/db-select.ts +++ b/src/lib/sqlite/db-select.ts @@ -1,5 +1,4 @@ import dbHandler from "../db-handler"; -import _ from "lodash"; import type { APIResponseObject, BunSQLiteConfig, @@ -31,25 +30,12 @@ export default async function DbSelect< let sqlObj: ReturnType | null = null; try { - let finalQuery = query || {}; - - if (targetId !== undefined && targetId !== null && targetId !== "") { - finalQuery = _.merge< - ServerQueryParam, - ServerQueryParam, - ServerQueryParam - >({}, finalQuery, { - query: { - id: { - value: String(targetId), - }, - }, - }); - } + const finalQuery = query || {}; sqlObj = sqlGenerator({ tableName: table, genObject: finalQuery, + targetId, }); const sql = sqlObj.string; @@ -80,6 +66,7 @@ export default async function DbSelect< const count_sql_object = sqlGenerator({ tableName: table, genObject: finalQuery, + targetId, count, }); diff --git a/src/lib/sqlite/db-update.ts b/src/lib/sqlite/db-update.ts index ff61b10..881053b 100644 --- a/src/lib/sqlite/db-update.ts +++ b/src/lib/sqlite/db-update.ts @@ -37,25 +37,12 @@ export default async function DbUpdate< }; try { - let finalQuery = query || {}; - - if (targetId !== undefined && targetId !== null && targetId !== "") { - finalQuery = _.merge< - ServerQueryParam, - ServerQueryParam, - ServerQueryParam - >({}, finalQuery, { - query: { - id: { - value: String(targetId), - }, - }, - }); - } + const finalQuery = query || {}; const sqlQueryObj = sqlGenerator({ tableName: table, genObject: _.pick(finalQuery, ["query", "searchOperator"]), + targetId, }); let values: SQLInsertGenValueType[] = []; diff --git a/src/types/index.ts b/src/types/index.ts index eecbdc9..8b35119 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -1580,6 +1580,28 @@ export type CreateDBSchemaParams = { db: import("bun:sqlite").Database; db_schema: BUN_SQLITE_DatabaseSchemaType; recreate_vector_table?: boolean; + /** + * Run the sync inside a transaction and roll it back. The planned + * changes are recorded in `changes`. + */ + dry_run?: boolean; + /** + * Collects every change made (or planned) by the sync + */ + changes?: SchemaChange[]; + /** + * Destructive changes that may run. `true` (or unset) allows all of + * them; otherwise only changes whose `description` is in the set. + */ + allow_destructive?: true | Set; +}; + +/** + * A change made by the schema sync. Destructive changes discard data. + */ +export type SchemaChange = { + description: string; + destructive?: boolean; }; /** diff --git a/src/utils/sql-generator-gen-search-str.ts b/src/utils/sql-generator-gen-search-str.ts index 985b6ae..be94db3 100644 --- a/src/utils/sql-generator-gen-search-str.ts +++ b/src/utils/sql-generator-gen-search-str.ts @@ -83,7 +83,10 @@ export default function sqlGenGenSearchStr({ .filter(isPresent); const equality = queryObj.equality; - if (equality == "IN" || equality == "NOT IN") { + if ((equality == "IN" || equality == "NOT IN") && !rawValues.length) { + // Empty IN () is invalid SQL: IN matches nothing, NOT IN matches everything + str = equality == "IN" ? "1 = 0" : "1 = 1"; + } else if (equality == "IN" || equality == "NOT IN") { str = `${finalFieldName} ${equality} (${rawValues .map(() => "?") .join(", ")})`; diff --git a/src/utils/sql-generator.ts b/src/utils/sql-generator.ts index 0189bb5..376cca0 100644 --- a/src/utils/sql-generator.ts +++ b/src/utils/sql-generator.ts @@ -18,6 +18,11 @@ type Param = { tableName: string; dbFullName?: string; count?: boolean; + /** + * Restrict to a single row id. Always combined with AND, regardless of + * `searchOperator`. + */ + targetId?: number | string; }; type Return = { @@ -40,7 +45,12 @@ type Return = { */ export default function sqlGenerator< T extends { [key: string]: any } = { [key: string]: any }, ->({ tableName: passedTableName, genObject, count }: Param): Return { +>({ + tableName: passedTableName, + genObject, + count, + targetId, +}: Param): Return { const tableName = sqlIdent(passedTableName, "table name"); const finalQuery = genObject?.query ? genObject.query : undefined; @@ -115,11 +125,31 @@ export default function sqlGenerator< (str) => typeof str == "string", ); - let whereString = ""; + const whereConditions: string[] = []; if (cleanedUpSearchStr?.[0]) { const stringOperator = sqlOperator(genObject?.searchOperator); - whereString = `WHERE ${cleanedUpSearchStr.join(` ${stringOperator} `)}`; + whereConditions.push( + `(${cleanedUpSearchStr.join(` ${stringOperator} `)})`, + ); + } + + if (targetId !== undefined && targetId !== null && targetId !== "") { + const id = Number(targetId); + + if (!Number.isSafeInteger(id)) { + throw new Error(`Invalid targetId: ${JSON.stringify(targetId)}`); + } + + whereConditions.push(`${genObject?.join ? `${tableName}.` : ""}id = ?`); + whereValues.push(id); + } + + const whereString = whereConditions.length + ? `WHERE ${whereConditions.join(" AND ")}` + : ""; + + if (whereString) { queryString += ` ${whereString}`; sqlSearhValues.push(...whereValues); } diff --git a/tests/sql-generator.test.ts b/tests/sql-generator.test.ts new file mode 100644 index 0000000..6bc3ca5 --- /dev/null +++ b/tests/sql-generator.test.ts @@ -0,0 +1,135 @@ +import { describe, expect, test } from "bun:test"; +import sqlGenerator from "../src/utils/sql-generator"; + +const gen = (genObject: any, extra: Record = {}) => + sqlGenerator({ tableName: "users", genObject, ...extra }); + +describe("identifiers", () => { + test("binds values and keeps identifiers", () => { + const { string, values } = gen({ query: { email: { value: "a@b.c" } } }); + expect(string).toBe("SELECT * FROM users WHERE (email = ?)"); + expect(values).toEqual(["a@b.c"]); + }); + + test.each([ + ["field name", { query: { "1=1 OR id": { value: 1 } } }], + ["quoted field name", { query: { 'a" OR 1=1 --': { value: 1 } } }], + ["select field", { selectFields: ["name; DROP TABLE users"] }], + ])("rejects an injected %s", (_name, genObject) => { + expect(() => gen(genObject)).toThrow(); + }); + + test("rejects an injected table name", () => { + expect(() => + sqlGenerator({ tableName: "users; DROP TABLE users", genObject: {} }), + ).toThrow(); + }); +}); + +describe("input validation", () => { + test.each([ + ["order strategy", { order: { field: "id", strategy: "DESC; DROP TABLE x" } }], + ["limit", { limit: "1; --" }], + ["negative offset", { offset: -1 }], + ["page", { limit: 10, page: "2 OR 1" }], + ["search operator", { query: { a: { value: 1 } }, searchOperator: "OR 1=1" }], + ["raw equality", { query: { a: { value: 1, raw_equality: "= 1 OR" } } }], + ["join type", { join: [{ tableName: "p", joinType: "CROSS JOIN x;", match: { source: "id", target: "uid" } }] }], + ])("rejects invalid %s", (_name, genObject) => { + expect(() => gen(genObject)).toThrow(); + }); + + test("rejects non-numeric targetId", () => { + expect(() => gen({}, { targetId: "abc" })).toThrow(); + }); +}); + +describe("targetId", () => { + test("is ANDed with the query, even with searchOperator OR", () => { + const { string, values } = gen( + { query: { a: { value: 1 }, b: { value: 2 } }, searchOperator: "OR" }, + { targetId: 5 }, + ); + expect(string).toBe( + "SELECT * FROM users WHERE (a = ? OR b = ?) AND id = ?", + ); + expect(values).toEqual([1, 2, 5]); + }); + + test("accepts numeric strings", () => { + const { string, values } = gen({}, { targetId: "7" }); + expect(string).toBe("SELECT * FROM users WHERE id = ?"); + expect(values).toEqual([7]); + }); + + test("0 is a real id", () => { + expect(gen({}, { targetId: 0 }).values).toEqual([0]); + }); + + test("is table-qualified with joins", () => { + const { string } = gen( + { + join: [ + { + tableName: "posts", + joinType: "LEFT JOIN", + match: { source: "id", target: "user_id" }, + }, + ], + }, + { targetId: 3 }, + ); + expect(string).toEndWith("WHERE users.id = ?"); + }); + + test("is part of the where clause used by update / delete", () => { + const { where } = gen({ query: { a: { value: 1 } } }, { targetId: 2 }); + expect(where).toEqual({ + string: "WHERE (a = ?) AND id = ?", + values: [1, 2], + }); + }); +}); + +describe("values", () => { + test("0 is a real value", () => { + expect(gen({ query: { age: { value: 0 } } }).values).toEqual([0]); + }); + + test("IN binds each value", () => { + const { string, values } = gen({ + query: { id: { value: [1, 2], equality: "IN" } }, + }); + expect(string).toBe("SELECT * FROM users WHERE (id IN (?, ?))"); + expect(values).toEqual([1, 2]); + }); + + test("empty IN matches nothing, empty NOT IN matches everything", () => { + expect(gen({ query: { id: { value: [], equality: "IN" } } }).string).toBe( + "SELECT * FROM users WHERE (1 = 0)", + ); + expect( + gen({ query: { id: { value: [], equality: "NOT IN" } } }).string, + ).toBe("SELECT * FROM users WHERE (1 = 1)"); + }); + + test("BETWEEN needs exactly two values", () => { + expect(() => + gen({ query: { age: { value: [1], equality: "BETWEEN" } } }), + ).toThrow(); + }); +}); + +describe("pagination", () => { + test("page is converted to an offset", () => { + expect(gen({ limit: 10, page: 3 }).string).toBe( + "SELECT * FROM users LIMIT 10 OFFSET 20", + ); + }); + + test("count queries skip pagination", () => { + expect(gen({ limit: 10, page: 3 }, { count: true }).string).not.toContain( + "LIMIT", + ); + }); +}); diff --git a/tests/sql-values.test.ts b/tests/sql-values.test.ts new file mode 100644 index 0000000..f4a0a72 --- /dev/null +++ b/tests/sql-values.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, test } from "bun:test"; +import sqlInsertGenerator, { sqlValue } from "../src/utils/sql-insert-generator"; + +describe("sqlValue", () => { + test("booleans become 0 / 1", () => { + expect(sqlValue(false)).toBe(0); + expect(sqlValue(true)).toBe(1); + }); + + test("0 and empty strings are kept", () => { + expect(sqlValue(0)).toBe(0); + expect(sqlValue("")).toBe(""); + }); + + test("undefined becomes NULL", () => { + expect(sqlValue(undefined)).toBeNull(); + }); +}); + +describe("sqlInsertGenerator", () => { + test("builds a multi-row insert", () => { + const res = sqlInsertGenerator({ + tableName: "items", + data: [ + { name: "a", active: false, qty: 0 }, + { name: "b", note: "" }, + ], + }); + expect(res?.query).toBe( + "INSERT INTO items (name,active,qty,note) VALUES (?,?,?,?),(?,?,?,?)", + ); + expect(res?.values).toEqual(["a", 0, 0, null, "b", null, null, ""]); + }); + + test("function values are called once", () => { + let calls = 0; + const res = sqlInsertGenerator({ + tableName: "items", + data: [ + { + emb: () => { + calls++; + return { value: "[1,2]", placeholder: "vec_f32(?)" }; + }, + }, + ], + }); + expect(calls).toBe(1); + expect(res?.query).toBe("INSERT INTO items (emb) VALUES (vec_f32(?))"); + expect(res?.values).toEqual(["[1,2]"]); + }); + + test("rejects injected column names", () => { + expect(() => + sqlInsertGenerator({ + tableName: "items", + data: [{ "name) VALUES (1); --": "x" }], + }), + ).toThrow(); + }); +});