Put multiple ops in one update

services/track-changes/app/coffee/DiffGenerator.coffee

@@ -9,7 +9,11 @@ module.exports = DiffGenerator =
 	ConsistencyError: ConsistencyError
 
 	rewindUpdate: (content, update) ->
-		op = update.op
+		for op in update.op.slice().reverse()
+			content = DiffGenerator.rewindOp content, op
+		return content
+
+	rewindOp: (content, op) ->
 		if op.i?
 			textToBeRemoved = content.slice(op.p, op.p + op.i.length)
 			if op.i != textToBeRemoved
@@ -33,9 +37,8 @@ module.exports = DiffGenerator =
 			diff = DiffGenerator.applyUpdateToDiff diff, update
 		return diff
 
-	applyUpdateToDiff: (diff, update) ->
+	applyOpToDiff: (diff, op, meta) ->
 		position = 0
-		op = update.op
 
 		remainingDiff = diff.slice()
 		{consumedDiff, remainingDiff} = DiffGenerator._consumeToOffset(remainingDiff, op.p)
@@ -44,15 +47,19 @@ module.exports = DiffGenerator =
 		if op.i?
 			newDiff.push
 				i: op.i
-				meta: update.meta
+				meta: meta
 		else if op.d?
-			{consumedDiff, remainingDiff} = DiffGenerator._consumeDiffAffectedByDeleteUpdate remainingDiff, update
+			{consumedDiff, remainingDiff} = DiffGenerator._consumeDiffAffectedByDeleteOp remainingDiff, op, meta
 			newDiff.push(consumedDiff...)
 
 		newDiff.push(remainingDiff...)
 
 		return newDiff
 
+	applyUpdateToDiff: (diff, update) ->
+		for op in update.op
+			diff = DiffGenerator.applyOpToDiff diff, op, update.meta
+		return diff
 
 	_consumeToOffset: (remainingDiff, totalOffset) ->
 		consumedDiff = []
@@ -76,25 +83,24 @@ module.exports = DiffGenerator =
 				consumedDiff.push part
 		throw new Error("Ran out of diff to consume. Offset is too small")
 
-	_consumeDiffAffectedByDeleteUpdate: (remainingDiff, deleteUpdate) ->
+	_consumeDiffAffectedByDeleteOp: (remainingDiff, deleteOp, meta) ->
 		consumedDiff = []
-		remainingUpdate = deleteUpdate
-		while remainingUpdate
-			{newPart, remainingDiff, remainingUpdate} = DiffGenerator._consumeDeletedPart remainingDiff, remainingUpdate
+		remainingOp = deleteOp
+		while remainingOp
+			{newPart, remainingDiff, remainingOp} = DiffGenerator._consumeDeletedPart remainingDiff, remainingOp, meta
 			consumedDiff.push newPart if newPart?
 		return {
 			consumedDiff: consumedDiff
 			remainingDiff: remainingDiff
 		}
 
-	_consumeDeletedPart: (remainingDiff, deleteUpdate) ->
+	_consumeDeletedPart: (remainingDiff, op, meta) ->
 		part = remainingDiff.shift()
 		partLength = DiffGenerator._getLengthOfDiffPart part
-		op = deleteUpdate.op
 
 		if part.d?
 			# Skip existing deletes
-			remainingUpdate = deleteUpdate
+			remainingOp = op
 			newPart = part
 
 		else if partLength > op.d.length
@@ -109,11 +115,11 @@ module.exports = DiffGenerator =
 			if part.u?
 				newPart =
 					d: op.d
-					meta: deleteUpdate.meta
+					meta: meta
 			else if part.i?
 				newPart = null
 
-			remainingUpdate = null
+			remainingOp = null
 
 		else if partLength == op.d.length
 			# The entire part has been deleted, but it is the last part
@@ -125,11 +131,11 @@ module.exports = DiffGenerator =
 			if part.u?
 				newPart =
 					d: op.d
-					meta: deleteUpdate.meta
+					meta: meta
 			else if part.i?
 				newPart = null
 
-			remainingUpdate = null
+			remainingOp = null
 
 		else if partLength < op.d.length
 			# The entire part has been deleted and there is more
@@ -142,18 +148,17 @@ module.exports = DiffGenerator =
 			if part.u
 				newPart =
 					d: part.u
-					meta: deleteUpdate.meta
+					meta: meta
 			else if part.i?
 				newPart = null
 
-			remainingUpdate =
-				op: { p: op.p, d: op.d.slice(DiffGenerator._getLengthOfDiffPart(part)) }
-				meta: deleteUpdate.meta
+			remainingOp =
+				p: op.p, d: op.d.slice(DiffGenerator._getLengthOfDiffPart(part))
 
 		return {
 			newPart: newPart
 			remainingDiff: remainingDiff
-			remainingUpdate: remainingUpdate
+			remainingOp: remainingOp
 		}
 
 	_slicePart: (basePart, from, to) ->

services/track-changes/app/coffee/UpdateCompressor.coffee

@@ -16,24 +16,38 @@ module.exports = UpdateCompressor =
 	# 	op: op2
 	# 	meta: { start_ts: ... , end_ts: ..., user_id: ... }
 	# }]
-	convertRawUpdatesToCompressedFormat: (updates) ->
-		normalizedUpdates = []
+	convertToSingleOpUpdates: (updates) ->
+		splitUpdates = []
 		for update in updates
 			for op in update.op
-				normalizedUpdates.push
+				splitUpdates.push
 					op: op
 					meta:
 						start_ts: update.meta.start_ts or update.meta.ts
 						end_ts:   update.meta.end_ts   or update.meta.ts
 						user_id:  update.meta.user_id
 					v: update.v
-		return normalizedUpdates
+		return splitUpdates
+
+	concatUpdatesWithSameVersion: (updates) ->
+		concattedUpdates = []
+		for update in updates
+			lastUpdate = concattedUpdates[concattedUpdates.length - 1]
+			if lastUpdate? and lastUpdate.v == update.v
+				lastUpdate.op.push update.op
+			else
+				concattedUpdates.push
+					op:   [ update.op ]
+					meta: update.meta
+					v:    update.v
+		return concattedUpdates
 
 	compressRawUpdates: (lastPreviousUpdate, rawUpdates) ->
-		updates = UpdateCompressor.convertRawUpdatesToCompressedFormat(rawUpdates)
 		if lastPreviousUpdate?
-			updates.unshift(lastPreviousUpdate)
-		return UpdateCompressor.compressUpdates(updates)
+			rawUpdates = [lastPreviousUpdate].concat(rawUpdates)
+		updates = UpdateCompressor.convertToSingleOpUpdates(rawUpdates)
+		updates = UpdateCompressor.compressUpdates(updates)
+		return UpdateCompressor.concatUpdatesWithSameVersion(updates)
 
 	compressUpdates: (updates) ->
 		return [] if updates.length == 0

services/track-changes/test/acceptance/coffee/AppendingUpdatesTests.coffee

@@ -34,9 +34,9 @@ describe "Appending doc ops to the history", ->
 					done()
 
 		it "should insert the compressed op into mongo", ->
-			expect(@updates[0].op).to.deep.equal {
+			expect(@updates[0].op).to.deep.equal [{
 				p: 3, i: "foo"
-			}
+			}]
 
 		it "should insert the correct version number into mongo", ->
 			expect(@updates[0].v).to.equal 5
@@ -84,9 +84,9 @@ describe "Appending doc ops to the history", ->
 						done()
 
 			it "should combine all the updates into one", ->
-				expect(@updates[0].op).to.deep.equal {
+				expect(@updates[0].op).to.deep.equal [{
 					p: 3, i: "foobar"
-				}
+				}]
 
 			it "should insert the correct version number into mongo", ->
 				expect(@updates[0].v).to.equal 8
@@ -114,26 +114,26 @@ describe "Appending doc ops to the history", ->
 						done()
 
 			it "should keep the updates separate", ->
-				expect(@updates[0].op).to.deep.equal {
+				expect(@updates[0].op).to.deep.equal [{
 					p: 3, i: "foo"
-				}
-				expect(@updates[1].op).to.deep.equal {
+				}]
+				expect(@updates[1].op).to.deep.equal [{
 					p: 6, i: "bar"
-				}
+				}]
 
 	describe "when the updates need processing in batches", ->
 		before (done) ->
 			@doc_id = ObjectId().toString()
 			@user_id = ObjectId().toString()
 			updates = []
-			@expectedOp = { p:0, i: "" }
+			@expectedOp = [{ p:0, i: "" }]
 			for i in [0..250]
 				updates.push {
 					op: [{i: "a", p: 0}]
 					meta: { ts: Date.now(), user_id: @user_id }
 					v: i
 				}
-				@expectedOp.i = "a" + @expectedOp.i
+				@expectedOp[0].i = "a" + @expectedOp[0].i
 
 			TrackChangesClient.pushRawUpdates @doc_id, updates, (error) =>
 				throw error if error?
@@ -147,3 +147,35 @@ describe "Appending doc ops to the history", ->
 		it "should insert the correct version number into mongo", ->
 			expect(@updates[0].v).to.equal 250
 
+
+	describe "when there are multiple ops in each update", ->
+		before (done) ->
+			@doc_id = ObjectId().toString()
+			@user_id = ObjectId().toString()
+			oneDay = 24 * 60 * 60 * 1000
+			TrackChangesClient.pushRawUpdates @doc_id, [{
+				op: [{ i: "f", p: 3 }, { i: "o", p: 4 }, { i: "o", p: 5 }]
+				meta: { ts: Date.now(), user_id: @user_id }
+				v: 3
+			}, {
+				op: [{ i: "b", p: 6 }, { i: "a", p: 7 }, { i: "r", p: 8 }]
+				meta: { ts: Date.now() + oneDay, user_id: @user_id }
+				v: 4
+			}], (error) =>
+				throw error if error?
+				TrackChangesClient.flushAndGetCompressedUpdates @doc_id, (error, @updates) =>
+					throw error if error?
+					done()
+
+		it "should insert the compressed ops into mongo", ->
+			expect(@updates[0].op).to.deep.equal [{
+				p: 3, i: "foo"
+			}]
+			expect(@updates[1].op).to.deep.equal [{
+				p: 6, i: "bar"
+			}]
+
+		it "should insert the correct version numbers into mongo", ->
+			expect(@updates[0].v).to.equal 3
+			expect(@updates[1].v).to.equal 4
+

services/track-changes/test/unit/coffee/DiffGenerator/DiffGeneratorTests.coffee

@@ -14,38 +14,40 @@ describe "DiffGenerator", ->
 			start_ts: @ts, end_ts: @ts, user_id: @user_id
 		}
 
-	describe "rewindUpdate", ->
+	describe "rewindOp", ->
 		describe "rewinding an insert", ->
 			it "should undo the insert", ->
 				content = "hello world"
-				update =
-					op: { p: 6, i: "wo" }
-				rewoundContent = @DiffGenerator.rewindUpdate content, update
+				rewoundContent = @DiffGenerator.rewindOp content, { p: 6, i: "wo" }
 				rewoundContent.should.equal "hello rld"
 
 		describe "rewinding a delete", ->
 			it "should undo the delete", ->
 				content = "hello rld"
-				update =
-					op: { p: 6, d: "wo" }
-				rewoundContent = @DiffGenerator.rewindUpdate content, update
+				rewoundContent = @DiffGenerator.rewindOp content, { p: 6, d: "wo" }
 				rewoundContent.should.equal "hello world"
 
 		describe "with an inconsistent update", ->
 			it "should throw an error", ->
 				content = "hello world"
-				update =
-					op: { p: 6, i: "foo" }
 				expect( () =>
-					@DiffGenerator.rewindUpdate content, update
+					@DiffGenerator.rewindOp content, { p: 6, i: "foo" }
 				).to.throw(@DiffGenerator.ConsistencyError)
 
+	describe "rewindUpdate", ->
+		it "should rewind ops in reverse", ->
+			content = "aaabbbccc"
+			update =
+				op: [{ p: 3, i: "bbb" }, { p: 6, i: "ccc" }]
+			rewoundContent = @DiffGenerator.rewindUpdate content, update
+			rewoundContent.should.equal "aaa"
+
 	describe "rewindUpdates", ->
 		it "should rewind updates in reverse", ->
 			content = "aaabbbccc"
 			updates = [
-				{ op: { p: 3, i: "bbb" } },
-				{ op: { p: 6, i: "ccc" } }
+				{ op: [{ p: 3, i: "bbb" }] },
+				{ op: [{ p: 6, i: "ccc" }] }
 			]
 			rewoundContent = @DiffGenerator.rewindUpdates content, updates
 			rewoundContent.should.equal "aaa"
@@ -83,7 +85,7 @@ describe "DiffGenerator", ->
 			it "should insert into the middle of (u)nchanged text", ->
 				diff = @DiffGenerator.applyUpdateToDiff(
 					[ { u: "foobar" } ],
-					{ op: { p: 3, i: "baz" }, meta: @meta }
+					{ op: [{ p: 3, i: "baz" }], meta: @meta }
 				)
 				expect(diff).to.deep.equal([
 					{ u: "foo" }
@@ -94,7 +96,7 @@ describe "DiffGenerator", ->
 			it "should insert into the start of (u)changed text", ->
 				diff = @DiffGenerator.applyUpdateToDiff(
 					[ { u: "foobar" } ],
-					{ op: { p: 0, i: "baz" }, meta: @meta }
+					{ op: [{ p: 0, i: "baz" }], meta: @meta }
 				)
 				expect(diff).to.deep.equal([
 					{ i: "baz", meta: @meta }
@@ -104,7 +106,7 @@ describe "DiffGenerator", ->
 			it "should insert into the end of (u)changed text", ->
 				diff = @DiffGenerator.applyUpdateToDiff(
 					[ { u: "foobar" } ],
-					{ op: { p: 6, i: "baz" }, meta: @meta }
+					{ op: [{ p: 6, i: "baz" }], meta: @meta }
 				)
 				expect(diff).to.deep.equal([
 					{ u: "foobar" }
@@ -114,7 +116,7 @@ describe "DiffGenerator", ->
 			it "should insert into the middle of (i)inserted text", ->
 				diff = @DiffGenerator.applyUpdateToDiff(
 					[ { i: "foobar", meta: @meta } ],
-					{ op: { p: 3, i: "baz" }, meta: @meta }
+					{ op: [{ p: 3, i: "baz" }], meta: @meta }
 				)
 				expect(diff).to.deep.equal([
 					{ i: "foo", meta: @meta }
@@ -128,7 +130,7 @@ describe "DiffGenerator", ->
 						{ d: "deleted", meta: @meta }
 						{ u: "foobar" }
 					],
-					{ op: { p: 3, i: "baz" }, meta: @meta }
+					{ op: [{ p: 3, i: "baz" }], meta: @meta }
 				)
 				expect(diff).to.deep.equal([
 					{ d: "deleted", meta: @meta }
@@ -142,7 +144,7 @@ describe "DiffGenerator", ->
 				it "should delete from the middle of (u)nchanged text", ->
 					diff = @DiffGenerator.applyUpdateToDiff(
 						[ { u: "foobazbar" } ],
-						{ op: { p: 3, d: "baz" }, meta: @meta }
+						{ op: [{ p: 3, d: "baz" }], meta: @meta }
 					)
 					expect(diff).to.deep.equal([
 						{ u: "foo" }
@@ -153,7 +155,7 @@ describe "DiffGenerator", ->
 				it "should delete from the start of (u)nchanged text", ->
 					diff = @DiffGenerator.applyUpdateToDiff(
 						[ { u: "foobazbar" } ],
-						{ op: { p: 0, d: "foo" }, meta: @meta }
+						{ op: [{ p: 0, d: "foo" }], meta: @meta }
 					)
 					expect(diff).to.deep.equal([
 						{ d: "foo", meta: @meta }
@@ -163,7 +165,7 @@ describe "DiffGenerator", ->
 				it "should delete from the end of (u)nchanged text", ->
 					diff = @DiffGenerator.applyUpdateToDiff(
 						[ { u: "foobazbar" } ],
-						{ op: { p: 6, d: "bar" }, meta: @meta }
+						{ op: [{ p: 6, d: "bar" }], meta: @meta }
 					)
 					expect(diff).to.deep.equal([
 						{ u: "foobaz" }
@@ -173,7 +175,7 @@ describe "DiffGenerator", ->
 				it "should delete across multiple (u)changed text parts", ->
 					diff = @DiffGenerator.applyUpdateToDiff(
 						[ { u: "foo" }, { u: "baz" }, { u: "bar" } ],
-						{ op: { p: 2, d: "obazb" }, meta: @meta }
+						{ op: [{ p: 2, d: "obazb" }], meta: @meta }
 					)
 					expect(diff).to.deep.equal([
 						{ u: "fo" }
@@ -187,7 +189,7 @@ describe "DiffGenerator", ->
 				it "should delete from the middle of (i)nserted text", ->
 					diff = @DiffGenerator.applyUpdateToDiff(
 						[ { i: "foobazbar", meta: @meta } ],
-						{ op: { p: 3, d: "baz" }, meta: @meta }
+						{ op: [{ p: 3, d: "baz" }], meta: @meta }
 					)
 					expect(diff).to.deep.equal([
 						{ i: "foo", meta: @meta }
@@ -197,7 +199,7 @@ describe "DiffGenerator", ->
 				it "should delete from the start of (u)nchanged text", ->
 					diff = @DiffGenerator.applyUpdateToDiff(
 						[ { i: "foobazbar", meta: @meta } ],
-						{ op: { p: 0, d: "foo" }, meta: @meta }
+						{ op: [{ p: 0, d: "foo" }], meta: @meta }
 					)
 					expect(diff).to.deep.equal([
 						{ i: "bazbar", meta: @meta }
@@ -206,7 +208,7 @@ describe "DiffGenerator", ->
 				it "should delete from the end of (u)nchanged text", ->
 					diff = @DiffGenerator.applyUpdateToDiff(
 						[ { i: "foobazbar", meta: @meta } ],
-						{ op: { p: 6, d: "bar" }, meta: @meta }
+						{ op: [{ p: 6, d: "bar" }], meta: @meta }
 					)
 					expect(diff).to.deep.equal([
 						{ i: "foobaz", meta: @meta }
@@ -215,7 +217,7 @@ describe "DiffGenerator", ->
 				it "should delete across multiple (u)changed and (i)nserted text parts", ->
 					diff = @DiffGenerator.applyUpdateToDiff(
 						[ { u: "foo" }, { i: "baz", meta: @meta }, { u: "bar" } ],
-						{ op: { p: 2, d: "obazb" }, meta: @meta }
+						{ op: [{ p: 2, d: "obazb" }], meta: @meta }
 					)
 					expect(diff).to.deep.equal([
 						{ u: "fo" }
@@ -228,7 +230,7 @@ describe "DiffGenerator", ->
 				it "should delete across multiple (u)changed and (d)deleted text parts", ->
 					diff = @DiffGenerator.applyUpdateToDiff(
 						[ { u: "foo" }, { d: "baz", meta: @meta }, { u: "bar" } ],
-						{ op: { p: 2, d: "ob" }, meta: @meta }
+						{ op: [{ p: 2, d: "ob" }], meta: @meta }
 					)
 					expect(diff).to.deep.equal([
 						{ u: "fo" }
@@ -243,7 +245,7 @@ describe "DiffGenerator", ->
 					expect(
 						() => @DiffGenerator.applyUpdateToDiff(
 							[ { u: "foobazbar" } ],
-							{ op: { p: 3, d: "xxx" }, meta: @meta }
+							{ op: [{ p: 3, d: "xxx" }], meta: @meta }
 						)
 					).to.throw(@DiffGenerator.ConsistencyError)
 
@@ -251,7 +253,7 @@ describe "DiffGenerator", ->
 					expect(
 						() => @DiffGenerator.applyUpdateToDiff(
 							[ { u: "foobazbar" } ],
-							{ op: { p: 0, d: "xxx" }, meta: @meta }
+							{ op: [{ p: 0, d: "xxx" }], meta: @meta }
 						)
 					).to.throw(@DiffGenerator.ConsistencyError)
 
@@ -259,7 +261,7 @@ describe "DiffGenerator", ->
 					expect(
 						() => @DiffGenerator.applyUpdateToDiff(
 							[ { u: "foobazbar" } ],
-							{ op: { p: 6, d: "xxx" }, meta: @meta }
+							{ op: [{ p: 6, d: "xxx" }] , meta: @meta }
 						)
 					).to.throw(@DiffGenerator.ConsistencyError)
 

services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee

@@ -13,9 +13,9 @@ describe "UpdateCompressor", ->
 		@ts1 = Date.now()
 		@ts2 = Date.now() + 1000
 
-	describe "convertRawUpdatesToCompressedFormat", ->
+	describe "convertToSingleOpUpdates", ->
 		it "should split grouped updates into individual updates", ->
-			expect(@UpdateCompressor.convertRawUpdatesToCompressedFormat [{
+			expect(@UpdateCompressor.convertToSingleOpUpdates [{
 				op:   [ @op1 = { p: 0, i: "Foo" }, @op2 = { p: 6, i: "bar"} ]
 				meta: { ts: @ts1, user_id: @user_id }
 				v: 42
@@ -38,6 +38,31 @@ describe "UpdateCompressor", ->
 				v: 43
 			}]
 
+	describe "concatUpdatesWithSameVersion", ->
+		it "should concat updates with the same version", ->
+			expect(@UpdateCompressor.concatUpdatesWithSameVersion [{
+				op:   @op1 = { p: 0, i: "Foo" }
+				meta: { start_ts: @ts1, end_ts: @ts1, user_id: @user_id }
+				v: 42
+			}, {
+				op:   @op2 = { p: 6, i: "bar" }
+				meta: { start_ts: @ts1, end_ts: @ts1, user_id: @user_id }
+				v: 42
+			}, {
+				op:   @op3 = { p: 10, i: "baz" }
+				meta: { start_ts: @ts2, end_ts: @ts2, user_id: @other_user_id }
+				v: 43
+			}])
+			.to.deep.equal [{
+				op:   [ @op1, @op2 ]
+				meta: { start_ts: @ts1, end_ts: @ts1, user_id: @user_id }
+				v: 42
+			}, {
+				op:   [ @op3 ]
+				meta: {  start_ts: @ts2, end_ts: @ts2, user_id: @other_user_id }
+				v: 43
+			}]
+
 	describe "compress", ->
 		describe "insert - insert", ->
 			it "should append one insert to the other", ->