sequelize: Attributes modified in hooks are not saved

If you add a hook to (for exemple) a User model, and save an instance of the model with some attributes modified, such as:

user.email = 'toto@toto.com';
user.save();

And if you modify some user attributes into the (for exemple…) afterValidate hook:

function afterUserValidate(user) {
  if(user.changed('email')) {
    return u.md5(user.email).then((md5) => user.emailMd5 = md5);
  }
}

The emailMd5 attribute is not saved. This is because dirty attributes are computed before the pre-save hooks.

So my question: could we imagine to compute dirty attributes after the pre-save hooks?

About this issue

  • Original URL
  • State: open
  • Created 9 years ago
  • Reactions: 8
  • Comments: 41 (12 by maintainers)

Commits related to this issue

Most upvoted comments

After digging further, for anyone else who ends up here, it is by design that the beforeUpdate (including beforeSave, on an existing record) is NOT allowed to change additional attributes that were not previously changed before the update/save call.

You can see the code in the current master branch starting here: https://github.com/sequelize/sequelize/blob/bb30483a95658bc1e729d5ae2a966f86872f955d/src/model.js#L4108

It collects a list of attributes in which it will ignore changes (ignoreChanged), which is all the configured attributes not already changed when the update/save is called. It uses that list to filter out any additional changes made during the execution of the hook.

You can override this behavior by manually adding the changed fields to options.fields before your hook function returns.

This bug still exists in 4.22.12. If you set an attribute in a beforeUpdate hook that was not part of the original list of update fields, the updated field appears in the response but not in the database. You can’t call .save in a beforeUpdate hook b/c it creates an endless loop.

This is a ridiculously aggressive stale policy.

Still seeing this in 2020 on a beforeValidate hook.

As far as minimal examples, would the one in the documentation work? https://sequelize.readthedocs.io/en/2.0/docs/hooks/#declaring-hooks

Creating a User object and saving it to the database would not actually persist the fields that are modified in the hook.

@Hoverbear: Yep, I’m confirm that’s only in beforeValidate and afterValidate. Here is a workaround:

afterValidate: function (instance, options) {
 // ... change `instance` fields ...
 options.fields = instance.changed()
}

I am having this issue only for the beforeUpsert hook. beforeCreate seems to work just fine for me. This is with the use case where I want to hash the password before storing it into the database upon the upsert.

Also, I think this warrants the bug label since the observed behavior seems to contradict how the documentation suggests these hooks should be used.

I had a similar problem. It turns out that I was using bcrypt in an async mode in a beforeSave hook to encrypt the user’s password. This worked fine for the create process. However, for the update process, this ended up writing the unencrypted (original value) of the password. I switched over to using the synchronous mode of bcrypt and it worked fine for both the create and update.

With the async version, it did actually encrypt the password and assign it back to the attribute, but that was too late to take effect.

I fixed the issue by calling the .save() method in the hooks

I’ve just spent the last day trying to figure out why my hooks weren’t working, only to find out that there is a difference between the documentation and implementation.

Arguments to hooks are passed by reference. This means, that you can change the values, and this will be reflected in the insert / update statement. https://sequelize.org/v4/manual/tutorial/hooks.html (we’re using v4 for now)

I defiantly read that to mean I can alter the model values in the hooks and have those changes persisted.

I still get the same thing when using beforeCreate and Sequelize version 5.21.3 The clear password is the one saved all the time. The only way I managed to solve it is to move and use the synchronous version of bcrypt.

User.beforeCreate((user, options) => {
    return bcrypt.hash(userToBeSaved.password, 10, (err, hash) => {
        if (err) throw err;
        userToBeSaved.password = hash;
        console.log(userToBeSaved);
    });
});

In the meantime, I’ve found a workaround by using beforeCreate and beforeUpdate.

const beforeValidate = (instance, options) => {
  // ...
}

const options = {
  hooks: {
    // Custom "beforeValidate" hook because of a bug on sequelize.
    // See https://github.com/sequelize/sequelize/issues/3534
    beforeCreate: beforeValidate,
    beforeUpdate: beforeValidate,
  },
}

const MyModel: Model = sequelize.define('myModel', schema, options)

I’m at version 4.37 and I’m also having this issue. As pointed out above when using create everything works fine but when upserting the altered values in the hook are ignored. Everything inside the hook is running sync so that should not be an issue. Even when logging the values at the last line of the hook I can see the altered values, but they are not used in the Sequelize query 😕

Any updates in this issue?

EDIT: When logging the value with a create it seems you get the complete Sequelize model, when running an upsert you only seem to get the raw values, which might explain why it isn’t working.

Running into the same thing right now (pg: 7.4.1, sequelize: 4.33.3). I’m trying to reset my updatedAt column before it updates:

hooks: {
  beforeUpdate: (entity) => {
    entity.updatedAt = null;
  },
},

As a work-around I can manually set updatedAt in my update call, but I’d much rather set it in the model definition.

Hi guys,

Just upgraded from v3.30.4 to v4.2.1 to try and solve this issue but apparently it’s still there…

The tests included by @sushantdhiman aren’t really testing the whole scenario since what’s persisted in the database isn’t actually what’s returned in the save/update callback. I hit the same pitfall in which the object in the callback is updated whereas getting the record directly from the database is not…

I’ll try to change the tests to show what I mean.

Looks like only beforeCreate is set up to figure out if values changed during a hook (in cases of default .changed() fields): https://github.com/sequelize/sequelize/blob/master/lib/instance.js#L583

Same code would need to be applied to after validation.