diff --git a/server/src/mcp/resources.ts b/server/src/mcp/resources.ts index f78b53fd3..622fd4ca8 100644 --- a/server/src/mcp/resources.ts +++ b/server/src/mcp/resources.ts @@ -1,5 +1,4 @@ import { McpServer, ResourceTemplate } from '@modelcontextprotocol/sdk/server/mcp'; -import { listBucketList, listVisitedCountries, getStats as getAtlasStats, listManuallyVisitedRegions } from '../services/atlasService'; import { getNotifications } from '../services/inAppNotifications'; import { isAddonEnabled } from '../nest/addons/addons.bridge'; import { ADDON_IDS } from '../addons'; @@ -82,27 +81,9 @@ export function registerResources(server: McpServer, userId: number, scopes: str // src/nest/categories/categories.mcp.ts (@Resource, attached via the // nest-mcp registry in registerTools). - // User's bucket list - if (isAddonEnabled(ADDON_IDS.ATLAS) && canRead(scopes, 'atlas')) server.registerResource( - 'bucket-list', - 'trek://bucket-list', - { description: 'Your personal travel bucket list', mimeType: 'application/json' }, - async (uri) => { - const items = listBucketList(userId); - return jsonContent(uri.href, items); - } - ); - - // User's visited countries - if (isAddonEnabled(ADDON_IDS.ATLAS) && canRead(scopes, 'atlas')) server.registerResource( - 'visited-countries', - 'trek://visited-countries', - { description: 'Countries you have marked as visited in Atlas', mimeType: 'application/json' }, - async (uri) => { - const countries = listVisitedCountries(userId); - return jsonContent(uri.href, countries); - } - ); + // The bucket-list and visited-countries resources moved to the DI-discovered + // src/nest/atlas/atlas.mcp.ts (@Resource, attached via the nest-mcp registry + // in registerTools). // The trip-budget-per-person and trip-budget-settlement resources moved to // the DI-discovered src/nest/budget/budget.mcp.ts (@ResourceTemplate). @@ -121,28 +102,9 @@ export function registerResources(server: McpServer, userId: number, scopes: str } ); - // Atlas stats and regions (addon-gated) - if (isAddonEnabled(ADDON_IDS.ATLAS) && canRead(scopes, 'atlas')) { - server.registerResource( - 'atlas-stats', - 'trek://atlas/stats', - { description: "User's atlas statistics — visited country counts and breakdown", mimeType: 'application/json' }, - async (uri) => { - const stats = await getAtlasStats(userId); - return jsonContent(uri.href, stats); - } - ); - - server.registerResource( - 'atlas-regions', - 'trek://atlas/regions', - { description: 'List of manually visited regions for the current user', mimeType: 'application/json' }, - async (uri) => { - const regions = listManuallyVisitedRegions(userId); - return jsonContent(uri.href, regions); - } - ); - } + // The atlas-stats and atlas-regions resources moved to the DI-discovered + // src/nest/atlas/atlas.mcp.ts (@Resource, attached via the nest-mcp registry + // in registerTools). // The vacay resources moved to the DI-discovered src/nest/vacay/vacay.mcp.ts // (@McpController, attached via the nest-mcp registry in tools.ts). diff --git a/server/src/mcp/tools.ts b/server/src/mcp/tools.ts index d7b91d81a..9dfe55ee3 100644 --- a/server/src/mcp/tools.ts +++ b/server/src/mcp/tools.ts @@ -2,7 +2,6 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; import { registerJourneyTools } from './tools/journey'; import { registerMapsWeatherTools } from './tools/mapsWeather'; import { registerNotificationTools } from './tools/notifications'; -import { registerAtlasTools } from './tools/atlas'; import { registerTransportTools } from './tools/transports'; import { registerMcpPrompts } from './tools/prompts'; import { getMcpRegistry } from './registry-handoff'; @@ -38,7 +37,8 @@ export function registerTools(server: McpServer, userId: number, scopes: string[ registerNotificationTools(server, userId, scopes); - registerAtlasTools(server, userId, scopes); + // The atlas tools moved to the DI-discovered src/nest/atlas/atlas.mcp.ts + // (@McpController, attached via the nest-mcp registry below). // The collab tools moved to the DI-discovered src/nest/collab/collab.mcp.ts // (@McpController, attached via the nest-mcp registry below). diff --git a/server/src/mcp/tools/atlas.ts b/server/src/mcp/tools/atlas.ts deleted file mode 100644 index 920db029d..000000000 --- a/server/src/mcp/tools/atlas.ts +++ /dev/null @@ -1,201 +0,0 @@ -import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { z } from 'zod'; -import { isDemoUser } from '../../services/authService'; -import { - markCountryVisited, unmarkCountryVisited, createBucketItem, deleteBucketItem, - getStats as getAtlasStats, listManuallyVisitedRegions, - markRegionVisited, unmarkRegionVisited, getCountryPlaces, updateBucketItem, -} from '../../services/atlasService'; -import { isAddonEnabled } from '../../nest/addons/addons.bridge'; -import { ADDON_IDS } from '../../addons'; -import { - TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - TOOL_ANNOTATIONS_READONLY, - demoDenied, ok, -} from './_shared'; -import { canRead, canWrite } from '../scopes'; - -export function registerAtlasTools(server: McpServer, userId: number, scopes: string[] | null): void { - const R = canRead(scopes, 'atlas'); - const W = canWrite(scopes, 'atlas'); - - if (!isAddonEnabled(ADDON_IDS.ATLAS)) return; - - // --- BUCKET LIST --- - - if (W) server.registerTool( - 'create_bucket_list_item', - { - description: 'Add a destination to your personal travel bucket list.', - inputSchema: { - name: z.string().min(1).max(200).describe('Destination or experience name'), - lat: z.number().optional(), - lng: z.number().optional(), - country_code: z.string().length(2).toUpperCase().optional().describe('ISO 3166-1 alpha-2 country code'), - notes: z.string().max(1000).optional(), - }, - annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, - }, - async ({ name, lat, lng, country_code, notes }) => { - if (isDemoUser(userId)) return demoDenied(); - const item = createBucketItem(userId, { name, lat, lng, country_code, notes }); - return ok({ item }); - } - ); - - if (W) server.registerTool( - 'delete_bucket_list_item', - { - description: 'Remove an item from your travel bucket list.', - inputSchema: { - itemId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_DELETE, - }, - async ({ itemId }) => { - if (isDemoUser(userId)) return demoDenied(); - const deleted = deleteBucketItem(userId, itemId); - if (!deleted) return { content: [{ type: 'text' as const, text: 'Bucket list item not found.' }], isError: true }; - return ok({ success: true }); - } - ); - - // --- ATLAS --- - - if (W) server.registerTool( - 'mark_country_visited', - { - description: 'Mark a country as visited in your Atlas.', - inputSchema: { - country_code: z.string().length(2).toUpperCase().describe('ISO 3166-1 alpha-2 country code (e.g. "FR", "JP")'), - }, - annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, - }, - async ({ country_code }) => { - if (isDemoUser(userId)) return demoDenied(); - markCountryVisited(userId, country_code.toUpperCase()); - return ok({ success: true, country_code: country_code.toUpperCase() }); - } - ); - - if (W) server.registerTool( - 'unmark_country_visited', - { - description: 'Remove a country from your visited countries in Atlas.', - inputSchema: { - country_code: z.string().length(2).toUpperCase().describe('ISO 3166-1 alpha-2 country code'), - }, - annotations: TOOL_ANNOTATIONS_DELETE, - }, - async ({ country_code }) => { - if (isDemoUser(userId)) return demoDenied(); - unmarkCountryVisited(userId, country_code.toUpperCase()); - return ok({ success: true, country_code: country_code.toUpperCase() }); - } - ); - - // --- ATLAS EXPANDED --- - - if (R) server.registerTool( - 'get_atlas_stats', - { - description: 'Get atlas statistics — total visited countries, region counts, continent breakdown.', - inputSchema: {}, - annotations: TOOL_ANNOTATIONS_READONLY, - }, - async () => { - const stats = await getAtlasStats(userId); - return ok({ stats }); - } - ); - - if (R) server.registerTool( - 'list_visited_regions', - { - description: 'List all manually visited sub-country regions for the current user.', - inputSchema: {}, - annotations: TOOL_ANNOTATIONS_READONLY, - }, - async () => { - const regions = listManuallyVisitedRegions(userId); - return ok({ regions }); - } - ); - - if (W) server.registerTool( - 'mark_region_visited', - { - description: 'Mark a sub-country region as visited.', - inputSchema: { - regionCode: z.string().describe('ISO region code e.g. US-CA'), - regionName: z.string(), - countryCode: z.string().describe('ISO 3166-1 alpha-2 country code'), - }, - annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, - }, - async ({ regionCode, regionName, countryCode }) => { - if (isDemoUser(userId)) return demoDenied(); - markRegionVisited(userId, regionCode, regionName, countryCode); - const row = listManuallyVisitedRegions(userId).find(r => r.region_code === regionCode); - // Echo in the client-facing shape ({ code, name, ... }) rather than raw DB columns. - const region = row - ? { code: row.region_code, name: row.region_name, country_code: row.country_code, manuallyMarked: true } - : undefined; - return ok({ region }); - } - ); - - if (W) server.registerTool( - 'unmark_region_visited', - { - description: 'Remove a region from the visited list.', - inputSchema: { - regionCode: z.string(), - }, - annotations: TOOL_ANNOTATIONS_DELETE, - }, - async ({ regionCode }) => { - if (isDemoUser(userId)) return demoDenied(); - unmarkRegionVisited(userId, regionCode); - return ok({ success: true }); - } - ); - - if (R) server.registerTool( - 'get_country_atlas_places', - { - description: 'Get places saved in the user\'s atlas for a specific country.', - inputSchema: { - countryCode: z.string().describe('ISO 3166-1 alpha-2 country code'), - }, - annotations: TOOL_ANNOTATIONS_READONLY, - }, - async ({ countryCode }) => { - const result = getCountryPlaces(userId, countryCode); - return ok(result); - } - ); - - if (W) server.registerTool( - 'update_bucket_list_item', - { - description: 'Update a bucket list item (notes, name, target date, location).', - inputSchema: { - itemId: z.number().int().positive(), - name: z.string().optional(), - notes: z.string().optional(), - lat: z.number().nullable().optional(), - lng: z.number().nullable().optional(), - country_code: z.string().optional(), - target_date: z.string().nullable().optional(), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ itemId, name, notes, lat, lng, country_code, target_date }) => { - if (isDemoUser(userId)) return demoDenied(); - const item = updateBucketItem(userId, itemId, { name, notes, lat, lng, country_code, target_date }); - if (!item) return { content: [{ type: 'text' as const, text: 'Bucket list item not found.' }], isError: true }; - return ok({ item }); - } - ); -} diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 49e99f9b7..c8e9d5d2a 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -312,6 +312,38 @@ the plugin host swapped its 7 collections imports for the injected service — its 22nd constructor dep — and NO bridge was needed anywhere; the dead `buildDedupSet` module helper was dropped in the move, the only line that didn't relocate verbatim). +atlasService followed (1612 lines split two ways, places precedent: the DB +half — stats aggregation, visited countries/regions with the #1490 +tombstone/cascade logic, bucket-list CRUD — folded into `AtlasService` over +DatabaseService alone, while the ~750-line pure-geo half — the bundled +admin0/admin1 stores and their #1576 OOM-shaped streaming builders, the +point-in-polygon indexes, Nominatim geocoding with its shared ≥1.1s throttle, +the 50k geocode cache with its import-time unref'd cleanup interval, and the +`geocodingInFlight` dedup set — moved to the plain module `atlas-geo.ts` so +those caches stay process-global across the container instance, the bridge +instance and test helpers; `assetPath` re-anchored one directory deeper, the +only non-verbatim line. The 10-tool legacy registrar `mcp/tools/atlas.ts` +plus all four atlas resources in `mcp/resources.ts` (`trek://bucket-list`, +`trek://visited-countries`, `trek://atlas/stats`, `trek://atlas/regions`) +moved to `atlas.mcp.ts` — here the `when:` atlas-addon gate IS parity, since +the legacy registrar and resources both gated on the addon while the REST +controller deliberately does not; the mark_region_visited / +get_country_atlas_places no-uppercase divergence from REST relocated +untouched. The plugin host swapped its 9 atlas imports for the injected +service — its 23rd constructor dep — and a minimal 2-export `atlas.bridge.ts` +exists solely for the legacy `authService.getTravelStats` edge +(`getCountryFromCoords` re-exported straight from atlas-geo, +`getHiddenCountries` over the bridge instance); it dies when authService +migrates. resources.test.ts retired with its last two cases — every resource +it covered now lives in the domain suites. The trailing `fix(server)` quirk +commit then wrapped the four multi-statement mark/unmark writes in +`db.transaction()` (the region cascade nests as a savepoint), made the +trip-less `countryPlaces` early return honour `manually_marked`, fixed the +`|| null` bindings that dropped `lat: 0`/`lng: 0`/`notes: ''` on bucket +update, user-scoped the mutating bucket SQL, and uppercased the MCP +region/country-places codes to match REST — each pinned by +`ATLAS-SVC-031…036` plus two MCP casing cases; see migration-graph.md's +"Quirks fixed after the atlas fold".) Repeat these steps per service (next up: **the notifications fan-in** — per the dependency-honest order in diff --git a/server/src/services/atlasService.ts b/server/src/nest/atlas/atlas-geo.ts similarity index 60% rename from server/src/services/atlasService.ts rename to server/src/nest/atlas/atlas-geo.ts index 0de196ec0..2b9aaf8d2 100644 --- a/server/src/services/atlasService.ts +++ b/server/src/nest/atlas/atlas-geo.ts @@ -1,11 +1,21 @@ -import { db } from '../db/database'; -import { Trip, Place } from '../types'; -import { CONTINENT_MAP } from '@trek/shared'; +import { Place } from '../../types'; import fs from 'fs'; import path from 'path'; import zlib from 'zlib'; +// ── Pure geo machinery for the atlas domain ───────────────────────────────── +// +// Plain module on purpose (same class as maps.helpers.ts / tripAccess): no DI, +// no DB access, no Nest imports. The caches below (admin0 gz bytes, admin1 +// per-country store, geocode/region caches, country poly/box indexes, the +// shared Nominatim throttle and the geocodingInFlight dedup set) are +// file-scoped so their lifetime stays process-global no matter how many +// AtlasService instances exist (container singleton + atlas.bridge.ts + +// test helpers) — instance state would duplicate the multi-MB indexes the +// #1576 OOM fix exists to avoid and split the ≥1.1s Nominatim throttle. +// Relocated verbatim from the legacy services/atlasService.ts fold. + // ── Bundled boundary GeoJSON (admin-0 countries + admin-1 regions) ───────── // // Sourced from geoBoundaries (CC BY 4.0), normalized + quantized offline by @@ -14,8 +24,8 @@ import zlib from 'zlib'; // (Replaces the previous runtime fetch of Natural Earth, which was stale for recent // sub-national reforms and depicts some contested borders in unwanted ways.) // -// __dirname is server/dist/services at runtime and server/src/services under vitest; -// both resolve ../../assets to server/assets. +// __dirname is server/dist/nest/atlas at runtime and server/src/nest/atlas under +// vitest; both resolve ../../../assets to server/assets. // Neither parsed bundle is cached. admin0 (~145MB) and admin1 (~260MB) parsed at once are // what exhausted a 512MB host while using Atlas (#1576). Instead we retain only compact @@ -25,7 +35,7 @@ import zlib from 'zlib'; // bundle is never materialised in one piece. function assetPath(name: 'admin0' | 'admin1'): string { - return path.join(__dirname, '..', '..', 'assets', 'atlas', `${name}.geojson.gz`); + return path.join(__dirname, '..', '..', '..', 'assets', 'atlas', `${name}.geojson.gz`); } let admin0Gz: Buffer | null | undefined; @@ -227,7 +237,10 @@ export function setCache(lat: number, lng: number, code: string | null): void { geocodeCache.set(cacheKey(lat, lng), code); } -// Periodically trim the cache so it doesn't grow unbounded +// Periodically trim the cache so it doesn't grow unbounded. +// Import-time side effect preserved verbatim from the legacy module (parity +// exception, same class as audit-log.logger.ts): unref'd so it never holds the +// process open. const CACHE_MAX = 50_000; const CACHE_CLEANUP_MS = 10 * 60 * 1000; setInterval(() => { @@ -802,7 +815,7 @@ async function resolveCountryCode(place: Place): Promise { return null; } -function resolveCountryCodeSync(place: Place): string | null { +export function resolveCountryCodeSync(place: Place): string | null { const hasCoords = !!(place.lat && place.lng); if (hasCoords) { const fromCoords = getCountryFromCoords(place.lat!, place.lng!); @@ -815,444 +828,6 @@ function resolveCountryCodeSync(place: Place): string | null { return null; } -// ── Shared query: all trips the user owns or is a member of ───────────────── - -function getUserTrips(userId: number): Trip[] { - return db - .prepare( - ` - SELECT DISTINCT t.* FROM trips t - LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? - WHERE t.user_id = ? OR m.user_id = ? - ORDER BY t.start_date DESC - `, - ) - .all(userId, userId, userId) as Trip[]; -} - -function getPlacesForTrips(tripIds: number[]): Place[] { - if (tripIds.length === 0) return []; - const placeholders = tripIds.map(() => '?').join(','); - return db.prepare(`SELECT * FROM places WHERE trip_id IN (${placeholders})`).all(...tripIds) as Place[]; -} - -// ── Country resolution (batch DB cache + sync fallback + background geocoding) ── - -function resolvePlaceCountries(places: Place[]): Map { - const out = new Map(); - const geoPlaces = places.filter((p) => p.lat && p.lng); - const placeIds = geoPlaces.map((p) => p.id); - - const cached = - placeIds.length > 0 - ? (db - .prepare( - `SELECT place_id, country_code FROM place_regions WHERE place_id IN (${placeIds.map(() => '?').join(',')})`, - ) - .all(...placeIds) as { place_id: number; country_code: string }[]) - : []; - const cachedMap = new Map(cached.map((r) => [r.place_id, r.country_code])); - - const uncachedForGeocode: Place[] = []; - for (const p of places) { - const fromDb = cachedMap.get(p.id); - if (fromDb) { - out.set(p.id, fromDb); - continue; - } - const sync = resolveCountryCodeSync(p); - if (sync) { - out.set(p.id, sync); - continue; - } - if (p.lat && p.lng && !geocodingInFlight.has(p.id)) { - uncachedForGeocode.push(p); - } - } - - if (uncachedForGeocode.length > 0) { - const insertStmt = db.prepare( - 'INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, ?, ?, ?)', - ); - for (const p of uncachedForGeocode) geocodingInFlight.add(p.id); - void (async () => { - try { - for (const place of uncachedForGeocode) { - try { - const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address); - if (info) insertStmt.run(place.id, info.country_code, info.region_code, info.region_name); - } catch { - /* continue */ - } finally { - geocodingInFlight.delete(place.id); - } - } - } catch { - for (const p of uncachedForGeocode) geocodingInFlight.delete(p.id); - } - })(); - } - - return out; -} - -// ── getStats ──────────────────────────────────────────────────────────────── - -export async function getStats(userId: number) { - const trips = getUserTrips(userId); - const tripIds = trips.map((t) => t.id); - - if (tripIds.length === 0) { - const hiddenOnly = getHiddenCountries(userId); - const manualCountries = db.prepare('SELECT country_code FROM visited_countries WHERE user_id = ?').all(userId) as { - country_code: string; - }[]; - const countries = manualCountries - .filter((mc) => !hiddenOnly.has(mc.country_code)) - .map((mc) => ({ code: mc.country_code, placeCount: 0, tripCount: 0, firstVisit: null, lastVisit: null })); - return { - countries, - trips: [], - stats: { totalTrips: 0, totalPlaces: 0, totalCountries: countries.length, totalDays: 0 }, - }; - } - - const places = getPlacesForTrips(tripIds); - - interface CountryEntry { - code: string; - places: { id: number; name: string; lat: number | null; lng: number | null }[]; - tripIds: Set; - } - const placeCountries = resolvePlaceCountries(places); - const countrySet = new Map(); - for (const place of places) { - const code = placeCountries.get(place.id); - if (code) { - if (!countrySet.has(code)) { - countrySet.set(code, { code, places: [], tripIds: new Set() }); - } - countrySet - .get(code)! - .places.push({ id: place.id, name: place.name, lat: place.lat ?? null, lng: place.lng ?? null }); - countrySet.get(code)!.tripIds.add(place.trip_id); - } - } - - let totalDays = 0; - for (const trip of trips) { - if (trip.start_date && trip.end_date) { - const start = new Date(trip.start_date); - const end = new Date(trip.end_date); - const diff = Math.ceil((end.getTime() - start.getTime()) / (1000 * 60 * 60 * 24)) + 1; - if (diff > 0) totalDays += diff; - } - } - - const countries = [...countrySet.values()].map((c) => { - const countryTrips = trips.filter((t) => c.tripIds.has(t.id)); - const dates = countryTrips - .map((t) => t.start_date) - .filter(Boolean) - .sort(); - return { - code: c.code, - placeCount: c.places.length, - tripCount: c.tripIds.size, - firstVisit: dates[0] || null, - lastVisit: dates[dates.length - 1] || null, - }; - }); - - const citySet = new Set(); - for (const place of places) { - if (place.address) { - const parts = place.address - .split(',') - .map((s: string) => s.trim()) - .filter(Boolean); - // The last part is the country; the city is usually right before it, but a - // full formatted address can have a postal code sitting between them - // (e.g. "Bucharest, 010071, Romania"). Walk back from the country and take - // the first part that still has letters once digits/postal noise is stripped. - const candidates = parts.length >= 2 ? parts.slice(0, -1) : parts; - let city = ''; - for (let i = candidates.length - 1; i >= 0; i--) { - const cleaned = candidates[i].replace(/[\d\-\u2212\u3012]+/g, '').trim(); - if (cleaned) { - city = cleaned.toLowerCase(); - break; - } - } - if (city) citySet.add(city); - } - } - const totalCities = citySet.size; - - // Countries the user explicitly removed. Only the zero-count passes below are - // suppressed — a country with real places isn't removable in the UI anyway, and once - // the user adds a place there the tombstone should stop mattering (#1490). - const hidden = getHiddenCountries(userId); - - // Merge manually marked countries - const manualCountries = db.prepare('SELECT country_code FROM visited_countries WHERE user_id = ?').all(userId) as { - country_code: string; - }[]; - for (const mc of manualCountries) { - if (hidden.has(mc.country_code)) continue; - if (!countries.find((c) => c.code === mc.country_code)) { - countries.push({ code: mc.country_code, placeCount: 0, tripCount: 0, firstVisit: null, lastVisit: null }); - } - } - - // Merge countries reached only by a transport booking. Those store geocoded from/to - // coordinates in reservation_endpoints but create no place row, so they never show up - // via resolvePlaceCountries above and would otherwise be missed (#1366). - // Only 'from'/'to' legs count as actually reached — a 'stop' is an intermediate - // connection/layover (e.g. a plane change) the traveler never really visited. - const endpoints = db - .prepare( - ` - SELECT DISTINCT e.lat, e.lng - FROM reservation_endpoints e - JOIN reservations r ON e.reservation_id = r.id - WHERE r.trip_id IN (${tripIds.map(() => '?').join(',')}) AND e.role IN ('from', 'to') - `, - ) - .all(...tripIds) as { lat: number; lng: number }[]; - for (const e of endpoints) { - const code = getCountryFromCoords(e.lat, e.lng); - if (code && !hidden.has(code) && !countries.find((c) => c.code === code)) { - countries.push({ code, placeCount: 0, tripCount: 0, firstVisit: null, lastVisit: null }); - } - } - - const mostVisited = countries.length > 0 ? countries.reduce((a, b) => (a.placeCount > b.placeCount ? a : b)) : null; - - const continents: Record = {}; - countries.forEach((c) => { - const cont = CONTINENT_MAP[c.code] || 'Other'; - continents[cont] = (continents[cont] || 0) + 1; - }); - - const now = new Date().toISOString().split('T')[0]; - const pastTrips = trips - .filter((t) => t.end_date && t.end_date <= now) - .sort((a, b) => b.end_date!.localeCompare(a.end_date!)); - const lastTrip: { - id: number; - title: string; - start_date?: string | null; - end_date?: string | null; - countryCode?: string; - } | null = pastTrips[0] - ? { - id: pastTrips[0].id, - title: pastTrips[0].title, - start_date: pastTrips[0].start_date, - end_date: pastTrips[0].end_date, - } - : null; - if (lastTrip) { - const lastTripPlaces = places.filter((p) => p.trip_id === lastTrip.id); - for (const p of lastTripPlaces) { - const code = resolveCountryCodeSync(p); - if (code) { - lastTrip.countryCode = code; - break; - } - } - } - - const futureTrips = trips - .filter((t) => t.start_date && t.start_date > now) - .sort((a, b) => a.start_date!.localeCompare(b.start_date!)); - const nextTrip: { id: number; title: string; start_date?: string | null; daysUntil?: number } | null = futureTrips[0] - ? { id: futureTrips[0].id, title: futureTrips[0].title, start_date: futureTrips[0].start_date } - : null; - if (nextTrip) { - const diff = Math.ceil((new Date(nextTrip.start_date!).getTime() - new Date().getTime()) / (1000 * 60 * 60 * 24)); - nextTrip.daysUntil = Math.max(0, diff); - } - - const tripYears = new Set(trips.filter((t) => t.start_date).map((t) => parseInt(t.start_date!.split('-')[0]))); - let streak = 0; - const currentYear = new Date().getFullYear(); - for (let y = currentYear; y >= 2000; y--) { - if (tripYears.has(y)) streak++; - else break; - } - const firstYear = tripYears.size > 0 ? Math.min(...tripYears) : null; - - return { - countries, - stats: { - totalTrips: trips.length, - totalPlaces: places.length, - totalCountries: countries.length, - totalDays, - totalCities, - }, - mostVisited, - continents, - lastTrip, - nextTrip, - streak, - firstYear, - tripsThisYear: trips.filter((t) => t.start_date && t.start_date.startsWith(String(currentYear))).length, - }; -} - -// ── getCountryPlaces ──────────────────────────────────────────────────────── - -export function getCountryPlaces(userId: number, code: string) { - const trips = getUserTrips(userId); - const tripIds = trips.map((t) => t.id); - if (tripIds.length === 0) return { places: [], trips: [], manually_marked: false }; - - const places = getPlacesForTrips(tripIds); - - const matchingPlaces: { - id: number; - name: string; - address: string | null; - lat: number | null; - lng: number | null; - trip_id: number; - }[] = []; - const matchingTripIds = new Set(); - - for (const place of places) { - const pCode = resolveCountryCodeSync(place); - if (pCode === code) { - matchingPlaces.push({ - id: place.id, - name: place.name, - address: place.address ?? null, - lat: place.lat ?? null, - lng: place.lng ?? null, - trip_id: place.trip_id, - }); - matchingTripIds.add(place.trip_id); - } - } - - const matchingTrips = trips - .filter((t) => matchingTripIds.has(t.id)) - .map((t) => ({ id: t.id, title: t.title, start_date: t.start_date, end_date: t.end_date })); - - const isManuallyMarked = !!db - .prepare('SELECT 1 FROM visited_countries WHERE user_id = ? AND country_code = ?') - .get(userId, code); - return { places: matchingPlaces, trips: matchingTrips, manually_marked: isManuallyMarked }; -} - -// ── Mark / unmark country ─────────────────────────────────────────────────── - -export function listVisitedCountries(userId: number): { country_code: string; created_at: string }[] { - return db - .prepare('SELECT country_code, created_at FROM visited_countries WHERE user_id = ? ORDER BY created_at DESC') - .all(userId) as { country_code: string; created_at: string }[]; -} - -/** Countries the user explicitly removed, which getStats must not re-derive (#1490). */ -export function getHiddenCountries(userId: number): Set { - const rows = db.prepare('SELECT country_code FROM hidden_countries WHERE user_id = ?').all(userId) as { - country_code: string; - }[]; - return new Set(rows.map((r) => r.country_code)); -} - -export function markCountryVisited(userId: number, code: string): void { - db.prepare('INSERT OR IGNORE INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(userId, code); - // Marking it visited again lifts a previous removal. - db.prepare('DELETE FROM hidden_countries WHERE user_id = ? AND country_code = ?').run(userId, code); -} - -export function unmarkCountryVisited(userId: number, code: string): void { - db.prepare('DELETE FROM visited_countries WHERE user_id = ? AND country_code = ?').run(userId, code); - db.prepare('DELETE FROM visited_regions WHERE user_id = ? AND country_code = ?').run(userId, code); - // A country derived from a place or a transport endpoint has no visited_countries row, - // so the deletes above are no-ops and getStats would re-derive it on the next request. - // Tombstone it so the removal actually sticks (#1490). - db.prepare('INSERT OR IGNORE INTO hidden_countries (user_id, country_code) VALUES (?, ?)').run(userId, code); -} - -// ── Mark / unmark region ──────────────────────────────────────────────────── - -export function listManuallyVisitedRegions( - userId: number, -): { region_code: string; region_name: string; country_code: string }[] { - return db - .prepare( - 'SELECT region_code, region_name, country_code FROM visited_regions WHERE user_id = ? ORDER BY created_at DESC', - ) - .all(userId) as { region_code: string; region_name: string; country_code: string }[]; -} - -/** Regions the user explicitly removed, which getVisitedRegions must not re-derive. */ -export function getHiddenRegions(userId: number): Set { - const rows = db.prepare('SELECT region_code FROM hidden_regions WHERE user_id = ?').all(userId) as { region_code: string }[]; - return new Set(rows.map(r => r.region_code)); -} - -// Bundled/geocoded region codes are always "-" (ISO 3166-2 format, and -// buildRegionInfo's synthesized fallback follows the same convention) — used to recover a -// region's country when there's no visited_regions row to read it from (a place-derived -// region was never inserted there). -function countryCodeFromRegionCode(regionCode: string): string { - return (regionCode.split('-')[0] || '').toUpperCase(); -} - -export function markRegionVisited(userId: number, regionCode: string, regionName: string, countryCode: string): void { - db.prepare('INSERT OR IGNORE INTO visited_regions (user_id, region_code, region_name, country_code) VALUES (?, ?, ?, ?)').run(userId, regionCode, regionName, countryCode); - // Re-marking lifts a previous removal of the region itself... - db.prepare('DELETE FROM hidden_regions WHERE user_id = ? AND region_code = ?').run(userId, regionCode); - // ...and of the parent country, which the "last region removed" cascade in - // unmarkRegionVisited below may have hidden — otherwise marking a region visited again - // would leave its country invisible. - db.prepare('INSERT OR IGNORE INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(userId, countryCode); - db.prepare('DELETE FROM hidden_countries WHERE user_id = ? AND country_code = ?').run(userId, countryCode); -} - -// True when the given country still has at least one region that would show as visited — -// derived from place_regions or manually marked — after excluding the given user's hidden -// regions. Used to decide whether removing a region should cascade into hiding the country. -function hasVisibleRegionForCountry(userId: number, countryCode: string, hidden: Set): boolean { - const tripIds = getUserTrips(userId).map(t => t.id); - const placeIds = getPlacesForTrips(tripIds).filter(p => p.lat && p.lng).map(p => p.id); - const placeRegionCodes = placeIds.length > 0 - ? (db.prepare( - `SELECT DISTINCT region_code FROM place_regions WHERE country_code = ? AND place_id IN (${placeIds.map(() => '?').join(',')})` - ).all(countryCode, ...placeIds) as { region_code: string }[]).map(r => r.region_code) - : []; - const manualRegionCodes = (db.prepare( - 'SELECT region_code FROM visited_regions WHERE user_id = ? AND country_code = ?' - ).all(userId, countryCode) as { region_code: string }[]).map(r => r.region_code); - return [...placeRegionCodes, ...manualRegionCodes].some(code => !hidden.has(code)); -} - -export function unmarkRegionVisited(userId: number, regionCode: string): void { - const region = db.prepare('SELECT country_code FROM visited_regions WHERE user_id = ? AND region_code = ?').get(userId, regionCode) as { country_code: string } | undefined; - const countryCode = region?.country_code || countryCodeFromRegionCode(regionCode); - - db.prepare('DELETE FROM visited_regions WHERE user_id = ? AND region_code = ?').run(userId, regionCode); - - // Tombstone unconditionally, not just for a manually-marked region — a region derived - // from real place data (the common case) needs to be dismissable too, mirroring - // unmarkCountryVisited's tombstone for a place-derived country (#1490). - if (countryCode) { - db.prepare('INSERT OR IGNORE INTO hidden_regions (user_id, region_code, country_code) VALUES (?, ?, ?)').run(userId, regionCode, countryCode); - - // If that was the country's last visible region, hide the country too — otherwise it - // keeps showing "visited" on the world map with nothing left to drill into. - const hidden = getHiddenRegions(userId); - if (!hasVisibleRegionForCountry(userId, countryCode, hidden)) { - unmarkCountryVisited(userId, countryCode); - } - } -} - // ── Sub-national region resolution ──────────────────────────────────────── export interface RegionInfo { @@ -1262,7 +837,9 @@ export interface RegionInfo { } // Tracks place IDs currently being geocoded in the background to prevent duplicate enqueuing. -const geocodingInFlight = new Set(); +// Exported so AtlasService's background-geocode loops (container + bridge instances) share +// one dedup set. +export const geocodingInFlight = new Set(); const regionCache = new Map(); @@ -1397,7 +974,7 @@ function buildRegionInfo(address: Record, preferFinest: boolean) }; } -async function reverseGeocodeRegion(lat: number, lng: number, placeAddress?: string | null): Promise { +export async function reverseGeocodeRegion(lat: number, lng: number, placeAddress?: string | null): Promise { const key = roundKey(lat, lng); if (regionCache.has(key)) return regionCache.get(key)!; @@ -1444,169 +1021,3 @@ async function reverseGeocodeRegion(lat: number, lng: number, placeAddress?: str regionCache.set(key, info); return info; } - -export async function getVisitedRegions( - userId: number, -): Promise<{ regions: Record }> { - const trips = getUserTrips(userId); - const tripIds = trips.map((t) => t.id); - const places = getPlacesForTrips(tripIds); - - // Check DB cache first - const placeIds = places.filter((p) => p.lat && p.lng).map((p) => p.id); - const cached = - placeIds.length > 0 - ? (db - .prepare(`SELECT * FROM place_regions WHERE place_id IN (${placeIds.map(() => '?').join(',')})`) - .all(...placeIds) as { place_id: number; country_code: string; region_code: string; region_name: string }[]) - : []; - const cachedMap = new Map(cached.map((c) => [c.place_id, c])); - - // Kick off background geocoding for uncached places; return cached data immediately. - const uncached = places.filter((p) => p.lat && p.lng && !cachedMap.has(p.id) && !geocodingInFlight.has(p.id)); - if (uncached.length > 0) { - const insertStmt = db.prepare( - 'INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, ?, ?, ?)', - ); - for (const p of uncached) geocodingInFlight.add(p.id); - void (async () => { - try { - for (const place of uncached) { - try { - const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address); - if (info) insertStmt.run(place.id, info.country_code, info.region_code, info.region_name); - } catch { - // individual failure — continue with remaining places - } finally { - geocodingInFlight.delete(place.id); - } - } - } catch { - for (const p of uncached) geocodingInFlight.delete(p.id); - } - })(); - } - - // Group by country → regions with place counts - const regionMap: Record> = {}; - for (const [, entry] of cachedMap) { - if (!regionMap[entry.country_code]) regionMap[entry.country_code] = new Map(); - const existing = regionMap[entry.country_code].get(entry.region_code); - if (existing) { - existing.placeCount++; - } else { - regionMap[entry.country_code].set(entry.region_code, { - code: entry.region_code, - name: entry.region_name, - placeCount: 1, - }); - } - } - - const result: Record = {}; - for (const [country, regions] of Object.entries(regionMap)) { - result[country] = [...regions.values()]; - } - - // Merge manually marked regions - const manualRegions = listManuallyVisitedRegions(userId); - for (const r of manualRegions) { - if (!result[r.country_code]) result[r.country_code] = []; - if (!result[r.country_code].find((x) => x.code === r.region_code)) { - result[r.country_code].push({ code: r.region_code, name: r.region_name, placeCount: 0, manuallyMarked: true }); - } - } - - // Suppress regions the user explicitly removed, same as getStats does for countries - // via getHiddenCountries (#1490) — otherwise a region derived fresh from place_regions - // (or a manual mark) on every request could never actually be dismissed. - const hidden = getHiddenRegions(userId); - if (hidden.size > 0) { - for (const country of Object.keys(result)) { - result[country] = result[country].filter(r => !hidden.has(r.code)); - if (result[country].length === 0) delete result[country]; - } - } - - return { regions: result }; -} - -// ── Bucket list CRUD ──────────────────────────────────────────────────────── - -export function listBucketList(userId: number) { - return db.prepare('SELECT * FROM bucket_list WHERE user_id = ? ORDER BY created_at DESC').all(userId); -} - -export function createBucketItem( - userId: number, - data: { - name: string; - lat?: number | null; - lng?: number | null; - country_code?: string | null; - notes?: string | null; - target_date?: string | null; - }, -) { - const result = db - .prepare( - 'INSERT INTO bucket_list (user_id, name, lat, lng, country_code, notes, target_date) VALUES (?, ?, ?, ?, ?, ?, ?)', - ) - .run( - userId, - data.name.trim(), - data.lat ?? null, - data.lng ?? null, - data.country_code ?? null, - data.notes ?? null, - data.target_date ?? null, - ); - return db.prepare('SELECT * FROM bucket_list WHERE id = ?').get(result.lastInsertRowid); -} - -export function updateBucketItem( - userId: number, - itemId: string | number, - data: { - name?: string; - notes?: string; - lat?: number | null; - lng?: number | null; - country_code?: string | null; - target_date?: string | null; - }, -) { - const item = db.prepare('SELECT * FROM bucket_list WHERE id = ? AND user_id = ?').get(itemId, userId); - if (!item) return null; - db.prepare( - `UPDATE bucket_list SET - name = COALESCE(?, name), - notes = CASE WHEN ? THEN ? ELSE notes END, - lat = CASE WHEN ? THEN ? ELSE lat END, - lng = CASE WHEN ? THEN ? ELSE lng END, - country_code = CASE WHEN ? THEN ? ELSE country_code END, - target_date = CASE WHEN ? THEN ? ELSE target_date END - WHERE id = ?`, - ).run( - data.name?.trim() || null, - data.notes !== undefined ? 1 : 0, - data.notes !== undefined ? data.notes || null : null, - data.lat !== undefined ? 1 : 0, - data.lat !== undefined ? data.lat || null : null, - data.lng !== undefined ? 1 : 0, - data.lng !== undefined ? data.lng || null : null, - data.country_code !== undefined ? 1 : 0, - data.country_code !== undefined ? data.country_code || null : null, - data.target_date !== undefined ? 1 : 0, - data.target_date !== undefined ? data.target_date || null : null, - itemId, - ); - return db.prepare('SELECT * FROM bucket_list WHERE id = ?').get(itemId); -} - -export function deleteBucketItem(userId: number, itemId: string | number): boolean { - const item = db.prepare('SELECT * FROM bucket_list WHERE id = ? AND user_id = ?').get(itemId, userId); - if (!item) return false; - db.prepare('DELETE FROM bucket_list WHERE id = ?').run(itemId); - return true; -} diff --git a/server/src/nest/atlas/atlas.bridge.ts b/server/src/nest/atlas/atlas.bridge.ts new file mode 100644 index 000000000..6d30f91f0 --- /dev/null +++ b/server/src/nest/atlas/atlas.bridge.ts @@ -0,0 +1,29 @@ +import { db } from '../../db/database'; +import { DatabaseService } from '../database/database.service'; +import { AtlasService } from './atlas.service'; + +/** + * Non-Nest entry point for the atlas domain — for code running OUTSIDE the + * Nest container. Sole consumer today: the legacy services/authService + * (getTravelStats resolves reservation endpoints to countries and subtracts + * the user's hidden ones). The atlas MCP tools and resources moved to the + * DI-discovered atlas.mcp.ts, and the plugin RPC host injects AtlasService, + * so neither needs this. Exports only the legacy services/atlasService names + * authService still consumes, 1:1, so the repoint is an import-path-only + * diff. Inside the container, inject AtlasService instead. Delete this file + * when authService migrates. + * + * getCountryFromCoords is pure geo (no DB) and re-exported straight from the + * atlas-geo helper module; its poly/box indexes are module-scoped there, so + * this entry point and the container share one copy (#1576). + * + * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto + * the shared better-sqlite3 singleton (same pattern as permissions.bridge.ts). + */ +const atlas = new AtlasService(new DatabaseService(db)); + +export { getCountryFromCoords } from './atlas-geo'; + +export function getHiddenCountries(userId: number): Set { + return atlas.getHiddenCountries(userId); +} diff --git a/server/src/nest/atlas/atlas.controller.ts b/server/src/nest/atlas/atlas.controller.ts index 496b4f8ff..62bf73cf0 100644 --- a/server/src/nest/atlas/atlas.controller.ts +++ b/server/src/nest/atlas/atlas.controller.ts @@ -17,6 +17,7 @@ import type { Response } from 'express'; import type { RegionGeo } from '@trek/shared'; import type { User } from '../../types'; import { AtlasService } from './atlas.service'; +import { AtlasMarkRegionDto, AtlasCreateBucketItemDto, AtlasUpdateBucketItemDto } from './atlas.dto'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; @@ -26,9 +27,13 @@ import { CurrentUser } from '../auth/current-user.decorator'; * Byte-identical to the legacy Express route (server/src/routes/atlas.ts): all * endpoints require auth; country/region codes are upper-cased; /regions is * always no-store while /regions/geo is cached for a day only on a non-empty - * result; the mark POSTs answer 200 (not Nest's default 201); and the bespoke - * 400/404 bodies are reproduced exactly. No addon gate — the legacy route has - * none, so adding one would break clients when the addon is off. + * result; the mark POSTs answer 200 (not Nest's default 201); the bespoke 404 + * bodies are reproduced exactly. Bodies validate via @trek/shared DTOs, so a + * missing/invalid field 400s in the ZodValidationPipe envelope (the former + * hand-rolled 'name and country_code are required' — the todo/places trade); + * the whitespace-only bucket name keeps its legacy 'Name is required' 400. + * No addon gate — the legacy route has none, so adding one would break + * clients when the addon is off. */ @Controller('api/addons/atlas') @UseGuards(JwtAuthGuard) @@ -103,13 +108,9 @@ export class AtlasController { markRegion( @CurrentUser() user: User, @Param('code') code: string, - @Body('name') name?: string, - @Body('country_code') countryCode?: string, + @Body() body: AtlasMarkRegionDto, ): { success: boolean } { - if (!name || !countryCode) { - throw new HttpException({ error: 'name and country_code are required' }, 400); - } - this.atlas.markRegion(user.id, code.toUpperCase(), name, countryCode.toUpperCase()); + this.atlas.markRegion(user.id, code.toUpperCase(), body.name, body.country_code.toUpperCase()); return { success: true }; } @@ -125,10 +126,9 @@ export class AtlasController { } @Post('bucket-list') - createBucketItem( - @CurrentUser() user: User, - @Body() body: { name?: string; lat?: number | null; lng?: number | null; country_code?: string | null; notes?: string | null; target_date?: string | null }, - ): { item: unknown } { + createBucketItem(@CurrentUser() user: User, @Body() body: AtlasCreateBucketItemDto): { item: unknown } { + // The schema's min(1) admits whitespace-only names — this trim guard keeps + // the legacy 400 for those (missing/empty names 400 in the pipe envelope). if (!body.name?.trim()) { throw new HttpException({ error: 'Name is required' }, 400); } @@ -140,7 +140,7 @@ export class AtlasController { updateBucketItem( @CurrentUser() user: User, @Param('id') id: string, - @Body() body: { name?: string; notes?: string; lat?: number | null; lng?: number | null; country_code?: string | null; target_date?: string | null }, + @Body() body: AtlasUpdateBucketItemDto, ): { item: unknown } { const { name, notes, lat, lng, country_code, target_date } = body; const item = this.atlas.updateBucketItem(user.id, id, { name, notes, lat, lng, country_code, target_date }); diff --git a/server/src/nest/atlas/atlas.dto.ts b/server/src/nest/atlas/atlas.dto.ts new file mode 100644 index 000000000..69edc448b --- /dev/null +++ b/server/src/nest/atlas/atlas.dto.ts @@ -0,0 +1,16 @@ +import { createZodDto } from 'nestjs-zod'; +import { + markRegionRequestSchema, + createBucketItemRequestSchema, + updateBucketItemRequestSchema, +} from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared atlas contracts. The + * global ZodValidationPipe (APP_PIPE in app.module.ts) validates any @Body() + * parameter typed with one of these classes by metatype — the Zod schemas in + * shared/ remain the single source of truth for the wire contract. + */ +export class AtlasMarkRegionDto extends createZodDto(markRegionRequestSchema) {} +export class AtlasCreateBucketItemDto extends createZodDto(createBucketItemRequestSchema) {} +export class AtlasUpdateBucketItemDto extends createZodDto(updateBucketItemRequestSchema) {} diff --git a/server/src/nest/atlas/atlas.mcp.ts b/server/src/nest/atlas/atlas.mcp.ts new file mode 100644 index 000000000..9aeb689e1 --- /dev/null +++ b/server/src/nest/atlas/atlas.mcp.ts @@ -0,0 +1,302 @@ +import { + McpController, Tool, Resource, type McpContext, + TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, + TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, + demoDenied, ok, +} from '@trek/nest-mcp'; +import { z } from 'zod'; +import { isDemoUser } from '../../services/authService'; +import { isAddonEnabled } from '../addons/addons.bridge'; +import { ADDON_IDS } from '../../addons'; +import { AtlasService } from './atlas.service'; + +/** Legacy registrar gate: the whole atlas surface (tools AND resources) rides + * the atlas addon — unlike the REST controller, which is deliberately ungated + * (see atlas.controller.ts). */ +const atlasAddonOn = () => isAddonEnabled(ADDON_IDS.ATLAS); + +function jsonContent(uri: string, data: unknown) { + return { + contents: [{ + uri, + mimeType: 'application/json', + text: JSON.stringify(data, null, 2), + }], + }; +} + +/** + * Atlas MCP surface — ported 1:1 from the legacy registrar + * src/mcp/tools/atlas.ts (10 tools) and the four atlas resources in + * src/mcp/resources.ts: identical names, descriptions, zod input schemas, + * annotations, and error/payload shapes. The legacy `if (R)` / `if (W)` scope + * checks map to the declarative atlas read/write access markers (resolved by + * trekMcpAccessPolicy) and the registration-time addon early-return becomes + * the `when:` gate. Atlas rows are uid-scoped, so there are no trip-access + * checks and no broadcasts; writes deny demo users, reads do not. The legacy + * registrar's casing divergence from REST (mark_region_visited / + * get_country_atlas_places passed codes through un-uppercased) was fixed in + * the trailing quirk commit — both sides now uppercase. + */ +@McpController() +export class AtlasMcp { + constructor(private readonly atlas: AtlasService) {} + + // ── Bucket list ───────────────────────────────────────────────────────── + + @Tool({ + name: 'create_bucket_list_item', + description: 'Add a destination to your personal travel bucket list.', + inputSchema: { + name: z.string().min(1).max(200).describe('Destination or experience name'), + lat: z.number().optional(), + lng: z.number().optional(), + country_code: z.string().length(2).toUpperCase().optional().describe('ISO 3166-1 alpha-2 country code'), + notes: z.string().max(1000).optional(), + }, + annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'write' }, + }) + async createBucketListItem( + { name, lat, lng, country_code, notes }: { name: string; lat?: number; lng?: number; country_code?: string; notes?: string }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + const item = this.atlas.createBucketItem(ctx.userId, { name, lat, lng, country_code, notes }); + return ok({ item }); + } + + @Tool({ + name: 'delete_bucket_list_item', + description: 'Remove an item from your travel bucket list.', + inputSchema: { + itemId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_DELETE, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'write' }, + }) + async deleteBucketListItem({ itemId }: { itemId: number }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + const deleted = this.atlas.deleteBucketItem(ctx.userId, itemId); + if (!deleted) return { content: [{ type: 'text' as const, text: 'Bucket list item not found.' }], isError: true }; + return ok({ success: true }); + } + + // ── Atlas ─────────────────────────────────────────────────────────────── + + @Tool({ + name: 'mark_country_visited', + description: 'Mark a country as visited in your Atlas.', + inputSchema: { + country_code: z.string().length(2).toUpperCase().describe('ISO 3166-1 alpha-2 country code (e.g. "FR", "JP")'), + }, + annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'write' }, + }) + async markCountryVisited({ country_code }: { country_code: string }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + this.atlas.markCountry(ctx.userId, country_code.toUpperCase()); + return ok({ success: true, country_code: country_code.toUpperCase() }); + } + + @Tool({ + name: 'unmark_country_visited', + description: 'Remove a country from your visited countries in Atlas.', + inputSchema: { + country_code: z.string().length(2).toUpperCase().describe('ISO 3166-1 alpha-2 country code'), + }, + annotations: TOOL_ANNOTATIONS_DELETE, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'write' }, + }) + async unmarkCountryVisited({ country_code }: { country_code: string }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + this.atlas.unmarkCountry(ctx.userId, country_code.toUpperCase()); + return ok({ success: true, country_code: country_code.toUpperCase() }); + } + + // ── Atlas expanded ────────────────────────────────────────────────────── + + @Tool({ + name: 'get_atlas_stats', + description: 'Get atlas statistics — total visited countries, region counts, continent breakdown.', + inputSchema: {}, + annotations: TOOL_ANNOTATIONS_READONLY, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'read' }, + }) + async getAtlasStats(_args: Record, ctx: McpContext) { + const stats = await this.atlas.stats(ctx.userId); + return ok({ stats }); + } + + @Tool({ + name: 'list_visited_regions', + description: 'List all manually visited sub-country regions for the current user.', + inputSchema: {}, + annotations: TOOL_ANNOTATIONS_READONLY, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'read' }, + }) + async listVisitedRegions(_args: Record, ctx: McpContext) { + const regions = this.atlas.listManuallyVisitedRegions(ctx.userId); + return ok({ regions }); + } + + @Tool({ + name: 'mark_region_visited', + description: 'Mark a sub-country region as visited.', + inputSchema: { + regionCode: z.string().describe('ISO region code e.g. US-CA'), + regionName: z.string(), + countryCode: z.string().describe('ISO 3166-1 alpha-2 country code'), + }, + annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'write' }, + }) + async markRegionVisited( + { regionCode, regionName, countryCode }: { regionCode: string; regionName: string; countryCode: string }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + // Post-fold quirk fix: uppercase both codes, matching the REST controller + // (the legacy registrar passed them through verbatim, so a lowercase mark + // created a row REST's uppercased unmark could never hit). + const upperRegion = regionCode.toUpperCase(); + this.atlas.markRegion(ctx.userId, upperRegion, regionName, countryCode.toUpperCase()); + const row = this.atlas.listManuallyVisitedRegions(ctx.userId).find((r) => r.region_code === upperRegion); + // Echo in the client-facing shape ({ code, name, ... }) rather than raw DB columns. + const region = row + ? { code: row.region_code, name: row.region_name, country_code: row.country_code, manuallyMarked: true } + : undefined; + return ok({ region }); + } + + @Tool({ + name: 'unmark_region_visited', + description: 'Remove a region from the visited list.', + inputSchema: { + regionCode: z.string(), + }, + annotations: TOOL_ANNOTATIONS_DELETE, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'write' }, + }) + async unmarkRegionVisited({ regionCode }: { regionCode: string }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + // Post-fold quirk fix: uppercase, matching the REST controller. + this.atlas.unmarkRegion(ctx.userId, regionCode.toUpperCase()); + return ok({ success: true }); + } + + @Tool({ + name: 'get_country_atlas_places', + description: 'Get places saved in the user\'s atlas for a specific country.', + inputSchema: { + countryCode: z.string().describe('ISO 3166-1 alpha-2 country code'), + }, + annotations: TOOL_ANNOTATIONS_READONLY, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'read' }, + }) + async getCountryAtlasPlaces({ countryCode }: { countryCode: string }, ctx: McpContext) { + // Post-fold quirk fix: uppercase, matching the REST controller (the legacy + // registrar passed 'fr' through and matched nothing). + const result = this.atlas.countryPlaces(ctx.userId, countryCode.toUpperCase()); + return ok(result); + } + + @Tool({ + name: 'update_bucket_list_item', + description: 'Update a bucket list item (notes, name, target date, location).', + inputSchema: { + itemId: z.number().int().positive(), + name: z.string().optional(), + notes: z.string().optional(), + lat: z.number().nullable().optional(), + lng: z.number().nullable().optional(), + country_code: z.string().optional(), + target_date: z.string().nullable().optional(), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + when: atlasAddonOn, + access: { group: 'atlas', mode: 'write' }, + }) + async updateBucketListItem( + { itemId, name, notes, lat, lng, country_code, target_date }: { + itemId: number; + name?: string; + notes?: string; + lat?: number | null; + lng?: number | null; + country_code?: string; + target_date?: string | null; + }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + const item = this.atlas.updateBucketItem(ctx.userId, itemId, { name, notes, lat, lng, country_code, target_date }); + if (!item) return { content: [{ type: 'text' as const, text: 'Bucket list item not found.' }], isError: true }; + return ok({ item }); + } + + // ------------------------------------------------------------------------- + // Resources (from src/mcp/resources.ts — payloads reproduce jsonContent verbatim) + // ------------------------------------------------------------------------- + + @Resource({ + name: 'bucket-list', + uri: 'trek://bucket-list', + description: 'Your personal travel bucket list', + mimeType: 'application/json', + when: atlasAddonOn, + access: { group: 'atlas', mode: 'read' }, + }) + async bucketListResource(uri: URL, ctx: McpContext) { + const items = this.atlas.bucketList(ctx.userId); + return jsonContent(uri.href, items); + } + + @Resource({ + name: 'visited-countries', + uri: 'trek://visited-countries', + description: 'Countries you have marked as visited in Atlas', + mimeType: 'application/json', + when: atlasAddonOn, + access: { group: 'atlas', mode: 'read' }, + }) + async visitedCountriesResource(uri: URL, ctx: McpContext) { + const countries = this.atlas.listVisitedCountries(ctx.userId); + return jsonContent(uri.href, countries); + } + + @Resource({ + name: 'atlas-stats', + uri: 'trek://atlas/stats', + description: "User's atlas statistics — visited country counts and breakdown", + mimeType: 'application/json', + when: atlasAddonOn, + access: { group: 'atlas', mode: 'read' }, + }) + async atlasStatsResource(uri: URL, ctx: McpContext) { + const stats = await this.atlas.stats(ctx.userId); + return jsonContent(uri.href, stats); + } + + @Resource({ + name: 'atlas-regions', + uri: 'trek://atlas/regions', + description: 'List of manually visited regions for the current user', + mimeType: 'application/json', + when: atlasAddonOn, + access: { group: 'atlas', mode: 'read' }, + }) + async atlasRegionsResource(uri: URL, ctx: McpContext) { + const regions = this.atlas.listManuallyVisitedRegions(ctx.userId); + return jsonContent(uri.href, regions); + } +} diff --git a/server/src/nest/atlas/atlas.module.ts b/server/src/nest/atlas/atlas.module.ts index 8d4844189..cd7719990 100644 --- a/server/src/nest/atlas/atlas.module.ts +++ b/server/src/nest/atlas/atlas.module.ts @@ -1,10 +1,17 @@ import { Module } from '@nestjs/common'; import { AtlasController } from './atlas.controller'; import { AtlasService } from './atlas.service'; +import { AtlasMcp } from './atlas.mcp'; -/** Atlas addon domain (L7 leaf module). Registered in AppModule. */ +/** + * Atlas addon domain (L7 leaf module). Registered in AppModule. Exports + * AtlasService for the plugin RPC host (PluginHostDepsFactory injects it). + * AtlasMcp is a provider (not a controller) — the nest-mcp registry discovers + * it after app.init(). + */ @Module({ controllers: [AtlasController], - providers: [AtlasService], + providers: [AtlasService, AtlasMcp], + exports: [AtlasService], }) export class AtlasModule {} diff --git a/server/src/nest/atlas/atlas.service.ts b/server/src/nest/atlas/atlas.service.ts index 4a8f89024..ccefeeb90 100644 --- a/server/src/nest/atlas/atlas.service.ts +++ b/server/src/nest/atlas/atlas.service.ts @@ -1,39 +1,617 @@ import { Injectable } from '@nestjs/common'; +import { CONTINENT_MAP } from '@trek/shared'; +import { Trip, Place } from '../../types'; +import { DatabaseService } from '../database/database.service'; import { - getStats, - getCountryPlaces, - markCountryVisited, - unmarkCountryVisited, - markRegionVisited, - unmarkRegionVisited, - getVisitedRegions, - getRegionGeo, + getCountryFromCoords, getCountryGeoGz, - listBucketList, - createBucketItem, - updateBucketItem, - deleteBucketItem, -} from '../../services/atlasService'; + getRegionGeo, + geocodingInFlight, + resolveCountryCodeSync, + reverseGeocodeRegion, +} from './atlas-geo'; -type CreateBucketData = Parameters[1]; -type UpdateBucketData = Parameters[2]; +export type CreateBucketData = { + name: string; + lat?: number | null; + lng?: number | null; + country_code?: string | null; + notes?: string | null; + target_date?: string | null; +}; + +export type UpdateBucketData = { + name?: string; + notes?: string; + lat?: number | null; + lng?: number | null; + country_code?: string | null; + target_date?: string | null; +}; + +// Bundled/geocoded region codes are always "-" (ISO 3166-2 format, and +// buildRegionInfo's synthesized fallback follows the same convention) — used to recover a +// region's country when there's no visited_regions row to read it from (a place-derived +// region was never inserted there). +function countryCodeFromRegionCode(regionCode: string): string { + return (regionCode.split('-')[0] || '').toUpperCase(); +} /** - * Thin Nest wrapper around the existing atlas service. The Admin-1 GeoJSON - * cache, the stats aggregation and the visited-region logic all stay in - * atlasService, so behaviour is unchanged. Returns native service shapes; the - * client-facing contracts live in @trek/shared. + * DI-native atlas domain service — the legacy services/atlasService SQL + * (stats aggregation, visited countries/regions with the #1490 tombstones, + * bucket-list CRUD) folded in verbatim over the injected DatabaseService. + * The pure-geo machinery (bundled admin0/admin1 boundaries, point-in-polygon + * indexes, Nominatim geocoding and its caches) lives in ./atlas-geo — a plain + * module so the multi-MB caches stay process-global across the container + * instance, the atlas.bridge.ts instance, and test helpers (#1576). + * Returns native service shapes; the client-facing contracts live in + * @trek/shared. No broadcasts: atlas rows are uid-scoped, not trip-scoped. */ @Injectable() export class AtlasService { - stats(userId: number) { - return getStats(userId); + constructor(private readonly db: DatabaseService) {} + + // ── Shared query: all trips the user owns or is a member of ─────────────── + + private getUserTrips(userId: number): Trip[] { + return this.db + .prepare( + ` + SELECT DISTINCT t.* FROM trips t + LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? + WHERE t.user_id = ? OR m.user_id = ? + ORDER BY t.start_date DESC + `, + ) + .all(userId, userId, userId) as Trip[]; } - visitedRegions(userId: number) { - return getVisitedRegions(userId); + private getPlacesForTrips(tripIds: number[]): Place[] { + if (tripIds.length === 0) return []; + const placeholders = tripIds.map(() => '?').join(','); + return this.db.prepare(`SELECT * FROM places WHERE trip_id IN (${placeholders})`).all(...tripIds) as Place[]; } + // ── Country resolution (batch DB cache + sync fallback + background geocoding) ── + + private resolvePlaceCountries(places: Place[]): Map { + const out = new Map(); + const geoPlaces = places.filter((p) => p.lat && p.lng); + const placeIds = geoPlaces.map((p) => p.id); + + const cached = + placeIds.length > 0 + ? (this.db + .prepare( + `SELECT place_id, country_code FROM place_regions WHERE place_id IN (${placeIds.map(() => '?').join(',')})`, + ) + .all(...placeIds) as { place_id: number; country_code: string }[]) + : []; + const cachedMap = new Map(cached.map((r) => [r.place_id, r.country_code])); + + const uncachedForGeocode: Place[] = []; + for (const p of places) { + const fromDb = cachedMap.get(p.id); + if (fromDb) { + out.set(p.id, fromDb); + continue; + } + const sync = resolveCountryCodeSync(p); + if (sync) { + out.set(p.id, sync); + continue; + } + if (p.lat && p.lng && !geocodingInFlight.has(p.id)) { + uncachedForGeocode.push(p); + } + } + + if (uncachedForGeocode.length > 0) { + const insertStmt = this.db.prepare( + 'INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, ?, ?, ?)', + ); + for (const p of uncachedForGeocode) geocodingInFlight.add(p.id); + void (async () => { + try { + for (const place of uncachedForGeocode) { + try { + const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address); + if (info) insertStmt.run(place.id, info.country_code, info.region_code, info.region_name); + } catch { + /* continue */ + } finally { + geocodingInFlight.delete(place.id); + } + } + } catch { + for (const p of uncachedForGeocode) geocodingInFlight.delete(p.id); + } + })(); + } + + return out; + } + + // ── Stats ───────────────────────────────────────────────────────────────── + + async stats(userId: number) { + const trips = this.getUserTrips(userId); + const tripIds = trips.map((t) => t.id); + + if (tripIds.length === 0) { + const hiddenOnly = this.getHiddenCountries(userId); + const manualCountries = this.db + .prepare('SELECT country_code FROM visited_countries WHERE user_id = ?') + .all(userId) as { + country_code: string; + }[]; + const countries = manualCountries + .filter((mc) => !hiddenOnly.has(mc.country_code)) + .map((mc) => ({ code: mc.country_code, placeCount: 0, tripCount: 0, firstVisit: null, lastVisit: null })); + return { + countries, + trips: [], + stats: { totalTrips: 0, totalPlaces: 0, totalCountries: countries.length, totalDays: 0 }, + }; + } + + const places = this.getPlacesForTrips(tripIds); + + interface CountryEntry { + code: string; + places: { id: number; name: string; lat: number | null; lng: number | null }[]; + tripIds: Set; + } + const placeCountries = this.resolvePlaceCountries(places); + const countrySet = new Map(); + for (const place of places) { + const code = placeCountries.get(place.id); + if (code) { + if (!countrySet.has(code)) { + countrySet.set(code, { code, places: [], tripIds: new Set() }); + } + countrySet + .get(code)! + .places.push({ id: place.id, name: place.name, lat: place.lat ?? null, lng: place.lng ?? null }); + countrySet.get(code)!.tripIds.add(place.trip_id); + } + } + + let totalDays = 0; + for (const trip of trips) { + if (trip.start_date && trip.end_date) { + const start = new Date(trip.start_date); + const end = new Date(trip.end_date); + const diff = Math.ceil((end.getTime() - start.getTime()) / (1000 * 60 * 60 * 24)) + 1; + if (diff > 0) totalDays += diff; + } + } + + const countries = [...countrySet.values()].map((c) => { + const countryTrips = trips.filter((t) => c.tripIds.has(t.id)); + const dates = countryTrips + .map((t) => t.start_date) + .filter(Boolean) + .sort(); + return { + code: c.code, + placeCount: c.places.length, + tripCount: c.tripIds.size, + firstVisit: dates[0] || null, + lastVisit: dates[dates.length - 1] || null, + }; + }); + + const citySet = new Set(); + for (const place of places) { + if (place.address) { + const parts = place.address + .split(',') + .map((s: string) => s.trim()) + .filter(Boolean); + // The last part is the country; the city is usually right before it, but a + // full formatted address can have a postal code sitting between them + // (e.g. "Bucharest, 010071, Romania"). Walk back from the country and take + // the first part that still has letters once digits/postal noise is stripped. + const candidates = parts.length >= 2 ? parts.slice(0, -1) : parts; + let city = ''; + for (let i = candidates.length - 1; i >= 0; i--) { + const cleaned = candidates[i].replace(/[\d\-\u2212\u3012]+/g, '').trim(); + if (cleaned) { + city = cleaned.toLowerCase(); + break; + } + } + if (city) citySet.add(city); + } + } + const totalCities = citySet.size; + + // Countries the user explicitly removed. Only the zero-count passes below are + // suppressed — a country with real places isn't removable in the UI anyway, and once + // the user adds a place there the tombstone should stop mattering (#1490). + const hidden = this.getHiddenCountries(userId); + + // Merge manually marked countries + const manualCountries = this.db + .prepare('SELECT country_code FROM visited_countries WHERE user_id = ?') + .all(userId) as { + country_code: string; + }[]; + for (const mc of manualCountries) { + if (hidden.has(mc.country_code)) continue; + if (!countries.find((c) => c.code === mc.country_code)) { + countries.push({ code: mc.country_code, placeCount: 0, tripCount: 0, firstVisit: null, lastVisit: null }); + } + } + + // Merge countries reached only by a transport booking. Those store geocoded from/to + // coordinates in reservation_endpoints but create no place row, so they never show up + // via resolvePlaceCountries above and would otherwise be missed (#1366). + // Only 'from'/'to' legs count as actually reached — a 'stop' is an intermediate + // connection/layover (e.g. a plane change) the traveler never really visited. + const endpoints = this.db + .prepare( + ` + SELECT DISTINCT e.lat, e.lng + FROM reservation_endpoints e + JOIN reservations r ON e.reservation_id = r.id + WHERE r.trip_id IN (${tripIds.map(() => '?').join(',')}) AND e.role IN ('from', 'to') + `, + ) + .all(...tripIds) as { lat: number; lng: number }[]; + for (const e of endpoints) { + const code = getCountryFromCoords(e.lat, e.lng); + if (code && !hidden.has(code) && !countries.find((c) => c.code === code)) { + countries.push({ code, placeCount: 0, tripCount: 0, firstVisit: null, lastVisit: null }); + } + } + + const mostVisited = countries.length > 0 ? countries.reduce((a, b) => (a.placeCount > b.placeCount ? a : b)) : null; + + const continents: Record = {}; + countries.forEach((c) => { + const cont = CONTINENT_MAP[c.code] || 'Other'; + continents[cont] = (continents[cont] || 0) + 1; + }); + + const now = new Date().toISOString().split('T')[0]; + const pastTrips = trips + .filter((t) => t.end_date && t.end_date <= now) + .sort((a, b) => b.end_date!.localeCompare(a.end_date!)); + const lastTrip: { + id: number; + title: string; + start_date?: string | null; + end_date?: string | null; + countryCode?: string; + } | null = pastTrips[0] + ? { + id: pastTrips[0].id, + title: pastTrips[0].title, + start_date: pastTrips[0].start_date, + end_date: pastTrips[0].end_date, + } + : null; + if (lastTrip) { + const lastTripPlaces = places.filter((p) => p.trip_id === lastTrip.id); + for (const p of lastTripPlaces) { + const code = resolveCountryCodeSync(p); + if (code) { + lastTrip.countryCode = code; + break; + } + } + } + + const futureTrips = trips + .filter((t) => t.start_date && t.start_date > now) + .sort((a, b) => a.start_date!.localeCompare(b.start_date!)); + const nextTrip: { id: number; title: string; start_date?: string | null; daysUntil?: number } | null = futureTrips[0] + ? { id: futureTrips[0].id, title: futureTrips[0].title, start_date: futureTrips[0].start_date } + : null; + if (nextTrip) { + const diff = Math.ceil((new Date(nextTrip.start_date!).getTime() - new Date().getTime()) / (1000 * 60 * 60 * 24)); + nextTrip.daysUntil = Math.max(0, diff); + } + + const tripYears = new Set(trips.filter((t) => t.start_date).map((t) => parseInt(t.start_date!.split('-')[0]))); + let streak = 0; + const currentYear = new Date().getFullYear(); + for (let y = currentYear; y >= 2000; y--) { + if (tripYears.has(y)) streak++; + else break; + } + const firstYear = tripYears.size > 0 ? Math.min(...tripYears) : null; + + return { + countries, + stats: { + totalTrips: trips.length, + totalPlaces: places.length, + totalCountries: countries.length, + totalDays, + totalCities, + }, + mostVisited, + continents, + lastTrip, + nextTrip, + streak, + firstYear, + tripsThisYear: trips.filter((t) => t.start_date && t.start_date.startsWith(String(currentYear))).length, + }; + } + + // ── Country places ──────────────────────────────────────────────────────── + + countryPlaces(userId: number, code: string) { + const trips = this.getUserTrips(userId); + const tripIds = trips.map((t) => t.id); + if (tripIds.length === 0) { + // Post-fold quirk fix: the legacy early return hardcoded manually_marked + // false, so a trip-less user's manually marked country read as unmarked. + const marked = !!this.db + .prepare('SELECT 1 FROM visited_countries WHERE user_id = ? AND country_code = ?') + .get(userId, code); + return { places: [], trips: [], manually_marked: marked }; + } + + const places = this.getPlacesForTrips(tripIds); + + const matchingPlaces: { + id: number; + name: string; + address: string | null; + lat: number | null; + lng: number | null; + trip_id: number; + }[] = []; + const matchingTripIds = new Set(); + + for (const place of places) { + const pCode = resolveCountryCodeSync(place); + if (pCode === code) { + matchingPlaces.push({ + id: place.id, + name: place.name, + address: place.address ?? null, + lat: place.lat ?? null, + lng: place.lng ?? null, + trip_id: place.trip_id, + }); + matchingTripIds.add(place.trip_id); + } + } + + const matchingTrips = trips + .filter((t) => matchingTripIds.has(t.id)) + .map((t) => ({ id: t.id, title: t.title, start_date: t.start_date, end_date: t.end_date })); + + const isManuallyMarked = !!this.db + .prepare('SELECT 1 FROM visited_countries WHERE user_id = ? AND country_code = ?') + .get(userId, code); + return { places: matchingPlaces, trips: matchingTrips, manually_marked: isManuallyMarked }; + } + + // ── Mark / unmark country ───────────────────────────────────────────────── + + listVisitedCountries(userId: number): { country_code: string; created_at: string }[] { + return this.db + .prepare('SELECT country_code, created_at FROM visited_countries WHERE user_id = ? ORDER BY created_at DESC') + .all(userId) as { country_code: string; created_at: string }[]; + } + + /** Countries the user explicitly removed, which stats() must not re-derive (#1490). */ + getHiddenCountries(userId: number): Set { + const rows = this.db.prepare('SELECT country_code FROM hidden_countries WHERE user_id = ?').all(userId) as { + country_code: string; + }[]; + return new Set(rows.map((r) => r.country_code)); + } + + markCountry(userId: number, code: string): void { + this.db.transaction(() => { + this.db.prepare('INSERT OR IGNORE INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(userId, code); + // Marking it visited again lifts a previous removal. + this.db.prepare('DELETE FROM hidden_countries WHERE user_id = ? AND country_code = ?').run(userId, code); + }); + } + + unmarkCountry(userId: number, code: string): void { + this.db.transaction(() => { + this.db.prepare('DELETE FROM visited_countries WHERE user_id = ? AND country_code = ?').run(userId, code); + this.db.prepare('DELETE FROM visited_regions WHERE user_id = ? AND country_code = ?').run(userId, code); + // A country derived from a place or a transport endpoint has no visited_countries row, + // so the deletes above are no-ops and stats() would re-derive it on the next request. + // Tombstone it so the removal actually sticks (#1490). + this.db.prepare('INSERT OR IGNORE INTO hidden_countries (user_id, country_code) VALUES (?, ?)').run(userId, code); + }); + } + + // ── Mark / unmark region ────────────────────────────────────────────────── + + listManuallyVisitedRegions(userId: number): { region_code: string; region_name: string; country_code: string }[] { + return this.db + .prepare( + 'SELECT region_code, region_name, country_code FROM visited_regions WHERE user_id = ? ORDER BY created_at DESC', + ) + .all(userId) as { region_code: string; region_name: string; country_code: string }[]; + } + + /** Regions the user explicitly removed, which visitedRegions() must not re-derive. */ + getHiddenRegions(userId: number): Set { + const rows = this.db.prepare('SELECT region_code FROM hidden_regions WHERE user_id = ?').all(userId) as { + region_code: string; + }[]; + return new Set(rows.map((r) => r.region_code)); + } + + markRegion(userId: number, code: string, name: string, countryCode: string): void { + this.db.transaction(() => { + this.db + .prepare( + 'INSERT OR IGNORE INTO visited_regions (user_id, region_code, region_name, country_code) VALUES (?, ?, ?, ?)', + ) + .run(userId, code, name, countryCode); + // Re-marking lifts a previous removal of the region itself... + this.db.prepare('DELETE FROM hidden_regions WHERE user_id = ? AND region_code = ?').run(userId, code); + // ...and of the parent country, which the "last region removed" cascade in + // unmarkRegion below may have hidden — otherwise marking a region visited again + // would leave its country invisible. + this.db + .prepare('INSERT OR IGNORE INTO visited_countries (user_id, country_code) VALUES (?, ?)') + .run(userId, countryCode); + this.db.prepare('DELETE FROM hidden_countries WHERE user_id = ? AND country_code = ?').run(userId, countryCode); + }); + } + + // True when the given country still has at least one region that would show as visited — + // derived from place_regions or manually marked — after excluding the given user's hidden + // regions. Used to decide whether removing a region should cascade into hiding the country. + private hasVisibleRegionForCountry(userId: number, countryCode: string, hidden: Set): boolean { + const tripIds = this.getUserTrips(userId).map((t) => t.id); + const placeIds = this.getPlacesForTrips(tripIds) + .filter((p) => p.lat && p.lng) + .map((p) => p.id); + const placeRegionCodes = + placeIds.length > 0 + ? ( + this.db + .prepare( + `SELECT DISTINCT region_code FROM place_regions WHERE country_code = ? AND place_id IN (${placeIds.map(() => '?').join(',')})`, + ) + .all(countryCode, ...placeIds) as { region_code: string }[] + ).map((r) => r.region_code) + : []; + const manualRegionCodes = ( + this.db + .prepare('SELECT region_code FROM visited_regions WHERE user_id = ? AND country_code = ?') + .all(userId, countryCode) as { region_code: string }[] + ).map((r) => r.region_code); + return [...placeRegionCodes, ...manualRegionCodes].some((code) => !hidden.has(code)); + } + + unmarkRegion(userId: number, code: string): void { + // One transaction across the delete, the tombstone and the country cascade — + // better-sqlite3 turns the nested unmarkCountry() transaction into a savepoint. + this.db.transaction(() => { + const region = this.db + .prepare('SELECT country_code FROM visited_regions WHERE user_id = ? AND region_code = ?') + .get(userId, code) as { country_code: string } | undefined; + const countryCode = region?.country_code || countryCodeFromRegionCode(code); + + this.db.prepare('DELETE FROM visited_regions WHERE user_id = ? AND region_code = ?').run(userId, code); + + // Tombstone unconditionally, not just for a manually-marked region — a region derived + // from real place data (the common case) needs to be dismissable too, mirroring + // unmarkCountry's tombstone for a place-derived country (#1490). + if (countryCode) { + this.db + .prepare('INSERT OR IGNORE INTO hidden_regions (user_id, region_code, country_code) VALUES (?, ?, ?)') + .run(userId, code, countryCode); + + // If that was the country's last visible region, hide the country too — otherwise it + // keeps showing "visited" on the world map with nothing left to drill into. + const hidden = this.getHiddenRegions(userId); + if (!this.hasVisibleRegionForCountry(userId, countryCode, hidden)) { + this.unmarkCountry(userId, countryCode); + } + } + }); + } + + // ── Visited regions ─────────────────────────────────────────────────────── + + async visitedRegions( + userId: number, + ): Promise<{ regions: Record }> { + const trips = this.getUserTrips(userId); + const tripIds = trips.map((t) => t.id); + const places = this.getPlacesForTrips(tripIds); + + // Check DB cache first + const placeIds = places.filter((p) => p.lat && p.lng).map((p) => p.id); + const cached = + placeIds.length > 0 + ? (this.db + .prepare(`SELECT * FROM place_regions WHERE place_id IN (${placeIds.map(() => '?').join(',')})`) + .all(...placeIds) as { place_id: number; country_code: string; region_code: string; region_name: string }[]) + : []; + const cachedMap = new Map(cached.map((c) => [c.place_id, c])); + + // Kick off background geocoding for uncached places; return cached data immediately. + const uncached = places.filter((p) => p.lat && p.lng && !cachedMap.has(p.id) && !geocodingInFlight.has(p.id)); + if (uncached.length > 0) { + const insertStmt = this.db.prepare( + 'INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, ?, ?, ?)', + ); + for (const p of uncached) geocodingInFlight.add(p.id); + void (async () => { + try { + for (const place of uncached) { + try { + const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address); + if (info) insertStmt.run(place.id, info.country_code, info.region_code, info.region_name); + } catch { + // individual failure — continue with remaining places + } finally { + geocodingInFlight.delete(place.id); + } + } + } catch { + for (const p of uncached) geocodingInFlight.delete(p.id); + } + })(); + } + + // Group by country → regions with place counts + const regionMap: Record> = {}; + for (const [, entry] of cachedMap) { + if (!regionMap[entry.country_code]) regionMap[entry.country_code] = new Map(); + const existing = regionMap[entry.country_code].get(entry.region_code); + if (existing) { + existing.placeCount++; + } else { + regionMap[entry.country_code].set(entry.region_code, { + code: entry.region_code, + name: entry.region_name, + placeCount: 1, + }); + } + } + + const result: Record = {}; + for (const [country, regions] of Object.entries(regionMap)) { + result[country] = [...regions.values()]; + } + + // Merge manually marked regions + const manualRegions = this.listManuallyVisitedRegions(userId); + for (const r of manualRegions) { + if (!result[r.country_code]) result[r.country_code] = []; + if (!result[r.country_code].find((x) => x.code === r.region_code)) { + result[r.country_code].push({ code: r.region_code, name: r.region_name, placeCount: 0, manuallyMarked: true }); + } + } + + // Suppress regions the user explicitly removed, same as stats() does for countries + // via getHiddenCountries (#1490) — otherwise a region derived fresh from place_regions + // (or a manual mark) on every request could never actually be dismissed. + const hidden = this.getHiddenRegions(userId); + if (hidden.size > 0) { + for (const country of Object.keys(result)) { + result[country] = result[country].filter((r) => !hidden.has(r.code)); + if (result[country].length === 0) delete result[country]; + } + } + + return { regions: result }; + } + + // ── Region GeoJSON / country geometry (delegates to the atlas-geo module) ── + regionGeo(countries: string[]) { return getRegionGeo(countries); } @@ -42,39 +620,71 @@ export class AtlasService { return getCountryGeoGz(); } - countryPlaces(userId: number, code: string) { - return getCountryPlaces(userId, code); - } - - markCountry(userId: number, code: string): void { - markCountryVisited(userId, code); - } - - unmarkCountry(userId: number, code: string): void { - unmarkCountryVisited(userId, code); - } - - markRegion(userId: number, code: string, name: string, countryCode: string): void { - markRegionVisited(userId, code, name, countryCode); - } - - unmarkRegion(userId: number, code: string): void { - unmarkRegionVisited(userId, code); - } + // ── Bucket list CRUD ────────────────────────────────────────────────────── bucketList(userId: number) { - return listBucketList(userId); + return this.db.prepare('SELECT * FROM bucket_list WHERE user_id = ? ORDER BY created_at DESC').all(userId); } createBucketItem(userId: number, data: CreateBucketData) { - return createBucketItem(userId, data); + const result = this.db + .prepare( + 'INSERT INTO bucket_list (user_id, name, lat, lng, country_code, notes, target_date) VALUES (?, ?, ?, ?, ?, ?, ?)', + ) + .run( + userId, + data.name.trim(), + data.lat ?? null, + data.lng ?? null, + data.country_code ?? null, + data.notes ?? null, + data.target_date ?? null, + ); + return this.db.prepare('SELECT * FROM bucket_list WHERE id = ?').get(result.lastInsertRowid); } - updateBucketItem(userId: number, itemId: string, data: UpdateBucketData) { - return updateBucketItem(userId, itemId, data); + updateBucketItem(userId: number, itemId: string | number, data: UpdateBucketData) { + const item = this.db.prepare('SELECT * FROM bucket_list WHERE id = ? AND user_id = ?').get(itemId, userId); + if (!item) return null; + // Post-fold quirk fixes: the value bindings use `?? null` (the legacy + // `|| null` wrote NULL for lat/lng 0 and empty-string notes), and the + // UPDATE + re-select are user-scoped (defense-in-depth; the ownership + // SELECT above stays the primary gate). The whitespace-only name no-op + // (`?.trim() || null` + COALESCE) is deliberate and preserved. + this.db + .prepare( + `UPDATE bucket_list SET + name = COALESCE(?, name), + notes = CASE WHEN ? THEN ? ELSE notes END, + lat = CASE WHEN ? THEN ? ELSE lat END, + lng = CASE WHEN ? THEN ? ELSE lng END, + country_code = CASE WHEN ? THEN ? ELSE country_code END, + target_date = CASE WHEN ? THEN ? ELSE target_date END + WHERE id = ? AND user_id = ?`, + ) + .run( + data.name?.trim() || null, + data.notes !== undefined ? 1 : 0, + data.notes !== undefined ? (data.notes ?? null) : null, + data.lat !== undefined ? 1 : 0, + data.lat !== undefined ? (data.lat ?? null) : null, + data.lng !== undefined ? 1 : 0, + data.lng !== undefined ? (data.lng ?? null) : null, + data.country_code !== undefined ? 1 : 0, + data.country_code !== undefined ? (data.country_code ?? null) : null, + data.target_date !== undefined ? 1 : 0, + data.target_date !== undefined ? (data.target_date ?? null) : null, + itemId, + userId, + ); + return this.db.prepare('SELECT * FROM bucket_list WHERE id = ? AND user_id = ?').get(itemId, userId); } - deleteBucketItem(userId: number, itemId: string): boolean { - return deleteBucketItem(userId, itemId); + deleteBucketItem(userId: number, itemId: string | number): boolean { + const item = this.db.prepare('SELECT * FROM bucket_list WHERE id = ? AND user_id = ?').get(itemId, userId); + if (!item) return false; + // Post-fold quirk fix: user-scoped DELETE (defense-in-depth, see updateBucketItem). + this.db.prepare('DELETE FROM bucket_list WHERE id = ? AND user_id = ?').run(itemId, userId); + return true; } } diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index 03db7f94e..9fca3f599 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -28,9 +28,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'AdminController.updateTemplateCategory', 'AdminController.updateTemplateItem', 'AdminController.updateUser', - 'AtlasController.createBucketItem', - 'AtlasController.markRegion', - 'AtlasController.updateBucketItem', 'AuthController.apiKeys', 'AuthController.changePassword', 'AuthController.createMcpToken', diff --git a/server/src/nest/plugins/DI-MIGRATION.md b/server/src/nest/plugins/DI-MIGRATION.md index 5bbc75c0c..10822d13d 100644 --- a/server/src/nest/plugins/DI-MIGRATION.md +++ b/server/src/nest/plugins/DI-MIGRATION.md @@ -130,8 +130,13 @@ dep; collectionsService swapped in 2026-08 with the collections fold — its seven imported symbols (`listCollections`, `getCollection`, `createCollection`, `updateCollection`, `savePlace`, `copyToTrip`, `deletePlace`) became the injected `CollectionsService`, the factory's 22nd constructor dep, with -`mapCollectionError` unchanged on top; the remaining factory -imports are all Wave-5 domains: journeys, atlas). +`mapCollectionError` unchanged on top; atlasService swapped in 2026-08 with +the atlas fold — its nine imported symbols (`listVisitedCountries`, +`listManuallyVisitedRegions`, `listBucketList`, `markCountryVisited`, +`unmarkCountryVisited`, `markRegionVisited`, `unmarkRegionVisited`, +`createBucketItem`, `deleteBucketItem`) became the injected `AtlasService`, +the factory's 23rd constructor dep; the remaining factory import is the +Wave-5 journeys domain). ### Test impact (as landed) @@ -148,7 +153,9 @@ imports are all Wave-5 domains: journeys, atlas). `budgetStub` gained `listBudgetItems` for the two swapped closures); the collectionsService path mock became `collectionsStub` with the 2026-08 collections fold (same sentinel behaviors — the status-tagged throws the - factory's `mapCollectionError` maps); a + factory's `mapCollectionError` maps); the atlasService path mock became + `atlasStub` with the 2026-08 atlas fold (same sentinel behaviors, keyed by + the service method names — `bucketList`, `markCountry`, …); a file-local shim keeps the historical `createRealRpcHost(id, granted)` call sites and supplies a default no-op router. diff --git a/server/src/nest/plugins/host/plugin-host-deps.factory.ts b/server/src/nest/plugins/host/plugin-host-deps.factory.ts index df2a93b92..0bfd8297a 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -20,7 +20,7 @@ import { AddonsService } from '../../addons/addons.service'; import { isDemoEmail } from '../../../services/demo'; import { ADDON_IDS } from '../../../addons'; import { listJourneys, listEntries as listJournalEntriesSvc, createEntry as createJournalEntrySvc, updateEntry as updateJournalEntrySvc, deleteEntry as deleteJournalEntrySvc, createJourney as createJourneySvc, deleteJourney as deleteJourneySvc, onPlaceCreated, onPlaceUpdated, onPlaceDeleted } from '../../../services/journeyService'; -import { listVisitedCountries, listManuallyVisitedRegions, listBucketList, markCountryVisited, unmarkCountryVisited, markRegionVisited, unmarkRegionVisited, createBucketItem as createBucketItemSvc, deleteBucketItem as deleteBucketItemSvc } from '../../../services/atlasService'; +import { AtlasService } from '../../atlas/atlas.service'; import { CollectionsService } from '../../collections/collections.service'; import { BudgetService } from '../../budget/budget.service'; import { ExchangeRatesService } from '../../budget/exchange-rates.service'; @@ -132,7 +132,7 @@ export interface PluginCallRouter { * * DI-native domain services (budget, exchange-rates, reservations, tags, * categories, todo, packing, day-notes, assignments, oauth, files, collab, - * vacay, trips, places, collections, the LLM config resolver) and + * vacay, trips, places, collections, atlas, the LLM config resolver) and * the DatabaseService (all inline SQL + the trip-access helper) are * constructor-injected; legacy `services/*` domains stay plain function * imports until their own migration lands (DI-MIGRATION.md), at which point @@ -163,6 +163,7 @@ export class PluginHostDepsFactory { private readonly trips: TripsService, private readonly places: PlacesService, private readonly collections: CollectionsService, + private readonly atlas: AtlasService, ) {} /** @@ -660,9 +661,9 @@ export class PluginHostDepsFactory { }, atlasVisitedForUser: (userId) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); - return { countries: listVisitedCountries(userId), regions: listManuallyVisitedRegions(userId) }; + return { countries: this.atlas.listVisitedCountries(userId), regions: this.atlas.listManuallyVisitedRegions(userId) }; }, - atlasBucketForUser: (userId) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); return listBucketList(userId) as unknown[]; }, + atlasBucketForUser: (userId) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); return this.atlas.bucketList(userId) as unknown[]; }, vacayForUser: (userId) => { requireAddon(ADDON_IDS.VACAY, 'vacay'); return this.vacay.getPlanData(userId); }, listCollectionsForUser: (userId) => { requireAddon(ADDON_IDS.COLLECTIONS, 'collections'); return this.collections.listCollections(userId); }, getCollectionForUser: (userId, id) => { requireAddon(ADDON_IDS.COLLECTIONS, 'collections'); return this.collections.getCollection(userId, id); }, @@ -690,18 +691,18 @@ export class PluginHostDepsFactory { return { deleted: true }; }, // --- Atlas write: plain uid-scoped rows, no broadcasts in the service. --- - markCountryVisited: (userId, code) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); markCountryVisited(userId, code); return { visited: true }; }, - unmarkCountryVisited: (userId, code) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); unmarkCountryVisited(userId, code); return { visited: false }; }, + markCountryVisited: (userId, code) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); this.atlas.markCountry(userId, code); return { visited: true }; }, + unmarkCountryVisited: (userId, code) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); this.atlas.unmarkCountry(userId, code); return { visited: false }; }, markRegionVisited: (userId, regionCode, regionName, countryCode) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); - markRegionVisited(userId, regionCode, regionName, countryCode); + this.atlas.markRegion(userId, regionCode, regionName, countryCode); return { visited: true }; }, - unmarkRegionVisited: (userId, regionCode) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); unmarkRegionVisited(userId, regionCode); return { visited: false }; }, - createBucketItem: (userId, input) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); return createBucketItemSvc(userId, input as never); }, + unmarkRegionVisited: (userId, regionCode) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); this.atlas.unmarkRegion(userId, regionCode); return { visited: false }; }, + createBucketItem: (userId, input) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); return this.atlas.createBucketItem(userId, input as never); }, deleteBucketItem: (userId, itemId) => { requireAddon(ADDON_IDS.ATLAS, 'atlas'); - if (!deleteBucketItemSvc(userId, itemId)) throw new ForbiddenResource(`no bucket item ${itemId} for this user`); + if (!this.atlas.deleteBucketItem(userId, itemId)) throw new ForbiddenResource(`no bucket item ${itemId} for this user`); return { deleted: true }; }, // --- Vacay write: the plan is the ACTING USER's active plan (resolved host-side); diff --git a/server/src/nest/plugins/plugins.module.ts b/server/src/nest/plugins/plugins.module.ts index 565ad9327..37cf4bc1a 100644 --- a/server/src/nest/plugins/plugins.module.ts +++ b/server/src/nest/plugins/plugins.module.ts @@ -42,6 +42,7 @@ import { PermissionsModule } from '../permissions/permissions.module'; import { AuditModule } from '../audit/audit.module'; import { AddonsModule } from '../addons/addons.module'; import { CollectionsModule } from '../collections/collections.module'; +import { AtlasModule } from '../atlas/atlas.module'; /** * Plugin system (#plugins). M0 read side + M2 isolated runtime + M3 frontend: @@ -53,7 +54,7 @@ import { CollectionsModule } from '../collections/collections.module'; @Module({ // The DI-native domain services the plugin host wiring injects // (PluginHostDepsFactory); DatabaseModule is @Global, so not listed. - imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule, TripsModule, PermissionsModule, AuditModule, AddonsModule, PlacesModule, CollectionsModule], + imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule, TripsModule, PermissionsModule, AuditModule, AddonsModule, PlacesModule, CollectionsModule, AtlasModule], controllers: [PluginsController, PluginsFeedController, PluginsProxyController, PluginFrameController, PlaceDetailsController, TripWarningsController, ViewContributionsController, TripCardContributionsController, PluginPhotosController, PluginCalendarController, MapMarkersController, MapLayersController, PluginRoutesController, DayScheduleController, PdfSectionsController, AtlasLayersController, JournalEntryRowsController, PluginUserSettingsController, PluginOAuthController, PluginActivityController], providers: [PluginsService, PluginRuntimeService, PluginRegistryService, PluginOAuthService, PluginHostDepsFactory], // Exported so the admin addon-toggle handler can cascade-disable plugins whose diff --git a/server/src/services/authService.ts b/server/src/services/authService.ts index 9289e7306..c9b09b85b 100644 --- a/server/src/services/authService.ts +++ b/server/src/services/authService.ts @@ -22,7 +22,7 @@ import { startTripReminders } from '../scheduler'; import { deleteUserCompletely } from './userCleanupService'; import { emitUserDeleted } from '../plugin-user-lifecycle'; import { getFlightDistanceKm } from './distanceService'; -import { getCountryFromCoords, getHiddenCountries } from './atlasService'; +import { getCountryFromCoords, getHiddenCountries } from '../nest/atlas/atlas.bridge'; import { verifyJwtAndLoadUser } from '../middleware/auth'; import { User } from '../types'; import { DEMO_EMAIL_PRIMARY, isDemoEmail } from './demo'; diff --git a/server/tests/e2e/atlas.e2e.test.ts b/server/tests/e2e/atlas.e2e.test.ts index aac640351..b53589745 100644 --- a/server/tests/e2e/atlas.e2e.test.ts +++ b/server/tests/e2e/atlas.e2e.test.ts @@ -1,71 +1,70 @@ /** - * Atlas module e2e — exercises the migrated /api/addons/atlas endpoints through - * the real JwtAuthGuard against a temp SQLite db. atlasService is mocked; this - * focuses on auth, status codes (mark POSTs stay 200), the cache headers and the - * bespoke 400/404 bodies. + * Atlas e2e — drives /api/addons/atlas through the REAL JwtAuthGuard AND the + * real DI-native AtlasService (DatabaseModule + AtlasModule) against a temp + * SQLite db (full schema). No service mock (the legacy path-mock died with the + * fold): auth, status codes (mark POSTs stay 200, bucket create stays 201), + * cache headers, the bespoke 400/404 bodies, and the SQL effects are all real. + * countries/geo serves the real bundled admin-0 gz. Seeded data stays + * coordinate-free so the background place_regions geocode never fires. */ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; -import zlib from 'zlib'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; import { Test } from '@nestjs/testing'; -import { seedUser, sessionCookie } from './harness'; +import { sessionCookie } from './harness'; const { db } = vi.hoisted(() => { // eslint-disable-next-line @typescript-eslint/no-require-imports const Database = require('better-sqlite3'); const tmp = new Database(':memory:'); tmp.exec('PRAGMA journal_mode = WAL'); - tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, - email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`); + tmp.exec('PRAGMA foreign_keys = ON'); return { db: tmp }; }); -vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); - -const { mocks } = vi.hoisted(() => ({ - mocks: { - getStats: vi.fn(), - getCountryPlaces: vi.fn(), - markCountryVisited: vi.fn(), - unmarkCountryVisited: vi.fn(), - markRegionVisited: vi.fn(), - unmarkRegionVisited: vi.fn(), - getVisitedRegions: vi.fn(), - getRegionGeo: vi.fn(), - getCountryGeoGz: vi.fn(), - listBucketList: vi.fn(), - createBucketItem: vi.fn(), - updateBucketItem: vi.fn(), - deleteBucketItem: vi.fn(), - }, +vi.mock('../../src/db/database', () => ({ + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: () => undefined, + isOwner: () => false, })); -vi.mock('../../src/services/atlasService', () => mocks); +vi.mock('../../src/websocket', () => ({ broadcastToUser: vi.fn(), broadcast: vi.fn() })); + +import { createTables } from '../../src/db/schema'; +import { runMigrations } from '../../src/db/migrations'; +import { createUser } from '../helpers/factories'; import { AtlasModule } from '../../src/nest/atlas/atlas.module'; +import { DatabaseModule } from '../../src/nest/database/database.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; -describe('Atlas e2e (real auth guard + temp SQLite)', () => { +describe('Atlas e2e (real auth guard + real service + temp SQLite)', () => { let server: Server; let app: Awaited>; + let userId: number; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [AtlasModule] }).compile(); + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, AtlasModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); + // Mirror the production APP_PIPE (app.module.ts): DTO-typed bodies validate + // by metatype, exactly as they do under buildApp(). + nest.useGlobalPipes(new ZodValidationPipe()); await nest.init(); return nest; } beforeAll(async () => { - seedUser(db as never, { id: 1 }); + createTables(db as never); + runMigrations(db as never); + userId = createUser(db as never, { username: 'atlas-e2e', email: 'atlas-e2e@test.example' }).user.id; app = await build(); server = app.getHttpServer(); - mocks.getStats.mockResolvedValue({ countries: 3 }); - mocks.markCountryVisited.mockReturnValue(undefined); - mocks.listBucketList.mockReturnValue([{ id: 1, name: 'Tokyo' }]); }); afterAll(async () => { @@ -77,60 +76,79 @@ describe('Atlas e2e (real auth guard + temp SQLite)', () => { expect(res.status).toBe(401); }); - it('200 countries/geo serves gzipped admin-0 that the client decompresses to a FeatureCollection', async () => { - const gz = zlib.gzipSync(JSON.stringify({ type: 'FeatureCollection', features: [{ id: 'NO' }] })); - mocks.getCountryGeoGz.mockReturnValue(gz); - const res = await request(server).get('/api/addons/atlas/countries/geo').set('Cookie', sessionCookie(1)); + it('200 countries/geo serves the bundled gzipped admin-0 that the client decompresses to a FeatureCollection', async () => { + const res = await request(server).get('/api/addons/atlas/countries/geo').set('Cookie', sessionCookie(userId)); expect(res.status).toBe(200); expect(res.headers['content-encoding']).toBe('gzip'); // superagent transparently decompresses, mirroring the browser. expect(res.body.type).toBe('FeatureCollection'); + expect(res.body.features.length).toBeGreaterThan(0); expect(res.headers['cache-control']).toContain('max-age=86400'); }); - it('200 stats for an authenticated user', async () => { - const res = await request(server).get('/api/addons/atlas/stats').set('Cookie', sessionCookie(1)); + it('200 stats for an authenticated user — the trip-less branch keeps its bespoke shape', async () => { + db.prepare('INSERT OR IGNORE INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(userId, 'JP'); + const res = await request(server).get('/api/addons/atlas/stats').set('Cookie', sessionCookie(userId)); expect(res.status).toBe(200); - expect(res.body).toEqual({ countries: 3 }); + // Zero-trip early return: has a `trips` key and only the four base stats — + // no totalCities/mostVisited/continents/… (preserved quirk). + expect(res.body).toEqual({ + countries: [{ code: 'JP', placeCount: 0, tripCount: 0, firstVisit: null, lastVisit: null }], + trips: [], + stats: { totalTrips: 0, totalPlaces: 0, totalCountries: 1, totalDays: 0 }, + }); + db.prepare('DELETE FROM visited_countries WHERE user_id = ?').run(userId); }); - it('200 (not 201) on POST country mark, with upper-cased code', async () => { - const res = await request(server).post('/api/addons/atlas/country/de/mark').set('Cookie', sessionCookie(1)); + it('200 (not 201) on POST country mark, with upper-cased code written to the db', async () => { + const res = await request(server).post('/api/addons/atlas/country/de/mark').set('Cookie', sessionCookie(userId)); expect(res.status).toBe(200); expect(res.body).toEqual({ success: true }); - expect(mocks.markCountryVisited).toHaveBeenCalledWith(1, 'DE'); + const row = db.prepare('SELECT country_code FROM visited_countries WHERE user_id = ?').get(userId); + expect(row).toEqual({ country_code: 'DE' }); + db.prepare('DELETE FROM visited_countries WHERE user_id = ?').run(userId); + db.prepare('DELETE FROM hidden_countries WHERE user_id = ?').run(userId); }); - it('400 on region mark without name/country_code', async () => { - const res = await request(server).post('/api/addons/atlas/region/by/mark').set('Cookie', sessionCookie(1)).send({ name: 'Bavaria' }); + it('400 on region mark without country_code (ZodValidationPipe envelope)', async () => { + const res = await request(server).post('/api/addons/atlas/region/by/mark').set('Cookie', sessionCookie(userId)).send({ name: 'Bavaria' }); expect(res.status).toBe(400); - expect(res.body).toEqual({ error: 'name and country_code are required' }); + // The legacy hand-rolled 'name and country_code are required' body became + // the pipe's `field: message` envelope with the atlas DTO ratchet. + expect(res.body.error).toMatch(/^country_code: /); + expect(db.prepare('SELECT COUNT(*) AS n FROM visited_regions WHERE user_id = ?').get(userId)).toEqual({ n: 0 }); + }); + + it("400 'Name is required' on whitespace-only bucket name (legacy trim guard survives the DTO)", async () => { + const res = await request(server).post('/api/addons/atlas/bucket-list').set('Cookie', sessionCookie(userId)).send({ name: ' ' }); + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: 'Name is required' }); }); it('no-store cache header on /regions', async () => { - mocks.getVisitedRegions.mockResolvedValue({ regions: {} }); - const res = await request(server).get('/api/addons/atlas/regions').set('Cookie', sessionCookie(1)); + const res = await request(server).get('/api/addons/atlas/regions').set('Cookie', sessionCookie(userId)); expect(res.status).toBe(200); + expect(res.body).toEqual({ regions: {} }); expect(res.headers['cache-control']).toBe('no-cache, no-store'); }); it('empty FeatureCollection (no cache header) when /regions/geo has no countries', async () => { - const res = await request(server).get('/api/addons/atlas/regions/geo').set('Cookie', sessionCookie(1)); + const res = await request(server).get('/api/addons/atlas/regions/geo').set('Cookie', sessionCookie(userId)); expect(res.status).toBe(200); expect(res.body).toEqual({ type: 'FeatureCollection', features: [] }); expect(res.headers['cache-control']).toBeUndefined(); }); - it('201 on bucket-list create', async () => { - mocks.createBucketItem.mockReturnValue({ id: 2, name: 'Kyoto' }); - const res = await request(server).post('/api/addons/atlas/bucket-list').set('Cookie', sessionCookie(1)).send({ name: 'Kyoto' }); + it('201 on bucket-list create, row persisted', async () => { + const res = await request(server).post('/api/addons/atlas/bucket-list').set('Cookie', sessionCookie(userId)).send({ name: 'Kyoto' }); expect(res.status).toBe(201); - expect(res.body).toEqual({ item: { id: 2, name: 'Kyoto' } }); + expect(res.body.item.name).toBe('Kyoto'); + const row = db.prepare('SELECT name, user_id FROM bucket_list WHERE id = ?').get(res.body.item.id); + expect(row).toEqual({ name: 'Kyoto', user_id: userId }); }); it('404 on delete of a missing bucket item', async () => { - mocks.deleteBucketItem.mockReturnValue(false); - const res = await request(server).delete('/api/addons/atlas/bucket-list/9').set('Cookie', sessionCookie(1)); + const res = await request(server).delete('/api/addons/atlas/bucket-list/999').set('Cookie', sessionCookie(userId)); expect(res.status).toBe(404); expect(res.body).toEqual({ error: 'Item not found' }); }); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index c1a42f308..6abfab025 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -3,6 +3,8 @@ import { db } from '../../src/db/database'; import { trekMcpAccessPolicy, trekMcpValidateAccess } from '../../src/mcp/nest-mcp-policy'; import { AssignmentsMcp } from '../../src/nest/assignments/assignments.mcp'; import { AssignmentsService } from '../../src/nest/assignments/assignments.service'; +import { AtlasMcp } from '../../src/nest/atlas/atlas.mcp'; +import { AtlasService } from '../../src/nest/atlas/atlas.service'; import { BudgetMcp } from '../../src/nest/budget/budget.mcp'; import { BudgetService } from '../../src/nest/budget/budget.service'; import { ExchangeRatesService } from '../../src/nest/budget/exchange-rates.service'; @@ -95,6 +97,7 @@ export function createMcpTestRegistry(): McpRegistry { new PlacesMcp(placesService, mapsService, dbService), new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService), dbService), new TransitMcp(new TransitService(), daysService, reservationsService, dbService), + new AtlasMcp(new AtlasService(dbService)), ], { accessPolicy: trekMcpAccessPolicy, validateAccess: trekMcpValidateAccess }, ); diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index c4edf0ddb..45f6ff020 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -21,6 +21,7 @@ import { VacayService } from '../../src/nest/vacay/vacay.service'; import { TripsService } from '../../src/nest/trips/trips.service'; import { PlacesService } from '../../src/nest/places/places.service'; import { CollectionsService } from '../../src/nest/collections/collections.service'; +import { AtlasService } from '../../src/nest/atlas/atlas.service'; import { MapsService } from '../../src/nest/maps/maps.service'; import { PermissionsService } from '../../src/nest/permissions/permissions.service'; import { AuditService } from '../../src/nest/audit/audit.service'; @@ -70,6 +71,7 @@ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFacto new TripsService(dbs, todos, packing, files, reservations, days, permissions, budget, collab, vacay, realtime, places), places, new CollectionsService(dbs, permissions, realtime), + new AtlasService(dbs), ); } diff --git a/server/tests/unit/mcp/resources.test.ts b/server/tests/unit/mcp/resources.test.ts deleted file mode 100644 index df0ac29d7..000000000 --- a/server/tests/unit/mcp/resources.test.ts +++ /dev/null @@ -1,174 +0,0 @@ -/** - * Unit tests for MCP resources (resources.ts). - * Tests the remaining legacy-registrar resources via InMemoryTransport + Client - * (trek://trips, trek://trips/{tripId} and .../members moved to the - * DI-discovered TripsMcp — see tools-trips.test.ts; - * trek://trips/{tripId}/budget, .../budget/per-person and - * .../budget/settlement moved to the DI-discovered BudgetMcp — see - * tools-budget.test.ts and tools-budget-advanced.test.ts; - * trek://categories moved to the DI-discovered CategoriesMcp — see - * tools-categories.test.ts; trek://trips/{tripId}/todos moved to TodoMcp — - * see tools-todos.test.ts; trek://trips/{tripId}/packing and .../packing/bags - * moved to PackingMcp — see tools-packing.test.ts; - * trek://trips/{tripId}/days/{dayId}/notes moved to DayNotesMcp — see - * tools-notes.test.ts; trek://trips/{tripId}/collab-notes moved to CollabMcp — - * see tools-notes.test.ts; trek://trips/{tripId}/days and - * .../accommodations moved to DaysMcp — see tools-days.test.ts and - * tools-days-accommodations.test.ts). - */ -import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; - -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); - const mock = { - db, - closeDb: () => {}, - reinitialize: () => {}, - getPlaceWithTags: (placeId: number) => { - const place: any = db.prepare(`SELECT p.*, c.name as category_name, c.color as category_color, c.icon as category_icon FROM places p LEFT JOIN categories c ON p.category_id = c.id WHERE p.id = ?`).get(placeId); - if (!place) return null; - const tags = db.prepare(`SELECT t.* FROM tags t JOIN place_tags pt ON t.id = pt.tag_id WHERE pt.place_id = ?`).all(placeId); - return { ...place, category: place.category_id ? { id: place.category_id, name: place.category_name, color: place.category_color, icon: place.category_icon } : null, tags }; - }, - canAccessTrip: (tripId: any, userId: number) => - db.prepare(`SELECT t.id, t.user_id FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)`).get(userId, tripId, userId), - isOwner: (tripId: any, userId: number) => - !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), - }; - return { testDb: db, dbMock: mock }; -}); - -vi.mock('../../../src/db/database', () => dbMock); -vi.mock('../../../src/config', () => ({ - JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', - ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', - updateJwtSecret: () => {}, -})); -vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); - -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; -import { resetTestDb } from '../../helpers/test-db'; -import { createUser, createTrip, createPlace, addTripMember, createReservation, createCollabNote, createBucketListItem, createVisitedCountry } from '../../helpers/factories'; -import { createMcpHarness, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; - -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - -beforeEach(() => { - resetTestDb(testDb); -}); - -afterAll(() => { - testDb.close(); -}); - -async function withHarness(userId: number, fn: (harness: McpHarness) => Promise) { - const harness = await createMcpHarness({ userId, withTools: false, withResources: true }); - try { - await fn(harness); - } finally { - await harness.cleanup(); - } -} - -// trek://trips and trek://trips/{tripId} moved to tools-trips.test.ts: they -// now register via the nest-mcp registry inside registerTools (TripsMcp -// @Resource/@ResourceTemplate), which this file's withTools: false harness -// never attaches. - -// The trek://trips/{tripId}/days resource moved to the DI-discovered -// DaysMcp — see tools-days.test.ts. - -// The trek://trips/{tripId}/places resource moved to the DI-discovered -// PlacesMcp — its cases live in tools-places.test.ts (the registry attaches -// via registerTools, which this withTools:false harness skips). - -// The trek://trips/{tripId}/budget resource moved to the DI-discovered -// src/nest/budget/budget.mcp.ts — its cases live in tools-budget.test.ts (the -// registry attaches via registerTools, which this withTools:false harness skips). - -// The trek://trips/{tripId}/packing and .../packing/bags resources moved to the -// DI-discovered PackingMcp — see tools-packing.test.ts. - -// The trek://trips/{tripId}/reservations resource moved to the DI-discovered -// ReservationsMcp — see tools-reservations.test.ts. - -// The trek://trips/{tripId}/days/{dayId}/notes resource moved to the -// DI-discovered DayNotesMcp — see tools-notes.test.ts. - -// The trek://trips/{tripId}/accommodations resource moved to the -// DI-discovered DaysMcp — see tools-days-accommodations.test.ts. - -// trek://trips/{tripId}/members moved to tools-trips.test.ts (TripsMcp -// @ResourceTemplate) for the same reason. - -// trek://trips/{tripId}/collab-notes moved to tools-notes.test.ts: it now -// registers via the nest-mcp registry inside registerTools (CollabMcp -// @ResourceTemplate), which this file's withTools: false harness never attaches. - -// trek://categories moved to tools-categories.test.ts: it now registers via -// the nest-mcp registry inside registerTools (CategoriesMcp @Resource), which -// this file's withTools: false harness never attaches. - -describe('Resource: trek://bucket-list', () => { - it('returns only the current user\'s bucket list items', async () => { - const { user } = createUser(testDb); - const { user: other } = createUser(testDb); - createBucketListItem(testDb, user.id, { name: 'Tokyo' }); - createBucketListItem(testDb, other.id, { name: 'Rome' }); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: 'trek://bucket-list' }); - const items = parseResourceResult(result) as any[]; - expect(items).toHaveLength(1); - expect(items[0].name).toBe('Tokyo'); - }); - }); - - it('returns empty array for user with no items', async () => { - const { user } = createUser(testDb); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: 'trek://bucket-list' }); - const items = parseResourceResult(result) as any[]; - expect(items).toEqual([]); - }); - }); -}); - -describe('Resource: trek://visited-countries', () => { - it('returns only the current user\'s visited countries', async () => { - const { user } = createUser(testDb); - const { user: other } = createUser(testDb); - createVisitedCountry(testDb, user.id, 'FR'); - createVisitedCountry(testDb, user.id, 'JP'); - createVisitedCountry(testDb, other.id, 'DE'); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: 'trek://visited-countries' }); - const countries = parseResourceResult(result) as any[]; - expect(countries).toHaveLength(2); - const codes = countries.map((c) => c.country_code); - expect(codes).toContain('FR'); - expect(codes).toContain('JP'); - expect(codes).not.toContain('DE'); - }); - }); - - it('returns empty array for user with no visited countries', async () => { - const { user } = createUser(testDb); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: 'trek://visited-countries' }); - const countries = parseResourceResult(result) as any[]; - expect(countries).toEqual([]); - }); - }); -}); diff --git a/server/tests/unit/mcp/tools-atlas-expanded.test.ts b/server/tests/unit/mcp/tools-atlas-expanded.test.ts index 5b9c3fe5a..5aaf5db63 100644 --- a/server/tests/unit/mcp/tools-atlas-expanded.test.ts +++ b/server/tests/unit/mcp/tools-atlas-expanded.test.ts @@ -43,7 +43,7 @@ vi.mock('../../../src/nest/addons/addons.bridge', () => ({ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; -import { createUser } from '../../helpers/factories'; +import { createUser, createBucketListItem, createVisitedCountry } from '../../helpers/factories'; import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; beforeAll(() => { @@ -66,8 +66,11 @@ async function withHarness(userId: number, fn: (h: McpHarness) => Promise) try { await fn(h); } finally { await h.cleanup(); } } +// The atlas resources register via the nest-mcp registry inside registerTools +// (AtlasMcp @Resource), so the harness must keep tools on for them to attach +// (same shape as tools-vacay.test.ts's withResourceHarness). async function withResourceHarness(userId: number, fn: (h: McpHarness) => Promise) { - const h = await createMcpHarness({ userId, withTools: false, withResources: true }); + const h = await createMcpHarness({ userId, withResources: true }); try { await fn(h); } finally { await h.cleanup(); } } @@ -120,6 +123,28 @@ describe('Tool: list_visited_regions', () => { // --------------------------------------------------------------------------- describe('Tool: mark_region_visited', () => { + it('uppercases both codes like REST does (post-fold quirk fix)', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'mark_region_visited', + arguments: { regionCode: 'jp-13', regionName: 'Tokyo', countryCode: 'jp' }, + }); + const data = parseToolResult(result) as any; + // Legacy stored 'jp-13'/'jp' verbatim, creating rows REST's uppercased + // unmark could never hit; both codes now normalize. + expect(data.region.code).toBe('JP-13'); + expect(data.region.country_code).toBe('JP'); + const unmark = await h.client.callTool({ + name: 'unmark_region_visited', + arguments: { regionCode: 'jp-13' }, + }); + expect(parseToolResult(unmark)).toEqual({ success: true }); + const row = testDb.prepare('SELECT 1 FROM visited_regions WHERE user_id = ?').get(user.id); + expect(row).toBeUndefined(); + }); + }); + it('inserts region and returns region object', async () => { const { user } = createUser(testDb); await withHarness(user.id, async (h) => { @@ -204,6 +229,22 @@ describe('Tool: get_country_atlas_places', () => { expect(Array.isArray(data.places)).toBe(true); }); }); + + it('uppercases the country code like REST does (post-fold quirk fix)', async () => { + const { user } = createUser(testDb); + testDb.prepare('INSERT INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(user.id, 'JP'); + // A trip so countryPlaces reaches the visited_countries lookup pre-quirk-fix too. + testDb.prepare('INSERT INTO trips (user_id, title) VALUES (?, ?)').run(user.id, 'Japan'); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'get_country_atlas_places', + arguments: { countryCode: 'jp' }, + }); + const data = parseToolResult(result) as any; + // Legacy passed 'jp' through verbatim and the lookup matched nothing. + expect(data.manually_marked).toBe(true); + }); + }); }); // --------------------------------------------------------------------------- @@ -314,3 +355,68 @@ describe('Resource: trek://atlas/regions', () => { }); }); }); + +// --------------------------------------------------------------------------- +// Resource: trek://bucket-list (moved from resources.test.ts with the fold — +// that suite's withTools:false harness never attaches registry resources) +// --------------------------------------------------------------------------- + +describe('Resource: trek://bucket-list', () => { + it('returns only the current user\'s bucket list items', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + createBucketListItem(testDb, user.id, { name: 'Tokyo' }); + createBucketListItem(testDb, other.id, { name: 'Rome' }); + + await withResourceHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: 'trek://bucket-list' }); + const items = parseResourceResult(result) as any[]; + expect(items).toHaveLength(1); + expect(items[0].name).toBe('Tokyo'); + }); + }); + + it('returns empty array for user with no items', async () => { + const { user } = createUser(testDb); + + await withResourceHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: 'trek://bucket-list' }); + const items = parseResourceResult(result) as any[]; + expect(items).toEqual([]); + }); + }); +}); + +// --------------------------------------------------------------------------- +// Resource: trek://visited-countries (moved from resources.test.ts, same reason) +// --------------------------------------------------------------------------- + +describe('Resource: trek://visited-countries', () => { + it('returns only the current user\'s visited countries', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + createVisitedCountry(testDb, user.id, 'FR'); + createVisitedCountry(testDb, user.id, 'JP'); + createVisitedCountry(testDb, other.id, 'DE'); + + await withResourceHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: 'trek://visited-countries' }); + const countries = parseResourceResult(result) as any[]; + expect(countries).toHaveLength(2); + const codes = countries.map((c) => c.country_code); + expect(codes).toContain('FR'); + expect(codes).toContain('JP'); + expect(codes).not.toContain('DE'); + }); + }); + + it('returns empty array for user with no visited countries', async () => { + const { user } = createUser(testDb); + + await withResourceHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: 'trek://visited-countries' }); + const countries = parseResourceResult(result) as any[]; + expect(countries).toEqual([]); + }); + }); +}); diff --git a/server/tests/unit/nest/atlas.controller.test.ts b/server/tests/unit/nest/atlas.controller.test.ts index 4ddd4adc0..91450380f 100644 --- a/server/tests/unit/nest/atlas.controller.test.ts +++ b/server/tests/unit/nest/atlas.controller.test.ts @@ -95,15 +95,13 @@ describe('AtlasController (parity with the legacy /api/addons/atlas route)', () }); describe('region', () => { - it('400 when name or country_code is missing', () => { - const markRegion = vi.fn(); - return thrown(() => makeController({ markRegion }).markRegion(user, 'by', undefined, 'DE')).then((r) => - expect(r).toEqual({ status: 400, body: { error: 'name and country_code are required' } })); - }); + // The legacy 'name and country_code are required' 400 is now produced by + // the global ZodValidationPipe (atlas.dto.ts / markRegionRequestSchema) + // before the handler runs — covered by the e2e suite. it('marks a region, upper-casing both codes', () => { const markRegion = vi.fn(); - expect(makeController({ markRegion }).markRegion(user, 'by', 'Bavaria', 'de')).toEqual({ success: true }); + expect(makeController({ markRegion }).markRegion(user, 'by', { name: 'Bavaria', country_code: 'de' })).toEqual({ success: true }); expect(markRegion).toHaveBeenCalledWith(8, 'BY', 'Bavaria', 'DE'); }); }); diff --git a/server/tests/unit/services/atlasService.test.ts b/server/tests/unit/nest/atlas.service.test.ts similarity index 82% rename from server/tests/unit/services/atlasService.test.ts rename to server/tests/unit/nest/atlas.service.test.ts index 7bbe2a1ed..89154628e 100644 --- a/server/tests/unit/services/atlasService.test.ts +++ b/server/tests/unit/nest/atlas.service.test.ts @@ -1,3 +1,12 @@ +/** + * Unit tests for the DI-native AtlasService and the atlas-geo helper module — + * ATLAS-UNIT-001..028 and ATLAS-SVC-001..028 moved 1:1 from the legacy + * tests/unit/services/atlasService.test.ts (case IDs preserved), plus + * ATLAS-SVC-029/030 pinning the atlas.bridge.ts delegation for the remaining + * legacy consumer (authService). Uses a real in-memory SQLite DB so the SQL + * logic is exercised faithfully; the pure-geo functions are imported straight + * from atlas-geo (their caches are module-scoped there on purpose). + */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; // ── DB setup (real in-memory SQLite — same pattern as mcp unit tests) ──────── @@ -31,7 +40,14 @@ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createReservation } from '../../helpers/factories'; -import { getStats, getCached, setCache, getCountryFromCoords, getCountryFromAddress, isPointInCountryBox, reverseGeocodeCountry, getRegionGeo, getCountryGeo, getCountryPlaces, getVisitedRegions, markCountryVisited, unmarkCountryVisited, markRegionVisited, unmarkRegionVisited } from '../../../src/services/atlasService'; +import { getCached, setCache, getCountryFromCoords, getCountryFromAddress, isPointInCountryBox, reverseGeocodeCountry, getRegionGeo, getCountryGeo } from '../../../src/nest/atlas/atlas-geo'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { AtlasService } from '../../../src/nest/atlas/atlas.service'; +import { getCountryFromCoords as bridgeGetCountryFromCoords, getHiddenCountries as bridgeGetHiddenCountries } from '../../../src/nest/atlas/atlas.bridge'; + +// Direct construction over the shared test connection — no TestingModule (repo +// convention for DI-native service unit tests). +const atlas = new AtlasService(new DatabaseService(testDb)); function insertReservationEndpoint( db: any, @@ -82,7 +98,7 @@ describe('getStats', () => { // Place with no address and no coordinates → can't resolve country insertPlace(testDb, trip.id, 'Unknown Place', null); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.mostVisited).toBeNull(); expect(stats.countries).toEqual([]); @@ -100,7 +116,7 @@ describe('getStats', () => { } insertPlace(testDb, trip.id, 'Berlin Place', 'Some Street, Berlin, Germany'); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.mostVisited).not.toBeNull(); expect(stats.mostVisited!.code).toBe('FR'); @@ -114,7 +130,7 @@ describe('getStats', () => { testDb.prepare('INSERT INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(user.id, 'JP'); testDb.prepare('INSERT INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(user.id, 'AU'); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.countries).toHaveLength(2); expect(stats.countries.map((c: { code: string }) => c.code).sort()).toEqual(['AU', 'JP']); @@ -127,7 +143,7 @@ describe('getStats', () => { const trip = createTrip(testDb, user.id, { title: 'Italy Trip' }); insertPlace(testDb, trip.id, 'Colosseum', 'Piazza del Colosseo, Rome, Italy'); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.mostVisited).not.toBeNull(); expect(stats.mostVisited!.code).toBe('IT'); @@ -142,7 +158,7 @@ describe('getStats', () => { insertReservationEndpoint(testDb, reservation.id, 'from', 0, 35.6762, 139.6503); insertReservationEndpoint(testDb, reservation.id, 'to', 1, 51.4700, -0.4543); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); const codes = stats.countries.map((c: { code: string }) => c.code); expect(codes).toContain('JP'); @@ -159,7 +175,7 @@ describe('getStats', () => { insertReservationEndpoint(testDb, reservation.id, 'stop', 1, 35.6762, 139.6503); insertReservationEndpoint(testDb, reservation.id, 'to', 2, -33.8688, 151.2093); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); const codes = stats.countries.map((c: { code: string }) => c.code); expect(codes).toContain('BE'); @@ -176,7 +192,7 @@ describe('getStats', () => { insertReservationEndpoint(testDb, reservation.id, 'from', 0, 50.9014, 4.4844); insertReservationEndpoint(testDb, reservation.id, 'to', 1, 36.6749, -4.4991); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); const codes = stats.countries.map((c: { code: string }) => c.code); expect(codes).toContain('ES'); @@ -351,12 +367,12 @@ describe('unmarkCountryVisited — tombstones', () => { insertReservationEndpoint(testDb, reservation.id, 'from', 0, 50.9014, 4.4844); insertReservationEndpoint(testDb, reservation.id, 'to', 1, 35.6762, 139.6503); - const before = await getStats(user.id); + const before = await atlas.stats(user.id); expect(before.countries.map((c: { code: string }) => c.code)).toContain('JP'); - unmarkCountryVisited(user.id, 'JP'); + atlas.unmarkCountry(user.id, 'JP'); - const after = await getStats(user.id); + const after = await atlas.stats(user.id); expect(after.countries.map((c: { code: string }) => c.code)).not.toContain('JP'); // BE is untouched — removal is scoped to the one country. expect(after.countries.map((c: { code: string }) => c.code)).toContain('BE'); @@ -369,11 +385,11 @@ describe('unmarkCountryVisited — tombstones', () => { insertReservationEndpoint(testDb, reservation.id, 'from', 0, 50.9014, 4.4844); insertReservationEndpoint(testDb, reservation.id, 'to', 1, 35.6762, 139.6503); - unmarkCountryVisited(user.id, 'JP'); - expect((await getStats(user.id)).countries.map((c: { code: string }) => c.code)).not.toContain('JP'); + atlas.unmarkCountry(user.id, 'JP'); + expect((await atlas.stats(user.id)).countries.map((c: { code: string }) => c.code)).not.toContain('JP'); - markCountryVisited(user.id, 'JP'); - expect((await getStats(user.id)).countries.map((c: { code: string }) => c.code)).toContain('JP'); + atlas.markCountry(user.id, 'JP'); + expect((await atlas.stats(user.id)).countries.map((c: { code: string }) => c.code)).toContain('JP'); }); it('ATLAS-SVC-023: #1490 a removed country reappears once it has a real place', async () => { @@ -385,12 +401,12 @@ describe('unmarkCountryVisited — tombstones', () => { insertReservationEndpoint(testDb, reservation.id, 'from', 0, 50.9014, 4.4844); insertReservationEndpoint(testDb, reservation.id, 'to', 1, 35.6762, 139.6503); - unmarkCountryVisited(user.id, 'JP'); - expect((await getStats(user.id)).countries.map((c: { code: string }) => c.code)).not.toContain('JP'); + atlas.unmarkCountry(user.id, 'JP'); + expect((await atlas.stats(user.id)).countries.map((c: { code: string }) => c.code)).not.toContain('JP'); insertPlace(testDb, trip.id, 'Senso-ji', 'Asakusa, Tokyo, Japan'); - const after = await getStats(user.id); + const after = await atlas.stats(user.id); const jp = after.countries.find((c: { code: string }) => c.code === 'JP'); expect(jp).toBeDefined(); expect(jp!.placeCount).toBe(1); @@ -538,7 +554,7 @@ describe('getStats — extended', () => { const { user } = createUser(testDb); createTrip(testDb, user.id, { title: 'Short Trip', start_date: '2024-03-01', end_date: '2024-03-03' }); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); // March 1, 2, 3 → diff = 2 + 1 = 3 expect(stats.stats.totalDays).toBe(3); @@ -548,7 +564,7 @@ describe('getStats — extended', () => { const { user } = createUser(testDb); createTrip(testDb, user.id, { title: 'Dateless' }); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.stats.totalDays).toBe(0); }); @@ -558,7 +574,7 @@ describe('getStats — extended', () => { createTrip(testDb, user.id, { title: 'Japan Trip', start_date: '2024-01-01', end_date: '2024-01-10' }); testDb.prepare('INSERT INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(user.id, 'JP'); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); const codes = stats.countries.map((c: any) => c.code); expect(codes).toContain('JP'); @@ -571,7 +587,7 @@ describe('getStats — extended', () => { const trip = createTrip(testDb, user.id, { title: 'Past France Trip', start_date: '2023-05-01', end_date: '2023-05-10' }); insertPlace(testDb, trip.id, 'Eiffel Tower', 'Champ de Mars, Paris, France'); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.lastTrip).not.toBeNull(); expect(stats.lastTrip!.countryCode).toBe('FR'); @@ -584,7 +600,7 @@ describe('getStats — extended', () => { const futureDateStr = futureDate.toISOString().split('T')[0]; createTrip(testDb, user.id, { title: 'Future Trip', start_date: futureDateStr }); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.nextTrip).not.toBeNull(); expect(stats.nextTrip!.daysUntil).toBeGreaterThan(0); @@ -596,7 +612,7 @@ describe('getStats — extended', () => { createTrip(testDb, user.id, { title: 'This Year', start_date: `${currentYear}-06-01`, end_date: `${currentYear}-06-10` }); createTrip(testDb, user.id, { title: 'Last Year', start_date: `${currentYear - 1}-07-01`, end_date: `${currentYear - 1}-07-10` }); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.streak).toBeGreaterThanOrEqual(1); expect(stats.firstYear).toBe(currentYear - 1); @@ -608,7 +624,7 @@ describe('getStats — extended', () => { createTrip(testDb, user.id, { title: 'This Year', start_date: `${currentYear}-03-01` }); createTrip(testDb, user.id, { title: 'Last Year', start_date: `${currentYear - 1}-03-01` }); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.tripsThisYear).toBe(1); }); @@ -618,7 +634,7 @@ describe('getStats — extended', () => { const nextYear = new Date().getFullYear() + 1; createTrip(testDb, user.id, { title: 'Future', start_date: `${nextYear}-01-01`, end_date: `${nextYear}-01-10` }); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.lastTrip).toBeNull(); }); @@ -633,7 +649,7 @@ describe('getStats — extended', () => { const trip = createTrip(testDb, user.id, { title: 'San Francisco Trip' }); insertPlaceWithCoords(testDb, trip.id, 'Hotel Pickwick', 37.7830549, -122.4066689, '85 5th St, San Francisco, CA'); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); const codes = stats.countries.map((c: any) => c.code); expect(codes).toContain('US'); @@ -647,7 +663,7 @@ describe('getStats — extended', () => { const trip = createTrip(testDb, user.id, { title: 'Past NY Trip', start_date: '2023-05-01', end_date: '2023-05-10' }); insertPlaceWithCoords(testDb, trip.id, 'Imperial Court Hotel', 40.7848394, -73.981643, '307 W 79th Street, New York, NY'); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.lastTrip).not.toBeNull(); expect(stats.lastTrip!.countryCode).toBe('US'); @@ -660,7 +676,7 @@ describe('getCountryPlaces', () => { it('ATLAS-UNIT-013: returns empty result when user has no trips', () => { const { user } = createUser(testDb); - const result = getCountryPlaces(user.id, 'FR'); + const result = atlas.countryPlaces(user.id, 'FR'); expect(result.places).toHaveLength(0); expect(result.trips).toHaveLength(0); @@ -673,7 +689,7 @@ describe('getCountryPlaces', () => { insertPlace(testDb, trip.id, 'Louvre', '75001 Paris, France'); insertPlace(testDb, trip.id, 'Berlin Wall', 'Bernauer Str., Berlin, Germany'); - const result = getCountryPlaces(user.id, 'FR'); + const result = atlas.countryPlaces(user.id, 'FR'); expect(result.places).toHaveLength(1); expect(result.places[0].name).toBe('Louvre'); @@ -686,7 +702,7 @@ describe('getCountryPlaces', () => { testDb.prepare('INSERT INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(user.id, 'JP'); createTrip(testDb, user.id, { title: 'Japan' }); - const result = getCountryPlaces(user.id, 'JP'); + const result = atlas.countryPlaces(user.id, 'JP'); expect(result.manually_marked).toBe(true); }); @@ -697,7 +713,7 @@ describe('getCountryPlaces', () => { // Paris coordinates (48.85°N, 2.35°E) — falls inside FR bounding box insertPlaceWithCoords(testDb, trip.id, 'Secret Paris Spot', 48.85, 2.35); - const result = getCountryPlaces(user.id, 'FR'); + const result = atlas.countryPlaces(user.id, 'FR'); expect(result.places).toHaveLength(1); expect(result.places[0].name).toBe('Secret Paris Spot'); @@ -710,7 +726,7 @@ describe('getVisitedRegions', () => { it('ATLAS-UNIT-017: returns empty regions object when user has no trips', async () => { const { user } = createUser(testDb); - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(result.regions).toEqual({}); }); @@ -720,7 +736,7 @@ describe('getVisitedRegions', () => { testDb.prepare('INSERT INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(user.id, 'DE'); testDb.prepare('INSERT INTO visited_regions (user_id, region_code, region_name, country_code) VALUES (?, ?, ?, ?)').run(user.id, 'DE-BY', 'Bayern', 'DE'); - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(result.regions['DE']).toBeDefined(); const codes = result.regions['DE'].map((r: any) => r.code); @@ -747,11 +763,11 @@ describe('getVisitedRegions', () => { insertPlaceWithCoords(testDb, trip.id, 'Paris Hotel', 48.85, 2.35); // First call triggers the background geocoding fire-and-forget - await getVisitedRegions(user.id); + await atlas.visitedRegions(user.id); // Advance all pending timers (including the 1100ms Nominatim rate-limit delay) await vi.runAllTimersAsync(); // Second call returns now-cached data - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(result.regions['FR']).toBeDefined(); @@ -771,7 +787,7 @@ describe('getVisitedRegions', () => { const mockFetch = vi.fn().mockResolvedValue({ ok: true, json: async () => ({}) }); vi.stubGlobal('fetch', mockFetch); - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(mockFetch).not.toHaveBeenCalled(); expect(result.regions['FR']).toBeDefined(); @@ -791,11 +807,11 @@ describe('getVisitedRegions', () => { const trip = createTrip(testDb, user.id, { title: 'Manchester Trip' }); insertPlaceWithCoords(testDb, trip.id, 'Old Trafford', 53.4631, -2.2913); - await getVisitedRegions(user.id); + await atlas.visitedRegions(user.id); // The background geocode is fire-and-forget; give its microtasks a turn to settle // before reading the now-cached result back. await new Promise(resolve => setTimeout(resolve, 10)); - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(mockFetch).not.toHaveBeenCalled(); expect(result.regions['GB']).toBeDefined(); @@ -815,11 +831,11 @@ describe('getVisitedRegions', () => { const trip = createTrip(testDb, user.id, { title: 'Barcelona Trip' }); insertPlaceWithCoords(testDb, trip.id, 'Sagrada Familia', 41.4036, 2.1744); - await getVisitedRegions(user.id); + await atlas.visitedRegions(user.id); // The background geocode is fire-and-forget; give its microtasks a turn to settle // before reading the now-cached result back. await new Promise(resolve => setTimeout(resolve, 10)); - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(mockFetch).not.toHaveBeenCalled(); expect(result.regions['ES']).toBeDefined(); @@ -842,9 +858,9 @@ describe('getVisitedRegions', () => { '4 Gruusswiss, Bollendorf-Pont, Distrikt Gréiwemaacher 6555, Luxembourg' ); - await getVisitedRegions(user.id); + await atlas.visitedRegions(user.id); await new Promise(resolve => setTimeout(resolve, 10)); - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(mockFetch).not.toHaveBeenCalled(); expect(result.regions['LU']).toBeDefined(); @@ -869,9 +885,9 @@ describe('getVisitedRegions', () => { '85 5th St, San Francisco, CA' ); - await getVisitedRegions(user.id); + await atlas.visitedRegions(user.id); await new Promise(resolve => setTimeout(resolve, 10)); - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(mockFetch).not.toHaveBeenCalled(); expect(result.regions['US']).toBeDefined(); @@ -897,9 +913,9 @@ describe('getVisitedRegions', () => { const trip = createTrip(testDb, user.id, { title: 'Middle of the ocean' }); insertPlaceWithCoords(testDb, trip.id, 'Buoy', 10, -40); - await getVisitedRegions(user.id); + await atlas.visitedRegions(user.id); await vi.runAllTimersAsync(); - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(mockFetch).toHaveBeenCalledTimes(1); expect(result.regions['GB']).toBeDefined(); @@ -934,9 +950,9 @@ describe('getVisitedRegions', () => { // exercising this test's fetch/gate path. insertPlaceWithCoords(testDb, trip.id, 'Weather buoy', 20, -35, '123 Nowhere Rd, JP'); - await getVisitedRegions(user.id); + await atlas.visitedRegions(user.id); await vi.runAllTimersAsync(); - const result = await getVisitedRegions(user.id); + const result = await atlas.visitedRegions(user.id); expect(mockFetch).toHaveBeenCalledTimes(1); // fell through to Nominatim, not a fabricated JP match expect(result.regions['JP']).toBeUndefined(); @@ -948,11 +964,11 @@ describe('getVisitedRegions', () => { // ── unmarkRegionVisited — tombstones + country cascade ────────────────────── // Places are region-resolved by a fire-and-forget background task (see reverseGeocodeRegion -// callers); a single getVisitedRegions() call returns before it settles. Populate the cache +// callers); a single atlas.visitedRegions() call returns before it settles. Populate the cache // deterministically before asserting against it or calling unmarkRegionVisited (which reads // place_regions directly, not through this function). async function primeRegionCache(userId: number): Promise { - await getVisitedRegions(userId); + await atlas.visitedRegions(userId); await new Promise(resolve => setTimeout(resolve, 10)); } @@ -968,12 +984,12 @@ describe('unmarkRegionVisited — tombstones + country cascade', () => { insertPlaceWithCoords(testDb, trip.id, 'Golden Gate Park', 37.7694, -122.4862); await primeRegionCache(user.id); - const before = await getVisitedRegions(user.id); + const before = await atlas.visitedRegions(user.id); expect(before.regions['US']?.map((r: any) => r.code)).toContain('US-CA'); - unmarkRegionVisited(user.id, 'US-CA'); + atlas.unmarkRegion(user.id, 'US-CA'); - const after = await getVisitedRegions(user.id); + const after = await atlas.visitedRegions(user.id); expect(after.regions['US']?.map((r: any) => r.code) ?? []).not.toContain('US-CA'); }); @@ -983,11 +999,11 @@ describe('unmarkRegionVisited — tombstones + country cascade', () => { insertPlaceWithCoords(testDb, trip.id, 'Golden Gate Park', 37.7694, -122.4862); await primeRegionCache(user.id); - unmarkRegionVisited(user.id, 'US-CA'); - expect((await getVisitedRegions(user.id)).regions['US']?.map((r: any) => r.code) ?? []).not.toContain('US-CA'); + atlas.unmarkRegion(user.id, 'US-CA'); + expect((await atlas.visitedRegions(user.id)).regions['US']?.map((r: any) => r.code) ?? []).not.toContain('US-CA'); - markRegionVisited(user.id, 'US-CA', 'California', 'US'); - expect((await getVisitedRegions(user.id)).regions['US']?.map((r: any) => r.code)).toContain('US-CA'); + atlas.markRegion(user.id, 'US-CA', 'California', 'US'); + expect((await atlas.visitedRegions(user.id)).regions['US']?.map((r: any) => r.code)).toContain('US-CA'); }); it('ATLAS-SVC-026: hiding a country\'s only visible region also hides the country', async () => { @@ -998,14 +1014,14 @@ describe('unmarkRegionVisited — tombstones + country cascade', () => { // unmarkCountryVisited's own tombstone tests above use flight-endpoint-derived // countries rather than real places for the same reason. const { user } = createUser(testDb); - markRegionVisited(user.id, 'JP-13', 'Tokyo', 'JP'); // also auto-marks JP visited + atlas.markRegion(user.id, 'JP-13', 'Tokyo', 'JP'); // also auto-marks JP visited - const beforeStats = await getStats(user.id); + const beforeStats = await atlas.stats(user.id); expect(beforeStats.countries.map((c: any) => c.code)).toContain('JP'); - unmarkRegionVisited(user.id, 'JP-13'); + atlas.unmarkRegion(user.id, 'JP-13'); - const afterStats = await getStats(user.id); + const afterStats = await atlas.stats(user.id); expect(afterStats.countries.map((c: any) => c.code)).not.toContain('JP'); }); @@ -1016,25 +1032,128 @@ describe('unmarkRegionVisited — tombstones + country cascade', () => { insertPlaceWithCoords(testDb, trip.id, 'Philly hotel', 39.9527237, -75.1635262); // PA await primeRegionCache(user.id); - unmarkRegionVisited(user.id, 'US-MA'); + atlas.unmarkRegion(user.id, 'US-MA'); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.countries.map((c: any) => c.code)).toContain('US'); - const regions = (await getVisitedRegions(user.id)).regions['US'].map((r: any) => r.code); + const regions = (await atlas.visitedRegions(user.id)).regions['US'].map((r: any) => r.code); expect(regions).not.toContain('US-MA'); expect(regions).toContain('US-PA'); }); it('ATLAS-SVC-028: re-marking a region whose country was cascade-hidden brings the country back too', async () => { const { user } = createUser(testDb); - markRegionVisited(user.id, 'JP-13', 'Tokyo', 'JP'); + atlas.markRegion(user.id, 'JP-13', 'Tokyo', 'JP'); - unmarkRegionVisited(user.id, 'JP-13'); - expect((await getStats(user.id)).countries.map((c: any) => c.code)).not.toContain('JP'); + atlas.unmarkRegion(user.id, 'JP-13'); + expect((await atlas.stats(user.id)).countries.map((c: any) => c.code)).not.toContain('JP'); - markRegionVisited(user.id, 'JP-13', 'Tokyo', 'JP'); + atlas.markRegion(user.id, 'JP-13', 'Tokyo', 'JP'); - const stats = await getStats(user.id); + const stats = await atlas.stats(user.id); expect(stats.countries.map((c: any) => c.code)).toContain('JP'); }); }); + +// ── atlas.bridge.ts delegation (the authService consumer path) ────────────── + +describe('atlas.bridge delegation', () => { + it('ATLAS-SVC-029: bridge getHiddenCountries reflects tombstones written through the service', () => { + const { user } = createUser(testDb); + atlas.unmarkCountry(user.id, 'JP'); + + // The bridge's own AtlasService instance runs over the db Proxy (mocked to + // this suite's testDb), so it must see the same hidden_countries row. + const hidden = bridgeGetHiddenCountries(user.id); + expect(hidden.has('JP')).toBe(true); + expect(hidden.has('FR')).toBe(false); + }); + + it('ATLAS-SVC-030: bridge getCountryFromCoords is the atlas-geo function (shared indexes)', () => { + expect(bridgeGetCountryFromCoords).toBe(getCountryFromCoords); + expect(bridgeGetCountryFromCoords(48.85, 2.35)).toBe('FR'); + }); +}); + +// ── Quirks fixed after the DI fold (trailing fix(server) commit) ───────────── + +describe('atlas quirk fixes', () => { + it('ATLAS-SVC-031: countryPlaces reports manually_marked for a trip-less user', () => { + // The legacy early return hardcoded manually_marked: false, so a user who + // marked a country visited before creating any trip saw it as unmarked. + const { user } = createUser(testDb); + testDb.prepare('INSERT INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(user.id, 'JP'); + + const result = atlas.countryPlaces(user.id, 'JP'); + + expect(result).toEqual({ places: [], trips: [], manually_marked: true }); + expect(atlas.countryPlaces(user.id, 'FR').manually_marked).toBe(false); + }); + + it('ATLAS-SVC-032: updateBucketItem persists lat/lng of exactly 0 (equator/prime meridian)', () => { + const { user } = createUser(testDb); + const item = atlas.createBucketItem(user.id, { name: 'Null Island', lat: 10, lng: 10 }) as { id: number }; + + const updated = atlas.updateBucketItem(user.id, item.id, { lat: 0, lng: 0 }) as { lat: number; lng: number }; + + // The legacy `|| null` binding read 0 as "clear" and wrote NULL. + expect(updated.lat).toBe(0); + expect(updated.lng).toBe(0); + }); + + it('ATLAS-SVC-033: updateBucketItem persists an explicit empty-string notes value', () => { + const { user } = createUser(testDb); + const item = atlas.createBucketItem(user.id, { name: 'Kyoto', notes: 'old notes' }) as { id: number }; + + const updated = atlas.updateBucketItem(user.id, item.id, { notes: '' }) as { notes: string | null }; + + // The legacy `|| null` binding collapsed '' to NULL. + expect(updated.notes).toBe(''); + }); + + it("ATLAS-SVC-034: bucket update/delete cannot touch another user's row (user-scoped SQL)", () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + const item = atlas.createBucketItem(user.id, { name: 'Mine' }) as { id: number; name: string }; + + expect(atlas.updateBucketItem(other.id, item.id, { name: 'Stolen' })).toBeNull(); + expect(atlas.deleteBucketItem(other.id, item.id)).toBe(false); + const row = testDb.prepare('SELECT name FROM bucket_list WHERE id = ?').get(item.id) as { name: string }; + expect(row.name).toBe('Mine'); + }); + + it('ATLAS-SVC-035: unmarkRegion is atomic — a failure mid-flow rolls the region delete back', () => { + const { user } = createUser(testDb); + atlas.markRegion(user.id, 'JP-13', 'Tokyo', 'JP'); + + // Force the tombstone insert (statement 2 of the flow) to fail by hiding + // the hidden_regions table; the visited_regions DELETE must roll back. + testDb.exec('ALTER TABLE hidden_regions RENAME TO hidden_regions_gone'); + try { + expect(() => atlas.unmarkRegion(user.id, 'JP-13')).toThrow(); + } finally { + testDb.exec('ALTER TABLE hidden_regions_gone RENAME TO hidden_regions'); + } + + const row = testDb + .prepare('SELECT 1 FROM visited_regions WHERE user_id = ? AND region_code = ?') + .get(user.id, 'JP-13'); + expect(row).toBeDefined(); + }); + + it('ATLAS-SVC-036: markCountry is atomic — a failure on the tombstone lift rolls the insert back', () => { + const { user } = createUser(testDb); + + testDb.exec('ALTER TABLE hidden_countries RENAME TO hidden_countries_gone'); + try { + expect(() => atlas.markCountry(user.id, 'DE')).toThrow(); + } finally { + testDb.exec('ALTER TABLE hidden_countries_gone RENAME TO hidden_countries'); + } + + const row = testDb + .prepare('SELECT 1 FROM visited_countries WHERE user_id = ? AND country_code = ?') + .get(user.id, 'DE'); + expect(row).toBeUndefined(); + }); +}); diff --git a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts index f65c6098a..17d55280b 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -5,7 +5,7 @@ * host, and trip broadcasts are force-namespaced to plugin:{id}:{event}. * DI-native domains (budget/exchange-rates/reservations/tags/categories/todo/ * packing/day-notes/days/assignments/oauth/llm-config/files/collab/vacay/ - * permissions/trips/places/collections) + * permissions/trips/places/collections/atlas) * are constructor-injected stubs; legacy services/* domains stay path-mocked * until their own DI migration lands. */ @@ -256,17 +256,19 @@ vi.mock('../../../src/services/journeyService', () => ({ updateEntry: vi.fn((entryId: number, _uid: number, data: unknown) => (entryId === 99 ? null : { id: entryId, ...(data as object) })), deleteEntry: vi.fn((entryId: number) => entryId !== 99), })); -vi.mock('../../../src/services/atlasService', () => ({ +// Atlas is a constructor-injected stub since the atlas fold (same behaviors as +// the old services/atlasService path mock, keyed by the service method names). +const atlasStub = { listVisitedCountries: vi.fn(() => [{ country_code: 'JP' }]), listManuallyVisitedRegions: vi.fn(() => [{ region_code: 'JP-13' }]), - listBucketList: vi.fn((uid: number) => [{ id: 5, user_id: uid, name: 'Kyoto' }]), - markCountryVisited: vi.fn(), - unmarkCountryVisited: vi.fn(), - markRegionVisited: vi.fn(), - unmarkRegionVisited: vi.fn(), + bucketList: vi.fn((uid: number) => [{ id: 5, user_id: uid, name: 'Kyoto' }]), + markCountry: vi.fn(), + unmarkCountry: vi.fn(), + markRegion: vi.fn(), + unmarkRegion: vi.fn(), createBucketItem: vi.fn((uid: number, data: { name: string }) => ({ id: 110, user_id: uid, name: data.name })), deleteBucketItem: vi.fn((_uid: number, itemId: number) => Number(itemId) !== 404), -})); +} as unknown as AtlasService; // Collections is a constructor-injected stub since the collections fold (same // behaviors as the old services/collectionsService path mock — the service // throws status-tagged errors the factory maps onto the RPC error classes). @@ -321,6 +323,7 @@ import type { VacayService } from '../../../src/nest/vacay/vacay.service'; import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import type { PlacesService } from '../../../src/nest/places/places.service'; import type { CollectionsService } from '../../../src/nest/collections/collections.service'; +import type { AtlasService } from '../../../src/nest/atlas/atlas.service'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { NotFoundError, ValidationError } from '../../../src/nest/trips/trips.service'; @@ -343,7 +346,7 @@ const tripsStub = { list: () => [{ id: 1 }], removeMember: vi.fn(), } as unknown as import('../../../src/nest/trips/trips.service').TripsService; -const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb), filesStub, collabStub, vacayStub, daysStub, permissionsStub, exchangeRatesStub, addonsStub, new RealtimeService(), tripsStub, placesStub, collectionsStub); +const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb), filesStub, collabStub, vacayStub, daysStub, permissionsStub, exchangeRatesStub, addonsStub, new RealtimeService(), tripsStub, placesStub, collectionsStub, atlasStub); const stubRouter: PluginCallRouter = { callPlugin: async () => undefined, emitPluginEvent: () => {} }; const createRealRpcHost = (id: string, granted: ReadonlySet, router: PluginCallRouter = stubRouter) => factory.create(id, granted, router); diff --git a/server/tests/unit/services/authServiceDb.test.ts b/server/tests/unit/services/authServiceDb.test.ts index 104aca89a..ae5ff5f99 100644 --- a/server/tests/unit/services/authServiceDb.test.ts +++ b/server/tests/unit/services/authServiceDb.test.ts @@ -94,7 +94,8 @@ import { generateToken, getTravelStats, } from '../../../src/services/authService'; -import { unmarkCountryVisited } from '../../../src/services/atlasService'; +import { AtlasService } from '../../../src/nest/atlas/atlas.service'; +import { DatabaseService } from '../../../src/nest/database/database.service'; import { verifyJwtAndLoadUser } from '../../../src/middleware/auth'; // --------------------------------------------------------------------------- @@ -794,7 +795,7 @@ describe('getTravelStats', () => { expect(getTravelStats(user.id).countries).toContain('JP'); - unmarkCountryVisited(user.id, 'JP'); + new AtlasService(new DatabaseService(testDb)).unmarkCountry(user.id, 'JP'); const after = getTravelStats(user.id); expect(after.countries).not.toContain('JP');