Merge pull request #5 from sharelatex/phase2patch

read docs from doc collection first

services/docstore/app/coffee/DocManager.coffee

@@ -5,24 +5,19 @@ _ = require "underscore"
 async = require "async"
 
 module.exports = DocManager =
-	getDoc: (project_id, doc_id, options, callback = (error, doc, mongoPath) ->) ->
-		if typeof(options) == "function"
-			callback = options
-			options.include_deleted = false
+	getDoc: (project_id, doc_id, callback = (error, doc, mongoPath) ->) ->
 
-		MongoManager.findProject project_id, (error, project) ->
-			return callback(error) if error?
-			return callback new Errors.NotFoundError("No such project: #{project_id}") if !project?
-			DocManager.findDocInProject project, doc_id, (error, doc, mongoPath) ->
+		MongoManager.findDoc doc_id, (err, docFromDocCollection)->
+			return callback(err) if err?
+			MongoManager.findProject project_id, (error, project) ->
 				return callback(error) if error?
-				if doc?
-					return callback null, doc, mongoPath
-				else
-					if options.include_deleted
-						MongoManager.findDoc doc_id, (error, doc) ->
-							return callback(error) if error?
-							return callback new Errors.NotFoundError("No such doc: #{project_id}") if !doc?
-							return callback null, doc
+				return callback new Errors.NotFoundError("No such project: #{project_id}") if !project?
+				DocManager.findDocInProject project, doc_id, (error, doc, mongoPath) ->
+					return callback(error) if error?
+					if docFromDocCollection?
+						return callback null, docFromDocCollection, mongoPath
+					else if doc?
+						return callback null, doc, mongoPath
 					else
 						return callback new Errors.NotFoundError("No such doc: #{project_id}")
 
@@ -37,7 +32,7 @@ module.exports = DocManager =
 	updateDoc: (project_id, doc_id, lines, callback = (error, modified, rev) ->) ->
 		DocManager.getDoc project_id, doc_id, (error, doc, mongoPath) ->
 			return callback(error) if error?
-			return callback new Errors.NotFoundError("No such project/doc: #{project_id}/#{doc_id}") if !doc? or !mongoPath?
+			return callback new Errors.NotFoundError("No such project/doc to update: #{project_id}/#{doc_id}") if !doc? or !mongoPath?
 
 			if _.isEqual(doc.lines, lines)
 				logger.log {
@@ -65,7 +60,7 @@ module.exports = DocManager =
 	deleteDoc: (project_id, doc_id, callback = (error) ->) ->
 		DocManager.getDoc project_id, doc_id, (error, doc) ->
 			return callback(error) if error?
-			return callback new Errors.NotFoundError("No such project/doc: #{project_id}/#{doc_id}") if !doc?
+			return callback new Errors.NotFoundError("No such project/doc to delete: #{project_id}/#{doc_id}") if !doc?
 			MongoManager.upsertIntoDocCollection project_id, doc_id, doc.lines, doc.rev, (error) ->
 				return callback(error) if error?
 				MongoManager.markDocAsDeleted doc_id, (error) ->

services/docstore/app/coffee/HttpController.coffee

@@ -5,9 +5,8 @@ module.exports = HttpController =
 	getDoc: (req, res, next = (error) ->) ->
 		project_id = req.params.project_id
 		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, include_deleted: include_deleted, (error, doc) ->
+		DocManager.getDoc project_id, doc_id, (error, doc) ->
 			return next(error) if error?
 			logger.log doc: doc, "got doc"
 			if !doc?

services/docstore/test/acceptance/coffee/GettingDocsTests.coffee

@@ -34,19 +34,13 @@ describe "Getting a doc", ->
 
 	describe "when the doc is a deleted doc", ->
 		beforeEach (done) ->
-			@deleted_doc_id = ObjectId() 
+			@deleted_doc_id = ObjectId()
 			DocstoreClient.createDeletedDoc @project_id, @deleted_doc_id, @lines, done
 
-		describe "with include_deleted=true", ->
-			it "should return the doc", (done) ->
-				DocstoreClient.getDoc @project_id, @deleted_doc_id, include_deleted: true, (error, res, doc) =>
-					doc.lines.should.deep.equal @lines
-					doc.deleted.should.equal true
-					done()
-
-		describe "without include_deleted=true", ->
-			it "should return 404", (done) ->
-				DocstoreClient.getDoc @project_id, @deleted_doc_id, (error, res, doc) =>
-					res.statusCode.should.equal 404
-					done()
+		it "should return the doc", (done) ->
+			DocstoreClient.getDoc @project_id, @deleted_doc_id, (error, res, doc) =>
+				doc.lines.should.deep.equal @lines
+				doc.deleted.should.equal true
+				done()
 
+	

services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee

@@ -31,12 +31,9 @@ module.exports = DocstoreClient =
 	deleteProject: (project_id, callback = (error, res, body) ->) ->
 		db.projects.remove _id: project_id, callback
 
-	getDoc: (project_id, doc_id, options, callback = (error, res, body) ->) ->
-		if typeof(options) == "function"
-			callback = options
-			options = { include_deleted: false }
+	getDoc: (project_id, doc_id, callback = (error, res, body) ->) ->
 		request.get {
-			url: "http://localhost:3016/project/#{project_id}/doc/#{doc_id}?include_deleted=#{options.include_deleted}"
+			url: "http://localhost:3016/project/#{project_id}/doc/#{doc_id}"
 			json: true
 		}, callback
 

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

@@ -15,14 +15,38 @@ describe "DocManager", ->
 		@doc_id = ObjectId().toString()
 		@project_id = ObjectId().toString()
 		@callback = sinon.stub()
+		@stubbedError = new Error("blew up")
 
 	describe "getDoc", ->
+		beforeEach ->
+			@project = { name: "mock-project" }
+			@doc = { _id: @doc_id, lines: ["mock-lines"] }
+			@docCollectionDoc = { _id: @doc_id, lines: ["mock-lines"] }
+
+		describe "when the doc is in the doc collection not projects collection", ->
+
+			beforeEach ->
+				@MongoManager.findDoc = sinon.stub()
+				@MongoManager.findProject = sinon.stub().callsArgWith(1, null, @project)
+				@DocManager.findDocInProject = sinon.stub().callsArgWith(2, null, @doc, @mongoPath)
+
+			it "should get the doc from the doc collection when it is present there", (done)->
+				@MongoManager.findDoc.callsArgWith(1, null, @docCollectionDoc)
+				@DocManager.getDoc @project_id, @doc_id, (err, doc)=>
+					doc.should.equal @docCollectionDoc
+					done()
+
+			it "should return the error from find doc", (done)->
+				@MongoManager.findDoc.callsArgWith(1, @stubbedError)
+				@DocManager.getDoc @project_id, @doc_id, (err, doc)=>
+					err.should.equal @stubbedError
+					done()
+
 		describe "when the project exists and the doc is in it", ->
 			beforeEach -> 
-				@project = { name: "mock-project" }
-				@doc = { _id: @doc_id, lines: ["mock-lines"] }
 				@mongoPath = "mock.mongo.path"
 				@MongoManager.findProject = sinon.stub().callsArgWith(1, null, @project)
+				@MongoManager.findDoc = sinon.stub().callsArg(1)
 				@DocManager.findDocInProject = sinon.stub().callsArgWith(2, null, @doc, @mongoPath)
 				@DocManager.getDoc @project_id, @doc_id, @callback
 
@@ -42,6 +66,7 @@ describe "DocManager", ->
 		describe "when the project does not exist", ->
 			beforeEach -> 
 				@MongoManager.findProject = sinon.stub().callsArgWith(1, null, null)
+				@MongoManager.findDoc = sinon.stub().callsArg(1)
 				@DocManager.findDocInProject = sinon.stub()
 				@DocManager.getDoc @project_id, @doc_id, @callback
 
@@ -59,9 +84,7 @@ describe "DocManager", ->
 				@DocManager.findDocInProject = sinon.stub().callsArgWith(2, null, null, null)
 				@MongoManager.findDoc = sinon.stub().callsArgWith(1, null, @doc)
 
-			describe "when include_deleted = true", ->
-				beforeEach ->
-					@DocManager.getDoc @project_id, @doc_id, include_deleted: true, @callback
+				@DocManager.getDoc @project_id, @doc_id, @callback
 
 				it "should try to find the doc in the docs collection", ->
 					@MongoManager.findDoc
@@ -73,17 +96,7 @@ describe "DocManager", ->
 						.calledWith(null, @doc)
 						.should.equal true
 
-			describe "when include_deleted is not set", ->
-				beforeEach ->
-					@DocManager.getDoc @project_id, @doc_id, @callback
-
-				it "should not try to find the doc in the docs collection", ->
-					@MongoManager.findDoc.called.should.equal false
-
-				it "should return a NotFoundError", ->
-					@callback
-						.calledWith(new Errors.NotFoundError("No such doc: #{@doc_id}"))
-						.should.equal true
+		
 
 		describe "when the doc does not exist anywhere", ->
 			beforeEach -> 
@@ -124,7 +137,7 @@ describe "DocManager", ->
 			beforeEach -> 
 				@MongoManager.findProject = sinon.stub().callsArgWith(1, null, null)
 				@DocManager.findAllDocsInProject = sinon.stub()
-				@DocManager.getDoc @project_id, @doc_id, @callback
+				@DocManager.getAllDocs @project_id, @callback
 
 			it "should not try to find the doc in the project", ->
 				@DocManager.findAllDocsInProject.called.should.equal false

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

@@ -29,7 +29,7 @@ describe "HttpController", ->
 			@req.params =
 				project_id: @project_id
 				doc_id: @doc_id
-			@DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc)
+			@DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc)
 
 		describe "without deleted docs", ->
 			beforeEach ->
@@ -37,7 +37,7 @@ describe "HttpController", ->
 
 			it "should get the document (including deleted)", ->
 				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id, include_deleted: false)
+					.calledWith(@project_id, @doc_id)
 					.should.equal true
 
 			it "should return the doc as JSON", ->
@@ -52,13 +52,11 @@ describe "HttpController", ->
 
 		describe "with deleted docs", ->
 			beforeEach ->
-				@req.query =
-					include_deleted: 'true'
 				@HttpController.getDoc @req, @res, @next
 
 			it "should get the document (without deleted)", ->
 				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id, include_deleted: true)
+					.calledWith(@project_id, @doc_id)
 					.should.equal true
 
 	describe "getRawDoc", ->