mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-24 16:03:02 +00:00
aa5002b2ed4b48834b61ba808636ba67692db1cc
15 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maurice
|
40957e909b |
refactor: drop two returns that could only ever say one thing
Both functions had two exits handing back the identical value, which reads as a decision and is none — Sonar's S3516, and the last two Blockers on the backend. colorizeImportedTracks returned the very object it was given, coloured in place. Saying `void` and letting the two importers return their own result is what the function was doing all along. PLACES-SVC-005 checked the pass-through and now checks the thing worth checking: an empty import writes no colours. acceptInvite bailed out early on a refusal only to return the same result the line below returns. The notification is what the early exit was really skipping, so it is the notification that gets the condition. |
||
|
jubnl
|
305b113d5f |
Feat/storage driver (#1949)
* feat(storage): storage key validation, driver contract suite, LocalDriver * feat(storage): MirrorDriver composite backend * feat(server): add TREK_UPLOADS_DIR env var for the uploads root * feat(storage): settings-backed registry, StorageService facade, module wiring * test(storage): pin src/nest/storage coverage thresholds * feat(storage): multer upload options factory * refactor(files): spool trip-file uploads through storage * refactor(collab): spool note attachments through storage; normalize files/ prefix * refactor(journey): spool journey media uploads through storage * refactor(trips): spool trip cover upload through storage * refactor(places): spool place-image and collection uploads through storage * refactor(auth): spool avatar upload through storage * fix(server): consolidate allowed_file_types onto AllowedFileTypesService * fix(storage): default defParamCharset to utf8 in the upload factory * test(db): re-point the trailing-migration guard at the trip_files prefix strip * fix(storage): age-gate the boot spool sweep * test(files): key FILE-P02 residue checks on upload size * test(storage): raise src/nest/storage coverage pin * refactor(platform): serve /uploads routes through storage * refactor(files): serve trip-file downloads through storage * refactor(memories): serve photo bytes through storage * refactor(maps): stream place-photo bytes through storage * refactor(share): stream shared place-photo bytes through storage * refactor(journey): serve public journey assets through storage * fix(share): answer place-photo cache miss with 204 like the maps proxy * test(server): close serving-swap gaps surfaced by the full tier * test(storage): raise src/nest/storage branches pin * refactor(place-photos): route place-photo cache through storage * refactor(places): reclaim place images and collection covers through storage * refactor(trips): trip covers through storage * refactor(auth): avatar reclaim through storage * refactor(collab): note-file deletion through storage * refactor(files): trash deletion through storage * refactor(memories): trek photo cache internals through storage * refactor(memories): thumbnail service category-addressed * refactor(memories): immich upload reads through storage; drop uploads-root * refactor(journey): photo deletion through storage * fix(memories): reclaim orphaned trek photo cache objects in the ttl sweep * feat(journey): journey-thumbs orphan sweep and delete-site derived-artifact reclaim * test(server): close slice-4 gaps surfaced by the full tier * test(coverage): raise slice-4 domain pins * refactor(backup): list, existence and delete through storage * refactor(backup): download through storage sendToResponse * refactor(backup): create backup through per-category storage enumeration * refactor(backup): restore and upload-restore through storage * refactor(backup): auto-backup retention through storage * refactor(server): boot dir-creation to driver init * test(server): close slice-5 gaps surfaced by the coverage tier * test(coverage): raise slice-5 backup pin * test(server): resolve async backupFileExists mocks for the tests typecheck * test(server): pin ranged-read stat and zero-byte roundtrip in the storage driver contract * feat(server): TREK_S3_* env wiring — schema, s3 namespace, all-or-nothing boot rule * feat(server): S3 driver core — aws-lite client seam, stat/delete, driver-owned deadlines * refactor(server): type the aws-lite S3 client via s3-types reference * feat(server): S3 driver getStream and list — ranged full-object stat, iterator pagination * feat(server): S3 driver put — bounded-peek routing, Upload inactivity deadline * fix(server): S3 driver Upload monitoring — Transform interposition, stream cleanup * feat(server): env-declared s3-main storage backend; settings-declared s3 rejected * test(server): env-gated S3 driver contract suite against MinIO * fix(server): S3 driver zero-byte put — work around aws4 duplicate Content-Length signing defect * fix(server): scope the zero-byte put workaround to buffered bodies; bind test MinIO to localhost * ci(server): MinIO-backed S3 driver contract job * ci(server): align s3-contract shim comment and MinIO bind with references * docs: S3 storage backend configuration and off-box backup mirroring * docs: restore README formatting, keep S3 additions only * ci(server): make live-MinIO contract runs manual-only (AIStor licensing) MinIO's community line is discontinued entirely; the maintained successor, AIStor, requires a license key that can't safely live in a shared CI runner. The s3-contract CI job is removed; live-provider contract runs against MinIO/AIStor are now a manual, license-gated procedure documented in docker-compose.minio-test.yml's header. * chore(server): final-review polish — stale CI refs, dockerignore key, boundary pins, zero-byte tempfile contract case * expose admin ui * feat(shared): storage backend type registry, wire schemas, category relocation * feat(server): accept settings-declared s3 backends — shared-schema options, decrypt-at-build, registry preview() * feat(server): remove TREK_S3_*/TREK_UPLOADS_DIR — tombstone boot rule, built-in override as the relocation path * feat(server): deriveSecurity.encryptionKeySet and storage secret plumbing (gate, encrypt, unmask, mask, redact) * chore(server): drop storage-secrets import that only Task 8's probe helper will use * feat(server): seed-once storage-config.json boot import — fail closed on any invalid seed * feat(server): registry snapshot() — effective storage config with provenance * feat(server): api/admin/storage GET/PUT — masked secrets, preview-gated saves, wired reload, audits * feat(server): api/admin/storage/test — per-target ephemeral probes, mirror targets probed individually * test(server): e2e coverage for api/admin/storage — guards, managed refusal, envelope, probes * test(server): give e2e harness DBs the app_settings table the storage registry reads at boot * test(coverage): raise pins after the storage admin-config slice * fix(server): final-review polish — encryption-gate wording, all-field sentinel guard, README/e2e notes * test(server): storage slice-1 review follow-ups — mirror s3-replica probe decrypt, all-encrypted seed without key, test-route 401 * feat(i18n): storage admin domain + tab label in every locale * feat(client): adminApi storage endpoints — getStorage, updateStorage, testStorageBackend * feat(client): storage admin draft-document helpers + relative-time util * feat(client): schema-driven storage BackendForm — fields by kind, mask echo, encryption banner * feat(client): admin storage panel — backends list, category map, health strip, one-PUT save * feat(client): storage tab on the admin page * feat(client): storage admin panel and section on the phone shell * fix(client): block renaming a storage backend onto an existing name * feat(server): allow mirrors on any storage category — lift the backups-only rule * feat(i18n): replicas-on-primary mirror strings in every locale * feat(client): mirror fold/synthesize model helpers for replicas-on-primary * feat(client): BackendForm mirror-targets composer; raw mirror type hidden * feat(client): desktop storage panel folds mirrors into their primaries * feat(client): phone storage panel folds mirrors into their primaries * fix(client): strip removed backends from remaining mirror replicas — no phantom refs * feat(storage): retire photos as a configurable category — served-only via the ServedCategory seam * feat(i18n): descriptive storage category names and descriptions in every locale * feat(client): descriptive category names, descriptions and id badges in the storage panel * docs(storage): fix stale nine-category comments after the eight-category shrink * docs: storage configuration via the admin UI and seed file — README + compose * docs(wiki): Admin Storage page and storage cross-links * docs(server): storage domain blueprint entry; TREK_PLACE_PHOTO_DIR tombstone clarifier * refactor(server): drop the storage env-var tombstones — the variables never shipped * docs: storage config is simply new in v4 — drop the removed-variables story * feat(shared): backfill-status and usage wire schemas on the storage admin state * feat(server): MirrorDriver.backfill — prefix-scoped, size-compared, cancellable replica catch-up * feat(server): one-at-a-time storage backfill jobs + the storage events bridge * feat(server): storage usage scan — nightly cron, on-demand service, persisted with computedAt * feat(server): replica_failure admin notifications — events bridge, hourly debounce, channel registration * feat(i18n): replica-failure notification text in every locale * feat(server): storage backfill and stats-refresh admin endpoints * feat(client): adminApi backfill and stats-refresh endpoints * feat(i18n): storage sync and usage strings in every locale * feat(client): desktop storage panel — sync now, usage display, backfill polling * feat(client): phone storage panel — sync now, usage display, backfill polling * fix(client): per-category usage line on the phone storage panel * docs: sync now, usage stats and replica-failure notifications * test(server): admin scope now includes replica_failure, not just version_available Commit |
||
|
Maurice
|
28765e220b |
fix(places): keep the Google feature id, and give list imports the free address lookup
Two halves of #1954, both about a place arriving through a list coming out thinner than the same place pasted as a single link. The feature id was never dropped on purpose. googleFtidFromMapsUrl only looked at the `?ftid=` query parameter, while a share link that resolveGoogleMapsUrl has followed to its final hop carries the id in the `data=!…!1s0x…:0x…` path blob — the same run the coordinates are already parsed out of two lines further down. So every pasted maps.app.goo.gl link lost it, and the Maps button opened a name search rather than the entry the link pointed at. The parser now reads the blob too, but only on a /place/ URL: a /maps/dir/ route carries one !1s per waypoint and the first is the origin, which would attach the wrong place. List imports never had a reverse lookup at all. The only pass behind them is the Google enrichment, which returns immediately without a Maps key, so an instance without one imported a list and left every address empty forever. They now get the same free Nominatim lookup the single-link path has always had: detached after the import answers, on the throttled background lane so it never queues in front of someone's search, through COALESCE so it can only fill an empty column, and each row pushed over the websocket so the sidebar fills in without a reload. Capped at 250 places — past that it stops being a backfill and starts being bulk geocoding. Not included: an osm_id for imported places. Single links do not produce one either, and one derived from a zoom-18 reverse lookup would name the building or the street rather than the POI. |
||
|
Maurice
|
c9124c9b63 |
fix(places): only attach tags owned by someone on the trip
A tag belongs to a user, not to a trip, and the place create/update body is an open record the Zod pipe does not police — so any tag id could be attached, and the tag projection ships tags.user_id, which answers who owns a tag the caller has no way to see. collection_place_tags had it verbatim. Scoped to the roster rather than to the caller on purpose: a member tags a shared place, another member re-saves it and, because the update is delete-then-insert, sends that id back. Caller-scoping would drop a co-traveller's tag on every foreign edit; roster-scoping keeps it. Collections resolve the same question through their own member list. No client sends a tags array today, so this only closes the hand-crafted path. |
||
|
Maurice
|
3c5e406229 |
feat(places): link an expense to a place, the way bookings already do (#1917)
* feat(places): link an expense to a place, the way bookings already do Recording what a museum ticket or a guided tour cost meant leaving the place, opening the Costs tab and typing it in again — while a booking has had a Costs block in its own form all along. The place form now carries that block. Same component, same flow: with nothing linked it offers Create expense, which saves the place and then opens the Costs editor for a new expense pointing at it; once linked it shows the expense with edit and remove. budget_items gains place_id beside the reservation_id it already had, with the same nullable FK and the same ON DELETE SET NULL. Desktop and phone both, and the mobile editor carries the link through its own save path rather than the desktop one. The expense belongs to the place, not to a day assignment, so a place sitting on five days is not five tickets. Hanging it off the assignment would have meant adding a place to three days quietly creating three expenses and removing a day quietly deleting one. Deleting a place takes its expense with it in the same transaction and announces it, on every path that deletes a place: REST, both MCP tools and the plugin RPC. Closes #1298 * test(places): teach the RPC stubs about linkedExpenseIds The delete paths read the linked expense before the place is gone, so the two hand-built PlacesService stubs threw on the new method instead of reaching the hook, the remove and the broadcast — four failures out of one missing line. |
||
|
Maurice
|
df3edac7b9 |
feat: six community requests — import dedup, accommodations in the feed, OAuth refresh race, visited-from-trip, journal jumps, phone packing rows (#1915)
* fix(places): match imports on the provider id so renamed places are not duplicated Re-importing a Google Maps list duplicated every place the user had renamed. buildDedupSet only ever collected names and, for unnamed places, coordinates, and isPlaceDuplicate returns on the name before it reaches the coordinate fallback, so a renamed place was findable by neither. The ids were stored all along, and even backfilled onto matched places, they were simply never read. Match on google_place_id / google_ftid / osm_id first, keeping name and coordinates in their old roles below it. Widening the coordinate check instead would have merged the restaurant and the bar in the same building. The collection copy carried a hand-mirrored port of the same predicate; it now uses the shared helpers and gets the fix with them. Closes #1550 * feat(calendar): put accommodations into the subscribed feed as all-day stays A hotel keeps its dates on the linked stay, not on the reservation: the booking form writes reservation_time = NULL for type 'hotel', and day_accommodations carries start day, end day and the check-in/out clock. The feed only ever read the reservation, so a stay showed up once on the arrival day at best, and not at all when it came from the booking form. Join the stay in and emit the reservation as an all-day range over every night, which also keeps it correct when days are reordered later — the dates now come from the day rows rather than from the copy stamped at creation time. Check-in and check-out become their own timed events when the stay records a clock, because an all-day event cannot carry the time you actually need reminding of. The check-in window, when set, becomes that event's DTEND. Closes #1586 * fix(oauth): treat a concurrent refresh as a race, not as a stolen token Refresh rotation is a race by construction. Several MCP clients share one refresh token, both post it within the same second, and the loser was read as a replay: chain revoked, MCP sessions torn down, browser window asking to log in again. That is the daily re-authentication people were seeing, not a short lifetime — access tokens last an hour and refresh tokens roll for thirty days. A revoked refresh token now only counts as a replay once it is outside a 30-second grace window, and only when the rotation that revoked it left a successor still alive. Inside the window the caller gets its own pair branched off the same parent, so both clients walk away with a working token. Explicit revocation leaves no live successor and a chain killed by a real replay has none either, so neither can slip through the window. Closes #1007 * feat(collections): mark a place visited from the trip it belongs to A place saved in several lists had to be opened in each of them to say you had been there, which is the wrong way round: you find out you have been somewhere while looking at the trip, not while browsing the library. The save picker now shows each list's own status and cycles it in place, with one action for "visited, wherever this is saved". The planner's selection bar and the mobile bulk toolbar carry the same thing for a whole selection — select all, one tap, done. Matching happens on the server with the signals findMembership already uses plus the source link a place saved from this trip carries, so a copy renamed inside a list is still found. Lists you may only read are skipped rather than refused; a shared list you cannot edit should not stop you marking your own. Closes #1469 * feat(journey): jump to the top or back to the last entry in a long journal Everything that adds to a journal sits at the top — the entry button, the gallery upload — and where you left off sits at the bottom, so adding one photo to a long journey meant scrolling the whole way twice. Two buttons float over the middle of the feed, clear of the Add Entry column on one side and the reorder arrows on the other, each appearing only when there is more than a screenful to travel in that direction. They live in a zero-height sticky box so they take no layout space and never push the timeline around. While in the header, the status pill also picks up the height of the round buttons beside it. Closes #1088 * fix(packing): give the item name the row back on phones An item name had roughly a third of the row to itself: a spelled-out sharing sentence, an empty weight column and two round action buttons took the rest, so anything longer than a word was cut off. Sharing is now the icon it always was next to a label rather than a sentence in the layout, an unset weight prints nothing instead of a dash, and edit/delete sit behind one overflow button that opens the same inline menu the bag picker already uses. The to-do list needed none of this — it wraps its chips onto a second line already. Closes #1525 * docs(wiki): cover the six community changes - Calendar feeds: accommodations as an all-day range plus check-in/check-out - Collections: setting the visited status from a trip, and how places are matched - Journey: the jump buttons on a long journal - Packing: what the phone row keeps and what moved behind the overflow button - Places: re-importing a list no longer duplicates renamed places - MCP setup: the rotation race no longer ends an OAuth session * test(collections): assert the stored status instead of a private method tests/ sits outside the build tsconfig, so reaching into getPlaceById only showed up in the dedicated typecheck step. Reading the row is the better assertion anyway — it pins what was persisted, not what a getter returns. |
||
|
jubnl
|
079064fdcd | refactor(server): delete trips.bridge via leaf membership reads and prompt relocation | ||
|
Maurice
|
49c3c57c31 |
Give the ephemeral tokens a provider, and clear the next wave of type errors
ephemeral-tokens.ts was free functions, and AuthService owned the cleanup timer only because it happened to be the domain that mints tokens. Issuing them and sweeping them are different jobs, and FilesService and TokenService mint or consume them too. EphemeralTokenService owns the lifecycle and hands the three operations out. The store and the interval stay module state next door, and that is load-bearing rather than a leftover: src/websocket.ts consumes ws tokens on the upgrade handshake, outside the container, before any injector is in reach. Per-instance state would hand it an empty Map and close every connection with 4001, which no test would catch because the ws suites drive the transport directly. WS-027 through WS-029 pin the session gate that runs through exactly that path. The MAX_STORE_SIZE back-pressure is the second reason: createEphemeralToken returns null at 10,000 live tokens and TokenService turns that into a 503. A second store would raise the real ceiling with nobody noticing. Its own leaf module rather than a provider in AuthModule: TokensModule mints download and ws tokens as well, AuthModule already imports TokensModule, and TokensModule is documented as a deliberate leaf that three graphs reach for. Same shape as MailerModule, which broke AuthModule <-> NotificationsModule. Twelve hand-wired construction sites in tests needed the new collaborator. The typecheck named all twelve; before this branch they would have been undefined at runtime. Sixteen more test files are cleared here too, each at its root cause. Four of them were passing for the wrong reason: files.controller.test.ts drove two handlers with shifted arguments, a string tripId where the trip row belongs and undefined where tripId and id belong. It stayed green because FilesService is fully stubbed and the stubs ignore what they are given, so DELETE /:id/permanent and DELETE /trash/empty never covered that the trip row reaches the permission check. conflictUpdate.test.ts was missing real collaborators on PlacesService and PackingService. None of the exercised paths reach them, so it passed by luck. assignments.service.test.ts builds its service by hand, and the one consumer of the collaborator it was missing swallows every throw in a bare catch. A broken journey reconciliation would still have shown 28 green tests. scheduler.test.ts mocks two modules with paths that resolve one directory above the server root, so they mock nothing at all. Harmless only because src/scheduler.ts imports neither. |
||
|
Maurice
|
c7fed4b1f8 |
refactor(server): make the place-photo cache a provider (#1833)
The last file in be-places-helpers-fold, and the one that had to be a provider rather than a move: it carries three in-memory structures that only do their job if the process has exactly one of them. recentFailures stops a screenful of markers hammering an API that is down inFlight stops a stampede when several requests miss the same placeId knownOnDisk saves an existsSync per cache hit A second instance gives the stampede guard its own inFlight map, so two simultaneous requests for the same uncached place both call the provider. That is why the scheduler takes this one from the container instead of constructing it: SchedulerDeps gains a placePhotos entry alongside backups, filled from app.get() in index.ts — the shape PR #1827 was built for. The module-scope mkdirSync becomes onModuleInit, and GOOGLE_PHOTO_DIR is read per call through RuntimeEnvService rather than frozen at import. That is not cosmetic: TREK_PLACE_PHOTO_DIR is the seam the tests use to keep off the real uploads tree, and its own test used to have to import the module dynamically to set the variable in time. The __dirname fallback moved a level with the file and is corrected accordingly. Consumers: maps, places and share inject it; places.helpers.reclaimPhotoCache takes it as a parameter, since it is a free function. Six test seams follow. Four path mocks become injected stubs with the same mock functions behind them; the collections case keeps the REAL implementation, because what it asserts is that removeIfUnreferenced counts collection_places as a reference (#1081) — a stub there would assert nothing. The share e2e overrides the provider in its container. services/ is down to 14 top-level files, from 44 when this started. |
||
|
Maurice
|
4e6b45016a |
refactor(server): move the KML importer and place-image helpers into places/ (#1832)
Two of the three files in be-places-helpers-fold. placePhotoCache is left for its own PR — it holds three module-level caches and its sweep hangs off the cron, so it needs both a provider with real state and a second entry in the scheduler contract. kmlImport.ts is 212 lines with no state, no database and no I/O — parsing, sanitising and coordinate arithmetic. It is nest/places/kml-import.helpers.ts now, unchanged, next to the places.helpers.ts that already consumes it. placeImage.ts moves to nest/places/place-image.ts as free functions. Making reclaimPlaceImage a provider would have split it from PLACE_IMAGES_DIR, which cannot be injected: nest/common/place-image-upload.ts reads it at module scope to configure multer's storage, before any container exists. That constant is the one thing here that could have broken quietly: path.resolve(__dirname, '../../uploads/places') __dirname moves with the file. From src/services that resolved to <server>/uploads/places; from src/nest/places the same string would have resolved to <server>/src/uploads/places — a directory multer would have happily created, leaving every uploaded place image in the wrong place and every existing one unreachable. The path is now '../../../uploads/places', verified to resolve to the identical absolute directory, and uploads-dirs.test.ts covers it. |
||
|
Maurice
|
90e35e6970 |
refactor(server): fold llmConfig and unsplash out of services/ (#1831)
Two of the three leftovers in be-misc-fold. The third (wikiService) needs a Zod contract in @trek/shared first and is left for its own PR. llmConfig.ts was pure movement: 70 lines of encryption-aware config shaping with no state and no database, sitting in services/ while all four of its callers are in src/nest. It is nest/llm-parse/llm-config.ts now, same functions, same file. unsplashService.ts needed an actual provider. Its key resolution reads both the env and the users table (env var, then the caller's stored key, then any admin's), so the injected connection was unavoidable. Its own module because trips and places both reach it, and because folding it into either would have made the other import a trip/place service for an image search. The four exports became methods on UnsplashService. The SSRF guard in saveUnsplashCover is untouched — it still refuses any host but the Unsplash image CDN and still goes through safeFetch — as is the unauthenticated-endpoint fallback when no key resolves (#1449). The env read stays live through RuntimeEnvService rather than a registerAs token. readEnv() is what the function did before, and a boot-frozen token would change when an operator's UNSPLASH_ACCESS_KEY takes effect. Two test seams follow the code: the unsplash unit tests bind the four methods so every existing assertion reads unchanged, and the trips controller's path mock of the module becomes an injected stub. services/ is down to 17 top-level files. |
||
|
Maurice
|
939452f9d8 |
refactor(server): consolidate trip access and fold the query helpers into the container (#1824)
* refactor(server): route every trip-access check through DatabaseService
Twelve Nest services carry the same verifyTripAccess/canEdit pair. Six already
asked DatabaseService.canAccessTrip; the other six went through
services/tripAccess, a nine-line re-export of the identical function. Same
query, same result, two ways to reach it.
All twelve now take the injected route, so services/tripAccess is gone.
The bigger duplication was in the types. Thirteen files declared their own
`Trip` for the same return value, in three different shapes:
NonNullable<ReturnType<typeof verifyTripAccess>> 6x
{ user_id: number } 4x
TripAccess 3x
The narrow one is why assignments, days, places and accommodations all wrote
`as Trip | null | undefined` on the call — the cast existed to shrink
TripAccess down to a local type that was never meant to differ from it. All
thirteen now say TripAccess, and the four casts are gone with no other change.
middleware/tripAccess.ts goes too: it was already dead in production — nothing
under src/ imported requireTripAccess or requireTripOwner, only comments in the
services that had absorbed them. Its test went with it.
* refactor(server): fold the query helpers and trip joins into the container
Two more legacy files whose only reason for staying outside the container was
that they predated it.
services/queryHelpers.ts held five exports doing two different jobs. Three are
the batch loaders that keep the list endpoints off N+1 queries — they hit the
database and are now QueryHelpersService over the injected connection, imported
by assignments, days, places and share. The other two, formatAssignmentWithPlace
and ratingAggregate, touch no database at all; they sat next to the loaders only
because the loaders were free functions too. Those stay free functions, in
common/rowShape.ts.
The README note on the assignments service claimed the loaders had to stay in
services/ because the day, share and place services were unmigrated. All three
have since moved, so the reason is gone.
services/tripMembership.ts becomes TripMembershipService in its own module. It
could have folded into auth or trip-invite, but both call it — as do oidc and
the plugin RPC host — and putting it in either would have made AuthModule and
TripInviteModule import each other.
Neither module is @Global, matching PermissionsModule: e2e TestingModules
resolve them transitively through each consumer's explicit import.
Test seams changed shape accordingly. Two path mocks of services/tripMembership
became injected stubs; the trip-invite e2e overrides the provider in the
container instead. The oidc test keeps the real implementation, which is what
OIDC-SVC-045 was always asserting.
ratingAggregate moved out of the ungated services/ tree into the coverage-gated
one, so it gets the direct tests it never had — as does the tags/participants
pass-through, which two long-dead fixture variables in the moved test file had
been waiting for.
Also drops four stale comments that outlived their subject: the requireTripAccess
middleware they pointed at was deleted in the previous commit.
|
||
|
jubnl
|
f02f255016 |
fix(places): cap the Naver list-import response like the Google one
The place fold capped the Google list-import body at 8 MB but left the Naver importer reading its pages unbounded — same attacker-influenced URL (the folder id), and worse in practice because that path is a pager that buffers a fresh body on every iteration. fetchPage now checks the declared content-length before reading and the actual length after, returning the existing 'Failed to fetch list from Naver Maps' 502 on either. Reading the body as text and parsing it explicitly (rather than apiRes.json()) is what makes the post-read check possible; the malformed-payload branch keeps its 'Invalid list data received from Naver Maps' 400 because the JSON.parse stays inside the same try. The post-read check is the one that matters for a hostile server: a chunked response carries no content-length, so the declared check alone is evadable. Covered for both providers now — PLACE-SVC-073b (Google, chunked) and PLACE-SVC-074..077 (Naver: declared, chunked, malformed, happy path). The five Naver fetch mocks in the integration suite move from json() to text(). |
||
|
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. |
||
|
Maurice
|
1b5317a817 |
Give GPX tracks their own colour (#776)
Imported tracks all render in the same blue: the GPX/KML importers never assign a category, so category_color is null for every one of them and several walks in the same area are impossible to tell apart. Adds a nullable route_color on places, kept out of the COALESCE group in updatePlace so an explicit null can reset a track back to inheriting its category colour. The hex is validated in the Nest controller rather than in the shared contract: the update body is an open record on purpose, and tightening it would mean removing PlacesController.update from the ratchet-only body-contract allow-list. The importers now hand each new track a colour of its own, picked from the palette entries the trip is not already using. That happens in the Nest service over the injected DatabaseService — the colour assignment is new logic, and new logic does not belong in the legacy place service (see server/src/nest/README.md). The field itself is only threaded through the existing update/create statements, which the proportionality rule covers. Also fixes duplicateTrip, whose explicit column list has been silently dropping route_geometry all along, so a duplicated trip lost its tracks. |