From 39a6d991ce3be34fe0c1cadf707fb56854ef98ec Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 22 Sep 2026 06:39:38 +0200 Subject: [PATCH] =?UTF-8?q?refactor(server):=20remove=20DatabaseService.tr?= =?UTF-8?q?ansaction=20=E2=80=94=20every=20transaction=20runs=20through=20?= =?UTF-8?q?UnitOfWork?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- server/src/nest/README.md | 5 +++-- server/src/nest/database/database.service.ts | 5 ----- server/tests/unit/nest/database-service.test.ts | 9 +++------ server/tests/unit/nest/google-route.test.ts | 2 +- server/tests/unit/nest/roadtrip-planning.test.ts | 1 - server/tests/unit/nest/roadtrip.service.test.ts | 1 - 6 files changed, 7 insertions(+), 16 deletions(-) diff --git a/server/src/nest/README.md b/server/src/nest/README.md index c918268e1..4032c5497 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -914,8 +914,9 @@ cycle seams are folded, nothing on the websocket path imports one, and the crons all inject now (`*.job.ts` providers on `scheduling/CronRegistrarService`). 1. **Move the SQL** into `.service.ts` as methods over an injected - `DatabaseService` (`this.db.all/get/run/prepare/transaction`; strict - constructor injection, no `@Optional()`). Preserve every quirk: falsy-coercion + `DatabaseService` (`this.db.all/get/run/prepare`; multi-statement writes go + through `await this.uow.transactional(...)` instead — see the root `CLAUDE.md`; + strict constructor injection, no `@Optional()`). Preserve every quirk: falsy-coercion defaults (`x || fallback`, never `??`), post-insert/post-update re-selects (no RETURNING), `COALESCE` semantics. If a controller already wraps the legacy functions, do not change the service's method surface. The module needs no diff --git a/server/src/nest/database/database.service.ts b/server/src/nest/database/database.service.ts index 103f4c8e1..53568f1a5 100644 --- a/server/src/nest/database/database.service.ts +++ b/server/src/nest/database/database.service.ts @@ -38,11 +38,6 @@ export class DatabaseService { return this.conn.prepare(sql).run(...params); } - /** Run `fn` inside a synchronous better-sqlite3 transaction. */ - transaction(fn: (conn: Database.Database) => T): T { - return this.conn.transaction(() => fn(this.conn))(); - } - // Trip-access helpers delegate to the db/database exports (not this.conn): // tests vi.mock that module and stub the helpers themselves, so the stubs // must keep flowing through here. diff --git a/server/tests/unit/nest/database-service.test.ts b/server/tests/unit/nest/database-service.test.ts index 05575f673..331fa7774 100644 --- a/server/tests/unit/nest/database-service.test.ts +++ b/server/tests/unit/nest/database-service.test.ts @@ -19,18 +19,15 @@ describe('DatabaseService (typed query helpers)', () => { expect(svc.all('SELECT 3 AS three')).toEqual([{ three: 3 }]); }); - it('run + transaction operate on a scratch table', () => { + it('run operates on a scratch table', () => { svc.run('CREATE TEMP TABLE IF NOT EXISTS _dbsvc_test (n INTEGER)'); svc.run('DELETE FROM _dbsvc_test'); const info = svc.run('INSERT INTO _dbsvc_test (n) VALUES (?)', 41); expect(info.changes).toBe(1); - const total = svc.transaction((conn) => { - conn.prepare('INSERT INTO _dbsvc_test (n) VALUES (?)').run(1); - return conn.prepare('SELECT SUM(n) AS s FROM _dbsvc_test').get() as { s: number }; - }); - expect(total.s).toBe(42); + const total = svc.get('SELECT SUM(n) AS s FROM _dbsvc_test') as { s: number }; + expect(total.s).toBe(41); svc.run('DROP TABLE _dbsvc_test'); }); diff --git a/server/tests/unit/nest/google-route.test.ts b/server/tests/unit/nest/google-route.test.ts index 81405a6c5..632a2c860 100644 --- a/server/tests/unit/nest/google-route.test.ts +++ b/server/tests/unit/nest/google-route.test.ts @@ -13,7 +13,7 @@ const routeTestDb = new Database(':memory:'); async function setup() { const maps = { geocodeQuery: vi.fn(), reverseGeocode: vi.fn().mockResolvedValue({ name: null, address: null }) }; - const db = { canAccessTrip: vi.fn(() => ({ user_id: 7 })), get: () => ({ role: 'user' }), transaction: (fn: () => T) => fn() }; + const db = { canAccessTrip: vi.fn(() => ({ user_id: 7 })), get: () => ({ role: 'user' }) }; const places = { create: vi.fn((_trip: string, stop: { name: string }) => ({ id: stop.name })), broadcast: vi.fn() }; const assignments = { dayExists: vi.fn(() => true), createAssignment: vi.fn((dayId: number, placeId: string) => ({ dayId, placeId })), broadcast: vi.fn(), reconcile: vi.fn() }; const permissions = { checkPermission: vi.fn(() => true) }; diff --git a/server/tests/unit/nest/roadtrip-planning.test.ts b/server/tests/unit/nest/roadtrip-planning.test.ts index 8aef0e5bb..741691883 100644 --- a/server/tests/unit/nest/roadtrip-planning.test.ts +++ b/server/tests/unit/nest/roadtrip-planning.test.ts @@ -40,7 +40,6 @@ function setup() { run: vi.fn((_sql: string, tripId: number, key: string, value: string) => tripSettings.set(tripId, { ...tripSettings.get(tripId), [key]: JSON.parse(value) }), ), - transaction: (fn: () => unknown) => fn(), }; const preferences = new RoadtripPreferencesService(preferenceDb as never, realtime as never, uowStub); const days = [{ id: 1, day_number: 1, title: null, date: '2026-09-11', default_transport_mode: 'driving' }]; diff --git a/server/tests/unit/nest/roadtrip.service.test.ts b/server/tests/unit/nest/roadtrip.service.test.ts index 3b86ddfa4..57e06fbc4 100644 --- a/server/tests/unit/nest/roadtrip.service.test.ts +++ b/server/tests/unit/nest/roadtrip.service.test.ts @@ -42,7 +42,6 @@ async function makeService() { get: (sql: string, ...params: unknown[]) => raw.prepare(sql).get(...params as never[]) as T | undefined, all: (sql: string, ...params: unknown[]) => raw.prepare(sql).all(...params as never[]) as T[], run: (sql: string, ...params: unknown[]) => raw.prepare(sql).run(...params as never[]), - transaction: (fn: (conn: unknown) => T) => raw.transaction(() => fn(raw))(), }; // The realtime side is exercised through the controller and the MCP layer; here the // service is under test for what it writes, so a broadcast that goes nowhere is right.