From f696ccb0d9b16cd85a20940b01b37f369a36de6d Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 10 Aug 2017 14:57:40 +0100
Subject: [PATCH] add unit tests for getProjectDocs

---
 .../app/coffee/ProjectManager.coffee          | 12 ++--
 .../HttpController/HttpControllerTests.coffee | 60 ++++++++++++++++++-
 2 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee
index 3ef16d03d4..c25f51277f 100644
--- a/services/document-updater/app/coffee/ProjectManager.coffee
+++ b/services/document-updater/app/coffee/ProjectManager.coffee
@@ -58,7 +58,7 @@ module.exports = ProjectManager =
 				else
 					callback(null)
 
-	getProjectDocs: (project_id, projectStateHash, excludeVersions = {}, _callback = (error) ->) ->
+	getProjectDocs: (project_id, projectStateHash, excludeVersions = {}, _callback = (error, docs) ->) ->
 		timer = new Metrics.Timer("projectManager.getProjectDocs")
 		callback = (args...) ->
 			timer.done()
@@ -78,12 +78,16 @@ module.exports = ProjectManager =
 						jobs.push (cb) ->
 							# check the doc version first
 							RedisManager.getDocVersion doc_id, (error, version) ->
-								return cb(error) if error?
+								if error?
+									logger.error err: error, project_id: project_id, doc_id: doc_id, "error getting project doc version"
+									return cb(error)
 								# skip getting the doc if we already have that version
-								return cb() if version is excludeVersions[doc_id]
+								return cb() if version? and version is excludeVersions[doc_id]
 								# otherwise get the doc lines from redis
 								RedisManager.getDocLines doc_id, (error, lines) ->
-									return cb(error) if error?
+									if error?
+										logger.error err: error, project_id: project_id, doc_id: doc_id, "error getting project doc lines"
+										return cb(error)
 									docs.push {_id: doc_id, lines: lines, v: version}
 									cb()
 				async.series jobs, (error) ->
diff --git a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee
index 617146e787..cadea20c04 100644
--- a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee
+++ b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee
@@ -12,7 +12,7 @@ describe "HttpController", ->
 			"./ProjectManager": @ProjectManager = {}
 			"logger-sharelatex" : @logger = { log: sinon.stub() }
 			"./Metrics": @Metrics = {}
-
+			"./Errors" : Errors
 		@Metrics.Timer = class Timer
 			done: sinon.stub()
 		@project_id = "project-id-123"
@@ -434,3 +434,61 @@ describe "HttpController", ->
 				@next
 					.calledWith(new Error("oops"))
 					.should.equal true
+
+	describe "getProjectDocs", ->
+		beforeEach ->
+			@state = "01234567890abcdef"
+			@docs = [{_id: "1234", lines: "hello", v: 23}, {_id: "4567", lines: "world", v: 45}]
+			@req =
+				params:
+					project_id: @project_id
+				query:
+					state: @state
+
+		describe "successfully", ->
+			beforeEach ->
+				@ProjectManager.getProjectDocs = sinon.stub().callsArgWith(3,null, @docs)
+				@HttpController.getProjectDocs(@req, @res, @next)
+
+			it "should get docs from the project manager", ->
+				@ProjectManager.getProjectDocs
+					.calledWith(@project_id, @state, {})
+					.should.equal true
+
+			it "should return a successful response", ->
+				@res.send
+					.calledWith(@docs)
+					.should.equal true
+
+			it "should log the request", ->
+				@logger.log
+					.calledWith({project_id: @project_id, exclude: []}, "getting docs via http")
+					.should.equal true
+
+			it "should log the response", ->
+				@logger.log
+					.calledWith({project_id: @project_id, result: ["1234:23", "4567:45"]}, "got docs via http")
+					.should.equal true
+
+			it "should time the request", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when there is a conflict", ->
+			beforeEach ->
+				@ProjectManager.getProjectDocs = sinon.stub().callsArgWith(3, new Errors.ProjectStateChangedError("project state changed"))
+				@HttpController.getProjectDocs(@req, @res, @next)
+
+			it "should return an HTTP 409 Conflict response", ->
+				@res.send
+					.calledWith(409)
+					.should.equal true
+
+		describe "when an error occurs", ->
+			beforeEach ->
+				@ProjectManager.getProjectDocs = sinon.stub().callsArgWith(3, new Error("oops"))
+				@HttpController.getProjectDocs(@req, @res, @next)
+
+			it "should call next with the error", ->
+				@next
+					.calledWith(new Error("oops"))
+					.should.equal true