From 38b80788e0ad1e30d0e4fc8a1c871f4c61d63c00 Mon Sep 17 00:00:00 2001 From: Abraham Ingersoll <586805+aberoham@users.noreply.github.com> Date: Sat, 29 Aug 2026 19:53:24 +0100 Subject: [PATCH] lib: one cascade graph for every store Which children a hard delete takes with it was decided separately in each backend. mysql has each of them from a FOREIGN KEY; the file stores have no constraints and never implemented them, so destroying a group left its nameservers behind, destroying a user left its sessions, and destroying a zone left its records. references.js names those three edges once. The file stores call cascade() from destroy(), mysql goes on getting it from the constraints, and references.test.js holds whichever store is configured to the same graph. Each case seeds a second parent whose child must survive, so a store that takes too much fails as loudly as one that takes too little. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014AUjj218gbhUHLi7kvDPtd --- lib/group/store/file.js | 3 + lib/references.js | 35 ++++++++ lib/references.test.js | 181 ++++++++++++++++++++++++++++++++++++++++ lib/user/store/file.js | 3 + lib/zone/store/file.js | 3 + 5 files changed, 225 insertions(+) create mode 100644 lib/references.js create mode 100644 lib/references.test.js diff --git a/lib/group/store/file.js b/lib/group/store/file.js index a5044c4..a5fd00b 100644 --- a/lib/group/store/file.js +++ b/lib/group/store/file.js @@ -2,6 +2,8 @@ import FileStore from '../../store/file.js' import GroupBase from './base.js' +import { cascade } from '../../references.js' + const defaultPermissions = { inherit: false, self_write: false, @@ -159,6 +161,7 @@ class GroupRepoFile extends GroupBase { const before = groups.length const filtered = groups.filter((g) => g.id !== args.id) if (filtered.length === before) return false + await cascade('group', args.id) await this._save(filtered) return true } diff --git a/lib/references.js b/lib/references.js new file mode 100644 index 0000000..2748d81 --- /dev/null +++ b/lib/references.js @@ -0,0 +1,35 @@ +// What a hard delete takes with it. mysql enforces each of these with a +// FOREIGN KEY; the file stores have none, so their destroy() calls cascade(). +const cascades = [ + { parent: 'group', child: 'nameserver', via: 'gid' }, + { parent: 'user', child: 'session', via: 'uid' }, + { parent: 'zone', child: 'zone_record', via: 'zid' }, +] + +// Imported when a cascade runs, so a child store loading its own parent does +// not close a circle at module load. +const childStores = { + nameserver: () => import('./nameserver/index.js'), + session: () => import('./session/index.js'), + zone_record: () => import('./zone_record/index.js'), +} + +export async function cascade(parent, id) { + for (const edge of cascades.filter((e) => e.parent === parent)) { + const store = (await childStores[edge.child]()).default + + // Session alone has no destroy(); its delete() is the hard one and clears + // every session for the parent in a single call. + if (typeof store.destroy !== 'function') { + await store.delete({ [edge.via]: id }) + continue + } + + for (const deleted of [false, true]) { + const found = await store.get({ [edge.via]: id, deleted }) + for (const child of [found].flat().filter(Boolean)) await store.destroy({ id: child.id }) + } + } +} + +export default cascades diff --git a/lib/references.test.js b/lib/references.test.js new file mode 100644 index 0000000..1d40262 --- /dev/null +++ b/lib/references.test.js @@ -0,0 +1,181 @@ +import assert from 'node:assert/strict' +import { after, before, describe, it } from 'node:test' + +import { storeType } from './config.js' +import cascades from './references.js' + +import Group from './group/index.js' +import Nameserver from './nameserver/index.js' +import Session from './session/index.js' +import User from './user/index.js' +import Zone from './zone/index.js' +import ZoneRecord from './zone_record/index.js' + +const GID = 47101 +const ids = { + group: [47101, 47111, 47121], + user: [47102, 47112, 47122], + zone: [47103, 47113, 47123], + nameserver: [47104, 47114, 47124], + session: [47105, 47115, 47125], + zone_record: [47106, 47116, 47126], +} + +const entities = { + group: { store: Group, body: (id) => ({ id, parent_gid: 0, name: `g${id}.refgraph.test` }) }, + user: { + store: User, + body: (id) => ({ + id, + gid: GID, + username: `refgraph${id}`, + email: `refgraph${id}@example.com`, + password: 'Wh@tA-Decent#P6ssw0rd', + first_name: 'Ref', + last_name: 'Graph', + is_admin: false, + }), + }, + zone: { + store: Zone, + body: (id) => ({ + id, + gid: GID, + zone: `z${id}.refgraph.test`, + mailaddr: `hostmaster.z${id}.refgraph.test.`, + serial: 1, + refresh: 1, + retry: 2, + expire: 3, + minimum: 4, + ttl: 3600, + }), + }, + nameserver: { + store: Nameserver, + body: (id) => ({ id, name: `ns${id}.refgraph.test.`, address: '203.0.113.10', type: 'nsd', ttl: 3600 }), + }, + session: { store: Session, body: (id) => ({ id, session: `refgraph-${id}`, last_access: 1700000000 }) }, + zone_record: { + store: ZoneRecord, + body: (id) => ({ id, owner: `r${id}.refgraph.test.`, type: 'A', address: '203.0.113.9', ttl: 3600 }), + }, +} + +const softDeletes = (name) => typeof entities[name].store.destroy === 'function' +const rows = (found) => [found].flat().filter(Boolean).length + +async function present(name, id) { + const { store } = entities[name] + if (!softDeletes(name)) return rows(await store.get({ id })) + let n = 0 + for (const deleted of [false, true]) n += rows(await store.get({ id, deleted })) + return n +} + +async function remove(name, id) { + const { store } = entities[name] + return softDeletes(name) ? store.destroy({ id }) : store.delete({ id }) +} + +// A store may allocate its own id, so seed() reports the one it used. +async function seed(name, body) { + const returned = await entities[name].store.create(body) + return Number.isInteger(returned) ? returned : body.id +} + +async function reset() { + for (const uid of ids.user) await Session.delete({ uid }) + for (const name of ['zone_record', 'session', 'zone', 'nameserver', 'user', 'group']) { + for (const id of ids[name]) await remove(name, id) + } +} + +before(reset) + +after(async () => { + await reset() + for (const { store } of Object.values(entities)) await store.disconnect?.() +}) + +describe('cascade graph', () => { + it('declares the edges the constraints enforce', () => { + assert.deepEqual( + cascades.map((e) => `${e.parent} -> ${e.child}`).sort(), + ['group -> nameserver', 'user -> session', 'zone -> zone_record'], + 'an edge was added or dropped; the file stores follow this list', + ) + for (const edge of cascades) { + assert.ok(entities[edge.parent], `no fixture for parent ${edge.parent}`) + assert.ok(entities[edge.child], `no fixture for child ${edge.child}`) + assert.ok(edge.via, `${edge.parent} -> ${edge.child} declares no via`) + } + }) + + for (const edge of cascades) { + it(`destroying a ${edge.parent} takes its ${edge.child} rows (${edge.via})`, async () => { + await reset() + await seed('group', entities.group.body(GID)) + + const [doomed, spared] = ids[edge.parent] + if (edge.parent === 'group') await seed('group', entities.group.body(spared)) + else for (const id of [doomed, spared]) await seed(edge.parent, entities[edge.parent].body(id)) + + const [first, other, extra] = ids[edge.child] + const child = await seed(edge.child, { ...entities[edge.child].body(first), [edge.via]: doomed }) + const sibling = await seed(edge.child, { ...entities[edge.child].body(extra), [edge.via]: doomed }) + const control = await seed(edge.child, { ...entities[edge.child].body(other), [edge.via]: spared }) + + if (softDeletes(edge.child)) await entities[edge.child].store.delete({ id: sibling }) + + for (const [id, what] of [ + [child, 'child'], + [sibling, 'sibling'], + [control, 'control'], + ]) { + assert.equal(await present(edge.child, id), 1, `the ${what} did not seed`) + } + + assert.ok(await remove(edge.parent, doomed), `destroying the ${edge.parent} reported no change`) + assert.equal(await present(edge.parent, doomed), 0, `the ${edge.parent} outlived its own destroy`) + + assert.equal(await present(edge.child, child), 0, `the ${edge.child} outlived its ${edge.parent}`) + assert.equal(await present(edge.child, sibling), 0, `a soft-deleted ${edge.child} was left behind`) + assert.equal(await present(edge.child, control), 1, `an unrelated ${edge.child} was taken too`) + + // mysql reads a session through its user, so a surviving row would read + // as absent. Deleting it again reports whether it is really gone. + for (const id of [child, sibling]) { + assert.equal(await remove(edge.child, id), false, `a ${edge.child} row was still there to delete`) + } + }) + } + + it('leaves the parent in place when a cascade fails', async () => { + await reset() + await seed('group', entities.group.body(GID)) + const zid = ids.zone[0] + await seed('zone', entities.zone.body(zid)) + const rec = await seed('zone_record', { ...entities.zone_record.body(ids.zone_record[0]), zid }) + + if (storeType() === 'mysql') { + assert.ok(await Zone.destroy({ id: zid }), 'the zone did not destroy') + assert.equal(await present('zone_record', rec), 0, 'the constraint left the record') + return + } + + const original = ZoneRecord.get.bind(ZoneRecord) + ZoneRecord.get = async () => { + throw new Error('cascade probe') + } + try { + await assert.rejects(() => Zone.destroy({ id: zid }), /cascade probe/) + } finally { + ZoneRecord.get = original + } + + assert.equal(await present('zone', zid), 1, 'the zone went while its records stayed') + assert.ok(await Zone.destroy({ id: zid }), 'the retry found no zone to destroy') + assert.equal(await present('zone_record', rec), 0, 'the record outlived the retry') + }) +}) diff --git a/lib/user/store/file.js b/lib/user/store/file.js index 4ee232e..7edb511 100644 --- a/lib/user/store/file.js +++ b/lib/user/store/file.js @@ -3,6 +3,8 @@ import Config from '../../config.js' import Credentials from '../credentials.js' import UserBase from './base.js' +import { cascade } from '../../references.js' + const boolFields = ['is_admin', 'deleted'] // Read the group file directly (never via the Group module) to avoid circular imports. @@ -206,6 +208,7 @@ class UserRepoFile extends UserBase { const before = users.length const filtered = users.filter((u) => u.id !== args.id) if (filtered.length === before) return false + await cascade('user', args.id) await this._save(filtered) return true } diff --git a/lib/zone/store/file.js b/lib/zone/store/file.js index 2db3eed..02ea7f2 100644 --- a/lib/zone/store/file.js +++ b/lib/zone/store/file.js @@ -2,6 +2,8 @@ import FileStore, { resolveCodec } from '../../store/file.js' import ZoneBase from './base.js' +import { cascade } from '../../references.js' + const zoneDefaults = { minimum: 3600, ttl: 3600, refresh: 86400, retry: 7200, expire: 1209600 } class ZoneRepoFile extends ZoneBase { @@ -161,6 +163,7 @@ class ZoneRepoFile extends ZoneBase { const before = zones.length const filtered = zones.filter((z) => z.id !== args.id) if (filtered.length === before) return false + await cascade('zone', args.id) await this._save(filtered) return true }