diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index c716838878..af0ad242c2 100644 --- a/services/document-updater/app.coffee +++ b/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 diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index ddeaceea2f..63df6d0d79 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/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) -> diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 6e474c9b96..8063b20ddd 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/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) -> diff --git a/services/document-updater/test/unit/coffee/DocumentManager/flushDocIfLoadedTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/flushDocIfLoadedTests.coffee index bda914999b..4a17e2b84c 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/flushDocIfLoadedTests.coffee +++ b/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", -> diff --git a/services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee index b11686ac3c..3edf4cb67d 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee +++ b/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", -> diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index 0e122e63fb..5ee3398e87 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/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 ->