|
| 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 | + * A `Date` comparand through `convertFiltersToAST` — objectui#8555. |
| 11 | + * |
| 12 | + * ## The defect |
| 13 | + * |
| 14 | + * The operator-object arm opened on `typeof value === 'object' && |
| 15 | + * !Array.isArray(value)`. A `Date` passes both tests, so it entered the |
| 16 | + * operator loop — and `Object.entries(someDate)` is `[]`, so the loop body |
| 17 | + * never ran and NO condition was pushed for that field. |
| 18 | + * |
| 19 | + * Not refused, not lowered wrongly: ABSENT. `{ status: 'a', created: someDate }` |
| 20 | + * lowered to `['status', '=', 'a']`, which is a WIDER result set than the author |
| 21 | + * asked for, with nothing thrown and nothing logged — the one failure direction |
| 22 | + * this file exists to avoid. |
| 23 | + * |
| 24 | + * It also made the field's fate depend on its SIBLINGS. With the Date alone, |
| 25 | + * `conditions` ended empty and `convertFiltersToAST` returned the ORIGINAL |
| 26 | + * OBJECT untouched, so the shape survived by accident and the defect only |
| 27 | + * appeared once a second field was present. That asymmetry is part of the bug; |
| 28 | + * section 2 is what pins it closed. |
| 29 | + * |
| 30 | + * ## The ruling — LOWER it, and the spec is what decides that |
| 31 | + * |
| 32 | + * This is the OPPOSITE answer to objectui#8514, which was resolved as a refusal |
| 33 | + * precisely because the spec DECLINED to rule on that shape. Here it rules, |
| 34 | + * measured against `@objectstack/spec` 17.3.0 and pinned in section 3: |
| 35 | + * |
| 36 | + * - `ACCEPTED_FILTER_COMPARAND_TYPES` is |
| 37 | + * `['string','number','bigint','boolean','null','Date']` — `Date` is a |
| 38 | + * first-class literal comparand, and `isAcceptedFilterComparand(new Date())` |
| 39 | + * is `true`. |
| 40 | + * - `parseFilterAST(['created', '=', d])` hands back `{ created: d }` with the |
| 41 | + * Date INSTANCE intact. |
| 42 | + * |
| 43 | + * So the wire-form question the card raised ("ISO string? epoch?") is answered |
| 44 | + * by NOT answering it here: the AST leaf carries the `Date`, exactly as the |
| 45 | + * operator arm has always emitted it for `{ created: { $gte: d } }`. |
| 46 | + * Stringifying in this arm would make the shorthand and the operator form emit |
| 47 | + * two different comparand types for one author intent. |
| 48 | + * |
| 49 | + * ## What carries the weight — three legs, RUN rather than predicted |
| 50 | + * |
| 51 | + * Each was applied to the committed implementation, proved on disk in both |
| 52 | + * directions, and restored by state. The counts and MODES below are measured, |
| 53 | + * not expected — one of them corrected the prediction written here first. |
| 54 | + * |
| 55 | + * 1. **Ablation** — the arm removed. 7 of 16 red. MODE: a MISSING condition. |
| 56 | + * `{ status, created }` produces `['status', '=', 'a']` and `node[2]` is |
| 57 | + * `undefined`. Sections 1 and 2 both carry it. |
| 58 | + * 2. **`value.toISOString()`** — the plausible wrong fix, since "what wire |
| 59 | + * form?" is the question the card asks. 6 of 16 red. MODE: the right node |
| 60 | + * with the WRONG COMPARAND TYPE — `'2026-01-01T00:00:00.000Z'` where a Date |
| 61 | + * was expected. |
| 62 | + * ⚠️ This paragraph first claimed only the `instanceof Date` and |
| 63 | + * operator-parity pins would discriminate. Measured, six do: every |
| 64 | + * `toEqual([..., D])` in sections 1 and 2 reddens too, because a string is |
| 65 | + * not deep-equal to a Date. The correction that matters runs the other way — |
| 66 | + * §2's "lone and sibling forms lower the SAME field" pin does NOT redden |
| 67 | + * here, because the caricature is symmetric too. Symmetry alone never |
| 68 | + * discriminates a wrong comparand type; the `instanceof` / `toBe(D)` pins |
| 69 | + * are what do, and they are in their own `it()` blocks so no earlier |
| 70 | + * assertion can stop them running (objectui#8506, objectui#8514). |
| 71 | + * 3. **`Object.keys(value).length === 0` as the gate** — handles the Date and |
| 72 | + * reads EVERY entry-less object as an equality comparand. 2 of 16 red, both |
| 73 | + * in section 4, and NOTHING else: for a Date input this caricature is |
| 74 | + * byte-identical to the shipped fix, which is exactly why section 4 has to |
| 75 | + * exist. MODE: over-promotion — `{}` and `/x/` appear in comparand position |
| 76 | + * (`['created', '=', {}]`), and a RegExp comparand is one |
| 77 | + * `normalizeFilterComparandTypes` refuses outright, so the caricature moves |
| 78 | + * the failure downstream instead of removing it. |
| 79 | + */ |
| 80 | + |
| 81 | +import { describe, it, expect } from 'vitest'; |
| 82 | +import { |
| 83 | + isFilterAST, |
| 84 | + parseFilterAST, |
| 85 | + isAcceptedFilterComparand, |
| 86 | + ACCEPTED_FILTER_COMPARAND_TYPES, |
| 87 | +} from '@objectstack/spec/data'; |
| 88 | +import { |
| 89 | + convertFiltersToAST, |
| 90 | + toFilterNode, |
| 91 | + mergeFilterNodes, |
| 92 | +} from '../filter-converter'; |
| 93 | + |
| 94 | +const D = new Date('2026-01-01T00:00:00.000Z'); |
| 95 | + |
| 96 | +// --------------------------------------------------------------------------- |
| 97 | +// 1. The Date is lowered — as a Date, not as a string this layer invented |
| 98 | +// --------------------------------------------------------------------------- |
| 99 | + |
| 100 | +describe('objectui#8555 — a Date comparand lowers to an equality node', () => { |
| 101 | + it('lowers { status, created } to BOTH conditions — the sibling no longer eats the Date', () => { |
| 102 | + // The whole node, so the pre-fix failure MODE is legible in the diff: the |
| 103 | + // old emission was `['status', '=', 'a']` — one condition, not two, and no |
| 104 | + // group at all. |
| 105 | + const node = convertFiltersToAST({ status: 'a', created: D }); |
| 106 | + expect(node).toEqual(['and', ['status', '=', 'a'], ['created', '=', D]]); |
| 107 | + }); |
| 108 | + |
| 109 | + it('emits the Date INSTANCE, not an ISO string', () => { |
| 110 | + // Its own block on purpose: this is the single assertion that separates the |
| 111 | + // shipped fix from `value.toISOString()`, and an earlier failing assertion |
| 112 | + // in a shared block would stop it ever running (objectui#8506 / #8514). |
| 113 | + const node = convertFiltersToAST({ created: D }) as [string, string, unknown]; |
| 114 | + expect(node[2]).toBeInstanceOf(Date); |
| 115 | + expect(node[2]).toBe(D); |
| 116 | + expect(typeof node[2]).not.toBe('string'); |
| 117 | + }); |
| 118 | + |
| 119 | + it('emits the SAME comparand the operator arm has always emitted', () => { |
| 120 | + // Parity with `{ created: { $gte: d } }` is the reason the shorthand is not |
| 121 | + // stringified: one author intent, one comparand type. A stringifying fix |
| 122 | + // breaks this and nothing else. |
| 123 | + const shorthand = convertFiltersToAST({ created: D }) as [string, string, unknown]; |
| 124 | + const operatorForm = convertFiltersToAST({ created: { $gte: D } }) as [string, string, unknown]; |
| 125 | + expect(shorthand[2]).toBe(operatorForm[2]); |
| 126 | + }); |
| 127 | + |
| 128 | + it('produces a node the spec accepts, with the Date intact through its doors', () => { |
| 129 | + const node = convertFiltersToAST({ created: D }); |
| 130 | + expect(isFilterAST(node)).toBe(true); |
| 131 | + const parsed = parseFilterAST(node) as { created: unknown }; |
| 132 | + expect(parsed).toEqual({ created: D }); |
| 133 | + expect(parsed.created).toBeInstanceOf(Date); |
| 134 | + }); |
| 135 | + |
| 136 | + it('lowers a Date inside $and / $or and through both public sinks', () => { |
| 137 | + expect(convertFiltersToAST({ $or: [{ created: D }, { status: 'open' }] })) |
| 138 | + .toEqual(['or', ['created', '=', D], ['status', '=', 'open']]); |
| 139 | + expect(toFilterNode({ status: 'a', created: D })) |
| 140 | + .toEqual(['and', ['status', '=', 'a'], ['created', '=', D]]); |
| 141 | + // One AST-node source beside the object source; `mergeFilterNodes` keeps |
| 142 | + // each source as its own child of the `and`. |
| 143 | + expect(mergeFilterNodes({ created: D }, ['stage', '=', 'won'])) |
| 144 | + .toEqual(['and', ['created', '=', D], ['stage', '=', 'won']]); |
| 145 | + }); |
| 146 | +}); |
| 147 | + |
| 148 | +// --------------------------------------------------------------------------- |
| 149 | +// 2. The ASYMMETRY — a Date ALONE behaves the same as a Date with siblings |
| 150 | +// --------------------------------------------------------------------------- |
| 151 | + |
| 152 | +describe('objectui#8555 — a lone Date is lowered, not handed back unchanged', () => { |
| 153 | + it('no longer returns the original filter object when the Date is the only field', () => { |
| 154 | + // Pre-fix: `conditions` was empty, so the `if (conditions.length === 0)` |
| 155 | + // fallback returned the INPUT OBJECT — `isFilterAST` false, and the wire |
| 156 | + // answers 400. The defect hid here, because the shape came back "unharmed". |
| 157 | + const node = convertFiltersToAST({ created: D }); |
| 158 | + expect(node).toEqual(['created', '=', D]); |
| 159 | + expect(node).not.toBe(undefined); |
| 160 | + expect(Array.isArray(node)).toBe(true); |
| 161 | + expect(isFilterAST(node)).toBe(true); |
| 162 | + }); |
| 163 | + |
| 164 | + it('lone and sibling forms lower the SAME field to the SAME condition', () => { |
| 165 | + const alone = convertFiltersToAST({ created: D }); |
| 166 | + const withSibling = convertFiltersToAST({ status: 'a', created: D }) as unknown[]; |
| 167 | + // The `created` child of the group is byte-for-byte the lone emission. |
| 168 | + expect(withSibling[2]).toEqual(alone); |
| 169 | + }); |
| 170 | +}); |
| 171 | + |
| 172 | +// --------------------------------------------------------------------------- |
| 173 | +// 3. Why LOWER and not REFUSE — the spec's own ruling, pinned |
| 174 | +// --------------------------------------------------------------------------- |
| 175 | + |
| 176 | +describe('objectui#8555 — the spec rules Date IN, which is why this is not a refusal', () => { |
| 177 | + it('declares Date one of its six accepted literal comparand types', () => { |
| 178 | + expect(ACCEPTED_FILTER_COMPARAND_TYPES).toContain('Date'); |
| 179 | + expect(isAcceptedFilterComparand(D)).toBe(true); |
| 180 | + }); |
| 181 | + |
| 182 | + it('accepts a Date in the AST leaf and keeps it a Date', () => { |
| 183 | + // The wire-form question, answered by the spec rather than by this adapter. |
| 184 | + expect(isFilterAST(['created', '=', D])).toBe(true); |
| 185 | + expect((parseFilterAST(['created', '=', D]) as { created: unknown }).created) |
| 186 | + .toBeInstanceOf(Date); |
| 187 | + }); |
| 188 | + |
| 189 | + it('has Date as the ONLY object-typed member, so this arm is a Date arm', () => { |
| 190 | + // The gate is `isAcceptedFilterComparand`, not `instanceof Date`. This pin |
| 191 | + // is what makes that safe to read: every OTHER accepted comparand type is a |
| 192 | + // primitive and can never reach the `typeof value === 'object'` branch. If |
| 193 | + // the spec ever adds a second object-shaped literal, this reddens and the |
| 194 | + // reader learns the arm's reach just widened. |
| 195 | + const objectTyped = ACCEPTED_FILTER_COMPARAND_TYPES.filter( |
| 196 | + (t) => t !== 'string' && t !== 'number' && t !== 'bigint' && t !== 'boolean' && t !== 'null', |
| 197 | + ); |
| 198 | + expect(objectTyped).toEqual(['Date']); |
| 199 | + }); |
| 200 | +}); |
| 201 | + |
| 202 | +// --------------------------------------------------------------------------- |
| 203 | +// 4. NON-regression — the operator-object path `Object.entries` is there for. |
| 204 | +// This is the axis that separates the fix from "anything entry-less is a |
| 205 | +// comparand". Every case asserts the PRODUCED node. |
| 206 | +// --------------------------------------------------------------------------- |
| 207 | + |
| 208 | +describe('objectui#8555 — operator objects lower exactly as before', () => { |
| 209 | + it('$gt / $gte / $lt / $lte still reach the operator loop', () => { |
| 210 | + expect(convertFiltersToAST({ age: { $gt: 26 } })).toEqual(['age', '>', 26]); |
| 211 | + expect(convertFiltersToAST({ age: { $gte: 18, $lte: 65 } })) |
| 212 | + .toEqual(['and', ['age', '>=', 18], ['age', '<=', 65]]); |
| 213 | + }); |
| 214 | + |
| 215 | + it('an operator object carrying a Date value still lowers through the loop', () => { |
| 216 | + // The Date arm must not swallow `{ created: { $gte: d } }`: the VALUE of the |
| 217 | + // field is a plain object there, and only its member is a Date. |
| 218 | + expect(convertFiltersToAST({ created: { $gte: D } })).toEqual(['created', '>=', D]); |
| 219 | + expect(convertFiltersToAST({ created: { $between: [D, D] } })) |
| 220 | + .toEqual(['created', 'between', [D, D]]); |
| 221 | + }); |
| 222 | + |
| 223 | + it('$in / $nin / $null / $exists and the refusals are untouched', () => { |
| 224 | + expect(convertFiltersToAST({ status: { $in: ['a', 'b'] } })).toEqual(['status', 'in', ['a', 'b']]); |
| 225 | + expect(convertFiltersToAST({ deleted: { $null: true } })).toEqual(['deleted', 'is_null', true]); |
| 226 | + expect(convertFiltersToAST({ deleted: { $exists: false } })).toEqual(['deleted', 'is_null', true]); |
| 227 | + expect(() => convertFiltersToAST({ name: { $regex: 'a.c' } })).toThrow(/\$regex/); |
| 228 | + expect(() => convertFiltersToAST({ tags: ['a'] })).toThrow(/bare ARRAY/); |
| 229 | + }); |
| 230 | + |
| 231 | + it('an EMPTY operator object still constrains nothing — it is not a comparand', () => { |
| 232 | + // `{}` has no entries either, so a fix gated on `Object.keys(value).length |
| 233 | + // === 0` would lower it to `['created', '=', {}]`. It must stay the TRUE |
| 234 | + // identity it has always been: no operators means no constraint, the same |
| 235 | + // reading `{ $and: [] }` gets in `lowerLogicalGroup`. |
| 236 | + expect(convertFiltersToAST({ status: 'a', created: {} })).toEqual(['status', '=', 'a']); |
| 237 | + }); |
| 238 | + |
| 239 | + it('a non-Date exotic object is NOT promoted into comparand position', () => { |
| 240 | + // A `RegExp` is entry-less too, so the same `Object.keys` caricature would |
| 241 | + // emit `['created', '=', /x/]` — a comparand `normalizeFilterComparandTypes` |
| 242 | + // refuses outright (INVALID_FILTER / 400), i.e. the failure moves downstream |
| 243 | + // rather than going away. |
| 244 | + // |
| 245 | + // ⚠️ This pins the NEGATIVE only. What this arm does with a RegExp today is |
| 246 | + // the same silent drop objectui#8555 describes, left in place deliberately: |
| 247 | + // the spec rules Date IN and rules RegExp OUT, so the two shapes need |
| 248 | + // different answers and only one of them was in scope. Filed separately. |
| 249 | + expect(isAcceptedFilterComparand(/x/)).toBe(false); |
| 250 | + expect(convertFiltersToAST({ status: 'a', created: /x/ })) |
| 251 | + .not.toEqual(['and', ['status', '=', 'a'], ['created', '=', /x/]]); |
| 252 | + }); |
| 253 | + |
| 254 | + it('plain scalars, null and undefined are exactly what they were', () => { |
| 255 | + expect(convertFiltersToAST({ status: 'active' })).toEqual(['status', '=', 'active']); |
| 256 | + expect(convertFiltersToAST({ count: 0, ok: false })).toEqual([ |
| 257 | + 'and', |
| 258 | + ['count', '=', 0], |
| 259 | + ['ok', '=', false], |
| 260 | + ]); |
| 261 | + // A null/undefined value is skipped before any of this, and an all-skipped |
| 262 | + // filter still returns the original object. |
| 263 | + expect(convertFiltersToAST({ a: null, b: undefined })).toEqual({ a: null, b: undefined }); |
| 264 | + }); |
| 265 | +}); |
0 commit comments