Refactored + cleaned up validation tools

- General code cleanup
- Removed unused notContains rule
- Swapped custom empty rule for builtin isEmpty rule
- Dropped usage of .extend on validator, as this was removed 2 years ago!
   - This will allow us to upgrade the validator dependency to a much newer version
- Changed our internal validator module to only expose the functions we use.
   - This gives us a clearer Public API
   - It makes it easier to see if we are affected by changes in validator
   - It's still easy to add another validator, we just have to update what we require
   - We can potentially use this to make smaller builds esp for client-side usage
   - Once ripped out into a module we can use ES imports :D
- Rejigged and _slightly_ improved the tests

core/server/data/schema/validator.js

@@ -47,7 +47,7 @@ function validateSchema(tableName, model, options) {
             schema[tableName][columnKey].nullable !== true &&
             !Object.prototype.hasOwnProperty.call(schema[tableName][columnKey], 'defaultTo')
         ) {
-            if (validator.empty(strVal)) {
+            if (validator.isEmpty(strVal)) {
                 message = tpl(messages.valueCannotBeBlank, {
                     tableName: tableName,
                     columnKey: columnKey
@@ -62,7 +62,7 @@ function validateSchema(tableName, model, options) {
         // validate boolean columns
         if (Object.prototype.hasOwnProperty.call(schema[tableName][columnKey], 'type')
             && schema[tableName][columnKey].type === 'bool') {
-            if (!(validator.isBoolean(strVal) || validator.empty(strVal))) {
+            if (!(validator.isBoolean(strVal) || validator.isEmpty(strVal))) {
                 message = tpl(messages.valueMustBeBoolean, {
                     tableName: tableName,
                     columnKey: columnKey
@@ -74,7 +74,7 @@ function validateSchema(tableName, model, options) {
             }
 
             // CASE: ensure we transform 0|1 to false|true
-            if (!validator.empty(strVal)) {
+            if (!validator.isEmpty(strVal)) {
                 model.set(columnKey, !!model.get(columnKey));
             }
         }

core/server/data/validation/validate.js

@@ -37,7 +37,7 @@ function validate(value, key, validations, tableName) {
     let message;
     value = _.toString(value);
 
-    _.each(validations, function each(validationOptions, validationName) {
+    _.each(validations, (validationOptions, validationName) => {
         let goodResult = true;
 
         if (_.isBoolean(validationOptions)) {
@@ -72,7 +72,7 @@ function validate(value, key, validations, tableName) {
         }
 
         validationOptions.shift();
-    }, this);
+    });
 
     return validationErrors;
 }

core/server/data/validation/validator.js

@@ -1,42 +1,48 @@
 const _ = require('lodash');
 
-const validator = require('validator');
+const baseValidator = require('validator');
 const moment = require('moment-timezone');
 const assert = require('assert');
 
+const allowedValidators = [
+    'isLength',
+    'isEmpty',
+    'isURL',
+    'isEmail',
+    'isIn',
+    'isUUID',
+    'isBoolean',
+    'isInt',
+    'isLowercase',
+    'equals',
+    'matches'
+];
+
 function assertString(input) {
-    assert(typeof input === 'string', 'Validator js validates strings only');
+    assert(typeof input === 'string', 'Validator validates strings only');
 }
 
-// extends has been removed in validator >= 5.0.0, need to monkey-patch it back in
-// @TODO: We modify the global validator dependency here! https://github.com/chriso/validator.js/issues/525#issuecomment-213149570
-validator.extend = function (name, fn) {
-    validator[name] = function () {
-        const args = Array.prototype.slice.call(arguments);
-        assertString(args[0]);
-        return fn.apply(validator, args);
-    };
+const validators = {};
+
+allowedValidators.forEach((name) => {
+    if (_.has(baseValidator, name)) {
+        validators[name] = baseValidator[name];
+    }
+});
+
+validators.isTimezone = function isTimezone(str) {
+    assertString(str);
+    return moment.tz.zone(str) ? true : false;
 };
 
-// Provide a few custom validators
-validator.extend('empty', function empty(str) {
-    return _.isEmpty(str);
-});
+validators.isEmptyOrURL = function isEmptyOrURL(str) {
+    assertString(str);
+    return (validators.isEmpty(str) || validators.isURL(str, {require_protocol: false}));
+};
 
-validator.extend('notContains', function notContains(str, badString) {
-    return !_.includes(str, badString);
-});
+validators.isSlug = function isSlug(str) {
+    assertString(str);
+    return validators.matches(str, /^[a-z0-9\-_]+$/);
+};
 
-validator.extend('isTimezone', function isTimezone(str) {
-    return moment.tz.zone(str) ? true : false;
-});
-
-validator.extend('isEmptyOrURL', function isEmptyOrURL(str) {
-    return (_.isEmpty(str) || validator.isURL(str, {require_protocol: false}));
-});
-
-validator.extend('isSlug', function isSlug(str) {
-    return validator.matches(str, /^[a-z0-9\-_]+$/);
-});
-
-module.exports = validator;
+module.exports = validators;

test/unit/data/validation/validate_spec.js

@@ -12,7 +12,5 @@ describe('Validation', function () {
         );
 
         validation.validate.should.be.a.Function();
-
-        validation.validator.should.have.properties(['empty', 'notContains', 'isTimezone', 'isEmptyOrURL', 'isSlug']);
     });
 });

test/unit/data/validation/validator_spec.js

@@ -1,19 +1,43 @@
 const should = require('should');
 
-const validation = require('../../../../core/server/data/validation');
+const {validator} = require('../../../../core/server/data/validation');
 
-describe('Validator dependency', function () {
-    const validator = validation.validator;
+const validators = ['isLength',
+    'isEmpty',
+    'isURL',
+    'isEmail',
+    'isIn',
+    'isUUID',
+    'isBoolean',
+    'isInt',
+    'isLowercase',
+    'equals',
+    'matches'
+];
 
-    it('isEmptyOrUrl filters javascript urls', function () {
-        validator.isEmptyOrURL('javascript:alert(0)').should.be.false();
-        validator.isEmptyOrURL('http://example.com/lol/<script>lalala</script>/').should.be.false();
-        validator.isEmptyOrURL('http://example.com/lol?somequery=<script>lalala</script>').should.be.false();
-        validator.isEmptyOrURL('').should.be.true();
-        validator.isEmptyOrURL('http://localhost:2368').should.be.true();
-        validator.isEmptyOrURL('http://example.com/test/').should.be.true();
-        validator.isEmptyOrURL('http://www.example.com/test/').should.be.true();
-        validator.isEmptyOrURL('http://example.com/foo?somequery=bar').should.be.true();
-        validator.isEmptyOrURL('example.com/test/').should.be.true();
+const custom = ['isTimezone', 'isEmptyOrURL', 'isSlug'];
+
+describe('Validator', function () {
+    it('should export our required functions', function () {
+        should.exist(validator);
+
+        validator.should.have.properties(validators);
+        validator.should.have.properties(custom);
+
+        Object.keys(validator).should.eql(validators.concat(custom));
+    });
+
+    describe('Custom Validators', function () {
+        it('isEmptyOrUrl filters javascript urls', function () {
+            validator.isEmptyOrURL('javascript:alert(0)').should.be.false();
+            validator.isEmptyOrURL('http://example.com/lol/<script>lalala</script>/').should.be.false();
+            validator.isEmptyOrURL('http://example.com/lol?somequery=<script>lalala</script>').should.be.false();
+            validator.isEmptyOrURL('').should.be.true();
+            validator.isEmptyOrURL('http://localhost:2368').should.be.true();
+            validator.isEmptyOrURL('http://example.com/test/').should.be.true();
+            validator.isEmptyOrURL('http://www.example.com/test/').should.be.true();
+            validator.isEmptyOrURL('http://example.com/foo?somequery=bar').should.be.true();
+            validator.isEmptyOrURL('example.com/test/').should.be.true();
+        });
     });
 });