Do not apply default values on data from database
Before this change, when a property was configured with a default value at LoopBack side and the database was returned a record with a missing value for such property, then we would supply use the configured default. This behavior is problematic for reasons explained in #1692. In this commit, we are introducing a new model-level setting called `applyDefaultsOnReads`, which is enabled by default for backwards compatibility. When this setting is set to `false`, operations like `find` and `findOrCreate` will NOT apply default property values on data returned by the database (connector). Please note that most of the other CRUD methods did not apply default values on database data as long as the connector provided native implementation of the operation, that aspect is not changing. Also note that default values are applied only on properties with `undefined` values. The value `null` does not trigger application of default values. This is important because SQL connectors return `null` for properties with no value set.
This commit is contained in:
parent
905fa20c36
commit
a647bb0736
21
lib/dao.js
21
lib/dao.js
|
@ -1065,8 +1065,15 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb)
|
|||
var obj, Model = self.lookupModel(data);
|
||||
|
||||
if (data) {
|
||||
obj = new Model(data, {fields: query.fields, applySetters: false,
|
||||
persisted: true});
|
||||
var ctorOpts = {
|
||||
fields: query.fields,
|
||||
applySetters: false,
|
||||
persisted: true,
|
||||
};
|
||||
if (Model.settings.applyDefaultsOnReads === false) {
|
||||
ctorOpts.applyDefaultValues = false;
|
||||
}
|
||||
obj = new Model(data, ctorOpts);
|
||||
}
|
||||
|
||||
if (created) {
|
||||
|
@ -1936,7 +1943,15 @@ DataAccessObject.find = function find(query, options, cb) {
|
|||
if (!err && Array.isArray(data)) {
|
||||
async.map(data, function(item, next) {
|
||||
var Model = self.lookupModel(item);
|
||||
var obj = new Model(item, {fields: query.fields, applySetters: false, persisted: true});
|
||||
var ctorOpts = {
|
||||
fields: query.fields,
|
||||
applySetters: false,
|
||||
persisted: true,
|
||||
};
|
||||
if (Model.settings.applyDefaultsOnReads === false) {
|
||||
ctorOpts.applyDefaultValues = false;
|
||||
}
|
||||
var obj = new Model(item, ctorOpts);
|
||||
|
||||
if (query && query.include) {
|
||||
if (query.collect) {
|
||||
|
|
|
@ -579,6 +579,75 @@ describe('basic-querying', function() {
|
|||
sample(['id']).expect(['id']);
|
||||
sample(['email']).expect(['email']);
|
||||
});
|
||||
|
||||
it('applies default values by default', function() {
|
||||
// Backwards compatibility, see
|
||||
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
|
||||
|
||||
// Initially, all Players were always active, no property was needed
|
||||
var Player = db.define('Player', {name: String});
|
||||
var created;
|
||||
return db.automigrate('Player')
|
||||
.then(function() { return Player.create({name: 'Pen'}); })
|
||||
.then(function(result) {
|
||||
created = result;
|
||||
|
||||
// Later on, we decide to introduce `active` property
|
||||
Player.defineProperty('active', {
|
||||
type: Boolean,
|
||||
default: false,
|
||||
});
|
||||
return db.autoupdate('Player');
|
||||
})
|
||||
.then(function() {
|
||||
// And query existing data
|
||||
return Player.findOne();
|
||||
})
|
||||
.then(function(found) {
|
||||
should(found.toObject().active).be.oneOf([
|
||||
// For databases supporting `undefined` value,
|
||||
// we convert `undefined` to property default.
|
||||
false,
|
||||
// For databases representing `undefined` as `null` (e.g. SQL),
|
||||
// we treat `null` as a defined value and don't apply defaults.
|
||||
null,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
it('preserves empty values from the database when "applyDefaultsOnReads" is false', function() {
|
||||
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
|
||||
|
||||
// Initially, all Players were always active, no property was needed
|
||||
var Player = db.define(
|
||||
'Player',
|
||||
{name: String},
|
||||
{applyDefaultsOnReads: false}
|
||||
);
|
||||
var created;
|
||||
return db.automigrate('Player')
|
||||
.then(function() { return Player.create({name: 'Pen'}); })
|
||||
.then(function(result) {
|
||||
created = result;
|
||||
|
||||
// Later on, we decide to introduce `active` property
|
||||
Player.defineProperty('active', {
|
||||
type: Boolean,
|
||||
default: false,
|
||||
});
|
||||
return db.autoupdate('Player');
|
||||
})
|
||||
.then(function() {
|
||||
// And query existing data
|
||||
return Player.findOne();
|
||||
})
|
||||
.then(function(found) {
|
||||
should(found.toObject().active).be.oneOf([
|
||||
undefined, // databases supporting `undefined` value
|
||||
null, // databases representing `undefined` as `null`
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('count', function() {
|
||||
|
|
|
@ -1240,6 +1240,82 @@ describe('manipulation', function() {
|
|||
})
|
||||
.catch(done);
|
||||
});
|
||||
|
||||
it('applies default values on returned data', function() {
|
||||
// Backwards compatibility, see
|
||||
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
|
||||
|
||||
// Initially, all Players were always active, no property was needed
|
||||
var Player = db.define('Player', {name: String});
|
||||
var created;
|
||||
return db.automigrate('Player')
|
||||
.then(function() { return Player.create({name: 'Pen'}); })
|
||||
.then(function(result) {
|
||||
created = result;
|
||||
|
||||
// Later on, we decide to introduce `active` property
|
||||
Player.defineProperty('active', {
|
||||
type: Boolean,
|
||||
default: false,
|
||||
});
|
||||
return db.autoupdate('Player');
|
||||
})
|
||||
.then(function() {
|
||||
// and findOrCreate an existing record
|
||||
return Player.findOrCreate({id: created.id}, {name: 'updated'});
|
||||
})
|
||||
.then(function(result) {
|
||||
var found = result[0];
|
||||
|
||||
// Backwards-compatibility
|
||||
// When Pen does not have "active" flag set, we change it to default
|
||||
should(found.toObject().active).be.oneOf([
|
||||
// For databases supporting `undefined` value,
|
||||
// we convert `undefined` to property default.
|
||||
false,
|
||||
// For databases representing `undefined` as `null` (e.g. SQL),
|
||||
// we treat `null` as a defined value and don't apply defaults.
|
||||
null,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
it('preserves empty values from the database when "applyDefaultsOnReads" is false', function() {
|
||||
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
|
||||
|
||||
// Initially, all Players were always active, no property was needed
|
||||
var Player = db.define(
|
||||
'Player',
|
||||
{name: String},
|
||||
{applyDefaultsOnReads: false}
|
||||
);
|
||||
var created;
|
||||
|
||||
return db.automigrate('Player')
|
||||
.then(function() { return Player.create({name: 'Pen'}); })
|
||||
.then(function(result) {
|
||||
created = result;
|
||||
|
||||
// Later on, we decide to introduce `active` property
|
||||
Player.defineProperty('active', {
|
||||
type: Boolean,
|
||||
default: false,
|
||||
});
|
||||
return db.autoupdate('Player');
|
||||
})
|
||||
.then(function() {
|
||||
// And findOrCreate an existing record
|
||||
return Player.findOrCreate({id: created.id}, {name: 'updated'});
|
||||
})
|
||||
.then(function(result) {
|
||||
var found = result[0];
|
||||
|
||||
should(found.toObject().active).be.oneOf([
|
||||
undefined, // databases supporting `undefined` value
|
||||
null, // databases representing `undefined` as `null`
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('destroy', function() {
|
||||
|
|
Loading…
Reference in New Issue