Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Batch deletetion of nested set objects #118

Merged
merged 3 commits into from Apr 23, 2012

Conversation

Projects
None yet
2 participants
Contributor

rozwell commented Feb 29, 2012

When removing nested set objects via batchDelete, there can be possible tree corruption.

Objects are retrieved only once and every time one of them is deleted, nested structure changes but it's not updated for objects in memory:


<?php
    $count = 0;
    foreach (MapPeer::retrieveByPks($ids) as $object)
    {
      $this->dispatcher->notify(new sfEvent($this, 'admin.delete_object', array('object' => $object)));

      $object->delete();
      if ($object->isDeleted())
      {
        $count++;
      }
    }

The simple fix is to reload each object before deletion:


<?php
    $count = 0;
    foreach (MapPeer::retrieveByPks($ids) as $object)
    {
      $this->dispatcher->notify(new sfEvent($this, 'admin.delete_object', array('object' => $object)));

      // reload the object if it's in a tree to avoid breaking nested set structure
      if ($count && method_exists($object, 'isInTree') && $object->isInTree())
      {
        $object->reload();
      }

      $object->delete();
      if ($object->isDeleted())
      {
        $count++;
      }
    }

Note: we don't need to reload first object since there were no changes yet.

Contributor

rozwell commented Feb 29, 2012

It seems that batchDelete is much more bugged than this, working on fix.

Contributor

rozwell commented Feb 29, 2012

Ok, seems to be working fine now.

Owner

willdurand commented Feb 29, 2012

@mazenovi @themouette Can you follow this PR?

willdurand added a commit that referenced this pull request Apr 23, 2012

Merge pull request #118 from rozwell/nested_batch_delete
Batch deletetion of nested set objects

@willdurand willdurand merged commit 8d55e00 into propelorm:master Apr 23, 2012

Owner

willdurand commented Apr 23, 2012

Merged, thanks.

hd-deman pushed a commit to hd-deman/sfPropelORMPlugin that referenced this pull request May 4, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment