From ab87ed51b9a63766453ceb13d531c2ecfa250af9 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 17 Feb 2021 11:40:56 +0000 Subject: [PATCH] Merge pull request #3658 from overleaf/jpa-change-write-path-file-deletion [ProjectEntityMongoUpdateHandler] track deleted files in own collection GitOrigin-RevId: 7dff10a4737626a2d02b93b346e7e02a9d9a8608 --- .../ProjectEntityMongoUpdateHandler.js | 23 +++----- .../web/app/src/infrastructure/mongodb.js | 1 + services/web/app/src/models/DeletedFile.js | 21 +++++++ .../ProjectEntityMongoUpdateHandlerTests.js | 57 ++++++++----------- .../ProjectEntityUpdateHandlerTests.js | 2 +- .../unit/src/helpers/models/DeletedFile.js | 3 + 6 files changed, 59 insertions(+), 48 deletions(-) create mode 100644 services/web/app/src/models/DeletedFile.js create mode 100644 services/web/test/unit/src/helpers/models/DeletedFile.js diff --git a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js index 08767f5a9e..1a0a2cc7d1 100644 --- a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js +++ b/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( diff --git a/services/web/app/src/infrastructure/mongodb.js b/services/web/app/src/infrastructure/mongodb.js index 8a3c54f5d4..6e447bd11e 100644 --- a/services/web/app/src/infrastructure/mongodb.js +++ b/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') diff --git a/services/web/app/src/models/DeletedFile.js b/services/web/app/src/models/DeletedFile.js new file mode 100644 index 0000000000..2fc3c643f8 --- /dev/null +++ b/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 diff --git a/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js b/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js index 7e3d54b8ca..045f3d4ec1 100644 --- a/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js +++ b/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() }) }) diff --git a/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js b/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js index 7e2e61994a..4b4108b556 100644 --- a/services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js +++ b/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) diff --git a/services/web/test/unit/src/helpers/models/DeletedFile.js b/services/web/test/unit/src/helpers/models/DeletedFile.js new file mode 100644 index 0000000000..8e0b6a43b8 --- /dev/null +++ b/services/web/test/unit/src/helpers/models/DeletedFile.js @@ -0,0 +1,3 @@ +const mockModel = require('../MockModel') + +module.exports = mockModel('DeletedFile')