Explorar el Código

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

Ja track changes
James Allen hace 9 años
padre
commit
f9d6b3f250

+ 1 - 0
services/docstore/app.coffee

@@ -29,6 +29,7 @@ app.param 'doc_id', (req, res, next, doc_id) ->
 		next new Error("invalid doc id")
 
 app.get  '/project/:project_id/doc', HttpController.getAllDocs
+app.get  '/project/:project_id/ranges', HttpController.getAllRanges
 app.get  '/project/:project_id/doc/:doc_id', HttpController.getDoc
 app.get  '/project/:project_id/doc/:doc_id/raw', HttpController.getRawDoc
 # Add 16kb overhead for the JSON encoding

+ 2 - 2
services/docstore/app/coffee/DocArchiveManager.coffee

@@ -11,7 +11,7 @@ thirtySeconds = 30 * 1000
 module.exports = DocArchive =
 
 	archiveAllDocs: (project_id, callback = (err, docs) ->) ->
-		MongoManager.getProjectsDocs project_id, {include_deleted: true}, (err, docs) ->
+		MongoManager.getProjectsDocs project_id, {include_deleted: true}, {lines: true, rev: true, inS3: true}, (err, docs) ->
 			if err?
 				return callback(err)
 			else if !docs?
@@ -67,7 +67,7 @@ module.exports = DocArchive =
 			if err? || res.statusCode != 200
 				logger.err err:err, res:res, project_id:project_id, doc_id:doc_id, "something went wrong unarchiving doc from aws"
 				return callback new Errors.NotFoundError("Error in S3 request")
-			MongoManager.upsertIntoDocCollection project_id, doc_id.toString(), lines, (err) ->
+			MongoManager.upsertIntoDocCollection project_id, doc_id.toString(), {lines}, (err) ->
 				return callback(err) if err?
 				logger.log project_id: project_id, doc_id: doc_id, "deleting doc from s3"
 				request.del options, (err, res, body)->

+ 54 - 41
services/docstore/app/coffee/DocManager.coffee

@@ -3,6 +3,7 @@ Errors = require "./Errors"
 logger = require "logger-sharelatex"
 _ = require "underscore"
 DocArchive = require "./DocArchiveManager"
+RangeManager = require "./RangeManager"
 
 module.exports = DocManager =
 	# TODO: For historical reasons, the doc version is currently stored in the docOps
@@ -10,7 +11,7 @@ module.exports = DocManager =
 	# migrate this version property to be part of the docs collection, to guarantee
 	# consitency between lines and version when writing/reading, and for a simpler schema.
 	getDoc: (project_id, doc_id, filter = { version: false }, callback = (error, doc) ->) ->
-		MongoManager.findDoc project_id, doc_id, (err, doc)->
+		MongoManager.findDoc project_id, doc_id, filter, (err, doc)->
 			if err?
 				return callback(err)
 			else if !doc?
@@ -30,9 +31,9 @@ module.exports = DocManager =
 				else
 					callback err, doc
 
-	getAllNonDeletedDocs: (project_id, callback = (error, docs) ->) ->
+	getAllNonDeletedDocs: (project_id, filter, callback = (error, docs) ->) ->
 		DocArchive.unArchiveAllDocs project_id, (error) ->
-			MongoManager.getProjectsDocs project_id, {include_deleted: false}, (error, docs) ->
+			MongoManager.getProjectsDocs project_id, {include_deleted: false}, filter, (error, docs) ->
 				if err?
 					return callback(error)
 				else if !docs?
@@ -40,52 +41,64 @@ module.exports = DocManager =
 				else
 					return callback(null, docs)
 
-	updateDoc: (project_id, doc_id, lines, version, callback = (error, modified, rev) ->) ->
-		DocManager.getDoc project_id, doc_id, {version: true}, (err, doc)->
+	updateDoc: (project_id, doc_id, lines, version, ranges, callback = (error, modified, rev) ->) ->
+		if !lines? or !version?
+			return callback(new Error("no lines or version provided"))
+	
+		DocManager.getDoc project_id, doc_id, {version: true, rev: true, lines: true, version: true, ranges: true}, (err, doc)->
 			if err? and !(err instanceof Errors.NotFoundError)
 				logger.err project_id: project_id, doc_id: doc_id, err:err, "error getting document for update"
 				return callback(err)
+			
+			ranges = RangeManager.jsonRangesToMongo(ranges)
 
-			isNewDoc = lines.length == 0
-			linesAreSame =  _.isEqual(doc?.lines, lines)
-			if version?
-				versionsAreSame = (doc?.version == version)
+			if !doc?
+				# If the document doesn't exist, we'll make sure to create/update all parts of it.
+				updateLines = true
+				updateVersion = true
+				updateRanges = true
 			else
-				versionsAreSame = true
+				updateLines = not _.isEqual(doc.lines, lines)
+				updateVersion = (doc.version != version)
+				updateRanges = RangeManager.shouldUpdateRanges(doc.ranges, ranges)
+			
+			modified = false
+			rev = doc?.rev || 0
 
-			if linesAreSame and versionsAreSame and !isNewDoc
-				logger.log project_id: project_id, doc_id: doc_id, rev: doc?.rev, "doc lines have not changed - not updating"
-				return callback null, false, doc?.rev
-			else
-				oldRev = doc?.rev || 0
-				logger.log {
-					project_id: project_id
-					doc_id: doc_id,
-					oldDocLines: doc?.lines
-					newDocLines: lines
-					rev: oldRev
-					oldVersion: doc?.version
-					newVersion: version
-				}, "updating doc lines"
-				MongoManager.upsertIntoDocCollection project_id, doc_id, lines, (error)->
-					return callback(callback) if error?
-					# TODO: While rolling out this code, setting the version via the docstore is optional,
-					# so if it hasn't been passed, just ignore it. Once the docupdater has totally
-					# handed control of this to the docstore, we can assume it will always be passed
-					# and an error guard on it not being set instead.
-					if version?
-						MongoManager.setDocVersion doc_id, version, (error) ->
-							return callback(error) if error?
-							callback null, true, oldRev + 1 # rev will have been incremented in mongo by MongoManager.updateDoc
-					else
-						callback null, true, oldRev + 1 # rev will have been incremented in mongo by MongoManager.updateDoc
+			updateLinesAndRangesIfNeeded = (cb) ->
+				if updateLines or updateRanges
+					update = {}
+					if updateLines
+						update.lines = lines
+					if updateRanges
+						update.ranges = ranges
+					logger.log { project_id, doc_id, oldDoc: doc, update: update }, "updating doc lines and ranges"
+					
+					modified = true
+					rev += 1 # rev will be incremented in mongo by MongoManager.upsertIntoDocCollection
+					MongoManager.upsertIntoDocCollection project_id, doc_id, update, cb
+				else
+					logger.log { project_id, doc_id, }, "doc lines have not changed - not updating"
+					cb()
+			
+			updateVersionIfNeeded = (cb) ->
+				if updateVersion
+					logger.log { project_id, doc_id, oldVersion: doc?.version, newVersion: version }, "updating doc version"
+					modified = true
+					MongoManager.setDocVersion doc_id, version, cb
+				else
+					logger.log { project_id, doc_id, version }, "doc version has not changed - not updating"
+					cb()
+			
+			updateLinesAndRangesIfNeeded (error) ->
+				return callback(error) if error?
+				updateVersionIfNeeded (error) ->
+					return callback(error) if error?
+					callback null, modified, rev
 
 	deleteDoc: (project_id, doc_id, callback = (error) ->) ->
 		DocManager.getDoc project_id, doc_id, { version: false }, (error, doc) ->
 			return callback(error) if error?
 			return callback new Errors.NotFoundError("No such project/doc to delete: #{project_id}/#{doc_id}") if !doc?
-			MongoManager.upsertIntoDocCollection project_id, doc_id, doc.lines, (error) ->
-				return callback(error) if error?
-				MongoManager.markDocAsDeleted doc_id, (error) ->
-					return callback(error) if error?
-					callback()
+			MongoManager.markDocAsDeleted project_id, doc_id, callback
+

+ 1 - 1
services/docstore/app/coffee/HealthChecker.coffee

@@ -19,7 +19,7 @@ module.exports =
 		jobs = [
 			(cb)->
 				opts = getOpts()
-				opts.json = {lines: lines}
+				opts.json = {lines: lines, version: 42}
 				request.post(opts, cb)
 			(cb)->
 				opts = getOpts()

+ 31 - 19
services/docstore/app/coffee/HttpController.coffee

@@ -10,7 +10,7 @@ module.exports = HttpController =
 		doc_id     = req.params.doc_id
 		include_deleted = req.query?.include_deleted == "true"
 		logger.log project_id: project_id, doc_id: doc_id, "getting doc"
-		DocManager.getDoc project_id, doc_id, {version: true}, (error, doc) ->
+		DocManager.getDoc project_id, doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true}, (error, doc) ->
 			return next(error) if error?
 			logger.log doc: doc, "got doc"
 			if !doc?
@@ -24,7 +24,7 @@ module.exports = HttpController =
 		project_id = req.params.project_id
 		doc_id     = req.params.doc_id
 		logger.log project_id: project_id, doc_id: doc_id, "getting raw doc"
-		DocManager.getDoc project_id, doc_id, {version: false}, (error, doc) ->
+		DocManager.getDoc project_id, doc_id, {lines: true}, (error, doc) ->
 			return next(error) if error?
 			if !doc?
 				res.send 404
@@ -35,29 +35,36 @@ module.exports = HttpController =
 	getAllDocs: (req, res, next = (error) ->) ->
 		project_id = req.params.project_id
 		logger.log project_id: project_id, "getting all docs"
-		DocManager.getAllNonDeletedDocs project_id, (error, docs = []) ->
+		DocManager.getAllNonDeletedDocs project_id, {lines: true, rev: true}, (error, docs = []) ->
 			return next(error) if error?
-			docViews = []
-			for doc in docs
-				if doc? # There can end up being null docs for some reason :( (probably a race condition)
-					docViews.push HttpController._buildDocView(doc)
-				else
-					logger.error err: new Error("null doc"), project_id: project_id, "encountered null doc"
-			res.json docViews
+			res.json HttpController._buildDocsArrayView(project_id, docs)
+	
+	getAllRanges: (req, res, next = (error) ->) ->
+		project_id = req.params.project_id
+		logger.log {project_id}, "getting all ranges"
+		DocManager.getAllNonDeletedDocs project_id, {ranges: true}, (error, docs = []) ->
+			return next(error) if error?
+			res.json HttpController._buildDocsArrayView(project_id, docs)
 
 	updateDoc: (req, res, next = (error) ->) ->
 		project_id = req.params.project_id
 		doc_id     = req.params.doc_id
 		lines      = req.body?.lines
 		version    = req.body?.version
+		ranges     = req.body?.ranges
 
 		if !lines? or lines not instanceof Array
 			logger.error project_id: project_id, doc_id: doc_id, "no doc lines provided"
 			res.send 400 # Bad Request
 			return
+		
+		if !version? or typeof version is not "number"
+			logger.error project_id: project_id, doc_id: doc_id, "no doc version provided"
+			res.send 400 # Bad Request
+			return
 
 		logger.log project_id: project_id, doc_id: doc_id, "got http request to update doc"
-		DocManager.updateDoc project_id, doc_id, lines, version, (error, modified, rev) ->
+		DocManager.updateDoc project_id, doc_id, lines, version, ranges, (error, modified, rev) ->
 			return next(error) if error?
 			res.json {
 				modified: modified
@@ -73,18 +80,23 @@ module.exports = HttpController =
 			res.send 204
 
 	_buildDocView: (doc) -> 
-		doc_view = {
-			_id:     doc._id?.toString()
-			lines:   doc.lines
-			rev:     doc.rev
-			deleted: !!doc.deleted
-		}
-		if doc.version?
-			doc_view.version = doc.version
+		doc_view = { _id: doc._id?.toString() }
+		for attribute in ["lines", "rev", "version", "ranges", "deleted"]
+			if doc[attribute]?
+				doc_view[attribute] = doc[attribute]
 		return doc_view
 
 	_buildRawDocView: (doc)->
 		return (doc?.lines or []).join("\n")
+	
+	_buildDocsArrayView: (project_id, docs) ->
+		docViews = []
+		for doc in docs
+			if doc? # There can end up being null docs for some reason :( (probably a race condition)
+				docViews.push HttpController._buildDocView(doc)
+			else
+				logger.error err: new Error("null doc"), project_id: project_id, "encountered null doc"
+		return docViews
 
 	archiveAllDocs: (req, res, next = (error) ->) ->
 		project_id = req.params.project_id

+ 17 - 18
services/docstore/app/coffee/MongoManager.coffee

@@ -2,15 +2,15 @@
 
 module.exports = MongoManager =
 
-	findDoc: (project_id, doc_id, callback = (error, doc) ->) ->
-		db.docs.find {_id: ObjectId(doc_id.toString()), project_id: ObjectId(project_id.toString())}, {}, (error, docs = []) ->
+	findDoc: (project_id, doc_id, filter, callback = (error, doc) ->) ->
+		db.docs.find {_id: ObjectId(doc_id.toString()), project_id: ObjectId(project_id.toString())}, filter, (error, docs = []) ->
 			callback error, docs[0]
 
-	getProjectsDocs: (project_id, options = {include_deleted: true}, callback)->
+	getProjectsDocs: (project_id, options = {include_deleted: true}, filter, callback)->
 		query = {project_id: ObjectId(project_id.toString())}
 		if !options.include_deleted
 			query.deleted = { $ne: true }
-		db.docs.find query, {}, callback
+		db.docs.find query, filter, callback
 
 	getArchivedProjectDocs: (project_id, callback)->
 		query =
@@ -18,24 +18,23 @@ module.exports = MongoManager =
 			inS3: true
 		db.docs.find query, {}, callback
 
-	upsertIntoDocCollection: (project_id, doc_id, lines, callback)->
+	upsertIntoDocCollection: (project_id, doc_id, updates, callback)->
 		update =
-			$set:{}
-			$inc:{}
-			$unset:{}
-		update.$set["lines"] = lines
+			$set: updates
+			$inc:
+				rev: 1
+			$unset:
+				inS3: true
 		update.$set["project_id"] = ObjectId(project_id)
-		update.$inc["rev"] = 1 #on new docs being created this will set the rev to 1
-		update.$unset["inS3"] = true
 		db.docs.update _id: ObjectId(doc_id), update, {upsert: true}, callback
 
-
-	markDocAsDeleted: (doc_id, callback)->
-		update =
-			$set: {}
-		update.$set["deleted"] = true
-		db.docs.update _id: ObjectId(doc_id), update, (err)->
-			callback(err)
+	markDocAsDeleted: (project_id, doc_id, callback)->
+		db.docs.update {
+			_id: ObjectId(doc_id),
+			project_id: ObjectId(project_id) 
+		}, {
+			$set: { deleted: true }
+		}, callback
 
 	markDocAsArchived: (doc_id, rev, callback)->
 		update =

+ 37 - 0
services/docstore/app/coffee/RangeManager.coffee

@@ -0,0 +1,37 @@
+_ = require "underscore"
+{ObjectId} = require("./mongojs")
+
+module.exports = RangeManager =
+	shouldUpdateRanges: (doc_ranges, incoming_ranges) ->
+		# TODO: If we have no incoming_ranges, just ignore for now while
+		# we're rolling this out, but eventually this should be a mandatory
+		# field and this will become an error condition
+		return false if !incoming_ranges?
+
+		# If the ranges are empty, we don't store them in the DB, so set
+		# doc_ranges to an empty object as default, since this is was the 
+		# incoming_ranges will be for an empty range set.
+		if !doc_ranges?
+			doc_ranges = {}
+
+		return not _.isEqual(doc_ranges, incoming_ranges)
+	
+	jsonRangesToMongo: (ranges) ->
+		return null if !ranges?
+		for change in ranges.changes or []
+			change.id = @_safeObjectId(change.id)
+			if change.metadata?.ts?
+				change.metadata.ts = new Date(change.metadata.ts)
+			if change.metadata?.user_id?
+				change.metadata.user_id = @_safeObjectId(change.metadata.user_id)
+		for comment in ranges.comments or []
+			comment.id = @_safeObjectId(comment.id)
+			if comment.op?.t?
+				comment.op.t = @_safeObjectId(comment.op.t)
+		return ranges
+	
+	_safeObjectId: (data) ->
+		try
+			return ObjectId(data)
+		catch
+			return data

La diferencia del archivo ha sido suprimido porque es demasiado grande
+ 1 - 1
services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee


+ 2 - 1
services/docstore/test/acceptance/coffee/DeletingDocsTests.coffee

@@ -11,7 +11,8 @@ describe "Deleting a doc", ->
 		@doc_id = ObjectId()
 		@lines = ["original", "lines"]
 		@version = 42
-		DocstoreClient.createDoc @project_id, @doc_id, @lines, (error) =>
+		@ranges = []
+		DocstoreClient.createDoc @project_id, @doc_id, @lines, @version, @ranges, (error) =>
 			throw error if error?
 			done()
 

+ 17 - 7
services/docstore/test/acceptance/coffee/GettingAllDocsTests.coffee

@@ -12,34 +12,36 @@ describe "Getting all docs", ->
 		@docs = [{
 			_id: ObjectId()
 			lines: ["one", "two", "three"]
+			ranges: {"mock": "one"}
 			rev: 2
 		}, {
 			_id: ObjectId()
 			lines: ["aaa", "bbb", "ccc"]
+			ranges: {"mock": "two"}
 			rev: 4
 		}, {
 			_id: ObjectId()
 			lines: ["111", "222", "333"]
+			ranges: {"mock": "three"}
 			rev: 6
 		}]
 		@deleted_doc = {
 			_id: ObjectId()
 			lines: ["deleted"]
+			ranges: {"mock": "four"}
 			rev: 8
 		}
+		version = 42
 		jobs = for doc in @docs
 			do (doc) =>
 				(callback) => 
-					DocstoreClient.createDoc @project_id, doc._id, doc.lines, (err)=>
-						doc.lines[0] = doc.lines[0]+" added"
-						DocstoreClient.updateDoc @project_id, doc._id, doc.lines, null, callback
+					DocstoreClient.createDoc @project_id, doc._id, doc.lines, version, doc.ranges, callback
 		jobs.push (cb) =>
-			DocstoreClient.createDoc @project_id, @deleted_doc._id, @deleted_doc.lines, (err)=>
-				DocstoreClient.updateDoc @project_id, @deleted_doc._id, @deleted_doc.lines, null, (err) =>
-					DocstoreClient.deleteDoc @project_id, @deleted_doc._id, done
+			DocstoreClient.createDoc @project_id, @deleted_doc._id, @deleted_doc.lines, version, @deleted_doc.ranges, (err)=>
+					DocstoreClient.deleteDoc @project_id, @deleted_doc._id, cb
 		async.series jobs, done 
 
-	it "should return all the (non-deleted) docs", (done) ->
+	it "getAllDocs should return all the (non-deleted) docs", (done) ->
 		DocstoreClient.getAllDocs @project_id, (error, res, docs) =>
 			throw error if error?
 			docs.length.should.equal @docs.length
@@ -47,4 +49,12 @@ describe "Getting all docs", ->
 				doc.lines.should.deep.equal @docs[i].lines
 			done()
 
+	it "getAllRanges should return all the (non-deleted) doc ranges", (done) ->
+		DocstoreClient.getAllRanges @project_id, (error, res, docs) =>
+			throw error if error?
+			docs.length.should.equal @docs.length
+			for doc, i in docs
+				doc.ranges.should.deep.equal @docs[i].ranges
+			done()
+
 

+ 18 - 2
services/docstore/test/acceptance/coffee/GettingDocsTests.coffee

@@ -10,7 +10,17 @@ describe "Getting a doc", ->
 		@project_id = ObjectId()
 		@doc_id = ObjectId()
 		@lines = ["original", "lines"]
-		DocstoreClient.createDoc @project_id, @doc_id, @lines, (error) =>
+		@version = 42
+		@ranges = {
+			changes: [{
+				id: ObjectId().toString()
+				op: { i: "foo", p: 3 }
+				meta:
+					user_id: ObjectId().toString()
+					ts: new Date().toString()
+			}]
+		}
+		DocstoreClient.createDoc @project_id, @doc_id, @lines, @version, @ranges, (error) =>
 			throw error if error?
 			done()
 
@@ -18,6 +28,8 @@ describe "Getting a doc", ->
 		it "should get the doc lines and version", (done) ->
 			DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) =>
 				doc.lines.should.deep.equal @lines
+				doc.version.should.equal @version
+				doc.ranges.should.deep.equal @ranges
 				done()
 
 	describe "when the doc does not exist", ->
@@ -30,11 +42,15 @@ describe "Getting a doc", ->
 	describe "when the doc is a deleted doc", ->
 		beforeEach (done) ->
 			@deleted_doc_id = ObjectId()
-			DocstoreClient.createDeletedDoc @project_id, @deleted_doc_id, @lines, done
+			DocstoreClient.createDoc @project_id, @deleted_doc_id, @lines, @version, @ranges, (error) =>
+				throw error if error?
+				DocstoreClient.deleteDoc @project_id, @deleted_doc_id, done
 
 		it "should return the doc", (done) ->
 			DocstoreClient.getDoc @project_id, @deleted_doc_id, {include_deleted:true},(error, res, doc) =>
 				doc.lines.should.deep.equal @lines
+				doc.version.should.equal @version
+				doc.ranges.should.deep.equal @ranges
 				doc.deleted.should.equal true
 				done()
 

+ 83 - 31
services/docstore/test/acceptance/coffee/UpdatingDocsTests.coffee

@@ -11,16 +11,47 @@ describe "Applying updates to a doc", ->
 		@doc_id = ObjectId()
 		@originalLines = ["original", "lines"]
 		@newLines = ["new", "lines"]
+		@originalRanges = {
+			changes: [{
+				id: ObjectId().toString()
+				op: { i: "foo", p: 3 }
+				meta:
+					user_id: ObjectId().toString()
+					ts: new Date().toString()
+			}]
+		}
+		@newRanges = {
+			changes: [{
+				id: ObjectId().toString()
+				op: { i: "bar", p: 6 }
+				meta:
+					user_id: ObjectId().toString()
+					ts: new Date().toString()
+			}]
+		}
 		@version = 42
-		DocstoreClient.createDoc @project_id, @doc_id, @originalLines, (error) =>
+		DocstoreClient.createDoc @project_id, @doc_id, @originalLines, @version, @originalRanges, (error) =>
 			throw error if error?
-			DocstoreClient.setDocVersion @doc_id, @version, (error) =>
-				throw error if error?
+			done()
+
+	describe "when nothing has been updated", ->
+		beforeEach (done) ->
+			DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version, @originalRanges, (error, res, @body) =>
 				done()
 
-	describe "when the content has changed", ->
+		it "should return modified = false", ->
+			@body.modified.should.equal false
+
+		it "should not update the doc in the API", (done) ->
+			DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) =>
+				doc.lines.should.deep.equal @originalLines
+				doc.version.should.equal @version
+				doc.ranges.should.deep.equal @originalRanges
+				done()
+
+	describe "when the lines have changed", ->
 		beforeEach (done) ->
-			DocstoreClient.updateDoc @project_id, @doc_id, @newLines, null, (error, res, @body) =>
+			DocstoreClient.updateDoc @project_id, @doc_id, @newLines, @version, @originalRanges, (error, res, @body) =>
 				done()
 
 		it "should return modified = true", ->
@@ -29,29 +60,53 @@ describe "Applying updates to a doc", ->
 		it "should return the rev", ->
 			@body.rev.should.equal 2
 
-		it "should update the doc in the API but not change the version", (done) ->
+		it "should update the doc in the API", (done) ->
 			DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) =>
 				doc.lines.should.deep.equal @newLines
 				doc.version.should.equal @version
+				doc.ranges.should.deep.equal @originalRanges
 				done()
 
-	describe "when the content has not been updated", ->
+	describe "when the version has changed", ->
 		beforeEach (done) ->
-			DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, null, (error, res, @body) =>
+			DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version + 1, @originalRanges, (error, res, @body) =>
 				done()
 
-		it "should return modified = false", ->
-			@body.modified.should.equal false
+		it "should return modified = true", ->
+			@body.modified.should.equal true
 
-		it "should not update the doc in the API", (done) ->
+		it "should return the rev", ->
+			@body.rev.should.equal 1
+
+		it "should update the doc in the API", (done) ->
+			DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) =>
+				doc.lines.should.deep.equal @originalLines
+				doc.version.should.equal @version + 1
+				doc.ranges.should.deep.equal @originalRanges
+				done()
+
+	describe "when the ranges have changed", ->
+		beforeEach (done) ->
+			DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version, @newRanges, (error, res, @body) =>
+				done()
+
+		it "should return modified = true", ->
+			@body.modified.should.equal true
+
+		it "should return the rev", ->
+			@body.rev.should.equal 2
+
+		it "should update the doc in the API", (done) ->
 			DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) =>
 				doc.lines.should.deep.equal @originalLines
+				doc.version.should.equal @version
+				doc.ranges.should.deep.equal @newRanges
 				done()
 
 	describe "when the doc does not exist", ->
 		beforeEach (done) ->
 			@missing_doc_id = ObjectId()
-			DocstoreClient.updateDoc @project_id, @missing_doc_id, @originalLines, null, (error, @res, @body) =>
+			DocstoreClient.updateDoc @project_id, @missing_doc_id, @originalLines, 0, @originalRanges, (error, @res, @body) =>
 				done()
 
 		it "should create the doc", ->
@@ -61,12 +116,13 @@ describe "Applying updates to a doc", ->
 			DocstoreClient.getDoc @project_id, @missing_doc_id, {}, (error, res, doc) =>
 				doc.lines.should.deep.equal @originalLines
 				doc.version.should.equal 0
+				doc.ranges.should.deep.equal @originalRanges
 				done()
 
 	describe "when malformed doc lines are provided", ->
 		describe "when the lines are not an array", ->
 			beforeEach (done) ->
-				DocstoreClient.updateDoc @project_id, @doc_id, { foo: "bar" }, null, (error, @res, @body) =>
+				DocstoreClient.updateDoc @project_id, @doc_id, { foo: "bar" }, @version, @originalRanges, (error, @res, @body) =>
 					done()
 
 			it "should return 400", ->
@@ -79,7 +135,7 @@ describe "Applying updates to a doc", ->
 
 		describe "when the lines are not present", ->
 			beforeEach (done) ->
-				DocstoreClient.updateDoc @project_id, @doc_id, null, null, (error, @res, @body) =>
+				DocstoreClient.updateDoc @project_id, @doc_id, null, @version, @originalRanges, (error, @res, @body) =>
 					done()
 
 			it "should return 400", ->
@@ -89,36 +145,32 @@ describe "Applying updates to a doc", ->
 				DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) =>
 					doc.lines.should.deep.equal @originalLines
 					done()
-
-	describe "when the content is large", ->
+	
+	describe "when no version is provided", ->
 		beforeEach (done) ->
-			line = new Array(1025).join("x") # 1kb
-			@largeLines = Array.apply(null, Array(1024)).map(() -> line) # 1mb
-			DocstoreClient.updateDoc @project_id, @doc_id, @largeLines, null, (error, res, @body) =>
+			DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, null, @originalRanges, (error, @res, @body) =>
 				done()
 
-		it "should return modified = true", ->
-			@body.modified.should.equal true
+		it "should return 400", ->
+			@res.statusCode.should.equal 400
 
-		it "should update the doc in the API", (done) ->
+		it "should not update the doc in the API", (done) ->
 			DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) =>
-				doc.lines.should.deep.equal @largeLines
+				doc.lines.should.deep.equal @originalLines
+				doc.version.should.equal @version
 				done()
 
-	describe "when the version has changed", ->
+	describe "when the content is large", ->
 		beforeEach (done) ->
-			DocstoreClient.updateDoc @project_id, @doc_id, @originalLines, @version + 1, (error, res, @body) =>
+			line = new Array(1025).join("x") # 1kb
+			@largeLines = Array.apply(null, Array(1024)).map(() -> line) # 1mb
+			DocstoreClient.updateDoc @project_id, @doc_id, @largeLines, @version, @originalRanges, (error, res, @body) =>
 				done()
 
 		it "should return modified = true", ->
 			@body.modified.should.equal true
 
-		it "should return the rev", ->
-			@body.rev.should.equal 2
-
 		it "should update the doc in the API", (done) ->
 			DocstoreClient.getDoc @project_id, @doc_id, {}, (error, res, doc) =>
-				doc.lines.should.deep.equal @originalLines
-				doc.version.should.equal @version + 1
+				doc.lines.should.deep.equal @largeLines
 				done()
-

+ 10 - 14
services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee

@@ -5,19 +5,8 @@ DocArchiveManager = require("../../../../app/js/DocArchiveManager.js")
 
 module.exports = DocstoreClient =
 
-	createDoc: (project_id, doc_id, lines, callback = (error) ->) ->
-		db.docs.save({_id: doc_id, project_id:project_id, lines: lines, rev:1}, callback)
-	
-	setDocVersion: (doc_id, version, callback = (error) ->) ->
-		db.docOps.save({doc_id: doc_id, version: version}, callback)
-
-	createDeletedDoc: (project_id, doc_id, lines, callback = (error) ->) ->
-		db.docs.insert {
-			_id: doc_id
-			project_id: project_id
-			lines: lines
-			deleted: true
-		}, callback
+	createDoc: (project_id, doc_id, lines, version, ranges, callback = (error) ->) ->
+		DocstoreClient.updateDoc project_id, doc_id, lines, version, ranges, callback
 
 	getDoc: (project_id, doc_id, qs, callback = (error, res, body) ->) ->
 		request.get {
@@ -32,12 +21,19 @@ module.exports = DocstoreClient =
 			json: true
 		}, callback
 
-	updateDoc: (project_id, doc_id, lines, version, callback = (error, res, body) ->) ->
+	getAllRanges: (project_id, callback = (error, res, body) ->) ->
+		request.get {
+			url: "http://localhost:#{settings.internal.docstore.port}/project/#{project_id}/ranges"
+			json: true
+		}, callback
+
+	updateDoc: (project_id, doc_id, lines, version, ranges, callback = (error, res, body) ->) ->
 		request.post {
 			url: "http://localhost:#{settings.internal.docstore.port}/project/#{project_id}/doc/#{doc_id}"
 			json:
 				lines: lines
 				version: version
+				ranges: ranges
 		}, callback
 
 	deleteDoc: (project_id, doc_id, callback = (error, res, body) ->) ->

+ 6 - 6
services/docstore/test/unit/coffee/DocArchiveManager.coffee

@@ -64,8 +64,8 @@ describe "DocArchiveManager", ->
 		@MongoManager =
 			markDocAsArchived: sinon.stub().callsArgWith(2, null)
 			upsertIntoDocCollection: sinon.stub().callsArgWith(3, null)
-			getProjectsDocs: sinon.stub().callsArgWith(2, null, @mongoDocs)
-			getArchivedProjectDocs: sinon.stub().callsArgWith(1, null, @mongoDocs)
+			getProjectsDocs: sinon.stub().callsArgWith(3, null, @mongoDocs)
+			getArchivedProjectDocs: sinon.stub().callsArgWith(2, null, @mongoDocs)
 
 		@requires = 
 			"settings-sharelatex": @settings
@@ -127,7 +127,7 @@ describe "DocArchiveManager", ->
 	describe "archiveAllDocs", ->
 
 		it "should archive all project docs which are not in s3", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, @mongoDocs)
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, @mongoDocs)
 			@DocArchiveManager.archiveDoc = sinon.stub().callsArgWith(2, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
@@ -142,14 +142,14 @@ describe "DocArchiveManager", ->
 				done()
 
 		it "should return error if have no docs", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, null)
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
 				should.exist err
 				done()
 
 		it "should return the error", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, @error, null)
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, @error, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
 				err.should.equal @error
@@ -163,7 +163,7 @@ describe "DocArchiveManager", ->
 				while --numberOfDocs != 0
 					@mongoDocs.push({inS3:true, _id: ObjectId()})
 
-				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, @mongoDocs)
+				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, @mongoDocs)
 				@DocArchiveManager.archiveDoc = sinon.stub().callsArgWith(2, null)	
 
 			it "should not throw and error", (done)->

+ 85 - 92
services/docstore/test/unit/coffee/DocManagerTests.coffee

@@ -12,6 +12,10 @@ describe "DocManager", ->
 		@DocManager = SandboxedModule.require modulePath, requires:
 			"./MongoManager": @MongoManager = {}
 			"./DocArchiveManager": @DocArchiveManager = {}
+			"./RangeManager": @RangeManager = {
+				jsonRangesToMongo: (r) -> r
+				shouldUpdateRanges: sinon.stub().returns false
+			}
 			"logger-sharelatex": @logger = 
 				log: sinon.stub()
 				warn:->
@@ -88,7 +92,7 @@ describe "DocManager", ->
 
 		describe "when the doc does not exist in the docs collection", ->
 			beforeEach -> 
-				@MongoManager.findDoc = sinon.stub().callsArgWith(2, null, null)
+				@MongoManager.findDoc = sinon.stub().yields(null, null)
 				@DocManager.getDoc @project_id, @doc_id, {version: true}, @callback
 
 			it "should return a NotFoundError", ->
@@ -100,13 +104,14 @@ describe "DocManager", ->
 		describe "when the project exists", ->
 			beforeEach -> 
 				@docs = [{ _id: @doc_id, project_id: @project_id, lines: ["mock-lines"] }]
-				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, @docs)
+				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, @docs)
 				@DocArchiveManager.unArchiveAllDocs = sinon.stub().callsArgWith(1, null, @docs)
-				@DocManager.getAllNonDeletedDocs @project_id, @callback
+				@filter = { lines: true }
+				@DocManager.getAllNonDeletedDocs @project_id, @filter, @callback
 
 			it "should get the project from the database", ->
 				@MongoManager.getProjectsDocs
-					.calledWith(@project_id, {include_deleted: false})
+					.calledWith(@project_id, {include_deleted: false}, @filter)
 					.should.equal true
 
 			it "should return the docs", ->
@@ -114,13 +119,13 @@ describe "DocManager", ->
 
 		describe "when there are no docs for the project", ->
 			beforeEach -> 
-				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, null)
-				@DocArchiveManager.unArchiveAllDocs = sinon.stub().callsArgWith(1, null, null)
-				@DocManager.getAllNonDeletedDocs @project_id, @callback
+				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(3, null, null)
+				@DocArchiveManager.unArchiveAllDocs = sinon.stub().callsArgWith(1, null)
+				@DocManager.getAllNonDeletedDocs @project_id, @filter, @callback
 
 			it "should return a NotFoundError", ->
 				@callback
-					.calledWith(new Errors.NotFoundError("No such docs for project #{@project_id}"))
+					.calledWith(new Errors.NotFoundError("No docs for project #{@project_id}"))
 					.should.equal true
 
 	describe "deleteDoc", ->
@@ -129,8 +134,7 @@ describe "DocManager", ->
 				@lines = ["mock", "doc", "lines"]
 				@rev = 77
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, {lines: @lines, rev:@rev})
-				@MongoManager.upsertIntoDocCollection = sinon.stub().callsArg(3)
-				@MongoManager.markDocAsDeleted = sinon.stub().callsArg(1)
+				@MongoManager.markDocAsDeleted = sinon.stub().callsArg(2)
 				@DocManager.deleteDoc @project_id, @doc_id, @callback
 
 			it "should get the doc", ->
@@ -138,14 +142,9 @@ describe "DocManager", ->
 					.calledWith(@project_id, @doc_id)
 					.should.equal true
 
-			it "should update the doc lines", ->
-				@MongoManager.upsertIntoDocCollection
-					.calledWith(@project_id, @doc_id, @lines)
-					.should.equal true
-
 			it "should mark doc as deleted", ->
 				@MongoManager.markDocAsDeleted
-					.calledWith(@doc_id)
+					.calledWith(@project_id, @doc_id)
 					.should.equal true
 
 			it "should return the callback", ->
@@ -154,11 +153,8 @@ describe "DocManager", ->
 		describe "when the doc does not exist", ->
 			beforeEach -> 
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, null)
-				@MongoManager.upsertIntoDocCollection = sinon.stub()
 				@DocManager.deleteDoc @project_id, @doc_id, @callback
 
-			it "should not try to insert a deleted doc", ->
-				@MongoManager.upsertIntoDocCollection.called.should.equal false
 
 			it "should return a NotFoundError", ->
 				@callback
@@ -169,115 +165,118 @@ describe "DocManager", ->
 		beforeEach ->
 			@oldDocLines = ["old", "doc", "lines"]
 			@newDocLines = ["new", "doc", "lines"]
+			@originalRanges = {
+				changes: [{
+					id: ObjectId().toString()
+					op: { i: "foo", p: 3 }
+					meta:
+						user_id: ObjectId().toString()
+						ts: new Date().toString()
+				}]
+			}
+			@newRanges = {
+				changes: [{
+					id: ObjectId().toString()
+					op: { i: "bar", p: 6 }
+					meta:
+						user_id: ObjectId().toString()
+						ts: new Date().toString()
+				}]
+			}
 			@version = 42
-			@doc = { _id: @doc_id, project_id: @project_id, lines: @oldDocLines, rev: @rev = 5, version: @version }
+			@doc = { _id: @doc_id, project_id: @project_id, lines: @oldDocLines, rev: @rev = 5, version: @version, ranges: @originalRanges }
 
 			@MongoManager.upsertIntoDocCollection = sinon.stub().callsArg(3)
 			@MongoManager.setDocVersion = sinon.stub().yields()
 			@DocManager.getDoc = sinon.stub()
 
-		describe "when the doc lines have changed", ->
+		describe "when only the doc lines have changed", ->
 			beforeEach ->
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc)
-				@DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback
+				@DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @originalRanges, @callback
 
 			it "should get the existing doc", ->
 				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id)
+					.calledWith(@project_id, @doc_id, {version: true, rev: true, lines: true, version: true, ranges: true})
 					.should.equal true
 
 			it "should upsert the document to the doc collection", ->
 				@MongoManager.upsertIntoDocCollection
-					.calledWith(@project_id, @doc_id, @newDocLines)
+					.calledWith(@project_id, @doc_id, {lines: @newDocLines})
 					.should.equal true
 			
-			it "should update the version", ->
-				@MongoManager.setDocVersion
-					.calledWith(@doc_id, @version)
-					.should.equal true
-
-			it "should log out the old and new doc lines", ->
-				@logger.log
-					.calledWith(
-						project_id: @project_id
-						doc_id: @doc_id
-						oldDocLines: @oldDocLines
-						newDocLines: @newDocLines
-						rev: @doc.rev
-						oldVersion: @version
-						newVersion: @version
-						"updating doc lines"
-					)
-					.should.equal true
+			it "should not update the version", ->
+				@MongoManager.setDocVersion.called.should.equal false
 
 			it "should return the callback with the new rev", ->
 				@callback.calledWith(null, true, @rev + 1).should.equal true
 
-		describe "when the version has changed", ->
+		describe "when the doc ranges have changed", ->
 			beforeEach ->
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc)
-				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version + 1, @callback
+				@RangeManager.shouldUpdateRanges.returns true
+				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version, @newRanges, @callback
 
-			it "should get the existing doc with the version", ->
-				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id, {version: true})
-					.should.equal true
-
-			it "should upsert the document to the doc collection", ->
+			it "should upsert the ranges", ->
 				@MongoManager.upsertIntoDocCollection
-					.calledWith(@project_id, @doc_id, @oldDocLines)
+					.calledWith(@project_id, @doc_id, {ranges: @newRanges})
 					.should.equal true
 			
-			it "should update the version", ->
-				@MongoManager.setDocVersion
-					.calledWith(@doc_id, @version + 1)
-					.should.equal true
+			it "should not update the version", ->
+				@MongoManager.setDocVersion.called.should.equal false
 
 			it "should return the callback with the new rev", ->
 				@callback.calledWith(null, true, @rev + 1).should.equal true
 
-		describe "when the version is null and the lines are different", ->
+		describe "when only the version has changed", ->
 			beforeEach ->
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc)
-				@DocManager.updateDoc @project_id, @doc_id, @newDocLines, null, @callback
+				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version + 1, @originalRanges, @callback
 
-			it "should get the existing doc with the version", ->
-				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id, {version: true})
-					.should.equal true
-
-			it "should upsert the document to the doc collection", ->
-				@MongoManager.upsertIntoDocCollection
-					.calledWith(@project_id, @doc_id, @newDocLines)
-					.should.equal true
+			it "should not change the lines or ranges", ->
+				@MongoManager.upsertIntoDocCollection.called.should.equal false
 			
-			it "should not update the version", ->
+			it "should update the version", ->
 				@MongoManager.setDocVersion
-					.called
-					.should.equal false
+					.calledWith(@doc_id, @version + 1)
+					.should.equal true
 
-			it "should return the callback with the new rev", ->
-				@callback.calledWith(null, true, @rev + 1).should.equal true
+			it "should return the callback with the old rev", ->
+				@callback.calledWith(null, true, @rev).should.equal true
 
-		describe "when the version is null and the lines are the same", ->
+		describe "when the doc has not changed at all", ->
 			beforeEach ->
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc)
-				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines, null, @callback
+				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version, @originalRanges, @callback
+
+			it "should not update the ranges or lines", ->
+				@MongoManager.upsertIntoDocCollection.called.should.equal false
 			
 			it "should not update the version", ->
 				@MongoManager.setDocVersion.called.should.equal false
 
-			it "should not update the doc", ->
-				@MongoManager.upsertIntoDocCollection.called.should.equal false
-
-			it "should return the callback with the existing rev", ->
+			it "should return the callback with the old rev and modified == false", ->
 				@callback.calledWith(null, false, @rev).should.equal true
 
+		describe "when the version is null", ->
+			beforeEach ->
+				@DocManager.updateDoc @project_id, @doc_id, @newDocLines, null, @originalRanges, @callback
+
+			it "should return an error", ->
+				@callback.calledWith(new Error("no lines or version provided")).should.equal true
+
+		describe "when the lines are null", ->
+			beforeEach ->
+				@DocManager.updateDoc @project_id, @doc_id, null, @version, @originalRanges, @callback
+
+			it "should return an error", ->
+				@callback.calledWith(new Error("no lines or version provided")).should.equal true
+
 		describe "when there is a generic error getting the doc", ->
 			beforeEach ->
 				@error =  new Error("doc could not be found")
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, @error, null, null)
-				@DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback
+				@DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @originalRanges, @callback
 			
 			it "should not upsert the document to the doc collection", ->
 				@MongoManager.upsertIntoDocCollection.called.should.equal false
@@ -288,7 +287,7 @@ describe "DocManager", ->
 		describe "when the doc lines have not changed", ->
 			beforeEach ->
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc)
-				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @version, @callback
+				@DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @version, @originalRanges, @callback
 
 			it "should not update the doc", ->
 				@MongoManager.upsertIntoDocCollection.called.should.equal false
@@ -296,25 +295,19 @@ describe "DocManager", ->
 			it "should return the callback with the existing rev", ->
 				@callback.calledWith(null, false, @rev).should.equal true
 
-		describe "when the doc lines are an empty array", ->
-			beforeEach ->
-				@doc.lines = []
-				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc)
-				@DocManager.updateDoc @project_id, @doc_id, @doc.lines, @callback
-
-			it "should upsert the document to the doc collection", ->
-				@MongoManager.upsertIntoDocCollection
-					.calledWith(@project_id, @doc_id,  @doc.lines)
-					.should.equal true	
-
 		describe "when the doc does not exist", ->
 			beforeEach ->
 				@DocManager.getDoc = sinon.stub().callsArgWith(3, null, null, null)
-				@DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback
+				@DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @originalRanges, @callback
 			
 			it "should upsert the document to the doc collection", ->
 				@MongoManager.upsertIntoDocCollection
-					.calledWith(@project_id, @doc_id, @newDocLines)
+					.calledWith(@project_id, @doc_id, {lines: @newDocLines, ranges: @originalRanges})
+					.should.equal true
+				
+			it "should set the version", ->
+				@MongoManager.setDocVersion
+					.calledWith(@doc_id, @version)
 					.should.equal true
 
 			it "should return the callback with the new rev", ->

+ 50 - 25
services/docstore/test/unit/coffee/HttpControllerTests.coffee

@@ -45,7 +45,7 @@ describe "HttpController", ->
 
 			it "should get the document with the version (including deleted)", ->
 				@DocManager.getDoc
-					.calledWith(@project_id, @doc_id, {version: true})
+					.calledWith(@project_id, @doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true})
 					.should.equal true
 
 			it "should return the doc as JSON", ->
@@ -54,7 +54,6 @@ describe "HttpController", ->
 						_id: @doc_id
 						lines: @doc.lines
 						rev: @doc.rev
-						deleted: false
 						version: @doc.version
 					})
 					.should.equal true
@@ -68,7 +67,7 @@ describe "HttpController", ->
 
 			it "should get the doc from the doc manager", ->
 				@HttpController.getDoc @req, @res, @next
-				@DocManager.getDoc.calledWith(@project_id, @doc_id, {version: true}).should.equal true
+				@DocManager.getDoc.calledWith(@project_id, @doc_id, {lines: true, rev: true, deleted: true, version: true, ranges: true}).should.equal true
 
 			it "should return 404 if the query string delete is not set ", ->
 				@HttpController.getDoc @req, @res, @next
@@ -97,7 +96,7 @@ describe "HttpController", ->
 
 		it "should get the document without the version", ->
 			@DocManager.getDoc
-				.calledWith(@project_id, @doc_id, {version: false})
+				.calledWith(@project_id, @doc_id, {lines: true})
 				.should.equal true
 
 		it "should set the content type header", ->
@@ -120,12 +119,12 @@ describe "HttpController", ->
 					lines: ["mock", "lines", "two"]
 					rev: 4
 				}]
-				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(1, null, @docs)
+				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(2, null, @docs)
 				@HttpController.getAllDocs @req, @res, @next
 
 			it "should get all the (non-deleted) docs", ->
 				@DocManager.getAllNonDeletedDocs
-					.calledWith(@project_id)
+					.calledWith(@project_id, {lines: true, rev: true})
 					.should.equal true
 
 			it "should return the doc as JSON", ->
@@ -134,12 +133,10 @@ describe "HttpController", ->
 						_id:   @docs[0]._id.toString()
 						lines: @docs[0].lines
 						rev:   @docs[0].rev
-						deleted: false
 					}, {
 						_id:   @docs[1]._id.toString()
 						lines: @docs[1].lines
 						rev:   @docs[1].rev
-						deleted: false
 					}])
 					.should.equal true
 
@@ -158,7 +155,7 @@ describe "HttpController", ->
 					lines: ["mock", "lines", "two"]
 					rev: 4
 				}]
-				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(1, null, @docs)
+				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(2, null, @docs)
 				@HttpController.getAllDocs @req, @res, @next
 
 			it "should return the non null docs as JSON", ->
@@ -167,12 +164,10 @@ describe "HttpController", ->
 						_id:   @docs[0]._id.toString()
 						lines: @docs[0].lines
 						rev:   @docs[0].rev
-						deleted: false
 					}, {
 						_id:   @docs[2]._id.toString()
 						lines: @docs[2].lines
 						rev:   @docs[2].rev
-						deleted: false
 					}])
 					.should.equal true
 
@@ -185,6 +180,37 @@ describe "HttpController", ->
 					)
 					.should.equal true
 
+	describe "getAllRanges", ->
+		describe "normally", ->
+			beforeEach ->
+				@req.params =
+					project_id: @project_id
+				@docs = [{
+					_id: ObjectId()
+					ranges: {"mock_ranges": "one"}
+				}, {
+					_id: ObjectId()
+					ranges: {"mock_ranges": "two"}
+				}]
+				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(2, null, @docs)
+				@HttpController.getAllRanges @req, @res, @next
+
+			it "should get all the (non-deleted) doc ranges", ->
+				@DocManager.getAllNonDeletedDocs
+					.calledWith(@project_id, {ranges: true})
+					.should.equal true
+
+			it "should return the doc as JSON", ->
+				@res.json
+					.calledWith([{
+						_id:    @docs[0]._id.toString()
+						ranges: @docs[0].ranges
+					}, {
+						_id:    @docs[1]._id.toString()
+						ranges: @docs[1].ranges
+					}])
+					.should.equal true
+
 	describe "updateDoc", ->
 		beforeEach ->
 			@req.params =
@@ -195,12 +221,14 @@ describe "HttpController", ->
 			beforeEach ->
 				@req.body =
 					lines: @lines = ["hello", "world"]
+					version: @version = 42
+					ranges: @ranges = { changes: "mock" }
 				@DocManager.updateDoc = sinon.stub().yields(null, true, @rev = 5)
 				@HttpController.updateDoc @req, @res, @next
 
 			it "should update the document", ->
 				@DocManager.updateDoc
-					.calledWith(@project_id, @doc_id, @lines, undefined)
+					.calledWith(@project_id, @doc_id, @lines, @version, @ranges)
 					.should.equal true
 
 			it "should return a modified status", ->
@@ -212,6 +240,7 @@ describe "HttpController", ->
 			beforeEach ->
 				@req.body =
 					lines: @lines = ["hello", "world"]
+					version: @version = 42
 				@DocManager.updateDoc = sinon.stub().yields(null, false, @rev = 5)
 				@HttpController.updateDoc @req, @res, @next
 
@@ -222,7 +251,7 @@ describe "HttpController", ->
 
 		describe "when the doc lines are not provided", ->
 			beforeEach ->
-				@req.body = {}
+				@req.body = { version: 42 }
 				@DocManager.updateDoc = sinon.stub().yields(null, false)
 				@HttpController.updateDoc @req, @res, @next
 
@@ -234,22 +263,18 @@ describe "HttpController", ->
 					.calledWith(400)
 					.should.equal true
 
-		describe "when the doc version is provided", ->
+		describe "when the doc version is not provided", ->
 			beforeEach ->
-				@req.body =
-					lines: @lines = ["hello", "world"]
-					version: @version = 42
-				@DocManager.updateDoc = sinon.stub().yields(null, true, @rev = 5)
+				@req.body = { lines : [ "foo" ]}
+				@DocManager.updateDoc = sinon.stub().yields(null, false)
 				@HttpController.updateDoc @req, @res, @next
 
-			it "should update the document with the lines and version", ->
-				@DocManager.updateDoc
-					.calledWith(@project_id, @doc_id, @lines, @version)
-					.should.equal true
+			it "should not update the document", ->
+				@DocManager.updateDoc.called.should.equal false
 
-			it "should return a modified status", ->
-				@res.json
-					.calledWith(modified: true, rev: @rev)
+			it "should return a 400 (bad request) response", ->
+				@res.send
+					.calledWith(400)
 					.should.equal true
 
 	describe "deleteDoc", ->

+ 15 - 13
services/docstore/test/unit/coffee/MongoManagerTests.coffee

@@ -20,14 +20,15 @@ describe "MongoManager", ->
 		beforeEach ->
 			@doc = { name: "mock-doc"}
 			@db.docs.find = sinon.stub().callsArgWith(2, null, [@doc])
-			@MongoManager.findDoc @project_id, @doc_id, @callback
+			@filter = { lines: true }
+			@MongoManager.findDoc @project_id, @doc_id, @filter, @callback
 
 		it "should find the doc", ->
 			@db.docs.find
 				.calledWith({
 					_id: ObjectId(@doc_id)
 					project_id: ObjectId(@project_id)
-				}, {})
+				}, @filter)
 				.should.equal true
 
 		it "should call the callback with the doc", ->
@@ -35,6 +36,7 @@ describe "MongoManager", ->
 
 	describe "getProjectsDocs", ->
 		beforeEach ->
+			@filter = {lines: true}
 			@doc1 = { name: "mock-doc1" }
 			@doc2 = { name: "mock-doc2" }
 			@doc3 = { name: "mock-doc3" }
@@ -43,28 +45,28 @@ describe "MongoManager", ->
 		
 		describe "with included_deleted = false", ->
 			beforeEach -> 
-				@MongoManager.getProjectsDocs @project_id, include_deleted: false, @callback
+				@MongoManager.getProjectsDocs @project_id, include_deleted: false, @filter, @callback
 
 			it "should find the non-deleted docs via the project_id", ->
 				@db.docs.find
 					.calledWith({
 						project_id: ObjectId(@project_id)
 						deleted: { $ne: true }
-					}, {})
+					}, @filter)
 					.should.equal true
 
 			it "should call the callback with the docs", ->
 				@callback.calledWith(null, [@doc, @doc3, @doc4]).should.equal true
 				
 		describe "with included_deleted = true", ->
-			beforeEach -> 
-				@MongoManager.getProjectsDocs @project_id, include_deleted: true, @callback
+			beforeEach ->
+				@MongoManager.getProjectsDocs @project_id, include_deleted: true, @filter, @callback
 
 			it "should find all via the project_id", ->
 				@db.docs.find
 					.calledWith({
 						project_id: ObjectId(@project_id)
-					}, {})
+					}, @filter)
 					.should.equal true
 
 			it "should call the callback with the docs", ->
@@ -76,7 +78,7 @@ describe "MongoManager", ->
 			@oldRev = 77
 
 		it "should upsert the document", (done)->	
-			@MongoManager.upsertIntoDocCollection @project_id, @doc_id, @lines, (err)=>
+			@MongoManager.upsertIntoDocCollection @project_id, @doc_id, {@lines}, (err)=>
 				args = @db.docs.update.args[0]
 				assert.deepEqual args[0], {_id: ObjectId(@doc_id)}
 				assert.equal args[1]["$set"]["lines"], @lines
@@ -85,7 +87,7 @@ describe "MongoManager", ->
 				done()
 
 		it "should return the error", (done)->
-			@MongoManager.upsertIntoDocCollection @project_id, @doc_id, @lines, (err)=>
+			@MongoManager.upsertIntoDocCollection @project_id, @doc_id, {@lines}, (err)=>
 				err.should.equal @stubbedErr
 				done()
 
@@ -94,15 +96,15 @@ describe "MongoManager", ->
 			@db.docs.update = sinon.stub().callsArgWith(2, @stubbedErr)
 			@oldRev = 77
 
-		it "should process the update", (done)->	
-			@MongoManager.markDocAsDeleted @doc_id, (err)=>
+		it "should process the update", (done) ->
+			@MongoManager.markDocAsDeleted @project_id, @doc_id, (err)=>
 				args = @db.docs.update.args[0]
-				assert.deepEqual args[0], {_id: ObjectId(@doc_id)}
+				assert.deepEqual args[0], {_id: ObjectId(@doc_id), project_id: ObjectId(@project_id)}
 				assert.equal args[1]["$set"]["deleted"], true
 				done()
 
 		it "should return the error", (done)->
-			@MongoManager.markDocAsDeleted @doc_id, (err)=>
+			@MongoManager.markDocAsDeleted @project_id, @doc_id, (err)=>
 				err.should.equal @stubbedErr
 				done()
 

+ 152 - 0
services/docstore/test/unit/coffee/RangeManagerTests.coffee

@@ -0,0 +1,152 @@
+SandboxedModule = require('sandboxed-module')
+sinon = require('sinon')
+require('chai').should()
+modulePath = require('path').join __dirname, '../../../app/js/RangeManager'
+ObjectId = require("mongojs").ObjectId
+assert = require("chai").assert
+_ = require "underscore"
+
+describe "RangeManager", ->
+	beforeEach ->
+		@RangeManager = SandboxedModule.require modulePath, requires:
+			"./mongojs":
+				ObjectId: ObjectId
+
+	describe "jsonRangesToMongo", ->
+		it "should convert ObjectIds and dates to proper objects", ->
+			change_id = ObjectId().toString()
+			comment_id = ObjectId().toString()
+			user_id = ObjectId().toString()
+			thread_id = ObjectId().toString()
+			ts = new Date().toJSON()
+			@RangeManager.jsonRangesToMongo({
+				changes: [{
+					id: change_id
+					op: { i: "foo", p: 3 }
+					metadata:
+						user_id: user_id
+						ts: ts
+				}]
+				comments: [{
+					id: comment_id
+					op: { c: "foo", p: 3, t: thread_id }
+				}]
+			}).should.deep.equal {
+				changes: [{
+					id: ObjectId(change_id)
+					op: { i: "foo", p: 3 }
+					metadata:
+						user_id: ObjectId(user_id)
+						ts: new Date(ts)
+				}]
+				comments: [{
+					id: ObjectId(comment_id)
+					op: { c: "foo", p: 3, t: ObjectId(thread_id) }
+				}]
+			}
+		
+		it "should leave malformed ObjectIds as they are", ->
+			change_id = "foo"
+			comment_id = "bar"
+			user_id = "baz"
+			@RangeManager.jsonRangesToMongo({
+				changes: [{
+					id: change_id
+					metadata:
+						user_id: user_id
+				}]
+				comments: [{
+					id: comment_id
+				}]
+			}).should.deep.equal {
+				changes: [{
+					id: change_id
+					metadata:
+						user_id: user_id
+				}]
+				comments: [{
+					id: comment_id
+				}]
+			}
+		
+		it "should be consistent when transformed through json -> mongo -> json", ->
+			change_id = ObjectId().toString()
+			comment_id = ObjectId().toString()
+			user_id = ObjectId().toString()
+			thread_id = ObjectId().toString()
+			ts = new Date().toJSON()
+			ranges1 = {
+				changes: [{
+					id: change_id
+					op: { i: "foo", p: 3 }
+					metadata:
+						user_id: user_id
+						ts: ts
+				}]
+				comments: [{
+					id: comment_id
+					op: { c: "foo", p: 3, t: thread_id }
+				}]
+			}
+			ranges1_copy = JSON.parse(JSON.stringify(ranges1)) # jsonRangesToMongo modifies in place
+			ranges2 = JSON.parse(JSON.stringify(@RangeManager.jsonRangesToMongo(ranges1_copy)))
+			ranges1.should.deep.equal ranges2
+			
+	describe "shouldUpdateRanges", ->
+		beforeEach () ->
+			@ranges = {
+				changes: [{
+					id: ObjectId()
+					op: { i: "foo", p: 3 }
+					metadata:
+						user_id: ObjectId()
+						ts: new Date()
+				}]
+				comments: [{
+					id: ObjectId()
+					op: { c: "foo", p: 3, t: ObjectId() }
+				}]
+			}
+			@ranges_copy = @RangeManager.jsonRangesToMongo(JSON.parse(JSON.stringify(@ranges)))
+
+		describe "with a blank new range", ->
+			it "should return false", ->
+				@RangeManager.shouldUpdateRanges(@ranges, null).should.equal false
+		
+		describe "with a blank old range", ->
+			it "should treat it like {}", ->
+				@RangeManager.shouldUpdateRanges(null, {}).should.equal false
+				@RangeManager.shouldUpdateRanges(null, @ranges).should.equal true
+		
+		describe "with no changes", ->
+			it "should return false", ->
+				@RangeManager.shouldUpdateRanges(@ranges, @ranges_copy).should.equal false
+		
+		describe "with changes", ->
+			it "should return true when the change id changes", ->
+				@ranges_copy.changes[0].id = ObjectId()
+				@RangeManager.shouldUpdateRanges(@ranges, @ranges_copy).should.equal true
+
+			it "should return true when the change user id changes", ->
+				@ranges_copy.changes[0].metadata.user_id = ObjectId()
+				@RangeManager.shouldUpdateRanges(@ranges, @ranges_copy).should.equal true
+
+			it "should return true when the change ts changes", ->
+				@ranges_copy.changes[0].metadata.ts = new Date(Date.now() + 1000)
+				@RangeManager.shouldUpdateRanges(@ranges, @ranges_copy).should.equal true
+
+			it "should return true when the change op changes", ->
+				@ranges_copy.changes[0].op.i = "bar"
+				@RangeManager.shouldUpdateRanges(@ranges, @ranges_copy).should.equal true
+
+			it "should return true when the comment id changes", ->
+				@ranges_copy.comments[0].id = ObjectId()
+				@RangeManager.shouldUpdateRanges(@ranges, @ranges_copy).should.equal true
+
+			it "should return true when the comment offset changes", ->
+				@ranges_copy.comments[0].op.p = 17
+				@RangeManager.shouldUpdateRanges(@ranges, @ranges_copy).should.equal true
+
+			it "should return true when the comment content changes", ->
+				@ranges_copy.comments[0].op.c = "bar"
+				@RangeManager.shouldUpdateRanges(@ranges, @ranges_copy).should.equal true

Algunos archivos no se mostraron porque demasiados archivos cambiaron en este cambio