Fix count queries with includes (#6190)

Add changelog entry

changelog.md

 # Future
-- [FIXED] Fixed an issue where custom-named model fields break when offsetting, ordering, and including hasMany simultaneously. [#5985] (https://github.com/sequelize/sequelize/issues/5985) 
+- [FIXED] Fixed an issue where custom-named model fields break when offsetting, ordering, and including hasMany simultaneously. [#5985](https://github.com/sequelize/sequelize/issues/5985)
+- [FIXED] Don't remove includes from count queries and unify findAndCount and count queries. [#6123](https://github.com/sequelize/sequelize/issues/6123)
+- [FIXED] `Model.count` don't include attributes [#5057](https://github.com/sequelize/sequelize/issues/5057)
 
 # 3.23.3
 - [FIXED] Pass ResourceLock instead of raw connection in MSSQL disconnect handling

lib/model.js

@@ -1595,34 +1595,23 @@ Model.prototype.aggregate = function(attribute, aggregateFunction, options) {
  * @return {Promise<Integer>}
  */
 Model.prototype.count = function(options) {
-  options = Utils.cloneDeep(options);
-  _.defaults(options, { hooks: true });
-
-  var col = '*';
-
   return Promise.bind(this).then(function() {
-    conformOptions(options, this);
-    this.$injectScope(options);
-
+    options = _.defaults(Utils.cloneDeep(options), { hooks: true });
     if (options.hooks) {
       return this.runHooks('beforeCount', options);
     }
   }).then(function() {
-    if (options.include) {
-      col = this.name + '.' + this.primaryKeyField;
-      expandIncludeAll.call(this, options);
-      validateIncludedElements.call(this, options);
-    }
+    var col = options.include ? this.name + '.' + this.primaryKeyField : '*';
 
-    Utils.mapOptionFieldNames(options, this);
-
-    options.plain = options.group ? false : true;
+    options.plain = !options.group;
     options.dataType = new DataTypes.INTEGER();
     options.includeIgnoreAttributes = false;
+
+    // No limit, offset, order or attributes for the options max be given to count()
+    // Set them to null to prevent scopes setting those values
     options.limit = null;
     options.offset = null;
     options.order = null;
-    options.attributes = [];
 
     return this.aggregate(col, 'count', options);
   });
@@ -1650,6 +1639,7 @@ Model.prototype.count = function(options) {
  *   include: [
  *      { model: Profile, required: true}
  *   ],
+ *   distinct: true,
  *   limit 3
  * });
  * ```
@@ -1666,38 +1656,11 @@ Model.prototype.findAndCount = function(options) {
     throw new Error('The argument passed to findAndCount must be an options object, use findById if you wish to pass a single primary key value');
   }
 
-  var self = this
-    // no limit, offset, order, attributes for the options given to count()
-    , countOptions = _.omit(_.clone(options), ['offset', 'limit', 'order', 'attributes']);
-
-  conformOptions(countOptions, this);
-
-  if (countOptions.include) {
-    countOptions.include = _.cloneDeepWith(countOptions.include, function (element) {
-      if (element instanceof Model) return element;
-      if (element instanceof Association) return element;
-      return undefined;
-    });
-
-    expandIncludeAll.call(this, countOptions);
-
-    validateIncludedElements.call(this, countOptions);
-
-    var keepNeeded = function(includes) {
-      return includes.filter(function (include) {
-        if (include.include) include.include = keepNeeded(include.include);
-
-        return include.required || include.hasIncludeRequired;
-      });
-    };
-    countOptions.include = keepNeeded(countOptions.include);
-
-    if (countOptions.include.length) {
-      // Use distinct to count the number of parent rows, instead of the number of matched includes
-      countOptions.distinct = true;
-    }
+  var self = this;
+  var countOptions = Utils.cloneDeep(options);
+  if (countOptions.attributes) {
+    countOptions.attributes = undefined;
   }
-
   return self.count(countOptions).then(function(count) {
     if (count === 0) {
       return {
@@ -1708,7 +1671,7 @@ Model.prototype.findAndCount = function(options) {
     return self.findAll(options).then(function(results) {
       return {
         count: count || 0,
-        rows: (results && Array.isArray(results) ? results : [])
+        rows: results
       };
     });
   });

test/integration/include/findAndCountAll.test.js

@@ -110,7 +110,7 @@ describe(Support.getTestDialectTeaser('Include'), function() {
       });
     });
 
-    it('should count on a where and not use an uneeded include', function() {
+    it('should count on a where', function() {
       var Project = this.sequelize.define('Project', {
         id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, autoIncrement: true },
         project_name: { type: DataTypes.STRING}
@@ -136,7 +136,8 @@ describe(Support.getTestDialectTeaser('Include'), function() {
       }).then(function() {
         return User.findAndCountAll({
           where: {id: userId},
-          include: [Project]
+          include: [Project],
+          distinct: true
         });
       }).then(function(result) {
         expect(result.rows.length).to.equal(1);
@@ -197,7 +198,8 @@ describe(Support.getTestDialectTeaser('Include'), function() {
         // Query for the first instance of Foo which have related Bars with m === 'yes'
         return Foo.findAndCountAll({
           include: [{ model: Bar, where: { m: 'yes' } }],
-          limit: 1
+          limit: 1,
+          distinct: true
         });
       }).then(function(result) {
         // There should be 2 instances matching the query (Instances 1 and 2), see the findAll statement

test/integration/model/count.test.js

@@ -51,5 +51,23 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         }]
       })).to.eventually.equal(1);
     });
+
+    it('should be able to use where clause on included models', function() {
+      var self = this;
+      var queryObject = {
+        col: 'username',
+        include: [self.Project],
+        where: {
+          '$Projects.name$': 'project1'
+        }
+      };
+      return self.User.count(queryObject).then(function(count) {
+        expect(parseInt(count)).to.be.eql(1);
+        queryObject.where['$Projects.name$'] = 'project2';
+        return self.User.count(queryObject);
+      }).then(function(count) {
+        expect(parseInt(count)).to.be.eql(0);
+      });
+    });
   });
 });

test/unit/model/count.test.js

+'use strict';
+
+/* jshint -W030 */
+var chai = require('chai')
+  , expect = chai.expect
+  , Support = require(__dirname + '/../support')
+  , current = Support.sequelize
+  , sinon = require('sinon')
+  , DataTypes = require(__dirname + '/../../../lib/data-types')
+  , Promise = require('bluebird');
+
+describe(Support.getTestDialectTeaser('Model'), function () {
+  describe('method count', function () {
+    before(function () {
+      this.oldFindAll = current.Model.prototype.findAll;
+      this.oldAggregate = current.Model.prototype.aggregate;
+
+      current.Model.prototype.findAll = sinon.stub().returns(Promise.resolve());
+
+      this.User = current.define('User', {
+        username: DataTypes.STRING,
+        age: DataTypes.INTEGER
+      });
+      this.Project = current.define('Project', {
+        name: DataTypes.STRING
+      });
+
+      this.User.hasMany(this.Project);
+      this.Project.belongsTo(this.User);
+    });
+
+    beforeEach(function () {
+      this.stub = current.Model.prototype.aggregate = sinon.stub().returns(Promise.resolve());
+    });
+
+    after(function () {
+      current.Model.prototype.findAll = this.oldFindAll;
+      current.Model.prototype.aggregate = this.oldAggregate;
+    });
+
+    describe('should pass the same options to model.aggregate as findAndCount', function () {
+      it('with includes', function () {
+        var self = this;
+        var queryObject = {
+          include: [self.Project]
+        };
+        return self.User.count(queryObject).then(function () {
+          return self.User.findAndCount(queryObject);
+        }).then(function () {
+          var count = self.stub.getCall(0).args;
+          var findAndCount = self.stub.getCall(1).args;
+          expect(count).to.eql(findAndCount);
+        });
+      });
+
+      it('attributes should be stripped in case of findAndCount', function () {
+        var self = this;
+        var queryObject = {
+          attributes: ['username']
+        };
+        return self.User.count(queryObject).then(function () {
+          return self.User.findAndCount(queryObject);
+        }).then(function () {
+          var count = self.stub.getCall(0).args;
+          var findAndCount = self.stub.getCall(1).args;
+          expect(count[2].attributes).to.eql(['username']);
+          expect(count).not.to.eql(findAndCount);
+          count[2].attributes = undefined;
+          expect(count).to.eql(findAndCount);
+        });
+      });
+    });
+
+  });
+});
\ No newline at end of file