From 4fadd75ef3da82bfb2fffd8eb413a9923bb91bfa Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 2 Dec 2016 11:04:21 +0000 Subject: [PATCH] Track changes based on flag on op, not global setting --- services/document-updater/app.coffee | 1 - .../app/coffee/DocumentManager.coffee | 28 ++---- .../app/coffee/HttpController.coffee | 11 --- .../app/coffee/PersistenceManager.coffee | 7 +- .../app/coffee/RedisManager.coffee | 23 ++--- .../app/coffee/TrackChangesManager.coffee | 6 +- .../app/coffee/UpdateManager.coffee | 4 +- .../coffee/TrackChangesTests.coffee | 88 ++++++------------- .../DocumentManagerTests.coffee | 23 +++-- .../PersistenceManagerTests.coffee | 13 ++- .../RedisManager/RedisManagerTests.coffee | 21 +---- .../UpdateManager/UpdateManagerTests.coffee | 5 +- 12 files changed, 71 insertions(+), 159 deletions(-) diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index f9099660c8..004b9f77bc 100644 --- a/services/document-updater/app.coffee +++ b/services/document-updater/app.coffee @@ -45,7 +45,6 @@ app.post '/project/:project_id/doc/:doc_id/flush', HttpController.flushDocIfLo app.delete '/project/:project_id/doc/:doc_id', HttpController.flushAndDeleteDoc app.delete '/project/:project_id', HttpController.deleteProject app.post '/project/:project_id/flush', HttpController.flushProject -app.post '/project/:project_id/track_changes', HttpController.setTrackChanges app.get '/total', (req, res)-> timer = new Metrics.Timer("http.allDocList") diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 9c7277c469..258d55bb65 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -13,18 +13,18 @@ module.exports = DocumentManager = timer.done() _callback(args...) - RedisManager.getDoc project_id, doc_id, (error, lines, version, track_changes, track_changes_entries) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, track_changes_entries) -> return callback(error) if error? if !lines? or !version? - logger.log {project_id, doc_id, track_changes}, "doc not in redis so getting from persistence API" - PersistenceManager.getDoc project_id, doc_id, (error, lines, version, track_changes, track_changes_entries) -> + logger.log {project_id, doc_id}, "doc not in redis so getting from persistence API" + PersistenceManager.getDoc project_id, doc_id, (error, lines, version, track_changes_entries) -> return callback(error) if error? - logger.log {project_id, doc_id, lines, version, track_changes}, "got doc from persistence API" - RedisManager.putDocInMemory project_id, doc_id, lines, version, track_changes, track_changes_entries, (error) -> + logger.log {project_id, doc_id, lines, version}, "got doc from persistence API" + RedisManager.putDocInMemory project_id, doc_id, lines, version, track_changes_entries, (error) -> return callback(error) if error? - callback null, lines, version, track_changes, track_changes_entries, false + callback null, lines, version, track_changes_entries, false else - callback null, lines, version, track_changes, track_changes_entries, true + callback null, lines, version, track_changes_entries, true getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, recentOps) ->) -> timer = new Metrics.Timer("docManager.getDocAndRecentOps") @@ -90,14 +90,14 @@ module.exports = DocumentManager = callback = (args...) -> timer.done() _callback(args...) - RedisManager.getDoc project_id, doc_id, (error, lines, version, track_changes, track_changes_entries) -> + RedisManager.getDoc project_id, doc_id, (error, lines, version, track_changes_entries) -> return callback(error) if error? if !lines? or !version? logger.log project_id: project_id, doc_id: doc_id, "doc is not loaded so not flushing" callback null # TODO: return a flag to bail out, as we go on to remove doc from memory? else logger.log project_id: project_id, doc_id: doc_id, version: version, "flushing doc" - PersistenceManager.setDoc project_id, doc_id, lines, version, track_changes, track_changes_entries, (error) -> + PersistenceManager.setDoc project_id, doc_id, lines, version, track_changes_entries, (error) -> return callback(error) if error? callback null @@ -119,12 +119,6 @@ module.exports = DocumentManager = RedisManager.removeDocFromMemory project_id, doc_id, (error) -> return callback(error) if error? callback null - - setTrackChanges: (project_id, doc_id, track_changes_on, callback = (error) ->) -> - RedisManager.setTrackChanges project_id, doc_id, track_changes_on, (error) -> - return callback(error) if error? - WebRedisManager.sendData {project_id, doc_id, track_changes_on} - callback() getDocWithLock: (project_id, doc_id, callback = (error, lines, version) ->) -> UpdateManager = require "./UpdateManager" @@ -145,7 +139,3 @@ module.exports = DocumentManager = flushAndDeleteDocWithLock: (project_id, doc_id, callback = (error) ->) -> UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.flushAndDeleteDoc, project_id, doc_id, callback - - setTrackChangesWithLock: (project_id, doc_id, track_changes_on, callback = (error) ->) -> - UpdateManager = require "./UpdateManager" - UpdateManager.lockUpdatesAndDo DocumentManager.setTrackChanges, project_id, doc_id, track_changes_on, callback \ No newline at end of file diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index 0366746d56..dc74833697 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -96,15 +96,4 @@ module.exports = HttpController = return next(error) if error? logger.log project_id: project_id, "deleted project via http" res.send 204 # No Content - - setTrackChanges: (req, res, next = (error) ->) -> - project_id = req.params.project_id - track_changes_on = req.body.on - if !track_changes_on? - return res.send 400 - track_changes_on = !!track_changes_on # Make boolean - logger.log {project_id, track_changes_on}, "turning on track changes via http" - ProjectManager.setTrackChangesWithLocks project_id, track_changes_on, (error) -> - return next(error) if error? - res.send 204 diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee index 25eb1f32a2..ee7674d80a 100644 --- a/services/document-updater/app/coffee/PersistenceManager.coffee +++ b/services/document-updater/app/coffee/PersistenceManager.coffee @@ -10,7 +10,7 @@ logger = require "logger-sharelatex" MAX_HTTP_REQUEST_LENGTH = 5000 # 5 seconds module.exports = PersistenceManager = - getDoc: (project_id, doc_id, _callback = (error, lines, version, track_changes, track_changes_entries) ->) -> + getDoc: (project_id, doc_id, _callback = (error, lines, version, track_changes_entries) ->) -> timer = new Metrics.Timer("persistenceManager.getDoc") callback = (args...) -> timer.done() @@ -39,13 +39,13 @@ module.exports = PersistenceManager = return callback(new Error("web API response had no doc lines")) if !body.version? or not body.version instanceof Number return callback(new Error("web API response had no valid doc version")) - return callback null, body.lines, body.version, body.track_changes, body.track_changes_entries + return callback null, body.lines, body.version, body.track_changes_entries else if res.statusCode == 404 return callback(new Errors.NotFoundError("doc not not found: #{url}")) else return callback(new Error("error accessing web API: #{url} #{res.statusCode}")) - setDoc: (project_id, doc_id, lines, version, track_changes, track_changes_entries, _callback = (error) ->) -> + setDoc: (project_id, doc_id, lines, version, track_changes_entries, _callback = (error) ->) -> timer = new Metrics.Timer("persistenceManager.setDoc") callback = (args...) -> timer.done() @@ -57,7 +57,6 @@ module.exports = PersistenceManager = method: "POST" json: lines: lines - track_changes: track_changes track_changes_entries: track_changes_entries version: version auth: diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 6ee764cb7e..cad5bd9f04 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -13,7 +13,7 @@ minutes = 60 # seconds for Redis expire module.exports = RedisManager = rclient: rclient - putDocInMemory : (project_id, doc_id, docLines, version, track_changes, track_changes_entries, _callback)-> + putDocInMemory : (project_id, doc_id, docLines, version, track_changes_entries, _callback)-> timer = new metrics.Timer("redis.put-doc") callback = (error) -> timer.done() @@ -23,7 +23,6 @@ module.exports = RedisManager = multi.set keys.docLines(doc_id:doc_id), JSON.stringify(docLines) multi.set keys.projectKey({doc_id:doc_id}), project_id multi.set keys.docVersion(doc_id:doc_id), version - multi.set keys.trackChangesEnabled(doc_id:doc_id), if track_changes then "1" else "0" multi.set keys.trackChangesEntries(doc_id:doc_id), JSON.stringify(track_changes_entries) multi.exec (error) -> return callback(error) if error? @@ -43,36 +42,32 @@ module.exports = RedisManager = multi.del keys.docLines(doc_id:doc_id) multi.del keys.projectKey(doc_id:doc_id) multi.del keys.docVersion(doc_id:doc_id) - multi.del keys.trackChangesEnabled(doc_id:doc_id) multi.del keys.trackChangesEntries(doc_id:doc_id) multi.exec (error) -> return callback(error) if error? rclient.srem keys.docsInProject(project_id:project_id), doc_id, callback - getDoc : (project_id, doc_id, callback = (error, lines, version, track_changes, track_changes_entries) ->)-> + getDoc : (project_id, doc_id, callback = (error, lines, version, track_changes_entries) ->)-> timer = new metrics.Timer("redis.get-doc") multi = rclient.multi() multi.get keys.docLines(doc_id:doc_id) multi.get keys.docVersion(doc_id:doc_id) multi.get keys.projectKey(doc_id:doc_id) - multi.get keys.trackChangesEnabled(doc_id:doc_id) multi.get keys.trackChangesEntries(doc_id:doc_id) - multi.exec (error, result)-> + multi.exec (error, [docLines, version, doc_project_id, track_changes_entries])-> timer.done() return callback(error) if error? try - docLines = JSON.parse result[0] - track_changes_entries = JSON.parse result[4] + docLines = JSON.parse docLines + track_changes_entries = JSON.parse track_changes_entries catch e return callback(e) - version = parseInt(result[1] or 0, 10) - doc_project_id = result[2] - track_changes = (result[3] == "1") + version = parseInt(version or 0, 10) # check doc is in requested project if doc_project_id? and doc_project_id isnt project_id logger.error project_id: project_id, doc_id: doc_id, doc_project_id: doc_project_id, "doc not in project" return callback(new Errors.NotFoundError("document not found")) - callback null, docLines, version, track_changes, track_changes_entries + callback null, docLines, version, track_changes_entries getDocVersion: (doc_id, callback = (error, version) ->) -> rclient.get keys.docVersion(doc_id: doc_id), (error, version) -> @@ -134,7 +129,3 @@ module.exports = RedisManager = getDocIdsInProject: (project_id, callback = (error, doc_ids) ->) -> rclient.smembers keys.docsInProject(project_id: project_id), callback - - setTrackChanges: (project_id, doc_id, track_changes_on, callback = (error) ->) -> - value = (if track_changes_on then "1" else "0") - rclient.set keys.trackChangesEnabled({doc_id}), value, callback diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee index 94f8a11ca1..65f1931bb4 100644 --- a/services/document-updater/app/coffee/TrackChangesManager.coffee +++ b/services/document-updater/app/coffee/TrackChangesManager.coffee @@ -1,12 +1,12 @@ ChangesTracker = require "./ChangesTracker" module.exports = TrackChangesManager = - applyUpdate: (project_id, doc_id, entries = {}, updates = [], track_changes, callback = (error, new_entries) ->) -> + applyUpdate: (project_id, doc_id, entries = {}, updates = [], callback = (error, new_entries) ->) -> {changes, comments} = entries changesTracker = new ChangesTracker(changes, comments) - changesTracker.track_changes = track_changes for update in updates + changesTracker.track_changes = !!update.meta.tc for op in update.op - changesTracker.applyOp(op, { user_id: update.meta?.user_id, }) + changesTracker.applyOp(op, { user_id: update.meta?.user_id }) {changes, comments} = changesTracker callback null, {changes, comments} \ No newline at end of file diff --git a/services/document-updater/app/coffee/UpdateManager.coffee b/services/document-updater/app/coffee/UpdateManager.coffee index d08d9a62f3..7c98b97eee 100644 --- a/services/document-updater/app/coffee/UpdateManager.coffee +++ b/services/document-updater/app/coffee/UpdateManager.coffee @@ -48,13 +48,13 @@ module.exports = UpdateManager = applyUpdate: (project_id, doc_id, update, callback = (error) ->) -> UpdateManager._sanitizeUpdate update - DocumentManager.getDoc project_id, doc_id, (error, lines, version, track_changes, track_changes_entries) -> + DocumentManager.getDoc project_id, doc_id, (error, lines, version, track_changes_entries) -> return callback(error) if error? if !lines? or !version? return callback(new Errors.NotFoundError("document not found: #{doc_id}")) ShareJsUpdateManager.applyUpdate project_id, doc_id, update, lines, version, (error, updatedDocLines, version, appliedOps) -> return callback(error) if error? - TrackChangesManager.applyUpdate project_id, doc_id, track_changes_entries, appliedOps, track_changes, (error, new_track_changes_entries) -> + TrackChangesManager.applyUpdate project_id, doc_id, track_changes_entries, appliedOps, (error, new_track_changes_entries) -> return callback(error) if error? logger.log doc_id: doc_id, version: version, "updating doc in redis" RedisManager.updateDocument doc_id, updatedDocLines, version, appliedOps, new_track_changes_entries, (error) -> diff --git a/services/document-updater/test/acceptance/coffee/TrackChangesTests.coffee b/services/document-updater/test/acceptance/coffee/TrackChangesTests.coffee index 406f46b430..43401cef6d 100644 --- a/services/document-updater/test/acceptance/coffee/TrackChangesTests.coffee +++ b/services/document-updater/test/acceptance/coffee/TrackChangesTests.coffee @@ -8,89 +8,51 @@ MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Track changes", -> - describe "turning on track changes", -> - before (done) -> - DocUpdaterClient.subscribeToAppliedOps @appliedOpsListener = sinon.stub() - @project_id = DocUpdaterClient.randomId() - @docs = [{ - id: doc_id0 = DocUpdaterClient.randomId() - lines: ["one", "two", "three"] - updatedLines: ["one", "one and a half", "two", "three"] - }, { - id: doc_id1 = DocUpdaterClient.randomId() - lines: ["four", "five", "six"] - updatedLines: ["four", "four and a half", "five", "six"] - }] - for doc in @docs - MockWebApi.insertDoc @project_id, doc.id, { - lines: doc.lines - version: 0 - } - async.series @docs.map((doc) => - (callback) => - DocUpdaterClient.preloadDoc @project_id, doc.id, callback - ), (error) => - throw error if error? - setTimeout () => - DocUpdaterClient.setTrackChangesOn @project_id, (error, res, body) => - @statusCode = res.statusCode - done() - , 200 - - it "should return a 204 status code", -> - @statusCode.should.equal 204 - - it "should send a track changes message to real-time for each doc", -> - @appliedOpsListener.calledWith("applied-ops", JSON.stringify({ - project_id: @project_id, doc_id: @docs[0].id, track_changes_on: true - })).should.equal true - @appliedOpsListener.calledWith("applied-ops", JSON.stringify({ - project_id: @project_id, doc_id: @docs[1].id, track_changes_on: true - })).should.equal true - - it "should set the track changes key in redis", (done) -> - rclient.get "TrackChangesEnabled:#{@docs[0].id}", (error, value) => - throw error if error? - value.should.equal "1" - rclient.get "TrackChangesEnabled:#{@docs[1].id}", (error, value) -> - throw error if error? - value.should.equal "1" - done() - describe "tracking changes", -> before (done) -> @project_id = DocUpdaterClient.randomId() + @user_id = DocUpdaterClient.randomId() @doc = { - id: doc_id0 = DocUpdaterClient.randomId() - lines: ["one", "two", "three"] + id: DocUpdaterClient.randomId() + lines: ["aaa"] } - @update = + @updates = [{ doc: @doc.id - op: [{ - i: "one and a half\n" - p: 4 - }] + op: [{ i: "123", p: 1 }] v: 0 - meta: - user_id: @user_id = DocUpdaterClient.randomId() + meta: { user_id: @user_id } + }, { + doc: @doc.id + op: [{ i: "456", p: 5 }] + v: 1 + meta: { user_id: @user_id, tc: 1 } + }, { + doc: @doc.id + op: [{ d: "12", p: 1 }] + v: 2 + meta: { user_id: @user_id } + }] MockWebApi.insertDoc @project_id, @doc.id, { lines: @doc.lines version: 0 } + jobs = [] + for update in @updates + do (update) => + jobs.push (callback) => DocUpdaterClient.sendUpdate @project_id, @doc.id, update, callback DocUpdaterClient.preloadDoc @project_id, @doc.id, (error) => throw error if error? - DocUpdaterClient.setTrackChangesOn @project_id, (error, res, body) => + async.series jobs, (error) -> throw error if error? - DocUpdaterClient.sendUpdate @project_id, @doc.id, @update, (error) -> - throw error if error? - setTimeout done, 200 + setTimeout done, 200 it "should set the updated track changes entries in redis", (done) -> + console.log "TODO: GET ME FROM HTTP REQUEST" rclient.get "TrackChangesEntries:#{@doc.id}", (error, value) => throw error if error? entries = JSON.parse(value) change = entries.changes[0] - change.op.should.deep.equal @update.op[0] + change.op.should.deep.equal { i: "456", p: 3 } change.metadata.user_id.should.equal @user_id done() diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index 2b0dce169b..d29a569d46 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -24,7 +24,6 @@ describe "DocumentManager", -> @lines = ["one", "two", "three"] @version = 42 @track_changes_entries = { comments: "mock", entries: "mock" } - @track_changes_on = true describe "flushAndDeleteDoc", -> describe "successfully", -> @@ -58,7 +57,7 @@ describe "DocumentManager", -> describe "flushDocIfLoaded", -> describe "when the doc is in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_on, @track_changes_entries) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_entries) @PersistenceManager.setDoc = sinon.stub().yields() @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback @@ -69,7 +68,7 @@ describe "DocumentManager", -> it "should write the doc lines to the persistence layer", -> @PersistenceManager.setDoc - .calledWith(@project_id, @doc_id, @lines, @version, @track_changes_on, @track_changes_entries) + .calledWith(@project_id, @doc_id, @lines, @version, @track_changes_entries) .should.equal true it "should call the callback without error", -> @@ -80,7 +79,7 @@ describe "DocumentManager", -> describe "when the document is not in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null) @PersistenceManager.setDoc = sinon.stub().yields() @DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2) @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback @@ -103,7 +102,7 @@ describe "DocumentManager", -> describe "getDocAndRecentOps", -> describe "with a previous version specified", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_on, @track_changes_entries) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_entries) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, @fromVersion, @callback @@ -125,7 +124,7 @@ describe "DocumentManager", -> describe "with no previous version specified", -> beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_on, @track_changes_entries) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_entries) @RedisManager.getPreviousDocOps = sinon.stub().callsArgWith(3, null, @ops) @DocumentManager.getDocAndRecentOps @project_id, @doc_id, -1, @callback @@ -146,7 +145,7 @@ describe "DocumentManager", -> describe "getDoc", -> describe "when the doc exists in Redis", -> beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_on, @track_changes_entries) + @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_entries) @DocumentManager.getDoc @project_id, @doc_id, @callback it "should get the doc from Redis", -> @@ -155,7 +154,7 @@ describe "DocumentManager", -> .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @track_changes_on, @track_changes_entries, true).should.equal true + @callback.calledWith(null, @lines, @version, @track_changes_entries, true).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -163,7 +162,7 @@ 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, @track_changes_on, @track_changes_entries) + @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @track_changes_entries) @RedisManager.putDocInMemory = sinon.stub().yields() @DocumentManager.getDoc @project_id, @doc_id, @callback @@ -179,11 +178,11 @@ describe "DocumentManager", -> it "should set the doc in Redis", -> @RedisManager.putDocInMemory - .calledWith(@project_id, @doc_id, @lines, @version, @track_changes_on, @track_changes_entries) + .calledWith(@project_id, @doc_id, @lines, @version, @track_changes_entries) .should.equal true it "should call the callback with the doc info", -> - @callback.calledWith(null, @lines, @version, @track_changes_on, @track_changes_entries, false).should.equal true + @callback.calledWith(null, @lines, @version, @track_changes_entries, false).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -193,7 +192,7 @@ describe "DocumentManager", -> beforeEach -> @beforeLines = ["before", "lines"] @afterLines = ["after", "lines"] - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @track_changes_on, @track_changes_entries, true) + @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, @track_changes_entries, true) @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee index e9f2cf212e..35c276a4f2 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/PersistenceManagerTests.coffee @@ -19,7 +19,6 @@ describe "PersistenceManager", -> @lines = ["one", "two", "three"] @version = 42 @callback = sinon.stub() - @track_changes_on = true @track_changes_entries = { comments: "mock", entries: "mock" } @Settings.apis = web: @@ -34,7 +33,6 @@ describe "PersistenceManager", -> @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify({ lines: @lines, version: @version, - track_changes: @track_changes_on, track_changes_entries: @track_changes_entries })) @PersistenceManager.getDoc(@project_id, @doc_id, @callback) @@ -56,7 +54,7 @@ describe "PersistenceManager", -> .should.equal true it "should call the callback with the doc lines, version and track changes state", -> - @callback.calledWith(null, @lines, @version, @track_changes_on, @track_changes_entries).should.equal true + @callback.calledWith(null, @lines, @version, @track_changes_entries).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true @@ -114,7 +112,7 @@ describe "PersistenceManager", -> describe "with a successful response from the web api", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 200}) - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_on, @track_changes_entries, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_entries, @callback) it "should call the web api", -> @request @@ -123,7 +121,6 @@ describe "PersistenceManager", -> json: lines: @lines version: @version - track_changes: @track_changes_on track_changes_entries: @track_changes_entries method: "POST" auth: @@ -144,7 +141,7 @@ describe "PersistenceManager", -> describe "when request returns an error", -> beforeEach -> @request.callsArgWith(1, @error = new Error("oops"), null, null) - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_on, @track_changes_entries, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_entries, @callback) it "should return the error", -> @callback.calledWith(@error).should.equal true @@ -155,7 +152,7 @@ describe "PersistenceManager", -> describe "when the request returns 404", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 404}, "") - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_on, @track_changes_entries, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_entries, @callback) it "should return a NotFoundError", -> @callback.calledWith(new Errors.NotFoundError("not found")).should.equal true @@ -166,7 +163,7 @@ describe "PersistenceManager", -> describe "when the request returns an error status code", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 500}, "") - @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_on, @track_changes_entries, @callback) + @PersistenceManager.setDoc(@project_id, @doc_id, @lines, @version, @track_changes_entries, @callback) it "should return an error", -> @callback.calledWith(new Error("web api error")).should.equal true diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index 6f9afc29d3..901af153c1 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -22,7 +22,6 @@ describe "RedisManager", -> projectKey: ({doc_id}) -> "ProjectId:#{doc_id}" pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}" docsInProject: ({project_id}) -> "DocsIn:#{project_id}" - trackChangesEnabled: ({doc_id}) -> "TrackChangesEnabled:#{doc_id}" trackChangesEntries: ({doc_id}) -> "TrackChangesEntries:#{doc_id}" "logger-sharelatex": @logger = { error: sinon.stub(), log: sinon.stub(), warn: sinon.stub() } "./Metrics": @metrics = @@ -40,11 +39,10 @@ describe "RedisManager", -> @jsonlines = JSON.stringify @lines @version = 42 @track_changes_on = true - @redis_track_changes_on = "1" @track_changes_entries = { comments: "mock", entries: "mock" } @json_track_changes_entries = JSON.stringify @track_changes_entries @rclient.get = sinon.stub() - @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @project_id, @redis_track_changes_on, @json_track_changes_entries]) + @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @project_id, @json_track_changes_entries]) describe "successfully", -> beforeEach -> @@ -60,11 +58,6 @@ describe "RedisManager", -> .calledWith("DocVersion:#{@doc_id}") .should.equal true - it "should get the track changes state", -> - @rclient.get - .calledWith("TrackChangesEnabled:#{@doc_id}") - .should.equal true - it "should get the track changes entries", -> @rclient.get .calledWith("TrackChangesEntries:#{@doc_id}") @@ -72,13 +65,13 @@ describe "RedisManager", -> it 'should return the document', -> @callback - .calledWith(null, @lines, @version, @track_changes_on, @track_changes_entries) + .calledWith(null, @lines, @version, @track_changes_entries) .should.equal true describe "getDoc with an invalid project id", -> beforeEach -> @another_project_id = "project-id-456" - @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @another_project_id, @redis_track_changes_on, @json_track_changes_entries]) + @rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @another_project_id, @json_track_changes_entries]) @RedisManager.getDoc @project_id, @doc_id, @callback it 'should return an error', -> @@ -263,9 +256,8 @@ describe "RedisManager", -> @rclient.exec.yields() @lines = ["one", "two", "three"] @version = 42 - @track_changes_on = true @track_changes_entries = { comments: "mock", entries: "mock" } - @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @track_changes_on, @track_changes_entries, done + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @track_changes_entries, done it "should set the lines", -> @rclient.set @@ -282,11 +274,6 @@ describe "RedisManager", -> .calledWith("TrackChangesEntries:#{@doc_id}", JSON.stringify(@track_changes_entries)) .should.equal true - it "should set the track changes state", -> - @rclient.set - .calledWith("TrackChangesEnabled:#{@doc_id}", "1") - .should.equal true - it "should set the project_id for the doc", -> @rclient.set .calledWith("ProjectId:#{@doc_id}", @project_id) diff --git a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee index 9969e42d61..fb9bc18eb1 100644 --- a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee @@ -158,11 +158,10 @@ describe "UpdateManager", -> @updatedDocLines = ["updated", "lines"] @version = 34 @lines = ["original", "lines"] - @track_changes_on = true @track_changes_entries = { entries: "mock", comments: "mock" } @updated_track_changes_entries = { entries: "updated", comments: "updated" } @appliedOps = ["mock-applied-ops"] - @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @track_changes_on, @track_changes_entries) + @DocumentManager.getDoc = sinon.stub().yields(null, @lines, @version, @track_changes_entries) @TrackChangesManager.applyUpdate = sinon.stub().yields(null, @updated_track_changes_entries) @ShareJsUpdateManager.applyUpdate = sinon.stub().yields(null, @updatedDocLines, @version, @appliedOps) @RedisManager.updateDocument = sinon.stub().yields() @@ -179,7 +178,7 @@ describe "UpdateManager", -> it "should update the track changes entries", -> @TrackChangesManager.applyUpdate - .calledWith(@project_id, @doc_id, @track_changes_entries, @appliedOps, @track_changes_on) + .calledWith(@project_id, @doc_id, @track_changes_entries, @appliedOps) .should.equal true it "should save the document", ->