Permalink
Browse files

Fixing issue where a beforeDelete() could trigger a table truncation.

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...
1 parent 38e128b commit c573fd0432964613567810afa0f0f00f2b9707ed @markstory markstory committed Sep 30, 2010
Showing with 10 additions and 5 deletions.
  1. +4 −3 cake/libs/model/model.php
  2. +6 −2 cake/tests/cases/libs/model/model_delete.test.php
View
7 cake/libs/model/model.php
@@ -1803,14 +1803,15 @@ function delete($id = null, $cascade = true) {
}
$id = $this->id;
- if ($this->exists() && $this->beforeDelete($cascade)) {
- $db =& ConnectionManager::getDataSource($this->useDbConfig);
+ if ($this->beforeDelete($cascade)) {
$filters = $this->Behaviors->trigger($this, 'beforeDelete', array($cascade), array(
'break' => true, 'breakOn' => false
));
- if (!$filters) {
+ if (!$filters || !$this->exists()) {
return false;
}
+ $db =& ConnectionManager::getDataSource($this->useDbConfig);
+
$this->_deleteDependent($id, $cascade);
$this->_deleteLinks($id);
$this->id = $id;
View
8 cake/tests/cases/libs/model/model_delete.test.php
@@ -737,14 +737,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.');
}
}

0 comments on commit c573fd0

Please sign in to comment.