mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
docs(server): name the query API per statement shape and hold QueryBuilder calls to a baseline
server/CLAUDE.md now says which API new repository code uses: Kysely for join, aggregate and set-based statements, the entity methods for single-table CRUD, no new QueryBuilder calls. The TrekRepository docstring no longer calls Kysely an escape hatch. lint:query-api counts this.qb()/createQueryBuilder() per repository file against a baseline that may only shrink and runs in the server CI job. The same file documents the JSON column and timestamp rules from the two changes before.
This commit is contained in:
Maurice
@@ -311,6 +311,12 @@ jobs:
|
||||
# through tests/helpers/factories (MikroORM) instead.
|
||||
run: cd server && npm run lint:test-sql
|
||||
|
||||
- name: QueryBuilder calls in the repositories may only shrink
|
||||
# Counts this.qb(...)/createQueryBuilder(...) per file under
|
||||
# src/db/repositories/ against scripts/query-api-baseline.json. New
|
||||
# code uses Kysely for join reads and the entity methods for CRUD.
|
||||
run: cd server && npm run lint:query-api
|
||||
|
||||
- name: Open request schemas may only shrink
|
||||
# Needs the shared build above. Counts open shapes (records of
|
||||
# unknown, passthrough, any) in shared's request schemas.
|
||||
|
||||
@@ -52,6 +52,7 @@ cd server && npm run lint:dialect # SQLite-only SQL spellings m
|
||||
cd server && npm run probe:pg # dialect helpers + the repository statements it reaches against Postgres (CI job; needs TREK_PG_PROBE_URL)
|
||||
cd server && npm run lint:tx # no new method writing twice outside one transaction (CI gate)
|
||||
cd server && npm run lint:test-sql # raw SQL fixtures in tests/ may only shrink; seed through tests/helpers/factories (CI gate)
|
||||
cd server && npm run lint:query-api # MikroORM QueryBuilder calls in the repositories may only shrink (CI gate)
|
||||
cd client && npm run theme:lint # flag styling that bypasses appearance tokens (local only)
|
||||
npm run typecheck --workspace=server # tsc --noEmit (also client/shared; server also has typecheck:tests)
|
||||
```
|
||||
@@ -126,7 +127,7 @@ These principles come out of a full-repo audit and shape all new code:
|
||||
|
||||
## Quality gates in CI
|
||||
|
||||
`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`), lint (client and server through `lint:warnings`, which also holds the warning count per rule), `lint:pages`, `lint:rtl`, `lint:size` (client and server), the server's `lint:strict`, `lint:boundaries`, `lint:tx`, `lint:dialect` and `lint:test-sql`, the client's and server's `lint:format`, `lint:layers`, `lint:offline`, `lint:dup`, `lint:pairs`, `lint:skips` and `lint:i18n-keys`, shared's `contracts:open`, `check:plugin-facts`, `i18n:parity:strict` (with the untranslated-strings ratchet), the S3 contract suite, the Postgres probe (`postgres-probe`: every dialect helper on SQLite and Postgres, and the statements every repository method reaches with sample arguments against a Postgres service; the SQL errors per method and the list of methods it cannot measure are held to a baseline that only shrinks and fails while it is still unmeasured; see `server/CLAUDE.md`), the browser end-to-end flows (Chromium and WebKit), a client build with `check:gl-split`, coverage for all four packages, then a SonarCloud scan. It also builds the Docker image and boots it until `/api/health/ready` answers (`docker-image`), and lints and renders the Helm chart against the Kubernetes schemas and runs `docker compose config` on every compose file (`deploy-manifests`). `lint-prettier.yml` additionally runs `lint` + `lint:format` on `shared/`, both check-only. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: `theme:lint` and the screenshot and help-media Playwright projects; run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags.
|
||||
`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`), lint (client and server through `lint:warnings`, which also holds the warning count per rule), `lint:pages`, `lint:rtl`, `lint:size` (client and server), the server's `lint:strict`, `lint:boundaries`, `lint:tx`, `lint:dialect`, `lint:test-sql` and `lint:query-api`, the client's and server's `lint:format`, `lint:layers`, `lint:offline`, `lint:dup`, `lint:pairs`, `lint:skips` and `lint:i18n-keys`, shared's `contracts:open`, `check:plugin-facts`, `i18n:parity:strict` (with the untranslated-strings ratchet), the S3 contract suite, the Postgres probe (`postgres-probe`: every dialect helper on SQLite and Postgres, and the statements every repository method reaches with sample arguments against a Postgres service; the SQL errors per method and the list of methods it cannot measure are held to a baseline that only shrinks and fails while it is still unmeasured; see `server/CLAUDE.md`), the browser end-to-end flows (Chromium and WebKit), a client build with `check:gl-split`, coverage for all four packages, then a SonarCloud scan. It also builds the Docker image and boots it until `/api/health/ready` answers (`docker-image`), and lints and renders the Helm chart against the Kubernetes schemas and runs `docker compose config` on every compose file (`deploy-manifests`). `lint-prettier.yml` additionally runs `lint` + `lint:format` on `shared/`, both check-only. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: `theme:lint` and the screenshot and help-media Playwright projects; run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags.
|
||||
|
||||
- **`ci-ok` is the required check.** The branch ruleset requires `ci-ok` (plus `lint` and `check-target` from their own workflows), and `ci-ok` needs every job above except the scan. It fails if any of them failed or was cancelled and passes if they passed or were skipped. A new gate job counts only once it is listed in `ci-ok`'s `needs:`.
|
||||
- **Guides name paths that exist**: the `changes` job runs `scripts/ci/doc-paths.mjs`, which resolves every backtick path in the CLAUDE.md files, `README.md`, `server/src/nest/README.md` and the wiki against the tracked files; the missing ones are listed in `scripts/ci/doc-paths-baseline.json`, which only shrinks (`--update` after a fix).
|
||||
|
||||
+10
-2
@@ -28,6 +28,7 @@ npm run lint:dialect # SQLite-only SQL spellings under src/ may only shrink
|
||||
npm run probe:pg # dialect helpers + the repository statements it reaches against Postgres (CI job postgres-probe; needs TREK_PG_PROBE_URL)
|
||||
npm run lint:tx # methods writing more than once outside one transaction may only shrink (CI gate)
|
||||
npm run lint:test-sql # raw better-sqlite3 statements in tests/ may only shrink (CI gate)
|
||||
npm run lint:query-api # MikroORM QueryBuilder calls in the repositories may only shrink (CI gate)
|
||||
npm run contracts:dto-open # open shapes in the createZodDto schemas may only shrink (runs in the unit tests)
|
||||
```
|
||||
|
||||
@@ -43,6 +44,8 @@ npm run contracts:dto-open # open shapes in the createZodDto schemas may only sh
|
||||
|
||||
**`lint:test-sql`** (`scripts/test-sql-ratchet.mjs`) counts every `.prepare(` under `tests/` per file against `scripts/test-sql-baseline.json`: a file may hold at most its entry, a file without one none. Growth fails, and so does an entry above what its file holds now, so a conversion lands with `npm run lint:test-sql -- --update` (it only lowers and drops entries). Files whose subject is raw SQL (the dialect functions, PRAGMA introspection, the legacy upgrade fixtures) sit in the baseline's `exempt` list with a reason; a single statement that must stay raw carries a `test-sql-allow: <reason>` comment on its line or the line above. `tests/unit/scripts/test-sql-ratchet.test.ts` also runs it on the tree.
|
||||
|
||||
**`lint:query-api`** (`scripts/query-api-ratchet.mjs`) counts every `.qb(` and `createQueryBuilder(` under `src/db/repositories/` per file (the `TrekRepository` base that defines `qb` is left out) against `scripts/query-api-baseline.json`: a file may hold at most its entry, a file without one none, and an entry above what its file holds now fails until `npm run lint:query-api -- --update` lowers it (it never raises or adds one). The rule it holds is "Which query API" under SQL dialect below.
|
||||
|
||||
Single test: `npx vitest run tests/unit/nest/weather.controller.test.ts`, or `npx vitest run -t "returns 401 without cookie"`.
|
||||
|
||||
**Build emits even with type errors** (`scripts/build.mjs` catches `tsc` failures and warns). `npm run build` succeeding does **not** mean the code typechecks — run `npm run typecheck`.
|
||||
@@ -79,12 +82,17 @@ Nest owns everything. Every domain is a DI module under `src/nest/<domain>/` (`c
|
||||
|
||||
SQLite is the engine today and Postgres is the planned second one. Code under `src/` is written so that a Postgres port only adds a branch to the dialect layer, never a sweep through the repositories:
|
||||
|
||||
- **Build statements, don't spell them.** Use the MikroORM QueryBuilder or entity methods, or `this.kysely<Pick<DB, ...>>()` on the generated table types. Repositories and seeders may not hold SQL text at all (ESLint), and `lint:dialect` holds every other file to its baseline.
|
||||
- **Build statements, don't spell them.** Use the entity methods or `this.kysely<Pick<DB, ...>>()` on the generated table types. Repositories and seeders may not hold SQL text at all (ESLint), and `lint:dialect` holds every other file to its baseline.
|
||||
- **Which query API.** New repository code picks by the shape of the statement, not by what the file already uses:
|
||||
- **Kysely** (`this.kysely<Pick<DB, ...>>()`) for every read that joins, aggregates, groups or projects columns across tables, and for set-based writes (a statement over many rows, a subquery, `RETURNING`). Its types come from the migrated schema, so a renamed column fails `tsc`.
|
||||
- **The entity methods** (`findOne`/`find`/`count`/`insert`/`nativeUpdate`/`nativeDelete`/`upsert`) for single-table CRUD by key or by a plain filter. They carry the `TrekRepository` request-context check and the identity-map default.
|
||||
- **No MikroORM QueryBuilder** (`this.qb(...)`, `createQueryBuilder(...)`) in new code. The existing calls stay until their method is touched for another reason, then move to one of the two above; there is no sweep. `lint:query-api` holds the count per file.
|
||||
- **Insert or ignore / upsert**: `.onConflict((oc) => oc.columns([...]).doNothing())` or `.doUpdateSet({...})`, or `em.upsert`. Name the conflict target where there is one. Never `INSERT OR IGNORE`/`INSERT OR REPLACE`.
|
||||
- **The new id**: `.returning('id').executeTakeFirstOrThrow()`. Never `InsertResult.insertId` or `last_insert_rowid()`; both are SQLite's rowid and undefined on Postgres.
|
||||
- **Time**: `currentTimestamp`/`currentTimestampKysely`, `nowMinusDays`, `nowMinusHours`, `nowPlusSeconds`/`nowPlusSecondsKysely`, `nowDateOffset`, `dateOf`, `dateAdd`, `unixEpochToIsoKysely` from `src/db/dialect/sql-functions.ts` (the MikroORM `raw()` helpers) and `src/db/dialect/kysely-functions.ts` (their Kysely-expression twins). Never `datetime('now')`, `strftime` or `julianday`.
|
||||
- **Strings and casts**: `concat`/`concatKysely` instead of `||`, `substring`/`substringKysely`, `lower`/`lowerTrim`, `collateNoCase`, `castInteger`/`castIntegerKysely`, and `startsWithIsoDate`/`startsWithIsoDateKysely` instead of `GLOB`.
|
||||
- **JSON columns** are read whole and parsed in the service; no `json_extract` in SQL.
|
||||
- **JSON columns** are read whole and parsed in the service; no `json_extract` in SQL. Each one is declared once in `src/db/json-columns.ts` (its Zod shape and the fallback a reader gets for an empty or broken row) and read with `decodeJson`/`decodeJsonResult`, written from a server-built value with `encodeJson` (`src/utils/json-column.ts`), never with a bare `JSON.parse` at the call site: one column, one answer to a broken row, logged.
|
||||
- **Timestamps** in a DATETIME column are the canonical `YYYY-MM-DD HH:MM:SS` UTC text, never `toISOString()`: in SQL the two spellings do not compare. A value written from JS is a `DbTimestamp` from `dbNow()` (`src/db/types`), and a stored value is read back as an instant with `parseDbTimestamp()` (a bare `new Date()` reads the canonical text as local time). Where a response has always carried the ISO spelling, convert on the way out (`utcSuffix`), not in the column.
|
||||
- **PRAGMA** belongs to the `src/db/` lifecycle files (`database.ts`, `connection.ts`, `durability.ts`, the migrations). `MaintenanceRepository` is the baselined SQLite maintenance adapter (`wal_checkpoint`, `VACUUM INTO`).
|
||||
- **A missing helper** goes into `sql-functions.ts` (or `kysely-functions.ts` for a Kysely expression), dispatching on the live platform through `src/db/dialect/platform.ts`: a SQLite branch and a Postgres branch (one spelling under `isKnownPlatform` where both engines read it alike), `unsupported(platform)` for anything else. The Postgres branch keeps the SQLite storage model, so a clock helper renders the same `YYYY-MM-DD HH:MM:SS` UTC text. Pin the Postgres text in `tests/unit/db/dialect/sql-functions.postgres.test.ts` and add a case to `scripts/pg-probe/helper-cases.ts` so the probe runs it on both engines. `src/db/dialect/` is the only place allowed to spell an engine's own SQL.
|
||||
- **Migrations** that have shipped are frozen as they are. A new migration's data statements follow the rules above; its DDL is written for the engine it runs on.
|
||||
|
||||
@@ -23,6 +23,7 @@
|
||||
"lint:dialect": "node scripts/dialect-lint.mjs",
|
||||
"lint:tx": "node scripts/tx-writes.mjs",
|
||||
"lint:test-sql": "node scripts/test-sql-ratchet.mjs",
|
||||
"lint:query-api": "node scripts/query-api-ratchet.mjs",
|
||||
"contracts:dto-open": "node scripts/dto-open-shapes.mjs",
|
||||
"gen:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts",
|
||||
"check:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts --check",
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
{
|
||||
"counts": {
|
||||
"src/db/repositories/_shared/packing-visibility.ts": 1,
|
||||
"src/db/repositories/_shared/reservation-travelers-owns.ts": 1,
|
||||
"src/db/repositories/AssignmentParticipants.repository.ts": 3,
|
||||
"src/db/repositories/BudgetItems.repository.ts": 1,
|
||||
"src/db/repositories/Categories.repository.ts": 1,
|
||||
"src/db/repositories/CollabMessageReactions.repository.ts": 2,
|
||||
"src/db/repositories/CollabPollVotes.repository.ts": 1,
|
||||
"src/db/repositories/DawarichConnections.repository.ts": 4,
|
||||
"src/db/repositories/DawarichVisitSuggestions.repository.ts": 4,
|
||||
"src/db/repositories/DayAccommodations.repository.ts": 12,
|
||||
"src/db/repositories/DayAssignments.repository.ts": 20,
|
||||
"src/db/repositories/DayNotes.repository.ts": 6,
|
||||
"src/db/repositories/Days.repository.ts": 7,
|
||||
"src/db/repositories/InviteTokens.repository.ts": 3,
|
||||
"src/db/repositories/JourneyBooks.repository.ts": 3,
|
||||
"src/db/repositories/JourneyContributors.repository.ts": 3,
|
||||
"src/db/repositories/JourneyEntries.repository.ts": 9,
|
||||
"src/db/repositories/JourneyPhotos.repository.ts": 6,
|
||||
"src/db/repositories/Journeys.repository.ts": 4,
|
||||
"src/db/repositories/JourneyShareTokens.repository.ts": 4,
|
||||
"src/db/repositories/JourneyTrips.repository.ts": 1,
|
||||
"src/db/repositories/McpTokens.repository.ts": 5,
|
||||
"src/db/repositories/OauthTokens.repository.ts": 3,
|
||||
"src/db/repositories/PlaceRatings.repository.ts": 1,
|
||||
"src/db/repositories/Places.repository.ts": 28,
|
||||
"src/db/repositories/PlaceShadowPicks.repository.ts": 3,
|
||||
"src/db/repositories/PushSubscriptions.repository.ts": 1,
|
||||
"src/db/repositories/ReservationDayPositions.repository.ts": 1,
|
||||
"src/db/repositories/ReservationEndpoints.repository.ts": 5,
|
||||
"src/db/repositories/Reservations.repository.ts": 14,
|
||||
"src/db/repositories/ReservationTravelers.repository.ts": 2,
|
||||
"src/db/repositories/RoadtripDayBoundaries.repository.ts": 3,
|
||||
"src/db/repositories/RoadtripDayTracks.repository.ts": 1,
|
||||
"src/db/repositories/RoadtripPreferences.repository.ts": 1,
|
||||
"src/db/repositories/RoadtripVias.repository.ts": 6,
|
||||
"src/db/repositories/Tags.repository.ts": 3,
|
||||
"src/db/repositories/TodoItems.repository.ts": 2,
|
||||
"src/db/repositories/TripDocumentLinks.repository.ts": 5,
|
||||
"src/db/repositories/TripInviteTokens.repository.ts": 4,
|
||||
"src/db/repositories/TripMembers.repository.ts": 8,
|
||||
"src/db/repositories/TripPhotos.repository.ts": 1,
|
||||
"src/db/repositories/Trips.repository.ts": 14,
|
||||
"src/db/repositories/Users.repository.ts": 1,
|
||||
"src/db/repositories/VacayPlanMembers.repository.ts": 3,
|
||||
"src/db/repositories/VacayShares.repository.ts": 2
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,155 @@
|
||||
#!/usr/bin/env node
|
||||
/*
|
||||
* lint:query-api: MikroORM QueryBuilder calls in the repositories may only shrink.
|
||||
*
|
||||
* A repository has three ways to build a statement: the entity methods
|
||||
* (findOne/find/insert/nativeUpdate/nativeDelete/upsert/count), Kysely on the
|
||||
* generated table types (`this.kysely<Pick<DB, ...>>()`), and MikroORM's
|
||||
* QueryBuilder (`this.qb(...)`, `createQueryBuilder(...)`). server/CLAUDE.md
|
||||
* ("Which query API") settles which one new code uses: Kysely for a read that
|
||||
* joins, aggregates or projects, the entity methods for single-table CRUD,
|
||||
* and no QueryBuilder. The ones already written stay until their method is
|
||||
* touched; this check makes sure they only recede.
|
||||
*
|
||||
* Every `.qb(` and `createQueryBuilder(` under src/db/repositories/ is
|
||||
* counted per file (the TrekRepository base that defines `qb` is left out).
|
||||
* A file may hold at most its entry in scripts/query-api-baseline.json, and a
|
||||
* file without an entry none. An entry above what its file holds now, or for
|
||||
* a file that is gone, fails too until --update lowers it.
|
||||
*
|
||||
* npm run lint:query-api check against the baseline (CI)
|
||||
* npm run lint:query-api -- --update lower the counts to what the files hold now;
|
||||
* it never raises an entry and never adds one
|
||||
*
|
||||
* --dir=<path> points the check at another server root (the unit tests use it).
|
||||
*/
|
||||
import { existsSync, readdirSync, readFileSync, realpathSync, statSync, writeFileSync } from 'node:fs';
|
||||
import { join, relative, resolve } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
/** The tree the check walks, relative to the server root. */
|
||||
export const ROOT = 'src/db/repositories';
|
||||
|
||||
/** The baseline, relative to the server root. */
|
||||
export const BASELINE = 'scripts/query-api-baseline.json';
|
||||
|
||||
/** Files that define the QueryBuilder entry point rather than use it. */
|
||||
export const IGNORED = new Set(['src/db/repositories/_shared/trek-repository.ts']);
|
||||
|
||||
const SOURCE = /\.ts$/;
|
||||
const QB_CALL = /\.qb\(|\bcreateQueryBuilder\(/g;
|
||||
|
||||
/** The QueryBuilder calls in one file's text, comments included. */
|
||||
export function countQueryBuilderCalls(text) {
|
||||
return text.match(QB_CALL)?.length ?? 0;
|
||||
}
|
||||
|
||||
function walk(dir, files = []) {
|
||||
for (const name of readdirSync(dir)) {
|
||||
const path = join(dir, name);
|
||||
if (statSync(path).isDirectory()) walk(path, files);
|
||||
else if (SOURCE.test(name)) files.push(path);
|
||||
}
|
||||
return files;
|
||||
}
|
||||
|
||||
/** QueryBuilder counts keyed by the server-relative POSIX path, files without any left out. */
|
||||
export function scan(serverDir) {
|
||||
const dir = join(serverDir, ROOT);
|
||||
if (!existsSync(dir) || !statSync(dir).isDirectory()) {
|
||||
throw new Error(`${ROOT}/ does not exist under ${serverDir}: the check would pass without looking at anything`);
|
||||
}
|
||||
const counts = {};
|
||||
for (const path of walk(dir)) {
|
||||
const file = relative(serverDir, path).split('\\').join('/');
|
||||
if (IGNORED.has(file)) continue;
|
||||
const n = countQueryBuilderCalls(readFileSync(path, 'utf8'));
|
||||
if (n > 0) counts[file] = n;
|
||||
}
|
||||
return counts;
|
||||
}
|
||||
|
||||
/** The baseline as committed; missing or malformed stops the run instead of reading as empty. */
|
||||
export function readBaseline(path) {
|
||||
let parsed;
|
||||
try {
|
||||
parsed = JSON.parse(readFileSync(path, 'utf8'));
|
||||
} catch (err) {
|
||||
throw new Error(`${BASELINE} cannot be read: ${err.message}`);
|
||||
}
|
||||
const isObject = (v) => v !== null && typeof v === 'object' && !Array.isArray(v);
|
||||
if (!isObject(parsed) || !isObject(parsed.counts)) {
|
||||
throw new Error(`${BASELINE} must be an object with a "counts" object`);
|
||||
}
|
||||
for (const [file, n] of Object.entries(parsed.counts)) {
|
||||
if (!Number.isInteger(n) || n <= 0) {
|
||||
throw new Error(`${BASELINE}: ${file} holds ${JSON.stringify(n)}, expected a whole number above 0`);
|
||||
}
|
||||
}
|
||||
return parsed;
|
||||
}
|
||||
|
||||
/** The counts lowered to what the files hold now. Never raises an entry, never adds one. */
|
||||
export function lowerCounts(baseline, counts) {
|
||||
const lowered = {};
|
||||
for (const [file, allowed] of Object.entries(baseline.counts)) {
|
||||
const now = counts[file] ?? 0;
|
||||
if (now > 0) lowered[file] = Math.min(allowed, now);
|
||||
}
|
||||
return Object.fromEntries(Object.entries(lowered).sort(([a], [b]) => a.localeCompare(b)));
|
||||
}
|
||||
|
||||
/** Files over their allowance, and entries above what their file holds now. */
|
||||
export function compare(baseline, counts) {
|
||||
const grown = Object.entries(counts).filter(([file, n]) => n > (baseline.counts[file] ?? 0));
|
||||
const lowered = lowerCounts(baseline, counts);
|
||||
const stale = Object.entries(baseline.counts).filter(([file, n]) => lowered[file] !== n);
|
||||
return { grown, stale };
|
||||
}
|
||||
|
||||
function main(argv) {
|
||||
const dirArg = argv.find((a) => a.startsWith('--dir='));
|
||||
const serverDir = dirArg ? resolve(dirArg.slice('--dir='.length)) : fileURLToPath(new URL('..', import.meta.url));
|
||||
const baselinePath = join(serverDir, BASELINE);
|
||||
|
||||
const counts = scan(serverDir);
|
||||
const baseline = readBaseline(baselinePath);
|
||||
if (argv.includes('--update')) {
|
||||
baseline.counts = lowerCounts(baseline, counts);
|
||||
writeFileSync(baselinePath, JSON.stringify(baseline, null, 2) + '\n');
|
||||
}
|
||||
|
||||
const { grown, stale } = compare(baseline, counts);
|
||||
for (const [file, n] of grown) {
|
||||
const entry = baseline.counts[file];
|
||||
console.error(
|
||||
`FAIL ${file}: ${n} QueryBuilder call(s), ` +
|
||||
(entry ? `its baseline is ${entry}. ` : 'a file without a baseline entry may hold none. ') +
|
||||
'Use Kysely for a join or aggregate read and the entity methods for single-table CRUD (server/CLAUDE.md).',
|
||||
);
|
||||
}
|
||||
for (const [file, entry] of stale) {
|
||||
console.error(
|
||||
`FAIL ${file} is held at ${entry} in ${BASELINE}, ` +
|
||||
(file in counts ? `but it has ${counts[file]} now.` : 'but it has none left (or the file is gone).'),
|
||||
);
|
||||
}
|
||||
if (stale.length) {
|
||||
console.error('Run npm run lint:query-api -- --update to lower the baseline with the change that made it smaller.');
|
||||
}
|
||||
const total = Object.values(counts).reduce((sum, n) => sum + n, 0);
|
||||
console.log(`query-api: ${total} QueryBuilder call(s) in ${Object.keys(counts).length} file(s) held at their baseline`);
|
||||
return grown.length || stale.length ? 1 : 0;
|
||||
}
|
||||
|
||||
// Compared by real path, so the check still runs when the script is started through a symlink.
|
||||
const isCli =
|
||||
Boolean(process.argv[1]) && existsSync(process.argv[1]) && realpathSync(process.argv[1]) === fileURLToPath(import.meta.url);
|
||||
if (isCli) {
|
||||
try {
|
||||
process.exitCode = main(process.argv.slice(2));
|
||||
} catch (err) {
|
||||
console.error(`FAIL ${err.message}`);
|
||||
process.exitCode = 1;
|
||||
}
|
||||
}
|
||||
@@ -70,12 +70,15 @@ import {
|
||||
* option itself any more. See
|
||||
* https://mikro-orm.io/docs/entity-manager#disableidentitymap.
|
||||
*
|
||||
* **Kysely.** `em.getKysely()` (used for the one or two statements per
|
||||
* repository the QueryBuilder cannot express — a recursive CTE, a
|
||||
* `DELETE … RETURNING`) resolves its `EntityManager` with `getContext(false)`
|
||||
* too (`node_modules/@mikro-orm/sql/SqlEntityManager.js`), so it has the
|
||||
* exact same gap as the write paths above. {@link TrekRepository.kysely} is
|
||||
* the one place a repository reaches Kysely from now on, instead of calling
|
||||
* **Kysely.** `em.getKysely()` is the query API for every read that joins,
|
||||
* aggregates or projects across tables and for set-based writes, typed
|
||||
* against the generated `DB` from `db/kysely/db.ts`; the entity methods cover
|
||||
* single-table CRUD, and new code adds no QueryBuilder calls (server/CLAUDE.md,
|
||||
* "Which query API"; `lint:query-api` holds the existing ones). It resolves its
|
||||
* `EntityManager` with `getContext(false)` too
|
||||
* (`node_modules/@mikro-orm/sql/SqlEntityManager.js`), so it has the exact
|
||||
* same gap as the write paths above. {@link TrekRepository.kysely} is the one
|
||||
* place a repository reaches Kysely, instead of calling
|
||||
* `this.getEntityManager().getKysely()` directly.
|
||||
*
|
||||
* See https://mikro-orm.io/docs/repositories (custom repositories) and
|
||||
@@ -95,11 +98,9 @@ export abstract class TrekRepository<Entity extends object> extends EntityReposi
|
||||
}
|
||||
|
||||
/**
|
||||
* `em.getKysely()`, validated first. The QueryBuilder has no recursive-CTE
|
||||
* support and its `DELETE … RETURNING` is silently dropped
|
||||
* (`WebauthnChallenges.repository.ts::claimChallenge`'s docstring), so a
|
||||
* handful of methods across the program need this escape hatch — always
|
||||
* through here now, never `this.getEntityManager().getKysely()` directly.
|
||||
* `em.getKysely()`, validated first: the query API for join, aggregate and
|
||||
* set-based statements (see the class docstring), always reached through
|
||||
* here, never through `this.getEntityManager().getKysely()` directly.
|
||||
* No explicit return-type annotation: `SqlEntityManager.getKysely`'s
|
||||
* return type is expressed in terms of `this` (the entity list the calling
|
||||
* `EntityManager` was constructed with), which does not survive being
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
/**
|
||||
* scripts/query-api-ratchet.mjs holds the MikroORM QueryBuilder calls in the
|
||||
* repositories to their baseline (server/CLAUDE.md, "Which query API"). It
|
||||
* runs the real script against throwaway server roots (--dir) and once on this
|
||||
* repository, so the committed baseline is checked by the unit suite too.
|
||||
*/
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
|
||||
const SCRIPT = path.join(__dirname, '../../../scripts/query-api-ratchet.mjs');
|
||||
const SERVER_ROOT = path.join(__dirname, '../../..');
|
||||
const REPOS = 'src/db/repositories';
|
||||
|
||||
// Spelled in pieces so this file reads as what it is, a fixture.
|
||||
const QB = ['this.', 'qb(', "'r')"].join('');
|
||||
const calls = (n: number) => `await ${QB}.select('*').execute();\n`.repeat(n);
|
||||
|
||||
const roots: string[] = [];
|
||||
|
||||
function serverRoot(files: Record<string, string>, baseline: unknown = { counts: {} }): string {
|
||||
const dir = mkdtempSync(path.join(tmpdir(), 'trek-query-api-'));
|
||||
roots.push(dir);
|
||||
mkdirSync(path.join(dir, REPOS), { recursive: true });
|
||||
mkdirSync(path.join(dir, 'scripts'), { recursive: true });
|
||||
for (const [file, text] of Object.entries(files)) {
|
||||
mkdirSync(path.dirname(path.join(dir, file)), { recursive: true });
|
||||
writeFileSync(path.join(dir, file), text);
|
||||
}
|
||||
if (baseline !== null) {
|
||||
writeFileSync(path.join(dir, 'scripts/query-api-baseline.json'), typeof baseline === 'string' ? baseline : JSON.stringify(baseline));
|
||||
}
|
||||
return dir;
|
||||
}
|
||||
|
||||
function run(dir: string, ...args: string[]): { status: number | null; out: string } {
|
||||
try {
|
||||
const stdout = execFileSync(process.execPath, [SCRIPT, `--dir=${dir}`, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] });
|
||||
return { status: 0, out: stdout };
|
||||
} catch (err) {
|
||||
const { status, stdout, stderr } = err as { status: number | null; stdout: string; stderr: string };
|
||||
return { status, out: `${stdout}${stderr}` };
|
||||
}
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const dir of roots.splice(0)) rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('query-api-ratchet.mjs', () => {
|
||||
it('QAPI-001: passes when no repository builds through the QueryBuilder', () => {
|
||||
const { status, out } = run(serverRoot({ [`${REPOS}/A.repository.ts`]: 'await this.findOne({ id });\n' }));
|
||||
expect(status).toBe(0);
|
||||
expect(out).toContain('query-api: 0 QueryBuilder call(s) in 0 file(s)');
|
||||
});
|
||||
|
||||
it('QAPI-002: fails a new file holding one, and counts createQueryBuilder too', () => {
|
||||
const { status, out } = run(serverRoot({ [`${REPOS}/New.repository.ts`]: `${calls(1)}em.createQueryBuilder(X);\n` }));
|
||||
expect(status).toBe(1);
|
||||
expect(out).toContain(`FAIL ${REPOS}/New.repository.ts: 2 QueryBuilder call(s), a file without a baseline entry may hold none.`);
|
||||
});
|
||||
|
||||
it('QAPI-003: holds a baselined file to its entry', () => {
|
||||
const baseline = { counts: { [`${REPOS}/A.repository.ts`]: 2 } };
|
||||
const grown = run(serverRoot({ [`${REPOS}/A.repository.ts`]: calls(3) }, baseline));
|
||||
expect(grown.status).toBe(1);
|
||||
expect(grown.out).toContain('its baseline is 2.');
|
||||
expect(run(serverRoot({ [`${REPOS}/A.repository.ts`]: calls(2) }, baseline)).status).toBe(0);
|
||||
});
|
||||
|
||||
it('QAPI-004: the base class that defines qb is not counted', () => {
|
||||
expect(run(serverRoot({ [`${REPOS}/_shared/trek-repository.ts`]: 'return super.qb(alias);\n' })).status).toBe(0);
|
||||
});
|
||||
|
||||
it('QAPI-005: an entry above its file fails until --update lowers it, and --update never raises or adds one', () => {
|
||||
const dir = serverRoot(
|
||||
{ [`${REPOS}/A.repository.ts`]: calls(1), [`${REPOS}/B.repository.ts`]: calls(4) },
|
||||
{ counts: { [`${REPOS}/A.repository.ts`]: 3, [`${REPOS}/Gone.repository.ts`]: 1, [`${REPOS}/B.repository.ts`]: 2 } },
|
||||
);
|
||||
const stale = run(dir);
|
||||
expect(stale.status).toBe(1);
|
||||
expect(stale.out).toContain(`FAIL ${REPOS}/A.repository.ts is held at 3`);
|
||||
expect(stale.out).toContain(`FAIL ${REPOS}/Gone.repository.ts is held at 1`);
|
||||
|
||||
run(dir, '--update');
|
||||
const lowered = JSON.parse(readFileSync(path.join(dir, 'scripts/query-api-baseline.json'), 'utf8')) as { counts: Record<string, number> };
|
||||
expect(lowered.counts).toEqual({ [`${REPOS}/A.repository.ts`]: 1, [`${REPOS}/B.repository.ts`]: 2 });
|
||||
});
|
||||
|
||||
it('QAPI-006: a missing or malformed baseline, or no repositories folder, stops the run', () => {
|
||||
expect(run(serverRoot({}, null)).out).toContain('cannot be read');
|
||||
expect(run(serverRoot({}, '{"counts":{"x":0}}')).out).toContain('expected a whole number above 0');
|
||||
expect(run(serverRoot({}, '[]')).out).toContain('must be an object with a "counts" object');
|
||||
const empty = mkdtempSync(path.join(tmpdir(), 'trek-query-api-'));
|
||||
roots.push(empty);
|
||||
expect(run(empty).out).toContain('does not exist');
|
||||
});
|
||||
|
||||
it('QAPI-007: this repository holds its committed baseline', () => {
|
||||
const { status, out } = run(SERVER_ROOT);
|
||||
expect(out).toContain('query-api:');
|
||||
expect(status).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -182,6 +182,7 @@ The `@trek/shared` package is the single source of truth for code shared between
|
||||
| `npm run lint:boundaries` | Import cycles and domain boundaries may not grow past their baseline (CI gate) |
|
||||
| `npm run lint:tx` | Methods that write more than once outside one transaction may not grow past their baseline (CI gate) |
|
||||
| `npm run lint:test-sql` | Raw SQL fixtures in `tests/` may not grow past their baseline (CI gate) |
|
||||
| `npm run lint:query-api` | MikroORM QueryBuilder calls in the repositories may not grow past their baseline (CI gate) |
|
||||
| `npm run lint:dialect` | SQLite-only SQL spellings under `src/` may not grow past their baseline (CI gate) |
|
||||
| `npm run gen:db-types` | Regenerate the Kysely table types in `src/db/kysely/` from the migrated schema |
|
||||
| `npm run check:db-types` | Verify the generated Kysely table types match the migrations (CI gate) |
|
||||
@@ -250,7 +251,7 @@ Then open a Pull Request from your fork to `liketrek/TREK` targeting the `dev` b
|
||||
- Always branch off from an up-to-date `dev` — run `git fetch upstream && git rebase upstream/dev` before starting new work.
|
||||
- Run tests before pushing: `npm test` at the repo root runs all workspaces. That alone is not the full CI gate. With `shared` built, these are the checks CI runs before any test (`.github/workflows/test.yml` is the source of truth):
|
||||
- in `shared/`: `npm run typecheck && npm run contracts:open`
|
||||
- in `server/`: `npm run build && npm run typecheck && npm run typecheck:tests && npm run typecheck:scripts && npm run check:entities && npm run check:db-types && npm run lint:check && npm run db:call-graph -- --sync --tx && npm run lint:size && npm run lint:boundaries && npm run lint:tx && npm run lint:test-sql && npm run lint:dialect && npm run check:plugin-facts`
|
||||
- in `server/`: `npm run build && npm run typecheck && npm run typecheck:tests && npm run typecheck:scripts && npm run check:entities && npm run check:db-types && npm run lint:check && npm run db:call-graph -- --sync --tx && npm run lint:size && npm run lint:boundaries && npm run lint:tx && npm run lint:test-sql && npm run lint:query-api && npm run lint:dialect && npm run check:plugin-facts`
|
||||
- in `client/`: `npm run typecheck && npm run lint:warnings && npm run lint:pages && npm run lint:rtl && npm run lint:size && npm run lint:format && npm run lint:layers && npm run lint:offline && npm run lint:dup && npm run lint:pairs && npm run lint:skips && npm run lint:i18n-keys`
|
||||
- at the root, if you touched translations: `npm run i18n:parity:strict --workspace=shared`
|
||||
- Follow the commit message conventions described in the [[Contributing]] guidelines.
|
||||
|
||||
Reference in New Issue
Block a user