Use Connection::isTransaction() instead of _primary flag.

The former is a more robust way to check if transaction is running and even accounts for transactions started outside Table::save().
cakephp · Feb 26, 2015 · 451dc35 · 451dc35
1 parent e2eeb3f
commit 451dc35
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 6 deletions.
diff --git a/src/ORM/Table.php b/src/ORM/Table.php
@@ -1334,7 +1334,6 @@ public function save(EntityInterface $entity, $options = [])
             'associated' => true,
             'checkRules' => true,
             'checkExisting' => true,
-            '_primary' => true
         ]);
 
         if ($entity->errors()) {
@@ -1351,7 +1350,7 @@ public function save(EntityInterface $entity, $options = [])
                 return $this->_processSave($entity, $options);
             });
             if ($success) {
-                if ($options['_primary']) {
+                if (!$connection->inTransaction()) {
                     $this->dispatchEvent('Model.afterSaveCommit', compact('entity', 'options'));
                 }
                 $entity->isNew(false);
@@ -1401,7 +1400,7 @@ protected function _processSave($entity, $options)
             $this,
             $entity,
             $options['associated'],
-            ['_primary' => false] + $options->getArrayCopy()
+            $options->getArrayCopy()
         );
 
         if (!$saved && $options['atomic']) {
@@ -1422,7 +1421,7 @@ protected function _processSave($entity, $options)
                 $this,
                 $entity,
                 $options['associated'],
-                ['_primary' => false] + $options->getArrayCopy()
+                $options->getArrayCopy()
             );
             if ($success || !$options['atomic']) {
                 $entity->clean();

diff --git a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php
@@ -466,7 +466,6 @@ function ($event, Entity $entity, \ArrayObject $options, $operation) {
                         'associated' => true,
                         'checkRules' => true,
                         'checkExisting' => true,
-                        '_primary' => true
                     ],
                     $options->getArrayCopy()
                 );
@@ -505,7 +504,6 @@ function ($event, Entity $entity, \ArrayObject $options, $result, $operation) {
                         'associated' => true,
                         'checkRules' => true,
                         'checkExisting' => true,
-                        '_primary' => true
                     ],
                     $options->getArrayCopy()
                 );

diff --git a/tests/TestCase/ORM/TableTest.php b/tests/TestCase/ORM/TableTest.php
@@ -1811,6 +1811,32 @@ public function testAfterSaveCommitForNonAtomic()
         $this->assertFalse($calledAfterCommit);
     }
 
+    /**
+     * Asserts the afterSaveCommit is not triggered if transaction is running.
+     *
+     * @return [type]
+     */
+    public function testAfterSaveCommitWithTransactionRunning()
+    {
+        $table = TableRegistry::get('users');
+        $data = new \Cake\ORM\Entity([
+            'username' => 'superuser',
+            'created' => new Time('2013-10-10 00:00'),
+            'updated' => new Time('2013-10-10 00:00')
+        ]);
+
+        $called = false;
+        $listener = function ($e, $entity, $options) use (&$called) {
+            $called = true;
+        };
+        $table->eventManager()->attach($listener, 'Model.afterSaveCommit');
+
+        $this->connection->begin();
+        $this->assertSame($data, $table->save($data));
+        $this->assertFalse($called);
+        $this->connection->commit();
+    }
+
     /**
      * Asserts that afterSave callback not is called on unsuccessful save
      *