diff --git a/apps/match/src/match.app.ts b/apps/match/src/match.app.ts index d306419..c3f0389 100644 --- a/apps/match/src/match.app.ts +++ b/apps/match/src/match.app.ts @@ -26,6 +26,7 @@ import { isClubMember, isPlayerBannedFromRoom, MessageType, + recordRoomVisit, refreshInstanceFullness, RoomInstanceType, setPresence, @@ -234,7 +235,8 @@ async function notifyFriendsPresence(c: Context, playerId: number): Promise } /** - * Store the room instance the player just matchmade into, preserving status. + * Store the room instance the player just matchmade into, preserving status, and count + * the visit against the room. * * With no live presence to carry forward (the player's first matchmake after login, * or one after their presence lapsed) the device fields would otherwise default — @@ -259,6 +261,22 @@ async function enterRoom(c: Context, id: number, roomInstance: RoomInstance // and the heartbeat can keep verifying against it. loginLock: prev?.loginLock, }) + + // Count the visit. Every matchmake route funnels through here with the instance the + // player landed in, and a matchmake is the only way into a room, so this is the one + // place a visit can be recorded once — whether they got here by room id, by subroom, + // by following a friend, from a club's clubhouse, or into their own dorm. Bumps the + // room's `visits` column, which is served as `Stats.VisitCount`. Best-effort: a + // counter is not worth failing the matchmake over. + try { + await recordRoomVisit(c.env.DB, roomInstance.roomId) + } catch (err) { + logger.error('failed to record room visit', { + roomId: roomInstance.roomId, + error: err instanceof Error ? err.message : String(err), + }) + } + // Keep the destination instance's is_full flag in sync with live presence (the // player's own presence, just written, is counted). Then re-evaluate the // instance they left — its head-count dropped — so a full room frees up when diff --git a/apps/match/src/test/integration/api.test.ts b/apps/match/src/test/integration/api.test.ts index b3fb610..1d45b81 100644 --- a/apps/match/src/test/integration/api.test.ts +++ b/apps/match/src/test/integration/api.test.ts @@ -278,6 +278,46 @@ describe('public endpoints', () => { }) }) + test('a matchmake counts a visit against the room', async () => { + const visits = async (roomId: number): Promise => + (await env.DB.prepare('SELECT visits FROM room WHERE room_id = ?1') + .bind(roomId) + .first<{ visits: number }>())!.visits + const enter = async (path: string, player: string) => { + const res = await exports.default.fetch(`${ORIGIN}${path}`, { + method: 'POST', + headers: { + ...(await bearer(player)), + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body: new URLSearchParams({ JoinMode: '2' }).toString(), + }) + expect(res.status).toBe(200) + } + + // Counted per matchmake, whichever route got the player there — the two-segment + // room form and the subroom form both land in room 77. + const before = await visits(77) + await enter('/matchmake/room/77', '94') + expect(await visits(77)).toBe(before + 1) + await enter('/matchmake/room/77/35', '95') + expect(await visits(77)).toBe(before + 2) + + // Same player entering again is another visit (VisitCount is visits, not visitors), + // and it's the entered room that's counted — not every room. + const otherBefore = await visits(2) + await enter('/matchmake/room/77', '94') + expect(await visits(77)).toBe(before + 3) + expect(await visits(2)).toBe(otherBefore) + + // A refused matchmake counts nothing: an unknown room has no row to bump. + const res = await exports.default.fetch(`${ORIGIN}/matchmake/room/99999`, { + method: 'POST', + headers: await bearer('96'), + }) + expect(((await res.json()) as { errorCode: number }).errorCode).toBe(20) + }) + test('POST /matchmake/room/:roomId seeds presence with the account device class', async () => { // A screen player (deviceClass 2, recorded by auth at login) matchmaking with no // live presence: without the account fallback they'd enter the room as deviceClass diff --git a/apps/rooms/migrations/0011_room_visits.sql b/apps/rooms/migrations/0011_room_visits.sql new file mode 100644 index 0000000..13daa3e --- /dev/null +++ b/apps/rooms/migrations/0011_room_visits.sql @@ -0,0 +1,17 @@ +-- Lifetime visit counter on `room`. The `match` worker bumps it once per successful +-- matchmake into the room (see recordRoomVisit, called from match's enterRoom), which +-- is the only way a player ever lands in a room, and every room read serves it as the +-- room's `Stats.VisitCount`. +-- +-- A real column rather than a field in the `data` blob: a visit has to be one atomic +-- `visits = visits + 1` UPDATE. Writing it into the blob would mean reading the whole +-- room, editing the JSON and writing it back, so two players entering at once would +-- lose one of the visits — and would race every other writer of the room besides. +-- +-- Unlike CheerCount/FavoriteCount it can't be derived on read either: a visit leaves +-- no per-player row to count (`interaction.last_visited_at` is only stamped by the +-- cheer/favorite toggles). Existing rooms start from 0 — the count begins now. +-- +-- Generated from packages/domain/src/rooms-db.ts (ROOM_SCHEMA_DDL) — keep in sync. + +ALTER TABLE room ADD COLUMN visits INTEGER NOT NULL DEFAULT 0; diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index e8ba043..5b06834 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -140,8 +140,10 @@ export const RoomTagDto = z.object({ /** * A room's engagement counters. `CheerCount`/`FavoriteCount` are aggregated from the - * per-player `interaction` rows on every read; nothing records visits yet, so - * `VisitorCount`/`VisitCount` stay at 0. + * per-player `interaction` rows on every read. `VisitCount` is the room's lifetime + * visits — the `room.visits` column, bumped by the `match` worker on every successful + * matchmake into the room. Nothing records distinct visitors, so `VisitorCount` stays + * at 0. */ export const RoomStatsDto = z.object({ CheerCount: z.int(), diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index f62120b..3b7f9dc 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -2066,7 +2066,8 @@ describe('rooms endpoints', () => { expect(await searched()).toMatchObject({ CheerCount: 2, FavoriteCount: 1 }) expect(await direct()).toMatchObject({ CheerCount: 2, FavoriteCount: 1 }) - // Clearing a cheer decrements it. Nothing records visits, so those stay 0. + // Clearing a cheer decrements it. Visits are counted by the `match` worker on + // matchmake and nobody has entered this room, so those stay 0. await interact('562', 'cheer', 'DELETE') expect(await direct()).toEqual({ CheerCount: 1, @@ -2074,6 +2075,35 @@ describe('rooms endpoints', () => { VisitorCount: 0, VisitCount: 0, }) + + // VisitCount is the `room.visits` column (what match bumps on each matchmake), + // served on every read of the room — here and in the search results — and it + // survives the cheer/favorite aggregation rather than being zeroed by it. + await env.DB.prepare('UPDATE room SET visits = 7 WHERE room_id = 15').run() + expect(await direct()).toEqual({ + CheerCount: 1, + FavoriteCount: 1, + VisitorCount: 0, + VisitCount: 7, + }) + expect(await searched()).toMatchObject({ VisitCount: 7 }) + + // A write to the room doesn't bake the count into the blob (nor reset it). + // Account 1 created room 15, so the description write is allowed. + const wrote = await SELF.fetch(`${ORIGIN}/rooms/15/description`, { + method: 'PUT', + headers: { + ...(await bearer('1')), + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body: new URLSearchParams({ description: 'counted' }).toString(), + }) + expect(wrote.status).toBe(200) + const blob = await env.DB.prepare('SELECT data FROM room WHERE room_id = 15').first<{ + data: string + }>() + expect((JSON.parse(blob!.data) as { Stats: Stats }).Stats.VisitCount).toBe(0) + expect(await direct()).toMatchObject({ VisitCount: 7 }) }) it('DELETE /rooms/:id/interactionby/me/cheer clears the cheer (auth-gated, idempotent)', async () => { diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index b593d18..423e227 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -28,7 +28,13 @@ export const ROOM_SCHEMA_DDL: string[] = [ name TEXT GENERATED ALWAYS AS (json_extract(data, '$.Name')) VIRTUAL, name_lower TEXT GENERATED ALWAYS AS (lower(json_extract(data, '$.Name'))) VIRTUAL, creator_account_id INTEGER GENERATED ALWAYS AS (json_extract(data, '$.CreatorAccountId')) VIRTUAL, - is_dorm INTEGER GENERATED ALWAYS AS (json_extract(data, '$.IsDorm')) VIRTUAL + is_dorm INTEGER GENERATED ALWAYS AS (json_extract(data, '$.IsDorm')) VIRTUAL, + -- Lifetime visit counter (migrations/0011_room_visits.sql, which appends it here): + -- bumped once per successful matchmake into the room by {@link recordRoomVisit}, + -- and served as the room's \`Stats.VisitCount\`. A real column rather than a field + -- in the blob so a visit is one atomic UPDATE that can't lose a concurrent + -- read-modify-write of the whole room. + visits INTEGER NOT NULL DEFAULT 0 )`, `CREATE UNIQUE INDEX IF NOT EXISTS idx_rooms_room_id ON room (room_id)`, `CREATE INDEX IF NOT EXISTS idx_rooms_name_lower ON room (name_lower)`, @@ -853,10 +859,28 @@ export async function deleteSubRoom( interface RoomRow { data: string + visits: number } -const parseOne = (row: RoomRow | null): Room | null => (row ? (JSON.parse(row.data) as Room) : null) -const parseAll = (rows: RoomRow[]): Room[] => rows.map((r) => JSON.parse(r.data) as Room) +/** + * The columns every room read selects. `visits` is authoritative for the room's + * `Stats.VisitCount` (the blob keeps it at 0 — see {@link storedStats}), so it has to + * come back with the blob on every read; a join aliases them (`r.data AS data`). + */ +const ROOM_COLUMNS = 'data, visits' + +/** + * Parse a room row: the stored blob with the counters the columns own folded back in. + * `visits` is a real column, so a room read straight from the DB carries the live count. + */ +const parseRow = (row: RoomRow): Room => { + const room = JSON.parse(row.data) as Room + room.Stats = { ...storedStats(room.Stats), VisitCount: row.visits ?? 0 } + return room +} + +const parseOne = (row: RoomRow | null): Room | null => (row ? parseRow(row) : null) +const parseAll = (rows: RoomRow[]): Room[] => rows.map(parseRow) // ---- Subrooms ------------------------------------------------------------- // Subrooms are their own table (globally-unique autoincrement `sub_room_id`); a @@ -986,8 +1010,14 @@ async function attachSubRooms(db: D1Database, rooms: Room[]): Promise { // A room's cheer/favorite counters are DERIVED from the `interaction` table rather than // stored: they're recomputed on every read, so a cheer shows up immediately and the // counts can't drift from the per-player rows they're made of. The blob keeps them at 0 -// (see {@link serializeRoom}). `VisitorCount`/`VisitCount` are left as the blob has them -// — nothing records a visit yet, and `interaction.last_visited_at` is only stamped by the +// (see {@link serializeRoom}). +// +// `VisitCount` is neither stored in the blob nor derived: it's the `room.visits` column, +// incremented by {@link recordRoomVisit} on each matchmake and read back with the blob +// (see {@link parseRow}). It can't be derived the way cheers are — a visit leaves no +// per-player row to count — and it can't live in the blob, where a read-modify-write of +// the whole room would drop concurrent visits. `VisitorCount` (distinct visitors) is +// still left as the blob has it: `interaction.last_visited_at` is only stamped by the // cheer/favorite toggles, so counting those rows would report cheerers as visitors. /** One room's derived engagement counters (the aggregate maps below key these by RoomId). */ @@ -1012,13 +1042,23 @@ const STATS_ID_LIMIT = 90 const roomIdOf = (room: Room): number => (typeof room.RoomId === 'number' ? room.RoomId : 0) /** - * The `Stats` object to persist: whatever the room carried, with the derived counters - * back at 0 so the blob never holds a stale copy of them. + * The `Stats` object to persist: whatever the room carried, with the counters the + * columns/tables own back at 0 so the blob never holds a stale copy of them. */ function storedStats(stats: unknown): Record { const stored = typeof stats === 'object' && stats !== null ? (stats as Record) : {} - return { ...ZERO_STATS, ...stored, CheerCount: 0, FavoriteCount: 0 } + return { ...ZERO_STATS, ...stored, CheerCount: 0, FavoriteCount: 0, VisitCount: 0 } +} + +/** + * Count one visit to a room — the `match` worker calls this on every successful + * matchmake (see its `enterRoom`), which is the only way a player ever lands in a room. + * A blind `visits = visits + 1` UPDATE: it's the whole write, so simultaneous visitors + * can't clobber each other, and an unknown room id simply matches nothing. + */ +export async function recordRoomVisit(db: D1Database, roomId: number): Promise { + await db.prepare('UPDATE room SET visits = visits + 1 WHERE room_id = ?1').bind(roomId).run() } /** @@ -1063,8 +1103,12 @@ async function attachStats( const byRoom = stats ?? (await getRoomStats(db, [...new Set(rooms.map(roomIdOf))])) for (const room of rooms) { const counts = byRoom.get(roomIdOf(room)) + // `storedStats` zeroes VisitCount (the blob doesn't own it), so carry over the + // value `parseRow` folded in from the `visits` column rather than losing it here. + const stats = (room.Stats ?? {}) as Record room.Stats = { - ...storedStats(room.Stats), + ...storedStats(stats), + VisitCount: typeof stats.VisitCount === 'number' ? stats.VisitCount : 0, CheerCount: counts?.CheerCount ?? 0, FavoriteCount: counts?.FavoriteCount ?? 0, } @@ -1384,7 +1428,10 @@ export async function getRoomById(db: D1Database, roomId: number): Promise() + await db + .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE room_id = ?1`) + .bind(roomId) + .first() ) ) } @@ -1422,7 +1469,7 @@ export async function getRoomByName(db: D1Database, name: string): Promise() ) @@ -1434,7 +1481,7 @@ export async function getRoomsByIds(db: D1Database, ids: number[]): Promise `?${i + 1}`).join(',') const { results } = await db - .prepare(`SELECT data FROM room WHERE room_id IN (${placeholders})`) + .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE room_id IN (${placeholders})`) .bind(...ids) .all() return hydrateRooms(db, parseAll(results)) @@ -1443,7 +1490,7 @@ export async function getRoomsByIds(db: D1Database, ids: number[]): Promise { const { results } = await db - .prepare('SELECT data FROM room WHERE creator_account_id = ?1') + .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE creator_account_id = ?1`) .bind(accountId) .all() return hydrateRooms(db, parseAll(results)) @@ -1490,7 +1537,7 @@ export async function getFavoritedRooms( ): Promise { const { results } = await db .prepare( - `SELECT r.data AS data + `SELECT r.data AS data, r.visits AS visits FROM interaction i JOIN room r ON r.room_id = i.room_id WHERE i.player_id = ?1 AND i.favorited = 1 @@ -1515,7 +1562,7 @@ export async function getVisitedRooms( ): Promise { const { results } = await db .prepare( - `SELECT r.data AS data + `SELECT r.data AS data, r.visits AS visits FROM interaction i JOIN room r ON r.room_id = i.room_id WHERE i.player_id = ?1 AND i.last_visited_at IS NOT NULL @@ -1674,7 +1721,7 @@ export async function searchRooms( if (q === '') return { Results: [], TotalResults: 0 } const terms = q.split(/[\s+]+/).filter(Boolean) - const { results } = await db.prepare('SELECT data FROM room').all() + const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() let rooms = parseAll(results).filter((r) => r.IsDorm !== true && r.Accessibility === 1) for (const term of terms) { @@ -1745,7 +1792,7 @@ export async function getHotRooms( skip: number, take: number ): Promise<{ Results: Room[]; TotalResults: number }> { - const { results } = await db.prepare('SELECT data FROM room').all() + const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() let rooms = parseAll(results).filter( (r) => r.IsDorm !== true && r.Accessibility === 1 && r.ExcludeFromLists !== true ) @@ -1796,7 +1843,7 @@ export async function getRecommendedRooms( skip: number, take: number ): Promise { - const { results } = await db.prepare('SELECT data FROM room').all() + const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() const stats = await getRoomStats(db) return hydrateRooms( db, @@ -1834,7 +1881,7 @@ export interface FeaturedRoomGroup { * Small dataset, so done in memory. */ export async function getFeaturedRooms(db: D1Database): Promise { - const { results } = await db.prepare('SELECT data FROM room').all() + const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() const rooms = parseAll(results).filter( (r) => r.IsDorm !== true && r.Accessibility === 1 && r.ExcludeFromLists !== true ) @@ -1881,7 +1928,7 @@ export async function getSimilarRooms( const targetTags = new Set(roomTags(target)) if (targetTags.size === 0) return empty - const { results } = await db.prepare('SELECT data FROM room').all() + const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() const sharedCount = (r: Room): number => roomTags(r).filter((t) => targetTags.has(t)).length const stats = await getRoomStats(db) @@ -1917,7 +1964,7 @@ export async function getSimilarRooms( * array. Small dataset, so done in memory. */ export async function getBaseRooms(db: D1Database, skip: number, take: number): Promise { - const { results } = await db.prepare('SELECT data FROM room').all() + const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() const base = new Set(['base']) return hydrateRooms( db, @@ -1948,7 +1995,9 @@ export async function getDormRoom(db: D1Database, accountId: number): Promise() )