Strict mode now always return validationError

- Deprecation of strict:validate and strict:throw
- When strict mode is enabled, it will now always
return validation error (previous strict:validate)

3.0-RELEASE-NOTES.md

@@ -102,3 +102,33 @@ bulk `updateOrCreate`, you could use `async.each` or `Promise.all` to
 repeatedly call `updateOfCreate` for each instance.
 
 See [related code change](https://github.com/strongloop/loopback-datasource-juggler/pull/889) here.
+
+## Removing strict: validate and strict: throw
+
+- `strict: 'validate'` and `strict: 'throw'` are removed in favor of `strict: true`.
+
+- `strict: true`'s behavior change:
+    Given a user model definition as follow:
+    ```js
+    ds.define('User', {
+      name: String, required: false,
+      age: Number, required: false
+    })
+
+    var johndoe = new User({ name: 'John doe', age: 15, gender: 'm'});
+    ```
+
+    **Previously:**
+
+    silently removing unknown properties
+
+    ```js
+    User.isValid(); //true
+    ```
+
+    **New Behavior:**
+
+    returning `ValidationError` (same behavior as previous `strict: 'validate'`),
+    >ValidationError: The `User` instance is not valid. Details: `gender` is not defined in the model (value: 'm').
+
+See [related code change](https://github.com/strongloop/loopback-datasource-juggler/pull/1084) here.

lib/dao.js

@@ -93,10 +93,11 @@ function applyStrictCheck(model, strict, data, inst, cb) {
     key = keys[i];
     if (props[key]) {
       result[key] = data[key];
-    } else if (strict === 'throw') {
-      cb(new Error(g.f('Unknown property: %s', key)));
-      return;
-    } else if (strict === 'validate') {
+    } else if (strict) {
+      if (strict === 'throw') {
+        g.warn('{{`strict: throw`}} has been removed, use {{`strict: true`}} instead ' +
+          'which returns {{`Validation Error`}} for unknown properties');
+      }
       inst.__unknownProperties.push(key);
     }
   }

lib/model.js

@@ -141,7 +141,7 @@ ModelBaseClass.prototype._initProperties = function(data, options) {
       },
     });
 
-    if (strict === 'validate') {
+    if (strict) {
       Object.defineProperty(this, '__unknownProperties', {
         writable: true,
         enumerable: false,
@@ -155,7 +155,7 @@ ModelBaseClass.prototype._initProperties = function(data, options) {
     this.__dataSource = options.dataSource;
     this.__strict = strict;
     this.__persisted = false;
-    if (strict === 'validate') {
+    if (strict) {
       this.__unknownProperties = [];
     }
   }
@@ -243,9 +243,10 @@ ModelBaseClass.prototype._initProperties = function(data, options) {
             this.constructor.modelName, p
           ));
         }
-      } else if (strict === 'throw') {
-        throw new Error(g.f('Unknown property: %s', p));
-      } else if (strict === 'validate') {
+      } else {
+        if (strict === 'throw')
+          g.warn('{{`strict: throw`}} has been removed, use {{`strict: true`}} instead ' +
+          'which returns {{`Validation Error`}} for unknown properties');
         this.__unknownProperties.push(p);
       }
     }

lib/validations.js

@@ -433,7 +433,7 @@ Validatable.prototype.isValid = function(callback, data, options) {
   var valid = true, inst = this, wait = 0, async = false;
   var validations = this.constructor.validations;
 
-  var reportDiscardedProperties = this.__strict === 'validate' &&
+  var reportDiscardedProperties = this.__strict &&
     this.__unknownProperties && this.__unknownProperties.length;
 
   // exit with success when no errors

test/datatype.test.js

@@ -206,7 +206,7 @@ describe('datatypes', function() {
 
     it('should set missing optional properties to null', function(done) {
       var EXPECTED = {desc: null, stars: null};
-      TestModel.create({name: 'a-test-name'}, function(err, created) {
+      TestModel.create({}, function(err, created) {
         if (err) return done(err);
         created.should.have.properties(EXPECTED);
 
@@ -220,12 +220,12 @@ describe('datatypes', function() {
 
     it('should convert property value undefined to null', function(done) {
       var EXPECTED = {desc: null, extra: null};
+      var data = {desc: undefined, extra: undefined};
       if (isStrict) {
         // SQL-based connectors don't support dynamic properties
         delete EXPECTED.extra;
+        delete data.extra;
       }
-
-      var data = {desc: undefined, extra: undefined};
       TestModel.create(data, function(err, created) {
         if (err) return done(err);
 

test/loopback-dl.test.js

@@ -1917,47 +1917,8 @@ describe('ModelBuilder options.models', function() {
       },
     });
     assert.equal(m1.address.number, undefined, 'm1 should not contain number property in address');
-  });
-
-  it('should use strictEmbeddedModels setting (validate) when applied on modelBuilder', function() {
-    var builder = new ModelBuilder();
-    builder.settings.strictEmbeddedModels = 'validate';
-    var M1 = builder.define('testEmbedded', {
-      name: 'string',
-      address: {
-        street: 'string',
-      },
-    });
-    var m1 = new M1({
-      name: 'Jim',
-      address: {
-        street: 'washington st',
-        number: 5512,
-      },
-    });
-    assert.equal(m1.address.number, undefined, 'm1 should not contain number property in address');
     assert.equal(m1.address.isValid(), false, 'm1 address should not validate with extra property');
     var codes = m1.address.errors && m1.address.errors.codes || {};
     assert.deepEqual(codes.number, ['unknown-property']);
   });
-
-  it('should use the strictEmbeddedModels setting (throw) when applied on modelBuilder', function() {
-    var builder = new ModelBuilder();
-    builder.settings.strictEmbeddedModels = 'throw';
-    var M1 = builder.define('testEmbedded', {
-      name: 'string',
-      address: {
-        street: 'string',
-      },
-    });
-    assert.throws(function() {
-      var m1 = new M1({
-        name: 'Jim',
-        address: {
-          street: 'washington st',
-          number: 5512,
-        },
-      });
-    });
-  });
 });

test/manipulation.test.js

@@ -544,31 +544,72 @@ describe('manipulation', function() {
         });
     });
 
-    it('should ignore unknown attributes when strict: true', function(done) {
-      // Using {foo: 'bar'} only causes dependent test failures due to the
-      // stripping of object properties when in strict mode (ie. {foo: 'bar'}
-      // changes to '{}' and breaks other tests
-      person.updateAttributes({name: 'John', foo: 'bar'},
-        function(err, p) {
-          if (err) return done(err);
-          should.not.exist(p.foo);
-          Person.findById(p.id, function(e, p) {
-            if (e) return done(e);
-            should.not.exist(p.foo);
-            done();
-          });
+    it('should discard undefined values before strict validation',
+      function(done) {
+        Person.definition.settings.strict = true;
+        Person.findById(person.id, function(err, p) {
+          p.updateAttributes({name: 'John', unknownVar: undefined},
+            function(err, p) {
+              // if uknownVar was defined, it would return validationError
+              if (err) return done(err);
+              Person.findById(p.id, function(e, p) {
+                if (e) return done(e);
+                p.name.should.equal('John');
+                should.not.exist(p.unknownVar);
+                done();
+              });
+            });
         });
-    });
+      });
 
-    it('should throw error on unknown attributes when strict: throw', function(done) {
+    // `strict: true` used to silently remove unknown properties,
+    // now return validationError upon unknown properties,
+    // same as previous validate behavior
+    it('should allow unknown attributes when strict: false',
+      function(done) {
+        Person.definition.settings.strict = false;
+        Person.findById(person.id, function(err, p) {
+          p.updateAttributes({name: 'John', foo: 'bar'},
+            function(err, p) {
+              if (err) return done(err);
+              should.not.exist(err);
+              should.exist(p.foo);
+              done();
+            });
+        });
+      });
+
+    // `strict: true` used to silently remove unknown properties,
+    // now return validationError upon unknown properties
+    it('should return error on unknown attributes when strict: true',
+      function(done) {
+        Person.definition.settings.strict = true;
+        Person.findById(person.id, function(err, p) {
+          p.updateAttributes({name: 'John', foo: 'bar'},
+            function(err, p) {
+              should.exist(err);
+              err.name.should.equal('ValidationError');
+              err.message.should.containEql('`foo` is not defined in the model');
+              should.not.exist(p.foo);
+              Person.findById(p.id, function(e, p) {
+                if (e) return done(e);
+                should.not.exist(p.foo);
+                done();
+              });
+            });
+        });
+      });
+
+    // strict: throw is deprecated, use strict: true instead
+    // which returns Validation Error for unknown properties
+    it('should fallback to strict:true when using strict: throw', function(done) {
       Person.definition.settings.strict = 'throw';
       Person.findById(person.id, function(err, p) {
         p.updateAttributes({foo: 'bar'},
           function(err, p) {
             should.exist(err);
-            err.name.should.equal('Error');
-            err.message.should.equal('Unknown property: foo');
-            should.not.exist(p);
+            err.name.should.equal('ValidationError');
+            err.message.should.containEql('`foo` is not defined in the model');
             Person.findById(person.id, function(e, p) {
               if (e) return done(e);
               should.not.exist(p.foo);
@@ -578,7 +619,9 @@ describe('manipulation', function() {
       });
     });
 
-    it('should throw error on unknown attributes when strict: throw', function(done) {
+    // strict: validate is deprecated, use strict: true instead
+    // behavior remains the same as before, because validate is now default behavior
+    it('should fallback to strict:true when using strict:validate', function(done) {
       Person.definition.settings.strict = 'validate';
       Person.findById(person.id, function(err, p) {
         p.updateAttributes({foo: 'bar'},