|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// #8738 — an undeclared field must be refused by the SCHEMA on the UPDATE path |
| 4 | +// too, before the `beforeUpdate` hooks run. The sibling of #8682's insert door |
| 5 | +// (`engine-undeclared-field-preflight.test.ts`), and deliberately the same |
| 6 | +// door: one condition, one implementation, two callers. |
| 7 | +// |
| 8 | +// ## The card filed this half as INFERRED — here is the measurement |
| 9 | +// |
| 10 | +// #8738 says in as many words that nobody had run an update-path reproduction, |
| 11 | +// and asks for one before anyone implements. Run on `origin/main` @ `e5eeb499c` |
| 12 | +// with a real `ObjectQL` and the recording driver below, one mistyped key: |
| 13 | +// |
| 14 | +// by-id driver.update received `zzz_nonexistent_field` → refused THERE |
| 15 | +// multi driver.updateMany received it likewise → refused THERE |
| 16 | +// hooks `beforeUpdate` ran FIRST on both branches |
| 17 | +// payload {name, zzz_nonexistent_field, description:'derived-for-bad'} |
| 18 | +// ← `description` is the HOOK's derived value, not the caller's: it |
| 19 | +// was computed for, and travelled with, a request the server had |
| 20 | +// already decided to refuse. |
| 21 | +// envelope the thrown error carried NO `code` and NO `status` — the driver's |
| 22 | +// raw string, which `mapDataError` translated at the REST boundary. |
| 23 | +// |
| 24 | +// Both inferred claims reproduce. The premise stands. |
| 25 | +// |
| 26 | +// ## What the pin is, and what it deliberately is NOT |
| 27 | +// |
| 28 | +// The insert half pinned an AUTONUMBER GAP, because an insert issues a sequence |
| 29 | +// value that a refused request consumed permanently. **Update has no such |
| 30 | +// observable** — no autonumber, nothing durable consumed — so the card is |
| 31 | +// milder by exactly that much, and the pin has to be the thing that IS at |
| 32 | +// stake: the HOOK RUN. `beforeUpdate` stamping a ledger, calling out, or |
| 33 | +// deriving a field for a request that is then refused is the whole defect here, |
| 34 | +// so `hookRuns` is asserted directly rather than inferred from a counter. |
| 35 | +// |
| 36 | +// A suite that only asserted "an undeclared key is refused" would be satisfied |
| 37 | +// by a door that refuses everything, so every case below has its positive |
| 38 | +// twin: declared keys still update on both branches, and each of the three |
| 39 | +// no-opinion cases is pinned as a CONTROL that must pass with the door removed |
| 40 | +// as well as with it in place. |
| 41 | + |
| 42 | +import { describe, it, expect } from 'vitest'; |
| 43 | +import { ObjectQL } from './engine.js'; |
| 44 | +import type { EngineUpdateOptions } from '@objectstack/spec/data'; |
| 45 | + |
| 46 | +/** |
| 47 | + * The predicate branch's options bag, TYPED rather than cast — the payload is |
| 48 | + * what these cases are about, and an `as any` here would erase the contract on |
| 49 | + * the argument that decides which branch of `update()` runs. |
| 50 | + */ |
| 51 | +const MULTI_OPTIONS: EngineUpdateOptions = { where: { name: 'stored' }, multi: true }; |
| 52 | + |
| 53 | +/** Records everything that reached the driver — presence is the point. */ |
| 54 | +function makeRecordingDriver(missingColumns: readonly string[] = []) { |
| 55 | + const writes: Array<{ fn: string; data: Record<string, unknown> }> = []; |
| 56 | + const stored: Record<string, unknown> = { id: 'row-1', name: 'stored', description: 'd' }; |
| 57 | + const driver: any = { |
| 58 | + name: 'recording', version: '0.0.0', supports: {}, |
| 59 | + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, |
| 60 | + async find() { return [{ ...stored }]; }, |
| 61 | + async findOne() { return { ...stored }; }, |
| 62 | + async create(_object: string, data: Record<string, unknown>) { |
| 63 | + writes.push({ fn: 'create', data: { ...data } }); |
| 64 | + return { id: 'rec_1', ...data }; |
| 65 | + }, |
| 66 | + async update(object: string, id: string, data: Record<string, unknown>) { |
| 67 | + writes.push({ fn: 'update', data: { ...data } }); |
| 68 | + const bad = missingColumns.find((c) => c in data); |
| 69 | + // The shape knex produces: the bound statement, then ` - `, then the |
| 70 | + // database's own diagnostic. |
| 71 | + if (bad) throw new Error(`update \`${object}\` set \`${bad}\` = 'v' where \`id\` = '${id}' - table ${object} has no column named ${bad}`); |
| 72 | + return { ...stored, ...data, id }; |
| 73 | + }, |
| 74 | + async updateMany(object: string, _ast: unknown, data: Record<string, unknown>) { |
| 75 | + writes.push({ fn: 'updateMany', data: { ...data } }); |
| 76 | + const bad = missingColumns.find((c) => c in data); |
| 77 | + if (bad) throw new Error(`update \`${object}\` set \`${bad}\` = 'v' - table ${object} has no column named ${bad}`); |
| 78 | + return 1; |
| 79 | + }, |
| 80 | + async delete() { return true; }, |
| 81 | + async deleteMany() { return 0; }, |
| 82 | + async count() { return 1; }, |
| 83 | + async bulkCreate(object: string, rows: Record<string, unknown>[]) { |
| 84 | + return Promise.all(rows.map((r) => driver.create(object, r))); |
| 85 | + }, |
| 86 | + async bulkUpdate() { return []; }, async bulkDelete() {}, |
| 87 | + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, |
| 88 | + async commit() {}, async rollback() {}, |
| 89 | + }; |
| 90 | + return { driver, writes }; |
| 91 | +} |
| 92 | + |
| 93 | +function silentLogger() { |
| 94 | + const logger: any = { |
| 95 | + trace() {}, debug() {}, info() {}, warn() {}, error() {}, fatal() {}, |
| 96 | + child() { return logger; }, |
| 97 | + }; |
| 98 | + return logger; |
| 99 | +} |
| 100 | + |
| 101 | +/** |
| 102 | + * The `beforeUpdate` hook DERIVES a value the caller never sent — the card's |
| 103 | + * `period_label = 'Q3 2026'` shape — so "the hook ran" is a fact about app |
| 104 | + * behaviour reaching the statement, not merely a counter ticking. |
| 105 | + */ |
| 106 | +async function makeEngine(options: { |
| 107 | + missingColumns?: readonly string[]; |
| 108 | + /** `'declared'` (default) · `'none'` (registry-less) */ |
| 109 | + registration?: 'declared' | 'none'; |
| 110 | + /** |
| 111 | + * Replace what `getObject('acct')` answers, AFTER a normal registration. |
| 112 | + * |
| 113 | + * Necessary rather than decorative, and it is the shape the real fixtures |
| 114 | + * have: `registerObject({ fields: {} })` does NOT leave the map empty — the |
| 115 | + * registry INJECTS `organization_id`, `created_at`, `created_by`, |
| 116 | + * `updated_at`, `updated_by`, `owner_id` and `owning_business_unit_id` on |
| 117 | + * top, measured here. So an object registered with an empty map is not an |
| 118 | + * object whose map the door SEES as empty, and a control built that way |
| 119 | + * would pass for a reason that has nothing to do with the rule it claims to |
| 120 | + * pin. The 15 `fields: {}` fixtures #8737 triaged carry a registry STUB, not |
| 121 | + * a registration — this reproduces that, and only for `acct`. |
| 122 | + */ |
| 123 | + stubFields?: Record<string, unknown>; |
| 124 | +} = {}) { |
| 125 | + const engine = new ObjectQL({ logger: silentLogger() }); |
| 126 | + const { driver, writes } = makeRecordingDriver(options.missingColumns ?? []); |
| 127 | + engine.registerDriver(driver, true); |
| 128 | + await engine.init(); |
| 129 | + const registration = options.registration ?? 'declared'; |
| 130 | + if (registration === 'declared') { |
| 131 | + engine.registry.registerObject({ |
| 132 | + name: 'acct', |
| 133 | + fields: { |
| 134 | + id: { name: 'id', type: 'text', primaryKey: true }, |
| 135 | + name: { name: 'name', type: 'text' }, |
| 136 | + description: { name: 'description', type: 'text' }, |
| 137 | + }, |
| 138 | + } as any, 'test'); |
| 139 | + } |
| 140 | + if (options.stubFields) { |
| 141 | + const inner = engine.registry.getObject.bind(engine.registry); |
| 142 | + (engine.registry as any).getObject = (name: string) => |
| 143 | + (name === 'acct' ? { name: 'acct', fields: options.stubFields } : inner(name)); |
| 144 | + } |
| 145 | + const hookRuns: string[] = []; |
| 146 | + engine.registerHook('beforeUpdate', (ctx: any) => { |
| 147 | + hookRuns.push(String(ctx.input.data?.name ?? '?')); |
| 148 | + ctx.input.data.description = `derived-for-${ctx.input.data?.name}`; |
| 149 | + }, { object: 'acct' }); |
| 150 | + return { engine, writes, hookRuns }; |
| 151 | +} |
| 152 | + |
| 153 | +async function refusalOf(run: () => Promise<unknown>): Promise<any> { |
| 154 | + try { |
| 155 | + await run(); |
| 156 | + } catch (e) { |
| 157 | + return e; |
| 158 | + } |
| 159 | + return null; |
| 160 | +} |
| 161 | + |
| 162 | +describe('#8738 — the declared-field door on update()', () => { |
| 163 | + describe('the ordering claim — the card`s actual subject', () => { |
| 164 | + it('by-id: the beforeUpdate hook does NOT run for a payload carrying an undeclared key', async () => { |
| 165 | + const { engine, hookRuns } = await makeEngine(); |
| 166 | + |
| 167 | + await refusalOf(() => engine.update('acct', { id: 'row-1', name: 'bad', zzz_nonexistent_field: 'x' } as any)); |
| 168 | + |
| 169 | + // `['bad']` on `origin/main`: the hook ran, and its derived `description` |
| 170 | + // reached the statement the driver then rejected. A hook is not a pure |
| 171 | + // function — it stamps ledgers and calls out — so running it for a |
| 172 | + // refused request is a side effect, not a wasted cycle. |
| 173 | + expect(hookRuns).toEqual([]); |
| 174 | + }); |
| 175 | + |
| 176 | + it('multi: the beforeUpdate hook does NOT run either — the predicate branch has the same hole', async () => { |
| 177 | + const { engine, hookRuns } = await makeEngine(); |
| 178 | + |
| 179 | + await refusalOf(() => engine.update( |
| 180 | + 'acct', |
| 181 | + { name: 'bad', zzz_nonexistent_field: 'x' } as any, |
| 182 | + MULTI_OPTIONS, |
| 183 | + )); |
| 184 | + |
| 185 | + expect(hookRuns).toEqual([]); |
| 186 | + }); |
| 187 | + |
| 188 | + it('the hook still runs — and is still the last word — when every key is declared', async () => { |
| 189 | + // The other direction of the ordering pin: the door refuses a payload, it |
| 190 | + // does not suppress the hook phase. Without this, "the hook did not run" |
| 191 | + // is satisfied by a door that refuses everything. |
| 192 | + const { engine, writes, hookRuns } = await makeEngine(); |
| 193 | + |
| 194 | + await engine.update('acct', { id: 'row-1', name: 'ok' } as any); |
| 195 | + |
| 196 | + expect(hookRuns).toEqual(['ok']); |
| 197 | + expect(writes).toHaveLength(1); |
| 198 | + expect(writes[0].data.description).toBe('derived-for-ok'); |
| 199 | + }); |
| 200 | + }); |
| 201 | + |
| 202 | + describe('the refusal', () => { |
| 203 | + it('by-id: nothing reaches the driver', async () => { |
| 204 | + const { engine, writes } = await makeEngine(); |
| 205 | + |
| 206 | + await refusalOf(() => engine.update('acct', { id: 'row-1', name: 'bad', zzz_nonexistent_field: 'x' } as any)); |
| 207 | + |
| 208 | + // Zero, not one: the pre-update read is skipped too. A refused write |
| 209 | + // should not cost a driver round-trip, and `previous` / the not-found |
| 210 | + // gate / the `readonlyWhen` gate — the read's three consumers — are all |
| 211 | + // downstream of a payload this door never lets through. |
| 212 | + expect(writes).toHaveLength(0); |
| 213 | + }); |
| 214 | + |
| 215 | + it('multi: nothing reaches the driver', async () => { |
| 216 | + const { engine, writes } = await makeEngine(); |
| 217 | + |
| 218 | + await refusalOf(() => engine.update( |
| 219 | + 'acct', |
| 220 | + { name: 'bad', zzz_nonexistent_field: 'x' } as any, |
| 221 | + MULTI_OPTIONS, |
| 222 | + )); |
| 223 | + |
| 224 | + expect(writes).toHaveLength(0); |
| 225 | + }); |
| 226 | + |
| 227 | + it('refuses in the ADR-0112 envelope, with the wire answer unchanged', async () => { |
| 228 | + const { engine } = await makeEngine(); |
| 229 | + |
| 230 | + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', name: 'bad', zzz_nonexistent_field: 'x' } as any)); |
| 231 | + |
| 232 | + // `code` AND `status` — the envelope, not merely "it threw". On |
| 233 | + // `origin/main` both were `undefined` here: what the engine threw was the |
| 234 | + // driver's raw string, and only `mapDataError` at the REST boundary gave |
| 235 | + // it a shape. |
| 236 | + expect(refusal?.code).toBe('INVALID_FIELD'); |
| 237 | + expect(refusal?.status).toBe(400); |
| 238 | + expect(refusal?.field).toBe('zzz_nonexistent_field'); |
| 239 | + expect(refusal?.object).toBe('acct'); |
| 240 | + // Byte-identical to what `mapDataError`'s driver-string branch produced |
| 241 | + // for the same mistake, so the caller reads exactly what it read before — |
| 242 | + // the refusal moved, the answer did not. |
| 243 | + expect(refusal?.message).toBe("Unknown field 'zzz_nonexistent_field' on object 'acct'"); |
| 244 | + }); |
| 245 | + |
| 246 | + it('names every undeclared key, not only the first', async () => { |
| 247 | + const { engine } = await makeEngine(); |
| 248 | + |
| 249 | + const refusal = await refusalOf(() => engine.update('acct', { |
| 250 | + id: 'row-1', name: 'bad', zzz_one: 1, zzz_two: 2, |
| 251 | + } as any)); |
| 252 | + |
| 253 | + expect(refusal?.field).toBe('zzz_one'); |
| 254 | + expect(refusal?.fields).toEqual(['zzz_one', 'zzz_two']); |
| 255 | + }); |
| 256 | + |
| 257 | + it('a key holding `undefined` is still an undeclared key', async () => { |
| 258 | + // `{ ...partial }` is how this arrives from code rather than from JSON, |
| 259 | + // and a mistyped key is a mistyped key whatever it holds. |
| 260 | + const { engine } = await makeEngine(); |
| 261 | + |
| 262 | + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', zzz_typo: undefined } as any)); |
| 263 | + |
| 264 | + expect(refusal?.code).toBe('INVALID_FIELD'); |
| 265 | + expect(refusal?.field).toBe('zzz_typo'); |
| 266 | + }); |
| 267 | + }); |
| 268 | + |
| 269 | + describe('declared keys still update normally', () => { |
| 270 | + it('by-id: a declared payload lands on the driver untouched', async () => { |
| 271 | + const { engine, writes } = await makeEngine(); |
| 272 | + |
| 273 | + await engine.update('acct', { id: 'row-1', name: 'renamed' } as any); |
| 274 | + |
| 275 | + expect(writes).toHaveLength(1); |
| 276 | + expect(writes[0].fn).toBe('update'); |
| 277 | + expect(writes[0].data.name).toBe('renamed'); |
| 278 | + }); |
| 279 | + |
| 280 | + it('multi: a declared payload lands on the driver untouched', async () => { |
| 281 | + const { engine, writes } = await makeEngine(); |
| 282 | + |
| 283 | + await engine.update( |
| 284 | + 'acct', |
| 285 | + { name: 'renamed' } as any, |
| 286 | + MULTI_OPTIONS, |
| 287 | + ); |
| 288 | + |
| 289 | + expect(writes).toHaveLength(1); |
| 290 | + expect(writes[0].fn).toBe('updateMany'); |
| 291 | + expect(writes[0].data.name).toBe('renamed'); |
| 292 | + }); |
| 293 | + }); |
| 294 | + |
| 295 | + // The three no-opinion cases are #8737's, reused rather than re-derived — |
| 296 | + // settled rules from the sibling card. Each is a CONTROL: it asserts the door |
| 297 | + // has NO verdict, so it must pass with the door removed as well as with it in |
| 298 | + // place, and a reverse verification that turned one of these red would mean |
| 299 | + // the door had grown an opinion it is not allowed to have. |
| 300 | + describe('where the door deliberately has NO opinion (reused from #8737)', () => { |
| 301 | + it('a registry-less host gets no verdict — the driver stays the backstop', async () => { |
| 302 | + const { engine, writes } = await makeEngine({ registration: 'none', missingColumns: ['zzz_nonexistent_field'] }); |
| 303 | + |
| 304 | + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', zzz_nonexistent_field: 'x' } as any)); |
| 305 | + |
| 306 | + expect(writes).toHaveLength(1); |
| 307 | + expect(String(refusal?.message)).toContain('has no column named zzz_nonexistent_field'); |
| 308 | + }); |
| 309 | + |
| 310 | + it('an EMPTY field map gets no verdict — an absence is not a prohibition', async () => { |
| 311 | + // A real registered object always carries at least its primary key and |
| 312 | + // the registry's injected audit columns, so a map the door sees as EMPTY |
| 313 | + // means the host did not fill it in. Refusing everything on that reading |
| 314 | + // would be a verdict made from an absence. |
| 315 | + const { engine, writes } = await makeEngine({ stubFields: {}, missingColumns: ['zzz_nonexistent_field'] }); |
| 316 | + |
| 317 | + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', zzz_nonexistent_field: 'x' } as any)); |
| 318 | + |
| 319 | + expect(writes).toHaveLength(1); |
| 320 | + expect(String(refusal?.message)).toContain('has no column named zzz_nonexistent_field'); |
| 321 | + }); |
| 322 | + |
| 323 | + it('`id` / `created_at` / `updated_at` pass even when the declaration omits them', async () => { |
| 324 | + // Mirrors the three names `find()` / `findOne()` already add to their |
| 325 | + // known set: platform-provisioned rather than authored, so a key accepted |
| 326 | + // by a read is not refused by a write. Stubbed rather than registered |
| 327 | + // for the reason `stubFields` records — the registry would otherwise |
| 328 | + // inject `created_at` / `updated_at` itself and the case would prove |
| 329 | + // nothing about the door. `id` is the one name the registry does NOT |
| 330 | + // inject, so it is the door's tolerance being read here, and only its. |
| 331 | + const { engine, writes } = await makeEngine({ |
| 332 | + stubFields: { name: { name: 'name', type: 'text' } }, |
| 333 | + }); |
| 334 | + |
| 335 | + const refusal = await refusalOf(() => engine.update('acct', { |
| 336 | + id: 'row-1', name: 'ok', created_at: '2026-01-01T00:00:00.000Z', updated_at: '2026-01-02T00:00:00.000Z', |
| 337 | + } as any)); |
| 338 | + |
| 339 | + expect(refusal).toBeNull(); |
| 340 | + expect(writes).toHaveLength(1); |
| 341 | + }); |
| 342 | + |
| 343 | + it('schema drift — a DECLARED field whose column is missing — still reaches the driver', async () => { |
| 344 | + // The door's scope is the SCHEMA's field map, so drift is invisible to it |
| 345 | + // by construction and stays the driver's to refuse. `mapDataError`'s |
| 346 | + // driver-string branch is still needed and still fires. |
| 347 | + const { engine, writes } = await makeEngine({ missingColumns: ['description'] }); |
| 348 | + |
| 349 | + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', description: 'v' } as any)); |
| 350 | + |
| 351 | + expect(writes).toHaveLength(1); |
| 352 | + expect(String(refusal?.message)).toContain('has no column named description'); |
| 353 | + }); |
| 354 | + }); |
| 355 | +}); |
0 commit comments