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:
Miroslav Bajtoš 2019-04-01 13:18:05 +02:00
parent 2f093ce254
commit fd03302f08
No known key found for this signature in database
GPG Key ID: 6F2304BA9361C7E3
3 changed files with 153 additions and 2 deletions

View File

@ -1109,8 +1109,15 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb)
let obj;
if (data) {
obj = new Model(data, {fields: query.fields, applySetters: false,
persisted: true});
const ctorOpts = {
fields: query.fields,
applySetters: false,
persisted: true,
};
if (Model.settings.applyDefaultsOnReads === false) {
ctorOpts.applyDefaultValues = false;
}
obj = new Model(data, ctorOpts);
}
if (created) {
@ -1632,6 +1639,9 @@ DataAccessObject.find = function find(query, options, cb) {
applySetters: false,
persisted: true,
};
if (Model.settings.applyDefaultsOnReads === false) {
ctorOpts.applyDefaultValues = false;
}
let obj;
try {
obj = new Model(data, ctorOpts);

View File

@ -917,6 +917,75 @@ describe('basic-querying', function() {
});
});
});
it('applies default values by default', () => {
// Backwards compatibility, see
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
// Initially, all Players were always active, no property was needed
const Player = db.define('Player', {name: String});
let created;
return db.automigrate('Player')
.then(() => Player.create({name: 'Pen'}))
.then(result => {
created = result;
// Later on, we decide to introduce `active` property
Player.defineProperty('active', {
type: Boolean,
default: false,
});
return db.autoupdate('Player');
})
.then(() => {
// And query existing data
return Player.findOne();
})
.then(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', () => {
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
// Initially, all Players were always active, no property was needed
const Player = db.define(
'Player',
{name: String},
{applyDefaultsOnReads: false}
);
let created;
return db.automigrate('Player')
.then(() => Player.create({name: 'Pen'}))
.then(result => {
created = result;
// Later on, we decide to introduce `active` property
Player.defineProperty('active', {
type: Boolean,
default: false,
});
return db.autoupdate('Player');
})
.then(() => {
// And query existing data
return Player.findOne();
})
.then(found => {
should(found.toObject().active).be.oneOf([
undefined, // databases supporting `undefined` value
null, // databases representing `undefined` as `null`
]);
});
});
});
describe('count', function() {

View File

@ -1619,6 +1619,78 @@ describe('manipulation', function() {
})
.catch(done);
});
it('applies default values on returned data', () => {
// Backwards compatibility, see
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
// Initially, all Players were always active, no property was needed
const Player = db.define('Player', {name: String});
let created;
return db.automigrate('Player')
.then(() => Player.create({name: 'Pen'}))
.then(result => {
created = result;
// Later on, we decide to introduce `active` property
Player.defineProperty('active', {
type: Boolean,
default: false,
});
return db.autoupdate('Player');
})
.then(() => {
// and findOrCreate an existing record
return Player.findOrCreate({id: created.id}, {name: 'updated'});
})
.spread(found => {
// 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', () => {
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
// Initially, all Players were always active, no property was needed
const Player = db.define(
'Player',
{name: String},
{applyDefaultsOnReads: false}
);
let created;
return db.automigrate('Player')
.then(() => Player.create({name: 'Pen'}))
.then(result => {
created = result;
// Later on, we decide to introduce `active` property
Player.defineProperty('active', {
type: Boolean,
default: false,
});
return db.autoupdate('Player');
})
.then(() => {
// And findOrCreate an existing record
return Player.findOrCreate({id: created.id}, {name: 'updated'});
})
.spread(found => {
should(found.toObject().active).be.oneOf([
undefined, // databases supporting `undefined` value
null, // databases representing `undefined` as `null`
]);
});
});
});
describe('destroy', function() {