made ProjectGetter.getProject more robust

it can deal with multiple types of query better, including mongoose ids which are not being matched like mongojs ids.

services/web/app/coffee/Features/Editor/EditorHttpController.coffee

@@ -67,6 +67,7 @@ module.exports = EditorHttpController =
 		project_id = req.params.Project_id
 		name = req.body.name
 		parent_folder_id = req.body.parent_folder_id
+		logger.log project_id:project_id, name:name, parent_folder_id:parent_folder_id, "getting request to add doc to project"
 		if !EditorHttpController._nameIsAcceptableLength(name)
 			return res.sendStatus 400
 		EditorController.addDoc project_id, parent_folder_id, name, [], "editor", (error, doc) ->

services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee

@@ -110,21 +110,22 @@ module.exports = ProjectEntityHandler =
 		DocstoreManager.getDoc project_id, doc_id, options, callback
 
 	addDoc: (project_or_id, folder_id, docName, docLines, callback = (error, doc, folder_id) ->)=>
-		ProjectGetter.getProjectWithOnlyFolders project_or_id, (err, project) ->
+		ProjectGetter.getProjectWithoutDocLines project_or_id, (err, project) ->
-			logger.log project: project._id, folder_id: folder_id, doc_name: docName, "adding doc"
 			return callback(err) if err?
+			logger.log project_id: project._id, folder_id: folder_id, doc_name: docName, "adding doc to project"
 			confirmFolder project, folder_id, (folder_id)=>
 				doc = new Doc name: docName
 				# Put doc in docstore first, so that if it errors, we don't have a doc_id in the project
 				# which hasn't been created in docstore.
 				DocstoreManager.updateDoc project._id.toString(), doc._id.toString(), docLines, (err, modified, rev) ->
 					return callback(err) if err? 
+
 					ProjectEntityHandler._putElement project._id, folder_id, doc, "doc", (err, result)=>
 						return callback(err) if err?
 						tpdsUpdateSender.addDoc {
-							project_id:   project._id,
+							project_id:   project?._id,
-							doc_id:		  doc._id	
+							doc_id:		  doc?._id	
-							path:         result.path.fileSystem,
+							path:         result?.path?.fileSystem,
 							project_name: project.name,
 							rev:          0
 						}, (err) ->
@@ -182,7 +183,7 @@ module.exports = ProjectEntityHandler =
 							callback()
 
 	copyFileFromExistingProject: (project_or_id, folder_id, originalProject_id, origonalFileRef, callback = (error, fileRef, folder_id) ->)->
-		ProjectGetter.getProjectWithOnlyFolders project_or_id, {name:true}, (err, project) ->
+		ProjectGetter.getProject 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)=>
@@ -194,7 +195,10 @@ module.exports = ProjectEntityHandler =
 					if err?
 						logger.err err:err, project_id:project._id, folder_id:folder_id, originalProject_id:originalProject_id, origonalFileRef:origonalFileRef, "error coping file in s3"
 					ProjectEntityHandler._putElement project._id, folder_id, fileRef, "file", (err, result)=>
-						tpdsUpdateSender.addFile {project_id:project._id, file_id:fileRef._id, path:result.path.fileSystem, rev:fileRef.rev, project_name:project.name}, (error) ->
+						if err?
+							logger.err err:err, project_id:project._id, folder_id:folder_id, "error putting element as part of copy"
+							return callback()
+						tpdsUpdateSender.addFile {project_id:project._id, file_id:fileRef._id, path:result?.path?.fileSystem, rev:fileRef.rev, project_name:project.name}, (error) ->
 							callback(error, fileRef, folder_id)
 
 	mkdirp: (project_id, path, callback = (err, newlyCreatedFolders, lastFolderInPath)->)->
@@ -240,7 +244,7 @@ module.exports = ProjectEntityHandler =
 		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"
+				logger.log project: project._id, parentFolder_id:parentFolder_id, folderName:folderName, "new folder added"
 				ProjectEntityHandler._putElement project._id, parentFolder_id, folder, "folder", (err, result)=>
 					if callback?
 						callback(err, folder, parentFolder_id)
@@ -447,7 +451,8 @@ module.exports = ProjectEntityHandler =
 
 		countFolder project.rootFolder[0], callback
 
-	_putElement: (project_id, folder_id, element, type, callback = (err, path)->)->
+	_putElement: (project_or_id, folder_id, element, type, callback = (err, path)->)->
+
 		sanitizeTypeOfElement = (elementType)->
 			lastChar = elementType.slice -1
 			if lastChar != "s"
@@ -461,32 +466,32 @@ module.exports = ProjectEntityHandler =
 			logger.err project_id:project_id, folder_id:folder_id, element:element, type:type, "failed trying to insert element as it was null"
 			return callback(e)
 		type = sanitizeTypeOfElement type
-		ProjectGetter.getProject project_id, {rootFolder:true}, (err, project)=>
+		ProjectGetter.getProject project_or_id, {rootFolder:true}, (err, project)=>
 			if err?
 				return callback(err)
 			if !folder_id?
 				folder_id = project.rootFolder[0]._id
 			ProjectEntityHandler._countElements project, (err, count)->
 				if count > settings.maxFilesPerProject
-					logger.warn project_id:project_id, "project too big, stopping insertions"
+					logger.warn project_id:project._id, "project too big, stopping insertions"
 					return callback("project_has_to_many_files")
 				projectLocator.findElement {project:project, element_id:folder_id, type:"folders"}, (err, folder, path)=>
+					if err?
+						logger.err err:err, project_id:project._id, folder_id:folder_id, type:type, element:element, "error finding folder for _putElement"
+						return callback(err)
 					newPath =
 						fileSystem: "#{path.fileSystem}/#{element.name}"
 						mongo: path.mongo
-					if err?
+					logger.log project_id: project._id, element_id: element._id, fileType: type, folder_id: folder_id, "adding element to project"
-						logger.err err:err, project_id:project_id, folder_id:folder_id, type:type, element:element, "error finding folder for _putElement"
-						return callback(err)
-					logger.log project_id: project_id, element_id: element._id, fileType: type, folder_id: folder_id, "adding element to project"
 					id = element._id+''
 					element._id = require('mongoose').Types.ObjectId(id)
-					conditions = _id:project_id
+					conditions = _id:project._id
 					mongopath = "#{path.mongo}.#{type}"
 					update = "$push":{}
 					update["$push"][mongopath] = element
 					Project.update conditions, update, {}, (err)->
 						if err?
-							logger.err err: err, project_id: project_id, 'error saving in putElement project'
+							logger.err err: err, project_id: project._id, 'error saving in putElement project'
 							return callback(err)
 						callback(err, {path:newPath})
 

services/web/app/coffee/Features/Project/ProjectGetter.coffee

@@ -3,6 +3,9 @@ db = mongojs.db
 ObjectId = mongojs.ObjectId
 async = require "async"
 Errors = require("../../errors")
+logger = require("logger-sharelatex")
+
+
 
 module.exports = ProjectGetter =
 	EXCLUDE_DEPTH: 8
@@ -39,12 +42,26 @@ module.exports = ProjectGetter =
 			db.projects.find _id: ObjectId(project_id.toString()), excludes, (error, projects = []) ->
 				callback error, projects[0]
 
-	getProject: (query, projection, callback = (error, project) ->) ->
+	getProject: (project_or_id, projection, callback = (error, project) ->) ->
+		if !project_or_id?
+			return callback("no id or project provided")
+
+		if typeof(projection) == "function"
+			callback = projection
+
 		ProjectGetter._returnProjectIfPassed project_or_id, callback, (err)->
-			if typeof query == "string"
+
-				query = _id: ObjectId(query)
+			if typeof project_or_id == "string"
-			else if query instanceof ObjectId
+				query = _id: ObjectId(project_or_id)
-				query = _id: query
+			else if project_or_id instanceof ObjectId
+				query = _id: project_or_id
+			else if project_or_id?.toString().length == 24 # sometimes mongoose ids are hard to identify, this will catch them
+				query = _id:  ObjectId(project_or_id.toString())
+			else
+				err = new Error("malformed get request")
+				logger.log project_or_id:project_or_id, err:err, type:typeof(project_or_id), "malformed get request"
+				return callback(err)
+
 			db.projects.find query, projection, (err, project)->
 				if err?
 					logger.err err:err, query:query, projection:projection, "error getting project"

services/web/app/coffee/Features/Spelling/SpellingController.coffee

@@ -12,5 +12,4 @@ module.exports = SpellingController =
 		request(url: Settings.apis.spelling.url + url, method: req.method, headers: req.headers, json: req.body, timeout:TEN_SECONDS)
 		.on "error", (error) ->
 			logger.error err: error, "Spelling API error"
-			res.sendStatus 500
 		.pipe(res)

services/web/test/UnitTests/coffee/Project/ProjectGetterTests.coffee

@@ -16,6 +16,9 @@ describe "ProjectGetter", ->
 					projects: {}
 					users: {}
 				ObjectId: ObjectId
+			"logger-sharelatex":
+				err:->
+				log:->
 
 	describe "getProjectWithoutDocLines", ->
 		beforeEach ->
@@ -106,30 +109,37 @@ describe "ProjectGetter", ->
 
 
 
-	describe "getProject", ->
+	describe "getProjectaaaaa", ->
 		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)->
+		it "should call find with the project id when string id is passed", (done)->
-				@ProjectGetter.getProject @project_id, (err, project)=>
+			@ProjectGetter.getProject @project_id, (err, project)=>
-					@db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true
+				@db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true
-					assert.deepEqual @project, project
+				assert.deepEqual @project, project
-					done()
+				done()
 
-			it "should call find with the project id when object id is passed", (done)->
+		it "should call find with the project id when object id is passed", (done)->
-				@ProjectGetter.getProject ObjectId(@project_id), (err, project)=>
+			@ProjectGetter.getProject ObjectId(@project_id), (err, project)=>
-					@db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true
+				@db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true
-					assert.deepEqual @project, project
+				assert.deepEqual @project, project
-					done()
+				done()
 
-			it "should not call db when project is passed", (done)->
+		it "should not call db when project is passed", (done)->
-				@ProjectGetter.getProject ObjectId(@project_id), (err, project)=>
+			@ProjectGetter.getProject @project, (err, project)=>
-					@db.projects.find.called.should.equal false
+				@db.projects.find.called.should.equal false
-					assert.deepEqual @project, project
+				assert.deepEqual @project, project
-					done()
+				done()
+
+		it "should call the db when a mongoose objectid is used", (done)->
+			mongooseID = require('mongoose').Types.ObjectId(@project_id)
+			@ProjectGetter.getProject mongooseID, (err, project)=>
+				@db.projects.find.calledWith(_id: ObjectId(@project_id)).should.equal true
+				assert.deepEqual @project, project
+				done()
 
 	describe "populateProjectWithUsers", ->
 		beforeEach ->