From 88fa13d55aa596c19a8d19c4dd192388adbf4f25 Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 24 Sep 2026 04:59:54 +0200 Subject: [PATCH] =?UTF-8?q?refactor(server):=20plan=204=20=E2=80=94=20conv?= =?UTF-8?q?ert=20the=20program's=20orphaned=20raw-SQL=20callers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../db/repositories/BucketList.repository.ts | 32 ++++ .../db/repositories/DayNotes.repository.ts | 88 +++++++++ server/src/db/repositories/Days.repository.ts | 26 +++ .../IdempotencyKeys.repository.ts | 73 ++++++- .../src/db/repositories/Photos.repository.ts | 29 ++- .../src/db/repositories/Places.repository.ts | 60 ++++++ .../TripInviteTokens.repository.ts | 115 ++++++++++- .../db/repositories/TripMembers.repository.ts | 18 ++ .../src/db/repositories/Trips.repository.ts | 26 +++ .../src/db/repositories/Users.repository.ts | 37 ++++ server/src/nest/addons/addons.service.ts | 29 +-- server/src/nest/app.module.ts | 8 +- server/src/nest/auth/auth.module.ts | 12 +- .../nest/auth/registration-invites.service.ts | 26 +-- server/src/nest/auth/user-cleanup.service.ts | 13 +- .../nest/common/idempotency-cleanup.job.ts | 18 +- server/src/nest/common/idempotency-cleanup.ts | 19 +- .../nest/common/idempotency.interceptor.ts | 126 ++++++++----- server/src/nest/day-notes/day-notes.module.ts | 16 +- .../src/nest/day-notes/day-notes.service.ts | 61 +++--- .../src/nest/llm-parse/llm-config.resolver.ts | 29 ++- server/src/nest/llm-parse/llm-parse.module.ts | 11 +- .../src/nest/mcp-shared/mcp-shared.module.ts | 9 +- .../mcp-shared/mcp-tool-guards.service.ts | 27 ++- server/src/nest/platform/platform.routes.ts | 43 ++--- .../src/nest/public-api/public-api.module.ts | 26 ++- .../src/nest/public-api/public-api.service.ts | 54 +++--- .../nest/realtime/realtime-gateway.module.ts | 9 +- server/src/nest/realtime/realtime.gateway.ts | 19 +- .../nest/transit/google-transit.provider.ts | 4 +- server/src/nest/transit/transit-provider.ts | 22 ++- server/src/nest/transit/transit.module.ts | 9 +- .../nest/trip-invite/trip-invite.module.ts | 8 +- .../nest/trip-invite/trip-invite.service.ts | 36 ++-- server/tests/helpers/mcp-test-controllers.ts | 16 +- server/tests/helpers/plugin-host.ts | 9 +- server/tests/helpers/test-uow.ts | 45 +++++ .../BucketList.repository.test.ts | 56 ++++++ .../repositories/DayNotes.repository.test.ts | 102 ++++++++++ .../db/repositories/Days.repository.test.ts | 22 +++ .../IdempotencyKeys.repository.test.ts | 103 ++++++++++ .../db/repositories/Places.repository.test.ts | 39 ++++ .../TripInviteTokens.repository.test.ts | 114 +++++++++++ .../TripMembers.repository.test.ts | 34 ++++ .../db/repositories/Trips.repository.test.ts | 19 ++ .../db/repositories/Users.repository.test.ts | 14 ++ server/tests/unit/idempotency-cleanup.test.ts | 62 +++--- server/tests/unit/nest/admin.service.test.ts | 25 +-- .../nest/admin.version-notification.test.ts | 23 +-- server/tests/unit/nest/auth.service.test.ts | 26 +-- .../tests/unit/nest/budget.service.db.test.ts | 27 +-- .../tests/unit/nest/day-notes.service.test.ts | 12 +- .../unit/nest/google-transit.provider.test.ts | 30 +-- .../unit/nest/idempotency.interceptor.test.ts | 178 +++++++++++------- .../llm-parse/llm-config.resolver.test.ts | 64 +++---- .../unit/nest/mcp-shared.service.test.ts | 33 ++-- server/tests/unit/nest/oidc.service.test.ts | 25 +-- .../tests/unit/nest/passkey.service.test.ts | 24 +-- .../unit/nest/platform.controller.test.ts | 51 ++--- .../tests/unit/nest/realtime.gateway.test.ts | 65 ++++--- .../nest/registration-invites.service.test.ts | 5 +- .../tests/unit/nest/transit.service.test.ts | 4 +- .../unit/nest/trip-invite.service.test.ts | 10 +- .../unit/nest/trip-members.service.test.ts | 33 ++-- .../unit/nest/trip-read-model.service.test.ts | 24 +-- server/tests/unit/nest/trips.service.test.ts | 29 +-- .../unit/nest/user-cleanup.service.test.ts | 22 ++- 67 files changed, 1812 insertions(+), 671 deletions(-) create mode 100644 server/tests/unit/db/repositories/BucketList.repository.test.ts create mode 100644 server/tests/unit/db/repositories/IdempotencyKeys.repository.test.ts create mode 100644 server/tests/unit/db/repositories/TripInviteTokens.repository.test.ts diff --git a/server/src/db/repositories/BucketList.repository.ts b/server/src/db/repositories/BucketList.repository.ts index b47fd2931..a8c5f9b1f 100644 --- a/server/src/db/repositories/BucketList.repository.ts +++ b/server/src/db/repositories/BucketList.repository.ts @@ -296,4 +296,36 @@ export class BucketListRepository extends TrekRepository { .where('lng', '<=', lngMax) .execute(); } + + // --------------------------------------------------------------------------- + // Plan 4 Task 1 (`public-api.service.ts::listBucketList`) — additive. + // --------------------------------------------------------------------------- + + /** + * `SELECT name, lat, lng, country_code, notes, target_date FROM bucket_list + * WHERE user_id = ? ORDER BY created_at DESC, id DESC`. Deliberately NOT + * {@link listForUser} (which `SELECT *`s and orders by `created_at DESC` + * only, no `id` tiebreak, for the Atlas UI's own list) — a genuinely + * different legacy statement on the same table (rule 2), not a variant + * worth merging. + */ + async listForPublicApi(userId: number): Promise { + return await this.readDb() + .selectFrom('bucket_list') + .select(['name', 'lat', 'lng', 'country_code', 'notes', 'target_date']) + .where('user_id', '=', userId) + .orderBy('created_at', 'desc') + .orderBy('id', 'desc') + .execute(); + } +} + +/** {@link BucketListRepository.listForPublicApi}'s projection. */ +export interface PublicApiBucketListItemRow { + name: string; + lat: number | null; + lng: number | null; + country_code: string | null; + notes: string | null; + target_date: string | null; } diff --git a/server/src/db/repositories/DayNotes.repository.ts b/server/src/db/repositories/DayNotes.repository.ts index 057d505c2..2a97cf429 100644 --- a/server/src/db/repositories/DayNotes.repository.ts +++ b/server/src/db/repositories/DayNotes.repository.ts @@ -118,4 +118,92 @@ export class DayNotesRepository extends TrekRepository { sort_order: input.sort_order, }); } + + // --------------------------------------------------------------------------- + // Plan 4 Task 1 (`day-notes.service.ts`) — additive: the remaining raw + // `this.dbs.all/get/run` statements ruling 8 mis-filed as already clean. + // --------------------------------------------------------------------------- + + /** + * `day-notes.service.ts::list` — `SELECT * FROM day_notes WHERE day_id = ? + * AND trip_id = ? ORDER BY sort_order ASC, created_at ASC`. Raw-bind + * (`number | string`, D4's T5 escape hatch, `DaysRepository.existsInTrip`'s + * precedent): the legacy statement binds the route's raw params with no + * `Number()`/`toRowId` conversion of its own. + */ + async listByDayAndTrip(day_id: number | string, trip_id: number | string): Promise { + return await this.qb('n') + .select(['n.*']) + .where('n.day_id = ? AND n.trip_id = ?', [day_id, trip_id]) + .orderBy({ sort_order: 'asc', created_at: 'asc' }) + .execute('all', false); + } + + /** + * `day-notes.service.ts::getNote` — `SELECT * FROM day_notes WHERE id = ? + * AND day_id = ? AND trip_id = ?`. Same raw-bind seam as + * {@link listByDayAndTrip}. + */ + async findByIdDayTrip(id: number | string, day_id: number | string, trip_id: number | string): Promise { + return await this.qb('n') + .select(['n.*']) + .where('n.id = ? AND n.day_id = ? AND n.trip_id = ?', [id, day_id, trip_id]) + .execute('get', false); + } + + /** + * `day-notes.service.ts::update` — `UPDATE day_notes SET text = ?, time = + * ?, icon = ?, sort_order = ?, color = ? WHERE id = ?` then the same + * `SELECT * FROM day_notes WHERE id = ?` re-select {@link createNote} uses. + * Every field is required (the caller — `DayNotesService.update` — + * already resolves each one's `fields.x !== undefined ? fields.x : + * current.x` fallback, same shape as `createNote`'s docstring). + */ + async updateNote(id: number | string, fields: { + text: string; + time: string | null; + icon: string | null; + sort_order: number | null; + color: string | null; + }): Promise { + await this.qb('n') + .update({ text: fields.text, time: fields.time, icon: fields.icon, sort_order: fields.sort_order, color: fields.color }) + .where('n.id = ?', [id]) + .execute('run'); + return await this.qb('n') + .select(['n.*']) + .where('n.id = ?', [id]) + .execute('get', false); + } + + /** `day-notes.service.ts::remove` — `DELETE FROM day_notes WHERE id = ?`. */ + async deleteById(id: number | string): Promise { + await this.qb('n') + .delete() + .where('n.id = ?', [id]) + .execute('run'); + } + + /** + * `public-api.service.ts::dayNotesByDay` — `SELECT day_id, text, time FROM + * day_notes WHERE trip_id = ? ORDER BY day_id ASC, sort_order ASC`. Kysely: + * `day_id` is a `persist(false)` mirror of the `day` relation + * (`DayNotes.entity.ts`) and is PROJECTED here, so a bare `qb().select([...])` + * would silently drop it (the `ShareTokensRepository`/`CollectionPlacesRepository` + * class-docstring trap). + */ + async listForPublicApi(trip_id: number): Promise<{ day_id: number; text: string; time: string | null }[]> { + return await this.kysely() + .selectFrom('day_notes') + .select(['day_id', 'text', 'time']) + .where('trip_id', '=', trip_id) + .orderBy('day_id', 'asc') + .orderBy('sort_order', 'asc') + .execute(); + } +} + +/** {@link DayNotesRepository.listForPublicApi}'s narrow `day_notes` shape. */ +interface DayNotesPublicApiKyselyDB { + day_notes: { day_id: number; trip_id: number; text: string; time: string | null; sort_order: number | null }; } diff --git a/server/src/db/repositories/Days.repository.ts b/server/src/db/repositories/Days.repository.ts index 23be47c93..9e41d8dc4 100644 --- a/server/src/db/repositories/Days.repository.ts +++ b/server/src/db/repositories/Days.repository.ts @@ -349,4 +349,30 @@ export class DaysRepository extends TrekRepository { } return empty; } + + // --------------------------------------------------------------------------- + // Plan 4 Task 1 (`public-api.service.ts::buildDays`) — additive. + // --------------------------------------------------------------------------- + + /** + * `SELECT id, day_number, date, title, notes FROM days WHERE trip_id = ? + * ORDER BY day_number ASC` — the same shape {@link listPlanDays} already + * establishes for this table, a narrower column set. + */ + async listForPublicApi(trip_id: number): Promise { + return await this.qb('d') + .select(['d.id', 'd.day_number', 'd.date', 'd.title', 'd.notes']) + .where({ trip: trip_id }) + .orderBy({ day_number: 'asc' }) + .execute('all', false); + } +} + +/** {@link DaysRepository.listForPublicApi}'s projection. */ +export interface PublicApiDayProjectionRow { + id: number; + day_number: number; + date: string | null; + title: string | null; + notes: string | null; } diff --git a/server/src/db/repositories/IdempotencyKeys.repository.ts b/server/src/db/repositories/IdempotencyKeys.repository.ts index cfd7f437f..adf402b48 100644 --- a/server/src/db/repositories/IdempotencyKeys.repository.ts +++ b/server/src/db/repositories/IdempotencyKeys.repository.ts @@ -1,4 +1,75 @@ import type { IdempotencyKeys } from '../entities/IdempotencyKeys.entity'; import { TrekRepository } from './_shared/trek-repository'; -export class IdempotencyKeysRepository extends TrekRepository {} +/** The cached response `idempotency.interceptor.ts#lookup`/`#replay` need. */ +export interface IdempotencyResponseRow { + status_code: number; + response_body: string; +} + +/** + * `idempotency_keys` (Plan 4 Task 1). The composite primary key + * (`key`, `user`, `method`, `path` — `IdempotencyKeys.entity.ts`'s own + * `PrimaryKeyProp`) is exactly the scope the interceptor's replay lookup and + * its `INSERT OR IGNORE` write both key on. + */ +export class IdempotencyKeysRepository extends TrekRepository { + /** + * `idempotency.interceptor.ts#lookup` — `SELECT status_code, + * response_body FROM idempotency_keys WHERE key = ? AND user_id = ? AND + * method = ? AND path = ?`. + */ + async findResponse(key: string, userId: number, method: string, path: string): Promise { + const row = await this.findOne( + { key, user: userId, method, path }, + { fields: ['status_code', 'response_body'] }, + ); + return row ? { status_code: row.status_code, response_body: row.response_body } : null; + } + + /** + * `idempotency.interceptor.ts#run`'s `res.json` capture — `INSERT OR + * IGNORE INTO idempotency_keys (key, user_id, method, path, status_code, + * response_body, created_at) VALUES (?, ?, ?, ?, ?, ?, ?)`. `created_at` + * is bound explicitly by the caller (the interceptor's own + * `Math.floor(Date.now() / 1000)`), never left to the column's own + * `defaultRaw` — the legacy statement always named the column, so this + * method always writes it too. `em.upsert` with `onConflictAction: + * 'ignore'` on the table's own composite primary key (rule 4's `INSERT OR + * IGNORE` → `em.upsert` mapping, `TripMembersRepository + * .addIgnoringConflict`'s precedent): two overlapping in-flight requests + * racing to store the SAME key must leave whichever one won untouched, + * never overwritten by the loser. + */ + async insertIfAbsent(input: { + key: string; + user_id: number; + method: string; + path: string; + status_code: number; + response_body: string; + created_at: number; + }): Promise { + await this.upsert( + { + key: input.key, + user: input.user_id, + method: input.method, + path: input.path, + status_code: input.status_code, + response_body: input.response_body, + created_at: input.created_at, + }, + { onConflictFields: ['key', 'user', 'method', 'path'], onConflictAction: 'ignore' }, + ); + } + + /** + * `idempotency-cleanup.ts#purgeExpiredIdempotencyKeys` — `DELETE FROM + * idempotency_keys WHERE created_at < ?`. Returns the row count, matching + * the legacy `result.changes`. + */ + async deleteExpired(cutoff: number): Promise { + return this.nativeDelete({ created_at: { $lt: cutoff } }); + } +} diff --git a/server/src/db/repositories/Photos.repository.ts b/server/src/db/repositories/Photos.repository.ts index 99502a2f5..f3e0efaf0 100644 --- a/server/src/db/repositories/Photos.repository.ts +++ b/server/src/db/repositories/Photos.repository.ts @@ -1,4 +1,31 @@ import type { Photos } from '../entities/Photos.entity'; import { TrekRepository } from './_shared/trek-repository'; -export class PhotosRepository extends TrekRepository {} +/** The narrow `photos` shape {@link PhotosRepository.findTripIdByFilename} needs. */ +interface PhotosKyselyDB { + photos: { filename: string; trip_id: number }; +} + +/** + * `photos` (Plan 4 Task 1's platform.routes.ts pickup — the last table with no + * repository at all; this class existed only as an empty stub before this + * plan). + */ +export class PhotosRepository extends TrekRepository { + /** + * (`platform.routes.ts`'s pre-init `servePhoto` handler, the share-token + * fallback) — `SELECT trip_id FROM photos WHERE filename = ?`. `trip_id` + * is a `persist(false)` mirror of the `trip` relation + * (`Photos.entity.ts`), so this reads through Kysely rather than a bare + * `qb().select(['trip_id'])` — the `ShareTokensRepository`/ + * `CollectionPlacesRepository` class-docstring trap (a bare `persist(false)` + * column select is silently dropped). + */ + async findTripIdByFilename(filename: string): Promise<{ trip_id: number } | undefined> { + return await this.kysely() + .selectFrom('photos') + .where('filename', '=', filename) + .select(['trip_id']) + .executeTakeFirst(); + } +} diff --git a/server/src/db/repositories/Places.repository.ts b/server/src/db/repositories/Places.repository.ts index 4b00fbd5d..a2339e9d4 100644 --- a/server/src/db/repositories/Places.repository.ts +++ b/server/src/db/repositories/Places.repository.ts @@ -1224,6 +1224,66 @@ export class PlacesRepository extends TrekRepository { { image_url, updated_at: currentTimestamp(platform) }, ); } + + // --------------------------------------------------------------------------- + // Plan 4 Task 1 (`public-api.service.ts::placesByDay`) — additive. + // --------------------------------------------------------------------------- + + /** + * `SELECT da.day_id, p.name, p.address, p.lat, p.lng, p.place_time, + * p.end_time, p.duration_minutes, p.notes, p.transport_mode, c.name AS + * category FROM day_assignments da JOIN places p ON p.id = da.place_id + * LEFT JOIN categories c ON c.id = p.category_id WHERE p.trip_id = ? AND + * da.accommodation_id IS NULL ORDER BY da.day_id ASC, da.order_index ASC`. + * Kysely — the SAME cross-table-read escape hatch `ReservationsRepository + * .listUnplannedPlacesForPublicApi`'s precedent (3d Task 4) uses for this + * exact `public-api.service.ts` surface; landed on `PlacesRepository` + * rather than grouped there because the projection is place-shaped, per + * this domain plan's own ruling. + */ + async listAssignedForPublicApi(trip_id: number): Promise { + const rows = await this.kysely() + .selectFrom('day_assignments as da') + .innerJoin('places as p', 'p.id', 'da.place_id') + .leftJoin('categories as c', 'c.id', 'p.category_id') + .select([ + 'da.day_id', + 'p.name', 'p.address', 'p.lat', 'p.lng', 'p.place_time', 'p.end_time', + 'p.duration_minutes', 'p.notes', 'p.transport_mode', 'c.name as category', + ]) + .where('p.trip_id', '=', trip_id) + .where('da.accommodation_id', 'is', null) + .orderBy('da.day_id', 'asc') + .orderBy('da.order_index', 'asc') + .execute(); + return rows as PublicApiAssignedPlaceRow[]; + } +} + +/** {@link PlacesRepository.listAssignedForPublicApi}'s projection. */ +export interface PublicApiAssignedPlaceRow { + day_id: number; + name: string; + address: string | null; + lat: number | null; + lng: number | null; + place_time: string | null; + end_time: string | null; + duration_minutes: number | null; + notes: string | null; + transport_mode: string | null; + category: string | null; +} + +/** {@link PlacesRepository.listAssignedForPublicApi}'s narrow `day_assignments`/`places`/`categories` shape. */ +interface PublicApiAssignedPlacesKyselyDB { + day_assignments: { day_id: number; place_id: number; accommodation_id: number | null; order_index: number | null }; + places: { + id: number; trip_id: number; name: string; address: string | null; lat: number | null; lng: number | null; + place_time: string | null; end_time: string | null; duration_minutes: number | null; notes: string | null; + transport_mode: string | null; category_id: number | null; + }; + categories: { id: number; name: string }; } /** {@link PlacesRepository.listImportable}'s row shape (CL45). */ diff --git a/server/src/db/repositories/TripInviteTokens.repository.ts b/server/src/db/repositories/TripInviteTokens.repository.ts index 37b729254..450721b63 100644 --- a/server/src/db/repositories/TripInviteTokens.repository.ts +++ b/server/src/db/repositories/TripInviteTokens.repository.ts @@ -1,4 +1,117 @@ import type { TripInviteTokens } from '../entities/TripInviteTokens.entity'; +import { currentTimestamp } from '../dialect/sql-functions'; import { TrekRepository } from './_shared/trek-repository'; -export class TripInviteTokensRepository extends TrekRepository {} +/** `trip-invite.service.ts::get`'s projection. */ +export interface TripInviteInfoRow { + token: string; + expires_at: string | null; + created_at: string; +} + +/** `trip-invite.service.ts::resolve`'s joined projection. */ +export interface TripInviteResolvedRow { + trip_id: number; + title: string; + expires_at: string | null; +} + +interface TripInviteResolveKyselyDB { + trip_invite_tokens: { token: string; trip_id: number; expires_at: string | null }; + trips: { id: number; title: string }; +} + +/** + * `trip_invite_tokens` (Plan 4 Task 1). One row per trip (`uniques: [{ + * properties: ['trip'] }]`, `TripInviteTokens.entity.ts`) — every method + * here is scoped by `trip_id`, raw-bind (`number | string`, D4's T5 escape + * hatch): `TripInviteService`'s own methods take `tripId: string | number` + * and bind it unconverted, matching the legacy statements exactly. + */ +export class TripInviteTokensRepository extends TrekRepository { + /** `get()` — `SELECT token, expires_at, created_at FROM trip_invite_tokens WHERE trip_id = ?`. */ + async findInfoByTrip(trip_id: number | string): Promise { + return await this.qb('t') + .select(['t.token', 't.expires_at', 't.created_at']) + .where('t.trip_id = ?', [trip_id]) + .execute('get', false); + } + + /** + * `createOrRotate`'s probe, inside `uow.transactional` — `SELECT id FROM + * trip_invite_tokens WHERE trip_id = ?`, a bare existence check deciding + * the UPDATE-vs-INSERT branch below. + */ + async existsForTrip(trip_id: number | string): Promise { + const row = await this.qb('t') + .select(['t.id']) + .where('t.trip_id = ?', [trip_id]) + .execute<{ id: number } | undefined>('get', false); + return !!row; + } + + /** + * `createOrRotate`'s UPDATE branch (rotate) — `UPDATE trip_invite_tokens + * SET token = ?, expires_at = ?, created_by = ?, created_at = + * CURRENT_TIMESTAMP WHERE trip_id = ?`. `created_by` writes through + * `createdByRef`, the real joined-column relation property — `created_by` + * itself is a `persist(false)` mirror (`TripInviteTokens.entity.ts`). + * `CURRENT_TIMESTAMP` through the shared dialect helper (rule 5), matching + * the column's own `defaultRaw` literal exactly. + */ + async updateForTrip(trip_id: number | string, input: { token: string; expires_at: string | null; created_by: number }): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.qb('t') + .update({ token: input.token, expires_at: input.expires_at, createdByRef: input.created_by, created_at: currentTimestamp(platform) }) + .where('t.trip_id = ?', [trip_id]) + .execute('run'); + } + + /** + * `createOrRotate`'s INSERT branch (first link) — `INSERT INTO + * trip_invite_tokens (trip_id, token, created_by, expires_at) VALUES (?, + * ?, ?, ?)`. `created_at` is not in the legacy column set — left to the + * column's own `defaultRaw` (`CURRENT_TIMESTAMP`), same as + * `DayNotesRepository.createNote`'s documented shape for an unnamed + * `defaultRaw` column. `trip`, not the raw `trip_id`: `insert()`'s typed + * relation property wants the entity's own primary-key type — the caller + * (`TripInviteService`) only ever calls this once a trip-scoped route has + * already resolved a REAL numeric trip id through `TripAccessGuard`, so + * `Number(...)` here mirrors the same seam `DayNotesService.create`'s own + * `Number(dayId)`/`Number(tripId)` conversion documents. + */ + async insertForTrip(input: { trip_id: number | string; token: string; created_by: number; expires_at: string | null }): Promise { + await this.insert({ + trip: Number(input.trip_id), + token: input.token, + createdByRef: input.created_by, + expires_at: input.expires_at, + }); + } + + /** `remove()` — `DELETE FROM trip_invite_tokens WHERE trip_id = ?`. */ + async deleteByTrip(trip_id: number | string): Promise { + await this.qb('t') + .delete() + .where('t.trip_id = ?', [trip_id]) + .execute('run'); + } + + /** + * `resolve(token)` — `SELECT t.id AS trip_id, t.title AS title, + * ti.expires_at AS expires_at FROM trip_invite_tokens ti JOIN trips t ON + * ti.trip_id = t.id WHERE ti.token = ?`. Kysely (a cross-table join to + * `trips`, the same escape-hatch order every other cross-table read in + * this program uses) — expiry itself is checked in JS by the caller + * (`TripInviteService.resolve`'s own docstring), never in this query. + */ + async resolveTokenToTrip(token: string): Promise { + const row = await this.kysely() + .selectFrom('trip_invite_tokens as ti') + .innerJoin('trips as t', 't.id', 'ti.trip_id') + .select(['t.id as trip_id', 't.title as title', 'ti.expires_at as expires_at']) + .where('ti.token', '=', token) + .executeTakeFirst(); + return row as TripInviteResolvedRow | undefined; + } +} diff --git a/server/src/db/repositories/TripMembers.repository.ts b/server/src/db/repositories/TripMembers.repository.ts index 77b5f8833..ffba34440 100644 --- a/server/src/db/repositories/TripMembers.repository.ts +++ b/server/src/db/repositories/TripMembers.repository.ts @@ -290,4 +290,22 @@ export class TripMembersRepository extends TrekRepository { .where('m.trip_id = ?', [trip_id]) .execute<{ id: number; username: string; display_name: string | null; avatar: string | null }[]>('all', false); } + + // --------------------------------------------------------------------------- + // Plan 4 Task 1 (UC4, `user-cleanup.service.ts::cleanupUserReferences`) — + // additive: the table's own Plan 3c ownership, mis-filed by an earlier + // ledger under Plan 3g/3j and left unconverted. + // --------------------------------------------------------------------------- + + /** + * UC4 — `UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?` + * (GDPR erasure: severs every membership row's reference to a user being + * deleted, without deleting the membership itself). Filtered and set + * through `invitedByRef`, the real joined-column relation property — + * `invited_by` itself is the `persist(false)` mirror `addIgnoringConflict`'s + * docstring above already names. + */ + async clearInvitedBy(userId: number): Promise { + await this.nativeUpdate({ invitedByRef: userId }, { invitedByRef: null }); + } } diff --git a/server/src/db/repositories/Trips.repository.ts b/server/src/db/repositories/Trips.repository.ts index c01a3c28d..f4e1d6e69 100644 --- a/server/src/db/repositories/Trips.repository.ts +++ b/server/src/db/repositories/Trips.repository.ts @@ -947,6 +947,32 @@ export class TripsRepository extends TrekRepository { .where({ id }) .execute<{ start_date: string | null; end_date: string | null } | undefined>('get', false); } + + // --------------------------------------------------------------------------- + // Plan 4 Task 1 (`registration-invites.service.ts::listTripsForInvite`) — + // additive, per the class's own docstring pointing at this precondition + // ("Plan 3c is the one that builds a TripsRepository") now being met. + // --------------------------------------------------------------------------- + + /** + * RI2 — `SELECT id, title FROM trips ORDER BY title COLLATE NOCASE ASC` + * (the admin invite dialog's trip picker). Kysely, `PlacesRepository + * .listImportable`'s precedent: `ORDER BY … COLLATE NOCASE` is Kysely's + * own `OrderByItemBuilder.collate('nocase')`, a portable Kysely builder + * API, not a raw SQLite fragment — no `sql-functions.ts` helper needed. + */ + async listIdTitleOrderedByTitle(): Promise<{ id: number; title: string }[]> { + return await this.kysely() + .selectFrom('trips') + .select(['id', 'title']) + .orderBy('title', (ob) => ob.collate('nocase').asc()) + .execute(); + } +} + +/** {@link TripsRepository.listIdTitleOrderedByTitle}'s narrow `trips` shape. */ +interface TripIdTitleKyselyDB { + trips: { id: number; title: string }; } /** {@link TripsRepository.lastStartedTrip}'s narrow `trips`/`trip_members` shape. */ diff --git a/server/src/db/repositories/Users.repository.ts b/server/src/db/repositories/Users.repository.ts index 08ddd75ed..0033621dd 100644 --- a/server/src/db/repositories/Users.repository.ts +++ b/server/src/db/repositories/Users.repository.ts @@ -173,6 +173,16 @@ export interface UserWithPasswordVersion { password_version: number; } +/** `realtime.gateway.ts#handleConnection`'s WS-handshake row (Plan 4 Task 1). */ +export interface UserWsHandshakeRow { + id: number; + username: string; + email: string; + role: string; + mfa_enabled: number | boolean | null; + password_version: number; +} + /** `AuthService.requestPasswordReset` (AU36). */ export interface PasswordResetLookup { id: number; @@ -304,6 +314,33 @@ export class UsersRepository extends TrekRepository { return row ? { id: row.id, username: row.username, email: row.email, role: row.role, password_version: row.password_version } : null; } + /** + * `SELECT id, username, email, role, mfa_enabled, password_version FROM + * users WHERE id = ?` (`realtime.gateway.ts#handleConnection`, Plan 4 Task + * 1). Same construction as {@link findByIdWithPasswordVersion} above (the + * canonical password-version read this class already establishes) plus + * `mfa_enabled`, which the WS handshake's own MFA gate also needs from the + * SAME row the legacy statement read — kept as one narrowed read rather + * than two separate repository calls, so a user deleted between two calls + * can't split what was one atomic check under the legacy raw statement. + */ + async findForWsHandshake(id: number): Promise { + const row = await this.findOne( + { id }, + { fields: ['id', 'username', 'email', 'role', 'mfa_enabled', 'password_version'] }, + ); + return row + ? { + id: row.id, + username: row.username, + email: row.email, + role: row.role, + mfa_enabled: row.mfa_enabled ?? null, + password_version: row.password_version, + } + : null; + } + // --------------------------------------------------------------------- // AU2 / TK7 / O2 — password_version (session-invalidation gate) // --------------------------------------------------------------------- diff --git a/server/src/nest/addons/addons.service.ts b/server/src/nest/addons/addons.service.ts index eceefd6ee..5dd084c80 100644 --- a/server/src/nest/addons/addons.service.ts +++ b/server/src/nest/addons/addons.service.ts @@ -31,18 +31,19 @@ import type { UsersRepository } from '../../db/repositories/Users.repository'; * sub-features are opt-out (`!== 'false'`, default ON). Reads are uncached * per-call repository calls so admin toggles stay immediately visible. * - * `DatabaseService` stays injected (Plan 3a Task 4) purely as a passthrough - * for `transit-provider.ts`'s readTransitProvider/writeTransitProvider - * (nest/transit, not one of this plan's six domains — its own conversion is - * a later phase). It does not read/write through `this.db` in THIS file — - * every `app_settings`/`addons`/`photo_providers`/`photo_provider_fields` - * site this service itself used to touch is repository-backed below — so this - * is the same kind of carve-out `permissions`' two guard delegations are: `grep - * DatabaseService src/nest/addons` still finds this constructor parameter and - * its import, not a raw query. `instance-api-keys.ts`'s resolveApiKey - * (nest/settings, Task 5) no longer needs it: `googleKeySource` below passes - * this service's own already-injected `AppSettingsRepository` plus a - * `UsersRepository` Task 5 added here for exactly that. + * `DatabaseService` was injected (Plan 3a Task 4) purely as a passthrough for + * `transit-provider.ts`'s readTransitProvider/writeTransitProvider. Plan 4 + * Task 1 converted those two functions to take an `AppSettingsRepository` + * instead, so this service now passes its own already-injected + * `AppSettingsRepository` (the same one `googleKeySource` below already + * uses) — `this.dbs` is no longer read anywhere in this file. The + * constructor parameter itself stays (rather than being removed) because + * `tests/integration/plugins/boot-registry-order.test.ts` hand-constructs + * this class with a trailing `DatabaseService` argument and is a + * concurrently-owned file this task does not touch (Plan 3j fix-wave + * territory, `tests/integration/plugins/**`); removing the parameter here + * would need editing that file in the same change. Safe to drop as dead-param + * cleanup once that file is free. */ @Injectable() export class AddonsService { @@ -258,10 +259,10 @@ export class AddonsService { } async getTransitProvider(userId = 0) { - return { provider: await readTransitProvider(this.dbs), googleKeySource: await this.googleKeySource(userId) }; + return { provider: await readTransitProvider(this.appSettings), googleKeySource: await this.googleKeySource(userId) }; } async updateTransitProvider(provider: TransitProvider, userId = 0) { - return { provider: await writeTransitProvider(this.dbs, provider), googleKeySource: await this.googleKeySource(userId) }; + return { provider: await writeTransitProvider(this.appSettings, provider), googleKeySource: await this.googleKeySource(userId) }; } } diff --git a/server/src/nest/app.module.ts b/server/src/nest/app.module.ts index f812e89fb..82db3062e 100644 --- a/server/src/nest/app.module.ts +++ b/server/src/nest/app.module.ts @@ -3,6 +3,7 @@ import mikroOrmConfig from '../mikro-orm.config'; import { AppSettings } from '../db/entities/AppSettings.entity'; import { Users } from '../db/entities/Users.entity'; import { WebauthnCredentials } from '../db/entities/WebauthnCredentials.entity'; +import { IdempotencyKeys } from '../db/entities/IdempotencyKeys.entity'; import { McpModule } from '../nest-mcp'; import { AccommodationsModule } from './accommodations/accommodations.module'; import { AddonsModule } from './addons/addons.module'; @@ -165,8 +166,11 @@ import { APP_FILTER, APP_GUARD, APP_INTERCEPTOR, APP_PIPE } from '@nestjs/core'; // that needs `forFeature` for its three repositories; `Users` is not // otherwise global (JwtAuthGuard and friends inject `EntityManager` // instead precisely to avoid needing this everywhere — see their own - // docstrings). - MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials]), + // docstrings). IdempotencyKeys: Plan 4 Task 1 — IdempotencyInterceptor/ + // IdempotencyCleanupJob are ALSO registered directly here (common/ has + // no module of its own), so this is the one place their repository's + // `forFeature` needs to live too. + MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, IdempotencyKeys]), ], providers: [ // Default-deny: a route is authenticated unless it carries @Public() or diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index c1abc137f..06792df95 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -15,6 +15,8 @@ import { Journeys } from '../../db/entities/Journeys.entity'; import { JourneyEntries } from '../../db/entities/JourneyEntries.entity'; import { JourneyContributors } from '../../db/entities/JourneyContributors.entity'; import { ShareTokens } from '../../db/entities/ShareTokens.entity'; +import { Trips } from '../../db/entities/Trips.entity'; +import { TripMembers } from '../../db/entities/TripMembers.entity'; import { TokensModule } from '../tokens/tokens.module'; import { AuthPublicController } from './auth-public.controller'; import { AuthController } from './auth.controller'; @@ -95,8 +97,14 @@ import { AllowedFileTypesModule } from '../files/allowed-file-types.module'; // McpTokens/OauthTokens above (owned by `nest/journey`, registered here // for THIS module's own `@InjectRepository` params). ShareTokens: Plan // 3h Task 6's own addition, the SAME cross-domain shape — UC6's - // `share_tokens` erasure delete (owned by `nest/share`). - MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems, JourneyShareTokens, Journeys, JourneyEntries, JourneyContributors, ShareTokens])], + // `share_tokens` erasure delete (owned by `nest/share`). Trips: Plan 4 + // Task 1 — `RegistrationInvitesService`'s RI2/RI3 reads, converted off + // `DatabaseService` onto `TripsRepository` now that it exists (Plan 3c). + // TripMembers: Plan 4 Task 1, the SAME cross-domain shape as + // JourneyShareTokens/ShareTokens above — `UserCleanupService + // .cleanupUserReferences`'s UC4 erasure update (owned by + // `nest/trip-membership`). + MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems, JourneyShareTokens, Journeys, JourneyEntries, JourneyContributors, ShareTokens, Trips, TripMembers])], controllers: [AuthPublicController, AuthController, PasskeyController], providers: [AuthService, UserProfileService, RegistrationInvitesService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp], exports: [AuthService, RegistrationInvitesService, PasskeyService, UserCleanupService], diff --git a/server/src/nest/auth/registration-invites.service.ts b/server/src/nest/auth/registration-invites.service.ts index 3ae137064..6957fcc53 100644 --- a/server/src/nest/auth/registration-invites.service.ts +++ b/server/src/nest/auth/registration-invites.service.ts @@ -1,9 +1,10 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import crypto from 'crypto'; -import { DatabaseService } from '../database/database.service'; import { InviteTokens } from '../../db/entities/InviteTokens.entity'; import type { InviteTokensRepository } from '../../db/repositories/InviteTokens.repository'; +import { Trips } from '../../db/entities/Trips.entity'; +import type { TripsRepository } from '../../db/repositories/Trips.repository'; import { toRowId } from '../common/row-id'; /** @@ -22,19 +23,17 @@ import { toRowId } from '../common/row-id'; * another domain's SQL. * * Plan 3b Task 3: `invite_tokens` reads/writes go through - * `InviteTokensRepository` (RI1, RI4–RI7). RI2/RI3 (`trips`) stay on - * `DatabaseService` — `trips` is `nest/trips`' table, not this domain's, and - * Plan 3c is the one that builds a `TripsRepository`; building a shim one - * here would be exactly the "manual synchronization" this migration exists - * to remove (same carve-out shape as Plan 3a's `addons.service.ts` - * `listTripsForInvite`/`createInvite`'s trip-binding validation, documented - * at the inventory's §6). + * `InviteTokensRepository` (RI1, RI4–RI7). RI2/RI3 (`trips`) went through + * `DatabaseService` until Plan 4 Task 1: `trips` is `nest/trips`' table, not + * this domain's, and the carve-out was pending `TripsRepository`'s + * existence — it now exists (Plan 3c) and both reads are additive methods + * on it (`listIdTitleOrderedByTitle`/`existsById`). */ @Injectable() export class RegistrationInvitesService { constructor( - private readonly db: DatabaseService, @InjectRepository(InviteTokens) private readonly inviteTokens: InviteTokensRepository, + @InjectRepository(Trips) private readonly trips: TripsRepository, ) {} /** RI1 — `InviteTokensRepository.listWithCreatorAndTrip()`'s joined projection. */ @@ -44,10 +43,10 @@ export class RegistrationInvitesService { /** * Trips an admin can bind an invite to — id + title only, for the picker - * (#1402). RI2 — stays raw on `DatabaseService` (see the class docstring). + * (#1402). RI2 — `TripsRepository.listIdTitleOrderedByTitle()`. */ async listTripsForInvite() { - return this.db.all('SELECT id, title FROM trips ORDER BY title COLLATE NOCASE ASC'); + return this.trips.listIdTitleOrderedByTitle(); } async createInvite( @@ -63,11 +62,12 @@ export class RegistrationInvitesService { // Optional trip binding: only persist a trip that actually exists, so a stale // or forged id can never bind (and never auto-adds anyone on registration). - // RI3 — stays raw on `DatabaseService` (see the class docstring). + // RI3 — `TripsRepository.existsById()`, the same bare `SELECT id FROM + // trips WHERE id = ?` existence probe. let tripId: number | null = null; if (data.trip_id != null && String(data.trip_id).trim() !== '') { const parsed = Number.parseInt(String(data.trip_id)); - if (!Number.isInteger(parsed) || !this.db.get('SELECT id FROM trips WHERE id = ?', parsed)) { + if (!Number.isInteger(parsed) || !(await this.trips.existsById(parsed))) { // Used to bind null silently, handing back a plain registration invite // the admin never asked for. return { error: 'Trip not found', status: 404 }; diff --git a/server/src/nest/auth/user-cleanup.service.ts b/server/src/nest/auth/user-cleanup.service.ts index 93ddaea54..ce65a8810 100644 --- a/server/src/nest/auth/user-cleanup.service.ts +++ b/server/src/nest/auth/user-cleanup.service.ts @@ -10,6 +10,8 @@ import { pluginsDataRoot } from '../plugins/paths'; import { UnitOfWork } from '../database/unit-of-work'; import { Users } from '../../db/entities/Users.entity'; import type { UsersRepository } from '../../db/repositories/Users.repository'; +import { TripMembers } from '../../db/entities/TripMembers.entity'; +import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository'; import { BudgetItems } from '../../db/entities/BudgetItems.entity'; import type { BudgetItemsRepository } from '../../db/repositories/BudgetItems.repository'; import { JourneyShareTokens } from '../../db/entities/JourneyShareTokens.entity'; @@ -42,6 +44,8 @@ export class UserCleanupService { private readonly budget: BudgetService, private readonly uow: UnitOfWork, @InjectRepository(Users) private readonly usersRepo: UsersRepository, + // Plan 4 Task 1 (UC4 only) — the table's own Plan 3c ownership. + @InjectRepository(TripMembers) private readonly tripMembersRepo: TripMembersRepository, // Plan 3e Task 2 (budget) — additive, UC5 only. @InjectRepository(BudgetItems) private readonly budgetItemsRepo: BudgetItemsRepository, // Plan 3g Task 4 (UC7-10) — the genuinely-journey GDPR-erasure deletes. @@ -101,9 +105,10 @@ export class UserCleanupService { /** * UC4–UC10: `trip_members` (`nest/trip-membership`, Plan 3c) and * `budget_items` (`nest/budget`, Plan 3e) are owned by domains outside - * this plan, so UC4 stays a raw `DatabaseService` call (Plan 3b Task 5 - * ruling; inventory §6) — UC5 is already converted (`budgetItemsRepo - * .clearPaidByUser`). `share_tokens` (UC6) belongs to `nest/share` + * this file's own domain — UC4 was mis-filed as "Plan 3g's" by an earlier + * ledger and stayed raw past that plan's close; it converts here (Plan 4 + * Task 1) onto `TripMembersRepository.clearInvitedBy` — UC5 is already + * converted (`budgetItemsRepo.clearPaidByUser`). `share_tokens` (UC6) belongs to `nest/share` * (trip-level share links) and converts here (Plan 3h Task 6, R10) onto * `ShareTokensRepository.deleteByCreator` — **DISTINCT from * `ShareService`'s own `remove()`**, which deletes `WHERE trip_id = ?`, a @@ -117,7 +122,7 @@ export class UserCleanupService { * unchanged. */ private async cleanupUserReferences(userId: number): Promise { - this.db.run('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?', userId); // UC4 — Plan 3c + await this.tripMembersRepo.clearInvitedBy(userId); // UC4 — converted (Plan 4 Task 1) await this.budgetItemsRepo.clearPaidByUser(userId); // UC5 — Plan 3e Task 2, converted. await this.budget.removeUserFromBudgetItems(userId); await this.shareTokensRepo.deleteByCreator(userId); // UC6 — converted (Plan 3h Task 6) diff --git a/server/src/nest/common/idempotency-cleanup.job.ts b/server/src/nest/common/idempotency-cleanup.job.ts index 6bac3a95d..fe698db51 100644 --- a/server/src/nest/common/idempotency-cleanup.job.ts +++ b/server/src/nest/common/idempotency-cleanup.job.ts @@ -1,6 +1,8 @@ import { Injectable, type OnApplicationBootstrap } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { logInfo, logError } from '../audit/audit-log.logger'; -import { DatabaseService } from '../database/database.service'; +import { IdempotencyKeys } from '../../db/entities/IdempotencyKeys.entity'; +import type { IdempotencyKeysRepository } from '../../db/repositories/IdempotencyKeys.repository'; import { CronRegistrarService } from '../scheduling/cron-registrar.service'; import { purgeExpiredIdempotencyKeys } from './idempotency-cleanup'; @@ -12,23 +14,23 @@ import { purgeExpiredIdempotencyKeys } from './idempotency-cleanup'; @Injectable() export class IdempotencyCleanupJob implements OnApplicationBootstrap { constructor( - private readonly db: DatabaseService, + @InjectRepository(IdempotencyKeys) private readonly idempotencyKeys: IdempotencyKeysRepository, private readonly registrar: CronRegistrarService, ) {} onApplicationBootstrap(): void { if (!this.registrar.isEnabled()) return; - // D6: `tick()` still reads/writes through raw `DatabaseService.prepare` (see - // `purgeExpiredIdempotencyKeys`), but the tick itself already runs inside a - // request context — CronRegistrarService.register wraps every job's onTick in - // one place, so this needs no wrapper of its own even once it gains a - // repository read. + // Plan 4 Task 1: `tick()` now reads/writes through `IdempotencyKeysRepository` + // (see `purgeExpiredIdempotencyKeys`) rather than raw `DatabaseService.prepare` + // — the tick itself already runs inside a request context — + // CronRegistrarService.register wraps every job's onTick in one place, so + // this needs no wrapper of its own even with a repository read/write. this.registrar.register('idempotency-cleanup', '0 3 * * *', () => this.tick()); } async tick(): Promise { try { - const removed = await purgeExpiredIdempotencyKeys(undefined, undefined, this.db); + const removed = await purgeExpiredIdempotencyKeys(undefined, undefined, this.idempotencyKeys); if (removed > 0) { logInfo(`Idempotency cleanup: removed ${removed} expired key(s)`); } diff --git a/server/src/nest/common/idempotency-cleanup.ts b/server/src/nest/common/idempotency-cleanup.ts index 507d58636..f62e502e8 100644 --- a/server/src/nest/common/idempotency-cleanup.ts +++ b/server/src/nest/common/idempotency-cleanup.ts @@ -1,4 +1,5 @@ import { readEnv } from '../../app-config'; +import type { IdempotencyKeysRepository } from '../../db/repositories/IdempotencyKeys.repository'; /** * Idempotency-key TTL purge (moved from src/scheduler.ts; the interceptor that @@ -9,21 +10,19 @@ import { readEnv } from '../../app-config'; * GC'd before the device comes back online would let the replay create a * duplicate. 24h was far too short for a multi-day offline trip; default 30d, * overridable via IDEMPOTENCY_TTL_SECONDS (default lives in app-config). + * + * Plan 4 Task 1: the raw `DELETE FROM idempotency_keys WHERE created_at < ?` + * moved onto `IdempotencyKeysRepository.deleteExpired` — the repository is a + * required parameter now, matching this function's own pre-existing "no lazy + * default" convention (it used to be a `DatabaseService`). */ -export interface PurgeDb { - prepare(sql: string): { run(...args: unknown[]): { changes: number } }; -} - -/** Delete idempotency keys older than the configured TTL. Returns rows removed. - * The db is a required parameter now (DatabaseService satisfies PurgeDb - * structurally) — the old lazy-require default died with the scheduler. */ +/** Delete idempotency keys older than the configured TTL. Returns rows removed. */ export async function purgeExpiredIdempotencyKeys( now: number = Date.now(), ttlSeconds: number = readEnv().session.idempotencyTtlSeconds, - database: PurgeDb, + repo: IdempotencyKeysRepository, ): Promise { const cutoff = Math.floor(now / 1000) - ttlSeconds; - const result = database.prepare('DELETE FROM idempotency_keys WHERE created_at < ?').run(cutoff); - return result.changes; + return await repo.deleteExpired(cutoff); } diff --git a/server/src/nest/common/idempotency.interceptor.ts b/server/src/nest/common/idempotency.interceptor.ts index 40b9ebedc..f6ef4fce2 100644 --- a/server/src/nest/common/idempotency.interceptor.ts +++ b/server/src/nest/common/idempotency.interceptor.ts @@ -1,8 +1,10 @@ import { CallHandler, ExecutionContext, HttpException, Injectable, NestInterceptor } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import type { Request, Response } from 'express'; import { Observable, from, of } from 'rxjs'; import { finalize, mergeAll, switchMap } from 'rxjs/operators'; -import { DatabaseService } from '../database/database.service'; +import { IdempotencyKeys } from '../../db/entities/IdempotencyKeys.entity'; +import type { IdempotencyKeysRepository, IdempotencyResponseRow } from '../../db/repositories/IdempotencyKeys.repository'; /** * Replaces the `applyIdempotency` middleware the Express `authenticate` ran on @@ -30,6 +32,17 @@ import { DatabaseService } from '../database/database.service'; * * Capturing wraps `res.json`, so 204 / `res.end()` responses are not cached — * matching the Express wrapper, which only fires on `res.json`. + * + * Plan 4 Task 1: `lookup`/the `res.json` capture moved off `DatabaseService` + * onto `IdempotencyKeysRepository` — both are now genuinely async repository + * calls, not a synchronous `better-sqlite3` read/write wrapped in a Promise. + * Program rule 11 names THIS file as the "check then act" race the async + * conversion opens up: the `inFlight` claim used to be race-free only because + * `lookup` resolved inside the same macrotask as the call that checked it. Now + * that `lookup` genuinely awaits, the claim is taken BEFORE the first await — + * see `intercept`'s own comment — so a second concurrent request for the same + * key always finds the first request's in-flight promise and waits, rather + * than racing it to an empty `inFlight` map. */ const MUTATING_METHODS = new Set(['POST', 'PUT', 'PATCH', 'DELETE']); @@ -38,20 +51,15 @@ const MAX_CACHED_BODY_BYTES = 256 * 1024; /** * (user, method, path, key) of every request currently running, resolved when it - * answers. In memory rather than a reservation row on purpose: better-sqlite3 is - * synchronous and the whole overlap lives inside one process, so a crash cannot - * leave a key wedged for the table's 30-day TTL. + * answers. In memory rather than a reservation row on purpose: the process + * that claims a signature is always the one that releases it, so a crash + * cannot leave a key wedged for the table's 30-day TTL. */ const inFlight = new Map>(); -interface IdempotencyRow { - status_code: number; - response_body: string; -} - @Injectable() export class IdempotencyInterceptor implements NestInterceptor { - constructor(private readonly database: DatabaseService) {} + constructor(@InjectRepository(IdempotencyKeys) private readonly idempotencyKeys: IdempotencyKeysRepository) {} async intercept(context: ExecutionContext, next: CallHandler): Promise> { const req = context.switchToHttp().getRequest(); @@ -71,9 +79,6 @@ export class IdempotencyInterceptor implements NestInterceptor { throw new HttpException({ error: 'X-Idempotency-Key exceeds maximum length of 128 characters' }, 400); } - const existing = await this.lookup(key, userId, req); - if (existing) return this.replay(existing, res); - const signature = `${userId}|${req.method}|${req.path}|${key}`; const pending = inFlight.get(signature); if (pending !== undefined) { @@ -87,7 +92,27 @@ export class IdempotencyInterceptor implements NestInterceptor { ); } - return await this.run(signature, key, userId, req, res, next); + // Claim the signature BEFORE the first `await` below (rule 11). Two + // requests for the same key that both reach here concurrently must not + // both see "nothing is in flight" — `lookup` is a real repository read + // now, so it can genuinely yield the event loop, and a claim taken only + // after it resolves would leave the exact window between two `awaits` + // that overlapping replays exploited before the in-flight map existed. + // Claiming here, synchronously, means the SECOND request always finds + // the `pending` branch above instead. + const release = this.claim(signature); + try { + const existing = await this.lookup(key, userId, req); + if (existing) { + release(); + return this.replay(existing, res); + } + } catch (err) { + release(); + throw err; + } + + return this.execute(key, userId, req, res, next, release); } /** @@ -105,58 +130,72 @@ export class IdempotencyInterceptor implements NestInterceptor { ): Promise> { const stored = await this.lookup(key, userId, req); if (stored) return this.replay(stored, res); - return await this.run(signature, key, userId, req, res, next); + const release = this.claim(signature); + return this.execute(key, userId, req, res, next, release); + } + + /** + * Reserve `signature` in the in-flight map and hand back its release. Split + * out from `execute` so `intercept` can call it synchronously, before its + * own first `await` — see the class docstring and `intercept`'s comment. + */ + private claim(signature: string): () => void { + let done!: () => void; + inFlight.set(signature, new Promise((resolve) => { done = resolve; })); + let released = false; + // Idempotent: whichever of the two paths below gets there first releases the + // waiter, and the other one is a no-op. + return () => { + if (released) return; + released = true; + inFlight.delete(signature); + done(); + }; } /** * Scope the lookup by method + path as well as user, so the same key replayed * against a different endpoint can't return an unrelated cached body. */ - private async lookup(key: string, userId: number, req: Request): Promise { - return this.database.get( - 'SELECT status_code, response_body FROM idempotency_keys WHERE key = ? AND user_id = ? AND method = ? AND path = ?', - key, userId, req.method, req.path, - ); + private async lookup(key: string, userId: number, req: Request): Promise { + return this.idempotencyKeys.findResponse(key, userId, req.method, req.path); } - private replay(row: IdempotencyRow, res: Response): Observable { + private replay(row: IdempotencyResponseRow, res: Response): Observable { res.status(row.status_code); return of(JSON.parse(row.response_body)); } - private async run( - signature: string, + private execute( key: string, userId: number, req: Request, res: Response, next: CallHandler, - ): Promise> { + release: () => void, + ): Observable { const originalJson = res.json.bind(res); - const database = this.database; - - let done!: () => void; - inFlight.set(signature, new Promise((resolve) => { done = resolve; })); - let released = false; - // Idempotent: whichever of the two paths below gets there first releases the - // waiter, and the other one is a no-op. - const release = () => { - if (released) return; - released = true; - inFlight.delete(signature); - done(); - }; + const idempotencyKeys = this.idempotencyKeys; res.json = function (body: unknown): Response { if (res.statusCode >= 200 && res.statusCode < 300) { try { const serialized = JSON.stringify(body); if (serialized.length <= MAX_CACHED_BODY_BYTES) { - database.run( - `INSERT OR IGNORE INTO idempotency_keys (key, user_id, method, path, status_code, response_body, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?)`, - key, userId, req.method, req.path, res.statusCode, serialized, Math.floor(Date.now() / 1000), - ); + // Fire-and-forget, not awaited: `res.json` is Express's synchronous + // override contract (it must return `Response` for the caller to + // keep chaining), and the store write's own errors are already + // non-fatal (below) — same shape as the legacy synchronous + // `database.run` this replaces, just genuinely async underneath. + void idempotencyKeys + .insertIfAbsent({ + key, user_id: userId, method: req.method, path: req.path, + status_code: res.statusCode, response_body: serialized, + created_at: Math.floor(Date.now() / 1000), + }) + .catch(() => { /* Non-fatal: if storage fails, the request still succeeds. */ }) + .finally(release); + return originalJson(body); } } catch { // Non-fatal: if storage fails, the request still succeeds. @@ -165,9 +204,6 @@ export class IdempotencyInterceptor implements NestInterceptor { // Release here, not in finalize: this is the point the row exists, and a // waiter woken any earlier looks the key up, misses, and runs the handler // a second time - the duplicate write the key is meant to prevent. - // Release here, not in finalize: this is the point the row exists, and a - // waiter woken any earlier looks the key up, misses, and runs the handler - // a second time - the duplicate write the key is meant to prevent. release(); return originalJson(body); }; diff --git a/server/src/nest/day-notes/day-notes.module.ts b/server/src/nest/day-notes/day-notes.module.ts index c2ff48110..52f63518a 100644 --- a/server/src/nest/day-notes/day-notes.module.ts +++ b/server/src/nest/day-notes/day-notes.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { DayNotesController } from './day-notes.controller'; import { DayNotesService } from './day-notes.service'; import { DayNotesMcp } from './day-notes.mcp'; @@ -8,20 +9,27 @@ import { AuthModule } from '../auth/auth.module'; import { RealtimeModule } from '../realtime/realtime.module'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import { Days } from '../../db/entities/Days.entity'; /** * Day notes. Its own domain rather than a second file set inside days/, which * carried two fachlichkeiten with a full controller/service/mcp/rpc/dto each. * - * Deliberately unconnected to DaysModule in both directions: `dayExists` is raw - * SQL on `days` here, and DaysService reads `day_notes` the same way. Wiring the - * modules to each other would buy nothing and cost a cycle. + * Deliberately unconnected to DaysModule in both directions: `dayExists` goes + * through `DaysRepository.existsInTrip` via this module's own `forFeature` + * registration (Plan 4 Task 1), not through `DaysModule`/`DaysService`, and + * `DaysService` reads `day_notes` through its own `DayNotesRepository` + * injection the same way. Wiring the modules to each other would buy + * nothing and cost a cycle. * * AuthModule is only for DayNotesMcp's demo-user gate, PluginGuardsModule only * for DayNotesRpc. */ @Module({ - imports: [McpSharedModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule], + // DayNotes/Days: Plan 4 Task 1 — DayNotesService's own DayNotesRepository/ + // DaysRepository.existsInTrip, replacing its raw `this.dbs.all/get/run`. + imports: [McpSharedModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, MikroOrmModule.forFeature([DayNotes, Days])], controllers: [DayNotesController], providers: [DayNotesService, DayNotesMcp, DayNotesRpc], exports: [DayNotesService], diff --git a/server/src/nest/day-notes/day-notes.service.ts b/server/src/nest/day-notes/day-notes.service.ts index c448da025..b1c1cae54 100644 --- a/server/src/nest/day-notes/day-notes.service.ts +++ b/server/src/nest/day-notes/day-notes.service.ts @@ -1,15 +1,26 @@ import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { NOTE_COLORS, type TrekWsPayload, type TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; import type { DayNote, User } from '../../types'; import { DatabaseService, type TripAccess } from '../database/database.service'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import type { DayNotesRepository } from '../../db/repositories/DayNotes.repository'; +import { Days } from '../../db/entities/Days.entity'; +import type { DaysRepository } from '../../db/repositories/Days.repository'; /** * Day-notes domain service — the legacy dayNoteService SQL folded in over * the injected DatabaseService (byte-identical statements and * coercions). Trip access rides DatabaseService.canAccessTrip; the 'day_edit' * permission reuses the legacy check. + * + * Plan 4 Task 1: the seven `day_notes`/`days` reads/writes below moved off + * `DatabaseService` onto `DayNotesRepository`/`DaysRepository.existsInTrip` + * (`DatabaseService` stays injected purely for `verifyTripAccess`'s + * `canAccessTrip` delegate — Plan 4 Task 2/3's facade-inline sweep, not this + * task's). */ /** * Only a colour the palette actually offers reaches the column (#1629). @@ -31,6 +42,8 @@ export class DayNotesService { private readonly dbs: DatabaseService, private readonly permissions: PermissionsService, private readonly realtime: RealtimeService, + @InjectRepository(DayNotes) private readonly dayNotes: DayNotesRepository, + @InjectRepository(Days) private readonly days: DaysRepository, ) {} async verifyTripAccess(tripId: string | number, userId: number): Promise { @@ -46,42 +59,46 @@ export class DayNotesService { } async list(dayId: string | number, tripId: string | number) { - return this.dbs.all( - 'SELECT * FROM day_notes WHERE day_id = ? AND trip_id = ? ORDER BY sort_order ASC, created_at ASC', - dayId, tripId, - ); + return this.dayNotes.listByDayAndTrip(dayId, tripId); } async dayExists(dayId: string | number, tripId: string | number) { - return this.dbs.get('SELECT id FROM days WHERE id = ? AND trip_id = ?', dayId, tripId); + return this.days.existsInTrip(dayId, tripId); } async getNote(id: string | number, dayId: string | number, tripId: string | number) { - return this.dbs.get('SELECT * FROM day_notes WHERE id = ? AND day_id = ? AND trip_id = ?', id, dayId, tripId); + return this.dayNotes.findByIdDayTrip(id, dayId, tripId); } async create(dayId: string | number, tripId: string | number, text: string, time?: string | null, icon?: string | null, sortOrder?: number, color?: string | null) { - const result = this.dbs.run( - 'INSERT INTO day_notes (day_id, trip_id, text, time, icon, sort_order, color) VALUES (?, ?, ?, ?, ?, ?, ?)', - dayId, tripId, text.trim(), time || null, icon || '📝', sortOrder ?? 9999, normalizeNoteColor(color), - ); - return this.dbs.get('SELECT * FROM day_notes WHERE id = ?', result.lastInsertRowid); + // `dayId`/`tripId` bind raw (D4's T5 escape hatch, matching the legacy + // statement's own no-conversion bind) — `createNote`'s column set wants + // numbers, but every real caller here already passes an id that matched + // a route/permission check earlier in the request; `Number(...)` mirrors + // that seam exactly (same shape `TripMembersRepository.addIgnoringConflict`'s + // docstring documents for an INSERT-shaped write behind a raw-bind guard). + return this.dayNotes.createNote({ + day_id: Number(dayId), + trip_id: Number(tripId), + text: text.trim(), + time: time || null, + icon: icon || '📝', + sort_order: sortOrder ?? 9999, + color: normalizeNoteColor(color), + }); } async update(id: string | number, current: DayNote, fields: { text?: string; time?: string | null; icon?: string | null; sort_order?: number; color?: string | null }) { - this.dbs.run( - 'UPDATE day_notes SET text = ?, time = ?, icon = ?, sort_order = ?, color = ? WHERE id = ?', - fields.text !== undefined ? fields.text.trim() : current.text, - fields.time !== undefined ? fields.time : current.time, - fields.icon !== undefined ? fields.icon : current.icon, - fields.sort_order !== undefined ? fields.sort_order : current.sort_order, - fields.color !== undefined ? normalizeNoteColor(fields.color) : (current.color ?? null), - id, - ); - return this.dbs.get('SELECT * FROM day_notes WHERE id = ?', id); + return this.dayNotes.updateNote(id, { + text: fields.text !== undefined ? fields.text.trim() : current.text, + time: fields.time !== undefined ? fields.time : current.time, + icon: fields.icon !== undefined ? fields.icon : current.icon, + sort_order: fields.sort_order !== undefined ? fields.sort_order : current.sort_order, + color: fields.color !== undefined ? normalizeNoteColor(fields.color) : (current.color ?? null), + }); } async remove(id: string | number): Promise { - this.dbs.run('DELETE FROM day_notes WHERE id = ?', id); + await this.dayNotes.deleteById(id); } } diff --git a/server/src/nest/llm-parse/llm-config.resolver.ts b/server/src/nest/llm-parse/llm-config.resolver.ts index bda49ba1e..685f2d18d 100644 --- a/server/src/nest/llm-parse/llm-config.resolver.ts +++ b/server/src/nest/llm-parse/llm-config.resolver.ts @@ -1,7 +1,9 @@ +import { InjectRepository } from '@mikro-orm/nestjs'; import { ADDON_IDS } from '../../addons'; import { AddonsService } from '../addons/addons.service'; import { decryptLlmApiKey, LLM_PROVIDERS, type LlmProvider, type ResolvedLlmConfig } from './llm-config'; -import { DatabaseService } from '../database/database.service'; +import { Addons } from '../../db/entities/Addons.entity'; +import type { AddonsRepository } from '../../db/repositories/Addons.repository'; import { SettingsService } from '../settings/settings.service'; import { Injectable } from '@nestjs/common'; @@ -11,14 +13,19 @@ function asProvider(v: unknown): LlmProvider | null { /** * Resolves the effective LLM config for a user, gated by the addon. Injectable - * (settings come from SettingsService); the addon-row read and the addon gate - * still go through the legacy db/adminService seams until their waves migrate. + * (settings come from SettingsService). + * + * Plan 4 Task 1: the addon-row read moved off `DatabaseService` onto + * `AddonsRepository.findById` — the same shape `admin.service.ts`'s own + * LLM-addon config read already established (3a/3i's precedent: `config` is + * a `p.json()` column, so the repository hands back an already-parsed + * object, never a JSON string needing its own `JSON.parse`). */ @Injectable() export class LlmConfigResolver { constructor( private readonly settings: SettingsService, - private readonly dbService: DatabaseService, + @InjectRepository(Addons) private readonly addonsRepo: AddonsRepository, private readonly addons: AddonsService, ) {} @@ -34,17 +41,9 @@ export class LlmConfigResolver { } private async readInstanceConfig(): Promise { - const row = this.dbService.get<{ config?: string } | undefined>( - 'SELECT config FROM addons WHERE id = ?', - ADDON_IDS.LLM_PARSING, - ); - if (!row?.config) return null; - let cfg: Record; - try { - cfg = JSON.parse(row.config || '{}'); - } catch { - return null; - } + const row = await this.addonsRepo.findById(ADDON_IDS.LLM_PARSING); + const cfg = row?.config; + if (!cfg) return null; const provider = asProvider(cfg.provider); const model = typeof cfg.model === 'string' ? cfg.model.trim() : ''; if (!provider || !model) return null; diff --git a/server/src/nest/llm-parse/llm-parse.module.ts b/server/src/nest/llm-parse/llm-parse.module.ts index 0b6ecfb8f..cbffedf1b 100644 --- a/server/src/nest/llm-parse/llm-parse.module.ts +++ b/server/src/nest/llm-parse/llm-parse.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { LlmParseService } from './llm-parse.service'; import { LlmLocalService } from './llm-local.service'; import { LlmLocalController } from './llm-local.controller'; @@ -6,13 +7,21 @@ import { LlmConfigResolver } from './llm-config.resolver'; import { SettingsModule } from '../settings/settings.module'; import { AddonsModule } from '../addons/addons.module'; import { AppConfigModule } from '../app-config/app-config.module'; +import { Addons } from '../../db/entities/Addons.entity'; /** * Provides the LLM booking-import fallback; imported by BookingImportModule. * Exports LlmConfigResolver for the plugin host surface (HostSurfaceRpc). + * + * MikroOrmModule.forFeature([Addons]): Plan 4 Task 1 — + * `LlmConfigResolver.readInstanceConfig`'s own `AddonsRepository` + * `@InjectRepository` param. `AddonsModule` already registers `Addons` for + * its own `AddonsService`, but does not export `MikroOrmModule` from its own + * `exports` list, so this module needs its own `forFeature` registration — + * the same leaf-module pattern `public-api.module.ts` documents. */ @Module({ - imports: [AppConfigModule, SettingsModule, AddonsModule], + imports: [AppConfigModule, SettingsModule, AddonsModule, MikroOrmModule.forFeature([Addons])], controllers: [LlmLocalController], providers: [LlmParseService, LlmLocalService, LlmConfigResolver], exports: [LlmParseService, LlmConfigResolver], diff --git a/server/src/nest/mcp-shared/mcp-shared.module.ts b/server/src/nest/mcp-shared/mcp-shared.module.ts index e384f5c2a..c56812465 100644 --- a/server/src/nest/mcp-shared/mcp-shared.module.ts +++ b/server/src/nest/mcp-shared/mcp-shared.module.ts @@ -1,15 +1,22 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { McpToolGuardsService } from './mcp-tool-guards.service'; import { PermissionsModule } from '../permissions/permissions.module'; +import { Trips } from '../../db/entities/Trips.entity'; +import { Users } from '../../db/entities/Users.entity'; /** * Shared guards for the @McpController domain classes. Deliberately NOT * @Global (the addons/permissions precedent): every consumer imports it * explicitly, so e2e TestingModules resolve it transitively and the module * graph stays honest. Database/Realtime come from their @Global modules. + * + * Trips/Users: Plan 4 Task 1 — `McpToolGuardsService.hasTripPermission`/ + * `isAdminUser`'s own `TripsRepository.getOwnerId`/`UsersRepository.getRole` + * reads, moved off `DatabaseService`. */ @Module({ - imports: [PermissionsModule], + imports: [PermissionsModule, MikroOrmModule.forFeature([Trips, Users])], providers: [McpToolGuardsService], exports: [McpToolGuardsService], }) diff --git a/server/src/nest/mcp-shared/mcp-tool-guards.service.ts b/server/src/nest/mcp-shared/mcp-tool-guards.service.ts index 87d7e66c7..d9bf7a311 100644 --- a/server/src/nest/mcp-shared/mcp-tool-guards.service.ts +++ b/server/src/nest/mcp-shared/mcp-tool-guards.service.ts @@ -1,5 +1,9 @@ import { Injectable } from '@nestjs/common'; -import { DatabaseService } from '../database/database.service'; +import { InjectRepository } from '@mikro-orm/nestjs'; +import { Trips } from '../../db/entities/Trips.entity'; +import type { TripsRepository } from '../../db/repositories/Trips.repository'; +import { Users } from '../../db/entities/Users.entity'; +import type { UsersRepository } from '../../db/repositories/Users.repository'; import { PermissionsService } from '../permissions/permissions.service'; import { RealtimeService } from '../realtime/realtime.service'; @@ -11,11 +15,18 @@ import { RealtimeService } from '../realtime/realtime.service'; * ordinary Nest providers, so they inject this instead. The pure result * helpers (noAccess/permissionDenied/adminRequired and the * src/nest-mcp re-exports) stay in _shared.ts — they carry no dependencies. + * + * Plan 4 Task 1: the trip's `user_id` and both `role` reads moved off + * `DatabaseService` onto `TripsRepository.getOwnerId`/`UsersRepository + * .getRole` — both already existed with the exact narrow column set this + * class needs (a trip-ownership lookup and a role lookup are each used by + * several other domains already converted). */ @Injectable() export class McpToolGuardsService { constructor( - private readonly db: DatabaseService, + @InjectRepository(Trips) private readonly trips: TripsRepository, + @InjectRepository(Users) private readonly users: UsersRepository, private readonly permissions: PermissionsService, private readonly realtime: RealtimeService, ) {} @@ -67,16 +78,14 @@ export class McpToolGuardsService { * on `tripId`. */ async hasTripPermission(action: string, tripId: number | string, userId: number): Promise { - const trip = this.db.get<{ user_id?: number }>('SELECT user_id FROM trips WHERE id = ?', tripId); - if (!trip) return false; - const userRow = this.db.get<{ role?: string }>('SELECT role FROM users WHERE id = ?', userId); - const tripOwnerId = typeof trip.user_id === 'number' ? trip.user_id : null; - return this.permissions.checkPermission(action, userRow?.role ?? 'user', tripOwnerId, userId, tripOwnerId !== userId); + const tripOwnerId = await this.trips.getOwnerId(tripId); + if (tripOwnerId === null) return false; + const role = await this.users.getRole(userId); + return this.permissions.checkPermission(action, role ?? 'user', tripOwnerId, userId, tripOwnerId !== userId); } /** True when the user has the global admin role (mirrors REST `user.role === 'admin'` gates). */ async isAdminUser(userId: number): Promise { - const userRow = this.db.get<{ role?: string }>('SELECT role FROM users WHERE id = ?', userId); - return userRow?.role === 'admin'; + return (await this.users.getRole(userId)) === 'admin'; } } diff --git a/server/src/nest/platform/platform.routes.ts b/server/src/nest/platform/platform.routes.ts index fc87c8ee1..f99d901ad 100644 --- a/server/src/nest/platform/platform.routes.ts +++ b/server/src/nest/platform/platform.routes.ts @@ -4,7 +4,6 @@ import type { EntityManager } from '@mikro-orm/core'; import { readEnv } from '../../app-config'; import { verifyJwtAndLoadUser } from '../auth/jwt-verify'; -import { db } from '../../db/database'; import { withRequestContext } from '../database/request-context'; import { StorageService } from '../storage/storage.service'; import { StorageInvalidKeyError, StorageNotFoundError, type StorageCategory } from '../storage/storage.types'; @@ -12,6 +11,8 @@ import { Users } from '../../db/entities/Users.entity'; import type { UsersRepository } from '../../db/repositories/Users.repository'; import { ShareTokens } from '../../db/entities/ShareTokens.entity'; import type { ShareTokensRepository } from '../../db/repositories/ShareTokens.repository'; +import { Photos } from '../../db/entities/Photos.entity'; +import type { PhotosRepository } from '../../db/repositories/Photos.repository'; // Platform / transport routes extracted verbatim from createApp() (app.ts) so they can be // mounted on either the legacy Express app or the NestJS Express instance (strangler A6/A8). @@ -76,7 +77,7 @@ export function storageStaticHandler(storage: StorageService, category: StorageC }; } -async function servePhoto(storage: StorageService, req: Request, res: Response, users: UsersRepository, shareTokens: ShareTokensRepository): Promise { +async function servePhoto(storage: StorageService, req: Request, res: Response, users: UsersRepository, shareTokens: ShareTokensRepository, photos: PhotosRepository): Promise { const safeName = path.basename(req.params.filename); // Parity: after basename(), the old resolve()+startsWith guard could only // fire when the remaining segment was '..' — keep that exact 403. @@ -113,7 +114,7 @@ async function servePhoto(storage: StorageService, req: Request, res: Response, // Share-token path: require the token to cover the exact trip the // photo belongs to. Expired tokens fall through to 401. - const photo = db.prepare('SELECT trip_id FROM photos WHERE filename = ?').get(safeName) as { trip_id: number } | undefined; + const photo = await photos.findTripIdByFilename(safeName); if (!photo) { res.status(401).send('Authentication required'); return; @@ -141,23 +142,23 @@ async function servePhoto(storage: StorageService, req: Request, res: Response, * `ShareTokensRepository.findTripIdByToken` the SAME way (Plan 3h Task 6, R1 * — the plan's single highest-severity finding: this branch ran unconverted, * against the legacy `db` proxy, for three plans after the JWT half above - * was fixed). Both `orm.em.getRepository(Users)` and - * `orm.em.getRepository(ShareTokens)` are resolved inside the arrow passed - * to `withRequestContext` below: `orm.em` is the global, context-resolving - * EntityManager, and MikroORM's repository object holds a reference to that - * same proxy rather than a snapshot, so a query issued through it later - * resolves whatever `AsyncLocalStorage` context is active AT QUERY TIME — - * not at the moment `getRepository()` was called. What actually gates this - * is `withRequestContext` wrapping the whole `servePhoto(...)` call (both - * repository reads included): removing the wrapper entirely reproduces Task - * 0's `ValidationError: Using global EntityManager instance...` (verified - * directly — mutation-proof in `task-1-report.md`), exactly the failure - * `PHOTOCTX-002`/`SEAM-002` guard against. The sibling `photos` table read - * (`db.prepare('SELECT trip_id FROM photos ...')`, a few lines into - * `servePhoto` below) stays on the legacy `db` proxy — a different domain's - * table, not this plan's — but its `trip_id` MATCH against the converted - * `share` read is the handler's whole authorization and is untouched: only - * the `share_tokens` lookup itself moved, not the comparison built on it. + * was fixed). The sibling `photos` table read (Plan 4 Task 1) now goes + * through `PhotosRepository.findTripIdByFilename` the same way — its + * `trip_id` MATCH against the converted `share` read is the handler's whole + * authorization and is untouched, only the read itself moved off the legacy + * `db` proxy. `orm.em.getRepository(Users)`, `orm.em.getRepository + * (ShareTokens)` and `orm.em.getRepository(Photos)` are all resolved inside + * the arrow passed to `withRequestContext` below: `orm.em` is the global, + * context-resolving EntityManager, and MikroORM's repository object holds a + * reference to that same proxy rather than a snapshot, so a query issued + * through it later resolves whatever `AsyncLocalStorage` context is active + * AT QUERY TIME — not at the moment `getRepository()` was called. What + * actually gates this is `withRequestContext` wrapping the whole + * `servePhoto(...)` call (every repository read included): removing the + * wrapper entirely reproduces Task 0's `ValidationError: Using global + * EntityManager instance...` (verified directly — mutation-proof in + * `task-1-report.md`), exactly the failure `PHOTOCTX-002`/`SEAM-002` guard + * against. * * Optional at the type level only, the same shape `TrekWsAdapter` uses for * its own D6 wrapper (`src/nest/realtime/trek-ws.adapter.ts`): `bootstrap.ts` @@ -202,7 +203,7 @@ export function applyPlatformUploads(app: express.Application, storage: StorageS next(new Error('applyPlatformUploads: no MikroORM available to build a request context for /uploads/photos/*')); return; } - return withRequestContext(orm, () => servePhoto(storage, req, res, orm.em.getRepository(Users), orm.em.getRepository(ShareTokens))).catch(next); + return withRequestContext(orm, () => servePhoto(storage, req, res, orm.em.getRepository(Users), orm.em.getRepository(ShareTokens), orm.em.getRepository(Photos))).catch(next); }); // Block direct access to /uploads/files diff --git a/server/src/nest/public-api/public-api.module.ts b/server/src/nest/public-api/public-api.module.ts index 44cf26a6e..84ff2e0b8 100644 --- a/server/src/nest/public-api/public-api.module.ts +++ b/server/src/nest/public-api/public-api.module.ts @@ -8,6 +8,10 @@ import { TripMembershipModule } from '../trip-membership/trip-membership.module' import { RateLimitModule } from '../common/rate-limit.module'; import { Trips } from '../../db/entities/Trips.entity'; import { Reservations } from '../../db/entities/Reservations.entity'; +import { Days } from '../../db/entities/Days.entity'; +import { Places } from '../../db/entities/Places.entity'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import { BucketList } from '../../db/entities/BucketList.entity'; /** * Public API v1 — the versioned read-only surface for third-party integrations. @@ -27,15 +31,19 @@ import { Reservations } from '../../db/entities/Reservations.entity'; * class itself. No module edge either way — which is the point. */ @Module({ - // `MikroOrmModule.forFeature([Trips, Reservations])` registers - // `TripsRepository` (Plan 3d Task 5 — its trip reads convert onto the same - // repository `TripsModule` owns) and `ReservationsRepository` (Plan 3d - // Task 4's pickup — `reservationsByDay`/`buildAccommodations`/ - // `buildUnplannedPlaces`/`buildUnscheduledReservations`) for - // `PublicApiService`'s `@InjectRepository` constructor params, without - // pulling `TripsModule`/`ReservationsModule` themselves in, per this - // module's leaf-module constraint above. - imports: [TokensModule, TripMembershipModule, RateLimitModule, MikroOrmModule.forFeature([Trips, Reservations])], + // `MikroOrmModule.forFeature([Trips, Reservations, Days, Places, DayNotes, + // BucketList])` registers `TripsRepository` (Plan 3d Task 5 — its trip + // reads convert onto the same repository `TripsModule` owns) and + // `ReservationsRepository` (Plan 3d Task 4's pickup — + // `reservationsByDay`/`buildAccommodations`/`buildUnplannedPlaces`/ + // `buildUnscheduledReservations`), plus `DaysRepository`/ + // `PlacesRepository`/`DayNotesRepository`/`BucketListRepository` (Plan 4 + // Task 1's pickup — `buildDays`/`placesByDay`/`dayNotesByDay`/ + // `listBucketList`) for `PublicApiService`'s `@InjectRepository` + // constructor params, without pulling `TripsModule`/`ReservationsModule`/ + // `DaysModule`/`PlacesModule`/`DayNotesModule`/`AtlasModule` themselves in, + // per this module's leaf-module constraint above. + imports: [TokensModule, TripMembershipModule, RateLimitModule, MikroOrmModule.forFeature([Trips, Reservations, Days, Places, DayNotes, BucketList])], controllers: [PublicApiController], providers: [PublicApiService, ApiTokenGuard], }) diff --git a/server/src/nest/public-api/public-api.service.ts b/server/src/nest/public-api/public-api.service.ts index ed180da58..b11422a57 100644 --- a/server/src/nest/public-api/public-api.service.ts +++ b/server/src/nest/public-api/public-api.service.ts @@ -16,6 +16,14 @@ import { Trips } from '../../db/entities/Trips.entity'; import type { TripsRepository } from '../../db/repositories/Trips.repository'; import { Reservations } from '../../db/entities/Reservations.entity'; import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; +import { Days } from '../../db/entities/Days.entity'; +import type { DaysRepository } from '../../db/repositories/Days.repository'; +import { Places } from '../../db/entities/Places.entity'; +import type { PlacesRepository, PublicApiAssignedPlaceRow } from '../../db/repositories/Places.repository'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import type { DayNotesRepository } from '../../db/repositories/DayNotes.repository'; +import { BucketList } from '../../db/entities/BucketList.entity'; +import type { BucketListRepository } from '../../db/repositories/BucketList.repository'; import { DatabaseService } from '../database/database.service'; import { TripMembershipService } from '../trip-membership/trip-membership.service'; @@ -37,6 +45,12 @@ import { TripMembershipService } from '../trip-membership/trip-membership.servic * The child queries are scoped by `trip_id` in SQL rather than by filtering a * wider result set, so a bug in the include handling cannot widen what a caller * sees; at worst it returns less. + * + * Plan 4 Task 1: the four raw `this.db.all(...)` reads (days, places, + * day-notes, bucket-list) moved onto `DaysRepository`/`PlacesRepository`/ + * `DayNotesRepository`/`BucketListRepository` — `DatabaseService` stays + * injected purely for `getTrip`'s `canAccessTrip` delegate (Plan 4 Task + * 2/3's facade-inline sweep, not this task's). */ @Injectable() export class PublicApiService { @@ -45,6 +59,10 @@ export class PublicApiService { private readonly membership: TripMembershipService, @InjectRepository(Trips) private readonly tripsRepo: TripsRepository, @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, + @InjectRepository(Days) private readonly daysRepo: DaysRepository, + @InjectRepository(Places) private readonly placesRepo: PlacesRepository, + @InjectRepository(DayNotes) private readonly dayNotesRepo: DayNotesRepository, + @InjectRepository(BucketList) private readonly bucketListRepo: BucketListRepository, ) {} /** Every trip the token's owner may read, newest first, without itineraries. */ @@ -103,11 +121,7 @@ export class PublicApiService { * trips for the same rows. */ private async buildDays(tripId: number, include: PublicApiInclude[], dayFields: boolean): Promise { - const days = this.db.all( - `SELECT id, day_number, date, title, notes - FROM days WHERE trip_id = ? ORDER BY day_number ASC`, - tripId, - ); + const days = await this.daysRepo.listForPublicApi(tripId); if (days.length === 0) return []; const placesByDay = include.includes('places') ? await this.placesByDay(tripId) : new Map(); @@ -141,29 +155,12 @@ export class PublicApiService { * the shortlist below. */ private async placesByDay(tripId: number): Promise> { - const rows = this.db.all( - `SELECT da.day_id, - p.name, p.address, p.lat, p.lng, p.place_time, p.end_time, - p.duration_minutes, p.notes, p.transport_mode, - c.name AS category - FROM day_assignments da - JOIN places p ON p.id = da.place_id - LEFT JOIN categories c ON c.id = p.category_id - WHERE p.trip_id = ? - AND da.accommodation_id IS NULL - ORDER BY da.day_id ASC, da.order_index ASC`, - tripId, - ); - return groupBy(rows, (r: PlaceRow) => r.day_id, toPlace); + const rows = await this.placesRepo.listAssignedForPublicApi(tripId); + return groupBy(rows, (r) => r.day_id, toPlace); } private async dayNotesByDay(tripId: number): Promise> { - const rows = this.db.all( - `SELECT day_id, text, time - FROM day_notes WHERE trip_id = ? - ORDER BY day_id ASC, sort_order ASC`, - tripId, - ); + const rows = await this.dayNotesRepo.listForPublicApi(tripId); return groupBy(rows, (r) => r.day_id, (r) => ({ text: r.text, time: r.time ?? null, @@ -247,12 +244,7 @@ export class PublicApiService { * answers change when an unrelated toggle moves is a key nobody can build on. */ async listBucketList(userId: number): Promise { - const rows = this.db.all( - `SELECT name, lat, lng, country_code, notes, target_date - FROM bucket_list WHERE user_id = ? - ORDER BY created_at DESC, id DESC`, - userId, - ); + const rows = await this.bucketListRepo.listForPublicApi(userId); return rows.map((r) => ({ name: r.name, lat: r.lat ?? null, diff --git a/server/src/nest/realtime/realtime-gateway.module.ts b/server/src/nest/realtime/realtime-gateway.module.ts index 2423ebee4..bdfc889e8 100644 --- a/server/src/nest/realtime/realtime-gateway.module.ts +++ b/server/src/nest/realtime/realtime-gateway.module.ts @@ -1,7 +1,10 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { RealtimeGateway } from './realtime.gateway'; import { EphemeralTokenModule } from '../auth/ephemeral-token.module'; import { JourneyDomainModule } from '../journey/journey-domain.module'; +import { AppSettings } from '../../db/entities/AppSettings.entity'; +import { Users } from '../../db/entities/Users.entity'; /** * The transport, kept out of RealtimeModule on purpose. @@ -19,8 +22,10 @@ import { JourneyDomainModule } from '../journey/journey-domain.module'; */ @Module({ // JourneyDomainModule for the book rooms: who may open a journey is asked - // of the same service the REST routes ask. - imports: [EphemeralTokenModule, JourneyDomainModule], + // of the same service the REST routes ask. Users/AppSettings: Plan 4 Task + // 1 — the handshake's password-version and require_mfa reads, moved off + // DatabaseService onto UsersRepository/AppSettingsRepository. + imports: [EphemeralTokenModule, JourneyDomainModule, MikroOrmModule.forFeature([Users, AppSettings])], providers: [RealtimeGateway], }) export class RealtimeGatewayModule {} diff --git a/server/src/nest/realtime/realtime.gateway.ts b/server/src/nest/realtime/realtime.gateway.ts index 21e087e66..110b4ceb4 100644 --- a/server/src/nest/realtime/realtime.gateway.ts +++ b/server/src/nest/realtime/realtime.gateway.ts @@ -10,8 +10,13 @@ import { } from '@nestjs/websockets'; import type { IncomingMessage } from 'node:http'; import type { WebSocketServer } from 'ws'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { DatabaseService } from '../database/database.service'; import { EphemeralTokenService } from '../auth/ephemeral-token.service'; +import { AppSettings } from '../../db/entities/AppSettings.entity'; +import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository'; +import { Users } from '../../db/entities/Users.entity'; +import type { UsersRepository } from '../../db/repositories/Users.repository'; import { User } from '../../types'; import { logError } from '../audit/audit-log.logger'; import { @@ -58,6 +63,10 @@ export class RealtimeGateway private heartbeat: ReturnType | null = null; constructor( + // Stays injected for `handleJoin`'s `this.db.canAccessTrip` delegate — + // the facade-inline sweep (Plan 4 Task 2/3) is a separate task; this + // class's raw `users`/`app_settings` reads converted below (Plan 4 + // Task 1) are unrelated to that delegate call. private readonly db: DatabaseService, private readonly tokens: EphemeralTokenService, /* @@ -66,6 +75,8 @@ export class RealtimeGateway * is a second thing to keep in step with the REST routes. */ private readonly journeys: JourneyDomainService, + @InjectRepository(Users) private readonly users: UsersRepository, + @InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository, ) {} afterInit(server: WebSocketServer): void { @@ -114,10 +125,7 @@ export class RealtimeGateway return; } - const row = this.db.get( - 'SELECT id, username, email, role, mfa_enabled, password_version FROM users WHERE id = ?', - consumed.userId, - ); + const row = await this.users.findForWsHandshake(consumed.userId); if (!row) { socket.close(4001, 'User not found'); return; @@ -136,8 +144,7 @@ export class RealtimeGateway // Don't leak password_version beyond the handshake. const { password_version: _pv, ...user } = row; - const requireMfa = - this.db.get<{ value: string }>("SELECT value FROM app_settings WHERE key = 'require_mfa'")?.value === 'true'; + const requireMfa = (await this.appSettings.getValue('require_mfa')) === 'true'; const mfaOk = user.mfa_enabled === 1 || user.mfa_enabled === true; if (requireMfa && !mfaOk) { socket.close(4403, 'MFA required'); diff --git a/server/src/nest/transit/google-transit.provider.ts b/server/src/nest/transit/google-transit.provider.ts index eb6a5c656..49a115d7d 100644 --- a/server/src/nest/transit/google-transit.provider.ts +++ b/server/src/nest/transit/google-transit.provider.ts @@ -1,7 +1,6 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import { readEnv } from '../../app-config'; -import { DatabaseService } from '../database/database.service'; import { toApiLang } from '../maps/maps.helpers'; import { resolveApiKey, type ApiKeySource } from '../settings/instance-api-keys'; import { AppSettings } from '../../db/entities/AppSettings.entity'; @@ -240,7 +239,6 @@ function stopFrom(stop: GoogleStop | undefined, fallback: GoogleLatLng | undefin @Injectable() export class GoogleTransitProvider { constructor( - private readonly database: DatabaseService, @InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository, @InjectRepository(Users) private readonly usersRepo: UsersRepository, ) {} @@ -256,7 +254,7 @@ export class GoogleTransitProvider { * on an install that flipped the switch before pasting a key. */ async isActive(userId: number): Promise { - if ((await readTransitProvider(this.database)) !== 'google') return false; + if ((await readTransitProvider(this.appSettings)) !== 'google') return false; return !!(await this.resolveKey(userId)).key; } diff --git a/server/src/nest/transit/transit-provider.ts b/server/src/nest/transit/transit-provider.ts index 25751a4b9..a1c969a57 100644 --- a/server/src/nest/transit/transit-provider.ts +++ b/server/src/nest/transit/transit-provider.ts @@ -1,5 +1,5 @@ import { TRANSIT_PROVIDERS, type TransitProvider } from '@trek/shared'; -import type { DatabaseService } from '../database/database.service'; +import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository'; /** * Which backend answers /api/transit (#1699), stored as one `app_settings` row. @@ -8,6 +8,12 @@ import type { DatabaseService } from '../database/database.service'; * precedent) because two modules need the value and neither should own the * other: AddonsService writes it from the admin panel, TransitService reads it * per request. One reader, one writer, one key — no mirrored copy. + * + * Plan 4 Task 1: both functions take an `AppSettingsRepository` instead of a + * `DatabaseService` — the exact single-key `ON CONFLICT(key) DO UPDATE` shape + * `AppSettingsRepository.setValue` already established for this table (3i's + * inventory note on the two distinct `app_settings` upsert shapes — this file + * used the `ON CONFLICT` spelling, not `INSERT OR REPLACE`, preserved as-is). */ export const TRANSIT_PROVIDER_SETTING = 'transit_provider'; @@ -23,16 +29,12 @@ function isTransitProvider(value: unknown): value is TransitProvider { * this one does not know, resolves to Transitous. The alternative is billing an * admin's Google key because a string did not parse. */ -export async function readTransitProvider(db: DatabaseService): Promise { - const row = db.get<{ value: string | null }>('SELECT value FROM app_settings WHERE key = ?', TRANSIT_PROVIDER_SETTING); - return isTransitProvider(row?.value) ? row.value : DEFAULT_TRANSIT_PROVIDER; +export async function readTransitProvider(appSettings: AppSettingsRepository): Promise { + const value = await appSettings.getValue(TRANSIT_PROVIDER_SETTING); + return isTransitProvider(value) ? value : DEFAULT_TRANSIT_PROVIDER; } -export async function writeTransitProvider(db: DatabaseService, provider: TransitProvider): Promise { - db.run( - `INSERT INTO app_settings (key, value) VALUES (?, ?) - ON CONFLICT(key) DO UPDATE SET value = excluded.value`, - TRANSIT_PROVIDER_SETTING, provider - ); +export async function writeTransitProvider(appSettings: AppSettingsRepository, provider: TransitProvider): Promise { + await appSettings.setValue(TRANSIT_PROVIDER_SETTING, provider); return provider; } diff --git a/server/src/nest/transit/transit.module.ts b/server/src/nest/transit/transit.module.ts index 8503fa070..ac06114cb 100644 --- a/server/src/nest/transit/transit.module.ts +++ b/server/src/nest/transit/transit.module.ts @@ -20,9 +20,12 @@ import { Users } from '../../db/entities/Users.entity'; * * MikroOrmModule.forFeature([AppSettings, Users]): GoogleTransitProvider * passes its own AppSettingsRepository/UsersRepository to - * instance-api-keys.ts's resolveApiKey now (Plan 3a Task 5) — everything else - * in this domain (transit-provider.ts included) is still raw SQL through - * DatabaseService (transit's own conversion is a later plan). + * instance-api-keys.ts's resolveApiKey (Plan 3a Task 5) and, since Plan 4 + * Task 1, to `transit-provider.ts`'s `readTransitProvider`/ + * `writeTransitProvider` too — the SAME `AppSettingsRepository` this module + * already registers. `transit.mcp.ts`'s remaining `DatabaseService` + * injection is unrelated: it is the `canAccessTrip` delegate (Plan 4 + * Task 2/3's facade-inline sweep, not this plan's). */ @Module({ // DaysModule + ReservationsModule: TransitMcp's create_transit_journey injects both. diff --git a/server/src/nest/trip-invite/trip-invite.module.ts b/server/src/nest/trip-invite/trip-invite.module.ts index b6a4e4195..f49966abd 100644 --- a/server/src/nest/trip-invite/trip-invite.module.ts +++ b/server/src/nest/trip-invite/trip-invite.module.ts @@ -1,5 +1,6 @@ import { RateLimitModule } from '../common/rate-limit.module'; import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { TripInviteLinkController, TripInviteController } from './trip-invite.controller'; import { TripInviteService } from './trip-invite.service'; import { TripInviteMcp } from './trip-invite.mcp'; @@ -10,14 +11,17 @@ import { RealtimeModule } from '../realtime/realtime.module'; import { AuditModule } from '../audit/audit.module'; import { TripMembershipModule } from '../trip-membership/trip-membership.module'; import { DemoModule } from '../common/demo.module'; +import { TripInviteTokens } from '../../db/entities/TripInviteTokens.entity'; @Module({ // The last four carry TripInviteMcp: McpSharedModule for the RBAC check the // controller does inline, and the three @Global modules it injects out of // (DemoModule added Plan 3i Task 4 fix wave — TripInviteMcp injects // DemoService), which a graph assembled without AppModule (the e2e harness) - // must instantiate itself. - imports: [RateLimitModule, PermissionsModule, AuditModule, TripMembershipModule, McpSharedModule, AppConfigModule, RealtimeModule, DemoModule], + // must instantiate itself. TripInviteTokens: Plan 4 Task 1 — + // TripInviteService's own TripInviteTokensRepository, replacing its raw + // `this.dbs.get/run` statements. + imports: [RateLimitModule, PermissionsModule, AuditModule, TripMembershipModule, McpSharedModule, AppConfigModule, RealtimeModule, DemoModule, MikroOrmModule.forFeature([TripInviteTokens])], controllers: [TripInviteLinkController, TripInviteController], providers: [TripInviteService, TripInviteMcp], }) diff --git a/server/src/nest/trip-invite/trip-invite.service.ts b/server/src/nest/trip-invite/trip-invite.service.ts index 7ff620c5b..c4f716d6d 100644 --- a/server/src/nest/trip-invite/trip-invite.service.ts +++ b/server/src/nest/trip-invite/trip-invite.service.ts @@ -1,10 +1,13 @@ import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import crypto from 'crypto'; import { DatabaseService } from '../database/database.service'; import { UnitOfWork } from '../database/unit-of-work'; import type { TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import { TripMembershipService } from '../trip-membership/trip-membership.service'; +import { TripInviteTokens } from '../../db/entities/TripInviteTokens.entity'; +import type { TripInviteTokensRepository } from '../../db/repositories/TripInviteTokens.repository'; import type { User } from '../../types'; type Trip = TripAccess; @@ -26,6 +29,11 @@ export interface TripInviteInfo { * immediately invalidates the old URL. Trip access and the 'share_manage' * permission mirror the public share link exactly (the invite link lives next * to it in the Share area). + * + * Plan 4 Task 1: `get`/`createOrRotate`/`remove`/`resolve` moved off + * `DatabaseService` onto `TripInviteTokensRepository` — `DatabaseService` + * stays injected purely for `verifyTripAccess`'s `canAccessTrip` delegate + * (Plan 4 Task 2/3's facade-inline sweep, not this task's). */ @Injectable() export class TripInviteService { @@ -34,6 +42,7 @@ export class TripInviteService { private readonly permissions: PermissionsService, private readonly membership: TripMembershipService, private readonly uow: UnitOfWork, + @InjectRepository(TripInviteTokens) private readonly tripInviteTokens: TripInviteTokensRepository, ) {} async verifyTripAccess(tripId: string, userId: number) { @@ -46,10 +55,7 @@ export class TripInviteService { /** The current invite link for a trip, or null if none exists. */ async get(tripId: string | number): Promise { - const row = this.dbs.get<{ token: string; expires_at: string | null; created_at: string }>( - 'SELECT token, expires_at, created_at FROM trip_invite_tokens WHERE trip_id = ?', - tripId, - ); + const row = await this.tripInviteTokens.findInfoByTrip(tripId); return row ? { token: row.token, expires_at: row.expires_at, created_at: row.created_at } : null; } @@ -70,17 +76,11 @@ export class TripInviteService { // can't interleave between them (the trip_id UNIQUE constraint would turn // that into a spurious 500). return await this.uow.transactional(async () => { - const existing = this.dbs.get('SELECT id FROM trip_invite_tokens WHERE trip_id = ?', tripId); + const existing = await this.tripInviteTokens.existsForTrip(tripId); if (existing) { - this.dbs.run( - 'UPDATE trip_invite_tokens SET token = ?, expires_at = ?, created_by = ?, created_at = CURRENT_TIMESTAMP WHERE trip_id = ?', - token, expiresAt, createdBy, tripId, - ); + await this.tripInviteTokens.updateForTrip(tripId, { token, expires_at: expiresAt, created_by: createdBy }); } else { - this.dbs.run( - 'INSERT INTO trip_invite_tokens (trip_id, token, created_by, expires_at) VALUES (?, ?, ?, ?)', - tripId, token, createdBy, expiresAt, - ); + await this.tripInviteTokens.insertForTrip({ trip_id: tripId, token, created_by: createdBy, expires_at: expiresAt }); } return (await this.get(tripId))!; }); @@ -88,7 +88,7 @@ export class TripInviteService { /** Remove the trip's invite link entirely (disable). */ async remove(tripId: string | number): Promise { - this.dbs.run('DELETE FROM trip_invite_tokens WHERE trip_id = ?', tripId); + await this.tripInviteTokens.deleteByTrip(tripId); } /** @@ -98,13 +98,7 @@ export class TripInviteService { * caller never reaches this (the endpoint is JWT-guarded). */ async resolve(token: string): Promise<{ trip_id: number; title: string } | null> { - const row = this.dbs.get<{ trip_id: number; title: string; expires_at: string | null }>( - `SELECT t.id AS trip_id, t.title AS title, ti.expires_at AS expires_at - FROM trip_invite_tokens ti - JOIN trips t ON ti.trip_id = t.id - WHERE ti.token = ?`, - token, - ); + const row = await this.tripInviteTokens.resolveTokenToTrip(token); if (!row) return null; // Check expiry in JS, not SQL: expires_at is stored as an ISO-8601 string // (…T…Z) which does not compare lexicographically against SQLite's diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 65996211d..a99045b59 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -124,7 +124,7 @@ import { AddonsMcp } from '../../src/nest/addons/addons.mcp'; import { createTestUnitOfWork, createTestAppSettingsRepo, createTestCategoriesRepo, createTestTagsRepo, createTestSettingsRepo, createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, - createTestTripMembersRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, + createTestTripMembersRepo, createTestTripInviteTokensRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo, createTestRoadtripViasRepo, createTestRoadtripDayTracksRepo, createTestPlaceDetailsCacheRepo, createTestReservationsRepo, @@ -214,14 +214,16 @@ export async function createMcpTestRegistry(): Promise { // old shape, so `membership` and `webauthn` held the wrong objects and // userCleanup/mailer/tokens were undefined. const realtimeService = new RealtimeService(); - const guards = new McpToolGuardsService(dbService, permissionsService, realtimeService); + // Plan 4 Task 1 constructor-ripple: the trip `user_id`/user `role` reads + // moved off `DatabaseService` onto `TripsRepository`/`UsersRepository`. + const guards = new McpToolGuardsService(mcpOrm.repo(Trips), usersRepo, permissionsService, realtimeService); const exchangeRatesService = new ExchangeRatesService(); const budgetService = new BudgetService(dbService, permissionsService, exchangeRatesService, realtimeService, await createTestUnitOfWork(dbService.connection), ...(await budgetRepoArgs(dbService.connection))); const authService = new AuthService( permissionsService, new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), new WebauthnConfigService(appSettings), - new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection), await createTestShareTokensRepo(dbService.connection)), + new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestTripMembersRepo(dbService.connection), await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection), await createTestShareTokensRepo(dbService.connection)), new MailerService(usersRepo, settingsRepo, appSettings), new EphemeralTokenService(), new AllowedFileTypesService(appSettings), await createTestUnitOfWork(dbService.connection), @@ -320,7 +322,7 @@ export async function createMcpTestRegistry(): Promise { ); // Built after it: a hotel booking writes the stay's day stop through this one. const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(dbService.connection), await createTestReservationEndpointsRepo(dbService.connection), await createTestReservationTravelersRepo(dbService.connection)), accommodationsService, await createTestUnitOfWork(dbService.connection), await createTestReservationsRepo(dbService.connection), await createTestReservationEndpointsRepo(dbService.connection), await createTestReservationTravelersRepo(dbService.connection), await createTestReservationDayPositionsRepo(dbService.connection), await createTestDayAccommodationsRepo(dbService.connection), await createTestDaysRepo(dbService.connection), await createTestPlacesRepo(dbService.connection), await createTestDayAssignmentsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), await createTestUsersRepo(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestBudgetItemsRepo(dbService.connection)); - const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection), await createTestShareTokensRepo(dbService.connection)), permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), usersRepo); + const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestTripMembersRepo(dbService.connection), await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection), await createTestShareTokensRepo(dbService.connection)), permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), usersRepo); const tripsService = new TripsService( dbService, reservationsService, @@ -397,7 +399,7 @@ export async function createMcpTestRegistry(): Promise { new PackingMcp(packingService, authService, addonsService, guards), new BudgetMcp(budgetService, exchangeRatesService, dbService, new RuntimeEnvService(), new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), addonsService, guards, await createTestUnitOfWork(dbService.connection), await createTestPlacesRepo(dbService.connection), await createTestTripsRepo(dbService.connection), demoService), new ReservationsMcp(reservationsService, daysService, budgetService, authService, assignmentsService, guards), - new DayNotesMcp(new DayNotesService(dbService, permissionsService, realtimeService), authService, guards), + new DayNotesMcp(new DayNotesService(dbService, permissionsService, realtimeService, await createTestDayNotesRepo(dbService.connection), await createTestDaysRepo(dbService.connection)), authService, guards), new DaysMcp(daysService, authService, guards), new RoadtripMcp( new RoadtripService( @@ -440,7 +442,7 @@ export async function createMcpTestRegistry(): Promise { new TripPromptsMcp(tripsService, readModelService, packingService, addonsService), new ShareMcp(new ShareService(new SettingsService(await createTestUnitOfWork(dbService.connection), appSettings, await createTestSettingsRepo(dbService.connection)), permissionsService, queryHelpersService, placePhotoCache, await createTestUnitOfWork(dbService.connection), ...(await shareServiceRepoArgs(dbService.connection))), authService, guards), new FeedsMcp(new FeedsService(await createTestTripsRepo(dbService.connection), usersRepo, calendarService), dbService, new RuntimeEnvService(), guards, demoService), - new TripInviteMcp(new TripInviteService(dbService, permissionsService, new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), await createTestUnitOfWork(dbService.connection)), dbService, new RuntimeEnvService(), guards, new AuditService(auditLogRepo, usersRepo), demoService), + new TripInviteMcp(new TripInviteService(dbService, permissionsService, new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), await createTestUnitOfWork(dbService.connection), await createTestTripInviteTokensRepo(dbService.connection)), dbService, new RuntimeEnvService(), guards, new AuditService(auditLogRepo, usersRepo), demoService), new MapsMcp(mapsService), new PlacesMcp(placesService, mapsService, await createTestTripsRepo(dbService.connection), authService, journeyDomain, assignmentsService, guards, await createTestUnitOfWork(dbService.connection)), new CollectionsMcp( @@ -466,7 +468,7 @@ export async function createMcpTestRegistry(): Promise { // (`findUsernameEmail`, UM11's precedent) added in its place. usersRepo, authService, addonsService, ), - new TransitMcp(new TransitService(new GoogleTransitProvider(dbService, appSettings, usersRepo)), daysService, reservationsService, dbService, authService, guards), + new TransitMcp(new TransitService(new GoogleTransitProvider(appSettings, usersRepo)), daysService, reservationsService, dbService, authService, guards), new AtlasMcp(new AtlasService( await createTestBucketListRepo(dbService.connection), await createTestHiddenCountriesRepo(dbService.connection), await createTestHiddenRegionsRepo(dbService.connection), await createTestVisitedCountriesRepo(dbService.connection), diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 3e7239860..32b3a380a 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -91,6 +91,7 @@ import { createTestUsersRepo, } from './test-uow'; import { AppSettings } from '../../src/db/entities/AppSettings.entity'; +import { Addons } from '../../src/db/entities/Addons.entity'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; import { Users } from '../../src/db/entities/Users.entity'; import { Plugins } from '../../src/db/entities/Plugins.entity'; @@ -248,7 +249,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< await createTestTripsRepo(dbs.connection), await createTestPlacesRepo(dbs.connection), await createTestReservationEndpointsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection), ); - const dayNotes = new DayNotesService(dbs, permissions, realtime); + const dayNotes = new DayNotesService(dbs, permissions, realtime, await createTestDayNotesRepo(dbs.connection), await createTestDaysRepo(dbs.connection)); const assignments = new AssignmentsService( dbs, permissions, realtime, queryHelpers, journey, await createTestUnitOfWork(dbs.connection), await createTestDayAssignmentsRepo(dbs.connection), @@ -260,7 +261,9 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< ); const membership = new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)); const notifications = await makeNotificationsService(dbs, realtime); - const llmConfig = new LlmConfigResolver(new SettingsService(await createTestUnitOfWork(dbs.connection), appSettings, await createTestSettingsRepo(dbs.connection)), dbs, addons); + // Plan 4 Task 1 constructor-ripple: LlmConfigResolver's addon-row read + // moved off DatabaseService onto AddonsRepository. + const llmConfig = new LlmConfigResolver(new SettingsService(await createTestUnitOfWork(dbs.connection), appSettings, await createTestSettingsRepo(dbs.connection)), (await sharedTestOrm(dbs.connection)).repo(Addons), addons); const pluginOrm = await sharedTestOrm(dbs.connection); const oauth = new PluginOAuthService(pluginOrm.repo(Plugins), pluginOrm.repo(PluginOauthTokens), pluginOrm.repo(PluginOauthState), pluginOrm.repo(PluginSettingsFields)); const accommodations = new AccommodationsService( @@ -289,7 +292,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< // After accommodations: a hotel booking writes the stay's day stop through it. const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(dbs.connection), await createTestReservationEndpointsRepo(dbs.connection), await createTestReservationTravelersRepo(dbs.connection)), accommodations, await createTestUnitOfWork(dbs.connection), await createTestReservationsRepo(dbs.connection), await createTestReservationEndpointsRepo(dbs.connection), await createTestReservationTravelersRepo(dbs.connection), await createTestReservationDayPositionsRepo(dbs.connection), await createTestDayAccommodationsRepo(dbs.connection), await createTestDaysRepo(dbs.connection), await createTestPlacesRepo(dbs.connection), await createTestDayAssignmentsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection)); const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash, generalStorage, await createTestUnitOfWork(dbs.connection), (await sharedTestOrm(dbs.connection)).em); - const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), usersRepo, await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestShareTokensRepo(dbs.connection)), permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), usersRepo); + const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), usersRepo, await createTestTripMembersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestShareTokensRepo(dbs.connection)), permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), usersRepo); // Plan 3j Task 1 — PluginGuards' own role lookup (PG3/PG4) now goes // through UsersRepository.getRole; `usersRepo` above is the same // sharedTestOrm-backed repository every other service in this file uses. diff --git a/server/tests/helpers/test-uow.ts b/server/tests/helpers/test-uow.ts index b219e08e4..53c752d0f 100644 --- a/server/tests/helpers/test-uow.ts +++ b/server/tests/helpers/test-uow.ts @@ -26,6 +26,12 @@ import { PasswordResetTokens } from '../../src/db/entities/PasswordResetTokens.e import type { PasswordResetTokensRepository } from '../../src/db/repositories/PasswordResetTokens.repository'; import { Trips } from '../../src/db/entities/Trips.entity'; import type { TripsRepository } from '../../src/db/repositories/Trips.repository'; +import { IdempotencyKeys } from '../../src/db/entities/IdempotencyKeys.entity'; +import type { IdempotencyKeysRepository } from '../../src/db/repositories/IdempotencyKeys.repository'; +import { TripInviteTokens } from '../../src/db/entities/TripInviteTokens.entity'; +import type { TripInviteTokensRepository } from '../../src/db/repositories/TripInviteTokens.repository'; +import { Photos } from '../../src/db/entities/Photos.entity'; +import type { PhotosRepository } from '../../src/db/repositories/Photos.repository'; import { TripMembers } from '../../src/db/entities/TripMembers.entity'; import type { TripMembersRepository } from '../../src/db/repositories/TripMembers.repository'; import { Places } from '../../src/db/entities/Places.entity'; @@ -661,3 +667,42 @@ export function createTestCollectionPlaceRatingsRepo(db: Database.Database): Pro collectionPlaceRatingsRepoPerHandle.set(db, pending); return pending; } + +// --------------------------------------------------------------------------- +// Plan 4 Task 1 — the orphaned raw-SQL conversions: `IdempotencyKeysRepository` +// (`idempotency.interceptor.ts`/`idempotency-cleanup.ts`), `TripInviteTokensRepository` +// (`trip-invite.service.ts`) and `PhotosRepository` (`platform.routes.ts`'s +// pre-init `servePhoto` handler) for hand-constructed services/tests. Same +// memoisation-per-handle pattern as every helper above. +// --------------------------------------------------------------------------- + +const idempotencyKeysRepoPerHandle = new WeakMap>(); +const tripInviteTokensRepoPerHandle = new WeakMap>(); +const photosRepoPerHandle = new WeakMap>(); + +/** The `IdempotencyKeysRepository` a hand-constructed `IdempotencyInterceptor`/`IdempotencyCleanupJob` needs. */ +export function createTestIdempotencyKeysRepo(db: Database.Database): Promise { + const existing = idempotencyKeysRepoPerHandle.get(db); + if (existing !== undefined) return existing; + const pending = sharedTestOrm(db).then((t) => t.repo(IdempotencyKeys)); + idempotencyKeysRepoPerHandle.set(db, pending); + return pending; +} + +/** The `TripInviteTokensRepository` a hand-constructed `TripInviteService` needs. */ +export function createTestTripInviteTokensRepo(db: Database.Database): Promise { + const existing = tripInviteTokensRepoPerHandle.get(db); + if (existing !== undefined) return existing; + const pending = sharedTestOrm(db).then((t) => t.repo(TripInviteTokens)); + tripInviteTokensRepoPerHandle.set(db, pending); + return pending; +} + +/** The `PhotosRepository` `platform.routes.ts`'s pre-init `servePhoto` handler needs. */ +export function createTestPhotosRepo(db: Database.Database): Promise { + const existing = photosRepoPerHandle.get(db); + if (existing !== undefined) return existing; + const pending = sharedTestOrm(db).then((t) => t.repo(Photos)); + photosRepoPerHandle.set(db, pending); + return pending; +} diff --git a/server/tests/unit/db/repositories/BucketList.repository.test.ts b/server/tests/unit/db/repositories/BucketList.repository.test.ts new file mode 100644 index 000000000..119a40b70 --- /dev/null +++ b/server/tests/unit/db/repositories/BucketList.repository.test.ts @@ -0,0 +1,56 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createUser } from '../../../helpers/factories'; +import { BucketList } from '../../../../src/db/entities/BucketList.entity'; +import type { BucketListRepository } from '../../../../src/db/repositories/BucketList.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let bucketList: BucketListRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + bucketList = t.repo(BucketList); +}); +beforeEach(() => { resetTestDb(testDb); t.clear(); }); +afterAll(async () => { await t.close(); testDb.close(); }); + +describe('BucketListRepository.listForPublicApi (Plan 4 Task 1, public-api.service.ts::listBucketList)', () => { + it('BUCKETREPO-001: name/lat/lng/country_code/notes/target_date only, ordered by created_at DESC then id DESC, scoped to the caller', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb, { username: 'other' }); + const insert = testDb.prepare( + 'INSERT INTO bucket_list (user_id, name, lat, lng, country_code, notes, target_date, created_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)', + ); + insert.run(user.id, 'Older', 1, 2, 'JP', 'note', null, '2026-01-01T00:00:00.000Z'); + insert.run(user.id, 'Newer', null, null, null, null, '2027-03-01', '2026-02-01T00:00:00.000Z'); + insert.run(other.id, 'Not mine', 9, 9, null, null, null, '2026-03-01T00:00:00.000Z'); + + const legacy = testDb.prepare( + 'SELECT name, lat, lng, country_code, notes, target_date FROM bucket_list WHERE user_id = ? ORDER BY created_at DESC, id DESC', + ).all(user.id); + const rows = await bucketList.listForPublicApi(user.id); + + expect(rows).toEqual(legacy); + expect(rows.map((r) => r.name)).toEqual(['Newer', 'Older']); + }); + + it('BUCKETREPO-002: same-instant rows tiebreak on id DESC (the second ORDER BY key)', async () => { + const { user } = createUser(testDb); + const insert = testDb.prepare( + 'INSERT INTO bucket_list (user_id, name, created_at) VALUES (?, ?, ?)', + ); + insert.run(user.id, 'First inserted', '2026-01-01T00:00:00.000Z'); + insert.run(user.id, 'Second inserted', '2026-01-01T00:00:00.000Z'); + + const rows = await bucketList.listForPublicApi(user.id); + expect(rows.map((r) => r.name)).toEqual(['Second inserted', 'First inserted']); + }); + + it('BUCKETREPO-003: empty array for a user with nothing on their list', async () => { + const { user } = createUser(testDb); + expect(await bucketList.listForPublicApi(user.id)).toEqual([]); + }); +}); diff --git a/server/tests/unit/db/repositories/DayNotes.repository.test.ts b/server/tests/unit/db/repositories/DayNotes.repository.test.ts index 854c6b36a..b44a0a602 100644 --- a/server/tests/unit/db/repositories/DayNotes.repository.test.ts +++ b/server/tests/unit/db/repositories/DayNotes.repository.test.ts @@ -174,3 +174,105 @@ describe('DayNotesRepository.insertNoteCopy (TP71)', () => { expect(row).toEqual({ day_id: day.id, trip_id: trip.id, text: 'Copied note', time: '09:00', icon: '🎒', sort_order: 5, color: null }); }); }); + +describe('DayNotesRepository.listByDayAndTrip (Plan 4 Task 1, day-notes.service.ts::list)', () => { + it('NOTEREPO-012: scoped to BOTH day and trip, ordered by sort_order then created_at', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const otherDay = createDay(testDb, trip.id); + const second = createDayNote(testDb, day.id, trip.id, { sort_order: 5 }); + const first = createDayNote(testDb, day.id, trip.id, { sort_order: 1 }); + createDayNote(testDb, otherDay.id, trip.id); + + const legacy = testDb.prepare('SELECT * FROM day_notes WHERE day_id = ? AND trip_id = ? ORDER BY sort_order ASC, created_at ASC').all(day.id, trip.id); + const rows = await notes.listByDayAndTrip(day.id, trip.id); + expect(rows).toEqual(legacy); + expect(rows.map((r) => r.id)).toEqual([first.id, second.id]); + }); + + it('NOTEREPO-013: a day with no notes returns []', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + expect(await notes.listByDayAndTrip(day.id, trip.id)).toEqual([]); + }); +}); + +describe('DayNotesRepository.findByIdDayTrip (Plan 4 Task 1, day-notes.service.ts::getNote)', () => { + it('NOTEREPO-014: found only under its own id/day/trip triple', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const otherDay = createDay(testDb, trip.id); + const note = createDayNote(testDb, day.id, trip.id, { text: 'Lunch' }); + + expect(await notes.findByIdDayTrip(note.id, day.id, trip.id)).toEqual(note); + expect(await notes.findByIdDayTrip(note.id, otherDay.id, trip.id)).toBeUndefined(); + expect(await notes.findByIdDayTrip(note.id, day.id, trip.id + 1)).toBeUndefined(); + }); +}); + +describe('DayNotesRepository.updateNote (Plan 4 Task 1, day-notes.service.ts::update)', () => { + it('NOTEREPO-015: updates every bound column and returns the re-selected row', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const note = createDayNote(testDb, day.id, trip.id, { text: 'Lunch', time: '12:00', icon: '🍜', sort_order: 2 }); + + const updated = await notes.updateNote(note.id, { text: 'Dinner', time: '19:00', icon: '🍣', sort_order: 3, color: '#2563eb' }); + + expect(updated).toMatchObject({ id: note.id, text: 'Dinner', time: '19:00', icon: '🍣', sort_order: 3, color: '#2563eb' }); + expect(testDb.prepare('SELECT text, time, icon, sort_order, color FROM day_notes WHERE id = ?').get(note.id)) + .toEqual({ text: 'Dinner', time: '19:00', icon: '🍣', sort_order: 3, color: '#2563eb' }); + }); + + it('NOTEREPO-016: undefined for an id that does not exist', async () => { + expect(await notes.updateNote(999999, { text: 'x', time: null, icon: null, sort_order: null, color: null })).toBeUndefined(); + }); +}); + +describe('DayNotesRepository.deleteById (Plan 4 Task 1, day-notes.service.ts::remove)', () => { + it('NOTEREPO-017: deletes by bare id', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const note = createDayNote(testDb, day.id, trip.id); + + await notes.deleteById(note.id); + + expect(testDb.prepare('SELECT id FROM day_notes WHERE id = ?').get(note.id)).toBeUndefined(); + }); + + it('NOTEREPO-018: deleting an id that does not exist is a no-op, not an error', async () => { + await expect(notes.deleteById(999999)).resolves.toBeUndefined(); + }); +}); + +describe('DayNotesRepository.listForPublicApi (Plan 4 Task 1, public-api.service.ts::dayNotesByDay)', () => { + it('NOTEREPO-019: day_id/text/time only, ordered by day_id then sort_order, scoped to the trip', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const other = createTrip(testDb, user.id); + const dayA = createDay(testDb, trip.id); + const dayB = createDay(testDb, trip.id); + createDayNote(testDb, dayB.id, trip.id, { text: 'B-second', sort_order: 5 }); + createDayNote(testDb, dayA.id, trip.id, { text: 'A-first', sort_order: 1 }); + createDayNote(testDb, dayB.id, trip.id, { text: 'B-first', sort_order: 1 }); + createDayNote(testDb, createDay(testDb, other.id).id, other.id, { text: 'Not this trip' }); + + const rows = await notes.listForPublicApi(trip.id); + + expect(rows).toEqual([ + { day_id: dayA.id, text: 'A-first', time: null }, + { day_id: dayB.id, text: 'B-first', time: null }, + { day_id: dayB.id, text: 'B-second', time: null }, + ]); + }); + + it('NOTEREPO-020: empty array for a trip with no notes', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + expect(await notes.listForPublicApi(trip.id)).toEqual([]); + }); +}); diff --git a/server/tests/unit/db/repositories/Days.repository.test.ts b/server/tests/unit/db/repositories/Days.repository.test.ts index 9510d1afa..cd029776d 100644 --- a/server/tests/unit/db/repositories/Days.repository.test.ts +++ b/server/tests/unit/db/repositories/Days.repository.test.ts @@ -331,3 +331,25 @@ describe('DaysRepository.insertDayCopy (TP39)', () => { expect(row).toEqual({ date: null, notes: null, title: null }); }); }); + +describe('DaysRepository.listForPublicApi (Plan 4 Task 1, public-api.service.ts::buildDays)', () => { + it('DAYREPO-027: id/day_number/date/title/notes only, ordered by day_number, scoped to the trip', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const other = createTrip(testDb, user.id); + const day2 = createDay(testDb, trip.id, { day_number: 2, date: '2026-06-02' }); + const day1 = createDay(testDb, trip.id, { day_number: 1, date: '2026-06-01' }); + createDay(testDb, other.id, { day_number: 1 }); + + const legacy = testDb.prepare('SELECT id, day_number, date, title, notes FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(trip.id); + const rows = await days.listForPublicApi(trip.id); + expect(rows).toEqual(legacy); + expect(rows.map((r) => r.id)).toEqual([day1.id, day2.id]); + }); + + it('DAYREPO-028: empty array for a trip with no days', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + expect(await days.listForPublicApi(trip.id)).toEqual([]); + }); +}); diff --git a/server/tests/unit/db/repositories/IdempotencyKeys.repository.test.ts b/server/tests/unit/db/repositories/IdempotencyKeys.repository.test.ts new file mode 100644 index 000000000..723f1f370 --- /dev/null +++ b/server/tests/unit/db/repositories/IdempotencyKeys.repository.test.ts @@ -0,0 +1,103 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createUser } from '../../../helpers/factories'; +import { IdempotencyKeys } from '../../../../src/db/entities/IdempotencyKeys.entity'; +import type { IdempotencyKeysRepository } from '../../../../src/db/repositories/IdempotencyKeys.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let idempotencyKeys: IdempotencyKeysRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + idempotencyKeys = t.repo(IdempotencyKeys); +}); +beforeEach(() => { resetTestDb(testDb); t.clear(); }); +afterAll(async () => { await t.close(); testDb.close(); }); + +describe('IdempotencyKeysRepository.findResponse (Plan 4 Task 1)', () => { + it('IDEMPREPO-001: returns status_code/response_body only, scoped by the full (key, user, method, path) composite key', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb, { username: 'other' }); + testDb.prepare('INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body) VALUES (?, ?, ?, ?, ?, ?)') + .run('k', user.id, 'POST', '/api/categories', 201, '{"id":1}'); + + expect(await idempotencyKeys.findResponse('k', user.id, 'POST', '/api/categories')).toEqual({ status_code: 201, response_body: '{"id":1}' }); + // Same key, different user — a different row entirely, not this one. + expect(await idempotencyKeys.findResponse('k', other.id, 'POST', '/api/categories')).toBeNull(); + // Same key/user, different method or path — must not cross-match. + expect(await idempotencyKeys.findResponse('k', user.id, 'PUT', '/api/categories')).toBeNull(); + expect(await idempotencyKeys.findResponse('k', user.id, 'POST', '/api/tags')).toBeNull(); + }); + + it('IDEMPREPO-002: null when no row matches at all', async () => { + const { user } = createUser(testDb); + expect(await idempotencyKeys.findResponse('missing', user.id, 'POST', '/x')).toBeNull(); + }); +}); + +describe('IdempotencyKeysRepository.insertIfAbsent (Plan 4 Task 1)', () => { + it('IDEMPREPO-003: inserts every bound column, including the caller-supplied created_at', async () => { + const { user } = createUser(testDb); + await idempotencyKeys.insertIfAbsent({ + key: 'k', user_id: user.id, method: 'POST', path: '/api/places', + status_code: 201, response_body: '{"ok":true}', created_at: 1_700_000_000, + }); + + const row = testDb.prepare('SELECT key, user_id, method, path, status_code, response_body, created_at FROM idempotency_keys WHERE key = ?').get('k'); + expect(row).toEqual({ + key: 'k', user_id: user.id, method: 'POST', path: '/api/places', + status_code: 201, response_body: '{"ok":true}', created_at: 1_700_000_000, + }); + }); + + it('IDEMPREPO-004: a second insert on the SAME (key, user, method, path) is ignored — the first write wins (INSERT OR IGNORE parity)', async () => { + const { user } = createUser(testDb); + await idempotencyKeys.insertIfAbsent({ + key: 'k', user_id: user.id, method: 'POST', path: '/api/places', + status_code: 201, response_body: '{"first":true}', created_at: 1_700_000_000, + }); + await idempotencyKeys.insertIfAbsent({ + key: 'k', user_id: user.id, method: 'POST', path: '/api/places', + status_code: 500, response_body: '{"second":true}', created_at: 1_800_000_000, + }); + + expect(await idempotencyKeys.findResponse('k', user.id, 'POST', '/api/places')).toEqual({ status_code: 201, response_body: '{"first":true}' }); + expect(testDb.prepare('SELECT COUNT(*) AS c FROM idempotency_keys').get()).toEqual({ c: 1 }); + }); + + it('IDEMPREPO-005: the SAME key for a DIFFERENT user is a separate row, not a conflict', async () => { + const { user: a } = createUser(testDb); + const { user: b } = createUser(testDb, { username: 'b' }); + await idempotencyKeys.insertIfAbsent({ key: 'k', user_id: a.id, method: 'POST', path: '/x', status_code: 200, response_body: '{}', created_at: 1 }); + await idempotencyKeys.insertIfAbsent({ key: 'k', user_id: b.id, method: 'POST', path: '/x', status_code: 200, response_body: '{}', created_at: 2 }); + expect(testDb.prepare('SELECT COUNT(*) AS c FROM idempotency_keys').get()).toEqual({ c: 2 }); + }); +}); + +describe('IdempotencyKeysRepository.deleteExpired (Plan 4 Task 1)', () => { + it('IDEMPREPO-006: deletes rows strictly older than the cutoff, returns the row count, leaves the rest', async () => { + const { user } = createUser(testDb); + testDb.prepare('INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body, created_at) VALUES (?, ?, ?, ?, ?, ?, ?)') + .run('old', user.id, 'POST', '/x', 200, '{}', 100); + testDb.prepare('INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body, created_at) VALUES (?, ?, ?, ?, ?, ?, ?)') + .run('boundary', user.id, 'POST', '/y', 200, '{}', 200); + testDb.prepare('INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body, created_at) VALUES (?, ?, ?, ?, ?, ?, ?)') + .run('fresh', user.id, 'POST', '/z', 200, '{}', 300); + + const removed = await idempotencyKeys.deleteExpired(200); + + expect(removed).toBe(1); + const keys = testDb.prepare('SELECT key FROM idempotency_keys ORDER BY key').all().map((r: { key: string }) => r.key); + expect(keys).toEqual(['boundary', 'fresh']); + }); + + it('IDEMPREPO-007: zero rows removed when nothing is old enough', async () => { + const { user } = createUser(testDb); + testDb.prepare('INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body, created_at) VALUES (?, ?, ?, ?, ?, ?, ?)') + .run('fresh', user.id, 'POST', '/z', 200, '{}', 1000); + expect(await idempotencyKeys.deleteExpired(500)).toBe(0); + }); +}); diff --git a/server/tests/unit/db/repositories/Places.repository.test.ts b/server/tests/unit/db/repositories/Places.repository.test.ts index d914b82dd..5bdab996b 100644 --- a/server/tests/unit/db/repositories/Places.repository.test.ts +++ b/server/tests/unit/db/repositories/Places.repository.test.ts @@ -1217,3 +1217,42 @@ describe('PlacesRepository.setImageUrlIfUnset (MAP9)', () => { expect((testDb.prepare('SELECT image_url FROM places WHERE id = ?').get(placeB.id) as { image_url: string }).image_url).toBe('/uploads/photo-cache/both.jpg'); }); }); + +describe('PlacesRepository.listAssignedForPublicApi (Plan 4 Task 1, public-api.service.ts::placesByDay)', () => { + it('PLACEREPO-030: day_id + place fields + category name, scoped to the trip, ordered by day then order_index — a booked-night stop is excluded', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const other = createTrip(testDb, user.id); + const category = createCategory(testDb, { name: 'Museum' }); + const dayA = createDay(testDb, trip.id); + const dayB = createDay(testDb, trip.id); + const placeA1 = createPlace(testDb, trip.id, { name: 'A-first', category_id: category.id }); + const placeA2 = createPlace(testDb, trip.id, { name: 'A-second' }); + testDb.prepare('UPDATE places SET category_id = NULL WHERE id = ?').run(placeA2.id); // no category, not the factory's default + const placeB1 = createPlace(testDb, trip.id, { name: 'B-first' }); + const hotelPlace = createPlace(testDb, trip.id, { name: 'Hotel' }); + createDayAssignment(testDb, dayA.id, placeA2.id, { order_index: 5 }); + createDayAssignment(testDb, dayA.id, placeA1.id, { order_index: 1 }); + createDayAssignment(testDb, dayB.id, placeB1.id, { order_index: 0 }); + // A booked-night stop: da.accommodation_id IS NOT NULL, must be excluded. + testDb.prepare('INSERT INTO day_assignments (day_id, place_id, order_index, accommodation_id) VALUES (?, ?, 0, 999)').run(dayA.id, hotelPlace.id); + // Unassigned place in the same trip — never a candidate at all. + createPlace(testDb, trip.id, { name: 'Unassigned' }); + // A place assigned on a DIFFERENT trip — must not leak in. + const otherDay = createDay(testDb, other.id); + createDayAssignment(testDb, otherDay.id, createPlace(testDb, other.id, { name: 'Elsewhere' }).id); + + const rows = await places.listAssignedForPublicApi(trip.id); + + expect(rows.map((r) => r.name)).toEqual(['A-first', 'A-second', 'B-first']); + expect(rows[0]).toMatchObject({ day_id: dayA.id, name: 'A-first', category: 'Museum' }); + expect(rows[1]).toMatchObject({ day_id: dayA.id, name: 'A-second', category: null }); + expect(rows[2]).toMatchObject({ day_id: dayB.id, name: 'B-first' }); + }); + + it('PLACEREPO-031: empty array when nothing is assigned', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + expect(await places.listAssignedForPublicApi(trip.id)).toEqual([]); + }); +}); diff --git a/server/tests/unit/db/repositories/TripInviteTokens.repository.test.ts b/server/tests/unit/db/repositories/TripInviteTokens.repository.test.ts new file mode 100644 index 000000000..e181580d5 --- /dev/null +++ b/server/tests/unit/db/repositories/TripInviteTokens.repository.test.ts @@ -0,0 +1,114 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createTrip, createUser } from '../../../helpers/factories'; +import { TripInviteTokens } from '../../../../src/db/entities/TripInviteTokens.entity'; +import type { TripInviteTokensRepository } from '../../../../src/db/repositories/TripInviteTokens.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let tripInviteTokens: TripInviteTokensRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + tripInviteTokens = t.repo(TripInviteTokens); +}); +beforeEach(() => { resetTestDb(testDb); t.clear(); }); +afterAll(async () => { await t.close(); testDb.close(); }); + +describe('TripInviteTokensRepository.findInfoByTrip / existsForTrip (Plan 4 Task 1)', () => { + it('TIREPO-001: findInfoByTrip returns token/expires_at/created_at only, undefined when no row', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + expect(await tripInviteTokens.findInfoByTrip(trip.id)).toBeUndefined(); + expect(await tripInviteTokens.existsForTrip(trip.id)).toBe(false); + + testDb.prepare("INSERT INTO trip_invite_tokens (trip_id, token, created_by, expires_at) VALUES (?, 'tok', ?, NULL)").run(trip.id, user.id); + + const legacy = testDb.prepare('SELECT token, expires_at, created_at FROM trip_invite_tokens WHERE trip_id = ?').get(trip.id); + expect(await tripInviteTokens.findInfoByTrip(trip.id)).toEqual(legacy); + expect(await tripInviteTokens.existsForTrip(trip.id)).toBe(true); + }); +}); + +describe('TripInviteTokensRepository.insertForTrip / updateForTrip (Plan 4 Task 1)', () => { + it('TIREPO-002: insertForTrip writes the 4-column set, created_at defaults', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + + await tripInviteTokens.insertForTrip({ trip_id: trip.id, token: 'abc', created_by: user.id, expires_at: null }); + + const row = testDb.prepare('SELECT trip_id, token, created_by, expires_at FROM trip_invite_tokens WHERE trip_id = ?').get(trip.id); + expect(row).toEqual({ trip_id: trip.id, token: 'abc', created_by: user.id, expires_at: null }); + expect((testDb.prepare('SELECT created_at FROM trip_invite_tokens WHERE trip_id = ?').get(trip.id) as { created_at: string }).created_at).toBeTruthy(); + }); + + it('TIREPO-003: updateForTrip rewrites token/expires_at/created_by AND bumps created_at, on the SAME row (single row per trip)', async () => { + const { user: owner } = createUser(testDb); + const { user: rotator } = createUser(testDb, { username: 'rotator' }); + const trip = createTrip(testDb, owner.id); + await tripInviteTokens.insertForTrip({ trip_id: trip.id, token: 'first', created_by: owner.id, expires_at: null }); + const before = testDb.prepare('SELECT id, created_at FROM trip_invite_tokens WHERE trip_id = ?').get(trip.id) as { id: number; created_at: string }; + + await tripInviteTokens.updateForTrip(trip.id, { token: 'second', expires_at: '2030-01-01T00:00:00.000Z', created_by: rotator.id }); + + const after = testDb.prepare('SELECT id, token, expires_at, created_by, created_at FROM trip_invite_tokens WHERE trip_id = ?').get(trip.id) as { + id: number; token: string; expires_at: string; created_by: number; created_at: string; + }; + expect(after.id).toBe(before.id); // same row, not a second insert + expect(after.token).toBe('second'); + expect(after.expires_at).toBe('2030-01-01T00:00:00.000Z'); + expect(after.created_by).toBe(rotator.id); + expect(testDb.prepare('SELECT COUNT(*) AS c FROM trip_invite_tokens WHERE trip_id = ?').get(trip.id)).toEqual({ c: 1 }); + }); +}); + +describe('TripInviteTokensRepository.deleteByTrip (Plan 4 Task 1)', () => { + it('TIREPO-004: removes the trip\'s row and only that trip\'s row', async () => { + const { user } = createUser(testDb); + const tripA = createTrip(testDb, user.id); + const tripB = createTrip(testDb, user.id); + await tripInviteTokens.insertForTrip({ trip_id: tripA.id, token: 'a', created_by: user.id, expires_at: null }); + await tripInviteTokens.insertForTrip({ trip_id: tripB.id, token: 'b', created_by: user.id, expires_at: null }); + + await tripInviteTokens.deleteByTrip(tripA.id); + + expect(await tripInviteTokens.existsForTrip(tripA.id)).toBe(false); + expect(await tripInviteTokens.existsForTrip(tripB.id)).toBe(true); + }); + + it('TIREPO-005: deleting a trip with no row is a no-op', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await expect(tripInviteTokens.deleteByTrip(trip.id)).resolves.toBeUndefined(); + }); +}); + +describe('TripInviteTokensRepository.resolveTokenToTrip (Plan 4 Task 1, mutation proof for token validation)', () => { + it('TIREPO-006: resolves a valid token to its trip id/title/expires_at', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Kyoto Loop' }); + await tripInviteTokens.insertForTrip({ trip_id: trip.id, token: 'real-token', created_by: user.id, expires_at: '2030-01-01T00:00:00.000Z' }); + + expect(await tripInviteTokens.resolveTokenToTrip('real-token')).toEqual({ trip_id: trip.id, title: 'Kyoto Loop', expires_at: '2030-01-01T00:00:00.000Z' }); + }); + + it('TIREPO-007: an unknown token resolves to undefined — never a row from a different trip', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await tripInviteTokens.insertForTrip({ trip_id: trip.id, token: 'real-token', created_by: user.id, expires_at: null }); + + expect(await tripInviteTokens.resolveTokenToTrip('forged-token')).toBeUndefined(); + }); + + it("TIREPO-008: a token that used to resolve stops resolving once it's rotated away (mutation proof)", async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await tripInviteTokens.insertForTrip({ trip_id: trip.id, token: 'old-token', created_by: user.id, expires_at: null }); + await tripInviteTokens.updateForTrip(trip.id, { token: 'new-token', expires_at: null, created_by: user.id }); + + expect(await tripInviteTokens.resolveTokenToTrip('old-token')).toBeUndefined(); + expect((await tripInviteTokens.resolveTokenToTrip('new-token'))?.trip_id).toBe(trip.id); + }); +}); diff --git a/server/tests/unit/db/repositories/TripMembers.repository.test.ts b/server/tests/unit/db/repositories/TripMembers.repository.test.ts index e15dbff42..dc9ecb9b8 100644 --- a/server/tests/unit/db/repositories/TripMembers.repository.test.ts +++ b/server/tests/unit/db/repositories/TripMembers.repository.test.ts @@ -380,3 +380,37 @@ describe('TripMembersRepository.listRawUsernameAndDisplayName (RP3)', () => { expect((await tripMembers.listRawUsernameAndDisplayName(String(trip.id))).map((r) => r.id)).toEqual([member.id]); }); }); + +describe('TripMembersRepository.clearInvitedBy (Plan 4 Task 1, UC4)', () => { + it('TMEMREPO-029: nulls invited_by on every row the departing user invited, across trips, leaving other rows untouched', async () => { + const { user: owner } = createUser(testDb); + const { user: departing } = createUser(testDb, { username: 'departing' }); + const { user: other } = createUser(testDb, { username: 'other' }); + const tripA = createTrip(testDb, owner.id); + const tripB = createTrip(testDb, owner.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(tripA.id, other.id, departing.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(tripB.id, owner.id, departing.id); + // Control row: invited by someone else entirely — must survive untouched. + const { user: controlMember } = createUser(testDb, { username: 'control-member' }); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(tripA.id, controlMember.id, owner.id); + + await tripMembers.clearInvitedBy(departing.id); + + const rows = testDb.prepare('SELECT trip_id, user_id, invited_by FROM trip_members ORDER BY trip_id, user_id').all(); + expect(rows).toEqual([ + { trip_id: tripA.id, user_id: other.id, invited_by: null }, + { trip_id: tripA.id, user_id: controlMember.id, invited_by: owner.id }, + { trip_id: tripB.id, user_id: owner.id, invited_by: null }, + ]); + }); + + it('TMEMREPO-030: a user who never invited anyone is a no-op', async () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + + await expect(tripMembers.clearInvitedBy(999999)).resolves.toBeUndefined(); + expect((testDb.prepare('SELECT invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, member.id) as { invited_by: number | null }).invited_by).toBeNull(); + }); +}); diff --git a/server/tests/unit/db/repositories/Trips.repository.test.ts b/server/tests/unit/db/repositories/Trips.repository.test.ts index fde840477..2056d1d5f 100644 --- a/server/tests/unit/db/repositories/Trips.repository.test.ts +++ b/server/tests/unit/db/repositories/Trips.repository.test.ts @@ -943,3 +943,22 @@ describe('TripsRepository.findDatesById (DTR1)', () => { expect(await trips.findDatesById(999999)).toBeUndefined(); }); }); + +describe('TripsRepository.listIdTitleOrderedByTitle (Plan 4 Task 1, RI2)', () => { + it('TRIPREPO-064: id/title only, case-insensitively ordered by title, matching the legacy statement byte-for-byte', async () => { + const { user } = createUser(testDb); + createTrip(testDb, user.id, { title: 'zulu' }); + createTrip(testDb, user.id, { title: 'Alpha' }); + createTrip(testDb, user.id, { title: 'bravo' }); + + const legacy = testDb + .prepare('SELECT id, title FROM trips ORDER BY title COLLATE NOCASE ASC') + .all(); + + expect(await trips.listIdTitleOrderedByTitle()).toEqual(legacy); + }); + + it('TRIPREPO-065: empty array when there are no trips', async () => { + expect(await trips.listIdTitleOrderedByTitle()).toEqual([]); + }); +}); diff --git a/server/tests/unit/db/repositories/Users.repository.test.ts b/server/tests/unit/db/repositories/Users.repository.test.ts index 66cd2317a..72660fb34 100644 --- a/server/tests/unit/db/repositories/Users.repository.test.ts +++ b/server/tests/unit/db/repositories/Users.repository.test.ts @@ -112,6 +112,20 @@ describe('UsersRepository', () => { expect(await users.findByIdWithPasswordVersion(999999)).toBeNull(); }); + it('USERSREPO-009b: findForWsHandshake (Plan 4 Task 1, realtime.gateway.ts) reads id/username/email/role/mfa_enabled/password_version', async () => { + const { user } = createUser(testDb); + testDb.prepare('UPDATE users SET password_version = 3, mfa_enabled = 1 WHERE id = ?').run(user.id); + const row = await users.findForWsHandshake(user.id); + expect(row).toEqual({ id: user.id, username: user.username, email: user.email, role: 'user', mfa_enabled: 1, password_version: 3 }); + expect(await users.findForWsHandshake(999999)).toBeNull(); + }); + + it('USERSREPO-009c: findForWsHandshake reports mfa_enabled as null, not 0, for a user who never had it set (Plan 4 Task 1)', async () => { + const { user } = createUser(testDb); + testDb.prepare('UPDATE users SET mfa_enabled = NULL WHERE id = ?').run(user.id); + expect((await users.findForWsHandshake(user.id))?.mfa_enabled).toBeNull(); + }); + it('USERSREPO-010: getPasswordVersion reads password_version, null for a missing user', async () => { const { user } = createUser(testDb); expect(await users.getPasswordVersion(user.id)).toBe(0); diff --git a/server/tests/unit/idempotency-cleanup.test.ts b/server/tests/unit/idempotency-cleanup.test.ts index a0be7e32f..4584d8b4b 100644 --- a/server/tests/unit/idempotency-cleanup.test.ts +++ b/server/tests/unit/idempotency-cleanup.test.ts @@ -6,32 +6,45 @@ * offline window — otherwise a key GC'd before the device returns lets the * replay create a duplicate. The TTL was raised from 24h to 30d (overridable). */ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { db } from '../../src/db/database'; +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; +import { createSnapshotTestDb } from '../helpers/db-mock'; +import { createTestOrm, type TestOrm } from '../helpers/test-orm'; import { purgeExpiredIdempotencyKeys } from '../../src/nest/common/idempotency-cleanup'; import { IdempotencyCleanupJob } from '../../src/nest/common/idempotency-cleanup.job'; -import { DatabaseService } from '../../src/nest/database/database.service'; +import { IdempotencyKeys } from '../../src/db/entities/IdempotencyKeys.entity'; +import type { IdempotencyKeysRepository } from '../../src/db/repositories/IdempotencyKeys.repository'; import type { CronRegistrarService } from '../../src/nest/scheduling/cron-registrar.service'; const DAY = 24 * 60 * 60; const NOW = 2_000_000_000_000; // fixed ms so the test is deterministic const NOW_SEC = Math.floor(NOW / 1000); -function insertKey(key: string, ageSeconds: number): void { - db.prepare( +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let idempotencyKeys: IdempotencyKeysRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + idempotencyKeys = t.repo(IdempotencyKeys); +}); +afterAll(async () => { await t.close(); testDb.close(); }); + +function insertKey(key: string, ageSeconds: number, nowSec = NOW_SEC): void { + testDb.prepare( `INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body, created_at) VALUES (?, 1, 'POST', '/x', 200, '{}', ?)`, - ).run(key, NOW_SEC - ageSeconds); + ).run(key, nowSec - ageSeconds); } beforeEach(() => { - db.pragma('foreign_keys = OFF'); // fixtures reference a user we don't seed here - db.prepare('DELETE FROM idempotency_keys').run(); + testDb.pragma('foreign_keys = OFF'); // fixtures reference a user we don't seed here + testDb.prepare('DELETE FROM idempotency_keys').run(); + t.clear(); }); afterEach(() => { - db.prepare('DELETE FROM idempotency_keys').run(); - db.pragma('foreign_keys = ON'); + testDb.prepare('DELETE FROM idempotency_keys').run(); + testDb.pragma('foreign_keys = ON'); delete process.env.IDEMPOTENCY_TTL_SECONDS; }); @@ -40,23 +53,23 @@ describe('purgeExpiredIdempotencyKeys', () => { insertKey('old', 31 * DAY); insertKey('fresh', 5 * DAY); - const removed = await purgeExpiredIdempotencyKeys(NOW, undefined, db); + const removed = await purgeExpiredIdempotencyKeys(NOW, undefined, idempotencyKeys); expect(removed).toBe(1); - const keys = db.prepare('SELECT key FROM idempotency_keys').all().map((r: { key: string }) => r.key); + const keys = testDb.prepare('SELECT key FROM idempotency_keys').all().map((r: { key: string }) => r.key); expect(keys).toEqual(['fresh']); }); it('keeps a 25-day-old key that the old 24h TTL would have dropped', async () => { insertKey('offline-trip', 25 * DAY); - expect(await purgeExpiredIdempotencyKeys(NOW, undefined, db)).toBe(0); - expect(db.prepare('SELECT COUNT(*) c FROM idempotency_keys').get()).toMatchObject({ c: 1 }); + expect(await purgeExpiredIdempotencyKeys(NOW, undefined, idempotencyKeys)).toBe(0); + expect(testDb.prepare('SELECT COUNT(*) c FROM idempotency_keys').get()).toMatchObject({ c: 1 }); }); it('respects the IDEMPOTENCY_TTL_SECONDS override', async () => { process.env.IDEMPOTENCY_TTL_SECONDS = String(DAY); insertKey('twoDays', 2 * DAY); - expect(await purgeExpiredIdempotencyKeys(NOW, undefined, db)).toBe(1); + expect(await purgeExpiredIdempotencyKeys(NOW, undefined, idempotencyKeys)).toBe(1); }); }); @@ -70,7 +83,7 @@ describe('IdempotencyCleanupJob', () => { register: vi.fn((_name: string, _expression: string, _onTick: () => void | Promise) => enabled), unregister: vi.fn(), }; - const job = new IdempotencyCleanupJob(new DatabaseService(db), registrar as unknown as CronRegistrarService); + const job = new IdempotencyCleanupJob(idempotencyKeys, registrar as unknown as CronRegistrarService); return { job, registrar }; } @@ -89,31 +102,26 @@ describe('IdempotencyCleanupJob', () => { expect(off.registrar.register).not.toHaveBeenCalled(); }); - it('the tick purges through the injected DatabaseService', async () => { + it('the tick purges through the injected IdempotencyKeysRepository', async () => { // The tick uses the live clock, so these fixtures age against Date.now() // (the pure-function cases above pin their own fixed NOW instead). const liveNowSec = Math.floor(Date.now() / 1000); - const liveInsert = (key: string, ageSeconds: number) => - db.prepare( - `INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body, created_at) - VALUES (?, 1, 'POST', '/x', 200, '{}', ?)`, - ).run(key, liveNowSec - ageSeconds); - liveInsert('old', 31 * DAY); - liveInsert('fresh', 5 * DAY); + insertKey('old', 31 * DAY, liveNowSec); + insertKey('fresh', 5 * DAY, liveNowSec); const { job } = makeJob(); await job.tick(); - const keys = db.prepare('SELECT key FROM idempotency_keys').all().map((r: { key: string }) => r.key); + const keys = testDb.prepare('SELECT key FROM idempotency_keys').all().map((r: { key: string }) => r.key); expect(keys).toEqual(['fresh']); }); it('a failing purge is contained to the Idempotency cleanup log line', async () => { - const broken = { prepare: () => { throw new Error('db gone'); } } as unknown as DatabaseService; + const broken = { deleteExpired: () => { throw new Error('db gone'); } } as unknown as IdempotencyKeysRepository; const job = new IdempotencyCleanupJob(broken, { isEnabled: () => true } as unknown as CronRegistrarService); await expect(job.tick()).resolves.toBeUndefined(); // Non-Error throws are stringified rather than crashing the catch itself. - const brokenString = { prepare: () => { throw 'db string'; } } as unknown as DatabaseService; + const brokenString = { deleteExpired: () => { throw 'db string'; } } as unknown as IdempotencyKeysRepository; const job2 = new IdempotencyCleanupJob(brokenString, { isEnabled: () => true } as unknown as CronRegistrarService); await expect(job2.tick()).resolves.toBeUndefined(); }); diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index f2f60b7e7..56fb0c35e 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -14,12 +14,10 @@ import { ADDON_IDS, MCP_GATED_ADDON_IDS } from '../../../src/addons'; // ── DB setup ────────────────────────────────────────────────────────────────── -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); +vi.mock('../../../src/db/database', async () => { + + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); const mock = { db, closeDb: () => {}, @@ -28,10 +26,10 @@ const { testDb, dbMock } = vi.hoisted(() => { canAccessTrip: () => null, isOwner: () => false, }; - return { testDb: db, dbMock: mock }; + return mock; }); -vi.mock('../../../src/db/database', () => dbMock); + vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', @@ -54,8 +52,7 @@ vi.mock('../../../src/demo/demo-reset', () => ({ saveBaseline: vi.fn(), })); -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; +import { db as testDb } from '../../../src/db/database'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createUserWithMfa, createAdmin, createInviteToken } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; @@ -131,7 +128,7 @@ let auditLogRepo: AuditLogRepository; beforeAll(async () => { webauthn = new WebauthnConfigService(await createTestAppSettingsRepo(dbs.connection)); permissions = new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)); - userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestShareTokensRepo(dbs.connection)); + userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestShareTokensRepo(dbs.connection)); auth = new AuthService( permissions, new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), webauthn, userCleanup, new MailerService(await createTestUsersRepo(dbs.connection), await createTestSettingsRepo(dbs.connection), await createTestAppSettingsRepo(dbs.connection)), new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(dbs.connection)), await createTestUnitOfWork(dbs.connection), await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestInviteTokensRepo(dbs.connection), await createTestMcpTokensRepo(dbs.connection), @@ -180,11 +177,6 @@ const checkVersion = () => svc.checkVersion(); const listAddons = () => svc.listAddons(); const updateAddon = (id: string, d: Parameters[1]) => svc.updateAddon(id, d); -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => { resetTestDb(testDb); }); @@ -703,7 +695,6 @@ describe('admin quirk fixes (post-fold)', () => { expect(row.username).toBe(user.username); }); - }); // ── What an admin password reset ends, and what an ordinary edit must not ───── diff --git a/server/tests/unit/nest/admin.version-notification.test.ts b/server/tests/unit/nest/admin.version-notification.test.ts index c282dc9ef..8d28f4632 100644 --- a/server/tests/unit/nest/admin.version-notification.test.ts +++ b/server/tests/unit/nest/admin.version-notification.test.ts @@ -6,19 +6,16 @@ * case, which would leak into the ADMIN-SVC-* suite. The notification path runs * for real against the temp db's notifications table. */ -import { runMigrations } from '../../../src/db/migrations'; -import { createTables } from '../../../src/db/schema'; import { __clearVersionCacheForTests } from '../../../src/nest/admin/admin.helpers'; import { createAdmin } from '../../helpers/factories'; import { resetTestDb } from '../../helpers/test-db'; import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); +vi.mock('../../../src/db/database', async () => { + + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); const mock = { db, closeDb: () => {}, @@ -27,10 +24,10 @@ const { testDb, dbMock } = vi.hoisted(() => { canAccessTrip: () => null, isOwner: () => false, }; - return { testDb: db, dbMock: mock }; + return mock; }); -vi.mock('../../../src/db/database', () => dbMock); + vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', @@ -41,6 +38,7 @@ vi.mock('../../../src/websocket', () => ({ broadcastToUser: vi.fn() })); vi.mock('../../../src/mcp', () => ({ revokeUserSessions: vi.fn(), invalidateMcpSessions: vi.fn() })); vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn(), revokeUserSessionsForClient: vi.fn() })); +import { db as testDb } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { createTestAddonsService } from '../../helpers/test-addons'; @@ -105,7 +103,7 @@ let svc: AdminService; beforeAll(async () => { webauthn = new WebauthnConfigService(await createTestAppSettingsRepo(dbs.connection)); permissions = new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)); - userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestShareTokensRepo(dbs.connection)); + userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestShareTokensRepo(dbs.connection)); auth = new AuthService( permissions, new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), webauthn, userCleanup, new MailerService(await createTestUsersRepo(dbs.connection), await createTestSettingsRepo(dbs.connection), await createTestAppSettingsRepo(dbs.connection)), new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(dbs.connection)), await createTestUnitOfWork(dbs.connection), await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestInviteTokensRepo(dbs.connection), await createTestMcpTokensRepo(dbs.connection), @@ -166,11 +164,6 @@ function getNotificationCount(): number { return (testDb.prepare('SELECT COUNT(*) as c FROM notifications').get() as { c: number }).c; } -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => { resetTestDb(testDb); __clearVersionCacheForTests(); diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index acd99acd3..911ca8b77 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -13,12 +13,10 @@ // vi.hoisted: build the real in-memory DB and the module mock before any import // --------------------------------------------------------------------------- -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); +vi.mock('../../../src/db/database', async () => { + + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); const mock = { db, closeDb: () => {}, @@ -32,10 +30,10 @@ const { testDb, dbMock } = vi.hoisted(() => { isOwner: (tripId: any, userId: number) => !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), }; - return { testDb: db, dbMock: mock }; + return mock; }); -vi.mock('../../../src/db/database', () => dbMock); + vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', @@ -60,9 +58,8 @@ vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn() // Imports (after mocks) // --------------------------------------------------------------------------- +import { db as testDb } from '../../../src/db/database'; import { describe, it, expect, beforeAll, beforeEach, afterEach, afterAll, vi } from 'vitest'; -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createAdmin, createInviteToken, createTrip, createPlace, createReservation } from '../../helpers/factories'; import { AuthService } from '../../../src/nest/auth/auth.service'; @@ -88,6 +85,7 @@ import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, + createTestTripMembersRepo, createTestMcpTokensRepo, createTestInviteTokensRepo, createTestOauthTokensRepo, @@ -119,7 +117,7 @@ beforeAll(async () => { new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), membershipStub, new WebauthnConfigService(await createTestAppSettingsRepo(testDb)), - new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestTripMembersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), mailerStub, new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)), await createTestUnitOfWork(testDb), @@ -133,11 +131,6 @@ beforeAll(async () => { // Lifecycle // --------------------------------------------------------------------------- -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => resetTestDb(testDb)); afterAll(() => testDb.close()); @@ -1078,7 +1071,6 @@ describe('ephemeral + demo helpers', () => { }); }); - // --------------------------------------------------------------------------- // Quirk fixes after the DI fold (trailing fix(server) commit): AUTH-DB-089+. // The relocation carried these verbatim; the fixes land on top with a pin each. diff --git a/server/tests/unit/nest/budget.service.db.test.ts b/server/tests/unit/nest/budget.service.db.test.ts index c86ea5374..61f949ca4 100644 --- a/server/tests/unit/nest/budget.service.db.test.ts +++ b/server/tests/unit/nest/budget.service.db.test.ts @@ -9,12 +9,10 @@ */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); +vi.mock('../../../src/db/database', async () => { + + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); const mock = { db, closeDb: () => {}, @@ -23,10 +21,10 @@ const { testDb, dbMock } = vi.hoisted(() => { canAccessTrip: () => null, isOwner: () => false, }; - return { testDb: db, dbMock: mock }; + return mock; }); -vi.mock('../../../src/db/database', () => dbMock); + vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', @@ -52,8 +50,7 @@ vi.mock('../../../src/nest/budget/exchange-rates.service', () => ({ }, })); -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; +import { db as testDb } from '../../../src/db/database'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, addTripMember } from '../../helpers/factories'; import { BudgetService } from '../../../src/nest/budget/budget.service'; @@ -80,8 +77,6 @@ import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourney import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; import { BudgetItemMembers } from '../../../src/db/entities/BudgetItemMembers.entity'; - - // Guest fixtures come from TripMembersService since the trip split (they were on // TripsService before, and on the deleted services/tripService before that); // deleteGuest routes through the SAME BudgetService domain SQL @@ -95,7 +90,6 @@ import { BudgetItemMembers } from '../../../src/db/entities/BudgetItemMembers.en let dbsEm: import('@mikro-orm/core').EntityManager | undefined; const dbs = () => new DatabaseService(testDb, dbsEm); - let budget: BudgetService; let membersSvc: TripMembersService; let createGuest: typeof membersSvc.createGuest; @@ -121,7 +115,7 @@ beforeAll(async () => { membersSvc = new TripMembersService( dbs(), budget, - new UserCleanupService(dbs(), budget, await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), + new UserCleanupService(dbs(), budget, await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestTripMembersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), @@ -134,11 +128,6 @@ beforeAll(async () => { deleteGuest = (...args) => membersSvc.deleteGuest(...args); }); -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => { resetTestDb(testDb); }); diff --git a/server/tests/unit/nest/day-notes.service.test.ts b/server/tests/unit/nest/day-notes.service.test.ts index e61e882d0..d6c9f6e88 100644 --- a/server/tests/unit/nest/day-notes.service.test.ts +++ b/server/tests/unit/nest/day-notes.service.test.ts @@ -51,11 +51,17 @@ import { PermissionsService } from '../../../src/nest/permissions/permissions.se import { DayNotesService } from '../../../src/nest/day-notes/day-notes.service'; import type { DayNote } from '../../../src/types'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; -import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService, createTestDayNotesRepo, createTestDaysRepo } from '../../helpers/test-uow'; let svc: DayNotesService; beforeAll(async () => { - svc = new DayNotesService(await createTestDatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService()); + svc = new DayNotesService( + await createTestDatabaseService(testDb), + new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), + new RealtimeService(), + await createTestDayNotesRepo(testDb), + await createTestDaysRepo(testDb), + ); }); beforeAll(() => { @@ -288,7 +294,7 @@ describe('DayNotesService.canEdit', () => { it('DAYNOTE-SVC-090 asks for day_edit and flags a non-owner as shared', async () => { const checkPermission = vi.fn(() => true); const permissions = { checkPermission } as unknown as PermissionsService; - const withStub = new DayNotesService(new DatabaseService(testDb), permissions, new RealtimeService()); + const withStub = new DayNotesService(new DatabaseService(testDb), permissions, new RealtimeService(), await createTestDayNotesRepo(testDb), await createTestDaysRepo(testDb)); const trip = { id: 1, user_id: 1 } as never; expect(await withStub.canEdit(trip, { id: 1, role: 'user' } as never)).toBe(true); diff --git a/server/tests/unit/nest/google-transit.provider.test.ts b/server/tests/unit/nest/google-transit.provider.test.ts index 42912cd4d..4d9b4ebea 100644 --- a/server/tests/unit/nest/google-transit.provider.test.ts +++ b/server/tests/unit/nest/google-transit.provider.test.ts @@ -16,7 +16,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { GoogleTransitProvider, clearGoogleTransitCache } from '../../../src/nest/transit/google-transit.provider'; import { decodePolyline, encodePolyline } from '../../../src/nest/transit/transit.helpers'; import { TransitService } from '../../../src/nest/transit/transit.service'; -import type { DatabaseService } from '../../../src/nest/database/database.service'; import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository'; import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; @@ -32,36 +31,25 @@ vi.mock('../../../src/app-config', async (importOriginal) => { const fetchMock = vi.fn(); -/** app_settings reads only — the transit provider name and the instance Places key. */ -function stubDb(settings: Record): DatabaseService { - return { - get: (_sql: string, ...params: unknown[]) => { - const value = settings[String(params[0])]; - return value === undefined ? undefined : { value }; - }, - run: () => undefined, - } as unknown as DatabaseService; -} - const GOOGLE_SETTINGS = { transit_provider: 'google', maps_api_key: 'test-key' }; /** - * resolveKey (google-transit.provider.ts) now reads AppSettingsRepository/ + * resolveKey (google-transit.provider.ts) reads AppSettingsRepository/ * UsersRepository directly (instance-api-keys.ts's resolveApiKey, Plan 3a - * Task 5) instead of a raw `this.database.get(...)` call through the stub - * above. `stubDb`'s settings object already carries exactly the instance-tier - * value every test here configures, so `appSettingsStub.getValue` wraps it - * directly; the per-user fallback tier was never exercised by this file even - * before this conversion (the old stub's `.get` matched on the bound - * parameter, and none of these cases seed a per-user row), so it always - * answers null. + * Task 5); isActive's own `readTransitProvider` call does too, since Plan 4 + * Task 1 — both go through the SAME `appSettingsStub` below now that + * `GoogleTransitProvider` no longer takes a `DatabaseService` at all. The + * settings object already carries exactly the instance-tier value every test + * here configures, so `appSettingsStub.getValue` wraps it directly; the + * per-user fallback tier was never exercised by this file (none of these + * cases seed a per-user row), so it always answers null. */ function makeProvider(settings: Record): GoogleTransitProvider { const appSettingsStub = { getValue: async (key: string) => settings[key] ?? null, } as unknown as AppSettingsRepository; const usersStub = { getApiKeyColumn: async () => null } as unknown as UsersRepository; - return new GoogleTransitProvider(stubDb(settings), appSettingsStub, usersStub); + return new GoogleTransitProvider(appSettingsStub, usersStub); } beforeEach(() => { diff --git a/server/tests/unit/nest/idempotency.interceptor.test.ts b/server/tests/unit/nest/idempotency.interceptor.test.ts index 78ce0fa50..f1631b6b8 100644 --- a/server/tests/unit/nest/idempotency.interceptor.test.ts +++ b/server/tests/unit/nest/idempotency.interceptor.test.ts @@ -1,9 +1,14 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; import { HttpException } from '@nestjs/common'; import type { CallHandler, ExecutionContext } from '@nestjs/common'; import { from, of, lastValueFrom } from 'rxjs'; +import { createSnapshotTestDb } from '../../helpers/db-mock'; +import { resetTestDb } from '../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; +import { createUser } from '../../helpers/factories'; +import { IdempotencyKeys } from '../../../src/db/entities/IdempotencyKeys.entity'; +import type { IdempotencyKeysRepository } from '../../../src/db/repositories/IdempotencyKeys.repository'; import { IdempotencyInterceptor } from '../../../src/nest/common/idempotency.interceptor'; -import type { DatabaseService } from '../../../src/nest/database/database.service'; type ReqShape = { method: string; @@ -34,51 +39,69 @@ function handler(result: unknown): CallHandler & { handle: ReturnType of(result)) }; } -function makeDb(overrides: Partial = {}): DatabaseService { - return { get: vi.fn(), run: vi.fn(), ...overrides } as unknown as DatabaseService; +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let idempotencyKeys: IdempotencyKeysRepository; +let userId: number; + +beforeAll(async () => { + t = await createTestOrm(testDb); + idempotencyKeys = t.repo(IdempotencyKeys); +}); +beforeEach(() => { + resetTestDb(testDb); + t.clear(); + const { user } = createUser(testDb); + userId = user.id; +}); +afterAll(async () => { await t.close(); testDb.close(); }); + +const findRow = (key: string, user: number, method: string, path: string) => + testDb.prepare('SELECT status_code, response_body FROM idempotency_keys WHERE key = ? AND user_id = ? AND method = ? AND path = ?').get(key, user, method, path); + +// A few `await Promise.resolve()` hops give the repository's async write +// chain (and its `.finally(release)`) the same room it gets under a real +// event loop, matching the timing this suite already needed before the +// conversion — `insertIfAbsent` is now a genuine repository call, not a +// synchronous `better-sqlite3` write wrapped in a resolved Promise. +async function settle(hops = 8): Promise { + for (let i = 0; i < hops; i++) await Promise.resolve(); } describe('IdempotencyInterceptor (parity with the legacy applyIdempotency middleware)', () => { it('passes a GET through without touching the store', async () => { - const db = makeDb(); const h = handler('weather'); const out = await lastValueFrom( - await new IdempotencyInterceptor(db).intercept(ctx({ method: 'GET', headers: {} }, makeRes()), h), + await new IdempotencyInterceptor(idempotencyKeys).intercept(ctx({ method: 'GET', headers: {} }, makeRes()), h), ); expect(out).toBe('weather'); expect(h.handle).toHaveBeenCalled(); - expect(db.get).not.toHaveBeenCalled(); }); it('passes a mutating request without a key through', async () => { - const db = makeDb(); const h = handler('done'); await lastValueFrom( - await new IdempotencyInterceptor(db).intercept(ctx({ method: 'POST', headers: {}, user: { id: 1 } }, makeRes()), h), + await new IdempotencyInterceptor(idempotencyKeys).intercept(ctx({ method: 'POST', headers: {}, user: { id: userId } }, makeRes()), h), ); expect(h.handle).toHaveBeenCalled(); - expect(db.get).not.toHaveBeenCalled(); }); it('passes through when there is no authenticated user', async () => { - const db = makeDb(); const h = handler('done'); await lastValueFrom( - await new IdempotencyInterceptor(db).intercept(ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' } }, makeRes()), h), + await new IdempotencyInterceptor(idempotencyKeys).intercept(ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' } }, makeRes()), h), ); expect(h.handle).toHaveBeenCalled(); - expect(db.get).not.toHaveBeenCalled(); }); it('rejects an over-long key with the exact legacy 400 body', async () => { - const db = makeDb(); const h = handler('done'); // `intercept` is async now, so the cap is reported as a rejection. Nest's // InterceptorsConsumer already awaited the interceptor's result, so a // synchronous throw reached it as a rejected promise too: same 400 body. const run = () => - new IdempotencyInterceptor(db).intercept( - ctx({ method: 'POST', headers: { 'x-idempotency-key': 'x'.repeat(129) }, user: { id: 1 } }, makeRes()), + new IdempotencyInterceptor(idempotencyKeys).intercept( + ctx({ method: 'POST', headers: { 'x-idempotency-key': 'x'.repeat(129) }, user: { id: userId } }, makeRes()), h, ); await expect(run()).rejects.toThrow(HttpException); @@ -93,119 +116,103 @@ describe('IdempotencyInterceptor (parity with the legacy applyIdempotency middle }); it('replays a cached response and skips the handler', async () => { - const db = makeDb({ get: vi.fn().mockReturnValue({ status_code: 201, response_body: '{"id":5}' }) }); + testDb.prepare('INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body) VALUES (?, ?, ?, ?, ?, ?)') + .run('k', userId, 'POST', '/api/categories', 201, '{"id":5}'); const res = makeRes(); const h = handler('should-not-run'); const out = await lastValueFrom( - await new IdempotencyInterceptor(db).intercept( - ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: 1 } }, res), + await new IdempotencyInterceptor(idempotencyKeys).intercept( + ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: userId } }, res), h, ), ); expect(res.status).toHaveBeenCalledWith(201); expect(out).toEqual({ id: 5 }); expect(h.handle).not.toHaveBeenCalled(); - expect(db.get).toHaveBeenCalledWith( - expect.stringContaining('idempotency_keys'), - 'k', 1, 'POST', '/api/categories', - ); }); it('captures a successful JSON response under the key', async () => { - const run = vi.fn(); - const db = makeDb({ get: vi.fn().mockReturnValue(undefined), run }); const res = makeRes(); const h = handler({ created: true }); await lastValueFrom( - await new IdempotencyInterceptor(db).intercept( - ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: 1 } }, res), + await new IdempotencyInterceptor(idempotencyKeys).intercept( + ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: userId } }, res), h, ), ); // Simulate Nest serialising the handler result through the wrapped res.json. res.statusCode = 201; res.json({ created: true }); - expect(run).toHaveBeenCalledTimes(1); - expect(run).toHaveBeenCalledWith( - expect.stringContaining('INSERT OR IGNORE INTO idempotency_keys'), - 'k', 1, 'POST', '/api/categories', 201, '{"created":true}', expect.any(Number), - ); + await settle(); + + expect(findRow('k', userId, 'POST', '/api/categories')).toEqual({ status_code: 201, response_body: '{"created":true}' }); }); it('does not cache a non-2xx response', async () => { - const run = vi.fn(); - const db = makeDb({ get: vi.fn().mockReturnValue(undefined), run }); const res = makeRes(); const h = handler({ error: 'bad' }); await lastValueFrom( - await new IdempotencyInterceptor(db).intercept( - ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: 1 } }, res), + await new IdempotencyInterceptor(idempotencyKeys).intercept( + ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: userId } }, res), h, ), ); res.statusCode = 400; res.json({ error: 'bad' }); - expect(run).not.toHaveBeenCalled(); + await settle(); + + expect(findRow('k', userId, 'POST', '/api/categories')).toBeUndefined(); }); it('does not cache a body that exceeds the 256 KiB cap', async () => { - const run = vi.fn(); - const db = makeDb({ get: vi.fn().mockReturnValue(undefined), run }); const res = makeRes(); const big = { blob: 'x'.repeat(300 * 1024) }; const h = handler(big); await lastValueFrom( - await new IdempotencyInterceptor(db).intercept( - ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: 1 } }, res), + await new IdempotencyInterceptor(idempotencyKeys).intercept( + ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: userId } }, res), h, ), ); res.statusCode = 200; res.json(big); - expect(run).not.toHaveBeenCalled(); + await settle(); + + expect(findRow('k', userId, 'POST', '/api/categories')).toBeUndefined(); }); it('swallows a storage failure so the response still succeeds', async () => { - const run = vi.fn(() => { - throw new Error('db is locked'); - }); - const db = makeDb({ get: vi.fn().mockReturnValue(undefined), run }); + const failing = { findResponse: vi.fn().mockResolvedValue(null), insertIfAbsent: vi.fn().mockRejectedValue(new Error('db is locked')) } as unknown as IdempotencyKeysRepository; const res = makeRes(); const h = handler({ ok: true }); await lastValueFrom( - await new IdempotencyInterceptor(db).intercept( - ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: 1 } }, res), + await new IdempotencyInterceptor(failing).intercept( + ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories', user: { id: userId } }, res), h, ), ); res.statusCode = 201; const returned = res.json({ ok: true }); - expect(run).toHaveBeenCalledTimes(1); + await settle(); + + expect(failing.insertIfAbsent).toHaveBeenCalledTimes(1); expect(returned).toEqual({ ok: true }); }); it('makes an overlapping replay of the same key wait, then answers it from the first response', async () => { // The row only appears once the first request answers, so without the // in-flight map both of these would miss the SELECT and both would run. - const rows = new Map(); - const db = makeDb({ - get: vi.fn((_sql: string, ...params: unknown[]) => rows.get(String(params[0]))) as unknown as DatabaseService['get'], - run: vi.fn((_sql: string, ...params: unknown[]) => { - rows.set(String(params[0]), { status_code: Number(params[4]), response_body: String(params[5]) }); - return {} as never; - }) as unknown as DatabaseService['run'], - }); - const interceptor = new IdempotencyInterceptor(db); + const interceptor = new IdempotencyInterceptor(idempotencyKeys); // The first handler is still running when the second request arrives. let finish!: (value: unknown) => void; const slow = { handle: vi.fn(() => from(new Promise((resolve) => { finish = resolve; }))) }; const firstRes = makeRes(); - const first = lastValueFrom(await interceptor.intercept(ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/places', user: { id: 1 } }, firstRes), slow)); + const first = lastValueFrom(await interceptor.intercept(ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/places', user: { id: userId } }, firstRes), slow)); const secondHandler = handler({ id: 'second' }); const secondRes = makeRes(); - const second = lastValueFrom(await interceptor.intercept(ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/places', user: { id: 1 } }, secondRes), secondHandler)); + const second = lastValueFrom(await interceptor.intercept(ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/places', user: { id: userId } }, secondRes), secondHandler)); // The order Nest uses, and the one that makes this test worth having: the // handler's observable completes FIRST, and the response - which is what @@ -214,10 +221,11 @@ describe('IdempotencyInterceptor (parity with the legacy applyIdempotency middle finish({ id: 'first' }); // Nest is several microtask hops from the completion to the write // (transformToResult -> lastValueFrom -> apply), so give the waiter the same - // room it gets in production to wake up too early. - for (let i = 0; i < 5; i++) await Promise.resolve(); + // room it gets in production to wake up too early — plus the repository + // write's own async chain (`insertIfAbsent`'s promise + its `.finally`). firstRes.statusCode = 201; firstRes.json({ id: 'first' }); + await settle(12); expect(await first).toEqual({ id: 'first' }); expect(await second).toEqual({ id: 'first' }); @@ -226,19 +234,18 @@ describe('IdempotencyInterceptor (parity with the legacy applyIdempotency middle }); it('runs the waiting request itself when the first one cached nothing', async () => { - const db = makeDb({ get: vi.fn().mockReturnValue(undefined), run: vi.fn() }); - const interceptor = new IdempotencyInterceptor(db); + const interceptor = new IdempotencyInterceptor(idempotencyKeys); let finish!: (value: unknown) => void; const slow = { handle: vi.fn(() => from(new Promise((resolve) => { finish = resolve; }))) }; const first = lastValueFrom(await interceptor.intercept( - ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/places', user: { id: 1 } }, makeRes()), + ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/places', user: { id: userId } }, makeRes()), slow, )); const secondHandler = handler({ id: 'second' }); const second = lastValueFrom(await interceptor.intercept( - ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/places', user: { id: 1 } }, makeRes()), + ctx({ method: 'POST', headers: { 'x-idempotency-key': 'k' }, path: '/api/places', user: { id: userId } }, makeRes()), secondHandler, )); @@ -251,16 +258,47 @@ describe('IdempotencyInterceptor (parity with the legacy applyIdempotency middle }); it('treats a PATCH as a mutating method', async () => { - const db = makeDb({ get: vi.fn().mockReturnValue(undefined), run: vi.fn() }); const res = makeRes(); const h = handler('done'); await lastValueFrom( - await new IdempotencyInterceptor(db).intercept( - ctx({ method: 'PATCH', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories/1', user: { id: 1 } }, res), + await new IdempotencyInterceptor(idempotencyKeys).intercept( + ctx({ method: 'PATCH', headers: { 'x-idempotency-key': 'k' }, path: '/api/categories/1', user: { id: userId } }, res), h, ), ); - expect(db.get).toHaveBeenCalled(); expect(h.handle).toHaveBeenCalled(); }); + + it('IDEMP-RACE-001 (rule 11 mutation proof): the in-flight slot is claimed BEFORE the first await, so a concurrent request for the same key never issues its own lookup', async () => { + // Regression guard for the exact bug rule 11 warns about: if + // `inFlight.set()` (this.claim) were moved back to AFTER `await + // this.lookup(...)`, the SECOND call below would run its synchronous + // prefix (which now includes NO claim yet) before the first call's + // lookup — gated open here by `gate` — resolves, and would issue its own + // `findResponse` call instead of finding `pending` and waiting. Holding + // the repository's lookup open with a controlled promise makes the race + // window deterministic instead of timing-dependent. + let resolveLookup!: () => void; + const gate = new Promise((resolve) => { resolveLookup = resolve; }); + const findResponse = vi.fn(async () => { await gate; return null; }); + const gatedRepo = { findResponse, insertIfAbsent: vi.fn().mockResolvedValue(undefined) } as unknown as IdempotencyKeysRepository; + const interceptor = new IdempotencyInterceptor(gatedRepo); + + const firstHandler = handler({ id: 'a' }); + const secondHandler = handler({ id: 'b' }); + const reqOpts = { method: 'POST', headers: { 'x-idempotency-key': 'race' }, path: '/api/places', user: { id: userId } }; + + // Fired back to back, synchronously — exactly how two nearly-simultaneous + // HTTP requests reach the interceptor. + const firstPromise = interceptor.intercept(ctx(reqOpts, makeRes()), firstHandler); + const secondPromise = interceptor.intercept(ctx(reqOpts, makeRes()), secondHandler); + + // Both `intercept()` calls have now run their full synchronous prefix + // (each up to its own first genuine await) while `gate` is still open. + await Promise.resolve(); + expect(findResponse).toHaveBeenCalledTimes(1); + + resolveLookup(); + await Promise.all([firstPromise, secondPromise]); + }); }); diff --git a/server/tests/unit/nest/llm-parse/llm-config.resolver.test.ts b/server/tests/unit/nest/llm-parse/llm-config.resolver.test.ts index 68c0e1b58..3c8f53b26 100644 --- a/server/tests/unit/nest/llm-parse/llm-config.resolver.test.ts +++ b/server/tests/unit/nest/llm-parse/llm-config.resolver.test.ts @@ -1,57 +1,35 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -// prepare() has to answer per statement now: the resolver reads the addon row -// AND the caller's role, and a single shared stub would hand the role lookup the -// addon row (silently green tests for the #1772 gate). -const { dbMock } = vi.hoisted(() => { - const addonStmt = { get: vi.fn() }; - const roleStmt = { get: vi.fn() }; - const other = { get: vi.fn(), all: vi.fn(), run: vi.fn() }; - return { - dbMock: { - prepare: vi.fn((sql: string) => { - if (sql.includes('FROM addons')) return addonStmt; - if (sql.includes('FROM users')) return roleStmt; - return other; - }), - _addon: addonStmt, - _role: roleStmt, - }, - }; -}); -vi.mock('../../../../src/db/database', () => ({ db: dbMock, closeDb: () => {}, reinitialize: () => {} })); - const isAddonEnabled = vi.fn(); +const findById = vi.fn(); import { LlmConfigResolver } from '../../../../src/nest/llm-parse/llm-config.resolver'; -import { DatabaseService } from '../../../../src/nest/database/database.service'; +import type { AddonsRepository } from '../../../../src/db/repositories/Addons.repository'; import type { SettingsService } from '../../../../src/nest/settings/settings.service'; import type { AddonsService } from '../../../../src/nest/addons/addons.service'; -// The resolver injects SettingsService — a stub instance instead of the old -// legacy-module path mock (same behaviors as before the DI move). The -// DatabaseService rides the same prepare/get mock the module-level db used. +// The resolver injects SettingsService — a stub instance (same behaviors as +// before the DI move). Plan 4 Task 1: the addon-row read is now +// `AddonsRepository.findById`, stubbed the same way — `config` comes back +// already-parsed (the repository's own `p.json()` column), never a JSON +// string this test has to stringify. const getUserSettings = vi.fn(() => ({}) as Record); const getAdminUserDefaults = vi.fn(() => ({}) as Record); const getDecryptedUserSetting = vi.fn(() => null as string | null); const settingsStub = { getUserSettings, getAdminUserDefaults, getDecryptedUserSetting } as unknown as SettingsService; const addonsStub = { isAddonEnabled } as unknown as AddonsService; -const resolver = new LlmConfigResolver(settingsStub, new DatabaseService(dbMock as never), addonsStub); +const addonsRepoStub = { findById } as unknown as AddonsRepository; +const resolver = new LlmConfigResolver(settingsStub, addonsRepoStub, addonsStub); -function setInstanceConfig(config: unknown) { - dbMock._addon.get.mockReturnValue(config === undefined ? undefined : { config: JSON.stringify(config) }); -} - -function setRole(role: 'user' | 'admin' | undefined) { - dbMock._role.get.mockReturnValue(role === undefined ? undefined : { role }); +function setInstanceConfig(config: Record | undefined) { + findById.mockResolvedValue(config === undefined ? null : { id: 'llm_parsing', config }); } beforeEach(() => { vi.clearAllMocks(); isAddonEnabled.mockReturnValue(true); setInstanceConfig(undefined); - setRole('user'); getUserSettings.mockReturnValue({}); getAdminUserDefaults.mockReturnValue({}); getDecryptedUserSetting.mockReturnValue(null); @@ -61,6 +39,8 @@ describe('resolveLlmConfig', () => { it('returns null when the addon is disabled', async () => { isAddonEnabled.mockReturnValue(false); expect(await resolver.resolve(1)).toBeNull(); + // Gated BEFORE the addon-row read: a disabled addon never even queries it. + expect(findById).not.toHaveBeenCalled(); }); it('uses instance config when present (and decrypts the key)', async () => { @@ -72,11 +52,11 @@ describe('resolveLlmConfig', () => { apiKey: 'sk-plain', multimodal: true, }); + expect(findById).toHaveBeenCalledWith('llm_parsing'); }); it('instance config with a base URL still wins for a plain user (#1772 does not touch it)', async () => { setInstanceConfig({ provider: 'local', model: 'nuextract', baseUrl: 'http://ollama:11434' }); - setRole('user'); expect(await resolver.resolve(7)).toMatchObject({ provider: 'local', baseUrl: 'http://ollama:11434' }); }); @@ -99,6 +79,11 @@ describe('resolveLlmConfig', () => { expect(await resolver.resolve(1)).toBeNull(); }); + it('returns null when the addon row has no config at all', async () => { + setInstanceConfig(undefined); + expect(await resolver.resolve(1)).toBeNull(); + }); + // #1772: the endpoint is instance configuration, so it may only come from an // admin-controlled source, whoever is asking. it('#1772: picking local personally gets no config at all (no silent reroute)', async () => { @@ -126,13 +111,14 @@ describe('resolveLlmConfig', () => { expect(await resolver.resolve(7)).toMatchObject({ provider: 'local', baseUrl: 'http://ollama.internal:11434' }); }); - it('#1772: the caller\'s role does not change the answer, and no role is looked up', async () => { + it('#1772: the caller\'s identity does not change the answer — the resolver never reads anything keyed by it besides the settings service', async () => { // An instance has one endpoint. An admin who parked one in their own row is - // in exactly the same position as anyone else, and the resolver no longer - // reads the users table at all. - setRole('admin'); + // in exactly the same position as anyone else. getUserSettings.mockReturnValue({ llm_provider: 'local', llm_model: 'nuextract', llm_base_url: 'http://192.168.1.5:11434' }); expect(await resolver.resolve(7)).toBeNull(); - expect(dbMock._role.get).not.toHaveBeenCalled(); + // The addon-row read is a single, userId-independent lookup — called once, + // with no user-scoped argument. + expect(findById).toHaveBeenCalledTimes(1); + expect(findById).toHaveBeenCalledWith('llm_parsing'); }); }); diff --git a/server/tests/unit/nest/mcp-shared.service.test.ts b/server/tests/unit/nest/mcp-shared.service.test.ts index 03b96968e..ac509685e 100644 --- a/server/tests/unit/nest/mcp-shared.service.test.ts +++ b/server/tests/unit/nest/mcp-shared.service.test.ts @@ -6,31 +6,35 @@ */ import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest'; -const { testDb, dbMock, broadcastMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - return { testDb: db, dbMock: { db, closeDb: () => {}, reinitialize: () => {} }, broadcastMock: vi.fn() }; +vi.mock('../../../src/db/database', async () => { + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); + return { db, closeDb: () => {}, reinitialize: () => {} }; }); - -vi.mock('../../../src/db/database', () => dbMock); +const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; +import { db as testDb } from '../../../src/db/database'; + import { createUser } from '../../helpers/factories'; import { McpToolGuardsService } from '../../../src/nest/mcp-shared/mcp-tool-guards.service'; import { McpSharedModule } from '../../../src/nest/mcp-shared/mcp-shared.module'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; -import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo, createTestTripsRepo, createTestUsersRepo } from '../../helpers/test-uow'; const dbs = new DatabaseService(testDb); let svc: McpToolGuardsService; beforeAll(async () => { - svc = new McpToolGuardsService(dbs, new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)), new RealtimeService()); + // Plan 4 Task 1 constructor-ripple: the trip `user_id` and user `role` + // reads moved off `DatabaseService` onto `TripsRepository`/`UsersRepository`. + svc = new McpToolGuardsService( + await createTestTripsRepo(testDb), + await createTestUsersRepo(testDb), + new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)), + new RealtimeService(), + ); }); function createTrip(ownerId: number): number { @@ -38,11 +42,6 @@ function createTrip(ownerId: number): number { return Number(r.lastInsertRowid); } -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => { vi.clearAllMocks(); testDb.prepare('DELETE FROM trip_members').run(); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index da95e5c5b..67c797899 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -15,12 +15,10 @@ import jwtLib from 'jsonwebtoken'; // ── DB setup ────────────────────────────────────────────────────────────────── -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); +vi.mock('../../../src/db/database', async () => { + + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); const mock = { db, closeDb: () => {}, @@ -35,10 +33,10 @@ const { testDb, dbMock } = vi.hoisted(() => { isOwner: (tripId: any, userId: number) => !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), }; - return { testDb: db, dbMock: mock }; + return mock; }); -vi.mock('../../../src/db/database', () => dbMock); + vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', @@ -75,7 +73,6 @@ vi.mock('../../../src/utils/ssrfGuard', async (importOriginal) => { }; }); - const { getAppUrlMock } = vi.hoisted(() => ({ getAppUrlMock: vi.fn() })); vi.mock('../../../src/app-config', async (importOriginal) => { const actual = await importOriginal(); @@ -88,9 +85,8 @@ vi.mock('../../../src/nest/common/cookie', async (importOriginal) => { return { ...actual, setAuthCookie: setAuthCookieMock }; }); +import { db as testDb } from '../../../src/db/database'; import type { Request, Response } from 'express'; -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; @@ -142,7 +138,7 @@ beforeAll(async () => { new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), membership, new WebauthnConfigService(await createTestAppSettingsRepo(testDb)), - new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestTripMembersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), mailerStub, new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)), await createTestUnitOfWork(testDb), @@ -161,11 +157,6 @@ const MOCK_CONFIG = { discoveryUrl: null, }; -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => { resetTestDb(testDb); delete process.env.OIDC_ADMIN_VALUE; diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index 7d6ec3ba7..aa472a987 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -17,12 +17,10 @@ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vites // ── DB setup ────────────────────────────────────────────────────────────────── -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); +vi.mock('../../../src/db/database', async () => { + + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); // Passkeys are trip-free, so the trip-access helpers are inert stubs. const mock = { db, @@ -32,10 +30,10 @@ const { testDb, dbMock } = vi.hoisted(() => { canAccessTrip: () => undefined, isOwner: () => false, }; - return { testDb: db, dbMock: mock }; + return mock; }); -vi.mock('../../../src/db/database', () => dbMock); + vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', @@ -71,9 +69,8 @@ const { swMock } = vi.hoisted(() => ({ })); vi.mock('@simplewebauthn/server', () => swMock); +import { db as testDb } from '../../../src/db/database'; import jwtLib from 'jsonwebtoken'; -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; @@ -135,7 +132,7 @@ beforeAll(async () => { new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new TripMembershipService(await createTestTripsRepo(testDb), await createTestTripMembersRepo(testDb)), new WebauthnConfigService(await createTestAppSettingsRepo(testDb)), - new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestTripMembersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), mailerStub, new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)), await createTestUnitOfWork(testDb), @@ -158,11 +155,6 @@ const CFG = { rpID: 'trek.example.com', rpName: 'TREK', origins: ['https://trek. const LOCALHOST_CFG = { rpID: 'localhost', rpName: 'TREK', origins: ['http://localhost:5173', 'http://localhost:3001'], explicitOrigins: false }; const NOT_CONFIGURED_ERROR = 'Passkey login is not configured for this server.'; -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => { vi.clearAllMocks(); resolveWebauthnConfigMock.mockReturnValue(CFG); diff --git a/server/tests/unit/nest/platform.controller.test.ts b/server/tests/unit/nest/platform.controller.test.ts index 6df472398..b98f8eed0 100644 --- a/server/tests/unit/nest/platform.controller.test.ts +++ b/server/tests/unit/nest/platform.controller.test.ts @@ -202,19 +202,24 @@ describe('applyPlatformUploads', () => { expect(res.statusCode).toBe(401); }); - // R1/R5 (Plan 3h Task 6): the share-token half of this handler now reads - // through `ShareTokensRepository.findTripIdByToken` — `orm.em.getRepository - // (ShareTokens)` inside the SAME `withRequestContext` wrap `applyPlatformUploads` - // already uses for the JWT half — so these cases seed REAL rows in - // `uploadsTestDb` (bound to `uploadsOrm.orm`) rather than mocking - // `db.prepare`'s return value the way the (still-unconverted) sibling - // `photos` read below continues to. `photoStmt` stays a `db.prepare` mock - // (untouched domain) throughout. + // R1/R5 (Plan 3h Task 6, R1 (Plan 4 Task 1)): both the share-token lookup + // AND the sibling `photos` read now go through the SAME real ORM + // (`ShareTokensRepository.findTripIdByToken` / `PhotosRepository + // .findTripIdByFilename`, `orm.em.getRepository(...)` inside the SAME + // `withRequestContext` wrap `applyPlatformUploads` already uses) — so + // every case here seeds REAL rows in `uploadsTestDb` (bound to + // `uploadsOrm.orm`) rather than mocking `db.prepare`'s return value; the + // `h.dbPrepare` stub is dead for this whole describe block now. function insertShareToken(tripId: number, userId: number, token: string, expiresAt: string | null = null) { uploadsTestDb.prepare('INSERT INTO share_tokens (trip_id, token, created_by, expires_at) VALUES (?, ?, ?, ?)') .run(tripId, token, userId, expiresAt); } + function insertPhoto(tripId: number, filename: string) { + uploadsTestDb.prepare('INSERT INTO photos (trip_id, filename, original_name) VALUES (?, ?, ?)') + .run(tripId, filename, filename); + } + it('401 when a share token does not cover the photo trip', async () => { const { user } = createUser(uploadsTestDb); const photoTrip = createTrip(uploadsTestDb, user.id); @@ -222,9 +227,9 @@ describe('applyPlatformUploads', () => { insertShareToken(otherTrip.id, user.id, 'share-mismatch'); h.exists.mockResolvedValue(true); h.verifyJwtAndLoadUser.mockReturnValue(null); - h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + insertPhoto(photoTrip.id, 'photo-mismatch.jpg'); const res = makeRes(); - await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-mismatch' } }, res, next); + await photoHandler()({ params: { filename: 'photo-mismatch.jpg' }, headers: {}, query: { token: 'share-mismatch' } }, res, next); expect(res.statusCode).toBe(401); }); @@ -233,9 +238,9 @@ describe('applyPlatformUploads', () => { const photoTrip = createTrip(uploadsTestDb, user.id); h.exists.mockResolvedValue(true); h.verifyJwtAndLoadUser.mockReturnValue(null); - h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + insertPhoto(photoTrip.id, 'photo-unknown-token.jpg'); const res = makeRes(); - await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'never-issued' } }, res, next); + await photoHandler()({ params: { filename: 'photo-unknown-token.jpg' }, headers: {}, query: { token: 'never-issued' } }, res, next); expect(res.statusCode).toBe(401); }); @@ -246,9 +251,9 @@ describe('applyPlatformUploads', () => { uploadsTestDb.prepare('DELETE FROM share_tokens WHERE token = ?').run('share-revoked'); h.exists.mockResolvedValue(true); h.verifyJwtAndLoadUser.mockReturnValue(null); - h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + insertPhoto(photoTrip.id, 'photo-revoked.jpg'); const res = makeRes(); - await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-revoked' } }, res, next); + await photoHandler()({ params: { filename: 'photo-revoked.jpg' }, headers: {}, query: { token: 'share-revoked' } }, res, next); expect(res.statusCode).toBe(401); }); @@ -258,9 +263,9 @@ describe('applyPlatformUploads', () => { insertShareToken(photoTrip.id, user.id, 'share-expired', '2020-01-01 00:00:00'); h.exists.mockResolvedValue(true); h.verifyJwtAndLoadUser.mockReturnValue(null); - h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + insertPhoto(photoTrip.id, 'photo-expired.jpg'); const res = makeRes(); - await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-expired' } }, res, next); + await photoHandler()({ params: { filename: 'photo-expired.jpg' }, headers: {}, query: { token: 'share-expired' } }, res, next); expect(res.statusCode).toBe(401); }); @@ -270,9 +275,9 @@ describe('applyPlatformUploads', () => { insertShareToken(photoTrip.id, user.id, 'share-CaseSensitive'); h.exists.mockResolvedValue(true); h.verifyJwtAndLoadUser.mockReturnValue(null); - h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + insertPhoto(photoTrip.id, 'photo-case.jpg'); const res = makeRes(); - await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-casesensitive' } }, res, next); + await photoHandler()({ params: { filename: 'photo-case.jpg' }, headers: {}, query: { token: 'share-casesensitive' } }, res, next); expect(res.statusCode).toBe(401); }); @@ -282,9 +287,9 @@ describe('applyPlatformUploads', () => { insertShareToken(photoTrip.id, user.id, 'share-nul'); h.exists.mockResolvedValue(true); h.verifyJwtAndLoadUser.mockReturnValue(null); - h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + insertPhoto(photoTrip.id, 'photo-nul.jpg'); const res = makeRes(); - await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-nul\0extra' } }, res, next); + await photoHandler()({ params: { filename: 'photo-nul.jpg' }, headers: {}, query: { token: 'share-nul\0extra' } }, res, next); expect(res.statusCode).toBe(401); }); @@ -295,14 +300,14 @@ describe('applyPlatformUploads', () => { h.exists.mockResolvedValue(true); h.sendToResponse.mockResolvedValue(undefined); h.verifyJwtAndLoadUser.mockReturnValue(null); - h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + insertPhoto(photoTrip.id, 'photo-valid.jpg'); const res = makeRes(); await photoHandler()( - { params: { filename: 'a.jpg' }, headers: { authorization: 'Bearer share-valid' }, query: {} }, + { params: { filename: 'photo-valid.jpg' }, headers: { authorization: 'Bearer share-valid' }, query: {} }, res, next, ); - expect(h.sendToResponse).toHaveBeenCalledWith('photos', 'a.jpg', res); + expect(h.sendToResponse).toHaveBeenCalledWith('photos', 'photo-valid.jpg', res); }); it('404 when the object vanishes between the exists check and the send', async () => { diff --git a/server/tests/unit/nest/realtime.gateway.test.ts b/server/tests/unit/nest/realtime.gateway.test.ts index 34f974400..76f46de7b 100644 --- a/server/tests/unit/nest/realtime.gateway.test.ts +++ b/server/tests/unit/nest/realtime.gateway.test.ts @@ -39,6 +39,8 @@ import { emitPluginEvent } from '../../../src/plugin-event-sink'; import type { DatabaseService } from '../../../src/nest/database/database.service'; import type { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; import type { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service'; +import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; +import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository'; import type { User } from '../../../src/types'; interface FakeSocket extends TrekWebSocket { @@ -64,10 +66,19 @@ function socket(): FakeSocket { const rows = new Map(); const db = { - get: (sql: string) => (sql.includes('app_settings') ? rows.get('mfa') : rows.get('user')), canAccessTrip: (tripId: number) => tripId === 7, } as unknown as DatabaseService; +const users = { + findForWsHandshake: vi.fn(async () => rows.get('user') ?? null), +} as unknown as UsersRepository; + +const appSettings = { + getValue: vi.fn(async (key: string) => + key === 'require_mfa' ? ((rows.get('mfa') as { value: string } | undefined)?.value ?? null) : null, + ), +} as unknown as AppSettingsRepository; + const consumeWithMeta = vi.fn(); const tokens = { consumeWithMeta } as unknown as EphemeralTokenService; @@ -76,7 +87,7 @@ const canAccessJourney = vi.fn((journeyId: number) => (journeyId === 4 ? null : const journeys = { canAccessJourney } as unknown as JourneyDomainService; async function connect(url: string) { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const ws = socket(); await gw.handleConnection(ws, { url } as never); return { gw, ws }; @@ -137,7 +148,7 @@ describe('RealtimeGateway handshake', () => { }); it('WSGW-007b: survives an upgrade request with no url, rather than throwing at it', async () => { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const ws = socket(); await expect(gw.handleConnection(ws, {} as never)).resolves.not.toThrow(); expect(ws.closedWith).toEqual([4001, 'Authentication required']); @@ -151,6 +162,12 @@ describe('RealtimeGateway handshake', () => { expect((await connect('/ws?token=x')).ws.closedWith).toBeNull(); }); + it('WSGW-007d: reads the user row and the require_mfa setting through the repositories, by the right keys (Plan 4 Task 1)', async () => { + await connect('/ws?token=x'); + expect(users.findForWsHandshake).toHaveBeenCalledWith(3); + expect(appSettings.getValue).toHaveBeenCalledWith('require_mfa'); + }); + it('WSGW-008: never leaks password_version past the handshake', async () => { // Asserted on what the socket registry HOLDS, not on the frames it sends. // The frames never carry the user object at all, so the earlier version of @@ -168,11 +185,10 @@ describe('RealtimeGateway handshake', () => { // Anything thrown inside the try — here, the user-row lookup blowing up — // must still answer the callback rather than escape as an uncaught // exception (recipe R1.5's catch site). - const throwing = { - get: () => { throw new Error('db exploded'); }, - canAccessTrip: (tripId: number) => tripId === 7, - } as unknown as DatabaseService; - const gw = new RealtimeGateway(throwing, tokens, journeys); + const throwingUsers = { + findForWsHandshake: () => { throw new Error('db exploded'); }, + } as unknown as UsersRepository; + const gw = new RealtimeGateway(db, tokens, journeys, throwingUsers, appSettings); const ws = socket(); await gw.handleConnection(ws, { url: '/ws?token=x' } as never); @@ -182,11 +198,10 @@ describe('RealtimeGateway handshake', () => { }); it('WSGW-009b: a non-Error thrown during the handshake is stringified rather than crashing the catch itself', async () => { - const throwing = { - get: () => { throw 'db string'; }, // non-Error throw, pinning the ternary's String(err) fallback - canAccessTrip: (tripId: number) => tripId === 7, - } as unknown as DatabaseService; - const gw = new RealtimeGateway(throwing, tokens, journeys); + const throwingUsers = { + findForWsHandshake: () => { throw 'db string'; }, // non-Error throw, pinning the ternary's String(err) fallback + } as unknown as UsersRepository; + const gw = new RealtimeGateway(db, tokens, journeys, throwingUsers, appSettings); const ws = socket(); await gw.handleConnection(ws, { url: '/ws?token=x' } as never); @@ -223,7 +238,7 @@ describe('RealtimeGateway heartbeat', () => { alive.isAlive = true; const stale = socket(); stale.isAlive = false; - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); gw.afterInit({ clients: new Set([alive, stale]) } as never); vi.advanceTimersByTime(30_000); @@ -244,7 +259,7 @@ describe('RealtimeGateway heartbeat', () => { try { const ws = socket(); ws.isAlive = true; - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); gw.afterInit({ clients: new Set([ws]) } as never); gw.onModuleDestroy(); @@ -353,7 +368,7 @@ describe('book rooms', () => { let nextJourney = 100; async function joined(journeyId: number) { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const ws = socket(); registerSocket(ws, { id: 3, username: 'm' } as User); const reply = await gw.handleBookJoin({ journeyId }, ws); @@ -369,7 +384,7 @@ describe('book rooms', () => { /* Same shape as the trip room's refusal, and for the same reason. */ it('WSGW-BOOK-002: refuses a journey the user cannot see, and adds nobody', async () => { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const ws = socket(); registerSocket(ws, { id: 3, username: 'm' } as User); @@ -420,7 +435,7 @@ describe('book pointers', () => { async function pair() { const journeyId = nextJourney++; - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const mine = socket(); const theirs = socket(); registerSocket(mine, { id: 3, username: 'm' } as User); @@ -485,7 +500,7 @@ describe('book messages that are refused', () => { let nextJourney = 300; it('WSGW-BOOK-006: a join without a journey id is ignored', async () => { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const ws = socket(); registerSocket(ws, { id: 3, username: 'm' } as User); @@ -495,7 +510,7 @@ describe('book messages that are refused', () => { /* An unauthenticated socket never got as far as the registry. */ it('WSGW-BOOK-007: a join from a socket with no user is ignored', async () => { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const j = nextJourney++; expect(await gw.handleBookJoin({ journeyId: j }, socket())).toBeUndefined(); @@ -503,7 +518,7 @@ describe('book messages that are refused', () => { }); it('WSGW-BOOK-008: a journey id that is not a number is refused, not joined', async () => { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const ws = socket(); registerSocket(ws, { id: 3, username: 'm' } as User); @@ -512,7 +527,7 @@ describe('book messages that are refused', () => { }); it('WSGW-BOOK-009: a leave without a journey id is ignored', () => { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const ws = socket(); registerSocket(ws, { id: 3, username: 'm' } as User); @@ -521,7 +536,7 @@ describe('book messages that are refused', () => { /* Leaving a room nobody is in must not create one on the way out. */ it('WSGW-BOOK-010: leaving a book that was never joined does nothing', () => { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const ws = socket(); registerSocket(ws, { id: 3, username: 'm' } as User); const j = nextJourney++; @@ -531,7 +546,7 @@ describe('book messages that are refused', () => { }); it('WSGW-CUR-005: a pointer without a journey id reaches nobody', async () => { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const mine = socket(); const theirs = socket(); registerSocket(mine, { id: 3, username: 'm' } as User); @@ -546,7 +561,7 @@ describe('book messages that are refused', () => { }); it('WSGW-CUR-006: a pointer from a socket with no user reaches nobody', async () => { - const gw = new RealtimeGateway(db, tokens, journeys); + const gw = new RealtimeGateway(db, tokens, journeys, users, appSettings); const theirs = socket(); registerSocket(theirs, { id: 4, username: 'o' } as User); const j = nextJourney++; diff --git a/server/tests/unit/nest/registration-invites.service.test.ts b/server/tests/unit/nest/registration-invites.service.test.ts index e5a1ca720..8d89014c6 100644 --- a/server/tests/unit/nest/registration-invites.service.test.ts +++ b/server/tests/unit/nest/registration-invites.service.test.ts @@ -25,15 +25,14 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createAdmin, createTrip, createInviteToken } from '../../helpers/factories'; import { RegistrationInvitesService } from '../../../src/nest/auth/registration-invites.service'; -import { DatabaseService } from '../../../src/nest/database/database.service'; -import { createTestInviteTokensRepo } from '../../helpers/test-uow'; +import { createTestInviteTokensRepo, createTestTripsRepo } from '../../helpers/test-uow'; let svc: RegistrationInvitesService; beforeAll(async () => { createTables(testDb); runMigrations(testDb); - svc = new RegistrationInvitesService(new DatabaseService(testDb), await createTestInviteTokensRepo(testDb)); + svc = new RegistrationInvitesService(await createTestInviteTokensRepo(testDb), await createTestTripsRepo(testDb)); }); beforeEach(() => { resetTestDb(testDb); vi.clearAllMocks(); }); afterAll(() => { testDb.close(); }); diff --git a/server/tests/unit/nest/transit.service.test.ts b/server/tests/unit/nest/transit.service.test.ts index e227fc0ed..266d65dd6 100644 --- a/server/tests/unit/nest/transit.service.test.ts +++ b/server/tests/unit/nest/transit.service.test.ts @@ -12,7 +12,6 @@ import { deriveTransitStats, type TransitLeg } from '../../../src/nest/transit/transit.helpers'; import { GoogleTransitProvider } from '../../../src/nest/transit/google-transit.provider'; import { TransitService } from '../../../src/nest/transit/transit.service'; -import type { DatabaseService } from '../../../src/nest/database/database.service'; import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository'; import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; @@ -27,10 +26,9 @@ vi.mock('../../../src/nest/maps/maps.helpers', () => ({ buildUserAgent: () => 'T const fetchMock = vi.fn(); // No `transit_provider` row means Transitous, so every case below keeps // exercising the MOTIS path — the Google branch has its own suite. -const db = { get: () => undefined, run: () => undefined } as unknown as DatabaseService; const noAppSettings = { getValue: async () => null } as unknown as AppSettingsRepository; const noUsers = { getApiKeyColumn: async () => null } as unknown as UsersRepository; -const svc = new TransitService(new GoogleTransitProvider(db, noAppSettings, noUsers)); +const svc = new TransitService(new GoogleTransitProvider(noAppSettings, noUsers)); beforeEach(() => { vi.stubGlobal('fetch', fetchMock); diff --git a/server/tests/unit/nest/trip-invite.service.test.ts b/server/tests/unit/nest/trip-invite.service.test.ts index 146830b06..788d14eb3 100644 --- a/server/tests/unit/nest/trip-invite.service.test.ts +++ b/server/tests/unit/nest/trip-invite.service.test.ts @@ -40,7 +40,7 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { TripInviteService } from '../../../src/nest/trip-invite/trip-invite.service'; -import { createTestUnitOfWork, createTestAppSettingsRepo, createTestTripsRepo, createTestTripMembersRepo } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo, createTestTripsRepo, createTestTripMembersRepo, createTestTripInviteTokensRepo } from '../../helpers/test-uow'; // One DatabaseService over the shared in-memory handle, so every collaborator // reads and writes the same rows. @@ -48,7 +48,13 @@ const dbs = new DatabaseService(testDb); let svc: TripInviteService; beforeAll(async () => { const uow = await createTestUnitOfWork(dbs.connection); - svc = new TripInviteService(dbs, new PermissionsService(await createTestAppSettingsRepo(dbs.connection), uow), new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), uow); + svc = new TripInviteService( + dbs, + new PermissionsService(await createTestAppSettingsRepo(dbs.connection), uow), + new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), + uow, + await createTestTripInviteTokensRepo(dbs.connection), + ); }); beforeAll(() => { createTables(testDb); runMigrations(testDb); }); diff --git a/server/tests/unit/nest/trip-members.service.test.ts b/server/tests/unit/nest/trip-members.service.test.ts index f2ec2d27a..1e8ada83a 100644 --- a/server/tests/unit/nest/trip-members.service.test.ts +++ b/server/tests/unit/nest/trip-members.service.test.ts @@ -15,13 +15,15 @@ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vites // ── DB setup ────────────────────────────────────────────────────────────────── -const { testDb, dbMock, broadcast, notifySend } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); - const mock = { +const { broadcast, notifySend } = vi.hoisted(() => ({ + broadcast: vi.fn(), + notifySend: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('../../../src/db/database', async () => { + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); + return { db, closeDb: () => {}, reinitialize: () => {}, @@ -35,15 +37,9 @@ const { testDb, dbMock, broadcast, notifySend } = vi.hoisted(() => { isOwner: (tripId: any, userId: number) => !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), }; - return { - testDb: db, - dbMock: mock, - broadcast: vi.fn(), - notifySend: vi.fn().mockResolvedValue(undefined), - }; }); -vi.mock('../../../src/db/database', () => dbMock); +import { db as testDb } from '../../../src/db/database'; vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', @@ -53,8 +49,6 @@ vi.mock('../../../src/websocket', () => ({ broadcast })); // notifyInvite reaches the bridge through a dynamic import — keep the send in scope // but out of the transports. -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, addTripMember } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; @@ -105,18 +99,13 @@ beforeAll(async () => { budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection), ...(await budgetRepoArgs(dbs().connection))); roster = new TripMembersService( dbs(), budgetSvc, - new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), usersRepo, await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestShareTokensRepo(dbs().connection)), + new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), usersRepo, await createTestTripMembersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestShareTokensRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(notifySend), await createTestUnitOfWork(dbs().connection), tripsRepo, tripMembersRepo, usersRepo, ); }); -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => { resetTestDb(testDb); broadcast.mockClear(); diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts index fa8882a0b..6e97e39e8 100644 --- a/server/tests/unit/nest/trip-read-model.service.test.ts +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -12,12 +12,10 @@ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vites // ── DB setup ────────────────────────────────────────────────────────────────── -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); +vi.mock('../../../src/db/database', async () => { + + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); const mock = { db, closeDb: () => {}, @@ -32,10 +30,10 @@ const { testDb, dbMock } = vi.hoisted(() => { isOwner: (tripId: any, userId: number) => !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), }; - return { testDb: db, dbMock: mock }; + return mock; }); -vi.mock('../../../src/db/database', () => dbMock); + vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', @@ -44,8 +42,7 @@ vi.mock('../../../src/config', () => ({ const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast })); -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; +import { db as testDb } from '../../../src/db/database'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, addTripMember } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; @@ -177,7 +174,7 @@ beforeAll(async () => { await createTestBudgetItemsRepo(dbs().connection), await createTestCollectionPlacesRepo(dbs().connection), ); - membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestShareTokensRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); + membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestShareTokensRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); }); const buildReadModel = async (tripsRepo: TripsRepository, roster: TripMembersService = membersSvc) => @@ -225,11 +222,6 @@ beforeAll(async () => { svc = await buildReadModel(tripsRepo); }); -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => { resetTestDb(testDb); }); diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 6e08df2ba..b199dc9e3 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -17,12 +17,10 @@ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vites // ── DB setup ────────────────────────────────────────────────────────────────── -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); +vi.mock('../../../src/db/database', async () => { + + const { createSnapshotTestDb } = await import('../../helpers/db-mock'); + const db = createSnapshotTestDb(); const mock = { db, closeDb: () => {}, @@ -37,10 +35,10 @@ const { testDb, dbMock } = vi.hoisted(() => { isOwner: (tripId: any, userId: number) => !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), }; - return { testDb: db, dbMock: mock }; + return mock; }); -vi.mock('../../../src/db/database', () => dbMock); + vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', @@ -50,8 +48,7 @@ const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast })); // notifyInvite fires a notification via a dynamic import — keep it out of unit scope -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; +import { db as testDb } from '../../../src/db/database'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createReservation, createPlace, createDay, createDayAssignment, createDayNote, addTripMember } from '../../helpers/factories'; import { MAX_TRIP_DAYS } from '@trek/shared'; @@ -142,7 +139,6 @@ import { createTestCollectionPlacesRepo } from '../../helpers/test-uow'; let dbsEm: import('@mikro-orm/core').EntityManager | undefined; const dbs = () => new DatabaseService(testDb, dbsEm); - // Same collaborator set the container hands PlacesService (see places.service.test.ts). // Only the read-model aggregation reaches into places here, but the photo cache, // Unsplash and journey domain are real instances over the same in-memory DB @@ -155,7 +151,6 @@ const dbs = () => new DatabaseService(testDb, dbsEm); let photoCache: PlacePhotoCacheService; const coversFx = makeStorageFixture('covers/'); - let accommodationsSvc: Awaited>; let createAccommodation: (...args: Parameters) => ReturnType; beforeAll(async () => { @@ -163,8 +158,6 @@ beforeAll(async () => { createAccommodation = (...args) => accommodationsSvc.createAccommodation(...args); }); - - let budgetSvc: BudgetService; let daysSvc: DaysService; let placesSvc: PlacesService; @@ -243,7 +236,7 @@ beforeAll(async () => { await createTestUnitOfWork(dbs().connection), (await sharedTestOrm(testDb)).em, ); - membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestShareTokensRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); + membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestShareTokensRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); readModelSvc = new TripReadModelService( await createTestTripsRepo(dbs().connection), membersSvc, daysSvc, accommodationsSvc, budgetSvc, new PackingService( @@ -282,12 +275,6 @@ beforeAll(async () => { ); }); - -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - beforeEach(() => { resetTestDb(testDb); }); diff --git a/server/tests/unit/nest/user-cleanup.service.test.ts b/server/tests/unit/nest/user-cleanup.service.test.ts index 1f8902655..241f60778 100644 --- a/server/tests/unit/nest/user-cleanup.service.test.ts +++ b/server/tests/unit/nest/user-cleanup.service.test.ts @@ -53,8 +53,9 @@ import type { JourneyShareTokensRepository } from '../../../src/db/repositories/ import type { JourneysRepository } from '../../../src/db/repositories/Journeys.repository'; import type { JourneyEntriesRepository } from '../../../src/db/repositories/JourneyEntries.repository'; import type { JourneyContributorsRepository } from '../../../src/db/repositories/JourneyContributors.repository'; +import type { TripMembersRepository } from '../../../src/db/repositories/TripMembers.repository'; import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; -import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTripMembersRepo, sharedTestOrm } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; @@ -77,7 +78,10 @@ beforeAll(async () => { vi.spyOn(dbs, 'getPlaceWithTags').mockImplementation((...a) => real.getPlaceWithTags(...a)); budget = new BudgetService(dbs, new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))); svc = new UserCleanupService( - dbs, budget, await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), + dbs, budget, await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), + // Plan 4 Task 1 constructor-ripple: UC4's repository. + await createTestTripMembersRepo(dbs.connection), + await createTestBudgetItemsRepo(dbs.connection), // Plan 3g Task 4 constructor-ripple: UC7-10's repositories. await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), @@ -178,11 +182,13 @@ describe('erasePluginUserData', () => { const slim = new (require('better-sqlite3'))(':memory:'); slim.exec('CREATE TABLE users (id INTEGER PRIMARY KEY)'); slim.prepare('INSERT INTO users (id) VALUES (1)').run(); - // `erasePluginUserData` (this test's only call) never reaches `budgetItemsRepo` - // (UC5's own method) or the Plan 3g Task 4 journey repositories (UC7-10) — - // stubs are enough, and the slim schema has no journey tables to bind against. + // `erasePluginUserData` (this test's only call) never reaches `tripMembersRepo` + // (UC4), `budgetItemsRepo` (UC5's own method) or the Plan 3g Task 4 journey + // repositories (UC7-10) — stubs are enough, and the slim schema has no + // trip_members/journey tables to bind against. const slimSvc = new UserCleanupService( - new DatabaseService(slim), budget, await createTestUnitOfWork(slim), await createTestUsersRepo(slim), {} as unknown as BudgetItemsRepository, + new DatabaseService(slim), budget, await createTestUnitOfWork(slim), await createTestUsersRepo(slim), + {} as unknown as TripMembersRepository, {} as unknown as BudgetItemsRepository, {} as unknown as JourneyShareTokensRepository, {} as unknown as JourneysRepository, {} as unknown as JourneyEntriesRepository, {} as unknown as JourneyContributorsRepository, // Plan 3h Task 6: a real repository (never a stub cast — `erasePluginUserData` @@ -320,7 +326,9 @@ describe('deleteUserCompletely', () => { const deleteByIdSpy = vi.spyOn(usersRepo, 'deleteById').mockRejectedValue(new Error('boom')); try { await expect(new UserCleanupService( - dbs, budget, await createTestUnitOfWork(testDb), usersRepo, await createTestBudgetItemsRepo(testDb), + dbs, budget, await createTestUnitOfWork(testDb), usersRepo, + await createTestTripMembersRepo(testDb), + await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb),