From 6adb3ab741661a9f3681cc102985c13b1bc5709b Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Wed, 2 Sep 2026 00:45:08 -0400 Subject: [PATCH] [rooms] fix #50 room leaderboards --- .../migrations/0016_room_leaderboard.sql | 19 +++ apps/rooms/src/openapi.ts | 28 +++++ apps/rooms/src/rooms.app.ts | 119 ++++++++++++++++++ apps/rooms/src/test/integration/api.test.ts | 86 +++++++++++++ packages/domain/src/rooms-db.ts | 84 +++++++++++++ 5 files changed, 336 insertions(+) create mode 100644 apps/rooms/migrations/0016_room_leaderboard.sql diff --git a/apps/rooms/migrations/0016_room_leaderboard.sql b/apps/rooms/migrations/0016_room_leaderboard.sql new file mode 100644 index 0000000..3fa0c3f --- /dev/null +++ b/apps/rooms/migrations/0016_room_leaderboard.sql @@ -0,0 +1,19 @@ +-- Per-room leaderboard definitions. Generated from packages/domain/src/rooms-db.ts +-- (ROOM_SCHEMA_DDL) — keep in sync. +-- +-- One row per (room, leaderboard). `leaderboard_id` is the client's slot number — small +-- ordinals (1, 2, 3…), unique only within the room, which is why the pair is the primary +-- key rather than the id alone. Re-posting a slot (POST /rooms/:id/leaderboards/:lid) +-- reconfigures it in place; DELETE on the same path removes it. +-- +-- `sort_ascending` stores the client's `sortAscending=True/False` as 1/0; `stat_format` +-- is the client's `statFormat` int, echoed back as stored. + +CREATE TABLE IF NOT EXISTS room_leaderboard ( + room_id INTEGER NOT NULL, + leaderboard_id INTEGER NOT NULL, + leaderboard_title TEXT NOT NULL, + stat_format INTEGER NOT NULL DEFAULT 0, + sort_ascending INTEGER NOT NULL DEFAULT 0, + PRIMARY KEY (room_id, leaderboard_id) + ); diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index df4410c..8e60e0f 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -98,6 +98,12 @@ export const playerIdParam = idParam('playerId', 'The account whose list to read /** The `:playerId` path parameter on the unban route. */ export const bannedPlayerIdParam = idParam('playerId', 'The banned account to unban') +/** The `:leaderboardId` path parameter on the room leaderboard routes. */ +export const leaderboardIdParam = idParam( + 'leaderboardId', + 'The leaderboard slot within the room — small ordinals (1, 2, 3…), unique per room only' +) + /** An optional string query parameter. */ export function stringQuery(name: string, description: string): OpenAPIV3_1.ParameterObject { return { name, in: 'query', required: false, description, schema: { type: 'string' } } @@ -609,6 +615,28 @@ export const RoomBanEnvelope = z.object({ value: RoomBanDto.nullable().describe('Null on a rejection'), }) +/** `POST /rooms/{roomId}/leaderboards/{leaderboardId}` — configure one leaderboard slot. */ +export const LeaderboardRequest = z.object({ + leaderboardTitle: z.string().describe('The title the board displays'), + statFormat: z.string().optional().describe('The stat-format int; defaults to 0'), + sortAscending: z + .string() + .optional() + .describe('`True` / `False` — whether lower scores rank first. Defaults to `False`'), +}) + +/** + * What both leaderboard routes answer: a bare success/failure carrying no entity. + * PascalCase `Success`/`Error` with a lowercase `error_id` — the same mixed casing the + * unprefixed isBanned envelope has ({@link IsBannedPascalEnvelope}), NOT the room + * mutations' lowercase `{ success, error, value }`. + */ +export const LeaderboardResultEnvelope = z.object({ + Success: z.boolean(), + Error: z.string().nullable().describe('The message shown on a rejection; null on success'), + error_id: z.string().nullable().describe('Null. Lowercase, unlike its siblings'), +}) + /** `PUT /rooms/{roomId}/warning`. */ export const WarningRequest = z.object({ warningMask: z.string().describe('Content-warning bit flags, as an integer'), diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index 52524d1..77f5a00 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -14,6 +14,7 @@ import { countRoomsByCreator, createSubRoom, deleteRoom, + deleteRoomLeaderboard, deleteSubRoom, findSubRoom, getBaseRooms, @@ -47,6 +48,7 @@ import { searchRooms, setRoomDescription, setRoomImage, + setRoomLeaderboard, setRoomName, setRoomRole, setSubRoomPermissions, @@ -90,6 +92,9 @@ import { IsBannedPascalEnvelope, json, jsonBody, + leaderboardIdParam, + LeaderboardRequest, + LeaderboardResultEnvelope, LoadScreenRequest, MissingLookupParam, ModifySubRoomRequest, @@ -643,6 +648,15 @@ function roomEnvelope(c: Context, value: unknown, error = '') { */ const banEnvelope = roomEnvelope +/** + * The envelope both leaderboard routes answer: `{ Success, Error, error_id }`, carrying + * no entity. PascalCase with a lowercase `error_id` — the same mixed casing the + * unprefixed isBanned route serves — NOT the room mutations' lowercase envelope. + */ +function leaderboardEnvelope(c: Context, error: string | null = null) { + return c.json({ Success: error === null, Error: error, error_id: null }) +} + /** Rooms created/owned by the authed caller (shared by the createdby routes). */ async function ownedRooms(c: Context) { const accountId = await authedAccountId(c) @@ -2236,6 +2250,111 @@ const app = new Hono() } ) + // Configure one of a room's leaderboard slots (form body `leaderboardTitle` + + // `statFormat` + `sortAscending`). Auth-gated (401) and owner/co-owner-only (403). + // One row per (room, slot) — re-posting a slot reconfigures it, so the call is + // idempotent. + .post( + '/rooms/:roomId{[0-9]+}/leaderboards/:leaderboardId{[0-9]+}', + describeRoute({ + tags: ['Room settings'], + summary: 'Configure a room leaderboard', + description: [ + 'Creates or reconfigures one leaderboard slot in the `room_leaderboard` table — one', + 'row per (room, slot), so re-posting a slot rewrites its title, format and direction', + 'rather than adding a second. The slot number in the path is the client’s small', + 'ordinal (1, 2, 3…), unique only within the room. Owner or co-owner only (403', + 'otherwise).', + '', + '`statFormat` is stored verbatim (default 0); `sortAscending` is the client’s', + '`True`/`False` string (default `False`).', + '', + 'Answers a bare `{ Success, Error, error_id }` — PascalCase with a lowercase', + '`error_id`, like the unprefixed isBanned route, carrying no entity. NOT the room', + 'mutations’ lowercase `{ success, error, value }`.', + ].join('\n'), + security: AUTHED, + parameters: [roomIdParam, leaderboardIdParam], + requestBody: form(LeaderboardRequest, 'The leaderboard configuration'), + responses: { + 200: json(LeaderboardResultEnvelope, 'Stored, or a rejection with `Success: false`'), + 401: UNAUTHORIZED_RESPONSE, + 403: FORBIDDEN_RESPONSE, + }, + }), + async (c) => { + const accountId = await authedAccountId(c) + if (accountId === null) return unauthorized(c) + + const roomId = Number.parseInt(c.req.param('roomId'), 10) + const room = await getRoomById(c.env.DB, roomId) + if (!room) return leaderboardEnvelope(c, 'This room does not exist!') + // A valid token but not the room's owner/co-owner → 403 (the auth gate above + // already returned 401 for a missing/invalid token). + if (!canManageRoom(room, accountId)) return c.body(null, 403) + + const leaderboardId = Number.parseInt(c.req.param('leaderboardId'), 10) + const body = (await c.req.parseBody().catch(() => ({}))) as Record + const title = typeof body.leaderboardTitle === 'string' ? body.leaderboardTitle : '' + const statFormat = + typeof body.statFormat === 'string' ? Number.parseInt(body.statFormat, 10) : Number.NaN + // The client sends .NET-style `True`/`False`; anything but a `true` reads false. + const sortAscending = + typeof body.sortAscending === 'string' && body.sortAscending.toLowerCase() === 'true' + + await setRoomLeaderboard( + c.env.DB, + roomId, + leaderboardId, + title, + Number.isNaN(statFormat) ? 0 : statFormat, + sortAscending + ) + return leaderboardEnvelope(c) + } + ) + + // Remove one of a room's leaderboard slots. Auth-gated (401) and owner/co-owner-only + // (403). The client fires these blindly for every slot when tearing boards down, so a + // slot that isn't configured is a rejection envelope, not an HTTP error. + .delete( + '/rooms/:roomId{[0-9]+}/leaderboards/:leaderboardId{[0-9]+}', + describeRoute({ + tags: ['Room settings'], + summary: 'Remove a room leaderboard', + description: [ + 'Removes the slot’s `room_leaderboard` row. Owner or co-owner only (403 otherwise).', + '', + 'Removing a slot that isn’t configured is a rejection (`Success: false`), not a', + 'silent success — the caller asked to undo something that was not there. The client', + 'deletes slots blindly when tearing boards down and tolerates the refusal.', + '', + 'Answers the same bare `{ Success, Error, error_id }` as the leaderboard write.', + ].join('\n'), + security: AUTHED, + parameters: [roomIdParam, leaderboardIdParam], + responses: { + 200: json(LeaderboardResultEnvelope, 'Removed, or a rejection with `Success: false`'), + 401: UNAUTHORIZED_RESPONSE, + 403: FORBIDDEN_RESPONSE, + }, + }), + async (c) => { + const accountId = await authedAccountId(c) + if (accountId === null) return unauthorized(c) + + const roomId = Number.parseInt(c.req.param('roomId'), 10) + const room = await getRoomById(c.env.DB, roomId) + if (!room) return leaderboardEnvelope(c, 'This room does not exist!') + if (!canManageRoom(room, accountId)) return c.body(null, 403) + + const leaderboardId = Number.parseInt(c.req.param('leaderboardId'), 10) + const removed = await deleteRoomLeaderboard(c.env.DB, roomId, leaderboardId) + if (!removed) return leaderboardEnvelope(c, 'This room has no such leaderboard!') + return leaderboardEnvelope(c) + } + ) + // Set a room's content warning: the `WarningMask` bit flags plus an optional // free-text `CustomWarning`. Auth-gated (401) and owner/co-owner-only (403). Body is // the `warningMask` form field (an integer) and an optional `customWarning` string diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index 514d740..b49ffdc 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -1818,6 +1818,90 @@ describe('rooms endpoints', () => { .run() }) + it('POST/DELETE /rooms/:id/leaderboards/:lid configures and removes a room’s leaderboard slots', async () => { + // RecCenter (room 2) is owned by account 1, with account 2 as co-owner. + const del = async (path: string, sub?: string) => + SELF.fetch(`${ORIGIN}${path}`, { + method: 'DELETE', + headers: sub ? await bearer(sub) : {}, + }) + const slotsOf = async (roomId: number) => + ( + await env.DB.prepare( + 'SELECT leaderboard_id, leaderboard_title, stat_format, sort_ascending FROM room_leaderboard WHERE room_id = ?1 ORDER BY leaderboard_id' + ) + .bind(roomId) + .all() + ).results + + // The real client body, verbatim. + const body = { leaderboardTitle: 'full name', statFormat: '1', sortAscending: 'False' } + + // No token → 401 (auth gate). + expect((await postForm('/rooms/2/leaderboards/1', body)).status).toBe(401) + expect((await del('/rooms/2/leaderboards/1')).status).toBe(401) + // A valid token but no role on the room → 403. + expect((await postForm('/rooms/2/leaderboards/1', body, '999')).status).toBe(403) + expect((await del('/rooms/2/leaderboards/1', '999')).status).toBe(403) + // The envelope both routes answer — PascalCase `Success`/`Error`, lowercase + // `error_id`, no entity. NOT the room mutations' lowercase `{ success, error, value }`. + const OK = { Success: true, Error: null, error_id: null } + + // Unknown room → failure envelope. + expect(await (await postForm('/rooms/99999/leaderboards/1', body, '1')).json()).toEqual({ + Success: false, + Error: 'This room does not exist!', + error_id: null, + }) + + // The owner configures slot 1 — a bare success, with the row persisted. + const ok = await postForm('/rooms/2/leaderboards/1', body, '1') + expect(ok.status).toBe(200) + expect(await ok.json()).toEqual(OK) + expect(await slotsOf(2)).toEqual([ + { leaderboard_id: 1, leaderboard_title: 'full name', stat_format: 1, sort_ascending: 0 }, + ]) + + // Re-posting the slot reconfigures the one row rather than appending — and the + // co-owner may do it. `sortAscending=True` parses case-insensitively. + const rewrite = await postForm( + '/rooms/2/leaderboards/1', + { leaderboardTitle: 'lap time', statFormat: '2', sortAscending: 'True' }, + '2' + ) + expect(rewrite.status).toBe(200) + expect(await rewrite.json()).toEqual(OK) + expect(await slotsOf(2)).toEqual([ + { leaderboard_id: 1, leaderboard_title: 'lap time', stat_format: 2, sort_ascending: 1 }, + ]) + + // Slots are per room: slot 2 here and slot 1 of another room are their own rows. + expect((await postForm('/rooms/2/leaderboards/2', body, '1')).status).toBe(200) + expect((await postForm('/rooms/3/leaderboards/1', body, '1')).status).toBe(200) + expect(await slotsOf(2)).toHaveLength(2) + expect(await slotsOf(3)).toHaveLength(1) + + // DELETE removes exactly the named slot. + const removed = await del('/rooms/2/leaderboards/1', '1') + expect(removed.status).toBe(200) + expect(await removed.json()).toEqual(OK) + expect(await slotsOf(2)).toEqual([ + { leaderboard_id: 2, leaderboard_title: 'full name', stat_format: 1, sort_ascending: 0 }, + ]) + expect(await slotsOf(3)).toHaveLength(1) + + // Deleting a slot that isn't configured is a rejection, not an HTTP error — the + // client tears boards down by deleting every slot blindly. + expect(await (await del('/rooms/2/leaderboards/1', '1')).json()).toEqual({ + Success: false, + Error: 'This room has no such leaderboard!', + error_id: null, + }) + + // Clean up the surviving rows so this test leaves no trace. + await env.DB.prepare('DELETE FROM room_leaderboard WHERE room_id IN (2, 3)').run() + }) + it('POST /rooms/:id/bans kicks the banned player', async () => { type Sent = { playerId: number; notificationType: string | number; data: unknown } const hub = () => env.RECFLARE_NOTIFICATIONS_HUB.getByName('global') @@ -3972,6 +4056,7 @@ describe('rooms endpoints', () => { 'DELETE /rooms/{roomId}/bans/{playerId}', 'DELETE /rooms/{roomId}/interactionby/me/cheer', 'DELETE /rooms/{roomId}/interactionby/me/favorite', + 'DELETE /rooms/{roomId}/leaderboards/{leaderboardId}', 'DELETE /rooms/{roomId}/subrooms/{subRoomId}', 'GET /', 'GET /Room_server/rooms/{roomId}/bans/{playerId}/isBanned', @@ -4010,6 +4095,7 @@ describe('rooms endpoints', () => { 'POST /rooms/bulk', 'POST /rooms/{roomId}/bans', 'POST /rooms/{roomId}/clone', + 'POST /rooms/{roomId}/leaderboards/{leaderboardId}', 'POST /rooms/{roomId}/subrooms', 'POST /rooms/{roomId}/subrooms/{subRoomId}/clone', 'POST /rooms/{roomId}/subrooms/{subRoomId}/data', diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index ef0d835..c68a162 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -103,6 +103,21 @@ export const ROOM_SCHEMA_DDL: string[] = [ PRIMARY KEY (room_id, banned_player_id) )`, `CREATE INDEX IF NOT EXISTS idx_room_ban_player ON room_ban (banned_player_id)`, + // Per-room leaderboard definitions (migrations/0016_room_leaderboard.sql). One row per + // (room, leaderboard): `leaderboard_id` is the client's slot number — small ordinals + // (1, 2, 3…), unique only within the room — so the pair is the key, and re-posting a + // slot reconfigures it in place rather than appending. + // + // Deliberately NOT in the room's `data` blob, same reasoning as `room_ban`: the blob is + // served verbatim as the room and the client doesn't read leaderboards off it. + `CREATE TABLE IF NOT EXISTS room_leaderboard ( + room_id INTEGER NOT NULL, + leaderboard_id INTEGER NOT NULL, + leaderboard_title TEXT NOT NULL, + stat_format INTEGER NOT NULL DEFAULT 0, + sort_ascending INTEGER NOT NULL DEFAULT 0, + PRIMARY KEY (room_id, leaderboard_id) + )`, ] /** @@ -286,6 +301,75 @@ export async function isPlayerBannedFromRoom( return row !== null } +/** A room's leaderboard definition — one configured slot (`leaderboard_id` is per-room). */ +export interface RoomLeaderboard { + RoomId: number + LeaderboardId: number + LeaderboardTitle: string + StatFormat: number + SortAscending: boolean +} + +interface RoomLeaderboardRow { + room_id: number + leaderboard_id: number + leaderboard_title: string + stat_format: number + sort_ascending: number +} + +const toRoomLeaderboard = (row: RoomLeaderboardRow): RoomLeaderboard => ({ + RoomId: row.room_id, + LeaderboardId: row.leaderboard_id, + LeaderboardTitle: row.leaderboard_title, + StatFormat: row.stat_format, + SortAscending: row.sort_ascending === 1, +}) + +/** + * Create or reconfigure one of a room's leaderboard slots, returning the stored + * definition. One row per (room, leaderboard): re-posting a slot rewrites its title, + * format and direction rather than appending a second row, so the call is idempotent. + */ +export async function setRoomLeaderboard( + db: D1Database, + roomId: number, + leaderboardId: number, + leaderboardTitle: string, + statFormat: number, + sortAscending: boolean +): Promise { + const row = await db + .prepare( + `INSERT INTO room_leaderboard (room_id, leaderboard_id, leaderboard_title, stat_format, sort_ascending) + VALUES (?1, ?2, ?3, ?4, ?5) + ON CONFLICT(room_id, leaderboard_id) DO UPDATE SET + leaderboard_title = ?3, stat_format = ?4, sort_ascending = ?5 + RETURNING *` + ) + .bind(roomId, leaderboardId, leaderboardTitle, statFormat, sortAscending ? 1 : 0) + .first() + // RETURNING always yields the upserted row. + return toRoomLeaderboard(row!) +} + +/** + * Remove one of a room's leaderboard slots, returning the definition that was removed — + * or null when the slot wasn't configured, which lets the caller tell a real delete + * from a no-op. + */ +export async function deleteRoomLeaderboard( + db: D1Database, + roomId: number, + leaderboardId: number +): Promise { + const row = await db + .prepare('DELETE FROM room_leaderboard WHERE room_id = ?1 AND leaderboard_id = ?2 RETURNING *') + .bind(roomId, leaderboardId) + .first() + return row ? toRoomLeaderboard(row) : null +} + /** * Clone an existing room into a new one owned by `accountId`. Copies the source * room's content (scene/subrooms/settings), assigning a fresh RoomId, the given