From 9379cff89d3f5dff680478c12f4d8b1776f9eab8 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 9 Jan 2017 15:25:27 +0100 Subject: [PATCH] Add end point for accepting change in doc updater --- .../DocumentUpdaterHandler.coffee | 24 +++++++---- .../Features/Ranges/RangesController.coffee | 8 ++++ services/web/app/coffee/router.coffee | 1 + .../public/coffee/ide/editor/Document.coffee | 2 +- .../controllers/ReviewPanelController.coffee | 3 +- .../DocumentUpdaterHandlerTests.coffee | 38 +++++++++++++++- .../GetNumberOfDocsInMemoryTests.coffee | 43 ------------------- 7 files changed, 63 insertions(+), 56 deletions(-) delete mode 100644 services/web/test/UnitTests/coffee/DocumentUpdater/GetNumberOfDocsInMemoryTests.coffee diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index 9b3364f8ad..bb4922704f 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -137,15 +137,21 @@ module.exports = DocumentUpdaterHandler = 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}") - getNumberOfDocsInMemory : (callback)-> - request.get "#{settings.apis.documentupdater.url}/total", (err, req, body)-> - try - body = JSON.parse body - catch err - logger.err err:err, "error parsing response from doc updater about the total number of docs" - callback(err, body?.total) - - + acceptChange: (project_id, doc_id, change_id, callback = (error) ->) -> + timer = new metrics.Timer("accept-change") + url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}/change/#{change_id}/accept" + logger.log {project_id, doc_id, change_id}, "accepting change in document updater" + request.post url, (error, res, body)-> + timer.done() + if error? + logger.error {err:error, project_id, doc_id, change_id}, "error accepting change in doc updater" + return callback(error) + if res.statusCode >= 200 and res.statusCode < 300 + logger.log {project_id, doc_id, change_id}, "accepted change in document updater" + return callback(null) + else + logger.error {project_id, doc_id, change_id}, "doc updater returned a non-success status code: #{res.statusCode}" + callback new Error("doc updater returned a non-success status code: #{res.statusCode}") PENDINGUPDATESKEY = "PendingUpdates" DOCLINESKEY = "doclines" diff --git a/services/web/app/coffee/Features/Ranges/RangesController.coffee b/services/web/app/coffee/Features/Ranges/RangesController.coffee index 78457c2e64..0c740cf892 100644 --- a/services/web/app/coffee/Features/Ranges/RangesController.coffee +++ b/services/web/app/coffee/Features/Ranges/RangesController.coffee @@ -1,6 +1,7 @@ RangesManager = require "./RangesManager" logger = require "logger-sharelatex" UserInfoController = require "../User/UserInfoController" +DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" module.exports = RangesController = getAllRanges: (req, res, next) -> @@ -18,3 +19,10 @@ module.exports = RangesController = return next(error) if error? users = (UserInfoController.formatPersonalInfo(user) for user in users) res.json users + + acceptChange: (req, res, next) -> + {project_id, doc_id, change_id} = req.params + logger.log {project_id, doc_id, change_id}, "request to accept change" + DocumentUpdaterHandler.acceptChange project_id, doc_id, change_id, (error) -> + return next(error) if error? + res.send 204 diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 1332a2d7e5..1d032c74ac 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -179,6 +179,7 @@ module.exports = class Router webRouter.get "/project/:project_id/ranges", AuthorizationMiddlewear.ensureUserCanReadProject, RangesController.getAllRanges webRouter.get "/project/:project_id/ranges/users", AuthorizationMiddlewear.ensureUserCanReadProject, RangesController.getAllRangesUsers + webRouter.post "/project/:project_id/doc/:doc_id/changes/:change_id/accept", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, RangesController.acceptChange webRouter.get '/Project/:Project_id/download/zip', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectDownloadsController.downloadProject webRouter.get '/project/download/zip', AuthorizationMiddlewear.ensureUserCanReadMultipleProjects, ProjectDownloadsController.downloadMultipleProjects diff --git a/services/web/public/coffee/ide/editor/Document.coffee b/services/web/public/coffee/ide/editor/Document.coffee index 6984479b21..d037aeb165 100644 --- a/services/web/public/coffee/ide/editor/Document.coffee +++ b/services/web/public/coffee/ide/editor/Document.coffee @@ -340,7 +340,7 @@ define [ _applyOpsToRanges: (ops = [], oldSnapshot, msg) -> track_changes_as = null remote_op = msg? - if msg.meta?.tc? + if msg?.meta?.tc? @ranges.setIdSeed(msg.meta.tc) if remote_op and msg.meta?.tc track_changes_as = msg.meta.user_id diff --git a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee index 70edf939ad..5fdc14f5e5 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -200,8 +200,9 @@ define [ $scope.$broadcast "review-panel:recalculate-screen-positions" $scope.$broadcast "review-panel:layout" - + $scope.acceptChange = (entry_id) -> + $http.post "/project/#{$scope.project_id}/doc/#{$scope.editor.open_doc_id}/changes/#{entry_id}/accept", {_csrf: window.csrfToken} $scope.$broadcast "change:accept", entry_id $scope.rejectChange = (entry_id) -> diff --git a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index eca005f295..3bde5e991a 100644 --- a/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -8,8 +8,7 @@ path = require 'path' _ = require 'underscore' modulePath = path.join __dirname, '../../../../app/js/Features/DocumentUpdater/DocumentUpdaterHandler' -describe 'DocumentUpdaterHandler - Flushing documents :', -> - +describe 'DocumentUpdaterHandler', -> beforeEach -> @project_id = "project-id-923" @doc_id = "doc-id-394" @@ -296,3 +295,38 @@ describe 'DocumentUpdaterHandler - Flushing documents :', -> @callback .calledWith(new Error("doc updater returned failure status code: 500")) .should.equal true + + describe "acceptChange", -> + beforeEach -> + @change_id = "mock-change-id-1" + @callback = sinon.stub() + + describe "successfully", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body) + @handler.acceptChange @project_id, @doc_id, @change_id, @callback + + it 'should accept the change in the document updater', -> + url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}/change/#{@change_id}/accept" + @request.post.calledWith(url).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) + @handler.acceptChange @project_id, @doc_id, @change_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.acceptChange @project_id, @doc_id, @change_id, @callback + + it "should return the callback with an error", -> + @callback + .calledWith(new Error("doc updater returned failure status code: 500")) + .should.equal true diff --git a/services/web/test/UnitTests/coffee/DocumentUpdater/GetNumberOfDocsInMemoryTests.coffee b/services/web/test/UnitTests/coffee/DocumentUpdater/GetNumberOfDocsInMemoryTests.coffee deleted file mode 100644 index 213fd2257b..0000000000 --- a/services/web/test/UnitTests/coffee/DocumentUpdater/GetNumberOfDocsInMemoryTests.coffee +++ /dev/null @@ -1,43 +0,0 @@ -path = require("path") -sinon = require("sinon") -SandboxedModule = require('sandboxed-module') - -modulePath = path.join __dirname, '../../../../app/js/Features/DocumentUpdater/DocumentUpdaterHandler' - -describe "getNumberOfDocsInMemory", -> - beforeEach -> - @host = "doc.updater" - @noOfDocs = 42 - @callback = sinon.stub() - @DocumentUpdateHandler = SandboxedModule.require modulePath, requires: - "redis-sharelatex" : - createClient: () -> - auth:-> - "soa-req-id": null - "logger-sharelatex": @logger = - log: sinon.stub() - error: sinon.stub() - "../../infrastructure/Metrics" : @metrics - "../../Features/Project/ProjectLocator": @ProjectLocator = {} - "../../models/Project":Project:{} - "request" : defaults: () => @request = {} - "settings-sharelatex": - apis: documentupdater: url: @host - redis: web:{} - - - @request.get = sinon.stub().callsArgWith(1, null, {statusCode: 200}, JSON.stringify(total: @noOfDocs)) - @DocumentUpdateHandler.getNumberOfDocsInMemory @callback - - it "should call the doc updater", -> - @request.get - .calledWith("#{@host}/total") - .should.equal true - - it "should return the number of docs", -> - @callback - .calledWith(null, @noOfDocs) - .should.equal true - - -