Merge pull request #3080 from janmeier/upsertFixes

Add field support to upsert

changelog.md

@@ -2,6 +2,7 @@
 - [BUG] Fixed `field` support for `increment` and `decrement`.
 - [FEATURE/BUG] Raw queries always return all results (including affected rows etc). This means you should change all promise listeners on `sequelize.query` to use `.spread` instead of `.then`, unless you are passing a query type.
 - [BUG] Support for composite primary keys in upsert [#3065](https://github.com/sequelize/sequelize/pull/3065)
+- [BUG] Support for `field` in upsert
 
 #### Backwards compatibility changes
 - The default query type for `sequelize.query` is now `RAW` - this means that two arguments (results and metadata) will be returned by default and you should use `.spread`

lib/instance-validator.js

@@ -99,6 +99,10 @@ function extendModelValidations(modelInstance) {
 var InstanceValidator = module.exports = function(modelInstance, options) {
   options = options || {};
 
+  if (options.fields && !options.skip) {
+    options.skip = Utils._.difference(Object.keys(modelInstance.Model.attributes), options.fields);
+  }
+
   // assign defined and default options
   this.options = Utils._.defaults(options, {
     skip: []

lib/instance.js

@@ -506,8 +506,6 @@ module.exports = (function() {
     return Promise.bind(this).then(function() {
       // Validate
       if (options.validate) {
-        options.skip = _.difference(Object.keys(this.Model.rawAttributes), options.fields);
-
         return Promise.bind(this).then(function () {
           // hookValidate rejects with errors, validate returns with errors
           if (options.hooks) return this.hookValidate(options);
@@ -515,8 +513,6 @@ module.exports = (function() {
           return this.validate(options).then(function (err) {
             if (err) throw err;
           });
-        }).then(function() {
-          delete options.skip;
         });
       }
     }).then(function() {
@@ -568,22 +564,10 @@ module.exports = (function() {
       }).then(function() {
         if (!options.fields.length) return this;
 
-        var values = {}
+        var values = Utils.mapValueFieldNames(this.dataValues, options.fields, this.Model)
           , query = null
           , args = [];
 
-        options.fields.forEach(function(attr) {
-          if (this.dataValues[attr] !== undefined && !this.Model._isVirtualAttribute(attr)) {
-            values[attr] = this.dataValues[attr];
-          }
-
-          // Field name mapping
-          if (this.Model.rawAttributes[attr] && this.Model.rawAttributes[attr].field && this.Model.rawAttributes[attr].field !== attr) {
-            values[this.Model.rawAttributes[attr].field] = values[attr];
-            delete values[attr];
-          }
-        }, this);
-
         if (updatedAtAttr && !options.silent) {
           self.dataValues[updatedAtAttr] = values[self.Model.rawAttributes[updatedAtAttr].field || updatedAtAttr] = self.Model.__getTimestamp(updatedAtAttr);
         }
@@ -592,18 +576,6 @@ module.exports = (function() {
           self.dataValues[createdAtAttr] = values[self.Model.rawAttributes[createdAtAttr].field || createdAtAttr] = self.Model.__getTimestamp(createdAtAttr);
         }
 
-        options.fields.forEach(function(attr) {
-          if (this.dataValues[attr] !== undefined && !this.Model._isVirtualAttribute(attr)) {
-            values[attr] = this.dataValues[attr];
-          }
-
-          // Field name mapping
-          if (this.Model.rawAttributes[attr] && this.Model.rawAttributes[attr].field && this.Model.rawAttributes[attr].field !== attr) {
-            values[this.Model.rawAttributes[attr].field] = values[attr];
-            delete values[attr];
-          }
-        }, this);
-
         if (self.isNewRecord) {
           query = 'insert';
           args = [self, self.Model.getTableName(options), values, options];
@@ -611,17 +583,9 @@ module.exports = (function() {
           var identifier = self.primaryKeyValues;
 
           if (identifier) {
-            for (var attrName in identifier) {
-              // Field name mapping
-              var rawAttribute = self.Model.rawAttributes[attrName];
-              if (rawAttribute.field && rawAttribute.field !== rawAttribute.fieldName) {
-                identifier[self.Model.rawAttributes[attrName].field] = identifier[attrName];
-                delete identifier[attrName];
-              }
-            }
-          }
+            identifier = Utils.mapValueFieldNames(identifier, Object.keys(identifier), this.Model);
 
-          if (identifier === null && self.__options.whereCollection !== null) {
+          } else if (identifier === null && self.__options.whereCollection !== null) {
             identifier = self.__options.whereCollection;
           }
 

lib/model.js

@@ -716,7 +716,7 @@ module.exports = (function() {
       if (options.attributes === undefined) {
         options.attributes = Object.keys(this.tableAttributes);
       }
-      mapFieldNames.call(this, options, this);
+      Utils.mapOptionFieldNames(options, this);
 
       options = paranoidClause(this, options);
 
@@ -838,7 +838,7 @@ module.exports = (function() {
       validateIncludedElements.call(this, options);
     }
 
-    mapFieldNames.call(this, options, this);
+    Utils.mapOptionFieldNames(options, this);
 
     options.dataType = DataTypes.INTEGER;
     options.includeIgnoreAttributes = false;
@@ -1207,13 +1207,11 @@ module.exports = (function() {
     var createdAtAttr = this._timestampAttributes.createdAt
       , updatedAtAttr = this._timestampAttributes.updatedAt
       , hadPrimary = this.primaryKeyField in values
-      , instance;
-
-    values = Utils._.pick(values, options.fields);
-    instance = this.build(values);
+      , instance = this.build(values);
 
     return instance.hookValidate(options).bind(this).then(function () {
-      values = instance.get(); // Get default values - this also adds a null value for the PK if none is given
+      // Map field names
+      values = Utils.mapValueFieldNames(instance.dataValues, options.fields, this);
 
       if (createdAtAttr && !values[createdAtAttr]) {
         values[createdAtAttr] = this.__getTimestamp(createdAtAttr);
@@ -1328,8 +1326,6 @@ module.exports = (function() {
 
       // Validate
       if (options.validate) {
-        options.skip = Utils._.difference(Object.keys(self.attributes), options.fields);
-
         var errors = [];
         return Promise.map(instances, function(instance) {
           // hookValidate rejects with errors, validate returns with errors
@@ -1446,7 +1442,7 @@ module.exports = (function() {
 
     options.type = QueryTypes.BULKDELETE;
 
-    mapFieldNames.call(this, options, this);
+    Utils.mapOptionFieldNames(options, this);
 
     return Promise.try(function() {
       // Run before hook
@@ -1517,7 +1513,7 @@ module.exports = (function() {
     var self = this
       , instances;
 
-    mapFieldNames.call(this, options, this);
+    Utils.mapOptionFieldNames(options, this);
 
     return Promise.try(function() {
       // Run before hook
@@ -1701,7 +1697,7 @@ module.exports = (function() {
         }
       });
 
-      mapFieldNames.call(self, options, self);
+      Utils.mapOptionFieldNames(options, self);
 
       // Run query to update all rows
       return self.QueryInterface.bulkUpdate(self.getTableName(options), valuesUse, options.where, options, self.tableAttributes).then(function(affectedRows) {
@@ -1751,46 +1747,6 @@ module.exports = (function() {
     }
   };
 
-  var mapFieldNames = function(options, Model) {
-    if (options.attributes) {
-      options.attributes = options.attributes.map(function(attr) {
-        // Object lookups will force any variable to strings, we don't want that for special objects etc
-        if (typeof attr !== "string") return attr;
-        // Map attributes to aliased syntax attributes
-        if (Model.rawAttributes[attr] && attr !== Model.rawAttributes[attr].field) {
-          return [Model.rawAttributes[attr].field, attr];
-        }
-        return attr;
-      });
-    }
-
-    if (options.where) {
-      var attributes = options.where
-        , attribute
-        , rawAttribute;
-
-      if (options.where instanceof Utils.and || options.where instanceof Utils.or) {
-        attributes = undefined;
-        options.where.args = options.where.args.map(function (where) {
-          return mapFieldNames({
-            where: where
-          }, Model).where;
-        });
-      }
-
-      if (attributes) {
-        for (attribute in attributes) {
-          rawAttribute = Model.rawAttributes[attribute];
-          if (rawAttribute && rawAttribute.field !== rawAttribute.fieldName) {
-            attributes[rawAttribute.field] = attributes[attribute];
-            delete attributes[attribute];
-          }
-        }
-      }
-    }
-    return options;
-  };
-
   // private
 
   // validateIncludedElements should have been called before this method
@@ -2002,7 +1958,7 @@ module.exports = (function() {
       include.attributes = Object.keys(include.model.tableAttributes);
     }
 
-    include = mapFieldNames(include, include.model);
+    include = Utils.mapOptionFieldNames(include, include.model);
 
     // pseudo include just needed the attribute logic, return
     if (include._pseudo) {

lib/utils.js

@@ -165,6 +165,66 @@ var Utils = module.exports = {
       return fn ? fn(elem) : undefined;
     });
   },
+
+  /* Used to map field names in attributes and where conditions */
+  mapOptionFieldNames: function(options, Model) {
+    if (options.attributes) {
+      options.attributes = options.attributes.map(function(attr) {
+        // Object lookups will force any variable to strings, we don't want that for special objects etc
+        if (typeof attr !== "string") return attr;
+        // Map attributes to aliased syntax attributes
+        if (Model.rawAttributes[attr] && attr !== Model.rawAttributes[attr].field) {
+          return [Model.rawAttributes[attr].field, attr];
+        }
+        return attr;
+      });
+    }
+
+    if (options.where) {
+      var attributes = options.where
+        , attribute
+        , rawAttribute;
+
+      if (options.where instanceof Utils.and || options.where instanceof Utils.or) {
+        attributes = undefined;
+        options.where.args = options.where.args.map(function (where) {
+          return Utils.mapOptionFieldNames({
+            where: where
+          }, Model).where;
+        });
+      }
+
+      if (attributes) {
+        for (attribute in attributes) {
+          rawAttribute = Model.rawAttributes[attribute];
+          if (rawAttribute && rawAttribute.field !== rawAttribute.fieldName) {
+            attributes[rawAttribute.field] = attributes[attribute];
+            delete attributes[attribute];
+          }
+        }
+      }
+    }
+    return options;
+  },
+
+  /* Used to map field names in values */
+  mapValueFieldNames: function (dataValues, fields, Model) {
+    var values = {};
+
+    fields.forEach(function(attr) {
+      if (dataValues[attr] !== undefined && !Model._isVirtualAttribute(attr)) {
+        values[attr] = dataValues[attr];
+      }
+
+      // Field name mapping
+      if (Model.rawAttributes[attr] && Model.rawAttributes[attr].field && Model.rawAttributes[attr].field !== attr) {
+        values[Model.rawAttributes[attr].field] = values[attr];
+        delete values[attr];
+      }
+    });
+
+    return values;
+  },
   // smartWhere can accept an array of {where} objects, or a single {where} object.
   // The smartWhere function breaks down the collection of where objects into a more
   // centralized object for each column so we can avoid duplicates

test/integration/model/upsert.test.js

@@ -28,6 +28,10 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         unique: 'foobar',
         type: DataTypes.INTEGER
       },
+      baz: {
+        type: DataTypes.STRING,
+        field: 'zab'
+      },
       blob: DataTypes.BLOB
     });
 
@@ -181,6 +185,30 @@ describe(Support.getTestDialectTeaser('Model'), function() {
           expect(user.updatedAt).to.be.afterTime(user.createdAt);
         });
       });
+
+      it('works with .field', function () {
+        return this.User.upsert({ id: 42, baz: 'foo' }).bind(this).then(function(created) {
+          if (dialect === 'sqlite') {
+            expect(created).not.to.be.defined;
+          } else {
+            expect(created).to.be.ok;
+          }
+
+          return this.sequelize.Promise.delay(1000).bind(this).then(function() {
+            return this.User.upsert({ id: 42, baz: 'oof' });
+          });
+        }).then(function(created) {
+          if (dialect === 'sqlite') {
+            expect(created).not.to.be.defined;
+          } else {
+            expect(created).not.to.be.ok;
+          }
+
+          return this.User.find(42);
+        }).then(function(user) {
+          expect(user.baz).to.equal('oof');
+        });
+      });
     });
   }
 });