From b537747ccd4cbca4f913bbe8062459a9a2bf1983 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 19 Jan 2018 12:01:23 +0000 Subject: [PATCH 01/31] check for duplicates in putElement --- .../Project/ProjectEntityHandler.coffee | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index c4992a29f8..750924beb3 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -170,7 +170,8 @@ module.exports = ProjectEntityHandler = if project_or_id._id? # project return cb(null, project_or_id) else # id - return ProjectGetter.getProjectWithOnlyFolders project_or_id, cb + # need to retrieve full project structure to check for duplicates + return ProjectGetter.getProject project_or_id, {}, cb getProject (error, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add doc" @@ -207,7 +208,7 @@ module.exports = ProjectEntityHandler = ProjectEntityHandler.addDoc project_id, null, name, lines, callback addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> - ProjectGetter.getProjectWithOnlyFolders project_id, (err, project) -> + ProjectGetter.getProject project_id, {}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" return callback(err) @@ -606,19 +607,32 @@ module.exports = ProjectEntityHandler = newPath = fileSystem: "#{path.fileSystem}/#{element.name}" mongo: path.mongo - id = element._id+'' - element._id = require('mongoose').Types.ObjectId(id) - conditions = _id:project._id - mongopath = "#{path.mongo}.#{type}" - 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.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}, project) + ProjectEntityHandler.checkElementName folder, element.name, (err) => + return callback(err) if err? + id = element._id+'' + element._id = require('mongoose').Types.ObjectId(id) + conditions = _id:project._id + mongopath = "#{path.mongo}.#{type}" + 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.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}, project) + checkElementName: (folder, name, callback = (err) ->) -> + # check if the name is already taken by a doc, file or + # folder. If so, return an error "file already exists". + err = new Errors.InvalidNameError("file already exists") + for doc in folder?.docs or [] + return callback(err) if doc.name is name + for file in folder?.fileRefs or [] + return callback(err) if file.name is name + for folder in folder?.folders or [] + return callback(err) if folder.name is name + callback() confirmFolder = (project, folder_id, callback)-> logger.log folder_id:folder_id, project_id:project._id, "confirming folder in project" From c6cca79737f21a1b87182f6a0b385f6073ee0a29 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 19 Jan 2018 12:02:04 +0000 Subject: [PATCH 02/31] check for duplicates in addFolder --- .../Project/ProjectEntityHandler.coffee | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 750924beb3..d606b94c40 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -341,7 +341,7 @@ module.exports = ProjectEntityHandler = callback(null, folders, lastFolder) addFolder: (project_id, parentFolder_id, folderName, callback) -> - ProjectGetter.getProjectWithOnlyFolders project_id, (err, project)=> + ProjectGetter.getProject project_id, {}, (err, project)=> if err? logger.err project_id:project_id, err:err, "error getting project for add folder" return callback(err) @@ -459,17 +459,20 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath)=> return callback(error) if error? - endPath = path.join(path.dirname(entPath.fileSystem), newName) - conditions = {_id:project_id} - update = "$set":{} - namePath = entPath.mongo+".name" - update["$set"][namePath] = newName - tpdsUpdateSender.moveEntity({project_id:project_id, startPath:entPath.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, newFiles) => + ProjectEntityHandler.checkElementName entity, newName, (err) => + return callback(err) if err? + endPath = path.join(path.dirname(entPath.fileSystem), newName) + conditions = {_id:project_id} + update = "$set":{} + namePath = entPath.mongo+".name" + update["$set"][namePath] = newName + # FIXME check if this would create a duplicate file! + tpdsUpdateSender.moveEntity({project_id:project_id, startPath:entPath.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) + Project.findOneAndUpdate conditions, update, { "new": true}, (error, newProject) -> return callback(error) if error? - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback + ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs, newFiles) => + return callback(error) if error? + DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> if(entityType.indexOf("file") != -1) From 3881eb1d78ceafad08872e48688bb66a587a4f45 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 18 Jan 2018 16:22:38 +0000 Subject: [PATCH 03/31] check for duplicates on rename --- .../coffee/Features/Project/ProjectEntityHandler.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index d606b94c40..74010fba33 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -457,16 +457,16 @@ module.exports = ProjectEntityHandler = return callback(error) if error? ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? - projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath)=> + projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath, parentFolder)=> return callback(error) if error? - ProjectEntityHandler.checkElementName entity, newName, (err) => - return callback(err) if err? + # check if the new name already exists in the current folder + ProjectEntityHandler.checkElementName parentFolder, newName, (error) => + return callback(error) if error? endPath = path.join(path.dirname(entPath.fileSystem), newName) conditions = {_id:project_id} update = "$set":{} namePath = entPath.mongo+".name" update["$set"][namePath] = newName - # FIXME check if this would create a duplicate file! tpdsUpdateSender.moveEntity({project_id:project_id, startPath:entPath.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) Project.findOneAndUpdate conditions, update, { "new": true}, (error, newProject) -> return callback(error) if error? From 4f5a5cb677215dd189e8f346c5d77d4ee425b0f0 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 18 Jan 2018 16:42:26 +0000 Subject: [PATCH 04/31] check for duplicates on move --- .../Project/ProjectEntityHandler.coffee | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 74010fba33..89fc9fb42a 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -395,7 +395,7 @@ module.exports = ProjectEntityHandler = return callback(err) if err? projectLocator.findElement {project, element_id: entity_id, type: entityType}, (err, entity, entityPath)-> return callback(err) if err? - self._checkValidMove project, entityType, entityPath, destFolderId, (error) -> + self._checkValidMove project, entityType, entity, entityPath, destFolderId, (error) -> return callback(error) if error? self.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? @@ -414,19 +414,20 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback - _checkValidMove: (project, entityType, entityPath, destFolderId, callback = (error) ->) -> - return callback() if !entityType.match(/folder/) - + _checkValidMove: (project, entityType, entity, entityPath, destFolderId, callback = (error) ->) -> 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 + # check if there is already a doc/file/folder with the same name + # in the destination folder + ProjectEntityHandler.checkElementName destEntity, entity.name, (err)-> + return callback(err) if err? + if entityType.match(/folder/) + 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 + return callback(new Error("destination folder is a child folder of me")) callback() - 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" From 9538df1644b5423f19229fa696514b9509054fef Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 18 Jan 2018 14:28:36 +0000 Subject: [PATCH 05/31] update unit tests for duplicate checks --- .../Project/ProjectEntityHandlerTests.coffee | 140 +++++++++++++++++- 1 file changed, 138 insertions(+), 2 deletions(-) diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index 3b6a6e8626..39c1eba75d 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -62,7 +62,7 @@ describe 'ProjectEntityHandler', -> @ProjectGetter = getProjectWithOnlyFolders : (project_id, callback)=> callback(null, @project) getProjectWithoutDocLines : (project_id, callback)=> callback(null, @project) - getProject:sinon.stub() + getProject: sinon.stub().callsArgWith(2, null, @project) @projectUpdater = markAsUpdated:sinon.stub() @projectLocator = findElement : sinon.stub() @@ -287,6 +287,8 @@ describe 'ProjectEntityHandler', -> @projectLocator.findElement = sinon.stub() @projectLocator.findElement.withArgs({project: @project, element_id: @docId, type: 'docs'}) .callsArgWith(1, null, @doc, @path) + @projectLocator.findElement.withArgs({project: @project, element_id: folder_id, type:"folder"},) + .callsArgWith(1, null, @destFolder, @destFolderPath) @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", userId, done it 'should find the doc to move', -> @@ -315,6 +317,46 @@ describe 'ProjectEntityHandler', -> }) .should.equal true + describe "moving a doc when another with the same name already exists", -> + beforeEach () -> + @docId = "4eecaffcbffa66588e000009" + @doc = { name: "another-doc.tex", lines:["1234","312343d"], rev: "1234"} + @path = { + mongo:"folders[0]" + fileSystem:"/old_folder/somewhere.txt" + } + @destFolder = { name: "folder", docs: [ {name:"another-doc.tex"} ] } + @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) + @projectLocator.findElement.withArgs({project: @project, element_id: folder_id, type:"folder"},) + .callsArgWith(1, null, @destFolder, @destFolderPath) + @callback = sinon.stub() + @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", userId, @callback + + it 'should return an error', -> + @callback.calledWith(new Errors.InvalidNameError("file already exists")).should.equal true + + it "should should not send the update to the doc updater", -> + @documentUpdaterHandler.updateProjectStructure + .called.should.equal false + + it 'should not remove the element from its current position', -> + @ProjectEntityHandler._removeElementFromMongoArray + .called.should.equal false + + it "should not put the element back in the new folder", -> + @ProjectEntityHandler._putElement.called.should.equal false + + it 'should not tell the third party data store', -> + @tpdsUpdateSender.moveEntity + .called.should.equal false + + describe "moving a folder", -> beforeEach -> @folder_id = "folder-to-move" @@ -379,10 +421,37 @@ describe 'ProjectEntityHandler', -> }) .should.equal true + describe "when the destination folder contains a file with the same name", -> + beforeEach -> + @path.fileSystem = "/one/src_dir" + @pathToMoveTo.fileSystem = "/two/dest_dir" + @folder_to_move_to = { name: "folder to move to", fileRefs: [ {name: "folder"}] } + @projectLocator.findElement.withArgs({project: @project, element_id: @move_to_folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder_to_move_to, @pathToMoveTo) + @callback = sinon.stub() + @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, @callback + + it 'should find the folder we are moving to element', -> + @projectLocator.findElement + .calledWith({ + element_id: @move_to_folder_id, + type: "folder", + project: @project + }) + .should.equal true + + it "should return an error", -> + @callback + .calledWith(new Errors.InvalidNameError("file already exists")) + .should.equal true + describe "when the destination folder is inside the moving folder", -> beforeEach -> @path.fileSystem = "/one/two" @pathToMoveTo.fileSystem = "/one/two/three" + + @projectLocator.findElement.withArgs({project: @project, element_id: @move_to_folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder_to_move_to, @pathToMoveTo) @callback = sinon.stub() @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, @callback @@ -472,6 +541,7 @@ describe 'ProjectEntityHandler', -> @lines = ['1234','abc'] @path = "/path/to/doc" + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project) @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}}) @callback = sinon.stub() @tpdsUpdateSender.addDoc = sinon.stub().callsArg(1) @@ -522,6 +592,7 @@ describe 'ProjectEntityHandler', -> @lines = ['1234','abc'] @path = "/path/to/doc" + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project) @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}}) @callback = sinon.stub() @tpdsUpdateSender.addDoc = sinon.stub().callsArg(1) @@ -1123,6 +1194,20 @@ describe 'ProjectEntityHandler', -> @newName = "new.tex" @path = mongo: "mongo.path", fileSystem: "/oldnamepath/oldname" + @project_id = project_id + @project = + _id: ObjectId(project_id) + rootFolder: [_id:ObjectId()] + @folder = + _id: ObjectId() + name: "someFolder" + docs: [ {name: "another-doc.tex"} ] + fileRefs: [ {name: "another-file.tex"} ] + folders: [ {name: "another-folder"} ] + @doc = + _id: ObjectId() + name: "new.tex" + @ProjectGetter.getProject.callsArgWith(2, null, @project) @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() @ProjectEntityHandler.getAllEntitiesFromProject @@ -1132,7 +1217,7 @@ describe 'ProjectEntityHandler', -> .onSecondCall() .callsArgWith(1, null, @newDocs = ['new-doc'], @newFiles = ['new-file']) - @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: @entity_id, name:"oldname", rev:4 }, @path) + @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: @entity_id, name:"oldname", rev:4 }, @path, @folder) @tpdsUpdateSender.moveEntity = sinon.stub() @ProjectModel.findOneAndUpdate = sinon.stub().callsArgWith(3, null, @project) @documentUpdaterHandler.updateProjectStructure = sinon.stub().yields() @@ -1154,6 +1239,27 @@ describe 'ProjectEntityHandler', -> @tpdsUpdateSender.moveEntity.calledWith({project_id:project_id, startPath:@path.fileSystem, endPath:"/oldnamepath/new.tex", project_name:@project.name, rev:4}).should.equal true done() + describe "when a document already exists with the same name", -> + beforeEach -> + @project = + _id: ObjectId(project_id) + rootFolder: [_id:ObjectId()] + @folder = + _id: ObjectId() + name: "someFolder" + docs: [ {name: "another-doc.tex"} ] + fileRefs: [ {name: "another-file.tex"} ] + folders: [ {name: "another-folder"} ] + @doc = + _id: ObjectId() + name: "new.tex" + @newName = "another-doc.tex" + + it "should return an error", (done)-> + @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, (err)=> + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + describe "_insertDeletedDocReference", -> beforeEach -> @doc = @@ -1248,6 +1354,9 @@ describe 'ProjectEntityHandler', -> @folder = _id: ObjectId() name: "someFolder" + docs: [ {name: "another-doc.tex"} ] + fileRefs: [ {name: "another-file.tex"} ] + folders: [ {name: "another-folder"} ] @doc = _id: ObjectId() name: "new.tex" @@ -1290,6 +1399,33 @@ describe 'ProjectEntityHandler', -> @ProjectModel.findOneAndUpdate.called.should.equal false done() + it "should error if a document already exists with the same name", (done)-> + doc = + _id: ObjectId() + name: "another-doc.tex" + @ProjectEntityHandler._putElement @project, @folder, doc, "doc", (err)=> + @ProjectModel.findOneAndUpdate.called.should.equal false + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + + it "should error if a file already exists with the same name", (done)-> + doc = + _id: ObjectId() + name: "another-file.tex" + @ProjectEntityHandler._putElement @project, @folder, doc, "doc", (err)=> + @ProjectModel.findOneAndUpdate.called.should.equal false + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + + it "should error if a folder already exists with the same name", (done)-> + doc = + _id: ObjectId() + name: "another-folder" + @ProjectEntityHandler._putElement @project, @folder, doc, "doc", (err)=> + @ProjectModel.findOneAndUpdate.called.should.equal false + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + describe "_countElements", -> beforeEach -> From 9b8ce78eb94ed6c6d0916c7e7a7a0ae63e40a021 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 17 Jan 2018 12:11:02 +0000 Subject: [PATCH 06/31] handle errors normally in addFolder modal --- .../web/app/coffee/Features/Editor/EditorHttpController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 16b1a79e31..614781211c 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -105,7 +105,7 @@ module.exports = EditorHttpController = else if error?.message == 'invalid element name' res.status(400).json(req.i18n.translate('invalid_file_name')) else if error? - res.status(500).json(req.i18n.translate('generic_something_went_wrong')) + next(error) else res.json doc From feb02dacd4d138c0eb724e84ec7aaa60d067e8e6 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 19 Jan 2018 11:27:22 +0000 Subject: [PATCH 07/31] only update client filetree on success --- .../coffee/ide/file-tree/FileTreeManager.coffee | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee index 390b9dba79..d46525bf20 100644 --- a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee +++ b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee @@ -341,12 +341,12 @@ define [ renameEntity: (entity, name, callback = (error) ->) -> return if entity.name == name - if name.length < 150 - entity.name = name - return @ide.$http.post "/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/rename", { - name: entity.name, + return if name.length >= 150 + @ide.$http.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/rename", { + name: name, _csrf: window.csrfToken - } + }).then () -> + entity.name = name deleteEntity: (entity, callback = (error) ->) -> # We'll wait for the socket.io notification to @@ -362,11 +362,11 @@ define [ # Abort move if the folder being moved (entity) has the parent_folder as child # since that would break the tree structure. return if @_isChildFolder(entity, parent_folder) - @_moveEntityInScope(entity, parent_folder) - return @ide.queuedHttp.post "/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/move", { + @ide.queuedHttp.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/move", { folder_id: parent_folder.id _csrf: window.csrfToken - } + }).then () => + @_moveEntityInScope(entity, parent_folder) _isChildFolder: (parent_folder, child_folder) -> parent_path = @getEntityPath(parent_folder) or "" # null if root folder From 2a0b0d3a870ba334f6a71e3655fc51a4f24aae8c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 23 Jan 2018 10:21:05 +0000 Subject: [PATCH 08/31] only need to load rootFolder from project --- .../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 89fc9fb42a..470545b8f9 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -171,7 +171,7 @@ module.exports = ProjectEntityHandler = return cb(null, project_or_id) else # id # need to retrieve full project structure to check for duplicates - return ProjectGetter.getProject project_or_id, {}, cb + return ProjectGetter.getProject project_or_id, {rootFolder:true, name:true}, cb getProject (error, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add doc" @@ -208,7 +208,7 @@ module.exports = ProjectEntityHandler = ProjectEntityHandler.addDoc project_id, null, name, lines, callback addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> - ProjectGetter.getProject project_id, {}, (err, project) -> + ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" return callback(err) @@ -341,7 +341,7 @@ module.exports = ProjectEntityHandler = callback(null, folders, lastFolder) addFolder: (project_id, parentFolder_id, folderName, callback) -> - ProjectGetter.getProject project_id, {}, (err, project)=> + ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project)=> if err? logger.err project_id:project_id, err:err, "error getting project for add folder" return callback(err) From ec93cedf27fb315f955b834e28dab04c47bf68f3 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 23 Jan 2018 10:35:38 +0000 Subject: [PATCH 09/31] rename checkElementName to checkValidElementName --- .../coffee/Features/Project/ProjectEntityHandler.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 470545b8f9..496bb73bd5 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -419,7 +419,7 @@ module.exports = ProjectEntityHandler = return callback(err) if err? # check if there is already a doc/file/folder with the same name # in the destination folder - ProjectEntityHandler.checkElementName destEntity, entity.name, (err)-> + ProjectEntityHandler.checkValidElementName destEntity, entity.name, (err)-> return callback(err) if err? if entityType.match(/folder/) logger.log destFolderPath: destFolderPath.fileSystem, folderPath: entityPath.fileSystem, "checking folder is not moving into child folder" @@ -461,7 +461,7 @@ module.exports = ProjectEntityHandler = projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath, parentFolder)=> return callback(error) if error? # check if the new name already exists in the current folder - ProjectEntityHandler.checkElementName parentFolder, newName, (error) => + ProjectEntityHandler.checkValidElementName parentFolder, newName, (error) => return callback(error) if error? endPath = path.join(path.dirname(entPath.fileSystem), newName) conditions = {_id:project_id} @@ -611,7 +611,7 @@ module.exports = ProjectEntityHandler = newPath = fileSystem: "#{path.fileSystem}/#{element.name}" mongo: path.mongo - ProjectEntityHandler.checkElementName folder, element.name, (err) => + ProjectEntityHandler.checkValidElementName folder, element.name, (err) => return callback(err) if err? id = element._id+'' element._id = require('mongoose').Types.ObjectId(id) @@ -626,7 +626,7 @@ module.exports = ProjectEntityHandler = return callback(err) callback(err, {path:newPath}, project) - checkElementName: (folder, name, callback = (err) ->) -> + checkValidElementName: (folder, name, callback = (err) ->) -> # check if the name is already taken by a doc, file or # folder. If so, return an error "file already exists". err = new Errors.InvalidNameError("file already exists") From 82a9fc97d9a4897f796f63486a032051ca26f2a8 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 19 Jan 2018 12:01:23 +0000 Subject: [PATCH 10/31] check for duplicates in putElement --- .../Project/ProjectEntityHandler.coffee | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index c4992a29f8..750924beb3 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -170,7 +170,8 @@ module.exports = ProjectEntityHandler = if project_or_id._id? # project return cb(null, project_or_id) else # id - return ProjectGetter.getProjectWithOnlyFolders project_or_id, cb + # need to retrieve full project structure to check for duplicates + return ProjectGetter.getProject project_or_id, {}, cb getProject (error, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add doc" @@ -207,7 +208,7 @@ module.exports = ProjectEntityHandler = ProjectEntityHandler.addDoc project_id, null, name, lines, callback addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> - ProjectGetter.getProjectWithOnlyFolders project_id, (err, project) -> + ProjectGetter.getProject project_id, {}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" return callback(err) @@ -606,19 +607,32 @@ module.exports = ProjectEntityHandler = newPath = fileSystem: "#{path.fileSystem}/#{element.name}" mongo: path.mongo - id = element._id+'' - element._id = require('mongoose').Types.ObjectId(id) - conditions = _id:project._id - mongopath = "#{path.mongo}.#{type}" - 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.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}, project) + ProjectEntityHandler.checkElementName folder, element.name, (err) => + return callback(err) if err? + id = element._id+'' + element._id = require('mongoose').Types.ObjectId(id) + conditions = _id:project._id + mongopath = "#{path.mongo}.#{type}" + 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.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}, project) + checkElementName: (folder, name, callback = (err) ->) -> + # check if the name is already taken by a doc, file or + # folder. If so, return an error "file already exists". + err = new Errors.InvalidNameError("file already exists") + for doc in folder?.docs or [] + return callback(err) if doc.name is name + for file in folder?.fileRefs or [] + return callback(err) if file.name is name + for folder in folder?.folders or [] + return callback(err) if folder.name is name + callback() confirmFolder = (project, folder_id, callback)-> logger.log folder_id:folder_id, project_id:project._id, "confirming folder in project" From 6e0881f85d80154e2b2ccd7ff56836068e077bb5 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 19 Jan 2018 12:02:04 +0000 Subject: [PATCH 11/31] check for duplicates in addFolder --- .../Project/ProjectEntityHandler.coffee | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 750924beb3..d606b94c40 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -341,7 +341,7 @@ module.exports = ProjectEntityHandler = callback(null, folders, lastFolder) addFolder: (project_id, parentFolder_id, folderName, callback) -> - ProjectGetter.getProjectWithOnlyFolders project_id, (err, project)=> + ProjectGetter.getProject project_id, {}, (err, project)=> if err? logger.err project_id:project_id, err:err, "error getting project for add folder" return callback(err) @@ -459,17 +459,20 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath)=> return callback(error) if error? - endPath = path.join(path.dirname(entPath.fileSystem), newName) - conditions = {_id:project_id} - update = "$set":{} - namePath = entPath.mongo+".name" - update["$set"][namePath] = newName - tpdsUpdateSender.moveEntity({project_id:project_id, startPath:entPath.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, newFiles) => + ProjectEntityHandler.checkElementName entity, newName, (err) => + return callback(err) if err? + endPath = path.join(path.dirname(entPath.fileSystem), newName) + conditions = {_id:project_id} + update = "$set":{} + namePath = entPath.mongo+".name" + update["$set"][namePath] = newName + # FIXME check if this would create a duplicate file! + tpdsUpdateSender.moveEntity({project_id:project_id, startPath:entPath.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) + Project.findOneAndUpdate conditions, update, { "new": true}, (error, newProject) -> return callback(error) if error? - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback + ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs, newFiles) => + return callback(error) if error? + DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> if(entityType.indexOf("file") != -1) From e421192b5c570d0b9adf54a6afaccf56ae2d62f7 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 18 Jan 2018 16:22:38 +0000 Subject: [PATCH 12/31] check for duplicates on rename --- .../coffee/Features/Project/ProjectEntityHandler.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index d606b94c40..74010fba33 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -457,16 +457,16 @@ module.exports = ProjectEntityHandler = return callback(error) if error? ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? - projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath)=> + projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath, parentFolder)=> return callback(error) if error? - ProjectEntityHandler.checkElementName entity, newName, (err) => - return callback(err) if err? + # check if the new name already exists in the current folder + ProjectEntityHandler.checkElementName parentFolder, newName, (error) => + return callback(error) if error? endPath = path.join(path.dirname(entPath.fileSystem), newName) conditions = {_id:project_id} update = "$set":{} namePath = entPath.mongo+".name" update["$set"][namePath] = newName - # FIXME check if this would create a duplicate file! tpdsUpdateSender.moveEntity({project_id:project_id, startPath:entPath.fileSystem, endPath:endPath, project_name:project.name, rev:entity.rev}) Project.findOneAndUpdate conditions, update, { "new": true}, (error, newProject) -> return callback(error) if error? From e183956d165cc9191a10ec8881eaceaf892bf13b Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 18 Jan 2018 16:42:26 +0000 Subject: [PATCH 13/31] check for duplicates on move --- .../Project/ProjectEntityHandler.coffee | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 74010fba33..89fc9fb42a 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -395,7 +395,7 @@ module.exports = ProjectEntityHandler = return callback(err) if err? projectLocator.findElement {project, element_id: entity_id, type: entityType}, (err, entity, entityPath)-> return callback(err) if err? - self._checkValidMove project, entityType, entityPath, destFolderId, (error) -> + self._checkValidMove project, entityType, entity, entityPath, destFolderId, (error) -> return callback(error) if error? self.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? @@ -414,19 +414,20 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback - _checkValidMove: (project, entityType, entityPath, destFolderId, callback = (error) ->) -> - return callback() if !entityType.match(/folder/) - + _checkValidMove: (project, entityType, entity, entityPath, destFolderId, callback = (error) ->) -> 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 + # check if there is already a doc/file/folder with the same name + # in the destination folder + ProjectEntityHandler.checkElementName destEntity, entity.name, (err)-> + return callback(err) if err? + if entityType.match(/folder/) + 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 + return callback(new Error("destination folder is a child folder of me")) callback() - 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" From ea9976994a27334e7593801ea54a3ba48bedbce4 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 18 Jan 2018 14:28:36 +0000 Subject: [PATCH 14/31] update unit tests for duplicate checks --- .../Project/ProjectEntityHandlerTests.coffee | 140 +++++++++++++++++- 1 file changed, 138 insertions(+), 2 deletions(-) diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index 3b6a6e8626..39c1eba75d 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -62,7 +62,7 @@ describe 'ProjectEntityHandler', -> @ProjectGetter = getProjectWithOnlyFolders : (project_id, callback)=> callback(null, @project) getProjectWithoutDocLines : (project_id, callback)=> callback(null, @project) - getProject:sinon.stub() + getProject: sinon.stub().callsArgWith(2, null, @project) @projectUpdater = markAsUpdated:sinon.stub() @projectLocator = findElement : sinon.stub() @@ -287,6 +287,8 @@ describe 'ProjectEntityHandler', -> @projectLocator.findElement = sinon.stub() @projectLocator.findElement.withArgs({project: @project, element_id: @docId, type: 'docs'}) .callsArgWith(1, null, @doc, @path) + @projectLocator.findElement.withArgs({project: @project, element_id: folder_id, type:"folder"},) + .callsArgWith(1, null, @destFolder, @destFolderPath) @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", userId, done it 'should find the doc to move', -> @@ -315,6 +317,46 @@ describe 'ProjectEntityHandler', -> }) .should.equal true + describe "moving a doc when another with the same name already exists", -> + beforeEach () -> + @docId = "4eecaffcbffa66588e000009" + @doc = { name: "another-doc.tex", lines:["1234","312343d"], rev: "1234"} + @path = { + mongo:"folders[0]" + fileSystem:"/old_folder/somewhere.txt" + } + @destFolder = { name: "folder", docs: [ {name:"another-doc.tex"} ] } + @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) + @projectLocator.findElement.withArgs({project: @project, element_id: folder_id, type:"folder"},) + .callsArgWith(1, null, @destFolder, @destFolderPath) + @callback = sinon.stub() + @ProjectEntityHandler.moveEntity project_id, @docId, folder_id, "docs", userId, @callback + + it 'should return an error', -> + @callback.calledWith(new Errors.InvalidNameError("file already exists")).should.equal true + + it "should should not send the update to the doc updater", -> + @documentUpdaterHandler.updateProjectStructure + .called.should.equal false + + it 'should not remove the element from its current position', -> + @ProjectEntityHandler._removeElementFromMongoArray + .called.should.equal false + + it "should not put the element back in the new folder", -> + @ProjectEntityHandler._putElement.called.should.equal false + + it 'should not tell the third party data store', -> + @tpdsUpdateSender.moveEntity + .called.should.equal false + + describe "moving a folder", -> beforeEach -> @folder_id = "folder-to-move" @@ -379,10 +421,37 @@ describe 'ProjectEntityHandler', -> }) .should.equal true + describe "when the destination folder contains a file with the same name", -> + beforeEach -> + @path.fileSystem = "/one/src_dir" + @pathToMoveTo.fileSystem = "/two/dest_dir" + @folder_to_move_to = { name: "folder to move to", fileRefs: [ {name: "folder"}] } + @projectLocator.findElement.withArgs({project: @project, element_id: @move_to_folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder_to_move_to, @pathToMoveTo) + @callback = sinon.stub() + @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, @callback + + it 'should find the folder we are moving to element', -> + @projectLocator.findElement + .calledWith({ + element_id: @move_to_folder_id, + type: "folder", + project: @project + }) + .should.equal true + + it "should return an error", -> + @callback + .calledWith(new Errors.InvalidNameError("file already exists")) + .should.equal true + describe "when the destination folder is inside the moving folder", -> beforeEach -> @path.fileSystem = "/one/two" @pathToMoveTo.fileSystem = "/one/two/three" + + @projectLocator.findElement.withArgs({project: @project, element_id: @move_to_folder_id, type: 'folder'}) + .callsArgWith(1, null, @folder_to_move_to, @pathToMoveTo) @callback = sinon.stub() @ProjectEntityHandler.moveEntity project_id, @folder_id, @move_to_folder_id, "folder", userId, @callback @@ -472,6 +541,7 @@ describe 'ProjectEntityHandler', -> @lines = ['1234','abc'] @path = "/path/to/doc" + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project) @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}}) @callback = sinon.stub() @tpdsUpdateSender.addDoc = sinon.stub().callsArg(1) @@ -522,6 +592,7 @@ describe 'ProjectEntityHandler', -> @lines = ['1234','abc'] @path = "/path/to/doc" + @ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, @project) @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}}) @callback = sinon.stub() @tpdsUpdateSender.addDoc = sinon.stub().callsArg(1) @@ -1123,6 +1194,20 @@ describe 'ProjectEntityHandler', -> @newName = "new.tex" @path = mongo: "mongo.path", fileSystem: "/oldnamepath/oldname" + @project_id = project_id + @project = + _id: ObjectId(project_id) + rootFolder: [_id:ObjectId()] + @folder = + _id: ObjectId() + name: "someFolder" + docs: [ {name: "another-doc.tex"} ] + fileRefs: [ {name: "another-file.tex"} ] + folders: [ {name: "another-folder"} ] + @doc = + _id: ObjectId() + name: "new.tex" + @ProjectGetter.getProject.callsArgWith(2, null, @project) @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() @ProjectEntityHandler.getAllEntitiesFromProject @@ -1132,7 +1217,7 @@ describe 'ProjectEntityHandler', -> .onSecondCall() .callsArgWith(1, null, @newDocs = ['new-doc'], @newFiles = ['new-file']) - @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: @entity_id, name:"oldname", rev:4 }, @path) + @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: @entity_id, name:"oldname", rev:4 }, @path, @folder) @tpdsUpdateSender.moveEntity = sinon.stub() @ProjectModel.findOneAndUpdate = sinon.stub().callsArgWith(3, null, @project) @documentUpdaterHandler.updateProjectStructure = sinon.stub().yields() @@ -1154,6 +1239,27 @@ describe 'ProjectEntityHandler', -> @tpdsUpdateSender.moveEntity.calledWith({project_id:project_id, startPath:@path.fileSystem, endPath:"/oldnamepath/new.tex", project_name:@project.name, rev:4}).should.equal true done() + describe "when a document already exists with the same name", -> + beforeEach -> + @project = + _id: ObjectId(project_id) + rootFolder: [_id:ObjectId()] + @folder = + _id: ObjectId() + name: "someFolder" + docs: [ {name: "another-doc.tex"} ] + fileRefs: [ {name: "another-file.tex"} ] + folders: [ {name: "another-folder"} ] + @doc = + _id: ObjectId() + name: "new.tex" + @newName = "another-doc.tex" + + it "should return an error", (done)-> + @ProjectEntityHandler.renameEntity project_id, @entity_id, @entityType, @newName, userId, (err)=> + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + describe "_insertDeletedDocReference", -> beforeEach -> @doc = @@ -1248,6 +1354,9 @@ describe 'ProjectEntityHandler', -> @folder = _id: ObjectId() name: "someFolder" + docs: [ {name: "another-doc.tex"} ] + fileRefs: [ {name: "another-file.tex"} ] + folders: [ {name: "another-folder"} ] @doc = _id: ObjectId() name: "new.tex" @@ -1290,6 +1399,33 @@ describe 'ProjectEntityHandler', -> @ProjectModel.findOneAndUpdate.called.should.equal false done() + it "should error if a document already exists with the same name", (done)-> + doc = + _id: ObjectId() + name: "another-doc.tex" + @ProjectEntityHandler._putElement @project, @folder, doc, "doc", (err)=> + @ProjectModel.findOneAndUpdate.called.should.equal false + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + + it "should error if a file already exists with the same name", (done)-> + doc = + _id: ObjectId() + name: "another-file.tex" + @ProjectEntityHandler._putElement @project, @folder, doc, "doc", (err)=> + @ProjectModel.findOneAndUpdate.called.should.equal false + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + + it "should error if a folder already exists with the same name", (done)-> + doc = + _id: ObjectId() + name: "another-folder" + @ProjectEntityHandler._putElement @project, @folder, doc, "doc", (err)=> + @ProjectModel.findOneAndUpdate.called.should.equal false + err.should.deep.equal new Errors.InvalidNameError("file already exists") + done() + describe "_countElements", -> beforeEach -> From 9d74a0a2f0d5e7a7838f9dd215a6e878518554e8 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 17 Jan 2018 12:11:02 +0000 Subject: [PATCH 15/31] handle errors normally in addFolder modal --- .../web/app/coffee/Features/Editor/EditorHttpController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 16b1a79e31..614781211c 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -105,7 +105,7 @@ module.exports = EditorHttpController = else if error?.message == 'invalid element name' res.status(400).json(req.i18n.translate('invalid_file_name')) else if error? - res.status(500).json(req.i18n.translate('generic_something_went_wrong')) + next(error) else res.json doc From 8e239e0c648b7b54918817c3332a8821c2dafd1c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 19 Jan 2018 11:27:22 +0000 Subject: [PATCH 16/31] only update client filetree on success --- .../coffee/ide/file-tree/FileTreeManager.coffee | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee index 390b9dba79..d46525bf20 100644 --- a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee +++ b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee @@ -341,12 +341,12 @@ define [ renameEntity: (entity, name, callback = (error) ->) -> return if entity.name == name - if name.length < 150 - entity.name = name - return @ide.$http.post "/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/rename", { - name: entity.name, + return if name.length >= 150 + @ide.$http.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/rename", { + name: name, _csrf: window.csrfToken - } + }).then () -> + entity.name = name deleteEntity: (entity, callback = (error) ->) -> # We'll wait for the socket.io notification to @@ -362,11 +362,11 @@ define [ # Abort move if the folder being moved (entity) has the parent_folder as child # since that would break the tree structure. return if @_isChildFolder(entity, parent_folder) - @_moveEntityInScope(entity, parent_folder) - return @ide.queuedHttp.post "/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/move", { + @ide.queuedHttp.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/move", { folder_id: parent_folder.id _csrf: window.csrfToken - } + }).then () => + @_moveEntityInScope(entity, parent_folder) _isChildFolder: (parent_folder, child_folder) -> parent_path = @getEntityPath(parent_folder) or "" # null if root folder From 7ba17a8875a2c2b2f61f7565bc9d80442144b6b4 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 23 Jan 2018 10:21:05 +0000 Subject: [PATCH 17/31] only need to load rootFolder from project --- .../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 89fc9fb42a..470545b8f9 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -171,7 +171,7 @@ module.exports = ProjectEntityHandler = return cb(null, project_or_id) else # id # need to retrieve full project structure to check for duplicates - return ProjectGetter.getProject project_or_id, {}, cb + return ProjectGetter.getProject project_or_id, {rootFolder:true, name:true}, cb getProject (error, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add doc" @@ -208,7 +208,7 @@ module.exports = ProjectEntityHandler = ProjectEntityHandler.addDoc project_id, null, name, lines, callback addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> - ProjectGetter.getProject project_id, {}, (err, project) -> + ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" return callback(err) @@ -341,7 +341,7 @@ module.exports = ProjectEntityHandler = callback(null, folders, lastFolder) addFolder: (project_id, parentFolder_id, folderName, callback) -> - ProjectGetter.getProject project_id, {}, (err, project)=> + ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project)=> if err? logger.err project_id:project_id, err:err, "error getting project for add folder" return callback(err) From 302bbe889333c80f9698a1f3fd25bcb5ab9e3008 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 23 Jan 2018 10:35:38 +0000 Subject: [PATCH 18/31] rename checkElementName to checkValidElementName --- .../coffee/Features/Project/ProjectEntityHandler.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 470545b8f9..496bb73bd5 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -419,7 +419,7 @@ module.exports = ProjectEntityHandler = return callback(err) if err? # check if there is already a doc/file/folder with the same name # in the destination folder - ProjectEntityHandler.checkElementName destEntity, entity.name, (err)-> + ProjectEntityHandler.checkValidElementName destEntity, entity.name, (err)-> return callback(err) if err? if entityType.match(/folder/) logger.log destFolderPath: destFolderPath.fileSystem, folderPath: entityPath.fileSystem, "checking folder is not moving into child folder" @@ -461,7 +461,7 @@ module.exports = ProjectEntityHandler = projectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath, parentFolder)=> return callback(error) if error? # check if the new name already exists in the current folder - ProjectEntityHandler.checkElementName parentFolder, newName, (error) => + ProjectEntityHandler.checkValidElementName parentFolder, newName, (error) => return callback(error) if error? endPath = path.join(path.dirname(entPath.fileSystem), newName) conditions = {_id:project_id} @@ -611,7 +611,7 @@ module.exports = ProjectEntityHandler = newPath = fileSystem: "#{path.fileSystem}/#{element.name}" mongo: path.mongo - ProjectEntityHandler.checkElementName folder, element.name, (err) => + ProjectEntityHandler.checkValidElementName folder, element.name, (err) => return callback(err) if err? id = element._id+'' element._id = require('mongoose').Types.ObjectId(id) @@ -626,7 +626,7 @@ module.exports = ProjectEntityHandler = return callback(err) callback(err, {path:newPath}, project) - checkElementName: (folder, name, callback = (err) ->) -> + checkValidElementName: (folder, name, callback = (err) ->) -> # check if the name is already taken by a doc, file or # folder. If so, return an error "file already exists". err = new Errors.InvalidNameError("file already exists") From 68ac597a935aae7aab402eee0c0cc8c6ce01d563 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 25 Jan 2018 16:58:33 +0000 Subject: [PATCH 19/31] initial acceptance tests --- .../coffee/ProjectDuplicateNameTests.coffee | 288 ++++++++++++++++++ 1 file changed, 288 insertions(+) create mode 100644 services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee diff --git a/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee new file mode 100644 index 0000000000..3002663d28 --- /dev/null +++ b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee @@ -0,0 +1,288 @@ +async = require "async" +expect = require("chai").expect +sinon = require "sinon" +mkdirp = require "mkdirp" +ObjectId = require("mongojs").ObjectId +Path = require "path" +fs = require "fs" +Settings = require "settings-sharelatex" +_ = require "underscore" + +ProjectGetter = require "../../../app/js/Features/Project/ProjectGetter.js" + +MockDocStoreApi = require './helpers/MockDocstoreApi' +MockFileStoreApi = require './helpers/MockFileStoreApi' +request = require "./helpers/request" +User = require "./helpers/User" + +describe "ProjectDuplicateNames", -> + before (done) -> + @owner = new User() + @owner.login done + @project = {} + @callback = sinon.stub() + + describe "creating a project from the example template", -> + before (done) -> + @owner.createProject "example-project", {template: "example"}, (error, project_id) => + throw error if error? + @example_project_id = project_id + @owner.getProject project_id, (error, project) => + @project = project + @mainTexDoc = _.find(project.rootFolder[0].docs, (doc) -> doc.name is 'main.tex') + @refBibDoc = _.find(project.rootFolder[0].docs, (doc) -> doc.name is 'references.bib') + @imageFile = _.find(project.rootFolder[0].fileRefs, (file) -> file.name is 'universe.jpg') + @rootFolderId = project.rootFolder[0]._id.toString() + # create a folder called 'testfolder' + @owner.request.post { + uri: "/project/#{@example_project_id}/folder" + json: + name: "testfolder" + parent_folder_id: @rootFolderId + }, (err, res, body) => + console.log "GOT FOLDER", body + @testFolderId = body._id + done() + + it "should create a project", -> + expect(@project.rootFolder[0].docs.length).to.equal(2) + expect(@project.rootFolder[0].fileRefs.length).to.equal(1) + + it "should create two docs in the docstore", -> + docs = MockDocStoreApi.docs[@example_project_id] + expect(Object.keys(docs).length).to.equal(2) + + it "should create one file in the filestore", -> + files = MockFileStoreApi.files[@example_project_id] + expect(Object.keys(files).length).to.equal(1) + + describe "for an existing doc", -> + describe "trying to add a doc with the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/doc" + json: + name: "main.tex" + parent_folder_id: @rootFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to add a folder with the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/folder" + json: + name: "main.tex" + parent_folder_id: @rootFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to upload a file with the same name", -> + before (done) -> + @owner.request.post + uri: "/project/#{@example_project_id}/upload" + json: true + qs: + folder_id: @rootFolderId + qqfilename: "main.tex" + formData: + qqfile: + value: fs.createReadStream Path.resolve(__dirname + '/../files/1pixel.png') + options: + filename: 'main.tex', + contentType: 'image/png' + , (err, res, body) => + @body = body + done() + + it "should respond with failure status", -> + expect(@body.success).to.equal false + + describe "trying to add a folder with the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/folder" + json: + name: "main.tex" + parent_folder_id: @rootFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to upload a file with the same name", -> + before (done) -> + @owner.request.post + uri: "/project/#{@example_project_id}/upload" + json: true + qs: + folder_id: @rootFolderId + qqfilename: "main.tex" + formData: + qqfile: + value: fs.createReadStream Path.resolve(__dirname + '/../files/1pixel.png') + options: + filename: 'main.tex', + contentType: 'image/png' + , (err, res, body) => + @body = body + done() + + it "should respond with failure status", -> + expect(@body.success).to.equal false + + + describe "for an existing file", -> + describe "trying to add a doc with the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/doc" + json: + name: "universe.jpg" + parent_folder_id: @rootFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to add a folder with the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/folder" + json: + name: "universe.jpg" + parent_folder_id: @rootFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to upload a file with the same name", -> + before (done) -> + @owner.request.post + uri: "/project/#{@example_project_id}/upload" + json: true + qs: + folder_id: @rootFolderId + qqfilename: "universe.jpg" + formData: + qqfile: + value: fs.createReadStream Path.resolve(__dirname + '/../files/1pixel.png') + options: + filename: 'universe.jpg', + contentType: 'image/jpeg' + , (err, res, body) => + @body = body + done() + + it "should succeed (overwriting the file)", -> + expect(@body.success).to.equal true + + describe "for an existing folder", -> + describe "trying to add a doc with the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/doc" + json: + name: "testfolder" + parent_folder_id: @rootFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to add a folder with the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/folder" + json: + name: "testfolder" + parent_folder_id: @rootFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to upload a file with the same name", -> + before (done) -> + @owner.request.post + uri: "/project/#{@example_project_id}/upload" + json: true + qs: + folder_id: @rootFolderId + qqfilename: "universe.jpg" + formData: + qqfile: + value: fs.createReadStream Path.resolve(__dirname + '/../files/1pixel.png') + options: + filename: 'testfolder', + contentType: 'image/jpeg' + , (err, res, body) => + @body = body + done() + + it "should respond with failure status", -> + expect(@body.success).to.equal false + + + describe "for an existing doc", -> + describe "trying to rename a doc to the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/doc/#{@refBibDoc._id}/rename" + json: + name: "main.tex" + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to rename a folder to the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/folder/#{@testFolderId}/rename" + json: + name: "main.tex" + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to rename a file to the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/file/#{@imageFile._id}/rename" + json: + name: "main.tex" + }, (err, res, body) => + @res = res + done() + + it "should respond with failure status", -> + expect(@res.statusCode).to.equal 400 + + + # webRouter.post '/project/:Project_id/:entity_type/:entity_id/rename', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.renameEntity + # webRouter.post '/project/:Project_id/:entity_type/:entity_id/move', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.moveEntity From 69bed6dbb2a929192bb9ce4ad878e7293a5cc490 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 25 Jan 2018 16:58:48 +0000 Subject: [PATCH 20/31] fix MockFileStoreApi to record uploaded files --- .../test/acceptance/coffee/helpers/MockFileStoreApi.coffee | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/services/web/test/acceptance/coffee/helpers/MockFileStoreApi.coffee b/services/web/test/acceptance/coffee/helpers/MockFileStoreApi.coffee index f3022302f4..f5fa79b641 100644 --- a/services/web/test/acceptance/coffee/helpers/MockFileStoreApi.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockFileStoreApi.coffee @@ -8,8 +8,11 @@ module.exports = MockFileStoreApi = app.post "/project/:project_id/file/:file_id", (req, res, next) => req.on 'data', -> - req.on 'end', -> - res.send 200 + req.on 'end', => + {project_id, file_id} = req.params + @files[project_id] ?= {} + @files[project_id][file_id] = { content : "test-file-content" } + res.sendStatus 200 app.listen 3009, (error) -> throw error if error? From aa5d8809022ad6e29128ed888f3696133c3dc610 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 25 Jan 2018 16:59:08 +0000 Subject: [PATCH 21/31] don't update project structure if file not created --- .../web/app/coffee/Features/Project/ProjectEntityHandler.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 496bb73bd5..43133d3dc4 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -230,6 +230,7 @@ module.exports = ProjectEntityHandler = addFile: (project_id, folder_id, fileName, fsPath, userId, callback = (error, fileRef, folder_id) ->)-> ProjectEntityHandler.addFileWithoutUpdatingHistory project_id, folder_id, fileName, fsPath, userId, (error, fileRef, folder_id, path, fileStoreUrl) -> + return callback(error) if error? newFiles = [ file: fileRef path: path From cd2688a740fd1c0a25d1a6117618f1777241084f Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 26 Jan 2018 17:00:33 +0000 Subject: [PATCH 22/31] added tests for rename/move --- .../coffee/ProjectDuplicateNameTests.coffee | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) diff --git a/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee index 3002663d28..c911c2a672 100644 --- a/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee @@ -284,5 +284,175 @@ describe "ProjectDuplicateNames", -> expect(@res.statusCode).to.equal 400 + describe "for an existing file", -> + describe "trying to rename a doc to the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/doc/#{@refBibDoc._id}/rename" + json: + name: "universe.jpg" + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to rename a folder to the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/folder/#{@testFolderId}/rename" + json: + name: "universe.jpg" + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to rename a file to the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/file/#{@imageFile._id}/rename" + json: + name: "universe.jpg" + }, (err, res, body) => + @res = res + done() + + it "should respond with failure status", -> + expect(@res.statusCode).to.equal 400 + + + describe "for an existing folder", -> + describe "trying to rename a doc to the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/doc/#{@refBibDoc._id}/rename" + json: + name: "testfolder" + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to rename a folder to the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/folder/#{@testFolderId}/rename" + json: + name: "testfolder" + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to rename a file to the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/file/#{@imageFile._id}/rename" + json: + name: "testfolder" + }, (err, res, body) => + @res = res + done() + + it "should respond with failure status", -> + expect(@res.statusCode).to.equal 400 + + + describe "for an existing folder with a file with the same name", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/doc" + json: + name: "main.tex" + parent_folder_id: @testFolderId + }, (err, res, body) => + @owner.request.post { + uri: "/project/#{@example_project_id}/doc" + json: + name: "universe.jpg" + parent_folder_id: @testFolderId + }, (err, res, body) => + @owner.request.post { + uri: "/project/#{@example_project_id}/folder" + json: + name: "otherFolder" + parent_folder_id: @testFolderId + }, (err, res, body) => + @subFolderId = body._id + @owner.request.post { + uri: "/project/#{@example_project_id}/folder" + json: + name: "otherFolder" + parent_folder_id: @rootFolderId + }, (err, res, body) => + @otherFolderId = body._id + done() + + describe "trying to move a doc into the folder", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/doc/#{@mainTexDoc._id}/move" + json: + folder_id: @testFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to move a file into the folder", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/file/#{@imageFile._id}/move" + json: + folder_id: @testFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to move a folder into the folder", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/folder/#{@otherFolderId}/move" + json: + folder_id: @testFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + describe "trying to move a folder into a subfolder of itself", -> + before (done) -> + @owner.request.post { + uri: "/project/#{@example_project_id}/folder/#{@testFolderId}/move" + json: + folder_id: @subFolderId + }, (err, res, body) => + @res = res + done() + + it "should respond with 400 error status", -> + expect(@res.statusCode).to.equal 400 + + + + + + + # webRouter.post '/project/:Project_id/:entity_type/:entity_id/rename', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.renameEntity # webRouter.post '/project/:Project_id/:entity_type/:entity_id/move', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.moveEntity From b30dd22f0e976e8e3e43703a65fcaeb35abe2be6 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 26 Jan 2018 17:00:55 +0000 Subject: [PATCH 23/31] return a 400 status code for invalid moves was previously returning 500 --- .../coffee/Features/Editor/EditorHttpController.coffee | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 614781211c..88fc1e4cc1 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -128,8 +128,12 @@ module.exports = EditorHttpController = folder_id = req.body.folder_id 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 + if error?.message == 'destination folder is a child folder of me' + res.status(400).json(req.i18n.translate('invalid_file_name')) + else if error? + next(error) + else + res.sendStatus 204 deleteDoc: (req, res, next)-> req.params.entity_type = "doc" From 2f52e6c4b41ad1d221de9413301ad28aa934d618 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 29 Jan 2018 10:38:25 +0000 Subject: [PATCH 24/31] remove comments --- .../acceptance/coffee/ProjectDuplicateNameTests.coffee | 9 --------- 1 file changed, 9 deletions(-) diff --git a/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee index c911c2a672..c175efe40a 100644 --- a/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee @@ -447,12 +447,3 @@ describe "ProjectDuplicateNames", -> it "should respond with 400 error status", -> expect(@res.statusCode).to.equal 400 - - - - - - - - # webRouter.post '/project/:Project_id/:entity_type/:entity_id/rename', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.renameEntity - # webRouter.post '/project/:Project_id/:entity_type/:entity_id/move', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.moveEntity From 63fa024d98c8008d34d75baa99f9d4fd77ee5a03 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 29 Jan 2018 15:24:51 +0000 Subject: [PATCH 25/31] client-side duplicate check for move/rename/create --- .../ide/file-tree/FileTreeManager.coffee | 26 +++++++++++++++++++ .../web/public/coffee/ide/services/ide.coffee | 3 ++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee index d46525bf20..4b8af090bc 100644 --- a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee +++ b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee @@ -321,7 +321,20 @@ define [ return null + existsInThisFolder: (folder, name) -> + for entity in folder?.children or [] + return true if entity.name is name + return false + + nameExistsError: (message = "already exists") -> + nameExists = @ide.$q.defer() + nameExists.reject({data: message}) + return nameExists.promise + createDoc: (name, parent_folder = @getCurrentFolder()) -> + # check if a doc/file/folder already exists with this name + if @existsInThisFolder parent_folder, name + return @nameExistsError() # We'll wait for the socket.io notification to actually # add the doc for us. @ide.$http.post "/project/#{@ide.project_id}/doc", { @@ -331,6 +344,9 @@ define [ } createFolder: (name, parent_folder = @getCurrentFolder()) -> + # check if a doc/file/folder already exists with this name + if @existsInThisFolder parent_folder, name + return @nameExistsError() # We'll wait for the socket.io notification to actually # add the folder for us. return @ide.$http.post "/project/#{@ide.project_id}/folder", { @@ -342,6 +358,12 @@ define [ renameEntity: (entity, name, callback = (error) ->) -> return if entity.name == name return if name.length >= 150 + # check if a doc/file/folder already exists with this name + parent_folder = @getCurrentFolder() + if @existsInThisFolder parent_folder, name + return @nameExistsError() + # We'll wait for the socket.io notification to actually + # do the rename for us. @ide.$http.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/rename", { name: name, _csrf: window.csrfToken @@ -362,6 +384,10 @@ define [ # Abort move if the folder being moved (entity) has the parent_folder as child # since that would break the tree structure. return if @_isChildFolder(entity, parent_folder) + # check if a doc/file/folder already exists with this name + if @existsInThisFolder entity.name, parent_folder + return @nameExistsError() + # Wait for the http response before doing the move @ide.queuedHttp.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/move", { folder_id: parent_folder.id _csrf: window.csrfToken diff --git a/services/web/public/coffee/ide/services/ide.coffee b/services/web/public/coffee/ide/services/ide.coffee index 57e0bbef3d..6462859df2 100644 --- a/services/web/public/coffee/ide/services/ide.coffee +++ b/services/web/public/coffee/ide/services/ide.coffee @@ -3,10 +3,11 @@ define [ ], (App) -> # We create and provide this as service so that we can access the global ide # from within other parts of the angular app. - App.factory "ide", ["$http", "queuedHttp", "$modal", ($http, queuedHttp, $modal) -> + App.factory "ide", ["$http", "queuedHttp", "$modal", "$q", ($http, queuedHttp, $modal, $q) -> ide = {} ide.$http = $http ide.queuedHttp = queuedHttp + ide.$q = $q @recentEvents = [] ide.pushEvent = (type, meta = {}) => From 6ed181d52c4e159e166ce46e30d6a3e500117585 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 29 Jan 2018 15:27:52 +0000 Subject: [PATCH 26/31] fix check for name in filetree move --- services/web/public/coffee/ide/file-tree/FileTreeManager.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee index 4b8af090bc..352f28a7bd 100644 --- a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee +++ b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee @@ -385,7 +385,7 @@ define [ # since that would break the tree structure. return if @_isChildFolder(entity, parent_folder) # check if a doc/file/folder already exists with this name - if @existsInThisFolder entity.name, parent_folder + if @existsInThisFolder parent_folder, entity.name return @nameExistsError() # Wait for the http response before doing the move @ide.queuedHttp.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/move", { From 1c6dc66ed144df7f47fdeb695b9b675f16faf99d Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 29 Jan 2018 16:06:56 +0000 Subject: [PATCH 27/31] prevent double calls to rename --- .../ide/file-tree/controllers/FileTreeEntityController.coffee | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/web/public/coffee/ide/file-tree/controllers/FileTreeEntityController.coffee b/services/web/public/coffee/ide/file-tree/controllers/FileTreeEntityController.coffee index e6573bbe57..55bf16a172 100644 --- a/services/web/public/coffee/ide/file-tree/controllers/FileTreeEntityController.coffee +++ b/services/web/public/coffee/ide/file-tree/controllers/FileTreeEntityController.coffee @@ -28,6 +28,9 @@ define [ invalidModalShowing = false $scope.finishRenaming = () -> + # avoid double events when blur and on-enter fire together + return if !$scope.entity.renaming + name = $scope.inputs.name if !name.match(new RegExp(ide.validFileRegex)) From 9721dffbb6b614ccdc6af6d472a87b56a5f86a81 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 31 Jan 2018 09:34:17 +0000 Subject: [PATCH 28/31] Add missing state reset to error handler --- .../coffee/ide/file-tree/controllers/FileTreeController.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/public/coffee/ide/file-tree/controllers/FileTreeController.coffee b/services/web/public/coffee/ide/file-tree/controllers/FileTreeController.coffee index 28964b9090..aee631f334 100644 --- a/services/web/public/coffee/ide/file-tree/controllers/FileTreeController.coffee +++ b/services/web/public/coffee/ide/file-tree/controllers/FileTreeController.coffee @@ -69,6 +69,7 @@ define [ .catch (response)-> { data } = response $scope.error = data + $scope.state.inflight = false $scope.cancel = () -> $modalInstance.dismiss('cancel') From aa6c16593e1509abdc28225cedcaf8676ecd8d8e Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 31 Jan 2018 09:34:34 +0000 Subject: [PATCH 29/31] Add renamingToName override name for instant apparent renames --- services/web/app/views/project/editor/file-tree.pug | 4 ++-- .../public/coffee/ide/file-tree/FileTreeManager.coffee | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/services/web/app/views/project/editor/file-tree.pug b/services/web/app/views/project/editor/file-tree.pug index cfbc9cedaf..fade8e0782 100644 --- a/services/web/app/views/project/editor/file-tree.pug +++ b/services/web/app/views/project/editor/file-tree.pug @@ -110,7 +110,7 @@ script(type='text/ng-template', id='entityListItemTemplate') i.fa.fa-fw(ng-if="entity.type != 'folder'", ng-class="'fa-' + iconTypeFromName(entity.name)") span( ng-hide="entity.renaming" - ) {{ entity.name }} + ) {{ entity.renamingToName || entity.name }} span.rename-input input( ng-if="permissions.write", @@ -198,7 +198,7 @@ script(type='text/ng-template', id='entityListItemTemplate') span( ng-hide="entity.renaming" - ) {{ entity.name }} + ) {{ entity.renamingToName || entity.name }} span.rename-input input( ng-if="permissions.write", diff --git a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee index 352f28a7bd..9891bd8e74 100644 --- a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee +++ b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee @@ -362,13 +362,15 @@ define [ parent_folder = @getCurrentFolder() if @existsInThisFolder parent_folder, name return @nameExistsError() - # We'll wait for the socket.io notification to actually - # do the rename for us. + entity.renamingToName = name @ide.$http.post("/project/#{@ide.project_id}/#{entity.type}/#{entity.id}/rename", { name: name, _csrf: window.csrfToken - }).then () -> - entity.name = name + }) + .then () -> + entity.name = name + .finally () -> + entity.renamingToName = null deleteEntity: (entity, callback = (error) ->) -> # We'll wait for the socket.io notification to From 21c1ea6687a08fc30f5e0b593a55eacabeca71ff Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 31 Jan 2018 10:26:06 +0000 Subject: [PATCH 30/31] remove comment in test --- .../web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee index c175efe40a..5da94856fc 100644 --- a/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectDuplicateNameTests.coffee @@ -40,7 +40,6 @@ describe "ProjectDuplicateNames", -> name: "testfolder" parent_folder_id: @rootFolderId }, (err, res, body) => - console.log "GOT FOLDER", body @testFolderId = body._id done() From c652abf7393c13e583cb78c5c3333c43dfed1823 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 31 Jan 2018 10:26:31 +0000 Subject: [PATCH 31/31] replace error message with Error object --- .../coffee/Features/Editor/EditorHttpController.coffee | 8 ++------ .../coffee/Features/Project/ProjectEntityHandler.coffee | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 88fc1e4cc1..614781211c 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -128,12 +128,8 @@ module.exports = EditorHttpController = folder_id = req.body.folder_id user_id = AuthenticationController.getLoggedInUserId(req) EditorController.moveEntity project_id, entity_id, folder_id, entity_type, user_id, (error) -> - if error?.message == 'destination folder is a child folder of me' - res.status(400).json(req.i18n.translate('invalid_file_name')) - else if error? - next(error) - else - res.sendStatus 204 + return next(error) if error? + res.sendStatus 204 deleteDoc: (req, res, next)-> req.params.entity_type = "doc" diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 43133d3dc4..b67180659e 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -426,7 +426,7 @@ module.exports = ProjectEntityHandler = 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 - return callback(new Error("destination folder is a child folder of me")) + return callback(new Errors.InvalidNameError("destination folder is a child folder of me")) callback() deleteEntity: (project_id, entity_id, entityType, userId, callback = (error) ->)->