diff --git a/lib/dao.js b/lib/dao.js index da08009f..9cc3e530 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -293,6 +293,7 @@ DataAccessObject.create = function (data, options, cb) { options: options }; Model.notifyObserversOf('loaded', context, function(err) { + if (err) return cb(err); // By default, the instance passed to create callback is NOT updated // with the changes made through persist/loaded hooks. To preserve @@ -500,6 +501,8 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data options: options }; Model.notifyObserversOf('loaded', context, function(err) { + if (err) return cb(err); + var obj; if (data && !(data instanceof Model)) { inst._initProperties(data, { persisted: true }); @@ -625,6 +628,8 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb) options: options }; Model.notifyObserversOf('loaded', context, function(err) { + if (err) return cb(err); + var obj, Model = self.lookupModel(data); if (data) { @@ -1378,66 +1383,70 @@ DataAccessObject.find = function find(query, options, cb) { var allCb = function(err, data) { var results = []; if (Array.isArray(data)) { - for (var i = 0, n = data.length; i < n; i++) { - var d = data[i]; + async.each(data, function(item, callback) { + var d = item;//data[i]; var Model = self.lookupModel(d); var obj = new Model(d, {fields: query.fields, applySetters: false, persisted: true}); - context = { - Model: Model, - instance: obj, - isNewInstance: false, - hookState: hookState, - options: options - }; - if (query && query.include) { - Model.notifyObserversOf('loaded', context, function(err) { - if (query.collect) { - // The collect property indicates that the query is to return the - // standalone items for a related model, not as child of the parent object - // For example, article.tags - obj = obj.__cachedRelations[query.collect]; - if (obj === null) { - obj = undefined; - } - } else { - // This handles the case to return parent items including the related - // models. For example, Article.find({include: 'tags'}, ...); - // Try to normalize the include - var includes = Inclusion.normalizeInclude(query.include || []); - includes.forEach(function(inc) { - var relationName = inc; - if (utils.isPlainObject(inc)) { - relationName = Object.keys(inc)[0]; - } - - // Promote the included model as a direct property - var included = obj.__cachedRelations[relationName]; - if (Array.isArray(included)) { - included = new List(included, null, obj); - } - if (included) obj.__data[relationName] = included; - }); - delete obj.__data.__cachedRelations; + if (query.collect) { + // The collect property indicates that the query is to return the + // standalone items for a related model, not as child of the parent object + // For example, article.tags + obj = obj.__cachedRelations[query.collect]; + if (obj === null) { + obj = undefined; } - }); + } else { + // This handles the case to return parent items including the related + // models. For example, Article.find({include: 'tags'}, ...); + // Try to normalize the include + var includes = Inclusion.normalizeInclude(query.include || []); + includes.forEach(function(inc) { + var relationName = inc; + if (utils.isPlainObject(inc)) { + relationName = Object.keys(inc)[0]; + } + + // Promote the included model as a direct property + var included = obj.__cachedRelations[relationName]; + if (Array.isArray(included)) { + included = new List(included, null, obj); + } + if (included) obj.__data[relationName] = included; + }); + delete obj.__data.__cachedRelations; + } } if (obj !== undefined) { + context = { + Model: Model, + instance: obj, + isNewInstance: false, + hookState: hookState, + options: options + }; + Model.notifyObserversOf('loaded', context, function(err) { + if (err) return callback(err); + results.push(obj); + callback(); }); } - } + }, + function(err) { + if (err) return cb(err); - if (data && data.countBeforeLimit) { - results.countBeforeLimit = data.countBeforeLimit; - } - if (!supportsGeo && near) { - results = geo.filter(results, near); - } + if (data && data.countBeforeLimit) { + results.countBeforeLimit = data.countBeforeLimit; + } + if (!supportsGeo && near) { + results = geo.filter(results, near); + } - cb(err, results); + cb(err, results); + }); } else cb(err, []); @@ -1876,6 +1885,8 @@ DataAccessObject.prototype.save = function (options, cb) { options: options }; Model.notifyObserversOf('loaded', context, function(err) { + if (err) return cb(err); + inst._initProperties(data, { persisted: true }); var context = { @@ -2398,7 +2409,9 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, op options: options }; Model.notifyObserversOf('loaded', ctx, function(err) { - if (!err) inst.__persisted = true; + if (err) return cb(err); + + inst.__persisted = true; // By default, the instance passed to updateAttributes callback is NOT updated // with the changes made through persist/loaded hooks. To preserve diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js index 6bc4e494..a2fb0d12 100644 --- a/test/persistence-hooks.suite.js +++ b/test/persistence-hooks.suite.js @@ -52,6 +52,22 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.find', function() { + it('triggers hooks in the correct order', function(done) { + monitorHookExecution(); + + TestModel.find( + { where: { id: '1' } }, + function(err, list) { + if (err) return done(err); + + triggered.should.eql([ + 'access', + 'loaded' + ]); + done(); + }); + }); + it('triggers `access` hook', function(done) { TestModel.observe('access', pushContextAndNext()); @@ -110,6 +126,40 @@ module.exports = function(dataSource, should) { done(); }); }); + + it('applies updates from `loaded` hook', function(done) { + TestModel.observe('loaded', pushContextAndNext(function(ctx) { + ctx.instance.extra = 'hook data'; + })); + + TestModel.find( + {where: {id: 1}}, + function (err, list) { + if (err) return done(err); + + observedContexts.should.eql(aTestModelCtx({ + instance: { + id: "1", + name: "first", + extra: "hook data" + }, + isNewInstance: false, + hookState: { test: true }, + options: {} + })); + done(); + }); + }) + + it('emits error when `loaded` hook fails', function(done) { + TestModel.observe('loaded', nextWithError(expectedError)); + TestModel.find( + {where: {id: 1}}, + function(err, list) { + [err].should.eql([expectedError]); + done(); + }); + }); }); describe('PersistedModel.create', function() { @@ -279,6 +329,16 @@ module.exports = function(dataSource, should) { }); }); + it('emits error when `loaded` hook fails', function(done) { + TestModel.observe('loaded', nextWithError(expectedError)); + TestModel.create( + { id: 'new-id', name: 'a name' }, + function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + it('applies updates from `loaded` hook', function(done) { TestModel.observe('loaded', pushContextAndNext(function(ctx){ ctx.data.extra = 'hook data'; @@ -734,6 +794,17 @@ module.exports = function(dataSource, should) { }); }); + it('emits error when `loaded` hook fails', function(done) { + TestModel.observe('loaded', nextWithError(expectedError)); + TestModel.findOrCreate( + { where: { name: 'new-record' } }, + { name: 'new-record' }, + function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + if (dataSource.connector.findOrCreate) { it('applies updates from `loaded` hook when found', function(done) { TestModel.observe('loaded', pushContextAndNext(function(ctx){ @@ -963,6 +1034,15 @@ module.exports = function(dataSource, should) { }); }); + it('emits error when `loaded` hook fails', function(done) { + TestModel.observe('loaded', nextWithError(expectedError)); + existingInstance.save( + function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + it('applies updates from `loaded` hook', function(done) { TestModel.observe('loaded', pushContextAndNext(function(ctx){ ctx.data.extra = 'hook data'; @@ -1219,6 +1299,16 @@ module.exports = function(dataSource, should) { }); }); + it('emits error when `loaded` hook fails', function(done) { + TestModel.observe('loaded', nextWithError(expectedError)); + existingInstance.updateAttributes( + { name: 'changed' }, + function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + it('applies updates from `loaded` hook updateAttributes', function(done) { TestModel.observe('loaded', pushContextAndNext(function(ctx){ ctx.data.extra = 'hook data'; @@ -1673,6 +1763,16 @@ module.exports = function(dataSource, should) { }); }); + it('emits error when `loaded` hook fails', function(done) { + TestModel.observe('loaded', nextWithError(expectedError)); + TestModel.updateOrCreate( + { id: 'new-id', name: 'a name' }, + function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + it('triggers `after save` hook on update', function(done) { TestModel.observe('after save', pushContextAndNext());