From ae8c7208f64fa4647709df0232ea8514a4299169 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 12 Jun 2020 18:39:58 -0700 Subject: [PATCH] Driver::connect() should throw only driver-level exceptions --- src/Connection.php | 14 +++-- .../PrimaryReadReplicaConnection.php | 8 ++- src/Driver.php | 3 ++ src/Driver/Mysqli/Driver.php | 7 +-- src/Driver/OCI8/Driver.php | 21 +++----- src/Driver/PDOMySql/Driver.php | 20 +++---- src/Driver/PDOOracle/Driver.php | 18 +++---- src/Driver/PDOPgSql/Driver.php | 54 +++++++++---------- src/Driver/PDOSqlite/Driver.php | 18 +++---- src/Driver/SQLAnywhere/Driver.php | 28 +++++----- tests/ConnectionTest.php | 5 +- 11 files changed, 86 insertions(+), 110 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index 6e430e7fca0..4f741bb3c3d 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -10,6 +10,7 @@ use Doctrine\DBAL\Cache\CachingResult; use Doctrine\DBAL\Cache\QueryCacheProfile; use Doctrine\DBAL\Driver\Connection as DriverConnection; +use Doctrine\DBAL\Driver\DriverException; use Doctrine\DBAL\Driver\PingableConnection; use Doctrine\DBAL\Driver\Result as DriverResult; use Doctrine\DBAL\Driver\ServerInfoAwareConnection; @@ -342,6 +343,8 @@ public function getExpressionBuilder() * * @return bool TRUE if the connection was successfully established, FALSE if * the connection is already open. + * + * @throws DBALException */ public function connect() { @@ -353,7 +356,12 @@ public function connect() $user = $this->params['user'] ?? null; $password = $this->params['password'] ?? null; - $this->_conn = $this->_driver->connect($this->params, $user, $password, $driverOptions); + try { + $this->_conn = $this->_driver->connect($this->params, $user, $password, $driverOptions); + } catch (DriverException $e) { + throw DBALException::driverException($this->_driver, $e); + } + $this->isConnected = true; $this->transactionNestingLevel = 0; @@ -420,7 +428,7 @@ private function getDatabasePlatformVersion() if ($this->_conn === null) { try { $this->connect(); - } catch (Throwable $originalException) { + } catch (DBALException $originalException) { if (! isset($this->params['dbname'])) { throw $originalException; } @@ -432,7 +440,7 @@ private function getDatabasePlatformVersion() try { $this->connect(); - } catch (Throwable $fallbackException) { + } catch (DBALException $fallbackException) { // Either the platform does not support database-less connections // or something else went wrong. // Reset connection parameters and rethrow the original exception. diff --git a/src/Connections/PrimaryReadReplicaConnection.php b/src/Connections/PrimaryReadReplicaConnection.php index df0ffe72527..8e3c6ee136b 100644 --- a/src/Connections/PrimaryReadReplicaConnection.php +++ b/src/Connections/PrimaryReadReplicaConnection.php @@ -5,8 +5,10 @@ use Doctrine\Common\EventManager; use Doctrine\DBAL\Configuration; use Doctrine\DBAL\Connection; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver; use Doctrine\DBAL\Driver\Connection as DriverConnection; +use Doctrine\DBAL\Driver\DriverException; use Doctrine\DBAL\Driver\Result; use Doctrine\DBAL\Driver\Statement; use Doctrine\DBAL\Event\ConnectionEventArgs; @@ -230,7 +232,11 @@ protected function connectTo($connectionName) $user = $connectionParams['user'] ?? null; $password = $connectionParams['password'] ?? null; - return $this->_driver->connect($connectionParams, $user, $password, $driverOptions); + try { + return $this->_driver->connect($connectionParams, $user, $password, $driverOptions); + } catch (DriverException $e) { + throw DBALException::driverException($this->_driver, $e); + } } /** diff --git a/src/Driver.php b/src/Driver.php index 57493e633bb..11aa6ecdf61 100644 --- a/src/Driver.php +++ b/src/Driver.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL; +use Doctrine\DBAL\Driver\DriverException; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Schema\AbstractSchemaManager; @@ -22,6 +23,8 @@ interface Driver * @param mixed[] $driverOptions The driver options to use when connecting. * * @return \Doctrine\DBAL\Driver\Connection The database connection. + * + * @throws DriverException */ public function connect(array $params, $username = null, $password = null, array $driverOptions = []); diff --git a/src/Driver/Mysqli/Driver.php b/src/Driver/Mysqli/Driver.php index 1166e8d4c2e..4430482dfc9 100644 --- a/src/Driver/Mysqli/Driver.php +++ b/src/Driver/Mysqli/Driver.php @@ -2,7 +2,6 @@ namespace Doctrine\DBAL\Driver\Mysqli; -use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\AbstractMySQLDriver; class Driver extends AbstractMySQLDriver @@ -12,11 +11,7 @@ class Driver extends AbstractMySQLDriver */ public function connect(array $params, $username = null, $password = null, array $driverOptions = []) { - try { - return new MysqliConnection($params, (string) $username, (string) $password, $driverOptions); - } catch (MysqliException $e) { - throw DBALException::driverException($this, $e); - } + return new MysqliConnection($params, (string) $username, (string) $password, $driverOptions); } /** diff --git a/src/Driver/OCI8/Driver.php b/src/Driver/OCI8/Driver.php index b749154845e..e2df60e2d52 100644 --- a/src/Driver/OCI8/Driver.php +++ b/src/Driver/OCI8/Driver.php @@ -2,7 +2,6 @@ namespace Doctrine\DBAL\Driver\OCI8; -use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\AbstractOracleDriver; use const OCI_NO_AUTO_COMMIT; @@ -17,18 +16,14 @@ class Driver extends AbstractOracleDriver */ public function connect(array $params, $username = null, $password = null, array $driverOptions = []) { - try { - return new OCI8Connection( - (string) $username, - (string) $password, - $this->_constructDsn($params), - $params['charset'] ?? '', - $params['sessionMode'] ?? OCI_NO_AUTO_COMMIT, - $params['persistent'] ?? false - ); - } catch (OCI8Exception $e) { - throw DBALException::driverException($this, $e); - } + return new OCI8Connection( + (string) $username, + (string) $password, + $this->_constructDsn($params), + $params['charset'] ?? '', + $params['sessionMode'] ?? OCI_NO_AUTO_COMMIT, + $params['persistent'] ?? false + ); } /** diff --git a/src/Driver/PDOMySql/Driver.php b/src/Driver/PDOMySql/Driver.php index 8266a5b915c..3beb43bc7a1 100644 --- a/src/Driver/PDOMySql/Driver.php +++ b/src/Driver/PDOMySql/Driver.php @@ -2,11 +2,9 @@ namespace Doctrine\DBAL\Driver\PDOMySql; -use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\AbstractMySQLDriver; use Doctrine\DBAL\Driver\PDOConnection; use PDO; -use PDOException; /** * PDO MySql driver. @@ -22,18 +20,12 @@ public function connect(array $params, $username = null, $password = null, array $driverOptions[PDO::ATTR_PERSISTENT] = true; } - try { - $conn = new PDOConnection( - $this->constructPdoDsn($params), - $username, - $password, - $driverOptions - ); - } catch (PDOException $e) { - throw DBALException::driverException($this, $e); - } - - return $conn; + return new PDOConnection( + $this->constructPdoDsn($params), + $username, + $password, + $driverOptions + ); } /** diff --git a/src/Driver/PDOOracle/Driver.php b/src/Driver/PDOOracle/Driver.php index 942cf6740dc..3267b6bf823 100644 --- a/src/Driver/PDOOracle/Driver.php +++ b/src/Driver/PDOOracle/Driver.php @@ -2,11 +2,9 @@ namespace Doctrine\DBAL\Driver\PDOOracle; -use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\AbstractOracleDriver; use Doctrine\DBAL\Driver\PDOConnection; use PDO; -use PDOException; /** * PDO Oracle driver. @@ -27,16 +25,12 @@ public function connect(array $params, $username = null, $password = null, array $driverOptions[PDO::ATTR_PERSISTENT] = true; } - try { - return new PDOConnection( - $this->constructPdoDsn($params), - $username, - $password, - $driverOptions - ); - } catch (PDOException $e) { - throw DBALException::driverException($this, $e); - } + return new PDOConnection( + $this->constructPdoDsn($params), + $username, + $password, + $driverOptions + ); } /** diff --git a/src/Driver/PDOPgSql/Driver.php b/src/Driver/PDOPgSql/Driver.php index 9202c0b00ed..cd352fc636f 100644 --- a/src/Driver/PDOPgSql/Driver.php +++ b/src/Driver/PDOPgSql/Driver.php @@ -2,11 +2,9 @@ namespace Doctrine\DBAL\Driver\PDOPgSql; -use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\AbstractPostgreSQLDriver; use Doctrine\DBAL\Driver\PDOConnection; use PDO; -use PDOException; use function defined; @@ -24,35 +22,31 @@ public function connect(array $params, $username = null, $password = null, array $driverOptions[PDO::ATTR_PERSISTENT] = true; } - try { - $connection = new PDOConnection( - $this->_constructPdoDsn($params), - $username, - $password, - $driverOptions - ); - - if ( - defined('PDO::PGSQL_ATTR_DISABLE_PREPARES') - && (! isset($driverOptions[PDO::PGSQL_ATTR_DISABLE_PREPARES]) - || $driverOptions[PDO::PGSQL_ATTR_DISABLE_PREPARES] === true - ) - ) { - $connection->getWrappedConnection()->setAttribute(PDO::PGSQL_ATTR_DISABLE_PREPARES, true); - } - - /* defining client_encoding via SET NAMES to avoid inconsistent DSN support - * - the 'client_encoding' connection param only works with postgres >= 9.1 - * - passing client_encoding via the 'options' param breaks pgbouncer support - */ - if (isset($params['charset'])) { - $connection->exec('SET NAMES \'' . $params['charset'] . '\''); - } - - return $connection; - } catch (PDOException $e) { - throw DBALException::driverException($this, $e); + $connection = new PDOConnection( + $this->_constructPdoDsn($params), + $username, + $password, + $driverOptions + ); + + if ( + defined('PDO::PGSQL_ATTR_DISABLE_PREPARES') + && (! isset($driverOptions[PDO::PGSQL_ATTR_DISABLE_PREPARES]) + || $driverOptions[PDO::PGSQL_ATTR_DISABLE_PREPARES] === true + ) + ) { + $connection->getWrappedConnection()->setAttribute(PDO::PGSQL_ATTR_DISABLE_PREPARES, true); } + + /* defining client_encoding via SET NAMES to avoid inconsistent DSN support + * - the 'client_encoding' connection param only works with postgres >= 9.1 + * - passing client_encoding via the 'options' param breaks pgbouncer support + */ + if (isset($params['charset'])) { + $connection->exec('SET NAMES \'' . $params['charset'] . '\''); + } + + return $connection; } /** diff --git a/src/Driver/PDOSqlite/Driver.php b/src/Driver/PDOSqlite/Driver.php index 76c49fda32c..779c2576cb8 100644 --- a/src/Driver/PDOSqlite/Driver.php +++ b/src/Driver/PDOSqlite/Driver.php @@ -2,11 +2,9 @@ namespace Doctrine\DBAL\Driver\PDOSqlite; -use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\AbstractSQLiteDriver; use Doctrine\DBAL\Driver\PDOConnection; use Doctrine\DBAL\Platforms\SqlitePlatform; -use PDOException; use function array_merge; @@ -35,16 +33,12 @@ public function connect(array $params, $username = null, $password = null, array unset($driverOptions['userDefinedFunctions']); } - try { - $connection = new PDOConnection( - $this->_constructPdoDsn($params), - $username, - $password, - $driverOptions - ); - } catch (PDOException $ex) { - throw DBALException::driverException($this, $ex); - } + $connection = new PDOConnection( + $this->_constructPdoDsn($params), + $username, + $password, + $driverOptions + ); $pdo = $connection->getWrappedConnection(); diff --git a/src/Driver/SQLAnywhere/Driver.php b/src/Driver/SQLAnywhere/Driver.php index 9d8641093b1..fd4ad27c350 100644 --- a/src/Driver/SQLAnywhere/Driver.php +++ b/src/Driver/SQLAnywhere/Driver.php @@ -21,22 +21,18 @@ class Driver extends AbstractSQLAnywhereDriver */ public function connect(array $params, $username = null, $password = null, array $driverOptions = []) { - try { - return new SQLAnywhereConnection( - $this->buildDsn( - $params['host'] ?? null, - $params['port'] ?? null, - $params['server'] ?? null, - $params['dbname'] ?? null, - $username, - $password, - $driverOptions - ), - $params['persistent'] ?? false - ); - } catch (SQLAnywhereException $e) { - throw DBALException::driverException($this, $e); - } + return new SQLAnywhereConnection( + $this->buildDsn( + $params['host'] ?? null, + $params['port'] ?? null, + $params['server'] ?? null, + $params['dbname'] ?? null, + $username, + $password, + $driverOptions + ), + $params['persistent'] ?? false + ); } /** diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index 7a35d980b8c..f2eebddc0d0 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -20,7 +20,6 @@ use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\VersionAwarePlatformDriver; -use Exception; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use stdClass; @@ -788,8 +787,8 @@ public function testRethrowsOriginalExceptionOnDeterminingPlatformWhenConnecting $driverMock = $this->createMock(VersionAwarePlatformDriver::class); $connection = new Connection(['dbname' => 'foo'], $driverMock); - $originalException = new Exception('Original exception'); - $fallbackException = new Exception('Fallback exception'); + $originalException = new DBALException('Original exception'); + $fallbackException = new DBALException('Fallback exception'); $driverMock->expects(self::at(0)) ->method('connect')