From 6a7395a287da30c6aa85c8a6a36a8c1bbf8f4b68 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Mon, 29 Feb 2016 17:34:38 +0000 Subject: [PATCH] brought back the project_or_id style Added functionality into project getter, its a big performance improvement for things like cloning projects. Clone a 500 element project, 1 mongo get or 500. --- .../Project/ProjectCreationHandler.coffee | 2 +- .../Features/Project/ProjectDuplicator.coffee | 7 +- .../Project/ProjectEntityHandler.coffee | 13 +- .../Features/Project/ProjectGetter.coffee | 59 +++++--- .../Project/ProjectDuplicatorTests.coffee | 2 +- .../coffee/Project/ProjectGetterTests.coffee | 143 ++++++++++++------ 6 files changed, 150 insertions(+), 76 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee b/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee index 7847fb19c0..a21dfc7f9e 100644 --- a/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee @@ -36,7 +36,7 @@ module.exports = return callback(error) if error? ProjectEntityHandler.addDoc project._id, project.rootFolder[0]._id, "main.tex", docLines, (error, doc)-> if error? - logger.err err:error, "error adding doc" + logger.err err:error, "error adding doc when creating basic project" return callback(error) ProjectEntityHandler.setRootDoc project._id, doc._id, (error) -> callback(error, project) diff --git a/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee b/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee index b43dcea77c..a554b3c829 100644 --- a/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee @@ -5,7 +5,6 @@ projectOptionsHandler = require('./ProjectOptionsHandler') DocumentUpdaterHandler = require("../DocumentUpdater/DocumentUpdaterHandler") DocstoreManager = require "../Docstore/DocstoreManager" ProjectGetter = require("./ProjectGetter") -Project = require("../../models/Project").Project _ = require('underscore') async = require('async') @@ -13,7 +12,7 @@ module.exports = duplicate: (owner, originalProjectId, newProjectName, callback)-> DocumentUpdaterHandler.flushProjectToMongo originalProjectId, (err) -> return callback(err) if err? - ProjectGetter.getProject originalProjectId, (err, originalProject) -> + ProjectGetter.getProject originalProjectId, {compiler:true, rootFolder:true, rootDoc_id:true}, (err, originalProject) -> return callback(err) if err? projectCreationHandler.createBlankProject owner._id, newProjectName, (err, newProject)-> return callback(err) if err? @@ -36,7 +35,7 @@ module.exports = return (callback)-> content = docContents[doc._id.toString()] return callback(new Error("doc_id not found: #{doc._id}")) if !content? - projectEntityHandler.addDoc newProject._id, newParentFolder._id, doc.name, content.lines, (err, newDoc)-> + projectEntityHandler.addDoc newProject, newParentFolder._id, doc.name, content.lines, (err, newDoc)-> if originalRootDoc? and newDoc.name == originalRootDoc.name setRootDoc newDoc._id callback() @@ -51,7 +50,7 @@ module.exports = copyFolder = (folder, desFolder, callback)-> jobs = folder.folders.map (childFolder)-> return (callback)-> - projectEntityHandler.addFolder newProject._id, desFolder._id, childFolder.name, (err, newFolder)-> + projectEntityHandler.addFolder newProject, desFolder._id, childFolder.name, (err, newFolder)-> copyFolder childFolder, newFolder, callback jobs.push (cb)-> copyDocs folder, desFolder, cb diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index e5bd3fe0fa..886559be81 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -109,8 +109,8 @@ module.exports = ProjectEntityHandler = options = {} DocstoreManager.getDoc project_id, doc_id, options, callback - addDoc: (project_id, folder_id, docName, docLines, callback = (error, doc, folder_id) ->)=> - ProjectGetter.getProjectWithOnlyFolders project_id, (err, project) -> + addDoc: (project_or_id, folder_id, docName, docLines, callback = (error, doc, folder_id) ->)=> + ProjectGetter.getProjectWithOnlyFolders project_or_id, (err, project) -> logger.log project: project._id, folder_id: folder_id, doc_name: docName, "adding doc" return callback(err) if err? confirmFolder project, folder_id, (folder_id)=> @@ -182,7 +182,7 @@ module.exports = ProjectEntityHandler = callback() copyFileFromExistingProject: (project_or_id, folder_id, originalProject_id, origonalFileRef, callback = (error, fileRef, folder_id) ->)-> - Project.getProject project_or_id, "name", (err, project) -> + ProjectGetter.getProjectWithOnlyFolders project_or_id, {name:true}, (err, project) -> logger.log project_id:project._id, folder_id:folder_id, originalProject_id:originalProject_id, origonalFileRef:origonalFileRef, "copying file in s3" return callback(err) if err? confirmFolder project, folder_id, (folder_id)=> @@ -216,7 +216,7 @@ module.exports = ProjectEntityHandler = if parentFolder? parentFolder_id = parentFolder._id builtUpPath = "#{builtUpPath}/#{folderName}" - projectLocator.findElementByPath project_id, builtUpPath, (err, foundFolder)=> + projectLocator.findElementByPath project, builtUpPath, (err, foundFolder)=> if !foundFolder? logger.log path:path, project_id:project._id, folderName:folderName, "making folder from mkdirp" @addFolder project_id, parentFolder_id, folderName, (err, newFolder, parentFolder_id)-> @@ -235,9 +235,9 @@ module.exports = ProjectEntityHandler = !folder.filterOut callback(null, folders, lastFolder) - addFolder: (project_id, parentFolder_id, folderName, callback) -> + addFolder: (project_or_id, parentFolder_id, folderName, callback) -> folder = new Folder name: folderName - ProjectGetter.getProjectWithOnlyFolders project_id, (err, project)=> + ProjectGetter.getProjectWithOnlyFolders project_or_id, (err, project)=> return callback(err) if err? confirmFolder project, parentFolder_id, (parentFolder_id)=> logger.log project: project_id, parentFolder_id:parentFolder_id, folderName:folderName, "new folder added" @@ -318,7 +318,6 @@ module.exports = ProjectEntityHandler = logger.err err: "No entityType set", project_id: project_id, entity_id: entity_id return callback("No entityType set") entityType = entityType.toLowerCase() - console.log "getting project" ProjectGetter.getProject project_id, {name:true, rootFolder:true}, (err, project)=> return callback(error) if error? projectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path)=> diff --git a/services/web/app/coffee/Features/Project/ProjectGetter.coffee b/services/web/app/coffee/Features/Project/ProjectGetter.coffee index 129ff831dc..38ce183a05 100644 --- a/services/web/app/coffee/Features/Project/ProjectGetter.coffee +++ b/services/web/app/coffee/Features/Project/ProjectGetter.coffee @@ -2,31 +2,54 @@ mongojs = require("../../infrastructure/mongojs") db = mongojs.db ObjectId = mongojs.ObjectId async = require "async" +Errors = require("../../errors") module.exports = ProjectGetter = EXCLUDE_DEPTH: 8 - getProjectWithoutDocLines: (project_id, callback=(error, project) ->) -> - excludes = {} - for i in [1..@EXCLUDE_DEPTH] - excludes["rootFolder#{Array(i).join(".folder")}.docs.lines"] = 0 - db.projects.find _id: ObjectId(project_id.toString()), excludes, (error, projects = []) -> - callback error, projects[0] - getProjectWithOnlyFolders: (project_id, callback=(error, project) ->) -> - excludes = {} - for i in [1..@EXCLUDE_DEPTH] - excludes["rootFolder#{Array(i).join(".folder")}.docs"] = 0 - excludes["rootFolder#{Array(i).join(".folder")}.fileRefs"] = 0 - db.projects.find _id: ObjectId(project_id.toString()), excludes, (error, projects = []) -> - callback error, projects[0] + _returnProjectIfPassed: (project_or_id, callback, continueCallback)-> + if project_or_id._id? + callback null, project_or_id + else + try + ObjectId(project_or_id.toString()) + catch e + return continueCallback(new Errors.NotFoundError(e.message)) + continueCallback() + + getProjectWithoutDocLines: (project_or_id, callback=(error, project) ->) -> + ProjectGetter._returnProjectIfPassed project_or_id, callback, (err)-> + return callback(err) if err? + project_id = project_or_id + excludes = {} + for i in [1..ProjectGetter.EXCLUDE_DEPTH] + excludes["rootFolder#{Array(i).join(".folder")}.docs.lines"] = 0 + db.projects.find _id: ObjectId(project_id.toString()), excludes, (error, projects = []) -> + callback error, projects[0] + + getProjectWithOnlyFolders: (project_or_id, callback=(error, project) ->) -> + ProjectGetter._returnProjectIfPassed project_or_id, callback, (err)-> + return callback(err) if err? + project_id = project_or_id + excludes = {} + for i in [1..ProjectGetter.EXCLUDE_DEPTH] + excludes["rootFolder#{Array(i).join(".folder")}.docs"] = 0 + excludes["rootFolder#{Array(i).join(".folder")}.fileRefs"] = 0 + db.projects.find _id: ObjectId(project_id.toString()), excludes, (error, projects = []) -> + callback error, projects[0] getProject: (query, projection, callback = (error, project) ->) -> - if typeof query == "string" - query = _id: ObjectId(query) - else if query instanceof ObjectId - query = _id: query - db.projects.findOne query, projection, callback + ProjectGetter._returnProjectIfPassed project_or_id, callback, (err)-> + if typeof query == "string" + query = _id: ObjectId(query) + else if query instanceof ObjectId + query = _id: query + db.projects.find query, projection, (err, project)-> + if err? + logger.err err:err, query:query, projection:projection, "error getting project" + return callback(err) + callback(null, project?[0]) populateProjectWithUsers: (project, callback=(error, project) ->) -> # eventually this should be in a UserGetter.getUser module diff --git a/services/web/test/UnitTests/coffee/Project/ProjectDuplicatorTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectDuplicatorTests.coffee index abc0b150fa..586b997fde 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectDuplicatorTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectDuplicatorTests.coffee @@ -76,7 +76,7 @@ describe 'ProjectDuplicator', -> findById: sinon.stub().callsArgWith(1, null, @project) @ProjectGetter = - getProject: sinon.stub().callsArgWith(1, null, @project) + getProject: sinon.stub().callsArgWith(2, null, @project) @duplicator = SandboxedModule.require modulePath, requires: '../../models/Project':{Project:@Project} diff --git a/services/web/test/UnitTests/coffee/Project/ProjectGetterTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectGetterTests.coffee index b8f1480be6..76923a5b3d 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectGetterTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectGetterTests.coffee @@ -20,63 +20,116 @@ describe "ProjectGetter", -> describe "getProjectWithoutDocLines", -> beforeEach -> @project = - _id: @project_id = "0123456789abcd9876543210" + _id: @project_id = "56d46b0a1d3422b87c5ebcb1" @db.projects.find = sinon.stub().callsArgWith(2, null, [@project]) - @ProjectGetter.getProjectWithoutDocLines @project_id, @callback - it "should call find with the project id", -> - @db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true + describe "passing an id", -> + beforeEach -> + @ProjectGetter.getProjectWithoutDocLines @project_id, @callback - it "should exclude the doc lines", -> - excludes = - "rootFolder.docs.lines": 0 - "rootFolder.folder.docs.lines": 0 - "rootFolder.folder.folder.docs.lines": 0 - "rootFolder.folder.folder.folder.docs.lines": 0 - "rootFolder.folder.folder.folder.folder.docs.lines": 0 - "rootFolder.folder.folder.folder.folder.folder.docs.lines": 0 - "rootFolder.folder.folder.folder.folder.folder.folder.docs.lines": 0 - "rootFolder.folder.folder.folder.folder.folder.folder.folder.docs.lines": 0 - @db.projects.find.calledWith(sinon.match.any, excludes) - .should.equal true + it "should call find with the project id", -> + @db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true + + it "should exclude the doc lines", -> + excludes = + "rootFolder.docs.lines": 0 + "rootFolder.folder.docs.lines": 0 + "rootFolder.folder.folder.docs.lines": 0 + "rootFolder.folder.folder.folder.docs.lines": 0 + "rootFolder.folder.folder.folder.folder.docs.lines": 0 + "rootFolder.folder.folder.folder.folder.folder.docs.lines": 0 + "rootFolder.folder.folder.folder.folder.folder.folder.docs.lines": 0 + "rootFolder.folder.folder.folder.folder.folder.folder.folder.docs.lines": 0 + @db.projects.find.calledWith(sinon.match.any, excludes) + .should.equal true + + it "should call the callback with the project", -> + @callback.calledWith(null, @project).should.equal true + + + describe "passing a project", -> + beforeEach -> + @ProjectGetter.getProjectWithoutDocLines @project, @callback + + it "should not call the db", -> + @db.projects.find.called.should.equal false + + it "should call the callback with the project", -> + @callback.calledWith(null, @project).should.equal true - it "should call the callback with the project", -> - @callback.calledWith(null, @project).should.equal true describe "getProjectWithOnlyFolders", -> - beforeEach -> + beforeEach ()-> @project = - _id: @project_id = "0123456789abcd9876543210" + _id: @project_id = "56d46b0a1d3422b87c5ebcb1" @db.projects.find = sinon.stub().callsArgWith(2, null, [@project]) - @ProjectGetter.getProjectWithOnlyFolders @project_id, @callback + + describe "passing an id", -> + beforeEach -> + @ProjectGetter.getProjectWithOnlyFolders @project_id, @callback - it "should call find with the project id", -> - @db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true + it "should call find with the project id", -> + @db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true - it "should exclude the docs and files lines", -> - excludes = - "rootFolder.docs": 0 - "rootFolder.fileRefs": 0 - "rootFolder.folder.docs": 0 - "rootFolder.folder.fileRefs": 0 - "rootFolder.folder.folder.docs": 0 - "rootFolder.folder.folder.fileRefs": 0 - "rootFolder.folder.folder.folder.docs": 0 - "rootFolder.folder.folder.folder.fileRefs": 0 - "rootFolder.folder.folder.folder.folder.docs": 0 - "rootFolder.folder.folder.folder.folder.fileRefs": 0 - "rootFolder.folder.folder.folder.folder.folder.docs": 0 - "rootFolder.folder.folder.folder.folder.folder.fileRefs": 0 - "rootFolder.folder.folder.folder.folder.folder.folder.docs": 0 - "rootFolder.folder.folder.folder.folder.folder.folder.fileRefs": 0 - "rootFolder.folder.folder.folder.folder.folder.folder.folder.docs": 0 - "rootFolder.folder.folder.folder.folder.folder.folder.folder.fileRefs": 0 - @db.projects.find.calledWith(sinon.match.any, excludes) - .should.equal true + it "should exclude the docs and files linesaaaa", -> + excludes = + "rootFolder.docs": 0 + "rootFolder.fileRefs": 0 + "rootFolder.folder.docs": 0 + "rootFolder.folder.fileRefs": 0 + "rootFolder.folder.folder.docs": 0 + "rootFolder.folder.folder.fileRefs": 0 + "rootFolder.folder.folder.folder.docs": 0 + "rootFolder.folder.folder.folder.fileRefs": 0 + "rootFolder.folder.folder.folder.folder.docs": 0 + "rootFolder.folder.folder.folder.folder.fileRefs": 0 + "rootFolder.folder.folder.folder.folder.folder.docs": 0 + "rootFolder.folder.folder.folder.folder.folder.fileRefs": 0 + "rootFolder.folder.folder.folder.folder.folder.folder.docs": 0 + "rootFolder.folder.folder.folder.folder.folder.folder.fileRefs": 0 + "rootFolder.folder.folder.folder.folder.folder.folder.folder.docs": 0 + "rootFolder.folder.folder.folder.folder.folder.folder.folder.fileRefs": 0 + @db.projects.find.calledWith(sinon.match.any, excludes).should.equal true - it "should call the callback with the project", -> - @callback.calledWith(null, @project).should.equal true + it "should call the callback with the project", -> + @callback.calledWith(null, @project).should.equal true + + describe "passing a project", -> + beforeEach -> + @ProjectGetter.getProjectWithoutDocLines @project, @callback + it "should not call the db", -> + @db.projects.find.called.should.equal false + + it "should call the callback with the project", -> + @callback.calledWith(null, @project).should.equal true + + + + describe "getProject", -> + beforeEach ()-> + @project = + _id: @project_id = "56d46b0a1d3422b87c5ebcb1" + @db.projects.find = sinon.stub().callsArgWith(2, null, [@project]) + + + it "should call find with the project id when string id is passed", (done)-> + @ProjectGetter.getProject @project_id, (err, project)=> + @db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true + assert.deepEqual @project, project + done() + + it "should call find with the project id when object id is passed", (done)-> + @ProjectGetter.getProject ObjectId(@project_id), (err, project)=> + @db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true + assert.deepEqual @project, project + done() + + it "should not call db when project is passed", (done)-> + @ProjectGetter.getProject ObjectId(@project_id), (err, project)=> + @db.projects.find.called.should.equal false + assert.deepEqual @project, project + done() describe "populateProjectWithUsers", -> beforeEach ->