From c5252f893bd21bba2fa8b5e4f376114c5fcee08e Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 28 Jun 2017 15:38:31 +0100 Subject: [PATCH] Don't return all updates if no version is given --- .../app/coffee/DiffManager.coffee | 8 +-- .../DiffManager/DiffManagerTests.coffee | 52 +++++++++++++------ .../PackManager/PackManagerTests.coffee | 1 + 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/services/track-changes/app/coffee/DiffManager.coffee b/services/track-changes/app/coffee/DiffManager.coffee index c01c73a533..af1cfaf03f 100644 --- a/services/track-changes/app/coffee/DiffManager.coffee +++ b/services/track-changes/app/coffee/DiffManager.coffee @@ -4,12 +4,14 @@ DiffGenerator = require "./DiffGenerator" logger = require "logger-sharelatex" module.exports = DiffManager = - getLatestDocAndUpdates: (project_id, doc_id, fromVersion, toVersion, callback = (error, content, version, updates) ->) -> + getLatestDocAndUpdates: (project_id, doc_id, fromVersion, callback = (error, content, version, updates) ->) -> # Get updates last, since then they must be ahead and it # might be possible to rewind to the same version as the doc. DocumentUpdaterManager.getDocument project_id, doc_id, (error, content, version) -> return callback(error) if error? - UpdatesManager.getDocUpdatesWithUserInfo project_id, doc_id, from: fromVersion, to: toVersion, (error, updates) -> + if !fromVersion? # If we haven't been given a version, just return lastest doc and no updates + return callback(null, content, version, []) + UpdatesManager.getDocUpdatesWithUserInfo project_id, doc_id, from: fromVersion, (error, updates) -> return callback(error) if error? callback(null, content, version, updates) @@ -56,7 +58,7 @@ module.exports = DiffManager = _tryGetDocumentBeforeVersion: (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) -> + DiffManager.getLatestDocAndUpdates project_id, doc_id, version, (error, content, version, updates) -> return callback(error) if error? # bail out if we hit a broken update diff --git a/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee b/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee index 723193403c..f0bca22e90 100644 --- a/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee @@ -26,20 +26,40 @@ describe "DiffManager", -> @DocumentUpdaterManager.getDocument = sinon.stub().callsArgWith(2, null, @content, @version) @UpdatesManager.getDocUpdatesWithUserInfo = sinon.stub().callsArgWith(3, null, @updates) - @DiffManager.getLatestDocAndUpdates @project_id, @doc_id, @from, @to, @callback - it "should get the latest version of the doc", -> - @DocumentUpdaterManager.getDocument - .calledWith(@project_id, @doc_id) - .should.equal true + describe "with a fromVersion", -> + beforeEach -> + @DiffManager.getLatestDocAndUpdates @project_id, @doc_id, @from, @callback - it "should get the latest updates", -> - @UpdatesManager.getDocUpdatesWithUserInfo - .calledWith(@project_id, @doc_id, from: @from, to: @to) - .should.equal true + it "should get the latest version of the doc", -> + @DocumentUpdaterManager.getDocument + .calledWith(@project_id, @doc_id) + .should.equal true - it "should call the callback with the content, version and updates", -> - @callback.calledWith(null, @content, @version, @updates).should.equal true + it "should get the latest updates", -> + @UpdatesManager.getDocUpdatesWithUserInfo + .calledWith(@project_id, @doc_id, from: @from) + .should.equal true + + it "should call the callback with the content, version and updates", -> + @callback.calledWith(null, @content, @version, @updates).should.equal true + + describe "with no fromVersion", -> + beforeEach -> + @DiffManager.getLatestDocAndUpdates @project_id, @doc_id, null, @callback + + it "should get the latest version of the doc", -> + @DocumentUpdaterManager.getDocument + .calledWith(@project_id, @doc_id) + .should.equal true + + it "should not get the latest updates", -> + @UpdatesManager.getDocUpdatesWithUserInfo + .called.should.equal false + + it "should call the callback with the content, version and blank updates", -> + @callback.calledWith(null, @content, @version, []).should.equal true + describe "getDiff", -> beforeEach -> @@ -80,7 +100,7 @@ describe "DiffManager", -> describe "when the updates are inconsistent", -> beforeEach -> - @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates) + @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(3, null, @content, @version, @updates) @DiffGenerator.buildDiff = sinon.stub().throws(@error = new Error("inconsistent!")) @DiffManager.getDiff @project_id, @doc_id, @fromVersion, @toVersion, @callback @@ -177,7 +197,7 @@ describe "DiffManager", -> describe "with matching versions", -> beforeEach -> - @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates) + @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(3, null, @content, @version, @updates) @DiffGenerator.rewindUpdates = sinon.spy (content, updates) => # the rewindUpdates method reverses the 'updates' array updates.reverse() @@ -187,7 +207,7 @@ describe "DiffManager", -> it "should get the latest doc and version with all recent updates", -> @DiffManager.getLatestDocAndUpdates - .calledWith(@project_id, @doc_id, @fromVersion, null) + .calledWith(@project_id, @doc_id, @fromVersion) .should.equal true it "should rewind the diff", -> @@ -200,7 +220,7 @@ describe "DiffManager", -> beforeEach -> @version = 50 @updates = [ { op: "mock-1", v: 40 }, { op: "mock-1", v: 39 } ] - @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates) + @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(3, null, @content, @version, @updates) @DiffManager._tryGetDocumentBeforeVersion @project_id, @doc_id, @fromVersion, @callback it "should call the callback with an error with retry = true set", -> @@ -210,7 +230,7 @@ describe "DiffManager", -> describe "when the updates are inconsistent", -> beforeEach -> - @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates) + @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(3, null, @content, @version, @updates) @DiffGenerator.rewindUpdates = sinon.stub().throws(@error = new Error("inconsistent!")) @DiffManager.getDocumentBeforeVersion @project_id, @doc_id, @fromVersion, @callback diff --git a/services/track-changes/test/unit/coffee/PackManager/PackManagerTests.coffee b/services/track-changes/test/unit/coffee/PackManager/PackManagerTests.coffee index 29c9cf6d3d..dd79d0bf21 100644 --- a/services/track-changes/test/unit/coffee/PackManager/PackManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/PackManager/PackManagerTests.coffee @@ -21,6 +21,7 @@ describe "PackManager", -> "./MongoAWS": {} "logger-sharelatex": { log: sinon.stub(), error: sinon.stub() } 'metrics-sharelatex': {inc: ()->} + "./ProjectIterator": require("../../../../app/js/ProjectIterator.js") # Cache for speed @callback = sinon.stub() @doc_id = ObjectId().toString() @project_id = ObjectId().toString()