diff --git a/services/document-updater/app/coffee/RangesManager.coffee b/services/document-updater/app/coffee/RangesManager.coffee index 64f7059399..25da4ec9db 100644 --- a/services/document-updater/app/coffee/RangesManager.coffee +++ b/services/document-updater/app/coffee/RangesManager.coffee @@ -2,6 +2,9 @@ RangesTracker = require "./RangesTracker" logger = require "logger-sharelatex" module.exports = RangesManager = + MAX_COMMENTS: 500 + MAX_CHANGES: 500 + applyUpdate: (project_id, doc_id, entries = {}, updates = [], callback = (error, new_entries) ->) -> {changes, comments} = entries logger.log {changes, comments, updates}, "applying updates to ranges" @@ -12,6 +15,9 @@ module.exports = RangesManager = rangesTracker.setIdSeed(update.meta.tc) for op in update.op 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 logger.log {response}, "applied updates to ranges" diff --git a/services/document-updater/app/coffee/ShareJsUpdateManager.coffee b/services/document-updater/app/coffee/ShareJsUpdateManager.coffee index 876d56e71b..1d36776b9f 100644 --- a/services/document-updater/app/coffee/ShareJsUpdateManager.coffee +++ b/services/document-updater/app/coffee/ShareJsUpdateManager.coffee @@ -37,13 +37,10 @@ module.exports = ShareJsUpdateManager = update.dup = true ShareJsUpdateManager._sendOp(project_id, doc_id, update) else - ShareJsUpdateManager._sendError(project_id, doc_id, error) return callback(error) logger.log project_id: project_id, doc_id: doc_id, error: error, "applied update" 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/) callback(null, docLines, data.v, model.db.appliedOps[doc_key] or []) @@ -55,6 +52,3 @@ module.exports = ShareJsUpdateManager = _sendOp: (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} - diff --git a/services/document-updater/app/coffee/UpdateManager.coffee b/services/document-updater/app/coffee/UpdateManager.coffee index 1678c4d4c0..89f58bfd1f 100644 --- a/services/document-updater/app/coffee/UpdateManager.coffee +++ b/services/document-updater/app/coffee/UpdateManager.coffee @@ -46,7 +46,12 @@ module.exports = UpdateManager = (update, cb) -> UpdateManager.applyUpdate project_id, doc_id, update, cb 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 DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> return callback(error) if error? diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index 4166f8499e..bdfe89b990 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -176,8 +176,12 @@ describe "Applying updates to a doc", -> describe "with a broken update", -> before (done) -> [@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} - 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? setTimeout done, 200 @@ -185,6 +189,16 @@ describe "Applying updates to a doc", -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @lines 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", -> before (done) -> diff --git a/services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee index f3b871149d..42ba3f331b 100644 --- a/services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee @@ -70,34 +70,22 @@ describe "ShareJsUpdateManager", -> describe "when applyOp fails", -> beforeEach (done) -> @error = new Error("Something went wrong") - @ShareJsUpdateManager._sendError = sinon.stub() @model.applyOp = sinon.stub().callsArgWith(2, @error) @ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version) => @callback(err, docLines, version) 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", -> @callback.calledWith(@error).should.equal true describe "when getSnapshot fails", -> beforeEach (done) -> @error = new Error("Something went wrong") - @ShareJsUpdateManager._sendError = sinon.stub() @model.getSnapshot.callsArgWith(1, @error) @ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version) => @callback(err, docLines, version) 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", -> @callback.calledWith(@error).should.equal true @@ -125,14 +113,3 @@ describe "ShareJsUpdateManager", -> .calledWith({project_id: @project_id, doc_id: @doc_id, op: @opData}) .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 - diff --git a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee index e87391af44..33578cb6f0 100644 --- a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee @@ -165,6 +165,7 @@ describe "UpdateManager", -> @RangesManager.applyUpdate = sinon.stub().yields(null, @updated_ranges) @ShareJsUpdateManager.applyUpdate = sinon.stub().yields(null, @updatedDocLines, @version, @appliedOps) @RedisManager.updateDocument = sinon.stub().yields() + @WebRedisManager.sendData = sinon.stub() @HistoryManager.pushUncompressedHistoryOps = sinon.stub().callsArg(3) describe "normally", -> @@ -206,6 +207,25 @@ describe "UpdateManager", -> # \uFFFD is 'replacement character' @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", -> beforeEach ->