From bb5d11794d09e47052c19f1e4c1953717dde674d Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Sat, 5 Nov 2016 15:08:00 +0100 Subject: [PATCH] Dropping bluebird as a runtime dependency. Still use it in some test-setup, will cleanup this laster. closes #128 --- .../unleash-api/lib/error/validate-request.js | 3 +- .../unleash-api/lib/routes/feature-archive.js | 21 ++++-- packages/unleash-api/lib/routes/feature.js | 72 +++++++++---------- packages/unleash-api/lib/routes/strategy.js | 65 +++++++++-------- packages/unleash-api/package.json | 2 +- .../unleash-api/test/e2e/feature-api.test.js | 8 +++ 6 files changed, 91 insertions(+), 80 deletions(-) diff --git a/packages/unleash-api/lib/error/validate-request.js b/packages/unleash-api/lib/error/validate-request.js index bbc51e1543..da9794f9c0 100644 --- a/packages/unleash-api/lib/error/validate-request.js +++ b/packages/unleash-api/lib/error/validate-request.js @@ -1,10 +1,9 @@ 'use strict'; -const BPromise = require('bluebird'); const ValidationError = require('./validation-error'); function validateRequest (req) { - return new BPromise((resolve, reject) => { + return new Promise((resolve, reject) => { if (req.validationErrors()) { reject(new ValidationError('Invalid syntax')); } else { diff --git a/packages/unleash-api/lib/routes/feature-archive.js b/packages/unleash-api/lib/routes/feature-archive.js index 8c037b3986..b0053ca35c 100644 --- a/packages/unleash-api/lib/routes/feature-archive.js +++ b/packages/unleash-api/lib/routes/feature-archive.js @@ -5,6 +5,21 @@ const eventType = require('../event-type'); const ValidationError = require('../error/validation-error'); const validateRequest = require('../error/validate-request'); +const handleErrors = (req, res, error) => { + switch (error.constructor) { + case ValidationError: + return res + .status(400) + .json(req.validationErrors()) + .end(); + default: + logger.error('Server failed executing request', error); + return res + .status(500) + .end(); + } +}; + module.exports = function (app, config) { const { featureToggleStore, eventStore } = config.stores; @@ -24,10 +39,6 @@ module.exports = function (app, config) { data: req.body, })) .then(() => res.status(200).end()) - .catch(ValidationError, () => res.status(400).json(req.validationErrors())) - .catch(err => { - logger.error('Could not revive feature toggle', err); - res.status(500).end(); - }); + .catch(error => handleErrors(req, res, error)); }); }; diff --git a/packages/unleash-api/lib/routes/feature.js b/packages/unleash-api/lib/routes/feature.js index 819b141b6d..70c1653812 100644 --- a/packages/unleash-api/lib/routes/feature.js +++ b/packages/unleash-api/lib/routes/feature.js @@ -1,6 +1,5 @@ 'use strict'; -const BPromise = require('bluebird'); const logger = require('../logger'); const eventType = require('../event-type'); const NameExistsError = require('../error/name-exists-error'); @@ -12,6 +11,30 @@ const extractUser = require('../extract-user'); const legacyFeatureMapper = require('../helper/legacy-feature-mapper'); const version = 1; +const handleErrors = (req, res, error) => { + switch (error.constructor) { + case NotFoundError: + return res + .status(404) + .end(); + case NameExistsError: + return res + .status(403) + .json([{ msg: `A feature named '${req.body.name}' already exists.` }]) + .end(); + case ValidationError: + return res + .status(400) + .json(req.validationErrors()) + .end(); + default: + logger.error('Server failed executing request', error); + return res + .status(500) + .end(); + } +}; + module.exports = function (app, config) { const { featureToggleStore, eventStore } = config.stores; @@ -36,16 +59,7 @@ module.exports = function (app, config) { .then(validateFormat) .then(validateUniqueName) .then(() => res.status(201).end()) - .catch(NameExistsError, () => { - res.status(403) - .json([{ msg: `A feature named '${req.body.name}' already exists.` }]) - .end(); - }) - .catch(ValidationError, () => res.status(400).json(req.validationErrors())) - .catch(err => { - logger.error('Could not create feature toggle', err); - res.status(500).end(); - }); + .catch(error => handleErrors(req, res, error)); }); app.post('/features', (req, res) => { @@ -61,16 +75,7 @@ module.exports = function (app, config) { data: legacyFeatureMapper.toNewFormat(req.body), })) .then(() => res.status(201).end()) - .catch(NameExistsError, () => { - res.status(403) - .json([{ msg: `A feature named '${req.body.name}' already exists.` }]) - .end(); - }) - .catch(ValidationError, () => res.status(400).json(req.validationErrors())) - .catch(err => { - logger.error('Could not create feature toggle', err); - res.status(500).end(); - }); + .catch(error => handleErrors(req, res, error)); }); app.put('/features/:featureName', (req, res) => { @@ -87,11 +92,7 @@ module.exports = function (app, config) { data: updatedFeature, })) .then(() => res.status(200).end()) - .catch(NotFoundError, () => res.status(404).end()) - .catch(err => { - logger.error(`Could not update feature toggle=${featureName}`, err); - res.status(500).end(); - }); + .catch(error => handleErrors(req, res, error)); }); app.delete('/features/:featureName', (req, res) => { @@ -107,29 +108,22 @@ module.exports = function (app, config) { }, })) .then(() => res.status(200).end()) - .catch(NotFoundError, () => res.status(404).end()) - .catch(err => { - logger.error(`Could not archive feature=${featureName}`, err); - res.status(500).end(); - }); + .catch(error => handleErrors(req, res, error)); }); function validateUniqueName (req) { - return new BPromise((resolve, reject) => { + return new Promise((resolve, reject) => { featureToggleStore.getFeature(req.body.name) - .then(() => { - reject(new NameExistsError('Feature name already exist')); - }, () => { - resolve(req); - }); + .then(() => reject(new NameExistsError('Feature name already exist'))) + .catch(() => resolve(req)); }); } function validateFormat (req) { if (req.body.strategy && req.body.strategies) { - return BPromise.reject(new ValidationError('Cannot use both "strategy" and "strategies".')); + return Promise.reject(new ValidationError('Cannot use both "strategy" and "strategies".')); } - return BPromise.resolve(req); + return Promise.resolve(req); } }; diff --git a/packages/unleash-api/lib/routes/strategy.js b/packages/unleash-api/lib/routes/strategy.js index 8719c08571..5768b3f260 100644 --- a/packages/unleash-api/lib/routes/strategy.js +++ b/packages/unleash-api/lib/routes/strategy.js @@ -1,6 +1,5 @@ 'use strict'; -const BPromise = require('bluebird'); const eventType = require('../event-type'); const logger = require('../logger'); const NameExistsError = require('../error/name-exists-error'); @@ -10,6 +9,30 @@ const validateRequest = require('../error/validate-request'); const extractUser = require('../extract-user'); const version = 1; +const handleError = (req, res, error) => { + switch (error.constructor) { + case NotFoundError: + return res + .status(404) + .end(); + case NameExistsError: + return res + .status(403) + .json([{ msg: `A strategy named '${req.body.name}' already exists.` }]) + .end(); + case ValidationError: + return res + .status(400) + .json(req.validationErrors()) + .end(); + default: + logger.error('Could perfom operation', error); + return res + .status(500) + .end(); + } +}; + module.exports = function (app, config) { const { strategyStore, eventStore } = config.stores; @@ -21,12 +44,8 @@ module.exports = function (app, config) { app.get('/strategies/:name', (req, res) => { strategyStore.getStrategy(req.params.name) - .then(strategy => { - res.json(strategy); - }) - .catch(() => { - res.status(404).json({ error: 'Could not find strategy' }); - }); + .then(strategy => res.json(strategy).end()) + .catch(() => res.status(404).json({ error: 'Could not find strategy' })); }); app.delete('/strategies/:name', (req, res) => { @@ -40,16 +59,8 @@ module.exports = function (app, config) { name: strategyName, }, })) - .then(() => { - res.status(200).end(); - }) - .catch(NotFoundError, () => { - res.status(404).end(); - }) - .catch(err => { - logger.error(`Could not delete strategy=${strategyName}`, err); - res.status(500).end(); - }); + .then(() => res.status(200).end()) + .catch(error => handleError(req, res, error)); }); app.post('/strategies', (req, res) => { @@ -66,26 +77,14 @@ module.exports = function (app, config) { data: newStrategy, })) .then(() => res.status(201).end()) - .catch(NameExistsError, () => { - res.status(403) - .json([{ msg: `A strategy named '${req.body.name}' already exists.` }]) - .end(); - }) - .catch(ValidationError, () => res.status(400).json(req.validationErrors())) - .catch(err => { - logger.error('Could not create strategy', err); - res.status(500).end(); - }); + .catch(error => handleError(req, res, error)); }); function validateStrategyName (req) { - return new BPromise((resolve, reject) => { + return new Promise((resolve, reject) => { strategyStore.getStrategy(req.body.name) - .then(() => { - reject(new NameExistsError('Feature name already exist')); - }, () => { - resolve(req); - }); + .then(() => reject(new NameExistsError('Feature name already exist'))) + .catch(() => resolve(req)); }); } }; diff --git a/packages/unleash-api/package.json b/packages/unleash-api/package.json index e74c41e5e8..830bf9b3e2 100644 --- a/packages/unleash-api/package.json +++ b/packages/unleash-api/package.json @@ -47,7 +47,6 @@ "test:coverage-report": "cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage" }, "dependencies": { - "bluebird": "^3.4.6", "body-parser": "1.15.2", "cookie-parser": "^1.4.1", "db-migrate": "0.10.0-beta.17", @@ -65,6 +64,7 @@ "yallist": "^2.0.0" }, "devDependencies": { + "bluebird": "^3.4.6", "coveralls": "^2.11.12", "istanbul": "^0.4.5", "mocha": "^3.0.2", diff --git a/packages/unleash-api/test/e2e/feature-api.test.js b/packages/unleash-api/test/e2e/feature-api.test.js index 2b338af228..a3af7726d6 100644 --- a/packages/unleash-api/test/e2e/feature-api.test.js +++ b/packages/unleash-api/test/e2e/feature-api.test.js @@ -113,6 +113,14 @@ describe('The features api', () => { .expect(403, done); }); + it('refuses to validate a feature with an existing name', done => { + request + .post('/features-validate') + .send({ name: 'featureX' }) + .set('Content-Type', 'application/json') + .expect(403, done); + }); + describe('new strategies api', () => { it('automatically map existing strategy to strategies array', (done) => { request