From ffd10a8439df6d5bfd5cc77197004aa7cec56a94 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 15 May 2014 11:13:16 +0100 Subject: [PATCH] Don't send or get versioning info to/from web The version number is only used by the doc updater and so can be cleanly encapsulated in a collection that only the doc updater knows about. The version is already stored under docOps, so continue to tore it there. --- .../app/coffee/PersistenceManager.coffee | 26 +-- .../coffee/ApplyingUpdatesToADocTests.coffee | 188 ++++++------------ .../coffee/DeletingADocumentTests.coffee | 37 ++-- .../coffee/FlushingDocsTests.coffee | 34 +--- .../coffee/GettingADocumentTests.coffee | 36 ++-- .../coffee/SettingADocumentTests.coffee | 21 +- .../coffee/helpers/MockWebApi.coffee | 14 +- .../getDocFromWebTests.coffee | 7 +- .../PersistenceManager/getDocTests.coffee | 46 +---- .../getDocVersionInMongoTests.coffee | 4 +- .../setDocInWebTests.coffee | 10 +- .../PersistenceManager/setDocTests.coffee | 5 +- 12 files changed, 155 insertions(+), 273 deletions(-) diff --git a/services/document-updater/app/coffee/PersistenceManager.coffee b/services/document-updater/app/coffee/PersistenceManager.coffee index bd119234fc..089700f23d 100644 --- a/services/document-updater/app/coffee/PersistenceManager.coffee +++ b/services/document-updater/app/coffee/PersistenceManager.coffee @@ -7,31 +7,20 @@ logger = require "logger-sharelatex" module.exports = PersistenceManager = getDoc: (project_id, doc_id, callback = (error, lines, version) ->) -> - PersistenceManager.getDocFromWeb project_id, doc_id, (error, lines, webVersion) -> + PersistenceManager.getDocFromWeb project_id, doc_id, (error, lines) -> return callback(error) if error? - PersistenceManager.getDocVersionInMongo doc_id, (error, mongoVersion) -> + PersistenceManager.getDocVersionInMongo doc_id, (error, version) -> return callback(error) if error? - if !webVersion? and !mongoVersion? - version = 0 - else if !webVersion? - logger.warn project_id: project_id, doc_id: doc_id, "loading doc version from mongo - deprecated" - version = mongoVersion - else if !mongoVersion? - version = webVersion - else if webVersion > mongoVersion - version = webVersion - else - version = mongoVersion callback null, lines, version setDoc: (project_id, doc_id, lines, version, callback = (error) ->) -> - PersistenceManager.setDocInWeb project_id, doc_id, lines, version, (error) -> + PersistenceManager.setDocInWeb project_id, doc_id, lines, (error) -> return callback(error) if error? PersistenceManager.setDocVersionInMongo doc_id, version, (error) -> return callback(error) if error? callback() - getDocFromWeb: (project_id, doc_id, _callback = (error, lines, version) ->) -> + getDocFromWeb: (project_id, doc_id, _callback = (error, lines) ->) -> timer = new Metrics.Timer("persistenceManager.getDoc") callback = (args...) -> timer.done() @@ -55,13 +44,13 @@ module.exports = PersistenceManager = body = JSON.parse body catch e return callback(e) - return callback null, body.lines, body.version + return callback null, body.lines 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}")) - setDocInWeb: (project_id, doc_id, lines, version, _callback = (error) ->) -> + setDocInWeb: (project_id, doc_id, lines, _callback = (error) ->) -> timer = new Metrics.Timer("persistenceManager.setDoc") callback = (args...) -> timer.done() @@ -73,7 +62,6 @@ module.exports = PersistenceManager = method: "POST" body: JSON.stringify lines: lines - version: parseInt(version, 10) headers: "content-type": "application/json" auth: @@ -98,7 +86,7 @@ module.exports = PersistenceManager = }, (error, docs) -> return callback(error) if error? if docs.length < 1 or !docs[0].version? - return callback null, null + return callback null, 0 else return callback null, docs[0].version diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index f312dbbd9e..1e9c2e2689 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -25,14 +25,17 @@ describe "Applying updates to a doc", -> describe "when the document is not loaded", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - version: @version - } sinon.spy MockWebApi, "getDocument" - DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> + + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert { + doc_id: ObjectId(@doc_id) + version: @version + }, (error) => throw error if error? - setTimeout done, 200 + DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> + throw error if error? + setTimeout done, 200 after -> MockWebApi.getDocument.restore() @@ -60,16 +63,16 @@ describe "Applying updates to a doc", -> describe "when the document is loaded", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - version: @version - } - DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => + + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert doc_id: ObjectId(@doc_id), version: @version, (error) => throw error if error? - sinon.spy MockWebApi, "getDocument" - DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> + DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => throw error if error? - setTimeout done, 200 + sinon.spy MockWebApi, "getDocument" + DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> + throw error if error? + setTimeout done, 200 after -> MockWebApi.getDocument.restore() @@ -91,28 +94,27 @@ describe "Applying updates to a doc", -> describe "when the document has been deleted", -> describe "when the ops come in a single linear order", -> - before -> + before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] lines = ["", "", ""] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: lines - version: 0 - } - - @updates = [ - { doc_id: @doc_id, v: 0, op: [i: "h", p: 0 ] } - { doc_id: @doc_id, v: 1, op: [i: "e", p: 1 ] } - { doc_id: @doc_id, v: 2, op: [i: "l", p: 2 ] } - { doc_id: @doc_id, v: 3, op: [i: "l", p: 3 ] } - { doc_id: @doc_id, v: 4, op: [i: "o", p: 4 ] } - { doc_id: @doc_id, v: 5, op: [i: " ", p: 5 ] } - { doc_id: @doc_id, v: 6, op: [i: "w", p: 6 ] } - { doc_id: @doc_id, v: 7, op: [i: "o", p: 7 ] } - { doc_id: @doc_id, v: 8, op: [i: "r", p: 8 ] } - { doc_id: @doc_id, v: 9, op: [i: "l", p: 9 ] } - { doc_id: @doc_id, v: 10, op: [i: "d", p: 10] } - ] - @my_result = ["hello world", "", ""] + MockWebApi.insertDoc @project_id, @doc_id, lines: lines + db.docOps.insert doc_id: ObjectId(@doc_id), version: 0, (error) => + throw error if error? + @updates = [ + { doc_id: @doc_id, v: 0, op: [i: "h", p: 0 ] } + { doc_id: @doc_id, v: 1, op: [i: "e", p: 1 ] } + { doc_id: @doc_id, v: 2, op: [i: "l", p: 2 ] } + { doc_id: @doc_id, v: 3, op: [i: "l", p: 3 ] } + { doc_id: @doc_id, v: 4, op: [i: "o", p: 4 ] } + { doc_id: @doc_id, v: 5, op: [i: " ", p: 5 ] } + { doc_id: @doc_id, v: 6, op: [i: "w", p: 6 ] } + { doc_id: @doc_id, v: 7, op: [i: "o", p: 7 ] } + { doc_id: @doc_id, v: 8, op: [i: "r", p: 8 ] } + { doc_id: @doc_id, v: 9, op: [i: "l", p: 9 ] } + { doc_id: @doc_id, v: 10, op: [i: "d", p: 10] } + ] + @my_result = ["hello world", "", ""] + done() it "should be able to continue applying updates when the project has been deleted", (done) -> actions = [] @@ -141,23 +143,24 @@ describe "Applying updates to a doc", -> done() describe "when older ops come in after the delete", -> - before -> + before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] lines = ["", "", ""] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: lines - version: 0 - } + MockWebApi.insertDoc @project_id, @doc_id, lines: lines + db.docOps.insert doc_id: ObjectId(@doc_id), version: 0, (error) => + throw error if error? - @updates = [ - { doc_id: @doc_id, v: 0, op: [i: "h", p: 0 ] } - { doc_id: @doc_id, v: 1, op: [i: "e", p: 1 ] } - { doc_id: @doc_id, v: 2, op: [i: "l", p: 2 ] } - { doc_id: @doc_id, v: 3, op: [i: "l", p: 3 ] } - { doc_id: @doc_id, v: 4, op: [i: "o", p: 4 ] } - { doc_id: @doc_id, v: 0, op: [i: "world", p: 1 ] } - ] - @my_result = ["hello", "world", ""] + @updates = [ + { doc_id: @doc_id, v: 0, op: [i: "h", p: 0 ] } + { doc_id: @doc_id, v: 1, op: [i: "e", p: 1 ] } + { doc_id: @doc_id, v: 2, op: [i: "l", p: 2 ] } + { doc_id: @doc_id, v: 3, op: [i: "l", p: 3 ] } + { doc_id: @doc_id, v: 4, op: [i: "o", p: 4 ] } + { doc_id: @doc_id, v: 0, op: [i: "world", p: 1 ] } + ] + @my_result = ["hello", "world", ""] + + done() it "should be able to continue applying updates when the project has been deleted", (done) -> actions = [] @@ -178,13 +181,12 @@ describe "Applying updates to a doc", -> describe "with a broken update", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - version: @version - } - DocUpdaterClient.sendUpdate @project_id, @doc_id, @undefined, (error) -> + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert doc_id: ObjectId(@doc_id), version: @version, (error) => throw error if error? - setTimeout done, 200 + DocUpdaterClient.sendUpdate @project_id, @doc_id, @undefined, (error) -> + throw error if error? + setTimeout done, 200 it "should not update the doc", (done) -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => @@ -194,10 +196,6 @@ describe "Applying updates to a doc", -> describe "with enough updates to flush to the track changes api", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - version: 0 - } updates = [] for v in [0..99] # Should flush after 50 ops updates.push @@ -207,9 +205,12 @@ describe "Applying updates to a doc", -> sinon.spy MockTrackChangesApi, "flushDoc" - DocUpdaterClient.sendUpdates @project_id, @doc_id, updates, (error) => + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert doc_id: ObjectId(@doc_id), version: 0, (error) => throw error if error? - setTimeout done, 200 + DocUpdaterClient.sendUpdates @project_id, @doc_id, updates, (error) => + throw error if error? + setTimeout done, 200 after -> MockTrackChangesApi.flushDoc.restore() @@ -217,72 +218,7 @@ describe "Applying updates to a doc", -> it "should flush the doc twice", -> MockTrackChangesApi.flushDoc.calledTwice.should.equal true - describe "when the document does not have a version in the web api but does in Mongo", -> - before (done) -> - [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - } - - db.docOps.insert { - doc_id: ObjectId(@doc_id) - version: @version - }, (error) => - throw error if error? - DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> - throw error if error? - setTimeout done, 200 - - it "should update the doc (using the mongo version)", (done) -> - DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => - doc.lines.should.deep.equal @result - done() - - describe "when the document version in the web api is ahead of Mongo", -> - before (done) -> - [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - version: @version - } - - db.docOps.insert { - doc_id: ObjectId(@doc_id) - version: @version - 20 - }, (error) => - throw error if error? - DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> - throw error if error? - setTimeout done, 200 - - it "should update the doc (using the web version)", (done) -> - DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => - doc.lines.should.deep.equal @result - done() - - describe "when the document version in Mongo is ahead of the web api", -> - before (done) -> - [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - version: @version - 20 - } - - db.docOps.insert { - doc_id: ObjectId(@doc_id) - version: @version - }, (error) => - throw error if error? - DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> - throw error if error? - setTimeout done, 200 - - it "should update the doc (using the web version)", (done) -> - DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => - doc.lines.should.deep.equal @result - done() - - describe "when there is no version yet", -> + describe "when there is no version in Mongo", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { diff --git a/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee index 171dfcc6e2..139ba9bbed 100644 --- a/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/DeletingADocumentTests.coffee @@ -1,6 +1,7 @@ sinon = require "sinon" chai = require("chai") chai.should() +{db, ObjectId} = require "../../../app/js/mongojs" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -21,21 +22,24 @@ describe "Deleting a document", -> describe "when the updated doc exists in the doc updater", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - version: @version - } sinon.spy MockWebApi, "setDocumentLines" sinon.spy MockWebApi, "getDocument" - DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => + + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert { + doc_id: ObjectId(@doc_id) + version: @version + }, (error) => throw error if error? - DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) => + DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => throw error if error? - setTimeout () => - DocUpdaterClient.deleteDoc @project_id, @doc_id, (error, res, body) => - @statusCode = res.statusCode - done() - , 200 + DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) => + throw error if error? + setTimeout () => + DocUpdaterClient.deleteDoc @project_id, @doc_id, (error, res, body) => + @statusCode = res.statusCode + done() + , 200 after -> MockWebApi.setDocumentLines.restore() @@ -49,6 +53,16 @@ describe "Deleting a document", -> .calledWith(@project_id, @doc_id, @result) .should.equal true + it "should write the version to mongo", (done) -> + db.docOps.find { + doc_id: ObjectId(@doc_id) + }, { + version: 1 + }, (error, docs) => + doc = docs[0] + doc.version.should.equal @version + 1 + done() + it "should need to reload the doc if read again", (done) -> MockWebApi.getDocument.called.should.equal.false DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => @@ -62,7 +76,6 @@ describe "Deleting a document", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines - version: @version } sinon.spy MockWebApi, "setDocumentLines" sinon.spy MockWebApi, "getDocument" diff --git a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee index 4f4f2f04e2..4513fd7d5c 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee @@ -19,41 +19,32 @@ describe "Flushing a doc to Mongo", -> }] v: @version @result = ["one", "one and a half", "two", "three"] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - version: @version - } describe "when the updated doc exists in the doc updater", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines - version: @version - } sinon.spy MockWebApi, "setDocumentLines" - sinon.spy MockWebApi, "setDocumentVersion" - DocUpdaterClient.sendUpdates @project_id, @doc_id, [@update], (error) => + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert { + doc_id: ObjectId(@doc_id) + version: @version + }, (error) => throw error if error? - setTimeout () => - DocUpdaterClient.flushDoc @project_id, @doc_id, done - , 200 + DocUpdaterClient.sendUpdates @project_id, @doc_id, [@update], (error) => + throw error if error? + setTimeout () => + DocUpdaterClient.flushDoc @project_id, @doc_id, done + , 200 after -> MockWebApi.setDocumentLines.restore() - MockWebApi.setDocumentVersion.restore() it "should flush the updated doc lines to the web api", -> MockWebApi.setDocumentLines .calledWith(@project_id, @doc_id, @result) .should.equal true - it "should flush the updated doc version to the web api", -> - MockWebApi.setDocumentVersion - .calledWith(@project_id, @doc_id, @version + 1) - .should.equal true - it "should store the updated doc version into mongo", (done) -> db.docOps.find { doc_id: ObjectId(@doc_id) @@ -70,18 +61,13 @@ describe "Flushing a doc to Mongo", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines - version: @version } sinon.spy MockWebApi, "setDocumentLines" - sinon.spy MockWebApi, "setDocumentVersion" DocUpdaterClient.flushDoc @project_id, @doc_id, done after -> MockWebApi.setDocumentLines.restore() - MockWebApi.setDocumentVersion.restore() it "should not flush the doc to the web api", -> MockWebApi.setDocumentLines.called.should.equal false - MockWebApi.setDocumentVersion.called.should.equal false - diff --git a/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee index 43c039a802..980d73fa93 100644 --- a/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/GettingADocumentTests.coffee @@ -1,20 +1,28 @@ sinon = require "sinon" chai = require("chai") chai.should() +{db, ObjectId} = require "../../../app/js/mongojs" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Getting a document", -> + beforeEach -> + @lines = ["one", "two", "three"] + @version = 42 + describe "when the document is not loaded", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines = ["one", "two", "three"] - version: @version = 42 - } sinon.spy MockWebApi, "getDocument" - DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, @returnedDoc) => done() + + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert { + doc_id: ObjectId(@doc_id) + version: @version + }, (error) => + throw error if error? + DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, @returnedDoc) => done() after -> MockWebApi.getDocument.restore() @@ -33,14 +41,17 @@ describe "Getting a document", -> describe "when the document is already loaded", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines = ["one", "two", "three"] - } - - DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => + + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert { + doc_id: ObjectId(@doc_id) + version: @version + }, (error) => throw error if error? - sinon.spy MockWebApi, "getDocument" - DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, @returnedDoc) => done() + DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => + throw error if error? + sinon.spy MockWebApi, "getDocument" + DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, @returnedDoc) => done() after -> MockWebApi.getDocument.restore() @@ -56,7 +67,6 @@ describe "Getting a document", -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines = ["one", "two", "three"] - version: 0 } @updates = for v in [0..99] diff --git a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee index 143fbc868e..5dfc5d95f0 100644 --- a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee @@ -1,12 +1,13 @@ sinon = require "sinon" chai = require("chai") chai.should() +{db, ObjectId} = require "../../../app/js/mongojs" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Setting a document", -> - before -> + before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @lines = ["one", "two", "three"] @version = 42 @@ -21,15 +22,19 @@ describe "Setting a document", -> @newLines = ["these", "are", "the", "new", "lines"] @source = "dropbox" @user_id = "user-id-123" - MockWebApi.insertDoc @project_id, @doc_id, { - lines: @lines + + MockWebApi.insertDoc @project_id, @doc_id, lines: @lines + db.docOps.insert { + doc_id: ObjectId(@doc_id) version: @version - } + }, (error) => + throw error if error? + done() + describe "when the updated doc exists in the doc updater", -> before (done) -> sinon.spy MockWebApi, "setDocumentLines" - sinon.spy MockWebApi, "setDocumentVersion" DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) => throw error if error? DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) => @@ -42,7 +47,6 @@ describe "Setting a document", -> after -> MockWebApi.setDocumentLines.restore() - MockWebApi.setDocumentVersion.restore() it "should return a 204 status code", -> @statusCode.should.equal 204 @@ -52,11 +56,6 @@ describe "Setting a document", -> .calledWith(@project_id, @doc_id, @newLines) .should.equal true - it "should send the updated doc version to the web api", -> - MockWebApi.setDocumentVersion - .calledWith(@project_id, @doc_id, @version + 2) - .should.equal true - it "should update the lines in the doc updater", (done) -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @newLines diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee index 7c7dd1e211..7bab5b9b9f 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -14,11 +14,6 @@ module.exports = MockWebApi = @docs["#{project_id}:#{doc_id}"].lines = lines callback null - setDocumentVersion: (project_id, doc_id, version, callback = (error) ->) -> - @docs["#{project_id}:#{doc_id}"] ||= {} - @docs["#{project_id}:#{doc_id}"].version = version - callback null - getDocument: (project_id, doc_id, callback = (error, doc) ->) -> callback null, @docs["#{project_id}:#{doc_id}"] @@ -34,11 +29,10 @@ module.exports = MockWebApi = app.post "/project/:project_id/doc/:doc_id", express.bodyParser(), (req, res, next) => MockWebApi.setDocumentLines req.params.project_id, req.params.doc_id, req.body.lines, (error) -> - MockWebApi.setDocumentVersion req.params.project_id, req.params.doc_id, req.body.version, (error) -> - if error1? or error2? - res.send 500 - else - res.send 204 + if error? + res.send 500 + else + res.send 204 app.listen 3000, (error) -> throw error if error? diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/getDocFromWebTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/getDocFromWebTests.coffee index b2aebdd84d..82ee937591 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/getDocFromWebTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/getDocFromWebTests.coffee @@ -16,7 +16,6 @@ describe "PersistenceManager.getDocFromWeb", -> @project_id = "project-id-123" @doc_id = "doc-id-123" @lines = ["one", "two", "three"] - @version = 42 @callback = sinon.stub() @Settings.apis = web: @@ -26,7 +25,7 @@ describe "PersistenceManager.getDocFromWeb", -> describe "with a successful response from the web api", -> beforeEach -> - @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines, version: @version)) + @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines)) @PersistenceManager.getDocFromWeb(@project_id, @doc_id, @callback) it "should call the web api", -> @@ -44,8 +43,8 @@ describe "PersistenceManager.getDocFromWeb", -> }) .should.equal true - it "should call the callback with the doc lines and version", -> - @callback.calledWith(null, @lines, @version).should.equal true + it "should call the callback with the doc lines", -> + @callback.calledWith(null, @lines).should.equal true it "should time the execution", -> @Metrics.Timer::done.called.should.equal true diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee index e9088fac30..ae3e476ec4 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/getDocTests.coffee @@ -24,10 +24,10 @@ describe "PersistenceManager.getDoc", -> @lines = ["mock", "doc", "lines"] @version = 42 - describe "when the version is set in the web api but not Mongo", -> + describe "successfully", -> beforeEach -> - @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, @version) - @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, null) + @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines) + @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version) @PersistenceManager.getDoc @project_id, @doc_id, @callback it "should look up the doc in the web api", -> @@ -43,44 +43,4 @@ describe "PersistenceManager.getDoc", -> it "should call the callback with the lines and version", -> @callback.calledWith(null, @lines, @version).should.equal true - describe "when the version is not set in the web api, but is in Mongo", -> - beforeEach -> - @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, null) - @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version) - @PersistenceManager.getDoc @project_id, @doc_id, @callback - - it "shoud log a warning", -> - @logger.warn - .calledWith(project_id: @project_id, doc_id: @doc_id, "loading doc version from mongo - deprecated") - .should.equal true - - it "should call the callback with the lines and version", -> - @callback.calledWith(null, @lines, @version).should.equal true - - describe "when the version in the web api is ahead of Mongo", -> - beforeEach -> - @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, @version) - @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version - 20) - @PersistenceManager.getDoc @project_id, @doc_id, @callback - - it "should call the callback with the web version", -> - @callback.calledWith(null, @lines, @version).should.equal true - - describe "when the version in the web api is behind Mongo", -> - beforeEach -> - @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, @version - 20) - @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, @version) - @PersistenceManager.getDoc @project_id, @doc_id, @callback - - it "should call the callback with the Mongo version", -> - @callback.calledWith(null, @lines, @version).should.equal true - - describe "when the version is not set", -> - beforeEach -> - @PersistenceManager.getDocFromWeb = sinon.stub().callsArgWith(2, null, @lines, null) - @PersistenceManager.getDocVersionInMongo = sinon.stub().callsArgWith(1, null, null) - @PersistenceManager.getDoc @project_id, @doc_id, @callback - - it "should call the callback with the lines and version = 0", -> - @callback.calledWith(null, @lines, 0).should.equal true diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/getDocVersionInMongoTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/getDocVersionInMongoTests.coffee index bbe6c43c48..a5015279fe 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/getDocVersionInMongoTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/getDocVersionInMongoTests.coffee @@ -42,5 +42,5 @@ describe "PersistenceManager.getDocVersionInMongo", -> @db.docOps.find = sinon.stub().callsArgWith(2, null, []) @PersistenceManager.getDocVersionInMongo @doc_id, @callback - it "should call the callback with null", -> - @callback.calledWith(null, null).should.equal true \ No newline at end of file + it "should call the callback with 0", -> + @callback.calledWith(null, 0).should.equal true \ No newline at end of file diff --git a/services/document-updater/test/unit/coffee/PersistenceManager/setDocInWebTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/setDocInWebTests.coffee index 915e72affe..ad218caa10 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/setDocInWebTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/setDocInWebTests.coffee @@ -16,7 +16,6 @@ describe "PersistenceManager.setDocInWeb", -> @project_id = "project-id-123" @doc_id = "doc-id-123" @lines = ["one", "two", "three"] - @version = 42 @callback = sinon.stub() @Settings.apis = web: @@ -27,7 +26,7 @@ describe "PersistenceManager.setDocInWeb", -> describe "with a successful response from the web api", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 200}, JSON.stringify(lines: @lines, version: @version)) - @PersistenceManager.setDocInWeb(@project_id, @doc_id, @lines, @version, @callback) + @PersistenceManager.setDocInWeb(@project_id, @doc_id, @lines, @callback) it "should call the web api", -> @request @@ -35,7 +34,6 @@ describe "PersistenceManager.setDocInWeb", -> url: "#{@url}/project/#{@project_id}/doc/#{@doc_id}" body: JSON.stringify lines: @lines - version: @version method: "POST" headers: "content-type": "application/json" @@ -56,7 +54,7 @@ describe "PersistenceManager.setDocInWeb", -> describe "when request returns an error", -> beforeEach -> @request.callsArgWith(1, @error = new Error("oops"), null, null) - @PersistenceManager.setDocInWeb(@project_id, @doc_id, @lines, @version, @callback) + @PersistenceManager.setDocInWeb(@project_id, @doc_id, @lines, @callback) it "should return the error", -> @callback.calledWith(@error).should.equal true @@ -67,7 +65,7 @@ describe "PersistenceManager.setDocInWeb", -> describe "when the request returns 404", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 404}, "") - @PersistenceManager.setDocInWeb(@project_id, @doc_id, @lines, @version, @callback) + @PersistenceManager.setDocInWeb(@project_id, @doc_id, @lines, @callback) it "should return a NotFoundError", -> @callback.calledWith(new Errors.NotFoundError("not found")).should.equal true @@ -78,7 +76,7 @@ describe "PersistenceManager.setDocInWeb", -> describe "when the request returns an error status code", -> beforeEach -> @request.callsArgWith(1, null, {statusCode: 500}, "") - @PersistenceManager.setDocInWeb(@project_id, @doc_id, @lines, @version, @callback) + @PersistenceManager.setDocInWeb(@project_id, @doc_id, @lines, @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/PersistenceManager/setDocTests.coffee b/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee index 7c8cacd095..80c0a5e18f 100644 --- a/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee +++ b/services/document-updater/test/unit/coffee/PersistenceManager/setDocTests.coffee @@ -18,16 +18,15 @@ describe "PersistenceManager.setDoc", -> @doc_id = "mock-doc-id" @callback = sinon.stub() @lines = ["mock", "doc", "lines"] - @version = 42 - @PersistenceManager.setDocInWeb = sinon.stub().callsArg(4) + @PersistenceManager.setDocInWeb = sinon.stub().callsArg(3) @PersistenceManager.setDocVersionInMongo = sinon.stub().callsArg(2) @PersistenceManager.setDoc @project_id, @doc_id, @lines, @version, @callback it "should set the doc in the web api", -> @PersistenceManager.setDocInWeb - .calledWith(@project_id, @doc_id, @lines, @version) + .calledWith(@project_id, @doc_id, @lines) .should.equal true it "should set the doc version in mongo", ->