TREK/migrate.md at f02f255016727ca58ca6c2b88563bad799983a0f

k1nq/TREK

mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00

Files

T

jubnl 4f0f80ad69 chore: move place backend to proper DI

Fold the 1029-line services/placeService.ts into the DI-native PlacesService:
the CRUD + ratings SQL, the GPX/KML/KMZ importers and the Google/Naver list
importers become methods over the injected DatabaseService, byte-identical
statements, COALESCE semantics and the If-Match conflict protocol (#1135).
The pure half — frozen XML parsers, the KMZ unpacker, the dedup predicates,
the Google hex-id parsers, reclaimPhotoCache — moves to places.helpers.ts
(maps.helpers precedent).

The 10-tool mcp/tools/places.ts registrar plus the trek://trips/{tripId}/places
resource move onto the decorator registry as places.mcp.ts. search_place travels
with them because its gate is places:read, not the read-only geo group, and now
injects MapsService. TripsService, DaysMcp, BookingImportService and the plugin
RPC host (PluginHostDepsFactory's 21st constructor dep) all inject PlacesService,
so the domain needs no bridge at all. The two assignments.bridge imports that
places.mcp.ts keeps are a deliberate cycle break: AssignmentsModule imports
DaysModule and DaysModule now imports PlacesModule for days.mcp.ts's place
creation, so injecting AssignmentsService there would close
DaysModule -> PlacesModule -> AssignmentsModule -> DaysModule. Same seam
reservations.mcp.ts uses, same trade trips.bridge.ts documents.

Also folds services/placeEnrichment.ts in — its DB/websocket/Maps half becomes
PlacesService methods over the injected DatabaseService/RealtimeService/
MapsService, its pure match selector joins places.helpers.ts — which retires
nest/maps/maps.bridge.ts with its last consumer.

Adopts places.dto.ts for the seven grandfathered body contracts and clears every
PlacesController entry from body-contract-allow-list.ts. Two consequences worth
knowing: the bespoke 400s 'Place name is required', 'ids must be an array of
numbers' and 'URL is required' are now the ZodValidationPipe envelope, and
because a pipe runs before the handler a malformed body 400s ahead of the
trip-access 404 it used to follow — the same trade the todo and trips migrations
took. placeBulkUpdateRequestSchema.ids drops .min(1) so the endpoint's
empty-list short-circuit stays reachable.

Fixes four defects the relocation had faithfully carried, each with a
failing-first regression test:

- create dropped lat/lng of exactly 0, so a place on the equator or the prime
  meridian lost its coordinates (`x || null` cannot tell absent from zero).
- duration_minutes: 0 was unsettable — replaced by the 60-minute default on
  create, and read as "absent" by COALESCE on update.
- the journey delete hook fired on unscoped ids: DELETE /:id, POST /bulk-delete
  and the delete_place MCP tool could detach another trip's journey entries for
  an id they then refused. All three scope first via the new
  PlacesService.scopedIds, mirroring the guard PluginHostDepsFactory already had.
  bulk_delete_places also moves its hook ahead of the DELETE —
  journey_entries.source_place_id is ON DELETE SET NULL, so running it afterwards
  left the entries as orphans.
- the place search treated % and _ as LIKE wildcards; a bare % returned the whole
  trip. The three clauses now carry ESCAPE with the term escaped.

Plus hardening on the Google list fetch: the response is capped at 8 MB
(declared content-length and post-read length, the transit.service precedent)
and its JSON.parse is guarded, so a malformed provider payload produces the
existing 'Invalid list data received from Google Maps' 400 instead of throwing.

Quirks deliberately preserved: the non-COALESCE route_color (#776),
currency/transport_mode staying unclearable, importGpx returning null for a file
that parses but yields no usable geometry, update skipping the If-Match check
when the stored updated_at is null, and every string-valued `x || null`.

Tests move with their case IDs: placeService.test.ts and the wrapper suite merge
into unit/nest/places.service.test.ts, kmzUnpack into places.helpers.test.ts,
placeEnrichment into places.enrichment.test.ts. The places e2e converts to the
DI-native pattern (real SQL against temp-db DDL), tools-places.test.ts is kept
and extended so it exercises the decorator path through the test registry, and
the plugin-host and booking-import path mocks become constructor stubs.

2026-07-30 17:07:12 +02:00

8.2 KiB

Raw Blame History

Recommended order

Wave 1 — pilots (establish the template on easy wins)

tagService (26 lines, zero deps, only consumers: TagsModule + 1 MCP tool) — the ideal first migration. Small enough that the whole pattern fits in one PR: move the SQL into TagsService with injected DatabaseService, retire the legacy file, convert its tests to overrideProvider(DATABASE_CONNECTION).
categoryService (31 lines, same shape) — immediate repeat to confirm the template.
todoService (120 lines, classic CRUD with transactions + broadcast) — first "full-shaped" domain: prepared statements, db.transaction, WebSocket broadcast, 3 MCP tools. After this one, the recipe is settled for everything else.

The pilots deliberately include MCP consumers, because they force the one open design decision early: MCP tools run outside the Nest container, so either the migrated module keeps thin function exports delegating to a shared instance, or the MCP layer gets a handle to the Nest app (app.get(TodoService)). Better to settle that on a 26-line service than on authService.

Wave 2 — cross-cutting leverage (parallel to wave 3, high payoff)

permissions (149 lines, zero deps, 17 Nest consumers) → an injectable PermissionsService (done 2026-07 — every module wrapper, the airtrail-import controller and PluginHostDepsFactory now inject it; the cache stays module-scoped so the DI singleton and permissions.bridge — kept for mcp/_shared.ts + adminService/authService/backupService/collectionsService — share one invalidation). Every module wrapper already calls checkPermission; this converts the single most-imported seam in one move.
auditLog (171 lines, 13 Nest consumers) → AuditService (done 2026-07 — writeAudit is the injectable; getClientIp and the log*/LOG_LEVEL logger stay plain modules in nest/audit/ (client-ip.ts, audit-log.logger.ts with its frozen-at-import level + mkdir as a documented parity exception); audit.bridge covers mcp/index, oauthProvider and the legacy airtrail/immich/oauth services).
tripAccess (9 lines) — don't migrate, delete: it's a wrapper around canAccessTrip, which DatabaseService now exposes. Absorb its 11 call sites opportunistically.

Wave 3 — domain services with existing modules, low fan-in

dayNoteService (done 2026-07), tripInviteService (done 2026-07), assignmentService (done 2026-07), shareService (done 2026-07), settingsService (done 2026-07), fileService (done 2026-07), collabService (done 2026-07), packingService (done 2026-07) — each maps 1:1 to a Nest module that's already a thin delegate; none is imported by more than one other legacy service.

Wave 4 — the coupled cluster (order matters here)

dayService → budgetService → reservationService in that order: reservations depend on both (budget-item linking, day resolution), so migrating the dependencies first lets ReservationsService inject the finished versions instead of importing legacy functions. — Correction (migration-graph.md, borne out by the migration): the claimed ordering constraint doesn't exist at the service layer. reservationService (done 2026-07) imported neither budgetService nor dayService; the budget/day coupling lives in the Nest wrapper's budget-sync seam and the MCP registrars, which keep their legacy imports until those domains migrate. Reservations went first as the frontier residue fold; dayService (done 2026-07 — the 592-line service folded into the wrapper DaysService, the accommodations seam in nest/reservations/ now injects it, and the hand-rolled reorder/insert transactions became db.transaction()) followed; budgetService followed (done 2026-07 — the 755-line money core folded into the wrapper BudgetService (settlement maths, FX freeze/rebase, items/members/payers CRUD; the raw settlement writes became insertSettlement/applySettlementUpdate so the MCP paths keep skipping the freeze); the 11-tool mcp registrar + 3 budget resources moved to the decorator-driven budget.mcp.ts; TripsService/ReservationsService/BookingImportService now inject BudgetService; a 4-export budget.bridge.ts serves tripService, userCleanupService and the legacy trips/transports registrars; exchange-rates.bridge.ts was deleted with its last consumers; and the controller adopted budget.dto.ts over the shared schemas, clearing all nine budget allow-list entries — tripService is now unblocked).
tripService (done 2026-07 — the 1121-line hub folded into the wrapper TripsService: generateDays, the updateTrip date-shift transaction, members/guests (#973/#1362), the ICS export, copyTripById and getTripSummary; its six bridge imports became injected services (CollabService/VacayService joined the constructor); the 10-tool trips registrar + 3 trip resources + the trip-summary prompt moved to the decorator-driven trips.mcp.ts (first @Prompt; the static-token deprecation notice rides the registry.attach ctx) and the 3 share-link tools to share.mcp.ts on the canShareTrips predicate; FeedsService and the plugin RPC host inject TripsService; a 3-export trips.bridge.ts serves the legacy prompts registrar and budget.mcp.ts's owner/member seam; todo/share/collab/vacay bridges deleted with their last consumers; the controller adopted trips.dto.ts, clearing all seven trip allow-list entries). placeService followed (done 2026-07 — the 1029-line place core folded into the wrapper PlacesService with its pure half in places.helpers.ts; the 10-tool registrar + the trip-places resource moved to the decorator-driven places.mcp.ts (search_place injects MapsService); TripsService/DaysMcp/BookingImportService/the plugin RPC host all inject it, so the domain needs no bridge at all; the sibling placeEnrichment fold absorbed the last maps.bridge consumer and deleted that bridge too, and the DTO ratchet cleared all seven PlacesController allow-list entries — transitItineraryService is the last step-4 link).

Wave 5 — the heavyweights, last

adminService (14 Nest + 11 MCP consumers), journeyService/atlasService/vacayService (done 2026-07 — pulled forward as a zero-dependency frontier member, see migration-graph.md)/collectionsService/mapsService (done 2026-07 — pulled forward as step 4 of the dependency-honest order: the 1429-line geo core folded into the wrapper MapsService, pure helpers to maps.helpers.ts, the 3 geo MCP tools to the decorator-driven maps.mcp.ts (weather/airport tools stay in the legacy registrar), BookingImportService injects it, and a 3-export maps.bridge.ts serves placeEnrichment + the places registrar — transitService → placeService now unblocked)/transitService (done 2026-07 — pulled forward as step 5 of the dependency-honest order: the first fully SQL-free domain fold — the 333-line Transitous/MOTIS proxy became a dep-free TransitService (no DatabaseService); pure deriveTransitStats + mode whitelist + itinerary types moved to transit.helpers.ts so the downstream transitItineraryService needs no bridge; the whole 3-tool registrar moved to the decorator-driven transit.mcp.ts, its days/reservations bridge imports becoming injected DaysService/ReservationsService — zero bridge files, placeService → transitItineraryService now unblocked), and finally the auth stack (authService at 1494 lines with 18 MCP consumers, plus oauthService/oidcService/passkeyService) — highest risk, most consumers, security-sensitive; do it when the pattern is thoroughly proven. backupService also stays late: it owns closeDb/reinitialize lifecycle, which is deliberately outside the DI wrapper.

Two special cases to keep in mind: airportService is lazily required by db/database.ts at boot (the flight-endpoint backfill) — whenever it migrates, that call should move into Nest's bootstrap instead, or you recreate the circular import; and notifications/notificationService are imported by 6+ other services, so like the wave-4 cluster they should migrate before their dependents or keep function shims in the interim.

The through-line: each finished migration lets that domain's tests drop vi.mock('db/database') for overrideProvider(DATABASE_CONNECTION), so the mock seam shrinks wave by wave and disappears with the auth stack. Want me to start on tagService as the pilot?

8.2 KiB Raw Blame History

8.2 KiB

Raw Blame History