Skip to content

Commit 3c2fa82

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-20122-aggregate-filter-doors
2 parents 1690b79 + 949e99b commit 3c2fa82

4 files changed

Lines changed: 577 additions & 2 deletions

File tree

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
'@objectstack/objectql': minor
3+
---
4+
5+
fix(objectql)!: an engine `where` that is not a filter — a string, a number, a `Map`, a boolean, a `Date` — is refused with `INVALID_FILTER` / 400 before any driver call, and a `multi: true` update or delete no longer rewrites or removes every row for it (#20121)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authored or stored moves: `packages/spec` is untouched, the flow record nodes already type their `filter` as a record (a string is refused at node parse), and the REST normalizer already answers a non-filter `?filter=` with this same code, so `objectstack migrate meta` has nothing to rewrite and the ledger has no row to gain. What is refused is an in-process call whose `where` was already off the declared type; a string such as `'amount > 100'` has no defined meaning to rewrite mechanically, so the repair is the caller's own filter. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id covers an engine input shape (not `registered` / `already-registered`); and the change is runtime behaviour, not a TypeScript declaration (not `runtime-interface-only` / `type-surface-only`). -->
10+
11+
**BREAKING** — an accept-set narrowing on the engine's `where`, shipped as `minor`
12+
under the launch-window convention (`check-changeset-no-major` refuses `major`
13+
until GA; breaking-ness is carried by this banner and the ADR-0087 disposition
14+
above, not by the level).
15+
16+
**What changed.** `find`, `findOne`, `count`, `aggregate`, `update` and `delete`
17+
on the engine now refuse a `where` that is neither absent, a filter object nor a
18+
filter array. The refusal is thrown before a driver is asked for anything, as
19+
`INVALID_FILTER` with `status` and `httpStatus` 400, and its message reads
20+
"`<verb>('<object>')`: 'where' must be a filter object or condition array,
21+
received …. It was not applied, …" — the REST normalizer's words for the same
22+
input. The refusal for an array that is not a filter (`[1, 2, 3]`, an infix
23+
join) keeps its message and now carries the same `INVALID_FILTER` / 400
24+
envelope; it used to have no `code` and no `status`.
25+
26+
**What it replaces.** A value with no filter keys fell through every check on
27+
the seam and the driver ignored it. Measured on `driver-memory` and
28+
`SqlDriver` (better-sqlite3) with four rows:
29+
30+
- `find` / `count` / `aggregate` answered for every row, as if no `where` had
31+
been given; `findOne` answered the first row (for a `Map`, its no-predicate
32+
guard refused the call, with no `code`).
33+
- `update(…, { where, multi: true })` rewrote all four rows, and
34+
`delete({ where, multi: true })` deleted all four. That held without
35+
`SecurityPlugin`, and with it under a system context.
36+
- For a caller scoped by row-level security, it depended on the value.
37+
- A string, a number, a `Map`, a `Date`, a `Set` or `true` was wrapped by the
38+
security middleware into its `$and`, where the driver refused it
39+
(`INVALID_FILTER`) and nothing was written.
40+
- The empty string was not refused. The middleware's composition reads a
41+
falsy `where` as absent and dropped it, so `find` answered all of the
42+
member's rows, and `update(multi)` / `delete(multi)` rewrote or deleted
43+
every row the member could reach (2 of 4 on both drivers).
44+
- Such a `where` also stepped past the unscoped-write guard that a hook opts
45+
into with `dispatchUnscopedMultiWrite`, because that guard treats only an
46+
absent or `null` `where` as unscoped.
47+
48+
**Unchanged.** An absent `where`, `null`, `{}` and `[]` still mean "no
49+
filter". A filter object is accepted when it is a non-array object whose
50+
built-in tag (`Object.prototype.toString`) is `[object Object]`. That covers a
51+
plain object, an `Object.create(null)` object, an instance of your own class
52+
carrying the filter on its own keys, a `Proxy` of one, and an object from
53+
another realm, and each filters exactly as before. A well-formed filter array is
54+
lowered as before.
55+
56+
**One accepted shape is now refused.** An object that overrides
57+
`Symbol.toStringTag`, as its own key or through its prototype chain, has a
58+
different built-in tag. It is refused and named by that tag (for example
59+
`received Criteria`). Before this change such an object filtered correctly on
60+
its own keys. No producer in this repository creates one: the wire is JSON, and
61+
the SDK builds arrays or plain objects. If yours does, pass its filter keys in a
62+
plain object instead. The REST door already answered a non-filter `?filter=` with
63+
`INVALID_FILTER` / 400 and is not touched.
64+
65+
**Fix.** Pass the predicate you meant as a filter object, for example
66+
`{ amount: { $gt: 100 } }`, or as a filter array (`[['amount', '>', 100]]`), and
67+
leave `where` out when you mean every row. If the value came from somewhere
68+
untyped, the refusal names what arrived (`received string "amount > 100"`,
69+
`received Map`), and an un-awaited promise shows up as `received Promise`.
Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,283 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #20121 — a `where` that is neither absent, a filter object nor a filter array
5+
* is refused at the engine's one filter seam, before any driver call.
6+
*
7+
* Measured on the base, through the real engine, on `driver-memory` and
8+
* `SqlDriver` (better-sqlite3), four rows seeded:
9+
*
10+
* find('order', { where: 'amount > 100' }) -> 4 rows (all)
11+
* update('order', {...}, { where: 42, multi: true }) -> 4 of 4 rows rewritten
12+
* delete('order', { where: new Map(), multi: true }) -> 4 of 4 rows deleted
13+
* find('order', { where: [1, 2, 3] }) -> refused, code/status undefined
14+
*
15+
* A string, number or `Map` has no keys for the seam's doors to walk, so every
16+
* door stepped around it and the driver ignored it — and on a write that is a
17+
* whole-table rewrite or delete. The same value also stepped past the
18+
* unscoped-write guard, which reads only an absent or `null` `where` as
19+
* unscoped.
20+
*
21+
* These cases assert what the ENGINE refuses and that the driver is never
22+
* asked: a refusal the driver produces after the fact would already have read,
23+
* and on a SQL driver the damage is done by the time anything answers.
24+
*/
25+
26+
import { describe, it, expect, beforeEach } from 'vitest';
27+
import type {
28+
EngineAggregateOptions,
29+
EngineCountOptions,
30+
EngineDeleteOptions,
31+
EngineQueryOptions,
32+
EngineUpdateOptions,
33+
} from '@objectstack/spec/data';
34+
import type { DriverQuery, IDataDriver } from '@objectstack/spec/contracts';
35+
import { ObjectQL } from './engine.js';
36+
37+
const deal = {
38+
name: 'deal',
39+
label: 'Deal',
40+
fields: {
41+
id: { name: 'id', type: 'text' as const, primaryKey: true },
42+
stage: { name: 'stage', type: 'text' as const },
43+
amount: { name: 'amount', type: 'number' as const },
44+
},
45+
};
46+
47+
/** A verb the pin does not reach, present only because `IDataDriver` requires it. */
48+
const unexercised = (verb: string): never => {
49+
throw new Error(`counting driver: ${verb}() is not exercised by this pin (#20121)`);
50+
};
51+
52+
/**
53+
* A driver double that COUNTS every call and executes only the filter-object
54+
* form. A `where` it cannot read throws here rather than matching everything —
55+
* so a refusal that is missing from the engine turns a case red here instead of
56+
* passing on a double that quietly agreed with the dropped filter.
57+
*/
58+
function makeCountingDriver() {
59+
const rows = new Map<string, Record<string, unknown>>();
60+
const calls: string[] = [];
61+
const matches = (row: Record<string, unknown>, where: unknown): boolean => {
62+
if (where == null) return true;
63+
if (typeof where !== 'object' || Array.isArray(where)) {
64+
throw new Error(`counting driver: received a non-object 'where' (${String(where)})`);
65+
}
66+
for (const [k, v] of Object.entries(where)) {
67+
// A combinator or an operator this double does not implement is REFUSED,
68+
// never read as a field name or skipped (`check:where-matcher`).
69+
if (k.startsWith('$')) throw new Error(`counting driver: unsupported combinator ${k}`);
70+
if (v && typeof v === 'object' && !Array.isArray(v)) {
71+
const ops = v as Record<string, unknown>;
72+
for (const op of Object.keys(ops)) {
73+
if (op !== '$gt') throw new Error(`counting driver: unsupported operator ${op}`);
74+
}
75+
if (!((row[k] as number) > (ops.$gt as number))) return false;
76+
continue;
77+
}
78+
if (row[k] !== v) return false;
79+
}
80+
return true;
81+
};
82+
const run = (ast: DriverQuery | undefined) => [...rows.values()].filter((r) => matches(r, ast?.where));
83+
const driver: IDataDriver = {
84+
name: 'counting', version: '0.0.0', supports: {},
85+
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
86+
async find(_o: string, ast: DriverQuery) {
87+
calls.push('find');
88+
const hit = run(ast);
89+
// The caller's bound, applied AFTER the filter and by presence
90+
// (`check:objectql-double-limit`).
91+
return typeof ast?.limit === 'number' ? hit.slice(0, ast.limit) : hit;
92+
},
93+
async findOne(_o: string, ast: DriverQuery) { calls.push('findOne'); return run(ast)[0] ?? null; },
94+
async count(_o: string, ast: DriverQuery) { calls.push('count'); return run(ast).length; },
95+
async aggregate(_o: string, ast: DriverQuery) { calls.push('aggregate'); return [{ n: run(ast).length }]; },
96+
async create(_o: string, data: Record<string, unknown>) {
97+
calls.push('create');
98+
const row = { ...data, id: data.id as string };
99+
rows.set(row.id, row);
100+
return row;
101+
},
102+
async update(_o: string, id: string, data: Record<string, unknown>) {
103+
calls.push('update');
104+
const up = { ...rows.get(id), ...data, id };
105+
rows.set(id, up);
106+
return up;
107+
},
108+
async updateMany(_o: string, ast: DriverQuery, data: Record<string, unknown>) {
109+
calls.push('updateMany');
110+
const hit = run(ast);
111+
for (const r of hit) rows.set(r.id as string, { ...r, ...data });
112+
return hit.length;
113+
},
114+
async delete(_o: string, id: string) { calls.push('delete'); return rows.delete(id); },
115+
async deleteMany(_o: string, ast: DriverQuery) {
116+
calls.push('deleteMany');
117+
const hit = run(ast);
118+
for (const r of hit) rows.delete(r.id as string);
119+
return hit.length;
120+
},
121+
async bulkCreate() { return unexercised('bulkCreate'); },
122+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
123+
async commit() {}, async rollback() {},
124+
async upsert() { return unexercised('upsert'); },
125+
async bulkUpdate() { return unexercised('bulkUpdate'); },
126+
async bulkDelete() { return unexercised('bulkDelete'); },
127+
async syncSchema() { return unexercised('syncSchema'); },
128+
async dropTable() { return unexercised('dropTable'); },
129+
};
130+
return { driver, rows, calls };
131+
}
132+
133+
/**
134+
* The six verbs that cross the seam, each driven with the caller's `where`.
135+
* The writes carry `multi: true` — the predicate path, where a dropped `where`
136+
* is a whole-table write. `where` is typed `unknown` and cast at the call:
137+
* every bad shape here is type-illegal on purpose, and the cast names the
138+
* contract being bypassed.
139+
*/
140+
const VERBS: ReadonlyArray<readonly [string, (engine: ObjectQL, where: unknown) => Promise<unknown>]> = [
141+
['find', (e, where) => e.find('deal', { where } as unknown as EngineQueryOptions)],
142+
['findOne', (e, where) => e.findOne('deal', { where } as unknown as EngineQueryOptions)],
143+
['count', (e, where) => e.count('deal', { where } as unknown as EngineCountOptions)],
144+
['aggregate', (e, where) => e.aggregate('deal', {
145+
where,
146+
groupBy: ['stage'],
147+
aggregations: [{ function: 'count', field: 'id', alias: 'n' }],
148+
} as unknown as EngineAggregateOptions)],
149+
['update', (e, where) => e.update('deal', { stage: 'archived' }, { where, multi: true } as unknown as EngineUpdateOptions)],
150+
['delete', (e, where) => e.delete('deal', { where, multi: true } as unknown as EngineDeleteOptions)],
151+
];
152+
153+
/** The four shapes the card names — each was dropped or refused without an envelope. */
154+
const BAD_SHAPES: ReadonlyArray<readonly [string, () => unknown]> = [
155+
['a string', () => 'amount > 100'],
156+
['a number', () => 42],
157+
['a Map', () => new Map([['amount', { $gt: 15 }]])],
158+
['a non-filter array', () => [1, 2, 3]],
159+
];
160+
161+
/** The two shapes that answer correctly — the controls. Both select d2 and d3. */
162+
const GOOD_SHAPES: ReadonlyArray<readonly [string, () => unknown]> = [
163+
['a filter object', () => ({ amount: { $gt: 15 } })],
164+
['a filter array', () => [['amount', '>', 15]]],
165+
];
166+
167+
interface Refusal { code?: unknown; status?: unknown; message?: unknown }
168+
169+
async function refusalOf(run: () => Promise<unknown>): Promise<Refusal> {
170+
try {
171+
await run();
172+
} catch (e) {
173+
return e as Refusal;
174+
}
175+
throw new Error('expected the engine to refuse, and it answered');
176+
}
177+
178+
describe('an engine `where` that is not a filter is refused at the seam, before any driver call (#20121)', () => {
179+
let engine: ObjectQL;
180+
let rows: Map<string, Record<string, unknown>>;
181+
let calls: string[];
182+
183+
beforeEach(async () => {
184+
const counting = makeCountingDriver();
185+
rows = counting.rows;
186+
calls = counting.calls;
187+
engine = new ObjectQL();
188+
engine.registerDriver(counting.driver, true);
189+
await engine.init();
190+
engine.registry.registerObject(deal);
191+
await engine.insert('deal', { id: 'd1', stage: 'open', amount: 10 });
192+
await engine.insert('deal', { id: 'd2', stage: 'open', amount: 20 });
193+
await engine.insert('deal', { id: 'd3', stage: 'won', amount: 30 });
194+
calls.length = 0;
195+
});
196+
197+
for (const [verb, run] of VERBS) {
198+
for (const [label, shape] of BAD_SHAPES) {
199+
it(`${verb}: ${label} is INVALID_FILTER / 400, and the driver is never called`, async () => {
200+
const err = await refusalOf(() => run(engine, shape()));
201+
202+
expect(err.code).toBe('INVALID_FILTER');
203+
expect(err.status).toBe(400);
204+
expect(String(err.message).startsWith(`${verb}('deal')`)).toBe(true);
205+
expect(calls).toEqual([]);
206+
// Nothing was written either — read past the engine, off the double.
207+
expect([...rows.values()].map((r) => `${r.id}:${r.stage}`).sort())
208+
.toEqual(['d1:open', 'd2:open', 'd3:won']);
209+
});
210+
}
211+
212+
for (const [label, shape] of GOOD_SHAPES) {
213+
it(`${verb}: ${label} still reaches the driver and selects exactly d2 and d3 (control)`, async () => {
214+
const answer = await run(engine, shape());
215+
216+
expect(calls.length).toBeGreaterThan(0);
217+
switch (verb) {
218+
case 'find':
219+
expect((answer as Array<{ id: string }>).map((r) => r.id).sort()).toEqual(['d2', 'd3']);
220+
break;
221+
case 'findOne':
222+
expect(['d2', 'd3']).toContain((answer as { id: string }).id);
223+
break;
224+
case 'count':
225+
expect(answer).toBe(2);
226+
break;
227+
case 'aggregate':
228+
expect(answer).toEqual([{ n: 2 }]);
229+
break;
230+
case 'update':
231+
expect([...rows.values()].filter((r) => r.stage === 'archived').map((r) => r.id).sort())
232+
.toEqual(['d2', 'd3']);
233+
break;
234+
case 'delete':
235+
expect([...rows.keys()]).toEqual(['d1']);
236+
break;
237+
}
238+
});
239+
}
240+
}
241+
242+
// ── a door that forwards to the seam: the scoped repository ───────────────
243+
244+
it('createContext().object(): find and delete(multi) forward to the seam and are refused the same way', async () => {
245+
const repo = engine.createContext({ isSystem: true }).object('deal');
246+
247+
const read = await refusalOf(() => repo.find({ where: 'amount > 15' }));
248+
const write = await refusalOf(() => repo.delete({ where: 42, multi: true }));
249+
250+
expect([read.code, read.status, write.code, write.status]).toEqual(['INVALID_FILTER', 400, 'INVALID_FILTER', 400]);
251+
expect(calls).toEqual([]);
252+
expect(rows.size).toBe(3);
253+
});
254+
255+
// ── the accept set's edges: nothing that answered correctly is refused ────
256+
257+
it.each<[string, () => unknown, number]>([
258+
['null (no filter)', () => null, 3],
259+
['{} (match-all)', () => ({}), 3],
260+
['[] (no filter)', () => [], 3],
261+
['Object.create(null) carrying the filter', () => Object.assign(Object.create(null), { amount: { $gt: 15 } }), 2],
262+
['a class instance carrying the filter on its own keys', () => new (class Criteria { amount = { $gt: 15 }; })(), 2],
263+
])('find: %s is accepted and answers %i rows', async (_label, shape, expected) => {
264+
const answer = await engine.find('deal', { where: shape() } as unknown as EngineQueryOptions);
265+
expect(answer).toHaveLength(expected);
266+
});
267+
268+
it.each<[string, () => unknown]>([
269+
['a Date', () => new Date(0)],
270+
['a Set', () => new Set(['open'])],
271+
['a boolean', () => true],
272+
['an empty string', () => ''],
273+
['an un-awaited Promise', () => Promise.resolve({ stage: 'open' })],
274+
])('delete(multi): %s is refused the same way — it too was a whole-table delete', async (_label, shape) => {
275+
const err = await refusalOf(() =>
276+
engine.delete('deal', { where: shape(), multi: true } as unknown as EngineDeleteOptions));
277+
278+
expect(err.code).toBe('INVALID_FILTER');
279+
expect(err.status).toBe(400);
280+
expect(calls).toEqual([]);
281+
expect(rows.size).toBe(3);
282+
});
283+
});

0 commit comments

Comments
 (0)