From fb1852a593b71955a91c5d9ae95e09479a966012 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 11 Apr 2018 11:03:20 +0100 Subject: [PATCH] cache projectHistoryId with doc in Redis --- .../app/coffee/DocumentManager.coffee | 28 +++++++-------- .../app/coffee/PersistenceManager.coffee | 4 +-- .../app/coffee/RedisManager.coffee | 15 ++++---- .../app/coffee/UpdateManager.coffee | 7 ++-- .../config/settings.defaults.coffee | 1 + .../DocumentManagerTests.coffee | 22 ++++++------ .../PersistenceManagerTests.coffee | 6 +++- .../RedisManager/RedisManagerTests.coffee | 36 +++++++++++++------ .../UpdateManager/UpdateManagerTests.coffee | 12 ++++--- 9 files changed, 80 insertions(+), 51 deletions(-) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 640ebf63cb..8b4f4192ec 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -13,39 +13,39 @@ async = require "async" MAX_UNFLUSHED_AGE = 300 * 1000 # 5 mins, document should be flushed to mongo this time after a change module.exports = DocumentManager = - getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname, unflushedTime, alreadyLoaded) ->) -> + getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname, projectHistoryId, unflushedTime, alreadyLoaded) ->) -> timer = new Metrics.Timer("docManager.getDoc") callback = (args...) -> timer.done() _callback(args...) - RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, unflushedTime) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId, unflushedTime) -> return callback(error) if error? if !lines? or !version? logger.log {project_id, doc_id}, "doc not in redis so getting from persistence API" - PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> + PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> return callback(error) if error? - logger.log {project_id, doc_id, lines, version, pathname}, "got doc from persistence API" - RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, pathname, (error) -> + logger.log {project_id, doc_id, lines, version, pathname, projectHistoryId}, "got doc from persistence API" + RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, pathname, projectHistoryId, (error) -> return callback(error) if error? - callback null, lines, version, ranges, pathname, null, false + callback null, lines, version, ranges, pathname, projectHistoryId, null, false else - callback null, lines, version, ranges, pathname, unflushedTime, true + callback null, lines, version, ranges, pathname, projectHistoryId, unflushedTime, true - getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, ops, ranges, pathname) ->) -> + getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, ops, ranges, pathname, projectHistoryId) ->) -> timer = new Metrics.Timer("docManager.getDocAndRecentOps") callback = (args...) -> timer.done() _callback(args...) - DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> return callback(error) if error? if fromVersion == -1 - callback null, lines, version, [], ranges, pathname + callback null, lines, version, [], ranges, pathname, projectHistoryId else RedisManager.getPreviousDocOps doc_id, fromVersion, version, (error, ops) -> return callback(error) if error? - callback null, lines, version, ops, ranges, pathname + callback null, lines, version, ops, ranges, pathname, projectHistoryId setDoc: (project_id, doc_id, newLines, source, user_id, undoing, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.setDoc") @@ -57,7 +57,7 @@ module.exports = DocumentManager = return callback(new Error("No lines were provided to setDoc")) UpdateManager = require "./UpdateManager" - DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, ranges, pathname, unflushedTime, alreadyLoaded) -> + DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, ranges, pathname, projectHistoryId, unflushedTime, alreadyLoaded) -> return callback(error) if error? if oldLines? and oldLines.length > 0 and oldLines[0].text? @@ -170,7 +170,7 @@ module.exports = DocumentManager = RedisManager.renameDoc project_id, doc_id, user_id, update, callback getDocAndFlushIfOld: (project_id, doc_id, callback = (error, doc) ->) -> - DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, unflushedTime, alreadyLoaded) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId, unflushedTime, alreadyLoaded) -> return callback(error) if error? # if doc was already loaded see if it needs to be flushed if alreadyLoaded and unflushedTime? and (Date.now() - unflushedTime) > MAX_UNFLUSHED_AGE @@ -195,7 +195,7 @@ module.exports = DocumentManager = UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDoc, project_id, doc_id, callback - getDocAndRecentOpsWithLock: (project_id, doc_id, fromVersion, callback = (error, lines, version, ops, ranges, pathname) ->) -> + getDocAndRecentOpsWithLock: (project_id, doc_id, fromVersion, callback = (error, lines, version, ops, ranges, pathname, projectHistoryId) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDocAndRecentOps, project_id, doc_id, fromVersion, callback diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee index bd5ce5239c..8a43d989a8 100644 --- a/services/document-updater/app/coffee/PersistenceManager.coffee +++ b/services/document-updater/app/coffee/PersistenceManager.coffee @@ -13,7 +13,7 @@ request = (require("requestretry")).defaults({ MAX_HTTP_REQUEST_LENGTH = 5000 # 5 seconds module.exports = PersistenceManager = - getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname) ->) -> + getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname, projectHistoryId) ->) -> timer = new Metrics.Timer("persistenceManager.getDoc") callback = (args...) -> timer.done() @@ -44,7 +44,7 @@ module.exports = PersistenceManager = return callback(new Error("web API response had no valid doc version")) if !body.pathname? return callback(new Error("web API response had no valid doc pathname")) - return callback null, body.lines, body.version, body.ranges, body.pathname + return callback null, body.lines, body.version, body.ranges, body.pathname, body.projectHistoryId else if res.statusCode == 404 return callback(new Errors.NotFoundError("doc not not found: #{url}")) else diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 382ff2b502..b483839302 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -36,7 +36,7 @@ historyKeys = Settings.redis.history.key_schema module.exports = RedisManager = rclient: rclient - putDocInMemory : (project_id, doc_id, docLines, version, ranges, pathname, _callback)-> + putDocInMemory : (project_id, doc_id, docLines, version, ranges, pathname, projectHistoryId, _callback)-> timer = new metrics.Timer("redis.put-doc") callback = (error) -> timer.done() @@ -47,7 +47,7 @@ module.exports = RedisManager = logger.error {err: error, doc_id: doc_id, docLines: docLines}, error.message return callback(error) docHash = RedisManager._computeHash(docLines) - logger.log {project_id, doc_id, version, docHash, pathname}, "putting doc in redis" + logger.log {project_id, doc_id, version, docHash, pathname, projectHistoryId}, "putting doc in redis" RedisManager._serializeRanges ranges, (error, ranges) -> if error? logger.error {err: error, doc_id, project_id}, error.message @@ -62,6 +62,7 @@ module.exports = RedisManager = else multi.del keys.ranges(doc_id:doc_id) multi.set keys.pathname(doc_id:doc_id), pathname + multi.set keys.projectHistoryId(doc_id:doc_id), projectHistoryId multi.exec (error, result) -> return callback(error) if error? # check the hash computed on the redis server @@ -88,6 +89,7 @@ module.exports = RedisManager = multi.del keys.docHash(doc_id:doc_id) multi.del keys.ranges(doc_id:doc_id) multi.del keys.pathname(doc_id:doc_id) + multi.del keys.projectHistoryId(doc_id:doc_id) multi.del keys.unflushedTime(doc_id:doc_id) multi.exec (error) -> return callback(error) if error? @@ -108,7 +110,7 @@ module.exports = RedisManager = clearProjectState: (project_id, callback = (error) ->) -> rclient.del keys.projectState(project_id:project_id), callback - getDoc : (project_id, doc_id, callback = (error, lines, version, ranges, pathname, unflushedTime) ->)-> + getDoc : (project_id, doc_id, callback = (error, lines, version, ranges, pathname, projectHistoryId, unflushedTime) ->)-> timer = new metrics.Timer("redis.get-doc") multi = rclient.multi() multi.get keys.docLines(doc_id:doc_id) @@ -117,8 +119,9 @@ module.exports = RedisManager = multi.get keys.projectKey(doc_id:doc_id) multi.get keys.ranges(doc_id:doc_id) multi.get keys.pathname(doc_id:doc_id) + multi.get keys.projectHistoryId(doc_id:doc_id) multi.get keys.unflushedTime(doc_id:doc_id) - multi.exec (error, [docLines, version, storedHash, doc_project_id, ranges, pathname, unflushedTime])-> + multi.exec (error, [docLines, version, storedHash, doc_project_id, ranges, pathname, projectHistoryId, unflushedTime])-> timeSpan = timer.done() return callback(error) if error? # check if request took too long and bail out. only do this for @@ -147,14 +150,14 @@ module.exports = RedisManager = # doc is not in redis, bail out if !docLines? - return callback null, docLines, version, ranges, pathname, unflushedTime + return callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime # doc should be in project set, check if missing (workaround for missing docs from putDoc) rclient.sadd keys.docsInProject(project_id:project_id), doc_id, (error, result) -> return callback(error) if error? if result isnt 0 # doc should already be in set logger.error project_id: project_id, doc_id: doc_id, doc_project_id: doc_project_id, "doc missing from docsInProject set" - callback null, docLines, version, ranges, pathname, unflushedTime + callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime getDocVersion: (doc_id, callback = (error, version) ->) -> rclient.get keys.docVersion(doc_id: doc_id), (error, version) -> diff --git a/services/document-updater/app/coffee/UpdateManager.coffee b/services/document-updater/app/coffee/UpdateManager.coffee index 02f0a1b8d1..bfcfb806ca 100644 --- a/services/document-updater/app/coffee/UpdateManager.coffee +++ b/services/document-updater/app/coffee/UpdateManager.coffee @@ -71,7 +71,7 @@ module.exports = UpdateManager = profile = new Profiler("applyUpdate", {project_id, doc_id}) UpdateManager._sanitizeUpdate update profile.log("sanitizeUpdate") - DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> profile.log("getDoc") return callback(error) if error? if !lines? or !version? @@ -80,7 +80,7 @@ module.exports = UpdateManager = profile.log("sharejs.applyUpdate") return callback(error) if error? RangesManager.applyUpdate project_id, doc_id, ranges, appliedOps, updatedDocLines, (error, new_ranges) -> - UpdateManager._addProjectHistoryMetadataToOps(appliedOps, pathname, lines) + UpdateManager._addProjectHistoryMetadataToOps(appliedOps, pathname, projectHistoryId, lines) profile.log("RangesManager.applyUpdate") return callback(error) if error? RedisManager.updateDocument project_id, doc_id, updatedDocLines, version, appliedOps, new_ranges, (error, doc_ops_length, project_ops_length) -> @@ -130,12 +130,13 @@ module.exports = UpdateManager = op.i = op.i.replace(/[\uD800-\uDFFF]/g, "\uFFFD") return update - _addProjectHistoryMetadataToOps: (updates, pathname, lines) -> + _addProjectHistoryMetadataToOps: (updates, pathname, projectHistoryId, lines) -> doc_length = _.reduce lines, (chars, line) -> chars + line.length, 0 doc_length += lines.length - 1 # count newline characters updates.forEach (update) -> + update.projectHistoryId = projectHistoryId update.meta ||= {} update.meta.pathname = pathname update.meta.doc_length = doc_length diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index 1c7ebf283e..18f2b1570b 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -46,6 +46,7 @@ module.exports = docsInProject: ({project_id}) -> "DocsIn:#{project_id}" ranges: ({doc_id}) -> "Ranges:#{doc_id}" pathname: ({doc_id}) -> "Pathname:#{doc_id}" + projectHistoryId: ({doc_id}) -> "ProjectHistoryId:#{doc_id}" projectState: ({project_id}) -> "ProjectState:#{project_id}" unflushedTime: ({doc_id}) -> "UnflushedTime:#{doc_id}" # cluster: [{ diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index 16e58a81a7..a2d55d1cb3 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -111,7 +111,7 @@ describe "DocumentManager", -> describe "getDocAndRecentOps", -> describe "with a previous version specified", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, @fromVersion, @callback @@ -126,14 +126,14 @@ describe "DocumentManager", -> .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ops, @ranges, @pathname).should.equal true + @callback.calledWith(null, @lines, @version, @ops, @ranges, @pathname, @projectHistoryId).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true describe "with no previous version specified", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, -1, @callback @@ -146,7 +146,7 @@ describe "DocumentManager", -> @RedisManager.getPreviousDocOps.called.should.equal false it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, [], @ranges, @pathname).should.equal true + @callback.calledWith(null, @lines, @version, [], @ranges, @pathname, @projectHistoryId).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -154,7 +154,7 @@ describe "DocumentManager", -> describe "getDoc", -> describe "when the doc exists in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @unflushedTime) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushedTime) @DocumentManager.getDoc @project_id, @doc_id, @callback it "should get the doc from Redis", -> @@ -163,7 +163,7 @@ describe "DocumentManager", -> .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ranges, @pathname, @unflushedTime, true).should.equal true + @callback.calledWith(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushedTime, true).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -171,7 +171,7 @@ describe "DocumentManager", -> describe "when the doc does not exist in Redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null, null) - @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) + @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId) @RedisManager.putDocInMemory = sinon.stub().yields() @DocumentManager.getDoc @project_id, @doc_id, @callback @@ -187,11 +187,11 @@ describe "DocumentManager", -> it "should set the doc in Redis", -> @RedisManager.putDocInMemory - .calledWith(@project_id, @doc_id, @lines, @version, @ranges, @pathname) + .calledWith(@project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId) .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @ranges, @pathname, null, false).should.equal true + @callback.calledWith(null, @lines, @version, @ranges, @pathname, @projectHistoryId, null, false).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -202,7 +202,7 @@ describe "DocumentManager", -> @beforeLines = ["before", "lines"] @afterLines = ["after", "lines"] @ops = [{ i: "foo", p: 4 }, { d: "bar", p: 42 }] - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @ranges, @pathname, @unflushedTime, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @ranges, @pathname, @projectHistoryId, @unflushedTime, true) @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) @@ -402,7 +402,7 @@ describe "DocumentManager", -> describe "when the doc is in Redis", -> describe "and has changes to be flushed", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, Date.now() - 1e9, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @projectHistoryId, @pathname, Date.now() - 1e9, true) @DocumentManager.getDocAndFlushIfOld @project_id, @doc_id, @callback it "should get the doc", -> diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee index 937dcf3a77..0f8ad59167 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee @@ -17,6 +17,7 @@ describe "PersistenceManager", -> done: sinon.stub() "logger-sharelatex": @logger = {log: sinon.stub(), err: sinon.stub()} @project_id = "project-id-123" + @projectHistoryId = "history-id-123" @doc_id = "doc-id-123" @lines = ["one", "two", "three"] @version = 42 @@ -36,6 +37,7 @@ describe "PersistenceManager", -> version: @version, ranges: @ranges pathname: @pathname, + projectHistoryId: @projectHistoryId } describe "with a successful response from the web api", -> @@ -60,7 +62,9 @@ describe "PersistenceManager", -> .should.equal true it "should call the callback with the doc lines, version and ranges", -> - @callback.calledWith(null, @lines, @version, @ranges, @pathname).should.equal true + @callback + .calledWith(null, @lines, @version, @ranges, @pathname, @projectHistoryId) + .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 a3b28d00cb..5166287a34 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -33,6 +33,7 @@ describe "RedisManager", -> docsInProject: ({project_id}) -> "DocsIn:#{project_id}" ranges: ({doc_id}) -> "Ranges:#{doc_id}" pathname: ({doc_id}) -> "Pathname:#{doc_id}" + projectHistoryId: ({doc_id}) -> "ProjectHistoryId:#{doc_id}" projectState: ({project_id}) -> "ProjectState:#{project_id}" unflushedTime: ({doc_id}) -> "UnflushedTime:#{doc_id}" history: @@ -72,7 +73,7 @@ describe "RedisManager", -> @unflushed_time = 12345 @pathname = '/a/b/c.tex' @multi.get = sinon.stub() - @multi.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @hash, @project_id, @json_ranges, @pathname, @unflushed_time]) + @multi.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @hash, @project_id, @json_ranges, @pathname, @projectHistoryId, @unflushed_time]) @rclient.sadd = sinon.stub().yields(null, 0) describe "successfully", -> @@ -109,6 +110,11 @@ describe "RedisManager", -> .calledWith("Pathname:#{@doc_id}") .should.equal true + it "should get the projectHistoryId", -> + @multi.get + .calledWith("ProjectHistoryId:#{@doc_id}") + .should.equal true + it "should check if the document is in the DocsIn set", -> @rclient.sadd .calledWith("DocsIn:#{@project_id}") @@ -116,7 +122,7 @@ describe "RedisManager", -> it 'should return the document', -> @callback - .calledWithExactly(null, @lines, @version, @ranges, @pathname, @unflushed_time) + .calledWithExactly(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushed_time) .should.equal true it 'should not log any errors', -> @@ -125,7 +131,7 @@ describe "RedisManager", -> describe "when the document is not present", -> beforeEach -> - @multi.exec = sinon.stub().callsArgWith(0, null, [null, null, null, null, null, null, null]) + @multi.exec = sinon.stub().callsArgWith(0, null, [null, null, null, null, null, null, null, null]) @rclient.sadd = sinon.stub().yields() @RedisManager.getDoc @project_id, @doc_id, @callback @@ -136,7 +142,7 @@ describe "RedisManager", -> it 'should return an empty result', -> @callback - .calledWithExactly(null, null, 0, {}, null, null) + .calledWithExactly(null, null, 0, {}, null, null, null) .should.equal true it 'should not log any errors', -> @@ -154,7 +160,7 @@ describe "RedisManager", -> it 'should return the document', -> @callback - .calledWithExactly(null, @lines, @version, @ranges, @pathname, @unflushed_time) + .calledWithExactly(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushed_time) .should.equal true describe "with a corrupted document", -> @@ -532,7 +538,7 @@ describe "RedisManager", -> describe "with non-empty ranges", -> beforeEach (done) -> - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, done + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId, done it "should set the lines", -> @multi.eval @@ -564,6 +570,11 @@ describe "RedisManager", -> .calledWith("Pathname:#{@doc_id}", @pathname) .should.equal true + it "should set the projectHistoryId for the doc", -> + @multi.set + .calledWith("ProjectHistoryId:#{@doc_id}", @projectHistoryId) + .should.equal true + it "should add the doc_id to the project set", -> @rclient.sadd .calledWith("DocsIn:#{@project_id}", @doc_id) @@ -575,7 +586,7 @@ describe "RedisManager", -> describe "with empty ranges", -> beforeEach (done) -> - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, {}, @pathname, done + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, {}, @pathname, @projectHistoryId, done it "should delete the ranges key", -> @multi.del @@ -590,7 +601,7 @@ describe "RedisManager", -> describe "with a corrupted write", -> beforeEach (done) -> @multi.exec = sinon.stub().callsArgWith(0, null, ["INVALID-HASH-VALUE"]) - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, done + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId, done it 'should log a hash error', -> @logger.error.calledWith() @@ -600,7 +611,7 @@ describe "RedisManager", -> beforeEach -> @_stringify = JSON.stringify @JSON.stringify = () -> return '["bad bytes! \u0000 <- here"]' - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @callback + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId, @callback afterEach -> @JSON.stringify = @_stringify @@ -614,7 +625,7 @@ describe "RedisManager", -> describe "with ranges that are too big", -> beforeEach -> @RedisManager._serializeRanges = sinon.stub().yields(new Error("ranges are too large")) - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @callback + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId, @callback it 'should log an error', -> @logger.error.called.should.equal true @@ -664,6 +675,11 @@ describe "RedisManager", -> .calledWith("Pathname:#{@doc_id}") .should.equal true + it "should delete the pathname for the doc", -> + @multi.del + .calledWith("ProjectHistoryId:#{@doc_id}") + .should.equal true + describe "clearProjectState", -> beforeEach (done) -> @rclient.del = sinon.stub().callsArg(1) diff --git a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee index e91c35f7e6..383bd1848e 100644 --- a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee @@ -7,6 +7,7 @@ SandboxedModule = require('sandboxed-module') describe "UpdateManager", -> beforeEach -> @project_id = "project-id-123" + @projectHistoryId = "history-id-123" @doc_id = "document-id-123" @callback = sinon.stub() @UpdateManager = SandboxedModule.require modulePath, requires: @@ -167,7 +168,7 @@ describe "UpdateManager", -> @doc_ops_length = sinon.stub() @project_ops_length = sinon.stub() @pathname = '/a/b/c.tex' - @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges, @pathname) + @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges, @pathname, @projectHistoryId) @RangesManager.applyUpdate = sinon.stub().yields(null, @updated_ranges) @ShareJsUpdateManager.applyUpdate = sinon.stub().yields(null, @updatedDocLines, @version, @appliedOps) @RedisManager.updateDocument = sinon.stub().yields(null, @doc_ops_length, @project_ops_length) @@ -196,7 +197,7 @@ describe "UpdateManager", -> it "should add metadata to the ops" , -> @UpdateManager._addProjectHistoryMetadataToOps - .calledWith(@appliedOps, @pathname, @lines) + .calledWith(@appliedOps, @pathname, @projectHistoryId, @lines) .should.equal true it "should push the applied ops into the history queue", -> @@ -239,7 +240,7 @@ describe "UpdateManager", -> @callback.calledWith(@error).should.equal true describe "_addProjectHistoryMetadataToOps", -> - it "should add pathname and doc_length metadata to the ops", -> + it "should add projectHistoryId, pathname and doc_length metadata to the ops", -> lines = [ 'some' 'test' @@ -250,20 +251,23 @@ describe "UpdateManager", -> { v: 45, op: [{d: "qux", p: 4}, { i: "bazbaz", p: 14 }] }, { v: 49, op: [{i: "penguin", p: 18}] } ] - @UpdateManager._addProjectHistoryMetadataToOps(appliedOps, @pathname, lines) + @UpdateManager._addProjectHistoryMetadataToOps(appliedOps, @pathname, @projectHistoryId, lines) appliedOps.should.deep.equal [{ + projectHistoryId: @projectHistoryId v: 42 op: [{i: "foo", p: 4}, { i: "bar", p: 6 }] meta: pathname: @pathname doc_length: 14 }, { + projectHistoryId: @projectHistoryId v: 45 op: [{d: "qux", p: 4}, { i: "bazbaz", p: 14 }] meta: pathname: @pathname doc_length: 20 # 14 + 'foo' + 'bar' }, { + projectHistoryId: @projectHistoryId v: 49 op: [{i: "penguin", p: 18}] meta: