diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 9ac651b5e7..be47ec4c8c 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -43,7 +43,7 @@ module.exports = DocumentManager = return callback(error) if error? callback null, lines, version, ops, ranges - setDoc: (project_id, doc_id, newLines, source, user_id, _callback = (error) ->) -> + setDoc: (project_id, doc_id, newLines, source, user_id, undoing, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.setDoc") callback = (args...) -> timer.done() @@ -63,6 +63,9 @@ module.exports = DocumentManager = logger.log doc_id: doc_id, project_id: project_id, oldLines: oldLines, newLines: newLines, "setting a document via http" DiffCodec.diffAsShareJsOp oldLines, newLines, (error, op) -> return callback(error) if error? + if undoing + for o in op or [] + o.u = true # Turn on undo flag for each op for track changes update = doc: doc_id op: op @@ -161,9 +164,9 @@ module.exports = DocumentManager = UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDocAndRecentOps, project_id, doc_id, fromVersion, callback - setDocWithLock: (project_id, doc_id, lines, source, user_id, callback = (error) ->) -> + setDocWithLock: (project_id, doc_id, lines, source, user_id, undoing, callback = (error) ->) -> UpdateManager = require "./UpdateManager" - UpdateManager.lockUpdatesAndDo DocumentManager.setDoc, project_id, doc_id, lines, source, user_id, callback + UpdateManager.lockUpdatesAndDo DocumentManager.setDoc, project_id, doc_id, lines, source, user_id, undoing, callback flushDocIfLoadedWithLock: (project_id, doc_id, callback = (error) ->) -> UpdateManager = require "./UpdateManager" diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index 8448361930..aae2b51f8e 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -40,16 +40,14 @@ module.exports = HttpController = setDoc: (req, res, next = (error) ->) -> doc_id = req.params.doc_id project_id = req.params.project_id - lines = req.body.lines - source = req.body.source - user_id = req.body.user_id + {lines, source, user_id, undoing} = req.body lineSize = HttpController._getTotalSizeOfLines(lines) if lineSize > TWO_MEGABYTES logger.log {project_id, doc_id, source, lineSize, user_id}, "document too large, returning 406 response" return res.send 406 - logger.log project_id: project_id, doc_id: doc_id, lines: lines, source: source, user_id: user_id, "setting doc via http" + logger.log {project_id, doc_id, lines, source, user_id, undoing}, "setting doc via http" timer = new Metrics.Timer("http.setDoc") - DocumentManager.setDocWithLock project_id, doc_id, lines, source, user_id, (error) -> + DocumentManager.setDocWithLock project_id, doc_id, lines, source, user_id, undoing, (error) -> timer.done() return next(error) if error? logger.log project_id: project_id, doc_id: doc_id, "set doc via http" diff --git a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee index 3232c6e219..1a5d790be8 100644 --- a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee @@ -41,7 +41,7 @@ describe "Setting a document", -> DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) => throw error if error? setTimeout () => - DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, (error, res, body) => + DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, false, (error, res, body) => @statusCode = res.statusCode done() , 200 @@ -74,7 +74,7 @@ describe "Setting a document", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} - DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, (error, res, body) => + DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, false, (error, res, body) => @statusCode = res.statusCode setTimeout done, 200 @@ -94,3 +94,60 @@ describe "Setting a document", -> throw error if error? expect(lines).to.not.exist done() + + describe "with track changes", -> + before -> + @lines = ["one", "one and a half", "two", "three"] + @id_seed = "587357bd35e64f6157" + @update = + doc: @doc_id + op: [{ + d: "one and a half\n" + p: 4 + }] + meta: + tc: @id_seed + user_id: @user_id + v: @version + + describe "with the undo flag", -> + before (done) -> + [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] + MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} + DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => + throw error if error? + DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) => + throw error if error? + # Go back to old lines, with undo flag + DocUpdaterClient.setDocLines @project_id, @doc_id, @lines, @source, @user_id, true, (error, res, body) => + @statusCode = res.statusCode + setTimeout done, 200 + + it "should undo the tracked changes", (done) -> + DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, data) => + throw error if error? + ranges = data.ranges + expect(ranges.changes).to.be.undefined + done() + + describe "without the undo flag", -> + before (done) -> + [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] + MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} + DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => + throw error if error? + DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) => + throw error if error? + # Go back to old lines, without undo flag + DocUpdaterClient.setDocLines @project_id, @doc_id, @lines, @source, @user_id, false, (error, res, body) => + @statusCode = res.statusCode + setTimeout done, 200 + + it "should not undo the tracked changes", (done) -> + DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, data) => + throw error if error? + ranges = data.ranges + expect(ranges.changes.length).to.equal 1 + done() + + diff --git a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee index d2e8dbe51d..7755b656f1 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee @@ -53,13 +53,14 @@ module.exports = DocUpdaterClient = request.post "http://localhost:3003/project/#{project_id}/doc/#{doc_id}/flush", (error, res, body) -> callback error, res, body - setDocLines: (project_id, doc_id, lines, source, user_id, callback = (error) ->) -> + setDocLines: (project_id, doc_id, lines, source, user_id, undoing, callback = (error) ->) -> request.post { url: "http://localhost:3003/project/#{project_id}/doc/#{doc_id}" json: lines: lines source: source user_id: user_id + undoing: undoing }, (error, res, body) -> callback error, res, body diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index b7ea49ffc9..47fbde021b 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -194,6 +194,7 @@ describe "DocumentManager", -> beforeEach -> @beforeLines = ["before", "lines"] @afterLines = ["after", "lines"] + @ops = [{ i: "foo", p: 4 }, { d: "bar", p: 42 }] @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @ranges, true) @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null) @@ -202,7 +203,7 @@ describe "DocumentManager", -> describe "when already loaded", -> beforeEach -> - @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback + @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, false, @callback it "should get the current doc lines", -> @DocumentManager.getDoc @@ -246,7 +247,7 @@ describe "DocumentManager", -> describe "when not already loaded", -> beforeEach -> @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, false) - @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback + @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, false, @callback it "should flush and delete the doc from the doc updater", -> @DocumentManager.flushAndDeleteDoc @@ -255,13 +256,24 @@ describe "DocumentManager", -> describe "without new lines", -> beforeEach -> - @DocumentManager.setDoc @project_id, @doc_id, null, @source, @user_id, @callback + @DocumentManager.setDoc @project_id, @doc_id, null, @source, @user_id, false, @callback it "should return the callback with an error", -> @callback.calledWith(new Error("No lines were passed to setDoc")) it "should not try to get the doc lines", -> @DocumentManager.getDoc.called.should.equal false + + describe "with the undoing flag", -> + beforeEach -> + # Copy ops so we don't interfere with other tests + @ops = [{ i: "foo", p: 4 }, { d: "bar", p: 42 }] + @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) + @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, true, @callback + + it "should set the undo flag on each op", -> + for op in @ops + op.u.should.equal true describe "acceptChanges", -> beforeEach -> diff --git a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee index 859a0d1089..69b40c85d2 100644 --- a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee +++ b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee @@ -125,15 +125,16 @@ describe "HttpController", -> lines: @lines source: @source user_id: @user_id + undoing: @undoing = true describe "successfully", -> beforeEach -> - @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5) + @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(6) @HttpController.setDoc(@req, @res, @next) it "should set the doc", -> @DocumentManager.setDocWithLock - .calledWith(@project_id, @doc_id, @lines, @source, @user_id) + .calledWith(@project_id, @doc_id, @lines, @source, @user_id, @undoing) .should.equal true it "should return a successful No Content response", -> @@ -143,7 +144,7 @@ describe "HttpController", -> it "should log the request", -> @logger.log - .calledWith(doc_id: @doc_id, project_id: @project_id, lines: @lines, source: @source, user_id: @user_id, "setting doc via http") + .calledWith(doc_id: @doc_id, project_id: @project_id, lines: @lines, source: @source, user_id: @user_id, undoing: @undoing, "setting doc via http") .should.equal true it "should time the request", -> @@ -151,7 +152,7 @@ describe "HttpController", -> describe "when an errors occurs", -> beforeEach -> - @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5, new Error("oops")) + @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(6, new Error("oops")) @HttpController.setDoc(@req, @res, @next) it "should call next with the error", -> @@ -165,7 +166,7 @@ describe "HttpController", -> for _ in [0..200000] lines.push "test test test" @req.body.lines = lines - @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5) + @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(6) @HttpController.setDoc(@req, @res, @next) it 'should send back a 406 response', ->