From 399ca58a7d6b23ae849c6d855e69510cdfca111b Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 23 Sep 2026 18:22:02 +0200 Subject: [PATCH] refactor(files): trip files and file links through repositories, with transactions added --- .../db/repositories/BudgetItems.repository.ts | 26 +- .../repositories/DayAssignments.repository.ts | 26 ++ .../db/repositories/FileLinks.repository.ts | 140 ++++++++- .../src/db/repositories/Places.repository.ts | 20 ++ .../repositories/Reservations.repository.ts | 24 ++ .../db/repositories/TripFiles.repository.ts | 207 ++++++++++++- .../nest/files/allowed-file-types.module.ts | 7 +- .../nest/files/allowed-file-types.service.ts | 16 +- server/src/nest/files/files.module.ts | 15 + server/src/nest/files/files.rpc.ts | 11 +- server/src/nest/files/files.service.ts | 285 +++++++++++------- server/tests/helpers/files-repos.ts | 40 +++ server/tests/unit/files/files.rpc.test.ts | 18 +- server/tests/unit/nest/files.service.test.ts | 189 +++++++++++- 14 files changed, 882 insertions(+), 142 deletions(-) create mode 100644 server/tests/helpers/files-repos.ts diff --git a/server/src/db/repositories/BudgetItems.repository.ts b/server/src/db/repositories/BudgetItems.repository.ts index 7d73a3da6..f7b79bef3 100644 --- a/server/src/db/repositories/BudgetItems.repository.ts +++ b/server/src/db/repositories/BudgetItems.repository.ts @@ -1,4 +1,28 @@ import type { BudgetItems } from '../entities/BudgetItems.entity'; +import { columnRef } from '../dialect/sql-functions'; import { TrekRepository } from './_shared/trek-repository'; -export class BudgetItemsRepository extends TrekRepository {} +// --------------------------------------------------------------------------- +// Plan 3e Task 1 (`FilesService.findForeignLinkTarget`, R12) — additive, +// append-only per that task's own file-ownership rule (Task 2/budget owns +// every other method appended to this file). +// --------------------------------------------------------------------------- + +export class BudgetItemsRepository extends TrekRepository { + /** + * FL4 (`FilesService.findForeignLinkTarget`'s budget-expense branch) — + * `SELECT 1 FROM budget_items WHERE id = ? AND trip_id = ?`, re-expressed + * as "what trip does this row belong to" (R12, the same shape as + * `ReservationsRepository.findTripId`'s docstring). `trip_id` is a + * `persist(false)` mirror of the `trip` relation — `columnRef`, not a bare + * select (the program-wide trap). + */ + async findTripId(id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + const row = await this.qb('b') + .select([columnRef(platform, 'b.trip_id').as('trip_id')]) + .where({ id }) + .execute<{ trip_id: number } | undefined>('get', false); + return row?.trip_id; + } +} diff --git a/server/src/db/repositories/DayAssignments.repository.ts b/server/src/db/repositories/DayAssignments.repository.ts index 2fa346bb0..d7eb81790 100644 --- a/server/src/db/repositories/DayAssignments.repository.ts +++ b/server/src/db/repositories/DayAssignments.repository.ts @@ -933,4 +933,30 @@ export class DayAssignmentsRepository extends TrekRepository { async clearStay(id: number): Promise { await this.nativeUpdate({ id }, { accommodation_id: null }); } + + // --------------------------------------------------------------------------- + // Plan 3e Task 1 (`FilesService.findForeignLinkTarget`, R12) — additive, + // append-only per that task's own file-ownership rule. + // --------------------------------------------------------------------------- + + /** + * FL3 (`FilesService.findForeignLinkTarget`'s assignment branch) — `SELECT + * 1 FROM day_assignments a JOIN days d ON a.day_id = d.id WHERE a.id = ? + * AND d.trip_id = ?`, re-expressed as "what trip does this row belong to" + * via its `day` relation (R12 — "DayAssignmentsRepository.findTripId via + * its days join"; `day_assignments` has no `trip_id` column of its own). + * The same join shape `ReservationsRepository.getAssignmentTripId` + * (RS22) already used from a DIFFERENT repository before this file was + * this task's own to append to — this copy lives here instead, per R12's + * "one method per target table, on that table's own repository" ruling. + */ + async findTripId(id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + const row = await this.qb('da') + .join('da.day', 'd') + .select([columnRef(platform, 'd.trip_id').as('trip_id')]) + .where({ id }) + .execute<{ trip_id: number } | undefined>('get', false); + return row?.trip_id; + } } diff --git a/server/src/db/repositories/FileLinks.repository.ts b/server/src/db/repositories/FileLinks.repository.ts index d1a94780c..a07cd1554 100644 --- a/server/src/db/repositories/FileLinks.repository.ts +++ b/server/src/db/repositories/FileLinks.repository.ts @@ -1,4 +1,142 @@ import type { FileLinks } from '../entities/FileLinks.entity'; +import { type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; -export class FileLinksRepository extends TrekRepository {} +/** A bare `file_links` row — every scalar column of the entity, incl. its five `persist(false)` relation mirrors. */ +export interface FileLinkRow { + id: number; + file_id: number; + reservation_id: number | null; + assignment_id: number | null; + place_id: number | null; + created_at: string | null; + budget_item_id: number | null; +} + +const _fileLinkRowKeys: AssertRowKeys = true; + +/** FL27's joined projection (`getFileLinks`). */ +export interface FileLinkWithReservationRow extends FileLinkRow { + reservation_title: string | null; +} + +/** FL8's narrow batch projection (`listFiles`'s link lookup). */ +export interface FileLinkTargetRow { + file_id: number; + reservation_id: number | null; + place_id: number | null; + budget_item_id: number | null; +} + +interface FileLinksKyselyDB { + file_links: FileLinkRow; + reservations: { id: number; title: string }; +} + +/** + * The insert-only shape — `id`/`created_at` are autoincrement/`DEFAULT + * CURRENT_TIMESTAMP` and omitted from `.values()` below (the legacy + * statement's own column list omits them too, matching this exactly), so a + * SEPARATE interface from {@link FileLinksKyselyDB} rather than making those + * two columns optional there — that would also loosen every SELECT method's + * inferred `id`/`created_at` to `T | undefined`, which is wrong for a row + * that unquestionably has both once read back (the `ReservationJoinKyselyDB` + * / `ReservationRestampKyselyDB` precedent, `Reservations.repository.ts`: + * one purpose-shaped Kysely interface per statement, not one reused + * everywhere). + */ +interface FileLinksWriteKyselyDB { + file_links: { + file_id: number; + reservation_id: number | null; + assignment_id: number | null; + place_id: number | null; + budget_item_id: number | null; + }; +} + +/** + * `file_links` — the many-to-many between a `trip_files` row and the + * reservation/assignment/place/budget-expense it points at. Kysely + * throughout, same reasoning as `TripFilesRepository`'s class docstring + * (five `persist(false)` mirrors on this entity too), and because the + * `INSERT OR IGNORE` write below has no `em.upsert`-expressible single + * conflict target (four independent partial-unique indexes, only one of + * which any given insert touches) — Kysely's target-less `ON CONFLICT DO + * NOTHING` is the exact SQLite equivalent of `OR IGNORE`, ignoring + * whichever constraint the row happens to violate, matching the legacy + * statement precisely (`Tags.repository.ts#insertIgnore`'s explicit-target + * precedent is the WRONG shape here — that pivot has exactly one unique key). + */ +export class FileLinksRepository extends TrekRepository { + /** + * FL8 (`FilesService.listFiles`'s link batch) — `SELECT file_id, + * reservation_id, place_id, budget_item_id FROM file_links WHERE file_id + * IN (dynamic)`. Empty `file_ids` short-circuits before any query, + * matching the legacy service's own guard. + */ + async listForFiles(file_ids: number[]): Promise { + if (file_ids.length === 0) return []; + return await this.kysely() + .selectFrom('file_links') + .select(['file_id', 'reservation_id', 'place_id', 'budget_item_id']) + .where('file_id', 'in', file_ids) + .execute(); + } + + /** + * FL10/FL13/FL24 (`createFile`'s conditional insert, `updateFile`'s + * conditional insert, `createFileLink`) — `INSERT OR IGNORE INTO + * file_links (file_id, reservation_id, assignment_id, place_id, + * budget_item_id) VALUES (?×5)`, the same statement shape at all three + * legacy sites (FL10/FL13 pass only `budget_item_id`; FL24 passes any + * combination the caller supplies) — one method, not three. + */ + async insertIgnore(row: { file_id: number; reservation_id?: number | null; assignment_id?: number | null; place_id?: number | null; budget_item_id?: number | null }): Promise { + await this.kysely() + .insertInto('file_links') + .values({ + file_id: row.file_id, + reservation_id: row.reservation_id ?? null, + assignment_id: row.assignment_id ?? null, + place_id: row.place_id ?? null, + budget_item_id: row.budget_item_id ?? null, + }) + .onConflict((oc) => oc.doNothing()) + .execute(); + } + + /** FL14 (`FilesService.updateFile`'s conditional clear) — `DELETE FROM file_links WHERE file_id = ? AND budget_item_id IS NOT NULL`. */ + async clearBudgetLink(file_id: number): Promise { + await this.kysely() + .deleteFrom('file_links') + .where('file_id', '=', file_id) + .where('budget_item_id', 'is not', null) + .execute(); + } + + /** FL26 (`FilesService.deleteFileLink`) — `DELETE FROM file_links WHERE id = ? AND file_id = ?`. */ + async deleteById(id: number, file_id: number): Promise { + await this.kysely() + .deleteFrom('file_links') + .where('id', '=', id) + .where('file_id', '=', file_id) + .execute(); + } + + /** FL25 (`createFileLink`'s bare re-select) — `SELECT * FROM file_links WHERE file_id = ?`. */ + async listForFile(file_id: number): Promise { + return await this.kysely().selectFrom('file_links').selectAll().where('file_id', '=', file_id).execute(); + } + + /** FL27 (`FilesService.getFileLinks`) — `SELECT fl.*, r.title as reservation_title FROM file_links fl LEFT JOIN reservations r ON fl.reservation_id = r.id WHERE fl.file_id = ?`. */ + async listForFileWithReservationTitle(file_id: number): Promise { + return await this.kysely() + .selectFrom('file_links as fl') + .leftJoin('reservations as r', 'r.id', 'fl.reservation_id') + .selectAll('fl') + .select(['r.title as reservation_title']) + .where('fl.file_id', '=', file_id) + .execute(); + } +} diff --git a/server/src/db/repositories/Places.repository.ts b/server/src/db/repositories/Places.repository.ts index e36ee90da..9f2bdc262 100644 --- a/server/src/db/repositories/Places.repository.ts +++ b/server/src/db/repositories/Places.repository.ts @@ -892,4 +892,24 @@ export class PlacesRepository extends TrekRepository { .where({ id }) .execute<{ name: string } | undefined>('get', false); } + + // --------------------------------------------------------------------------- + // Plan 3e Task 1 (`FilesService.findForeignLinkTarget`, R12) — additive, + // append-only per that task's own file-ownership rule. + // --------------------------------------------------------------------------- + + /** + * FL2 (`FilesService.findForeignLinkTarget`'s place branch) — `SELECT 1 + * FROM places WHERE id = ? AND trip_id = ?`, re-expressed as "what trip + * does this row belong to" (R12, the same shape as + * `ReservationsRepository.findTripId`'s docstring). + */ + async findTripId(id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + const row = await this.qb('p') + .select([columnRef(platform, 'p.trip_id').as('trip_id')]) + .where({ id }) + .execute<{ trip_id: number } | undefined>('get', false); + return row?.trip_id; + } } diff --git a/server/src/db/repositories/Reservations.repository.ts b/server/src/db/repositories/Reservations.repository.ts index 9ba0acd2a..cd213b40a 100644 --- a/server/src/db/repositories/Reservations.repository.ts +++ b/server/src/db/repositories/Reservations.repository.ts @@ -1439,6 +1439,30 @@ export class ReservationsRepository extends TrekRepository { .execute(); return rows as PublicApiUnplannedPlaceRow[]; } + + // --------------------------------------------------------------------------- + // Plan 3e Task 1 (`FilesService.findForeignLinkTarget`, R12) — additive, + // append-only per that task's own file-ownership rule. + // --------------------------------------------------------------------------- + + /** + * FL1 (`FilesService.findForeignLinkTarget`'s reservation branch) — + * `SELECT 1 FROM reservations WHERE id = ? AND trip_id = ?`, re-expressed + * as "what trip does this row belong to" (R12: one method per target + * table, no dynamic identifier dispatch — the RS21/RS23/RS25 D4 precedent) + * so the caller compares the returned `trip_id` against its own, already + * `toRowId`-parsed value instead of binding two ids into the query itself. + * `trip_id` is a `persist(false)` mirror of the `trip` relation — + * `columnRef`, not a bare select (the program-wide trap). + */ + async findTripId(id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + const row = await this.qb('r') + .select([columnRef(platform, 'r.trip_id').as('trip_id')]) + .where({ id }) + .execute<{ trip_id: number } | undefined>('get', false); + return row?.trip_id; + } } /** `SELECT *` — every scalar column of `Reservations`, RS35's shape. */ diff --git a/server/src/db/repositories/TripFiles.repository.ts b/server/src/db/repositories/TripFiles.repository.ts index 4cea6c0e8..8678b3b64 100644 --- a/server/src/db/repositories/TripFiles.repository.ts +++ b/server/src/db/repositories/TripFiles.repository.ts @@ -1,4 +1,209 @@ +import { currentTimestamp } from '../dialect/sql-functions'; import type { TripFiles } from '../entities/TripFiles.entity'; +import { type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; -export class TripFilesRepository extends TrekRepository {} +/** + * A bare `trip_files` row — every scalar column of the entity, incl. the SIX + * `persist(false)` relation mirrors (`trip_id`, `place_id`, `reservation_id`, + * `note_id`, `uploaded_by`, `message_id` — the trap the class docstring + * below names). Kysely's `selectAll()` reads real DB columns directly, so it + * carries every one of these without the bare-QB-`.select([...])` drop a + * `columnRef` would otherwise be needed to avoid. + */ +export interface TripFileRow { + id: number; + trip_id: number; + place_id: number | null; + reservation_id: number | null; + filename: string; + original_name: string; + file_size: number | null; + mime_type: string | null; + description: string | null; + created_at: string | null; + note_id: number | null; + uploaded_by: number | null; + starred: number | null; + deleted_at: string | null; + message_id: number | null; +} + +const _tripFileRowKeys: AssertRowKeys = true; + +/** FILE_SELECT's joined projection (FL7/FL11/FL15/FL17/FL20). */ +export interface TripFileJoinRow extends TripFileRow { + reservation_title: string | null; + uploaded_by_name: string | null; + uploaded_by_avatar: string | null; +} + +interface TripFilesKyselyDB { + trip_files: TripFileRow; + reservations: { id: number; title: string }; + users: { id: number; username: string; avatar: string | null }; +} + +/** + * `trip_files` — trip attachments (uploads, expense receipts). Every read + * below goes through Kysely rather than the QueryBuilder: this entity has + * SIX `persist(false)` mirror columns (see {@link TripFileRow}'s docstring), + * and a bare QB `.select([...])` silently drops any of them it names without + * `columnRef` (the program-wide trap `RoadtripDayBoundariesRepository` + * documents) — Kysely's `selectAll()`/typed column list reads the real DB + * columns directly and has no such gap, so it is the simpler, uniformly-safe + * choice for a row this wide rather than six `columnRef` calls repeated + * across five read methods. + */ +export class TripFilesRepository extends TrekRepository { + /** FILE_SELECT (`:22-27`) — `trip_files f LEFT JOIN reservations r ON f.reservation_id = r.id LEFT JOIN users u ON f.uploaded_by = u.id`. */ + private joinedQuery() { + return this.kysely() + .selectFrom('trip_files as f') + .leftJoin('reservations as r', 'r.id', 'f.reservation_id') + .leftJoin('users as u', 'u.id', 'f.uploaded_by') + .selectAll('f') + .select(['r.title as reservation_title', 'u.username as uploaded_by_name', 'u.avatar as uploaded_by_avatar']); + } + + /** + * FL5 (`FilesService.getFileById`) — `SELECT * FROM trip_files WHERE id = ? + * AND trip_id = ?`, the trip-scoping guard reused by budget/collab reads + * (BG29/44, CB21) and by `permanentDeleteFile`/`softDeleteFile`/`remove`'s + * pre-image reads. + */ + async findInTrip(id: number, trip_id: number): Promise { + return await this.kysely() + .selectFrom('trip_files') + .selectAll() + .where('id', '=', id) + .where('trip_id', '=', trip_id) + .executeTakeFirst(); + } + + /** FL6 (`FilesService.getDeletedFile`) — same as {@link findInTrip} plus `AND deleted_at IS NOT NULL`, the trash-scoped variant. */ + async findDeletedInTrip(id: number, trip_id: number): Promise { + return await this.kysely() + .selectFrom('trip_files') + .selectAll() + .where('id', '=', id) + .where('trip_id', '=', trip_id) + .where('deleted_at', 'is not', null) + .executeTakeFirst(); + } + + /** + * FL7 (`FilesService.listFiles`) — `` FILE_SELECT WHERE f.trip_id = ? AND + * f.message_id IS NULL AND f.deleted_at IS (NOT) NULL ORDER BY f.starred + * DESC, f.created_at DESC ``. `showTrash` toggles the `deleted_at` arm + * (D4: two WHERE shapes, one method, matching the legacy ternary). + */ + async listForTrip(trip_id: number, showTrash: boolean): Promise { + let q = this.joinedQuery().where('f.trip_id', '=', trip_id).where('f.message_id', 'is', null); + q = showTrash ? q.where('f.deleted_at', 'is not', null) : q.where('f.deleted_at', 'is', null); + return await q.orderBy('f.starred', 'desc').orderBy('f.created_at', 'desc').execute(); + } + + /** + * FL11/FL15/FL17/FL20 — `` FILE_SELECT WHERE f.id = ? `` , the SAME + * re-select text run after `createFile`/`updateFile`/`toggleStarred`/ + * `restoreFile`. No `trip_id` filter (matching the legacy statement — the + * caller already knows `id` is in-scope by construction, having just + * written it). + */ + async findByIdWithJoins(id: number): Promise { + return await this.joinedQuery().where('f.id', '=', id).executeTakeFirst(); + } + + /** FL22 (`FilesService.emptyTrash`'s read half) — `SELECT * FROM trip_files WHERE trip_id = ? AND deleted_at IS NOT NULL`. */ + async listTrashed(trip_id: number): Promise { + return await this.kysely() + .selectFrom('trip_files') + .selectAll() + .where('trip_id', '=', trip_id) + .where('deleted_at', 'is not', null) + .execute(); + } + + /** + * FL9 (`FilesService.createFile`) — `INSERT INTO trip_files (trip_id, + * place_id, reservation_id, filename, original_name, file_size, mime_type, + * description, uploaded_by) VALUES (?×9)`. Relation properties + * (`trip`/`place`/`reservation`/`uploadedByRef`), not their `persist(false)` + * `_id` mirrors — MikroORM accepts a raw scalar id for a `Ref`-typed + * property (`InviteTokensRepository.insertInvite`'s precedent). + * `uploadedByRef` is the entity's own name for the `uploaded_by` + * `joinColumn` relation (`TripFiles.entity.ts`). `trip_id: number | + * string` — the caller's own trip id, already vouched for by the + * guard/access check upstream, written (never compared) here; SQLite's + * own INTEGER-affinity conversion on INSERT normalises either shape the + * same way `DayAccommodationsRepository.insertStay`'s `trip_id` does, so + * no `toRowId` gate is needed on a write-only value (rule 21 governs the + * read-then-write seam, not every scalar a write ever carries). + */ + async insertFile(row: { + trip_id: number | string; + place_id: number | null; + reservation_id: number | null; + filename: string; + original_name: string; + file_size: number; + mime_type: string; + description: string | null; + uploaded_by: number; + }): Promise { + return await this.insert({ + trip: row.trip_id, + place: row.place_id, + reservation: row.reservation_id, + filename: row.filename, + original_name: row.original_name, + file_size: row.file_size, + mime_type: row.mime_type, + description: row.description, + uploadedByRef: row.uploaded_by, + }); + } + + /** + * FL12 (`FilesService.updateFile`) — `UPDATE trip_files SET description = + * ?, place_id = ?, reservation_id = ? WHERE id = ?`. The caller resolves + * the `!== undefined ? (x || null) : current.x` presence sentinel BEFORE + * calling this (matching the legacy inline ternaries) — this method just + * writes the three final values verbatim. + */ + async updateFile(id: number, write: { description: string | null; place_id: number | null; reservation_id: number | null }): Promise { + await this.nativeUpdate({ id }, { description: write.description, place: write.place_id, reservation: write.reservation_id }); + } + + /** FL16 (`FilesService.toggleStarred`) — `UPDATE trip_files SET starred = ? WHERE id = ?`. */ + async setStarred(id: number, starred: number): Promise { + await this.nativeUpdate({ id }, { starred }); + } + + /** FL18 (`FilesService.softDeleteFile`) — `UPDATE trip_files SET deleted_at = CURRENT_TIMESTAMP WHERE id = ?`. */ + async softDelete(id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.nativeUpdate({ id }, { deleted_at: currentTimestamp(platform) }); + } + + /** FL19 (`FilesService.restoreFile`) — `UPDATE trip_files SET deleted_at = NULL WHERE id = ?`. */ + async restore(id: number): Promise { + await this.nativeUpdate({ id }, { deleted_at: null }); + } + + /** FL21 (`FilesService.permanentDeleteFile`) — `DELETE FROM trip_files WHERE id = ?`, run after a successful (or idempotent-missing) storage delete. */ + async deleteById(id: number): Promise { + await this.nativeDelete({ id }); + } + + /** + * FL23 (`FilesService.emptyTrash`'s bulk delete) — `DELETE FROM trip_files + * WHERE id IN (dynamic)`, only for the ids whose storage delete already + * succeeded. `$in` for the dynamic list (rule 23) — no interpolated SQL. + */ + async deleteMany(ids: number[]): Promise { + if (ids.length === 0) return; + await this.nativeDelete({ id: { $in: ids } }); + } +} diff --git a/server/src/nest/files/allowed-file-types.module.ts b/server/src/nest/files/allowed-file-types.module.ts index 3778fe183..86a8244b0 100644 --- a/server/src/nest/files/allowed-file-types.module.ts +++ b/server/src/nest/files/allowed-file-types.module.ts @@ -1,11 +1,16 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { AllowedFileTypesService } from './allowed-file-types.service'; +import { AppSettings } from '../../db/entities/AppSettings.entity'; /** * A leaf so both upload paths can reach the extension list without importing - * each other's domain. DatabaseModule is @Global, so this imports nothing. + * each other's domain. `MikroOrmModule.forFeature([AppSettings])` registers + * `AppSettingsRepository` for `AllowedFileTypesService`'s `@InjectRepository` + * (FL29 — the existing 3a repository, this table is not this plan's own). */ @Module({ + imports: [MikroOrmModule.forFeature([AppSettings])], providers: [AllowedFileTypesService], exports: [AllowedFileTypesService], }) diff --git a/server/src/nest/files/allowed-file-types.service.ts b/server/src/nest/files/allowed-file-types.service.ts index 403388c54..c2bfae4fb 100644 --- a/server/src/nest/files/allowed-file-types.service.ts +++ b/server/src/nest/files/allowed-file-types.service.ts @@ -1,5 +1,7 @@ import { Injectable } from '@nestjs/common'; -import { DatabaseService } from '../database/database.service'; +import { InjectRepository } from '@mikro-orm/nestjs'; +import { AppSettings } from '../../db/entities/AppSettings.entity'; +import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository'; import { DEFAULT_ALLOWED_EXTENSIONS } from './files.constants'; /** @@ -10,18 +12,20 @@ import { DEFAULT_ALLOWED_EXTENSIONS } from './files.constants'; * thing an upload's fileFilter needs from the container, and it is a live read: * an admin changing the list in settings applies to the next upload, with no * invalidation wiring. + * + * FL29 — `SELECT value FROM app_settings WHERE key = 'allowed_file_types'`, + * now `AppSettingsRepository.getValue` (the existing 3a repository, per the + * plan's R12/§5 note — this table is not this plan's own). */ @Injectable() export class AllowedFileTypesService { - constructor(private readonly db: DatabaseService) {} + constructor(@InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository) {} /** Comma-separated, as the admin panel stores it. `*` means anything. */ async get(): Promise { try { - const row = this.db.get<{ value: string }>( - "SELECT value FROM app_settings WHERE key = 'allowed_file_types'", - ); - return row?.value || DEFAULT_ALLOWED_EXTENSIONS; + const value = await this.appSettings.getValue('allowed_file_types'); + return value || DEFAULT_ALLOWED_EXTENSIONS; } catch { return DEFAULT_ALLOWED_EXTENSIONS; } diff --git a/server/src/nest/files/files.module.ts b/server/src/nest/files/files.module.ts index 6542f0230..708689d04 100644 --- a/server/src/nest/files/files.module.ts +++ b/server/src/nest/files/files.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { FilesController } from './files.controller'; import { FilesDownloadController } from './files-download.controller'; import { FilesService } from './files.service'; @@ -19,6 +20,13 @@ import { StorageService } from '../storage/storage.service'; import { buildStorageUploadOptions } from '../storage/storage-upload.factory'; import { filesUploadFileFilter } from './files.controller'; import { MAX_VIDEO_SIZE } from './files.constants'; +import { TripFiles } from '../../db/entities/TripFiles.entity'; +import { FileLinks } from '../../db/entities/FileLinks.entity'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import { Places } from '../../db/entities/Places.entity'; +import { DayAssignments } from '../../db/entities/DayAssignments.entity'; +import { BudgetItems } from '../../db/entities/BudgetItems.entity'; +import { Users } from '../../db/entities/Users.entity'; @Module({ imports: [ @@ -36,6 +44,13 @@ import { MAX_VIDEO_SIZE } from './files.constants'; }), }), StorageModule, + // TripFiles/FileLinks are this domain's own tables (Plan 3e Task 1). + // Reservations/Places/DayAssignments/BudgetItems/Users are owned + // elsewhere — registered here only for `findForeignLinkTarget`'s + // `findTripId` reads and FL28's `UsersRepository.getEmail` (the entity + // classes only, never the owning module — the `AccommodationsDomainModule` + // precedent). + MikroOrmModule.forFeature([TripFiles, FileLinks, Reservations, Places, DayAssignments, BudgetItems, Users]), // AuthModule + McpSharedModule feed FilesMcp's demo and RBAC guards. Neither is // @Global, and AuthModule reaches this domain only through the leaf // AllowedFileTypesModule, so importing it here stays cycle-free. diff --git a/server/src/nest/files/files.rpc.ts b/server/src/nest/files/files.rpc.ts index e7d0a2b1a..a9d06252a 100644 --- a/server/src/nest/files/files.rpc.ts +++ b/server/src/nest/files/files.rpc.ts @@ -1,13 +1,15 @@ import pathMod from 'node:path'; import { randomUUID } from 'node:crypto'; import { Readable } from 'node:stream'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators'; import { PluginGuards } from '../plugins/host/plugin-guards.service'; import { BadParams, ForbiddenResource } from '../plugins/host/rpc-errors'; import { asPayload, num } from '../plugins/host/rpc-params'; import type { PluginRpcContext } from '../plugins/host/rpc-kit/types'; import { RealtimeService } from '../realtime/realtime.service'; -import { DatabaseService } from '../database/database.service'; +import { Users } from '../../db/entities/Users.entity'; +import type { UsersRepository } from '../../db/repositories/Users.repository'; import { readEnv } from '../../app-config'; import { isDemoEmail } from '../common/demo'; import { BLOCKED_EXTENSIONS } from './files.constants'; @@ -48,7 +50,7 @@ export class FilesRpc { constructor( private readonly files: FilesService, private readonly realtime: RealtimeService, - private readonly db: DatabaseService, + @InjectRepository(Users) private readonly users: UsersRepository, private readonly guards: PluginGuards, private readonly storage: StorageService, ) {} @@ -110,8 +112,9 @@ export class FilesRpc { // demo instance, not even through a plugin's db:write:files. The email is only // resolved when demo mode is actually on, so self-hosted installs pay nothing. if (readEnv().demo.enabled) { - const uploader = this.db.prepare('SELECT email FROM users WHERE id = ?').get(actingUserId) as { email?: string } | undefined; - if (isDemoEmail(uploader?.email)) throw new ForbiddenResource('Uploads are disabled in demo mode.'); + // FL28 — `SELECT email FROM users WHERE id = ?`, now `UsersRepository.getEmail`. + const email = await this.users.getEmail(actingUserId); + if (isDemoEmail(email)) throw new ForbiddenResource('Uploads are disabled in demo mode.'); } const original = pathMod.basename(input.name); const ext = pathMod.extname(original).toLowerCase(); diff --git a/server/src/nest/files/files.service.ts b/server/src/nest/files/files.service.ts index 4d1e61c07..0a376110b 100644 --- a/server/src/nest/files/files.service.ts +++ b/server/src/nest/files/files.service.ts @@ -1,4 +1,5 @@ import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import path from 'path'; import type { Readable } from 'node:stream'; import type { Request } from 'express'; @@ -10,23 +11,29 @@ import { EphemeralTokenService } from '../auth/ephemeral-token.service'; import { verifyJwtAndLoadUser } from '../auth/jwt-verify'; import { EntityManager } from '@mikro-orm/core'; import { Users } from '../../db/entities/Users.entity'; -import type { User, TripFile } from '../../types'; +import type { User } from '../../types'; import { DatabaseService, type TripAccess } from '../database/database.service'; -import { DEFAULT_ALLOWED_EXTENSIONS } from './files.constants'; +import { UnitOfWork } from '../database/unit-of-work'; +import { toRowId } from '../common/row-id'; import { StorageService } from '../storage/storage.service'; import { StorageNotFoundError, StorageInvalidKeyError, type ObjectStat } from '../storage/storage.types'; +import { TripFiles } from '../../db/entities/TripFiles.entity'; +import type { TripFilesRepository, TripFileRow, TripFileJoinRow } from '../../db/repositories/TripFiles.repository'; +import { FileLinks } from '../../db/entities/FileLinks.entity'; +import type { FileLinksRepository, FileLinkTargetRow } from '../../db/repositories/FileLinks.repository'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; +import { Places } from '../../db/entities/Places.entity'; +import type { PlacesRepository } from '../../db/repositories/Places.repository'; +import { DayAssignments } from '../../db/entities/DayAssignments.entity'; +import type { DayAssignmentsRepository } from '../../db/repositories/DayAssignments.repository'; +import { BudgetItems } from '../../db/entities/BudgetItems.entity'; +import type { BudgetItemsRepository } from '../../db/repositories/BudgetItems.repository'; type Trip = TripAccess; type FilePermission = 'file_upload' | 'file_edit' | 'file_delete'; -const FILE_SELECT = ` - SELECT f.*, r.title as reservation_title, u.username as uploaded_by_name, u.avatar as uploaded_by_avatar - FROM trip_files f - LEFT JOIN reservations r ON f.reservation_id = r.id - LEFT JOIN users u ON f.uploaded_by = u.id -`; - -function formatFile(file: TripFile & { uploaded_by_avatar?: string | null }) { +function formatFile(file: TripFileJoinRow) { const tripId = file.trip_id; return { ...file, @@ -57,21 +64,52 @@ export class FileContentError extends Error { } } +/** + * A truthy, non-canonical link-target id is narrowed to `null` (rule 15's + * accepted deviation, applied everywhere a foreign-key id from the wire + * reaches a typed repository write): the legacy statements bound the raw + * value straight into `WHERE id = ? AND trip_id = ?`/an INSERT column and + * let SQLite's loose affinity or a constraint failure decide, matching + * `|| null` on falsy. A falsy value (`0`/`''`/`null`/`undefined`) always + * meant "no link" and still does; a truthy value that isn't a canonical + * decimal id now also resolves to "no link" instead of whatever SQLite's + * affinity would have matched or thrown on. + */ +function coerceLinkId(value: string | number | null | undefined): number | null { + if (!value) return null; + return toRowId(value); +} + /** * File domain service — owns the file SQL (moved 1:1 from the legacy * services/fileService.ts: identical statements, the `||` falsy-coercion * defaults, the post-write FILE_SELECT re-selects, the dynamic IN batches and - * the unlink-first delete semantics). Trip access goes through the injected - * DatabaseService's canAccessTrip (the legacy verifyTripAccess re-export was a - * plain wrapper over the same helper); the file_* permissions, path-resolution - * guard, download-token auth and WebSocket broadcasts are unchanged. The - * load-time constants live in files.constants.ts; the admin allowed-types - * live-read is AllowedFileTypesService (the single query owner since the - * storage slice-2 consolidation). + * the unlink-first delete semantics; Plan 3e Task 1 moved the SQL itself onto + * `TripFilesRepository`/`FileLinksRepository`). Trip access goes through the + * injected DatabaseService's canAccessTrip (the legacy verifyTripAccess + * re-export was a plain wrapper over the same helper); the file_* + * permissions, path-resolution guard, download-token auth and WebSocket + * broadcasts are unchanged. The load-time constants live in files.constants.ts; + * the admin allowed-types live-read is AllowedFileTypesService (the single + * query owner since the storage slice-2 consolidation). * - * Two deliberate post-migration fixes over the legacy behavior: createFileLink - * no longer swallows insert errors, and updateFile coerces an empty-string - * description to NULL exactly like createFile does. + * Two deliberate post-migration fixes over the legacy behavior, carried + * unchanged from the pre-repository version: createFileLink no longer + * swallows insert errors, and updateFile coerces an empty-string description + * to NULL exactly like createFile does. + * + * **R2 (Plan 3e, flagged behaviour change — failure atomicity):** `files` + * was the one domain in this cluster with ZERO `uow.transactional` calls + * despite three genuinely multi-statement writes (`createFile`+its + * conditional `file_links` insert, `updateFile`+its conditional + * `file_links` insert/delete, `emptyTrash`'s DB-only bulk delete). Every + * sibling domain already carries this fix from its own 2026-08 fold; files + * never got it. `createFile`, `updateFile` and `emptyTrash`'s bulk delete + * now run inside `uow.transactional` — a failed second statement no longer + * leaves an orphaned `trip_files` row with no matching `file_links` row (or + * vice versa). The storage I/O in `permanentDeleteFile`/`emptyTrash` stays + * OUTSIDE every transactional body (program rule 24); `emptyTrash`'s read + * (the trashed-file list) also stays outside, matching the DB-only-body rule. */ @Injectable() export class FilesService { @@ -86,6 +124,13 @@ export class FilesService { // kept uniform with the guards rather than adding a one-off // MikroOrmModule.forFeature([Users]) to FilesModule for this single call. private readonly em: EntityManager, + private readonly uow: UnitOfWork, + @InjectRepository(TripFiles) private readonly tripFilesRepo: TripFilesRepository, + @InjectRepository(FileLinks) private readonly fileLinksRepo: FileLinksRepository, + @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, + @InjectRepository(Places) private readonly placesRepo: PlacesRepository, + @InjectRepository(DayAssignments) private readonly dayAssignmentsRepo: DayAssignmentsRepository, + @InjectRepository(BudgetItems) private readonly budgetItemsRepo: BudgetItemsRepository, ) {} async verifyTripAccess(tripId: string | number, userId: number) { @@ -142,22 +187,39 @@ export class FilesService { * (or a file_link) at trip B's reservation id and read the title back. Returns * the first field that escapes `tripId`, or null when every supplied id belongs * to the trip. Absent / null / zero ids are ignored (they clear the link). + * + * FL1-4 (`findForeignLinkTarget`'s four-table dispatch) are each now one + * `findTripId(id)` method on the target's OWN repository (R12 — no + * interpolated/dynamic table-name dispatch): the id is parsed once via + * `toRowId` here and compared against the row's own `trip_id` — a + * non-canonical id or a row in a different trip both read as "foreign" + * (rule 15's accepted narrowing: a non-canonical id can never have been + * produced by our own client). */ async findForeignLinkTarget( tripId: string | number, opts: { reservation_id?: string | number | null; assignment_id?: string | number | null; place_id?: string | number | null; budget_item_id?: string | number | null } ): Promise<'reservation_id' | 'assignment_id' | 'place_id' | 'budget_item_id' | null> { - if (opts.reservation_id && !this.db.get('SELECT 1 FROM reservations WHERE id = ? AND trip_id = ?', opts.reservation_id, tripId)) { - return 'reservation_id'; + const tripIdNum = toRowId(tripId); + if (opts.reservation_id) { + const idNum = toRowId(opts.reservation_id); + const rowTripId = idNum === null ? undefined : await this.reservationsRepo.findTripId(idNum); + if (rowTripId === undefined || rowTripId !== tripIdNum) return 'reservation_id'; } - if (opts.place_id && !this.db.get('SELECT 1 FROM places WHERE id = ? AND trip_id = ?', opts.place_id, tripId)) { - return 'place_id'; + if (opts.place_id) { + const idNum = toRowId(opts.place_id); + const rowTripId = idNum === null ? undefined : await this.placesRepo.findTripId(idNum); + if (rowTripId === undefined || rowTripId !== tripIdNum) return 'place_id'; } - if (opts.assignment_id && !this.db.get('SELECT 1 FROM day_assignments a JOIN days d ON a.day_id = d.id WHERE a.id = ? AND d.trip_id = ?', opts.assignment_id, tripId)) { - return 'assignment_id'; + if (opts.assignment_id) { + const idNum = toRowId(opts.assignment_id); + const rowTripId = idNum === null ? undefined : await this.dayAssignmentsRepo.findTripId(idNum); + if (rowTripId === undefined || rowTripId !== tripIdNum) return 'assignment_id'; } - if (opts.budget_item_id && !this.db.get('SELECT 1 FROM budget_items WHERE id = ? AND trip_id = ?', opts.budget_item_id, tripId)) { - return 'budget_item_id'; + if (opts.budget_item_id) { + const idNum = toRowId(opts.budget_item_id); + const rowTripId = idNum === null ? undefined : await this.budgetItemsRepo.findTripId(idNum); + if (rowTripId === undefined || rowTripId !== tripIdNum) return 'budget_item_id'; } return null; } @@ -166,12 +228,18 @@ export class FilesService { // CRUD // --------------------------------------------------------------------------- - async getFileById(id: string | number, tripId: string | number): Promise { - return this.db.get('SELECT * FROM trip_files WHERE id = ? AND trip_id = ?', id, tripId); + async getFileById(id: string | number, tripId: string | number): Promise { + const idNum = toRowId(id); + const tripIdNum = toRowId(tripId); + if (idNum === null || tripIdNum === null) return undefined; + return await this.tripFilesRepo.findInTrip(idNum, tripIdNum); } - async getDeletedFile(id: string | number, tripId: string | number): Promise { - return this.db.get('SELECT * FROM trip_files WHERE id = ? AND trip_id = ? AND deleted_at IS NOT NULL', id, tripId); + async getDeletedFile(id: string | number, tripId: string | number): Promise { + const idNum = toRowId(id); + const tripIdNum = toRowId(tripId); + if (idNum === null || tripIdNum === null) return undefined; + return await this.tripFilesRepo.findDeletedInTrip(idNum, tripIdNum); } /** @@ -233,20 +301,15 @@ export class FilesService { } async listFiles(tripId: string | number, showTrash: boolean) { - const where = showTrash - ? 'f.trip_id = ? AND f.deleted_at IS NOT NULL AND f.message_id IS NULL' - : 'f.trip_id = ? AND f.deleted_at IS NULL AND f.message_id IS NULL'; - const files = this.db.all(`${FILE_SELECT} WHERE ${where} ORDER BY f.starred DESC, f.created_at DESC`, tripId); + const tripIdNum = toRowId(tripId) ?? -1; + const files = await this.tripFilesRepo.listForTrip(tripIdNum, showTrash); const fileIds = files.map(f => f.id); - const linksMap: Record = {}; - if (fileIds.length > 0) { - const placeholders = fileIds.map(() => '?').join(','); - const links = this.db.all(`SELECT file_id, reservation_id, place_id, budget_item_id FROM file_links WHERE file_id IN (${placeholders})`, ...fileIds); - for (const link of links) { - if (!linksMap[link.file_id]) linksMap[link.file_id] = []; - linksMap[link.file_id].push(link); - } + const links = await this.fileLinksRepo.listForFiles(fileIds); + const linksMap: Record = {}; + for (const link of links) { + if (!linksMap[link.file_id]) linksMap[link.file_id] = []; + linksMap[link.file_id].push(link); } return files.map(f => { @@ -260,84 +323,87 @@ export class FilesService { }); } + /** R2: `createFile`'s `trip_files` insert and its conditional `file_links` insert (FL9+FL10) run inside one transaction. */ async createFile( tripId: string | number, file: { filename: string; originalname: string; size: number; mimetype: string }, uploadedBy: number, opts: { place_id?: string | number | null; reservation_id?: string | number | null; budget_item_id?: string | number | null; description?: string | null } ) { - const result = this.db.run(` - INSERT INTO trip_files (trip_id, place_id, reservation_id, filename, original_name, file_size, mime_type, description, uploaded_by) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) - `, - tripId, - opts.place_id || null, - opts.reservation_id || null, - file.filename, - file.originalname, - file.size, - file.mimetype, - opts.description || null, - uploadedBy - ); + const newId = await this.uow.transactional(async () => { + const id = await this.tripFilesRepo.insertFile({ + trip_id: tripId, + place_id: coerceLinkId(opts.place_id), + reservation_id: coerceLinkId(opts.reservation_id), + filename: file.filename, + original_name: file.originalname, + file_size: file.size, + mime_type: file.mimetype, + description: opts.description || null, + uploaded_by: uploadedBy, + }); - if (opts.budget_item_id) { - this.db.run('INSERT OR IGNORE INTO file_links (file_id, budget_item_id) VALUES (?, ?)', result.lastInsertRowid, opts.budget_item_id); - } + const budgetItemId = coerceLinkId(opts.budget_item_id); + if (budgetItemId) { + await this.fileLinksRepo.insertIgnore({ file_id: id, budget_item_id: budgetItemId }); + } + return id; + }); - const created = this.db.get(`${FILE_SELECT} WHERE f.id = ?`, result.lastInsertRowid)!; - return formatFile(created); + const created = await this.tripFilesRepo.findByIdWithJoins(newId); + return formatFile(created!); } + /** R2: `updateFile`'s field update and its conditional `file_links` insert/delete (FL12+FL13+FL14) run inside one transaction. */ async updateFile( id: string | number, - current: TripFile, + current: TripFileRow, updates: { description?: string; place_id?: string | number | null; reservation_id?: string | number | null; budget_item_id?: string | number | null } ) { - this.db.run(` - UPDATE trip_files SET - description = ?, - place_id = ?, - reservation_id = ? - WHERE id = ? - `, - updates.description !== undefined ? (updates.description || null) : current.description, - updates.place_id !== undefined ? (updates.place_id || null) : current.place_id, - updates.reservation_id !== undefined ? (updates.reservation_id || null) : current.reservation_id, - id - ); + const idNum = toRowId(id) ?? -1; - if (updates.budget_item_id !== undefined) { - if (updates.budget_item_id) { - this.db.run('INSERT OR IGNORE INTO file_links (file_id, budget_item_id) VALUES (?, ?)', id, updates.budget_item_id); - } else { - this.db.run('DELETE FROM file_links WHERE file_id = ? AND budget_item_id IS NOT NULL', id); + await this.uow.transactional(async () => { + await this.tripFilesRepo.updateFile(idNum, { + description: updates.description !== undefined ? (updates.description || null) : (current.description ?? null), + place_id: updates.place_id !== undefined ? coerceLinkId(updates.place_id) : (current.place_id ?? null), + reservation_id: updates.reservation_id !== undefined ? coerceLinkId(updates.reservation_id) : (current.reservation_id ?? null), + }); + + if (updates.budget_item_id !== undefined) { + const budgetItemId = coerceLinkId(updates.budget_item_id); + if (budgetItemId) { + await this.fileLinksRepo.insertIgnore({ file_id: idNum, budget_item_id: budgetItemId }); + } else { + await this.fileLinksRepo.clearBudgetLink(idNum); + } } - } + }); - const updated = this.db.get(`${FILE_SELECT} WHERE f.id = ?`, id)!; - return formatFile(updated); + const updated = await this.tripFilesRepo.findByIdWithJoins(idNum); + return formatFile(updated!); } async toggleStarred(id: string | number, currentStarred: number | undefined) { + const idNum = toRowId(id) ?? -1; const newStarred = currentStarred ? 0 : 1; - this.db.run('UPDATE trip_files SET starred = ? WHERE id = ?', newStarred, id); + await this.tripFilesRepo.setStarred(idNum, newStarred); - const updated = this.db.get(`${FILE_SELECT} WHERE f.id = ?`, id)!; - return formatFile(updated); + const updated = await this.tripFilesRepo.findByIdWithJoins(idNum); + return formatFile(updated!); } async softDeleteFile(id: string | number) { - this.db.run('UPDATE trip_files SET deleted_at = CURRENT_TIMESTAMP WHERE id = ?', id); + await this.tripFilesRepo.softDelete(toRowId(id) ?? -1); } async restoreFile(id: string | number) { - this.db.run('UPDATE trip_files SET deleted_at = NULL WHERE id = ?', id); - const restored = this.db.get(`${FILE_SELECT} WHERE f.id = ?`, id)!; - return formatFile(restored); + const idNum = toRowId(id) ?? -1; + await this.tripFilesRepo.restore(idNum); + const restored = await this.tripFilesRepo.findByIdWithJoins(idNum); + return formatFile(restored!); } - async permanentDeleteFile(file: TripFile): Promise { + async permanentDeleteFile(file: TripFileRow): Promise { // storage.delete is idempotent on a missing object (the old rm force:true // contract). Only drop the DB row when the delete either succeeded or the // object was already gone — otherwise a permission / ENOSPC failure @@ -348,11 +414,13 @@ export class FilesService { console.error(`[files] unlink failed for ${file.filename}, keeping DB row:`, e); throw e; } - this.db.run('DELETE FROM trip_files WHERE id = ?', file.id); + await this.tripFilesRepo.deleteById(file.id); } + /** R2: the storage-delete loop stays outside any transaction (rule 24); only the final bulk `trip_files` delete (FL23) runs inside one. */ async emptyTrash(tripId: string | number): Promise { - const trashed = this.db.all('SELECT * FROM trip_files WHERE trip_id = ? AND deleted_at IS NOT NULL', tripId); + const tripIdNum = toRowId(tripId) ?? -1; + const trashed = await this.tripFilesRepo.listTrashed(tripIdNum); // Collect successful IDs separately so we only DELETE rows whose disk // content was actually removed — failing unlinks keep their DB row // and a retry via the single-file delete path can try again. @@ -360,14 +428,15 @@ export class FilesService { await Promise.all(trashed.map(async (file) => { try { await this.storage.delete('files', path.basename(file.filename)); - successfullyUnlinked.push(Number(file.id)); + successfullyUnlinked.push(file.id); } catch (e) { console.error(`[files] unlink failed for ${file.filename}, keeping DB row:`, e); } })); if (successfullyUnlinked.length > 0) { - const placeholders = successfullyUnlinked.map(() => '?').join(','); - this.db.run(`DELETE FROM trip_files WHERE id IN (${placeholders})`, ...successfullyUnlinked); + await this.uow.transactional(async () => { + await this.tripFilesRepo.deleteMany(successfullyUnlinked); + }); } return successfullyUnlinked.length; } @@ -383,22 +452,22 @@ export class FilesService { fileId: string | number, opts: { reservation_id?: string | number | null; assignment_id?: string | number | null; place_id?: string | number | null; budget_item_id?: string | number | null } ) { - this.db.run('INSERT OR IGNORE INTO file_links (file_id, reservation_id, assignment_id, place_id, budget_item_id) VALUES (?, ?, ?, ?, ?)', - fileId, opts.reservation_id || null, opts.assignment_id || null, opts.place_id || null, opts.budget_item_id || null - ); - return this.db.all('SELECT * FROM file_links WHERE file_id = ?', fileId); + const idNum = toRowId(fileId) ?? -1; + await this.fileLinksRepo.insertIgnore({ + file_id: idNum, + reservation_id: coerceLinkId(opts.reservation_id), + assignment_id: coerceLinkId(opts.assignment_id), + place_id: coerceLinkId(opts.place_id), + budget_item_id: coerceLinkId(opts.budget_item_id), + }); + return await this.fileLinksRepo.listForFile(idNum); } async deleteFileLink(linkId: string | number, fileId: string | number) { - this.db.run('DELETE FROM file_links WHERE id = ? AND file_id = ?', linkId, fileId); + await this.fileLinksRepo.deleteById(toRowId(linkId) ?? -1, toRowId(fileId) ?? -1); } async getFileLinks(fileId: string | number) { - return this.db.all(` - SELECT fl.*, r.title as reservation_title - FROM file_links fl - LEFT JOIN reservations r ON fl.reservation_id = r.id - WHERE fl.file_id = ? - `, fileId); + return await this.fileLinksRepo.listForFileWithReservationTitle(toRowId(fileId) ?? -1); } } diff --git a/server/tests/helpers/files-repos.ts b/server/tests/helpers/files-repos.ts new file mode 100644 index 000000000..1437b7b92 --- /dev/null +++ b/server/tests/helpers/files-repos.ts @@ -0,0 +1,40 @@ +import type Database from 'better-sqlite3'; +import { sharedTestOrm } from './test-uow'; +import { TripFiles } from '../../src/db/entities/TripFiles.entity'; +import type { TripFilesRepository } from '../../src/db/repositories/TripFiles.repository'; +import { FileLinks } from '../../src/db/entities/FileLinks.entity'; +import type { FileLinksRepository } from '../../src/db/repositories/FileLinks.repository'; +import { BudgetItems } from '../../src/db/entities/BudgetItems.entity'; +import type { BudgetItemsRepository } from '../../src/db/repositories/BudgetItems.repository'; + +/** + * Plan 3e Task 1 (`FilesService`) test-only repository factories, bound to a + * suite's own better-sqlite3 handle via the SAME memoised `sharedTestOrm` + * `test-uow.ts`'s helpers use — required so a repository built here and a + * `UnitOfWork` built from `createTestUnitOfWork` (that file) resolve the + * IDENTICAL context-resolving `EntityManager` (a repository from a second, + * independent ORM instance would write outside the open transaction and + * deadlock on Kysely's connection mutex — `test-uow.ts`'s own docstring). + * + * A NEW file rather than an addition to `test-uow.ts`: that file is Plan 3e + * Task 6's named file set in this window (`tests/helpers/{mcp-test- + * controllers,plugin-host,test-uow}.ts`), and `sharedTestOrm`/`t.repo(...)` + * are exported precisely so a later task's tests don't need to edit it to + * get a repository this way. `ReservationsRepository`/`PlacesRepository`/ + * `DayAssignmentsRepository`/`UsersRepository`/`AppSettingsRepository` + * already have their own factories in `test-uow.ts` (`createTestReservationsRepo` + * etc.) and are reused directly from there — only `TripFilesRepository`, + * `FileLinksRepository` and `BudgetItemsRepository` are new here. + */ +export function createTestTripFilesRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(TripFiles)); +} + +export function createTestFileLinksRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(FileLinks)); +} + +/** `BudgetItemsRepository` — Task 2 (budget) owns every method appended to the production file; this task only needs `findTripId` (R12). */ +export function createTestBudgetItemsRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(BudgetItems)); +} diff --git a/server/tests/unit/files/files.rpc.test.ts b/server/tests/unit/files/files.rpc.test.ts index 8d7b08650..d2067fee9 100644 --- a/server/tests/unit/files/files.rpc.test.ts +++ b/server/tests/unit/files/files.rpc.test.ts @@ -20,6 +20,7 @@ import { FilesModule } from '../../../src/nest/files/files.module'; import { FilesService } from '../../../src/nest/files/files.service'; import type { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import type { DatabaseService } from '../../../src/nest/database/database.service'; +import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import type { AddonsService } from '../../../src/nest/addons/addons.service'; import type { StorageService } from '../../../src/nest/storage/storage.service'; @@ -47,8 +48,11 @@ function build(opts: { file?: Record | undefined; foreign?: str } as unknown as FilesService & Record>; const db = { canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)), - prepare: vi.fn(() => ({ get: () => ({ role: 'user', email: 'real@example.test' }) })), + // Still needed by PluginGuards' own role lookup (requireTripEdit) — unrelated to FL28. + prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })), } as unknown as DatabaseService; + // FL28 — `SELECT email FROM users WHERE id = ?`, now `UsersRepository.getEmail`. + const usersRepo = { getEmail: vi.fn(async () => 'real@example.test') } as unknown as UsersRepository; const permissions = { checkPermission: vi.fn((action: string) => (opts.allow ? opts.allow(action) : true)), } as unknown as PermissionsService; @@ -66,7 +70,7 @@ function build(opts: { file?: Record | undefined; foreign?: str // keep exercising that code rather than a stub of it. (files as unknown as { readContent: FilesService['readContent'] }).readContent = FilesService.prototype.readContent.bind({ getFileById: files.getFileById, storage } as unknown as FilesService); - const rpc = new FilesRpc(files, realtime, db, guards, storage); + const rpc = new FilesRpc(files, realtime, usersRepo, guards, storage); const host = (...grants: string[]) => new PluginRpcHost('p', new Set(grants), makeDeps(), createTestPluginRegistry([rpc])); return { files, realtime, permissions, storage, host }; } @@ -255,10 +259,10 @@ describe('FilesRpc writes', () => { process.env.DEMO_MODE = 'true'; try { const f = build(); - // The demo guard resolves the uploader's email; user 9 is the demo account. const db = { canAccessTrip: vi.fn(async () => ({ id: 1, user_id: 42 })), - prepare: vi.fn(() => ({ get: (id: number) => (id === 9 ? { role: 'user', email: 'demo@trek.app' } : { role: 'user', email: 'real@example.test' }) })), + // Still needed by PluginGuards' own role lookup — unrelated to FL28. + prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })), } as unknown as DatabaseService; const guards = new PluginGuards( db, @@ -270,7 +274,11 @@ describe('FilesRpc writes', () => { createFile: vi.fn(() => ({ id: 130 })), } as unknown as FilesService; const storage = { getStream: vi.fn(), put: vi.fn(async () => undefined) } as unknown as StorageService; - const rpc = new FilesRpc(files, { broadcast: vi.fn() } as unknown as RealtimeService, db, guards, storage); + // The demo guard resolves the uploader's email; user 9 is the demo account. + const usersRepo = { + getEmail: vi.fn(async (id: number) => (id === 9 ? 'demo@trek.app' : 'real@example.test')), + } as unknown as UsersRepository; + const rpc = new FilesRpc(files, { broadcast: vi.fn() } as unknown as RealtimeService, usersRepo, guards, storage); const host = new PluginRpcHost('p', new Set(['db:write:files']), makeDeps(), createTestPluginRegistry([rpc])); const input = { name: 'a.pdf', content_base64: b64('x') }; const denied = (await host.dispatch(req('files.create', { tripId: 1, input }), 9)) as RpcError; diff --git a/server/tests/unit/nest/files.service.test.ts b/server/tests/unit/nest/files.service.test.ts index 96dcd6dec..f7ff3ba1f 100644 --- a/server/tests/unit/nest/files.service.test.ts +++ b/server/tests/unit/nest/files.service.test.ts @@ -57,9 +57,12 @@ import type { Request } from 'express'; import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; -import { createUser, createTrip, addTripMember, createPlace, createReservation, createDay, createDayAssignment, setAppSetting } from '../../helpers/factories'; +import { createUser, createTrip, addTripMember, createPlace, createReservation, createDay, createDayAssignment, setAppSetting, createCollabNote } from '../../helpers/factories'; import { DatabaseService, type TripAccess } from '../../../src/nest/database/database.service'; -import { sharedTestOrm } from '../../helpers/test-uow'; +import { sharedTestOrm, createTestUnitOfWork, createTestAppSettingsRepo, createTestReservationsRepo, createTestPlacesRepo, createTestDayAssignmentsRepo } from '../../helpers/test-uow'; +import { createTestTripFilesRepo, createTestFileLinksRepo, createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import type { TripFilesRepository } from '../../../src/db/repositories/TripFiles.repository'; +import type { FileLinksRepository } from '../../../src/db/repositories/FileLinks.repository'; import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { FilesService } from '../../../src/nest/files/files.service'; import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service'; @@ -72,7 +75,7 @@ import { isVideoMime, isVideoExtension, } from '../../../src/nest/files/files.constants'; -import type { TripFile, User } from '../../../src/types'; +import type { User } from '../../../src/types'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; import type { EntityManager } from '@mikro-orm/core'; @@ -90,19 +93,48 @@ const storageStub = { delete: storageDelete } as unknown as import('../../../src // `Users` specifically. const getRepository = vi.fn(() => ({})); const emStub = { getRepository } as unknown as EntityManager; -const svc = new FilesService(new DatabaseService(testDb), permissionsStub, new RealtimeService(), new EphemeralTokenService(), storageStub, emStub); + +// Constructed in `beforeAll` (async — the repositories below resolve through +// `sharedTestOrm`), not at module load: Plan 3e Task 1 adds the `UnitOfWork` +// and the five repositories `FilesService` now needs. `tripFilesRepo`/ +// `fileLinksRepo` are kept as named bindings (not only inside `svc`) so the +// R2 rollback tests below can spy on them directly, the same shape +// `reservations.service.test.ts`'s `RESV-FIX-001/002` already use. +let svc: FilesService; +let tripFilesRepo: TripFilesRepository; +let fileLinksRepo: FileLinksRepository; beforeAll(() => { createTables(testDb); runMigrations(testDb); }); +beforeAll(async () => { + tripFilesRepo = await createTestTripFilesRepo(testDb); + fileLinksRepo = await createTestFileLinksRepo(testDb); + svc = new FilesService( + new DatabaseService(testDb), + permissionsStub, + new RealtimeService(), + new EphemeralTokenService(), + storageStub, + emStub, + await createTestUnitOfWork(testDb), + tripFilesRepo, + fileLinksRepo, + await createTestReservationsRepo(testDb), + await createTestPlacesRepo(testDb), + await createTestDayAssignmentsRepo(testDb), + await createTestBudgetItemsRepo(testDb), + ); +}); + beforeEach(async () => { resetTestDb(testDb); vi.clearAllMocks(); - // Plan 3c Task 0b: `svc`'s `DatabaseService` is constructed at module load, - // before any `beforeAll`/`beforeEach` can resolve a real `EntityManager` — - // spied directly on that instance, routed to a real `DatabaseService` built + // Plan 3c Task 0b: `svc`'s `DatabaseService` is constructed in `beforeAll`, + // before any `beforeEach` can resolve a real `EntityManager` — spied + // directly on that instance, routed to a real `DatabaseService` built // with one. Re-applied every test (not `beforeAll`) because `afterEach` // below `vi.restoreAllMocks()`s it away after each one. const real = new DatabaseService(testDb, (await sharedTestOrm(testDb)).em); @@ -191,20 +223,20 @@ describe('files.constants', () => { describe('AllowedFileTypesService.get', () => { it('FILE-SVC-006: returns the app_settings value when set', async () => { setAppSetting(testDb, 'allowed_file_types', 'pdf,txt'); - expect(await new AllowedFileTypesService(new DatabaseService(testDb)).get()).toBe('pdf,txt'); + expect(await new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)).get()).toBe('pdf,txt'); }); it('FILE-SVC-007: returns the default when the row is absent', async () => { - expect(await new AllowedFileTypesService(new DatabaseService(testDb)).get()).toBe(DEFAULT_ALLOWED_EXTENSIONS); + expect(await new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)).get()).toBe(DEFAULT_ALLOWED_EXTENSIONS); }); it('FILE-SVC-008: returns the default for an empty value (|| coercion, not ??)', async () => { setAppSetting(testDb, 'allowed_file_types', ''); - expect(await new AllowedFileTypesService(new DatabaseService(testDb)).get()).toBe(DEFAULT_ALLOWED_EXTENSIONS); + expect(await new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)).get()).toBe(DEFAULT_ALLOWED_EXTENSIONS); }); it('FILE-SVC-009: returns the default when the query throws (no app_settings table)', async () => { - expect(await new AllowedFileTypesService(new DatabaseService(bareDb)).get()).toBe(DEFAULT_ALLOWED_EXTENSIONS); + expect(await new AllowedFileTypesService(await createTestAppSettingsRepo(bareDb)).get()).toBe(DEFAULT_ALLOWED_EXTENSIONS); }); }); @@ -359,6 +391,15 @@ describe('createFile', () => { expect(file.file_size).toBe(99); expect(file.uploaded_by).toBe(user.id); }); + + it('FILE-SVC-060 (R2): a failing file_links insert rolls back the trip_files row too', async () => { + const { user, trip } = seedTrip(); + const item = Number(testDb.prepare('INSERT INTO budget_items (trip_id, name) VALUES (?, ?)').run(trip.id, 'Dinner').lastInsertRowid); + const spy = vi.spyOn(fileLinksRepo, 'insertIgnore').mockRejectedValueOnce(new Error('boom')); + await expect(makeFile(trip.id, user.id, {}, { budget_item_id: item })).rejects.toThrow('boom'); + expect(testDb.prepare('SELECT COUNT(*) c FROM trip_files WHERE trip_id = ?').get(trip.id)).toEqual({ c: 0 }); + spy.mockRestore(); + }); }); describe('updateFile', () => { @@ -383,6 +424,20 @@ describe('updateFile', () => { expect(updated.place_id).toBeNull(); expect(updated.reservation_id).toBeNull(); }); + + it('FILE-SVC-061 (R2): a failing file_links swap rolls back the description/place/reservation update too', async () => { + const { user, trip } = seedTrip(); + const item = Number(testDb.prepare('INSERT INTO budget_items (trip_id, name) VALUES (?, ?)').run(trip.id, 'Dinner').lastInsertRowid); + const place = createPlace(testDb, trip.id); + const file = await makeFile(trip.id, user.id, {}, { description: 'old' }); + const current = (await svc.getFileById(file.id, trip.id))!; + const spy = vi.spyOn(fileLinksRepo, 'insertIgnore').mockRejectedValueOnce(new Error('boom')); + await expect(svc.updateFile(file.id, current, { description: 'new', place_id: String(place.id), budget_item_id: item })).rejects.toThrow('boom'); + const row = testDb.prepare('SELECT description, place_id FROM trip_files WHERE id = ?').get(file.id) as Record; + expect(row.description).toBe('old'); + expect(row.place_id).toBeNull(); + spy.mockRestore(); + }); }); describe('toggleStarred / softDeleteFile / restoreFile', () => { @@ -420,7 +475,7 @@ describe('permanentDeleteFile', () => { const { user, trip } = seedTrip(); const file = await makeFile(trip.id, user.id, { filename: 'on-disk.pdf' }); storageDelete.mockResolvedValue(undefined); - await svc.permanentDeleteFile(await svc.getFileById(file.id, trip.id) as TripFile); + await svc.permanentDeleteFile((await svc.getFileById(file.id, trip.id))!); expect(storageDelete).toHaveBeenCalledWith('files', 'on-disk.pdf'); expect(await svc.getFileById(file.id, trip.id)).toBeUndefined(); }); @@ -431,7 +486,7 @@ describe('permanentDeleteFile', () => { const boom = new Error('EACCES'); storageDelete.mockRejectedValue(boom); const err = vi.spyOn(console, 'error').mockImplementation(() => {}); - await expect(svc.permanentDeleteFile(await svc.getFileById(file.id, trip.id) as TripFile)).rejects.toThrow('EACCES'); + await expect(svc.permanentDeleteFile((await svc.getFileById(file.id, trip.id))!)).rejects.toThrow('EACCES'); expect(err).toHaveBeenCalledWith('[files] unlink failed for stuck.pdf, keeping DB row:', boom); expect(await svc.getFileById(file.id, trip.id)).toBeDefined(); }); @@ -475,6 +530,17 @@ describe('emptyTrash', () => { await expect(svc.emptyTrash(trip.id)).resolves.toBe(0); expect(storageDelete).not.toHaveBeenCalled(); }); + + it('FILE-SVC-062 (R2): a failing bulk delete leaves every trashed row in place', async () => { + const { user, trip } = seedTrip(); + const a = await makeFile(trip.id, user.id, { filename: 'a.pdf' }); + await svc.softDeleteFile(a.id); + storageDelete.mockResolvedValue(undefined); + const spy = vi.spyOn(tripFilesRepo, 'deleteMany').mockRejectedValueOnce(new Error('boom')); + await expect(svc.emptyTrash(trip.id)).rejects.toThrow('boom'); + expect(testDb.prepare('SELECT COUNT(*) c FROM trip_files WHERE id = ?').get(a.id)).toEqual({ c: 1 }); + spy.mockRestore(); + }); }); // ── findForeignLinkTarget ───────────────────────────────────────────────────── @@ -552,9 +618,22 @@ describe('createFileLink / deleteFileLink / getFileLinks', () => { }); it('FILE-SVC-030: an insert error propagates (post-migration fix: no silent swallow)', async () => { + // Plan 3e Task 1 deviation: the legacy raw statement bound whatever JS + // value the caller passed straight into the driver, so a non-primitive + // (an object) reached better-sqlite3's bind and threw there. The write + // is now typed `number | null` end to end (`FileLinksRepository + // .insertIgnore`), so a value that shape can never reach the repository + // at all — the `toRowId`-backed `coerceLinkId` narrows it to `null` + // (no link) instead, the same accepted narrowing every other + // non-canonical id gets. What the "no silent swallow" fix still means is + // tested directly against the repository call: a genuine insert failure + // (any error, not a type mismatch this layer now prevents) propagates + // rather than being caught and turned into a success-shaped links list. const { user, trip } = seedTrip(); const file = await makeFile(trip.id, user.id); - await expect(svc.createFileLink(file.id, { reservation_id: { bad: true } as never })).rejects.toThrow(); + const spy = vi.spyOn(fileLinksRepo, 'insertIgnore').mockRejectedValueOnce(new Error('boom')); + await expect(svc.createFileLink(file.id, { reservation_id: 1 })).rejects.toThrow('boom'); + spy.mockRestore(); }); it('FILE-SVC-031: deleteFileLink is scoped to (id AND file_id)', async () => { @@ -571,6 +650,86 @@ describe('createFileLink / deleteFileLink / getFileLinks', () => { }); }); +// ── Plan 3e Task 1 — full-key parity on the fully-seeded read models ────────── +// Rule 19: one `toEqual()` +// per converted read model, on rows with every optional column populated +// (incl. all six `trip_files` `persist(false)` mirrors) and both trashed and +// live rows. + +describe('read-model parity: raw SQL vs. the repository-backed reads', () => { + it('FILE-SVC-070: getFileById (FL5) is byte-identical to `SELECT * FROM trip_files WHERE id = ? AND trip_id = ?`, all six persist(false) mirrors set', async () => { + const { user, trip } = seedTrip(); + const reservation = createReservation(testDb, trip.id); + const place = createPlace(testDb, trip.id); + const note = createCollabNote(testDb, trip.id, user.id); + const messageId = Number( + testDb.prepare('INSERT INTO collab_messages (trip_id, user_id, text) VALUES (?, ?, ?)').run(trip.id, user.id, 'hi').lastInsertRowid, + ); + // Raw insert, not svc.createFile: note_id/message_id are set by OTHER + // domains (collab), never by FilesService itself — this seeds every + // persist(false) mirror at once to prove the Kysely `selectAll()` read + // carries all six, the trap the class docstring names. + const inserted = testDb.prepare(` + INSERT INTO trip_files (trip_id, place_id, reservation_id, filename, original_name, file_size, mime_type, description, note_id, uploaded_by, starred, message_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `).run(trip.id, place.id, reservation.id, 'a.pdf', 'A.pdf', 10, 'application/pdf', 'a note', note.id, user.id, 1, messageId); + const id = Number(inserted.lastInsertRowid); + + const legacy = testDb.prepare('SELECT * FROM trip_files WHERE id = ? AND trip_id = ?').get(id, trip.id); + expect(await svc.getFileById(id, trip.id)).toEqual(legacy); + }); + + it('FILE-SVC-071: listFiles (FL7, FILE_SELECT) is byte-identical to the legacy joined SELECT, live and trashed alike, with reservation+place+budget_item links all present', async () => { + const { user, trip } = seedTrip(); + const reservation = createReservation(testDb, trip.id, { title: 'Night train' }); + const place = createPlace(testDb, trip.id); + const item = Number(testDb.prepare('INSERT INTO budget_items (trip_id, name) VALUES (?, ?)').run(trip.id, 'Dinner').lastInsertRowid); + // `linked_*_ids` come from `file_links` rows (FL8), a SEPARATE mechanism + // from `trip_files.reservation_id`/`place_id` (FILE_SELECT's own join) — + // one `createFileLink` call can attach several targets to the same file + // at once (the legacy statement writes all four columns in one INSERT). + const live = await makeFile(trip.id, user.id); + await svc.createFileLink(live.id, { reservation_id: String(reservation.id), place_id: String(place.id), budget_item_id: item }); + const trashed = await makeFile(trip.id, user.id, { filename: 'gone.pdf' }); + await svc.softDeleteFile(trashed.id); + + const FILE_SELECT = ` + SELECT f.*, r.title as reservation_title, u.username as uploaded_by_name, u.avatar as uploaded_by_avatar + FROM trip_files f + LEFT JOIN reservations r ON f.reservation_id = r.id + LEFT JOIN users u ON f.uploaded_by = u.id + `; + const legacyLive = testDb.prepare(`${FILE_SELECT} WHERE f.id = ?`).get(live.id) as Record; + const legacyTrashed = testDb.prepare(`${FILE_SELECT} WHERE f.id = ?`).get(trashed.id) as Record; + + const activeRow = (await svc.listFiles(trip.id, false) as Record[]).find((f) => f.id === live.id)!; + const pickedActive = Object.fromEntries(Object.keys(legacyLive).map((k) => [k, activeRow[k]])); + expect(pickedActive).toEqual(legacyLive); + expect(activeRow.linked_reservation_ids).toEqual([reservation.id]); + expect(activeRow.linked_place_ids).toEqual([place.id]); + expect(activeRow.linked_budget_item_ids).toEqual([item]); + + const trashedRow = (await svc.listFiles(trip.id, true) as Record[]).find((f) => f.id === trashed.id)!; + const pickedTrashed = Object.fromEntries(Object.keys(legacyTrashed).map((k) => [k, trashedRow[k]])); + expect(pickedTrashed).toEqual(legacyTrashed); + }); + + it('FILE-SVC-072: getFileLinks (FL27) is byte-identical to the legacy joined SELECT', async () => { + const { user, trip } = seedTrip(); + const reservation = createReservation(testDb, trip.id, { title: 'Ferry' }); + const file = await makeFile(trip.id, user.id); + await svc.createFileLink(file.id, { reservation_id: String(reservation.id) }); + + const legacy = testDb.prepare(` + SELECT fl.*, r.title as reservation_title + FROM file_links fl + LEFT JOIN reservations r ON fl.reservation_id = r.id + WHERE fl.file_id = ? + `).all(file.id); + expect(await svc.getFileLinks(file.id)).toEqual(legacy); + }); +}); + // ── download auth & glue ────────────────────────────────────────────────────── describe('authenticateDownload', () => { @@ -648,7 +807,7 @@ describe('verifyTripAccess / can / files.bridge', () => { // list moved to a leaf service the multer factories inject, and the property // that matters is unchanged: an admin editing the list in settings applies // to the next upload, with no invalidation wiring. - const allowed = new AllowedFileTypesService(new DatabaseService(testDb)); + const allowed = new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)); setAppSetting(testDb, 'allowed_file_types', 'md,markdown'); expect(await allowed.get()).toBe('md,markdown'); });