From 30cf22c8a97a3190600d3c0649a1f1822d97fd93 Mon Sep 17 00:00:00 2001
From: Jakob Ackermann <jakob.ackermann@overleaf.com>
Date: Fri, 19 Feb 2021 09:54:40 +0000
Subject: [PATCH] Merge pull request #3678 from
 overleaf/jpa-back-fill-deleted-files-handle-duplicates

[scripts] back_fill_deleted_files: handle duplicate deleted files

GitOrigin-RevId: 3f2f1b662727d61d2da2800ad0d635b65562164b
---
 .../web/scripts/back_fill_deleted_files.js    | 11 ++++++++++
 .../src/BackFillDeletedFilesTests.js          | 21 ++++++++++++++-----
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/services/web/scripts/back_fill_deleted_files.js b/services/web/scripts/back_fill_deleted_files.js
index 1f78b0706c..c9fed972eb 100644
--- a/services/web/scripts/back_fill_deleted_files.js
+++ b/services/web/scripts/back_fill_deleted_files.js
@@ -46,12 +46,23 @@ async function processProject(project) {
 
 async function backFillFiles(project) {
   const projectId = project._id
+  filterDuplicatesInPlace(project)
   project.deletedFiles.forEach(file => {
     file.projectId = projectId
   })
   await db.deletedFiles.insertMany(project.deletedFiles)
 }
 
+function filterDuplicatesInPlace(project) {
+  const fileIds = new Set()
+  project.deletedFiles = project.deletedFiles.filter(file => {
+    const id = file._id.toString()
+    if (fileIds.has(id)) return false
+    fileIds.add(id)
+    return true
+  })
+}
+
 async function cleanupProject(project) {
   await db.projects.updateOne(
     { _id: project._id },
diff --git a/services/web/test/acceptance/src/BackFillDeletedFilesTests.js b/services/web/test/acceptance/src/BackFillDeletedFilesTests.js
index 5cb1583659..3a5b35d6ba 100644
--- a/services/web/test/acceptance/src/BackFillDeletedFilesTests.js
+++ b/services/web/test/acceptance/src/BackFillDeletedFilesTests.js
@@ -21,7 +21,7 @@ async function unsetDeletedFiles(projectId) {
 }
 
 describe('BackFillDeletedFiles', function() {
-  let user, projectId1, projectId2, projectId3, projectId4
+  let user, projectId1, projectId2, projectId3, projectId4, projectId5
 
   beforeEach('create projects', async function() {
     user = new User()
@@ -31,14 +31,16 @@ describe('BackFillDeletedFiles', function() {
     projectId2 = ObjectId(await user.createProject('project2'))
     projectId3 = ObjectId(await user.createProject('project3'))
     projectId4 = ObjectId(await user.createProject('project4'))
+    projectId5 = ObjectId(await user.createProject('project5'))
   })
 
-  let fileId1, fileId2, fileId3
+  let fileId1, fileId2, fileId3, fileId4
   beforeEach('create files', function() {
     // take a short cut and just allocate file ids
     fileId1 = ObjectId()
     fileId2 = ObjectId()
     fileId3 = ObjectId()
+    fileId4 = ObjectId()
   })
   const otherFileDetails = {
     name: 'universe.jpg',
@@ -47,7 +49,7 @@ describe('BackFillDeletedFiles', function() {
     deletedAt: new Date(),
     __v: 0
   }
-  let deletedFiles1, deletedFiles2
+  let deletedFiles1, deletedFiles2, deletedFiles3
   beforeEach('set deletedFiles details', async function() {
     deletedFiles1 = [
       { _id: fileId1, ...otherFileDetails },
@@ -61,6 +63,12 @@ describe('BackFillDeletedFiles', function() {
     await setDeletedFiles(projectId3, [])
     // a project without deletedFiles array
     await unsetDeletedFiles(projectId4)
+    // duplicate entry
+    deletedFiles3 = [
+      { _id: fileId4, ...otherFileDetails },
+      { _id: fileId4, ...otherFileDetails }
+    ]
+    await setDeletedFiles(projectId5, deletedFiles3)
   })
 
   async function runScript(args = []) {
@@ -81,7 +89,7 @@ describe('BackFillDeletedFiles', function() {
     expect(stdOut).to.include(projectId1.toString())
     expect(stdOut).to.include(projectId2.toString())
 
-    expect(stdErr).to.include(`Completed batch ending ${projectId2}`)
+    expect(stdErr).to.include(`Completed batch ending ${projectId5}`)
   }
 
   function checkAreFilesBackFilled() {
@@ -92,7 +100,8 @@ describe('BackFillDeletedFiles', function() {
       expect(docs).to.deep.equal([
         { _id: fileId1, projectId: projectId1, ...otherFileDetails },
         { _id: fileId2, projectId: projectId1, ...otherFileDetails },
-        { _id: fileId3, projectId: projectId2, ...otherFileDetails }
+        { _id: fileId3, projectId: projectId2, ...otherFileDetails },
+        { _id: fileId4, projectId: projectId5, ...otherFileDetails }
       ])
     })
   }
@@ -105,6 +114,7 @@ describe('BackFillDeletedFiles', function() {
     it('should leave the deletedFiles as is', async function() {
       expect(await getDeletedFiles(projectId1)).to.deep.equal(deletedFiles1)
       expect(await getDeletedFiles(projectId2)).to.deep.equal(deletedFiles2)
+      expect(await getDeletedFiles(projectId5)).to.deep.equal(deletedFiles3)
     })
   })
 
@@ -118,6 +128,7 @@ describe('BackFillDeletedFiles', function() {
     it('should cleanup the deletedFiles', async function() {
       expect(await getDeletedFiles(projectId1)).to.deep.equal([])
       expect(await getDeletedFiles(projectId2)).to.deep.equal([])
+      expect(await getDeletedFiles(projectId5)).to.deep.equal([])
     })
   })
 })