Browse Source

Trigger afterSaveCommit for non-atomic saves too for primary table.

ADmad 11 years ago
parent
commit
3ee2e90ab5

+ 16 - 10
src/ORM/Table.php

@@ -1337,6 +1337,7 @@ class Table implements RepositoryInterface, EventListenerInterface
             'associated' => true,
             'checkRules' => true,
             'checkExisting' => true,
+            '_primary' => true
         ]);
 
         if ($entity->errors()) {
@@ -1347,20 +1348,25 @@ class Table implements RepositoryInterface, EventListenerInterface
             return $entity;
         }
 
+        $connection = $this->connection();
         if ($options['atomic']) {
-            $connection = $this->connection();
             $success = $connection->transactional(function () use ($entity, $options) {
                 return $this->_processSave($entity, $options);
             });
-            if ($success) {
-                if (!$connection->inTransaction()) {
-                    $this->dispatchEvent('Model.afterSaveCommit', compact('entity', 'options'));
-                }
+        } else {
+            $success = $this->_processSave($entity, $options);
+        }
+
+        if ($success) {
+            if (!$connection->inTransaction() &&
+                ($options['atomic'] || (!$options['atomic'] && $options['_primary']))
+            ) {
+                $this->dispatchEvent('Model.afterSaveCommit', compact('entity', 'options'));
+            }
+            if ($options['atomic'] || $options['_primary']) {
                 $entity->isNew(false);
                 $entity->source($this->registryAlias());
             }
-        } else {
-            $success = $this->_processSave($entity, $options);
         }
 
         return $success;
@@ -1403,7 +1409,7 @@ class Table implements RepositoryInterface, EventListenerInterface
             $this,
             $entity,
             $options['associated'],
-            $options->getArrayCopy()
+            ['_primary' => false] + $options->getArrayCopy()
         );
 
         if (!$saved && $options['atomic']) {
@@ -1424,12 +1430,12 @@ class Table implements RepositoryInterface, EventListenerInterface
                 $this,
                 $entity,
                 $options['associated'],
-                $options->getArrayCopy()
+                ['_primary' => false] + $options->getArrayCopy()
             );
             if ($success || !$options['atomic']) {
                 $entity->clean();
                 $this->dispatchEvent('Model.afterSave', compact('entity', 'options'));
-                if (!$options['atomic']) {
+                if (!$options['atomic'] && !$options['_primary']) {
                     $entity->isNew(false);
                     $entity->source($this->registryAlias());
                 }

+ 2 - 0
tests/TestCase/ORM/RulesCheckerIntegrationTest.php

@@ -466,6 +466,7 @@ class RulesCheckerIntegrationTest extends TestCase
                         'associated' => true,
                         'checkRules' => true,
                         'checkExisting' => true,
+                        '_primary' => true,
                     ],
                     $options->getArrayCopy()
                 );
@@ -504,6 +505,7 @@ class RulesCheckerIntegrationTest extends TestCase
                         'associated' => true,
                         'checkRules' => true,
                         'checkExisting' => true,
+                        '_primary' => true,
                     ],
                     $options->getArrayCopy()
                 );

+ 29 - 3
tests/TestCase/ORM/TableTest.php

@@ -1779,7 +1779,7 @@ class TableTest extends TestCase
     }
 
     /**
-     * Asserts that afterSaveCommit is not triggered for non-atomic saves
+     * Asserts that afterSaveCommit is also triggered for non-atomic saves
      *
      * @return void
      */
@@ -1808,13 +1808,13 @@ class TableTest extends TestCase
         $this->assertSame($data, $table->save($data, ['atomic' => false]));
         $this->assertEquals($data->id, self::$nextUserId);
         $this->assertTrue($called);
-        $this->assertFalse($calledAfterCommit);
+        $this->assertTrue($calledAfterCommit);
     }
 
     /**
      * Asserts the afterSaveCommit is not triggered if transaction is running.
      *
-     * @return [type]
+     * @return void
      */
     public function testAfterSaveCommitWithTransactionRunning()
     {
@@ -1838,6 +1838,32 @@ class TableTest extends TestCase
     }
 
     /**
+     * Asserts the afterSaveCommit is not triggered if transaction is running.
+     *
+     * @return void
+     */
+    public function testAfterSaveCommitWithNonAtomicAndTransactionRunning()
+    {
+        $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, ['atomic' => false]));
+        $this->assertFalse($called);
+        $this->connection->commit();
+    }
+
+    /**
      * Asserts that afterSave callback not is called on unsuccessful save
      *
      * @group save