diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 8063b20ddd..87e31b7826 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -115,7 +115,8 @@ module.exports = RedisManager = multi = rclient.multi() multi.set keys.docLines(doc_id:doc_id), JSON.stringify(docLines) multi.set keys.docVersion(doc_id:doc_id), newVersion - multi.rpush keys.docOps(doc_id: doc_id), jsonOps... # TODO: Really double check that these are going onto the array in the correct order + if jsonOps.length > 0 + multi.rpush keys.docOps(doc_id: doc_id), jsonOps... multi.expire keys.docOps(doc_id: doc_id), RedisManager.DOC_OPS_TTL multi.ltrim keys.docOps(doc_id: doc_id), -RedisManager.DOC_OPS_MAX_LENGTH, -1 multi.exec (error, replys) -> diff --git a/services/document-updater/app/coffee/WebRedisManager.coffee b/services/document-updater/app/coffee/WebRedisManager.coffee index 85f301752f..85a056f961 100644 --- a/services/document-updater/app/coffee/WebRedisManager.coffee +++ b/services/document-updater/app/coffee/WebRedisManager.coffee @@ -23,6 +23,8 @@ module.exports = WebRedisManager = rclient.llen "PendingUpdates:#{doc_id}", callback pushUncompressedHistoryOps: (project_id, doc_id, ops = [], callback = (error, length) ->) -> + if ops.length == 0 + return callback(null, 0) jsonOps = ops.map (op) -> JSON.stringify op async.parallel [ (cb) -> rclient.rpush "UncompressedHistoryOps:#{doc_id}", jsonOps..., cb diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index 94463f4ad6..89f1acbcfe 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -1,6 +1,7 @@ sinon = require "sinon" chai = require("chai") chai.should() +expect = chai.expect async = require "async" rclient = require("redis").createClient() {db, ObjectId} = require "../../../app/js/mongojs" @@ -251,3 +252,55 @@ describe "Applying updates to a doc", -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @result done() + + describe "when the sending duplicate ops", -> + before (done) -> + [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert { + doc_id: ObjectId(@doc_id) + version: @version + }, (error) => + throw error if error? + # One user delete 'one', the next turns it into 'once'. The second becomes a NOP. + DocUpdaterClient.sendUpdate @project_id, @doc_id, { + doc: @doc_id + op: [{ + i: "one and a half\n" + p: 4 + }] + v: @version + meta: + source: "ikHceq3yfAdQYzBo4-xZ" + }, (error) => + throw error if error? + setTimeout () => + DocUpdaterClient.sendUpdate @project_id, @doc_id, { + doc: @doc_id + op: [{ + i: "one and a half\n" + p: 4 + }] + v: @version + dupIfSource: ["ikHceq3yfAdQYzBo4-xZ"] + meta: + source: "ikHceq3yfAdQYzBo4-xZ" + }, (error) => + throw error if error? + setTimeout done, 200 + , 200 + + DocUpdaterClient.subscribeToAppliedOps @messageCallback = sinon.stub() + + it "should update the doc", (done) -> + DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => + doc.lines.should.deep.equal @result + done() + + it "should return a message about duplicate ops", -> + @messageCallback.calledTwice.should.equal true + @messageCallback.args[0][0].should.equal "applied-ops" + expect(JSON.parse(@messageCallback.args[0][1]).op.dup).to.be.undefined + @messageCallback.args[1][0].should.equal "applied-ops" + expect(JSON.parse(@messageCallback.args[1][1]).op.dup).to.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 5ee3398e87..205692d634 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -219,6 +219,21 @@ describe "RedisManager", -> @callback .calledWith(new Error("Version mismatch. '#{@doc_id}' is corrupted.")) .should.equal true + + describe "with no updates", -> + beforeEach -> + @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version) + @RedisManager.updateDocument @doc_id, @lines, @version, [], @callback + + it "should not do an rpush", -> + @rclient.rpush + .called + .should.equal false + + it "should still set the doclines", -> + @rclient.set + .calledWith("doclines:#{@doc_id}", JSON.stringify(@lines)) + .should.equal true describe "putDocInMemory", -> beforeEach (done) -> diff --git a/services/document-updater/test/unit/coffee/WebRedisManager/WebRedisManagerTests.coffee b/services/document-updater/test/unit/coffee/WebRedisManager/WebRedisManagerTests.coffee index cd0ce7e9fe..107fdaee53 100644 --- a/services/document-updater/test/unit/coffee/WebRedisManager/WebRedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/WebRedisManager/WebRedisManagerTests.coffee @@ -71,23 +71,45 @@ describe "WebRedisManager", -> @callback.calledWith(null, @length).should.equal true describe "pushUncompressedHistoryOps", -> - beforeEach (done) -> + beforeEach -> @ops = [{ op: [{ i: "foo", p: 4 }] },{ op: [{ i: "bar", p: 56 }] }] @rclient.rpush = sinon.stub().yields(null, @length = 42) @rclient.sadd = sinon.stub().yields() - @WebRedisManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, (args...) => - @callback(args...) - done() - it "should push the doc op into the doc ops list as JSON", -> - @rclient.rpush - .calledWith("UncompressedHistoryOps:#{@doc_id}", JSON.stringify(@ops[0]), JSON.stringify(@ops[1])) - .should.equal true + describe "with ops", -> + beforeEach (done) -> + @WebRedisManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, (args...) => + @callback(args...) + done() + + it "should push the doc op into the doc ops list as JSON", -> + @rclient.rpush + .calledWith("UncompressedHistoryOps:#{@doc_id}", JSON.stringify(@ops[0]), JSON.stringify(@ops[1])) + .should.equal true - it "should add the doc_id to the set of which records the project docs", -> - @rclient.sadd - .calledWith("DocsWithHistoryOps:#{@project_id}", @doc_id) - .should.equal true + it "should add the doc_id to the set of which records the project docs", -> + @rclient.sadd + .calledWith("DocsWithHistoryOps:#{@project_id}", @doc_id) + .should.equal true - it "should call the callback with the length", -> - @callback.calledWith(null, @length).should.equal true + it "should call the callback with the length", -> + @callback.calledWith(null, @length).should.equal true + + describe "with no ops", -> + beforeEach (done) -> + @WebRedisManager.pushUncompressedHistoryOps @project_id, @doc_id, [], (args...) => + @callback(args...) + done() + + it "should not push the doc op into the doc ops list as JSON", -> + @rclient.rpush + .called + .should.equal false + + it "should not add the doc_id to the set of which records the project docs", -> + @rclient.sadd + .called + .should.equal false + + it "should call the callback with the length", -> + @callback.calledWith(null, 0).should.equal true