diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index 5736587bd..49957268d 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -4588,6 +4588,33 @@ function runMigrations(db: Database.Database): void { const cols = db.prepare("SELECT name FROM pragma_table_info('budget_settlements')").all() as Array<{ name: string }>; if (!cols.some(c => c.name === 'settled_at')) { db.exec('ALTER TABLE budget_settlements ADD COLUMN settled_at TEXT'); + /** + * Amap (高德) as a keyed places provider. + * + * users.amap_api_key has the same shape and role as maps_api_key: encrypted + * with apiKeyCrypto, and the last step of the resolver after the operator + * env var and the instance-wide app_settings row. Nullable with no default, + * because "this install does not use Amap" is the correct state for almost + * everybody. Nothing is backfilled: a Google key is not an Amap key, and + * the two are chosen by the places_provider setting, not by which column + * happens to be populated. + * + * places.amap_poi_id is the provider id a place was found by, beside + * google_place_id and osm_id. Amap ids are bare strings shaped like Google + * ones, so the column holds them with the `amap:` prefix the maps domain + * uses everywhere, and a place keeps opening against Amap after the admin + * switches provider. + * + * Appended LAST: the array is index-addressed against schema_version. + */ + () => { + const userCols = db.prepare("SELECT name FROM pragma_table_info('users')").all() as Array<{ name: string }>; + if (!userCols.some(c => c.name === 'amap_api_key')) { + db.exec('ALTER TABLE users ADD COLUMN amap_api_key TEXT'); + } + const placeCols = db.prepare("SELECT name FROM pragma_table_info('places')").all() as Array<{ name: string }>; + if (!placeCols.some(c => c.name === 'amap_poi_id')) { + db.exec('ALTER TABLE places ADD COLUMN amap_poi_id TEXT'); } }, ]; diff --git a/server/src/nest/maps/maps.mcp.ts b/server/src/nest/maps/maps.mcp.ts index cc0abc281..156e1d441 100644 --- a/server/src/nest/maps/maps.mcp.ts +++ b/server/src/nest/maps/maps.mcp.ts @@ -70,9 +70,9 @@ export class MapsMcp { @Tool({ name: 'resolve_maps_url', - description: 'Resolve a Google Maps share URL to coordinates and place name.', + description: 'Resolve a Google Maps or Amap (高德地图) share URL to coordinates and place name.', inputSchema: { - url: z.string().describe('Google Maps share URL'), + url: z.string().describe('Google Maps or Amap share URL'), }, annotations: TOOL_ANNOTATIONS_READONLY, access: { group: 'geo', mode: 'read' }, diff --git a/server/src/nest/maps/maps.service.ts b/server/src/nest/maps/maps.service.ts index 35ee45e42..cc5bd15c3 100644 --- a/server/src/nest/maps/maps.service.ts +++ b/server/src/nest/maps/maps.service.ts @@ -12,6 +12,14 @@ import { readEnv, getAppUrl } from '../../app-config'; import { safeFetchFollow, SsrfBlockedError } from '../../utils/ssrfGuard'; import { discardBody, exceedsDeclaredLength, readCappedText } from '../../utils/cappedFetch'; import { resolveApiKey, type ApiKeySource } from '../settings/instance-api-keys'; +import { isPlacesProviderChoice, type PlacesProviderChoice } from './providers/places-provider'; +import { + AMAP_SHORT_HOSTS, + AmapPlacesProvider, + isAmapHost, + isAmapPlaceId, + parseAmapUrl, +} from './providers/amap.provider'; // ── Photo cache (disk-backed) ──────────────────────────────────────────────── import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; import { DatabaseService } from '../database/database.service'; @@ -633,6 +641,24 @@ async function overpassFetch(query: string): Promise { type LocationBias = { low: { lat: number; lng: number }; high: { lat: number; lng: number } }; +/** + * The app_settings row that names the keyed places provider. + * + * A setting rather than "whichever key is configured": an install can hold both + * credentials (a team split between China and elsewhere), and then only an + * admin can say which one should answer. Absent, which is every install that + * predates Amap, means `auto`, which keeps Google. + */ +export const PLACES_PROVIDER_SETTING = 'places_provider'; + +/** + * Whoever holds the keyed slot beside the index for one request: Google's + * credential, an Amap provider, or nobody (the OpenStreetMap stack alone). + */ +type KeyedProvider = + | { id: 'google'; key: string; source: ApiKeySource | null } + | { id: 'amap'; provider: AmapPlacesProvider }; + /** * /api/maps domain service — geocoding, the provider fan-out * (Nominatim/Overpass/Google), the place-details/photo caches and the SSRF @@ -946,6 +972,88 @@ export class MapsService { return this.resolveMapsKey(userId).key; } + /** The Amap credential, resolved through the identical three-step chain. */ + resolveAmapKey(userId: number): { key: string | null; source: ApiKeySource | null } { + return resolveApiKey(this.database, 'amap_api_key', userId, readEnv().maps.amapApiKey); + } + + // ── Keyed provider selection ─────────────────────────────────────────────── + + /** + * Which keyed provider the admin picked, or `auto`. + * + * An unrecognised stored value degrades to `auto` rather than throwing: this + * is read on the hot path of every search, and a hand-edited settings row must + * not take place search down. + */ + placesProviderChoice(): PlacesProviderChoice { + const row = this.database.get<{ value: string }>( + 'SELECT value FROM app_settings WHERE key = ?', + PLACES_PROVIDER_SETTING, + ); + return isPlacesProviderChoice(row?.value) ? row.value : 'auto'; + } + + /** + * Who holds the keyed slot for this request, or null for the OpenStreetMap + * stack alone. The index and OpenStreetMap are asked either way; this only + * decides what answers once they have nothing. + * + * `auto`, the default and what every install that predates Amap has, prefers + * Google. That is deliberately the incumbent rather than "the newest provider + * wins": an existing install must not silently start querying somewhere + * else, with a different bill and different results, because a release added + * a provider. An admin who wants Amap says so. + * + * Key resolution is ordered to match: under `auto` the Amap chain is only + * walked when there is no Google key, so an install on Google issues exactly + * the database reads it always did. + */ + keyedProvider(userId: number): KeyedProvider | null { + const choice = this.placesProviderChoice(); + if (choice === 'openstreetmap') return null; + + if (choice !== 'amap') { + const google = this.resolveMapsKey(userId); + if (google.key) return { id: 'google', key: google.key, source: google.source }; + // An explicit 'google' choice with no key is not a reason to query Amap + // instead: this install is on Google and is misconfigured. OSM answers, + // the way a keyless install has always been answered. + if (choice === 'google') return null; + } + + const amap = this.resolveAmapKey(userId); + return amap.key + ? { id: 'amap', provider: new AmapPlacesProvider({ key: amap.key, source: amap.source, userId }) } + : null; + } + + /** The Amap provider, when Amap holds the keyed slot; null otherwise. */ + resolvePlacesProvider(userId: number): AmapPlacesProvider | null { + const keyed = this.keyedProvider(userId); + return keyed?.id === 'amap' ? keyed.provider : null; + } + + /** + * The Amap provider for an `amap:` id, regardless of which provider is + * currently selected. + * + * Places outlive the setting. An install that ran on Amap for a year and then + * switches to Google still holds its `amap:` places, and every one of those + * keeps opening against the Amap key that is still configured. Google ids do + * not come through here at all: they take the inline Google path, which + * resolves its own key the same way. + * + * Null means nobody can resolve it: a Google id, or an Amap place on an + * install that has since dropped its Amap key. Callers treat that as a miss, + * not an error. + */ + private providerForPlaceId(userId: number, placeId: string): AmapPlacesProvider | null { + if (!isAmapPlaceId(placeId)) return null; + const amap = this.resolveAmapKey(userId); + return amap.key ? new AmapPlacesProvider({ key: amap.key, source: amap.source, userId }) : null; + } + /** * A coordinate for a name that came out of an import, from whichever source * has it. @@ -1874,7 +1982,8 @@ export class MapsService { locationBias?: { lat: number; lng: number; radius?: number }, opts: { googleIdentityOnly?: boolean } = {}, ): Promise<{ places: Record[]; source: string }> { - const { key: apiKey, source: keySource } = this.resolveMapsKey(userId); + const keyed = this.keyedProvider(userId); + const { key: apiKey, source: keySource } = keyed?.id === 'google' ? keyed : { key: null, source: null }; // The TREK index answers first, whether or not a Google key exists. It is // the only source here that may be stored, works offline as a country @@ -1943,6 +2052,13 @@ export class MapsService { } } + // Amap in the slot Google otherwise holds: asked only once the index and + // OpenStreetMap came back empty, exactly like the Google call below. + if (keyed?.id === 'amap') { + const places = await keyed.provider.searchText(query, lang, locationBias); + return { places, source: 'amap' }; + } + if (!apiKey) { // Reuse what OpenStreetMap already said rather than asking again. The // first call carried a viewbox with bounded=0, which orders results @@ -2022,7 +2138,8 @@ export class MapsService { locationBias?: { low: { lat: number; lng: number }; high: { lat: number; lng: number } }, sessionToken?: string, ): Promise { - const { key: apiKey, source: keySource } = this.resolveMapsKey(userId); + const keyed = this.keyedProvider(userId); + const { key: apiKey, source: keySource } = keyed?.id === 'google' ? keyed : { key: null, source: null }; // This is the path that mattered most. Nominatim's usage policy names // autocomplete as unacceptable use in its own words, regardless of rate, @@ -2100,6 +2217,11 @@ export class MapsService { } } + if (keyed?.id === 'amap') { + const suggestions = await keyed.provider.autocomplete(input, lang, locationBias); + return { suggestions, source: 'amap' }; + } + if (!apiKey) { return this.autocompleteNominatim(input, lang); } @@ -2252,6 +2374,11 @@ export class MapsService { }; } + // An Amap id is `amap:` and so carries a colon too. Before the OSM + // branch, which would otherwise send "amap" to Overpass as an element type + // and answer every Chinese place with an empty record. + if (isAmapPlaceId(placeId)) return this.amapDetails(userId, placeId, lang); + // OSM details: placeId is "node:123456" or "way:123456" etc. if (placeId.includes(':')) { const [osmType, osmId] = placeId.split(':'); @@ -2374,6 +2501,50 @@ export class MapsService { return { place }; } + /** + * The Amap half of getPlaceDetails, behind the same cache the Google half + * uses. Keyed by place_id, and an Amap id carries its `amap:` prefix, so the + * two providers' rows cannot collide. + * + * No key for the id is an empty result, not a client error, for the same + * reason the Google half answers its keyless case that way: an Amap place + * opened on an install that has since dropped its Amap key is a miss. + */ + private async amapDetails( + userId: number, + placeId: string, + lang?: string, + ): Promise<{ place: Record | null }> { + const provider = this.providerForPlaceId(userId, placeId); + if (!provider) return { place: null }; + + const langKey = toApiLang(lang); + const DETAILS_TTL = 7 * 24 * 60 * 60 * 1000; + const cached = this.database.get<{ payload_json: string; fetched_at: number }>( + 'SELECT payload_json, fetched_at FROM place_details_cache WHERE place_id = ? AND lang = ? AND expanded = 0', + placeId, + langKey, + ); + if (cached && Date.now() - cached.fetched_at < DETAILS_TTL) return { place: JSON.parse(cached.payload_json) }; + + const place = await provider.placeDetails(placeId, lang); + if (!place) return { place: null }; + + try { + this.database.run( + 'INSERT OR REPLACE INTO place_details_cache (place_id, lang, expanded, payload_json, fetched_at) VALUES (?, ?, 0, ?, ?)', + placeId, + langKey, + JSON.stringify(place), + Date.now(), + ); + } catch (dbErr) { + console.error('Failed to cache place details:', dbErr); + } + + return { place }; + } + async getPlaceDetailsExpanded( userId: number, placeId: string, @@ -2390,9 +2561,10 @@ export class MapsService { // whole record for them — name, address, contact, hours — and only the reviews // and the editorial summary are Google's to add. Answering `expand=1` with a // null while `expand=0` answers in full would make the richer request the - // poorer one. + // poorer one. An Amap id has no richer tier either, so it takes the plain + // lookup as well. if (!isGooglePlaceId(placeId)) { - return OSM_PLACE_ID.test(placeId) || placeId.startsWith('gers:') + return OSM_PLACE_ID.test(placeId) || placeId.startsWith('gers:') || isAmapPlaceId(placeId) ? this.getPlaceDetails(userId, placeId, lang) : { place: null }; } @@ -2647,6 +2819,26 @@ export class MapsService { lang?: string, opts?: { lane?: GeoLane; timeoutMs?: number; locality?: boolean }, ): Promise<{ name: string | null; address: string | null }> { + // Amap answers first when it holds the keyed slot. Resolved at userId 0, + // because most callers here have no person behind them (a booking import, + // an Atlas tile, a right-click on a shared map): the chain stops at the + // operator env var and the instance-wide row, and nobody's personal key is + // read on somebody else's behalf (#1939). Nominatim stays the fallback, so + // an Amap outage does not take a right-click down with it. + const amap = this.resolvePlacesProvider(0); + if (amap) { + const latNum = Number.parseFloat(lat); + const lngNum = Number.parseFloat(lng); + if (Number.isFinite(latNum) && Number.isFinite(lngNum)) { + try { + const answer = await amap.reverse(latNum, lngNum, lang); + if (answer) return answer; + } catch (err) { + console.error('[Maps] amap reverse geocode failed, falling back to Nominatim:', (err as Error).message); + } + } + } + const params = new URLSearchParams({ lat, lon: lng, @@ -2705,20 +2897,37 @@ export class MapsService { // usually carries the !3d!4d data param we can then parse. Redirects are // followed manually so every hop is SSRF-re-checked. const parsed = new URL(url); - const isShort = GOOGLE_SHORT_HOSTS.includes(parsed.hostname); + const isShort = GOOGLE_SHORT_HOSTS.includes(parsed.hostname) || AMAP_SHORT_HOSTS.includes(parsed.hostname); const isGoogleMaps = isGoogleMapsHost(parsed.hostname); if (isShort || (isGoogleMaps && !extractCoords(url))) { resolvedUrl = (await followRedirects(url)).url || resolvedUrl; } + let resolvedHost = ''; + try { resolvedHost = new URL(resolvedUrl).hostname; } catch { /* keep the empty host, both host branches are skipped */ } + + // Amap links first, and on their own: they spell the coordinate `lng,lat` + // in GCJ-02, which the Google patterns below would read as a WGS-84 + // `lat,lng` and put a Shanghai restaurant in the East China Sea. + // parseAmapUrl owns both the ordering and the datum conversion. + if (isAmapHost(resolvedHost)) { + const amap = parseAmapUrl(resolvedUrl); + // A POI page without a coordinate would need a keyed detail lookup, and + // this method has no user to resolve a key for: the same answer a Google + // page without coordinates gets. + if (!amap || !Number.isFinite(amap.lat) || !Number.isFinite(amap.lng)) { + throw Object.assign(new Error('Could not extract coordinates from URL'), { status: 400 }); + } + const reverse = await this.reverseGeocode(String(amap.lat), String(amap.lng), undefined, { timeoutMs: 8000 }); + return { lat: amap.lat, lng: amap.lng, name: amap.name || reverse.name, address: reverse.address, google_ftid: null }; + } + let coords = extractCoords(resolvedUrl); // Still nothing (e.g. a cid page whose final URL lacks coordinates): fetch the // page body once and parse the coordinates out of the embedded map data. // Only Google's own pages get read; the resolved host is what counts, so a // short link that lands on maps.google.com still qualifies. - let resolvedHost = ''; - try { resolvedHost = new URL(resolvedUrl).hostname; } catch { /* keep the empty host, the branch is skipped */ } if (!coords && isGoogleMapsHost(resolvedHost)) { try { const pageRes = await followRedirects(resolvedUrl, { diff --git a/server/src/nest/maps/providers/amap.provider.ts b/server/src/nest/maps/providers/amap.provider.ts index 43071f5ac..633518ff3 100644 --- a/server/src/nest/maps/providers/amap.provider.ts +++ b/server/src/nest/maps/providers/amap.provider.ts @@ -34,7 +34,6 @@ import { readEnv } from '../../../app-config'; import { safeFetchFollow } from '../../../utils/ssrfGuard'; import { UA, parseOpeningHours } from '../maps.helpers'; import type { - DetailsOptions, PlacesProvider, ProviderCredential, ProviderPlace, @@ -163,10 +162,6 @@ export class AmapPlacesProvider implements PlacesProvider { constructor(private readonly credential: ProviderCredential) {} - ownsPlaceId(placeId: string): boolean { - return isAmapPlaceId(placeId); - } - // ── Outbound plumbing ────────────────────────────────────────────────────── /** @@ -188,10 +183,10 @@ export class AmapPlacesProvider implements PlacesProvider { * Amap's optional 数字签名 (digital signature). * * A key can be created with a private secret, and such a key rejects every - * unsigned request. The scheme is an MD5 of the request parameters sorted by - * name, with the secret appended — MD5 because Amap specifies MD5, not because - * anything here is choosing a hash. The key itself is deliberately NOT part of - * the signed set on the query-string form of the API. + * unsigned request. The scheme is an MD5 over every query parameter sorted by + * name, the key and the output format included, with the secret appended. + * MD5 because Amap specifies MD5, not because anything here is choosing a + * hash. * * Unset, which is the common case, nothing is added. */ @@ -257,7 +252,7 @@ export class AmapPlacesProvider implements PlacesProvider { * tell two identically named branches apart in the place picker. Province, * city and district come as separate fields for exactly this purpose. */ - private toPlace(poi: AmapPoi): ProviderPlace | null { + private toPlace(poi: AmapPoi): ProviderPlace { const coords = fromAmapLocation(poi.location); const business = poi.business ?? {}; const bizExt = poi.biz_ext ?? {}; @@ -325,19 +320,12 @@ export class AmapPlacesProvider implements PlacesProvider { ) : await this.call('/v3/place/text', common, 'place/text'); - return (data.pois ?? []) - .map((poi) => this.toPlace(poi)) - .filter((p): p is ProviderPlace => p !== null); + return (data.pois ?? []).map((poi) => this.toPlace(poi)); } - async autocomplete( - input: string, - lang?: string, - bias?: ViewportBias, - // Amap does not bill per keystroke and has no session concept, so the token - // the Google path uses to group a search has nothing to attach to here. - _sessionToken?: string, - ): Promise { + // Amap does not bill per keystroke and has no session concept, so the token + // the Google path uses to group a search has nothing to attach to here. + async autocomplete(input: string, lang?: string, bias?: ViewportBias): Promise { const params: Record = { keywords: input, // Without this, inputtips also returns bus stops and road names, which @@ -371,7 +359,7 @@ export class AmapPlacesProvider implements PlacesProvider { })); } - async placeDetails(placeId: string, opts: DetailsOptions = {}): Promise { + async placeDetails(placeId: string, lang?: string): Promise { const poiId = amapPoiId(placeId); if (!poiId) return null; @@ -384,15 +372,13 @@ export class AmapPlacesProvider implements PlacesProvider { { id: poiId, show_fields: 'business,indoor', - language: toAmapLang(opts.lang), + language: toAmapLang(lang), }, `place/detail(${poiId})`, ); const poi = data.pois?.[0]; - if (!poi) return null; - const place = this.toPlace(poi); - return place ? { ...place, cached_at: Date.now() } : null; + return poi ? { ...this.toPlace(poi), cached_at: Date.now() } : null; } /** @@ -419,7 +405,8 @@ export class AmapPlacesProvider implements PlacesProvider { { location: toAmapLocation(lat, lng), extensions: 'all', - // 18 is street-address detail, matching the zoom the Nominatim path asks for. + // Metres around the point that count as "here", roughly the block the + // Nominatim path's zoom 18 resolves to. radius: '200', language: toAmapLang(lang), }, diff --git a/server/src/nest/maps/providers/places-provider.ts b/server/src/nest/maps/providers/places-provider.ts index 9c59411e0..bd82ee053 100644 --- a/server/src/nest/maps/providers/places-provider.ts +++ b/server/src/nest/maps/providers/places-provider.ts @@ -1,34 +1,29 @@ /** - * The places-provider seam. + * The keyed places-provider seam. * - * Before this file, MapsService reached Google inline and every method carried - * its own `if (!apiKey) → OpenStreetMap` branch. That worked for two sources and - * stops working at three: adding Amap that way means a three-way conditional in - * `searchPlaces`, another in `autocompletePlaces`, two more in the details pair, - * and a fourth spelling of "which credential did we end up using" in each. - * - * So the *keyed* providers — the ones an admin configures and that bill somebody - * — are implemented behind this interface, and MapsService keeps what is - * genuinely its own: the caches, the kill switches, the photo orchestration and - * the Wikimedia enrichment. + * Search, autocomplete and details ask the TREK index and OpenStreetMap first; + * neither needs a credential, and together they answer on every install. + * Behind them sits one keyed provider, the one an admin configures and that + * bills somebody: Google, which MapsService still calls inline, or Amap, + * which comes through this interface. The `places_provider` row in + * app_settings says which of the two holds that slot, and `auto` keeps + * whatever the install already used. * * OpenStreetMap deliberately does NOT implement this interface. It is the * keyless floor every install falls back to, it has no credential to resolve, * and its details path is a Nominatim/Overpass merge that answers a different * question from a single provider lookup. Dressing it up as a provider would - * mean either a fake key or an interface full of optionals. `null` from - * `resolvePlacesProvider` means "use the OSM stack", which is also exactly what - * the code did before. + * mean either a fake key or an interface full of optionals. * - * Every method returns the same normalised place shape regardless of provider — - * `name/address/lat/lng` in WGS-84, plus whatever provider-specific ids the - * client already persists (`google_place_id`, `amap_poi_id`, …). Coordinates are - * WGS-84 by contract: a provider that speaks another datum converts at its own - * boundary (see amap.provider.ts) so nothing downstream has to care. + * Every method returns the same normalised place shape regardless of provider: + * `name/address/lat/lng` in WGS-84, plus the provider's own id under the column + * the client persists it in (`amap_poi_id`). Coordinates are WGS-84 by + * contract: a provider that speaks another datum converts at its own boundary + * (see amap.provider.ts) so nothing downstream has to care. */ import type { ApiKeySource } from '../../settings/instance-api-keys'; -/** The keyed providers. OpenStreetMap is absent on purpose — see the file header. */ +/** The keyed providers. OpenStreetMap is absent on purpose, see the file header. */ export type PlacesProviderId = 'google' | 'amap'; /** @@ -50,7 +45,7 @@ export function isPlacesProviderChoice(value: unknown): value is PlacesProviderC return typeof value === 'string' && (PLACES_PROVIDER_CHOICES as readonly string[]).includes(value); } -/** A place as the rest of TREK consumes it. Open by design — see maps.schema.ts. */ +/** A place as the rest of TREK consumes it. Open by design, see maps.schema.ts. */ export type ProviderPlace = Record; export interface ProviderSuggestion { @@ -72,30 +67,10 @@ export interface ViewportBias { high: { lat: number; lng: number }; } -export interface DetailsOptions { - lang?: string; - /** - * Ask for the fields only the richer (and pricier) lookup returns — reviews - * and the editorial summary at Google. A provider without a two-tier lookup - * ignores it and answers the same either way. - */ - expanded?: boolean; - /** Closes a billed autocomplete session, where the provider has the concept. */ - sessionToken?: string; -} - -/** One photo the provider holds, as a reference to fetch bytes for later. */ -export interface ProviderPhotoRef { - /** Provider-opaque handle passed straight back to `photoBytes`. */ - name: string; - /** The author as the provider names them — not the provider itself. */ - attribution: string | null; -} - /** * The credential and its provenance, carried so an error can say WHICH of the - * three configured places a key can come from was used without ever logging the - * key. This is the #1939 breadcrumb, kept when the code moved out of the service. + * three places a key can come from was used, without ever logging the key. + * This is the #1939 breadcrumb, kept when the code moved out of the service. */ export interface ProviderCredential { key: string; @@ -107,42 +82,13 @@ export interface ProviderCredential { export interface PlacesProvider { readonly id: PlacesProviderId; - /** - * True when this provider is the one that can resolve `placeId`. - * - * Sending an id to the wrong provider is not a harmless miss: Google answers - * a foreign id with a *billable* 400 INVALID_ARGUMENT, which is the reason - * `isGooglePlaceId` exists at all. Every details/photo path checks this first. - */ - ownsPlaceId(placeId: string): boolean; - searchText(query: string, lang?: string, bias?: SearchBias): Promise; - autocomplete( - input: string, - lang?: string, - bias?: ViewportBias, - sessionToken?: string, - ): Promise; + autocomplete(input: string, lang?: string, bias?: ViewportBias): Promise; - /** Null when the provider has no such place — a miss, not an error. */ - placeDetails(placeId: string, opts?: DetailsOptions): Promise; + /** Null when the provider has no such place: a miss, not an error. */ + placeDetails(placeId: string, lang?: string): Promise; - /** - * Reverse geocoding, where the provider offers it. Absent means "use - * Nominatim", which is what every install did before and still does for the - * OSM stack. Null means the provider had nothing for this point. - */ - reverse?(lat: number, lng: number, lang?: string): Promise<{ name: string | null; address: string | null } | null>; - - /** - * Photo candidates for the enrichment picker. Optional because a provider - * whose image licensing we cannot state is better off not offering any — the - * Wikimedia fallback then supplies the picture, with its licence attached. - */ - photoRefs?(placeId: string, cap: number): Promise; - photoBytes?(photoName: string, maxHeightPx?: number): Promise; - - /** A one-paragraph description of the place, where the provider has one. */ - editorialSummary?(placeId: string, lang?: string): Promise; + /** Reverse geocoding. Null means the provider had nothing for this point. */ + reverse(lat: number, lng: number, lang?: string): Promise<{ name: string | null; address: string | null } | null>; } diff --git a/server/tests/unit/nest/maps.amap.test.ts b/server/tests/unit/nest/maps.amap.test.ts index f98ec772c..022448616 100644 --- a/server/tests/unit/nest/maps.amap.test.ts +++ b/server/tests/unit/nest/maps.amap.test.ts @@ -51,6 +51,15 @@ vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({ vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: '0'.repeat(64) })); +// The index answers search and autocomplete before any keyed provider, and +// trekPlacesEnabled fails open, so a service-level case that reached it with +// the Amap fetch stub in place would leave the runner for places.liketrek.com. +// Stubbed to "nothing found", which is the state that hands the query on. +vi.mock('../../../src/nest/maps/trek-places.client', async (importOriginal) => ({ + ...(await importOriginal()), + trekPlacesSearch: vi.fn(async (): Promise => []), +})); + import { db } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { MapsService } from '../../../src/nest/maps/maps.service'; @@ -61,7 +70,6 @@ import { isAmapPlaceId, parseAmapUrl, } from '../../../src/nest/maps/providers/amap.provider'; -import { GooglePlacesProvider } from '../../../src/nest/maps/providers/google.provider'; import { isGooglePlaceId } from '../../../src/nest/maps/maps.helpers'; import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; @@ -454,28 +462,31 @@ describe('parseAmapUrl', () => { // ── Provider selection ─────────────────────────────────────────────────────── -describe('MapsService.resolvePlacesProvider', () => { - /** Make the Google key chain answer, the Amap chain answer, or neither. */ - function keys(opts: { google?: string; amap?: string }) { - mockDbGet.mockImplementation((...args: unknown[]) => { - // resolveApiKey reads the caller's own row per name; the SQL differs but - // the stub only sees the bound userId, so answer both columns at once. - return { maps_api_key: opts.google ?? null, amap_api_key: opts.amap ?? null } as any; - }); - } +/** Make the Google key chain answer, the Amap chain answer, or neither. */ +function keys(opts: { google?: string; amap?: string }) { + mockDbGet.mockImplementation((..._args: unknown[]) => { + // resolveApiKey reads the caller's own row per name; the SQL differs but + // the stub only sees the bound userId, so answer both columns at once. + return { maps_api_key: opts.google ?? null, amap_api_key: opts.amap ?? null } as any; + }); +} +describe('MapsService.keyedProvider', () => { it('AMAP-070: auto keeps Google when a Google key is configured', () => { keys({ google: 'gkey', amap: 'akey' }); - expect(svc.resolvePlacesProvider(1)).toBeInstanceOf(GooglePlacesProvider); + expect(svc.keyedProvider(1)).toMatchObject({ id: 'google', key: 'gkey', source: 'user-row' }); + expect(svc.resolvePlacesProvider(1)).toBeNull(); }); it('AMAP-071: auto falls to Amap only when there is no Google key', () => { keys({ amap: 'akey' }); + expect(svc.keyedProvider(1)?.id).toBe('amap'); expect(svc.resolvePlacesProvider(1)).toBeInstanceOf(AmapPlacesProvider); }); it('AMAP-072: auto with no key at all means the OpenStreetMap stack', () => { keys({}); + expect(svc.keyedProvider(1)).toBeNull(); expect(svc.resolvePlacesProvider(1)).toBeNull(); }); @@ -489,20 +500,20 @@ describe('MapsService.resolvePlacesProvider', () => { mockProviderGet.mockReturnValue({ value: 'google' }); keys({ amap: 'akey' }); // Misconfigured means "answer with OSM", not "bill somebody else's provider". - expect(svc.resolvePlacesProvider(1)).toBeNull(); + expect(svc.keyedProvider(1)).toBeNull(); }); it('AMAP-075: openstreetmap ignores both keys', () => { mockProviderGet.mockReturnValue({ value: 'openstreetmap' }); keys({ google: 'gkey', amap: 'akey' }); - expect(svc.resolvePlacesProvider(1)).toBeNull(); + expect(svc.keyedProvider(1)).toBeNull(); }); it('AMAP-076: a hand-edited nonsense value degrades to auto instead of failing', () => { mockProviderGet.mockReturnValue({ value: 'not-a-provider' }); keys({ google: 'gkey' }); expect(svc.placesProviderChoice()).toBe('auto'); - expect(svc.resolvePlacesProvider(1)).toBeInstanceOf(GooglePlacesProvider); + expect(svc.keyedProvider(1)?.id).toBe('google'); }); it('AMAP-077: an Amap place stays with Amap even while Google is selected', async () => { @@ -556,10 +567,23 @@ describe('MapsService.resolvePlacesProvider', () => { }); }); -describe('MapsService.searchPlaces with Amap selected', () => { - it('AMAP-080: reports amap as the source so the client can credit it', async () => { +describe('MapsService with Amap in the keyed slot', () => { + // The index and OpenStreetMap are asked first either way; these cases are + // about what answers once they have nothing, so the index is switched off + // the way maps.service.test.ts does it for the Google cases. + let indexSpy: { mockRestore: () => void } | null = null; + afterEach(() => { + indexSpy?.mockRestore(); + indexSpy = null; + }); + function amapSelected() { + indexSpy = vi.spyOn(svc, 'trekPlacesEnabled').mockReturnValue(false); mockProviderGet.mockReturnValue({ value: 'amap' }); - mockDbGet.mockReturnValue({ maps_api_key: null, amap_api_key: 'akey' } as any); + keys({ amap: 'akey' }); + } + + it('AMAP-080: search reports amap as the source so the client can credit it', async () => { + amapSelected(); vi.stubGlobal( 'fetch', vi.fn().mockResolvedValue(ok({ pois: [{ id: 'B1', name: '外滩', location: TIANANMEN_LOCATION }] })), @@ -568,5 +592,60 @@ describe('MapsService.searchPlaces with Amap selected', () => { const result = await svc.searchPlaces(1, '外滩'); expect(result.source).toBe('amap'); expect(result.places).toHaveLength(1); + expect(result.places[0].amap_poi_id).toBe('amap:B1'); + }); + + it('AMAP-081: autocomplete goes to inputtips instead of Nominatim', async () => { + amapSelected(); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(ok({ tips: [{ id: 'T1', name: '外滩', district: '上海市黄浦区' }] }))); + + const result = await svc.autocompletePlaces(1, '外滩'); + expect(result.source).toBe('amap'); + expect(result.suggestions[0].placeId).toBe('amap:T1'); + expect(calledUrl()).toContain('/v3/assistant/inputtips'); + }); + + it('AMAP-082: reverse geocoding asks Amap at the instance key, and falls back to Nominatim when it fails', async () => { + // No user behind a reverse lookup: the chain stops at the instance-wide row. + mockProviderGet.mockReturnValue({ value: 'amap' }); + mockInstanceGet.mockReturnValue({ value: 'akey' }); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockResolvedValueOnce(amapError('10003')) + .mockResolvedValueOnce({ ok: true, json: async () => ({ name: '某处', display_name: '某地址' }) }), + ); + + const answer = await svc.reverseGeocode('39.9', '116.4'); + expect(answer).toEqual({ name: '某处', address: '某地址' }); + const calls = (globalThis.fetch as any).mock.calls.map((c: unknown[]) => String(c[0])); + expect(calls[0]).toContain('/v3/geocode/regeo'); + expect(calls[1]).toContain('nominatim'); + errorSpy.mockRestore(); + }); + + it('AMAP-083: a pasted Amap marker link resolves without touching the Google path', async () => { + // Auto with no key at all: the address comes from Nominatim, the coordinate + // and the name from the link itself, already converted to WGS-84. + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => ({ display_name: '北京市东城区' }) })); + + const result = await svc.resolveGoogleMapsUrl( + `https://uri.amap.com/marker?position=${TIANANMEN_LOCATION}&name=%E5%A4%A9%E5%AE%89%E9%97%A8`, + ); + expect(result.lat).toBeCloseTo(39.90869, 4); + expect(result.lng).toBeCloseTo(116.39124, 4); + expect(result.name).toBe('天安门'); + expect(result.address).toBe('北京市东城区'); + expect(result.google_ftid).toBeNull(); + expect(calledUrl()).toContain('nominatim'); + }); + + it('AMAP-084: an Amap POI page with no coordinate is the same 400 a bare Google page gets', async () => { + const fetchSpy = vi.fn(); + vi.stubGlobal('fetch', fetchSpy); + await expect(svc.resolveGoogleMapsUrl('https://www.amap.com/place/B000A83M61')).rejects.toMatchObject({ status: 400 }); + expect(fetchSpy).not.toHaveBeenCalled(); }); });