add class method "increment" (#7394)

* add class method "increment"

Basically moving code from instance method `increment` to class method `increment`.

close #6359

* fix tests. change: require options.where

changelog.md

+# Next
+- [ADDED] `Model.increment`, to increment multiple rows at a time [#7394](https://github.com/sequelize/sequelize/pull/7394)
+
 # 4.0.0 (final)
 - [ADDED] Add `isSoftDeleted` helper method to model instance [#7408](https://github.com/sequelize/sequelize/issues/7408)
 - [FIXED] Map isolation level strings to tedious isolation level [MSSQL] [#7296](https://github.com/sequelize/sequelize/issues/7296)

lib/dialects/abstract/query-generator.js

@@ -385,7 +385,7 @@ const QueryGenerator = {
                  If you use a string, you have to escape it on your own.
    @private
   */
-  arithmeticQuery(operator, tableName, attrValueHash, where, options) {
+  arithmeticQuery(operator, tableName, attrValueHash, where, options, attributes) {
     attrValueHash = Utils.removeNullValuesFromHash(attrValueHash, this.options.omitNull);
 
     const values = [];
@@ -394,6 +394,7 @@ const QueryGenerator = {
 
     if (this._dialect.supports.returnValues) {
       if (this._dialect.supports.returnValues.returning) {
+        options.mapToModel = true;
         query += ' RETURNING *';
       } else if (this._dialect.supports.returnValues.output) {
         outputFragment = ' OUTPUT INSERTED.*';
@@ -405,9 +406,9 @@ const QueryGenerator = {
       values.push(this.quoteIdentifier(key) + '=' + this.quoteIdentifier(key) + operator + this.escape(value));
     }
 
-    options = options || {};
-    for (const key in options) {
-      const value = options[key];
+    attributes = attributes || {};
+    for (const key in attributes) {
+      const value = attributes[key];
       values.push(this.quoteIdentifier(key) + '=' + this.escape(value));
     }
 

lib/model.js

@@ -16,6 +16,7 @@ const DataTypes = require('./data-types');
 const Hooks = require('./hooks');
 const associationsMixin = require('./associations/mixin');
 const defaultsOptions = { raw: true };
+const assert = require('assert');
 
 /**
  * A Model represents a table in the database. Instances of this class represent a database row.
@@ -2588,14 +2589,7 @@ class Model {
    * @return {Promise<Array<affectedCount,affectedRows>>}
    */
   static update(values, options) {
-
-    if (!options || !options.where) {
-      throw new Error('Missing where attribute in the options parameter passed to update.');
-    }
-
-    if (!_.isPlainObject(options.where) && !_.isArray(options.where) && !(options.where instanceof Utils.SequelizeMethod)) {
-      throw new Error('Expected plain object, array or sequelize method in the options.where parameter of model.update.');
-    }
+    this._optionsMustContainWhere(options);
 
     options = Utils.cloneDeep(options);
     options = _.defaults(options, {
@@ -2854,6 +2848,95 @@ class Model {
     return this.associations.hasOwnProperty(alias);
   }
 
+   /**
+   * Increment the value of one or more columns. This is done in the database, which means it does not use the values currently stored on the Instance. The increment is done using a
+   * ```sql
+   * SET column = column + X WHERE foo = 'bar'
+   * ```
+   * query. To get the correct value after an increment into the Instance you should do a reload.
+   *
+   *```js
+  * Model.increment('number', { where: { foo: 'bar' }) // increment number by 1
+  * Model.increment(['number', 'count'], { by: 2, where: { foo: 'bar' } }) // increment number and count by 2
+  * Model.increment({ answer: 42, tries: -1}, { by: 2, where: { foo: 'bar' } }) // increment answer by 42, and decrement tries by 1.
+  *                                                        // `by` is ignored, since each column has its own value
+  * ```
+  *
+  * @see {@link Model#reload}
+  * @param {String|Array|Object} fields If a string is provided, that column is incremented by the value of `by` given in options. If an array is provided, the same is true for each column. If and object is provided, each column is incremented by the value given.
+  * @param {Object} options
+  * @param {Object} options.where
+  * @param {Integer} [options.by=1] The number to increment by
+  * @param {Boolean} [options.silent=false] If true, the updatedAt timestamp will not be updated.
+  * @param {Function} [options.logging=false] A function that gets executed while running the query to log the sql.
+  * @param {Transaction} [options.transaction]
+  * @param  {String}       [options.searchPath=DEFAULT] An optional parameter to specify the schema search_path (Postgres only)
+  *
+  * @return {Promise<this>}
+  */
+  static increment(fields, options) {
+    this._optionsMustContainWhere(options);
+
+    const updatedAtAttr = this._timestampAttributes.updatedAt;
+    const versionAttr = this._versionAttribute;
+    const updatedAtAttribute = this.rawAttributes[updatedAtAttr];
+    options = _.defaults({}, options, {
+      by: 1,
+      attributes: {},
+      where: {},
+      increment: true
+    });
+
+    const where = _.extend({}, options.where);
+
+    let values = {};
+    if (Utils._.isString(fields)) {
+      values[fields] = options.by;
+    } else if (Utils._.isArray(fields)) {
+      Utils._.each(fields, field => {
+        values[field] = options.by;
+      });
+    } else { // Assume fields is key-value pairs
+      values = fields;
+    }
+
+    if (!options.silent && updatedAtAttr && !values[updatedAtAttr]) {
+      options.attributes[updatedAtAttribute.field || updatedAtAttr] = this._getDefaultTimestamp(updatedAtAttr) || Utils.now(this.sequelize.options.dialect);
+    }
+    if (versionAttr) {
+      values[versionAttr] = 1;
+    }
+
+    for (const attr of Object.keys(values)) {
+      // Field name mapping
+      if (this.rawAttributes[attr] && this.rawAttributes[attr].field && this.rawAttributes[attr].field !== attr) {
+        values[this.rawAttributes[attr].field] = values[attr];
+        delete values[attr];
+      }
+    }
+
+    let promise;
+    if (!options.increment) {
+      promise = this.sequelize.getQueryInterface().decrement(this, this.getTableName(options), values, where, options);
+    } else {
+      promise = this.sequelize.getQueryInterface().increment(this, this.getTableName(options), values, where, options);
+    }
+
+    return promise.then(affectedRows => {
+      if (options.returning) {
+        return [affectedRows, affectedRows.length];
+      }
+
+      return [affectedRows];
+    });
+  }
+
+  static _optionsMustContainWhere(options) {
+    assert(options && options.where, 'Missing where attribute in the options parameter');
+    assert(_.isPlainObject(options.where) || _.isArray(options.where) || options.where instanceof Utils.SequelizeMethod,
+      'Expected plain object, array or sequelize method in the options.where parameter');
+  }
+
   /**
    * Builds a new model instance.
    * @param {Object}  [values={}] an object of key value pairs
@@ -3839,53 +3922,16 @@ class Model {
   * @param  {String}       [options.searchPath=DEFAULT] An optional parameter to specify the schema search_path (Postgres only)
   *
   * @return {Promise<this>}
+  * @since 4.0.0
   */
   increment(fields, options) {
     const identifier = this.where();
-    const updatedAtAttr = this.constructor._timestampAttributes.updatedAt;
-    const versionAttr = this.constructor._versionAttribute;
-    const updatedAtAttribute = this.constructor.rawAttributes[updatedAtAttr];
 
-    options = _.defaults({}, options, {
-      by: 1,
-      attributes: {},
-      where: {},
-      increment: true
-    });
-
-    const where = _.extend({}, options.where, identifier);
-
-    let values = {};
-    if (Utils._.isString(fields)) {
-      values[fields] = options.by;
-    } else if (Utils._.isArray(fields)) {
-      Utils._.each(fields, field => {
-        values[field] = options.by;
-      });
-    } else { // Assume fields is key-value pairs
-      values = fields;
-    }
-
-    if (!options.silent && updatedAtAttr && !values[updatedAtAttr]) {
-      options.attributes[updatedAtAttribute.field || updatedAtAttr] = this.constructor._getDefaultTimestamp(updatedAtAttr) || Utils.now(this.sequelize.options.dialect);
-    }
-    if (versionAttr) {
-      values[versionAttr] = 1;
-    }
-
-    for (const attr of Object.keys(values)) {
-      // Field name mapping
-      if (this.constructor.rawAttributes[attr] && this.constructor.rawAttributes[attr].field && this.constructor.rawAttributes[attr].field !== attr) {
-        values[this.constructor.rawAttributes[attr].field] = values[attr];
-        delete values[attr];
-      }
-    }
-
-    if (!options.increment) {
-      return this.sequelize.getQueryInterface().decrement(this, this.constructor.getTableName(options), values, where, options).return(this);
-    }
+    options = Utils.cloneDeep(options);
+    options.where = _.extend({}, options.where, identifier);
+    options.instance = this;
 
-    return this.sequelize.getQueryInterface().increment(this, this.constructor.getTableName(options), values, where, options).return(this);
+    return this.constructor.increment(fields, options).return(this);
   }
 
   /**

lib/query-interface.js

@@ -674,18 +674,19 @@ class QueryInterface {
     );
   }
 
-  increment(instance, tableName, values, identifier, options) {
-    const sql = this.QueryGenerator.arithmeticQuery('+', tableName, values, identifier, options.attributes);
+  increment(model, tableName, values, identifier, options) {
+    options = Utils.cloneDeep(options);
 
-    options = _.clone(options) || {};
+    const sql = this.QueryGenerator.arithmeticQuery('+', tableName, values, identifier, options, options.attributes);
 
     options.type = QueryTypes.UPDATE;
-    options.instance = instance;
+    options.model = model;
+
     return this.sequelize.query(sql, options);
   }
 
   decrement(instance, tableName, values, identifier, options) {
-    const sql = this.QueryGenerator.arithmeticQuery('-', tableName, values, identifier, options.attributes);
+    const sql = this.QueryGenerator.arithmeticQuery('-', tableName, values, identifier, options, options.attributes);
 
     options = _.clone(options) || {};
 

test/integration/model.test.js

@@ -809,7 +809,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         throw new Error('Update should throw an error if no where clause is given.');
       }, err => {
         expect(err).to.be.an.instanceof(Error);
-        expect(err.message).to.equal('Missing where attribute in the options parameter passed to update.');
+        expect(err.message).to.equal('Missing where attribute in the options parameter');
       });
     });
 
@@ -2736,4 +2736,162 @@ describe(Support.getTestDialectTeaser('Model'), () => {
       })).to.be.rejectedWith(Promise.AggregateError);
     });
   });
+
+  describe('increment', () => {
+    beforeEach(function() {
+      this.User = this.sequelize.define('User', {
+        id: { type: DataTypes.INTEGER, primaryKey: true },
+        aNumber: { type: DataTypes.INTEGER },
+        bNumber: { type: DataTypes.INTEGER }
+      });
+
+      const self = this;
+      return this.User.sync({ force: true }).then(() => {
+        return self.User.bulkCreate([{
+          id: 1,
+          aNumber: 0,
+          bNumber: 0
+        }, {
+          id: 2,
+          aNumber: 0,
+          bNumber: 0
+        }, {
+          id: 3,
+          aNumber: 0,
+          bNumber: 0
+        }]);
+      });
+    });
+
+    it('supports where conditions', function() {
+      const self = this;
+      return this.User.findById(1).then(() => {
+        return self.User.increment(['aNumber'], { by: 2, where: { id: 1 } }).then(() => {
+          return self.User.findById(2).then(user3 => {
+            expect(user3.aNumber).to.be.equal(0);
+          });
+        });
+      });
+    });
+
+    it('should still work right with other concurrent increments', function() {
+      const self = this;
+      return this.User.findAll().then(aUsers => {
+        return self.sequelize.Promise.all([
+          self.User.increment(['aNumber'], { by: 2, where: {} }),
+          self.User.increment(['aNumber'], { by: 2, where: {} }),
+          self.User.increment(['aNumber'], { by: 2, where: {} })
+        ]).then(() => {
+          return self.User.findAll().then(bUsers => {
+            for (let i = 0; i < bUsers.length; i++) {
+              expect(bUsers[i].aNumber).to.equal(aUsers[i].aNumber + 6);
+            }
+          });
+        });
+      });
+    });
+
+    it('with array', function() {
+      const self = this;
+      return this.User.findAll().then(aUsers => {
+        return self.User.increment(['aNumber'], { by: 2, where: {} }).then(() => {
+          return self.User.findAll().then(bUsers => {
+            for (let i = 0; i < bUsers.length; i++) {
+              expect(bUsers[i].aNumber).to.equal(aUsers[i].aNumber + 2);
+            }
+          });
+        });
+      });
+    });
+
+    it('with single field', function() {
+      const self = this;
+      return this.User.findAll().then(aUsers => {
+        return self.User.increment('aNumber', { by: 2, where: {} }).then(() => {
+          return self.User.findAll().then(bUsers => {
+            for (let i = 0; i < bUsers.length; i++) {
+              expect(bUsers[i].aNumber).to.equal(aUsers[i].aNumber + 2);
+            }
+          });
+        });
+      });
+    });
+
+    it('with single field and no value', function() {
+      const self = this;
+      return this.User.findAll().then(aUsers => {
+        return self.User.increment('aNumber', { where: {}}).then(() => {
+          return self.User.findAll().then(bUsers => {
+            for (let i = 0; i < bUsers.length; i++) {
+              expect(bUsers[i].aNumber).to.equal(aUsers[i].aNumber + 1);
+            }
+          });
+        });
+      });
+    });
+
+    it('with key value pair', function() {
+      const self = this;
+      return this.User.findAll().then(aUsers => {
+        return self.User.increment({ 'aNumber': 1, 'bNumber': 2 }, { where: { }}).then(() => {
+          return self.User.findAll().then(bUsers => {
+            for (let i = 0; i < bUsers.length; i++) {
+              expect(bUsers[i].aNumber).to.equal(aUsers[i].aNumber + 1);
+              expect(bUsers[i].bNumber).to.equal(aUsers[i].bNumber + 2);
+            }
+          });
+        });
+      });
+    });
+
+    it('should still work right with other concurrent updates', function() {
+      const self = this;
+      return this.User.findAll().then(aUsers => {
+        return self.User.update({ 'aNumber': 2 }, { where: {} }).then(() => {
+          return self.User.increment(['aNumber'], { by: 2, where: {} }).then(() => {
+            return self.User.findAll().then(bUsers => {
+              for (let i = 0; i < bUsers.length; i++) {
+                expect(bUsers[i].aNumber).to.equal(aUsers[i].aNumber + 4);
+              }
+            });
+          });
+        });
+      });
+    });
+
+    it('with timestamps set to true', function() {
+      const User = this.sequelize.define('IncrementUser', {
+        aNumber: DataTypes.INTEGER
+      }, { timestamps: true });
+      let oldDate;
+
+      return User.sync({ force: true }).bind(this).then(() => {
+        return User.create({aNumber: 1});
+      }).then(function(user) {
+        oldDate = user.updatedAt;
+
+        this.clock.tick(1000);
+        return User.increment('aNumber', {by: 1, where: {}});
+      }).then(() => {
+        return expect(User.findById(1)).to.eventually.have.property('updatedAt').afterTime(oldDate);
+      });
+    });
+
+    it('with timestamps set to true and options.silent set to true', function() {
+      const User = this.sequelize.define('IncrementUser', {
+        aNumber: DataTypes.INTEGER
+      }, { timestamps: true });
+      let oldDate;
+
+      return User.sync({ force: true }).bind(this).then(() => {
+        return User.create({aNumber: 1});
+      }).then(function(user) {
+        oldDate = user.updatedAt;
+        this.clock.tick(1000);
+        return User.increment('aNumber', {by: 1, silent: true, where: { }});
+      }).then(() => {
+        return expect(User.findById(1)).to.eventually.have.property('updatedAt').equalTime(oldDate);
+      });
+    });
+  });
 });

test/unit/dialects/mssql/query-generator.test.js

@@ -174,22 +174,22 @@ if (current.dialect.name === 'mssql') {
     test('arithmeticQuery', () => {
       [{
         title:'Should use the plus operator',
-        arguments: ['+', 'myTable', { foo: 'bar' }, {}],
+        arguments: ['+', 'myTable', { foo: 'bar' }, {}, {}],
         expectation: 'UPDATE myTable SET foo=foo+\'bar\' '
       },
       {
         title:'Should use the plus operator with where clause',
-        arguments: ['+', 'myTable', { foo: 'bar' }, { bar: 'biz'}],
+        arguments: ['+', 'myTable', { foo: 'bar' }, { bar: 'biz'}, {}],
         expectation: 'UPDATE myTable SET foo=foo+\'bar\' WHERE bar = \'biz\''
       },
       {
         title:'Should use the minus operator',
-        arguments: ['-', 'myTable', { foo: 'bar' }],
+        arguments: ['-', 'myTable', { foo: 'bar' }, {}, {}],
         expectation: 'UPDATE myTable SET foo=foo-\'bar\' '
       },
       {
         title:'Should use the minus operator with where clause',
-        arguments: ['-', 'myTable', { foo: 'bar' }, { bar: 'biz'}],
+        arguments: ['-', 'myTable', { foo: 'bar' }, { bar: 'biz'}, {}],
         expectation: 'UPDATE myTable SET foo=foo-\'bar\' WHERE bar = \'biz\''
       }].forEach(test => {
         it(test.title, () => {

test/unit/dialects/mysql/query-generator.test.js

@@ -13,22 +13,22 @@ if (dialect === 'mysql') {
       arithmeticQuery: [
         {
           title:'Should use the plus operator',
-          arguments: ['+', 'myTable', { foo: 'bar' }, {}],
+          arguments: ['+', 'myTable', { foo: 'bar' }, {}, {}],
           expectation: 'UPDATE `myTable` SET `foo`=`foo`+\'bar\' '
         },
         {
           title:'Should use the plus operator with where clause',
-          arguments: ['+', 'myTable', { foo: 'bar' }, { bar: 'biz'}],
+          arguments: ['+', 'myTable', { foo: 'bar' }, { bar: 'biz'}, {}],
           expectation: 'UPDATE `myTable` SET `foo`=`foo`+\'bar\' WHERE `bar` = \'biz\''
         },
         {
           title:'Should use the minus operator',
-          arguments: ['-', 'myTable', { foo: 'bar' }],
+          arguments: ['-', 'myTable', { foo: 'bar' }, {}, {}],
           expectation: 'UPDATE `myTable` SET `foo`=`foo`-\'bar\' '
         },
         {
           title:'Should use the minus operator with where clause',
-          arguments: ['-', 'myTable', { foo: 'bar' }, { bar: 'biz'}],
+          arguments: ['-', 'myTable', { foo: 'bar' }, { bar: 'biz'}, {}],
           expectation: 'UPDATE `myTable` SET `foo`=`foo`-\'bar\' WHERE `bar` = \'biz\''
         }
       ],

test/unit/dialects/postgres/query-generator.test.js

@@ -16,22 +16,22 @@ if (dialect.match(/^postgres/)) {
       arithmeticQuery: [
         {
           title:'Should use the plus operator',
-          arguments: ['+', 'myTable', { foo: 'bar' }, {}],
+          arguments: ['+', 'myTable', { foo: 'bar' }, {}, {}],
           expectation: 'UPDATE "myTable" SET "foo"="foo"+\'bar\'  RETURNING *'
         },
         {
           title:'Should use the plus operator with where clause',
-          arguments: ['+', 'myTable', { foo: 'bar' }, { bar: 'biz'}],
+          arguments: ['+', 'myTable', { foo: 'bar' }, { bar: 'biz'}, {}],
           expectation: 'UPDATE "myTable" SET "foo"="foo"+\'bar\' WHERE "bar" = \'biz\' RETURNING *'
         },
         {
           title:'Should use the minus operator',
-          arguments: ['-', 'myTable', { foo: 'bar' }],
+          arguments: ['-', 'myTable', { foo: 'bar' }, {}, {}],
           expectation: 'UPDATE "myTable" SET "foo"="foo"-\'bar\'  RETURNING *'
         },
         {
           title:'Should use the minus operator with where clause',
-          arguments: ['-', 'myTable', { foo: 'bar' }, { bar: 'biz'}],
+          arguments: ['-', 'myTable', { foo: 'bar' }, { bar: 'biz'}, {}],
           expectation: 'UPDATE "myTable" SET "foo"="foo"-\'bar\' WHERE "bar" = \'biz\' RETURNING *'
         }
       ],

test/unit/increment.test.js

+'use strict';
+
+const chai = require('chai'),
+  expect = chai.expect,
+  Support   = require(__dirname + '/../support'),
+  current   = Support.sequelize,
+  Sequelize = Support.Sequelize;
+
+describe(Support.getTestDialectTeaser('Model'), () => {
+  describe('increment', () => {
+    describe('options tests', () => {
+      const Model = current.define('User', {
+        id: {
+          type:          Sequelize.BIGINT,
+          primaryKey:    true,
+          autoIncrement: true
+        },
+        count: Sequelize.BIGINT
+      });
+
+      it('should reject if options are missing', () => {
+        return expect(() => Model.increment(['id', 'count']))
+          .to.throw('Missing where attribute in the options parameter');
+      });
+
+      it('should reject if options.where are missing', () => {
+        return expect(() => Model.increment(['id', 'count'], { by: 10}))
+          .to.throw('Missing where attribute in the options parameter');
+      });
+    });
+  });
+});