From eab86ec223b6ad6d78c89eb2a30e91519dbaff0e Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Tue, 20 Oct 2026 16:25:51 -0400 Subject: [PATCH] [rooms] fix slow room role query --- apps/rooms/migrations/0029_room_role.sql | 31 +++++ .../rooms/migrations/0030_room_role_entry.sql | 46 +++++++ apps/rooms/src/test/integration/api.test.ts | 33 ++++- packages/domain/src/rooms-db.ts | 129 +++++++++++++----- 4 files changed, 204 insertions(+), 35 deletions(-) create mode 100644 apps/rooms/migrations/0029_room_role.sql create mode 100644 apps/rooms/migrations/0030_room_role_entry.sql diff --git a/apps/rooms/migrations/0029_room_role.sql b/apps/rooms/migrations/0029_room_role.sql new file mode 100644 index 0000000..c86c8d1 --- /dev/null +++ b/apps/rooms/migrations/0029_room_role.sql @@ -0,0 +1,31 @@ +-- Index a room's `Roles` by account, so "rooms this account has a role in" +-- (`getContributedRooms`) is a lookup instead of a scan. +-- +-- Roles live in the room blob, and the only way to ask "which rooms name account N" of +-- the blob is a `json_each` over every room's `$.Roles`: the whole table, every blob +-- parsed, on every call. `room_role` is a DERIVED copy of those entries, one row per +-- (account, room). The blob stays the source of truth; every write in +-- `packages/domain/src/rooms-db.ts` that changes `Roles` batches `roomRoleWrites` with it. +-- +-- Every entry with an `AccountId` counts, pending invites (`InvitedRole` only) included, +-- which is exactly what the json_each query matched. + +CREATE TABLE IF NOT EXISTS room_role ( + account_id INTEGER NOT NULL, + room_id INTEGER NOT NULL, + PRIMARY KEY (account_id, room_id) +) WITHOUT ROWID; + +CREATE INDEX IF NOT EXISTS idx_room_role_room ON room_role (room_id); + +-- Backfill from the rooms already stored. A full rebuild rather than an append, so this can +-- be re-run as-is to catch role changes made by the old code between migrating and deploying. +-- Non-object entries are skipped through a CASE rather than handed to json_extract, which +-- raises on a bare string. +DELETE FROM room_role; +INSERT OR IGNORE INTO room_role (account_id, room_id) +SELECT account_id, room_id FROM ( + SELECT CASE WHEN role.type = 'object' THEN json_extract(role.value, '$.AccountId') END AS account_id, + room.room_id AS room_id + FROM room, json_each(room.data, '$.Roles') AS role +) WHERE account_id > 0 AND room_id IS NOT NULL; diff --git a/apps/rooms/migrations/0030_room_role_entry.sql b/apps/rooms/migrations/0030_room_role_entry.sql new file mode 100644 index 0000000..86c9c18 --- /dev/null +++ b/apps/rooms/migrations/0030_room_role_entry.sql @@ -0,0 +1,46 @@ +-- Give `room_role` the whole role entry, not just which account a room names. +-- +-- 0029 stored (account_id, room_id) alone, which is all `getContributedRooms` asks of it. +-- A table of a room's roles should carry the roles themselves, though: each row now mirrors +-- one `Roles` entry from the room blob, so a query that cares about the tier (managers only, +-- pending invites) can filter on it here instead of going back to `json_each` over every +-- room. The blob is still the source of truth, and `roomRoleWrites` still rewrites a room's +-- rows alongside every write that changes `Roles`. +-- +-- Columns follow `roomRoles()`, which fills in what older entries lack: `role` and +-- `invited_role` default to 0 (Role.None), `last_changed_by_account_id` to null. +-- +-- Nothing reads the table until the worker deploys, so it is rebuilt rather than altered. +-- The triggers are dropped in case an early draft of 0029 created them; they are not used. + +DROP TRIGGER IF EXISTS room_role_insert; +DROP TRIGGER IF EXISTS room_role_update; +DROP TRIGGER IF EXISTS room_role_delete; +DROP TABLE IF EXISTS room_role; + +CREATE TABLE room_role ( + account_id INTEGER NOT NULL, + room_id INTEGER NOT NULL, + role INTEGER NOT NULL DEFAULT 0, + invited_role INTEGER NOT NULL DEFAULT 0, + last_changed_by_account_id INTEGER, + PRIMARY KEY (account_id, room_id) +) WITHOUT ROWID; + +CREATE INDEX IF NOT EXISTS idx_room_role_room ON room_role (room_id); + +-- Backfill from the rooms already stored. Like 0029's, it is a full rebuild, so it can be +-- re-run as-is after the deploy to catch role changes the old code made in between. +-- Non-object entries are skipped through a CASE rather than handed to json_extract, which +-- raises on a bare string. +DELETE FROM room_role; +INSERT OR IGNORE INTO room_role (account_id, room_id, role, invited_role, last_changed_by_account_id) +SELECT account_id, room_id, COALESCE(role, 0), COALESCE(invited_role, 0), last_changed_by_account_id +FROM ( + SELECT room.room_id AS room_id, + CASE WHEN r.type = 'object' THEN json_extract(r.value, '$.AccountId') END AS account_id, + CASE WHEN r.type = 'object' THEN json_extract(r.value, '$.Role') END AS role, + CASE WHEN r.type = 'object' THEN json_extract(r.value, '$.InvitedRole') END AS invited_role, + CASE WHEN r.type = 'object' THEN json_extract(r.value, '$.LastChangedByAccountId') END AS last_changed_by_account_id + FROM room, json_each(room.data, '$.Roles') AS r +) WHERE account_id > 0 AND room_id IS NOT NULL; diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index 3888e0d..deee9cf 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -13,6 +13,7 @@ import { countRoomsUsingInvention, createRoomInstance, decodeInventionUsage, + getRoomById, getRoomInstance, MessageType, NOTIFICATION_SCHEMA_DDL, @@ -20,11 +21,14 @@ import { publicStudioBundleFilename, putUnityAsset, recordRoomVisit, + removeRoomRole, ROOM_INSTANCE_SCHEMA_DDL, ROOM_INVITE_SCHEMA_DDL, ROOM_SCHEMA_DDL, ROOM_XP_SCHEMA_DDL, + roomRoleWrites, seedRoomWithSubRooms, + setRoomRole, STUDIO_CLOUD_BUILD_SCHEMA_DDL, SUBROOM_SCHEMA_DDL, UNITY_ASSET_SCHEMA_DDL, @@ -456,8 +460,20 @@ describe('rooms endpoints', () => { }) it('GET /rooms/contributedby/me lists rooms the caller owns or has a role in', async () => { + // The roles half reads `room_role`, so a seeded room's roles go there too, as every + // write in rooms-db does. const seed = (data: Record) => - env.DB.prepare('INSERT INTO room (data) VALUES (?1)').bind(JSON.stringify(data)).run() + env.DB.batch([ + env.DB.prepare('INSERT INTO room (data) VALUES (?1)').bind(JSON.stringify(data)), + ...roomRoleWrites(env.DB, Number(data.RoomId), data as Room), + ]) + const contributed = async (accountId: string) => { + const res = await SELF.fetch(`${ORIGIN}/rooms/contributedby/me`, { + headers: await bearer(accountId), + }) + const rooms = (await res.json()) as Array<{ RoomId: number }> + return rooms.map((r) => r.RoomId).sort((a, b) => a - b) + } // A room somebody else made, where 820 is a co-owner... await seed({ @@ -545,9 +561,24 @@ describe('rooms endpoints', () => { // Auth-scoped: `me` is the token, so no token is a 401. expect((await SELF.fetch(`${ORIGIN}/rooms/contributedby/me`)).status).toBe(401) + // Role changes move a room in and out of the list: granting 829 a role in 30404 + // lists it for them, and taking the role back drops it again. + const other = (await getRoomById(env.DB, 30404))! + const granted = await setRoomRole(env.DB, 30404, 829, 10, 821, other) + expect(await contributed('829')).toEqual([30404]) + // The row carries the whole entry, as the blob does. + expect( + await env.DB.prepare( + 'SELECT role, invited_role, last_changed_by_account_id FROM room_role WHERE room_id = 30404 AND account_id = 829' + ).first() + ).toEqual({ role: 10, invited_role: 0, last_changed_by_account_id: 821 }) + await removeRoomRole(env.DB, 30404, 829, 821, granted!) + expect(await contributed('829')).toEqual([]) + // The DB is shared across this file, and these are the only player-made public rooms // in it — leaving them behind changes what the `new`/`community` room feeds serve. await env.DB.prepare('DELETE FROM room WHERE room_id BETWEEN 30401 AND 30407').run() + await env.DB.prepare('DELETE FROM room_role WHERE room_id BETWEEN 30401 AND 30407').run() }) it('GET /rooms/:roomId/experience serves the room’s stored XP settings, no auth', async () => { diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index e9c23da..8893ccd 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -85,6 +85,21 @@ export const ROOM_SCHEMA_DDL: string[] = [ PRIMARY KEY (room_id, tag) )`, `CREATE INDEX IF NOT EXISTS idx_room_tag_tag ON room_tag (tag)`, + // A room's `Roles`, one row per entry (migrations/0029_room_role.sql, rebuilt with the + // whole entry by 0030_room_role_entry.sql). Unlike `room_tag` this is a DERIVED copy: the + // blob keeps `Roles` and stays authoritative, and every write that changes them batches + // {@link roomRoleWrites} alongside. It exists so "which rooms name this account" (and at + // what tier) is an index lookup instead of a `json_each` over every room in the table. + // Columns mirror {@link roomRoles}, defaults included. + `CREATE TABLE IF NOT EXISTS room_role ( + account_id INTEGER NOT NULL, + room_id INTEGER NOT NULL, + role INTEGER NOT NULL DEFAULT 0, + invited_role INTEGER NOT NULL DEFAULT 0, + last_changed_by_account_id INTEGER, + PRIMARY KEY (account_id, room_id) + ) WITHOUT ROWID`, + `CREATE INDEX IF NOT EXISTS idx_room_role_room ON room_role (room_id)`, // Per-player interaction state with a room (cheered/favorited + last visit). // One row per (player, room); cheer/favorite are toggled in place. `CREATE TABLE IF NOT EXISTS interaction ( @@ -267,6 +282,40 @@ export function roomRoles(room: Room): RoomRole[] { })) } +/** + * The statements that bring `room_role` in line with a room's `Roles`: drop the room's rows, + * then insert each entry whole, as {@link roomRoles} reads it. Every entry is stored, + * pending invites (`Role` 0 with an `InvitedRole`) included. An account named twice keeps + * its first entry, which is the one {@link roomRoles}' `find` callers see. + * + * Batch these WITH the write that changes the blob, so the copy can't drift from it. The + * entries go in as one JSON array rather than binds per row, so no role list can hit D1's + * bound-parameter cap. + */ +export function roomRoleWrites(db: D1Database, roomId: number, room: Room): D1PreparedStatement[] { + // A malformed tier reads as NaN, which JSON writes as null and the NOT NULL column would + // refuse — failing the room write it is batched with. Store it as None instead. + const tier = (n: number) => (Number.isFinite(n) ? n : Role.None) + const entries = roomRoles(room) + .filter((r) => Number.isInteger(r.AccountId) && r.AccountId > 0) + .map((r) => ({ ...r, Role: tier(r.Role), InvitedRole: tier(r.InvitedRole) })) + const statements = [db.prepare('DELETE FROM room_role WHERE room_id = ?1').bind(roomId)] + if (entries.length > 0) { + statements.push( + db + .prepare( + `INSERT OR IGNORE INTO room_role + (account_id, room_id, role, invited_role, last_changed_by_account_id) + SELECT json_extract(value, '$.AccountId'), ?1, json_extract(value, '$.Role'), + json_extract(value, '$.InvitedRole'), json_extract(value, '$.LastChangedByAccountId') + FROM json_each(?2)` + ) + .bind(roomId, JSON.stringify(entries)) + ) + } + return statements +} + /** * Room roles that confer owner-level management of a room: Creator (255) and * CoOwner (30). The reference gates its room-admin actions on this set. (Host and @@ -830,7 +879,10 @@ export async function cloneRoom( // serializeRoom drops the hydrated SubRooms from the blob; the clone's subrooms are // inserted into the subroom table below with fresh globally-unique ids. - await db.prepare('INSERT INTO room (data) VALUES (?1)').bind(serializeRoom(cloned)).run() + await db.batch([ + db.prepare('INSERT INTO room (data) VALUES (?1)').bind(serializeRoom(cloned)), + ...roomRoleWrites(db, newRoomId, cloned), + ]) // Tags live in `room_tag`, not the blob: the system tags need their own write. if (systemTags.length > 0) await setRoomTags(db, newRoomId, systemTags) const sourceSubRooms = Array.isArray(source.SubRooms) ? (source.SubRooms as SubRoom[]) : [] @@ -922,6 +974,7 @@ export async function createStudioRoom( .first<{ data: string }>() if (!inserted) return null const created = JSON.parse(inserted.data) as Room + await db.batch(roomRoleWrites(db, Number(created.RoomId), created)) // Studio's picker and empty-save loader support these modern Maker Room scenes, // not the legacy sandbox scene used by generic game subroom creation. const scene = @@ -1059,10 +1112,10 @@ export async function updateRoomFields( patch: Record ): Promise { const updated: Room = { ...room, ...patch } - await db - .prepare('UPDATE room SET data = ?2 WHERE room_id = ?1') - .bind(roomId, serializeRoom(updated)) - .run() + await db.batch([ + db.prepare('UPDATE room SET data = ?2 WHERE room_id = ?1').bind(roomId, serializeRoom(updated)), + ...('Roles' in patch ? roomRoleWrites(db, roomId, updated) : []), + ]) return updated } @@ -1124,10 +1177,10 @@ export async function setRoomRole( } const updated: Room = { ...room, Roles: roles } - await db - .prepare('UPDATE room SET data = ?2 WHERE room_id = ?1') - .bind(roomId, serializeRoom(updated)) - .run() + await db.batch([ + db.prepare('UPDATE room SET data = ?2 WHERE room_id = ?1').bind(roomId, serializeRoom(updated)), + ...roomRoleWrites(db, roomId, updated), + ]) return updated } @@ -1181,10 +1234,10 @@ export async function removeRoomRole( roles.splice(index, 1) const updated: Room = { ...room, Roles: roles } - await db - .prepare('UPDATE room SET data = ?2 WHERE room_id = ?1') - .bind(roomId, serializeRoom(updated)) - .run() + await db.batch([ + db.prepare('UPDATE room SET data = ?2 WHERE room_id = ?1').bind(roomId, serializeRoom(updated)), + ...roomRoleWrites(db, roomId, updated), + ]) return updated } @@ -1244,10 +1297,10 @@ export async function transferRoomOwnership( setRole(fromAccountId, Role.CoOwner) const updated: Room = { ...room, CreatorAccountId: toAccountId, Roles: roles } - await db - .prepare('UPDATE room SET data = ?2 WHERE room_id = ?1') - .bind(roomId, serializeRoom(updated)) - .run() + await db.batch([ + db.prepare('UPDATE room SET data = ?2 WHERE room_id = ?1').bind(roomId, serializeRoom(updated)), + ...roomRoleWrites(db, roomId, updated), + ]) return updated } @@ -1304,10 +1357,10 @@ export async function answerRoomRoleInvite( } const updated: Room = { ...room, Roles: roles } - await db - .prepare('UPDATE room SET data = ?2 WHERE room_id = ?1') - .bind(roomId, serializeRoom(updated)) - .run() + await db.batch([ + db.prepare('UPDATE room SET data = ?2 WHERE room_id = ?1').bind(roomId, serializeRoom(updated)), + ...roomRoleWrites(db, roomId, updated), + ]) return updated } @@ -1345,10 +1398,10 @@ export async function inviteRoomRole( }) } const updated: Room = { ...room, Roles: roles } - await db - .prepare('UPDATE room SET data = ?2 WHERE room_id = ?1') - .bind(roomId, serializeRoom(updated)) - .run() + await db.batch([ + db.prepare('UPDATE room SET data = ?2 WHERE room_id = ?1').bind(roomId, serializeRoom(updated)), + ...roomRoleWrites(db, roomId, updated), + ]) return updated } @@ -3131,7 +3184,12 @@ async function setCurrentSave(db: D1Database, subRoomId: number, saveId: number) export async function seedRoomWithSubRooms(db: D1Database, room: Room): Promise { const roomId = Number(room.RoomId) const subRooms = Array.isArray(room.SubRooms) ? (room.SubRooms as SubRoom[]) : [] - await db.prepare('INSERT OR IGNORE INTO room (data) VALUES (?1)').bind(serializeRoom(room)).run() + const seeded = await db + .prepare('INSERT OR IGNORE INTO room (data) VALUES (?1)') + .bind(serializeRoom(room)) + .run() + // An ignored insert left an existing room in place; its role rows are already its own. + if (seeded.meta.changes > 0) await db.batch(roomRoleWrites(db, roomId, room)) // `serializeRoom` drops `Tags`, so a seeded room's tags have to go to their own table or // they'd vanish — the same step the migration's backfill takes for the imported rooms. if (Array.isArray(room.Tags) && room.Tags.length > 0) { @@ -3178,6 +3236,7 @@ export async function deleteRoom(db: D1Database, roomId: number): Promise // Tag rows outlive the blob otherwise, and would keep answering `#tag` searches and // category rows for a room nobody can open. db.prepare('DELETE FROM room_tag WHERE room_id = ?1').bind(roomId), + db.prepare('DELETE FROM room_role WHERE room_id = ?1').bind(roomId), // Saves and permission overrides first — both are keyed by subroom, so they'd be // unreachable once the subrooms themselves are gone. db @@ -3270,19 +3329,18 @@ export async function countRoomsByCreator(db: D1Database, accountId: number): Pr * room the player made. A room matching BOTH halves appears once — the roles half is an * EXISTS, not a join. * - * Roles live inside the room blob rather than in a table of their own, so the match is a - * `json_each` over `$.Roles`. A room with no `Roles` key (or a null one) simply yields no - * rows there rather than erroring, so it only reaches the list if the account created it. + * Roles live inside the room blob, so the roles half reads `room_role`, the trigger-kept + * index of them, rather than a `json_each` over every room's `$.Roles` (which scanned and + * parsed the whole table on every call). Both halves are index lookups, and SQLite runs + * the OR as a multi-index union. A room with no `Roles` has no `room_role` rows, so it + * only reaches the list if the account created it. */ export async function getContributedRooms(db: D1Database, accountId: number): Promise { const { results } = await db .prepare( `SELECT ${ROOM_COLUMNS} FROM room WHERE creator_account_id = ?1 - OR EXISTS ( - SELECT 1 FROM json_each(room.data, '$.Roles') AS role - WHERE json_extract(role.value, '$.AccountId') = ?1 - )` + OR room_id IN (SELECT room_id FROM room_role WHERE account_id = ?1)` ) .bind(accountId) .all() @@ -4245,7 +4303,10 @@ export async function getOrCreateDormRoom(db: D1Database, accountId: number): Pr } // serializeRoom drops any SubRooms carried over from the template; the dorm's own // subroom is inserted into the subroom table below with a fresh globally-unique id. - await db.prepare('INSERT INTO room (data) VALUES (?1)').bind(serializeRoom(room)).run() + await db.batch([ + db.prepare('INSERT INTO room (data) VALUES (?1)').bind(serializeRoom(room)), + ...roomRoleWrites(db, roomId, room), + ]) const subRoom = await insertSubRoom(db, roomId, { ...templateSub, CreatorAccountId: accountId }) room.SubRooms = [subRoom] // The template carries these (it was parsed), but a dorm minted without one wouldn't.