From 748315aadc48b363b7038643b395ee3128f2ade7 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 29 Sep 2017 11:06:20 +0100 Subject: [PATCH] handle pathname in DocumentManager.getDoc --- .../app/coffee/DocumentManager.coffee | 20 +++--- .../DocumentManagerTests.coffee | 67 ++++++++++--------- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 21251933d2..366fd645ef 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -17,14 +17,14 @@ module.exports = DocumentManager = timer.done() _callback(args...) - RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, unflushedTime) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, unflushedTime) -> return callback(error) if error? if !lines? or !version? logger.log {project_id, doc_id}, "doc not in redis so getting from persistence API" - PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> + PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname) -> return callback(error) if error? logger.log {project_id, doc_id, lines, version}, "got doc from persistence API" - RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, (error) -> + RedisManager.putDocInMemory project_id, doc_id, lines, version, ranges, pathname, (error) -> return callback(error) if error? callback null, lines, version, ranges, null, false else @@ -35,7 +35,7 @@ module.exports = DocumentManager = callback = (args...) -> timer.done() _callback(args...) - + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges) -> return callback(error) if error? if fromVersion == -1 @@ -57,7 +57,7 @@ module.exports = DocumentManager = UpdateManager = require "./UpdateManager" DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, ranges, unflushedTime, alreadyLoaded) -> return callback(error) if error? - + if oldLines? and oldLines.length > 0 and oldLines[0].text? logger.log doc_id: doc_id, project_id: project_id, oldLines: oldLines, newLines: newLines, "document is JSON so not updating" return callback(null) @@ -115,7 +115,7 @@ module.exports = DocumentManager = DocumentManager.flushDocIfLoaded project_id, doc_id, (error) -> return callback(error) if error? - + # Flush in the background since it requires and http request # to track changes HistoryManager.flushDocChanges project_id, doc_id, (err) -> @@ -141,7 +141,7 @@ module.exports = DocumentManager = RedisManager.updateDocument doc_id, lines, version, [], new_ranges, (error) -> return callback(error) if error? callback() - + deleteComment: (project_id, doc_id, comment_id, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.deleteComment") callback = (args...) -> @@ -159,7 +159,7 @@ module.exports = DocumentManager = callback() getDocAndFlushIfOld: (project_id, doc_id, callback = (error, doc) ->) -> - DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, unflushedTime, alreadyLoaded) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, unflushedTime, alreadyLoaded) -> return callback(error) if error? # if doc was already loaded see if it needs to be flushed if alreadyLoaded and unflushedTime? and (Date.now() - unflushedTime) > MAX_UNFLUSHED_AGE @@ -172,7 +172,7 @@ module.exports = DocumentManager = getDocWithLock: (project_id, doc_id, callback = (error, lines, version) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDoc, project_id, doc_id, callback - + getDocAndRecentOpsWithLock: (project_id, doc_id, fromVersion, callback = (error, lines, version) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDocAndRecentOps, project_id, doc_id, fromVersion, callback @@ -184,7 +184,7 @@ module.exports = DocumentManager = setDocWithLock: (project_id, doc_id, lines, source, user_id, undoing, callback = (error) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.setDoc, project_id, doc_id, lines, source, user_id, undoing, callback - + flushDocIfLoadedWithLock: (project_id, doc_id, callback = (error) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.flushDocIfLoaded, project_id, doc_id, callback diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index aff1cf0bc6..30db63a923 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -27,6 +27,7 @@ describe "DocumentManager", -> @lines = ["one", "two", "three"] @version = 42 @ranges = { comments: "mock", entries: "mock" } + @pathname = '/a/b/c.tex' @unflushedTime = Date.now() describe "flushAndDeleteDoc", -> @@ -36,7 +37,7 @@ describe "DocumentManager", -> @DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2) @HistoryManager.flushDocChanges = sinon.stub().callsArg(2) @DocumentManager.flushAndDeleteDoc @project_id, @doc_id, @callback - + it "should flush the doc", -> @DocumentManager.flushDocIfLoaded .calledWith(@project_id, @doc_id) @@ -52,12 +53,12 @@ describe "DocumentManager", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + it "should flush to the history api", -> @HistoryManager.flushDocChanges .calledWith(@project_id, @doc_id) .should.equal true - + describe "flushDocIfLoaded", -> describe "when the doc is in Redis", -> beforeEach -> @@ -75,7 +76,7 @@ describe "DocumentManager", -> @PersistenceManager.setDoc .calledWith(@project_id, @doc_id, @lines, @version, @ranges) .should.equal true - + it "should call the callback without error", -> @callback.calledWith(null).should.equal true @@ -103,7 +104,7 @@ describe "DocumentManager", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + describe "getDocAndRecentOps", -> describe "with a previous version specified", -> beforeEach -> @@ -146,18 +147,18 @@ describe "DocumentManager", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + describe "getDoc", -> describe "when the doc exists in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @unflushedTime) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, @unflushedTime) @DocumentManager.getDoc @project_id, @doc_id, @callback it "should get the doc from Redis", -> @RedisManager.getDoc .calledWith(@project_id, @doc_id) .should.equal true - + it "should call the callback with the doc info", -> @callback.calledWith(null, @lines, @version, @ranges, @unflushedTime, true).should.equal true @@ -166,8 +167,8 @@ describe "DocumentManager", -> describe "when the doc does not exist in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null) - @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null, null) + @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname) @RedisManager.putDocInMemory = sinon.stub().yields() @DocumentManager.getDoc @project_id, @doc_id, @callback @@ -183,7 +184,7 @@ describe "DocumentManager", -> it "should set the doc in Redis", -> @RedisManager.putDocInMemory - .calledWith(@project_id, @doc_id, @lines, @version, @ranges) + .calledWith(@project_id, @doc_id, @lines, @version, @ranges, @pathname) .should.equal true it "should call the callback with the doc info", -> @@ -191,7 +192,7 @@ describe "DocumentManager", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + describe "setDoc", -> describe "with plain tex lines", -> beforeEach -> @@ -240,13 +241,13 @@ describe "DocumentManager", -> @DocumentManager.flushDocIfLoaded .calledWith(@project_id, @doc_id) .should.equal true - + it "should call the callback", -> @callback.calledWith(null).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - + describe "when not already loaded", -> beforeEach -> @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, null, false) @@ -263,21 +264,21 @@ describe "DocumentManager", -> it "should return the callback with an error", -> @callback.calledWith(new Error("No lines were passed to setDoc")) - + it "should not try to get the doc lines", -> @DocumentManager.getDoc.called.should.equal false - + describe "with the undoing flag", -> beforeEach -> # Copy ops so we don't interfere with other tests @ops = [{ i: "foo", p: 4 }, { d: "bar", p: 42 }] @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, true, @callback - + it "should set the undo flag on each op", -> for op in @ops op.u.should.equal true - + describe "acceptChanges", -> beforeEach -> @change_id = "mock-change-id" @@ -289,33 +290,33 @@ describe "DocumentManager", -> @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges) @RangesManager.acceptChanges = sinon.stub().yields(null, @updated_ranges) @RedisManager.updateDocument = sinon.stub().yields() - + describe "successfully with a single change", -> beforeEach -> @DocumentManager.acceptChanges @project_id, @doc_id, [ @change_id ], @callback - + it "should get the document's current ranges", -> @DocumentManager.getDoc .calledWith(@project_id, @doc_id) .should.equal true - + it "should apply the accept change to the ranges", -> @RangesManager.acceptChanges .calledWith([ @change_id ], @ranges) .should.equal true - + it "should save the updated ranges", -> @RedisManager.updateDocument .calledWith(@doc_id, @lines, @version, [], @updated_ranges) .should.equal true - + it "should call the callback", -> @callback.called.should.equal true describe "successfully with multiple changes", -> beforeEach -> @DocumentManager.acceptChanges @project_id, @doc_id, @change_ids, @callback - + it "should apply the accept change to the ranges", -> @RangesManager.acceptChanges .calledWith(@change_ids, @ranges) @@ -328,11 +329,11 @@ describe "DocumentManager", -> it "should not save anything", -> @RedisManager.updateDocument.called.should.equal false - + it "should call the callback with a not found error", -> error = new Errors.NotFoundError("document not found: #{@doc_id}") @callback.calledWith(error).should.equal true - + describe "deleteComment", -> beforeEach -> @comment_id = "mock-comment-id" @@ -343,26 +344,26 @@ describe "DocumentManager", -> @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @ranges) @RangesManager.deleteComment = sinon.stub().yields(null, @updated_ranges) @RedisManager.updateDocument = sinon.stub().yields() - + describe "successfully", -> beforeEach -> @DocumentManager.deleteComment @project_id, @doc_id, @comment_id, @callback - + it "should get the document's current ranges", -> @DocumentManager.getDoc .calledWith(@project_id, @doc_id) .should.equal true - + it "should delete the comment from the ranges", -> @RangesManager.deleteComment .calledWith(@comment_id, @ranges) .should.equal true - + it "should save the updated ranges", -> @RedisManager.updateDocument .calledWith(@doc_id, @lines, @version, [], @updated_ranges) .should.equal true - + it "should call the callback", -> @callback.called.should.equal true @@ -373,7 +374,7 @@ describe "DocumentManager", -> it "should not save anything", -> @RedisManager.updateDocument.called.should.equal false - + it "should call the callback with a not found error", -> error = new Errors.NotFoundError("document not found: #{@doc_id}") @callback.calledWith(error).should.equal true @@ -389,7 +390,7 @@ describe "DocumentManager", -> describe "when the doc is in Redis", -> describe "and has changes to be flushed", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, Date.now() - 1e9, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges, @pathname, Date.now() - 1e9, true) @DocumentManager.getDocAndFlushIfOld @project_id, @doc_id, @callback it "should get the doc", ->