From e0700d2b3a85427a444250a41b47d1aaa1f57ad5 Mon Sep 17 00:00:00 2001 From: Rin <58572875+TurtIeSocks@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:36:37 -0400 Subject: [PATCH 1/4] feat(geocoder): add native Photon provider Photon speaks GeoJSON rather than Nominatim's JSON, so it cannot be driven through node-geocoder's openstreetmap provider. This adds a small module that talks to Photon directly and returns entries in the same shape that provider produces, so formatter and the webhook resolvers cannot tell which backend answered. Selected per webhook with geocoderProvider, which defaults to nominatim. The existing Nominatim path is unchanged in behaviour; it moves into its own function so the two branches read side by side. nominatimUrl keeps its name and holds the base URL for either provider, so no existing config needs editing. Two parts of the mapping are not mechanical. Photon reports a result's own label only in properties.name and uses the hierarchy fields purely for what contains the result, while Nominatim echoes that name into the matching address field. Without reproducing the echo, searching for a city returns a result with no city in it. osm_key and osm_value decide which field the name belongs in, and a value Photon already supplied always wins. formattedAddress is composed rather than read, since Photon has no display_name. Components are joined most specific first, absent parts are skipped, and a component is never repeated: a postcode search puts the same value in both the name and the postcode field, and Nominatim renders it once. Where a component recurs further down the hierarchy the broader one is kept, so a city sharing its state's name does not cost the address line its state. address.suburb and address.neighbourhood are always empty. Photon's nearest field is district, which is a different OSM concept, and equating them would be an invention rather than a translation. The tests cover the mapping and assert that both providers emit the same entry for the same address, using node-geocoder's own _formatResult with the same patch geocoder.js applies. --- packages/types/lib/config.d.ts | 7 + server/src/graphql/resolvers.js | 2 + server/src/services/geocoder.js | 53 ++++-- server/src/services/photonGeocoder.js | 209 +++++++++++++++++++++++ server/test/geocoder.test.js | 228 ++++++++++++++++++++++++++ 5 files changed, 482 insertions(+), 17 deletions(-) create mode 100644 server/src/services/photonGeocoder.js create mode 100644 server/test/geocoder.test.js diff --git a/packages/types/lib/config.d.ts b/packages/types/lib/config.d.ts index 4c098138e..8355a9be6 100644 --- a/packages/types/lib/config.d.ts +++ b/packages/types/lib/config.d.ts @@ -199,7 +199,14 @@ export interface Webhook { port: number poracleSecret: string addressFormat?: string + /** Base URL of the geocoding backend. Used for both providers. */ nominatimUrl?: string + /** + * 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`. + */ + geocoderProvider?: 'nominatim' | 'photon' trialPeriodEligible?: boolean areasToSkip: string[] discordRoles: [] diff --git a/server/src/graphql/resolvers.js b/server/src/graphql/resolvers.js index 50efb5235..58f80e613 100644 --- a/server/src/graphql/resolvers.js +++ b/server/src/graphql/resolvers.js @@ -197,6 +197,7 @@ const resolvers = { search, false, webhook.addressFormat, + webhook.geocoderProvider, ) } } @@ -465,6 +466,7 @@ const resolvers = { { lat: result.lat, lon: result.lon }, true, webhook.addressFormat, + webhook.geocoderProvider, ), })), ) diff --git a/server/src/services/geocoder.js b/server/src/services/geocoder.js index 1b15d3dc6..94e32a114 100644 --- a/server/src/services/geocoder.js +++ b/server/src/services/geocoder.js @@ -3,6 +3,8 @@ const NodeGeocoder = require('node-geocoder') const { log, TAGS } = require('@rm/logger') +const { photonGeocoder } = require('./photonGeocoder') + /** * @param {string} addressFormat * @param {NodeGeocoder.Entry} result @@ -18,34 +20,51 @@ function formatter(addressFormat, result) { .trim() } +/** + * Nominatim, via node-geocoder's `openstreetmap` provider. + * @param {string} url + * @param {string | { lat: number, lon: number }} search + * @param {boolean} isReverse + */ +async function nominatimGeocoder(url, search, isReverse) { + const stockGeocoder = NodeGeocoder({ + provider: 'openstreetmap', + 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.reverse(search) + : stockGeocoder.geocode(String(search)) +} + /** * @template {boolean} T - * @param {string} nominatimUrl + * @param {string} nominatimUrl Base URL of the geocoding backend * @param {T extends true ? { lat: number, lon: number } : string} search * @param {T} reverse * @param {string} format + * @param {'nominatim' | 'photon'} [provider] Defaults to nominatim * @returns */ -async function geocoder(nominatimUrl, search, reverse, format) { +async function geocoder(nominatimUrl, search, reverse, format, provider) { try { if (!nominatimUrl) { - throw new Error('Nominatim url not provided') + throw new Error('Geocoder url not provided') } - const stockGeocoder = NodeGeocoder({ - provider: 'openstreetmap', - osmServer: nominatimUrl, - timeout: 5000, - }) - stockGeocoder._geocoder._formatResult = ((original) => (result) => ({ - ...original(result), - suburb: result.address.suburb || '', - town: result.address.town || '', - village: result.address.village || '', - }))(stockGeocoder._geocoder._formatResult) + // 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' const results = - typeof search === 'object' - ? await stockGeocoder.reverse(search) - : await stockGeocoder.geocode(search) + provider === 'photon' + ? await photonGeocoder(nominatimUrl, search, isReverse) + : await nominatimGeocoder(nominatimUrl, search, isReverse) return reverse ? formatter(format, results[0]) : format diff --git a/server/src/services/photonGeocoder.js b/server/src/services/photonGeocoder.js new file mode 100644 index 000000000..75c998595 --- /dev/null +++ b/server/src/services/photonGeocoder.js @@ -0,0 +1,209 @@ +// @ts-check + +const { fetchJson } = require('../utils/fetchJson') + +/** + * Photon (https://github.com/komoot/photon) speaks GeoJSON rather than + * Nominatim's JSON, so it cannot be driven through node-geocoder's + * `openstreetmap` provider. This module talks to it directly and returns + * entries in the same shape that provider produces, so everything downstream + * (`formatter`, the webhook resolvers) is unaware of which backend answered. + */ + +/** + * The Photon properties this module reads. Photon returns more; anything not + * listed here has no consumer in ReactMap. + * @typedef {object} PhotonProperties + * @property {string} [name] The result's own label + * @property {string} [housenumber] + * @property {string} [street] + * @property {string} [postcode] + * @property {string} [city] + * @property {string} [county] + * @property {string} [state] + * @property {string} [country] + * @property {string} [countrycode] Upper-case, e.g. "US" + * @property {string} [osm_key] OSM tag key, e.g. "place" or "highway" + * @property {string} [osm_value] OSM tag value, e.g. "city" + */ + +/** + * @typedef {object} PhotonFeature + * @property {{ coordinates?: number[] }} [geometry] GeoJSON order: [lon, lat] + * @property {PhotonProperties} [properties] + */ + +/** Results requested from Photon for a forward search. */ +const SEARCH_LIMIT = 10 + +/** + * Photon reports a result's own label only in `properties.name`, and uses the + * hierarchy fields purely for what *contains* the result. Nominatim echoes the + * name into the matching address field, so searching "Denver" yields a city of + * Denver. These are the OSM classifications where that echo matters, because + * they are the ones `_formatResult` reads. + * @type {Record} + */ +const PLACE_SELF_REFERENCE = { + city: 'city', + town: 'town', + village: 'village', + hamlet: 'hamlet', +} + +/** + * @param {string} base + * @param {string} path + * @param {Record} params + */ +function buildUrl(base, path, params) { + // Trailing slashes are tolerated here, unlike node-geocoder's own + // `osmServer + '/search'` concatenation. + const url = new URL(`${base.replace(/\/+$/, '')}${path}`) + Object.entries(params).forEach(([key, value]) => + url.searchParams.set(key, String(value)), + ) + return url.toString() +} + +/** + * Joins address components the way Nominatim renders `display_name`: most + * specific first, comma separated, skipping absent parts and never repeating a + * component. A postcode search puts the same value in both the result name and + * the postcode field, and Nominatim shows it once. + * @param {(string | undefined)[]} parts + */ +function joinComponents(parts) { + const seen = new Set() + return parts + .map((part) => (part || '').trim()) + .filter((part) => { + if (!part || seen.has(part)) return false + seen.add(part) + return true + }) + .join(', ') +} + +/** + * Blanks any component that reappears later in the hierarchy, keeping the + * broader of the two. A city sharing its state's name (the Statue of Liberty + * sits in city "New York", state "New York") should keep the state: dropping it + * would strip the state out of a US address line entirely. + * @param {(string | undefined)[]} parts + */ +function preferBroader(parts) { + return parts.map((part, i) => + part && parts.slice(i + 1).includes(part) ? undefined : part, + ) +} + +/** + * @param {PhotonProperties} properties + * @param {string} locality + */ +function buildFormattedAddress(properties, locality) { + const street = joinComponents([properties.housenumber, properties.street]) + + // 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, + locality, + properties.county, + properties.state, + properties.postcode, + properties.country, + ]), + ]) +} + +/** + * Maps one Photon feature onto the entry shape node-geocoder's `openstreetmap` + * provider produces, including the three fields ReactMap patches on top of it. + * + * Returns null for a feature without usable coordinates: `parseFloat` of an + * absent value is NaN, and a NaN marker is worse than a missing result. + * + * `suburb` and `neighbourhood` are always empty. Photon's nearest field is + * `district`, which is a different OSM concept, and equating them would be an + * invention rather than a translation. + * @param {PhotonFeature} feature + */ +function formatPhotonFeature(feature) { + const coordinates = feature?.geometry?.coordinates + if (!Array.isArray(coordinates) || coordinates.length < 2) return null + + // GeoJSON is [longitude, latitude]. Never the other way around. + const [longitude, latitude] = coordinates + const properties = feature.properties || {} + + const selfReferenced = PLACE_SELF_REFERENCE[properties.osm_value] + const isPlace = properties.osm_key === 'place' && !!selfReferenced + const named = (/** @type {string} */ field) => + isPlace && selfReferenced === field ? properties.name : undefined + + const city = properties.city || named('city') + const town = named('town') + const village = named('village') + const locality = city || town || village || named('hamlet') || '' + + return { + latitude, + longitude, + formattedAddress: buildFormattedAddress(properties, locality), + country: properties.country, + // Mirrors _formatResult's own city/town/village/hamlet fallback. + city: locality || undefined, + state: properties.state, + zipcode: properties.postcode, + streetName: + properties.street || + (properties.osm_key === 'highway' ? properties.name : undefined), + streetNumber: properties.housenumber, + // 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: '', + town: town || '', + village: village || '', + } +} + +/** + * @param {string} photonUrl + * @param {string | { lat: number, lon: number }} search + * @param {boolean} isReverse + */ +async function photonGeocoder(photonUrl, search, isReverse) { + const url = + isReverse && typeof search === 'object' + ? buildUrl(photonUrl, '/reverse', { + lat: search.lat, + lon: search.lon, + limit: 1, + }) + : buildUrl(photonUrl, '/api', { + q: String(search), + limit: SEARCH_LIMIT, + }) + + const response = await fetchJson(url) + // 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. + const features = Array.isArray(response?.features) ? response.features : [] + + return features.map(formatPhotonFeature).filter(Boolean) +} + +module.exports = { + photonGeocoder, + // Exported for the tests, which check the mapping without a Photon instance. + formatPhotonFeature, + joinComponents, + preferBroader, +} diff --git a/server/test/geocoder.test.js b/server/test/geocoder.test.js new file mode 100644 index 000000000..79026488f --- /dev/null +++ b/server/test/geocoder.test.js @@ -0,0 +1,228 @@ +const assert = require('node:assert/strict') +const { test } = require('node:test') + +const NodeGeocoder = require('node-geocoder') + +const { + formatPhotonFeature, + joinComponents, + preferBroader, +} = require('../src/services/photonGeocoder') + +/** @param {object} properties @param {number[]} [coordinates] */ +const feature = (properties, coordinates = [-104.9903, 39.7392]) => ({ + geometry: { type: 'Point', coordinates }, + properties, +}) + +const DENVER = feature({ + name: 'Denver', + county: 'Denver County', + state: 'Colorado', + country: 'United States', + countrycode: 'US', + osm_key: 'place', + osm_value: 'city', +}) + +const STREET_ADDRESS = feature( + { + housenumber: '123A', + street: 'Main Street', + postcode: '62704', + city: 'Springfield', + county: 'Sangamon County', + state: 'Illinois', + country: 'United States', + countrycode: 'US', + osm_key: 'building', + osm_value: 'yes', + }, + [-89.6501, 39.7817], +) + +test('maps a Photon city onto the geocoder entry shape', () => { + assert.deepEqual(formatPhotonFeature(DENVER), { + latitude: 39.7392, + longitude: -104.9903, + formattedAddress: 'Denver, Denver County, Colorado, United States', + country: 'United States', + city: 'Denver', + state: 'Colorado', + zipcode: undefined, + streetName: undefined, + streetNumber: undefined, + countryCode: 'US', + neighbourhood: '', + suburb: '', + town: '', + village: '', + }) +}) + +test('maps a full street address', () => { + const got = formatPhotonFeature(STREET_ADDRESS) + assert.equal(got.streetNumber, '123A') + assert.equal(got.streetName, 'Main Street') + assert.equal(got.zipcode, '62704') + assert.equal(got.city, 'Springfield') + assert.equal( + got.formattedAddress, + '123A, Main Street, Springfield, Sangamon County, Illinois, 62704, United States', + ) +}) + +// GeoJSON is [longitude, latitude]. Reversing it puts every US result in the +// wrong hemisphere. +test('reads coordinates in GeoJSON order', () => { + const got = formatPhotonFeature(DENVER) + assert.equal(got.latitude, 39.7392) + assert.equal(got.longitude, -104.9903) +}) + +test('drops a feature with no usable coordinates', () => { + assert.equal(formatPhotonFeature(feature({ name: 'Nowhere' }, [])), null) + assert.equal(formatPhotonFeature(feature({ name: 'Nowhere' }, [1])), null) + assert.equal(formatPhotonFeature({ properties: { name: 'Nowhere' } }), null) +}) + +// Photon reports a result's own label only in properties.name. Nominatim +// echoes it into the matching address field, and the whole locality fallback +// depends on that echo happening. +test('echoes the result name into its locality field', () => { + const cases = [ + { osm_value: 'city', name: 'Denver', field: 'city' }, + { osm_value: 'town', name: 'Lyman', field: 'town' }, + { osm_value: 'village', name: 'Arcola', field: 'village' }, + { osm_value: 'hamlet', name: 'Bootjack', field: null }, + ] + cases.forEach(({ osm_value, name, field }) => { + const got = formatPhotonFeature( + feature({ name, osm_key: 'place', osm_value }), + ) + assert.equal(got.city, name, `${osm_value} should resolve city`) + if (field) + assert.equal(got[field], name, `${osm_value} should set ${field}`) + }) +}) + +test('uses the result name as the street for a road', () => { + const got = formatPhotonFeature( + feature({ + name: 'Lake Shore Drive', + osm_key: 'highway', + osm_value: 'trunk', + }), + ) + assert.equal(got.streetName, 'Lake Shore Drive') +}) + +test("Photon's own city wins over the echoed name", () => { + const got = formatPhotonFeature( + feature({ + name: 'Denver', + city: 'Aurora', + osm_key: 'place', + osm_value: 'city', + }), + ) + assert.equal(got.city, 'Aurora') +}) + +// 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', () => { + const got = formatPhotonFeature(STREET_ADDRESS) + assert.equal(got.suburb, '') + assert.equal(got.neighbourhood, '') +}) + +test('joinComponents skips absent parts and repeats', () => { + assert.equal( + joinComponents(['Denver', undefined, 'Colorado']), + 'Denver, Colorado', + ) + assert.equal(joinComponents([' ', '']), '') + // A postcode search puts the same value in the name and the postcode field. + assert.equal( + joinComponents(['62704', 'Leland Grove', '62704', 'Illinois']), + '62704, Leland Grove, Illinois', + ) +}) + +test('preferBroader keeps the later of two identical components', () => { + assert.deepEqual(preferBroader(['New York', 'New York County', 'New York']), [ + undefined, + 'New York County', + 'New York', + ]) +}) + +// A city sharing its state's name must not cost the address line its state. +test('a city named after its state keeps the state', () => { + const got = formatPhotonFeature( + feature({ + name: 'Statue of Liberty', + street: 'Flagpole Plaza', + city: 'New York', + county: 'New York County', + state: 'New York', + postcode: '10004', + country: 'United States', + countrycode: 'US', + osm_key: 'tourism', + osm_value: 'attraction', + }), + ) + assert.equal( + got.formattedAddress, + 'Statue of Liberty, Flagpole Plaza, New York County, New York, 10004, United States', + ) +}) + +// The load-bearing test. Everything downstream of the geocoder service reads +// the entry shape node-geocoder's openstreetmap provider produces, so the +// Photon path has to produce that same shape rather than something similar. +// This builds the provider exactly as geocoder.js does, patch and all, and +// compares the keys it emits against the keys the Photon path emits. +test('the Photon path emits the same keys as the Nominatim path', () => { + const stockGeocoder = NodeGeocoder({ + provider: 'openstreetmap', + 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)) + + // The same place as STREET_ADDRESS, in Nominatim's response shape. + const fromNominatim = stockGeocoder._geocoder._formatResult({ + lat: '39.7817', + lon: '-89.6501', + display_name: + '123A, Main Street, Springfield, Sangamon County, Illinois, 62704, United States', + address: { + house_number: '123A', + road: 'Main Street', + city: 'Springfield', + county: 'Sangamon County', + state: 'Illinois', + postcode: '62704', + country: 'United States', + country_code: 'us', + }, + }) + const fromPhoton = formatPhotonFeature(STREET_ADDRESS) + + assert.deepEqual( + Object.keys(fromPhoton).sort(), + Object.keys(fromNominatim).sort(), + 'the two providers must produce the same fields', + ) + // And for this address the values agree too, which is the point of the + // exercise: ReactMap should not be able to tell which backend answered. + assert.deepEqual(fromPhoton, fromNominatim) +}) From b46654aaf7515b5a3f751defe2ff6106209e8713 Mon Sep 17 00:00:00 2001 From: Rin <58572875+TurtIeSocks@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:29:33 -0400 Subject: [PATCH 2/4] fix(geocoder): drop Photon features whose coordinates are not finite The length check alone accepted a coordinate pair of [null, null] and emitted an entry with a null latitude, which contradicts the filtering contract the function documents. Checking the values rather than the shape also covers NaN, Infinity, undefined and numeric strings. The added test fails without the guard. --- server/src/services/photonGeocoder.js | 4 ++++ server/test/geocoder.test.js | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/server/src/services/photonGeocoder.js b/server/src/services/photonGeocoder.js index 75c998595..8fe4f75af 100644 --- a/server/src/services/photonGeocoder.js +++ b/server/src/services/photonGeocoder.js @@ -138,6 +138,10 @@ function formatPhotonFeature(feature) { // GeoJSON is [longitude, latitude]. Never the other way around. const [longitude, latitude] = coordinates + // A well behaved Photon sends two numbers, but the length check alone would + // pass [null, null] straight through to an entry with a null latitude. Check + // the values, not just the shape. + if (!Number.isFinite(longitude) || !Number.isFinite(latitude)) return null const properties = feature.properties || {} const selfReferenced = PLACE_SELF_REFERENCE[properties.osm_value] diff --git a/server/test/geocoder.test.js b/server/test/geocoder.test.js index 79026488f..26908201e 100644 --- a/server/test/geocoder.test.js +++ b/server/test/geocoder.test.js @@ -86,6 +86,28 @@ test('drops a feature with no usable coordinates', () => { assert.equal(formatPhotonFeature({ properties: { name: 'Nowhere' } }), null) }) +// A pair of the right length is not the same as a pair of usable numbers. +// Emitting these would put a null or NaN latitude on the map rather than +// dropping the result. +test('drops a feature whose coordinates are not finite numbers', () => { + const unusable = [ + [null, null], + [-104.9903, null], + [null, 39.7392], + ['-104.9903', '39.7392'], + [undefined, undefined], + [NaN, NaN], + [Infinity, 39.7392], + ] + unusable.forEach((coordinates) => { + assert.equal( + formatPhotonFeature(feature({ name: 'Nowhere' }, coordinates)), + null, + `${JSON.stringify(coordinates)} should be dropped`, + ) + }) +}) + // Photon reports a result's own label only in properties.name. Nominatim // echoes it into the matching address field, and the whole locality fallback // depends on that echo happening. From f33185d4f22223fbf9a12136dcba573422e576f2 Mon Sep 17 00:00:00 2001 From: Rin <58572875+TurtIeSocks@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:59:05 -0400 Subject: [PATCH 3/4] fix(geocoder): carry geocoderProvider onto the PoracleAPI instance Event.webhookObj holds PoracleAPI instances rather than the raw webhook config, and the constructor copies fields across one at a time. geocoderProvider was not among them, so it was undefined by the time the resolvers read it and every request took the Nominatim branch. Configuring Photon did nothing at all. The mapping tests all passed because they exercised the response mapping and never the config to instance to resolver path. Two tests now cover that boundary: one asserting a configured provider survives construction alongside nominatimUrl and addressFormat, and one asserting it stays undefined when unset, which is what keeps existing configs on Nominatim. Both fail without the constructor change. --- server/src/services/Poracle.js | 1 + server/test/geocoder.test.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/server/src/services/Poracle.js b/server/src/services/Poracle.js index 81edc6d64..06a8c0ce6 100644 --- a/server/src/services/Poracle.js +++ b/server/src/services/Poracle.js @@ -55,6 +55,7 @@ class PoracleAPI { this.areasToSkip = webhook.areasToSkip?.map((x) => x.toLowerCase()) || [] this.addressFormat = webhook.addressFormat this.nominatimUrl = webhook.nominatimUrl + this.geocoderProvider = webhook.geocoderProvider this.enabled = webhook.enabled || false this.discordRoles = webhook.discordRoles || [] diff --git a/server/test/geocoder.test.js b/server/test/geocoder.test.js index 26908201e..d65ebdfbf 100644 --- a/server/test/geocoder.test.js +++ b/server/test/geocoder.test.js @@ -3,6 +3,7 @@ const { test } = require('node:test') const NodeGeocoder = require('node-geocoder') +const { PoracleAPI } = require('../src/services/Poracle') const { formatPhotonFeature, joinComponents, @@ -248,3 +249,36 @@ test('the Photon path emits the same keys as the Nominatim path', () => { // exercise: ReactMap should not be able to tell which backend answered. assert.deepEqual(fromPhoton, fromNominatim) }) + +// Event.webhookObj holds PoracleAPI instances, not the raw webhook config, and +// the constructor copies fields across one by one. A setting it forgets is +// undefined by the time the resolvers read it, so the whole feature silently +// takes the Nominatim branch. These cover that boundary rather than the +// mapping. +const webhookConfig = (overrides = {}) => ({ + name: 'test', + host: 'http://127.0.0.1', + port: 3030, + enabled: true, + nominatimUrl: 'http://127.0.0.1:2322', + addressFormat: '{{city}}, {{state}}', + ...overrides, +}) + +test('PoracleAPI carries the configured geocoder provider through to the resolvers', () => { + const api = new PoracleAPI(webhookConfig({ geocoderProvider: 'photon' })) + + // The three values resolvers.js hands to geocoder(). + assert.equal(api.geocoderProvider, 'photon') + assert.equal(api.nominatimUrl, 'http://127.0.0.1:2322') + assert.equal(api.addressFormat, '{{city}}, {{state}}') +}) + +test('PoracleAPI leaves the provider undefined when it is not configured', () => { + const api = new PoracleAPI(webhookConfig()) + + // Undefined is what sends geocoder() down the Nominatim branch, which is the + // correct default for every existing config. + assert.equal(api.geocoderProvider, undefined) + assert.equal(api.nominatimUrl, 'http://127.0.0.1:2322') +}) From 38654d1c6d9f3ccf851cfb32e38456b34524e129 Mon Sep 17 00:00:00 2001 From: Rin <58572875+TurtIeSocks@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:15:49 -0400 Subject: [PATCH 4/4] fix(geocoder): fall back to Photon's locality layer for the settlement Photon's address hierarchy runs city, district, locality, street. An address whose containing settlement sits below the city layer, such as a house in a hamlet, carries that name in properties.locality and has no city at all. The fallback only read city, so those addresses lost their settlement from both the city field and formattedAddress, and any format using {{city}} rendered a blank component in forward search and in gym reverse geocoding alike. city still wins where Photon sends both, matching its own hierarchy. The local variable is now called settlement rather than locality, so it is not mistaken for the Photon field it falls back to. Two fixtures cover it: a house in a hamlet, and a response carrying both city and locality. The first fails without the change. --- server/src/services/photonGeocoder.js | 24 ++++++++++----- server/test/geocoder.test.js | 44 +++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/server/src/services/photonGeocoder.js b/server/src/services/photonGeocoder.js index 8fe4f75af..d4a2d598e 100644 --- a/server/src/services/photonGeocoder.js +++ b/server/src/services/photonGeocoder.js @@ -19,6 +19,8 @@ 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} [county] * @property {string} [state] * @property {string} [country] @@ -100,9 +102,9 @@ function preferBroader(parts) { /** * @param {PhotonProperties} properties - * @param {string} locality + * @param {string} settlement The resolved city, town, village, hamlet or locality */ -function buildFormattedAddress(properties, locality) { +function buildFormattedAddress(properties, settlement) { const street = joinComponents([properties.housenumber, properties.street]) // The result's own name leads. When it was already echoed into a hierarchy @@ -111,7 +113,7 @@ function buildFormattedAddress(properties, locality) { properties.name, ...preferBroader([ street, - locality, + settlement, properties.county, properties.state, properties.postcode, @@ -152,15 +154,23 @@ function formatPhotonFeature(feature) { const city = properties.city || named('city') const town = named('town') const village = named('village') - const locality = city || town || village || named('hamlet') || '' + // 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. + const settlement = + city || town || village || named('hamlet') || properties.locality || '' return { latitude, longitude, - formattedAddress: buildFormattedAddress(properties, locality), + formattedAddress: buildFormattedAddress(properties, settlement), country: properties.country, - // Mirrors _formatResult's own city/town/village/hamlet fallback. - city: locality || undefined, + // Mirrors _formatResult's own city/town/village/hamlet fallback, with + // Photon's locality layer appended to it. + city: settlement || undefined, state: properties.state, zipcode: properties.postcode, streetName: diff --git a/server/test/geocoder.test.js b/server/test/geocoder.test.js index d65ebdfbf..a5398ede7 100644 --- a/server/test/geocoder.test.js +++ b/server/test/geocoder.test.js @@ -129,6 +129,50 @@ test('echoes the result name into its locality field', () => { }) }) +// A house in a hamlet: Photon puts the containing settlement in `locality` and +// 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', () => { + 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], + ), + ) + assert.equal(got.city, 'Bootjack') + assert.equal( + got.formattedAddress, + '4, County Road 15, Bootjack, Mariposa County, California, 95338, 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', () => { + const got = formatPhotonFeature( + feature({ + city: 'Mariposa', + locality: 'Bootjack', + state: 'California', + country: 'United States', + countrycode: 'US', + }), + ) + assert.equal(got.city, 'Mariposa') +}) + test('uses the result name as the street for a road', () => { const got = formatPhotonFeature( feature({