|
| 1 | +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * The view vocabulary and the AST vocabulary must agree. (#3948) |
| 5 | + * |
| 6 | + * `VIEW_FILTER_OPERATORS` (`ui/view.zod.ts`) is what an author may declare on a |
| 7 | + * `ViewFilterRule`, and what `ViewFilterRuleSchema` validates against. |
| 8 | + * `VALID_AST_OPERATORS` (`data/filter.zod.ts`) gates `isFilterAST()`, which |
| 9 | + * decides whether a filter is parsed into a query at all. |
| 10 | + * |
| 11 | + * They disagreed on **8 of 19** members — `equals`, `not_equals`, |
| 12 | + * `greater_than`, `less_than`, `greater_than_or_equal`, `less_than_or_equal`, |
| 13 | + * `before`, `after`. An author could declare any of them, the schema accepted |
| 14 | + * them, `defineStack` accepted them, and then `isFilterAST()` refused the filter, |
| 15 | + * the protocol passed the array through unconverted, and the driver dropped it: |
| 16 | + * an unfiltered result set with no error anywhere. |
| 17 | + * |
| 18 | + * Six of the eight were reachable only in theory, because ObjectUI's adapter |
| 19 | + * alias table happened to translate them. The safety of the query path was |
| 20 | + * resting on a hand-written table in a different repository being complete, and |
| 21 | + * it wasn't — `before`/`after` had no entry, which is how this surfaced. |
| 22 | + * |
| 23 | + * `data/` cannot import `ui/` (that direction is already taken, so it would be |
| 24 | + * circular), which is why the AST map is not literally derived from the view |
| 25 | + * vocabulary. This test is the enforcement instead: it fails the moment the view |
| 26 | + * layer gains an operator the AST layer cannot lower. |
| 27 | + */ |
| 28 | + |
| 29 | +import { describe, it, expect } from 'vitest'; |
| 30 | +import { |
| 31 | + VALID_AST_OPERATORS, |
| 32 | + isFilterAST, |
| 33 | + parseFilterAST, |
| 34 | +} from './filter.zod'; |
| 35 | +import { |
| 36 | + VIEW_FILTER_OPERATORS, |
| 37 | + VIEW_FILTER_OPERATOR_ALIASES, |
| 38 | +} from '../ui/view.zod'; |
| 39 | + |
| 40 | +/** |
| 41 | + * View operators that resolve to a value-shape before an operator is ever |
| 42 | + * emitted, so they legitimately need no AST lowering. |
| 43 | + * |
| 44 | + * Empty today: `is_empty`/`is_not_empty` DO have lowerings (to `$null`), because |
| 45 | + * clients send them as operators rather than resolving them client-side. Kept as |
| 46 | + * an explicit, empty exemption list so that adding to it is a visible decision |
| 47 | + * rather than a quiet edit to the assertion. |
| 48 | + */ |
| 49 | +const NO_AST_LOWERING_REQUIRED = new Set<string>([]); |
| 50 | + |
| 51 | +describe('every view filter operator has an AST lowering', () => { |
| 52 | + it('reads both vocabularies', () => { |
| 53 | + // Guards the assertions below from passing vacuously. |
| 54 | + expect(VIEW_FILTER_OPERATORS.length).toBeGreaterThan(0); |
| 55 | + expect(VALID_AST_OPERATORS.size).toBeGreaterThan(0); |
| 56 | + }); |
| 57 | + |
| 58 | + it('VALID_AST_OPERATORS covers every canonical view operator', () => { |
| 59 | + const missing = VIEW_FILTER_OPERATORS |
| 60 | + .filter((op) => !NO_AST_LOWERING_REQUIRED.has(op)) |
| 61 | + .filter((op) => !VALID_AST_OPERATORS.has(op.toLowerCase())); |
| 62 | + expect( |
| 63 | + missing, |
| 64 | + 'an author can declare these on a ViewFilterRule and the schema validates them, ' |
| 65 | + + 'but isFilterAST() refuses the filter — it is passed through unconverted and ' |
| 66 | + + 'the driver cannot apply it. Add a lowering to AST_OPERATOR_MAP.', |
| 67 | + ).toEqual([]); |
| 68 | + }); |
| 69 | + |
| 70 | + it('VALID_AST_OPERATORS covers every legacy alias spelling too', () => { |
| 71 | + // `saveMeta` persists the authored body verbatim, so the schema's own |
| 72 | + // `z.preprocess` normalization never reaches the stored row — every alias in |
| 73 | + // this table is live in metadata, not merely historical. |
| 74 | + const missing = Object.keys(VIEW_FILTER_OPERATOR_ALIASES) |
| 75 | + .filter((alias) => !NO_AST_LOWERING_REQUIRED.has(VIEW_FILTER_OPERATOR_ALIASES[alias])) |
| 76 | + .filter((alias) => !VALID_AST_OPERATORS.has(alias.toLowerCase())); |
| 77 | + expect( |
| 78 | + missing, |
| 79 | + 'these spellings exist in stored view metadata and have no AST lowering', |
| 80 | + ).toEqual([]); |
| 81 | + }); |
| 82 | + |
| 83 | + it.each([...VIEW_FILTER_OPERATORS])('%s survives isFilterAST as a bare triple', (op) => { |
| 84 | + // The bare triple is the shape that used to vanish: a single AND condition |
| 85 | + // is emitted as `[field, op, value]`, and when isFilterAST() rejects it the |
| 86 | + // whole filter is silently dropped. |
| 87 | + if (NO_AST_LOWERING_REQUIRED.has(op)) return; |
| 88 | + const value = op === 'in' || op === 'not_in' ? ['a'] : op === 'between' ? [1, 2] : 'x'; |
| 89 | + expect(isFilterAST(['some_field', op, value]), `isFilterAST rejects "${op}"`).toBe(true); |
| 90 | + }); |
| 91 | + |
| 92 | + it('lowers each view operator to a real $-operator, never the $${op} fallback', () => { |
| 93 | + // `convertComparison` ends in `{ [field]: { [`$${op}`]: value } }` for an |
| 94 | + // unmapped operator, which produces e.g. `$before` — a key no driver knows, |
| 95 | + // so the failure moves from "silently unfiltered" to "driver throws". Both |
| 96 | + // are wrong; this asserts we produce a real operator. |
| 97 | + const KNOWN = new Set([ |
| 98 | + '$eq', '$ne', '$gt', '$gte', '$lt', '$lte', '$in', '$nin', |
| 99 | + '$between', '$contains', '$notContains', '$startsWith', '$endsWith', |
| 100 | + '$null', '$exists', |
| 101 | + ]); |
| 102 | + const bad: string[] = []; |
| 103 | + for (const op of VIEW_FILTER_OPERATORS) { |
| 104 | + if (NO_AST_LOWERING_REQUIRED.has(op)) continue; |
| 105 | + const value = op === 'in' || op === 'not_in' ? ['a'] : op === 'between' ? [1, 2] : 'x'; |
| 106 | + const parsed = parseFilterAST(['some_field', op, value]) as Record<string, unknown>; |
| 107 | + const arm = parsed.some_field; |
| 108 | + // The equality shorthand is `{ field: value }` — a bare value, not an |
| 109 | + // operator object. That is a real lowering, not a fallback. |
| 110 | + if (arm === null || typeof arm !== 'object') continue; |
| 111 | + const keys = Object.keys(arm as Record<string, unknown>); |
| 112 | + if (!keys.every((k) => KNOWN.has(k))) bad.push(`${op} → ${keys.join(',')}`); |
| 113 | + } |
| 114 | + expect(bad, 'these fell through to the $${op} fallback').toEqual([]); |
| 115 | + }); |
| 116 | + |
| 117 | + it('the date comparisons that regressed now lower correctly', () => { |
| 118 | + expect(parseFilterAST(['close_date', 'before', '2024-01-01'])) |
| 119 | + .toEqual({ close_date: { $lt: '2024-01-01' } }); |
| 120 | + expect(parseFilterAST(['close_date', 'after', '2024-01-01'])) |
| 121 | + .toEqual({ close_date: { $gt: '2024-01-01' } }); |
| 122 | + }); |
| 123 | + |
| 124 | + it('spells equality one way regardless of which alias the author used', () => { |
| 125 | + const shorthand = { status: 'active' }; |
| 126 | + for (const op of ['=', '==', 'equals', 'eq']) { |
| 127 | + expect(parseFilterAST(['status', op, 'active']), `via "${op}"`).toEqual(shorthand); |
| 128 | + } |
| 129 | + }); |
| 130 | + |
| 131 | + it('keeps null-direction keyed on the operator name, not the filler value', () => { |
| 132 | + // Clients send a truthy placeholder for both directions. |
| 133 | + expect(parseFilterAST(['note', 'is_empty', true])).toEqual({ note: { $null: true } }); |
| 134 | + expect(parseFilterAST(['note', 'isempty', true])).toEqual({ note: { $null: true } }); |
| 135 | + expect(parseFilterAST(['note', 'is_not_empty', true])).toEqual({ note: { $null: false } }); |
| 136 | + expect(parseFilterAST(['note', 'isnotempty', true])).toEqual({ note: { $null: false } }); |
| 137 | + }); |
| 138 | + |
| 139 | + it('still refuses an operator in neither vocabulary', () => { |
| 140 | + // Widening must not turn the gate off. |
| 141 | + expect(isFilterAST(['some_field', 'sounds_like', 'x'])).toBe(false); |
| 142 | + }); |
| 143 | +}); |
0 commit comments