From af5d01e440d47106154c88492d357534daebc04c Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 20 Jan 2016 15:05:31 +0000 Subject: [PATCH] Flush track changes when unloading data from redis --- .../app/coffee/DocumentManager.coffee | 13 ++++++++----- .../coffee/ApplyingUpdatesToADocTests.coffee | 4 ++-- .../coffee/DeletingADocumentTests.coffee | 16 ++++++++++++++-- .../coffee/DeletingAProjectTests.coffee | 10 ++++++++++ .../coffee/SettingADocumentTests.coffee | 6 +++--- .../flushAndDeleteDocTests.coffee | 7 +++++++ 6 files changed, 44 insertions(+), 12 deletions(-) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 1b1afeda3c..69311bd979 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -4,6 +4,7 @@ DocOpsManager = require "./DocOpsManager" DiffCodec = require "./DiffCodec" logger = require "logger-sharelatex" Metrics = require "./Metrics" +TrackChangesManager = require "./TrackChangesManager" module.exports = DocumentManager = getDoc: (project_id, doc_id, _callback = (error, lines, version) ->) -> @@ -109,11 +110,13 @@ module.exports = DocumentManager = DocumentManager.flushDocIfLoaded project_id, doc_id, (error) -> return callback(error) if error? - # We should flush pending ops to track-changes here but this is - # already done in the real-time WebsocketController.leaveProject - # method so we leave it there. Note, if you ever add the flush - # in here be sure to do it in the background because it can take - # a long time. + + # Flush in the background since it requires and http request + # to track changes + TrackChangesManager.flushDocChanges project_id, doc_id, (err) -> + if err? + logger.err {err, project_id, doc_id}, "error flushing to track changes" + RedisManager.removeDocFromMemory project_id, doc_id, (error) -> return callback(error) if error? callback null diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index e9b5da5b1d..a1c56ee41e 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -365,8 +365,8 @@ describe "Applying updates to a large doc (uses compression)", -> after -> MockTrackChangesApi.flushDoc.restore() - it "should flush the doc twice", -> - MockTrackChangesApi.flushDoc.calledTwice.should.equal true + it "should flush the doc", -> + MockTrackChangesApi.flushDoc.called.should.equal true describe "when there is no version in Mongo", -> before (done) -> diff --git a/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee index 139ba9bbed..e08b7fc12f 100644 --- a/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee @@ -3,6 +3,7 @@ chai = require("chai") chai.should() {db, ObjectId} = require "../../../app/js/mongojs" +MockTrackChangesApi = require "./helpers/MockTrackChangesApi" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -18,6 +19,11 @@ describe "Deleting a document", -> }] v: @version @result = ["one", "one and a half", "two", "three"] + + sinon.spy MockTrackChangesApi, "flushDoc" + + after -> + MockTrackChangesApi.flushDoc.restore() describe "when the updated doc exists in the doc updater", -> before (done) -> @@ -38,7 +44,7 @@ describe "Deleting a document", -> setTimeout () => DocUpdaterClient.deleteDoc @project_id, @doc_id, (error, res, body) => @statusCode = res.statusCode - done() + setTimeout done, 200 , 200 after -> @@ -70,6 +76,9 @@ describe "Deleting a document", -> .calledWith(@project_id, @doc_id) .should.equal true done() + + it "should flush track changes", -> + MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true describe "when the doc is not in the doc updater", -> before (done) -> @@ -81,7 +90,7 @@ describe "Deleting a document", -> sinon.spy MockWebApi, "getDocument" DocUpdaterClient.deleteDoc @project_id, @doc_id, (error, res, body) => @statusCode = res.statusCode - done() + setTimeout done, 200 after -> MockWebApi.setDocumentLines.restore() @@ -100,6 +109,9 @@ describe "Deleting a document", -> .calledWith(@project_id, @doc_id) .should.equal true done() + + it "should flush track changes", -> + MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true diff --git a/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee b/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee index 5bfc5a6ee8..2f7a47ff8b 100644 --- a/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee +++ b/services/document-updater/test/acceptance/coffee/DeletingAProjectTests.coffee @@ -3,6 +3,7 @@ chai = require("chai") chai.should() async = require "async" +MockTrackChangesApi = require "./helpers/MockTrackChangesApi" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -37,6 +38,11 @@ describe "Deleting a project", -> lines: doc.lines version: doc.update.v } + + sinon.spy MockTrackChangesApi, "flushDoc" + + after -> + MockTrackChangesApi.flushDoc.restore() describe "with documents which have been updated", -> before (done) -> @@ -78,5 +84,9 @@ describe "Deleting a project", -> ), () -> MockWebApi.getDocument.restore() done() + + it "should flush each doc in track changes", -> + for doc in @docs + MockTrackChangesApi.flushDoc.calledWith(doc.id).should.equal true diff --git a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee index cf0d224995..0d05e30982 100644 --- a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee @@ -86,7 +86,7 @@ describe "Setting a document", -> throw error if error? DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, (error, res, body) => @statusCode = res.statusCode - done() + setTimeout done, 200 it "should return a 204 status code", -> @statusCode.should.equal 204 @@ -96,8 +96,8 @@ describe "Setting a document", -> .calledWith(@project_id, @doc_id, @newLines) .should.equal true - it "should flush track changes"#, -> - # MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true + it "should flush track changes", -> + MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true it "should remove the document from redis", (done) -> rclient.get "doclines:#{@doc_id}", (error, lines) => diff --git a/services/document-updater/test/unit/coffee/DocumentManager/flushAndDeleteDocTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/flushAndDeleteDocTests.coffee index 85a25ee5a7..3b9a4314a4 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/flushAndDeleteDocTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/flushAndDeleteDocTests.coffee @@ -9,6 +9,7 @@ describe "DocumentUpdater - flushAndDeleteDoc", -> @DocumentManager = SandboxedModule.require modulePath, requires: "./RedisManager": @RedisManager = {} "./PersistenceManager": @PersistenceManager = {} + "./TrackChangesManager": @TrackChangesManager = {} "logger-sharelatex": @logger = {log: sinon.stub()} "./DocOpsManager" :{} "./Metrics": @Metrics = @@ -22,6 +23,7 @@ describe "DocumentUpdater - flushAndDeleteDoc", -> beforeEach -> @RedisManager.removeDocFromMemory = sinon.stub().callsArg(2) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2) + @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(2) @DocumentManager.flushAndDeleteDoc @project_id, @doc_id, @callback it "should flush the doc", -> @@ -39,3 +41,8 @@ describe "DocumentUpdater - flushAndDeleteDoc", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true + + it "should flush to track changes", -> + @TrackChangesManager.flushDocChanges + .calledWith(@project_id, @doc_id) + .should.equal true