From 42aea53307adf7bf38dcd5d42934939111a46f6f Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Thu, 10 Apr 2025 08:52:26 -0400 Subject: [PATCH] Merge pull request #24754 from overleaf/em-promisify-history-controller Promisify HistoryController GitOrigin-RevId: e1783acb8c7ba7e00b109a4f4a514973cc3163d2 --- .../src/Features/History/HistoryController.js | 876 ++++++++---------- .../src/History/HistoryControllerTests.js | 327 +++---- 2 files changed, 541 insertions(+), 662 deletions(-) diff --git a/services/web/app/src/Features/History/HistoryController.js b/services/web/app/src/Features/History/HistoryController.js index f6ca483dce..cc1bc67847 100644 --- a/services/web/app/src/Features/History/HistoryController.js +++ b/services/web/app/src/Features/History/HistoryController.js @@ -1,8 +1,17 @@ -let HistoryController +// @ts-check + +const { setTimeout } = require('timers/promises') +const { pipeline } = require('stream/promises') const OError = require('@overleaf/o-error') -const async = require('async') const logger = require('@overleaf/logger') -const request = require('request') +const { expressify } = require('@overleaf/promise-utils') +const { + fetchStream, + fetchStreamWithResponse, + fetchJson, + fetchNothing, + RequestFailedError, +} = require('@overleaf/fetch-utils') const settings = require('@overleaf/settings') const SessionManager = require('../Authentication/SessionManager') const UserGetter = require('../User/UserGetter') @@ -12,11 +21,8 @@ const HistoryManager = require('./HistoryManager') const ProjectDetailsHandler = require('../Project/ProjectDetailsHandler') const ProjectEntityUpdateHandler = require('../Project/ProjectEntityUpdateHandler') const RestoreManager = require('./RestoreManager') -const { pipeline } = require('stream') -const Stream = require('stream') const { prepareZipAttachment } = require('../../infrastructure/Response') const Features = require('../../infrastructure/Features') -const { expressify } = require('@overleaf/promise-utils') // Number of seconds after which the browser should send a request to revalidate // blobs @@ -26,6 +32,8 @@ const REVALIDATE_BLOB_AFTER_SECONDS = 86400 // 1 day // revalidating const STALE_WHILE_REVALIDATE_SECONDS = 365 * 86400 // 1 year +const MAX_HISTORY_ZIP_ATTEMPTS = 40 + async function getBlob(req, res) { await requestBlob('GET', req, res) } @@ -44,9 +52,9 @@ async function requestBlob(method, req, res) { } const range = req.get('Range') - let url, stream, source, contentLength + let stream, source, contentLength try { - ;({ url, stream, source, contentLength } = + ;({ stream, source, contentLength } = await HistoryManager.promises.requestBlobWithFallback( projectId, hash, @@ -65,14 +73,13 @@ async function requestBlob(method, req, res) { setBlobCacheHeaders(res, hash) try { - await Stream.promises.pipeline(stream, res) + await pipeline(stream, res) } catch (err) { // If the downstream request is cancelled, we get an // ERR_STREAM_PREMATURE_CLOSE, ignore these "errors". - if (err?.code === 'ERR_STREAM_PREMATURE_CLOSE') return - - logger.warn({ err, url, method, range }, 'streaming blob error') - throw err + if (!isPrematureClose(err)) { + throw err + } } } @@ -88,492 +95,393 @@ function setBlobCacheHeaders(res, etag) { res.set('ETag', etag) } -module.exports = HistoryController = { - getBlob: expressify(getBlob), - headBlob: expressify(headBlob), +async function proxyToHistoryApi(req, res, next) { + const userId = SessionManager.getLoggedInUserId(req.session) + const url = settings.apis.project_history.url + req.url - proxyToHistoryApi(req, res, next) { - const userId = SessionManager.getLoggedInUserId(req.session) - const url = settings.apis.project_history.url + req.url + const { stream, response } = await fetchStreamWithResponse(url, { + method: req.method, + headers: { 'X-User-Id': userId }, + }) - const getReq = request({ - url, - method: req.method, - headers: { - 'X-User-Id': userId, - }, - }) - pipeline(getReq, res, function (err) { + const contentType = response.headers.get('Content-Type') + const contentLength = response.headers.get('Content-Length') + if (contentType != null) { + res.set('Content-Type', contentType) + } + if (contentLength != null) { + res.set('Content-Length', contentLength) + } + + try { + await pipeline(stream, res) + } catch (err) { + // If the downstream request is cancelled, we get an + // ERR_STREAM_PREMATURE_CLOSE. + if (!isPrematureClose(err)) { + throw err + } + } +} + +async function proxyToHistoryApiAndInjectUserDetails(req, res, next) { + const userId = SessionManager.getLoggedInUserId(req.session) + const url = settings.apis.project_history.url + req.url + const body = await fetchJson(url, { + method: req.method, + headers: { 'X-User-Id': userId }, + }) + const data = await HistoryManager.promises.injectUserDetails(body) + res.json(data) +} + +async function resyncProjectHistory(req, res, next) { + // increase timeout to 6 minutes + res.setTimeout(6 * 60 * 1000) + const projectId = req.params.Project_id + const opts = {} + const historyRangesMigration = req.body.historyRangesMigration + if (historyRangesMigration) { + opts.historyRangesMigration = historyRangesMigration + } + if (req.body.resyncProjectStructureOnly) { + opts.resyncProjectStructureOnly = req.body.resyncProjectStructureOnly + } + + try { + await ProjectEntityUpdateHandler.promises.resyncProjectHistory( + projectId, + opts + ) + } catch (err) { + if (err instanceof Errors.ProjectHistoryDisabledError) { + return res.sendStatus(404) + } else { + throw err + } + } + + res.sendStatus(204) +} + +async function restoreFileFromV2(req, res, next) { + const { project_id: projectId } = req.params + const { version, pathname } = req.body + const userId = SessionManager.getLoggedInUserId(req.session) + + const entity = await RestoreManager.promises.restoreFileFromV2( + userId, + projectId, + version, + pathname + ) + + res.json({ + type: entity.type, + id: entity._id, + }) +} + +async function revertFile(req, res, next) { + const { project_id: projectId } = req.params + const { version, pathname } = req.body + const userId = SessionManager.getLoggedInUserId(req.session) + + const entity = await RestoreManager.promises.revertFile( + userId, + projectId, + version, + pathname, + {} + ) + + res.json({ + type: entity.type, + id: entity._id, + }) +} + +async function revertProject(req, res, next) { + const { project_id: projectId } = req.params + const { version } = req.body + const userId = SessionManager.getLoggedInUserId(req.session) + + await RestoreManager.promises.revertProject(userId, projectId, version) + + res.sendStatus(200) +} + +async function getLabels(req, res, next) { + const projectId = req.params.Project_id + + let labels = await fetchJson( + `${settings.apis.project_history.url}/project/${projectId}/labels` + ) + labels = await _enrichLabels(labels) + + res.json(labels) +} + +async function createLabel(req, res, next) { + const projectId = req.params.Project_id + const { comment, version } = req.body + const userId = SessionManager.getLoggedInUserId(req.session) + + let label = await fetchJson( + `${settings.apis.project_history.url}/project/${projectId}/labels`, + { + method: 'POST', + json: { comment, version, user_id: userId }, + } + ) + label = await _enrichLabel(label) + + res.json(label) +} + +async function _enrichLabel(label) { + const newLabel = Object.assign({}, label) + if (!label.user_id) { + newLabel.user_display_name = _displayNameForUser(null) + return newLabel + } + + const user = await UserGetter.promises.getUser(label.user_id, { + first_name: 1, + last_name: 1, + email: 1, + }) + newLabel.user_display_name = _displayNameForUser(user) + return newLabel +} + +async function _enrichLabels(labels) { + if (!labels || !labels.length) { + return [] + } + const uniqueUsers = new Set(labels.map(label => label.user_id)) + + // For backwards compatibility, and for anonymously created labels in SP + // expect missing user_id fields + uniqueUsers.delete(undefined) + + if (!uniqueUsers.size) { + return labels + } + + const rawUsers = await UserGetter.promises.getUsers(Array.from(uniqueUsers), { + first_name: 1, + last_name: 1, + email: 1, + }) + const users = new Map(rawUsers.map(user => [String(user._id), user])) + + labels.forEach(label => { + const user = users.get(label.user_id) + label.user_display_name = _displayNameForUser(user) + }) + return labels +} + +function _displayNameForUser(user) { + if (user == null) { + return 'Anonymous' + } + if (user.name) { + return user.name + } + let name = [user.first_name, user.last_name] + .filter(n => n != null) + .join(' ') + .trim() + if (name === '') { + name = user.email.split('@')[0] + } + if (!name) { + return '?' + } + return name +} + +async function deleteLabel(req, res, next) { + const { Project_id: projectId, label_id: labelId } = req.params + const userId = SessionManager.getLoggedInUserId(req.session) + + const project = await ProjectGetter.promises.getProject(projectId, { + owner_ref: true, + }) + + // If the current user is the project owner, we can use the non-user-specific + // delete label endpoint. Otherwise, we have to use the user-specific version + // (which only deletes the label if it is owned by the user) + const deleteEndpointUrl = project.owner_ref.equals(userId) + ? `${settings.apis.project_history.url}/project/${projectId}/labels/${labelId}` + : `${settings.apis.project_history.url}/project/${projectId}/user/${userId}/labels/${labelId}` + + await fetchNothing(deleteEndpointUrl, { + method: 'DELETE', + }) + res.sendStatus(204) +} + +async function downloadZipOfVersion(req, res, next) { + const { project_id: projectId, version } = req.params + + const project = await ProjectDetailsHandler.promises.getDetails(projectId) + 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) + } + + await _pipeHistoryZipToResponse( + v1Id, + version, + `${project.name} (Version ${version})`, + req, + res + ) +} + +async function _pipeHistoryZipToResponse(v1ProjectId, version, name, req, res) { + if (req.destroyed) { + // client has disconnected -- skip project history api call and download + return + } + // increase timeout to 6 minutes + res.setTimeout(6 * 60 * 1000) + const url = `${settings.apis.v1_history.url}/projects/${v1ProjectId}/version/${version}/zip` + const basicAuth = { + user: settings.apis.v1_history.user, + password: settings.apis.v1_history.pass, + } + + if (!Features.hasFeature('saas')) { + let stream + try { + stream = await fetchStream(url, { basicAuth }) + } catch (err) { + if (err instanceof RequestFailedError && err.response.status === 404) { + return res.sendStatus(404) + } else { + throw err + } + } + + prepareZipAttachment(res, `${name}.zip`) + + try { + await pipeline(stream, res) + } catch (err) { // If the downstream request is cancelled, we get an // ERR_STREAM_PREMATURE_CLOSE. - if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') { - logger.warn({ url, err }, 'history API error') - next(err) + if (!isPrematureClose(err)) { + throw err } + } + return + } + + let body + try { + body = await fetchJson(url, { method: 'POST', basicAuth }) + } catch (err) { + if (err instanceof RequestFailedError && err.response.status === 404) { + throw new Errors.NotFoundError('zip not found') + } else { + throw err + } + } + + if (req.destroyed) { + // client has disconnected -- skip delayed s3 download + return + } + + if (!body.zipUrl) { + throw new OError('Missing zipUrl, cannot fetch zip file', { + v1ProjectId, + body, }) - }, + } - proxyToHistoryApiAndInjectUserDetails(req, res, next) { - const userId = SessionManager.getLoggedInUserId(req.session) - const url = settings.apis.project_history.url + req.url - HistoryController._makeRequest( - { - url, - method: req.method, - json: true, - headers: { - 'X-User-Id': userId, - }, - }, - function (err, body) { - if (err) { - return next(err) - } - HistoryManager.injectUserDetails(body, function (err, data) { - if (err) { - return next(err) - } - res.json(data) - }) - } - ) - }, + // retry for about 6 minutes starting with short delay + let retryDelay = 2000 + let attempt = 0 + while (true) { + attempt += 1 + await setTimeout(retryDelay) - resyncProjectHistory(req, res, next) { - // increase timeout to 6 minutes - res.setTimeout(6 * 60 * 1000) - const projectId = req.params.Project_id - const opts = {} - const historyRangesMigration = req.body.historyRangesMigration - if (historyRangesMigration) { - opts.historyRangesMigration = historyRangesMigration - } - if (req.body.resyncProjectStructureOnly) { - opts.resyncProjectStructureOnly = req.body.resyncProjectStructureOnly - } - ProjectEntityUpdateHandler.resyncProjectHistory( - projectId, - opts, - function (err) { - if (err instanceof Errors.ProjectHistoryDisabledError) { - return res.sendStatus(404) - } - if (err) { - return next(err) - } - res.sendStatus(204) - } - ) - }, - - restoreFileFromV2(req, res, next) { - const { project_id: projectId } = req.params - const { version, pathname } = req.body - const userId = SessionManager.getLoggedInUserId(req.session) - RestoreManager.restoreFileFromV2( - userId, - projectId, - version, - pathname, - function (err, entity) { - if (err) { - return next(err) - } - res.json({ - type: entity.type, - id: entity._id, - }) - } - ) - }, - - revertFile(req, res, next) { - const { project_id: projectId } = req.params - const { version, pathname } = req.body - const userId = SessionManager.getLoggedInUserId(req.session) - RestoreManager.revertFile( - userId, - projectId, - version, - pathname, - {}, - function (err, entity) { - if (err) { - return next(err) - } - res.json({ - type: entity.type, - id: entity._id, - }) - } - ) - }, - - revertProject(req, res, next) { - const { project_id: projectId } = req.params - const { version } = req.body - const userId = SessionManager.getLoggedInUserId(req.session) - RestoreManager.revertProject(userId, projectId, version, function (err) { - if (err) { - return next(err) - } - res.sendStatus(200) - }) - }, - - getLabels(req, res, next) { - const projectId = req.params.Project_id - HistoryController._makeRequest( - { - method: 'GET', - url: `${settings.apis.project_history.url}/project/${projectId}/labels`, - json: true, - }, - function (err, labels) { - if (err) { - return next(err) - } - HistoryController._enrichLabels(labels, (err, labels) => { - if (err) { - return next(err) - } - res.json(labels) - }) - } - ) - }, - - createLabel(req, res, next) { - const projectId = req.params.Project_id - const { comment, version } = req.body - const userId = SessionManager.getLoggedInUserId(req.session) - HistoryController._makeRequest( - { - method: 'POST', - url: `${settings.apis.project_history.url}/project/${projectId}/labels`, - json: { comment, version, user_id: userId }, - }, - function (err, label) { - if (err) { - return next(err) - } - HistoryController._enrichLabel(label, (err, label) => { - if (err) { - return next(err) - } - res.json(label) - }) - } - ) - }, - - _enrichLabel(label, callback) { - if (!label.user_id) { - const newLabel = Object.assign({}, label) - newLabel.user_display_name = HistoryController._displayNameForUser(null) - return callback(null, newLabel) - } - UserGetter.getUser( - label.user_id, - { first_name: 1, last_name: 1, email: 1 }, - (err, user) => { - if (err) { - return callback(err) - } - const newLabel = Object.assign({}, label) - newLabel.user_display_name = HistoryController._displayNameForUser(user) - callback(null, newLabel) - } - ) - }, - - _enrichLabels(labels, callback) { - if (!labels || !labels.length) { - return callback(null, []) - } - const uniqueUsers = new Set(labels.map(label => label.user_id)) - - // For backwards compatibility, and for anonymously created labels in SP - // expect missing user_id fields - uniqueUsers.delete(undefined) - - if (!uniqueUsers.size) { - return callback(null, labels) - } - - UserGetter.getUsers( - Array.from(uniqueUsers), - { first_name: 1, last_name: 1, email: 1 }, - function (err, rawUsers) { - if (err) { - return callback(err) - } - const users = new Map(rawUsers.map(user => [String(user._id), user])) - - labels.forEach(label => { - const user = users.get(label.user_id) - label.user_display_name = HistoryController._displayNameForUser(user) - }) - callback(null, labels) - } - ) - }, - - _displayNameForUser(user) { - if (user == null) { - return 'Anonymous' - } - if (user.name) { - return user.name - } - let name = [user.first_name, user.last_name] - .filter(n => n != null) - .join(' ') - .trim() - if (name === '') { - name = user.email.split('@')[0] - } - if (!name) { - return '?' - } - return name - }, - - deleteLabel(req, res, next) { - const { Project_id: projectId, label_id: labelId } = req.params - const userId = SessionManager.getLoggedInUserId(req.session) - - ProjectGetter.getProject( - projectId, - { - owner_ref: true, - }, - (err, project) => { - if (err) { - return next(err) - } - - // If the current user is the project owner, we can use the non-user-specific delete label endpoint. - // Otherwise, we have to use the user-specific version (which only deletes the label if it is owned by the user) - const deleteEndpointUrl = project.owner_ref.equals(userId) - ? `${settings.apis.project_history.url}/project/${projectId}/labels/${labelId}` - : `${settings.apis.project_history.url}/project/${projectId}/user/${userId}/labels/${labelId}` - - HistoryController._makeRequest( - { - method: 'DELETE', - url: deleteEndpointUrl, - }, - function (err) { - if (err) { - return next(err) - } - res.sendStatus(204) - } - ) - } - ) - }, - - _makeRequest(options, callback) { - return request(options, function (err, response, body) { - if (err) { - return callback(err) - } - if (response.statusCode >= 200 && response.statusCode < 300) { - callback(null, body) - } else { - err = new Error( - `history api responded with non-success code: ${response.statusCode}` - ) - callback(err) - } - }) - }, - - downloadZipOfVersion(req, res, next) { - const { project_id: projectId, version } = req.params - ProjectDetailsHandler.getDetails(projectId, function (err, project) { - if (err) { - return next(err) - } - 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) - } - HistoryController._pipeHistoryZipToResponse( - v1Id, - version, - `${project.name} (Version ${version})`, - req, - res, - next - ) - }) - }, - - _pipeHistoryZipToResponse(v1ProjectId, version, name, req, res, next) { if (req.destroyed) { - // client has disconnected -- skip project history api call and download - return - } - // increase timeout to 6 minutes - res.setTimeout(6 * 60 * 1000) - const url = `${settings.apis.v1_history.url}/projects/${v1ProjectId}/version/${version}/zip` - const options = { - auth: { - user: settings.apis.v1_history.user, - pass: settings.apis.v1_history.pass, - }, - json: true, - url, - } - - if (!Features.hasFeature('saas')) { - const getReq = request({ ...options, method: 'get' }) - - getReq.on('error', function (err) { - logger.warn({ err, v1ProjectId, version }, 'history zip download error') - res.sendStatus(500) - }) - getReq.on('response', function (response) { - const statusCode = response.statusCode - if (statusCode !== 200) { - logger.warn( - { v1ProjectId, version, statusCode }, - 'history zip download failed' - ) - if (statusCode === 404) { - res.sendStatus(404) - } else { - res.sendStatus(500) - } - return - } - - prepareZipAttachment(res, `${name}.zip`) - pipeline(response, res, function (err) { - // If the downstream request is cancelled, we get an - // ERR_STREAM_PREMATURE_CLOSE. - if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') { - logger.error({ err, v1ProjectId, version }, 'history API error') - next(err) - } - }) - }) + // client has disconnected -- skip s3 download return } - request({ ...options, method: 'post' }, function (err, response, body) { - if (err) { - OError.tag(err, 'history API error', { - v1ProjectId, - version, - }) - return next(err) + // increase delay by 1 second up to 10 + if (retryDelay < 10000) { + retryDelay += 1000 + } + + try { + const stream = await fetchStream(body.zipUrl) + prepareZipAttachment(res, `${name}.zip`) + await pipeline(stream, res) + } catch (err) { + if (attempt > MAX_HISTORY_ZIP_ATTEMPTS) { + throw err } - if (response.statusCode !== 200) { - if (response.statusCode === 404) { - return next(new Errors.NotFoundError('zip not found')) - } else { - return next( - new OError('Error while getting zip for download', { - v1ProjectId, - statusCode: response.statusCode, - }) - ) - } - } - if (req.destroyed) { - // client has disconnected -- skip delayed s3 download - return - } - if (!body.zipUrl) { - return next( - new OError('Missing zipUrl, cannot fetch zip file', { - v1ProjectId, - body, - statusCode: response.statusCode, - }) + + if (err instanceof RequestFailedError && err.response.status === 404) { + // File not ready yet. Retry. + continue + } else if (isPrematureClose(err)) { + // Downstream request cancelled. Retry. + continue + } else { + // Unknown error. Log and retry. + logger.warn( + { err, v1ProjectId, version, retryAttempt: attempt }, + 'history s3 proxying error' ) + continue } - let retryAttempt = 0 - let retryDelay = 2000 - // retry for about 6 minutes starting with short delay - async.retry( - 40, - callback => - setTimeout(function () { - if (req.destroyed) { - // client has disconnected -- skip s3 download - return callback() // stop async.retry loop - } + } - // increase delay by 1 second up to 10 - if (retryDelay < 10000) { - retryDelay += 1000 - } - retryAttempt++ - const getReq = request({ - url: body.zipUrl, - sendImmediately: true, - }) - const abortS3Request = () => getReq.abort() - req.on('close', abortS3Request) - res.on('timeout', abortS3Request) - function cleanupAbortTrigger() { - req.off('close', abortS3Request) - res.off('timeout', abortS3Request) - } - getReq.on('response', function (response) { - if (response.statusCode !== 200) { - cleanupAbortTrigger() - return callback(new Error('invalid response')) - } - // pipe also proxies the headers, but we want to customize these ones - delete response.headers['content-disposition'] - delete response.headers['content-type'] - res.status(response.statusCode) - prepareZipAttachment(res, `${name}.zip`) - pipeline(response, res, err => { - // If the downstream request is cancelled, we get an - // ERR_STREAM_PREMATURE_CLOSE. - if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') { - logger.warn( - { err, v1ProjectId, version, retryAttempt }, - 'history s3 proxying error' - ) - } - }) - callback() - }) - getReq.on('error', function (err) { - logger.warn( - { err, v1ProjectId, version, retryAttempt }, - 'history s3 download error' - ) - cleanupAbortTrigger() - callback(err) - }) - }, retryDelay), - function (err) { - if (err) { - OError.tag(err, 'history s3 download failed', { - v1ProjectId, - version, - retryAttempt, - }) - next(err) - } - } - ) - }) + // We made it through. No need to retry anymore. Exit loop + break + } +} + +function isPrematureClose(err) { + return ( + err instanceof Error && + 'code' in err && + err.code === 'ERR_STREAM_PREMATURE_CLOSE' + ) +} + +module.exports = { + getBlob: expressify(getBlob), + headBlob: expressify(headBlob), + proxyToHistoryApi: expressify(proxyToHistoryApi), + proxyToHistoryApiAndInjectUserDetails: expressify( + proxyToHistoryApiAndInjectUserDetails + ), + resyncProjectHistory: expressify(resyncProjectHistory), + restoreFileFromV2: expressify(restoreFileFromV2), + revertFile: expressify(revertFile), + revertProject: expressify(revertProject), + getLabels: expressify(getLabels), + createLabel: expressify(createLabel), + deleteLabel: expressify(deleteLabel), + downloadZipOfVersion: expressify(downloadZipOfVersion), + _displayNameForUser, + promises: { + _pipeHistoryZipToResponse, }, } diff --git a/services/web/test/unit/src/History/HistoryControllerTests.js b/services/web/test/unit/src/History/HistoryControllerTests.js index 8f37e6d258..f575859073 100644 --- a/services/web/test/unit/src/History/HistoryControllerTests.js +++ b/services/web/test/unit/src/History/HistoryControllerTests.js @@ -1,16 +1,6 @@ -/* eslint-disable - max-len, - no-return-assign, -*/ -// 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 - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const sinon = require('sinon') const { expect } = require('chai') +const { RequestFailedError } = require('@overleaf/fetch-utils') const Errors = require('../../../../app/src/Features/Errors/Errors') @@ -22,25 +12,59 @@ describe('HistoryController', function () { beforeEach(function () { this.callback = sinon.stub() this.user_id = 'user-id-123' + this.project_id = 'mock-project-id' + this.stream = sinon.stub() + this.fetchResponse = { + headers: { + get: sinon.stub(), + }, + } + this.next = sinon.stub() + this.SessionManager = { getLoggedInUserId: sinon.stub().returns(this.user_id), } + this.Stream = { - pipeline: sinon.stub(), + pipeline: sinon.stub().resolves(), } + + this.HistoryManager = { + promises: { + injectUserDetails: sinon.stub(), + }, + } + + this.ProjectEntityUpdateHandler = { + promises: { + resyncProjectHistory: sinon.stub().resolves(), + }, + } + + this.fetchJson = sinon.stub() + this.fetchStream = sinon.stub().resolves(this.stream) + this.fetchStreamWithResponse = sinon + .stub() + .resolves({ stream: this.stream, response: this.fetchResponse }) + this.fetchNothing = sinon.stub().resolves() + this.HistoryController = SandboxedModule.require(modulePath, { requires: { - request: (this.request = sinon.stub()), + 'stream/promises': this.Stream, '@overleaf/settings': (this.settings = {}), - '@overleaf/fetch-utils': {}, + '@overleaf/fetch-utils': { + fetchJson: this.fetchJson, + fetchStream: this.fetchStream, + fetchStreamWithResponse: this.fetchStreamWithResponse, + fetchNothing: this.fetchNothing, + }, '@overleaf/Metrics': {}, '../../infrastructure/mongodb': { ObjectId }, - stream: this.Stream, '../Authentication/SessionManager': this.SessionManager, - './HistoryManager': (this.HistoryManager = {}), + './HistoryManager': this.HistoryManager, '../Project/ProjectDetailsHandler': (this.ProjectDetailsHandler = {}), '../Project/ProjectEntityUpdateHandler': - (this.ProjectEntityUpdateHandler = {}), + this.ProjectEntityUpdateHandler, '../User/UserGetter': (this.UserGetter = {}), '../Project/ProjectGetter': (this.ProjectGetter = {}), './RestoreManager': (this.RestoreManager = {}), @@ -50,172 +74,125 @@ describe('HistoryController', function () { .returns(true)), }, }) - return (this.settings.apis = { + this.settings.apis = { project_history: { url: 'http://project_history.example.com', }, - }) + } }) describe('proxyToHistoryApi', function () { - beforeEach(function () { - this.req = { url: '/mock/url', method: 'POST' } - this.res = 'mock-res' - this.next = sinon.stub() - this.proxy = sinon.stub() - this.request.returns(this.proxy) + beforeEach(async function () { + this.req = { url: '/mock/url', method: 'POST', session: sinon.stub() } + this.res = { + set: sinon.stub(), + } + this.contentType = 'application/json' + this.contentLength = 212 + this.fetchResponse.headers.get + .withArgs('Content-Type') + .returns(this.contentType) + this.fetchResponse.headers.get + .withArgs('Content-Length') + .returns(this.contentLength) + await this.HistoryController.proxyToHistoryApi( + this.req, + this.res, + this.next + ) }) - describe('for a project with the project history flag', function () { - beforeEach(function () { - this.req.useProjectHistory = true - return this.HistoryController.proxyToHistoryApi( - this.req, - this.res, - this.next - ) - }) - - it('should get the user id', function () { - return this.SessionManager.getLoggedInUserId - .calledWith(this.req.session) - .should.equal(true) - }) - - it('should call the project history api', function () { - return this.request - .calledWith({ - url: `${this.settings.apis.project_history.url}${this.req.url}`, - method: this.req.method, - headers: { - 'X-User-Id': this.user_id, - }, - }) - .should.equal(true) - }) - - it('should pipe the response to the client', function () { - expect(this.Stream.pipeline).to.have.been.calledWith( - this.proxy, - this.res - ) - }) + it('should get the user id', function () { + this.SessionManager.getLoggedInUserId.should.have.been.calledWith( + this.req.session + ) }) - describe('for a project without the project history flag', function () { - beforeEach(function () { - this.req.useProjectHistory = false - return this.HistoryController.proxyToHistoryApi( - this.req, - this.res, - this.next - ) - }) + it('should call the project history api', function () { + this.fetchStreamWithResponse.should.have.been.calledWith( + `${this.settings.apis.project_history.url}${this.req.url}`, + { + method: this.req.method, + headers: { + 'X-User-Id': this.user_id, + }, + } + ) + }) - it('should get the user id', function () { - return this.SessionManager.getLoggedInUserId - .calledWith(this.req.session) - .should.equal(true) - }) + it('should pipe the response to the client', function () { + expect(this.Stream.pipeline).to.have.been.calledWith( + this.stream, + this.res + ) + }) - it('should pipe the response to the client', function () { - expect(this.Stream.pipeline).to.have.been.calledWith( - this.proxy, - this.res - ) - }) + it('should propagate the appropriate headers', function () { + expect(this.res.set).to.have.been.calledWith( + 'Content-Type', + this.contentType + ) + expect(this.res.set).to.have.been.calledWith( + 'Content-Length', + this.contentLength + ) }) }) describe('proxyToHistoryApiAndInjectUserDetails', function () { - beforeEach(function () { + beforeEach(async function () { this.req = { url: '/mock/url', method: 'POST' } this.res = { json: sinon.stub() } - this.next = sinon.stub() - this.request.yields(null, { statusCode: 200 }, (this.data = 'mock-data')) - return (this.HistoryManager.injectUserDetails = sinon - .stub() - .yields(null, (this.data_with_users = 'mock-injected-data'))) + this.data = 'mock-data' + this.dataWithUsers = 'mock-injected-data' + this.fetchJson.resolves(this.data) + this.HistoryManager.promises.injectUserDetails.resolves( + this.dataWithUsers + ) + await this.HistoryController.proxyToHistoryApiAndInjectUserDetails( + this.req, + this.res, + this.next + ) }) - describe('for a project with the project history flag', function () { - beforeEach(function () { - this.req.useProjectHistory = true - return this.HistoryController.proxyToHistoryApiAndInjectUserDetails( - this.req, - this.res, - this.next - ) - }) - - it('should get the user id', function () { - return this.SessionManager.getLoggedInUserId - .calledWith(this.req.session) - .should.equal(true) - }) - - it('should call the project history api', function () { - return this.request - .calledWith({ - url: `${this.settings.apis.project_history.url}${this.req.url}`, - method: this.req.method, - json: true, - headers: { - 'X-User-Id': this.user_id, - }, - }) - .should.equal(true) - }) - - it('should inject the user data', function () { - return this.HistoryManager.injectUserDetails - .calledWith(this.data) - .should.equal(true) - }) - - it('should return the data with users to the client', function () { - return this.res.json.calledWith(this.data_with_users).should.equal(true) - }) + it('should get the user id', function () { + this.SessionManager.getLoggedInUserId.should.have.been.calledWith( + this.req.session + ) }) - describe('for a project without the project history flag', function () { - beforeEach(function () { - this.req.useProjectHistory = false - return this.HistoryController.proxyToHistoryApiAndInjectUserDetails( - this.req, - this.res, - this.next - ) - }) + it('should call the project history api', function () { + this.fetchJson.should.have.been.calledWith( + `${this.settings.apis.project_history.url}${this.req.url}`, + { + method: this.req.method, + headers: { + 'X-User-Id': this.user_id, + }, + } + ) + }) - it('should get the user id', function () { - return this.SessionManager.getLoggedInUserId - .calledWith(this.req.session) - .should.equal(true) - }) + it('should inject the user data', function () { + this.HistoryManager.promises.injectUserDetails.should.have.been.calledWith( + this.data + ) + }) - it('should inject the user data', function () { - return this.HistoryManager.injectUserDetails - .calledWith(this.data) - .should.equal(true) - }) - - it('should return the data with users to the client', function () { - return this.res.json.calledWith(this.data_with_users).should.equal(true) - }) + it('should return the data with users to the client', function () { + this.res.json.should.have.been.calledWith(this.dataWithUsers) }) }) describe('proxyToHistoryApiAndInjectUserDetails (with the history API failing)', function () { - beforeEach(function () { - this.req = { url: '/mock/url', method: 'POST', useProjectHistory: true } + beforeEach(async function () { + this.url = '/mock/url' + this.req = { url: this.url, method: 'POST' } this.res = { json: sinon.stub() } - this.next = sinon.stub() - this.request.yields(null, { statusCode: 500 }, (this.data = 'mock-data')) - this.HistoryManager.injectUserDetails = sinon - .stub() - .yields(null, (this.data_with_users = 'mock-injected-data')) - return this.HistoryController.proxyToHistoryApiAndInjectUserDetails( + this.err = new RequestFailedError(this.url, {}, { status: 500 }) + this.fetchJson.rejects(this.err) + await this.HistoryController.proxyToHistoryApiAndInjectUserDetails( this.req, this.res, this.next @@ -223,30 +200,30 @@ describe('HistoryController', function () { }) it('should not inject the user data', function () { - return this.HistoryManager.injectUserDetails - .calledWith(this.data) - .should.equal(false) + this.HistoryManager.promises.injectUserDetails.should.not.have.been.called }) it('should not return the data with users to the client', function () { - return this.res.json.calledWith(this.data_with_users).should.equal(false) + this.res.json.should.not.have.been.called + }) + + it('should throw an error', function () { + this.next.should.have.been.calledWith(this.err) }) }) describe('resyncProjectHistory', function () { describe('for a project without project-history enabled', function () { - beforeEach(function () { - this.project_id = 'mock-project-id' + beforeEach(async function () { this.req = { params: { Project_id: this.project_id }, body: {} } this.res = { setTimeout: sinon.stub(), sendStatus: sinon.stub() } - this.next = sinon.stub() this.error = new Errors.ProjectHistoryDisabledError() - this.ProjectEntityUpdateHandler.resyncProjectHistory = sinon - .stub() - .yields(this.error) + this.ProjectEntityUpdateHandler.promises.resyncProjectHistory.rejects( + this.error + ) - return this.HistoryController.resyncProjectHistory( + await this.HistoryController.resyncProjectHistory( this.req, this.res, this.next @@ -254,22 +231,16 @@ describe('HistoryController', function () { }) it('response with a 404', function () { - return this.res.sendStatus.calledWith(404).should.equal(true) + this.res.sendStatus.should.have.been.calledWith(404) }) }) describe('for a project with project-history enabled', function () { - beforeEach(function () { - this.project_id = 'mock-project-id' + beforeEach(async function () { this.req = { params: { Project_id: this.project_id }, body: {} } this.res = { setTimeout: sinon.stub(), sendStatus: sinon.stub() } - this.next = sinon.stub() - this.ProjectEntityUpdateHandler.resyncProjectHistory = sinon - .stub() - .yields() - - return this.HistoryController.resyncProjectHistory( + await this.HistoryController.resyncProjectHistory( this.req, this.res, this.next @@ -281,13 +252,13 @@ describe('HistoryController', function () { }) it('resyncs the project', function () { - return this.ProjectEntityUpdateHandler.resyncProjectHistory - .calledWith(this.project_id) - .should.equal(true) + this.ProjectEntityUpdateHandler.promises.resyncProjectHistory.should.have.been.calledWith( + this.project_id + ) }) it('responds with a 204', function () { - return this.res.sendStatus.calledWith(204).should.equal(true) + this.res.sendStatus.should.have.been.calledWith(204) }) }) })