From a28fd34a32a1e1b4f99128937914b2dfcce61a17 Mon Sep 17 00:00:00 2001 From: Peter Skoglund Date: Mon, 28 Sep 2026 09:56:39 +0200 Subject: [PATCH] [mssql] Report transactional SQL to onSqlCommand The Persister gives the driver-level Transaction an execute command of its own, which never went through getWrappedCommand. Since the JSON persister does its work inside that transaction, the callback stayed silent for both load() and save(), where the other database Persisters report every statement. Co-Authored-By: Claude Opus 5 --- src/persisters/persister-mssql/index.ts | 8 +++- test/unit/persisters/database/mssql.test.ts | 53 +++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/persisters/persister-mssql/index.ts b/src/persisters/persister-mssql/index.ts index 039b1215e5e..dc6ef008d79 100644 --- a/src/persisters/persister-mssql/index.ts +++ b/src/persisters/persister-mssql/index.ts @@ -12,6 +12,7 @@ import type {Store} from '../../@types/store/index.d.ts'; import {arrayForEach} from '../../common/array.ts'; import {IdObj} from '../../common/obj.ts'; import {noop} from '../../common/other.ts'; +import {getWrappedCommand} from '../common/database/common.ts'; import {createCustomMsSqlPersister} from '../common/database/mssql.ts'; // The shared code emits positional parameters as @p1, @p2, and so on, which @@ -49,8 +50,13 @@ export const createMsSqlPersister = (async ( const transaction = mssql.transaction(); await transaction.begin(); try { + // The command is wrapped here too, since the statements inside the + // transaction would otherwise never reach onSqlCommand. const result = await actions( - getExecuteCommand(() => transaction.request()), + getWrappedCommand( + getExecuteCommand(() => transaction.request()), + onSqlCommand, + ), ); await transaction.commit(); return result; diff --git a/test/unit/persisters/database/mssql.test.ts b/test/unit/persisters/database/mssql.test.ts index 763b803b52b..187aa45d9e3 100644 --- a/test/unit/persisters/database/mssql.test.ts +++ b/test/unit/persisters/database/mssql.test.ts @@ -1,5 +1,6 @@ import {createMergeableStore, createStore} from 'tinybase'; import {createCustomMsSqlPersister, Persists} from 'tinybase/persisters'; +import {createMsSqlPersister} from 'tinybase/persisters/persister-mssql'; import {expect, test, vi} from 'vitest'; import {pause} from '../../common/other.ts'; @@ -323,3 +324,55 @@ test('returns the thing it was given', async () => { expect((persister as any).getMsSql()).toBe(fake); await persister.destroy(); }); + +// A stand-in for the driver's ConnectionPool, built on the fake above, so that +// the real entry point rather than the shared code is under test. +const getFakePool = (fake: FakeMsSql) => { + const getRequest = () => { + const params: any[] = []; + const request: any = { + input: (name: string, param: any) => { + params[Number(name.slice(1)) - 1] = param; + return request; + }, + query: async (sql: string) => ({ + recordset: await fake.executeCommand(sql, params), + }), + }; + return request; + }; + return { + request: getRequest, + transaction: () => ({ + begin: async () => {}, + commit: async () => {}, + rollback: async () => {}, + request: getRequest, + }), + }; +}; + +test('reports the SQL it runs in a transaction to onSqlCommand', async () => { + const fake = getFakeMsSql(); + const store = createStore().setTables({pets: {fido: {species: 'dog'}}}); + const sql: string[] = []; + const persister = await createMsSqlPersister( + store, + getFakePool(fake) as any, + {mode: 'json', storeTableName: STORE_TABLE_NAME}, + (oneSql) => sql.push(oneSql), + ); + + await persister.save(); + + // Saving runs in a Transaction, which takes its own connection, so its + // statements would otherwise never reach the callback. + expect( + sql.filter((oneSql) => oneSql.startsWith('CREATE TABLE')), + ).toHaveLength(1); + expect(sql.filter((oneSql) => oneSql.startsWith('MERGE INTO'))).toHaveLength( + 1, + ); + + await persister.destroy(); +});