From d6b827426ccd3cbfbe9667d0e482ca0772c79af3 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 23 Sep 2015 13:22:38 +0100 Subject: [PATCH] support forcing new compressed update in popLastCompressedUpdate callback with a null update, passing the version as an additional argument --- .../app/coffee/MongoManager.coffee | 19 ++++++++--- .../app/coffee/UpdatesManager.coffee | 25 +++++++------- .../MongoManager/MongoManagerTests.coffee | 21 +++++++++++- .../UpdatesManager/UpdatesManagerTests.coffee | 33 +++++++++++++++++++ 4 files changed, 81 insertions(+), 17 deletions(-) diff --git a/services/track-changes/app/coffee/MongoManager.coffee b/services/track-changes/app/coffee/MongoManager.coffee index 3bcf50af2a..383cacc198 100644 --- a/services/track-changes/app/coffee/MongoManager.coffee +++ b/services/track-changes/app/coffee/MongoManager.coffee @@ -20,13 +20,24 @@ module.exports = MongoManager = deleteCompressedUpdate: (id, callback = (error) ->) -> db.docHistory.remove({ _id: ObjectId(id.toString()) }, callback) - popLastCompressedUpdate: (doc_id, callback = (error, update) ->) -> + popLastCompressedUpdate: (doc_id, callback = (error, update, version) ->) -> + # under normal use we pass back the last update as + # callback(null,update). + # + # when we have an existing last update but want to force a new one + # to start, we pass it back as callback(null,null,version), just + # giving the version so we can check consistency. MongoManager.getLastCompressedUpdate doc_id, (error, update) -> return callback(error) if error? if update? - MongoManager.deleteCompressedUpdate update._id, (error) -> - return callback(error) if error? - callback null, update + if update.inS3? + # we want to force a new update, but ensure that it is + # consistent with the version of the existing one in S3 + return callback null, null, update.v + else + MongoManager.deleteCompressedUpdate update._id, (error) -> + return callback(error) if error? + callback null, update else callback null, null diff --git a/services/track-changes/app/coffee/UpdatesManager.coffee b/services/track-changes/app/coffee/UpdatesManager.coffee index ffec3d1efe..0eb69b2b23 100644 --- a/services/track-changes/app/coffee/UpdatesManager.coffee +++ b/services/track-changes/app/coffee/UpdatesManager.coffee @@ -7,7 +7,6 @@ UpdateTrimmer = require "./UpdateTrimmer" logger = require "logger-sharelatex" async = require "async" DocArchiveManager = require "./DocArchiveManager" -_ = require "underscore" module.exports = UpdatesManager = compressAndSaveRawUpdates: (project_id, doc_id, rawUpdates, temporary, callback = (error) ->) -> @@ -15,27 +14,29 @@ module.exports = UpdatesManager = if length == 0 return callback() - MongoManager.popLastCompressedUpdate doc_id, (error, lastCompressedUpdate) -> + MongoManager.popLastCompressedUpdate doc_id, (error, lastCompressedUpdate, lastVersion = lastCompressedUpdate?.v) -> + # lastCompressedUpdate may be null if we are forcing the start + # of a new compressed update, in which case we have the + # lastVersion to check consistency (defaults to lastCompressedUpdate.v) return callback(error) if error? - # Ensure that raw updates start where lastCompressedUpdate left off - if lastCompressedUpdate? + # Ensure that raw updates start where lastVersion left off + if lastVersion? rawUpdates = rawUpdates.slice(0) - while rawUpdates[0]? and rawUpdates[0].v <= lastCompressedUpdate.v + while rawUpdates[0]? and rawUpdates[0].v <= lastVersion rawUpdates.shift() - if rawUpdates[0]? and rawUpdates[0].v != lastCompressedUpdate.v + 1 - error = new Error("Tried to apply raw op at version #{rawUpdates[0].v} to last compressed update with version #{lastCompressedUpdate.v}") + if rawUpdates[0]? and rawUpdates[0].v != lastVersion + 1 + error = new Error("Tried to apply raw op at version #{rawUpdates[0].v} to last compressed update with version #{lastVersion}") logger.error err: error, doc_id: doc_id, project_id: project_id, "inconsistent doc versions" - # Push the update back into Mongo - catching errors at this + # Push the update back into Mongo - if it was present - catching errors at this # point is useless, we're already bailing - MongoManager.insertCompressedUpdates project_id, doc_id, [lastCompressedUpdate], temporary, () -> - return callback error + if lastCompressedUpdate? + MongoManager.insertCompressedUpdates project_id, doc_id, [lastCompressedUpdate], temporary, () -> + return callback error return compressedUpdates = UpdateCompressor.compressRawUpdates lastCompressedUpdate, rawUpdates - if lastCompressedUpdate?.inS3? and not _.some(compressedUpdates, (update) -> update.inS3) - compressedUpdates[compressedUpdates.length-1].inS3 = lastCompressedUpdate.inS3 MongoManager.insertCompressedUpdates project_id, doc_id, compressedUpdates, temporary,(error) -> return callback(error) if error? diff --git a/services/track-changes/test/unit/coffee/MongoManager/MongoManagerTests.coffee b/services/track-changes/test/unit/coffee/MongoManager/MongoManagerTests.coffee index 8db054b40f..6c127cd343 100644 --- a/services/track-changes/test/unit/coffee/MongoManager/MongoManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/MongoManager/MongoManagerTests.coffee @@ -104,6 +104,25 @@ describe "MongoManager", -> it "should call the callback with the update", -> @callback.calledWith(null, @update).should.equal true + describe "when there is a last update in S3", -> + beforeEach -> + @update = { _id: Object(), v: 12345, inS3: true } + @MongoManager.getLastCompressedUpdate = sinon.stub().callsArgWith(1, null, @update) + @MongoManager.deleteCompressedUpdate = sinon.stub() + @MongoManager.popLastCompressedUpdate @doc_id, @callback + + it "should get the last update", -> + @MongoManager.getLastCompressedUpdate + .calledWith(@doc_id) + .should.equal true + + it "should not try to delete the last update", -> + @MongoManager.deleteCompressedUpdate.called.should.equal false + + it "should call the callback with a null update and the correct version", -> + @callback.calledWith(null, null, @update.v).should.equal true + + describe "insertCompressedUpdate", -> beforeEach -> @update = { op: "op", meta: "meta", v: "v"} @@ -483,4 +502,4 @@ describe "MongoManager", -> .should.equal true it "should call the callback", -> - @callback.called.should.equal true \ No newline at end of file + @callback.called.should.equal true diff --git a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee index f29f15f195..04a2111893 100644 --- a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee @@ -122,6 +122,39 @@ describe "UpdatesManager", -> .calledWith(@project_id, @doc_id, [@lastCompressedUpdate], @temporary) .should.equal true + describe "when the raw ops need appending to existing history which is in S3", -> + beforeEach -> + @lastCompressedUpdate = null + @lastVersion = 11 + @compressedUpdates = { v: 13, op: "compressed-op-12" } + + @MongoManager.popLastCompressedUpdate = sinon.stub().callsArgWith(1, null, null, @lastVersion) + @MongoManager.insertCompressedUpdates = sinon.stub().callsArg(4) + @UpdateCompressor.compressRawUpdates = sinon.stub().returns(@compressedUpdates) + + describe "when the raw ops start where the existing history ends", -> + beforeEach -> + @rawUpdates = [{ v: 12, op: "mock-op-12" }, { v: 13, op: "mock-op-13" }] + @UpdatesManager.compressAndSaveRawUpdates @project_id, @doc_id, @rawUpdates, @temporary, @callback + + it "should try to pop the last compressed op", -> + @MongoManager.popLastCompressedUpdate + .calledWith(@doc_id) + .should.equal true + + it "should compress the last compressed op and the raw ops", -> + @UpdateCompressor.compressRawUpdates + .calledWith(@lastCompressedUpdate, @rawUpdates) + .should.equal true + + it "should save the compressed ops", -> + @MongoManager.insertCompressedUpdates + .calledWith(@project_id, @doc_id, @compressedUpdates, @temporary) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + describe "processUncompressedUpdates", -> beforeEach -> @UpdatesManager.compressAndSaveRawUpdates = sinon.stub().callsArgWith(4)