mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
build(server): the response-contract exempt marker only excuses a handler that writes through @Res()
A response-contract-exempt comment counted any handler as covered, whatever it returned, so one comment line let a JSON handler past the ratchet. The marker now counts only on a handler taking @Res() or @Next(). The five vacay holiday proxies return the provider's JSON, so they lose their markers and are counted again until they declare a schema.
This commit is contained in:
+1
-1
@@ -55,7 +55,7 @@ npm run contracts:dto-open # open shapes in the createZodDto schemas may only sh
|
||||
|
||||
**`lint:mcp-zod`** (`scripts/mcp-inline-zod.mjs`) and **`lint:service-http`** (`scripts/service-http-ratchet.mjs`) share the per-file count rules of `scripts/lib/count-ratchet.mjs`: a file may hold at most its entry in its baseline (`scripts/mcp-inline-zod-baseline.json`, `scripts/service-http-baseline.json`), a file without one none, and an entry above what its file holds now fails until `-- --update` lowers it (never raises or adds). `lint:mcp-zod` counts every `z.` in a `*.mcp.ts`: a tool field derives from the request schema in `@trek/shared` or a shared primitive (`idSchema`); a bound REST deliberately lacks stays inline and counted. `lint:service-http` counts, in every file under `src/nest` but the HTTP layer (controllers, guards, pipes, filters, interceptors, DTOs and `domain-error.ts` itself), the `status: 4xx`/`5xx` literals of a returned `{ error, status }` and every `new HttpException(`, and in a `*.controller.ts`, `*.mcp.ts` or `*.rpc.ts` every branch on a returned error (`if (result.error)`, `if (!r.error)`, `'error' in r`, `r.error === 'not_found'`, `r?.error && ...`, `r.error ? a : b`). A service refuses by throwing a `DomainError` (`src/nest/common/domain-error.ts`) instead; `src/nest/README.md` ("Refusals") has the shape.
|
||||
|
||||
**`lint:response-contracts`** (`scripts/response-contract-ratchet.mjs`) reads every class under `src/nest/` with the TypeScript parser and counts, per file, the route handlers (`@Get`/`@Post`/`@Put`/`@Patch`/`@Delete`/`@All`/`@Head`/`@Options`) that carry no `@ResponseContract(...)` against `scripts/response-contract-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:response-contracts -- --update` lowers it (it never raises or adds one). A handler that writes through `@Res()` (a stream, a redirect) or passes a third party's answer through unchanged has no body of TREK's to check and is marked with a `response-contract-exempt: <reason>` comment above it instead. The run prints the covered share.
|
||||
**`lint:response-contracts`** (`scripts/response-contract-ratchet.mjs`) reads every class under `src/nest/` with the TypeScript parser and counts, per file, the route handlers (`@Get`/`@Post`/`@Put`/`@Patch`/`@Delete`/`@All`/`@Head`/`@Options`) that carry no `@ResponseContract(...)` against `scripts/response-contract-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:response-contracts -- --update` lowers it (it never raises or adds one). A handler that writes through `@Res()` (a stream, a redirect) has no return value to check and is marked with a `response-contract-exempt: <reason>` comment above it instead; the marker excuses only such a handler, so one that returns a body, a third party's answer passed through included, stays counted until it declares a schema. The run prints the covered share.
|
||||
|
||||
Single test: `npx vitest run tests/unit/nest/weather.controller.test.ts`, or `npx vitest run -t "returns 401 without cookie"`.
|
||||
|
||||
|
||||
@@ -96,6 +96,7 @@
|
||||
"src/nest/trip-invite/trip-invite.controller.ts": 5,
|
||||
"src/nest/trip-members/trip-members.controller.ts": 7,
|
||||
"src/nest/trips/trips.controller.ts": 12,
|
||||
"src/nest/vacay/vacay.controller.ts": 5,
|
||||
"src/nest/weather/weather.controller.ts": 2
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,9 +10,11 @@
|
||||
*
|
||||
* Every method under src/nest/ carrying a route decorator (@Get, @Post, @Put,
|
||||
* @Patch, @Delete, @All, @Head, @Options) counts as a handler. One counts as
|
||||
* covered when it also carries @ResponseContract(...), or when a comment
|
||||
* above it reads `response-contract-exempt: <reason>` (a handler that writes
|
||||
* through @Res(), a redirect or a stream, has no return value to check). The
|
||||
* covered when it also carries @ResponseContract(...), or when it writes
|
||||
* through @Res() (or hands off with @Next()) and a comment above it reads
|
||||
* `response-contract-exempt: <reason>`: a redirect or a stream has no return
|
||||
* value to check. The marker on a handler that returns its body excuses
|
||||
* nothing, so a comment cannot stand in for a schema. The
|
||||
* uncovered handlers are counted per file against
|
||||
* scripts/response-contract-baseline.json: a file may hold at most its entry,
|
||||
* a file without one none. An entry above what its file holds now, or for a
|
||||
@@ -60,6 +62,16 @@ function isExempt(text, method) {
|
||||
return head.split('\n').some((line) => /^\s*(\/\/|\/?\*)/.test(line) && line.includes(EXEMPT_MARKER));
|
||||
}
|
||||
|
||||
/** Whether the handler takes the response (or next) object, so it answers through it rather than by returning. */
|
||||
function writesThroughResponse(method) {
|
||||
return method.parameters.some((param) =>
|
||||
(ts.getDecorators(param) ?? []).some((d) => {
|
||||
const name = decoratorName(d);
|
||||
return name === 'Res' || name === 'Next' || name === 'Response';
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
/** The route handlers of one file's text: `{ name, covered }` in source order. */
|
||||
export function handlersOf(text, fileName = 'file.ts') {
|
||||
const source = ts.createSourceFile(fileName, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
|
||||
@@ -69,7 +81,8 @@ export function handlersOf(text, fileName = 'file.ts') {
|
||||
const names = (ts.getDecorators(node) ?? []).map(decoratorName);
|
||||
if (names.some((n) => n && ROUTE_DECORATORS.has(n))) {
|
||||
const name = node.name && ts.isIdentifier(node.name) ? node.name.text : (node.name?.getText(source) ?? '?');
|
||||
handlers.push({ name, covered: names.includes('ResponseContract') || isExempt(text, node) });
|
||||
const exempt = isExempt(text, node) && writesThroughResponse(node);
|
||||
handlers.push({ name, covered: names.includes('ResponseContract') || exempt });
|
||||
}
|
||||
}
|
||||
ts.forEachChild(node, visit);
|
||||
|
||||
@@ -360,28 +360,28 @@ export class VacayController {
|
||||
return { success: true };
|
||||
}
|
||||
|
||||
// response-contract-exempt: passes the holiday provider's answer through as it came; its shape is the provider's.
|
||||
// No contract yet: passes the holiday provider's answer through as it came, so its shape is the provider's.
|
||||
@Get('holidays/countries')
|
||||
async holidayCountries() {
|
||||
const result = await this.vacay.getCountries();
|
||||
return result.data;
|
||||
}
|
||||
|
||||
// response-contract-exempt: passes the holiday provider's answer through as it came; its shape is the provider's.
|
||||
// No contract yet: passes the holiday provider's answer through as it came, so its shape is the provider's.
|
||||
@Get('holidays/:year/:country')
|
||||
async holidays(@Param('year') year: string, @Param('country') country: string) {
|
||||
const result = await this.vacay.getHolidays(year, country);
|
||||
return result.data;
|
||||
}
|
||||
|
||||
// response-contract-exempt: passes the holiday provider's answer through as it came; its shape is the provider's.
|
||||
// No contract yet: passes the holiday provider's answer through as it came, so its shape is the provider's.
|
||||
@Get('school-holidays/regions/:country')
|
||||
async schoolHolidayRegions(@Param('country') country: string) {
|
||||
const result = await this.vacay.getSchoolHolidayRegions(country, country.toUpperCase() === 'DE' ? 'DE' : 'EN');
|
||||
return result.data;
|
||||
}
|
||||
|
||||
// response-contract-exempt: passes the holiday provider's answer through as it came; its shape is the provider's.
|
||||
// No contract yet: passes the holiday provider's answer through as it came, so its shape is the provider's.
|
||||
@Get('school-holidays/:year/:country')
|
||||
async schoolHolidaysForCountry(
|
||||
@Param('year') year: string,
|
||||
@@ -391,7 +391,7 @@ export class VacayController {
|
||||
return this.schoolHolidays(year, country, undefined, group);
|
||||
}
|
||||
|
||||
// response-contract-exempt: passes the holiday provider's answer through as it came; its shape is the provider's.
|
||||
// No contract yet: passes the holiday provider's answer through as it came, so its shape is the provider's.
|
||||
@Get('school-holidays/:year/:country/:subdivision')
|
||||
async schoolHolidaysForSubdivision(
|
||||
@Param('year') year: string,
|
||||
|
||||
@@ -68,6 +68,23 @@ describe('response-contract-ratchet.mjs', () => {
|
||||
expect(out).toContain('response-contracts: 2 of 2 route handler(s) covered (100.0 %)');
|
||||
});
|
||||
|
||||
it('RC-008: the exempt marker excuses only a handler that writes through @Res() or @Next()', () => {
|
||||
const marked = (name: string, param: string) =>
|
||||
` // response-contract-exempt: says so\n @Get('${name}')\n ${name}(${param}) {\n return {};\n }\n`;
|
||||
const { status, out } = run(
|
||||
serverRoot({
|
||||
[`${NEST}/x/x.controller.ts`]: controller(
|
||||
marked('returnsBody', "@Param('id') id: string"),
|
||||
marked('streams', '@Res() res: Response'),
|
||||
marked('handsOff', '@Next() next: () => void'),
|
||||
),
|
||||
}),
|
||||
);
|
||||
expect(status).toBe(1);
|
||||
expect(out).toContain(`FAIL ${NEST}/x/x.controller.ts: 1 route handler(s) without @ResponseContract`);
|
||||
expect(out).toContain('2 of 3 route handler(s) covered');
|
||||
});
|
||||
|
||||
it('RC-002: fails a new file holding a handler without one, and ignores plain methods', () => {
|
||||
const helper = ' private helper() {\n return 1;\n }\n';
|
||||
const { status, out } = run(serverRoot({ [`${NEST}/x/x.controller.ts`]: controller(bare('a'), helper) }));
|
||||
|
||||
Reference in New Issue
Block a user