verdnatura-mirror
/
loopback-datasource-juggler
mirror of https://github.com/loopbackio/loopback-datasource-juggler
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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)
This commit is contained in:
David Cheung 2016-09-07 14:24:48 -04:00
parent 5bf18b0728
commit 805db78e19
7 changed files with 127 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
3.0-RELEASE-NOTES.md
View File

@ -102,3 +102,55 @@ 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
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'});
```
- #### strict: false
behavior will remain the same and unknown properties will be saved along with other properties
```js
//johndoe would be created as
{
id: 1,
name: 'John doe',
age: 15,
geneder: 'm'
}
```
- #### strict: `'validate'` and strict: `'throw'`
are removed in favor of `strict: true`, and because non-empty string
resolves as true, they will share the same behavior as `strict: true`
- #### strict: `true`'s behavior change:
**Previously:** (2.x)
`strict:true` would silently removing unknown properties, and User instance will be created with unknown properties discarded
```js
johndoe.isValid(); //true
//johndoe would be created as
{
id: 1,
name: 'John doe',
age: 15
}
```
**New Behavior:** (3.x)
```js
johndoe.isValid(); //false
//johndoe would NOT be created and will result in the following error
//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.

5
lib/dao.js
View File

@ -93,10 +93,7 @@ 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) {
inst.__unknownProperties.push(key);
}
}

12
lib/model.js
View File

@ -94,6 +94,10 @@ ModelBaseClass.prototype._initProperties = function(data, options) {
if (strict === undefined) {
strict = ctor.definition.settings.strict;
} else if (strict === 'throw') {
g.warn('Warning: Model %s, {{strict mode: `throw`}} has been removed, ' +
'please use {{`strict: true`}} instead, which returns' +
'{{`Validation Error`}} for unknown properties,', ctor.modelName);
}
var persistUndefinedAsNull = ctor.definition.settings.persistUndefinedAsNull;
@ -141,7 +145,7 @@ ModelBaseClass.prototype._initProperties = function(data, options) {
},
});
if (strict === 'validate') {
if (strict) {
Object.defineProperty(this, '__unknownProperties', {
writable: true,
enumerable: false,
@ -155,7 +159,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 +247,7 @@ 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 {
this.__unknownProperties.push(p);
}
}

2
lib/validations.js
View File

@ -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

5
test/datatype.test.js
View File

@ -192,6 +192,7 @@ describe('datatypes', function() {
TestModel = db.define(
'TestModel',
{
name: {type: String, required: false},
desc: {type: String, required: false},
stars: {type: Number, required: false},
},
@ -220,12 +221,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);

39
test/loopback-dl.test.js
View File

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

70
test/manipulation.test.js
View File

@ -544,41 +544,83 @@ 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'},
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);
should.not.exist(p.foo);
Person.findById(p.id, function(e, p) {
if (e) return done(e);
should.not.exist(p.foo);
p.name.should.equal('John');
p.should.not.have.property('unknownVar');
done();
});
});
});
});
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);
p.should.have.property('foo');
done();
});
});
});
it('should throw error on unknown attributes when strict: throw', function(done) {
// Prior to version 3.0 `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) {
// 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.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');
p.should.not.have.property('foo');
Person.findById(p.id, function(e, p) {
if (e) return done(e);
p.should.not.have.property('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);
p.should.not.have.property('foo');
done();
});
});
});
});
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'},
@ -588,7 +630,7 @@ describe('manipulation', function() {
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);
p.should.not.have.property('foo');
done();
});
});