|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// Two internal `engine.find` calls sorted by `direction`, a key nothing on that |
| 4 | +// path reads (#4674). |
| 5 | +// |
| 6 | +// The QueryAST sort shape is `SortNodeSchema` = `{ field, order }` |
| 7 | +// (`packages/spec/src/data/query.zod.ts`), and both real drivers normalize off |
| 8 | +// `.order` with no fallback — `sql-driver` maps `item.order === 'desc'`, |
| 9 | +// `mongodb-driver` the same. With `order` absent, `undefined === 'desc'` is |
| 10 | +// false and both land on ASCENDING. `direction` is `IReportService`'s |
| 11 | +// vocabulary; it is a genuinely different contract, which is how the wrong |
| 12 | +// spelling looked plausible. |
| 13 | +// |
| 14 | +// Because both queries carry a `limit`, the wrong direction did not merely |
| 15 | +// reorder a page — it changed WHICH ROWS CAME BACK. So these tests assert on |
| 16 | +// identity, not sequence: with a limit smaller than the fixture, sorting the |
| 17 | +// wrong way returns a disjoint set. An order-only assertion would have passed |
| 18 | +// against a fake that ignored `orderBy` entirely. |
| 19 | +// |
| 20 | +// Nothing caught this because both sites erased their types (`} as any)` and |
| 21 | +// `const opts: any`), the protocol's `INVALID_SORT` normalizer does not run on |
| 22 | +// calls the protocol makes to `this.engine.find` directly, and that normalizer |
| 23 | +// rejects bad VALUES rather than unknown KEYS — the schema is not `.strict()`, |
| 24 | +// so `direction` was dropped rather than flagged. |
| 25 | + |
| 26 | +import { describe, it, expect, vi } from 'vitest'; |
| 27 | +import { ObjectStackProtocolImplementation } from './protocol.js'; |
| 28 | + |
| 29 | +/** |
| 30 | + * A `find` that honours the QueryAST contract for sort and limit, and nothing |
| 31 | + * else. Filtering is deliberately not implemented: what is under test is which |
| 32 | + * rows survive `orderBy` + `limit`, and a double that also filtered would let a |
| 33 | + * sort bug hide behind a `where` that happened to select the right rows. |
| 34 | + * |
| 35 | + * It reads `order` — the shape the drivers read. A double that read `direction` |
| 36 | + * would agree with the bug instead of catching it, which is exactly what the |
| 37 | + * publish-rollback double did until this change. |
| 38 | + */ |
| 39 | +function makeFind(rowsByObject: Record<string, any[]>) { |
| 40 | + return vi.fn(async (object: string, opts: any = {}) => { |
| 41 | + const rows = [...(rowsByObject[object] ?? [])]; |
| 42 | + for (const { field, order } of [...(opts.orderBy ?? [])].reverse()) { |
| 43 | + rows.sort((a, b) => { |
| 44 | + const av = a[field], bv = b[field]; |
| 45 | + if (av === bv) return 0; |
| 46 | + return (av < bv ? -1 : 1) * (order === 'desc' ? -1 : 1); |
| 47 | + }); |
| 48 | + } |
| 49 | + return typeof opts.limit === 'number' ? rows.slice(0, opts.limit) : rows; |
| 50 | + }); |
| 51 | +} |
| 52 | + |
| 53 | +/** The options the protocol handed to `engine.find` on its first call. */ |
| 54 | +const optionsFrom = (find: any) => find.mock.calls[0][1]; |
| 55 | + |
| 56 | +const AUDIT_ROWS = ['2024-01-01', '2024-02-01', '2024-03-01', '2024-04-01', '2024-05-01'].map( |
| 57 | + (d, i) => ({ |
| 58 | + id: `a${i + 1}`, |
| 59 | + occurred_at: `${d}T00:00:00.000Z`, |
| 60 | + actor: 'someone', |
| 61 | + operation: 'save', |
| 62 | + outcome: 'allowed', |
| 63 | + code: 'OK', |
| 64 | + }), |
| 65 | +); |
| 66 | + |
| 67 | +describe('auditMetaItem sorts newest-first (#4674)', () => { |
| 68 | + function makeProtocol() { |
| 69 | + const find = makeFind({ sys_metadata_audit: AUDIT_ROWS }); |
| 70 | + const engine = { registry: { getObject: () => undefined }, find }; |
| 71 | + return { p: new ObjectStackProtocolImplementation(engine as any), find }; |
| 72 | + } |
| 73 | + |
| 74 | + it('returns the NEWEST `limit` events, not the oldest', async () => { |
| 75 | + const { p } = makeProtocol(); |
| 76 | + const { events } = await p.auditMetaItem({ type: 'objects', name: 'invoice', limit: 2 }); |
| 77 | + |
| 78 | + // The whole defect in one assertion: ascending returns a1/a2 here. |
| 79 | + expect(events.map(e => e.id)).toEqual(['a5', 'a4']); |
| 80 | + }); |
| 81 | + |
| 82 | + it('asks for `order`, never `direction`', async () => { |
| 83 | + const { p, find } = makeProtocol(); |
| 84 | + await p.auditMetaItem({ type: 'objects', name: 'invoice', limit: 2 }); |
| 85 | + |
| 86 | + const sort = optionsFrom(find).orderBy; |
| 87 | + expect(sort).toEqual([{ field: 'occurred_at', order: 'desc' }]); |
| 88 | + // Named explicitly: `direction` reads as a well-formed "sort by |
| 89 | + // occurred_at, direction unspecified" and passes every existing check. |
| 90 | + expect(sort[0]).not.toHaveProperty('direction'); |
| 91 | + }); |
| 92 | +}); |
| 93 | + |
| 94 | +const SEARCH_ROWS = ['2024-01-01', '2024-02-01', '2024-03-01', '2024-04-01'].map((d, i) => ({ |
| 95 | + id: `c${i + 1}`, |
| 96 | + name: `Acme ${i + 1}`, |
| 97 | + updated_at: `${d}T00:00:00.000Z`, |
| 98 | +})); |
| 99 | + |
| 100 | +const CONTACT = { |
| 101 | + name: 'contact', |
| 102 | + fields: { name: { name: 'name', type: 'text', searchable: true } }, |
| 103 | +}; |
| 104 | + |
| 105 | +describe('searchAll sorts newest-first (#4674)', () => { |
| 106 | + function makeProtocol() { |
| 107 | + const find = makeFind({ contact: SEARCH_ROWS }); |
| 108 | + const engine = { |
| 109 | + registry: { getObject: (n: string) => (n === 'contact' ? CONTACT : undefined), getAllObjects: () => [CONTACT] }, |
| 110 | + find, |
| 111 | + }; |
| 112 | + return { p: new ObjectStackProtocolImplementation(engine as any), find }; |
| 113 | + } |
| 114 | + |
| 115 | + it('returns the most recently updated matches, not the stalest', async () => { |
| 116 | + const { p } = makeProtocol(); |
| 117 | + const { hits } = await p.searchAll({ q: 'Acme', perObject: 2 }); |
| 118 | + |
| 119 | + // Ascending returned c1/c2 — the stalest rows, with the recently-edited |
| 120 | + // ones truncated away by `perObject`. |
| 121 | + expect(hits.map(h => h.id)).toEqual(['c4', 'c3']); |
| 122 | + }); |
| 123 | + |
| 124 | + it('asks for `order`, never `direction`', async () => { |
| 125 | + const { p, find } = makeProtocol(); |
| 126 | + await p.searchAll({ q: 'Acme', perObject: 2 }); |
| 127 | + |
| 128 | + const sort = optionsFrom(find).orderBy; |
| 129 | + expect(sort).toEqual([{ field: 'updated_at', order: 'desc' }]); |
| 130 | + expect(sort[0]).not.toHaveProperty('direction'); |
| 131 | + }); |
| 132 | +}); |
0 commit comments