Merge pull request #4176 from gutobortolozzo/nothing-change-and-generate-sql

Update sent to database when nothing changed in instance

changelog.md

 # NEXT
 - [FIXED] Fix findOrCreate regression trying to add a transaction even if there is none
 - [FEATURE] Added default validation based on attribute types. [#3472](https://github.com/sequelize/sequelize/pull/3472). The validation _cannot_ be disabled. If you really want to completely disable it, you can remove the `validate` function from the corresponding datatype, but know that this permanently disables the validation.
+- [FIXED] Fix save to be noop when nothing changed
 
 # 3.4.1
 - [FIXED] Fix belongs-to-many `countAssociations` - ambigious id when through model has id

docs/docs/instances.md

@@ -104,6 +104,8 @@ task.updateAttributes({ title: 'foooo', description: 'baaaaaar'}, {fields: ['tit
 })
 ```
 
+When you call `save` without changing any attribute, this method will execute nothing;
+
 ## Destroying / Deleting persistent instances
 
 Once you created an object and got a reference to it, you can delete it from the database. The relevant method is `destroy`:

lib/instance.js

@@ -464,7 +464,7 @@ Instance.prototype._setInclude = function(key, value, options) {
 };
 
 /**
- * Validate this instance, and if the validation passes, persist it to the database.
+ * Validate this instance, and if the validation passes, persist it to the database. It will only save changed fields, and do nothing if no fields have changed.
  *
  * On success, the callback will be called with this instance. On validation error, the callback will be called with an instance of `Sequelize.ValidationError`.
  * This error will have a property for each of the fields for which validation failed, with the error message for that field.
@@ -621,6 +621,7 @@ Instance.prototype.save = function(options) {
     })
     .then(function() {
       if (!options.fields.length) return this;
+      if (!this.changed() && !this.isNewRecord) return this;
 
       var values = Utils.mapValueFieldNames(this.dataValues, options.fields, this.Model)
         , query = null

test/integration/hooks.test.js

@@ -1936,7 +1936,8 @@ describe(Support.getTestDialectTeaser('Hooks'), function() {
 
         return User.sync({ force: true }).then(function() {
           return User.create({ username: 'bob' }).then(function(user) {
-            return user.save({ username: 'bawb' }).then(function() {
+            user.username = 'bawb';
+            return user.save({ fields: ['username'] }).then(function() {
               expect(beforeHooked).to.be.true;
               expect(afterHooked).to.be.true;
             });

test/integration/instance.test.js

@@ -981,6 +981,30 @@ describe(Support.getTestDialectTeaser('Instance'), function() {
       });
     });
 
+    describe('when nothing changed', function() {
+
+      beforeEach(function () {
+        this.clock = sinon.useFakeTimers();
+      });
+
+      afterEach(function () {
+        this.clock.restore();
+      });
+
+      it('does not update timestamps', function() {
+        var self = this;
+        return self.User.create({ username: 'John' }).then(function() {
+          return self.User.findOne({ username: 'John' }).then(function(user) {
+            var updatedAt = user.updatedAt;
+            self.clock.tick(2000);
+            return user.save().then(function(newlySavedUser) {
+              expect(newlySavedUser.updatedAt).to.equalTime(updatedAt);
+            });
+          });
+        });
+      });
+    });
+
     it('updates with function and column value', function() {
       var self = this;