diff --git a/server/src/nest/maps/maps-index.helpers.ts b/server/src/nest/maps/maps-index.helpers.ts index 9f0b1e327..32800f422 100644 --- a/server/src/nest/maps/maps-index.helpers.ts +++ b/server/src/nest/maps/maps-index.helpers.ts @@ -351,6 +351,22 @@ export function mergeSuggestions(fromIndex: Suggestion[], fromPhoton: Suggestion return interleave(fromIndex, extra, limit); } +/** How long Photon's suggestions may still take once the index has answered. */ +export const PHOTON_GRACE_MS = 300; + +/** What `pending` answers within `ms`, or null when it takes longer than that. */ +export async function answerWithin(pending: Promise, ms: number): Promise { + let timer: ReturnType | undefined; + const late = new Promise((resolve) => { + timer = setTimeout(() => resolve(null), ms); + }); + try { + return await Promise.race([pending, late]); + } finally { + clearTimeout(timer); + } +} + /** * The details record of a place picked out of the index, with what * OpenStreetMap knows about the same building filled in. diff --git a/server/src/nest/maps/maps.service.ts b/server/src/nest/maps/maps.service.ts index e0e90b06f..1e252bd97 100644 --- a/server/src/nest/maps/maps.service.ts +++ b/server/src/nest/maps/maps.service.ts @@ -5,6 +5,7 @@ import type { GeoLane } from '../geo/nominatim.client'; import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; import type { ApiKeySource } from '../settings/instance-api-keys'; import { + answerWithin, belonging, indexPoiAnswer, indexPoiPlan, @@ -12,6 +13,7 @@ import { indexSuggestions, mergeIndexPages, mergeSuggestions, + PHOTON_GRACE_MS, } from './maps-index.helpers'; import { NEARBY_DEFAULT_LIMIT, NEARBY_DEFAULT_RADIUS_M, nearbyCacheKey, nearestFirst } from './maps-nearby.helpers'; import { MapsUrlResolver, type ResolvedMapsUrl } from './maps-url.resolver'; @@ -557,22 +559,25 @@ export class MapsService { // Photon beside them (#2877). The layer is a slice of OpenStreetMap, and // a place missing from it showed up only once the search was sent. // Photon is built for this keystroke and answers null when the services - // are off, so the pair costs the slower one and an install without - // Photon reads exactly as before. Each side swallows its own failure, so - // one that is down does not throw away what the other found. - const [found, photon] = await Promise.all([ - trekPlacesSearch(input, { - lat: boxCentre?.lat, - lng: boxCentre?.lng, - limit: 8, - sources: 'index,osm', - lang, - }).catch((err: unknown) => { - console.warn('TREK Places autocomplete failed, falling back:', (err as Error).message); - return []; - }), - this.osm.autocompletePhoton(input, lang, boxCentre), - ]); + // are off, so an install without Photon reads exactly as before. Each + // side swallows its own failure, so one that is down does not throw away + // what the other found. Photon is the extra here, so once the index has + // answered it gets a moment more and is then dropped: a slow Photon must + // not hold back what the index already has. + const typing = new AbortController(); + const asked = this.osm.autocompletePhoton(input, lang, boxCentre, typing.signal); + const found = await trekPlacesSearch(input, { + lat: boxCentre?.lat, + lng: boxCentre?.lng, + limit: 8, + sources: 'index,osm', + lang, + }).catch((err: unknown) => { + console.warn('TREK Places autocomplete failed, falling back:', (err as Error).message); + return []; + }); + const photon = found.length > 0 ? await answerWithin(asked, PHOTON_GRACE_MS) : await asked; + typing.abort(); const suggestions = mergeSuggestions(indexSuggestions(found, lang), photon ?? []); if (suggestions.length > 0) { return { suggestions, source: found.length > 0 ? 'trek-places' : 'openstreetmap' }; diff --git a/server/src/nest/maps/providers/osm.client.ts b/server/src/nest/maps/providers/osm.client.ts index b33fb4ec4..28a903fee 100644 --- a/server/src/nest/maps/providers/osm.client.ts +++ b/server/src/nest/maps/providers/osm.client.ts @@ -529,10 +529,11 @@ export class OsmClient { /** * Suggestions from Photon, which is built for search-as-you-type, or null when * it is not in use or did not answer. Never Nominatim in its place: its usage - * policy rules out autocomplete. + * policy rules out autocomplete. Aborting `signal` drops a question nobody + * waits for any more. */ - async autocompletePhoton(input: string, lang?: string, bias?: { lat: number; lng: number }) { - const found = await this.photon.search(input, { lang, bias }); + async autocompletePhoton(input: string, lang?: string, bias?: { lat: number; lng: number }, signal?: AbortSignal) { + const found = await this.photon.suggest(input, { lang, bias, signal }); return found?.map(asSuggestion).filter((row) => row !== null) ?? null; } diff --git a/server/src/nest/maps/providers/photon.client.ts b/server/src/nest/maps/providers/photon.client.ts index ee398292e..dd6d6248b 100644 --- a/server/src/nest/maps/providers/photon.client.ts +++ b/server/src/nest/maps/providers/photon.client.ts @@ -14,6 +14,12 @@ * A spent allowance says itself how long to wait (Retry-After), and that wait * is kept instead. * + * The suggestions asked on every keystroke keep a tally of their own. They + * come far more often than a search, under a shorter deadline, and many are + * dropped unanswered once the next keystroke is in, so their misses and their + * spent allowance pause only the suggestions. A pause of the search holds the + * suggestions back too: a host the search cannot reach is not asked per letter. + * * The key comes from serviceIdentitySlot, as it does for the Places request * (trek-places.fetch.ts), so the maps domain reads the installation identity * one way and needs no import of ServiceIdentityModule; the slot holds the @@ -33,6 +39,8 @@ import type { ConfigType } from '@nestjs/config'; /** Short, because Nominatim still has to fit behind it inside the browser's wait. */ const TIMEOUT_MS = 2500; +/** The suggestion list is waiting on the index beside it, and the next keystroke is near. */ +const SUGGEST_TIMEOUT_MS = 1500; /** Ten hits are a few kB. */ const MAX_BYTES = 512_000; const SEARCH_LIMIT = 10; @@ -51,22 +59,33 @@ interface PhotonSearchOptions { bias?: { lat: number; lng: number }; } +/** The misses in a row and the pause of one kind of question. */ +interface Tally { + /** What answers instead while Photon does not. */ + fallback: string; + misses: number; + pausedUntil: number; +} + @Injectable() export class PhotonClient { - private misses = 0; - private pausedUntil = 0; + private readonly lookups: Tally = { fallback: 'asking Nominatim', misses: 0, pausedUntil: 0 }; + private readonly suggestions: Tally = { fallback: 'suggesting from the index', misses: 0, pausedUntil: 0 }; constructor(@Inject(servicesConfig.KEY) private readonly config: ConfigType) {} /** Places matching the query, best first, or null when Nominatim has to answer. */ search(query: string, opts: PhotonSearchOptions = {}): Promise { - const params = new URLSearchParams({ q: query, limit: String(SEARCH_LIMIT), lang: photonLang(opts.lang) }); - if (opts.bias && Number.isFinite(opts.bias.lat) && Number.isFinite(opts.bias.lng)) { - params.set('lat', String(opts.bias.lat)); - params.set('lon', String(opts.bias.lng)); - params.set('zoom', String(BIAS_ZOOM)); - } - return this.ask('api', params); + return this.ask('api', searchParams(query, opts), this.lookups); + } + + /** + * Places for the suggestion list, or null when it goes without them. A caller + * that stops waiting aborts `signal`, and that counts as no miss. + */ + suggest(query: string, opts: PhotonSearchOptions & { signal?: AbortSignal } = {}): Promise { + if (Date.now() < this.lookups.pausedUntil) return Promise.resolve(null); + return this.ask('api', searchParams(query, opts), this.suggestions, SUGGEST_TIMEOUT_MS, opts.signal); } /** @@ -76,15 +95,18 @@ export class PhotonClient { reverse(lat: number, lng: number, lang?: string, timeoutMs?: number): Promise { if (!Number.isFinite(lat) || !Number.isFinite(lng)) return Promise.resolve(null); const params = new URLSearchParams({ lat: String(lat), lon: String(lng), limit: '1', lang: photonLang(lang) }); - return this.ask('reverse', params, timeoutMs ? Math.min(TIMEOUT_MS, timeoutMs / 2) : TIMEOUT_MS); + const deadline = timeoutMs ? Math.min(TIMEOUT_MS, timeoutMs / 2) : TIMEOUT_MS; + return this.ask('reverse', params, this.lookups, deadline); } private async ask( path: 'api' | 'reverse', params: URLSearchParams, + tally: Tally, timeoutMs = TIMEOUT_MS, + signal?: AbortSignal, ): Promise { - if (!this.mayAsk()) return null; + if (!this.mayAsk(tally)) return null; const url = new URL(`${this.config.photonUrl}/${path}`); url.search = params.toString(); const identity = serviceIdentitySlot.get(); @@ -95,13 +117,13 @@ export class PhotonClient { headers: { Accept: 'application/json', ...headers }, // The key goes to this host alone, never on to wherever it points. redirect: 'error', - signal: AbortSignal.timeout(timeoutMs), + signal: signal ? AbortSignal.any([signal, AbortSignal.timeout(timeoutMs)]) : AbortSignal.timeout(timeoutMs), }); identity.observe(url, res); const wait = res.status === 429 ? retryAfterMs(res.headers.get('retry-after')) : null; if (wait !== null) { await res.body?.cancel(); - this.pauseFor(wait); + this.pauseFor(tally, wait); return null; } if (!res.ok) { @@ -109,37 +131,47 @@ export class PhotonClient { throw new Error(`answered ${res.status}`); } const { features } = photonAnswerSchema.parse(await capResponse(res, MAX_BYTES).json()); - this.misses = 0; + tally.misses = 0; return features; } catch (err) { - this.noteMiss(err); + if (!signal?.aborted) this.noteMiss(tally, err); return null; } } /** Off with the services, beside an operator's own Nominatim, and during the pause. */ - private mayAsk(): boolean { + private mayAsk(tally: Tally): boolean { if (this.config.mode === 'off' || readEnv().integrations.nominatimOwn) return false; - return Date.now() >= this.pausedUntil; + return Date.now() >= tally.pausedUntil; } - private noteMiss(err: unknown): void { + private noteMiss(tally: Tally, err: unknown): void { const reason = err instanceof Error ? err.message : String(err); - this.misses += 1; - if (this.misses < MISSES_BEFORE_PAUSE) { - logDebug(`[Photon] ${reason}; asking Nominatim instead`); + tally.misses += 1; + if (tally.misses < MISSES_BEFORE_PAUSE) { + logDebug(`[Photon] ${reason}; ${tally.fallback} instead`); return; } - this.pausedUntil = Date.now() + PAUSE_MS; + tally.pausedUntil = Date.now() + PAUSE_MS; // One more miss after the pause starts the next one. - this.misses = MISSES_BEFORE_PAUSE - 1; - logWarn(`[Photon] ${reason}; asking Nominatim alone for a minute`); + tally.misses = MISSES_BEFORE_PAUSE - 1; + logWarn(`[Photon] ${reason}; ${tally.fallback} alone for a minute`); } - private pauseFor(ms: number): void { + private pauseFor(tally: Tally, ms: number): void { const wait = Math.min(ms, MAX_PAUSE_MS); - this.pausedUntil = Date.now() + wait; - this.misses = MISSES_BEFORE_PAUSE - 1; - logWarn(`[Photon] allowance spent; asking Nominatim alone for ${Math.ceil(wait / 1000)} s`); + tally.pausedUntil = Date.now() + wait; + tally.misses = MISSES_BEFORE_PAUSE - 1; + logWarn(`[Photon] allowance spent; ${tally.fallback} alone for ${Math.ceil(wait / 1000)} s`); } } + +function searchParams(query: string, opts: PhotonSearchOptions): URLSearchParams { + const params = new URLSearchParams({ q: query, limit: String(SEARCH_LIMIT), lang: photonLang(opts.lang) }); + if (opts.bias && Number.isFinite(opts.bias.lat) && Number.isFinite(opts.bias.lng)) { + params.set('lat', String(opts.bias.lat)); + params.set('lon', String(opts.bias.lng)); + params.set('zoom', String(BIAS_ZOOM)); + } + return params; +} diff --git a/server/tests/unit/nest/maps.autocomplete.test.ts b/server/tests/unit/nest/maps.autocomplete.test.ts index fd7f83c78..5733f2ea0 100644 --- a/server/tests/unit/nest/maps.autocomplete.test.ts +++ b/server/tests/unit/nest/maps.autocomplete.test.ts @@ -1,5 +1,5 @@ /** - * MAPS-AUTO-001..024: the suggestion list behind the place search box. + * MAPS-AUTO-001..026: the suggestion list behind the place search box. * * This is the path the index was built for. Nominatim's usage policy names * autocomplete as unacceptable use in its own words, whatever the rate, and the @@ -11,6 +11,7 @@ */ import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository'; import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; +import { PHOTON_GRACE_MS } from '../../../src/nest/maps/maps-index.helpers'; import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; import { noGoogleQuota } from '../../helpers/google-quota'; import { buildMapsParts, buildMapsService } from '../../helpers/maps-service'; @@ -475,7 +476,7 @@ describe('MapsService.autocompletePlaces', () => { const result = await svc.autocompletePlaces(1, INPUT, 'de', bias); - expect(photon).toHaveBeenCalledWith(INPUT, 'de', { lat: 54.1, lng: 12.1 }); + expect(photon).toHaveBeenCalledWith(INPUT, 'de', { lat: 54.1, lng: 12.1 }, expect.any(AbortSignal)); expect(result.source).toBe('trek-places'); expect(result.suggestions.map((s) => s.placeId)).toEqual([ 'gers:abc-123', @@ -516,4 +517,33 @@ describe('MapsService.autocompletePlaces', () => { expect(result.source).toBe('google'); vi.unstubAllGlobals(); }); + + it('MAPS-AUTO-025: once the index has answered, a slow Photon gets a moment more and is then dropped', async () => { + mockSearch.mockResolvedValue([hit()]); + const { svc, photon } = withPhoton(null); + photon.mockImplementation(() => new Promise(() => undefined)); + const started = Date.now(); + + const result = await svc.autocompletePlaces(1, INPUT); + + expect(Date.now() - started).toBeGreaterThanOrEqual(PHOTON_GRACE_MS - 50); + expect(result).toEqual({ + suggestions: [expect.objectContaining({ placeId: 'gers:abc-123' })], + source: 'trek-places', + }); + // The question nobody waits for any more is called off, which is no miss for Photon. + expect((photon.mock.calls[0][3] as AbortSignal).aborted).toBe(true); + }); + + it('MAPS-AUTO-026: with nothing from the index, Photon is waited for past that moment', async () => { + mockSearch.mockResolvedValue([]); + const { svc, photon } = withPhoton(null); + photon.mockImplementation( + () => new Promise((resolve) => setTimeout(() => resolve([photonRow()]), PHOTON_GRACE_MS + 100)), + ); + + const result = await svc.autocompletePlaces(1, INPUT); + + expect(result).toEqual({ suggestions: [photonRow()], source: 'openstreetmap' }); + }); }); diff --git a/server/tests/unit/nest/photon.client.test.ts b/server/tests/unit/nest/photon.client.test.ts index 7e657b1cd..a12f171a3 100644 --- a/server/tests/unit/nest/photon.client.test.ts +++ b/server/tests/unit/nest/photon.client.test.ts @@ -1,5 +1,5 @@ /** - * PHOTON-001..016: when Photon is asked, what it is asked, and every way it + * PHOTON-001..019: when Photon is asked, what it is asked, and every way it * hands the question back to Nominatim. The network is a stubbed global fetch, * the installation identity a fake in the slot the service-identity module fills. */ @@ -197,3 +197,63 @@ describe('a spent allowance and a short deadline', () => { timeout.mockRestore(); }); }); + +describe('the suggestions asked per keystroke', () => { + it('PHOTON-017: their misses and a spent allowance pause the suggestions alone, never the search', async () => { + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(new Date('2026-10-29T10:00:00Z')); + const photon = client(); + fetchMock.mockImplementation(async () => photonAnswer([], 503)); + expect(await oneByOne([1, 2, 3, 4], () => photon.suggest('Ros'))).toEqual([null, null, null, null]); + expect(await photon.suggest('Rost')).toBeNull(); + expect(fetchMock).toHaveBeenCalledTimes(4); + + fetchMock.mockImplementation(async () => photonAnswer([gateFeature()])); + expect(await photon.search('Rostock')).toHaveLength(1); + expect(fetchMock).toHaveBeenCalledTimes(5); + + vi.setSystemTime(new Date('2026-10-29T10:01:01Z')); + fetchMock.mockResolvedValueOnce( + new Response('{}', { status: 429, headers: { 'content-type': 'application/json', 'retry-after': '600' } }), + ); + expect(await photon.suggest('Rost')).toBeNull(); + expect(await photon.suggest('Rosto')).toBeNull(); + expect(await photon.search('Rostock')).toHaveLength(1); + expect(await photon.reverse(52.5, 13.4)).toHaveLength(1); + expect(fetchMock).toHaveBeenCalledTimes(8); + }); + + it('PHOTON-018: a pause of the search holds them back too', async () => { + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(new Date('2026-10-29T10:00:00Z')); + const photon = client(); + fetchMock.mockImplementation(async () => photonAnswer([], 503)); + await oneByOne([1, 2, 3, 4], () => photon.search('Rostock')); + expect(await photon.suggest('Ros')).toBeNull(); + expect(fetchMock).toHaveBeenCalledTimes(4); + }); + + it('PHOTON-019: they wait less than a search, and one called off by its caller is no miss', async () => { + const timeout = vi.spyOn(AbortSignal, 'timeout'); + const photon = client(); + expect(await photon.suggest('Ros', { lang: 'de', bias: { lat: 54.1, lng: 12.1 } })).toHaveLength(1); + expect(timeout.mock.calls.map(([ms]) => ms)).toEqual([1500]); + expect(request().url.searchParams.get('lat')).toBe('54.1'); + timeout.mockRestore(); + + fetchMock.mockImplementation( + (_url: URL, init: RequestInit) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError'))); + }), + ); + for (let i = 0; i < 5; i += 1) { + const dropped = new AbortController(); + const asked = photon.suggest('Rost', { signal: dropped.signal }); + dropped.abort(); + expect(await asked).toBeNull(); + } + fetchMock.mockImplementation(async () => photonAnswer([gateFeature()])); + expect(await photon.suggest('Rostock')).toHaveLength(1); + }); +});