From 93bf7cc4db5e4e478907407296e870a0c746ca53 Mon Sep 17 00:00:00 2001 From: Ersun Warncke Date: Wed, 5 Feb 2020 08:53:31 -0400 Subject: [PATCH] Revert "Revert "clear rootDoc_id when deleting doc, reset on compiles if invalid"" This reverts commit 7acba5876581044a08d6deb4767a4a2196dcb765. GitOrigin-RevId: 034ae6fa4d8515944683395ef14d99801829cb6a --- .../src/Features/Compile/CompileManager.js | 2 +- .../ProjectEntityMongoUpdateHandler.js | 29 ++++----- .../acceptance/src/ProjectStructureTests.js | 63 +++++++++++++++++++ .../src/Compile/CompileControllerTests.js | 6 +- .../unit/src/Compile/CompileManagerTests.js | 4 +- 5 files changed, 83 insertions(+), 21 deletions(-) diff --git a/services/web/app/src/Features/Compile/CompileManager.js b/services/web/app/src/Features/Compile/CompileManager.js index 454dc97d16..e452b421a8 100644 --- a/services/web/app/src/Features/Compile/CompileManager.js +++ b/services/web/app/src/Features/Compile/CompileManager.js @@ -62,7 +62,7 @@ module.exports = CompileManager = { return callback(null, 'autocompile-backoff', []) } - return ProjectRootDocManager.ensureRootDocumentIsSet( + return ProjectRootDocManager.ensureRootDocumentIsValid( project_id, function(error) { if (error != null) { diff --git a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js index 379c03967a..221b4bc0e3 100644 --- a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js @@ -314,8 +314,9 @@ async function moveEntity(projectId, entityId, destFolderId, entityType) { async function deleteEntity(projectId, entityId, entityType, callback) { const project = await ProjectGetter.promises.getProjectWithoutLock( projectId, - { name: true, rootFolder: true, overleaf: true } + { name: true, rootFolder: true, overleaf: true, rootDoc_id: true } ) + const deleteRootDoc = project.rootDoc_id && entityId && project.rootDoc_id.toString() === entityId.toString() const { element: entity, path } = await ProjectLocator.promises.findElement({ project, element_id: entityId, @@ -325,7 +326,8 @@ async function deleteEntity(projectId, entityId, entityType, callback) { Project, projectId, path.mongo, - entityId + entityId, + deleteRootDoc ) return { entity, path, projectBeforeDeletion: project, newProject } } @@ -414,19 +416,18 @@ async function _insertDeletedFileReference(projectId, fileRef) { ).exec() } -async function _removeElementFromMongoArray(model, modelId, path, elementId) { +async function _removeElementFromMongoArray(model, modelId, path, elementId, deleteRootDoc=false) { const nonArrayPath = path.slice(0, path.lastIndexOf('.')) - const newDoc = model - .findOneAndUpdate( - { _id: modelId }, - { - $pull: { [nonArrayPath]: { _id: elementId } }, - $inc: { version: 1 } - }, - { new: true } - ) - .exec() - return newDoc + const options = { new: true } + const query = { _id: modelId } + const update = { + $pull: { [nonArrayPath]: { _id: elementId } }, + $inc: { version: 1 } + } + if (deleteRootDoc) { + update.$unset = { rootDoc_id: 1 } + } + return model.findOneAndUpdate(query, update, options).exec() } function _countElements(project) { diff --git a/services/web/test/acceptance/src/ProjectStructureTests.js b/services/web/test/acceptance/src/ProjectStructureTests.js index 17f15ac240..69eea73d23 100644 --- a/services/web/test/acceptance/src/ProjectStructureTests.js +++ b/services/web/test/acceptance/src/ProjectStructureTests.js @@ -6,6 +6,7 @@ const fs = require('fs') const Settings = require('settings-sharelatex') const _ = require('underscore') +const { Project } = require('../../../app/src/models/Project') const ProjectGetter = require('../../../app/src/Features/Project/ProjectGetter.js') const MockDocUpdaterApi = require('./helpers/MockDocUpdaterApi') @@ -1098,6 +1099,68 @@ describe('ProjectStructureChanges', function() { }) }) + describe('deleting docs', function() { + beforeEach(function(done) { + createExampleProject(this, () => { + createExampleFolder(this, () => { + createExampleDoc(this, () => { + MockDocUpdaterApi.clearProjectStructureUpdates() + ProjectGetter.getProject( + this.exampleProjectId, + (error, project) => { + if (error) { + throw error + } + this.project0 = project + done() + }) + }) + }) + }) + }) + + describe('when rootDoc_id matches doc being deleted', () => { + beforeEach(function(done) { + Project.update({_id: this.exampleProjectId}, {$set: {rootDoc_id: this.exampleDocId}}, done) + }) + + it('should clear rootDoc_id', function(done) { + deleteItem(this, 'doc', this.exampleDocId, () => { + ProjectGetter.getProject( + this.exampleProjectId, + (error, project) => { + if (error) { + throw error + } + expect(project.rootDoc_id).to.be.undefined + done() + }) + }) + }) + }) + + describe('when rootDoc_id does not match doc being deleted', () => { + beforeEach(function(done) { + this.exampleRootDocId = new ObjectId() + Project.update({_id: this.exampleProjectId}, {$set: {rootDoc_id: this.exampleRootDocId}}, done) + }) + + it('should not clear rootDoc_id', function(done) { + deleteItem(this, 'doc', this.exampleDocId, () => { + ProjectGetter.getProject( + this.exampleProjectId, + (error, project) => { + if (error) { + throw error + } + expect(project.rootDoc_id.toString()).to.equal(this.exampleRootDocId.toString()) + done() + }) + }) + }) + }) + }) + describe('tpds', function() { let projectName, exampleProjectId, oldVersion, rootFolderId diff --git a/services/web/test/unit/src/Compile/CompileControllerTests.js b/services/web/test/unit/src/Compile/CompileControllerTests.js index 944a7d06d5..4120eb0eaf 100644 --- a/services/web/test/unit/src/Compile/CompileControllerTests.js +++ b/services/web/test/unit/src/Compile/CompileControllerTests.js @@ -120,14 +120,12 @@ describe('CompileController', function() { }) it('should set the content-type of the response to application/json', function() { - return this.res.contentType - .calledWith('application/json') - .should.equal(true) + this.res.type.should.equal('application/json') }) it('should send a successful response reporting the status and files', function() { this.res.statusCode.should.equal(200) - return this.res.body.should.equal( + this.res.body.should.equal( JSON.stringify({ status: this.status, outputFiles: this.outputFiles diff --git a/services/web/test/unit/src/Compile/CompileManagerTests.js b/services/web/test/unit/src/Compile/CompileManagerTests.js index afadd137bb..d9e0007e5e 100644 --- a/services/web/test/unit/src/Compile/CompileManagerTests.js +++ b/services/web/test/unit/src/Compile/CompileManagerTests.js @@ -76,7 +76,7 @@ describe('CompileManager', function() { this.CompileManager._checkIfRecentlyCompiled = sinon .stub() .callsArgWith(2, null, false) - this.ProjectRootDocManager.ensureRootDocumentIsSet = sinon + this.ProjectRootDocManager.ensureRootDocumentIsValid = sinon .stub() .callsArgWith(1, null) this.CompileManager.getProjectCompileLimits = sinon @@ -115,7 +115,7 @@ describe('CompileManager', function() { }) it('should ensure that the root document is set', function() { - return this.ProjectRootDocManager.ensureRootDocumentIsSet + return this.ProjectRootDocManager.ensureRootDocumentIsValid .calledWith(this.project_id) .should.equal(true) })