DAO.create: don't return the instance

Simplify DataAccessObject.create() and stop returning the
instance/array of instances. Users should always use callback (or
returned promise) to get the instance(s) created.
This commit is contained in:
Miroslav Bajtoš 2016-04-29 09:08:15 +02:00
parent 89bf94245b
commit 8ad53a4c0e
3 changed files with 24 additions and 13 deletions

View File

@ -34,3 +34,18 @@ checks `if (ctx.instance)` then you can remove this condition together with
the branch that follows. the branch that follows.
See also the commit [30283291](https://github.com/strongloop/loopback-datasource-juggler/commit/30283291?w=1) See also the commit [30283291](https://github.com/strongloop/loopback-datasource-juggler/commit/30283291?w=1)
## DAO.create no longer returns the instance(s) created
While implementing support for promises in `DAO.create`, we found that this
method returns the instance object synchronously. This is inconsistent with
the usual convention of accessing the result via callback/promise and it may
not work correctly in cases where some fields like the `id` property are
generated by the database.
We have changed the API to be consistent with other DAO methods: when invoked
with a callback argument, the method does not return anything. When invoked
without any callback, a promise is returned.
See [pull request 918](https://github.com/strongloop/loopback-datasource-juggler/pull/918)
for more details.

View File

@ -271,7 +271,7 @@ DataAccessObject.create = function(data, options, cb) {
} }
cb(errors, data); cb(errors, data);
}); });
return data; return;
} }
var enforced = {}; var enforced = {};
@ -402,10 +402,7 @@ DataAccessObject.create = function(data, options, cb) {
}, obj, cb); }, obj, cb);
} }
// Does this make any sense? How would chaining be used here? -partap return cb.promise;
// for chaining
return cb.promise || obj;
}; };
function stillConnecting(dataSource, obj, args) { function stillConnecting(dataSource, obj, args) {

View File

@ -115,14 +115,12 @@ describe('manipulation', function() {
}); });
it('should return instance of object', function(done) { it('should not return instance of object', function(done) {
var person = Person.create(function(err, p) { var person = Person.create(function(err, p) {
p.id.should.eql(person.id); should.exist(p.id);
if (person) person.should.not.be.an.instanceOf(Person);
done(); done();
}); });
should.exist(person);
person.should.be.an.instanceOf(Person);
should.not.exist(person.id);
}); });
it('should not allow user-defined value for the id of object - create', function(done) { it('should not allow user-defined value for the id of object - create', function(done) {
@ -235,7 +233,8 @@ describe('manipulation', function() {
{ name: 'Boltay' }, { name: 'Boltay' },
{}, {},
]; ];
Person.create(batch, function(e, ps) { var res = Person.create(batch, function(e, ps) {
if (res) res.should.not.be.instanceOf(Array);
should.not.exist(e); should.not.exist(e);
should.exist(ps); should.exist(ps);
ps.should.be.instanceOf(Array); ps.should.be.instanceOf(Array);
@ -254,8 +253,8 @@ describe('manipulation', function() {
persons.should.have.lengthOf(batch.length); persons.should.have.lengthOf(batch.length);
persons[0].errors.should.be.false; persons[0].errors.should.be.false;
done(); done();
}).should.be.instanceOf(Array); });
}).should.have.lengthOf(3); });
}); });
it('should create batch of objects with beforeCreate', function(done) { it('should create batch of objects with beforeCreate', function(done) {