diff --git a/.changeset/26871-predicate-delete-referential-walk.md b/.changeset/26871-predicate-delete-referential-walk.md new file mode 100644 index 00000000000..80d4e4f4d42 --- /dev/null +++ b/.changeset/26871-predicate-delete-referential-walk.md @@ -0,0 +1,19 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql)!: a predicate delete (`multi: true` with a filter) honours `deleteBehavior` for every row it matches: a `restrict` refuses, a `master_detail` cascades, a `set_null` clears (#26871) + +Clause-②: no (narrowing) — a predicate delete (`engine.delete(object, { where, multi: true })`, which a flow's `delete_record` node with `multi: true` and a filter reaches) refuses with 409 `DELETE_RESTRICTED` where it succeeded: when a row it matches is held by a `restrict` reference (authored, or a required lookup's `set_null`, which is refused as `restrict`). Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the predicate delete (`@objectstack/objectql`), served by every caller of `delete` with `multi: true` and a filter, the flow `delete_record` node among them. + +- **Before.** A predicate delete went straight to the driver's bulk delete and ran no referential walk. No `restrict` refused it, the matched rows' `master_detail` children were left behind, and `set_null` lookups kept pointing at deleted rows. The same rows deleted one at a time by id (`DELETE /api/v1/data/:object/:id`, the bulk `deleteManyData` door) refused or cascaded. +- **Now.** When any relation of the delete family (`master_detail`, `lookup`, `tree`) points at the object, the predicate delete reads the rows it matches under the caller's scope, runs the by-id delete's referential walk for each, and then deletes exactly those rows, all in one unit of work on a single datasource. A `restrict` on any matched row refuses the whole delete with the existing `409 DELETE_RESTRICTED`, and nothing is deleted; `master_detail` children (and every `cascade`) are deleted; `set_null` references are cleared. The answer is still the count of rows the delete removed. +- **Unchanged.** An object no relation points at keeps the single bulk delete: no row is read and no transaction is opened. A cascade that reaches another datasource runs without a transaction and warns once, as the by-id delete does. A predicate delete of `sys_organization` is still refused. +- **Who sees it.** A flow (or in-process caller) that bulk-deletes rows held by a `restrict` reference now fails instead of deleting them; one that bulk-deletes masters now deletes their details and clears their lookups instead of leaving them dangling. + +**The remedy.** Delete or reassign the restricting rows first, then run the bulk delete. If the referencing rows are meant to go with the rows deleted, declare `deleteBehavior: 'cascade'` on that reference. diff --git a/packages/objectql/src/engine-cascade-delete-atomic.test.ts b/packages/objectql/src/engine-cascade-delete-atomic.test.ts index a51bacc20a0..6f25bee0348 100644 --- a/packages/objectql/src/engine-cascade-delete-atomic.test.ts +++ b/packages/objectql/src/engine-cascade-delete-atomic.test.ts @@ -68,6 +68,11 @@ function makeDriver(opts: { transactional?: boolean } = {}) { if (!where || typeof where !== 'object') return true; for (const [k, v] of Object.entries(where)) { if (k.startsWith('$')) continue; + // [#26871] The predicate delete removes the rows it walked by id set. + if (v && typeof v === 'object' && Array.isArray((v as any).$in)) { + if (!(v as any).$in.some((x: unknown) => String(x) === String(row[k]))) return false; + continue; + } const exp = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v; if ((row[k] ?? null) !== (exp ?? null)) return false; } @@ -404,16 +409,168 @@ describe('a plain non-cascade delete is unchanged (#7413 control)', () => { expect(rec.at('warn')).toHaveLength(0); }); - it('opens no transaction when the object HAS dependents declared but the delete is by predicate', async () => { - // The predicate branch never called `cascadeDeleteRelations` and still does - // not — the wrap is the by-id branch's alone. - const { engine, driver } = await makeEngine([parent, kid]); - await engine.insert('parent', { name: 'P' }); + it('opens no transaction for a predicate delete when no relation points at the object', async () => { + // [#26871] The predicate twin of the control above: nothing to walk, so the + // predicate arm keeps its single `deleteMany` — no id resolution, no wrap. + const { engine, driver, rec } = await makeEngine([loner]); + await engine.insert('loner', { name: 'solo' }); + await engine.insert('loner', { name: 'solo' }); driver.writes.length = 0; - await engine.delete('parent', { where: { name: 'P' }, multi: true } as any); + const removed = await engine.delete('loner', { where: { name: 'solo' }, multi: true }); + expect(removed).toBe(2); expect(driver.begun).toHaveLength(0); + expect(driver.rowsOf('loner')).toHaveLength(0); + for (const w of driver.writes) expect(w.transaction).toBeUndefined(); + expect(rec.at('warn')).toHaveLength(0); + }); +}); + +// --------------------------------------------------------------------------- +// 3b. [#26871] A predicate delete walks every matched row, as one unit of work. +// --------------------------------------------------------------------------- + +/** A `master_detail` detail of `parent`: the default behaviour is `cascade`. */ +const detail = { + name: 'detail', + label: 'Detail', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + parent: { name: 'parent', type: 'master_detail' as const, reference: 'parent' }, + }, +}; +/** An authored `restrict` lookup to `parent`. */ +const guard = { + name: 'guard', + label: 'Guard', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + parent: { name: 'parent', type: 'lookup' as const, reference: 'parent', deleteBehavior: 'restrict' }, + }, +}; + +describe('[#26871] a predicate delete runs the referential walk for every matched row', () => { + it('a restrict on one matched row refuses the whole delete, and nothing is deleted', async () => { + const { engine, driver } = await makeEngine([parent, detail, guard]); + const p1 = await engine.insert('parent', { name: 'doomed' }); + const p2 = await engine.insert('parent', { name: 'doomed' }); + await engine.insert('parent', { name: 'kept' }); + await engine.insert('detail', { parent: p1.id }); + await engine.insert('guard', { parent: p2.id }); + + await expect(engine.delete('parent', { where: { name: 'doomed' }, multi: true })) + .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, object: 'parent', dependentObject: 'guard' }); + + // p1's detail was cascaded before p2's restrict refused; the one unit of + // work puts it back, so the 409 means nothing changed. + expect(driver.rowsOf('parent')).toHaveLength(3); + expect(driver.rowsOf('detail')).toHaveLength(1); + expect(driver.rowsOf('guard')).toHaveLength(1); + expect(driver.begun).toHaveLength(1); + expect(driver.rolledBack).toHaveLength(1); + expect(driver.committed).toHaveLength(0); + }); + + it('cascades master_detail children and clears set_null references of every matched row', async () => { + const { engine, driver } = await makeEngine([parent, detail, link]); + const p1 = await engine.insert('parent', { name: 'doomed' }); + const p2 = await engine.insert('parent', { name: 'doomed' }); + const kept = await engine.insert('parent', { name: 'kept' }); + await engine.insert('detail', { parent: p1.id }); + await engine.insert('detail', { parent: p2.id }); + const keptDetail = await engine.insert('detail', { parent: kept.id }); + const l1 = await engine.insert('link', { parent: p1.id }); + const l2 = await engine.insert('link', { parent: p2.id }); + const keptLink = await engine.insert('link', { parent: kept.id }); + + const removed = await engine.delete('parent', { where: { name: 'doomed' }, multi: true }); + + expect(removed).toBe(2); + expect(driver.rowsOf('parent').map((r: any) => r.id)).toEqual([kept.id]); + expect(driver.rowsOf('detail').map((r: any) => r.id)).toEqual([keptDetail.id]); + const links = new Map(driver.rowsOf('link').map((r: any) => [r.id, r.parent])); + expect(links.get(l1.id)).toBeNull(); + expect(links.get(l2.id)).toBeNull(); + expect(links.get(keptLink.id)).toBe(kept.id); + expect(driver.begun).toHaveLength(1); + expect(driver.committed).toHaveLength(1); + }); + + it('routes the walk and the predicate delete onto ONE transaction handle', async () => { + const { engine, driver } = await makeEngine([parent, detail, link]); + const p = await engine.insert('parent', { name: 'doomed' }); + await engine.insert('detail', { parent: p.id }); + await engine.insert('link', { parent: p.id }); + driver.writes.length = 0; + + await engine.delete('parent', { where: { name: 'doomed' }, multi: true }); + + expect(driver.begun).toHaveLength(1); + const handle = driver.begun[0]; + expect(driver.writes.some((w: any) => w.object === 'parent' && w.op === 'delete')).toBe(true); + expect(driver.writes.some((w: any) => w.object === 'detail' && w.op === 'delete')).toBe(true); + expect(driver.writes.some((w: any) => w.object === 'link' && w.op === 'update')).toBe(true); + for (const w of driver.writes) expect(w.transaction).toBe(handle); + }); + + it('deletes only the rows it walked, even when the walk makes another row match the predicate', async () => { + // A self-referencing set_null lookup: clearing `boss` on the report makes + // it match `{ boss: null }`. The delete removes the rows it resolved and + // walked, never a row the walk itself brought into the predicate. + const node = { + name: 'node', + label: 'Node', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const }, + boss: { name: 'boss', type: 'lookup' as const, reference: 'node' }, + }, + }; + const { engine, driver } = await makeEngine([node]); + const root = await engine.insert('node', { name: 'root', boss: null }); + const report = await engine.insert('node', { name: 'report', boss: root.id }); + + const removed = await engine.delete('node', { where: { boss: null }, multi: true }); + + expect(removed).toBe(1); + expect(driver.rowsOf('node').map((r: any) => [r.id, r.boss ?? null])).toEqual([[report.id, null]]); + }); + + it('a detail that also restricts its own master refuses the predicate delete, as it does by id (#26718)', async () => { + // The predicate arm runs the by-id walk itself, so every rule the walk + // judges holds here too: the detail is in its master's cascade set by its + // own master_detail, and its restrict to that master still refuses. + const pledge = { + name: 'pledge', + label: 'Pledge', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + parent: { name: 'parent', type: 'master_detail' as const, reference: 'parent' }, + guard: { name: 'guard', type: 'lookup' as const, reference: 'parent', deleteBehavior: 'restrict' }, + }, + }; + const { engine, driver } = await makeEngine([parent, pledge]); + const p = await engine.insert('parent', { name: 'doomed' }); + await engine.insert('pledge', { parent: p.id, guard: p.id }); + + await expect(engine.delete('parent', { where: { name: 'doomed' }, multi: true })) + .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: 'pledge' }); + expect(driver.rowsOf('parent')).toHaveLength(1); + expect(driver.rowsOf('pledge')).toHaveLength(1); + }); + + it('a matched set of zero rows walks nothing and deletes nothing', async () => { + const { engine, driver } = await makeEngine([parent, detail]); + const p = await engine.insert('parent', { name: 'kept' }); + await engine.insert('detail', { parent: p.id }); + driver.writes.length = 0; + + const removed = await engine.delete('parent', { where: { name: 'nobody' }, multi: true }); + + expect(removed).toBe(0); + expect(driver.writes).toEqual([]); + expect(driver.rowsOf('detail')).toHaveLength(1); }); }); diff --git a/packages/objectql/src/engine-nested-relation-lowering.test.ts b/packages/objectql/src/engine-nested-relation-lowering.test.ts index b8c300a2716..c9b4e32e62a 100644 --- a/packages/objectql/src/engine-nested-relation-lowering.test.ts +++ b/packages/objectql/src/engine-nested-relation-lowering.test.ts @@ -203,15 +203,21 @@ describe('[#20802] the nested-relation form is lowered at the engine\'s where se expect(outer.length, verb).toBeGreaterThan(0); for (const read of outer) expect(read.ast.where, verb).toEqual(lowered); } - for (const [verb, call] of [ - ['update', () => engine.update(OBJECT, { title: 'x' }, { where, multi: true })], - ['delete', () => engine.delete(OBJECT, { where, multi: true })], - ] as const) { - rec.writes.length = 0; - await call(); - expect(rec.writes, verb).toHaveLength(1); - expect(rec.writes[0].ast.where, verb).toEqual(lowered); - } + rec.writes.length = 0; + await engine.update(OBJECT, { title: 'x' }, { where, multi: true }); + expect(rec.writes, 'update').toHaveLength(1); + expect(rec.writes[0].ast.where, 'update').toEqual(lowered); + // [#26871] Relations point at this ledger (the owner's `account`, its own + // `parent` tree), so a predicate delete first reads the rows it will walk + // and then deletes those rows by id. The predicate reaches the driver on + // that read, lowered; nothing matches here, so nothing is written. + rec.reads.length = 0; + rec.writes.length = 0; + await engine.delete(OBJECT, { where, multi: true }); + const resolution = rec.reads.filter((r) => r.object === OBJECT && Array.isArray(r.ast?.fields) && r.ast.fields.includes('id')); + expect(resolution, 'delete').toHaveLength(1); + expect(resolution[0].ast.where, 'delete').toEqual(lowered); + expect(rec.writes, 'delete').toHaveLength(0); expect(engine.judgeFilter(OBJECT, where)).toEqual({ ok: true }); expect(engine.judgeFilter(OBJECT, { owners: { region: 'NA' } })).toEqual({ ok: true }); }); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index fb4acc2ce16..76a32537e60 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -789,9 +789,12 @@ export function writeFailureLogMeta( /** * [#24948] The refusal of a predicate (`multi`) delete of `sys_organization`: - * the standard `DELETE_RESTRICTED`, saying to delete by id. Only the by-id - * delete runs the referential pass that refuses an organization still owning - * rows and keeps every row's reference to it. + * the standard `DELETE_RESTRICTED`, saying to delete by id. An organization is + * deleted one at a time, through plugin-auth's organization delete: its + * refusal of an organization that still owns records and its + * `beforeDeleteOrganization` clean-up of the organization's teams, team members + * and admin grants run there, once per organization. A delete matching + * organizations by a filter runs neither, and no caller needs one. */ function organizationPredicateDeleteRefusal(): Error { const err: any = new Error( @@ -799,9 +802,10 @@ function organizationPredicateDeleteRefusal(): Error { 'Delete each organization by its id.', ); err.developerMessage = - `A predicate delete of ${ORGANIZATION_OBJECT} is refused: it runs no referential pass, so it could neither ` + - 'refuse an organization that still owns rows nor keep the rows naming it. Delete each organization by id ' + - `(engine.delete('${ORGANIZATION_OBJECT}', { where: { id } })).`; + `A predicate delete of ${ORGANIZATION_OBJECT} is refused: organizations are deleted one at a time, because ` + + "plugin-auth's organization delete refuses an organization that still owns records and removes its teams, " + + 'team members and admin grants per organization, and a predicate delete runs neither. Delete each organization ' + + `by id (engine.delete('${ORGANIZATION_OBJECT}', { where: { id } })).`; err.code = 'DELETE_RESTRICTED'; err.status = 409; err.httpStatus = 409; @@ -18240,8 +18244,9 @@ export class ObjectQL implements IObjectQLEngine { * required lookup that escalation is decided per ROW after the dependents * probe (#9688): a row that keeps another member takes the member removal, * a row the removal would EMPTY keeps the refusal, and the refusal counts - * only those rows. Only runs for single-id deletes — multi/predicate - * deletes skip cascade (logged). + * only those rows. A by-id delete runs it for its one record; a predicate + * delete (`multi: true`) runs it for every row it matched ([#26871], see + * `delete()`'s predicate arm). * * [#22305] Two phases, so the walk knows its own set before it judges * anything: @@ -19645,11 +19650,11 @@ export class ObjectQL implements IObjectQLEngine { // update twin says why; for every pre-#11009 input the two spellings // agree. const isPredicatePath = !isByIdDelete && dispatch.kind === 'multi' && typeof driver.deleteMany === 'function'; - // [#24948] An organization is deleted by id or not at all. The predicate - // arm runs no referential cascade, so it could neither refuse an - // organization that still owns rows nor keep a single row from naming a - // deleted one; any delete not by id is refused here, whatever the driver - // offers, before any hook or write. + // [#24948] An organization is deleted by id or not at all: plugin-auth's + // organization delete refuses one that still owns records and removes its + // auth structure per organization, which a predicate delete would skip + // (see `organizationPredicateDeleteRefusal`). Any delete not by id is + // refused here, whatever the driver offers, before any hook or write. if (!isByIdDelete && object === ORGANIZATION_OBJECT) { throw organizationPredicateDeleteRefusal(); } @@ -19857,7 +19862,64 @@ export class ObjectQL implements IObjectQLEngine { // [#2982] The AST asserted present and already used for the // pre-phase row read above. // `deleteMany` presence is part of the ladder verdict resolved above. - result = await driver.deleteMany!(object, opCtx.ast!, hookContext.input.options as any); + // + // [#26871] A predicate delete honours `deleteBehavior` as the + // by-id branch does: a `restrict` refuses, a `cascade` (a + // `master_detail` by default) deletes the dependents, a + // `set_null` clears them. This arm used to go straight to + // `deleteMany`, so a flow's `delete_record` with `multi: true` + // skipped every declared rule the same delete done record by + // record honours. + // + // `planCascadeAtomicity` is the by-id branch's own question: + // `'none'` means no relation of the delete family points at + // this object, so the walk could write nothing and the arm keeps + // its single `deleteMany`, no rows read and no transaction. + const plan = this.planCascadeAtomicity(object); + if (plan === 'none') { + result = await driver.deleteMany!(object, opCtx.ast!, hookContext.input.options as any); + } else { + // Otherwise: resolve the matched rows through the composed, + // row-scoped AST, run `cascadeDeleteRelations` for each, then + // delete exactly those rows by id. By id, not by re-running the + // predicate: the walk's own writes (a `set_null` on a + // self-reference) can move a row into or out of the predicate, + // and the rows deleted must be the rows walked. The ids were + // read under this caller's scope inside the same unit of work, + // as the by-id branch deletes its id after reading its + // pre-image under that scope. The unit of work is the by-id + // branch's, for the same reasons (#7413). + const runPredicateDelete = async (writeContext?: ExecutionContext): Promise => { + const writeOptions = this.buildDriverOptions(object, writeContext, hookContext.input.options as any); + const matched = ((await driver.find(object, { ...opCtx.ast!, fields: ['id'] }, writeOptions)) ?? []) as Array>; + const matchedIds = matched.map((row) => row?.id).filter((v): v is string | number => v != null); + for (const matchedId of matchedIds) { + await this.cascadeDeleteRelations(object, matchedId, writeContext); + } + // The id set rides an `$in`; batched so one statement never + // outgrows a driver's bind-parameter limit. + const batchSize = 500; + let removed = 0; + for (let i = 0; i < matchedIds.length; i += batchSize) { + const batchAst: QueryAST = { object, where: { id: { $in: matchedIds.slice(i, i + batchSize) } } }; + const count = await driver.deleteMany!(object, batchAst, writeOptions); + // A non-count answer goes back as the driver gave it, so + // the `isDeleteResultShape` check below refuses it. + if (typeof count !== 'number') return count; + removed += count; + } + return removed; + }; + if (plan === 'atomic') { + result = await this.transaction( + async (trxCtx: any) => await runPredicateDelete(trxCtx as ExecutionContext), + opCtx.context, + ); + } else { + this.warnCascadeNotAtomic(object); + result = await runPredicateDelete(opCtx.context); + } + } isPredicateWrite = true; } diff --git a/packages/objectql/src/lifecycle/lifecycle-service.ts b/packages/objectql/src/lifecycle/lifecycle-service.ts index d42236c4920..d2f8fc875b5 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.ts @@ -1789,9 +1789,10 @@ export class LifecycleService { }); if (confirmed.length === 0) break; } - // Per-id deletes (NOT a `{$in}` filter): the engine's delete path reads - // `where.id` as a scalar target, and by-id deletes get referential - // cascade handling a filter-delete would bypass. + // Per-id deletes (NOT a `{$in}` filter): the engine reads a scalar + // `where.id` as a by-id target even beside `multi: true`, so each + // confirmed row is deleted on its own, with its own pre-image read, + // hooks and referential walk. for (const row of confirmed) { await engine.delete(object, { where: { id: row.id }, diff --git a/packages/plugins/plugin-auth/src/organization-delete.ts b/packages/plugins/plugin-auth/src/organization-delete.ts index c46c1e398a9..936ba5f2641 100644 --- a/packages/plugins/plugin-auth/src/organization-delete.ts +++ b/packages/plugins/plugin-auth/src/organization-delete.ts @@ -262,9 +262,10 @@ export async function isDefaultOrganizationTarget( * request's transaction, so a later failure rolls it back. * * Team members first, by their team: `sys_team_member` reaches the - * organization only through `team_id`, and a multi-row delete runs no cascade. - * Then each team by id, so the engine's cascade clears what still points at it - * (a session's active team). The grants go explicitly, here, rather than + * organization only through `team_id`, a required lookup, so a team still + * holding members would refuse its own delete (a required reference is + * restricted, never cleared). Then each team by id, so the engine's cascade + * clears what still points at it (a session's active team). The grants go explicitly, here, rather than * waiting on plugin-security's member-delete reconcile: measured on a booted * showcase, they were still present when the organization row was deleted. */ diff --git a/packages/services/service-automation/src/builtin/delete-record-predicate-referential-walk.integration.test.ts b/packages/services/service-automation/src/builtin/delete-record-predicate-referential-walk.integration.test.ts new file mode 100644 index 00000000000..52376cba29c --- /dev/null +++ b/packages/services/service-automation/src/builtin/delete-record-predicate-referential-walk.integration.test.ts @@ -0,0 +1,198 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A `delete_record` node with `multi: true` and a filter honours every + * declared `deleteBehavior` of the rows it matches: a `restrict` refuses the + * whole delete, a `master_detail` child is cascaded, a `set_null` lookup is + * cleared. + * + * The node forwards the author's bulk intent to `data.delete({ where, multi: + * true })`. The engine's predicate arm used to go straight to + * `driver.deleteMany`, so a flow deleting parents by filter skipped the + * referential walk the by-id delete runs: no restrict refused, master_detail + * children and lookups were left pointing at deleted rows. The same delete + * done record by record through the data door refused or cascaded. The arm + * now resolves the rows it matches and runs the by-id walk for each, in one + * unit of work. + * + * Composition: the real stack the other `*.integration.test.ts` files in this + * package boot: `ObjectKernel`, `ObjectQLPlugin`, `driver-sql` on + * better-sqlite3 `:memory:`, and the real `AutomationServicePlugin` running + * the flow. Rows are counted out of the DRIVER, not through the engine. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import type { FilterCondition } from '@objectstack/spec/data'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQLPlugin, type ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { AutomationServicePlugin } from '../plugin.js'; +import { AutomationEngine } from '../engine.js'; +import { registerCrudNodes } from './crud-nodes.js'; +import { installDefaultOrganization } from '../default-organization.testkit.js'; + +const SYS = { isSystem: true } as const; + +const PARENT = { + name: 'pin_bulk_parent', + label: 'Pin bulk parent', + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + grp: { name: 'grp', label: 'Group', type: 'text' }, + }, +}; +/** A detail: `master_detail` cascades by default. */ +const DETAIL = { + name: 'pin_bulk_detail', + label: 'Pin bulk detail', + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + parent: { name: 'parent', label: 'Parent', type: 'master_detail', reference: 'pin_bulk_parent' }, + }, +}; +/** An authored `restrict` lookup. */ +const GUARD = { + name: 'pin_bulk_guard', + label: 'Pin bulk guard', + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + parent: { name: 'parent', label: 'Parent', type: 'lookup', reference: 'pin_bulk_parent', deleteBehavior: 'restrict' }, + }, +}; +/** An optional `lookup`: `set_null` by default. */ +const LINK = { + name: 'pin_bulk_link', + label: 'Pin bulk link', + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + parent: { name: 'parent', label: 'Parent', type: 'lookup', reference: 'pin_bulk_parent' }, + }, +}; + +function makeLogger(): any { + const l: any = { info() {}, warn() {}, error() {}, debug() {}, trace() {}, fatal() {} }; + l.child = () => l; + return l; +} + +type Executor = { execute: (node: any, variables: Map, context: any) => Promise }; + +describe('[#26871] a multi delete_record runs the referential walk for every row its filter matches', () => { + let kernel: ObjectKernel; + let ql: ObjectQL; + let driver: SqlDriver; + let automation: AutomationEngine; + let deleteRecord: Executor; + + beforeAll(async () => { + kernel = new ObjectKernel({ logger: { level: 'fatal' } }); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(new AutomationServicePlugin({ suspendedRunStore: 'memory' })); + await kernel.bootstrap(); + ql = kernel.getService('objectql'); + automation = kernel.getService('automation'); + + driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + await driver.connect(); + ql.registerDriver(driver, true); + for (const o of [PARENT, DETAIL, GUARD, LINK]) ql.registry.registerObject(o as any, 'pin-bulk-delete', 'pin-bulk-delete'); + await ql.syncSchemas(); + await installDefaultOrganization(ql); + + // The registered executor itself, over the same engine, so the refusal's + // structured half can be read off the node's own answer. + const executors = new Map(); + const capture = new Proxy(new AutomationEngine(makeLogger()), { + get(target, key, receiver) { + if (key === 'registerNodeExecutor') return (e: any) => { executors.set(e.type, e); }; + return Reflect.get(target, key, receiver); + }, + }); + registerCrudNodes(capture as AutomationEngine, { logger: makeLogger(), getService: (n: string) => (n === 'data' ? ql : undefined) } as any); + deleteRecord = executors.get('delete_record')!; + expect(deleteRecord, 'control: the delete_record executor was registered').toBeDefined(); + }, 60_000); + + afterAll(async () => { + try { await kernel?.shutdown(); } catch { /* best-effort teardown */ } + }); + + const insert = async (object: string, data: Record): Promise => + ((await ql.insert(object, data, { context: SYS })) as any).id; + /** Rows of `object`, read from the driver with no engine in between. */ + const rows = async (object: string, where: FilterCondition = {}): Promise => + (await driver.find(object, { where })) as any[]; + + function bulkDeleteFlow(name: string, grp: string) { + return { + name, label: name, type: 'autolaunched', runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'sweep', type: 'delete_record', label: 'Sweep', config: { objectName: PARENT.name, filter: { grp }, multi: true } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'sweep' }, + { id: 'e2', source: 'sweep', target: 'end' }, + ], + }; + } + + it('a restrict held on one matched parent refuses the whole delete, and nothing is deleted', async () => { + const p1 = await insert(PARENT.name, { title: 'h1', grp: 'held' }); + const p2 = await insert(PARENT.name, { title: 'h2', grp: 'held' }); + await insert(DETAIL.name, { title: 'd-h1', parent: p1 }); + await insert(LINK.name, { title: 'l-h1', parent: p1 }); + await insert(GUARD.name, { title: 'g-h2', parent: p2 }); + + const flow = bulkDeleteFlow('pin_bulk_delete_held', 'held'); + automation.registerFlow(flow.name, flow as any); + const res: any = await automation.execute(flow.name, {}); + expect(res.success, 'the run fails on the restrict').toBe(false); + + // The node's own answer carries the engine's refusal. + const node: any = await deleteRecord.execute( + { id: 'sweep', type: 'delete_record', config: { objectName: PARENT.name, filter: { grp: 'held' }, multi: true } }, + new Map(), + { runAs: 'system', flowName: flow.name }, + ); + expect(node.success).toBe(false); + expect(node.innerRefusal).toMatchObject({ code: 'DELETE_RESTRICTED' }); + + // Nothing changed: both parents, the detail the walk reached first, and + // the lookup it cleared are all as they were. + expect((await rows(PARENT.name, { grp: 'held' })).map((r) => r.id).sort()).toEqual([p1, p2].sort()); + expect(await rows(DETAIL.name, { parent: p1 })).toHaveLength(1); + expect(await rows(LINK.name, { parent: p1 })).toHaveLength(1); + expect(await rows(GUARD.name, { parent: p2 })).toHaveLength(1); + }); + + it('cascades master_detail children and clears set_null lookups of every matched parent', async () => { + const p1 = await insert(PARENT.name, { title: 'x1', grp: 'doomed' }); + const p2 = await insert(PARENT.name, { title: 'x2', grp: 'doomed' }); + const kept = await insert(PARENT.name, { title: 'k', grp: 'kept' }); + await insert(DETAIL.name, { title: 'd-x1', parent: p1 }); + await insert(DETAIL.name, { title: 'd-x2', parent: p2 }); + const keptDetail = await insert(DETAIL.name, { title: 'd-k', parent: kept }); + const l1 = await insert(LINK.name, { title: 'l-x1', parent: p1 }); + const l2 = await insert(LINK.name, { title: 'l-x2', parent: p2 }); + const keptLink = await insert(LINK.name, { title: 'l-k', parent: kept }); + + const flow = bulkDeleteFlow('pin_bulk_delete_doomed', 'doomed'); + automation.registerFlow(flow.name, flow as any); + const res: any = await automation.execute(flow.name, {}); + expect(res.success, `run failed: ${JSON.stringify(res.error ?? res)}`).toBe(true); + + expect(await rows(PARENT.name, { grp: 'doomed' })).toEqual([]); + expect((await rows(PARENT.name, { grp: 'kept' })).map((r) => r.id)).toEqual([kept]); + // master_detail: the matched parents' details are gone, the kept one stays. + expect(await rows(DETAIL.name, { parent: p1 })).toEqual([]); + expect(await rows(DETAIL.name, { parent: p2 })).toEqual([]); + expect((await rows(DETAIL.name, { parent: kept })).map((r) => r.id)).toEqual([keptDetail]); + // set_null: the matched parents' lookups are cleared, not left dangling. + const links = new Map((await rows(LINK.name)).map((r) => [r.id, r.parent ?? null])); + expect(links.get(l1)).toBeNull(); + expect(links.get(l2)).toBeNull(); + expect(links.get(keptLink)).toBe(kept); + }); +});