Merge pull request #3994 from FredKSchott/upsert-no-defaults

upsert overwrites data with default values on update

lib/model.js

@@ -1706,24 +1706,27 @@ Model.prototype.upsert = function (values, options) {
 
 
   return instance.hookValidate(options).bind(this).then(function () {
   return instance.hookValidate(options).bind(this).then(function () {
     // Map field names
     // Map field names
-    values = Utils.mapValueFieldNames(instance.dataValues, options.fields, this);
+    var updatedDataValues = _.pick(instance.dataValues, Object.keys(instance._changed))
+      , insertValues = Utils.mapValueFieldNames(instance.dataValues, options.fields, this)
+      , updateValues = Utils.mapValueFieldNames(updatedDataValues, options.fields, this)
+      , now = Utils.now(this.sequelize.options.dialect);
 
 
-    var now = Utils.now(this.sequelize.options.dialect);
-
-    if (createdAtAttr && !values[createdAtAttr]) {
-      values[createdAtAttr] = this.$getDefaultTimestamp(createdAtAttr) || now;
+    // Attach createdAt
+    if (createdAtAttr && !updateValues[createdAtAttr]) {
+      insertValues[createdAtAttr] = this.$getDefaultTimestamp(createdAtAttr) || now;
     }
     }
-    if (updatedAtAttr && !values[updatedAtAttr]) {
-      values[updatedAtAttr] = this.$getDefaultTimestamp(updatedAtAttr) || now;
+    if (updatedAtAttr && !insertValues[updatedAtAttr]) {
+      insertValues[updatedAtAttr] = updateValues[updatedAtAttr] = this.$getDefaultTimestamp(updatedAtAttr) || now;
     }
     }
 
 
     // Build adds a null value for the primary key, if none was given by the user.
     // Build adds a null value for the primary key, if none was given by the user.
     // We need to remove that because of some Postgres technicalities.
     // We need to remove that because of some Postgres technicalities.
     if (!hadPrimary && this.primaryKeyAttribute && !this.rawAttributes[this.primaryKeyAttribute].defaultValue) {
     if (!hadPrimary && this.primaryKeyAttribute && !this.rawAttributes[this.primaryKeyAttribute].defaultValue) {
-      delete values[this.primaryKeyField];
+      delete insertValues[this.primaryKeyField];
+      delete updateValues[this.primaryKeyField];
     }
     }
 
 
-    return this.QueryInterface.upsert(this.getTableName(options), values, this, options);
+    return this.QueryInterface.upsert(this.getTableName(options), insertValues, updateValues, this, options);
   });
   });
 };
 };
 
 

lib/query-interface.js

@@ -510,12 +510,11 @@ QueryInterface.prototype.insert = function(instance, tableName, values, options)
   });
   });
 };
 };
 
 
-QueryInterface.prototype.upsert = function(tableName, values, model, options) {
+QueryInterface.prototype.upsert = function(tableName, values, updateValues, model, options) {
   var wheres = []
   var wheres = []
     , where
     , where
     , indexFields
     , indexFields
     , indexes = []
     , indexes = []
-    , updateValues
     , attributes = Object.keys(values);
     , attributes = Object.keys(values);
 
 
   where = {};
   where = {};
@@ -563,16 +562,6 @@ QueryInterface.prototype.upsert = function(tableName, values, model, options) {
   options.type = QueryTypes.UPSERT;
   options.type = QueryTypes.UPSERT;
   options.raw = true;
   options.raw = true;
 
 
-
-  if (model._timestampAttributes.createdAt) {
-    // If we are updating an existing row, we shouldn't set createdAt
-    updateValues = Utils.cloneDeep(values);
-
-    delete updateValues[model._timestampAttributes.createdAt];
-  } else {
-    updateValues = values;
-  }
-
   var sql = this.QueryGenerator.upsertQuery(tableName, values, updateValues, where, model.rawAttributes, options);
   var sql = this.QueryGenerator.upsertQuery(tableName, values, updateValues, where, model.rawAttributes, options);
   return this.sequelize.query(sql, options).then(function (rowCount) {
   return this.sequelize.query(sql, options).then(function (rowCount) {
     if (rowCount === undefined) {
     if (rowCount === undefined) {

test/integration/model/upsert.test.js

@@ -2,6 +2,7 @@
 
 
 /* jshint -W030 */
 /* jshint -W030 */
 var chai = require('chai')
 var chai = require('chai')
+  , sinon = require('sinon')
   , Sequelize = require('../../../index')
   , Sequelize = require('../../../index')
   , Promise = Sequelize.Promise
   , Promise = Sequelize.Promise
   , expect = chai.expect
   , expect = chai.expect
@@ -24,7 +25,8 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       },
       },
       baz: {
       baz: {
         type: DataTypes.STRING,
         type: DataTypes.STRING,
-        field: 'zab'
+        field: 'zab',
+        defaultValue: 'BAZ_DEFAULT_VALUE'
       },
       },
       blob: DataTypes.BLOB
       blob: DataTypes.BLOB
     });
     });
@@ -248,6 +250,45 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         });
         });
       });
       });
 
 
+      it('does not overwrite createdAt time on update', function() {
+        var originalCreatedAt;
+        var originalUpdatedAt;
+        var clock = sinon.useFakeTimers();
+        return this.User.create({ id: 42, username: 'john'}).bind(this).then(function() {
+          return this.User.findById(42);
+        }).then(function(user) {
+          originalCreatedAt = user.createdAt;
+          originalUpdatedAt = user.updatedAt;
+          clock.tick(5000);
+          return this.User.upsert({ id: 42, username: 'doe'});
+        }).then(function() {
+          return this.User.findById(42);
+        }).then(function(user) {
+          expect(user.updatedAt).to.be.gt(originalUpdatedAt);
+          expect(user.createdAt).to.deep.equal(originalCreatedAt);
+          clock.restore();
+        });
+      });
+
+      it('does not update using default values', function() {
+        return this.User.create({ id: 42, username: 'john', baz: 'new baz value'}).bind(this).then(function() {
+          return this.User.findById(42);
+        }).then(function(user) {
+          // 'username' should be 'john' since it was set
+          expect(user.username).to.equal('john');
+          // 'baz' should be 'new baz value' since it was set
+          expect(user.baz).to.equal('new baz value');
+          return this.User.upsert({ id: 42, username: 'doe'});
+        }).then(function() {
+          return this.User.findById(42);
+        }).then(function(user) {
+          // 'username' was updated
+          expect(user.username).to.equal('doe');
+          // 'baz' should still be 'new baz value' since it was not updated
+          expect(user.baz).to.equal('new baz value');
+        });
+      });
+
     });
     });
   }
   }
 });
 });