From ba535e5aab5f83c6de14fe934a3d4754aedf1cce Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sat, 12 Sep 2026 17:25:23 +0200
Subject: [PATCH] fix(maps): ask Amap only where Amap can answer

With Amap in the keyed slot every reverse lookup went to Amap first,
worldwide. Outside its box that buys a round trip and an empty answer
before Nominatim is asked anyway, which is latency on every right click in
Lisbon. The client side of the same question already filters by the box
(placeNavigation.ts); isOutsideChina is the shared helper both use.

The two fixtures in the file that were cast to any now carry a type: the
key row and the fetch spy whose calls the reverse case reads back.
---
 server/src/nest/maps/maps.service.ts     |  7 +++--
 server/tests/unit/nest/maps.amap.test.ts | 36 +++++++++++++++++-------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/server/src/nest/maps/maps.service.ts b/server/src/nest/maps/maps.service.ts
index cc5bd15c3..f2666f1f4 100644
--- a/server/src/nest/maps/maps.service.ts
+++ b/server/src/nest/maps/maps.service.ts
@@ -1,4 +1,5 @@
 import { Injectable } from '@nestjs/common';
+import { isOutsideChina } from '@trek/shared';
 import type {
   MapsSearchResult,
   MapsAutocompleteResult,
@@ -2819,7 +2820,9 @@ 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,
+    // Amap answers first when it holds the keyed slot, and only for a point it
+    // can possibly know: outside its box the call would cost a round trip to
+    // come back empty before Nominatim is asked anyway. 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
@@ -2829,7 +2832,7 @@ export class MapsService {
     if (amap) {
       const latNum = Number.parseFloat(lat);
       const lngNum = Number.parseFloat(lng);
-      if (Number.isFinite(latNum) && Number.isFinite(lngNum)) {
+      if (Number.isFinite(latNum) && Number.isFinite(lngNum) && !isOutsideChina(latNum, lngNum)) {
         try {
           const answer = await amap.reverse(latNum, lngNum, lang);
           if (answer) return answer;
diff --git a/server/tests/unit/nest/maps.amap.test.ts b/server/tests/unit/nest/maps.amap.test.ts
index 022448616..dc3a26664 100644
--- a/server/tests/unit/nest/maps.amap.test.ts
+++ b/server/tests/unit/nest/maps.amap.test.ts
@@ -462,12 +462,15 @@ describe('parseAmapUrl', () => {
 
 // ── Provider selection ───────────────────────────────────────────────────────
 
+/** The user row resolveApiKey reads, with both key columns on it. */
+type KeyRow = { maps_api_key: string | null; amap_api_key: string | null };
+
 /** Make the Google key chain answer, the Amap chain answer, or neither. */
 function keys(opts: { google?: string; amap?: string }) {
-  mockDbGet.mockImplementation((..._args: unknown[]) => {
+  mockDbGet.mockImplementation((..._args: unknown[]): KeyRow => {
     // 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;
+    return { maps_api_key: opts.google ?? null, amap_api_key: opts.amap ?? null };
   });
 }
 
@@ -610,22 +613,35 @@ describe('MapsService with Amap in the keyed slot', () => {
     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 fetchSpy = vi
+      .fn()
+      .mockResolvedValueOnce(amapError('10003'))
+      .mockResolvedValueOnce({ ok: true, json: async () => ({ name: '某处', display_name: '某地址' }) });
+    vi.stubGlobal('fetch', fetchSpy);
 
     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]));
+    const calls = fetchSpy.mock.calls.map(call => String(call[0]));
     expect(calls[0]).toContain('/v3/geocode/regeo');
     expect(calls[1]).toContain('nominatim');
     errorSpy.mockRestore();
   });
 
+  it('AMAP-082b: a point outside the Amap box goes straight to Nominatim', async () => {
+    // Lisbon. Amap holds the slot, but it knows nothing out here, so asking it
+    // buys a round trip and an empty answer before the fallback runs anyway.
+    mockProviderGet.mockReturnValue({ value: 'amap' });
+    mockInstanceGet.mockReturnValue({ value: 'akey' });
+    const fetchSpy = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ name: 'Belém', display_name: 'Lisboa' }) });
+    vi.stubGlobal('fetch', fetchSpy);
+
+    const answer = await svc.reverseGeocode('38.6916', '-9.2160');
+
+    expect(answer).toEqual({ name: 'Belém', address: 'Lisboa' });
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+    expect(String(fetchSpy.mock.calls[0][0])).toContain('nominatim');
+  });
+
   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.