[api] fix a few 2023 compat issues

2026-10-25 23:18:51 -07:00 · 2026-09-15 11:22:33 -04:00
parent 3299f05940
commit 71bb29b8c7
14 changed files with 360 additions and 130 deletions
@@ -12,13 +12,13 @@ import {
 	deleteEmptyRoomInstances,
 	deleteExpiredPresence,
 	deletePresence,
+	deleteRoomInvite,
 	GAME_VERSION,
 	getAccount,
 	getClubSummary,
 	getExpiredPresenceInstanceIds,
 	getFriendIds,
 	getJoinableInstance,
-	deleteRoomInvite,
 	getLatestRoomInviteBetween,
 	getMostActiveClubhouses,
 	getOrCreateDormRoom,
@@ -28,8 +28,8 @@ import {
 	getRoomByName,
 	getRoomInstance,
 	getRoomInstancesByRoom,
-	getRoomInvite,
 	getRoomInstanceSummariesByRoom,
+	getRoomInvite,
 	getStoredRoomInstance,
 	InviteMode,
 	isClubMember,
@@ -64,6 +64,7 @@ import {
 	AUTHED,
 	AvoidJuniorsRequest,
 	AvoidJuniorsResponse,
+	BatchPlayerIdsRequest,
 	ConnectionInfoResponse,
 	CorrelationIdRequest,
 	EMPTY_OK,
@@ -91,6 +92,7 @@ import {
 } from './openapi'

 import type { Context } from 'hono'
+import type { DescribeRouteOptions } from 'hono-openapi'
 import type { Room, StoredPresence } from '@repo/domain'
 import type { App, Env } from './context'

@@ -991,6 +993,62 @@ async function readRequestFields(c: Context<App>): Promise<Record<string, unknow
 	return await c.req.parseBody({ all: true }).catch(() => empty)
 }

+/**
+ * OpenAPI spec for the two `/player` registrations, which differ only in where the ids ride.
+ */
+function batchPlayerRouteSpec(source: 'query' | 'body'): DescribeRouteOptions {
+	return {
+		tags: ['Presence'],
+		summary: 'Batch player presence lookup',
+		description: [
+			'Returns each requested player’s presence. `id` is repeated once per player',
+			'(`?id=2&id=155&id=153`, or the same fields in a form-urlencoded body) — one value',
+			'each, not comma-separated. With no ids, serves a single default (online) player.',
+		].join(' '),
+		...(source === 'query'
+			? {
+					parameters: [
+						{
+							name: 'id',
+							in: 'query',
+							required: false,
+							description: 'Repeated once per player id (`?id=2&id=155`); not comma-separated',
+							schema: { type: 'array', items: { type: 'string' } },
+						},
+					],
+				}
+			: { requestBody: form(BatchPlayerIdsRequest, 'Repeated `id` fields') }),
+		responses: { 200: json(PlayerDto.array(), 'One entry per requested player') },
+	}
+}
+
+/**
+ * Returns each requested player's presence. Ids come from repeated `id` query params AND,
+ * on the POST form, repeated `id` body fields; with none, serves the static
+ * getplayer.json default. Deduped (first occurrence wins) so a repeated id can't double
+ * an entry or waste a bound parameter.
+ */
+async function batchPlayers(c: Context<App>) {
+	// `readRequestFields` serves the v2 client's JSON too, where an id is a real number
+	// rather than a form field's string — coerce rather than filtering those away.
+	const body = c.req.method === 'POST' ? await readRequestFields(c) : {}
+	const raw = body['id'] ?? body['Id']
+	const ids = [
+		...new Set(
+			[...(c.req.queries('id') ?? []), ...(Array.isArray(raw) ? raw : [raw])]
+				.filter((v): v is string | number => typeof v === 'string' || typeof v === 'number')
+				.map((v) => Number.parseInt(String(v).trim(), 10))
+				.filter((n) => !Number.isNaN(n))
+		),
+	]
+	if (ids.length === 0) return c.json(DEFAULT_GET_PLAYER)
+
+	// One query per 99-id chunk (D1 `WHERE account_id IN (…)`), rather than a point read
+	// per id as the KV store required.
+	const presences = await getPresences<RoomInstance>(c.env.DB, ids)
+	return c.json(ids.map((playerId) => playerPayload(playerId, presences.get(playerId))))
+}
+
 /**
 * Look a field up case-insensitively — the client's casing isn't guaranteed across its
 * surfaces — taking the first value when the form encoding repeated it.
@@ -1742,42 +1800,11 @@ const app = new Hono<App>()
 		async (c) => c.json(await getMostActiveClubhouses(c.env.DB))
 	)

-	.get(
-		'/player',
-		describeRoute({
-			tags: ['Presence'],
-			summary: 'Batch player presence lookup',
-			description: [
-				'Returns each requested player’s presence. `id` is a repeated query param',
-				'(`?id=2&id=155&id=153`) — one value each, not comma-separated. With no ids, serves',
-				'a single default (online) player.',
-			].join(' '),
-			parameters: [
-				{
-					name: 'id',
-					in: 'query',
-					required: false,
-					description: 'Repeated once per player id (`?id=2&id=155`); not comma-separated',
-					schema: { type: 'array', items: { type: 'string' } },
-				},
-			],
-			responses: { 200: json(PlayerDto.array(), 'One entry per requested player') },
-		}),
-		async (c) => {
-			// Returns each requested player's presence. Reads the repeated `id` query
-			// param(s); with none it serves the static getplayer.json default.
-			const ids = c.req
-				.queries('id')
-				?.map((s) => Number.parseInt(s.trim(), 10))
-				.filter((n) => !Number.isNaN(n))
-			if (!ids || ids.length === 0) return c.json(DEFAULT_GET_PLAYER)
-
-			// One query for the whole batch (D1 `WHERE account_id IN (…)`), rather than a
-			// point read per id as the KV store required.
-			const presences = await getPresences<RoomInstance>(c.env.DB, ids)
-			return c.json(ids.map((playerId) => playerPayload(playerId, presences.get(playerId))))
-		}
-	)
+	.get('/player', describeRoute(batchPlayerRouteSpec('query')), batchPlayers)
+	// The 2023 client asks about its whole friends list at once and posts the ids in a
+	// form-urlencoded body rather than hanging a few hundred of them off the URL. Same
+	// lookup as the GET, which stays for callers that prefer a query string.
+	.post('/player', describeRoute(batchPlayerRouteSpec('body')), batchPlayers)

 	.post(
 		'/player/heartbeat',
@@ -160,6 +160,15 @@ export const MatchmakeResponse = z.object({
 		.describe('Echoes the request’s CorrelationId; all-zero GUID when it sent none'),
 })

+/**
+ * `POST /player` form body. `id` repeats once per player (the client asks about its whole
+ * friends list at once, too many ids to hang off a URL). One id per value — unlike
+ * `accounts`' bulk lookup, these are not comma-separated.
+ */
+export const BatchPlayerIdsRequest = z.object({
+	id: z.union([z.string(), z.string().array()]).describe('Repeated once per player id'),
+})
+
 /**
 * `GET /player/avoidjuniors` — a BARE JSON boolean (`true`/`false`), not an envelope and
 * not a `{ value }` wrapper. The whole body is the preference.
@@ -469,6 +469,41 @@ describe('public endpoints', () => {
 		expect(players.every((p) => p.isOnline === false)).toBe(true)
 	})

+	test('POST /player reads the ids from a form body', async () => {
+		// The 2023 client asks about its friends list as a POST — the ids are in a
+		// form-urlencoded body, not the query string. GET-only left it a 404.
+		const res = await exports.default.fetch(`${ORIGIN}/player`, {
+			method: 'POST',
+			headers: { 'content-type': 'application/x-www-form-urlencoded' },
+			body: 'id=1070&id=1380&id=1070',
+		})
+		expect(res.status).toBe(200)
+		const players = (await res.json()) as Array<{ playerId: number; isOnline: boolean }>
+		// One entry per id, deduped, in request order.
+		expect(players.map((p) => p.playerId)).toEqual([1070, 1380])
+	})
+
+	test('POST /player answers more ids than D1 will bind at once', async () => {
+		// D1 caps a statement at 100 bound parameters and the expiry check takes one of
+		// them, so a real friends list overruns an unchunked `IN (…)` and 500s.
+		const ids = Array.from({ length: 250 }, (_, i) => 4000 + i)
+		const res = await exports.default.fetch(`${ORIGIN}/player`, {
+			method: 'POST',
+			headers: { 'content-type': 'application/x-www-form-urlencoded' },
+			body: ids.map((id) => `id=${id}`).join('&'),
+		})
+		expect(res.status).toBe(200)
+		const players = (await res.json()) as Array<{ playerId: number }>
+		expect(players.map((p) => p.playerId)).toEqual(ids)
+	})
+
+	test('POST /player without an id returns the default payload', async () => {
+		const res = await exports.default.fetch(`${ORIGIN}/player`, { method: 'POST' })
+		expect(res.status).toBe(200)
+		const players = (await res.json()) as Array<{ playerId: number; isOnline: boolean }>
+		expect(players[0]).toMatchObject({ playerId: 1, isOnline: true, appVersion: GAME_VERSION })
+	})
+
 	test('GET /player without an id returns the default payload', async () => {
 		const res = await exports.default.fetch(`${ORIGIN}/player`)
 		expect(res.status).toBe(200)
@@ -3280,6 +3315,7 @@ describe('auth-gated endpoints', () => {
 			'POST /matchmake/v2/player/{playerId}',
 			'POST /matchmake/v2/room/{roomId}',
 			'POST /matchmake/v2/room/{roomId}/{subRoomId}',
+			'POST /player',
 			'POST /player/exclusivelogin',
 			'POST /player/heartbeat',
 			'POST /player/login',