|
| 1 | +/** |
| 2 | + * ObjectUI |
| 3 | + * Copyright (c) 2024-present ObjectStack Inc. |
| 4 | + * |
| 5 | + * This source code is licensed under the MIT license found in the |
| 6 | + * LICENSE file in the root directory of this source tree. |
| 7 | + */ |
| 8 | + |
| 9 | +/** |
| 10 | + * The TRUE-identity combinators reach the wire as "no constraint" — |
| 11 | + * objectui#8770. |
| 12 | + * |
| 13 | + * ## What was wrong |
| 14 | + * |
| 15 | + * `lowerLogicalGroup` answers `undefined` for a group that reduces to the TRUE |
| 16 | + * identity, and that is correct and deliberate (see its docblock: a childless |
| 17 | + * `['and']` is `isFilterAST` FALSE, which reads as no filter at all). But when |
| 18 | + * such a group was the ONLY thing in the filter, that `undefined` fell through |
| 19 | + * to `convertFiltersToAST`'s general tail — `if (conditions.length === 0) |
| 20 | + * return filter` — and the CALLER'S ORIGINAL OBJECT came back. So the group did |
| 21 | + * not disappear at that level after all; it reappeared one level up, in the `$` |
| 22 | + * dialect, in a slot the AST is expected to occupy. |
| 23 | + * |
| 24 | + * Three of the four identities went out that way and one did not, which is what |
| 25 | + * made this an internal inconsistency rather than an open question: the same |
| 26 | + * function already lowered `{ $or: [] }` correctly. |
| 27 | + * |
| 28 | + * ## Why the assertions are shaped the way they are |
| 29 | + * |
| 30 | + * Two halves, because either alone passes on something worse than the bug: |
| 31 | + * |
| 32 | + * - a SHAPE assertion (`toBeUndefined`) alone passes on a converter that has |
| 33 | + * stopped emitting anything at all; |
| 34 | + * - a ROW-SET assertion alone passes on a filter that never ran — `$filter: |
| 35 | + * undefined` returns EVERY row, and so does a correct TRUE identity, so the |
| 36 | + * row set cannot tell "honoured" from "dropped". |
| 37 | + * |
| 38 | + * So the shape half is asserted against the spec's own door (`isFilterAST`) |
| 39 | + * rather than against a literal, the row-set half names exact ids, and the |
| 40 | + * FALSE identity `{ $or: [] }` is carried through every section as a control: |
| 41 | + * it must keep answering NO rows. A change that flattened the identities into |
| 42 | + * one arm would take the control with it. |
| 43 | + * |
| 44 | + * `FILTER_LOGIC_ROWS` (`@objectstack/spec/data`) is the published cross-backend |
| 45 | + * fixture the identity ruling (objectstack#5322, merged objectstack#5365) is |
| 46 | + * held to, so the row sets here are the platform's own and not a local |
| 47 | + * restatement of them. |
| 48 | + */ |
| 49 | + |
| 50 | +import { describe, it, expect } from 'vitest'; |
| 51 | +import { FILTER_LOGIC_ROWS, isFilterAST, parseFilterAST } from '@objectstack/spec/data'; |
| 52 | +import { convertFiltersToAST, toFilterNode, mergeFilterNodes } from '../filter-converter'; |
| 53 | +import { ValueDataSource } from '../../adapters/ValueDataSource'; |
| 54 | + |
| 55 | +const ALL_IDS = FILTER_LOGIC_ROWS.map((r) => String(r.id)); |
| 56 | + |
| 57 | +async function selectedIds(filter: unknown): Promise<string[]> { |
| 58 | + const ds = new ValueDataSource({ items: FILTER_LOGIC_ROWS as any[] }); |
| 59 | + const result = await ds.find('rows', { $filter: filter as any }); |
| 60 | + return result.data.map((r) => String(r.id)); |
| 61 | +} |
| 62 | + |
| 63 | +/** The three groups objectstack#5322 rules TRUE — "every row". */ |
| 64 | +const TRUE_IDENTITIES: ReadonlyArray<[string, Record<string, any>]> = [ |
| 65 | + ['{ $and: [] }', { $and: [] }], |
| 66 | + ['{ $or: [{}] }', { $or: [{}] }], |
| 67 | + ['{ $and: [{}] }', { $and: [{}] }], |
| 68 | +]; |
| 69 | + |
| 70 | +// --------------------------------------------------------------------------- |
| 71 | +// 0. The harness has to be able to fail |
| 72 | +// --------------------------------------------------------------------------- |
| 73 | + |
| 74 | +describe('objectui#8770 — harness', () => { |
| 75 | + it('every row and no rows are distinguishable answers here', async () => { |
| 76 | + expect(ALL_IDS).toEqual(['1', '2', '3', '4']); |
| 77 | + // A filter that really constrains lands strictly between the two, so |
| 78 | + // "every row" below is a measurement and not the fixture's only answer. |
| 79 | + expect(await selectedIds(['a', '=', 'x'])).toEqual(['1', '2']); |
| 80 | + expect(await selectedIds(undefined)).toEqual(ALL_IDS); |
| 81 | + }); |
| 82 | +}); |
| 83 | + |
| 84 | +// --------------------------------------------------------------------------- |
| 85 | +// 1. The card's own table, re-measured |
| 86 | +// --------------------------------------------------------------------------- |
| 87 | + |
| 88 | +describe('objectui#8770 — the four identity groups', () => { |
| 89 | + it.each(TRUE_IDENTITIES)('%s lowers to "no constraint"', (_label, filter) => { |
| 90 | + // Not `['and']` and not the caller's object: the absence of the slot is how |
| 91 | + // this dialect says TRUE, which is exactly what `lowerLogicalGroup` decided |
| 92 | + // one level down and what the tail used to throw away. |
| 93 | + expect(convertFiltersToAST(filter)).toBeUndefined(); |
| 94 | + }); |
| 95 | + |
| 96 | + it('CONTROL — `{ $or: [] }` is FALSE and keeps the leaf it always had', () => { |
| 97 | + // The one identity that was already right. It must not be swept into the |
| 98 | + // fold above: FALSE is not "no constraint", and the AST has no |
| 99 | + // contradiction literal, so the pre-existing leaf is still the emission. |
| 100 | + const node = convertFiltersToAST({ $or: [] }); |
| 101 | + expect(node).toEqual(['$or', '=', []]); |
| 102 | + expect(parseFilterAST(node)).toEqual({ $or: [] }); |
| 103 | + }); |
| 104 | +}); |
| 105 | + |
| 106 | +// --------------------------------------------------------------------------- |
| 107 | +// 2. The invariant that actually broke — what reaches the wire |
| 108 | +// --------------------------------------------------------------------------- |
| 109 | + |
| 110 | +describe('objectui#8770 — nothing unreadable is handed back', () => { |
| 111 | + it.each([...TRUE_IDENTITIES, ['{ $or: [] }', { $or: [] }] as const])( |
| 112 | + '%s is either absent or a node the spec can read', |
| 113 | + (_label, filter) => { |
| 114 | + // The property, stated once for all four: a value this function returns |
| 115 | + // is either `undefined` — no filter — or something `isFilterAST` |
| 116 | + // accepts. Before the fix three of the four returned a plain OBJECT |
| 117 | + // instead, which is neither: `client.data.find()` spreads such an object's |
| 118 | + // entries as query parameters (`?$and=`), so no `filter` parameter was |
| 119 | + // sent and the server refused the unknown `$`-prefixed one with `400 |
| 120 | + // UNSUPPORTED_QUERY_PARAM`. |
| 121 | + const out = convertFiltersToAST(filter as Record<string, any>); |
| 122 | + expect(out === undefined || isFilterAST(out)).toBe(true); |
| 123 | + }, |
| 124 | + ); |
| 125 | +}); |
| 126 | + |
| 127 | +// --------------------------------------------------------------------------- |
| 128 | +// 3. The ruled row sets, through a real matcher |
| 129 | +// --------------------------------------------------------------------------- |
| 130 | + |
| 131 | +/** |
| 132 | + * ⚠️ This section does NOT witness this card's fix, and is kept anyway — say so |
| 133 | + * rather than let the next reader assume it does. Both legs of the ablation |
| 134 | + * (fold removed / fold restored) leave every case here GREEN, because |
| 135 | + * objectui#8513 already taught `ValueDataSource`'s matcher to answer the object |
| 136 | + * dialect correctly: the unlowered `{ $and: [] }` and the absent filter select |
| 137 | + * the same four rows. What it pins is the direction the triage ruling names — |
| 138 | + * the producer aligning to the consumer — so a fix that moved the two apart |
| 139 | + * again, or that started answering these identities FALSE, reddens here. |
| 140 | + */ |
| 141 | +describe('objectui#8770 — the ruling reaches the consumer', () => { |
| 142 | + it.each(TRUE_IDENTITIES)('%s selects EVERY row after lowering', async (_label, filter) => { |
| 143 | + expect(await selectedIds(toFilterNode(filter))).toEqual(ALL_IDS); |
| 144 | + }); |
| 145 | + |
| 146 | + it('CONTROL — `{ $or: [] }` still selects NO row after lowering', async () => { |
| 147 | + expect(await selectedIds(toFilterNode({ $or: [] }))).toEqual([]); |
| 148 | + }); |
| 149 | +}); |
| 150 | + |
| 151 | +// --------------------------------------------------------------------------- |
| 152 | +// 4. Composition — the shape the two real sinks build |
| 153 | +// --------------------------------------------------------------------------- |
| 154 | + |
| 155 | +describe('objectui#8770 — composing with a filter that does constrain', () => { |
| 156 | + it.each(TRUE_IDENTITIES)('%s contributes nothing to the merged `and`', async (_label, filter) => { |
| 157 | + // `plugin-list`'s `buildEffectiveFilter` and `plugin-view`'s `ObjectView` |
| 158 | + // both merge several sources here. Before the fix the object landed in AST |
| 159 | + // CHILD position — `['and', { $and: [] }, ['a','=','x']]` — which |
| 160 | + // `isFilterAST` refuses, so the whole list 400'd on a filter that was |
| 161 | + // supposed to narrow nothing. |
| 162 | + const merged = mergeFilterNodes(filter, ['a', '=', 'x']); |
| 163 | + expect(merged).toEqual(['a', '=', 'x']); |
| 164 | + expect(isFilterAST(merged)).toBe(true); |
| 165 | + expect(await selectedIds(merged)).toEqual(['1', '2']); |
| 166 | + }); |
| 167 | + |
| 168 | + it('a TRUE identity merged with a FALSE one is still FALSE', async () => { |
| 169 | + const merged = mergeFilterNodes({ $and: [] }, { $or: [] }); |
| 170 | + expect(merged).toEqual(['$or', '=', []]); |
| 171 | + expect(await selectedIds(merged)).toEqual([]); |
| 172 | + }); |
| 173 | + |
| 174 | + it('and the two identities inside ONE filter reduce the same way', async () => { |
| 175 | + // TRUE ∧ FALSE = FALSE, and the `$and` key must not drag the `$or` leaf out |
| 176 | + // of the result on its way to being dropped. |
| 177 | + const node = convertFiltersToAST({ $and: [], $or: [] }); |
| 178 | + expect(node).toEqual(['$or', '=', []]); |
| 179 | + expect(await selectedIds(node)).toEqual([]); |
| 180 | + }); |
| 181 | +}); |
| 182 | + |
| 183 | +// --------------------------------------------------------------------------- |
| 184 | +// 5. Nesting — an identity group inside an identity group |
| 185 | +// --------------------------------------------------------------------------- |
| 186 | + |
| 187 | +describe('objectui#8770 — nested identities fold all the way up', () => { |
| 188 | + it('a TRUE-identity child drops out of an `$and` and absorbs an `$or`', () => { |
| 189 | + expect(convertFiltersToAST({ $and: [{ $and: [] }] })).toBeUndefined(); |
| 190 | + expect(convertFiltersToAST({ $or: [{ $and: [] }] })).toBeUndefined(); |
| 191 | + }); |
| 192 | + |
| 193 | + it('a nested identity beside a real conjunct leaves the conjunct alone', () => { |
| 194 | + expect(convertFiltersToAST({ $and: [{ $and: [] }, { a: 'x' }] })).toEqual([ |
| 195 | + 'and', |
| 196 | + ['a', '=', 'x'], |
| 197 | + ]); |
| 198 | + }); |
| 199 | +}); |
| 200 | + |
| 201 | +// --------------------------------------------------------------------------- |
| 202 | +// 6. The boundary this fix deliberately did NOT move |
| 203 | +// --------------------------------------------------------------------------- |
| 204 | + |
| 205 | +describe('objectui#8770 — the non-combinator tail is untouched', () => { |
| 206 | + // The same tail serves inputs that are not combinators at all. Those are a |
| 207 | + // different question and were left exactly as they were: a null-valued key is |
| 208 | + // this converter's own tolerance rather than a ruled identity, and the object |
| 209 | + // handed back travels the `$expand` / `$search` route as `filter={"a":null}`, |
| 210 | + // which the server reads as a REAL `a IS NULL` predicate. Folding them into |
| 211 | + // "no constraint" would return MORE rows on a path objectstack#5322 said |
| 212 | + // nothing about — the one direction this file exists to avoid. |
| 213 | + |
| 214 | + it('an all-null filter still returns the original object', () => { |
| 215 | + expect(convertFiltersToAST({ a: null, b: undefined })).toEqual({ a: null, b: undefined }); |
| 216 | + }); |
| 217 | + |
| 218 | + it('an empty operator map still returns the original object', () => { |
| 219 | + expect(convertFiltersToAST({ a: {} })).toEqual({ a: {} }); |
| 220 | + }); |
| 221 | + |
| 222 | + it('an identity group beside a NON-combinator key returns the original object', () => { |
| 223 | + // The fold is keyed on "every key was such a group", not on "any key was". |
| 224 | + // `a: null` is not one, so this filter is not the ruled family and keeps the |
| 225 | + // answer it has always had. |
| 226 | + expect(convertFiltersToAST({ $and: [], a: null })).toEqual({ $and: [], a: null }); |
| 227 | + }); |
| 228 | + |
| 229 | + it('an identity group beside a key that DOES lower is unchanged', async () => { |
| 230 | + // Already correct before this card — the group was dropped and the sibling |
| 231 | + // carried the filter. Pinned because the fold must not start swallowing the |
| 232 | + // sibling. |
| 233 | + const node = convertFiltersToAST({ $and: [], a: 'x' }); |
| 234 | + expect(node).toEqual(['a', '=', 'x']); |
| 235 | + expect(await selectedIds(node)).toEqual(['1', '2']); |
| 236 | + }); |
| 237 | +}); |
0 commit comments