feat: fort map-data consumer — gyms, stations & pokestops - with DNF filtering#1228
feat: fort map-data consumer — gyms, stations & pokestops - with DNF filtering#1228jfberry wants to merge 50 commits into
Conversation
…wMap#1225) * fix(scanArea): prevent crash when area feature has no name/key Guard the scan area search filter against features missing a properties.key (which happens when a scan area polygon has no name set), instead of throwing TypeError: Cannot read properties of undefined (reading 'toLowerCase'). Also fixes a longstanding typo (geoJsonFilName / geoJsonFilname -> geoJsonFileName) in the multi-domain example config and docs. * fix: copilot comments --------- Co-authored-by: Mygod <contact-git@mygod.be>
Validating in a test environmentScope: this branch routes only gyms through Golbat ( 1. Golbat prereqs — deploy #385, curl -H "X-Golbat-Secret: $S" "$G/api/gym/available"
curl -H "X-Golbat-Secret: $S" -H 'Content-Type: application/json' -XPOST "$G/api/gym/scan" \
-d '{"min":{"latitude":LAT1,"longitude":LON1},"max":{"latitude":LAT2,"longitude":LON2},"limit":1000,"filters":[]}'
curl -H "X-Golbat-Secret: $S" "$G/api/gym/id/<gym-id>"2. ReactMap — 3. Config — add 4. Confirm the endpoint path (logs):
5. Functional — gym + raid markers render; a popup shows team/slots/defenders and raid boss + moves + CP; the filter drawer populates teams, tiers ( 6. Golden parity (endpoint vs SQL): 7. Fallback — stop Golbat or set 8. Area restrictions — as an area-restricted user, confirm gyms appear only within permitted areas (exercises the new client-side |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…st-layer availability fix Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dadba73 to
af62c30
Compare
… badge poison Review findings: - tier-override mode matches raid_level alone (curated boss/egg keys under-returned tier raids on endpoint sources) - b<display_type> keys move to the onlyEventStops gate (secondaryFilter's events branch consumes them, not the invasions branch) - endpoint rows resolve the quest layer as dual-capable (pure-endpoint ctx flags made effectiveQuestLayer resolve to 'both') - poison on onlyGymBadges only: onlyBadge defaults to 'all' for every user with the gymBadges perm and was silently disabling gym DNF entirely Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the all-gyms/ex/in-battle poison with real narrowing: every gym-layer display requires the team/slot match (hasGym = enabler && (team || slot)), so team_id/available_slots clauses mirroring finalTeams/finalSlots are a tight superset for all four enablers — ex/ar/in-battle narrowing stays residual. The standalone is_ar_scan_eligible clause is subsumed (ar-shown gyms also need the team match). Badge viewing still poisons. Power-up narrowing is gone for good — power-ups are no longer in the game. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This is now in my production and I have verified the DNFs through manual testing |
The endpoint (in-memory) getAll paths filter rows with filterRTree, which returns true for empty area inputs — so unlike the SQL path's getAreaSql it did NOT deny a user who has no assigned areas while strictAreaRestrictions is on and restrictions are configured. Such a user received every fort in the viewport (access-control bypass). Add areaRestrictionsDenyAll (mirroring getAreaSql's strict-deny and empty-consolidated deny) and short-circuit each fort model's endpoint branch to [] before accepting rows. Addresses Mygod review comment WatWowMap#1 (P1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…resilience Three endpoint-source correctness fixes from the Mygod review: - WatWowMap#4 (P1): a pure-endpoint pokestop source now reports hasConfirmed:true. The Golbat scan always returns confirmed incident data, but no DB schema check runs for a knex-less source, so onlyConfirmed and confirmed rocket-reward (a) filters were silently degrading to the grunt possible-encounter pool. - WatWowMap#6 (P2): historicalRarity now skips only pure-endpoint sources (no bound knex), not dual sources. Testing source.mem alone dropped a dual source DB and cleared the historical rarity map on every refresh. - WatWowMap#8 (P2): DbManager.getOne uses Promise.allSettled so a pure-endpoint source whose by-id fetch misses (falling through to an unbound this.query() that throws) no longer fails the whole single-fort lookup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Golbat now exposes confirmed invasion slots 2/3 in /api/fort/available. Read them in the availability mapper, adding an a<id>-<form> key per slot the event config marks as a reward (second/thirdReward) — matching the SQL path, which already advertises all three confirmed slots. Leaders/Giovanni (41-44) stay excluded. Addresses Mygod review comment WatWowMap#9 (P2). Requires Golbat with the slots-2/3 availability change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the thorough review, @Mygod. Walking through all nine — seven are addressed (across this PR and the Golbat companion #385), and two look like they were reviewed against a Golbat without #385's changes. Details below. Fixed
Not reproduced (appear to predate Golbat #385)
All changes build/lint clean; Golbat decoder tests pass (incl. new slots-2/3 coverage). #3, #9, and the type-20 path assume a deployed Golbat ≥ #385 + #382. |
|
I've taken the change to support slot2/slot3 availability and querying (which I previously had not added because these fields are always empty). Just like AR searching, these fields are legacy fields at the moment - but slot2/3 data coming back is something we would hope for though unlikely, whereas AR is something which is likely gone forever. A focused change later to drop AR data capture & reporting will be a wider impact in golbat |
|
Type-20 workaround has been removed in 9acf867. Please merge main and resolve conflicts. Hopefully it will make things easier. |
…Fest stopgap" This reverts commit cade9f6.
# Conflicts: # server/src/models/Pokestop.js
develop's canonical type-20 handling (9acf867 + reward-definition refactor) superseded my SQL-side type-20 work, which was reverted before the merge. The endpoint DNF builder is ReactMap-fort-consumer-only (not in develop), so re-apply the mega clause quest_reward_type [12, 20] so Golbat returns type-20 (temp-evo branch) mega stops for the in-memory scan path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merged My type-20 handling has converged on develop's: I reverted my SQL-side type-20 generalization (it was redundant with
No conflict markers, lint clean; the DNF, type-20 key, and rocket slots-2/3 mapper pass their unit checks. This assumes deployed Golbat ≥ #385 + #382 for the type-20 / slots-2/3 paths. |
|
The endpoint Pokestop path currently returns no markers because it omits a required filter limit, and one supported pure-endpoint configuration can fail during startup. Additional endpoint capability, availability, manual-ID, performance, and source-maintainability regressions also remain. Full review comments:
Also please change the merge target to |
- [P1] Endpoint Pokestop scan passed no resultLimit to secondaryFilter, whose loop runs while filteredResults.length < resultLimit — so undefined returned zero markers. Pass queryLimits.pokestops (mirroring the SQL call) and drop the pre-truncation, which also fixes [P2] dropping the appended off-viewport manual-id row before filtering. - [P1] getFilterContext ran this.query() on an unbound (pure-endpoint) model when fallbackRocketPokemonFiltering is off and hasConfirmed is set, rejecting at startup. Recognize mem and return endpoint capability without SQL. - [P2] Dual endpoint sources now marked hasConfirmed:true even when the bound DB lacks the confirmed column (getAll uses Golbat rows with confirmation). - [P2] Availability mapper advertises type-20 mega only when both pokemon_id and amount are present (never u20 or m<id>-0), matching what secondaryFilter keys. - [P2] Cache-key separators are the \0 escape instead of literal NUL bytes, so git no longer classifies fortAvailable.js as binary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # server/src/services/EventManager.js
|
Thanks — all seven addressed, plus retargeted to P1
P2
Base is now |
P1:
- getOne (gym/pokestop) endpoint now projects to {lat, lon} like the SQL path,
instead of returning the raw Golbat record. The client controls the GraphQL
selection, so returning the full record leaked raid/team/lure/detail past the
sub-perm split and area restrictions for any fort id.
- Pokestop endpoint onlyLevels power-up gate now guarded on !onlyAllPokestops
(like the gym sibling). SQL only applies it under onlyAllPokestops, so the
endpoint was under-returning the entire quest/invasion/lure layer for a user
with a non-all levels filter.
P2:
- DbManager.search / submissionCells use Promise.allSettled: the fort
search/getSubmissions methods have no endpoint branch, so a pure-endpoint
source rejected the whole batch (crashing search/submissions) instead of
degrading. Mirrors the getOne fix.
- fetchJson redacts Authorization / X-Golbat-Secret before the debug log and
the failed-request payload dump, so credentials are no longer written to disk.
- EventManager only arms the availability TTL on a non-empty refresh, so a
failed (empty) endpoint refresh no longer suppresses recovery for the window.
P3:
- encodeURIComponent on all fort by-id fetch URLs (getOne, manual-id,
getDynamaxMons) — no path traversal via id/onlyManualId.
- historicalRarity uses Promise.allSettled so a dual source whose DB lacks
pokemon_stats no longer fails the whole batch and blanks every rarity map.
(Skipped review P3: force does not bypass the 30s combined-availability cache —
that cache deliberately caches failures to avoid hammering; the TTL fix already
cuts the drawer-empty window to <=30s, and force-bypass needs invasive threading
for a minor gain.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Whole-PR review + fixesGiven how much moved recently (the P1 (fixed)
P2 (fixed)
P3
Verified solid (no action)
|
|
(running in my production, will also ask my other testers of this PR to update after a short period of no further review comments) |
|
The endpoint paths can omit valid forts, break advertised dual-source fallback, and replace cached availability or rarity with failure-derived empty data. Lint and production build pass, but they do not cover these runtime contract and failure-mode regressions. Full review comments:
|
Four ReactMap-side regressions from Mygod review; the three that would modify Golbat or broaden the Golbat queries are held (see PR discussion). - Availability mapper runs generic u<type> quest keys through process() so they carry their title/target conditions, matching develop SQL genericQuests (endpoint deployments were losing the advanced quest-condition selector). - EventManager #refreshAvailable returns early on an empty result: a failed (empty) refresh no longer replaces the last-good drawer + conditions or arms the TTL — it retains the cache and retries next call. - DbManager no longer forces hasConfirmed:true on a dual source. That flag gates the SQL fallback confirmed-column query, so a dual DB lacking confirmed would reject on fallback. The endpoint scan branch marks itself confirmed-capable locally instead, keeping endpoint capability separate from the fallback flag. - historicalRarity retains the last-good rarity map when every eligible pokemon_stats query fails (total DB outage) instead of clearing it — the allSettled change had lost Promise.all catch-path retention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — four of these were real ReactMap-side regressions and are fixed in 4b278a3; the other three reference the Golbat companion contract and are addressed there rather than by broadening the queries. Fixed (4b278a3)
Addressed on the Golbat side / not broadenedThese three are contract points against Golbat #385 (the companion), and #385 already covers them — so the fix is to rely on the contract, not to widen what Golbat returns:
All four fixes are lint/ |
Deploying this (end users)
Two small config changes — one on Golbat, one on ReactMap. Once both are set, fort map-data (gyms, pokestops, stations) is served from Golbat's in-memory fort API instead of SQL, with automatic SQL fallback if Golbat is ever unreachable — so it's safe to enable on a dual (endpoint + DB) source.
1 · Golbat — run Golbat with UnownHash/Golbat#385 (it already includes the reward type-20 quest decode from #382) and turn on the in-memory fort index.
fort_in_memoryis a top-level option and defaults tofalse:Make sure your Golbat API
secretis set. Sanity-check the endpoint is live:2 · ReactMap — add
endpoint+secretto the scanner DB source whoseuseForincludes your fort types (gym/pokestop/station). This is the same Golbat endpoint most instances already point their pokémon source at; the DB source simply becomes dual (endpoint + DB):Restart ReactMap — this is a server-only change (no client rebuild). Gyms/pokestops/stations now fetch from Golbat; markers, popups, filters and deep-links keep working; and if Golbat is down or
fort_in_memoryis off, each fort type transparently falls back to SQL on the same DB.Confirm it's live (logs): each ~15-min availability refresh logs a line like
[GYM] loaded available from http://<golbat>:<port>/api/fort/available — …, and map pans stop issuing fort SQL on endpoint-backed sources. A Golbat outage instead logs a fallback warn and reverts to SQL.What this does
Routes fort map data — gyms, pokestops, and stations (
getAll,getOne,getAvailable) — through Golbat's in-memory fort API (UnownHash/Golbat#385) when a scanner source has anendpoint, with transparent SQL fallback. Map pans stop issuing fort SQL entirely on endpoint-backed sources, and Golbat's DNF filtering returns only the forts that will render.Architecture
memset on the source):POST /api/<type>/scan→ per-record pure mapper → the existing, unchangedsecondaryFilter. On any failure (503 / network / bad shape) the model logs and falls through to SQL — a dual source degrades gracefully.secondaryFilterguarantees exactness. Pure backends (server/src/filters/fort/*) translate the map filters into Golbat DNF clauses (OR across clauses, AND within). The one hard invariant is never under-return; anything inexpressible (quest title/target conditions, gender, gym badges, time-window cutoffs) poisons to match-all or stays a client-side residual.secondaryFilter's per-perm field gating — andgetOnereturns only{lat, lon}like SQL.DNF(n): X matched -> Y after secondaryFilter (-residual) | <clause shapes>), so any translation gap is visible in production logs.onlyManualIdfort is fetched via the by-id endpoint and joins the candidate set, mirroring SQL's(bbox) OR id = ?.Source shapes
A scanner source can be pure-DB (unchanged), dual (endpoint + DB — fort reads use Golbat, everything else and the fallback use the bound DB), or pure-endpoint (endpoint only, no DB — degrades gracefully where a query would need a DB). Enabling the endpoint on an existing DB source (the common case above) makes it dual.
Verified live (dual source, endpoint vs SQL)
station_active, gmax)Requirements
fort_in_memory = true), deployed before this — Golbat's scan decoder ignores unknown request fields, so an out-of-date Golbat degrades to match-all/SQL rather than erroring.endpoint+secret(and optionalhttpAuth) as for pokémon; DB config unchanged (used for fallback + un-migrated queries).No test framework exists in this repo; verification was throwaway node goldens per pure module (mappers, DNF backends),
eslint/prettier, a full correctness/security/concurrency review of the branch, and the live parity runs above.🤖 Generated with Claude Code