From bbd43dae161869a566dcf4d3da76d6ccdbaaf14a Mon Sep 17 00:00:00 2001 From: Rin <58572875+TurtIeSocks@users.noreply.github.com> Date: Sun, 23 Aug 2026 13:17:16 -0400 Subject: [PATCH 1/2] feat(geocoder): derive geocoderProvider from Poracle's remote config nominatimUrl already falls back to Poracle's providerURL, so a deployment can get its geocoder URL from Poracle without restating it here. The backend type had no such fallback, which left half the pair auto-populating and the other half not: an operator whose URL came from Poracle still had to hand-set geocoderProvider locally, or every request silently took the Nominatim branch. PoracleNG now reports the backend behind providerURL as `provider`, so geocoderProvider follows the same rule as its URL. Local config still wins, matching how providerURL and addressFormat already behave. Poracle also offers google and none. Neither has a ReactMap equivalent, so they resolve to undefined rather than being mapped onto nominatim, which would claim something untrue about the backend. The match is exact, so a differently-cased value is ignored rather than guessed at. `provider` is destructured out of remoteConfig whether or not it is usable. this.provider is the *webhook* provider, an unrelated field that happens to share the name, and letting Poracle's value through Object.assign(this, rest) would silently overwrite it. The resolution is a small exported function because #fetchConfig is private and would otherwise need a stubbed HTTP round trip to reach. Four tests cover the derivation, local precedence, the ignored backends, and the webhook provider surviving a remote payload that carries a geocoding provider. --- packages/types/lib/config.d.ts | 5 ++++ server/src/services/Poracle.js | 37 +++++++++++++++++++++-- server/test/geocoder.test.js | 54 +++++++++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 3 deletions(-) diff --git a/packages/types/lib/config.d.ts b/packages/types/lib/config.d.ts index 0a2b80246..ee285fb2e 100644 --- a/packages/types/lib/config.d.ts +++ b/packages/types/lib/config.d.ts @@ -199,6 +199,11 @@ 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, + * the same way `nominatimUrl` falls back to Poracle's `providerURL`. Setting + * it here always wins. */ geocoderProvider?: 'nominatim' | 'photon' trialPeriodEligible?: boolean diff --git a/server/src/services/Poracle.js b/server/src/services/Poracle.js index 06a8c0ce6..452ce726f 100644 --- a/server/src/services/Poracle.js +++ b/server/src/services/Poracle.js @@ -41,6 +41,31 @@ 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. + * @param {string} [remote] The `provider` field from Poracle's remote config + * @param {string} [local] geocoderProvider from this webhook's own config + * @returns {'nominatim' | 'photon' | undefined} + */ +function resolveGeocoderProvider(remote, local) { + if (local) return /** @type {'nominatim' | 'photon'} */ (local) + return REMOTE_GEOCODERS.has(remote) + ? /** @type {'nominatim' | 'photon'} */ (remote) + : undefined +} + class PoracleAPI { /** @param {import("@rm/types").Config['webhooks'][number]} webhook */ constructor(webhook) { @@ -231,7 +256,11 @@ 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 @@ -239,6 +268,10 @@ class PoracleAPI { if (providerURL && !this.nominatimUrl) { this.nominatimUrl = providerURL } + this.geocoderProvider = resolveGeocoderProvider( + provider, + this.geocoderProvider, + ) this.leagues = [ { name: 'great', cp: 1500, min: remoteConfig.pvpFilterGreatMinCP }, { name: 'ultra', cp: 2500, min: remoteConfig.pvpFilterUltraMinCP }, @@ -1257,4 +1290,4 @@ class PoracleAPI { } } -module.exports = { PoracleAPI } +module.exports = { PoracleAPI, resolveGeocoderProvider } diff --git a/server/test/geocoder.test.js b/server/test/geocoder.test.js index 67a7f5c8a..7db114685 100644 --- a/server/test/geocoder.test.js +++ b/server/test/geocoder.test.js @@ -5,7 +5,10 @@ const NodeGeocoder = require('node-geocoder') const http = require('node:http') -const { PoracleAPI } = require('../src/services/Poracle') +const { + PoracleAPI, + resolveGeocoderProvider, +} = require('../src/services/Poracle') const { formatter, geocoder, @@ -1147,3 +1150,52 @@ test('a formatted result keeps the mapped fields for the Geocoder type', async ( 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', () => { + assert.equal(resolveGeocoderProvider('photon', undefined), 'photon') + assert.equal(resolveGeocoderProvider('nominatim', undefined), 'nominatim') +}) + +test('local configuration beats what Poracle reports', () => { + assert.equal(resolveGeocoderProvider('nominatim', 'photon'), 'photon') + assert.equal(resolveGeocoderProvider('photon', 'nominatim'), 'nominatim') +}) + +// 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, undefined), + 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(provider, api.geocoderProvider) + + assert.equal(api.provider, 'poracle') + assert.equal(api.geocoderProvider, 'photon') +}) From b6471267dd66b2852abe3d68eb9d7c94d98a8458 Mon Sep 17 00:00:00 2001 From: Rin <58572875+TurtIeSocks@users.noreply.github.com> Date: Sun, 23 Aug 2026 16:23:22 -0400 Subject: [PATCH 2/2] fix(geocoder): only inherit Poracle's provider alongside its URL The fallback was unconditional, so a webhook with a local nominatimUrl and no geocoderProvider inherited Photon from a Photon-backed Poracle while keeping its own Nominatim URL. That is the default shape of every existing Nominatim deployment, and it made searches and reverse lookups return {} against an endpoint that had been working. A URL and the protocol used to talk to it are one setting in two fields, and splitting them across two sources cannot produce a working pair by accident. The backend type is now inherited only when the URL was inherited with it. An explicit local geocoderProvider still wins in either case, which is the only way to run a backend Poracle does not report. resolveGeocoderProvider takes a named argument object rather than a third positional boolean, so the pairing rule is legible at the call site. --- packages/types/lib/config.d.ts | 7 +++-- server/src/services/Poracle.js | 29 +++++++++++++----- server/test/geocoder.test.js | 55 ++++++++++++++++++++++++++++++---- 3 files changed, 74 insertions(+), 17 deletions(-) diff --git a/packages/types/lib/config.d.ts b/packages/types/lib/config.d.ts index ee285fb2e..15bbf422a 100644 --- a/packages/types/lib/config.d.ts +++ b/packages/types/lib/config.d.ts @@ -201,9 +201,10 @@ export interface Webhook { * 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, - * the same way `nominatimUrl` falls back to Poracle's `providerURL`. Setting - * it here always wins. + * `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 452ce726f..400487743 100644 --- a/server/src/services/Poracle.js +++ b/server/src/services/Poracle.js @@ -55,12 +55,21 @@ const REMOTE_GEOCODERS = new Set(['nominatim', 'photon']) * 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. - * @param {string} [remote] The `provider` field from Poracle's remote config - * @param {string} [local] geocoderProvider from this webhook's own config + * + * 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) { +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 @@ -265,13 +274,17 @@ class PoracleAPI { 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( - provider, - this.geocoderProvider, - ) + 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 }, diff --git a/server/test/geocoder.test.js b/server/test/geocoder.test.js index 7db114685..659dd6580 100644 --- a/server/test/geocoder.test.js +++ b/server/test/geocoder.test.js @@ -1156,13 +1156,48 @@ test('a formatted result keeps the mapped fields for the Geocoder type', async ( // 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', () => { - assert.equal(resolveGeocoderProvider('photon', undefined), 'photon') - assert.equal(resolveGeocoderProvider('nominatim', undefined), 'nominatim') + 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('nominatim', 'photon'), 'photon') - assert.equal(resolveGeocoderProvider('photon', 'nominatim'), 'nominatim') + 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 @@ -1171,7 +1206,11 @@ test('ignores Poracle backends ReactMap cannot drive', () => { const unusable = ['google', 'none', '', undefined, 'PHOTON'] unusable.forEach((remote) => { assert.equal( - resolveGeocoderProvider(remote, undefined), + resolveGeocoderProvider({ + remote, + local: undefined, + usingRemoteURL: true, + }), undefined, `${remote} should not resolve to a provider`, ) @@ -1194,7 +1233,11 @@ test('a Poracle geocoding provider never overwrites the webhook provider', () => const remoteConfig = { provider: 'photon', providerURL: 'http://photon:2322' } const { provider, ...rest } = remoteConfig Object.assign(api, rest) - api.geocoderProvider = resolveGeocoderProvider(provider, api.geocoderProvider) + api.geocoderProvider = resolveGeocoderProvider({ + remote: provider, + local: api.geocoderProvider, + usingRemoteURL: true, + }) assert.equal(api.provider, 'poracle') assert.equal(api.geocoderProvider, 'photon')