Skip to content

Commit f5fe061

Browse files
authored
fix(data): implicit field filters AND-merge with an explicit filter instead of being silently dropped (#4164) (#4190)
`?filter={"status":"open"}&owner_id=usr_1` applied only the explicit filter: the implicit-filter pass ran under `if (!options.where)`, so leftover REAL field params were neither merged nor reported — they rode to the engine as stray AST keys no driver reads, and the response over-returned. The mirror of #4134's silent zero. They now compose as `{ $and: [explicit, implicit] }` — the same combinator the engine uses to fold the $search predicate and expand filters into an existing `where`, pinned across backends by the #3774 conformance suite. The #4134 field gate runs before the merge, so every merged key is a verified field name. Contradictory sides intersect to an honest zero; a vacuous explicit filter ({} or '') leaves implicit predicates standing alone; a non-object where (unparseable filter JSON, #4181) is deliberately not merged into; pagination totals are computed over the merged predicate. Filed while reproducing the excluded edge: #4181 — an unparseable filter string silently returns the unfiltered page.
1 parent d7851cf commit f5fe061

5 files changed

Lines changed: 200 additions & 23 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(data): implicit field filters compose with an explicit `filter` by AND instead of being silently dropped (#4164)
6+
7+
`GET /api/v1/data/:object?filter={"status":"open"}&owner_id=usr_1` used to
8+
apply only the explicit filter: the bare `owner_id` predicate was neither
9+
merged nor reported — it rode to the engine as a stray AST key no driver
10+
reads, and the response over-returned. The mirror of #4134's silent zero,
11+
same disease, opposite direction.
12+
13+
The two now compose the way the request reads: `{ $and: [explicit, implicit] }`
14+
— the same combinator the engine already uses to fold the `search` predicate
15+
into an existing `where`, and one the cross-backend filter-logic conformance
16+
suite pins. Contradictory sides (`?filter={"status":"open"}&status=closed`)
17+
apply both predicates and intersect to an honest empty set. Pagination totals
18+
(`total` / `hasMore`) are computed over the merged predicate, so they cannot
19+
disagree with `records`.
20+
21+
**What changes for callers:** requests that sent both an explicit `filter` and
22+
bare field parameters now get the narrower, as-written result set instead of
23+
the explicit filter alone. Requests sending only one of the two mechanisms are
24+
unaffected. Thanks to #4134 (shipped previously), every bare parameter that
25+
reaches the merge is a verified field name, so the merge can never introduce a
26+
zero-matching predicate.

content/docs/api/data-api.mdx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,12 @@ Query records with filtering, sorting, selection, and pagination.
3232

3333
A query parameter the endpoint does not reserve is read as a field-level
3434
equality filter, so `?status=done` is shorthand for
35-
`?filter={"status":"done"}`. Because such a parameter *is* a predicate, one
36-
naming a field the object does not have could only ever match zero records —
37-
so the endpoint rejects it instead of returning an empty page:
35+
`?filter={"status":"done"}`. When an explicit `filter` is also present, the
36+
two compose by AND — `?filter={"amount":{"$gte":100}}&status=done` applies
37+
both predicates, the same way the `search` parameter composes with `filter`.
38+
Because such a parameter *is* a predicate, one naming a field the object does
39+
not have could only ever match zero records — so the endpoint rejects it
40+
instead of returning an empty page:
3841

3942
```http
4043
GET /api/v1/data/showcase_task?pageSize=5

packages/metadata-protocol/src/protocol.ts

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3182,27 +3182,44 @@ export class ObjectStackProtocolImplementation implements
31823182
}
31833183

31843184
// Flat field filters: REST-style query params like ?id=abc&status=open
3185-
// After extracting all known query parameters, any remaining keys are
3186-
// treated as implicit field-level equality filters merged into `where`.
3187-
// Every one of them is a verified field name by this point.
3185+
// are implicit field-level equality predicates. Every leftover key is a
3186+
// verified field name by this point — the #4134 gate above runs FIRST,
3187+
// which is exactly what makes the merge below safe: an unknown name
3188+
// already 400'd, so nothing merged here can be a predicate that matches
3189+
// nothing.
31883190
//
3189-
// The `!options.where` guard is #4134's deliberate scope edge: when an
3190-
// explicit filter won, a leftover REAL field name is still dropped
3191-
// silently (it rides on to `engine.find` as a stray AST key no driver
3192-
// reads). Same disease, opposite direction — over-returning rather than
3193-
// zeroing — and fixing it means choosing a semantics (AND-merge vs.
3194-
// reject the conflict), so it is filed as #4164 rather than smuggled in
3195-
// here. The UNKNOWN half is already loud: the gate above runs before
3196-
// this branch, so a bad name 400s either way.
3197-
if (!options.where) {
3191+
// [#4164] Implicit predicates now COMPOSE with an explicit `where`
3192+
// instead of being silently dropped. `?filter={...}&status=open` means
3193+
// what it says — both apply, `{ $and: [explicit, implicit] }` — the
3194+
// same composition the engine itself uses to fold the `$search`
3195+
// predicate or an expand's declared filter into an existing `where`,
3196+
// and a combinator the #3774 conformance suite pins across every
3197+
// FilterCondition backend. Contradictory sides need no special case:
3198+
// both predicates apply and the intersection is an HONEST zero (the
3199+
// filters ran), unlike the pre-#4134 zero (a filter that never ran).
3200+
// Consuming the keys here also stops them riding to `engine.find` as
3201+
// stray top-level AST junk, and count() below reads the same
3202+
// `options.where`, so pagination totals see the merged predicate too.
3203+
//
3204+
// Deliberately NOT merged: a non-object truthy `where` — the parse
3205+
// tolerance above keeps an unparseable `filter` JSON string as-is
3206+
// (#4181), and folding real predicates into that garbage would neither
3207+
// apply them nor surface the actual bug. That path keeps its pre-#4164
3208+
// shape until the tolerance itself is fixed at the source.
3209+
const explicitWhere = options.where;
3210+
const whereIsMergeable = !explicitWhere || typeof explicitWhere === 'object';
3211+
if (leftoverParams.length > 0 && whereIsMergeable) {
31983212
const implicitFilters: Record<string, unknown> = {};
31993213
for (const key of leftoverParams) {
32003214
implicitFilters[key] = options[key];
32013215
delete options[key];
32023216
}
3203-
if (Object.keys(implicitFilters).length > 0) {
3204-
options.where = implicitFilters;
3205-
}
3217+
// An absent or empty explicit filter (`?filter={}`, `?filter=`) is
3218+
// vacuous — the implicit predicates stand alone rather than being
3219+
// wrapped in a one-armed `$and`.
3220+
options.where = !explicitWhere || Object.keys(explicitWhere).length === 0
3221+
? implicitFilters
3222+
: { $and: [explicitWhere, implicitFilters] };
32063223
}
32073224

32083225
// Route to engine.aggregate() when the query has GROUP BY / aggregations.

packages/objectql/src/protocol-data.test.ts

Lines changed: 88 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -760,16 +760,99 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => {
760760
).rejects.toMatchObject({ field: 'zzzz', fields: ['zzzz', 'yyyy'] });
761761
});
762762

763-
it('leaves the known-field-plus-explicit-filter case alone (#4164)', async () => {
764-
// The scope edge, pinned so a later change to it is deliberate: a
765-
// REAL field name alongside an explicit filter is still dropped
766-
// silently. #4134 made only the UNKNOWN half loud.
763+
it('AND-merges a known field with an explicit filter instead of dropping it (#4164)', async () => {
764+
// The #4134 pin test used to freeze the drop behavior here; #4164
765+
// is the deliberate change it was waiting for. Both predicates now
766+
// apply, and the consumed key no longer rides to the engine as a
767+
// stray top-level AST key.
767768
const { protocol, engine } = makeProtocol();
768769
await protocol.findData({
769770
object: 'showcase_task',
770771
query: { filter: { status: 'open' }, owner_id: 'usr_1' },
771772
});
772-
expect(engine.find.mock.calls[0][1].where).toEqual({ status: 'open' });
773+
const opts = engine.find.mock.calls[0][1];
774+
expect(opts.where).toEqual({ $and: [{ status: 'open' }, { owner_id: 'usr_1' }] });
775+
expect(opts.owner_id).toBeUndefined();
776+
});
777+
778+
it('merges every implicit field as ONE $and arm', async () => {
779+
const { protocol, engine } = makeProtocol();
780+
await protocol.findData({
781+
object: 'showcase_task',
782+
query: { filter: { status: 'open' }, owner_id: 'usr_1', due_date: '2026-08-01' },
783+
});
784+
expect(engine.find.mock.calls[0][1].where).toEqual({
785+
$and: [{ status: 'open' }, { owner_id: 'usr_1', due_date: '2026-08-01' }],
786+
});
787+
});
788+
789+
it('a contradictory pair applies both sides — an honest empty set, not a silent wide one', async () => {
790+
// `?filter={"status":"open"}&status=closed` — both predicates run;
791+
// the intersection being empty is the caller's contradiction, not a
792+
// dropped filter. No special-casing of same-field collisions.
793+
const { protocol, engine } = makeProtocol();
794+
await protocol.findData({
795+
object: 'showcase_task',
796+
query: { filter: { status: 'open' }, status: 'closed' },
797+
});
798+
expect(engine.find.mock.calls[0][1].where).toEqual({
799+
$and: [{ status: 'open' }, { status: 'closed' }],
800+
});
801+
});
802+
803+
it('a vacuous explicit filter ({} or empty string) lets implicit predicates stand alone', async () => {
804+
const { protocol, engine } = makeProtocol();
805+
await protocol.findData({
806+
object: 'showcase_task',
807+
query: { filter: {}, owner_id: 'usr_1' },
808+
});
809+
expect(engine.find.mock.calls[0][1].where).toEqual({ owner_id: 'usr_1' });
810+
// `?filter=` — the parse tolerance keeps '' as-is; the old guard
811+
// treated it as no-filter, and the merge must keep doing so.
812+
await protocol.findData({
813+
object: 'showcase_task',
814+
query: { filter: '', owner_id: 'usr_1' },
815+
});
816+
expect(engine.find.mock.calls[1][1].where).toEqual({ owner_id: 'usr_1' });
817+
});
818+
819+
it('count() sees the merged where — pagination totals cannot disagree with records (#4164)', async () => {
820+
const { protocol, engine } = makeProtocol();
821+
await protocol.findData({
822+
object: 'showcase_task',
823+
query: { filter: { status: 'open' }, owner_id: 'usr_1', top: 5 },
824+
});
825+
expect(engine.count).toHaveBeenCalledTimes(1);
826+
expect(engine.count.mock.calls[0][1].where).toEqual({
827+
$and: [{ status: 'open' }, { owner_id: 'usr_1' }],
828+
});
829+
});
830+
831+
it('merges identically through the $filter JSON-string alias', async () => {
832+
const { protocol, engine } = makeProtocol();
833+
await protocol.findData({
834+
object: 'showcase_task',
835+
query: { $filter: JSON.stringify({ status: 'open' }), owner_id: 'usr_1' },
836+
});
837+
expect(engine.find.mock.calls[0][1].where).toEqual({
838+
$and: [{ status: 'open' }, { owner_id: 'usr_1' }],
839+
});
840+
});
841+
842+
it('leaves a non-object where (unparseable filter JSON) untouched — pinned until #4181', async () => {
843+
// `?filter={oops` — the parse tolerance keeps the raw string and it
844+
// becomes `where`. Folding real predicates into that garbage would
845+
// neither apply them nor surface the tolerance bug itself (#4181),
846+
// so this path keeps its pre-#4164 shape: string where forwarded,
847+
// implicit key left as a stray option.
848+
const { protocol, engine } = makeProtocol();
849+
await protocol.findData({
850+
object: 'showcase_task',
851+
query: { filter: '{oops', owner_id: 'usr_1' },
852+
});
853+
const opts = engine.find.mock.calls[0][1];
854+
expect(opts.where).toBe('{oops');
855+
expect(opts.owner_id).toBe('usr_1');
773856
});
774857

775858
it('rejects even when an explicit filter rode along — the 400 must not depend on it', async () => {

packages/objectql/src/protocol-unknown-query-param.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@ function makeMemoryDriver() {
4646
const matchesWhere = (row: Record<string, unknown>, where: any): boolean => {
4747
if (!where || typeof where !== 'object') return true;
4848
for (const [k, v] of Object.entries(where)) {
49+
// `$and` must be honored, not skipped — #4164 composes the explicit
50+
// filter and the implicit field predicates through it, so a matcher
51+
// that ignores `$and` would green-light a merge that does nothing.
52+
if (k === '$and' && Array.isArray(v)) {
53+
if (!v.every((arm) => matchesWhere(row, arm))) return false;
54+
continue;
55+
}
4956
if (k.startsWith('$')) continue;
5057
const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
5158
const a = row[k] === undefined ? null : row[k];
@@ -206,4 +213,45 @@ describe('#4134 — unknown list query params (real ObjectQL engine)', () => {
206213
protocol.findData({ object: 'no_such_object', query: { pageSize: '5' } }),
207214
).rejects.toMatchObject({ status: 404, code: 'OBJECT_NOT_FOUND' });
208215
});
216+
217+
// ─────────────────────────────────────────────────────────────
218+
// #4164 — implicit predicates compose with an explicit filter
219+
// ─────────────────────────────────────────────────────────────
220+
221+
it('a bare field param NARROWS an explicit filter instead of vanishing (#4164)', async () => {
222+
// Pre-#4164 the `title` predicate was dropped and this returned all 8
223+
// open tasks — over-returning, the mirror of #4134's zeroing.
224+
const r: any = await protocol.findData({
225+
object: 'showcase_task',
226+
query: { filter: JSON.stringify({ status: 'open' }), title: 'Task 3' },
227+
});
228+
expect(r.total).toBe(1);
229+
expect(r.records.map((x: any) => x.id)).toEqual(['t3']);
230+
});
231+
232+
it('contradictory sides intersect to an honest zero — both filters ran', async () => {
233+
// 'Task 3' is open, so requiring status=done alongside it matches
234+
// nothing. Unlike the #4134 zero, every written predicate was applied.
235+
const r: any = await protocol.findData({
236+
object: 'showcase_task',
237+
query: { filter: JSON.stringify({ status: 'done' }), title: 'Task 3' },
238+
});
239+
expect(r.total).toBe(0);
240+
});
241+
242+
it('pagination totals are computed over the MERGED predicate', async () => {
243+
// 8 open rows share created_at; page of 3 → total must be 8 (the
244+
// merged match count), not 10 (unfiltered) nor 3 (page-local).
245+
const r: any = await protocol.findData({
246+
object: 'showcase_task',
247+
query: {
248+
filter: JSON.stringify({ status: 'open' }),
249+
created_at: '2026-07-30T00:00:00.000Z',
250+
top: 3,
251+
},
252+
});
253+
expect(r.records).toHaveLength(3);
254+
expect(r.total).toBe(8);
255+
expect(r.hasMore).toBe(true);
256+
});
209257
});

0 commit comments

Comments
 (0)