diff --git a/.changeset/26954-engine-projection-empty.md b/.changeset/26954-engine-projection-empty.md new file mode 100644 index 00000000000..dc140fe2e93 --- /dev/null +++ b/.changeset/26954-engine-projection-empty.md @@ -0,0 +1,18 @@ +--- +'@objectstack/spec': patch +--- + +fix(spec)!: the engine-side projection declarations (`EngineQueryOptionsSchema.fields`, the RPC request's `select`, the legacy `DataEngineQueryOptionsSchema.select` and the vector-find request's `fields`) declare the projection their transport siblings declare, instead of any list (#26954) + +Clause-②: no (narrowing) — the published engine-side declarations of a read's projection (`EngineQueryOptionsSchema.fields`; the deprecated `select` of the RPC query options reached through `DataEngineFindRequestSchema`, `DataEngineFindOneRequestSchema` and `DataEngineRequestSchema`; the deprecated `DataEngineQueryOptionsSchema.select`; and `DataEngineVectorFindRequestSchema.fields`, also an arm of `DataEngineRequestSchema`) refuse a list naming no field, which they admitted before. Nothing is widened and no key is added, removed or renamed. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing of published declarations (`@objectstack/spec/data`). No runtime in this repository parses any of these members, so no request served here changes; what can break is code outside it that parses with these exported schemas, or a client generated from their JSON Schema, and sent an empty projection. + +- **Before.** `EngineQueryOptionsSchema.fields`, the RPC query's `select`, `DataEngineQueryOptionsSchema.select` and `DataEngineVectorFindRequestSchema.fields` admitted any list of strings: `[]`, and a list with a blank or whitespace-only name. +- **Now.** A list must hold at least one name, and no name may be blank (JSON Schema `minItems: 1`, and a non-blank pattern on each name). All four read this rule from the same declaration as `QueryTransportParamsSchema.select` / `$select` and `GetDataRequestSchema.select`, so the refusal messages are the same. +- **Unchanged.** The inferred types (`string[]`), a named projection, and leaving the projection out. The in-process ObjectQL engine does not parse its options with this schema: `engine.find(object, { fields: [] })` still reads every column, as before. + +**The remedy.** Name at least one field, or leave `fields` / `select` out to read every column. diff --git a/packages/spec/src/data/data-engine.test.ts b/packages/spec/src/data/data-engine.test.ts index 021ae7da141..6ad303fbb6e 100644 --- a/packages/spec/src/data/data-engine.test.ts +++ b/packages/spec/src/data/data-engine.test.ts @@ -25,6 +25,7 @@ import { DataEngineVectorFindRequestSchema, DataEngineRequestSchema, DroppedFieldsEventSchema, + QueryTransportParamsSchema, } from './data-engine.zod'; import { QuerySchema } from './query.zod'; @@ -1242,3 +1243,104 @@ describe('DroppedFieldsEventSchema.reason — why a write dropped submitted fiel expect(issue?.path).toEqual(['reason']); }); }); + +describe('the engine-side projection declarations admit exactly what their transport siblings admit', () => { + /** + * [#26954] `EngineQueryOptionsSchema.fields` and the RPC request's + * deprecated `select` (`DataEngineFindRequestSchema.query`, and so + * `DataEngineFindOneRequestSchema` and the `DataEngineRequestSchema` union) + * take the projection's array arm from the one declaration their transport + * siblings use: at least one name, none blank. `fields` shares + * `FieldNodeSchema` with `$select`; `select` shares `z.string()` with the + * bare `select`. So each refusal reads exactly as its sibling's does. + */ + const REFUSED: ReadonlyArray]> = [ + ['an empty list', [], []], + ['a list with a blank name', ['name', ''], [1]], + ['a list with a whitespace name', ['name', ' '], [1]], + ]; + const KEPT: ReadonlyArray = [ + ['one name', ['name']], + ['two names', ['name', 'email']], + ]; + + /** The first issue's message and path when `value` sits under `key` of a transport bag. */ + const sibling = (key: 'select' | '$select', value: unknown) => { + const parsed = QueryTransportParamsSchema.safeParse({ [key]: value }); + expect(parsed.success, `${key} sibling refuses ${JSON.stringify(value)}`).toBe(false); + return parsed.success ? undefined : parsed.error.issues[0]; + }; + + it.each(REFUSED)('EngineQueryOptionsSchema.fields refuses %s, naming the path, as `$select` does', (_name, value, tail) => { + const parsed = EngineQueryOptionsSchema.safeParse({ fields: value }); + expect(parsed.success).toBe(false); + const issue = parsed.success ? undefined : parsed.error.issues[0]; + expect(issue?.path).toEqual(['fields', ...tail]); + const twin = sibling('$select', value); + expect(issue?.message).toBe(twin?.message); + expect(twin?.path).toEqual(['$select', ...tail]); + }); + + it.each(KEPT)('EngineQueryOptionsSchema.fields keeps %s, unchanged', (_name, value) => { + expect(EngineQueryOptionsSchema.parse({ fields: value }).fields).toEqual(value); + }); + + for (const [label, schema, method] of [ + ['DataEngineFindRequestSchema', DataEngineFindRequestSchema, 'find'], + ['DataEngineFindOneRequestSchema', DataEngineFindOneRequestSchema, 'findOne'], + ['DataEngineRequestSchema', DataEngineRequestSchema, 'find'], + ] as const) { + it.each(REFUSED)(`${label}: query.select refuses %s, naming the path, as the bare \`select\` does`, (_name, value, tail) => { + const parsed = schema.safeParse({ method, object: 'account', query: { select: value } }); + expect(parsed.success).toBe(false); + const issue = parsed.success ? undefined : parsed.error.issues[0]; + expect(issue?.path).toEqual(['query', 'select', ...tail]); + expect(issue?.message).toBe(sibling('select', value)?.message); + }); + + it.each(REFUSED)(`${label}: query.fields refuses %s, naming the path`, (_name, value, tail) => { + const parsed = schema.safeParse({ method, object: 'account', query: { fields: value } }); + expect(parsed.success).toBe(false); + const issue = parsed.success ? undefined : parsed.error.issues[0]; + expect(issue?.path).toEqual(['query', 'fields', ...tail]); + expect(issue?.message).toBe(sibling('$select', value)?.message); + }); + + it.each(KEPT)(`${label}: query.select keeps %s, folded onto \`fields\``, (_name, value) => { + const parsed = schema.parse({ method, object: 'account', query: { select: value } }) as { query?: Record }; + expect(parsed.query).toEqual({ fields: value }); + }); + } + + // The legacy options bag's `select` and the vector-find request's `fields` + // are `z.string()` lists, so their sibling is the bare `select`. + it.each(REFUSED)('DataEngineQueryOptionsSchema.select refuses %s, naming the path, as the bare `select` does', (_name, value, tail) => { + const parsed = DataEngineQueryOptionsSchema.safeParse({ select: value }); + expect(parsed.success).toBe(false); + const issue = parsed.success ? undefined : parsed.error.issues[0]; + expect(issue?.path).toEqual(['select', ...tail]); + expect(issue?.message).toBe(sibling('select', value)?.message); + }); + + it.each(KEPT)('DataEngineQueryOptionsSchema.select keeps %s, unchanged', (_name, value) => { + expect(DataEngineQueryOptionsSchema.parse({ select: value }).select).toEqual(value); + }); + + for (const [label, schema] of [ + ['DataEngineVectorFindRequestSchema', DataEngineVectorFindRequestSchema], + ['DataEngineRequestSchema', DataEngineRequestSchema], + ] as const) { + it.each(REFUSED)(`${label}: vectorFind fields refuses %s, naming the path, as the bare \`select\` does`, (_name, value, tail) => { + const parsed = schema.safeParse({ method: 'vectorFind', object: 'doc', vector: [0.1, 0.2], fields: value }); + expect(parsed.success).toBe(false); + const issue = parsed.success ? undefined : parsed.error.issues[0]; + expect(issue?.path).toEqual(['fields', ...tail]); + expect(issue?.message).toBe(sibling('select', value)?.message); + }); + + it.each(KEPT)(`${label}: vectorFind fields keeps %s, unchanged`, (_name, value) => { + const parsed = schema.parse({ method: 'vectorFind', object: 'doc', vector: [0.1, 0.2], fields: value }) as { fields?: string[] }; + expect(parsed.fields).toEqual(value); + }); + } +}); diff --git a/packages/spec/src/data/data-engine.zod.ts b/packages/spec/src/data/data-engine.zod.ts index 1bf1df25e8a..028003d2bda 100644 --- a/packages/spec/src/data/data-engine.zod.ts +++ b/packages/spec/src/data/data-engine.zod.ts @@ -75,6 +75,41 @@ export const BaseEngineOptionsSchema = lazySchema(() => z.object({ context: ExecutionContextSchema.partial().optional(), })); +/** + * [#26909] The projection's accept set — the `select` / `$select` spellings + * of `fields` — as the doors that read it judge it (`projectionFieldsValue` in + * `@objectstack/metadata-protocol`, shared by the list's `findData` and the + * by-id `getData`). A comma string is split, trimmed and emptied of blank + * members there, and a value naming NO field is refused with a 400: absent + * means "every column", so an empty projection read as absent is a widening. + * + * The string arm is that rule as a pattern: `[^\s,]` holds iff some comma + * segment has a non-whitespace character, and `\s` is exactly the set + * `trim()` strips. A pattern, not a refinement, so the published JSON Schema + * states it too. The array arm is not split: the door keeps every member, + * refuses `[]`, and checks every member against the object's fields, where a + * blank name is never one. + * + * `HttpFindQueryParamsSchema.select` and `GetDataRequestSchema.select` + * (`../api/protocol.zod.ts`) read these arms off the `QueryTransportParamsSchema` + * members rather than restating them, so the three declarations of one door + * rule cannot drift apart. + * + * [#26954] The engine-side declarations of the same projection — + * `EngineQueryOptionsSchema.fields`, `RpcQueryOptionsSchema.select`, the + * legacy `DataEngineQueryOptionsSchema.select` and + * `DataEngineVectorFindRequestSchema.fields` — take the array arm from here + * too. It is declared ahead of both because + * `RpcQueryOptionsSchema` extends `EngineQueryOptionsSchema` at module load, + * which runs the lazy factory before any later `const` exists. + */ +const PROJECTION_NAMES_NO_FIELD = 'Name at least one field'; +const TransportProjectionStringSchema = z.string().regex(/[^\s,]/, PROJECTION_NAMES_NO_FIELD); +const transportProjectionArray = (member: T) => + z.array(member.regex(/\S/, 'A field name cannot be blank')).min(1, PROJECTION_NAMES_NO_FIELD); +const PROJECTION_ACCEPT_SET = 'at least one field name; a value naming no field (blank, only commas and spaces, ' + + 'an empty list, or a list with a blank name) is refused'; + // ========================================================================== // 2. method: FIND (QueryAST-aligned) // ========================================================================== @@ -101,8 +136,8 @@ export const EngineQueryOptionsSchema = lazySchema(() => BaseEngineOptionsSchema /** Filter conditions (WHERE) — standard QueryAST `where` */ where: z.union([z.record(z.string(), z.unknown()), FilterConditionSchema]).optional(), - /** Fields to retrieve (SELECT) — standard QueryAST `fields` */ - fields: z.array(FieldNodeSchema).optional(), + /** Fields to retrieve (SELECT) — standard QueryAST `fields`; at least one, none blank (#26954) */ + fields: transportProjectionArray(FieldNodeSchema).optional(), /** Sorting instructions (ORDER BY) — standard QueryAST `orderBy` */ orderBy: z.array(SortNodeSchema).optional(), @@ -179,8 +214,8 @@ export const EngineQueryOptionsSchema = lazySchema(() => BaseEngineOptionsSchema export const DataEngineQueryOptionsSchema = lazySchema(() => BaseEngineOptionsSchema.extend({ /** @deprecated Use `where` (EngineQueryOptionsSchema) */ filter: DataEngineFilterSchema.optional(), - /** @deprecated Use `fields` (EngineQueryOptionsSchema) */ - select: z.array(z.string()).optional(), + /** @deprecated Use `fields` (EngineQueryOptionsSchema) — the same projection rule (#26954) */ + select: transportProjectionArray(z.string()).optional(), /** @deprecated Use `orderBy` (EngineQueryOptionsSchema) */ sort: DataEngineSortSchema.optional(), limit: z.number().int().min(1).optional(), @@ -774,8 +809,8 @@ function foldRpcQueryOptions(input: object, ctx: z.core.$RefinementCtx): EngineQ */ const RpcQueryOptionsSchema = EngineQueryOptionsSchema.extend({ ...RpcLegacyFilterMixin, - /** @deprecated Use `fields` */ - select: z.array(z.string()).optional(), + /** @deprecated Use `fields` — the same projection rule (#26954) */ + select: transportProjectionArray(z.string()).optional(), /** @deprecated Use `orderBy` */ sort: DataEngineSortSchema.optional(), /** @deprecated Use `offset` */ @@ -907,33 +942,6 @@ const TransportFilterValueSchema = lazySchema(() => z.union([FilterArraySchema, */ const TransportCountValueSchema = lazySchema(() => z.union([z.boolean(), z.enum(['true', 'false'])])); -/** - * [#26909] The projection's accept set — the `select` / `$select` spellings - * of `fields` — as the doors that read it judge it (`projectionFieldsValue` in - * `@objectstack/metadata-protocol`, shared by the list's `findData` and the - * by-id `getData`). A comma string is split, trimmed and emptied of blank - * members there, and a value naming NO field is refused with a 400: absent - * means "every column", so an empty projection read as absent is a widening. - * - * The string arm is that rule as a pattern: `[^\s,]` holds iff some comma - * segment has a non-whitespace character, and `\s` is exactly the set - * `trim()` strips. A pattern, not a refinement, so the published JSON Schema - * states it too. The array arm is not split: the door keeps every member, - * refuses `[]`, and checks every member against the object's fields, where a - * blank name is never one. - * - * `HttpFindQueryParamsSchema.select` and `GetDataRequestSchema.select` - * (`../api/protocol.zod.ts`) read these arms off the members below rather - * than restating them, so the three declarations of one door rule cannot - * drift apart. - */ -const PROJECTION_NAMES_NO_FIELD = 'Name at least one field'; -const TransportProjectionStringSchema = z.string().regex(/[^\s,]/, PROJECTION_NAMES_NO_FIELD); -const transportProjectionArray = (member: T) => - z.array(member.regex(/\S/, 'A field name cannot be blank')).min(1, PROJECTION_NAMES_NO_FIELD); -const PROJECTION_ACCEPT_SET = 'at least one field name; a value naming no field (blank, only commas and spaces, ' - + 'an empty list, or a list with a blank name) is refused'; - /** * [#25269] The page window — `limit` / `offset` and every spelling that folds * onto them (`top`, `$top`, `skip`, `$skip`) — as the Data API list doors @@ -1513,8 +1521,8 @@ export const DataEngineVectorFindRequestSchema = lazySchema(() => z.object({ vector: z.array(z.number()), /** Optional pre-filter (Metadata filtering) — standard QueryAST `where` */ where: z.union([z.record(z.string(), z.unknown()), FilterConditionSchema]).optional(), - /** Fields to retrieve — standard QueryAST `fields` */ - fields: z.array(z.string()).optional(), + /** Fields to retrieve — standard QueryAST `fields`; the same projection rule (#26954) */ + fields: transportProjectionArray(z.string()).optional(), /** Number of results */ limit: z.number().int().default(5).optional(), /** Minimum similarity score (0-1) or distance threshold */