From 4d2f0218afd34436777424a05f997ae68e2e0fc5 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 28 Feb 2018 17:31:26 +0000 Subject: [PATCH 1/9] add resync project history endpoint --- .../DocumentUpdaterHandler.coffee | 29 +++++++++++++++++++ .../Features/History/HistoryController.coffee | 7 +++++ services/web/app/coffee/router.coffee | 1 + 3 files changed, 37 insertions(+) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index 04c11c6419..e3cbf4ce8e 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -5,7 +5,9 @@ _ = require 'underscore' async = require 'async' logger = require('logger-sharelatex') metrics = require('metrics-sharelatex') +FileStoreHandler = require("../FileStore/FileStoreHandler") Project = require("../../models/Project").Project +ProjectGetter = require "../Project/ProjectGetter" module.exports = DocumentUpdaterHandler = flushProjectToMongo: (project_id, callback = (error) ->)-> @@ -203,6 +205,33 @@ module.exports = DocumentUpdaterHandler = logger.error {project_id, doc_id, thread_id}, "doc updater returned a non-success status code: #{res.statusCode}" callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + resyncProject: (project_id, callback) -> + ProjectEntityHandler = require "../Project/ProjectEntityHandler" + ProjectGetter.getProject project_id, rootFolder: true, (error, project) -> + return callback(error) if error? + ProjectEntityHandler.getAllEntitiesFromProject project, (error, docs, files) -> + return callback(error) if error? + + docs = _.map docs, (doc) -> + doc: doc.doc._id + path: doc.path + + files = _.map files, (file) -> + file: file.file._id + path: file.path + url: FileStoreHandler._buildUrl(project_id, file.file._id) + + request.post + url: "#{settings.apis.documentupdater.url}/project/#{project_id}/resync" + json: { docs, files } + , (error, res, body) -> + if error? + logger.error {error, project_id}, "error resyncing project in doc updater" + callback(error) + else if res.statusCode >= 200 and res.statusCode < 300 + logger.log {project_id}, "resynced project in doc updater" + callback() + updateProjectStructure : (project_id, userId, changes, callback = (error) ->)-> return callback() if !settings.apis.project_history?.sendProjectStructureOps Project.findOne {_id: project_id}, {version:true}, (err, currentProject) -> diff --git a/services/web/app/coffee/Features/History/HistoryController.coffee b/services/web/app/coffee/Features/History/HistoryController.coffee index d158cd86c6..5ce0338306 100644 --- a/services/web/app/coffee/Features/History/HistoryController.coffee +++ b/services/web/app/coffee/Features/History/HistoryController.coffee @@ -2,6 +2,7 @@ logger = require "logger-sharelatex" request = require "request" settings = require "settings-sharelatex" AuthenticationController = require "../Authentication/AuthenticationController" +DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" ProjectDetailsHandler = require "../Project/ProjectDetailsHandler" HistoryManager = require "./HistoryManager" @@ -62,3 +63,9 @@ module.exports = HistoryController = return settings.apis.project_history.url else return settings.apis.trackchanges.url + + resyncProject: (req, res, next = (error) ->) -> + project_id = req.params.Project_id + DocumentUpdaterHandler.resyncProject project_id, (error) -> + return next(error) if error? + res.sendStatus 204 diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 1241e16684..fb2a7cb756 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -201,6 +201,7 @@ module.exports = class Router webRouter.get "/project/:Project_id/doc/:doc_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApi webRouter.get "/project/:Project_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApiAndInjectUserDetails webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApi + privateApiRouter.post "/project/:Project_id/history/resync", AuthenticationController.httpAuth, HistoryController.resyncProject webRouter.get '/Project/:Project_id/download/zip', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectDownloadsController.downloadProject webRouter.get '/project/download/zip', AuthorizationMiddlewear.ensureUserCanReadMultipleProjects, ProjectDownloadsController.downloadMultipleProjects From 926f91dd3bbe5351f8aef01eea045843b488290e Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 7 Mar 2018 11:02:29 +0000 Subject: [PATCH 2/9] wrap project resync in project structure lock --- .../DocumentUpdaterHandler.coffee | 37 ++++++------------- .../Features/History/HistoryController.coffee | 4 +- .../Project/ProjectEntityUpdateHandler.coffee | 19 ++++++++++ 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index e3cbf4ce8e..3d8ae48478 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -205,32 +205,17 @@ module.exports = DocumentUpdaterHandler = logger.error {project_id, doc_id, thread_id}, "doc updater returned a non-success status code: #{res.statusCode}" callback new Error("doc updater returned a non-success status code: #{res.statusCode}") - resyncProject: (project_id, callback) -> - ProjectEntityHandler = require "../Project/ProjectEntityHandler" - ProjectGetter.getProject project_id, rootFolder: true, (error, project) -> - return callback(error) if error? - ProjectEntityHandler.getAllEntitiesFromProject project, (error, docs, files) -> - return callback(error) if error? - - docs = _.map docs, (doc) -> - doc: doc.doc._id - path: doc.path - - files = _.map files, (file) -> - file: file.file._id - path: file.path - url: FileStoreHandler._buildUrl(project_id, file.file._id) - - request.post - url: "#{settings.apis.documentupdater.url}/project/#{project_id}/resync" - json: { docs, files } - , (error, res, body) -> - if error? - logger.error {error, project_id}, "error resyncing project in doc updater" - callback(error) - else if res.statusCode >= 200 and res.statusCode < 300 - logger.log {project_id}, "resynced project in doc updater" - callback() + resyncProject: (project_id, docs, files, callback) -> + request.post + url: "#{settings.apis.documentupdater.url}/project/#{project_id}/resync" + json: { docs, files } + , (error, res, body) -> + if error? + logger.error {error, project_id}, "error resyncing project in doc updater" + callback(error) + else if res.statusCode >= 200 and res.statusCode < 300 + logger.log {project_id}, "resynced project in doc updater" + callback() updateProjectStructure : (project_id, userId, changes, callback = (error) ->)-> return callback() if !settings.apis.project_history?.sendProjectStructureOps diff --git a/services/web/app/coffee/Features/History/HistoryController.coffee b/services/web/app/coffee/Features/History/HistoryController.coffee index 5ce0338306..aa0a1f5101 100644 --- a/services/web/app/coffee/Features/History/HistoryController.coffee +++ b/services/web/app/coffee/Features/History/HistoryController.coffee @@ -2,8 +2,8 @@ logger = require "logger-sharelatex" request = require "request" settings = require "settings-sharelatex" AuthenticationController = require "../Authentication/AuthenticationController" -DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" ProjectDetailsHandler = require "../Project/ProjectDetailsHandler" +ProjectEntityUpdateHandler = require "../Project/ProjectEntityUpdateHandler" HistoryManager = require "./HistoryManager" module.exports = HistoryController = @@ -66,6 +66,6 @@ module.exports = HistoryController = resyncProject: (req, res, next = (error) ->) -> project_id = req.params.Project_id - DocumentUpdaterHandler.resyncProject project_id, (error) -> + ProjectEntityUpdateHandler.resyncProject project_id, (error) -> return next(error) if error? res.sendStatus 204 diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index aadc0ac0f8..af41c3269c 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -313,6 +313,25 @@ module.exports = ProjectEntityUpdateHandler = self = TpdsUpdateSender.moveEntity({project_id, startPath, endPath, project_name, rev}) DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback + # This doesn't directly update project structure but we need to take the lock + # to prevent anything else being queued before the resync update + resyncProject: wrapWithLock (project_id, callback) -> + ProjectGetter.getProject project_id, rootFolder: true, (error, project) -> + return callback(error) if error? + ProjectEntityHandler.getAllEntitiesFromProject project, (error, docs, files) -> + return callback(error) if error? + + docs = _.map docs, (doc) -> + doc: doc.doc._id + path: doc.path + + files = _.map files, (file) -> + file: file.file._id + path: file.path + url: FileStoreHandler._buildUrl(project_id, file.file._id) + + DocumentUpdaterHandler.resyncProject project_id, docs, files, callback + _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> if(entityType.indexOf("file") != -1) self._cleanUpFile project, entity, path, userId, callback From 50fdfec6e8b5ab5f5cbbed034890a22431af9df9 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 7 Mar 2018 11:18:40 +0000 Subject: [PATCH 3/9] add unit tests for project history resync --- .../History/HistoryControllerTests.coffee | 22 +++++++++ .../ProjectEntityUpdateHandlerTests.coffee | 45 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee index 7dc3c88169..35c512426e 100644 --- a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee +++ b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee @@ -17,6 +17,7 @@ describe "HistoryController", -> "./HistoryManager": @HistoryManager = {} "../Authentication/AuthenticationController": @AuthenticationController "../Project/ProjectDetailsHandler": @ProjectDetailsHandler = {} + "../Project/ProjectEntityUpdateHandler": @ProjectEntityUpdateHandler = {} @settings.apis = trackchanges: enabled: false @@ -199,3 +200,24 @@ describe "HistoryController", -> it "should not return the data with users to the client", -> @res.json.calledWith(@data_with_users).should.equal false + + describe "resyncProject", -> + beforeEach -> + @project_id = 'mock-project-id' + @req = params: Project_id: @project_id + @res = sendStatus: sinon.stub() + @next = sinon.stub() + + @ProjectEntityUpdateHandler.resyncProject = sinon.stub().yields() + + @HistoryController.resyncProject @req, @res, @next + + it "resyncs the project", -> + @ProjectEntityUpdateHandler.resyncProject + .calledWith(@project_id) + .should.equal true + + it "responds with a 204", -> + @res.sendStatus + .calledWith(204) + .should.equal true diff --git a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee index ba40fa32db..06beca3cb3 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee @@ -745,6 +745,51 @@ describe 'ProjectEntityUpdateHandler', -> .calledWith(project_id, userId, @changes, @callback) .should.equal true + describe "resyncProject", -> + beforeEach -> + @ProjectGetter.getProject = sinon.stub().yields(null, @project) + docs = [ + doc: _id: doc_id + path: 'main.tex' + ] + files = [ + file: _id: file_id + path: 'universe.png' + ] + @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub().yields(null, docs, files) + @FileStoreHandler._buildUrl = (project_id, file_id) -> + "www.filestore.test/#{project_id}/#{file_id}" + @DocumentUpdaterHandler.resyncProject = sinon.stub().yields() + + @ProjectEntityUpdateHandler.resyncProject project_id, @callback + + it 'gets the project', -> + @ProjectGetter.getProject + .calledWith(project_id) + .should.equal true + + it 'gets the entities for the project', -> + @ProjectEntityHandler.getAllEntitiesFromProject + .calledWith(@project) + .should.equal true + + it 'tells the doc updater to sync the project', -> + docs = [ + doc: doc_id + path: 'main.tex' + ] + files = [ + file: file_id + path: 'universe.png' + url: "www.filestore.test/#{project_id}/#{file_id}" + ] + @DocumentUpdaterHandler.resyncProject + .calledWith(project_id, docs, files) + .should.equal true + + it 'calls the callback', -> + @callback.called.should.equal true + describe "_cleanUpEntity", -> beforeEach -> @entity_id = "4eecaffcbffa66588e000009" From 5b2e7d981a2096b6474bb0799de0ad34e8696922 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 7 Mar 2018 11:19:47 +0000 Subject: [PATCH 4/9] resyncProject -> resyncProjectHistory --- .../DocumentUpdater/DocumentUpdaterHandler.coffee | 2 +- .../app/coffee/Features/History/HistoryController.coffee | 4 ++-- .../Features/Project/ProjectEntityUpdateHandler.coffee | 4 ++-- .../unit/coffee/History/HistoryControllerTests.coffee | 8 ++++---- .../coffee/Project/ProjectEntityUpdateHandlerTests.coffee | 8 ++++---- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index 3d8ae48478..a9ce174482 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -205,7 +205,7 @@ module.exports = DocumentUpdaterHandler = logger.error {project_id, doc_id, thread_id}, "doc updater returned a non-success status code: #{res.statusCode}" callback new Error("doc updater returned a non-success status code: #{res.statusCode}") - resyncProject: (project_id, docs, files, callback) -> + resyncProjectHistory: (project_id, docs, files, callback) -> request.post url: "#{settings.apis.documentupdater.url}/project/#{project_id}/resync" json: { docs, files } diff --git a/services/web/app/coffee/Features/History/HistoryController.coffee b/services/web/app/coffee/Features/History/HistoryController.coffee index aa0a1f5101..78a63942d7 100644 --- a/services/web/app/coffee/Features/History/HistoryController.coffee +++ b/services/web/app/coffee/Features/History/HistoryController.coffee @@ -64,8 +64,8 @@ module.exports = HistoryController = else return settings.apis.trackchanges.url - resyncProject: (req, res, next = (error) ->) -> + resyncProjectHistory: (req, res, next = (error) ->) -> project_id = req.params.Project_id - ProjectEntityUpdateHandler.resyncProject project_id, (error) -> + ProjectEntityUpdateHandler.resyncProjectHistory project_id, (error) -> return next(error) if error? res.sendStatus 204 diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index af41c3269c..82cb63e761 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -315,7 +315,7 @@ module.exports = ProjectEntityUpdateHandler = self = # This doesn't directly update project structure but we need to take the lock # to prevent anything else being queued before the resync update - resyncProject: wrapWithLock (project_id, callback) -> + resyncProjectHistory: wrapWithLock (project_id, callback) -> ProjectGetter.getProject project_id, rootFolder: true, (error, project) -> return callback(error) if error? ProjectEntityHandler.getAllEntitiesFromProject project, (error, docs, files) -> @@ -330,7 +330,7 @@ module.exports = ProjectEntityUpdateHandler = self = path: file.path url: FileStoreHandler._buildUrl(project_id, file.file._id) - DocumentUpdaterHandler.resyncProject project_id, docs, files, callback + DocumentUpdaterHandler.resyncProjectHistory project_id, docs, files, callback _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> if(entityType.indexOf("file") != -1) diff --git a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee index 35c512426e..eeb206f94b 100644 --- a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee +++ b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee @@ -201,19 +201,19 @@ describe "HistoryController", -> it "should not return the data with users to the client", -> @res.json.calledWith(@data_with_users).should.equal false - describe "resyncProject", -> + describe "resyncProjectHistory", -> beforeEach -> @project_id = 'mock-project-id' @req = params: Project_id: @project_id @res = sendStatus: sinon.stub() @next = sinon.stub() - @ProjectEntityUpdateHandler.resyncProject = sinon.stub().yields() + @ProjectEntityUpdateHandler.resyncProjectHistory = sinon.stub().yields() - @HistoryController.resyncProject @req, @res, @next + @HistoryController.resyncProjectHistory @req, @res, @next it "resyncs the project", -> - @ProjectEntityUpdateHandler.resyncProject + @ProjectEntityUpdateHandler.resyncProjectHistory .calledWith(@project_id) .should.equal true diff --git a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee index 06beca3cb3..c00826a5dd 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee @@ -745,7 +745,7 @@ describe 'ProjectEntityUpdateHandler', -> .calledWith(project_id, userId, @changes, @callback) .should.equal true - describe "resyncProject", -> + describe "resyncProjectHistory", -> beforeEach -> @ProjectGetter.getProject = sinon.stub().yields(null, @project) docs = [ @@ -759,9 +759,9 @@ describe 'ProjectEntityUpdateHandler', -> @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub().yields(null, docs, files) @FileStoreHandler._buildUrl = (project_id, file_id) -> "www.filestore.test/#{project_id}/#{file_id}" - @DocumentUpdaterHandler.resyncProject = sinon.stub().yields() + @DocumentUpdaterHandler.resyncProjectHistory = sinon.stub().yields() - @ProjectEntityUpdateHandler.resyncProject project_id, @callback + @ProjectEntityUpdateHandler.resyncProjectHistory project_id, @callback it 'gets the project', -> @ProjectGetter.getProject @@ -783,7 +783,7 @@ describe 'ProjectEntityUpdateHandler', -> path: 'universe.png' url: "www.filestore.test/#{project_id}/#{file_id}" ] - @DocumentUpdaterHandler.resyncProject + @DocumentUpdaterHandler.resyncProjectHistory .calledWith(project_id, docs, files) .should.equal true From 9ab0ded8f17d77058bf4c76e72ee85aa934b8dd7 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 7 Mar 2018 11:31:54 +0000 Subject: [PATCH 5/9] update project history resync url --- .../Features/DocumentUpdater/DocumentUpdaterHandler.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index a9ce174482..cfe922922c 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -207,7 +207,7 @@ module.exports = DocumentUpdaterHandler = resyncProjectHistory: (project_id, docs, files, callback) -> request.post - url: "#{settings.apis.documentupdater.url}/project/#{project_id}/resync" + url: "#{settings.apis.documentupdater.url}/project/#{project_id}/history/resync" json: { docs, files } , (error, res, body) -> if error? From 792e8bbd7e2a46e40d4aa47533501791d91d63fa Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 7 Mar 2018 14:10:53 +0000 Subject: [PATCH 6/9] fix route name --- services/web/app/coffee/router.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index fb2a7cb756..9329713b8e 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -201,7 +201,7 @@ module.exports = class Router webRouter.get "/project/:Project_id/doc/:doc_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApi webRouter.get "/project/:Project_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApiAndInjectUserDetails webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApi - privateApiRouter.post "/project/:Project_id/history/resync", AuthenticationController.httpAuth, HistoryController.resyncProject + privateApiRouter.post "/project/:Project_id/history/resync", AuthenticationController.httpAuth, HistoryController.resyncProjectHistory webRouter.get '/Project/:Project_id/download/zip', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectDownloadsController.downloadProject webRouter.get '/project/download/zip', AuthorizationMiddlewear.ensureUserCanReadMultipleProjects, ProjectDownloadsController.downloadMultipleProjects From af9cc2841926eb1e5e32367d8e2c72b9abeb958d Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 7 Mar 2018 16:08:58 +0000 Subject: [PATCH 7/9] additional logging --- .../Features/DocumentUpdater/DocumentUpdaterHandler.coffee | 4 ++++ .../app/coffee/Features/Project/ProjectEntityHandler.coffee | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index cfe922922c..966996a364 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -206,6 +206,7 @@ module.exports = DocumentUpdaterHandler = callback new Error("doc updater returned a non-success status code: #{res.statusCode}") resyncProjectHistory: (project_id, docs, files, callback) -> + logger.info {project_id}, "resyncing project in doc updater" request.post url: "#{settings.apis.documentupdater.url}/project/#{project_id}/history/resync" json: { docs, files } @@ -216,6 +217,9 @@ module.exports = DocumentUpdaterHandler = else if res.statusCode >= 200 and res.statusCode < 300 logger.log {project_id}, "resynced project in doc updater" callback() + else + logger.error {project_id, doc_id}, "doc updater returned a non-success status code: #{res.statusCode}" + callback new Error("doc updater returned a non-success status code: #{res.statusCode}") updateProjectStructure : (project_id, userId, changes, callback = (error) ->)-> return callback() if !settings.apis.project_history?.sendProjectStructureOps diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index e2645f1618..85e41292da 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -52,7 +52,7 @@ module.exports = ProjectEntityHandler = self = callback null, files getAllEntitiesFromProject: (project, callback) -> - logger.log project:project, "getting all files for project" + logger.log project:project, "getting all entities for project" self._getAllFoldersFromProject project, (err, folders = {}) -> return callback(err) if err? docs = [] From 75f5fbcdbfa31dff651517dc53dc98f3d5faa360 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 9 Mar 2018 10:23:48 +0000 Subject: [PATCH 8/9] refactor DocumentUpdaterHandler --- .../DocumentUpdaterHandler.coffee | 229 ++++++------------ .../DocumentUpdaterHandlerTests.coffee | 177 ++++++++------ 2 files changed, 170 insertions(+), 236 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index 966996a364..c9e51484f9 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -5,26 +5,15 @@ _ = require 'underscore' async = require 'async' logger = require('logger-sharelatex') metrics = require('metrics-sharelatex') -FileStoreHandler = require("../FileStore/FileStoreHandler") Project = require("../../models/Project").Project -ProjectGetter = require "../Project/ProjectGetter" module.exports = DocumentUpdaterHandler = flushProjectToMongo: (project_id, callback = (error) ->)-> logger.log project_id:project_id, "flushing project from document updater" - timer = new metrics.Timer("flushing.mongo.project") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}/flush" - request.post url, (error, res, body)-> - if error? - logger.error err: error, project_id: project_id, "error flushing project from document updater" - return callback(error) - else if res.statusCode >= 200 and res.statusCode < 300 - logger.log project_id: project_id, "flushed project 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, "document updater returned failure status code: #{res.statusCode}" - return callback(error) + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}/flush" + method: "POST" + }, project_id, "flushing.mongo.project", callback flushMultipleProjectsToMongo: (project_ids, callback = (error) ->) -> jobs = [] @@ -35,94 +24,46 @@ module.exports = DocumentUpdaterHandler = async.series jobs, callback flushProjectToMongoAndDelete: (project_id, callback = ()->) -> - logger.log project_id:project_id, "deleting project from document updater" timer = new metrics.Timer("delete.mongo.project") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}" - request.del url, (error, res, body)-> - if error? - logger.error err: error, project_id: project_id, "error deleting project from document updater" - return callback(error) - else if res.statusCode >= 200 and res.statusCode < 300 - logger.log project_id: project_id, "deleted project 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, "document updater returned failure status code: #{res.statusCode}" - return callback(error) + url = "#{settings.apis.documentupdater.url}" + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}" + method: "DELETE" + }, project_id, "flushing.mongo.project", callback 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) - + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}/doc/#{doc_id}/flush" + method: "POST" + }, project_id, "flushing.mongo.doc", callback + deleteDoc : (project_id, doc_id, callback = ()->)-> logger.log project_id:project_id, doc_id: doc_id, "deleting doc from document updater" - timer = new metrics.Timer("delete.mongo.doc") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}" - request.del url, (error, res, body)-> - if error? - logger.error err: error, project_id: project_id, doc_id: doc_id, "error deleting 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, "deleted 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) + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}/doc/#{doc_id}" + method: "DELETE" + }, project_id, "delete.mongo.doc", callback getDocument: (project_id, doc_id, fromVersion, callback = (error, doclines, version, ranges, ops) ->) -> - timer = new metrics.Timer("get-document") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}?fromVersion=#{fromVersion}" logger.log project_id:project_id, doc_id: doc_id, "getting doc from document updater" - request.get url, (error, res, body)-> - timer.done() - if error? - logger.error err:error, url:url, project_id:project_id, doc_id:doc_id, "error getting doc from doc updater" - return callback(error) - if res.statusCode >= 200 and res.statusCode < 300 - logger.log project_id:project_id, doc_id:doc_id, "got doc from document document updater" - try - body = JSON.parse(body) - catch error - return callback(error) - callback null, body.lines, body.version, body.ranges, body.ops - else - logger.error project_id:project_id, doc_id:doc_id, url: url, "doc updater returned a non-success status code: #{res.statusCode}" - callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}/doc/#{doc_id}?fromVersion=#{fromVersion}" + json: true + }, project_id, "get-document", (error, doc) -> + return callback(error) if error? + callback null, doc.lines, doc.version, doc.ranges, doc.ops setDocument : (project_id, doc_id, user_id, docLines, source, callback = (error) ->)-> - timer = new metrics.Timer("set-document") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}" - body = - url: url + logger.log project_id:project_id, doc_id: doc_id, source: source, user_id: user_id, "setting doc in document updater" + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}/doc/#{doc_id}" + method: "POST" json: lines: docLines source: source user_id: user_id - logger.log project_id:project_id, doc_id: doc_id, source: source, user_id: user_id, "setting doc in document updater" - request.post body, (error, res, body)-> - timer.done() - if error? - logger.error err:error, url:url, project_id:project_id, doc_id:doc_id, "error setting doc in doc updater" - return callback(error) - if res.statusCode >= 200 and res.statusCode < 300 - logger.log project_id: project_id, doc_id: doc_id, "set doc in document updater" - return callback(null) - else - logger.error project_id:project_id, doc_id:doc_id, url: url, "doc updater returned a non-success status code: #{res.statusCode}" - callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + }, project_id, "set-document", callback getProjectDocsIfMatch: (project_id, projectStateHash, callback = (error, docs) ->) -> # If the project state hasn't changed, we can get all the latest @@ -155,74 +96,42 @@ module.exports = DocumentUpdaterHandler = callback new Error("doc updater returned a non-success status code: #{res.statusCode}") clearProjectState: (project_id, callback = (error) ->) -> - timer = new metrics.Timer("clear-project-state") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}/clearState" logger.log project_id:project_id, "clearing project state from document updater" - request.post url, (error, res)-> - timer.done() - if error? - logger.error err:error, url:url, project_id:project_id, "error clearing project state from doc updater" - return callback(error) - else if res.statusCode is 200 - logger.log project_id:project_id, "cleared project state from doc updater" - callback() - else - logger.error project_id:project_id, url: url, "doc updater returned a non-success status code: #{res.statusCode}" - callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}/clearState" + method: "POST" + }, project_id, "clear-project-state", callback acceptChanges: (project_id, doc_id, change_ids = [], callback = (error) ->) -> - timer = new metrics.Timer("accept-changes") - reqSettings = - url: "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}/change/accept" + logger.log {project_id, doc_id }, "accepting #{ change_ids.length } changes" + + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}/doc/#{doc_id}/change/accept" json: change_ids: change_ids - logger.log {project_id, doc_id }, "accepting #{ change_ids.length } changes" - request.post reqSettings, (error, res, body)-> - timer.done() - if error? - logger.error {err:error, project_id, doc_id }, "error accepting #{ change_ids.length } changes in doc updater" - return callback(error) - if res.statusCode >= 200 and res.statusCode < 300 - logger.log {project_id, doc_id }, "accepted #{ change_ids.length } changes in document updater" - return callback(null) - else - logger.error {project_id, doc_id }, "doc updater returned a non-success status code: #{res.statusCode}" - callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + method: "POST" + }, project_id, "accept-changes", callback deleteThread: (project_id, doc_id, thread_id, callback = (error) ->) -> timer = new metrics.Timer("delete-thread") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}/comment/#{thread_id}" logger.log {project_id, doc_id, thread_id}, "deleting comment range in document updater" - request.del url, (error, res, body)-> - timer.done() - if error? - logger.error {err:error, project_id, doc_id, thread_id}, "error deleting comment range in doc updater" - return callback(error) - if res.statusCode >= 200 and res.statusCode < 300 - logger.log {project_id, doc_id, thread_id}, "deleted comment rangee in document updater" - return callback(null) - else - logger.error {project_id, doc_id, thread_id}, "doc updater returned a non-success status code: #{res.statusCode}" - callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}/doc/#{doc_id}/comment/#{thread_id}" + method: "DELETE" + }, project_id, "delete-thread", callback resyncProjectHistory: (project_id, docs, files, callback) -> logger.info {project_id}, "resyncing project in doc updater" - request.post - url: "#{settings.apis.documentupdater.url}/project/#{project_id}/history/resync" + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}/history/resync" json: { docs, files } - , (error, res, body) -> - if error? - logger.error {error, project_id}, "error resyncing project in doc updater" - callback(error) - else if res.statusCode >= 200 and res.statusCode < 300 - logger.log {project_id}, "resynced project in doc updater" - callback() - else - logger.error {project_id, doc_id}, "doc updater returned a non-success status code: #{res.statusCode}" - callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + method: "POST" + }, project_id, "resync-project-history", callback updateProjectStructure : (project_id, userId, changes, callback = (error) ->)-> return callback() if !settings.apis.project_history?.sendProjectStructureOps + Project.findOne {_id: project_id}, {version:true}, (err, currentProject) -> return callback(err) if err? return callback new Error("project not found") if !currentProject? @@ -230,25 +139,31 @@ module.exports = DocumentUpdaterHandler = docUpdates = DocumentUpdaterHandler._getUpdates('doc', changes.oldDocs, changes.newDocs) fileUpdates = DocumentUpdaterHandler._getUpdates('file', changes.oldFiles, changes.newFiles) - timer = new metrics.Timer("set-document") - url = "#{settings.apis.documentupdater.url}/project/#{project_id}" - body = - url: url - json: { docUpdates, fileUpdates, userId, version: currentProject.version } - return callback() if (docUpdates.length + fileUpdates.length) < 1 - request.post body, (error, res, body)-> - timer.done() - if error? - logger.error {error, url, project_id}, "error update project structure in doc updater" - callback(error) - else if res.statusCode >= 200 and res.statusCode < 300 - logger.log {project_id}, "updated project structure in doc updater" - callback(null) - else - logger.error {project_id, url}, "doc updater returned a non-success status code: #{res.statusCode}" - callback new Error("doc updater returned a non-success status code: #{res.statusCode}") + DocumentUpdaterHandler._makeRequest { + path: "/project/#{project_id}" + json: { docUpdates, fileUpdates, userId, version: currentProject.version } + method: "POST" + }, project_id, "update-project-structure", callback + + _makeRequest: (options, project_id, metricsKey, callback) -> + timer = new metrics.Timer(metricsKey) + request { + url: "#{settings.apis.documentupdater.url}#{options.path}" + json: options.json + method: options.method || "GET" + }, (error, res, body)-> + timer.done() + if error? + logger.error {error, project_id}, "error making request to document updater" + callback error + else if res.statusCode >= 200 and res.statusCode < 300 + callback null, body + else + error = new Error("document updater returned a failure status code: #{res.statusCode}") + logger.error {error, project_id}, "document updater returned failure status code: #{res.statusCode}" + callback error _getUpdates: (entityType, oldEntities, newEntities) -> oldEntities ||= [] diff --git a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 8db9ed4830..47010c2b9d 100644 --- a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -19,7 +19,7 @@ describe 'DocumentUpdaterHandler', -> @project = _id: @project_id - @request = {} + @request = sinon.stub() @projectEntityHandler = {} @settings = apis: @@ -43,19 +43,21 @@ describe 'DocumentUpdaterHandler', -> describe 'flushProjectToMongo', -> describe "successfully", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") + @request.callsArgWith(1, null, {statusCode: 204}, "") @handler.flushProjectToMongo @project_id, @callback it 'should flush the document from the document updater', -> - url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/flush" - @request.post.calledWith(url).should.equal true + @request.calledWithMatch( + url: "#{@settings.apis.documentupdater.url}/project/#{@project_id}/flush" + method: "POST" + ).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) + @request.callsArgWith(1, @error = new Error("something went wrong"), null, null) @handler.flushProjectToMongo @project_id, @callback it "should return an error to the callback", -> @@ -63,7 +65,7 @@ describe 'DocumentUpdaterHandler', -> describe "when the document updater returns a failure error code", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @request.callsArgWith(1, null, { statusCode: 500 }, "") @handler.flushProjectToMongo @project_id, @callback it "should return the callback with an error", -> @@ -74,19 +76,21 @@ describe 'DocumentUpdaterHandler', -> describe 'flushProjectToMongoAndDelete', -> describe "successfully", -> beforeEach -> - @request.del = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") + @request.callsArgWith(1, null, {statusCode: 204}, "") @handler.flushProjectToMongoAndDelete @project_id, @callback it 'should delete the project from the document updater', -> - url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}" - @request.del.calledWith(url).should.equal true + @request.calledWithMatch( + url: "#{@settings.apis.documentupdater.url}/project/#{@project_id}" + method: "DELETE" + ).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.del = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null) + @request.callsArgWith(1, @error = new Error("something went wrong"), null, null) @handler.flushProjectToMongoAndDelete @project_id, @callback it "should return an error to the callback", -> @@ -94,7 +98,7 @@ describe 'DocumentUpdaterHandler', -> describe "when the document updater returns a failure error code", -> beforeEach -> - @request.del = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @request.callsArgWith(1, null, { statusCode: 500 }, "") @handler.flushProjectToMongoAndDelete @project_id, @callback it "should return the callback with an error", -> @@ -105,19 +109,21 @@ describe 'DocumentUpdaterHandler', -> describe 'flushDocToMongo', -> describe "successfully", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") + @request.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 + @request.calledWithMatch( + url: "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}/flush" + method: "POST" + ).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) + @request.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", -> @@ -125,7 +131,7 @@ describe 'DocumentUpdaterHandler', -> describe "when the document updater returns a failure error code", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @request.callsArgWith(1, null, { statusCode: 500 }, "") @handler.flushDocToMongo @project_id, @doc_id, @callback it "should return the callback with an error", -> @@ -136,19 +142,21 @@ describe 'DocumentUpdaterHandler', -> describe "deleteDoc", -> describe "successfully", -> beforeEach -> - @request.del = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") + @request.callsArgWith(1, null, {statusCode: 204}, "") @handler.deleteDoc @project_id, @doc_id, @callback it 'should delete the document from the document updater', -> - url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}" - @request.del.calledWith(url).should.equal true + @request.calledWithMatch( + url: "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}" + method: "DELETE" + ).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.del = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null) + @request.callsArgWith(1, @error = new Error("something went wrong"), null, null) @handler.deleteDoc @project_id, @doc_id, @callback it "should return an error to the callback", -> @@ -156,7 +164,7 @@ describe 'DocumentUpdaterHandler', -> describe "when the document updater returns a failure error code", -> beforeEach -> - @request.del = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @request.callsArgWith(1, null, { statusCode: 500 }, "") @handler.deleteDoc @project_id, @doc_id, @callback it "should return the callback with an error", -> @@ -170,27 +178,25 @@ describe 'DocumentUpdaterHandler', -> describe "successfully", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") + @request.callsArgWith(1, null, {statusCode: 204}, "") @handler.setDocument @project_id, @doc_id, @user_id, @lines, @source, @callback it 'should set the document in the document updater', -> - url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}" - @request.post - .calledWith({ - url: url - json: - lines: @lines - source: @source - user_id: @user_id - }) - .should.equal true + @request.calledWith( + url: "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}" + json: + lines: @lines + source: @source + user_id: @user_id + method: "POST" + ).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) + @request.callsArgWith(1, @error = new Error("something went wrong"), null, null) @handler.setDocument @project_id, @doc_id, @user_id, @lines, @source, @callback it "should return an error to the callback", -> @@ -198,7 +204,7 @@ describe 'DocumentUpdaterHandler', -> describe "when the document updater returns a failure error code", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @request.callsArgWith(1, null, { statusCode: 500 }, "") @handler.setDocument @project_id, @doc_id, @user_id, @lines, @source, @callback it "should return the callback with an error", -> @@ -209,25 +215,28 @@ describe 'DocumentUpdaterHandler', -> describe "getDocument", -> describe "successfully", -> beforeEach -> - @body = JSON.stringify + @body = lines: @lines version: @version ops: @ops = ["mock-op-1", "mock-op-2"] ranges: @ranges = {"mock":"ranges"} @fromVersion = 2 - @request.get = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body) + @request.callsArgWith(1, null, {statusCode: 200}, @body) @handler.getDocument @project_id, @doc_id, @fromVersion, @callback it 'should get the document from the document updater', -> - url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}?fromVersion=#{@fromVersion}" - @request.get.calledWith(url).should.equal true + @request.calledWith( + url: "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}?fromVersion=#{@fromVersion}" + method: "GET" + json: true + ).should.equal true it "should call the callback with the lines and version", -> @callback.calledWith(null, @lines, @version, @ranges, @ops).should.equal true describe "when the document updater API returns an error", -> beforeEach -> - @request.get = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null) + @request.callsArgWith(1, @error = new Error("something went wrong"), null, null) @handler.getDocument @project_id, @doc_id, @fromVersion, @callback it "should return an error to the callback", -> @@ -235,7 +244,7 @@ describe 'DocumentUpdaterHandler', -> describe "when the document updater returns a failure error code", -> beforeEach -> - @request.get = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @request.callsArgWith(1, null, { statusCode: 500 }, "") @handler.getDocument @project_id, @doc_id, @fromVersion, @callback it "should return the callback with an error", -> @@ -258,7 +267,7 @@ describe 'DocumentUpdaterHandler', -> @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body) @handler.getProjectDocsIfMatch @project_id, @project_state_hash, @callback - it 'should get the documenst from the document updater', -> + it 'should get the documents from the document updater', -> url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/get_and_flush_if_old?state=#{@project_state_hash}" @request.post.calledWith(url).should.equal true @@ -283,36 +292,37 @@ describe 'DocumentUpdaterHandler', -> .alwaysCalledWithExactly() .should.equal true - describe "clearProjectState", -> describe "successfully", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}) + @request.callsArgWith(1, null, {statusCode: 200}) @handler.clearProjectState @project_id, @callback it 'should clear the project state from the document updater', -> - url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/clearState" - @request.post.calledWith(url).should.equal true + @request.calledWithMatch( + url: "#{@settings.apis.documentupdater.url}/project/#{@project_id}/clearState" + method: "POST" + ).should.equal true it "should call the callback", -> - @callback.calledWithExactly().should.equal true + @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.getProjectDocsIfMatch @project_id, @project_state_hash, @callback + @request.callsArgWith(1, @error = new Error("something went wrong"), null, null) + @handler.clearProjectState @project_id, @callback it "should return an error to the callback", -> @callback.calledWith(@error).should.equal true - describe "when the document updater returns a conflict error code", -> + describe "when the document updater returns an error code", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, { statusCode: 409 }, "Conflict") - @handler.getProjectDocsIfMatch @project_id, @project_state_hash, @callback + @request.callsArgWith(1, null, { statusCode: 500 }, null) + @handler.clearProjectState @project_id, @callback it "should return the callback with no documents", -> @callback - .alwaysCalledWithExactly() + .calledWith(new Error("doc updater returned failure status code: 500")) .should.equal true @@ -322,22 +332,23 @@ describe 'DocumentUpdaterHandler', -> describe "successfully", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body) + @request.callsArgWith(1, null, {statusCode: 200}, @body) @handler.acceptChanges @project_id, @doc_id, [ @change_id ], @callback it 'should accept the change in the document updater', -> - req = + @request.calledWith( url: "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}/change/accept" json: change_ids: [ @change_id ] - @request.post.calledWith(req).should.equal true + method: "POST" + ).should.equal true it "should call the callback", -> @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) + @request.callsArgWith(1, @error = new Error("something went wrong"), null, null) @handler.acceptChanges @project_id, @doc_id, [ @change_id ], @callback it "should return an error to the callback", -> @@ -345,7 +356,7 @@ describe 'DocumentUpdaterHandler', -> describe "when the document updater returns a failure error code", -> beforeEach -> - @request.post = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @request.callsArgWith(1, null, { statusCode: 500 }, "") @handler.acceptChanges @project_id, @doc_id, [ @change_id ], @callback it "should return the callback with an error", -> @@ -359,19 +370,21 @@ describe 'DocumentUpdaterHandler', -> describe "successfully", -> beforeEach -> - @request.del = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body) + @request.callsArgWith(1, null, {statusCode: 200}, @body) @handler.deleteThread @project_id, @doc_id, @thread_id, @callback it 'should delete the thread in the document updater', -> - url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}/comment/#{@thread_id}" - @request.del.calledWith(url).should.equal true + @request.calledWithMatch( + url: "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}/comment/#{@thread_id}" + method: "DELETE" + ).should.equal true it "should call the callback", -> @callback.calledWith(null).should.equal true describe "when the document updater API returns an error", -> beforeEach -> - @request.del = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null) + @request.callsArgWith(1, @error = new Error("something went wrong"), null, null) @handler.deleteThread @project_id, @doc_id, @thread_id, @callback it "should return an error to the callback", -> @@ -379,7 +392,7 @@ describe 'DocumentUpdaterHandler', -> describe "when the document updater returns a failure error code", -> beforeEach -> - @request.del = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @request.callsArgWith(1, null, { statusCode: 500 }, "") @handler.deleteThread @project_id, @doc_id, @thread_id, @callback it "should return the callback with an error", -> @@ -396,12 +409,10 @@ describe 'DocumentUpdaterHandler', -> describe "with project history disabled", -> beforeEach -> @settings.apis.project_history.sendProjectStructureOps = false - @request.post = sinon.stub() - @handler.updateProjectStructure @project_id, @user_id, {}, @callback it 'does not make a web request', -> - @request.post.called.should.equal false + @request.called.should.equal false it 'calls the callback', -> @callback.called.should.equal true @@ -410,7 +421,7 @@ describe 'DocumentUpdaterHandler', -> beforeEach -> @settings.apis.project_history.sendProjectStructureOps = true @url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}" - @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") + @request.callsArgWith(1, null, {statusCode: 204}, "") describe "when an entity has changed name", -> it 'should send the structure update to the document updater', (done) -> @@ -435,9 +446,12 @@ describe 'DocumentUpdaterHandler', -> ] @handler.updateProjectStructure @project_id, @user_id, @changes, () => - @request.post - .calledWith(url: @url, json: {docUpdates, fileUpdates: [], userId: @user_id, version:@version}) - .should.equal true + @request.calledWith( + url: @url, + method: "POST" + json: {docUpdates, fileUpdates: [], userId: @user_id, @version} + ) + .should.equal true done() describe "when a doc has been added", -> @@ -455,9 +469,11 @@ describe 'DocumentUpdaterHandler', -> ] @handler.updateProjectStructure @project_id, @user_id, @changes, () => - @request.post - .calledWith(url: @url, json: {docUpdates, fileUpdates: [], userId: @user_id, version:@version}) - .should.equal true + @request.calledWith( + url: @url + method: "POST" + json: {docUpdates, fileUpdates: [], userId: @user_id, @version} + ).should.equal true done() describe "when a file has been added", -> @@ -475,9 +491,11 @@ describe 'DocumentUpdaterHandler', -> ] @handler.updateProjectStructure @project_id, @user_id, @changes, () => - @request.post - .calledWith(url: @url, json: {docUpdates: [], fileUpdates, userId: @user_id, version:@version}) - .should.equal true + @request.calledWith( + url: @url + method: "POST" + json: {docUpdates: [], fileUpdates, userId: @user_id, @version} + ).should.equal true done() describe "when an entity has been deleted", -> @@ -494,8 +512,9 @@ describe 'DocumentUpdaterHandler', -> ] @handler.updateProjectStructure @project_id, @user_id, @changes, () => - @request.post - .calledWith(url: @url, json: {docUpdates, fileUpdates: [], userId: @user_id, version:@version}) - .should.equal true + @request.calledWith( + url: @url + method: "POST" + json: {docUpdates, fileUpdates: [], userId: @user_id, @version} + ).should.equal true done() - From a7945e9b960e9822b25499b26dd2a37c8d951447 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 9 Mar 2018 11:05:17 +0000 Subject: [PATCH 9/9] extra logging for DocumentUpdaterHandler --- services/web/app.coffee | 2 ++ .../DocumentUpdaterHandler.coffee | 3 ++- .../infrastructure/LoggerSerializers.coffee | 16 +++++++++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/services/web/app.coffee b/services/web/app.coffee index d8590689b3..5eb36f6439 100644 --- a/services/web/app.coffee +++ b/services/web/app.coffee @@ -2,6 +2,8 @@ Settings = require('settings-sharelatex') logger = require 'logger-sharelatex' logger.initialize("web-sharelatex") logger.logger.serializers.user = require("./app/js/infrastructure/LoggerSerializers").user +logger.logger.serializers.docs = require("./app/js/infrastructure/LoggerSerializers").docs +logger.logger.serializers.files = require("./app/js/infrastructure/LoggerSerializers").files logger.logger.serializers.project = require("./app/js/infrastructure/LoggerSerializers").project if Settings.sentry?.dsn? logger.initializeErrorReporting(Settings.sentry.dsn) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index c9e51484f9..e340187dbb 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -122,7 +122,7 @@ module.exports = DocumentUpdaterHandler = }, project_id, "delete-thread", callback resyncProjectHistory: (project_id, docs, files, callback) -> - logger.info {project_id}, "resyncing project in doc updater" + logger.info {project_id, docs, files}, "resyncing project history in doc updater" DocumentUpdaterHandler._makeRequest { path: "/project/#{project_id}/history/resync" json: { docs, files } @@ -141,6 +141,7 @@ module.exports = DocumentUpdaterHandler = return callback() if (docUpdates.length + fileUpdates.length) < 1 + logger.log {project_id}, "updating project structure in doc updater" DocumentUpdaterHandler._makeRequest { path: "/project/#{project_id}" json: { docUpdates, fileUpdates, userId, version: currentProject.version } diff --git a/services/web/app/coffee/infrastructure/LoggerSerializers.coffee b/services/web/app/coffee/infrastructure/LoggerSerializers.coffee index 7bd90c3bf5..c45f0f0bb8 100644 --- a/services/web/app/coffee/infrastructure/LoggerSerializers.coffee +++ b/services/web/app/coffee/infrastructure/LoggerSerializers.coffee @@ -10,7 +10,7 @@ module.exports = first_name: user.name last_name: user.name } - + project: (project) -> if !project? return null @@ -20,3 +20,17 @@ module.exports = id: project._id name: project.name } + + docs: (docs) -> + docs.map (doc) -> + { + path: doc.path + id: doc.doc + } + + files: (files) -> + files.map (file) -> + { + path: file.path + id: file.file + }