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 fixing DAO operations like `find` and
`findOrCreate` so that they do NOT apply default property values on
data returned by the database (connector).

Please note that most of the other CRUD methods were already not
applying default values on database data as long as the connector
provided native implementation of the operation.
This commit is contained in:
Miroslav Bajtoš 2019-04-01 13:18:05 +02:00
parent ff5a3fa8bf
commit e45292de08
No known key found for this signature in database
GPG Key ID: 6F2304BA9361C7E3
4 changed files with 111 additions and 8 deletions

View File

@ -1109,8 +1109,14 @@ 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,
// see https://github.com/strongloop/loopback-datasource-juggler/issues/1692
applyDefaultValues: false,
};
obj = new Model(data, ctorOpts);
}
if (created) {
@ -1631,6 +1637,8 @@ DataAccessObject.find = function find(query, options, cb) {
fields: query.fields,
applySetters: false,
persisted: true,
// see https://github.com/strongloop/loopback-datasource-juggler/issues/1692
applyDefaultValues: false,
};
let obj;
try {

View File

@ -922,6 +922,30 @@ describe('basic-querying', function() {
});
});
});
it('preserves empty values from the database', async () => {
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
// Initially, all Products were always active, no property was needed
const Product = db.define('Product', {name: String});
await db.automigrate('Product');
const created = await Product.create({name: 'Pen'});
// Later on, we decide to introduce `active` property
Product.defineProperty('active', {
type: Boolean,
default: false,
});
// And query existing data
const found = await Product.findOne();
found.toObject().should.eql({
id: created.id,
name: 'Pen',
active: undefined,
});
});
});
describe('count', function() {

View File

@ -34,13 +34,13 @@ describe('defaults', function() {
});
});
it('should apply defaults on read', function(done) {
it('should NOT apply defaults on read', function(done) {
db.defineProperty('Server', 'host', {
type: String,
default: 'localhost',
});
Server.all(function(err, servers) {
(new String('localhost')).should.equal(servers[0].host);
should(servers[0].host).be.undefined();
done();
});
});
@ -49,10 +49,9 @@ describe('defaults', function() {
Server.create({host: 'localhost', port: 8080}, function(err, s) {
should.not.exist(err);
s.port.should.equal(8080);
Server.find({fields: ['host']}, function(err, servers) {
servers[0].host.should.equal('localhost');
servers[0].should.have.property('host');
servers[0].should.have.property('port', undefined);
Server.findById(s.id, {fields: ['host']}, function(err, server) {
server.should.have.property('host', 'localhost');
server.should.have.property('port', undefined);
done();
});
});

View File

@ -981,6 +981,30 @@ describe('manipulation', function() {
}
);
});
it('preserves empty values from the database', async () => {
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
// Initially, all Products were always active, no property was needed
const Product = db.define('Product', {name: String});
await db.automigrate('Product');
const created = await Product.create({name: 'Pen'});
// Later on, we decide to introduce `active` property
Product.defineProperty('active', {
type: Boolean,
default: false,
});
// And updateOrCreate an existing record
const found = await Product.updateOrCreate({id: created.id, name: 'updated'});
found.toObject().should.eql({
id: created.id,
name: 'updated',
active: undefined,
});
});
});
bdd.describeIf(connectorCapabilities.supportForceId !== false,
@ -1619,6 +1643,30 @@ describe('manipulation', function() {
})
.catch(done);
});
it('preserves empty values from the database', async () => {
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
// Initially, all Products were always active, no property was needed
const Product = db.define('Product', {name: String});
await db.automigrate('Product');
const created = await Product.create({name: 'Pen'});
// Later on, we decide to introduce `active` property
Product.defineProperty('active', {
type: Boolean,
default: false,
});
// And findOrCreate an existing record
const [found] = await Product.findOrCreate({id: created.id}, {name: 'updated'});
found.toObject().should.eql({
id: created.id,
name: 'Pen',
active: undefined,
});
});
});
describe('destroy', function() {
@ -2563,6 +2611,30 @@ describe('manipulation', function() {
});
});
});
it('preserves empty values from the database', async () => {
// https://github.com/strongloop/loopback-datasource-juggler/issues/1692
// Initially, all Products were always active, no property was needed
const Product = db.define('Product', {name: String});
await db.automigrate('Product');
const created = await Product.create({name: 'Pen'});
// Later on, we decide to introduce `active` property
Product.defineProperty('active', {
type: Boolean,
default: false,
});
// And upsertWithWhere an existing record
const found = await Product.upsertWithWhere({id: created.id}, {name: 'updated'});
found.toObject().should.eql({
id: created.id,
name: 'updated',
active: undefined,
});
});
});
});