fix(core): execute the declared $ operators in ValueDataSource, refuse the rest

.changeset/8447-valuedatasource-dollar-operator-refusal.md

@@ -0,0 +1,58 @@
+---
+'@object-ui/core': minor
+---
+
+`ValueDataSource`'s object-dialect matcher executes the operators the spec declares and
+refuses the rest, instead of waving every unrecognised one through (objectui#8447).
+
+**Breaking, deliberately — this MOVES RESULTS.** `matchesFilter` ended its operator
+switch with `default: break`, which adds no constraint, so an unrecognised operator
+matched **every** row, silently. The asymmetry sat inside one `if` in `find()`: an
+array `$filter` goes to `matchesASTFilter`, which refuses an unknown node, excludes the
+row and logs it; an object `$filter` went to `matchesFilter`, which admitted it and
+included the row. Same file, opposite defaults, and nothing told an author which dialect
+their filter took.
+
+**Now executed** — one arm per member of the spec's `FILTER_OPERATORS`, **all sixteen**,
+each answering the same question its AST twin already answered: `$eq`, `$ne`, `$gt`,
+`$gte`, `$lt`, `$lte`, `$in`, `$nin`, `$between`, `$contains`, `$icontains`,
+`$notContains`, `$startsWith`, `$endsWith`, `$null`, `$exists`. Eight of those were
+already implemented; the other eight selected every row. `$eq` is the sharpest of them:
+`{ age: { $eq: 26 } }` selected everything while the plain `{ age: 26 }` beside it was
+correct all along. `$exists` is the exact inverse of `$null` — `$exists: true` is
+IS NOT NULL — which is the lowering `convertFiltersToAST` already performs, not a
+reading invented here.
+
+**The closed vocabulary matters more than any one arm.** Because nothing the spec
+declares is refused by name, the parity guard in the companion test can assert that the
+case table equals `FILTER_OPERATORS` *exactly* — so a future spec release that adds a
+`$` operator turns this file red instead of letting the new spelling reach the refusal
+arm unannounced. An operator parked on a refused list would have been a hole that guard
+could not see into.
+
+**Now refused** — excluded and logged once per distinct refusal per `find()`, in
+`matchesASTFilter`'s own idiom. Everything here is OUTSIDE the declared vocabulary:
+
+- `$like` / `$ilike`, declared by `StringOperatorSchema` but deliberately staged out of
+  `FILTER_OPERATORS`; this matcher has no pattern engine.
+- `$regex` / `$options`, retired from the protocol — the refusal prints the spec's own
+  `RETIRED_FILTER_OPERATORS` prescription verbatim, the way the driver-side refusal
+  sites do.
+- Lowercase aliases (`$startswith`, `$notcontains`, `$notin`) and the off-spec
+  `$ncontains`: the canonical `$` spellings are camelCase, and growing alias arms in the
+  renderer would fossilise a second dialect.
+- A nested relation constraint (`{ profile: { verified: true } }`), refused by the key
+  it could not read; this matcher does not descend into relations.
+- `$and` / `$or` / `$not`, which are combinators rather than field operators. `$and` and
+  `$or` carry an array and were already excluding every row through the equality branch,
+  silently — those rows do not move, only the silence does. `$not` carries an object, so
+  it entered the operator branch, its inner field names were read as operator names and
+  each hit `default: break`: it matched **every** row, and now matches none. Executing a
+  group in this dialect is a feature with its own semantics to settle and is not part of
+  this repair; the AST array `$filter`, which the sibling arm of `find()` already
+  executes, is the door that works today.
+
+**Migration.** A filter that used any of the eight newly-executed operators was
+returning unfiltered data; it now returns the rows it names. A filter using a refused
+spelling now returns nothing and says why on the console — rewrite it in the canonical
+spelling the refusal prints, or express it as an AST array `$filter`.

packages/core/src/adapters/README.md

@@ -78,6 +78,40 @@ It implements `$filter` (both MongoDB-style objects and FilterNode AST arrays),
 `aggregate()` and `onMutation()`. `getAll()` returns a cloned snapshot and
 `count` the current length.
 
+#### What `$filter` executes, and what it refuses
+
+`find()` picks a matcher on the SHAPE of `$filter` — a FilterNode **array** goes
+to the AST matcher, an **object** to the `$`-dialect matcher — and since
+objectui#8447 the two answer the same question and refuse in the same way.
+
+The object dialect executes one arm per member of the spec's `FILTER_OPERATORS`:
+
+```text
+$eq  $ne  $gt  $gte  $lt  $lte  $in  $nin  $between
+$contains  $icontains  $notContains  $startsWith  $endsWith  $null  $exists
+```
+
+That is **all sixteen** — nothing the spec declares is refused by name.
+
+`$contains`, `$notContains`, `$startsWith` and `$endsWith` are **case-sensitive**;
+`$icontains` is the one case-insensitive member and its fold is **ASCII-only**.
+`$null` takes its direction from the value: `$null: true` is IS NULL, `$null: false`
+is IS NOT NULL. `$exists` is its exact inverse — `$exists: true` is IS NOT NULL — which
+is the lowering `convertFiltersToAST` already performs, not a reading invented here.
+
+Anything else is **refused**: the row is excluded and the reason is logged once per
+distinct refusal per `find()` — never passed through as "no constraint", which is
+what an unrecognised operator used to mean here. Refused on purpose, each with a
+prescription in the message:
+
+| spelling | why | write instead |
+| --- | --- | --- |
+| `$like` / `$ilike` | declared, but staged out of `FILTER_OPERATORS`; no pattern engine in memory | `$contains` / `$icontains` |
+| `$regex` / `$options` | retired from the protocol | `$icontains` |
+| `$startswith`, `$notcontains`, `$notin`, `$ncontains` | non-canonical spellings | the camelCase spelling |
+| `$and` / `$or` / `$not` | combinators, not field operators | an AST array `$filter` |
+| `{ relation: { field: … } }` | this matcher does not descend into relations | filter on the stored key |
+
 ### `resolveDataSource`
 
 Turns a `ViewData` config into a concrete adapter. This is the function a

packages/core/src/adapters/ValueDataSource.ts

@@ -18,7 +18,11 @@ import type {
   AggregateParams,
   AggregateResult,
 } from '@object-ui/types';
-import { asciiCaseInsensitiveContains, canonicalAstOperator } from '@objectstack/spec/data';
+import {
+  asciiCaseInsensitiveContains,
+  canonicalAstOperator,
+  RETIRED_FILTER_OPERATORS,
+} from '@objectstack/spec/data';
 import { emulateBatchTransaction } from './batchTransaction.js';
 
 // ---------------------------------------------------------------------------
@@ -236,54 +240,193 @@ function matchesASTFilter(record: any, filterNode: any, refusals: Set<string>):
 }
 
 /**
- * Simple in-memory filter evaluation.
- * Supports flat key-value equality and basic operators ($gt, $gte, $lt, $lte, $ne, $in).
+ * Evaluate ONE `$`-dialect operator against a record's value for a field.
+ *
+ * The vocabulary is the spec's own `FILTER_OPERATORS` (`@objectstack/spec/data`)
+ * and each arm answers the same question its AST twin answers in
+ * {@link matchesComparisonNode} — `$eq`/`=`, `$nin`/`nin`, `$startsWith`/
+ * `starts_with`, and so on, one-to-one across ALL SIXTEEN: every declared
+ * operator is executed here, none is refused by name. That pairing IS the
+ * fix for objectui#8447: `find()` picks between the two matchers on nothing
+ * more than whether `$filter` arrived as an array or an object, so any operator
+ * one of them executes and the other waves through is a result that changes
+ * with the SHAPE of the filter rather than with its meaning.
+ *
+ * ## What the `default` arm used to be
+ *
+ * `default: break` — which adds NO constraint. An operator this switch did not
+ * name therefore matched EVERY row, silently, while the array arm of the very
+ * same `if` refused the unknown node, excluded the row and logged it
+ * (objectui#7349). Measured before the fix: `$nin`, `$startsWith`, `$null` and
+ * `$exists` each returned the full set, and so did `$eq` — the most ordinary
+ * operator an author can reach for, whose plain-equality sibling
+ * (`{ age: 26 }`) was correct all along.
+ *
+ * The direction of the change is worth stating plainly, because it MOVES
+ * RESULTS: a filter that silently selected everything now selects the rows it
+ * names, and one this matcher cannot execute selects nothing and says so. Both
+ * are louder than what they replace; neither is "match everything", which was
+ * nobody's intent.
+ *
+ * ## What is refused, and why each one
+ *
+ * Nothing the spec DECLARES is on this list — `REFUSED_OPERATORS` in the
+ * companion test is empty, so the `FILTER_OPERATORS` parity guard covers the
+ * whole published vocabulary rather than a subset of it. What follows is
+ * everything OUTSIDE that vocabulary.
+ *
+ * - **`$like` / `$ilike`** — declared by `StringOperatorSchema` but deliberately
+ *   kept OUT of `FILTER_OPERATORS` while the faces that cannot execute them are
+ *   staged in (objectstack#7536). This matcher has no pattern engine, and the
+ *   spec's own note says naming them before an arm exists turns a loud refusal
+ *   into a dropped predicate — i.e. every row, the defect this file just fixed.
+ * - **`$regex` / `$options`** — RETIRED from the protocol (objectstack#4706).
+ *   The refusal prints `RETIRED_FILTER_OPERATORS`' prescription verbatim, the
+ *   way the five driver-side refusal sites do, so six faces say one thing.
+ * - **Anything else**, including a nested relation constraint
+ *   (`{ profile: { verified: true } }`), whose keys are field names rather than
+ *   operators: this matcher does not descend into relations.
+ */
+function matchesDollarOperator(
+  value: any,
+  operator: string,
+  target: any,
+  field: string,
+  refusals: Set<string>,
+): boolean {
+  switch (operator) {
+    case '$eq':
+      return value === target;
+    case '$ne':
+      return value !== target;
+    case '$gt':
+      return value > target;
+    case '$gte':
+      return value >= target;
+    case '$lt':
+      return value < target;
+    case '$lte':
+      return value <= target;
+    case '$in':
+      return Array.isArray(target) && target.includes(value);
+    case '$nin':
+      return Array.isArray(target) && !target.includes(value);
+    case '$between':
+      return Array.isArray(target) && target.length === 2
+        && value >= target[0] && value <= target[1];
+    // -- Text operators. Same ruling the AST arms carry (objectstack#4706 Q2):
+    // the `$contains` family is case-SENSITIVE and `$icontains` is its one
+    // case-insensitive member, folding ASCII only on both sides.
+    case '$contains':
+      return typeof value === 'string' && value.includes(String(target));
+    case '$icontains':
+      return typeof value === 'string' && asciiCaseInsensitiveContains(value, String(target));
+    case '$notContains':
+      return typeof value === 'string' && !value.includes(String(target));
+    case '$startsWith':
+      return typeof value === 'string' && value.startsWith(String(target));
+    case '$endsWith':
+      return typeof value === 'string' && value.endsWith(String(target));
+    // -- Null-ness. Unlike the AST spelling, direction comes from the VALUE:
+    // `$null: true` is IS NULL and `$null: false` is IS NOT NULL, which is the
+    // lowering `convertFiltersToAST` already performs.
+    case '$null':
+      return target
+        ? value === null || value === undefined
+        : value !== null && value !== undefined;
+
+    // `$exists` is the exact INVERSE of `$null`, and that is the platform's own
+    // reading rather than one invented here: `convertFiltersToAST`
+    // (`../utils/filter-converter.ts`) lowers `$exists: true` to `is_not_null`
+    // and `$exists: false` to `is_null`, three lines below where it lowers
+    // `$null` the other way round. Both operators are absent from that file's
+    // `convertOperatorToAST` map and both are special-cased BEFORE it, so that
+    // map's silence is not a decision about either of them. Two producers in
+    // this repo emit `$exists` — `FilterConditionField`'s `exists` /
+    // `notExists` (kept reachable on purpose by objectui#4736) and
+    // `datasetFilterCondition`'s `isEmpty` / `isNotEmpty` — so refusing it
+    // would have turned "every row" into "no rows" on a filter that looks like
+    // it works, which is the one outcome worse than the bug.
+    case '$exists':
+      return target
+        ? value !== null && value !== undefined
+        : value === null || value === undefined;
+
+    default: {
+      const retired = RETIRED_FILTER_OPERATORS[operator];
+      if (retired) {
+        return refuseFilterNode(
+          refusals,
+          `filter operator '${operator}' on field '${field}' is retired. ${retired.why}`,
+        );
+      }
+      if (!operator.startsWith('$')) {
+        return refuseFilterNode(
+          refusals,
+          `filter condition on field '${field}' carries the non-operator key `
+          + `'${operator}'; the in-memory matcher does not descend into a nested `
+          + `relation constraint`,
+        );
+      }
+      return refuseFilterNode(
+        refusals,
+        `filter operator '${operator}' on field '${field}' is not implemented by the `
+        + `in-memory matcher`,
+      );
+    }
+  }
+}
+
+/**
+ * In-memory evaluation of an OBJECT-shaped (`$`-dialect) filter.
+ *
+ * Reads a flat key/value equality (`{ age: 26 }`) or a `FieldOperatorsSchema`
+ * condition object (`{ age: { $gte: 25 } }`), one entry per field, ANDed. What
+ * it cannot execute it refuses through {@link refuseFilterNode} — excluded and
+ * logged once per distinct refusal per `find()` — rather than adding no
+ * constraint, which is what its `default: break` used to do (objectui#8447).
+ *
+ * ## Combinators are refused here, not implemented (objectui#8447, its own case)
+ *
+ * `$and` / `$or` / `$not` are `LOGICAL_OPERATORS`, not field names, and this
+ * matcher has no grouping. They were already excluded-and-silent in two of
+ * three cases before this card and fail-OPEN in the third, which is why they
+ * get an arm now rather than being swept into the operator fix:
+ *
+ * - `{ $and: [...] }` / `{ $or: [...] }` carry an ARRAY, so they fell to the
+ *   simple-equality branch below (`record['$and'] !== [...]` is always true) and
+ *   excluded every row with no diagnostic. The rows do not move; the silence does.
+ * - `{ $not: {...} }` carries an OBJECT (`FilterConditionSchema`, not an array),
+ *   so it entered the operator branch, its inner FIELD names were read as
+ *   operator names, and every one of them hit `default: break`. It therefore
+ *   matched EVERY row — the same fail-open direction as the operators, and the
+ *   one behaviour here whose result changes.
+ *
+ * Executing them is a feature with its own semantics to settle (the empty-group
+ * identities and `$not`'s NULL-safe rule, objectstack#5146 / #5322), not part of
+ * this repair. An author who needs a group today writes the AST array `$filter`,
+ * which the sibling arm of `find()` already executes.
  */
-function matchesFilter(record: any, filter: Record<string, any>): boolean {
+function matchesFilter(
+  record: any,
+  filter: Record<string, any>,
+  refusals: Set<string>,
+): boolean {
   for (const [key, condition] of Object.entries(filter)) {
+    if (key.startsWith('$')) {
+      return refuseFilterNode(
+        refusals,
+        `filter combinator '${key}' is not implemented by the object-dialect matcher; `
+        + `express the group as an AST array $filter, which this adapter executes`,
+      );
+    }
+
     const value = record[key];
 
     if (condition && typeof condition === 'object' && !Array.isArray(condition)) {
-      // Operator-based filter
+      // Operator-based filter — every operator on the field is ANDed.
       for (const [op, target] of Object.entries(condition)) {
-        switch (op) {
-          case '$gt':
-            if (!(value > (target as any))) return false;
-            break;
-          case '$gte':
-            if (!(value >= (target as any))) return false;
-            break;
-          case '$lt':
-            if (!(value < (target as any))) return false;
-            break;
-          case '$lte':
-            if (!(value <= (target as any))) return false;
-            break;
-          case '$ne':
-            if (value === target) return false;
-            break;
-          case '$in':
-            if (!Array.isArray(target) || !target.includes(value)) return false;
-            break;
-          // The `$` dialect of the same ruling the AST arms carry
-          // (objectui#7379): `$contains` is case-SENSITIVE, `$icontains` is the
-          // case-insensitive one, and the fold is ASCII-only on both sides.
-          // This arm lower-cased both sides, so the two spellings named one
-          // predicate here too — and `$icontains` had no arm at all, which in
-          // this switch means the `default` below and therefore NO constraint:
-          // a case-insensitive filter selected every row. Making `$contains`
-          // exact without adding its twin would have left the dialect with no
-          // working case-insensitive door.
-          case '$contains':
-            if (typeof value !== 'string' || !value.includes(String(target))) return false;
-            break;
-          case '$icontains':
-            if (typeof value !== 'string'
-              || !asciiCaseInsensitiveContains(value, String(target))) return false;
-            break;
-          default:
-            break;
-        }
+        if (!matchesDollarOperator(value, op, target, key, refusals)) return false;
       }
     } else {
       // Simple equality
@@ -399,15 +542,20 @@ export class ValueDataSource<T = any> implements DataSource<T> {
 
     // Filter — support both MongoDB-style objects and AST-format arrays
     if (params?.$filter) {
+      // ONE collector for BOTH arms, drained after the pass: a node either
+      // matcher refuses would otherwise log once PER ROW. Shared on purpose —
+      // the two arms differ only in the SHAPE of `$filter`, and objectui#8447
+      // was exactly the asymmetry of one arm refusing loudly while the other
+      // waved everything through in silence.
+      const refusals = new Set<string>();
       if (Array.isArray(params.$filter) && params.$filter.length > 0) {
-        // One collector per `find()`, drained after the pass: a node the matcher
-        // refuses would otherwise log once PER ROW.
-        const refusals = new Set<string>();
         result = result.filter((r) => matchesASTFilter(r, params.$filter as any[], refusals));
-        for (const message of refusals) console.warn(message);
       } else if (!Array.isArray(params.$filter) && Object.keys(params.$filter).length > 0) {
-        result = result.filter((r) => matchesFilter(r, params.$filter!));
+        result = result.filter(
+          (r) => matchesFilter(r, params.$filter as Record<string, any>, refusals),
+        );
       }
+      for (const message of refusals) console.warn(message);
     }
 
     // Search (simple text search across all string fields)