From c73b7fae69682f2bdb815c4151d84d83322fcca1 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 7 May 2014 14:31:46 +0100 Subject: [PATCH 1/3] Use docstore when creating a new doc --- .../Features/Docstore/DocstoreManager.coffee | 19 ++++- .../Project/ProjectEntityHandler.coffee | 16 ++++- .../Docstore/DocstoreManagerTests.coffee | 39 +++++++++++ .../Project/ProjectEntityHandlerTests.coffee | 69 ++++++++++--------- 4 files changed, 108 insertions(+), 35 deletions(-) diff --git a/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee b/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee index ea9ecdb721..c3ceeb5a35 100644 --- a/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee +++ b/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee @@ -28,4 +28,21 @@ module.exports = DocstoreManager = else error = new Error("docstore api responded with non-success code: #{res.statusCode}") logger.error err: error, project_id: project_id, "error getting all docs in docstore" - callback(error) \ No newline at end of file + callback(error) + + updateDoc: (project_id, doc_id, lines, version, callback = (error, rev) ->) -> + logger.log project_id: project_id, doc_id: doc_id, version: version, "updating doc in docstore api" + url = "#{settings.apis.docstore.url}/project/#{project_id}/doc/#{doc_id}" + request.post { + url: url + json: + lines: lines + version: version + }, (error, res, body) -> + return callback(error) if error? + if 200 <= res.statusCode < 300 + callback(null) + else + error = new Error("docstore api responded with non-success code: #{res.statusCode}") + logger.error err: error, project_id: project_id, doc_id: doc_id, "error updating doc in docstore" + callback(error) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index ccf8a0870b..ffcf0e0d3e 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -117,10 +117,20 @@ module.exports = ProjectEntityHandler = logger.log sl_req_id: sl_req_id, project: project._id, folder_id: folder_id, doc_name: docName, "adding doc" return callback(err) if err? confirmFolder project, folder_id, (folder_id)=> - doc = new Doc name: docName, lines: docLines + doc = new Doc name: docName Project.putElement project._id, folder_id, doc, "doc", (err, result)=> - tpdsUpdateSender.addDoc {project_id:project._id, docLines:docLines, path:result.path.fileSystem, project_name:project.name, rev:doc.rev}, sl_req_id, -> - callback(err, doc, folder_id) + return callback(err) if err? + DocstoreManager.updateDoc project._id.toString(), doc._id.toString(), docLines, 0, (err, rev) -> + return callback(err) if err? + tpdsUpdateSender.addDoc { + project_id: project._id, + docLines: docLines, + path: result.path.fileSystem, + project_name: project.name, + rev: 0 + }, sl_req_id, (err) -> + return callback(err) if err? + callback(null, doc, folder_id) addFile: (project_or_id, folder_id, fileName, path, sl_req_id, callback = (error, fileRef, folder_id) ->)-> {callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id) diff --git a/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee b/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee index b16dad0541..f54bde7832 100644 --- a/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee @@ -52,6 +52,45 @@ describe "DocstoreManager", -> }, "error deleting doc in docstore") .should.equal true + describe "updateDoc", -> + beforeEach -> + @lines = ["mock", "doc", "lines"] + @version = 42 + + describe "with a successful response code", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204, "") + @DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback + + it "should update the doc in the docstore api", -> + @request.post + .calledWith({ + url: "#{@settings.apis.docstore.url}/project/#{@project_id}/doc/#{@doc_id}" + json: + lines: @lines + version: @version + }) + .should.equal true + + it "should call the callback without an error", -> + @callback.calledWith(null).should.equal true + + describe "with a failed response code", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, statusCode: 500, "") + @DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback + + it "should call the callback with an error", -> + @callback.calledWith(new Error("docstore api responded with non-success code: 500")).should.equal true + + it "should log the error", -> + @logger.error + .calledWith({ + err: new Error("docstore api responded with a non-success code: 500") + project_id: @project_id + doc_id: @doc_id + }, "error updating doc in docstore") + .should.equal true describe "getAllDocs", -> describe "with a successful response code", -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index cd86eb4e8e..89ac18cd71 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -1,6 +1,7 @@ chai = require('chai') assert = require('chai').assert should = chai.should() +expect = chai.expect sinon = require 'sinon' modulePath = "../../../../app/js/Features/Project/ProjectEntityHandler" SandboxedModule = require('sandboxed-module') @@ -34,7 +35,8 @@ describe 'ProjectEntityHandler', -> rootFolder:[@rootFolder] @DocModel = class Doc constructor:(options)-> - {@name, @lines} = options + {@name, @lines} = options + @_id = "mock-id" @rev = 0 @FileModel = class File constructor:(options)-> @@ -277,42 +279,47 @@ describe 'ProjectEntityHandler', -> done() @ProjectEntityHandler._removeElementFromMongoArray model, id, mongoPath, -> - describe 'adding doc', -> - docName = "some new doc" - docLines = ['1234','abc'] + describe 'addDoc', -> + beforeEach -> + @name = "some new doc" + @lines = ['1234','abc'] + @path = "/path/to/doc" - it 'should call put element', (done)-> - @ProjectModel.putElement = (passedProject_id, passedFolder_id, passedDoc, passedType, callback)-> - passedProject_id.should.equal project_id - passedFolder_id.should.equal folder_id - passedDoc.name.should.equal docName - passedDoc.lines[0].should.equal docLines[0] - passedDoc.lines[1].should.equal docLines[1] - passedType.should.equal 'doc' - done() - @ProjectEntityHandler.addDoc project_id, folder_id, docName, docLines, "", (err, doc, parentFolder)-> + @ProjectModel.putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}}) + @callback = sinon.stub() + @tpdsUpdateSender.addDoc = sinon.stub().callsArg(2) + @DocstoreManager.updateDoc = sinon.stub().callsArgWith(4, null, 0) - it 'should return doc and parent folder', (done)-> - @ProjectEntityHandler.addDoc project_id, folder_id, docName, docLines, "", (err, doc, parentFolder)-> - parentFolder.should.equal folder_id - doc.name.should.equal docName - done() + @ProjectEntityHandler.addDoc project_id, folder_id, @name, @lines, @callback - it 'should call third party data store', (done)-> - fileSystemPath = "/somehwere/#{docName}" - @ProjectModel.putElement = (project_id, folder_id, doc, type, callback)-> callback(null, {path:{fileSystem:fileSystemPath}}) + # Created doc + @doc = @ProjectModel.putElement.args[0][2] + @doc.name.should.equal @name + expect(@doc.lines).to.be.undefined - @tpdsUpdateSender.addDoc = (options)=> - options.project_id.should.equal project_id - options.docLines.should.equal docLines - options.path.should.equal fileSystemPath - options.project_name.should.equal @project.name - options.rev.should.equal 0 - done() + it 'should call put element', -> + @ProjectModel.putElement + .calledWith(project_id, folder_id, @doc) + .should.equal true - @ProjectEntityHandler.addDoc project_id, folder_id, docName, docLines, "",-> + it 'should return doc and parent folder', -> + @callback.calledWith(null, @doc, folder_id).should.equal true - + it 'should call third party data store', -> + @tpdsUpdateSender.addDoc + .calledWith({ + project_id: project_id + docLines: @lines + path: @path + project_name: @project.name + rev: 0 + }) + .should.equal true + + it "should send the doc lines to the doc store", -> + @DocstoreManager.updateDoc + .calledWith(project_id, @doc._id.toString(), @lines, 0) + .should.equal true describe 'adding file', -> fileName = "something.jpg" From 8315de58c72970d19174b9bdc3fa52e1a0755dc1 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 8 May 2014 13:42:30 +0100 Subject: [PATCH 2/3] Proxy get doc requests to the docstore --- .../Features/Docstore/DocstoreManager.coffee | 18 +++++++- .../Documents/DocumentController.coffee | 10 ++--- .../Project/ProjectEntityHandler.coffee | 3 ++ .../Docstore/DocstoreManagerTests.coffee | 42 ++++++++++++++++++- .../Documents/DocumentControllerTests.coffee | 15 +++---- .../Project/ProjectEntityHandlerTests.coffee | 17 ++++++++ 6 files changed, 90 insertions(+), 15 deletions(-) diff --git a/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee b/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee index c3ceeb5a35..88dde1393f 100644 --- a/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee +++ b/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee @@ -27,7 +27,23 @@ module.exports = DocstoreManager = callback(null, docs) else error = new Error("docstore api responded with non-success code: #{res.statusCode}") - logger.error err: error, project_id: project_id, "error getting all docs in docstore" + logger.error err: error, project_id: project_id, "error getting all docs from docstore" + callback(error) + + getDoc: (project_id, doc_id, callback = (error, lines, version, rev) ->) -> + logger.log project_id: project_id, doc_id: doc_id, "getting doc in docstore api" + url = "#{settings.apis.docstore.url}/project/#{project_id}/doc/#{doc_id}" + request.get { + url: url + json: true + }, (error, res, doc) -> + return callback(error) if error? + if 200 <= res.statusCode < 300 + logger.log doc_id: doc_id, project_id: project_id, version: doc.version, rev: doc.rev, "got doc from docstore api" + callback(null, doc.lines, doc.version, doc.rev) + else + error = new Error("docstore api responded with non-success code: #{res.statusCode}") + logger.error err: error, project_id: project_id, doc_id: doc_id, "error getting doc from docstore" callback(error) updateDoc: (project_id, doc_id, lines, version, callback = (error, rev) ->) -> diff --git a/services/web/app/coffee/Features/Documents/DocumentController.coffee b/services/web/app/coffee/Features/Documents/DocumentController.coffee index bc6fda789b..6e6f33665a 100644 --- a/services/web/app/coffee/Features/Documents/DocumentController.coffee +++ b/services/web/app/coffee/Features/Documents/DocumentController.coffee @@ -1,25 +1,23 @@ -ProjectLocator = require "../Project/ProjectLocator" ProjectEntityHandler = require "../Project/ProjectEntityHandler" Errors = require "../../errors" logger = require("logger-sharelatex") -module.exports = - +module.exports = getDocument: (req, res, next = (error) ->) -> project_id = req.params.Project_id doc_id = req.params.doc_id logger.log doc_id:doc_id, project_id:project_id, "receiving get document request from api (docupdater)" - ProjectLocator.findElement project_id: project_id, element_id: doc_id, type: "doc", (error, doc) -> + ProjectEntityHandler.getDoc project_id, doc_id, (error, lines, version, rev) -> if error? logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument" return next(error) res.type "json" res.send JSON.stringify { - lines: doc.lines + lines: lines + version: version } req.session.destroy() - setDocument: (req, res, next = (error) ->) -> project_id = req.params.Project_id doc_id = req.params.doc_id diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index ffcf0e0d3e..bc6b579a43 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -111,6 +111,9 @@ module.exports = ProjectEntityHandler = logger.log sl_req_id: sl_req_id, project_id: project_id, "removing root doc" Project.update {_id:project_id}, {$unset: {rootDoc_id: true}}, {}, callback + getDoc: (project_id, doc_id, callback = (error, lines, version, rev) ->) -> + DocstoreManager.getDoc project_id, doc_id, callback + addDoc: (project_or_id, folder_id, docName, docLines, sl_req_id, callback = (error, doc, folder_id) ->)=> {callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id) Project.getProject project_or_id, "", (err, project) -> diff --git a/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee b/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee index f54bde7832..b29d7812e0 100644 --- a/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee @@ -92,6 +92,46 @@ describe "DocstoreManager", -> }, "error updating doc in docstore") .should.equal true + describe "getDoc", -> + beforeEach -> + @doc = + lines: @lines = ["mock", "doc", "lines"] + version: @version = 42 + rev: @rev = 5 + + describe "with a successful response code", -> + beforeEach -> + @request.get = sinon.stub().callsArgWith(1, null, statusCode: 204, @doc) + @DocstoreManager.getDoc @project_id, @doc_id, @callback + + it "should get the doc from the docstore api", -> + @request.get + .calledWith({ + url: "#{@settings.apis.docstore.url}/project/#{@project_id}/doc/#{@doc_id}" + json: true + }) + .should.equal true + + it "should call the callback with the lines, version and rev", -> + @callback.calledWith(null, @lines, @version, @rev).should.equal true + + describe "with a failed response code", -> + beforeEach -> + @request.get = sinon.stub().callsArgWith(1, null, statusCode: 500, "") + @DocstoreManager.getDoc @project_id, @doc_id, @callback + + it "should call the callback with an error", -> + @callback.calledWith(new Error("docstore api responded with non-success code: 500")).should.equal true + + it "should log the error", -> + @logger.error + .calledWith({ + err: new Error("docstore api responded with a non-success code: 500") + project_id: @project_id + doc_id: @doc_id + }, "error getting doc from docstore") + .should.equal true + describe "getAllDocs", -> describe "with a successful response code", -> beforeEach -> @@ -122,5 +162,5 @@ describe "DocstoreManager", -> .calledWith({ err: new Error("docstore api responded with a non-success code: 500") project_id: @project_id - }, "error getting all docs in docstore") + }, "error getting all docs from docstore") .should.equal true diff --git a/services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee b/services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee index c5f62fe792..eac78ce97a 100644 --- a/services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee @@ -11,8 +11,7 @@ Errors = require "../../../../app/js/errors" describe "DocumentController", -> beforeEach -> - @DocumentController = SandboxedModule.require modulePath, requires: - "../Project/ProjectLocator": @ProjectLocator = {} + @DocumentController = SandboxedModule.require modulePath, requires: "../Project/ProjectEntityHandler": @ProjectEntityHandler = {} @res = new MockResponse() @req = new MockRequest() @@ -20,32 +19,34 @@ describe "DocumentController", -> @project_id = "project-id-123" @doc_id = "doc-id-123" @doc_lines = ["one", "two", "three"] + @version = 42 + @rev = 5 describe "getDocument", -> beforeEach -> @req.params = Project_id: @project_id doc_id: @doc_id - describe "when the document exists", -> beforeEach -> - @ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, lines: @doc_lines) + @ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, null, @doc_lines, @version, @rev) @DocumentController.getDocument(@req, @res, @next) it "should get the document from Mongo", -> - @ProjectLocator.findElement - .calledWith(project_id: @project_id, element_id: @doc_id, type: "doc") + @ProjectEntityHandler.getDoc + .calledWith(@project_id, @doc_id) .should.equal true it "should return the document data to the client as JSON", -> @res.type.should.equal "json" @res.body.should.equal JSON.stringify lines: @doc_lines + version: @version describe "when the document doesn't exist", -> beforeEach -> - @ProjectLocator.findElement = sinon.stub().callsArgWith(1, new Errors.NotFoundError("not found"), null) + @ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, new Errors.NotFoundError("not found"), null) @DocumentController.getDocument(@req, @res, @next) it "should call next with the NotFoundError", -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index 89ac18cd71..4b8fe460a2 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -10,6 +10,7 @@ tk = require 'timekeeper' describe 'ProjectEntityHandler', -> project_id = '4eecb1c1bffa66588e0000a1' + doc_id = '4eecb1c1bffa66588e0000a2' folder_id = "4eecaffcbffa66588e000008" rootFolderId = "4eecaffcbffa66588e000007" @@ -279,6 +280,22 @@ describe 'ProjectEntityHandler', -> done() @ProjectEntityHandler._removeElementFromMongoArray model, id, mongoPath, -> + describe 'getDoc', -> + beforeEach -> + @lines = ["mock", "doc", "lines"] + @version = 42 + @rev = 5 + @DocstoreManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @rev) + @ProjectEntityHandler.getDoc project_id, doc_id, @callback + + it "should call the docstore", -> + @DocstoreManager.getDoc + .calledWith(project_id, doc_id) + .should.equal true + + it "should call the callback with the lines, version and rev", -> + @callback.calledWith(null, @lines, @version, @rev).should.equal true + describe 'addDoc', -> beforeEach -> @name = "some new doc" From 0bddc5552ee86b63ae6bd7713f343ccf540927e2 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 8 May 2014 15:47:50 +0100 Subject: [PATCH 3/3] Use docstore for updating documents --- .../Features/Docstore/DocstoreManager.coffee | 6 +- .../DocumentUpdaterHandler.coffee | 16 +++ .../Documents/DocumentController.coffee | 3 +- .../Features/Editor/EditorController.coffee | 3 +- .../Project/ProjectEntityHandler.coffee | 58 +++----- .../Docstore/DocstoreManagerTests.coffee | 8 +- .../DocumentUpdaterHandlerTests.coffee | 34 +++++ .../Documents/DocumentControllerTests.coffee | 8 +- .../Editor/EditorControllerTests.coffee | 11 +- .../Project/ProjectEntityHandlerTests.coffee | 136 ++++++++++++------ 10 files changed, 186 insertions(+), 97 deletions(-) diff --git a/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee b/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee index 88dde1393f..56e74b6f63 100644 --- a/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee +++ b/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee @@ -46,7 +46,7 @@ module.exports = DocstoreManager = logger.error err: error, project_id: project_id, doc_id: doc_id, "error getting doc from docstore" callback(error) - updateDoc: (project_id, doc_id, lines, version, callback = (error, rev) ->) -> + updateDoc: (project_id, doc_id, lines, version, callback = (error, modified, rev) ->) -> logger.log project_id: project_id, doc_id: doc_id, version: version, "updating doc in docstore api" url = "#{settings.apis.docstore.url}/project/#{project_id}/doc/#{doc_id}" request.post { @@ -54,10 +54,10 @@ module.exports = DocstoreManager = json: lines: lines version: version - }, (error, res, body) -> + }, (error, res, result) -> return callback(error) if error? if 200 <= res.statusCode < 300 - callback(null) + callback(null, result.modified, result.rev) else error = new Error("docstore api responded with non-success code: #{res.statusCode}") logger.error err: error, project_id: project_id, doc_id: doc_id, "error updating doc in docstore" diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index e5c8186811..ba38137fb0 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -58,6 +58,22 @@ module.exports = error = new Error("document updater returned a failure status code: #{res.statusCode}") logger.error err: error, project_id: project_id, sl_req_id: sl_req_id, "document updater returned failure status code: #{res.statusCode}" return callback(error) + + flushDocToMongo: (project_id, doc_id, callback = (error) ->) -> + logger.log project_id:project_id, doc_id: doc_id, "flushing doc from document updater" + timer = new metrics.Timer("flushing.mongo.doc") + url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}/flush" + request.post url, (error, res, body)-> + if error? + logger.error err: error, project_id: project_id, doc_id: doc_id, "error flushing doc from document updater" + return callback(error) + else if res.statusCode >= 200 and res.statusCode < 300 + logger.log project_id: project_id, doc_id: doc_id, "flushed doc from document updater" + return callback(null) + else + error = new Error("document updater returned a failure status code: #{res.statusCode}") + logger.error err: error, project_id: project_id, doc_id: doc_id, "document updater returned failure status code: #{res.statusCode}" + return callback(error) deleteDoc : (project_id, doc_id, sl_req_id, callback = ()->)-> {callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id) diff --git a/services/web/app/coffee/Features/Documents/DocumentController.coffee b/services/web/app/coffee/Features/Documents/DocumentController.coffee index 6e6f33665a..dc32b05aed 100644 --- a/services/web/app/coffee/Features/Documents/DocumentController.coffee +++ b/services/web/app/coffee/Features/Documents/DocumentController.coffee @@ -22,8 +22,9 @@ module.exports = project_id = req.params.Project_id doc_id = req.params.doc_id lines = req.body.lines + version = req.body.version logger.log doc_id:doc_id, project_id:project_id, "receiving set document request from api (docupdater)" - ProjectEntityHandler.updateDocLines project_id, doc_id, lines, (error) -> + ProjectEntityHandler.updateDocLines project_id, doc_id, lines, version, (error) -> if error? logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument" return next(error) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 4bda1d4874..0093fc0a52 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -158,8 +158,7 @@ module.exports = EditorController = DocumentUpdaterHandler.setDocument project_id, doc_id, docLines, (err)=> logger.log project_id:project_id, doc_id:doc_id, "notifying users that the document has been updated" EditorRealTimeController.emitToRoom(project_id, "entireDocUpdate", doc_id) - ProjectEntityHandler.updateDocLines project_id, doc_id, docLines, sl_req_id, (err)-> - callback(err) + DocumentUpdaterHandler.flushDocToMongo project_id, doc_id, callback addDoc: (project_id, folder_id, docName, docLines, sl_req_id, callback = (error, doc)->)-> {callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index bc6b579a43..9762093ecd 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -123,7 +123,7 @@ module.exports = ProjectEntityHandler = doc = new Doc name: docName Project.putElement project._id, folder_id, doc, "doc", (err, result)=> return callback(err) if err? - DocstoreManager.updateDoc project._id.toString(), doc._id.toString(), docLines, 0, (err, rev) -> + DocstoreManager.updateDoc project._id.toString(), doc._id.toString(), docLines, 0, (err, modified, rev) -> return callback(err) if err? tpdsUpdateSender.addDoc { project_id: project._id, @@ -234,45 +234,31 @@ module.exports = ProjectEntityHandler = if callback? callback(err, folder, parentFolder_id) - updateDocLines : (project_or_id, doc_id, docLines, sl_req_id, callback = (error) ->)-> - {callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id) - Project.getProject project_or_id, "", (err, project)-> + updateDocLines : (project_id, doc_id, lines, version, callback = (error) ->)-> + ProjectGetter.getProjectWithoutDocLines project_id, (err, project)-> return callback(err) if err? return callback(new Errors.NotFoundError("project not found")) if !project? - project_id = project._id - if err? - logger.err err:err,project_id:project_id, "error finding project" - callback err - else if !project? - logger.err project_id:project_id, doc_id:doc_id, err: new Error("project #{project_id} could not be found for doc #{doc_id}") - callback "could not find project #{project_id}" - else - projectLocator.findElement {project:project, element_id:doc_id, type:"docs"}, (err, doc, path)-> + logger.log project_id: project_id, doc_id: doc_id, version: version, "updating doc lines" + projectLocator.findElement {project:project, element_id:doc_id, type:"docs"}, (err, doc, path)-> + if err? + logger.error err: err, doc_id: doc_id, project_id: project_id, version: version, lines: lines, "error finding doc while updating doc lines" + return callback err + if !doc? + error = new Errors.NotFoundError("doc not found") + logger.error err: error, doc_id: doc_id, project_id: project_id, version: version, lines: lines, "doc not found while updating doc lines" + return callback(error) + + DocstoreManager.updateDoc project_id, doc_id, lines, version, (err, modified, rev) -> if err? - logger.err "error putting doc #{doc_id} in project #{project_id} #{err}" - callback err - else if docComparitor.areSame docLines, doc.lines - logger.log sl_req_id: sl_req_id, project_id:project_id, doc_id:doc_id, rev:doc.rev, "old doc lines are same as the new doc lines, not updating them" - callback() + logger.error err: err, doc_id: doc_id, project_id:project_id, lines: lines, version: version, "error sending doc to docstore" + return callback(err) + + if modified + # Don't need to block for marking as updated + projectUpdateHandler.markAsUpdated project_id + tpdsUpdateSender.addDoc {project_id:project_id, path:path.fileSystem, docLines:lines, project_name:project.name, rev:rev}, callback else - logger.log sl_req_id: sl_req_id, project_id:project_id, doc_id:doc_id, docLines: docLines, oldDocLines: doc.lines, rev:doc.rev, "updating doc lines" - conditons = _id:project_id - update = {$set:{}, $inc:{}} - changeLines = {} - changeLines["#{path.mongo}.lines"] = docLines - inc = {} - inc["#{path.mongo}.rev"] = 1 - update["$set"] = changeLines - update["$inc"] = inc - Project.update conditons, update, {}, (err, second)-> - if(err) - logger.err(sl_req_id:sl_req_id, doc_id:doc_id, project_id:project_id, err:err, "error saving doc to mongo") - callback(err) - else - logger.log sl_req_id:sl_req_id, doc_id:doc_id, project_id:project_id, newDocLines:docLines, oldDocLines:doc.lines, "doc saved to mongo" - rev = doc.rev+1 - projectUpdateHandler.markAsUpdated project_id - tpdsUpdateSender.addDoc {project_id:project_id, path:path.fileSystem, docLines:docLines, project_name:project.name, rev:rev}, sl_req_id, callback + callback() moveEntity: (project_id, entity_id, folder_id, entityType, sl_req_id, callback = (error) ->)-> {callback, sl_req_id} = slReqIdHelper.getCallbackAndReqId(callback, sl_req_id) diff --git a/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee b/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee index b29d7812e0..572cff3b29 100644 --- a/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee @@ -56,10 +56,12 @@ describe "DocstoreManager", -> beforeEach -> @lines = ["mock", "doc", "lines"] @version = 42 + @rev = 5 + @modified = true describe "with a successful response code", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204, "") + @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204, { modified: @modified, rev: @rev }) @DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback it "should update the doc in the docstore api", -> @@ -72,8 +74,8 @@ describe "DocstoreManager", -> }) .should.equal true - it "should call the callback without an error", -> - @callback.calledWith(null).should.equal true + it "should call the callback with the modified status and revision", -> + @callback.calledWith(null, @modified, @rev).should.equal true describe "with a failed response code", -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 11ca80a4dc..097269a731 100644 --- a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -156,6 +156,40 @@ describe 'Flushing documents :', -> .calledWith(new Error("doc updater returned failure status code: 500")) .should.equal true + describe 'flushDocToMongo', -> + beforeEach -> + @callback = sinon.stub() + + describe "successfully", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") + @handler.flushDocToMongo @project_id, @doc_id, @callback + + it 'should flush the document from the document updater', -> + url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}/flush" + @request.post.calledWith(url).should.equal true + + it "should call the callback with no error", -> + @callback.calledWith(null).should.equal true + + describe "when the document updater API returns an error", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null) + @handler.flushDocToMongo @project_id, @doc_id, @callback + + it "should return an error to the callback", -> + @callback.calledWith(@error).should.equal true + + describe "when the document updater returns a failure error code", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @handler.flushDocToMongo @project_id, @doc_id, @callback + + it "should return the callback with an error", -> + @callback + .calledWith(new Error("doc updater returned failure status code: 500")) + .should.equal true + describe "deleteDoc", -> beforeEach -> @callback = sinon.stub() diff --git a/services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee b/services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee index eac78ce97a..99b883d668 100644 --- a/services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee @@ -61,14 +61,15 @@ describe "DocumentController", -> describe "when the document exists", -> beforeEach -> - @ProjectEntityHandler.updateDocLines = sinon.stub().callsArg(3) + @ProjectEntityHandler.updateDocLines = sinon.stub().callsArg(4) @req.body = lines: @doc_lines + version: @version @DocumentController.setDocument(@req, @res, @next) it "should update the document in Mongo", -> @ProjectEntityHandler.updateDocLines - .calledWith(@project_id, @doc_id, @doc_lines) + .calledWith(@project_id, @doc_id, @doc_lines, @version) .should.equal true it "should return a successful response", -> @@ -76,9 +77,10 @@ describe "DocumentController", -> describe "when the document doesn't exist", -> beforeEach -> - @ProjectEntityHandler.updateDocLines = sinon.stub().callsArgWith(3, new Errors.NotFoundError("document does not exist")) + @ProjectEntityHandler.updateDocLines = sinon.stub().callsArgWith(4, new Errors.NotFoundError("document does not exist")) @req.body = lines: @doc_lines + version: @version @DocumentController.setDocument(@req, @res, @next) it "should call next with the NotFoundError", -> diff --git a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee index 406c68b0ea..755dc2e960 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee @@ -372,7 +372,7 @@ describe "EditorController", -> describe 'set document', -> beforeEach -> @docLines = ["foo", "bar"] - @ProjectEntityHandler.updateDocLines = sinon.stub().callsArg(4) + @DocumentUpdaterHandler.flushDocToMongo = sinon.stub().callsArg(2) @DocumentUpdaterHandler.setDocument = sinon.stub().callsArg(3) @EditorRealTimeController.emitToRoom = sinon.stub() @@ -394,12 +394,9 @@ describe "EditorController", -> mock.verify() done() - it 'should update the document lines', (done)-> - @ProjectEntityHandler.updateDocLines = -> - mock = sinon.mock(@ProjectEntityHandler).expects("updateDocLines").withArgs(@project_id, @doc_id, @docLines).once().callsArg(4) - - @EditorController.setDoc @project_id, @doc_id, @docLines, (err)-> - mock.verify() + it 'should flush the doc to mongo', (done)-> + @EditorController.setDoc @project_id, @doc_id, @docLines, (err)=> + @DocumentUpdaterHandler.flushDocToMongo.calledWith(@project_id, @doc_id).should.equal true done() diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index 4b8fe460a2..708874125a 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -7,6 +7,7 @@ modulePath = "../../../../app/js/Features/Project/ProjectEntityHandler" SandboxedModule = require('sandboxed-module') ObjectId = require("mongoose").Types.ObjectId tk = require 'timekeeper' +Errors = require "../../../../app/js/errors" describe 'ProjectEntityHandler', -> project_id = '4eecb1c1bffa66588e0000a1' @@ -65,7 +66,7 @@ describe 'ProjectEntityHandler', -> './ProjectLocator':@projectLocator = {} '../../Features/DocumentUpdater/DocumentUpdaterHandler':@documentUpdaterHandler = {} '../Docstore/DocstoreManager': @DocstoreManager = {} - 'logger-sharelatex':{log:->} + 'logger-sharelatex': @logger = {log:sinon.stub(), error: sinon.stub()} './ProjectUpdateHandler': @projectUpdater "./ProjectGetter": @ProjectGetter = {} @@ -305,7 +306,7 @@ describe 'ProjectEntityHandler', -> @ProjectModel.putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}}) @callback = sinon.stub() @tpdsUpdateSender.addDoc = sinon.stub().callsArg(2) - @DocstoreManager.updateDoc = sinon.stub().callsArgWith(4, null, 0) + @DocstoreManager.updateDoc = sinon.stub().callsArgWith(4, null, true, 0) @ProjectEntityHandler.addDoc project_id, folder_id, @name, @lines, @callback @@ -457,54 +458,105 @@ describe 'ProjectEntityHandler', -> done() - describe 'updating document lines', -> - docId = "123456" - docLines = ['1234','abc', '543543'] - mongoPath = "folders[0].folders[5]" - fileSystemPath = "/somehwere/something.tex" + describe 'updateDocLines', -> + beforeEach -> + @lines = ['mock', 'doc', 'lines'] + @path = "/somewhere/something.tex" + @doc = { + _id: doc_id + } + @version = 42 + @ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, @project) + @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, {fileSystem: @path}) + @tpdsUpdateSender.addDoc = sinon.stub().callsArg(1) + @projectUpdater.markAsUpdated = sinon.stub() + @callback = sinon.stub() - it 'should find project via getProject', (done)-> - @ProjectModel.getProject = (passedProject_id, callback)-> - passedProject_id.should.equal project_id - done() + describe "when the doc has been modified", -> + beforeEach -> + @DocstoreManager.updateDoc = sinon.stub().callsArgWith(4, null, true, @rev = 5) + @ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback - @ProjectEntityHandler.updateDocLines project_id, "", [], -> + it "should get the project without doc lines", -> + @ProjectGetter.getProjectWithoutDocLines + .calledWith(project_id) + .should.equal true - it 'should find the doc', (done)-> - - @projectLocator.findElement = (options, callback)-> - options.element_id.should.equal docId - options.type.should.equal 'docs' - done() + it "should find the doc", -> + @projectLocator.findElement + .calledWith({ + project: @project + type: "docs" + element_id: doc_id + }) + .should.equal true - @ProjectEntityHandler.updateDocLines project_id, docId, "", -> + it "should update the doc in the docstore", -> + @DocstoreManager.updateDoc + .calledWith(project_id, doc_id, @lines, @version) + .should.equal true - it 'should build mongo update statment', (done)-> - @projectLocator.findElement = (opts, callback)-> - callback(null, {lines:[], rev:0}, {mongo:mongoPath}) + it "should mark the project as updated", -> + @projectUpdater.markAsUpdated + .calledWith(project_id) + .should.equal true - @ProjectModel.update = (conditions, update, options, callback)-> - conditions._id.should.equal project_id - update.$set["#{mongoPath}.lines"].should.equal docLines - update.$inc["#{mongoPath}.rev"].should.equal 1 - done() + it "should send the doc the to the TPDS", -> + @tpdsUpdateSender.addDoc + .calledWith({ + project_id: project_id + project_name: @project.name + docLines: @lines + rev: @rev + path: @path + }) + .should.equal true - @ProjectEntityHandler.updateDocLines project_id, docId, docLines, -> + it "should call the callback", -> + @callback.called.should.equal true + + describe "when the doc has not been modified", -> + beforeEach -> + @DocstoreManager.updateDoc = sinon.stub().callsArgWith(4, null, false, @rev = 5) + @ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback + + it "should not mark the project as updated", -> + @projectUpdater.markAsUpdated.called.should.equal false + + it "should not send the doc the to the TPDS", -> + @tpdsUpdateSender.addDoc.called.should.equal false + + it "should call the callback", -> + @callback.called.should.equal true + + describe "when the project is not found", -> + beforeEach -> + @ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, null) + @ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback + + it "should return a not found error", -> + @callback.calledWith(new Errors.NotFoundError()).should.equal true + + describe "when the doc is not found", -> + beforeEach -> + @projectLocator.findElement = sinon.stub().callsArgWith(1, null, null, null) + @ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback + + it "should log out the error", -> + @logger.error + .calledWith( + project_id: project_id + doc_id: doc_id + version: @version + lines: @lines + err: new Errors.NotFoundError("doc not found") + "doc not found while updating doc lines" + ) + .should.equal true + + it "should return a not found error", -> + @callback.calledWith(new Errors.NotFoundError()).should.equal true - it 'should call third party data store ', (done)-> - rev = 3 - @projectLocator.findElement = (opts, callback)-> - callback(null, {lines:[],rev:rev}, {fileSystem:fileSystemPath}) - @ProjectModel.update = (conditions, update, options, callback)-> callback() - @tpdsUpdateSender.addDoc = (options, _, callback)=> - options.project_id.should.equal project_id - options.docLines.should.equal docLines - options.path.should.equal fileSystemPath - options.project_name.should.equal @project.name - options.rev.should.equal (rev+1) - callback() - @projectUpdater.markAsUpdated.calledWith(project_id).should.equal true - @ProjectEntityHandler.updateDocLines project_id, docId, docLines, done describe "getting folders, docs and files", -> beforeEach ->