Merge pull request #170 from Unleash/add_strategies_check

Added strategies validation when creating/updating toggles

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),
+                createdBy: userName,
-                data: legacyFeatureMapper.toNewFormat(req.body),
+                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 featureName = req.params.featureName;
-        const userName       = extractUser(req);
+        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 featureName = req.params.featureName;
-        const userName       = extractUser(req);
+        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);
+    }
 };

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);
 });

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)
+});

test/unit/routes/fixtures/fake-event-store.js

@@ -0,0 +1,7 @@
+'use strict';
+
+module.exports = () => {
+    return {
+        store: () => Promise.resolve(),
+    };
+};

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),
     };

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(),
         };
     },