diff --git a/services/track-changes/app/coffee/DiffManager.coffee b/services/track-changes/app/coffee/DiffManager.coffee index e517a5ef4e..c791fe1e09 100644 --- a/services/track-changes/app/coffee/DiffManager.coffee +++ b/services/track-changes/app/coffee/DiffManager.coffee @@ -4,36 +4,42 @@ DiffGenerator = require "./DiffGenerator" logger = require "logger-sharelatex" module.exports = DiffManager = - getLatestDocAndUpdates: (project_id, doc_id, fromVersion, toVersion, callback = (error, lines, version, updates) ->) -> + getLatestDocAndUpdates: (project_id, doc_id, fromVersion, toVersion, callback = (error, content, version, updates) ->) -> UpdatesManager.getUpdatesWithUserInfo doc_id, from: fromVersion, to: toVersion, (error, updates) -> return callback(error) if error? - DocumentUpdaterManager.getDocument project_id, doc_id, (error, lines, version) -> + DocumentUpdaterManager.getDocument project_id, doc_id, (error, content, version) -> return callback(error) if error? - callback(null, lines, version, updates) + callback(null, content, version, updates) getDiff: (project_id, doc_id, fromVersion, toVersion, callback = (error, diff) ->) -> logger.log project_id: project_id, doc_id: doc_id, from: fromVersion, to: toVersion, "getting diff" - DiffManager.getLatestDocAndUpdates project_id, doc_id, fromVersion, null, (error, lines, version, updates) -> + DiffManager.getDocumentBeforeVersion project_id, doc_id, fromVersion, (error, startingContent, updates) -> return callback(error) if error? - logger.log lines: lines, version: version, updates: updates, "got doc and updates" + updatesToApply = [] + for update in updates.slice().reverse() + if update.v <= toVersion + updatesToApply.push update + + try + diff = DiffGenerator.buildDiff startingContent, updatesToApply + catch e + return callback(e) + + callback(null, diff) + + getDocumentBeforeVersion: (project_id, doc_id, version, callback = (error, document, rewoundUpdates) ->) -> + logger.log project_id: project_id, doc_id: doc_id, version: version, "getting document before version" + DiffManager.getLatestDocAndUpdates project_id, doc_id, version, null, (error, content, version, updates) -> + return callback(error) if error? lastUpdate = updates[0] if lastUpdate? and lastUpdate.v != version - 1 return callback new Error("latest update version, #{lastUpdate.v}, does not match doc version, #{version}") - updatesToApply = [] - for update in updates.reverse() - if update.v <= toVersion - updatesToApply.push update - - logger.log project_id: project_id, doc_id: doc_id, updatesToApply: updatesToApply, "got updates to apply" - try - startingContent = DiffGenerator.rewindUpdates lines.join("\n"), updates - logger.log project_id: project_id, doc_id: doc_id, startingContent: startingContent, "rewound doc" - diff = DiffGenerator.buildDiff startingContent, updatesToApply + startingContent = DiffGenerator.rewindUpdates content, updates.slice().reverse() catch e return callback(e) - callback(null, diff) \ No newline at end of file + callback(null, startingContent, updates) \ No newline at end of file diff --git a/services/track-changes/app/coffee/DocumentUpdaterManager.coffee b/services/track-changes/app/coffee/DocumentUpdaterManager.coffee index 0b02a9a16c..e88f0f3520 100644 --- a/services/track-changes/app/coffee/DocumentUpdaterManager.coffee +++ b/services/track-changes/app/coffee/DocumentUpdaterManager.coffee @@ -14,7 +14,7 @@ module.exports = DocumentUpdaterManager = body = JSON.parse(body) catch error return callback(error) - callback null, body.lines, body.version + callback null, body.lines.join("\n"), body.version else error = new Error("doc updater returned a non-success status code: #{res.statusCode}") logger.error err: error, project_id:project_id, doc_id:doc_id, url: url, "error accessing doc updater" diff --git a/services/track-changes/app/coffee/UpdatesManager.coffee b/services/track-changes/app/coffee/UpdatesManager.coffee index ee9bb12655..1014001835 100644 --- a/services/track-changes/app/coffee/UpdatesManager.coffee +++ b/services/track-changes/app/coffee/UpdatesManager.coffee @@ -14,7 +14,6 @@ module.exports = UpdatesManager = MongoManager.popLastCompressedUpdate doc_id, (error, lastCompressedUpdate) -> return callback(error) if error? - logger.log doc_id: doc_id, "popped last update" # Ensure that raw updates start where lastCompressedUpdate left off if lastCompressedUpdate? @@ -39,11 +38,9 @@ module.exports = UpdatesManager = REDIS_READ_BATCH_SIZE: 100 processUncompressedUpdates: (doc_id, callback = (error) ->) -> - logger.log "processUncompressedUpdates" RedisManager.getOldestRawUpdates doc_id, UpdatesManager.REDIS_READ_BATCH_SIZE, (error, rawUpdates) -> return callback(error) if error? length = rawUpdates.length - logger.log doc_id: doc_id, length: length, "got raw updates from redis" UpdatesManager.compressAndSaveRawUpdates doc_id, rawUpdates, (error) -> return callback(error) if error? logger.log doc_id: doc_id, "compressed and saved doc updates" diff --git a/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee b/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee index 7b1ce33af1..57c367db2a 100644 --- a/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee @@ -20,11 +20,11 @@ describe "DiffManager", -> describe "getLatestDocAndUpdates", -> beforeEach -> - @lines = [ "hello", "world" ] + @content = "hello world" @version = 42 @updates = [ "mock-update-1", "mock-update-2" ] - @DocumentUpdaterManager.getDocument = sinon.stub().callsArgWith(2, null, @lines, @version) + @DocumentUpdaterManager.getDocument = sinon.stub().callsArgWith(2, null, @content, @version) @UpdatesManager.getUpdatesWithUserInfo = sinon.stub().callsArgWith(2, null, @updates) @DiffManager.getLatestDocAndUpdates @project_id, @doc_id, @from, @to, @callback @@ -38,12 +38,12 @@ describe "DiffManager", -> .calledWith(@doc_id, from: @from, to: @to) .should.equal true - it "should call the callback with the lines, version and updates", -> - @callback.calledWith(null, @lines, @version, @updates).should.equal true + it "should call the callback with the content, version and updates", -> + @callback.calledWith(null, @content, @version, @updates).should.equal true describe "getDiff", -> beforeEach -> - @lines = [ "hello", "world" ] + @content = "hello world" # Op versions are the version they were applied to, so doc is always one version # ahead.s @version = 43 @@ -61,11 +61,56 @@ describe "DiffManager", -> describe "with matching versions", -> beforeEach -> - @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @lines, @version, @updates) - @DiffGenerator.rewindUpdates = sinon.stub().returns(@rewound_content) + @DiffManager.getDocumentBeforeVersion = sinon.stub().callsArgWith(3, null, @rewound_content, @updates) @DiffGenerator.buildDiff = sinon.stub().returns(@diff) @DiffManager.getDiff @project_id, @doc_id, @fromVersion, @toVersion, @callback + it "should get the latest doc and version with all recent updates", -> + @DiffManager.getDocumentBeforeVersion + .calledWith(@project_id, @doc_id, @fromVersion) + .should.equal true + + it "should generate the diff", -> + @DiffGenerator.buildDiff + .calledWith(@rewound_content, @diffed_updates.slice().reverse()) + .should.equal true + + it "should call the callback with the diff", -> + @callback.calledWith(null, @diff).should.equal true + + describe "when the updates are inconsistent", -> + beforeEach -> + @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates) + @DiffGenerator.buildDiff = sinon.stub().throws(@error = new Error("inconsistent!")) + @DiffManager.getDiff @project_id, @doc_id, @fromVersion, @toVersion, @callback + + it "should call the callback with an error", -> + @callback + .calledWith(@error) + .should.equal true + + describe "getDocumentBeforeVersion", -> + beforeEach -> + @content = "hello world" + # Op versions are the version they were applied to, so doc is always one version + # ahead.s + @version = 43 + @updates = [ + { op: "mock-4", v: 42, meta: { start_ts: new Date(@to.getTime() + 20)} } + { op: "mock-3", v: 41, meta: { start_ts: new Date(@to.getTime() + 10)} } + { op: "mock-2", v: 40, meta: { start_ts: new Date(@to.getTime() - 10)} } + { op: "mock-1", v: 39, meta: { start_ts: new Date(@to.getTime() - 20)} } + ] + @fromVersion = 39 + @rewound_content = "rewound-content" + @diff = [ u: "mock-diff" ] + + describe "with matching versions", -> + beforeEach -> + @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates) + @DiffGenerator.rewindUpdates = sinon.stub().returns(@rewound_content) + @DiffManager.getDocumentBeforeVersion @project_id, @doc_id, @fromVersion, @callback + it "should get the latest doc and version with all recent updates", -> @DiffManager.getLatestDocAndUpdates .calledWith(@project_id, @doc_id, @fromVersion, null) @@ -73,23 +118,18 @@ describe "DiffManager", -> it "should rewind the diff", -> @DiffGenerator.rewindUpdates - .calledWith(@lines.join("\n"), @updates) + .calledWith(@content, @updates.slice().reverse()) .should.equal true - it "should generate the diff", -> - @DiffGenerator.buildDiff - .calledWith(@rewound_content, @diffed_updates.reverse()) - .should.equal true - - it "should call the callback with the diff", -> - @callback.calledWith(null, @diff).should.equal true + it "should call the callback with the rewound document and updates", -> + @callback.calledWith(null, @rewound_content, @updates).should.equal true describe "with mismatching versions", -> beforeEach -> @version = 50 @updates = [ { op: "mock-1", v: 40 }, { op: "mock-1", v: 39 } ] - @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @lines, @version, @updates) - @DiffManager.getDiff @project_id, @doc_id, @fromVersion, @toVersion, @callback + @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates) + @DiffManager.getDocumentBeforeVersion @project_id, @doc_id, @fromVersion, @callback it "should call the callback with an error", -> @callback @@ -98,12 +138,11 @@ describe "DiffManager", -> describe "when the updates are inconsistent", -> beforeEach -> - @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @lines, @version, @updates) + @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates) @DiffGenerator.rewindUpdates = sinon.stub().throws(@error = new Error("inconsistent!")) - @DiffManager.getDiff @project_id, @doc_id, @fromVersion, @toVersion, @callback + @DiffManager.getDocumentBeforeVersion @project_id, @doc_id, @fromVersion, @callback it "should call the callback with an error", -> @callback .calledWith(@error) .should.equal true - diff --git a/services/track-changes/test/unit/coffee/DocumentUpdaterManager/DocumentUpdaterManagerTests.coffee b/services/track-changes/test/unit/coffee/DocumentUpdaterManager/DocumentUpdaterManagerTests.coffee index 5941eefd8d..642909a787 100644 --- a/services/track-changes/test/unit/coffee/DocumentUpdaterManager/DocumentUpdaterManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/DocumentUpdaterManager/DocumentUpdaterManagerTests.coffee @@ -30,8 +30,8 @@ describe "DocumentUpdaterManager", -> url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}" @request.get.calledWith(url).should.equal true - it "should call the callback with the lines and version", -> - @callback.calledWith(null, @lines, @version, @ops).should.equal true + it "should call the callback with the content and version", -> + @callback.calledWith(null, @lines.join("\n"), @version, @ops).should.equal true describe "when the document updater API returns an error", -> beforeEach ->