fix(model): destroying paranoid models with custom deletedAt (#11255)

lib/model.js

@@ -4052,29 +4052,19 @@ class Model {
       const where = this.where(true);
 
       if (this.constructor._timestampAttributes.deletedAt && options.force === false) {
-        const attribute = this.constructor.rawAttributes[this.constructor._timestampAttributes.deletedAt];
-        const field = attribute.field || this.constructor._timestampAttributes.deletedAt;
-        const values = Utils.mapValueFieldNames(this.dataValues, this.changed() || [], this.constructor);
-
-        values[field] = new Date();
-        where[field] = Object.prototype.hasOwnProperty.call(attribute, 'defaultValue')
+        const attributeName = this.constructor._timestampAttributes.deletedAt;
+        const attribute = this.constructor.rawAttributes[attributeName];
+        const defaultValue = Object.prototype.hasOwnProperty.call(attribute, 'defaultValue')
           ? attribute.defaultValue
           : null;
-
-        this.setDataValue(field, values[field]);
-
-        return this.constructor.QueryInterface.update(
-          this, this.constructor.getTableName(options), values, where, _.defaults({ hooks: false, model: this.constructor }, options)
-        ).then(([results, rowsUpdated]) => {
-          if (this.constructor._versionAttribute && rowsUpdated < 1) {
-            throw new sequelizeErrors.OptimisticLockError({
-              modelName: this.constructor.name,
-              values,
-              where
-            });
+        const currentValue = this.getDataValue(attributeName);
+        const undefinedOrNull = currentValue == null && defaultValue == null;
+        if (undefinedOrNull || _.isEqual(currentValue, defaultValue)) {
+          // only update timestamp if it wasn't already set
+          this.setDataValue(attributeName, new Date());
         }
-          return results;
-        });
+
+        return this.save(_.defaults({ hooks: false }, options));
       }
       return this.constructor.QueryInterface.delete(this, this.constructor.getTableName(options), where, Object.assign({ type: QueryTypes.DELETE, limit: null }, options));
     }).tap(() => {

test/integration/instance.test.js

@@ -685,5 +685,109 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
         expect(user.username).to.equal('Peter');
       });
     });
+
+    it('supports custom deletedAt field', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: DataTypes.STRING,
+        destroyTime: DataTypes.DATE
+      }, { paranoid: true, deletedAt: 'destroyTime' });
+
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        return user.destroy();
+      }).then(user => {
+        expect(user.destroyTime).to.be.ok;
+        expect(user.deletedAt).to.not.be.ok;
+        return user.restore();
+      }).then(user => {
+        expect(user.destroyTime).to.not.be.ok;
+        return ParanoidUser.findOne({ where: { username: 'username' } });
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(user.destroyTime).to.not.be.ok;
+        expect(user.deletedAt).to.not.be.ok;
+      });
+    });
+
+    it('supports custom deletedAt field name', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: DataTypes.STRING,
+        deletedAt: { type: DataTypes.DATE, field: 'deleted_at' }
+      }, { paranoid: true });
+
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        return user.destroy();
+      }).then(user => {
+        expect(user.dataValues.deletedAt).to.be.ok;
+        expect(user.dataValues.deleted_at).to.not.be.ok;
+        return user.restore();
+      }).then(user => {
+        expect(user.dataValues.deletedAt).to.not.be.ok;
+        expect(user.dataValues.deleted_at).to.not.be.ok;
+        return ParanoidUser.findOne({ where: { username: 'username' } });
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(user.deletedAt).to.not.be.ok;
+        expect(user.deleted_at).to.not.be.ok;
+      });
+    });
+
+    it('supports custom deletedAt field and database column', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: DataTypes.STRING,
+        destroyTime: { type: DataTypes.DATE, field: 'destroy_time' }
+      }, { paranoid: true, deletedAt: 'destroyTime' });
+
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        return user.destroy();
+      }).then(user => {
+        expect(user.dataValues.destroyTime).to.be.ok;
+        expect(user.dataValues.deletedAt).to.not.be.ok;
+        expect(user.dataValues.destroy_time).to.not.be.ok;
+        return user.restore();
+      }).then(user => {
+        expect(user.dataValues.destroyTime).to.not.be.ok;
+        expect(user.dataValues.destroy_time).to.not.be.ok;
+        return ParanoidUser.findOne({ where: { username: 'username' } });
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(user.destroyTime).to.not.be.ok;
+        expect(user.destroy_time).to.not.be.ok;
+      });
+    });
+
+    it('supports custom default value', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: DataTypes.STRING,
+        deletedAt: { type: DataTypes.DATE, defaultValue: new Date(0) }
+      }, { paranoid: true });
+
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        return user.destroy();
+      }).then(user => {
+        return user.restore();
+      }).then(user => {
+        expect(user.dataValues.deletedAt.toISOString()).to.equal(new Date(0).toISOString());
+        return ParanoidUser.findOne({ where: { username: 'username' } });
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(user.deletedAt.toISOString()).to.equal(new Date(0).toISOString());
+      });
+    });
   });
 });

test/integration/instance/destroy.test.js

@@ -2,8 +2,9 @@
 
 const chai = require('chai'),
   expect = chai.expect,
+  sinon = require('sinon'),
+  moment = require('moment'),
   Support = require('../support'),
-  DataTypes = require('../../../lib/data-types'),
   dialect = Support.getTestDialect(),
   current = Support.sequelize;
 
@@ -56,6 +57,32 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
       });
     });
 
+    it('does not update deletedAt with custom default in subsequent destroys', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: Support.Sequelize.STRING,
+        deletedAt: { type: Support.Sequelize.DATE, defaultValue: new Date(0) }
+      }, { paranoid: true });
+
+      let deletedAt;
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        return user.destroy();
+      }).then(user => {
+        deletedAt = user.deletedAt;
+        expect(deletedAt).to.be.ok;
+        expect(deletedAt.getTime()).to.be.ok;
+
+        return user.destroy();
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(user.deletedAt).to.be.ok;
+        expect(user.deletedAt.toISOString()).to.equal(deletedAt.toISOString());
+      });
+    });
+
     it('deletes a record from the database if dao is not paranoid', function() {
       const UserDestroy = this.sequelize.define('UserDestroy', {
         name: Support.Sequelize.STRING,
@@ -76,24 +103,257 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
       });
     });
 
+    it('allows updating soft deleted instance', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: Support.Sequelize.STRING
+      }, { paranoid: true });
+
+      let deletedAt;
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        return user.destroy();
+      }).then(user => {
+        expect(user.deletedAt).to.be.ok;
+        deletedAt = user.deletedAt;
+        user.username = 'foo';
+        return user.save();
+      }).then(user => {
+        expect(user.username).to.equal('foo');
+        expect(user.deletedAt).to.equal(deletedAt, 'should not update deletedAt');
+
+        return ParanoidUser.findOne({
+          paranoid: false,
+          where: {
+            username: 'foo'
+          }
+        });
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(user.deletedAt).to.be.ok;
+      });
+    });
+
+    it('supports custom deletedAt field', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: Support.Sequelize.STRING,
+        destroyTime: Support.Sequelize.DATE
+      }, { paranoid: true, deletedAt: 'destroyTime' });
+
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        return user.destroy();
+      }).then(user => {
+        expect(user.destroyTime).to.be.ok;
+        expect(user.deletedAt).to.not.be.ok;
+
+        return ParanoidUser.findOne({
+          paranoid: false,
+          where: {
+            username: 'username'
+          }
+        });
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(user.destroyTime).to.be.ok;
+        expect(user.deletedAt).to.not.be.ok;
+      });
+    });
+
+    it('supports custom deletedAt database column', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: Support.Sequelize.STRING,
+        deletedAt: { type: Support.Sequelize.DATE, field: 'deleted_at' }
+      }, { paranoid: true });
+
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        return user.destroy();
+      }).then(user => {
+        expect(user.dataValues.deletedAt).to.be.ok;
+        expect(user.dataValues.deleted_at).to.not.be.ok;
+
+        return ParanoidUser.findOne({
+          paranoid: false,
+          where: {
+            username: 'username'
+          }
+        });
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(user.deletedAt).to.be.ok;
+        expect(user.deleted_at).to.not.be.ok;
+      });
+    });
+
+    it('supports custom deletedAt field and database column', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: Support.Sequelize.STRING,
+        destroyTime: { type: Support.Sequelize.DATE, field: 'destroy_time' }
+      }, { paranoid: true, deletedAt: 'destroyTime' });
+
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        return user.destroy();
+      }).then(user => {
+        expect(user.dataValues.destroyTime).to.be.ok;
+        expect(user.dataValues.destroy_time).to.not.be.ok;
+
+        return ParanoidUser.findOne({
+          paranoid: false,
+          where: {
+            username: 'username'
+          }
+        });
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(user.destroyTime).to.be.ok;
+        expect(user.destroy_time).to.not.be.ok;
+      });
+    });
+
+    it('persists other model changes when soft deleting', function() {
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: Support.Sequelize.STRING
+      }, { paranoid: true });
+
+      let deletedAt;
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        user.username = 'foo';
+        return user.destroy();
+      }).then(user => {
+        expect(user.username).to.equal('foo');
+        expect(user.deletedAt).to.be.ok;
+        deletedAt = user.deletedAt;
+
+        return ParanoidUser.findOne({
+          paranoid: false,
+          where: {
+            username: 'foo'
+          }
+        });
+      }).tap(user => {
+        expect(user).to.be.ok;
+        expect(moment.utc(user.deletedAt).startOf('second').toISOString())
+          .to.equal(moment.utc(deletedAt).startOf('second').toISOString());
+        expect(user.username).to.equal('foo');
+      }).then(user => {
+        // update model and delete again
+        user.username = 'bar';
+        return user.destroy();
+      }).then(user => {
+        expect(moment.utc(user.deletedAt).startOf('second').toISOString())
+          .to.equal(moment.utc(deletedAt).startOf('second').toISOString(),
+            'should not updated deletedAt when destroying multiple times');
+
+        return ParanoidUser.findOne({
+          paranoid: false,
+          where: {
+            username: 'bar'
+          }
+        });
+      }).then(user => {
+        expect(user).to.be.ok;
+        expect(moment.utc(user.deletedAt).startOf('second').toISOString())
+          .to.equal(moment.utc(deletedAt).startOf('second').toISOString());
+        expect(user.username).to.equal('bar');
+      });
+    });
+
     it('allows sql logging of delete statements', function() {
       const UserDelete = this.sequelize.define('UserDelete', {
         name: Support.Sequelize.STRING,
         bio: Support.Sequelize.TEXT
       });
 
+      const logging = sinon.spy();
+
       return UserDelete.sync({ force: true }).then(() => {
         return UserDelete.create({ name: 'hallo', bio: 'welt' }).then(u => {
           return UserDelete.findAll().then(users => {
             expect(users.length).to.equal(1);
-            return u.destroy({
-              logging(sql) {
+            return u.destroy({ logging });
+          });
+        });
+      }).then(() => {
+        expect(logging.callCount).to.equal(1, 'should call logging');
+        const sql = logging.firstCall.args[0];
         expect(sql).to.exist;
         expect(sql.toUpperCase()).to.include('DELETE');
-              }
       });
     });
+
+    it('allows sql logging of update statements', function() {
+      const UserDelete = this.sequelize.define('UserDelete', {
+        name: Support.Sequelize.STRING,
+        bio: Support.Sequelize.TEXT
+      }, { paranoid: true });
+
+      const logging = sinon.spy();
+
+      return UserDelete.sync({ force: true }).then(() => {
+        return UserDelete.create({ name: 'hallo', bio: 'welt' }).then(u => {
+          return UserDelete.findAll().then(users => {
+            expect(users.length).to.equal(1);
+            return u.destroy({ logging });
+          });
+        });
+      }).then(() => {
+        expect(logging.callCount).to.equal(1, 'should call logging');
+        const sql = logging.firstCall.args[0];
+        expect(sql).to.exist;
+        expect(sql.toUpperCase()).to.include('UPDATE');
+      });
     });
+
+    it('should not call save hooks when soft deleting', function() {
+      const beforeSave = sinon.spy();
+      const afterSave = sinon.spy();
+
+      const ParanoidUser = this.sequelize.define('ParanoidUser', {
+        username: Support.Sequelize.STRING
+      }, {
+        paranoid: true,
+        hooks: {
+          beforeSave,
+          afterSave
+        }
+      });
+
+      return ParanoidUser.sync({ force: true }).then(() => {
+        return ParanoidUser.create({
+          username: 'username'
+        });
+      }).then(user => {
+        // clear out calls from .create
+        beforeSave.resetHistory();
+        afterSave.resetHistory();
+
+        return user.destroy();
+      }).tap(() => {
+        expect(beforeSave.callCount).to.equal(0, 'should not call beforeSave');
+        expect(afterSave.callCount).to.equal(0, 'should not call afterSave');
+      }).then(user => {
+        // now try with `hooks: true`
+        return user.destroy({ hooks: true });
+      }).tap(() => {
+        expect(beforeSave.callCount).to.equal(0, 'should not call beforeSave even if `hooks: true`');
+        expect(afterSave.callCount).to.equal(0, 'should not call afterSave even if `hooks: true`');
       });
     });
 
@@ -140,11 +400,11 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
         const Date = this.sequelize.define('Date',
           {
             date: {
-              type: DataTypes.DATE,
+              type: Support.Sequelize.DATE,
               primaryKey: true
             },
             deletedAt: {
-              type: DataTypes.DATE,
+              type: Support.Sequelize.DATE,
               defaultValue: Infinity
             }
           },

test/unit/model/destroy.test.js

@@ -35,7 +35,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
       this.stubDelete.restore();
     });
 
-    it('can detect complexe objects', () => {
+    it('can detect complex objects', () => {
       const Where = function() { this.secretValue = '1'; };
 
       expect(() => {