[rooms] obey excludefromlists

This commit is contained in:
Devin Zuczek
2026-10-10 14:35:11 -04:00
parent ab948f1f28
commit c049c8f14d
5 changed files with 116 additions and 30 deletions
+26 -3
View File
@@ -273,10 +273,23 @@ beforeAll(async () => {
SubRooms: [], SubRooms: [],
} as Record<string, unknown>) } as Record<string, unknown>)
} }
// Non-public and a dorm: neither belongs in a discovery row. // Non-public, a dorm, and a public room OPTED OUT of lists: none belongs in a discovery
// row. Room 9 is built to lead every row it is wrongly let into — newest of all, tagged
// `quest`, and (in the hot test) occupied — so a feed that forgets `ExcludeFromLists`
// fails loudly rather than by luck of ordering.
for (const room of [ for (const room of [
{ RoomId: 5, Name: 'SecretRoom', CreatorAccountId: 500, Accessibility: 0, IsDorm: false }, { RoomId: 5, Name: 'SecretRoom', CreatorAccountId: 500, Accessibility: 0, IsDorm: false },
{ RoomId: 6, Name: '@Dorm', CreatorAccountId: 502, Accessibility: 1, IsDorm: true }, { RoomId: 6, Name: '@Dorm', CreatorAccountId: 502, Accessibility: 1, IsDorm: true },
{
RoomId: 9,
Name: 'OptedOut',
CreatorAccountId: 505,
Accessibility: 1,
IsDorm: false,
ExcludeFromLists: true,
CreatedAt: '2026-04-01T00:00:00Z',
Tags: [{ Tag: 'quest', Type: 0 }],
},
]) { ]) {
await seedRoomWithSubRooms(env.DB, { ...room, SubRooms: [] } as Record<string, unknown>) await seedRoomWithSubRooms(env.DB, { ...room, SubRooms: [] } as Record<string, unknown>)
} }
@@ -1201,6 +1214,9 @@ it('serves the live hot-room ranking for /algorithmiclists/HotList', async () =>
await putInRoom(904, 2) await putInRoom(904, 2)
await putInRoom(905, 2) await putInRoom(905, 2)
await putInRoom(906, 2) await putInRoom(906, 2)
// Room 9 is as busy as the leader and opted out of lists: it must not place.
await putInRoom(907, 9)
await putInRoom(908, 9)
const res = await SELF.fetch(`${ORIGIN}/algorithmiclists/HotList?type=1`) const res = await SELF.fetch(`${ORIGIN}/algorithmiclists/HotList?type=1`)
expect(res.status).toBe(200) expect(res.status).toBe(200)
@@ -1220,9 +1236,11 @@ it('serves the live hot-room ranking for /algorithmiclists/HotList', async () =>
// the Coach, whose stock rooms the hot row leaves out — a "Hot" row full of Rec Center // the Coach, whose stock rooms the hot row leaves out — a "Hot" row full of Rec Center
// is a row about the server rather than about what players are doing. // is a row about the server rather than about what players are doing.
expect(ids).not.toContain('2') expect(ids).not.toContain('2')
// The private room and the dorm are not in it either. // The private room and the dorm are not in it either, nor the public room whose
// `ExcludeFromLists` is set — busy as it is.
expect(ids).not.toContain('5') expect(ids).not.toContain('5')
expect(ids).not.toContain('6') expect(ids).not.toContain('6')
expect(ids).not.toContain('9')
// The row key is matched case-insensitively — it reaches us from a curated page's // The row key is matched case-insensitively — it reaches us from a curated page's
// ItemIds, whose casing is the reference's. // ItemIds, whose casing is the reference's.
@@ -1264,7 +1282,7 @@ it('orders /algorithmiclists/new by creation time', async () => {
}) })
it.each(['recentlyupdated', 'new'])( it.each(['recentlyupdated', 'new'])(
'leaves stock, private and dorm rooms out of %s', 'leaves stock, private, dorm and list-excluded rooms out of %s',
async (list) => { async (list) => {
const ids = await rowIds(list) const ids = await rowIds(list)
// Room 2 is the Coach's — this server's stock rooms, which a row about what players have // Room 2 is the Coach's — this server's stock rooms, which a row about what players have
@@ -1273,6 +1291,9 @@ it.each(['recentlyupdated', 'new'])(
// Not public, and a dorm. // Not public, and a dorm.
expect(ids).not.toContain('5') expect(ids).not.toContain('5')
expect(ids).not.toContain('6') expect(ids).not.toContain('6')
// Public and player-made, but `ExcludeFromLists` — and the newest room seeded, so it
// would head `new` if the flag were ignored.
expect(ids).not.toContain('9')
} }
) )
@@ -1338,6 +1359,8 @@ it('answers recentlyvisited empty for a caller with no history and no token', as
it('serves the rooms tagged `quest` for /algorithmiclists/quests_algoendpoint', async () => { it('serves the rooms tagged `quest` for /algorithmiclists/quests_algoendpoint', async () => {
const ids = await rowIds('quests_algoendpoint') const ids = await rowIds('quests_algoendpoint')
// Ordering is the hot feed's (live players, then engagement), so assert membership. // Ordering is the hot feed's (live players, then engagement), so assert membership.
// Room 9 carries the tag too and is absent: a category row asks what a room is about,
// but `ExcludeFromLists` still keeps it out of every row.
expect([...ids].sort()).toEqual(['2', '3']) expect([...ids].sort()).toEqual(['2', '3'])
// Room 2 is the COACH's and belongs here all the same. Every quest-tagged room this // Room 2 is the COACH's and belongs here all the same. Every quest-tagged room this
@@ -0,0 +1,16 @@
-- The flag SEARCH filters on, as an indexed column — the search-side twin of
-- `exclude_from_lists` (0014).
--
-- Generated from packages/domain/src/rooms-db.ts (ROOM_SCHEMA_DDL) — keep in sync.
--
-- `ExcludeFromLists` keeps a room out of the browse and discovery feeds; `ExcludeFromSearch`
-- keeps it out of `/rooms/search` and the search box's suggestions. They are independent:
-- a room may opt out of the feeds and still be findable by name, or the other way round.
-- Until now nothing read `ExcludeFromSearch` at all, so an opted-out room was served by
-- every search that matched it.
--
-- VIRTUAL like `exclude_from_lists`, so the blob stays the only copy and no backfill is
-- needed. It is not added to `idx_room_public`: that partial index already narrows a search
-- to the public, non-dorm rooms, and this term is then checked against the handful of rows
-- it fetched — see SEARCHABLE_WHERE.
ALTER TABLE room ADD COLUMN exclude_from_search INTEGER GENERATED ALWAYS AS (json_extract(data, '$.ExcludeFromSearch')) VIRTUAL;
+12 -7
View File
@@ -1179,8 +1179,9 @@ const app = new Hono<App>()
) )
// Room search: `query` is space/`+`-separated terms — `#tag` matches room tags, // Room search: `query` is space/`+`-separated terms — `#tag` matches room tags,
// plain terms match the name. Public, non-dorm rooms only. Paginated via // plain terms match the name. Public, non-dorm rooms not opted out of search
// skip/take. Returns `{ Results, TotalResults }`. // (`ExcludeFromSearch`) only — `ExcludeFromLists` is the feeds' flag and does not hide
// a room here. Paginated via skip/take. Returns `{ Results, TotalResults }`.
// //
// `#community` is the one tag term that isn't a tag lookup: it is the browse chip's // `#community` is the one tag term that isn't a tag lookup: it is the browse chip's
// pseudo-tag reaching search, and means rooms a PLAYER made rather than the seeded // pseudo-tag reaching search, and means rooms a PLAYER made rather than the seeded
@@ -1192,7 +1193,9 @@ const app = new Hono<App>()
summary: 'Search rooms', summary: 'Search rooms',
description: [ description: [
'Full room search. `query` is space- or `+`-separated terms: a `#tag` term matches the', 'Full room search. `query` is space- or `+`-separated terms: a `#tag` term matches the',
'room’s tags, a plain term matches its name. Public, non-dorm rooms only.', 'room’s tags, a plain term matches its name. Public, non-dorm rooms only, and never one',
'whose `ExcludeFromSearch` is set; `ExcludeFromLists` hides a room from the feeds, not',
'from here.',
'`#community` is a pseudo-tag no room carries — it narrows to rooms a player made', '`#community` is a pseudo-tag no room carries — it narrows to rooms a player made',
'(anything the system Coach account didn’t create), like `/rooms/hot?tag=community`.', '(anything the system Coach account didn’t create), like `/rooms/hot?tag=community`.',
].join(' '), ].join(' '),
@@ -1218,8 +1221,9 @@ const app = new Hono<App>()
// //
// Every suggestion is something `/rooms/search` will actually find, so submitting one // Every suggestion is something `/rooms/search` will actually find, so submitting one
// can't come back empty: they're drawn from room names (what a plain search term // can't come back empty: they're drawn from room names (what a plain search term
// matches) and room tags (what a `#tag` term matches), over the same public, non-dorm // matches) and room tags (what a `#tag` term matches), over the same public, non-dorm,
// rooms search considers. Tags come back with their `#` for that reason. // searchable rooms search considers — a room opted out of search is not suggested
// either. Tags come back with their `#` for that reason.
// //
// `searchSessionId` is the client's own correlation id for a typing session — it ties // `searchSessionId` is the client's own correlation id for a typing session — it ties
// the keystrokes and the eventual search together in the reference's analytics. Nothing // the keystrokes and the eventual search together in the reference's analytics. Nothing
@@ -1231,8 +1235,9 @@ const app = new Hono<App>()
summary: 'Search suggestions for the search box', summary: 'Search suggestions for the search box',
description: [ description: [
'Type-ahead suggestions as a bare array of strings — not rooms, not an envelope.', 'Type-ahead suggestions as a bare array of strings — not rooms, not an envelope.',
'Drawn from room names and room tags over the public, non-dorm rooms `/rooms/search`', 'Drawn from room names and room tags over the public, non-dorm, searchable rooms',
'searches, so every suggestion is one that finds something when submitted; a tag', '`/rooms/search` searches, so every suggestion is one that finds something when',
'submitted and a room opted out of search is never suggested; a tag',
'suggestion carries its `#` so it searches by tag. A `query` starting with `#`', 'suggestion carries its `#` so it searches by tag. A `query` starting with `#`',
'suggests tags only. Matches that START with the query come first, then ones that', 'suggests tags only. Matches that START with the query come first, then ones that',
'merely contain it, alphabetically within each — the same query always suggests the', 'merely contain it, alphabetically within each — the same query always suggests the',
+23 -4
View File
@@ -1204,7 +1204,7 @@ describe('rooms endpoints', () => {
IsDorm: false, IsDorm: false,
SubRooms: [], SubRooms: [],
}) })
// Same room, opted out. // Same room, opted out of the feeds.
await seed({ await seed({
RoomId: 30802, RoomId: 30802,
Name: 'OptedOut', Name: 'OptedOut',
@@ -1214,6 +1214,16 @@ describe('rooms endpoints', () => {
ExcludeFromLists: true, ExcludeFromLists: true,
SubRooms: [], SubRooms: [],
}) })
// And one opted out of SEARCH instead — the other flag, which the feeds ignore.
await seed({
RoomId: 30803,
Name: 'Unsearchable',
CreatorAccountId: 830,
Accessibility: 1,
IsDorm: false,
ExcludeFromSearch: true,
SubRooms: [],
})
const namesIn = async (path: string) => { const namesIn = async (path: string) => {
const body = (await (await SELF.fetch(`${ORIGIN}${path}`)).json()) as const body = (await (await SELF.fetch(`${ORIGIN}${path}`)).json()) as
@@ -1224,13 +1234,22 @@ describe('rooms endpoints', () => {
for (const feed of ['/rooms/hot?take=200', '/rooms/recommendations?take=200']) { for (const feed of ['/rooms/hot?take=200', '/rooms/recommendations?take=200']) {
expect(await namesIn(feed)).toContain('NoExcludeKey') expect(await namesIn(feed)).toContain('NoExcludeKey')
expect(await namesIn(feed)).not.toContain('OptedOut') expect(await namesIn(feed)).not.toContain('OptedOut')
// `ExcludeFromSearch` is search's flag, not the feeds': the room still lists.
expect(await namesIn(feed)).toContain('Unsearchable')
} }
// SEARCH is the wider filter (PUBLIC_WHERE): a room can opt out of the browse feeds // SEARCH is its own filter (SEARCHABLE_WHERE): a room can opt out of the browse feeds
// and still be findable by name, so this must not have narrowed with the feeds. // and still be findable by name, so this must not have narrowed with the feeds…
expect(await namesIn('/rooms/search?query=optedout')).toContain('OptedOut') expect(await namesIn('/rooms/search?query=optedout')).toContain('OptedOut')
// …while a room opted out of SEARCH is not found, by name or by the missing key's
// opposite — and the suggestion box, which draws from the same rooms, never offers it.
expect(await namesIn('/rooms/search?query=unsearchable')).toEqual([])
expect(await namesIn('/rooms/search?query=noexcludekey')).toContain('NoExcludeKey')
expect(
await (await SELF.fetch(`${ORIGIN}/rooms/autocomplete_search?query=unsearch&take=10`)).json()
).toEqual([])
await env.DB.prepare('DELETE FROM room WHERE room_id IN (30801, 30802)').run() await env.DB.prepare('DELETE FROM room WHERE room_id IN (30801, 30802, 30803)').run()
}) })
// Served only to the client builds that render it — the 2023 client's other room // Served only to the client builds that render it — the 2023 client's other room
+39 -16
View File
@@ -45,7 +45,10 @@ export const ROOM_SCHEMA_DDL: string[] = [
-- rest so the blob stays the only copy; they exist to be INDEXED — see -- rest so the blob stays the only copy; they exist to be INDEXED — see
-- {@link LISTABLE_WHERE}. -- {@link LISTABLE_WHERE}.
accessibility INTEGER GENERATED ALWAYS AS (json_extract(data, '$.Accessibility')) VIRTUAL, accessibility INTEGER GENERATED ALWAYS AS (json_extract(data, '$.Accessibility')) VIRTUAL,
exclude_from_lists INTEGER GENERATED ALWAYS AS (json_extract(data, '$.ExcludeFromLists')) VIRTUAL exclude_from_lists INTEGER GENERATED ALWAYS AS (json_extract(data, '$.ExcludeFromLists')) VIRTUAL,
-- The flag SEARCH filters on (migrations/0027_room_searchable.sql) — see
-- {@link SEARCHABLE_WHERE}.
exclude_from_search INTEGER GENERATED ALWAYS AS (json_extract(data, '$.ExcludeFromSearch')) VIRTUAL
)`, )`,
`CREATE UNIQUE INDEX IF NOT EXISTS idx_rooms_room_id ON room (room_id)`, `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)`, `CREATE INDEX IF NOT EXISTS idx_rooms_name_lower ON room (name_lower)`,
@@ -2069,11 +2072,23 @@ const LISTABLE_WHERE = 'is_dorm IS NOT 1 AND accessibility = 1 AND exclude_from_
/** /**
* The wider half of {@link LISTABLE_WHERE}: public and not a dorm, without the * The wider half of {@link LISTABLE_WHERE}: public and not a dorm, without the
* `ExcludeFromLists` term. What SEARCH considers — a room can opt out of the browse feeds * `ExcludeFromLists` term. What `idx_room_public` covers, and the base of
* and still be findable by name — so the two searching reads select on this instead. * {@link SEARCHABLE_WHERE}.
*/ */
const PUBLIC_WHERE = 'is_dorm IS NOT 1 AND accessibility = 1' const PUBLIC_WHERE = 'is_dorm IS NOT 1 AND accessibility = 1'
/**
* What SEARCH considers — the same test {@link isSearchable} makes in memory, pushed down
* like {@link LISTABLE_WHERE} is for the feeds. `ExcludeFromSearch` is a room's own flag,
* independent of `ExcludeFromLists`: a room can opt out of the browse feeds and still be
* findable by name, or be in every feed and unfindable by name. So the two searching reads
* ({@link searchRooms}, {@link autocompleteRoomSearch}) select on this and NOT on the
* listable predicate; `idx_room_public` still narrows them to the public rooms, and the
* search term is checked against the rows it fetched. The in-memory filter stays, for the
* same reason it does with the feeds.
*/
const SEARCHABLE_WHERE = `${PUBLIC_WHERE} AND exclude_from_search IS NOT 1`
/** /**
* The columns a scan-then-rank feed selects in place of {@link ROOM_COLUMNS}: a STUB room * The columns a scan-then-rank feed selects in place of {@link ROOM_COLUMNS}: a STUB room
* holding only the fields the feeds filter and sort on, not the whole blob. The feeds rank * holding only the fields the feeds filter and sort on, not the whole blob. The feeds rank
@@ -2473,7 +2488,7 @@ function roomsByTagsQuery(
where = '', where = '',
columns = ROOM_COLUMNS columns = ROOM_COLUMNS
): { sql: string; binds: string[] } { ): { sql: string; binds: string[] } {
// `where` is the caller's row filter ({@link LISTABLE_WHERE} or {@link PUBLIC_WHERE}) — // `where` is the caller's row filter ({@link LISTABLE_WHERE} or {@link SEARCHABLE_WHERE}) —
// unqualified, which is unambiguous under either shape below. It matters most when // unqualified, which is unambiguous under either shape below. It matters most when
// `tagSets` is EMPTY: that branch is the full scan every pseudo-tag feed still runs. // `tagSets` is EMPTY: that branch is the full scan every pseudo-tag feed still runs.
const filter = where === '' ? '' : ` WHERE ${where}` const filter = where === '' ? '' : ` WHERE ${where}`
@@ -3338,11 +3353,11 @@ function roomHasAnyTag(room: Room, tags: Set<string>): boolean {
} }
/** /**
* Search public, non-dorm rooms. The query is split into terms (space/`+`): * Search public, non-dorm rooms not opted out of search. The query is split into terms
* `#tag` terms match the room's Tags; plain terms match the room name * (space/`+`): `#tag` terms match the room's Tags; plain terms match the room name
* (substring). All terms must match. Returns a paginated `{ Results, TotalResults }`. * (substring). All terms must match. Returns a paginated `{ Results, TotalResults }`.
* The rows narrow in SQL — public, non-dorm ({@link PUBLIC_WHERE}) — and the name terms * The rows narrow in SQL — public, non-dorm, searchable ({@link SEARCHABLE_WHERE}) — and
* match in memory over what comes back. * the name terms match in memory over what comes back.
* *
* `#community` is the one tag term that isn't a tag lookup — see {@link COMMUNITY_TAG}. * `#community` is the one tag term that isn't a tag lookup — see {@link COMMUNITY_TAG}.
*/ */
@@ -3370,12 +3385,12 @@ export async function searchRooms(
const tagSets = tagTerms const tagSets = tagTerms
.filter((tag) => tag !== COMMUNITY_TAG) .filter((tag) => tag !== COMMUNITY_TAG)
.map((tag) => [tag, ...(TAG_ALIASES[tag] ?? [])]) .map((tag) => [tag, ...(TAG_ALIASES[tag] ?? [])])
const { sql, binds } = roomsByTagsQuery(tagSets, PUBLIC_WHERE) const { sql, binds } = roomsByTagsQuery(tagSets, SEARCHABLE_WHERE)
const { results } = await db const { results } = await db
.prepare(sql) .prepare(sql)
.bind(...binds) .bind(...binds)
.all<RoomRow>() .all<RoomRow>()
let rooms = parseAll(results).filter((r) => r.IsDorm !== true && r.Accessibility === 1) let rooms = parseAll(results).filter(isSearchable)
// The same test the hot feed's `community` chip applies: every room a player made, which // The same test the hot feed's `community` chip applies: every room a player made, which
// is every room the Coach account doesn't own. It narrows the other terms rather than // is every room the Coach account doesn't own. It narrows the other terms rather than
@@ -3403,8 +3418,9 @@ export async function searchRooms(
* which is the whole point of the endpoint: a suggestion that returns nothing is worse * which is the whole point of the endpoint: a suggestion that returns nothing is worse
* than no suggestion. So the candidates are drawn from the two things that search matches * than no suggestion. So the candidates are drawn from the two things that search matches
* — room NAMES for a plain term, and TAGS for a `#tag` term — over the same public, * — room NAMES for a plain term, and TAGS for a `#tag` term — over the same public,
* non-dorm rooms search itself considers. A tag comes back with its `#` so submitting the * non-dorm, searchable rooms search itself considers. A room opted out of search must not
* suggestion verbatim searches by tag rather than for a room called "horror". * leak through its name or tags as a suggestion. A tag comes back with its `#` so
* submitting the suggestion verbatim searches by tag rather than for a room called "horror".
* *
* A query starting with `#` is asking for tags, so only tags are suggested. Otherwise * A query starting with `#` is asking for tags, so only tags are suggested. Otherwise
* names come first (the likelier intent), then tags, and within each, matches that START * names come first (the likelier intent), then tags, and within each, matches that START
@@ -3425,13 +3441,11 @@ export async function autocompleteRoomSearch(
if (q === '' || take <= 0) return [] if (q === '' || take <= 0) return []
const { results } = await db const { results } = await db
.prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE ${PUBLIC_WHERE}`) .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE ${SEARCHABLE_WHERE}`)
.all<RoomRow>() .all<RoomRow>()
// Tags attached up front: suggestions are drawn from them, and this reads every candidate // Tags attached up front: suggestions are drawn from them, and this reads every candidate
// room for its NAME regardless, so the tags cost one extra query rather than a second scan. // room for its NAME regardless, so the tags cost one extra query rather than a second scan.
const rooms = (await parseAllWithTags(db, results)).filter( const rooms = (await parseAllWithTags(db, results)).filter(isSearchable)
(r) => r.IsDorm !== true && r.Accessibility === 1
)
const tagQuery = q.startsWith('#') const tagQuery = q.startsWith('#')
const term = tagQuery ? q.slice(1) : q const term = tagQuery ? q.slice(1) : q
@@ -3515,6 +3529,15 @@ function isListable(room: Room): boolean {
return room.IsDorm !== true && room.Accessibility === 1 && room.ExcludeFromLists !== true return room.IsDorm !== true && room.Accessibility === 1 && room.ExcludeFromLists !== true
} }
/**
* True when the room may come back from a SEARCH: public, not a dorm, and not opted out of
* search. The search-side twin of {@link isListable} — `ExcludeFromSearch` is its own flag,
* so neither test implies the other; see {@link SEARCHABLE_WHERE}.
*/
function isSearchable(room: Room): boolean {
return room.IsDorm !== true && room.Accessibility === 1 && room.ExcludeFromSearch !== true
}
/** /**
* True when the room is a PLAYER's rather than one of this server's stock ones — the same * True when the room is a PLAYER's rather than one of this server's stock ones — the same
* test {@link COMMUNITY_TAG} applies, since the Coach account owns every seeded room. * test {@link COMMUNITY_TAG} applies, since the Coach account owns every seeded room.