Use version from web, with fallback to old mongo collection

services/document-updater/Gruntfile.coffee

@@ -52,6 +52,7 @@ module.exports = (grunt) ->
 		clean:
 			app: ["app/js"]
 			acceptance_tests: ["test/acceptance/js"]
+			unit_tests: ["test/unit/js"]
 
 		mochaTest:
 			unit:
@@ -102,7 +103,7 @@ module.exports = (grunt) ->
 	grunt.registerTask 'help', 'Display this help list', 'availabletasks'
 
 	grunt.registerTask 'compile:server', 'Compile the server side coffee script', ['clean:app', 'coffee:app', 'coffee:app_dir']
-	grunt.registerTask 'compile:unit_tests', 'Compile the unit tests', ['coffee:unit_tests']
+	grunt.registerTask 'compile:unit_tests', 'Compile the unit tests', ['clean:unit_tests', 'coffee:unit_tests']
 	grunt.registerTask 'compile:acceptance_tests', 'Compile the acceptance tests', ['clean:acceptance_tests', 'coffee:acceptance_tests']
 	grunt.registerTask 'compile:tests', 'Compile all the tests', ['compile:acceptance_tests', 'compile:unit_tests']
 	grunt.registerTask 'compile', 'Compiles everything need to run document-updater-sharelatex', ['compile:server']

services/document-updater/app/coffee/DocOpsManager.coffee

@@ -14,3 +14,4 @@ module.exports = DocOpsManager =
 				return callback(error) if error?
 				callback null, version
 
+

services/document-updater/app/coffee/PersistenceManager.coffee

@@ -2,9 +2,35 @@ request = require "request"
 Settings = require "settings-sharelatex"
 Errors = require "./Errors"
 Metrics = require "./Metrics"
+{db, ObjectId} = require("./mongojs")
 
 module.exports = PersistenceManager =
-	getDoc: (project_id, doc_id, _callback = (error, lines) ->) ->
+	getDoc: (project_id, doc_id, callback = (error, lines, version) ->) ->
+		PersistenceManager.getDocFromWeb project_id, doc_id, (error, lines, version) ->
+			return callback(error) if error?
+			if version?
+				callback null, lines, version
+			else
+				PersistenceManager.getDocVersionInMongo doc_id, (error, version) ->
+					return callback(error) if error?
+					if version?
+						callback null, lines, version
+					else
+						callback null, lines, 0
+		
+	getDocVersionInMongo: (doc_id, callback = (error, version) ->) ->
+		db.docOps.find {
+			doc_id: ObjectId(doc_id)
+		}, {
+			version: 1
+		}, (error, docs) ->
+			return callback(error) if error?
+			if docs.length < 1 or !docs[0].version?
+				return callback null, null
+			else
+				return callback null, docs[0].version
+
+	getDocFromWeb: (project_id, doc_id, _callback = (error, lines, version) ->) ->
 		timer = new Metrics.Timer("persistenceManager.getDoc")
 		callback = (args...) ->
 			timer.done()
@@ -64,4 +90,3 @@ module.exports = PersistenceManager =
 				return callback(new Error("error accessing web API: #{url} #{res.statusCode}"))
 
 			
-			

services/document-updater/app/coffee/mongojs.coffee

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

services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee

@@ -3,6 +3,7 @@ chai = require("chai")
 chai.should()
 async = require "async"
 rclient = require("redis").createClient()
+{db, ObjectId} = require "../../../app/js/mongojs"
 
 MockTrackChangesApi = require "./helpers/MockTrackChangesApi"
 MockWebApi = require "./helpers/MockWebApi"
@@ -92,9 +93,9 @@ describe "Applying updates to a doc", ->
 		describe "when the ops come in a single linear order", ->
 			before ->
 				[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
-				@lines = ["", "", ""]
+				lines = ["", "", ""]
 				MockWebApi.insertDoc @project_id, @doc_id, {
-					lines: @lines
+					lines: lines
 					version: 0
 				}
 
@@ -111,7 +112,7 @@ describe "Applying updates to a doc", ->
 					{ doc_id: @doc_id, v: 9,  op: [i: "l", p: 9 ] }
 					{ doc_id: @doc_id, v: 10, op: [i: "d", p: 10] }
 				]
-				@result = ["hello world", "", ""]
+				@my_result = ["hello world", "", ""]
 
 			it "should be able to continue applying updates when the project has been deleted", (done) ->
 				actions = []
@@ -126,7 +127,7 @@ describe "Applying updates to a doc", ->
 				async.series actions, (error) =>
 					throw error if error?
 					DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
-						doc.lines.should.deep.equal @result
+						doc.lines.should.deep.equal @my_result
 						done()
 
 			it "should push the applied updates to the track changes api", (done) ->
@@ -142,9 +143,9 @@ describe "Applying updates to a doc", ->
 		describe "when older ops come in after the delete", ->
 			before ->
 				[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
-				@lines = ["", "", ""]
+				lines = ["", "", ""]
 				MockWebApi.insertDoc @project_id, @doc_id, {
-					lines: @lines
+					lines: lines
 					version: 0
 				}
 
@@ -156,7 +157,7 @@ describe "Applying updates to a doc", ->
 					{ doc_id: @doc_id, v: 4, op: [i: "o", p: 4 ] }
 					{ doc_id: @doc_id, v: 0, op: [i: "world", p: 1 ] }
 				]
-				@result = ["hello", "world", ""]
+				@my_result = ["hello", "world", ""]
 
 			it "should be able to continue applying updates when the project has been deleted", (done) ->
 				actions = []
@@ -171,7 +172,7 @@ describe "Applying updates to a doc", ->
 				async.series actions, (error) =>
 					throw error if error?
 					DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
-						doc.lines.should.deep.equal @result
+						doc.lines.should.deep.equal @my_result
 						done()
 
 	describe "with a broken update", ->
@@ -191,28 +192,91 @@ describe "Applying updates to a doc", ->
 				done()
 
 	describe "with enough updates to flush to the track changes api", ->
-		beforeEach (done) ->
+		before (done) ->
 			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
 			MockWebApi.insertDoc @project_id, @doc_id, {
 				lines: @lines
 				version: 0
 			}
-			@updates = []
+			updates = []
 			for v in [0..99] # Should flush after 50 ops
-				@updates.push
+				updates.push
 					doc_id: @doc_id,
 					op: [i: v.toString(), p: 0]
 					v: v
 
 			sinon.spy MockTrackChangesApi, "flushDoc"
 
-			DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) =>
+			DocUpdaterClient.sendUpdates @project_id, @doc_id, updates, (error) =>
 				throw error if error?
 				setTimeout done, 200
 
-		afterEach ->
+		after ->
 			MockTrackChangesApi.flushDoc.restore()
 
 		it "should flush the doc twice", ->
 			MockTrackChangesApi.flushDoc.calledTwice.should.equal true
 
+	describe "when the document does not have a version in the web api but does in Mongo", ->
+		before (done) ->
+			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+			MockWebApi.insertDoc @project_id, @doc_id, {
+				lines: @lines
+			}
+
+			db.docOps.insert {
+				doc_id: ObjectId(@doc_id)
+				version: @version
+			}, (error) =>
+				throw error if error?
+				DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) ->
+					throw error if error?
+					setTimeout done, 200
+
+		it "should update the doc (using the mongo version)", (done) ->
+			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
+				doc.lines.should.deep.equal @result
+				done()
+
+	describe "when the document version in the web api is ahead of Mongo", ->
+		before (done) ->
+			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+			MockWebApi.insertDoc @project_id, @doc_id, {
+				lines: @lines
+				version: @version
+			}
+
+			db.docOps.insert {
+				doc_id: ObjectId(@doc_id)
+				version: @version - 20
+			}, (error) =>
+				throw error if error?
+				DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) ->
+					throw error if error?
+					setTimeout done, 200
+
+		it "should update the doc (using the web version)", (done) ->
+			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
+				doc.lines.should.deep.equal @result
+				done()
+
+	describe "when there is no version yet", ->
+		before (done) ->
+			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+			MockWebApi.insertDoc @project_id, @doc_id, {
+				lines: @lines
+			}
+
+			update = 
+				doc: @doc_id
+				op: @update.op
+				v: 0
+			DocUpdaterClient.sendUpdate @project_id, @doc_id, update, (error) ->
+				throw error if error?
+				setTimeout done, 200
+
+		it "should update the doc (using version = 0)", (done) ->
+			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
+				doc.lines.should.deep.equal @result
+				done()
+

services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee

@@ -6,15 +6,11 @@ SandboxedModule = require('sandboxed-module')
 
 describe "DocOpsManager", ->
 	beforeEach ->
-		@doc_id = "doc-id"
+		@doc_id = ObjectId().toString()
-		@project_id = "project-id"
+		@project_id = ObjectId().toString()
 		@callback = sinon.stub()
 		@DocOpsManager = SandboxedModule.require modulePath, requires:
 			"./RedisManager": @RedisManager = {}
-			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
-			"./Metrics": @Metrics =
-				Timer: class Timer
-					done: sinon.stub()
 			"./TrackChangesManager": @TrackChangesManager = {}
 
 	describe "getPreviousDocOps", ->

services/document-updater/test/unit/coffee/PersistenceManager/getDocFromWebTests.coffee

@@ -0,0 +1,86 @@
+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.getDocFromWeb", ->
+	beforeEach ->
+		@PersistenceManager = SandboxedModule.require modulePath, requires:
+			"request": @request = sinon.stub()
+			"settings-sharelatex": @Settings = {}
+			"./Metrics": @Metrics =
+				Timer: class Timer
+					done: 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.getDocFromWeb(@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
+				})
+				.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.getDocFromWeb(@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.getDocFromWeb(@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.getDocFromWeb(@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
+		
+		

services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee

@@ -3,7 +3,7 @@ chai = require('chai')
 should = chai.should()
 modulePath = "../../../../app/js/PersistenceManager.js"
 SandboxedModule = require('sandboxed-module')
-Errors = require "../../../../app/js/Errors"
+{ObjectId} = require("mongojs")
 
 describe "PersistenceManager.getDoc", ->
 	beforeEach ->
@@ -13,74 +13,54 @@ describe "PersistenceManager.getDoc", ->
 			"./Metrics": @Metrics =
 				Timer: class Timer
 					done: sinon.stub()
-		@project_id = "project-id-123"
+			"./mongojs":
-		@doc_id = "doc-id-123"
+				db: @db = { docOps: {} }
-		@lines = ["one", "two", "three"]
+				ObjectId: ObjectId
-		@version = 42
+
+		@project_id = ObjectId().toString()
+		@doc_id = ObjectId().toString()
 		@callback = sinon.stub()
-		@Settings.apis =
+		@lines = ["mock", "doc", "lines"]
-			web:
+		@version = 42
-				url: @url = "www.example.com"
-				user: @user = "sharelatex"
-				pass: @pass = "password"
 
-	describe "with a successful response from the web api", ->
+	describe "when the version is set in the web api", ->
 		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines, version: @version))
+			@PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, @version)
-			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+			@PersistenceManager.getDocVersionInMongo = sinon.stub()
+			@PersistenceManager.getDoc @project_id, @doc_id, @callback
 
-		it "should call the web api", ->
+		it "should look up the doc in the web api", ->
-			@request
+			@PersistenceManager.getDocFromWeb
-				.calledWith({
+				.calledWith(@project_id, @doc_id)
-					url: "#{@url}/project/#{@project_id}/doc/#{@doc_id}"
-					method: "GET"
-					headers:
-						"accept": "application/json"
-					auth:
-						user: @user
-						pass: @pass
-						sendImmediately: true
-					jar: false
-				})
 				.should.equal true
 
-		it "should call the callback with the doc lines and version", ->
+		it "should not look up the version in Mongo", ->
+			@PersistenceManager.getDocVersionInMongo
+				.called.should.equal false
+
+		it "should call the callback with the lines and version", ->
 			@callback.calledWith(null, @lines, @version).should.equal true
 
-		it "should time the execution", ->
+	describe "when the version is not set in the web api, but is in Mongo", ->
-			@Metrics.Timer::done.called.should.equal true
-
-	describe "when request returns an error", ->
 		beforeEach ->
-			@request.callsArgWith(1, @error = new Error("oops"), null, null)
+			@PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, null)
-			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+			@PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version)
+			@PersistenceManager.getDoc @project_id, @doc_id, @callback
 
-		it "should return the error", ->
+		it "should look up the version in Mongo", ->
-			@callback.calledWith(@error).should.equal true
+			@PersistenceManager.getDocVersionInMongo
+				.calledWith(@doc_id)
+				.should.equal true
 
-		it "should time the execution", ->
+		it "should call the callback with the lines and version", ->
-			@Metrics.Timer::done.called.should.equal true
+			@callback.calledWith(null, @lines, @version).should.equal true
 
-	describe "when the request returns 404", ->
+	describe "when the version is not set", ->
 		beforeEach ->
-			@request.callsArgWith(1, null, {statusCode: 404}, "")
+			@PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, null)
-			@PersistenceManager.getDoc(@project_id, @doc_id, @callback)
+			@PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, null)
-			
+			@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
 
+		it "should call the callback with the lines and version = 0", ->
+			@callback.calledWith(null, @lines, 0).should.equal true
 

services/document-updater/test/unit/coffee/PersistenceManager/getDocVersionInMongoTests.coffee

@@ -0,0 +1,46 @@
+sinon = require('sinon')
+chai = require('chai')
+should = chai.should()
+modulePath = "../../../../app/js/PersistenceManager.js"
+SandboxedModule = require('sandboxed-module')
+Errors = require "../../../../app/js/Errors"
+{ObjectId} = require("mongojs")
+
+describe "PersistenceManager.getDocVersionInMongo", ->
+	beforeEach ->
+		@PersistenceManager = SandboxedModule.require modulePath, requires:
+			"request": @request = sinon.stub()
+			"settings-sharelatex": @Settings = {}
+			"./Metrics": @Metrics =
+				Timer: class Timer
+					done: sinon.stub()
+			"./mongojs":
+				db: @db = { docOps: {} }
+				ObjectId: ObjectId
+
+		@doc_id = ObjectId().toString()
+		@callback = sinon.stub()
+
+	describe "getDocVersionInMongo", ->
+		describe "when the doc exists", ->
+			beforeEach ->
+				@doc =
+					version: @version = 42
+				@db.docOps.find = sinon.stub().callsArgWith(2, null, [@doc])
+				@PersistenceManager.getDocVersionInMongo @doc_id, @callback
+
+			it "should look for the doc in the database", ->
+				@db.docOps.find
+					.calledWith({ doc_id: ObjectId(@doc_id) }, {version: 1})
+					.should.equal true
+
+			it "should call the callback with the version", ->
+				@callback.calledWith(null, @version).should.equal true
+
+		describe "when the doc doesn't exist", ->
+			beforeEach ->
+				@db.docOps.find = sinon.stub().callsArgWith(2, null, [])
+				@PersistenceManager.getDocVersionInMongo @doc_id, @callback
+
+			it "should call the callback with null", ->
+				@callback.calledWith(null, null).should.equal true

services/document-updater/test/unit/js/module-loader.js

@@ -1,29 +0,0 @@
-var vm = require('vm');
-var fs = require('fs');
-var path = require('path');
-
-module.exports.loadModule = function(filePath, mocks) {
-  mocks = mocks || {};
-
-  // this is necessary to allow relative path modules within loaded file
-  // i.e. requiring ./some inside file /a/b.js needs to be resolved to /a/some
-  var resolveModule = function(module) {
-    if (module.charAt(0) !== '.') return module;
-    return path.resolve(path.dirname(filePath), module);
-  };
-
-  var exports = {};
-  var context = {
-    require: function(name) {
-      return mocks[name] || require(resolveModule(name));
-    },
-    console: console,
-    exports: exports,
-    module: {
-      exports: exports
-    }
-  };
-  file = fs.readFileSync(filePath);
-  vm.runInNewContext(file, context);
-  return context;
-};