Merge branch 'master' into hotfix/transaction_timeout

Sascha Depold
Commit 308e7006 authored Feb 24, 2014 by Sascha Depold
Showing with 317 additions and 32 deletions
changelog.md
lib/dao-factory.js
lib/dao.js
lib/dialects/abstract/query-generator.js
package.json
test/dao-factory/findAll.test.js
test/include.test.js
test/include/findAll.test.js
--- a/changelog.md
+++ b/changelog.md
 Notice: All 1.7.x changes are present in 2.0.x aswell
-# v1.7.0-rc7 (next)
+# v1.7.0-rc9 (next)
+- [PERFORMANCE] fixes performance regression introduced in rc7
+- [FEATURE] include all relations for a model [#1421](https://github.com/sequelize/sequelize/pull/1421)
+- [FEATURE] covers more advanced include cases with limiting and filtering
+# v1.7.0-rc8
+- [BUG] fixes bug with required includes without wheres with subqueries
+# v1.7.0-rc7
 - [BUG] ORDER BY statements when using includes should now be places in the appropriate sub/main query more intelligently.
 - [BUG] using include.attributes with primary key attributes specified should no longer result in multiple primary key attributes being selected [#1410](https://github.com/sequelize/sequelize/pull/1410)
 - [DEPENDENCIES] all dependencies, including Validator have been updated to the latest versions.

--- a/lib/dao-factory.js
+++ b/lib/dao-factory.js
@@ -294,6 +294,9 @@ module.exports = (function() {
    Object.defineProperties(this.DAO.prototype, attributeManipulation)
    this.DAO.prototype.attributes = Object.keys(this.DAO.prototype.rawAttributes)
+    this.DAO.prototype._isAttribute = Utils._.memoize(function (key) {
+      return self.DAO.prototype.attributes.indexOf(key) !== -1
+    })
  }
  DAOFactory.prototype.sync = function(options) {
@@ -1342,8 +1345,38 @@ module.exports = (function() {
    options.includeMap = {}
    options.hasSingleAssociation = false
    options.hasMultiAssociation = false
-    options.include = options.include.map(function(include) {
-      include = validateIncludedElement.call(this, include, options.daoFactory, tableNames)
+    // if include is not an array, wrap in an array
+    if (!Array.isArray(options.include)) {
+      options.include = [options.include]
+    }
+    // convert all included elements to { daoFactory: Model } form
+    var includes = options.include = options.include.map(function(include) {
+      if (include instanceof DAOFactory) {
+        return { daoFactory: include }
+      } else if (typeof include !== 'object') {
+        throw new Error('Include unexpected. Element has to be either an instance of DAOFactory or an object.')
+      } else if (include.hasOwnProperty('model')) {
+        include.daoFactory = include.model
+        delete include.model
+      }
+      return include
+    })
+    // validate all included elements
+    for (var index = 0; index < includes.length; index++) {
+      var include = includes[index]
+      if (include.all) {
+        includes.splice(index, 1)
+        index--
+        validateIncludedAllElement.call(this, includes, include)
+        continue
+      }
+      include = includes[index] = validateIncludedElement.call(this, include, tableNames)
      options.includeMap[include.as] = include
      options.includeNames.push(include.as)
@@ -1354,28 +1387,11 @@ module.exports = (function() {
      options.hasIncludeWhere = options.hasIncludeWhere || include.hasIncludeWhere || !!include.where
      options.hasIncludeRequired = options.hasIncludeRequired || include.hasIncludeRequired || !!include.required
-      return include
-    }.bind(this))
-  };
-  var validateIncludedElement = function(include, parent, tableNames) {
-    if (include instanceof DAOFactory) {
-      include = { daoFactory: include }
-    }
-    if (typeof parent === "undefined") {
-      parent = this
-    }
-    if (typeof include !== 'object') {
-      throw new Error('Include unexpected. Element has to be either an instance of DAOFactory or an object.')
    }
+  }
-    if (include.hasOwnProperty('model')) {
+  var validateIncludedElement = function(include, tableNames) {
-      include.daoFactory = include.model
+    if (!include.hasOwnProperty('daoFactory')) {
-      delete include.model
-    } else if (!include.hasOwnProperty('daoFactory')) {
      throw new Error('Include malformed. Expected attributes: daoFactory, as!')
    }
@@ -1400,7 +1416,7 @@ module.exports = (function() {
    if (include._pseudo) return include
    // check if the current daoFactory is actually associated with the passed daoFactory - or it's a pseudo include
-    var association = parent.getAssociation(include.daoFactory, include.as)
+    var association = this.getAssociation(include.daoFactory, include.as)
    if (association) {
      include.association = association
      include.as = association.as
@@ -1429,7 +1445,7 @@ module.exports = (function() {
      // Validate child includes
      if (include.hasOwnProperty('include')) {
-        validateIncludedElements(include, tableNames)
+        validateIncludedElements.call(include.daoFactory, include, tableNames)
      }
      return include
@@ -1446,6 +1462,100 @@ module.exports = (function() {
    }
  }
+  var validateIncludedAllElement = function(includes, include) {
+    // check 'all' attribute provided is valid
+    var all = include.all
+    delete include.all
+    if (all !== true) {
+      if (!Array.isArray(all)) {
+        all = [all]
+      }
+      var validTypes = {
+        BelongsTo: true,
+        HasOne: true,
+        HasMany: true,
+        One: ['BelongsTo', 'HasOne'],
+        Has: ['HasOne', 'HasMany'],
+        Many: ['HasMany']
+      }
+      for (var i = 0; i < all.length; i++) {
+        var type = all[i]
+        if (type == 'All') {
+          all = true
+          break
+        }
+        var types = validTypes[type]
+        if (!types) {
+          throw new Error('include all \'' + type + '\' is not valid - must be BelongsTo, HasOne, HasMany, One, Has, Many or All')
+        }
+        if (types !== true) {
+          // replace type placeholder e.g. 'One' with it's constituent types e.g. 'HasOne', 'BelongsTo'
+          all.splice(i, 1)
+          i--
+          for (var j = 0; j < types.length; j++) {
+            if (all.indexOf(types[j]) == -1) {
+              all.unshift(types[j])
+              i++
+            }
+          }
+        }
+      }
+    }
+    // add all associations of types specified to includes
+    var nested = include.nested
+    if (nested) {
+      delete include.nested
+      if (!include.include) {
+        include.include = []
+      } else if (!Array.isArray(include.include)) {
+        include.include = [include.include]
+      }
+    }
+    var used = []
+    ;(function addAllIncludes(parent, includes) {
+      used.push(parent)
+      Utils._.forEach(parent.associations, function(association) {
+        if (all !== true && all.indexOf(association.associationType) == -1) {
+          return
+        }
+        // check if model already included, and skip if so
+        var model = association.target
+        var as = association.options.as
+        if (Utils._.find(includes, {daoFactory: model, as: as})) {
+          return
+        }
+        // skip if recursing over a model already nested
+        if (nested && used.indexOf(model) != -1) {
+          return
+        }
+        // include this model
+        var thisInclude = optClone(include)
+        thisInclude.daoFactory = model
+        if (as) {
+          thisInclude.as = as
+        }
+        includes.push(thisInclude)
+        // run recursively if nested
+        if (nested) {
+          addAllIncludes(model, thisInclude.include)
+        }
+      })
+      used.pop()
+    })(this, includes)
+  }
  var replaceReferencesWithTableNames = function(attributes) {
    Object.keys(attributes).forEach(function(attrName) {
      if (attributes[attrName].references instanceof DAOFactory) {

--- a/lib/dao.js
+++ b/lib/dao.js
@@ -162,8 +162,8 @@ module.exports = (function() {
          this._setInclude(key, value, options)
          return
        } else {
-          // If not raw, and attribute is not in model definition
+          // If not raw, and attribute is not in model definition, return
-          if (!options.raw && Object.keys(this.Model.attributes).indexOf(key) === -1) {
+          if (!options.raw && !this._isAttribute(key)) {
            return;
          }

--- a/lib/dialects/abstract/query-generator.js
+++ b/lib/dialects/abstract/query-generator.js
@@ -724,7 +724,7 @@ module.exports = (function() {
          var joinQueryItem = generateJoinQuery(include, tableName)
          // If not many to many, and we're doing a subquery, add joinQuery to subQueries
-          if (!include.association.isMultiAssociation && (subQuery && (include.hasIncludeWhere || include.where))) {
+          if (!include.association.isMultiAssociation && (subQuery && (include.hasIncludeRequired || include.required))) {
            subJoinQueries.push(joinQueryItem)
          } else {
            mainJoinQueries.push(joinQueryItem)

--- a/package.json
+++ b/package.json
 {
  "name": "sequelize",
  "description": "Multi dialect ORM for Node.JS",
-  "version": "1.7.0-rc6",
+  "version": "1.7.0-rc8",
  "author": "Sascha Depold <sascha@depold.com>",
  "contributors": [
    {

--- a/test/dao-factory/findAll.test.js
+++ b/test/dao-factory/findAll.test.js
@@ -830,6 +830,123 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
          })
        })
      })
+      describe('include all', function() {
+        beforeEach(function(done) {
+          var self = this
+          self.Continent = this.sequelize.define('Continent', { name: Sequelize.STRING })
+          self.Country = this.sequelize.define('Country', { name: Sequelize.STRING })
+          self.Industry = this.sequelize.define('Industry', { name: Sequelize.STRING })
+          self.Person = this.sequelize.define('Person', { name: Sequelize.STRING, lastName: Sequelize.STRING })
+          self.Continent.hasMany(self.Country)
+          self.Country.belongsTo(self.Continent)
+          self.Country.hasMany(self.Industry)
+          self.Industry.hasMany(self.Country)
+          self.Country.hasMany(self.Person)
+          self.Person.belongsTo(self.Country)
+          self.Country.hasMany(self.Person, { as: 'Residents', foreignKey: 'CountryResidentId' })
+          self.Person.belongsTo(self.Country, { as: 'CountryResident', foreignKey: 'CountryResidentId' })
+          async.forEach([ self.Continent, self.Country, self.Industry, self.Person ], function(model, callback) {
+            model.sync({ force: true }).done(callback)
+          }, function () {
+            async.parallel({
+              europe: function(callback) {self.Continent.create({ name: 'Europe' }).done(callback)},
+              england: function(callback) {self.Country.create({ name: 'England' }).done(callback)},
+              coal: function(callback) {self.Industry.create({ name: 'Coal' }).done(callback)},
+              bob: function(callback) {self.Person.create({ name: 'Bob', lastName: 'Becket' }).done(callback)}
+            }, function(err, r) {
+              if (err) throw err
+              _.forEach(r, function(item, itemName) {
+                self[itemName] = item
+              })
+              async.parallel([
+                function(callback) {self.england.setContinent(self.europe).done(callback)},
+                function(callback) {self.england.addIndustry(self.coal).done(callback)},
+                function(callback) {self.bob.setCountry(self.england).done(callback)},
+                function(callback) {self.bob.setCountryResident(self.england).done(callback)}
+              ], function(err) {
+                if (err) throw err
+                done()
+              })
+            })
+          })
+        })
+        it('includes all associations', function(done) {
+          this.Country.findAll({ include: [ { all: true } ] }).done(function(err, countries) {
+            expect(err).not.to.be.ok
+            expect(countries).to.exist
+            expect(countries[0]).to.exist
+            expect(countries[0].continent).to.exist
+            expect(countries[0].industries).to.exist
+            expect(countries[0].persons).to.exist
+            expect(countries[0].residents).to.exist
+            done()
+          })
+        })
+        it('includes specific type of association', function(done) {
+          this.Country.findAll({ include: [ { all: 'BelongsTo' } ] }).done(function(err, countries) {
+            expect(err).not.to.be.ok
+            expect(countries).to.exist
+            expect(countries[0]).to.exist
+            expect(countries[0].continent).to.exist
+            expect(countries[0].industries).not.to.exist
+            expect(countries[0].persons).not.to.exist
+            expect(countries[0].residents).not.to.exist
+            done()
+          })
+        })
+        it('utilises specified attributes', function(done) {
+          this.Country.findAll({ include: [ { all: 'HasMany', attributes: [ 'name' ] } ] }).done(function(err, countries) {
+            expect(err).not.to.be.ok
+            expect(countries).to.exist
+            expect(countries[0]).to.exist
+            expect(countries[0].industries).to.exist
+            expect(countries[0].persons).to.exist
+            expect(countries[0].persons[0]).to.exist
+            expect(countries[0].persons[0].name).not.to.be.undefined
+            expect(countries[0].persons[0].lastName).to.be.undefined
+            expect(countries[0].residents).to.exist
+            expect(countries[0].residents[0]).to.exist
+            expect(countries[0].residents[0].name).not.to.be.undefined
+            expect(countries[0].residents[0].lastName).to.be.undefined
+            done()
+          })
+        })
+        it('is over-ruled by specified include', function(done) {
+          this.Country.findAll({ include: [ { all: true }, { model: this.Continent, attributes: [] } ] }).done(function(err, countries) {
+            expect(err).not.to.be.ok
+            expect(countries).to.exist
+            expect(countries[0]).to.exist
+            expect(countries[0].continent).to.exist
+            expect(countries[0].continent.name).to.be.undefined
+            done()
+          })
+        })
+        it('includes all nested associations', function(done) {
+          this.Continent.findAll({ include: [ { all: true, nested: true } ] }).done(function(err, continents) {
+            expect(err).not.to.be.ok
+            expect(continents).to.exist
+            expect(continents[0]).to.exist
+            expect(continents[0].countries).to.exist
+            expect(continents[0].countries[0]).to.exist
+            expect(continents[0].countries[0].industries).to.exist
+            expect(continents[0].countries[0].persons).to.exist
+            expect(continents[0].countries[0].residents).to.exist
+            expect(continents[0].countries[0].continent).not.to.exist
+            done()
+          })
+        })
+      })
    })
    describe('order by eager loaded tables', function() {

--- a/test/include.test.js
+++ b/test/include.test.js
@@ -282,7 +282,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
              {title: 'Dress'},
              {title: 'Bed'}
            ]).done(function () {
-              Product.findAll().done(callback)
+              Product.findAll({order: [ ['id'] ]}).done(callback)
            })
          },
          tags: function(callback) {
@@ -291,7 +291,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
              {name: 'B'},
              {name: 'C'}
            ]).done(function () {
-              Tag.findAll().done(callback)
+              Tag.findAll({order: [ ['id'] ]}).done(callback)
            })
          },
          userProducts: ['user', 'products', function (callback, results) {
@@ -317,7 +317,8 @@ describe(Support.getTestDialectTeaser("Include"), function () {
              {model: Product, include: [
                {model: Tag}
              ]}
-            ]
+            ],
+            order: [ ['id'], [Product, 'id'] ]
          }).done(function (err, user) {
            expect(err).not.to.be.ok

--- a/test/include/findAll.test.js
+++ b/test/include/findAll.test.js
@@ -780,6 +780,55 @@ describe(Support.getTestDialectTeaser("Include"), function () {
      })
    })
+    it('should be possible to define a belongsTo include as required with limit', function (done) {
+      var User = this.sequelize.define('User', {})
+        , Group = this.sequelize.define('Group', {
+            name: DataTypes.STRING
+          })
+      User.belongsTo(Group)
+      this.sequelize.sync({force: true}).done(function () {
+        async.auto({
+          groups: function (callback) {
+            Group.bulkCreate([
+              {name: 'A'},
+              {name: 'B'}
+            ]).done(function () {
+              Group.findAll().done(callback)
+            })
+          },
+          users: function (callback) {
+            User.bulkCreate([{}, {}]).done(function () {
+              User.findAll().done(callback)
+            })
+          },
+          userGroups: ['users', 'groups', function (callback, results) {
+            var chainer = new Sequelize.Utils.QueryChainer()
+            chainer.add(results.users[0].setGroup(results.groups[1]))
+            chainer.add(results.users[1].setGroup(results.groups[0]))
+            chainer.run().done(callback)
+          }]
+        }, function (err, results) {
+          expect(err).not.to.be.ok
+          User.findAll({
+            include: [
+              {model: Group, required: true}
+            ],
+            limit: 1
+          }).done(function (err, users) {
+            expect(err).not.to.be.ok
+            expect(users.length).to.equal(1)
+            users.forEach(function (user) {
+              expect(user.group).to.be.ok
+            })
+            done()
+          })
+        })
+      })
+    })
    it('should be possible to extend the on clause with a where option on a hasOne include', function (done) {
      var User = this.sequelize.define('User', {})
        , Project = this.sequelize.define('Project', {