refactor(server): plan 4 — convert the program's orphaned raw-SQL callers

This commit is contained in:
jubnl
2026-09-24 04:59:54 +02:00
parent fe69a3b9a7
commit 88fa13d55a
67 changed files with 1812 additions and 671 deletions
@@ -296,4 +296,36 @@ export class BucketListRepository extends TrekRepository<BucketList> {
.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<PublicApiBucketListItemRow[]> {
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;
}
@@ -118,4 +118,92 @@ export class DayNotesRepository extends TrekRepository<DayNotes> {
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<DayNoteRow[]> {
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<DayNoteRow[]>('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<DayNoteRow | undefined> {
return await this.qb('n')
.select(['n.*'])
.where('n.id = ? AND n.day_id = ? AND n.trip_id = ?', [id, day_id, trip_id])
.execute<DayNoteRow | undefined>('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<DayNoteRow | undefined> {
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<DayNoteRow | undefined>('get', false);
}
/** `day-notes.service.ts::remove` — `DELETE FROM day_notes WHERE id = ?`. */
async deleteById(id: number | string): Promise<void> {
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<DayNotesPublicApiKyselyDB>()
.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 };
}
@@ -349,4 +349,30 @@ export class DaysRepository extends TrekRepository<Days> {
}
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<PublicApiDayProjectionRow[]> {
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<PublicApiDayProjectionRow[]>('all', false);
}
}
/** {@link DaysRepository.listForPublicApi}'s projection. */
export interface PublicApiDayProjectionRow {
id: number;
day_number: number;
date: string | null;
title: string | null;
notes: string | null;
}
@@ -1,4 +1,75 @@
import type { IdempotencyKeys } from '../entities/IdempotencyKeys.entity';
import { TrekRepository } from './_shared/trek-repository';
export class IdempotencyKeysRepository extends TrekRepository<IdempotencyKeys> {}
/** 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<IdempotencyKeys> {
/**
* `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<IdempotencyResponseRow | null> {
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<void> {
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<number> {
return this.nativeDelete({ created_at: { $lt: cutoff } });
}
}
@@ -1,4 +1,31 @@
import type { Photos } from '../entities/Photos.entity';
import { TrekRepository } from './_shared/trek-repository';
export class PhotosRepository extends TrekRepository<Photos> {}
/** 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<Photos> {
/**
* (`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<PhotosKyselyDB>()
.selectFrom('photos')
.where('filename', '=', filename)
.select(['trip_id'])
.executeTakeFirst();
}
}
@@ -1224,6 +1224,66 @@ export class PlacesRepository extends TrekRepository<Places> {
{ 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<PublicApiAssignedPlaceRow[]> {
const rows = await this.kysely<PublicApiAssignedPlacesKyselyDB>()
.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). */
@@ -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<TripInviteTokens> {}
/** `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<TripInviteTokens> {
/** `get()` — `SELECT token, expires_at, created_at FROM trip_invite_tokens WHERE trip_id = ?`. */
async findInfoByTrip(trip_id: number | string): Promise<TripInviteInfoRow | undefined> {
return await this.qb('t')
.select(['t.token', 't.expires_at', 't.created_at'])
.where('t.trip_id = ?', [trip_id])
.execute<TripInviteInfoRow | undefined>('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<boolean> {
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<void> {
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<void> {
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<void> {
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<TripInviteResolvedRow | undefined> {
const row = await this.kysely<TripInviteResolveKyselyDB>()
.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;
}
}
@@ -290,4 +290,22 @@ export class TripMembersRepository extends TrekRepository<TripMembers> {
.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<void> {
await this.nativeUpdate({ invitedByRef: userId }, { invitedByRef: null });
}
}
@@ -947,6 +947,32 @@ export class TripsRepository extends TrekRepository<Trips> {
.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<TripIdTitleKyselyDB>()
.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. */
@@ -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<Users> {
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<UserWsHandshakeRow | null> {
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)
// ---------------------------------------------------------------------
+15 -14
View File
@@ -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) };
}
}
+6 -2
View File
@@ -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
+10 -2
View File
@@ -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],
@@ -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 };
+9 -4
View File
@@ -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<void> {
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)
@@ -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<void> {
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)`);
}
+9 -10
View File
@@ -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<number> {
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);
}
@@ -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<string, Promise<void>>();
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<Observable<unknown>> {
const req = context.switchToHttp().getRequest<Request & { user?: { id: number } }>();
@@ -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<Observable<unknown>> {
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<void>((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<IdempotencyRow | undefined> {
return this.database.get<IdempotencyRow>(
'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<IdempotencyResponseRow | null> {
return this.idempotencyKeys.findResponse(key, userId, req.method, req.path);
}
private replay(row: IdempotencyRow, res: Response): Observable<unknown> {
private replay(row: IdempotencyResponseRow, res: Response): Observable<unknown> {
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<Observable<unknown>> {
release: () => void,
): Observable<unknown> {
const originalJson = res.json.bind(res);
const database = this.database;
let done!: () => void;
inFlight.set(signature, new Promise<void>((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);
};
+12 -4
View File
@@ -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],
+39 -22
View File
@@ -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<TripAccess | undefined> {
@@ -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<DayNote>('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<void> {
this.dbs.run('DELETE FROM day_notes WHERE id = ?', id);
await this.dayNotes.deleteById(id);
}
}
@@ -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<ResolvedLlmConfig | null> {
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<string, unknown>;
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;
+10 -1
View File
@@ -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],
@@ -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],
})
@@ -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<boolean> {
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<boolean> {
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';
}
}
+22 -21
View File
@@ -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<void> {
async function servePhoto(storage: StorageService, req: Request, res: Response, users: UsersRepository, shareTokens: ShareTokensRepository, photos: PhotosRepository): Promise<void> {
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
@@ -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],
})
@@ -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<PublicApiDay[]> {
const days = this.db.all<DayRow>(
`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<Map<number, PublicApiPlace[]>> {
const rows = this.db.all<PlaceRow>(
`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<PublicApiAssignedPlaceRow, PublicApiPlace>(rows, (r) => r.day_id, toPlace);
}
private async dayNotesByDay(tripId: number): Promise<Map<number, PublicApiDayNote[]>> {
const rows = this.db.all<DayNoteRow>(
`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<PublicApiBucketListItem[]> {
const rows = this.db.all<BucketListRow>(
`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,
@@ -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 {}
+13 -6
View File
@@ -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<typeof setInterval> | 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<User & { password_version?: number }>(
'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');
@@ -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<boolean> {
if ((await readTransitProvider(this.database)) !== 'google') return false;
if ((await readTransitProvider(this.appSettings)) !== 'google') return false;
return !!(await this.resolveKey(userId)).key;
}
+12 -10
View File
@@ -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<TransitProvider> {
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<TransitProvider> {
const value = await appSettings.getValue(TRANSIT_PROVIDER_SETTING);
return isTransitProvider(value) ? value : DEFAULT_TRANSIT_PROVIDER;
}
export async function writeTransitProvider(db: DatabaseService, provider: TransitProvider): Promise<TransitProvider> {
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<TransitProvider> {
await appSettings.setValue(TRANSIT_PROVIDER_SETTING, provider);
return provider;
}
+6 -3
View File
@@ -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.
@@ -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],
})
@@ -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<TripInviteInfo | null> {
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<void> {
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