diff --git a/packages/types/lib/config.d.ts b/packages/types/lib/config.d.ts index 0a2b80246..15bbf422a 100644 --- a/packages/types/lib/config.d.ts +++ b/packages/types/lib/config.d.ts @@ -199,6 +199,12 @@ export interface Webhook { * Which geocoding backend `nominatimUrl` points at. Photon speaks GeoJSON * rather than Nominatim's JSON, so it needs its own request and response * handling. Defaults to `nominatim`. + * + * Optional when Poracle reports it. Poracle sends the backend behind its + * `providerURL` as `provider`, and ReactMap uses that when this is unset and + * `nominatimUrl` was itself inherited from Poracle. A locally configured URL + * keeps its local backend, since the two are one setting in two fields. + * Setting this explicitly always wins. */ geocoderProvider?: 'nominatim' | 'photon' trialPeriodEligible?: boolean diff --git a/server/src/services/Poracle.js b/server/src/services/Poracle.js index 06a8c0ce6..400487743 100644 --- a/server/src/services/Poracle.js +++ b/server/src/services/Poracle.js @@ -41,6 +41,40 @@ const SUBCATEGORIES = /** @type {const} */ ({ gym: ['raid', 'egg', 'gym'], }) +/** + * Geocoding backends ReactMap can drive. Poracle also offers `google` and + * `none`, which have no equivalent here, so they are ignored rather than + * mapped onto something they are not. + */ +const REMOTE_GEOCODERS = new Set(['nominatim', 'photon']) + +/** + * Resolves which geocoding backend a webhook should use. + * + * Poracle reports the backend behind its `providerURL` as `provider`, so a + * deployment that gets its geocoder URL from Poracle no longer has to restate + * the backend type in ReactMap's own config. Local config still wins, matching + * how `providerURL` and `addressFormat` already behave. + * + * The backend type is only inherited when the URL was inherited with it. A URL + * and the protocol used to talk to it are one setting in two fields, and + * splitting them across two sources is how a working Nominatim deployment ends + * up being addressed as Photon: keep a local `nominatimUrl`, omit + * `geocoderProvider`, and let a Photon-backed Poracle supply the type. + * @param {object} args + * @param {string} [args.remote] The `provider` field from Poracle's remote config + * @param {string} [args.local] geocoderProvider from this webhook's own config + * @param {boolean} args.usingRemoteURL Whether nominatimUrl also came from Poracle + * @returns {'nominatim' | 'photon' | undefined} + */ +function resolveGeocoderProvider({ remote, local, usingRemoteURL }) { + if (local) return /** @type {'nominatim' | 'photon'} */ (local) + if (!usingRemoteURL) return undefined + return REMOTE_GEOCODERS.has(remote) + ? /** @type {'nominatim' | 'photon'} */ (remote) + : undefined +} + class PoracleAPI { /** @param {import("@rm/types").Config['webhooks'][number]} webhook */ constructor(webhook) { @@ -231,14 +265,26 @@ class PoracleAPI { `Poracle must be at least version 4.8.4, current version is ${this.version}`, ) } - const { providerURL, addressFormat, ...rest } = remoteConfig + // `provider` is pulled out of remoteConfig even when it is not usable. + // this.provider is the *webhook* provider ('poracle'); letting Poracle's + // geocoding provider through the spread below would silently overwrite it + // with an unrelated meaning. + const { providerURL, provider, addressFormat, ...rest } = remoteConfig Object.assign(this, rest) if (addressFormat && !this.addressFormat) { this.addressFormat = addressFormat } - if (providerURL && !this.nominatimUrl) { + // Evaluated before the assignment below, because it is the assignment that + // decides whether the backend type may be inherited too. + const usingRemoteURL = !!providerURL && !this.nominatimUrl + if (usingRemoteURL) { this.nominatimUrl = providerURL } + this.geocoderProvider = resolveGeocoderProvider({ + remote: provider, + local: this.geocoderProvider, + usingRemoteURL, + }) this.leagues = [ { name: 'great', cp: 1500, min: remoteConfig.pvpFilterGreatMinCP }, { name: 'ultra', cp: 2500, min: remoteConfig.pvpFilterUltraMinCP }, @@ -1257,4 +1303,4 @@ class PoracleAPI { } } -module.exports = { PoracleAPI } +module.exports = { PoracleAPI, resolveGeocoderProvider } diff --git a/server/src/services/geocoder.js b/server/src/services/geocoder.js index 94e32a114..55aef9224 100644 --- a/server/src/services/geocoder.js +++ b/server/src/services/geocoder.js @@ -12,7 +12,7 @@ const { photonGeocoder } = require('./photonGeocoder') function formatter(addressFormat, result) { return addressFormat .replace( - /{{(streetNumber|streetName|city|state|country|zipcode|latitude|longitude|countryCode|neighborhoods|suburb|town|village)}}/g, + /{{(streetNumber|streetName|city|state|country|zipcode|latitude|longitude|countryCode|neighborhoods|neighborhood|neighbourhood|suburb|town|village)}}/g, (a, b) => result[b] || '', ) .trim() @@ -20,6 +20,99 @@ function formatter(addressFormat, result) { .trim() } +/** + * The empty answer for a caller, in the shape its schema declares. + * + * Query.geocoder is [Geocoder] and Gym.formatted is String, so an object here + * makes GraphQL discard the field with "Expected Iterable" or "String cannot + * represent value" rather than degrading to an empty result. + * @param {boolean} reverse + */ +function emptyResult(reverse) { + return reverse ? '' : [] +} + +/** + * Fails loudly when the configured URL answered with something that is not a + * Nominatim response. + * + * node-geocoder decides how to read a body by its shape: an array is treated as + * a result list, and anything else is treated as a single result. Photon + * answers with a GeoJSON object, so the whole FeatureCollection gets handed to + * _formatResult as though it were one place. Nothing throws, because + * node-geocoder guards the address lookup, so the caller silently receives one + * entry with every field undefined. + * + * That is a misconfiguration rather than a geocoding failure, and it is + * invisible in the results, so it is worth an error naming the fix. + * @param {any} results + * @param {string} url + */ +function assertNominatimResponse(results, url) { + const raw = results?.raw + if (!raw || Array.isArray(raw) || typeof raw !== 'object') return + + // Photon rarely gets far enough to answer with GeoJSON here, because + // node-geocoder asks for routes and parameters it does not serve. Verified + // against a live instance: + // + // GET /search?q=..&format=json&addressdetails=1 + // 404 {"title":"Endpoint GET /search not found","status":404,...} + // Photon serves /api, not /search. + // + // GET /reverse?lat=..&lon=..&format=json&addressdetails=1 + // 400 {"message":"Unknown query parameter 'format'. Allowed parameters + // are: [include, debug, dedupe, ...]"} + // Photon rejects anything outside its allow list, and format and + // addressdetails are forced onto every request by node-geocoder. + // + // node-geocoder ignores the status and parses the body regardless, so both + // arrive here as an object with no address and format into a blank result. + // The 404 body is matched on Javalin's own documentation URL rather than on + // the generic title/status pair, because that pair is RFC 7807's shape: a + // rate-limiting proxy in front of a healthy Nominatim answers + // {"title":"Too Many Requests","status":429}, and diagnosing that as "your + // provider is wrong" would turn a transient failure into advice to break a + // working configuration permanently. + const isPhoton = + raw.type === 'FeatureCollection' || + (typeof raw.type === 'string' && raw.type.includes('javalin.io')) || + (typeof raw.message === 'string' && + raw.message.includes('Unknown query parameter')) + + if (isPhoton) { + throw new Error( + `${url} answered as a Photon instance rather than a Nominatim one. Set "geocoderProvider": "photon" on this webhook, or point the URL at a Nominatim instance.`, + ) + } + + // Anything else object-shaped that carries no Nominatim result fields is an + // upstream error body, such as RFC 7807 problem JSON from a proxy in front + // of Nominatim. node-geocoder has already mapped it onto one blank entry by + // the time it gets here, and letting that through hands the caller a result + // with null coordinates -- Location.jsx pans the map to it. Rejecting keeps + // geocoder()'s fallback shape ([] or ''), and the message stays generic + // because this is a failing upstream, not a misconfigured provider. + // {"error": ...} bodies never reach this: node-geocoder converts those to a + // thrown Error itself. + // + // The test is on coordinate values, not key presence. RFC 7807 permits + // extension members, so {"title":"Too Many Requests","status":429, + // "address":null} would pass a presence check and still format into an entry + // with undefined coordinates. A genuine /reverse result always carries lat + // and lon as parseable strings (all 42 captured fixtures do), and a result + // node-geocoder cannot derive coordinates from is unusable to every caller + // regardless of what else it carries. + const isNominatimResult = + Number.isFinite(Number.parseFloat(raw.lat)) && + Number.isFinite(Number.parseFloat(raw.lon)) + if (!isNominatimResult) { + throw new Error( + `${url} answered with an error body instead of a geocoding result: ${JSON.stringify(raw).slice(0, 200)}`, + ) + } +} + /** * Nominatim, via node-geocoder's `openstreetmap` provider. * @param {string} url @@ -32,15 +125,36 @@ async function nominatimGeocoder(url, search, isReverse) { osmServer: url, timeout: 5000, }) - stockGeocoder._geocoder._formatResult = ((original) => (result) => ({ - ...original(result), - suburb: result.address.suburb || '', - town: result.address.town || '', - village: result.address.village || '', - }))(stockGeocoder._geocoder._formatResult) - return isReverse && typeof search === 'object' + stockGeocoder._geocoder._formatResult = ((original) => (result) => { + const formatted = original(result) + return { + ...formatted, + suburb: result.address?.suburb || '', + town: result.address?.town || '', + village: result.address?.village || '', + // node-geocoder emits the British spelling. The GraphQL schema exposes + // `neighborhood` and formatter() templates on `neighborhoods`, so the + // value reached neither consumer. Carrying the alias is what makes it + // visible without changing what node-geocoder itself produces. + // + // quarter is the fallback because Nominatim reports neighbourhood-level + // data under that key for many places: 1521 N Hoyne Ave carries + // quarter "Wicker Park" and no neighbourhood at all, and node-geocoder + // only reads address.neighbourhood, leaving the pair empty. + neighbourhood: formatted.neighbourhood || result.address?.quarter || '', + neighborhood: formatted.neighbourhood || result.address?.quarter || '', + } + })(stockGeocoder._geocoder._formatResult) + // Awaited rather than returned so the shape check runs here. A throw inside + // _formatResult would not reach geocoder()'s catch at all: node-geocoder + // resolves through bluebird's asCallback, so it surfaces as an uncaught + // exception and takes the process down. Anything thrown from this function + // rejects normally and is caught. + const results = await (isReverse && typeof search === 'object' ? stockGeocoder.reverse(search) - : stockGeocoder.geocode(String(search)) + : stockGeocoder.geocode(String(search))) + assertNominatimResponse(results, url) + return results } /** @@ -60,7 +174,22 @@ async function geocoder(nominatimUrl, search, reverse, format, provider) { // A coordinate pair means a reverse lookup. `reverse` separately controls // whether a single formatted string comes back, so the two are not // interchangeable. - const isReverse = typeof search === 'object' + // + // The null check is load-bearing: typeof null is 'object', so a null search + // would otherwise be taken for a coordinate pair and sent to /reverse with + // no lat or lon at all. + const isReverse = typeof search === 'object' && search !== null + + // Clearing the search box sends an empty string, and there is nothing to + // look up. Photon rejects it outright with + // {"message":"q parameter is required when no include categories are + // specified"} and a 400, which now surfaces as an error and fills the log + // with failures caused by a user deleting their own input. Answering + // directly costs nothing and keeps a blank box quiet. + if (!isReverse && !String(search ?? '').trim()) { + return emptyResult(reverse) + } + const results = provider === 'photon' ? await photonGeocoder(nominatimUrl, search, isReverse) @@ -68,16 +197,19 @@ async function geocoder(nominatimUrl, search, reverse, format, provider) { return reverse ? formatter(format, results[0]) : format - ? results.map((result) => ({ + ? // The mapped fields ride along rather than being rebuilt, because the + // Geocoder GraphQL type resolves neighborhood, suburb and the rest + // straight off this object. Returning only the formatted triple made + // every other field null whenever an addressFormat was configured. + results.map((result) => ({ + ...result, formatted: formatter(format, result), - latitude: result.latitude, - longitude: result.longitude, })) : results } catch (e) { log.warn(TAGS.geocoder, 'Unable to geocode for', search, e) - return {} + return emptyResult(reverse) } } -module.exports = { geocoder, formatter } +module.exports = { geocoder, formatter, nominatimGeocoder } diff --git a/server/src/services/photonGeocoder.js b/server/src/services/photonGeocoder.js index 2759ce9e4..1e7420ee5 100644 --- a/server/src/services/photonGeocoder.js +++ b/server/src/services/photonGeocoder.js @@ -1,5 +1,7 @@ // @ts-check +const { Response } = require('node-fetch') + const { fetchJson } = require('../utils/fetchJson') /** @@ -19,8 +21,13 @@ const { fetchJson } = require('../utils/fetchJson') * @property {string} [street] * @property {string} [postcode] * @property {string} [city] - * @property {string} [locality] A settlement below Photon's city layer, such - * as the hamlet a rural address sits in. Present when `city` is not. + * @property {string} [locality] The finest named area containing the result, + * equivalent to Nominatim's `neighbourhood` or `quarter`. Verified against a + * live index: Photon's `locality` for 1521 N Hoyne Ave is "Wicker Park", and + * Nominatim's `quarter` for the same building is "Wicker Park". + * @property {string} [district] The city subdivision containing the result, + * equivalent to Nominatim's `suburb` or `borough`. Same building: Photon's + * `district` is "West Town", Nominatim's `suburb` is "West Town". * @property {string} [county] * @property {string} [state] * @property {string} [country] @@ -62,6 +69,11 @@ const TYPE_TO_PROPERTY = { state: 'state', county: 'county', city: 'city', + // A search for a suburb by name comes back as type=district with the label in + // `name` and no `district` field, so without this the result being asked for + // is the one value missing from the answer. Live index, q=West Town Chicago: + // {name: "West Town", type: "district", osm_key: "place", osm_value: "suburb"} + district: 'district', locality: 'locality', street: 'street', } @@ -143,12 +155,19 @@ function preferBroader(parts) { function buildFormattedAddress(properties, settlement) { const street = joinComponents([properties.housenumber, properties.street]) + // locality and district sit between the street and the settlement, which is + // where Nominatim puts the levels they correspond to. For 1521 N Hoyne Ave it + // renders "1521, North Hoyne Avenue, Wicker Park, West Town, ...", matching + // the order of Nominatim's own quarter and suburb for that building. + // // The result's own name leads. When it was already echoed into a hierarchy // field, joinComponents drops the repeat rather than printing it twice. return joinComponents([ properties.name, ...preferBroader([ street, + properties.locality, + properties.district, settlement, properties.county, properties.state, @@ -196,31 +215,24 @@ function formatPhotonFeature(feature) { const town = named('town') const village = named('village') - // Photon's hierarchy runs city > district > locality > street, so an address - // whose containing settlement sits below the city layer carries that name in - // `locality` and has no `city` at all. Without it a rural address loses its - // settlement from both `city` and formattedAddress. Named `settlement` rather - // than `locality` so it is not confused with the Photon field it falls back - // to. // properties.city covers both what Photon sent and what resolveSelfReference // placed there from the type layer; named('city') covers a place=city tagged // result that carries no type at all. + // + // `locality` is deliberately not in this chain. It is neighbourhood-level + // rather than a smaller settlement: a live index returns locality "Wicker + // Park" alongside a city, and a genuine hamlet such as Bootjack arrives as + // osm_value=locality with its name in `name` and no `locality` field at all. + // Reporting it as the city would name a neighbourhood as a town. const settlement = - properties.city || - named('city') || - town || - village || - named('hamlet') || - properties.locality || - '' + properties.city || named('city') || town || village || named('hamlet') || '' return { latitude, longitude, formattedAddress: buildFormattedAddress(properties, settlement), country: properties.country, - // Mirrors _formatResult's own city/town/village/hamlet fallback, with - // Photon's locality layer appended to it. + // Mirrors _formatResult's own city/town/village/hamlet fallback. city: settlement || undefined, state: properties.state, zipcode: properties.postcode, @@ -231,13 +243,46 @@ function formatPhotonFeature(feature) { // Photon already sends this upper-case, which is what node-geocoder // produces after upper-casing Nominatim's lower-case value. countryCode: properties.countrycode, - neighbourhood: '', - suburb: '', + // Both verified against a live Photon index rather than inferred: for the + // same building, Photon's locality and district carry the values Nominatim + // returns as quarter and suburb. + neighbourhood: properties.locality || '', + // The same value under the spelling the GraphQL schema and formatter use. + // Geocoder.neighborhood is American and formatter() templates on + // neighborhoods, while node-geocoder emits neighbourhood, so without the + // alias the mapped value reaches no consumer at all. + neighborhood: properties.locality || '', + suburb: properties.district || '', town: town || '', village: village || '', } } +/** + * Recognises a response that came from Nominatim rather than Photon. + * + * An array is Nominatim's /search shape. A single object carrying result fields + * and no `features` is its /reverse shape. Both mean the URL and the configured + * provider disagree. + * + * The test is deliberately positive: it looks for fields a Nominatim result + * has, rather than treating anything without `features` as suspect. fetchJson + * hands back a Response object on a failed request, and Nominatim answers a + * miss with {"error":"Unable to geocode"} -- neither is a provider mismatch, + * and an empty result set is already the right outcome for both. + * @param {any} response + */ +function isNominatimResponse(response) { + if (Array.isArray(response)) return true + if (!response || typeof response !== 'object') return false + if ('features' in response) return false + return ( + 'address' in response || + 'display_name' in response || + 'place_id' in response + ) +} + /** * @param {string} photonUrl * @param {string | { lat: number, lon: number }} search @@ -256,7 +301,54 @@ async function photonGeocoder(photonUrl, search, isReverse) { limit: SEARCH_LIMIT, }) - const response = await fetchJson(url) + let response + try { + response = await fetchJson(url) + } catch (err) { + // fetchJson does not catch this itself: it does `return response.json()` + // inside its try block, so a body that fails to parse rejects the promise + // it already returned and escapes its local catch. + // + // This is the reverse half of the provider mismatch. Nominatim's /reverse + // defaults to XML and only sends JSON when asked, and Photon rejects any + // parameter outside its allow list with a 400, so asking would break every + // correctly configured Photon. Recognising the unparseable body instead + // costs nothing and needs no extra parameter. + throw new Error( + `${photonUrl} returned a body that is not JSON. Nominatim answers /reverse with XML unless asked otherwise, so this is usually a Nominatim URL on the Photon provider. Remove "geocoderProvider": "photon" from this webhook, or point the URL at a Photon instance. (${err instanceof Error ? err.message : err})`, + ) + } + // The mirror of the Nominatim check. Both of Nominatim's shapes have to be + // recognised: /search answers with an array, and /reverse answers with a + // single object. Checking only the array would let gym reverse geocoding + // fall through to an empty result set with no reason given, which is the + // path resolvers.js takes for every fort lookup. + // fetchJson hands back the Response itself when the request failed, so this + // covers every non-2xx. Photon serves /api and a Nominatim host has no such + // endpoint, so a forward search against a misconfigured webhook answers 404 + // and is reported here. + + if (response instanceof Response) { + // Only a 404 on Photon's own route is evidence about the provider: Photon + // serves /api, so a host without it is not a Photon instance. Any other + // status -- a 429 or 500 from a healthy but struggling Photon, a 401/403 + // from an authentication proxy in front of one -- is an operational + // failure, and advising the operator to remove geocoderProvider there + // prompts them to break a working configuration to cure a transient error. + if (response.status === 404) { + throw new Error( + `${photonUrl} answered 404 for Photon's /api endpoint, so it is not a Photon instance. Check the URL, or remove "geocoderProvider": "photon" from this webhook.`, + ) + } + throw new Error( + `${photonUrl} answered ${response.status}; the upstream geocoder is failing, not misconfigured.`, + ) + } + if (isNominatimResponse(response)) { + throw new Error( + `${photonUrl} answered in Nominatim's format rather than Photon's. Remove "geocoderProvider": "photon" from this webhook, or point the URL at a Photon instance.`, + ) + } // fetchJson answers a failed request with the Response rather than throwing, // so an absent features array covers both a network failure and an empty // result set. diff --git a/server/test/geocoder.test.js b/server/test/geocoder.test.js index 797093151..9ff39d809 100644 --- a/server/test/geocoder.test.js +++ b/server/test/geocoder.test.js @@ -3,10 +3,21 @@ const { test } = require('node:test') const NodeGeocoder = require('node-geocoder') -const { PoracleAPI } = require('../src/services/Poracle') +const http = require('node:http') + +const { + PoracleAPI, + resolveGeocoderProvider, +} = require('../src/services/Poracle') +const { + formatter, + geocoder, + nominatimGeocoder, +} = require('../src/services/geocoder') const { formatPhotonFeature, joinComponents, + photonGeocoder, preferBroader, } = require('../src/services/photonGeocoder') @@ -55,6 +66,7 @@ test('maps a Photon city onto the geocoder entry shape', () => { streetNumber: undefined, countryCode: 'US', neighbourhood: '', + neighborhood: '', suburb: '', town: '', village: '', @@ -133,56 +145,74 @@ test('echoes the result name into its locality field', () => { // sends no `city` at all. Reading only `city` drops the settlement from both // the entry and the formatted address, so a {{city}} format renders blank for // every rural address. -test('falls back to Photon locality when there is no city', () => { +// Captured verbatim from a live Photon index: reverse at 41.9088,-87.6796. +// Nominatim's own answer for the same building carries quarter "Wicker Park" +// and suburb "West Town", which is what fixes these two mappings. +const WICKER_PARK_REVERSE = { + city: 'West Chicago Township', + country: 'United States', + countrycode: 'US', + county: 'Cook County', + district: 'West Town', + housenumber: '1521', + locality: 'Wicker Park', + osm_key: 'building', + osm_value: 'yes', + postcode: '60647', + state: 'Illinois', + street: 'North Hoyne Avenue', + type: 'house', +} + +test("maps Photon's locality to neighbourhood and district to suburb", () => { const got = formatPhotonFeature( - feature( - { - housenumber: '4', - street: 'County Road 15', - locality: 'Bootjack', - county: 'Mariposa County', - state: 'California', - postcode: '95338', - country: 'United States', - countrycode: 'US', - osm_key: 'building', - osm_value: 'yes', - }, - [-119.9515, 37.4744], - ), + feature(WICKER_PARK_REVERSE, [-87.6796, 41.9088]), + ) + assert.equal(got.neighbourhood, 'Wicker Park') + assert.equal(got.suburb, 'West Town') + // Neither is the settlement. Reporting a neighbourhood as the city would name + // an area of Chicago as a town. + assert.equal(got.city, 'West Chicago Township') +}) + +test('places locality and district between the street and the settlement', () => { + const got = formatPhotonFeature( + feature(WICKER_PARK_REVERSE, [-87.6796, 41.9088]), ) - assert.equal(got.city, 'Bootjack') assert.equal( got.formattedAddress, - '4, County Road 15, Bootjack, Mariposa County, California, 95338, United States', + '1521, North Hoyne Avenue, Wicker Park, West Town, West Chicago Township, Cook County, Illinois, 60647, United States', ) }) -// Photon's own hierarchy puts city above locality, so a response carrying both -// must not demote the city. -test('prefers city over locality when Photon sends both', () => { +// A hamlet does not arrive in `locality`. Live index, q=Bootjack California: +// osm_value=locality, type=other, name carries the hamlet, no locality field. +test('a place tagged as a locality keeps its name out of the address fields', () => { const got = formatPhotonFeature( feature({ - city: 'Mariposa', - locality: 'Bootjack', + name: 'Bootjack', + type: 'other', + osm_key: 'place', + osm_value: 'locality', + county: 'Mariposa', state: 'California', country: 'United States', countrycode: 'US', }), ) - assert.equal(got.city, 'Mariposa') + assert.equal(got.neighbourhood, '') + assert.equal( + got.formattedAddress, + 'Bootjack, Mariposa, California, United States', + ) }) -// Photon reports the result's own layer in properties.type, and a city, state -// or country is usually an administrative boundary in OSM, arriving as -// osm_key=boundary. Gating self-reference on osm_key=place alone would keep -// only the exception and drop the common case. test('places the result name using the Photon type layer', () => { const cases = [ { type: 'city', name: 'Denver', field: 'city' }, { type: 'state', name: 'Illinois', field: 'state' }, { type: 'country', name: 'United States', field: 'country' }, - { type: 'locality', name: 'Bootjack', field: 'city' }, + { type: 'locality', name: 'Wicker Park', field: 'neighbourhood' }, { type: 'street', name: 'Lake Shore Drive', field: 'streetName' }, ] cases.forEach(({ type, name, field }) => { @@ -292,7 +322,7 @@ test("Photon's own city wins over the echoed name", () => { // Photon's nearest field is `district`, a different OSM concept. Equating them // would be an invention rather than a translation. -test('leaves suburb and neighbourhood empty', () => { +test('leaves suburb and neighbourhood empty when Photon sends neither', () => { const got = formatPhotonFeature(STREET_ADDRESS) assert.equal(got.suburb, '') assert.equal(got.neighbourhood, '') @@ -352,12 +382,18 @@ test('the Photon path emits the same keys as the Nominatim path', () => { osmServer: 'http://127.0.0.1:0', timeout: 5000, }) - stockGeocoder._geocoder._formatResult = ((original) => (result) => ({ - ...original(result), - suburb: result.address.suburb || '', - town: result.address.town || '', - village: result.address.village || '', - }))(stockGeocoder._geocoder._formatResult.bind(stockGeocoder._geocoder)) + // Mirrors the patch in geocoder.js, alias included. The duplication is the + // point: if the two drift apart, this test stops proving parity. + stockGeocoder._geocoder._formatResult = ((original) => (result) => { + const formatted = original(result) + return { + ...formatted, + suburb: result.address?.suburb || '', + town: result.address?.town || '', + village: result.address?.village || '', + neighborhood: formatted.neighbourhood || '', + } + })(stockGeocoder._geocoder._formatResult.bind(stockGeocoder._geocoder)) // The same place as STREET_ADDRESS, in Nominatim's response shape. const fromNominatim = stockGeocoder._geocoder._formatResult({ @@ -420,3 +456,863 @@ test('PoracleAPI leaves the provider undefined when it is not configured', () => assert.equal(api.geocoderProvider, undefined) assert.equal(api.nominatimUrl, 'http://127.0.0.1:2322') }) + +// A misconfigured webhook -- a Photon URL left on the Nominatim provider, or the +// reverse -- used to reach node-geocoder, which reads a GeoJSON object as a +// single result and hands the whole FeatureCollection to _formatResult. The +// unguarded address lookup in ReactMap's patch then threw, and because +// node-geocoder resolves through bluebird's asCallback the throw never reached +// geocoder()'s catch: it surfaced as an uncaught exception and killed the +// process. +const serveOnce = async (body, status = 200) => { + const server = http.createServer((_, res) => { + res.writeHead(status, { 'Content-Type': 'application/json' }) + res.end(typeof body === 'string' ? body : JSON.stringify(body)) + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + return { + url: `http://127.0.0.1:${server.address().port}`, + close: () => + new Promise((resolve) => { + server.close(resolve) + }), + } +} + +const PHOTON_BODY = { + type: 'FeatureCollection', + features: [ + { + geometry: { type: 'Point', coordinates: [-104.9903, 39.7392] }, + properties: { name: 'Denver', type: 'city', countrycode: 'US' }, + }, + ], +} + +const NOMINATIM_BODY = [ + { + lat: '39.7392', + lon: '-104.9903', + display_name: 'Denver, Colorado, United States', + address: { city: 'Denver', state: 'Colorado', country_code: 'us' }, + }, +] + +test('a Photon URL on the Nominatim provider fails without crashing', async () => { + const server = await serveOnce(PHOTON_BODY) + try { + // geocoder() catches and returns {}. What matters is that the process + // survives to get here at all. + const result = await geocoder(server.url, 'Denver', false, '{{city}}') + assert.deepEqual(result, []) + } finally { + await server.close() + } +}) + +test('a Nominatim URL on the Photon provider fails without returning nothing silently', async () => { + const server = await serveOnce(NOMINATIM_BODY) + try { + const result = await geocoder( + server.url, + 'Denver', + false, + '{{city}}', + 'photon', + ) + assert.deepEqual(result, []) + } finally { + await server.close() + } +}) + +// The matched pairs still work, so the checks above are not rejecting valid +// responses. +test('a correctly configured Photon webhook still geocodes', async () => { + const server = await serveOnce(PHOTON_BODY) + try { + const result = await geocoder( + server.url, + 'Denver', + false, + '{{city}}', + 'photon', + ) + assert.equal(result.length, 1) + assert.equal(result[0].formatted, 'Denver') + assert.equal(result[0].latitude, 39.7392) + assert.equal(result[0].longitude, -104.9903) + // The mapped fields ride along with formatted: Geocoder resolves + // neighborhood, suburb and the rest straight off this object. + assert.equal(result[0].city, 'Denver') + } finally { + await server.close() + } +}) + +test('a correctly configured Nominatim webhook still geocodes', async () => { + const server = await serveOnce(NOMINATIM_BODY) + try { + const result = await geocoder(server.url, 'Denver', false, '{{city}}') + assert.equal(result.length, 1) + assert.equal(result[0].formatted, 'Denver') + assert.equal(result[0].latitude, 39.7392) + assert.equal(result[0].longitude, -104.9903) + assert.equal(result[0].city, 'Denver') + } finally { + await server.close() + } +}) + +// Nominatim answers /reverse with a single object rather than the array /search +// returns, and gym reverse geocoding is the path resolvers.js takes for every +// fort lookup. Checking only for an array let that mismatch through as an empty +// result set with no reason given. +const NOMINATIM_REVERSE_BODY = { + place_id: 315787599, + lat: '39.7392', + lon: '-104.9903', + display_name: '1437, Bannock Street, Denver, Colorado, 80202, United States', + address: { + house_number: '1437', + road: 'Bannock Street', + city: 'Denver', + country_code: 'us', + }, +} + +// Asserted against photonGeocoder rather than geocoder(), deliberately. +// geocoder() catches everything and returns {}, so a mismatch and a plain miss +// look identical from there -- the whole point of this change is which error +// reaches the log, and only the inner function exposes that. +// Behaves the way a real Nominatim host does, rather than answering the same +// JSON on every path. That distinction matters: a stub that always returns +// Nominatim JSON made these checks look like they worked when they did not. +// +// /api Photon's endpoint. Nominatim has none, so it answers 404. +// /reverse defaults to XML unless format=json is asked for. +// /search the array shape. +const serveNominatim = async () => { + const server = http.createServer((req, res) => { + const url = new URL(req.url, 'http://127.0.0.1') + const json = (code, body) => { + res.writeHead(code, { 'Content-Type': 'application/json' }) + res.end(JSON.stringify(body)) + } + if (url.pathname === '/reverse') { + if (url.searchParams.get('format') !== 'json') { + res.writeHead(200, { 'Content-Type': 'text/xml' }) + res.end('') + return + } + json(200, NOMINATIM_REVERSE_BODY) + return + } + if (url.pathname === '/search') { + json(200, NOMINATIM_BODY) + return + } + json(404, { title: '404 Not Found' }) + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + return { + url: `http://127.0.0.1:${server.address().port}`, + close: () => + new Promise((resolve) => { + server.close(resolve) + }), + } +} + +// Documents a limitation rather than a behaviour. Nominatim's /reverse answers +// XML unless format=json is asked for, and Photon rejects any parameter outside +// its allow list with a 400, so asking would break every correctly configured +// Photon. The body therefore fails to parse and is logged as a failed fetch. +// The forward path below is where this misconfiguration is actually reported. +// Nominatim's /reverse answers XML unless format=json is asked for, and Photon +// rejects any parameter outside its allow list with a 400, so asking would +// break every correctly configured Photon. The unparseable body is the signal +// instead. +test('a Nominatim reverse response on the Photon provider raises a provider error', async () => { + const server = await serveNominatim() + try { + await assert.rejects( + () => photonGeocoder(server.url, { lat: 39.7392, lon: -104.9903 }, true), + /not a Photon instance|Photon provider|answers \/reverse with XML/, + ) + } finally { + await server.close() + } +}) + +// The forward half of the same misconfiguration. Nominatim has no /api, so it +// answers 404 rather than a readable body, and the previous check could never +// have seen a Nominatim search array here. +test('a Nominatim host on the Photon provider raises an error on the forward path', async () => { + const server = await serveNominatim() + try { + await assert.rejects( + () => photonGeocoder(server.url, 'Denver', false), + /not a Photon instance/, + ) + } finally { + await server.close() + } +}) + +// The classifier only ever sees a JSON body because the request asks for one. +// Without format=json this endpoint answers XML, fetchJson cannot parse it, and +// the mismatch goes unreported. +// The request must carry nothing outside Photon's allow list. A live instance +// answers 400 to any unknown parameter, so an extra one added to help diagnose +// a misconfiguration would break every correctly configured deployment. +test('the outgoing requests send only parameters Photon accepts', async () => { + const PHOTON_PARAMS = new Set([ + 'include', + 'debug', + 'dedupe', + 'query_string_filter', + 'lon', + 'layer', + 'limit', + 'osm_tag', + 'distance_sort', + 'geometry', + 'exclude', + 'lang', + 'radius', + 'lat', + 'q', + ]) + const seen = [] + const server = http.createServer((req, res) => { + seen.push(req.url) + res.writeHead(200, { 'Content-Type': 'application/json' }) + res.end(JSON.stringify({ type: 'FeatureCollection', features: [] })) + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + const url = `http://127.0.0.1:${server.address().port}` + try { + await photonGeocoder(url, 'Denver', false) + await photonGeocoder(url, { lat: 39.7392, lon: -104.9903 }, true) + seen.forEach((requested) => { + const params = new URL(requested, 'http://127.0.0.1').searchParams + params.forEach((_, key) => { + assert.ok( + PHOTON_PARAMS.has(key), + `${key} is not a parameter Photon accepts (${requested})`, + ) + }) + }) + } finally { + await new Promise((resolve) => { + server.close(resolve) + }) + } +}) + +// A JSON array is still Nominatim's /search shape, whatever served it. +test('a Nominatim search array on the Photon provider raises a provider error', async () => { + const server = await serveOnce(NOMINATIM_BODY) + try { + await assert.rejects( + () => photonGeocoder(server.url, 'Denver', false), + /Nominatim's format/, + ) + } finally { + await server.close() + } +}) + +// A miss is not a mismatch. An unmatched reverse lookup carries no result +// fields, and an empty result is already the right outcome, so it must not be +// reported as a provider error. +test('a no-match reverse response is not mistaken for a provider mismatch', async () => { + const server = await serveOnce({ error: 'Unable to geocode' }) + try { + const results = await photonGeocoder(server.url, { lat: 0, lon: 0 }, true) + assert.deepEqual(results, []) + } finally { + await server.close() + } +}) + +// The matched pair still works on the reverse path. +test('a correctly configured Photon webhook still reverse geocodes', async () => { + const server = await serveOnce({ + type: 'FeatureCollection', + features: [ + { + geometry: { type: 'Point', coordinates: [-104.9903, 39.7392] }, + properties: { city: 'Denver', state: 'Colorado', countrycode: 'US' }, + }, + ], + }) + try { + const result = await geocoder( + server.url, + { lat: 39.7392, lon: -104.9903 }, + true, + '{{city}}', + 'photon', + ) + assert.equal(result, 'Denver') + } finally { + await server.close() + } +}) + +// Captured from a live index, q=West Town Chicago. A suburb searched by name +// comes back as its own result: the label is in `name` and the containing +// `district` field is absent, so without a district entry in the type map the +// one value being asked for is the one missing from the answer. +test('a district searched by name populates suburb', () => { + const got = formatPhotonFeature( + feature( + { + city: 'West Chicago Township', + country: 'United States', + countrycode: 'US', + county: 'Cook County', + name: 'West Town', + osm_key: 'place', + osm_value: 'suburb', + state: 'Illinois', + type: 'district', + }, + [-87.6796, 41.9088], + ), + ) + assert.equal(got.suburb, 'West Town') + assert.equal( + got.formattedAddress, + 'West Town, West Chicago Township, Cook County, Illinois, United States', + ) +}) + +// Query.geocoder is [Geocoder] and Gym.formatted is String. An object fallback +// makes GraphQL discard the field entirely rather than degrade to an empty +// answer, so a transient upstream failure has to keep the caller's shape. +test('a transient upstream failure keeps the shape the schema expects', async () => { + const failing = http.createServer((_, res) => { + res.writeHead(500, { 'Content-Type': 'application/json' }) + res.end(JSON.stringify({ message: 'overloaded' })) + }) + await new Promise((resolve) => { + failing.listen(0, '127.0.0.1', resolve) + }) + const url = `http://127.0.0.1:${failing.address().port}` + try { + const forward = await geocoder(url, 'Denver', false, '{{city}}', 'photon') + assert.ok(Array.isArray(forward), `forward returned ${typeof forward}`) + assert.deepEqual(forward, []) + + const reverse = await geocoder( + url, + { lat: 39.7392, lon: -104.9903 }, + true, + '{{city}}', + 'photon', + ) + assert.equal(typeof reverse, 'string', 'reverse must stay a String') + assert.equal(reverse, '') + } finally { + await new Promise((resolve) => { + failing.close(resolve) + }) + } +}) + +// Behaves like a real Photon instance rather than accepting anything: it serves +// /api and /reverse only, and rejects any parameter outside its allow list. +// node-geocoder forces format and addressdetails onto every request and calls +// /search, so a Photon URL left on the Nominatim provider never reaches a +// FeatureCollection at all. Captured from a live instance. +const servePhoton = async () => { + const ALLOWED = new Set([ + 'include', + 'debug', + 'dedupe', + 'query_string_filter', + 'lon', + 'layer', + 'limit', + 'osm_tag', + 'distance_sort', + 'geometry', + 'exclude', + 'lang', + 'radius', + 'lat', + 'q', + ]) + const server = http.createServer((req, res) => { + const url = new URL(req.url, 'http://127.0.0.1') + const json = (code, body) => { + res.writeHead(code, { 'Content-Type': 'application/json' }) + res.end(JSON.stringify(body)) + } + if (url.pathname !== '/api' && url.pathname !== '/reverse') { + json(404, { + title: `Endpoint ${req.method} ${url.pathname} not found`, + status: 404, + type: 'https://javalin.io/documentation#endpointnotfound', + details: {}, + }) + return + } + const bad = [...url.searchParams.keys()].find((k) => !ALLOWED.has(k)) + if (bad) { + json(400, { + message: `Unknown query parameter '${bad}'. Allowed parameters are: [${[...ALLOWED].join(', ')}]`, + }) + return + } + json(200, { type: 'FeatureCollection', features: [] }) + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + return { + url: `http://127.0.0.1:${server.address().port}`, + close: () => + new Promise((resolve) => { + server.close(resolve) + }), + } +} + +// The exact production misconfiguration: a Photon URL inherited from Poracle +// with no geocoderProvider set. It used to format into a single blank result. +test('a Photon URL on the Nominatim provider is reported on the forward path', async () => { + const server = await servePhoton() + try { + await assert.rejects( + () => nominatimGeocoder(server.url, 'Denver', false), + /answered as a Photon instance/, + ) + } finally { + await server.close() + } +}) + +test('a Photon URL on the Nominatim provider is reported on the reverse path', async () => { + const server = await servePhoton() + try { + await assert.rejects( + () => + nominatimGeocoder(server.url, { lat: 39.7392, lon: -104.9903 }, true), + /answered as a Photon instance/, + ) + } finally { + await server.close() + } +}) + +// The value has to reach the two consumers that actually exist: the GraphQL +// field is `neighborhood` and formatter() templates on `neighborhoods`, while +// node-geocoder produces `neighbourhood`. +test('locality reaches the public neighborhood contract', () => { + const got = formatPhotonFeature( + feature(WICKER_PARK_REVERSE, [-87.6796, 41.9088]), + ) + assert.equal(got.neighbourhood, 'Wicker Park') + assert.equal(got.neighborhood, 'Wicker Park') + assert.equal(formatter('{{neighborhood}}', got), 'Wicker Park') + assert.equal(formatter('{{neighbourhood}}', got), 'Wicker Park') +}) + +// The alias has to exist on the Nominatim path too. node-geocoder emits +// neighbourhood there, and the GraphQL field and formatter token both use the +// American spellings, so a Nominatim deployment had the same invisible value. +test('the Nominatim path also carries the neighborhood alias', async () => { + const server = http.createServer((_, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }) + res.end( + JSON.stringify([ + { + lat: '41.9088', + lon: '-87.6796', + display_name: 'Wicker Park, Chicago', + address: { + neighbourhood: 'Wicker Park', + city: 'Chicago', + country_code: 'us', + }, + }, + ]), + ) + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + try { + const [entry] = await nominatimGeocoder( + `http://127.0.0.1:${server.address().port}`, + 'Wicker Park', + false, + ) + assert.equal(entry.neighbourhood, 'Wicker Park') + assert.equal(entry.neighborhood, 'Wicker Park') + assert.equal(formatter('{{neighborhood}}', entry), 'Wicker Park') + } finally { + await new Promise((resolve) => { + server.close(resolve) + }) + } +}) + +// Clearing the search box sends an empty string. Photon rejects it with a 400, +// which surfaces as an error, so a user deleting their own input would fill the +// log with failures. Nothing should leave the process at all. +test('an empty search never reaches the upstream', async () => { + const seen = [] + const server = http.createServer((req, res) => { + seen.push(req.url) + res.writeHead(200, { 'Content-Type': 'application/json' }) + res.end(JSON.stringify({ type: 'FeatureCollection', features: [] })) + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + const url = `http://127.0.0.1:${server.address().port}` + try { + const blank = ['', ' ', '\t\n', undefined, null] + // eslint-disable-next-line no-restricted-syntax + await Promise.all( + blank.map(async (search) => { + const photon = await geocoder(url, search, false, '{{city}}', 'photon') + assert.deepEqual( + photon, + [], + `photon returned a result for ${JSON.stringify(search)}`, + ) + const nominatim = await geocoder(url, search, false, '{{city}}') + assert.deepEqual( + nominatim, + [], + `nominatim returned a result for ${JSON.stringify(search)}`, + ) + }), + ) + assert.deepEqual(seen, [], `requests were sent: ${seen.join(', ')}`) + + // A real query still goes out, so the guard is not swallowing everything. + await geocoder(url, 'Denver', false, '{{city}}', 'photon') + assert.equal(seen.length, 1) + } finally { + await new Promise((resolve) => { + server.close(resolve) + }) + } +}) + +// A reverse lookup at 0,0 is a real coordinate pair, not an empty search. +test('the empty guard does not swallow a zero coordinate', async () => { + const seen = [] + const server = http.createServer((req, res) => { + seen.push(req.url) + res.writeHead(200, { 'Content-Type': 'application/json' }) + res.end(JSON.stringify({ type: 'FeatureCollection', features: [] })) + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + try { + await geocoder( + `http://127.0.0.1:${server.address().port}`, + { lat: 0, lon: 0 }, + true, + '{{city}}', + 'photon', + ) + assert.equal(seen.length, 1, 'a reverse lookup at 0,0 must still be sent') + assert.match(seen[0], /lat=0/) + } finally { + await new Promise((resolve) => { + server.close(resolve) + }) + } +}) + +// An RFC 7807 problem body is not a Photon signature. A rate-limiting proxy in +// front of a healthy Nominatim answers {"title":"Too Many Requests","status": +// 429}, and diagnosing that as a provider mismatch advises the operator to +// break a working configuration to fix a transient failure. +test('an RFC 7807 error from a Nominatim proxy is not diagnosed as Photon', async () => { + const server = await serveOnce({ title: 'Too Many Requests', status: 429 }) + try { + // A problem body rejects with a generic upstream error. It must not carry + // the provider-mismatch advice, which would tell the operator to change + // geocoderProvider to cure a transient 429, and it must not fall through as + // a blank entry either: node-geocoder maps the object onto one result with + // undefined coordinates, and Location.jsx pans the map to [null, null]. + await assert.rejects( + () => nominatimGeocoder(server.url, 'Denver', false), + (err) => /error body/.test(err.message) && !/Photon/.test(err.message), + ) + + // Through the public entry point the caller gets the schema's empty shape, + // never a result with null coordinates. + const viaGeocoder = await geocoder(server.url, 'Denver', false, '{{city}}') + assert.deepEqual(viaGeocoder, []) + } finally { + await server.close() + } +}) + +// The Javalin signatures still fire, so narrowing did not disable the check. +test('the Javalin 404 body is still diagnosed as Photon', async () => { + const server = await serveOnce({ + title: 'Endpoint GET /search not found', + status: 404, + type: 'https://javalin.io/documentation#endpointnotfound', + }) + try { + await assert.rejects( + () => nominatimGeocoder(server.url, 'Denver', false), + /Photon instance/, + ) + } finally { + await server.close() + } +}) + +// Nominatim reports neighbourhood-level data under `quarter` for many places: +// 1521 N Hoyne Ave carries quarter "Wicker Park" and no neighbourhood key. +// node-geocoder only reads address.neighbourhood, so both spellings came out +// empty for exactly the responses the alias exists to surface. +test('a quarter-shaped Nominatim response populates both neighborhood spellings', async () => { + const server = await serveOnce([ + { + lat: '41.9088', + lon: '-87.6796', + display_name: + '1521, North Hoyne Avenue, Wicker Park, West Town, Chicago, Illinois, 60622, United States', + address: { + house_number: '1521', + road: 'North Hoyne Avenue', + quarter: 'Wicker Park', + suburb: 'West Town', + city: 'Chicago', + state: 'Illinois', + postcode: '60622', + country: 'United States', + country_code: 'us', + }, + }, + ]) + try { + const results = await nominatimGeocoder(server.url, 'Denver', false) + assert.equal(results[0].neighbourhood, 'Wicker Park') + assert.equal(results[0].neighborhood, 'Wicker Park') + } finally { + await server.close() + } +}) + +// When an addressFormat is configured, the resolver's list still resolves the +// Geocoder fields off these objects. Returning only the formatted triple made +// neighborhood, suburb and the rest null for every formatted call. +test('a formatted result keeps the mapped fields for the Geocoder type', async () => { + const server = await serveOnce({ + type: 'FeatureCollection', + features: [ + { + geometry: { type: 'Point', coordinates: [-87.6796, 41.9088] }, + properties: { + housenumber: '1521', + street: 'North Hoyne Avenue', + locality: 'Wicker Park', + district: 'West Town', + city: 'Chicago', + state: 'Illinois', + postcode: '60622', + country: 'United States', + countrycode: 'US', + osm_key: 'building', + osm_value: 'yes', + type: 'house', + }, + }, + ], + }) + try { + const results = await geocoder( + server.url, + '1521 N Hoyne', + false, + '{{city}}', + 'photon', + ) + assert.equal(results[0].formatted, 'Chicago') + assert.equal(results[0].neighborhood, 'Wicker Park') + assert.equal(results[0].suburb, 'West Town') + assert.equal(results[0].streetNumber, '1521') + } finally { + await server.close() + } +}) + +// Poracle reports the backend behind its providerURL as `provider`, so a +// deployment whose geocoder URL comes from Poracle no longer has to restate the +// backend type in ReactMap's own config. Local config still wins, matching how +// providerURL and addressFormat already behave. +test('derives the geocoder provider from Poracle when it is not set locally', () => { + const args = { local: undefined, usingRemoteURL: true } + assert.equal(resolveGeocoderProvider({ ...args, remote: 'photon' }), 'photon') + assert.equal( + resolveGeocoderProvider({ ...args, remote: 'nominatim' }), + 'nominatim', + ) +}) + +test('local configuration beats what Poracle reports', () => { + assert.equal( + resolveGeocoderProvider({ + remote: 'nominatim', + local: 'photon', + usingRemoteURL: true, + }), + 'photon', + ) + // Explicit local config wins even when the URL is local too, which is the + // only way to run a backend Poracle does not know about. + assert.equal( + resolveGeocoderProvider({ + remote: 'nominatim', + local: 'photon', + usingRemoteURL: false, + }), + 'photon', + ) +}) + +// The URL and the protocol used to talk to it are one setting in two fields. +// Inheriting the type while keeping a local URL is how a working Nominatim +// deployment ends up being addressed as Photon: local nominatimUrl, no +// geocoderProvider, and a Photon-backed Poracle supplying the type. +test('never inherits the provider without the URL that goes with it', () => { + assert.equal( + resolveGeocoderProvider({ + remote: 'photon', + local: undefined, + usingRemoteURL: false, + }), + undefined, + ) +}) + +// Poracle supports google and none. Neither has a ReactMap equivalent, and +// mapping them onto nominatim would claim something untrue about the backend. +test('ignores Poracle backends ReactMap cannot drive', () => { + const unusable = ['google', 'none', '', undefined, 'PHOTON'] + unusable.forEach((remote) => { + assert.equal( + resolveGeocoderProvider({ + remote, + local: undefined, + usingRemoteURL: true, + }), + undefined, + `${remote} should not resolve to a provider`, + ) + }) +}) + +// The reason `provider` is destructured out of remoteConfig rather than left to +// the spread: this.provider is the webhook provider, an unrelated field that +// happens to share the name. +test('a Poracle geocoding provider never overwrites the webhook provider', () => { + const api = new PoracleAPI({ + name: 'test', + host: 'http://127.0.0.1', + port: 3030, + provider: 'poracle', + }) + assert.equal(api.provider, 'poracle') + + // Mirrors what #fetchConfig does with a remote payload. + const remoteConfig = { provider: 'photon', providerURL: 'http://photon:2322' } + const { provider, ...rest } = remoteConfig + Object.assign(api, rest) + api.geocoderProvider = resolveGeocoderProvider({ + remote: provider, + local: api.geocoderProvider, + usingRemoteURL: true, + }) + + assert.equal(api.provider, 'poracle') + assert.equal(api.geocoderProvider, 'photon') +}) + +// Only a 404 on /api is evidence about the provider. Any other status is an +// operational failure from a correctly configured Photon (429, 500) or a proxy +// in front of one (401, 403), and advice to remove geocoderProvider there +// prompts the operator to break a working configuration. +test('a transient Photon failure is not blamed on configuration', async () => { + const statuses = [429, 500, 401, 403] + // eslint-disable-next-line no-restricted-syntax + for (const status of statuses) { + // eslint-disable-next-line no-await-in-loop + const server = await serveOnce('', status) + try { + // eslint-disable-next-line no-await-in-loop + await assert.rejects( + () => photonGeocoder(server.url, 'Denver', false), + (err) => + !/geocoderProvider/.test(err.message) && + new RegExp(String(status)).test(err.message), + `a ${status} must read as an upstream failure, not provider advice`, + ) + } finally { + // eslint-disable-next-line no-await-in-loop + await server.close() + } + } +}) + +// RFC 7807 permits extension members, so a presence check on address or lat is +// defeatable: this body carries an address key and still formats into an entry +// with undefined coordinates. The classifier validates coordinate values. +test('an extended problem body does not pass as a Nominatim result', async () => { + const server = await serveOnce({ + title: 'Too Many Requests', + status: 429, + address: null, + }) + try { + await assert.rejects( + () => nominatimGeocoder(server.url, 'Denver', false), + (err) => /error body/.test(err.message) && !/Photon/.test(err.message), + ) + const viaGeocoder = await geocoder(server.url, 'Denver', false, '{{city}}') + assert.deepEqual(viaGeocoder, []) + } finally { + await server.close() + } +}) + +// The coordinate validation must not reject what it exists to protect: a +// genuine reverse result, whose lat and lon are parseable strings. +test('a genuine Nominatim reverse result passes the coordinate validation', async () => { + const server = await serveOnce(NOMINATIM_REVERSE_BODY) + try { + const results = await nominatimGeocoder( + server.url, + { lat: 39.7392, lon: -104.9903 }, + true, + ) + assert.equal(results.length, 1) + assert.equal(results[0].latitude, 39.7392) + assert.equal(results[0].city, 'Denver') + } finally { + await server.close() + } +})