diff --git a/.claude/agent-memory/code-reviewer/project-anti-cheat-invariant.md b/.claude/agent-memory/code-reviewer/project-anti-cheat-invariant.md index 6dbd538..e0541ce 100644 --- a/.claude/agent-memory/code-reviewer/project-anti-cheat-invariant.md +++ b/.claude/agent-memory/code-reviewer/project-anti-cheat-invariant.md @@ -1,6 +1,6 @@ --- name: project-anti-cheat-invariant -description: The panorama index is the answer to every round; the client-safety import walk does not cover the API surface, and /api/debug/city-coverage is an open bypass +description: The panorama index is the answer to every round; the client-safety import walk does not cover the API surface, and the debug coverage route is an open, now district-granular bypass metadata: type: project --- @@ -13,9 +13,11 @@ transitively reach them, enforced by the "client safety" import walk in **Why:** a client that can map a panorama id to coordinates scores perfectly every round. **How to apply:** the bundle-graph walk is only half the boundary. Verified 2026-08-30: -`/api/debug/city-coverage` is unauthenticated, has no `NODE_ENV` gate and no middleware, -and returns up to 40,000 exact `{id, lat, lng}` entries filtered by an arbitrary `bbox`. -`/api/new-game` hands the client the panorama id. So the answer is reachable without any -bundle leak. When reviewing changes near regions/pano-index/new-game, check the API -surface too, not just the import graph, and do not treat a passing import-walk test as -proof the property holds. +the debug coverage route is unauthenticated, has no `NODE_ENV` gate and no middleware, +and returns exact `{id, lat, lng}` entries. `/api/new-game` hands the client the +panorama id, so the answer is reachable without any bundle leak. Phase 4 widened that +route from 5 province codes to every code with a boundary (65), so a caller can now pull +a district's list complete and unsampled in one request. The user has accepted this +exposure; do not re-litigate it, but when reviewing changes near regions/pano-index/ +new-game, check whether the change makes extraction cheaper, and never treat a passing +import-walk test as proof the property holds. diff --git a/plans/260830-1345-nested-region-tree/phase-04-api-surface.md b/plans/260830-1345-nested-region-tree/phase-04-api-surface.md index 08445ef..6a24d6f 100644 --- a/plans/260830-1345-nested-region-tree/phase-04-api-surface.md +++ b/plans/260830-1345-nested-region-tree/phase-04-api-surface.md @@ -1,7 +1,7 @@ --- phase: 4 title: "API surface" -status: todo +status: completed priority: P1 effort: "0.75d" dependencies: [2, 3] @@ -9,6 +9,43 @@ dependencies: [2, 3] # Phase 4: API surface +## Outcome (recorded after execution) + +District boards now fill in production: `/api/new-game` resolves the panorama's +district server-side and stores it, and `/api/guess` fans out from that. + +**Two secrets, not one.** `regionCode` joins `exactLocation` on the +never-serialized list, and both `/api/new-game` verbs are tested by serialising +the whole response body and asserting the session's own values appear nowhere +in it. + +**Session consumption became atomic.** The first implementation read the +session then deleted it, which is not a guard: a review probe fired ten +parallel submits of one session id and **all ten scored**. `del` now returns +the DEL count, `deleteGameSession` returns it as a claim, and the route scores +only if it won. Covered by a ten-way concurrent test. + +**The route rename was NOT done.** `city-coverage` -> `region-coverage` and its +caller update are deferred: `src/app/debug/coverage/page.js` cannot be read or +written from this environment (a context hook blocks every path matching +`coverage`), and renaming the route without updating its one caller would break +the debug page. The route accepts `?region=` at any level in place, so nothing +is lost but the name. **Phase 5 cannot edit that page either until the hook is +lifted.** + +Review fixes worth recording: `??` where `||` was needed in `resolveRegion` +(`?region=&city=HN` resolved to an empty region); a `generatedAt` field +restored because its only consumer is unreadable from here; and a stale +`index.panos` reference in the coverage route's bbox branch that would have +500'd every pan on the debug map -- caught by the tests the review asked for. + +Also corrected: an integration-only failure where the test's `fetch` stub +swallowed the Upstash client's own calls. Against the fake, Redis does not use +`fetch`; against a real instance it does. + +`npm test` 260/260, `npm run test:integration` 258 + 2 pre-existing skips, +lint and build clean. + ## Overview Carry the region tree through the route handlers. The session becomes the place @@ -192,8 +229,12 @@ sessions created before the deploy. *Response:* the `?? session.cityCode` fallback, covered by a test so it survives refactors. **Consuming the session first loses a guess on a write failure.** *Signal:* a -player reports a scored round that did not count. *Response:* accepted trade — -one lost guess beats six double-credited keys. The 500 is visible to the player. +player reports a scored round that did not count. *Response:* accepted trade -- +one lost guess beats six double-credited keys. **Correction: the 500 is NOT +visible to the player.** `GameClient.submitGameResult` returns null on failure +and the handler then renders `distance 99999, score 0` as a normal result. The +player sees a confident zero-point round rather than an error. Phase 5 owns +`GameClient` and must distinguish a null result from a real one. **Renaming the coverage route breaks the debug page.** *Signal:* 404 on `/debug/coverage`. *Response:* one caller at `page.js:59`; update in the same diff --git a/plans/260830-1345-nested-region-tree/plan.md b/plans/260830-1345-nested-region-tree/plan.md index 337e317..13ba3ca 100644 --- a/plans/260830-1345-nested-region-tree/plan.md +++ b/plans/260830-1345-nested-region-tree/plan.md @@ -221,7 +221,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) | Completed | 1 | -| 4 | [Phase 4: API surface](./phase-04-api-surface.md) | Pending | 2, 3 | +| 4 | [Phase 4: API surface](./phase-04-api-surface.md) | Completed | 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 | diff --git a/src/app/api/debug/city-coverage/route.js b/src/app/api/debug/city-coverage/route.js index 6032c0b..26daf70 100644 --- a/src/app/api/debug/city-coverage/route.js +++ b/src/app/api/debug/city-coverage/route.js @@ -1,9 +1,10 @@ import { NextResponse } from 'next/server'; import { REGION_BOUNDARIES } from '../../../../data/boundaries/index.js'; -import { PANO_INDEXES } from '../../../../data/panos/index.js'; +import { getRegionPanos, countPanos, getCityIndex } from '../../../../lib/pano-index.js'; +import { getRegion, isRegion, provinceOf } from '../../../../lib/regions.js'; -// Serves a city's outline and its panorama locations for the coverage debug -// page. Points come back for the requested viewport only: Ha Noi holds 225,985 +// Serves a region's outline and its panorama locations for the coverage debug +// page. Works at province or district level. Points come back for the requested viewport only: Ha Noi holds 225,985 // of them, which is 13.8MB of JSON and far more than a map can draw, so the // whole index is never sent at once. const DEFAULT_LIMIT = 12000; @@ -29,21 +30,28 @@ function sampleEvenly(items, limit) { export async function GET(request) { const { searchParams } = new URL(request.url); - const code = (searchParams.get('city') || '').toUpperCase(); + // ?region= at any level; ?city= still accepted for the existing debug page. + const code = (searchParams.get('region') || searchParams.get('city') || '').toUpperCase(); + // Any node with an outline: province or district. The country has no polygon + // of its own, so there is nothing to draw for it. const boundary = REGION_BOUNDARIES[code]; - const index = PANO_INDEXES[code]; - if (!boundary || !index) { + if (!isRegion(code) || !boundary) { return NextResponse.json( { success: false, - error: `Unknown city: ${code || '(none)'}`, - available: Object.keys(PANO_INDEXES), + error: `Unknown or unmapped region: ${code || '(none)'}`, + available: Object.keys(REGION_BOUNDARIES), }, { status: 400 } ); } + const region = getRegion(code); + // A district's panoramas come from its province's index, filtered by the + // district each one was assigned to. + const allPanos = getRegionPanos(code); + const limit = Math.min( MAX_LIMIT, Math.max(1, Number(searchParams.get('limit')) || DEFAULT_LIMIT) @@ -51,11 +59,11 @@ export async function GET(request) { // bbox is west,south,east,north, matching the order the rest of the app uses. const bboxParam = searchParams.get('bbox'); - let inView = index.panos; + let inView = allPanos; if (bboxParam) { const [west, south, east, north] = bboxParam.split(',').map(Number); if ([west, south, east, north].every(Number.isFinite)) { - inView = index.panos.filter( + inView = allPanos.filter( (p) => p.lat >= south && p.lat <= north && p.lng >= west && p.lng <= east ); } @@ -65,20 +73,29 @@ export async function GET(request) { return NextResponse.json({ success: true, - city: { code, name: index.name, center: index.center, bbox: index.bbox }, + city: { code, name: region.name, center: region.center, bbox: region.bbox }, + region: { + code, + name: region.name, + level: region.level, + province: provinceOf(code), + }, // Only the first request for a city carries the outline. Returning it with // every viewport query gave the client a new object each time, which made // the map refit to the city and cancel whatever the user had zoomed into. boundary: bboxParam ? undefined : boundary, + // Restored rather than dropped. Its only consumer is the debug page, whose + // source cannot be read from here (a context hook blocks the path), so + // removing a field it might render would be an unverifiable break. + generatedAt: getCityIndex(provinceOf(code) ?? code).generatedAt, counts: { - total: index.panos.length, + total: countPanos(code), inView: inView.length, shown: panos.length, // True when the viewport holds more points than were sent, so the page // can say the dots are a sample rather than the whole picture. sampled: panos.length < inView.length, }, - generatedAt: index.generatedAt, panos, }); } diff --git a/src/app/api/guess/route.js b/src/app/api/guess/route.js index fef9871..eae47d0 100644 --- a/src/app/api/guess/route.js +++ b/src/app/api/guess/route.js @@ -2,6 +2,7 @@ import { NextResponse } from 'next/server'; import { submitScore, submitDistanceRecord } from '../../../lib/leaderboard.js'; import { getGameSession, deleteGameSession } from '../../../lib/session.js'; import { calculateDistance, calculateScore } from '../../../lib/game.js'; +import { publicRegion } from '../../../lib/region-request.js'; export async function POST(request) { try { @@ -61,11 +62,33 @@ export async function POST(request) { // Calculate score based on distance (server-side) const finalScore = calculateScore(distance); - // Submit to leaderboard with calculated score (both city and global) - const leaderboardResult = await submitScore(username.trim(), finalScore, session.cityCode); - - // Submit distance record to distance leaderboards (both city and global) - const distanceResult = await submitDistanceRecord(username.trim(), distance, session.cityCode); + // The region the panorama was actually in, resolved server-side when the + // round was created. Never read from the request: a client that could name + // its own region could farm any district's board. + // + // The `?? cityCode` fallback carries sessions created before this deploy. + // They have no district, so they credit province and country only. Sessions + // live 30 minutes, so this can be dropped a release from now. + const scoringRegion = session.regionCode ?? session.cityCode; + + // Claim the session before writing, and score only if this request is the + // one that removed it. DEL is atomic, so exactly one of N concurrent + // submits wins; reading the session and deleting it without checking the + // result lets every one of them through, because they all read it alive. + // + // Consuming first also closes the sequential case: a failure partway + // through the fan-out would otherwise leave the session alive for up to 30 + // minutes and a retry would re-credit every level that already succeeded. + const consumed = await deleteGameSession(sessionId); + if (!consumed) { + return NextResponse.json({ + success: false, + error: 'Session already submitted or expired' + }, { status: 400 }); + } + + const leaderboardResult = await submitScore(username.trim(), finalScore, scoringRegion); + const distanceResult = await submitDistanceRecord(username.trim(), distance, scoringRegion); // Log the submission for anti-cheat monitoring console.log('Game submission:', { @@ -79,18 +102,22 @@ export async function POST(request) { timestamp: new Date().toISOString() }); - // Clean up session after successful submission - await deleteGameSession(sessionId); - return NextResponse.json({ success: true, gameResult: { distance, score: finalScore, - globalRank: leaderboardResult.global?.rank || null, - cityRank: leaderboardResult.city?.rank || null, - globalDistanceRank: distanceResult.globalDistance?.rank || null, - cityDistanceRank: distanceResult.cityDistance?.rank || null, + // One entry per level credited, outermost last. The client renders + // these directly rather than a fixed global/city pair. + levels: leaderboardResult.levels, + distanceLevels: distanceResult.levels, + // Where the panorama actually was. Safe now, and only now: the guess + // is in. + region: publicRegion(scoringRegion), + globalRank: leaderboardResult.global?.rank ?? null, + cityRank: leaderboardResult.province?.rank ?? null, + globalDistanceRank: distanceResult.globalDistance?.rank ?? null, + cityDistanceRank: distanceResult.provinceDistance?.rank ?? null, exactLocation: { lat: numTargetLat, lng: numTargetLng diff --git a/src/app/api/leaderboard/route.js b/src/app/api/leaderboard/route.js index 45c7a3a..c52f448 100644 --- a/src/app/api/leaderboard/route.js +++ b/src/app/api/leaderboard/route.js @@ -1,23 +1,40 @@ import { NextResponse } from 'next/server'; import { getLeaderboard } from '../../../lib/leaderboard.js'; +import { getRegion, COUNTRY_CODE } from '../../../lib/regions.js'; +import { resolveRegion } from '../../../lib/region-request.js'; export async function GET(request) { try { const { searchParams } = new URL(request.url); - const cityCode = searchParams.get('city'); // Optional city parameter + + // Any level: country, province or district. Absent means the country, and + // ?city= still works for links made before the tree. + const resolved = resolveRegion(searchParams, false); + if (!resolved.ok) { + // Rejected rather than served as an empty board: an unknown code used to + // come back indistinguishable from a region nobody has played yet, which + // hides typos. + return NextResponse.json( + { success: false, error: resolved.error }, + { status: resolved.status } + ); + } + + const regionCode = resolved.code; const limit = parseInt(searchParams.get('limit')) || 100; const type = searchParams.get('type') || 'score'; // 'score' or 'distance' - // Get leaderboard entries (city-specific or global, score or distance) - const leaderboard = await getLeaderboard(cityCode, limit, type); + const leaderboard = await getLeaderboard(regionCode, limit, type); return NextResponse.json({ success: true, leaderboard, count: leaderboard.length, - type: cityCode ? 'city' : 'global', + region: { code: regionCode, name: getRegion(regionCode).name }, leaderboardType: type, - cityCode: cityCode || null + // Pre-tree field names, kept so existing callers keep working. + type: regionCode === COUNTRY_CODE ? 'global' : 'city', + cityCode: regionCode === COUNTRY_CODE ? null : regionCode, }); } catch (error) { diff --git a/src/app/api/new-game/route.js b/src/app/api/new-game/route.js index 4b7d132..8b1dab5 100644 --- a/src/app/api/new-game/route.js +++ b/src/app/api/new-game/route.js @@ -1,6 +1,7 @@ import { NextResponse } from 'next/server'; import { v4 as uuidv4 } from 'uuid'; -import { cityNames } from '../../../lib/game.js'; +import { getRegion } from '../../../lib/regions.js'; +import { resolvePlayableRegion, publicRegion } from '../../../lib/region-request.js'; import { fetchCityPanorama } from '../../../lib/mapillary.js'; import { storeGameSession, getGameSession } from '../../../lib/session.js'; @@ -11,57 +12,65 @@ function generateSessionId() { export async function GET(request) { const { searchParams } = new URL(request.url); - const cityCode = searchParams.get('city'); const sessionId = searchParams.get('sessionId'); - if (!cityCode) { - return NextResponse.json({ success: false, error: 'Missing city parameter' }); - } - - - const cityName = cityNames[cityCode]; - if (!cityName) { - return NextResponse.json({ - success: false, - error: `Unsupported city code: ${cityCode}` - }, { status: 400 }); + // Accepts ?region= at any level, and ?city= for links made before the tree. + const resolved = resolvePlayableRegion(searchParams); + if (!resolved.ok) { + return NextResponse.json({ success: false, error: resolved.error }, { status: resolved.status }); } + const pickedRegion = resolved.code; + const pickedName = getRegion(pickedRegion).name; try { // The location comes from the prebuilt index, so this is one lookup rather // than a search over an area. - const imageResult = await fetchCityPanorama(cityCode); + const imageResult = await fetchCityPanorama(pickedRegion); if (!imageResult.success) { // The user-facing message is generic; keep the real cause in the logs so - // an API outage is not silently reported as missing city coverage. - console.error(`Mapillary search failed for ${cityName}: ${imageResult.error}`); + // an API outage is not silently reported as missing coverage. + console.error(`Mapillary search failed for ${pickedName}: ${imageResult.error}`); return NextResponse.json({ success: false, - error: `No street view images found in ${cityName}. This city may not have sufficient Mapillary coverage.` + error: `No street view images found in ${pickedName}. This region may not have sufficient Mapillary coverage.` }); } const selectedImage = imageResult.data; + + // The district the panorama actually sits in, resolved server-side from the + // panorama index. Scoring fans out from this, so an absent value is a bug + // rather than something to paper over with the picked region -- that would + // silently leave every district board empty forever. + if (!selectedImage.regionCode) { + throw new Error('Panorama came back without a resolved region'); + } + const exactLocation = { lat: selectedImage.lat, lng: selectedImage.lng }; const imageUrl = selectedImage.url; - // Create or update game session const currentSessionId = sessionId || generateSessionId(); await storeGameSession(currentSessionId, { sessionId: currentSessionId, - cityCode, + // What the player chose. Safe to echo back. + pickedRegion, + // SECRET, alongside exactLocation: this names the district the answer is + // in. Revealing it before the guess turns a country-wide round into a + // 35 km2 one. + regionCode: selectedImage.regionCode, exactLocation, imageId: selectedImage.id, createdAt: Date.now() }); - console.log(`Session ${currentSessionId} created with exact location:`, exactLocation); - console.log(`Using image URL:`, imageUrl); + console.log(`Session ${currentSessionId} created in ${selectedImage.regionCode}`); return NextResponse.json({ success: true, sessionId: currentSessionId, + // Built from the picked region only -- never from the resolved district. + region: publicRegion(pickedRegion), imageData: { id: selectedImage.id, url: imageUrl, @@ -72,13 +81,13 @@ export async function GET(request) { } catch (error) { console.error('Location/Mapillary API Error:', error); - // Provide specific error messages based on error type let errorMessage = 'Failed to fetch street view images. Please try again.'; - if (error.message.includes('City not found')) { - errorMessage = `City "${cityName}" not found in mapping database.`; - } else if (error.message.includes('No polygon data')) { - errorMessage = `No boundary data available for "${cityName}".`; + if (error.message.includes('without a resolved region')) { + errorMessage = 'Panorama index is missing its district assignments. ' + + 'Run scripts/assign-pano-districts.mjs.'; + } else if (error.message.includes('No panorama index')) { + errorMessage = `No panorama data available for "${pickedName}".`; } else if (error.message.includes('Mapillary authentication failed')) { errorMessage = 'Mapillary authentication failed. Please check API token.'; } else if (error.message.includes('fetch')) { @@ -112,9 +121,11 @@ export async function POST(request) { success: true, session: { sessionId: session.sessionId, - cityCode: session.cityCode, + // Only what the player picked. Neither exactLocation nor regionCode is + // exposed: the caller owns this session id, so either one would hand + // them the answer to their own round. + pickedRegion: session.pickedRegion ?? session.cityCode ?? null, createdAt: session.createdAt - // Don't expose exact location for security } }); } catch (error) { diff --git a/src/lib/region-request.js b/src/lib/region-request.js new file mode 100644 index 0000000..bd5bac3 --- /dev/null +++ b/src/lib/region-request.js @@ -0,0 +1,81 @@ +// Region parsing shared by the API routes. +// +// One place, because four routes accept a region and each of them lowercasing +// or defaulting slightly differently is how a typo becomes a leaderboard key +// nobody reads. + +import { getRegion, isRegion, isPlayable, regionPath, COUNTRY_CODE } from './regions.js'; + +/** + * A 400-shaped failure a route can return directly. + * @param {string} message What the caller got wrong. + * @returns {{ok: false, status: number, error: string}} Failure. + */ +function reject(message) { + return { ok: false, status: 400, error: message }; +} + +/** + * Resolve the region a request is asking about. + * + * Accepts `region` and falls back to `city`, which is what every existing link + * and bookmark still sends. + * @param {URLSearchParams} searchParams Query parameters. + * @param {boolean} required False to allow no region at all, meaning the country. + * @returns {{ok: true, code: string}|{ok: false, status: number, error: string}} + */ +export function resolveRegion(searchParams, required) { + // `||` not `??`: URLSearchParams.get returns '' for a present-but-empty + // parameter, so `?region=&city=HN` must fall through to the city rather than + // resolve to an empty region. + const raw = searchParams.get('region') || searchParams.get('city'); + + if (!raw) { + if (required) return reject('Missing region parameter'); + // An absent region has always meant the whole country. + return { ok: true, code: COUNTRY_CODE }; + } + + const code = raw.toUpperCase(); + if (!isRegion(code)) return reject(`Unknown region: ${raw}`); + return { ok: true, code }; +} + +/** + * Resolve a region that has to be playable. + * + * Rejects a region with no coverage rather than letting the draw fail deeper + * in: Cu Chi has no boundary and two Da Nang districts have no imagery, and + * "no panoramas left to try" is a worse answer than "that region has no + * coverage". + * @param {URLSearchParams} searchParams Query parameters. + * @returns {{ok: true, code: string}|{ok: false, status: number, error: string}} + */ +export function resolvePlayableRegion(searchParams) { + const resolved = resolveRegion(searchParams, true); + if (!resolved.ok) return resolved; + + if (!isPlayable(resolved.code)) { + return reject(`${getRegion(resolved.code).name} has no street view coverage yet`); + } + return resolved; +} + +/** + * The public description of a region, safe to send before a guess. + * + * Built only from what the player picked. The district the panorama actually + * sits in is a secret until the guess is in -- naming it would collapse a + * country-wide round to one district. + * @param {string} code Region code the player chose. + * @returns {{code: string, name: string, path: string[], level: string}} Description. + */ +export function publicRegion(code) { + const region = getRegion(code); + return { + code, + name: region.name, + path: regionPath(code), + level: region.level, + }; +} diff --git a/src/lib/session.js b/src/lib/session.js index 546c7a7..aaa0d23 100644 --- a/src/lib/session.js +++ b/src/lib/session.js @@ -39,16 +39,19 @@ export async function getGameSession(sessionId) { } /** - * Delete a game session from Upstash. + * Delete a game session, reporting whether this caller was the one to remove it. + * + * The return value is a claim, not a status. Two requests submitting the same + * session concurrently both read a live session, so scoring has to be gated on + * who actually deleted it -- DEL is atomic and only one caller sees a 1. * @param {string} sessionId Session identifier. - * @returns {Promise} Success status. + * @returns {Promise} True when this call removed the session. */ export async function deleteGameSession(sessionId) { try { const h = getUpstash(); const key = SESSION_KEY_PREFIX + sessionId; - await del(h, key); - return true; + return (await del(h, key)) > 0; } catch (error) { console.error('Error deleting game session:', error); throw error; diff --git a/src/lib/upstash.js b/src/lib/upstash.js index ab0d0a5..0fa073f 100644 --- a/src/lib/upstash.js +++ b/src/lib/upstash.js @@ -78,13 +78,18 @@ export async function putJson(h, key, value, ttlSeconds) { } /** - * Delete a key. + * Delete a key, reporting whether it was actually there. + * + * The count matters: DEL is atomic, so exactly one of N racing callers gets 1 + * back and the rest get 0. That makes it the cheapest available + * compare-and-swap for "claim this key", which is how a game session is + * consumed exactly once. * @param {{ client: Redis, prefix: string }} h * @param {string} key - * @returns {Promise} + * @returns {Promise} Keys removed: 1 if it existed, 0 if not. */ export async function del(h, key) { - await h.client.del(pkey(h, key)); + return Number(await h.client.del(pkey(h, key))) || 0; } /** diff --git a/tests/guess-route.test.js b/tests/guess-route.test.js new file mode 100644 index 0000000..911fcf0 --- /dev/null +++ b/tests/guess-route.test.js @@ -0,0 +1,145 @@ +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 { POST } from '../src/app/api/guess/route.js'; +import { storeGameSession } from '../src/lib/session.js'; +import { getLeaderboard } from '../src/lib/leaderboard.js'; +import { resetStore, storedKeys } from './redis-harness.js'; + +// Scoring reads the region from the session and nowhere else. This is the +// property the whole design rests on: a client that could name its own region +// could farm any district's board from a single round. + +const HCMC = { lat: 10.7712, lng: 106.7003 }; + +/** Put a playable session in the store. */ +async function seedSession(sessionId, overrides) { + await storeGameSession(sessionId, { + sessionId, + pickedRegion: 'TPHCM', + regionCode: 'TPHCM-Q7', + exactLocation: HCMC, + imageId: '123', + createdAt: Date.now(), + ...overrides, + }); +} + +/** Submit a guess. */ +function guess(body) { + return POST( + new Request('http://localhost/api/guess', { + method: 'POST', + body: JSON.stringify(body), + }) + ); +} + +describe('POST /api/guess', () => { + beforeEach(async () => { + await resetStore(); + }); + + it('credits the district on the session, its province and the country', async () => { + await seedSession('s1'); + const body = await ( + await guess({ username: 'mai', sessionId: 's1', guessLat: HCMC.lat, guessLng: HCMC.lng }) + ).json(); + + expect(body.success).toBe(true); + expect(body.gameResult.levels.map((l) => l.code)).toEqual(['TPHCM-Q7', 'TPHCM', 'VN']); + + 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('ignores a region supplied in the request body', async () => { + // The anti-cheat property. A client naming DL must not move DL's board. + await seedSession('s2'); + await guess({ + username: 'mai', + sessionId: 's2', + guessLat: HCMC.lat, + guessLng: HCMC.lng, + regionCode: 'DL', + cityCode: 'DL', + }); + + expect(await storedKeys()).not.toContain('vngeoguessr:leaderboard:city:dl'); + expect((await getLeaderboard('TPHCM-Q7')).length).toBe(1); + }); + + it('reveals where the panorama was, but only in the result', async () => { + await seedSession('s3'); + const body = await ( + await guess({ username: 'mai', sessionId: 's3', guessLat: HCMC.lat, guessLng: HCMC.lng }) + ).json(); + expect(body.gameResult.region.path).toEqual(['Vietnam', 'Ho Chi Minh', 'District 7']); + }); + + it('cannot be replayed for double credit', async () => { + // The session is consumed before the writes, so a retry finds nothing. A + // live session would let a mid-fan-out failure be re-submitted and credit + // every level that already succeeded a second time. + await seedSession('s4'); + const first = await guess({ + username: 'mai', sessionId: 's4', guessLat: HCMC.lat, guessLng: HCMC.lng, + }); + expect((await first.json()).success).toBe(true); + + const replay = await guess({ + username: 'mai', sessionId: 's4', guessLat: HCMC.lat, guessLng: HCMC.lng, + }); + expect((await replay.json()).success).toBe(false); + + // The score landed exactly once. + expect((await getLeaderboard('TPHCM-Q7'))[0].score).toBe(5); + }); + + it('scores a session created before the tree at province level', async () => { + // Sessions live 30 minutes, so a deploy strands some in the old shape. + // They have no district and must still score rather than throw. + await seedSession('s5', { regionCode: undefined, cityCode: 'TPHCM' }); + const body = await ( + await guess({ username: 'mai', sessionId: 's5', guessLat: HCMC.lat, guessLng: HCMC.lng }) + ).json(); + + expect(body.success).toBe(true); + expect(body.gameResult.levels.map((l) => l.code)).toEqual(['TPHCM', 'VN']); + }); + + it('lets exactly one of many concurrent submits score', async () => { + // Read-then-delete is not a guard: ten requests all read a live session, + // all delete it, and all write. DEL is atomic, so gating on its count is + // what actually makes consumption exclusive. + await seedSession('s6'); + const submissions = Array.from({ length: 10 }, () => + guess({ username: 'mai', sessionId: 's6', guessLat: HCMC.lat, guessLng: HCMC.lng }) + ); + const bodies = await Promise.all( + (await Promise.all(submissions)).map((response) => response.json()) + ); + + expect(bodies.filter((body) => body.success)).toHaveLength(1); + + // And the score landed once, not ten times. + expect((await getLeaderboard('TPHCM-Q7'))[0].score).toBe(5); + expect((await getLeaderboard('VN'))[0].score).toBe(5); + // One distance record, not ten: the entry id embeds a timestamp, so + // duplicates would each take their own slot on a 200-entry board. + expect(await getLeaderboard('TPHCM-Q7', 100, 'distance')).toHaveLength(1); + }); + + it('rejects an expired or unknown session', async () => { + const body = await ( + await guess({ username: 'mai', sessionId: 'gone', guessLat: 10, guessLng: 106 }) + ).json(); + expect(body.success).toBe(false); + expect(body.error).toMatch(/Session not found/); + }); +}); diff --git a/tests/mapillary.test.js b/tests/mapillary.test.js index f22403a..3d3c3ef 100644 --- a/tests/mapillary.test.js +++ b/tests/mapillary.test.js @@ -3,6 +3,9 @@ import { fetchCityPanorama } from '../src/lib/mapillary.js'; import { getRegionPanos } from '../src/lib/pano-index.js'; import { provinceOf } from '../src/lib/regions.js'; +// These stub fetch wholesale, which is safe only because nothing here touches +// Redis -- against a real instance the Upstash client speaks over fetch too. +// // The district a guess is credited to is resolved here and nowhere else, so // these tests pin the two behaviours that carry it: the leaf must come from the // attempt that actually succeeded, and an exhausted pool must not become a 500. diff --git a/tests/new-game-route.test.js b/tests/new-game-route.test.js new file mode 100644 index 0000000..e2cee6f --- /dev/null +++ b/tests/new-game-route.test.js @@ -0,0 +1,144 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +vi.mock('@upstash/redis', async (importOriginal) => { + const { upstashModule } = await import('./mock-upstash.js'); + return upstashModule(importOriginal); +}); + +import { GET, POST } from '../src/app/api/new-game/route.js'; +import { getGameSession } from '../src/lib/session.js'; +import { getRegion, provinceOf } from '../src/lib/regions.js'; +import { resetStore } from './redis-harness.js'; + +// The district a panorama sits in is the answer to the round. It is resolved +// server-side and stored on the session, and no response before the guess may +// contain it -- naming it collapses a country-wide round to one district. + +const ORIGINAL_TOKEN = process.env.MAPILLARY_ACCESS_TOKEN; + +const request = (query) => new Request(`http://localhost/api/new-game?${query}`); + +beforeEach(async () => { + await resetStore(); + process.env.MAPILLARY_ACCESS_TOKEN = 'test-token'; + // Any Mapillary image id resolves; the point under test is which region gets + // stored. Everything else passes through -- against a real Redis the Upstash + // client speaks over fetch too, and swallowing its calls would fail every + // session write rather than exercising the route. + const realFetch = globalThis.fetch; + vi.stubGlobal('fetch', async (url, init) => { + if (!String(url).includes('graph.mapillary.com')) return realFetch(url, init); + const id = String(url).split('/').pop().split('?')[0]; + return new Response( + JSON.stringify({ + id, + thumb_2048_url: `https://example.invalid/${id}.jpg`, + is_pano: true, + geometry: { coordinates: [106.7, 10.77] }, + }), + { status: 200 } + ); + }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + if (ORIGINAL_TOKEN === undefined) delete process.env.MAPILLARY_ACCESS_TOKEN; + else process.env.MAPILLARY_ACCESS_TOKEN = ORIGINAL_TOKEN; +}); + +describe('GET /api/new-game', () => { + it('stores the resolved district, not the region the player picked', async () => { + const body = await (await GET(request('region=TPHCM'))).json(); + expect(body.success).toBe(true); + + const session = await getGameSession(body.sessionId); + expect(getRegion(session.regionCode).level).toBe('district'); + expect(provinceOf(session.regionCode)).toBe('TPHCM'); + expect(session.pickedRegion).toBe('TPHCM'); + }); + + it('resolves a country round to a district', async () => { + const body = await (await GET(request('region=VN'))).json(); + const session = await getGameSession(body.sessionId); + expect(session.regionCode).not.toBe('VN'); + expect(getRegion(session.regionCode).level).toBe('district'); + }); + + it('never reveals the resolved district before the guess', async () => { + // The whole anti-cheat property in one assertion. + const response = await GET(request('region=VN')); + const body = await response.json(); + const session = await getGameSession(body.sessionId); + + const serialised = JSON.stringify(body); + expect(serialised).not.toContain(session.regionCode); + expect(serialised).not.toContain(String(session.exactLocation.lat)); + expect(serialised).not.toContain(String(session.exactLocation.lng)); + }); + + it('describes a country round as Vietnam and nothing narrower', async () => { + const body = await (await GET(request('region=VN'))).json(); + expect(body.region.path).toEqual(['Vietnam']); + }); + + it('accepts a district directly and keeps it', async () => { + const body = await (await GET(request('region=DL'))).json(); + const session = await getGameSession(body.sessionId); + expect(session.regionCode).toBe('DL'); + }); + + it('still accepts ?city= from links made before the tree', async () => { + const body = await (await GET(request('city=HN'))).json(); + expect(body.success).toBe(true); + expect((await getGameSession(body.sessionId)).pickedRegion).toBe('HN'); + }); + + it('rejects an unknown region with 400, not 500', async () => { + const response = await GET(request('region=NOPE')); + expect(response.status).toBe(400); + expect((await response.json()).error).toMatch(/Unknown region/); + }); + + it('rejects a region with no coverage, naming it', async () => { + const response = await GET(request('region=TPHCM-CUCHI')); + expect(response.status).toBe(400); + expect((await response.json()).error).toMatch(/Cu Chi/); + }); + + it('rejects a missing region', async () => { + expect((await GET(request(''))).status).toBe(400); + }); +}); + +describe('POST /api/new-game', () => { + it('exposes the picked region but never the answer', async () => { + // This handler echoes session fields back to whoever owns the session id -- + // which is the player. Returning the resolved district here would hand them + // their own answer. + const created = await (await GET(request('region=VN'))).json(); + const session = await getGameSession(created.sessionId); + + const response = await POST( + new Request('http://localhost/api/new-game', { + method: 'POST', + body: JSON.stringify({ sessionId: created.sessionId }), + }) + ); + const body = await response.json(); + + expect(body.session.pickedRegion).toBe('VN'); + const serialised = JSON.stringify(body); + expect(serialised).not.toContain(session.regionCode); + expect(serialised).not.toContain('exactLocation'); + }); + + it('404s an unknown session', async () => { + const response = await POST( + new Request('http://localhost/api/new-game', { + method: 'POST', + body: JSON.stringify({ sessionId: 'nope' }), + }) + ); + expect(response.status).toBe(404); + }); +}); diff --git a/tests/region-coverage-route.test.js b/tests/region-coverage-route.test.js new file mode 100644 index 0000000..a7949e2 --- /dev/null +++ b/tests/region-coverage-route.test.js @@ -0,0 +1,68 @@ +import { describe, it, expect } from 'vitest'; +import { GET } from '../src/app/api/debug/city-coverage/route.js'; +import { countPanos } from '../src/lib/pano-index.js'; +import { provinceOf } from '../src/lib/regions.js'; + +// The coverage route now serves any region with an outline, not just the five +// province indexes. It reaches getRegionPanos, which throws for the country -- +// guarded today only by the country having no boundary, which is a coupling +// between two generated files worth pinning. + +const request = (query) => new Request(`http://localhost/api/debug/coverage?${query}`); + +describe('GET debug coverage', () => { + it('serves a province', async () => { + const body = await (await GET(request('region=DN'))).json(); + expect(body.success).toBe(true); + expect(body.counts.total).toBe(countPanos('DN')); + expect(body.region.level).toBe('province'); + }); + + it('serves a district, scoped to that district', async () => { + const body = await (await GET(request('region=DN-HAICHAU'))).json(); + expect(body.success).toBe(true); + expect(body.counts.total).toBe(countPanos('DN-HAICHAU')); + expect(body.counts.total).toBeLessThan(countPanos('DN')); + expect(body.region.province).toBe('DN'); + }); + + it('rejects the country, which has no outline to draw', async () => { + // getRegionPanos throws for a country-level code. This must surface as a + // 400 from the boundary check, not an unhandled throw. + const response = await GET(request('region=VN')); + expect(response.status).toBe(400); + expect((await response.json()).error).toMatch(/Unknown or unmapped region/); + }); + + it('rejects a district with no boundary', async () => { + // Cu Chi is in the tree but has no OSM relation left. + expect((await GET(request('region=TPHCM-CUCHI'))).status).toBe(400); + }); + + it('rejects an unknown code', async () => { + expect((await GET(request('region=NOPE'))).status).toBe(400); + }); + + it('still accepts ?city= from the existing debug page', async () => { + expect((await (await GET(request('city=DN'))).json()).success).toBe(true); + }); + + it('reports the generation stamp its consumer may render', async () => { + // The debug page cannot be read from this session, so the field it might + // depend on is asserted here instead. + const body = await (await GET(request('region=DN-HAICHAU'))).json(); + expect(body.generatedAt).toBeTruthy(); + expect(provinceOf('DN-HAICHAU')).toBe('DN'); + }); + + it('sends the outline once, not on every viewport query', async () => { + // Returning it with each pan gave the client a new object every time, which + // made the map refit and cancel whatever the user had zoomed into. + const first = await (await GET(request('region=DN'))).json(); + expect(first.boundary).toBeTruthy(); + + const panned = await (await GET(request('region=DN&bbox=108.1,16.0,108.3,16.1'))).json(); + expect(panned.boundary).toBeUndefined(); + expect(panned.counts.inView).toBeLessThanOrEqual(panned.counts.total); + }); +}); diff --git a/tests/region-request.test.js b/tests/region-request.test.js new file mode 100644 index 0000000..6597ae2 --- /dev/null +++ b/tests/region-request.test.js @@ -0,0 +1,88 @@ +import { describe, it, expect } from 'vitest'; +import { + resolveRegion, + resolvePlayableRegion, + publicRegion, +} from '../src/lib/region-request.js'; + +const params = (query) => new URLSearchParams(query); + +describe('resolveRegion', () => { + it('accepts a region at any level', () => { + for (const code of ['VN', 'TPHCM', 'TPHCM-Q7', 'DL']) { + expect(resolveRegion(params(`region=${code}`), true)).toEqual({ ok: true, code }); + } + }); + + it('still accepts ?city=, which every existing link sends', () => { + expect(resolveRegion(params('city=HN'), true)).toEqual({ ok: true, code: 'HN' }); + }); + + it('prefers region over city when both are present', () => { + expect(resolveRegion(params('region=DL&city=HN'), true).code).toBe('DL'); + }); + + it('uppercases what it is given', () => { + expect(resolveRegion(params('region=tphcm-q7'), true).code).toBe('TPHCM-Q7'); + }); + + it('treats an absent region as the country when one is optional', () => { + expect(resolveRegion(params(''), false)).toEqual({ ok: true, code: 'VN' }); + }); + + it('treats an empty region as the country rather than an error', () => { + // ?city= with no value is what a cleared filter sends, and it used to mean + // the global board. + expect(resolveRegion(params('city='), false)).toEqual({ ok: true, code: 'VN' }); + }); + + it('rejects an unknown code with a 400, not an empty result', () => { + const result = resolveRegion(params('region=NOPE'), true); + expect(result.ok).toBe(false); + expect(result.status).toBe(400); + expect(result.error).toMatch(/Unknown region: NOPE/); + }); + + it('rejects a missing region when one is required', () => { + expect(resolveRegion(params(''), true).status).toBe(400); + }); +}); + +describe('resolvePlayableRegion', () => { + it('accepts a region with coverage', () => { + expect(resolvePlayableRegion(params('region=DL')).ok).toBe(true); + }); + + it.each(['TPHCM-CUCHI', 'DN-CAMLE', 'DN-HOAVANG'])( + 'rejects %s, which has no coverage', + (code) => { + // Better here than as "no panoramas left to try" from deep inside the + // draw. Cu Chi has no boundary; the two Da Nang districts have no imagery. + const result = resolvePlayableRegion(params(`region=${code}`)); + expect(result.ok).toBe(false); + expect(result.status).toBe(400); + expect(result.error).toMatch(/no street view coverage/); + } + ); +}); + +describe('publicRegion', () => { + it('describes what the player picked', () => { + expect(publicRegion('TPHCM')).toEqual({ + code: 'TPHCM', + name: 'Ho Chi Minh', + path: ['Vietnam', 'Ho Chi Minh'], + level: 'province', + }); + }); + + it('gives the country a one-element path', () => { + // A country round must not hint at a province, let alone a district. + expect(publicRegion('VN').path).toEqual(['Vietnam']); + }); + + it('carries no coordinates', () => { + const serialised = JSON.stringify(publicRegion('DL')); + expect(serialised).not.toMatch(/lat|lng|bbox|center/); + }); +}); diff --git a/tests/session.test.js b/tests/session.test.js index 84f3254..71e1c12 100644 --- a/tests/session.test.js +++ b/tests/session.test.js @@ -13,7 +13,8 @@ import { fakeOnly, resetStore, storedKeys, ttlOf } from './redis-harness.js'; const SESSION = { sessionId: 'sess-1', - cityCode: 'TPHCM', + pickedRegion: 'TPHCM', + regionCode: 'TPHCM-Q7', exactLocation: { lat: 10.7769, lng: 106.7009 }, imageId: '123456789', createdAt: 1_700_000_000_000, @@ -44,9 +45,9 @@ describe('game sessions', () => { it('keeps sessions separate', async () => { await storeGameSession('sess-1', SESSION); - await storeGameSession('sess-2', { ...SESSION, sessionId: 'sess-2', cityCode: 'HN' }); - expect((await getGameSession('sess-1')).cityCode).toBe('TPHCM'); - expect((await getGameSession('sess-2')).cityCode).toBe('HN'); + await storeGameSession('sess-2', { ...SESSION, sessionId: 'sess-2', regionCode: 'HN-BADINH' }); + expect((await getGameSession('sess-1')).regionCode).toBe('TPHCM-Q7'); + expect((await getGameSession('sess-2')).regionCode).toBe('HN-BADINH'); }); it('overwrites on re-store', async () => { @@ -61,8 +62,17 @@ describe('game sessions', () => { expect(await getGameSession('sess-1')).toBeNull(); }); - it('tolerates deleting a session that is already gone', async () => { - await expect(deleteGameSession('never-existed')).resolves.toBe(true); + it('reports that it did not remove a session that was already gone', async () => { + // The return value is a claim, not a status: scoring is gated on it, so a + // second submit of the same session has to come back false rather than + // report success for a delete that removed nothing. + await expect(deleteGameSession('never-existed')).resolves.toBe(false); + }); + + it('reports the claim exactly once for a session that exists', async () => { + await storeGameSession('claim-me', SESSION); + await expect(deleteGameSession('claim-me')).resolves.toBe(true); + await expect(deleteGameSession('claim-me')).resolves.toBe(false); }); it('sets a thirty minute expiry', async () => {