Merge pull request #3480 from morozov/transaction-void

Transaction-related Statement methods return void

UPGRADE.md

@@ -1,5 +1,9 @@
 # Upgrade to 3.0
 
+## BC BREAK Transaction-related `Statement` methods return `void`.
+
+`Statement::beginTransaction()`, `::commit()` and `::rollBack()` no longer return a boolean value. They will throw a `DriverException` in case of failure.
+
 ## MINOR BC BREAK `Statement::fetchColumn()` with an invalid index.
 
 Similarly to `PDOStatement::fetchColumn()`, DBAL statements throw an exception in case of an invalid column index.

lib/Doctrine/DBAL/Connection.php

@@ -9,6 +9,7 @@
 use Doctrine\DBAL\Cache\QueryCacheProfile;
 use Doctrine\DBAL\Cache\ResultCacheStatement;
 use Doctrine\DBAL\Driver\Connection as DriverConnection;
+use Doctrine\DBAL\Driver\DriverException;
 use Doctrine\DBAL\Driver\PingableConnection;
 use Doctrine\DBAL\Driver\ResultStatement;
 use Doctrine\DBAL\Driver\ServerInfoAwareConnection;
@@ -493,12 +494,11 @@ public function isAutoCommit()
      *
      * @see   isAutoCommit
      *
-     * @param bool $autoCommit True to enable auto-commit mode; false to disable it.
+     * @throws ConnectionException
+     * @throws DriverException
      */
-    public function setAutoCommit($autoCommit)
+    public function setAutoCommit(bool $autoCommit) : void
     {
-        $autoCommit = (bool) $autoCommit;
-
         // Mode not changed, no-op.
         if ($autoCommit === $this->autoCommit) {
             return;
@@ -1178,7 +1178,7 @@ protected function _getNestedTransactionSavePointName()
     /**
      * {@inheritDoc}
      */
-    public function beginTransaction()
+    public function beginTransaction() : void
     {
         $connection = $this->getWrappedConnection();
 
@@ -1188,15 +1188,17 @@ public function beginTransaction()
 
         if ($this->transactionNestingLevel === 1) {
             $logger->startQuery('"START TRANSACTION"');
-            $connection->beginTransaction();
-            $logger->stopQuery();
+
+            try {
+                $connection->beginTransaction();
+            } finally {
+                $logger->stopQuery();
+            }
         } elseif ($this->nestTransactionsWithSavepoints) {
             $logger->startQuery('"SAVEPOINT"');
             $this->createSavepoint($this->_getNestedTransactionSavePointName());
             $logger->stopQuery();
         }
-
-        return true;
     }
 
     /**
@@ -1205,11 +1207,12 @@ public function beginTransaction()
      * @throws ConnectionException If the commit failed due to no active transaction or
      *                                            because the transaction was marked for rollback only.
      */
-    public function commit()
+    public function commit() : void
     {
         if ($this->transactionNestingLevel === 0) {
             throw ConnectionException::noActiveTransaction();
         }
+
         if ($this->isRollbackOnly) {
             throw ConnectionException::commitFailedRollbackOnly();
         }
@@ -1220,8 +1223,12 @@ public function commit()
 
         if ($this->transactionNestingLevel === 1) {
             $logger->startQuery('"COMMIT"');
-            $connection->commit();
-            $logger->stopQuery();
+
+            try {
+                $connection->commit();
+            } finally {
+                $logger->stopQuery();
+            }
         } elseif ($this->nestTransactionsWithSavepoints) {
             $logger->startQuery('"RELEASE SAVEPOINT"');
             $this->releaseSavepoint($this->_getNestedTransactionSavePointName());
@@ -1231,18 +1238,19 @@ public function commit()
         --$this->transactionNestingLevel;
 
         if ($this->autoCommit !== false || $this->transactionNestingLevel !== 0) {
-            return true;
+            return;
         }
 
         $this->beginTransaction();
-
-        return true;
     }
 
     /**
      * Commits all current nesting transactions.
+     *
+     * @throws ConnectionException
+     * @throws DriverException
      */
-    private function commitAll()
+    private function commitAll() : void
     {
         while ($this->transactionNestingLevel !== 0) {
             if ($this->autoCommit === false && $this->transactionNestingLevel === 1) {
@@ -1258,11 +1266,11 @@ private function commitAll()
     }
 
     /**
-     * Cancels any database changes done during the current transaction.
+     * {@inheritDoc}
      *
      * @throws ConnectionException If the rollback operation failed.
      */
-    public function rollBack()
+    public function rollBack() : void
     {
         if ($this->transactionNestingLevel === 0) {
             throw ConnectionException::noActiveTransaction();
@@ -1275,12 +1283,16 @@ public function rollBack()
         if ($this->transactionNestingLevel === 1) {
             $logger->startQuery('"ROLLBACK"');
             $this->transactionNestingLevel = 0;
-            $connection->rollBack();
-            $this->isRollbackOnly = false;
-            $logger->stopQuery();
 
-            if ($this->autoCommit === false) {
-                $this->beginTransaction();
+            try {
+                $connection->rollBack();
+            } finally {
+                $this->isRollbackOnly = false;
+                $logger->stopQuery();
+
+                if ($this->autoCommit === false) {
+                    $this->beginTransaction();
+                }
             }
         } elseif ($this->nestTransactionsWithSavepoints) {
             $logger->startQuery('"ROLLBACK TO SAVEPOINT"');

lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php

@@ -230,31 +230,31 @@ public function executeUpdate(string $query, array $params = [], array $types =
     /**
      * {@inheritDoc}
      */
-    public function beginTransaction()
+    public function beginTransaction() : void
     {
         $this->connect('master');
 
-        return parent::beginTransaction();
+        parent::beginTransaction();
     }
 
     /**
      * {@inheritDoc}
      */
-    public function commit()
+    public function commit() : void
     {
         $this->connect('master');
 
-        return parent::commit();
+        parent::commit();
     }
 
     /**
      * {@inheritDoc}
      */
-    public function rollBack()
+    public function rollBack() : void
     {
         $this->connect('master');
 
-        return parent::rollBack();
+        parent::rollBack();
     }
 
     /**

lib/Doctrine/DBAL/Driver/Connection.php

@@ -54,23 +54,23 @@ public function lastInsertId($name = null);
     /**
      * Initiates a transaction.
      *
-     * @return bool TRUE on success or FALSE on failure.
+     * @throws DriverException
      */
-    public function beginTransaction();
+    public function beginTransaction() : void;
 
     /**
      * Commits a transaction.
      *
-     * @return bool TRUE on success or FALSE on failure.
+     * @throws DriverException
      */
-    public function commit();
+    public function commit() : void;
 
     /**
      * Rolls back the current transaction, as initiated by beginTransaction().
      *
-     * @return bool TRUE on success or FALSE on failure.
+     * @throws DriverException
      */
-    public function rollBack();
+    public function rollBack() : void;
 
     /**
      * Returns the error code associated with the last operation on the database handle.

lib/Doctrine/DBAL/Driver/IBMDB2/DB2Connection.php

@@ -137,31 +137,39 @@ public function lastInsertId($name = null)
     /**
      * {@inheritdoc}
      */
-    public function beginTransaction()
+    public function beginTransaction() : void
     {
-        db2_autocommit($this->conn, DB2_AUTOCOMMIT_OFF);
+        if (! db2_autocommit($this->conn, DB2_AUTOCOMMIT_OFF)) {
+            throw new DB2Exception(db2_conn_errormsg($this->conn));
+        }
     }
 
     /**
      * {@inheritdoc}
      */
-    public function commit()
+    public function commit() : void
     {
         if (! db2_commit($this->conn)) {
             throw new DB2Exception(db2_conn_errormsg($this->conn));
         }
-        db2_autocommit($this->conn, DB2_AUTOCOMMIT_ON);
+
+        if (! db2_autocommit($this->conn, DB2_AUTOCOMMIT_ON)) {
+            throw new DB2Exception(db2_conn_errormsg($this->conn));
+        }
     }
 
     /**
      * {@inheritdoc}
      */
-    public function rollBack()
+    public function rollBack() : void
     {
         if (! db2_rollback($this->conn)) {
             throw new DB2Exception(db2_conn_errormsg($this->conn));
         }
-        db2_autocommit($this->conn, DB2_AUTOCOMMIT_ON);
+
+        if (! db2_autocommit($this->conn, DB2_AUTOCOMMIT_ON)) {
+            throw new DB2Exception(db2_conn_errormsg($this->conn));
+        }
     }
 
     /**

lib/Doctrine/DBAL/Driver/IBMDB2/DB2Exception.php

@@ -2,8 +2,8 @@
 
 namespace Doctrine\DBAL\Driver\IBMDB2;
 
-use Exception;
+use Doctrine\DBAL\Driver\AbstractDriverException;
 
-class DB2Exception extends Exception
+class DB2Exception extends AbstractDriverException
 {
 }

lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php

@@ -174,27 +174,29 @@ public function lastInsertId($name = null)
     /**
      * {@inheritdoc}
      */
-    public function beginTransaction()
+    public function beginTransaction() : void
     {
         $this->conn->query('START TRANSACTION');
-
-        return true;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function commit()
+    public function commit() : void
     {
-        return $this->conn->commit();
+        if (! $this->conn->commit()) {
+            throw new MysqliException($this->conn->error, $this->conn->sqlstate, $this->conn->errno);
+        }
     }
 
     /**
-     * {@inheritdoc}non-PHPdoc)
+     * {@inheritdoc}
      */
-    public function rollBack()
+    public function rollBack() : void
     {
-        return $this->conn->rollback();
+        if (! $this->conn->rollback()) {
+            throw new MysqliException($this->conn->error, $this->conn->sqlstate, $this->conn->errno);
+        }
     }
 
     /**

lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php

@@ -177,37 +177,33 @@ public function getExecuteMode()
     /**
      * {@inheritdoc}
      */
-    public function beginTransaction()
+    public function beginTransaction() : void
     {
         $this->executeMode = OCI_NO_AUTO_COMMIT;
-
-        return true;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function commit()
+    public function commit() : void
     {
         if (! oci_commit($this->dbh)) {
             throw OCI8Exception::fromErrorInfo($this->errorInfo());
         }
-        $this->executeMode = OCI_COMMIT_ON_SUCCESS;
 
-        return true;
+        $this->executeMode = OCI_COMMIT_ON_SUCCESS;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function rollBack()
+    public function rollBack() : void
     {
         if (! oci_rollback($this->dbh)) {
             throw OCI8Exception::fromErrorInfo($this->errorInfo());
         }
-        $this->executeMode = OCI_COMMIT_ON_SUCCESS;
 
-        return true;
+        $this->executeMode = OCI_COMMIT_ON_SUCCESS;
     }
 
     /**

lib/Doctrine/DBAL/Driver/PDOConnection.php

@@ -126,25 +126,25 @@ protected function createStatement(\PDOStatement $stmt) : PDOStatement
     /**
      * {@inheritDoc}
      */
-    public function beginTransaction()
+    public function beginTransaction() : void
     {
-        return $this->connection->beginTransaction();
+        $this->connection->beginTransaction();
     }
 
     /**
      * {@inheritDoc}
      */
-    public function commit()
+    public function commit() : void
     {
-        return $this->connection->commit();
+        $this->connection->commit();
     }
 
     /**
      * {@inheritDoc}
      */
-    public function rollBack()
+    public function rollBack() : void
     {
-        return $this->connection->rollBack();
+        $this->connection->rollBack();
     }
 
     /**