From 3ce57ed44232d5433494b5b0c49b240171ac1538 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 30 Nov 2020 13:02:12 +0000 Subject: [PATCH] Merge pull request #3014 from overleaf/ns-delete-project-history-cleanup Decaf cleanup HistoryController GitOrigin-RevId: e5df4cde30d8b9e65062e1484699326e96c4eb92 --- .../src/Features/History/HistoryController.js | 250 +++++++----------- .../src/History/HistoryControllerTests.js | 2 +- 2 files changed, 102 insertions(+), 150 deletions(-) diff --git a/services/web/app/src/Features/History/HistoryController.js b/services/web/app/src/Features/History/HistoryController.js index adb5bcef1c..69addedd70 100644 --- a/services/web/app/src/Features/History/HistoryController.js +++ b/services/web/app/src/Features/History/HistoryController.js @@ -1,22 +1,5 @@ -/* eslint-disable - camelcase, - handle-callback-err, - max-len, - no-undef, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS103: Rewrite code to no longer use __guard__ - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ let HistoryController const OError = require('@overleaf/o-error') -const _ = require('lodash') const async = require('async') const logger = require('logger-sharelatex') const request = require('request') @@ -32,45 +15,32 @@ const { pipeline } = require('stream') module.exports = HistoryController = { selectHistoryApi(req, res, next) { - if (next == null) { - next = function(error) {} - } - const project_id = req.params != null ? req.params.Project_id : undefined + const { Project_id: projectId } = req.params // find out which type of history service this project uses - return ProjectDetailsHandler.getDetails(project_id, function(err, project) { - if (err != null) { + ProjectDetailsHandler.getDetails(projectId, function(err, project) { + if (err) { return next(err) } - const history = - project.overleaf != null ? project.overleaf.history : undefined - if ( - (history != null ? history.id : undefined) != null && - (history != null ? history.display : undefined) - ) { + const history = project.overleaf && project.overleaf.history + if (history && history.id && history.display) { req.useProjectHistory = true } else { req.useProjectHistory = false } - return next() + next() }) }, ensureProjectHistoryEnabled(req, res, next) { - if (next == null) { - next = function(error) {} - } - if (req.useProjectHistory != null) { - return next() + if (req.useProjectHistory) { + next() } else { - return res.sendStatus(404) + res.sendStatus(404) } }, proxyToHistoryApi(req, res, next) { - if (next == null) { - next = function(error) {} - } - const user_id = AuthenticationController.getLoggedInUserId(req) + const userId = AuthenticationController.getLoggedInUserId(req) const url = HistoryController.buildHistoryServiceUrl(req.useProjectHistory) + req.url @@ -78,41 +48,38 @@ module.exports = HistoryController = { url, method: req.method, headers: { - 'X-User-Id': user_id + 'X-User-Id': userId } }) getReq.pipe(res) - return getReq.on('error', function(error) { - logger.warn({ url, err: error }, 'history API error') - return next(error) + getReq.on('error', function(err) { + logger.warn({ url, err }, 'history API error') + next(err) }) }, proxyToHistoryApiAndInjectUserDetails(req, res, next) { - if (next == null) { - next = function(error) {} - } - const user_id = AuthenticationController.getLoggedInUserId(req) + const userId = AuthenticationController.getLoggedInUserId(req) const url = HistoryController.buildHistoryServiceUrl(req.useProjectHistory) + req.url - return HistoryController._makeRequest( + HistoryController._makeRequest( { url, method: req.method, json: true, headers: { - 'X-User-Id': user_id + 'X-User-Id': userId } }, - function(error, body) { - if (error != null) { - return next(error) + function(err, body) { + if (err) { + return next(err) } - return HistoryManager.injectUserDetails(body, function(error, data) { - if (error != null) { - return next(error) + HistoryManager.injectUserDetails(body, function(err, data) { + if (err) { + return next(err) } - return res.json(data) + res.json(data) }) } ) @@ -129,37 +96,32 @@ module.exports = HistoryController = { }, resyncProjectHistory(req, res, next) { - if (next == null) { - next = function(error) {} - } - const project_id = req.params.Project_id - return ProjectEntityUpdateHandler.resyncProjectHistory(project_id, function( - error - ) { - if (error instanceof Errors.ProjectHistoryDisabledError) { + const projectId = req.params.Project_id + ProjectEntityUpdateHandler.resyncProjectHistory(projectId, function(err) { + if (err instanceof Errors.ProjectHistoryDisabledError) { return res.sendStatus(404) } - if (error != null) { - return next(error) + if (err) { + return next(err) } - return res.sendStatus(204) + res.sendStatus(204) }) }, restoreFileFromV2(req, res, next) { - const { project_id } = req.params + const { project_id: projectId } = req.params const { version, pathname } = req.body - const user_id = AuthenticationController.getLoggedInUserId(req) - return RestoreManager.restoreFileFromV2( - user_id, - project_id, + const userId = AuthenticationController.getLoggedInUserId(req) + RestoreManager.restoreFileFromV2( + userId, + projectId, version, pathname, - function(error, entity) { - if (error != null) { - return next(error) + function(err, entity) { + if (err) { + return next(err) } - return res.json({ + res.json({ type: entity.type, id: entity._id }) @@ -168,19 +130,19 @@ module.exports = HistoryController = { }, restoreDocFromDeletedDoc(req, res, next) { - const { project_id, doc_id } = req.params + const { project_id: projectId, doc_id: docId } = req.params const { name } = req.body - const user_id = AuthenticationController.getLoggedInUserId(req) + const userId = AuthenticationController.getLoggedInUserId(req) if (name == null) { return res.sendStatus(400) // Malformed request } - return RestoreManager.restoreDocFromDeletedDoc( - user_id, - project_id, - doc_id, + RestoreManager.restoreDocFromDeletedDoc( + userId, + projectId, + docId, name, - (error, doc) => { - if (error != null) return next(error) + (err, doc) => { + if (err) return next(err) res.json({ doc_id: doc._id }) @@ -189,51 +151,48 @@ module.exports = HistoryController = { }, getLabels(req, res, next) { - const project_id = req.params.Project_id - const user_id = AuthenticationController.getLoggedInUserId(req) - return HistoryController._makeRequest( + const projectId = req.params.Project_id + HistoryController._makeRequest( { method: 'GET', - url: `${ - settings.apis.project_history.url - }/project/${project_id}/labels`, + url: `${settings.apis.project_history.url}/project/${projectId}/labels`, json: true }, - function(error, labels) { - if (error != null) { - return next(error) + function(err, labels) { + if (err) { + return next(err) } HistoryController._enrichLabels(labels, (err, labels) => { if (err) { return next(err) } - return res.json(labels) + res.json(labels) }) } ) }, createLabel(req, res, next) { - const project_id = req.params.Project_id + const projectId = req.params.Project_id const { comment, version } = req.body - const user_id = AuthenticationController.getLoggedInUserId(req) - return HistoryController._makeRequest( + const userId = AuthenticationController.getLoggedInUserId(req) + HistoryController._makeRequest( { method: 'POST', url: `${ settings.apis.project_history.url - }/project/${project_id}/user/${user_id}/labels`, + }/project/${projectId}/user/${userId}/labels`, json: { comment, version } }, - function(error, label) { - if (error != null) { - return next(error) + function(err, label) { + if (err) { + return next(err) } HistoryController._enrichLabel(label, (err, label) => { if (err) { return next(err) } - return res.json(label) + res.json(label) }) } ) @@ -293,7 +252,7 @@ module.exports = HistoryController = { if (user == null) { return 'Anonymous' } - if (user.name != null) { + if (user.name) { return user.name } let name = [user.first_name, user.last_name] @@ -303,67 +262,66 @@ module.exports = HistoryController = { if (name === '') { name = user.email.split('@')[0] } - if (name == null || name === '') { + if (!name) { return '?' } return name }, deleteLabel(req, res, next) { - const project_id = req.params.Project_id - const { label_id } = req.params - const user_id = AuthenticationController.getLoggedInUserId(req) - return HistoryController._makeRequest( + const { Project_id: projectId, label_id: labelId } = req.params + const userId = AuthenticationController.getLoggedInUserId(req) + HistoryController._makeRequest( { method: 'DELETE', url: `${ settings.apis.project_history.url - }/project/${project_id}/user/${user_id}/labels/${label_id}` + }/project/${projectId}/user/${userId}/labels/${labelId}` }, - function(error) { - if (error != null) { - return next(error) + function(err) { + if (err) { + return next(err) } - return res.sendStatus(204) + res.sendStatus(204) } ) }, _makeRequest(options, callback) { - return request(options, function(error, response, body) { - if (error != null) { - return callback(error) + return request(options, function(err, response, body) { + if (err) { + return callback(err) } if (response.statusCode >= 200 && response.statusCode < 300) { - return callback(null, body) + callback(null, body) } else { - error = new Error( + err = new Error( `history api responded with non-success code: ${response.statusCode}` ) - return callback(error) + callback(err) } }) }, downloadZipOfVersion(req, res, next) { - const { project_id, version } = req.params - return ProjectDetailsHandler.getDetails(project_id, function(err, project) { - if (err != null) { + const { project_id: projectId, version } = req.params + ProjectDetailsHandler.getDetails(projectId, function(err, project) { + if (err) { return next(err) } - const v1_id = __guard__( - project.overleaf != null ? project.overleaf.history : undefined, - x => x.id - ) - if (v1_id == null) { - logger.err( - { project_id, version }, + const v1Id = + project.overleaf && + project.overleaf.history && + project.overleaf.history.id + if (v1Id == null) { + logger.error( + { projectId, version }, 'got request for zip version of non-v1 history project' ) return res.sendStatus(402) } - return HistoryController._pipeHistoryZipToResponse( - v1_id, + HistoryController._pipeHistoryZipToResponse( + v1Id, version, `${project.name} (Version ${version})`, req, @@ -373,7 +331,7 @@ module.exports = HistoryController = { }) }, - _pipeHistoryZipToResponse(v1_project_id, version, name, req, res, next) { + _pipeHistoryZipToResponse(v1ProjectId, version, name, req, res, next) { if (req.aborted) { // client has disconnected -- skip project history api call and download return @@ -382,7 +340,7 @@ module.exports = HistoryController = { res.setTimeout(6 * 60 * 1000) const url = `${ settings.apis.v1_history.url - }/projects/${v1_project_id}/version/${version}/zip` + }/projects/${v1ProjectId}/version/${version}/zip` const options = { auth: { user: settings.apis.v1_history.user, @@ -392,10 +350,10 @@ module.exports = HistoryController = { method: 'post', url } - return request(options, function(err, response, body) { + request(options, function(err, response, body) { if (err) { OError.tag(err, 'history API error', { - v1_project_id, + v1ProjectId, version }) return next(err) @@ -407,7 +365,7 @@ module.exports = HistoryController = { let retryAttempt = 0 let retryDelay = 2000 // retry for about 6 minutes starting with short delay - return async.retry( + async.retry( 40, callback => setTimeout(function() { @@ -448,39 +406,33 @@ module.exports = HistoryController = { pipeline(response, res, err => { if (err) { logger.warn( - { err, v1_project_id, version, retryAttempt }, + { err, v1ProjectId, version, retryAttempt }, 'history s3 proxying error' ) } }) callback() }) - return getReq.on('error', function(err) { + getReq.on('error', function(err) { logger.warn( - { err, v1_project_id, version, retryAttempt }, + { err, v1ProjectId, version, retryAttempt }, 'history s3 download error' ) cleanupAbortTrigger() - return callback(err) + callback(err) }) }, retryDelay), function(err) { if (err) { OError.tag(err, 'history s3 download failed', { - v1_project_id, + v1ProjectId, version, retryAttempt }) - return next(err) + next(err) } } ) }) } } - -function __guard__(value, transform) { - return typeof value !== 'undefined' && value !== null - ? transform(value) - : undefined -} diff --git a/services/web/test/unit/src/History/HistoryControllerTests.js b/services/web/test/unit/src/History/HistoryControllerTests.js index eb821d4e64..dbb0215fc6 100644 --- a/services/web/test/unit/src/History/HistoryControllerTests.js +++ b/services/web/test/unit/src/History/HistoryControllerTests.js @@ -59,7 +59,7 @@ describe('HistoryController', function() { describe('selectHistoryApi', function() { beforeEach(function() { - this.req = { url: '/mock/url', method: 'POST' } + this.req = { url: '/mock/url', method: 'POST', params: {} } this.res = 'mock-res' return (this.next = sinon.stub()) })