Don't try to redis rpush with no arguments

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) ->

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

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
+

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) ->

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