chore(modernize): Use joi schema-validation in FeatureController

lib/error/name-invalid-error.js

@@ -0,0 +1,14 @@
+'use strict';
+
+class NameInvalidError extends Error {
+    constructor(message) {
+        super();
+        Error.captureStackTrace(this, this.constructor);
+
+        this.name = this.constructor.name;
+        this.message = message;
+        this.msg = message;
+    }
+}
+
+module.exports = NameInvalidError;

lib/routes/admin-api/feature-schema.js

@@ -0,0 +1,30 @@
+'use strict';
+
+const joi = require('joi');
+
+const strategiesSchema = joi.object().keys({
+    name: joi
+        .string()
+        .regex(/^[a-zA-Z0-9\\.\\-]{3,100}$/)
+        .required(),
+    parameters: joi.object(),
+});
+
+const featureShema = joi
+    .object()
+    .keys({
+        name: joi
+            .string()
+            .regex(/^[a-zA-Z0-9\\.\\-]{3,100}$/)
+            .required(),
+        enabled: joi.boolean().default(false),
+        description: joi.string(),
+        strategies: joi
+            .array()
+            .required()
+            .min(1)
+            .items(strategiesSchema),
+    })
+    .options({ allowUnknown: false, stripUnknown: true });
+
+module.exports = { featureShema, strategiesSchema };

lib/routes/admin-api/feature.js

@@ -10,25 +10,32 @@ const {
     FEATURE_ARCHIVED,
 } = require('../../event-type');
 const NameExistsError = require('../../error/name-exists-error');
-const NotFoundError = require('../../error/notfound-error');
-const ValidationError = require('../../error/validation-error.js');
-const validateRequest = require('../../error/validate-request');
+const NameInvalidError = require('../../error/name-invalid-error');
+const { isUrlFriendlyName } = require('./util');
+
 const extractUser = require('../../extract-user');
 
+const { featureShema } = require('./feature-schema');
+
 const handleErrors = (req, res, error) => {
     logger.warn('Error creating or updating feature', error);
-    switch (error.constructor) {
-        case NotFoundError:
+    switch (error.name) {
+        case 'NotFoundError':
             return res.status(404).end();
-        case NameExistsError:
+        case 'NameInvalidError':
+            return res
+                .status(400)
+                .json([{ msg: error.message }])
+                .end();
+        case 'NameExistsError':
             return res
                 .status(403)
                 .json([{ msg: error.message }])
                 .end();
-        case ValidationError:
+        case 'ValidationError':
             return res
                 .status(400)
-                .json(req.validationErrors())
+                .json(error)
                 .end();
         default:
             logger.error('Server failed executing request', error);
@@ -36,39 +43,6 @@ const handleErrors = (req, res, error) => {
     }
 };
 
-const strategiesSchema = joi.object().keys({
-    name: joi
-        .string()
-        .regex(/^[a-zA-Z0-9\\.\\-]{3,100}$/)
-        .required(),
-    parameters: joi.object(),
-});
-
-function validateStrategy(featureToggle) {
-    return new Promise((resolve, reject) => {
-        if (
-            !featureToggle.strategies ||
-            featureToggle.strategies.length === 0
-        ) {
-            return reject(
-                new ValidationError('You must define at least one strategy')
-            );
-        }
-
-        featureToggle.strategies = featureToggle.strategies.map(
-            strategyConfig => {
-                const result = joi.validate(strategyConfig, strategiesSchema);
-                if (result.error) {
-                    throw result.error;
-                }
-                return result.value;
-            }
-        );
-
-        return resolve(featureToggle);
-    });
-}
-
 const version = 1;
 
 class FeatureController extends Controller {
@@ -104,42 +78,49 @@ class FeatureController extends Controller {
     }
 
     async validate(req, res) {
-        req.checkBody('name', 'Name is required').notEmpty();
-        req.checkBody('name', 'Name must be URL friendly').isUrlFriendlyName();
+        const name = req.body.name;
 
         try {
-            await validateRequest(req);
-            await this.validateUniqueName(req);
+            await this.validateNameFormat(name);
+            await this.validateUniqueName(name);
             res.status(201).end();
         } catch (error) {
             handleErrors(req, res, error);
         }
     }
 
-    validateUniqueName(req) {
-        return new Promise((resolve, reject) => {
-            this.featureToggleStore
-                .hasFeature(req.body.name)
-                .then(definition => {
-                    const msg = definition.archived
-                        ? 'An archived toggle with that name already exist'
-                        : 'A toggle with that name already exist';
-                    reject(new NameExistsError(msg));
-                })
-                .catch(() => resolve(req));
-        });
+    // TODO: cleanup this validation
+    async validateUniqueName(name) {
+        let msg;
+        try {
+            const definition = await this.featureToggleStore.hasFeature(name);
+            msg = definition.archived
+                ? 'An archived toggle with that name already exist'
+                : 'A toggle with that name already exist';
+        } catch (error) {
+            // No conflict, everything ok!
+            return;
+        }
+
+        // Interntional throw here!
+        throw new NameExistsError(msg);
+    }
+
+    // TODO: this should be part of the schema validation!
+    validateNameFormat(name) {
+        if (!isUrlFriendlyName(name)) {
+            throw new NameInvalidError('Name must be URL friendly');
+        }
     }
 
     async createToggle(req, res) {
-        req.checkBody('name', 'Name is required').notEmpty();
-        req.checkBody('name', 'Name must be URL friendly').isUrlFriendlyName();
-
+        const toggleName = req.body.name;
         const userName = extractUser(req);
 
         try {
-            await validateRequest(req);
-            await this.validateUniqueName(req);
-            const featureToggle = await validateStrategy(req.body);
+            await this.validateNameFormat(toggleName);
+            await this.validateUniqueName(toggleName);
+            const featureToggle = await joi.validate(req.body, featureShema);
             await this.eventStore.store({
                 type: FEATURE_CREATED,
                 createdBy: userName,
@@ -160,7 +141,7 @@ class FeatureController extends Controller {
 
         try {
             await this.featureToggleStore.getFeature(featureName);
-            await validateStrategy(updatedFeature);
+            await joi.validate(updatedFeature, featureShema);
             await this.eventStore.store({
                 type: FEATURE_UPDATED,
                 createdBy: userName,

lib/routes/admin-api/feature.test.js

@@ -200,3 +200,23 @@ test('invalid feature names should not pass validation', t => {
         )
     );
 });
+
+test('invalid feature names should have error msg', t => {
+    t.plan(1);
+    const { request, base } = getSetup();
+
+    const name = 'ØÆ`';
+
+    return request
+        .post(`${base}/api/admin/features`)
+        .send({
+            name,
+            enabled: false,
+            strategies: [{ name: 'default' }],
+        })
+        .set('Content-Type', 'application/json')
+        .expect(400)
+        .expect(res => {
+            t.true(res.body[0].msg === 'Name must be URL friendly');
+        });
+});

lib/routes/admin-api/util.js

@@ -0,0 +1,5 @@
+'use strict';
+
+const isUrlFriendlyName = input => encodeURIComponent(input) === input;
+
+module.exports = { isUrlFriendlyName };