diff --git a/.changeset/19911-readonlywhen-interdependent-locks.md b/.changeset/19911-readonlywhen-interdependent-locks.md index 9cc5f5feafa..025803bf96a 100644 --- a/.changeset/19911-readonlywhen-interdependent-locks.md +++ b/.changeset/19911-readonlywhen-interdependent-locks.md @@ -20,11 +20,15 @@ affected too (a `readonlyWhen` lock binds them). **What happens now.** A value one `readonlyWhen` lock drops can no longer unlock another: no field is written while its `readonlyWhen` is TRUE on the row the update stores. The locks are judged together, again with each dropped -value put back to the row's stored one, until no further field locks; then a -field that was held only by a value that was later put back is released, if -the result agrees with the stored row. In the example above `amount` is -dropped as locked, exactly as `update(c1, { amount: 999 })` on its own always -was. Values a `beforeUpdate` hook wrote are still stored and read as before. +value put back to the row's stored one, until no further field locks. Then the +fields held only by a value that was later put back are released, and every +lock is judged again against what that release stores, round after round, +until the dropped fields are exactly the ones locked on the row the update +stores; after one round more than the caller sent fields carrying a +`readonlyWhen`, the first, larger set of drops stands instead. In the example +above `amount` is dropped as locked, exactly as `update(c1, { amount: 999 })` +on its own always was. Values a `beforeUpdate` hook wrote are still stored and +read as before. **What else you may see move:** @@ -39,16 +43,21 @@ was. Values a `beforeUpdate` hook wrote are still stored and read as before. the amount the row keeps: a requirement only the let-through amount raised no longer refuses the write, and clearing a field the kept amount requires is now refused (`VALIDATION_FAILED`). -- The release is one step, not a search. When it does not settle the drops, - every lock involved holds and the field is dropped, never written — so a - field whose own lock is FALSE on the stored row can still be dropped. That - happens when locks read each other in a cycle (no set of drops agrees with - the stored row), and in a cascade where releasing one field changes another's - verdict: with `c` locked by `previous.c == 'L'`, `x` by `record.c == 'open'` - and `y` by `record.x == 'xv'`, `update(r, { c: 'open', x: 'xv', y: 'yv' })` - on a row with `c: 'L'` drops all three, although `x` is unlocked on the - stored row. That was dropped before this change too; it is tracked as - #19927. +- A field whose own lock is FALSE on the stored row can still be dropped, but + only where locks read each other in a cycle (a `parent`-scoped lock counts + as reading the master-detail field, which picks the header). Without such a + cycle the update drops exactly the caller's fields whose lock is TRUE on the + row it stores (on a bulk update, on at least one matched row). In a cycle, + no set of drops may agree with the stored row: with `a` locked by `record.b + == 'x'` and `b` by `record.a == 'old_a'`, `update(r, { a: 'new_a', b: 'x' + })` on a row `{ a: 'old_a', b: 'y' }` drops both, although `a` is unlocked + on the row it stores. A cycle can also have more than one set that agrees: + with `a` locked by `record.b == 'new_b'` and `b` by `record.a == 'new_a'`, + `update(r, { a: 'new_a', b: 'new_b' })` on a row holding neither new value + would agree with the row by dropping either one, and it drops both. Some + other updates with two such sets store one of them. Where the drops do not + settle, the first, larger set stands: a lock the update cannot settle is + not waived. - A master-detail repoint that the field's own `record`-scoped lock used to hold can now land. Its lock is judged together with the other locks on the header the update names, so when the value that lock reads is itself locked diff --git a/.changeset/19927-readonlywhen-exact-drop-set.md b/.changeset/19927-readonlywhen-exact-drop-set.md new file mode 100644 index 00000000000..ccfce14c40d --- /dev/null +++ b/.changeset/19927-readonlywhen-exact-drop-set.md @@ -0,0 +1,40 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): a chain of `readonlyWhen` locks no longer ignores an edit whose own lock is FALSE on the row the update stores (#19927) + +**What happened before.** When `readonlyWhen` locks read each other in a chain, +an update could ignore an edit whose own lock was FALSE on the row it stored. +With `c: { readonlyWhen: "previous.c == 'L'" }`, `x: { readonlyWhen: "record.c +== 'open'" }` and `y: { readonlyWhen: "record.x == 'xv'" }`, `update(r, { c: +'open', x: 'xv', y: 'yv' })` on a row with `c: 'L'` ignored all three fields. +The row keeps `c: 'L'`, so `x`'s lock is FALSE there, yet its edit was lost +with no refusal, and a `strictReadonlyWrites` refusal named `x` as read-only. + +**What happens now.** That update stores `x: 'xv'` and ignores `c` and `y`: +`c` is locked, and `y`'s lock reads the `x` the row now holds. This holds by +id and on bulk (`multi: true`) updates, and for `isSystem` callers. + +**What else you may see move:** + +- `onFieldsDropped` reports only the fields the update ignored (`['c', 'y']` + above), and a `strictReadonlyWrites` refusal names only those. Whether a + write is refused under that option does not change. +- An edit that used to be stored can now be ignored. When a field that used + to be ignored now lands, a lock that reads it can be TRUE on the row the + update stores, and that lock's field is then ignored instead of written. + With `p` locked by `previous.p == 'L'`, `m` by `record.p == 'L'`, `j` by + `record.m == 'new'` and `k` by `record.p == 'L' && record.j == 'new'`, + `update(r, { p: 'new', m: 'new', j: 'new', k: 'new' })` on a row `{ p: 'L', + m: 'old', j: 'old', k: 'old' }` used to ignore `j` and store `k: 'new'`; it + now stores `j: 'new'` and ignores `k`, whose lock reads that `j`. A + `strictReadonlyWrites` refusal of that update now names `k` instead of `j`. +- A master-detail repoint that such a chain used to hold can now land. With + `c` locked by `previous.c == 'L'`, the master-detail `invoice` by `record.c == + 'open'`, `y` by `record.invoice == 'h_open'` and `amt` by `parent.status == + 'paid'`, an update setting all four on a line with `c: 'L'` under a paid + invoice used to keep the line there, store `y` and ignore `amt`. It now moves + the line to `h_open`, stores `amt` (unlocked under the open invoice) and + ignores `y`, by id and on bulk updates. A `strictReadonlyWrites` refusal of + that update now names `c` and `y` instead of `c`, `invoice` and `amt`. diff --git a/packages/objectql/src/engine-readonly-when-exact-drop-set.test.ts b/packages/objectql/src/engine-readonly-when-exact-drop-set.test.ts new file mode 100644 index 00000000000..e41ae4467b9 --- /dev/null +++ b/packages/objectql/src/engine-readonly-when-exact-drop-set.test.ts @@ -0,0 +1,394 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #19927 — a chain of `readonlyWhen` locks settles on the drop set that agrees +// with the row the write stores, not the fixpoint's larger one. +// +// `c` locked by `previous.c == 'L'`, `x` by `record.c == 'open'`, `y` by +// `record.x == 'xv'`; row `{ c: 'L', x: 'old', y: 'old' }`; `update({ c: +// 'open', x: 'xv', y: 'yv' })`. `c` is locked, so the row keeps `c: 'L'`; on +// that row `x`'s lock is FALSE, so `x: 'xv'` lands; `y`'s lock then reads +// `'xv'` and is TRUE. The only drop set that agrees with the stored row is +// `{c, y}`. The settlement (`settleReadonlyWhenDrops`) released once and gave +// up when that one step did not settle, so it dropped `{c, x, y}`: a write +// whose own lock was FALSE was ignored, and a `strictReadonlyWrites` refusal +// named `x`. The release now repeats until a set gives back itself. +// +// Every expected row below was checked against a brute-force enumeration of +// the drop sets that agree with the row. A cycle with no such set keeps the +// fail-safe drop; a cycle with several gets the one the iteration reaches, or +// the fail-safe drop when it reaches none. +// +// #19911's suite (`engine-readonly-when-interdependent-locks.test.ts`) pins +// the fixpoint and the one-step release this extends; it is unchanged. +// +// Real engine, the #19911 suite's in-memory driver shape; by-id and bulk. + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { stripReadonlyWhenFields } from './validation/rule-validator.js'; + +function makeDriver() { + const stores = new Map>(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = new Map(); stores.set(o, s); } + return s; + }; + const checkOp = (value: any, cond: any): boolean => { + if (cond === null || typeof cond !== 'object' || Array.isArray(cond) || cond instanceof Date) { + return value === cond; + } + return Object.entries(cond).every(([op, target]: [string, any]) => { + switch (op) { + case '$eq': return value === target; + case '$ne': return value !== target; + case '$in': return Array.isArray(target) && target.includes(value); + default: return true; + } + }); + }; + const matches = (row: any, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]: [string, any]) => { + if (k === '$and') return (v as any[]).every((w) => matches(row, w)); + if (k === '$or') return (v as any[]).some((w) => matches(row, w)); + if (k === '$not') return !matches(row, v); + return checkOp(row?.[k], v); + }); + }; + let n = 0; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(object: string, ast: any) { + const rows = Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)); + return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matches(r, ast?.where)) return r; + return null; + }, + async create(object: string, data: Record) { + n += 1; + const id = (data.id as string) ?? `r_${n}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const row = { ...s.get(id), ...data, id }; + s.set(id, row); + return row; + }, + async updateMany(object: string, ast: any, data: Record) { + const s = storeFor(object); + let count = 0; + for (const row of [...s.values()]) { + if (!matches(row, ast?.where)) continue; + s.set(row.id, { ...row, ...data, id: row.id }); + count += 1; + } + return count; + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count() { return 0; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r, undefined))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, storeFor }; +} + +describe('a chain of readonlyWhen locks settles on the drop set the stored row agrees with (#19927)', () => { + let engine: ObjectQL; + let storeFor: ReturnType['storeFor']; + let warns: string[]; + + beforeEach(async () => { + warns = []; + const logger: any = { + warn: (m: string) => warns.push(String(m)), + debug() {}, info() {}, error() {}, trace() {}, fatal() {}, + child() { return logger; }, + }; + engine = new ObjectQL({ logger }); + const d = makeDriver(); + storeFor = d.storeFor; + engine.registerDriver(d.driver, true); + await engine.init(); + // The card's cascade: exact set {c, y}. + engine.registry.registerObject({ + name: 'cascade', + fields: { + c: { type: 'text', readonlyWhen: "previous.c == 'L'" }, + x: { type: 'text', readonlyWhen: "record.c == 'open'" }, + y: { type: 'text', readonlyWhen: "record.x == 'xv'" }, + tag: { type: 'text' }, + }, + } as any); + // A six-lock cascade: each release moves the next lock's verdict, so the + // set settles only after several rounds. Exact set {c, x2, x4}. + engine.registry.registerObject({ + name: 'cascade_six', + fields: { + c: { type: 'text', readonlyWhen: "previous.c == 'L'" }, + x1: { type: 'text', readonlyWhen: "record.c == 'open'" }, + x2: { type: 'text', readonlyWhen: "record.x1 == 'v'" }, + x3: { type: 'text', readonlyWhen: "record.x2 == 'v'" }, + x4: { type: 'text', readonlyWhen: "record.x3 == 'v'" }, + x5: { type: 'text', readonlyWhen: "record.x4 == 'v'" }, + }, + } as any); + // No cycle, and the exact set is not inside the fixpoint's: releasing `j` + // locks `k`, which the fixpoint kept. Exact set {p, m, k}. + engine.registry.registerObject({ + name: 'knock_on', + fields: { + p: { type: 'text', readonlyWhen: "previous.p == 'L'" }, + m: { type: 'text', readonlyWhen: "record.p == 'L'" }, + j: { type: 'text', readonlyWhen: "record.m == 'new'" }, + k: { type: 'text', readonlyWhen: "record.p == 'L' && record.j == 'new'" }, + tag: { type: 'text' }, + }, + } as any); + // A cycle with NO exact set. + engine.registry.registerObject({ + name: 'cycle_none', + fields: { + a: { type: 'text', readonlyWhen: "record.b == 'x'" }, + b: { type: 'text', readonlyWhen: "record.a == 'old_a'" }, + tag: { type: 'text' }, + }, + } as any); + // A cycle with TWO exact sets, {a} and {b}: the iteration reaches neither. + engine.registry.registerObject({ + name: 'cycle_two', + fields: { + a: { type: 'text', readonlyWhen: "record.b == 'new_b'" }, + b: { type: 'text', readonlyWhen: "record.a == 'new_a'" }, + tag: { type: 'text' }, + }, + } as any); + // A cycle with TWO exact sets, {} and {a, b}: the fixpoint's first pass + // locks nothing, so {} is the answer. + engine.registry.registerObject({ + name: 'cycle_empty', + fields: { + a: { type: 'text', readonlyWhen: "record.b == 'old'" }, + b: { type: 'text', readonlyWhen: "record.a == 'old'" }, + }, + } as any); + // The cascade through #19853's settlement: the FK's own lock reads a value + // a `previous`-locked field keeps, a third lock reads the FK, and a + // `parent`-scoped lock makes the settlement run. + engine.registry.registerObject({ name: 'hdr', fields: { status: { type: 'text' } } } as any); + engine.registry.registerObject({ + name: 'line_cascade', + fields: { + c: { type: 'text', readonlyWhen: "previous.c == 'L'" }, + invoice: { type: 'master_detail', reference: 'hdr', readonlyWhen: "record.c == 'open'" }, + y: { type: 'text', readonlyWhen: "record.invoice == 'h_open'" }, + amt: { type: 'text', readonlyWhen: "parent.status == 'paid'" }, + tag: { type: 'text' }, + }, + } as any); + + storeFor('cascade').set('r1', { id: 'r1', c: 'L', x: 'old', y: 'old', tag: 't' }); + storeFor('cascade').set('r2', { id: 'r2', c: 'L', x: 'old', y: 'old', tag: 't' }); + storeFor('cascade_six').set('s1', { id: 's1', c: 'L', x1: 'o', x2: 'o', x3: 'o', x4: 'o', x5: 'o' }); + storeFor('knock_on').set('k1', { id: 'k1', p: 'L', m: 'old', j: 'old', k: 'old', tag: 't' }); + storeFor('knock_on').set('k2', { id: 'k2', p: 'L', m: 'old', j: 'old', k: 'old', tag: 't' }); + storeFor('cycle_none').set('n1', { id: 'n1', a: 'old_a', b: 'y', tag: 't' }); + storeFor('cycle_none').set('n2', { id: 'n2', a: 'old_a', b: 'y', tag: 't' }); + storeFor('cycle_two').set('w1', { id: 'w1', a: 'old', b: 'old', tag: 't' }); + storeFor('cycle_empty').set('e1', { id: 'e1', a: 'old', b: 'old' }); + storeFor('hdr').set('h_paid', { id: 'h_paid', status: 'paid' }); + storeFor('hdr').set('h_open', { id: 'h_open', status: 'open' }); + storeFor('line_cascade').set('l1', { id: 'l1', c: 'L', invoice: 'h_paid', y: 'old', amt: 'a0', tag: 't' }); + storeFor('line_cascade').set('l2', { id: 'l2', c: 'L', invoice: 'h_paid', y: 'old', amt: 'a0', tag: 't' }); + }); + + const row = (object: string, id: string) => storeFor(object).get(id); + + async function rejection(p: Promise): Promise { + try { + await p; + } catch (err) { + return err; + } + throw new Error('expected the write to be refused, but it resolved'); + } + + function dropEvents(): { events: any[]; options: any } { + const events: any[] = []; + return { events, options: { onFieldsDropped: (e: any) => events.push(e) } }; + } + + /** Every header id the write reads, in order (single-id and batched reads). */ + function recordHeaderReads(): unknown[] { + const reads: unknown[] = []; + const findOne = (engine as any).findOne.bind(engine); + (engine as any).findOne = async (name: string, q: any, o?: any) => { + if (name === 'hdr') reads.push(q?.where?.id); + return findOne(name, q, o); + }; + return reads; + } + + // ── The card ────────────────────────────────────────────────────────── + + it('THE CARD: x, unlocked on the row the write stores, lands; c and y drop', async () => { + const { events, options } = dropEvents(); + await engine.update('cascade', { id: 'r1', c: 'open', x: 'xv', y: 'yv' }, options); + expect(row('cascade', 'r1')).toMatchObject({ c: 'L', x: 'xv', y: 'old' }); + expect(events).toEqual([{ object: 'cascade', fields: ['c', 'y'], reason: 'readonly_when' }]); + expect(warns.filter((w) => w.includes('is read-only (readonlyWhen)'))).toEqual([ + "Field 'c' is read-only (readonlyWhen) — ignoring incoming change", + "Field 'y' is read-only (readonlyWhen) — ignoring incoming change", + ]); + }); + + it('THE CARD in BULK: every matched row takes x and keeps c and y', async () => { + const { events, options } = dropEvents(); + await engine.update('cascade', { c: 'open', x: 'xv', y: 'yv' }, { ...options, where: { tag: 't' }, multi: true } as any); + expect(row('cascade', 'r1')).toMatchObject({ c: 'L', x: 'xv', y: 'old' }); + expect(row('cascade', 'r2')).toMatchObject({ c: 'L', x: 'xv', y: 'old' }); + expect(events).toEqual([{ object: 'cascade', fields: ['c', 'y'], reason: 'readonly_when' }]); + }); + + it('THE CARD under strictReadonlyWrites: refused, naming only the locked c and y, and nothing lands', async () => { + const err = await rejection(engine.update('cascade', { id: 'r1', c: 'open', x: 'xv', y: 'yv' }, { strictReadonlyWrites: true } as any)); + expect(err.code).toBe('ERR_READONLY_FIELD_REJECTED'); + expect(err.fields).toEqual(['c', 'y']); + expect(err.drops).toEqual([{ object: 'cascade', fields: ['c', 'y'], reason: 'readonly_when' }]); + expect(row('cascade', 'r1')).toMatchObject({ c: 'L', x: 'old', y: 'old' }); + }); + + it('THE CARD in BULK under strictReadonlyWrites: refused, naming only c and y, and no row moves', async () => { + const err = await rejection(engine.update('cascade', { c: 'open', x: 'xv', y: 'yv' }, { where: { tag: 't' }, multi: true, strictReadonlyWrites: true } as any)); + expect(err.code).toBe('ERR_READONLY_FIELD_REJECTED'); + expect(err.fields).toEqual(['c', 'y']); + expect(row('cascade', 'r1')).toMatchObject({ c: 'L', x: 'old', y: 'old' }); + expect(row('cascade', 'r2')).toMatchObject({ c: 'L', x: 'old', y: 'old' }); + }); + + it('THE CARD for an isSystem caller: the locks bind it the same way', async () => { + await engine.update('cascade', { id: 'r1', c: 'open', x: 'xv', y: 'yv' }, { context: { isSystem: true } } as any); + expect(row('cascade', 'r1')).toMatchObject({ c: 'L', x: 'xv', y: 'old' }); + }); + + it('a six-lock cascade settles after several rounds on the one set the row agrees with', async () => { + const { events, options } = dropEvents(); + await engine.update('cascade_six', { id: 's1', c: 'open', x1: 'v', x2: 'v', x3: 'v', x4: 'v', x5: 'v' }, options); + expect(row('cascade_six', 's1')).toMatchObject({ c: 'L', x1: 'v', x2: 'o', x3: 'v', x4: 'o', x5: 'v' }); + expect(events).toEqual([{ object: 'cascade_six', fields: ['c', 'x2', 'x4'], reason: 'readonly_when' }]); + }); + + it('KNOCK-ON: j, unlocked on the stored row, lands; that locks k, which used to land, so k drops', async () => { + const { events, options } = dropEvents(); + await engine.update('knock_on', { id: 'k1', p: 'new', m: 'new', j: 'new', k: 'new' }, options); + expect(row('knock_on', 'k1')).toMatchObject({ p: 'L', m: 'old', j: 'new', k: 'old' }); + expect(events).toEqual([{ object: 'knock_on', fields: ['p', 'm', 'k'], reason: 'readonly_when' }]); + }); + + it('KNOCK-ON in BULK: every matched row takes j and keeps k', async () => { + await engine.update('knock_on', { p: 'new', m: 'new', j: 'new', k: 'new' }, { where: { tag: 't' }, multi: true } as any); + expect(row('knock_on', 'k1')).toMatchObject({ p: 'L', m: 'old', j: 'new', k: 'old' }); + expect(row('knock_on', 'k2')).toMatchObject({ p: 'L', m: 'old', j: 'new', k: 'old' }); + }); + + it('KNOCK-ON under strictReadonlyWrites: still refused, now naming k instead of j', async () => { + const err = await rejection(engine.update('knock_on', { id: 'k1', p: 'new', m: 'new', j: 'new', k: 'new' }, { strictReadonlyWrites: true } as any)); + expect(err.code).toBe('ERR_READONLY_FIELD_REJECTED'); + expect(err.fields).toEqual(['p', 'm', 'k']); + expect(row('knock_on', 'k1')).toMatchObject({ p: 'L', m: 'old', j: 'old', k: 'old' }); + }); + + // ── Cycles ──────────────────────────────────────────────────────────── + + it('CYCLE with no exact set: both locks hold (fail-safe), by id', async () => { + const { events, options } = dropEvents(); + await engine.update('cycle_none', { id: 'n1', a: 'new_a', b: 'x' }, options); + expect(row('cycle_none', 'n1')).toMatchObject({ a: 'old_a', b: 'y' }); + expect(events).toEqual([{ object: 'cycle_none', fields: ['a', 'b'], reason: 'readonly_when' }]); + }); + + it('CYCLE with no exact set in BULK: both locks hold in every row (fail-safe)', async () => { + await engine.update('cycle_none', { a: 'new_a', b: 'x' }, { where: { tag: 't' }, multi: true } as any); + expect(row('cycle_none', 'n1')).toMatchObject({ a: 'old_a', b: 'y' }); + expect(row('cycle_none', 'n2')).toMatchObject({ a: 'old_a', b: 'y' }); + }); + + it('CYCLE with two exact sets, {a} and {b}: the iteration reaches neither, and both locks hold (fail-safe)', async () => { + const { events, options } = dropEvents(); + await engine.update('cycle_two', { id: 'w1', a: 'new_a', b: 'new_b' }, options); + expect(row('cycle_two', 'w1')).toMatchObject({ a: 'old', b: 'old' }); + expect(events).toEqual([{ object: 'cycle_two', fields: ['a', 'b'], reason: 'readonly_when' }]); + }); + + it('CYCLE with two exact sets, {} and {a, b}: nothing locks on the first pass, so both land', async () => { + const { events, options } = dropEvents(); + await engine.update('cycle_empty', { id: 'e1', a: 'new_a', b: 'new_b' }, options); + expect(row('cycle_empty', 'e1')).toMatchObject({ a: 'new_a', b: 'new_b' }); + expect(events).toEqual([]); + }); + + // ── The same search inside #19853's settlement ──────────────────────── + + it('SETTLEMENT: the repoint the cascade used to hold now lands, y drops, and amt is judged under the header it lands on', async () => { + const { events, options } = dropEvents(); + const reads = recordHeaderReads(); + await engine.update('line_cascade', { id: 'l1', c: 'open', invoice: 'h_open', y: 'yv', amt: 'a1' }, options); + expect(row('line_cascade', 'l1')).toMatchObject({ c: 'L', invoice: 'h_open', y: 'old', amt: 'a1' }); + expect(events).toEqual([{ object: 'line_cascade', fields: ['c', 'y'], reason: 'readonly_when' }]); + // The header the update names, then the repoint's reference check on it. + expect(reads).toEqual(['h_open', 'h_open']); + }); + + it('SETTLEMENT in BULK: every matched line lands on the named header, and every y drops', async () => { + const { events, options } = dropEvents(); + await engine.update('line_cascade', { c: 'open', invoice: 'h_open', y: 'yv', amt: 'a1' }, { ...options, where: { tag: 't' }, multi: true } as any); + expect(row('line_cascade', 'l1')).toMatchObject({ c: 'L', invoice: 'h_open', y: 'old', amt: 'a1' }); + expect(row('line_cascade', 'l2')).toMatchObject({ c: 'L', invoice: 'h_open', y: 'old', amt: 'a1' }); + expect(events).toEqual([{ object: 'line_cascade', fields: ['c', 'y'], reason: 'readonly_when' }]); + }); + + it('SETTLEMENT under strictReadonlyWrites: refused, naming c and y, and the line stays where it was', async () => { + const err = await rejection(engine.update('line_cascade', { id: 'l1', c: 'open', invoice: 'h_open', y: 'yv', amt: 'a1' }, { strictReadonlyWrites: true } as any)); + expect(err.code).toBe('ERR_READONLY_FIELD_REJECTED'); + expect(err.fields).toEqual(['c', 'y']); + expect(row('line_cascade', 'l1')).toMatchObject({ c: 'L', invoice: 'h_paid', y: 'old', amt: 'a0' }); + }); +}); + +describe('stripReadonlyWhenFields settles the cascade (#19927)', () => { + const schema = { + fields: { + c: { type: 'text', readonlyWhen: "previous.c == 'L'" }, + x: { type: 'text', readonlyWhen: "record.c == 'open'" }, + y: { type: 'text', readonlyWhen: "record.x == 'xv'" }, + }, + } as any; + const prior = { c: 'L', x: 'old', y: 'old' }; + + it('takes c and y, and leaves x', () => { + expect(stripReadonlyWhenFields(schema, { c: 'open', x: 'xv', y: 'yv' }, prior)).toEqual({ x: 'xv' }); + }); + + it('`only: x` leaves the payload whole: x is not taken, and nothing is said', () => { + const warned: string[] = []; + const data = { c: 'open', x: 'xv', y: 'yv' }; + const out = stripReadonlyWhenFields(schema, data, prior, { warn: (m: string) => warned.push(m) }, undefined, { only: 'x' }); + expect(out).toBe(data); + expect(warned).toEqual([]); + }); + + it('`only: y` takes y alone, judged over the settled drops of the others', () => { + const out = stripReadonlyWhenFields(schema, { c: 'open', x: 'xv', y: 'yv' }, prior, undefined, undefined, { only: 'y' }); + expect(out).toEqual({ c: 'open', x: 'xv' }); + }); +}); diff --git a/packages/objectql/src/engine-readonly-when-interdependent-locks.test.ts b/packages/objectql/src/engine-readonly-when-interdependent-locks.test.ts index 1230677fdfe..148b83b9447 100644 --- a/packages/objectql/src/engine-readonly-when-interdependent-locks.test.ts +++ b/packages/objectql/src/engine-readonly-when-interdependent-locks.test.ts @@ -13,12 +13,14 @@ // another field locked it, though the row never took that value. // // The strip now settles its drops (`settleReadonlyWhenDrops`): a monotone -// fixpoint, which never opens a lock, then one release step, kept only when -// every dropped key is locked and every kept key unlocked on the row the write -// stores. When that one step does not settle the set — a cycle, or a cascade -// where releasing one key moves another's verdict (#19927) — the fixpoint's -// larger, fail-safe drop set stands. Every expected row below was checked -// against a brute-force enumeration of the drop sets that agree with the row. +// fixpoint, which never opens a lock, then a release, kept only when every +// dropped key is locked and every kept key unlocked on the row the write +// stores. Since #19927 the release repeats until a set gives back itself +// (`engine-readonly-when-exact-drop-set.test.ts`); when none does within its +// bound — always on a cycle with no drop set that agrees with the row — the +// fixpoint's larger, fail-safe drop set stands. Every expected row below was +// checked against a brute-force enumeration of the drop sets that agree with +// the row. // // #19887's `stored` view (`engine-readonly-when-stored-view.test.ts`) and // #19853's master-detail settlement (`engine-readonly-when-parent.test.ts`) diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index f3934b80f68..6df2d1e25b7 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -769,8 +769,8 @@ function withoutKeys( /** * [#19911] Which judged keys the conditional strip drops — so that a value * another lock in the same strip takes back out can never unlock a lock, and, - * where one release step can settle it, no key is dropped that is unlocked on - * the row the write stores. + * [#19927] wherever the locks' reads of one another form no cycle, no key is + * dropped that is unlocked on the row the write stores. * * ## Why one pass was not enough * @@ -786,12 +786,13 @@ function withoutKeys( * against a `'closed'` the row never took. `stored` (#19887) closed the static * strip's twin of both; this is the conditional strip's own. * - * ## The rule: never open a lock, then agree with the stored row where one step can + * ## The rule: never open a lock, then agree with the stored row * * A key is judged with its OWN incoming value (a lock that reads its own field * judges the write) and every OTHER dropped key reverted to the prior row's * value. A drop set is EXACT when every key it drops is locked, and every key - * it keeps is unlocked, on that view. Reached in two steps: + * it keeps is unlocked, on that view: judging every key against an exact set + * gives back the set itself. Reached in two steps: * * ① A monotone fixpoint. Judge every key; drop the ones that lock; re-judge * the rest with those reverted; repeat until a pass locks nothing new. @@ -799,23 +800,47 @@ function withoutKeys( * keeps is unlocked on the row it stores: it never opens a lock. Its first * pass IS the single pass it replaced, so a write whose drops move no * other lock's verdict is judged exactly as before. - * ② An exact release. A key ① dropped but that is unlocked on ①'s row was + * ② A release iteration. A key ① dropped but that is unlocked on ①'s row was * locked by a value a later pass reverted. Every such key is released at - * once, and the result is kept only if it is exact; otherwise ①'s answer + * once; then every key is judged against that set, the keys that lock are + * the next set, and so on until a set gives back itself. That set is exact + * and is the answer. After n + 1 sets (n keys judged) the iteration stops, + * and ①'s answer stands. + * + * What that guarantees: + * - No lock opens. The answer is ①'s set, or a set that gave back itself, + * which is exact; both keep only keys unlocked on the row they store. + * - Without a cycle the exact set is unique, and ② reaches it. A key's + * verdict then depends only on the keys its `record` reads name, so after + * t sets every key at depth below t in that read graph holds its final + * verdict: the n-th set is exact and the (n + 1)-th gives it back. The + * #19927 cascade — `c` locked by `previous.c == 'L'`, `x` by `record.c == + * 'open'`, `y` by `record.x == 'xv'`, a write setting all three on a row + * with `c: 'L'` — releases `x` and `y` to `{c}`, locks `y` again in the + * second set `{c, y}`, and the third gives that back: its exact set, where + * the single release step this replaced stopped at `{c}`'s failed check. + * - A cycle can have no exact set, one, or several. With none, no set gives + * back itself and ①'s larger drop set stands: the fail-safe direction, where + * a lock that cannot be settled is not waived (#4889's frozen lines depend + * on that). `a` locked by `record.b == 'x'`, `b` by `record.a == 'old'`, a + * write setting both, is one. With several, the answer is the one ② + * reaches, or ①'s when it reaches none: `a` locked by `record.b == + * 'new_b'` and `b` by `record.a == 'new_a'`, a write setting both, has + * `{a}` and `{b}`; ② alternates between `{}` and `{a, b}`, and ①'s `{a, b}` * stands. - * - * ① alone is guaranteed; ② is one step, not a search. When it does not settle - * the set, ①'s larger drop set stands, so a key whose own lock is FALSE on - * the stored row can still be dropped — the fail-safe direction, where a lock - * that cannot be settled is not waived (#4889's frozen lines depend on that). - * Two shapes, measured: - * - a CYCLE has no exact set at all — `a` locked by `record.b == 'x'`, `b` by - * `record.a == 'old'`, a write setting both; - * - a CASCADE can have one that ② misses (#19927) — `c` locked by - * `previous.c == 'L'`, `x` by `record.c == 'open'`, `y` by `record.x == - * 'xv'`, a write setting all three on a row with `c: 'L'`. The exact set is - * `{c, y}`, but releasing `x` moves `y`'s verdict, so the release fails its - * check and `{c, x, y}` stands, as it did before this function existed. + * - No order matters: each step judges every key against one set, so field + * declaration order and payload key order cannot move the answer. + * - The exact set need not lie inside ①'s: a key ① kept can lock once + * another key is released, and the exact set drops it. + * - Each claim is about the views this function is handed. The engine's + * master-detail settlement judges the FK here against the header it NAMES + * and, when the FK does not land, judges the rest against the header the + * row KEEPS (#19853): an FK dropped that way can be unlocked on the row the + * write stores, as it could before #19927. + * + * Cost, counted in key judgements (a bulk write judges each over its matched + * rows): at most n(n + 1) / 2 in ①, at most n + n² in ②. When ① or its first + * release settles the set, ② judges exactly what it did before #19927. * * Each key's warnings come from the evaluation that decided it and are handed * back rather than logged, so a key judged more than once still warns once. @@ -856,17 +881,26 @@ function settleReadonlyWhenDrops( standing = standing.filter((name) => !dropped.has(name)); } // ② Every standing key is unlocked on ①'s row; a dropped key unlocked there - // too is over-locked. - const overLocked = [...dropped].filter((name) => !judge(name, dropped).locked); - if (overLocked.length === 0) return { dropped, warnings }; - const released = new Set([...dropped].filter((name) => !overLocked.includes(name))); - const releasedWarnings = new Map(); - for (const name of judged) { - const verdict = judge(name, released); - if (verdict.locked !== released.has(name)) return { dropped, warnings }; - releasedWarnings.set(name, verdict.said); + // too is over-locked. The first set releases them all at once; each later + // set is every key that locks when judged against the one before, until a + // set gives back itself — n more sets at most. The answer's warnings are + // those of the judgements that gave it back. + let current = new Set([...dropped].filter((name) => judge(name, dropped).locked)); + if (current.size === dropped.size) return { dropped, warnings }; + for (let step = 0; step < judged.length; step++) { + const next = new Set(); + const nextWarnings = new Map(); + for (const name of judged) { + const verdict = judge(name, current); + nextWarnings.set(name, verdict.said); + if (verdict.locked) next.add(name); + } + if (next.size === current.size && [...next].every((name) => current.has(name))) { + return { dropped: current, warnings: nextWarnings }; + } + current = next; } - return { dropped: released, warnings: releasedWarnings }; + return { dropped, warnings }; } /**