From 23be656aec3d0bb88ee11edd270d293f9e985008 Mon Sep 17 00:00:00 2001
From: Jakob Ackermann <jakob.ackermann@overleaf.com>
Date: Thu, 1 Apr 2021 12:39:48 +0200
Subject: [PATCH] Merge pull request #3746 from
 overleaf/jpa-hard-deletion-from-deleted-files

[ProjectDeleter] hard deletion of project deletes deletedFiles entries

GitOrigin-RevId: b514c34465d5fdc66b40aae5bcdb8b66975bc350
---
 .../src/Features/Project/ProjectDeleter.js    | 22 +++++-
 .../web/test/acceptance/src/DeletionTests.js  | 75 +++++++++++++++++++
 .../unit/src/Project/ProjectDeleterTests.js   |  4 +
 3 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/services/web/app/src/Features/Project/ProjectDeleter.js b/services/web/app/src/Features/Project/ProjectDeleter.js
index b6843d2472..f17b618bf5 100644
--- a/services/web/app/src/Features/Project/ProjectDeleter.js
+++ b/services/web/app/src/Features/Project/ProjectDeleter.js
@@ -354,7 +354,8 @@ async function expireDeletedProject(projectId) {
         deletedProject.project._id,
         historyId
       ),
-      FilestoreHandler.promises.deleteProject(deletedProject.project._id)
+      FilestoreHandler.promises.deleteProject(deletedProject.project._id),
+      hardDeleteDeletedFiles(deletedProject.project._id)
     ])
 
     await DeletedProject.updateOne(
@@ -383,3 +384,22 @@ function filterDuplicateDeletedFilesInPlace(project) {
     return true
   })
 }
+
+let deletedFilesProjectIdIndexExist
+async function doesDeletedFilesProjectIdIndexExist() {
+  if (typeof deletedFilesProjectIdIndexExist !== 'boolean') {
+    // Resolve this about once. No need for locking or retry handling.
+    deletedFilesProjectIdIndexExist = await db.deletedFiles.indexExists(
+      'projectId_1'
+    )
+  }
+  return deletedFilesProjectIdIndexExist
+}
+
+async function hardDeleteDeletedFiles(projectId) {
+  if (!(await doesDeletedFilesProjectIdIndexExist())) {
+    // Running the deletion command w/o index would kill mongo performance
+    return
+  }
+  return db.deletedFiles.deleteMany({ projectId })
+}
diff --git a/services/web/test/acceptance/src/DeletionTests.js b/services/web/test/acceptance/src/DeletionTests.js
index 1cc0b737f4..ac4621f12b 100644
--- a/services/web/test/acceptance/src/DeletionTests.js
+++ b/services/web/test/acceptance/src/DeletionTests.js
@@ -239,6 +239,81 @@ describe('Deleting a project', function() {
     })
   })
 
+  describe('when the project has deleted files', function() {
+    beforeEach('get rootFolder id', function(done) {
+      this.user.getProject(this.projectId, (error, project) => {
+        if (error) return done(error)
+        this.rootFolder = project.rootFolder[0]._id
+        done()
+      })
+    })
+
+    let allFileIds
+    beforeEach('reset allFileIds', function() {
+      allFileIds = []
+    })
+    function createAndDeleteFile(name) {
+      let fileId
+      beforeEach(`create file ${name}`, function(done) {
+        this.user.uploadExampleFileInProject(
+          this.projectId,
+          this.rootFolder,
+          name,
+          (error, theFileId) => {
+            fileId = theFileId
+            allFileIds.push(theFileId)
+            done(error)
+          }
+        )
+      })
+      beforeEach(`delete file ${name}`, function(done) {
+        this.user.deleteItemInProject(this.projectId, 'file', fileId, done)
+      })
+    }
+    for (const name of ['a.png', 'another.png']) {
+      createAndDeleteFile(name)
+    }
+
+    it('should have two deleteFiles entries', async function() {
+      const files = await db.deletedFiles
+        .find({}, { sort: { _id: 1 } })
+        .toArray()
+      expect(files).to.have.length(2)
+      expect(files.map(file => file._id.toString())).to.deep.equal(allFileIds)
+    })
+
+    describe('When the deleted project is expired', function() {
+      beforeEach('soft delete the project', function(done) {
+        this.user.deleteProject(this.projectId, done)
+      })
+      beforeEach('hard delete the project', function(done) {
+        request.post(
+          `/internal/project/${this.projectId}/expire-deleted-project`,
+          {
+            auth: {
+              user: settings.apis.web.user,
+              pass: settings.apis.web.pass,
+              sendImmediately: true
+            }
+          },
+          (error, res) => {
+            expect(error).not.to.exist
+            expect(res.statusCode).to.equal(200)
+
+            done()
+          }
+        )
+      })
+
+      it('should cleanup the deleteFiles', async function() {
+        const files = await db.deletedFiles
+          .find({}, { sort: { _id: 1 } })
+          .toArray()
+        expect(files).to.deep.equal([])
+      })
+    })
+  })
+
   describe('When the project has docs', function() {
     beforeEach(function(done) {
       this.user.getProject(this.projectId, (error, project) => {
diff --git a/services/web/test/unit/src/Project/ProjectDeleterTests.js b/services/web/test/unit/src/Project/ProjectDeleterTests.js
index a45c190ea8..d8185552ca 100644
--- a/services/web/test/unit/src/Project/ProjectDeleterTests.js
+++ b/services/web/test/unit/src/Project/ProjectDeleterTests.js
@@ -103,6 +103,10 @@ describe('ProjectDeleter', function() {
     }
 
     this.db = {
+      deletedFiles: {
+        indexExists: sinon.stub().resolves(false),
+        deleteMany: sinon.stub()
+      },
       projects: {
         insertOne: sinon.stub().resolves()
       }