From 6b80d3563d4e65830a335e0f559bea6c0879a446 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 14 Sep 2018 11:08:03 +0100 Subject: [PATCH 1/2] add support for creating unique project names --- .../Project/ProjectDetailsHandler.coffee | 30 +++++++++++++ .../Project/ProjectDetailsHandlerTests.coffee | 42 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee index 9767d148b9..3525bcddbc 100644 --- a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee @@ -67,6 +67,36 @@ module.exports = ProjectDetailsHandler = else return callback() + _addSuffixToProjectName: (name, suffix = '') -> + # append the suffix and truncate the project title if needed + truncatedLength = ProjectDetailsHandler.MAX_PROJECT_NAME_LENGTH - suffix.length + return name.substr(0, truncatedLength) + suffix + + # FIXME: we should put a lock around this to make it completely safe, but we would need to do that at + # the point of project creation, rather than just checking the name at the start of the import. + # If we later move this check into ProjectCreationHandler we can ensure all new projects are created + # with a unique name. But that requires thinking through how we would handle incoming projects from + # dropbox for example. + ensureProjectNameIsUnique: (user_id, name, suffixes = [], callback = (error, name, changed)->) -> + ProjectGetter.findAllUsersProjects user_id, {name: 1}, (error, allUsersProjects) -> + return callback(error) if error? + {owned, readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly} = allUsersProjects + # create a set of all project names + allProjectNames = new Set() + for projectName in owned.concat(readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly) + allProjectNames.add(projectName) + isUnique = (x) -> !allProjectNames.has(x) + # check if the supplied name is already unique + if isUnique(name) + return callback(null, name, false) + # the name already exists, try adding the user-supplied suffixes to generate a unique name + for suffix in suffixes + candidateName = ProjectDetailsHandler._addSuffixToProjectName(name, suffix) + if isUnique(candidateName) + return callback(null, candidateName, true) + # we couldn't make the name unique, something is wrong + return callback new Errors.InvalidNameError("Project name could not be made unique") + setPublicAccessLevel : (project_id, newAccessLevel, callback = ->)-> logger.log project_id: project_id, level: newAccessLevel, "set public access level" # DEPRECATED: `READ_ONLY` and `READ_AND_WRITE` are still valid in, but should no longer diff --git a/services/web/test/unit/coffee/Project/ProjectDetailsHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectDetailsHandlerTests.coffee index 3b47c48420..7fadf0b12c 100644 --- a/services/web/test/unit/coffee/Project/ProjectDetailsHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectDetailsHandlerTests.coffee @@ -155,6 +155,48 @@ describe 'ProjectDetailsHandler', -> expect(error).to.not.exist done() + describe "ensureProjectNameIsUnique", -> + beforeEach -> + @result = { + owned: ["name", "name1", "name11"] + readAndWrite: ["name2", "name22"] + readOnly: ["name3", "name33"] + tokenReadAndWrite: ["name4", "name44"] + tokenReadOnly: ["name5", "name55", "x".repeat(15)] + } + @ProjectGetter.findAllUsersProjects = sinon.stub().callsArgWith(2, null, @result) + + it "should leave a unique name unchanged", (done) -> + @handler.ensureProjectNameIsUnique @user_id, "unique-name", ["-test-suffix"], (error, name, changed) -> + expect(name).to.equal "unique-name" + expect(changed).to.equal false + done() + + it "should append a suffix to an existing name", (done) -> + @handler.ensureProjectNameIsUnique @user_id, "name1", ["-test-suffix"], (error, name, changed) -> + expect(name).to.equal "name1-test-suffix" + expect(changed).to.equal true + done() + + it "should fallback to a second suffix when needed", (done) -> + @handler.ensureProjectNameIsUnique @user_id, "name1", ["1", "-test-suffix"], (error, name, changed) -> + expect(name).to.equal "name1-test-suffix" + expect(changed).to.equal true + done() + + it "should truncate the name when append a suffix if the result is too long", (done) -> + @handler.MAX_PROJECT_NAME_LENGTH = 20 + @handler.ensureProjectNameIsUnique @user_id, "x".repeat(15), ["-test-suffix"], (error, name, changed) -> + expect(name).to.equal "x".repeat(8) + "-test-suffix" + expect(changed).to.equal true + done() + + it "should return an error if the name cannot be made unique", (done) -> + @handler.ensureProjectNameIsUnique @user_id, "name", ["1", "5", "55"], (error, name, changed) -> + expect(error).to.eql new Errors.InvalidNameError("Project name could not be made unique") + done() + + describe "setPublicAccessLevel", -> beforeEach -> @ProjectModel.update.callsArgWith(2) From 8f8694ad94fa971607552d95a8eb855dc00b4cc0 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 27 Sep 2018 16:41:45 +0100 Subject: [PATCH 2/2] iterate over owned projects in a more robust way --- .../coffee/Features/Project/ProjectDetailsHandler.coffee | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee index 3525bcddbc..4946d8126e 100644 --- a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee @@ -78,12 +78,14 @@ module.exports = ProjectDetailsHandler = # with a unique name. But that requires thinking through how we would handle incoming projects from # dropbox for example. ensureProjectNameIsUnique: (user_id, name, suffixes = [], callback = (error, name, changed)->) -> - ProjectGetter.findAllUsersProjects user_id, {name: 1}, (error, allUsersProjects) -> + ProjectGetter.findAllUsersProjects user_id, {name: 1}, (error, allUsersProjectNames) -> return callback(error) if error? - {owned, readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly} = allUsersProjects + # allUsersProjectNames is returned as a hash {owned: [name1, name2, ...], readOnly: [....]} + # collect all of the names and flatten them into a single array + projectNameList = _.flatten(_.values(allUsersProjectNames)) # create a set of all project names allProjectNames = new Set() - for projectName in owned.concat(readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly) + for projectName in projectNameList allProjectNames.add(projectName) isUnique = (x) -> !allProjectNames.has(x) # check if the supplied name is already unique