Skip to content

Commit 7dbf4c3

Browse files
os-zhuangclaude
andauthored
test: conjoin $or/$and with sibling filters in twelve driver doubles (part of #7620) (#8493)
Twelve in-memory WHERE matchers across packages/plugins/plugin-sharing, packages/plugins/plugin-security and packages/runtime returned early on $or/$and, discarding every sibling equality key in the same filter object — a real driver ANDs them. Corrected to the same conjoin-with-siblings shape already used by packages/objectql's six (#7846) and by several already-fixed siblings in these two packages. Measured live-vs-dormant per file via an fs.appendFileSync probe (with a positive control proving it would catch a live case): all twelve are dormant today, for two different reasons. plugin-sharing's six never receive $or/$and at all. plugin-security's five and the one runtime file do receive them, but always as the sole key in their filter object (no sibling ever present alongside), so early-return and conjoin produce identical results in every observed call. No test outcome changes. Re-grepped the issue's file enumeration at this branch's base ref rather than trusting it: plugin-sharing/src/sharing-rule.test.ts was already fixed, and three files this commit touches were never named in the issue (plugin-sharing/src/sharing-service.test.ts, plugin-security/src/check-only-write-scope.test.ts, plugin-security/src/select-only-write-visibility.test.ts). Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8c8f0df commit 7dbf4c3

13 files changed

Lines changed: 153 additions & 26 deletions
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
---
2+
"@objectstack/plugin-sharing": patch
3+
"@objectstack/plugin-security": patch
4+
"@objectstack/runtime": patch
5+
---
6+
7+
test: twelve in-memory driver doubles in `plugin-sharing`, `plugin-security` and
8+
`runtime` conjoin `$or`/`$and` with their sibling filters instead of short-circuiting
9+
(part of #7620)
10+
11+
Twelve test files across three packages built an in-memory driver whose `WHERE`
12+
matcher **returned early** on `$or` (and usually `$and`), discarding every sibling
13+
equality key in the same object:
14+
15+
```ts
16+
if (Array.isArray(filter.$or)) return filter.$or.some((f) => matches(row, f));
17+
if (Array.isArray(filter.$and)) return filter.$and.every((f) => matches(row, f));
18+
for (const [k, v] of Object.entries(filter)) { /* siblings, never reached */ }
19+
```
20+
21+
A real driver ANDs them. So a query mixing an equality key with a top-level `$or`
22+
would have been answered on the `$or` alone, handing back rows the sibling key
23+
would have excluded — not a stricter or looser edge case, a different query, with
24+
the suite staying green while testing it.
25+
26+
The fix is the ~2-line change already established by `packages/objectql`'s six
27+
(#7846) and by several already-corrected siblings in these same two packages
28+
(`bu-tree-recompute.test.ts`, `recipient-width.test.ts`, `sharing-rule.test.ts`,
29+
`system-caller-inert-grant.test.ts`, `business-unit-graph.test.ts`): fold the
30+
combinator check into a guard that only short-circuits on **failure**, so the
31+
sibling-key loop still runs afterward.
32+
33+
**This lane's file enumeration differs from the issue body**, which is stale
34+
(confirmed in-thread): `plugin-sharing/src/sharing-rule.test.ts` was already fixed
35+
before this PR, and three files this PR fixes were never named in the issue at all
36+
(`plugin-sharing/src/sharing-service.test.ts`,
37+
`plugin-security/src/check-only-write-scope.test.ts`,
38+
`plugin-security/src/select-only-write-visibility.test.ts` — found by re-grepping
39+
`$or` across both packages' `src/*.test.ts` at this PR's base ref rather than
40+
trusting the issue's list). Files corrected here:
41+
42+
- `packages/plugins/plugin-sharing/src/`: `authored-row-write-deferral.test.ts`,
43+
`boot-backfill.test.ts`, `bulk-recompute.test.ts`, `record-share-cascade.test.ts`,
44+
`sharing-service.test.ts`, `system-write-skip-notice.test.ts`
45+
- `packages/plugins/plugin-security/src/`: `authored-row-write-verdict.test.ts`,
46+
`check-only-write-scope.test.ts`, `row-write-widener-composition.test.ts`,
47+
`select-only-write-visibility.test.ts`, `vama-write-path-convergence.test.ts`
48+
- `packages/runtime/src/domains/`: `share-links-enforcement-context.test.ts`
49+
50+
**All twelve are dormant today — measured, not assumed**, via the same
51+
`fs.appendFileSync` probe discipline #7846 used (a `console.log` probe was tried
52+
first, returned nothing, and was correctly distrusted rather than read as "zero
53+
calls"). Per-package results, with a positive control proving the probe would have
54+
caught a live case:
55+
56+
- `plugin-sharing`'s six: **0 combinator calls out of ~1.52M matcher invocations**
57+
across the six suites (dominated by one bulk-recompute test's own scale; the
58+
other five totalled ~2,755). Positive control: instrumenting the already-fixed
59+
`sharing-rule.test.ts` with the identical probe, then reverting it, recorded 151
60+
`$or` and 3 `$and` calls in the same run — proof the zero above is a real
61+
absence, not a dead probe.
62+
- `plugin-security`'s five: **not** all-zero like objectql/sharing — `$and`
63+
appears 23–71 times per file and `$or` appears twice in one file
64+
(`authored-row-write-verdict.test.ts`). But every single one of those calls
65+
carried the combinator as the **only** key in its filter object (`other: []`),
66+
so early-return and conjoin produce identical results in every case observed —
67+
dormant in the sense that decides this PR, for a different reason than
68+
objectql/sharing (never invoked, vs. invoked but never mixed with a sibling).
69+
- `runtime`'s one file: 1 `$or` and 10 `$and` calls, same "combinator-only, no
70+
siblings" shape — dormant for the same reason as `plugin-security`.
71+
72+
No existing test outcome changes anywhere, and none should: `plugin-sharing`
73+
(21 files / 569 tests), `plugin-security` (52 files / 1037 tests) and `runtime`
74+
(151 files / 2317 tests) are green before and after, byte-identical assertions.
75+
76+
**Operator-support measurement, per the card's ask**: unlike the objectql six
77+
(measured byte-for-byte identical operator support), these twelve are **not** a
78+
single lowest common denominator. `plugin-sharing`'s matchers mostly support
79+
`$in`, several add `$ne` or `$gte`/`$gt` (the tree/graph-shaped ones), and
80+
`sharing-service.test.ts` supports only `$in`. `plugin-security`'s five and the
81+
`runtime` one are the most uniform subset (`$in` only, identical shape). A shared
82+
helper across all sixteen files repo-wide would have to be a strict superset or
83+
force some suites to drop operators they use — a materially different tradeoff
84+
than the objectql lane's "no lowest common denominator to flatten to" finding.
85+
86+
Deliberately **not** extracted into a shared helper here either, for the same
87+
reason `packages/objectql`'s six gave: keeping each test double's substrate
88+
self-contained so it can fail independently of any other suite's fixture file.
89+
Where a cross-package helper would live, and whether the missing regression guard
90+
is worth adding, are open questions for the PM now that all three lanes of #7620
91+
have landed — not decided in this PR.

packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,11 @@ function makeEngine(opts: { findOneThrows?: boolean } = {}) {
192192
};
193193
const matches = (row: any, filter: any): boolean => {
194194
if (!filter || typeof filter !== 'object') return true;
195-
if (Array.isArray(filter.$or)) return filter.$or.some((f: any) => matches(row, f));
196-
if (Array.isArray(filter.$and)) return filter.$and.every((f: any) => matches(row, f));
195+
// `$or` / `$and` are conjoined WITH their sibling keys, the way a real
196+
// driver ANDs them — a short-circuiting `return` here would discard every
197+
// sibling equality key in the same object. See #7620.
198+
if (Array.isArray(filter.$or) && !filter.$or.some((f: any) => matches(row, f))) return false;
199+
if (Array.isArray(filter.$and) && !filter.$and.every((f: any) => matches(row, f))) return false;
197200
for (const [k, v] of Object.entries(filter)) {
198201
if (k === '$or' || k === '$and') continue;
199202
if (v != null && typeof v === 'object' && '$in' in (v as any)) {

packages/plugins/plugin-security/src/check-only-write-scope.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,11 @@ function makeEngine() {
253253
};
254254
const matches = (row: any, filter: any): boolean => {
255255
if (!filter || typeof filter !== 'object') return true;
256-
if (Array.isArray(filter.$or)) return filter.$or.some((f: any) => matches(row, f));
257-
if (Array.isArray(filter.$and)) return filter.$and.every((f: any) => matches(row, f));
256+
// `$or` / `$and` are conjoined WITH their sibling keys, the way a real
257+
// driver ANDs them — a short-circuiting `return` here would discard every
258+
// sibling equality key in the same object. See #7620.
259+
if (Array.isArray(filter.$or) && !filter.$or.some((f: any) => matches(row, f))) return false;
260+
if (Array.isArray(filter.$and) && !filter.$and.every((f: any) => matches(row, f))) return false;
258261
for (const [k, v] of Object.entries(filter)) {
259262
if (k === '$or' || k === '$and') continue;
260263
if (v != null && typeof v === 'object' && '$in' in (v as any)) {

packages/plugins/plugin-security/src/row-write-widener-composition.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,11 @@ function makeEngine() {
218218
};
219219
const matches = (row: any, filter: any): boolean => {
220220
if (!filter || typeof filter !== 'object') return true;
221-
if (Array.isArray(filter.$or)) return filter.$or.some((f: any) => matches(row, f));
222-
if (Array.isArray(filter.$and)) return filter.$and.every((f: any) => matches(row, f));
221+
// `$or` / `$and` are conjoined WITH their sibling keys, the way a real
222+
// driver ANDs them — a short-circuiting `return` here would discard every
223+
// sibling equality key in the same object. See #7620.
224+
if (Array.isArray(filter.$or) && !filter.$or.some((f: any) => matches(row, f))) return false;
225+
if (Array.isArray(filter.$and) && !filter.$and.every((f: any) => matches(row, f))) return false;
223226
for (const [k, v] of Object.entries(filter)) {
224227
if (k === '$or' || k === '$and') continue;
225228
if (v != null && typeof v === 'object' && '$in' in (v as any)) {

packages/plugins/plugin-security/src/select-only-write-visibility.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,11 @@ function makeEngine() {
243243
};
244244
const matches = (row: any, filter: any): boolean => {
245245
if (!filter || typeof filter !== 'object') return true;
246-
if (Array.isArray(filter.$or)) return filter.$or.some((f: any) => matches(row, f));
247-
if (Array.isArray(filter.$and)) return filter.$and.every((f: any) => matches(row, f));
246+
// `$or` / `$and` are conjoined WITH their sibling keys, the way a real
247+
// driver ANDs them — a short-circuiting `return` here would discard every
248+
// sibling equality key in the same object. See #7620.
249+
if (Array.isArray(filter.$or) && !filter.$or.some((f: any) => matches(row, f))) return false;
250+
if (Array.isArray(filter.$and) && !filter.$and.every((f: any) => matches(row, f))) return false;
248251
for (const [k, v] of Object.entries(filter)) {
249252
if (k === '$or' || k === '$and') continue;
250253
if (v != null && typeof v === 'object' && '$in' in (v as any)) {

packages/plugins/plugin-security/src/vama-write-path-convergence.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,11 @@ function makeEngine() {
102102
};
103103
const matches = (row: any, filter: any): boolean => {
104104
if (!filter || typeof filter !== 'object') return true;
105-
if (Array.isArray(filter.$or)) return filter.$or.some((f: any) => matches(row, f));
106-
if (Array.isArray(filter.$and)) return filter.$and.every((f: any) => matches(row, f));
105+
// `$or` / `$and` are conjoined WITH their sibling keys, the way a real
106+
// driver ANDs them — a short-circuiting `return` here would discard every
107+
// sibling equality key in the same object. See #7620.
108+
if (Array.isArray(filter.$or) && !filter.$or.some((f: any) => matches(row, f))) return false;
109+
if (Array.isArray(filter.$and) && !filter.$and.every((f: any) => matches(row, f))) return false;
107110
for (const [k, v] of Object.entries(filter)) {
108111
if (k === '$or' || k === '$and') continue;
109112
if (v != null && typeof v === 'object' && '$in' in (v as any)) {

packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,11 @@ function makeEngine() {
181181
};
182182
const matches = (row: any, filter: any): boolean => {
183183
if (!filter || typeof filter !== 'object') return true;
184-
if (Array.isArray(filter.$or)) return filter.$or.some((f: any) => matches(row, f));
185-
if (Array.isArray(filter.$and)) return filter.$and.every((f: any) => matches(row, f));
184+
// `$or` / `$and` are conjoined WITH their sibling keys, the way a real
185+
// driver ANDs them — a short-circuiting `return` here would discard every
186+
// sibling equality key in the same object. See #7620.
187+
if (Array.isArray(filter.$or) && !filter.$or.some((f: any) => matches(row, f))) return false;
188+
if (Array.isArray(filter.$and) && !filter.$and.every((f: any) => matches(row, f))) return false;
186189
for (const [k, v] of Object.entries(filter)) {
187190
if (k === '$or' || k === '$and') continue;
188191
if (v != null && typeof v === 'object' && '$in' in (v as any)) {

packages/plugins/plugin-sharing/src/boot-backfill.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@ function makeEngine() {
2626
const ensure = (n: string) => (tables[n] ??= []);
2727
function matches(row: Row, f: any): boolean {
2828
if (!f || typeof f !== 'object') return true;
29-
if (Array.isArray(f.$or)) return f.$or.some((x: any) => matches(row, x));
30-
if (Array.isArray(f.$and)) return f.$and.every((x: any) => matches(row, x));
29+
// `$or` / `$and` are conjoined WITH their sibling keys, the way a real
30+
// driver ANDs them — a short-circuiting `return` here would discard every
31+
// sibling equality key in the same object. See #7620.
32+
if (Array.isArray(f.$or) && !f.$or.some((x: any) => matches(row, x))) return false;
33+
if (Array.isArray(f.$and) && !f.$and.every((x: any) => matches(row, x))) return false;
3134
for (const [k, v] of Object.entries(f)) {
3235
if (k === '$or' || k === '$and') continue;
3336
const rv = row[k];

packages/plugins/plugin-sharing/src/bulk-recompute.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,11 @@ function makeEngine() {
6161

6262
function matches(row: Row, f: any): boolean {
6363
if (!f || typeof f !== 'object') return true;
64-
if (Array.isArray(f.$or)) return f.$or.some((x: any) => matches(row, x));
65-
if (Array.isArray(f.$and)) return f.$and.every((x: any) => matches(row, x));
64+
// `$or` / `$and` are conjoined WITH their sibling keys, the way a real
65+
// driver ANDs them — a short-circuiting `return` here would discard every
66+
// sibling equality key in the same object. See #7620.
67+
if (Array.isArray(f.$or) && !f.$or.some((x: any) => matches(row, x))) return false;
68+
if (Array.isArray(f.$and) && !f.$and.every((x: any) => matches(row, x))) return false;
6669
for (const [k, v] of Object.entries(f)) {
6770
if (k === '$or' || k === '$and') continue;
6871
const rv = row[k];

packages/plugins/plugin-sharing/src/record-share-cascade.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,11 @@ function makeEngine() {
6262

6363
function matches(row: Row, f: any): boolean {
6464
if (!f || typeof f !== 'object') return true;
65-
if (Array.isArray(f.$or)) return f.$or.some((x: any) => matches(row, x));
66-
if (Array.isArray(f.$and)) return f.$and.every((x: any) => matches(row, x));
65+
// `$or` / `$and` are conjoined WITH their sibling keys, the way a real
66+
// driver ANDs them — a short-circuiting `return` here would discard every
67+
// sibling equality key in the same object. See #7620.
68+
if (Array.isArray(f.$or) && !f.$or.some((x: any) => matches(row, x))) return false;
69+
if (Array.isArray(f.$and) && !f.$and.every((x: any) => matches(row, x))) return false;
6770
for (const [k, v] of Object.entries(f)) {
6871
if (k === '$or' || k === '$and') continue;
6972
const rv = row[k];

0 commit comments

Comments
 (0)