From 6bd775eb841543580975ff8ee99f6948a812a274 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Tue, 12 May 2026 20:31:12 -0400 Subject: [PATCH] fix: Batch document deletes when emptying trash (#12328) * fix: Batch document deletes when emptying trash Splits the final parentDocumentId clear and destroy in documentPermanentDeleter into chunks of 100 to keep the exclusive lock window on the documents table short, preventing concurrent web SELECTs from queueing behind a single large DELETE. * fix: Skip parentDocumentId clear for documents restored mid-flight Re-checks deletedAt in the database before clearing parentDocumentId on children, so a parent restored between the caller's query and now keeps its children attached. --- .../commands/documentPermanentDeleter.test.ts | 21 +++++++ server/commands/documentPermanentDeleter.ts | 60 +++++++++++++------ 2 files changed, 64 insertions(+), 17 deletions(-) diff --git a/server/commands/documentPermanentDeleter.test.ts b/server/commands/documentPermanentDeleter.test.ts index 08eb7166bf..eaa13ccb3a 100644 --- a/server/commands/documentPermanentDeleter.test.ts +++ b/server/commands/documentPermanentDeleter.test.ts @@ -132,6 +132,27 @@ describe("documentPermanentDeleter", () => { ).toEqual(1); }); + it("should not detach children of a document restored between query and destroy", async () => { + const parent = await buildDocument({ + publishedAt: subDays(new Date(), 90), + deletedAt: subDays(new Date(), 60), + }); + const child = await buildDocument({ + teamId: parent.teamId, + parentDocumentId: parent.id, + }); + + await Document.unscoped().update( + { deletedAt: null }, + { where: { id: parent.id }, paranoid: false } + ); + + await documentPermanentDeleter([parent]); + + await child.reload(); + expect(child.parentDocumentId).toEqual(parent.id); + }); + it("should not destroy attachments referenced in other documents", async () => { const document1 = await buildDocument(); const document = await buildDocument({ diff --git a/server/commands/documentPermanentDeleter.ts b/server/commands/documentPermanentDeleter.ts index b136716391..4e090f14ad 100644 --- a/server/commands/documentPermanentDeleter.ts +++ b/server/commands/documentPermanentDeleter.ts @@ -1,4 +1,4 @@ -import { uniq } from "es-toolkit/compat"; +import { chunk, uniq } from "es-toolkit/compat"; import { Op, QueryTypes } from "sequelize"; import Logger from "@server/logging/Logger"; import { Document, Attachment } from "@server/models"; @@ -76,26 +76,52 @@ export default async function documentPermanentDeleter(documents: Document[]) { ); } - const documentIds = documents.map((document) => document.id); - await Document.update( - { - parentDocumentId: null, - }, - { - where: { - parentDocumentId: { - [Op.in]: documentIds, - }, - }, - paranoid: false, - } - ); + // Number of documents to delete per database statement. Keeps the exclusive + // lock window short enough to avoid blocking concurrent web requests that + // read from the documents table. + const BATCH_SIZE = 100; - return Document.scope("withDrafts").destroy({ + const documentIds = documents.map((document) => document.id); + + // Re-check deletedAt in the database to exclude documents that were restored + // between the caller's query and now. Otherwise the parentDocumentId clear + // below would detach children of a restored parent, breaking the hierarchy. + const stillDeleted = await Document.unscoped().findAll({ + attributes: ["id"], where: { id: documentIds, deletedAt: { [Op.ne]: null }, }, - force: true, + paranoid: false, }); + const deletedIds = stillDeleted.map((document) => document.id); + const batches = chunk(deletedIds, BATCH_SIZE); + + for (const batch of batches) { + await Document.update( + { + parentDocumentId: null, + }, + { + where: { + parentDocumentId: { + [Op.in]: batch, + }, + }, + paranoid: false, + } + ); + } + + let totalDeleted = 0; + for (const batch of batches) { + totalDeleted += await Document.scope("withDrafts").destroy({ + where: { + id: batch, + deletedAt: { [Op.ne]: null }, + }, + force: true, + }); + } + return totalDeleted; }