diff --git a/services/web/app/src/Features/V1/V1Api.mjs b/services/web/app/src/Features/V1/V1Api.mjs index 29c42c6820..4aa5f1072f 100644 --- a/services/web/app/src/Features/V1/V1Api.mjs +++ b/services/web/app/src/Features/V1/V1Api.mjs @@ -1,12 +1,3 @@ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ import request from 'request' import settings from '@overleaf/settings' @@ -27,82 +18,61 @@ const DEFAULT_V1_PARAMS = { const v1Request = request.defaults(DEFAULT_V1_PARAMS) -const DEFAULT_V1_OAUTH_PARAMS = { - baseUrl: settings.apis.v1.url, - json: true, - timeout: settings.apis.v1.timeout, +function makeRequest(options, callback) { + if (!callback) { + return request(options) + } + v1Request(options, (error, response, body) => + _responseHandler(options, error, response, body, callback) + ) } -const v1OauthRequest = request.defaults(DEFAULT_V1_OAUTH_PARAMS) +function _responseHandler(options, error, response, body, callback) { + const { expectedStatusCodes } = options + if (error) { + return callback( + new Errors.V1ConnectionError('error from V1 API').withCause(error) + ) + } + if (response && response.statusCode >= 500) { + return callback( + new Errors.V1ConnectionError({ + message: 'error from V1 API', + info: { status: response.statusCode, body }, + }) + ) + } + if ( + (response && response.statusCode >= 200 && response.statusCode < 300) || + (expectedStatusCodes || []).includes(response?.statusCode) + ) { + return callback(null, response, body) + } else if (response?.statusCode === 403) { + error = new Errors.ForbiddenError('overleaf v1 returned forbidden') + error.statusCode = response.statusCode + return callback(error) + } else if (response?.statusCode === 404) { + error = new Errors.NotFoundError( + `overleaf v1 returned non-success code: ${response.statusCode} ${options.method} ${options.uri}` + ) + error.statusCode = response.statusCode + return callback(error) + } else { + error = new OError('overleaf v1 returned non-success code', { + status: response?.statusCode, + method: options.method, + url: options.uri, + }) + error.statusCode = response?.statusCode + return callback(error) + } +} const V1Api = { - request(options, callback) { - if (callback == null) { - return request(options) - } - return v1Request(options, (error, response, body) => - V1Api._responseHandler(options, error, response, body, callback) - ) - }, - - oauthRequest(options, token, callback) { - if (options.uri == null) { - return callback(new Error('uri required')) - } - if (options.method == null) { - options.method = 'GET' - } - options.auth = { bearer: token } - return v1OauthRequest(options, (error, response, body) => - V1Api._responseHandler(options, error, response, body, callback) - ) - }, - - _responseHandler(options, error, response, body, callback) { - if (error != null) { - return callback( - new Errors.V1ConnectionError('error from V1 API').withCause(error) - ) - } - if (response && response.statusCode >= 500) { - return callback( - new Errors.V1ConnectionError({ - message: 'error from V1 API', - info: { status: response.statusCode, body }, - }) - ) - } - if ( - (response && response.statusCode >= 200 && response.statusCode < 300) || - Array.from(options.expectedStatusCodes || []).includes( - response?.statusCode - ) - ) { - return callback(null, response, body) - } else if (response?.statusCode === 403) { - error = new Errors.ForbiddenError('overleaf v1 returned forbidden') - error.statusCode = response.statusCode - return callback(error) - } else if (response?.statusCode === 404) { - error = new Errors.NotFoundError( - `overleaf v1 returned non-success code: ${response.statusCode} ${options.method} ${options.uri}` - ) - error.statusCode = response.statusCode - return callback(error) - } else { - error = new OError('overleaf v1 returned non-success code', { - status: response?.statusCode, - method: options.method, - url: options.uri, - }) - error.statusCode = response?.statusCode - return callback(error) - } - }, + request: makeRequest, } V1Api.promises = { request: promisifyMultiResult(V1Api.request, ['response', 'body']), - oauthRequest: promisifyMultiResult(V1Api.oauthRequest, ['response', 'body']), } export default V1Api diff --git a/services/web/test/acceptance/src/V1ApiTests.mjs b/services/web/test/acceptance/src/V1ApiTests.mjs new file mode 100644 index 0000000000..718a07bfc8 --- /dev/null +++ b/services/web/test/acceptance/src/V1ApiTests.mjs @@ -0,0 +1,111 @@ +import { expect } from 'chai' +import V1Api from '../../../app/src/Features/V1/V1Api.mjs' +import MockV1ApiClass from './mocks/MockV1Api.mjs' +import Features from '../../../app/src/infrastructure/Features.mjs' + +describe('V1Api', function () { + const testPath = '/api/v1/overleaf/fake_route_api_handler_tests' + + function testPathWithResponse(statusCode, body = '') { + const encodedBody = encodeURIComponent(JSON.stringify(body)) + return `${testPath}?expectedStatus=${statusCode}&expectedBody=${encodedBody}` + } + + beforeEach(function () { + if (!Features.hasFeature('saas')) { + this.skip() + } + + MockV1ApiClass.instance() + }) + + it('returns errors from request handling', async function () { + let error + try { + await V1Api.promises.request({}) + } catch (e) { + error = e + } + expect(error).to.exist + expect(error.cause.message).to.equal( + 'options.uri must be a string when using options.baseUrl' + ) + }) + + it('returns 500 errors', async function () { + let error + const expectedBody = { testing: 'test' } + const url = testPathWithResponse(500, expectedBody) + + try { + await V1Api.promises.request({ url }) + } catch (e) { + error = e + } + expect(error).to.exist + expect(error.message).to.equal('error from V1 API') + expect(error.info.status).to.equal(500) + expect(error.info.body).to.deep.equal(expectedBody) + }) + + it('returns 2xx status responses', async function () { + const expectedBody = { testing: 'success' } + const url = testPathWithResponse(200, expectedBody) + const response = await V1Api.promises.request({ url }) + expect(response.response.statusCode).to.equal(200) + expect(response.body).to.deep.equal(expectedBody) + }) + + it('returns 403 errors', async function () { + let error + const url = testPathWithResponse(403) + + try { + await V1Api.promises.request({ url }) + } catch (e) { + error = e + } + expect(error).to.exist + expect(error.message).to.equal('overleaf v1 returned forbidden') + expect(error.statusCode).to.equal(403) + }) + + it('returns 404 errors', async function () { + let error + const url = testPathWithResponse(404) + + try { + await V1Api.promises.request({ uri: url, method: 'GET' }) + } catch (e) { + error = e + } + expect(error).to.exist + expect(error.message).to.equal( + `overleaf v1 returned non-success code: 404 GET ${url}` + ) + expect(error.statusCode).to.equal(404) + }) + + it('returns errors', async function () { + let error + const url = testPathWithResponse(402) + + try { + await V1Api.promises.request({ uri: url, method: 'GET' }) + } catch (e) { + error = e + } + expect(error).to.exist + expect(error.message).to.equal('overleaf v1 returned non-success code') + expect(error.statusCode).to.equal(402) + }) + + it('does not return error if expected status', async function () { + const url = testPathWithResponse(404) + const response = await V1Api.promises.request({ + url, + expectedStatusCodes: [404], + }) + expect(response.response.statusCode).to.equal(404) + }) +}) diff --git a/services/web/test/acceptance/src/mocks/MockV1Api.mjs b/services/web/test/acceptance/src/mocks/MockV1Api.mjs index 982bf4b6c7..0e803151f4 100644 --- a/services/web/test/acceptance/src/mocks/MockV1Api.mjs +++ b/services/web/test/acceptance/src/mocks/MockV1Api.mjs @@ -476,6 +476,25 @@ class MockV1Api extends AbstractMockApi { } res.json(template) }) + + this.app.get( + '/api/v1/overleaf/fake_route_api_handler_tests', + (req, res) => { + const expectedStatus = req.query.expectedStatus + const expectedBody = req.query.expectedBody + const statusCode = Number(expectedStatus) + if ( + !Number.isInteger(statusCode) || + statusCode < 100 || + statusCode > 599 + ) { + return res + .status(500) + .json({ error: 'Invalid expectedStatus query parameter' }) + } + return res.status(statusCode).json(JSON.parse(expectedBody)) + } + ) } }