diff --git a/services/track-changes/app.coffee b/services/track-changes/app.coffee index 7398ca8252..85aa7ced6b 100644 --- a/services/track-changes/app.coffee +++ b/services/track-changes/app.coffee @@ -5,10 +5,11 @@ ConversoinManager = require "./app/js/ConversionManager" logger = require "logger-sharelatex" logger.initialize("history") -app.post "/doc/:doc_id/flush", (req, res, next) -> +app.post express.bodyParser(), "/doc/:doc_id/flush", (req, res, next) -> project_id = req.params.project_id + docOps = req.body.docOps logger.log doc_id: doc_id, "compressing doc history" - ConversionManager.convertOldRawUpdates doc_id, (error) -> + ConversionManager.convertAndSaveRawOps doc_id, docOps, (error) -> return next(error) if error? res.send 204 # No content diff --git a/services/track-changes/app/coffee/ConversionManager.coffee b/services/track-changes/app/coffee/ConversionManager.coffee index 188339d150..2db3aaad51 100644 --- a/services/track-changes/app/coffee/ConversionManager.coffee +++ b/services/track-changes/app/coffee/ConversionManager.coffee @@ -1,11 +1,11 @@ {db, ObjectId} = require "./mongojs" -ConcatManager = require "./ConcatManager" +HistoryBuilder = require "./HistoryBuilder" logger = require "logger-sharelatex" module.exports = ConversionManager = OPS_TO_LEAVE: 100 - popLatestCompressedUpdate: (doc_id, callback = (error, update) ->) -> + popLastCompressedOp: (doc_id, callback = (error, op) ->) -> db.docHistory.findAndModify query: { doc_id: ObjectId(doc_id) } fields: { docOps: { $slice: -1 } } @@ -14,68 +14,35 @@ module.exports = ConversionManager = return callback(error) if error? callback null, history.docOps[0] - insertCompressedUpdates: (doc_id, updates, callback = (error) ->) -> - db.docHistory.update { doc_id: ObjectId(doc_id) }, { $push: { docOps: { $each: updates } } }, { upsert: true }, callback + insertCompressedOps: (doc_id, docOps, callback = (error) ->) -> + db.docHistory.update { + doc_id: ObjectId(doc_id) + }, { + $push: + docOps: + $each: docOps + }, { + upsert: true + }, callback - popOldRawUpdates: (doc_id, callback = (error, updates) ->) -> - db.docOps.find { doc_id: ObjectId(doc_id) }, { version: true, tailVersion: true }, (error, docs) -> - return callback(error) if error? - return callback(new Error("doc not found")) if docs.length == 0 - doc = docs[0] - currentVersion = doc.version - tailVersion = doc.tailVersion or 0 - if currentVersion - tailVersion > ConversionManager.OPS_TO_LEAVE - db.docOps.findAndModify - query: - doc_id: ObjectId(doc_id) - version: currentVersion - update: - $push: - docOps: - $each: [] - $slice: - ConversionManager.OPS_TO_LEAVE - $set: - tailVersion: currentVersion - ConversionManager.OPS_TO_LEAVE - fields: - docOps: - $slice: currentVersion - tailVersion - ConversionManager.OPS_TO_LEAVE - , (error, doc) -> - return callback(error) if error? - if !doc? - # Version was modified since so try again - return ConversionManager.popOldRawUpdates doc_id, callback - else - return callback null, doc.docOps - - else - callback null, [] + convertAndSaveRawOps: (doc_id, rawOps, callback = (error) ->) -> + length = rawOps.length + if length == 0 + return callback() - convertOldRawUpdates: (doc_id, callback = (error) ->) -> - ConversionManager.popOldRawUpdates doc_id, (error, rawUpdates) -> + + ConversionManager.popLastCompressedOp doc_id, (error, lastCompressedOp) -> return callback(error) if error? - length = rawUpdates.length + if !lastCompressedOp? + rawOps = rawOps.slice(0) # Clone so we can modify in place + lastCompressedOp = rawOps.shift() - normalizedRawUpdates = [] - for rawUpdate in rawUpdates - normalizedRawUpdates = normalizedRawUpdates.concat ConcatManager.normalizeUpdate(rawUpdate) - rawUpdates = normalizedRawUpdates + uncompressedOps = [lastCompressedOp].concat rawOps + compressedOps = HistoryBuilder.compressOps uncompressedOps - ConversionManager.popLatestCompressedUpdate doc_id, (error, lastCompressedUpdate) -> + ConversionManager.insertCompressedOps doc_id, compressedOps, (error) -> return callback(error) if error? - - if !lastCompressedUpdate? - lastCompressedUpdate = rawUpdates.shift() - - if !lastCompressedUpdate? - # No saved versions, no raw updates, nothing to do - return callback() - - uncompressedUpdates = [lastCompressedUpdate].concat rawUpdates - compressedUpdates = ConcatManager.compressUpdates uncompressedUpdates - - ConversionManager.insertCompressedUpdates doc_id, compressedUpdates, (error) -> - return callback(error) if error? - logger.log doc_id: doc_id, rawOpsLength: length, compressedOpsLength: compressedUpdates.length, "compressed doc ops" - callback() + logger.log doc_id: doc_id, rawOpsLength: length, compressedOpsLength: compressedOps.length, "compressed doc ops" + callback() diff --git a/services/track-changes/app/coffee/ConcatManager.coffee b/services/track-changes/app/coffee/HistoryBuilder.coffee similarity index 85% rename from services/track-changes/app/coffee/ConcatManager.coffee rename to services/track-changes/app/coffee/HistoryBuilder.coffee index d56a53c811..2a9405f439 100644 --- a/services/track-changes/app/coffee/ConcatManager.coffee +++ b/services/track-changes/app/coffee/HistoryBuilder.coffee @@ -1,8 +1,11 @@ strInject = (s1, pos, s2) -> s1[...pos] + s2 + s1[pos..] strRemove = (s1, pos, length) -> s1[...pos] + s1[(pos + length)..] -module.exports = ConcatManager = +module.exports = HistoryBuilder = normalizeUpdate: (update) -> + if update.meta.start_ts? + return [update] # already normalized + updates = [] for op in update.op updates.push @@ -15,11 +18,16 @@ module.exports = ConcatManager = compressUpdates: (rawUpdates) -> return [] if rawUpdates.length == 0 - firstPass = [rawUpdates.shift()] - for update in rawUpdates + normalizedUpdates = [] + for rawUpdate in rawUpdates + normalizedUpdates = normalizedUpdates.concat HistoryBuilder.normalizeUpdate(rawUpdate) + + return [] if normalizedUpdates.length == 0 + firstPass = [normalizedUpdates.shift()] + for update in normalizedUpdates lastCompressedUpdate = firstPass.pop() if lastCompressedUpdate? - firstPass = firstPass.concat ConcatManager._concatTwoUpdates lastCompressedUpdate, update, false + firstPass = firstPass.concat HistoryBuilder._concatTwoUpdates lastCompressedUpdate, update, false else firstPass.push rawUpdate @@ -28,7 +36,7 @@ module.exports = ConcatManager = for update in firstPass lastCompressedUpdate = secondPass.pop() if lastCompressedUpdate? - secondPass = secondPass.concat ConcatManager._concatTwoUpdates lastCompressedUpdate, update, true + secondPass = secondPass.concat HistoryBuilder._concatTwoUpdates lastCompressedUpdate, update, true else secondPass.push update @@ -53,7 +61,7 @@ module.exports = ConcatManager = if firstUpdate.meta.user_id != secondUpdate.meta.user_id return [firstUpdate, secondUpdate] - if secondUpdate.meta.start_ts - firstUpdate.meta.end_ts > ConcatManager.MAX_TIME_BETWEEN_UPDATES + if secondUpdate.meta.start_ts - firstUpdate.meta.end_ts > HistoryBuilder.MAX_TIME_BETWEEN_UPDATES return [firstUpdate, secondUpdate] firstOp = firstUpdate.op[0] diff --git a/services/track-changes/test/unit/coffee/ConversionManager/ConversionManagerTests.coffee b/services/track-changes/test/unit/coffee/ConversionManager/ConversionManagerTests.coffee new file mode 100644 index 0000000000..cdc72b69ca --- /dev/null +++ b/services/track-changes/test/unit/coffee/ConversionManager/ConversionManagerTests.coffee @@ -0,0 +1,87 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = "../../../../app/js/ConversionManager.js" +SandboxedModule = require('sandboxed-module') + +describe "ConversionManager", -> + beforeEach -> + @ConversionManager = SandboxedModule.require modulePath, requires: + "./HistoryBuilder": @HistoryBuilder = {} + "./mongojs" : {} + "logger-sharelatex": { log: sinon.stub() } + @doc_id = "doc-id-123" + @callback = sinon.stub() + + describe "when there are no raw ops", -> + beforeEach -> + @ConversionManager.popLastCompressedOp = sinon.stub() + @ConversionManager.insertCompressedOps = sinon.stub() + @ConversionManager.convertAndSaveRawOps @doc_id, [], @callback + + it "should not need to access the database", -> + @ConversionManager.popLastCompressedOp.called.should.equal false + @ConversionManager.insertCompressedOps.called.should.equal false + + it "should call the callback", -> + @callback.called.should.equal true + + describe "when there is no compressed history to begin with", -> + beforeEach -> + @rawOps = ["mock-raw-op-1", "mock-raw-op-2"] + @compressedOps = ["mock-compressed-op"] + + @ConversionManager.popLastCompressedOp = sinon.stub().callsArgWith(1, null, null) + @ConversionManager.insertCompressedOps = sinon.stub().callsArg(2) + @HistoryBuilder.compressOps = sinon.stub().returns(@compressedOps) + @ConversionManager.convertAndSaveRawOps @doc_id, @rawOps, @callback + + it "should try to pop the last compressed op", -> + @ConversionManager.popLastCompressedOp + .calledWith(@doc_id) + .should.equal true + + it "should compress the raw ops", -> + @HistoryBuilder.compressOps + .calledWith(@rawOps) + .should.equal true + + it "should save the compressed ops", -> + @ConversionManager.insertCompressedOps + .calledWith(@doc_id, @compressedOps) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + describe "when the raw ops need appending to existing history", -> + beforeEach -> + @rawOps = ["mock-raw-op-1", "mock-raw-op-2"] + @lastCompressedOp = "mock-last-compressed-op-0" + @compressedOps = ["mock-compressed-op-1"] + + @ConversionManager.popLastCompressedOp = sinon.stub().callsArgWith(1, null, @lastCompressedOp) + @ConversionManager.insertCompressedOps = sinon.stub().callsArg(2) + @HistoryBuilder.compressOps = sinon.stub().returns(@compressedOps) + @ConversionManager.convertAndSaveRawOps @doc_id, @rawOps, @callback + + it "should try to pop the last compressed op", -> + @ConversionManager.popLastCompressedOp + .calledWith(@doc_id) + .should.equal true + + it "should compress the last compressed op and the raw ops", -> + @HistoryBuilder.compressOps + .calledWith([@lastCompressedOp].concat(@rawOps)) + .should.equal true + + it "should save the compressed ops", -> + @ConversionManager.insertCompressedOps + .calledWith(@doc_id, @compressedOps) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + diff --git a/services/track-changes/test/unit/coffee/ConcatManager/ConcatManagerTests.coffee b/services/track-changes/test/unit/coffee/HistoryBuilder/HistoryBuilderTests.coffee similarity index 87% rename from services/track-changes/test/unit/coffee/ConcatManager/ConcatManagerTests.coffee rename to services/track-changes/test/unit/coffee/HistoryBuilder/HistoryBuilderTests.coffee index 93909bc99c..bcbb0a157a 100644 --- a/services/track-changes/test/unit/coffee/ConcatManager/ConcatManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/HistoryBuilder/HistoryBuilderTests.coffee @@ -2,12 +2,12 @@ sinon = require('sinon') chai = require('chai') should = chai.should() expect = chai.expect -modulePath = "../../../../app/js/ConcatManager.js" +modulePath = "../../../../app/js/HistoryBuilder.js" SandboxedModule = require('sandboxed-module') -describe "ConcatManager", -> +describe "HistoryBuilder", -> beforeEach -> - @ConcatManager = SandboxedModule.require modulePath + @HistoryBuilder = SandboxedModule.require modulePath @user_id = "user-id-1" @other_user_id = "user-id-2" @ts1 = Date.now() @@ -16,7 +16,7 @@ describe "ConcatManager", -> describe "compress", -> describe "insert - insert", -> it "should append one insert to the other", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -29,7 +29,7 @@ describe "ConcatManager", -> }] it "should insert one insert inside the other", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -42,7 +42,7 @@ describe "ConcatManager", -> }] it "should not append separated inserts", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -59,7 +59,7 @@ describe "ConcatManager", -> describe "delete - delete", -> it "should append one delete to the other", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -72,7 +72,7 @@ describe "ConcatManager", -> }] it "should insert one delete inside the other", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -85,7 +85,7 @@ describe "ConcatManager", -> }] it "should not append separated deletes", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -102,7 +102,7 @@ describe "ConcatManager", -> describe "insert - delete", -> it "should undo a previous insert", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -115,7 +115,7 @@ describe "ConcatManager", -> }] it "should remove part of an insert from the middle", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, i: "fobaro" ] meta: ts: @ts1, user_id: @user_id }, { @@ -128,7 +128,7 @@ describe "ConcatManager", -> }] it "should cancel out two opposite updates", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -138,7 +138,7 @@ describe "ConcatManager", -> .to.deep.equal [] it "should not combine separated updates", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -155,7 +155,7 @@ describe "ConcatManager", -> describe "delete - insert", -> it "should redo a previous delete at the beginning", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, { @@ -168,7 +168,7 @@ describe "ConcatManager", -> }] it "should redo a previous delete from halfway through", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, d: "foobar" ] meta: ts: @ts1, user_id: @user_id }, { @@ -184,7 +184,7 @@ describe "ConcatManager", -> }] it "should keep words together", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, d: "abcdefghijklmnopqrstuvwxyz hello world" ] meta: ts: @ts1, user_id: @user_id }, { @@ -210,7 +210,7 @@ describe "ConcatManager", -> it "should not combine the ops if the insert text does not match the delete text", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, d: "foobar" ] meta: ts: @ts1, user_id: @user_id }, { @@ -226,7 +226,7 @@ describe "ConcatManager", -> }] it "should cancel two equal updates", -> - expect(@ConcatManager.compressUpdates [{ + expect(@HistoryBuilder.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, {