diff --git a/src/lib/db/transaction.test.ts b/src/lib/db/transaction.test.ts new file mode 100644 index 0000000000..37891d1b55 --- /dev/null +++ b/src/lib/db/transaction.test.ts @@ -0,0 +1,268 @@ +import { + withTransactional, + withRollbackTransaction, + withFakeTransactional, + inTransaction, + type TransactionUserParams, +} from './transaction.js'; +import { type Mock, vi } from 'vitest'; + +interface MockService { + getData: () => string; + saveData: (data: string) => Promise; +} + +describe('transaction utilities', () => { + let mockDb: any; + let mockTransaction: any; + let mockServiceFactory: Mock; + let mockService: MockService; + + beforeEach(() => { + mockTransaction = { + commit: vi.fn(), + rollback: vi.fn(), + isTransaction: true, + select: vi.fn(), + insert: vi.fn(), + update: vi.fn(), + userParams: undefined, + }; + + mockDb = { + transaction: vi + .fn() + .mockImplementation((callback) => callback(mockTransaction)), + isTransaction: false, + }; + + mockService = { + getData: vi.fn().mockReturnValue('test-data'), + saveData: vi.fn().mockResolvedValue(undefined), + }; + + mockServiceFactory = vi.fn().mockReturnValue(mockService); + }); + + describe('withTransactional', () => { + it('should add transactional method to service', () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + expect(typeof serviceWithTransactional.transactional).toBe( + 'function', + ); + expect(serviceWithTransactional.getData).toBe(mockService.getData); + expect(serviceWithTransactional.saveData).toBe( + mockService.saveData, + ); + }); + + it('should execute callback within database transaction', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + const result = await serviceWithTransactional.transactional( + (service) => { + return service.getData(); + }, + ); + + expect(mockDb.transaction).toHaveBeenCalledTimes(1); + expect(result).toBe('test-data'); + }); + + it('should set default userParams when no transactionContext provided', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + await serviceWithTransactional.transactional((service) => { + return service.getData(); + }); + + expect(mockTransaction.userParams).toBeDefined(); + expect(mockTransaction.userParams.type).toBe('transaction'); + expect(mockTransaction.userParams.value).toBeDefined(); + expect(typeof mockTransaction.userParams.value).toBe('number'); + }); + + it('should use provided transactionContext when given', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + const customContext: TransactionUserParams = { + type: 'change-request', + value: 42, + }; + + await serviceWithTransactional.transactional((service) => { + return service.getData(); + }, customContext); + + expect(mockTransaction.userParams).toEqual(customContext); + expect(mockTransaction.userParams.type).toBe('change-request'); + expect(mockTransaction.userParams.value).toBe(42); + }); + + it('should generate unique numeric IDs for default context', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + const userParamsValues: number[] = []; + + for (let i = 0; i < 3; i++) { + await serviceWithTransactional.transactional((service) => { + userParamsValues.push(mockTransaction.userParams.value); + return service.getData(); + }); + } + + expect(userParamsValues).toHaveLength(3); + expect(userParamsValues.every((id) => typeof id === 'number')).toBe( + true, + ); + expect(new Set(userParamsValues).size).toBe(3); + }); + + it('should create transactional service with transaction instance', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + await serviceWithTransactional.transactional((service) => { + return service.getData(); + }); + + expect(mockServiceFactory).toHaveBeenCalledWith(mockTransaction); + }); + + it('should handle promise-based callbacks', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + + const result = await serviceWithTransactional.transactional( + async (service) => { + await service.saveData('new-data'); + return 'success'; + }, + ); + + expect(result).toBe('success'); + expect(mockService.saveData).toHaveBeenCalledWith('new-data'); + }); + + it('should propagate errors from callback', async () => { + const serviceWithTransactional = withTransactional( + mockServiceFactory, + mockDb, + ); + const error = new Error('Test error'); + + await expect( + serviceWithTransactional.transactional(() => { + throw error; + }), + ).rejects.toThrow('Test error'); + }); + }); + + describe('withRollbackTransaction', () => { + beforeEach(() => { + mockDb.transaction = vi.fn().mockResolvedValue(mockTransaction); + }); + + it('should add rollbackTransaction method to service', () => { + const serviceWithRollback = withRollbackTransaction( + mockServiceFactory, + mockDb, + ); + + expect(typeof serviceWithRollback.rollbackTransaction).toBe( + 'function', + ); + expect(serviceWithRollback.getData).toBe(mockService.getData); + expect(serviceWithRollback.saveData).toBe(mockService.saveData); + }); + + it('should execute callback and rollback transaction', async () => { + const serviceWithRollback = withRollbackTransaction( + mockServiceFactory, + mockDb, + ); + + const result = await serviceWithRollback.rollbackTransaction( + (service) => { + return service.getData(); + }, + ); + + expect(mockDb.transaction).toHaveBeenCalledTimes(1); + expect(mockTransaction.rollback).toHaveBeenCalledTimes(1); + expect(result).toBe('test-data'); + }); + }); + + describe('withFakeTransactional', () => { + it('should add transactional method to service', () => { + const serviceWithFakeTransactional = + withFakeTransactional(mockService); + + expect(typeof serviceWithFakeTransactional.transactional).toBe( + 'function', + ); + expect(serviceWithFakeTransactional.getData).toBe( + mockService.getData, + ); + expect(serviceWithFakeTransactional.saveData).toBe( + mockService.saveData, + ); + }); + + it('should execute callback directly without transaction', async () => { + const serviceWithFakeTransactional = + withFakeTransactional(mockService); + + const result = await serviceWithFakeTransactional.transactional( + (service) => { + return service.getData(); + }, + ); + + expect(result).toBe('test-data'); + }); + }); + + describe('inTransaction', () => { + it('should execute callback directly when db is already a transaction', async () => { + const transactionDb = { ...mockDb, isTransaction: true }; + const callback = vi.fn().mockReturnValue('result'); + + const result = await inTransaction(transactionDb, callback); + + expect(result).toBe('result'); + expect(callback).toHaveBeenCalledWith(transactionDb); + expect(transactionDb.transaction).not.toHaveBeenCalled(); + }); + + it('should create new transaction when db is not a transaction', async () => { + const callback = vi.fn().mockReturnValue('result'); + + const result = await inTransaction(mockDb, callback); + + expect(result).toBe('result'); + expect(mockDb.transaction).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith(mockTransaction); + }); + }); +}); diff --git a/src/lib/db/transaction.ts b/src/lib/db/transaction.ts index 82fb98c7cd..f3dff5deb6 100644 --- a/src/lib/db/transaction.ts +++ b/src/lib/db/transaction.ts @@ -1,6 +1,17 @@ import type { Knex } from 'knex'; import type { IUnleashConfig } from '../types/index.ts'; -import { TransactionContext } from '../util/transactionContext.js'; + +export interface TransactionUserParams { + type: 'change-request' | 'transaction'; + value: number; +} + +// Generate a numeric transaction ID based on timestamp + random component +function generateNumericTransactionId(): number { + const timestamp = Date.now(); + const random = Math.floor(Math.random() * 1000); + return timestamp * 1000 + random; +} export type KnexTransaction = Knex.Transaction; @@ -39,12 +50,9 @@ export type ServiceFactory = ( ) => DeferredServiceFactory; export type WithTransactional = S & { - transactional: (fn: (service: S) => R) => Promise; -}; - -export type withContextualTransactional = S & { - trackedTransactional: ( - fn: (transactionContext: TransactionContext) => R, + transactional: ( + fn: (service: S) => R, + transactionContext?: TransactionUserParams, ) => Promise; }; @@ -82,10 +90,17 @@ export function withTransactional( ): WithTransactional { const service = serviceFactory(db) as WithTransactional; - service.transactional = async (fn: (service: S) => R) => - // Maybe: inTransaction(db, async (trx: Knex.Transaction) => fn(serviceFactory(trx))); - // this assumes that the caller didn't start a transaction already and opens a new one. + service.transactional = async ( + fn: (service: S) => R, + transactionContext?: TransactionUserParams, + ) => db.transaction(async (trx: Knex.Transaction) => { + const defaultContext: TransactionUserParams = { + type: 'transaction', + value: generateNumericTransactionId(), + }; + + trx.userParams = transactionContext || defaultContext; const transactionalService = serviceFactory(trx); return fn(transactionalService); }); @@ -93,24 +108,6 @@ export function withTransactional( return service; } -export function withContextualTransactional( - serviceFactory: (db: Knex) => S, - db: Knex, -): withContextualTransactional { - const service = serviceFactory(db) as withContextualTransactional; - - service.trackedTransactional = async ( - fn: (transactionContext: TransactionContext) => R, - ): Promise => { - return db.transaction(async (trx) => { - const transactionContext = new TransactionContext(trx); - return fn(transactionContext); - }); - }; - - return service; -} - export function withRollbackTransaction( serviceFactory: (db: Knex) => S, db: Knex, diff --git a/src/lib/util/transactionContext.test.ts b/src/lib/util/transactionContext.test.ts deleted file mode 100644 index f2fb78528e..0000000000 --- a/src/lib/util/transactionContext.test.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { TransactionContext } from './transactionContext.js'; -import { vi } from 'vitest'; - -describe('TransactionContext', () => { - let mockTransaction: any; - - beforeEach(() => { - mockTransaction = { - select: vi.fn(), - insert: vi.fn(), - update: vi.fn(), - delete: vi.fn(), - where: vi.fn(), - commit: vi.fn(), - rollback: vi.fn(), - isTransaction: true, - }; - }); - - describe('constructor', () => { - it('should create transaction context with default operation', () => { - const txContext = new TransactionContext(mockTransaction); - - expect(txContext.operationContext.type).toBe('transaction'); - expect(txContext.operationContext.id).toBeDefined(); - expect(typeof txContext.operationContext.id).toBe('number'); - expect(txContext.transaction).toBe(mockTransaction); - }); - - it('should generate unique IDs for different contexts', () => { - const txContext1 = new TransactionContext(mockTransaction); - const txContext2 = new TransactionContext(mockTransaction); - - expect(txContext1.operationContext.id).not.toBe( - txContext2.operationContext.id, - ); - }); - }); - - describe('operationContext', () => { - it('should allow setting custom operation context', () => { - const txContext = new TransactionContext(mockTransaction); - - expect(txContext.operationContext.type).toBe('transaction'); - - txContext.operationContext = { - type: 'change-request', - id: 42, - }; - - expect(txContext.operationContext.type).toBe('change-request'); - expect(txContext.operationContext.id).toBe(42); - }); - - it('should allow partial updates to operation context', () => { - const txContext = new TransactionContext(mockTransaction); - const originalId = txContext.operationContext.id; - - Object.assign(txContext.operationContext, { - type: 'change-request', - }); - - expect(txContext.operationContext.type).toBe('change-request'); - expect(txContext.operationContext.id).toBe(originalId); - }); - }); - - describe('transaction access', () => { - it('should provide access to the underlying transaction', () => { - const txContext = new TransactionContext(mockTransaction); - - expect(txContext.transaction).toBe(mockTransaction); - expect(txContext.transaction.select).toBe(mockTransaction.select); - expect(txContext.transaction.insert).toBe(mockTransaction.insert); - expect(txContext.transaction.commit).toBe(mockTransaction.commit); - }); - }); -}); diff --git a/src/lib/util/transactionContext.ts b/src/lib/util/transactionContext.ts deleted file mode 100644 index 0ee0078156..0000000000 --- a/src/lib/util/transactionContext.ts +++ /dev/null @@ -1,25 +0,0 @@ -import type { Knex } from 'knex'; - -export interface OperationContext { - type: 'change-request' | 'transaction'; - id: number; -} - -function generateNumericTransactionId(): number { - const timestamp = Date.now(); - const random = Math.floor(Math.random() * 1000); - return timestamp * 1000 + random; -} - -export class TransactionContext { - public readonly transaction: Knex.Transaction; - public operationContext: OperationContext; - - constructor(transaction: Knex.Transaction) { - this.transaction = transaction; - this.operationContext = { - type: 'transaction', - id: generateNumericTransactionId(), - }; - } -}