From be2136de7c1e5a5676a9e402028c0a7847f2582f Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 3 Dec 2015 15:47:55 +0000
Subject: [PATCH] fix update-in-place bug for array ops

---
 .../app/coffee/UpdateCompressor.coffee        |  4 ++++
 .../app/coffee/UpdatesManager.coffee          |  3 ++-
 .../UpdateCompressorTests.coffee              | 22 +++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/services/track-changes/app/coffee/UpdateCompressor.coffee b/services/track-changes/app/coffee/UpdateCompressor.coffee
index 6d7527057f..95e3f58e86 100644
--- a/services/track-changes/app/coffee/UpdateCompressor.coffee
+++ b/services/track-changes/app/coffee/UpdateCompressor.coffee
@@ -56,6 +56,10 @@ module.exports = UpdateCompressor =
 		return concattedUpdates
 
 	compressRawUpdates: (lastPreviousUpdate, rawUpdates) ->
+		if lastPreviousUpdate?.op?.length > 1
+			# if the last previous update was an array op, don't compress onto it.
+			# The avoids cases where array length changes but version number doesn't
+			return [lastPreviousUpdate].concat UpdateCompressor.compressRawUpdates(null,rawUpdates)
 		if lastPreviousUpdate?
 			rawUpdates = [lastPreviousUpdate].concat(rawUpdates)
 		updates = UpdateCompressor.convertToSingleOpUpdates(rawUpdates)
diff --git a/services/track-changes/app/coffee/UpdatesManager.coffee b/services/track-changes/app/coffee/UpdatesManager.coffee
index 4edfdb0ad7..ffc2cef19a 100644
--- a/services/track-changes/app/coffee/UpdatesManager.coffee
+++ b/services/track-changes/app/coffee/UpdatesManager.coffee
@@ -7,6 +7,7 @@ UpdateTrimmer = require "./UpdateTrimmer"
 logger = require "logger-sharelatex"
 async = require "async"
 DocArchiveManager = require "./DocArchiveManager"
+_ = require "underscore"
 
 module.exports = UpdatesManager =
 	compressAndSaveRawUpdates: (project_id, doc_id, rawUpdates, temporary, callback = (error) ->) ->
@@ -50,7 +51,7 @@ module.exports = UpdatesManager =
 				# compress them together with the new ones
 				[firstUpdate, additionalUpdates...] = compressedUpdates
 
-				if firstUpdate.v == lastCompressedUpdate.v
+				if firstUpdate.v == lastCompressedUpdate.v and _.isEqual(firstUpdate, lastCompressedUpdate)
 					# first update version hasn't changed, skip it and insert remaining updates
 					# this is an optimisation, we could update the existing op with itself
 					updateToModify = null
diff --git a/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee b/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee
index 3d14a84eb9..be3bec5350 100644
--- a/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee
+++ b/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee
@@ -304,3 +304,25 @@ describe "UpdateCompressor", ->
 					meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id
 					v: 43
 				}]
+
+	describe "compressRawUpdates", ->
+		describe "merging in-place with an array op", ->
+			it "should not change the existing last updates", ->
+				expect(@UpdateCompressor.compressRawUpdates {
+					op: [ {"p":1000,"d":"hello"}, {"p":1000,"i":"HELLO()"} ]
+					meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id
+					v: 42
+				}, [{
+					op: [{ p: 1006, i: "WORLD" }]
+					meta: ts: @ts2, user_id: @user_id
+					v: 43
+				}])
+				.to.deep.equal [{
+					op: [{"p":1000,"d":"hello"}, {"p":1000,"i":"HELLO()"} ]
+					meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id
+					v: 42
+				},{
+					op: [{"p":1006,"i":"WORLD"}]
+					meta: start_ts: @ts2, end_ts: @ts2, user_id: @user_id
+					v: 43
+				}]