Merge branch 'PradnyaBaviskar-issue656-findLoadedAsync'

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++) {
+      async.each(data, function(item, callback) {
-        var d = data[i];
+        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) {
-            if (query.collect) {
+            // The collect property indicates that the query is to return the
-              // The collect property indicates that the query is to return the
+            // standalone items for a related model, not as child of the parent object
-              // standalone items for a related model, not as child of the parent object
+            // For example, article.tags
-              // For example, article.tags
+            obj = obj.__cachedRelations[query.collect];
-              obj = obj.__cachedRelations[query.collect];
+            if (obj === null) {
-              if (obj === null) {
+              obj = undefined;
-                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;
             }
-          });
+          } 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) {
+        if (data && data.countBeforeLimit) {
-        results.countBeforeLimit = data.countBeforeLimit;
+          results.countBeforeLimit = data.countBeforeLimit;
-      }
+        }
-      if (!supportsGeo && near) {
+        if (!supportsGeo && near) {
-        results = geo.filter(results, 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

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());