From e5e75a8ccb0685fa7a36b3c43cd9d36823c15705 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 14 Dec 2017 13:06:27 +0000 Subject: [PATCH 1/9] update DocumentUpdaterHandler.updateProjectStructure to support entity deletions --- .../DocumentUpdater/DocumentUpdaterHandler.coffee | 15 ++++++++++++--- .../DocumentUpdaterHandlerTests.coffee | 13 ++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index f7dd9f3e17..f1336a7f77 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -207,8 +207,8 @@ module.exports = DocumentUpdaterHandler = updateProjectStructure : (project_id, userId, changes, callback = (error) ->)-> return callback() if !settings.apis.project_history?.enabled - docUpdates = DocumentUpdaterHandler._getRenameUpdates('doc', changes.oldDocs, changes.newDocs) - fileUpdates = DocumentUpdaterHandler._getRenameUpdates('file', changes.oldFiles, changes.newFiles) + docUpdates = DocumentUpdaterHandler._getUpdates('doc', changes.oldDocs, changes.newDocs) + fileUpdates = DocumentUpdaterHandler._getUpdates('file', changes.oldFiles, changes.newFiles) timer = new metrics.Timer("set-document") url = "#{settings.apis.documentupdater.url}/project/#{project_id}" @@ -230,7 +230,7 @@ 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) -> + _getUpdates: (entityType, oldEntities, newEntities) -> oldEntities ||= [] newEntities ||= [] updates = [] @@ -255,6 +255,15 @@ module.exports = DocumentUpdaterHandler = pathname: oldEntity.path newPathname: newEntity.path + for id, oldEntity of oldEntitiesHash + newEntity = newEntitiesHash[id] + + if !newEntity? + # entity deleted + updates.push + id: id + pathname: oldEntity.path + updates PENDINGUPDATESKEY = "PendingUpdates" diff --git a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 14ccaa3a33..266de9bfb7 100644 --- a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -478,14 +478,21 @@ describe 'DocumentUpdaterHandler', -> .should.equal true done() - describe "when a doc has been deleted", -> - it 'should do nothing', (done) -> + describe "when an entity has been deleted", -> + it 'should end the structure update to the document updater', (done) -> @docId = new ObjectId() @changes = oldDocs: [ { path: '/foo', docLines: 'a\nb', doc: _id: @docId } ] + docUpdates = [ + id: @docId.toString(), + pathname: "/foo", + ] + @handler.updateProjectStructure @project_id, @user_id, @changes, () => - @request.post.called.should.equal false + @request.post + .calledWith(url: @url, json: {docUpdates, fileUpdates: [], userId: @user_id}) + .should.equal true done() From 81c061c6a7ce3236213d67ca491f5b925742602d Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 14 Dec 2017 14:39:27 +0000 Subject: [PATCH 2/9] acceptance test moving entities --- .../DocumentUpdaterHandler.coffee | 1 + .../coffee/ProjectStructureTests.coffee | 89 +++++++++++++++++++ .../DocumentUpdaterHandlerTests.coffee | 3 +- 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index f1336a7f77..bbd7ca4919 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -263,6 +263,7 @@ module.exports = DocumentUpdaterHandler = updates.push id: id pathname: oldEntity.path + newPathname: '' updates diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index e54d4fac9d..92fcd2e34e 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -91,6 +91,7 @@ describe "ProjectStructureChanges", -> throw error if error? if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to add doc #{res.statusCode}") + @example_doc_id = body._id done() it "should version the doc added", -> @@ -162,6 +163,8 @@ describe "ProjectStructureChanges", -> if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to upload file #{res.statusCode}") + @example_file_id = JSON.parse(body).entity_id + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] @@ -199,6 +202,92 @@ describe "ProjectStructureChanges", -> done() + describe "moving entities", -> + before (done) -> + @owner.request.post { + uri: "project/#{@example_project_id}/folder", + formData: + name: 'foo' + }, (error, res, body) => + throw error if error? + @example_folder_id_1 = JSON.parse(body)._id + done() + + beforeEach () -> + MockDocUpdaterApi.clearProjectStructureUpdates() + + it "should version moving a doc", (done) -> + @owner.request.post { + uri: "project/#{@example_project_id}/Doc/#{@example_doc_id}/move", + json: + folder_id: @example_folder_id_1 + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to move doc #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/new.tex") + expect(update.newPathname).to.equal("/foo/new.tex") + + done() + + it "should version moving a file", (done) -> + @owner.request.post { + uri: "project/#{@example_project_id}/File/#{@example_file_id}/move", + json: + folder_id: @example_folder_id_1 + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to move file #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/1pixel.png") + expect(update.newPathname).to.equal("/foo/1pixel.png") + + done() + + it "should version moving a folder", (done) -> + @owner.request.post { + uri: "project/#{@example_project_id}/folder", + formData: + name: 'bar' + }, (error, res, body) => + throw error if error? + example_folder_id_2 = JSON.parse(body)._id + + @owner.request.post { + uri: "project/#{@example_project_id}/Folder/#{@example_folder_id_1}/move", + json: + folder_id: example_folder_id_2 + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to move folder #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/foo/new.tex") + expect(update.newPathname).to.equal("/bar/foo/new.tex") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/foo/1pixel.png") + expect(update.newPathname).to.equal("/bar/foo/1pixel.png") + + done() + describe "tpds", -> before (done) -> @tpds_project_name = "tpds-project-#{new ObjectId().toString()}" diff --git a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 266de9bfb7..111d2ace68 100644 --- a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -487,7 +487,8 @@ describe 'DocumentUpdaterHandler', -> docUpdates = [ id: @docId.toString(), - pathname: "/foo", + pathname: '/foo', + newPathname: '' ] @handler.updateProjectStructure @project_id, @user_id, @changes, () => From 475e84b0390cc8c1b8872abf6c9fef9443aef6de Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 14 Dec 2017 16:03:43 +0000 Subject: [PATCH 3/9] version entity deletions in ProjectEntityHandler --- .../Project/ProjectEntityHandler.coffee | 33 ++++++----- .../Project/ProjectEntityHandlerTests.coffee | 57 ++++++++++++------- 2 files changed, 58 insertions(+), 32 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 446786fa57..4ff3818516 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -423,7 +423,7 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path)=> return callback(error) if error? - ProjectEntityHandler._cleanUpEntity project, entity, entityType, (error) -> + ProjectEntityHandler._cleanUpEntity project, entity, entityType, path.fileSystem, (error) -> return callback(error) if error? tpdsUpdateSender.deleteEntity project_id:project_id, path:path.fileSystem, project_name:project.name, (error) -> return callback(error) if error? @@ -456,17 +456,17 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback - _cleanUpEntity: (project, entity, entityType, callback = (error) ->) -> + _cleanUpEntity: (project, entity, entityType, path, callback = (error) ->) -> if(entityType.indexOf("file") != -1) - ProjectEntityHandler._cleanUpFile project, entity, callback + ProjectEntityHandler._cleanUpFile project, entity, path, callback else if (entityType.indexOf("doc") != -1) - ProjectEntityHandler._cleanUpDoc project, entity, callback + ProjectEntityHandler._cleanUpDoc project, entity, path, callback else if (entityType.indexOf("folder") != -1) - ProjectEntityHandler._cleanUpFolder project, entity, callback + ProjectEntityHandler._cleanUpFolder project, entity, path, callback else callback() - _cleanUpDoc: (project, doc, callback = (error) ->) -> + _cleanUpDoc: (project, doc, path, callback = (error) ->) -> project_id = project._id.toString() doc_id = doc._id.toString() unsetRootDocIfRequired = (callback) => @@ -483,26 +483,33 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocstoreManager.deleteDoc project_id, doc_id, (error) -> return callback(error) if error? - callback() + changes = oldDocs: [ {doc, path} ] + DocumentUpdaterHandler.updateProjectStructure project_id, null, changes, callback - _cleanUpFile: (project, file, callback = (error) ->) -> + _cleanUpFile: (project, file, path, callback = (error) ->) -> project_id = project._id.toString() file_id = file._id.toString() - FileStoreHandler.deleteFile project_id, file_id, callback + FileStoreHandler.deleteFile project_id, file_id, (error) -> + return callback(error) if error? + changes = oldFiles: [ {file, path} ] + DocumentUpdaterHandler.updateProjectStructure project_id, null, changes, callback - _cleanUpFolder: (project, folder, callback = (error) ->) -> + _cleanUpFolder: (project, folder, folderPath, callback = (error) ->) -> jobs = [] for doc in folder.docs do (doc) -> - jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, doc, callback + docPath = path.join(folderPath, doc.name) + jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, doc, docPath, callback for file in folder.fileRefs do (file) -> - jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, file, callback + filePath = path.join(folderPath, file.name) + jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, file, filePath, callback for childFolder in folder.folders do (childFolder) -> - jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, childFolder, callback + folderPath = path.join(folderPath, childFolder.name) + jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, childFolder, folderPath, callback async.series jobs, callback diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index f62690e226..faaaa146da 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -157,7 +157,7 @@ describe 'ProjectEntityHandler', -> @ProjectGetter.getProject.callsArgWith(2, null, @project) @tpdsUpdateSender.deleteEntity = sinon.stub().callsArg(1) @ProjectEntityHandler._removeElementFromMongoArray = sinon.stub().callsArg(3) - @ProjectEntityHandler._cleanUpEntity = sinon.stub().callsArg(3) + @ProjectEntityHandler._cleanUpEntity = sinon.stub().callsArg(4) @path = mongo: "mongo.path", fileSystem: "/file/system/path" @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: entity_id }, @path) @@ -182,7 +182,7 @@ describe 'ProjectEntityHandler', -> it "should clean up the entity from the rest of the system", -> @ProjectEntityHandler._cleanUpEntity - .calledWith(@project, @entity, @type) + .calledWith(@project, @entity, @type, @path.fileSystem) .should.equal true describe "_cleanUpEntity", -> @@ -193,7 +193,9 @@ describe 'ProjectEntityHandler', -> describe "a file", -> beforeEach (done) -> - @ProjectEntityHandler._cleanUpEntity @project, _id: @entity_id, 'file', done + @path = "/file/system/path.png" + @entity = _id: @entity_id + @ProjectEntityHandler._cleanUpEntity @project, @entity, 'file', @path, done it "should delete the file from FileStoreHandler", -> @FileStoreHandler.deleteFile.calledWith(project_id, @entity_id).should.equal true @@ -201,38 +203,48 @@ describe 'ProjectEntityHandler', -> it "should not attempt to delete from the document updater", -> @documentUpdaterHandler.deleteDoc.called.should.equal false + it "should should send the update to the doc updater", -> + oldFiles = [ file: @entity, path: @path ] + @documentUpdaterHandler.updateProjectStructure + .calledWith(project_id, null, {oldFiles}) + .should.equal true + describe "a doc", -> beforeEach (done) -> - @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(2) - @ProjectEntityHandler._cleanUpEntity @project, @entity = {_id: @entity_id}, 'doc', done + @path = "/file/system/path.tex" + @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(3) + @entity = {_id: @entity_id} + @ProjectEntityHandler._cleanUpEntity @project, @entity, 'doc', @path, done it "should clean up the doc", -> @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, @entity) + .calledWith(@project, @entity, @path) .should.equal true describe "a folder", -> beforeEach (done) -> @folder = folders: [ - fileRefs: [ @file1 = {_id: "file-id-1" } ] - docs: [ @doc1 = { _id: "doc-id-1" } ] + name: "subfolder" + fileRefs: [ @file1 = { _id: "file-id-1", name: "file-name-1"} ] + docs: [ @doc1 = { _id: "doc-id-1", name: "doc-name-1" } ] folders: [] ] - fileRefs: [ @file2 = { _id: "file-id-2" } ] - docs: [ @doc2 = { _id: "doc-id-2" } ] + fileRefs: [ @file2 = { _id: "file-id-2", name: "file-name-2" } ] + docs: [ @doc2 = { _id: "doc-id-2", name: "doc-name-2" } ] - @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(2) - @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(2) - @ProjectEntityHandler._cleanUpEntity @project, @folder, "folder", done + @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(3) + @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(3) + path = "/folder" + @ProjectEntityHandler._cleanUpEntity @project, @folder, "folder", path, done it "should clean up all sub files", -> - @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file1).should.equal true - @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file2).should.equal true + @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file1, "/folder/subfolder/file-name-1").should.equal true + @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file2, "/folder/file-name-2").should.equal true it "should clean up all sub docs", -> - @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc1).should.equal true - @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc2).should.equal true + @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc1, "/folder/subfolder/doc-name-1").should.equal true + @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc2, "/folder/doc-name-2").should.equal true describe 'moveEntity', -> beforeEach -> @@ -1116,6 +1128,7 @@ describe 'ProjectEntityHandler', -> @doc = _id: ObjectId() name: "test.tex" + @path = "/path/to/doc" @ProjectEntityHandler.unsetRootDoc = sinon.stub().callsArg(1) @ProjectEntityHandler._insertDeletedDocReference = sinon.stub().callsArg(2) @documentUpdaterHandler.deleteDoc = sinon.stub().callsArg(2) @@ -1125,7 +1138,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is the root doc", -> beforeEach -> @project.rootDoc_id = @doc._id - @ProjectEntityHandler._cleanUpDoc @project, @doc, @callback + @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, @callback it "should unset the root doc", -> @ProjectEntityHandler.unsetRootDoc @@ -1146,13 +1159,19 @@ describe 'ProjectEntityHandler', -> .calledWith(project_id, @doc._id.toString()) .should.equal true + it "should should send the update to the doc updater", -> + oldDocs = [ doc: @doc, path: @path ] + @documentUpdaterHandler.updateProjectStructure + .calledWith(project_id, null, {oldDocs}) + .should.equal true + it "should call the callback", -> @callback.called.should.equal true describe "when the doc is not the root doc", -> beforeEach -> @project.rootDoc_id = ObjectId() - @ProjectEntityHandler._cleanUpDoc @project, @doc, @callback + @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, @callback it "should not unset the root doc", -> @ProjectEntityHandler.unsetRootDoc.called.should.equal false From 99a52d48c8bc32c1685b2a411e98c5c929493f28 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 14 Dec 2017 16:03:57 +0000 Subject: [PATCH 4/9] acceptance test versioning entity deletions --- .../coffee/ProjectStructureTests.coffee | 32 +++++++++++++++++-- .../coffee/helpers/MockDocUpdaterApi.coffee | 3 ++ .../coffee/helpers/MockDocstoreApi.coffee | 10 ++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index 92fcd2e34e..0f8086647f 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -261,12 +261,12 @@ describe "ProjectStructureChanges", -> name: 'bar' }, (error, res, body) => throw error if error? - example_folder_id_2 = JSON.parse(body)._id + @example_folder_id_2 = JSON.parse(body)._id @owner.request.post { uri: "project/#{@example_project_id}/Folder/#{@example_folder_id_1}/move", json: - folder_id: example_folder_id_2 + folder_id: @example_folder_id_2 }, (error, res, body) => throw error if error? if res.statusCode < 200 || res.statusCode >= 300 @@ -288,6 +288,34 @@ describe "ProjectStructureChanges", -> done() + describe "deleting entities", -> + beforeEach () -> + MockDocUpdaterApi.clearProjectStructureUpdates() + + it "should version deleting a folder", (done) -> + @owner.request.delete { + uri: "project/#{@example_project_id}/Folder/#{@example_folder_id_2}", + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to delete folder #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + expect(updates.length).to.equal(1) + update = updates[0] + #expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/bar/foo/new.tex") + expect(update.newPathname).to.equal("") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + expect(updates.length).to.equal(1) + update = updates[0] + #expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/bar/foo/1pixel.png") + expect(update.newPathname).to.equal("") + + done() + describe "tpds", -> before (done) -> @tpds_project_name = "tpds-project-#{new ObjectId().toString()}" diff --git a/services/web/test/acceptance/coffee/helpers/MockDocUpdaterApi.coffee b/services/web/test/acceptance/coffee/helpers/MockDocUpdaterApi.coffee index b00cd6b173..b21fb1adab 100644 --- a/services/web/test/acceptance/coffee/helpers/MockDocUpdaterApi.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockDocUpdaterApi.coffee @@ -33,6 +33,9 @@ module.exports = MockDocUpdaterApi = @addProjectStructureUpdates(project_id, userId, docUpdates, fileUpdates) res.sendStatus 200 + app.delete "/project/:project_id/doc/:doc_id", (req, res, next) => + res.send 204 + app.listen 3003, (error) -> throw error if error? .on "error", (error) -> diff --git a/services/web/test/acceptance/coffee/helpers/MockDocstoreApi.coffee b/services/web/test/acceptance/coffee/helpers/MockDocstoreApi.coffee index c5b003ac75..631538fd89 100644 --- a/services/web/test/acceptance/coffee/helpers/MockDocstoreApi.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockDocstoreApi.coffee @@ -23,6 +23,16 @@ module.exports = MockDocStoreApi = docs = (doc for doc_id, doc of @docs[req.params.project_id]) res.send JSON.stringify docs + app.delete "/project/:project_id/doc/:doc_id", (req, res, next) => + {project_id, doc_id} = req.params + if !@docs[project_id]? + res.send 404 + else if !@docs[project_id][doc_id]? + res.send 404 + else + @docs[project_id][doc_id] = undefined + res.send 204 + app.listen 3016, (error) -> throw error if error? .on "error", (error) -> From 2ac74b9adcadb3bd9fb166346b6f43c376cc60e8 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 10:14:01 +0000 Subject: [PATCH 5/9] pass userId into _clean methods in ProjectEntityHandler --- .../Project/ProjectEntityHandler.coffee | 26 ++++++------ .../Project/ProjectEntityHandlerTests.coffee | 42 +++++++++++-------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 4ff3818516..644f60349a 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -423,7 +423,7 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path)=> return callback(error) if error? - ProjectEntityHandler._cleanUpEntity project, entity, entityType, path.fileSystem, (error) -> + ProjectEntityHandler._cleanUpEntity project, null, entity, entityType, path.fileSystem, (error) -> return callback(error) if error? tpdsUpdateSender.deleteEntity project_id:project_id, path:path.fileSystem, project_name:project.name, (error) -> return callback(error) if error? @@ -456,17 +456,17 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback - _cleanUpEntity: (project, entity, entityType, path, callback = (error) ->) -> + _cleanUpEntity: (project, userId, entity, entityType, path, callback = (error) ->) -> if(entityType.indexOf("file") != -1) - ProjectEntityHandler._cleanUpFile project, entity, path, callback + ProjectEntityHandler._cleanUpFile project, userId, entity, path, callback else if (entityType.indexOf("doc") != -1) - ProjectEntityHandler._cleanUpDoc project, entity, path, callback + ProjectEntityHandler._cleanUpDoc project, userId, entity, path, callback else if (entityType.indexOf("folder") != -1) - ProjectEntityHandler._cleanUpFolder project, entity, path, callback + ProjectEntityHandler._cleanUpFolder project, userId, entity, path, callback else callback() - _cleanUpDoc: (project, doc, path, callback = (error) ->) -> + _cleanUpDoc: (project, userId, doc, path, callback = (error) ->) -> project_id = project._id.toString() doc_id = doc._id.toString() unsetRootDocIfRequired = (callback) => @@ -484,32 +484,32 @@ module.exports = ProjectEntityHandler = DocstoreManager.deleteDoc project_id, doc_id, (error) -> return callback(error) if error? changes = oldDocs: [ {doc, path} ] - DocumentUpdaterHandler.updateProjectStructure project_id, null, changes, callback + DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback - _cleanUpFile: (project, file, path, callback = (error) ->) -> + _cleanUpFile: (project, userId, file, path, callback = (error) ->) -> project_id = project._id.toString() file_id = file._id.toString() FileStoreHandler.deleteFile project_id, file_id, (error) -> return callback(error) if error? changes = oldFiles: [ {file, path} ] - DocumentUpdaterHandler.updateProjectStructure project_id, null, changes, callback + DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback - _cleanUpFolder: (project, folder, folderPath, callback = (error) ->) -> + _cleanUpFolder: (project, userId, folder, folderPath, callback = (error) ->) -> jobs = [] for doc in folder.docs do (doc) -> docPath = path.join(folderPath, doc.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, doc, docPath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, userId, doc, docPath, callback for file in folder.fileRefs do (file) -> filePath = path.join(folderPath, file.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, file, filePath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, userId, file, filePath, callback for childFolder in folder.folders do (childFolder) -> folderPath = path.join(folderPath, childFolder.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, childFolder, folderPath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, userId, childFolder, folderPath, callback async.series jobs, callback diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index faaaa146da..63240f670e 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -157,7 +157,7 @@ describe 'ProjectEntityHandler', -> @ProjectGetter.getProject.callsArgWith(2, null, @project) @tpdsUpdateSender.deleteEntity = sinon.stub().callsArg(1) @ProjectEntityHandler._removeElementFromMongoArray = sinon.stub().callsArg(3) - @ProjectEntityHandler._cleanUpEntity = sinon.stub().callsArg(4) + @ProjectEntityHandler._cleanUpEntity = sinon.stub().callsArg(5) @path = mongo: "mongo.path", fileSystem: "/file/system/path" @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: entity_id }, @path) @@ -182,7 +182,7 @@ describe 'ProjectEntityHandler', -> it "should clean up the entity from the rest of the system", -> @ProjectEntityHandler._cleanUpEntity - .calledWith(@project, @entity, @type, @path.fileSystem) + .calledWith(@project, null, @entity, @type, @path.fileSystem) .should.equal true describe "_cleanUpEntity", -> @@ -195,7 +195,7 @@ describe 'ProjectEntityHandler', -> beforeEach (done) -> @path = "/file/system/path.png" @entity = _id: @entity_id - @ProjectEntityHandler._cleanUpEntity @project, @entity, 'file', @path, done + @ProjectEntityHandler._cleanUpEntity @project, userId, @entity, 'file', @path, done it "should delete the file from FileStoreHandler", -> @FileStoreHandler.deleteFile.calledWith(project_id, @entity_id).should.equal true @@ -206,19 +206,19 @@ describe 'ProjectEntityHandler', -> it "should should send the update to the doc updater", -> oldFiles = [ file: @entity, path: @path ] @documentUpdaterHandler.updateProjectStructure - .calledWith(project_id, null, {oldFiles}) + .calledWith(project_id, userId, {oldFiles}) .should.equal true describe "a doc", -> beforeEach (done) -> @path = "/file/system/path.tex" - @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(3) + @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(4) @entity = {_id: @entity_id} - @ProjectEntityHandler._cleanUpEntity @project, @entity, 'doc', @path, done + @ProjectEntityHandler._cleanUpEntity @project, userId, @entity, 'doc', @path, done it "should clean up the doc", -> @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, @entity, @path) + .calledWith(@project, userId, @entity, @path) .should.equal true describe "a folder", -> @@ -233,18 +233,26 @@ describe 'ProjectEntityHandler', -> fileRefs: [ @file2 = { _id: "file-id-2", name: "file-name-2" } ] docs: [ @doc2 = { _id: "doc-id-2", name: "doc-name-2" } ] - @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(3) - @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(3) + @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(4) + @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(4) path = "/folder" - @ProjectEntityHandler._cleanUpEntity @project, @folder, "folder", path, done + @ProjectEntityHandler._cleanUpEntity @project, userId, @folder, "folder", path, done it "should clean up all sub files", -> - @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file1, "/folder/subfolder/file-name-1").should.equal true - @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file2, "/folder/file-name-2").should.equal true + @ProjectEntityHandler._cleanUpFile + .calledWith(@project, userId, @file1, "/folder/subfolder/file-name-1") + .should.equal true + @ProjectEntityHandler._cleanUpFile + .calledWith(@project, userId, @file2, "/folder/file-name-2") + .should.equal true it "should clean up all sub docs", -> - @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc1, "/folder/subfolder/doc-name-1").should.equal true - @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc2, "/folder/doc-name-2").should.equal true + @ProjectEntityHandler._cleanUpDoc + .calledWith(@project, userId, @doc1, "/folder/subfolder/doc-name-1") + .should.equal true + @ProjectEntityHandler._cleanUpDoc + .calledWith(@project, userId, @doc2, "/folder/doc-name-2") + .should.equal true describe 'moveEntity', -> beforeEach -> @@ -1138,7 +1146,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is the root doc", -> beforeEach -> @project.rootDoc_id = @doc._id - @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, @callback + @ProjectEntityHandler._cleanUpDoc @project, userId, @doc, @path, @callback it "should unset the root doc", -> @ProjectEntityHandler.unsetRootDoc @@ -1162,7 +1170,7 @@ describe 'ProjectEntityHandler', -> it "should should send the update to the doc updater", -> oldDocs = [ doc: @doc, path: @path ] @documentUpdaterHandler.updateProjectStructure - .calledWith(project_id, null, {oldDocs}) + .calledWith(project_id, userId, {oldDocs}) .should.equal true it "should call the callback", -> @@ -1171,7 +1179,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is not the root doc", -> beforeEach -> @project.rootDoc_id = ObjectId() - @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, @callback + @ProjectEntityHandler._cleanUpDoc @project, userId, @doc, @path, @callback it "should not unset the root doc", -> @ProjectEntityHandler.unsetRootDoc.called.should.equal false From 5f6686ed3b5da84b4a67205bb8d2e9046084a64d Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 10:23:10 +0000 Subject: [PATCH 6/9] pass userId to ProjectEntityHandler.deleteEntity --- .../Features/Editor/EditorController.coffee | 6 ++-- .../Project/ProjectEntityHandler.coffee | 4 +-- .../Editor/EditorControllerTests.coffee | 35 +++++++++---------- .../Project/ProjectEntityHandlerTests.coffee | 4 +-- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index f66094f9e8..75741ceef2 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -110,14 +110,14 @@ module.exports = EditorController = if err? logger.err err:err, project_id:project_id, "could not get lock to deleteEntity" return callback(err) - EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, (err)-> + EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, null, (err)-> LockManager.releaseLock project_id, ()-> callback(err) - deleteEntityWithoutLock: (project_id, entity_id, entityType, source, callback)-> + deleteEntityWithoutLock: (project_id, entity_id, entityType, source, userId, callback)-> logger.log {project_id, entity_id, entityType, source}, "start delete process of entity" Metrics.inc "editor.delete-entity" - ProjectEntityHandler.deleteEntity project_id, entity_id, entityType, (err)-> + ProjectEntityHandler.deleteEntity project_id, entity_id, entityType, userId, (err)-> if err? logger.err err:err, project_id:project_id, entity_id:entity_id, entityType:entityType, "error deleting entity" return callback(err) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 644f60349a..0dec3f191b 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -412,7 +412,7 @@ module.exports = ProjectEntityHandler = callback() - deleteEntity: (project_id, entity_id, entityType, callback = (error) ->)-> + deleteEntity: (project_id, entity_id, entityType, userId, callback = (error) ->)-> self = @ logger.log entity_id:entity_id, entityType:entityType, project_id:project_id, "deleting project entity" if !entityType? @@ -423,7 +423,7 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path)=> return callback(error) if error? - ProjectEntityHandler._cleanUpEntity project, null, entity, entityType, path.fileSystem, (error) -> + ProjectEntityHandler._cleanUpEntity project, userId, entity, entityType, path.fileSystem, (error) -> return callback(error) if error? tpdsUpdateSender.deleteEntity project_id:project_id, path:path.fileSystem, project_name:project.name, (error) -> return callback(error) if error? diff --git a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee index 4e1af79b46..1e84352d38 100644 --- a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee @@ -382,11 +382,13 @@ describe "EditorController", -> beforeEach -> @LockManager.getLock.callsArgWith(1) @LockManager.releaseLock.callsArgWith(1) - @EditorController.deleteEntityWithoutLock = sinon.stub().callsArgWith(4) + @EditorController.deleteEntityWithoutLock = sinon.stub().callsArgWith(5) it "should call deleteEntityWithoutLock", (done)-> @EditorController.deleteEntity @project_id, @entity_id, @type, @source, => - @EditorController.deleteEntityWithoutLock.calledWith(@project_id, @entity_id, @type, @source).should.equal true + @EditorController.deleteEntityWithoutLock + .calledWith(@project_id, @entity_id, @type, @source, null) + .should.equal true done() it "should take the lock", (done)-> @@ -404,30 +406,25 @@ describe "EditorController", -> @EditorController.deleteEntity @project_id, @entity_id, @type, @source, (err)=> expect(err).to.exist err.should.equal "timed out" - done() - - + done() describe 'deleteEntityWithoutLock', -> - beforeEach -> - @ProjectEntityHandler.deleteEntity = (project_id, entity_id, type, callback)-> callback() + beforeEach (done) -> @entity_id = "entity_id_here" @type = "doc" @EditorRealTimeController.emitToRoom = sinon.stub() + @ProjectEntityHandler.deleteEntity = sinon.stub().callsArg(4) + @EditorController.deleteEntityWithoutLock @project_id, @entity_id, @type, @source, @user_id, done - it 'should delete the folder using the project entity handler', (done)-> - mock = sinon.mock(@ProjectEntityHandler).expects("deleteEntity").withArgs(@project_id, @entity_id, @type).callsArg(3) + it 'should delete the folder using the project entity handler', -> + @ProjectEntityHandler.deleteEntity + .calledWith(@project_id, @entity_id, @type, @user_id) + .should.equal.true - @EditorController.deleteEntityWithoutLock @project_id, @entity_id, @type, @source, -> - mock.verify() - done() - - it 'notify users an entity has been deleted', (done)-> - @EditorController.deleteEntityWithoutLock @project_id, @entity_id, @type, @source, => - @EditorRealTimeController.emitToRoom - .calledWith(@project_id, "removeEntity", @entity_id, @source) - .should.equal true - done() + it 'notify users an entity has been deleted', -> + @EditorRealTimeController.emitToRoom + .calledWith(@project_id, "removeEntity", @entity_id, @source) + .should.equal true describe "getting a list of project paths", -> diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index 63240f670e..7a6b62d99f 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -163,7 +163,7 @@ describe 'ProjectEntityHandler', -> describe "deleting from Mongo", -> beforeEach (done) -> - @ProjectEntityHandler.deleteEntity project_id, entity_id, @type = 'file', done + @ProjectEntityHandler.deleteEntity project_id, entity_id, @type = 'file', userId, done it "should retreive the path", -> @projectLocator.findElement.called.should.equal true @@ -182,7 +182,7 @@ describe 'ProjectEntityHandler', -> it "should clean up the entity from the rest of the system", -> @ProjectEntityHandler._cleanUpEntity - .calledWith(@project, null, @entity, @type, @path.fileSystem) + .calledWith(@project, userId, @entity, @type, @path.fileSystem) .should.equal true describe "_cleanUpEntity", -> From ca15fdb6eb74f6873ad3c409bcb44282884b82fe Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 10:53:38 +0000 Subject: [PATCH 7/9] pass userId to EditorHttpController.deleteEntity --- .../Features/Editor/EditorController.coffee | 4 ++-- .../Features/Editor/EditorHttpController.coffee | 3 ++- .../coffee/ProjectStructureTests.coffee | 4 ++-- .../coffee/Editor/EditorControllerTests.coffee | 16 +++++++--------- .../Editor/EditorHttpControllerTests.coffee | 4 ++-- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 75741ceef2..532f515a10 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -105,12 +105,12 @@ module.exports = EditorController = async.series jobs, (err)-> callback err, newFolders, lastFolder - deleteEntity : (project_id, entity_id, entityType, source, callback)-> + deleteEntity : (project_id, entity_id, entityType, source, userId, callback)-> LockManager.getLock project_id, (err)-> if err? logger.err err:err, project_id:project_id, "could not get lock to deleteEntity" return callback(err) - EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, null, (err)-> + EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, userId, (err)-> LockManager.releaseLock project_id, ()-> callback(err) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index cbc296b69f..16b1a79e31 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -147,6 +147,7 @@ module.exports = EditorHttpController = project_id = req.params.Project_id entity_id = req.params.entity_id entity_type = req.params.entity_type - EditorController.deleteEntity project_id, entity_id, entity_type, "editor", (error) -> + user_id = AuthenticationController.getLoggedInUserId(req) + EditorController.deleteEntity project_id, entity_id, entity_type, "editor", user_id, (error) -> return next(error) if error? res.sendStatus 204 diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index 0f8086647f..fa91f7aeff 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -303,14 +303,14 @@ describe "ProjectStructureChanges", -> updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates expect(updates.length).to.equal(1) update = updates[0] - #expect(update.userId).to.equal(@owner._id) + expect(update.userId).to.equal(@owner._id) expect(update.pathname).to.equal("/bar/foo/new.tex") expect(update.newPathname).to.equal("") updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] - #expect(update.userId).to.equal(@owner._id) + expect(update.userId).to.equal(@owner._id) expect(update.pathname).to.equal("/bar/foo/1pixel.png") expect(update.newPathname).to.equal("") diff --git a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee index 1e84352d38..85760f510d 100644 --- a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee @@ -376,36 +376,34 @@ describe "EditorController", -> err.should.equal "timed out" done() - describe "deleteEntity", -> - beforeEach -> @LockManager.getLock.callsArgWith(1) @LockManager.releaseLock.callsArgWith(1) @EditorController.deleteEntityWithoutLock = sinon.stub().callsArgWith(5) it "should call deleteEntityWithoutLock", (done)-> - @EditorController.deleteEntity @project_id, @entity_id, @type, @source, => + @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, => @EditorController.deleteEntityWithoutLock - .calledWith(@project_id, @entity_id, @type, @source, null) + .calledWith(@project_id, @entity_id, @type, @source, @user_id) .should.equal true done() it "should take the lock", (done)-> - @EditorController.deleteEntity @project_id, @entity_id, @type, @source, => + @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, => @LockManager.getLock.calledWith(@project_id).should.equal true done() it "should release the lock", (done)-> - @EditorController.deleteEntity @project_id, @entity_id, @type, @source, (error)=> + @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, (error) => @LockManager.releaseLock.calledWith(@project_id).should.equal true done() it "should error if it can't cat the lock", (done)-> @LockManager.getLock = sinon.stub().callsArgWith(1, "timed out") - @EditorController.deleteEntity @project_id, @entity_id, @type, @source, (err)=> - expect(err).to.exist - err.should.equal "timed out" + @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, (error) => + expect(error).to.exist + error.should.equal "timed out" done() describe 'deleteEntityWithoutLock', -> diff --git a/services/web/test/unit/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/unit/coffee/Editor/EditorHttpControllerTests.coffee index e05846c5d5..38419e6b46 100644 --- a/services/web/test/unit/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/unit/coffee/Editor/EditorHttpControllerTests.coffee @@ -331,12 +331,12 @@ describe "EditorHttpController", -> Project_id: @project_id entity_id: @entity_id = "entity-id-123" entity_type: @entity_type = "entity-type" - @EditorController.deleteEntity = sinon.stub().callsArg(4) + @EditorController.deleteEntity = sinon.stub().callsArg(5) @EditorHttpController.deleteEntity @req, @res it "should call EditorController.deleteEntity", -> @EditorController.deleteEntity - .calledWith(@project_id, @entity_id, @entity_type, "editor") + .calledWith(@project_id, @entity_id, @entity_type, "editor", @userId) .should.equal true it "should send back a success response", -> From 938caed4f72dd17d83bac61cab28e6895926f60c Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 11:32:43 +0000 Subject: [PATCH 8/9] set userId when deleting entities via the tpds --- .../TpdsUpdateHandler.coffee | 2 +- .../ThirdPartyDataStore/UpdateMerger.coffee | 4 ++-- .../coffee/ProjectStructureTests.coffee | 22 +++++++++++++++++++ .../TpdsUpdateHandlerTests.coffee | 8 ++++--- .../UpdateMergerTests.coffee | 6 ++--- 5 files changed, 33 insertions(+), 9 deletions(-) diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee index c78b588e8a..78e3f12ed6 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee @@ -47,7 +47,7 @@ module.exports = logger.log user_id:user_id, filePath:path, projectName:projectName, project_id:project._id, "project found for delete update, path is root so marking project as deleted" return projectDeleter.markAsDeletedByExternalSource project._id, callback else - updateMerger.deleteUpdate project._id, path, source, (err)-> + updateMerger.deleteUpdate user_id, project._id, path, source, (err)-> callback(err) diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee index 010594d16a..e49c52aab2 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee @@ -32,13 +32,13 @@ module.exports = else self.p.processDoc project_id, elementId, user_id, fsPath, path, source, callback - deleteUpdate: (project_id, path, source, callback)-> + deleteUpdate: (user_id, project_id, path, source, callback)-> projectLocator.findElementByPath project_id, path, (err, element, type)-> if err? || !element? logger.log element:element, project_id:project_id, path:path, "could not find entity for deleting, assuming it was already deleted" return callback() logger.log project_id:project_id, path:path, type:type, element:element, "processing update to delete entity from tpds" - editorController.deleteEntity project_id, element._id, type, source, (err)-> + editorController.deleteEntity project_id, element._id, type, source, user_id, (err)-> logger.log project_id:project_id, path:path, "finished processing update to delete entity from tpds" callback() diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index fa91f7aeff..cb59efe1df 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -422,3 +422,25 @@ describe "ProjectStructureChanges", -> done() image_file.pipe(req) + + it "should version deleting a doc", (done) -> + req = @owner.request.delete { + uri: "/user/#{@owner._id}/update/#{@tpds_project_name}/test.tex", + auth: + user: _.keys(Settings.httpAuthUsers)[0] + pass: _.values(Settings.httpAuthUsers)[0] + sendImmediately: true + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to delete doc #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@tpds_project_id).docUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/test.tex") + expect(update.newPathname).to.equal("") + + done() + diff --git a/services/web/test/unit/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee b/services/web/test/unit/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee index dedd3ea7c8..3984d6341f 100644 --- a/services/web/test/unit/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee @@ -8,7 +8,7 @@ describe 'TpdsUpdateHandler', -> beforeEach -> @requestQueuer = {} @updateMerger = - deleteUpdate: (user_id, path, source, cb)->cb() + deleteUpdate: (user_id, project_id, path, source, cb)->cb() mergeUpdate:(user_id, project_id, path, update, source, cb)->cb() @editorController = {} @project_id = "dsjajilknaksdn" @@ -107,11 +107,13 @@ describe 'TpdsUpdateHandler', -> it 'should call deleteEntity in the collaberation manager', (done)-> path = "/delete/this" update = {} - @updateMerger.deleteUpdate = sinon.stub().callsArg(3) + @updateMerger.deleteUpdate = sinon.stub().callsArg(4) @handler.deleteUpdate @user_id, @project.name, path, @source, => @projectDeleter.markAsDeletedByExternalSource.calledWith(@project._id).should.equal false - @updateMerger.deleteUpdate.calledWith(@project_id, path, @source).should.equal true + @updateMerger.deleteUpdate + .calledWith(@user_id, @project_id, path, @source) + .should.equal true done() it 'should mark the project as deleted by external source if path is a single slash', (done)-> diff --git a/services/web/test/unit/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee b/services/web/test/unit/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee index e20419765f..cb2aa059ea 100644 --- a/services/web/test/unit/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee +++ b/services/web/test/unit/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee @@ -145,13 +145,13 @@ describe 'UpdateMerger :', -> it 'should get the element id', -> @projectLocator.findElementByPath = sinon.spy() - @updateMerger.deleteUpdate @project_id, @path, @source, -> + @updateMerger.deleteUpdate @user_id, @project_id, @path, @source, -> @projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true it 'should delete the entity in the editor controller with the correct type', (done)-> @entity.lines = [] - mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, @type, @source).callsArg(4) - @updateMerger.deleteUpdate @project_id, @path, @source, -> + mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, @type, @source, @user_id).callsArg(5) + @updateMerger.deleteUpdate @user_id, @project_id, @path, @source, -> mock.verify() done() From ac36de962924e09f98008178b5911161adad7bed Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 13:17:26 +0000 Subject: [PATCH 9/9] make ProjectEntityHandler._clean* argument signatures consistent --- .../Project/ProjectEntityHandler.coffee | 22 +++++++++---------- .../Project/ProjectEntityHandlerTests.coffee | 22 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 0dec3f191b..a9d7c708ee 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -423,7 +423,7 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path)=> return callback(error) if error? - ProjectEntityHandler._cleanUpEntity project, userId, entity, entityType, path.fileSystem, (error) -> + ProjectEntityHandler._cleanUpEntity project, entity, entityType, path.fileSystem, userId, (error) -> return callback(error) if error? tpdsUpdateSender.deleteEntity project_id:project_id, path:path.fileSystem, project_name:project.name, (error) -> return callback(error) if error? @@ -456,17 +456,17 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback - _cleanUpEntity: (project, userId, entity, entityType, path, callback = (error) ->) -> + _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> if(entityType.indexOf("file") != -1) - ProjectEntityHandler._cleanUpFile project, userId, entity, path, callback + ProjectEntityHandler._cleanUpFile project, entity, path, userId, callback else if (entityType.indexOf("doc") != -1) - ProjectEntityHandler._cleanUpDoc project, userId, entity, path, callback + ProjectEntityHandler._cleanUpDoc project, entity, path, userId, callback else if (entityType.indexOf("folder") != -1) - ProjectEntityHandler._cleanUpFolder project, userId, entity, path, callback + ProjectEntityHandler._cleanUpFolder project, entity, path, userId, callback else callback() - _cleanUpDoc: (project, userId, doc, path, callback = (error) ->) -> + _cleanUpDoc: (project, doc, path, userId, callback = (error) ->) -> project_id = project._id.toString() doc_id = doc._id.toString() unsetRootDocIfRequired = (callback) => @@ -486,7 +486,7 @@ module.exports = ProjectEntityHandler = changes = oldDocs: [ {doc, path} ] DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback - _cleanUpFile: (project, userId, file, path, callback = (error) ->) -> + _cleanUpFile: (project, file, path, userId, callback = (error) ->) -> project_id = project._id.toString() file_id = file._id.toString() FileStoreHandler.deleteFile project_id, file_id, (error) -> @@ -494,22 +494,22 @@ module.exports = ProjectEntityHandler = changes = oldFiles: [ {file, path} ] DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback - _cleanUpFolder: (project, userId, folder, folderPath, callback = (error) ->) -> + _cleanUpFolder: (project, folder, folderPath, userId, callback = (error) ->) -> jobs = [] for doc in folder.docs do (doc) -> docPath = path.join(folderPath, doc.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, userId, doc, docPath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, doc, docPath, userId, callback for file in folder.fileRefs do (file) -> filePath = path.join(folderPath, file.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, userId, file, filePath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, file, filePath, userId, callback for childFolder in folder.folders do (childFolder) -> folderPath = path.join(folderPath, childFolder.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, userId, childFolder, folderPath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, childFolder, folderPath, userId, callback async.series jobs, callback diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index 7a6b62d99f..7c4188052f 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -182,7 +182,7 @@ describe 'ProjectEntityHandler', -> it "should clean up the entity from the rest of the system", -> @ProjectEntityHandler._cleanUpEntity - .calledWith(@project, userId, @entity, @type, @path.fileSystem) + .calledWith(@project, @entity, @type, @path.fileSystem, userId) .should.equal true describe "_cleanUpEntity", -> @@ -195,7 +195,7 @@ describe 'ProjectEntityHandler', -> beforeEach (done) -> @path = "/file/system/path.png" @entity = _id: @entity_id - @ProjectEntityHandler._cleanUpEntity @project, userId, @entity, 'file', @path, done + @ProjectEntityHandler._cleanUpEntity @project, @entity, 'file', @path, userId, done it "should delete the file from FileStoreHandler", -> @FileStoreHandler.deleteFile.calledWith(project_id, @entity_id).should.equal true @@ -214,11 +214,11 @@ describe 'ProjectEntityHandler', -> @path = "/file/system/path.tex" @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(4) @entity = {_id: @entity_id} - @ProjectEntityHandler._cleanUpEntity @project, userId, @entity, 'doc', @path, done + @ProjectEntityHandler._cleanUpEntity @project, @entity, 'doc', @path, userId, done it "should clean up the doc", -> @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, userId, @entity, @path) + .calledWith(@project, @entity, @path, userId) .should.equal true describe "a folder", -> @@ -236,22 +236,22 @@ describe 'ProjectEntityHandler', -> @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(4) @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(4) path = "/folder" - @ProjectEntityHandler._cleanUpEntity @project, userId, @folder, "folder", path, done + @ProjectEntityHandler._cleanUpEntity @project, @folder, "folder", path, userId, done it "should clean up all sub files", -> @ProjectEntityHandler._cleanUpFile - .calledWith(@project, userId, @file1, "/folder/subfolder/file-name-1") + .calledWith(@project, @file1, "/folder/subfolder/file-name-1", userId) .should.equal true @ProjectEntityHandler._cleanUpFile - .calledWith(@project, userId, @file2, "/folder/file-name-2") + .calledWith(@project, @file2, "/folder/file-name-2", userId) .should.equal true it "should clean up all sub docs", -> @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, userId, @doc1, "/folder/subfolder/doc-name-1") + .calledWith(@project, @doc1, "/folder/subfolder/doc-name-1", userId) .should.equal true @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, userId, @doc2, "/folder/doc-name-2") + .calledWith(@project, @doc2, "/folder/doc-name-2", userId) .should.equal true describe 'moveEntity', -> @@ -1146,7 +1146,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is the root doc", -> beforeEach -> @project.rootDoc_id = @doc._id - @ProjectEntityHandler._cleanUpDoc @project, userId, @doc, @path, @callback + @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, userId, @callback it "should unset the root doc", -> @ProjectEntityHandler.unsetRootDoc @@ -1179,7 +1179,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is not the root doc", -> beforeEach -> @project.rootDoc_id = ObjectId() - @ProjectEntityHandler._cleanUpDoc @project, userId, @doc, @path, @callback + @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, userId, @callback it "should not unset the root doc", -> @ProjectEntityHandler.unsetRootDoc.called.should.equal false