diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 84897ac3c..1c8b324f7 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -262,11 +262,14 @@ predicted: `MulterModule.registerAsync` over an `AllowedFileTypesService` leaf, and `files.controller.ts` / `journey.controller.ts` build their multer options from that. -**Four bridges have no production consumer at all** — `days`, `packing`, -`photos`, `reservations`. Only their own delegation tests import them, and each -still builds a service outside the container at module load for nobody. They are -deletable, along with the comments in `days.service.ts`, `packing.service.ts` and -`reservations.service.ts` that still send readers to them. +`days`, `packing`, `photos` and `reservations` had no production consumer left +and are deleted. Each still built a service outside the container at module load +for nobody, and three domain docblocks were still sending readers to them. The +cases that exercised them kept their assertions and point at the service. + +Twelve bridges remain. When one loses its last outside-container consumer, +delete it in the same change rather than leaving it as a courtesy: an unused +bridge reads as a supported entry point. The nine fire-and-forget notification senders inject too. They were lazy `import('../notifications/notifications.bridge').then(({ send }) => …)` calls diff --git a/server/src/nest/days/days.bridge.ts b/server/src/nest/days/days.bridge.ts deleted file mode 100644 index afe9abae1..000000000 --- a/server/src/nest/days/days.bridge.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { RealtimeService } from '../realtime/realtime.service'; -import { DaysService } from './days.service'; -import { PermissionsService } from '../permissions/permissions.service'; -import { QueryHelpersService } from '../query-helpers/query-helpers.service'; - -/** - * Non-Nest entry point for the day domain — for code running OUTSIDE the - * Nest container (the legacy transports MCP registrar in src/mcp/tools/; - * the day MCP tools and resources moved to the DI-discovered days.mcp.ts, - * the transit tools to the DI-discovered transit.mcp.ts (which injects - * DaysService), the plugin RPC host injects DaysService via - * DaysRpc / AccommodationsRpc, and the trip domain folded into the DI-native - * TripsService, which injects DaysService — its bridge exports were pruned - * with it). Exports only the legacy services/dayService names still consumed - * outside the container, 1:1, so repointing a consumer is an import-path-only - * diff. Inside the container, inject DaysService instead. Delete this file - * when the transports registrar migrates. - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton. - */ -const days = new DaysService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new RealtimeService(), new QueryHelpersService(new DatabaseService(db))); - -export function getDay(id: string | number, tripId: string | number) { - return days.getDay(id, tripId); -} - -export function listDays(tripId: string | number) { - return days.list(tripId); -} diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index daf2e54df..0b0942136 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -56,9 +56,8 @@ export class DayReorderError extends Error {} * query per assignment. Trip access * rides DatabaseService.canAccessTrip; mutations use the * 'day_edit' permission; the WebSocket broadcast keeps its legacy call path. - * Non-Nest consumers (legacy tripService and the transit/transports MCP - * registrars) go through days.bridge.ts instead of importing this class - * directly. + * There are no non-Nest consumers left. days.bridge.ts served them and was + * deleted once the last one folded into the container. */ @Injectable() export class DaysService { diff --git a/server/src/nest/packing/packing.bridge.ts b/server/src/nest/packing/packing.bridge.ts deleted file mode 100644 index d67c1613e..000000000 --- a/server/src/nest/packing/packing.bridge.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { RealtimeService } from '../realtime/realtime.service'; -import { PackingService } from './packing.service'; -import { PermissionsService } from '../permissions/permissions.service'; -import { notificationsInstance } from '../notifications/notifications.instance'; - -/** - * Non-Nest entry point for the packing domain — for code running OUTSIDE the - * Nest container (the legacy packing-list prompt registrar in - * src/mcp/tools/prompts.ts; the packing MCP tools and resources moved to the - * DI-discovered packing.mcp.ts, the plugin RPC host injects PackingService - * via PackingRpc, and the trip domain folded into the DI-native - * TripsService, which injects PackingService). Exports only the legacy - * services/packingService names still consumed outside the container, 1:1, so - * repointing a consumer is an import-path-only diff. Inside the container, - * inject PackingService instead. Delete this file when the prompts registrar - * migrates. - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton. - */ -const packing = new PackingService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new RealtimeService(), notificationsInstance()); - -export function listItems(tripId: string | number, userId?: number) { - return packing.listItems(tripId, userId); -} diff --git a/server/src/nest/packing/packing.service.ts b/server/src/nest/packing/packing.service.ts index 16665994b..c55c04549 100644 --- a/server/src/nest/packing/packing.service.ts +++ b/server/src/nest/packing/packing.service.ts @@ -35,8 +35,8 @@ const BAG_COLORS = ['#6366f1', '#ec4899', '#f97316', '#10b981', '#06b6d4', '#8b5 * legacy call paths. Post-migration fixes over the legacy code: a single * 'Other' category default, bodyKeys-gated weight_limit_grams (explicit null * clears it), and transactions around every multi-statement write. The - * remaining non-Nest consumer (the legacy MCP prompts registrar) goes through - * packing.bridge.ts instead of importing this class directly. + * remaining non-Nest consumer went with the legacy MCP prompts registrar, and + * packing.bridge.ts was deleted with it. */ @Injectable() export class PackingService { diff --git a/server/src/nest/photos/photos.bridge.ts b/server/src/nest/photos/photos.bridge.ts deleted file mode 100644 index 50775dd06..000000000 --- a/server/src/nest/photos/photos.bridge.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { TrekPhotosRepository } from './trek-photos.repository'; - -/** - * Non-Nest entry point for the `trek_photos` store — for the modules still - * living in `src/services/` that only need to register or drop a row: - * journeyService (photo upload, Immich sync, entry cleanup) and the memories - * unifiedService. Exports the legacy photoResolverService names 1:1, so - * repointing a consumer is an import-path-only diff. Inside the container, - * inject TrekPhotosRepository instead. Delete this file when journeyService and - * the memories cluster fold. - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton, and the repository holds no state. - */ -const photos = new TrekPhotosRepository(new DatabaseService(db)); - -export function getOrCreateTrekPhoto(...args: Parameters) { - return photos.getOrCreate(...args); -} - -export function getOrCreateLocalTrekPhoto(...args: Parameters) { - return photos.getOrCreateLocal(...args); -} - -export function resolveTrekPhoto(photoId: number) { - return photos.resolve(photoId); -} - -export function setTrekPhotoProvider(...args: Parameters) { - return photos.setProvider(...args); -} - -export function deleteTrekPhotoIfOrphan(photoId: number): void { - photos.deleteIfOrphan(photoId); -} diff --git a/server/src/nest/reservations/reservations.bridge.ts b/server/src/nest/reservations/reservations.bridge.ts deleted file mode 100644 index 78c84ce98..000000000 --- a/server/src/nest/reservations/reservations.bridge.ts +++ /dev/null @@ -1,74 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { RealtimeService } from '../realtime/realtime.service'; -import { ReservationsService } from './reservations.service'; -import { PermissionsService } from '../permissions/permissions.service'; -import { BudgetService } from '../budget/budget.service'; -import { ExchangeRatesService } from '../budget/exchange-rates.service'; -import { notificationsInstance } from '../notifications/notifications.instance'; - -export type { EndpointInput, ReservationEndpoint, ReservationTraveler } from './reservations.service'; - -/** - * Non-Nest entry point for the reservations domain — for code running OUTSIDE - * the Nest container (the legacy tripService, the airtrail import/sync - * services, and the still-legacy transports MCP registrar; the reservation - * MCP tools and resource moved to the DI-discovered reservations.mcp.ts, the - * transit tools to the DI-discovered transit.mcp.ts (which injects - * ReservationsService), and the plugin RPC host injects ReservationsService - * via ReservationsRpc). Exports only the legacy services/reservationService - * names still consumed outside the container, 1:1, so repointing a consumer is - * an import-path-only diff. Inside the container, inject ReservationsService - * instead. Delete exports here as their consumers migrate. - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton. - */ -const reservations = new ReservationsService( - new DatabaseService(db), - new PermissionsService(new DatabaseService(db)), - new BudgetService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new ExchangeRatesService(), new RealtimeService()), - new RealtimeService(), - notificationsInstance(), -); - -export function listReservations(tripId: string | number) { - return reservations.list(tripId); -} - -export function loadEndpointsByTrip(tripId: string | number) { - return reservations.loadEndpointsByTrip(tripId); -} - -export function resyncReservationDays(tripId: string | number): void { - reservations.resyncReservationDays(tripId); -} - -export function createReservation(tripId: string | number, data: Parameters[1]) { - return reservations.create(tripId, data); -} - -export function getReservation(id: string | number, tripId: string | number) { - return reservations.getReservation(id, tripId); -} - -export function getReservationWithJoins(id: string | number) { - return reservations.getReservationWithJoins(id); -} - -export function updateReservation( - id: string | number, - tripId: string | number, - data: Parameters[2], - current: Parameters[3] -) { - return reservations.update(id, tripId, data, current); -} - -export function deleteReservation(id: string | number, tripId: string | number) { - return reservations.remove(id, tripId); -} - -export function notifyBookingChange(tripId: string | number, actorId: number, booking: string, type: string): void { - reservations.notifyBookingChange(tripId, actorId, booking, type); -} diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 6e54f5a71..33cd14788 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -123,10 +123,10 @@ type AccommodationTimesMeta = { * legacy call paths. The legacy route's budget side effects (auto-create / * update / delete a linked budget item) and the booking notification are * encapsulated here so the controller stays thin — behaviour is 1:1. - * Non-Nest consumers (legacy tripService, airtrail import/sync, the transit + - * transports MCP registrars) go through reservations.bridge.ts instead of - * importing this class directly; the plugin RPC host injects it via - * ReservationsRpc. + * Every consumer injects this class now. reservations.bridge.ts existed for + * the legacy tripService, airtrail import/sync and the transit/transports MCP + * registrars, all of which have folded in; the plugin RPC host reaches it + * through ReservationsRpc. */ @Injectable() export class ReservationsService { diff --git a/server/tests/integration/memories-synology.test.ts b/server/tests/integration/memories-synology.test.ts index bfb3a2efd..436a0f826 100644 --- a/server/tests/integration/memories-synology.test.ts +++ b/server/tests/integration/memories-synology.test.ts @@ -1186,7 +1186,15 @@ describe('Synology SSRF blocked error handling', () => { // ── Passphrase persistence fixes ───────────────────────────────────────────── -import { getOrCreateTrekPhoto, deleteTrekPhotoIfOrphan } from '../../src/nest/photos/photos.bridge'; +import { TrekPhotosRepository } from '../../src/nest/photos/trek-photos.repository'; +import { DatabaseService } from '../../src/nest/database/database.service'; +import { db as trekDb } from '../../src/db/database'; + +// Was photos.bridge, which existed for consumers outside the container and had +// none left. The repository is what it delegated to. +const trekPhotos = new TrekPhotosRepository(new DatabaseService(trekDb)); +const getOrCreateTrekPhoto = (...a: Parameters) => trekPhotos.getOrCreate(...a); +const deleteTrekPhotoIfOrphan = (id: number) => trekPhotos.deleteIfOrphan(id); import { decrypt_api_key } from '../../src/nest/common/crypto/apiKeyCrypto'; describe('trek_photos passphrase healing (SYNO-090)', () => { diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts index b3b100a83..66e39f618 100644 --- a/server/tests/unit/nest/days.service.test.ts +++ b/server/tests/unit/nest/days.service.test.ts @@ -56,7 +56,10 @@ import { createUser, createTrip, createDay, createPlace, createDayAssignment, cr import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { DaysService, DayReorderError, addDays } from '../../../src/nest/days/days.service'; -import { getDay as bridgeGetDay, listDays as bridgeListDays } from '../../../src/nest/days/days.bridge'; +// Was days.bridge, deleted with the other three that had no consumer outside the +// container. The assertions stayed; they point at the service now. +const bridgeGetDay = (id: string | number, tripId: string | number) => svc.getDay(id, tripId); +const bridgeListDays = (tripId: string | number) => svc.list(tripId); import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import { AccommodationsService } from '../../../src/nest/accommodations/accommodations.service'; @@ -252,7 +255,7 @@ describe('remove', () => { // consumer (legacy tripService) folded into the DI-native TripsService — // 029/031/032 pin the same behavior on the service. -describe('days.bridge', () => { +describe('DaysService — the surface the deleted bridge exposed', () => { it('DAY-SVC-027 — getDay delegates to DaysService.getDay', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); diff --git a/server/tests/unit/nest/packing.service.test.ts b/server/tests/unit/nest/packing.service.test.ts index 837809db8..88131ba1a 100644 --- a/server/tests/unit/nest/packing.service.test.ts +++ b/server/tests/unit/nest/packing.service.test.ts @@ -63,7 +63,9 @@ import { createUser, createAdmin, createTrip, addTripMember } from '../../helper import { DatabaseService } from '../../../src/nest/database/database.service'; import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { PackingService } from '../../../src/nest/packing/packing.service'; -import { listItems as bridgeListItems } from '../../../src/nest/packing/packing.bridge'; +// Was packing.bridge, deleted with the other three that had no consumer outside the +// container. The assertions stayed; they point at the service now. +const bridgeListItems = (tripId: string | number, viewerId?: number) => svc.listItems(tripId, viewerId); import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { notificationsStub } from '../../helpers/notifications'; @@ -773,7 +775,7 @@ describe('notifyTagged', () => { // ── Bridge delegation ───────────────────────────────────────────────────────── -describe('packing.bridge', () => { +describe('PackingService — the surface the deleted bridge exposed', () => { it('PACK-SVC-050: listItems delegates to PackingService over the shared db Proxy', () => { const { user: owner } = createUser(testDb); const { user: other } = createUser(testDb); diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 15c77e97a..f63d33db8 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -2,7 +2,7 @@ * Unit tests for the DI-native ReservationsService — RESV-SVC-001+ (real-SQL * cases over the folded legacy services/reservationService.ts statements), * RES-TRAV-001..005 (moved 1:1 from tests/unit/services/reservationTravelers.test.ts) - * and RESV-BRIDGE-001..009 (one delegation case per reservations.bridge export). + * and RESV-BRIDGE-001..009, which kept their assertions when the bridge went. * Uses a real in-memory SQLite DB so the SQL logic — falsy-coercion defaults, * COALESCE update semantics, day derivation, the accommodation/budget cascades * and the TEXT accommodation_id normalization — is exercised faithfully. @@ -57,7 +57,26 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { ReservationsService } from '../../../src/nest/reservations/reservations.service'; import type { BudgetService } from '../../../src/nest/budget/budget.service'; -import * as bridge from '../../../src/nest/reservations/reservations.bridge'; +// Was reservations.bridge, deleted along with the other three that had no +// consumer outside the container. The cases below kept their assertions and +// point at the service the bridge delegated to; only the delegation itself is +// gone, and there was nothing left to delegate for. +const bridge = { + listReservations: (tripId: string | number) => svc.list(tripId), + loadEndpointsByTrip: (tripId: string | number) => svc.loadEndpointsByTrip(tripId), + resyncReservationDays: (tripId: string | number) => svc.resyncReservationDays(tripId), + createReservation: (tripId: string | number, data: Parameters[1]) => svc.create(tripId, data), + getReservation: (id: string | number, tripId: string | number) => svc.getReservation(id, tripId), + getReservationWithJoins: (id: string | number) => svc.getReservationWithJoins(id), + updateReservation: ( + id: string | number, + tripId: string | number, + data: Parameters[2], + current: Parameters[3], + ) => svc.update(id, tripId, data, current), + deleteReservation: (id: string | number, tripId: string | number) => svc.remove(id, tripId), + notifyBookingChange: (...a: Parameters) => svc.notifyBookingChange(...a), +}; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { notificationsStub } from '../../helpers/notifications'; @@ -513,7 +532,7 @@ describe('setReservationTravelers / loadTravelers (#1517)', () => { }); }); -describe('reservations.bridge (legacy-named delegation for outside-container consumers)', () => { +describe('ReservationsService — the surface the deleted bridge exposed', () => { it('RESV-BRIDGE-001: listReservations delegates to ReservationsService.list', () => { const { trip } = ownerTrip(); createReservation(testDb, trip.id, { title: 'A' }); diff --git a/server/tests/unit/services/trekPhotoMedia.test.ts b/server/tests/unit/services/trekPhotoMedia.test.ts index 1ac6b187f..f1efdd4e9 100644 --- a/server/tests/unit/services/trekPhotoMedia.test.ts +++ b/server/tests/unit/services/trekPhotoMedia.test.ts @@ -23,7 +23,16 @@ vi.mock('../../../src/config', () => ({ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { createUser } from '../../helpers/factories'; -import { getOrCreateLocalTrekPhoto, getOrCreateTrekPhoto, resolveTrekPhoto } from '../../../src/nest/photos/photos.bridge'; +import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { db as trekDb } from '../../../src/db/database'; + +// Was photos.bridge, deleted with the other three that had no consumer outside +// the container. These call the repository directly now. +const trekPhotos = new TrekPhotosRepository(new DatabaseService(trekDb)); +const getOrCreateTrekPhoto = (...a: Parameters) => trekPhotos.getOrCreate(...a); +const getOrCreateLocalTrekPhoto = (...a: Parameters) => trekPhotos.getOrCreateLocal(...a); +const resolveTrekPhoto = (id: number) => trekPhotos.resolve(id); beforeAll(() => { createTables(testDb); diff --git a/server/vitest.config.ts b/server/vitest.config.ts index f6d759f4f..64d80aa49 100644 --- a/server/vitest.config.ts +++ b/server/vitest.config.ts @@ -125,6 +125,26 @@ export default defineConfig({ 'src/nest/vacay/**/*.ts': { statements: 82, branches: 66, functions: 90, lines: 86 }, 'src/nest/weather/**/*.ts': { statements: 93, branches: 78, functions: 91, lines: 97 }, 'src/nest/**/*.ts': { statements: 80, branches: 80, functions: 80, lines: 80 }, + + // Outside the container. Coverage MEASURES src/**, but until now only + // src/nest/** was gated, so the boot path, the scheduler, the MCP + // handler and the migrations sat under no floor at all — which is + // exactly where the worst finding of the last audit lived (a swallowed + // EADDRINUSE that no test could have caught). These entries pin what is + // measured today so it cannot slide; they are floors to raise, not + // targets that have been met. + // + // src/demo/** is deliberately absent: it measures 0%, and a floor of + // zero asserts nothing. It needs tests before it needs a threshold. + 'src/app-config/**/*.ts': { statements: 99, branches: 93, functions: 99, lines: 99 }, + 'src/db/**/*.ts': { statements: 73, branches: 38, functions: 59, lines: 80 }, + 'src/mcp/**/*.ts': { statements: 58, branches: 43, functions: 63, lines: 60 }, + 'src/middleware/**/*.ts': { statements: 91, branches: 89, functions: 87, lines: 94 }, + 'src/systemNotices/**/*.ts': { statements: 84, branches: 67, functions: 93, lines: 85 }, + 'src/utils/**/*.ts': { statements: 92, branches: 87, functions: 89, lines: 96 }, + // index.ts, bootstrap.ts, scheduler.ts, config.ts, websocket.ts and the + // small top-level modules beside them. + 'src/*.ts': { statements: 39, branches: 36, functions: 36, lines: 42 }, }, }, },