From c1c23e4bee82c750c4e0f844dc7e69b196a2350e Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Mon, 22 Apr 2019 20:02:48 -0400 Subject: [PATCH] record last author id on document flush This is a multi-steps process: * get a update's `user_id` from the metadata * store the `user_id` (`lastUpdatedBy`) and current date (`lastUpdatedAt`) for the document in Redis on every updates * fetch `lastUpdatedAt` and `lastUpdatedBy` from Redis on document flush * send the data to web to be persisted in Mongo --- .../app/coffee/DocumentManager.coffee | 8 +- .../app/coffee/PersistenceManager.coffee | 4 +- .../app/coffee/RedisManager.coffee | 17 ++++- .../app/coffee/UpdateManager.coffee | 2 +- .../config/settings.defaults.coffee | 2 + .../coffee/FlushingDocsTests.coffee | 10 ++- .../coffee/helpers/MockWebApi.coffee | 6 +- .../DocumentManagerTests.coffee | 10 ++- .../PersistenceManagerTests.coffee | 12 ++- .../RedisManager/RedisManagerTests.coffee | 75 +++++++++++++++---- .../UpdateManager/UpdateManagerTests.coffee | 5 +- 11 files changed, 115 insertions(+), 36 deletions(-) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 39713a1981..5183a3aaea 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -102,14 +102,14 @@ module.exports = DocumentManager = callback = (args...) -> timer.done() _callback(args...) - RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId, unflushedTime, lastUpdatedAt, lastUpdatedBy) -> 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 # 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, ranges, (error) -> + PersistenceManager.setDoc project_id, doc_id, lines, version, ranges, lastUpdatedAt, lastUpdatedBy, (error) -> return callback(error) if error? RedisManager.clearUnflushedTime doc_id, callback @@ -141,7 +141,7 @@ module.exports = DocumentManager = return callback(new Errors.NotFoundError("document not found: #{doc_id}")) RangesManager.acceptChanges change_ids, ranges, (error, new_ranges) -> return callback(error) if error? - RedisManager.updateDocument project_id, doc_id, lines, version, [], new_ranges, (error) -> + RedisManager.updateDocument project_id, doc_id, lines, version, [], new_ranges, {}, (error) -> return callback(error) if error? callback() @@ -157,7 +157,7 @@ module.exports = DocumentManager = return callback(new Errors.NotFoundError("document not found: #{doc_id}")) RangesManager.deleteComment comment_id, ranges, (error, new_ranges) -> return callback(error) if error? - RedisManager.updateDocument project_id, doc_id, lines, version, [], new_ranges, (error) -> + RedisManager.updateDocument project_id, doc_id, lines, version, [], new_ranges, {}, (error) -> return callback(error) if error? callback() diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee index 8a43d989a8..ee80453137 100644 --- a/services/document-updater/app/coffee/PersistenceManager.coffee +++ b/services/document-updater/app/coffee/PersistenceManager.coffee @@ -50,7 +50,7 @@ module.exports = PersistenceManager = else return callback(new Error("error accessing web API: #{url} #{res.statusCode}")) - setDoc: (project_id, doc_id, lines, version, ranges, _callback = (error) ->) -> + setDoc: (project_id, doc_id, lines, version, ranges, lastUpdatedAt, lastUpdatedBy,_callback = (error) ->) -> timer = new Metrics.Timer("persistenceManager.setDoc") callback = (args...) -> timer.done() @@ -64,6 +64,8 @@ module.exports = PersistenceManager = lines: lines ranges: ranges version: version + lastUpdatedBy: lastUpdatedBy + lastUpdatedAt: lastUpdatedAt auth: user: Settings.apis.web.user pass: Settings.apis.web.pass diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 76c7a9a8d0..85918f4608 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -90,6 +90,8 @@ module.exports = RedisManager = 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.del keys.lastUpdatedAt(doc_id: doc_id) + multi.del keys.lastUpdatedBy(doc_id: doc_id) multi.exec (error) -> return callback(error) if error? multi = rclient.multi() @@ -120,7 +122,9 @@ module.exports = RedisManager = 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, projectHistoryId, unflushedTime])-> + multi.get keys.lastUpdatedAt(doc_id: doc_id) + multi.get keys.lastUpdatedBy(doc_id: doc_id) + multi.exec (error, [docLines, version, storedHash, doc_project_id, ranges, pathname, projectHistoryId, unflushedTime, lastUpdatedAt, lastUpdatedBy])-> timeSpan = timer.done() return callback(error) if error? # check if request took too long and bail out. only do this for @@ -152,14 +156,14 @@ module.exports = RedisManager = # doc is not in redis, bail out if !docLines? - return callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime + return callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime, lastUpdatedAt, lastUpdatedBy # 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, projectHistoryId, unflushedTime + callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime, lastUpdatedAt, lastUpdatedBy getDocVersion: (doc_id, callback = (error, version) ->) -> rclient.get keys.docVersion(doc_id: doc_id), (error, version) -> @@ -209,7 +213,7 @@ module.exports = RedisManager = DOC_OPS_TTL: 60 * minutes DOC_OPS_MAX_LENGTH: 100 - updateDocument : (project_id, doc_id, docLines, newVersion, appliedOps = [], ranges, callback = (error) ->)-> + updateDocument : (project_id, doc_id, docLines, newVersion, appliedOps = [], ranges, updateMeta, callback = (error) ->)-> RedisManager.getDocVersion doc_id, (error, currentVersion) -> return callback(error) if error? if currentVersion + appliedOps.length != newVersion @@ -261,6 +265,11 @@ module.exports = RedisManager = # hasn't been modified before (the content in mongo has been # valid up to this point). Otherwise leave it alone ("NX" flag). multi.set keys.unflushedTime(doc_id: doc_id), Date.now(), "NX" + multi.set keys.lastUpdatedAt(doc_id: doc_id), Date.now() # index 8 + if updateMeta?.user_id + multi.set keys.lastUpdatedBy(doc_id: doc_id), updateMeta.user_id # index 9 + else + multi.del keys.lastUpdatedBy(doc_id: doc_id) # index 9 multi.exec (error, result) -> return callback(error) if error? # check the hash computed on the redis server diff --git a/services/document-updater/app/coffee/UpdateManager.coffee b/services/document-updater/app/coffee/UpdateManager.coffee index 2fe7508fed..e5ede11173 100644 --- a/services/document-updater/app/coffee/UpdateManager.coffee +++ b/services/document-updater/app/coffee/UpdateManager.coffee @@ -85,7 +85,7 @@ module.exports = UpdateManager = 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) -> + RedisManager.updateDocument project_id, doc_id, updatedDocLines, version, appliedOps, new_ranges, update.meta, (error, doc_ops_length, project_ops_length) -> profile.log("RedisManager.updateDocument") return callback(error) if error? HistoryManager.recordAndFlushHistoryOps project_id, doc_id, appliedOps, doc_ops_length, project_ops_length, (error) -> diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index d3dd079ace..f68077bb8b 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -79,6 +79,8 @@ module.exports = projectHistoryId: ({doc_id}) -> "ProjectHistoryId:{#{doc_id}}" projectState: ({project_id}) -> "ProjectState:{#{project_id}}" pendingUpdates: ({doc_id}) -> "PendingUpdates:{#{doc_id}}" + lastUpdatedBy: ({doc_id}) -> "lastUpdatedBy:{#{doc_id}}" + lastUpdatedAt: ({doc_id}) -> "lastUpdatedAt:{#{doc_id}}" redisOptions: keepAlive: 100 diff --git a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee index 709159ccfb..4f19f13c2f 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee @@ -14,6 +14,7 @@ describe "Flushing a doc to Mongo", -> @version = 42 @update = doc: @doc_id + meta: { user_id: 'last-author-fake-id' } op: [{ i: "one and a half\n" p: 4 @@ -42,6 +43,13 @@ describe "Flushing a doc to Mongo", -> .calledWith(@project_id, @doc_id, @result, @version + 1) .should.equal true + it "should flush the last update author and time to the web api", -> + lastUpdatedAt = MockWebApi.setDocument.lastCall.args[5] + parseInt(lastUpdatedAt).should.be.closeTo((new Date()).getTime(), 30000) + + lastUpdatedBy = MockWebApi.setDocument.lastCall.args[6] + lastUpdatedBy.should.equal 'last-author-fake-id' + describe "when the doc does not exist in the doc updater", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @@ -65,7 +73,7 @@ describe "Flushing a doc to Mongo", -> version: @version } t = 30000 - sinon.stub MockWebApi, "setDocument", (project_id, doc_id, lines, version, ranges, callback = (error) ->) -> + sinon.stub MockWebApi, "setDocument", (project_id, doc_id, lines, version, ranges, lastUpdatedAt, lastUpdatedBy, callback = (error) ->) -> setTimeout callback, t t = 0 DocUpdaterClient.preloadDoc @project_id, @doc_id, done diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee index 8523f7752e..5ee673ccf7 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -12,12 +12,14 @@ module.exports = MockWebApi = doc.pathname = '/a/b/c.tex' @docs["#{project_id}:#{doc_id}"] = doc - setDocument: (project_id, doc_id, lines, version, ranges, callback = (error) ->) -> + setDocument: (project_id, doc_id, lines, version, ranges, lastUpdatedAt, lastUpdatedBy, callback = (error) ->) -> doc = @docs["#{project_id}:#{doc_id}"] ||= {} doc.lines = lines doc.version = version doc.ranges = ranges doc.pathname = '/a/b/c.tex' + doc.lastUpdatedAt = lastUpdatedAt + doc.lastUpdatedBy = lastUpdatedBy callback null getDocument: (project_id, doc_id, callback = (error, doc) ->) -> @@ -34,7 +36,7 @@ module.exports = MockWebApi = res.send 404 app.post "/project/:project_id/doc/:doc_id", express.bodyParser(), (req, res, next) => - MockWebApi.setDocument req.params.project_id, req.params.doc_id, req.body.lines, req.body.version, req.body.ranges, (error) -> + MockWebApi.setDocument req.params.project_id, req.params.doc_id, req.body.lines, req.body.version, req.body.ranges, req.body.lastUpdatedAt, req.body.lastUpdatedBy, (error) -> if error? res.send 500 else diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index c52bb4b30d..dc57022a5a 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -35,6 +35,8 @@ describe "DocumentManager", -> @ranges = { comments: "mock", entries: "mock" } @pathname = '/a/b/c.tex' @unflushedTime = Date.now() + @lastUpdatedAt = Date.now() + @lastUpdatedBy = 'last-author-id' afterEach -> tk.reset() @@ -70,7 +72,7 @@ describe "DocumentManager", -> describe "flushDocIfLoaded", -> describe "when the doc is in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushedTime, @lastUpdatedAt, @lastUpdatedBy) @RedisManager.clearUnflushedTime = sinon.stub().callsArgWith(1, null) @PersistenceManager.setDoc = sinon.stub().yields() @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback @@ -82,7 +84,7 @@ describe "DocumentManager", -> it "should write the doc lines to the persistence layer", -> @PersistenceManager.setDoc - .calledWith(@project_id, @doc_id, @lines, @version, @ranges) + .calledWith(@project_id, @doc_id, @lines, @version, @ranges, @lastUpdatedAt, @lastUpdatedBy) .should.equal true it "should call the callback without error", -> @@ -324,7 +326,7 @@ describe "DocumentManager", -> it "should save the updated ranges", -> @RedisManager.updateDocument - .calledWith(@project_id, @doc_id, @lines, @version, [], @updated_ranges) + .calledWith(@project_id, @doc_id, @lines, @version, [], @updated_ranges, {}) .should.equal true it "should call the callback", -> @@ -378,7 +380,7 @@ describe "DocumentManager", -> it "should save the updated ranges", -> @RedisManager.updateDocument - .calledWith(@project_id, @doc_id, @lines, @version, [], @updated_ranges) + .calledWith(@project_id, @doc_id, @lines, @version, [], @updated_ranges, {}) .should.equal true it "should call the callback", -> diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee index 0f8ad59167..d1308ad899 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee @@ -24,6 +24,8 @@ describe "PersistenceManager", -> @callback = sinon.stub() @ranges = { comments: "mock", entries: "mock" } @pathname = '/a/b/c.tex' + @lastUpdatedAt = Date.now() + @lastUpdatedBy = 'last-author-id' @Settings.apis = web: url: @url = "www.example.com" @@ -133,7 +135,7 @@ describe "PersistenceManager", -> describe "with a successful response from the web api", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 200}) - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @lastUpdatedAt, @lastUpdatedBy, @callback) it "should call the web api", -> @request @@ -143,6 +145,8 @@ describe "PersistenceManager", -> lines: @lines version: @version ranges: @ranges + lastUpdatedAt: @lastUpdatedAt + lastUpdatedBy: @lastUpdatedBy method: "POST" auth: user: @user @@ -162,7 +166,7 @@ describe "PersistenceManager", -> describe "when request returns an error", -> beforeEach -> @request.callsArgWith(1, @error = new Error("oops"), null, null) - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @lastUpdatedAt, @lastUpdatedBy, @callback) it "should return the error", -> @callback.calledWith(@error).should.equal true @@ -173,7 +177,7 @@ describe "PersistenceManager", -> describe "when the request returns 404", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 404}, "") - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @lastUpdatedAt, @lastUpdatedBy, @callback) it "should return a NotFoundError", -> @callback.calledWith(new Errors.NotFoundError("not found")).should.equal true @@ -184,7 +188,7 @@ describe "PersistenceManager", -> describe "when the request returns an error status code", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 500}, "") - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @lastUpdatedAt, @lastUpdatedBy, @callback) it "should return an error", -> @callback.calledWith(new Error("web api error")).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 4f6c24720e..cdfdc45ac2 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -36,6 +36,8 @@ describe "RedisManager", -> projectHistoryId: ({doc_id}) -> "ProjectHistoryId:#{doc_id}" projectState: ({project_id}) -> "ProjectState:#{project_id}" unflushedTime: ({doc_id}) -> "UnflushedTime:#{doc_id}" + lastUpdatedBy: ({doc_id}) -> "lastUpdatedBy:#{doc_id}" + lastUpdatedAt: ({doc_id}) -> "lastUpdatedAt:#{doc_id}" history: key_schema: uncompressedHistoryOps: ({doc_id}) -> "UncompressedHistoryOps:#{doc_id}" @@ -116,6 +118,16 @@ describe "RedisManager", -> .calledWith("ProjectHistoryId:#{@doc_id}") .should.equal true + it "should get lastUpdatedAt", -> + @multi.get + .calledWith("lastUpdatedAt:#{@doc_id}") + .should.equal true + + it "should get lastUpdatedBy", -> + @multi.get + .calledWith("lastUpdatedBy:#{@doc_id}") + .should.equal true + it "should check if the document is in the DocsIn set", -> @rclient.sadd .calledWith("DocsIn:#{@project_id}") @@ -123,7 +135,7 @@ describe "RedisManager", -> it 'should return the document', -> @callback - .calledWithExactly(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushed_time) + .calledWithExactly(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushed_time, @lastUpdatedAt, @lastUpdatedBy) .should.equal true it 'should not log any errors', -> @@ -132,7 +144,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, null]) + @multi.exec = sinon.stub().callsArgWith(0, null, [null, null, null, null, null, null, null, null, null, null]) @rclient.sadd = sinon.stub().yields() @RedisManager.getDoc @project_id, @doc_id, @callback @@ -143,7 +155,7 @@ describe "RedisManager", -> it 'should return an empty result', -> @callback - .calledWithExactly(null, null, 0, {}, null, null, null) + .calledWithExactly(null, null, 0, {}, null, null, null, null, null) .should.equal true it 'should not log any errors', -> @@ -161,7 +173,7 @@ describe "RedisManager", -> it 'should return the document', -> @callback - .calledWithExactly(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushed_time) + .calledWithExactly(null, @lines, @version, @ranges, @pathname, @projectHistoryId, @unflushed_time, @lastUpdatedAt, @lastUpdatedBy) .should.equal true describe "with a corrupted document", -> @@ -329,6 +341,7 @@ describe "RedisManager", -> @version = 42 @hash = crypto.createHash('sha1').update(JSON.stringify(@lines),'utf8').digest('hex') @ranges = { comments: "mock", entries: "mock" } + @updateMeta = { user_id: 'last-author-fake-id' } @doc_update_list_length = sinon.stub() @project_update_list_length = sinon.stub() @@ -340,7 +353,7 @@ describe "RedisManager", -> @multi.del = sinon.stub() @multi.eval = sinon.stub() @multi.exec = sinon.stub().callsArgWith(0, null, - [@hash, null, null, null, null, null, null, @doc_update_list_length] + [@hash, null, null, null, null, null, null, @doc_update_list_length, null, null] ) @ProjectHistoryRedisManager.queueOps = sinon.stub().callsArgWith( @ops.length + 1, null, @project_update_list_length @@ -353,7 +366,7 @@ describe "RedisManager", -> describe "with project history enabled", -> beforeEach -> @settings.apis.project_history.enabled = true - @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @updateMeta, @callback it "should get the current doc version to check for consistency", -> @RedisManager.getDocVersion @@ -385,6 +398,16 @@ describe "RedisManager", -> .calledWith("UnflushedTime:#{@doc_id}", Date.now(), "NX") .should.equal true + it "should set the last updated time", -> + @multi.set + .calledWith("lastUpdatedAt:#{@doc_id}", Date.now()) + .should.equal true + + it "should set the last updater", -> + @multi.set + .calledWith("lastUpdatedBy:#{@doc_id}", 'last-author-fake-id') + .should.equal true + it "should push the doc op into the doc ops list", -> @multi.rpush .calledWith("DocOps:#{@doc_id}", JSON.stringify(@ops[0]), JSON.stringify(@ops[1])) @@ -423,7 +446,7 @@ describe "RedisManager", -> beforeEach -> @rclient.rpush = sinon.stub() @settings.apis.project_history.enabled = false - @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @updateMeta, @callback it "should not push the updates into the project history ops list", -> @rclient.rpush.called.should.equal false @@ -436,7 +459,7 @@ describe "RedisManager", -> describe "with an inconsistent version", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length - 1) - @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @updateMeta, @callback it "should not call multi.exec", -> @multi.exec.called.should.equal false @@ -450,7 +473,7 @@ describe "RedisManager", -> beforeEach -> @rclient.rpush = sinon.stub().callsArgWith(1, null, @project_update_list_length) @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version) - @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, [], @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, [], @ranges, @updateMeta, @callback it "should not try to enqueue doc updates", -> @multi.rpush @@ -470,7 +493,7 @@ describe "RedisManager", -> describe "with empty ranges", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) - @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, {}, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, {}, @updateMeta, @callback it "should not set the ranges", -> @multi.set @@ -487,7 +510,7 @@ describe "RedisManager", -> @badHash = "INVALID-HASH-VALUE" @multi.exec = sinon.stub().callsArgWith(0, null, [@badHash]) @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) - @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @updateMeta, @callback it 'should log a hash error', -> @logger.error.calledWith() @@ -501,7 +524,7 @@ describe "RedisManager", -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) @_stringify = JSON.stringify @JSON.stringify = () -> return '["bad bytes! \u0000 <- here"]' - @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @updateMeta, @callback afterEach -> @JSON.stringify = @_stringify @@ -516,7 +539,7 @@ describe "RedisManager", -> beforeEach -> @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) @RedisManager._serializeRanges = sinon.stub().yields(new Error("ranges are too large")) - @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @callback + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @updateMeta, @callback it 'should log an error', -> @logger.error.called.should.equal true @@ -524,6 +547,21 @@ describe "RedisManager", -> it "should call the callback with the error", -> @callback.calledWith(new Error("ranges are too large")).should.equal true + describe "without user id from meta", -> + beforeEach -> + @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) + @RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, {}, @callback + + it "should set the last updater to null", -> + @multi.del + .calledWith("lastUpdatedBy:#{@doc_id}") + .should.equal true + + it "should still set the last updated time", -> + @multi.set + .calledWith("lastUpdatedAt:#{@doc_id}", Date.now()) + .should.equal true + describe "putDocInMemory", -> beforeEach -> @multi.set = sinon.stub() @@ -681,6 +719,17 @@ describe "RedisManager", -> .calledWith("ProjectHistoryId:#{@doc_id}") .should.equal true + it "should delete lastUpdatedAt", -> + @multi.del + .calledWith("lastUpdatedAt:#{@doc_id}") + .should.equal true + + it "should delete lastUpdatedBy", -> + @multi.del + .calledWith("lastUpdatedBy:#{@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 0fdbbb9728..ac8d4c742c 100644 --- a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee @@ -159,7 +159,8 @@ describe "UpdateManager", -> describe "applyUpdate", -> beforeEach -> - @update = {op: [{p: 42, i: "foo"}]} + @updateMeta = { user_id: 'last-author-fake-id' } + @update = {op: [{p: 42, i: "foo"}], meta: @updateMeta} @updatedDocLines = ["updated", "lines"] @version = 34 @lines = ["original", "lines"] @@ -193,7 +194,7 @@ describe "UpdateManager", -> it "should save the document", -> @RedisManager.updateDocument - .calledWith(@project_id, @doc_id, @updatedDocLines, @version, @appliedOps, @updated_ranges) + .calledWith(@project_id, @doc_id, @updatedDocLines, @version, @appliedOps, @updated_ranges, @updateMeta) .should.equal true it "should add metadata to the ops" , ->