diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index 004b9f77bc..20170cdd34 100644 --- a/services/document-updater/app.coffee +++ b/services/document-updater/app.coffee @@ -45,6 +45,7 @@ app.post '/project/:project_id/doc/:doc_id/flush', HttpController.flushDocIfLo app.delete '/project/:project_id/doc/:doc_id', HttpController.flushAndDeleteDoc app.delete '/project/:project_id', HttpController.deleteProject app.post '/project/:project_id/flush', HttpController.flushProject +app.post '/project/:project_id/doc/:doc_id/change/:change_id/accept', HttpController.acceptChange app.get '/total', (req, res)-> timer = new Metrics.Timer("http.allDocList") diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index c6d4773036..4f02bbcf5c 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -5,6 +5,8 @@ logger = require "logger-sharelatex" Metrics = require "./Metrics" HistoryManager = require "./HistoryManager" WebRedisManager = require "./WebRedisManager" +Errors = require "./Errors" +RangesManager = require "./RangesManager" module.exports = DocumentManager = getDoc: (project_id, doc_id, _callback = (error, lines, version, alreadyLoaded) ->) -> @@ -83,7 +85,6 @@ module.exports = DocumentManager = DocumentManager.flushAndDeleteDoc project_id, doc_id, (error) -> return callback(error) if error? callback null - flushDocIfLoaded: (project_id, doc_id, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.flushDocIfLoaded") @@ -119,6 +120,22 @@ module.exports = DocumentManager = RedisManager.removeDocFromMemory project_id, doc_id, (error) -> return callback(error) if error? callback null + + acceptChange: (project_id, doc_id, change_id, _callback = (error) ->) -> + timer = new Metrics.Timer("docManager.acceptChange") + callback = (args...) -> + timer.done() + _callback(args...) + + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> + return callback(error) if error? + if !lines? or !version? + return callback(new Errors.NotFoundError("document not found: #{doc_id}")) + RangesManager.acceptChange change_id, ranges, (error, new_ranges) -> + return callback(error) if error? + RedisManager.updateDocument doc_id, lines, version, [], new_ranges, (error) -> + return callback(error) if error? + callback() getDocWithLock: (project_id, doc_id, callback = (error, lines, version) ->) -> UpdateManager = require "./UpdateManager" @@ -139,3 +156,7 @@ module.exports = DocumentManager = flushAndDeleteDocWithLock: (project_id, doc_id, callback = (error) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.flushAndDeleteDoc, project_id, doc_id, callback + + acceptChangeWithLock: (project_id, doc_id, change_id, callback = (error) ->) -> + UpdateManager = require "./UpdateManager" + UpdateManager.lockUpdatesAndDo DocumentManager.acceptChange, project_id, doc_id, change_id, callback diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index e138fe8bc4..683b94230f 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -97,4 +97,15 @@ module.exports = HttpController = return next(error) if error? logger.log project_id: project_id, "deleted project via http" res.send 204 # No Content + + acceptChange: (req, res, next = (error) ->) -> + {project_id, doc_id, change_id} = req.params + logger.log {project_id, doc_id, change_id}, "accepting change via http" + timer = new Metrics.Timer("http.acceptChange") + DocumentManager.acceptChangeWithLock project_id, doc_id, change_id, (error) -> + timer.done() + return next(error) if error? + logger.log {project_id, doc_id, change_id}, "accepted change via http" + res.send 204 # No Content + diff --git a/services/document-updater/app/coffee/RangesManager.coffee b/services/document-updater/app/coffee/RangesManager.coffee index ac1dcbe75f..64f7059399 100644 --- a/services/document-updater/app/coffee/RangesManager.coffee +++ b/services/document-updater/app/coffee/RangesManager.coffee @@ -4,7 +4,7 @@ logger = require "logger-sharelatex" module.exports = RangesManager = applyUpdate: (project_id, doc_id, entries = {}, updates = [], callback = (error, new_entries) ->) -> {changes, comments} = entries - logger.log {changes, comments, updates}, "appliyng updates to ranges" + logger.log {changes, comments, updates}, "applying updates to ranges" rangesTracker = new RangesTracker(changes, comments) for update in updates rangesTracker.track_changes = !!update.meta.tc @@ -12,7 +12,20 @@ module.exports = RangesManager = rangesTracker.setIdSeed(update.meta.tc) for op in update.op rangesTracker.applyOp(op, { user_id: update.meta?.user_id }) - + + response = RangesManager._getRanges rangesTracker + logger.log {response}, "applied updates to ranges" + callback null, response + + acceptChange: (change_id, ranges, callback = (error, ranges) ->) -> + {changes, comments} = ranges + logger.log {changes, comments, change_id}, "accepting change in ranges" + rangesTracker = new RangesTracker(changes, comments) + rangesTracker.removeChangeId(change_id) + response = RangesManager._getRanges(rangesTracker) + callback null, response + + _getRanges: (rangesTracker) -> # Return the minimal data structure needed, since most documents won't have any # changes or comments response = {} @@ -22,5 +35,4 @@ module.exports = RangesManager = if rangesTracker.comments?.length > 0 response ?= {} response.comments = rangesTracker.comments - logger.log {response}, "applied updates to ranges" - callback null, response \ No newline at end of file + return response \ No newline at end of file diff --git a/services/document-updater/test/acceptance/coffee/RangesTests.coffee b/services/document-updater/test/acceptance/coffee/RangesTests.coffee index 8da51c0899..7498a8087b 100644 --- a/services/document-updater/test/acceptance/coffee/RangesTests.coffee +++ b/services/document-updater/test/acceptance/coffee/RangesTests.coffee @@ -1,6 +1,7 @@ sinon = require "sinon" chai = require("chai") chai.should() +expect = chai.expect async = require "async" rclient = require("redis").createClient() @@ -182,3 +183,45 @@ describe "Ranges", -> changes[0].op.should.deep.equal { i: "123", p: 1 } changes[1].op.should.deep.equal { i: "456", p: 5 } done() + + describe "accepting a change", -> + before (done) -> + @project_id = DocUpdaterClient.randomId() + @user_id = DocUpdaterClient.randomId() + @id_seed = "587357bd35e64f6157" + @doc = { + id: DocUpdaterClient.randomId() + lines: ["aaa"] + } + @update = { + doc: @doc.id + op: [{ i: "456", p: 1 }] + v: 0 + meta: { user_id: @user_id, tc: @id_seed } + } + MockWebApi.insertDoc @project_id, @doc.id, { + lines: @doc.lines + version: 0 + } + DocUpdaterClient.preloadDoc @project_id, @doc.id, (error) => + throw error if error? + DocUpdaterClient.sendUpdate @project_id, @doc.id, @update, (error) => + throw error if error? + setTimeout () => + DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) => + throw error if error? + ranges = data.ranges + change = ranges.changes[0] + change.op.should.deep.equal { i: "456", p: 1 } + change.id.should.equal @id_seed + "000001" + change.metadata.user_id.should.equal @user_id + done() + , 200 + + it "should remove the change after accepting", (done) -> + DocUpdaterClient.acceptChange @project_id, @doc.id, @id_seed + "000001", (error) => + throw error if error? + DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) => + throw error if error? + expect(data.ranges.changes).to.be.undefined + done() \ No newline at end of file diff --git a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee index d704daefd1..afcbfd4b45 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee @@ -72,3 +72,6 @@ module.exports = DocUpdaterClient = deleteProject: (project_id, callback = () ->) -> request.del "http://localhost:3003/project/#{project_id}", callback + + acceptChange: (project_id, doc_id, change_id, callback = () ->) -> + request.post "http://localhost:3003/project/#{project_id}/doc/#{doc_id}/change/#{change_id}/accept", callback \ No newline at end of file diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index 5966843f5a..3f0279b5c7 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -3,6 +3,7 @@ chai = require('chai') should = chai.should() modulePath = "../../../../app/js/DocumentManager.js" SandboxedModule = require('sandboxed-module') +Errors = require "../../../../app/js/Errors" describe "DocumentManager", -> beforeEach -> @@ -18,6 +19,7 @@ describe "DocumentManager", -> "./WebRedisManager": @WebRedisManager = {} "./DiffCodec": @DiffCodec = {} "./UpdateManager": @UpdateManager = {} + "./RangesManager": @RangesManager = {} @project_id = "project-id-123" @doc_id = "doc-id-123" @callback = sinon.stub() @@ -259,4 +261,50 @@ describe "DocumentManager", -> @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 \ No newline at end of file + @DocumentManager.getDoc.called.should.equal false + + describe "acceptChanges", -> + beforeEach -> + @change_id = "mock-change-id" + @version = 34 + @lines = ["original", "lines"] + @ranges = { entries: "mock", comments: "mock" } + @updated_ranges = { entries: "updated", comments: "updated" } + @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges) + @RangesManager.acceptChange = sinon.stub().yields(null, @updated_ranges) + @RedisManager.updateDocument = sinon.stub().yields() + + describe "successfully", -> + beforeEach -> + @DocumentManager.acceptChange @project_id, @doc_id, @change_id, @callback + + it "should get the document's current ranges", -> + @DocumentManager.getDoc + .calledWith(@project_id, @doc_id) + .should.equal true + + it "should apply the accept change to the ranges", -> + @RangesManager.acceptChange + .calledWith(@change_id, @ranges) + .should.equal true + + it "should save the updated ranges", -> + @RedisManager.updateDocument + .calledWith(@doc_id, @lines, @version, [], @updated_ranges) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + describe "when the doc is not found", -> + beforeEach -> + @DocumentManager.getDoc = sinon.stub().yields(null, null, null, null) + @DocumentManager.acceptChange @project_id, @doc_id, @change_id, @callback + + it "should not save anything", -> + @RedisManager.updateDocument.called.should.equal false + + it "should call the callback with a not found error", -> + error = new Errors.NotFoundError("document not found: #{@doc_id}") + @callback.calledWith(error).should.equal true + \ No newline at end of file diff --git a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee index cf0f71a301..4000b402aa 100644 --- a/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee +++ b/services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee @@ -332,4 +332,45 @@ describe "HttpController", -> it "should call next with the error", -> @next .calledWith(new Error("oops")) - .should.equal true \ No newline at end of file + .should.equal true + + describe "acceptChange", -> + beforeEach -> + @req = + params: + project_id: @project_id + doc_id: @doc_id + change_id: @change_id = "mock-change-od-1" + + describe "successfully", -> + beforeEach -> + @DocumentManager.acceptChangeWithLock = sinon.stub().callsArgWith(3) + @HttpController.acceptChange(@req, @res, @next) + + it "should accept the change", -> + @DocumentManager.acceptChangeWithLock + .calledWith(@project_id, @doc_id, @change_id) + .should.equal true + + it "should return a successful No Content response", -> + @res.send + .calledWith(204) + .should.equal true + + it "should log the request", -> + @logger.log + .calledWith({@project_id, @doc_id, @change_id}, "accepting change via http") + .should.equal true + + it "should time the request", -> + @Metrics.Timer::done.called.should.equal true + + describe "when an errors occurs", -> + beforeEach -> + @DocumentManager.acceptChangeWithLock = sinon.stub().callsArgWith(3, new Error("oops")) + @HttpController.acceptChange(@req, @res, @next) + + it "should call next with the error", -> + @next + .calledWith(new Error("oops")) + .should.equal true