From c6e5026767e07585d43ff63de397535d652c18cc Mon Sep 17 00:00:00 2001 From: mark_story Date: Wed, 22 Jul 2015 00:40:45 -0400 Subject: [PATCH] Fix issues saveMany & saveAssociated with boolean values. For non-atomic, save operations that include models with boolean fields. The first false value would cause the save to abort. This regression was introduced in #6947. Instead of checking the data from save() we should be boolean casting save() to capture the success/failure. Refs #7069 --- lib/Cake/Model/Model.php | 8 +- lib/Cake/Test/Case/Model/ModelWriteTest.php | 90 +++++++++++++++++++++ 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php index 6cfe4713a..bfce2dd41 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -1717,7 +1717,7 @@ class Model extends Object implements CakeEventListener { * - validate: Set to true/false to enable or disable validation. * - fieldList: An array of fields you want to allow for saving. * - callbacks: Set to false to disable callbacks. Using 'before' or 'after' - * will enable only those callbacks. + * will enable only those callbacks. * - `counterCache`: Boolean to control updating of counter caches (if any) * * @param array $fieldList List of fields to allow to be saved @@ -2335,7 +2335,7 @@ class Model extends Object implements CakeEventListener { if ($options['deep']) { $saved = $this->saveAssociated($record, array('atomic' => false) + $options); } else { - $saved = $this->save($record, array('atomic' => false) + $options); + $saved = (bool)$this->save($record, array('atomic' => false) + $options); } } @@ -2478,7 +2478,7 @@ class Model extends Object implements CakeEventListener { if ($options['deep']) { $saved = $Model->saveAssociated($values, array('atomic' => false) + $options); } else { - $saved = $Model->save($values, array('atomic' => false) + $options); + $saved = (bool)$Model->save($values, array('atomic' => false) + $options); } $validates = ($saved === true || (is_array($saved) && !in_array(false, Hash::flatten($saved), true))); } @@ -2534,7 +2534,7 @@ class Model extends Object implements CakeEventListener { if ($options['deep']) { $saved = $Model->saveAssociated($values, array('atomic' => false) + $options); } else { - $saved = $Model->save($values, $options); + $saved = (bool)$Model->save($values, $options); } } diff --git a/lib/Cake/Test/Case/Model/ModelWriteTest.php b/lib/Cake/Test/Case/Model/ModelWriteTest.php index 62818f6f5..9e3e78927 100644 --- a/lib/Cake/Test/Case/Model/ModelWriteTest.php +++ b/lib/Cake/Test/Case/Model/ModelWriteTest.php @@ -7684,6 +7684,37 @@ class ModelWriteTest extends BaseModelTest { $this->assertEquals(2, count($result['Attachment'])); } +/** + * Test that boolean fields don't cause saveMany to fail + * + * @return void + */ + public function testSaveManyBooleanFields() { + $this->loadFixtures('Item', 'Syfile', 'Image'); + $data = array( + array( + 'Item' => array( + 'name' => 'testing', + 'syfile_id' => 1, + 'published' => false + ) + ), + array( + 'Item' => array( + 'name' => 'testing 2', + 'syfile_id' => 1, + 'published' => true + ) + ), + ); + $item = ClassRegistry::init('Item'); + $result = $item->saveMany($data, array('atomic' => false)); + + $this->assertCount(2, $result, '2 records should have been saved.'); + $this->assertTrue($result[0], 'Both should have succeded'); + $this->assertTrue($result[1], 'Both should have succeded'); + } + /** * testSaveManyDeepHasManyValidationFailure method * @@ -7806,6 +7837,65 @@ class ModelWriteTest extends BaseModelTest { ), $TestModel->validationErrors); } +/** + * Test that boolean fields don't cause saveAssociated to fail + * + * @return void + */ + public function testSaveAssociatedHasOneBooleanFields() { + $this->loadFixtures('Item', 'Syfile', 'Image'); + $data = array( + 'Syfile' => array( + 'image_id' => 1, + 'name' => 'Some file', + ), + 'Item' => array( + 'name' => 'testing', + 'published' => false + ), + ); + $syfile = ClassRegistry::init('Syfile'); + $syfile->bindModel(array('hasOne' => array('Item')), false); + $result = $syfile->saveAssociated($data, array('atomic' => false)); + + $this->assertCount(2, $result, '2 records should have been saved.'); + $this->assertTrue($result['Syfile'], 'Both should have succeded'); + $this->assertTrue($result['Item'], 'Both should have succeded'); + } + +/** + * Test that boolean fields don't cause saveAssociated to fail + * + * @return void + */ + public function testSaveAssociatedBelongsToBooleanFields() { + $this->loadFixtures('Item', 'Syfile', 'Image'); + $data = array( + 'Syfile' => array( + 'image_id' => 1, + 'name' => 'Some file', + ), + 'Item' => array( + 'name' => 'testing', + 'syfile_id' => 2, + 'published' => false + ), + ); + $item = ClassRegistry::init('Item'); + $item->bindModel(array( + 'belongsTo' => array( + 'Item' => array( + 'foreignKey' => 'image_id' + ) + ) + ), false); + $result = $item->saveAssociated($data, array('atomic' => false)); + + $this->assertCount(2, $result, '2 records should have been saved.'); + $this->assertTrue($result['Syfile'], 'Both should have succeded'); + $this->assertTrue($result['Item'], 'Both should have succeded'); + } + /** * testUpdateAllBoolean *