From ca0a46b5bb7a4f952896e7b68951230401ec73cf Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 15 Nov 2024 17:19:05 +0100 Subject: [PATCH] Merge pull request #21928 from overleaf/jpa-handle-already-hard-deleted [history-v1] backup-deletion-app: use deletedProjectOverleafHistoryId GitOrigin-RevId: 169ba0fba71c42b0415e5fa40424547b054dd5b0 --- .../history-v1/storage/lib/backupDeletion.mjs | 10 ++++-- .../acceptance/js/api/backupDeletion.test.mjs | 32 ++++++++++++------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/services/history-v1/storage/lib/backupDeletion.mjs b/services/history-v1/storage/lib/backupDeletion.mjs index dfbc178281..b52c31a160 100644 --- a/services/history-v1/storage/lib/backupDeletion.mjs +++ b/services/history-v1/storage/lib/backupDeletion.mjs @@ -37,7 +37,7 @@ async function deleteProjectBackup(projectId) { { 'deleterData.deletedProjectId': new ObjectId(projectId) }, { projection: { - 'project.overleaf.history.id': 1, + 'deleterData.deletedProjectOverleafHistoryId': 1, 'deleterData.deletedAt': 1, }, } @@ -51,7 +51,13 @@ async function deleteProjectBackup(projectId) { throw new NotReadyToDelete('refusing to delete non-expired project') } - const historyId = deletedProject.project.overleaf.history.id + const historyId = deletedProject.deleterData.deletedProjectOverleafHistoryId + if (!historyId) { + throw new NotReadyToDelete( + 'refusing to delete project with unknown historyId' + ) + } + if (await projectHasLatestChunk(historyId)) { throw new NotReadyToDelete( 'refusing to delete project with remaining chunks' diff --git a/services/history-v1/test/acceptance/js/api/backupDeletion.test.mjs b/services/history-v1/test/acceptance/js/api/backupDeletion.test.mjs index c701753c33..d6d4ae0db9 100644 --- a/services/history-v1/test/acceptance/js/api/backupDeletion.test.mjs +++ b/services/history-v1/test/acceptance/js/api/backupDeletion.test.mjs @@ -96,6 +96,7 @@ describe('backupDeletion', function () { const projectIdNonDeleted = new ObjectId('000000000000000000000003') const projectIdNonExpired = new ObjectId('000000000000000000000004') const projectIdWithChunks = new ObjectId('000000000000000000000005') + const projectIdNoHistoryId = new ObjectId('000000000000000000000006') beforeEach('cleanup s3 buckets', async function () { await backupPersistor.deleteDirectory(deksBucket, '') @@ -107,36 +108,34 @@ describe('backupDeletion', function () { await deletedProjectsCollection.insertMany([ { _id: new ObjectId(), - project: { - _id: projectIdPostgres, - overleaf: { history: { id: postgresHistoryId } }, - }, deleterData: { deletedProjectId: projectIdPostgres, deletedAt: new Date('2024-01-01T00:00:00Z'), + deletedProjectOverleafHistoryId: postgresHistoryId, }, }, { _id: new ObjectId(), - project: { - _id: projectIdNonExpired, - overleaf: { history: { id: projectIdNonExpired.toString() } }, - }, deleterData: { deletedProjectId: projectIdNonExpired, deletedAt: new Date(), + deletedProjectOverleafHistoryId: projectIdNonExpired.toString(), + }, + }, + { + _id: new ObjectId(), + deleterData: { + deletedProjectId: projectIdNoHistoryId, + deletedAt: new Date('2024-01-01T00:00:00Z'), }, }, ...[projectIdMongoDB, projectIdWithChunks].map(projectId => { return { _id: new ObjectId(), - project: { - _id: projectId, - overleaf: { history: { id: projectId.toString() } }, - }, deleterData: { deletedProjectId: projectId, deletedAt: new Date('2024-01-01T00:00:00Z'), + deletedProjectOverleafHistoryId: projectId.toString(), }, } }), @@ -154,6 +153,7 @@ describe('backupDeletion', function () { projectIdNonDeleted, projectIdNonExpired, projectIdWithChunks, + projectIdNoHistoryId, ] const jobs = [] for (const historyId of historyIds) { @@ -226,6 +226,14 @@ describe('backupDeletion', function () { ) await expectToHaveBackup(projectIdWithChunks) }) + it('returns 422 when historyId is unknown', async function () { + const response = await deleteProject(projectIdNoHistoryId) + expect(response.status).to.equal(422) + expect(await response.text()).to.equal( + 'refusing to delete project with unknown historyId' + ) + await expectToHaveBackup(projectIdNoHistoryId) + }) it('should successfully delete postgres id', async function () { await expectToHaveBackup(postgresHistoryId) const response = await deleteProject(projectIdPostgres)