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

filter track-changes updates for projects using project-history

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

@@ -23,12 +23,14 @@ module.exports = DocumentManager =
 			return callback(error) if error?
 			if !lines? or !version?
 				logger.log {project_id, doc_id}, "doc not in redis so getting from persistence API"
-				PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) ->
+				PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId, projectHistoryType) ->
 					return callback(error) if error?
-					logger.log {project_id, doc_id, lines, version, pathname, projectHistoryId}, "got doc from persistence API"
+					logger.log {project_id, doc_id, lines, version, pathname, projectHistoryId, projectHistoryType}, "got doc from persistence API"
 					RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, pathname, projectHistoryId, (error) ->
 						return callback(error) if error?
-						callback null, lines, version, ranges, pathname, projectHistoryId, null, false
+						RedisManager.setHistoryType doc_id, projectHistoryType, (error) ->
+							return callback(error) if error?
+							callback null, lines, version, ranges, pathname, projectHistoryId, null, false
 			else
 				callback null, lines, version, ranges, pathname, projectHistoryId, unflushedTime, true
 

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

@@ -12,14 +12,21 @@ module.exports = HistoryManager =
 		if !Settings.apis?.trackchanges?
 			logger.warn { doc_id }, "track changes API is not configured, so not flushing"
 			return
-		metrics.inc 'history-flush', 1, { status: 'track-changes'}
-		url = "#{Settings.apis.trackchanges.url}/project/#{project_id}/doc/#{doc_id}/flush"
-		logger.log { project_id, doc_id, url }, "flushing doc in track changes api"
-		request.post url, (error, res, body)->
-			if error?
-				logger.error { error, doc_id, project_id}, "track changes doc to track changes api"
-			else if res.statusCode < 200 and res.statusCode >= 300
-				logger.error { doc_id, project_id }, "track changes api returned a failure status code: #{res.statusCode}"
+		RedisManager.getHistoryType doc_id, (err, projectHistoryType) ->
+			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"
+				logger.debug {doc_id, projectHistoryType}, "skipping track-changes flush"
+			else
+				metrics.inc 'history-flush', 1, { status: 'track-changes'}
+				url = "#{Settings.apis.trackchanges.url}/project/#{project_id}/doc/#{doc_id}/flush"
+				logger.log { project_id, doc_id, url, projectHistoryType }, "flushing doc in track changes api"
+				request.post url, (error, res, body)->
+					if error?
+						logger.error { error, doc_id, project_id}, "track changes doc to track changes api"
+					else if res.statusCode < 200 and res.statusCode >= 300
+						logger.error { doc_id, project_id }, "track changes api returned a failure status code: #{res.statusCode}"
 
 	# flush changes in the background
 	flushProjectChangesAsync: (project_id) ->
@@ -54,6 +61,7 @@ module.exports = HistoryManager =
 		if ops.length == 0
 			return callback()
 
+		# record updates for project history
 		if Settings.apis?.project_history?.enabled
 			if HistoryManager.shouldFlushHistoryOps(project_ops_length, ops.length, HistoryManager.FLUSH_PROJECT_EVERY_N_OPS)
 				# Do this in the background since it uses HTTP and so may be too
@@ -61,6 +69,13 @@ module.exports = HistoryManager =
 				logger.log { project_ops_length, project_id }, "flushing project history api"
 				HistoryManager.flushProjectChangesAsync project_id
 
+		# if the doc_ops_length is undefined it means the project is not using track-changes
+		# so we can bail out here
+		if typeof(doc_ops_length) is 'undefined'
+			logger.debug { project_id, doc_id}, "skipping flush to track-changes, only using project-history"
+			return callback()
+
+		# record updates for track-changes
 		HistoryRedisManager.recordDocHasHistoryOps project_id, doc_id, ops, (error) ->
 			return callback(error) if error?
 			if HistoryManager.shouldFlushHistoryOps(doc_ops_length, ops.length, HistoryManager.FLUSH_DOC_EVERY_N_OPS)

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

@@ -13,7 +13,7 @@ request = (require("requestretry")).defaults({
 MAX_HTTP_REQUEST_LENGTH = 5000 # 5 seconds
 
 module.exports = PersistenceManager =
-	getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname, projectHistoryId) ->) ->
+	getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges, pathname, projectHistoryId, projectHistoryType) ->) ->
 		timer = new Metrics.Timer("persistenceManager.getDoc")
 		callback = (args...) ->
 			timer.done()
@@ -44,7 +44,7 @@ module.exports = PersistenceManager =
 					return callback(new Error("web API response had no valid doc version"))
 				if !body.pathname?
 					return callback(new Error("web API response had no valid doc pathname"))
-				return callback null, body.lines, body.version, body.ranges, body.pathname, body.projectHistoryId
+				return callback null, body.lines, body.version, body.ranges, body.pathname, body.projectHistoryId, body.projectHistoryType
 			else if res.statusCode == 404
 				return callback(new Errors.NotFoundError("doc not not found: #{url}"))
 			else

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

@@ -78,6 +78,7 @@ module.exports = RedisManager =
 		multi.del keys.ranges(doc_id:doc_id)
 		multi.del keys.pathname(doc_id:doc_id)
 		multi.del keys.projectHistoryId(doc_id:doc_id)
+		multi.del keys.projectHistoryType(doc_id:doc_id)
 		multi.del keys.unflushedTime(doc_id:doc_id)
 		multi.del keys.lastUpdatedAt(doc_id: doc_id)
 		multi.del keys.lastUpdatedBy(doc_id: doc_id)
@@ -154,11 +155,11 @@ module.exports = RedisManager =
 					logger.error project_id: project_id, doc_id: doc_id, doc_project_id: doc_project_id, "doc missing from docsInProject set"
 				callback null, docLines, version, ranges, pathname, projectHistoryId, unflushedTime, lastUpdatedAt, lastUpdatedBy
 
-	getDocVersion: (doc_id, callback = (error, version) ->) ->
-		rclient.get keys.docVersion(doc_id: doc_id), (error, version) ->
+	getDocVersion: (doc_id, callback = (error, version, projectHistoryType) ->) ->
+		rclient.mget keys.docVersion(doc_id: doc_id), keys.projectHistoryType(doc_id:doc_id), (error, version, projectHistoryType) ->
 			return callback(error) if error?
 			version = parseInt(version, 10)
-			callback null, version
+			callback null, version, projectHistoryType
 
 	getDocLines: (doc_id, callback = (error, version) ->) ->
 		rclient.get keys.docLines(doc_id: doc_id), (error, docLines) ->
@@ -200,10 +201,18 @@ module.exports = RedisManager =
 						return callback(error)
 					callback null, ops
 
+	getHistoryType: (doc_id, callback = (error, projectHistoryType) ->) ->
+		rclient.get keys.projectHistoryType(doc_id:doc_id), (error, projectHistoryType) ->
+			return callback(error) if error?
+			callback null, projectHistoryType
+
+	setHistoryType: (doc_id, projectHistoryType, callback = (error) ->) ->
+		rclient.set keys.projectHistoryType(doc_id:doc_id), projectHistoryType, callback
+
 	DOC_OPS_TTL: 60 * minutes
 	DOC_OPS_MAX_LENGTH: 100
 	updateDocument : (project_id, doc_id, docLines, newVersion, appliedOps = [], ranges, updateMeta, callback = (error) ->)->
-		RedisManager.getDocVersion doc_id, (error, currentVersion) ->
+		RedisManager.getDocVersion doc_id, (error, currentVersion, projectHistoryType) ->
 			return callback(error) if error?
 			if currentVersion + appliedOps.length != newVersion
 				error = new Error("Version mismatch. '#{doc_id}' is corrupted.")
@@ -249,7 +258,11 @@ module.exports = RedisManager =
 					multi.rpush  keys.docOps(doc_id: doc_id), jsonOps...                         # index 5
 					# 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
-					multi.rpush  historyKeys.uncompressedHistoryOps(doc_id: doc_id), jsonOps...  # index 7
+					if projectHistoryType is "project-history"
+						logger.debug {doc_id}, "skipping push of uncompressed ops for project using project-history"
+					else
+						# project is using old track-changes history service
+						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
 					# valid up to this point). Otherwise leave it alone ("NX" flag).
@@ -262,8 +275,11 @@ module.exports = RedisManager =
 				multi.exec (error, result) ->
 						return callback(error) if error?
 
-						# length of uncompressedHistoryOps queue (index 7)
-						docUpdateCount = result[7]
+						if projectHistoryType is 'project-history'
+							docUpdateCount = undefined  # only using project history, don't bother with track-changes
+						else
+							# project is using old track-changes history service
+							docUpdateCount = result[7] # length of uncompressedHistoryOps queue (index 7)
 
 						if jsonOps.length > 0 && Settings.apis?.project_history?.enabled
 							ProjectHistoryRedisManager.queueOps project_id, jsonOps..., (error, projectUpdateCount) ->

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

@@ -70,13 +70,14 @@ module.exports =
 				unflushedTime: ({doc_id}) -> "UnflushedTime:{#{doc_id}}"
 				pathname: ({doc_id}) -> "Pathname:{#{doc_id}}"
 				projectHistoryId: ({doc_id}) -> "ProjectHistoryId:{#{doc_id}}"
+				projectHistoryType: ({doc_id}) -> "ProjectHistoryType:{#{doc_id}}"
 				projectState: ({project_id}) -> "ProjectState:{#{project_id}}"
 				pendingUpdates: ({doc_id}) -> "PendingUpdates:{#{doc_id}}"
 				lastUpdatedBy: ({doc_id}) -> "lastUpdatedBy:{#{doc_id}}"
 				lastUpdatedAt: ({doc_id}) -> "lastUpdatedAt:{#{doc_id}}"
 				pendingUpdates: ({doc_id}) -> "PendingUpdates:{#{doc_id}}"
 				flushAndDeleteQueue: () -> "DocUpdaterFlushAndDeleteQueue"
-	
+
 	max_doc_length: 2 * 1024 * 1024 # 2mb
 
 	dispatcherCount: process.env["DISPATCHER_COUNT"]
@@ -91,4 +92,4 @@ module.exports =
 
 	continuousBackgroundFlush: process.env['CONTINUOUS_BACKGROUND_FLUSH'] or false
 
-	smoothingOffset: process.env['SMOOTHING_OFFSET'] or 1000 # milliseconds
+	smoothingOffset: process.env['SMOOTHING_OFFSET'] or 1000 # milliseconds

services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee

@@ -27,6 +27,7 @@ describe "DocumentManager", ->
 			"./RangesManager": @RangesManager = {}
 		@project_id = "project-id-123"
 		@projectHistoryId = "history-id-123"
+		@projectHistoryType = "project-history"
 		@doc_id = "doc-id-123"
 		@user_id = 1234
 		@callback = sinon.stub()
@@ -178,8 +179,9 @@ describe "DocumentManager", ->
 		describe "when the doc does not exist in Redis", ->
 			beforeEach ->
 				@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null, null)
-				@PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId)
+				@PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @projectHistoryId, @projectHistoryType)
 				@RedisManager.putDocInMemory = sinon.stub().yields()
+				@RedisManager.setHistoryType = sinon.stub().yields()
 				@DocumentManager.getDoc @project_id, @doc_id, @callback
 
 			it "should try to get the doc from Redis", ->
@@ -197,6 +199,11 @@ describe "DocumentManager", ->
 					.calledWith(@project_id, @doc_id, @lines, @version, @ranges, @pathname, @projectHistoryId)
 					.should.equal true
 
+			it "should set the history type in Redis", ->
+				@RedisManager.setHistoryType
+					.calledWith(@doc_id, @projectHistoryType)
+					.should.equal true
+
 			it "should call the callback with the doc info", ->
 				@callback.calledWith(null, @lines, @version, @ranges, @pathname, @projectHistoryId, null, false).should.equal true
 

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

@@ -15,7 +15,7 @@ describe "HistoryManager", ->
 					trackchanges:
 						url: "http://trackchanges.example.com"
 			}
-			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
+			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), debug: sinon.stub() }
 			"./DocumentManager": @DocumentManager = {}
 			"./HistoryRedisManager": @HistoryRedisManager = {}
 			"./RedisManager": @RedisManager = {}
@@ -29,12 +29,25 @@ describe "HistoryManager", ->
 		beforeEach ->
 			@request.post = sinon.stub().callsArgWith(1, null, statusCode: 204)
 
-			@HistoryManager.flushDocChangesAsync @project_id, @doc_id
+		describe "when the project uses track changes", ->
+			beforeEach ->
+				@RedisManager.getHistoryType = sinon.stub().yields(null, 'track-changes')
+				@HistoryManager.flushDocChangesAsync @project_id, @doc_id
 
-		it "should send a request to the track changes api", ->
-			@request.post
-				.calledWith("#{@Settings.apis.trackchanges.url}/project/#{@project_id}/doc/#{@doc_id}/flush")
-				.should.equal true
+			it "should send a request to the track changes api", ->
+				@request.post
+					.calledWith("#{@Settings.apis.trackchanges.url}/project/#{@project_id}/doc/#{@doc_id}/flush")
+					.should.equal true
+
+		describe "when the project uses project history", ->
+			beforeEach ->
+				@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 ->

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

@@ -34,6 +34,7 @@ describe "RedisManager", ->
 								ranges: ({doc_id}) -> "Ranges:#{doc_id}"
 								pathname: ({doc_id}) -> "Pathname:#{doc_id}"
 								projectHistoryId: ({doc_id}) -> "ProjectHistoryId:#{doc_id}"
+								projectHistoryType: ({doc_id}) -> "ProjectHistoryType:#{doc_id}"
 								projectState: ({project_id}) -> "ProjectState:#{project_id}"
 								unflushedTime: ({doc_id}) -> "UnflushedTime:#{doc_id}"
 								lastUpdatedBy: ({doc_id}) -> "lastUpdatedBy:#{doc_id}"