From 010958509291be90ec33432ee629f3bf9f7308af Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Fri, 28 Jul 2017 16:34:32 +0100
Subject: [PATCH 01/14] provide endpoint for current project docs in redis

---
 services/document-updater/app.coffee          |  1 +
 .../app/coffee/HttpController.coffee          | 17 ++++++++++++
 .../app/coffee/ProjectManager.coffee          | 27 +++++++++++++++++++
 .../app/coffee/RedisManager.coffee            |  7 ++++-
 4 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee
index 49cfa1f1b8..288995a201 100644
--- a/services/document-updater/app.coffee
+++ b/services/document-updater/app.coffee
@@ -38,6 +38,7 @@ app.param 'doc_id', (req, res, next, doc_id) ->
 		next new Error("invalid doc id")
 
 app.get    '/project/:project_id/doc/:doc_id',                          HttpController.getDoc
+app.get    '/project/:project_id',                                      HttpController.getProjectDocs
 app.post   '/project/:project_id/doc/:doc_id',                          HttpController.setDoc
 app.post   '/project/:project_id/doc/:doc_id/flush',                    HttpController.flushDocIfLoaded
 app.delete '/project/:project_id/doc/:doc_id',                          HttpController.flushAndDeleteDoc
diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee
index 13e618e734..4dad5b7333 100644
--- a/services/document-updater/app/coffee/HttpController.coffee
+++ b/services/document-updater/app/coffee/HttpController.coffee
@@ -37,6 +37,23 @@ module.exports = HttpController =
 			size += (line.length + 1)
 		return size
 
+	getProjectDocs: (req, res, next = (error) ->) ->
+		project_id = req.params.project_id
+		# filter is string of existing docs "id:version,id:version,..."
+		filterItems = req.query?.filter?.split(',') or []
+		logger.log project_id: project_id, filter: filterItems, "getting docs via http"
+		timer = new Metrics.Timer("http.getAllDocs")
+		excludeVersions = {}
+		for item in filterItems
+			[id,version] = item?.split(':')
+			excludeVersions[id] = version
+		logger.log {project_id: project_id, excludeVersions: excludeVersions}, "excluding versions"
+		ProjectManager.getProjectDocs project_id, excludeVersions, (error, result) ->
+			timer.done()
+			return next(error) if error?
+			logger.log project_id: project_id, result: result, "got docs via http"
+			res.send result
+
 	setDoc: (req, res, next = (error) ->) ->
 		doc_id = req.params.doc_id
 		project_id = req.params.project_id
diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee
index cd4c66ae8d..e1d346204b 100644
--- a/services/document-updater/app/coffee/ProjectManager.coffee
+++ b/services/document-updater/app/coffee/ProjectManager.coffee
@@ -56,3 +56,30 @@ module.exports = ProjectManager =
 					callback new Error("Errors deleting docs. See log for details")
 				else
 					callback(null)
+
+	getProjectDocs: (project_id, excludeVersions = {}, _callback = (error) ->) ->
+		timer = new Metrics.Timer("projectManager.getProjectDocs")
+		callback = (args...) ->
+			timer.done()
+			_callback(args...)
+
+		RedisManager.getDocIdsInProject project_id, (error, doc_ids) ->
+			return callback(error) if error?
+			jobs = []
+			docs = []
+			for doc_id in doc_ids or []
+				do (doc_id) ->
+					jobs.push (cb) ->
+						# check the doc version first
+						RedisManager.getDocVersion doc_id, (error, version) ->
+							return cb(error) if error?
+							# skip getting the doc if we already have that version
+							return cb() if version is excludeVersions[doc_id]
+							# otherwise get the doc lines from redis
+							RedisManager.getDocLines doc_id, (error, lines) ->
+								return cb(error) if error?
+								docs.push {_id: doc_id, lines: lines, rev: version}
+								cb()
+			async.series jobs, (error) ->
+				return callback(error) if error?
+				callback(null, docs)
diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee
index be5018297d..5d3ae922b5 100644
--- a/services/document-updater/app/coffee/RedisManager.coffee
+++ b/services/document-updater/app/coffee/RedisManager.coffee
@@ -142,6 +142,11 @@ module.exports = RedisManager =
 			version = parseInt(version, 10)
 			callback null, version
 
+	getDocLines: (doc_id, callback = (error, version) ->) ->
+		rclient.get keys.docLines(doc_id: doc_id), (error, docLines) ->
+			return callback(error) if error?
+			callback null, docLines
+
 	getPreviousDocOps: (doc_id, start, end, callback = (error, jsonOps) ->) ->
 		timer = new metrics.Timer("redis.get-prev-docops")
 		rclient.llen keys.docOps(doc_id: doc_id), (error, length) ->
@@ -239,7 +244,7 @@ module.exports = RedisManager =
 
 	getDocIdsInProject: (project_id, callback = (error, doc_ids) ->) ->
 		rclient.smembers keys.docsInProject(project_id: project_id), callback
-	
+
 	_serializeRanges: (ranges, callback = (error, serializedRanges) ->) ->
 		jsonRanges = JSON.stringify(ranges)
 		if jsonRanges? and jsonRanges.length > MAX_RANGES_SIZE

From 54e02cd89533cc2739a526d56d831d0b1b73a2ee Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 3 Aug 2017 14:41:34 +0100
Subject: [PATCH 02/14] update docupdater endpoint to /project/id/docs

---
 services/document-updater/app.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee
index 288995a201..a580f8f113 100644
--- a/services/document-updater/app.coffee
+++ b/services/document-updater/app.coffee
@@ -38,7 +38,7 @@ app.param 'doc_id', (req, res, next, doc_id) ->
 		next new Error("invalid doc id")
 
 app.get    '/project/:project_id/doc/:doc_id',                          HttpController.getDoc
-app.get    '/project/:project_id',                                      HttpController.getProjectDocs
+app.get    '/project/:project_id/doc',                                  HttpController.getProjectDocs
 app.post   '/project/:project_id/doc/:doc_id',                          HttpController.setDoc
 app.post   '/project/:project_id/doc/:doc_id/flush',                    HttpController.flushDocIfLoaded
 app.delete '/project/:project_id/doc/:doc_id',                          HttpController.flushAndDeleteDoc

From 80be5adc0d96498a9a7e6040b38dd0f6ca6fddd2 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 3 Aug 2017 14:42:08 +0100
Subject: [PATCH 03/14] change filter to exclude in getProjectDocs

---
 .../document-updater/app/coffee/HttpController.coffee     | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee
index 4dad5b7333..20eaa2c6f3 100644
--- a/services/document-updater/app/coffee/HttpController.coffee
+++ b/services/document-updater/app/coffee/HttpController.coffee
@@ -39,12 +39,12 @@ module.exports = HttpController =
 
 	getProjectDocs: (req, res, next = (error) ->) ->
 		project_id = req.params.project_id
-		# filter is string of existing docs "id:version,id:version,..."
-		filterItems = req.query?.filter?.split(',') or []
-		logger.log project_id: project_id, filter: filterItems, "getting docs via http"
+		# exclude is string of existing docs "id:version,id:version,..."
+		excludeItems = req.query?.exclude?.split(',') or []
+		logger.log project_id: project_id, exclude: excludeItems, "getting docs via http"
 		timer = new Metrics.Timer("http.getAllDocs")
 		excludeVersions = {}
-		for item in filterItems
+		for item in excludeItems
 			[id,version] = item?.split(':')
 			excludeVersions[id] = version
 		logger.log {project_id: project_id, excludeVersions: excludeVersions}, "excluding versions"

From 860537146b35a1947b780205d3c67c17645f28b7 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 3 Aug 2017 14:42:21 +0100
Subject: [PATCH 04/14] avoid logging doclines in getProjectDocs

---
 services/document-updater/app/coffee/HttpController.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee
index 20eaa2c6f3..73d5d24b23 100644
--- a/services/document-updater/app/coffee/HttpController.coffee
+++ b/services/document-updater/app/coffee/HttpController.coffee
@@ -51,7 +51,7 @@ module.exports = HttpController =
 		ProjectManager.getProjectDocs project_id, excludeVersions, (error, result) ->
 			timer.done()
 			return next(error) if error?
-			logger.log project_id: project_id, result: result, "got docs via http"
+			logger.log project_id: project_id, result: ("#{doc._id}:#{doc.rev}" for doc in result), "got docs via http"
 			res.send result
 
 	setDoc: (req, res, next = (error) ->) ->

From f5f516a910d03d1c069f1fe5736b0e35a20c73dd Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 3 Aug 2017 15:03:30 +0100
Subject: [PATCH 05/14] delete clsi state when deleting doc (if used)

---
 services/document-updater/app/coffee/RedisManager.coffee | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee
index 5d3ae922b5..472098d91f 100644
--- a/services/document-updater/app/coffee/RedisManager.coffee
+++ b/services/document-updater/app/coffee/RedisManager.coffee
@@ -88,7 +88,11 @@ module.exports = RedisManager =
 		multi.del keys.ranges(doc_id:doc_id)
 		multi.exec (error) ->
 			return callback(error) if error?
-			rclient.srem keys.docsInProject(project_id:project_id), doc_id, callback
+			multi = rclient.multi()
+			multi.srem keys.docsInProject(project_id:project_id), doc_id
+			if keys.clsiState?
+				multi.del keys.clsiState(project_id:project_id)
+			multi.exec callback
 
 	getDoc : (project_id, doc_id, callback = (error, lines, version, ranges) ->)->
 		timer = new metrics.Timer("redis.get-doc")

From 9f3ec72f8126fb8db2b608f218e986b2bf8f4e16 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Mon, 7 Aug 2017 14:43:28 +0100
Subject: [PATCH 06/14] switch to single get/set method for getProjectDocs

if project state hasn't changed, return the docs. Otherwise set the hash
and return a 409 Conflict response.
---
 .../document-updater/app/coffee/Errors.coffee |  9 +++-
 .../app/coffee/HttpController.coffee          | 15 ++++---
 .../app/coffee/ProjectManager.coffee          | 44 +++++++++++--------
 .../app/coffee/RedisManager.coffee            | 16 ++++++-
 4 files changed, 57 insertions(+), 27 deletions(-)

diff --git a/services/document-updater/app/coffee/Errors.coffee b/services/document-updater/app/coffee/Errors.coffee
index 941bfcc9f1..e5e93fa458 100644
--- a/services/document-updater/app/coffee/Errors.coffee
+++ b/services/document-updater/app/coffee/Errors.coffee
@@ -12,7 +12,14 @@ OpRangeNotAvailableError = (message) ->
 	return error
 OpRangeNotAvailableError.prototype.__proto__ = Error.prototype
 
+ProjectStateChangedError = (message) ->
+	error = new Error(message)
+	error.name = "ProjectStateChangedError"
+	error.__proto__ = ProjectStateChangedError.prototype
+	return error
+ProjectStateChangedError.prototype.__proto__ = Error.prototype
+
 module.exports = Errors =
 	NotFoundError: NotFoundError
 	OpRangeNotAvailableError: OpRangeNotAvailableError
-
+	ProjectStateChangedError: ProjectStateChangedError
diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee
index 73d5d24b23..8da4b68c2d 100644
--- a/services/document-updater/app/coffee/HttpController.coffee
+++ b/services/document-updater/app/coffee/HttpController.coffee
@@ -39,6 +39,7 @@ module.exports = HttpController =
 
 	getProjectDocs: (req, res, next = (error) ->) ->
 		project_id = req.params.project_id
+		projectStateHash = req.query?.state
 		# exclude is string of existing docs "id:version,id:version,..."
 		excludeItems = req.query?.exclude?.split(',') or []
 		logger.log project_id: project_id, exclude: excludeItems, "getting docs via http"
@@ -47,12 +48,16 @@ module.exports = HttpController =
 		for item in excludeItems
 			[id,version] = item?.split(':')
 			excludeVersions[id] = version
-		logger.log {project_id: project_id, excludeVersions: excludeVersions}, "excluding versions"
-		ProjectManager.getProjectDocs project_id, excludeVersions, (error, result) ->
+		logger.log {project_id: project_id, projectStateHash: projectStateHash, excludeVersions: excludeVersions}, "excluding versions"
+		ProjectManager.getProjectDocs project_id, projectStateHash, excludeVersions, (error, result) ->
 			timer.done()
-			return next(error) if error?
-			logger.log project_id: project_id, result: ("#{doc._id}:#{doc.rev}" for doc in result), "got docs via http"
-			res.send result
+			if error instanceof Errors.ProjectStateChangedError
+				res.send 409 # conflict
+			else if error?
+				return next(error)
+			else
+				logger.log project_id: project_id, result: ("#{doc._id}:#{doc.rev}" for doc in result), "got docs via http"
+				res.send result
 
 	setDoc: (req, res, next = (error) ->) ->
 		doc_id = req.params.doc_id
diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee
index e1d346204b..80f9b84a25 100644
--- a/services/document-updater/app/coffee/ProjectManager.coffee
+++ b/services/document-updater/app/coffee/ProjectManager.coffee
@@ -3,6 +3,7 @@ DocumentManager = require "./DocumentManager"
 async = require "async"
 logger = require "logger-sharelatex"
 Metrics = require "./Metrics"
+Errors = require "./Errors"
 
 module.exports = ProjectManager =
 	flushProjectWithLocks: (project_id, _callback = (error) ->) ->
@@ -57,29 +58,34 @@ module.exports = ProjectManager =
 				else
 					callback(null)
 
-	getProjectDocs: (project_id, excludeVersions = {}, _callback = (error) ->) ->
+	getProjectDocs: (project_id, projectStateHash, excludeVersions = {}, _callback = (error) ->) ->
 		timer = new Metrics.Timer("projectManager.getProjectDocs")
 		callback = (args...) ->
 			timer.done()
 			_callback(args...)
 
-		RedisManager.getDocIdsInProject project_id, (error, doc_ids) ->
+		RedisManager.checkOrSetProjectState project_id, projectStateHash, (error, projectStateChanged) ->
 			return callback(error) if error?
-			jobs = []
-			docs = []
-			for doc_id in doc_ids or []
-				do (doc_id) ->
-					jobs.push (cb) ->
-						# check the doc version first
-						RedisManager.getDocVersion doc_id, (error, version) ->
-							return cb(error) if error?
-							# skip getting the doc if we already have that version
-							return cb() if version is excludeVersions[doc_id]
-							# otherwise get the doc lines from redis
-							RedisManager.getDocLines doc_id, (error, lines) ->
-								return cb(error) if error?
-								docs.push {_id: doc_id, lines: lines, rev: version}
-								cb()
-			async.series jobs, (error) ->
+			# we can't return docs if project structure has changed
+			return callback Errors.ProjectStateChangedError("project state changed") if projectStateChanged
+			# project structure hasn't changed, return doc content from redis
+			RedisManager.getDocIdsInProject project_id, (error, doc_ids) ->
 				return callback(error) if error?
-				callback(null, docs)
+				jobs = []
+				docs = []
+				for doc_id in doc_ids or []
+					do (doc_id) ->
+						jobs.push (cb) ->
+							# check the doc version first
+							RedisManager.getDocVersion doc_id, (error, version) ->
+								return cb(error) if error?
+								# skip getting the doc if we already have that version
+								return cb() if version is excludeVersions[doc_id]
+								# otherwise get the doc lines from redis
+								RedisManager.getDocLines doc_id, (error, lines) ->
+									return cb(error) if error?
+									docs.push {_id: doc_id, lines: lines, rev: version}
+									cb()
+				async.series jobs, (error) ->
+					return callback(error) if error?
+					callback(null, docs)
diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee
index 472098d91f..b3e6132e4e 100644
--- a/services/document-updater/app/coffee/RedisManager.coffee
+++ b/services/document-updater/app/coffee/RedisManager.coffee
@@ -90,10 +90,22 @@ module.exports = RedisManager =
 			return callback(error) if error?
 			multi = rclient.multi()
 			multi.srem keys.docsInProject(project_id:project_id), doc_id
-			if keys.clsiState?
-				multi.del keys.clsiState(project_id:project_id)
+			if keys.projectState?
+				multi.del keys.projectState(project_id:project_id)
 			multi.exec callback
 
+	checkOrSetProjectState: (project_id, newState, callback = (error, stateChanged) ->) ->
+		if keys.projectState?
+			multi = rclient.multi()
+			multi.getset keys.projectState(project_id:project_id), newState
+			multi.expire keys.projectState(project_id:project_id), 30 * minutes
+			multi.exec (error, response) ->
+				return callback(error) if error?
+				logger.log project_id: project_id, newState:newState, oldState: response[0], "checking project state"
+				callback(null, response[0] isnt newState)
+		else
+			callback(null,true)
+
 	getDoc : (project_id, doc_id, callback = (error, lines, version, ranges) ->)->
 		timer = new metrics.Timer("redis.get-doc")
 		multi = rclient.multi()

From ca7d9dce85d92a5a3de5748995557d9218e5a98d Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Tue, 8 Aug 2017 09:40:39 +0100
Subject: [PATCH 07/14] fix unit tests

---
 .../test/unit/coffee/RedisManager/RedisManagerTests.coffee      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee
index d0521fa87e..0fe4470d32 100644
--- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee
+++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee
@@ -551,7 +551,7 @@ describe "RedisManager", ->
 	describe "removeDocFromMemory", ->
 		beforeEach (done) ->
 			@rclient.del = sinon.stub()
-			@rclient.srem = sinon.stub().yields()
+			@rclient.srem = sinon.stub()
 			@rclient.exec.yields()
 			@RedisManager.removeDocFromMemory @project_id, @doc_id, done
 		

From bd6b5b2c596dcab95b5f004f7b71648178040952 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Wed, 9 Aug 2017 15:29:58 +0100
Subject: [PATCH 08/14] use v instead of rev in getProjectDocs

---
 services/document-updater/app/coffee/ProjectManager.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee
index 80f9b84a25..3ef16d03d4 100644
--- a/services/document-updater/app/coffee/ProjectManager.coffee
+++ b/services/document-updater/app/coffee/ProjectManager.coffee
@@ -84,7 +84,7 @@ module.exports = ProjectManager =
 								# otherwise get the doc lines from redis
 								RedisManager.getDocLines doc_id, (error, lines) ->
 									return cb(error) if error?
-									docs.push {_id: doc_id, lines: lines, rev: version}
+									docs.push {_id: doc_id, lines: lines, v: version}
 									cb()
 				async.series jobs, (error) ->
 					return callback(error) if error?

From 47bebf963861947eef0e861fd16688072261b591 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Wed, 9 Aug 2017 16:45:08 +0100
Subject: [PATCH 09/14] no need to support optional keys.projectState

---
 .../app/coffee/RedisManager.coffee            | 20 ++++++++-----------
 .../config/settings.defaults.coffee           |  2 ++
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee
index b3e6132e4e..718eb1f5b8 100644
--- a/services/document-updater/app/coffee/RedisManager.coffee
+++ b/services/document-updater/app/coffee/RedisManager.coffee
@@ -90,21 +90,17 @@ module.exports = RedisManager =
 			return callback(error) if error?
 			multi = rclient.multi()
 			multi.srem keys.docsInProject(project_id:project_id), doc_id
-			if keys.projectState?
-				multi.del keys.projectState(project_id:project_id)
+			multi.del keys.projectState(project_id:project_id)
 			multi.exec callback
 
 	checkOrSetProjectState: (project_id, newState, callback = (error, stateChanged) ->) ->
-		if keys.projectState?
-			multi = rclient.multi()
-			multi.getset keys.projectState(project_id:project_id), newState
-			multi.expire keys.projectState(project_id:project_id), 30 * minutes
-			multi.exec (error, response) ->
-				return callback(error) if error?
-				logger.log project_id: project_id, newState:newState, oldState: response[0], "checking project state"
-				callback(null, response[0] isnt newState)
-		else
-			callback(null,true)
+		multi = rclient.multi()
+		multi.getset keys.projectState(project_id:project_id), newState
+		multi.expire keys.projectState(project_id:project_id), 30 * minutes
+		multi.exec (error, response) ->
+			return callback(error) if error?
+			logger.log project_id: project_id, newState:newState, oldState: response[0], "checking project state"
+			callback(null, response[0] isnt newState)
 
 	getDoc : (project_id, doc_id, callback = (error, lines, version, ranges) ->)->
 		timer = new metrics.Timer("redis.get-doc")
diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee
index b06b9d8bf9..838ffa19a8 100755
--- a/services/document-updater/config/settings.defaults.coffee
+++ b/services/document-updater/config/settings.defaults.coffee
@@ -41,6 +41,7 @@ module.exports =
 				projectKey: ({doc_id}) -> "ProjectId:#{doc_id}"
 				docsInProject: ({project_id}) -> "DocsIn:#{project_id}"
 				ranges: ({doc_id}) -> "Ranges:#{doc_id}"
+				projectState: ({project_id}) -> "ProjectState:#{project_id}"
 			# cluster: [{
 			# 	port: "7000"
 			# 	host: "localhost"
@@ -54,6 +55,7 @@ module.exports =
 			# 	projectKey: ({doc_id}) -> "ProjectId:{#{doc_id}}"
 			# 	docsInProject: ({project_id}) -> "DocsIn:{#{project_id}}"
 			# 	ranges: ({doc_id}) -> "Ranges:{#{doc_id}}"
+			# 	projectState: ({project_id}) -> "ProjectState:{#{project_id}}"
 		history:
 			port:"6379"
 			host:"localhost"

From 1eb80936f2e34492db2a2424bcd3940aea4be1c2 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 10 Aug 2017 14:57:27 +0100
Subject: [PATCH 10/14] fix log line

---
 services/document-updater/app/coffee/HttpController.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee
index 8da4b68c2d..369b429ab1 100644
--- a/services/document-updater/app/coffee/HttpController.coffee
+++ b/services/document-updater/app/coffee/HttpController.coffee
@@ -56,7 +56,7 @@ module.exports = HttpController =
 			else if error?
 				return next(error)
 			else
-				logger.log project_id: project_id, result: ("#{doc._id}:#{doc.rev}" for doc in result), "got docs via http"
+				logger.log project_id: project_id, result: ("#{doc._id}:#{doc.v}" for doc in result), "got docs via http"
 				res.send result
 
 	setDoc: (req, res, next = (error) ->) ->

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 11/14] 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

From f6be68eb080436465c25165ff0987203288bf760 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Fri, 11 Aug 2017 16:49:16 +0100
Subject: [PATCH 12/14] added missing unit tests file

---
 .../ProjectManager/getProjectDocsTests.coffee | 101 ++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 services/document-updater/test/unit/coffee/ProjectManager/getProjectDocsTests.coffee

diff --git a/services/document-updater/test/unit/coffee/ProjectManager/getProjectDocsTests.coffee b/services/document-updater/test/unit/coffee/ProjectManager/getProjectDocsTests.coffee
new file mode 100644
index 0000000000..21ed1d0eb1
--- /dev/null
+++ b/services/document-updater/test/unit/coffee/ProjectManager/getProjectDocsTests.coffee
@@ -0,0 +1,101 @@
+sinon = require('sinon')
+chai = require('chai')
+should = chai.should()
+modulePath = "../../../../app/js/ProjectManager.js"
+SandboxedModule = require('sandboxed-module')
+Errors = require "../../../../app/js/Errors.js"
+
+describe "ProjectManager - getProjectDocs", ->
+	beforeEach ->
+		@ProjectManager = SandboxedModule.require modulePath, requires:
+			"./RedisManager": @RedisManager = {}
+			"./DocumentManager": @DocumentManager = {}
+			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
+			"./Metrics": @Metrics =
+				Timer: class Timer
+					done: sinon.stub()
+		@project_id = "project-id-123"
+		@callback = sinon.stub()
+
+	describe "successfully", ->
+		beforeEach (done) ->
+			@doc_ids = ["doc-id-1", "doc-id-2", "doc-id-3"]
+			@doc_versions = [111, 222, 333]
+			@doc_lines = [["aaa","aaa"],["bbb","bbb"],["ccc","ccc"]]
+			@docs = [
+				{_id: @doc_ids[0], lines: @doc_lines[0], v: @doc_versions[0]}
+				{_id: @doc_ids[1], lines: @doc_lines[1], v: @doc_versions[1]}
+				{_id: @doc_ids[2], lines: @doc_lines[2], v: @doc_versions[2]}
+			]
+			@RedisManager.checkOrSetProjectState = sinon.stub().callsArgWith(2, null)
+			@RedisManager.getDocIdsInProject = sinon.stub().callsArgWith(1, null, @doc_ids)
+			@RedisManager.getDocVersion = sinon.stub()
+			@RedisManager.getDocVersion.withArgs(@doc_ids[0]).callsArgWith(1, null, @doc_versions[0])
+			@RedisManager.getDocVersion.withArgs(@doc_ids[1]).callsArgWith(1, null, @doc_versions[1])
+			@RedisManager.getDocVersion.withArgs(@doc_ids[2]).callsArgWith(1, null, @doc_versions[2])
+			@RedisManager.getDocLines = sinon.stub()
+			@RedisManager.getDocLines.withArgs(@doc_ids[0]).callsArgWith(1, null, @doc_lines[0])
+			@RedisManager.getDocLines.withArgs(@doc_ids[1]).callsArgWith(1, null, @doc_lines[1])
+			@RedisManager.getDocLines.withArgs(@doc_ids[2]).callsArgWith(1, null, @doc_lines[2])
+			@ProjectManager.getProjectDocs @project_id, @projectStateHash, @excludeVersions,  (error, docs) =>
+				@callback(error, docs)
+				done()
+
+		it "should check the project state", ->
+			@RedisManager.checkOrSetProjectState
+				.calledWith(@project_id, @projectStateHash)
+				.should.equal true
+
+		it "should get the doc ids in the project", ->
+			@RedisManager.getDocIdsInProject
+				.calledWith(@project_id)
+				.should.equal true
+
+		it "should call the callback without error", ->
+			@callback.calledWith(null, @docs).should.equal true
+
+		it "should time the execution", ->
+			@Metrics.Timer::done.called.should.equal true
+
+	describe "when the state does not match", ->
+		beforeEach (done) ->
+			@doc_ids = ["doc-id-1", "doc-id-2", "doc-id-3"]
+			@RedisManager.checkOrSetProjectState = sinon.stub().callsArgWith(2, null, true)
+			@ProjectManager.getProjectDocs @project_id, @projectStateHash, @excludeVersions,  (error, docs) =>
+				@callback(error, docs)
+				done()
+
+		it "should check the project state", ->
+			@RedisManager.checkOrSetProjectState
+				.calledWith(@project_id, @projectStateHash)
+				.should.equal true
+
+		it "should call the callback with an error", ->
+			@callback.calledWith(new Errors.ProjectStateChangedError("project state changed")).should.equal true
+
+		it "should time the execution", ->
+			@Metrics.Timer::done.called.should.equal true
+
+	describe "when a doc errors", ->
+		beforeEach (done) ->
+			@doc_ids = ["doc-id-1", "doc-id-2", "doc-id-3"]
+			@RedisManager.checkOrSetProjectState = sinon.stub().callsArgWith(2, null)
+			@RedisManager.getDocIdsInProject = sinon.stub().callsArgWith(1, null, @doc_ids)
+			@RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null)
+			@RedisManager.getDocLines = sinon.stub()
+			@RedisManager.getDocLines.withArgs("doc-id-1").callsArgWith(1, null)
+			@RedisManager.getDocLines.withArgs("doc-id-2").callsArgWith(1, @error = new Error("oops")) # trigger an error
+			@ProjectManager.getProjectDocs @project_id, @projectStateHash, @excludeVersions, (error, docs) =>
+				@callback(error)
+				done()
+
+		it "should record the error", ->
+			@logger.error
+				.calledWith(err: @error, project_id: @project_id, doc_id: "doc-id-2", "error getting project doc lines")
+				.should.equal true
+
+		it "should call the callback with an error", ->
+			@callback.calledWith(new Error("oops")).should.equal true
+
+		it "should time the execution", ->
+			@Metrics.Timer::done.called.should.equal true

From 5971a19084d0b4df28a1ea2af65e8220f46a2913 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Fri, 11 Aug 2017 16:55:31 +0100
Subject: [PATCH 13/14] added acceptance tests

---
 .../coffee/GettingProjectDocsTests.coffee     | 68 +++++++++++++++++++
 .../coffee/helpers/DocUpdaterClient.coffee    |  6 ++
 2 files changed, 74 insertions(+)
 create mode 100644 services/document-updater/test/acceptance/coffee/GettingProjectDocsTests.coffee

diff --git a/services/document-updater/test/acceptance/coffee/GettingProjectDocsTests.coffee b/services/document-updater/test/acceptance/coffee/GettingProjectDocsTests.coffee
new file mode 100644
index 0000000000..708176ea69
--- /dev/null
+++ b/services/document-updater/test/acceptance/coffee/GettingProjectDocsTests.coffee
@@ -0,0 +1,68 @@
+sinon = require "sinon"
+chai = require("chai")
+chai.should()
+expect = chai.expect
+
+MockWebApi = require "./helpers/MockWebApi"
+DocUpdaterClient = require "./helpers/DocUpdaterClient"
+
+describe "Getting documents for project", ->
+	before (done) ->
+		@lines = ["one", "two", "three"]
+		@version = 42
+		setTimeout done, 200 # Give MockWebApi a chance to start
+
+	describe "when project state hash does not match", ->
+		before (done) ->
+			@projectStateHash = DocUpdaterClient.randomId()
+			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+
+			MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version}
+			DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
+				throw error if error?
+				DocUpdaterClient.getProjectDocs @project_id, @projectStateHash, (error, @res, @returnedDocs)	=>
+					done()
+
+		it "should return a 409 Conflict response", ->
+			@res.statusCode.should.equal 409
+
+
+	describe "when project state hash matches", ->
+		before (done) ->
+			@projectStateHash = DocUpdaterClient.randomId()
+			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+
+			MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version}
+			DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
+				throw error if error?
+				DocUpdaterClient.getProjectDocs @project_id, @projectStateHash, (error, @res0, @returnedDocs0)	=>
+					# set the hash
+					DocUpdaterClient.getProjectDocs @project_id, @projectStateHash, (error, @res, @returnedDocs)	=>
+						# the hash should now match
+						done()
+
+		it "should return a 200 response", ->
+			@res.statusCode.should.equal 200
+
+		it "should return the documents", ->
+			@returnedDocs.should.deep.equal [ {_id: @doc_id, lines: @lines, v: @version} ]
+
+
+	describe "when the doc has been removed", ->
+		before (done) ->
+			@projectStateHash = DocUpdaterClient.randomId()
+			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+
+			MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version}
+			DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
+				throw error if error?
+				DocUpdaterClient.getProjectDocs @project_id, @projectStateHash, (error, @res0, @returnedDocs0)	=>
+					# set the hash
+					DocUpdaterClient.deleteDoc @project_id, @doc_id, (error, res, body) =>
+					# delete the doc
+						DocUpdaterClient.getProjectDocs @project_id, @projectStateHash, (error, @res, @returnedDocs)	=>
+							# the hash would match, but the doc has been deleted
+							done()
+
+		it "should return a 409 Conflict response", ->
+			@res.statusCode.should.equal 409
diff --git a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee
index 4b57e0659f..6b2a5ac2fb 100644
--- a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee
+++ b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee
@@ -80,3 +80,9 @@ module.exports = DocUpdaterClient =
 
 	removeComment: (project_id, doc_id, comment, callback = () ->) ->
 		request.del "http://localhost:3003/project/#{project_id}/doc/#{doc_id}/comment/#{comment}", callback
+
+	getProjectDocs: (project_id, projectStateHash, callback = () ->) ->
+		request.get "http://localhost:3003/project/#{project_id}/doc?state=#{projectStateHash}", (error, res, body) ->
+			if body? and res.statusCode >= 200 and res.statusCode < 300
+				body = JSON.parse(body)
+			callback error, res, body

From 8372911a1b8cb057dd4883f47a453e35297568d6 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Fri, 11 Aug 2017 16:56:30 +0100
Subject: [PATCH 14/14] return doclines as array from getProjectDocs

for compatibility with getDoc
---
 services/document-updater/app/coffee/ProjectManager.coffee | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee
index c25f51277f..60446485b0 100644
--- a/services/document-updater/app/coffee/ProjectManager.coffee
+++ b/services/document-updater/app/coffee/ProjectManager.coffee
@@ -88,7 +88,10 @@ module.exports = ProjectManager =
 									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}
+									try
+										docs.push {_id: doc_id, lines: JSON.parse(lines), v: version}
+									catch e
+										return cb(e)
 									cb()
 				async.series jobs, (error) ->
 					return callback(error) if error?