Skip to content

Commit 3adb37d

Browse files
committed
fix(analytics): refuse a zero-operator field constraint on the draft preview
`matchesWhere` iterated a field constraint's entries; `{ name: {} }` has none, so the row fell through to a match and the draft preview charted every row for a filter all three shipped drivers refuse. Refuse it in the INVALID_FILTER / 400 envelope the preview already speaks, from the row-independent gate (whole tree, nested combinators included) and from matchesWhere's field arm. Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude <noreply@anthropic.com>
1 parent dabf8d7 commit 3adb37d

3 files changed

Lines changed: 239 additions & 5 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
The draft-data preview **refuses** a field constraint with zero operators (`{ name: {} }`) instead of answering it with every row, so a drafted chart no longer shows rows for a filter publish refuses outright (#19835).
6+
7+
`preview-evaluator.ts`'s `matchesWhere` iterated a field constraint's entries; an empty object has none, so the loop never ran and the row fell through to a MATCH. `matchesWhere({ name: 'Globex' }, { name: {} })` answered `true`. Every data driver refuses this shape (`driver-memory`, `driver-mongodb`, and `driver-sql` at the top level and inside `$and`/`$or`/`$not`), and so does this package's own `where` door, so the preview and publish gave opposite answers to the same filter.
8+
9+
- **Refused in the ADR-0112 `INVALID_FILTER` / 400 envelope**, through the same `invalidFilterError` the preview already uses for an operator it cannot evaluate. No new error code and no new exported symbol. The message follows the drivers' wording: it names the constraint and its position (`where.$or[1].amount`), and gives the two legal repairs (name an operator, or write a direct comparand).
10+
- **Not answered as "matches zero rows" either.** `{ status: {} }` does not mean "no rows". Read literally it means "rows whose status is anything", and the shape is almost always an authoring accident: a filter builder that recorded a field but never its operator. Only a refusal names the constraint to repair.
11+
- **Nesting cannot route around it.** The check walks the whole `where` before any row is read, `$and` / `$or` / `$not` arms included. So a constraint in an `$or` arm that a matching row would short-circuit past still refuses, and so does a seed draft holding zero rows.
12+
- ⚠️ **What it costs**: a drafted chart whose filter carries `{ field: {} }` now returns `400 INVALID_FILTER` in preview, where before it rendered a number computed over every row. Fix: name the operator the constraint was meant to carry, e.g. `{ status: { $eq: 'open' } }` or `{ status: 'open' }`.
13+
- **Unchanged**: constraints that name an operator, implicit-equality comparands, and an empty *node* (`where: {}` or `$and: [{}]`, which is the identity and not a field constraint).
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#19835] The draft-preview matcher answered a field constraint with ZERO
5+
* operators — `{ name: {} }` — with EVERY row.
6+
*
7+
* `matchesWhere`'s per-field loop iterates the constraint's entries; an empty
8+
* object has none, so the loop never ran and the row fell through to the
9+
* closing `return true`. Probe at `origin/main` before the repair:
10+
* `matchesWhere({ name: 'Globex' }, { name: {} })` → `true`. The #19810
11+
* operator-vocabulary refusal could not reach it: no key, no lookup to fail.
12+
*
13+
* Every shipped driver refuses the shape (`driver-memory` / `driver-mongodb`
14+
* `emptyFieldConstraintError`, `driver-sql` at the top level and inside
15+
* combinators) — ruled on #5240, refused everywhere. So the preview charted
16+
* every row for a filter publish refuses outright.
17+
*
18+
* ## What this file pins
19+
*
20+
* 1. **Refused** — `INVALID_FILTER` / 400 (ADR-0112), pinned by `code` +
21+
* `status`, never by a bare `toThrow()`. ⛔ Not "matches zero rows": that
22+
* is the other silent reading the ruling declined.
23+
* 2. **Not bypassable by nesting** — under `$and`, `$or` (including the arm a
24+
* short-circuit would never reach) and `$not`, and over an EMPTY seed.
25+
* 3. **Unchanged** — a constraint that names an operator, an implicit
26+
* comparand, and an empty NODE (`{}` as the whole `where` or as a
27+
* combinator arm — the identity, not a field constraint) answer exactly
28+
* what they answered before.
29+
*/
30+
31+
import { describe, it, expect } from 'vitest';
32+
import { DatasetSchema } from '@objectstack/spec/ui';
33+
import type { Cube } from '@objectstack/spec/data';
34+
import { AnalyticsService } from '../analytics-service.js';
35+
import { evaluateAnalyticsQueryOverRows, matchesWhere } from '../preview-evaluator.js';
36+
37+
type Refusal = Error & { code?: string; status?: number };
38+
39+
const ROWS: Record<string, unknown>[] = [
40+
{ id: '1', name: 'Acme Corp', amount: 1200 },
41+
{ id: '2', name: 'Globex', amount: 800 },
42+
];
43+
44+
const CUBE: Cube = new AnalyticsService().registerDataset(
45+
DatasetSchema.parse({
46+
name: 'expense_ds',
47+
label: 'Expense',
48+
object: 'expense',
49+
dimensions: [{ name: 'name', field: 'name', type: 'string', label: 'Name' }],
50+
measures: [{ name: 'count', aggregate: 'count' }],
51+
}),
52+
).cube;
53+
54+
function run(where: Record<string, unknown>, rows = ROWS) {
55+
return evaluateAnalyticsQueryOverRows(
56+
{ cube: 'expense_ds', measures: ['count'], dimensions: ['name'], where },
57+
CUBE,
58+
rows,
59+
);
60+
}
61+
62+
function refusalFor(thunk: () => unknown): Refusal | undefined {
63+
try {
64+
thunk();
65+
return undefined;
66+
} catch (e) {
67+
return e as Refusal;
68+
}
69+
}
70+
71+
function namesAnswered(where: Record<string, unknown>): string[] {
72+
return run(where).rows.map((r) => String(r.name)).sort();
73+
}
74+
75+
describe('[#19835] a field constraint with zero operators on the draft preview', () => {
76+
it('the card probe — `matchesWhere({ name: "Globex" }, { name: {} })` — refuses instead of matching', () => {
77+
const err = refusalFor(() => matchesWhere({ name: 'Globex' }, { name: {} }));
78+
expect(err).toBeInstanceOf(Error);
79+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
80+
});
81+
82+
it('refuses a whole preview query at the top level, naming the field and its position', () => {
83+
const err = refusalFor(() => run({ name: {} }));
84+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
85+
expect(err?.message).toContain('where.name');
86+
expect(err?.message).toContain('{ "name": {} }');
87+
});
88+
89+
it('refuses over an EMPTY seed draft — the walk is not a function of the data', () => {
90+
expect(refusalFor(() => run({ name: {} }, []))).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
91+
});
92+
93+
describe('nesting cannot route around it', () => {
94+
it('inside `$and`', () => {
95+
const err = refusalFor(() => run({ $and: [{ name: 'Globex' }, { amount: {} }] }));
96+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
97+
expect(err?.message).toContain('where.$and[1].amount');
98+
});
99+
100+
it('inside `$or`, in the arm a short-circuit would never reach for a matching row', () => {
101+
// Row `Globex` satisfies arm 0, so a per-row walk would `some()` past arm
102+
// 1 and answer it; the refusal must not depend on which row is tested.
103+
const err = refusalFor(() => run({ $or: [{ name: 'Globex' }, { amount: {} }] }));
104+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
105+
expect(err?.message).toContain('where.$or[1].amount');
106+
});
107+
108+
it('under `$not`', () => {
109+
const err = refusalFor(() => run({ $not: { name: {} } }));
110+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
111+
expect(err?.message).toContain('where.$not.name');
112+
});
113+
114+
it('several combinators deep', () => {
115+
const err = refusalFor(() => run({ $and: [{ $or: [{ $not: { name: {} } }] }] }));
116+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
117+
});
118+
119+
it('through `matchesWhere` directly, under `$and`', () => {
120+
const err = refusalFor(() => matchesWhere({ name: 'Globex' }, { $and: [{ name: {} }] }));
121+
expect(err).toMatchObject({ code: 'INVALID_FILTER', status: 400 });
122+
});
123+
});
124+
125+
describe('non-empty constraints answer exactly as before', () => {
126+
it('an operator constraint', () => {
127+
expect(namesAnswered({ name: { $eq: 'Globex' } })).toEqual(['Globex']);
128+
expect(namesAnswered({ amount: { $gt: 1000 } })).toEqual(['Acme Corp']);
129+
expect(matchesWhere({ name: 'Globex' }, { name: { $ne: 'Globex' } })).toBe(false);
130+
});
131+
132+
it('an implicit-equality comparand', () => {
133+
expect(namesAnswered({ name: 'Globex' })).toEqual(['Globex']);
134+
});
135+
136+
it('an empty NODE — the whole `where`, or a combinator arm — is the identity, not a field constraint', () => {
137+
expect(namesAnswered({})).toEqual(['Acme Corp', 'Globex']);
138+
expect(namesAnswered({ $and: [{}] })).toEqual(['Acme Corp', 'Globex']);
139+
expect(matchesWhere({ name: 'Globex' }, {})).toBe(true);
140+
});
141+
});
142+
});

‎packages/services/service-analytics/src/preview-evaluator.ts‎

Lines changed: 84 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
// live only in the pending seed draft, so there is no live path to hand the
2323
// query to. It is REFUSED — `INVALID_FILTER` / 400, the envelope this package's
2424
// `where` door already speaks — and never answered true. See
25-
// PREVIEW_FIELD_OPERATORS.
25+
// PREVIEW_FIELD_OPERATORS. [#19835] So is a field constraint carrying ZERO
26+
// operators (`{ name: {} }`), at any depth — see isEmptyFieldConstraint.
2627

2728
import {
2829
calendarPartsInTzOrUtc,
@@ -193,6 +194,66 @@ function previewUnevaluableOperatorError(op: string, field: string): Error {
193194
);
194195
}
195196

197+
/**
198+
* [#19835] Is this field spec `{}` — a field constrained by ZERO operators?
199+
*
200+
* A plain object with no own enumerable keys, and nothing else — the predicate
201+
* `driver-memory`, `driver-mongodb`, `driver-sql` and `@objectstack/formula`
202+
* each apply under the same name. A `Date`, a `RegExp` or a class instance also
203+
* enumerates to nothing, but it is a COMPARAND rather than a constraint, so the
204+
* prototype check keeps it out of this refusal exactly as it does there.
205+
*
206+
* Mirrored locally, not imported: the exported copy lives in `driver-memory`
207+
* (`filter-refusal.ts`), a package this service does not depend on, and a
208+
* dependency on a driver for a four-line predicate is the wrong direction.
209+
*/
210+
function isEmptyFieldConstraint(spec: unknown): boolean {
211+
if (spec === null || typeof spec !== 'object' || Array.isArray(spec)) return false;
212+
const proto = Object.getPrototypeOf(spec);
213+
if (proto !== Object.prototype && proto !== null) return false;
214+
return Object.keys(spec as Row).length === 0;
215+
}
216+
217+
/**
218+
* [#19835] `{ field: {} }` — a field constrained by ZERO operators — REFUSED,
219+
* in the same `INVALID_FILTER` / 400 envelope as
220+
* {@link previewUnevaluableOperatorError}.
221+
*
222+
* ⛔ It used to MATCH EVERY ROW, and not by anyone's decision: the per-field
223+
* loop in {@link matchesWhere} iterates the constraint's entries, an empty
224+
* object has none, so the loop body never ran and the row fell through to the
225+
* function's closing `return true`. The operator-vocabulary refusal (#19810)
226+
* could not reach it either — with no key there is no operator to look up and
227+
* no lookup to fail.
228+
*
229+
* Every shipped backend already refuses this shape (#5240 ruled it: refused
230+
* everywhere, one wording): `driver-memory`'s and `driver-mongodb`'s
231+
* `emptyFieldConstraintError`, and `driver-sql`'s at the top level and inside
232+
* `$and`/`$or`/`$not` alike. This package's own `where` door refuses it too
233+
* (`filter-normalizer`'s wrapper arm). So the draft preview charted every row
234+
* for a filter publish never answers at all — the preview/publish divergence
235+
* this evaluator's other refusals exist to make visible.
236+
*
237+
* ⛔ Refused, NOT answered as "matches nothing". `{ status: {} }` does not mean
238+
* "no rows" — read literally it means "rows whose status is anything", and the
239+
* shape is almost always an authoring accident (a filter builder that recorded
240+
* a field and never its operator). Either silent reading hands the author a row
241+
* count they never asked for; only the refusal names the constraint to repair.
242+
*
243+
* The message follows the drivers' wording (constraint, position, the two legal
244+
* repairs) and carries no tracker number, per the runtime-string rule.
245+
*/
246+
function previewEmptyFieldConstraintError(field: string, path: string): Error {
247+
return invalidFilterError(
248+
`[analytics] Field constraint at ${path} carries zero operators ({ "${field}": {} }). A field ` +
249+
`constraint must name at least one operator (e.g. { "${field}": { "$eq": "value" } }) or be a ` +
250+
`direct comparand (e.g. { "${field}": "value" }). It is refused rather than evaluated: the ` +
251+
`draft-data preview used to answer it with EVERY row, while every data driver and the live ` +
252+
`analytics filter refuse it — so the drafted chart was drawn over rows the published one never ` +
253+
`returns. It does not mean "no rows" either; name the operator the constraint was meant to carry.`,
254+
);
255+
}
256+
196257
/**
197258
* [#19810] Refuse a `where` this face cannot evaluate BEFORE any row is read.
198259
*
@@ -206,17 +267,30 @@ function previewUnevaluableOperatorError(op: string, field: string): Error {
206267
*
207268
* It mirrors {@link matchesWhere}'s own traversal exactly, malformed shapes
208269
* included — a non-array `$and` is left for `matchesWhere` to fault on as it
209-
* always has, so this gate widens no refusal beyond the operator vocabulary.
270+
* always has, so this gate widens no refusal beyond the operator vocabulary
271+
* and the zero-operator constraint.
272+
*
273+
* [#19835] The zero-operator constraint is judged HERE, for the whole tree,
274+
* rather than only where {@link matchesWhere} meets it: that walk
275+
* short-circuits (`every`/`some`, and a node returns on its first false entry),
276+
* so a `{ $or: [{ name: 'Globex' }, { amount: {} }] }` would refuse or answer
277+
* depending on which ROW was being tested. A malformed filter is refused for
278+
* every row or none — the posture `@objectstack/formula`'s `assertFilterShape`
279+
* takes for the same shape — and nesting under `$and`/`$or`/`$not` cannot
280+
* route around it, the position `driver-sql` once dropped it in.
210281
*/
211-
function assertPreviewCanEvaluate(where: Record<string, unknown> | undefined): void {
282+
function assertPreviewCanEvaluate(where: Record<string, unknown> | undefined, path = 'where'): void {
212283
if (!where) return;
213284
for (const [key, cond] of Object.entries(where)) {
285+
const here = `${path}.${key}`;
214286
if (key === '$and' || key === '$or') {
215-
if (Array.isArray(cond)) for (const arm of cond) assertPreviewCanEvaluate(arm as Row);
287+
if (Array.isArray(cond)) cond.forEach((arm, i) => assertPreviewCanEvaluate(arm as Row, `${here}[${i}]`));
216288
} else if (key === '$not') {
217289
if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) {
218-
assertPreviewCanEvaluate(cond as Row);
290+
assertPreviewCanEvaluate(cond as Row, here);
219291
}
292+
} else if (isEmptyFieldConstraint(cond)) {
293+
throw previewEmptyFieldConstraintError(key, here);
220294
} else if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) {
221295
for (const op of Object.keys(cond as Row)) {
222296
if (!PREVIEW_FIELD_OPERATORS.has(op)) throw previewUnevaluableOperatorError(op, key);
@@ -240,6 +314,11 @@ export function matchesWhere(row: Row, where: Record<string, unknown> | undefine
240314
if (!(cond as Row[]).some((c) => matchesWhere(row, c as Row))) return false;
241315
} else if (key === '$not') {
242316
if (matchesWhere(row, cond as Row)) return false;
317+
} else if (isEmptyFieldConstraint(cond)) {
318+
// [#19835] Zero entries would leave the loop below unrun and fall through
319+
// to a MATCH. Refused here too, so a direct caller of this matcher gets
320+
// the same answer the row-independent gate gives a whole query.
321+
throw previewEmptyFieldConstraintError(key, key);
243322
} else if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) {
244323
for (const [op, expected] of Object.entries(cond as Row)) {
245324
if (!matchOp(row[key], op, expected, key)) return false;

0 commit comments

Comments
 (0)