support forcing new compressed update in popLastCompressedUpdate

callback with a null update, passing the version as an additional
argument

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

@@ -20,13 +20,24 @@ module.exports = MongoManager =
 	deleteCompressedUpdate: (id, callback = (error) ->) ->
 		db.docHistory.remove({ _id: ObjectId(id.toString()) }, callback)
 
-	popLastCompressedUpdate: (doc_id, callback = (error, update) ->) ->
+	popLastCompressedUpdate: (doc_id, callback = (error, update, version) ->) ->
+		# under normal use we pass back the last update as
+		# callback(null,update).
+		#
+		# when we have an existing last update but want to force a new one
+		# to start, we pass it back as callback(null,null,version), just
+		# giving the version so we can check consistency.
 		MongoManager.getLastCompressedUpdate doc_id, (error, update) ->
 			return callback(error) if error?
 			if update?
-				MongoManager.deleteCompressedUpdate update._id, (error) ->
-					return callback(error) if error?
-					callback null, update
+				if update.inS3?
+					# we want to force a new update, but ensure that it is
+					# consistent with the version of the existing one in S3
+					return callback null, null, update.v
+				else
+					MongoManager.deleteCompressedUpdate update._id, (error) ->
+						return callback(error) if error?
+						callback null, update
 			else
 				callback null, null
 

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

@@ -7,7 +7,6 @@ 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) ->) ->
@@ -15,27 +14,29 @@ module.exports = UpdatesManager =
 		if length == 0
 			return callback()
 
-		MongoManager.popLastCompressedUpdate doc_id, (error, lastCompressedUpdate) ->
+		MongoManager.popLastCompressedUpdate doc_id, (error, lastCompressedUpdate, lastVersion = lastCompressedUpdate?.v) ->
+			# lastCompressedUpdate may be null if we are forcing the start
+			# of a new compressed update, in which case we have the
+			# lastVersion to check consistency (defaults to lastCompressedUpdate.v)
 			return callback(error) if error?
 
-			# Ensure that raw updates start where lastCompressedUpdate left off
-			if lastCompressedUpdate?
+			# Ensure that raw updates start where lastVersion left off
+			if lastVersion?
 				rawUpdates = rawUpdates.slice(0)
-				while rawUpdates[0]? and rawUpdates[0].v <= lastCompressedUpdate.v
+				while rawUpdates[0]? and rawUpdates[0].v <= lastVersion
 					rawUpdates.shift()
 
-				if rawUpdates[0]? and rawUpdates[0].v != lastCompressedUpdate.v + 1
-					error = new Error("Tried to apply raw op at version #{rawUpdates[0].v} to last compressed update with version #{lastCompressedUpdate.v}")
+				if rawUpdates[0]? and rawUpdates[0].v != lastVersion + 1
+					error = new Error("Tried to apply raw op at version #{rawUpdates[0].v} to last compressed update with version #{lastVersion}")
 					logger.error err: error, doc_id: doc_id, project_id: project_id, "inconsistent doc versions"
-					# Push the update back into Mongo - catching errors at this
+					# Push the update back into Mongo - if it was present - catching errors at this
 					# point is useless, we're already bailing
-					MongoManager.insertCompressedUpdates project_id, doc_id, [lastCompressedUpdate], temporary, () ->
-						return callback error
+					if lastCompressedUpdate?
+						MongoManager.insertCompressedUpdates project_id, doc_id, [lastCompressedUpdate], temporary, () ->
+							return callback error
 					return
 
 			compressedUpdates = UpdateCompressor.compressRawUpdates lastCompressedUpdate, rawUpdates
-			if lastCompressedUpdate?.inS3? and not _.some(compressedUpdates, (update) -> update.inS3)
-				compressedUpdates[compressedUpdates.length-1].inS3 = lastCompressedUpdate.inS3
 
 			MongoManager.insertCompressedUpdates project_id, doc_id, compressedUpdates, temporary,(error) ->
 				return callback(error) if error?

services/track-changes/test/unit/coffee/MongoManager/MongoManagerTests.coffee

@@ -104,6 +104,25 @@ describe "MongoManager", ->
 			it "should call the callback with the update", ->
 				@callback.calledWith(null, @update).should.equal true
 
+		describe "when there is a last update in S3", ->
+			beforeEach ->
+				@update = { _id: Object(), v: 12345, inS3: true }
+				@MongoManager.getLastCompressedUpdate = sinon.stub().callsArgWith(1, null, @update)
+				@MongoManager.deleteCompressedUpdate = sinon.stub()
+				@MongoManager.popLastCompressedUpdate @doc_id, @callback
+
+			it "should get the last update", ->
+				@MongoManager.getLastCompressedUpdate
+					.calledWith(@doc_id)
+					.should.equal true
+
+			it "should not try to delete the last update", ->
+				@MongoManager.deleteCompressedUpdate.called.should.equal false
+
+			it "should call the callback with a null update and the correct version", ->
+				@callback.calledWith(null, null, @update.v).should.equal true
+
+
 	describe "insertCompressedUpdate", ->
 		beforeEach ->
 			@update = { op: "op", meta: "meta", v: "v"}
@@ -483,4 +502,4 @@ describe "MongoManager", ->
 				.should.equal true
 
 		it "should call the callback", ->
-			@callback.called.should.equal true
+			@callback.called.should.equal true

services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee

@@ -122,6 +122,39 @@ describe "UpdatesManager", ->
 						.calledWith(@project_id, @doc_id, [@lastCompressedUpdate], @temporary)
 						.should.equal true
 
+		describe "when the raw ops need appending to existing history which is in S3", ->
+			beforeEach ->
+				@lastCompressedUpdate = null
+				@lastVersion = 11
+				@compressedUpdates = { v: 13, op: "compressed-op-12" }
+
+				@MongoManager.popLastCompressedUpdate = sinon.stub().callsArgWith(1, null, null, @lastVersion)
+				@MongoManager.insertCompressedUpdates = sinon.stub().callsArg(4)
+				@UpdateCompressor.compressRawUpdates = sinon.stub().returns(@compressedUpdates)
+
+			describe "when the raw ops start where the existing history ends", ->
+				beforeEach ->
+					@rawUpdates = [{ v: 12, op: "mock-op-12" }, { v: 13, op: "mock-op-13" }]
+					@UpdatesManager.compressAndSaveRawUpdates @project_id, @doc_id, @rawUpdates, @temporary, @callback
+
+				it "should try to pop the last compressed op", ->
+					@MongoManager.popLastCompressedUpdate
+						.calledWith(@doc_id)
+						.should.equal true
+				
+				it "should compress the last compressed op and the raw ops", ->
+					@UpdateCompressor.compressRawUpdates
+						.calledWith(@lastCompressedUpdate, @rawUpdates)
+						.should.equal true
+				
+				it "should save the compressed ops", ->
+					@MongoManager.insertCompressedUpdates
+						.calledWith(@project_id, @doc_id, @compressedUpdates, @temporary)
+						.should.equal true
+
+				it "should call the callback", ->
+					@callback.called.should.equal true
+
 	describe "processUncompressedUpdates", ->
 		beforeEach ->
 			@UpdatesManager.compressAndSaveRawUpdates = sinon.stub().callsArgWith(4)