Skip to content

Commit

Permalink
Fixing issue where a beforeDelete() could trigger a table truncation.
Browse files Browse the repository at this point in the history
Moving the exists check below beforeDelete() and behavior->beforeDelete() so any records deleted in the callbacks will not exist when db->delete() is called.  Test updated. Fixes #250
  • Loading branch information
markstory committed Sep 30, 2010
1 parent 8d2d83c commit 16fc261
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
11 changes: 8 additions & 3 deletions cake/libs/model/model.php
Expand Up @@ -1746,11 +1746,16 @@ function del($id = null, $cascade = true) {
}
$id = $this->id;

if ($this->exists() && $this->beforeDelete($cascade)) {
$db =& ConnectionManager::getDataSource($this->useDbConfig);
if (!$this->Behaviors->trigger($this, 'beforeDelete', array($cascade), array('break' => true, 'breakOn' => false))) {

if ($this->beforeDelete($cascade)) {
$filters = $this->Behaviors->trigger($this, 'beforeDelete', array($cascade), array(
'break' => true, 'breakOn' => false
));
if (!$filters || !$this->exists()) {
return false;
}
$db =& ConnectionManager::getDataSource($this->useDbConfig);

$this->_deleteDependent($id, $cascade);
$this->_deleteLinks($id);
$this->id = $id;
Expand Down
8 changes: 6 additions & 2 deletions cake/tests/cases/libs/model/model_delete.test.php
Expand Up @@ -664,14 +664,18 @@ function testBeforeDeleteWipingTable() {
$this->assertTrue($result > 1, 'Comments are all gone.');
}

function testBeforeDeleteWipingTable2() {
/**
* test that deleting the same record from the beforeDelete and the delete doesn't truncate the table.
*
* @return void
*/
function testBeforeDeleteWipingTableWithDuplicateDelete() {
$this->loadFixtures('Comment');

$Comment =& new BeforeDeleteComment();
$Comment->delete(1);

$result = $Comment->find('count');

$this->assertTrue($result > 1, 'Comments are all gone.');
}
}
Expand Down

0 comments on commit 16fc261

Please sign in to comment.