Browse Source

Merge pull request #9085 from cakephp/fix/rollback-in-after-save

Throwing an exception when the afterSave aborts the transaction
Mark Story 9 years ago
parent
commit
2fd23dfd91

+ 25 - 0
src/ORM/Exception/RolledbackTransactionException.php

@@ -0,0 +1,25 @@
+<?php
+/**
+ * CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @since         3.2.13
+ * @license       http://www.opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\ORM\Exception;
+
+use Cake\Core\Exception\Exception;
+
+/**
+ * Used when a transaction was rolled back from a callback event.
+ *
+ */
+class RolledbackTransactionException extends Exception
+{
+
+    protected $_messageTemplate = 'The afterSave event in "%s" is aborting the transaction before the save process is done.';
+}

+ 44 - 18
src/ORM/Table.php

@@ -33,6 +33,7 @@ use Cake\ORM\Association\BelongsToMany;
 use Cake\ORM\Association\HasMany;
 use Cake\ORM\Association\HasOne;
 use Cake\ORM\Exception\MissingEntityException;
+use Cake\ORM\Exception\RolledbackTransactionException;
 use Cake\ORM\Rule\IsUnique;
 use Cake\Utility\Inflector;
 use Cake\Validation\Validation;
@@ -1436,6 +1437,8 @@ class Table implements RepositoryInterface, EventListenerInterface, EventDispatc
      * $articles->save($entity, ['associated' => false]);
      * ```
      *
+     * @throws \Cake\ORM\Exception\RolledbackTransactionException If the transaction
+     *   is aborted in the afterSave event.
      */
     public function save(EntityInterface $entity, $options = [])
     {
@@ -1486,6 +1489,8 @@ class Table implements RepositoryInterface, EventListenerInterface, EventDispatc
      * @param \ArrayObject $options the options to use for the save operation
      * @return \Cake\Datasource\EntityInterface|bool
      * @throws \RuntimeException When an entity is missing some of the primary keys.
+     * @throws \Cake\ORM\Exception\RolledbackTransactionException If the transaction
+     *   is aborted in the afterSave event.
      */
     protected function _processSave($entity, $options)
     {
@@ -1533,32 +1538,53 @@ class Table implements RepositoryInterface, EventListenerInterface, EventDispatc
         }
 
         if ($success) {
-            $success = $this->_associations->saveChildren(
-                $this,
-                $entity,
-                $options['associated'],
-                ['_primary' => false] + $options->getArrayCopy()
-            );
-            if ($success || !$options['atomic']) {
-                $this->dispatchEvent('Model.afterSave', compact('entity', 'options'));
-                $entity->clean();
-                if (!$options['atomic'] && !$options['_primary']) {
-                    $entity->isNew(false);
-                    $entity->source($this->registryAlias());
-                }
-                $success = true;
-            }
+            $success = $this->_onSaveSuccess($entity, $options);
         }
 
         if (!$success && $isNew) {
             $entity->unsetProperty($this->primaryKey());
             $entity->isNew(true);
         }
-        if ($success) {
-            return $entity;
+
+        return $success ? $entity : false;
+    }
+
+    /**
+     * Handles the saving of children associations and executing the afterSave logic
+     * once the entity for this table has been saved successfully.
+     *
+     * @param \Cake\Datasource\EntityInterface $entity the entity to be saved
+     * @param \ArrayObject $options the options to use for the save operation
+     * @return bool True on success
+     * @throws \Cake\ORM\Exception\RolledbackTransactionException If the transaction
+     *   is aborted in the afterSave event.
+     */
+    protected function _onSaveSuccess($entity, $options)
+    {
+        $success = $this->_associations->saveChildren(
+            $this,
+            $entity,
+            $options['associated'],
+            ['_primary' => false] + $options->getArrayCopy()
+        );
+
+        if (!$success && $options['atomic']) {
+            return false;
+        }
+
+        $this->dispatchEvent('Model.afterSave', compact('entity', 'options'));
+
+        if ($options['atomic'] && !$this->connection()->inTransaction()) {
+            throw new RolledbackTransactionException(['table' => get_class($this)]);
+        }
+
+        $entity->clean();
+        if (!$options['atomic'] && !$options['_primary']) {
+            $entity->isNew(false);
+            $entity->source($this->registryAlias());
         }
 
-        return false;
+        return true;
     }
 
     /**

+ 70 - 0
tests/TestCase/ORM/TableRegressionTest.php

@@ -0,0 +1,70 @@
+<?php
+/**
+ * CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @link          http://cakephp.org CakePHP(tm) Project
+ * @since         3.2.13
+ * @license       http://www.opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Test\TestCase\ORM;
+
+use Cake\Core\Plugin;
+use Cake\I18n\Time;
+use Cake\ORM\TableRegistry;
+use Cake\TestSuite\TestCase;
+
+/**
+ * Contains regression test for the Table class
+ *
+ */
+class TableRegressionTest extends TestCase
+{
+
+    /**
+     * Fixture to be used
+     *
+     * @var array
+     */
+    public $fixtures = [
+        'core.authors',
+    ];
+
+    /**
+     * Tear down
+     *
+     * @return void
+     */
+    public function tearDown()
+    {
+        parent::tearDown();
+
+        TableRegistry::clear();
+    }
+
+    /**
+     * Tests that an exception is thrown if the transaction is aborted
+     * in the afterSave callback
+     *
+     * @see https://github.com/cakephp/cakephp/issues/9079
+     * @expectedException Cake\ORM\Exception\RolledbackTransactionException
+     * @return void
+     */
+    public function testAfterSaveRollbackTransaction()
+    {
+        $table = TableRegistry::get('Authors');
+        $table->eventManager()->on(
+            'Model.afterSave',
+            function () use ($table) {
+                $table->connection()->rollback();
+            }
+        );
+        $entity = $table->newEntity(['name' => 'Jon']);
+        $table->save($entity);
+    }
+}

+ 2 - 1
tests/TestCase/ORM/TableTest.php

@@ -2427,7 +2427,7 @@ class TableTest extends TestCase
         $config = ConnectionManager::config('test');
 
         $connection = $this->getMockBuilder('\Cake\Database\Connection')
-            ->setMethods(['begin', 'commit'])
+            ->setMethods(['begin', 'commit', 'inTransaction'])
             ->setConstructorArgs([$config])
             ->getMock();
         $connection->driver($this->connection->driver());
@@ -2441,6 +2441,7 @@ class TableTest extends TestCase
 
         $connection->expects($this->once())->method('begin');
         $connection->expects($this->once())->method('commit');
+        $connection->expects($this->any())->method('inTransaction')->will($this->returnValue(true));
         $data = new \Cake\ORM\Entity([
             'username' => 'superuser',
             'created' => new Time('2013-10-10 00:00'),