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

Fix regression in `.save()` from 1fd6eff1 

The commit 1fd6eff1 (intent-based hooks) introduced a subtle regression
in `.save()` method where dynamic property setters were invoked twice.

This commit fixes the problem by moving pre-save data normalization
into `before save` callback.
This commit is contained in:
Miroslav Bajtoš 2015-01-30 11:01:48 +01:00
parent ce39f8ab01
commit 5cfbfe3a19
2 changed files with 73 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
lib/dao.js
View File

@ -1132,21 +1132,19 @@ DataAccessObject.prototype.save = function (options, callback) {
options.throws = false;
}
var inst = this;
var data = inst.toObject(true);
var modelName = Model.modelName;
Model.applyProperties(data, this);
if (this.isNewRecord()) {
return Model.create(this, callback);
} else {
inst.setAttributes(data);
}
var inst = this;
var modelName = Model.modelName;
Model.notifyObserversOf('before save', { Model: Model, instance: inst }, function(err) {
if (err) return callback(err);
data = inst.toObject(true);
var data = inst.toObject(true);
Model.applyProperties(data, inst);
inst.setAttributes(data);
// validate first
if (!options.validate) {

63
test/manipulation.test.js
View File

@ -22,6 +22,22 @@ describe('manipulation', function () {
});
// A simplified implementation of LoopBack's User model
// to reproduce problems related to properties with dynamic setters
// For the purpose of the tests, we use a counter instead of a hash fn.
var StubUser, stubPasswordCounter;
before(function setupStubUserModel(done) {
StubUser = db.createModel('StubUser', { password: String }, { forceId: true });
StubUser.setter.password = function(plain) {
this.$password = plain + '-' + (++stubPasswordCounter);
};
db.automigrate('StubUser', done);
});
beforeEach(function resetStubPasswordCounter() {
stubPasswordCounter = 0;
});
describe('create', function () {
before(function (done) {
@ -214,6 +230,23 @@ describe('manipulation', function () {
});
});
it('should preserve properties with dynamic setters', function(done) {
// This test reproduces a problem discovered by LoopBack unit-test
// "User.hasPassword() should match a password after it is changed"
StubUser.create({ password: 'foo' }, function(err, created) {
if (err) return done(err);
created.password = 'bar';
created.save(function(err, saved) {
if (err) return done(err);
saved.password.should.equal('bar-2');
StubUser.findById(created.id, function(err, found) {
if (err) return done(err);
found.password.should.equal('bar-2');
done();
});
});
});
});
});
describe('updateAttributes', function () {
@ -238,6 +271,36 @@ describe('manipulation', function () {
});
});
describe('updateOrCreate', function() {
it('should preserve properties with dynamic setters on create', function(done) {
StubUser.updateOrCreate({ id: 'newid', password: 'foo' }, function(err, created) {
if (err) return done(err);
created.password.should.equal('foo-1');
StubUser.findById(created.id, function(err, found) {
if (err) return done(err);
found.password.should.equal('foo-1');
done();
});
});
});
it('should preserve properties with dynamic setters on update', function(done) {
StubUser.create({ password: 'foo' }, function(err, created) {
if (err) return done(err);
var data = { id: created.id, password: 'bar' };
StubUser.updateOrCreate(data, function(err, updated) {
if (err) return done(err);
updated.password.should.equal('bar-2');
StubUser.findById(created.id, function(err, found) {
if (err) return done(err);
found.password.should.equal('bar-2');
done();
});
});
});
});
});
describe('destroy', function () {
it('should destroy record', function (done) {