Merge branch 'master' into ja-consistency-refactor

services/document-updater/app.coffee

@@ -28,6 +28,18 @@ app.configure ->
 
 DispatchManager.createAndStartDispatchers(Settings.dispatcherCount || 10)
 
+app.param 'project_id', (req, res, next, project_id) ->
+	if project_id?.match /^[0-9a-f]{24}$/
+		next()
+	else
+		next new Error("invalid project id")
+
+app.param 'doc_id', (req, res, next, doc_id) ->
+	if doc_id?.match /^[0-9a-f]{24}$/
+		next()
+	else
+		next new Error("invalid doc id")
+
 app.get    '/project/:project_id/doc/:doc_id',       HttpController.getDoc
 app.post   '/project/:project_id/doc/:doc_id',       HttpController.setDoc
 app.post   '/project/:project_id/doc/:doc_id/flush', HttpController.flushDocIfLoaded

services/document-updater/app/coffee/DocumentManager.coffee

@@ -6,13 +6,13 @@ Metrics = require "./Metrics"
 TrackChangesManager = require "./TrackChangesManager"
 
 module.exports = DocumentManager =
-	getDoc: (project_id, doc_id, _callback = (error, lines, version) ->) ->
+	getDoc: (project_id, doc_id, _callback = (error, lines, version, alreadyLoaded) ->) ->
 		timer = new Metrics.Timer("docManager.getDoc")
 		callback = (args...) ->
 			timer.done()
 			_callback(args...)
 
-		RedisManager.getDoc doc_id, (error, lines, version, alreadyLoaded) ->
+		RedisManager.getDoc project_id, doc_id, (error, lines, version) ->
 			return callback(error) if error?
 			if !lines? or !version?
 				logger.log project_id: project_id, doc_id: doc_id, "doc not in redis so getting from persistence API"
@@ -89,12 +89,11 @@ module.exports = DocumentManager =
 		callback = (args...) ->
 			timer.done()
 			_callback(args...)
-
-		RedisManager.getDoc doc_id, (error, lines, version) ->
+		RedisManager.getDoc project_id, doc_id, (error, lines, version) ->
 			return callback(error) if error?
 			if !lines? or !version?
 				logger.log project_id: project_id, doc_id: doc_id, "doc is not loaded so not flushing"
-				callback null
+				callback null  # TODO: return a flag to bail out, as we go on to remove doc from memory?
 			else
 				logger.log project_id: project_id, doc_id: doc_id, version: version, "flushing doc"
 				PersistenceManager.setDoc project_id, doc_id, lines, version, (error) ->

services/document-updater/app/coffee/RedisManager.coffee

@@ -45,11 +45,12 @@ module.exports = RedisManager =
 			return callback(error) if error?
 			rclient.srem keys.docsInProject(project_id:project_id), doc_id, callback
 
-	getDoc : (doc_id, callback = (error, lines, version) ->)->
+	getDoc : (project_id, doc_id, callback = (error, lines, version, project_id) ->)->
 		timer = new metrics.Timer("redis.get-doc")
 		multi = rclient.multi()
 		multi.get keys.docLines(doc_id:doc_id)
 		multi.get keys.docVersion(doc_id:doc_id)
+		multi.get keys.projectKey(doc_id:doc_id)
 		multi.exec (error, result)->
 			timer.done()
 			return callback(error) if error?
@@ -58,7 +59,12 @@ module.exports = RedisManager =
 			catch e
 				return callback(e)
 			version = parseInt(result[1] or 0, 10)
-			callback null, docLines, version
+			doc_project_id = result[2]
+			# check doc is in requested project
+			if doc_project_id? and doc_project_id isnt project_id
+				logger.error project_id: project_id, doc_id: doc_id, doc_project_id: doc_project_id, "doc not in project"
+				return callback(new Errors.NotFoundError("document not found"))
+			callback null, docLines, version, project_id
 
 	getDocVersion: (doc_id, callback = (error, version) ->) ->
 		rclient.get keys.docVersion(doc_id: doc_id), (error, version) ->

services/document-updater/test/unit/coffee/DocumentManager/flushDocIfLoadedTests.coffee

@@ -23,13 +23,13 @@ describe "DocumentManager.flushDocIfLoaded", ->
 
 	describe "when the doc is in Redis", ->
 		beforeEach ->
-			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, @lines, @version)
+			@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
 			@PersistenceManager.setDoc = sinon.stub().callsArgWith(4)
 			@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
 
 		it "should get the doc from redis", ->
 			@RedisManager.getDoc
-				.calledWith(@doc_id)
+				.calledWith(@project_id, @doc_id)
 				.should.equal true
 
 		it "should write the doc lines to the persistence layer", ->
@@ -45,14 +45,14 @@ describe "DocumentManager.flushDocIfLoaded", ->
 
 	describe "when the document is not in Redis", ->
 		beforeEach ->
-			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null)
+			@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null)
 			@PersistenceManager.setDoc = sinon.stub().callsArgWith(4)
 			@DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2)
 			@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
 
 		it "should get the doc from redis", ->
 			@RedisManager.getDoc
-				.calledWith(@doc_id)
+				.calledWith(@project_id, @doc_id)
 				.should.equal true
 
 		it "should not write anything to the persistence layer", ->

services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee

@@ -24,12 +24,12 @@ describe "DocumentUpdater.getDoc", ->
 
 	describe "when the doc exists in Redis", ->
 		beforeEach ->
-			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, @lines, @version)
+			@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
 			@DocumentManager.getDoc @project_id, @doc_id, @callback
 
 		it "should get the doc from Redis", ->
 			@RedisManager.getDoc
-				.calledWith(@doc_id)
+				.calledWith(@project_id, @doc_id)
 				.should.equal true
 		
 		it "should call the callback with the doc info", ->
@@ -40,14 +40,14 @@ describe "DocumentUpdater.getDoc", ->
 
 	describe "when the doc does not exist in Redis", ->
 		beforeEach ->
-			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null)
+			@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null)
 			@PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
 			@RedisManager.putDocInMemory = sinon.stub().callsArg(4)
 			@DocumentManager.getDoc @project_id, @doc_id, @callback
 
 		it "should try to get the doc from Redis", ->
 			@RedisManager.getDoc
-				.calledWith(@doc_id)
+				.calledWith(@project_id, @doc_id)
 				.should.equal true
 
 		it "should get the doc from the PersistenceManager", ->

services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee

@@ -38,8 +38,8 @@ describe "RedisManager", ->
 			@jsonlines = JSON.stringify @lines
 			@version = 42
 			@rclient.get = sinon.stub()
-			@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version])
-			@RedisManager.getDoc @doc_id, @callback
+			@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @project_id])
+			@RedisManager.getDoc @project_id, @doc_id, @callback
 		
 		it "should get the lines from redis", ->
 			@rclient.get
@@ -56,6 +56,21 @@ describe "RedisManager", ->
 				.calledWith(null, @lines, @version)
 				.should.equal true
 
+	describe "getDoc with an invalid project id", ->
+		beforeEach ->
+			@lines = ["one", "two", "three"]
+			@jsonlines = JSON.stringify @lines
+			@version = 42
+			@another_project_id = "project-id-456"
+			@rclient.get = sinon.stub()
+			@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @another_project_id])
+			@RedisManager.getDoc @project_id, @doc_id, @callback
+
+		it 'should return an error', ->
+			@callback
+				.calledWith(new Errors.NotFoundError("not found"))
+				.should.equal true
+
 	describe "getPreviousDocOpsTests", ->
 		describe "with a start and an end value", ->
 			beforeEach ->