Be explicit about the data we return from mongo

services/docstore/app/coffee/DocArchiveManager.coffee

@@ -11,7 +11,7 @@ thirtySeconds = 30 * 1000
 module.exports = DocArchive =
 
 	archiveAllDocs: (project_id, callback = (err, docs) ->) ->
-		MongoManager.getProjectsDocs project_id, {include_deleted: true}, (err, docs) ->
+		MongoManager.getProjectsDocs project_id, {include_deleted: true}, {lines: true, rev: true, inS3: true}, (err, docs) ->
 			if err?
 				return callback(err)
 			else if !docs?

services/docstore/app/coffee/DocManager.coffee

@@ -11,7 +11,7 @@ module.exports = DocManager =
 	# migrate this version property to be part of the docs collection, to guarantee
 	# consitency between lines and version when writing/reading, and for a simpler schema.
 	getDoc: (project_id, doc_id, filter = { version: false }, callback = (error, doc) ->) ->
-		MongoManager.findDoc project_id, doc_id, (err, doc)->
+		MongoManager.findDoc project_id, doc_id, filter, (err, doc)->
 			if err?
 				return callback(err)
 			else if !doc?
@@ -31,9 +31,9 @@ module.exports = DocManager =
 				else
 					callback err, doc
 
-	getAllNonDeletedDocs: (project_id, callback = (error, docs) ->) ->
+	getAllNonDeletedDocs: (project_id, filter, callback = (error, docs) ->) ->
 		DocArchive.unArchiveAllDocs project_id, (error) ->
-			MongoManager.getProjectsDocs project_id, {include_deleted: false}, (error, docs) ->
+			MongoManager.getProjectsDocs project_id, {include_deleted: false}, filter, (error, docs) ->
 				if err?
 					return callback(error)
 				else if !docs?
@@ -45,7 +45,7 @@ module.exports = DocManager =
 		if !lines? or !version?
 			return callback(new Error("no lines or version provided"))
 	
-		DocManager.getDoc project_id, doc_id, {version: true}, (err, doc)->
+		DocManager.getDoc project_id, doc_id, {version: true, rev: true, lines: true, version: true, ranges: true}, (err, doc)->
 			if err? and !(err instanceof Errors.NotFoundError)
 				logger.err project_id: project_id, doc_id: doc_id, err:err, "error getting document for update"
 				return callback(err)
@@ -93,7 +93,7 @@ module.exports = DocManager =
 			updateLinesAndRangesIfNeeded (error) ->
 				return callback(error) if error?
 				updateVersionIfNeeded (error) ->
-					return callback(callback) if error?
+					return callback(error) if error?
 					callback null, modified, rev
 
 	deleteDoc: (project_id, doc_id, callback = (error) ->) ->

services/docstore/app/coffee/HttpController.coffee

@@ -10,7 +10,7 @@ module.exports = HttpController =
 		doc_id     = req.params.doc_id
 		include_deleted = req.query?.include_deleted == "true"
 		logger.log project_id: project_id, doc_id: doc_id, "getting doc"
-		DocManager.getDoc project_id, doc_id, {version: true}, (error, doc) ->
+		DocManager.getDoc project_id, doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true}, (error, doc) ->
 			return next(error) if error?
 			logger.log doc: doc, "got doc"
 			if !doc?
@@ -24,7 +24,7 @@ module.exports = HttpController =
 		project_id = req.params.project_id
 		doc_id     = req.params.doc_id
 		logger.log project_id: project_id, doc_id: doc_id, "getting raw doc"
-		DocManager.getDoc project_id, doc_id, {version: false}, (error, doc) ->
+		DocManager.getDoc project_id, doc_id, {lines: true}, (error, doc) ->
 			return next(error) if error?
 			if !doc?
 				res.send 404
@@ -35,7 +35,7 @@ module.exports = HttpController =
 	getAllDocs: (req, res, next = (error) ->) ->
 		project_id = req.params.project_id
 		logger.log project_id: project_id, "getting all docs"
-		DocManager.getAllNonDeletedDocs project_id, (error, docs = []) ->
+		DocManager.getAllNonDeletedDocs project_id, {lines: true, rev: true}, (error, docs = []) ->
 			return next(error) if error?
 			docViews = []
 			for doc in docs
@@ -83,12 +83,13 @@ module.exports = HttpController =
 			_id:     doc._id?.toString()
 			lines:   doc.lines
 			rev:     doc.rev
-			deleted: !!doc.deleted
 		}
 		if doc.version?
 			doc_view.version = doc.version
 		if doc.ranges?
 			doc_view.ranges = doc.ranges
+		if doc.deleted?
+			doc_view.deleted = doc.deleted
 		return doc_view
 
 	_buildRawDocView: (doc)->

services/docstore/app/coffee/MongoManager.coffee

@@ -2,15 +2,15 @@
 
 module.exports = MongoManager =
 
-	findDoc: (project_id, doc_id, callback = (error, doc) ->) ->
-		db.docs.find {_id: ObjectId(doc_id.toString()), project_id: ObjectId(project_id.toString())}, {}, (error, docs = []) ->
+	findDoc: (project_id, doc_id, filter, callback = (error, doc) ->) ->
+		db.docs.find {_id: ObjectId(doc_id.toString()), project_id: ObjectId(project_id.toString())}, filter, (error, docs = []) ->
 			callback error, docs[0]
 
-	getProjectsDocs: (project_id, options = {include_deleted: true}, callback)->
+	getProjectsDocs: (project_id, options = {include_deleted: true}, filter, callback)->
 		query = {project_id: ObjectId(project_id.toString())}
 		if !options.include_deleted
 			query.deleted = { $ne: true }
-		db.docs.find query, {}, callback
+		db.docs.find query, filter, callback
 
 	getArchivedProjectDocs: (project_id, callback)->
 		query =

services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee

@@ -76,7 +76,7 @@ describe "Applying updates to a doc", ->
 			@body.modified.should.equal true
 
 		it "should return the rev", ->
-			@body.rev.should.equal 2
+			@body.rev.should.equal 1
 
 		it "should update the doc in the API", (done) ->
 			DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) =>

services/docstore/test/unit/coffee/DocArchiveManager.coffee

@@ -64,8 +64,8 @@ describe "DocArchiveManager", ->
 		@MongoManager =
 			markDocAsArchived: sinon.stub().callsArgWith(2, null)
 			upsertIntoDocCollection: sinon.stub().callsArgWith(3, null)
-			getProjectsDocs: sinon.stub().callsArgWith(2, null, @mongoDocs)
-			getArchivedProjectDocs: sinon.stub().callsArgWith(1, null, @mongoDocs)
+			getProjectsDocs: sinon.stub().callsArgWith(3, null, @mongoDocs)
+			getArchivedProjectDocs: sinon.stub().callsArgWith(2, null, @mongoDocs)
 
 		@requires = 
 			"settings-sharelatex": @settings
@@ -127,7 +127,7 @@ describe "DocArchiveManager", ->
 	describe "archiveAllDocs", ->
 
 		it "should archive all project docs which are not in s3", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, @mongoDocs)
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, @mongoDocs)
 			@DocArchiveManager.archiveDoc = sinon.stub().callsArgWith(2, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
@@ -142,14 +142,14 @@ describe "DocArchiveManager", ->
 				done()
 
 		it "should return error if have no docs", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, null)
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
 				should.exist err
 				done()
 
 		it "should return the error", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, @error, null)
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, @error, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
 				err.should.equal @error
@@ -163,7 +163,7 @@ describe "DocArchiveManager", ->
 				while --numberOfDocs != 0
 					@mongoDocs.push({inS3:true, _id: ObjectId()})
 
-				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, @mongoDocs)
+				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, @mongoDocs)
 				@DocArchiveManager.archiveDoc = sinon.stub().callsArgWith(2, null)	
 
 			it "should not throw and error", (done)->

services/docstore/test/unit/coffee/DocManagerTests.coffee

@@ -92,7 +92,7 @@ describe "DocManager", ->
 
 		describe "when the doc does not exist in the docs collection", ->
 			beforeEach -> 
-				@MongoManager.findDoc = sinon.stub().callsArgWith(2, null, null)
+				@MongoManager.findDoc = sinon.stub().yields(null, null)
 				@DocManager.getDoc @project_id, @doc_id, {version: true}, @callback
 
 			it "should return a NotFoundError", ->
@@ -104,13 +104,14 @@ describe "DocManager", ->
 		describe "when the project exists", ->
 			beforeEach -> 
 				@docs = [{ _id: @doc_id, project_id: @project_id, lines: ["mock-lines"] }]
-				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, @docs)
+				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, @docs)
 				@DocArchiveManager.unArchiveAllDocs = sinon.stub().callsArgWith(1, null, @docs)
-				@DocManager.getAllNonDeletedDocs @project_id, @callback
+				@filter = { lines: true }
+				@DocManager.getAllNonDeletedDocs @project_id, @filter, @callback
 
 			it "should get the project from the database", ->
 				@MongoManager.getProjectsDocs
-					.calledWith(@project_id, {include_deleted: false})
+					.calledWith(@project_id, {include_deleted: false}, @filter)
 					.should.equal true
 
 			it "should return the docs", ->
@@ -118,13 +119,13 @@ describe "DocManager", ->
 
 		describe "when there are no docs for the project", ->
 			beforeEach -> 
-				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, null)
-				@DocArchiveManager.unArchiveAllDocs = sinon.stub().callsArgWith(1, null, null)
-				@DocManager.getAllNonDeletedDocs @project_id, @callback
+				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, null)
+				@DocArchiveManager.unArchiveAllDocs = sinon.stub().callsArgWith(1, null)
+				@DocManager.getAllNonDeletedDocs @project_id, @filter, @callback
 
 			it "should return a NotFoundError", ->
 				@callback
-					.calledWith(new Errors.NotFoundError("No such docs for project #{@project_id}"))
+					.calledWith(new Errors.NotFoundError("No docs for project #{@project_id}"))
 					.should.equal true
 
 	describe "deleteDoc", ->
@@ -196,7 +197,7 @@ describe "DocManager", ->
 
 			it "should get the existing doc", ->
 				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id)
+					.calledWith(@project_id, @doc_id, {version: true, rev: true, lines: true, version: true, ranges: true})
 					.should.equal true
 
 			it "should upsert the document to the doc collection", ->
@@ -216,11 +217,6 @@ describe "DocManager", ->
 				@RangeManager.shouldUpdateRanges.returns true
 				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version, @newRanges, @callback
 
-			it "should get the existing doc", ->
-				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id)
-					.should.equal true
-
 			it "should upsert the ranges", ->
 				@MongoManager.upsertIntoDocCollection
 					.calledWith(@project_id, @doc_id, {ranges: @newRanges})
@@ -237,11 +233,6 @@ describe "DocManager", ->
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc)
 				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version + 1, @originalRanges, @callback
 
-			it "should get the existing doc with the version", ->
-				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id, {version: true})
-					.should.equal true
-
 			it "should not change the lines or ranges", ->
 				@MongoManager.upsertIntoDocCollection.called.should.equal false
 			
@@ -258,11 +249,6 @@ describe "DocManager", ->
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc)
 				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version, @originalRanges, @callback
 
-			it "should get the existing doc", ->
-				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id)
-					.should.equal true
-
 			it "should not update the ranges or lines", ->
 				@MongoManager.upsertIntoDocCollection.called.should.equal false
 			

services/docstore/test/unit/coffee/HttpControllerTests.coffee

@@ -45,7 +45,7 @@ describe "HttpController", ->
 
 			it "should get the document with the version (including deleted)", ->
 				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id, {version: true})
+					.calledWith(@project_id, @doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true})
 					.should.equal true
 
 			it "should return the doc as JSON", ->
@@ -54,7 +54,6 @@ describe "HttpController", ->
 						_id: @doc_id
 						lines: @doc.lines
 						rev: @doc.rev
-						deleted: false
 						version: @doc.version
 					})
 					.should.equal true
@@ -68,7 +67,7 @@ describe "HttpController", ->
 
 			it "should get the doc from the doc manager", ->
 				@HttpController.getDoc @req, @res, @next
-				@DocManager.getDoc.calledWith(@project_id, @doc_id, {version: true}).should.equal true
+				@DocManager.getDoc.calledWith(@project_id, @doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true}).should.equal true
 
 			it "should return 404 if the query string delete is not set ", ->
 				@HttpController.getDoc @req, @res, @next
@@ -97,7 +96,7 @@ describe "HttpController", ->
 
 		it "should get the document without the version", ->
 			@DocManager.getDoc
-				.calledWith(@project_id, @doc_id, {version: false})
+				.calledWith(@project_id, @doc_id, {lines: true})
 				.should.equal true
 
 		it "should set the content type header", ->
@@ -120,12 +119,12 @@ describe "HttpController", ->
 					lines: ["mock", "lines", "two"]
 					rev: 4
 				}]
-				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(1, null, @docs)
+				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(2, null, @docs)
 				@HttpController.getAllDocs @req, @res, @next
 
 			it "should get all the (non-deleted) docs", ->
 				@DocManager.getAllNonDeletedDocs
-					.calledWith(@project_id)
+					.calledWith(@project_id, {lines: true, rev: true})
 					.should.equal true
 
 			it "should return the doc as JSON", ->
@@ -134,12 +133,10 @@ describe "HttpController", ->
 						_id:   @docs[0]._id.toString()
 						lines: @docs[0].lines
 						rev:   @docs[0].rev
-						deleted: false
 					}, {
 						_id:   @docs[1]._id.toString()
 						lines: @docs[1].lines
 						rev:   @docs[1].rev
-						deleted: false
 					}])
 					.should.equal true
 
@@ -158,7 +155,7 @@ describe "HttpController", ->
 					lines: ["mock", "lines", "two"]
 					rev: 4
 				}]
-				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(1, null, @docs)
+				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(2, null, @docs)
 				@HttpController.getAllDocs @req, @res, @next
 
 			it "should return the non null docs as JSON", ->
@@ -167,12 +164,10 @@ describe "HttpController", ->
 						_id:   @docs[0]._id.toString()
 						lines: @docs[0].lines
 						rev:   @docs[0].rev
-						deleted: false
 					}, {
 						_id:   @docs[2]._id.toString()
 						lines: @docs[2].lines
 						rev:   @docs[2].rev
-						deleted: false
 					}])
 					.should.equal true
 

services/docstore/test/unit/coffee/MongoManagerTests.coffee

@@ -20,14 +20,15 @@ describe "MongoManager", ->
 		beforeEach ->
 			@doc = { name: "mock-doc"}
 			@db.docs.find = sinon.stub().callsArgWith(2, null, [@doc])
-			@MongoManager.findDoc @project_id, @doc_id, @callback
+			@filter = { lines: true }
+			@MongoManager.findDoc @project_id, @doc_id, @filter, @callback
 
 		it "should find the doc", ->
 			@db.docs.find
 				.calledWith({
 					_id: ObjectId(@doc_id)
 					project_id: ObjectId(@project_id)
-				}, {})
+				}, @filter)
 				.should.equal true
 
 		it "should call the callback with the doc", ->
@@ -35,6 +36,7 @@ describe "MongoManager", ->
 
 	describe "getProjectsDocs", ->
 		beforeEach ->
+			@filter = {lines: true}
 			@doc1 = { name: "mock-doc1" }
 			@doc2 = { name: "mock-doc2" }
 			@doc3 = { name: "mock-doc3" }
@@ -43,28 +45,28 @@ describe "MongoManager", ->
 		
 		describe "with included_deleted = false", ->
 			beforeEach -> 
-				@MongoManager.getProjectsDocs @project_id, include_deleted: false, @callback
+				@MongoManager.getProjectsDocs @project_id, include_deleted: false, @filter, @callback
 
 			it "should find the non-deleted docs via the project_id", ->
 				@db.docs.find
 					.calledWith({
 						project_id: ObjectId(@project_id)
 						deleted: { $ne: true }
-					}, {})
+					}, @filter)
 					.should.equal true
 
 			it "should call the callback with the docs", ->
 				@callback.calledWith(null, [@doc, @doc3, @doc4]).should.equal true
 				
 		describe "with included_deleted = true", ->
-			beforeEach -> 
-				@MongoManager.getProjectsDocs @project_id, include_deleted: true, @callback
+			beforeEach ->
+				@MongoManager.getProjectsDocs @project_id, include_deleted: true, @filter, @callback
 
 			it "should find all via the project_id", ->
 				@db.docs.find
 					.calledWith({
 						project_id: ObjectId(@project_id)
-					}, {})
+					}, @filter)
 					.should.equal true
 
 			it "should call the callback with the docs", ->