diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee index 4b084f1574..bd119234fc 100644 --- a/services/document-updater/app/coffee/PersistenceManager.coffee +++ b/services/document-updater/app/coffee/PersistenceManager.coffee @@ -7,30 +7,29 @@ logger = require "logger-sharelatex" module.exports = PersistenceManager = getDoc: (project_id, doc_id, callback = (error, lines, version) ->) -> - PersistenceManager.getDocFromWeb project_id, doc_id, (error, lines, version) -> + PersistenceManager.getDocFromWeb project_id, doc_id, (error, lines, webVersion) -> return callback(error) if error? - if version? + PersistenceManager.getDocVersionInMongo doc_id, (error, mongoVersion) -> + return callback(error) if error? + if !webVersion? and !mongoVersion? + version = 0 + else if !webVersion? + logger.warn project_id: project_id, doc_id: doc_id, "loading doc version from mongo - deprecated" + version = mongoVersion + else if !mongoVersion? + version = webVersion + else if webVersion > mongoVersion + version = webVersion + else + version = mongoVersion callback null, lines, version - else - logger.warn project_id: project_id, doc_id: doc_id, "loading doc version from mongo - deprecated" - 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) -> + + setDoc: (project_id, doc_id, lines, version, callback = (error) ->) -> + PersistenceManager.setDocInWeb project_id, doc_id, lines, version, (error) -> return callback(error) if error? - if docs.length < 1 or !docs[0].version? - return callback null, null - else - return callback null, docs[0].version + PersistenceManager.setDocVersionInMongo doc_id, version, (error) -> + return callback(error) if error? + callback() getDocFromWeb: (project_id, doc_id, _callback = (error, lines, version) ->) -> timer = new Metrics.Timer("persistenceManager.getDoc") @@ -62,7 +61,7 @@ module.exports = PersistenceManager = else return callback(new Error("error accessing web API: #{url} #{res.statusCode}")) - setDoc: (project_id, doc_id, lines, version, _callback = (error) ->) -> + setDocInWeb: (project_id, doc_id, lines, version, _callback = (error) ->) -> timer = new Metrics.Timer("persistenceManager.setDoc") callback = (args...) -> timer.done() @@ -90,5 +89,27 @@ module.exports = PersistenceManager = return callback(new Errors.NotFoundError("doc not not found: #{url}")) else return callback(new Error("error accessing web API: #{url} #{res.statusCode}")) + + 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 + + setDocVersionInMongo: (doc_id, version, callback = (error) ->) -> + db.docOps.update { + doc_id: ObjectId(doc_id) + }, { + $set: version: version + }, { + upsert: true + }, callback + diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index 94730b406c..f312dbbd9e 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -260,6 +260,28 @@ describe "Applying updates to a doc", -> doc.lines.should.deep.equal @result done() + describe "when the document version in Mongo is ahead of the web api", -> + before (done) -> + [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] + MockWebApi.insertDoc @project_id, @doc_id, { + lines: @lines + version: @version - 20 + } + + 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 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()] diff --git a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee index 04cb52c478..4f4f2f04e2 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee @@ -2,6 +2,7 @@ sinon = require "sinon" chai = require("chai") chai.should() async = require "async" +{db, ObjectId} = require "../../../app/js/mongojs" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -53,6 +54,17 @@ describe "Flushing a doc to Mongo", -> .calledWith(@project_id, @doc_id, @version + 1) .should.equal true + it "should store the updated doc version into mongo", (done) -> + db.docOps.find { + doc_id: ObjectId(@doc_id) + }, { + version: 1 + }, (error, docs) => + doc = docs[0] + doc.version.should.equal @version + 1 + done() + + describe "when the doc does not exist in the doc updater", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee index 5b4efe402f..e9088fac30 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee @@ -24,10 +24,10 @@ describe "PersistenceManager.getDoc", -> @lines = ["mock", "doc", "lines"] @version = 42 - describe "when the version is set in the web api", -> + describe "when the version is set in the web api but not Mongo", -> beforeEach -> @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, @version) - @PersistenceManager.getDocVersionInMongo = sinon.stub() + @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, null) @PersistenceManager.getDoc @project_id, @doc_id, @callback it "should look up the doc in the web api", -> @@ -35,9 +35,10 @@ describe "PersistenceManager.getDoc", -> .calledWith(@project_id, @doc_id) .should.equal true - it "should not look up the version in Mongo", -> + it "should look up the version in Mongo", -> @PersistenceManager.getDocVersionInMongo - .called.should.equal false + .calledWith(@doc_id) + .should.equal true it "should call the callback with the lines and version", -> @callback.calledWith(null, @lines, @version).should.equal true @@ -48,11 +49,6 @@ describe "PersistenceManager.getDoc", -> @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version) @PersistenceManager.getDoc @project_id, @doc_id, @callback - it "should look up the version in Mongo", -> - @PersistenceManager.getDocVersionInMongo - .calledWith(@doc_id) - .should.equal true - it "shoud log a warning", -> @logger.warn .calledWith(project_id: @project_id, doc_id: @doc_id, "loading doc version from mongo - deprecated") @@ -61,6 +57,24 @@ describe "PersistenceManager.getDoc", -> it "should call the callback with the lines and version", -> @callback.calledWith(null, @lines, @version).should.equal true + describe "when the version in the web api is ahead of Mongo", -> + beforeEach -> + @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, @version) + @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version - 20) + @PersistenceManager.getDoc @project_id, @doc_id, @callback + + it "should call the callback with the web version", -> + @callback.calledWith(null, @lines, @version).should.equal true + + describe "when the version in the web api is behind Mongo", -> + beforeEach -> + @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, @version - 20) + @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version) + @PersistenceManager.getDoc @project_id, @doc_id, @callback + + it "should call the callback with the Mongo version", -> + @callback.calledWith(null, @lines, @version).should.equal true + describe "when the version is not set", -> beforeEach -> @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, null) diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/setDocInWebTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/setDocInWebTests.coffee new file mode 100644 index 0000000000..915e72affe --- /dev/null +++ b/services/document-updater/test/unit/coffee/PersistenceManager/setDocInWebTests.coffee @@ -0,0 +1,88 @@ +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.setDocInWeb", -> + 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.setDocInWeb(@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 + }) + .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.setDocInWeb(@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.setDocInWeb(@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.setDocInWeb(@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 + diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee index 82850e3074..7c8cacd095 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee @@ -3,7 +3,6 @@ chai = require('chai') should = chai.should() modulePath = "../../../../app/js/PersistenceManager.js" SandboxedModule = require('sandboxed-module') -Errors = require "../../../../app/js/Errors" describe "PersistenceManager.setDoc", -> beforeEach -> @@ -13,76 +12,28 @@ describe "PersistenceManager.setDoc", -> "./Metrics": @Metrics = Timer: class Timer done: sinon.stub() - @project_id = "project-id-123" - @doc_id = "doc-id-123" - @lines = ["one", "two", "three"] - @version = 42 + "logger-sharelatex": @logger = {warn: sinon.stub()} + + @project_id = "mock-project-id" + @doc_id = "mock-doc-id" @callback = sinon.stub() - @Settings.apis = - web: - url: @url = "www.example.com" - user: @user = "sharelatex" - pass: @pass = "password" + @lines = ["mock", "doc", "lines"] + @version = 42 - 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) + @PersistenceManager.setDocInWeb = sinon.stub().callsArg(4) + @PersistenceManager.setDocVersionInMongo = sinon.stub().callsArg(2) - 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 - }) - .should.equal true + @PersistenceManager.setDoc @project_id, @doc_id, @lines, @version, @callback - it "should call the callback without error", -> - @callback.calledWith(null).should.equal true + it "should set the doc in the web api", -> + @PersistenceManager.setDocInWeb + .calledWith(@project_id, @doc_id, @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.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 + it "should set the doc version in mongo", -> + @PersistenceManager.setDocVersionInMongo + .calledWith(@doc_id, @version) + .should.equal true + it "should call the callback", -> + @callback.called.should.equal true diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/setDocVersionInMongo.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/setDocVersionInMongo.coffee new file mode 100644 index 0000000000..7f228fc341 --- /dev/null +++ b/services/document-updater/test/unit/coffee/PersistenceManager/setDocVersionInMongo.coffee @@ -0,0 +1,43 @@ +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 "setDocVersionInMongo", -> + beforeEach -> + @version = 42 + @db.docOps.update = sinon.stub().callsArg(3) + @PersistenceManager.setDocVersionInMongo @doc_id, @version, @callback + + it "should update the doc version", -> + @db.docOps.update + .calledWith({ + doc_id: ObjectId(@doc_id) + }, { + $set: + version: @version + }, { + upsert: true + }) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true