feat(geocoder): derive geocoderProvider from Poracle's remote config

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

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 }

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,95 @@ 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', () => {
+  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')
+})