Simplify the inclusion processing

examples/relations.js

@@ -2,7 +2,6 @@ var DataSource = require('../index').DataSource;
 var ds = new DataSource('memory');
 
 var Order = ds.createModel('Order', {
-  customerId: Number,
   items: [String],
   orderDate: Date
 });
@@ -13,8 +12,11 @@ var Customer = ds.createModel('Customer', {
 
 Order.belongsTo(Customer);
 
+var order1, order2, order3;
+
 Customer.create({name: 'John'}, function (err, customer) {
   Order.create({customerId: customer.id, orderDate: new Date(), items: ['Book']}, function (err, order) {
+    order1 = order;
     order.customer(console.log);
     order.customer(true, console.log);
 
@@ -24,13 +26,16 @@ Customer.create({name: 'John'}, function (err, customer) {
     });
   });
 
-  Order.create({orderDate: new Date(), items: ['Phone']}, function (err, order2) {
+  Order.create({orderDate: new Date(), items: ['Phone']}, function (err, order) {
 
-    order2.customer.create({name: 'Smith'}, function(err, customer2) {
+    order.customer.create({name: 'Smith'}, function(err, customer2) {
-      console.log(order2, customer2);
+      console.log(order, customer2);
+      order.save(function(err, order) {
+        order2 = order;
+      });
     });
 
-    var customer3 = order2.customer.build({name: 'Tom'});
+    var customer3 = order.customer.build({name: 'Tom'});
     console.log('Customer 3', customer3);
   });
 });
@@ -39,14 +44,15 @@ Customer.hasMany(Order, {as: 'orders', foreignKey: 'customerId'});
 
 Customer.create({name: 'Ray'}, function (err, customer) {
   Order.create({customerId: customer.id, orderDate: new Date()}, function (err, order) {
+    order3 = order;
     customer.orders(console.log);
     customer.orders.create({orderDate: new Date()}, function (err, order) {
       console.log(order);
       Customer.include([customer], 'orders', function (err, results) {
         console.log('Results: ', results);
       });
-      customer.orders.findById('2', console.log);
+      customer.orders.findById(order3.id, console.log);
-      customer.orders.destroy('2', console.log);
+      customer.orders.destroy(order3.id, console.log);
     });
   });
 });
@@ -114,15 +120,21 @@ Part.hasAndBelongsToMany(Assembly);
 Assembly.create({name: 'car'}, function (err, assembly) {
   Part.create({partNumber: 'engine'}, function (err, part) {
     assembly.parts.add(part, function (err) {
-      assembly.parts(console.log);
+      assembly.parts(function(err, parts) {
+        console.log('Parts: ', parts);
+      });
 
       // Build an part?
       var part3 = assembly.parts.build({partNumber: 'door'});
       console.log('Part3: ', part3, part3.constructor.modelName);
 
       // Create a part?
-      assembly.parts.create({name: 'door'}, function(err, part4) {
+      assembly.parts.create({partNumber: 'door'}, function(err, part4) {
         console.log('Part4: ', part4, part4.constructor.modelName);
+
+        Assembly.find({include: 'parts'}, function(err, assemblies) {
+          console.log('Assemblies: ', assemblies);
+        });
       });
     });
 

lib/dao.js

@@ -732,7 +732,6 @@ DataAccessObject.prototype.save = function (options, callback) {
 
   var inst = this;
   var data = inst.toObject(true);
-  var Model = this.constructor;
   var modelName = Model.modelName;
 
   if (!getIdValue(Model, this)) {

lib/include.js

@@ -1,3 +1,4 @@
+var async = require('async');
 var utils = require('./utils');
 var isPlainObject = utils.isPlainObject;
 var defineCachedRelations = utils.defineCachedRelations;
@@ -42,52 +43,46 @@ function Inclusion() {
 Inclusion.include = function (objects, include, cb) {
   var self = this;
 
-  if (
+  if (!include || (Array.isArray(include) && include.length === 0) ||
-    !include || (Array.isArray(include) && include.length === 0) ||
+      (isPlainObject(include) && Object.keys(include).length === 0)) {
-      (isPlainObject(include) && Object.keys(include).length === 0)
+    // The objects are empty
-    ) {
+    return process.nextTick(function() {
-    cb(null, objects);
+      cb && cb(null, objects);
-    return;
-  }
-
-  include = processIncludeJoin(include);
-
-  var keyVals = {};
-  var objsByKeys = {};
-
-  var nbCallbacks = 0;
-  for (var i = 0; i < include.length; i++) {
-    var callback = processIncludeItem(objects, include[i], keyVals, objsByKeys);
-    if (callback !== null) {
-      nbCallbacks++;
-      callback(function () {
-        nbCallbacks--;
-        if (nbCallbacks === 0) {
-          cb(null, objects);
-        }
     });
-    } else {
-      cb(null, objects);
-    }
   }
 
-  function processIncludeJoin(ij) {
+  include = normalizeInclude(include);
-    if (typeof ij === 'string') {
+
-      ij = [ij];
+  async.each(include, function(item, callback) {
-    }
+    processIncludeItem(objects, item, callback);
-    if (isPlainObject(ij)) {
+  }, function(err) {
-      var newIj = [];
+    cb && cb(err, objects);
-      for (var key in ij) {
+  });
+
+
+  /*!
+   * Normalize the include to be an array
+   * @param include
+   * @returns {*}
+   */
+  function normalizeInclude(include) {
+    if (typeof include === 'string') {
+      return [include];
+    } else if (isPlainObject(include)) {
+      // Build an array of key/value pairs
+      var newInclude = [];
+      for (var key in include) {
         var obj = {};
-        obj[key] = ij[key];
+        obj[key] = include[key];
-        newIj.push(obj);
+        newInclude.push(obj);
       }
-      return newIj;
+      return newInclude;
+    } else {
+      return include;
     }
-    return ij;
   }
 
-  function processIncludeItem(objs, include, keyVals, objsByKeys) {
+  function processIncludeItem(objs, include, cb) {
     var relations = self.relations;
 
     var relationName, subInclude;
@@ -96,7 +91,7 @@ Inclusion.include = function (objects, include, cb) {
       subInclude = include[relationName];
     } else {
       relationName = include;
-      subInclude = [];
+      subInclude = null;
     }
     var relation = relations[relationName];
 
@@ -107,68 +102,40 @@ Inclusion.include = function (objects, include, cb) {
       };
     }
 
-    var req = {'where': {}};
+    // Calling the relation method for each object
-
+    async.each(objs, function (obj, callback) {
-    if (!keyVals[relation.keyFrom]) {
+      if(relation.type === 'belongsTo') {
-      objsByKeys[relation.keyFrom] = {};
+        // If the belongsTo relation doesn't have an owner
-      objs.filter(Boolean).forEach(function (obj) {
+        if(obj[relation.keyFrom] === null || obj[relation.keyFrom] === undefined) {
-        if (!objsByKeys[relation.keyFrom][obj[relation.keyFrom]]) {
+          defineCachedRelations(obj);
-          objsByKeys[relation.keyFrom][obj[relation.keyFrom]] = [];
+          // Set to null if the owner doesn't exist
-        }
+          obj.__cachedRelations[relationName] = null;
-        objsByKeys[relation.keyFrom][obj[relation.keyFrom]].push(obj);
+          obj[relationName] = null;
-      });
+          return callback();
-      keyVals[relation.keyFrom] = Object.keys(objsByKeys[relation.keyFrom]);
-    }
-
-    if (keyVals[relation.keyFrom].length > 0) {
-      // deep clone is necessary since inq seems to change the processed array
-      var keysToBeProcessed = {};
-      var inValues = [];
-      for (var j = 0; j < keyVals[relation.keyFrom].length; j++) {
-        keysToBeProcessed[keyVals[relation.keyFrom][j]] = true;
-        if (keyVals[relation.keyFrom][j] !== 'null'
-          && keyVals[relation.keyFrom][j] !== 'undefined') {
-          inValues.push(keyVals[relation.keyFrom][j]);
         }
       }
-
+      var inst = (obj instanceof self) ? obj : new self(obj);
-      req.where[relation.keyTo] = {inq: inValues};
+      var relationMethod = inst[relationName];
-      req.include = subInclude;
+      // FIXME: [rfeng] How do we pass in the refresh flag?
-
+      relationMethod(function (err, result) {
-      return function (cb) {
+        if (err) {
-        relation.modelTo.find(req, function (err, objsIncluded) {
+          return callback(err);
-          var objectsFrom, j;
-          for (var i = 0; i < objsIncluded.length; i++) {
-            delete keysToBeProcessed[objsIncluded[i][relation.keyTo]];
-            objectsFrom = objsByKeys[relation.keyFrom][objsIncluded[i][relation.keyTo]];
-            for (j = 0; j < objectsFrom.length; j++) {
-              defineCachedRelations(objectsFrom[j]);
-              if (relation.multiple) {
-                if (!objectsFrom[j].__cachedRelations[relationName]) {
-                  objectsFrom[j].__cachedRelations[relationName] = [];
-                }
-                objectsFrom[j].__cachedRelations[relationName].push(objsIncluded[i]);
         } else {
-                objectsFrom[j].__cachedRelations[relationName] = objsIncluded[i];
+          defineCachedRelations(obj);
-              }
+          obj.__cachedRelations[relationName] = result;
-            }
+          obj[relationName] = result;
-          }
 
-          // No relation have been found for these keys
+          if (subInclude && result) {
-          for (var key in keysToBeProcessed) {
+            var subItems = relation.multiple ? result : [result];
-            objectsFrom = objsByKeys[relation.keyFrom][key];
+            // Recursively include the related models
-            for (j = 0; j < objectsFrom.length; j++) {
+            relation.modelTo.include(subItems, subInclude, callback);
-              defineCachedRelations(objectsFrom[j]);
+          } else {
-              objectsFrom[j].__cachedRelations[relationName] =
+            callback(null, result);
-                relation.multiple ? [] : null;
           }
         }
-          cb(err, objsIncluded);
       });
-      };
+    }, cb);
-    }
 
-    return null;
   }
 };
 

lib/model.js

@@ -116,7 +116,10 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
     if (i in properties && typeof data[i] !== 'function') {
       this.__data[i] = this.__dataWas[i] = clone(data[i]);
     } else if (i in ctor.relations) {
+      if (ctor.relations[i].type === 'belongsTo' && data[i] !== null && data[i] !== undefined) {
+        // If the related model is populated
         this.__data[ctor.relations[i].keyFrom] = this.__dataWas[i] = data[i][ctor.relations[i].keyTo];
+      }
       this.__cachedRelations[i] = data[i];
     } else {
       if (strict === false) {

lib/relations.js

@@ -78,6 +78,10 @@ Relation.hasMany = function hasMany(anotherClass, params) {
     modelTo: anotherClass,
     multiple: true
   };
+
+  if (params.through) {
+    this.relations[methodName].modelThrough = params.through;
+  }
   // each instance of this class should have method named
   // pluralize(anotherClass.modelName)
   // which is actually just anotherClass.find({where: {thisModelNameId: this[idName]}}, cb);

lib/scope.js

@@ -47,6 +47,7 @@ function defineScope(cls, targetClass, name, params, methods) {
      *
      */
     get: function () {
+      var self = this;
       var f = function caller(condOrRefresh, cb) {
         var actualCond = {};
         var actualRefresh = false;
@@ -65,8 +66,9 @@ function defineScope(cls, targetClass, name, params, methods) {
           throw new Error('Method can be only called with one or two arguments');
         }
 
-        if (!this.__cachedRelations || (this.__cachedRelations[name] === undefined) || actualRefresh) {
+        if (!self.__cachedRelations || self.__cachedRelations[name] === undefined
-          var self = this;
+          || actualRefresh) {
+          // It either doesn't hit the cache or reresh is required
           var params = mergeParams(actualCond, caller._scope);
           return targetClass.find(params, function (err, data) {
             if (!err && saveOnCache) {
@@ -76,7 +78,8 @@ function defineScope(cls, targetClass, name, params, methods) {
             cb(err, data);
           });
         } else {
-          cb(null, this.__cachedRelations[name]);
+          // Return from cache
+          cb(null, self.__cachedRelations[name]);
         }
       };
       f._scope = typeof params === 'function' ? params.call(this) : params;

lib/sql.js

@@ -194,8 +194,11 @@ BaseSQL.prototype.exists = function (model, id, callback) {
  * @param {Function} callback The callback function
  */
 BaseSQL.prototype.find = function find(model, id, callback) {
+  var idQuery = (id === null || id === undefined)
+    ? this.idColumnEscaped(model) + ' IS NULL'
+    : this.idColumnEscaped(model) + ' = ' + id;
   var sql = 'SELECT * FROM ' +
-    this.tableEscaped(model) + ' WHERE ' + this.idColumnEscaped(model) + ' = ' + id + ' LIMIT 1';
+    this.tableEscaped(model) + ' WHERE ' + idQuery + ' LIMIT 1';
 
   this.query(sql, function (err, data) {
     if (data && data.length === 1) {

test/include.test.js

@@ -1,15 +1,14 @@
 // This test written in mocha+should.js
 var should = require('./init.js');
 
-var db, User, Post, Passport, City, Street, Building;
+var db, User, Post, Passport, City, Street, Building, Assembly, Part;
-var nbSchemaRequests = 0;
 
 describe('include', function () {
 
   before(setup);
 
   it('should fetch belongsTo relation', function (done) {
-    Passport.all({include: 'owner'}, function (err, passports) {
+    Passport.find({include: 'owner'}, function (err, passports) {
       passports.length.should.be.ok;
       passports.forEach(function (p) {
         p.__cachedRelations.should.have.property('owner');
@@ -32,7 +31,7 @@ describe('include', function () {
   });
 
   it('should fetch hasMany relation', function (done) {
-    User.all({include: 'posts'}, function (err, users) {
+    User.find({include: 'posts'}, function (err, users) {
       should.not.exist(err);
       should.exist(users);
       users.length.should.be.ok;
@@ -52,7 +51,7 @@ describe('include', function () {
   });
 
   it('should fetch Passport - Owner - Posts', function (done) {
-    Passport.all({include: {owner: 'posts'}}, function (err, passports) {
+    Passport.find({include: {owner: 'posts'}}, function (err, passports) {
       should.not.exist(err);
       should.exist(passports);
       passports.length.should.be.ok;
@@ -81,7 +80,7 @@ describe('include', function () {
   });
 
   it('should fetch Passports - User - Posts - User', function (done) {
-    Passport.all({
+    Passport.find({
       include: {owner: {posts: 'author'}}
     }, function (err, passports) {
       should.not.exist(err);
@@ -109,7 +108,7 @@ describe('include', function () {
   });
 
   it('should fetch User - Posts AND Passports', function (done) {
-    User.all({include: ['posts', 'passports']}, function (err, users) {
+    User.find({include: ['posts', 'passports']}, function (err, users) {
       should.not.exist(err);
       should.exist(users);
       users.length.should.be.ok;
@@ -140,6 +139,39 @@ describe('include', function () {
     });
   });
 
+  it('should support hasAndBelongsToMany', function (done) {
+
+    Assembly.destroyAll(function(err) {
+      Part.destroyAll(function(err) {
+        Assembly.relations.parts.modelThrough.destroyAll(function(err) {
+        Assembly.create({name: 'car'}, function (err, assembly) {
+          Part.create({partNumber: 'engine'}, function (err, part) {
+            assembly.parts.add(part, function (err, data) {
+              assembly.parts(function (err, parts) {
+                should.not.exist(err);
+                should.exists(parts);
+                parts.length.should.equal(1);
+                parts[0].partNumber.should.equal('engine');
+
+                // Create a part
+                assembly.parts.create({partNumber: 'door'}, function (err, part4) {
+
+                  Assembly.find({include: 'parts'}, function (err, assemblies) {
+                    assemblies.length.should.equal(1);
+                    assemblies[0].parts.length.should.equal(2);
+                    done();
+                  });
+
+                });
+              });
+            });
+          });
+        });
+        });
+      });
+    });
+  });
+
 });
 
 function setup(done) {
@@ -163,6 +195,17 @@ function setup(done) {
   User.hasMany('posts', {foreignKey: 'userId'});
   Post.belongsTo('author', {model: User, foreignKey: 'userId'});
 
+  Assembly = db.define('Assembly', {
+    name: String
+  });
+
+  Part = db.define('Part', {
+    partNumber: String
+  });
+
+  Assembly.hasAndBelongsToMany(Part);
+  Part.hasAndBelongsToMany(Assembly);
+
   db.automigrate(function () {
     var createdUsers = [];
     var createdPassports = [];