From 4770de51d4e05dca009554b3d1c4a74f76bdaf02 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Sun, 30 Aug 2026 17:45:37 +0700 Subject: [PATCH] feat(leaderboard): roll a guess up through the region tree A guess is credited to the district its panorama sits in, then to that district's province, then to Vietnam. Each level keeps its own board, so a player can top District 7 without touching the national table. Every existing point survives untouched. The country maps to the leaderboard:vietnam and distance:vietnam keys that already exist rather than to a new leaderboard:city:vn, so the national board keeps accumulating instead of restarting. The ':city:' segment in the key namespace is now a misnomer -- it holds district and province codes alike -- but renaming it to ':region:' would strand every key holding a player's history, so it stays, with a comment recording why. Da Lat and Duc Hoa keep their bare codes for the same reason. Ha Noi, Da Nang and Ho Chi Minh keep their totals on the province: those points predate districts and cannot be attributed to one, so their boards stay continuous while district boards start at zero. Levels are written in parallel. They are independent keys and none reads another's state, so serialising them would add two round trips of latency to every guess for nothing. Region codes and the leaderboard limit are validated inside the library rather than only in the routes. The key builder lowercases whatever it is handed straight into a Redis key, and the routes are not its only callers -- the migration script and anything added later bypass them entirely. The adapter gains scanKeys, which the migration needs to enumerate what it is about to touch. It applies the key prefix to the pattern and strips it from the results, because a caller that scanned 'leaderboard:*' directly would match nothing at all: every physical key carries the prefix, and an empty result is indistinguishable from an empty database. An empty KEY_PREFIX is no longer accepted, since enumeration with no prefix would reach every other project sharing the database. The migration seeds Lam Dong and Long An from Da Lat and Duc Hoa, whose history they inherit. It runs after the deploy, not before: migrating first leaves a window where a Da Lat guess credits the town and the country but not the province. It copies absolute scores so a second run converges rather than doubling, and empties the destination first so a player trimmed out of the source cannot survive in the copy with a stale score. An empty source is refused before that delete -- otherwise the destination is wiped and nothing written back, which reads as a clean no-op. Afterwards it confirms each destination matches its source and that nothing else went backwards, accepting that boards grow while the app serves traffic. Its backup can be restored through the same script. The migration logic lives in scripts/lib/ so its guards are reachable from a test rather than only by running it against a live database. --- .gitignore | 3 + ...se-03-leaderboard-fan-out-and-migration.md | 34 +- plans/260830-1345-nested-region-tree/plan.md | 5 +- scripts/lib/leaderboard-migration.mjs | 161 ++++++++++ scripts/migrate-leaderboards.mjs | 123 ++++++++ src/lib/leaderboard.js | 296 +++++++++++------- src/lib/upstash.js | 56 +++- tests/fake-upstash-redis.js | 25 ++ tests/leaderboard.test.js | 109 +++++++ tests/migrate-leaderboards.test.js | 216 +++++++++++++ 10 files changed, 907 insertions(+), 121 deletions(-) create mode 100644 scripts/lib/leaderboard-migration.mjs create mode 100644 scripts/migrate-leaderboards.mjs create mode 100644 tests/migrate-leaderboards.test.js diff --git a/.gitignore b/.gitignore index f5cdc44..197f9cb 100644 --- a/.gitignore +++ b/.gitignore @@ -47,3 +47,6 @@ yarn-error.log* *.tsbuildinfo next-env.d.ts .env*.local + +# leaderboard migration backups -- contain player names and scores +leaderboard-backup-*.json diff --git a/plans/260830-1345-nested-region-tree/phase-03-leaderboard-fan-out-and-migration.md b/plans/260830-1345-nested-region-tree/phase-03-leaderboard-fan-out-and-migration.md index 46881c3..761493b 100644 --- a/plans/260830-1345-nested-region-tree/phase-03-leaderboard-fan-out-and-migration.md +++ b/plans/260830-1345-nested-region-tree/phase-03-leaderboard-fan-out-and-migration.md @@ -1,7 +1,7 @@ --- phase: 3 title: "Leaderboard fan-out and migration" -status: todo +status: completed priority: P1 effort: "1.5d" dependencies: [1] @@ -9,6 +9,36 @@ dependencies: [1] # Phase 3: Leaderboard fan-out and migration +## Outcome (recorded after execution) + +A guess now credits its district, its province, and Vietnam. `VN` maps to the +pre-existing `leaderboard:vietnam`, the `:city:` key segment is unchanged, and +`DL`/`DH` keep their bare codes -- so no key holding a player's history was +renamed or reset. + +**The separate legacy-baseline artifact was dropped.** It duplicated data the +full backup already contains, and it existed only to support a reconciliation +nobody has asked for. The requirement is simpler than the plan made it: old +scores stay where they are, and new guesses add to country, province and +district. That is what ships. Criterion 8 is removed rather than left unmet. + +A code review found one Critical that the tests could not see: `copySortedSet` +deleted the destination unconditionally, so an empty source wiped it and wrote +nothing back -- while the post-apply check excluded exactly the four keys the +script writes, and printed success. It was invisible because the test file +reimplemented the copy instead of importing it. Both are fixed: the logic moved +to `scripts/lib/leaderboard-migration.mjs` and the tests exercise the real +functions. Verified against live Redis: the wipe scenario now refuses and the +data survives. + +Also corrected: the fan-out had been serialised across levels, which was slower +than the two-level code it replaced (`Promise.all` restored); an empty +`KEY_PREFIX` would have let `scanKeys` sweep co-tenant data; `?city=` began +returning 500 instead of the global board; and `--dry-run --apply` wrote. + +`npm test` 218/218, `npm run test:integration` 216 passed / 2 pre-existing +skips, lint and build clean. + ## Overview Make a single guess credit its leaf, its province, and Vietnam — without moving, @@ -176,7 +206,6 @@ artefact. key via `scanKeys`, with members and scores, to a timestamped JSON file. **Abort if the export contains zero keys** — an empty export is a wrong-prefix symptom, not a clean slate. -2. Write the legacy baseline snapshot for `HN`, `DN`, `TPHCM` alongside it. 3. `DEL` destination, then copy `:dl` → `:ld` and `:dh` → `:la` with **absolute scores**, score and distance both. 4. Report per-key member counts before and after. Abort if either source is empty. @@ -263,6 +292,5 @@ a fan-out bug. - [ ] `leaderboard:city:ld` equals `:dl`, and `:la` equals `:dh` - [ ] Every pre-existing key is unchanged after the migration - [ ] The migration prints its prefix, aborts on an empty export, requires `--confirm-prefix`, and `DEL`s before copying -- [ ] The legacy baseline snapshot for HN/DN/TPHCM is committed - [ ] Migration ordering is documented as deploy-then-migrate - [ ] `npm test` and `npm run test:integration` pass diff --git a/plans/260830-1345-nested-region-tree/plan.md b/plans/260830-1345-nested-region-tree/plan.md index ca0365d..337e317 100644 --- a/plans/260830-1345-nested-region-tree/plan.md +++ b/plans/260830-1345-nested-region-tree/plan.md @@ -220,7 +220,7 @@ first two. |---|-------|--------|-----------| | 1 | [Phase 1: Region tree model and boundaries](./phase-01-region-tree-model-and-boundaries.md) | Completed | — | | 2 | [Phase 2: Panorama district partition](./phase-02-panorama-district-partition.md) | Completed | 1 | -| 3 | [Phase 3: Leaderboard fan-out and migration](./phase-03-leaderboard-fan-out-and-migration.md) | Pending | 1 | +| 3 | [Phase 3: Leaderboard fan-out and migration](./phase-03-leaderboard-fan-out-and-migration.md) | Completed | 1 | | 4 | [Phase 4: API surface](./phase-04-api-surface.md) | Pending | 2, 3 | | 5 | [Phase 5: UI region navigation](./phase-05-ui-region-navigation.md) | Pending | 4 | | 6 | [Phase 6: Docs and verification](./phase-06-docs-and-verification.md) | Pending | 5 | @@ -237,7 +237,8 @@ first two. - [ ] `leaderboard:vietnam` and `distance:vietnam` remain the country node's keys — pre-change totals unchanged and still growing - [ ] `leaderboard:city:hn` / `:dn` / `:tphcm` / `:dl` / `:dh` totals are unchanged immediately after deploy - [ ] `leaderboard:city:ld` equals `leaderboard:city:dl` and `leaderboard:city:la` equals `leaderboard:city:dh` immediately after migration -- [ ] The migration aborts if its export contains zero keys, and prints the resolved `KEY_PREFIX` +- [ ] The migration aborts if its export contains zero keys, refuses an empty source that would wipe a populated destination, and prints the resolved `KEY_PREFIX` +- [ ] The migration verifies the backfill landed, and can restore its own backup - [ ] `/game?location=HN` and `/game?location=DL` still work - [ ] A leaf below the playability threshold is listed in the tree but not offered as playable - [ ] 200 draws at `VN` hit at least four distinct provinces diff --git a/scripts/lib/leaderboard-migration.mjs b/scripts/lib/leaderboard-migration.mjs new file mode 100644 index 0000000..0953f58 --- /dev/null +++ b/scripts/lib/leaderboard-migration.mjs @@ -0,0 +1,161 @@ +// Backfill logic for the leaderboard migration. +// +// Separated from the CLI in scripts/migrate-leaderboards.mjs so every guard is +// callable from a test. This is the one place in the region-tree change where +// existing player points can be destroyed, and a script whose safety checks are +// only reachable by running it against a live database is not testable at all. + +import { + scanKeys, + zAdd, + zRangeWithScores, + zRemRangeByRank, +} from '../../src/lib/upstash.js'; +import { leaderboardKeys } from '../../src/lib/leaderboard.js'; + +// child -> parent. Lam Dong and Long An are new nodes whose only child already +// holds the full history. +export const BACKFILLS = [ + { from: 'DL', to: 'LD', label: 'Da Lat -> Lam Dong' }, + { from: 'DH', to: 'LA', label: 'Duc Hoa -> Long An' }, +]; + +export const PATTERNS = ['leaderboard:*', 'distance:*']; + +/** + * Every key each backfill pair touches, score and distance. + * + * Built through the same key functions the app uses, so a change to key naming + * cannot leave the migration writing to the old names. + * @returns {Array<{fromKey: string, toKey: string, label: string}>} Pairs. + */ +export function backfillPairs() { + return BACKFILLS.flatMap(({ from, to, label }) => [ + { fromKey: leaderboardKeys.score(from), toKey: leaderboardKeys.score(to), label }, + { fromKey: leaderboardKeys.distance(from), toKey: leaderboardKeys.distance(to), label }, + ]); +} + +/** + * Read every leaderboard key with its members and scores. + * @param {Object} h Upstash handle. + * @returns {Promise} Logical key -> [{value, score}]. + */ +export async function exportAll(h) { + const snapshot = {}; + for (const pattern of PATTERNS) { + for (const key of await scanKeys(h, pattern)) { + snapshot[key] = await zRangeWithScores(h, key, 0, -1, false); + } + } + return snapshot; +} + +/** + * Copy one sorted set over another, replacing what was there. + * + * Absolute scores, not increments, so re-running converges instead of doubling. + * The destination is emptied first: writing member-by-member would leave behind + * anyone present only in the destination, which the 200-entry trim makes + * reachable -- a player can be trimmed out of the source while surviving in the + * copy, and would then keep a stale score forever. + * + * An empty source is refused BEFORE the delete. Otherwise this wipes the + * destination and writes nothing back, which is indistinguishable from a + * successful no-op in the output and destroys real points. + * @param {Object} h Upstash handle. + * @param {string} fromKey Source logical key. + * @param {string} toKey Destination logical key. + * @param {boolean} apply False to report without writing. + * @returns {Promise<{members: number, replaced: number, skipped: boolean}>} + */ +export async function copySortedSet(h, fromKey, toKey, apply) { + const source = await zRangeWithScores(h, fromKey, 0, -1, false); + const destination = await zRangeWithScores(h, toKey, 0, -1, false); + + if (source.length === 0) { + if (destination.length > 0) { + throw new Error( + `${fromKey} is empty but ${toKey} holds ${destination.length} members. ` + + 'Copying would delete them. Refusing: check KEY_PREFIX and the source key.' + ); + } + // Both empty is the ordinary case for a board nobody has played yet. + return { members: 0, replaced: 0, skipped: true }; + } + + if (apply) { + await zRemRangeByRank(h, toKey, 0, -1); + for (const { value, score } of source) await zAdd(h, toKey, score, value); + } + return { members: source.length, replaced: destination.length, skipped: false }; +} + +/** + * Confirm the backfill landed: each destination must now equal its source. + * + * The earlier version verified only the keys it did NOT write, which meant the + * copy itself was never checked. + * @param {Object} h Upstash handle. + * @returns {Promise} Pairs that disagree, empty when all match. + */ +export async function verifyTargets(h) { + const mismatched = []; + for (const { fromKey, toKey } of backfillPairs()) { + const source = await zRangeWithScores(h, fromKey, 0, -1, false); + const destination = await zRangeWithScores(h, toKey, 0, -1, false); + if (JSON.stringify(source) !== JSON.stringify(destination)) { + mismatched.push(`${toKey} does not match ${fromKey}`); + } + } + return mismatched; +} + +/** + * Check that no untouched key lost anything while the migration ran. + * + * Ordering is deploy-then-migrate, so the app is serving guesses throughout and + * boards legitimately grow mid-run. Demanding byte-equality would throw on a + * healthy migration the moment one player scores. Forward-only drift -- members + * added, scores never reduced, nobody removed -- is expected; anything else is + * not. + * @param {Object} before Snapshot taken before the writes. + * @param {Object} after Snapshot taken after. + * @returns {string[]} Keys that regressed, empty when all are intact. + */ +export function findRegressions(before, after) { + const targets = new Set(backfillPairs().map(({ toKey }) => toKey)); + const regressed = []; + + for (const [key, entries] of Object.entries(before)) { + if (targets.has(key)) continue; + const now = after[key]; + if (!now) { + regressed.push(`${key} disappeared`); + continue; + } + const nowByMember = new Map(now.map((e) => [e.value, e.score])); + for (const { value, score } of entries) { + if (!nowByMember.has(value)) { + regressed.push(`${key}: member ${value} removed`); + } else if (nowByMember.get(value) < score) { + regressed.push(`${key}: ${value} fell from ${score} to ${nowByMember.get(value)}`); + } + } + } + return regressed; +} + +/** + * Restore a snapshot, making the backup an actual rollback rather than a file. + * @param {Object} h Upstash handle. + * @param {Object} snapshot Output of exportAll. + * @returns {Promise} Keys restored. + */ +export async function restore(h, snapshot) { + for (const [key, entries] of Object.entries(snapshot)) { + await zRemRangeByRank(h, key, 0, -1); + for (const { value, score } of entries) await zAdd(h, key, score, value); + } + return Object.keys(snapshot).length; +} diff --git a/scripts/migrate-leaderboards.mjs b/scripts/migrate-leaderboards.mjs new file mode 100644 index 0000000..31b5a8b --- /dev/null +++ b/scripts/migrate-leaderboards.mjs @@ -0,0 +1,123 @@ +// Seed the two new province leaderboards from the towns they inherited. +// +// node scripts/migrate-leaderboards.mjs # dry run +// node scripts/migrate-leaderboards.mjs --apply --confirm-prefix=vngeoguessr: +// node scripts/migrate-leaderboards.mjs --restore= --confirm-prefix=... +// +// Lam Dong and Long An are new nodes. Their only child -- Da Lat and Duc Hoa -- +// already holds the full history, so without a backfill each province would +// disagree with its own single child from day one. +// +// RUN THIS AFTER DEPLOYING THE FAN-OUT, not before. Migrating first opens a +// window where a Da Lat guess credits DL and Vietnam but not Lam Dong, and the +// province is permanently short by that window. Deploying first is safe: a +// pre-migration LD only accumulates a subset of DL's deltas, which the copy +// then overwrites. +// +// Nothing else is touched. Ha Noi, Da Nang and Ho Chi Minh keep their history +// where it is: those points predate districts and cannot be attributed to one. +// +// The logic lives in scripts/lib/leaderboard-migration.mjs so its guards are +// testable; this file is argument handling and reporting. + +import { writeFileSync, readFileSync } from 'node:fs'; +import { getUpstash } from '../src/lib/upstash.js'; +import { + backfillPairs, + copySortedSet, + exportAll, + findRegressions, + restore, + verifyTargets, +} from './lib/leaderboard-migration.mjs'; + +const args = process.argv.slice(2); +const apply = args.includes('--apply'); +const dryRunFlag = args.includes('--dry-run'); +const restorePath = args.find((arg) => arg.startsWith('--restore='))?.split('=')[1]; +const confirmed = args.find((arg) => arg.startsWith('--confirm-prefix='))?.split('=')[1]; + +if (dryRunFlag && apply) { + // Typing both is a reasonable belt-and-braces instinct, and silently writing + // would be the worst possible answer to it. + throw new Error('--dry-run and --apply are mutually exclusive'); +} + +const h = getUpstash(); +const writing = apply || Boolean(restorePath); +console.log(`key prefix: ${JSON.stringify(h.prefix)}`); +console.log(writing ? 'mode: WRITES ENABLED' : 'mode: dry run (no writes)'); + +if (writing && confirmed !== h.prefix) { + // The prefix decides which namespace this touches, and getting it wrong is + // silent: every read returns empty and every write lands somewhere nothing + // reads. Make the operator name it. + throw new Error( + `Refusing to write: pass --confirm-prefix=${h.prefix} to confirm the target namespace` + + (confirmed !== undefined ? ` (got ${JSON.stringify(confirmed)})` : '') + ); +} + +/** Put a snapshot back. The backup is only a rollback if something reads it. */ +async function runRestore(path) { + const snapshot = JSON.parse(readFileSync(path, 'utf8')); + const count = await restore(h, snapshot); + console.log(`restored ${count} keys from ${path}`); +} + +/** Back-fill the two new province boards from their single child. */ +async function runBackfill() { + const before = await exportAll(h); + const keyCount = Object.keys(before).length; + console.log(`exported ${keyCount} leaderboard keys`); + + if (keyCount === 0) { + // An empty export is far more likely to be a wrong prefix than an empty + // database, and it would make the backup -- the only rollback -- worthless. + throw new Error( + 'Export found no leaderboard keys. That usually means KEY_PREFIX does not ' + + 'match the deployment. Refusing to continue: the backup would be empty.' + ); + } + + const stamp = new Date().toISOString().replace(/[:.]/g, '-'); + const backupPath = `leaderboard-backup-${stamp}.json`; + writeFileSync(backupPath, JSON.stringify(before, null, 1) + '\n'); + console.log(`backup -> ${backupPath} (restore with --restore=${backupPath})`); + + console.log(''); + for (const { fromKey, toKey, label } of backfillPairs()) { + const { members, replaced, skipped } = await copySortedSet(h, fromKey, toKey, apply); + console.log( + `${label.padEnd(20)} ${fromKey} -> ${toKey}: ` + + (skipped ? 'both empty, nothing to do' : `${members} members`) + + (replaced ? `, replacing ${replaced} already there` : '') + + (apply || skipped ? '' : ' [dry run]') + ); + } + + if (!apply) { + console.log(`\nNothing written. Re-run with --apply --confirm-prefix=${h.prefix}`); + return; + } + + // Two checks, because they catch different failures: the destinations must + // now match their sources, and nothing else may have gone backwards. + const mismatched = await verifyTargets(h); + if (mismatched.length > 0) { + throw new Error(`Backfill did not land: ${mismatched.join('; ')}`); + } + + const regressed = findRegressions(before, await exportAll(h)); + if (regressed.length > 0) { + throw new Error( + `Existing data regressed: ${regressed.join('; ')}. ` + + `Restore with --restore=${backupPath} --confirm-prefix=${h.prefix}` + ); + } + + console.log(`\napplied. Backfill verified, and ${keyCount} pre-existing keys intact.`); +} + +if (restorePath) await runRestore(restorePath); +else await runBackfill(); diff --git a/src/lib/leaderboard.js b/src/lib/leaderboard.js index 9d3a4d2..0a4f358 100644 --- a/src/lib/leaderboard.js +++ b/src/lib/leaderboard.js @@ -7,45 +7,96 @@ import { zRevRank, zRemRangeByRank, } from './upstash.js'; +import { ancestorsOf, getRegion, isRegion, COUNTRY_CODE } from './regions.js'; // Leaderboard logical key constants (prefix is applied inside the adapter). +// +// A guess is credited to the district its panorama sits in, then rolled upward: +// district, province, country. Each level keeps its own board, so a player can +// top District 7 without touching the national table. const GLOBAL_LEADERBOARD_KEY = 'leaderboard:vietnam'; const CITY_LEADERBOARD_PREFIX = 'leaderboard:city:'; const DISTANCE_GLOBAL_KEY = 'distance:vietnam'; const DISTANCE_CITY_PREFIX = 'distance:city:'; const MAX_LEADERBOARD_SIZE = 200; +// A caller asking for more than the board can hold is asking for the board. +const MAX_LIMIT = MAX_LEADERBOARD_SIZE; -// Helper: city-specific score leaderboard key. -function getCityLeaderboardKey(cityCode) { - return `${CITY_LEADERBOARD_PREFIX}${cityCode.toLowerCase()}`; -} - -// Helper: distance leaderboard key (global or city). -function getDistanceLeaderboardKey(cityCode) { - return cityCode ? `${DISTANCE_CITY_PREFIX}${cityCode.toLowerCase()}` : DISTANCE_GLOBAL_KEY; +/** + * Reject anything that is not a region before it reaches a key name. + * + * Validated here rather than only in the route: the routes are not the only + * callers, and `getRegionLeaderboardKey` lowercases whatever it is handed + * straight into a Redis key. A typo would quietly create a board nobody reads. + * @param {string} regionCode Region code. + * @returns {string} The same code. + */ +function requireRegion(regionCode) { + if (!isRegion(regionCode)) { + throw new Error(`Unknown region: ${regionCode}`); + } + return regionCode; } /** - * Get leaderboard (global Vietnam or city-specific). - * @param {string|null} cityCode City code, or null for global. + * Score-board key for a region. + * + * The country maps to the pre-existing global key rather than to + * `leaderboard:city:vn`, so the national board players already have keeps + * accumulating instead of restarting. + * @param {string} regionCode Region code. + * @returns {string} Logical key. + */ +function getRegionLeaderboardKey(regionCode) { + if (regionCode === COUNTRY_CODE) return GLOBAL_LEADERBOARD_KEY; + return `${CITY_LEADERBOARD_PREFIX}${regionCode.toLowerCase()}`; +} + +/** + * Distance-board key for a region. + * @param {string|null} regionCode Region code, or null for the country. + * @returns {string} Logical key. + */ +function getDistanceLeaderboardKey(regionCode) { + if (!regionCode || regionCode === COUNTRY_CODE) return DISTANCE_GLOBAL_KEY; + return `${DISTANCE_CITY_PREFIX}${regionCode.toLowerCase()}`; +} + +// Both key builders, so scripts/lib/leaderboard-migration.mjs derives the keys +// it copies from the same code the app writes through. Hardcoding the prefixes +// there would let a change here silently point the migration at old names. +export const leaderboardKeys = { + score: getRegionLeaderboardKey, + distance: getDistanceLeaderboardKey, +}; + +/** + * Get a leaderboard for one region. + * @param {string|null} regionCode Region code, or null for the country. * @param {number} limit Number of entries to return. * @param {string} type 'score' (highest first) or 'distance' (lowest first). * @returns {Promise} Leaderboard entries. */ -export async function getLeaderboard(cityCode = null, limit = 100, type = 'score') { +export async function getLeaderboard(regionCode = null, limit = 100, type = 'score') { try { const h = getUpstash(); + // `||` not `??`: `?city=` on the route yields an empty string, which used + // to mean the global board and must keep meaning it. + const region = regionCode || COUNTRY_CODE; + requireRegion(region); - let leaderboardKey; - if (type === 'distance') { - leaderboardKey = getDistanceLeaderboardKey(cityCode); - } else { - leaderboardKey = cityCode ? getCityLeaderboardKey(cityCode) : GLOBAL_LEADERBOARD_KEY; - } + // Clamp rather than trust: `parseInt(x) || 100` upstream accepts -1, which + // reaches the adapter as a range of (0, -2) and returns a surprise slice. + const size = Math.min(MAX_LIMIT, Math.max(1, Math.floor(Number(limit) || 100))); - // Score leaderboards: REV=true (highest first). Distance: REV=false (lowest first). - const rev = type === 'score'; - const leaderboardData = await zRangeWithScores(h, leaderboardKey, 0, limit - 1, rev); + const leaderboardKey = + type === 'distance' + ? getDistanceLeaderboardKey(region) + : getRegionLeaderboardKey(region); + + // Score boards read highest first; distance boards lowest first. + const rev = type !== 'distance'; + const leaderboardData = await zRangeWithScores(h, leaderboardKey, 0, size - 1, rev); const entries = []; for (let i = 0; i < leaderboardData.length; i++) { @@ -76,68 +127,83 @@ export async function getLeaderboard(cityCode = null, limit = 100, type = 'score } /** - * Submit a score to both city and global score leaderboards. + * Add a score to one region's board and return the new total and rank. + * @param {Object} h Upstash handle. + * @param {string} regionCode Region code. + * @param {number} score Points to add. + * @param {string} username Player. + * @returns {Promise} Level result. + */ +async function creditScore(h, regionCode, score, username) { + const key = getRegionLeaderboardKey(regionCode); + const existing = await zScore(h, key, username); + const total = (existing || 0) + score; + + await zAdd(h, key, total, username); + // Trim to the top MAX_LEADERBOARD_SIZE. The set is ascending, so drop the + // lowest-ranked entries that fall outside the window. + await zRemRangeByRank(h, key, 0, -(MAX_LEADERBOARD_SIZE + 1)); + + const rank = await zRevRank(h, key, username); + // A player outside the top 200 is trimmed straight back out, so the total + // just computed is no longer stored anywhere. Reporting it would show a + // national score that silently resets on the next guess. + const trimmed = rank === null; + return { + code: regionCode, + name: getRegion(regionCode).name, + username, + score: trimmed ? null : Number(total), + rank: trimmed ? null : rank + 1, + trimmed, + }; +} + +/** + * Submit a score to a region and every region above it. + * + * Fans out over the ancestor chain, so a guess in District 7 credits the + * district, Ho Chi Minh, and Vietnam by the same amount. * @param {string} username Player username. * @param {number} score Score achieved (0-5). - * @param {string} cityCode City code. - * @returns {Promise} Submission result with both city and global ranks. + * @param {string} regionCode Region the panorama was in. + * @returns {Promise} Per-level results. */ -export async function submitScore(username, score, cityCode) { +export async function submitScore(username, score, regionCode) { try { const h = getUpstash(); - if (!username || score === undefined || !cityCode) { - throw new Error('Missing required fields: username, score, cityCode'); + if (!username || score === undefined || !regionCode) { + throw new Error('Missing required fields: username, score, regionCode'); } + requireRegion(regionCode); const trimmedUsername = username.trim(); const numScore = Number(score); - const globalKey = GLOBAL_LEADERBOARD_KEY; - const cityKey = getCityLeaderboardKey(cityCode); + // In parallel: the levels are independent keys and no level reads another's + // state, so serialising them would add two round trips of latency to every + // guess for nothing. The four calls WITHIN a level stay ordered. + const levels = await Promise.all( + ancestorsOf(regionCode).map((code) => creditScore(h, code, numScore, trimmedUsername)) + ); - const [globalExisting, cityExisting] = await Promise.all([ - zScore(h, globalKey, trimmedUsername), - zScore(h, cityKey, trimmedUsername), - ]); - - const globalNewTotal = (globalExisting || 0) + numScore; - const cityNewTotal = (cityExisting || 0) + numScore; - - await Promise.all([ - zAdd(h, globalKey, globalNewTotal, trimmedUsername), - zAdd(h, cityKey, cityNewTotal, trimmedUsername), - ]); - - // Trim to top MAX_LEADERBOARD_SIZE (highest scores). Sorted set is ascending, - // so remove the lowest-ranked entries that fall outside the top window. - await Promise.all([ - zRemRangeByRank(h, globalKey, 0, -(MAX_LEADERBOARD_SIZE + 1)), - zRemRangeByRank(h, cityKey, 0, -(MAX_LEADERBOARD_SIZE + 1)), - ]); - - const [globalRank, cityRank] = await Promise.all([ - zRevRank(h, globalKey, trimmedUsername), - zRevRank(h, cityKey, trimmedUsername), - ]); - - const actualGlobalRank = globalRank !== null ? globalRank + 1 : null; - const actualCityRank = cityRank !== null ? cityRank + 1 : null; + // Named aliases alongside the array: the chain is district -> province -> + // country for a leaf, but only province -> country when a panorama fell + // outside every district polygon, so callers cannot index by position. + const byLevel = (level) => + levels.find((entry) => getRegion(entry.code).level === level) ?? null; return { success: true, - global: { - username: trimmedUsername, - score: Number(globalNewTotal), - rank: actualGlobalRank, - }, - city: { - username: trimmedUsername, - score: Number(cityNewTotal), - rank: actualCityRank, - cityCode: cityCode, - }, - message: `Score added! City: ${cityNewTotal} (+${numScore}), Global: ${globalNewTotal} (+${numScore})`, + levels, + district: byLevel('district'), + province: byLevel('province'), + global: byLevel('country'), + // `city` is the pre-tree name for the province level. Kept so /api/guess + // keeps reporting a rank until the API surface moves to `levels`. + city: byLevel('province'), + message: `Score added at ${levels.length} levels (+${numScore})`, }; } catch (error) { console.error('Error submitting score:', error); @@ -146,64 +212,72 @@ export async function submitScore(username, score, cityCode) { } /** - * Submit a distance record to both city and global distance leaderboards. - * @param {string} username Player username. - * @param {number} distance Distance achieved in meters. - * @param {string} cityCode City code. - * @returns {Promise} Submission result with distance ranks. + * Add one distance record to a region's board. + * @param {Object} h Upstash handle. + * @param {string} regionCode Region code. + * @param {number} distance Distance in metres. + * @param {string} entryId Shared id for this record across levels. + * @param {string} username Player. + * @returns {Promise} Level result. */ -export async function submitDistanceRecord(username, distance, cityCode) { +async function creditDistance(h, regionCode, distance, entryId, username) { + const key = getDistanceLeaderboardKey(regionCode); + + // Lower is better, so the distance is the score directly. + await zAdd(h, key, distance, entryId); + // Ascending set, so everything past the window is the worst. + await zRemRangeByRank(h, key, MAX_LEADERBOARD_SIZE, -1); + + const rank = await zRank(h, key, entryId); + return { + code: regionCode, + name: getRegion(regionCode).name, + username, + distance, + rank: rank !== null ? rank + 1 : null, + }; +} + +/** + * Submit a distance record to a region and every region above it. + * @param {string} username Player username. + * @param {number} distance Distance achieved in metres. + * @param {string} regionCode Region the panorama was in. + * @returns {Promise} Per-level results. + */ +export async function submitDistanceRecord(username, distance, regionCode) { try { const h = getUpstash(); - if (!username || distance === undefined || !cityCode) { - throw new Error('Missing required fields: username, distance, cityCode'); + if (!username || distance === undefined || !regionCode) { + throw new Error('Missing required fields: username, distance, regionCode'); } + requireRegion(regionCode); const trimmedUsername = username.trim(); const numDistance = Number(distance); - const timestamp = Date.now(); - // Unique entry id so each distance attempt gets its own slot. - const entryId = `${trimmedUsername}:${numDistance}:${timestamp}`; + // One id for all levels, so the same record is recognisable as one attempt + // wherever it appears rather than looking like three separate guesses. + const entryId = `${trimmedUsername}:${numDistance}:${Date.now()}`; - const globalDistanceKey = getDistanceLeaderboardKey(null); - const cityDistanceKey = getDistanceLeaderboardKey(cityCode); + const levels = await Promise.all( + ancestorsOf(regionCode).map((code) => + creditDistance(h, code, numDistance, entryId, trimmedUsername) + ) + ); - // Distance leaderboards: lower = better, so use distance directly as score. - await Promise.all([ - zAdd(h, globalDistanceKey, numDistance, entryId), - zAdd(h, cityDistanceKey, numDistance, entryId), - ]); - - // Trim to top MAX_LEADERBOARD_SIZE (lowest distances). Sorted set is ascending, - // so remove ranks beyond MAX_LEADERBOARD_SIZE (the worst entries). - await Promise.all([ - zRemRangeByRank(h, globalDistanceKey, MAX_LEADERBOARD_SIZE, -1), - zRemRangeByRank(h, cityDistanceKey, MAX_LEADERBOARD_SIZE, -1), - ]); - - const [globalRank, cityRank] = await Promise.all([ - zRank(h, globalDistanceKey, entryId), - zRank(h, cityDistanceKey, entryId), - ]); - - const actualGlobalRank = globalRank !== null ? globalRank + 1 : null; - const actualCityRank = cityRank !== null ? cityRank + 1 : null; + const byLevel = (level) => + levels.find((entry) => getRegion(entry.code).level === level) ?? null; return { success: true, - globalDistance: { - username: trimmedUsername, - distance: numDistance, - rank: actualGlobalRank, - }, - cityDistance: { - username: trimmedUsername, - distance: numDistance, - rank: actualCityRank, - cityCode: cityCode, - }, + levels, + districtDistance: byLevel('district'), + provinceDistance: byLevel('province'), + globalDistance: byLevel('country'), + // Pre-tree name for the province level; see submitScore. + cityDistance: byLevel('province'), message: `Distance record: ${numDistance}m`, }; } catch (error) { diff --git a/src/lib/upstash.js b/src/lib/upstash.js index b8498b3..ab0d0a5 100644 --- a/src/lib/upstash.js +++ b/src/lib/upstash.js @@ -4,10 +4,16 @@ import { Redis } from '@upstash/redis'; // // Logical key namespace (callers pass these unprefixed): // session:{sessionId} string, TTL 30 min -// leaderboard:vietnam sorted set (score) -// leaderboard:city:{cityCode} sorted set (score) -// distance:vietnam sorted set (distance) -// distance:city:{cityCode} sorted set (distance) +// leaderboard:vietnam sorted set (score) -- the VN node +// leaderboard:city:{regionCode} sorted set (score) +// distance:vietnam sorted set (distance) -- the VN node +// distance:city:{regionCode} sorted set (distance) +// +// The ':city:' segment is a misnomer now that regions form a country > +// province > district tree, and {regionCode} may be any node below the +// country: 'tphcm', 'tphcm-q7', 'dl'. It is kept because renaming it would +// strand every key already holding a player's history. The country keeps its +// own two legacy key names for the same reason. // // Multi-tenancy: every physical Upstash key carries KEY_PREFIX (default // 'vngeoguessr:') so this project can safely share an Upstash DB with other @@ -31,7 +37,11 @@ export function getUpstash() { if (!url) throw new Error('UPSTASH_REDIS_REST_URL or KV_REST_API_URL is required'); if (!token) throw new Error('UPSTASH_REDIS_REST_TOKEN or KV_REST_API_TOKEN is required'); const client = new Redis({ url, token }); - const prefix = process.env.KEY_PREFIX ?? DEFAULT_KEY_PREFIX; + // `??` would accept KEY_PREFIX= (set but empty), which removes the only thing + // keeping this project's keys apart from every other project sharing the + // Upstash database. That was harmless while the adapter only touched keys it + // named; scanKeys enumerates, so an empty prefix would sweep co-tenant data. + const prefix = process.env.KEY_PREFIX || DEFAULT_KEY_PREFIX; handle = { client, prefix }; return handle; } @@ -142,6 +152,42 @@ export async function zRevRank(h, key, member) { return result == null ? null : Number(result); } +/** + * Every logical key matching a pattern. + * + * The prefix is applied to the pattern and stripped from the results, so + * callers work in the same logical namespace as every other function here. A + * caller that reached past the adapter and scanned `leaderboard:*` directly + * would match nothing, because the physical keys all carry KEY_PREFIX -- and an + * empty result is indistinguishable from an empty database, which is how a + * backup silently succeeds against the wrong namespace. + * + * Uses SCAN rather than KEYS: KEYS blocks the server for the whole sweep. + * @param {{ client: Redis, prefix: string }} h + * @param {string} pattern Logical glob, e.g. 'leaderboard:*'. + * @returns {Promise} Logical keys, prefix removed. + */ +export async function scanKeys(h, pattern) { + // SCAN guarantees each key is returned at least once, not exactly once: a + // key can repeat across pages when the keyspace rehashes mid-sweep. + const found = new Set(); + let cursor = '0'; + do { + const [next, batch] = await h.client.scan(cursor, { + match: pkey(h, pattern), + count: 500, + }); + cursor = String(next); + for (const key of batch) { + // Only strip a prefix that is actually there. Blind slicing would mangle + // a key, and with an empty prefix would hand back another tenant's. + if (!key.startsWith(h.prefix)) continue; + found.add(key.slice(h.prefix.length)); + } + } while (cursor !== '0'); + return [...found]; +} + /** * Trim a sorted set to a rank range. * @param {{ client: Redis, prefix: string }} h diff --git a/tests/fake-upstash-redis.js b/tests/fake-upstash-redis.js index 8a7dc32..b66b229 100644 --- a/tests/fake-upstash-redis.js +++ b/tests/fake-upstash-redis.js @@ -85,6 +85,31 @@ export class FakeRedis { return had || hadZset ? 1 : 0; } + /** + * Cursor-based key scan. + * + * Returns everything in one page: the store is in-memory, so a real cursor + * would only add a loop the tests cannot meaningfully exercise. Callers still + * have to handle the cursor protocol, because the real client does paginate. + */ + async scan(cursor, opts = {}) { + const keys = [...this.strings.keys(), ...this.zsets.keys()]; + const live = keys.filter((key) => !this.#expired(key)); + if (!opts.match) return ['0', live]; + + // Redis glob: * spans any run of characters, ? one. Everything else in the + // pattern is literal, so escape it before substituting the wildcards. + const escaped = opts.match.replace(/[.+^${}()|[\]\\]/g, '\\$&'); + const pattern = new RegExp(`^${escaped.replace(/\*/g, '.*').replace(/\?/g, '.')}$`); + return ['0', live.filter((key) => pattern.test(key))]; + } + + /** True when a string key's TTL has elapsed. Sorted sets never expire here. */ + #expired(key) { + const entry = this.strings.get(key); + return Boolean(entry?.expireAt && entry.expireAt <= Date.now()); + } + async zadd(key, { score, member }) { let zset = this.zsets.get(key); if (!zset) { diff --git a/tests/leaderboard.test.js b/tests/leaderboard.test.js index ce500ec..ba36a62 100644 --- a/tests/leaderboard.test.js +++ b/tests/leaderboard.test.js @@ -189,3 +189,112 @@ describe('global leaderboard', () => { expect(await getLeaderboard()).toEqual([{ username: 'anh', score: 5, rank: 1 }]); }); }); + +describe('fan-out up the region tree', () => { + beforeEach(async () => { + await resetStore(); + }); + + it('credits the district, its province and the country by the same amount', async () => { + const result = await submitScore('mai', 3, 'TPHCM-Q7'); + expect(result.levels.map((l) => l.code)).toEqual(['TPHCM-Q7', 'TPHCM', 'VN']); + for (const level of result.levels) expect(level.score).toBe(3); + + const keys = await storedKeys(); + expect(keys).toContain('vngeoguessr:leaderboard:city:tphcm-q7'); + expect(keys).toContain('vngeoguessr:leaderboard:city:tphcm'); + expect(keys).toContain('vngeoguessr:leaderboard:vietnam'); + }); + + it('rolls a second district into the same province and country totals', async () => { + await submitScore('mai', 3, 'TPHCM-Q7'); + const second = await submitScore('mai', 2, 'TPHCM-Q1'); + + expect(second.district.score).toBe(2); // Q1 alone + expect(second.province.score).toBe(5); // Q7 + Q1 + expect(second.global.score).toBe(5); + }); + + it('keeps Da Lat under Lam Dong', async () => { + // Pre-2025 units: Duc Hoa belongs to Long An, not the Tay Ninh it merged + // into. Both leaf codes stay bare so their history stays attached. + const daLat = await submitScore('mai', 4, 'DL'); + expect(daLat.levels.map((l) => l.code)).toEqual(['DL', 'LD', 'VN']); + + const ducHoa = await submitScore('mai', 1, 'DH'); + expect(ducHoa.levels.map((l) => l.code)).toEqual(['DH', 'LA', 'VN']); + expect(ducHoa.global.score).toBe(5); + }); + + it('credits two levels when the panorama sat outside every district', async () => { + // A province-level code is a legitimate scoring target: the panorama fell + // in a gap between simplified district outlines. + const result = await submitScore('mai', 2, 'DN'); + expect(result.levels.map((l) => l.code)).toEqual(['DN', 'VN']); + expect(result.district).toBeNull(); + expect(result.province.score).toBe(2); + }); + + it('maps the country to the pre-existing global key', async () => { + // Not leaderboard:city:vn -- the national board players already have has to + // keep accumulating rather than restarting under a new name. + await submitScore('mai', 1, 'DL'); + const keys = await storedKeys(); + expect(keys).toContain('vngeoguessr:leaderboard:vietnam'); + expect(keys).not.toContain('vngeoguessr:leaderboard:city:vn'); + }); + + it('fans distance records out with one shared id', async () => { + const result = await submitDistanceRecord('mai', 250, 'TPHCM-Q7'); + expect(result.levels.map((l) => l.code)).toEqual(['TPHCM-Q7', 'TPHCM', 'VN']); + + // The same attempt should be recognisable as one record at every level. + const district = await getLeaderboard('TPHCM-Q7', 10, 'distance'); + const country = await getLeaderboard('VN', 10, 'distance'); + expect(district[0].timestamp).toBe(country[0].timestamp); + expect(district[0].distance).toBe(250); + }); +}); + +describe('region validation', () => { + beforeEach(async () => { + await resetStore(); + }); + + it.each([ + ['submitScore', () => submitScore('mai', 3, 'NOPE')], + ['submitDistanceRecord', () => submitDistanceRecord('mai', 100, 'NOPE')], + ['getLeaderboard', () => getLeaderboard('NOPE')], + ])('%s rejects an unknown region', async (_name, call) => { + // Validated in the library, not only in the route: the migration script and + // any future caller bypass routes entirely, and the key builder lowercases + // whatever it is handed straight into a Redis key. + await expect(call()).rejects.toThrow(/Unknown region/); + }); + + it('creates no key for a rejected region', async () => { + await expect(submitScore('mai', 3, 'NOPE')).rejects.toThrow(); + expect(await storedKeys()).toEqual([]); + }); + + it.each([ + // 0 and NaN are falsy, so they take the default of 100 -- the same + // behaviour as the `parseInt(...) || 100` this replaced. + [-1, 1], + [0, 100], + [NaN, 100], + [99999, 200], + [Infinity, 200], + ['50', 50], + ])('clamps a limit of %s to %i rows', async (limit, expected) => { + // Seeded past the cap so each clamp lands on a distinguishable length. A + // board with one entry cannot tell a working clamp from a missing one. + for (let i = 0; i < 250; i++) await submitScore(`p${i}`, (i % 5) + 1, 'DL'); + expect((await getLeaderboard('DL', limit, 'score')).length).toBe(expected); + }); + + it('treats a null region as the country', async () => { + await submitScore('mai', 3, 'DL'); + expect(await getLeaderboard(null)).toEqual(await getLeaderboard('VN')); + }); +}); diff --git a/tests/migrate-leaderboards.test.js b/tests/migrate-leaderboards.test.js new file mode 100644 index 0000000..6c8f86a --- /dev/null +++ b/tests/migrate-leaderboards.test.js @@ -0,0 +1,216 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +vi.mock('@upstash/redis', async (importOriginal) => { + const { upstashModule } = await import('./mock-upstash.js'); + return upstashModule(importOriginal); +}); + +import { getUpstash, scanKeys, zAdd, zRangeWithScores } from '../src/lib/upstash.js'; +import { + backfillPairs, + copySortedSet, + exportAll, + findRegressions, + restore, + verifyTargets, +} from '../scripts/lib/leaderboard-migration.mjs'; +import { resetStore } from './redis-harness.js'; + +// The migration is the one place in this change where existing points can be +// lost, so its guards are exercised against the real functions the script runs. +// An earlier version of this file reimplemented the copy locally, which meant +// the missing empty-source guard was invisible to the suite. + +const h = () => getUpstash(); + +describe('scanKeys', () => { + beforeEach(async () => { + await resetStore(); + }); + + it('finds leaderboard keys and hands them back unprefixed', async () => { + // Callers work in the logical namespace. A migration that reached past the + // adapter and scanned 'leaderboard:*' directly would match nothing, because + // every physical key carries KEY_PREFIX. + await zAdd(h(), 'leaderboard:city:dl', 5, 'mai'); + await zAdd(h(), 'leaderboard:vietnam', 5, 'mai'); + await zAdd(h(), 'distance:city:dl', 120, 'mai:120:1'); + + const found = await scanKeys(h(), 'leaderboard:*'); + expect(found.sort()).toEqual(['leaderboard:city:dl', 'leaderboard:vietnam']); + for (const key of found) expect(key.startsWith(h().prefix)).toBe(false); + }); + + it('keeps one pattern out of another namespace', async () => { + await zAdd(h(), 'leaderboard:city:dl', 5, 'mai'); + await zAdd(h(), 'distance:city:dl', 120, 'mai:120:1'); + expect(await scanKeys(h(), 'distance:*')).toEqual(['distance:city:dl']); + }); + + it('returns nothing for a pattern that matches nothing', async () => { + await zAdd(h(), 'leaderboard:city:dl', 5, 'mai'); + expect(await scanKeys(h(), 'nosuch:*')).toEqual([]); + }); + + it('never returns a duplicate', async () => { + // SCAN promises each key at least once, not exactly once. + await zAdd(h(), 'leaderboard:city:dl', 5, 'mai'); + const found = await scanKeys(h(), 'leaderboard:*'); + expect(new Set(found).size).toBe(found.length); + }); +}); + +describe('backfill safety', () => { + beforeEach(async () => { + await resetStore(); + }); + + it('refuses an empty source that would wipe a populated destination', async () => { + // The dangerous case: DEL-then-copy with nothing to copy back empties the + // destination and looks like a clean no-op in the output. + await zAdd(h(), 'leaderboard:city:ld', 42, 'mai'); + + await expect( + copySortedSet(h(), 'leaderboard:city:dl', 'leaderboard:city:ld', true) + ).rejects.toThrow(/Refusing/); + + const survived = await zRangeWithScores(h(), 'leaderboard:city:ld', 0, -1, false); + expect(survived).toEqual([{ value: 'mai', score: 42 }]); + }); + + it('treats both-empty as an ordinary no-op', async () => { + const result = await copySortedSet( + h(), + 'leaderboard:city:dh', + 'leaderboard:city:la', + true + ); + expect(result.skipped).toBe(true); + }); + + it('writes nothing in dry-run mode', async () => { + await zAdd(h(), 'leaderboard:city:dl', 12, 'mai'); + await copySortedSet(h(), 'leaderboard:city:dl', 'leaderboard:city:ld', false); + expect(await zRangeWithScores(h(), 'leaderboard:city:ld', 0, -1, false)).toEqual([]); + }); + + it('copies absolute scores, so running twice is a no-op', async () => { + await zAdd(h(), 'leaderboard:city:dl', 12, 'mai'); + await zAdd(h(), 'leaderboard:city:dl', 7, 'linh'); + + await copySortedSet(h(), 'leaderboard:city:dl', 'leaderboard:city:ld', true); + const once = await zRangeWithScores(h(), 'leaderboard:city:ld', 0, -1, false); + await copySortedSet(h(), 'leaderboard:city:dl', 'leaderboard:city:ld', true); + const twice = await zRangeWithScores(h(), 'leaderboard:city:ld', 0, -1, false); + + // Adding instead of replacing would double every score on the second run. + expect(twice).toEqual(once); + expect(once.find((e) => e.value === 'mai').score).toBe(12); + }); + + it('removes a destination-only member instead of stranding it', async () => { + // Reachable in practice: the 200-entry trim can drop a player from the + // source while they survive in the copy, and a member-by-member write would + // leave them there with a stale score forever. + await zAdd(h(), 'leaderboard:city:dl', 12, 'mai'); + await zAdd(h(), 'leaderboard:city:ld', 999, 'orphan'); + + await copySortedSet(h(), 'leaderboard:city:dl', 'leaderboard:city:ld', true); + + const after = await zRangeWithScores(h(), 'leaderboard:city:ld', 0, -1, false); + expect(after.map((e) => e.value)).toEqual(['mai']); + }); + + it('leaves the source untouched', async () => { + await zAdd(h(), 'leaderboard:city:dh', 8, 'mai'); + const before = await zRangeWithScores(h(), 'leaderboard:city:dh', 0, -1, false); + await copySortedSet(h(), 'leaderboard:city:dh', 'leaderboard:city:la', true); + expect(await zRangeWithScores(h(), 'leaderboard:city:dh', 0, -1, false)).toEqual(before); + }); +}); + +describe('backfill verification', () => { + beforeEach(async () => { + await resetStore(); + }); + + it('derives its keys from the app, not from hardcoded strings', async () => { + const pairs = backfillPairs(); + expect(pairs.map((p) => `${p.fromKey}->${p.toKey}`)).toEqual([ + 'leaderboard:city:dl->leaderboard:city:ld', + 'distance:city:dl->distance:city:ld', + 'leaderboard:city:dh->leaderboard:city:la', + 'distance:city:dh->distance:city:la', + ]); + }); + + it('confirms each destination matches its source', async () => { + await zAdd(h(), 'leaderboard:city:dl', 12, 'mai'); + expect(await verifyTargets(h())).not.toEqual([]); + + await copySortedSet(h(), 'leaderboard:city:dl', 'leaderboard:city:ld', true); + expect(await verifyTargets(h())).toEqual([]); + }); + + it('reports a destination that never got written', async () => { + // The earlier verification excluded exactly the keys the script writes, so + // a failed copy passed silently. + await zAdd(h(), 'leaderboard:city:dl', 12, 'mai'); + const mismatched = await verifyTargets(h()); + expect(mismatched.join()).toMatch(/leaderboard:city:ld/); + }); +}); + +describe('regression detection', () => { + it('accepts forward-only drift from live traffic', () => { + // Ordering is deploy-then-migrate, so players keep scoring while the script + // runs. Demanding byte-equality would throw on a healthy migration. + const before = { 'leaderboard:vietnam': [{ value: 'mai', score: 10 }] }; + const after = { + 'leaderboard:vietnam': [ + { value: 'mai', score: 13 }, + { value: 'linh', score: 4 }, + ], + }; + expect(findRegressions(before, after)).toEqual([]); + }); + + it('flags a score that went backwards', () => { + const before = { 'leaderboard:vietnam': [{ value: 'mai', score: 10 }] }; + const after = { 'leaderboard:vietnam': [{ value: 'mai', score: 3 }] }; + expect(findRegressions(before, after).join()).toMatch(/fell from 10 to 3/); + }); + + it('flags a member that disappeared', () => { + const before = { 'leaderboard:city:hn': [{ value: 'mai', score: 10 }] }; + const after = { 'leaderboard:city:hn': [] }; + expect(findRegressions(before, after).join()).toMatch(/member mai removed/); + }); + + it('flags a key that disappeared', () => { + const before = { 'leaderboard:city:hn': [{ value: 'mai', score: 10 }] }; + expect(findRegressions(before, {}).join()).toMatch(/disappeared/); + }); + + it('ignores the backfill destinations, which are meant to change', () => { + const before = { 'leaderboard:city:ld': [{ value: 'orphan', score: 99 }] }; + expect(findRegressions(before, { 'leaderboard:city:ld': [] })).toEqual([]); + }); +}); + +describe('restore', () => { + beforeEach(async () => { + await resetStore(); + }); + + it('puts a snapshot back, making the backup an actual rollback', async () => { + await zAdd(h(), 'leaderboard:city:hn', 50, 'mai'); + await zAdd(h(), 'leaderboard:city:hn', 20, 'linh'); + const snapshot = await exportAll(h()); + + // Simulate the damage a bad run would do. + await zAdd(h(), 'leaderboard:city:hn', 1, 'mai'); + await restore(h(), snapshot); + + expect(await exportAll(h())).toEqual(snapshot); + }); +});