diff --git a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js index 46b16ffdd9..89800c7b2c 100644 --- a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js @@ -160,7 +160,7 @@ async function replaceFileWithNew(projectId, fileId, newFileRef) { }) await _insertDeletedFileReference(projectId, fileRef) const newProject = await Project.findOneAndUpdate( - { _id: project._id }, + { _id: project._id, [path.mongo]: { $exists: true } }, { $set: { [`${path.mongo}._id`]: newFileRef._id, @@ -173,14 +173,20 @@ async function replaceFileWithNew(projectId, fileId, newFileRef) { [`${path.mongo}.rev`]: 1, }, }, + // Note: Mongoose uses new:true to return the modified document + // https://mongoosejs.com/docs/api.html#model_Model.findOneAndUpdate + // but Mongo uses returnNewDocument:true instead + // https://docs.mongodb.com/manual/reference/method/db.collection.findOneAndUpdate/ + // We are using Mongoose here, but if we ever switch to a direct mongo call + // the next line will need to be updated. { new: true } ).exec() - // Note: Mongoose uses new:true to return the modified document - // https://mongoosejs.com/docs/api.html#model_Model.findOneAndUpdate - // but Mongo uses returnNewDocument:true instead - // https://docs.mongodb.com/manual/reference/method/db.collection.findOneAndUpdate/ - // We are using Mongoose here, but if we ever switch to a direct mongo call - // the next line will need to be updated. + if (newProject == null) { + throw new OError('Project not found or path not found in filetree', { + projectId, + path, + }) + } return { oldFileRef: fileRef, project, path, newProject } } @@ -196,7 +202,7 @@ async function replaceDocWithFile(projectId, docId, fileRef) { }) const folderMongoPath = _getParentMongoPath(path.mongo) const newProject = await Project.findOneAndUpdate( - { _id: project._id }, + { _id: project._id, [folderMongoPath]: { $exists: true } }, { $pull: { [`${folderMongoPath}.docs`]: { _id: docId }, @@ -208,6 +214,12 @@ async function replaceDocWithFile(projectId, docId, fileRef) { }, { new: true } ).exec() + if (newProject == null) { + throw new OError('Project not found or path not found in filetree', { + projectId, + path, + }) + } return newProject } @@ -223,7 +235,7 @@ async function replaceFileWithDoc(projectId, fileId, newDoc) { }) const folderMongoPath = _getParentMongoPath(path.mongo) const newProject = await Project.findOneAndUpdate( - { _id: project._id }, + { _id: project._id, [folderMongoPath]: { $exists: true } }, { $pull: { [`${folderMongoPath}.fileRefs`]: { _id: fileId }, @@ -235,6 +247,12 @@ async function replaceFileWithDoc(projectId, fileId, newDoc) { }, { new: true } ).exec() + if (newProject == null) { + throw new OError('Project not found or path not found in filetree', { + projectId, + path, + }) + } return newProject } @@ -414,10 +432,16 @@ async function renameEntity( // we need to increment the project version number for any structure change const newProject = await Project.findOneAndUpdate( - { _id: projectId }, + { _id: projectId, [entPath.mongo]: { $exists: true } }, { $set: { [`${entPath.mongo}.name`]: newName }, $inc: { version: 1 } }, { new: true } ).exec() + if (newProject == null) { + throw new OError('Project not found or path not found in filetree', { + projectId, + path: entPath, + }) + } const { docs: newDocs, files: newFiles } = ProjectEntityHandler.getAllEntitiesFromProject(newProject) @@ -541,10 +565,16 @@ async function _putElement(project, folderId, element, type) { element._id = ObjectId(element._id.toString()) const mongoPath = `${path.mongo}.${pathSegment}` const newProject = await Project.findOneAndUpdate( - { _id: project._id }, + { _id: project._id, [path.mongo]: { $exists: true } }, { $push: { [mongoPath]: element }, $inc: { version: 1 } }, { new: true } ).exec() + if (newProject == null) { + throw new OError('Project not found or path not found in filetree', { + projectId: project._id, + path, + }) + } return { result: { path: newPath }, project: newProject } } diff --git a/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js b/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js index a149f926f0..d5ade92860 100644 --- a/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js @@ -210,7 +210,10 @@ describe('ProjectEntityMongoUpdateHandler', function () { const doc = { _id: ObjectId(), name: 'other.txt' } this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { + _id: this.project._id, + 'rootFolder.0.folders.0': { $exists: true }, + }, { $push: { 'rootFolder.0.folders.0.docs': doc }, $inc: { version: 1 }, @@ -247,7 +250,10 @@ describe('ProjectEntityMongoUpdateHandler', function () { this.newFile = { _id: ObjectId(), name: 'picture.jpg' } this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { + _id: this.project._id, + 'rootFolder.0.folders.0': { $exists: true }, + }, { $push: { 'rootFolder.0.folders.0.fileRefs': this.newFile }, $inc: { version: 1 }, @@ -314,7 +320,10 @@ describe('ProjectEntityMongoUpdateHandler', function () { }) this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { + _id: this.project._id, + 'rootFolder.0.folders.0': { $exists: true }, + }, { $push: { 'rootFolder.0.folders.0.folders': sinon.match({ @@ -360,7 +369,10 @@ describe('ProjectEntityMongoUpdateHandler', function () { // Update the file in place this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { + _id: this.project._id, + 'rootFolder.0.fileRefs.0': { $exists: true }, + }, { $set: { 'rootFolder.0.fileRefs.0._id': newFile._id, @@ -438,7 +450,7 @@ describe('ProjectEntityMongoUpdateHandler', function () { this.exactCaseMatch = false this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { _id: this.project._id, 'rootFolder.0': { $exists: true } }, { $push: { 'rootFolder.0.folders': this.newFolder }, $inc: { version: 1 }, @@ -481,7 +493,10 @@ describe('ProjectEntityMongoUpdateHandler', function () { this.FolderModel.returns(this.newFolder) this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { + _id: this.project._id, + 'rootFolder.0.folders.0': { $exists: true }, + }, { $push: { 'rootFolder.0.folders.0.folders': sinon.match({ @@ -552,7 +567,10 @@ describe('ProjectEntityMongoUpdateHandler', function () { }) this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { + _id: this.project._id, + 'rootFolder.0.folders.0': { $exists: true }, + }, { $push: { 'rootFolder.0.folders.0.folders': sinon.match({ @@ -566,7 +584,10 @@ describe('ProjectEntityMongoUpdateHandler', function () { .resolves(this.project) this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { + _id: this.project._id, + 'rootFolder.0.folders.0.folders.0': { $exists: true }, + }, { $push: { 'rootFolder.0.folders.0.folders.0.folders': sinon.match({ @@ -642,7 +663,10 @@ describe('ProjectEntityMongoUpdateHandler', function () { this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { + _id: this.project._id, + 'rootFolder.0.folders.0': { $exists: true }, + }, { $push: { 'rootFolder.0.folders.0.docs': this.doc }, $inc: { version: 1 }, @@ -758,7 +782,7 @@ describe('ProjectEntityMongoUpdateHandler', function () { this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { _id: this.project._id, 'rootFolder.0.docs.0': { $exists: true } }, { $set: { 'rootFolder.0.docs.0.name': this.newName }, $inc: { version: 1 }, @@ -816,7 +840,10 @@ describe('ProjectEntityMongoUpdateHandler', function () { this.newFile = { _id: ObjectId(), name: 'new file.png' } this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { + _id: this.project._id, + 'rootFolder.0.folders.0': { $exists: true }, + }, { $push: { 'rootFolder.0.folders.0.fileRefs': this.newFile }, $inc: { version: 1 }, @@ -946,7 +973,7 @@ describe('ProjectEntityMongoUpdateHandler', function () { this.newFile = { _id: ObjectId(), name: 'new file.png' } this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { _id: this.project._id, 'rootFolder.0': { $exists: true } }, { $push: { 'rootFolder.0.fileRefs': this.newFile }, $inc: { version: 1 }, @@ -1045,7 +1072,7 @@ describe('ProjectEntityMongoUpdateHandler', function () { it('should simultaneously remove the doc and add the file', async function () { this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { _id: this.project._id, 'rootFolder.0': { $exists: true } }, { $pull: { 'rootFolder.0.docs': { _id: this.doc._id } }, $push: { 'rootFolder.0.fileRefs': this.file }, @@ -1068,7 +1095,7 @@ describe('ProjectEntityMongoUpdateHandler', function () { it('should simultaneously remove the file and add the doc', async function () { this.ProjectMock.expects('findOneAndUpdate') .withArgs( - { _id: this.project._id }, + { _id: this.project._id, 'rootFolder.0': { $exists: true } }, { $pull: { 'rootFolder.0.fileRefs': { _id: this.file._id } }, $push: { 'rootFolder.0.docs': this.doc },