DBAL-55 - Refactored a bit and expaned tests

lib/Doctrine/DBAL/Connection.php

@@ -751,6 +751,7 @@ class Connection implements DriverConnection
      * Set if nested transactions should use savepoints
      *
      * @param boolean
+     * @return void
      */
     public function setNestTransactionsWithSavepoints($nestTransactionsWithSavepoints)
     {
@@ -758,8 +759,8 @@ class Connection implements DriverConnection
             throw ConnectionException::mayNotAlterNestedTransactionWithSavepointsInTransaction();
         }
 
-        if ($nestTransactionsWithSavepoints && !$this->_platform->supportsSavepoints()) {
-            ConnectionException::savepointsNotSupported();
+        if (!$this->_platform->supportsSavepoints()) {
+            throw ConnectionException::savepointsNotSupported();
         }
 
         $this->_nestTransactionsWithSavepoints = $nestTransactionsWithSavepoints;
@@ -781,12 +782,9 @@ class Connection implements DriverConnection
      *
      * @return mixed a string with the savepoint name or false
      */
-    protected function _getNestedTransactionSavePointName() {
-        if ($this->_platform->supportsSavepoints() && !empty($this->_nestTransactionsWithSavepoints)) {
-            return 'DOCTRINE2_SAVEPOINT_'.$this->_transactionNestingLevel;
-        }
-
-        return false;
+    protected function _getNestedTransactionSavePointName()
+    {
+        return 'DOCTRINE2_SAVEPOINT_'.$this->_transactionNestingLevel;
     }
 
     /**
@@ -802,11 +800,8 @@ class Connection implements DriverConnection
 
         if ($this->_transactionNestingLevel == 1) {
             $this->_conn->beginTransaction();
-        } else {
-            $savepointName = $this->_getNestedTransactionSavePointName();
-            if ($savepointName) {
-                $this->createSavePoint($savepointName);
-            }
+        } else if ($this->_nestTransactionsWithSavepoints) {
+            $this->createSavepoint($this->_getNestedTransactionSavePointName());
         }
     }
 
@@ -830,11 +825,8 @@ class Connection implements DriverConnection
 
         if ($this->_transactionNestingLevel == 1) {
             $this->_conn->commit();
-        } else {
-            $savepointName = $this->_getNestedTransactionSavePointName();
-            if ($savepointName && $this->_platform->supportsReleaseSavepoints()) {
-                $this->releaseSavePoint($savepointName);
-            }
+        } else if ($this->_nestTransactionsWithSavepoints) {
+            $this->releaseSavepoint($this->_getNestedTransactionSavePointName());
         }
 
         --$this->_transactionNestingLevel;
@@ -860,13 +852,11 @@ class Connection implements DriverConnection
             $this->_transactionNestingLevel = 0;
             $this->_conn->rollback();
             $this->_isRollbackOnly = false;
+        } else if ($this->_nestTransactionsWithSavepoints) {
+            $this->rollbackSavepoint($this->_getNestedTransactionSavePointName());
+            --$this->_transactionNestingLevel;
         } else {
-            $savepointName = $this->_getNestedTransactionSavePointName();
-            if (!$this->_isRollbackOnly && $savepointName) {
-                $this->rollbackSavePoint($savepointName);
-            } else {
-                $this->_isRollbackOnly = true;
-            }
+            $this->_isRollbackOnly = true;
             --$this->_transactionNestingLevel;
         }
     }
@@ -878,10 +868,10 @@ class Connection implements DriverConnection
      * @param string $savepoint     name of a savepoint to set
      * @return void
      */
-    public function createSavePoint($savepoint)
+    public function createSavepoint($savepoint)
     {
         if (!$this->_platform->supportsSavepoints()) {
-            ConnectionException::savepointsNotSupported();
+            throw ConnectionException::savepointsNotSupported();
         }
 
         $this->_conn->exec($this->_platform->createSavePoint($savepoint));
@@ -894,10 +884,10 @@ class Connection implements DriverConnection
      * @param string $savepoint     name of a savepoint to release
      * @return void
      */
-    public function releaseSavePoint($savepoint)
+    public function releaseSavepoint($savepoint)
     {
         if (!$this->_platform->supportsSavepoints()) {
-            ConnectionException::savepointsNotSupported();
+            throw ConnectionException::savepointsNotSupported();
         }
 
         if ($this->_platform->supportsReleaseSavepoints()) {
@@ -912,10 +902,10 @@ class Connection implements DriverConnection
      * @param string $savepoint     name of a savepoint to rollback to
      * @return void
      */
-    public function rollbackSavePoint($savepoint)
+    public function rollbackSavepoint($savepoint)
     {
         if (!$this->_platform->supportsSavepoints()) {
-            ConnectionException::savepointsNotSupported();
+            throw ConnectionException::savepointsNotSupported();
         }
 
         $this->_conn->exec($this->_platform->rollbackSavePoint($savepoint));

lib/Doctrine/DBAL/ConnectionException.php

@@ -44,7 +44,7 @@ class ConnectionException extends DBALException
 
     public static function savepointsNotSupported()
     {
-        return new self("Savepoints are not supported transaction.");
+        return new self("Savepoints are not supported by this driver.");
     }
 
     public static function mayNotAlterNestedTransactionWithSavepointsInTransaction()

tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php

@@ -14,6 +14,12 @@ class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase
         parent::setUp();
     }
 
+    public function tearDown()
+    {
+        parent::tearDown();
+        $this->resetSharedConn();
+    }
+
     public function testGetWrappedConnection()
     {
         $this->assertType('Doctrine\DBAL\Driver\Connection', $this->_conn->getWrappedConnection());
@@ -93,6 +99,65 @@ class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase
         }
     }
 
+    public function testTransactionNestingBehaviorCantBeChangedInActiveTransaction()
+    {
+        if (!$this->_conn->getDatabasePlatform()->supportsSavepoints()) {
+            $this->markTestSkipped('This test requires the platform to support savepoints.');
+        }
+
+        $this->_conn->beginTransaction();
+        try {
+            $this->_conn->setNestTransactionsWithSavepoints(true);
+            $this->fail('An exception should have been thrown by chaning the nesting transaction behavior within an transaction.');
+        } catch(ConnectionException $e) {
+            $this->_conn->rollBack();
+        }
+    }
+
+    public function testSetNestedTransactionsThroughSavepointsNotSupportedThrowsException()
+    {
+        if ($this->_conn->getDatabasePlatform()->supportsSavepoints()) {
+            $this->markTestSkipped('This test requires the platform not to support savepoints.');
+        }
+
+        $this->setExpectedException('Doctrine\DBAL\ConnectionException', "Savepoints are not supported by this driver.");
+
+        $this->_conn->setNestTransactionsWithSavepoints(true);
+    }
+
+    public function testCreateSavepointsNotSupportedThrowsException()
+    {
+        if ($this->_conn->getDatabasePlatform()->supportsSavepoints()) {
+            $this->markTestSkipped('This test requires the platform not to support savepoints.');
+        }
+
+        $this->setExpectedException('Doctrine\DBAL\ConnectionException', "Savepoints are not supported by this driver.");
+
+        $this->_conn->createSavepoint('foo');
+    }
+
+    public function testReleaseSavepointsNotSupportedThrowsException()
+    {
+        if ($this->_conn->getDatabasePlatform()->supportsSavepoints()) {
+            $this->markTestSkipped('This test requires the platform not to support savepoints.');
+        }
+
+        $this->setExpectedException('Doctrine\DBAL\ConnectionException', "Savepoints are not supported by this driver.");
+
+        $this->_conn->releaseSavepoint('foo');
+    }
+
+    public function testRollbackSavepointsNotSupportedThrowsException()
+    {
+        if ($this->_conn->getDatabasePlatform()->supportsSavepoints()) {
+            $this->markTestSkipped('This test requires the platform not to support savepoints.');
+        }
+
+        $this->setExpectedException('Doctrine\DBAL\ConnectionException', "Savepoints are not supported by this driver.");
+
+        $this->_conn->rollbackSavepoint('foo');
+    }
+
     public function testTransactionBehaviorWithRollback()
     {
         try {
@@ -101,7 +166,7 @@ class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase
             
             throw new \Exception;
               
-            $this->_connx->commit(); // never reached
+            $this->_conn->commit(); // never reached
         } catch (\Exception $e) {
             $this->assertEquals(1, $this->_conn->getTransactionNestingLevel());
             $this->_conn->rollback();