From 17362dd44f389f8db6a5c3fa17d1f537b086c56d Mon Sep 17 00:00:00 2001 From: hokiepokedad2 <38219945+hokiepokedad2@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:41:30 -0400 Subject: [PATCH 1/4] feat(tracking): write eight more alarm types through PoracleNG's /api/v2 Raid, egg, quest, nest, gym, max battle, fort change and lure join pokemon on the uid-addressed PUT. Invasion deliberately does not: a v2 read of a named-grunt rule carries no targeting field, and PoracleWeb.NET holds only the grunt name, which live data fills with values it cannot reverse. Taking the v2 branch skips the whole v1 repair path as a unit, which is the point. Lure loses NaturalKeyTrackingUpdate's delete-create-restore dance and max battle loses its delete-then-create window; the other seven skip the reconcile of the duplicate a v1 create leaves behind. The pre-write guards stay -- v2's POST still merges and its 409 covers only an exact duplicate. TrackingV2Translator becomes table-driven, one table per type, because V2PokemonRule declares 28 integer filters and V2FortRule declares one, and additionalProperties:false turns a leaked field into a 422 the fallback silently papers over. TrackingV2TypeTranslationTests asserts each table against the schema's property list in both directions. Verified against the live 5.2.1 instance rather than the Go source, per CLAUDE.md. Every translated body was POSTed and PUT at it and the v1 read diffed: byte-identical but for the rotated uid. Three traps came out of that, all now handled -- egg level is required with minimum 1, fort has no clean/edit/summary at all, and fort include_empty defaults TRUE on v2 where v1 defaults FALSE, so omitting it flipped a stored 0 to 1. Sentinels turn out to store exactly as v1 stores them despite the migration guide, so they are sent verbatim rather than omitted. Two prerequisites landed with it. PoracleServerProfileService.GetAsync is single-flighted behind a static gate -- it is registered transient, a cold cache costs a /health GET plus a schema_migrations SELECT, and a dashboard load now sends several version-gated writes at once. The absent-route flag is keyed per type, so one gin 404 cannot drop the other eight, and for incident (whose only surface is v2) a shared flag would mean the type vanishing. Eight alarm lists gain a stable content order. They rendered PoracleNG's insertion order, so an edited card would have jumped to the end of the grid now that an edit rotates the uid. --- .../fort-change-list.component.ts | 3 +- .../app/modules/gyms/gym-list.component.ts | 5 +- .../app/modules/lures/lure-list.component.ts | 3 +- .../max-battles/max-battle-list.component.ts | 5 +- .../app/modules/nests/nest-list.component.ts | 5 +- .../modules/quests/quest-list.component.ts | 3 +- .../app/modules/raids/raid-list.component.ts | 5 +- .../src/app/shared/utils/alarm-order.spec.ts | 65 +++ .../src/app/shared/utils/alarm-order.ts | 45 ++ CHANGELOG.md | 2 + CLAUDE.md | 112 +++- .../Services/IPoracleTrackingProxy.cs | 20 + .../EggService.cs | 9 + .../FortChangeService.cs | 9 + .../GymService.cs | 9 + .../LureService.cs | 16 +- .../MaxBattleService.cs | 10 + .../NestService.cs | 9 + .../PoracleServerProfileService.cs | 47 +- .../PoracleTrackingProxy.cs | 72 ++- .../QuestService.cs | 9 + .../RaidService.cs | 9 + .../TrackingV2Replacement.cs | 55 ++ .../TrackingV2Translator.cs | 538 ++++++++++++++---- .../UserOwnedOverrideAreaProxy.cs | 56 +- .../Services/AlarmServiceV2UpdatePathTests.cs | 160 ++++++ .../Services/PoracleServerProfileTests.cs | 40 ++ .../Services/PoracleTrackingProxyV2Tests.cs | 39 +- .../TrackingV2TypeTranslationTests.cs | 362 ++++++++++++ 29 files changed, 1515 insertions(+), 207 deletions(-) create mode 100644 Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.spec.ts create mode 100644 Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.ts create mode 100644 Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Replacement.cs create mode 100644 Tests/Pgan.PoracleWebNet.Tests/Services/AlarmServiceV2UpdatePathTests.cs create mode 100644 Tests/Pgan.PoracleWebNet.Tests/Services/TrackingV2TypeTranslationTests.cs diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/fort-changes/fort-change-list.component.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/fort-changes/fort-change-list.component.ts index 7a458c9d..9e98ba29 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/fort-changes/fort-change-list.component.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/fort-changes/fort-change-list.component.ts @@ -22,6 +22,7 @@ import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/componen import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component'; import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component'; import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component'; +import { orderAlarms } from '../../shared/utils/alarm-order'; import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope'; @Component({ @@ -249,7 +250,7 @@ export class FortChangeListComponent implements OnInit { .subscribe({ error: () => this.loading.set(false), next: items => { - this.fortChanges.set(items); + this.fortChanges.set(orderAlarms(items, f => [f.fortType, f.changeTypes.join(',')])); this.loading.set(false); }, }); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/gyms/gym-list.component.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/gyms/gym-list.component.ts index 78197195..620828bf 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/gyms/gym-list.component.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/gyms/gym-list.component.ts @@ -22,10 +22,11 @@ import { ScannerService } from '../../core/services/scanner.service'; import { TestAlertService } from '../../core/services/test-alert.service'; import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/components/confirm-dialog/confirm-dialog.component'; import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component'; -import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component'; import { QuietChipComponent } from '../../shared/components/quiet-chip/quiet-chip.component'; +import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component'; import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component'; import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component'; +import { orderAlarms } from '../../shared/utils/alarm-order'; import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope'; @Component({ @@ -264,7 +265,7 @@ export class GymListComponent implements OnInit { .subscribe({ error: () => this.loading.set(false), next: g => { - this.gyms.set(g); + this.gyms.set(orderAlarms(g, x => [x.team, x.gymId])); this.loading.set(false); this.resolveGymNames(g); }, diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/lures/lure-list.component.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/lures/lure-list.component.ts index 2a744dc0..f6551d17 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/lures/lure-list.component.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/lures/lure-list.component.ts @@ -24,6 +24,7 @@ import { DistanceDialogComponent } from '../../shared/components/distance-dialog import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component'; import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component'; import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component'; +import { orderAlarms } from '../../shared/utils/alarm-order'; import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope'; @Component({ @@ -272,7 +273,7 @@ export class LureListComponent implements OnInit { .subscribe({ error: () => this.loading.set(false), next: l => { - this.lures.set(l); + this.lures.set(orderAlarms(l, x => [x.lureId])); this.loading.set(false); }, }); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/max-battles/max-battle-list.component.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/max-battles/max-battle-list.component.ts index 3641b455..9a88f8ec 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/max-battles/max-battle-list.component.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/max-battles/max-battle-list.component.ts @@ -21,9 +21,10 @@ import { MaxBattleService } from '../../core/services/max-battle.service'; import { AlarmInfoComponent } from '../../shared/components/alarm-info/alarm-info.component'; import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/components/confirm-dialog/confirm-dialog.component'; import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component'; -import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component'; import { QuietChipComponent } from '../../shared/components/quiet-chip/quiet-chip.component'; +import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component'; import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component'; +import { orderAlarms } from '../../shared/utils/alarm-order'; import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope'; @Component({ @@ -297,7 +298,7 @@ export class MaxBattleListComponent implements OnInit { this.loading.set(false); }, next: maxBattles => { - this.maxBattles.set(maxBattles); + this.maxBattles.set(orderAlarms(maxBattles, m => [m.pokemonId, m.level, m.form, m.stationId])); this.loading.set(false); }, }); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/nests/nest-list.component.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/nests/nest-list.component.ts index 79a4ccba..621a5cab 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/nests/nest-list.component.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/nests/nest-list.component.ts @@ -23,10 +23,11 @@ import { NestService } from '../../core/services/nest.service'; import { TestAlertService } from '../../core/services/test-alert.service'; import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/components/confirm-dialog/confirm-dialog.component'; import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component'; -import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component'; import { QuietChipComponent } from '../../shared/components/quiet-chip/quiet-chip.component'; +import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component'; import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component'; import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component'; +import { orderAlarms } from '../../shared/utils/alarm-order'; import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope'; import { isAutoDelete as cleanIsAutoDelete } from '../../shared/utils/clean-flags'; @@ -240,7 +241,7 @@ export class NestListComponent implements OnInit { .subscribe({ error: () => this.loading.set(false), next: n => { - this.nests.set(n); + this.nests.set(orderAlarms(n, x => [x.pokemonId, x.minSpawnAvg])); this.loading.set(false); }, }); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/quests/quest-list.component.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/quests/quest-list.component.ts index 7f4be21d..20f1bb06 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/quests/quest-list.component.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/quests/quest-list.component.ts @@ -27,6 +27,7 @@ import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/componen import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component'; import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component'; import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component'; +import { orderAlarms } from '../../shared/utils/alarm-order'; import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope'; @Component({ @@ -316,7 +317,7 @@ export class QuestListComponent implements OnInit { this.loading.set(false); }, next: quests => { - this.quests.set(quests); + this.quests.set(orderAlarms(quests, q => [q.rewardType, q.reward, q.amount])); this.loading.set(false); }, }); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/raids/raid-list.component.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/raids/raid-list.component.ts index e6212d6d..1273a099 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/raids/raid-list.component.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/raids/raid-list.component.ts @@ -32,6 +32,7 @@ import { RsvpPillComponent } from '../../shared/components/rsvp-pill/rsvp-pill.c import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component'; import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component'; import { LevelLabelPipe } from '../../shared/pipes/level-label.pipe'; +import { orderAlarms } from '../../shared/utils/alarm-order'; import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope'; import { NO_COSTUME } from '../../shared/utils/costumes'; @@ -406,8 +407,8 @@ export class RaidListComponent implements OnInit { this.loading.set(false); }, next: ([raids, eggs]) => { - this.raids.set(raids); - this.eggs.set(eggs); + this.raids.set(orderAlarms(raids, r => [r.pokemonId, r.level, r.form, r.gymId])); + this.eggs.set(orderAlarms(eggs, e => [e.level, e.team, e.gymId])); this.loading.set(false); this.resolveGymNames([...raids, ...eggs]); }, diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.spec.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.spec.ts new file mode 100644 index 00000000..bbd2a172 --- /dev/null +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.spec.ts @@ -0,0 +1,65 @@ +import { orderAlarms } from './alarm-order'; + +describe('orderAlarms', () => { + it('keeps a rule where it was after an edit gave it a new id', () => { + // The regression this exists for. PoracleNG 5.2.0 replaces a rule rather than updating it, so the + // edited row comes back with the highest id in the list and the card jumped to the end of the grid. + const before = [ + { level: 5, pokemonId: 25, uid: 10 }, + { level: 5, pokemonId: 150, uid: 11 }, + { level: 5, pokemonId: 380, uid: 12 }, + ]; + const afterEditingMewtwo = [ + { level: 5, pokemonId: 25, uid: 10 }, + { level: 5, pokemonId: 380, uid: 12 }, + { level: 5, pokemonId: 150, uid: 99 }, + ]; + + const key = (r: { level: number; pokemonId: number }) => [r.pokemonId, r.level]; + + expect(orderAlarms(afterEditingMewtwo, key).map(r => r.pokemonId)).toEqual(orderAlarms(before, key).map(r => r.pokemonId)); + expect(orderAlarms(afterEditingMewtwo, key)[1].uid).toBe(99); + }); + + it('falls back to the id so the order is total', () => { + const items = [{ uid: 3 }, { uid: 1 }, { uid: 2 }]; + + expect(orderAlarms(items, () => []).map(i => i.uid)).toEqual([1, 2, 3]); + }); + + it('compares numbers as numbers and strings as strings', () => { + const items = [ + { uid: 1, value: 10 }, + { uid: 2, value: 9 }, + ]; + + expect(orderAlarms(items, i => [i.value]).map(i => i.uid)).toEqual([2, 1]); + expect( + orderAlarms( + [ + { name: 'b', uid: 1 }, + { name: 'a', uid: 2 }, + ], + i => [i.name], + ).map(i => i.uid), + ).toEqual([2, 1]); + }); + + it('treats a null or absent key as the empty string rather than throwing', () => { + // gymId, stationId and fortType are all nullable, and "any gym" is the common case. + const items = [ + { gymId: 'abc', uid: 1 }, + { gymId: null, uid: 2 }, + ]; + + expect(orderAlarms(items, i => [i.gymId]).map(i => i.uid)).toEqual([2, 1]); + }); + + it('does not mutate the list it was given', () => { + const items = [{ uid: 3 }, { uid: 1 }]; + + orderAlarms(items, () => []); + + expect(items.map(i => i.uid)).toEqual([3, 1]); + }); +}); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.ts new file mode 100644 index 00000000..a9f2cf07 --- /dev/null +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.ts @@ -0,0 +1,45 @@ +/** + * A stable display order for an alarm list, independent of the rule id. + * + * PoracleNG returns tracking rows in id order, and eight of the ten lists rendered that order straight + * through. On PoracleNG 5.2.0 and newer an edit is a replace: the rule comes back under a new, higher id, + * so the card the user just saved jumped to the end of the grid and, on a long list, off the screen — + * with nothing to say it had moved. + * + * Ordering on the rule's own content instead keeps a card where it was, and is a better order than + * insertion sequence regardless. The keys are the fields the card is titled by, in their raw form: dex + * number rather than species name, reward type rather than reward name. Sorting on the resolved name + * would mean re-ordering the grid once the master data and the gym names arrive, which is a worse flicker + * than the problem being fixed — and the resolved name changes with the display language, so the order + * would too. + * + * Invasion is deliberately absent: it stays on PoracleNG's v1 write surface, so its ids do not rotate. + */ +export function orderAlarms( + items: readonly T[], + key: (item: T) => readonly (number | string | null | undefined)[], +): T[] { + return [...items].sort((a, b) => { + const left = key(a); + const right = key(b); + + for (let i = 0; i < left.length; i++) { + const difference = compare(left[i], right[i]); + if (difference !== 0) { + return difference; + } + } + + // Two rules that are alike in everything the card shows. The id is the last tiebreak, so the order is + // total and a re-render cannot shuffle them against each other. + return a.uid - b.uid; + }); +} + +function compare(a: number | string | null | undefined, b: number | string | null | undefined): number { + if (typeof a === 'number' && typeof b === 'number') { + return a - b; + } + + return String(a ?? '').localeCompare(String(b ?? '')); +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fe31515..9a388b48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **A refused alarm explains itself the same way whichever Poracle surface answered.** The v2 write path already read PoracleNG's newer RFC 9457 error bodies and named the individual field it refused; the older v1 path, still the one most installs use, was reading only the older shape and answering a validation refusal as though the server had broken. Both paths now share one reader, and where many fields are refused at once the message names the first few and counts the rest rather than rendering a dozen clauses into a snackbar ([#803](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/803)). +- **Alarm lists keep a card where it was after you edit it.** Raids, eggs, quests, lures, nests, gyms, max battles and fort changes were rendered in the order Poracle returned them, which is the order the rules were created. On PoracleNG 5.2.0 and newer an edit replaces the rule rather than updating it, so the card you had just saved would have jumped to the end of the grid — off the screen entirely on a long list, with nothing to say it had moved. Each of those lists now has its own order, on what the card is titled by: Pokémon and level for raids and max battles, level for eggs, reward for quests, lure type, species for nests, team for gyms, change type for fort changes. The Pokemon list already sorted itself and is unchanged. +- **Eight more alarm types have their edits written through PoracleNG 5.2.1's strict `/api/v2` surface, and lure edits stop being risky.** Raid, egg, quest, nest, gym, max battle, fort change and lure join Pokemon on the newer write path, which addresses a rule by its id and replaces it in place. For most of them nothing changes on screen. Lure is the exception worth naming: PoracleNG's older surface has no way to update a lure alarm at all, so editing one had to delete the rule, re-create it, and put the original back if that failed — a sequence with a window in which the alarm did not exist. Max battle edits carried the same window. Neither does now. Invasion alarms stay on the older surface deliberately: the new one cannot report which grunt a rule targets, so an edit could not be written back faithfully. Anything older than PoracleNG 5.2.0 keeps the path it has always used, unchanged, and an edit carrying something the new surface cannot express — a role mention, an egg with no level, a fort rule missing its empty-changes setting — takes the old path rather than failing. Set `PORACLE_TRACKING_API_VERSION` to `v1` or `v2` to pin it. - **Pokemon alarm edits are written through PoracleNG 5.2.1’s strict `/api/v2` surface, where the server can tell an edit apart from a takeover.** Nothing changes on screen. What changes is underneath: an edit now addresses the rule by its id, so Poracle refuses outright if the uid is not yours or if the result would duplicate an alarm you already have, instead of PoracleWeb.NET having to work that out from a success response and undo it afterwards. Poracle also explains a rejected filter field by name now, so the message on the dialog says which one. Anything older than 5.2.0 keeps the surface it has always used, unchanged, and so do the other nine alarm types; an edit carrying anything the new surface cannot express takes the old path rather than failing. Set `PORACLE_TRACKING_API_VERSION` to `v1` or `v2` to pin it ([#805](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/805)). ### Fixed diff --git a/CLAUDE.md b/CLAUDE.md index 0908585a..0bd3d391 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -587,48 +587,94 @@ When comparing, **a field PoracleWeb does not supply cannot be compared** — Po See #462, #463, #531, #553, #561. -### The `/api/v2` Pilot: Pokemon Edits Only, And Pokemon Now Rotates Its uid +### `/api/v2` Writes: Nine Types, Edits Only, And Every Type Rotates Its uid -PoracleNG 5.2.0 added a second tracking surface. `PUT /api/v2/humans/{id}/tracking/pokemon/{uid}` is +PoracleNG 5.2.0 added a second tracking surface. `PUT /api/v2/humans/{id}/tracking/{type}/{uid}` is addressed by uid: it 404s when the uid is not that human's and 409s when the replacement would exactly duplicate another rule, so the server enforces what `EnsureNoMergeIntoAnotherAlarmAsync` had to reconstruct from a 200. **v2 POST still diffs and merges** — the #561 takeover reproduces on it — so creates stay on v1 and the reconciler stays. -**Only `MonsterService.UpdateAsync` uses it.** Everything else — every read, `CreateAsync`, -`BulkCreateAsync`, both distance endpoints, and all nine other types — is unchanged on v1, which 5.2.1 -left frozen. Reads deliberately stay on v1: v2 answers `null` for every field at its wildcard where v1 -answers the sentinel, and both `Monster` (C#) and `Monster` (TS) are built on the sentinels. Rebuilding -them from nulls means a per-field default table that must match PoracleNG exactly, and one wrong entry -silently rewrites a filter on the user's next save. - -**The v2 PUT is delete-then-insert, so pokemon now rotates its uid on edit like the other nine.** It was -the one exception, and three places in this file used to say so. `MonsterService` therefore takes -`ITrackedUidRemapper`, and `TrackedUidRemapperCoverageTests` lists it among the rotating services rather -than exempting it. Quick-pick applied state is the thing that actually breaks without the remap (#403). -Note that `EnsureNoMergeIntoAnotherAlarmAsync` already early-returned for pokemon *updates* (#606), so -moving to v2 removes no guard that was running. - -Three shape differences, all handled once at the wire in `TrackingV2Translator`: - -| v1 | v2 | +**Only `UpdateAsync` uses it, on nine of the ten types.** Everything else — every read, `CreateAsync`, +`BulkCreateAsync`, both distance endpoints and cleaning — is unchanged on v1, which 5.2.1 left frozen. +Reads deliberately stay on v1: v2 answers `null` for every field at its wildcard where v1 answers the +sentinel, and the C# and TypeScript models are both built on the sentinels. Rebuilding them from nulls +means a per-field default table that must match PoracleNG exactly, and one wrong entry silently rewrites +a filter on the user's next save. Bulk distance and cleaning stay on v1 because v2 has no bulk write: +305 uid-addressed PUTs at 7.8ms would replace one 20ms POST. + +**Invasion is the tenth and stays on v1 in both directions.** A v2 read of a named-grunt rule comes back +with no targeting field at all, so a GET-then-PUT round-trip 422s, and PoracleWeb holds only `GruntType` +as a string — live data fills it with values it cannot reverse into a `type_id` or `grunt_id`: `blanche`, +`candela`, `spark`, `npc 0`…`npc 10`, `player team leader`. Filed upstream. `TrackingV2Translator.Handles` +is the single place that decides, and having no field table for a type is what keeps it on v1. + +**The v2 PUT is delete-then-insert, so every type rotates its uid on edit.** Pokemon was the one exception +and no longer is. Quick-pick applied state is the thing that actually breaks without the remap (#403), so +every service hands `ITrackedUidRemapper` to `TrackingV2Replacement.TryApplyAsync`, the shared v2 branch +each `UpdateAsync` takes before its own v1 path. + +**Taking the v2 branch skips the whole v1 repair path, as a unit.** That is the point of moving, and it is +where the wins are: + +| Type | What the v1 path does that v2 makes unnecessary | |---|---| -| `clean` 3-bit mask | separate `clean` / `edit` / `summary` booleans | -| `gender` 0-3 | `any` / `male` / `female` / `genderless` | -| `pvp_ranking_league` any int | enum of `{0, 500, 1500, 2500}` | - -Plus `uid`, `id`, `profile_no`, `ping` and `description`, which v2 has no place for and refuses outright: -`V2PokemonRule` sets `additionalProperties: false`, so one stray property is a 422 and the write fails. +| lure | `NaturalKeyTrackingUpdate` deletes the row to free `lure_tracking(id, profile_no, lure_id)`, re-creates it, and restores the original if that fails — PoracleNG's v1 create has no upsert path for it | +| maxbattle | delete-then-create, with a window where the alarm exists nowhere | +| the other seven | `TrackingUpdateReconciler.ReconcileAsync` cleans up the duplicate a v1 create leaves behind | + +The pre-write guards stay. `EnsureNoMergeIntoAnotherAlarmAsync`, lure's sibling `lure_id` check and max +battle's identical-alarm check all run before the v2 attempt: v2's POST still merges, and the 409 covers +only an exact duplicate. + +**One field table per type in `TrackingV2Translator`, never a shared one.** `V2PokemonRule` declares 28 +integer filters and `V2FortRule` declares one; every rule sets `additionalProperties: false`, so a leaked +field is a 422 that the v1 fallback silently papers over — the failure nobody notices. +`TrackingV2TypeTranslationTests` asserts each table against the schema's property list in both directions, +so a field the table leaks and a field it misses both fail the build. + +Shape differences, all handled once at the wire: + +| v1 | v2 | Types | +|---|---|---| +| `clean` 3-bit mask | `clean` / `edit` / `summary` booleans | all but fort | +| `gender` 0-3 | `any` / `male` / `female` / `genderless` | pokemon | +| `team` 0-4 | `harmony` / `mystic` / `valor` / `instinct` / `any` | raid, egg, gym | +| `rsvp_changes` 0-2 | `none` / `rsvp` / `rsvp_only` | raid, egg | +| 0/1 columns | real booleans | `exclusive`, `slot_changes`, `battle_changes`, `gmax`, `shiny`, `include_empty` | +| `change_types` JSON string | array | fort | +| `pvp_ranking_league` any int | enum of `{0, 500, 1500, 2500}` | pokemon | + +Plus `uid`, `id`, `profile_no`, `ping` and `description`, which v2 has no place for and refuses outright. The v1 shape stays the single internal currency — `TrackingFieldPreserver`, `TrackingUpdateReconciler`, `BulkUidRemap` and `QuickPickService` all build and compare it — and the translator is the only exit onto v2, which is what stops a v1-shaped row reaching a v2 body. -**The translator never changes what PoracleNG will accept.** A property it does not know, a gender outside -0-3, a league outside the enum: it answers false and the row goes to v1. Refusing would mean a newer -PoracleNG broke every pokemon edit; dropping the field would be #730 again. +**Sentinels are sent verbatim, against the migration guide's advice.** The guide says to omit `level: 9000`, +`costume: 9000`, `move: 9000` and the rest. Verified on 5.2.1 instead: a raid, an egg and a max battle +written through both surfaces produced byte-identical v1 reads but for the rotated uid — the v2 *response* +reports them as null, the row does not. Omitting them would leave `CountUpdatableDifferences` comparing a +stored 9000 against an absent field. + +Three per-type traps, all verified live and each a 422 if ignored: + +- **egg `level` is required with minimum 1**, and `Egg.Level` is a plain int defaulting to 0. Profile + import, quick-pick apply and the cleaning fetch-mutate-POST all build eggs without one, so the + translator declines and they go to v1, which has stored level 0 for years. +- **fort has no `clean`, `edit` or `summary`** — no v2 field and no column. A shared clean helper applied + blindly is a 422. +- **fort `include_empty` defaults to TRUE on v2 and FALSE on v1.** A PUT that omitted it flipped a stored + 0 to 1 and the alert text gained "including empty changes". The translator states it explicitly and + declines a fort row that carries none. + +**The translator never changes what PoracleNG will accept.** A property it does not know, an enum outside +its range, an egg without a level: it answers false and the row goes to v1. Refusing would mean a newer +PoracleNG broke every edit; dropping the field would be #730 again. **A v2 PUT is a full replace** — omitting `min_iv` wipes a stored 90, verified live — so -`TrackingFieldPreserver` matters more here than it did on v1, not less. +`TrackingFieldPreserver` matters more here than it did on v1, not less. `UserOwnedOverrideAreaProxy` +decorates the v2 replace too, and has to write its areas back to the uid PoracleNG just reported rather +than the one addressed, or the alarm silently widens from one geofence to the whole profile. Errors are RFC 9457 problem+json at **422**, not 400, in two shapes: a schema failure carries `errors[]` whose `location` is `body.x` on a PUT and `body[0].x` on a POST, and a semantic refusal carries only @@ -641,8 +687,11 @@ Gating: `PoracleServerProfile.SupportsV2Tracking` is version >= 5.2.0. Not the ` though `UpstreamFeatureFlagService` deliberately fails the other way. `Poracle:TrackingApiVersion` (`auto` | `v1` | `v2`, env `PORACLE_TRACKING_API_VERSION`) pins it for a fork whose version says the wrong thing, and the proxy falls back to v1 for five minutes when the route answers gin's plaintext -`404 page not found` — that fallback is the only thing standing between a downgraded server and an outage -window the length of the profile cache. +`404 page not found`. **That flag is keyed per type**: one absent route must not drop the other eight, and +for `incident` — whose only surface is v2 — a shared flag would mean the type vanishing rather than +degrading. `PoracleServerProfileService.GetAsync` is single-flighted behind a static gate, because it is +registered transient, a cold cache costs a `/health` GET plus a `schema_migrations` SELECT, and a +dashboard load now sends several version-gated writes at once. ### Keep the PoracleNG Checkout Pinned To What Prod Runs @@ -865,6 +914,7 @@ dotnet ef migrations script \ | PoracleHumanProxy | `Core/Pgan.PoracleWebNet.Core.Services/PoracleHumanProxy.cs` | | PoracleJsonHelper | `Core/Pgan.PoracleWebNet.Core.Services/PoracleJsonHelper.cs` | | TrackingV2Translator (v1 row -> /api/v2 body) | `Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs` | +| TrackingV2Replacement (the v2 branch each UpdateAsync takes) | `Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Replacement.cs` | | PoracleProblemDetails (RFC 9457 + v1 errors) | `Core/Pgan.PoracleWebNet.Core.Services/PoracleProblemDetails.cs` | | Repositories (non-alarm) | `Core/Pgan.PoracleWebNet.Core.Repositories/` | | SiteSettingRepository | `Core/Pgan.PoracleWebNet.Core.Repositories/SiteSettingRepository.cs` | diff --git a/Core/Pgan.PoracleWebNet.Core.Abstractions/Services/IPoracleTrackingProxy.cs b/Core/Pgan.PoracleWebNet.Core.Abstractions/Services/IPoracleTrackingProxy.cs index 23165ce3..17130ee3 100644 --- a/Core/Pgan.PoracleWebNet.Core.Abstractions/Services/IPoracleTrackingProxy.cs +++ b/Core/Pgan.PoracleWebNet.Core.Abstractions/Services/IPoracleTrackingProxy.cs @@ -43,6 +43,26 @@ public interface IPoracleTrackingProxy public Task UpdateByUidAsync( string type, string userId, int uid, System.Text.Json.JsonElement body); + /// + /// Full-replaces one existing tracking alarm through /api/v2, or answers null when that surface + /// cannot be used for this write. + /// + /// + /// + /// Null is the ordinary answer, not a fault: the type has no v2 field table, the operator pinned v1, + /// the server does not carry the route, or the row holds something v2 could not be told faithfully + /// (see TrackingV2Translator). The caller then takes its own v1 path, which is why this is + /// separate from — every alarm service wraps its v1 update in guards + /// and repairs that the v2 PUT makes unnecessary, and those have to be skipped as a unit rather than + /// run against a write that already happened. + /// + /// + /// is the same single v1-shaped alarm object every other write takes. + /// + /// + public Task TryReplaceV2Async( + string type, string userId, int uid, System.Text.Json.JsonElement body); + /// /// Deletes a single tracking alarm by UID. /// Maps to DELETE /api/tracking/{type}/{userId}/byUid/{uid} diff --git a/Core/Pgan.PoracleWebNet.Core.Services/EggService.cs b/Core/Pgan.PoracleWebNet.Core.Services/EggService.cs index 431ba563..b6b64c72 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/EggService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/EggService.cs @@ -63,6 +63,15 @@ public async Task UpdateAsync(string userId, Egg model) await TrackingUpdateReconciler.EnsureNoMergeIntoAnotherAlarmAsync( this._proxy, TrackingType, userId, oldUid, body); + // /api/v2 replaces the rule in place, addressed by its uid, and answers 409 itself when the + // replacement would duplicate another rule. None of the v1 repair below applies to it. + if (await TrackingV2Replacement.TryApplyAsync( + this._proxy, TrackingType, userId, oldUid, body, this._uidRemapper) is { } v2Uid) + { + model.Uid = v2Uid; + return model; + } + var result = await this._proxy.CreateAsync(TrackingType, userId, body); // PoracleNG inserts instead of upserting when the edit changes a dedup-key field, diff --git a/Core/Pgan.PoracleWebNet.Core.Services/FortChangeService.cs b/Core/Pgan.PoracleWebNet.Core.Services/FortChangeService.cs index e1c8be5f..76c04a0b 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/FortChangeService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/FortChangeService.cs @@ -75,6 +75,15 @@ public async Task UpdateAsync(string userId, FortChange model) await TrackingUpdateReconciler.EnsureNoMergeIntoAnotherAlarmAsync( this._proxy, TrackingType, userId, oldUid, body); + // /api/v2 replaces the rule in place, addressed by its uid, and answers 409 itself when the + // replacement would duplicate another rule. None of the v1 repair below applies to it. + if (await TrackingV2Replacement.TryApplyAsync( + this._proxy, TrackingType, userId, oldUid, body, this._uidRemapper) is { } v2Uid) + { + model.Uid = v2Uid; + return model; + } + var result = await this._proxy.CreateAsync(TrackingType, userId, body); // PoracleNG inserts instead of upserting when the edit changes a dedup-key field, diff --git a/Core/Pgan.PoracleWebNet.Core.Services/GymService.cs b/Core/Pgan.PoracleWebNet.Core.Services/GymService.cs index e4467547..d943bf5e 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/GymService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/GymService.cs @@ -61,6 +61,15 @@ public async Task UpdateAsync(string userId, Gym model) await TrackingUpdateReconciler.EnsureNoMergeIntoAnotherAlarmAsync( this._proxy, TrackingType, userId, oldUid, body); + // /api/v2 replaces the rule in place, addressed by its uid, and answers 409 itself when the + // replacement would duplicate another rule. None of the v1 repair below applies to it. + if (await TrackingV2Replacement.TryApplyAsync( + this._proxy, TrackingType, userId, oldUid, body, this._uidRemapper) is { } v2Uid) + { + model.Uid = v2Uid; + return model; + } + var result = await this._proxy.CreateAsync(TrackingType, userId, body); // PoracleNG inserts instead of upserting when the edit changes a dedup-key field, diff --git a/Core/Pgan.PoracleWebNet.Core.Services/LureService.cs b/Core/Pgan.PoracleWebNet.Core.Services/LureService.cs index 45aad22d..65a41afb 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/LureService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/LureService.cs @@ -76,14 +76,26 @@ public async Task UpdateAsync(string userId, Lure model) } } - var original = oldUid > 0 ? await this.GetByUidAsync(userId, oldUid) : null; - var body = SerializeToElement(model); // Carry forward anything the stored row holds that the model does not declare. See #730. body = await TrackingFieldPreserver.PreserveStoredFieldsAsync( this._proxy, TrackingType, userId, oldUid, body); + // /api/v2's PUT is addressed by uid and replaces the row rather than inserting beside it, so the + // natural key is never in contention and none of the delete-create-restore below is needed -- + // which is the single biggest reason to move this type. Verified live on 5.2.1: a PUT changing + // only the distance of lure 266 replaced it as 267, where the v1 create-carrying-a-uid inserts a + // second row and leaves 266 behind. + if (await TrackingV2Replacement.TryApplyAsync( + this._proxy, TrackingType, userId, oldUid, body, this._uidRemapper) is { } v2Uid) + { + model.Uid = v2Uid; + return model; + } + + var original = oldUid > 0 ? await this.GetByUidAsync(userId, oldUid) : null; + model.Uid = await NaturalKeyTrackingUpdate.ReplaceAsync( this._proxy, TrackingType, diff --git a/Core/Pgan.PoracleWebNet.Core.Services/MaxBattleService.cs b/Core/Pgan.PoracleWebNet.Core.Services/MaxBattleService.cs index f7463d2d..f285022a 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/MaxBattleService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/MaxBattleService.cs @@ -78,6 +78,16 @@ public async Task UpdateAsync(string userId, MaxBattle model) "You already have an identical max battle alarm."); } + // /api/v2 replaces the rule in place, addressed by its uid, so this type stops being insert-only + // and the delete-then-create above it is skipped entirely -- with it, the window where the alarm + // exists nowhere. + if (await TrackingV2Replacement.TryApplyAsync( + this._proxy, TrackingType, userId, oldUid, body, this._uidRemapper) is { } v2Uid) + { + model.Uid = v2Uid; + return model; + } + await this._proxy.DeleteByUidAsync(TrackingType, userId, oldUid); var result = await this._proxy.CreateAsync(TrackingType, userId, body); diff --git a/Core/Pgan.PoracleWebNet.Core.Services/NestService.cs b/Core/Pgan.PoracleWebNet.Core.Services/NestService.cs index 5d66bc8c..75721ad7 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/NestService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/NestService.cs @@ -61,6 +61,15 @@ public async Task UpdateAsync(string userId, Nest model) await TrackingUpdateReconciler.EnsureNoMergeIntoAnotherAlarmAsync( this._proxy, TrackingType, userId, oldUid, body); + // /api/v2 replaces the rule in place, addressed by its uid, and answers 409 itself when the + // replacement would duplicate another rule. None of the v1 repair below applies to it. + if (await TrackingV2Replacement.TryApplyAsync( + this._proxy, TrackingType, userId, oldUid, body, this._uidRemapper) is { } v2Uid) + { + model.Uid = v2Uid; + return model; + } + var result = await this._proxy.CreateAsync(TrackingType, userId, body); // PoracleNG inserts instead of upserting when the edit changes a dedup-key field, diff --git a/Core/Pgan.PoracleWebNet.Core.Services/PoracleServerProfileService.cs b/Core/Pgan.PoracleWebNet.Core.Services/PoracleServerProfileService.cs index 2ec0c57a..d9d1defd 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/PoracleServerProfileService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/PoracleServerProfileService.cs @@ -38,6 +38,18 @@ public partial class PoracleServerProfileService( /// private static readonly TimeSpan CacheFor = TimeSpan.FromMinutes(5); + /// + /// One probe at a time, process-wide. + /// + /// + /// A cold cache costs an HTTP /health GET and a schema_migrations SELECT, and the callers + /// arrive in parallel: every version-gated tracking write consults this, and a dashboard load fires + /// several at once. Without the gate they all miss the same empty cache and each runs the pair. The + /// gate is static because AddHttpClient registers this transient, so an instance field would + /// serialise nothing; the cache it guards is the shared singleton either way. + /// + private static readonly SemaphoreSlim ProbeGate = new(1, 1); + private readonly HttpClient _httpClient = httpClient; private readonly IPoracleSchemaVersionReader _schemaReader = schemaReader; private readonly IMemoryCache _cache = cache; @@ -47,15 +59,42 @@ public partial class PoracleServerProfileService( /// public async Task GetAsync(CancellationToken cancellationToken = default) { - if (this._cache.TryGetValue(CacheKey, out PoracleServerProfile? cached) && cached is not null) + if (this.TryGetCached(out var cached)) { return cached; } - var profile = await this.ProbeAsync(cancellationToken); - this._cache.Set(CacheKey, profile, CacheFor); + await ProbeGate.WaitAsync(cancellationToken); + + try + { + // Someone else may have probed while this call was queued behind the gate. + if (this.TryGetCached(out cached)) + { + return cached; + } + + var profile = await this.ProbeAsync(cancellationToken); + this._cache.Set(CacheKey, profile, CacheFor); + + return profile; + } + finally + { + ProbeGate.Release(); + } + } + + private bool TryGetCached(out PoracleServerProfile profile) + { + if (this._cache.TryGetValue(CacheKey, out PoracleServerProfile? cached) && cached is not null) + { + profile = cached; + return true; + } - return profile; + profile = null!; + return false; } /// diff --git a/Core/Pgan.PoracleWebNet.Core.Services/PoracleTrackingProxy.cs b/Core/Pgan.PoracleWebNet.Core.Services/PoracleTrackingProxy.cs index 6643b56d..1fee3ccb 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/PoracleTrackingProxy.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/PoracleTrackingProxy.cs @@ -17,18 +17,16 @@ public partial class PoracleTrackingProxy( ILogger logger) : IPoracleTrackingProxy { /// - /// The only type this build writes through /api/v2. See #805 — the other nine stay on the - /// frozen v1 surface, which 5.2.1 left unchanged, so leaving them is a no-op rather than a deferred - /// defect. Each needs its own field translation derived from its own schema. + /// Set when the v2 route for one type answered gin's plaintext 404, meaning this server does not carry + /// it whatever its version said. Held for as long as the server profile is cached, so an upgrade is + /// picked up on the same clock as everything else version-gated. /// - private const string V2PilotType = "pokemon"; - - /// - /// Set when the v2 route answered gin's plaintext 404, meaning this server does not carry it whatever - /// its version said. Held for as long as the server profile is cached, so an upgrade is picked up on - /// the same clock as everything else version-gated. - /// - private const string V2AbsentCacheKey = "poracle:v2-tracking-absent"; + /// + /// Keyed per type on purpose. A single flag let one 404 from one route drop every type back to v1 — + /// which for incident, whose only surface is v2, would mean the type vanishing rather than + /// degrading. + /// + private static string V2AbsentCacheKey(string type) => $"poracle:v2-tracking-absent:{type}"; private static readonly TimeSpan V2AbsentFor = TimeSpan.FromMinutes(5); @@ -130,26 +128,33 @@ public async Task CreateAsync(string type, string userId, root.TryGetProperty("insert", out var ins) ? ins.GetInt32() : 0); } + /// + public async Task TryReplaceV2Async( + string type, string userId, int uid, JsonElement body) + { + if (uid <= 0 || !this.ShouldTryV2(type) || !await this.ServerCarriesV2Async(type)) + { + return null; + } + + if (!TrackingV2Translator.TryTranslate(type, body, out var v2Body, out var unsupported)) + { + // Not a fault. The row carries something v2 has no faithful place for, so it goes to v1, + // which stores whatever it is given. See TrackingV2Translator. + LogV2Untranslatable(this._logger, type, uid, unsupported ?? "unknown"); + return null; + } + + return await this.PutV2Async(type, userId, uid, v2Body); + } + /// public async Task UpdateByUidAsync( string type, string userId, int uid, JsonElement body) { - if (uid > 0 && this.ShouldTryV2(type) && await this.ServerCarriesV2Async()) + if (await this.TryReplaceV2Async(type, userId, uid, body) is { } replaced) { - if (TrackingV2Translator.TryTranslatePokemon(body, out var v2Body, out var unsupported)) - { - var applied = await this.PutV2Async(type, userId, uid, v2Body); - if (applied is { } result) - { - return result; - } - } - else - { - // Not a fault. The row carries something v2 has no faithful place for, so it goes to v1, - // which stores whatever it is given. See TrackingV2Translator. - LogV2Untranslatable(this._logger, type, uid, unsupported ?? "unknown"); - } + return replaced; } // v1: an update is a create carrying the uid, which PoracleNG upserts. Byte-identical to what @@ -225,18 +230,23 @@ public async Task ReloadStateAsync() } /// Whether this type and this deployment are in scope for the v2 write path at all. + /// + /// Invasion is the one type with a v2 surface that PoracleWeb deliberately stays off. A v2 read of a + /// named-grunt rule carries no targeting field at all, and PoracleWeb holds only the grunt name, which + /// live data fills with values it cannot reverse into an id (blanche, npc 0, player + /// team leader). Filed upstream. + /// private bool ShouldTryV2(string type) => - string.Equals(type, V2PilotType, StringComparison.Ordinal) - && this._trackingApiVersion != "v1"; + TrackingV2Translator.Handles(type) && this._trackingApiVersion != "v1"; /// /// Whether the server is believed to carry v2. Pinned to v2 this skips the probe but not the /// runtime fallback, so pinning a server that turns out not to have the route degrades to v1 rather /// than failing every edit. /// - private async Task ServerCarriesV2Async() + private async Task ServerCarriesV2Async(string type) { - if (this._cache.TryGetValue(V2AbsentCacheKey, out _)) + if (this._cache.TryGetValue(V2AbsentCacheKey(type), out _)) { return false; } @@ -279,7 +289,7 @@ private async Task ServerCarriesV2Async() case HttpStatusCode.NotFound when !PoracleProblemDetails.IsProblemJson(payload): // gin answers a missing route with plaintext "404 page not found", so the route does not // exist on this build whatever /health claimed. Verified against 5.1.0. - this._cache.Set(V2AbsentCacheKey, true, V2AbsentFor); + this._cache.Set(V2AbsentCacheKey(type), true, V2AbsentFor); LogV2RouteAbsent(this._logger, type); return null; diff --git a/Core/Pgan.PoracleWebNet.Core.Services/QuestService.cs b/Core/Pgan.PoracleWebNet.Core.Services/QuestService.cs index 320a313f..ebd855e7 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/QuestService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/QuestService.cs @@ -69,6 +69,15 @@ public async Task UpdateAsync(string userId, Quest model) await TrackingUpdateReconciler.EnsureNoMergeIntoAnotherAlarmAsync( this._proxy, TrackingType, userId, oldUid, body); + // /api/v2 replaces the rule in place, addressed by its uid, and answers 409 itself when the + // replacement would duplicate another rule. None of the v1 repair below applies to it. + if (await TrackingV2Replacement.TryApplyAsync( + this._proxy, TrackingType, userId, oldUid, body, this._uidRemapper) is { } v2Uid) + { + model.Uid = v2Uid; + return model; + } + var result = await this._proxy.CreateAsync(TrackingType, userId, body); // PoracleNG inserts instead of upserting when the edit changes a dedup-key field, diff --git a/Core/Pgan.PoracleWebNet.Core.Services/RaidService.cs b/Core/Pgan.PoracleWebNet.Core.Services/RaidService.cs index 4620a30d..6bb3190a 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/RaidService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/RaidService.cs @@ -78,6 +78,15 @@ public async Task UpdateAsync(string userId, Raid model) await TrackingUpdateReconciler.EnsureNoMergeIntoAnotherAlarmAsync( this._proxy, TrackingType, userId, oldUid, body); + // /api/v2 replaces the rule in place, addressed by its uid, and answers 409 itself when the + // replacement would duplicate another rule. None of the v1 repair below applies to it. + if (await TrackingV2Replacement.TryApplyAsync( + this._proxy, TrackingType, userId, oldUid, body, this._uidRemapper) is { } v2Uid) + { + model.Uid = v2Uid; + return model; + } + var result = await this._proxy.CreateAsync(TrackingType, userId, body); // PoracleNG inserts instead of upserting when the edit changes a dedup-key field, diff --git a/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Replacement.cs b/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Replacement.cs new file mode 100644 index 00000000..91f7a00f --- /dev/null +++ b/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Replacement.cs @@ -0,0 +1,55 @@ +using System.Text.Json; +using Pgan.PoracleWebNet.Core.Abstractions.Services; + +namespace Pgan.PoracleWebNet.Core.Services; + +/// +/// The v2 branch every alarm service takes before its v1 update path. +/// +/// +/// +/// PUT /api/v2/humans/{id}/tracking/{type}/{uid} is a genuine full replace addressed by uid, so it +/// needs none of the machinery the v1 create-carrying-a-uid does: no reconcile of a stray insert, no +/// delete-first to free a natural key, no restore-on-failure. Skipping those as a unit is the point — +/// running any of them against a write that already landed would be worse than not moving at all. +/// +/// +/// The uid rotates, because v2's engine is delete-then-insert. Quick-pick applied state stores uids +/// captured at apply time and has to follow the row, or its "remove" button silently deletes nothing. +/// See #403. +/// +/// +internal static class TrackingV2Replacement +{ + /// + /// Replaces the rule through /api/v2 when that surface is available for this type, this server + /// and this row. + /// + /// + /// The uid the rule now lives under, or null when the caller should take its own v1 path — the type + /// has no v2 field table, the operator pinned v1, the route is absent, or the row carries something + /// v2 could not be told faithfully. + /// + public static async Task TryApplyAsync( + IPoracleTrackingProxy proxy, + string trackingType, + string userId, + int oldUid, + JsonElement body, + ITrackedUidRemapper uidRemapper) + { + if (await proxy.TryReplaceV2Async(trackingType, userId, oldUid, body) is not { } replaced) + { + return null; + } + + var newUid = replaced.Uid > 0 ? replaced.Uid : oldUid; + + if (newUid != oldUid) + { + await uidRemapper.RemapAsync(userId, trackingType, oldUid, newUid); + } + + return newUid; + } +} diff --git a/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs b/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs index 07a1ef67..def4eccf 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs @@ -4,29 +4,45 @@ namespace Pgan.PoracleWebNet.Core.Services; /// -/// Rewrites a v1-shaped pokemon row into the body /api/v2 will accept. +/// Rewrites a v1-shaped tracking row into the body /api/v2 will accept, for the nine types +/// PoracleWeb writes through that surface. /// /// /// /// The whole of PoracleWeb builds and compares alarm bodies in v1's shape — , /// , BulkUidRemap and QuickPickService all do. Keeping /// that shape as the single internal currency and translating once, at the wire, is what stops a v1-shaped -/// row leaking into a v2 request: V2PokemonRule sets additionalProperties: false, so one stray +/// row leaking into a v2 request: every V2*Rule sets additionalProperties: false, so one stray /// ping is a 422 and the write fails outright. Verified live against 5.2.1. /// /// -/// Three fields genuinely change shape. clean is a 3-bit mask on v1 and three booleans on v2 -/// (clean/edit/summary); gender is an int on v1 and -/// any|male|female|genderless on v2; pvp_ranking_league is any int on v1 and an enum of -/// {0, 500, 1500, 2500} on v2. Five more — uid, id, profile_no, ping, -/// description — are addressing and presentation rather than filter, and v2 has no place for them. +/// One field table per type, never a shared one. V2PokemonRule declares 28 integer filters and +/// V2FortRule declares one; V2FortRule has no clean/edit/summary at all. +/// A shared set would leak a field into a type that refuses it, and additionalProperties: false turns +/// that into a 422 the fallback silently papers over — the failure nobody notices. +/// +/// +/// Four kinds of field genuinely change shape. clean is a 3-bit mask on v1 and three booleans on v2; +/// several 0/1 columns (exclusive, slot_changes, battle_changes, gmax, +/// shiny, include_empty) become real booleans; three integer columns become string enums +/// (team, gender, rsvp_changes); and change_types is stored as a JSON string but +/// wanted as an array. Four more — uid, id, profile_no, description — are +/// addressing and presentation rather than filter, and v2 has no place for them. +/// +/// +/// Sentinels are sent verbatim. The migration guide says to omit them, but a v2 write carrying +/// level: 9000, costume: 9000, evolution: 9000 or move: 9000 stores exactly what +/// v1 stores — verified on 5.2.1 by writing a raid, an egg and a max battle through both surfaces and +/// diffing the v1 read, which came back byte-identical but for the rotated uid. The v2 response +/// reports them as null, which is what the guide describes; the row does not. Omitting them instead would +/// leave comparing a stored 9000 against an absent field. /// /// /// The translator never widens or narrows what PoracleNG will accept. Anything it cannot express -/// faithfully — a property it does not know, a gender outside 0-3, a league outside the enum — makes it +/// faithfully — a property it does not know, an enum outside its range, an egg without a level — makes it /// answer false, and the caller sends the row to the frozen v1 surface instead. Refusing outright would -/// mean a PoracleNG newer than this translator broke every pokemon edit; dropping the field silently would -/// be #730 all over again. Falling back does neither. +/// mean a PoracleNG newer than this translator broke every edit; dropping the field silently would be #730 +/// all over again. Falling back does neither. /// /// internal static class TrackingV2Translator @@ -35,50 +51,166 @@ internal static class TrackingV2Translator /// Addressing and presentation. v2 carries none of it in the rule body, and reconstructs all of it /// itself: uid/id/profile_no come from the route, and description is a /// display string PoracleNG computes rather than a stored column. ping is NOT here — it is a - /// real column that v2 blanks, so it is handled in . + /// real column that v2 blanks, so it is handled in . /// private static readonly HashSet Dropped = new(StringComparer.Ordinal) { "uid", "id", "profile_no", "description", }; - /// Every integer filter V2PokemonRule declares, taken from 5.2.1's openapi.golden.json. - private static readonly HashSet IntegerFields = new(StringComparer.Ordinal) - { - "atk", "costume", "def", "distance", "form", "max_atk", "max_cp", "max_def", "max_iv", - "max_level", "max_rarity", "max_size", "max_sta", "max_weight", "min_cp", "min_iv", - "min_level", "min_time", "min_weight", "pokemon_id", "pvp_ranking_best", "pvp_ranking_cap", - "pvp_ranking_evolution", "pvp_ranking_min_cp", "pvp_ranking_worst", "rarity", "size", "sta", - }; + /// v2's gender enum: index is the v1 integer. + private static readonly string[] Genders = ["any", "male", "female", "genderless"]; + + /// v2's team enum: index is the v1 integer, and 4 ("any") is PoracleWeb's default. + private static readonly string[] Teams = ["harmony", "mystic", "valor", "instinct", "any"]; + + /// v2's RSVP enum: index is the v1 integer. + private static readonly string[] RsvpChanges = ["none", "rsvp", "rsvp_only"]; /// The four leagues V2PokemonRule permits. 0 means "no PVP filter". private static readonly HashSet Leagues = [0, 500, 1500, 2500]; - /// v2's gender enum: index is the v1 integer. - private static readonly string[] Genders = ["any", "male", "female", "genderless"]; + /// The three fort types V2FortRule permits. + private static readonly HashSet FortTypes = new(StringComparer.Ordinal) + { + "pokestop", "gym", "everything", + }; + + /// + /// Every field each V2*Rule declares, taken from 5.2.1's openapi.golden.json, sorted into + /// how it has to be written. A schema field missing from its type's table would go to v1 forever + /// without anyone noticing, which is why TrackingV2SchemaCoverageTests asserts the tables against + /// the schema rather than against themselves. + /// + private static readonly Dictionary Specs = new(StringComparer.Ordinal) + { + ["pokemon"] = new TypeSpec + { + Integers = + [ + "atk", "costume", "def", "distance", "form", "max_atk", "max_cp", "max_def", "max_iv", + "max_level", "max_rarity", "max_size", "max_sta", "max_weight", "min_cp", "min_iv", + "min_level", "min_time", "min_weight", "pokemon_id", "pvp_ranking_best", "pvp_ranking_cap", + "pvp_ranking_evolution", "pvp_ranking_min_cp", "pvp_ranking_worst", "rarity", "size", "sta", + ], + IntEnums = new Dictionary(StringComparer.Ordinal) { ["gender"] = Genders }, + BoundedIntegers = new Dictionary>(StringComparer.Ordinal) + { + ["pvp_ranking_league"] = Leagues, + }, + + // v2 makes pokemon_id the one required field. A row without it could only ever 422. + Required = ["pokemon_id"], + }, + ["raid"] = new TypeSpec + { + Integers = ["costume", "distance", "evolution", "form", "level", "move", "pokemon_id"], + Strings = ["gym_id"], + Booleans = ["exclusive"], + IntEnums = new Dictionary(StringComparer.Ordinal) + { + ["team"] = Teams, + ["rsvp_changes"] = RsvpChanges, + }, + }, + ["egg"] = new TypeSpec + { + Integers = ["distance", "level"], + Strings = ["gym_id"], + Booleans = ["exclusive"], + IntEnums = new Dictionary(StringComparer.Ordinal) + { + ["team"] = Teams, + ["rsvp_changes"] = RsvpChanges, + }, + + // V2EggRule declares level required with minimum 1, and Egg.Level is a plain int defaulting to + // 0. A profile import, a quick-pick apply or a cleaning write-back that never set one builds a + // body v2 answers 422 to — verified live on 5.2.1. Those rows go to v1, which stores level 0 + // as it always has. + Required = ["level"], + PositiveIntegers = ["level"], + }, + ["quest"] = new TypeSpec + { + Integers = ["amount", "distance", "form", "reward", "reward_type"], + Booleans = ["shiny"], + Required = ["reward_type"], + }, + ["gym"] = new TypeSpec + { + Integers = ["distance"], + Strings = ["gym_id"], + Booleans = ["battle_changes", "slot_changes"], + IntEnums = new Dictionary(StringComparer.Ordinal) { ["team"] = Teams }, + Required = ["team"], + }, + ["maxbattle"] = new TypeSpec + { + Integers = ["distance", "evolution", "form", "level", "move", "pokemon_id"], + Strings = ["station_id"], + Booleans = ["gmax"], + }, + ["nest"] = new TypeSpec + { + Integers = ["distance", "form", "min_spawn_avg", "pokemon_id"], + }, + ["lure"] = new TypeSpec + { + Integers = ["distance", "lure_id"], + Required = ["lure_id"], + }, + ["fort"] = new TypeSpec + { + Integers = ["distance"], + Booleans = ["include_empty"], + StringArrays = ["change_types"], + AllowedStrings = new Dictionary>(StringComparer.Ordinal) + { + ["fort_type"] = FortTypes, + }, + + // V2FortRule has no clean/edit/summary — and fort_tracking has no column for them either. + HasCleanFlags = false, + + // include_empty defaults to TRUE on v2 and FALSE on v1. Verified live: a PUT that omitted it + // flipped a stored 0 to 1, and the alert text gained "including empty changes". So it is + // required here even though the schema does not require it — a fort body without one is not + // something this translator can send faithfully. + Required = ["include_empty"], + }, + }; + + /// The tracking types this build has a v2 field table for. + public static bool Handles(string type) => Specs.ContainsKey(type); /// - /// Translates one v1-shaped pokemon row. Returns false — leaving - /// untouched — when the row carries something v2 cannot be told faithfully. + /// Translates one v1-shaped row. Returns false — leaving untouched — + /// when the row carries something v2 cannot be told faithfully, or the type has no v2 table. /// + /// The tracking type, as PoracleNG names it in the route. /// A single v1-shaped alarm object, as every alarm service already builds. /// The v2 body on success. /// What stopped the translation, for the log. Null on success. - public static bool TryTranslatePokemon(JsonElement row, out JsonElement translated, out string? unsupported) + public static bool TryTranslate(string type, JsonElement row, out JsonElement translated, out string? unsupported) { translated = default; unsupported = null; + if (!Specs.TryGetValue(type, out var spec)) + { + unsupported = $"no v2 field table for {type}"; + return false; + } + if (row.ValueKind != JsonValueKind.Object) { unsupported = "the body is not a single rule object"; return false; } - if (!row.TryGetProperty("pokemon_id", out var speciesId) || speciesId.ValueKind != JsonValueKind.Number) + if (!SatisfiesRequired(spec, row, out unsupported)) { - // v2 makes pokemon_id the one required field. A row without it could only ever 422. - unsupported = "pokemon_id is missing"; return false; } @@ -94,7 +226,7 @@ public static bool TryTranslatePokemon(JsonElement row, out JsonElement translat continue; } - if (!TryWriteProperty(writer, property, out unsupported)) + if (!TryWriteProperty(writer, spec, property, out unsupported)) { return false; } @@ -107,137 +239,226 @@ public static bool TryTranslatePokemon(JsonElement row, out JsonElement translat return true; } - private static bool TryWriteProperty(Utf8JsonWriter writer, JsonProperty property, out string? unsupported) + private static bool SatisfiesRequired(TypeSpec spec, JsonElement row, out string? unsupported) + { + unsupported = null; + + foreach (var required in spec.Required) + { + if (!row.TryGetProperty(required, out var value) || value.ValueKind == JsonValueKind.Null) + { + unsupported = $"{required} is missing, and v2 requires it"; + return false; + } + + if (spec.PositiveIntegers.Contains(required) + && (value.ValueKind != JsonValueKind.Number || !value.TryGetInt32(out var number) || number < 1)) + { + unsupported = $"{required} must be at least 1 on v2"; + return false; + } + } + + return true; + } + + private static bool TryWriteProperty( + Utf8JsonWriter writer, TypeSpec spec, JsonProperty property, out string? unsupported) { unsupported = null; switch (property.Name) { case "clean": - return TryWriteClean(writer, property.Value, out unsupported); + return spec.HasCleanFlags + ? TryWriteClean(writer, property.Value, out unsupported) + : TryWriteAbsentClean(property.Value, out unsupported); case "ping": return TryWritePing(property.Value, out unsupported); - case "gender": - return TryWriteGender(writer, property.Value, out unsupported); - - case "pvp_ranking_league": - return TryWriteLeague(writer, property.Value, out unsupported); + case "override_areas": + return TryWriteArrayLike(writer, "override_areas", property.Value, out unsupported); case "template": case "override_location_label": - if (property.Value.ValueKind is not (JsonValueKind.String or JsonValueKind.Null)) - { - unsupported = $"{property.Name} is not a string"; - return false; - } + return TryWriteString(writer, property, out unsupported); + } - property.WriteTo(writer); - return true; + if (spec.IntEnums.TryGetValue(property.Name, out var names)) + { + return TryWriteIntEnum(writer, property, names, out unsupported); + } - case "override_areas": - return TryWriteOverrideAreas(writer, property.Value, out unsupported); + if (spec.BoundedIntegers.TryGetValue(property.Name, out var allowedNumbers)) + { + return TryWriteBoundedInteger(writer, property, allowedNumbers, out unsupported); + } - default: - if (!IntegerFields.Contains(property.Name)) - { - // A field this build has never heard of. Newer PoracleNG, older PoracleWeb — send the - // row to v1, which takes anything, rather than dropping the user's value. - unsupported = $"unknown property {property.Name}"; - return false; - } + if (spec.AllowedStrings.TryGetValue(property.Name, out var allowedStrings)) + { + return TryWriteAllowedString(writer, property, allowedStrings, out unsupported); + } - if (property.Value.ValueKind is not (JsonValueKind.Number or JsonValueKind.Null)) - { - unsupported = $"{property.Name} is not a number"; - return false; - } + if (spec.Booleans.Contains(property.Name)) + { + return TryWriteBoolean(writer, property, out unsupported); + } - property.WriteTo(writer); - return true; + if (spec.StringArrays.Contains(property.Name)) + { + return TryWriteArrayLike(writer, property.Name, property.Value, out unsupported); } + + if (spec.Strings.Contains(property.Name)) + { + return TryWriteString(writer, property, out unsupported); + } + + if (!spec.Integers.Contains(property.Name)) + { + // A field this build has never heard of. Newer PoracleNG, older PoracleWeb — send the + // row to v1, which takes anything, rather than dropping the user's value. + unsupported = $"unknown property {property.Name}"; + return false; + } + + if (property.Value.ValueKind is not (JsonValueKind.Number or JsonValueKind.Null)) + { + unsupported = $"{property.Name} is not a number"; + return false; + } + + property.WriteTo(writer); + return true; } - /// The 3-bit mask becomes three booleans. Bits outside the three known ones are not v2's. - private static bool TryWriteClean(Utf8JsonWriter writer, JsonElement value, out string? unsupported) + private static bool TryWriteString(Utf8JsonWriter writer, JsonProperty property, out string? unsupported) { unsupported = null; - if (value.ValueKind == JsonValueKind.Null) + if (property.Value.ValueKind is not (JsonValueKind.String or JsonValueKind.Null)) { - return true; + unsupported = $"{property.Name} is not a string"; + return false; } - if (value.ValueKind != JsonValueKind.Number || !value.TryGetInt32(out var mask)) + property.WriteTo(writer); + return true; + } + + private static bool TryWriteAllowedString( + Utf8JsonWriter writer, JsonProperty property, HashSet allowed, out string? unsupported) + { + unsupported = null; + + if (property.Value.ValueKind == JsonValueKind.Null) { - unsupported = "clean is not a bitmask"; - return false; + property.WriteTo(writer); + return true; } - if ((mask & ~CleanFlags.All) != 0) + if (property.Value.ValueKind != JsonValueKind.String + || property.Value.GetString() is not { } value + || !allowed.Contains(value)) { - // A bit PoracleWeb does not model. v2 has no field for it, so translating would drop it; - // v1 stores the integer as-is. See the clean-bitmask note in CLAUDE.md. - unsupported = $"clean carries bits outside the three known flags ({mask})"; + unsupported = $"{property.Name} is not one of {string.Join(", ", allowed.Order(StringComparer.Ordinal))}"; return false; } - writer.WriteBoolean("clean", CleanFlags.IsAutoDelete(mask)); - writer.WriteBoolean("edit", CleanFlags.IsEdit(mask)); - writer.WriteBoolean("summary", CleanFlags.IsSummary(mask)); + property.WriteTo(writer); return true; } - /// - /// ping is the mention prepended to the DM — a role or user the alert is meant to notify. It is - /// a real monsters column and the v1 body carries it, but V2PokemonRule has no field for - /// it and the handler stores Ping: "" unconditionally ("server-managed"). Verified live against - /// 5.2.1: a rule holding <@&400027130022592512> came back with an empty ping after one - /// v2 PUT. - /// - /// - /// So an empty ping is dropped — v2 would store the same empty string — but a set one sends the row to - /// v1, which keeps it. Silently discarding it here is exactly the #730 shape this translator exists to - /// avoid, and it lands on webhook alarms, where the role mention is the entire point of the alert. - /// - private static bool TryWritePing(JsonElement value, out string? unsupported) + private static bool TryWriteBoundedInteger( + Utf8JsonWriter writer, JsonProperty property, HashSet allowed, out string? unsupported) { unsupported = null; - if (value.ValueKind is JsonValueKind.Null - || (value.ValueKind == JsonValueKind.String && string.IsNullOrEmpty(value.GetString()))) + if (property.Value.ValueKind == JsonValueKind.Null) { return true; } - unsupported = "ping is set, and v2 would blank it"; - return false; + if (property.Value.ValueKind != JsonValueKind.Number || !property.Value.TryGetInt32(out var value)) + { + unsupported = $"{property.Name} is not a number"; + return false; + } + + if (!allowed.Contains(value)) + { + unsupported = + $"{property.Name} {value} is not one of {string.Join(", ", allowed.Order())}"; + return false; + } + + writer.WriteNumber(property.Name, value); + return true; } - private static bool TryWriteGender(Utf8JsonWriter writer, JsonElement value, out string? unsupported) + /// A v1 0/1 column that v2 declares as a real boolean. + private static bool TryWriteBoolean(Utf8JsonWriter writer, JsonProperty property, out string? unsupported) { unsupported = null; - if (value.ValueKind == JsonValueKind.Null) + switch (property.Value.ValueKind) + { + case JsonValueKind.Null: + case JsonValueKind.True: + case JsonValueKind.False: + property.WriteTo(writer); + return true; + + case JsonValueKind.Number when property.Value.TryGetInt32(out var flag) && flag is 0 or 1: + writer.WriteBoolean(property.Name, flag == 1); + return true; + + default: + unsupported = $"{property.Name} is not a 0/1 flag"; + return false; + } + } + + private static bool TryWriteIntEnum( + Utf8JsonWriter writer, JsonProperty property, string[] names, out string? unsupported) + { + unsupported = null; + + if (property.Value.ValueKind == JsonValueKind.Null) { return true; } - if (value.ValueKind != JsonValueKind.Number - || !value.TryGetInt32(out var gender) - || gender < 0 - || gender >= Genders.Length) + // Already a v2 enum string — reached when a stored row came back from a v2 read rather than a v1 one. + if (property.Value.ValueKind == JsonValueKind.String) + { + var existing = property.Value.GetString(); + if (existing is not null && Array.IndexOf(names, existing) >= 0) + { + property.WriteTo(writer); + return true; + } + + unsupported = $"{property.Name} is not one of {string.Join(", ", names)}"; + return false; + } + + if (property.Value.ValueKind != JsonValueKind.Number + || !property.Value.TryGetInt32(out var index) + || index < 0 + || index >= names.Length) { - unsupported = "gender is outside 0-3"; + unsupported = $"{property.Name} is outside 0-{names.Length - 1}"; return false; } - writer.WriteString("gender", Genders[gender]); + writer.WriteString(property.Name, names[index]); return true; } - private static bool TryWriteLeague(Utf8JsonWriter writer, JsonElement value, out string? unsupported) + /// The 3-bit mask becomes three booleans. Bits outside the three known ones are not v2's. + private static bool TryWriteClean(Utf8JsonWriter writer, JsonElement value, out string? unsupported) { unsupported = null; @@ -246,29 +467,79 @@ private static bool TryWriteLeague(Utf8JsonWriter writer, JsonElement value, out return true; } - if (value.ValueKind != JsonValueKind.Number || !value.TryGetInt32(out var league)) + if (value.ValueKind != JsonValueKind.Number || !value.TryGetInt32(out var mask)) { - unsupported = "pvp_ranking_league is not a number"; + unsupported = "clean is not a bitmask"; return false; } - if (!Leagues.Contains(league)) + if ((mask & ~CleanFlags.All) != 0) { - unsupported = $"pvp_ranking_league {league} is not one of 0, 500, 1500, 2500"; + // A bit PoracleWeb does not model. v2 has no field for it, so translating would drop it; + // v1 stores the integer as-is. See the clean-bitmask note in CLAUDE.md. + unsupported = $"clean carries bits outside the three known flags ({mask})"; return false; } - writer.WriteNumber("pvp_ranking_league", league); + writer.WriteBoolean("clean", CleanFlags.IsAutoDelete(mask)); + writer.WriteBoolean("edit", CleanFlags.IsEdit(mask)); + writer.WriteBoolean("summary", CleanFlags.IsSummary(mask)); return true; } /// - /// Areas arrive as a real array from the model, but a stored row carried forward by - /// can hold the column verbatim, and PoracleNG's own column is a - /// JSON string. Normalise rather than refuse — this is the one shape difference worth absorbing, - /// because it is PoracleWeb's own read that produced it. + /// fort is the one type with no clean anywhere — no v2 field and no column. A zero is + /// nothing to carry and is dropped; anything else is a value v2 could not be told, so the row goes + /// to v1. + /// + private static bool TryWriteAbsentClean(JsonElement value, out string? unsupported) + { + unsupported = null; + + if (value.ValueKind == JsonValueKind.Null + || (value.ValueKind == JsonValueKind.Number && value.TryGetInt32(out var mask) && mask == 0)) + { + return true; + } + + unsupported = "clean is set on a type v2 has no clean field for"; + return false; + } + + /// + /// ping is the mention prepended to the DM — a role or user the alert is meant to notify. It is + /// a real tracking column on every type and the v1 body carries it, but no V2*Rule has a field + /// for it and the handlers store Ping: "" unconditionally ("server-managed"). Verified live + /// against 5.2.1: a rule holding <@&400027130022592512> came back with an empty ping + /// after one v2 PUT. /// - private static bool TryWriteOverrideAreas(Utf8JsonWriter writer, JsonElement value, out string? unsupported) + /// + /// So an empty ping is dropped — v2 would store the same empty string — but a set one sends the row to + /// v1, which keeps it. Silently discarding it here is exactly the #730 shape this translator exists to + /// avoid, and it lands on webhook alarms, where the role mention is the entire point of the alert. + /// + private static bool TryWritePing(JsonElement value, out string? unsupported) + { + unsupported = null; + + if (value.ValueKind is JsonValueKind.Null + || (value.ValueKind == JsonValueKind.String && string.IsNullOrEmpty(value.GetString()))) + { + return true; + } + + unsupported = "ping is set, and v2 would blank it"; + return false; + } + + /// + /// override_areas and change_types arrive as real arrays from the models, but a stored row + /// carried forward by holds the column verbatim, and PoracleNG's + /// columns are JSON strings. Normalise rather than refuse — this is PoracleWeb's own read that produced + /// the string. + /// + private static bool TryWriteArrayLike( + Utf8JsonWriter writer, string name, JsonElement value, out string? unsupported) { unsupported = null; @@ -276,7 +547,7 @@ private static bool TryWriteOverrideAreas(Utf8JsonWriter writer, JsonElement val { case JsonValueKind.Null: case JsonValueKind.Array: - writer.WritePropertyName("override_areas"); + writer.WritePropertyName(name); value.WriteTo(writer); return true; @@ -284,7 +555,7 @@ private static bool TryWriteOverrideAreas(Utf8JsonWriter writer, JsonElement val var raw = value.GetString(); if (string.IsNullOrWhiteSpace(raw)) { - writer.WriteNull("override_areas"); + writer.WriteNull(name); return true; } @@ -293,23 +564,60 @@ private static bool TryWriteOverrideAreas(Utf8JsonWriter writer, JsonElement val using var parsed = JsonDocument.Parse(raw); if (parsed.RootElement.ValueKind != JsonValueKind.Array) { - unsupported = "override_areas is a string that is not a JSON array"; + unsupported = $"{name} is a string that is not a JSON array"; return false; } - writer.WritePropertyName("override_areas"); + writer.WritePropertyName(name); parsed.RootElement.WriteTo(writer); return true; } catch (JsonException) { - unsupported = "override_areas is a string that is not JSON"; + unsupported = $"{name} is a string that is not JSON"; return false; } default: - unsupported = "override_areas is neither an array nor null"; + unsupported = $"{name} is neither an array nor null"; return false; } } + + /// How one type's v1 columns map onto its V2*Rule. + private sealed record TypeSpec + { + /// Names v2 declares as integers, written through unchanged. + public required HashSet Integers { get; init; } + + /// + /// Names accepted as free strings. template and override_location_label are implicit, + /// because every V2*Rule declares both. + /// + public HashSet Strings { get; init; } = new(StringComparer.Ordinal); + + /// v1 0/1 columns v2 declares as booleans. + public HashSet Booleans { get; init; } = new(StringComparer.Ordinal); + + /// v1 integer columns v2 declares as string enums, the array indexed by the v1 value. + public Dictionary IntEnums { get; init; } = new(StringComparer.Ordinal); + + /// Integer columns v2 restricts to a fixed set of values. + public Dictionary> BoundedIntegers { get; init; } = new(StringComparer.Ordinal); + + /// v1 string columns v2 restricts to a fixed set. + public Dictionary> AllowedStrings { get; init; } = new(StringComparer.Ordinal); + + /// Columns stored as a JSON string that v2 declares as an array. + public HashSet StringArrays { get; init; } = new(StringComparer.Ordinal); + + /// Whether this type has clean/edit/summary at all. Fort does not. + public bool HasCleanFlags { get; init; } = true; + + /// Fields that must be present and non-null, or the row goes to v1. + public string[] Required { get; init; } = []; + + /// Required fields v2 also constrains to 1 or more. + public HashSet PositiveIntegers { get; init; } = new(StringComparer.Ordinal); + } } diff --git a/Core/Pgan.PoracleWebNet.Core.Services/UserOwnedOverrideAreaProxy.cs b/Core/Pgan.PoracleWebNet.Core.Services/UserOwnedOverrideAreaProxy.cs index a54c85ab..eacf96c5 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/UserOwnedOverrideAreaProxy.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/UserOwnedOverrideAreaProxy.cs @@ -61,6 +61,43 @@ public Task GetAllTrackingAllProfilesAsync(string userId) => public Task ReloadStateAsync() => this._inner.ReloadStateAsync(); + /// + /// + /// The v2 replace needs the same write-back as , and needs it under the + /// same rule: the replacement is a new row under a new uid, so the areas go to the uid PoracleNG just + /// reported. A null answer means nothing was written, so there is nothing to write back to -- the + /// caller is about to take its own v1 path, which comes back through this decorator anyway. + /// + public async Task TryReplaceV2Async( + string type, string userId, int uid, JsonElement body) + { + EnsureScopeIsCoherent(body); + + if (!MentionsAnyOverrideArea(body)) + { + return await this._inner.TryReplaceV2Async(type, userId, uid, body); + } + + var owned = await this.OwnedGeofenceNamesAsync(userId); + var full = OverrideAreasOf(body); + + if (owned.Count == 0 || full is null || !full.Any(a => owned.Contains(a))) + { + return await this._inner.TryReplaceV2Async(type, userId, uid, body); + } + + var sanitised = StripOwned(body, owned); + + if (await this._inner.TryReplaceV2Async(type, userId, uid, sanitised) is not { } result) + { + return null; + } + + await this.WriteBackOwnedAreasAsync(type, userId, result.Uid > 0 ? result.Uid : uid, full); + + return result; + } + /// /// /// The same workaround as , with one difference that matters: on the v2 @@ -88,22 +125,29 @@ public async Task UpdateByUidAsync( var sanitised = StripOwned(body, owned); var result = await this._inner.UpdateByUidAsync(type, userId, uid, sanitised); - var written = await this._areaWriter.SetAlarmOverrideAreasAsync( - userId, type, result.Uid > 0 ? result.Uid : uid, full); - if (!written) + await this.WriteBackOwnedAreasAsync(type, userId, result.Uid > 0 ? result.Uid : uid, full); + + return result; + } + + /// + /// Writes the full area list, the user's own geofences included, straight onto the row PoracleNG just + /// stored. + /// + private async Task WriteBackOwnedAreasAsync(string type, string userId, int uid, List areas) + { + if (!await this._areaWriter.SetAlarmOverrideAreasAsync(userId, type, uid, areas)) { // The row PoracleNG just reported is not there to write to. Refusing loudly beats an alarm // that silently alerts on the whole profile instead of one small geofence. - LogWriteBackMissedRow(this._logger, type, result.Uid, userId); + LogWriteBackMissedRow(this._logger, type, uid, userId); throw new InvalidOperationException( $"Could not apply the area restriction to the {type} alarm that was just saved."); } // PoracleNG reloads its state on its own mutations, and a direct column write is not one. await this._inner.ReloadStateAsync(); - - return result; } public async Task CreateAsync(string type, string userId, JsonElement body) diff --git a/Tests/Pgan.PoracleWebNet.Tests/Services/AlarmServiceV2UpdatePathTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Services/AlarmServiceV2UpdatePathTests.cs new file mode 100644 index 00000000..6184a5a7 --- /dev/null +++ b/Tests/Pgan.PoracleWebNet.Tests/Services/AlarmServiceV2UpdatePathTests.cs @@ -0,0 +1,160 @@ +using System.Text.Json; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Pgan.PoracleWebNet.Core.Abstractions.Services; +using Pgan.PoracleWebNet.Core.Models; +using Pgan.PoracleWebNet.Core.Services; +using Pgan.PoracleWebNet.Tests.TestDoubles; + +namespace Pgan.PoracleWebNet.Tests.Services; + +/// +/// What each alarm service does once the proxy answers that /api/v2 replaced the rule. +/// +/// +/// +/// The uid-addressed PUT is a genuine full replace, so every repair the v1 path wraps around its create +/// has to be skipped as a unit: the reconcile of a stray insert, lure's delete-to-free-the-natural-key, +/// max battle's delete-then-recreate. Running any of them against a write that already landed would be +/// worse than not moving the type at all. +/// +/// +/// The other half is the uid. v2's engine is delete-then-insert, so the replacement arrives under a new +/// uid, and quick-pick applied state has to follow the row or its "remove" button silently deletes +/// nothing. See #403. +/// +/// +public class AlarmServiceV2UpdatePathTests +{ + private readonly Mock _proxy = new(); + private readonly Mock _featureGate = new(); + private readonly Mock _remapper = new(); + + public AlarmServiceV2UpdatePathTests() + { + this._featureGate.Setup(g => g.EnsureEnabledAsync(It.IsAny())).Returns(Task.CompletedTask); + this._remapper + .Setup(r => r.RemapAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + this._proxy + .Setup(p => p.GetByUserAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(() => JsonDocument.Parse("[]").RootElement.Clone()); + } + + public static TheoryData MovedTypes() => + ["raid", "egg", "quest", "nest", "gym", "maxbattle", "fort", "lure"]; + + [Theory] + [MemberData(nameof(MovedTypes))] + public async Task AV2ReplaceSkipsTheWholeV1RepairPath(string type) + { + this.AcceptV2(type, newUid: 991); + + Assert.Equal(991, await this.UpdateAsync(type, uid: 990)); + + // No create, so no reconcile. No delete, so no window where the alarm exists nowhere. + this._proxy.Verify( + p => p.CreateAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + this._proxy.Verify( + p => p.DeleteByUidAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + + [Theory] + [MemberData(nameof(MovedTypes))] + public async Task TheRotatedUidIsCarriedIntoQuickPickAppliedState(string type) + { + this.AcceptV2(type, newUid: 991); + + await this.UpdateAsync(type, uid: 990); + + this._remapper.Verify(r => r.RemapAsync("u1", type, 990, 991), Times.Once); + } + + [Theory] + [MemberData(nameof(MovedTypes))] + public async Task DecliningV2LeavesTheV1PathExactlyAsItWas(string type) + { + // The legitimate-case half. A 5.1.0 server, an operator pinned to v1, or a row v2 cannot carry + // faithfully all answer null -- and the type has to write exactly what it wrote before. + this._proxy + .Setup(p => p.TryReplaceV2Async(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((TrackingUpdateResult?)null); + this._proxy + .Setup(p => p.CreateAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new TrackingCreateResult([991], 0, 0, 1)); + + await this.UpdateAsync(type, uid: 990); + + this._proxy.Verify( + p => p.CreateAsync(type, "u1", It.IsAny()), Times.AtLeastOnce); + } + + [Fact] + public async Task LureNoLongerDeletesItsRowToFreeTheNaturalKey() + { + // The single biggest reason to move this type. PoracleNG's v1 create has no upsert path for + // lure_tracking(id, profile_no, lure_id), so editing a lure's distance had to delete the row, + // re-create it, and restore the original if that failed. Verified live on 5.2.1: the v2 PUT + // replaced lure 266 in place as 267, where the v1 create-carrying-a-uid inserted 264 alongside + // 263 and left both. + this.AcceptV2("lure", newUid: 267); + + var updated = await new LureService( + this._proxy.Object, this._featureGate.Object, NullLogger.Instance, this._remapper.Object) + .UpdateAsync("u1", new Lure { Uid = 266, LureId = 501, Distance = 5000 }); + + Assert.Equal(267, updated.Uid); + this._proxy.Verify(p => p.DeleteByUidAsync("lure", "u1", 266), Times.Never); + } + + [Fact] + public async Task MaxBattleStopsBeingInsertOnly() + { + // Its v1 path deletes the row and creates a replacement, so a failed create leaves the user with + // no alarm at all. The v2 PUT has no such window. + this.AcceptV2("maxbattle", newUid: 92); + + var updated = await new MaxBattleService( + this._proxy.Object, this._featureGate.Object, NullLogger.Instance, this._remapper.Object) + .UpdateAsync("u1", new MaxBattle { Uid = 91, PokemonId = 150 }); + + Assert.Equal(92, updated.Uid); + this._proxy.Verify(p => p.DeleteByUidAsync("maxbattle", "u1", 91), Times.Never); + } + + private void AcceptV2(string type, int newUid) => + this._proxy + .Setup(p => p.TryReplaceV2Async(type, It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new TrackingUpdateResult(newUid, true)); + + private async Task UpdateAsync(string type, int uid) => type switch + { + "raid" => (await new RaidService( + this._proxy.Object, this._featureGate.Object, NullLogger.Instance, + this._remapper.Object, CostumeCapabilityDoubles.Supported()) + .UpdateAsync("u1", new Raid { Uid = uid, PokemonId = 150 })).Uid, + "egg" => (await new EggService( + this._proxy.Object, this._featureGate.Object, NullLogger.Instance, this._remapper.Object) + .UpdateAsync("u1", new Egg { Uid = uid, Level = 5 })).Uid, + "quest" => (await new QuestService( + this._proxy.Object, this._featureGate.Object, PokecoinCapabilityStub.Supported, + NullLogger.Instance, this._remapper.Object) + .UpdateAsync("u1", new Quest { Uid = uid, RewardType = 7, Reward = 25 })).Uid, + "nest" => (await new NestService( + this._proxy.Object, this._featureGate.Object, NullLogger.Instance, this._remapper.Object) + .UpdateAsync("u1", new Nest { Uid = uid, PokemonId = 25 })).Uid, + "gym" => (await new GymService( + this._proxy.Object, this._featureGate.Object, NullLogger.Instance, this._remapper.Object) + .UpdateAsync("u1", new Gym { Uid = uid, Team = 4 })).Uid, + "maxbattle" => (await new MaxBattleService( + this._proxy.Object, this._featureGate.Object, NullLogger.Instance, this._remapper.Object) + .UpdateAsync("u1", new MaxBattle { Uid = uid, PokemonId = 150 })).Uid, + "fort" => (await new FortChangeService( + this._proxy.Object, this._featureGate.Object, NullLogger.Instance, this._remapper.Object) + .UpdateAsync("u1", new FortChange { Uid = uid, FortType = "gym" })).Uid, + "lure" => (await new LureService( + this._proxy.Object, this._featureGate.Object, NullLogger.Instance, this._remapper.Object) + .UpdateAsync("u1", new Lure { Uid = uid, LureId = 501 })).Uid, + _ => throw new ArgumentOutOfRangeException(nameof(type), type, "No fixture for this tracking type."), + }; +} diff --git a/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleServerProfileTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleServerProfileTests.cs index f4fb8fa6..66844a49 100644 --- a/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleServerProfileTests.cs +++ b/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleServerProfileTests.cs @@ -232,4 +232,44 @@ public async Task AHealthPayloadWithNoCapabilitiesStillGivesTheVersion() Assert.Empty(profile.Capabilities); Assert.True(profile.IsBelowMinimum); } + + [Fact] + public async Task ManyCallersArrivingOnAColdCacheProbeOnce() + { + // AddHttpClient registers this transient, so every caller gets its own instance and the cache is + // the only thing they share. A dashboard load fires several version-gated tracking writes at + // once; without the gate each of them misses the same empty cache and runs its own /health GET + // and schema_migrations SELECT. + var probes = 0; + var handler = new Mock(); + handler.Protected() + .Setup>( + "SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(async () => + { + // Slow enough that every caller is genuinely in flight before the first one answers. + Interlocked.Increment(ref probes); + await Task.Delay(50); + return new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(HealthyResponse) }; + }); + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { ["Poracle:ApiAddress"] = "http://poracle:3030" }) + .Build(); + var shared = new MemoryCache(new MemoryCacheOptions()); + + PoracleServerProfileService Instance() => new( + new HttpClient(handler.Object), + this._schema.Object, + shared, + configuration, + NullLogger.Instance); + + var results = await Task.WhenAll( + Enumerable.Range(0, 8).Select(_ => Task.Run(() => Instance().GetAsync()))); + + Assert.Equal(1, probes); + this._schema.Verify(r => r.GetAppliedMigrationAsync(It.IsAny()), Times.Once); + Assert.All(results, profile => Assert.Equal("5.1.0", profile.Version)); + } } diff --git a/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleTrackingProxyV2Tests.cs b/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleTrackingProxyV2Tests.cs index 14423a3e..a3c42315 100644 --- a/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleTrackingProxyV2Tests.cs +++ b/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleTrackingProxyV2Tests.cs @@ -12,8 +12,9 @@ namespace Pgan.PoracleWebNet.Tests.Services; /// -/// The pokemon write path on PoracleNG's strict /api/v2 surface, and every way it must decline to -/// use it. See #805. +/// The write path on PoracleNG's strict /api/v2 surface -- routing, gating, error shapes and every +/// way it must decline to use it. Exercised on pokemon; the per-type field tables have their own suite in +/// . See #805. /// /// /// @@ -115,16 +116,19 @@ public async Task AnUnreachableServerWritesThroughV1() } [Fact] - public async Task TheOtherNineTypesStayOnV1EvenOnA521Server() + public async Task InvasionStaysOnV1EvenOnA521Server() { - // #805 is a pilot on pokemon. Each remaining type needs its own field translation derived from its - // own schema, and v1 is frozen and unchanged on 5.2.1, so leaving them is a no-op not a deferral. + // The one type with a v2 surface PoracleWeb deliberately stays off. A v2 read of a named-grunt + // rule carries no targeting field at all, and PoracleWeb holds only the grunt name -- which live + // data fills with values it cannot reverse into a type_id or grunt_id (blanche, candela, spark, + // npc 0..npc 10, player team leader). Filed upstream; until it is answered, invasion has no + // faithful v2 body in either direction. var handler = ScriptedHandler.Ok("""{"newUids":[9],"alreadyPresent":0,"updates":1,"insert":0}"""); var sut = CreateSut(handler, version: "5.2.1"); - await sut.UpdateByUidAsync("raid", "user1", 9, Row("""{"uid":9,"pokemon_id":9000,"level":5}""")); + await sut.UpdateByUidAsync("invasion", "user1", 9, Row("""{"uid":9,"grunt_type":"blanche"}""")); - Assert.Equal($"{ApiAddress}/api/tracking/raid/user1?silent=true", Assert.Single(handler.Requests).Url); + Assert.Equal($"{ApiAddress}/api/tracking/invasion/user1?silent=true", Assert.Single(handler.Requests).Url); } [Theory] @@ -178,6 +182,27 @@ public async Task AMissingV2RouteIsRememberedSoTheNextEditDoesNotProbeAgain() Assert.All(handler.Requests.Skip(1), r => Assert.Equal(HttpMethod.Post, r.Method)); } + [Fact] + public async Task AnAbsentRouteOnOneTypeDoesNotDropTheOtherEightBackToV1() + { + // The absent flag is keyed per type. A single flag let one gin 404 from one route disable v2 for + // every type at once -- and the types do not ship together, so a build that carries the raid route + // and not the fort one is an ordinary state, not a broken server. + var handler = new ScriptedHandler( + new Reply(HttpStatusCode.NotFound, "404 page not found", "text/plain"), + new Reply(HttpStatusCode.OK, """{"newUids":[63],"alreadyPresent":0,"updates":1,"insert":0}"""), + new Reply(HttpStatusCode.OK, RotatedOk)); + var cache = new MemoryCache(new MemoryCacheOptions()); + var sut = CreateSut(handler, version: "5.2.1", cache: cache); + + await sut.UpdateByUidAsync("fort", "user1", 63, Row("""{"uid":63,"fort_type":"gym","include_empty":0}""")); + var pokemon = await sut.UpdateByUidAsync("pokemon", "user1", 36486, Row(StoredRow)); + + Assert.True(pokemon.UsedV2); + Assert.Equal(HttpMethod.Put, handler.Requests[2].Method); + Assert.Equal($"{ApiAddress}/api/v2/humans/user1/tracking/pokemon/36486?silent=true", handler.Requests[2].Url); + } + [Fact] public async Task ARuleThatIsNotTheirsIsReportedAsNotFound() { diff --git a/Tests/Pgan.PoracleWebNet.Tests/Services/TrackingV2TypeTranslationTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Services/TrackingV2TypeTranslationTests.cs new file mode 100644 index 00000000..8564d473 --- /dev/null +++ b/Tests/Pgan.PoracleWebNet.Tests/Services/TrackingV2TypeTranslationTests.cs @@ -0,0 +1,362 @@ +using System.Text.Json; +using Pgan.PoracleWebNet.Core.Services; + +namespace Pgan.PoracleWebNet.Tests.Services; + +/// +/// The nine per-type field tables carries, checked against the schema +/// they were derived from and against the rows PoracleNG actually stores. +/// +/// +/// +/// Every V2*Rule sets additionalProperties: false, so a field the table leaks is a 422 and a +/// field the table misses is a value quietly sent to v1 forever. Both failures are invisible at runtime, +/// because the fallback catches them and the write still succeeds — which is exactly why they need a test +/// that reads the schema rather than the translator. +/// +/// +/// V2Schema is the property list of each V2*Rule in 5.2.1's +/// processor/internal/api/testdata/openapi.golden.json, and V1Row is a full row of the +/// matching v1 tracking columns. Both were confirmed against the live 5.2.1 instance rather than read from +/// the Go source, per CLAUDE.md. +/// +/// +public class TrackingV2TypeTranslationTests +{ + /// Every property each V2*Rule declares, from 5.2.1's openapi.golden.json. + private static readonly Dictionary V2Schema = new(StringComparer.Ordinal) + { + ["pokemon"] = + [ + "atk", "clean", "costume", "def", "distance", "edit", "form", "gender", "max_atk", "max_cp", + "max_def", "max_iv", "max_level", "max_rarity", "max_size", "max_sta", "max_weight", "min_cp", + "min_iv", "min_level", "min_time", "min_weight", "override_areas", "override_location_label", + "pokemon_id", "pvp_ranking_best", "pvp_ranking_cap", "pvp_ranking_evolution", + "pvp_ranking_league", "pvp_ranking_min_cp", "pvp_ranking_worst", "rarity", "size", "sta", + "summary", "template", + ], + ["raid"] = + [ + "clean", "costume", "distance", "edit", "evolution", "exclusive", "form", "gym_id", "level", + "move", "override_areas", "override_location_label", "pokemon_id", "rsvp_changes", "summary", + "team", "template", + ], + ["egg"] = + [ + "clean", "distance", "edit", "exclusive", "gym_id", "level", "override_areas", + "override_location_label", "rsvp_changes", "summary", "team", "template", + ], + ["quest"] = + [ + "amount", "clean", "distance", "edit", "form", "override_areas", "override_location_label", + "reward", "reward_type", "shiny", "summary", "template", + ], + ["gym"] = + [ + "battle_changes", "clean", "distance", "edit", "gym_id", "override_areas", + "override_location_label", "slot_changes", "summary", "team", "template", + ], + ["maxbattle"] = + [ + "clean", "distance", "edit", "evolution", "form", "gmax", "level", "move", "override_areas", + "override_location_label", "pokemon_id", "station_id", "summary", "template", + ], + ["nest"] = + [ + "clean", "distance", "edit", "form", "min_spawn_avg", "override_areas", + "override_location_label", "pokemon_id", "summary", "template", + ], + ["lure"] = + [ + "clean", "distance", "edit", "lure_id", "override_areas", "override_location_label", "summary", + "template", + ], + ["fort"] = + [ + "change_types", "distance", "fort_type", "include_empty", "override_areas", + "override_location_label", "template", + ], + }; + + /// + /// A full v1 row of each type, in the shape PoracleWeb's models serialize and PoracleNG's v1 read + /// returns. Sentinels are present on purpose: they are sent verbatim and 5.2.1 stores them exactly as + /// v1 does. + /// + private static readonly Dictionary V1Row = new(StringComparer.Ordinal) + { + ["pokemon"] = """ + {"uid":36486,"id":"user1","profile_no":1,"ping":"","description":"**Pikachu**","clean":3, + "distance":1000,"template":"1","pokemon_id":25,"form":0,"costume":9000,"min_iv":90, + "max_iv":100,"min_cp":0,"max_cp":9000,"min_level":0,"max_level":55,"atk":0,"def":0,"sta":0, + "max_atk":15,"max_def":15,"max_sta":15,"gender":2,"min_weight":0,"max_weight":9000000, + "min_time":0,"rarity":0,"max_rarity":6,"size":0,"max_size":5,"pvp_ranking_league":1500, + "pvp_ranking_best":1,"pvp_ranking_worst":100,"pvp_ranking_min_cp":0,"pvp_ranking_cap":50, + "pvp_ranking_evolution":0,"override_location_label":"","override_areas":null} + """, + ["raid"] = """ + {"uid":414,"id":"user1","profile_no":1,"ping":"","clean":0,"distance":0,"template":"1", + "team":4,"pokemon_id":150,"form":0,"costume":9000,"level":9000,"exclusive":0,"move":9000, + "evolution":9000,"gym_id":null,"rsvp_changes":0,"override_location_label":"", + "override_areas":null,"description":"**Mewtwo**"} + """, + ["egg"] = """ + {"uid":178,"id":"user1","profile_no":1,"ping":"","clean":0,"distance":0,"template":"1", + "team":4,"level":5,"exclusive":0,"gym_id":null,"rsvp_changes":0, + "override_location_label":"","override_areas":null,"description":"**Level 5 eggs**"} + """, + ["quest"] = """ + {"uid":900,"id":"user1","profile_no":1,"ping":"","clean":0,"distance":0,"template":"1", + "reward_type":7,"reward":25,"form":0,"shiny":0,"amount":1,"override_location_label":"", + "override_areas":null,"description":"**Pikachu**"} + """, + ["gym"] = """ + {"uid":150,"id":"user1","profile_no":1,"ping":"","clean":0,"distance":0,"template":"1", + "team":4,"slot_changes":1,"battle_changes":0,"gym_id":"","override_location_label":"", + "override_areas":null,"description":"**All team's gyms**"} + """, + ["maxbattle"] = """ + {"uid":91,"id":"user1","profile_no":1,"ping":"","clean":0,"distance":0,"template":"1", + "pokemon_id":150,"form":0,"level":9000,"move":9000,"gmax":0,"evolution":9000, + "station_id":null,"override_location_label":"","override_areas":null, + "description":"**Mewtwo**"} + """, + ["nest"] = """ + {"uid":965,"id":"user1","profile_no":1,"ping":"","clean":0,"distance":0,"template":"1", + "pokemon_id":25,"min_spawn_avg":1,"form":0,"override_location_label":"", + "override_areas":null,"description":"**Pikachu**"} + """, + ["lure"] = """ + {"uid":266,"id":"user1","profile_no":1,"ping":"","clean":0,"distance":0,"template":"1", + "lure_id":501,"override_location_label":"","override_areas":null, + "description":"Lure type: **Normal Lure**"} + """, + ["fort"] = """ + {"uid":63,"id":"user1","profile_no":1,"ping":"","distance":0,"template":"1","fort_type":"gym", + "include_empty":0,"change_types":"[\"name\"]","override_location_label":"", + "override_areas":null,"description":"Fort updates: **gym**"} + """, + }; + + public static TheoryData V2Types() + { + var data = new TheoryData(); + foreach (var type in V2Schema.Keys) + { + data.Add(type); + } + + return data; + } + + [Theory] + [MemberData(nameof(V2Types))] + public void AFullStoredRowTranslatesForEveryTypeThatMoved(string type) + { + Assert.True( + TrackingV2Translator.TryTranslate(type, Row(V1Row[type]), out _, out var unsupported), + $"A stored {type} row must reach /api/v2, not fall back: {unsupported}"); + } + + [Theory] + [MemberData(nameof(V2Types))] + public void NothingOutsideTheSchemaReachesTheWire(string type) + { + // additionalProperties: false. One leaked field is a 422, and the fallback hides it. + TrackingV2Translator.TryTranslate(type, Row(V1Row[type]), out var translated, out _); + + var leaked = translated.EnumerateObject() + .Select(p => p.Name) + .Where(name => !V2Schema[type].Contains(name, StringComparer.Ordinal)) + .ToList(); + + Assert.True(leaked.Count == 0, $"V2{type}Rule has no field for: {string.Join(", ", leaked)}"); + } + + [Theory] + [MemberData(nameof(V2Types))] + public void EverySchemaFieldWithAV1SourceIsWritten(string type) + { + // The other half. A schema field missing from the type's table is a filter PoracleWeb would send + // to v1 forever without anyone noticing, because the write still succeeds. + TrackingV2Translator.TryTranslate(type, Row(V1Row[type]), out var translated, out _); + + var written = translated.EnumerateObject().Select(p => p.Name).ToHashSet(StringComparer.Ordinal); + var missing = V2Schema[type].Where(name => !written.Contains(name)).ToList(); + + Assert.True(missing.Count == 0, $"The {type} table does not write: {string.Join(", ", missing)}"); + } + + // ────────────────────────────────────────────────────────────── + // The shape changes, one per kind. Values taken from a live 5.2.1 round-trip. + // ────────────────────────────────────────────────────────────── + + [Theory] + [InlineData(0, "harmony")] + [InlineData(1, "mystic")] + [InlineData(2, "valor")] + [InlineData(3, "instinct")] + [InlineData(4, "any")] + public void TeamBecomesItsEnumName(int team, string expected) + { + var body = Translate("gym", $$"""{"team":{{team}},"distance":0}"""); + + Assert.Equal(expected, body.GetProperty("team").GetString()); + } + + [Theory] + [InlineData(0, "none")] + [InlineData(1, "rsvp")] + [InlineData(2, "rsvp_only")] + public void RsvpChangesBecomesItsEnumName(int rsvp, string expected) + { + var body = Translate("egg", $$"""{"level":5,"rsvp_changes":{{rsvp}}}"""); + + Assert.Equal(expected, body.GetProperty("rsvp_changes").GetString()); + } + + [Theory] + [InlineData("raid", """{"exclusive":1}""", "exclusive")] + [InlineData("gym", """{"team":4,"slot_changes":1}""", "slot_changes")] + [InlineData("gym", """{"team":4,"battle_changes":1}""", "battle_changes")] + [InlineData("quest", """{"reward_type":7,"shiny":1}""", "shiny")] + [InlineData("maxbattle", """{"gmax":1}""", "gmax")] + [InlineData("fort", """{"include_empty":1}""", "include_empty")] + public void EveryZeroOrOneColumnBecomesARealBoolean(string type, string row, string field) + { + Assert.True(Translate(type, row).GetProperty(field).GetBoolean()); + } + + [Fact] + public void FortAlwaysStatesIncludeEmptyBecauseOmittingItFlipsTheDefault() + { + // Verified live on 5.2.1: a PUT that omitted include_empty turned a stored 0 into a 1 and the + // alert text gained "including empty changes". v1 defaults it FALSE, v2 defaults it TRUE. + Assert.False(Translate("fort", """{"fort_type":"gym","include_empty":0}""") + .GetProperty("include_empty").GetBoolean()); + + Assert.False( + TrackingV2Translator.TryTranslate( + "fort", Row("""{"fort_type":"gym","distance":0}"""), out _, out var unsupported), + "A fort row with no include_empty cannot be sent faithfully and must fall back to v1."); + Assert.Contains("include_empty", unsupported, StringComparison.Ordinal); + } + + [Fact] + public void FortCarriesNoCleanFieldAtAll() + { + // V2FortRule has no clean/edit/summary, and fort_tracking has no column for them either. + var body = Translate("fort", """{"fort_type":"gym","include_empty":0,"clean":0}"""); + + foreach (var name in new[] { "clean", "edit", "summary" }) + { + Assert.False(body.TryGetProperty(name, out _), $"V2FortRule has no {name}"); + } + } + + [Fact] + public void AJsonStringChangeTypesColumnBecomesAnArray() + { + // The v1 read returns change_types as a JSON string, and TrackingFieldPreserver carries the column + // forward verbatim. v2 declares an array. + var body = Translate("fort", """{"fort_type":"gym","include_empty":0,"change_types":"[\"name\"]"}"""); + + Assert.Equal("name", body.GetProperty("change_types").EnumerateArray().Single().GetString()); + } + + [Fact] + public void SentinelsAreSentVerbatimRatherThanOmitted() + { + // The migration guide says to omit them. Verified on 5.2.1 instead: a raid PUT carrying level, + // costume, move and evolution at 9000 stored exactly what the v1 create stores, and the v1 read + // came back byte-identical but for the rotated uid. Omitting them would leave + // TrackingUpdateReconciler comparing a stored 9000 against an absent field. + var body = Translate("raid", """{"pokemon_id":150,"level":9000,"costume":9000,"move":9000,"evolution":9000}"""); + + foreach (var name in new[] { "level", "costume", "move", "evolution" }) + { + Assert.Equal(9000, body.GetProperty(name).GetInt32()); + } + } + + // ────────────────────────────────────────────────────────────── + // Required fields. Each refusal is paired with the legitimate case that must still reach v2. + // ────────────────────────────────────────────────────────────── + + [Fact] + public void AnEggWithoutALevelGoesToV1RatherThanBeingRefused() + { + // V2EggRule declares level required with minimum 1 and Egg.Level is a plain int defaulting to 0, + // so profile import, quick-pick apply and the cleaning fetch-mutate-POST all build eggs v2 answers + // 422 to -- verified live. v1 has stored level 0 for years and keeps doing so. + Assert.False( + TrackingV2Translator.TryTranslate("egg", Row("""{"level":0,"team":4}"""), out _, out var unsupported)); + Assert.Contains("level", unsupported, StringComparison.Ordinal); + + Assert.True(TrackingV2Translator.TryTranslate("egg", Row("""{"level":1,"team":4}"""), out _, out _)); + Assert.True(TrackingV2Translator.TryTranslate("egg", Row("""{"level":5,"team":4}"""), out _, out _)); + } + + [Theory] + [InlineData("lure", """{"distance":0}""", """{"lure_id":501}""")] + [InlineData("quest", """{"distance":0}""", """{"reward_type":7}""")] + [InlineData("gym", """{"distance":0}""", """{"team":4}""")] + [InlineData("pokemon", """{"distance":0}""", """{"pokemon_id":25}""")] + public void ARowMissingWhatV2RequiresGoesToV1AndAnOrdinaryOneDoesNot( + string type, string without, string with) + { + Assert.False(TrackingV2Translator.TryTranslate(type, Row(without), out _, out _)); + Assert.True(TrackingV2Translator.TryTranslate(type, Row(with), out _, out var unsupported), unsupported); + } + + // ────────────────────────────────────────────────────────────── + // Declining rather than guessing, on the eight types that joined the pilot. + // ────────────────────────────────────────────────────────────── + + [Theory] + [InlineData("raid", """{"pokemon_id":150,"some_field_from_a_newer_poracle":4}""")] + [InlineData("raid", """{"pokemon_id":150,"team":9}""")] + [InlineData("raid", """{"pokemon_id":150,"rsvp_changes":7}""")] + [InlineData("raid", """{"pokemon_id":150,"ping":"<@&400027130022592512>"}""")] + [InlineData("raid", """{"pokemon_id":150,"clean":9}""")] + [InlineData("gym", """{"team":4,"slot_changes":2}""")] + [InlineData("fort", """{"fort_type":"stadium","include_empty":0}""")] + [InlineData("fort", """{"fort_type":"gym","include_empty":0,"clean":1}""")] + public void ARowV2CannotCarryFaithfullyDeclines(string type, string row) + { + Assert.False(TrackingV2Translator.TryTranslate(type, Row(row), out _, out var unsupported)); + Assert.NotNull(unsupported); + } + + [Theory] + [InlineData("pokestop")] + [InlineData("gym")] + [InlineData("everything")] + public void EveryFortTypeThatIsActuallyStoredStillReachesV2(string fortType) + { + // The legitimate-case half of the fort_type refusal above. These three are the only values + // FortChangeOptions.ValidFortTypes permits, so all of them must translate. + Assert.True( + TrackingV2Translator.TryTranslate( + "fort", + Row($$"""{"fort_type":"{{fortType}}","include_empty":0}"""), + out _, + out var unsupported), + unsupported); + } + + [Fact] + public void InvasionAndAnythingUnknownHasNoTableAndSaysSo() + { + Assert.False(TrackingV2Translator.Handles("invasion")); + Assert.False(TrackingV2Translator.TryTranslate("invasion", Row("""{"grunt_type":"blanche"}"""), out _, out _)); + } + + private static JsonElement Row(string json) => JsonDocument.Parse(json).RootElement.Clone(); + + private static JsonElement Translate(string type, string row) + { + Assert.True(TrackingV2Translator.TryTranslate(type, Row(row), out var translated, out var unsupported), unsupported); + return translated; + } +} From f5b3d89ca18f5b5a9a83d5938017090f3d95f8c3 Mon Sep 17 00:00:00 2001 From: hokiepokedad2 <38219945+hokiepokedad2@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:00:58 -0400 Subject: [PATCH 2/4] fix(pokestop-events): give the list a stable order too Pokestop events are the eleventh tracking type and the one this workstream missed. That type has only ever had PoracleNG's v2 write surface, so a replace has re-keyed the rule on every edit since it shipped -- the same card-jumps-to-the-end defect the eight moved types were just given orderAlarms for. Order on the event type, the field the card is titled by. The rendering test asserted PoracleNG's insertion order, so it was defending the defect; it now asserts the content order, and a named regression test drives the uid-rotation case. --- .../pokestop-event-list.component.spec.ts | 17 ++++++++++++++++- .../pokestop-event-list.component.ts | 3 ++- .../src/app/shared/utils/alarm-order.ts | 6 ++++-- CHANGELOG.md | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/pokestop-events/pokestop-event-list.component.spec.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/pokestop-events/pokestop-event-list.component.spec.ts index 9b4854e9..be357d7f 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/pokestop-events/pokestop-event-list.component.spec.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/pokestop-events/pokestop-event-list.component.spec.ts @@ -113,7 +113,22 @@ describe('PokestopEventListComponent', () => { { ...base, uid: 3, displayType: GOLD_STOP, eventName: 'gold-stop' }, ]); - expect(cardTitles()).toEqual(['INVASIONS.EVENT_TYPES.SHOWCASE', 'INVASIONS.EVENT_TYPES.KECLEON', 'INVASIONS.EVENT_TYPES.GOLD_STOP']); + // Ordered by display type rather than by the order PoracleNG returned them in. + expect(cardTitles()).toEqual(['INVASIONS.EVENT_TYPES.GOLD_STOP', 'INVASIONS.EVENT_TYPES.KECLEON', 'INVASIONS.EVENT_TYPES.SHOWCASE']); + }); + + it('keeps a rule where it was after an edit rotated its uid', () => { + // This type has only ever had a v2 write surface, and a v2 replace is delete-then-insert, so the + // edited rule comes back under the highest uid in the list. Rendering PoracleNG's own order threw + // the card the user had just saved to the end of the grid. + setup([ + // PoracleNG's own order: by uid, with the just-edited Kecleon rule re-keyed to the highest. + { ...base, uid: 1, displayType: GOLD_STOP, eventName: 'gold-stop' }, + { ...base, uid: 3, displayType: SHOWCASE }, + { ...base, uid: 99, displayType: KECLEON, eventName: 'kecleon' }, + ]); + + expect(cardTitles()).toEqual(['INVASIONS.EVENT_TYPES.GOLD_STOP', 'INVASIONS.EVENT_TYPES.KECLEON', 'INVASIONS.EVENT_TYPES.SHOWCASE']); }); it('shows the empty state, and no cards, when the profile tracks nothing', () => { diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/pokestop-events/pokestop-event-list.component.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/pokestop-events/pokestop-event-list.component.ts index 5d01662c..2e5de128 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/pokestop-events/pokestop-event-list.component.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/modules/pokestop-events/pokestop-event-list.component.ts @@ -22,6 +22,7 @@ import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/componen import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component'; import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component'; import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component'; +import { orderAlarms } from '../../shared/utils/alarm-order'; import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope'; import { isAutoDelete } from '../../shared/utils/clean-flags'; import { pokestopEventInfo } from '../../shared/utils/pokestop-events'; @@ -241,7 +242,7 @@ export class PokestopEventListComponent implements OnInit { .subscribe({ error: () => this.loading.set(false), next: items => { - this.events.set(items); + this.events.set(orderAlarms(items, e => [e.displayType])); this.loading.set(false); }, }); diff --git a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.ts b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.ts index a9f2cf07..7ccef617 100644 --- a/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.ts +++ b/Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.ts @@ -1,7 +1,7 @@ /** * A stable display order for an alarm list, independent of the rule id. * - * PoracleNG returns tracking rows in id order, and eight of the ten lists rendered that order straight + * PoracleNG returns tracking rows in id order, and nine of the eleven lists rendered that order straight * through. On PoracleNG 5.2.0 and newer an edit is a replace: the rule comes back under a new, higher id, * so the card the user just saved jumped to the end of the grid and, on a long list, off the screen — * with nothing to say it had moved. @@ -13,7 +13,9 @@ * than the problem being fixed — and the resolved name changes with the display language, so the order * would too. * - * Invasion is deliberately absent: it stays on PoracleNG's v1 write surface, so its ids do not rotate. + * Invasion is deliberately absent: it stays on PoracleNG's v1 write surface, so its ids do not rotate. The + * pokemon list already sorts on its own controls. Pokestop events are here too — that type has only ever + * had a v2 surface, so its ids have rotated on every edit since it shipped. */ export function orderAlarms( items: readonly T[], diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a388b48..ff9db68c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **A refused alarm explains itself the same way whichever Poracle surface answered.** The v2 write path already read PoracleNG's newer RFC 9457 error bodies and named the individual field it refused; the older v1 path, still the one most installs use, was reading only the older shape and answering a validation refusal as though the server had broken. Both paths now share one reader, and where many fields are refused at once the message names the first few and counts the rest rather than rendering a dozen clauses into a snackbar ([#803](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/803)). -- **Alarm lists keep a card where it was after you edit it.** Raids, eggs, quests, lures, nests, gyms, max battles and fort changes were rendered in the order Poracle returned them, which is the order the rules were created. On PoracleNG 5.2.0 and newer an edit replaces the rule rather than updating it, so the card you had just saved would have jumped to the end of the grid — off the screen entirely on a long list, with nothing to say it had moved. Each of those lists now has its own order, on what the card is titled by: Pokémon and level for raids and max battles, level for eggs, reward for quests, lure type, species for nests, team for gyms, change type for fort changes. The Pokemon list already sorted itself and is unchanged. +- **Alarm lists keep a card where it was after you edit it.** Raids, eggs, quests, lures, nests, gyms, max battles and fort changes were rendered in the order Poracle returned them, which is the order the rules were created. On PoracleNG 5.2.0 and newer an edit replaces the rule rather than updating it, so the card you had just saved would have jumped to the end of the grid — off the screen entirely on a long list, with nothing to say it had moved. Each of those lists now has its own order, on what the card is titled by: Pokémon and level for raids and max battles, level for eggs, reward for quests, lure type, species for nests, team for gyms, change type for fort changes, event type for Pokéstop events. Pokéstop events are in that list for a different reason: that type has only ever had the newer write surface, so its cards have been jumping since it shipped. The Pokemon list already sorted itself and is unchanged. - **Eight more alarm types have their edits written through PoracleNG 5.2.1's strict `/api/v2` surface, and lure edits stop being risky.** Raid, egg, quest, nest, gym, max battle, fort change and lure join Pokemon on the newer write path, which addresses a rule by its id and replaces it in place. For most of them nothing changes on screen. Lure is the exception worth naming: PoracleNG's older surface has no way to update a lure alarm at all, so editing one had to delete the rule, re-create it, and put the original back if that failed — a sequence with a window in which the alarm did not exist. Max battle edits carried the same window. Neither does now. Invasion alarms stay on the older surface deliberately: the new one cannot report which grunt a rule targets, so an edit could not be written back faithfully. Anything older than PoracleNG 5.2.0 keeps the path it has always used, unchanged, and an edit carrying something the new surface cannot express — a role mention, an egg with no level, a fort rule missing its empty-changes setting — takes the old path rather than failing. Set `PORACLE_TRACKING_API_VERSION` to `v1` or `v2` to pin it. - **Pokemon alarm edits are written through PoracleNG 5.2.1’s strict `/api/v2` surface, where the server can tell an edit apart from a takeover.** Nothing changes on screen. What changes is underneath: an edit now addresses the rule by its id, so Poracle refuses outright if the uid is not yours or if the result would duplicate an alarm you already have, instead of PoracleWeb.NET having to work that out from a success response and undo it afterwards. Poracle also explains a rejected filter field by name now, so the message on the dialog says which one. Anything older than 5.2.0 keeps the surface it has always used, unchanged, and so do the other nine alarm types; an edit carrying anything the new surface cannot express takes the old path rather than failing. Set `PORACLE_TRACKING_API_VERSION` to `v1` or `v2` to pin it ([#805](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/805)). From e43cc578b5c5312dbf1b24762b9f74fc6f0bfde1 Mon Sep 17 00:00:00 2001 From: hokiepokedad2 <38219945+hokiepokedad2@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:02:06 -0400 Subject: [PATCH 3/4] docs(tracking): name the test class that actually asserts the field tables --- Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs b/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs index def4eccf..aa450a56 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/TrackingV2Translator.cs @@ -79,7 +79,7 @@ internal static class TrackingV2Translator /// /// Every field each V2*Rule declares, taken from 5.2.1's openapi.golden.json, sorted into /// how it has to be written. A schema field missing from its type's table would go to v1 forever - /// without anyone noticing, which is why TrackingV2SchemaCoverageTests asserts the tables against + /// without anyone noticing, which is why TrackingV2TypeTranslationTests asserts the tables against /// the schema rather than against themselves. /// private static readonly Dictionary Specs = new(StringComparer.Ordinal) From 35cf2622ff2b3f42a490ccd00c0caf4ccaeb0158 Mon Sep 17 00:00:00 2001 From: hokiepokedad2 <38219945+hokiepokedad2@users.noreply.github.com> Date: Mon, 24 Aug 2026 18:47:01 -0400 Subject: [PATCH 4/4] docs(changelog): one heading per section after the merge The squash merges each appended their own heading rather than joining the existing one, so the section carried duplicates again. Bullets are untouched; only the headings are merged, into Keep a Changelog's order. --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b29e93e4..75f0bad1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **Nothing on screen: the last code that could write an alarm straight to Poracle's database has been taken out.** Every alarm write has gone through PoracleNG's API since 2.0, so its deduplication, its field defaults and its immediate state reload all run -- but the database tables were still mapped in code beside it, one line away from being used again. That mapping is gone, along with a set of profile methods nothing had called since the same migration. The two places that still reach the alarm tables directly are unchanged and are there for reasons written down beside them. - - **A refused alarm explains itself the same way whichever Poracle surface answered.** The v2 write path already read PoracleNG's newer RFC 9457 error bodies and named the individual field it refused; the older v1 path, still the one most installs use, was reading only the older shape and answering a validation refusal as though the server had broken. Both paths now share one reader, and where many fields are refused at once the message names the first few and counts the rest rather than rendering a dozen clauses into a snackbar ([#803](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/803)). - **Alarm lists keep a card where it was after you edit it.** Raids, eggs, quests, lures, nests, gyms, max battles and fort changes were rendered in the order Poracle returned them, which is the order the rules were created. On PoracleNG 5.2.0 and newer an edit replaces the rule rather than updating it, so the card you had just saved would have jumped to the end of the grid — off the screen entirely on a long list, with nothing to say it had moved. Each of those lists now has its own order, on what the card is titled by: Pokémon and level for raids and max battles, level for eggs, reward for quests, lure type, species for nests, team for gyms, change type for fort changes, event type for Pokéstop events. Pokéstop events are in that list for a different reason: that type has only ever had the newer write surface, so its cards have been jumping since it shipped. The Pokemon list already sorted itself and is unchanged. - **Eight more alarm types have their edits written through PoracleNG 5.2.1's strict `/api/v2` surface, and lure edits stop being risky.** Raid, egg, quest, nest, gym, max battle, fort change and lure join Pokemon on the newer write path, which addresses a rule by its id and replaces it in place. For most of them nothing changes on screen. Lure is the exception worth naming: PoracleNG's older surface has no way to update a lure alarm at all, so editing one had to delete the rule, re-create it, and put the original back if that failed — a sequence with a window in which the alarm did not exist. Max battle edits carried the same window. Neither does now. Invasion alarms stay on the older surface deliberately: the new one cannot report which grunt a rule targets, so an edit could not be written back faithfully. Anything older than PoracleNG 5.2.0 keeps the path it has always used, unchanged, and an edit carrying something the new surface cannot express — a role mention, an egg with no level, a fort rule missing its empty-changes setting — takes the old path rather than failing. Set `PORACLE_TRACKING_API_VERSION` to `v1` or `v2` to pin it.