Merge pull request #103 from overleaf/bg-filter-track-changes-updates

filter track changes updates

services/document-updater/app/coffee/HistoryManager.coffee

@@ -16,7 +16,7 @@ module.exports = HistoryManager =
 			if err?
 				logger.warn {err, doc_id}, "error getting history type"
 				# if there's an error continue and flush to track-changes for safety
-			if projectHistoryType is "project-history"
+			if Settings.disableDoubleFlush and projectHistoryType is "project-history"
 				logger.debug {doc_id, projectHistoryType}, "skipping track-changes flush"
 			else
 				metrics.inc 'history-flush', 1, { status: 'track-changes'}

services/document-updater/app/coffee/RedisManager.coffee

@@ -156,8 +156,9 @@ module.exports = RedisManager =
 				callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime, lastUpdatedAt, lastUpdatedBy
 
 	getDocVersion: (doc_id, callback = (error, version, projectHistoryType) ->) ->
-		rclient.mget keys.docVersion(doc_id: doc_id), keys.projectHistoryType(doc_id:doc_id), (error, version, projectHistoryType) ->
+		rclient.mget keys.docVersion(doc_id: doc_id), keys.projectHistoryType(doc_id:doc_id), (error, result) ->
 			return callback(error) if error?
+			[version, projectHistoryType] = result || []
 			version = parseInt(version, 10)
 			callback null, version, projectHistoryType
 
@@ -259,9 +260,11 @@ module.exports = RedisManager =
 					# expire must come after rpush since before it will be a no-op if the list is empty
 					multi.expire keys.docOps(doc_id: doc_id), RedisManager.DOC_OPS_TTL           # index 6
 					if projectHistoryType is "project-history"
-						logger.debug {doc_id}, "skipping push of uncompressed ops for project using project-history"
+						metrics.inc 'history-queue', 1, {status: 'skip-track-changes'}
+						logger.log {doc_id}, "skipping push of uncompressed ops for project using project-history"
 					else
 						# project is using old track-changes history service
+						metrics.inc 'history-queue', 1, {status: 'track-changes'}
 						multi.rpush  historyKeys.uncompressedHistoryOps(doc_id: doc_id), jsonOps...  # index 7
 					# Set the unflushed timestamp to the current time if the doc
 					# hasn't been modified before (the content in mongo has been
@@ -282,6 +285,7 @@ module.exports = RedisManager =
 							docUpdateCount = result[7] # length of uncompressedHistoryOps queue (index 7)
 
 						if jsonOps.length > 0 && Settings.apis?.project_history?.enabled
+							metrics.inc 'history-queue', 1, {status: 'project-history'}
 							ProjectHistoryRedisManager.queueOps project_id, jsonOps..., (error, projectUpdateCount) ->
 								callback null, docUpdateCount, projectUpdateCount
 						else

services/document-updater/config/settings.defaults.coffee

@@ -93,3 +93,5 @@ module.exports =
 	continuousBackgroundFlush: process.env['CONTINUOUS_BACKGROUND_FLUSH'] or false
 
 	smoothingOffset: process.env['SMOOTHING_OFFSET'] or 1000 # milliseconds
+
+	disableDoubleFlush: process.env['DISABLE_DOUBLE_FLUSH'] or false # don't flush track-changes for projects using project-history

services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee

@@ -135,6 +135,39 @@ describe "Applying updates to a doc", ->
 				done()
 			return null
 
+	describe "when the document is loaded and is using project-history only", ->
+		before (done) ->
+			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+
+			MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version, projectHistoryType: 'project-history'}
+			DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
+				throw error if error?
+				sinon.spy MockWebApi, "getDocument"
+				DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) ->
+					throw error if error?
+					setTimeout done, 200
+			return null
+
+		after ->
+			MockWebApi.getDocument.restore()
+
+		it "should update the doc", (done) ->
+			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
+				doc.lines.should.deep.equal @result
+				done()
+			return null
+
+		it "should not push any applied updates to the track changes api", (done) ->
+			rclient_history.lrange HistoryKeys.uncompressedHistoryOps({@doc_id}), 0, -1, (error, updates) =>
+				updates.length.should.equal 0
+				done()
+			return null
+
+		it "should push the applied updates to the project history changes api", (done) ->
+			rclient_history.lrange ProjectHistoryKeys.projectHistoryOps({@project_id}), 0, -1, (error, updates) =>
+				JSON.parse(updates[0]).op.should.deep.equal @update.op
+				done()
+			return null
 
 	describe "when the document has been deleted", ->
 		describe "when the ops come in a single linear order", ->

services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee

@@ -39,16 +39,28 @@ describe "HistoryManager", ->
 					.calledWith("#{@Settings.apis.trackchanges.url}/project/#{@project_id}/doc/#{@doc_id}/flush")
 					.should.equal true
 
-		describe "when the project uses project history", ->
+		describe "when the project uses project history and double flush is not disabled", ->
 			beforeEach ->
 				@RedisManager.getHistoryType = sinon.stub().yields(null, 'project-history')
 				@HistoryManager.flushDocChangesAsync @project_id, @doc_id
 
+			it "should send a request to the track changes api", ->
+				@request.post
+					.called
+					.should.equal true
+
+		describe "when the project uses project history and double flush is disabled", ->
+			beforeEach ->
+				@Settings.disableDoubleFlush = true
+				@RedisManager.getHistoryType = sinon.stub().yields(null, 'project-history')
+				@HistoryManager.flushDocChangesAsync @project_id, @doc_id
+
 			it "should not send a request to the track changes api", ->
 				@request.post
 					.called
 					.should.equal false
 
+
 	describe "flushProjectChangesAsync", ->
 		beforeEach ->
 			@request.post = sinon.stub().callsArgWith(1, null, statusCode: 204)

services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee

@@ -361,11 +361,12 @@ describe "RedisManager", ->
 
 		describe "with a consistent version", ->
 			beforeEach ->
-				@RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length)
+
 
 			describe "with project history enabled", ->
 				beforeEach ->
 					@settings.apis.project_history.enabled = true
+					@RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length)
 					@RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @updateMeta, @callback
 
 				it "should get the current doc version to check for consistency", ->
@@ -446,6 +447,7 @@ describe "RedisManager", ->
 				beforeEach ->
 					@rclient.rpush = sinon.stub()
 					@settings.apis.project_history.enabled = false
+					@RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length)
 					@RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @updateMeta, @callback
 
 				it "should not push the updates into the project history ops list", ->
@@ -456,6 +458,26 @@ describe "RedisManager", ->
 						.calledWith(null, @doc_update_list_length)
 						.should.equal true
 
+			describe "with a doc using project history only", ->
+				beforeEach ->
+					@RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length, 'project-history')
+					@RedisManager.updateDocument @project_id, @doc_id, @lines, @version, @ops, @ranges, @updateMeta, @callback
+
+				it "should not push the updates to the track-changes ops list", ->
+					@multi.rpush
+						.calledWith("UncompressedHistoryOps:#{@doc_id}")
+						.should.equal false
+
+				it "should push the updates into the project history ops list", ->
+					@ProjectHistoryRedisManager.queueOps
+						.calledWith(@project_id, JSON.stringify(@ops[0]))
+						.should.equal true
+
+				it "should call the callback with the project update count only", ->
+					@callback
+						.calledWith(null, undefined, @project_update_list_length)
+						.should.equal true
+
 		describe "with an inconsistent version", ->
 			beforeEach ->
 				@RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length - 1)
@@ -754,3 +776,23 @@ describe "RedisManager", ->
 				@ProjectHistoryRedisManager.queueRenameEntity
 					.calledWithExactly(@project_id, @projectHistoryId, 'doc', @doc_id, @userId, @update, @callback)
 					.should.equal true
+
+		describe "getDocVersion", ->
+			beforeEach ->
+				@version = 12345
+
+			describe "when the document does not have a project history type set", ->
+				beforeEach ->
+					@rclient.mget = sinon.stub().withArgs("DocVersion:#{@doc_id}", "ProjectHistoryType:#{@doc_id}").callsArgWith(2, null, ["#{@version}"])
+					@RedisManager.getDocVersion @doc_id, @callback
+
+				it "should return the document version and an undefined history type", ->
+					@callback.calledWithExactly(null, @version, undefined).should.equal true
+
+			describe "when the document has a project history type set", ->
+				beforeEach ->
+					@rclient.mget = sinon.stub().withArgs("DocVersion:#{@doc_id}", "ProjectHistoryType:#{@doc_id}").callsArgWith(2, null, ["#{@version}", 'project-history'])
+					@RedisManager.getDocVersion @doc_id, @callback
+
+				it "should return the document version and history type", ->
+					@callback.calledWithExactly(null, @version, 'project-history').should.equal true