diff --git a/services/track-changes/app/coffee/UpdateCompressor.coffee b/services/track-changes/app/coffee/UpdateCompressor.coffee index 5ba379a8d9..6d7527057f 100644 --- a/services/track-changes/app/coffee/UpdateCompressor.coffee +++ b/services/track-changes/app/coffee/UpdateCompressor.coffee @@ -2,6 +2,8 @@ strInject = (s1, pos, s2) -> s1[...pos] + s2 + s1[pos..] strRemove = (s1, pos, length) -> s1[...pos] + s1[(pos + length)..] module.exports = UpdateCompressor = + NOOP: "noop" + # Updates come from the doc updater in format # { # op: [ { ... op1 ... }, { ... op2 ... } ] @@ -19,14 +21,23 @@ module.exports = UpdateCompressor = convertToSingleOpUpdates: (updates) -> splitUpdates = [] for update in updates - for op in update.op + if update.op.length == 0 splitUpdates.push - op: op + op: UpdateCompressor.NOOP meta: start_ts: update.meta.start_ts or update.meta.ts end_ts: update.meta.end_ts or update.meta.ts user_id: update.meta.user_id v: update.v + else + for op in update.op + splitUpdates.push + op: op + meta: + start_ts: update.meta.start_ts or update.meta.ts + end_ts: update.meta.end_ts or update.meta.ts + user_id: update.meta.user_id + v: update.v return splitUpdates concatUpdatesWithSameVersion: (updates) -> @@ -34,12 +45,14 @@ module.exports = UpdateCompressor = for update in updates lastUpdate = concattedUpdates[concattedUpdates.length - 1] if lastUpdate? and lastUpdate.v == update.v - lastUpdate.op.push update.op + lastUpdate.op.push update.op unless update.op == UpdateCompressor.NOOP else - concattedUpdates.push - op: [ update.op ] + nextUpdate = + op: [] meta: update.meta v: update.v + nextUpdate.op.push update.op unless update.op == UpdateCompressor.NOOP + concattedUpdates.push nextUpdate return concattedUpdates compressRawUpdates: (lastPreviousUpdate, rawUpdates) -> diff --git a/services/track-changes/test/acceptance/coffee/AppendingUpdatesTests.coffee b/services/track-changes/test/acceptance/coffee/AppendingUpdatesTests.coffee index 7a8d465504..b86da90c19 100644 --- a/services/track-changes/test/acceptance/coffee/AppendingUpdatesTests.coffee +++ b/services/track-changes/test/acceptance/coffee/AppendingUpdatesTests.coffee @@ -194,3 +194,35 @@ describe "Appending doc ops to the history", -> expect(@updates[0].v).to.equal 3 expect(@updates[1].v).to.equal 4 + describe "when there is a no-op update", -> + before (done) -> + @project_id = ObjectId().toString() + @doc_id = ObjectId().toString() + @user_id = ObjectId().toString() + oneDay = 24 * 60 * 60 * 1000 + TrackChangesClient.pushRawUpdates @project_id, @doc_id, [{ + op: [] + meta: { ts: Date.now(), user_id: @user_id } + v: 3 + }, { + op: [{ i: "foo", p: 3 }] + meta: { ts: Date.now() + oneDay, user_id: @user_id } + v: 4 + }], (error) => + throw error if error? + TrackChangesClient.flushAndGetCompressedUpdates @project_id, @doc_id, (error, @updates) => + throw error if error? + done() + + it "should insert the compressed no-op into mongo", -> + expect(@updates[0].op).to.deep.equal [] + + + it "should insert the compressed next update into mongo", -> + expect(@updates[1].op).to.deep.equal [{ + p: 3, i: "foo" + }] + + it "should insert the correct version numbers into mongo", -> + expect(@updates[0].v).to.equal 3 + expect(@updates[1].v).to.equal 4 diff --git a/services/track-changes/test/acceptance/coffee/FlushingUpdatesTests.coffee b/services/track-changes/test/acceptance/coffee/FlushingUpdatesTests.coffee new file mode 100644 index 0000000000..70ee40307e --- /dev/null +++ b/services/track-changes/test/acceptance/coffee/FlushingUpdatesTests.coffee @@ -0,0 +1,56 @@ +sinon = require "sinon" +chai = require("chai") +chai.should() +expect = chai.expect +mongojs = require "../../../app/js/mongojs" +ObjectId = mongojs.ObjectId +Settings = require "settings-sharelatex" +request = require "request" +rclient = require("redis").createClient() # Only works locally for now + +TrackChangesClient = require "./helpers/TrackChangesClient" + +describe "Flushing updates", -> + describe "flushing a doc's updates", -> + before (done) -> + @project_id = ObjectId().toString() + @doc_id = ObjectId().toString() + @user_id = ObjectId().toString() + TrackChangesClient.pushRawUpdates @project_id, @doc_id, [{ + op: [{ i: "f", p: 3 }] + meta: { ts: Date.now(), user_id: @user_id } + v: 3 + }], (error) => + throw error if error? + TrackChangesClient.flushDoc @project_id, @doc_id, (error) -> + throw error if error? + done() + + it "should flush the op into mongo", (done) -> + TrackChangesClient.getCompressedUpdates @doc_id, (error, updates) -> + expect(updates[0].op).to.deep.equal [{ + p: 3, i: "f" + }] + done() + + describe "flushing a project's updates", -> + before (done) -> + @project_id = ObjectId().toString() + @doc_id = ObjectId().toString() + @user_id = ObjectId().toString() + TrackChangesClient.pushRawUpdates @project_id, @doc_id, [{ + op: [{ i: "f", p: 3 }] + meta: { ts: Date.now(), user_id: @user_id } + v: 3 + }], (error) => + throw error if error? + TrackChangesClient.flushProject @project_id, (error) -> + throw error if error? + done() + + it "should flush the op into mongo", (done) -> + TrackChangesClient.getCompressedUpdates @doc_id, (error, updates) -> + expect(updates[0].op).to.deep.equal [{ + p: 3, i: "f" + }] + done() diff --git a/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee b/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee index ec41dcca30..3d14a84eb9 100644 --- a/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee +++ b/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee @@ -38,6 +38,18 @@ describe "UpdateCompressor", -> v: 43 }] + it "should return no-op updates when the op list is empty", -> + expect(@UpdateCompressor.convertToSingleOpUpdates [{ + op: [] + meta: { ts: @ts1, user_id: @user_id } + v: 42 + }]) + .to.deep.equal [{ + op: @UpdateCompressor.NOOP + meta: { start_ts: @ts1, end_ts: @ts1, user_id: @user_id }, + v: 42 + }] + describe "concatUpdatesWithSameVersion", -> it "should concat updates with the same version", -> expect(@UpdateCompressor.concatUpdatesWithSameVersion [{ @@ -63,6 +75,18 @@ describe "UpdateCompressor", -> v: 43 }] + it "should turn a noop into an empty op", -> + expect(@UpdateCompressor.concatUpdatesWithSameVersion [{ + op: @UpdateCompressor.NOOP + meta: { start_ts: @ts1, end_ts: @ts1, user_id: @user_id } + v: 42 + }]) + .to.deep.equal [{ + op: [] + meta: { start_ts: @ts1, end_ts: @ts1, user_id: @user_id } + v: 42 + }] + describe "compress", -> describe "insert - insert", -> it "should append one insert to the other", -> @@ -239,4 +263,44 @@ describe "UpdateCompressor", -> v: 43 }] - + describe "noop - insert", -> + it "should leave them untouched", -> + expect(@UpdateCompressor.compressUpdates [{ + op: @UpdateCompressor.NOOP + meta: ts: @ts1, user_id: @user_id + v: 42 + }, { + op: { p: 6, i: "bar" } + meta: ts: @ts1, user_id: @user_id + v: 43 + }]) + .to.deep.equal [{ + op: @UpdateCompressor.NOOP + meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id + v: 42 + }, { + op: { p: 6, i: "bar" } + meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id + v: 43 + }] + + describe "noop - delete", -> + it "should leave them untouched", -> + expect(@UpdateCompressor.compressUpdates [{ + op: @UpdateCompressor.NOOP + meta: ts: @ts1, user_id: @user_id + v: 42 + }, { + op: { p: 6, d: "bar" } + meta: ts: @ts1, user_id: @user_id + v: 43 + }]) + .to.deep.equal [{ + op: @UpdateCompressor.NOOP + meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id + v: 42 + }, { + op: { p: 6, d: "bar" } + meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id + v: 43 + }]