From 04bb83f3de552b5c7640df58fb33a007f61f99b3 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 12 Jul 2018 09:28:24 +0100 Subject: [PATCH 1/2] add v2 history labels endpoints --- .../Features/History/HistoryController.coffee | 56 +++++++++++++++ services/web/app/coffee/router.coffee | 4 ++ .../test/acceptance/coffee/LabelsTests.coffee | 68 +++++++++++++++++++ .../helpers/MockProjectHistoryApi.coffee | 48 ++++++++++++- 4 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 services/web/test/acceptance/coffee/LabelsTests.coffee diff --git a/services/web/app/coffee/Features/History/HistoryController.coffee b/services/web/app/coffee/Features/History/HistoryController.coffee index 7d9d347531..4d4cb93b34 100644 --- a/services/web/app/coffee/Features/History/HistoryController.coffee +++ b/services/web/app/coffee/Features/History/HistoryController.coffee @@ -21,6 +21,17 @@ module.exports = HistoryController = req.useProjectHistory = false next() + ensureProjectHistoryEnabled: (req, res, next) -> + {project_id} = req.params + ProjectDetailsHandler.getDetails project_id, (err, project) -> + return next(err) if err? + historyEnabled = project.overleaf?.history?.id? + if historyEnabled + next() + else + logger.log {project_id}, "project history not enabled" + res.sendStatus(404) + proxyToHistoryApi: (req, res, next = (error) ->) -> user_id = AuthenticationController.getLoggedInUserId req url = HistoryController.buildHistoryServiceUrl(req.useProjectHistory) + req.url @@ -98,3 +109,48 @@ module.exports = HistoryController = doc_id: doc._id } + getLabels: (req, res, next) -> + {project_id} = req.params + user_id = AuthenticationController.getLoggedInUserId(req) + request.get { + url: "#{settings.apis.project_history.url}/project/#{project_id}/labels" + json: true + }, (error, response, body) -> + return next(error) if error? + if 200 <= response.statusCode < 300 + res.json body + else + error = new Error("history api responded with non-success code: #{response.statusCode}") + logger.error err: error, user_id: user_id, "error getting labels from project-history" + next(error) + + createLabel: (req, res, next) -> + {project_id} = req.params + {comment, version} = req.body + user_id = AuthenticationController.getLoggedInUserId(req) + request.post { + url: "#{settings.apis.project_history.url}/project/#{project_id}/user/#{user_id}/labels" + json: {comment, version} + }, (error, response, body) -> + return next(error) if error? + if 200 <= response.statusCode < 300 + res.json body + else + error = new Error("history api responded with non-success code: #{response.statusCode}") + logger.error err: error, user_id: user_id, "error creating label in project-history" + next(error) + + deleteLabel: (req, res, next) -> + {project_id, label_id} = req.params + user_id = AuthenticationController.getLoggedInUserId(req) + console.log "hof-debug DEL #{settings.apis.project_history.url}/project/#{project_id}/user/#{user_id}/labels/#{label_id}" + request.del { + url: "#{settings.apis.project_history.url}/project/#{project_id}/user/#{user_id}/labels/#{label_id}" + }, (error, response, body) -> + return next(error) if error? + if 200 <= response.statusCode < 300 + res.sendStatus 204 + else + error = new Error("history api responded with non-success code: #{response.statusCode}") + logger.error err: error, user_id: user_id, "error deleting label in project-history" + next(error) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 72a0ddebc4..d77af1fb96 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -239,6 +239,10 @@ module.exports = class Router webRouter.post "/project/:project_id/restore_file", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, HistoryController.restoreFileFromV2 privateApiRouter.post "/project/:Project_id/history/resync", AuthenticationController.httpAuth, HistoryController.resyncProjectHistory + webRouter.get "/project/:project_id/labels", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.ensureProjectHistoryEnabled, HistoryController.getLabels + webRouter.post "/project/:project_id/labels", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, HistoryController.ensureProjectHistoryEnabled, HistoryController.createLabel + webRouter.delete "/project/:project_id/labels/:label_id", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, HistoryController.ensureProjectHistoryEnabled, HistoryController.deleteLabel + webRouter.post '/project/:project_id/export/:brand_variation_id', AuthorizationMiddlewear.ensureUserCanAdminProject, ExportsController.exportProject webRouter.get '/project/:project_id/export/:export_id', AuthorizationMiddlewear.ensureUserCanAdminProject, ExportsController.exportStatus diff --git a/services/web/test/acceptance/coffee/LabelsTests.coffee b/services/web/test/acceptance/coffee/LabelsTests.coffee new file mode 100644 index 0000000000..8832f2a326 --- /dev/null +++ b/services/web/test/acceptance/coffee/LabelsTests.coffee @@ -0,0 +1,68 @@ +_ = require 'underscore' +{expect} = require 'chai' +{ObjectId} = require 'mongojs' +request = require './helpers/request' + +MockProjectHistoryApi = require './helpers/MockProjectHistoryApi' +User = require './helpers/User' + +describe 'Labels', -> + beforeEach (done) -> + @owner = new User() + @owner.login (error) => + throw error if error? + @owner.createProject 'example-project', {template: 'example'}, (error, @project_id) => + throw error if error? + done() + + afterEach -> + MockProjectHistoryApi.reset() + + it 'getting labels', (done) -> + label_id = new ObjectId().toString() + comment = 'a label comment' + version = 3 + MockProjectHistoryApi.addLabel @project_id, label_id, comment, version + + @owner.request { + method: 'GET' + url: "/project/#{@project_id}/labels" + json: true + }, (error, response, body) => + throw error if error? + expect(response.statusCode).to.equal 200 + expect(body).to.deep.equal [{ label_id, comment, version }] + done() + + it 'creating a label', (done) -> + comment = 'a label comment' + version = 3 + + @owner.request { + method: 'POST' + url: "/project/#{@project_id}/labels" + json: {comment, version} + }, (error, response, body) => + throw error if error? + expect(response.statusCode).to.equal 200 + {label_id} = body + expect( + MockProjectHistoryApi.getLabels(@project_id) + ).to.deep.equal [{label_id, comment, version} ] + done() + + it 'deleting a label', (done) -> + label_id = new ObjectId().toString() + comment = 'a label comment' + version = 3 + MockProjectHistoryApi.addLabel @project_id, label_id, comment, version + + @owner.request { + method: 'DELETE' + url: "/project/#{@project_id}/labels/#{label_id}" + json: true + }, (error, response, body) => + throw error if error? + expect(response.statusCode).to.equal 204 + expect(MockProjectHistoryApi.getLabels(@project_id)).to.deep.equal [] + done() diff --git a/services/web/test/acceptance/coffee/helpers/MockProjectHistoryApi.coffee b/services/web/test/acceptance/coffee/helpers/MockProjectHistoryApi.coffee index b7df202d72..f73b6007a4 100644 --- a/services/web/test/acceptance/coffee/helpers/MockProjectHistoryApi.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockProjectHistoryApi.coffee @@ -1,5 +1,8 @@ -express = require("express") +_ = require 'lodash' +express = require 'express' +bodyParser = require "body-parser" app = express() +{ObjectId} = require 'mongojs' module.exports = MockProjectHistoryApi = docs: {} @@ -8,12 +11,30 @@ module.exports = MockProjectHistoryApi = projectVersions: {} + labels: {} + addOldFile: (project_id, version, pathname, content) -> @oldFiles["#{project_id}:#{version}:#{pathname}"] = content setProjectVersion: (project_id, version) -> @projectVersions[project_id] = version + addLabel: (project_id, label_id, comment, version) -> + @labels[project_id] ?= {} + @labels[project_id][label_id] = {label_id,comment,version} + + deleteLabel: (project_id, label_id) -> + delete @labels[project_id][label_id] + + getLabels: (project_id) -> + return null unless @labels[project_id]? + _.values @labels[project_id] + + reset: () -> + @oldFiles = {} + @projectVersions = {} + @labels = {} + run: () -> app.post "/project", (req, res, next) => res.json project: id: 1 @@ -33,11 +54,34 @@ module.exports = MockProjectHistoryApi = else res.send 404 + app.get "/project/:project_id/labels", (req, res, next) => + {project_id} = req.params + labels = @getLabels project_id + if labels? + res.json labels + else + res.send 404 + + app.post "/project/:project_id/user/:user_id/labels", bodyParser.json(), (req, res, next) => + {project_id} = req.params + {comment, version} = req.body + label_id = new ObjectId().toString() + @addLabel project_id, label_id, comment, version + res.json {label_id, comment, version} + + app.delete "/project/:project_id/user/:user_id/labels/:label_id", (req, res, next) => + {project_id, label_id} = req.params + label = @labels[project_id]?[label_id] + if label? + @deleteLabel project_id, label_id + res.send 204 + else + res.send 404 + app.listen 3054, (error) -> throw error if error? .on "error", (error) -> console.error "error starting MockProjectHistoryApi:", error.message process.exit(1) - MockProjectHistoryApi.run() From f6def039bf6623ae75fed7e2736727fb32b16a33 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Mon, 16 Jul 2018 16:01:39 +0100 Subject: [PATCH 2/2] code review feedback --- .../Features/History/HistoryController.coffee | 79 ++++++++----------- services/web/app/coffee/router.coffee | 6 +- 2 files changed, 37 insertions(+), 48 deletions(-) diff --git a/services/web/app/coffee/Features/History/HistoryController.coffee b/services/web/app/coffee/Features/History/HistoryController.coffee index 4d4cb93b34..189e07b0f5 100644 --- a/services/web/app/coffee/Features/History/HistoryController.coffee +++ b/services/web/app/coffee/Features/History/HistoryController.coffee @@ -21,16 +21,12 @@ module.exports = HistoryController = req.useProjectHistory = false next() - ensureProjectHistoryEnabled: (req, res, next) -> - {project_id} = req.params - ProjectDetailsHandler.getDetails project_id, (err, project) -> - return next(err) if err? - historyEnabled = project.overleaf?.history?.id? - if historyEnabled - next() - else - logger.log {project_id}, "project history not enabled" - res.sendStatus(404) + ensureProjectHistoryEnabled: (req, res, next = (error) ->) -> + if req.useProjectHistory? + next() + else + logger.log {project_id}, "project history not enabled" + res.sendStatus(404) proxyToHistoryApi: (req, res, next = (error) ->) -> user_id = AuthenticationController.getLoggedInUserId req @@ -52,22 +48,17 @@ module.exports = HistoryController = user_id = AuthenticationController.getLoggedInUserId req url = HistoryController.buildHistoryServiceUrl(req.useProjectHistory) + req.url logger.log url: url, "proxying to history api" - request { + HistoryController._makeRequest { url: url method: req.method json: true headers: "X-User-Id": user_id - }, (error, response, body) -> + }, (error, body) -> return next(error) if error? - if 200 <= response.statusCode < 300 - HistoryManager.injectUserDetails body, (error, data) -> - return next(error) if error? - res.json data - else - error = new Error("history api responded with non-success code: #{response.statusCode}") - logger.error err: error, user_id: user_id, "error proxying request to history api" - next(error) + HistoryManager.injectUserDetails body, (error, data) -> + return next(error) if error? + res.json data buildHistoryServiceUrl: (useProjectHistory) -> # choose a history service, either document-level (trackchanges) @@ -110,47 +101,45 @@ module.exports = HistoryController = } getLabels: (req, res, next) -> - {project_id} = req.params + project_id = req.params.Project_id user_id = AuthenticationController.getLoggedInUserId(req) - request.get { + HistoryController._makeRequest { + method: "GET" url: "#{settings.apis.project_history.url}/project/#{project_id}/labels" json: true - }, (error, response, body) -> + }, (error, labels) -> return next(error) if error? - if 200 <= response.statusCode < 300 - res.json body - else - error = new Error("history api responded with non-success code: #{response.statusCode}") - logger.error err: error, user_id: user_id, "error getting labels from project-history" - next(error) + res.json labels createLabel: (req, res, next) -> - {project_id} = req.params + project_id = req.params.Project_id {comment, version} = req.body user_id = AuthenticationController.getLoggedInUserId(req) - request.post { + HistoryController._makeRequest { + method: "POST" url: "#{settings.apis.project_history.url}/project/#{project_id}/user/#{user_id}/labels" json: {comment, version} - }, (error, response, body) -> + }, (error, label) -> return next(error) if error? - if 200 <= response.statusCode < 300 - res.json body - else - error = new Error("history api responded with non-success code: #{response.statusCode}") - logger.error err: error, user_id: user_id, "error creating label in project-history" - next(error) + res.json label deleteLabel: (req, res, next) -> - {project_id, label_id} = req.params + project_id = req.params.Project_id + label_id = req.params.label_id user_id = AuthenticationController.getLoggedInUserId(req) - console.log "hof-debug DEL #{settings.apis.project_history.url}/project/#{project_id}/user/#{user_id}/labels/#{label_id}" - request.del { + HistoryController._makeRequest { + method: "DELETE" url: "#{settings.apis.project_history.url}/project/#{project_id}/user/#{user_id}/labels/#{label_id}" - }, (error, response, body) -> + }, (error) -> return next(error) if error? + res.sendStatus 204 + + _makeRequest: (options, callback) -> + request options, (error, response, body) -> + return callback(error) if error? if 200 <= response.statusCode < 300 - res.sendStatus 204 + callback(null, body) else error = new Error("history api responded with non-success code: #{response.statusCode}") - logger.error err: error, user_id: user_id, "error deleting label in project-history" - next(error) + logger.error err: error, "project-history api responded with non-success code: #{response.statusCode}" + callback(error) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index d77af1fb96..9ae764781c 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -239,9 +239,9 @@ module.exports = class Router webRouter.post "/project/:project_id/restore_file", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, HistoryController.restoreFileFromV2 privateApiRouter.post "/project/:Project_id/history/resync", AuthenticationController.httpAuth, HistoryController.resyncProjectHistory - webRouter.get "/project/:project_id/labels", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.ensureProjectHistoryEnabled, HistoryController.getLabels - webRouter.post "/project/:project_id/labels", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, HistoryController.ensureProjectHistoryEnabled, HistoryController.createLabel - webRouter.delete "/project/:project_id/labels/:label_id", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, HistoryController.ensureProjectHistoryEnabled, HistoryController.deleteLabel + webRouter.get "/project/:Project_id/labels", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.ensureProjectHistoryEnabled, HistoryController.getLabels + webRouter.post "/project/:Project_id/labels", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, HistoryController.selectHistoryApi, HistoryController.ensureProjectHistoryEnabled, HistoryController.createLabel + webRouter.delete "/project/:Project_id/labels/:label_id", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, HistoryController.selectHistoryApi, HistoryController.ensureProjectHistoryEnabled, HistoryController.deleteLabel webRouter.post '/project/:project_id/export/:brand_variation_id', AuthorizationMiddlewear.ensureUserCanAdminProject, ExportsController.exportProject webRouter.get '/project/:project_id/export/:export_id', AuthorizationMiddlewear.ensureUserCanAdminProject, ExportsController.exportStatus