Skip to content

Commit d831680

Browse files
committed
test(objectql,metadata-protocol,rest): pin the per-aggregation filter's where doors; changesets
Engine pins (both driver kinds, empty and populated, grouped and ungrouped, zero driver calls) for the temporal door, the declared-referent and addDays class rules with their withheld words and logged diagnostic, and the Date instant comparison; a findData pin for the filter-key gate; and a REST pin over a real SqlDriver with the where twin beside every row. Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3129311 commit d831680

6 files changed

Lines changed: 672 additions & 8 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@objectstack/metadata-protocol": minor
3+
---
4+
5+
fix(metadata-protocol)!: the read door judges the keys inside each per-aggregation `filter` with the gate an explicit `where` meets — an unknown key is `INVALID_FIELD` / 400, not a count of zero (#20148)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) this change adds no transition to migrate. A per-aggregation filter key that names no field of the object never had a meaning: no row carries it, so the aggregation read "no value" in every row and counted zero, or every row under a negation, while the same key in `where` was already refused by this gate. There is no accepted spelling it can be mechanically rewritten to: which field the author meant is an authoring decision, and the refusal names the key and suggests the nearest declared field. The gate is the one `where` already meets, with the same verdicts. The request is not a stored document, so there is nothing for `objectstack migrate meta` to rewrite. The table below records the answer each shape had and has; it prescribes no rewrite. -->
10+
11+
**BREAKING**: this narrows what `POST /data/:object/query` (`findData`) accepts in `aggregations[i].filter`. Each entry's filter now goes through the gate the explicit `where` meets, `assertFilterFieldsExist`: the same field set, the same verdicts, `INVALID_FIELD` / 400, with the entry's position as the parameter (`aggregations[1].filter`). The refusal is raised before the engine is called. It ships as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
Measured on the base through `POST /data/:object/query` on `driver-memory` and `driver-sql`, over six rows in three groups:
14+
15+
| in `aggregations[i].filter` | before | now |
16+
|:--|:--|:--|
17+
| a key naming no field of the object (`{ nope: 1 }`), also under `$and` | `200`, that count 0 | `INVALID_FIELD` / 400, naming the key |
18+
| the same key under `$ne`, `$not`, or behind a `$or` branch that holds | `200`, every row counted | `INVALID_FIELD` / 400 |
19+
| an unknown key carrying an unknown operator (`{ nope: { $median: 1 } }`) | `INVALID_FILTER` / 400 from the engine | `INVALID_FIELD` / 400: the field gate runs first, as it does for the same key in `where` |
20+
| a dotted key on a scalar head, or a key naming a `formula` field | `INVALID_FIELD` / 400 from the engine's own filter seam | `INVALID_FIELD` / 400 from this gate, in the words `where` gets |
21+
22+
Run after the entry checks and the aggregated-field check, so an entry the spec cannot read keeps its shape refusal and an unknown aggregated `field` keeps its own. A filter that is not a plain object names no key here and is left to the engine's shape gate.
23+
24+
Not changed: a filter on declared keys, on `id`, `created_at` or `updated_at`, and a relation head in the nested-object form reach the engine with the filter untouched.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
fix(objectql)!: a per-aggregation `filter` (`aggregations[i].filter`) on `engine.aggregate` takes the doors `where` takes — the temporal-comparand door, a `{ $field }` referent that must be a declared field, and the `addDays` class rule — and a `Date` bound is compared as an instant, as it is in `where` (#20148)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) this change adds no transition to migrate. Each refused input is one this position never evaluated as written: it answered a count by coercion (usually zero, every row under a negation), while the same condition as a `where` was already refused, by the engine's temporal-comparand door or by driver-sql's cross-field compiler. There is no accepted spelling a refused input can be mechanically rewritten to: which date, which declared field, or which pair of temporal fields the author meant is an authoring decision, and the refusal says what the position takes. A stored measure filter reaches this position through the analytics service, which lowers it; there is no stored document for `objectstack migrate meta` to rewrite. The table below records the answer each shape had and has; it prescribes no rewrite. -->
10+
11+
**BREAKING**: this narrows what a per-aggregation `filter` accepts on `engine.aggregate`, and on the REST aggregate query (`POST /data/:object/query` with `aggregations`) that forwards it there. Every refusal below is `INVALID_FILTER` / 400, raised in the engine's per-aggregation loop before any driver is asked for a row, identically on an empty and on a populated table. It ships as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
Measured on the base through `engine.aggregate` and through `POST /data/:object/query`, on `driver-memory` and `driver-sql`, over six rows in three groups, with the same condition written as the call's `where` beside each:
14+
15+
| in `aggregations[i].filter` | before | now |
16+
|:--|:--|:--|
17+
| a comparand a declared temporal field cannot read: `{ placed_on: { $gt: 'not-a-date' } }` on a `date`, the same as an `$in` member, a `$between` endpoint, in the implicit slot, behind a `$or` branch that holds or under `$not`; a preset name (`'last_30_days'`) on a `datetime`; `'noon'` on a `time` | counted as written: no row for the bad bound, only the readable members of the `$in`, every row for the `$between`, held-`$or` and `$not` shapes. The same condition as a `where` was refused | refused by the temporal-comparand door `where` takes, run unchanged on this position, in its words. A `{placeholder}` is stepped around and resolved, as there |
18+
| a `{ $field }` naming no declared field of the object (`{ amount: { $gt: { $field: 'nope' } } }`), a dotted referent, or an `addDays` offset column the object does not declare | no row counted; every row under `$ne`, `$not` or a held `$or`. `driver-sql` refused the same comparison in a `where` | refused |
19+
| `addDays` on a pair `FieldReferenceSchema.addDays` does not declare it for: two numeric fields (`{ amount: { $gt: { $field: 'cap', addDays: 1 } } }`), a numeric referent, a text or a time pair, a `date` against a `datetime`, or an offset read from a column that is not numeric | answered by the evaluator's epoch-millisecond coercion: no row on the fixture, 4 of 6 for the `date` / `datetime` pair. `driver-sql` refused the same pair in a `where` | refused |
20+
21+
The two `{ $field }` rows are refused in the words `driver-sql` uses for the same comparison in a `where`: the message names the aggregation that carries the reference (`aggregations[1].filter`) and the rule, and withholds the fields, the operator and the specific reason. The engine logs the withheld diagnostic at `warn`, once per refusal. A referent is judged against the object's declared field map plus `id`, `created_at` and `updated_at`, the set the REST field gate reads; on a host whose registry holds no field map for the object, nothing is judged.
22+
23+
These refusals run after the walker's own (an unknown operator, a malformed reference), so a filter carrying both gets the walker's refusal first.
24+
25+
Not refused, but answering differently:
26+
27+
- **A `Date` bound is compared as an instant.** `{ opened_at: { $gt: new Date('2026-02-01') } }` against the ISO text a `datetime` column holds counted no row: JS compared the `Date` with the string by coercion. `$ne` and `$nin` counted every row, and a `$between` of two `Date`s counted every row. The same bound in a `where` counted 4 of 6 on both drivers. The comparison now reads the pair through `@objectstack/spec/data`'s `utcInstantMs`, the lift `@objectstack/formula`'s evaluator applies, whenever one side is a `Date` and both sides denote an instant. Every `datetime` row and a UTC-midnight `Date` on a `date` field now count what the same bound counts in a `where`. `having` shares this comparison, so a `Date` bound in `having` keeps the groups its ISO spelling keeps. A `Date` is not JSON, so this reaches in-process callers only. Two readings still differ from a `where`, because the per-row comparison holds no declaration: a `Date` carrying a time of day against a `date` field, which a `where` reads as that UTC calendar day, and a `Date` against a `time` field, which is not an instant and is left as before.
28+
29+
Not changed, measured identical before and after on both drivers: every `where`, `groupBy` and `having` answer outside the `Date` shapes above, and a per-aggregation filter that uses implicit equality, ordering bounds, `$in`, `$between`, `$or`, `{}`, a `{ $field }` between two declared numeric fields, `addDays` between two `date` or two `datetime` fields (literal or read from a numeric column), and a reference to `created_at` or `id`.
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#20148] The KEYS inside a per-aggregation `filter`, at the read door.
5+
*
6+
* `findData` judged a `where`'s keys against the object's field set (#7534,
7+
* {@link ObjectStackProtocolImplementation} `assertFilterFieldsExist`) and an
8+
* aggregation's `function` / `alias` / `field` (#4254), but never the keys of
9+
* the filter an aggregation carries. Measured on the base through
10+
* `POST /api/v1/data/:object/query` on driver-memory and driver-sql:
11+
*
12+
* ```
13+
* aggregations: [{ …, filter: { nope: 1 } }] -> 200, that count 0
14+
* aggregations: [{ …, filter: { nope: { $ne: 1 } } }] -> 200, EVERY row counted
15+
* where: { nope: 1 } -> 400 INVALID_FIELD
16+
* ```
17+
*
18+
* Now each aggregation's filter goes through the `where` gate itself: the same
19+
* field set, the same `unknown` > `dotted` > virtual ladder, the same
20+
* `INVALID_FIELD` / 400, the caller's position (`aggregations[1].filter`) as
21+
* the parameter — and the engine is never reached. Every pin below asserts
22+
* `code` AND `status` AND that `engine.aggregate` was not called; the
23+
* preservation half asserts what the engine received.
24+
*/
25+
26+
import { describe, it, expect, vi } from 'vitest';
27+
import { ObjectStackProtocolImplementation } from './protocol.js';
28+
29+
const SCHEMA = {
30+
name: 'invoice',
31+
fields: {
32+
name: { name: 'name', type: 'text' },
33+
status: { name: 'status', type: 'text' },
34+
amount: { name: 'amount', type: 'number' },
35+
doubled: { name: 'doubled', type: 'formula', expression: 'amount * 2' },
36+
owner_id: { name: 'owner_id', type: 'lookup', reference: 'sys_user' },
37+
},
38+
};
39+
40+
function makeProtocol() {
41+
const find = vi.fn(async () => [] as unknown[]);
42+
const aggregate = vi.fn(async () => [] as unknown[]);
43+
const engine = {
44+
registry: { getObject: (n: string) => (n === 'invoice' ? SCHEMA : undefined) },
45+
find,
46+
aggregate,
47+
count: vi.fn(async () => 0),
48+
};
49+
return { p: new ObjectStackProtocolImplementation(engine as any), find, aggregate };
50+
}
51+
52+
/** The second of two aggregations carries `filter`, so the position must name WHICH one. */
53+
function withFilter(filter: unknown) {
54+
return {
55+
aggregations: [
56+
{ function: 'count', alias: 'n' },
57+
{ function: 'count', alias: 'm', filter },
58+
],
59+
};
60+
}
61+
62+
async function refusalFor(query: Record<string, unknown>) {
63+
const { p, find, aggregate } = makeProtocol();
64+
let answered: unknown;
65+
try {
66+
answered = await p.findData({ object: 'invoice', query } as never);
67+
} catch (e) {
68+
expect(find, 'the engine was reached before the refusal').not.toHaveBeenCalled();
69+
expect(aggregate, 'the engine was reached before the refusal').not.toHaveBeenCalled();
70+
return e as Error & { status?: number; code?: string; param?: string; field?: string; fields?: string[] };
71+
}
72+
throw new Error(`${JSON.stringify(query)} was ACCEPTED (answered ${JSON.stringify(answered)}) instead of refused`);
73+
}
74+
75+
describe('[#20148] findData — the keys inside a per-aggregation filter meet the where gate', () => {
76+
const REFUSED: ReadonlyArray<readonly [string, Record<string, unknown>, string]> = [
77+
['an unknown key (the card\'s row 4)', { nope: 1 }, 'nope'],
78+
['an unknown key under $ne', { nope: { $ne: 1 } }, 'nope'],
79+
['an unknown key under $and', { $and: [{ amount: { $gt: 0 } }, { nope: 1 }] }, 'nope'],
80+
['an unknown key behind a $or branch that holds', { $or: [{ amount: { $gt: 0 } }, { nope: 1 }] }, 'nope'],
81+
['an unknown key under $not', { $not: { nope: 1 } }, 'nope'],
82+
['an unknown key beside an unknown operator', { nope: { $median: 1 } }, 'nope'],
83+
];
84+
85+
for (const [name, filter, key] of REFUSED) {
86+
it(`${name}: 400 INVALID_FIELD at the aggregation's position, as the same key in where is`, async () => {
87+
const err = await refusalFor(withFilter(filter));
88+
expect(err.code).toBe('INVALID_FIELD');
89+
expect(err.status).toBe(400);
90+
expect(err.field).toBe(key);
91+
expect(err.param).toBe('aggregations[1].filter');
92+
// The twin: the same condition as the call's `where`.
93+
const twin = await refusalFor({ where: filter, aggregations: [{ function: 'count', alias: 'n' }] });
94+
expect(twin.code).toBe('INVALID_FIELD');
95+
expect(twin.status).toBe(400);
96+
expect(twin.field).toBe(key);
97+
});
98+
}
99+
100+
it('the gate\'s other two verdicts come with it — a dotted path and a virtual field, each as in where', async () => {
101+
for (const [filter, field] of [[{ 'status.name': 'x' }, 'status.name'], [{ doubled: { $gt: 10 } }, 'doubled']] as const) {
102+
const err = await refusalFor(withFilter(filter));
103+
const twin = await refusalFor({ where: filter, aggregations: [{ function: 'count', alias: 'n' }] });
104+
for (const e of [err, twin]) {
105+
expect(e.code).toBe('INVALID_FIELD');
106+
expect(e.status).toBe(400);
107+
expect(e.field).toBe(field);
108+
}
109+
expect(err.param).toBe('aggregations[1].filter');
110+
}
111+
});
112+
113+
it('an unknown aggregated field keeps its own verdict ahead of a filter key', async () => {
114+
const err = await refusalFor({
115+
aggregations: [{ function: 'sum', field: 'amout', alias: 't', filter: { nope: 1 } }],
116+
});
117+
expect(err.code).toBe('INVALID_FIELD');
118+
expect(err.param).toBe('aggregations');
119+
expect(err.field).toBe('amout');
120+
});
121+
122+
const ADMITTED: ReadonlyArray<readonly [string, unknown]> = [
123+
['declared keys, composed', { $or: [{ amount: { $gt: 100 } }, { status: 'paid' }] }],
124+
['the columns every row carries', { id: 'i1', created_at: { $ne: null }, updated_at: { $ne: null } }],
125+
['a relation head in the nested-object form', { owner_id: { $ne: null } }],
126+
['a filter that is not an object (left to the engine\'s shape gate)', 'status = paid'],
127+
['an array filter (left to the engine\'s shape gate)', [['amount', '>', 1]]],
128+
];
129+
130+
for (const [name, filter] of ADMITTED) {
131+
it(`${name}: reaches engine.aggregate with the filter untouched`, async () => {
132+
const { p, aggregate } = makeProtocol();
133+
await p.findData({ object: 'invoice', query: withFilter(filter) } as never);
134+
expect(aggregate).toHaveBeenCalledTimes(1);
135+
const opts = (aggregate.mock.calls[0] as unknown[])[1] as { aggregations: Array<{ filter?: unknown }> };
136+
expect(opts.aggregations[1].filter).toEqual(filter);
137+
});
138+
}
139+
});

0 commit comments

Comments
 (0)