Merge pull request #3658 from overleaf/jpa-change-write-path-file-deletion

[ProjectEntityMongoUpdateHandler] track deleted files in own collection

GitOrigin-RevId: 7dff10a4737626a2d02b93b346e7e02a9d9a8608

services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js

@@ -16,6 +16,7 @@ const ProjectGetter = require('./ProjectGetter')
 const ProjectLocator = require('./ProjectLocator')
 const FolderStructureBuilder = require('./FolderStructureBuilder')
 const SafePath = require('./SafePath')
+const { DeletedFile } = require('../../models/DeletedFile')
 
 const LOCK_NAMESPACE = 'mongoTransaction'
 const ENTITY_TYPE_TO_MONGO_PATH_SEGMENT = {
@@ -456,20 +457,14 @@ async function _insertDeletedDocReference(projectId, doc) {
 }
 
 async function _insertDeletedFileReference(projectId, fileRef) {
-  await Project.updateOne(
-    { _id: projectId },
-    {
-      $push: {
-        deletedFiles: {
-          _id: fileRef._id,
-          name: fileRef.name,
-          linkedFileData: fileRef.linkedFileData,
-          hash: fileRef.hash,
-          deletedAt: new Date()
-        }
-      }
-    }
-  ).exec()
+  await DeletedFile.create({
+    projectId,
+    _id: fileRef._id,
+    name: fileRef.name,
+    linkedFileData: fileRef.linkedFileData,
+    hash: fileRef.hash,
+    deletedAt: new Date()
+  })
 }
 
 async function _removeElementFromMongoArray(

services/web/app/src/infrastructure/mongodb.js

@@ -28,6 +28,7 @@ async function setupDb() {
   const internalDb = (await clientPromise).db()
 
   db.contacts = internalDb.collection('contacts')
+  db.deletedFiles = internalDb.collection('deletedFiles')
   db.deletedProjects = internalDb.collection('deletedProjects')
   db.deletedSubscriptions = internalDb.collection('deletedSubscriptions')
   db.deletedUsers = internalDb.collection('deletedUsers')

services/web/app/src/models/DeletedFile.js

@@ -0,0 +1,21 @@
+const mongoose = require('../infrastructure/Mongoose')
+const { Schema } = mongoose
+
+const DeletedFileSchema = new Schema(
+  {
+    name: String,
+    projectId: Schema.ObjectId,
+    created: {
+      type: Date
+    },
+    linkedFileData: { type: Schema.Types.Mixed },
+    hash: {
+      type: String
+    },
+    deletedAt: { type: Date }
+  },
+  { collection: 'deletedFiles' }
+)
+
+exports.DeletedFile = mongoose.model('DeletedFile', DeletedFileSchema)
+exports.DeletedFileSchema = DeletedFileSchema

services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js

@@ -4,6 +4,7 @@ const tk = require('timekeeper')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
 const { ObjectId } = require('mongodb')
 const SandboxedModule = require('sandboxed-module')
+const { DeletedFile } = require('../helpers/models/DeletedFile')
 const { Project } = require('../helpers/models/Project')
 
 const MODULE_PATH =
@@ -76,6 +77,7 @@ describe('ProjectEntityMongoUpdateHandler', function() {
     }
 
     this.FolderModel = sinon.stub()
+    this.DeletedFileMock = sinon.mock(DeletedFile)
     this.ProjectMock = sinon.mock(Project)
     this.ProjectEntityHandler = {
       promises: {
@@ -192,6 +194,7 @@ describe('ProjectEntityMongoUpdateHandler', function() {
         '../Cooldown/CooldownManager': this.CooldownManager,
         '../../models/Folder': { Folder: this.FolderModel },
         '../../infrastructure/LockManager': this.LockManager,
+        '../../models/DeletedFile': { DeletedFile },
         '../../models/Project': { Project },
         './ProjectEntityHandler': this.ProjectEntityHandler,
         './ProjectLocator': this.ProjectLocator,
@@ -202,6 +205,7 @@ describe('ProjectEntityMongoUpdateHandler', function() {
   })
 
   afterEach(function() {
+    this.DeletedFileMock.restore()
     this.ProjectMock.restore()
   })
 
@@ -355,22 +359,15 @@ describe('ProjectEntityMongoUpdateHandler', function() {
         hash: 'some-hash'
       }
       // Add a deleted file record
-      this.ProjectMock.expects('updateOne')
-        .withArgs(
-          { _id: this.project._id },
-          {
-            $push: {
-              deletedFiles: {
-                _id: this.file._id,
-                name: this.file.name,
-                linkedFileData: this.file.linkedFileData,
-                hash: this.file.hash,
-                deletedAt: sinon.match.date
-              }
-            }
-          }
-        )
-        .chain('exec')
+      this.DeletedFileMock.expects('create')
+        .withArgs({
+          projectId: this.project._id,
+          _id: this.file._id,
+          name: this.file.name,
+          linkedFileData: this.file.linkedFileData,
+          hash: this.file.hash,
+          deletedAt: sinon.match.date
+        })
         .resolves()
       // Update the file in place
       this.ProjectMock.expects('findOneAndUpdate')
@@ -399,6 +396,7 @@ describe('ProjectEntityMongoUpdateHandler', function() {
     })
 
     it('updates the database', function() {
+      this.DeletedFileMock.verify()
       this.ProjectMock.verify()
     })
   })
@@ -1008,22 +1006,15 @@ describe('ProjectEntityMongoUpdateHandler', function() {
 
   describe('_insertDeletedFileReference', function() {
     beforeEach(async function() {
-      this.ProjectMock.expects('updateOne')
-        .withArgs(
-          { _id: this.project._id },
-          {
-            $push: {
-              deletedFiles: {
-                _id: this.file._id,
-                name: this.file.name,
-                linkedFileData: this.file.linkedFileData,
-                hash: this.file.hash,
-                deletedAt: sinon.match.date
-              }
-            }
-          }
-        )
-        .chain('exec')
+      this.DeletedFileMock.expects('create')
+        .withArgs({
+          projectId: this.project._id,
+          _id: this.file._id,
+          name: this.file.name,
+          linkedFileData: this.file.linkedFileData,
+          hash: this.file.hash,
+          deletedAt: sinon.match.date
+        })
         .resolves()
       await this.subject.promises._insertDeletedFileReference(
         this.project._id,
@@ -1032,7 +1023,7 @@ describe('ProjectEntityMongoUpdateHandler', function() {
     })
 
     it('should update the database', function() {
-      this.ProjectMock.verify()
+      this.DeletedFileMock.verify()
     })
   })
 

services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js

@@ -2006,7 +2006,7 @@ describe('ProjectEntityUpdateHandler', function() {
         )
       })
 
-      it('should insert the file into the deletedFiles array', function() {
+      it('should insert the file into the deletedFiles collection', function() {
         this.ProjectEntityMongoUpdateHandler._insertDeletedFileReference
           .calledWith(this.project._id, this.entity)
           .should.equal(true)

services/web/test/unit/src/helpers/models/DeletedFile.js

@@ -0,0 +1,3 @@
+const mockModel = require('../MockModel')
+
+module.exports = mockModel('DeletedFile')