From 241d1b27d5c0de1a2ea700b36784d3b8b7c6749a Mon Sep 17 00:00:00 2001 From: Michael Walker Date: Wed, 31 Jan 2018 11:27:40 +0000 Subject: [PATCH] Remove _shouldFlushHistoryOps wrapper --- .../app/coffee/HistoryManager.coffee | 7 ++---- .../HistoryManager/HistoryManagerTests.coffee | 22 +++++++++---------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/services/document-updater/app/coffee/HistoryManager.coffee b/services/document-updater/app/coffee/HistoryManager.coffee index cc3da22b11..9ec5db2aa3 100644 --- a/services/document-updater/app/coffee/HistoryManager.coffee +++ b/services/document-updater/app/coffee/HistoryManager.coffee @@ -41,7 +41,7 @@ module.exports = HistoryManager = return callback() if Settings.apis?.project_history?.enabled - if HistoryManager._shouldFlushHistoryOps(project_ops_length, ops, HistoryManager.FLUSH_PROJECT_EVERY_N_OPS) + if HistoryManager.shouldFlushHistoryOps(project_ops_length, ops.length, HistoryManager.FLUSH_PROJECT_EVERY_N_OPS) # Do this in the background since it uses HTTP and so may be too # slow to wait for when processing a doc update. logger.log { project_ops_length, project_id }, "flushing project history api" @@ -49,16 +49,13 @@ module.exports = HistoryManager = HistoryRedisManager.recordDocHasHistoryOps project_id, doc_id, ops, (error) -> return callback(error) if error? - if HistoryManager._shouldFlushHistoryOps(doc_ops_length, ops, HistoryManager.FLUSH_DOC_EVERY_N_OPS) + if HistoryManager.shouldFlushHistoryOps(doc_ops_length, ops.length, HistoryManager.FLUSH_DOC_EVERY_N_OPS) # Do this in the background since it uses HTTP and so may be too # slow to wait for when processing a doc update. logger.log { doc_ops_length, doc_id, project_id }, "flushing track changes api" HistoryManager._flushDocChangesAsync project_id, doc_id callback() - _shouldFlushHistoryOps: (length, ops, threshold) -> - return HistoryManager.shouldFlushHistoryOps(length, ops.length, threshold) - shouldFlushHistoryOps: (length, ops_length, threshold) -> return false if !length # don't flush unless we know the length # We want to flush every 100 ops, i.e. 100, 200, 300, etc diff --git a/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee b/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee index b4f622c7b5..75327a7ae9 100644 --- a/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee @@ -90,9 +90,9 @@ describe "HistoryManager", -> describe "with enough ops to flush project changes", -> beforeEach -> - @HistoryManager._shouldFlushHistoryOps = sinon.stub() - @HistoryManager._shouldFlushHistoryOps.withArgs(@project_ops_length).returns(true) - @HistoryManager._shouldFlushHistoryOps.withArgs(@doc_ops_length).returns(false) + @HistoryManager.shouldFlushHistoryOps = sinon.stub() + @HistoryManager.shouldFlushHistoryOps.withArgs(@project_ops_length).returns(true) + @HistoryManager.shouldFlushHistoryOps.withArgs(@doc_ops_length).returns(false) @HistoryManager.recordAndFlushHistoryOps( @project_id, @doc_id, @ops, @doc_ops_length, @project_ops_length, @callback @@ -115,9 +115,9 @@ describe "HistoryManager", -> describe "with enough ops to flush doc changes", -> beforeEach -> - @HistoryManager._shouldFlushHistoryOps = sinon.stub() - @HistoryManager._shouldFlushHistoryOps.withArgs(@project_ops_length).returns(false) - @HistoryManager._shouldFlushHistoryOps.withArgs(@doc_ops_length).returns(true) + @HistoryManager.shouldFlushHistoryOps = sinon.stub() + @HistoryManager.shouldFlushHistoryOps.withArgs(@project_ops_length).returns(false) + @HistoryManager.shouldFlushHistoryOps.withArgs(@doc_ops_length).returns(true) @HistoryManager.recordAndFlushHistoryOps( @project_id, @doc_id, @ops, @doc_ops_length, @project_ops_length, @callback @@ -154,24 +154,24 @@ describe "HistoryManager", -> it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true - describe "_shouldFlushHistoryOps", -> + describe "shouldFlushHistoryOps", -> it "should return false if the number of ops is not known", -> - @HistoryManager._shouldFlushHistoryOps(null, ['a', 'b', 'c'], 1).should.equal false + @HistoryManager.shouldFlushHistoryOps(null, ['a', 'b', 'c'].length, 1).should.equal false it "should return false if the updates didn't take us past the threshold", -> # Currently there are 14 ops # Previously we were on 11 ops # We didn't pass over a multiple of 5 - @HistoryManager._shouldFlushHistoryOps(14, ['a', 'b', 'c'], 5).should.equal false + @HistoryManager.shouldFlushHistoryOps(14, ['a', 'b', 'c'].length, 5).should.equal false it "should return true if the updates took to the threshold", -> # Currently there are 15 ops # Previously we were on 12 ops # We've reached a new multiple of 5 - @HistoryManager._shouldFlushHistoryOps(15, ['a', 'b', 'c'], 5).should.equal true + @HistoryManager.shouldFlushHistoryOps(15, ['a', 'b', 'c'].length, 5).should.equal true it "should return true if the updates took past the threshold", -> # Currently there are 19 ops # Previously we were on 16 ops # We didn't pass over a multiple of 5 - @HistoryManager._shouldFlushHistoryOps(17, ['a', 'b', 'c'], 5).should.equal true + @HistoryManager.shouldFlushHistoryOps(17, ['a', 'b', 'c'].length, 5).should.equal true