From d29141d583a197aa7c00104de0d326a6ad18d118 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 30 Mar 2017 11:48:26 +0100
Subject: [PATCH 1/2] return error when out-of-order ops detected

---
 .../track-changes/app/coffee/UpdatesManager.coffee |  9 +++++++++
 .../UpdatesManager/UpdatesManagerTests.coffee      | 14 ++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/services/track-changes/app/coffee/UpdatesManager.coffee b/services/track-changes/app/coffee/UpdatesManager.coffee
index fa4016f632..fa56f463cd 100644
--- a/services/track-changes/app/coffee/UpdatesManager.coffee
+++ b/services/track-changes/app/coffee/UpdatesManager.coffee
@@ -16,6 +16,15 @@ module.exports = UpdatesManager =
 		if length == 0
 			return callback()
 
+		# check that ops are in the correct order
+		for op, i in rawUpdates when i > 0
+			thisVersion = op?.v
+			prevVersion = rawUpdates[i-1]?.v
+			if not (prevVersion < thisVersion)
+				logger.error project_id: project_id, doc_id: doc_id, rawUpdates:rawUpdates, temporary: temporary, thisVersion:thisVersion, prevVersion:prevVersion, "op versions out of order"
+				# TODO try to recover by sorting the ops
+				return callback(new Error("incoming op versions out of order"))
+
 		# FIXME: we no longer need the lastCompressedUpdate, so change functions not to need it
 		# CORRECTION:  we do use it to log the time in case of error
 		MongoManager.peekLastCompressedUpdate doc_id, (error, lastCompressedUpdate, lastVersion) ->
diff --git a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee
index 4a7d463b37..dd0ba835d3 100644
--- a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee
+++ b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee
@@ -137,6 +137,20 @@ describe "UpdatesManager", ->
 				it "should not insert any update into mongo", ->
 					@PackManager.insertCompressedUpdates.called.should.equal false
 
+			describe "when the raw ops are out of order", ->
+				beforeEach ->
+					@rawUpdates = [{ v: 13, op: "mock-op-13" }, { v: 12, op: "mock-op-12" }]
+					@UpdatesManager.compressAndSaveRawUpdates @project_id, @doc_id, @rawUpdates, @temporary, @callback
+
+				it "should call the callback with an error", ->
+					@callback
+						.calledWith(new Error("incoming op versions out of order"))
+						.should.equal true
+
+				it "should not insert any update into mongo", ->
+					@PackManager.insertCompressedUpdates.called.should.equal false
+
+
 		describe "when the raw ops need appending to existing history which is in S3", ->
 			beforeEach ->
 				@lastCompressedUpdate = null

From 8bb3dd07a438276446b7616f94e6f208f63b1afe Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 30 Mar 2017 13:37:11 +0100
Subject: [PATCH 2/2] continue when ops are out of order

---
 services/track-changes/app/coffee/UpdatesManager.coffee         | 2 --
 .../test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee  | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/services/track-changes/app/coffee/UpdatesManager.coffee b/services/track-changes/app/coffee/UpdatesManager.coffee
index fa56f463cd..f01681e5f0 100644
--- a/services/track-changes/app/coffee/UpdatesManager.coffee
+++ b/services/track-changes/app/coffee/UpdatesManager.coffee
@@ -22,8 +22,6 @@ module.exports = UpdatesManager =
 			prevVersion = rawUpdates[i-1]?.v
 			if not (prevVersion < thisVersion)
 				logger.error project_id: project_id, doc_id: doc_id, rawUpdates:rawUpdates, temporary: temporary, thisVersion:thisVersion, prevVersion:prevVersion, "op versions out of order"
-				# TODO try to recover by sorting the ops
-				return callback(new Error("incoming op versions out of order"))
 
 		# FIXME: we no longer need the lastCompressedUpdate, so change functions not to need it
 		# CORRECTION:  we do use it to log the time in case of error
diff --git a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee
index dd0ba835d3..c422fffec0 100644
--- a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee
+++ b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee
@@ -144,7 +144,7 @@ describe "UpdatesManager", ->
 
 				it "should call the callback with an error", ->
 					@callback
-						.calledWith(new Error("incoming op versions out of order"))
+						.calledWith(new Error)
 						.should.equal true
 
 				it "should not insert any update into mongo", ->