refactor(include): refactor N:M subquery filtering to use the query generator re…

…cursively by extracting the needed include tree

lib/dialects/abstract/query-generator.js

@@ -723,7 +723,9 @@ module.exports = (function() {
         , mainAttributes = options.attributes && options.attributes.slice(0)
         , mainAttributes = options.attributes && options.attributes.slice(0)
         , mainJoinQueries = []
         , mainJoinQueries = []
         // We'll use a subquery if we have hasMany associations and a limit and a filtered/required association
         // We'll use a subquery if we have hasMany associations and a limit and a filtered/required association
-        , subQuery = limit && (options.hasIncludeWhere || options.hasIncludeRequired || options.hasMultiAssociation) && options.subQuery !== false
+        , subQuery = options.subQuery === undefined ?
+                     limit && (options.hasIncludeWhere || options.hasIncludeRequired || options.hasMultiAssociation) && options.subQuery !== false :
+                     options.subquery
         , subQueryItems = []
         , subQueryItems = []
         , subQueryAttributes = null
         , subQueryAttributes = null
         , subJoinQueries = []
         , subJoinQueries = []
@@ -920,17 +922,74 @@ module.exports = (function() {
               joinQueryItem += ' AND ' + targetWhere;
               joinQueryItem += ' AND ' + targetWhere;
               if (subQuery && include.required) {
               if (subQuery && include.required) {
                 if (!options.where) options.where = {};
                 if (!options.where) options.where = {};
-
-                // Creating the as-is where for the subQuery, checks that the required association exists
-                options.where['__' + throughAs] = self.sequelize.asIs(['(',
-
-                  'SELECT ' + self.quoteIdentifier(throughAs) + '.' + self.quoteIdentifier(identSource) + ' FROM ' + self.quoteTable(throughTable, throughAs),
-                  ! include.required && joinType + self.quoteTable(association.source.tableName, tableSource) + ' ON ' + sourceJoinOn || '',
-                  joinType + self.quoteTable(table, as) + ' ON ' + targetJoinOn,
-                  'WHERE ' + (! include.required && targetWhere || sourceJoinOn + ' AND ' + targetWhere),
-                  'LIMIT 1',
-
-                ')', 'IS NOT NULL'].join(' '));
+                (function (include) {
+                  // Closure to use sane local variables
+
+                  var parent = include
+                    , child = include
+                    , nestedIncludes = []
+                    , topParent
+                    , topInclude
+                    , $query;
+
+                  while (parent = parent.parent) {
+                    nestedIncludes = [_.extend({}, child, {include: nestedIncludes})];
+                    child = parent;
+                  }
+
+                  topInclude = nestedIncludes[0];
+                  topParent = topInclude.parent;
+
+                  if (topInclude.through && Object(topInclude.through.model) === topInclude.through.model) {
+                    $query = self.selectQuery(topInclude.through.model.getTableName(), {
+                      attributes: [topInclude.through.model.primaryKeyAttributes[0]],
+                      include: [{
+                        model: topInclude.model,
+                        as: topInclude.model.name,
+                        attributes: [],
+                        association: {
+                          associationType: 'BelongsTo',
+                          isSingleAssociation: true,
+                          source: topInclude.association.target,
+                          target: topInclude.association.source,
+                          identifier: topInclude.association.foreignIdentifier,
+                          identifierField: topInclude.association.foreignIdentifierField
+                        },
+                        required: true,
+                        include: topInclude.include,
+                        _pseudo: true
+                      }],
+                      where: {
+                        $join: self.sequelize.asIs([
+                          self.quoteTable(topParent.model.name) + '.' + self.quoteIdentifier(topParent.model.primaryKeyAttributes[0]),
+                          self.quoteIdentifier(topInclude.through.model.name) + '.' + self.quoteIdentifier(topInclude.association.identifierField)
+                        ].join(" = "))
+                      },
+                      limit: 1,
+                      includeIgnoreAttributes: false
+                    }, topInclude.through.model);
+                  } else {
+                    $query = self.selectQuery(topInclude.model.tableName, {
+                      attributes: [topInclude.model.primaryKeyAttributes[0]],
+                      include: topInclude.include,
+                      where: {
+                        $join: self.sequelize.asIs([
+                          self.quoteTable(topParent.model.name) + '.' + self.quoteIdentifier(topParent.model.primaryKeyAttributes[0]),
+                          self.quoteIdentifier(topInclude.model.name) + '.' + self.quoteIdentifier(topInclude.association.identifierField)
+                        ].join(" = "))
+                      },
+                      limit: 1,
+                      includeIgnoreAttributes: false
+                    }, topInclude.model);
+                  }
+
+                  options.where['__' + throughAs] = self.sequelize.asIs([
+                    '(',
+                      $query.replace(/\;$/, ""),
+                    ')',
+                    'IS NOT NULL'
+                  ].join(' '));
+                })(include);
               }
               }
             }
             }
           } else {
           } else {

lib/model.js

@@ -752,7 +752,7 @@ module.exports = (function() {
         type: QueryTypes.SELECT,
         type: QueryTypes.SELECT,
         hasJoin: hasJoin,
         hasJoin: hasJoin,
         tableNames: Object.keys(tableNames)
         tableNames: Object.keys(tableNames)
-      }, queryOptions, { transaction: (options || {}).transaction }));
+      }, queryOptions, { transaction: options.transaction }));
     }).tap(function(results) {
     }).tap(function(results) {
       if (options.hooks) {
       if (options.hooks) {
         return this.runHooks('afterFind', results, options);
         return this.runHooks('afterFind', results, options);
@@ -1816,6 +1816,7 @@ module.exports = (function() {
       options.hasIncludeRequired = options.hasIncludeRequired || include.hasIncludeRequired || !!include.required;
       options.hasIncludeRequired = options.hasIncludeRequired || include.hasIncludeRequired || !!include.required;
     }
     }
   };
   };
+  Model.$validateIncludedElements = validateIncludedElements;
 
 
   var validateIncludedElement = function(include, tableNames) {
   var validateIncludedElement = function(include, tableNames) {
     if (!include.hasOwnProperty('model') && !include.hasOwnProperty('association')) {
     if (!include.hasOwnProperty('model') && !include.hasOwnProperty('association')) {

test/include/findAll.test.js

@@ -7,6 +7,7 @@ var chai      = require('chai')
   , DataTypes = require(__dirname + "/../../lib/data-types")
   , DataTypes = require(__dirname + "/../../lib/data-types")
   , datetime  = require('chai-datetime')
   , datetime  = require('chai-datetime')
   , async     = require('async')
   , async     = require('async')
+  , Promise   = Sequelize.Promise;
 
 
 chai.use(datetime)
 chai.use(datetime)
 chai.config.includeStack = true
 chai.config.includeStack = true
@@ -235,7 +236,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
       }
       }
     })
     })
 
 
-    it('should accept nested `where` and `limit` at the same time', function (done) {
+    it('should accept nested `where` and `limit` at the same time', function () {
       var Product = this.sequelize.define('Product', {
       var Product = this.sequelize.define('Product', {
             title: DataTypes.STRING
             title: DataTypes.STRING
           })
           })
@@ -247,76 +248,53 @@ describe(Support.getTestDialectTeaser("Include"), function () {
         })
         })
         , Set = this.sequelize.define('Set', {
         , Set = this.sequelize.define('Set', {
             title: DataTypes.STRING
             title: DataTypes.STRING
-        })
-
-      Set.hasMany(Product)
-      Product.belongsTo(Set)
-      Product.hasMany(Tag, {through: ProductTag})
-      Tag.hasMany(Product, {through: ProductTag})
+        });
 
 
-      this.sequelize.sync({force: true}).then(function () {
+      Set.hasMany(Product);
+      Product.belongsTo(Set);
+      Product.hasMany(Tag, {through: ProductTag});
+      Tag.hasMany(Product, {through: ProductTag});
 
 
-        return Set.bulkCreate([
+      return this.sequelize.sync({force: true}).then(function () {
+        return Promise.join(
+          Set.bulkCreate([
             {title: 'office'}
             {title: 'office'}
-        ])
-        .then( function() {
-          return Product.bulkCreate([
+          ]),
+          Product.bulkCreate([
             {title: 'Chair'},
             {title: 'Chair'},
             {title: 'Desk'},
             {title: 'Desk'},
             {title: 'Dress'}
             {title: 'Dress'}
-          ])
-        })
-        .then( function() {
-          return Tag.bulkCreate([
+          ]),
+          Tag.bulkCreate([
             {name: 'A'},
             {name: 'A'},
             {name: 'B'},
             {name: 'B'},
             {name: 'C'}
             {name: 'C'}
-          ]).done(function () {
-            return Tag.findAll()
-          })
-        })
-        .then(function() {
-          return Set.findAll()
-        })
-        .then(function(sets) {
-          return [sets, Product.findAll()]
-        })
-        .spread( function(sets, products) {
-          return sets[0].addProduct(products[0])
-          .then( function() {
-            return sets[0].addProduct(products[1])
+          ])
+        ).then(function () {
+          return Promise.join(
+            Set.findAll(),
+            Product.findAll(),
+            Tag.findAll()
+          );
+        }).spread(function (sets, products, tags) {
+          return Promise.join(
+            sets[0].addProducts([products[0], products[1]]),
+            products[0].addTag(tags[0], {priority: 1}).then(function() {
+              return products[0].addTag(tags[1], {priority:2});
+            }).then(function() {
+              return products[0].addTag(tags[2], {priority:1});
+            }),
+            products[1].addTag(tags[1], {priority:2}).then(function() {
+              return products[2].addTag(tags[1], {priority:3});
             }).then( function() {
             }).then( function() {
-            return [sets, products]
-          })
-        })
-        .spread( function(sets, products) {
-          return [sets, products, Tag.findAll()]
-        })
-        .spread( function(sets, products, tags) {
-          return products[0].addTag(tags[0], {priority:1})
-          .then( function() {
-            return products[0].addTag(tags[1], {priority:2})
-          })
-          .then( function() {
-            return products[0].addTag(tags[2], {priority:1})
-          })
-          .then( function() {
-            return products[1].addTag(tags[1], {priority:2})
+              return products[2].addTag(tags[2], {priority:0});
             })
             })
-          .then( function() {
-            return products[2].addTag(tags[1], {priority:3})
-          })
-          .then( function() {
-            return products[2].addTag(tags[2], {priority:0})
-          })
-        })
-        .then( function() {
+          );
+        }).then(function() {
           return Set.findAll({
           return Set.findAll({
-            include: [
-              {
+            include: [{
               model: Product,
               model: Product,
-              include: [
-                {
+              include: [{
                 model: Tag,
                 model: Tag,
                 where: {
                 where: {
                   name: 'A'
                   name: 'A'
@@ -324,17 +302,12 @@ describe(Support.getTestDialectTeaser("Include"), function () {
               }]
               }]
             }],
             }],
             limit: 1
             limit: 1
-          })
-        })
-        .then( function() {
-          done()
-        })
-        .catch(function (err) {
-            expect(err).not.to.be.ok
-            done()
-          })
-        })
-      })
+          }).on('sql', function (sql) {
+            console.log(sql);
+          });
+        });
+      });
+    });
     it('should support an include with multiple different association types', function (done) {
     it('should support an include with multiple different association types', function (done) {
       var User = this.sequelize.define('User', {})
       var User = this.sequelize.define('User', {})
         , Product = this.sequelize.define('Product', {
         , Product = this.sequelize.define('Product', {