From 599f2cb3ae7fcd1a8b2427b9b7b056401d13cb1d Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 1 Nov 2017 17:54:58 +0000 Subject: [PATCH 1/6] add DocumentUpdaterHandler.updateProjectStructure --- .../DocumentUpdaterHandler.coffee | 36 ++++++++ .../DocumentUpdaterHandlerTests.coffee | 84 +++++++++++++------ 2 files changed, 95 insertions(+), 25 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index e806ddf8dd..6ec7613cfd 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -204,6 +204,42 @@ module.exports = DocumentUpdaterHandler = logger.error {project_id, doc_id, thread_id}, "doc updater returned a non-success status code: #{res.statusCode}" callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + updateProjectStructure : (project_id, user_id, oldDocs, newDocs, callback = (error) ->)-> + return callback() if !settings.apis.project_history?.enabled + + docRenameUpdates = [] + + for oldDoc in oldDocs + newDoc = _.find newDocs, (newDoc) -> + newDoc.doc._id.toString() == oldDoc.doc._id.toString() + if newDoc.path != oldDoc.path + docRenameUpdates.push + id: oldDoc.doc._id + pathname: oldDoc.path + newPathname: newDoc.path + + timer = new metrics.Timer("set-document") + url = "#{settings.apis.documentupdater.url}/project/#{project_id}" + body = + url: url + json: + updates: docRenameUpdates + user_id: user_id + + return callback() if docRenameUpdates.length < 1 + + request.post body, (error, res, body)-> + timer.done() + if error? + logger.error {error, url, project_id}, "error update project structure in doc updater" + callback(error) + else if res.statusCode >= 200 and res.statusCode < 300 + logger.error {project_id}, "updated project structure in doc updater" + callback(null) + else + logger.error {project_id, url}, "doc updater returned a non-success status code: #{res.statusCode}" + callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + PENDINGUPDATESKEY = "PendingUpdates" DOCLINESKEY = "doclines" DOCIDSWITHPENDINGUPDATES = "DocsWithPendingUpdates" diff --git a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index d7e88a6615..1a9c12a754 100644 --- a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -6,6 +6,7 @@ SandboxedModule = require('sandboxed-module') assert = require('chai').assert path = require 'path' _ = require 'underscore' +ObjectId = require("mongojs").ObjectId; modulePath = path.join __dirname, '../../../../app/js/Features/DocumentUpdater/DocumentUpdaterHandler' describe 'DocumentUpdaterHandler', -> @@ -20,8 +21,14 @@ describe 'DocumentUpdaterHandler', -> @request = {} @projectEntityHandler = {} - @settings = - apis : documentupdater: url : "http://something.com" + @settings = + apis: + documentupdater: + url : "http://document_updater.example.com" + project_history: + url: "http://project_history.example.com" + + @callback = sinon.stub() @handler = SandboxedModule.require modulePath, requires: 'request': defaults:=> return @request 'settings-sharelatex':@settings @@ -29,14 +36,11 @@ describe 'DocumentUpdaterHandler', -> '../Project/ProjectEntityHandler':@projectEntityHandler "../../models/Project": Project: @Project={} '../../Features/Project/ProjectLocator':{} - "metrics-sharelatex": + "metrics-sharelatex": Timer:-> done:-> describe 'flushProjectToMongo', -> - beforeEach -> - @callback = sinon.stub() - describe "successfully", -> beforeEach -> @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") @@ -68,9 +72,6 @@ describe 'DocumentUpdaterHandler', -> .should.equal true describe 'flushProjectToMongoAndDelete', -> - beforeEach -> - @callback = sinon.stub() - describe "successfully", -> beforeEach -> @request.del = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") @@ -102,9 +103,6 @@ describe 'DocumentUpdaterHandler', -> .should.equal true describe 'flushDocToMongo', -> - beforeEach -> - @callback = sinon.stub() - describe "successfully", -> beforeEach -> @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") @@ -136,9 +134,6 @@ describe 'DocumentUpdaterHandler', -> .should.equal true describe "deleteDoc", -> - beforeEach -> - @callback = sinon.stub() - describe "successfully", -> beforeEach -> @request.del = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") @@ -171,7 +166,6 @@ describe 'DocumentUpdaterHandler', -> describe "setDocument", -> beforeEach -> - @callback = sinon.stub() @source = "dropbox" describe "successfully", -> @@ -213,9 +207,6 @@ describe 'DocumentUpdaterHandler', -> .should.equal true describe "getDocument", -> - beforeEach -> - @callback = sinon.stub() - describe "successfully", -> beforeEach -> @body = JSON.stringify @@ -254,7 +245,6 @@ describe 'DocumentUpdaterHandler', -> describe "getProjectDocsIfMatch", -> beforeEach -> - @callback = sinon.stub() @project_state_hash = "1234567890abcdef" describe "successfully", -> @@ -295,9 +285,6 @@ describe 'DocumentUpdaterHandler', -> describe "clearProjectState", -> - beforeEach -> - @callback = sinon.stub() - describe "successfully", -> beforeEach -> @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}) @@ -332,7 +319,6 @@ describe 'DocumentUpdaterHandler', -> describe "acceptChanges", -> beforeEach -> @change_id = "mock-change-id-1" - @callback = sinon.stub() describe "successfully", -> beforeEach -> @@ -370,7 +356,6 @@ describe 'DocumentUpdaterHandler', -> describe "deleteThread", -> beforeEach -> @thread_id = "mock-thread-id-1" - @callback = sinon.stub() describe "successfully", -> beforeEach -> @@ -401,3 +386,52 @@ describe 'DocumentUpdaterHandler', -> @callback .calledWith(new Error("doc updater returned failure status code: 500")) .should.equal true + + describe "updateProjectStructure ", -> + beforeEach -> + @user_id = 1234 + @docIdA = new ObjectId() + @docIdB = new ObjectId() + @oldDocs = [ + { path: '/old_a', doc: _id: @docIdA } + { path: '/old_b', doc: _id: @docIdB } + ] + # create new instances of the same ObjectIds so that == doens't pass + @newDocs = [ + { path: '/old_a', doc: _id: new ObjectId(@docIdA.toString()) } + { path: '/new_b', doc: _id: new ObjectId(@docIdB.toString()) } + ] + + describe "with project history disabled", -> + beforeEach -> + @settings.apis.project_history.enabled = false + @request.post = sinon.stub() + + @handler.updateProjectStructure @project_id, @user_id, @oldDocs, @newDocs, @callback + + it 'does not make a web request', -> + @request.post.called.should.equal false + + it 'calls the callback', -> + @callback.called.should.equal true + + describe "with project history enabled", -> + beforeEach -> + @settings.apis.project_history.enabled = true + @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") + @handler.updateProjectStructure @project_id, @user_id, @oldDocs, @newDocs, @callback + + it 'should send the structure update to the document updater', -> + updates = [ + id: @docIdB, + pathname: "/old_b" + newPathname: "/new_b" + ] + + url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}" + @request.post + .calledWith(url: url, json: {updates, @user_id}) + .should.equal true + + it "should call the callback with no error", -> + @callback.calledWith(null).should.equal true From 929b9996d397bdbeae7132b0899550ed3af919a5 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 1 Nov 2017 18:21:05 +0000 Subject: [PATCH 2/6] version doc renames --- .../Features/Editor/EditorController.coffee | 4 +-- .../Editor/EditorHttpController.coffee | 4 ++- .../Project/ProjectEntityHandler.coffee | 33 +++++++++++-------- .../Editor/EditorControllerTests.coffee | 10 +++--- .../Editor/EditorHttpControllerTests.coffee | 13 +++++--- .../Project/ProjectEntityHandlerTests.coffee | 29 ++++++++++++---- 6 files changed, 59 insertions(+), 34 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index b748085daf..5026f62595 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -150,11 +150,11 @@ module.exports = EditorController = logger.log project_id:project_id, "recived message to delete project" ProjectDeleter.deleteProject project_id, callback - renameEntity: (project_id, entity_id, entityType, newName, callback)-> + renameEntity: (project_id, entity_id, entityType, newName, userId, callback)-> newName = sanitize.escape(newName) Metrics.inc "editor.rename-entity" logger.log entity_id:entity_id, entity_id:entity_id, entity_id:entity_id, "reciving new name for entity for project" - ProjectEntityHandler.renameEntity project_id, entity_id, entityType, newName, -> + ProjectEntityHandler.renameEntity project_id, entity_id, entityType, newName, userId, (err) -> if err? logger.err err:err, project_id:project_id, entity_id:entity_id, entityType:entityType, newName:newName, "error renaming entity" return callback(err) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 058799a495..63eeef70e9 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -12,6 +12,7 @@ CollaboratorsHandler = require("../Collaborators/CollaboratorsHandler") CollaboratorsInviteHandler = require("../Collaborators/CollaboratorsInviteHandler") PrivilegeLevels = require "../Authorization/PrivilegeLevels" TokenAccessHandler = require '../TokenAccess/TokenAccessHandler' +AuthenticationController = require "../Authentication/AuthenticationController" module.exports = EditorHttpController = joinProject: (req, res, next) -> @@ -112,9 +113,10 @@ module.exports = EditorHttpController = entity_id = req.params.entity_id entity_type = req.params.entity_type name = req.body.name + user_id = AuthenticationController.getLoggedInUserId(req) if !EditorHttpController._nameIsAcceptableLength(name) return res.sendStatus 400 - EditorController.renameEntity project_id, entity_id, entity_type, name, (error) -> + EditorController.renameEntity project_id, entity_id, entity_type, name, user_id, (error) -> return next(error) if error? res.sendStatus 204 diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 6489b2bbea..03849762a2 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -417,25 +417,30 @@ module.exports = ProjectEntityHandler = callback null - renameEntity: (project_id, entity_id, entityType, newName, callback)-> + renameEntity: (project_id, entity_id, entityType, newName, userId, callback)-> logger.log(entity_id: entity_id, project_id: project_id, ('renaming '+entityType)) if !entityType? logger.err err: "No entityType set", project_id: project_id, entity_id: entity_id return callback("No entityType set") entityType = entityType.toLowerCase() - ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project)=> - projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (err, entity, path, folder)=> - if err? - return callback err - conditons = {_id:project_id} - update = "$set":{} - namePath = path.mongo+".name" - update["$set"][namePath] = newName - endPath = path.fileSystem.replace(entity.name, newName) - tpdsUpdateSender.moveEntity({project_id:project_id, startPath:path.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) - Project.update conditons, update, {}, (err)-> - if callback? - callback err + ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (error, project)=> + return callback(error) if error? + ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs) => + return callback(error) if error? + projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, path)=> + return callback(error) if error? + endPath = path.fileSystem.replace(entity.name, newName) + conditions = {_id:project_id} + update = "$set":{} + namePath = path.mongo+".name" + update["$set"][namePath] = newName + tpdsUpdateSender.moveEntity({project_id:project_id, startPath:path.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) + Project.findOneAndUpdate conditions, update, { "new": true}, (error, newProject) -> + return callback(error) if error? + ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs) => + return callback(error) if error? + documentUpdaterHandler = require('../../Features/DocumentUpdater/DocumentUpdaterHandler') + documentUpdaterHandler.updateProjectStructure project_id, userId, oldDocs, newDocs, callback _cleanUpEntity: (project, entity, entityType, callback = (error) ->) -> if(entityType.indexOf("file") != -1) diff --git a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee index 1c69b375b8..2d9e344c8e 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee @@ -489,23 +489,21 @@ describe "EditorController", -> describe "renameEntity", -> - beforeEach -> - @err = "errro" @entity_id = "entity_id_here" @entityType = "doc" @newName = "bobsfile.tex" - @ProjectEntityHandler.renameEntity = sinon.stub().callsArgWith(4, @err) + @ProjectEntityHandler.renameEntity = sinon.stub().callsArg(5) @EditorRealTimeController.emitToRoom = sinon.stub() it "should call the project handler", (done)-> - @EditorController.renameEntity @project_id, @entity_id, @entityType, @newName, => - @ProjectEntityHandler.renameEntity.calledWith(@project_id, @entity_id, @entityType, @newName).should.equal true + @EditorController.renameEntity @project_id, @entity_id, @entityType, @newName, @user_id, => + @ProjectEntityHandler.renameEntity.calledWith(@project_id, @entity_id, @entityType, @newName, @user_id).should.equal true done() it "should emit the update to the room", (done)-> - @EditorController.renameEntity @project_id, @entity_id, @entityType, @newName, => + @EditorController.renameEntity @project_id, @entity_id, @entityType, @newName, @user_id, => @EditorRealTimeController.emitToRoom.calledWith(@project_id, 'reciveEntityRename', @entity_id, @newName).should.equal true done() diff --git a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee index 2bde0cdab2..2567230a36 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee @@ -19,6 +19,7 @@ describe "EditorHttpController", -> "../Collaborators/CollaboratorsHandler": @CollaboratorsHandler = {} "../Collaborators/CollaboratorsInviteHandler": @CollaboratorsInviteHandler = {} "../TokenAccess/TokenAccessHandler": @TokenAccessHandler = {} + "../Authentication/AuthenticationController": @AuthenticationController = {} @project_id = "mock-project-id" @doc_id = "mock-doc-id" @@ -264,12 +265,14 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = name: @name = "new-name" - @EditorController.renameEntity = sinon.stub().callsArg(4) + @userId = 1234 + @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) + @EditorController.renameEntity = sinon.stub().callsArg(5) @EditorHttpController.renameEntity @req, @res it "should call EditorController.renameEntity", -> @EditorController.renameEntity - .calledWith(@project_id, @entity_id, @entity_type, @name) + .calledWith(@project_id, @entity_id, @entity_type, @name, @userId) .should.equal true it "should send back a success response", -> @@ -283,6 +286,8 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = name: @name = "EDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOTEDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOT" + @userId = 1234 + @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @EditorController.renameEntity = sinon.stub().callsArg(4) @EditorHttpController.renameEntity @req, @res @@ -290,7 +295,6 @@ describe "EditorHttpController", -> @res.sendStatus.calledWith(400).should.equal true describe "rename entity with 0 length name", -> - beforeEach -> @req.params = Project_id: @project_id @@ -298,13 +302,14 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = name: @name = "" + @userId = 1234 + @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @EditorController.renameEntity = sinon.stub().callsArg(4) @EditorHttpController.renameEntity @req, @res it "should send back a bad request status code", -> @res.sendStatus.calledWith(400).should.equal true - describe "moveEntity", -> beforeEach -> @req.params = diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index 0bf3460cbb..9263ebed9f 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -976,19 +976,34 @@ describe 'ProjectEntityHandler', -> @entityType = "doc" @newName = "new.tex" @path = mongo: "mongo.path", fileSystem: "/file/system/old.tex" - @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: @entity_id, name:"old.tex", rev:4 }, @path) - @ProjectModel.update = sinon.stub().callsArgWith(3) - @tpdsUpdateSender.moveEntity = sinon.stub() + @userId = 1234 + @ProjectGetter.getProject.callsArgWith(2, null, @project) + @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() + @ProjectEntityHandler.getAllEntitiesFromProject + .onFirstCall().callsArgWith(1, null, @oldDocs = []) + @ProjectEntityHandler.getAllEntitiesFromProject + .onSecondCall().callsArgWith(1, null, @newDocs = []) + + @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: @entity_id, name:"old.tex", rev:4 }, @path) + @tpdsUpdateSender.moveEntity = sinon.stub() + @ProjectModel.findOneAndUpdate = sinon.stub().callsArgWith(3, null, @project) + @documentUpdaterHandler.updateProjectStructure = sinon.stub().callsArg(4) + + it "should should send the old and new project structure to the doc updater", (done) -> + @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, @userId, => + @documentUpdaterHandler.updateProjectStructure.calledWith( + @project_id, @userId, @oldDocs, @newDocs, + ).should.equal true + done() it "should update the name in mongo", (done)-> - - @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, => - @ProjectModel.update.calledWith({_id : @project_id}, {"$set":{"mongo.path.name":@newName}}).should.equal true + @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, @userId, => + @ProjectModel.findOneAndUpdate.calledWith({_id: @project_id}, {"$set":{"mongo.path.name": @newName}}, {"new": true}).should.equal true done() it "should send the update to the tpds", (done)-> - @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, => + @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, @userId, => @tpdsUpdateSender.moveEntity.calledWith({project_id:@project_id, startPath:@path.fileSystem, endPath:"/file/system/new.tex", project_name:@project.name, rev:4}).should.equal true done() From 607f0125fc02226dda0bd2dfd97f14b6b07b82db Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Mon, 6 Nov 2017 12:43:04 +0000 Subject: [PATCH 3/6] return project from _removeElementFromMongoArray and _putElement --- .../Project/ProjectEntityHandler.coffee | 17 ++--- .../Project/ProjectEntityHandlerTests.coffee | 65 ++++++++++--------- 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 03849762a2..2eb39d2f95 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -492,17 +492,15 @@ module.exports = ProjectEntityHandler = async.series jobs, callback - _removeElementFromMongoArray : (model, model_id, path, callback)-> - conditons = {_id:model_id} + _removeElementFromMongoArray : (model, model_id, path, callback = (err, project) ->)-> + conditions = {_id:model_id} update = {"$unset":{}} update["$unset"][path] = 1 - model.update conditons, update, {}, (err)-> + model.update conditions, update, {}, (err)-> pullUpdate = {"$pull":{}} nonArrayPath = path.slice(0, path.lastIndexOf(".")) pullUpdate["$pull"][nonArrayPath] = null - model.update conditons, pullUpdate, {}, (err)-> - if callback? - callback(err) + model.findOneAndUpdate conditions, pullUpdate, {"new": true}, callback _insertDeletedDocReference: (project_id, doc, callback = (error) ->) -> Project.update { @@ -539,8 +537,7 @@ module.exports = ProjectEntityHandler = countFolder project.rootFolder[0], callback - _putElement: (project, folder_id, element, type, callback = (err, path)->)-> - + _putElement: (project, folder_id, element, type, callback = (err, path, project)->)-> sanitizeTypeOfElement = (elementType)-> lastChar = elementType.slice -1 if lastChar != "s" @@ -581,11 +578,11 @@ module.exports = ProjectEntityHandler = update = "$push":{} update["$push"][mongopath] = element logger.log project_id: project._id, element_id: element._id, fileType: type, folder_id: folder_id, mongopath:mongopath, "adding element to project" - Project.update conditions, update, {}, (err)-> + Project.findOneAndUpdate conditions, update, {"new": true}, (err, project)-> if err? logger.err err: err, project_id: project._id, 'error saving in putElement project' return callback(err) - callback(err, {path:newPath}) + callback(err, {path:newPath}, project) confirmFolder = (project, folder_id, callback)-> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index 9263ebed9f..fa6fba40a7 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -361,23 +361,29 @@ describe 'ProjectEntityHandler', -> .calledWith(new Error("destination folder is a child folder of me")) .should.equal true - describe 'removing element from mongo array', -> - it 'should call update with log the path', (done)-> - mongoPath = "folders[0].folders[5]" - id = "12344" - firstUpdate = true - model = - update: (conditions, update, opts, callback)-> - if firstUpdate - conditions._id.should.equal id - update.$unset[mongoPath].should.equal 1 - firstUpdate = false - callback() - else - conditions._id.should.equal id - assert.deepEqual update, { '$pull': { 'folders[0]': null } } - done() - @ProjectEntityHandler._removeElementFromMongoArray model, id, mongoPath, -> + describe '_removeElementFromMongoArray ', -> + beforeEach -> + @mongoPath = "folders[0].folders[5]" + @id = "12344" + @project = 'a project' + @ProjectModel.update = sinon.stub().callsArg(3) + @ProjectModel.findOneAndUpdate = sinon.stub().callsArgWith(3, null, @project) + @ProjectEntityHandler._removeElementFromMongoArray @ProjectModel, @id, @mongoPath, @callback + + it 'should unset', -> + update = { '$unset': { } } + update['$unset'][@mongoPath] = 1 + @ProjectModel.update + .calledWith({ _id: @id }, update, {}) + .should.equal true + + it 'should pull', -> + @ProjectModel.findOneAndUpdate + .calledWith({ _id: @id }, { '$pull': { 'folders[0]': null } }, {'new': true}) + .should.equal true + + it 'should call the callback', -> + @callback.calledWith(null, @project).should.equal true describe 'getDoc', -> beforeEach -> @@ -1100,27 +1106,28 @@ describe 'ProjectEntityHandler', -> @path = mongo: "mongo.path", fileSystem: "/file/system/old.tex" @ProjectGetter.getProject.callsArgWith(2, null, @project) @projectLocator.findElement.callsArgWith(1, null, @folder, @path) - @ProjectUpdateStub.callsArgWith(3) - + @ProjectModel.findOneAndUpdate = sinon.stub().callsArgWith(3, null, @project) describe "updating the project", -> - - it "should use the correct mongo path", (done)-> @ProjectEntityHandler._putElement @project, @folder._id, @doc, "docs", (err)=> - @ProjectModel.update.args[0][0]._id.should.equal @project._id - assert.deepEqual @ProjectModel.update.args[0][1].$push[@path.mongo+".docs"], @doc + @ProjectModel.findOneAndUpdate.args[0][0]._id.should.equal @project._id + assert.deepEqual @ProjectModel.findOneAndUpdate.args[0][1].$push[@path.mongo+".docs"], @doc + done() + + it "should return the project in the callback", (done)-> + @ProjectEntityHandler._putElement @project, @folder._id, @doc, "docs", (err, path, project)=> + expect(project).to.equal @project done() it "should add an s onto the type if not included", (done)-> @ProjectEntityHandler._putElement @project, @folder._id, @doc, "doc", (err)=> - assert.deepEqual @ProjectModel.update.args[0][1].$push[@path.mongo+".docs"], @doc + assert.deepEqual @ProjectModel.findOneAndUpdate.args[0][1].$push[@path.mongo+".docs"], @doc done() - - it "should not call update if elemenet is null", (done)-> + it "should not call update if element is null", (done)-> @ProjectEntityHandler._putElement @project, @folder._id, null, "doc", (err)=> - @ProjectModel.update.called.should.equal false + @ProjectModel.findOneAndUpdate.called.should.equal false done() it "should default to root folder insert", (done)-> @@ -1132,11 +1139,9 @@ describe 'ProjectEntityHandler', -> doc = name:"something" @ProjectEntityHandler._putElement @project, @folder._id, doc, "doc", (err)=> - @ProjectModel.update.called.should.equal false + @ProjectModel.findOneAndUpdate.called.should.equal false done() - - describe "_countElements", -> beforeEach -> From 06116dc956989ac8466ffa8121d38a6a75b15d8b Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 2 Nov 2017 09:44:23 +0000 Subject: [PATCH 4/6] version moving entities --- .../Features/Editor/EditorController.coffee | 4 +- .../Editor/EditorHttpController.coffee | 3 +- .../Project/ProjectEntityHandler.coffee | 62 +++++++------- .../Editor/EditorControllerTests.coffee | 14 ++-- .../Editor/EditorHttpControllerTests.coffee | 12 +-- .../Project/ProjectEntityHandlerTests.coffee | 82 +++++++++++-------- 6 files changed, 94 insertions(+), 83 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 5026f62595..a43d6f0447 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -162,13 +162,13 @@ module.exports = EditorController = EditorRealTimeController.emitToRoom project_id, 'reciveEntityRename', entity_id, newName callback?() - moveEntity: (project_id, entity_id, folder_id, entityType, callback)-> + moveEntity: (project_id, entity_id, folder_id, entityType, userId, callback)-> Metrics.inc "editor.move-entity" LockManager.getLock project_id, (err)-> if err? logger.err err:err, project_id:project_id, "could not get lock for move entity" return callback(err) - ProjectEntityHandler.moveEntity project_id, entity_id, folder_id, entityType, => + ProjectEntityHandler.moveEntity project_id, entity_id, folder_id, entityType, userId, => if err? logger.err err:err, project_id:project_id, entity_id:entity_id, folder_id:folder_id, "error moving entity" return callback(err) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 63eeef70e9..e9bac7300d 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -125,7 +125,8 @@ module.exports = EditorHttpController = entity_id = req.params.entity_id entity_type = req.params.entity_type folder_id = req.body.folder_id - EditorController.moveEntity project_id, entity_id, folder_id, entity_type, (error) -> + user_id = AuthenticationController.getLoggedInUserId(req) + EditorController.moveEntity project_id, entity_id, folder_id, entity_type, user_id, (error) -> return next(error) if error? res.sendStatus 204 diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 2eb39d2f95..e519140b35 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -355,47 +355,49 @@ module.exports = ProjectEntityHandler = else callback() - moveEntity: (project_id, entity_id, folder_id, entityType, callback = (error) ->)-> + moveEntity: (project_id, entity_id, destFolderId, entityType, userId, callback = (error) ->)-> self = @ - destinationFolder_id = folder_id - logger.log entityType:entityType, entity_id:entity_id, project_id:project_id, folder_id:folder_id, "moving entity" + logger.log {entityType, entity_id, project_id, destFolderId}, "moving entity" if !entityType? - logger.err err: "No entityType set", project_id: project_id, entity_id: entity_id + logger.err {err: "No entityType set", project_id, entity_id} return callback("No entityType set") entityType = entityType.toLowerCase() ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project)=> return callback(err) if err? - projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (err, entity, path)-> + projectLocator.findElement {project, element_id: entity_id, type: entityType}, (err, entity, entityPath)-> return callback(err) if err? - - if entityType.match(/folder/) - ensureFolderIsNotMovedIntoChild = (callback = (error) ->) -> - projectLocator.findElement {project: project, element_id: folder_id, type:"folder"}, (err, destEntity, destPath) -> - logger.log destPath: destPath.fileSystem, folderPath: path.fileSystem, "checking folder is not moving into child folder" - if (destPath.fileSystem.slice(0, path.fileSystem.length) == path.fileSystem) - logger.log "destination is a child folder, aborting" - callback(new Error("destination folder is a child folder of me")) - else - callback() - else - ensureFolderIsNotMovedIntoChild = (callback = () ->) -> callback() - - ensureFolderIsNotMovedIntoChild (error) -> + self._checkValidMove project, entityType, entityPath, destFolderId, (error) -> return callback(error) if error? - self._removeElementFromMongoArray Project, project_id, path.mongo, (err)-> - return callback(err) if err? - # We've updated the project structure by removing the element, so must refresh it. - ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project)=> + ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs) => + return callback(error) if error? + self._removeElementFromMongoArray Project, project_id, entityPath.mongo, (err, newProject)-> return callback(err) if err? - ProjectEntityHandler._putElement project, destinationFolder_id, entity, entityType, (err, result)-> + ProjectEntityHandler._putElement newProject, destFolderId, entity, entityType, (err, result, newProject)-> return callback(err) if err? opts = - project_id:project_id - project_name:project.name - startPath:path.fileSystem - endPath:result.path.fileSystem, - rev:entity.rev - tpdsUpdateSender.moveEntity opts, callback + project_id: project_id + project_name: project.name + startPath: entityPath.fileSystem + endPath: result.path.fileSystem, + rev: entity.rev + tpdsUpdateSender.moveEntity opts + ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs) => + return callback(error) if error? + documentUpdaterHandler = require('../../Features/DocumentUpdater/DocumentUpdaterHandler') + documentUpdaterHandler.updateProjectStructure project_id, userId, oldDocs, newDocs, callback + + _checkValidMove: (project, entityType, entityPath, destFolderId, callback = (error) ->) -> + return callback() if !entityType.match(/folder/) + + projectLocator.findElement { project, element_id: destFolderId, type:"folder"}, (err, destEntity, destFolderPath) -> + return callback(err) if err? + logger.log destFolderPath: destFolderPath.fileSystem, folderPath: entityPath.fileSystem, "checking folder is not moving into child folder" + isNestedFolder = destFolderPath.fileSystem.slice(0, entityPath.fileSystem.length) == entityPath.fileSystem + if isNestedFolder + callback(new Error("destination folder is a child folder of me")) + else + callback() + deleteEntity: (project_id, entity_id, entityType, callback = (error) ->)-> self = @ diff --git a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee index 2d9e344c8e..ae5e8d79e3 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee @@ -508,34 +508,32 @@ describe "EditorController", -> done() describe "moveEntity", -> - beforeEach -> - @err = "errro" @entity_id = "entity_id_here" @entityType = "doc" @folder_id = "313dasd21dasdsa" - @ProjectEntityHandler.moveEntity = sinon.stub().callsArgWith(4, @err) + @ProjectEntityHandler.moveEntity = sinon.stub().callsArg(5) @EditorRealTimeController.emitToRoom = sinon.stub() @LockManager.releaseLock.callsArgWith(1) @LockManager.getLock.callsArgWith(1) it "should call the ProjectEntityHandler", (done)-> - @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => - @ProjectEntityHandler.moveEntity.calledWith(@project_id, @entity_id, @folder_id, @entityType).should.equal true + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => + @ProjectEntityHandler.moveEntity.calledWith(@project_id, @entity_id, @folder_id, @entityType, @user_id).should.equal true done() it "should take the lock", (done)-> - @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => @LockManager.getLock.calledWith(@project_id).should.equal true done() it "should release the lock", (done)-> - @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => @LockManager.releaseLock.calledWith(@project_id).should.equal true done() it "should emit the update to the room", (done)-> - @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => @EditorRealTimeController.emitToRoom.calledWith(@project_id, 'reciveEntityMove', @entity_id, @folder_id).should.equal true done() diff --git a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee index 2567230a36..2df4af72ec 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee @@ -25,6 +25,8 @@ describe "EditorHttpController", -> @doc_id = "mock-doc-id" @user_id = "mock-user-id" @parent_folder_id = "mock-folder-id" + @userId = 1234 + @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @req = {} @res = send: sinon.stub() @@ -265,8 +267,6 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = name: @name = "new-name" - @userId = 1234 - @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @EditorController.renameEntity = sinon.stub().callsArg(5) @EditorHttpController.renameEntity @req, @res @@ -286,8 +286,6 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = name: @name = "EDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOTEDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOT" - @userId = 1234 - @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @EditorController.renameEntity = sinon.stub().callsArg(4) @EditorHttpController.renameEntity @req, @res @@ -302,8 +300,6 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = name: @name = "" - @userId = 1234 - @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) @EditorController.renameEntity = sinon.stub().callsArg(4) @EditorHttpController.renameEntity @req, @res @@ -318,12 +314,12 @@ describe "EditorHttpController", -> entity_type: @entity_type = "entity-type" @req.body = folder_id: @folder_id = "folder-id-123" - @EditorController.moveEntity = sinon.stub().callsArg(4) + @EditorController.moveEntity = sinon.stub().callsArg(5) @EditorHttpController.moveEntity @req, @res it "should call EditorController.moveEntity", -> @EditorController.moveEntity - .calledWith(@project_id, @entity_id, @folder_id, @entity_type) + .calledWith(@project_id, @entity_id, @folder_id, @entity_type, @userId) .should.equal true it "should send back a success response", -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index fa6fba40a7..c4902f2ff0 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -14,6 +14,7 @@ describe 'ProjectEntityHandler', -> doc_id = '4eecb1c1bffa66588e0000a2' folder_id = "4eecaffcbffa66588e000008" rootFolderId = "4eecaffcbffa66588e000007" + userId = 1234 beforeEach -> @FileStoreHandler = @@ -235,10 +236,16 @@ describe 'ProjectEntityHandler', -> @pathAfterMove = { fileSystem: "/somewhere/else.txt" } - @ProjectEntityHandler._removeElementFromMongoArray = sinon.stub().callsArg(3) + @ProjectEntityHandler._removeElementFromMongoArray = sinon.stub().callsArgWith(3, null, @project) @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, path: @pathAfterMove) @ProjectGetter.getProject.callsArgWith(2, null, @project) - @tpdsUpdateSender.moveEntity = sinon.stub().callsArg(1) + @tpdsUpdateSender.moveEntity = sinon.stub() + @documentUpdaterHandler.updateProjectStructure = sinon.stub().callsArg(4) + @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() + @ProjectEntityHandler.getAllEntitiesFromProject + .onFirstCall().callsArgWith(1, null, @oldDocs = []) + @ProjectEntityHandler.getAllEntitiesFromProject + .onSecondCall().callsArgWith(1, null, @newDocs = []) describe "moving a doc", -> beforeEach (done) -> @@ -246,14 +253,26 @@ describe 'ProjectEntityHandler', -> @doc = {lines:["1234","312343d"], rev: "1234"} @path = { mongo:"folders[0]" - fileSystem:"/somewhere.txt" + fileSystem:"/old_folder/somewhere.txt" } - @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, @path) - @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", done + @destFolder = { name: "folder" } + @destFolderPath = { + mongo: "folders[0]" + fileSystem: "/dest_folder" + } + @projectLocator.findElement = sinon.stub() + @projectLocator.findElement.withArgs({project: @project, element_id: @docId, type: 'docs'}) + .callsArgWith(1, null, @doc, @path) + @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", userId, done - it 'should find the project then element', -> + it 'should find the doc to move', -> @projectLocator.findElement.calledWith({element_id: @docId, type: "docs", project: @project }).should.equal true + it "should should send the update to the doc updater", -> + @documentUpdaterHandler.updateProjectStructure + .calledWith(project_id, userId, @oldDocs, @newDocs) + .should.equal true + it 'should remove the element from its current position', -> @ProjectEntityHandler._removeElementFromMongoArray .calledWith(@ProjectModel, project_id, @path.mongo ).should.equal true @@ -278,28 +297,19 @@ describe 'ProjectEntityHandler', -> @move_to_folder_id = "folder-to-move-to" @folder = { name: "folder" } @folder_to_move_to = { name: "folder to move to" } - @path = { - mongo: "folders[0]" - fileSystem: "/somewhere.txt" - } - @pathToMoveTo = { - mongo: "folders[0]" - fileSystem: "/somewhere.txt" - } - @projectLocator.findElement = (options, callback) => - if options.element_id == @folder_id - callback(null, @folder, @path) - else if options.element_id == @move_to_folder_id - callback(null, @folder_to_move_to, @pathToMoveTo) - else - console.log "UNKNOWN ID", options - sinon.spy @projectLocator, "findElement" + @path = { mongo:"folders[0]" } + @pathToMoveTo = { mongo: "folders[0]" } + @projectLocator.findElement = sinon.stub() + @projectLocator.findElement.withArgs({project: @project, element_id: @folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder, @path) + @projectLocator.findElement.withArgs({project: @project, element_id: @move_to_folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder_to_move_to, @pathToMoveTo) describe "when the destination folder is outside the moving folder", -> beforeEach (done) -> - @path.fileSystem = "/one/directory" - @pathToMoveTo.fileSystem = "/another/directory" - @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", done + @path.fileSystem = "/one/src_dir" + @pathToMoveTo.fileSystem = "/two/dest_dir" + @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, done it 'should find the project then element', -> @projectLocator.findElement @@ -310,6 +320,11 @@ describe 'ProjectEntityHandler', -> }) .should.equal true + it "should should send the update to the doc updater", -> + @documentUpdaterHandler.updateProjectStructure + .calledWith(project_id, userId, @oldDocs, @newDocs) + .should.equal true + it 'should remove the element from its current position', -> @ProjectEntityHandler._removeElementFromMongoArray .calledWith( @@ -345,9 +360,9 @@ describe 'ProjectEntityHandler', -> @path.fileSystem = "/one/two" @pathToMoveTo.fileSystem = "/one/two/three" @callback = sinon.stub() - @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", @callback + @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, @callback - it 'should find the folder we are moving to as well element', -> + it 'should find the folder we are moving to element', -> @projectLocator.findElement .calledWith({ element_id: @move_to_folder_id, @@ -982,7 +997,6 @@ describe 'ProjectEntityHandler', -> @entityType = "doc" @newName = "new.tex" @path = mongo: "mongo.path", fileSystem: "/file/system/old.tex" - @userId = 1234 @ProjectGetter.getProject.callsArgWith(2, null, @project) @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() @@ -997,20 +1011,20 @@ describe 'ProjectEntityHandler', -> @documentUpdaterHandler.updateProjectStructure = sinon.stub().callsArg(4) it "should should send the old and new project structure to the doc updater", (done) -> - @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, @userId, => + @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, => @documentUpdaterHandler.updateProjectStructure.calledWith( - @project_id, @userId, @oldDocs, @newDocs, + project_id, userId, @oldDocs, @newDocs, ).should.equal true done() it "should update the name in mongo", (done)-> - @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, @userId, => - @ProjectModel.findOneAndUpdate.calledWith({_id: @project_id}, {"$set":{"mongo.path.name": @newName}}, {"new": true}).should.equal true + @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, => + @ProjectModel.findOneAndUpdate.calledWith({_id: project_id}, {"$set":{"mongo.path.name": @newName}}, {"new": true}).should.equal true done() it "should send the update to the tpds", (done)-> - @ProjectEntityHandler.renameEntity @project_id, @entity_id, @entityType, @newName, @userId, => - @tpdsUpdateSender.moveEntity.calledWith({project_id:@project_id, startPath:@path.fileSystem, endPath:"/file/system/new.tex", project_name:@project.name, rev:4}).should.equal true + @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, => + @tpdsUpdateSender.moveEntity.calledWith({project_id:project_id, startPath:@path.fileSystem, endPath:"/file/system/new.tex", project_name:@project.name, rev:4}).should.equal true done() describe "_insertDeletedDocReference", -> From 3ce03a40f922efdd154a1416ea898b58fc6bd165 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Mon, 6 Nov 2017 15:11:33 +0000 Subject: [PATCH 5/6] send both doc update and file updates to doc-updater --- .../DocumentUpdaterHandler.coffee | 36 +++++++++++-------- .../Project/ProjectEntityHandler.coffee | 13 +++---- .../DocumentUpdaterHandlerTests.coffee | 10 +++--- .../Project/ProjectEntityHandlerTests.coffee | 26 ++++++++------ 4 files changed, 49 insertions(+), 36 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index 6ec7613cfd..d8613680a8 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -204,29 +204,19 @@ module.exports = DocumentUpdaterHandler = logger.error {project_id, doc_id, thread_id}, "doc updater returned a non-success status code: #{res.statusCode}" callback new Error("doc updater returned a non-success status code: #{res.statusCode}") - updateProjectStructure : (project_id, user_id, oldDocs, newDocs, callback = (error) ->)-> + updateProjectStructure : (project_id, userId, oldDocs, newDocs, oldFiles, newFiles, callback = (error) ->)-> return callback() if !settings.apis.project_history?.enabled - docRenameUpdates = [] - - for oldDoc in oldDocs - newDoc = _.find newDocs, (newDoc) -> - newDoc.doc._id.toString() == oldDoc.doc._id.toString() - if newDoc.path != oldDoc.path - docRenameUpdates.push - id: oldDoc.doc._id - pathname: oldDoc.path - newPathname: newDoc.path + docUpdates = DocumentUpdaterHandler._getRenameUpdates('doc', oldDocs, newDocs) + fileUpdates = DocumentUpdaterHandler._getRenameUpdates('file', oldFiles, newFiles) timer = new metrics.Timer("set-document") url = "#{settings.apis.documentupdater.url}/project/#{project_id}" body = url: url - json: - updates: docRenameUpdates - user_id: user_id + json: { docUpdates, fileUpdates, userId } - return callback() if docRenameUpdates.length < 1 + return callback() if (docUpdates.length + fileUpdates.length) < 1 request.post body, (error, res, body)-> timer.done() @@ -240,6 +230,22 @@ module.exports = DocumentUpdaterHandler = logger.error {project_id, url}, "doc updater returned a non-success status code: #{res.statusCode}" callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + _getRenameUpdates: (entityType, oldEntities, newEntities) -> + updates = [] + + for oldEntity in oldEntities + id = oldEntity[entityType]._id + newEntity = _.find newEntities, (newEntity) -> + newEntity[entityType]._id.toString() == id.toString() + + if newEntity.path != oldEntity.path + updates.push + id: id + pathname: oldEntity.path + newPathname: newEntity.path + + updates + PENDINGUPDATESKEY = "PendingUpdates" DOCLINESKEY = "doclines" DOCIDSWITHPENDINGUPDATES = "DocsWithPendingUpdates" diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index e519140b35..30f3789778 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -368,7 +368,7 @@ module.exports = ProjectEntityHandler = return callback(err) if err? self._checkValidMove project, entityType, entityPath, destFolderId, (error) -> return callback(error) if error? - ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs) => + ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? self._removeElementFromMongoArray Project, project_id, entityPath.mongo, (err, newProject)-> return callback(err) if err? @@ -381,10 +381,11 @@ module.exports = ProjectEntityHandler = endPath: result.path.fileSystem, rev: entity.rev tpdsUpdateSender.moveEntity opts - ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs) => + ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs, newFiles + ) => return callback(error) if error? documentUpdaterHandler = require('../../Features/DocumentUpdater/DocumentUpdaterHandler') - documentUpdaterHandler.updateProjectStructure project_id, userId, oldDocs, newDocs, callback + documentUpdaterHandler.updateProjectStructure project_id, userId, oldDocs, newDocs, oldFiles, newFiles, callback _checkValidMove: (project, entityType, entityPath, destFolderId, callback = (error) ->) -> return callback() if !entityType.match(/folder/) @@ -427,7 +428,7 @@ module.exports = ProjectEntityHandler = entityType = entityType.toLowerCase() ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (error, project)=> return callback(error) if error? - ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs) => + ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, path)=> return callback(error) if error? @@ -439,10 +440,10 @@ module.exports = ProjectEntityHandler = tpdsUpdateSender.moveEntity({project_id:project_id, startPath:path.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) Project.findOneAndUpdate conditions, update, { "new": true}, (error, newProject) -> return callback(error) if error? - ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs) => + ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs, newFiles) => return callback(error) if error? documentUpdaterHandler = require('../../Features/DocumentUpdater/DocumentUpdaterHandler') - documentUpdaterHandler.updateProjectStructure project_id, userId, oldDocs, newDocs, callback + documentUpdaterHandler.updateProjectStructure project_id, userId, oldDocs, newDocs, oldFiles, newFiles, callback _cleanUpEntity: (project, entity, entityType, callback = (error) ->) -> if(entityType.indexOf("file") != -1) diff --git a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 1a9c12a754..ec81ae1c2e 100644 --- a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -401,13 +401,15 @@ describe 'DocumentUpdaterHandler', -> { path: '/old_a', doc: _id: new ObjectId(@docIdA.toString()) } { path: '/new_b', doc: _id: new ObjectId(@docIdB.toString()) } ] + @oldFiles = [] + @newFiles = [] describe "with project history disabled", -> beforeEach -> @settings.apis.project_history.enabled = false @request.post = sinon.stub() - @handler.updateProjectStructure @project_id, @user_id, @oldDocs, @newDocs, @callback + @handler.updateProjectStructure @project_id, @user_id, @oldDocs, @newDocs, @oldFiles, @newFiles, @callback it 'does not make a web request', -> @request.post.called.should.equal false @@ -419,10 +421,10 @@ describe 'DocumentUpdaterHandler', -> beforeEach -> @settings.apis.project_history.enabled = true @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") - @handler.updateProjectStructure @project_id, @user_id, @oldDocs, @newDocs, @callback + @handler.updateProjectStructure @project_id, @user_id, @oldDocs, @newDocs, @oldFiles, @newFiles, @callback it 'should send the structure update to the document updater', -> - updates = [ + docUpdates = [ id: @docIdB, pathname: "/old_b" newPathname: "/new_b" @@ -430,7 +432,7 @@ describe 'DocumentUpdaterHandler', -> url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}" @request.post - .calledWith(url: url, json: {updates, @user_id}) + .calledWith(url: url, json: {docUpdates, fileUpdates: [], userId: @user_id}) .should.equal true it "should call the callback with no error", -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index c4902f2ff0..dc723bf8bd 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -240,12 +240,14 @@ describe 'ProjectEntityHandler', -> @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, path: @pathAfterMove) @ProjectGetter.getProject.callsArgWith(2, null, @project) @tpdsUpdateSender.moveEntity = sinon.stub() - @documentUpdaterHandler.updateProjectStructure = sinon.stub().callsArg(4) + @documentUpdaterHandler.updateProjectStructure = sinon.stub().callsArg(6) @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() @ProjectEntityHandler.getAllEntitiesFromProject - .onFirstCall().callsArgWith(1, null, @oldDocs = []) + .onFirstCall() + .callsArgWith(1, null, @oldDocs = ['old-doc'], @oldFiles = ['old-file']) @ProjectEntityHandler.getAllEntitiesFromProject - .onSecondCall().callsArgWith(1, null, @newDocs = []) + .onSecondCall() + .callsArgWith(1, null, @newDocs = ['new-doc'], @newFiles = ['new-file']) describe "moving a doc", -> beforeEach (done) -> @@ -270,7 +272,7 @@ describe 'ProjectEntityHandler', -> it "should should send the update to the doc updater", -> @documentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, @oldDocs, @newDocs) + .calledWith(project_id, userId, @oldDocs, @newDocs, @oldFiles, @newFiles) .should.equal true it 'should remove the element from its current position', -> @@ -322,7 +324,7 @@ describe 'ProjectEntityHandler', -> it "should should send the update to the doc updater", -> @documentUpdaterHandler.updateProjectStructure - .calledWith(project_id, userId, @oldDocs, @newDocs) + .calledWith(project_id, userId, @oldDocs, @newDocs, @oldFiles, @newFiles) .should.equal true it 'should remove the element from its current position', -> @@ -1001,20 +1003,22 @@ describe 'ProjectEntityHandler', -> @ProjectGetter.getProject.callsArgWith(2, null, @project) @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() @ProjectEntityHandler.getAllEntitiesFromProject - .onFirstCall().callsArgWith(1, null, @oldDocs = []) + .onFirstCall() + .callsArgWith(1, null, @oldDocs = ['old-doc'], @oldFiles = ['old-file']) @ProjectEntityHandler.getAllEntitiesFromProject - .onSecondCall().callsArgWith(1, null, @newDocs = []) + .onSecondCall() + .callsArgWith(1, null, @newDocs = ['new-doc'], @newFiles = ['new-file']) @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: @entity_id, name:"old.tex", rev:4 }, @path) @tpdsUpdateSender.moveEntity = sinon.stub() @ProjectModel.findOneAndUpdate = sinon.stub().callsArgWith(3, null, @project) - @documentUpdaterHandler.updateProjectStructure = sinon.stub().callsArg(4) + @documentUpdaterHandler.updateProjectStructure = sinon.stub().callsArg(6) it "should should send the old and new project structure to the doc updater", (done) -> @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, => - @documentUpdaterHandler.updateProjectStructure.calledWith( - project_id, userId, @oldDocs, @newDocs, - ).should.equal true + @documentUpdaterHandler.updateProjectStructure + .calledWith(project_id, userId, @oldDocs, @newDocs, @oldFiles, @newFiles) + .should.equal true done() it "should update the name in mongo", (done)-> From 086a0829e3456aa2cf844d547dfb5df16112d4d8 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Tue, 7 Nov 2017 10:06:12 +0000 Subject: [PATCH 6/6] use self rather than ProtectEntityHandler --- .../app/coffee/Features/Project/ProjectEntityHandler.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 30f3789778..abd5c7f3d8 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -368,11 +368,11 @@ module.exports = ProjectEntityHandler = return callback(err) if err? self._checkValidMove project, entityType, entityPath, destFolderId, (error) -> return callback(error) if error? - ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => + self.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? self._removeElementFromMongoArray Project, project_id, entityPath.mongo, (err, newProject)-> return callback(err) if err? - ProjectEntityHandler._putElement newProject, destFolderId, entity, entityType, (err, result, newProject)-> + self._putElement newProject, destFolderId, entity, entityType, (err, result, newProject)-> return callback(err) if err? opts = project_id: project_id @@ -381,7 +381,7 @@ module.exports = ProjectEntityHandler = endPath: result.path.fileSystem, rev: entity.rev tpdsUpdateSender.moveEntity opts - ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs, newFiles + self.getAllEntitiesFromProject newProject, (error, newDocs, newFiles ) => return callback(error) if error? documentUpdaterHandler = require('../../Features/DocumentUpdater/DocumentUpdaterHandler')