Merge pull request #4023 from smihica/#4011

option object shouldn't be updated internally.

lib/instance.js

@@ -482,7 +482,7 @@ Instance.prototype.save = function(options) {
   if (arguments.length > 1) {
   if (arguments.length > 1) {
     throw new Error('The second argument was removed in favor of the options object.');
     throw new Error('The second argument was removed in favor of the options object.');
   }
   }
-  options = _.defaults(options || {}, {
+  options = _.defaults({}, options, {
     hooks: true,
     hooks: true,
     validate: true
     validate: true
   });
   });
@@ -724,7 +724,7 @@ Instance.prototype.save = function(options) {
 * @return {Promise<this>}
 * @return {Promise<this>}
 */
 */
 Instance.prototype.reload = function(options) {
 Instance.prototype.reload = function(options) {
-  options = _.defaults(options || {}, {
+  options = _.defaults({}, options, {
     where: this.where(),
     where: this.where(),
     limit: 1,
     limit: 1,
     include: this.options.include || null
     include: this.options.include || null
@@ -809,7 +809,7 @@ Instance.prototype.destroy = function(options) {
   options = Utils._.extend({
   options = Utils._.extend({
     hooks: true,
     hooks: true,
     force: false
     force: false
-  }, options || {});
+  }, options);
 
 
   return Promise.bind(this).then(function() {
   return Promise.bind(this).then(function() {
     // Run before hook
     // Run before hook
@@ -821,7 +821,7 @@ Instance.prototype.destroy = function(options) {
 
 
     if (this.Model._timestampAttributes.deletedAt && options.force === false) {
     if (this.Model._timestampAttributes.deletedAt && options.force === false) {
       this.setDataValue(this.Model._timestampAttributes.deletedAt, new Date());
       this.setDataValue(this.Model._timestampAttributes.deletedAt, new Date());
-      return this.save(_.extend(_.clone(options), {hooks : false}));
+      return this.save(_.extend({}, options, {hooks : false}));
     } else {
     } else {
       where = {};
       where = {};
       var primaryKeys = this.Model.primaryKeyAttributes;
       var primaryKeys = this.Model.primaryKeyAttributes;
@@ -855,7 +855,7 @@ Instance.prototype.restore = function(options) {
   options = Utils._.extend({
   options = Utils._.extend({
     hooks: true,
     hooks: true,
     force: false
     force: false
-  }, options || {});
+  }, options);
 
 
   return Promise.bind(this).then(function() {
   return Promise.bind(this).then(function() {
     // Run before hook
     // Run before hook
@@ -864,7 +864,7 @@ Instance.prototype.restore = function(options) {
     }
     }
   }).then(function() {
   }).then(function() {
     this.setDataValue(this.Model._timestampAttributes.deletedAt, null);
     this.setDataValue(this.Model._timestampAttributes.deletedAt, null);
-    return this.save(_.extend(_.clone(options), {hooks : false, omitNull : false}));
+    return this.save(_.extend({}, options, {hooks : false, omitNull : false}));
   }).tap(function() {
   }).tap(function() {
     // Run after hook
     // Run after hook
     if (options.hooks) {
     if (options.hooks) {
@@ -913,13 +913,13 @@ Instance.prototype.increment = function(fields, options) {
     }
     }
   }
   }
 
 
-  options = _.defaults(options || {}, {
+  options = _.defaults({}, options, {
     by: 1,
     by: 1,
     attributes: {},
     attributes: {},
     where: {}
     where: {}
   });
   });
 
 
-  where = _.extend(options.where || {}, identifier);
+  where = _.extend({}, options.where, identifier);
 
 
   if (Utils._.isString(fields)) {
   if (Utils._.isString(fields)) {
     values[fields] = options.by;
     values[fields] = options.by;
@@ -970,7 +970,7 @@ Instance.prototype.increment = function(fields, options) {
  * @return {Promise}
  * @return {Promise}
  */
  */
 Instance.prototype.decrement = function(fields, options) {
 Instance.prototype.decrement = function(fields, options) {
-  options = _.defaults(options || {}, {
+  options = _.defaults({}, options, {
     by: 1
     by: 1
   });
   });
 
 

test/unit/instance/decrement.test.js

+'use strict';
+
+var chai = require('chai')
+  , expect = chai.expect
+  , Support   = require(__dirname + '/../support')
+  , current   = Support.sequelize
+  , Sequelize = Support.Sequelize
+  , sinon     = require('sinon');
+
+describe(Support.getTestDialectTeaser('Instance'), function() {
+  describe('decrement', function () {
+    describe('options tests', function() {
+      var stub
+        , Model = current.define('User', {
+          id: {
+            type:          Sequelize.BIGINT,
+            primaryKey:    true,
+            autoIncrement: true,
+          }
+        })
+        , instance;
+
+      before(function() {
+        stub = sinon.stub(current, 'query').returns(
+          Sequelize.Promise.resolve({
+            _previousDataValues: {id: 3},
+            dataValues: {id: 1}
+          })
+        );
+      });
+
+      after(function() {
+        stub.restore();
+      });
+
+      it('should allow decrements even if options are not given', function () {
+        instance = Model.build({id: 3}, {isNewRecord: false});
+        expect(function () {
+          instance.decrement(['id']);
+        }).to.not.throw();
+      });
+
+      it('should not modify options when it given to decrement', function () {
+        instance = Model.build({id: 3}, {isNewRecord: false});
+        var options = { by: 2 };
+        instance.decrement(['id'], options);
+        expect(options).to.deep.equal({ by: 2 });
+      });
+    });
+  });
+});

test/unit/instance/destroy.test.js

+'use strict';
+
+var chai = require('chai')
+  , expect = chai.expect
+  , Support   = require(__dirname + '/../support')
+  , current   = Support.sequelize
+  , Sequelize = Support.Sequelize
+  , sinon     = require('sinon');
+
+describe(Support.getTestDialectTeaser('Instance'), function() {
+  describe('destroy', function () {
+    describe('options tests', function() {
+      var stub
+        , Model = current.define('User', {
+          id: {
+            type:          Sequelize.BIGINT,
+            primaryKey:    true,
+            autoIncrement: true,
+          }
+        })
+        , instance;
+
+      before(function() {
+        stub = sinon.stub(current, 'query').returns(
+          Sequelize.Promise.resolve({
+            _previousDataValues: {},
+            dataValues: {id: 1}
+          })
+        );
+      });
+
+      after(function() {
+        stub.restore();
+      });
+
+      it('should allow destroies even if options are not given', function () {
+        instance = Model.build({id: 1}, {isNewRecord: false});
+        expect(function () {
+          instance.destroy();
+        }).to.not.throw();
+      });
+
+      it('should not modify options when it given to destroy', function () {
+        instance = Model.build({id: 1}, {isNewRecord: false});
+        var options = { transaction: null };
+        instance.destroy(options);
+        expect(options).to.deep.equal({ transaction: null });
+      });
+    });
+  });
+});

test/unit/instance/increment.test.js

+'use strict';
+
+var chai = require('chai')
+  , expect = chai.expect
+  , Support   = require(__dirname + '/../support')
+  , current   = Support.sequelize
+  , Sequelize = Support.Sequelize
+  , sinon     = require('sinon');
+
+describe(Support.getTestDialectTeaser('Instance'), function() {
+  describe('increment', function () {
+    describe('options tests', function() {
+      var stub
+        , Model = current.define('User', {
+          id: {
+            type:          Sequelize.BIGINT,
+            primaryKey:    true,
+            autoIncrement: true,
+          }
+        })
+        , instance;
+
+      before(function() {
+        stub = sinon.stub(current, 'query').returns(
+          Sequelize.Promise.resolve({
+            _previousDataValues: {id: 1},
+            dataValues: {id: 3}
+          })
+        );
+      });
+
+      after(function() {
+        stub.restore();
+      });
+
+      it('should allow increments even if options are not given', function () {
+        instance = Model.build({id: 1}, {isNewRecord: false});
+        expect(function () {
+          instance.increment(['id']);
+        }).to.not.throw();
+      });
+
+      it('should not modify options when it given to increment', function () {
+        instance = Model.build({id: 1}, {isNewRecord: false});
+        var options = { by: 2 };
+        instance.increment(['id'], options);
+        expect(options).to.deep.equal({ by: 2 });
+      });
+    });
+  });
+});

test/unit/instance/reload.test.js

+'use strict';
+
+var chai = require('chai')
+  , expect = chai.expect
+  , Support   = require(__dirname + '/../support')
+  , current   = Support.sequelize
+  , Sequelize = Support.Sequelize
+  , sinon     = require('sinon');
+
+describe(Support.getTestDialectTeaser('Instance'), function() {
+  describe('reload', function () {
+    describe('options tests', function() {
+      var stub
+        , Model = current.define('User', {
+          id: {
+            type:          Sequelize.BIGINT,
+            primaryKey:    true,
+            autoIncrement: true,
+          },
+          deletedAt: {
+            type:          Sequelize.DATE,
+          }
+        }, {
+          paranoid: true
+        })
+        , instance;
+
+      before(function() {
+        stub = sinon.stub(current, 'query').returns(
+          Sequelize.Promise.resolve({
+            _previousDataValues: {id: 1},
+            dataValues: {id: 2}
+          })
+        );
+      });
+
+      after(function() {
+        stub.restore();
+      });
+
+      it('should allow reloads even if options are not given', function () {
+        instance = Model.build({id: 1}, {isNewRecord: false});
+        expect(function () {
+          instance.reload();
+        }).to.not.throw();
+      });
+
+      it('should not modify options when it given to reload', function () {
+        instance = Model.build({id: 1}, {isNewRecord: false});
+        var options = { transaction: null };
+        instance.reload(options);
+        expect(options).to.deep.equal({ transaction: null });
+      });
+
+    });
+  });
+});

test/unit/instance/restore.test.js

+'use strict';
+
+var chai = require('chai')
+  , expect = chai.expect
+  , Support   = require(__dirname + '/../support')
+  , current   = Support.sequelize
+  , Sequelize = Support.Sequelize
+  , sinon     = require('sinon');
+
+describe(Support.getTestDialectTeaser('Instance'), function() {
+  describe('restore', function () {
+    describe('options tests', function() {
+      var stub
+        , Model = current.define('User', {
+          id: {
+            type:          Sequelize.BIGINT,
+            primaryKey:    true,
+            autoIncrement: true,
+          },
+          deletedAt: {
+            type:          Sequelize.DATE,
+          }
+        }, {
+          paranoid: true
+        })
+        , instance;
+
+      before(function() {
+        stub = sinon.stub(current, 'query').returns(
+          Sequelize.Promise.resolve({
+            _previousDataValues: {id: 1},
+            dataValues: {id: 2}
+          })
+        );
+      });
+
+      after(function() {
+        stub.restore();
+      });
+
+      it('should allow restores even if options are not given', function () {
+        instance = Model.build({id: 1}, {isNewRecord: false});
+        expect(function () {
+          instance.restore();
+        }).to.not.throw();
+      });
+
+      it('should not modify options when it given to restore', function () {
+        instance = Model.build({id: 1}, {isNewRecord: false});
+        var options = { transaction: null };
+        instance.restore(options);
+        expect(options).to.deep.equal({ transaction: null });
+      });
+    });
+  });
+
+});

test/unit/instance/save.test.js

@@ -3,7 +3,9 @@
 var chai = require('chai')
 var chai = require('chai')
   , expect = chai.expect
   , expect = chai.expect
   , Support   = require(__dirname + '/../support')
   , Support   = require(__dirname + '/../support')
-  , current   = Support.sequelize;
+  , current   = Support.sequelize
+  , Sequelize = Support.Sequelize
+  , sinon     = require('sinon');
 
 
 describe(Support.getTestDialectTeaser('Instance'), function() {
 describe(Support.getTestDialectTeaser('Instance'), function() {
   describe('save', function () {
   describe('save', function () {
@@ -19,5 +21,44 @@ describe(Support.getTestDialectTeaser('Instance'), function() {
         instance.save();
         instance.save();
       }).to.throw();
       }).to.throw();
     });
     });
+
+    describe('options tests', function() {
+      var stub
+        , Model = current.define('User', {
+          id: {
+            type:          Sequelize.BIGINT,
+            primaryKey:    true,
+            autoIncrement: true,
+          }
+        })
+        , instance;
+
+      before(function() {
+        stub = sinon.stub(current, 'query').returns(
+          Sequelize.Promise.resolve({
+            _previousDataValues: {},
+            dataValues: {id: 1}
+          })
+        );
+      });
+
+      after(function() {
+        stub.restore();
+      });
+
+      it('should allow saves even if options are not given', function () {
+        instance = Model.build({});
+        expect(function () {
+          instance.save();
+        }).to.not.throw();
+      });
+
+      it('should not modify options when it given to save', function () {
+        instance = Model.build({});
+        var options = { transaction: null };
+        instance.save(options);
+        expect(options).to.deep.equal({ transaction: null });
+      });
+    });
   });
   });
 });
 });