From 277248b8b784472ac24bc0755ce1bf92028ab5d5 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Mon, 18 Apr 2016 21:56:58 +0200 Subject: [PATCH] switch to standard response format in v3 --- .../v3/unit/middlewares/errorHandler.test.js | 7 ++++++ test/api/v3/unit/middlewares/response.js | 6 +++-- test/helpers/api-integration/requester.js | 24 ++++++++++++++++--- website/src/controllers/api-v3/content.js | 4 +++- .../src/middlewares/api-v3/errorHandler.js | 3 ++- website/src/middlewares/api-v3/response.js | 6 ++++- 6 files changed, 42 insertions(+), 8 deletions(-) diff --git a/test/api/v3/unit/middlewares/errorHandler.test.js b/test/api/v3/unit/middlewares/errorHandler.test.js index 7c75be9f28..89ce1b7db6 100644 --- a/test/api/v3/unit/middlewares/errorHandler.test.js +++ b/test/api/v3/unit/middlewares/errorHandler.test.js @@ -38,6 +38,7 @@ describe('errorHandler', () => { expect(res.status).to.be.calledWith(500); expect(res.json).to.be.calledWith({ + success: false, error: 'InternalServerError', message: 'An unexpected error occurred.', }); @@ -54,6 +55,7 @@ describe('errorHandler', () => { expect(res.status).to.be.calledWith(400); expect(res.json).to.be.calledWith({ + success: false, error: 'Error', message: 'Error message', }); @@ -70,6 +72,7 @@ describe('errorHandler', () => { expect(res.status).to.be.calledWith(500); expect(res.json).to.be.calledWith({ + success: false, error: 'InternalServerError', message: 'An unexpected error occurred.', }); @@ -85,6 +88,7 @@ describe('errorHandler', () => { expect(res.status).to.be.calledWith(400); expect(res.json).to.be.calledWith({ + success: false, error: 'BadRequest', message: 'Bad request.', }); @@ -101,6 +105,7 @@ describe('errorHandler', () => { expect(res.status).to.be.calledWith(error.statusCode); expect(res.json).to.be.calledWith({ + success: false, error: error.name, message: error.message, }); @@ -116,6 +121,7 @@ describe('errorHandler', () => { expect(res.status).to.be.calledWith(400); expect(res.json).to.be.calledWith({ + success: false, error: 'BadRequest', message: 'Invalid request parameters.', errors: [ @@ -143,6 +149,7 @@ describe('errorHandler', () => { expect(res.status).to.be.calledWith(400); expect(res.json).to.be.calledWith({ + success: false, error: 'BadRequest', message: 'User validation failed.', errors: [ diff --git a/test/api/v3/unit/middlewares/response.js b/test/api/v3/unit/middlewares/response.js index 25916e1ef3..296a216ec2 100644 --- a/test/api/v3/unit/middlewares/response.js +++ b/test/api/v3/unit/middlewares/response.js @@ -30,7 +30,8 @@ describe('response middleware', () => { expect(res.status).to.be.calledWith(200); expect(res.json).to.be.calledWith({ - field: 1, + success: true, + data: {field: 1}, }); }); @@ -43,7 +44,8 @@ describe('response middleware', () => { expect(res.status).to.be.calledWith(403); expect(res.json).to.be.calledWith({ - field: 1, + success: false, + data: {field: 1}, }); }); }); diff --git a/test/helpers/api-integration/requester.js b/test/helpers/api-integration/requester.js index d56593548a..a634421d13 100644 --- a/test/helpers/api-integration/requester.js +++ b/test/helpers/api-integration/requester.js @@ -61,7 +61,7 @@ function _requestMaker (user, method, additionalSets = {}) { let parsedError = _parseError(err); - reject(parsedError); + return reject(parsedError); } // if any cookies was sent, save it for the next request @@ -71,13 +71,31 @@ function _requestMaker (user, method, additionalSets = {}) { }).join('; '); } - let contentType = response.headers['content-type'] || ''; - resolve(contentType.indexOf('json') !== -1 ? response.body : response.text); + resolve(_parseRes(response)); }); }); }; } +function _parseRes (res) { + let contentType = res.headers['content-type'] || ''; + let contentDisposition = res.headers['content-disposition'] || ''; + + if (contentType.indexOf('json') === -1) { // not a json response + return res.text; + } + + if (contentDisposition.indexOf('attachment') !== -1) { + return res.body; + } + + if (apiVersion === 'v2') { + return res.body; + } else if (apiVersion === 'v3') { + return res.body.data; + } +} + function _parseError (err) { let parsedError; diff --git a/website/src/controllers/api-v3/content.js b/website/src/controllers/api-v3/content.js index 46eedf9b91..b9d7495a97 100644 --- a/website/src/controllers/api-v3/content.js +++ b/website/src/controllers/api-v3/content.js @@ -96,7 +96,9 @@ api.getContent = { res.set({ 'Content-Type': 'application/json', }); - res.status(200).send(content); // TODO how to use res.respond here? + + let jsonResString = `{"success": true, "data": ${content}}`; + res.status(200).send(jsonResString); // save the file in background unless it's already cached or being written right now if (cachedContentResponses[language] !== true && cacheBeingWritten[language] !== true) { diff --git a/website/src/middlewares/api-v3/errorHandler.js b/website/src/middlewares/api-v3/errorHandler.js index a4194435c4..d775292a5b 100644 --- a/website/src/middlewares/api-v3/errorHandler.js +++ b/website/src/middlewares/api-v3/errorHandler.js @@ -62,6 +62,7 @@ module.exports = function errorHandler (err, req, res, next) { // eslint-disable } let jsonRes = { + success: false, error: responseErr.name, message: responseErr.message, }; @@ -72,5 +73,5 @@ module.exports = function errorHandler (err, req, res, next) { // eslint-disable // In some occasions like when invalid JSON is supplied `res.respond` might be not yet avalaible, // in this case we use the standard res.status(...).json(...) - return res.respond ? res.respond(responseErr.httpCode, jsonRes) : res.status(responseErr.httpCode).json(jsonRes); + return res.status(responseErr.httpCode).json(jsonRes); }; diff --git a/website/src/middlewares/api-v3/response.js b/website/src/middlewares/api-v3/response.js index a3a84fd818..7fe4bc7e35 100644 --- a/website/src/middlewares/api-v3/response.js +++ b/website/src/middlewares/api-v3/response.js @@ -1,6 +1,10 @@ module.exports = function responseHandler (req, res, next) { + // Only used for successful responses res.respond = function respond (status = 200, data = {}) { - res.status(status).json(data); + res.status(status).json({ + success: status < 400, + data, + }); }; next();