fix: customize allowNull message with notNull validator (#9549)

docs/models-definition.md

@@ -460,7 +460,25 @@ See [the validator.js project][3] for more details on the built in validation me
 
 ### Validators and `allowNull`
 
-If a particular field of a model is set to allow null (with `allowNull: true`) and that value has been set to `null` , its validators do not run. This means you can, for instance, have a string field which validates its length to be at least 5 characters, but which also allows`null`.
+If a particular field of a model is set to allow null (with `allowNull: true`) and that value has been set to `null` , its validators do not run.
+
+This means you can, for instance, have a string field which validates its length to be at least 5 characters, but which also allows `null`.
+
+You can customize `allowNull` error message by setting `notNull` validator, like this
+
+```js
+const User = sequelize.define('user', {
+  name: {
+    type: Sequelize.STRING,
+    allowNull: false,
+    validate: {
+      notNull: {
+        msg: 'Please enter your name'
+      }
+    }
+  }
+});
+```
 
 ### Model validations
 

lib/instance-validator.js

@@ -13,12 +13,11 @@ const _ = require('lodash');
  * The Main Instance Validator.
  *
  * @param {Instance} modelInstance The model instance.
- * @param {Object} options A dict with options.
+ * @param {Object} options A dictionary with options.
  * @constructor
  * @private
  */
 class InstanceValidator {
-
   constructor(modelInstance, options) {
     options = _.clone(options) || {};
 
@@ -66,14 +65,14 @@ class InstanceValidator {
    * @private
    */
   _validate() {
-    if (this.inProgress) {
-      throw new Error('Validations already in progress.');
-    }
+    if (this.inProgress) throw new Error('Validations already in progress.');
+
     this.inProgress = true;
 
-    return Promise.all(
-      [this._builtinValidators(), this._customValidators()].map(promise => promise.reflect())
-    ).then(() => {
+    return Promise.all([
+      this._builtinValidators().reflect(),
+      this._customValidators().reflect()
+    ]).then(() => {
       if (this.errors.length) {
         throw new sequelizeError.ValidationError(null, this.errors);
       }
@@ -125,6 +124,7 @@ class InstanceValidator {
   _builtinValidators() {
     // promisify all attribute invocations
     const validators = [];
+
     _.forIn(this.modelInstance.rawAttributes, (rawAttribute, field) => {
       if (this.options.skip.indexOf(field) >= 0) {
         return;
@@ -337,7 +337,7 @@ class InstanceValidator {
       }
     }
 
-    if (rawAttribute.type === DataTypes.STRING || rawAttribute.type instanceof DataTypes.STRING || rawAttribute.type === DataTypes.TEXT || rawAttribute.type instanceof DataTypes.TEXT) {
+    if (rawAttribute.type instanceof DataTypes.STRING || rawAttribute.type instanceof DataTypes.TEXT) {
       if (Array.isArray(value) || _.isObject(value) && !(value instanceof Utils.SequelizeMethod) && !Buffer.isBuffer(value)) {
         this.errors.push(new sequelizeError.ValidationErrorItem(
           `${field} cannot be an array or an object`,

lib/model.js

@@ -89,7 +89,6 @@ class Model {
     this._changed = {};
     this._modelOptions = this.constructor.options;
     this._options = options || {};
-    this.__eagerlyLoadedAssociations = [];
 
     /**
      * Returns true if this instance has not yet been persisted to the database
@@ -906,7 +905,11 @@ class Model {
       attribute = this.sequelize.normalizeAttribute(attribute);
 
       if (attribute.type === undefined) {
-        throw new Error('Unrecognized data type for field ' + name);
+        throw new Error(`Unrecognized datatype for attribute "${this.name}.${name}"`);
+      }
+
+      if (attribute.allowNull !== false && _.get(attribute, 'validate.notNull')) {
+        throw new Error(`Invalid definition for "${this.name}.${name}", "notNull" validator is only allowed with "allowNull:false"`);
       }
 
       if (_.get(attribute, 'references.model.prototype') instanceof Model) {

lib/utils/validator-extras.js

@@ -77,9 +77,8 @@ function extendModelValidations(modelInstance) {
 }
 exports.extendModelValidations = extendModelValidations;
 
-// Deprecate this.
-validator.notNull = function() {
-  throw new Error('Warning "notNull" validation has been deprecated in favor of Schema based "allowNull"');
+validator.notNull = function(val) {
+  return ![null, undefined].includes(val);
 };
 
 // https://github.com/chriso/validator.js/blob/6.2.0/validator.js

test/integration/model/create.test.js

@@ -1063,13 +1063,13 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         self.sequelize.define('UserBadDataType', {
           activity_date: Sequelize.DATe
         });
-      }).to.throw(Error, 'Unrecognized data type for field activity_date');
+      }).to.throw(Error, 'Unrecognized datatype for attribute "UserBadDataType.activity_date"');
 
       expect(() => {
         self.sequelize.define('UserBadDataType', {
           activity_date: {type: Sequelize.DATe}
         });
-      }).to.throw(Error, 'Unrecognized data type for field activity_date');
+      }).to.throw(Error, 'Unrecognized datatype for attribute "UserBadDataType.activity_date"');
     });
 
     it('sets a 64 bit int in bigint', function() {

test/unit/model/define.test.js

@@ -41,6 +41,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         });
       }).to.throw("A column called 'id' was added to the attributes of 'bars' but not marked with 'primaryKey: true'");
     });
+
     it('should defend against null or undefined "unique" attributes', () => {
       expect(() => {
         current.define('baz', {
@@ -58,5 +59,44 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         });
       }).not.to.throw();
     });
+
+    it('should throw for unknown data type', () => {
+      expect(() => {
+        current.define('bar', {
+          name: {
+            type: DataTypes.MY_UNKNOWN_TYPE
+          }
+        });
+      }).to.throw('Unrecognized datatype for attribute "bar.name"');
+    });
+
+    it('should throw for notNull validator without allowNull', () => {
+      expect(() => {
+        current.define('user', {
+          name: {
+            type: DataTypes.STRING,
+            allowNull: true,
+            validate: {
+              notNull: {
+                msg: 'Please enter the name'
+              }
+            }
+          }
+        });
+      }).to.throw('Invalid definition for "user.name", "notNull" validator is only allowed with "allowNull:false"');
+
+      expect(() => {
+        current.define('part', {
+          name: {
+            type: DataTypes.STRING,
+            validate: {
+              notNull: {
+                msg: 'Please enter the part name'
+              }
+            }
+          }
+        });
+      }).to.throw('Invalid definition for "part.name", "notNull" validator is only allowed with "allowNull:false"');
+    });
   });
 });