Closed aleffert closed 1 year ago
I guess it would be more in line with the WYSIWYG philosophy of Kysely to have this kind of transaction API:
try {
const trx = await db.begin().isolationLevel('serializable').execute()
await trx.insertInto(...).execute()
await trx.savepoint('foobar')
try {
await trx.updateTable(...).execute()
await trx.updateTable(...).execute()
} catch (err) {
await trx.rollbackToSavepoint('foobar')
}
await trx.commit()
} catch (err) {
await trx.rollback()
}
But that wouldn't solve your case. You want to have a begin()
method that implicitly creates a savepoint if a transaction doesn't exist. That seems like bad and surprising API design.
I guess we could add a beginOrCreateSavepoint('foobar')
method and rollbackToToSavepointOrCompletely()
or something like that.
Or maybe beginNested('savepoint_name')
and rollbackNested('savepoint_name')
? Kysely
instance would create new transaction and ignore the savepoint name. Transaction
instance would create a savepoint.
You wouldn't be able to set the isolation level etc. since those are not available for savepoints.
If I'm following, I'm not sure that would work, since the issue is, when my code under test is executing, it just wants to create a "nesting", but the object it has at hand is always a Kysely object. To get more concrete, I ended up writing the following helpers:
let contained = false;
export async function beginTransaction<DB>(db: Kysely<DB>): Promise<void> {
await sql`begin transaction`.execute(db);
contained = true;
}
export async function rollbackTransaction<DB>(db: Kysely<DB>): Promise<void> {
await sql`rollback`.execute(db);
contained = false;
}
export async function nested<DB, T>(db: Kysely<DB>, f: (db: Kysely<DB> | Transaction<DB>) => Promise<T>): Promise<T> {
if(contained) {
await sql`savepoint nesting`.execute(db);
}
try {
if(contained) {
const result = await f(db);
await sql`release savepoint nesting`.execute(db);
return result;
}
else {
const result = await db.transaction().execute(f);
return result;
}
}
catch(e) {
if(contained) {
await sql`rollback to nesting;`.execute(db);
}
throw e;
}
};
And then in my jest setup:
global.beforeEach(async () => {
await beginTransaction(db);
});
global.afterEach(async () => {
await rollbackTransaction(db);
});
And in my code:
nested(db, async trx => {
// do some stuff with trx
});
Where db
is my Kysely object.
That kind of API can't be added. If transactions worked like that, all requests in a server would suddenly share a transaction.
I mean, I'm only using it for integration tests, so totally understand if you feel like it's awkward as part of a more general API, but I do feel like that's a pretty common use case? For example, the django test runner does the same sort of drop transactions on the floor at the end setup and here's someone providing a way to do it with typeorm https://www.npmjs.com/package/typeorm-test-transactions
Very open to alternate ways to solve this use case. How would you do it? Explicitly truncate all the tables at the start of a test?
And obviously, thanks for the library! I'm here asking because I like it and want to use it.
What we do is that we create a whole new database from a template at the beginning of each test suite. It's surprisingly fast, definitely fast enough that it's not a bottleneck of our tests, and pretty easy to do with Jest at least, since it runs each spec file in isolation, you can just pass the name of the newly created database via process.env.DATABASE
or whatever.
I simply truncate all tables + insert seed data before each test. My current project has a pretty complex schema with ~30 tables and it takes milliseconds to do that.
Hello friens, I ran into this issue recently, so I implemented a thing that seems to work for this case. Here is the testing hook impl:
import { sql } from 'kysely';
import { beforeEach, afterEach } from 'vitest'; // omit for Jest/Mocha anything using global API
import { MyDbTypeFromCodegen, db } from './db.js';
export const useTestingKysely = () => {
let kyselyTransaction: MyDbTypeFromCodegen;
let onTransactionFinished: () => void;
beforeEach(async () => {
await new Promise<void>(resolve => {
db.transaction()
.execute(kyselyTx => {
kyselyTransaction = kyselyTx;
resolve(); // beforeEach can finish, we've opened a transaction now
// This is a promise marking the transaction scope -- the whole test; that's why we save the reject (to explicitly rollback by calling it) to call it later in the afterEach
return new Promise((_, reject) => {
// save the reject to local variable to call in afterEach
onTransactionFinished = reject;
});
})
.catch(() => {
// always reject (explicitly) to rollback, no handling
});
});
});
afterEach(() => {
// Release the transaction scope
onTransactionFinished();
});
// To avoid working with extra getter, Proxy does lazy delegation of Kysely calls
return new Proxy(
{},
{
get: (_target, prop) => {
if (prop == 'transaction') {
return () => ({
execute: async (
cb: (kyselyTx: MyDbTypeFromCodegen) => Promise<MyDbTypeFromCodegen>
) => {
// multiple savepoints don't need to be implemented (since Kysely forbids nested transactions), but it's possible
// create savepoint
await sql`SAVEPOINT test;`.execute(kyselyTransaction);
// run transaction cb
return cb(kyselyTransaction).catch(async () => {
// on explicit reject, rollback to savepoint
sql`ROLLBACK TO SAVEPOINT test;`.execute(kyselyTransaction);
});
},
});
}
// @ts-expect-error Property access, keys not inferred
const attr = kyselyTransaction[prop];
if (typeof attr === 'object') {
return attr;
}
return Object.assign(
typeof attr === 'function' ? attr.bind(kyselyTransaction) : {},
attr
);
},
}
) as MyDbTypeFromCodegen;
};
describe('Test suite', () => {
const db = useTestingKysely();
// use in before*/after* hooks, and in tests, not available to call in describe
test('That runs in seperate transaction', async () => {
//
})
test('That runs in another seperate transaction', async () => {
//
})
});
The proxy might cause some issues when using the value directly instead of accessing methods, but should be fairly safe for 99% cases. Mind that it can always be dropped out adding an extra getter: const getDb = useTestingKyselyGetter();
edit: updated some edge-cases with accessing non-function attributes
I like to run my integration tests in a transaction so that I can roll them back at the end of each test for easy clean up. This would work fine except I also use transactions in my code, and, on postgres at least, transactions don't nest directly. Instead you switch to savepoints inside a top level transaction. It would be great to support this use case.
One API design thing that's tricky here is that, at least in common test runners like jest, there is not a single top level function you could wrap a transaction around, so I think the API would probably need to be more imperative/stateful with an explicit begin/end