diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index 49cfa1f1b8..a580f8f113 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/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 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 13e618e734..369b429ab1 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -37,6 +37,28 @@ module.exports = HttpController = size += (line.length + 1) return size + 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" + timer = new Metrics.Timer("http.getAllDocs") + excludeVersions = {} + for item in excludeItems + [id,version] = item?.split(':') + excludeVersions[id] = version + logger.log {project_id: project_id, projectStateHash: projectStateHash, excludeVersions: excludeVersions}, "excluding versions" + ProjectManager.getProjectDocs project_id, projectStateHash, excludeVersions, (error, result) -> + timer.done() + 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.v}" for doc in 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..60446485b0 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) ->) -> @@ -56,3 +57,42 @@ module.exports = ProjectManager = callback new Error("Errors deleting docs. See log for details") else callback(null) + + getProjectDocs: (project_id, projectStateHash, excludeVersions = {}, _callback = (error, docs) ->) -> + timer = new Metrics.Timer("projectManager.getProjectDocs") + callback = (args...) -> + timer.done() + _callback(args...) + + RedisManager.checkOrSetProjectState project_id, projectStateHash, (error, projectStateChanged) -> + return callback(error) if 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? + 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) -> + 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? and version is excludeVersions[doc_id] + # otherwise get the doc lines from redis + RedisManager.getDocLines doc_id, (error, lines) -> + if error? + logger.error err: error, project_id: project_id, doc_id: doc_id, "error getting project doc lines" + return cb(error) + 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? + callback(null, docs) diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index be5018297d..718eb1f5b8 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -88,7 +88,19 @@ 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 + multi.del keys.projectState(project_id:project_id) + multi.exec callback + + checkOrSetProjectState: (project_id, newState, callback = (error, stateChanged) ->) -> + 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") @@ -142,6 +154,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 +256,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 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" 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 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 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 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