From ae0fdce395cd065b28043f9c8abd6279e74345fe Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Wed, 10 Dec 2014 18:45:02 +0100 Subject: [PATCH] Cleaned up and changed promises used in the feature API --- lib/error/NameExistsError.js | 13 ++++++ lib/error/NotFoundError.js | 13 ++++++ lib/error/ValidationError.js | 13 ++++++ lib/error/requestValidator.js | 14 ++++++ lib/featureApi.js | 84 +++++++++++++++++------------------ lib/featureDb.js | 19 +++++++- test/featureDbMock.js | 3 +- 7 files changed, 113 insertions(+), 46 deletions(-) create mode 100644 lib/error/NameExistsError.js create mode 100644 lib/error/NotFoundError.js create mode 100644 lib/error/ValidationError.js create mode 100644 lib/error/requestValidator.js diff --git a/lib/error/NameExistsError.js b/lib/error/NameExistsError.js new file mode 100644 index 0000000000..ecee9feedf --- /dev/null +++ b/lib/error/NameExistsError.js @@ -0,0 +1,13 @@ +var util = require('util'); + +function NameExistsError(message) { + Error.call(this); + Error.captureStackTrace(this, this.constructor); + + this.name = this.constructor.name; + this.message = message; +} + +util.inherits(NameExistsError, Error); + +module.exports = NameExistsError; \ No newline at end of file diff --git a/lib/error/NotFoundError.js b/lib/error/NotFoundError.js new file mode 100644 index 0000000000..235cb85313 --- /dev/null +++ b/lib/error/NotFoundError.js @@ -0,0 +1,13 @@ +var util = require('util'); + +function NotFoundError(message) { + Error.call(this); + Error.captureStackTrace(this, this.constructor); + + this.name = this.constructor.name; + this.message = message; +} + +util.inherits(NotFoundError, Error); + +module.exports = NotFoundError; \ No newline at end of file diff --git a/lib/error/ValidationError.js b/lib/error/ValidationError.js new file mode 100644 index 0000000000..a1f624de49 --- /dev/null +++ b/lib/error/ValidationError.js @@ -0,0 +1,13 @@ +var util = require('util'); + +function ValidationError(message) { + Error.call(this); + Error.captureStackTrace(this, this.constructor); + + this.name = this.constructor.name; + this.message = message; +} + +util.inherits(ValidationError, Error); + +module.exports = ValidationError; \ No newline at end of file diff --git a/lib/error/requestValidator.js b/lib/error/requestValidator.js new file mode 100644 index 0000000000..58819b0730 --- /dev/null +++ b/lib/error/requestValidator.js @@ -0,0 +1,14 @@ +var Promise = require("bluebird"); +var ValidationError = require('./ValidationError'); + +function validateRequest(req) { + return new Promise(function(resolve, reject) { + if (req.validationErrors()) { + reject(new ValidationError("Invalid syntax")); + } else { + resolve(req); + } + }); +} + +module.exports = validateRequest; \ No newline at end of file diff --git a/lib/featureApi.js b/lib/featureApi.js index a9a7a10313..b34b6e9079 100644 --- a/lib/featureApi.js +++ b/lib/featureApi.js @@ -1,6 +1,11 @@ +var logger = require('./logger'); var eventStore = require('./eventStore'); var eventType = require('./eventType'); var featureDb = require('./featureDb'); +var NameExistsError = require('./error/NameExistsError'); +var NotFoundError = require('./error/NotFoundError'); +var ValidationError = require('./error/ValidationError'); +var requestValidator = require('./error/requestValidator'); module.exports = function (app) { @@ -22,62 +27,55 @@ module.exports = function (app) { req.checkBody('name', 'Name is required').notEmpty(); req.checkBody('name', 'Name must match format ^[a-zA-Z\\.\\-]+$').matches(/^[a-zA-Z\\.\\-]+$/i); - var errors = req.validationErrors(); - - if (errors) { - res.status(400).json(errors); - return; - } - - var newFeature = req.body; - - var handleFeatureExist = function() { - var errors = [ - {msg: "A feature named '" + newFeature.name + "' already exists."} - ]; - - res.status(403).json(errors).end(); - }; - - var handleCreateFeature = function () { - eventStore.create({ - type: eventType.featureCreated, - createdBy: req.connection.remoteAddress, - data: newFeature + requestValidator(req) + .then(featureDb.validateUniqueName) + .then(function() { + return eventStore.create({ + type: eventType.featureCreated, + createdBy: req.connection.remoteAddress, + data: req.body + }); }) - .then(function () { res.status(201).end(); }) - .catch(function () { res.status(500).end(); }); - }; - - featureDb.getFeature(newFeature.name) - .then(handleFeatureExist) - .catch(handleCreateFeature); - }); + .then(function () { + res.status(201).end(); + }) + .catch(NameExistsError, function() { + res.status(403).json([{msg: "A feature named '" + req.body.name + "' already exists."}]).end(); + }) + .catch(ValidationError, function() { + res.status(400).json(req.validationErrors()); + }) + .catch(function(err) { + logger.error("Could not create feature toggle", err); + res.status(500).end(); + }); + }); app.put('/features/:featureName', function (req, res) { var featureName = req.params.featureName; - var createdBy = req.connection.remoteAddress; + var userName = req.connection.remoteAddress; var updatedFeature = req.body; updatedFeature.name = featureName; - var event = { - type: eventType.featureUpdated, - createdBy: createdBy, - data: updatedFeature - }; - featureDb.getFeature(featureName) .then(function () { - eventStore - .create(event) - .then(function () { res.status(200).end(); }) - .catch(function () { res.status(500).end(); }); + return eventStore.create({ + type: eventType.featureUpdated, + createdBy: userName, + data: updatedFeature + }); }) - .catch(function () { + .then(function () { + res.status(200).end(); + }) + .catch(NotFoundError, function () { res.status(404).end(); + }) + .catch(function (err) { + logger.error("Could not update feature toggle="+featureName, err); + res.status(500).end(); }); }); - }; diff --git a/lib/featureDb.js b/lib/featureDb.js index bca0732023..3b94289b3a 100644 --- a/lib/featureDb.js +++ b/lib/featureDb.js @@ -2,6 +2,9 @@ var eventStore = require('./eventStore'); var eventType = require('./eventType'); var logger = require('./logger'); var knex = require('./dbPool'); +var Promise = require("bluebird"); +var NotFoundError = require('./error/NotFoundError'); +var NameExistsError = require('./error/NameExistsError'); var FEATURE_COLUMNS = ['name', 'description', 'enabled', 'strategy_name', 'parameters']; eventStore.on(eventType.featureCreated, function (event) { @@ -39,7 +42,7 @@ function getFeature(name) { function rowToFeature(row) { if (!row) { - throw new Error('invalid row'); + throw new NotFoundError('No feature toggle found'); } return { @@ -61,8 +64,20 @@ function eventToRow(event) { }; } +function validateUniqueName(req) { + return new Promise(function(resolve, reject) { + getFeature(req.body.name) + .then(function() { + reject(new NameExistsError("Feature name already exist")); + }, function() { + resolve(req); + }); + }); +} + module.exports = { getFeatures: getFeatures, - getFeature: getFeature + getFeature: getFeature, + validateUniqueName: validateUniqueName }; diff --git a/test/featureDbMock.js b/test/featureDbMock.js index 9ecdf12d58..fcf9082ca4 100644 --- a/test/featureDbMock.js +++ b/test/featureDbMock.js @@ -1,4 +1,5 @@ var Promise = require("bluebird"); +var NotFoundError = require('../lib/error/NotFoundError'); var features = [ { @@ -49,7 +50,7 @@ module.exports = { if(feature) { return Promise.resolve(feature); } else { - return Promise.reject("feature not found"); + return Promise.reject(new NotFoundError("feature not found")); } } }; \ No newline at end of file