From 8c12c04c8bcf91b006578372fc9f399ba448d719 Mon Sep 17 00:00:00 2001 From: HafizMMoaz Date: Tue, 28 Jul 2026 08:55:36 +0500 Subject: [PATCH] fix: resolve dependents against every deleted resource Deleting a resource with `_dependent` worked by nullifying foreign keys and then removing the rows whose key was null. That has two problems. Rows that already held a null foreign key before the request were indistinguishable from rows nullified by it, so deleting one post removed every comment that was never attached to a post in the first place. The sweep also used the deleted resource's own foreign key for all the dependents, so only direct children could ever match. Deleting a village with `?_dependent=houses&_dependent=citizens` looked for `villageId` on the citizens, found none, and left them behind pointing at houses that no longer exist. Dependents are now matched by id against every resource deleted so far, repeating until nothing new is removed, so a chain of dependents is resolved whatever order it is listed in. Foreign keys are nullified afterwards and cover rows pointing at deleted dependents too. Dependents still have to be named explicitly, so nothing the caller did not ask for is deleted. Fixes #1415 --- src/service.test.ts | 59 ++++++++++++++++++++++++++++++++++++ src/service.ts | 73 +++++++++++++++++++++++++++++++++------------ 2 files changed, 113 insertions(+), 19 deletions(-) diff --git a/src/service.test.ts b/src/service.test.ts index d89e0654e..7a367c327 100644 --- a/src/service.test.ts +++ b/src/service.test.ts @@ -173,6 +173,65 @@ await test('destroy', async (t) => { assert.equal(db.data[COMMENTS].length, 0) }) + await t.test('keeps items whose foreign key was already null', async () => { + db.data = { + posts: [post1, post2], + comments: [ + { id: '1', postId: post1.id }, + { id: '2', postId: null }, + { id: '3', postId: post2.id }, + ], + } + + await service.destroyById(POSTS, post1.id, [COMMENTS]) + + assert.deepEqual(db.data[COMMENTS], [ + { id: '2', postId: null }, + { id: '3', postId: post2.id }, + ]) + }) + + await t.test('deletes dependents of dependents', async () => { + const remaining = { + villages: [{ id: '2' }], + houses: [{ id: '2', villageId: '2' }], + citizens: [{ id: '2', houseId: '2' }], + } + + for (const dependents of [ + ['houses', 'citizens'], + ['citizens', 'houses'], + ]) { + db.data = { + villages: [{ id: '1' }, { id: '2' }], + houses: [ + { id: '1', villageId: '1' }, + { id: '2', villageId: '2' }, + ], + citizens: [ + { id: '1', houseId: '1' }, + { id: '2', houseId: '2' }, + ], + } + + await service.destroyById('villages', '1', dependents) + + assert.deepEqual(db.data, remaining, `dependents listed as ${dependents.join(',')}`) + } + }) + + await t.test('nullifies foreign keys pointing at deleted dependents', async () => { + db.data = { + villages: [{ id: '1' }], + houses: [{ id: '1', villageId: '1' }], + pets: [{ id: '1', houseId: '1' }], + } + + await service.destroyById('villages', '1', ['houses']) + + assert.deepEqual(db.data['pets'], [{ id: '1', houseId: null }]) + }) + await t.test('ignores unknown resources', async () => { assert.equal(await service.destroyById(UNKNOWN_RESOURCE, post1.id), undefined) assert.equal(await service.destroyById(POSTS, UNKNOWN_ID), undefined) diff --git a/src/service.ts b/src/service.ts index ad06bfa8f..77693c403 100644 --- a/src/service.ts +++ b/src/service.ts @@ -46,17 +46,36 @@ function embed(db: Low, name: string, item: Item, related: string): Item { return { ...item, [related]: relatedItems } } -function nullifyForeignKey(db: Low, name: string, id: string) { - const foreignKey = `${inflection.singularize(name)}Id` +function foreignKeyOf(name: string): string { + return `${inflection.singularize(name)}Id` +} + +// Ids removed from each resource, so that a dependent can be matched against +// every resource deleted so far and not just against the requested one +type DeletedIds = Map> + +function isDependentOf(item: Item, deleted: DeletedIds): boolean { + for (const [name, ids] of deleted) { + const value = item[foreignKeyOf(name)] + if (typeof value === 'string' && ids.has(value)) return true + } + + return false +} +function nullifyForeignKeys(db: Low, deleted: DeletedIds) { Object.entries(db.data).forEach(([key, items]) => { - // Skip - if (key === name) return + if (!Array.isArray(items)) return + + for (const [name, ids] of deleted) { + // Skip + if (key === name) continue - // Nullify - if (Array.isArray(items)) { + // Nullify + const foreignKey = foreignKeyOf(name) items.forEach((item) => { - if (item[foreignKey] === id) { + const value = item[foreignKey] + if (typeof value === 'string' && ids.has(value)) { item[foreignKey] = null } }) @@ -64,18 +83,34 @@ function nullifyForeignKey(db: Low, name: string, id: string) { }) } -function deleteDependents(db: Low, name: string, dependents: string[]) { - const foreignKey = `${inflection.singularize(name)}Id` +function deleteDependents(db: Low, name: string, deleted: DeletedIds, dependents: string[]) { + // A dependent can itself be the parent of another one, so keep going until + // nothing new is deleted. A chain can be at most as deep as the number of + // dependents, and this makes the result independent of the order they are + // listed in. + for (let pass = 0; pass < dependents.length; pass++) { + let deletedAny = false - Object.entries(db.data).forEach(([key, items]) => { - // Skip - if (key === name || !dependents.includes(key)) return + for (const dependent of dependents) { + const items = db.data[dependent] + + // Skip + if (dependent === name || !Array.isArray(items)) continue + + // Delete items related to an already deleted one + const ids = deleted.get(dependent) ?? new Set() + db.data[dependent] = items.filter((item) => { + if (!isDependentOf(item, deleted)) return true - // Delete if foreign key is null - if (Array.isArray(items)) { - db.data[key] = items.filter((item) => item[foreignKey] !== null) + deletedAny = true + if (typeof item['id'] === 'string') ids.add(item['id']) + return false + }) + deleted.set(dependent, ids) } - }) + + if (!deletedAny) break + } } export class Service { @@ -212,9 +247,9 @@ export class Service { const index = items.indexOf(item) items.splice(index, 1) - nullifyForeignKey(this.#db, name, id) - const dependents = ensureArray(dependent) - deleteDependents(this.#db, name, dependents) + const deleted: DeletedIds = new Map([[name, new Set([id])]]) + deleteDependents(this.#db, name, deleted, ensureArray(dependent)) + nullifyForeignKeys(this.#db, deleted) await this.#db.write() return item