From ba62206b91e478fa137b39dde3b4df278e58e9ea Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 19 May 2017 16:21:02 +0100 Subject: [PATCH 1/3] Refactor project name validation into one place and restrict /s --- .../Features/Editor/EditorController.coffee | 2 +- .../Features/Errors/ErrorController.coffee | 4 ++ .../app/coffee/Features/Errors/Errors.coffee | 9 +++- .../Features/Project/ProjectController.coffee | 22 +++------ .../Project/ProjectCreationHandler.coffee | 29 ++++++----- .../Project/ProjectDetailsHandler.coffee | 34 ++++++++----- .../app/views/project/editor/left-menu.pug | 2 + .../web/app/views/project/list/modals.pug | 12 ++++- .../CloneProjectModalController.coffee | 10 +++- .../controllers/ProjectNameController.coffee | 10 +++- .../ide/settings/services/settings.coffee | 10 ++-- .../project-list/modal-controllers.coffee | 48 ++++++++++++++++--- .../main/project-list/project-list.coffee | 35 +++----------- .../public/coffee/services/queued-http.coffee | 21 ++++---- .../Project/ProjectControllerTests.coffee | 18 ++----- .../ProjectCreationHandlerTests.coffee | 15 ++++++ .../Project/ProjectDetailsHandlerTests.coffee | 29 +++++++++++ .../Project/ProjectEntityHandlerTests.coffee | 1 + 18 files changed, 201 insertions(+), 110 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index a90ab5504a..a2444d34d7 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -176,7 +176,7 @@ module.exports = EditorController = callback?() renameProject: (project_id, newName, callback = (err) ->) -> - ProjectDetailsHandler.renameProject project_id, newName, -> + ProjectDetailsHandler.renameProject project_id, newName, (err) -> if err? logger.err err:err, project_id:project_id, newName:newName, "error renaming project" return callback(err) diff --git a/services/web/app/coffee/Features/Errors/ErrorController.coffee b/services/web/app/coffee/Features/Errors/ErrorController.coffee index 45a743f282..e1929e684a 100644 --- a/services/web/app/coffee/Features/Errors/ErrorController.coffee +++ b/services/web/app/coffee/Features/Errors/ErrorController.coffee @@ -25,6 +25,10 @@ module.exports = ErrorController = else if error instanceof Errors.TooManyRequestsError logger.warn {err: error, url: req.url}, "too many requests error" res.sendStatus(429) + else if error instanceof Errors.InvalidNameError + logger.warn {err: error, url: req.url}, "invalid name error" + res.status(400) + res.send(error.message) else logger.error err: error, url:req.url, method:req.method, user:user, "error passed to top level next middlewear" ErrorController.serverError req, res diff --git a/services/web/app/coffee/Features/Errors/Errors.coffee b/services/web/app/coffee/Features/Errors/Errors.coffee index 56c0ada7d5..2e46dd692d 100644 --- a/services/web/app/coffee/Features/Errors/Errors.coffee +++ b/services/web/app/coffee/Features/Errors/Errors.coffee @@ -5,7 +5,6 @@ NotFoundError = (message) -> return error NotFoundError.prototype.__proto__ = Error.prototype - ServiceNotConfiguredError = (message) -> error = new Error(message) error.name = "ServiceNotConfiguredError" @@ -13,7 +12,6 @@ ServiceNotConfiguredError = (message) -> return error ServiceNotConfiguredError.prototype.__proto__ = Error.prototype - TooManyRequestsError = (message) -> error = new Error(message) error.name = "TooManyRequestsError" @@ -21,8 +19,15 @@ TooManyRequestsError = (message) -> return error TooManyRequestsError.prototype.__proto__ = Error.prototype +InvalidNameError = (message) -> + error = new Error(message) + error.name = "InvalidNameError" + error.__proto__ = InvalidNameError.prototype + return error +InvalidNameError.prototype.__proto__ = Error.prototype module.exports = Errors = NotFoundError: NotFoundError ServiceNotConfiguredError: ServiceNotConfiguredError TooManyRequestsError: TooManyRequestsError + InvalidNameError: InvalidNameError diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index f967a98ffb..92750a5912 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -101,7 +101,7 @@ module.exports = ProjectController = res.send(project_id:project._id) - newProject: (req, res)-> + newProject: (req, res, next)-> user_id = AuthenticationController.getLoggedInUserId(req) projectName = req.body.projectName?.trim() template = req.body.template @@ -113,25 +113,17 @@ module.exports = ProjectController = else projectCreationHandler.createBasicProject user_id, projectName, cb ], (err, project)-> - if err? - logger.error err: err, project: project, user: user_id, name: projectName, templateType: template, "error creating project" - res.sendStatus 500 - else - logger.log project: project, user: user_id, name: projectName, templateType: template, "created project" - res.send {project_id:project._id} + return next(err) if err? + logger.log project: project, user: user_id, name: projectName, templateType: template, "created project" + res.send {project_id:project._id} - renameProject: (req, res)-> + renameProject: (req, res, next)-> project_id = req.params.Project_id newName = req.body.newProjectName - if newName.length > 150 - return res.sendStatus 400 editorController.renameProject project_id, newName, (err)-> - if err? - logger.err err:err, project_id:project_id, newName:newName, "problem renaming project" - res.sendStatus 500 - else - res.sendStatus 200 + return next(err) if err? + res.sendStatus 200 projectListPage: (req, res, next)-> timer = new metrics.Timer("project-list") diff --git a/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee b/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee index ffb69abeac..41b40c11b9 100644 --- a/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee @@ -6,6 +6,7 @@ ObjectId = require('mongoose').Types.ObjectId Project = require('../../models/Project').Project Folder = require('../../models/Folder').Folder ProjectEntityHandler = require('./ProjectEntityHandler') +ProjectDetailsHandler = require('./ProjectDetailsHandler') User = require('../../models/User').User fs = require('fs') Path = require "path" @@ -15,19 +16,21 @@ module.exports = ProjectCreationHandler = createBlankProject : (owner_id, projectName, callback = (error, project) ->)-> metrics.inc("project-creation") - logger.log owner_id:owner_id, projectName:projectName, "creating blank project" - rootFolder = new Folder {'name':'rootFolder'} - project = new Project - owner_ref : new ObjectId(owner_id) - name : projectName - if Settings.currentImageName? - project.imageName = Settings.currentImageName - project.rootFolder[0] = rootFolder - User.findById owner_id, "ace.spellCheckLanguage", (err, user)-> - project.spellCheckLanguage = user.ace.spellCheckLanguage - project.save (err)-> - return callback(err) if err? - callback err, project + ProjectDetailsHandler.validateProjectName projectName, (error) -> + return callback(error) if error? + logger.log owner_id:owner_id, projectName:projectName, "creating blank project" + rootFolder = new Folder {'name':'rootFolder'} + project = new Project + owner_ref : new ObjectId(owner_id) + name : projectName + if Settings.currentImageName? + project.imageName = Settings.currentImageName + project.rootFolder[0] = rootFolder + User.findById owner_id, "ace.spellCheckLanguage", (err, user)-> + project.spellCheckLanguage = user.ace.spellCheckLanguage + project.save (err)-> + return callback(err) if err? + callback err, project createBasicProject : (owner_id, projectName, callback = (error, project) ->)-> self = @ diff --git a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee index 9c823c9f17..8234907ff4 100644 --- a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee @@ -7,8 +7,7 @@ _ = require("underscore") PublicAccessLevels = require("../Authorization/PublicAccessLevels") Errors = require("../Errors/Errors") -module.exports = - +module.exports = ProjectDetailsHandler = getDetails: (project_id, callback)-> ProjectGetter.getProject project_id, {name:true, description:true, compiler:true, features:true, owner_ref:true}, (err, project)-> if err? @@ -39,16 +38,29 @@ module.exports = callback(err) renameProject: (project_id, newName, callback = ->)-> - logger.log project_id: project_id, newName:newName, "renaming project" - ProjectGetter.getProject project_id, {name:true}, (err, project)-> - if err? or !project? - logger.err err:err, project_id:project_id, "error getting project or could not find it todo project rename" - return callback(err) - oldProjectName = project.name - Project.update _id:project_id, {name: newName}, (err, project)=> - if err? + ProjectDetailsHandler.validateProjectName newName, (error) -> + return callback(error) if error? + logger.log project_id: project_id, newName:newName, "renaming project" + ProjectGetter.getProject project_id, {name:true}, (err, project)-> + if err? or !project? + logger.err err:err, project_id:project_id, "error getting project or could not find it todo project rename" return callback(err) - tpdsUpdateSender.moveEntity {project_id:project_id, project_name:oldProjectName, newProjectName:newName}, callback + oldProjectName = project.name + Project.update _id:project_id, {name: newName}, (err, project)=> + if err? + return callback(err) + tpdsUpdateSender.moveEntity {project_id:project_id, project_name:oldProjectName, newProjectName:newName}, callback + + MAX_PROJECT_NAME_LENGTH: 150 + validateProjectName: (name, callback = (error) ->) -> + if name.length == 0 + return callback(new Errors.InvalidNameError("Project name cannot be blank")) + else if name.length > @MAX_PROJECT_NAME_LENGTH + return callback(new Errors.InvalidNameError("Project name is too long")) + else if name.indexOf("/") > -1 + return callback(new Errors.InvalidNameError("Project name cannot not contain / characters")) + else + return callback() setPublicAccessLevel : (project_id, newAccessLevel, callback = ->)-> logger.log project_id: project_id, level: newAccessLevel, "set public access level" diff --git a/services/web/app/views/project/editor/left-menu.pug b/services/web/app/views/project/editor/left-menu.pug index 4e86b31620..831dc212ae 100644 --- a/services/web/app/views/project/editor/left-menu.pug +++ b/services/web/app/views/project/editor/left-menu.pug @@ -167,6 +167,8 @@ script(type='text/ng-template', id='cloneProjectModalTemplate') .modal-header h3 #{translate("copy_project")} .modal-body + .alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} + .alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")} form(name="cloneProjectForm", novalidate) .form-group label #{translate("new_name")} diff --git a/services/web/app/views/project/list/modals.pug b/services/web/app/views/project/list/modals.pug index 9111b2a347..01cd71bca4 100644 --- a/services/web/app/views/project/list/modals.pug +++ b/services/web/app/views/project/list/modals.pug @@ -98,6 +98,8 @@ script(type='text/ng-template', id='renameProjectModalTemplate') ) × h3 #{translate("rename_project")} .modal-body + .alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} + .alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")} form(name="renameProjectForm", novalidate) input.form-control( type="text", @@ -111,8 +113,10 @@ script(type='text/ng-template', id='renameProjectModalTemplate') button.btn.btn-default(ng-click="cancel()") #{translate("cancel")} button.btn.btn-primary( ng-click="rename()", - ng-disabled="renameProjectForm.$invalid" - ) #{translate("rename")} + ng-disabled="renameProjectForm.$invalid || state.inflight" + ) + span(ng-show="!state.inflight") #{translate("rename")} + span(ng-show="state.inflight") #{translate("renaming")}... script(type='text/ng-template', id='cloneProjectModalTemplate') .modal-header @@ -123,6 +127,8 @@ script(type='text/ng-template', id='cloneProjectModalTemplate') ) × h3 #{translate("copy_project")} .modal-body + .alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} + .alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")} form(name="cloneProjectForm", novalidate) .form-group label #{translate("new_name")} @@ -155,6 +161,8 @@ script(type='text/ng-template', id='newProjectModalTemplate') ) × h3 #{translate("new_project")} .modal-body + .alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} + .alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")} form(novalidate, name="newProjectForm") input.form-control( type="text", diff --git a/services/web/public/coffee/ide/clone/controllers/CloneProjectModalController.coffee b/services/web/public/coffee/ide/clone/controllers/CloneProjectModalController.coffee index 5c7cc55ea3..a08f95a918 100644 --- a/services/web/public/coffee/ide/clone/controllers/CloneProjectModalController.coffee +++ b/services/web/public/coffee/ide/clone/controllers/CloneProjectModalController.coffee @@ -6,6 +6,7 @@ define [ projectName: ide.$scope.project.name + " (Copy)" $scope.state = inflight: false + error: false $modalInstance.opened.then () -> $timeout () -> @@ -20,9 +21,16 @@ define [ $scope.clone = () -> $scope.state.inflight = true + $scope.state.error = false cloneProject($scope.inputs.projectName) - .then (data) -> + .success (data) -> window.location = "/project/#{data.data.project_id}" + .error (body, statusCode) -> + $scope.state.inflight = false + if statusCode == 400 + $scope.state.error = { message: body } + else + $scope.state.error = true $scope.cancel = () -> $modalInstance.dismiss('cancel') \ No newline at end of file diff --git a/services/web/public/coffee/ide/settings/controllers/ProjectNameController.coffee b/services/web/public/coffee/ide/settings/controllers/ProjectNameController.coffee index 1cb6644bed..2163bbc8cb 100644 --- a/services/web/public/coffee/ide/settings/controllers/ProjectNameController.coffee +++ b/services/web/public/coffee/ide/settings/controllers/ProjectNameController.coffee @@ -19,12 +19,18 @@ define [ $scope.finishRenaming = () -> $scope.state.renaming = false newName = $scope.inputs.name - if !newName? or newName.length == 0 or newName.length > MAX_PROJECT_NAME_LENGTH - return if $scope.project.name == newName return + oldName = $scope.project.name $scope.project.name = newName settings.saveProjectSettings({name: $scope.project.name}) + .error (response, statusCode) -> + $scope.project.name = oldName + if statusCode == 400 + ide.showGenericMessageModal("Error renaming project", response) + else + ide.showGenericMessageModal("Error renaming project", "Please try again in a moment") + console.log arguments ide.socket.on "projectNameUpdated", (name) -> $scope.$apply () -> diff --git a/services/web/public/coffee/ide/settings/services/settings.coffee b/services/web/public/coffee/ide/settings/services/settings.coffee index 04a9ccb5e3..f653359d17 100644 --- a/services/web/public/coffee/ide/settings/services/settings.coffee +++ b/services/web/public/coffee/ide/settings/services/settings.coffee @@ -12,8 +12,7 @@ define [ # End of tracking code. data._csrf = window.csrfToken - ide.$http.post "/user/settings", data - + return ide.$http.post "/user/settings", data saveProjectSettings: (data) -> # Tracking code. @@ -24,9 +23,8 @@ define [ # End of tracking code. data._csrf = window.csrfToken - ide.$http.post "/project/#{ide.project_id}/settings", data + return ide.$http.post "/project/#{ide.project_id}/settings", data - saveProjectAdminSettings: (data) -> # Tracking code. for key in Object.keys(data) @@ -36,8 +34,6 @@ define [ # End of tracking code. data._csrf = window.csrfToken - ide.$http.post "/project/#{ide.project_id}/settings/admin", data - - + return ide.$http.post "/project/#{ide.project_id}/settings/admin", data } ] \ No newline at end of file diff --git a/services/web/public/coffee/main/project-list/modal-controllers.coffee b/services/web/public/coffee/main/project-list/modal-controllers.coffee index dbf3c613ac..4e2285a4de 100644 --- a/services/web/public/coffee/main/project-list/modal-controllers.coffee +++ b/services/web/public/coffee/main/project-list/modal-controllers.coffee @@ -1,9 +1,13 @@ define [ "base" ], (App) -> - App.controller 'RenameProjectModalController', ($scope, $modalInstance, $timeout, projectName) -> + App.controller 'RenameProjectModalController', ($scope, $modalInstance, $timeout, project, queuedHttp) -> $scope.inputs = - projectName: projectName + projectName: project.name + + $scope.state = + inflight: false + error: false $modalInstance.opened.then () -> $timeout () -> @@ -11,7 +15,20 @@ define [ , 200 $scope.rename = () -> - $modalInstance.close($scope.inputs.projectName) + $scope.state.inflight = true + $scope.state.error = false + $scope + .renameProject(project, $scope.inputs.projectName) + .success () -> + $scope.state.inflight = false + $scope.state.error = false + $modalInstance.close() + .error (body, statusCode) -> + $scope.state.inflight = false + if statusCode == 400 + $scope.state.error = { message: body } + else + $scope.state.error = true $scope.cancel = () -> $modalInstance.dismiss('cancel') @@ -21,6 +38,7 @@ define [ projectName: project.name + " (Copy)" $scope.state = inflight: false + error: false $modalInstance.opened.then () -> $timeout () -> @@ -31,9 +49,16 @@ define [ $scope.state.inflight = true $scope .cloneProject(project, $scope.inputs.projectName) - .then (project_id) -> + .success () -> $scope.state.inflight = false - $modalInstance.close(project_id) + $scope.state.error = false + $modalInstance.close() + .error (body, statusCode) -> + $scope.state.inflight = false + if statusCode == 400 + $scope.state.error = { message: body } + else + $scope.state.error = true $scope.cancel = () -> $modalInstance.dismiss('cancel') @@ -43,6 +68,7 @@ define [ projectName: "" $scope.state = inflight: false + error: false $modalInstance.opened.then () -> $timeout () -> @@ -51,11 +77,19 @@ define [ $scope.create = () -> $scope.state.inflight = true + $scope.state.error = false $scope .createProject($scope.inputs.projectName, template) - .then (project_id) -> + .success (data) -> $scope.state.inflight = false - $modalInstance.close(project_id) + $scope.state.error = false + $modalInstance.close(data.project_id) + .error (body, statusCode) -> + $scope.state.inflight = false + if statusCode == 400 + $scope.state.error = { message: body } + else + $scope.state.error = true $scope.cancel = () -> $modalInstance.dismiss('cancel') diff --git a/services/web/public/coffee/main/project-list/project-list.coffee b/services/web/public/coffee/main/project-list/project-list.coffee index ad9d0f3582..1ce584fe8b 100644 --- a/services/web/public/coffee/main/project-list/project-list.coffee +++ b/services/web/public/coffee/main/project-list/project-list.coffee @@ -256,9 +256,7 @@ define [ ) $scope.createProject = (name, template = "none") -> - deferred = $q.defer() - - queuedHttp + return queuedHttp .post("/project/new", { _csrf: window.csrfToken projectName: name @@ -273,13 +271,7 @@ define [ # to the rest of the app } $scope.updateVisibleProjects() - deferred.resolve(data.project_id) ) - .error((data, status, headers, config) -> - deferred.reject() - ) - - return deferred.promise $scope.openCreateProjectModal = (template = "none") -> event_tracking.send 'project-list-page-interaction', 'new-project', template @@ -294,15 +286,13 @@ define [ modalInstance.result.then (project_id) -> window.location = "/project/#{project_id}" - MAX_PROJECT_NAME_LENGTH = 150 $scope.renameProject = (project, newName) -> - if !newName? or newName.length == 0 or newName.length > MAX_PROJECT_NAME_LENGTH - return - project.name = newName - queuedHttp.post "/project/#{project.id}/rename", { - newProjectName: project.name + return queuedHttp.post "/project/#{project.id}/rename", { + newProjectName: newName, _csrf: window.csrfToken } + .success () -> + project.name = newName $scope.openRenameProjectModal = () -> project = $scope.getFirstSelectedProject() @@ -312,16 +302,11 @@ define [ templateUrl: "renameProjectModalTemplate" controller: "RenameProjectModalController" resolve: - projectName: () -> project.name - ) - - modalInstance.result.then( - (newName) -> - $scope.renameProject(project, newName) + project: () -> project + scope: $scope ) $scope.cloneProject = (project, cloneName) -> - deferred = $q.defer() event_tracking.send 'project-list-page-interaction', 'project action', 'Clone' queuedHttp .post("/project/#{project.id}/clone", { @@ -337,13 +322,7 @@ define [ # to the rest of the app } $scope.updateVisibleProjects() - deferred.resolve(data.project_id) ) - .error((data, status, headers, config) -> - deferred.reject() - ) - - return deferred.promise $scope.openCloneProjectModal = () -> project = $scope.getFirstSelectedProject() diff --git a/services/web/public/coffee/services/queued-http.coffee b/services/web/public/coffee/services/queued-http.coffee index c4c6862fae..8bf208f500 100644 --- a/services/web/public/coffee/services/queued-http.coffee +++ b/services/web/public/coffee/services/queued-http.coffee @@ -19,24 +19,29 @@ define [ processPendingRequests() queuedHttp = (args...) -> - deferred = $q.defer() - promise = deferred.promise + # We can't use Angular's $q.defer promises, because it only passes + # a single argument on error, and $http passes multiple. + promise = {} + successCallbacks = [] + errorCallbacks = [] # Adhere to the $http promise conventions promise.success = (callback) -> - promise.then(callback) + successCallbacks.push callback return promise promise.error = (callback) -> - promise.catch(callback) + errorCallbacks.push callback return promise doRequest = () -> $http(args...) - .success (successArgs...) -> - deferred.resolve(successArgs...) - .error (errorArgs...) -> - deferred.reject(errorArgs...) + .success (args...) -> + for cb in successCallbacks + cb(args...) + .error (args...) -> + for cb in errorCallbacks + cb(args...) pendingRequests.push doRequest processPendingRequests() diff --git a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee index 87120deced..626ff71442 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee @@ -280,20 +280,12 @@ describe "ProjectController", -> done() @ProjectController.renameProject @req, @res - it "should send a 500 if there is a problem", (done)-> - @EditorController.renameProject.callsArgWith(2, "problem") - @res.sendStatus = (code)=> - code.should.equal 500 - @EditorController.renameProject.calledWith(@project_id, @newProjectName).should.equal true + it "should send an error to next() if there is a problem", (done)-> + @EditorController.renameProject.callsArgWith(2, error = new Error("problem")) + next = (e)=> + e.should.equal error done() - @ProjectController.renameProject @req, @res - - it "should return an error if the name is over 150 chars", (done)-> - @req.body.newProjectName = "EDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOTEDMUBEEBKBXUUUZERMNSXFFWIBHGSDAWGMRIQWJBXGWSBVWSIKLFPRBYSJEKMFHTRZBHVKJSRGKTBHMJRXPHORFHAKRNPZGGYIOT" - @res.sendStatus = (code)=> - code.should.equal 400 - done() - @ProjectController.renameProject @req, @res + @ProjectController.renameProject @req, @res, next describe "loadEditor", -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectCreationHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectCreationHandlerTests.coffee index f88d1700fd..8aa750b80a 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectCreationHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectCreationHandlerTests.coffee @@ -34,6 +34,8 @@ describe 'ProjectCreationHandler', -> addDoc: sinon.stub().callsArgWith(4, null, {_id: docId}) addFile: sinon.stub().callsArg(4) setRootDoc: sinon.stub().callsArg(2) + @ProjectDetailsHandler = + validateProjectName: sinon.stub().yields() @user = first_name:"first name here" @@ -48,6 +50,7 @@ describe 'ProjectCreationHandler', -> '../../models/Project':{Project:@ProjectModel} '../../models/Folder':{Folder:@FolderModel} './ProjectEntityHandler':@ProjectEntityHandler + "./ProjectDetailsHandler":@ProjectDetailsHandler "settings-sharelatex": @Settings = {} 'logger-sharelatex': {log:->} "metrics-sharelatex": { @@ -96,6 +99,18 @@ describe 'ProjectCreationHandler', -> it 'should return the error to the callback', -> should.exist @callback.args[0][0] + + describe "with an invalid name", -> + beforeEach -> + @ProjectDetailsHandler.validateProjectName = sinon.stub().yields(new Error("bad name")) + @handler.createBlankProject ownerId, projectName, @callback + + it 'should return the error to the callback', -> + should.exist @callback.args[0][0] + + it 'should not try to create the project', -> + @ProjectModel::save.called.should.equal false + describe 'Creating a basic project', -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee index 501e48f1a5..c4c3b3ef07 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee @@ -4,6 +4,7 @@ Errors = require "../../../../app/js/Features/Errors/Errors" SandboxedModule = require('sandboxed-module') sinon = require('sinon') assert = require("chai").assert +expect = require("chai").expect require('chai').should() describe 'ProjectDetailsHandler', -> @@ -95,6 +96,7 @@ describe 'ProjectDetailsHandler', -> describe "renameProject", -> beforeEach -> + @handler.validateProjectName = sinon.stub().yields() @ProjectModel.update.callsArgWith(2) @newName = "new name here" @@ -108,7 +110,34 @@ describe 'ProjectDetailsHandler', -> @handler.renameProject @project_id, @newName, => @tpdsUpdateSender.moveEntity.calledWith({project_id:@project_id, project_name:@project.name, newProjectName:@newName}).should.equal true done() + + it "should not do anything with an invalid name", (done) -> + @handler.validateProjectName = sinon.stub().yields(new Error("invalid name")) + @handler.renameProject @project_id, @newName, => + @tpdsUpdateSender.moveEntity.called.should.equal false + @ProjectModel.update.called.should.equal false + done() + describe "validateProjectName", -> + it "should reject empty names", (done) -> + @handler.validateProjectName "", (error) -> + expect(error).to.exist + done() + + it "should reject empty names with /s", (done) -> + @handler.validateProjectName "foo/bar", (error) -> + expect(error).to.exist + done() + + it "should reject long names", (done) -> + @handler.validateProjectName new Array(1000).join("a"), (error) -> + expect(error).to.exist + done() + + it "should accept normal names", (done) -> + @handler.validateProjectName "foobar", (error) -> + expect(error).to.not.exist + done() describe "setPublicAccessLevel", -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index f3dcda07cf..187d59d03d 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -80,6 +80,7 @@ describe 'ProjectEntityHandler', -> './ProjectUpdateHandler': @projectUpdater "./ProjectGetter": @ProjectGetter "settings-sharelatex":@settings + "../Cooldown/CooldownManager": @CooldownManager = {} describe 'mkdirp', -> From 3105c6743e3b581c5ef33aa44789d76f8301618b Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 22 May 2017 15:24:52 +0100 Subject: [PATCH 2/3] Fix unit tests --- .../test/UnitTests/coffee/Editor/EditorControllerTests.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee index 72285a1f44..0b7bfb1d2b 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee @@ -547,7 +547,7 @@ describe "EditorController", -> @err = "errro" @window_id = "kdsjklj290jlk" @newName = "new name here" - @ProjectDetailsHandler.renameProject = sinon.stub().callsArgWith(2, @err) + @ProjectDetailsHandler.renameProject = sinon.stub().callsArg(2) @EditorRealTimeController.emitToRoom = sinon.stub() it "should call the EditorController", (done)-> From 59085c1dddf8b60e1add82424f3e671e9f53ec0b Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 22 May 2017 15:33:52 +0100 Subject: [PATCH 3/3] Add missing require stubs and remove console.logs in unit tests --- .../web/app/coffee/Features/Project/ProjectUpdateHandler.coffee | 1 - .../web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee | 1 - .../UnitTests/coffee/Project/ProjectUpdateHandlerTests.coffee | 1 + services/web/test/UnitTests/coffee/User/UserLocatorTests.coffee | 1 + .../UnitTests/coffee/User/UserRegistrationHandlerTests.coffee | 1 - 5 files changed, 2 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectUpdateHandler.coffee index 7738425c8e..ba3b3b6295 100644 --- a/services/web/app/coffee/Features/Project/ProjectUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectUpdateHandler.coffee @@ -1,6 +1,5 @@ Project = require('../../models/Project').Project logger = require('logger-sharelatex') -Project = require("../../models/Project").Project module.exports = markAsUpdated : (project_id, callback)-> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee index 9257c2e83e..3d9808503b 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee @@ -70,7 +70,6 @@ describe 'ProjectLocator', -> it 'should give error if element could not be found', (done)-> @locator.findElement {project_id:project._id, element_id:"ddsd432nj42", type:"docs"}, (err, foundElement, path, parentFolder)-> - console.log err err.should.deep.equal new Errors.NotFoundError("entity not found") done() diff --git a/services/web/test/UnitTests/coffee/Project/ProjectUpdateHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectUpdateHandlerTests.coffee index 5922a027e3..a68a9be1f1 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectUpdateHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectUpdateHandlerTests.coffee @@ -11,6 +11,7 @@ describe 'ProjectUpdateHandler', -> @ProjectModel.update = sinon.stub().callsArg(3) @handler = SandboxedModule.require modulePath, requires: '../../models/Project':{Project:@ProjectModel} + 'logger-sharelatex' : { log: sinon.stub() } describe 'marking a project as recently updated', -> it 'should send an update to mongo', (done)-> diff --git a/services/web/test/UnitTests/coffee/User/UserLocatorTests.coffee b/services/web/test/UnitTests/coffee/User/UserLocatorTests.coffee index 73c178f934..dc3fc84dfa 100644 --- a/services/web/test/UnitTests/coffee/User/UserLocatorTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserLocatorTests.coffee @@ -11,6 +11,7 @@ describe "UserLocator", -> @UserLocator = SandboxedModule.require modulePath, requires: "../../infrastructure/mongojs": db: @db = { users: {} } "metrics-sharelatex": timeAsyncMethod: sinon.stub() + 'logger-sharelatex' : { log: sinon.stub() } @db.users = findOne : sinon.stub().callsArgWith(1, null, @user) diff --git a/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee b/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee index c59b481dc5..d0b96da2de 100644 --- a/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee @@ -96,7 +96,6 @@ describe "UserRegistrationHandler", -> it "should return email registered in the error if there is a non holdingAccount there", (done)-> @User.findOne.callsArgWith(1, null, @user = {holdingAccount:false}) @handler.registerNewUser @passingRequest, (err, user)=> - console.log err, user err.should.deep.equal new Error("EmailAlreadyRegistered") user.should.deep.equal @user done()