From f3192da87f1f7f90ee2250f401ea7a5f7d4c1110 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 26 Feb 2014 15:56:52 +0000 Subject: [PATCH] Tell track changes api to flush doc when flushing doc to mongo --- services/document-updater/Gruntfile.coffee | 6 +-- services/document-updater/app.coffee | 9 ----- .../app/coffee/DocumentManager.coffee | 5 +++ .../app/coffee/TrackChangesManager.coffee | 20 ++++++++++ .../config/settings.development.coffee | 2 + .../coffee/ApplyingUpdatesToADocTests.coffee | 18 +++++++++ .../coffee/FlushingAProjectTests.coffee | 14 +++++++ .../coffee/FlushingDocsTests.coffee | 12 +++++- .../coffee/helpers/MockTrackChangesApi.coffee | 20 ++++++++++ .../coffee/helpers/MockWebApi.coffee | 3 +- .../DocumentManager/flushDocTests.coffee | 8 ++++ .../TrackChangesManagerTests.coffee | 38 +++++++++++++++++++ 12 files changed, 141 insertions(+), 14 deletions(-) create mode 100644 services/document-updater/app/coffee/TrackChangesManager.coffee create mode 100644 services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee create mode 100644 services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee diff --git a/services/document-updater/Gruntfile.coffee b/services/document-updater/Gruntfile.coffee index 717ebd464c..8905cabf81 100644 --- a/services/document-updater/Gruntfile.coffee +++ b/services/document-updater/Gruntfile.coffee @@ -45,16 +45,16 @@ module.exports = (grunt) -> clean: app: ["app/js"] - acceptance_tests: ["test/unit/js"] + acceptance_tests: ["test/acceptance/js"] mochaTest: unit: - src: ['test/unit/js/**/*.js'] + src: ["test/unit/js/#{grunt.option('feature') or '**'}/*.js"] options: reporter: grunt.option('reporter') or 'spec' grep: grunt.option("grep") acceptance: - src: ['test/acceptance/js/**/*.js'] + src: ["test/acceptance/js/#{grunt.option('feature') or '*'}.js"] options: reporter: grunt.option('reporter') or 'spec' grep: grunt.option("grep") diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index 1974169f4e..7168017790 100644 --- a/services/document-updater/app.coffee +++ b/services/document-updater/app.coffee @@ -21,15 +21,6 @@ app.configure -> app.use express.bodyParser() app.use app.router -app.configure 'development', ()-> - console.log "Development Enviroment" - app.use express.errorHandler({ dumpExceptions: true, showStack: true }) - -app.configure 'production', ()-> - console.log "Production Enviroment" - app.use express.logger() - app.use express.errorHandler() - rclient.subscribe("pending-updates") rclient.on "message", (channel, doc_key)-> [project_id, doc_id] = Keys.splitProjectIdAndDocId(doc_key) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index aa64ac3d7f..2f3cfb8d79 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -2,6 +2,7 @@ RedisManager = require "./RedisManager" PersistenceManager = require "./PersistenceManager" DocOpsManager = require "./DocOpsManager" DiffCodec = require "./DiffCodec" +TrackChangesManager = require "./TrackChangesManager" logger = require "logger-sharelatex" Metrics = require "./Metrics" @@ -81,6 +82,10 @@ module.exports = DocumentManager = timer.done() _callback(args...) + TrackChangesManager.flushDocChanges doc_id, (error) -> + if error? + logger.error err: error, project_id: project_id, doc_id: doc_id, "error flushing doc to track changes api" + RedisManager.getDoc doc_id, (error, lines, version) -> return callback(error) if error? if !lines? or !version? diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee new file mode 100644 index 0000000000..c0b40331dc --- /dev/null +++ b/services/document-updater/app/coffee/TrackChangesManager.coffee @@ -0,0 +1,20 @@ +settings = require "settings-sharelatex" +request = require "request" +logger = require "logger-sharelatex" + +module.exports = + flushDocChanges: (doc_id, callback = (error) ->) -> + if !settings.apis?.trackchanges? + logger.warn doc_id: doc_id, "track changes API is not configured, so not flushing" + return callback() + + url = "#{settings.apis.trackchanges.url}/doc/#{doc_id}/flush" + logger.log doc_id: doc_id, url: url, "flushing doc in track changes api" + request.post url, (error, res, body)-> + if error? + return callback(error) + else if res.statusCode >= 200 and res.statusCode < 300 + return callback(null) + else + error = new Error("track changes api returned a failure status code: #{res.statusCode}") + return callback(error) diff --git a/services/document-updater/config/settings.development.coffee b/services/document-updater/config/settings.development.coffee index d730bb0f2d..b4f12ed81c 100755 --- a/services/document-updater/config/settings.development.coffee +++ b/services/document-updater/config/settings.development.coffee @@ -12,6 +12,8 @@ module.exports = url: "http://localhost:3000" user: "sharelatex" pass: "password" + trackchanges: + url: "http://localhost:3014" redis: web: diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index 5108a4c2cc..ac1a24223a 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -5,6 +5,7 @@ async = require "async" mongojs = require "../../../app/js/mongojs" db = mongojs.db ObjectId = mongojs.ObjectId +rclient = require("redis").createClient() MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -45,6 +46,11 @@ describe "Applying updates to a doc", -> doc.lines.should.deep.equal @result done() + it "should push the applied updates to the track changes api", (done) -> + rclient.lrange "UncompressedHistoryOps:#{@doc_id}", 0, -1, (error, updates) => + JSON.parse(updates[0]).op.should.deep.equal @update.op + done() + describe "when the document is loaded", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @@ -69,6 +75,11 @@ describe "Applying updates to a doc", -> doc.lines.should.deep.equal @result done() + it "should push the applied updates to the track changes api", (done) -> + rclient.lrange "UncompressedHistoryOps:#{@doc_id}", 0, -1, (error, updates) => + JSON.parse(updates[0]).op.should.deep.equal @update.op + done() + describe "when the document has been deleted", -> describe "when the ops come in a single linear order", -> before -> @@ -112,6 +123,13 @@ describe "Applying updates to a doc", -> doc.lines.should.deep.equal @result done() + it "should push the applied updates to the track changes api", (done) -> + rclient.lrange "UncompressedHistoryOps:#{@doc_id}", 0, -1, (error, updates) => + updates = (JSON.parse(u) for u in updates) + for appliedUpdate, i in @updates + appliedUpdate.op.should.deep.equal updates[i].op + done() + describe "when older ops come in after the delete", -> before -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] diff --git a/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee index 02b44e3fd6..9adbc2458c 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee @@ -4,6 +4,7 @@ chai.should() async = require "async" MockWebApi = require "./helpers/MockWebApi" +MockTrackChangesApi = require "./helpers/MockTrackChangesApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Flushing a project", -> @@ -40,6 +41,8 @@ describe "Flushing a project", -> describe "with documents which have been updated", -> before (done) -> sinon.spy MockWebApi, "setDocumentLines" + sinon.spy MockTrackChangesApi, "flushDoc" + async.series @docs.map((doc) => (callback) => DocUpdaterClient.preloadDoc @project_id, doc.id, (error) => @@ -56,6 +59,7 @@ describe "Flushing a project", -> after -> MockWebApi.setDocumentLines.restore() + MockTrackChangesApi.flushDoc.restore() it "should return a 204 status code", -> @statusCode.should.equal 204 @@ -74,3 +78,13 @@ describe "Flushing a project", -> callback() ), done + it "should flush the docs in the track changes api", (done) -> + # This is done in the background, so wait a little while to ensure it has happened + setTimeout () => + async.series @docs.map((doc) => + (callback) => + MockTrackChangesApi.flushDoc.calledWith(doc.id).should.equal true + ), done + done() + , 100 + diff --git a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee index aaaef99936..6d67dd68a0 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee @@ -4,6 +4,7 @@ chai.should() async = require "async" MockWebApi = require "./helpers/MockWebApi" +MockTrackChangesApi = require "./helpers/MockTrackChangesApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" mongojs = require "../../../app/js/mongojs" db = mongojs.db @@ -31,6 +32,7 @@ describe "Flushing a doc to Mongo", -> lines: @lines } sinon.spy MockWebApi, "setDocumentLines" + sinon.spy MockTrackChangesApi, "flushDoc" DocUpdaterClient.sendUpdates @project_id, @doc_id, [@update], (error) => throw error if error? @@ -40,6 +42,7 @@ describe "Flushing a doc to Mongo", -> after -> MockWebApi.setDocumentLines.restore() + MockTrackChangesApi.flushDoc.restore() it "should flush the updated document to the web api", -> MockWebApi.setDocumentLines @@ -52,6 +55,13 @@ describe "Flushing a doc to Mongo", -> doc.docOps[0].op.should.deep.equal @update.op done() + it "should flush the doc in the track changes api", (done) -> + # This is done in the background, so wait a little while to ensure it has happened + setTimeout () => + MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true + done() + , 100 + describe "when the doc has a large number of ops to be flushed", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @@ -93,5 +103,5 @@ describe "Flushing a doc to Mongo", -> it "should not flush the doc to the web api", -> MockWebApi.setDocumentLines.called.should.equal false - + diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee new file mode 100644 index 0000000000..2fdff0d3ca --- /dev/null +++ b/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee @@ -0,0 +1,20 @@ +express = require("express") +app = express() + +module.exports = MockTrackChangesApi = + flushDoc: (doc_id, callback = (error) ->) -> + callback() + + run: () -> + app.post "/doc/:doc_id/flush", (req, res, next) => + @flushDoc req.params.doc_id, (error) -> + if error? + res.send 500 + else + res.send 204 + + app.listen 3014, (error) -> + throw error if error? + +MockTrackChangesApi.run() + diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee index 7d50eb8377..693e98f8ad 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -34,7 +34,8 @@ module.exports = MockWebApi = else res.send 204 - app.listen(3000) + app.listen 3000, (error) -> + throw error if error? MockWebApi.run() diff --git a/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee index 079341a536..5a4adc4a36 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee @@ -10,6 +10,7 @@ describe "DocumentUpdater - flushDocIfLoaded", -> "./RedisManager": @RedisManager = {} "./PersistenceManager": @PersistenceManager = {} "./DocOpsManager": @DocOpsManager = {} + "./TrackChangesManager": @TrackChangesManager = {} "logger-sharelatex": @logger = {log: sinon.stub()} "./Metrics": @Metrics = Timer: class Timer @@ -25,6 +26,7 @@ describe "DocumentUpdater - flushDocIfLoaded", -> @RedisManager.getDoc = sinon.stub().callsArgWith(1, null, @lines, @version) @PersistenceManager.setDoc = sinon.stub().callsArgWith(3) @DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2) + @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(1) @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback it "should get the doc from redis", -> @@ -48,11 +50,17 @@ describe "DocumentUpdater - flushDocIfLoaded", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true + it "should flush the doc in the track changes api", -> + @TrackChangesManager.flushDocChanges + .calledWith(@doc_id) + .should.equal true + describe "when the document is not in Redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null) @PersistenceManager.setDoc = sinon.stub().callsArgWith(3) @DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2) + @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(1) @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback it "should get the doc from redis", -> diff --git a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee new file mode 100644 index 0000000000..672cdcfaa4 --- /dev/null +++ b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee @@ -0,0 +1,38 @@ +SandboxedModule = require('sandboxed-module') +sinon = require('sinon') +require('chai').should() +modulePath = require('path').join __dirname, '../../../../app/js/TrackChangesManager' + +describe "TrackChangesManager", -> + beforeEach -> + @TrackChangesManager = SandboxedModule.require modulePath, requires: + "request": @request = {} + "settings-sharelatex": @Settings = {} + @doc_id = "mock-doc-id" + @callback = sinon.stub() + + describe "flushDocChanges", -> + beforeEach -> + @Settings.apis = + trackchanges: url: "http://trackchanges.example.com" + + describe "successfully", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204) + @TrackChangesManager.flushDocChanges @doc_id, @callback + + it "should send a request to the track changes api", -> + @request.post + .calledWith("#{@Settings.apis.trackchanges.url}/doc/#{@doc_id}/flush") + .should.equal true + + it "should return the callback", -> + @callback.calledWith(null).should.equal true + + describe "when the track changes api returns an error", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, statusCode: 500) + @TrackChangesManager.flushDocChanges @doc_id, @callback + + it "should return the callback with an error", -> + @callback.calledWith(new Error("track changes api return non-success code: 500")).should.equal true