Просмотр исходного кода

Don't store ops in Mongo, just keep them in redis with a timeout

James Allen 12 лет назад
Родитель
Сommit
7a977ca597
23 измененных файлов с 149 добавлено и 630 удалено
  1. 2 2
      services/document-updater/Gruntfile.coffee
  2. 3 119
      services/document-updater/app/coffee/DocOpsManager.coffee
  3. 7 11
      services/document-updater/app/coffee/DocumentManager.coffee
  4. 3 2
      services/document-updater/app/coffee/PersistenceManager.coffee
  5. 15 14
      services/document-updater/app/coffee/RedisManager.coffee
  6. 0 7
      services/document-updater/app/coffee/mongojs.coffee
  7. 7 59
      services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee
  8. 4 1
      services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee
  9. 1 0
      services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee
  10. 1 0
      services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee
  11. 16 37
      services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee
  12. 4 2
      services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee
  13. 12 3
      services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee
  14. 3 1
      services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee
  15. 11 5
      services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee
  16. 0 4
      services/document-updater/test/unit/coffee/AddingDocsToMemory.coffee
  17. 1 280
      services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee
  18. 3 9
      services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee
  19. 1 7
      services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee
  20. 4 3
      services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee
  21. 7 5
      services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee
  22. 0 32
      services/document-updater/test/unit/coffee/RedisManager/prependDocOpsTests.coffee
  23. 44 27
      services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee

+ 2 - 2
services/document-updater/Gruntfile.coffee

@@ -45,7 +45,7 @@ module.exports = (grunt) ->
 
 
 		clean:
 		clean:
 			app: ["app/js"]
 			app: ["app/js"]
-			acceptance_tests: ["test/unit/js"]
+			acceptance_tests: ["test/acceptance/js"]
 
 
 		mochaTest:
 		mochaTest:
 			unit:
 			unit:
@@ -103,7 +103,7 @@ module.exports = (grunt) ->
 
 
 	grunt.registerTask 'install', "Compile everything when installing as an npm module", ['compile']
 	grunt.registerTask 'install', "Compile everything when installing as an npm module", ['compile']
 
 
-	grunt.registerTask 'test:unit', 'Run the unit tests (use --grep=<regex> for individual tests)', ['compile:unit_tests', 'mochaTest:unit']
+	grunt.registerTask 'test:unit', 'Run the unit tests (use --grep=<regex> for individual tests)', ['compile:server', 'compile:unit_tests', 'mochaTest:unit']
 	grunt.registerTask 'test:acceptance', 'Run the acceptance tests (use --grep=<regex> for individual tests)', ['compile:acceptance_tests', 'mochaTest:acceptance']
 	grunt.registerTask 'test:acceptance', 'Run the acceptance tests (use --grep=<regex> for individual tests)', ['compile:acceptance_tests', 'mochaTest:acceptance']
 
 
 	grunt.registerTask 'run', "Compile and run the document-updater-sharelatex server", ['compile', 'bunyan', 'execute']
 	grunt.registerTask 'run', "Compile and run the document-updater-sharelatex server", ['compile', 'bunyan', 'execute']

+ 3 - 119
services/document-updater/app/coffee/DocOpsManager.coffee

@@ -1,127 +1,11 @@
 RedisManager = require "./RedisManager"
 RedisManager = require "./RedisManager"
-mongojs = require("./mongojs")
-db = mongojs.db
-ObjectId = mongojs.ObjectId
-logger = require "logger-sharelatex"
-async = require "async"
-Metrics = require("./Metrics")
 
 
 module.exports = DocOpsManager =
 module.exports = DocOpsManager =
-	flushDocOpsToMongo: (project_id, doc_id, _callback = (error) ->) ->
-		timer = new Metrics.Timer("docOpsManager.flushDocOpsToMongo")
-		callback = (args...) ->
-			timer.done()
-			_callback(args...)
-
-		DocOpsManager.getDocVersionInMongo doc_id, (error, mongoVersion) ->
+	getPreviousDocOps: (project_id, doc_id, start, end, callback = (error, ops) ->) ->
+		RedisManager.getPreviousDocOps doc_id, start, end, (error, ops) ->
 			return callback(error) if error?
 			return callback(error) if error?
-			RedisManager.getDocVersion doc_id, (error, redisVersion) ->
-				return callback(error) if error?
-				if !mongoVersion? or !redisVersion? or mongoVersion > redisVersion
-					logger.error doc_id: doc_id, redisVersion: redisVersion, mongoVersion: mongoVersion, "mongo version is ahead of redis"
-					return callback(new Error("inconsistent versions"))
-
-				RedisManager.getPreviousDocOps doc_id, mongoVersion, -1, (error, ops) ->
-					return callback(error) if error?
-					if ops.length != redisVersion - mongoVersion
-						logger.error doc_id: doc_id, redisVersion: redisVersion, mongoVersion: mongoVersion, opsLength: ops.length, "version difference does not match ops length"
-						return callback(new Error("inconsistent versions"))
-					logger.log doc_id: doc_id, redisVersion: redisVersion, mongoVersion: mongoVersion, "flushing doc ops to mongo"
-					DocOpsManager._appendDocOpsInMongo doc_id, ops, redisVersion, (error) ->
-						return callback(error) if error?
-						callback null
-
-	getPreviousDocOps: (project_id, doc_id, start, end, _callback = (error, ops) ->) ->
-		timer = new Metrics.Timer("docOpsManager.getPreviousDocOps")
-		callback = (args...) ->
-			timer.done()
-			_callback(args...)
-
-		DocOpsManager._ensureOpsAreLoaded project_id, doc_id, start, (error) ->
-			return callback(error) if error?
-			RedisManager.getPreviousDocOps doc_id, start, end, (error, ops) ->
-				return callback(error) if error?
-				callback null, ops
+			callback null, ops
 
 
 	pushDocOp: (project_id, doc_id, op, callback = (error) ->) ->
 	pushDocOp: (project_id, doc_id, op, callback = (error) ->) ->
 		RedisManager.pushDocOp doc_id, op, callback
 		RedisManager.pushDocOp doc_id, op, callback
 
 
-	_ensureOpsAreLoaded: (project_id, doc_id, backToVersion, callback = (error) ->) ->
-		RedisManager.getDocVersion doc_id, (error, redisVersion) ->
-			return callback(error) if error?
-			RedisManager.getDocOpsLength doc_id, (error, opsLength) ->
-				return callback(error) if error?
-				oldestVersionInRedis = redisVersion - opsLength
-				if oldestVersionInRedis > backToVersion
-					# _getDocOpsFromMongo(<id>, 4, 6, ...) will return the ops in positions 4 and 5, but not 6.
-					logger.log doc_id: doc_id, backToVersion: backToVersion, oldestVersionInRedis: oldestVersionInRedis, "loading old ops from mongo"
-					DocOpsManager._getDocOpsFromMongo doc_id, backToVersion, oldestVersionInRedis, (error, ops) ->
-						logger.log doc_id: doc_id, backToVersion: backToVersion, oldestVersionInRedis: oldestVersionInRedis, ops: ops, "loaded old ops from mongo"
-						return callback(error) if error?
-						RedisManager.prependDocOps doc_id, ops, (error) ->
-							return callback(error) if error?
-							callback null
-				else
-					logger.log doc_id: doc_id, backToVersion: backToVersion, oldestVersionInRedis: oldestVersionInRedis, "ops already in redis"
-					callback()
-
-	getDocVersionInMongo: (doc_id, callback = (error, version) ->) ->
-		t = new Metrics.Timer("mongo-time")
-		db.docOps.find {
-			doc_id: ObjectId(doc_id)
-		}, {
-			version: 1
-		}, (error, docs) ->
-			t.done()
-			return callback(error) if error?
-			if docs.length < 1 or !docs[0].version?
-				return callback null, 0
-			else
-				return callback null, docs[0].version
-
-	APPEND_OPS_BATCH_SIZE: 100
-
-	_appendDocOpsInMongo: (doc_id, docOps, newVersion, callback = (error) ->) ->
-		currentVersion = newVersion - docOps.length
-		batchSize = DocOpsManager.APPEND_OPS_BATCH_SIZE
-		noOfBatches = Math.ceil(docOps.length / batchSize)
-		if noOfBatches <= 0
-			return callback()
-		jobs = []
-		for batchNo in [0..(noOfBatches-1)]
-			do (batchNo) ->
-				jobs.push (callback) ->
-					batch = docOps.slice(batchNo * batchSize, (batchNo + 1) * batchSize)
-					currentVersion += batch.length
-					logger.log doc_id: doc_id, batchNo: batchNo, "appending doc op batch to Mongo"
-					t = new Metrics.Timer("mongo-time")
-					db.docOps.update {
-						doc_id: ObjectId(doc_id)
-					}, {
-						$push: docOps: { $each: batch, $slice: -100 }
-						$set: version: currentVersion
-					}, {
-						upsert: true
-					}, (err)->
-						t.done()
-						callback(err)
-
-		async.series jobs, (error) -> callback(error)
-
-	_getDocOpsFromMongo: (doc_id, start, end, callback = (error, ops) ->) ->
-		DocOpsManager.getDocVersionInMongo doc_id, (error, version) ->
-			return callback(error) if error?
-			offset = - (version - start) # Negative tells mongo to count from the end backwards
-			limit = end - start
-			t = new Metrics.Timer("mongo-time")
-			db.docOps.find {
-				doc_id: ObjectId(doc_id)
-			}, {
-				docOps: $slice: [offset, limit]
-			}, (error, docs) ->
-				t.done()
-				if docs.length < 1 or !docs[0].docOps?
-					return callback null, []
-				else
-					return callback null, docs[0].docOps
-		

+ 7 - 11
services/document-updater/app/coffee/DocumentManager.coffee

@@ -16,14 +16,12 @@ module.exports = DocumentManager =
 			return callback(error) if error?
 			return callback(error) if error?
 			if !lines? or !version?
 			if !lines? or !version?
 				logger.log project_id: project_id, doc_id: doc_id, "doc not in redis so getting from persistence API"
 				logger.log project_id: project_id, doc_id: doc_id, "doc not in redis so getting from persistence API"
-				PersistenceManager.getDoc project_id, doc_id, (error, lines) ->
+				PersistenceManager.getDoc project_id, doc_id, (error, lines, version) ->
 					return callback(error) if error?
 					return callback(error) if error?
-					DocOpsManager.getDocVersionInMongo doc_id, (error, version) ->
+					logger.log project_id: project_id, doc_id: doc_id, lines: lines, version: version, "got doc from persistence API"
+					RedisManager.putDocInMemory project_id, doc_id, lines, version, (error) ->
 						return callback(error) if error?
 						return callback(error) if error?
-						logger.log project_id: project_id, doc_id: doc_id, lines: lines, version: version, "got doc from persistence API"
-						RedisManager.putDocInMemory project_id, doc_id, lines, version, (error) ->
-							return callback(error) if error?
-							callback null, lines, version
+						callback null, lines, version
 			else
 			else
 				callback null, lines, version
 				callback null, lines, version
 
 
@@ -87,12 +85,10 @@ module.exports = DocumentManager =
 				logger.log project_id: project_id, doc_id: doc_id, "doc is not loaded so not flushing"
 				logger.log project_id: project_id, doc_id: doc_id, "doc is not loaded so not flushing"
 				callback null
 				callback null
 			else
 			else
-				logger.log project_id: project_id, doc_id: doc_id, "flushing doc"
-				PersistenceManager.setDoc project_id, doc_id, lines, (error) ->
+				logger.log project_id: project_id, doc_id: doc_id, version: version, "flushing doc"
+				PersistenceManager.setDoc project_id, doc_id, lines, version, (error) ->
 					return callback(error) if error?
 					return callback(error) if error?
-					DocOpsManager.flushDocOpsToMongo project_id, doc_id, (error) ->
-						return callback(error) if error?
-						callback null
+					callback null
 
 
 	flushAndDeleteDoc: (project_id, doc_id, _callback = (error) ->) ->
 	flushAndDeleteDoc: (project_id, doc_id, _callback = (error) ->) ->
 		timer = new Metrics.Timer("docManager.flushAndDeleteDoc")
 		timer = new Metrics.Timer("docManager.flushAndDeleteDoc")

+ 3 - 2
services/document-updater/app/coffee/PersistenceManager.coffee

@@ -28,13 +28,13 @@ module.exports = PersistenceManager =
 					body = JSON.parse body
 					body = JSON.parse body
 				catch e
 				catch e
 					return callback(e)
 					return callback(e)
-				return callback null, body.lines
+				return callback null, body.lines, body.version
 			else if res.statusCode == 404
 			else if res.statusCode == 404
 				return callback(new Errors.NotFoundError("doc not not found: #{url}"))
 				return callback(new Errors.NotFoundError("doc not not found: #{url}"))
 			else
 			else
 				return callback(new Error("error accessing web API: #{url} #{res.statusCode}"))
 				return callback(new Error("error accessing web API: #{url} #{res.statusCode}"))
 
 
-	setDoc: (project_id, doc_id, lines, _callback = (error) ->) ->
+	setDoc: (project_id, doc_id, lines, version, _callback = (error) ->) ->
 		timer = new Metrics.Timer("persistenceManager.setDoc")
 		timer = new Metrics.Timer("persistenceManager.setDoc")
 		callback = (args...) ->
 		callback = (args...) ->
 			timer.done()
 			timer.done()
@@ -46,6 +46,7 @@ module.exports = PersistenceManager =
 			method: "POST"
 			method: "POST"
 			body: JSON.stringify
 			body: JSON.stringify
 				lines: lines
 				lines: lines
+				version: parseInt(version, 10)
 			headers:
 			headers:
 				"content-type": "application/json"
 				"content-type": "application/json"
 			auth:
 			auth:

+ 15 - 14
services/document-updater/app/coffee/RedisManager.coffee

@@ -9,7 +9,10 @@ keys = require('./RedisKeyBuilder')
 logger = require('logger-sharelatex')
 logger = require('logger-sharelatex')
 metrics = require('./Metrics')
 metrics = require('./Metrics')
 
 
-module.exports =
+# Make times easy to read
+minutes = 60 # seconds for Redis expire
+
+module.exports = RedisManager =
 	putDocInMemory : (project_id, doc_id, docLines, version, callback)->
 	putDocInMemory : (project_id, doc_id, docLines, version, callback)->
 		timer = new metrics.Timer("redis.put-doc")
 		timer = new metrics.Timer("redis.put-doc")
 		logger.log project_id:project_id, doc_id:doc_id, docLines:docLines, version: version, "putting doc in redis"
 		logger.log project_id:project_id, doc_id:doc_id, docLines:docLines, version: version, "putting doc in redis"
@@ -17,7 +20,6 @@ module.exports =
 		multi.set keys.docLines(doc_id:doc_id), JSON.stringify(docLines)
 		multi.set keys.docLines(doc_id:doc_id), JSON.stringify(docLines)
 		multi.set keys.projectKey({doc_id:doc_id}), project_id
 		multi.set keys.projectKey({doc_id:doc_id}), project_id
 		multi.set keys.docVersion(doc_id:doc_id), version
 		multi.set keys.docVersion(doc_id:doc_id), version
-		multi.del keys.docOps(doc_id:doc_id)
 		multi.sadd keys.allDocs, doc_id
 		multi.sadd keys.allDocs, doc_id
 		multi.sadd keys.docsInProject(project_id:project_id), doc_id
 		multi.sadd keys.docsInProject(project_id:project_id), doc_id
 		multi.exec (err, replys)->
 		multi.exec (err, replys)->
@@ -31,7 +33,6 @@ module.exports =
 		multi.del keys.docLines(doc_id:doc_id)
 		multi.del keys.docLines(doc_id:doc_id)
 		multi.del keys.projectKey(doc_id:doc_id)
 		multi.del keys.projectKey(doc_id:doc_id)
 		multi.del keys.docVersion(doc_id:doc_id)
 		multi.del keys.docVersion(doc_id:doc_id)
-		multi.del keys.docOps(doc_id:doc_id)
 		multi.srem keys.docsInProject(project_id:project_id), doc_id
 		multi.srem keys.docsInProject(project_id:project_id), doc_id
 		multi.srem keys.allDocs, doc_id
 		multi.srem keys.allDocs, doc_id
 		multi.exec (err, replys)->
 		multi.exec (err, replys)->
@@ -111,7 +112,6 @@ module.exports =
 		rclient.srem keys.docsWithPendingUpdates, doc_key, callback
 		rclient.srem keys.docsWithPendingUpdates, doc_key, callback
 
 
 	getPreviousDocOps: (doc_id, start, end, callback = (error, jsonOps) ->) ->
 	getPreviousDocOps: (doc_id, start, end, callback = (error, jsonOps) ->) ->
-		# TODO: parse the ops and return them as objects, not JSON
 		rclient.llen keys.docOps(doc_id: doc_id), (error, length) ->
 		rclient.llen keys.docOps(doc_id: doc_id), (error, length) ->
 			return callback(error) if error?
 			return callback(error) if error?
 			rclient.get keys.docVersion(doc_id: doc_id), (error, version) ->
 			rclient.get keys.docVersion(doc_id: doc_id), (error, version) ->
@@ -141,19 +141,20 @@ module.exports =
 						return callback(e)
 						return callback(e)
 					callback null, ops
 					callback null, ops
 
 
+	DOC_OPS_TTL: 60 * minutes
+	DOC_OPS_MAX_LENGTH: 100
 	pushDocOp: (doc_id, op, callback = (error, new_version) ->) ->
 	pushDocOp: (doc_id, op, callback = (error, new_version) ->) ->
-		# TODO: take a raw op object and JSONify it here
 		jsonOp = JSON.stringify op
 		jsonOp = JSON.stringify op
-		rclient.rpush keys.docOps(doc_id: doc_id), jsonOp, (error) ->
+		multi = rclient.multi()
+		multi.rpush  keys.docOps(doc_id: doc_id), jsonOp
+		multi.expire keys.docOps(doc_id: doc_id), RedisManager.DOC_OPS_TTL
+		multi.ltrim  keys.docOps(doc_id: doc_id), -RedisManager.DOC_OPS_MAX_LENGTH, -1
+		multi.incr   keys.docVersion(doc_id: doc_id)
+		multi.exec (error, replys) ->
+			[_, __, ___, version] = replys
 			return callback(error) if error?
 			return callback(error) if error?
-			rclient.incr keys.docVersion(doc_id: doc_id), (error, version) ->
-				return callback(error) if error?
-				version = parseInt(version, 10)
-				callback null, version
-
-	prependDocOps: (doc_id, ops, callback = (error) ->) ->
-		jsonOps = ops.map (op) -> JSON.stringify op
-		rclient.lpush keys.docOps(doc_id: doc_id), jsonOps.reverse(), callback
+			version = parseInt(version, 10)
+			callback null, version
 
 
 	getDocOpsLength: (doc_id, callback = (error, length) ->) ->
 	getDocOpsLength: (doc_id, callback = (error, length) ->) ->
 		rclient.llen keys.docOps(doc_id: doc_id), callback
 		rclient.llen keys.docOps(doc_id: doc_id), callback

+ 0 - 7
services/document-updater/app/coffee/mongojs.coffee

@@ -1,7 +0,0 @@
-Settings = require "settings-sharelatex"
-mongojs = require "mongojs"
-db = mongojs.connect(Settings.mongo.url, ["docOps"])
-module.exports =
-	db: db
-	ObjectId: mongojs.ObjectId
-

+ 7 - 59
services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee

@@ -2,9 +2,6 @@ sinon = require "sinon"
 chai = require("chai")
 chai = require("chai")
 chai.should()
 chai.should()
 async = require "async"
 async = require "async"
-mongojs = require "../../../app/js/mongojs"
-db = mongojs.db
-ObjectId = mongojs.ObjectId
 
 
 MockWebApi = require "./helpers/MockWebApi"
 MockWebApi = require "./helpers/MockWebApi"
 DocUpdaterClient = require "./helpers/DocUpdaterClient"
 DocUpdaterClient = require "./helpers/DocUpdaterClient"
@@ -12,13 +9,14 @@ DocUpdaterClient = require "./helpers/DocUpdaterClient"
 describe "Applying updates to a doc", ->
 describe "Applying updates to a doc", ->
 	before ->
 	before ->
 		@lines = ["one", "two", "three"]
 		@lines = ["one", "two", "three"]
+		@version = 42
 		@update =
 		@update =
 			doc: @doc_id
 			doc: @doc_id
 			op: [{
 			op: [{
 				i: "one and a half\n"
 				i: "one and a half\n"
 				p: 4
 				p: 4
 			}]
 			}]
-			v: 0
+			v: @version
 		@result = ["one", "one and a half", "two", "three"]
 		@result = ["one", "one and a half", "two", "three"]
 
 
 	describe "when the document is not loaded", ->
 	describe "when the document is not loaded", ->
@@ -26,6 +24,7 @@ describe "Applying updates to a doc", ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines
 				lines: @lines
+				version: @version
 			}
 			}
 			sinon.spy MockWebApi, "getDocument"
 			sinon.spy MockWebApi, "getDocument"
 			DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) ->
 			DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) ->
@@ -50,6 +49,7 @@ describe "Applying updates to a doc", ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines
 				lines: @lines
+				version: @version
 			}
 			}
 			DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
 			DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
 				throw error if error?
 				throw error if error?
@@ -76,6 +76,7 @@ describe "Applying updates to a doc", ->
 				@lines = ["", "", ""]
 				@lines = ["", "", ""]
 				MockWebApi.insertDoc @project_id, @doc_id, {
 				MockWebApi.insertDoc @project_id, @doc_id, {
 					lines: @lines
 					lines: @lines
+					version: 0
 				}
 				}
 
 
 				@updates = [
 				@updates = [
@@ -92,9 +93,6 @@ describe "Applying updates to a doc", ->
 					{ doc_id: @doc_id, v: 10, op: [i: "d", p: 10] }
 					{ doc_id: @doc_id, v: 10, op: [i: "d", p: 10] }
 				]
 				]
 				@result = ["hello world", "", ""]
 				@result = ["hello world", "", ""]
-				MockWebApi.insertDoc @project_id, @doc_id, {
-					lines: @lines
-				}
 
 
 			it "should be able to continue applying updates when the project has been deleted", (done) ->
 			it "should be able to continue applying updates when the project has been deleted", (done) ->
 				actions = []
 				actions = []
@@ -118,6 +116,7 @@ describe "Applying updates to a doc", ->
 				@lines = ["", "", ""]
 				@lines = ["", "", ""]
 				MockWebApi.insertDoc @project_id, @doc_id, {
 				MockWebApi.insertDoc @project_id, @doc_id, {
 					lines: @lines
 					lines: @lines
+					version: 0
 				}
 				}
 
 
 				@updates = [
 				@updates = [
@@ -129,9 +128,6 @@ describe "Applying updates to a doc", ->
 					{ doc_id: @doc_id, v: 0, op: [i: "world", p: 1 ] }
 					{ doc_id: @doc_id, v: 0, op: [i: "world", p: 1 ] }
 				]
 				]
 				@result = ["hello", "world", ""]
 				@result = ["hello", "world", ""]
-				MockWebApi.insertDoc @project_id, @doc_id, {
-					lines: @lines
-				}
 
 
 			it "should be able to continue applying updates when the project has been deleted", (done) ->
 			it "should be able to continue applying updates when the project has been deleted", (done) ->
 				actions = []
 				actions = []
@@ -148,61 +144,13 @@ describe "Applying updates to a doc", ->
 					DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
 					DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
 						doc.lines.should.deep.equal @result
 						doc.lines.should.deep.equal @result
 						done()
 						done()
-		
-	describe "when the mongo array has been trimmed", ->
-		before ->
-			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
-			@lines = ["", "", ""]
-			MockWebApi.insertDoc @project_id, @doc_id, {
-				lines: @lines
-			}
-
-			@updates = [
-				{ doc_id: @doc_id, v: 0, op: [i: "h", p: 0 ] }
-				{ doc_id: @doc_id, v: 1, op: [i: "e", p: 1 ] }
-				{ doc_id: @doc_id, v: 2, op: [i: "l", p: 2 ] }
-				{ doc_id: @doc_id, v: 3, op: [i: "l", p: 3 ] }
-				{ doc_id: @doc_id, v: 4, op: [i: "o", p: 4 ] }
-				{ doc_id: @doc_id, v: 3, op: [i: "world", p: 4 ] }
-			]
-			@result = ["hello", "world", ""]
-			MockWebApi.insertDoc @project_id, @doc_id, {
-				lines: @lines
-			}
-
-		it "should be able to reload the required ops from the trimmed mongo array", (done) ->
-			actions = []
-			# Apply first set of ops
-			for update in @updates.slice(0,5)
-				do (update) =>
-					actions.push (callback) => DocUpdaterClient.sendUpdate @project_id, @doc_id, update, callback
-			# Delete doc from redis and trim ops back to version 3
-			actions.push (callback) => DocUpdaterClient.deleteDoc @project_id, @doc_id, callback
-			actions.push (callback) =>
-				db.docOps.update({doc_id: ObjectId(@doc_id)}, {$push: docOps: { $each: [], $slice: -2 }}, callback)
-			# Apply older update back from version 3
-			for update in @updates.slice(5)
-				do (update) =>
-					actions.push (callback) => DocUpdaterClient.sendUpdate @project_id, @doc_id, update, callback
-			# Flush ops to mongo
-			actions.push (callback) => DocUpdaterClient.flushDoc @project_id, @doc_id, callback
-
-			async.series actions, (error) =>
-				throw error if error?
-				DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
-					db.docOps.find {doc_id: ObjectId(@doc_id)}, (error, docOps) =>
-						# Check mongo array has been trimmed
-						docOps = docOps[0]
-						docOps.docOps.length.should.equal 3
-						# Check ops have all be applied properly
-						doc.lines.should.deep.equal @result
-						done()
 
 
 	describe "with a broken update", ->
 	describe "with a broken update", ->
 		before (done) ->
 		before (done) ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines
 				lines: @lines
+				version: @version
 			}
 			}
 			DocUpdaterClient.sendUpdate @project_id, @doc_id, @undefined, (error) ->
 			DocUpdaterClient.sendUpdate @project_id, @doc_id, @undefined, (error) ->
 				throw error if error?
 				throw error if error?

+ 4 - 1
services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee

@@ -8,13 +8,14 @@ DocUpdaterClient = require "./helpers/DocUpdaterClient"
 describe "Deleting a document", ->
 describe "Deleting a document", ->
 	before ->
 	before ->
 		@lines = ["one", "two", "three"]
 		@lines = ["one", "two", "three"]
+		@version = 42
 		@update =
 		@update =
 			doc: @doc_id
 			doc: @doc_id
 			op: [{
 			op: [{
 				i: "one and a half\n"
 				i: "one and a half\n"
 				p: 4
 				p: 4
 			}]
 			}]
-			v: 0
+			v: @version
 		@result = ["one", "one and a half", "two", "three"]
 		@result = ["one", "one and a half", "two", "three"]
 
 
 	describe "when the updated doc exists in the doc updater", ->
 	describe "when the updated doc exists in the doc updater", ->
@@ -22,6 +23,7 @@ describe "Deleting a document", ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines
 				lines: @lines
+				version: @version
 			}
 			}
 			sinon.spy MockWebApi, "setDocumentLines"
 			sinon.spy MockWebApi, "setDocumentLines"
 			sinon.spy MockWebApi, "getDocument"
 			sinon.spy MockWebApi, "getDocument"
@@ -60,6 +62,7 @@ describe "Deleting a document", ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines
 				lines: @lines
+				version: @version
 			}
 			}
 			sinon.spy MockWebApi, "setDocumentLines"
 			sinon.spy MockWebApi, "setDocumentLines"
 			sinon.spy MockWebApi, "getDocument"
 			sinon.spy MockWebApi, "getDocument"

+ 1 - 0
services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee

@@ -35,6 +35,7 @@ describe "Deleting a project", ->
 		for doc in @docs
 		for doc in @docs
 			MockWebApi.insertDoc @project_id, doc.id, {
 			MockWebApi.insertDoc @project_id, doc.id, {
 				lines: doc.lines
 				lines: doc.lines
+				version: doc.update.v
 			}
 			}
 
 
 	describe "with documents which have been updated", ->
 	describe "with documents which have been updated", ->

+ 1 - 0
services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee

@@ -35,6 +35,7 @@ describe "Flushing a project", ->
 		for doc in @docs
 		for doc in @docs
 			MockWebApi.insertDoc @project_id, doc.id, {
 			MockWebApi.insertDoc @project_id, doc.id, {
 				lines: doc.lines
 				lines: doc.lines
+				version: doc.update.v
 			}
 			}
 
 
 	describe "with documents which have been updated", ->
 	describe "with documents which have been updated", ->

+ 16 - 37
services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee

@@ -5,23 +5,22 @@ async = require "async"
 
 
 MockWebApi = require "./helpers/MockWebApi"
 MockWebApi = require "./helpers/MockWebApi"
 DocUpdaterClient = require "./helpers/DocUpdaterClient"
 DocUpdaterClient = require "./helpers/DocUpdaterClient"
-mongojs = require "../../../app/js/mongojs"
-db = mongojs.db
-ObjectId = mongojs.ObjectId
 
 
 describe "Flushing a doc to Mongo", ->
 describe "Flushing a doc to Mongo", ->
 	before ->
 	before ->
 		@lines = ["one", "two", "three"]
 		@lines = ["one", "two", "three"]
+		@version = 42
 		@update =
 		@update =
 			doc: @doc_id
 			doc: @doc_id
 			op: [{
 			op: [{
 				i: "one and a half\n"
 				i: "one and a half\n"
 				p: 4
 				p: 4
 			}]
 			}]
-			v: 0
+			v: @version
 		@result = ["one", "one and a half", "two", "three"]
 		@result = ["one", "one and a half", "two", "three"]
 		MockWebApi.insertDoc @project_id, @doc_id, {
 		MockWebApi.insertDoc @project_id, @doc_id, {
 			lines: @lines
 			lines: @lines
+			version: @version
 		}
 		}
 
 
 	describe "when the updated doc exists in the doc updater", ->
 	describe "when the updated doc exists in the doc updater", ->
@@ -29,8 +28,10 @@ describe "Flushing a doc to Mongo", ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines
 				lines: @lines
+				version: @version
 			}
 			}
 			sinon.spy MockWebApi, "setDocumentLines"
 			sinon.spy MockWebApi, "setDocumentLines"
+			sinon.spy MockWebApi, "setDocumentVersion"
 
 
 			DocUpdaterClient.sendUpdates @project_id, @doc_id, [@update], (error) =>
 			DocUpdaterClient.sendUpdates @project_id, @doc_id, [@update], (error) =>
 				throw error if error?
 				throw error if error?
@@ -40,58 +41,36 @@ describe "Flushing a doc to Mongo", ->
 
 
 		after ->
 		after ->
 			MockWebApi.setDocumentLines.restore()
 			MockWebApi.setDocumentLines.restore()
+			MockWebApi.setDocumentVersion.restore()
 
 
-		it "should flush the updated document to the web api", ->
+		it "should flush the updated doc lines to the web api", ->
 			MockWebApi.setDocumentLines
 			MockWebApi.setDocumentLines
 				.calledWith(@project_id, @doc_id, @result)
 				.calledWith(@project_id, @doc_id, @result)
 				.should.equal true
 				.should.equal true
 
 
-		it "should flush the doc ops to Mongo", (done) ->
-			db.docOps.find doc_id: ObjectId(@doc_id), (error, docs) =>
-				doc = docs[0]
-				doc.docOps[0].op.should.deep.equal @update.op
-				done()
-
-	describe "when the doc has a large number of ops to be flushed", ->
-		before (done) ->
-			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
-			MockWebApi.insertDoc @project_id, @doc_id, {
-				lines: @lines
-			}
-			@updates = []
-			for v in [0..999]
-				@updates.push
-					doc_id: @doc_id,
-					op: [i: v.toString(), p: 0]
-					v: v
-
-			DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) =>
-				throw error if error?
-				setTimeout () =>
-					DocUpdaterClient.flushDoc @project_id, @doc_id, done
-				, 200
-
-		it "should flush the doc ops to Mongo in order", (done) ->
-			db.docOps.find doc_id: ObjectId(@doc_id), (error, docs) =>
-				doc = docs[0]
-				updates = @updates.slice(-100)
-				for update, i in doc.docOps
-					update.op.should.deep.equal updates[i].op
-				done()
+		it "should flush the updated doc version to the web api", ->
+			MockWebApi.setDocumentVersion
+				.calledWith(@project_id, @doc_id, @version + 1)
+				.should.equal true
 
 
 	describe "when the doc does not exist in the doc updater", ->
 	describe "when the doc does not exist in the doc updater", ->
 		before (done) ->
 		before (done) ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines
 				lines: @lines
+				version: @version
 			}
 			}
 			sinon.spy MockWebApi, "setDocumentLines"
 			sinon.spy MockWebApi, "setDocumentLines"
+			sinon.spy MockWebApi, "setDocumentVersion"
 			DocUpdaterClient.flushDoc @project_id, @doc_id, done
 			DocUpdaterClient.flushDoc @project_id, @doc_id, done
 
 
 		after ->
 		after ->
 			MockWebApi.setDocumentLines.restore()
 			MockWebApi.setDocumentLines.restore()
+			MockWebApi.setDocumentVersion.restore()
 
 
 		it "should not flush the doc to the web api", ->
 		it "should not flush the doc to the web api", ->
 			MockWebApi.setDocumentLines.called.should.equal false
 			MockWebApi.setDocumentLines.called.should.equal false
+			MockWebApi.setDocumentVersion.called.should.equal false
+
 			
 			
 
 

+ 4 - 2
services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee

@@ -11,6 +11,7 @@ describe "Getting a document", ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines = ["one", "two", "three"]
 				lines: @lines = ["one", "two", "three"]
+				version: @version = 42
 			}
 			}
 			sinon.spy MockWebApi, "getDocument"
 			sinon.spy MockWebApi, "getDocument"
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, @returnedDoc) => done()
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, @returnedDoc) => done()
@@ -26,8 +27,8 @@ describe "Getting a document", ->
 		it "should return the document lines", ->
 		it "should return the document lines", ->
 			@returnedDoc.lines.should.deep.equal @lines
 			@returnedDoc.lines.should.deep.equal @lines
 
 
-		it "should return the document at version 0", ->
-			@returnedDoc.version.should.equal 0
+		it "should return the document at its current version", ->
+			@returnedDoc.version.should.equal @version
 
 
 	describe "when the document is already loaded", ->
 	describe "when the document is already loaded", ->
 		before (done) ->
 		before (done) ->
@@ -55,6 +56,7 @@ describe "Getting a document", ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines = ["one", "two", "three"]
 				lines: @lines = ["one", "two", "three"]
+				version: 0
 			}
 			}
 
 
 			@updates = for v in [0..99]
 			@updates = for v in [0..99]

+ 12 - 3
services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee

@@ -9,22 +9,25 @@ describe "Setting a document", ->
 	before ->
 	before ->
 		[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 		[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 		@lines = ["one", "two", "three"]
 		@lines = ["one", "two", "three"]
+		@version = 42
 		@update =
 		@update =
 			doc: @doc_id
 			doc: @doc_id
 			op: [{
 			op: [{
 				i: "one and a half\n"
 				i: "one and a half\n"
 				p: 4
 				p: 4
 			}]
 			}]
-			v: 0
+			v: @version
 		@result = ["one", "one and a half", "two", "three"]
 		@result = ["one", "one and a half", "two", "three"]
 		@newLines = ["these", "are", "the", "new", "lines"]
 		@newLines = ["these", "are", "the", "new", "lines"]
 		MockWebApi.insertDoc @project_id, @doc_id, {
 		MockWebApi.insertDoc @project_id, @doc_id, {
 			lines: @lines
 			lines: @lines
+			version: @version
 		}
 		}
 
 
 	describe "when the updated doc exists in the doc updater", ->
 	describe "when the updated doc exists in the doc updater", ->
 		before (done) ->
 		before (done) ->
 			sinon.spy MockWebApi, "setDocumentLines"
 			sinon.spy MockWebApi, "setDocumentLines"
+			sinon.spy MockWebApi, "setDocumentVersion"
 			DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
 			DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
 				throw error if error?
 				throw error if error?
 				DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) =>
 				DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) =>
@@ -37,15 +40,21 @@ describe "Setting a document", ->
 
 
 		after ->
 		after ->
 			MockWebApi.setDocumentLines.restore()
 			MockWebApi.setDocumentLines.restore()
+			MockWebApi.setDocumentVersion.restore()
 
 
 		it "should return a 204 status code", ->
 		it "should return a 204 status code", ->
 			@statusCode.should.equal 204
 			@statusCode.should.equal 204
 
 
-		it "should send the updated document to the web api", ->
+		it "should send the updated doc lines to the web api", ->
 			MockWebApi.setDocumentLines
 			MockWebApi.setDocumentLines
 				.calledWith(@project_id, @doc_id, @newLines)
 				.calledWith(@project_id, @doc_id, @newLines)
 				.should.equal true
 				.should.equal true
 
 
+		it "should send the updated doc version to the web api", ->
+			MockWebApi.setDocumentVersion
+				.calledWith(@project_id, @doc_id, @version + 2)
+				.should.equal true
+
 		it "should update the lines in the doc updater", (done) ->
 		it "should update the lines in the doc updater", (done) ->
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
 				doc.lines.should.deep.equal @newLines
 				doc.lines.should.deep.equal @newLines
@@ -53,6 +62,6 @@ describe "Setting a document", ->
 
 
 		it "should bump the version in the doc updater", (done) ->
 		it "should bump the version in the doc updater", (done) ->
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
-				doc.version.should.equal 2
+				doc.version.should.equal @version + 2
 				done()
 				done()
 
 

+ 3 - 1
services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee

@@ -4,7 +4,9 @@ async = require "async"
 
 
 module.exports = DocUpdaterClient =
 module.exports = DocUpdaterClient =
 	randomId: () ->
 	randomId: () ->
-		return require("../../../../app/js/mongojs").ObjectId().toString()
+		chars = for i in [1..24]
+			Math.random().toString(16)[2]
+		return chars.join("")
 
 
 	sendUpdate: (project_id, doc_id, update, callback = (error) ->) ->
 	sendUpdate: (project_id, doc_id, update, callback = (error) ->) ->
 		rclient.rpush "PendingUpdates:#{doc_id}", JSON.stringify(update), (error)->
 		rclient.rpush "PendingUpdates:#{doc_id}", JSON.stringify(update), (error)->

+ 11 - 5
services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee

@@ -14,6 +14,11 @@ module.exports = MockWebApi =
 		@docs["#{project_id}:#{doc_id}"].lines = lines
 		@docs["#{project_id}:#{doc_id}"].lines = lines
 		callback null
 		callback null
 
 
+	setDocumentVersion: (project_id, doc_id, version, callback = (error) ->) ->
+		@docs["#{project_id}:#{doc_id}"] ||= {}
+		@docs["#{project_id}:#{doc_id}"].version = version
+		callback null
+
 	getDocument: (project_id, doc_id, callback = (error, doc) ->) ->
 	getDocument: (project_id, doc_id, callback = (error, doc) ->) ->
 		callback null, @docs["#{project_id}:#{doc_id}"]
 		callback null, @docs["#{project_id}:#{doc_id}"]
 
 
@@ -28,11 +33,12 @@ module.exports = MockWebApi =
 					res.send 404
 					res.send 404
 
 
 		app.post "/project/:project_id/doc/:doc_id", express.bodyParser(), (req, res, next) =>
 		app.post "/project/:project_id/doc/:doc_id", express.bodyParser(), (req, res, next) =>
-			@setDocumentLines req.params.project_id, req.params.doc_id, req.body.lines, (error) ->
-				if error?
-					res.send 500
-				else
-					res.send 204
+			MockWebApi.setDocumentLines req.params.project_id, req.params.doc_id, req.body.lines, (error) ->
+				MockWebApi.setDocumentVersion req.params.project_id, req.params.doc_id, req.body.version, (error) ->
+					if error1? or error2?
+						res.send 500
+					else
+						res.send 204
 
 
 		app.listen(3000)
 		app.listen(3000)
 
 

+ 0 - 4
services/document-updater/test/unit/coffee/AddingDocsToMemory.coffee

@@ -20,9 +20,6 @@ describe 'putting a doc into memory', ()->
 	potentialSAdds[keys.allDocs] = doc_id
 	potentialSAdds[keys.allDocs] = doc_id
 	potentialSAdds[keys.docsInProject(project_id:project_id)] = doc_id
 	potentialSAdds[keys.docsInProject(project_id:project_id)] = doc_id
 
 
-	potentialDels = {}
-	potentialDels[keys.docOps(doc_id:doc_id)] = true
-
 	mocks =
 	mocks =
 		"logger-sharelatex": log:->
 		"logger-sharelatex": log:->
 		redis:
 		redis:
@@ -53,6 +50,5 @@ describe 'putting a doc into memory', ()->
 		redisManager.putDocInMemory project_id, doc_id, lines, version, ()->
 		redisManager.putDocInMemory project_id, doc_id, lines, version, ()->
 			assert.deepEqual potentialSets, {}
 			assert.deepEqual potentialSets, {}
 			assert.deepEqual potentialSAdds, {}
 			assert.deepEqual potentialSAdds, {}
-			assert.deepEqual potentialDels, {}
 			done()
 			done()
 
 

+ 1 - 280
services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee

@@ -3,109 +3,15 @@ chai = require('chai')
 should = chai.should()
 should = chai.should()
 modulePath = "../../../../app/js/DocOpsManager.js"
 modulePath = "../../../../app/js/DocOpsManager.js"
 SandboxedModule = require('sandboxed-module')
 SandboxedModule = require('sandboxed-module')
-ObjectId = require("../../../../app/js/mongojs").ObjectId
 
 
 describe "DocOpsManager", ->
 describe "DocOpsManager", ->
 	beforeEach ->
 	beforeEach ->
-		@doc_id = ObjectId().toString()
+		@doc_id = "doc-id"
 		@project_id = "project-id"
 		@project_id = "project-id"
 		@callback = sinon.stub()
 		@callback = sinon.stub()
 		@DocOpsManager = SandboxedModule.require modulePath, requires:
 		@DocOpsManager = SandboxedModule.require modulePath, requires:
 			"./RedisManager": @RedisManager = {}
 			"./RedisManager": @RedisManager = {}
-			"./mongojs":
-				db: @db = { docOps: {} }
-				ObjectId: ObjectId
 			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
 			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
-
-	describe "flushDocOpsToMongo", ->
-		describe "when versions are consistent", ->
-			beforeEach ->
-				@mongo_version = 40
-				@redis_version = 42
-				@ops = [ "mock-op-1", "mock-op-2" ]
-				@DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @mongo_version)
-				@RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redis_version)
-				@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
-				@DocOpsManager._appendDocOpsInMongo = sinon.stub().callsArg(3)
-				@DocOpsManager.flushDocOpsToMongo @project_id, @doc_id, @callback
-
-			it "should get the version from Mongo", ->
-				@DocOpsManager.getDocVersionInMongo
-					.calledWith(@doc_id)
-					.should.equal true
-
-			it "should get the version from REdis", ->
-				@RedisManager.getDocVersion
-					.calledWith(@doc_id)
-					.should.equal true
-
-			it "should get all doc ops since the version in Mongo", ->
-				@RedisManager.getPreviousDocOps
-					.calledWith(@doc_id, @mongo_version, -1)
-					.should.equal true
-
-			it "should update Mongo with the new ops", ->
-				@DocOpsManager._appendDocOpsInMongo
-					.calledWith(@doc_id, @ops, @redis_version)
-					.should.equal true
-
-			it "should call the callback", ->
-				@callback.called.should.equal true
-
-			it "should time the execution", ->
-				@Metrics.Timer::done.called.should.equal true
-
-		describe "when the number of ops does not match the difference in versions", ->
-			beforeEach ->
-				@mongo_version = 40
-				@redis_version = 45
-				@ops = [ "mock-op-1", "mock-op-2" ]
-				@DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @mongo_version)
-				@RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redis_version)
-				@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
-				@DocOpsManager._appendDocOpsInMongo = sinon.stub().callsArg(3)
-				@DocOpsManager.flushDocOpsToMongo @project_id, @doc_id, @callback
-
-			it "should call the callback with an error", ->
-				@callback.calledWith(new Error("inconsistet versions")).should.equal true
-
-			it "should log an error", ->
-				@logger.error
-					.calledWith(doc_id: @doc_id, mongoVersion: @mongo_version, redisVersion: @redis_version, opsLength: @ops.length, "version difference does not match ops length")
-					.should.equal true
-
-			it "should not modify mongo", ->
-				@DocOpsManager._appendDocOpsInMongo.called.should.equal false
-
-			it "should time the execution", ->
-				@Metrics.Timer::done.called.should.equal true
-
-		describe "when redis version is behind mongo version", ->
-			beforeEach ->
-				@mongo_version = 40
-				@redis_version = 30
-				@DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @mongo_version)
-				@RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redis_version)
-				@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
-				@DocOpsManager._appendDocOpsInMongo = sinon.stub().callsArg(3)
-				@DocOpsManager.flushDocOpsToMongo @project_id, @doc_id, @callback
-			
-			it "should call the callback with an error", ->
-				@callback.calledWith(new Error("inconsistet versions")).should.equal true
-
-			it "should log an error", ->
-				@logger.error
-					.calledWith(doc_id: @doc_id, mongoVersion: @mongo_version, redisVersion: @redis_version, "mongo version is ahead of redis")
-					.should.equal true
-
-			it "should not modify mongo", ->
-				@DocOpsManager._appendDocOpsInMongo.called.should.equal false
-
-			it "should time the execution", ->
-				@Metrics.Timer::done.called.should.equal true
 
 
 	describe "getPreviousDocOps", ->
 	describe "getPreviousDocOps", ->
 		beforeEach ->
 		beforeEach ->
@@ -113,14 +19,8 @@ describe "DocOpsManager", ->
 			@start = 30
 			@start = 30
 			@end = 32
 			@end = 32
 			@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
 			@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
-			@DocOpsManager._ensureOpsAreLoaded = sinon.stub().callsArg(3)
 			@DocOpsManager.getPreviousDocOps @project_id, @doc_id, @start, @end, @callback
 			@DocOpsManager.getPreviousDocOps @project_id, @doc_id, @start, @end, @callback
 
 
-		it "should ensure the ops are loaded back far enough", ->
-			@DocOpsManager._ensureOpsAreLoaded
-				.calledWith(@project_id, @doc_id, @start)
-				.should.equal true
-
 		it "should get the previous doc ops", ->
 		it "should get the previous doc ops", ->
 			@RedisManager.getPreviousDocOps
 			@RedisManager.getPreviousDocOps
 				.calledWith(@doc_id, @start, @end)
 				.calledWith(@doc_id, @start, @end)
@@ -128,182 +28,3 @@ describe "DocOpsManager", ->
 
 
 		it "should call the callback with the ops", ->
 		it "should call the callback with the ops", ->
 			@callback.calledWith(null, @ops).should.equal true
 			@callback.calledWith(null, @ops).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "_ensureOpsAreLoaded", ->
-		describe "when the ops are not loaded", ->
-			beforeEach ->
-				@redisVersion = 42
-				@redisOpsLength = 10
-				@backToVersion = 30
-				@ops = [ "mock-op-1", "mock-op-2" ]
-				@RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redisVersion)
-				@RedisManager.getDocOpsLength = sinon.stub().callsArgWith(1, null, @redisOpsLength)
-				@DocOpsManager._getDocOpsFromMongo = sinon.stub().callsArgWith(3, null, @ops)
-				@RedisManager.prependDocOps = sinon.stub().callsArgWith(2, null)
-				@DocOpsManager._ensureOpsAreLoaded @project_id, @doc_id, @backToVersion, @callback
-
-			it "should get the doc version from redis", ->
-				@RedisManager.getDocVersion
-					.calledWith(@doc_id)
-					.should.equal true
-
-			it "should get the doc ops length in redis", ->
-				@RedisManager.getDocOpsLength
-					.calledWith(@doc_id)
-					.should.equal true
-
-			it "should get the doc ops that need loading from Mongo", ->
-				@DocOpsManager._getDocOpsFromMongo
-					.calledWith(@doc_id, @backToVersion, @redisVersion - @redisOpsLength)
-					.should.equal true
-
-			it "should prepend the retrieved ops to redis", ->
-				@RedisManager.prependDocOps
-					.calledWith(@doc_id, @ops)
-					.should.equal true
-
-			it "should call the callback", ->
-				@callback.called.should.equal true
-
-		describe "when the ops are loaded", ->
-			beforeEach ->
-				@redisVersion = 42
-				@redisOpsLength = 10
-				@backToVersion = 35
-				@RedisManager.getDocVersion = sinon.stub().callsArgWith(1, null, @redisVersion)
-				@RedisManager.getDocOpsLength = sinon.stub().callsArgWith(1, null, @redisOpsLength)
-				@DocOpsManager._getDocOpsFromMongo = sinon.stub().callsArgWith(3, null, @ops)
-				@RedisManager.prependDocOps = sinon.stub().callsArgWith(2, null)
-				@DocOpsManager._ensureOpsAreLoaded @project_id, @doc_id, @backToVersion, @callback
-
-			it "should not need to get the docs from Mongo or put any into redis", ->
-				@DocOpsManager._getDocOpsFromMongo.called.should.equal false
-				@RedisManager.prependDocOps.called.should.equal false
-
-			it "should call the callback", ->
-				@callback.called.should.equal true
-
-	describe "getDocVersionInMongo", ->
-		describe "when the doc exists", ->
-			beforeEach ->
-				@doc =
-					version: @version = 42
-				@db.docOps.find = sinon.stub().callsArgWith(2, null, [@doc])
-				@DocOpsManager.getDocVersionInMongo @doc_id, @callback
-
-			it "should look for the doc in the database", ->
-				@db.docOps.find
-					.calledWith({ doc_id: ObjectId(@doc_id) }, {version: 1})
-					.should.equal true
-
-			it "should call the callback with the version", ->
-				@callback.calledWith(null, @version).should.equal true
-
-		describe "when the doc doesn't exist", ->
-			beforeEach ->
-				@db.docOps.find = sinon.stub().callsArgWith(2, null, [])
-				@DocOpsManager.getDocVersionInMongo @doc_id, @callback
-			
-			it "should call the callback with 0", ->
-				@callback.calledWith(null, 0).should.equal true
-
-	describe "_appendDocOpsInMongo", ->
-		describe "with a small set of updates", ->
-			beforeEach (done) ->
-				@ops = [ "mock-op-1", "mock-op-2" ]
-				@version = 42
-				@db.docOps.update = sinon.stub().callsArg(3)
-				@DocOpsManager._appendDocOpsInMongo @doc_id, @ops, @version, (error) =>
-					@callback(error)
-					done()
-
-			it "should update the database", ->
-				@db.docOps.update
-					.calledWith({
-						doc_id: ObjectId(@doc_id)
-					}, {
-						$push: docOps: { $each: @ops, $slice: -100 }
-						$set: version: @version
-					}, {
-						upsert: true
-					})
-					.should.equal true
-
-			it "should call the callbak", ->
-				@callback.called.should.equal true
-
-		describe "with a large set of updates", ->
-			beforeEach (done) ->
-				@ops = [ "mock-op-1", "mock-op-2", "mock-op-3", "mock-op-4", "mock-op-5" ]
-				@version = 42
-				@DocOpsManager.APPEND_OPS_BATCH_SIZE = 2
-				@db.docOps.update = sinon.stub().callsArg(3)
-				@DocOpsManager._appendDocOpsInMongo @doc_id, @ops, @version, (error) =>
-					@callback(error)
-					done()
-
-			it "should update the database in batches", ->
-				@db.docOps.update
-					.calledWith({ doc_id: ObjectId(@doc_id) }, {
-						$push: docOps: { $each: @ops.slice(0,2), $slice: -100 }
-						$set: version: @version - 3
-					}, { upsert: true })
-					.should.equal true
-				@db.docOps.update
-					.calledWith({ doc_id: ObjectId(@doc_id) }, {
-						$push: docOps: { $each: @ops.slice(2,4), $slice: -100 }
-						$set: version: @version - 1
-					}, { upsert: true })
-					.should.equal true
-				@db.docOps.update
-					.calledWith({ doc_id: ObjectId(@doc_id) }, {
-						$push: docOps: { $each: @ops.slice(4,5), $slice: -100 }
-						$set: version: @version
-					}, { upsert: true })
-					.should.equal true
-
-			it "should call the callbak", ->
-				@callback.called.should.equal true
-
-		describe "with no updates", ->
-			beforeEach (done) ->
-				@ops = []
-				@version = 42
-				@db.docOps.update = sinon.stub().callsArg(3)
-				@DocOpsManager._appendDocOpsInMongo @doc_id, @ops, @version, (error) =>
-					@callback(error)
-					done()
-
-			it "should not try to update the database", ->
-				@db.docOps.update.called.should.equal false
-
-	describe "_getDocsOpsFromMongo", ->
-		beforeEach ->
-			@version = 42
-			@start = 32
-			@limit = 5
-			@doc =
-				docOps: ["mock-ops"]
-			@DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version)
-			@db.docOps.find = sinon.stub().callsArgWith(2, null, [@doc])
-			@DocOpsManager._getDocOpsFromMongo @doc_id, @start, @start + @limit, @callback
-
-		it "should get the current version", ->
-			@DocOpsManager.getDocVersionInMongo
-				.calledWith(@doc_id)
-				.should.equal true
-
-		it "should get the doc ops", ->
-			@db.docOps.find
-				.calledWith({ doc_id: ObjectId(@doc_id) }, {
-					docOps: $slice: [-(@version - @start), @limit]
-				})
-				.should.equal true
-
-		it "should return the ops", ->
-			@callback.calledWith(null, @doc.docOps).should.equal true
-
-	

+ 3 - 9
services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee

@@ -23,8 +23,7 @@ describe "DocumentUpdater - flushDocIfLoaded", ->
 	describe "when the doc is in Redis", ->
 	describe "when the doc is in Redis", ->
 		beforeEach ->
 		beforeEach ->
 			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, @lines, @version)
 			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, @lines, @version)
-			@PersistenceManager.setDoc = sinon.stub().callsArgWith(3)
-			@DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2)
+			@PersistenceManager.setDoc = sinon.stub().callsArgWith(4)
 			@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
 			@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
 
 
 		it "should get the doc from redis", ->
 		it "should get the doc from redis", ->
@@ -34,14 +33,9 @@ describe "DocumentUpdater - flushDocIfLoaded", ->
 
 
 		it "should write the doc lines to the persistence layer", ->
 		it "should write the doc lines to the persistence layer", ->
 			@PersistenceManager.setDoc
 			@PersistenceManager.setDoc
-				.calledWith(@project_id, @doc_id, @lines)
+				.calledWith(@project_id, @doc_id, @lines, @version)
 				.should.equal true
 				.should.equal true
 		
 		
-		it "should write the doc ops to mongo", ->
-			@DocOpsManager.flushDocOpsToMongo
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
 		it "should call the callback without error", ->
 		it "should call the callback without error", ->
 			@callback.calledWith(null).should.equal true
 			@callback.calledWith(null).should.equal true
 
 
@@ -51,7 +45,7 @@ describe "DocumentUpdater - flushDocIfLoaded", ->
 	describe "when the document is not in Redis", ->
 	describe "when the document is not in Redis", ->
 		beforeEach ->
 		beforeEach ->
 			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null)
 			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null)
-			@PersistenceManager.setDoc = sinon.stub().callsArgWith(3)
+			@PersistenceManager.setDoc = sinon.stub().callsArgWith(4)
 			@DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2)
 			@DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2)
 			@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
 			@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
 
 

+ 1 - 7
services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee

@@ -40,8 +40,7 @@ describe "DocumentUpdater - getDoc", ->
 	describe "when the doc does not exist in Redis", ->
 	describe "when the doc does not exist in Redis", ->
 		beforeEach ->
 		beforeEach ->
 			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null)
 			@RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null)
-			@PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines)
-			@DocOpsManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version)
+			@PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
 			@RedisManager.putDocInMemory = sinon.stub().callsArg(4)
 			@RedisManager.putDocInMemory = sinon.stub().callsArg(4)
 			@DocumentManager.getDoc @project_id, @doc_id, @callback
 			@DocumentManager.getDoc @project_id, @doc_id, @callback
 
 
@@ -50,11 +49,6 @@ describe "DocumentUpdater - getDoc", ->
 				.calledWith(@doc_id)
 				.calledWith(@doc_id)
 				.should.equal true
 				.should.equal true
 
 
-		it "should get the doc version from Mongo", ->
-			@DocOpsManager.getDocVersionInMongo
-				.calledWith(@doc_id)
-				.should.equal true
-
 		it "should get the doc from the PersistenceManager", ->
 		it "should get the doc from the PersistenceManager", ->
 			@PersistenceManager.getDoc
 			@PersistenceManager.getDoc
 				.calledWith(@project_id, @doc_id)
 				.calledWith(@project_id, @doc_id)

+ 4 - 3
services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee

@@ -16,6 +16,7 @@ describe "PersistenceManager.getDoc", ->
 		@project_id = "project-id-123"
 		@project_id = "project-id-123"
 		@doc_id = "doc-id-123"
 		@doc_id = "doc-id-123"
 		@lines = ["one", "two", "three"]
 		@lines = ["one", "two", "three"]
+		@version = 42
 		@callback = sinon.stub()
 		@callback = sinon.stub()
 		@Settings.apis =
 		@Settings.apis =
 			web:
 			web:
@@ -25,7 +26,7 @@ describe "PersistenceManager.getDoc", ->
 
 
 	describe "with a successful response from the web api", ->
 	describe "with a successful response from the web api", ->
 		beforeEach ->
 		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines))
+			@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines, version: @version))
 			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
 			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
 
 
 		it "should call the web api", ->
 		it "should call the web api", ->
@@ -43,8 +44,8 @@ describe "PersistenceManager.getDoc", ->
 				})
 				})
 				.should.equal true
 				.should.equal true
 
 
-		it "should call the callback with the doc lines", ->
-			@callback.calledWith(null, @lines).should.equal true
+		it "should call the callback with the doc lines and version", ->
+			@callback.calledWith(null, @lines, @version).should.equal true
 
 
 		it "should time the execution", ->
 		it "should time the execution", ->
 			@Metrics.Timer::done.called.should.equal true
 			@Metrics.Timer::done.called.should.equal true

+ 7 - 5
services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee

@@ -16,6 +16,7 @@ describe "PersistenceManager.setDoc", ->
 		@project_id = "project-id-123"
 		@project_id = "project-id-123"
 		@doc_id = "doc-id-123"
 		@doc_id = "doc-id-123"
 		@lines = ["one", "two", "three"]
 		@lines = ["one", "two", "three"]
+		@version = 42
 		@callback = sinon.stub()
 		@callback = sinon.stub()
 		@Settings.apis =
 		@Settings.apis =
 			web:
 			web:
@@ -25,8 +26,8 @@ describe "PersistenceManager.setDoc", ->
 
 
 	describe "with a successful response from the web api", ->
 	describe "with a successful response from the web api", ->
 		beforeEach ->
 		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines))
-			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @callback)
+			@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines, version: @version))
+			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback)
 
 
 		it "should call the web api", ->
 		it "should call the web api", ->
 			@request
 			@request
@@ -34,6 +35,7 @@ describe "PersistenceManager.setDoc", ->
 					url: "#{@url}/project/#{@project_id}/doc/#{@doc_id}"
 					url: "#{@url}/project/#{@project_id}/doc/#{@doc_id}"
 					body: JSON.stringify
 					body: JSON.stringify
 						lines: @lines
 						lines: @lines
+						version: @version
 					method: "POST"
 					method: "POST"
 					headers:
 					headers:
 						"content-type": "application/json"
 						"content-type": "application/json"
@@ -54,7 +56,7 @@ describe "PersistenceManager.setDoc", ->
 	describe "when request returns an error", ->
 	describe "when request returns an error", ->
 		beforeEach ->
 		beforeEach ->
 			@request.callsArgWith(1, @error = new Error("oops"), null, null)
 			@request.callsArgWith(1, @error = new Error("oops"), null, null)
-			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @callback)
+			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback)
 
 
 		it "should return the error", ->
 		it "should return the error", ->
 			@callback.calledWith(@error).should.equal true
 			@callback.calledWith(@error).should.equal true
@@ -65,7 +67,7 @@ describe "PersistenceManager.setDoc", ->
 	describe "when the request returns 404", ->
 	describe "when the request returns 404", ->
 		beforeEach ->
 		beforeEach ->
 			@request.callsArgWith(1, null, {statusCode: 404}, "")
 			@request.callsArgWith(1, null, {statusCode: 404}, "")
-			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @callback)
+			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback)
 			
 			
 		it "should return a NotFoundError", ->
 		it "should return a NotFoundError", ->
 			@callback.calledWith(new Errors.NotFoundError("not found")).should.equal true
 			@callback.calledWith(new Errors.NotFoundError("not found")).should.equal true
@@ -76,7 +78,7 @@ describe "PersistenceManager.setDoc", ->
 	describe "when the request returns an error status code", ->
 	describe "when the request returns an error status code", ->
 		beforeEach ->
 		beforeEach ->
 			@request.callsArgWith(1, null, {statusCode: 500}, "")
 			@request.callsArgWith(1, null, {statusCode: 500}, "")
-			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @callback)
+			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback)
 			
 			
 		it "should return an error", ->
 		it "should return an error", ->
 			@callback.calledWith(new Error("web api error")).should.equal true
 			@callback.calledWith(new Error("web api error")).should.equal true

+ 0 - 32
services/document-updater/test/unit/coffee/RedisManager/prependDocOpsTests.coffee

@@ -1,32 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/RedisManager"
-SandboxedModule = require('sandboxed-module')
-
-describe "RedisManager.clearDocFromPendingUpdatesSet", ->
-	beforeEach ->
-		@doc_id = "document-id"
-		@callback = sinon.stub()
-		@RedisManager = SandboxedModule.require modulePath, requires:
-			"redis" : createClient: () =>
-				@rclient = auth:->
-
-		@rclient.lpush = sinon.stub().callsArg(2)
-		@ops = [
-			{ "mock" : "op-1" },
-			{ "mock" : "op-2" }
-		]
-		@reversedJsonOps = @ops.map((op) -> JSON.stringify op).reverse()
-		@RedisManager.prependDocOps(@doc_id, @ops, @callback)
-
-	it "should push the reversed JSONed ops", ->
-		@rclient.lpush
-			.calledWith("DocOps:#{@doc_id}", @reversedJsonOps)
-			.should.equal true
-	
-	it "should return the callback", ->
-		@callback.called.should.equal true
-
-
-

+ 44 - 27
services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee

@@ -1,37 +1,54 @@
 sinon = require('sinon')
 sinon = require('sinon')
 chai = require('chai')
 chai = require('chai')
 should = chai.should()
 should = chai.should()
-modulePath = "../../../../app/js/RedisManager"
+modulePath = "../../../../app/js/RedisManager.js"
 SandboxedModule = require('sandboxed-module')
 SandboxedModule = require('sandboxed-module')
 
 
-describe "RedisManager.getPreviousDocOpsTests", ->
+describe "RedisManager.pushDocOp", ->
 	beforeEach ->
 	beforeEach ->
-		@callback = sinon.stub()
 		@RedisManager = SandboxedModule.require modulePath, requires:
 		@RedisManager = SandboxedModule.require modulePath, requires:
-			"redis" : createClient: () =>
+			"redis": createClient: () =>
 				@rclient =
 				@rclient =
-					auth: ->
-					multi: => @rclient
+					auth: () ->
+					multi: () => @rclient
+			"logger-sharelatex": @logger = {log: sinon.stub()}
 		@doc_id = "doc-id-123"
 		@doc_id = "doc-id-123"
+		@callback = sinon.stub()
+		@rclient.rpush = sinon.stub()
+		@rclient.expire = sinon.stub()
+		@rclient.incr = sinon.stub()
+		@rclient.ltrim = sinon.stub()
+
+	describe "successfully", ->
+		beforeEach ->
+			@op = { op: [{ i: "foo", p: 4 }] }
+			@version = 42
+			_ = null
+			@rclient.exec = sinon.stub().callsArgWith(0, null, [_, _, _, @version])
+			@RedisManager.pushDocOp @doc_id, @op, @callback
+		
+		it "should push the doc op into the doc ops list", ->
+			@rclient.rpush
+				.calledWith("DocOps:#{@doc_id}", JSON.stringify(@op))
+				.should.equal true
+
+		it "should renew the expiry ttl on the doc ops array", ->
+			@rclient.expire
+				.calledWith("DocOps:#{@doc_id}", @RedisManager.DOC_OPS_TTL)
+				.should.equal true
+
+		it "should truncate the list to 100 members", ->
+			@rclient.ltrim
+				.calledWith("DocOps:#{@doc_id}", -@RedisManager.DOC_OPS_MAX_LENGTH, -1)
+				.should.equal true
+
+		it "should increment the version number", ->
+			@rclient.incr
+				.calledWith("DocVersion:#{@doc_id}")
+				.should.equal true
+
+		it "should call the callback with the version number", ->
+			@callback.calledWith(null, parseInt(@version, 10)).should.equal true
+
+
 
 
-	beforeEach ->
-		@version = 70
-		@op =
-			{ "mock": "op-1" }
-		@jsonOp = JSON.stringify @op
-		@rclient.rpush = sinon.stub().callsArgWith(2, null)
-		@rclient.incr = sinon.stub().callsArgWith(1, null, @version.toString())
-		@RedisManager.pushDocOp(@doc_id, @op, @callback)
-
-	it "should push the op into redis", ->
-		@rclient.rpush
-			.calledWith("DocOps:#{@doc_id}", @jsonOp)
-			.should.equal true
-
-	it "should increment the version number", ->
-		@rclient.incr
-			.calledWith("DocVersion:#{@doc_id}")
-			.should.equal true
-	
-	it "should call the callback with the new version", ->
-		@callback.calledWith(null, @version).should.equal true