diff --git a/README.md b/README.md index 99681b3..7ada51f 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ We are building a web application that helps gymnastics coaches plan their gymna ## Status -The first delivery was a specification. There is no working application yet, but the catalog data has been extracted from the Code of Points and is in [`data/`](data/), and the database schema is in place: the Prisma schema and its migrations create all the tables of the data model in PostgreSQL. The seed script loads `data/` into the database. The GraphQL server and the search are the next steps. +The first delivery was a specification. There is no working application yet, but the catalog data has been extracted from the Code of Points and is in [`data/`](data/), and the database schema is in place: the Prisma schema and its migrations create all the tables of the data model in PostgreSQL. The seed script loads `data/` into the database, and the catalog search is written as a backend function. The GraphQL server, which will call it, is the next step. The code in `src/` is the Vite starter project with one example test. The tooling is set up: TypeScript, ESLint, Prettier and Vitest. @@ -50,7 +50,7 @@ GraphQL is required by the assignment, but it also suits this application. The f - **The data is nested.** A page often needs an apparatus with its groups and the items in them, or a gymnast with its routines and their elements. With REST that means several requests, or one endpoint for each page. With GraphQL the client asks for exactly the nested fields it needs in one request. - **Less data over the network.** A list of search results only needs a code, a description and a value, while a detail page needs more. The client chooses the fields, so we do not send what is not shown. This is part of our sustainability choices. - **One typed contract.** The schema describes every query, argument and field. We can generate TypeScript types from it, so a change in the API shows up as a type error in the client and not as a bug at runtime. -- **Search, filter, sort and pagination in one query.** `searchCatalog` takes all of them as typed arguments and runs on the server over the whole data set. Pages are fetched with cursors, which keeps every page fast whatever the size of the catalog. +- **Search, filter, sort and pagination in one query.** `searchCatalog` takes all of them as typed arguments and runs on the server over the whole data set. Pages are fetched with cursors, so a deep page does not read the earlier pages again (measured on a large catalog in a later sub-issue). The costs are that HTTP caching is harder than with REST, because all requests go to one endpoint, and that the server must guard against expensive queries. We handle the last one with an upper limit on the page size, and we avoid repeated database queries for nested fields with DataLoader. @@ -64,6 +64,7 @@ In the repository now: - A backend package in `backend/` with TypeScript and Prisma, set up as an npm workspace - PostgreSQL 18, installed directly on our own machines (not in Docker), with the Prisma schema and migrations for all the tables of the data model - A seed script that loads the catalog from `data/` into the database +- The catalog search, with Prisma only and cursor pagination. It is not yet reachable through a GraphQL server - ESLint and Prettier for linting and formatting, for both the frontend and the backend - Vitest for testing - Tailwind CSS, shadcn/ui and Lucide icons for the interface, see [UI components](#ui-components-shadcnui-tailwind-css-and-lucide) @@ -307,11 +308,19 @@ Prisma Studio uses the connection in `backend/.env`, so there is nothing more to To write your own SQL queries, connect with `psql -h localhost -U cop -d cop` (on Windows, use _SQL Shell (psql)_) and enter the password from `backend/.env`. The tables have snake_case names in the database, such as `catalog_entries` and `element_group_id`. `\dt` lists the tables, `\d elements` shows the columns of a table, and `\q` quits. +## Catalog search + +The catalog search is in `backend/src/catalog/search.ts`. It finds elements and vaults by the start of their words, with filters, sorting and cursor pagination, using Prisma only. How it works and why is described in "Search without raw SQL" in [docs/API.md](docs/API.md#search-without-raw-sql). + +The tests in `backend/src/catalog/` run with `npm test`. The search tests use your local database and need the seeded catalog, so run `npm run db:seed` first. They only read from the database. + +**Without `DATABASE_URL` in `backend/.env`, the database tests are skipped.** Vitest then reports them as skipped and the run still passes, so a passing run without a database does not mean the search was tested. Check that the output says 0 skipped. + ## Project structure | Folder | Contents | | ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `backend/` | The backend package: TypeScript, Prisma and the draft GraphQL schema | +| `backend/` | The backend package: TypeScript, Prisma, the seed script, the catalog search and the draft GraphQL schema | | `data/` | The catalog data extracted from the Code of Points: JSON files and one image per element | | `docs/` | The specification: user interface plans, data model, example data and AI usage. [docs/EXTRACTION.md](docs/EXTRACTION.md) explains how `data/` was made | | `public/` | Static files served as they are, such as the favicon | diff --git a/backend/src/catalog/cursor.ts b/backend/src/catalog/cursor.ts new file mode 100644 index 0000000..8c246dd --- /dev/null +++ b/backend/src/catalog/cursor.ts @@ -0,0 +1,76 @@ +// The cursor of the catalog search: a text that says where the next page +// starts. It is base64url-encoded JSON with the sort field and direction, the +// sort values of the last item on the page, and its id. +// +// The client should treat it as opaque, but it is not secret: the catalog is +// public, so a changed or hand-made cursor can only lead to another page of +// public data. What matters is that it is checked strictly when it is decoded, +// before any value reaches Prisma. + +import { BadUserInputError } from '../errors.js' +import { + SORTS, + type SortDirection, + type SortField, + type SortValues, + isText, +} from './sorts.js' + +/** What a cursor points at: the sort values and the id of the last item. */ +export type CursorPosition = { values: SortValues; id: string } + +const INVALID = + 'The cursor is not valid. Use the endCursor from the previous page.' + +export function encodeCursor( + field: SortField, + direction: SortDirection, + position: CursorPosition, +): string { + const payload = { field, direction, id: position.id, ...position.values } + return Buffer.from(JSON.stringify(payload), 'utf8').toString('base64url') +} + +/** + * Decodes and checks a cursor: it must be for the same sort field and + * direction as the current request, have exactly the expected keys, and every + * value must have the right type. Anything else gives BAD_USER_INPUT. + */ +export function decodeCursor( + cursor: string, + field: SortField, + direction: SortDirection, +): CursorPosition { + let payload: unknown + try { + payload = JSON.parse(Buffer.from(cursor, 'base64url').toString('utf8')) + } catch { + throw new BadUserInputError(INVALID) + } + if ( + typeof payload !== 'object' || + payload === null || + Array.isArray(payload) + ) { + throw new BadUserInputError(INVALID) + } + + const { + field: f, + direction: d, + id, + ...values + } = payload as Record + if (f !== field || d !== direction) { + throw new BadUserInputError( + `The cursor was made for another sort (${String(f)} ${String(d)}), not ${field} ${direction}. Start again without a cursor.`, + ) + } + const sort = SORTS[field] + const sameKeys = + Object.keys(values).sort().join() === [...sort.keys].sort().join() + if (!sameKeys || !isText(id) || id === '' || !sort.isValid(values)) { + throw new BadUserInputError(INVALID) + } + return { values: values as SortValues, id } +} diff --git a/backend/src/catalog/search.test.ts b/backend/src/catalog/search.test.ts new file mode 100644 index 0000000..71adfbb --- /dev/null +++ b/backend/src/catalog/search.test.ts @@ -0,0 +1,505 @@ +// Tests for the catalog search. The first part has no database. The rest runs +// against the local database from backend/.env and needs the seeded catalog +// (run `npm run db:seed` first). It only reads: no catalog rows or user data +// are changed. Without DATABASE_URL the database tests are skipped. + +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { PrismaPg } from '@prisma/adapter-pg' +import { createPrismaClient } from '../db.js' +import { PrismaClient as LoggingClient } from '../generated/prisma/client.js' +import type { PrismaClient } from '../generated/prisma/client.js' +import { + BadUserInputError, + type CatalogSearchArgs, + listElementGroups, + searchCatalog, + searchTerms, +} from './search.js' + +describe('searchTerms', () => { + it('uses the same rules as the stored words: lowercase, no accents, codes kept whole', () => { + expect(searchTerms('SAL')).toEqual(['sal']) + expect(searchTerms('Fouetté')).toEqual(['fouette']) + expect(searchTerms('3.304')).toEqual(['3.304']) + }) + + it('splits "hop-grip" into two words, like the stored words', () => { + expect(searchTerms('hop-grip')).toEqual(['hop', 'grip']) + }) + + it('drops the dot after an abbreviation such as "sec."', () => { + expect(searchTerms('sec.')).toEqual(['sec']) + }) + + it('splits "1/1" into numbers', () => { + expect(searchTerms('1/1')).toEqual(['1']) + }) + + it('removes a trailing "s", but not from codes or numbers', () => { + expect(searchTerms('saltos circles')).toEqual(['salto', 'circle']) + expect(searchTerms('3.3 180s')).toEqual(['3.3', '180s']) + }) + + it('drops a word that is only "s", and empty words', () => { + expect(searchTerms('s')).toEqual([]) + expect(searchTerms(' ')).toEqual([]) + expect(searchTerms(null)).toEqual([]) + }) + + it('removes % and _, which are wildcards in LIKE', () => { + expect(searchTerms('%')).toEqual([]) + expect(searchTerms('sal%')).toEqual(['sal']) + expect(searchTerms('_alto')).toEqual(['alto']) + }) +}) + +const hasDatabase = Boolean(process.env['DATABASE_URL']) + +describe.runIf(hasDatabase)('searchCatalog on the seeded catalog', () => { + let prisma: PrismaClient + let totalEntries: number + let rowCounts: number[] + + const counts = () => + Promise.all([ + prisma.catalogEntry.count(), + prisma.catalogWord.count(), + prisma.element.count(), + prisma.vault.count(), + prisma.coach.count(), + prisma.gymnast.count(), + prisma.routine.count(), + prisma.routineElement.count(), + prisma.gymnastElement.count(), + ]) + + /** Follows the cursors to the end and returns all ids, in order. */ + async function all(args: CatalogSearchArgs, pageSize = 50) { + const firstPage = await searchCatalog(prisma, { + ...args, + first: pageSize, + withTotalCount: true, + }) + const nodes = firstPage.edges.map((e) => e.node) + let page = firstPage + while (page.pageInfo.hasNextPage) { + page = await searchCatalog(prisma, { + ...args, + first: pageSize, + after: page.pageInfo.endCursor, + }) + nodes.push(...page.edges.map((e) => e.node)) + } + return { + nodes, + ids: nodes.map((n) => n.id), + totalCount: firstPage.totalCount!, + } + } + + const ids = async (search: string, filter?: CatalogSearchArgs['filter']) => + (await all({ search, filter }, 100)).ids + + const expectBadInput = async (args: CatalogSearchArgs, message: RegExp) => { + const error = await searchCatalog(prisma, args).catch((e: unknown) => e) + expect(error).toBeInstanceOf(BadUserInputError) + expect((error as BadUserInputError).code).toBe('BAD_USER_INPUT') + expect((error as Error).message).toMatch(message) + } + + beforeAll(async () => { + prisma = createPrismaClient() + totalEntries = await prisma.catalogEntry.count() + if (totalEntries === 0) + throw new Error('The catalog is empty. Run `npm run db:seed` first.') + rowCounts = await counts() + }) + + afterAll(async () => { + // The search only reads: nothing in the catalog or the user data changed. + expect(await counts()).toEqual(rowCounts) + await prisma.$disconnect() + }) + + it('requires every search word to match', async () => { + const giant = await ids('giant') + const circle = await ids('circle') + const both = await ids('giant circle') + expect(both.length).toBeGreaterThan(0) + // Exactly the items that each word finds on its own. + expect(both).toEqual(giant.filter((id) => circle.includes(id))) + expect(both.length).toBeLessThan(circle.length) + }) + + it('removes a trailing "s" from each search word', async () => { + expect(await ids('circles')).toEqual(await ids('circle')) + expect(await ids('Saltos')).toEqual(await ids('salto')) + }) + + it('finds a code by a word that is only in its variant -b', async () => { + // UB-2.506-a is named Hindorff; Martins is the name of variant -b. + const entry = await prisma.catalogEntry.findUniqueOrThrow({ + where: { id: 'UB-2.506-a' }, + }) + expect(`${entry.name} ${entry.description}`.toLowerCase()).not.toContain( + 'martins', + ) + expect(await ids('martins')).toContain('UB-2.506-a') + }) + + it('finds an element by a written-out abbreviation', async () => { + const entry = await prisma.catalogEntry.findUniqueOrThrow({ + where: { id: 'UB-2.305-a' }, + }) + expect(entry.description).toBe('Clear hip circle to hstd') + expect(await ids('handstand')).toContain('UB-2.305-a') + }) + + it('treats "hop-grip", "sec." and "1/1" like the stored words', async () => { + expect(await ids('hop-grip')).toEqual(await ids('hop grip')) + expect((await ids('hop-grip')).length).toBeGreaterThan(0) + expect(await ids('sec.')).toEqual(await ids('sec')) + expect((await ids('sec.')).length).toBeGreaterThan(0) + expect(await ids('1/1 turn')).toEqual(await ids('1 turn')) + }) + + it('finds codes by a code prefix', async () => { + const found = await all({ search: '3.3' }, 100) + expect(found.totalCount).toBeGreaterThan(0) + expect(found.nodes.every((n) => n.code.startsWith('3.3'))).toBe(true) + }) + + it('is not fooled by % and _, and ignores case', async () => { + expect(await ids('SAL')).toEqual(await ids('sal')) + expect(await ids('sal%')).toEqual(await ids('sal')) + // As a LIKE pattern, "_alto%" would match "salto". Here it is just "alto". + expect(await ids('_alto')).toEqual(await ids('alto')) + expect(await ids('_alto')).not.toEqual(await ids('salto')) + // "%" alone is cleaned away and leaves no words, so it finds nothing. + expect((await all({ search: '%' }, 100)).totalCount).toBe(0) + }) + + it('finds nothing for text without searchable words, and everything for empty text', async () => { + for (const search of ['ø', 'ß', '日本語', '😀', '%_', 's']) { + const page = await searchCatalog(prisma, { search, withTotalCount: true }) + expect(page.totalCount).toBe(0) + expect(page.edges).toEqual([]) + expect(page.pageInfo).toEqual({ hasNextPage: false, endCursor: null }) + } + for (const search of [null, '', ' ']) { + const page = await searchCatalog(prisma, { search, withTotalCount: true }) + expect(page.totalCount).toBe(totalEntries) + } + }) + + it('filters by difficulty values, only elements', async () => { + const found = await all({ filter: { difficultyValues: [0.1, 0.2] } }, 100) + expect(found.nodes.every((n) => n.type === 'ELEMENT')).toBe(true) + expect( + found.nodes.every((n) => + ['0.1', '0.2'].includes(n.difficulty.toFixed(1)), + ), + ).toBe(true) + expect(found.totalCount).toBe( + await prisma.catalogEntry.count({ + where: { type: 'ELEMENT', difficulty: { in: ['0.1', '0.2'] } }, + }), + ) + // A float such as 0.30000000000000004 is rounded to 0.3. + const point3 = await searchCatalog(prisma, { + filter: { difficultyValues: [0.1 + 0.2] }, + withTotalCount: true, + }) + expect(point3.totalCount).toBe( + await prisma.catalogEntry.count({ + where: { type: 'ELEMENT', difficulty: '0.3' }, + }), + ) + }) + + it('rejects difficulty values that are not 0.1–1.0 in steps of 0.1, and vault', async () => { + for (const value of [0, 0.15, 1.1, -0.1, Number.NaN]) { + await expectBadInput( + { filter: { difficultyValues: [value] } }, + /Difficulty value .* is not valid/, + ) + } + await expectBadInput( + { filter: { apparatusId: 'VT', difficultyValues: [0.3] } }, + /cannot be used with vault/, + ) + // A vault group without an apparatus filter is vault too, not 0 hits. + await expectBadInput( + { filter: { elementGroupId: 'VT-1', difficultyValues: [0.3] } }, + /cannot be used with vault/, + ) + }) + + it('sorts by difficulty only within an apparatus', async () => { + await expectBadInput( + { sort: { field: 'DIFFICULTY' } }, + /needs an apparatus filter/, + ) + for (const direction of ['ASC', 'DESC'] as const) { + const found = await all( + { + filter: { apparatusId: 'BB' }, + sort: { field: 'DIFFICULTY', direction }, + }, + 25, + ) + const values = found.nodes.map((n) => Number(n.difficulty)) + const sorted = [...values].sort((a, b) => + direction === 'ASC' ? a - b : b - a, + ) + expect(values).toEqual(sorted) + expect(found.nodes.every((n) => n.apparatusId === 'BB')).toBe(true) + } + }) + + it('follows the cursors to the end without skipping or repeating items', async () => { + const cases: [CatalogSearchArgs, number][] = [ + [{ sort: { field: 'CODE', direction: 'ASC' } }, 37], + [{ sort: { field: 'CODE', direction: 'DESC' } }, 50], + // Most items have no name, so nameSort is "" for most rows. + [{ sort: { field: 'NAME', direction: 'ASC' } }, 50], + [{ sort: { field: 'NAME', direction: 'DESC' } }, 23], + // Many items share a value, so id decides the order. + [ + { + filter: { apparatusId: 'UB' }, + sort: { field: 'DIFFICULTY', direction: 'ASC' }, + }, + 20, + ], + [{ search: 'salto', sort: { field: 'NAME', direction: 'ASC' } }, 7], + ] + for (const [args, pageSize] of cases) { + const found = await all(args, pageSize) + const expected = await searchCatalog(prisma, { + ...args, + first: 1, + withTotalCount: true, + }) + expect(new Set(found.ids).size).toBe(found.ids.length) // nothing repeated + expect(found.ids.length).toBe(expected.totalCount) // nothing skipped + } + const all1 = await all({}, 100) + expect(all1.totalCount).toBe(totalEntries) + }) + + it('puts named items first in both directions of the name sort', async () => { + for (const direction of ['ASC', 'DESC'] as const) { + const { nodes } = await all({ sort: { field: 'NAME', direction } }, 100) + const firstUnnamed = nodes.findIndex((n) => !n.hasName) + expect(firstUnnamed).toBeGreaterThan(0) + expect(nodes.slice(firstUnnamed).every((n) => !n.hasName)).toBe(true) + } + }) + + it('filters over the whole catalog', async () => { + const bb = await all({ filter: { apparatusId: 'BB' } }, 100) + expect(bb.totalCount).toBe( + await prisma.catalogEntry.count({ where: { apparatusId: 'BB' } }), + ) + expect(bb.ids.length).toBe(bb.totalCount) + + const group = await all( + { filter: { apparatusId: 'UB', elementGroupId: 'UB-3' } }, + 100, + ) + expect(group.totalCount).toBe( + await prisma.catalogEntry.count({ where: { elementGroupId: 'UB-3' } }), + ) + + const named = await all({ filter: { namedOnly: true } }, 100) + expect(named.totalCount).toBe( + await prisma.catalogEntry.count({ where: { hasName: true } }), + ) + expect(named.nodes.every((n) => n.name !== null)).toBe(true) + }) + + it('only accepts groups from the chosen apparatus, and lists groups per apparatus', async () => { + await expectBadInput( + { filter: { apparatusId: 'BB', elementGroupId: 'UB-3' } }, + /belongs to UB/, + ) + await expectBadInput( + { filter: { elementGroupId: 'UB-99' } }, + /Unknown element group/, + ) + const groups = await listElementGroups(prisma, 'UB') + expect(groups.map((g) => g.number)).toEqual([1, 2, 3, 4, 5, 6]) + expect(groups.every((g) => g.apparatusId === 'UB')).toBe(true) + await expect(listElementGroups(prisma, 'XX')).rejects.toBeInstanceOf( + BadUserInputError, + ) + }) + + it('treats SQL in the search text, the filters and the cursor as plain values', async () => { + const found = await searchCatalog(prisma, { + search: "'; DROP TABLE catalog_entries; --", + withTotalCount: true, + }) + expect(found.totalCount).toBe(0) + expect(await prisma.catalogEntry.count()).toBe(totalEntries) + await expectBadInput( + { filter: { apparatusId: "UB' OR '1'='1" } }, + /Unknown apparatus/, + ) + await expectBadInput({ after: "' OR 1=1 --" }, /cursor is not valid/) + }) + + it('rejects a cursor made for another sort, or with the wrong shape', async () => { + const page = await searchCatalog(prisma, { + first: 2, + sort: { field: 'CODE' }, + }) + const cursor = page.pageInfo.endCursor! + await expectBadInput( + { after: cursor, sort: { field: 'NAME' } }, + /made for another sort/, + ) + await expectBadInput( + { after: cursor, sort: { field: 'CODE', direction: 'DESC' } }, + /made for another sort/, + ) + + const encode = (value: unknown) => + Buffer.from(JSON.stringify(value)).toString('base64url') + const code = { field: 'CODE', direction: 'ASC', codeSort: '0001.0101' } + const wrong = [ + 'not base64 json', + encode('a string'), + encode([1, 2]), + encode(code), // no id + encode({ ...code, id: '' }), + encode({ ...code, id: 42 }), + encode({ ...code, id: 'UB-1.101-a', codeSort: 7 }), + encode({ ...code, id: 'UB-1.101-a', extra: 'x' }), + ] + for (const after of wrong) { + await expectBadInput( + { after, sort: { field: 'CODE' } }, + /cursor is not valid/, + ) + } + await expectBadInput( + { + after: encode({ + field: 'DIFFICULTY', + direction: 'ASC', + difficulty: '0.15', + id: 'x', + }), + filter: { apparatusId: 'UB' }, + sort: { field: 'DIFFICULTY' }, + }, + /cursor is not valid/, + ) + }) + + it('accepts a hand-made cursor with the right shape, which only leads to public data', async () => { + const cursor = Buffer.from( + JSON.stringify({ + field: 'CODE', + direction: 'ASC', + codeSort: '0003.0000', + id: 'x', + }), + ).toString('base64url') + const page = await searchCatalog(prisma, { + first: 3, + after: cursor, + sort: { field: 'CODE' }, + }) + expect(page.edges.every((e) => e.node.codeSort > '0003.0000')).toBe(true) + }) + + it('limits the search to 80 characters and 8 words', async () => { + await expectBadInput({ search: 'a'.repeat(81) }, /at most 80 characters/) + await expectBadInput( + { search: 'one two three four five six seven eight nine' }, + /at most 8 words, not 9/, + ) + // Exactly at the limits is fine, and repeated words count once. + await searchCatalog(prisma, { search: 'a'.repeat(80) }) + await searchCatalog(prisma, { + search: 'salto salto bwd fwd tucked piked stretched twist turn', + }) + }) + + it('checks first and the sort', async () => { + for (const first of [0, 101, 1.5]) + await expectBadInput({ first }, /first must be a whole number/) + await expectBadInput( + { sort: { field: 'POPULARITY' as never } }, + /Unknown sort field/, + ) + }) + + it('checks the apparatus and group filters without a database call', async () => { + // Records every use of the apparatus and element group tables. + const used: string[] = [] + const counting = new Proxy(prisma, { + get(target, key, receiver) { + if (key === 'apparatus' || key === 'elementGroup') + used.push(String(key)) + return Reflect.get(target, key, receiver) + }, + }) + // The first search reads the apparatus and groups once, into the cache. + await searchCatalog(counting, { filter: { apparatusId: 'UB' } }) + expect(used.length).toBeGreaterThan(0) + used.length = 0 + // After that, checking the filters and listing groups use only the cache. + await searchCatalog(counting, { + filter: { apparatusId: 'UB', elementGroupId: 'UB-3' }, + }) + await listElementGroups(counting, 'BB') + expect(used).toEqual([]) + }) + + it('continues after a cursor without OR, so the id is an index bound', async () => { + // With "value > v OR (value = v AND id > x)" the database can only use + // the id as a filter, and reads every row with the same value again on + // every page. Each query must be a plain condition instead. + const logging = new LoggingClient({ + adapter: new PrismaPg({ connectionString: process.env['DATABASE_URL']! }), + log: [{ emit: 'event', level: 'query' }], + }) + const sql: string[] = [] + logging.$on('query', (e) => sql.push(e.query)) + try { + for (const args of [ + { sort: { field: 'NAME' as const } }, + { + filter: { apparatusId: 'UB' }, + sort: { field: 'DIFFICULTY' as const }, + }, + { sort: { field: 'CODE' as const, direction: 'DESC' as const } }, + ]) { + const page1 = await searchCatalog(logging, { ...args, first: 30 }) + sql.length = 0 + await searchCatalog(logging, { + ...args, + first: 30, + after: page1.pageInfo.endCursor, + }) + const pageQueries = sql.filter((q) => q.includes('catalog_entries')) + expect(pageQueries.length).toBeGreaterThan(0) + expect(pageQueries.some((q) => /\bOR\b/.test(q))).toBe(false) + expect(pageQueries.some((q) => /\bOFFSET\b\s+[1-9]/.test(q))).toBe( + false, + ) + } + } finally { + await logging.$disconnect() + } + }) + + it('only counts when asked to', async () => { + const page = await searchCatalog(prisma, { first: 1 }) + expect(page.totalCount).toBeUndefined() + }) +}) diff --git a/backend/src/catalog/search.ts b/backend/src/catalog/search.ts new file mode 100644 index 0000000..6115d75 --- /dev/null +++ b/backend/src/catalog/search.ts @@ -0,0 +1,257 @@ +// The catalog search behind `searchCatalog` in backend/src/schema.graphql. +// The design is "Search without raw SQL" in docs/API.md: Prisma only, a prefix +// match on CatalogWord.word, and keyset pagination with an opaque cursor. + +import { + Prisma, + type CatalogEntry, + type PrismaClient, +} from '../generated/prisma/client.js' +import { BadUserInputError } from '../errors.js' +import { decodeCursor, encodeCursor } from './cursor.js' +import { + SORTS, + SORT_FIELDS, + type SortDirection, + type SortField, +} from './sorts.js' +import { catalogStructure } from './structure.js' +import { normalizeWords } from './words.js' + +export { BadUserInputError } from '../errors.js' +export type { SortDirection, SortField } from './sorts.js' + +export type CatalogFilter = { + apparatusId?: string | null + elementGroupId?: string | null + /** Difficulty values 0.1–1.0 in steps of 0.1. Only elements, never vaults. */ + difficultyValues?: number[] | null + namedOnly?: boolean | null +} + +export type CatalogSearchArgs = { + search?: string | null + filter?: CatalogFilter | null + sort?: { field?: SortField | null; direction?: SortDirection | null } | null + /** Page size, 1–100. */ + first?: number | null + after?: string | null + /** Count all matches. Only set it when the client asks for totalCount. */ + withTotalCount?: boolean +} + +export type CatalogSearchResult = { + edges: { cursor: string; node: CatalogEntry }[] + pageInfo: { hasNextPage: boolean; endCursor: string | null } + /** Only set when withTotalCount was true. */ + totalCount?: number +} + +const DIRECTIONS: readonly SortDirection[] = ['ASC', 'DESC'] +const MAX_PAGE_SIZE = 100 +/** Limits for the search text. Far more than anyone types, and each word adds a condition to the query. */ +export const MAX_SEARCH_LENGTH = 80 +export const MAX_SEARCH_WORDS = 8 +const DEFAULT_PAGE_SIZE = 20 + +/** + * The words to search for. The same rules as the stored words + * (normalizeWords), and a trailing "s" is removed, so "saltos" finds "salto". + * A word that is only "s" is dropped. Codes and numbers (words that start + * with a digit) are left exactly as they are. + */ +export function searchTerms(text: string | null | undefined): string[] { + const terms = new Set() + for (const word of normalizeWords(text ?? '')) { + const term = /^\d/.test(word) ? word : word.replace(/s$/, '') + if (term) terms.add(term) + } + return [...terms] +} + +/** Checks a difficulty value and returns it as a Decimal with one decimal. */ +function difficultyValue(value: unknown): Prisma.Decimal { + const tenths = typeof value === 'number' ? Math.round(value * 10) : NaN + if ( + !Number.isFinite(tenths) || + Math.abs((value as number) * 10 - tenths) > 1e-6 || + tenths < 1 || + tenths > 10 + ) { + throw new BadUserInputError( + `Difficulty value ${String(value)} is not valid. Use 0.1–1.0 in steps of 0.1.`, + ) + } + return new Prisma.Decimal(tenths).dividedBy(10) +} + +/** Checks the arguments and builds the Prisma where for the filters and the text. */ +async function buildWhere( + prisma: PrismaClient, + args: CatalogSearchArgs, + field: SortField, +): Promise { + const filter = args.filter ?? {} + const and: Prisma.CatalogEntryWhereInput[] = [] + let groupApparatusId: string | null = null + // From memory, so checking the filters costs no database call. + const structure = await catalogStructure(prisma) + + if (filter.apparatusId != null) { + if (!structure.apparatusIds.has(filter.apparatusId)) + throw new BadUserInputError(`Unknown apparatus "${filter.apparatusId}".`) + and.push({ apparatusId: filter.apparatusId }) + } + if (filter.elementGroupId != null) { + const group = structure.groupsById.get(filter.elementGroupId) + if (!group) + throw new BadUserInputError( + `Unknown element group "${filter.elementGroupId}".`, + ) + if ( + filter.apparatusId != null && + group.apparatusId !== filter.apparatusId + ) { + throw new BadUserInputError( + `Element group "${group.id}" belongs to ${group.apparatusId}, not to the chosen apparatus ${filter.apparatusId}.`, + ) + } + and.push({ elementGroupId: filter.elementGroupId }) + groupApparatusId = group.apparatusId + } + if (filter.difficultyValues != null && filter.difficultyValues.length > 0) { + // A vault group without an apparatus filter is vault too. + if (filter.apparatusId === 'VT' || groupApparatusId === 'VT') { + throw new BadUserInputError( + 'The difficulty filter cannot be used with vault. Vaults have D-values, not difficulty values.', + ) + } + const values = filter.difficultyValues.map(difficultyValue) + and.push({ type: 'ELEMENT', difficulty: { in: values } }) + } + if (filter.namedOnly === true) and.push({ hasName: true }) + + if (field === 'DIFFICULTY' && filter.apparatusId == null) { + throw new BadUserInputError( + 'Sorting by difficulty needs an apparatus filter. Element values and vault D-values cannot be compared.', + ) + } + + // Every search word must match a word of the entry. Never mode: 'insensitive': + // it becomes ILIKE, which cannot use the index on word. + for (const term of searchTerms(args.search)) { + and.push({ words: { some: { word: { startsWith: term } } } }) + } + return { AND: and } +} + +/** + * Searches, filters, sorts and pages the catalog. Throws BadUserInputError + * for input the client must fix. Returns CatalogEntry rows. The fields that + * only exist on elements or vaults are loaded separately by the resolver. + */ +export async function searchCatalog( + prisma: PrismaClient, + args: CatalogSearchArgs, +): Promise { + const field = args.sort?.field ?? 'CODE' + const direction = args.sort?.direction ?? 'ASC' + if (!SORT_FIELDS.includes(field)) { + throw new BadUserInputError( + `Unknown sort field "${String(field)}". Use ${SORT_FIELDS.join(', ')}.`, + ) + } + if (!DIRECTIONS.includes(direction)) { + throw new BadUserInputError( + `Unknown sort direction "${String(direction)}". Use ASC or DESC.`, + ) + } + const first = args.first ?? DEFAULT_PAGE_SIZE + if (!Number.isInteger(first) || first < 1 || first > MAX_PAGE_SIZE) { + throw new BadUserInputError( + `first must be a whole number from 1 to ${MAX_PAGE_SIZE}, not ${String(first)}.`, + ) + } + + const text = args.search?.trim() ?? '' + if (text.length > MAX_SEARCH_LENGTH) { + throw new BadUserInputError( + `The search can have at most ${MAX_SEARCH_LENGTH} characters, not ${text.length}.`, + ) + } + const termCount = searchTerms(text).length + if (termCount > MAX_SEARCH_WORDS) { + throw new BadUserInputError( + `The search can have at most ${MAX_SEARCH_WORDS} words, not ${termCount}.`, + ) + } + + const sort = SORTS[field] + const where = await buildWhere(prisma, args, field) + const cursor = args.after ? decodeCursor(args.after, field, direction) : null + + // Text that contains no searchable words (such as "ø", "😀" or "%") finds + // nothing. Only an empty text means no text filter. + if (args.search?.trim() && searchTerms(args.search).length === 0) { + return { + edges: [], + pageInfo: { hasNextPage: false, endCursor: null }, + ...(args.withTotalCount ? { totalCount: 0 } : {}), + } + } + const orderBy = sort.orderBy(direction === 'ASC' ? 'asc' : 'desc') + // Keyset pagination: the first page is one query. After a cursor, the parts + // that follow it are read in order until the page is full, so every query + // starts in the index exactly after the cursor. One extra row tells whether + // there is a next page. + const parts = cursor + ? sort.afterParts( + cursor.values, + cursor.id, + direction === 'ASC' ? 'gt' : 'lt', + ) + : [{}] + const rows: CatalogEntry[] = [] + for (const part of parts) { + if (rows.length > first) break + rows.push( + ...(await prisma.catalogEntry.findMany({ + where: { AND: [where, part] }, + orderBy, + take: first + 1 - rows.length, + })), + ) + } + const page = rows.slice(0, first) + const edges = page.map((node) => ({ + cursor: encodeCursor(field, direction, { + values: sort.values(node), + id: node.id, + }), + node, + })) + const result: CatalogSearchResult = { + edges, + pageInfo: { + hasNextPage: rows.length > first, + endCursor: edges.at(-1)?.cursor ?? null, + }, + } + // Counting reads every match, so it only runs when the client asks for it. + if (args.withTotalCount) + result.totalCount = await prisma.catalogEntry.count({ where }) + return result +} + +/** The element groups of one apparatus, in the order of the Code of Points. */ +export async function listElementGroups( + prisma: PrismaClient, + apparatusId: string, +) { + const structure = await catalogStructure(prisma) + if (!structure.apparatusIds.has(apparatusId)) + throw new BadUserInputError(`Unknown apparatus "${apparatusId}".`) + return [...structure.groupsById.values()] + .filter((g) => g.apparatusId === apparatusId) + .sort((a, b) => a.number - b.number) +} diff --git a/backend/src/catalog/sorts.ts b/backend/src/catalog/sorts.ts new file mode 100644 index 0000000..892051c --- /dev/null +++ b/backend/src/catalog/sorts.ts @@ -0,0 +1,91 @@ +// The sorts of the catalog search, one entry per sort. Everything a sort needs +// is here: the order, the values a cursor stores, how those values are +// checked when a cursor is decoded, and the condition for the rows after a +// cursor (keyset pagination). To add a sort, add an entry here and to +// CatalogSortField in backend/src/schema.graphql. + +import { Prisma, type CatalogEntry } from '../generated/prisma/client.js' + +export type SortField = 'CODE' | 'DIFFICULTY' | 'NAME' +export type SortDirection = 'ASC' | 'DESC' + +/** The sort values of one row, as stored in a cursor (without the id). */ +export type SortValues = Record + +type Where = Prisma.CatalogEntryWhereInput +type OrderBy = Prisma.CatalogEntryOrderByWithRelationInput[] + +/** "gt" for ascending and "lt" for descending. */ +export type After = 'gt' | 'lt' + +type Sort = { + /** The keys of the sort values in a cursor. */ + keys: string[] + /** The sort values of a row, for its cursor. */ + values(entry: CatalogEntry): SortValues + /** Whether sort values read from a cursor have the right types. */ + isValid(values: Record): boolean + /** The order of the rows. id is always last, so the order is stable. */ + orderBy(direction: 'asc' | 'desc'): OrderBy + /** + * The rows that come after the row with these values and this id, as parts + * in sort order. Each part is a plain condition the index can start from: + * first the rows with the same sort value and a greater id, then the rows + * with a greater sort value. One condition with OR would leave the id as a + * filter, so the database would read every row with the same value again + * on every page (see "Keyset pagination" in docs/API.md). + */ + afterParts(values: SortValues, id: string, after: After): Where[] +} + +export const isText = (v: unknown): v is string => + typeof v === 'string' && v.length <= 200 + +export const SORTS: Record = { + CODE: { + keys: ['codeSort'], + values: (entry) => ({ codeSort: entry.codeSort }), + isValid: (v) => isText(v['codeSort']), + orderBy: (dir) => [{ codeSort: dir }, { id: dir }], + afterParts: (v, id, after) => [ + { codeSort: v['codeSort'] as string, id: { [after]: id } }, + { codeSort: { [after]: v['codeSort'] } }, + ], + }, + + DIFFICULTY: { + keys: ['difficulty'], + values: (entry) => ({ difficulty: entry.difficulty.toFixed(1) }), + isValid: (v) => + isText(v['difficulty']) && /^\d+(\.\d)?$/.test(v['difficulty']), + orderBy: (dir) => [{ difficulty: dir }, { id: dir }], + afterParts: (v, id, after) => { + const d = new Prisma.Decimal(v['difficulty'] as string) + return [ + { difficulty: d, id: { [after]: id } }, + { difficulty: { [after]: d } }, + ] + }, + }, + + // Named items first in both directions, so hasName is always descending. + NAME: { + keys: ['hasName', 'nameSort'], + values: (entry) => ({ hasName: entry.hasName, nameSort: entry.nameSort }), + isValid: (v) => typeof v['hasName'] === 'boolean' && isText(v['nameSort']), + orderBy: (dir) => [{ hasName: 'desc' }, { nameSort: dir }, { id: dir }], + afterParts: (v, id, after) => { + const hasName = v['hasName'] as boolean + const nameSort = v['nameSort'] as string + const parts: Where[] = [ + { hasName, nameSort, id: { [after]: id } }, + { hasName, nameSort: { [after]: nameSort } }, + ] + // After the named items come the unnamed ones. + if (hasName) parts.push({ hasName: false }) + return parts + }, + }, +} + +export const SORT_FIELDS = Object.keys(SORTS) as SortField[] diff --git a/backend/src/catalog/structure.ts b/backend/src/catalog/structure.ts new file mode 100644 index 0000000..8937dee --- /dev/null +++ b/backend/src/catalog/structure.ts @@ -0,0 +1,51 @@ +// The apparatus and element groups, kept in memory. The search checks the +// apparatus and group filters against them on every request, and the catalog +// is read-only, so reading them from the database each time is wasted work. +// +// The list is read again after CACHE_TTL_MS, so a new seed shows up without a +// restart. Apparatus and groups only change with a new Code of Points. + +import type { ElementGroup, PrismaClient } from '../generated/prisma/client.js' + +export const CACHE_TTL_MS = 10 * 60 * 1000 + +export type CatalogStructure = { + apparatusIds: ReadonlySet + groupsById: ReadonlyMap +} + +const cache = new WeakMap< + PrismaClient, + { loadedAt: number; structure: Promise } +>() + +async function load(prisma: PrismaClient): Promise { + const [apparatus, groups] = await Promise.all([ + prisma.apparatus.findMany({ select: { id: true } }), + prisma.elementGroup.findMany({ orderBy: { number: 'asc' } }), + ]) + return { + apparatusIds: new Set(apparatus.map((a) => a.id)), + groupsById: new Map(groups.map((g) => [g.id, g])), + } +} + +/** The apparatus and groups, from memory when they were read recently. */ +export function catalogStructure( + prisma: PrismaClient, +): Promise { + const cached = cache.get(prisma) + if (cached && Date.now() - cached.loadedAt < CACHE_TTL_MS) { + return cached.structure + } + const structure = load(prisma) + cache.set(prisma, { loadedAt: Date.now(), structure }) + // Do not keep a failed read: try again on the next request. + structure.catch(() => cache.delete(prisma)) + return structure +} + +/** Forgets the cached apparatus and groups, for example after a seed in a test. */ +export function clearCatalogStructure(prisma: PrismaClient): void { + cache.delete(prisma) +} diff --git a/backend/src/db.ts b/backend/src/db.ts index 308cfb3..1ba7778 100644 --- a/backend/src/db.ts +++ b/backend/src/db.ts @@ -1,7 +1,15 @@ -import 'dotenv/config' +import { config } from 'dotenv' +import { fileURLToPath } from 'node:url' import { PrismaPg } from '@prisma/adapter-pg' import { PrismaClient } from './generated/prisma/client.js' +// backend/.env, also when the code runs from the repository root (for example +// in the tests). +config({ + path: fileURLToPath(new URL('../.env', import.meta.url)), + quiet: true, +}) + /** A Prisma client connected to DATABASE_URL (from backend/.env). */ export function createPrismaClient(): PrismaClient { const connectionString = process.env['DATABASE_URL'] diff --git a/backend/src/errors.ts b/backend/src/errors.ts new file mode 100644 index 0000000..b4c18af --- /dev/null +++ b/backend/src/errors.ts @@ -0,0 +1,13 @@ +/** + * Wrong input from the client. The GraphQL layer turns it into an error with + * `extensions.code` "BAD_USER_INPUT" and this message (see "Error handling" + * in docs/API.md). + */ +export class BadUserInputError extends Error { + readonly code = 'BAD_USER_INPUT' + + constructor(message: string) { + super(message) + this.name = 'BadUserInputError' + } +} diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index d6d2894..49e8d28 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -161,3 +161,26 @@ checked in a build that a used class ends up in the CSS and an unused class does not, and checked the browser support and the cascade layers in the Tailwind documentation. Claude checked lint, build and tests. + +## Catalog search (`backend/src/catalog/search.ts`, `eslint.config.js`, `docs/API.md`) + +- **Tool:** Claude Code +- **What the team did:** Wrote the task from the issue and the search design + in `docs/API.md`, with the rules: Prisma only, the same word rules as the + seed, never `mode: 'insensitive'`, keyset pagination, and tests against the + seeded catalog. When Claude found that the seed splits words differently + from what the issue said, the team decided to keep the seed's rules for the + search too, and asked for tests that lock this in. Decided to use a plain + base64url cursor instead of an encrypted one, since the catalog is public. +- **What Claude did:** Wrote the search function, the cursor, the group list, + the lint rule against raw SQL and the tests, and updated the README and + `docs/API.md`. Ran the example searches, followed the cursors through whole + result lists, and checked the SQL Prisma sends (prefix `LIKE`, no `ILIKE`, + keyset condition). The tests found a bug in the check of difficulty values, + which Claude fixed. +- **After the review:** The team chose the answers to each review point: limits + of 80 characters and 8 words, an empty result for text without words, a + prefix match for numbers, a cache for apparatus and groups, and splitting the + keyset condition into separate queries. They also moved the variants hint to + its own issue. Claude made the changes, checked the query plans with + `EXPLAIN ANALYZE`, added tests for each point and updated the docs. diff --git a/docs/API.md b/docs/API.md index 807dbc3..3013d85 100644 --- a/docs/API.md +++ b/docs/API.md @@ -94,8 +94,10 @@ query { } ``` -- **Search** is one or more words and is not case sensitive. It finds items that have, for every search word, a word that starts with it. An empty search means no text filter. The client should wait until the user stops typing (debounce) before it sends the query. -- **Filter** by apparatus, group, difficulty values, and whether the item is named after a gymnast. The difficulty filter is a multiselect of values, for example `[0.4, 0.5]`. The interface shows the values as letters, so choosing D and E sends `[0.4, 0.5]`. Each value must be 0.1–1.0 in steps of 0.1. The filter only applies to elements, because vaults have D-values and not difficulty values, so vaults are left out when it is set. Together with vault it gives `BAD_USER_INPUT`. +- **Search** is one or more words and is not case sensitive. It finds items that have, for every search word, a word that starts with it. An empty search means no text filter, while a search with no searchable words (such as `%` or `ø`) finds nothing. At most 80 characters and 8 words, otherwise `BAD_USER_INPUT` (see Search without raw SQL). The client should wait until the user stops typing (debounce) before it sends the query. +- **Filter** by apparatus, group, difficulty values, and whether the item is named after a gymnast. The difficulty filter is a multiselect of values, for example `[0.4, 0.5]`. The interface shows the values as letters, so choosing D and E sends `[0.4, 0.5]`. Each value must be 0.1–1.0 in steps of 0.1. The backend rounds each value to one decimal and compares it as a `Decimal`, because a GraphQL `Float` is a binary floating-point number: 0.1 + 0.2 arrives as 0.30000000000000004, which would otherwise match nothing. The filter only applies to elements, because vaults have D-values and not difficulty values, so vaults are left out when it is set. Together with vault it gives `BAD_USER_INPUT`, also when vault is given only through a vault group (`VT-1`) and not as the apparatus. +- **The apparatus and groups are checked from memory.** The search checks the apparatus and group filters against a list that is read from the database once and kept for ten minutes (`backend/src/catalog/structure.ts`), so a search page needs no extra database calls for it. The catalog is read-only, and the apparatus and groups only change with a new Code of Points. `listElementGroups` uses the same list. +- **Unknown values in the filter are errors, not empty results.** An apparatus or element group that does not exist gives `BAD_USER_INPUT`, and so does a group from another apparatus than the chosen one. An empty list would hide a typo or an old id in the client, so the client is told what is wrong instead. The element group filter can be used without an apparatus, because a group belongs to exactly one apparatus. - **Sort** by code, difficulty or name, ascending or descending. `DIFFICULTY` means `value` for elements and `dValue` for vaults, and is only sorted within one apparatus: it is only allowed together with an `apparatusId` filter, and gives `BAD_USER_INPUT` without it. Element values (0.1–1.0) and vault D-values (about 2.0–6.4) are different scales, so one list sorted by both would always put the vaults at one end. - **Pagination** uses cursors (the Relay connection pattern: `edges`, `node`, `cursor`, `pageInfo`). `totalCount` is the number of matches in the whole result set. @@ -131,7 +133,7 @@ The requirement is that the solution works with any amount of data. The catalog ### The catalog - **Every list is paged with cursors**, and the page size has an upper limit of 100 in the schema documentation (`first`: 1–100). A client cannot ask for the whole catalog in one request. -- **A page costs the same on page 1 and page 10 000.** The cursor holds the sort values and the `id` of the last row, and the database continues after it with an index. Offset pagination would read and throw away all earlier rows. We check this with `EXPLAIN ANALYZE` (see Search without raw SQL). +- **A deep page must not cost more than the first page.** The cursor holds the sort values and the `id` of the last row, and the database continues after it with an index. Offset pagination would read and throw away all earlier rows. After a cursor the search runs up to three small queries, each starting in the index right after the cursor (see Keyset pagination under Search without raw SQL). We checked the plans with `EXPLAIN ANALYZE` on the real catalog. Measuring this on a large catalog is sub-issue 5. - **Sorting must be stable.** Many items have the same difficulty, so `id` must always be the last sort key. Without it rows can be skipped or repeated between two pages. - **Filter and sort need indexes.** `schema.prisma` declares an index for each sort field combined with `id` (`codeSort`, `difficulty`, and `hasName` with `nameSort`), and for the filter columns (apparatus, group). Without them the database sorts the whole catalog for every request. The name sort puts named items first in both directions (`hasName` descending), so ascending and descending each have their own index. The apparatus filter is often combined with a sort, so there are also indexes on apparatus with `difficulty` and with `codeSort`. The difficulty sort always has an apparatus filter, so it uses the index on apparatus and `difficulty`. The difficulty filter without an apparatus uses the index on `difficulty`. Without them, a filtered page walks past the rows of the other apparatus first, which is slow in one direction (for example vault sorted by ascending difficulty). - **Sort by name needs a rule for items without a name.** Most items have no name. A cursor does not accept `nulls` placement, so the sort uses a plain `hasName` field and a `nameSort` field that is never `null` (see Search without raw SQL). @@ -181,14 +183,14 @@ The catalog, `searchCatalog` and `calculateDScore` do not need a login. Everythi Errors are thrown from the resolvers as GraphQL errors with a machine-readable code in `extensions.code`. The client reacts to the code and never to the text of the message. -| Code | When | -| ----------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `UNAUTHENTICATED` | The operation needs a logged-in coach. | -| `NOT_FOUND` | The id does not exist, or belongs to another coach. | -| `NAME_TAKEN` | The gymnast already has a routine with this name on this apparatus (constraint 6). | -| `INVALID_ITEMS` | The items do not fit the routine: a vault outside vault, an element on vault, more than one vault, or an item from another apparatus (constraints 7 and 8). | -| `BAD_USER_INPUT` | Empty name, malformed id, `first` out of range, a `DIFFICULTY` sort without an apparatus filter, a difficulty filter together with vault, or a difficulty value that is not 0.1–1.0 in steps of 0.1. | -| `LIMIT_REACHED` | The coach has reached a limit: 200 gymnasts, 50 routines per gymnast, or 50 items in a routine. See [USER_LIST_PAGINATION.md](USER_LIST_PAGINATION.md). | +| Code | When | +| ----------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `UNAUTHENTICATED` | The operation needs a logged-in coach. | +| `NOT_FOUND` | The id does not exist, or belongs to another coach. | +| `NAME_TAKEN` | The gymnast already has a routine with this name on this apparatus (constraint 6). | +| `INVALID_ITEMS` | The items do not fit the routine: a vault outside vault, an element on vault, more than one vault, or an item from another apparatus (constraints 7 and 8). | +| `BAD_USER_INPUT` | Empty name, malformed id, `first` out of range, a search text over 80 characters or 8 words, an unknown apparatus or element group in the filter, an element group from another apparatus, a `DIFFICULTY` sort without an apparatus filter, a difficulty filter together with vault, a difficulty value that is not 0.1–1.0 in steps of 0.1, or a cursor that is not valid or was made for another sort. | +| `LIMIT_REACHED` | The coach has reached a limit: 200 gymnasts, 50 routines per gymnast, or 50 items in a routine. See [USER_LIST_PAGINATION.md](USER_LIST_PAGINATION.md). | An error response looks like this: @@ -225,7 +227,7 @@ These are the choices for the implementation. They are proposals until the backe ### Search without raw SQL -**The backend never uses raw SQL.** No `$queryRaw`, `$executeRaw` or their `Unsafe` versions. Every query goes through Prisma Client, which sends all values as parameters. This removes SQL injection by the way the code is built, and not by careful escaping. A lint rule (`no-restricted-properties` on those names) will make the build fail if someone adds a raw query. It is added together with the search (issue #24). Indexes are declared in `schema.prisma` with `@@index`, and the migrations are generated by Prisma, with a few additions written by hand in SQL: CHECK constraints and the collation of `catalog_words.word`, which Prisma cannot express. These are migrations, not queries in the backend, so they do not break the rule. +**The backend never uses raw SQL.** No `$queryRaw`, `$executeRaw`, their `Unsafe` versions or `$queryRawTyped` (Prisma's TypedSQL). Every query goes through Prisma Client, which sends all values as parameters. This removes SQL injection by the way the code is built, and not by careful escaping. A lint rule (`no-restricted-properties` on those names, in `eslint.config.js`) makes the build fail if someone adds a raw query anywhere in the backend, including the tests. Indexes are declared in `schema.prisma` with `@@index`, and the migrations are generated by Prisma, with a few additions written by hand in SQL: CHECK constraints and the collation of `catalog_words.word`, which Prisma cannot express. These are migrations, not queries in the backend, so they do not break the rule. The requirement is search on words that is not case sensitive, together with filter, sort and cursors over the whole catalog. Prisma can do this if the data is shaped for it. Two things stop Prisma from doing it on the element and vault tables directly: a query cannot combine two tables into one sorted list, and a cursor only works when every sort field is a plain field of the model (a sort with `nulls` placement, or on a related record, makes it throw an error). @@ -261,14 +263,20 @@ where: { } ``` -- The search text is split into words with `normalizeWords` in `backend/src/catalog/words.ts`: lowercase, accents removed, and only letters and digits kept, except that a code like `3.304` is kept as one word. The seed script uses the same function for the stored words, so the search term and the words always follow the same rules. Empty words are dropped. +- The search text is split into words the same way as the stored words: lowercase, without accents, letters and digits, with codes like `3.304` kept whole. Both sides use `normalizeWords` in `backend/src/catalog/words.ts`, so the search words and the stored words always follow the same rules. Any other character, including `%` and `_`, is always removed. So "hop-grip" becomes `hop` and `grip`, "sec." becomes `sec`, and "1/1" becomes `1`. Empty words are dropped. +- **The search text has limits: at most 80 characters and 8 words** (after cleaning, and a word that is repeated counts once). More gives `BAD_USER_INPUT` with a message that says which limit was passed, so the interface can tell the user. A catalog description has 12 words and 65 characters at the median, so the limits are far more than anyone types, while each word adds a condition to the query. +- **Text without searchable words finds nothing.** If the search text is not empty but gives no words, for example "ø", "日本語", "😀" or "%", the result is empty. The user typed something, so showing the whole catalog would be surprising. Only an empty search, or one with only spaces, means no text filter. +- **Numbers are matched by prefix, like words.** "36" also finds "360", so the results follow the user while they type. A whole number is not matched exactly. - **The cleaning is the only protection against wildcards.** Prisma does not escape `%` and `_` in `startsWith`: a search for `%` becomes `LIKE '%%'` and matches every word (in a test, all 225 000 entries in about 460 ms). The cleaning removes them, so it must always run before the query, and it must have tests. - **The cleaning is also what makes the search case-insensitive.** The words are stored in lowercase, so an uppercase term such as "SAL" finds nothing unless it is lowercased first. - **All words must match.** An item is found only if it has, for every search word, a word that starts with it. "giant turn" finds giant circles with a turn, and not every giant circle and every turn. -- **A trailing `s` is removed from each search word**, so "saltos" finds "salto" and "hips" finds "hip". Because the match is on the start of a word, the shorter word still finds the longer one: "salto" also finds "saltos". A search word that is only `s` is dropped. -- Both the stored words and the term are lowercase, so the search is not case sensitive without `mode: 'insensitive'`. **The search query must never use `mode: 'insensitive'`.** It turns `LIKE` into `ILIKE`, and then the index is no longer used. The seed script stores the words in lowercase, and the backend lowercases the search term before the query. +- **A trailing `s` is removed from each search word**, so "saltos" finds "salto" and "hips" finds "hip". Because the match is on the start of a word, the shorter word still finds the longer one: "salto" also finds "saltos". A search word that is only `s` is dropped. Codes and numbers (words that start with a digit) are left as they are, because they have no plural and a code must be matched exactly. This is the only rule that the search side adds to `normalizeWords` (`searchTerms` in `backend/src/catalog/search.ts`). +- Both the stored words and the term are lowercase, so the search is not case sensitive without `mode: 'insensitive'`. **The search query must never use `mode: 'insensitive'`, and a lint rule (`no-restricted-syntax` in `eslint.config.js`) fails the build if it is used anywhere in the backend.** It turns `LIKE` into `ILIKE`, and then the index is no longer used. The seed script stores the words in lowercase, and the backend lowercases the search term before the query. - `startsWith` finds the rows that have a word starting with the search word, so "sal" finds "salto". It works from the first letter. -- The filter, sort and cursor use the same `findMany` call, and `totalCount` uses `count` with the same `where`. +- The filter, sort and cursor use the same `findMany` query, and `totalCount` uses `count` with the same `where`, without the cursor. The count only runs when the caller asks for it (`withTotalCount`), which the resolver does only when the query contains `totalCount`. +- **Keyset pagination.** The page after a cursor is selected with conditions on the sort values in the `WHERE`, not with Prisma's `cursor` and `skip` options. The rows after a cursor are split into parts in sort order, and each part is its own query: for the code sort, first `codeSort = v` and `id > id`, then `codeSort > v`. The name sort has a third part, the items without a name, when the cursor is on a named item. The parts are read in order until the page is full, so most pages need one or two queries. **Why not one condition with `OR`:** with `codeSort > v OR (codeSort = v AND id > id)` PostgreSQL can only use the sort value as an index bound and checks `id` as a filter. Rows with the same sort value are then read again on every page. That is many rows here: about 69 % of the items have no name (same `nameSort`), and the difficulty sort has only ten values. On the seeded catalog `EXPLAIN ANALYZE` showed `Rows Removed by Filter: 223` for one page with `OR`, and `id` in the `Index Cond` with the split. The parts are defined per sort in `afterParts` in `backend/src/catalog/sorts.ts`, and a test checks that the queries after a cursor have no `OR`. Prisma's `cursor` takes an `id` and looks up that row to find its sort values, so it needs the row to still exist and to be found again with the same filters. Our cursor carries the sort values itself, so the next page works without that lookup, and the values are checked when the cursor is decoded. Prisma still adds `OFFSET 0` to every query with `take`. It skips no rows, so it costs nothing. + +The search is `searchCatalog` in `backend/src/catalog/search.ts`. It returns `CatalogEntry` rows, the page info and, when asked, the total count. The fields that only exist on elements or vaults are loaded by the resolver. `listElementGroups` in the same file lists the groups of one apparatus, for `Apparatus.groups`. **What the index needs.** A B-tree index can serve `LIKE 'sal%'` only if the column uses the `C` collation, or the index uses `text_pattern_ops`. Our databases use the `en-US` collation (the default for PostgreSQL 18 on our machines), so an ordinary index on `word` is not used for a prefix search, and every search would read the whole table. We checked this with `EXPLAIN`. @@ -289,7 +297,7 @@ We tried `text_pattern_ops` on the index first. Prisma accepts it in `schema.pri **Why not Prisma's `search` filter.** Full text search for PostgreSQL in Prisma is a preview feature (`fullTextSearchPostgres`). The documentation does not say if prefix matching is supported, and the index for it cannot be declared in `schema.prisma`, which would mean raw SQL in a migration. We do not use it. -**The cursor.** The cursor given to the client is an opaque text that holds the sort field, the values of the sort fields for the last row, and its `id`. The server decodes and checks it before Prisma gets it. A cursor that cannot be decoded, or that was made for another sort field, gives `BAD_USER_INPUT`. +**The cursor.** The cursor given to the client is base64url-encoded JSON that holds the sort field and direction, the values of the sort fields for the last row, and its `id`. The client should treat it as opaque and only pass the `endCursor` back as `after`. It is not encrypted or signed, on purpose: the catalog is public, so a changed or hand-made cursor can only lead to another page of public data, and a secret key would make every cursor stop working when the key changes or the server restarts without it. What protects the query is the check when the cursor is decoded: the sort field and direction must match the request, the cursor must have exactly the expected keys, the `id` must be a non-empty text, and every sort value must have the right type (a difficulty must look like `0.4`). Anything else gives `BAD_USER_INPUT`, and only checked values reach Prisma, which sends them as parameters. **Query plans.** Prisma sends the search as `word::text LIKE ($1 || '%')`. PostgreSQL normally plans each query with the actual value, sees the prefix, and uses the index (`word >= 'sal' AND word < 'sam'`, about 0.03 ms). With a generic plan, made without knowing the value, it cannot use the prefix and reads the whole table (86 ms on 3 million words in a test). Do not set `plan_cache_mode = force_generic_plan`. Be careful with a connection pooler that uses named prepared statements, because PostgreSQL can switch a prepared statement to a generic plan after it has run a few times. Check with `EXPLAIN` that the index is still used if the connection setup changes. @@ -317,6 +325,8 @@ The course staff must be able to start the backend, so the README (or a file rea ## Testing the API +The search function already has its tests, in `backend/src/catalog/search.test.ts`. They run against the local database with the seeded catalog and only read from it. They are skipped when `DATABASE_URL` is not set, so that `npm test` also works for someone who only works on the frontend. A run with skipped tests has therefore not tested the search (see the README). + Vitest runs the API tests against a test database, with queries sent to the server. The cases that matter most for this API: - **Search:** not case sensitive, no text gives all items, and elements and vaults appear in the same list. @@ -324,6 +334,9 @@ Vitest runs the API tests against a test database, with queries sent to the serv - **Filter and sort:** done over the whole catalog, not only the loaded page. - **Errors:** each code in the table is returned for the right case. A coach that asks for another coach's gymnast gets `NOT_FOUND`, and a request without a login gets `UNAUTHENTICATED` (but `me` gives `null`). - **Wildcards and case:** a search for `%`, `_` or a word containing them does not match everything, and an uppercase search such as "SAL" finds the same items as "sal". +- **Search text limits:** 81 characters or 9 words give `BAD_USER_INPUT`, and a text with no searchable words gives no results, while an empty text or only spaces gives all items. +- **Deep pages:** the queries after a cursor contain no `OR` and no `OFFSET` above 0, so each one starts in the index right after the cursor. +- **Cache:** a second search does not read the apparatus and groups from the database again. - **Injection:** a search term, an `apparatusId` and a cursor that contain SQL (for example a quote followed by DROP TABLE) are handled as plain values and give no results or `BAD_USER_INPUT`, and the tables are still there afterwards. A cursor made for another sort field is rejected. The lint rule against raw queries passes. - **Constraints 6–8:** a routine name that exists, a vault in a beam routine, more than one vault, and an item from the wrong apparatus. - **The gymnast's list:** adding the same item twice keeps one row, removing an item that is not there does nothing, and adding a removed item gives `INVALID_ITEMS`. diff --git a/docs/DATA_MODEL.md b/docs/DATA_MODEL.md index 13476d3..5c4d990 100644 --- a/docs/DATA_MODEL.md +++ b/docs/DATA_MODEL.md @@ -223,7 +223,7 @@ The words the catalog search looks in, one row per distinct word of an entry. Re | `entryId` | `"UB-1.101-a"` | The catalog entry the word finds | | `word` | `"glide"` | One word in lowercase. The column has the `C` collation, so the index can serve a prefix search | -The words come from the code, the name and the description of **all** variants of the code, so a word that is only in variant `-b` still finds the code. The seed script also adds abbreviations from the Code of Points written out as words, for example `handstand` for `hstd`, so that a search for the full word finds elements whose description uses the abbreviation. +The words come from the code, the name and the description of **all** variants of the code, so a word that is only in variant `-b` still finds the code. They are split with `normalizeWords` in `backend/src/catalog/words.ts`, the same function the search uses on the search text, so both sides follow the same rules. The seed script also adds abbreviations from the Code of Points written out as words, for example `handstand` for `hstd`, so that a search for the full word finds elements whose description uses the abbreviation. ## Constraints diff --git a/eslint.config.js b/eslint.config.js index 24af8be..2043a57 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -27,6 +27,35 @@ export default defineConfig([ languageOptions: { globals: globals.node, }, + rules: { + // The backend never uses raw SQL: every query goes through Prisma, which + // sends all values as parameters. See "Search without raw SQL" in + // docs/API.md. + 'no-restricted-properties': [ + 'error', + ...[ + '$queryRaw', + '$executeRaw', + '$queryRawUnsafe', + '$executeRawUnsafe', + '$queryRawTyped', + ].map((property) => ({ + property, + message: + 'Raw SQL is not allowed. Use the Prisma Client API (see docs/API.md).', + })), + ], + // mode: 'insensitive' turns LIKE into ILIKE, which cannot use the index + // on CatalogWord.word. The words are stored in lowercase instead. + 'no-restricted-syntax': [ + 'error', + { + selector: "Property[key.name='mode'][value.value='insensitive']", + message: + "mode: 'insensitive' is not allowed: it becomes ILIKE, which cannot use the index. Lowercase the search words instead (see docs/API.md).", + }, + ], + }, }, { // The shadcn/ui CLI copies components into this folder. They also export