From d89ce7590699fb216dbc37c868de0ceab14f7662 Mon Sep 17 00:00:00 2001 From: Jari Bakken Date: Tue, 25 Nov 2014 14:41:11 +0100 Subject: [PATCH 1/4] Test refactoring: * reduce setup duplication * consistent test file names --- test/eventApi.test.js | 42 ------------------------------------------ test/eventApiSpec.js | 23 +++++++++++++++++++++++ test/featureApiSpec.js | 29 +++++------------------------ test/routerSpec.js | 18 ++++++++++++++++++ test/routers.test.js | 38 -------------------------------------- test/specHelper.js | 35 +++++++++++++++++++++++++++++++++++ 6 files changed, 81 insertions(+), 104 deletions(-) delete mode 100644 test/eventApi.test.js create mode 100644 test/eventApiSpec.js create mode 100644 test/routerSpec.js delete mode 100644 test/routers.test.js create mode 100644 test/specHelper.js diff --git a/test/eventApi.test.js b/test/eventApi.test.js deleted file mode 100644 index 8480c620ad..0000000000 --- a/test/eventApi.test.js +++ /dev/null @@ -1,42 +0,0 @@ -var request = require('supertest'), - mockery = require('mockery'); - -describe('The event api', function () { - var server; - - before(function () { - mockery.enable({ - warnOnReplace: false, - warnOnUnregistered: false, - useCleanCache: true - }); - - mockery.registerSubstitute('./eventDb', '../test/eventDbMock'); - mockery.registerSubstitute('./featureDb', '../test/featureDbMock'); - mockery.registerSubstitute('./strategyDb', '../test/strategyDbMock'); - - server = require('../server'); - request = request('http://localhost:' + server.app.get('port')); - }); - - after(function () { - mockery.disable(); - mockery.deregisterAll(); - server.server.close(); - }); - - it('returns events', function (done) { - request - .get('/events') - .expect('Content-Type', /json/) - .expect(200, done); - }); - - it('returns events given a name', function (done) { - request - .get('/events/myname') - .expect('Content-Type', /json/) - .expect(200, done); - }); - -}); \ No newline at end of file diff --git a/test/eventApiSpec.js b/test/eventApiSpec.js new file mode 100644 index 0000000000..3191aef91a --- /dev/null +++ b/test/eventApiSpec.js @@ -0,0 +1,23 @@ +var specHelper = require('./specHelper'); + +describe('The event api', function () { + var request; + + before(function () { request = specHelper.setupMockServer(); }); + after(specHelper.tearDownMockServer); + + it('returns events', function (done) { + request + .get('/events') + .expect('Content-Type', /json/) + .expect(200, done); + }); + + it('returns events given a name', function (done) { + request + .get('/events/myname') + .expect('Content-Type', /json/) + .expect(200, done); + }); + +}); \ No newline at end of file diff --git a/test/featureApiSpec.js b/test/featureApiSpec.js index 52ac7e8eab..60c210b12d 100644 --- a/test/featureApiSpec.js +++ b/test/featureApiSpec.js @@ -1,29 +1,10 @@ -var request = require('supertest'); -var mockery = require('mockery'); +var specHelper = require('./specHelper'); -describe('The api', function () { - var server; +describe('The features api', function () { + var request; - before(function () { - mockery.enable({ - warnOnReplace: false, - warnOnUnregistered: false, - useCleanCache: true - }); - - mockery.registerSubstitute('./eventDb', '../test/eventDbMock'); - mockery.registerSubstitute('./featureDb', '../test/featureDbMock'); - mockery.registerSubstitute('./strategyDb', '../test/strategyDbMock'); - - server = require('../server'); - request = request('http://localhost:' + server.app.get('port')); - }); - - after(function () { - mockery.disable(); - mockery.deregisterAll(); - server.server.close(); - }); + before(function () { request = specHelper.setupMockServer(); }); + after(specHelper.tearDownMockServer); it('returns three mocked feature toggles', function (done) { request diff --git a/test/routerSpec.js b/test/routerSpec.js new file mode 100644 index 0000000000..35ada85e1d --- /dev/null +++ b/test/routerSpec.js @@ -0,0 +1,18 @@ +var specHelper = require('./specHelper'); + +describe('The routes', function () { + var request; + + before(function () { request = specHelper.setupMockServer(); }); + after(specHelper.tearDownMockServer); + + describe('healthcheck', function () { + it('returns health good', function (done) { + request.get('/health') + .expect('Content-Type', /json/) + .expect(200) + .expect('{"health":"GOOD"}', done); + }); + }); + +}); \ No newline at end of file diff --git a/test/routers.test.js b/test/routers.test.js deleted file mode 100644 index 7345bf639b..0000000000 --- a/test/routers.test.js +++ /dev/null @@ -1,38 +0,0 @@ -var request = require('supertest'), - mockery = require('mockery'); - -describe('The routes', function () { - var server; - - before(function () { - mockery.enable({ - warnOnReplace: false, - warnOnUnregistered: false, - useCleanCache: true - }); - - mockery.registerSubstitute('./eventDb', '../test/eventDbMock'); - mockery.registerSubstitute('./featureDb', '../test/featureDbMock'); - mockery.registerSubstitute('./strategyDb', '../test/strategyDbMock'); - - server = require('../server'); - request = request('http://localhost:' + server.app.get('port')); - }); - - after(function () { - mockery.disable(); - mockery.deregisterAll(); - server.server.close(); - }); - - describe('healthcheck', function () { - - it('returns health good', function (done) { - request.get('/health') - .expect('Content-Type', /json/) - .expect(200) - .expect('{"health":"GOOD"}', done); - }); - }); - -}); \ No newline at end of file diff --git a/test/specHelper.js b/test/specHelper.js new file mode 100644 index 0000000000..840a6f9864 --- /dev/null +++ b/test/specHelper.js @@ -0,0 +1,35 @@ +var request = require('supertest'); +var mockery = require('mockery'); + +var server; + +function setupMockServer() { + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }); + + mockery.registerSubstitute('./eventDb', '../test/eventDbMock'); + mockery.registerSubstitute('./featureDb', '../test/featureDbMock'); + mockery.registerSubstitute('./strategyDb', '../test/strategyDbMock'); + + server = require('../server'); + + return request('http://localhost:' + server.app.get('port')); +} + +function tearDownMockServer() { + mockery.disable(); + mockery.deregisterAll(); + + if (server) { + server.server.close(); + server = null; + } +} + +module.exports = { + setupMockServer: setupMockServer, + tearDownMockServer: tearDownMockServer +}; \ No newline at end of file From 20c4fe67026fae3ee5ee94d57ebea0864b26c2df Mon Sep 17 00:00:00 2001 From: Jari Bakken Date: Tue, 25 Nov 2014 15:28:08 +0100 Subject: [PATCH 2/4] Add ability to create custom stratgies. Closes #11. --- lib/featureApi.js | 14 ++--- lib/strategyApi.js | 62 +++++++++++-------- .../strategy/StrategiesComponent.jsx | 20 ++++-- public/js/stores/EventStore.js | 4 +- public/js/stores/FeatureStore.js | 14 ++--- public/js/stores/StrategyStore.js | 4 +- test/featureApiSpec.js | 7 +++ test/strategyApiSpec.js | 40 ++++++++++++ test/strategyDbMock.js | 8 +-- 9 files changed, 117 insertions(+), 56 deletions(-) create mode 100644 test/strategyApiSpec.js diff --git a/lib/featureApi.js b/lib/featureApi.js index 3f511d423d..a9a7a10313 100644 --- a/lib/featureApi.js +++ b/lib/featureApi.js @@ -1,6 +1,6 @@ -var eventStore = require('./eventStore'), - eventType = require('./eventType'), - featureDb = require('./featureDb'); +var eventStore = require('./eventStore'); +var eventType = require('./eventType'); +var featureDb = require('./featureDb'); module.exports = function (app) { @@ -44,11 +44,9 @@ module.exports = function (app) { type: eventType.featureCreated, createdBy: req.connection.remoteAddress, data: newFeature - }).then(function () { - res.status(201).end(); - }, function () { - res.status(500).end(); - }); + }) + .then(function () { res.status(201).end(); }) + .catch(function () { res.status(500).end(); }); }; featureDb.getFeature(newFeature.name) diff --git a/lib/strategyApi.js b/lib/strategyApi.js index 19413f1236..ec46c0f411 100644 --- a/lib/strategyApi.js +++ b/lib/strategyApi.js @@ -1,25 +1,7 @@ +var eventStore = require('./eventStore'); +var eventType = require('./eventType'); var strategyDb = require('./strategyDb'); -var strategies = [ - { - name: "default", - description: "Default on or off Strategy." - }, - { - name: "usersWithEmail", - description: "Active for users defined in the comma-separated emails-parameter.", - parametersTemplate: { - emails: "String" - } - } -]; - -function byName(name) { - return strategies.filter(function(s) { - return s.name === name; - })[0]; -} - module.exports = function (app) { app.get('/strategies', function (req, res) { @@ -29,16 +11,42 @@ module.exports = function (app) { }); app.get('/strategies/:name', function (req, res) { - var strategy = byName(req.params.name); - if (strategy) { - res.json(strategy); - } else { - res.json(404, {error: 'Could not find strategy'}); - } + strategyDb.getStrategy(req.params.name) + .then(function (strategy) { res.json(strategy); }) + .catch(function () { res.json(404, {error: 'Could not find strategy'}); }); }); app.post('/strategies', function (req, res) { - res.json(500, {error: 'Not implemented yet'}); + 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 newStrategy = req.body; + + var handleStrategyExists = function() { + var errors = [{msg: "A strategy named " + newStrategy.name + " already exists."}]; + res.status(403).json(errors); + }; + + var handleCreateStrategy = function() { + eventStore.create({ + type: eventType.strategyCreated, + createdBy: req.connection.remoteAddress, + data: newStrategy + }) + .then(function () { res.status(201).end(); }) + .catch(function () { res.status(500).end(); }); + }; + + strategyDb.getStrategy(newStrategy.name) + .then(handleStrategyExists) + .catch(handleCreateStrategy); }); }; diff --git a/public/js/components/strategy/StrategiesComponent.jsx b/public/js/components/strategy/StrategiesComponent.jsx index 092f5bdd5e..9fc29a97f0 100644 --- a/public/js/components/strategy/StrategiesComponent.jsx +++ b/public/js/components/strategy/StrategiesComponent.jsx @@ -41,12 +41,20 @@ var StrategiesComponent = React.createClass({ }, onSave: function(strategy) { - var strategies = this.state.strategies.concat([strategy]); - this.setState({ - createView: false, - strategies: strategies - }); - console.log("Saved strategy: ", strategy); + function handleSuccess() { + var strategies = this.state.strategies.concat([strategy]); + + this.setState({ + createView: false, + strategies: strategies + }); + + console.log("Saved strategy: ", strategy); + } + + strategyStore.createStrategy(strategy) + .then(handleSuccess.bind(this)) + .catch(this.onError); }, render: function() { diff --git a/public/js/stores/EventStore.js b/public/js/stores/EventStore.js index a26ffe135f..696fb66c22 100644 --- a/public/js/stores/EventStore.js +++ b/public/js/stores/EventStore.js @@ -1,7 +1,7 @@ var reqwest = require('reqwest'); -TYPE = 'json'; -CONTENT_TYPE = 'application/json'; +var TYPE = 'json'; +var CONTENT_TYPE = 'application/json'; var EventStore = { getEvents: function () { diff --git a/public/js/stores/FeatureStore.js b/public/js/stores/FeatureStore.js index 7dc52c5d5a..b70dbd2932 100644 --- a/public/js/stores/FeatureStore.js +++ b/public/js/stores/FeatureStore.js @@ -3,16 +3,16 @@ var reqwest = require('reqwest'); var FeatureStore = function () { }; -FeatureStore.TYPE = 'json'; -FeatureStore.CONTENT_TYPE = 'application/json'; +var TYPE = 'json'; +var CONTENT_TYPE = 'application/json'; FeatureStore.prototype = { updateFeature: function (feature) { return reqwest({ url: 'features/' + feature.name, method: 'put', - type: FeatureStore.TYPE, - contentType: FeatureStore.CONTENT_TYPE, + type: TYPE, + contentType: CONTENT_TYPE, data: JSON.stringify(feature) }); }, @@ -21,8 +21,8 @@ FeatureStore.prototype = { return reqwest({ url: 'features', method: 'post', - type: FeatureStore.TYPE, - contentType: FeatureStore.CONTENT_TYPE, + type: TYPE, + contentType: CONTENT_TYPE, data: JSON.stringify(feature) }); }, @@ -31,7 +31,7 @@ FeatureStore.prototype = { return reqwest({ url: 'features', method: 'get', - type: FeatureStore.TYPE + type: TYPE }); } }; diff --git a/public/js/stores/StrategyStore.js b/public/js/stores/StrategyStore.js index e28febca7c..80674d9e38 100644 --- a/public/js/stores/StrategyStore.js +++ b/public/js/stores/StrategyStore.js @@ -1,7 +1,7 @@ var reqwest = require('reqwest'); -TYPE = 'json'; -CONTENT_TYPE = 'application/json'; +var TYPE = 'json'; +var CONTENT_TYPE = 'application/json'; var StrategyStore = { createStrategy: function (strategy) { diff --git a/test/featureApiSpec.js b/test/featureApiSpec.js index 60c210b12d..50a6cd2895 100644 --- a/test/featureApiSpec.js +++ b/test/featureApiSpec.js @@ -13,6 +13,13 @@ describe('The features api', function () { .expect(200, done); }); + it('gets a feature by name', function (done) { + request + .get('/features/featureX') + .expect('Content-Type', /json/) + .expect(200, done); + }); + it('creates new feature toggle', function (done) { request .post('/features') diff --git a/test/strategyApiSpec.js b/test/strategyApiSpec.js new file mode 100644 index 0000000000..d11c1e0200 --- /dev/null +++ b/test/strategyApiSpec.js @@ -0,0 +1,40 @@ +var specHelper = require('./specHelper'); + +describe('The strategy api', function () { + var request; + + before(function () { request = specHelper.setupMockServer(); }); + after(specHelper.tearDownMockServer); + + it('gets all strategies', function (done) { + request + .get('/strategies') + .expect('Content-Type', /json/) + .expect(200, done); + }); + + it('gets a strategy by name', function (done) { + request + .get('/strategies/default') + .expect('Content-Type', /json/) + .expect(200, done); + }); + + it('creates a new strategy', function (done) { + request + .post('/strategies') + .send({name: 'myCustomStrategy', description: 'Best strategy ever.'}) + .set('Content-Type', 'application/json') + .expect(201, done); + }); + + it('requires new strategies to have a name', function (done) { + request + .post('/strategies') + .send({name: ''}) + .set('Content-Type', 'application/json') + .expect(400, done); + }); + + +}); \ No newline at end of file diff --git a/test/strategyDbMock.js b/test/strategyDbMock.js index e518d51c08..ebc1400c64 100644 --- a/test/strategyDbMock.js +++ b/test/strategyDbMock.js @@ -26,10 +26,10 @@ module.exports = { resolve(strategies); }); }, - getFeature: function(name) { - var feature = byName(name); - if(feature) { - return Promise.resolve(feature); + getStrategy: function(name) { + var strategy = byName(name); + if(strategy) { + return Promise.resolve(strategy); } else { return Promise.reject("strategy not found"); } From 459cb30badf115409529a50485b2c2a63c382d36 Mon Sep 17 00:00:00 2001 From: Jari Bakken Date: Tue, 25 Nov 2014 15:52:15 +0100 Subject: [PATCH 3/4] StrategyForm should use 'default' as default strategy for empty forms. --- public/js/components/feature/FeatureForm.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/js/components/feature/FeatureForm.jsx b/public/js/components/feature/FeatureForm.jsx index 783db336b7..243a49a62e 100644 --- a/public/js/components/feature/FeatureForm.jsx +++ b/public/js/components/feature/FeatureForm.jsx @@ -71,7 +71,7 @@ var FeatureForm = React.createClass({ }, renderStrategyOptions: function() { - var currentStrategy = this.props.feature ? this.props.feature.strategy : ""; + var currentStrategy = this.props.feature ? this.props.feature.strategy : "default"; return this.state.strategyOptions.map(function(name) { return ( From 74df1aa6ec339f7dc480e8ad3680ef285dc0aa49 Mon Sep 17 00:00:00 2001 From: Jari Bakken Date: Tue, 25 Nov 2014 16:34:24 +0100 Subject: [PATCH 4/4] Add test for /strategies/:name 403 if strategy exists. --- test/strategyApiSpec.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/strategyApiSpec.js b/test/strategyApiSpec.js index d11c1e0200..5da09317ac 100644 --- a/test/strategyApiSpec.js +++ b/test/strategyApiSpec.js @@ -36,5 +36,13 @@ describe('The strategy api', function () { .expect(400, done); }); + it('refuses to create a strategy with an existing name', function (done) { + request + .post('/strategies') + .send({name: 'default'}) + .set('Content-Type', 'application/json') + .expect(403, done); + }); + }); \ No newline at end of file