Add field support to upsert

Jan Aagaard Meier
Commit ff8a672f authored Feb 09, 2015 by Jan Aagaard Meier
Showing with 99 additions and 46 deletions
changelog.md
lib/instance-validator.js
lib/instance.js
lib/model.js
lib/utils.js
test/integration/model/upsert.test.js
--- a/changelog.md
+++ b/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`

--- a/lib/instance-validator.js
+++ b/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: []

--- a/lib/instance.js
+++ b/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) {
+            identifier = Utils.mapValueFieldNames(identifier, Object.keys(identifier), this.Model);
-              // 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];
-              }
-            }
-          }
-          if (identifier === null && self.__options.whereCollection !== null) {
+          } else if (identifier === null && self.__options.whereCollection !== null) {
            identifier = self.__options.whereCollection;
          }

--- a/lib/model.js
+++ b/lib/model.js
@@ -1207,13 +1207,11 @@ module.exports = (function() {
    var createdAtAttr = this._timestampAttributes.createdAt
      , updatedAtAttr = this._timestampAttributes.updatedAt
      , hadPrimary = this.primaryKeyField in values
-      , instance;
+      , instance = this.build(values);
-    values = Utils._.pick(values, options.fields);
-    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

--- a/lib/utils.js
+++ b/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

--- a/test/integration/model/upsert.test.js
+++ b/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');
+        });
+      });
    });
  }
 });