فهرست منبع

Add max limit on number of comments and changes per doc

James Allen 9 سال پیش
والد
کامیت
0706feb26b

+ 6 - 0
services/document-updater/app/coffee/RangesManager.coffee

@@ -2,6 +2,9 @@ RangesTracker = require "./RangesTracker"
 logger = require "logger-sharelatex"
 logger = require "logger-sharelatex"
 
 
 module.exports = RangesManager =
 module.exports = RangesManager =
+	MAX_COMMENTS: 500
+	MAX_CHANGES: 500
+
 	applyUpdate: (project_id, doc_id, entries = {}, updates = [], callback = (error, new_entries) ->) ->
 	applyUpdate: (project_id, doc_id, entries = {}, updates = [], callback = (error, new_entries) ->) ->
 		{changes, comments} = entries
 		{changes, comments} = entries
 		logger.log {changes, comments, updates}, "applying updates to ranges"
 		logger.log {changes, comments, updates}, "applying updates to ranges"
@@ -12,6 +15,9 @@ module.exports = RangesManager =
 				rangesTracker.setIdSeed(update.meta.tc)
 				rangesTracker.setIdSeed(update.meta.tc)
 			for op in update.op
 			for op in update.op
 				rangesTracker.applyOp(op, { user_id: update.meta?.user_id })
 				rangesTracker.applyOp(op, { user_id: update.meta?.user_id })
+		
+		if rangesTracker.changes?.length > RangesManager.MAX_CHANGES or rangesTracker.comments?.length > RangesManager.MAX_COMMENTS
+			return callback new Error("too many comments or tracked changes")
 
 
 		response = RangesManager._getRanges rangesTracker
 		response = RangesManager._getRanges rangesTracker
 		logger.log {response}, "applied updates to ranges"
 		logger.log {response}, "applied updates to ranges"

+ 1 - 7
services/document-updater/app/coffee/ShareJsUpdateManager.coffee

@@ -37,13 +37,10 @@ module.exports = ShareJsUpdateManager =
 					update.dup = true
 					update.dup = true
 					ShareJsUpdateManager._sendOp(project_id, doc_id, update)
 					ShareJsUpdateManager._sendOp(project_id, doc_id, update)
 				else
 				else
-					ShareJsUpdateManager._sendError(project_id, doc_id, error)
 					return callback(error)
 					return callback(error)
 			logger.log project_id: project_id, doc_id: doc_id, error: error, "applied update"
 			logger.log project_id: project_id, doc_id: doc_id, error: error, "applied update"
 			model.getSnapshot doc_key, (error, data) =>
 			model.getSnapshot doc_key, (error, data) =>
-				if error?
-					ShareJsUpdateManager._sendError(project_id, doc_id, error)
-					return callback(error)
+				return callback(error) if error?
 				docLines = data.snapshot.split(/\r\n|\n|\r/)
 				docLines = data.snapshot.split(/\r\n|\n|\r/)
 				callback(null, docLines, data.v, model.db.appliedOps[doc_key] or [])
 				callback(null, docLines, data.v, model.db.appliedOps[doc_key] or [])
 
 
@@ -55,6 +52,3 @@ module.exports = ShareJsUpdateManager =
 	_sendOp: (project_id, doc_id, op) ->
 	_sendOp: (project_id, doc_id, op) ->
 		WebRedisManager.sendData {project_id, doc_id, op}
 		WebRedisManager.sendData {project_id, doc_id, op}
 
 
-	_sendError: (project_id, doc_id, error) ->
-		WebRedisManager.sendData {project_id, doc_id, error: error.message || error}
-		

+ 6 - 1
services/document-updater/app/coffee/UpdateManager.coffee

@@ -46,7 +46,12 @@ module.exports = UpdateManager =
 				(update, cb) -> UpdateManager.applyUpdate project_id, doc_id, update, cb
 				(update, cb) -> UpdateManager.applyUpdate project_id, doc_id, update, cb
 				callback
 				callback
 
 
-	applyUpdate: (project_id, doc_id, update, callback = (error) ->) ->
+	applyUpdate: (project_id, doc_id, update, _callback = (error) ->) ->
+		callback = (error) ->
+			if error?
+				WebRedisManager.sendData {project_id, doc_id, error: error.message || error}
+			_callback(error)
+		
 		UpdateManager._sanitizeUpdate update
 		UpdateManager._sanitizeUpdate update
 		DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) ->
 		DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) ->
 			return callback(error) if error?
 			return callback(error) if error?

+ 15 - 1
services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee

@@ -176,8 +176,12 @@ describe "Applying updates to a doc", ->
 	describe "with a broken update", ->
 	describe "with a broken update", ->
 		before (done) ->
 		before (done) ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+			@broken_update = { doc_id: @doc_id, v: @version, op: [d: "not the correct content", p: 0 ] }
 			MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version}
 			MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version}
-			DocUpdaterClient.sendUpdate @project_id, @doc_id, @undefined, (error) ->
+	
+			DocUpdaterClient.subscribeToAppliedOps @messageCallback = sinon.stub()
+
+			DocUpdaterClient.sendUpdate @project_id, @doc_id, @broken_update, (error) ->
 				throw error if error?
 				throw error if error?
 				setTimeout done, 200
 				setTimeout done, 200
 
 
@@ -185,6 +189,16 @@ describe "Applying updates to a doc", ->
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
 				doc.lines.should.deep.equal @lines
 				doc.lines.should.deep.equal @lines
 				done()
 				done()
+		
+		it "should send a message with an error", ->
+			@messageCallback.called.should.equal true
+			[channel, message] = @messageCallback.args[0]
+			channel.should.equal "applied-ops"
+			JSON.parse(message).should.deep.equal {
+				project_id: @project_id,
+				doc_id: @doc_id, 
+				error:'Delete component \'not the correct content\' does not match deleted text \'one\ntwo\nthree\''
+			}
 
 
 	describe "with enough updates to flush to the track changes api", ->
 	describe "with enough updates to flush to the track changes api", ->
 		before (done) ->
 		before (done) ->

+ 0 - 23
services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee

@@ -70,34 +70,22 @@ describe "ShareJsUpdateManager", ->
 		describe "when applyOp fails", ->
 		describe "when applyOp fails", ->
 			beforeEach (done) ->
 			beforeEach (done) ->
 				@error = new Error("Something went wrong")
 				@error = new Error("Something went wrong")
-				@ShareJsUpdateManager._sendError = sinon.stub()
 				@model.applyOp = sinon.stub().callsArgWith(2, @error)
 				@model.applyOp = sinon.stub().callsArgWith(2, @error)
 				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version) =>
 				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version) =>
 					@callback(err, docLines, version)
 					@callback(err, docLines, version)
 					done()
 					done()
 
 
-			it "should call sendError with the error", ->
-				@ShareJsUpdateManager._sendError
-					.calledWith(@project_id, @doc_id, @error)
-					.should.equal true
-
 			it "should call the callback with the error", ->
 			it "should call the callback with the error", ->
 				@callback.calledWith(@error).should.equal true
 				@callback.calledWith(@error).should.equal true
 
 
 		describe "when getSnapshot fails", ->
 		describe "when getSnapshot fails", ->
 			beforeEach (done) ->
 			beforeEach (done) ->
 				@error = new Error("Something went wrong")
 				@error = new Error("Something went wrong")
-				@ShareJsUpdateManager._sendError = sinon.stub()
 				@model.getSnapshot.callsArgWith(1, @error)
 				@model.getSnapshot.callsArgWith(1, @error)
 				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version) =>
 				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version) =>
 					@callback(err, docLines, version)
 					@callback(err, docLines, version)
 					done()
 					done()
 
 
-			it "should call sendError with the error", ->
-				@ShareJsUpdateManager._sendError
-					.calledWith(@project_id, @doc_id, @error)
-					.should.equal true
-
 			it "should call the callback with the error", ->
 			it "should call the callback with the error", ->
 				@callback.calledWith(@error).should.equal true
 				@callback.calledWith(@error).should.equal true
 
 
@@ -125,14 +113,3 @@ describe "ShareJsUpdateManager", ->
 					.calledWith({project_id: @project_id, doc_id: @doc_id, op: @opData})
 					.calledWith({project_id: @project_id, doc_id: @doc_id, op: @opData})
 					.should.equal true
 					.should.equal true
 
 
-	describe "_sendError", ->
-		beforeEach ->
-			@error_text = "Something went wrong"
-			@WebRedisManager.sendData = sinon.stub()
-			@ShareJsUpdateManager._sendError(@project_id, @doc_id, new Error(@error_text))
-
-		it "should publish the error to the redis stream", ->
-			@WebRedisManager.sendData
-				.calledWith({project_id: @project_id, doc_id: @doc_id, error: @error_text})
-				.should.equal true
-

+ 20 - 0
services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee

@@ -165,6 +165,7 @@ describe "UpdateManager", ->
 			@RangesManager.applyUpdate = sinon.stub().yields(null, @updated_ranges)
 			@RangesManager.applyUpdate = sinon.stub().yields(null, @updated_ranges)
 			@ShareJsUpdateManager.applyUpdate = sinon.stub().yields(null, @updatedDocLines, @version, @appliedOps)
 			@ShareJsUpdateManager.applyUpdate = sinon.stub().yields(null, @updatedDocLines, @version, @appliedOps)
 			@RedisManager.updateDocument = sinon.stub().yields()
 			@RedisManager.updateDocument = sinon.stub().yields()
+			@WebRedisManager.sendData = sinon.stub()
 			@HistoryManager.pushUncompressedHistoryOps = sinon.stub().callsArg(3)
 			@HistoryManager.pushUncompressedHistoryOps = sinon.stub().callsArg(3)
 		
 		
 		describe "normally", ->
 		describe "normally", ->
@@ -206,6 +207,25 @@ describe "UpdateManager", ->
 				
 				
 				# \uFFFD is 'replacement character'
 				# \uFFFD is 'replacement character'
 				@update.op[0].i.should.equal "\uFFFD\uFFFD"
 				@update.op[0].i.should.equal "\uFFFD\uFFFD"
+		
+		describe "with an error", ->
+			beforeEach ->
+				@error = new Error("something went wrong")
+				@ShareJsUpdateManager.applyUpdate = sinon.stub().yields(@error)
+				@UpdateManager.applyUpdate @project_id, @doc_id, @update, @callback
+			
+			it "should call WebRedisManager.sendData with the error", ->
+				@WebRedisManager.sendData
+					.calledWith({
+						project_id: @project_id,
+						doc_id: @doc_id,
+						error: @error.message
+					})
+					.should.equal true
+
+			it "should call the callback with the error", ->
+				@callback.calledWith(@error).should.equal true
+			
 
 
 	describe "lockUpdatesAndDo", ->
 	describe "lockUpdatesAndDo", ->
 		beforeEach ->
 		beforeEach ->