diff --git a/.changeset/27732-engine-empty-search-fields.md b/.changeset/27732-engine-empty-search-fields.md new file mode 100644 index 00000000000..5bd88d38ca0 --- /dev/null +++ b/.changeset/27732-engine-empty-search-fields.md @@ -0,0 +1,18 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql)!: the in-process engine refuses a search-field override that names no field, as the Data API doors and the shared declarations do (#27732) + +Clause-②: no (narrowing) — the in-process engine (`find`, `findOne`, `count`, `aggregate`) refuses a `searchFields` or object-form `search.fields` that names no field (an empty list, or a comma string with no name in it), which it read as absent before. The doors already refuse it (#27484) and `QuerySchema.searchFields` / `FullTextSearchSchema.fields` declare the refusal (#27683). Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the engine's search override (`@objectstack/objectql`). + +- **Before.** `engine.find(obj, { search: 'acme', searchFields: [] })`, the same on `findOne` / `count` / `aggregate`, and `search: { query: 'acme', fields: [] }` scanned the object's whole default search set: an empty override read as no override. The caller wrote the key to narrow and was not told. +- **Now.** Each is refused `400 VALIDATION_ERROR` (`param: 'searchFields'` or `'search.fields'`) before any driver read, whether or not a search term rides along, and for a system-context caller too. The message carries the declaration's own words ("Name at least one field, or leave the key out to search the default search fields"), and `mapDataError` serves it the status and code the REST door serves. +- **Unchanged.** An absent (or `null`) override searches the default set. A named override narrows as before, and a name outside the searchable set is still intersected away in process (refused at the doors). + +**The remedy.** Leave the key out to search the default fields, or name at least one field. diff --git a/content/docs/protocol/objectql/query-syntax.mdx b/content/docs/protocol/objectql/query-syntax.mdx index 5adedde25d1..f71d6ee24a1 100644 --- a/content/docs/protocol/objectql/query-syntax.mdx +++ b/content/docs/protocol/objectql/query-syntax.mdx @@ -1194,7 +1194,9 @@ the tolerant intersection, but not a search with nothing to scan: when no `searchableFields` entry names a stored field and the auto-default finds none (`internal: true` fields never count), the engine itself refuses a non-blank `search` with `400 INVALID_FIELD` (`param: 'search'`) on `find`, `findOne`, `count` and -`aggregate` alike, rather than answering every row. Multiple whitespace-separated terms are AND-ed and +`aggregate` alike, rather than answering every row. An override that names no field (`searchFields: []`, +`search.fields: []`, or a comma string with no name in it) is refused there too, `400 VALIDATION_ERROR`, as +at the ingress: leave the key out to search the default set. Multiple whitespace-separated terms are AND-ed and fields are OR-ed. Case sensitivity comes from the operator the expansion emits, not from the expansion: it emits `$icontains`, which ignores **ASCII case** by the rule in [Case Sensitivity](#case-sensitivity) above. So a user typing `acme` does diff --git a/packages/objectql/src/engine-empty-search-fields-refusal.test.ts b/packages/objectql/src/engine-empty-search-fields-refusal.test.ts new file mode 100644 index 00000000000..d8e9257d103 --- /dev/null +++ b/packages/objectql/src/engine-empty-search-fields-refusal.test.ts @@ -0,0 +1,223 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27732] The in-process engine refuses a search-field override that names no + * field, as the Data API doors (#27484) and the shared declarations (#27683) + * do. + * + * `searchFields` and the object-form `search.fields` narrow which columns a + * `search` scans; absent means "scan the object's default search fields". The + * door refuses a value naming no field with `400 VALIDATION_ERROR` + * (`assertSearchFieldsAreSearchable`, `@objectstack/metadata-protocol`), and + * `QuerySchema.searchFields` / `FullTextSearchSchema.fields` refuse it too. The + * engine never parses its options with those schemas, and `resolveSearchFields` + * read an empty override as absent, so `find` / `findOne` / `count` / + * `aggregate` scanned the whole default set: a caller who wrote the override + * meant to narrow, and got the widest search instead, untold. + * + * Driven against a REAL {@link ObjectQL} engine over a driver double that + * evaluates `$or` / `$icontains` and records each read, so "refused before any + * driver read" is measured rather than assumed. `harbour` hits `name` on two + * rows and `city` on a third, so a narrowing is observable. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { QuerySchema, FullTextSearchSchema } from '@objectstack/spec/data'; +import type { EngineQueryOptions, EngineCountOptions, EngineAggregateOptions } from '@objectstack/spec/data'; +import { mapDataError } from '@objectstack/types'; +import { ObjectQL } from './engine.js'; + +const account = { + name: 'acct_27732', + label: 'Account', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + city: { name: 'city', label: 'City', type: 'text' as const }, + region: { name: 'region', label: 'Region', type: 'text' as const }, + }, +}; + +const ROWS: ReadonlyArray> = [ + { id: 'a1', name: 'Harbour Freight', city: 'Boston', region: 'east' }, + { id: 'a2', name: 'Harbour Lights', city: 'Seattle', region: 'west' }, + { id: 'a3', name: 'Northgate Mills', city: 'Harbourton', region: 'east' }, + { id: 'a4', name: 'Riverside Co', city: 'Portland', region: 'west' }, +]; + +/** The shared declarations' own refusal words, read off the published schemas. */ +const declaredRefusal = (): string => { + const parsed = (QuerySchema as any).shape.searchFields.safeParse([]); + expect(parsed.success).toBe(false); + const nested = FullTextSearchSchema.shape.fields.safeParse([]); + expect(nested.success).toBe(false); + expect(nested.error!.issues[0].message).toBe(parsed.error!.issues[0].message); + return parsed.error!.issues[0].message; +}; + +function matches(row: Record, where: unknown): boolean { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where as Record)) { + if (k === '$and') { if (!(v as unknown[]).every((w) => matches(row, w))) return false; continue; } + if (k === '$or') { if (!(v as unknown[]).some((w) => matches(row, w))) return false; continue; } + if (k.startsWith('$')) throw new Error(`driver double does not implement the '${k}' combinator`); + if (v && typeof v === 'object' && '$icontains' in (v as any)) { + if (!String(row[k] ?? '').toLowerCase().includes(String((v as any).$icontains).toLowerCase())) return false; + continue; + } + const expected = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v; + if (row[k] !== expected) return false; + } + return true; +} + +function makeDriver() { + const reads: Array<{ via: string; where: unknown }> = []; + const driver: any = { + name: 'memory', + version: '0.0.0', + supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(_o: string, ast: any) { + reads.push({ via: 'find', where: ast?.where }); + const matched = ROWS.filter((r) => matches(r, ast?.where)); + const page = typeof ast?.limit === 'number' ? matched.slice(0, ast.limit) : matched; + return page.map((r) => ({ ...r })); + }, + async findOne(_o: string, ast: any) { + reads.push({ via: 'findOne', where: ast?.where }); + const hit = ROWS.find((r) => matches(r, ast?.where)); + return hit ? { ...hit } : null; + }, + async count(_o: string, ast: any) { + reads.push({ via: 'count', where: ast?.where }); + return ROWS.filter((r) => matches(r, ast?.where)).length; + }, + async create(_o: string, d: any) { return d; }, + async update(_o: string, _id: string, d: any) { return d; }, + async delete() { return true; }, + async bulkCreate(_o: string, r: any[]) { return r; }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, reads }; +} + +/** The refusal as the REST door would answer it: `mapDataError`'s status and code. */ +async function wireAnswer(read: Promise) { + const err = await read.then( + () => { throw new Error('expected a refusal, got an answer'); }, + (e: unknown) => e, + ); + const mapped = mapDataError(err); + return { status: mapped.status, code: mapped.body.code, message: (err as Error).message, param: (err as any).param }; +} + +const COUNT = [{ function: 'count' as const, alias: 'n' }]; + +type Verb = 'find' | 'findOne' | 'count' | 'aggregate'; +const VERBS: readonly Verb[] = ['find', 'findOne', 'count', 'aggregate']; + +/** + * Run one verb with the search keys in `bag`. Deliberately off-contract: the + * bags carry the comma-string and `null` spellings the engine-side declaration + * does not admit, as the wire path hands them over. + */ +function run(engine: ObjectQL, verb: Verb, bag: Record): Promise { + switch (verb) { + case 'find': return engine.find(account.name, bag as unknown as EngineQueryOptions); + case 'findOne': return engine.findOne(account.name, bag as unknown as EngineQueryOptions); + case 'count': return engine.count(account.name, bag as unknown as EngineCountOptions); + case 'aggregate': return engine.aggregate(account.name, { groupBy: ['region'], aggregations: COUNT, ...bag } as unknown as EngineAggregateOptions); + } +} + +/** The ids a verb's answer covers, so a narrowing is read off the result. */ +async function answered(engine: ObjectQL, verb: Verb, bag: Record): Promise { + const out: any = await run(engine, verb, bag); + if (verb === 'find') return (out as any[]).map((r) => r.id).sort(); + if (verb === 'findOne') return out?.id ?? null; + if (verb === 'count') return out; + return Object.fromEntries((out as any[]).map((r) => [r.region, r.n])); +} + +/** What each verb answers for `harbour` over the DEFAULT set, and narrowed to `name`. */ +const DEFAULT_ANSWER: Record = { + find: ['a1', 'a2', 'a3'], findOne: 'a1', count: 3, aggregate: { east: 2, west: 1 }, +}; +const NAME_ONLY_ANSWER: Record = { + find: ['a1', 'a2'], findOne: 'a1', count: 2, aggregate: { east: 1, west: 1 }, +}; + +describe('[#27732] the in-process engine refuses a search-field override naming no field', () => { + let engine: ObjectQL; + let reads: Array<{ via: string; where: unknown }>; + + beforeEach(async () => { + engine = new ObjectQL(); + const made = makeDriver(); + reads = made.reads; + engine.registerDriver(made.driver, true); + await engine.init(); + engine.registry.registerObject(account, 'test-package'); + }); + + describe.each(VERBS)('%s', (verb) => { + it('searchFields: [] is refused 400 VALIDATION_ERROR in the declarations\' words, before any driver read', async () => { + const answer = await wireAnswer(run(engine, verb, { search: 'harbour', searchFields: [] })); + expect(answer).toMatchObject({ status: 400, code: 'VALIDATION_ERROR', param: 'searchFields' }); + expect(answer.message).toContain(declaredRefusal()); + expect(reads).toEqual([]); + }); + + it('search: { query, fields: [] } is refused the same way (item 1, the search.fields coupling)', async () => { + const answer = await wireAnswer(run(engine, verb, { search: { query: 'harbour', fields: [] } })); + expect(answer).toMatchObject({ status: 400, code: 'VALIDATION_ERROR', param: 'search.fields' }); + expect(answer.message).toContain(declaredRefusal()); + expect(reads).toEqual([]); + }); + + it('a comma string naming no field (the GET transport form) is refused the same way', async () => { + const answer = await wireAnswer(run(engine, verb, { search: 'harbour', searchFields: ' , ' })); + expect(answer).toMatchObject({ status: 400, code: 'VALIDATION_ERROR', param: 'searchFields' }); + expect(reads).toEqual([]); + }); + + it('CONTROL: a named override still narrows', async () => { + expect(await answered(engine, verb, { search: 'harbour', searchFields: ['name'] })).toEqual(NAME_ONLY_ANSWER[verb]); + expect(await answered(engine, verb, { search: { query: 'harbour', fields: ['name'] } })).toEqual(NAME_ONLY_ANSWER[verb]); + }); + + it('CONTROL: an absent override still searches the default set', async () => { + expect(await answered(engine, verb, { search: 'harbour' })).toEqual(DEFAULT_ANSWER[verb]); + expect(await answered(engine, verb, { search: { query: 'harbour' } })).toEqual(DEFAULT_ANSWER[verb]); + }); + }); + + it('an empty override is refused whether or not a search term rode along, as the door judges it', async () => { + const answer = await wireAnswer(engine.find(account.name, { searchFields: [] })); + expect(answer).toMatchObject({ status: 400, code: 'VALIDATION_ERROR', param: 'searchFields' }); + expect(reads).toEqual([]); + }); + + it('a system-context read is refused too: the rule is about the shape, not the caller', async () => { + const answer = await wireAnswer(engine.find(account.name, { search: 'harbour', searchFields: [], context: { isSystem: true } })); + expect(answer).toMatchObject({ status: 400, code: 'VALIDATION_ERROR' }); + expect(reads).toEqual([]); + }); + + it('the REST door answers the same status and code for the same override', async () => { + const protocol = new ObjectStackProtocolImplementation(engine); + const door = await wireAnswer(protocol.findData({ object: account.name, query: { search: 'harbour', searchFields: '' } as any })); + const inProcess = await wireAnswer(engine.find(account.name, { search: 'harbour', searchFields: [] })); + expect({ status: inProcess.status, code: inProcess.code }).toEqual({ status: door.status, code: door.code }); + expect(door).toMatchObject({ status: 400, code: 'VALIDATION_ERROR' }); + expect(reads).toEqual([]); + }); + + it('CONTROL: a null override is a withdrawal and searches the default set', async () => { + expect(await answered(engine, 'find', { search: 'harbour', searchFields: null })).toEqual(DEFAULT_ANSWER.find); + }); +}); diff --git a/packages/objectql/src/engine-search-empty-set.test.ts b/packages/objectql/src/engine-search-empty-set.test.ts index 59a81cfae76..b592fb4053b 100644 --- a/packages/objectql/src/engine-search-empty-set.test.ts +++ b/packages/objectql/src/engine-search-empty-set.test.ts @@ -138,7 +138,10 @@ describe('[#24284] a search over an empty resolved search set is refused', () => }); it('an explicit searchFields narrowing does not rescue it — the allowed set is empty, so the fallback is too', async () => { - for (const searchFields of [['title'], ['book'], ['nope'], []]) { + // [#27732] An empty override is no longer a narrowing to rescue: it + // names no field, and is refused before the expansion + // (engine-empty-search-fields-refusal.test.ts). + for (const searchFields of [['title'], ['book'], ['nope']]) { const err: any = await engine.find('lib_loan', { search: 'Dune', searchFields }).then(() => null, (e) => e); expect(err, `searchFields ${JSON.stringify(searchFields)}`).toMatchObject(REFUSAL); } diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 69fd391273c..05eca3197d2 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -5,7 +5,7 @@ import { QueryAST, QueryInput, HookContext, ServiceObject } from '@objectstack/s // [commit 74155c735] The defaulting node schema `fillQueryAstDefaults` runs author input // through — the declared `.default()` stays in `packages/spec`, the engine // only invokes it. -import { SortNodeSchema } from '@objectstack/spec/data'; +import { SortNodeSchema, FullTextSearchSchema } from '@objectstack/spec/data'; import { EngineQueryOptions, DataEngineInsertOptions, @@ -735,6 +735,40 @@ const ENGINE_AGGREGATE_OPTION_KEYS: ReadonlySet = new Set([ 'search', 'searchFields', ]); +/** + * [#27732] A search-field override — `searchFields`, or the object-form + * `search.fields` — that names no field is refused `400 VALIDATION_ERROR`: + * an empty list, or a comma string with no name in it (the `GET` door's + * transport spelling, which reaches the engine as sent). Absent (or `null`) + * still searches the object's default search fields. + * + * `resolveSearchFields` reads an empty override as absent, so the search + * widened to the whole default set: the caller wrote the key to narrow and was + * not told. The door refuses this set (`assertSearchFieldsAreSearchable`, + * #27484) and `QuerySchema.searchFields` / `FullTextSearchSchema.fields` + * declare the same refusal (#27683); the words are read off that declaration + * rather than copied, and the status and code are the door's, so + * `mapDataError` answers an in-process caller and a REST caller alike. Any + * other value is left to the expansion's existing tolerance. + */ +function refuseSearchOverrideNamingNoField(object: string, requested: unknown, param: string): void { + const names = typeof requested === 'string' + ? requested.split(',').map((s) => s.trim()).filter(Boolean) + : Array.isArray(requested) ? requested : undefined; + if (names === undefined || names.length > 0) return; + const declared = FullTextSearchSchema.shape.fields.safeParse([]).error?.issues[0]?.message; + const err: any = new Error( + `ObjectQL search on '${object}' names no field in '${param}': got ${JSON.stringify(requested)}. The query was not run: ` + + "an empty search-field list was read as no narrowing, which scans the object's default search fields. " + + `${declared}.`, + ); + err.status = 400; + err.code = 'VALIDATION_ERROR'; + err.param = param; + err.object = object; + throw err; +} + /** * Rows per page for the autonumber seeding scan (#6249). This is a PAGE size, * not a cap: the walk pages until the scope is exhausted. The number is the one @@ -13397,6 +13431,11 @@ export class ObjectQL implements IObjectQLEngine { // unknown option — rejected at the entry point, not silently dropped // (#4371). const raw = (ast as any).search; + // [#27732] An override naming no field is refused, never read as absent. + // Judged before the expansion and so before any driver read, whether or + // not a search term rode along, on both slots, as the door does. + refuseSearchOverrideNamingNoField(ast.object, (ast as any).searchFields, 'searchFields'); + if (raw !== null && typeof raw === 'object') refuseSearchOverrideNamingNoField(ast.object, raw.fields, 'search.fields'); if (raw != null && schema?.fields) { const requestedFields = (ast as any).searchFields ?? (typeof raw === 'object' ? raw?.fields : undefined);