Explorar el Código

Merge pull request #13 from sharelatex/ja-track-changes

Ja track changes
James Allen hace 9 años
padre
commit
3b63865be9
Se han modificado 46 ficheros con 2457 adiciones y 1490 borrados
  1. 1 0
      services/document-updater/app.coffee
  2. 39 17
      services/document-updater/app/coffee/DocumentManager.coffee
  3. 4 4
      services/document-updater/app/coffee/HistoryManager.coffee
  4. 14 1
      services/document-updater/app/coffee/HttpController.coffee
  5. 5 6
      services/document-updater/app/coffee/PersistenceManager.coffee
  6. 0 2
      services/document-updater/app/coffee/ProjectManager.coffee
  7. 44 0
      services/document-updater/app/coffee/RangesManager.coffee
  8. 480 0
      services/document-updater/app/coffee/RangesTracker.coffee
  9. 2 0
      services/document-updater/app/coffee/RedisKeyBuilder.coffee
  10. 34 9
      services/document-updater/app/coffee/RedisManager.coffee
  11. 9 20
      services/document-updater/app/coffee/ShareJsDB.coffee
  12. 8 26
      services/document-updater/app/coffee/ShareJsUpdateManager.coffee
  13. 20 6
      services/document-updater/app/coffee/UpdateManager.coffee
  14. 4 1
      services/document-updater/app/coffee/WebRedisManager.coffee
  15. 60 6
      services/document-updater/app/coffee/sharejs/types/text.coffee
  16. 2 0
      services/document-updater/config/settings.defaults.coffee
  17. 15 1
      services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee
  18. 226 0
      services/document-updater/test/acceptance/coffee/RangesTests.coffee
  19. 3 4
      services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee
  20. 3 2
      services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee
  21. 310 0
      services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee
  22. 0 48
      services/document-updater/test/unit/coffee/DocumentManager/flushAndDeleteDocTests.coffee
  23. 0 68
      services/document-updater/test/unit/coffee/DocumentManager/flushDocIfLoadedTests.coffee
  24. 0 67
      services/document-updater/test/unit/coffee/DocumentManager/getDocAndRecentOpsTests.coffee
  25. 0 70
      services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee
  26. 0 107
      services/document-updater/test/unit/coffee/DocumentManager/setDocTests.coffee
  27. 19 19
      services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee
  28. 376 0
      services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee
  29. 0 63
      services/document-updater/test/unit/coffee/HttpController/deleteProjectTests.coffee
  30. 0 64
      services/document-updater/test/unit/coffee/HttpController/flushAndDeleteDocTests.coffee
  31. 0 65
      services/document-updater/test/unit/coffee/HttpController/flushDocIfLoadedTests.coffee
  32. 0 62
      services/document-updater/test/unit/coffee/HttpController/flushProjectTests.coffee
  33. 0 110
      services/document-updater/test/unit/coffee/HttpController/getDocTests.coffee
  34. 0 83
      services/document-updater/test/unit/coffee/HttpController/setDocTests.coffee
  35. 173 0
      services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee
  36. 0 104
      services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee
  37. 0 90
      services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee
  38. 107 54
      services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee
  39. 283 0
      services/document-updater/test/unit/coffee/ShareJS/TextTransformTests.coffee
  40. 0 55
      services/document-updater/test/unit/coffee/ShareJsDB/GetOpsTests.coffee
  41. 0 87
      services/document-updater/test/unit/coffee/ShareJsDB/GetSnapshotTests.coffee
  42. 93 0
      services/document-updater/test/unit/coffee/ShareJsDB/ShareJsDBTests.coffee
  43. 0 39
      services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee
  44. 12 32
      services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee
  45. 111 8
      services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee
  46. 0 90
      services/document-updater/test/unit/coffee/UpdateManager/lockUpdatesAndDoTests.coffee

+ 1 - 0
services/document-updater/app.coffee

@@ -45,6 +45,7 @@ app.post   '/project/:project_id/doc/:doc_id/flush', HttpController.flushDocIfLo
 app.delete '/project/:project_id/doc/:doc_id',       HttpController.flushAndDeleteDoc
 app.delete '/project/:project_id',                   HttpController.deleteProject
 app.post   '/project/:project_id/flush',             HttpController.flushProject
+app.post   '/project/:project_id/doc/:doc_id/change/:change_id/accept', HttpController.acceptChange
 
 app.get '/total', (req, res)->
 	timer = new Metrics.Timer("http.allDocList")	

+ 39 - 17
services/document-updater/app/coffee/DocumentManager.coffee

@@ -3,7 +3,10 @@ PersistenceManager = require "./PersistenceManager"
 DiffCodec = require "./DiffCodec"
 logger = require "logger-sharelatex"
 Metrics = require "./Metrics"
-TrackChangesManager = require "./TrackChangesManager"
+HistoryManager = require "./HistoryManager"
+WebRedisManager = require "./WebRedisManager"
+Errors = require "./Errors"
+RangesManager = require "./RangesManager"
 
 module.exports = DocumentManager =
 	getDoc: (project_id, doc_id, _callback = (error, lines, version, alreadyLoaded) ->) ->
@@ -12,33 +15,33 @@ module.exports = DocumentManager =
 			timer.done()
 			_callback(args...)
 
-		RedisManager.getDoc project_id, doc_id, (error, lines, version) ->
+		RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges) ->
 			return callback(error) if error?
 			if !lines? or !version?
-				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, version) ->
+				logger.log {project_id, doc_id}, "doc not in redis so getting from persistence API"
+				PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges) ->
 					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) ->
+					logger.log {project_id, doc_id, lines, version}, "got doc from persistence API"
+					RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, (error) ->
 						return callback(error) if error?
-						callback null, lines, version, false
+						callback null, lines, version, ranges, false
 			else
-				callback null, lines, version, true
+				callback null, lines, version, ranges, true
 
-	getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, recentOps) ->) ->
+	getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, recentOps, ranges) ->) ->
 		timer = new Metrics.Timer("docManager.getDocAndRecentOps")
 		callback = (args...) ->
 			timer.done()
 			_callback(args...)
 		
-		DocumentManager.getDoc project_id, doc_id, (error, lines, version) ->
+		DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) ->
 			return callback(error) if error?
 			if fromVersion == -1
-				callback null, lines, version, []
+				callback null, lines, version, [], ranges
 			else
 				RedisManager.getPreviousDocOps doc_id, fromVersion, version, (error, ops) ->
 					return callback(error) if error?
-					callback null, lines, version, ops
+					callback null, lines, version, ops, ranges
 
 	setDoc: (project_id, doc_id, newLines, source, user_id, _callback = (error) ->) ->
 		timer = new Metrics.Timer("docManager.setDoc")
@@ -50,7 +53,7 @@ module.exports = DocumentManager =
 			return callback(new Error("No lines were provided to setDoc"))
 
 		UpdateManager = require "./UpdateManager"
-		DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, alreadyLoaded) ->
+		DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, ranges, alreadyLoaded) ->
 			return callback(error) if error?
 			
 			if oldLines? and oldLines.length > 0 and oldLines[0].text?
@@ -82,21 +85,20 @@ module.exports = DocumentManager =
 						DocumentManager.flushAndDeleteDoc project_id, doc_id, (error) ->
 							return callback(error) if error?
 							callback null
-		
 
 	flushDocIfLoaded: (project_id, doc_id, _callback = (error) ->) ->
 		timer = new Metrics.Timer("docManager.flushDocIfLoaded")
 		callback = (args...) ->
 			timer.done()
 			_callback(args...)
-		RedisManager.getDoc project_id, doc_id, (error, lines, version) ->
+		RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges) ->
 			return callback(error) if error?
 			if !lines? or !version?
 				logger.log project_id: project_id, doc_id: doc_id, "doc is not loaded so not flushing"
 				callback null  # TODO: return a flag to bail out, as we go on to remove doc from memory?
 			else
 				logger.log project_id: project_id, doc_id: doc_id, version: version, "flushing doc"
-				PersistenceManager.setDoc project_id, doc_id, lines, version, (error) ->
+				PersistenceManager.setDoc project_id, doc_id, lines, version, ranges, (error) ->
 					return callback(error) if error?
 					callback null
 
@@ -111,13 +113,29 @@ module.exports = DocumentManager =
 			
 			# Flush in the background since it requires and http request
 			# to track changes
-			TrackChangesManager.flushDocChanges project_id, doc_id, (err) ->
+			HistoryManager.flushDocChanges project_id, doc_id, (err) ->
 				if err?
 					logger.err {err, project_id, doc_id}, "error flushing to track changes"
 
 			RedisManager.removeDocFromMemory project_id, doc_id, (error) ->
 				return callback(error) if error?
 				callback null
+	
+	acceptChange: (project_id, doc_id, change_id, _callback = (error) ->) ->
+		timer = new Metrics.Timer("docManager.acceptChange")
+		callback = (args...) ->
+			timer.done()
+			_callback(args...)
+
+		DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) ->
+			return callback(error) if error?
+			if !lines? or !version?
+				return callback(new Errors.NotFoundError("document not found: #{doc_id}"))
+			RangesManager.acceptChange change_id, ranges, (error, new_ranges) ->
+				return callback(error) if error?
+				RedisManager.updateDocument doc_id, lines, version, [], new_ranges, (error) ->
+					return callback(error) if error?
+					callback()
 
 	getDocWithLock: (project_id, doc_id, callback = (error, lines, version) ->) ->
 		UpdateManager = require "./UpdateManager"
@@ -138,3 +156,7 @@ module.exports = DocumentManager =
 	flushAndDeleteDocWithLock: (project_id, doc_id, callback = (error) ->) ->
 		UpdateManager = require "./UpdateManager"
 		UpdateManager.lockUpdatesAndDo DocumentManager.flushAndDeleteDoc, project_id, doc_id, callback
+
+	acceptChangeWithLock: (project_id, doc_id, change_id, callback = (error) ->) ->
+		UpdateManager = require "./UpdateManager"
+		UpdateManager.lockUpdatesAndDo DocumentManager.acceptChange, project_id, doc_id, change_id, callback

+ 4 - 4
services/document-updater/app/coffee/TrackChangesManager.coffee → services/document-updater/app/coffee/HistoryManager.coffee

@@ -4,7 +4,7 @@ logger = require "logger-sharelatex"
 async = require "async"
 WebRedisManager = require "./WebRedisManager"
 
-module.exports = TrackChangesManager =
+module.exports = HistoryManager =
 	flushDocChanges: (project_id, doc_id, callback = (error) ->) ->
 		if !settings.apis?.trackchanges?
 			logger.warn doc_id: doc_id, "track changes API is not configured, so not flushing"
@@ -32,13 +32,13 @@ module.exports = TrackChangesManager =
 			# ops. If we've changed, then we've gone over a multiple of 50 and should flush.
 			# (Most of the time, we will only hit 50 and then flushing will put us back to 0)
 			previousLength = length - ops.length
-			prevBlock = Math.floor(previousLength / TrackChangesManager.FLUSH_EVERY_N_OPS)
-			newBlock  = Math.floor(length / TrackChangesManager.FLUSH_EVERY_N_OPS)
+			prevBlock = Math.floor(previousLength / HistoryManager.FLUSH_EVERY_N_OPS)
+			newBlock  = Math.floor(length / HistoryManager.FLUSH_EVERY_N_OPS)
 			if newBlock != prevBlock
 				# Do this in the background since it uses HTTP and so may be too
 				# slow to wait for when processing a doc update.
 				logger.log length: length, doc_id: doc_id, project_id: project_id, "flushing track changes api"
-				TrackChangesManager.flushDocChanges project_id, doc_id,  (error) ->
+				HistoryManager.flushDocChanges project_id, doc_id,  (error) ->
 					if error?
 						logger.error err: error, doc_id: doc_id, project_id: project_id, "error flushing doc to track changes api"
 			callback()

+ 14 - 1
services/document-updater/app/coffee/HttpController.coffee

@@ -18,7 +18,7 @@ module.exports = HttpController =
 		else
 			fromVersion = -1
 
-		DocumentManager.getDocAndRecentOpsWithLock project_id, doc_id, fromVersion, (error, lines, version, ops) ->
+		DocumentManager.getDocAndRecentOpsWithLock project_id, doc_id, fromVersion, (error, lines, version, ops, ranges) ->
 			timer.done()
 			return next(error) if error?
 			logger.log project_id: project_id, doc_id: doc_id, "got doc via http"
@@ -29,6 +29,7 @@ module.exports = HttpController =
 				lines: lines
 				version: version
 				ops: ops
+				ranges: ranges
 
 	_getTotalSizeOfLines: (lines) ->
 		size = 0
@@ -96,3 +97,15 @@ module.exports = HttpController =
 			return next(error) if error?
 			logger.log project_id: project_id, "deleted project via http"
 			res.send 204 # No Content
+	
+	acceptChange: (req, res, next = (error) ->) ->
+		{project_id, doc_id, change_id} = req.params
+		logger.log {project_id, doc_id, change_id}, "accepting change via http"
+		timer = new Metrics.Timer("http.acceptChange")
+		DocumentManager.acceptChangeWithLock project_id, doc_id, change_id, (error) ->
+			timer.done()
+			return next(error) if error?
+			logger.log {project_id, doc_id, change_id}, "accepted change via http"
+			res.send 204 # No Content
+		
+

+ 5 - 6
services/document-updater/app/coffee/PersistenceManager.coffee

@@ -10,7 +10,7 @@ logger = require "logger-sharelatex"
 MAX_HTTP_REQUEST_LENGTH = 5000 # 5 seconds
 
 module.exports = PersistenceManager =
-	getDoc: (project_id, doc_id, _callback = (error, lines, version) ->) ->
+	getDoc: (project_id, doc_id, _callback = (error, lines, version, ranges) ->) ->
 		timer = new Metrics.Timer("persistenceManager.getDoc")
 		callback = (args...) ->
 			timer.done()
@@ -39,13 +39,13 @@ module.exports = PersistenceManager =
 					return callback(new Error("web API response had no doc lines"))
 				if !body.version? or not body.version instanceof Number
 					return callback(new Error("web API response had no valid doc version"))
-				return callback null, body.lines, body.version
+				return callback null, body.lines, body.version, body.ranges
 			else if res.statusCode == 404
 				return callback(new Errors.NotFoundError("doc not not found: #{url}"))
 			else
 				return callback(new Error("error accessing web API: #{url} #{res.statusCode}"))
 
-	setDoc: (project_id, doc_id, lines, version, _callback = (error) ->) ->
+	setDoc: (project_id, doc_id, lines, version, ranges, _callback = (error) ->) ->
 		timer = new Metrics.Timer("persistenceManager.setDoc")
 		callback = (args...) ->
 			timer.done()
@@ -55,11 +55,10 @@ module.exports = PersistenceManager =
 		request {
 			url: url
 			method: "POST"
-			body: JSON.stringify
+			json:
 				lines: lines
+				ranges: ranges
 				version: version
-			headers:
-				"content-type": "application/json"
 			auth:
 				user: Settings.apis.web.user
 				pass: Settings.apis.web.pass

+ 0 - 2
services/document-updater/app/coffee/ProjectManager.coffee

@@ -56,5 +56,3 @@ module.exports = ProjectManager =
 					callback new Error("Errors deleting docs. See log for details")
 				else
 					callback(null)
-
-		

+ 44 - 0
services/document-updater/app/coffee/RangesManager.coffee

@@ -0,0 +1,44 @@
+RangesTracker = require "./RangesTracker"
+logger = require "logger-sharelatex"
+
+module.exports = RangesManager =
+	MAX_COMMENTS: 500
+	MAX_CHANGES: 500
+
+	applyUpdate: (project_id, doc_id, entries = {}, updates = [], callback = (error, new_entries) ->) ->
+		{changes, comments} = entries
+		logger.log {changes, comments, updates}, "applying updates to ranges"
+		rangesTracker = new RangesTracker(changes, comments)
+		for update in updates
+			rangesTracker.track_changes = !!update.meta.tc
+			if !!update.meta.tc
+				rangesTracker.setIdSeed(update.meta.tc)
+			for op in update.op
+				rangesTracker.applyOp(op, { user_id: update.meta?.user_id })
+		
+		if rangesTracker.changes?.length > RangesManager.MAX_CHANGES or rangesTracker.comments?.length > RangesManager.MAX_COMMENTS
+			return callback new Error("too many comments or tracked changes")
+
+		response = RangesManager._getRanges rangesTracker
+		logger.log {response}, "applied updates to ranges"
+		callback null, response
+
+	acceptChange: (change_id, ranges, callback = (error, ranges) ->) ->
+		{changes, comments} = ranges
+		logger.log {changes, comments, change_id}, "accepting change in ranges"
+		rangesTracker = new RangesTracker(changes, comments)
+		rangesTracker.removeChangeId(change_id)
+		response = RangesManager._getRanges(rangesTracker)
+		callback null, response
+	
+	_getRanges: (rangesTracker) ->
+		# Return the minimal data structure needed, since most documents won't have any
+		# changes or comments
+		response = {}
+		if rangesTracker.changes?.length > 0
+			response ?= {}
+			response.changes = rangesTracker.changes
+		if rangesTracker.comments?.length > 0
+			response ?= {}
+			response.comments = rangesTracker.comments
+		return response

+ 480 - 0
services/document-updater/app/coffee/RangesTracker.coffee

@@ -0,0 +1,480 @@
+load = (EventEmitter) ->
+	class RangesTracker extends EventEmitter
+		# The purpose of this class is to track a set of inserts and deletes to a document, like
+		# track changes in Word. We store these as a set of ShareJs style ranges:
+		#   {i: "foo", p: 42} # Insert 'foo' at offset 42
+		#   {d: "bar", p: 37} # Delete 'bar' at offset 37
+		# We only track the inserts and deletes, not the whole document, but by being given all
+		# updates that are applied to a document, we can update these appropriately.
+		# 
+		# Note that the set of inserts and deletes we store applies to the document as-is at the moment.
+		# So inserts correspond to text which is in the document, while deletes correspond to text which
+		# is no longer there, so their lengths do not affect the position of later offsets.
+		# E.g.
+		#             this is the current text of the document
+		#                         |-----|            |
+		#  {i: "current ", p:12} -^                   ^- {d: "old ", p: 31}
+		#
+		# Track changes rules (should be consistent with Word):
+		#   * When text is inserted at a delete, the text goes to the left of the delete
+		#       I.e. "foo|bar" -> "foobaz|bar", where | is the delete, and 'baz' is inserted
+		#   * Deleting content flagged as 'inserted' does not create a new delete marker, it only
+		#     removes the insert marker. E.g.
+		#       * "abdefghijkl"        -> "abfghijkl"        when 'de' is deleted. No delete marker added
+		#           |---| <- inserted       |-| <- inserted
+		#       * Deletes overlapping regular text and inserted text will insert a delete marker for the
+		#         regular text:
+		#         "abcdefghijkl"    ->    "abcdejkl"   when 'fghi' is deleted
+		#           |----|                  |--||
+		#           ^- inserted 'bcdefg'      \ ^- deleted 'hi'
+		#                                      \--inserted 'bcde'
+		#   * Deletes overlapping other deletes are merged. E.g.
+		#      "abcghijkl"        ->   "ahijkl"     when 'bcg is deleted'
+		#          | <- delete 'def'     | <- delete 'bcdefg'
+		#   * Deletes by another user will consume deletes by the first user
+		#   * Inserts by another user will not combine with inserts by the first user. If they are in the
+		#     middle of a previous insert by the first user, the original insert will be split into two.
+		constructor: (@changes = [], @comments = []) ->
+			@setIdSeed(RangesTracker.generateIdSeed())
+
+		getIdSeed: () ->
+			return @id_seed
+
+		setIdSeed: (seed) ->
+			@id_seed = seed
+			@id_increment = 0
+		
+		@generateIdSeed: () ->
+			# Generate a the first 18 characters of Mongo ObjectId, leaving 6 for the increment part
+			# Reference: https://github.com/dreampulse/ObjectId.js/blob/master/src/main/javascript/Objectid.js
+			pid = Math.floor(Math.random() * (32767)).toString(16)
+			machine = Math.floor(Math.random() * (16777216)).toString(16)
+			timestamp = Math.floor(new Date().valueOf() / 1000).toString(16)
+			return '00000000'.substr(0, 8 - timestamp.length) + timestamp +
+				'000000'.substr(0, 6 - machine.length) + machine +
+				'0000'.substr(0, 4 - pid.length) + pid
+		
+		@generateId: () ->
+			@generateIdSeed() + "000001"
+
+		newId: () ->
+			@id_increment++
+			increment = @id_increment.toString(16)
+			id = @id_seed + '000000'.substr(0, 6 - increment.length) + increment;
+			return id
+		
+		getComment: (comment_id) ->
+			comment = null
+			for c in @comments
+				if c.id == comment_id
+					comment = c
+					break
+			return comment
+		
+		removeCommentId: (comment_id) ->
+			comment = @getComment(comment_id)
+			return if !comment?
+			@comments = @comments.filter (c) -> c.id != comment_id
+			@emit "comment:removed", comment
+		
+		getChange: (change_id) ->
+			change = null
+			for c in @changes
+				if c.id == change_id
+					change = c
+					break
+			return change
+
+		removeChangeId: (change_id) ->
+			change = @getChange(change_id)
+			return if !change?
+			@_removeChange(change)
+
+		applyOp: (op, metadata = {}) ->
+			metadata.ts ?= new Date()
+			# Apply an op that has been applied to the document to our changes to keep them up to date
+			if op.i?
+				@applyInsertToChanges(op, metadata)
+				@applyInsertToComments(op)
+			else if op.d?
+				@applyDeleteToChanges(op, metadata)
+				@applyDeleteToComments(op)
+			else if op.c?
+				@addComment(op, metadata)
+			else
+				throw new Error("unknown op type")
+			
+		addComment: (op, metadata) ->
+			# TODO: Don't allow overlapping comments?
+			@comments.push comment = {
+				id: @newId()
+				op: # Copy because we'll modify in place
+					c: op.c
+					p: op.p
+					t: op.t
+				metadata
+			}
+			@emit "comment:added", comment
+			return comment
+		
+		applyInsertToComments: (op) ->
+			for comment in @comments
+				if op.p <= comment.op.p
+					comment.op.p += op.i.length
+					@emit "comment:moved", comment
+				else if op.p < comment.op.p + comment.op.c.length
+					offset = op.p - comment.op.p
+					comment.op.c = comment.op.c[0..(offset-1)] + op.i + comment.op.c[offset...]
+					@emit "comment:moved", comment
+
+		applyDeleteToComments: (op) ->
+			op_start = op.p
+			op_length = op.d.length
+			op_end = op.p + op_length
+			for comment in @comments
+				comment_start = comment.op.p
+				comment_end = comment.op.p + comment.op.c.length
+				comment_length = comment_end - comment_start
+				if op_end <= comment_start
+					# delete is fully before comment
+					comment.op.p -= op_length
+					@emit "comment:moved", comment
+				else if op_start >= comment_end
+					# delete is fully after comment, nothing to do
+				else
+					# delete and comment overlap
+					if op_start <= comment_start
+						remaining_before = ""
+					else
+						remaining_before = comment.op.c.slice(0, op_start - comment_start)
+					if op_end >= comment_end
+						remaining_after = ""
+					else
+						remaining_after = comment.op.c.slice(op_end - comment_start)
+					
+					# Check deleted content matches delete op
+					deleted_comment = comment.op.c.slice(remaining_before.length, comment_length - remaining_after.length)
+					offset = Math.max(0, comment_start - op_start)
+					deleted_op_content = op.d.slice(offset).slice(0, deleted_comment.length)
+					if deleted_comment != deleted_op_content
+						throw new Error("deleted content does not match comment content")
+					
+					comment.op.p = Math.min(comment_start, op_start)
+					comment.op.c = remaining_before + remaining_after
+					@emit "comment:moved", comment
+
+		applyInsertToChanges: (op, metadata) ->
+			op_start = op.p
+			op_length = op.i.length
+			op_end = op.p + op_length
+
+			already_merged = false
+			previous_change = null
+			moved_changes = []
+			remove_changes = []
+			new_changes = []
+			for change in @changes
+				change_start = change.op.p
+				
+				if change.op.d?
+					# Shift any deletes after this along by the length of this insert
+					if op_start < change_start
+						change.op.p += op_length
+						moved_changes.push change
+					else if op_start == change_start
+						# If the insert matches the start of the delete, just remove it from the delete instead
+						if change.op.d.length >= op.i.length and change.op.d.slice(0, op.i.length) == op.i
+							change.op.d = change.op.d.slice(op.i.length)
+							change.op.p += op.i.length
+							if change.op.d == ""
+								remove_changes.push change
+							else
+								moved_changes.push change
+							already_merged = true
+						else
+							change.op.p += op_length
+							moved_changes.push change
+				else if change.op.i?
+					change_end = change_start + change.op.i.length
+					is_change_overlapping = (op_start >= change_start and op_start <= change_end)
+					
+					# Only merge inserts if they are from the same user
+					is_same_user = metadata.user_id == change.metadata.user_id
+					
+					# If there is a delete at the start of the insert, and we're inserting
+					# at the start, we SHOULDN'T merge since the delete acts as a partition.
+					# The previous op will be the delete, but it's already been shifted by this insert
+					#
+					# I.e.
+					# Originally: |-- existing insert --|
+					#             | <- existing delete at same offset
+					#
+					# Now:        |-- existing insert --| <- not shifted yet
+					#             |-- this insert --|| <- existing delete shifted along to end of this op
+					# 
+					# After:                         |-- existing insert --|  
+					#             |-- this insert --|| <- existing delete
+					# 
+					# Without the delete, the inserts would be merged.
+					is_insert_blocked_by_delete = (previous_change? and previous_change.op.d? and previous_change.op.p == op_end)
+
+					# If the insert is overlapping another insert, either at the beginning in the middle or touching the end,
+					# then we merge them into one.
+					if @track_changes and
+							is_change_overlapping and
+							!is_insert_blocked_by_delete and
+							!already_merged and 
+							is_same_user
+						offset = op_start - change_start
+						change.op.i = change.op.i.slice(0, offset) + op.i + change.op.i.slice(offset)
+						change.metadata.ts = metadata.ts
+						already_merged = true
+						moved_changes.push change
+					else if op_start <= change_start
+						# If we're fully before the other insert we can just shift the other insert by our length.
+						# If they are touching, and should have been merged, they will have been above.
+						# If not merged above, then it must be blocked by a delete, and will be after this insert, so we shift it along as well
+						change.op.p += op_length
+						moved_changes.push change
+					else if (!is_same_user or !@track_changes) and change_start < op_start < change_end
+						# This user is inserting inside a change by another user, so we need to split the
+						# other user's change into one before and after this one.
+						offset = op_start - change_start
+						before_content = change.op.i.slice(0, offset)
+						after_content = change.op.i.slice(offset)
+						
+						# The existing change can become the 'before' change
+						change.op.i = before_content
+						moved_changes.push change
+						
+						# Create a new op afterwards
+						after_change = {
+							op: {
+								i: after_content
+								p: change_start + offset + op_length
+							}
+							metadata: {}
+						}
+						after_change.metadata[key] = value for key, value of change.metadata
+						new_changes.push after_change
+						
+				previous_change = change
+
+			if @track_changes and !already_merged
+				@_addOp op, metadata
+			for {op, metadata} in new_changes
+				@_addOp op, metadata
+		
+			for change in remove_changes
+				@_removeChange change
+			
+			if moved_changes.length > 0
+				@emit "changes:moved", moved_changes
+		
+		applyDeleteToChanges: (op, metadata) ->
+			op_start = op.p
+			op_length = op.d.length
+			op_end = op.p + op_length
+			remove_changes = []
+			moved_changes = []
+			
+			# We might end up modifying our delete op if it merges with existing deletes, or cancels out
+			# with an existing insert. Since we might do multiple modifications, we record them and do
+			# all the modifications after looping through the existing changes, so as not to mess up the
+			# offset indexes as we go.
+			op_modifications = []
+			for change in @changes
+				if change.op.i?
+					change_start = change.op.p
+					change_end = change_start + change.op.i.length
+					if op_end <= change_start
+						# Shift ops after us back by our length
+						change.op.p -= op_length
+						moved_changes.push change
+					else if op_start >= change_end
+						# Delete is after insert, nothing to do
+					else
+						# When the new delete overlaps an insert, we should remove the part of the insert that
+						# is now deleted, and also remove the part of the new delete that overlapped. I.e.
+						# the two cancel out where they overlap.
+						if op_start >= change_start
+							#                            |-- existing insert --|
+							# insert_remaining_before -> |.....||--   new delete   --|
+							delete_remaining_before = ""
+							insert_remaining_before = change.op.i.slice(0, op_start - change_start)
+						else
+							# delete_remaining_before -> |.....||-- existing insert --|
+							#                            |-- new delete   --|
+							delete_remaining_before = op.d.slice(0, change_start - op_start)
+							insert_remaining_before = ""
+
+						if op_end <= change_end
+							#    |--  existing insert  --|
+							# |--  new delete   --||.....| <- insert_remaining_after
+							delete_remaining_after = ""
+							insert_remaining_after = change.op.i.slice(op_end - change_start)
+						else
+							# |--  existing insert  --||.....| <- delete_remaining_after
+							#            |--  new delete   --|
+							delete_remaining_after = op.d.slice(change_end - op_start)
+							insert_remaining_after = ""
+
+						insert_remaining = insert_remaining_before + insert_remaining_after
+						if insert_remaining.length > 0
+							change.op.i = insert_remaining
+							change.op.p = Math.min(change_start, op_start)
+							change.metadata.ts = metadata.ts
+							moved_changes.push change
+						else
+							remove_changes.push change
+
+						# We know what we want to preserve of our delete op before (delete_remaining_before) and what we want to preserve
+						# afterwards (delete_remaining_before). Now we need to turn that into a modification which deletes the 
+						# chunk in the middle not covered by these.
+						delete_removed_length = op.d.length - delete_remaining_before.length - delete_remaining_after.length
+						delete_removed_start = delete_remaining_before.length
+						modification = {
+							d: op.d.slice(delete_removed_start, delete_removed_start + delete_removed_length)
+							p: delete_removed_start
+						}
+						if modification.d.length > 0
+							op_modifications.push modification
+				else if change.op.d?
+					change_start = change.op.p
+					if op_end < change_start or (!@track_changes and op_end == change_start)
+						# Shift ops after us back by our length.
+						# If we're tracking changes, it must be strictly before, since we'll merge 
+						# below if they are touching. Otherwise, touching is fine.
+						change.op.p -= op_length
+						moved_changes.push change
+					else if op_start <= change_start <= op_end
+						if @track_changes
+							# If we overlap a delete, add it in our content, and delete the existing change.
+							# It's easier to do it this way, rather than modifying the existing delete in case
+							# we overlap many deletes and we'd need to track that. We have a workaround to
+							# update the delete in place if possible below.
+							offset = change_start - op_start
+							op_modifications.push { i: change.op.d, p: offset }
+							remove_changes.push change
+						else
+							change.op.p = op_start
+							moved_changes.push change
+
+			# Copy rather than modify because we still need to apply it to comments
+			op = {
+				p: op.p
+				d: @_applyOpModifications(op.d, op_modifications)
+			}
+
+			for change in remove_changes
+				# This is a bit of hack to avoid removing one delete and replacing it with another.
+				# If we don't do this, it causes the UI to flicker
+				if op.d.length > 0 and change.op.d? and op.p <= change.op.p <= op.p + op.d.length
+					change.op.p = op.p
+					change.op.d = op.d
+					change.metadata = metadata
+					moved_changes.push change
+					op.d = "" # stop it being added
+				else
+					@_removeChange change
+
+			if @track_changes and op.d.length > 0
+				@_addOp op, metadata
+			else
+				# It's possible that we deleted an insert between two other inserts. I.e.
+				# If we delete 'user_2 insert' in:
+				#   |-- user_1 insert --||-- user_2 insert --||-- user_1 insert --|
+				# it becomes:
+				#   |-- user_1 insert --||-- user_1 insert --|
+				# We need to merge these together again
+				results = @_scanAndMergeAdjacentUpdates()
+				moved_changes = moved_changes.concat(results.moved_changes)
+				for change in results.remove_changes
+					@_removeChange change
+					moved_changes = moved_changes.filter (c) -> c != change
+			
+			if moved_changes.length > 0
+				@emit "changes:moved", moved_changes
+
+		_addOp: (op, metadata) ->
+			change = {
+				id: @newId()
+				op: op
+				metadata: metadata
+			}
+			@changes.push change
+
+			# Keep ops in order of offset, with deletes before inserts
+			@changes.sort (c1, c2) ->
+				result = c1.op.p - c2.op.p
+				if result != 0
+					return result
+				else if c1.op.i? and c2.op.d?
+					return 1
+				else
+					return -1
+
+			if op.d?
+				@emit "delete:added", change
+			else if op.i?
+				@emit "insert:added", change
+		
+		_removeChange: (change) ->
+			@changes = @changes.filter (c) -> c.id != change.id
+			if change.op.d?
+				@emit "delete:removed", change
+			else if change.op.i?
+				@emit "insert:removed", change
+			
+		_applyOpModifications: (content, op_modifications) ->
+			# Put in descending position order, with deleting first if at the same offset 
+			# (Inserting first would modify the content that the delete will delete)
+			op_modifications.sort (a, b) ->
+				result = b.p - a.p 
+				if result != 0
+					return result
+				else if a.i? and b.d?
+					return 1
+				else
+					return -1
+
+			for modification in op_modifications
+				if modification.i?
+					content = content.slice(0, modification.p) + modification.i + content.slice(modification.p)
+				else if modification.d?
+					if content.slice(modification.p, modification.p + modification.d.length) != modification.d
+						throw new Error("deleted content does not match. content: #{JSON.stringify(content)}; modification: #{JSON.stringify(modification)}")
+					content = content.slice(0, modification.p) + content.slice(modification.p + modification.d.length)
+			return content
+		
+		_scanAndMergeAdjacentUpdates: () ->
+			# This should only need calling when deleting an update between two
+			# other updates. There's no other way to get two adjacent updates from the
+			# same user, since they would be merged on insert.
+			previous_change = null
+			remove_changes = []
+			moved_changes = []
+			for change in @changes
+				if previous_change?.op.i? and change.op.i?
+					previous_change_end = previous_change.op.p + previous_change.op.i.length
+					previous_change_user_id = previous_change.metadata.user_id
+					change_start = change.op.p
+					change_user_id = change.metadata.user_id
+					if previous_change_end == change_start and previous_change_user_id == change_user_id
+						remove_changes.push change
+						previous_change.op.i += change.op.i
+						moved_changes.push previous_change
+				else if previous_change?.op.d? and change.op.d? and previous_change.op.p == change.op.p
+					# Merge adjacent deletes
+					previous_change.op.d += change.op.d
+					remove_changes.push change
+					moved_changes.push previous_change
+				else # Only update to the current change if we haven't removed it.
+					previous_change = change
+			return { moved_changes, remove_changes }
+
+if define?
+	define ["utils/EventEmitter"], load
+else
+	EventEmitter = require("events").EventEmitter
+	module.exports = load(EventEmitter)

+ 2 - 0
services/document-updater/app/coffee/RedisKeyBuilder.coffee

@@ -34,6 +34,8 @@ module.exports = RedisKeyBuilder =
 		return (key_schema) -> key_schema.uncompressedHistoryOp({doc_id})
 	pendingUpdates: ({doc_id}) ->
 		return (key_schema) -> key_schema.pendingUpdates({doc_id})
+	ranges: ({doc_id}) ->
+		return (key_schema) -> key_schema.ranges({doc_id})
 	docsInProject: ({project_id}) ->
 		return (key_schema) -> key_schema.docsInProject({project_id})
 	docsWithHistoryOps: ({project_id}) ->

+ 34 - 9
services/document-updater/app/coffee/RedisManager.coffee

@@ -13,16 +13,21 @@ minutes = 60 # seconds for Redis expire
 module.exports = RedisManager =
 	rclient: rclient
 
-	putDocInMemory : (project_id, doc_id, docLines, version, _callback)->
+	putDocInMemory : (project_id, doc_id, docLines, version, ranges, _callback)->
 		timer = new metrics.Timer("redis.put-doc")
 		callback = (error) ->
 			timer.done()
 			_callback(error)
 		logger.log project_id:project_id, doc_id:doc_id, version: version, "putting doc in redis"
+		ranges = RedisManager._serializeRanges(ranges)
 		multi = rclient.multi()
 		multi.set keys.docLines(doc_id:doc_id), JSON.stringify(docLines)
 		multi.set keys.projectKey({doc_id:doc_id}), project_id
 		multi.set keys.docVersion(doc_id:doc_id), version
+		if ranges?
+			multi.set keys.ranges(doc_id:doc_id), ranges
+		else
+			multi.del keys.ranges(doc_id:doc_id)
 		multi.exec (error) ->
 			return callback(error) if error?
 			rclient.sadd keys.docsInProject(project_id:project_id), doc_id, callback
@@ -41,30 +46,33 @@ module.exports = RedisManager =
 		multi.del keys.docLines(doc_id:doc_id)
 		multi.del keys.projectKey(doc_id:doc_id)
 		multi.del keys.docVersion(doc_id:doc_id)
+		multi.del keys.ranges(doc_id:doc_id)
 		multi.exec (error) ->
 			return callback(error) if error?
 			rclient.srem keys.docsInProject(project_id:project_id), doc_id, callback
 
-	getDoc : (project_id, doc_id, callback = (error, lines, version, project_id) ->)->
+	getDoc : (project_id, doc_id, callback = (error, lines, version, ranges) ->)->
 		timer = new metrics.Timer("redis.get-doc")
 		multi = rclient.multi()
 		multi.get keys.docLines(doc_id:doc_id)
 		multi.get keys.docVersion(doc_id:doc_id)
 		multi.get keys.projectKey(doc_id:doc_id)
-		multi.exec (error, result)->
+		multi.get keys.ranges(doc_id:doc_id)
+		multi.exec (error, [docLines, version, doc_project_id, ranges])->
 			timer.done()
 			return callback(error) if error?
 			try
-				docLines = JSON.parse result[0]
+				docLines = JSON.parse docLines
+				ranges = RedisManager._deserializeRanges(ranges)
 			catch e
 				return callback(e)
-			version = parseInt(result[1] or 0, 10)
-			doc_project_id = result[2]
+
+			version = parseInt(version or 0, 10)
 			# check doc is in requested project
 			if doc_project_id? and doc_project_id isnt project_id
 				logger.error project_id: project_id, doc_id: doc_id, doc_project_id: doc_project_id, "doc not in project"
 				return callback(new Errors.NotFoundError("document not found"))
-			callback null, docLines, version, project_id
+			callback null, docLines, version, ranges
 
 	getDocVersion: (doc_id, callback = (error, version) ->) ->
 		rclient.get keys.docVersion(doc_id: doc_id), (error, version) ->
@@ -104,7 +112,7 @@ module.exports = RedisManager =
 
 	DOC_OPS_TTL: 60 * minutes
 	DOC_OPS_MAX_LENGTH: 100
-	updateDocument : (doc_id, docLines, newVersion, appliedOps = [], callback = (error) ->)->
+	updateDocument : (doc_id, docLines, newVersion, appliedOps = [], ranges, callback = (error) ->)->
 		RedisManager.getDocVersion doc_id, (error, currentVersion) ->
 			return callback(error) if error?
 			if currentVersion + appliedOps.length != newVersion
@@ -119,10 +127,27 @@ module.exports = RedisManager =
 				multi.rpush  keys.docOps(doc_id: doc_id), jsonOps...
 			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
+			ranges = RedisManager._serializeRanges(ranges)
+			if ranges?
+				multi.set keys.ranges(doc_id:doc_id), ranges
+			else
+				multi.del keys.ranges(doc_id:doc_id)
 			multi.exec (error, replys) ->
 					return callback(error) if error?
 					return callback()
 
 	getDocIdsInProject: (project_id, callback = (error, doc_ids) ->) ->
 		rclient.smembers keys.docsInProject(project_id: project_id), callback
-
+	
+	_serializeRanges: (ranges) ->
+		jsonRanges = JSON.stringify(ranges)
+		if jsonRanges == '{}'
+			# Most doc will have empty ranges so don't fill redis with lots of '{}' keys
+			jsonRanges = null
+		return jsonRanges
+	
+	_deserializeRanges: (ranges) ->
+		if !ranges? or ranges == ""
+			return {}
+		else
+			return JSON.parse(ranges)

+ 9 - 20
services/document-updater/app/coffee/ShareJsDB.coffee

@@ -1,12 +1,9 @@
 Keys = require('./UpdateKeys')
-Settings = require('settings-sharelatex')
-DocumentManager = require "./DocumentManager"
 RedisManager = require "./RedisManager"
 Errors = require "./Errors"
-logger = require "logger-sharelatex"
 
 module.exports = class ShareJsDB
-	constructor: () ->
+	constructor: (@project_id, @doc_id, @lines, @version) ->
 		@appliedOps = {}
 		# ShareJS calls this detacted from the instance, so we need
 		# bind it to keep our context that can access @appliedOps
@@ -31,22 +28,14 @@ module.exports = class ShareJsDB
 		callback()
 
 	getSnapshot: (doc_key, callback) ->
-		[project_id, doc_id] = Keys.splitProjectIdAndDocId(doc_key)
-		DocumentManager.getDoc project_id, doc_id, (error, lines, version) ->
-			return callback(error) if error?
-			if !lines? or !version?
-				return callback(new Errors.NotFoundError("document not found: #{doc_id}"))
-
-			if lines.length > 0 and lines[0].text?
-				type = "json"
-				snapshot = lines: lines
-			else
-				type = "text"
-				snapshot = lines.join("\n")
-			callback null,
-				snapshot: snapshot
-				v: parseInt(version, 10)
-				type: type
+		if doc_key != Keys.combineProjectIdAndDocId(@project_id, @doc_id)
+			return callback(new Errors.NotFoundError("unexpected doc_key #{doc_key}, expected #{Keys.combineProjectIdAndDocId(@project_id, @doc_id)}"))
+		else
+			return callback null, {
+				snapshot: @lines.join("\n")
+				v: parseInt(@version, 10)
+				type: "text"
+			}
 
 	# To be able to remove a doc from the ShareJS memory
 	# we need to called Model::delete, which calls this 

+ 8 - 26
services/document-updater/app/coffee/ShareJsUpdateManager.coffee

@@ -6,22 +6,19 @@ Settings = require('settings-sharelatex')
 Keys = require "./UpdateKeys"
 {EventEmitter} = require "events"
 util = require "util"
-
-redis = require("redis-sharelatex")
-rclient = redis.createClient(Settings.redis.web)
-
+WebRedisManager = require "./WebRedisManager"
 
 ShareJsModel:: = {}
 util.inherits ShareJsModel, EventEmitter
 
 module.exports = ShareJsUpdateManager =
-	getNewShareJsModel: () ->
-		db = new ShareJsDB()
+	getNewShareJsModel: (project_id, doc_id, lines, version) ->
+		db = new ShareJsDB(project_id, doc_id, lines, version)
 		model = new ShareJsModel(db, maxDocLength: Settings.max_doc_length)
 		model.db = db
 		return model
 
-	applyUpdate: (project_id, doc_id, update, callback = (error, updatedDocLines) ->) ->
+	applyUpdate: (project_id, doc_id, update, lines, version, callback = (error, updatedDocLines) ->) ->
 		logger.log project_id: project_id, doc_id: doc_id, update: update, "applying sharejs updates"
 		jobs = []
 
@@ -30,7 +27,7 @@ module.exports = ShareJsUpdateManager =
 		# getting stuck due to queued callbacks (line 260 of sharejs/server/model.coffee)
 		# This adds a small but hopefully acceptable overhead (~12ms per 1000 updates on
 		# my 2009 MBP).
-		model = @getNewShareJsModel()
+		model = @getNewShareJsModel(project_id, doc_id, lines, version)
 		@_listenForOps(model)
 		doc_key = Keys.combineProjectIdAndDocId(project_id, doc_id)
 		model.applyOp doc_key, update, (error) ->
@@ -40,13 +37,10 @@ module.exports = ShareJsUpdateManager =
 					update.dup = true
 					ShareJsUpdateManager._sendOp(project_id, doc_id, update)
 				else
-					ShareJsUpdateManager._sendError(project_id, doc_id, error)
 					return callback(error)
 			logger.log project_id: project_id, doc_id: doc_id, error: error, "applied update"
 			model.getSnapshot doc_key, (error, data) =>
-				if error?
-					ShareJsUpdateManager._sendError(project_id, doc_id, error)
-					return callback(error)
+				return callback(error) if error?
 				docLines = data.snapshot.split(/\r\n|\n|\r/)
 				callback(null, docLines, data.v, model.db.appliedOps[doc_key] or [])
 
@@ -55,18 +49,6 @@ module.exports = ShareJsUpdateManager =
 			[project_id, doc_id] = Keys.splitProjectIdAndDocId(doc_key)
 			ShareJsUpdateManager._sendOp(project_id, doc_id, opData)
 	
-	_sendOp: (project_id, doc_id, opData) ->
-		data =
-			project_id: project_id
-			doc_id: doc_id
-			op: opData
-		data = JSON.stringify data
-		rclient.publish "applied-ops", data
+	_sendOp: (project_id, doc_id, op) ->
+		WebRedisManager.sendData {project_id, doc_id, op}
 
-	_sendError: (project_id, doc_id, error) ->
-		data = JSON.stringify
-			project_id: project_id
-			doc_id: doc_id
-			error: error.message || error
-		rclient.publish "applied-ops", data
-		

+ 20 - 6
services/document-updater/app/coffee/UpdateManager.coffee

@@ -2,11 +2,14 @@ LockManager = require "./LockManager"
 RedisManager = require "./RedisManager"
 WebRedisManager = require "./WebRedisManager"
 ShareJsUpdateManager = require "./ShareJsUpdateManager"
-TrackChangesManager = require "./TrackChangesManager"
+HistoryManager = require "./HistoryManager"
 Settings = require('settings-sharelatex')
 async = require("async")
 logger = require('logger-sharelatex')
 Metrics = require "./Metrics"
+Errors = require "./Errors"
+DocumentManager = require "./DocumentManager"
+RangesManager = require "./RangesManager"
 
 module.exports = UpdateManager =
 	processOutstandingUpdates: (project_id, doc_id, callback = (error) ->) ->
@@ -43,14 +46,25 @@ module.exports = UpdateManager =
 				(update, cb) -> UpdateManager.applyUpdate project_id, doc_id, update, cb
 				callback
 
-	applyUpdate: (project_id, doc_id, update, callback = (error) ->) ->
+	applyUpdate: (project_id, doc_id, update, _callback = (error) ->) ->
+		callback = (error) ->
+			if error?
+				WebRedisManager.sendData {project_id, doc_id, error: error.message || error}
+			_callback(error)
+		
 		UpdateManager._sanitizeUpdate update
-		ShareJsUpdateManager.applyUpdate project_id, doc_id, update, (error, updatedDocLines, version, appliedOps) ->
+		DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) ->
 			return callback(error) if error?
-			logger.log doc_id: doc_id, version: version, "updating doc in redis"
-			RedisManager.updateDocument doc_id, updatedDocLines, version, appliedOps, (error) ->
+			if !lines? or !version?
+				return callback(new Errors.NotFoundError("document not found: #{doc_id}"))
+			ShareJsUpdateManager.applyUpdate project_id, doc_id, update, lines, version, (error, updatedDocLines, version, appliedOps) ->
 				return callback(error) if error?
-				TrackChangesManager.pushUncompressedHistoryOps project_id, doc_id, appliedOps, callback
+				RangesManager.applyUpdate project_id, doc_id, ranges, appliedOps, (error, new_ranges) ->
+					return callback(error) if error?
+					logger.log doc_id: doc_id, version: version, "updating doc in redis"
+					RedisManager.updateDocument doc_id, updatedDocLines, version, appliedOps, new_ranges, (error) ->
+						return callback(error) if error?
+						HistoryManager.pushUncompressedHistoryOps project_id, doc_id, appliedOps, callback
 
 	lockUpdatesAndDo: (method, project_id, doc_id, args..., callback) ->
 		LockManager.getLock doc_id, (error, lockValue) ->

+ 4 - 1
services/document-updater/app/coffee/WebRedisManager.coffee

@@ -32,4 +32,7 @@ module.exports = WebRedisManager =
 		], (error, results) ->
 			return callback(error) if error?
 			[length, _] = results
-			callback(error, length)
+			callback(error, length)
+	
+	sendData: (data) ->
+		rclient.publish "applied-ops", JSON.stringify(data)

+ 60 - 6
services/document-updater/app/coffee/sharejs/types/text.coffee

@@ -31,7 +31,8 @@ checkValidComponent = (c) ->
 
   i_type = typeof c.i
   d_type = typeof c.d
-  throw new Error 'component needs an i or d field' unless (i_type == 'string') ^ (d_type == 'string')
+  c_type = typeof c.c
+  throw new Error 'component needs an i, d or c field' unless (i_type == 'string') ^ (d_type == 'string') ^ (c_type == 'string')
 
   throw new Error 'position cannot be negative' unless c.p >= 0
 
@@ -44,11 +45,15 @@ text.apply = (snapshot, op) ->
   for component in op
     if component.i?
       snapshot = strInject snapshot, component.p, component.i
-    else
+    else if component.d?
       deleted = snapshot[component.p...(component.p + component.d.length)]
       throw new Error "Delete component '#{component.d}' does not match deleted text '#{deleted}'" unless component.d == deleted
       snapshot = snapshot[...component.p] + snapshot[(component.p + component.d.length)..]
-  
+    else if component.c?
+      comment = snapshot[component.p...(component.p + component.c.length)]
+      throw new Error "Comment component '#{component.c}' does not match commented text '#{comment}'" unless component.c == comment
+    else
+      throw new Error "Unknown op type"
   snapshot
 
 
@@ -112,7 +117,7 @@ transformPosition = (pos, c, insertAfter) ->
       pos + c.i.length
     else
       pos
-  else
+  else if c.d?
     # I think this could also be written as: Math.min(c.p, Math.min(c.p - otherC.p, otherC.d.length))
     # but I think its harder to read that way, and it compiles using ternary operators anyway
     # so its no slower written like this.
@@ -122,6 +127,10 @@ transformPosition = (pos, c, insertAfter) ->
       c.p
     else
       pos - c.d.length
+  else if c.c?
+    pos
+  else
+    throw new Error("unknown op type")
 
 # Helper method to transform a cursor position as a result of an op.
 #
@@ -143,7 +152,7 @@ text._tc = transformComponent = (dest, c, otherC, side) ->
   if c.i?
     append dest, {i:c.i, p:transformPosition(c.p, otherC, side == 'right')}
 
-  else # Delete
+  else if c.d? # Delete
     if otherC.i? # delete vs insert
       s = c.d
       if c.p < otherC.p
@@ -152,7 +161,7 @@ text._tc = transformComponent = (dest, c, otherC, side) ->
       if s != ''
         append dest, {d:s, p:c.p + otherC.i.length}
 
-    else # Delete vs delete
+    else if otherC.d? # Delete vs delete
       if c.p >= otherC.p + otherC.d.length
         append dest, {d:c.d, p:c.p - otherC.d.length}
       else if c.p + c.d.length <= otherC.p
@@ -177,6 +186,51 @@ text._tc = transformComponent = (dest, c, otherC, side) ->
           # This could be rewritten similarly to insert v delete, above.
           newC.p = transformPosition newC.p, otherC
           append dest, newC
+    
+    else if otherC.c?
+      append dest, c
+    
+    else
+      throw new Error("unknown op type")
+
+  else if c.c? # Comment
+    if otherC.i?
+      if c.p < otherC.p < c.p + c.c.length
+        offset = otherC.p - c.p
+        new_c = (c.c[0..(offset-1)] + otherC.i + c.c[offset...])
+        append dest, {c:new_c, p:c.p, t: c.t}
+      else
+        append dest, {c:c.c, p:transformPosition(c.p, otherC, true), t: c.t}
+    
+    else if otherC.d?
+      if c.p >= otherC.p + otherC.d.length
+        append dest, {c:c.c, p:c.p - otherC.d.length, t: c.t}
+      else if c.p + c.c.length <= otherC.p
+        append dest, c
+      else # Delete overlaps comment
+        # They overlap somewhere.
+        newC = {c:'', p:c.p, t: c.t}
+        if c.p < otherC.p
+          newC.c = c.c[...(otherC.p - c.p)]
+        if c.p + c.c.length > otherC.p + otherC.d.length
+          newC.c += c.c[(otherC.p + otherC.d.length - c.p)..]
+
+        # This is entirely optional - just for a check that the deleted
+        # text in the two ops matches
+        intersectStart = Math.max c.p, otherC.p
+        intersectEnd = Math.min c.p + c.c.length, otherC.p + otherC.d.length
+        cIntersect = c.c[intersectStart - c.p...intersectEnd - c.p]
+        otherIntersect = otherC.d[intersectStart - otherC.p...intersectEnd - otherC.p]
+        throw new Error 'Delete ops delete different text in the same region of the document' unless cIntersect == otherIntersect
+
+        newC.p = transformPosition newC.p, otherC
+        append dest, newC
+    
+    else if otherC.c?
+      append dest, c
+    
+    else
+      throw new Error("unknown op type")
   
   dest
 

+ 2 - 0
services/document-updater/config/settings.defaults.coffee

@@ -32,6 +32,7 @@ module.exports =
 				docVersion: ({doc_id}) -> "DocVersion:#{doc_id}"
 				projectKey: ({doc_id}) -> "ProjectId:#{doc_id}"
 				docsInProject: ({project_id}) -> "DocsIn:#{project_id}"
+				ranges: ({doc_id}) -> "Ranges:#{doc_id}"
 		# }, {
 		# 	cluster: [{
 		# 		port: "7000"
@@ -44,6 +45,7 @@ module.exports =
 		# 		docVersion: ({doc_id}) -> "DocVersion:{#{doc_id}}"
 		# 		projectKey: ({doc_id}) -> "ProjectId:{#{doc_id}}"
 		# 		docsInProject: ({project_id}) -> "DocsIn:{#{project_id}}"
+		# 		ranges: ({doc_id}) -> "Ranges:{#{doc_id}}"
 		}]
 	
 	max_doc_length: 2 * 1024 * 1024 # 2mb

+ 15 - 1
services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee

@@ -176,8 +176,12 @@ describe "Applying updates to a doc", ->
 	describe "with a broken update", ->
 		before (done) ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+			@broken_update = { doc_id: @doc_id, v: @version, op: [d: "not the correct content", p: 0 ] }
 			MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version}
-			DocUpdaterClient.sendUpdate @project_id, @doc_id, @undefined, (error) ->
+	
+			DocUpdaterClient.subscribeToAppliedOps @messageCallback = sinon.stub()
+
+			DocUpdaterClient.sendUpdate @project_id, @doc_id, @broken_update, (error) ->
 				throw error if error?
 				setTimeout done, 200
 
@@ -185,6 +189,16 @@ describe "Applying updates to a doc", ->
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
 				doc.lines.should.deep.equal @lines
 				done()
+		
+		it "should send a message with an error", ->
+			@messageCallback.called.should.equal true
+			[channel, message] = @messageCallback.args[0]
+			channel.should.equal "applied-ops"
+			JSON.parse(message).should.deep.equal {
+				project_id: @project_id,
+				doc_id: @doc_id, 
+				error:'Delete component \'not the correct content\' does not match deleted text \'one\ntwo\nthree\''
+			}
 
 	describe "with enough updates to flush to the track changes api", ->
 		before (done) ->

+ 226 - 0
services/document-updater/test/acceptance/coffee/RangesTests.coffee

@@ -0,0 +1,226 @@
+sinon = require "sinon"
+chai = require("chai")
+chai.should()
+expect = chai.expect
+async = require "async"
+
+MockWebApi = require "./helpers/MockWebApi"
+DocUpdaterClient = require "./helpers/DocUpdaterClient"
+
+describe "Ranges", ->
+	describe "tracking changes from ops", ->
+		before (done) ->
+			@project_id = DocUpdaterClient.randomId()
+			@user_id = DocUpdaterClient.randomId()
+			@id_seed = "587357bd35e64f6157"
+			@doc = {
+				id: DocUpdaterClient.randomId()
+				lines: ["aaa"]
+			}
+			@updates = [{
+				doc: @doc.id
+				op: [{ i: "123", p: 1 }]
+				v: 0
+				meta: { user_id: @user_id }
+			}, {
+				doc: @doc.id
+				op: [{ i: "456", p: 5 }]
+				v: 1
+				meta: { user_id: @user_id, tc: @id_seed }
+			}, {
+				doc: @doc.id
+				op: [{ d: "12", p: 1 }]
+				v: 2
+				meta: { user_id: @user_id }
+			}]
+			MockWebApi.insertDoc @project_id, @doc.id, {
+				lines: @doc.lines
+				version: 0
+			}
+			jobs = []
+			for update in @updates
+				do (update) =>
+					jobs.push (callback) => DocUpdaterClient.sendUpdate @project_id, @doc.id, update, callback
+			DocUpdaterClient.preloadDoc @project_id, @doc.id, (error) =>
+				throw error if error?
+				async.series jobs, (error) ->
+					throw error if error?
+					setTimeout done, 200
+		
+		it "should update the ranges", (done) ->
+			DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) =>
+				throw error if error?
+				ranges = data.ranges
+				change = ranges.changes[0]
+				change.op.should.deep.equal { i: "456", p: 3 }
+				change.id.should.equal @id_seed + "000001"
+				change.metadata.user_id.should.equal @user_id
+				done()
+	
+		describe "Adding comments", ->
+			describe "standalone", ->
+				before (done) ->
+					@project_id = DocUpdaterClient.randomId()
+					@user_id = DocUpdaterClient.randomId()
+					@doc = {
+						id: DocUpdaterClient.randomId()
+						lines: ["foo bar baz"]
+					}
+					@updates = [{
+						doc: @doc.id
+						op: [{ c: "bar", p: 4, t: @tid = DocUpdaterClient.randomId() }]
+						v: 0
+					}]
+					MockWebApi.insertDoc @project_id, @doc.id, {
+						lines: @doc.lines
+						version: 0
+					}
+					jobs = []
+					for update in @updates
+						do (update) =>
+							jobs.push (callback) => DocUpdaterClient.sendUpdate @project_id, @doc.id, update, callback
+					DocUpdaterClient.preloadDoc @project_id, @doc.id, (error) =>
+						throw error if error?
+						async.series jobs, (error) ->
+							throw error if error?
+							setTimeout done, 200
+				
+				it "should update the ranges", (done) ->
+					DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) =>
+						throw error if error?
+						ranges = data.ranges
+						comment = ranges.comments[0]
+						comment.op.should.deep.equal { c: "bar", p: 4, t: @tid }
+						done()
+
+			describe "with conflicting ops needing OT", ->
+				before (done) ->
+					@project_id = DocUpdaterClient.randomId()
+					@user_id = DocUpdaterClient.randomId()
+					@doc = {
+						id: DocUpdaterClient.randomId()
+						lines: ["foo bar baz"]
+					}
+					@updates = [{
+						doc: @doc.id
+						op: [{ i: "ABC", p: 3 }]
+						v: 0
+						meta: { user_id: @user_id }
+					}, {
+						doc: @doc.id
+						op: [{ c: "bar", p: 4, t: @tid = DocUpdaterClient.randomId() }]
+						v: 0
+					}]
+					MockWebApi.insertDoc @project_id, @doc.id, {
+						lines: @doc.lines
+						version: 0
+					}
+					jobs = []
+					for update in @updates
+						do (update) =>
+							jobs.push (callback) => DocUpdaterClient.sendUpdate @project_id, @doc.id, update, callback
+					DocUpdaterClient.preloadDoc @project_id, @doc.id, (error) =>
+						throw error if error?
+						async.series jobs, (error) ->
+							throw error if error?
+							setTimeout done, 200
+				
+				it "should update the comments with the OT shifted comment", (done) ->
+					DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) =>
+						throw error if error?
+						ranges = data.ranges
+						comment = ranges.comments[0]
+						comment.op.should.deep.equal { c: "bar", p: 7, t: @tid }
+						done()
+
+	describe "Loading ranges from persistence layer", ->
+		before (done) ->
+			@project_id = DocUpdaterClient.randomId()
+			@user_id = DocUpdaterClient.randomId()
+			@id_seed = "587357bd35e64f6157"
+			@doc = {
+				id: DocUpdaterClient.randomId()
+				lines: ["a123aa"]
+			}
+			@update = {
+				doc: @doc.id
+				op: [{ i: "456", p: 5 }]
+				v: 0
+				meta: { user_id: @user_id, tc: @id_seed }
+			}
+			MockWebApi.insertDoc @project_id, @doc.id, {
+				lines: @doc.lines
+				version: 0
+				ranges: {
+					changes: [{
+						op: { i: "123", p: 1 }
+						metadata:
+							user_id: @user_id
+							ts: new Date()
+					}]
+				}
+			}
+			DocUpdaterClient.preloadDoc @project_id, @doc.id, (error) =>
+				throw error if error?
+				DocUpdaterClient.sendUpdate @project_id, @doc.id, @update, (error) ->
+					throw error if error?
+					setTimeout done, 200
+		
+		it "should have preloaded the existing ranges", (done) ->
+			DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) =>
+				throw error if error?
+				{changes} = data.ranges
+				changes[0].op.should.deep.equal { i: "123", p: 1 }
+				changes[1].op.should.deep.equal { i: "456", p: 5 }
+				done()
+		
+		it "should flush the ranges to the persistence layer again", (done) ->
+			DocUpdaterClient.flushDoc @project_id, @doc.id, (error) =>
+				throw error if error?
+				MockWebApi.getDocument @project_id, @doc.id, (error, doc) =>
+					{changes} = doc.ranges
+					changes[0].op.should.deep.equal { i: "123", p: 1 }
+					changes[1].op.should.deep.equal { i: "456", p: 5 }
+					done()
+
+	describe "accepting a change", ->
+		before (done) ->
+			@project_id = DocUpdaterClient.randomId()
+			@user_id = DocUpdaterClient.randomId()
+			@id_seed = "587357bd35e64f6157"
+			@doc = {
+				id: DocUpdaterClient.randomId()
+				lines: ["aaa"]
+			}
+			@update = {
+				doc: @doc.id
+				op: [{ i: "456", p: 1 }]
+				v: 0
+				meta: { user_id: @user_id, tc: @id_seed }
+			}
+			MockWebApi.insertDoc @project_id, @doc.id, {
+				lines: @doc.lines
+				version: 0
+			}
+			DocUpdaterClient.preloadDoc @project_id, @doc.id, (error) =>
+				throw error if error?
+				DocUpdaterClient.sendUpdate @project_id, @doc.id, @update, (error) =>
+					throw error if error?
+					setTimeout () =>
+						DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) =>
+							throw error if error?
+							ranges = data.ranges
+							change = ranges.changes[0]
+							change.op.should.deep.equal { i: "456", p: 1 }
+							change.id.should.equal @id_seed + "000001"
+							change.metadata.user_id.should.equal @user_id
+							done()
+					, 200
+		
+		it "should remove the change after accepting", (done) ->
+			DocUpdaterClient.acceptChange @project_id, @doc.id, @id_seed + "000001", (error) =>
+				throw error if error?
+				DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) =>
+					throw error if error?
+					expect(data.ranges.changes).to.be.undefined
+					done()

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

@@ -72,7 +72,6 @@ module.exports = DocUpdaterClient =
 
 	deleteProject: (project_id, callback = () ->) ->
 		request.del "http://localhost:3003/project/#{project_id}", callback
-		
-		
-		
-	
+
+	acceptChange: (project_id, doc_id, change_id, callback = () ->) ->
+		request.post "http://localhost:3003/project/#{project_id}/doc/#{doc_id}/change/#{change_id}/accept", callback

+ 3 - 2
services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee

@@ -11,10 +11,11 @@ module.exports = MockWebApi =
 		doc.lines ?= []
 		@docs["#{project_id}:#{doc_id}"] = doc
 
-	setDocument: (project_id, doc_id, lines, version, callback = (error) ->) ->
+	setDocument: (project_id, doc_id, lines, version, ranges, callback = (error) ->) ->
 		doc = @docs["#{project_id}:#{doc_id}"] ||= {}
 		doc.lines = lines
 		doc.version = version
+		doc.ranges = ranges
 		callback null
 
 	getDocument: (project_id, doc_id, callback = (error, doc) ->) ->
@@ -31,7 +32,7 @@ module.exports = MockWebApi =
 					res.send 404
 
 		app.post "/project/:project_id/doc/:doc_id", express.bodyParser(), (req, res, next) =>
-			MockWebApi.setDocument req.params.project_id, req.params.doc_id, req.body.lines, req.body.version, (error) ->
+			MockWebApi.setDocument req.params.project_id, req.params.doc_id, req.body.lines, req.body.version, req.body.ranges, (error) ->
 				if error?
 					res.send 500
 				else

+ 310 - 0
services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee

@@ -0,0 +1,310 @@
+sinon = require('sinon')
+chai = require('chai')
+should = chai.should()
+modulePath = "../../../../app/js/DocumentManager.js"
+SandboxedModule = require('sandboxed-module')
+Errors = require "../../../../app/js/Errors"
+
+describe "DocumentManager", ->
+	beforeEach ->
+		@DocumentManager = SandboxedModule.require modulePath, requires:
+			"./RedisManager": @RedisManager = {}
+			"./PersistenceManager": @PersistenceManager = {}
+			"./HistoryManager": @HistoryManager = {}
+			"logger-sharelatex": @logger = {log: sinon.stub()}
+			"./DocOpsManager": @DocOpsManager = {}
+			"./Metrics": @Metrics =
+				Timer: class Timer
+					done: sinon.stub()
+			"./WebRedisManager": @WebRedisManager = {}
+			"./DiffCodec": @DiffCodec = {}
+			"./UpdateManager": @UpdateManager = {}
+			"./RangesManager": @RangesManager = {}
+		@project_id = "project-id-123"
+		@doc_id = "doc-id-123"
+		@callback = sinon.stub()
+		@lines = ["one", "two", "three"]
+		@version = 42
+		@ranges = { comments: "mock", entries: "mock" }
+
+	describe "flushAndDeleteDoc", ->
+		describe "successfully", ->
+			beforeEach ->
+				@RedisManager.removeDocFromMemory = sinon.stub().callsArg(2)
+				@DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2)
+				@HistoryManager.flushDocChanges = sinon.stub().callsArg(2)
+				@DocumentManager.flushAndDeleteDoc @project_id, @doc_id, @callback
+			
+			it "should flush the doc", ->
+				@DocumentManager.flushDocIfLoaded
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should remove the doc from redis", ->
+				@RedisManager.removeDocFromMemory
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should call the callback without error", ->
+				@callback.calledWith(null).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+			
+			it "should flush to the history api", ->
+				@HistoryManager.flushDocChanges
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+	
+	describe "flushDocIfLoaded", ->
+		describe "when the doc is in Redis", ->
+			beforeEach ->
+				@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges)
+				@PersistenceManager.setDoc = sinon.stub().yields()
+				@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
+
+			it "should get the doc from redis", ->
+				@RedisManager.getDoc
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should write the doc lines to the persistence layer", ->
+				@PersistenceManager.setDoc
+					.calledWith(@project_id, @doc_id, @lines, @version, @ranges)
+					.should.equal true
+			
+			it "should call the callback without error", ->
+				@callback.calledWith(null).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when the document is not in Redis", ->
+			beforeEach ->
+				@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null)
+				@PersistenceManager.setDoc = sinon.stub().yields()
+				@DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2)
+				@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
+
+			it "should get the doc from redis", ->
+				@RedisManager.getDoc
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should not write anything to the persistence layer", ->
+				@PersistenceManager.setDoc.called.should.equal false
+				@DocOpsManager.flushDocOpsToMongo.called.should.equal false
+
+			it "should call the callback without error", ->
+				@callback.calledWith(null).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+	
+	describe "getDocAndRecentOps", ->
+		describe "with a previous version specified", ->
+			beforeEach ->
+				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges)
+				@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
+				@DocumentManager.getDocAndRecentOps @project_id, @doc_id, @fromVersion, @callback
+
+			it "should get the doc", ->
+				@DocumentManager.getDoc
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should get the doc ops", ->
+				@RedisManager.getPreviousDocOps
+					.calledWith(@doc_id, @fromVersion, @version)
+					.should.equal true
+
+			it "should call the callback with the doc info", ->
+				@callback.calledWith(null, @lines, @version, @ops, @ranges).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "with no previous version specified", ->
+			beforeEach ->
+				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges)
+				@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
+				@DocumentManager.getDocAndRecentOps @project_id, @doc_id, -1, @callback
+
+			it "should get the doc", ->
+				@DocumentManager.getDoc
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should not need to get the doc ops", ->
+				@RedisManager.getPreviousDocOps.called.should.equal false
+
+			it "should call the callback with the doc info", ->
+				@callback.calledWith(null, @lines, @version, [], @ranges).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+	
+	describe "getDoc", ->
+		describe "when the doc exists in Redis", ->
+			beforeEach ->
+				@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges)
+				@DocumentManager.getDoc @project_id, @doc_id, @callback
+
+			it "should get the doc from Redis", ->
+				@RedisManager.getDoc
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+			
+			it "should call the callback with the doc info", ->
+				@callback.calledWith(null, @lines, @version, @ranges, true).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when the doc does not exist in Redis", ->
+			beforeEach ->
+				@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null)
+				@PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges)
+				@RedisManager.putDocInMemory = sinon.stub().yields()
+				@DocumentManager.getDoc @project_id, @doc_id, @callback
+
+			it "should try to get the doc from Redis", ->
+				@RedisManager.getDoc
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should get the doc from the PersistenceManager", ->
+				@PersistenceManager.getDoc
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should set the doc in Redis", ->
+				@RedisManager.putDocInMemory
+					.calledWith(@project_id, @doc_id, @lines, @version, @ranges)
+					.should.equal true
+
+			it "should call the callback with the doc info", ->
+				@callback.calledWith(null, @lines, @version, @ranges, false).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+	
+	describe "setDoc", ->
+		describe "with plain tex lines", ->
+			beforeEach ->
+				@beforeLines = ["before", "lines"]
+				@afterLines = ["after", "lines"]
+				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @ranges, true)
+				@DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops)
+				@UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null)
+				@DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2)
+				@DocumentManager.flushAndDeleteDoc = sinon.stub().callsArg(2)
+
+			describe "when already loaded", ->
+				beforeEach ->
+					@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback
+
+				it "should get the current doc lines", ->
+					@DocumentManager.getDoc
+						.calledWith(@project_id, @doc_id)
+						.should.equal true
+
+				it "should return a diff of the old and new lines", ->
+					@DiffCodec.diffAsShareJsOp
+						.calledWith(@beforeLines, @afterLines)
+						.should.equal true
+
+				it "should apply the diff as a ShareJS op", ->
+					@UpdateManager.applyUpdate
+						.calledWith(
+							@project_id,
+							@doc_id,
+							{
+								doc: @doc_id,
+								v: @version,
+								op: @ops,
+								meta: {
+									type: "external"
+									source: @source
+									user_id: @user_id
+								}
+							}
+						)
+						.should.equal true
+
+				it "should flush the doc to Mongo", ->
+					@DocumentManager.flushDocIfLoaded
+						.calledWith(@project_id, @doc_id)
+						.should.equal true
+				
+				it "should call the callback", ->
+					@callback.calledWith(null).should.equal true
+
+				it "should time the execution", ->
+					@Metrics.Timer::done.called.should.equal true
+			
+			describe "when not already loaded", ->
+				beforeEach ->
+					@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, false)
+					@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback
+
+				it "should flush and delete the doc from the doc updater", ->
+					@DocumentManager.flushAndDeleteDoc
+						.calledWith(@project_id, @doc_id)
+						.should.equal true
+
+			describe "without new lines", ->
+				beforeEach ->
+					@DocumentManager.setDoc @project_id, @doc_id, null, @source, @user_id, @callback
+
+				it "should return the callback with an error", ->
+					@callback.calledWith(new Error("No lines were passed to setDoc"))
+					
+				it "should not try to get the doc lines", ->
+					@DocumentManager.getDoc.called.should.equal false
+	
+	describe "acceptChanges", ->
+		beforeEach ->
+			@change_id = "mock-change-id"
+			@version = 34
+			@lines = ["original", "lines"]
+			@ranges = { entries: "mock", comments: "mock" }
+			@updated_ranges = { entries: "updated", comments: "updated" }
+			@DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges)
+			@RangesManager.acceptChange = sinon.stub().yields(null, @updated_ranges)
+			@RedisManager.updateDocument = sinon.stub().yields()
+		
+		describe "successfully", ->
+			beforeEach ->
+				@DocumentManager.acceptChange @project_id, @doc_id, @change_id, @callback
+			
+			it "should get the document's current ranges", ->
+				@DocumentManager.getDoc
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+			
+			it "should apply the accept change to the ranges", ->
+				@RangesManager.acceptChange
+					.calledWith(@change_id, @ranges)
+					.should.equal true
+					
+			it "should save the updated ranges", ->
+				@RedisManager.updateDocument
+					.calledWith(@doc_id, @lines, @version, [], @updated_ranges)
+					.should.equal true
+			
+			it "should call the callback", ->
+				@callback.called.should.equal true
+
+		describe "when the doc is not found", ->
+			beforeEach ->
+				@DocumentManager.getDoc = sinon.stub().yields(null, null, null, null)
+				@DocumentManager.acceptChange @project_id, @doc_id, @change_id, @callback
+
+			it "should not save anything", ->
+				@RedisManager.updateDocument.called.should.equal false
+			
+			it "should call the callback with a not found error", ->
+				error = new Errors.NotFoundError("document not found: #{@doc_id}")
+				@callback.calledWith(error).should.equal true
+			

+ 0 - 48
services/document-updater/test/unit/coffee/DocumentManager/flushAndDeleteDocTests.coffee

@@ -1,48 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/DocumentManager.js"
-SandboxedModule = require('sandboxed-module')
-
-describe "DocumentUpdater.flushAndDeleteDoc", ->
-	beforeEach ->
-		@DocumentManager = SandboxedModule.require modulePath, requires:
-			"./RedisManager": @RedisManager = {}
-			"./PersistenceManager": @PersistenceManager = {}
-			"./TrackChangesManager": @TrackChangesManager = {}
-			"logger-sharelatex": @logger = {log: sinon.stub()}
-			"./DocOpsManager" :{}
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@callback = sinon.stub()
-
-	describe "successfully", ->
-		beforeEach ->
-			@RedisManager.removeDocFromMemory = sinon.stub().callsArg(2)
-			@DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2)
-			@TrackChangesManager.flushDocChanges = sinon.stub().callsArg(2)
-			@DocumentManager.flushAndDeleteDoc @project_id, @doc_id, @callback
-		
-		it "should flush the doc", ->
-			@DocumentManager.flushDocIfLoaded
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should remove the doc from redis", ->
-			@RedisManager.removeDocFromMemory
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should call the callback without error", ->
-			@callback.calledWith(null).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-		
-		it "should flush to track changes", ->
-			@TrackChangesManager.flushDocChanges
-				.calledWith(@project_id, @doc_id)
-				.should.equal true

+ 0 - 68
services/document-updater/test/unit/coffee/DocumentManager/flushDocIfLoadedTests.coffee

@@ -1,68 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/DocumentManager.js"
-SandboxedModule = require('sandboxed-module')
-
-describe "DocumentManager.flushDocIfLoaded", ->
-	beforeEach ->
-		@DocumentManager = SandboxedModule.require modulePath, requires:
-			"./RedisManager": @RedisManager = {}
-			"./PersistenceManager": @PersistenceManager = {}
-			"./DocOpsManager": @DocOpsManager = {}
-			"logger-sharelatex": @logger = {log: sinon.stub()}
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
-			"./TrackChangesManager": {}
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@lines = ["one", "two", "three"]
-		@version = 42
-		@callback = sinon.stub()
-
-	describe "when the doc is in Redis", ->
-		beforeEach ->
-			@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
-			@PersistenceManager.setDoc = sinon.stub().callsArgWith(4)
-			@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
-
-		it "should get the doc from redis", ->
-			@RedisManager.getDoc
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should write the doc lines to the persistence layer", ->
-			@PersistenceManager.setDoc
-				.calledWith(@project_id, @doc_id, @lines, @version)
-				.should.equal true
-		
-		it "should call the callback without error", ->
-			@callback.calledWith(null).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when the document is not in Redis", ->
-		beforeEach ->
-			@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null)
-			@PersistenceManager.setDoc = sinon.stub().callsArgWith(4)
-			@DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2)
-			@DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback
-
-		it "should get the doc from redis", ->
-			@RedisManager.getDoc
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should not write anything to the persistence layer", ->
-			@PersistenceManager.setDoc.called.should.equal false
-			@DocOpsManager.flushDocOpsToMongo.called.should.equal false
-
-		it "should call the callback without error", ->
-			@callback.calledWith(null).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-		
-

+ 0 - 67
services/document-updater/test/unit/coffee/DocumentManager/getDocAndRecentOpsTests.coffee

@@ -1,67 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/DocumentManager.js"
-SandboxedModule = require('sandboxed-module')
-
-describe "DocumentManager.getDocAndRecentOps", ->
-	beforeEach ->
-		@DocumentManager = SandboxedModule.require modulePath, requires:
-			"./RedisManager": @RedisManager = {}
-			"./PersistenceManager": @PersistenceManager = {}
-			"logger-sharelatex": @logger = {log: sinon.stub()}
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
-			"./TrackChangesManager": {}
-
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@lines = ["one", "two", "three"]
-		@version = 42
-		@fromVersion = 40
-		@ops = ["mock-op-1", "mock-op-2"]
-		@callback = sinon.stub()
-
-	describe "with a previous version specified", ->
-		beforeEach ->
-			@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
-			@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
-			@DocumentManager.getDocAndRecentOps @project_id, @doc_id, @fromVersion, @callback
-
-		it "should get the doc", ->
-			@DocumentManager.getDoc
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should get the doc ops", ->
-			@RedisManager.getPreviousDocOps
-				.calledWith(@doc_id, @fromVersion, @version)
-				.should.equal true
-
-		it "should call the callback with the doc info", ->
-			@callback.calledWith(null, @lines, @version, @ops).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "with no previous version specified", ->
-		beforeEach ->
-			@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
-			@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
-			@DocumentManager.getDocAndRecentOps @project_id, @doc_id, -1, @callback
-
-		it "should get the doc", ->
-			@DocumentManager.getDoc
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should not need to get the doc ops", ->
-			@RedisManager.getPreviousDocOps.called.should.equal false
-
-		it "should call the callback with the doc info", ->
-			@callback.calledWith(null, @lines, @version, []).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-

+ 0 - 70
services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee

@@ -1,70 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/DocumentManager.js"
-SandboxedModule = require('sandboxed-module')
-
-describe "DocumentUpdater.getDoc", ->
-	beforeEach ->
-		@DocumentManager = SandboxedModule.require modulePath, requires:
-			"./RedisManager": @RedisManager = {}
-			"./PersistenceManager": @PersistenceManager = {}
-			"./DocOpsManager": @DocOpsManager = {}
-			"logger-sharelatex": @logger = {log: sinon.stub()}
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
-			"./TrackChangesManager": {}
-
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@lines = ["one", "two", "three"]
-		@version = 42
-		@callback = sinon.stub()
-
-	describe "when the doc exists in Redis", ->
-		beforeEach ->
-			@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
-			@DocumentManager.getDoc @project_id, @doc_id, @callback
-
-		it "should get the doc from Redis", ->
-			@RedisManager.getDoc
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-		
-		it "should call the callback with the doc info", ->
-			@callback.calledWith(null, @lines, @version, true).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when the doc does not exist in Redis", ->
-		beforeEach ->
-			@RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null)
-			@PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
-			@RedisManager.putDocInMemory = sinon.stub().callsArg(4)
-			@DocumentManager.getDoc @project_id, @doc_id, @callback
-
-		it "should try to get the doc from Redis", ->
-			@RedisManager.getDoc
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should get the doc from the PersistenceManager", ->
-			@PersistenceManager.getDoc
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should set the doc in Redis", ->
-			@RedisManager.putDocInMemory
-				.calledWith(@project_id, @doc_id, @lines, @version)
-				.should.equal true
-
-		it "should call the callback with the doc info", ->
-			@callback.calledWith(null, @lines, @version, false).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-		
-
-

+ 0 - 107
services/document-updater/test/unit/coffee/DocumentManager/setDocTests.coffee

@@ -1,107 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/DocumentManager.js"
-SandboxedModule = require('sandboxed-module')
-
-describe "DocumentManager.setDoc", ->
-	beforeEach ->
-		@DocumentManager = SandboxedModule.require modulePath, requires:
-			"./RedisManager": @RedisManager = {}
-			"./PersistenceManager": @PersistenceManager = {}
-			"./DiffCodec": @DiffCodec = {}
-			"./DocOpsManager":{}
-			"./UpdateManager": @UpdateManager = {}
-			"logger-sharelatex": @logger = {log: sinon.stub()}
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
-			"./TrackChangesManager": {}
-
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@version = 42
-		@ops = ["mock-ops"]
-		@callback = sinon.stub()
-		@source = "dropbox"
-		@user_id = "mock-user-id"
-
-	describe "with plain tex lines", ->
-		beforeEach ->
-			@beforeLines = ["before", "lines"]
-			@afterLines = ["after", "lines"]
-			@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, true)
-			@DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops)
-			@UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null)
-			@DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2)
-			@DocumentManager.flushAndDeleteDoc = sinon.stub().callsArg(2)
-
-		describe "when already loaded", ->
-			beforeEach ->
-				@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback
-
-			it "should get the current doc lines", ->
-				@DocumentManager.getDoc
-					.calledWith(@project_id, @doc_id)
-					.should.equal true
-
-			it "should return a diff of the old and new lines", ->
-				@DiffCodec.diffAsShareJsOp
-					.calledWith(@beforeLines, @afterLines)
-					.should.equal true
-
-			it "should apply the diff as a ShareJS op", ->
-				@UpdateManager.applyUpdate
-					.calledWith(
-						@project_id,
-						@doc_id,
-						{
-							doc: @doc_id,
-							v: @version,
-							op: @ops,
-							meta: {
-								type: "external"
-								source: @source
-								user_id: @user_id
-							}
-						}
-					)
-					.should.equal true
-
-			it "should flush the doc to Mongo", ->
-				@DocumentManager.flushDocIfLoaded
-					.calledWith(@project_id, @doc_id)
-					.should.equal true
-			
-			it "should call the callback", ->
-				@callback.calledWith(null).should.equal true
-
-			it "should time the execution", ->
-				@Metrics.Timer::done.called.should.equal true
-		
-		describe "when not already loaded", ->
-			beforeEach ->
-				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, false)
-				@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback
-
-			it "should flush and delete the doc from the doc updater", ->
-				@DocumentManager.flushAndDeleteDoc
-					.calledWith(@project_id, @doc_id)
-					.should.equal true
-
-		describe "without new lines", ->
-			beforeEach ->
-				@DocumentManager.setDoc @project_id, @doc_id, null, @source, @user_id, @callback
-
-			it "should return the callback with an error", ->
-				@callback.calledWith(new Error("No lines were passed to setDoc"))
-				
-			it "should not try to get the doc lines", ->
-				@DocumentManager.getDoc.called.should.equal false
-
-		
-
-		
-
-
-

+ 19 - 19
services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee → services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee

@@ -1,11 +1,11 @@
 SandboxedModule = require('sandboxed-module')
 sinon = require('sinon')
 require('chai').should()
-modulePath = require('path').join __dirname, '../../../../app/js/TrackChangesManager'
+modulePath = require('path').join __dirname, '../../../../app/js/HistoryManager'
 
-describe "TrackChangesManager", ->
+describe "HistoryManager", ->
 	beforeEach ->
-		@TrackChangesManager = SandboxedModule.require modulePath, requires:
+		@HistoryManager = SandboxedModule.require modulePath, requires:
 			"request": @request = {}
 			"settings-sharelatex": @Settings = {}
 			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
@@ -22,7 +22,7 @@ describe "TrackChangesManager", ->
 		describe "successfully", ->
 			beforeEach ->
 				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 204)
-				@TrackChangesManager.flushDocChanges @project_id, @doc_id, @callback
+				@HistoryManager.flushDocChanges @project_id, @doc_id, @callback
 
 			it "should send a request to the track changes api", ->
 				@request.post
@@ -35,7 +35,7 @@ describe "TrackChangesManager", ->
 		describe "when the track changes api returns an error", ->
 			beforeEach ->
 				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 500)
-				@TrackChangesManager.flushDocChanges @project_id, @doc_id, @callback
+				@HistoryManager.flushDocChanges @project_id, @doc_id, @callback
 
 			it "should return the callback with an error", ->
 				@callback.calledWith(new Error("track changes api return non-success code: 500")).should.equal true
@@ -43,12 +43,12 @@ describe "TrackChangesManager", ->
 	describe "pushUncompressedHistoryOps", ->
 		beforeEach ->
 			@ops = ["mock-ops"]
-			@TrackChangesManager.flushDocChanges = sinon.stub().callsArg(2)
+			@HistoryManager.flushDocChanges = sinon.stub().callsArg(2)
 
 		describe "pushing the op", ->
 			beforeEach ->
 				@WebRedisManager.pushUncompressedHistoryOps = sinon.stub().callsArgWith(3, null, 1)
-				@TrackChangesManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback
+				@HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback
 
 			it "should push the ops into redis", ->
 				@WebRedisManager.pushUncompressedHistoryOps
@@ -59,16 +59,16 @@ describe "TrackChangesManager", ->
 				@callback.called.should.equal true
 
 			it "should not try to flush the op", ->
-				@TrackChangesManager.flushDocChanges.called.should.equal false
+				@HistoryManager.flushDocChanges.called.should.equal false
 
 		describe "when we hit a multiple of FLUSH_EVERY_N_OPS ops", ->
 			beforeEach ->
 				@WebRedisManager.pushUncompressedHistoryOps =
-					sinon.stub().callsArgWith(3, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS)
-				@TrackChangesManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback
+					sinon.stub().callsArgWith(3, null, 2 * @HistoryManager.FLUSH_EVERY_N_OPS)
+				@HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback
 
 			it "should tell the track changes api to flush", ->
-				@TrackChangesManager.flushDocChanges
+				@HistoryManager.flushDocChanges
 					.calledWith(@project_id, @doc_id)
 					.should.equal true
 
@@ -76,20 +76,20 @@ describe "TrackChangesManager", ->
 			beforeEach ->
 				@ops = ["op1", "op2", "op3"]
 				@WebRedisManager.pushUncompressedHistoryOps =
-					sinon.stub().callsArgWith(3, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS + 1)
-				@TrackChangesManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback
+					sinon.stub().callsArgWith(3, null, 2 * @HistoryManager.FLUSH_EVERY_N_OPS + 1)
+				@HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback
 
 			it "should tell the track changes api to flush", ->
-				@TrackChangesManager.flushDocChanges
+				@HistoryManager.flushDocChanges
 					.calledWith(@project_id, @doc_id)
 					.should.equal true
 
-		describe "when TrackChangesManager errors", ->
+		describe "when HistoryManager errors", ->
 			beforeEach ->
 				@WebRedisManager.pushUncompressedHistoryOps =
-					sinon.stub().callsArgWith(3, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS)
-				@TrackChangesManager.flushDocChanges = sinon.stub().callsArgWith(2, @error = new Error("oops"))
-				@TrackChangesManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback
+					sinon.stub().callsArgWith(3, null, 2 * @HistoryManager.FLUSH_EVERY_N_OPS)
+				@HistoryManager.flushDocChanges = sinon.stub().callsArgWith(2, @error = new Error("oops"))
+				@HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback
 
 			it "should log out the error", ->
 				@logger.error
@@ -104,7 +104,7 @@ describe "TrackChangesManager", ->
 		describe "with no ops", ->
 			beforeEach ->
 				@WebRedisManager.pushUncompressedHistoryOps = sinon.stub().callsArgWith(3, null, 1)
-				@TrackChangesManager.pushUncompressedHistoryOps @project_id, @doc_id, [], @callback
+				@HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, [], @callback
 			
 			it "should not call WebRedisManager.pushUncompressedHistoryOps", ->
 				@WebRedisManager.pushUncompressedHistoryOps.called.should.equal false

+ 376 - 0
services/document-updater/test/unit/coffee/HttpController/HttpControllerTests.coffee

@@ -0,0 +1,376 @@
+sinon = require('sinon')
+chai = require('chai')
+should = chai.should()
+modulePath = "../../../../app/js/HttpController.js"
+SandboxedModule = require('sandboxed-module')
+Errors = require "../../../../app/js/Errors.js"
+
+describe "HttpController", ->
+	beforeEach ->
+		@HttpController = SandboxedModule.require modulePath, requires:
+			"./DocumentManager": @DocumentManager = {}
+			"./ProjectManager": @ProjectManager = {}
+			"logger-sharelatex" : @logger = { log: sinon.stub() }
+			"./Metrics": @Metrics = {}
+
+		@Metrics.Timer = class Timer
+			done: sinon.stub()
+		@project_id = "project-id-123"
+		@doc_id = "doc-id-123"
+		@next = sinon.stub()
+		@res =
+			send: sinon.stub()
+	
+	describe "getDoc", ->
+		beforeEach ->
+			@lines = ["one", "two", "three"]
+			@ops = ["mock-op-1", "mock-op-2"]
+			@version = 42
+			@fromVersion = 42
+			@ranges = { changes: "mock", comments: "mock" }
+			@req =
+				params:
+					project_id: @project_id
+					doc_id: @doc_id
+
+		describe "when the document exists and no recent ops are requested", ->
+			beforeEach ->
+				@DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, @lines, @version, [], @ranges)
+				@HttpController.getDoc(@req, @res, @next)
+
+			it "should get the doc", ->
+				@DocumentManager.getDocAndRecentOpsWithLock
+					.calledWith(@project_id, @doc_id, -1)
+					.should.equal true
+
+			it "should return the doc as JSON", ->
+				@res.send
+					.calledWith(JSON.stringify({
+						id: @doc_id
+						lines: @lines
+						version: @version
+						ops: []
+						ranges: @ranges
+					}))
+					.should.equal true
+
+			it "should log the request", ->
+				@logger.log
+					.calledWith(doc_id: @doc_id, project_id: @project_id, "getting doc via http")
+					.should.equal true
+
+			it "should time the request", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when recent ops are requested", ->
+			beforeEach ->
+				@DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, @lines, @version, @ops)
+				@req.query = fromVersion: "#{@fromVersion}"
+				@HttpController.getDoc(@req, @res, @next)
+
+			it "should get the doc", ->
+				@DocumentManager.getDocAndRecentOpsWithLock
+					.calledWith(@project_id, @doc_id, @fromVersion)
+					.should.equal true
+
+			it "should return the doc as JSON", ->
+				@res.send
+					.calledWith(JSON.stringify({
+						id: @doc_id
+						lines: @lines
+						version: @version
+						ops: @ops
+					}))
+					.should.equal true
+
+			it "should log the request", ->
+				@logger.log
+					.calledWith(doc_id: @doc_id, project_id: @project_id, "getting doc via http")
+					.should.equal true
+
+			it "should time the request", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when the document does not exist", ->
+			beforeEach ->
+				@DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, null, null)
+				@HttpController.getDoc(@req, @res, @next)
+
+			it "should call next with NotFoundError", ->
+				@next
+					.calledWith(new Errors.NotFoundError("not found"))
+					.should.equal true
+
+		describe "when an errors occurs", ->
+			beforeEach ->
+				@DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, new Error("oops"), null, null)
+				@HttpController.getDoc(@req, @res, @next)
+
+			it "should call next with the error", ->
+				@next
+					.calledWith(new Error("oops"))
+					.should.equal true
+	
+	describe "setDoc", ->
+		beforeEach ->
+			@lines = ["one", "two", "three"]
+			@source = "dropbox"
+			@user_id = "user-id-123"
+			@req =
+				headers: {}
+				params:
+					project_id: @project_id
+					doc_id: @doc_id
+				body:
+					lines: @lines
+					source: @source
+					user_id: @user_id
+
+		describe "successfully", ->
+			beforeEach ->
+				@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5)
+				@HttpController.setDoc(@req, @res, @next)
+
+			it "should set the doc", ->
+				@DocumentManager.setDocWithLock
+					.calledWith(@project_id, @doc_id, @lines, @source, @user_id)
+					.should.equal true
+
+			it "should return a successful No Content response", ->
+				@res.send
+					.calledWith(204)
+					.should.equal true
+
+			it "should log the request", ->
+				@logger.log
+					.calledWith(doc_id: @doc_id, project_id: @project_id, lines: @lines, source: @source, user_id: @user_id, "setting doc via http")
+					.should.equal true
+
+			it "should time the request", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when an errors occurs", ->
+			beforeEach ->
+				@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5, new Error("oops"))
+				@HttpController.setDoc(@req, @res, @next)
+
+			it "should call next with the error", ->
+				@next
+					.calledWith(new Error("oops"))
+					.should.equal true
+
+		describe "when the payload is too large", ->
+			beforeEach ->
+				lines = []
+				for _ in [0..200000]
+					lines.push "test test test"
+				@req.body.lines = lines
+				@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5)
+				@HttpController.setDoc(@req, @res, @next)
+
+			it 'should send back a 406 response', ->
+				@res.send.calledWith(406).should.equal true
+
+			it 'should not call setDocWithLock', ->
+				@DocumentManager.setDocWithLock.callCount.should.equal 0
+
+	describe "flushProject", ->
+		beforeEach ->
+			@req =
+				params:
+					project_id: @project_id
+
+		describe "successfully", ->
+			beforeEach ->
+				@ProjectManager.flushProjectWithLocks = sinon.stub().callsArgWith(1)
+				@HttpController.flushProject(@req, @res, @next)
+
+			it "should flush the project", ->
+				@ProjectManager.flushProjectWithLocks
+					.calledWith(@project_id)
+					.should.equal true
+
+			it "should return a successful No Content response", ->
+				@res.send
+					.calledWith(204)
+					.should.equal true
+
+			it "should log the request", ->
+				@logger.log
+					.calledWith(project_id: @project_id, "flushing project via http")
+					.should.equal true
+
+			it "should time the request", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when an errors occurs", ->
+			beforeEach ->
+				@ProjectManager.flushProjectWithLocks = sinon.stub().callsArgWith(1, new Error("oops"))
+				@HttpController.flushProject(@req, @res, @next)
+
+			it "should call next with the error", ->
+				@next
+					.calledWith(new Error("oops"))
+					.should.equal true
+	
+	describe "flushDocIfLoaded", ->
+		beforeEach ->
+			@lines = ["one", "two", "three"]
+			@version = 42
+			@req =
+				params:
+					project_id: @project_id
+					doc_id: @doc_id
+
+		describe "successfully", ->
+			beforeEach ->
+				@DocumentManager.flushDocIfLoadedWithLock = sinon.stub().callsArgWith(2)
+				@HttpController.flushDocIfLoaded(@req, @res, @next)
+
+			it "should flush the doc", ->
+				@DocumentManager.flushDocIfLoadedWithLock
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should return a successful No Content response", ->
+				@res.send
+					.calledWith(204)
+					.should.equal true
+
+			it "should log the request", ->
+				@logger.log
+					.calledWith(doc_id: @doc_id, project_id: @project_id, "flushing doc via http")
+					.should.equal true
+
+			it "should time the request", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when an errors occurs", ->
+			beforeEach ->
+				@DocumentManager.flushDocIfLoadedWithLock = sinon.stub().callsArgWith(2, new Error("oops"))
+				@HttpController.flushDocIfLoaded(@req, @res, @next)
+
+			it "should call next with the error", ->
+				@next
+					.calledWith(new Error("oops"))
+					.should.equal true
+	
+	describe "flushAndDeleteDoc", ->
+		beforeEach ->
+			@req =
+				params:
+					project_id: @project_id
+					doc_id: @doc_id
+
+		describe "successfully", ->
+			beforeEach ->
+				@DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArgWith(2)
+				@HttpController.flushAndDeleteDoc(@req, @res, @next)
+
+			it "should flush and delete the doc", ->
+				@DocumentManager.flushAndDeleteDocWithLock
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should return a successful No Content response", ->
+				@res.send
+					.calledWith(204)
+					.should.equal true
+
+			it "should log the request", ->
+				@logger.log
+					.calledWith(doc_id: @doc_id, project_id: @project_id, "deleting doc via http")
+					.should.equal true
+
+			it "should time the request", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when an errors occurs", ->
+			beforeEach ->
+				@DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArgWith(2, new Error("oops"))
+				@HttpController.flushAndDeleteDoc(@req, @res, @next)
+
+			it "should call next with the error", ->
+				@next
+					.calledWith(new Error("oops"))
+					.should.equal true
+	
+	describe "deleteProject", ->
+		beforeEach ->
+			@req =
+				params:
+					project_id: @project_id
+
+		describe "successfully", ->
+			beforeEach ->
+				@ProjectManager.flushAndDeleteProjectWithLocks = sinon.stub().callsArgWith(1)
+				@HttpController.deleteProject(@req, @res, @next)
+
+			it "should delete the project", ->
+				@ProjectManager.flushAndDeleteProjectWithLocks
+					.calledWith(@project_id)
+					.should.equal true
+
+			it "should return a successful No Content response", ->
+				@res.send
+					.calledWith(204)
+					.should.equal true
+
+			it "should log the request", ->
+				@logger.log
+					.calledWith(project_id: @project_id, "deleting project via http")
+					.should.equal true
+
+			it "should time the request", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when an errors occurs", ->
+			beforeEach ->
+				@ProjectManager.flushAndDeleteProjectWithLocks = sinon.stub().callsArgWith(1, new Error("oops"))
+				@HttpController.deleteProject(@req, @res, @next)
+
+			it "should call next with the error", ->
+				@next
+					.calledWith(new Error("oops"))
+					.should.equal true
+	
+	describe "acceptChange", ->
+		beforeEach ->
+			@req =
+				params:
+					project_id: @project_id
+					doc_id: @doc_id
+					change_id: @change_id = "mock-change-od-1"
+
+		describe "successfully", ->
+			beforeEach ->
+				@DocumentManager.acceptChangeWithLock = sinon.stub().callsArgWith(3)
+				@HttpController.acceptChange(@req, @res, @next)
+
+			it "should accept the change", ->
+				@DocumentManager.acceptChangeWithLock
+					.calledWith(@project_id, @doc_id, @change_id)
+					.should.equal true
+
+			it "should return a successful No Content response", ->
+				@res.send
+					.calledWith(204)
+					.should.equal true
+
+			it "should log the request", ->
+				@logger.log
+					.calledWith({@project_id, @doc_id, @change_id}, "accepting change via http")
+					.should.equal true
+
+			it "should time the request", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when an errors occurs", ->
+			beforeEach ->
+				@DocumentManager.acceptChangeWithLock = sinon.stub().callsArgWith(3, new Error("oops"))
+				@HttpController.acceptChange(@req, @res, @next)
+
+			it "should call next with the error", ->
+				@next
+					.calledWith(new Error("oops"))
+					.should.equal true

+ 0 - 63
services/document-updater/test/unit/coffee/HttpController/deleteProjectTests.coffee

@@ -1,63 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/HttpController.js"
-SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors.js"
-
-describe "HttpController.deleteProject", ->
-	beforeEach ->
-		@HttpController = SandboxedModule.require modulePath, requires:
-			"./DocumentManager": @DocumentManager = {}
-			"./ProjectManager": @ProjectManager = {}
-			"logger-sharelatex" : @logger = { log: sinon.stub() }
-			"./Metrics": @Metrics = {}
-
-		@Metrics.Timer = class Timer
-			done: sinon.stub()
-
-		@project_id = "project-id-123"
-		@res =
-			send: sinon.stub()
-		@req =
-			params:
-				project_id: @project_id
-		@next = sinon.stub()
-
-	describe "successfully", ->
-		beforeEach ->
-			@ProjectManager.flushAndDeleteProjectWithLocks = sinon.stub().callsArgWith(1)
-			@HttpController.deleteProject(@req, @res, @next)
-
-		it "should delete the project", ->
-			@ProjectManager.flushAndDeleteProjectWithLocks
-				.calledWith(@project_id)
-				.should.equal true
-
-		it "should return a successful No Content response", ->
-			@res.send
-				.calledWith(204)
-				.should.equal true
-
-		it "should log the request", ->
-			@logger.log
-				.calledWith(project_id: @project_id, "deleting project via http")
-				.should.equal true
-
-		it "should time the request", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when an errors occurs", ->
-		beforeEach ->
-			@ProjectManager.flushAndDeleteProjectWithLocks = sinon.stub().callsArgWith(1, new Error("oops"))
-			@HttpController.deleteProject(@req, @res, @next)
-
-		it "should call next with the error", ->
-			@next
-				.calledWith(new Error("oops"))
-				.should.equal true
-		
-
-
-
-

+ 0 - 64
services/document-updater/test/unit/coffee/HttpController/flushAndDeleteDocTests.coffee

@@ -1,64 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/HttpController.js"
-SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors.js"
-
-describe "HttpController.flushAndDeleteDoc", ->
-	beforeEach ->
-		@HttpController = SandboxedModule.require modulePath, requires:
-			"./DocumentManager": @DocumentManager = {}
-			"./ProjectManager":{}
-			"logger-sharelatex" : @logger = { log: sinon.stub() }
-			"./Metrics": @Metrics = {}
-
-		@Metrics.Timer = class Timer
-			done: sinon.stub()
-
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@res =
-			send: sinon.stub()
-		@req =
-			params:
-				project_id: @project_id
-				doc_id: @doc_id
-		@next = sinon.stub()
-
-	describe "successfully", ->
-		beforeEach ->
-			@DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArgWith(2)
-			@HttpController.flushAndDeleteDoc(@req, @res, @next)
-
-		it "should flush and delete the doc", ->
-			@DocumentManager.flushAndDeleteDocWithLock
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should return a successful No Content response", ->
-			@res.send
-				.calledWith(204)
-				.should.equal true
-
-		it "should log the request", ->
-			@logger.log
-				.calledWith(doc_id: @doc_id, project_id: @project_id, "deleting doc via http")
-				.should.equal true
-
-		it "should time the request", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when an errors occurs", ->
-		beforeEach ->
-			@DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArgWith(2, new Error("oops"))
-			@HttpController.flushAndDeleteDoc(@req, @res, @next)
-
-		it "should call next with the error", ->
-			@next
-				.calledWith(new Error("oops"))
-				.should.equal true
-		
-
-
-

+ 0 - 65
services/document-updater/test/unit/coffee/HttpController/flushDocIfLoadedTests.coffee

@@ -1,65 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/HttpController.js"
-SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors.js"
-
-describe "HttpController.flushDocIfLoaded", ->
-	beforeEach ->
-		@HttpController = SandboxedModule.require modulePath, requires:
-			"./DocumentManager": @DocumentManager = {}
-			"./ProjectManager": {}
-			"logger-sharelatex" : @logger = { log: sinon.stub() }
-			"./Metrics": @Metrics = {}
-
-		@Metrics.Timer = class Timer
-			done: sinon.stub()
-
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@lines = ["one", "two", "three"]
-		@version = 42
-		@res =
-			send: sinon.stub()
-		@req =
-			params:
-				project_id: @project_id
-				doc_id: @doc_id
-		@next = sinon.stub()
-
-	describe "successfully", ->
-		beforeEach ->
-			@DocumentManager.flushDocIfLoadedWithLock = sinon.stub().callsArgWith(2)
-			@HttpController.flushDocIfLoaded(@req, @res, @next)
-
-		it "should flush the doc", ->
-			@DocumentManager.flushDocIfLoadedWithLock
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should return a successful No Content response", ->
-			@res.send
-				.calledWith(204)
-				.should.equal true
-
-		it "should log the request", ->
-			@logger.log
-				.calledWith(doc_id: @doc_id, project_id: @project_id, "flushing doc via http")
-				.should.equal true
-
-		it "should time the request", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when an errors occurs", ->
-		beforeEach ->
-			@DocumentManager.flushDocIfLoadedWithLock = sinon.stub().callsArgWith(2, new Error("oops"))
-			@HttpController.flushDocIfLoaded(@req, @res, @next)
-
-		it "should call next with the error", ->
-			@next
-				.calledWith(new Error("oops"))
-				.should.equal true
-		
-
-

+ 0 - 62
services/document-updater/test/unit/coffee/HttpController/flushProjectTests.coffee

@@ -1,62 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/HttpController.js"
-SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors.js"
-
-describe "HttpController.flushProject", ->
-	beforeEach ->
-		@HttpController = SandboxedModule.require modulePath, requires:
-			"./DocumentManager": @DocumentManager = {}
-			"./ProjectManager": @ProjectManager = {}
-			"logger-sharelatex" : @logger = { log: sinon.stub() }
-			"./Metrics": @Metrics = {}
-
-		@Metrics.Timer = class Timer
-			done: sinon.stub()
-
-		@project_id = "project-id-123"
-		@res =
-			send: sinon.stub()
-		@req =
-			params:
-				project_id: @project_id
-		@next = sinon.stub()
-
-	describe "successfully", ->
-		beforeEach ->
-			@ProjectManager.flushProjectWithLocks = sinon.stub().callsArgWith(1)
-			@HttpController.flushProject(@req, @res, @next)
-
-		it "should flush the project", ->
-			@ProjectManager.flushProjectWithLocks
-				.calledWith(@project_id)
-				.should.equal true
-
-		it "should return a successful No Content response", ->
-			@res.send
-				.calledWith(204)
-				.should.equal true
-
-		it "should log the request", ->
-			@logger.log
-				.calledWith(project_id: @project_id, "flushing project via http")
-				.should.equal true
-
-		it "should time the request", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when an errors occurs", ->
-		beforeEach ->
-			@ProjectManager.flushProjectWithLocks = sinon.stub().callsArgWith(1, new Error("oops"))
-			@HttpController.flushProject(@req, @res, @next)
-
-		it "should call next with the error", ->
-			@next
-				.calledWith(new Error("oops"))
-				.should.equal true
-		
-
-
-

+ 0 - 110
services/document-updater/test/unit/coffee/HttpController/getDocTests.coffee

@@ -1,110 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/HttpController.js"
-SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors.js"
-
-describe "HttpController.getDoc", ->
-	beforeEach ->
-		@HttpController = SandboxedModule.require modulePath, requires:
-			"./DocumentManager": @DocumentManager = {}
-			"./ProjectManager": {}
-			"logger-sharelatex" : @logger = { log: sinon.stub() }
-			"./Metrics": @Metrics = {}
-
-		@Metrics.Timer = class Timer
-			done: sinon.stub()
-
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@lines = ["one", "two", "three"]
-		@ops = ["mock-op-1", "mock-op-2"]
-		@version = 42
-		@fromVersion = 42
-		@res =
-			send: sinon.stub()
-		@req =
-			params:
-				project_id: @project_id
-				doc_id: @doc_id
-		@next = sinon.stub()
-
-	describe "when the document exists and no recent ops are requested", ->
-		beforeEach ->
-			@DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, @lines, @version, [])
-			@HttpController.getDoc(@req, @res, @next)
-
-		it "should get the doc", ->
-			@DocumentManager.getDocAndRecentOpsWithLock
-				.calledWith(@project_id, @doc_id, -1)
-				.should.equal true
-
-		it "should return the doc as JSON", ->
-			@res.send
-				.calledWith(JSON.stringify({
-					id: @doc_id
-					lines: @lines
-					version: @version
-					ops: []
-				}))
-				.should.equal true
-
-		it "should log the request", ->
-			@logger.log
-				.calledWith(doc_id: @doc_id, project_id: @project_id, "getting doc via http")
-				.should.equal true
-
-		it "should time the request", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when recent ops are requested", ->
-		beforeEach ->
-			@DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, @lines, @version, @ops)
-			@req.query = fromVersion: "#{@fromVersion}"
-			@HttpController.getDoc(@req, @res, @next)
-
-		it "should get the doc", ->
-			@DocumentManager.getDocAndRecentOpsWithLock
-				.calledWith(@project_id, @doc_id, @fromVersion)
-				.should.equal true
-
-		it "should return the doc as JSON", ->
-			@res.send
-				.calledWith(JSON.stringify({
-					id: @doc_id
-					lines: @lines
-					version: @version
-					ops: @ops
-				}))
-				.should.equal true
-
-		it "should log the request", ->
-			@logger.log
-				.calledWith(doc_id: @doc_id, project_id: @project_id, "getting doc via http")
-				.should.equal true
-
-		it "should time the request", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when the document does not exist", ->
-		beforeEach ->
-			@DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, null, null, null)
-			@HttpController.getDoc(@req, @res, @next)
-
-		it "should call next with NotFoundError", ->
-			@next
-				.calledWith(new Errors.NotFoundError("not found"))
-				.should.equal true
-
-	describe "when an errors occurs", ->
-		beforeEach ->
-			@DocumentManager.getDocAndRecentOpsWithLock = sinon.stub().callsArgWith(3, new Error("oops"), null, null)
-			@HttpController.getDoc(@req, @res, @next)
-
-		it "should call next with the error", ->
-			@next
-				.calledWith(new Error("oops"))
-				.should.equal true
-		
-

+ 0 - 83
services/document-updater/test/unit/coffee/HttpController/setDocTests.coffee

@@ -1,83 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/HttpController.js"
-SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors.js"
-
-describe "HttpController.setDoc", ->
-	beforeEach ->
-		@HttpController = SandboxedModule.require modulePath, requires:
-			"./DocumentManager": @DocumentManager = {}
-			"./ProjectManager": {}
-			"logger-sharelatex" : @logger = { log: sinon.stub() }
-			"./Metrics": @Metrics = {}
-
-		@Metrics.Timer = class Timer
-			done: sinon.stub()
-
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@lines = ["one", "two", "three"]
-		@source = "dropbox"
-		@user_id = "user-id-123"
-		@res =
-			send: sinon.stub()
-		@req =
-			headers: {}
-			params:
-				project_id: @project_id
-				doc_id: @doc_id
-			body:
-				lines: @lines
-				source: @source
-				user_id: @user_id
-		@next = sinon.stub()
-
-	describe "successfully", ->
-		beforeEach ->
-			@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5)
-			@HttpController.setDoc(@req, @res, @next)
-
-		it "should set the doc", ->
-			@DocumentManager.setDocWithLock
-				.calledWith(@project_id, @doc_id, @lines, @source, @user_id)
-				.should.equal true
-
-		it "should return a successful No Content response", ->
-			@res.send
-				.calledWith(204)
-				.should.equal true
-
-		it "should log the request", ->
-			@logger.log
-				.calledWith(doc_id: @doc_id, project_id: @project_id, lines: @lines, source: @source, user_id: @user_id, "setting doc via http")
-				.should.equal true
-
-		it "should time the request", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when an errors occurs", ->
-		beforeEach ->
-			@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5, new Error("oops"))
-			@HttpController.setDoc(@req, @res, @next)
-
-		it "should call next with the error", ->
-			@next
-				.calledWith(new Error("oops"))
-				.should.equal true
-
-	describe "when the payload is too large", ->
-		beforeEach ->
-			lines = []
-			for _ in [0..200000]
-				lines.push "test test test"
-			@req.body.lines = lines
-			@DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5)
-			@HttpController.setDoc(@req, @res, @next)
-
-		it 'should send back a 406 response', ->
-			@res.send.calledWith(406).should.equal true
-
-		it 'should not call setDocWithLock', ->
-			@DocumentManager.setDocWithLock.callCount.should.equal 0

+ 173 - 0
services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee

@@ -0,0 +1,173 @@
+sinon = require('sinon')
+chai = require('chai')
+should = chai.should()
+modulePath = "../../../../app/js/PersistenceManager.js"
+SandboxedModule = require('sandboxed-module')
+Errors = require "../../../../app/js/Errors"
+
+describe "PersistenceManager", ->
+	beforeEach ->
+		@PersistenceManager = SandboxedModule.require modulePath, requires:
+			"request": @request = sinon.stub()
+			"settings-sharelatex": @Settings = {}
+			"./Metrics": @Metrics =
+				Timer: class Timer
+					done: sinon.stub()
+			"logger-sharelatex": @logger = {log: sinon.stub(), err: sinon.stub()}
+		@project_id = "project-id-123"
+		@doc_id = "doc-id-123"
+		@lines = ["one", "two", "three"]
+		@version = 42
+		@callback = sinon.stub()
+		@ranges = { comments: "mock", entries: "mock" }
+		@Settings.apis =
+			web:
+				url: @url = "www.example.com"
+				user: @user = "sharelatex"
+				pass: @pass = "password"
+
+	describe "getDoc", ->
+	
+		describe "with a successful response from the web api", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify({
+					lines: @lines,
+					version: @version,
+					ranges: @ranges
+				}))
+				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+
+			it "should call the web api", ->
+				@request
+					.calledWith({
+						url: "#{@url}/project/#{@project_id}/doc/#{@doc_id}"
+						method: "GET"
+						headers:
+							"accept": "application/json"
+						auth:
+							user: @user
+							pass: @pass
+							sendImmediately: true
+						jar: false
+						timeout: 5000
+					})
+					.should.equal true
+
+			it "should call the callback with the doc lines, version and ranges", ->
+				@callback.calledWith(null, @lines, @version, @ranges).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when request returns an error", ->
+			beforeEach ->
+				@request.callsArgWith(1, @error = new Error("oops"), null, null)
+				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+
+			it "should return the error", ->
+				@callback.calledWith(@error).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when the request returns 404", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 404}, "")
+				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+				
+			it "should return a NotFoundError", ->
+				@callback.calledWith(new Errors.NotFoundError("not found")).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when the request returns an error status code", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 500}, "")
+				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+				
+			it "should return an error", ->
+				@callback.calledWith(new Error("web api error")).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when request returns an doc without lines", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(version: @version))
+				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+
+			it "should return and error", ->
+				@callback.calledWith(new Error("web API response had no doc lines")).should.equal true
+
+		describe "when request returns an doc without a version", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines))
+				@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+
+			it "should return and error", ->
+				@callback.calledWith(new Error("web API response had no valid doc version")).should.equal true
+
+	describe "setDoc", ->
+		describe "with a successful response from the web api", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 200})
+				@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback)
+
+			it "should call the web api", ->
+				@request
+					.calledWith({
+						url: "#{@url}/project/#{@project_id}/doc/#{@doc_id}"
+						json:
+							lines: @lines
+							version: @version
+							ranges: @ranges
+						method: "POST"
+						auth:
+							user: @user
+							pass: @pass
+							sendImmediately: true
+						jar: false
+						timeout: 5000
+					})
+					.should.equal true
+
+			it "should call the callback without error", ->
+				@callback.calledWith(null).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when request returns an error", ->
+			beforeEach ->
+				@request.callsArgWith(1, @error = new Error("oops"), null, null)
+				@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback)
+
+			it "should return the error", ->
+				@callback.calledWith(@error).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when the request returns 404", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 404}, "")
+				@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback)
+				
+			it "should return a NotFoundError", ->
+				@callback.calledWith(new Errors.NotFoundError("not found")).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+
+		describe "when the request returns an error status code", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 500}, "")
+				@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @ranges, @callback)
+				
+			it "should return an error", ->
+				@callback.calledWith(new Error("web api error")).should.equal true
+
+			it "should time the execution", ->
+				@Metrics.Timer::done.called.should.equal true
+

+ 0 - 104
services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee

@@ -1,104 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/PersistenceManager.js"
-SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors"
-
-describe "PersistenceManager.getDoc", ->
-	beforeEach ->
-		@PersistenceManager = SandboxedModule.require modulePath, requires:
-			"request": @request = sinon.stub()
-			"settings-sharelatex": @Settings = {}
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
-			"logger-sharelatex": @logger = {log: sinon.stub(), err: sinon.stub()}
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@lines = ["one", "two", "three"]
-		@version = 42
-		@callback = sinon.stub()
-		@Settings.apis =
-			web:
-				url: @url = "www.example.com"
-				user: @user = "sharelatex"
-				pass: @pass = "password"
-
-	describe "with a successful response from the web api", ->
-		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines, version: @version))
-			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
-
-		it "should call the web api", ->
-			@request
-				.calledWith({
-					url: "#{@url}/project/#{@project_id}/doc/#{@doc_id}"
-					method: "GET"
-					headers:
-						"accept": "application/json"
-					auth:
-						user: @user
-						pass: @pass
-						sendImmediately: true
-					jar: false
-					timeout: 5000
-				})
-				.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", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when request returns an error", ->
-		beforeEach ->
-			@request.callsArgWith(1, @error = new Error("oops"), null, null)
-			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
-
-		it "should return the error", ->
-			@callback.calledWith(@error).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when the request returns 404", ->
-		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 404}, "")
-			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
-			
-		it "should return a NotFoundError", ->
-			@callback.calledWith(new Errors.NotFoundError("not found")).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when the request returns an error status code", ->
-		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 500}, "")
-			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
-			
-		it "should return an error", ->
-			@callback.calledWith(new Error("web api error")).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when request returns an doc without lines", ->
-		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(version: @version))
-			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
-
-		it "should return and error", ->
-			@callback.calledWith(new Error("web API response had no doc lines")).should.equal true
-
-	describe "when request returns an doc without a version", ->
-		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines))
-			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
-
-		it "should return and error", ->
-			@callback.calledWith(new Error("web API response had no valid doc version")).should.equal true
-
-		

+ 0 - 90
services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee

@@ -1,90 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/PersistenceManager.js"
-SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors"
-
-describe "PersistenceManager.setDoc", ->
-	beforeEach ->
-		@PersistenceManager = SandboxedModule.require modulePath, requires:
-			"request": @request = sinon.stub()
-			"settings-sharelatex": @Settings = {}
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
-			"logger-sharelatex": @logger = {log: sinon.stub(), err: sinon.stub()}
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@lines = ["one", "two", "three"]
-		@version = 42
-		@callback = sinon.stub()
-		@Settings.apis =
-			web:
-				url: @url = "www.example.com"
-				user: @user = "sharelatex"
-				pass: @pass = "password"
-
-	describe "with a successful response from the web api", ->
-		beforeEach ->
-			@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", ->
-			@request
-				.calledWith({
-					url: "#{@url}/project/#{@project_id}/doc/#{@doc_id}"
-					body: JSON.stringify
-						lines: @lines
-						version: @version
-					method: "POST"
-					headers:
-						"content-type": "application/json"
-					auth:
-						user: @user
-						pass: @pass
-						sendImmediately: true
-					jar: false
-					timeout: 5000
-				})
-				.should.equal true
-
-		it "should call the callback without error", ->
-			@callback.calledWith(null).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when request returns an error", ->
-		beforeEach ->
-			@request.callsArgWith(1, @error = new Error("oops"), null, null)
-			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback)
-
-		it "should return the error", ->
-			@callback.calledWith(@error).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when the request returns 404", ->
-		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 404}, "")
-			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback)
-			
-		it "should return a NotFoundError", ->
-			@callback.calledWith(new Errors.NotFoundError("not found")).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when the request returns an error status code", ->
-		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 500}, "")
-			@PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @callback)
-			
-		it "should return an error", ->
-			@callback.calledWith(new Error("web api error")).should.equal true
-
-		it "should time the execution", ->
-			@Metrics.Timer::done.called.should.equal true
-

+ 107 - 54
services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee

@@ -22,6 +22,7 @@ describe "RedisManager", ->
 				projectKey: ({doc_id}) -> "ProjectId:#{doc_id}"
 				pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}"
 				docsInProject: ({project_id}) -> "DocsIn:#{project_id}"
+				ranges: ({doc_id}) -> "Ranges:#{doc_id}"
 			"logger-sharelatex": @logger = { error: sinon.stub(), log: sinon.stub(), warn: sinon.stub() }
 			"./Metrics": @metrics =
 				inc: sinon.stub()
@@ -37,39 +38,45 @@ describe "RedisManager", ->
 			@lines = ["one", "two", "three"]
 			@jsonlines = JSON.stringify @lines
 			@version = 42
+			@ranges = { comments: "mock", entries: "mock" }
+			@json_ranges = JSON.stringify @ranges
 			@rclient.get = sinon.stub()
-			@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @project_id])
-			@RedisManager.getDoc @project_id, @doc_id, @callback
-		
-		it "should get the lines from redis", ->
-			@rclient.get
-				.calledWith("doclines:#{@doc_id}")
-				.should.equal true
-		
-		it "should get the version from", ->
-			@rclient.get
-				.calledWith("DocVersion:#{@doc_id}")
-				.should.equal true
+			@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @project_id, @json_ranges])
 
-		it 'should return the document', ->
-			@callback
-				.calledWith(null, @lines, @version)
-				.should.equal true
+		describe "successfully", ->
+			beforeEach ->
+				@RedisManager.getDoc @project_id, @doc_id, @callback
 
-	describe "getDoc with an invalid project id", ->
-		beforeEach ->
-			@lines = ["one", "two", "three"]
-			@jsonlines = JSON.stringify @lines
-			@version = 42
-			@another_project_id = "project-id-456"
-			@rclient.get = sinon.stub()
-			@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @another_project_id])
-			@RedisManager.getDoc @project_id, @doc_id, @callback
+			it "should get the lines from redis", ->
+				@rclient.get
+					.calledWith("doclines:#{@doc_id}")
+					.should.equal true
+			
+			it "should get the version from", ->
+				@rclient.get
+					.calledWith("DocVersion:#{@doc_id}")
+					.should.equal true
+			
+			it "should get the ranges", ->
+				@rclient.get
+					.calledWith("Ranges:#{@doc_id}")
+					.should.equal true
 
-		it 'should return an error', ->
-			@callback
-				.calledWith(new Errors.NotFoundError("not found"))
-				.should.equal true
+			it 'should return the document', ->
+				@callback
+					.calledWith(null, @lines, @version, @ranges)
+					.should.equal true
+
+		describe "getDoc with an invalid project id", ->
+			beforeEach ->
+				@another_project_id = "project-id-456"
+				@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @another_project_id, @json_ranges])
+				@RedisManager.getDoc @project_id, @doc_id, @callback
+
+			it 'should return an error', ->
+				@callback
+					.calledWith(new Errors.NotFoundError("not found"))
+					.should.equal true
 
 	describe "getPreviousDocOpsTests", ->
 		describe "with a start and an end value", ->
@@ -161,18 +168,20 @@ describe "RedisManager", ->
 			@rclient.rpush = sinon.stub()
 			@rclient.expire = sinon.stub()
 			@rclient.ltrim = sinon.stub()
+			@rclient.del = sinon.stub()
 			@RedisManager.getDocVersion = sinon.stub()
 			
 			@lines = ["one", "two", "three"]
 			@ops = [{ op: [{ i: "foo", p: 4 }] },{ op: [{ i: "bar", p: 8 }] }]
 			@version = 42
+			@ranges = { comments: "mock", entries: "mock" }
 
 			@rclient.exec = sinon.stub().callsArg(0)
 
 		describe "with a consistent version", ->
 			beforeEach ->
 				@RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length)
-				@RedisManager.updateDocument @doc_id, @lines, @version, @ops, @callback
+				@RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback
 		
 			it "should get the current doc version to check for consistency", ->
 				@RedisManager.getDocVersion
@@ -188,6 +197,11 @@ describe "RedisManager", ->
 				@rclient.set
 					.calledWith("DocVersion:#{@doc_id}", @version)
 					.should.equal true
+				
+			it "should set the ranges", ->
+				@rclient.set
+					.calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges))
+					.should.equal true
 
 			it "should push the doc op into the doc ops list", ->
 				@rclient.rpush
@@ -210,7 +224,7 @@ describe "RedisManager", ->
 		describe "with an inconsistent version", ->
 			beforeEach ->
 				@RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length - 1)
-				@RedisManager.updateDocument @doc_id, @lines, @version, @ops, @callback
+				@RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback
 			
 			it "should not call multi.exec", ->
 				@rclient.exec.called.should.equal false
@@ -223,7 +237,7 @@ describe "RedisManager", ->
 		describe "with no updates", ->
 			beforeEach ->
 				@RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version)
-				@RedisManager.updateDocument @doc_id, @lines, @version, [], @callback
+				@RedisManager.updateDocument @doc_id, @lines, @version, [], @ranges, @callback
 			
 			it "should not do an rpush", ->
 				@rclient.rpush
@@ -234,36 +248,75 @@ describe "RedisManager", ->
 				@rclient.set
 					.calledWith("doclines:#{@doc_id}", JSON.stringify(@lines))
 					.should.equal true
+		
+		describe "with empty ranges", ->
+			beforeEach ->
+				@RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length)
+				@RedisManager.updateDocument @doc_id, @lines, @version, @ops, {}, @callback
+			
+			it "should not set the ranges", ->
+				@rclient.set
+					.calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges))
+					.should.equal false
+					
+			it "should delete the ranges key", ->
+				@rclient.del
+					.calledWith("Ranges:#{@doc_id}")
+					.should.equal true
 
 	describe "putDocInMemory", ->
-		beforeEach (done) ->
+		beforeEach ->
 			@rclient.set = sinon.stub()
 			@rclient.sadd = sinon.stub().yields()
+			@rclient.del = sinon.stub()
 			@rclient.exec.yields()
 			@lines = ["one", "two", "three"]
 			@version = 42
-			@RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, done
-		
-		it "should set the lines", ->
-			@rclient.set
-				.calledWith("doclines:#{@doc_id}", JSON.stringify @lines)
-				.should.equal true
-		
-		it "should set the version", ->
-			@rclient.set
-				.calledWith("DocVersion:#{@doc_id}", @version)
-				.should.equal true
-		
-		it "should set the project_id for the doc", ->
-			@rclient.set
-				.calledWith("ProjectId:#{@doc_id}", @project_id)
-				.should.equal true
-		
-		it "should add the doc_id to the project set", ->
-			@rclient.sadd
-				.calledWith("DocsIn:#{@project_id}", @doc_id)
-				.should.equal true
+			@ranges = { comments: "mock", entries: "mock" }
 		
+		describe "with non-empty ranges", ->
+			beforeEach (done) ->
+				@RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, done
+			
+			it "should set the lines", ->
+				@rclient.set
+					.calledWith("doclines:#{@doc_id}", JSON.stringify @lines)
+					.should.equal true
+			
+			it "should set the version", ->
+				@rclient.set
+					.calledWith("DocVersion:#{@doc_id}", @version)
+					.should.equal true
+				
+			it "should set the ranges", ->
+				@rclient.set
+					.calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges))
+					.should.equal true
+				
+			it "should set the project_id for the doc", ->
+				@rclient.set
+					.calledWith("ProjectId:#{@doc_id}", @project_id)
+					.should.equal true
+			
+			it "should add the doc_id to the project set", ->
+				@rclient.sadd
+					.calledWith("DocsIn:#{@project_id}", @doc_id)
+					.should.equal true
+	
+		describe "with empty ranges", ->
+			beforeEach (done) ->
+				@RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, {}, done
+				
+			it "should delete the ranges key", ->
+				@rclient.del
+					.calledWith("Ranges:#{@doc_id}")
+					.should.equal true
+				
+			it "should not set the ranges", ->
+				@rclient.set
+					.calledWith("Ranges:#{@doc_id}", JSON.stringify(@ranges))
+					.should.equal false
+
 	describe "removeDocFromMemory", ->
 		beforeEach (done) ->
 			@rclient.del = sinon.stub()

+ 283 - 0
services/document-updater/test/unit/coffee/ShareJS/TextTransformTests.coffee

@@ -0,0 +1,283 @@
+text = require "../../../../app/js/sharejs/types/text"
+require("chai").should()
+RangesTracker = require "../../../../app/js/RangesTracker"
+
+describe "ShareJS text type", ->
+	beforeEach ->
+		@t = "mock-thread-id"
+		
+	describe "transform", ->
+		describe "insert / insert", ->
+			it "with an insert before", ->
+				dest = []
+				text._tc(dest, { i: "foo", p: 9 }, { i: "bar", p: 3 })
+				dest.should.deep.equal [{ i: "foo", p: 12 }]
+
+			it "with an insert after", ->
+				dest = []
+				text._tc(dest, { i: "foo", p: 3 }, { i: "bar", p: 9 })
+				dest.should.deep.equal [{ i: "foo", p: 3 }]
+
+			it "with an insert at the same place with side == 'right'", ->
+				dest = []
+				text._tc(dest, { i: "foo", p: 3 }, { i: "bar", p: 3 }, 'right')
+				dest.should.deep.equal [{ i: "foo", p: 6 }]
+
+			it "with an insert at the same place with side == 'left'", ->
+				dest = []
+				text._tc(dest, { i: "foo", p: 3 }, { i: "bar", p: 3 }, 'left')
+				dest.should.deep.equal [{ i: "foo", p: 3 }]
+
+		describe "insert / delete", ->
+			it "with a delete before", ->
+				dest = []
+				text._tc(dest, { i: "foo", p: 9 }, { d: "bar", p: 3 })
+				dest.should.deep.equal [{ i: "foo", p: 6 }]
+
+			it "with a delete after", ->
+				dest = []
+				text._tc(dest, { i: "foo", p: 3 }, { d: "bar", p: 9 })
+				dest.should.deep.equal [{ i: "foo", p: 3 }]
+
+			it "with a delete at the same place with side == 'right'", ->
+				dest = []
+				text._tc(dest, { i: "foo", p: 3 }, { d: "bar", p: 3 }, 'right')
+				dest.should.deep.equal [{ i: "foo", p: 3 }]
+
+			it "with a delete at the same place with side == 'left'", ->
+				dest = []
+				
+				text._tc(dest, { i: "foo", p: 3 }, { d: "bar", p: 3 }, 'left')
+				dest.should.deep.equal [{ i: "foo", p: 3 }]
+
+		describe "delete / insert", ->
+			it "with an insert before", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 9 }, { i: "bar", p: 3 })
+				dest.should.deep.equal [{ d: "foo", p: 12 }]
+
+			it "with an insert after", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 3 }, { i: "bar", p: 9 })
+				dest.should.deep.equal [{ d: "foo", p: 3 }]
+
+			it "with an insert at the same place with side == 'right'", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 3 }, { i: "bar", p: 3 }, 'right')
+				dest.should.deep.equal [{ d: "foo", p: 6 }]
+
+			it "with an insert at the same place with side == 'left'", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 3 }, { i: "bar", p: 3 }, 'left')
+				dest.should.deep.equal [{ d: "foo", p: 6 }]
+			
+			it "with a delete that overlaps the insert location", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 3 }, { i: "bar", p: 4 })
+				dest.should.deep.equal [{ d: "f", p: 3 }, { d: "oo", p: 6 }]
+				
+
+		describe "delete / delete", ->
+			it "with a delete before", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 9 }, { d: "bar", p: 3 })
+				dest.should.deep.equal [{ d: "foo", p: 6 }]
+
+			it "with a delete after", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 3 }, { d: "bar", p: 9 })
+				dest.should.deep.equal [{ d: "foo", p: 3 }]
+
+			it "with deleting the same content", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 3 }, { d: "foo", p: 3 }, 'right')
+				dest.should.deep.equal []
+
+			it "with the delete overlapping before", ->
+				dest = []
+				text._tc(dest, { d: "foobar", p: 3 }, { d: "abcfoo", p: 0 }, 'right')
+				dest.should.deep.equal [{ d: "bar", p: 0 }]
+
+			it "with the delete overlapping after", ->
+				dest = []
+				text._tc(dest, { d: "abcfoo", p: 3 }, { d: "foobar", p: 6 })
+				dest.should.deep.equal [{ d: "abc", p: 3 }]
+
+			it "with the delete overlapping the whole delete", ->
+				dest = []
+				text._tc(dest, { d: "abcfoo123", p: 3 }, { d: "foo", p: 6 })
+				dest.should.deep.equal [{ d: "abc123", p: 3 }]
+
+			it "with the delete inside the whole delete", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 6 }, { d: "abcfoo123", p: 3 })
+				dest.should.deep.equal []
+	
+		describe "comment / insert", ->
+			it "with an insert before", ->
+				dest = []
+				text._tc(dest, { c: "foo", p: 9, @t }, { i: "bar", p: 3 })
+				dest.should.deep.equal [{ c: "foo", p: 12, @t }]
+
+			it "with an insert after", ->
+				dest = []
+				text._tc(dest, { c: "foo", p: 3, @t }, { i: "bar", p: 9 })
+				dest.should.deep.equal [{ c: "foo", p: 3, @t }]
+
+			it "with an insert at the left edge", ->
+				dest = []
+				text._tc(dest, { c: "foo", p: 3, @t }, { i: "bar", p: 3 })
+				# RangesTracker doesn't inject inserts into comments on edges, so neither should we
+				dest.should.deep.equal [{ c: "foo", p: 6, @t }]
+
+			it "with an insert at the right edge", ->
+				dest = []
+				text._tc(dest, { c: "foo", p: 3, @t }, { i: "bar", p: 6 })
+				# RangesTracker doesn't inject inserts into comments on edges, so neither should we
+				dest.should.deep.equal [{ c: "foo", p: 3, @t }]
+
+			it "with an insert in the middle", ->
+				dest = []
+				text._tc(dest, { c: "foo", p: 3, @t }, { i: "bar", p: 5 })
+				dest.should.deep.equal [{ c: "fobaro", p: 3, @t }]
+	
+		describe "comment / delete", ->
+			it "with a delete before", ->
+				dest = []
+				text._tc(dest, { c: "foo", p: 9, @t }, { d: "bar", p: 3 })
+				dest.should.deep.equal [{ c: "foo", p: 6, @t }]
+
+			it "with a delete after", ->
+				dest = []
+				text._tc(dest, { c: "foo", p: 3, @t }, { i: "bar", p: 9 })
+				dest.should.deep.equal [{ c: "foo", p: 3, @t }]
+
+			it "with a delete overlapping the comment content before", ->
+				dest = []
+				text._tc(dest, { c: "foobar", p: 6, @t }, { d: "123foo", p: 3 })
+				dest.should.deep.equal [{ c: "bar", p: 3, @t }]
+
+			it "with a delete overlapping the comment content after", ->
+				dest = []
+				text._tc(dest, { c: "foobar", p: 6, @t }, { d: "bar123", p: 9 })
+				dest.should.deep.equal [{ c: "foo", p: 6, @t }]
+
+			it "with a delete overlapping the comment content in the middle", ->
+				dest = []
+				text._tc(dest, { c: "foo123bar", p: 6, @t }, { d: "123", p: 9 })
+				dest.should.deep.equal [{ c: "foobar", p: 6, @t }]
+
+			it "with a delete overlapping the whole comment", ->
+				dest = []
+				text._tc(dest, { c: "foo", p: 6, @t }, { d: "123foo456", p: 3 })
+				dest.should.deep.equal [{ c: "", p: 3, @t }]
+	
+		describe "comment / insert", ->
+			it "should not do anything", ->
+				dest = []
+				text._tc(dest, { i: "foo", p: 6 }, { c: "bar", p: 3 })
+				dest.should.deep.equal [{ i: "foo", p: 6 }]
+	
+		describe "comment / delete", ->
+			it "should not do anything", ->
+				dest = []
+				text._tc(dest, { d: "foo", p: 6 }, { c: "bar", p: 3 })
+				dest.should.deep.equal [{ d: "foo", p: 6 }]
+	
+		describe "comment / comment", ->
+			it "should not do anything", ->
+				dest = []
+				text._tc(dest, { c: "foo", p: 6 }, { c: "bar", p: 3 })
+				dest.should.deep.equal [{ c: "foo", p: 6 }]
+
+	describe "apply", ->
+		it "should apply an insert", ->
+			text.apply("foo", [{ i: "bar", p: 2 }]).should.equal "fobaro"
+
+		it "should apply a delete", ->
+			text.apply("foo123bar", [{ d: "123", p: 3 }]).should.equal "foobar"
+
+		it "should do nothing with a comment", ->
+			text.apply("foo123bar", [{ c: "123", p: 3 }]).should.equal "foo123bar"
+		
+		it "should throw an error when deleted content does not match", ->
+			(() ->
+				text.apply("foo123bar", [{ d: "456", p: 3 }])
+			).should.throw(Error)
+		
+		it "should throw an error when comment content does not match", ->
+			(() ->
+				text.apply("foo123bar", [{ c: "456", p: 3 }])
+			).should.throw(Error)
+	
+	describe "applying ops and comments in different orders", ->
+		it "should not matter which op or comment is applied first", ->
+			transform = (op1, op2, side) ->
+				d = []
+				text._tc(d, op1, op2, side)
+				return d
+			
+			applySnapshot = (snapshot, op) ->
+				return text.apply(snapshot, op)
+			
+			applyRanges = (rangesTracker, ops) ->
+				for op in ops
+					rangesTracker.applyOp(op, {})
+				return rangesTracker
+			
+			commentsEqual = (comments1, comments2) ->
+				return false if comments1.length != comments2.length
+				comments1.sort (a,b) ->
+					if a.offset - b.offset == 0
+						return a.length - b.length
+					else
+						return a.offset - b.offset
+				comments2.sort (a,b) ->
+					if a.offset - b.offset == 0
+						return a.length - b.length
+					else
+						return a.offset - b.offset
+				for comment1, i in comments1
+					comment2 = comments2[i]
+					if comment1.offset != comment2.offset or comment1.length != comment2.length
+						return false
+				return true
+			
+			SNAPSHOT = "123"
+			
+			OPS = []
+			# Insert ops
+			for p in [0..SNAPSHOT.length]
+				OPS.push {i: "a", p: p}
+				OPS.push {i: "bc", p: p}
+			for p in [0..(SNAPSHOT.length-1)]
+				for length in [1..(SNAPSHOT.length - p)]
+					OPS.push {d: SNAPSHOT.slice(p, p+length), p}
+			for p in [0..(SNAPSHOT.length-1)]
+				for length in [1..(SNAPSHOT.length - p)]
+					OPS.push {c: SNAPSHOT.slice(p, p+length), p, @t}
+
+			for op1 in OPS
+				for op2 in OPS
+					op1_t = transform(op1, op2, "left")
+					op2_t = transform(op2, op1, "right")
+					
+					rt12 = new RangesTracker()
+					snapshot12 = applySnapshot(applySnapshot(SNAPSHOT, [op1]), op2_t)
+					applyRanges(rt12, [op1])
+					applyRanges(rt12, op2_t)
+					
+					rt21 = new RangesTracker()
+					snapshot21 = applySnapshot(applySnapshot(SNAPSHOT, [op2]), op1_t)
+					applyRanges(rt21, [op2])
+					applyRanges(rt21, op1_t)
+					
+					if snapshot12 != snapshot21
+						console.error {op1, op2, op1_t, op2_t, snapshot12, snapshot21}, "Ops are not consistent"
+						throw new Error("OT is inconsistent")
+					
+					if !commentsEqual(rt12.comments, rt21.comments)
+						console.log rt12.comments
+						console.log rt21.comments
+						console.error {op1, op2, op1_t, op2_t, rt12_comments: rt12.comments, rt21_comments: rt21.comments}, "Comments are not consistent"
+						throw new Error("OT is inconsistent")

+ 0 - 55
services/document-updater/test/unit/coffee/ShareJsDB/GetOpsTests.coffee

@@ -1,55 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/ShareJsDB.js"
-SandboxedModule = require('sandboxed-module')
-
-describe "ShareJsDB.getOps", ->
-	beforeEach ->
-		@doc_id = "document-id"
-		@project_id = "project-id"
-		@doc_key = "#{@project_id}:#{@doc_id}"
-		@callback = sinon.stub()
-		@ops = [{p: 20, t: "foo"}]
-		@redis_ops = (JSON.stringify(op) for op in @ops)
-		@ShareJsDB = SandboxedModule.require modulePath, requires:
-			"./RedisManager": @RedisManager = {}
-			"./DocumentManager":{}
-			"logger-sharelatex": {}
-		@db = new @ShareJsDB()
-
-	describe "with start == end", ->
-		beforeEach ->
-			@start = @end = 42
-			@db.getOps @doc_key, @start, @end, @callback
-
-		it "should return an empty array", ->
-			@callback.calledWith(null, []).should.equal true
-	
-	describe "with a non empty range", ->
-		beforeEach ->
-			@start = 35
-			@end = 42
-			@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
-			@db.getOps @doc_key, @start, @end, @callback
-
-		it "should get the range from redis", ->
-			@RedisManager.getPreviousDocOps
-				.calledWith(@doc_id, @start, @end-1)
-				.should.equal true
-
-		it "should return the ops", ->
-			@callback.calledWith(null, @ops).should.equal true
-
-	describe "with no specified end", ->
-		beforeEach ->
-			@start = 35
-			@end = null
-			@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
-			@db.getOps @doc_key, @start, @end, @callback
-		
-		it "should get until the end of the list", ->
-			@RedisManager.getPreviousDocOps
-				.calledWith(@doc_id, @start, -1)
-				.should.equal true
-

+ 0 - 87
services/document-updater/test/unit/coffee/ShareJsDB/GetSnapshotTests.coffee

@@ -1,87 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-expect = chai.expect
-modulePath = "../../../../app/js/ShareJsDB.js"
-SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors"
-
-describe "ShareJsDB.getSnapshot", ->
-	beforeEach ->
-		@doc_id = "document-id"
-		@project_id = "project-id"
-		@doc_key = "#{@project_id}:#{@doc_id}"
-		@callback = sinon.stub()
-		@ShareJsDB = SandboxedModule.require modulePath, requires:
-			"./DocumentManager": @DocumentManager = {}
-			"./RedisManager": {}
-			"./DocOpsManager": {}
-			"logger-sharelatex": {}
-		@db = new @ShareJsDB()
-
-		@version = 42
-
-	describe "with a text document", ->
-		beforeEach ->
-			@lines = ["one", "two", "three"]
-
-		describe "successfully", ->
-			beforeEach ->
-				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
-				@db.getSnapshot @doc_key, @callback
-
-			it "should get the doc", ->
-				@DocumentManager.getDoc
-					.calledWith(@project_id, @doc_id)
-					.should.equal true
-
-			it "should return the doc lines", ->
-				@callback.args[0][1].snapshot.should.equal @lines.join("\n")
-
-			it "should return the doc version", ->
-				@callback.args[0][1].v.should.equal @version
-
-			it "should return the type as text", ->
-				@callback.args[0][1].type.should.equal "text"
-
-		describe "when the doclines do not exist", ->
-			beforeEach ->
-				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, null, null)
-				@db.getSnapshot @doc_key, @callback
-
-			it "should return the callback with a NotFoundError", ->
-				@callback.calledWith(new Errors.NotFoundError("not found")).should.equal true
-
-		describe "when getDoc returns an error", ->
-			beforeEach ->
-				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, @error = new Error("oops"), null, null)
-				@db.getSnapshot @doc_key, @callback
-
-			it "should return the callback with an error", ->
-				@callback.calledWith(@error).should.equal true
-
-	describe "with a JSON document", ->
-		beforeEach ->
-			@lines = [{text: "one"}, {text:"two"}, {text:"three"}]
-
-		describe "successfully", ->
-			beforeEach ->
-				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version)
-				@db.getSnapshot @doc_key, @callback
-
-			it "should get the doc", ->
-				@DocumentManager.getDoc
-					.calledWith(@project_id, @doc_id)
-					.should.equal true
-
-			it "should return the doc lines", ->
-				expect(@callback.args[0][1].snapshot).to.deep.equal lines: @lines
-
-			it "should return the doc version", ->
-				@callback.args[0][1].v.should.equal @version
-
-			it "should return the type as text", ->
-				@callback.args[0][1].type.should.equal "json"
-
-
-	

+ 93 - 0
services/document-updater/test/unit/coffee/ShareJsDB/ShareJsDBTests.coffee

@@ -0,0 +1,93 @@
+sinon = require('sinon')
+chai = require('chai')
+should = chai.should()
+expect = chai.expect
+modulePath = "../../../../app/js/ShareJsDB.js"
+SandboxedModule = require('sandboxed-module')
+Errors = require "../../../../app/js/Errors"
+
+describe "ShareJsDB", ->
+	beforeEach ->
+		@doc_id = "document-id"
+		@project_id = "project-id"
+		@doc_key = "#{@project_id}:#{@doc_id}"
+		@callback = sinon.stub()
+		@ShareJsDB = SandboxedModule.require modulePath, requires:
+			"./RedisManager": @RedisManager = {}
+
+		@version = 42
+		@lines = ["one", "two", "three"]
+		@db = new @ShareJsDB(@project_id, @doc_id, @lines, @version)
+
+	describe "getSnapshot", ->
+		describe "successfully", ->
+			beforeEach ->
+				@db.getSnapshot @doc_key, @callback
+
+			it "should return the doc lines", ->
+				@callback.args[0][1].snapshot.should.equal @lines.join("\n")
+
+			it "should return the doc version", ->
+				@callback.args[0][1].v.should.equal @version
+
+			it "should return the type as text", ->
+				@callback.args[0][1].type.should.equal "text"
+
+		describe "when the key does not match", ->
+			beforeEach ->
+				@db.getSnapshot "bad:key", @callback
+
+			it "should return the callback with a NotFoundError", ->
+				@callback.calledWith(new Errors.NotFoundError("not found")).should.equal true
+
+	describe "getOps", ->
+		describe "with start == end", ->
+			beforeEach ->
+				@start = @end = 42
+				@db.getOps @doc_key, @start, @end, @callback
+
+			it "should return an empty array", ->
+				@callback.calledWith(null, []).should.equal true
+		
+		describe "with a non empty range", ->
+			beforeEach ->
+				@start = 35
+				@end = 42
+				@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
+				@db.getOps @doc_key, @start, @end, @callback
+
+			it "should get the range from redis", ->
+				@RedisManager.getPreviousDocOps
+					.calledWith(@doc_id, @start, @end-1)
+					.should.equal true
+
+			it "should return the ops", ->
+				@callback.calledWith(null, @ops).should.equal true
+
+		describe "with no specified end", ->
+			beforeEach ->
+				@start = 35
+				@end = null
+				@RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops)
+				@db.getOps @doc_key, @start, @end, @callback
+			
+			it "should get until the end of the list", ->
+				@RedisManager.getPreviousDocOps
+					.calledWith(@doc_id, @start, -1)
+					.should.equal true
+
+	describe "writeOps", ->
+		describe "writing an op", ->
+			beforeEach ->
+				@opData =
+					op: {p: 20, t: "foo"}
+					meta: {source: "bar"}
+					v: @version
+				@db.writeOp @doc_key, @opData, @callback
+
+			it "should write into appliedOps", ->
+				expect(@db.appliedOps[@doc_key]).to.deep.equal [@opData]
+
+			it "should call the callback without an error", ->
+				@callback.called.should.equal true
+				(@callback.args[0][0]?).should.equal false

+ 0 - 39
services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee

@@ -1,39 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-expect = chai.expect
-should = chai.should()
-modulePath = "../../../../app/js/ShareJsDB.js"
-SandboxedModule = require('sandboxed-module')
-
-describe "ShareJsDB.writeOps", ->
-	beforeEach ->
-		@project_id = "project-id"
-		@doc_id = "document-id"
-		@doc_key = "#{@project_id}:#{@doc_id}"
-		@callback = sinon.stub()
-		@opData =
-			op: {p: 20, t: "foo"}
-			meta: {source: "bar"}
-		@ShareJsDB = SandboxedModule.require modulePath, requires:
-			"./RedisManager": @RedisManager = {}
-			"./DocOpsManager": @DocOpsManager = {}
-			"./DocumentManager": {}
-			"logger-sharelatex": @logger = {error: sinon.stub()}
-		@db = new @ShareJsDB()
-
-	describe "writing an op", ->
-		beforeEach ->
-			@version = 42
-			@opData.v = @version
-			@db.writeOp @doc_key, @opData, @callback
-
-		it "should write into appliedOps", ->
-			expect(@db.appliedOps[@doc_key]).to.deep.equal [@opData]
-
-		it "should call the callback without an error", ->
-			@callback.called.should.equal true
-			(@callback.args[0][0]?).should.equal false
-
-
-	
-

+ 12 - 32
services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee

@@ -17,12 +17,16 @@ describe "ShareJsUpdateManager", ->
 				"./ShareJsDB" : @ShareJsDB = { mockDB: true }
 				"redis-sharelatex" : createClient: () => @rclient = auth:->
 				"logger-sharelatex": @logger = { log: sinon.stub() }
+				"./WebRedisManager": @WebRedisManager = {}
 			globals:
 				clearTimeout: @clearTimeout = sinon.stub()
 
 	describe "applyUpdate", ->
 		beforeEach ->
+			@lines = ["one", "two"]
 			@version = 34
+			@update = {p: 4, t: "foo"}
+			@updatedDocLines = ["onefoo", "two"]
 			@model =
 				applyOp: sinon.stub().callsArg(2)
 				getSnapshot: sinon.stub()
@@ -31,20 +35,19 @@ describe "ShareJsUpdateManager", ->
 			@ShareJsUpdateManager.getNewShareJsModel = sinon.stub().returns(@model)
 			@ShareJsUpdateManager._listenForOps = sinon.stub()
 			@ShareJsUpdateManager.removeDocFromCache = sinon.stub().callsArg(1)
-			@update = {p: 4, t: "foo"}
-			@updatedDocLines = ["one", "two"]
 
 		describe "successfully", ->
 			beforeEach (done) ->
 				@model.getSnapshot.callsArgWith(1, null, {snapshot: @updatedDocLines.join("\n"), v: @version})
 				@model.db.appliedOps["#{@project_id}:#{@doc_id}"] = @appliedOps = ["mock-ops"]
-				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, (err, docLines, version, appliedOps) =>
+				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version, appliedOps) =>
 					@callback(err, docLines, version, appliedOps)
 					done()
 
 			it "should create a new ShareJs model", ->
 				@ShareJsUpdateManager.getNewShareJsModel
-					.called.should.equal true
+					.calledWith(@project_id, @doc_id, @lines, @version)
+					.should.equal true
 
 			it "should listen for ops on the model", ->
 				@ShareJsUpdateManager._listenForOps
@@ -67,34 +70,22 @@ describe "ShareJsUpdateManager", ->
 		describe "when applyOp fails", ->
 			beforeEach (done) ->
 				@error = new Error("Something went wrong")
-				@ShareJsUpdateManager._sendError = sinon.stub()
 				@model.applyOp = sinon.stub().callsArgWith(2, @error)
-				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, (err, docLines, version) =>
+				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version) =>
 					@callback(err, docLines, version)
 					done()
 
-			it "should call sendError with the error", ->
-				@ShareJsUpdateManager._sendError
-					.calledWith(@project_id, @doc_id, @error)
-					.should.equal true
-
 			it "should call the callback with the error", ->
 				@callback.calledWith(@error).should.equal true
 
 		describe "when getSnapshot fails", ->
 			beforeEach (done) ->
 				@error = new Error("Something went wrong")
-				@ShareJsUpdateManager._sendError = sinon.stub()
 				@model.getSnapshot.callsArgWith(1, @error)
-				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, (err, docLines, version) =>
+				@ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version) =>
 					@callback(err, docLines, version)
 					done()
 
-			it "should call sendError with the error", ->
-				@ShareJsUpdateManager._sendError
-					.calledWith(@project_id, @doc_id, @error)
-					.should.equal true
-
 			it "should call the callback with the error", ->
 				@callback.calledWith(@error).should.equal true
 
@@ -114,22 +105,11 @@ describe "ShareJsUpdateManager", ->
 				@opData =
 					op: {t: "foo", p: 1}
 					meta: source: "bar"
-				@rclient.publish = sinon.stub()
+				@WebRedisManager.sendData = sinon.stub()
 				@callback("#{@project_id}:#{@doc_id}", @opData)
 
 			it "should publish the op to redis", ->
-				@rclient.publish
-					.calledWith("applied-ops", JSON.stringify(project_id: @project_id, doc_id: @doc_id, op: @opData))
+				@WebRedisManager.sendData
+					.calledWith({project_id: @project_id, doc_id: @doc_id, op: @opData})
 					.should.equal true
 
-	describe "_sendError", ->
-		beforeEach ->
-			@error_text = "Something went wrong"
-			@rclient.publish = sinon.stub()
-			@ShareJsUpdateManager._sendError(@project_id, @doc_id, new Error(@error_text))
-
-		it "should publish the error to the redis stream", ->
-			@rclient.publish
-				.calledWith("applied-ops", JSON.stringify(project_id: @project_id, doc_id: @doc_id, error: @error_text))
-				.should.equal true
-

+ 111 - 8
services/document-updater/test/unit/coffee/UpdateManager/ApplyingUpdates.coffee → services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee

@@ -14,12 +14,14 @@ describe "UpdateManager", ->
 			"./RedisManager" : @RedisManager = {}
 			"./WebRedisManager" : @WebRedisManager = {}
 			"./ShareJsUpdateManager" : @ShareJsUpdateManager = {}
-			"./TrackChangesManager" : @TrackChangesManager = {}
+			"./HistoryManager" : @HistoryManager = {}
 			"logger-sharelatex": @logger = { log: sinon.stub() }
 			"./Metrics": @Metrics =
 				Timer: class Timer
 					done: sinon.stub()
 			"settings-sharelatex": Settings = {}
+			"./DocumentManager": @DocumentManager = {}
+			"./RangesManager": @RangesManager = {}
 
 	describe "processOutstandingUpdates", ->
 		beforeEach ->
@@ -155,10 +157,16 @@ describe "UpdateManager", ->
 			@update = {op: [{p: 42, i: "foo"}]}
 			@updatedDocLines = ["updated", "lines"]
 			@version = 34
+			@lines = ["original", "lines"]
+			@ranges = { entries: "mock", comments: "mock" }
+			@updated_ranges = { entries: "updated", comments: "updated" }
 			@appliedOps = ["mock-applied-ops"]
-			@ShareJsUpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null, @updatedDocLines, @version, @appliedOps)
-			@RedisManager.updateDocument = sinon.stub().callsArg(4)
-			@TrackChangesManager.pushUncompressedHistoryOps = sinon.stub().callsArg(3)
+			@DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges)
+			@RangesManager.applyUpdate = sinon.stub().yields(null, @updated_ranges)
+			@ShareJsUpdateManager.applyUpdate = sinon.stub().yields(null, @updatedDocLines, @version, @appliedOps)
+			@RedisManager.updateDocument = sinon.stub().yields()
+			@WebRedisManager.sendData = sinon.stub()
+			@HistoryManager.pushUncompressedHistoryOps = sinon.stub().callsArg(3)
 		
 		describe "normally", ->
 			beforeEach ->
@@ -166,16 +174,21 @@ describe "UpdateManager", ->
 			
 			it "should apply the updates via ShareJS", ->
 				@ShareJsUpdateManager.applyUpdate
-					.calledWith(@project_id, @doc_id, @update)
+					.calledWith(@project_id, @doc_id, @update, @lines, @version)
+					.should.equal true
+			
+			it "should update the ranges", ->
+				@RangesManager.applyUpdate
+					.calledWith(@project_id, @doc_id, @ranges, @appliedOps)
 					.should.equal true
 
 			it "should save the document", ->
 				@RedisManager.updateDocument
-					.calledWith(@doc_id, @updatedDocLines, @version, @appliedOps)
+					.calledWith(@doc_id, @updatedDocLines, @version, @appliedOps, @updated_ranges)
 					.should.equal true
 			
-			it "should push the applied ops into the track changes queue", ->
-				@TrackChangesManager.pushUncompressedHistoryOps
+			it "should push the applied ops into the history queue", ->
+				@HistoryManager.pushUncompressedHistoryOps
 					.calledWith(@project_id, @doc_id, @appliedOps)
 					.should.equal true
 
@@ -194,4 +207,94 @@ describe "UpdateManager", ->
 				
 				# \uFFFD is 'replacement character'
 				@update.op[0].i.should.equal "\uFFFD\uFFFD"
+		
+		describe "with an error", ->
+			beforeEach ->
+				@error = new Error("something went wrong")
+				@ShareJsUpdateManager.applyUpdate = sinon.stub().yields(@error)
+				@UpdateManager.applyUpdate @project_id, @doc_id, @update, @callback
+			
+			it "should call WebRedisManager.sendData with the error", ->
+				@WebRedisManager.sendData
+					.calledWith({
+						project_id: @project_id,
+						doc_id: @doc_id,
+						error: @error.message
+					})
+					.should.equal true
+
+			it "should call the callback with the error", ->
+				@callback.calledWith(@error).should.equal true
+			
+
+	describe "lockUpdatesAndDo", ->
+		beforeEach ->
+			@method = sinon.stub().callsArgWith(3, null, @response_arg1)
+			@callback = sinon.stub()
+			@arg1 = "argument 1"
+			@response_arg1 = "response argument 1"
+			@lockValue = "mock-lock-value"
+			@LockManager.getLock = sinon.stub().callsArgWith(1, null, @lockValue)
+			@LockManager.releaseLock = sinon.stub().callsArg(2)
+
+		describe "successfully", ->
+			beforeEach ->
+				@UpdateManager.continueProcessingUpdatesWithLock = sinon.stub()
+				@UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
+				@UpdateManager.lockUpdatesAndDo @method, @project_id, @doc_id, @arg1, @callback
+
+			it "should lock the doc", ->
+				@LockManager.getLock
+					.calledWith(@doc_id)
+					.should.equal true
+
+			it "should process any outstanding updates", ->
+				@UpdateManager.processOutstandingUpdates
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+			it "should call the method", ->
+				@method
+					.calledWith(@project_id, @doc_id, @arg1)
+					.should.equal true
+
+			it "should return the method response to the callback", ->
+				@callback
+					.calledWith(null, @response_arg1)
+					.should.equal true
+
+			it "should release the lock", ->
+				@LockManager.releaseLock
+					.calledWith(@doc_id, @lockValue)
+					.should.equal true
+
+			it "should continue processing updates", ->
+				@UpdateManager.continueProcessingUpdatesWithLock
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
+
+		describe "when processOutstandingUpdates returns an error", ->
+			beforeEach ->
+				@UpdateManager.processOutstandingUpdates = sinon.stub().callsArgWith(2, @error = new Error("Something went wrong"))
+				@UpdateManager.lockUpdatesAndDo @method, @project_id, @doc_id, @arg1, @callback
+
+			it "should free the lock", ->
+				@LockManager.releaseLock.calledWith(@doc_id, @lockValue).should.equal true
+				
+			it "should return the error in the callback", ->
+				@callback.calledWith(@error).should.equal true
+
+		describe "when the method returns an error", ->
+			beforeEach ->
+				@UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
+				@method = sinon.stub().callsArgWith(3, @error = new Error("something went wrong"), @response_arg1)
+				@UpdateManager.lockUpdatesAndDo @method, @project_id, @doc_id, @arg1, @callback
+
+			it "should free the lock", ->
+				@LockManager.releaseLock.calledWith(@doc_id, @lockValue).should.equal true
+				
+			it "should return the error in the callback", ->
+				@callback.calledWith(@error).should.equal true
+
+
 

+ 0 - 90
services/document-updater/test/unit/coffee/UpdateManager/lockUpdatesAndDoTests.coffee

@@ -1,90 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/UpdateManager.js"
-SandboxedModule = require('sandboxed-module')
-
-describe 'UpdateManager - lockUpdatesAndDo', ->
-	beforeEach ->
-		@UpdateManager = SandboxedModule.require modulePath, requires:
-			"./LockManager" : @LockManager = {}
-			"./RedisManager" : @RedisManager = {}
-			"./WebRedisManager" : @WebRedisManager = {}
-			"./ShareJsUpdateManager" : @ShareJsUpdateManager = {}
-			"./TrackChangesManager" : @TrackChangesManager = {}
-			"logger-sharelatex": @logger = { log: sinon.stub() }
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
-			"settings-sharelatex": Settings = {}
-		@project_id = "project-id-123"
-		@doc_id = "doc-id-123"
-		@method = sinon.stub().callsArgWith(3, null, @response_arg1)
-		@callback = sinon.stub()
-		@arg1 = "argument 1"
-		@response_arg1 = "response argument 1"
-		@lockValue = "mock-lock-value"
-		@LockManager.getLock = sinon.stub().callsArgWith(1, null, @lockValue)
-		@LockManager.releaseLock = sinon.stub().callsArg(2)
-
-	describe "successfully", ->
-		beforeEach ->
-			@UpdateManager.continueProcessingUpdatesWithLock = sinon.stub()
-			@UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
-			@UpdateManager.lockUpdatesAndDo @method, @project_id, @doc_id, @arg1, @callback
-
-		it "should lock the doc", ->
-			@LockManager.getLock
-				.calledWith(@doc_id)
-				.should.equal true
-
-		it "should process any outstanding updates", ->
-			@UpdateManager.processOutstandingUpdates
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-		it "should call the method", ->
-			@method
-				.calledWith(@project_id, @doc_id, @arg1)
-				.should.equal true
-
-		it "should return the method response to the callback", ->
-			@callback
-				.calledWith(null, @response_arg1)
-				.should.equal true
-
-		it "should release the lock", ->
-			@LockManager.releaseLock
-				.calledWith(@doc_id, @lockValue)
-				.should.equal true
-
-		it "should continue processing updates", ->
-			@UpdateManager.continueProcessingUpdatesWithLock
-				.calledWith(@project_id, @doc_id)
-				.should.equal true
-
-	describe "when processOutstandingUpdates returns an error", ->
-		beforeEach ->
-			@UpdateManager.processOutstandingUpdates = sinon.stub().callsArgWith(2, @error = new Error("Something went wrong"))
-			@UpdateManager.lockUpdatesAndDo @method, @project_id, @doc_id, @arg1, @callback
-
-		it "should free the lock", ->
-			@LockManager.releaseLock.calledWith(@doc_id, @lockValue).should.equal true
-			
-		it "should return the error in the callback", ->
-			@callback.calledWith(@error).should.equal true
-
-	describe "when the method returns an error", ->
-		beforeEach ->
-			@UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
-			@method = sinon.stub().callsArgWith(3, @error = new Error("something went wrong"), @response_arg1)
-			@UpdateManager.lockUpdatesAndDo @method, @project_id, @doc_id, @arg1, @callback
-
-		it "should free the lock", ->
-			@LockManager.releaseLock.calledWith(@doc_id, @lockValue).should.equal true
-			
-		it "should return the error in the callback", ->
-			@callback.calledWith(@error).should.equal true
-
-
-