Merge pull request #999 from strongloop/fix/order-in-find-2.x

Ensure stable order of items in DAO.find() [2.x]

lib/dao.js

@@ -1680,12 +1680,10 @@ DataAccessObject.find = function find(query, options, cb) {
   }
 
   var allCb = function(err, data) {
-    var results = [];
     if (!err && Array.isArray(data)) {
-      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 });
+      async.map(data, function(item, next) {
+        var Model = self.lookupModel(item);
+        var obj = new Model(item, { fields: query.fields, applySetters: false, persisted: true });
 
         if (query && query.include) {
           if (query.collect) {
@@ -1727,18 +1725,23 @@ DataAccessObject.find = function find(query, options, cb) {
           };
 
           Model.notifyObserversOf('loaded', context, function(err) {
-            if (err) return callback(err);
+            if (err) return next(err);
 
-            results.push(obj);
-            callback();
+            next(null, obj);
           });
         } else {
-          callback();
+          next();
         }
       },
-      function(err) {
+      function(err, results) {
         if (err) return cb(err);
 
+        // When applying query.collect, some root items may not have
+        // any related/linked item. We store `undefined` in the results
+        // array in such case, which is not desirable from API consumer's
+        // point of view.
+        results = results.filter(isDefined);
+
         if (data && data.countBeforeLimit) {
           results.countBeforeLimit = data.countBeforeLimit;
         }
@@ -1777,6 +1780,10 @@ DataAccessObject.find = function find(query, options, cb) {
   return cb.promise;
 };
 
+function isDefined(value) {
+  return value !== undefined;
+}
+
 /**
  * Find one record, same as `find`, but limited to one result. This function returns an object, not a collection.
  *

test/basic-querying.test.js

@@ -123,6 +123,14 @@ describe('basic-querying', function() {
   describe('find', function() {
     before(seed);
 
+    before(function setupDelayingLoadedHook() {
+      User.observe('loaded', nextAfterDelay);
+    });
+
+    after(function removeDelayingLoadHook() {
+      User.removeObserver('loaded', nextAfterDelay);
+    });
+
     it('should query collection', function(done) {
       User.find(function(err, users) {
         should.exists(users);
@@ -214,6 +222,18 @@ describe('basic-querying', function() {
       });
     });
 
+    it('should query sorted desc by order integer field even though there' +
+        'is an async model loaded hook', function(done) {
+      User.find({ order: 'order DESC' }, function(err, users) {
+        if (err) return done(err);
+
+        should.exists(users);
+        var order = users.map(function(u) { return u.order; });
+        order.should.eql([6, 5, 4, 3, 2, 1]);
+        done();
+      });
+    });
+
     it('should support "and" operator that is satisfied', function(done) {
       User.find({ where: { and: [
         { name: 'John Lennon' },
@@ -813,3 +833,8 @@ function seed(done) {
     },
   ], done);
 }
+
+function nextAfterDelay(ctx, next) {
+  var randomTimeoutTrigger = Math.floor(Math.random() * 100);
+  setTimeout(function() { process.nextTick(next); }, randomTimeoutTrigger);
+}