Bläddra i källkod

Return a summary of the entire project changes

James Allen 12 år sedan
förälder
incheckning
5595910122

+ 1 - 1
services/track-changes/app.coffee

@@ -14,7 +14,7 @@ app.post "/project/:project_id/doc/:doc_id/flush", HttpController.flushUpdatesWi
 
 app.get "/project/:project_id/doc/:doc_id/diff", HttpController.getDiff
 
-app.get "/project/:project_id/doc/:doc_id/updates", HttpController.getUpdates
+app.get "/project/:project_id/updates", HttpController.getUpdates
 
 app.post "/project/:project_id/doc/:doc_id/version/:version/restore", HttpController.restore
 

+ 8 - 7
services/track-changes/app/coffee/HttpController.coffee

@@ -31,17 +31,18 @@ module.exports = HttpController =
 			res.send JSON.stringify(diff: diff)
 
 	getUpdates: (req, res, next = (error) ->) ->
-		doc_id = req.params.doc_id
 		project_id = req.params.project_id
 
-		if req.query.to?
-			to = parseInt(req.query.to, 10)
-		if req.query.limit?
-			limit = parseInt(req.query.limit, 10)
+		if req.query.before?
+			before = parseInt(req.query.before, 10)
+		if req.query.min_count?
+			min_count = parseInt(req.query.min_count, 10)
 
-		UpdatesManager.getSummarizedDocUpdates project_id, doc_id, to: to, limit: limit, (error, updates) ->
+		UpdatesManager.getSummarizedProjectUpdates project_id, before: before, min_count: min_count, (error, updates, nextBeforeTimestamp) ->
 			return next(error) if error?
-			res.send JSON.stringify updates: updates
+			res.send JSON.stringify
+				updates: updates
+				nextBeforeTimestamp: nextBeforeTimestamp
 
 	restore: (req, res, next = (error) ->) ->
 		{doc_id, project_id, version} = req.params

+ 31 - 16
services/track-changes/app/coffee/UpdatesManager.coffee

@@ -86,39 +86,49 @@ module.exports = UpdatesManager =
 				return callback(error) if error?
 				callback null, updates
 
-	getSummarizedDocUpdates: (project_id, doc_id, options = {}, callback = (error, updates) ->) ->
-		options.limit ||= 25
+	getSummarizedProjectUpdates: (project_id, options = {}, callback = (error, updates) ->) ->
+		options.min_count ||= 25
 		summarizedUpdates = []
-		to = options.to
+		before = options.before
 		do fetchNextBatch = () ->
-			UpdatesManager._extendBatchOfSummarizedUpdates project_id, doc_id, summarizedUpdates, to, options.limit, (error, updates, endOfDatabase) ->
+			UpdatesManager._extendBatchOfSummarizedUpdates project_id, summarizedUpdates, before, options.min_count, (error, updates, nextBeforeUpdate) ->
 				return callback(error) if error?
-				if endOfDatabase or updates.length >= options.limit
-					callback null, updates
+				if !nextBeforeUpdate? or updates.length >= options.min_count
+					callback null, updates, nextBeforeUpdate
 				else
-					to = updates[updates.length - 1].fromV - 1
+					before = nextBeforeUpdate
 					summarizedUpdates = updates
 					fetchNextBatch()
 
 	_extendBatchOfSummarizedUpdates: (
-		project_id, 
-		doc_id,
+		project_id,
 		existingSummarizedUpdates,
-		to, desiredLength,
+		before, desiredLength,
 		callback = (error, summarizedUpdates, endOfDatabase) ->
 	) ->
-		UpdatesManager.getDocUpdatesWithUserInfo project_id, doc_id, { to: to, limit: 3 * desiredLength }, (error, updates) ->
+		UpdatesManager.getProjectUpdatesWithUserInfo project_id, { before: before, limit: 3 * desiredLength }, (error, updates) ->
 			return callback(error) if error?
-			if !updates? or updates.length == 0
-				endOfDatabase = true
+
+			# Suppose in this request we have fetch the solid updates. In the next request we need
+			# to fetch the dotted updates. These are defined by having an end timestamp less than
+			# the last update's end timestamp (updates are ordered by descending end_ts). I.e.
+			#                 start_ts--v       v--end_ts
+			#   doc1: |......|  |...|   |-------|
+			#   doc2:     |------------------|
+			#                                ^----- Next time, fetch all updates with an
+			#                                       end_ts less than this
+			#          
+			if updates? and updates.length > 0
+				nextBeforeTimestamp = updates[updates.length - 1].meta.end_ts
 			else
-				endOfDatabase = false
+				nextBeforeTimestamp = null
+
 			summarizedUpdates = UpdatesManager._summarizeUpdates(
 				updates, existingSummarizedUpdates
 			)
 			callback null,
-				summarizedUpdates.slice(0, desiredLength),
-				endOfDatabase
+				summarizedUpdates,
+				nextBeforeTimestamp
 
 	fillUserInfo: (updates, callback = (error, updates) ->) ->
 		users = {}
@@ -165,6 +175,10 @@ module.exports = UpdatesManager =
 							break
 					if !userExists
 						earliestUpdate.meta.users.push update.meta.user
+
+				if update.doc_id.toString() not in earliestUpdate.doc_ids
+					earliestUpdate.doc_ids.push update.doc_id.toString()
+
 				earliestUpdate.meta.start_ts = Math.min(earliestUpdate.meta.start_ts, update.meta.start_ts)
 				earliestUpdate.meta.end_ts   = Math.max(earliestUpdate.meta.end_ts, update.meta.end_ts)
 				earliestUpdate.fromV = update.v
@@ -176,6 +190,7 @@ module.exports = UpdatesManager =
 						end_ts: update.meta.end_ts
 					fromV: update.v
 					toV: update.v
+					doc_ids: [update.doc_id.toString()]
 
 				if update.meta.user?
 					newUpdate.meta.users.push update.meta.user

+ 12 - 5
services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee

@@ -43,14 +43,16 @@ describe "Getting updates", ->
 
 		TrackChangesClient.pushRawUpdates @doc_id, @updates, (error) =>
 			throw error if error?
-			done()
+			TrackChangesClient.flushUpdates @project_id, @doc_id, (error) =>
+				throw error if error?
+				done()
 
 	after: () ->
 		MockWebApi.getUser.restore()
 
 	describe "getting updates up to the limit", ->
 		before (done) ->
-			TrackChangesClient.getUpdates @project_id, @doc_id, { to: 20, limit: 3 }, (error, body) =>
+			TrackChangesClient.getUpdates @project_id, { before: @to + 1, min_count: 3 }, (error, body) =>
 				throw error if error?
 				@updates = body.updates
 				done()
@@ -60,8 +62,9 @@ describe "Getting updates", ->
 				.calledWith(@user_id)
 				.should.equal true
 
-		it "should return the same number of summarized updates as the limit", ->
-			expect(@updates).to.deep.equal [{
+		it "should return at least the min_count number of summarized updates", ->
+			expect(@updates.slice(0,3)).to.deep.equal [{
+				doc_ids: [@doc_id]
 				meta:
 					start_ts: @to - 2 * @minutes
 					end_ts: @to
@@ -69,6 +72,7 @@ describe "Getting updates", ->
 				toV: 20
 				fromV: 19
 			}, {
+				doc_ids: [@doc_id]
 				meta:
 					start_ts: @to - 1 * @days - 2 * @minutes
 					end_ts: @to - 1 * @days
@@ -76,6 +80,7 @@ describe "Getting updates", ->
 				toV: 18
 				fromV: 17
 			}, {
+				doc_ids: [@doc_id]
 				meta:
 					start_ts: @to - 2 * @days - 2 * @minutes
 					end_ts: @to - 2 * @days
@@ -87,13 +92,14 @@ describe "Getting updates", ->
 
 	describe "getting updates beyond the end of the database", ->
 		before (done) ->
-			TrackChangesClient.getUpdates @project_id, @doc_id, { to: 4, limit: 30 }, (error, body) =>
+			TrackChangesClient.getUpdates @project_id, { before: @to - 8 * @days + 1, min_count: 30 }, (error, body) =>
 				throw error if error?
 				@updates = body.updates
 				done()
 
 		it "should return as many updates as it can", ->
 			expect(@updates).to.deep.equal [{
+				doc_ids: [@doc_id]
 				meta:
 					start_ts: @to - 8 * @days - 2 * @minutes
 					end_ts: @to - 8 * @days
@@ -101,6 +107,7 @@ describe "Getting updates", ->
 				toV: 4
 				fromV: 3
 			}, {
+				doc_ids: [@doc_id]
 				meta:
 					start_ts: @to - 9 * @days - 2 * @minutes
 					end_ts: @to - 9 * @days

+ 14 - 6
services/track-changes/test/acceptance/coffee/helpers/TrackChangesClient.coffee

@@ -4,14 +4,22 @@ rclient = require("redis").createClient() # Only works locally for now
 
 module.exports = TrackChangesClient =
 	flushAndGetCompressedUpdates: (project_id, doc_id, callback = (error, updates) ->) ->
+		TrackChangesClient.flushUpdates project_id, doc_id, (error) ->
+			return callback(error) if error?
+			TrackChangesClient.getCompressedUpdates doc_id, callback
+
+	flushUpdates: (project_id, doc_id, callback = (error) ->) ->
 		request.post {
 			url: "http://localhost:3015/project/#{project_id}/doc/#{doc_id}/flush"
 		}, (error, response, body) =>
 			response.statusCode.should.equal 204
-			db.docHistory
-				.find(doc_id: ObjectId(doc_id))
-				.sort("meta.end_ts": 1)
-				.toArray callback
+			callback(error)
+
+	getCompressedUpdates: (doc_id, callback = (error, updates) ->) ->
+		db.docHistory
+			.find(doc_id: ObjectId(doc_id))
+			.sort("meta.end_ts": 1)
+			.toArray callback
 
 	pushRawUpdates: (doc_id, updates, callback = (error) ->) ->
 		rclient.rpush "UncompressedHistoryOps:#{doc_id}", (JSON.stringify(u) for u in updates)..., callback
@@ -23,9 +31,9 @@ module.exports = TrackChangesClient =
 			response.statusCode.should.equal 200
 			callback null, JSON.parse(body)
 
-	getUpdates: (project_id, doc_id, options, callback = (error, body) ->) ->
+	getUpdates: (project_id, options, callback = (error, body) ->) ->
 		request.get {
-			url: "http://localhost:3015/project/#{project_id}/doc/#{doc_id}/updates?to=#{options.to}&limit=#{options.limit}"
+			url: "http://localhost:3015/project/#{project_id}/updates?before=#{options.before}&min_count=#{options.min_count}"
 		}, (error, response, body) =>
 			response.statusCode.should.equal 200
 			callback null, JSON.parse(body)

+ 9 - 9
services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee

@@ -64,28 +64,28 @@ describe "HttpController", ->
 
 	describe "getUpdates", ->
 		beforeEach ->
-			@to = 42
-			@limit = 10
+			@before = Date.now()
+			@nextBeforeTimestamp = @before - 100
+			@min_count = 10
 			@req =
 				params:
-					doc_id: @doc_id
 					project_id: @project_id
 				query:
-					to:    @to.toString()
-					limit: @limit.toString()
+					before:    @before.toString()
+					min_count: @min_count.toString()
 			@res =
 				send: sinon.stub()
 			@updates = ["mock-summarized-updates"]
-			@UpdatesManager.getSummarizedDocUpdates = sinon.stub().callsArgWith(3, null, @updates)
+			@UpdatesManager.getSummarizedProjectUpdates = sinon.stub().callsArgWith(2, null, @updates, @nextBeforeTimestamp)
 			@HttpController.getUpdates @req, @res, @next
 
 		it "should get the updates", ->
-			@UpdatesManager.getSummarizedDocUpdates
-				.calledWith(@project_id, @doc_id, to: @to, limit: @limit)
+			@UpdatesManager.getSummarizedProjectUpdates
+				.calledWith(@project_id, before: @before, min_count: @min_count)
 				.should.equal true
 
 		it "should return the formatted updates", ->
-			@res.send.calledWith(JSON.stringify(updates: @updates)).should.equal true
+			@res.send.calledWith(JSON.stringify(updates: @updates, nextBeforeTimestamp: @nextBeforeTimestamp)).should.equal true
 
 	describe "RestoreManager", ->
 		beforeEach ->

+ 61 - 42
services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee

@@ -284,23 +284,26 @@ describe "UpdatesManager", ->
 
 	describe "_extendBatchOfSummarizedUpdates", ->
 		beforeEach ->
-			@to = 42
-			@limit = 2
+			@before = Date.now()
+			@min_count = 2
 			@existingSummarizedUpdates = ["summarized-updates-3"]
 			@summarizedUpdates = ["summarized-updates-3", "summarized-update-2", "summarized-update-1"]
 
 		describe "when there are updates to get", ->
 			beforeEach ->
-				@updates = ["mock-updates"]
+				@updates = [
+					{op: "mock-op-1", meta: end_ts: @before - 10},
+					{op: "mock-op-1", meta: end_ts: @nextBeforeTimestamp = @before - 20}
+				]
 				@existingSummarizedUpdates = ["summarized-updates-3"]
 				@summarizedUpdates = ["summarized-updates-3", "summarized-update-2", "summarized-update-1"]
 				@UpdatesManager._summarizeUpdates = sinon.stub().returns(@summarizedUpdates)
-				@UpdatesManager.getDocUpdatesWithUserInfo = sinon.stub().callsArgWith(3, null, @updates)
-				@UpdatesManager._extendBatchOfSummarizedUpdates @project_id, @doc_id, @existingSummarizedUpdates, @to, @limit, @callback
+				@UpdatesManager.getProjectUpdatesWithUserInfo = sinon.stub().callsArgWith(2, null, @updates)
+				@UpdatesManager._extendBatchOfSummarizedUpdates @project_id, @existingSummarizedUpdates, @before, @min_count, @callback
 
 			it "should get the updates", ->
-				@UpdatesManager.getDocUpdatesWithUserInfo
-					.calledWith(@project_id, @doc_id, { to: @to, limit: 3 * @limit })
+				@UpdatesManager.getProjectUpdatesWithUserInfo
+					.calledWith(@project_id, { before: @before, limit: 3 * @min_count })
 					.should.equal true
 
 			it "should summarize the updates", ->
@@ -308,78 +311,81 @@ describe "UpdatesManager", ->
 					.calledWith(@updates, @existingSummarizedUpdates)
 					.should.equal true
 
-			it "should call the callback with the summarized updates and false for end-of-databse", ->
-				@callback.calledWith(null, @summarizedUpdates.slice(0, @limit), false).should.equal true
+			it "should call the callback with the summarized updates and the next before timestamp", ->
+				@callback.calledWith(null, @summarizedUpdates, @nextBeforeTimestamp).should.equal true
 
 		describe "when there are no more updates", ->
 			beforeEach ->
 				@updates = []
 				@UpdatesManager._summarizeUpdates = sinon.stub().returns(@summarizedUpdates)
-				@UpdatesManager.getDocUpdatesWithUserInfo = sinon.stub().callsArgWith(3, null, @updates)
-				@UpdatesManager._extendBatchOfSummarizedUpdates @project_id, @doc_id, @existingSummarizedUpdates, @to, @limit, @callback
+				@UpdatesManager.getProjectUpdatesWithUserInfo = sinon.stub().callsArgWith(2, null, @updates)
+				@UpdatesManager._extendBatchOfSummarizedUpdates @project_id, @existingSummarizedUpdates, @before, @min_count, @callback
 
-			it "should call the callback with the summarized updates and true for end-of-database", ->
-				@callback.calledWith(null, @summarizedUpdates.slice(0, @limit), true).should.equal true
+			it "should call the callback with the summarized updates and null for nextBeforeTimestamp", ->
+				@callback.calledWith(null, @summarizedUpdates, null).should.equal true
 
-	describe "getSummarizedDocUpdates", ->
+	describe "getSummarizedProjectUpdates", ->
 		describe "when one batch of updates is enough to meet the limit", ->
 			beforeEach ->
-				@to = 42
-				@limit = 2
+				@before = Date.now()
+				@min_count = 2
 				@updates = ["summarized-updates-3", "summarized-updates-2"]
-				@UpdatesManager._extendBatchOfSummarizedUpdates = sinon.stub().callsArgWith(5, null, @updates)
-				@UpdatesManager.getSummarizedDocUpdates @project_id, @doc_id, { to: @to, limit: @limit }, @callback
+				@nextBeforeTimestamp = @before - 100
+				@UpdatesManager._extendBatchOfSummarizedUpdates = sinon.stub().callsArgWith(4, null, @updates, @nextBeforeTimestamp)
+				@UpdatesManager.getSummarizedProjectUpdates @project_id, { before: @before, min_count: @min_count }, @callback
 
 			it "should get the batch of summarized updates", ->
 				@UpdatesManager._extendBatchOfSummarizedUpdates
-					.calledWith(@project_id, @doc_id, [], @to, @limit)
+					.calledWith(@project_id, [], @before, @min_count)
 					.should.equal true
 
 			it "should call the callback with the updates", ->
-				@callback.calledWith(null, @updates).should.equal true
+				@callback.calledWith(null, @updates, @nextBeforeTimestamp).should.equal true
 
 		describe "when multiple batches are needed to meet the limit", ->
 			beforeEach ->
-				@to = 6
-				@limit = 4
+				@before = Date.now()
+				@min_count = 4
 				@firstBatch =  [{ toV: 6, fromV: 6 }, { toV: 5, fromV: 5 }]
+				@nextBeforeTimestamp = @before - 100
 				@secondBatch = [{ toV: 4, fromV: 4 }, { toV: 3, fromV: 3 }]
-				@UpdatesManager._extendBatchOfSummarizedUpdates = (project_id, doc_id, existingUpdates, to, limit, callback) =>
+				@nextNextBeforeTimestamp = @before - 200
+				@UpdatesManager._extendBatchOfSummarizedUpdates = (project_id, existingUpdates, before, desiredLength, callback) =>
 					if existingUpdates.length == 0
-						callback null, @firstBatch, false
+						callback null, @firstBatch, @nextBeforeTimestamp
 					else
-						callback null, @firstBatch.concat(@secondBatch), false
+						callback null, @firstBatch.concat(@secondBatch), @nextNextBeforeTimestamp
 				sinon.spy @UpdatesManager, "_extendBatchOfSummarizedUpdates"
-				@UpdatesManager.getSummarizedDocUpdates @project_id, @doc_id, { to: @to, limit: @limit }, @callback
+				@UpdatesManager.getSummarizedProjectUpdates @project_id, { before: @before, min_count: @min_count }, @callback
 
 			it "should get the first batch of summarized updates", ->
 				@UpdatesManager._extendBatchOfSummarizedUpdates
-					.calledWith(@project_id, @doc_id, [], @to, @limit)
+					.calledWith(@project_id, [], @before, @min_count)
 					.should.equal true
 
 			it "should get the second batch of summarized updates", ->
 				@UpdatesManager._extendBatchOfSummarizedUpdates
-					.calledWith(@project_id, @doc_id, @firstBatch, 4, @limit)
+					.calledWith(@project_id, @firstBatch, @nextBeforeTimestamp, @min_count)
 					.should.equal true
 
 			it "should call the callback with all the updates", ->
-				@callback.calledWith(null, @firstBatch.concat(@secondBatch)).should.equal true
+				@callback.calledWith(null, @firstBatch.concat(@secondBatch), @nextNextBeforeTimestamp).should.equal true
 
 		describe "when the end of the database is hit", ->
 			beforeEach ->
-				@to = 6
-				@limit = 4
+				@before = Date.now()
+				@min_count = 4
 				@updates =  [{ toV: 6, fromV: 6 }, { toV: 5, fromV: 5 }]
-				@UpdatesManager._extendBatchOfSummarizedUpdates = sinon.stub().callsArgWith(5, null, @updates, true)
-				@UpdatesManager.getSummarizedDocUpdates @project_id, @doc_id, { to: @to, limit: @limit }, @callback
+				@UpdatesManager._extendBatchOfSummarizedUpdates = sinon.stub().callsArgWith(4, null, @updates, null)
+				@UpdatesManager.getSummarizedProjectUpdates @project_id, { before: @before, min_count: @min_count }, @callback
 
 			it "should get the batch of summarized updates", ->
 				@UpdatesManager._extendBatchOfSummarizedUpdates
-					.calledWith(@project_id, @doc_id, [], @to, @limit)
+					.calledWith(@project_id, [], @before, @min_count)
 					.should.equal true
 
 			it "should call the callback with the updates", ->
-				@callback.calledWith(null, @updates).should.equal true
+				@callback.calledWith(null, @updates, null).should.equal true
 
 	describe "fillUserInfo", ->
 		describe "with valid users", ->
@@ -469,24 +475,31 @@ describe "UpdatesManager", ->
 					op: "mock-op-2"
 				}]
 
-	describe "_buildUpdatesView", ->
+	describe "_summarizeUpdates", ->
 		beforeEach ->
 			@now = Date.now()
+			@user_1 = { id: "mock-user-1" }
+			@user_2 = { id: "mock-user-2" }
+			@doc_id_1 = "mock-doc-id-1"
+			@doc_id_2 = "mock-doc-id-2"
 
 		it "should concat updates that are close in time", ->
 			expect(@UpdatesManager._summarizeUpdates [{
+				doc_id: @doc_id_1
 				meta:
-					user: @user_1 = { id: "mock-user-1" }
+					user: @user_1
 					start_ts: @now + 20
 					end_ts:   @now + 30
 				v: 5
 			}, {
+				doc_id: @doc_id_1
 				meta:
-					user: @user_2 = { id: "mock-user-2" }
+					user: @user_2
 					start_ts: @now
 					end_ts:   @now + 10
 				v: 4
 			}]).to.deep.equal [{
+				doc_ids: [@doc_id_1]
 				meta:
 					users: [@user_1, @user_2]
 					start_ts: @now
@@ -498,18 +511,21 @@ describe "UpdatesManager", ->
 		it "should leave updates that are far apart in time", ->
 			oneDay = 1000 * 60 * 60 * 24
 			expect(@UpdatesManager._summarizeUpdates [{
+				doc_id: @doc_id_1
 				meta:
-					user: @user_2 = { id: "mock-user-2" }
+					user: @user_2
 					start_ts: @now + oneDay
 					end_ts:   @now + oneDay + 10
 				v: 5
 			}, {
+				doc_id: @doc_id_1
 				meta:
-					user: @user_1 = { id: "mock-user-2" }
+					user: @user_1
 					start_ts: @now
 					end_ts:   @now + 10
 				v: 4
 			}]).to.deep.equal [{
+				doc_ids: [@doc_id_1]
 				meta:
 					users: [@user_2]
 					start_ts: @now + oneDay
@@ -517,6 +533,7 @@ describe "UpdatesManager", ->
 				fromV: 5
 				toV: 5
 			}, {
+				doc_ids: [@doc_id_1]
 				meta:
 					users: [@user_1]
 					start_ts: @now
@@ -526,21 +543,22 @@ describe "UpdatesManager", ->
 			}]
 
 		it "should concat onto existing summarized updates", ->
-			@user_1 = { id: "mock-user-1" }
-			@user_2 = { id: "mock-user-2" }
 			expect(@UpdatesManager._summarizeUpdates [{
+				doc_id: @doc_id_2
 				meta:
 					user: @user_1
 					start_ts: @now + 20
 					end_ts:   @now + 30
 				v: 5
 			}, {
+				doc_id: @doc_id_2
 				meta:
 					user: @user_2
 					start_ts: @now
 					end_ts:   @now + 10
 				v: 4
 			}], [{
+				doc_ids: [@doc_id_1]
 				meta:
 					users: [@user_1]
 					start_ts: @now + 40
@@ -548,6 +566,7 @@ describe "UpdatesManager", ->
 				fromV: 6
 				toV: 8
 			}]).to.deep.equal [{
+				doc_ids: [@doc_id_1, @doc_id_2]
 				meta:
 					users: [@user_1, @user_2]
 					start_ts: @now