diff --git a/services/track-changes/app/coffee/HttpController.coffee b/services/track-changes/app/coffee/HttpController.coffee index 70b6ca2557..215d440129 100644 --- a/services/track-changes/app/coffee/HttpController.coffee +++ b/services/track-changes/app/coffee/HttpController.coffee @@ -40,12 +40,40 @@ module.exports = HttpController = UpdatesManager.getUpdatesWithUserInfo doc_id, to: to, limit: limit, (error, updates) -> return next(error) if error? - formattedUpdates = for update in updates - { - meta: update.meta - v: update.v - } - res.send JSON.stringify updates: formattedUpdates + res.send JSON.stringify updates: HttpController._buildUpdatesView(updates) + + TIME_BETWEEN_DISTINCT_UPDATES: fiveMinutes = 5 * 60 * 1000 + _buildUpdatesView: (updates) -> + view = [] + for update in updates.slice().reverse() + lastUpdate = view[view.length - 1] + if lastUpdate and update.meta.start_ts - lastUpdate.meta.end_ts < @TIME_BETWEEN_DISTINCT_UPDATES + if update.meta.user? + userExists = false + for user in lastUpdate.meta.users + if user.id == update.meta.user.id + userExists = true + break + if !userExists + lastUpdate.meta.users.push update.meta.user + lastUpdate.meta.start_ts = Math.min(lastUpdate.meta.start_ts, update.meta.start_ts) + lastUpdate.meta.end_ts = Math.max(lastUpdate.meta.end_ts, update.meta.end_ts) + lastUpdate.toV = update.v + else + newUpdate = + meta: + users: [] + start_ts: update.meta.start_ts + end_ts: update.meta.end_ts + fromV: update.v + toV: update.v + + if update.meta.user? + newUpdate.meta.users.push update.meta.user + + view.push newUpdate + + return view.reverse() restore: (req, res, next = (error) ->) -> {doc_id, project_id, version} = req.params diff --git a/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee b/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee index 85dc9a1bd6..99a103bb74 100644 --- a/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee +++ b/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee @@ -11,12 +11,10 @@ TrackChangesClient = require "./helpers/TrackChangesClient" MockWebApi = require "./helpers/MockWebApi" describe "Getting updates", -> - before (done) -> + before -> @now = Date.now() @to = @now @user_id = ObjectId().toString() - @doc_id = ObjectId().toString() - @project_id = ObjectId().toString() @minutes = 60 * 1000 @@ -27,49 +25,87 @@ describe "Getting updates", -> id: @user_id sinon.spy MockWebApi, "getUser" - @updates = [{ - op: [{ i: "one ", p: 0 }] - meta: { ts: @to - 4 * @minutes, user_id: @user_id } - v: 3 - }, { - op: [{ i: "two ", p: 4 }] - meta: { ts: @to - 2 * @minutes } - v: 4 - }, { - op: [{ i: "three ", p: 8 }] - meta: { ts: @to, user_id: @user_id } - v: @toVersion = 5 - }, { - op: [{ i: "four", p: 14 }] - meta: { ts: @to + 2 * @minutes, user_id: @user_id } - v: 6 - }] - - TrackChangesClient.pushRawUpdates @doc_id, @updates, (error) => - throw error if error? - TrackChangesClient.getUpdates @project_id, @doc_id, { to: @toVersion, limit: 2 }, (error, body) => - throw error if error? - @updates = body.updates - done() - after: () -> MockWebApi.getUser.restore() - it "should fetch the user details from the web api", -> - MockWebApi.getUser - .calledWith(@user_id) - .should.equal true + describe "getting updates in the middle", -> + before (done) -> + @doc_id = ObjectId().toString() + @project_id = ObjectId().toString() + @updates = [{ + op: [{ i: "one ", p: 0 }] + meta: { ts: @to - 20 * @minutes, user_id: @user_id } + v: 3 + }, { + op: [{ i: "two ", p: 4 }] + meta: { ts: @to - 10 * @minutes } + v: 4 + }, { + op: [{ i: "three ", p: 8 }] + meta: { ts: @to, user_id: @user_id } + v: @toVersion = 5 + }, { + op: [{ i: "four", p: 14 }] + meta: { ts: @to + 2 * @minutes, user_id: @user_id } + v: 6 + }] - it "should return the updates", -> - expect(@updates).to.deep.equal [{ - meta: - start_ts: @to - end_ts: @to - user: @user - v: 5 - }, { - meta: - start_ts: @to - 2 * @minutes - end_ts: @to - 2 * @minutes - v: 4 - }] + TrackChangesClient.pushRawUpdates @doc_id, @updates, (error) => + throw error if error? + TrackChangesClient.getUpdates @project_id, @doc_id, { to: @toVersion, limit: 2 }, (error, body) => + throw error if error? + @updates = body.updates + done() + + it "should fetch the user details from the web api", -> + MockWebApi.getUser + .calledWith(@user_id) + .should.equal true + + it "should return the updates", -> + expect(@updates).to.deep.equal [{ + meta: + start_ts: @to + end_ts: @to + users: [@user] + fromV: 5 + toV: 5 + }, { + meta: + users: [] + start_ts: @to - 10 * @minutes + end_ts: @to - 10 * @minutes + toV: 4 + fromV: 4 + }] + + describe "getting updates that should be combined", -> + before (done) -> + @doc_id = ObjectId().toString() + @project_id = ObjectId().toString() + @updates = [{ + op: [{ i: "two ", p: 4 }] + meta: { ts: @to - 2 * @minutes, user_id: @user_id } + v: 4 + }, { + op: [{ i: "three ", p: 8 }] + meta: { ts: @to, user_id: @user_id } + v: @toVersion = 5 + }] + + TrackChangesClient.pushRawUpdates @doc_id, @updates, (error) => + throw error if error? + TrackChangesClient.getUpdates @project_id, @doc_id, { to: @toVersion, limit: 2 }, (error, body) => + throw error if error? + @updates = body.updates + done() + + it "should return the updates", -> + expect(@updates).to.deep.equal [{ + meta: + start_ts: @to - 2 * @minutes + end_ts: @to + users: [@user] + fromV: 4 + toV: 5 + }] diff --git a/services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee index da84db026f..f6ec9bdee8 100644 --- a/services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee +++ b/services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee @@ -16,6 +16,7 @@ describe "HttpController", -> @project_id = "project-id-123" @next = sinon.stub() @user_id = "mock-user-123" + @now = Date.now() describe "flushUpdatesWithLock", -> beforeEach -> @@ -73,12 +74,9 @@ describe "HttpController", -> limit: @limit.toString() @res = send: sinon.stub() - @rawUpdates = [{ - v: @v = 42 - op: @op = "mock-op" - meta: @meta = "mock-meta" - doc_id: @doc_id - }] + @rawUpdates = ["raw-updates"] + @updatesView = ["updates-view"] + @HttpController._buildUpdatesView = sinon.stub().returns(@updatesView) @UpdatesManager.getUpdatesWithUserInfo = sinon.stub().callsArgWith(2, null, @rawUpdates) @HttpController.getUpdates @req, @res, @next @@ -87,13 +85,66 @@ describe "HttpController", -> .calledWith(@doc_id, to: @to, limit: @limit) .should.equal true + it "should build the updates view", -> + @HttpController._buildUpdatesView + .calledWith(@rawUpdates) + .should.equal true + it "should return the formatted updates", -> - updates = for update in @rawUpdates - { - meta: @meta - v: @v - } - @res.send.calledWith(JSON.stringify(updates: updates)).should.equal true + @res.send.calledWith(JSON.stringify(updates: @updatesView)).should.equal true + + describe "_buildUpdatesView", -> + it "should concat updates that are close in time", -> + expect(@HttpController._buildUpdatesView [{ + meta: + user: @user_2 = { id: "mock-user-2" } + start_ts: @now + 20 + end_ts: @now + 30 + v: 5 + }, { + meta: + user: @user_1 = { id: "mock-user-1" } + start_ts: @now + end_ts: @now + 10 + v: 4 + }]).to.deep.equal [{ + meta: + users: [@user_1, @user_2] + start_ts: @now + end_ts: @now + 30 + fromV: 4 + toV: 5 + }] + + it "should leave updates that are far apart in time", -> + oneDay = 1000 * 60 * 60 * 24 + expect(@HttpController._buildUpdatesView [{ + meta: + user: @user_2 = { id: "mock-user-2" } + start_ts: @now + oneDay + end_ts: @now + oneDay + 10 + v: 5 + }, { + meta: + user: @user_1 = { id: "mock-user-2" } + start_ts: @now + end_ts: @now + 10 + v: 4 + }]).to.deep.equal [{ + meta: + users: [@user_2] + start_ts: @now + oneDay + end_ts: @now + oneDay + 10 + fromV: 5 + toV: 5 + }, { + meta: + users: [@user_1] + start_ts: @now + end_ts: @now + 10 + fromV: 4 + toV: 4 + }] describe "RestoreManager", -> beforeEach ->