diff --git a/lib/routes/feature.js b/lib/routes/feature.js index be2e18e28b..ae59d8a507 100644 --- a/lib/routes/feature.js +++ b/lib/routes/feature.js @@ -65,27 +65,31 @@ module.exports = function (app, config) { app.post('/features', (req, res) => { req.checkBody('name', 'Name is required').notEmpty(); req.checkBody('name', 'Name must match format ^[0-9a-zA-Z\\.\\-]+$').matches(/^[0-9a-zA-Z\\.\\-]+$/i); + const userName = extractUser(req); validateRequest(req) .then(validateFormat) .then(validateUniqueName) - .then(() => eventStore.store({ + .then((req) => legacyFeatureMapper.toNewFormat(req.body)) + .then(validateStrategy) + .then((featureToggle) => eventStore.store({ type: eventType.featureCreated, - createdBy: extractUser(req), - data: legacyFeatureMapper.toNewFormat(req.body), + createdBy: userName, + data: featureToggle, })) .then(() => res.status(201).end()) .catch(error => handleErrors(req, res, error)); }); app.put('/features/:featureName', (req, res) => { - const featureName = req.params.featureName; - const userName = extractUser(req); + const featureName = req.params.featureName; + const userName = extractUser(req); const updatedFeature = legacyFeatureMapper.toNewFormat(req.body); updatedFeature.name = featureName; featureToggleStore.getFeature(featureName) + .then(() => validateStrategy(updatedFeature)) .then(() => eventStore.store({ type: eventType.featureUpdated, createdBy: userName, @@ -96,8 +100,8 @@ module.exports = function (app, config) { }); app.delete('/features/:featureName', (req, res) => { - const featureName = req.params.featureName; - const userName = extractUser(req); + const featureName = req.params.featureName; + const userName = extractUser(req); featureToggleStore.getFeature(featureName) .then(() => eventStore.store({ @@ -126,4 +130,12 @@ module.exports = function (app, config) { return Promise.resolve(req); } + + function validateStrategy (featureToggle) { + if (!featureToggle.strategies || featureToggle.strategies.length === 0) { + return Promise.reject(new ValidationError('You must define at least one strategy')); + } + + return Promise.resolve(featureToggle); + } }; diff --git a/test/e2e/feature-api.test.js b/test/e2e/feature-api.test.js index 73f0dac032..7b00d0e51c 100644 --- a/test/e2e/feature-api.test.js +++ b/test/e2e/feature-api.test.js @@ -43,7 +43,7 @@ test.serial('creates new feature toggle', async t => { const { request, destroy } = await setupApp('feature_api_serial'); return request .post('/features') - .send({ name: 'com.test.feature', enabled: false }) + .send({ name: 'com.test.feature', enabled: false, strategies: [{name: 'default'}] }) .set('Content-Type', 'application/json') .expect(201) .then(destroy); @@ -54,7 +54,7 @@ test.serial('creates new feature toggle with createdBy', async t => { logger.setLevel('FATAL'); request .post('/features') - .send({ name: 'com.test.Username', enabled: false }) + .send({ name: 'com.test.Username', enabled: false, strategies: [{name: 'default'}] }) .set('Cookie', ['username=ivaosthu']) .set('Content-Type', 'application/json') .end(() => { @@ -93,7 +93,7 @@ test.serial('can change status of feature toggle that does exist', async t => { logger.setLevel('FATAL'); return request .put('/features/featureY') - .send({ name: 'featureY', enabled: true }) + .send({ name: 'featureY', enabled: true, strategies: [{name: 'default'}] }) .set('Content-Type', 'application/json') .expect(200).then(destroy); }); diff --git a/test/unit/routes/feature.test.js b/test/unit/routes/feature.test.js index 23d055eefa..1972673c11 100644 --- a/test/unit/routes/feature.test.js +++ b/test/unit/routes/feature.test.js @@ -62,3 +62,23 @@ test('should add version numbers for /features', t => { }); }); +test('should require at least one strategy when creating a feature toggle', t => { + const { request, base } = getSetup(); + + return request + .post(`${base}/features`) + .send({ name: 'sample.missing.strategy' }) + .set('Content-Type', 'application/json') + .expect(400) +}); + +test('should require at least one strategy when updating a feature toggle', t => { + const { request, featureToggleStore, base } = getSetup(); + featureToggleStore.addFeature({ name: 'ts', strategies: [{ name: 'default' }] }); + + return request + .put(`${base}/features/ts`) + .send({ name: 'ts' }) + .set('Content-Type', 'application/json') + .expect(400) +}); diff --git a/test/unit/routes/fixtures/fake-event-store.js b/test/unit/routes/fixtures/fake-event-store.js new file mode 100644 index 0000000000..7c32668241 --- /dev/null +++ b/test/unit/routes/fixtures/fake-event-store.js @@ -0,0 +1,7 @@ +'use strict'; + +module.exports = () => { + return { + store: () => Promise.resolve(), + }; +}; diff --git a/test/unit/routes/fixtures/fake-feature-toggle-store.js b/test/unit/routes/fixtures/fake-feature-toggle-store.js index d25aa78edc..2b2327fcaa 100644 --- a/test/unit/routes/fixtures/fake-feature-toggle-store.js +++ b/test/unit/routes/fixtures/fake-feature-toggle-store.js @@ -4,6 +4,14 @@ module.exports = () => { const _features = []; return { + getFeature: (name) => { + const toggle = _features.find(f => f.name === name); + if (toggle) { + return Promise.resolve(toggle); + } else { + return Promise.reject(); + } + }, getFeatures: () => Promise.resolve(_features), addFeature: (feature) => _features.push(feature), }; diff --git a/test/unit/routes/fixtures/store.js b/test/unit/routes/fixtures/store.js index d88e012e7f..3b2ee9563f 100644 --- a/test/unit/routes/fixtures/store.js +++ b/test/unit/routes/fixtures/store.js @@ -4,6 +4,7 @@ const clientMetricsStore = require('./fake-metrics-store'); const clientStrategyStore = require('./fake-client-strategy-store'); const clientInstanceStore = require('./fake-client-instance-store'); const featureToggleStore = require('./fake-feature-toggle-store'); +const eventStore = require('./fake-event-store'); const strategyStore = require('./fake-strategies-store'); @@ -22,6 +23,7 @@ module.exports = { clientStrategyStore: clientStrategyStore(), clientInstanceStore: clientInstanceStore(), featureToggleStore: featureToggleStore(), + eventStore: eventStore(), strategyStore: strategyStore(), }; },