Clone options object within all Sequelize methods (#5874)

* Clone options

* Remove whitespace

changelog.md

 # Future
 - [FIXED] Type validation now works with non-strings due to updated validator@5.0.0 [#5861](https://github.com/sequelize/sequelize/pull/5861)
 - [FIXED] Improved offset and limit support for SQL server 2008 [#5616](https://github.com/sequelize/sequelize/pull/5616)
+- [FIXED] options object cloned in all Sequelize methods (so not modified within Sequelize)
 
 # 3.23.1
 - [FIXED] Postgres DECIMAL precision. (PostgreSQL) [#4893](https://github.com/sequelize/sequelize/issues/4893)

lib/associations/belongs-to-many.js

@@ -432,7 +432,7 @@ BelongsToMany.prototype.injectGetter = function(obj) {
   var association = this;
 
   obj[this.accessors.get] = function(options) {
-    options = association.target.__optClone(options) || {};
+    options = Utils.cloneDeep(options) || {};
 
     var instance = this
       , through = association.through
@@ -494,7 +494,7 @@ BelongsToMany.prototype.injectGetter = function(obj) {
     var model = association.target
       , sequelize = model.sequelize;
 
-    options = association.target.__optClone(options) || {};
+    options = Utils.cloneDeep(options);
     options.attributes = [
       [sequelize.fn('COUNT', sequelize.col([association.target.name, model.primaryKeyAttribute].join('.'))), 'count']
     ];
@@ -514,11 +514,10 @@ BelongsToMany.prototype.injectGetter = function(obj) {
       instances = [instances];
     }
 
-    options = options || {};
-    options.scope = false;
-
-    _.defaults(options, {
+    options = _.assign({
       raw: true
+    }, options, {
+      scope: false
     });
 
     where.$or = instances.map(function (instance) {
@@ -648,7 +647,7 @@ BelongsToMany.prototype.injectSetter = function(obj) {
     // If newInstances is null or undefined, no-op
     if (!newInstances) return Utils.Promise.resolve();
 
-    additionalAttributes = additionalAttributes || {};
+    additionalAttributes = _.clone(additionalAttributes) || {};
 
     var instance = this
       , defaultAttributes = _.omit(additionalAttributes, ['transaction', 'hooks', 'individualHooks', 'ignoreDuplicates', 'validate', 'fields', 'logging'])

lib/associations/belongs-to.js

@@ -150,7 +150,7 @@ BelongsTo.prototype.get = function(instances, options) {
     , instance
     , where = {};
 
-  options = association.target.$optClone(options) || {};
+  options = Utils.cloneDeep(options);
 
   if (options.hasOwnProperty('scope')) {
     if (!options.scope) {

lib/associations/has-many.js

@@ -272,7 +272,7 @@ HasMany.prototype.get = function(instances, options) {
     instances = undefined;
   }
 
-  options = association.target.$optClone(options) || {};
+  options = Utils.cloneDeep(options) || {};
 
   if (association.scope) {
     _.assign(where, association.scope);
@@ -340,7 +340,7 @@ HasMany.prototype.count = function(instance, options) {
     , model = association.target
     , sequelize = model.sequelize;
 
-  options = association.target.__optClone(options) || {};
+  options = Utils.cloneDeep(options);
   options.attributes = [
     [sequelize.fn('COUNT', sequelize.col(model.primaryKeyField)), 'count']
   ];
@@ -360,9 +360,10 @@ HasMany.prototype.has = function(sourceInstance, targetInstances, options) {
     targetInstances = [targetInstances];
   }
 
-  options = options || {};
-  options.scope = false;
-  options.raw = true;
+  options = _.assign({}, options, {
+    scope: false,
+    raw: true
+  });
 
   where.$or = targetInstances.map(function (instance) {
     if (instance instanceof association.target.Instance) {

lib/associations/has-one.js

@@ -148,7 +148,7 @@ HasOne.prototype.get = function(instances, options) {
     , instance
     , where = {};
 
-  options = association.target.$optClone(options) || {};
+  options = Utils.cloneDeep(options);
 
   if (options.hasOwnProperty('scope')) {
     if (!options.scope) {
@@ -209,8 +209,9 @@ HasOne.prototype.injectSetter = function(instancePrototype) {
     var instance = this,
       alreadyAssociated;
 
-    options = options || {};
-    options.scope = false;
+    options = _.assign({}, options, {
+      scope: false
+    });
     return instance[association.accessors.get](options).then(function(oldInstance) {
       // TODO Use equals method once #5605 is resolved
       alreadyAssociated = oldInstance && associatedInstance && _.every(association.target.primaryKeyAttributes, function(attribute) {

lib/instance-validator.js

@@ -16,7 +16,7 @@ var validator = require('./utils/validator-extras').validator
  * @constructor
  */
 var InstanceValidator = module.exports = function(modelInstance, options) {
-  options = options || {};
+  options = _.clone(options) || {};
 
   if (options.fields && !options.skip) {
     options.skip = Utils._.difference(Object.keys(modelInstance.Model.attributes), options.fields);

lib/instance.js

@@ -495,7 +495,7 @@ Instance.prototype.save = function(options) {
     throw new Error('The second argument was removed in favor of the options object.');
   }
 
-  options = this.$Model.$optClone(options || {});
+  options = Utils.cloneDeep(options);
   options = _.defaults(options, {
     hooks: true,
     validate: true
@@ -634,7 +634,7 @@ Instance.prototype.save = function(options) {
         var instance = self.get(include.as);
         if (!instance) return Promise.resolve();
 
-        var includeOptions =  _(self.$Model.$optClone(include))
+        var includeOptions =  _(Utils.cloneDeep(include))
           .omit(['association'])
           .defaults({
             transaction: options.transaction,
@@ -712,7 +712,7 @@ Instance.prototype.save = function(options) {
             if (!Array.isArray(instances)) instances = [instances];
             if (!instances.length) return Promise.resolve();
 
-            var includeOptions =  _(self.$Model.$optClone(include))
+            var includeOptions =  _(Utils.cloneDeep(include))
               .omit(['association'])
               .defaults({
                 transaction: options.transaction,
@@ -815,8 +815,8 @@ Instance.prototype.update = function(values, options) {
   options = options || {};
   if (Array.isArray(options)) options = {fields: options};
 
-  options = this.$Model.$optClone(options);
-  setOptions = this.$Model.$optClone(options);
+  options = Utils.cloneDeep(options);
+  setOptions = Utils.cloneDeep(options);
   setOptions.attributes = options.fields;
   this.set(values, setOptions);
 

lib/model.js

@@ -8,7 +8,6 @@ var Utils = require('./utils')
   , HasMany = require('./associations/has-many')
   , DataTypes = require('./data-types')
   , Util = require('util')
-  , Transaction = require('./transaction')
   , Promise = require('./promise')
   , QueryTypes = require('./query-types')
   , Hooks = require('./hooks')
@@ -307,27 +306,6 @@ function conformInclude(include, self) {
 
 Model.$conformInclude = conformInclude;
 
-var optClone = Model.prototype.__optClone = Model.prototype.$optClone = function(options) {
-  options = options || {};
-  return Utils.cloneDeep(options, function(elem) {
-    // The InstanceFactories used for include are pass by ref, so don't clone them.
-    if (elem && typeof elem === 'object' &&
-      (
-        elem._isSequelizeMethod ||
-        elem instanceof Model ||
-        elem instanceof Instance ||
-        elem instanceof Transaction ||
-        elem instanceof Association
-      )
-    ) {
-      return elem;
-    }
-
-    // Otherwise return undefined, meaning, 'handle this lodash'
-    return undefined;
-  });
-};
-
 var expandIncludeAllElement = function(includes, include) {
   // check 'all' attribute provided is valid
   var all = include.all;
@@ -413,7 +391,7 @@ var expandIncludeAllElement = function(includes, include) {
       used.push(parent);
 
       // include this model
-      var thisInclude = optClone(include);
+      var thisInclude = Utils.cloneDeep(include);
       thisInclude.model = model;
       if (as) {
         thisInclude.as = as;
@@ -990,9 +968,8 @@ Model.prototype.removeAttribute = function(attribute) {
  * @return {Promise<this>}
  */
 Model.prototype.sync = function(options) {
-  options = options || {};
+  options = _.extend({}, this.options, options);
   options.hooks = options.hooks === undefined ? true : !!options.hooks;
-  options = Utils._.extend({}, this.options, options);
 
   var self = this
     , attributes = this.tableAttributes;
@@ -1360,7 +1337,7 @@ Model.prototype.findAll = function(options) {
     , originalOptions;
 
   tableNames[this.getTableName(options)] = true;
-  options = optClone(options);
+  options = Utils.cloneDeep(options);
 
   _.defaults(options, { hooks: true, rejectOnEmpty: this.options.rejectOnEmpty });
 
@@ -1408,7 +1385,7 @@ Model.prototype.findAll = function(options) {
       return this.runHooks('beforeFindAfterOptions', options);
     }
   }).then(function() {
-    originalOptions = optClone(options);
+    originalOptions = Utils.cloneDeep(options);
     options.tableNames = Object.keys(tableNames);
     return this.QueryInterface.select(this, this.getTableName(options), options);
   }).tap(function(results) {
@@ -1498,7 +1475,7 @@ Model.prototype.findById = function(param, options) {
     return Promise.resolve(null);
   }
 
-  options = optClone(options) || {};
+  options = Utils.cloneDeep(options) || {};
 
   if (typeof param === 'number' || typeof param === 'string' || Buffer.isBuffer(param)) {
     options.where = {};
@@ -1527,7 +1504,7 @@ Model.prototype.findOne = function(options) {
   if (options !== undefined && !_.isPlainObject(options)) {
     throw new Error('The argument passed to findOne must be an options object, use findById if you wish to pass a single primary key value');
   }
-  options = optClone(options);
+  options = Utils.cloneDeep(options);
 
   if (options.limit === undefined) {
     var pkVal = options.where && options.where[this.primaryKeyAttribute];
@@ -1563,7 +1540,8 @@ Model.prototype.find = Model.prototype.findOne;
  * @return {Promise<options.dataType|object>}                Returns the aggregate result cast to `options.dataType`, unless `options.plain` is false, in which case the complete data result is returned.
  */
 Model.prototype.aggregate = function(attribute, aggregateFunction, options) {
-  options = Utils._.extend({ attributes: [] }, options || {});
+  options = Utils.cloneDeep(options);
+  options = _.defaults(options, { attributes: [] });
   conformOptions(options, this);
   this.$injectScope(options);
 
@@ -1617,8 +1595,7 @@ Model.prototype.aggregate = function(attribute, aggregateFunction, options) {
  * @return {Promise<Integer>}
  */
 Model.prototype.count = function(options) {
-  options = Utils._.clone(options || {});
-
+  options = Utils.cloneDeep(options);
   _.defaults(options, { hooks: true });
 
   var col = '*';
@@ -1862,7 +1839,7 @@ Model.prototype.bulkBuild = function(valueSets, options) { // testhint options:n
  * @return {Promise<Instance>}
  */
 Model.prototype.create = function(values, options) {
-  options = optClone(options || {});
+  options = Utils.cloneDeep(options || {});
 
   return this.build(values, {
     isNewRecord: true,
@@ -1937,6 +1914,8 @@ Model.prototype.findOrCreate = function(options) {
     );
   }
 
+  options = _.assign({}, options);
+
   if (options.transaction === undefined && this.sequelize.constructor.cls) {
     var t = this.sequelize.constructor.cls.get('transaction');
     if (t) {
@@ -2065,7 +2044,7 @@ Model.prototype.findCreateFind = function(options) {
  * @return {Promise<created>} Returns a boolean indicating whether the row was created or updated.
  */
 Model.prototype.upsert = function (values, options) {
-  options = optClone(options) || {};
+  options = Utils.cloneDeep(options) || {};
 
   if (!options.fields) {
     options.fields = Object.keys(this.attributes);
@@ -2281,7 +2260,7 @@ Model.prototype.bulkCreate = function(records, options) {
  * @see {Model#destroy} for more information
  */
 Model.prototype.truncate = function(options) {
-  options = optClone(options) || {};
+  options = Utils.cloneDeep(options) || {};
   options.truncate = true;
   return this.destroy(options);
 };
@@ -2314,12 +2293,13 @@ Model.prototype.destroy = function(options) {
     throw new Error('Expected plain object, array or sequelize method in the options.where parameter of model.destroy.');
   }
 
-  options = Utils._.extend({
+  options = Utils.cloneDeep(options);
+  options = _.defaults(options, {
     hooks: true,
     individualHooks: false,
     force: false,
     cascade: false
-  }, options || {});
+  });
 
   options.type = QueryTypes.BULKDELETE;
   this.$injectScope(options);
@@ -2479,14 +2459,15 @@ Model.prototype.update = function(values, options) {
     throw new Error('Expected plain object, array or sequelize method in the options.where parameter of model.update.');
   }
 
-  options = Utils._.extend({
+  options = Utils.cloneDeep(options);
+  options = _.defaults(options, {
     validate: true,
     hooks: true,
     individualHooks: false,
     returning: false,
     force: false,
     sideEffects: true
-  }, options || {});
+  });
 
   options.type = QueryTypes.BULKUPDATE;
 
@@ -2699,7 +2680,7 @@ Model.prototype.$expandAttributes = function (options) {
 // Inject $scope into options. Includes should have been conformed (conformOptions) before calling this
 Model.prototype.$injectScope = function (options) {
   var self = this;
-  var scope = optClone(this.$scope);
+  var scope = Utils.cloneDeep(this.$scope);
 
   var filteredScope = _.omit(scope, 'include'); // Includes need special treatment
 

lib/query-interface.js

@@ -49,8 +49,7 @@ QueryInterface.prototype.showAllSchemas = function(options) {
 
   options = _.assign({}, options, {
     raw: true,
-    type: this.sequelize.QueryTypes.SELECT,
-    logging: false
+    type: this.sequelize.QueryTypes.SELECT
   });
 
   var showSchemasSql = self.QueryGenerator.showSchemasQuery();
@@ -78,7 +77,7 @@ QueryInterface.prototype.createTable = function(tableName, attributes, options,
     , sql = ''
     , i = 0;
 
-  options = options || {};
+  options = _.clone(options) || {};
 
   attributes = Utils._.mapValues(attributes, function(attribute) {
     if (!Utils._.isPlainObject(attribute)) {
@@ -123,18 +122,19 @@ QueryInterface.prototype.createTable = function(tableName, attributes, options,
 
             vals.forEach(function(value, idx) {
               // reset out after/before options since it's for every enum value
-              options.before = null;
-              options.after = null;
+              var valueOptions = _.clone(options);
+              valueOptions.before = null;
+              valueOptions.after = null;
 
               if (enumVals.indexOf(value) === -1) {
                 if (!!vals[idx + 1]) {
-                  options.before = vals[idx + 1];
+                  valueOptions.before = vals[idx + 1];
                 }
                 else if (!!vals[idx - 1]) {
-                  options.after = vals[idx - 1];
+                  valueOptions.after = vals[idx - 1];
                 }
-                options.supportsSearchPath = false;
-                promises.push(self.sequelize.query(self.QueryGenerator.pgEnumAdd(tableName, keys[i], value, options), options));
+                valueOptions.supportsSearchPath = false;
+                promises.push(self.sequelize.query(self.QueryGenerator.pgEnumAdd(tableName, keys[i], value, valueOptions), valueOptions));
               }
             });
             enumIdx++;
@@ -179,7 +179,7 @@ QueryInterface.prototype.createTable = function(tableName, attributes, options,
 
 QueryInterface.prototype.dropTable = function(tableName, options) {
   // if we're forcing we should be cascading unless explicitly stated otherwise
-  options = options || {};
+  options = _.clone(options) || {};
   options.cascade = options.cascade || options.force || false;
 
   var sql = this.QueryGenerator.dropTableQuery(tableName, options)
@@ -278,7 +278,7 @@ QueryInterface.prototype.dropAllEnums = function(options) {
 
   var self = this;
 
-  return this.pgListEnums(options).map(function(result) {
+  return this.pgListEnums(null, options).map(function(result) {
     return self.sequelize.query(
       self.QueryGenerator.pgEnumDrop(null, null, self.QueryGenerator.pgEscapeAndQuote(result.enum_name)),
       _.assign({}, options, { raw: true })
@@ -443,7 +443,7 @@ QueryInterface.prototype.addIndex = function(tableName, attributes, options, raw
     rawTablename = tableName;
   }
 
-  options = options || {};
+  options = Utils.cloneDeep(options);
   options.fields = attributes;
   var sql = this.QueryGenerator.addIndexQuery(tableName, options, rawTablename);
   return this.sequelize.query(sql, _.assign({}, options, { supportsSearchPath: false }));
@@ -492,7 +492,7 @@ QueryInterface.prototype.removeIndex = function(tableName, indexNameOrAttributes
 };
 
 QueryInterface.prototype.insert = function(instance, tableName, values, options) {
-  options = options || {};
+  options = Utils.cloneDeep(options);
   options.hasTrigger = instance && instance.Model.options.hasTrigger;
   var sql = this.QueryGenerator.insertQuery(tableName, values, instance && instance.Model.rawAttributes, options);
 
@@ -511,6 +511,8 @@ QueryInterface.prototype.upsert = function(tableName, valuesByField, updateValue
     , indexes = []
     , attributes = Object.keys(valuesByField);
 
+  options = _.clone(options);
+
   if (!Utils._.isEmpty(where)) {
     wheres.push(where);
   }
@@ -561,14 +563,14 @@ QueryInterface.prototype.upsert = function(tableName, valuesByField, updateValue
 };
 
 QueryInterface.prototype.bulkInsert = function(tableName, records, options, attributes) {
-  options = options || {};
+  options = _.clone(options) || {};
   options.type = QueryTypes.INSERT;
   var sql = this.QueryGenerator.bulkInsertQuery(tableName, records, options, attributes);
   return this.sequelize.query(sql, options);
 };
 
 QueryInterface.prototype.update = function(instance, tableName, values, identifier, options) {
-  options = options || {};
+  options = _.clone(options || {});
   options.hasTrigger = !!(instance && instance.$modelOptions && instance.$modelOptions.hasTrigger);
 
 
@@ -596,11 +598,13 @@ QueryInterface.prototype.update = function(instance, tableName, values, identifi
 };
 
 QueryInterface.prototype.bulkUpdate = function(tableName, values, identifier, options, attributes) {
+  options = Utils.cloneDeep(options);
+  if (typeof identifier === 'object') identifier = Utils.cloneDeep(identifier);
+
   var sql = this.QueryGenerator.updateQuery(tableName, values, identifier, options, attributes)
     , table = Utils._.isObject(tableName) ? tableName : { tableName: tableName }
     , model = Utils._.find(this.sequelize.modelManager.models, { tableName: table.tableName });
 
-  options = options || {};
   options.model = model;
   return this.sequelize.query(sql, options);
 };
@@ -610,7 +614,7 @@ QueryInterface.prototype.delete = function(instance, tableName, identifier, opti
     , cascades = []
     , sql = self.QueryGenerator.deleteQuery(tableName, identifier, null, instance.Model);
 
-  options = options || {};
+  options = _.clone(options) || {};
 
   // Check for a restrict field
   if (!!instance.Model && !!instance.Model.associations) {
@@ -648,12 +652,16 @@ QueryInterface.prototype.delete = function(instance, tableName, identifier, opti
 };
 
 QueryInterface.prototype.bulkDelete = function(tableName, identifier, options, model) {
-  var sql = this.QueryGenerator.deleteQuery(tableName, identifier, Utils._.defaults(options || {}, {limit: null}), model);
+  options = Utils.cloneDeep(options);
+  options = _.defaults(options, {limit: null});
+  if (typeof identifier === 'object') identifier = Utils.cloneDeep(identifier);
+
+  var sql = this.QueryGenerator.deleteQuery(tableName, identifier, options, model);
   return this.sequelize.query(sql, options);
 };
 
 QueryInterface.prototype.select = function(model, tableName, options) {
-  options = options || {};
+  options = Utils.cloneDeep(options);
   options.type = QueryTypes.SELECT;
   options.model = model;
 
@@ -666,7 +674,7 @@ QueryInterface.prototype.select = function(model, tableName, options) {
 QueryInterface.prototype.increment = function(instance, tableName, values, identifier, options) {
   var sql = this.QueryGenerator.incrementQuery(tableName, values, identifier, options.attributes);
 
-  options = options || {};
+  options = _.clone(options) || {};
 
   options.type = QueryTypes.UPDATE;
   options.instance = instance;
@@ -681,7 +689,8 @@ QueryInterface.prototype.rawSelect = function(tableName, options, attributeSelec
     });
   }
 
-  options = _.defaults(options || {}, {
+  options = Utils.cloneDeep(options);
+  options = _.defaults(options, {
     raw: true,
     plain: true,
     type: QueryTypes.SELECT

lib/sequelize.js

@@ -739,7 +739,7 @@ Sequelize.prototype.query = function(sql, options) {
 
   var self = this;
 
-  options = options || {};
+  options = _.assign({}, this.options.query, options);
 
   if (options.instance && !options.model) {
     options.model = options.instance.Model;
@@ -750,7 +750,7 @@ Sequelize.prototype.query = function(sql, options) {
     options.fieldMap =  options.model.fieldAttributeMap;
   }
 
-  if (Utils._.isPlainObject(sql)) {
+  if (_.isPlainObject(sql)) {
     if (sql.values !== undefined) {
       if (options.replacements !== undefined) {
         throw new Error('Both `sql.values` and `options.replacements` cannot be set at the same time');
@@ -796,8 +796,7 @@ Sequelize.prototype.query = function(sql, options) {
     bindParameters = bindSql[1];
   }
 
-  options = Utils._.extend(Utils._.clone(this.options.query), options);
-  options = Utils._.defaults(options, {
+  options = _.defaults(options, {
     logging: this.options.hasOwnProperty('logging') ? this.options.logging : console.log,
     searchPath: this.options.hasOwnProperty('searchPath') ? this.options.searchPath : 'DEFAULT',
   });
@@ -973,7 +972,7 @@ Sequelize.prototype.dropAllSchemas = function(options) {
 Sequelize.prototype.sync = function(options) {
   var self = this;
 
-  options = options || {};
+  options = _.clone(options) || {};
   options.hooks = options.hooks === undefined ? true : !!options.hooks;
   options.logging = options.logging === undefined ? false : options.logging;
   options = Utils._.defaults(options, this.options.sync, this.options);

lib/utils.js

@@ -87,20 +87,23 @@ var Utils = module.exports = {
     var timeZone = null;
     return SqlString.formatNamedParameters(sql, parameters, timeZone, dialect);
   },
-  cloneDeep: function(obj, fn) {
+  cloneDeep: function(obj) {
+    obj = obj || {};
     return _.cloneDeepWith(obj, function (elem) {
-      // Do not try to customize cloning of plain objects and strings
+      // Do not try to customize cloning of arrays or POJOs
       if (Array.isArray(elem) || _.isPlainObject(elem)) {
         return undefined;
       }
+
+      // Don't clone stuff that's an object, but not a plain one - fx example sequelize models and instances
+      if (typeof elem === 'object') {
+        return elem;
+      }
+
       // Preserve special data-types like `fn` across clones. _.get() is used for checking up the prototype chain
       if (elem && typeof elem.clone === 'function') {
         return elem.clone();
       }
-      // Unfortunately, lodash.cloneDeep doesn't preserve Buffer.isBuffer, which we have to rely on for binary data
-      if (Buffer.isBuffer(elem)) { return elem; }
-
-      return fn ? fn(elem) : undefined;
     });
   },