From 59d63a8d1bbd9fef22e594dc84f121eaaf25a4c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 12:59:33 +0000 Subject: [PATCH] refactor(driver-sql): move the transaction doors out of sql-driver.ts into sql-driver-transactions.ts (#25521 cut 3) Move-only: the bodies of beginTransaction, assertBareKnexSafe, commit, rollback, commitTransaction and rollbackTransaction move unedited into sql-driver-transactions.ts and reach the class through one named host, TransactionHost. The class keeps every method with its signature and docblock; releaseTransaction stays in the class whole. Claude-Session: https://claude.ai/code/session_01JVTd3PprStGa26aez5omLB Co-authored-by: Claude --- .../driver-sql/src/sql-driver-transactions.ts | 119 ++++++++++++++++++ packages/drivers/driver-sql/src/sql-driver.ts | 89 ++++++++----- 2 files changed, 177 insertions(+), 31 deletions(-) create mode 100644 packages/drivers/driver-sql/src/sql-driver-transactions.ts diff --git a/packages/drivers/driver-sql/src/sql-driver-transactions.ts b/packages/drivers/driver-sql/src/sql-driver-transactions.ts new file mode 100644 index 00000000000..2e099665e50 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-transactions.ts @@ -0,0 +1,119 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25521 cut 3] The transaction doors of `SqlDriver`: `beginTransaction`, + * `commit`, `rollback`, their deprecated aliases `commitTransaction` and + * `rollbackTransaction`, and the SQLite single-connection guard + * `assertBareKnexSafe`. They moved out of `sql-driver.ts` whole. + * + * A MOVE, not a redesign. Each function below is the body of the `SqlDriver` + * method of the same name as it stood, re-indented one level and otherwise + * unedited: the same open-transaction count, the same release on every close, + * the same guard. + * + * `releaseTransaction` did NOT move. Its body deletes from the + * `openTransactions` set (`this.openTransactions.delete(`), and the series + * keeps a body with such a site in the class. `commit` and `rollback` reach it + * through the host, like any other member. + * + * ## How the moved bodies reach the class + * + * The shape `sql-driver-introspection.ts` (cut 1) and + * `sql-driver-raw-execution.ts` (cut 2) apply, after #23050. The class keeps + * every method, because the published `dist/*.d.ts` lists each one's name, + * docblock and position. `TursoDriver` overrides `beginTransaction`, `commit` + * and `rollback`, and each override calls `super`. Each body is now one + * literal of exactly the {@link TransactionHost} members that method reads, + * plus a `.call`. The five `async` methods are no longer `async`: each returns + * the moved function's own promise, so a caller awaits the same promise the + * body settles, with no extra wrapper. + * + * `commit`, `rollback`, `releaseTransaction` and `isProductionEnv` go as + * closures that call them on the driver at call time. So a subclass override + * is still the one a body reaches: the deprecated `commitTransaction` reaches + * `TursoDriver`'s refusing `commit`, as it did when it called `this.commit`. + * A test spy on the instance is reached the same way. + * + * The fields `knex`, `openTransactions` and `activeTransactions`, and the + * getter `isSqlite`, go as GETTERS, so each read happens when the body makes + * it, as it did on the class (`SqliteWasmDriver` overrides `isSqlite`). + * `beginTransaction` also WRITES `activeTransactions` (`++`), so its literal + * carries a setter beside the getter, which writes the field on the driver. + * Both fields stay on the class: `releaseTransaction` and the tests read them. + * + * The bodies read no module-level declaration of `sql-driver.ts`, so nothing + * gained an `export` there. `index.ts` re-exports nothing from this module, so + * the published surface does not move. + */ + +import type { Knex } from 'knex'; +import type { SqlDriver } from './sql-driver.js'; + +/** + * The `SqlDriver` members the functions below read — every one of them, and + * no other; each function's `this` is the `Pick` of the members IT reads. Each + * is typed AS the class member (an indexed access on the class), so a change + * to a member's signature on the class is a change here too, and naming a + * member the class does not have is a compile error. See the module header + * for which members are getters, and for the one that is also written. + */ +export interface TransactionHost { + activeTransactions: SqlDriver['activeTransactions']; + commit: SqlDriver['commit']; + isProductionEnv: SqlDriver['isProductionEnv']; + readonly isSqlite: SqlDriver['isSqlite']; + readonly knex: SqlDriver['knex']; + readonly openTransactions: SqlDriver['openTransactions']; + releaseTransaction: SqlDriver['releaseTransaction']; + rollback: SqlDriver['rollback']; +} + +/** The body of `SqlDriver#beginTransaction`; the method's docblock there is its contract. */ +export async function beginTransaction(this: Pick): Promise { + const trx = await this.knex.transaction(); + this.openTransactions.add(trx as unknown as object); + this.activeTransactions++; + return trx; +} + +/** The body of `SqlDriver#assertBareKnexSafe`; the method's docblock there is its contract. */ +export function assertBareKnexSafe(this: Pick, op: string): void { + if (this.isProductionEnv()) return; + if (!this.isSqlite) return; + if (this.activeTransactions === 0) return; + throw new Error( + `[driver-sql] refusing to run '${op}' on a fresh pooled connection while a ` + + `transaction is open: SQLite's pool has a single connection, so acquiring a ` + + `second one would dead-lock (surfacing later as "Knex: Timeout acquiring a ` + + `connection"). Bind this operation to the active transaction instead of using ` + + `this.knex.`, + ); +} + +/** The body of `SqlDriver#commit`; the method's docblock there is its contract. */ +export async function commit(this: Pick, transaction: unknown): Promise { + try { + await (transaction as Knex.Transaction).commit(); + } finally { + this.releaseTransaction(transaction); + } +} + +/** The body of `SqlDriver#rollback`; the method's docblock there is its contract. */ +export async function rollback(this: Pick, transaction: unknown): Promise { + try { + await (transaction as Knex.Transaction).rollback(); + } finally { + this.releaseTransaction(transaction); + } +} + +/** The body of `SqlDriver#commitTransaction`; the method's docblock there is its contract. */ +export async function commitTransaction(this: Pick, trx: Knex.Transaction): Promise { + await this.commit(trx); +} + +/** The body of `SqlDriver#rollbackTransaction`; the method's docblock there is its contract. */ +export async function rollbackTransaction(this: Pick, trx: Knex.Transaction): Promise { + await this.rollback(trx); +} diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index c1187581011..80341a2b4a7 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -175,6 +175,18 @@ import { // index DDL `execute` ran); the methods of the same names below hand each body // the members it reads (`RawExecutionHost`). import { execute, noteRuntimeIndexDdl, rawStatementFault } from './sql-driver-raw-execution.js'; +// [#25521 cut 3] The transaction doors (`beginTransaction`, `commit`, `rollback`, +// their deprecated aliases, and the SQLite single-connection guard +// `assertBareKnexSafe`); the methods of the same names below hand each body the +// members it reads (`TransactionHost`). +import { + assertBareKnexSafe, + beginTransaction, + commit, + commitTransaction, + rollback, + rollbackTransaction, +} from './sql-driver-transactions.js'; import { makeDatabaseDirectory, probeAbsentSqliteDatabaseFile, SqliteDatabaseFileError } from './sqlite-database-file.js'; import { jsonMembershipPredicate } from '@objectstack/core'; import knex, { Knex } from 'knex'; @@ -10900,11 +10912,19 @@ export class SqlDriver implements IDataDriver { // Transactions // =================================== - async beginTransaction(): Promise { - const trx = await this.knex.transaction(); - this.openTransactions.add(trx as unknown as object); - this.activeTransactions++; - return trx; + beginTransaction(): Promise { + // [#25521 cut 3] The body moved to `sql-driver-transactions.ts` (see `TransactionHost`). + const readActiveTransactions = () => this.activeTransactions; + const writeActiveTransactions = (v: SqlDriver['activeTransactions']) => { this.activeTransactions = v; }; + const readKnex = () => this.knex; + const readOpenTransactions = () => this.openTransactions; + const host: ThisParameterType = { + get activeTransactions() { return readActiveTransactions(); }, + set activeTransactions(v) { writeActiveTransactions(v); }, + get knex() { return readKnex(); }, + get openTransactions() { return readOpenTransactions(); }, + }; + return beginTransaction.call(host); } /** Idempotently drop a transaction from the open-count (safe on double close). */ @@ -10934,44 +10954,51 @@ export class SqlDriver implements IDataDriver { * `this.knex`. */ protected assertBareKnexSafe(op: string): void { - if (this.isProductionEnv()) return; - if (!this.isSqlite) return; - if (this.activeTransactions === 0) return; - throw new Error( - `[driver-sql] refusing to run '${op}' on a fresh pooled connection while a ` + - `transaction is open: SQLite's pool has a single connection, so acquiring a ` + - `second one would dead-lock (surfacing later as "Knex: Timeout acquiring a ` + - `connection"). Bind this operation to the active transaction instead of using ` + - `this.knex.`, - ); + // [#25521 cut 3] The body moved to `sql-driver-transactions.ts` (see `TransactionHost`). + const readActiveTransactions = () => this.activeTransactions; + const readIsSqlite = () => this.isSqlite; + const host: ThisParameterType = { + get activeTransactions() { return readActiveTransactions(); }, + isProductionEnv: (...a) => this.isProductionEnv(...a), + get isSqlite() { return readIsSqlite(); }, + }; + return assertBareKnexSafe.call(host, op); } /** IDataDriver standard */ - async commit(transaction: unknown): Promise { - try { - await (transaction as Knex.Transaction).commit(); - } finally { - this.releaseTransaction(transaction); - } + commit(transaction: unknown): Promise { + // [#25521 cut 3] The body moved to `sql-driver-transactions.ts` (see `TransactionHost`). + const host: ThisParameterType = { + releaseTransaction: (...a) => this.releaseTransaction(...a), + }; + return commit.call(host, transaction); } /** IDataDriver standard */ - async rollback(transaction: unknown): Promise { - try { - await (transaction as Knex.Transaction).rollback(); - } finally { - this.releaseTransaction(transaction); - } + rollback(transaction: unknown): Promise { + // [#25521 cut 3] The body moved to `sql-driver-transactions.ts` (see `TransactionHost`). + const host: ThisParameterType = { + releaseTransaction: (...a) => this.releaseTransaction(...a), + }; + return rollback.call(host, transaction); } /** @deprecated Use commit() instead */ - async commitTransaction(trx: Knex.Transaction): Promise { - await this.commit(trx); + commitTransaction(trx: Knex.Transaction): Promise { + // [#25521 cut 3] The body moved to `sql-driver-transactions.ts` (see `TransactionHost`). + const host: ThisParameterType = { + commit: (...a) => this.commit(...a), + }; + return commitTransaction.call(host, trx); } /** @deprecated Use rollback() instead */ - async rollbackTransaction(trx: Knex.Transaction): Promise { - await this.rollback(trx); + rollbackTransaction(trx: Knex.Transaction): Promise { + // [#25521 cut 3] The body moved to `sql-driver-transactions.ts` (see `TransactionHost`). + const host: ThisParameterType = { + rollback: (...a) => this.rollback(...a), + }; + return rollbackTransaction.call(host, trx); } // ===================================