From ef4b564345ff440c01a44530a12e33fef710b0f9 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Mon, 29 Jun 2026 07:42:20 -0400 Subject: [PATCH] refactor(geocode): source near-me URL + UA from shared geocoder module (om-slun) geocode.ts hardcoded NOMINATIM_SEARCH_URL + USER_AGENT, duplicating literals that already have a canonical home in geocoder.ts. Converge on one source so they can't drift: - geocoder.ts: export DEFAULT_GEOCODER_URL + DEFAULT_GEOCODER_USER_AGENT and add resolveGeocoderUserAgent(env) (honors GEOCODER_USER_AGENT); createGeocoder now uses the resolver too. - geocode.ts: drop its own literals; import the shared URL + UA helper; accept an optional env so the near-me box honors GEOCODER_USER_AGENT. Stays on public Nominatim deliberately (does NOT adopt GEOCODER_URL: that may be a keyed LocationIQ endpoint and this hot-path forward search sends no ?key=, so it would 401 on the live config). - geocode.remote.ts: thread platform.env into geocodeLocation. - geocode.test.ts: assert against the shared source + cover the UA env override. Lands on the open search PR (#55) before it merges upstream, per om-slun. --- apps/web/src/lib/search/geocode.remote.ts | 5 ++-- .../web/src/lib/search/server/geocode.test.ts | 23 ++++++++++++++++--- apps/web/src/lib/search/server/geocode.ts | 17 +++++++++----- apps/web/src/lib/search/server/geocoder.ts | 22 ++++++++++++++---- 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/apps/web/src/lib/search/geocode.remote.ts b/apps/web/src/lib/search/geocode.remote.ts index d5ca806..2016ded 100644 --- a/apps/web/src/lib/search/geocode.remote.ts +++ b/apps/web/src/lib/search/geocode.remote.ts @@ -1,4 +1,4 @@ -import { command } from '$app/server'; +import { command, getRequestEvent } from '$app/server'; import { error } from '@sveltejs/kit'; import { geocodeLocation } from './server/geocode'; import { geocodeInput } from './geocode-input'; @@ -8,8 +8,9 @@ import { geocodeInput } from './geocode-input'; * geocoder sees the Worker, not the user. Returns null when nothing * matches (distinct from upstream failure, which is a 502). */ export const geocodeQuery = command(geocodeInput, async ({ q }) => { + const { platform } = getRequestEvent(); try { - return await geocodeLocation(q); + return await geocodeLocation(q, fetch, platform?.env); } catch { error(502, 'Location lookup is unavailable right now'); } diff --git a/apps/web/src/lib/search/server/geocode.test.ts b/apps/web/src/lib/search/server/geocode.test.ts index d72aeb2..e9d15ea 100644 --- a/apps/web/src/lib/search/server/geocode.test.ts +++ b/apps/web/src/lib/search/server/geocode.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import { geocodeLocation } from './geocode'; +import { DEFAULT_GEOCODER_URL, DEFAULT_GEOCODER_USER_AGENT } from './geocoder'; function fakeFetch(status: number, body: unknown) { return vi.fn(async () => new Response(JSON.stringify(body), { status })); @@ -20,13 +21,18 @@ describe('geocodeLocation', () => { expect(fetchImpl).toHaveBeenCalledTimes(1); const [input, init] = fetchImpl.mock.calls[0] as unknown as [URL | string, RequestInit]; const url = new URL(String(input)); + // URL is sourced from the shared geocoder constant, not a local literal. + const shared = new URL(DEFAULT_GEOCODER_URL); + expect(url.hostname).toBe(shared.hostname); + expect(url.pathname).toBe(shared.pathname); expect(url.hostname).toBe('nominatim.openstreetmap.org'); - expect(url.pathname).toBe('/search'); expect(url.searchParams.get('q')).toBe('Louisville, KY'); expect(url.searchParams.get('format')).toBe('jsonv2'); expect(url.searchParams.get('limit')).toBe('1'); - // Nominatim's usage policy requires an identifying User-Agent. - expect(new Headers(init.headers).get('user-agent')).toMatch(/atmo/i); + // Nominatim's usage policy requires an identifying User-Agent; default + // comes from the shared env-driven helper. + expect(new Headers(init.headers).get('user-agent')).toBe(DEFAULT_GEOCODER_USER_AGENT); + expect(DEFAULT_GEOCODER_USER_AGENT).toMatch(/atmo/i); expect(result).toEqual({ lat: 38.2542, @@ -35,6 +41,17 @@ describe('geocodeLocation', () => { }); }); + it('sources the User-Agent from GEOCODER_USER_AGENT when set', async () => { + const fetchImpl = fakeFetch(200, louisville); + + await geocodeLocation('Louisville, KY', fetchImpl, { + GEOCODER_USER_AGENT: 'custom-agent/9.9 (https://example.test)' + }); + + const [, init] = fetchImpl.mock.calls[0] as unknown as [URL | string, RequestInit]; + expect(new Headers(init.headers).get('user-agent')).toBe('custom-agent/9.9 (https://example.test)'); + }); + it('returns null when nothing matches', async () => { expect(await geocodeLocation('zzzz no such place', fakeFetch(200, []))).toBeNull(); }); diff --git a/apps/web/src/lib/search/server/geocode.ts b/apps/web/src/lib/search/server/geocode.ts index 22156cd..f2599f2 100644 --- a/apps/web/src/lib/search/server/geocode.ts +++ b/apps/web/src/lib/search/server/geocode.ts @@ -6,6 +6,13 @@ // requirement is satisfied by the OpenStreetMap credit shown on the near-me // page. Heavier-traffic compliance (app-wide rate limiting, caching) is still // a follow-up before high-volume exposure. +// +// Base URL and User-Agent come from the shared geocoder module (one canonical +// source, no duplicate literals). This path stays on public Nominatim — it does +// NOT honor a configured GEOCODER_URL, because that endpoint may be a keyed +// LocationIQ URL and this hot-path forward search sends no ?key= (it would 401). +import { DEFAULT_GEOCODER_URL, resolveGeocoderUserAgent, type GeocoderEnv } from './geocoder'; + export type GeocodeResult = { lat: number; lng: number; @@ -13,20 +20,18 @@ export type GeocodeResult = { label: string; }; -const NOMINATIM_SEARCH_URL = 'https://nominatim.openstreetmap.org/search'; -const USER_AGENT = 'atmo-events (https://atmo.rsvp)'; - export async function geocodeLocation( q: string, - fetchImpl: typeof fetch = fetch + fetchImpl: typeof fetch = fetch, + env: GeocoderEnv = {} ): Promise { - const url = new URL(NOMINATIM_SEARCH_URL); + const url = new URL(DEFAULT_GEOCODER_URL); url.searchParams.set('q', q); url.searchParams.set('format', 'jsonv2'); url.searchParams.set('limit', '1'); const response = await fetchImpl(url, { - headers: { accept: 'application/json', 'user-agent': USER_AGENT } + headers: { accept: 'application/json', 'user-agent': resolveGeocoderUserAgent(env) } }); if (!response.ok) { throw new Error(`geocode request failed: ${response.status}`); diff --git a/apps/web/src/lib/search/server/geocoder.ts b/apps/web/src/lib/search/server/geocoder.ts index 595db97..6e6cf54 100644 --- a/apps/web/src/lib/search/server/geocoder.ts +++ b/apps/web/src/lib/search/server/geocoder.ts @@ -25,15 +25,27 @@ export interface GeocoderEnv { GEOCODER_USER_AGENT?: string; } -const DEFAULT_URL = 'https://nominatim.openstreetmap.org/search'; -const DEFAULT_USER_AGENT = 'atmo-events (https://atmo.rsvp)'; +/** Public OSM Nominatim /search endpoint — the keyless default and the single + * source of the base geocoder URL. The near-me search box (geocode.ts) imports + * this so the two paths can't drift onto different hardcoded literals. */ +export const DEFAULT_GEOCODER_URL = 'https://nominatim.openstreetmap.org/search'; +/** Default identifying User-Agent. Nominatim's usage policy requires one; an + * operator can override it via GEOCODER_USER_AGENT (see resolveGeocoderUserAgent). */ +export const DEFAULT_GEOCODER_USER_AGENT = 'atmo-events (https://atmo.rsvp)'; const PUBLIC_NOMINATIM_HOST = 'nominatim.openstreetmap.org'; +/** Resolve the geocoder User-Agent: operator override (GEOCODER_USER_AGENT) else + * the atmo default. The single env-driven source both the bulk client and the + * near-me search box use, so the UA can't drift across paths. */ +export function resolveGeocoderUserAgent(env: GeocoderEnv = {}): string { + return env.GEOCODER_USER_AGENT || DEFAULT_GEOCODER_USER_AGENT; +} + /** True when the effective geocoder endpoint is public OSM Nominatim — i.e. the * shared, usage-policy-bound host. The in-Worker drip uses this to pick SAFE * defaults (a smaller per-tick cap + a slower throttle floor) when on Nominatim, * rather than refusing to run: the drip works keyless out of the box, just - * policy-compliantly. `createGeocoder` falls back to DEFAULT_URL (this host) + * policy-compliantly. `createGeocoder` falls back to DEFAULT_GEOCODER_URL (this host) * when GEOCODER_URL is unset — even with a key present, in which case the ?key= * is ignored and we're really on Nominatim — so an unset URL counts as public. * Malformed URL → treated as public (fail safe toward the slower limits). A @@ -139,9 +151,9 @@ export function derivePrecision(top: { } export function createGeocoder(env: GeocoderEnv = {}, fetchImpl: typeof fetch = fetch): Geocoder { - const base = env.GEOCODER_URL || DEFAULT_URL; + const base = env.GEOCODER_URL || DEFAULT_GEOCODER_URL; const key = env.GEOCODER_KEY; - const userAgent = env.GEOCODER_USER_AGENT || DEFAULT_USER_AGENT; + const userAgent = resolveGeocoderUserAgent(env); return { async geocode(q: string): Promise { -- 2.51.2