diff --git a/packages/agent-bff/src/data/agent-query.ts b/packages/agent-bff/src/data/agent-query.ts index 8be98697b0..f89166a9da 100644 --- a/packages/agent-bff/src/data/agent-query.ts +++ b/packages/agent-bff/src/data/agent-query.ts @@ -1,40 +1,33 @@ -import type { ZodType } from 'zod'; - -import { CountFlatInputs, ListFlatInputs } from './request-schemas'; +import type { PageInput, SortClauseInput } from './request-schemas'; +import type { BffHttpError } from '../http/bff-http-error'; +import type { Logger } from '../ports/logger-port'; +import type { ZodType, z } from 'zod'; + +import { + CountFlatInputs, + ListFlatInputs, + RelationCountFlatInputs, + RelationListFlatInputs, +} from './request-schemas'; import { invalidRequest } from '../http/bff-local-errors'; import { MAX_FILTER_DEPTH, isBranch, isLeaf } from '../validation/capabilities-validator'; import { filterTooDeep } from '../validation/validation-errors'; export { MAX_FILTER_DEPTH as MAX_PARSED_FILTER_DEPTH }; -export interface BffSortClause { - field: string; - direction?: 'asc' | 'desc'; -} +export type BffSortClause = z.infer; -export interface BffPage { - limit: number; - offset: number; -} +export type BffPage = z.infer; -export interface ListRequestBody { - filter?: unknown; - projection?: string[]; - sort?: BffSortClause[]; - page?: BffPage; - search?: string; - searchExtended?: boolean; -} +export type ListRequestBody = z.infer; -export interface CountRequestBody { - filter?: unknown; - search?: string; - searchExtended?: boolean; -} +export type CountRequestBody = z.infer; -export type RelationListRequestBody = ListRequestBody & { parentId: string }; +export type RelationListRequestBody = z.infer & { parentId: string }; -export type RelationCountRequestBody = CountRequestBody & { parentId: string }; +export type RelationCountRequestBody = z.infer & { + parentId: string; +}; export type AgentQuery = Record & { timezone: string }; @@ -42,64 +35,90 @@ function isPlainObject(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } -function assertNoNodeReadableAsBothLeafAndBranch(node: unknown, depth = 0): void { - if (depth > MAX_FILTER_DEPTH) throw filterTooDeep(MAX_FILTER_DEPTH); - if (typeof node !== 'object' || node === null) return; +const LEAF_KEYS = ['field', 'operator', 'value']; +const BRANCH_KEYS = ['aggregator', 'conditions']; + +function rejectBody(logger: Logger, error: BffHttpError): never { + logger('Warn', 'Request body rejected', { reason: error.message }); + + throw error; +} + +// The tree is closed like the flat body is: a leaf carrying `valu` instead of `value` builds a +// condition with `value: undefined`, which the agent reads as null and runs — a typo must not +// silently change the returned rows. +function assertNoStrayKey(node: Record, allowed: string[], logger: Logger): void { + const stray = Object.keys(node).find(key => !allowed.includes(key)); + + if (stray !== undefined) { + rejectBody(logger, invalidRequest(`A filter node cannot carry "${stray}"`)); + } +} + +function assertFilterNode(node: unknown, logger: Logger, depth = 0): void { + if (depth > MAX_FILTER_DEPTH) rejectBody(logger, filterTooDeep(MAX_FILTER_DEPTH)); + if (!isPlainObject(node)) return; const readableAsBranch = isBranch(node); if (isLeaf(node) && readableAsBranch) { - throw invalidRequest('A filter node cannot carry both "field" and "conditions"'); + rejectBody(logger, invalidRequest('A filter node cannot carry both "field" and "conditions"')); } if (readableAsBranch) { - node.conditions.forEach(condition => - assertNoNodeReadableAsBothLeafAndBranch(condition, depth + 1), - ); + assertNoStrayKey(node, BRANCH_KEYS, logger); + node.conditions.forEach(condition => assertFilterNode(condition, logger, depth + 1)); + + return; + } + + if (isLeaf(node)) { + assertNoStrayKey(node, LEAF_KEYS, logger); + + return; } + + // Neither readable as a leaf nor as a branch: `feild` instead of `field` reaches the agent as a + // node it cannot act on. An empty object stays allowed — it is how an absent filter is spelled. + assertNoStrayKey(node, [], logger); } -/** - * Checks the flat inputs against the shared schema and reports the first failure as - * 400 invalid_request, so a malformed shape (`projection` as a string, a fractional `page.limit`) - * surfaces as a client error rather than a 500 from an array method blowing up downstream. - * - * Unknown keys are left alone: the object schemas ignore them, so `filter`, `timezone` and - * `parentId` travel through untouched and the body is returned by reference, not rebuilt. - */ -function assertFlatInputs(schema: ZodType, body: Record): void { +function assertFlatInputs(schema: ZodType, body: Record, logger: Logger): void { const result = schema.safeParse(body); if (result.success) return; - const [issue] = result.error.issues; + const { issues } = result.error; + const issue = issues.find(candidate => candidate.code === 'unrecognized_keys') ?? issues[0]; const path = issue.path.join('.'); + const reason = path ? `${path}: ${issue.message}` : issue.message; - throw invalidRequest(path ? `${path}: ${issue.message}` : issue.message); + rejectBody(logger, invalidRequest(reason)); } -function assertFilter(filter: unknown): void { +function assertFilter(filter: unknown, logger: Logger): void { if (filter === undefined) return; - if (!isPlainObject(filter)) throw invalidRequest('filter must be an object'); + if (!isPlainObject(filter)) rejectBody(logger, invalidRequest('filter must be an object')); - assertNoNodeReadableAsBothLeafAndBranch(filter); + assertFilterNode(filter, logger); } -export function parseListRequest(body: unknown): ListRequestBody { - if (!isPlainObject(body)) throw invalidRequest('Request body must be an object'); +function parseRequest(schema: S, body: unknown, logger: Logger): z.output { + if (!isPlainObject(body)) { + rejectBody(logger, invalidRequest('Request body must be an object')); + } - assertFlatInputs(ListFlatInputs, body); - assertFilter(body.filter); + assertFlatInputs(schema, body, logger); + assertFilter(body.filter, logger); - return body as ListRequestBody; + return body as z.output; } -export function parseCountRequest(body: unknown): CountRequestBody { - if (!isPlainObject(body)) throw invalidRequest('Request body must be an object'); - - assertFlatInputs(CountFlatInputs, body); - assertFilter(body.filter); +export function parseListRequest(body: unknown, logger: Logger): ListRequestBody { + return parseRequest(ListFlatInputs, body, logger); +} - return body as CountRequestBody; +export function parseCountRequest(body: unknown, logger: Logger): CountRequestBody { + return parseRequest(CountFlatInputs, body, logger); } function collectFilterFields(filter: unknown, acc: string[]): void { @@ -188,9 +207,11 @@ export function collectListFieldPaths(body: ListRequestBody): string[] { // The parent record id is opaque: a packed/composite id must survive unchanged, so its content is // never inspected — only presence and primitive type. A finite number (single numeric pk) is // coerced to string; anything else is a BFF-local 400 with no agent call. -export function parseParentId(parentId: unknown): string { +export function parseParentId(parentId: unknown, logger: Logger): string { if (typeof parentId === 'string') { - if (parentId.trim() === '') throw invalidRequest('parentId must not be empty'); + if (parentId.trim() === '') { + rejectBody(logger, invalidRequest('parentId must not be empty')); + } return parentId; } @@ -199,19 +220,22 @@ export function parseParentId(parentId: unknown): string { return String(parentId); } - throw invalidRequest('parentId is required and must be a non-empty string or a number'); + rejectBody( + logger, + invalidRequest('parentId is required and must be a non-empty string or a number'), + ); } -export function parseRelationListRequest(body: unknown): RelationListRequestBody { - const parentId = parseParentId((body as { parentId?: unknown } | null)?.parentId); +export function parseRelationListRequest(body: unknown, logger: Logger): RelationListRequestBody { + const parentId = parseParentId((body as { parentId?: unknown } | null)?.parentId, logger); - return { ...parseListRequest(body), parentId }; + return { ...parseRequest(RelationListFlatInputs, body, logger), parentId }; } -export function parseRelationCountRequest(body: unknown): RelationCountRequestBody { - const parentId = parseParentId((body as { parentId?: unknown } | null)?.parentId); +export function parseRelationCountRequest(body: unknown, logger: Logger): RelationCountRequestBody { + const parentId = parseParentId((body as { parentId?: unknown } | null)?.parentId, logger); - return { ...parseCountRequest(body), parentId }; + return { ...parseRequest(RelationCountFlatInputs, body, logger), parentId }; } export function collectCountFieldPaths(body: CountRequestBody): string[] { diff --git a/packages/agent-bff/src/data/data-routes-middleware.ts b/packages/agent-bff/src/data/data-routes-middleware.ts index 1c9e633af6..be4915cca9 100644 --- a/packages/agent-bff/src/data/data-routes-middleware.ts +++ b/packages/agent-bff/src/data/data-routes-middleware.ts @@ -295,12 +295,12 @@ async function handleRelation( const relationDeps: RelationHandlerDeps = { ...deps, relation, foreignCollection }; if (operation === 'list') { - await handleRelationList(ctx, parseRelationListRequest(rawBody), { + await handleRelationList(ctx, parseRelationListRequest(rawBody, deps.logger), { ...relationDeps, primaryKeys: readModel.getPrimaryKeys(foreignCollection), }); } else { - await handleRelationCount(ctx, parseRelationCountRequest(rawBody), relationDeps); + await handleRelationCount(ctx, parseRelationCountRequest(rawBody, deps.logger), relationDeps); } } @@ -355,10 +355,10 @@ export default function createDataRoutesMiddleware({ const operation = match[2] as 'list' | 'count'; if (operation === 'list') { - const body = parseListRequest(rawBody); + const body = parseListRequest(rawBody, logger); await handleList(ctx, body, { ...deps, primaryKeys: readModel.getPrimaryKeys(collection) }); } else { - const body = parseCountRequest(rawBody); + const body = parseCountRequest(rawBody, logger); await handleCount(ctx, body, deps); } }; diff --git a/packages/agent-bff/src/data/request-schemas.ts b/packages/agent-bff/src/data/request-schemas.ts index e474ebd45f..a64597d8ce 100644 --- a/packages/agent-bff/src/data/request-schemas.ts +++ b/packages/agent-bff/src/data/request-schemas.ts @@ -1,20 +1,11 @@ import { z } from '../zod'; -/** - * The validation rules of a request body, declared once. `src/openapi/schemas.ts` decorates these - * with names and descriptions to publish them, and the parsers below check bodies against them, so - * the document and the runtime cannot drift apart. - * - * `filter` is deliberately absent: its operators are checked against the collection capabilities - * (`capabilities-validator`), and a node readable as both leaf and branch has no schema equivalent, - * so it keeps its own path in `agent-query.ts`. - */ -export const SortClauseInput = z.object({ +export const SortClauseInput = z.strictObject({ field: z.string(), direction: z.enum(['asc', 'desc']).optional(), }); -export const PageInput = z.object({ +export const PageInput = z.strictObject({ limit: z.number().int().positive(), offset: z.number().int().nonnegative(), }); @@ -25,21 +16,30 @@ export const SearchInput = z.string(); export const SearchExtendedInput = z.boolean(); -export const TimezoneInput = z.string(); +export const TimezoneInput = z.string().regex(/\S/); export const ParentIdInput = z.union([z.string().regex(/\S/), z.number()]); -/** Everything a list body carries except `filter`, which is validated separately. */ -export const ListFlatInputs = z.object({ +const validatedElsewhere = z.unknown().optional(); + +export const ListFlatInputs = z.strictObject({ + filter: validatedElsewhere, projection: ProjectionInput.optional(), sort: z.array(SortClauseInput).optional(), page: PageInput.optional(), search: SearchInput.optional(), searchExtended: SearchExtendedInput.optional(), + timezone: TimezoneInput.optional(), }); /** A count body carries no projection, sort or page. */ -export const CountFlatInputs = z.object({ +export const CountFlatInputs = z.strictObject({ + filter: validatedElsewhere, search: SearchInput.optional(), searchExtended: SearchExtendedInput.optional(), + timezone: TimezoneInput.optional(), }); + +export const RelationListFlatInputs = ListFlatInputs.extend({ parentId: validatedElsewhere }); + +export const RelationCountFlatInputs = CountFlatInputs.extend({ parentId: validatedElsewhere }); diff --git a/packages/agent-bff/src/openapi/schemas.ts b/packages/agent-bff/src/openapi/schemas.ts index 9f1aa14cb0..39463212eb 100644 --- a/packages/agent-bff/src/openapi/schemas.ts +++ b/packages/agent-bff/src/openapi/schemas.ts @@ -2,9 +2,12 @@ import { allOperators } from '@forestadmin/datasource-toolkit'; import { z } from './zod-openapi'; import { + CountFlatInputs, + ListFlatInputs, PageInput, ParentIdInput, - ProjectionInput, + RelationCountFlatInputs, + RelationListFlatInputs, SearchExtendedInput, SearchInput, SortClauseInput, @@ -16,7 +19,7 @@ import { MAX_FILTER_DEPTH } from '../validation/capabilities-validator'; const OPERATORS = [...allOperators] as [string, ...string[]]; const ConditionTreeLeafSchema = z - .object({ + .strictObject({ field: z.string(), operator: z.enum(OPERATORS), value: z.unknown().optional(), @@ -31,7 +34,7 @@ const ConditionTreeSchema: z.ZodType = z .lazy(() => z.union([ ConditionTreeLeafSchema, - z.object({ + z.strictObject({ aggregator: z.enum(['And', 'Or']), conditions: z.array(ConditionTreeSchema), }), @@ -64,8 +67,9 @@ export const PageSchema = PageInput.openapi('Page', { export const TimezoneSchema = TimezoneInput.openapi('Timezone', { description: 'Used when the X-Forest-Timezone header is absent. The header wins when both are sent. A ' + - 'deployment with no configured default rejects a request carrying neither with 400 ' + - 'missing_timezone.', + 'blank or whitespace-only string is rejected with 400 rather than silently resolving from ' + + 'the header or the default. A deployment with no configured default rejects a request ' + + 'carrying neither with 400 missing_timezone.', }); export const SearchSchema = SearchInput.openapi('Search', { @@ -90,48 +94,64 @@ export const SearchExtendedSchema = SearchExtendedInput.openapi('SearchExtended' '`relation.column:value` syntax, which does so without this flag.', }); -export const ListRequestSchema = z - .object({ - filter: ConditionTreeSchema.optional(), - projection: ProjectionInput.optional(), - sort: z.array(SortClauseSchema).optional(), - page: PageSchema.optional(), - search: SearchSchema.optional(), - searchExtended: SearchExtendedSchema.optional(), - timezone: TimezoneSchema.optional(), - }) - .openapi('ListRequest'); - -export const CountRequestSchema = z - .object({ - filter: ConditionTreeSchema.optional(), - search: SearchSchema.optional(), - searchExtended: SearchExtendedSchema.optional(), - timezone: TimezoneSchema.optional(), - }) - .openapi('CountRequest', { - description: - 'Accepts the same search inputs as list, so a client can count exactly the rows its search ' + - 'returns.', - }); - const ParentIdSchema = ParentIdInput.openapi('ParentId', { description: 'The parent record id, opaque: a composite or packed id must be passed unchanged. A ' + 'blank string is rejected.', }); -export const RelationListRequestSchema = ListRequestSchema.extend({ - parentId: ParentIdSchema, -}).openapi('RelationListRequest', { +export const CLOSED_BODY_NOTE = + 'The runtime rejects an undeclared key with 400 invalid_request, at the top level and inside ' + + 'the filter tree alike: `filters` instead of `filter` is an error rather than a silently ' + + 'unfiltered result, and a leaf carrying `valu` instead of `value` is rejected rather than run ' + + 'with its value dropped.'; + +type OverridesOf = Partial>; + +const countOverrides = { + filter: ConditionTreeSchema.optional(), + search: SearchSchema.optional(), + searchExtended: SearchExtendedSchema.optional(), + timezone: TimezoneSchema.optional(), +} satisfies OverridesOf; + +const listOverrides = { + ...countOverrides, + sort: z.array(SortClauseSchema).optional(), + page: PageSchema.optional(), +} satisfies OverridesOf; + +export const ListRequestSchema = ListFlatInputs.extend(listOverrides).openapi('ListRequest', { + description: CLOSED_BODY_NOTE, +}); + +export const CountRequestSchema = CountFlatInputs.extend(countOverrides).openapi('CountRequest', { description: - 'Filter, sort, projection and search apply to the FOREIGN collection; the parent only resolves ' + - 'which records are related.', + 'Accepts the same search inputs as list, so a client can count exactly the rows its search ' + + `returns. ${CLOSED_BODY_NOTE}`, }); -export const RelationCountRequestSchema = CountRequestSchema.extend({ +const relationListOverrides = { + ...listOverrides, parentId: ParentIdSchema, -}).openapi('RelationCountRequest'); +} satisfies OverridesOf; + +const relationCountOverrides = { + ...countOverrides, + parentId: ParentIdSchema, +} satisfies OverridesOf; + +export const RelationListRequestSchema = RelationListFlatInputs.extend( + relationListOverrides, +).openapi('RelationListRequest', { + description: + 'Filter, sort, projection and search apply to the FOREIGN collection; the parent only ' + + `resolves which records are related. ${CLOSED_BODY_NOTE}`, +}); + +export const RelationCountRequestSchema = RelationCountFlatInputs.extend( + relationCountOverrides, +).openapi('RelationCountRequest', { description: CLOSED_BODY_NOTE }); export const ActionRequestSchema = z .object({ diff --git a/packages/agent-bff/src/openapi/unfolded-paths.ts b/packages/agent-bff/src/openapi/unfolded-paths.ts index 6886878bfe..6b61951890 100644 --- a/packages/agent-bff/src/openapi/unfolded-paths.ts +++ b/packages/agent-bff/src/openapi/unfolded-paths.ts @@ -16,11 +16,14 @@ import type { ReferenceObject, SchemaObject } from 'openapi3-ts/oas31'; import toFieldSchema from './field-schemas'; import createNamer from './names'; import { + CLOSED_BODY_NOTE, CountResponseSchema, ListResponseSchema, OPERATORS, PageSchema, ParentIdSchema, + SearchExtendedSchema, + SearchSchema, SortClauseSchema, TimezoneSchema, } from './schemas'; @@ -44,10 +47,18 @@ interface RequestRefs { count: ReferenceObject; } +type BodyProperties = Record; + +interface RequestProperties { + list: BodyProperties; + count: BodyProperties; +} + interface CollectionPlan { collection: UnfoldedCollection; key: string; requests: RequestRefs; + properties: RequestProperties; } interface Deps { @@ -116,12 +127,6 @@ function groupByOperators(filterable: FilterableField[]): OperatorGroup[] { return [...groups.values()]; } -// A node readable as BOTH a leaf and a branch is rejected with 400 (`agent-query.ts` -// `assertNoNodeReadableAsBothLeafAndBranch`), so each alternative excludes the other's discriminator. -// The exclusion carries the TYPE the runtime looks for — `isBranch` needs an ARRAY `conditions` and -// `isLeaf` a STRING `field` — because `{field, operator, conditions: "x"}` is a plain leaf the runtime -// accepts. Expressed as `not: { required }` rather than `additionalProperties: false`, which would -// also forbid an unknown extra key the runtime strips. function leafShape( field: ReferenceObject | SchemaObject, operators: string[], @@ -136,7 +141,7 @@ function leafShape( value: {}, }, required: ['field', 'operator'], - not: { properties: { conditions: { type: 'array' } }, required: ['conditions'] }, + additionalProperties: false, }; } @@ -209,7 +214,7 @@ function filterSchema( conditions: { type: 'array', items: treeRef }, }, required: ['aggregator', 'conditions'], - not: { properties: { field: { type: 'string' } }, required: ['field'] }, + additionalProperties: false, }, ], }); @@ -250,6 +255,7 @@ function fieldRefs(deps: Deps, plan: Pick) direction: { type: 'string', enum: ['asc', 'desc'] }, }, required: ['field'], + additionalProperties: false, }), }; } @@ -257,14 +263,35 @@ function fieldRefs(deps: Deps, plan: Pick) function requestDescription(collection: UnfoldedCollection, subject: string): string { const note = collection.fields.degraded ? ` ${DEGRADED_NOTE[collection.fields.degraded]}` : ''; - return `${subject}${note}`; + return `${subject}${note} ${CLOSED_BODY_NOTE}`; } -function registerRequests(deps: Deps, plan: Omit): RequestRefs { +function requestProperties(deps: Deps, plan: Pick) { const { pool } = deps; - const { collection, key } = plan; const refs = fieldRefs(deps, plan); const timezone = pool.reuse('Timezone', TimezoneSchema); + const search = pool.reuse('Search', SearchSchema); + const searchExtended = pool.reuse('SearchExtended', SearchExtendedSchema); + const count: BodyProperties = { filter: refs.filter, search, searchExtended, timezone }; + + return { + count, + list: { + ...count, + projection: { type: 'array', items: refs.projectable }, + sort: { type: 'array', items: refs.sort }, + page: pool.reuse('Page', PageSchema), + } satisfies BodyProperties, + }; +} + +function registerRequests( + deps: Deps, + plan: Pick, + properties: RequestProperties, +): RequestRefs { + const { pool } = deps; + const { collection, key } = plan; return { list: pool.add(`ListRequest_${key}`, { @@ -273,13 +300,8 @@ function registerRequests(deps: Deps, plan: Omit): R collection, `Filter, sort and project records of ${quoted(collection.name)}.`, ), - properties: { - filter: refs.filter, - projection: { type: 'array', items: refs.projectable }, - sort: { type: 'array', items: refs.sort }, - page: pool.reuse('Page', PageSchema), - timezone, - }, + properties: properties.list, + additionalProperties: false, }), count: pool.add(`CountRequest_${key}`, { type: 'object', @@ -287,7 +309,8 @@ function registerRequests(deps: Deps, plan: Omit): R collection, `Count records of ${quoted(collection.name)} matching a filter.`, ), - properties: { filter: refs.filter, timezone }, + properties: properties.count, + additionalProperties: false, }), }; } @@ -341,25 +364,27 @@ function registerRelationRequests( ): RequestRefs { const { pool } = deps; const parentId = parentIdSchema(pool, plan.collection.name, plan.collection.primaryKeys); - const parentProperties = { - type: 'object' as const, - properties: { parentId }, + const { degraded } = foreign.collection.fields; + const foreignNote = degraded ? ` ${DEGRADED_NOTE[degraded]}` : ''; + const appliesTo = + `apply to ${quoted(foreign.collection.name)}, the foreign collection of ` + + `${quoted(plan.collection.name)}.${quoted(relation.name)}; the parent only resolves which ` + + 'records are related.'; + + const body = (inputs: 'list' | 'count', subject: string): SchemaObject => ({ + type: 'object', + description: `${subject} ${appliesTo}${foreignNote} ${CLOSED_BODY_NOTE}`, + properties: { ...foreign.properties[inputs], parentId }, required: ['parentId'], - }; - const description = - `Filter, sort and projection apply to ${quoted(foreign.collection.name)}, the foreign ` + - `collection of ${quoted(plan.collection.name)}.${quoted(relation.name)}; the parent only ` + - 'resolves which records are related.'; + additionalProperties: false, + }); return { - list: pool.add(`RelationListRequest_${relationKey}`, { - description, - allOf: [foreign.requests.list, parentProperties], - }), - count: pool.add(`RelationCountRequest_${relationKey}`, { - description, - allOf: [foreign.requests.count, parentProperties], - }), + list: pool.add( + `RelationListRequest_${relationKey}`, + body('list', 'Filter, sort, projection and search'), + ), + count: pool.add(`RelationCountRequest_${relationKey}`, body('count', 'Filter and search')), }; } @@ -588,7 +613,14 @@ export default function registerUnfoldedPaths(deps: Deps, unfolding: Unfolding): const plans = unfolding.collections.map(collection => { const key = collections(collection.name); - return { collection, key, requests: registerRequests(deps, { collection, key }) }; + const properties = requestProperties(deps, { collection, key }); + + return { + collection, + key, + properties, + requests: registerRequests(deps, { collection, key }, properties), + }; }); const plansByName = new Map(plans.map(plan => [plan.collection.name, plan])); diff --git a/packages/agent-bff/test/data/agent-query.test.ts b/packages/agent-bff/test/data/agent-query.test.ts index 0e48abc991..26accaad78 100644 --- a/packages/agent-bff/test/data/agent-query.test.ts +++ b/packages/agent-bff/test/data/agent-query.test.ts @@ -11,6 +11,20 @@ import { parseRelationListRequest, } from '../../src/data/agent-query'; +type Parser = [string, (body: unknown) => unknown]; + +const logger = jest.fn(); + +const FLAT_PARSERS: Parser[] = [ + ['parseListRequest', body => parseListRequest(body, logger)], + ['parseCountRequest', body => parseCountRequest(body, logger)], +]; + +const RELATION_PARSERS: Parser[] = [ + ['parseRelationListRequest', body => parseRelationListRequest(body, logger)], + ['parseRelationCountRequest', body => parseRelationCountRequest(body, logger)], +]; + describe('buildListAgentQuery', () => { it('should always pass the resolved timezone', () => { expect(buildListAgentQuery('users', 'America/New_York', {})).toEqual({ @@ -175,7 +189,7 @@ describe('parseListRequest', () => { page: { limit: 10, offset: 0 }, }; - expect(parseListRequest(body)).toBe(body); + expect(parseListRequest(body, logger)).toBe(body); }); it.each([ @@ -191,13 +205,13 @@ describe('parseListRequest', () => { ['a non-integer page.limit', { page: { limit: 2.5, offset: 0 } }], ['a negative page.offset', { page: { limit: 10, offset: -10 } }], ])('should reject %s with 400 invalid_request', (_label, body) => { - expect(() => parseListRequest(body)).toThrow( + expect(() => parseListRequest(body, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }); it('should pass search and searchExtended through rather than strip them', () => { - expect(parseListRequest({ search: 'ada', searchExtended: true })).toMatchObject({ + expect(parseListRequest({ search: 'ada', searchExtended: true }, logger)).toMatchObject({ search: 'ada', searchExtended: true, }); @@ -209,13 +223,13 @@ describe('parseListRequest', () => { ['projection.0', { projection: [1] }], ['searchExtended', { searchExtended: 'true' }], ])('should name %s in the rejection message', (path, body) => { - expect(() => parseListRequest(body)).toThrow( + expect(() => parseListRequest(body, logger)).toThrow( expect.objectContaining({ message: expect.stringContaining(`${path}: `) }), ); }); it('should accept a blank search rather than rejecting a cleared search box', () => { - expect(parseListRequest({ search: ' ' })).toMatchObject({ search: ' ' }); + expect(parseListRequest({ search: ' ' }, logger)).toMatchObject({ search: ' ' }); }); it.each([ @@ -223,7 +237,7 @@ describe('parseListRequest', () => { ['a null search', { search: null }], ['an array search', { search: ['ada'] }], ])('should reject %s with 400 invalid_request', (_label, body) => { - expect(() => parseListRequest(body)).toThrow( + expect(() => parseListRequest(body, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }); @@ -237,7 +251,7 @@ describe('parseListRequest', () => { ])( 'should reject searchExtended sent as %s rather than coercing it like the agent does', (_label, body) => { - expect(() => parseListRequest(body)).toThrow( + expect(() => parseListRequest(body, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }, @@ -246,7 +260,7 @@ describe('parseListRequest', () => { describe('parseCountRequest', () => { it('should reject a string filter with 400 invalid_request', () => { - expect(() => parseCountRequest({ filter: 'id' })).toThrow( + expect(() => parseCountRequest({ filter: 'id' }, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }); @@ -256,13 +270,13 @@ describe('parseCountRequest', () => { ['a string', 'foo'], ['an array', []], ])('should reject a non-object body (%s) with 400 invalid_request', (_label, body) => { - expect(() => parseCountRequest(body)).toThrow( + expect(() => parseCountRequest(body, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }); it('should pass search and searchExtended through rather than strip them', () => { - expect(parseCountRequest({ search: 'ada', searchExtended: false })).toMatchObject({ + expect(parseCountRequest({ search: 'ada', searchExtended: false }, logger)).toMatchObject({ search: 'ada', searchExtended: false, }); @@ -272,7 +286,7 @@ describe('parseCountRequest', () => { ['a non-string search', { search: 42 }], ['a non-boolean searchExtended', { search: 'ada', searchExtended: 'true' }], ])('should reject %s with 400 invalid_request', (_label, body) => { - expect(() => parseCountRequest(body)).toThrow( + expect(() => parseCountRequest(body, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }); @@ -286,10 +300,7 @@ describe('a filter node readable as both a leaf and a branch', () => { conditions: [], }; - it.each([ - ['parseListRequest', parseListRequest], - ['parseCountRequest', parseCountRequest], - ])('should reject it in %s with 400 invalid_request', (_label, parse) => { + it.each(FLAT_PARSERS)('should reject it in %s with 400 invalid_request', (_label, parse) => { expect(() => parse({ filter: READABLE_AS_BOTH })).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); @@ -297,7 +308,7 @@ describe('a filter node readable as both a leaf and a branch', () => { it('should reject it nested inside a legitimate branch', () => { expect(() => - parseListRequest({ filter: { aggregator: 'And', conditions: [READABLE_AS_BOTH] } }), + parseListRequest({ filter: { aggregator: 'And', conditions: [READABLE_AS_BOTH] } }, logger), ).toThrow(expect.objectContaining({ type: 'invalid_request', status: 400 })); }); @@ -308,9 +319,9 @@ describe('a filter node readable as both a leaf and a branch', () => { it('should still accept a plain leaf and a plain branch', () => { const leaf = { field: 'title', operator: 'Present' }; - expect(() => parseCountRequest({ filter: leaf })).not.toThrow(); + expect(() => parseCountRequest({ filter: leaf }, logger)).not.toThrow(); expect(() => - parseListRequest({ filter: { aggregator: 'And', conditions: [leaf] } }), + parseListRequest({ filter: { aggregator: 'And', conditions: [leaf] } }, logger), ).not.toThrow(); }); @@ -321,7 +332,7 @@ describe('a filter node readable as both a leaf and a branch', () => { filter = { aggregator: 'And', conditions: [filter] }; } - expect(() => parseCountRequest({ filter })).toThrow( + expect(() => parseCountRequest({ filter }, logger)).toThrow( expect.objectContaining({ type: 'filter_too_deep', status: 400, @@ -331,16 +342,70 @@ describe('a filter node readable as both a leaf and a branch', () => { }); }); +describe('a filter node carrying an unknown key', () => { + it.each(FLAT_PARSERS)('should reject a misspelled leaf value in %s', (_label, parse) => { + expect(() => parse({ filter: { field: 'title', operator: 'Equal', valu: 'x' } })).toThrow( + expect.objectContaining({ + type: 'invalid_request', + status: 400, + message: 'A filter node cannot carry "valu"', + }), + ); + }); + + it('should reject a misspelled leaf value nested inside a branch', () => { + expect(() => + parseListRequest( + { + filter: { + aggregator: 'And', + conditions: [{ field: 'title', operator: 'Equal', valu: 'x' }], + }, + }, + logger, + ), + ).toThrow(expect.objectContaining({ type: 'invalid_request', status: 400 })); + }); + + it('should reject a misspelled aggregator on a branch', () => { + expect(() => + parseCountRequest({ filter: { agregator: 'And', conditions: [] } }, logger), + ).toThrow( + expect.objectContaining({ + type: 'invalid_request', + status: 400, + message: 'A filter node cannot carry "agregator"', + }), + ); + }); + + it('should reject a node readable as neither a leaf nor a branch', () => { + expect(() => + parseCountRequest({ filter: { feild: 'title', operator: 'Equal', value: 'x' } }, logger), + ).toThrow(expect.objectContaining({ type: 'invalid_request', status: 400 })); + }); + + it('should accept an empty filter object, which is how an absent filter is spelled', () => { + expect(() => parseCountRequest({ filter: {} }, logger)).not.toThrow(); + }); + + it('should accept a leaf carrying field, operator and value', () => { + expect(() => + parseCountRequest({ filter: { field: 'title', operator: 'Equal', value: 'x' } }, logger), + ).not.toThrow(); + }); +}); + describe('parseParentId', () => { it('should return a non-empty string unchanged, including a composite packed id', () => { - expect(parseParentId('a|b')).toBe('a|b'); - expect(parseParentId('550e8400-e29b-41d4-a716-446655440000')).toBe( + expect(parseParentId('a|b', logger)).toBe('a|b'); + expect(parseParentId('550e8400-e29b-41d4-a716-446655440000', logger)).toBe( '550e8400-e29b-41d4-a716-446655440000', ); }); it('should coerce a finite number to a string', () => { - expect(parseParentId(42)).toBe('42'); + expect(parseParentId(42, logger)).toBe('42'); }); it.each([ @@ -354,7 +419,7 @@ describe('parseParentId', () => { ['NaN', NaN], ['Infinity', Infinity], ])('should reject %s with invalid_request', (_label, value) => { - expect(() => parseParentId(value)).toThrow( + expect(() => parseParentId(value, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }); @@ -362,20 +427,20 @@ describe('parseParentId', () => { describe('parseRelationListRequest', () => { it('should return the validated list body with the parsed parentId', () => { - expect(parseRelationListRequest({ parentId: 'a|b', projection: ['id'] })).toEqual({ + expect(parseRelationListRequest({ parentId: 'a|b', projection: ['id'] }, logger)).toEqual({ parentId: 'a|b', projection: ['id'], }); }); it('should reject a missing parentId with 400 invalid_request', () => { - expect(() => parseRelationListRequest({ projection: ['id'] })).toThrow( + expect(() => parseRelationListRequest({ projection: ['id'] }, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }); it('should reject an invalid list body with 400 invalid_request', () => { - expect(() => parseRelationListRequest({ parentId: '7', projection: 'id' })).toThrow( + expect(() => parseRelationListRequest({ parentId: '7', projection: 'id' }, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }); @@ -384,17 +449,164 @@ describe('parseRelationListRequest', () => { describe('parseRelationCountRequest', () => { it('should return the validated count body with the parsed parentId', () => { expect( - parseRelationCountRequest({ parentId: '7', filter: { field: 'a', operator: 'present' } }), + parseRelationCountRequest( + { parentId: '7', filter: { field: 'a', operator: 'present' } }, + logger, + ), ).toEqual({ parentId: '7', filter: { field: 'a', operator: 'present' } }); }); it('should reject a missing parentId with 400 invalid_request', () => { - expect(() => parseRelationCountRequest({})).toThrow( + expect(() => parseRelationCountRequest({}, logger)).toThrow( expect.objectContaining({ type: 'invalid_request', status: 400 }), ); }); }); +describe('unknown keys', () => { + const REJECTED = expect.objectContaining({ type: 'invalid_request', status: 400 }); + + it.each(FLAT_PARSERS)( + 'should reject a misspelled filter on %s rather than run unfiltered', + (_label, parse) => { + expect(() => parse({ filters: { field: 'plan', operator: 'Equal', value: 'free' } })).toThrow( + REJECTED, + ); + }, + ); + + it.each(FLAT_PARSERS)('should name the unknown key in the %s rejection', (_label, parse) => { + expect(() => parse({ totallyUnknownField: 123 })).toThrow( + expect.objectContaining({ message: expect.stringContaining('totallyUnknownField') }), + ); + }); + + it('should reject a misspelled projection rather than return every field', () => { + expect(() => parseListRequest({ projections: ['id'] }, logger)).toThrow(REJECTED); + }); + + it('should reject a list-only key on a count body, where paging and projection mean nothing', () => { + expect(() => parseCountRequest({ projection: ['id'] }, logger)).toThrow(REJECTED); + expect(() => parseCountRequest({ sort: [{ field: 'id' }] }, logger)).toThrow(REJECTED); + expect(() => parseCountRequest({ page: { limit: 10, offset: 0 } }, logger)).toThrow(REJECTED); + }); + + it('should reject parentId on a plain list, where a parent id means nothing', () => { + expect(() => parseListRequest({ parentId: '7' }, logger)).toThrow(REJECTED); + }); + + it.each(RELATION_PARSERS)('should reject a misspelled filter on %s', (_label, parse) => { + expect(() => parse({ parentId: '7', filters: { field: 'a', operator: 'present' } })).toThrow( + REJECTED, + ); + }); + + it.each(RELATION_PARSERS)('should still accept parentId on %s', (_label, parse) => { + expect(parse({ parentId: '7' })).toMatchObject({ parentId: '7' }); + }); + + it.each(FLAT_PARSERS)( + 'should still accept the timezone the middleware reads on %s', + (_label, parse) => { + expect(parse({ timezone: 'Europe/Paris' })).toMatchObject({ timezone: 'Europe/Paris' }); + }, + ); + + it.each(FLAT_PARSERS)( + 'should reject a non-string timezone on %s rather than resolve one the caller did not send', + (_label, parse) => { + expect(() => parse({ timezone: null })).toThrow(REJECTED); + expect(() => parse({ timezone: 42 })).toThrow(REJECTED); + }, + ); + + it.each(FLAT_PARSERS)( + 'should reject a blank timezone on %s rather than silently resolve another one', + (_label, parse) => { + expect(() => parse({ timezone: '' })).toThrow(REJECTED); + expect(() => parse({ timezone: ' ' })).toThrow(REJECTED); + }, + ); + + it.each(FLAT_PARSERS)('should log the rejected key on %s, never the value', (_label, parse) => { + logger.mockClear(); + expect(() => parse({ filters: { value: 'secret' } })).toThrow(REJECTED); + expect(logger).toHaveBeenCalledWith('Warn', 'Request body rejected', { + reason: 'Unrecognized key: "filters"', + }); + expect(JSON.stringify(logger.mock.calls)).not.toContain('secret'); + }); + + it.each(FLAT_PARSERS)( + 'should log a stray key inside the filter tree on %s, never the value', + (_label, parse) => { + logger.mockClear(); + expect(() => + parse({ filter: { field: 'title', operator: 'Equal', valu: 'secret' } }), + ).toThrow(REJECTED); + expect(logger).toHaveBeenCalledWith('Warn', 'Request body rejected', { + reason: 'A filter node cannot carry "valu"', + }); + expect(JSON.stringify(logger.mock.calls)).not.toContain('secret'); + }, + ); + + it.each([ + ['a non-object body', 'nope', 'Request body must be an object'], + ['a non-object filter', { filter: 'id' }, 'filter must be an object'], + ])('should log %s too, not only zod rejections', (_label, body, reason) => { + logger.mockClear(); + expect(() => parseListRequest(body, logger)).toThrow(REJECTED); + expect(logger).toHaveBeenCalledWith('Warn', 'Request body rejected', { reason }); + }); + + it.each(RELATION_PARSERS)( + 'should log a parentId failure on %s, which the schema never sees', + (_label, parse) => { + logger.mockClear(); + expect(() => parse({ projection: ['id'] })).toThrow(REJECTED); + expect(logger).toHaveBeenCalledWith('Warn', 'Request body rejected', { + reason: 'parentId is required and must be a non-empty string or a number', + }); + }, + ); + + it('should log a filter nested past the depth cap', () => { + let filter: unknown = { field: 'title', operator: 'Present' }; + + for (let i = 0; i <= MAX_PARSED_FILTER_DEPTH; i += 1) { + filter = { aggregator: 'And', conditions: [filter] }; + } + + logger.mockClear(); + expect(() => parseCountRequest({ filter }, logger)).toThrow( + expect.objectContaining({ type: 'filter_too_deep' }), + ); + expect(logger).toHaveBeenCalledWith('Warn', 'Request body rejected', { + reason: `Filter nesting exceeds the maximum depth of ${MAX_PARSED_FILTER_DEPTH}`, + }); + }); + + it('should name the unrecognized key even when another issue comes first', () => { + logger.mockClear(); + expect(() => parseListRequest({ filters: {}, page: { limit: 0, offset: 0 } }, logger)).toThrow( + expect.objectContaining({ message: expect.stringContaining('filters') }), + ); + }); + + it('should reject a misspelled sort direction rather than sort ascending', () => { + expect(() => + parseListRequest({ sort: [{ field: 'createdAt', direciton: 'desc' }] }, logger), + ).toThrow(REJECTED); + }); + + it('should reject an unknown key inside page', () => { + expect(() => parseListRequest({ page: { limit: 10, offset: 0, cursor: 'x' } }, logger)).toThrow( + REJECTED, + ); + }); +}); + describe('collectCountFieldPaths', () => { it('should collect field paths from the filter only', () => { const paths = collectCountFieldPaths({ diff --git a/packages/agent-bff/test/data/data-routes-middleware.test.ts b/packages/agent-bff/test/data/data-routes-middleware.test.ts index f52e5bb466..a13974706b 100644 --- a/packages/agent-bff/test/data/data-routes-middleware.test.ts +++ b/packages/agent-bff/test/data/data-routes-middleware.test.ts @@ -384,6 +384,23 @@ describe('data routes middleware', () => { expect(list).not.toHaveBeenCalled(); }); + it('should return 400 for a misspelled filter instead of listing every record', async () => { + const list = jest.fn(); + const app = buildApp(storeOf(usersReadModel), { list }); + + const response = await request(app.callback()) + .post('/agent/v1/users/list') + .send({ filters: { field: 'email', operator: 'present' } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatchObject({ + type: 'invalid_request', + status: 400, + message: expect.stringContaining('filters'), + }); + expect(list).not.toHaveBeenCalled(); + }); + it('should expose a Number primary key as a number in __forest.primaryKey', async () => { const numericPkReadModel = new ReadModel([ collection('metrics', [{ ...column('id'), type: 'Number' }]), diff --git a/packages/agent-bff/test/openapi/openapi-document.test.ts b/packages/agent-bff/test/openapi/openapi-document.test.ts index 6944850481..ffab6d6a9f 100644 --- a/packages/agent-bff/test/openapi/openapi-document.test.ts +++ b/packages/agent-bff/test/openapi/openapi-document.test.ts @@ -274,22 +274,6 @@ describe('generateOpenApiDocument', () => { expect(messageless.properties.error.required).toEqual(['type', 'status']); }); - it('should say on the form response that htmlBlock layout content is sanitized server-side', () => { - const form = responsesOf(`${ROUTE_PREFIX}/{collection}/actions/{action}/form`); - - expect(form['200'].description).toContain( - 'htmlBlock layout content is sanitized server-side against an allowlist', - ); - }); - - it('should say on the execute response that a success html is sanitized server-side', () => { - const execute = responsesOf(`${ROUTE_PREFIX}/{collection}/actions/{action}/execute`); - - expect(execute['200'].description).toContain( - 'html field is sanitized server-side against an allowlist', - ); - }); - it('should keep a plain error body for the 501 the agent stub returns on other routes', () => { expect(listResponses()['501'].content?.['application/json'].schema.$ref).toBe( '#/components/schemas/ErrorResponse', @@ -434,6 +418,10 @@ describe('generateOpenApiDocument', () => { expect(Object.keys(schemas.ActionRequest.properties as object)).toContain('timezone'); }); + it('should publish the blank-timezone rejection in the schema, not only at runtime', () => { + expect(schemas.Timezone).toEqual(expect.objectContaining({ type: 'string', pattern: '\\S' })); + }); + it('should declare Retry-After on 503, the only status that sets it', () => { const list = listResponses(); @@ -477,6 +465,39 @@ describe('generateOpenApiDocument', () => { }); }); +describe('the closed request bodies', () => { + it.each([ + ['ListRequest'], + ['CountRequest'], + ['RelationListRequest'], + ['RelationCountRequest'], + ['SortClause'], + ['Page'], + ])('should forbid an undeclared key on %s', name => { + expect(schemas[name].additionalProperties).toBe(false); + }); + + it('should forbid an undeclared key on both shapes of a condition tree node', () => { + const { anyOf } = schemas.ConditionTree as unknown as { anyOf: unknown[] }; + const [leaf, branch] = anyOf; + + expect(leaf).toEqual({ $ref: '#/components/schemas/ConditionTreeLeaf' }); + expect(branch).toEqual(expect.objectContaining({ additionalProperties: false })); + expect(schemas.ConditionTreeLeaf.additionalProperties).toBe(false); + }); + + it.each([['RelationListRequest'], ['RelationCountRequest']])( + 'should publish %s flat rather than as an allOf of a closed base', + name => { + expect(schemas[name].allOf).toBeUndefined(); + expect( + (schemas[name] as { properties: Record }).properties.parentId, + ).toEqual({ $ref: '#/components/schemas/ParentId' }); + }, + ); +}); + + describe('the documented ai query path', () => { it('should say a non-json body is rejected here, matching the 415 the guard raises', () => { const { description } = document.paths[`${ROUTE_PREFIX}/ai/query`].post as { @@ -557,25 +578,6 @@ describe('the documented search inputs', () => { }); }); -describe('the documented pagination inputs', () => { - it('should publish the default page applied when page is absent', () => { - expect(schemas.Page.description).toContain('the object itself is optional'); - expect(schemas.Page.description).toContain('the first page (offset 0)'); - expect(schemas.Page.description).toContain('a limit of 15 records on the Node agent'); - expect(schemas.Page.description).toContain('silently missing'); - }); - - it('should require both limit and offset once page is sent', () => { - expect(schemas.Page.required).toEqual(['limit', 'offset']); - }); - - it('should warn on the response that a page-less list is one page, not the collection', () => { - expect(schemas.ListResponse.description).toContain('not guaranteed to be the whole collection'); - expect(schemas.ListResponse.description).toContain('up to 15 records from the start'); - expect(schemas.ListResponse.description).toContain('probably truncated'); - }); -}); - describe('serializeOpenApi', () => { it('should produce indented JSON that parses back to the same document', () => { const serialized = serializeOpenApi(document); diff --git a/packages/agent-bff/test/openapi/openapi-unfolded.test.ts b/packages/agent-bff/test/openapi/openapi-unfolded.test.ts index e01da74a72..86e5fbccb0 100644 --- a/packages/agent-bff/test/openapi/openapi-unfolded.test.ts +++ b/packages/agent-bff/test/openapi/openapi-unfolded.test.ts @@ -81,10 +81,9 @@ function collectionOf( }; } -// A relation request composes the foreign collection request, so its filter sits one hop further. +// A relation request spreads the foreign collection properties, so its filter sits on the body. function relationFilterOf(path: string): string { - const { allOf } = requestSchema(path) as unknown as { allOf: [{ $ref: string }, unknown] }; - const { properties } = dereference(allOf[0].$ref) as unknown as { + const { properties } = requestSchema(path) as unknown as { properties: { filter: { $ref: string } }; }; @@ -140,24 +139,6 @@ describe('the unfolded document', () => { ); }); - it('should say on the form response that htmlBlock layout content is sanitized server-side', () => { - const response = operation('My%20Coll/actions/Mark%20as%20paid%2Fdone/form') - .responses as Record; - - expect(response['200'].description).toContain( - 'htmlBlock layout content is sanitized server-side against an allowlist', - ); - }); - - it('should say on the execute response that a success html is sanitized server-side', () => { - const response = operation('My%20Coll/actions/Mark%20as%20paid%2Fdone/execute') - .responses as Record; - - expect(response['200'].description).toContain( - 'html field is sanitized server-side against an allowlist', - ); - }); - it('should name the exact action in prose, since the operationId cannot carry it', () => { expect(operation('My%20Coll/actions/Mark%20as%20paid%2Fdone/form').description).toContain( 'exactly "Mark as paid/done"', @@ -210,6 +191,7 @@ describe('the unfolded document', () => { expect(request.properties.sort.items?.$ref).toBe('#/components/schemas/SortClause_My_Coll'); }); + it('should leave page optional here too, pointing at the shared Page component', () => { const request = requestSchema('My%20Coll/list') as unknown as { required?: string[]; @@ -220,60 +202,132 @@ describe('the unfolded document', () => { expect(request.properties.page.$ref).toBe('#/components/schemas/Page'); }); - it('should make every leaf and the branch mutually exclusive, which the runtime enforces', () => { - const branch = branchOf('Filter_My_Coll') as unknown as { not: { required: string[] } }; + it('should publish search and searchExtended on list and count, which the runtime accepts', () => { + const list = requestSchema('My%20Coll/list') as unknown as { + properties: Record; + }; + const count = requestSchema('My%20Coll/count') as unknown as { + properties: Record; + }; - ['FilterLeaf_My_Coll-1', 'FilterLeaf_My_Coll-2'].forEach(name => { - const leaf = schemas[name] as unknown as { not: { required: string[] } }; + expect(list.properties.search.$ref).toBe('#/components/schemas/Search'); + expect(list.properties.searchExtended.$ref).toBe('#/components/schemas/SearchExtended'); + expect(count.properties.search.$ref).toBe('#/components/schemas/Search'); + expect(count.properties.searchExtended.$ref).toBe('#/components/schemas/SearchExtended'); + }); - expect(leaf.not.required).toEqual(['conditions']); + it('should close every leaf and the branch of a filter tree, so a stray key is refused before it leaves', () => { + ['FilterLeaf_My_Coll-1', 'FilterLeaf_My_Coll-2', 'FilterLeaf_orders'].forEach(name => { + expect((schemas[name] as { additionalProperties?: boolean }).additionalProperties).toBe( + false, + ); }); - expect(branch.not.required).toEqual(['field']); + expect( + (branchOf('Filter_My_Coll') as { additionalProperties?: boolean }).additionalProperties, + ).toBe(false); + }); + + it.each([ + ['My%20Coll/list'], + ['My%20Coll/count'], + ['My%20Coll/relations/orders/list'], + ['My%20Coll/relations/orders/count'], + ])( + 'should say on %s that an undeclared key is rejected, in the body and the tree alike', + path => { + const request = requestSchema(path) as unknown as { description: string }; + + expect(request.description).toContain('undeclared key with 400 invalid_request'); + expect(request.description).toContain('`valu` instead of `value` is rejected'); + }, + ); + + it('should close a per-collection sort clause, so a misspelled direction is not silently kept', () => { + const sort = schemas.SortClause_My_Coll as unknown as { additionalProperties: boolean }; + + expect(sort.additionalProperties).toBe(false); + }); + + it('should close the per-collection list and count bodies structurally', () => { + const list = requestSchema('My%20Coll/list') as unknown as { additionalProperties: boolean }; + const count = requestSchema('My%20Coll/count') as unknown as { additionalProperties: boolean }; + + expect(list.additionalProperties).toBe(false); + expect(count.additionalProperties).toBe(false); }); - it('should exclude only the type the runtime reads as the other shape', () => { - // `isBranch` needs an ARRAY `conditions` and `isLeaf` a STRING `field`, so a leaf carrying - // `conditions: "x"` is a plain leaf the runtime accepts and the document must not refuse. - const leaf = schemas['FilterLeaf_My_Coll-1'] as unknown as { - not: { properties: { conditions: { type: string } } }; + it('should describe a relation request with the FOREIGN collection fields, not the parent ones', () => { + const foreign = dereference('#/components/schemas/ListRequest_orders') as unknown as { + properties: Record; }; - const branch = branchOf('Filter_My_Coll') as unknown as { - not: { properties: { field: { type: string } } }; + const request = requestSchema('My%20Coll/relations/orders/list') as unknown as { + properties: Record; }; - expect(leaf.not.properties.conditions.type).toBe('array'); - expect(branch.not.properties.field.type).toBe('string'); + expect(Object.keys(request.properties).sort()).toEqual( + [...Object.keys(foreign.properties), 'parentId'].sort(), + ); + expect(request.properties.filter).toEqual(foreign.properties.filter); }); - it('should not forbid an unknown extra key on a filter node, which the runtime strips', () => { - const leaf = schemas['FilterLeaf_My_Coll-1'] as { additionalProperties?: unknown }; + it('should carry the foreign degraded note on a relation request, like the collection body', () => { + const relation = requestSchema('My%20Coll/relations/orders/list') as unknown as { + description: string; + }; + const foreign = requestSchema('orders/list') as unknown as { description: string }; + const note = 'The field names are NOT enumerated here'; - expect(leaf.additionalProperties).toBeUndefined(); + expect(foreign.description).toContain(note); + expect(relation.description).toContain(note); }); - it('should describe a relation request with the FOREIGN collection fields, not the parent ones', () => { + it('should not advertise list-only keys on the relation count body or its description', () => { + const list = requestSchema('My%20Coll/relations/orders/list') as unknown as { + description: string; + }; + const count = requestSchema('My%20Coll/relations/orders/count') as unknown as { + description: string; + properties: Record; + }; + + expect(list.description).toContain('Filter, sort, projection and search'); + expect(count.description).toContain('Filter and search'); + expect(Object.keys(count.properties).sort()).toEqual([ + 'filter', + 'parentId', + 'search', + 'searchExtended', + 'timezone', + ]); + }); + + it('should close a relation request structurally, not only in prose', () => { const request = requestSchema('My%20Coll/relations/orders/list') as unknown as { - allOf: [{ $ref: string }, Record]; + additionalProperties: boolean; + description: string; }; - expect(request.allOf[0].$ref).toBe('#/components/schemas/ListRequest_orders'); + expect(request.additionalProperties).toBe(false); + expect(request.description).toContain('undeclared key with 400 invalid_request'); + expect(request.description).not.toContain('does not say so structurally'); }); it('should require parentId on a relation request and name the parent key it belongs to', () => { const request = requestSchema('My%20Coll/relations/orders/list') as unknown as { - allOf: [unknown, { properties: { parentId: { description: string } }; required: string[] }]; + properties: { parentId: { description: string } }; + required: string[]; }; - expect(request.allOf[1].required).toEqual(['parentId']); - expect(request.allOf[1].properties.parentId.description).toContain('(id, Number)'); + expect(request.required).toEqual(['parentId']); + expect(request.properties.parentId.description).toContain('(id, Number)'); }); it('should accept a numeric parent id as a number or its string form, like the runtime', () => { const request = requestSchema('My%20Coll/relations/orders/list') as unknown as { - allOf: [unknown, { properties: { parentId: { anyOf: unknown[] } } }]; + properties: { parentId: { anyOf: unknown[] } }; }; - expect(request.allOf[1].properties.parentId.anyOf).toEqual([ + expect(request.properties.parentId.anyOf).toEqual([ { type: 'string', pattern: '\\S' }, { type: 'number' }, ]); @@ -281,21 +335,19 @@ describe('the unfolded document', () => { it('should document a composite parent key as its packed string form', () => { const request = requestSchema('orders/relations/buyers/list') as unknown as { - allOf: [unknown, { properties: { parentId: { type: string; description: string } } }]; + properties: { parentId: { type: string; description: string } }; }; - expect(request.allOf[1].properties.parentId.type).toBe('string'); - expect(request.allOf[1].properties.parentId.description).toContain( - 'shop, number joined by "|"', - ); + expect(request.properties.parentId.type).toBe('string'); + expect(request.properties.parentId.description).toContain('shop, number joined by "|"'); }); it('should fall back to the opaque parent id when the parent exposes no key metadata', () => { const request = requestSchema('users.address/relations/orders/list') as unknown as { - allOf: [unknown, { properties: { parentId: { $ref?: string } } }]; + properties: { parentId: { $ref?: string } }; }; - expect(request.allOf[1].properties.parentId.$ref).toBe('#/components/schemas/ParentId'); + expect(request.properties.parentId.$ref).toBe('#/components/schemas/ParentId'); }); it('should keep the paths of a collection whose capabilities failed, with free-form fields', () => {