From 31615ce4159f439ba7f877ec87b7b8ef3037b2d7 Mon Sep 17 00:00:00 2001 From: Haithem BEN GHORBAL Date: Tue, 3 Jun 2014 09:37:17 +0200 Subject: [PATCH] add 'atomic' option to "save()" API This commit adds a transaction context to 'save()' API in order to rollback possible modifications done in some 'Model.beforeSave' listener callback. This will allow cakephp 2.x to behave like 3.0 . It uses try/catch to better handle transaction. Previous save() API is renamed to protected _doSave() method. A new save() method is created for transaction handling. 'atomic' option is disabled for internal 'save()' call. --- lib/Cake/Model/Model.php | 65 ++++- lib/Cake/Test/Case/Model/ModelWriteTest.php | 252 ++++++++++++++++++++ 2 files changed, 306 insertions(+), 11 deletions(-) diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php index 31fd36459..d3e5be4cd 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -1671,13 +1671,15 @@ class Model extends Object implements CakeEventListener { /** * Saves model data (based on white-list, if supplied) to the database. By - * default, validation occurs before save. + * default, validation occurs before save. Passthrough method to _doSave() with + * transaction handling. * * @param array $data Data to save. * @param boolean|array $validate Either a boolean, or an array. * If a boolean, indicates whether or not to validate before saving. * If an array, can have following keys: * + * - atomic: If true (default), will attempt to save the record in a single transaction. * - 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' @@ -1691,16 +1693,59 @@ class Model extends Object implements CakeEventListener { public function save($data = null, $validate = true, $fieldList = array()) { $defaults = array( 'validate' => true, 'fieldList' => array(), - 'callbacks' => true, 'counterCache' => true + 'callbacks' => true, 'counterCache' => true, + 'atomic' => true ); - $_whitelist = $this->whitelist; - $fields = array(); if (!is_array($validate)) { $options = compact('validate', 'fieldList') + $defaults; } else { $options = $validate + $defaults; } + + if (!$options['atomic']) { + return $this->_doSave($data, $options); + } + + $db = $this->getDataSource(); + $transactionBegun = $db->begin(); + try { + $success = $this->_doSave($data, $options); + if ($transactionBegun) { + if ($success) { + $db->commit(); + } else { + $db->rollback(); + } + } + return $success; + } catch (Exception $e) { + if ($transactionBegun) { + $db->rollback(); + } + throw $e; + } + } + +/** + * Saves model data (based on white-list, if supplied) to the database. By + * default, validation occurs before save. + * + * @param array $data Data to save. + * @param array $options can have following keys: + * + * - 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. + * - `counterCache`: Boolean to control updating of counter caches (if any) + * + * @return mixed On success Model::$data if its not empty or true, false on failure + * @link http://book.cakephp.org/2.0/en/models/saving-your-data.html + */ + protected function _doSave($data = null, $options = array()) { + $_whitelist = $this->whitelist; + $fields = array(); if (!empty($options['fieldList'])) { if (!empty($options['fieldList'][$this->alias]) && is_array($options['fieldList'][$this->alias])) { @@ -1779,8 +1824,6 @@ class Model extends Object implements CakeEventListener { } } - $db = $this->getDataSource(); - if (empty($this->data[$this->alias][$this->primaryKey])) { unset($this->data[$this->alias][$this->primaryKey]); } @@ -1997,7 +2040,7 @@ class Model extends Object implements CakeEventListener { $Model->create(); } - $Model->save($data); + $Model->save($data, array('atomic' => false)); } } @@ -2241,9 +2284,9 @@ class Model extends Object implements CakeEventListener { $saved = false; if ($validates) { if ($options['deep']) { - $saved = $this->saveAssociated($record, array_merge($options, array('atomic' => false))); + $saved = $this->saveAssociated($record, array('atomic' => false) + $options); } else { - $saved = $this->save($record, $options); + $saved = $this->save($record, array('atomic' => false) + $options); } } @@ -2395,7 +2438,7 @@ class Model extends Object implements CakeEventListener { $return[$association] = $validates; } - if ($validates && !($this->create(null) !== null && $this->save($data, $options))) { + if ($validates && !($this->create(null) !== null && $this->save($data, array('atomic' => false) + $options))) { $validationErrors[$this->alias] = $this->validationErrors; $validates = false; } @@ -2431,7 +2474,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 = $Model->save($values, array('atomic' => false) + $options); } } diff --git a/lib/Cake/Test/Case/Model/ModelWriteTest.php b/lib/Cake/Test/Case/Model/ModelWriteTest.php index cd427a9aa..6c7672de6 100644 --- a/lib/Cake/Test/Case/Model/ModelWriteTest.php +++ b/lib/Cake/Test/Case/Model/ModelWriteTest.php @@ -702,6 +702,258 @@ class ModelWriteTest extends BaseModelTest { $this->assertFalse($result); } +/** + * testSaveAtomic method + * + * @return void + */ + public function testSaveAtomic() { + $this->loadFixtures('Article'); + $TestModel = new Article(); + + // Create record with 'atomic' = false + + $data = array( + 'Article' => array( + 'user_id' => '1', + 'title' => 'Fourth Article', + 'body' => 'Fourth Article Body', + 'published' => 'Y' + ) + ); + $TestModel->create(); + $result = $TestModel->save($data, array('atomic' => false)); + $this->assertTrue((bool)$result); + + // Check record we created + + $TestModel->recursive = -1; + $result = $TestModel->read(array('id', 'user_id', 'title', 'body', 'published'), 4); + $expected = array( + 'Article' => array( + 'id' => '4', + 'user_id' => '1', + 'title' => 'Fourth Article', + 'body' => 'Fourth Article Body', + 'published' => 'Y' + ) + ); + $this->assertEquals($expected, $result); + + // Create record with 'atomic' = true + + $data = array( + 'Article' => array( + 'user_id' => '4', + 'title' => 'Fifth Article', + 'body' => 'Fifth Article Body', + 'published' => 'Y' + ) + ); + $TestModel->create(); + $result = $TestModel->save($data, array('atomic' => true)); + $this->assertTrue((bool)$result); + + // Check record we created + + $TestModel->recursive = -1; + $result = $TestModel->read(array('id', 'user_id', 'title', 'body', 'published'), 5); + $expected = array( + 'Article' => array( + 'id' => '5', + 'user_id' => '4', + 'title' => 'Fifth Article', + 'body' => 'Fifth Article Body', + 'published' => 'Y' + ) + ); + $this->assertEquals($expected, $result); + } + +/** + * test save with transaction and ensure there is no missing rollback. + * + * @return void + */ + public function testSaveTransactionNoRollback() { + $this->loadFixtures('Post', 'Article'); + + $db = $this->getMock('DboSource', array('begin', 'connect', 'rollback', 'describe')); + + $db->expects($this->once()) + ->method('describe') + ->will($this->returnValue(array())); + $db->expects($this->once()) + ->method('begin') + ->will($this->returnValue(true)); + $db->expects($this->once()) + ->method('rollback'); + + $Post = new TestPost(); + $Post->setDataSourceObject($db); + + $callback = array($this, 'callbackForTestSaveTransaction'); + $Post->getEventManager()->attach($callback, 'Model.beforeSave'); + + $data = array( + 'Post' => array( + 'author_id' => 1, + 'title' => 'New Fourth Post' + ) + ); + $Post->save($data, array('atomic' => true)); + } + +/** + * test callback used in testSaveTransaction method + * + * @return boolean false to stop event propagation + */ + public function callbackForTestSaveTransaction($event) { + $TestModel = new Article(); + + // Create record. Do not use same model as in testSaveTransaction + // to avoid infinite loop. + + $data = array( + 'Article' => array( + 'user_id' => '1', + 'title' => 'Fourth Article', + 'body' => 'Fourth Article Body', + 'published' => 'Y' + ) + ); + $TestModel->create(); + $result = $TestModel->save($data); + $this->assertTrue((bool)$result); + + // force transaction to be rolled back in Post model + $event->stopPropagation(); + return false; + } + +/** + * testSaveTransaction method + * + * @return void + */ + public function testSaveTransaction() { + $this->loadFixtures('Post', 'Article'); + $PostModel = new Post(); + + // Check if Database supports transactions + + $PostModel->validate = array('title' => 'notEmpty'); + $data = array( + array('author_id' => 1, 'title' => 'New Fourth Post'), + array('author_id' => 1, 'title' => 'New Fifth Post'), + array('author_id' => 1, 'title' => '') + ); + $this->assertFalse($PostModel->saveAll($data)); + + $result = $PostModel->find('all', array('recursive' => -1)); + $expectedPosts = array( + array( + 'Post' => array( + 'id' => '1', + 'author_id' => 1, + 'title' => 'First Post', + 'body' => 'First Post Body', + 'published' => 'Y', + 'created' => '2007-03-18 10:39:23', + 'updated' => '2007-03-18 10:41:31' + ) + ), + array( + 'Post' => array( + 'id' => '2', + 'author_id' => 3, + 'title' => 'Second Post', + 'body' => 'Second Post Body', + 'published' => 'Y', + 'created' => '2007-03-18 10:41:23', + 'updated' => '2007-03-18 10:43:31' + ) + ), + array( + 'Post' => array( + 'id' => '3', + 'author_id' => 1, + 'title' => 'Third Post', + 'body' => 'Third Post Body', + 'published' => 'Y', + 'created' => '2007-03-18 10:43:23', + 'updated' => '2007-03-18 10:45:31' + ) + ) + ); + + $this->skipIf(count($result) !== 3, 'Database does not support transactions.'); + + $this->assertEquals($expectedPosts, $result); + + // Database supports transactions --> continue tests + + $data = array( + 'Post' => array( + 'author_id' => 1, + 'title' => 'New Fourth Post' + ) + ); + + $callback = array($this, 'callbackForTestSaveTransaction'); + $PostModel->getEventManager()->attach($callback, 'Model.beforeSave'); + + $PostModel->create(); + $result = $PostModel->save($data, array('atomic' => true)); + $this->assertFalse($result); + + $result = $PostModel->find('all', array('recursive' => -1)); + $this->assertEquals($expectedPosts, $result); + + // Check record we created in callbackForTestSaveTransaction method. + // record should not exist due to rollback + + $ArticleModel = new Article(); + $result = $ArticleModel->find('all', array('recursive' => -1)); + $expectedArticles = array( + array( + 'Article' => array( + 'user_id' => '1', + 'title' => 'First Article', + 'body' => 'First Article Body', + 'published' => 'Y', + 'created' => '2007-03-18 10:39:23', + 'updated' => '2007-03-18 10:41:31', + 'id' => '1' + ) + ), + array( + 'Article' => array( + 'user_id' => '3', + 'title' => 'Second Article', + 'body' => 'Second Article Body', + 'published' => 'Y', + 'created' => '2007-03-18 10:41:23', + 'updated' => '2007-03-18 10:43:31', + 'id' => '2' + ) + ), + array( + 'Article' => array( + 'user_id' => '1', + 'title' => 'Third Article', + 'body' => 'Third Article Body', + 'published' => 'Y', + 'created' => '2007-03-18 10:43:23', + 'updated' => '2007-03-18 10:45:31', + 'id' => '3' + ) + ) + ); + $this->assertEquals($expectedArticles, $result); + } + /** * testSaveField method *