From f3900e89fdf85c06384aaab5c1cfa30200edf56f Mon Sep 17 00:00:00 2001 From: ADmad Date: Tue, 3 Dec 2013 22:14:26 +0530 Subject: [PATCH] Fixed bug where deleteAll tried to delete same id multiple times. Ensure find done in deleteAll only returns distinct ids. A wacky combination of association and conditions can sometimes generate multiple rows per id. --- lib/Cake/Model/Model.php | 2 +- lib/Cake/Test/Case/Model/ModelDeleteTest.php | 37 ++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php index 2c1b66ce1..ce6c13d2d 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -2699,7 +2699,7 @@ class Model extends Object implements CakeEventListener { } $ids = $this->find('all', array_merge(array( - 'fields' => "{$this->alias}.{$this->primaryKey}", + 'fields' => "DISTINCT {$this->alias}.{$this->primaryKey}", 'recursive' => 0), compact('conditions')) ); diff --git a/lib/Cake/Test/Case/Model/ModelDeleteTest.php b/lib/Cake/Test/Case/Model/ModelDeleteTest.php index 5cadf2cfa..9abcb5101 100644 --- a/lib/Cake/Test/Case/Model/ModelDeleteTest.php +++ b/lib/Cake/Test/Case/Model/ModelDeleteTest.php @@ -454,6 +454,43 @@ class ModelDeleteTest extends BaseModelTest { $this->assertFalse($result); } +/** + * testDeleteAllMultipleRowsPerId method + * + * Ensure find done in deleteAll only returns distinct ids. A wacky combination + * of association and conditions can sometimes generate multiple rows per id. + * + * @return void + */ + public function testDeleteAllMultipleRowsPerId() { + $this->loadFixtures('Article', 'User'); + + $TestModel = new Article(); + $TestModel->unbindModel(array( + 'belongsTo' => array('User'), + 'hasMany' => array('Comment'), + 'hasAndBelongsToMany' => array('Tag') + ), false); + $TestModel->bindModel(array( + 'belongsTo' => array( + 'User' => array( + 'foreignKey' => false, + 'conditions' => array( + 'Article.user_id = 1' + ) + ) + ) + ), false); + + $result = $TestModel->deleteAll( + array('Article.user_id' => array(1, 3)), + true, + true + ); + + $this->assertTrue($result); + } + /** * testRecursiveDel method *