diff --git a/README.md b/README.md index 3627cda..8747e34 100644 --- a/README.md +++ b/README.md @@ -307,15 +307,41 @@ type BUN_MARIADB_FieldSchemaType = { ### Foreign Key ```ts +type BUN_MARIADB_ForeignKeyRule = + | "CASCADE" + | "SET NULL" + | "RESTRICT" + | "NO ACTION"; + interface BUN_MARIADB_ForeignKeyType { foreignKeyName?: string; destinationTableName?: string; destinationTableColumnName?: string; - cascadeDelete?: boolean; - cascadeUpdate?: boolean; + cascadeDelete?: boolean; // shorthand for onDelete: "CASCADE" + cascadeUpdate?: boolean; // shorthand for onUpdate: "CASCADE" + onDelete?: BUN_MARIADB_ForeignKeyRule; // default RESTRICT + onUpdate?: BUN_MARIADB_ForeignKeyRule; // default RESTRICT } ``` +```ts +{ + fieldName: "author_id", + dataType: "BIGINT", + foreignKey: { + foreignKeyName: "post_author_fk", + destinationTableName: "users", + destinationTableColumnName: "id", + onDelete: "SET NULL", // column must be nullable + }, +} +``` + +- `onDelete` / `onUpdate` take precedence over the `cascadeDelete` / `cascadeUpdate` shorthands. The deprecated field-level `onDeleteLiteral` / `onDelete` are still honored as the FK's ON DELETE action when `foreignKey.onDelete` is not set. +- `SET NULL` on a `notNullValue` or primary key column is rejected when the schema syncs. +- Changing a rule on an existing FK drops and recreates the constraint. `NO ACTION` and `RESTRICT` are treated as the same rule, so switching between them causes no churn. +- Foreign keys are created after every table exists. Self references (e.g. `comments.parent_comment_id`), forward references and cycles between tables are all supported. + ### Index ```ts diff --git a/dist/lib/schema/build-foreign-key-constraint.d.ts b/dist/lib/schema/build-foreign-key-constraint.d.ts index 15c1809..75d2bcd 100644 --- a/dist/lib/schema/build-foreign-key-constraint.d.ts +++ b/dist/lib/schema/build-foreign-key-constraint.d.ts @@ -1,4 +1,20 @@ -import type { BUN_MARIADB_FieldSchemaType } from "../../types"; +import { type BUN_MARIADB_FieldSchemaType, type BUN_MARIADB_ForeignKeyRule } from "../../types"; export declare function defaultForeignKeyName(tableName: string, fieldName: string): string; export declare function resolveForeignKeyName(field: BUN_MARIADB_FieldSchemaType, tableName: string): string; +/** + * Normalize a referential action as MariaDB reports it in + * `information_schema.REFERENTIAL_CONSTRAINTS`. InnoDB treats `NO ACTION` + * exactly like `RESTRICT`, so both compare as `RESTRICT`. + */ +export declare function normalizeForeignKeyRule(rule: string | undefined | null): BUN_MARIADB_ForeignKeyRule; +/** + * Resolve the ON DELETE / ON UPDATE actions for a field's foreign key. + * Precedence: `foreignKey.onDelete` > field `onDeleteLiteral` / `onDelete` + * (legacy) > `cascadeDelete` > `RESTRICT`. Same for update, except the + * field-level `onUpdate*` keys are column `ON UPDATE` clauses, not FK rules. + */ +export declare function resolveForeignKeyRules(field: BUN_MARIADB_FieldSchemaType, tableName?: string): { + onDelete: BUN_MARIADB_ForeignKeyRule; + onUpdate: BUN_MARIADB_ForeignKeyRule; +}; export default function buildForeignKeyConstraint(field: BUN_MARIADB_FieldSchemaType, tableName: string): string; diff --git a/dist/lib/schema/build-foreign-key-constraint.js b/dist/lib/schema/build-foreign-key-constraint.js index fa89d41..087c8d3 100644 --- a/dist/lib/schema/build-foreign-key-constraint.js +++ b/dist/lib/schema/build-foreign-key-constraint.js @@ -1,3 +1,4 @@ +import { BUN_MARIADB_ForeignKeyRules, } from "../../types"; import MariaDBQuoteGen from "./mariadb-quote-gen"; export function defaultForeignKeyName(tableName, fieldName) { return `fk_${tableName}_${fieldName}`; @@ -6,15 +7,52 @@ export function resolveForeignKeyName(field, tableName) { const fieldName = field.fieldName || "column"; return field.foreignKey?.foreignKeyName || defaultForeignKeyName(tableName, fieldName); } +/** + * Normalize a referential action as MariaDB reports it in + * `information_schema.REFERENTIAL_CONSTRAINTS`. InnoDB treats `NO ACTION` + * exactly like `RESTRICT`, so both compare as `RESTRICT`. + */ +export function normalizeForeignKeyRule(rule) { + const normalized = String(rule || "RESTRICT") + .trim() + .toUpperCase() + .replace(/[_\s]+/g, " "); + if (normalized === "NO ACTION") + return "RESTRICT"; + if (!BUN_MARIADB_ForeignKeyRules.includes(normalized)) { + throw new Error(`Invalid foreign key rule \`${rule}\`. Expected one of: ${BUN_MARIADB_ForeignKeyRules.join(", ")}`); + } + return normalized; +} +/** + * Resolve the ON DELETE / ON UPDATE actions for a field's foreign key. + * Precedence: `foreignKey.onDelete` > field `onDeleteLiteral` / `onDelete` + * (legacy) > `cascadeDelete` > `RESTRICT`. Same for update, except the + * field-level `onUpdate*` keys are column `ON UPDATE` clauses, not FK rules. + */ +export function resolveForeignKeyRules(field, tableName) { + const fk = field.foreignKey || {}; + const onDelete = normalizeForeignKeyRule(fk.onDelete || + field.onDeleteLiteral || + field.onDelete || + (fk.cascadeDelete ? "CASCADE" : "RESTRICT")); + const onUpdate = normalizeForeignKeyRule(fk.onUpdate || (fk.cascadeUpdate ? "CASCADE" : "RESTRICT")); + if ((onDelete === "SET NULL" || onUpdate === "SET NULL") && + (field.notNullValue || field.primaryKey)) { + throw new Error(`Foreign key on \`${tableName ? `${tableName}.` : ""}${field.fieldName}\` uses SET NULL but the column is NOT NULL`); + } + return { onDelete, onUpdate }; +} export default function buildForeignKeyConstraint(field, tableName) { const fk = field.foreignKey; const constraintName = resolveForeignKeyName(field, tableName); + const { onDelete, onUpdate } = resolveForeignKeyRules(field, tableName); let constraint = `CONSTRAINT ${MariaDBQuoteGen(constraintName)} FOREIGN KEY (${MariaDBQuoteGen(field.fieldName)}) REFERENCES ${MariaDBQuoteGen(fk.destinationTableName)}(${MariaDBQuoteGen(fk.destinationTableColumnName)})`; - if (fk.cascadeDelete) { - constraint += " ON DELETE CASCADE"; + if (onDelete !== "RESTRICT") { + constraint += ` ON DELETE ${onDelete}`; } - if (fk.cascadeUpdate) { - constraint += " ON UPDATE CASCADE"; + if (onUpdate !== "RESTRICT") { + constraint += ` ON UPDATE ${onUpdate}`; } return constraint; } diff --git a/dist/lib/schema/create-table.js b/dist/lib/schema/create-table.js index e6303ac..0cabd94 100644 --- a/dist/lib/schema/create-table.js +++ b/dist/lib/schema/create-table.js @@ -1,5 +1,4 @@ import buildColumnDefinition from "./build-column-definition"; -import buildForeignKeyConstraint from "./build-foreign-key-constraint"; import buildTableOptions from "./build-table-options"; import MariaDBQuoteGen from "./mariadb-quote-gen"; import runSchemaQuery from "./run-schema-query"; @@ -8,16 +7,12 @@ export default async function createTable({ table, config, }) { console.log(`Creating table: ${table.tableName}`); } const columnDefinitions = []; - const foreignKeys = []; const primaryKeys = []; for (const field of table.fields || []) { columnDefinitions.push(buildColumnDefinition(field)); if (field.primaryKey && field.fieldName) { primaryKeys.push(field.fieldName); } - if (field.foreignKey) { - foreignKeys.push(buildForeignKeyConstraint(field, table.tableName)); - } } if (primaryKeys.length > 0) { const pkCols = primaryKeys.map((k) => MariaDBQuoteGen(k)).join(", "); @@ -36,6 +31,11 @@ export default async function createTable({ table, config, }) { columnDefinitions.push(`CONSTRAINT ${MariaDBQuoteGen(constraintName)} UNIQUE (${fields})`); } } - const sql = `CREATE TABLE IF NOT EXISTS ${MariaDBQuoteGen(table.tableName)} (${[...columnDefinitions, ...foreignKeys].join(", ")})${buildTableOptions(table)}`; + /** + * Foreign keys are not declared inline. `handleDBSchemaTables` adds them + * with `ensureForeignKeys` once every table exists, so forward references, + * self references and FK cycles all work regardless of table order. + */ + const sql = `CREATE TABLE IF NOT EXISTS ${MariaDBQuoteGen(table.tableName)} (${columnDefinitions.join(", ")})${buildTableOptions(table)}`; await runSchemaQuery({ query: sql, config }); } diff --git a/dist/lib/schema/handle-db-schema-table.js b/dist/lib/schema/handle-db-schema-table.js index 35b1b5b..6a28dfc 100644 --- a/dist/lib/schema/handle-db-schema-table.js +++ b/dist/lib/schema/handle-db-schema-table.js @@ -3,7 +3,7 @@ import MariaDBQuoteGen from "./mariadb-quote-gen"; import resolveTable from "./resolve-table"; import runSchemaQuery, { querySchemaRows } from "./run-schema-query"; import schemaCondition from "./schema-condition"; -import { dropObsoleteForeignKeys, ensureForeignKeys, } from "./sync-foreign-keys"; +import { dropObsoleteForeignKeys } from "./sync-foreign-keys"; import syncIndexes from "./sync-indexes"; import syncPrimaryKey from "./sync-primary-key"; import syncTableOptions from "./sync-table-options"; @@ -64,11 +64,11 @@ export default async function handleDBSchemaTable({ db_schema, config, table, db // 2. Drop obsolete/changed FKs (unlocks index cleanup) // 3. Primary key // 4. Indexes / uniques (FK columns need supporting indexes) - // 5. Ensure foreign keys exist + // Foreign keys are ensured by `handleDBSchemaTables` after every table + // has been created/updated. await syncTableOptions({ table: resolvedTable, config }); await dropObsoleteForeignKeys({ table: resolvedTable, config }); await syncPrimaryKey({ table: resolvedTable, config }); await syncIndexes({ table: resolvedTable, config }); await syncUniqueConstraints({ table: resolvedTable, config }); - await ensureForeignKeys({ table: resolvedTable, config }); } diff --git a/dist/lib/schema/handle-db-schema-tables.js b/dist/lib/schema/handle-db-schema-tables.js index da6f5f3..29f59f0 100644 --- a/dist/lib/schema/handle-db-schema-tables.js +++ b/dist/lib/schema/handle-db-schema-tables.js @@ -3,6 +3,8 @@ import { AppData } from "../../data/app-data"; import handleDBSchemaTable from "./handle-db-schema-table"; import getExistingTablesFromTablesManagerTable from "./get-existing-tables-from-tables-manager-table"; import MariaDBQuoteGen from "./mariadb-quote-gen"; +import resolveTable from "./resolve-table"; +import { ensureForeignKeys } from "./sync-foreign-keys"; import runSchemaQuery, { querySchemaRows } from "./run-schema-query"; import schemaCondition from "./schema-condition"; import { removeDbManagerTable } from "./upsert-db-manager-table"; @@ -32,6 +34,18 @@ export default async function handleDBSchemaTables(params) { existing_live_table, }); } + /** + * Foreign keys last: every referenced table now exists, so forward + * references, self references and cycles between tables all resolve. + */ + for (const table of db_schema.tables) { + if (!table) + continue; + await ensureForeignKeys({ + table: resolveTable(table, db_schema), + config, + }); + } /** * Drop tables tracked by the manager but no longer in the schema. * Skip drops when remaining schema tables still reference the table via FK diff --git a/dist/lib/schema/order-db-schema.d.ts b/dist/lib/schema/order-db-schema.d.ts index 52a43c0..058cd24 100644 --- a/dist/lib/schema/order-db-schema.d.ts +++ b/dist/lib/schema/order-db-schema.d.ts @@ -1,2 +1,13 @@ import type { BUN_MARIADB_DatabaseSchemaType, CreateDBSchemaParams } from "../../types"; +/** + * Topologically order tables so referenced (parent) tables come before the + * tables that reference them. Declaration order is kept wherever the FK + * graph allows it. + * + * - Self references (e.g. `comments.parent_comment_id`) are ignored. + * - References to tables outside the schema are ignored. + * - Cycles do not throw: a table already on the current path is skipped, + * so every table is still emitted exactly once. Foreign keys are created + * after all tables exist, so cycles are safe to sync. + */ export default function orderDBSchema({ db_schema, }: CreateDBSchemaParams): Promise; diff --git a/dist/lib/schema/order-db-schema.js b/dist/lib/schema/order-db-schema.js index 1555a8d..a525c82 100644 --- a/dist/lib/schema/order-db-schema.js +++ b/dist/lib/schema/order-db-schema.js @@ -1,57 +1,49 @@ import _ from "lodash"; +/** + * Topologically order tables so referenced (parent) tables come before the + * tables that reference them. Declaration order is kept wherever the FK + * graph allows it. + * + * - Self references (e.g. `comments.parent_comment_id`) are ignored. + * - References to tables outside the schema are ignored. + * - Cycles do not throw: a table already on the current path is skipped, + * so every table is still emitted exactly once. Foreign keys are created + * after all tables exist, so cycles are safe to sync. + */ export default async function orderDBSchema({ db_schema, }) { - let new_db_schema = _.cloneDeep(db_schema); + const new_db_schema = _.cloneDeep(db_schema); const tables = new_db_schema.tables; - let new_tables_set = new Set(); - let new_tables_start_set = new Set(); - let new_tables_end_set = new Set(); - function setParentTable(table) { - const fields = table.fields; - let does_table_have_foreign_keys = false; - for (let f = 0; f < fields.length; f++) { - const field = fields[f]; - if (!field) + const tables_by_name = new Map(); + for (const table of tables) { + if (table) + tables_by_name.set(table.tableName, table); + } + const ordered = []; + const visited = new Set(); + const visiting = new Set(); + function visit(table) { + if (visited.has(table.tableName) || visiting.has(table.tableName)) { + return; + } + visiting.add(table.tableName); + for (const field of table.fields || []) { + const dst_table_name = field?.foreignKey?.destinationTableName; + if (!dst_table_name || dst_table_name === table.tableName) { continue; - const dst_table_name = field.foreignKey?.destinationTableName; - if (dst_table_name) { - const fk_table = tables.find((tb) => tb.tableName == dst_table_name); - if (fk_table && - !new_tables_start_set.has(fk_table) && - !new_tables_end_set.has(fk_table)) { - setParentTable(fk_table); - } - new_tables_end_set.add(table); - if (fk_table) { - new_tables_start_set.add(fk_table); - } - does_table_have_foreign_keys = true; + } + const fk_table = tables_by_name.get(dst_table_name); + if (fk_table) { + visit(fk_table); } } - return { does_table_have_foreign_keys }; + visiting.delete(table.tableName); + visited.add(table.tableName); + ordered.push(table); } - for (let i = 0; i < tables.length; i++) { - const table = tables[i]; - if (!table) { - continue; - } - let { does_table_have_foreign_keys } = setParentTable(table); - if (does_table_have_foreign_keys) { - new_tables_end_set.add(table); - } - else { - new_tables_start_set.add(table); - } + for (const table of tables) { + if (table) + visit(table); } - const parsed_tables = [ - ...Array.from(new_tables_start_set), - ...Array.from(new_tables_end_set), - ]; - for (let nt = 0; nt < parsed_tables.length; nt++) { - const new_table = parsed_tables[nt]; - if (new_table) { - new_tables_set.add(new_table); - } - } - new_db_schema.tables = Array.from(new_tables_set); + new_db_schema.tables = ordered; return new_db_schema; } diff --git a/dist/lib/schema/sync-foreign-keys.d.ts b/dist/lib/schema/sync-foreign-keys.d.ts index d80460d..4cfcc80 100644 --- a/dist/lib/schema/sync-foreign-keys.d.ts +++ b/dist/lib/schema/sync-foreign-keys.d.ts @@ -1,11 +1,11 @@ -import type { BUN_MARIADB_FieldSchemaType, BUN_MARIADB_TableSchemaType, BunMariaDBConfig } from "../../types"; +import type { BUN_MARIADB_FieldSchemaType, BUN_MARIADB_ForeignKeyRule, BUN_MARIADB_TableSchemaType, BunMariaDBConfig } from "../../types"; export type DesiredForeignKey = { name: string; column: string; refTable: string; refColumn: string; - cascadeDelete: boolean; - cascadeUpdate: boolean; + onDelete: BUN_MARIADB_ForeignKeyRule; + onUpdate: BUN_MARIADB_ForeignKeyRule; field: BUN_MARIADB_FieldSchemaType; }; export type LiveForeignKey = { diff --git a/dist/lib/schema/sync-foreign-keys.js b/dist/lib/schema/sync-foreign-keys.js index 693004c..8eaccc7 100644 --- a/dist/lib/schema/sync-foreign-keys.js +++ b/dist/lib/schema/sync-foreign-keys.js @@ -1,18 +1,12 @@ -import buildForeignKeyConstraint, { resolveForeignKeyName, } from "./build-foreign-key-constraint"; +import buildForeignKeyConstraint, { normalizeForeignKeyRule, resolveForeignKeyName, resolveForeignKeyRules, } from "./build-foreign-key-constraint"; import MariaDBQuoteGen from "./mariadb-quote-gen"; import runSchemaQuery, { querySchemaRows } from "./run-schema-query"; -function normalizeRule(rule) { - return String(rule || "RESTRICT").toUpperCase().replace(/_/g, " "); -} -function ruleIsCascade(rule) { - return normalizeRule(rule) === "CASCADE"; -} function fkIdentityKey(fk) { return `${fk.column}\0${fk.refTable}\0${fk.refColumn}`; } function rulesMatch(live, desired) { - return (ruleIsCascade(live.deleteRule) === desired.cascadeDelete && - ruleIsCascade(live.updateRule) === desired.cascadeUpdate); + return (normalizeForeignKeyRule(live.deleteRule) === desired.onDelete && + normalizeForeignKeyRule(live.updateRule) === desired.onUpdate); } export function grabDesiredForeignKeys(table) { const desired = []; @@ -23,13 +17,14 @@ export function grabDesiredForeignKeys(table) { !fk.destinationTableColumnName) { continue; } + const { onDelete, onUpdate } = resolveForeignKeyRules(field, table.tableName); desired.push({ name: resolveForeignKeyName(field, table.tableName), column: field.fieldName, refTable: fk.destinationTableName, refColumn: fk.destinationTableColumnName, - cascadeDelete: Boolean(fk.cascadeDelete), - cascadeUpdate: Boolean(fk.cascadeUpdate), + onDelete, + onUpdate, field, }); } diff --git a/dist/lib/schema/sync-indexes.js b/dist/lib/schema/sync-indexes.js index 2a58642..33744aa 100644 --- a/dist/lib/schema/sync-indexes.js +++ b/dist/lib/schema/sync-indexes.js @@ -58,12 +58,12 @@ async function createIndex({ table, index, config, }) { }); return; } - console.log(`Creating standard index: ${index.indexName}`); const fields = index.indexTableFields .map((field) => MariaDBQuoteGen(field)) .join(", "); const typeUpper = index.indexType?.toUpperCase(); const isSpecialType = typeUpper === "FULLTEXT" || typeUpper === "SPATIAL"; + console.log(`Creating ${isSpecialType ? typeUpper : "standard"} index: ${index.indexName}`); const indexPrefix = isSpecialType ? `${typeUpper} ` : ""; const indexSuffix = !isSpecialType && (typeUpper === "BTREE" || typeUpper === "HASH") ? ` USING ${typeUpper}` diff --git a/dist/types/index.d.ts b/dist/types/index.d.ts index a29f462..f568448 100644 --- a/dist/types/index.d.ts +++ b/dist/types/index.d.ts @@ -206,7 +206,15 @@ export type BUN_MARIADB_FieldSchemaType = { patternFlags?: string; onUpdate?: string; onUpdateLiteral?: string; + /** + * @deprecated Use `foreignKey.onDelete`. Still honored as the foreign + * key's ON DELETE action when `foreignKey.onDelete` is not set. + */ onDelete?: string; + /** + * @deprecated Use `foreignKey.onDelete`. Still honored as the foreign + * key's ON DELETE action when `foreignKey.onDelete` is not set. + */ onDeleteLiteral?: string; cssFiles?: string[]; /** @@ -224,13 +232,32 @@ export type BUN_MARIADB_FieldSchemaType = { /** * Defines a foreign key relationship from one field to another table column. */ +export declare const BUN_MARIADB_ForeignKeyRules: readonly ["CASCADE", "SET NULL", "RESTRICT", "NO ACTION"]; +export type BUN_MARIADB_ForeignKeyRule = (typeof BUN_MARIADB_ForeignKeyRules)[number]; export interface BUN_MARIADB_ForeignKeyType { foreignKeyName?: string; destinationTableName?: string; destinationTableColumnName?: string; destinationTableColumnType?: string; + /** + * Shorthand for `onDelete: "CASCADE"`. Ignored when `onDelete` is set. + */ cascadeDelete?: boolean; + /** + * Shorthand for `onUpdate: "CASCADE"`. Ignored when `onUpdate` is set. + */ cascadeUpdate?: boolean; + /** + * Referential action when the referenced row is deleted. + * Defaults to `CASCADE` when `cascadeDelete` is true, else `RESTRICT`. + * `SET NULL` requires a nullable column. + */ + onDelete?: BUN_MARIADB_ForeignKeyRule; + /** + * Referential action when the referenced key is updated. + * Defaults to `CASCADE` when `cascadeUpdate` is true, else `RESTRICT`. + */ + onUpdate?: BUN_MARIADB_ForeignKeyRule; } export declare const MariaDBIndexTypes: readonly ["BTREE", "HASH", "FULLTEXT", "SPATIAL", "VECTOR"]; export type MariaDBIndexType = (typeof MariaDBIndexTypes)[number]; diff --git a/dist/types/index.js b/dist/types/index.js index e5b858f..4a89e5a 100644 --- a/dist/types/index.js +++ b/dist/types/index.js @@ -79,6 +79,15 @@ export const BUN_MARIADB_DATATYPES = [ { value: "SET" }, { value: "VECTOR" }, ]; +/** + * Defines a foreign key relationship from one field to another table column. + */ +export const BUN_MARIADB_ForeignKeyRules = [ + "CASCADE", + "SET NULL", + "RESTRICT", + "NO ACTION", +]; export const MariaDBIndexTypes = [ "BTREE", "HASH", diff --git a/package.json b/package.json index eb0b577..f212471 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@moduletrace/bun-mariadb", - "version": "1.0.20", + "version": "1.0.21", "description": "Schema-driven MariaDB manager for Bun", "author": "Benjamin Toby", "license": "MIT", diff --git a/src/lib/schema/build-foreign-key-constraint.test.ts b/src/lib/schema/build-foreign-key-constraint.test.ts new file mode 100644 index 0000000..7a75496 --- /dev/null +++ b/src/lib/schema/build-foreign-key-constraint.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, test } from "bun:test"; +import type { BUN_MARIADB_FieldSchemaType } from "../../types"; +import buildForeignKeyConstraint, { + normalizeForeignKeyRule, + resolveForeignKeyRules, +} from "./build-foreign-key-constraint"; + +function field( + overrides: Partial = {}, + fk: NonNullable = {}, +): BUN_MARIADB_FieldSchemaType { + return { + fieldName: "user_id", + dataType: "BIGINT", + ...overrides, + foreignKey: { + foreignKeyName: "post_user_fk", + destinationTableName: "users", + destinationTableColumnName: "id", + ...fk, + }, + }; +} + +describe("resolveForeignKeyRules", () => { + test("defaults to RESTRICT", () => { + expect(resolveForeignKeyRules(field())).toEqual({ + onDelete: "RESTRICT", + onUpdate: "RESTRICT", + }); + }); + + test("cascadeDelete / cascadeUpdate shorthands", () => { + expect( + resolveForeignKeyRules( + field({}, { cascadeDelete: true, cascadeUpdate: true }), + ), + ).toEqual({ onDelete: "CASCADE", onUpdate: "CASCADE" }); + }); + + test("foreignKey.onDelete wins over cascadeDelete", () => { + expect( + resolveForeignKeyRules( + field({}, { cascadeDelete: true, onDelete: "SET NULL" }), + ).onDelete, + ).toBe("SET NULL"); + }); + + test("legacy field-level onDeleteLiteral is honored", () => { + expect( + resolveForeignKeyRules( + field({ onDeleteLiteral: "SET NULL" }, { cascadeDelete: false }), + ).onDelete, + ).toBe("SET NULL"); + }); + + test("SET NULL on a NOT NULL column throws", () => { + expect(() => + resolveForeignKeyRules( + field({ notNullValue: true }, { onDelete: "SET NULL" }), + "posts", + ), + ).toThrow("posts.user_id"); + }); +}); + +describe("normalizeForeignKeyRule", () => { + test("treats NO ACTION as RESTRICT and normalizes case", () => { + expect(normalizeForeignKeyRule("NO ACTION")).toBe("RESTRICT"); + expect(normalizeForeignKeyRule("set_null")).toBe("SET NULL"); + expect(normalizeForeignKeyRule(null)).toBe("RESTRICT"); + }); + + test("rejects unknown rules", () => { + expect(() => normalizeForeignKeyRule("DROP TABLE")).toThrow(); + }); +}); + +describe("buildForeignKeyConstraint", () => { + test("emits ON DELETE SET NULL", () => { + expect( + buildForeignKeyConstraint( + field({}, { onDelete: "SET NULL" }), + "posts", + ), + ).toBe( + "CONSTRAINT `post_user_fk` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE SET NULL", + ); + }); + + test("omits clauses for RESTRICT", () => { + expect(buildForeignKeyConstraint(field(), "posts")).toBe( + "CONSTRAINT `post_user_fk` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`)", + ); + }); + + test("emits both cascade clauses", () => { + expect( + buildForeignKeyConstraint( + field({}, { cascadeDelete: true, cascadeUpdate: true }), + "posts", + ), + ).toBe( + "CONSTRAINT `post_user_fk` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE CASCADE ON UPDATE CASCADE", + ); + }); +}); diff --git a/src/lib/schema/build-foreign-key-constraint.ts b/src/lib/schema/build-foreign-key-constraint.ts index 76ac498..4b71c28 100644 --- a/src/lib/schema/build-foreign-key-constraint.ts +++ b/src/lib/schema/build-foreign-key-constraint.ts @@ -1,4 +1,8 @@ -import type { BUN_MARIADB_FieldSchemaType } from "../../types"; +import { + BUN_MARIADB_ForeignKeyRules, + type BUN_MARIADB_FieldSchemaType, + type BUN_MARIADB_ForeignKeyRule, +} from "../../types"; import MariaDBQuoteGen from "./mariadb-quote-gen"; export function defaultForeignKeyName( @@ -16,21 +20,81 @@ export function resolveForeignKeyName( return field.foreignKey?.foreignKeyName || defaultForeignKeyName(tableName, fieldName); } +/** + * Normalize a referential action as MariaDB reports it in + * `information_schema.REFERENTIAL_CONSTRAINTS`. InnoDB treats `NO ACTION` + * exactly like `RESTRICT`, so both compare as `RESTRICT`. + */ +export function normalizeForeignKeyRule( + rule: string | undefined | null, +): BUN_MARIADB_ForeignKeyRule { + const normalized = String(rule || "RESTRICT") + .trim() + .toUpperCase() + .replace(/[_\s]+/g, " "); + + if (normalized === "NO ACTION") return "RESTRICT"; + + if (!(BUN_MARIADB_ForeignKeyRules as readonly string[]).includes(normalized)) { + throw new Error( + `Invalid foreign key rule \`${rule}\`. Expected one of: ${BUN_MARIADB_ForeignKeyRules.join(", ")}`, + ); + } + + return normalized as BUN_MARIADB_ForeignKeyRule; +} + +/** + * Resolve the ON DELETE / ON UPDATE actions for a field's foreign key. + * Precedence: `foreignKey.onDelete` > field `onDeleteLiteral` / `onDelete` + * (legacy) > `cascadeDelete` > `RESTRICT`. Same for update, except the + * field-level `onUpdate*` keys are column `ON UPDATE` clauses, not FK rules. + */ +export function resolveForeignKeyRules( + field: BUN_MARIADB_FieldSchemaType, + tableName?: string, +): { onDelete: BUN_MARIADB_ForeignKeyRule; onUpdate: BUN_MARIADB_ForeignKeyRule } { + const fk = field.foreignKey || {}; + + const onDelete = normalizeForeignKeyRule( + fk.onDelete || + field.onDeleteLiteral || + field.onDelete || + (fk.cascadeDelete ? "CASCADE" : "RESTRICT"), + ); + + const onUpdate = normalizeForeignKeyRule( + fk.onUpdate || (fk.cascadeUpdate ? "CASCADE" : "RESTRICT"), + ); + + if ( + (onDelete === "SET NULL" || onUpdate === "SET NULL") && + (field.notNullValue || field.primaryKey) + ) { + throw new Error( + `Foreign key on \`${tableName ? `${tableName}.` : ""}${field.fieldName}\` uses SET NULL but the column is NOT NULL`, + ); + } + + return { onDelete, onUpdate }; +} + export default function buildForeignKeyConstraint( field: BUN_MARIADB_FieldSchemaType, tableName: string, ): string { const fk = field.foreignKey!; const constraintName = resolveForeignKeyName(field, tableName); + const { onDelete, onUpdate } = resolveForeignKeyRules(field, tableName); let constraint = `CONSTRAINT ${MariaDBQuoteGen(constraintName)} FOREIGN KEY (${MariaDBQuoteGen(field.fieldName!)}) REFERENCES ${MariaDBQuoteGen(fk.destinationTableName!)}(${MariaDBQuoteGen(fk.destinationTableColumnName!)})`; - if (fk.cascadeDelete) { - constraint += " ON DELETE CASCADE"; + if (onDelete !== "RESTRICT") { + constraint += ` ON DELETE ${onDelete}`; } - if (fk.cascadeUpdate) { - constraint += " ON UPDATE CASCADE"; + if (onUpdate !== "RESTRICT") { + constraint += ` ON UPDATE ${onUpdate}`; } return constraint; diff --git a/src/lib/schema/create-table.ts b/src/lib/schema/create-table.ts index 6144c01..4bdd329 100644 --- a/src/lib/schema/create-table.ts +++ b/src/lib/schema/create-table.ts @@ -3,7 +3,6 @@ import type { BunMariaDBConfig, } from "../../types"; import buildColumnDefinition from "./build-column-definition"; -import buildForeignKeyConstraint from "./build-foreign-key-constraint"; import buildTableOptions from "./build-table-options"; import MariaDBQuoteGen from "./mariadb-quote-gen"; import runSchemaQuery from "./run-schema-query"; @@ -20,7 +19,6 @@ export default async function createTable({ } const columnDefinitions: string[] = []; - const foreignKeys: string[] = []; const primaryKeys: string[] = []; for (const field of table.fields || []) { @@ -29,12 +27,6 @@ export default async function createTable({ if (field.primaryKey && field.fieldName) { primaryKeys.push(field.fieldName); } - - if (field.foreignKey) { - foreignKeys.push( - buildForeignKeyConstraint(field, table.tableName), - ); - } } if (primaryKeys.length > 0) { @@ -62,7 +54,12 @@ export default async function createTable({ } } - const sql = `CREATE TABLE IF NOT EXISTS ${MariaDBQuoteGen(table.tableName)} (${[...columnDefinitions, ...foreignKeys].join(", ")})${buildTableOptions(table)}`; + /** + * Foreign keys are not declared inline. `handleDBSchemaTables` adds them + * with `ensureForeignKeys` once every table exists, so forward references, + * self references and FK cycles all work regardless of table order. + */ + const sql = `CREATE TABLE IF NOT EXISTS ${MariaDBQuoteGen(table.tableName)} (${columnDefinitions.join(", ")})${buildTableOptions(table)}`; await runSchemaQuery({ query: sql, config }); } diff --git a/src/lib/schema/handle-db-schema-table.ts b/src/lib/schema/handle-db-schema-table.ts index 88a42d8..88389f6 100644 --- a/src/lib/schema/handle-db-schema-table.ts +++ b/src/lib/schema/handle-db-schema-table.ts @@ -4,10 +4,7 @@ import MariaDBQuoteGen from "./mariadb-quote-gen"; import resolveTable from "./resolve-table"; import runSchemaQuery, { querySchemaRows } from "./run-schema-query"; import schemaCondition from "./schema-condition"; -import { - dropObsoleteForeignKeys, - ensureForeignKeys, -} from "./sync-foreign-keys"; +import { dropObsoleteForeignKeys } from "./sync-foreign-keys"; import syncIndexes from "./sync-indexes"; import syncPrimaryKey from "./sync-primary-key"; import syncTableOptions from "./sync-table-options"; @@ -85,11 +82,11 @@ export default async function handleDBSchemaTable({ // 2. Drop obsolete/changed FKs (unlocks index cleanup) // 3. Primary key // 4. Indexes / uniques (FK columns need supporting indexes) - // 5. Ensure foreign keys exist + // Foreign keys are ensured by `handleDBSchemaTables` after every table + // has been created/updated. await syncTableOptions({ table: resolvedTable, config }); await dropObsoleteForeignKeys({ table: resolvedTable, config }); await syncPrimaryKey({ table: resolvedTable, config }); await syncIndexes({ table: resolvedTable, config }); await syncUniqueConstraints({ table: resolvedTable, config }); - await ensureForeignKeys({ table: resolvedTable, config }); } diff --git a/src/lib/schema/handle-db-schema-tables.ts b/src/lib/schema/handle-db-schema-tables.ts index b809d62..7375514 100644 --- a/src/lib/schema/handle-db-schema-tables.ts +++ b/src/lib/schema/handle-db-schema-tables.ts @@ -7,6 +7,8 @@ import type { import handleDBSchemaTable from "./handle-db-schema-table"; import getExistingTablesFromTablesManagerTable from "./get-existing-tables-from-tables-manager-table"; import MariaDBQuoteGen from "./mariadb-quote-gen"; +import resolveTable from "./resolve-table"; +import { ensureForeignKeys } from "./sync-foreign-keys"; import runSchemaQuery, { querySchemaRows } from "./run-schema-query"; import schemaCondition from "./schema-condition"; import { removeDbManagerTable } from "./upsert-db-manager-table"; @@ -51,6 +53,19 @@ export default async function handleDBSchemaTables( }); } + /** + * Foreign keys last: every referenced table now exists, so forward + * references, self references and cycles between tables all resolve. + */ + for (const table of db_schema.tables) { + if (!table) continue; + + await ensureForeignKeys({ + table: resolveTable(table, db_schema), + config, + }); + } + /** * Drop tables tracked by the manager but no longer in the schema. * Skip drops when remaining schema tables still reference the table via FK diff --git a/src/lib/schema/order-db-schema.test.ts b/src/lib/schema/order-db-schema.test.ts new file mode 100644 index 0000000..d92ddd0 --- /dev/null +++ b/src/lib/schema/order-db-schema.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, test } from "bun:test"; +import type { BUN_MARIADB_TableSchemaType } from "../../types"; +import orderDBSchema from "./order-db-schema"; + +function table({ + name, + refs = [], +}: { + name: string; + refs?: string[]; +}): BUN_MARIADB_TableSchemaType { + return { + tableName: name, + fields: refs.map((ref) => ({ + fieldName: `${ref}_id`, + dataType: "BIGINT", + foreignKey: { + destinationTableName: ref, + destinationTableColumnName: "id", + }, + })), + }; +} + +async function order(tables: BUN_MARIADB_TableSchemaType[]) { + const res = await orderDBSchema({ db_schema: { tables } }); + return res.tables.map((t) => t.tableName); +} + +describe("orderDBSchema", () => { + test("places parents before children", async () => { + expect( + await order([ + table({ name: "comments", refs: ["posts", "users"] }), + table({ name: "posts", refs: ["users"] }), + table({ name: "users" }), + ]), + ).toEqual(["users", "posts", "comments"]); + }); + + test("keeps declaration order when there are no dependencies", async () => { + expect( + await order([table({ name: "b" }), table({ name: "a" })]), + ).toEqual(["b", "a"]); + }); + + test("handles self references without recursing forever", async () => { + expect( + await order([ + table({ name: "categories", refs: ["categories"] }), + table({ name: "comments", refs: ["comments", "users"] }), + table({ name: "users" }), + ]), + ).toEqual(["categories", "users", "comments"]); + }); + + test("handles cycles and emits each table once", async () => { + const res = await order([ + table({ name: "a", refs: ["b"] }), + table({ name: "b", refs: ["a"] }), + table({ name: "c", refs: ["a"] }), + ]); + expect(res).toHaveLength(3); + expect(new Set(res).size).toBe(3); + expect(res.indexOf("c")).toBeGreaterThan(res.indexOf("a")); + }); + + test("ignores references to tables outside the schema", async () => { + expect( + await order([table({ name: "posts", refs: ["external_users"] })]), + ).toEqual(["posts"]); + }); +}); diff --git a/src/lib/schema/order-db-schema.ts b/src/lib/schema/order-db-schema.ts index 86b0c10..a4c13b7 100644 --- a/src/lib/schema/order-db-schema.ts +++ b/src/lib/schema/order-db-schema.ts @@ -5,82 +5,63 @@ import type { CreateDBSchemaParams, } from "../../types"; +/** + * Topologically order tables so referenced (parent) tables come before the + * tables that reference them. Declaration order is kept wherever the FK + * graph allows it. + * + * - Self references (e.g. `comments.parent_comment_id`) are ignored. + * - References to tables outside the schema are ignored. + * - Cycles do not throw: a table already on the current path is skipped, + * so every table is still emitted exactly once. Foreign keys are created + * after all tables exist, so cycles are safe to sync. + */ export default async function orderDBSchema({ db_schema, }: CreateDBSchemaParams): Promise { - let new_db_schema = _.cloneDeep(db_schema); - + const new_db_schema = _.cloneDeep(db_schema); const tables = new_db_schema.tables; - let new_tables_set = new Set(); - let new_tables_start_set = new Set(); - let new_tables_end_set = new Set(); + const tables_by_name = new Map(); + for (const table of tables) { + if (table) tables_by_name.set(table.tableName, table); + } - function setParentTable(table: BUN_MARIADB_TableSchemaType) { - const fields = table.fields; - let does_table_have_foreign_keys = false; + const ordered: BUN_MARIADB_TableSchemaType[] = []; + const visited = new Set(); + const visiting = new Set(); - for (let f = 0; f < fields.length; f++) { - const field = fields[f]; - if (!field) continue; + function visit(table: BUN_MARIADB_TableSchemaType) { + if (visited.has(table.tableName) || visiting.has(table.tableName)) { + return; + } - const dst_table_name = field.foreignKey?.destinationTableName; + visiting.add(table.tableName); - if (dst_table_name) { - const fk_table = tables.find( - (tb) => tb.tableName == dst_table_name, - ); + for (const field of table.fields || []) { + const dst_table_name = field?.foreignKey?.destinationTableName; - if ( - fk_table && - !new_tables_start_set.has(fk_table) && - !new_tables_end_set.has(fk_table) - ) { - setParentTable(fk_table); - } + if (!dst_table_name || dst_table_name === table.tableName) { + continue; + } - new_tables_end_set.add(table); + const fk_table = tables_by_name.get(dst_table_name); - if (fk_table) { - new_tables_start_set.add(fk_table); - } - - does_table_have_foreign_keys = true; + if (fk_table) { + visit(fk_table); } } - return { does_table_have_foreign_keys }; + visiting.delete(table.tableName); + visited.add(table.tableName); + ordered.push(table); } - for (let i = 0; i < tables.length; i++) { - const table = tables[i]; - - if (!table) { - continue; - } - - let { does_table_have_foreign_keys } = setParentTable(table); - - if (does_table_have_foreign_keys) { - new_tables_end_set.add(table); - } else { - new_tables_start_set.add(table); - } + for (const table of tables) { + if (table) visit(table); } - const parsed_tables = [ - ...Array.from(new_tables_start_set), - ...Array.from(new_tables_end_set), - ]; - - for (let nt = 0; nt < parsed_tables.length; nt++) { - const new_table = parsed_tables[nt]; - if (new_table) { - new_tables_set.add(new_table); - } - } - - new_db_schema.tables = Array.from(new_tables_set); + new_db_schema.tables = ordered; return new_db_schema; } diff --git a/src/lib/schema/sync-foreign-keys.ts b/src/lib/schema/sync-foreign-keys.ts index d075e26..9c1675b 100644 --- a/src/lib/schema/sync-foreign-keys.ts +++ b/src/lib/schema/sync-foreign-keys.ts @@ -1,10 +1,13 @@ import type { BUN_MARIADB_FieldSchemaType, + BUN_MARIADB_ForeignKeyRule, BUN_MARIADB_TableSchemaType, BunMariaDBConfig, } from "../../types"; import buildForeignKeyConstraint, { + normalizeForeignKeyRule, resolveForeignKeyName, + resolveForeignKeyRules, } from "./build-foreign-key-constraint"; import MariaDBQuoteGen from "./mariadb-quote-gen"; import runSchemaQuery, { querySchemaRows } from "./run-schema-query"; @@ -14,8 +17,8 @@ export type DesiredForeignKey = { column: string; refTable: string; refColumn: string; - cascadeDelete: boolean; - cascadeUpdate: boolean; + onDelete: BUN_MARIADB_ForeignKeyRule; + onUpdate: BUN_MARIADB_ForeignKeyRule; field: BUN_MARIADB_FieldSchemaType; }; @@ -28,14 +31,6 @@ export type LiveForeignKey = { updateRule: string; }; -function normalizeRule(rule: string | undefined | null): string { - return String(rule || "RESTRICT").toUpperCase().replace(/_/g, " "); -} - -function ruleIsCascade(rule: string): boolean { - return normalizeRule(rule) === "CASCADE"; -} - function fkIdentityKey(fk: { column: string; refTable: string; @@ -49,8 +44,8 @@ function rulesMatch( desired: DesiredForeignKey, ): boolean { return ( - ruleIsCascade(live.deleteRule) === desired.cascadeDelete && - ruleIsCascade(live.updateRule) === desired.cascadeUpdate + normalizeForeignKeyRule(live.deleteRule) === desired.onDelete && + normalizeForeignKeyRule(live.updateRule) === desired.onUpdate ); } @@ -69,13 +64,18 @@ export function grabDesiredForeignKeys( continue; } + const { onDelete, onUpdate } = resolveForeignKeyRules( + field, + table.tableName, + ); + desired.push({ name: resolveForeignKeyName(field, table.tableName), column: field.fieldName, refTable: fk.destinationTableName, refColumn: fk.destinationTableColumnName, - cascadeDelete: Boolean(fk.cascadeDelete), - cascadeUpdate: Boolean(fk.cascadeUpdate), + onDelete, + onUpdate, field, }); } diff --git a/src/lib/schema/sync-indexes.ts b/src/lib/schema/sync-indexes.ts index c95b5e5..41d848b 100644 --- a/src/lib/schema/sync-indexes.ts +++ b/src/lib/schema/sync-indexes.ts @@ -91,12 +91,14 @@ async function createIndex({ return; } - console.log(`Creating standard index: ${index.indexName}`); const fields = index.indexTableFields .map((field) => MariaDBQuoteGen(field)) .join(", "); const typeUpper = index.indexType?.toUpperCase(); const isSpecialType = typeUpper === "FULLTEXT" || typeUpper === "SPATIAL"; + console.log( + `Creating ${isSpecialType ? typeUpper : "standard"} index: ${index.indexName}`, + ); const indexPrefix = isSpecialType ? `${typeUpper} ` : ""; const indexSuffix = !isSpecialType && (typeUpper === "BTREE" || typeUpper === "HASH") diff --git a/src/types/index.ts b/src/types/index.ts index fff2ca8..383df69 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -195,7 +195,15 @@ export type BUN_MARIADB_FieldSchemaType = { patternFlags?: string; onUpdate?: string; onUpdateLiteral?: string; + /** + * @deprecated Use `foreignKey.onDelete`. Still honored as the foreign + * key's ON DELETE action when `foreignKey.onDelete` is not set. + */ onDelete?: string; + /** + * @deprecated Use `foreignKey.onDelete`. Still honored as the foreign + * key's ON DELETE action when `foreignKey.onDelete` is not set. + */ onDeleteLiteral?: string; cssFiles?: string[]; /** @@ -214,13 +222,40 @@ export type BUN_MARIADB_FieldSchemaType = { /** * Defines a foreign key relationship from one field to another table column. */ +export const BUN_MARIADB_ForeignKeyRules = [ + "CASCADE", + "SET NULL", + "RESTRICT", + "NO ACTION", +] as const; + +export type BUN_MARIADB_ForeignKeyRule = + (typeof BUN_MARIADB_ForeignKeyRules)[number]; + export interface BUN_MARIADB_ForeignKeyType { foreignKeyName?: string; destinationTableName?: string; destinationTableColumnName?: string; destinationTableColumnType?: string; + /** + * Shorthand for `onDelete: "CASCADE"`. Ignored when `onDelete` is set. + */ cascadeDelete?: boolean; + /** + * Shorthand for `onUpdate: "CASCADE"`. Ignored when `onUpdate` is set. + */ cascadeUpdate?: boolean; + /** + * Referential action when the referenced row is deleted. + * Defaults to `CASCADE` when `cascadeDelete` is true, else `RESTRICT`. + * `SET NULL` requires a nullable column. + */ + onDelete?: BUN_MARIADB_ForeignKeyRule; + /** + * Referential action when the referenced key is updated. + * Defaults to `CASCADE` when `cascadeUpdate` is true, else `RESTRICT`. + */ + onUpdate?: BUN_MARIADB_ForeignKeyRule; } export const MariaDBIndexTypes = [ diff --git a/tsconfig.json b/tsconfig.json index 34ebb79..2c745f4 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -25,5 +25,5 @@ "outDir": "dist" }, "include": ["src"], - "exclude": ["node_modules", "dist"] + "exclude": ["node_modules", "dist", "src/**/*.test.ts"] }