Make updateAll() fire behavior callbacks

For a while now updateAll() would not trigger any of the model’s behavior callbacks.

This presents a problem for a couple of reasons:

  1. The updateAll() method allows to easily do things like field_value = field_value + 1, because unlike other similar methods it does not escape fields/values
  2. Secondly we cannot rely on proper Behavior execution, which can lead to some unexpected results

What we have below is a simple override (and a very rough patch at this point) that will trigger afterSave() of the attached behavior as expected.

Take the code below and place into the model, which uses updateAll() at some point.

public function updateAll($fields, $conditions = true) {
    $db =& ConnectionManager::getDataSource($this->useDbConfig);
    $created = FALSE;
    $options = array();
    if($db->update($this, $fields, null, $conditions)) {
      $created = TRUE;
      $this->Behaviors->trigger($this, 'afterSave', array($created, $options));
      $this->afterSave($created);
      $this->_clearCache();
      $this->id = false;
      return true;
    }
  return FALSE;
 }

Again, this works for my immediate needs and doesn’t break any tests. However there are a few things that can be improved to make it more consistent with other methods like save(), for example.

Well, either way I am hoping that this will help someone.

m4s0n501
  • http://fahad19.com Fahad
  • http://teknoid.wordpress.com teknoid

    @Fahad

    What are the chances?… but I’m still glad I didn’t decide to go buy a pony instead of writing this up.

  • Pingback: CakePHP : signets remarquables du 25/01/2010 au 29/01/2010 | Cherry on the...()

  • http://fahad19.com Fahad

    line 10 may cause problem with TreeBehavior when creating new records with parent_id.

  • http://teknoid.wordpress.com teknoid

    @Fahad

    That’s taken straight from the core… Haven’t tested with the tree behavior at all.
    Maybe a test case is in order ;)

  • Seb

    Which version if cake is this for? I noticed similar behaviour with saveAll() in 1.3 and it seems like a pretty big bug. I’m just not sure If I’m using it correctly. What’s the point of an afterSave() method if you still have to worry about edge cases? Are you aware of other similar cases where the hook wouldn’t get called?

  • http://teknoid.wordpress.com teknoid

    @Seb

    It was still an issue in 1.3…
    I’m not sure if it has been addressed in 1.3.1

    That being said, saveAll() had never caused me any problems, and I do use it quite often.

    • Seb

      I’m definately having issues with saveAll(). If I save my model while associated to another with saveAll(), the “many” in the hasMany relationship does not get it’s trigger fired. If I save that model directly by itself, it gets fired. (I’m using the AclBehavior to create ACOs when saved). It appears that when saveAll() instantiates the associated model, $this is of type AppModel and $actsAs is not defined. When I save it directly, it’s of type and $actsAs _is_ defined…

      On another note, would it be safe to put the updateAll() AppModel so that all models use it?

      I’m new to cakePHP, so like I said, I might be doing it wrong..

  • http://teknoid.wordpress.com teknoid

    @Seb

    It’s hard to tell because each case is different, especially when using behaviors.
    As you see from comments above my “fix” didn’t seem to work with the Tree behavior.
    That being said, if you write tests for your models you can be fairly safe before moving to production.

  • http://alexandersitedesign.com Matt Alexander

    Do you have code that you use to trigger beforeValidate and beforeSave as well?

  • http://teknoid.wordpress.com teknoid

    @Matt Alexander

    No, but I imagine it would be something quite similar.

  • YOMorales

    Hi teknoid:

    This proved to be quite useful. However, I wonder the reason for line #6. After all, this is an update, so why pass $created as true to the afterSave() callback?

    Also, this is a very circumstantial hack of mine, but if you are using updateAll() for a single record and need the $this->id in the afterSave() callback, then you can put the following after line #5:

    if (!empty($conditions[‘ModelName.id’])) {
    $this->id = $conditions[‘ModelName.id’];
    } else {
    $this->id = false;
    }

    Thanks for the post.

  • teknoid

    @YOMorales

    That’s what afterSave() expects. Take a look at the API.

  • http://www.deansofer.com ProLoser

    Just a note since I may add this to my BakingPlate plugin, I would actually overload the method by adding a third argument, $callback = true

    If $callback isn’t passed it triggers the parent method or if $callback != true then call the parent method explicitly. This would allow you to make callback triggering optional and seems graceful.