From c889b18a7bb97356b32eafe8b4040e1370887078 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 23:00:38 +0000 Subject: [PATCH] fix(core): ValueDataSource's `contains` is case-sensitive and `icontains` is its ASCII-folding twin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-memory matcher lower-cased both sides of `contains`, `not_contains`, `starts_with`, `ends_with` — and `icontains`, whose arm was stacked onto the `contains` one. So `contains` executed `icontains`, the two spellings named one predicate, and a `provider: 'value'` list filtered with `contains` returned strictly more rows than the same filter run against a real driver. Nothing errored; the list was just longer. `$contains` is contractually case-SENSITIVE (objectstack#4706 Q2 = A) and `$icontains` folds ASCII ONLY (Q1 = A). All five backends plus objectql's `having` matcher import `FILTER_TEXT_CASES` and answer those rows; this adapter was the last face that did not. - The four AST arms compare exactly. `VALID_AST_OPERATORS` has `icontains` and no other `i`-prefixed spelling, so case-sensitive is the only reading the other three have — and the one `not_contains` needs so no row can fail an operator and its negation. - `icontains` takes the spec's `asciiCaseInsensitiveContains`: `toLowerCase()` is the full Unicode fold, a promise the SQLite-backed drivers cannot keep. - The `$` dialect follows: `$contains` compares exactly and `$icontains` gains an arm — it had none, and an unrecognised `$` operator in that switch adds no constraint, so it used to select every row. The pin asserts row SETS in pairs over one fixture: the case-differing row `contains` must exclude, and the `icontains` query that must still include it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .changeset/wise-poems-behave.md | 17 ++ packages/core/src/adapters/ValueDataSource.ts | 67 ++++-- .../__tests__/ValueDataSource.test.ts | 9 +- .../ValueDataSource.textOperatorCase.test.ts | 216 ++++++++++++++++++ 4 files changed, 289 insertions(+), 20 deletions(-) create mode 100644 .changeset/wise-poems-behave.md create mode 100644 packages/core/src/adapters/__tests__/ValueDataSource.textOperatorCase.test.ts diff --git a/.changeset/wise-poems-behave.md b/.changeset/wise-poems-behave.md new file mode 100644 index 0000000000..7e8bcfa723 --- /dev/null +++ b/.changeset/wise-poems-behave.md @@ -0,0 +1,17 @@ +--- +"@object-ui/core": minor +--- + +`ValueDataSource`'s text operators answer the case question the wire answers: `contains` is case-SENSITIVE, `icontains` is its ASCII-folding twin (objectui#7379) + +The in-memory matcher lower-cased BOTH sides of `contains`, `not_contains`, `starts_with`, `ends_with` and — since the `icontains` arm was stacked onto the `contains` one — `icontains` too. So `contains` executed `icontains`, the two spellings named a single predicate, and a `provider: 'value'` list filtered with `contains` returned strictly more rows than the same filter run against a real driver. Nothing errored; the list was just longer, and both answers looked plausible. + +`$contains` is contractually case-sensitive (objectstack#4706 Q2 = A) and `$icontains` is the case-insensitive member, folding **ASCII only** (Q1 = A, because three of the five backends are SQLite underneath and its `lower()` folds ASCII only). All five drivers — `driver-sql`, `driver-sqlite-wasm`, `driver-turso`, `driver-mongodb`, `driver-memory` — plus objectql's `having` matcher import `FILTER_TEXT_CASES` from `@objectstack/spec/data` and answer its case rows. This adapter was the last face that did not. + +What changed: + +- `contains`, `not_contains`, `starts_with`, `ends_with` compare exactly. There is no `i` twin for the last three — `VALID_AST_OPERATORS` has `icontains` and nothing else with an `i` prefix — so case-sensitive is the only reading available to them, and it is the one `not_contains` needs so that no row can fail an operator *and* its negation. +- `icontains` now uses the spec's own `asciiCaseInsensitiveContains`, so `CAFÉ` no longer matches `café`. `String.prototype.toLowerCase()`, which this arm used, is the full Unicode fold — a promise the SQL family cannot keep. +- The `$`-dialect matcher follows: `$contains` compares exactly, and `$icontains` gains an arm. It had none, and an unrecognised `$` operator in that switch adds no constraint at all, so a `$icontains` filter used to select every row. + +**Behaviour change.** Metadata that relied on the lenient matching gets fewer rows and no error. A filter that means "match regardless of case" should be authored as `icontains` (AST/view dialect) or `$icontains` (`$` dialect); both now execute, and both fold ASCII case on either side. diff --git a/packages/core/src/adapters/ValueDataSource.ts b/packages/core/src/adapters/ValueDataSource.ts index 4ebeef5bc4..2722d1ba8d 100644 --- a/packages/core/src/adapters/ValueDataSource.ts +++ b/packages/core/src/adapters/ValueDataSource.ts @@ -18,7 +18,7 @@ import type { AggregateParams, AggregateResult, } from '@object-ui/types'; -import { canonicalAstOperator } from '@objectstack/spec/data'; +import { asciiCaseInsensitiveContains, canonicalAstOperator } from '@objectstack/spec/data'; import { emulateBatchTransaction } from './batchTransaction.js'; // --------------------------------------------------------------------------- @@ -122,23 +122,41 @@ function matchesComparisonNode( return Array.isArray(target) && target.includes(value); case 'nin': return Array.isArray(target) && !target.includes(value); + // -- Text operators. THE DIRECTION, so the next reader does not take it for + // a typo and fold it back (objectui#7379): the `$contains` FAMILY is + // case-SENSITIVE and `icontains` is its one case-insensitive member. That + // is the platform's ruling (objectstack#4706 Q2 = A), not this file's + // preference, and it is what every backend executes — `driver-sql`, + // `driver-sqlite-wasm`, `driver-turso`, `driver-mongodb` and + // `driver-memory` all import `FILTER_TEXT_CASES` (`@objectstack/spec/data`) + // and answer its `$contains is case-SENSITIVE` rows. These four arms used + // to lower-case BOTH sides, so `contains` executed `icontains`, the two + // were one predicate, and a `provider: 'value'` list answered a filter with + // strictly more rows than the same filter run against the wire. + // + // There is no `i` twin for the other three: `VALID_AST_OPERATORS` has + // `icontains` and NOTHING else with an `i` prefix — no `istartswith`, no + // `iendswith`, no `not_icontains` (the `$` dialect has no `$notIcontains`, + // and the AST table mirrors the executed set rather than widening it). So + // case-sensitive is the ONLY reading available to them, and it is the one + // `$notContains` needs for complementarity: a folding `not_contains` beside + // a case-exact `contains` lets one row fail an operator AND its negation. case 'contains': - case 'icontains': { - const lv = typeof value === 'string' ? value.toLowerCase() : ''; - return typeof value === 'string' && lv.includes(String(target).toLowerCase()); - } - case 'not_contains': { - const lv = typeof value === 'string' ? value.toLowerCase() : ''; - return typeof value === 'string' && !lv.includes(String(target).toLowerCase()); - } - case 'starts_with': { - const lv = typeof value === 'string' ? value.toLowerCase() : ''; - return typeof value === 'string' && lv.startsWith(String(target).toLowerCase()); - } - case 'ends_with': { - const lv = typeof value === 'string' ? value.toLowerCase() : ''; - return typeof value === 'string' && lv.endsWith(String(target).toLowerCase()); - } + return typeof value === 'string' && value.includes(String(target)); + // The fold is ASCII-ONLY (objectstack#4706 Q1 = A) and it runs on BOTH + // sides, which is why this borrows the spec's own predicate instead of + // spelling one here. `String.prototype.toLowerCase()` — what this arm used + // to reach for — is the FULL Unicode fold, so it matched `CAFÉ` against + // `café`; three of the five backends are SQLite underneath, whose `lower()` + // folds ASCII only, so a Unicode promise here is one the wire cannot keep. + case 'icontains': + return typeof value === 'string' && asciiCaseInsensitiveContains(value, String(target)); + case 'not_contains': + return typeof value === 'string' && !value.includes(String(target)); + case 'starts_with': + return typeof value === 'string' && value.startsWith(String(target)); + case 'ends_with': + return typeof value === 'string' && value.endsWith(String(target)); case 'between': return Array.isArray(target) && target.length === 2 && value >= target[0] && value <= target[1]; @@ -247,8 +265,21 @@ function matchesFilter(record: any, filter: Record): boolean { 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.toLowerCase().includes(String(target).toLowerCase())) return false; + 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; diff --git a/packages/core/src/adapters/__tests__/ValueDataSource.test.ts b/packages/core/src/adapters/__tests__/ValueDataSource.test.ts index 4430019ea7..cbd0beb458 100644 --- a/packages/core/src/adapters/__tests__/ValueDataSource.test.ts +++ b/packages/core/src/adapters/__tests__/ValueDataSource.test.ts @@ -78,9 +78,14 @@ describe('ValueDataSource — find', () => { it('should filter with $contains operator', async () => { const ds = createDS(); const result = await ds.find('users', { - $filter: { name: { $contains: 'ali' } }, + // `Ali`, not `ali`: this case used the lower-case spelling and passed only + // because `$contains` folded both sides, which objectui#7379 ended. The + // row it means to select is still the same one, and the case rule itself + // is pinned in `ValueDataSource.textOperatorCase.test.ts` rather than + // riding along here. + $filter: { name: { $contains: 'Ali' } }, }); - expect(result.data).toHaveLength(1); // Alice only ('ali' is not in 'Charlie') + expect(result.data).toHaveLength(1); // Alice only ('Ali' is not in 'Charlie') }); it('should sort ascending by Record format', async () => { diff --git a/packages/core/src/adapters/__tests__/ValueDataSource.textOperatorCase.test.ts b/packages/core/src/adapters/__tests__/ValueDataSource.textOperatorCase.test.ts new file mode 100644 index 0000000000..d8cf20baa6 --- /dev/null +++ b/packages/core/src/adapters/__tests__/ValueDataSource.textOperatorCase.test.ts @@ -0,0 +1,216 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#7379 — `ValueDataSource`'s text operators answer the case question + * the wire answers. + * + * ## What was wrong + * + * The `contains` / `icontains` / `not_contains` / `starts_with` / `ends_with` + * arms lower-cased BOTH sides, so `contains` executed `icontains` and the two + * spellings named one predicate. A `provider: 'value'` list filtered with + * `contains` returned strictly more rows than the same filter run against a + * real driver — and both answers look plausible on screen, because nothing + * errors: the list is just longer. + * + * ## Why case-SENSITIVE is the answer, and not this file's opinion + * + * `$contains` is contractually case-sensitive (objectstack#4706 Q2 = A) and + * `$icontains` is its case-insensitive twin, folding ASCII ONLY (Q1 = A). All + * five backends execute that: `driver-sql`, `driver-sqlite-wasm`, + * `driver-turso`, `driver-mongodb` and `driver-memory` each import + * `FILTER_TEXT_CASES` from `@objectstack/spec/data` and answer its + * `$contains is case-SENSITIVE` rows, as does objectql's `having` matcher. So + * this adapter was the odd one out among the faces, not a face disagreeing + * with a document. + * + * ## How these cases are written, and why + * + * Every assertion is a row-SET equality, never an operator-name check: a + * matcher that returns every row constructs exactly the right operator string. + * And each case comes in a PAIR over the SAME fixture — the case-differing row + * that `contains` must EXCLUDE, and the `icontains` query that must still + * INCLUDE it. The second half is what rules out an implementation strictly + * worse than the bug: a matcher answering `[]` for everything satisfies every + * exclusion on its own. {@link `the fixture discriminates in both directions`} + * states that requirement as its own case. + * + * The fixture mirrors `FILTER_TEXT_ROWS`' first four rows rather than importing + * the table: enrolling this adapter in `FILTER_TEXT_CASES` means answering ALL + * of it, including the non-string-value rows this card does not touch, and that + * table's own rule is that a face's rows join it in the PR that closes the gap. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { ValueDataSource } from '../ValueDataSource'; + +/** + * Two ASCII rows differing only in case, and two non-ASCII rows differing only + * in the case of a letter outside ASCII. The first pair catches a fold that + * fails to happen (`icontains`) or happens when it must not (`contains`); the + * second catches a fold WIDER than ASCII, which is what `toLowerCase()` gives. + */ +const ROWS = [ + { id: 'upper', name: 'ACME Corp' }, + { id: 'lower', name: 'acme corp' }, + { id: 'accent-upper', name: 'CAFÉ' }, + { id: 'accent-lower', name: 'café' }, +]; + +const ALL_IDS = ['upper', 'lower', 'accent-upper', 'accent-lower']; + +async function selectedIds(filter: unknown): Promise { + const ds = new ValueDataSource({ items: ROWS }); + const result = await ds.find('rows', { $filter: filter as any }); + return result.data.map((r) => r.id as string); +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +// --------------------------------------------------------------------------- +// 0. The harness discriminates — in BOTH directions +// --------------------------------------------------------------------------- + +describe('objectui#7379 — the fixture discriminates in both directions', () => { + it('an empty filter returns every row, so an exclusion is a real narrowing', async () => { + const ids = await selectedIds([]); + expect(ids, 'no filter must select all four rows').toEqual(ALL_IDS); + }); + + it('a matcher answering [] for everything FAILS these cases, not just passes the exclusions', async () => { + // The guard against an implementation strictly worse than the bug. Every + // `expected` below is non-empty for at least one member of each pair, and + // this case says so out loud: `icontains` over the very comparand + // `contains` must reject has to come back with BOTH ASCII rows. + const ids = await selectedIds(['name', 'icontains', 'acme']); + expect(ids.length, 'the inclusion half must be non-empty or the pin is vacuous').toBeGreaterThan(0); + expect(ids).toEqual(['upper', 'lower']); + }); + + it('an operator that was already case-exact before this card still selects — the live control', async () => { + // `=` never folded. Green on the broken tree and the fixed one; its job is + // to prove the rows, the adapter and the id projection work, so a red below + // is about case semantics rather than the harness. + expect(await selectedIds(['name', '=', 'ACME Corp'])).toEqual(['upper']); + }); +}); + +// --------------------------------------------------------------------------- +// 1. `contains` EXCLUDES the case-differing row; `icontains` INCLUDES it +// --------------------------------------------------------------------------- + +describe('objectui#7379 — `contains` and `icontains` are two predicates, not two spellings', () => { + it('a lower-case comparand: `contains` misses the upper-case row, `icontains` takes both', async () => { + expect( + await selectedIds(['name', 'contains', 'acme']), + '`contains` is case-SENSITIVE — ACME Corp must NOT come back', + ).toEqual(['lower']); + expect( + await selectedIds(['name', 'icontains', 'acme']), + '`icontains` over the SAME fixture must still include the row `contains` dropped', + ).toEqual(['upper', 'lower']); + }); + + it('an upper-case comparand: the mirror, so neither direction is a lucky count', async () => { + expect(await selectedIds(['name', 'contains', 'ACME'])).toEqual(['upper']); + expect(await selectedIds(['name', 'icontains', 'ACME'])).toEqual(['upper', 'lower']); + }); + + it('the two operators cannot silently re-converge: they disagree on this fixture', async () => { + const exact = await selectedIds(['name', 'contains', 'acme']); + const folded = await selectedIds(['name', 'icontains', 'acme']); + expect(exact, 'a re-folded `contains` would equal `icontains` here').not.toEqual(folded); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The fold is ASCII-ONLY (objectstack#4706 Q1 = A) +// --------------------------------------------------------------------------- + +describe('objectui#7379 — `icontains` folds ASCII case only', () => { + it('É does not fold to é, in either direction', async () => { + // `toLowerCase()` — what this arm used to use — folds the whole Unicode + // range and answers both rows to both queries. Three of the five backends + // are SQLite underneath, whose `lower()` folds ASCII only, so a Unicode + // promise here is one the wire cannot keep. + expect(await selectedIds(['name', 'icontains', 'café'])).toEqual(['accent-lower']); + expect(await selectedIds(['name', 'icontains', 'CAFÉ'])).toEqual(['accent-upper']); + }); + + it('the ASCII letters in the same comparand still fold — the fold happens, it is just narrow', async () => { + // Both rows share `caf`/`CAF`, so an ASCII fold matches the row whose + // accented letter agrees and no other. A matcher that folded NOTHING would + // answer `[]` to the first of these. + expect(await selectedIds(['name', 'icontains', 'cAf'])).toEqual(['accent-upper', 'accent-lower']); + }); +}); + +// --------------------------------------------------------------------------- +// 3. The rest of the family — case-sensitive, with no `i` twin to fall back on +// --------------------------------------------------------------------------- + +describe('objectui#7379 — the whole `$contains` family is case-sensitive', () => { + it('`starts_with` is case-sensitive', async () => { + expect(await selectedIds(['name', 'starts_with', 'ACME'])).toEqual(['upper']); + expect(await selectedIds(['name', 'starts_with', 'acme'])).toEqual(['lower']); + }); + + it('`ends_with` is case-sensitive', async () => { + expect(await selectedIds(['name', 'ends_with', 'Corp'])).toEqual(['upper']); + expect(await selectedIds(['name', 'ends_with', 'corp'])).toEqual(['lower']); + }); + + it('`not_contains` is case-sensitive, and complements `contains` exactly', async () => { + // The reason `not_contains` could not be left folding: a case-exact + // `contains` beside a folding `not_contains` lets `ACME Corp` fail the + // operator AND its negation for the same comparand. + const positive = await selectedIds(['name', 'contains', 'acme']); + const negative = await selectedIds(['name', 'not_contains', 'acme']); + expect(negative).toEqual(['upper', 'accent-upper', 'accent-lower']); + expect( + [...positive, ...negative].sort(), + 'every row satisfies exactly one of the two — no row fails both', + ).toEqual([...ALL_IDS].sort()); + }); +}); + +// --------------------------------------------------------------------------- +// 4. The `$` dialect of the same adapter answers the same way +// --------------------------------------------------------------------------- + +describe('objectui#7379 — the `$` dialect agrees with the AST dialect', () => { + it('`$contains` excludes the case-differing row and `$icontains` includes it', async () => { + expect( + await selectedIds({ name: { $contains: 'acme' } }), + '`$contains` is the case-SENSITIVE spelling', + ).toEqual(['lower']); + expect( + await selectedIds({ name: { $icontains: 'acme' } }), + '`$icontains` had NO arm before this card, so it selected every row', + ).toEqual(['upper', 'lower']); + }); + + it('`$icontains` narrows rather than waving every row through', async () => { + // The specific pre-card answer this replaces: an unrecognised `$` operator + // reaches `default: break`, which adds no constraint at all. + const ids = await selectedIds({ name: { $icontains: 'acme' } }); + expect(ids, 'the accented rows contain no ASCII "acme"').not.toEqual(ALL_IDS); + }); + + it('both dialects answer the same rows for the same question', async () => { + expect(await selectedIds({ name: { $contains: 'ACME' } })).toEqual( + await selectedIds(['name', 'contains', 'ACME']), + ); + expect(await selectedIds({ name: { $icontains: 'ACME' } })).toEqual( + await selectedIds(['name', 'icontains', 'ACME']), + ); + }); +});