From 9455fc1dcab4d1d18d1073855ef695cf8ab7e684 Mon Sep 17 00:00:00 2001 From: Mathieu Rochette Date: Tue, 1 Dec 2015 16:38:42 +0100 Subject: [PATCH] avoid having nested Doctrine\DBAL\Connection-s only nest Doctrine\DBAL\Driver\Connection-s --- composer.json | 10 +---- features/bootstrap/FeatureContext.php | 38 +++++----------- features/bootstrap/MySQLContext.php | 22 ++++++--- features/bootstrap/PostgreSQLContext.php | 10 ++--- features/retry.feature | 2 +- src/Driver/Connection/CallAndRetry.php | 34 ++++---------- .../Connection/MasterSlavesConnection.php | 28 ++++++------ src/Driver/Connection/RetryConnection.php | 45 ++++++++----------- src/Driver/Connection/RetryStatement.php | 29 ++---------- src/Driver/MasterSlavesDriver.php | 8 +++- src/Driver/RetryDriver.php | 1 - src/RetryStrategy.php | 8 ++-- src/RetryStrategy/MysqlRetryStrategy.php | 9 ++-- src/RetryStrategy/PostgreSQLRetryStrategy.php | 4 +- src/RetryStrategy/RetryStrategy.php | 20 ++++----- 15 files changed, 102 insertions(+), 166 deletions(-) diff --git a/composer.json b/composer.json index cc48894..a98ea01 100644 --- a/composer.json +++ b/composer.json @@ -13,15 +13,9 @@ } }, "require": { - "doctrine/dbal": "dev-avoid-over-nesting-exceptions as 2.5.x-dev" + "doctrine/dbal": "^2.5.2" }, "require-dev": { "behat/behat": "~3.0" - }, - "repositories": [ - { - "type": "vcs", - "url": "https://github.com/mathroc/dbal" - } - ] + } } diff --git a/features/bootstrap/FeatureContext.php b/features/bootstrap/FeatureContext.php index f434819..204fdb4 100644 --- a/features/bootstrap/FeatureContext.php +++ b/features/bootstrap/FeatureContext.php @@ -1,7 +1,8 @@ getWrappedConnection($connectionName); if ($connection instanceof Ez\DbLinker\Driver\Connection\RetryConnection) { - $connection = $connection->wrappedConnection()->getWrappedConnection(); + $connection = $connection->wrappedConnection(); } $connection->connectToMaster(); } @@ -315,7 +316,7 @@ public function databaseHasGoneAwayOn($connectionName) */ public function retryLimitShouldBe($connectionName, $n) { - $retryLimit = $this->getWrappedConnection($connectionName)->retryStrategy()->retryLimit(); + $retryLimit = $this->connections[$connectionName]['params']['retryStrategy']->retryLimit(); assert($retryLimit === (int) $n, "Retry limit is $retryLimit, $n expected."); } @@ -384,7 +385,7 @@ public function shouldHaveSlave($connectionName, $n) { $connection = $this->getWrappedConnection($connectionName); if ($connection instanceof \Ez\DbLinker\Driver\Connection\RetryConnection) { - $connection = $connection->wrappedConnection()->getWrappedConnection(); + $connection = $connection->wrappedConnection(); } $slaveCount = $connection->slaves()->count(); assert($slaveCount === (int)$n, "Slaves count is $slaveCount, $n expected."); @@ -446,18 +447,6 @@ public function theLastStatementSucceeded() } } - /** - * @Then the last statement error should be :errorName - */ - public function theLastStatementExpectedErrorCodeShouldBe($errorName) - { - $errorCode = $this->errorCode( - $this->lastStatementError ?: - $this->statement->connection->getWrappedConnection()->retryStrategy()->lastError() - ); - $this->errorCodeMatchesErrorName($errorCode, $errorName); - } - /** * @Then the last error should be :errorName on :connectionName */ @@ -465,7 +454,7 @@ public function theLastErrorShouldBeOn($errorName, $connectionName) { $errorCode = $this->errorCode( $this->connections[$connectionName]['last-error'] ?: - $this->getWrappedConnection($connectionName)->retryStrategy()->lastError() + $retryStrategy = $this->connections[$connectionName]['params']['retryStrategy']->lastError() ); $this->errorCodeMatchesErrorName($errorCode, $errorName); } @@ -481,7 +470,7 @@ private function errorCodeMatchesErrorName($errorCode, $errorName) } abstract protected function errorToCode($errorName); - abstract protected function errorCode($exception); + abstract protected function errorCode(Exception $exception); private function getWrappedConnection($connectionName) { @@ -507,15 +496,12 @@ abstract protected function masterParams($username = null, $password = ''); */ public function tableCanBeCreatedAutomaticallyOn($tableName, $connectionName) { - $connection = $this->getWrappedConnection($connectionName); - $connection->retryStrategy()->addHandler(function ( - Doctrine\DBAL\DBALException $exception, - Ez\DbLinker\Driver\Connection\RetryConnection $connection + $retryStrategy = $this->connections[$connectionName]['params']['retryStrategy']; + $retryStrategy->addHandler(function ( + Exception $exception, + RetryConnection $connection ) use ($tableName) { - if ( - $exception instanceof TableNotFoundException && - strpos($exception->getMessage(), $tableName) !== false - ) { + if (strpos($exception->getMessage(), $tableName) !== false) { $connection->exec("CREATE TABLE {$tableName} (id INT)"); return true; } diff --git a/features/bootstrap/MySQLContext.php b/features/bootstrap/MySQLContext.php index c9b57ad..d4fde75 100644 --- a/features/bootstrap/MySQLContext.php +++ b/features/bootstrap/MySQLContext.php @@ -2,6 +2,11 @@ use Doctrine\DBAL\DriverManager; +use Doctrine\DBAL\DBALException; +use Doctrine\DBAL\Driver\DriverException as DDriverException; +use Doctrine\DBAL\Exception\DriverException as EDriverException; +use Ez\DbLinker\Driver\Connection\RetryConnection; + trait MySQLContext { private function masterParams($username = null, $password = '') { @@ -71,9 +76,14 @@ private function retryStrategy($n) return new MysqlRetryStrategy($n); } - private function errorCode($exception) + private function errorCode(Exception $exception) { - return $exception->getErrorCode(); + if ($exception instanceof DBALException) { + $exception = $exception->getPrevious(); + } + if ($exception instanceof DDriverException || $exception instanceof EDriverException) { + return $exception->getErrorCode(); + } } private function retryDriverClass() @@ -116,15 +126,13 @@ class MysqlRetryStrategy extends Ez\DbLinker\RetryStrategy\MysqlRetryStrategy private $handlers = []; public function shouldRetry( - Doctrine\DBAL\DBALException $exception, - Ez\DbLinker\Driver\Connection\RetryConnection $connection, - $method, - Array $arguments + Exception $exception, + RetryConnection $connection ) { $this->lastError = $exception; return array_reduce($this->handlers, function($retry, Closure $handler) use ($exception, $connection) { return $retry || $handler($exception, $connection); - }, false) || parent::shouldRetry($exception, $connection, $method, $arguments); + }, false) || parent::shouldRetry($exception, $connection); } public function lastError() diff --git a/features/bootstrap/PostgreSQLContext.php b/features/bootstrap/PostgreSQLContext.php index 9fde370..fa12d3a 100644 --- a/features/bootstrap/PostgreSQLContext.php +++ b/features/bootstrap/PostgreSQLContext.php @@ -63,7 +63,7 @@ private function retryStrategy($n) } - private function errorCode($exception) + private function errorCode(Exception $exception) { if(preg_match("/SQLSTATE\[(?[A-Z0-9]*)\]/", $exception->getMessage(), $matches)) { return $matches["errorCode"]; @@ -113,15 +113,13 @@ class PostgreSQLRetryStrategy extends Ez\DbLinker\RetryStrategy\PostgreSQLRetryS private $handlers = []; public function shouldRetry( - Doctrine\DBAL\DBALException $exception, - Ez\DbLinker\Driver\Connection\RetryConnection $connection, - $method, - Array $arguments + Exception $exception, + RetryConnection $connection ) { $this->lastError = $exception; return array_reduce($this->handlers, function($retry, Closure $handler) use ($exception, $connection) { return $retry || $handler($exception, $connection); - }, false) || parent::shouldRetry($exception, $connection, $method, $arguments); + }, false) || parent::shouldRetry($exception, $connection); } public function lastError() diff --git a/features/retry.feature b/features/retry.feature index 2a39144..69970d8 100644 --- a/features/retry.feature +++ b/features/retry.feature @@ -83,4 +83,4 @@ Feature: Retry When I prepare a statement "SELECT * FROM not_here_yet" on "conn" And I execute this statement Then the last statement succeeded - And the last statement error should be "NO_SUCH_TABLE" + And the last error should be "NO_SUCH_TABLE" on "conn" diff --git a/src/Driver/Connection/CallAndRetry.php b/src/Driver/Connection/CallAndRetry.php index 61f51f5..824fad1 100644 --- a/src/Driver/Connection/CallAndRetry.php +++ b/src/Driver/Connection/CallAndRetry.php @@ -2,26 +2,23 @@ namespace Ez\DbLinker\Driver\Connection; -use Doctrine\DBAL\DBALException; +use Exception; +use Ez\DbLinker\RetryStrategy; trait CallAndRetry { /** - * call $method woth $arguments and retry if necessary - * @param string $method method name - * @param array $arguments [description] + * call $callable and retry if necessary */ - private function callAndRetry($method, array $arguments) + private function callAndRetry(callable $callable, RetryStrategy $strategy, RetryConnection $connection) { do { try { - return @call_user_func_array([$this->wrappedObject(), $method], $arguments); - } catch (DBALException $exception) { - if (!$this->retryStrategy()->shouldRetry( + return @$callable(); + } catch (Exception $exception) { + if (!$strategy->shouldRetry( $exception, - $this->retryConnection(), - $method, - $arguments + $connection )) { // stop trying throw $exception; @@ -29,19 +26,4 @@ private function callAndRetry($method, array $arguments) } } while (true); } - - /** - * @return mixed - */ - abstract protected function wrappedObject(); - - /** - * @return Ez\DbLinker\RetryConnection - */ - abstract protected function retryConnection(); - - /** - * @return Ez\DbLinker\RetryStrategy - */ - abstract protected function retryStrategy(); } diff --git a/src/Driver/Connection/MasterSlavesConnection.php b/src/Driver/Connection/MasterSlavesConnection.php index 98e56ba..048470b 100644 --- a/src/Driver/Connection/MasterSlavesConnection.php +++ b/src/Driver/Connection/MasterSlavesConnection.php @@ -11,8 +11,9 @@ class MasterSlavesConnection implements Connection private $master; private $slavesWeights; private $connection; + private $currentConnectionFactory; - public function __construct(Connection $master, SplObjectStorage $slavesWeights) + public function __construct($master, SplObjectStorage $slavesWeights) { $this->master = $master; $this->checkSlavesWeights($slavesWeights); @@ -22,13 +23,7 @@ public function __construct(Connection $master, SplObjectStorage $slavesWeights) private function checkSlavesWeights(SplObjectStorage $slavesWeights) { foreach ($slavesWeights as $slave) { - $weight = $slavesWeights[$slave]; - if (!$slave instanceof Connection) { - throw new Exception( - 'All objects attached to $slavesWeights must implements Doctrine\DBAL\Driver\Connection' - ); - } - if ((int)$weight < 0) { + if ((int)$slavesWeights[$slave] < 0) { throw new Exception('Slave weight must be >= 0'); } } @@ -36,23 +31,29 @@ private function checkSlavesWeights(SplObjectStorage $slavesWeights) public function connectToMaster() { - $this->connection = $this->master; + $this->currentConnectionFactory = $this->master; + $this->connection = null; } public function connectToSlave() { + $this->currentConnectionFactory = null; $this->connection = null; } public function isConnectedToMaster() { - return $this->connection === $this->master; + return $this->currentConnectionFactory === $this->master; } private function connection() { if ($this->connection === null) { - $this->connection = $this->chooseASlave(); + if ($this->currentConnectionFactory === null) { + $this->currentConnectionFactory = $this->chooseASlave() ?: $this->master; + } + $factory = $this->currentConnectionFactory; + $this->connection = $factory(); } return $this->connection; } @@ -66,7 +67,7 @@ private function chooseASlave() { $totalSlavesWeight = $this->totalSlavesWeight(); if ($totalSlavesWeight < 1) { - return $this->master; + return null; } $weightTarget = mt_rand(1, $totalSlavesWeight); foreach ($this->slavesWeights as $slave) { @@ -88,7 +89,8 @@ private function totalSlavesWeight() public function disableCurrentSlave() { - $this->slavesWeights->detach($this->connection); + $this->slavesWeights->detach($this->currentConnectionFactory); + $this->currentConnectionFactory = null; $this->connection = null; } diff --git a/src/Driver/Connection/RetryConnection.php b/src/Driver/Connection/RetryConnection.php index 253421a..11e817f 100644 --- a/src/Driver/Connection/RetryConnection.php +++ b/src/Driver/Connection/RetryConnection.php @@ -30,7 +30,7 @@ public function __construct(Array $wrappedConnectionParams, RetryStrategy $retry */ public function prepare($prepareString) { return new RetryStatement( - $this->callAndRetry(__FUNCTION__, func_get_args()), + $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()), $this, $this->retryStrategy ); @@ -42,7 +42,7 @@ public function prepare($prepareString) { * @return \Doctrine\DBAL\Driver\Statement */ public function query() { - return $this->callAndRetry(__FUNCTION__, func_get_args()); + return $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()); } /** @@ -54,7 +54,7 @@ public function query() { * @return string */ public function quote($input, $type = \PDO::PARAM_STR) { - return $this->callAndRetry(__FUNCTION__, func_get_args()); + return $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()); } /** @@ -65,7 +65,7 @@ public function quote($input, $type = \PDO::PARAM_STR) { * @return integer */ public function exec($statement) { - return $this->callAndRetry(__FUNCTION__, func_get_args()); + return $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()); } /** @@ -76,7 +76,7 @@ public function exec($statement) { * @return string */ public function lastInsertId($name = null) { - return $this->callAndRetry(__FUNCTION__, func_get_args()); + return $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()); } /** @@ -86,7 +86,7 @@ public function lastInsertId($name = null) { */ public function beginTransaction() { $this->transactionLevel++; - return $this->callAndRetry(__FUNCTION__, func_get_args()); + return $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()); } /** @@ -96,7 +96,7 @@ public function beginTransaction() { */ public function commit() { $this->transactionLevel--; - return $this->callAndRetry(__FUNCTION__, func_get_args()); + return $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()); } /** @@ -106,7 +106,7 @@ public function commit() { */ public function rollBack() { $this->transactionLevel--; - return $this->callAndRetry(__FUNCTION__, func_get_args()); + return $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()); } /** @@ -115,7 +115,7 @@ public function rollBack() { * @return string|null The error code, or null if no operation has been run on the database handle. */ public function errorCode() { - return $this->callAndRetry(__FUNCTION__, func_get_args()); + return $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()); } /** @@ -124,15 +124,12 @@ public function errorCode() { * @return array */ public function errorInfo() { - return $this->callAndRetry(__FUNCTION__, func_get_args()); + return $this->callWrappedConnectionAndRetry(__FUNCTION__, func_get_args()); } public function close() { - if ($this->wrappedConnection !== null) { - $this->wrappedConnection->close(); - $this->wrappedConnection = null; - } + $this->wrappedConnection = null; } public function transactionLevel() @@ -140,26 +137,20 @@ public function transactionLevel() return $this->transactionLevel; } - private function wrappedObject() + private function callWrappedConnectionAndRetry($method, array $arguments) { - return $this->wrappedConnection(); + return $this->callAndRetry(function () use ($method, $arguments) { + return call_user_func_array([$this->wrappedConnection(), $method], $arguments); + }, $this->retryStrategy, $this); } public function wrappedConnection() { if ($this->wrappedConnection === null) { - $this->wrappedConnection = DriverManager::getConnection($this->wrappedConnectionParams); + $this->wrappedConnection = DriverManager::getConnection( + $this->wrappedConnectionParams + )->getWrappedConnection(); } return $this->wrappedConnection; } - - public function retryStrategy() - { - return $this->retryStrategy; - } - - public function retryConnection() - { - return $this; - } } diff --git a/src/Driver/Connection/RetryStatement.php b/src/Driver/Connection/RetryStatement.php index 891ab18..1e3b7df 100644 --- a/src/Driver/Connection/RetryStatement.php +++ b/src/Driver/Connection/RetryStatement.php @@ -4,7 +4,6 @@ use IteratorAggregate; use Doctrine\DBAL\Driver\Statement; -use Ez\DbLinker\Driver\Connection\RetryConnection; use Ez\DbLinker\RetryStrategy; class RetryStatement implements IteratorAggregate, Statement @@ -30,31 +29,9 @@ public function __construct(Statement $statement, RetryConnection $retryConnecti */ public function execute($params = NULL) { - return (bool)$this->callAndRetry(__FUNCTION__, func_get_args()); - } - - /** - * {@inheritdoc} - */ - private function wrappedObject() - { - return $this->statement; - } - - /** - * {@inheritdoc} - */ - private function retryConnection() - { - return $this->retryConnection; - } - - /** - * {@inheritdoc} - */ - private function retryStrategy() - { - return $this->retryStrategy; + return (bool)$this->callAndRetry(function () use ($params) { + return $this->statement->execute($params); + }, $this->retryStrategy, $this->retryConnection); } /** diff --git a/src/Driver/MasterSlavesDriver.php b/src/Driver/MasterSlavesDriver.php index fb8a523..d5afcae 100644 --- a/src/Driver/MasterSlavesDriver.php +++ b/src/Driver/MasterSlavesDriver.php @@ -23,11 +23,15 @@ trait MasterSlavesDriver public function connect(Array $params, $username = null, $password = null, Array $driverOptions = []) { $driverKey = array_key_exists('driverClass', $params['master']) ? 'driverClass' : 'driver'; $driverValue = $params['master'][$driverKey]; - $master = DriverManager::getConnection($params['master']); + $master = function () use ($params) { + return DriverManager::getConnection($params['master'])->getWrappedConnection(); + }; $slaves = new SplObjectStorage; foreach ($params['slaves'] as $slaveParams) { $slaveParams[$driverKey] = $driverValue; - $slaves->attach(DriverManager::getConnection($slaveParams), $slaveParams['weight']); + $slaves->attach(function () use ($slaveParams) { + return DriverManager::getConnection($slaveParams)->getWrappedConnection(); + }, $slaveParams['weight']); } return new MasterSlavesConnection($master, $slaves); } diff --git a/src/Driver/RetryDriver.php b/src/Driver/RetryDriver.php index 3b7a890..59d19cc 100644 --- a/src/Driver/RetryDriver.php +++ b/src/Driver/RetryDriver.php @@ -3,7 +3,6 @@ namespace Ez\DbLinker\Driver; use \Doctrine\DBAL\Connection; -use \Doctrine\DBAL\DriverManager; use Ez\DbLinker\Driver\Connection\RetryConnection; trait RetryDriver diff --git a/src/RetryStrategy.php b/src/RetryStrategy.php index 44515f9..31df7a1 100644 --- a/src/RetryStrategy.php +++ b/src/RetryStrategy.php @@ -2,15 +2,13 @@ namespace Ez\DbLinker; -use Doctrine\DBAL\DBALException; +use Exception; use Ez\DbLinker\Driver\Connection\RetryConnection; interface RetryStrategy { public function shouldRetry( - DBALException $exception, - RetryConnection $connection, - $method, - Array $arguments + Exception $exception, + RetryConnection $connection ); } diff --git a/src/RetryStrategy/MysqlRetryStrategy.php b/src/RetryStrategy/MysqlRetryStrategy.php index 8e0abf8..dbac066 100644 --- a/src/RetryStrategy/MysqlRetryStrategy.php +++ b/src/RetryStrategy/MysqlRetryStrategy.php @@ -2,8 +2,9 @@ namespace Ez\DbLinker\RetryStrategy; -use Doctrine\DBAL\DBALException; -use Doctrine\DBAL\Exception\DriverException; +use Exception; +use Doctrine\DBAL\Driver\DriverException as DDriverException; +use Doctrine\DBAL\Exception\DriverException as EDriverException; use Ez\DbLinker\RetryStrategy as RetryStrategyInterface; class MysqlRetryStrategy implements RetryStrategyInterface @@ -31,9 +32,9 @@ private function errorCodeStrategies() { ]; } - private function errorCode(DBALException $exception) + private function errorCode(Exception $exception) { - if ($exception instanceof DriverException) { + if ($exception instanceof DDriverException || $exception instanceof EDriverException) { return $exception->getErrorCode(); } } diff --git a/src/RetryStrategy/PostgreSQLRetryStrategy.php b/src/RetryStrategy/PostgreSQLRetryStrategy.php index 0b9058b..5faea5b 100644 --- a/src/RetryStrategy/PostgreSQLRetryStrategy.php +++ b/src/RetryStrategy/PostgreSQLRetryStrategy.php @@ -2,7 +2,7 @@ namespace Ez\DbLinker\RetryStrategy; -use Doctrine\DBAL\DBALException; +use Exception; use Ez\DbLinker\RetryStrategy as RetryStrategyInterface; class PostgreSQLRetryStrategy implements RetryStrategyInterface @@ -18,7 +18,7 @@ private function errorCodeStrategies() { ]; } - private function errorCode(DBALException $exception) + private function errorCode(Exception $exception) { if (preg_match("/SQLSTATE\[(?[A-Z0-9]*)\]/", $exception->getMessage(), $matches)) { return $matches["errorCode"]; diff --git a/src/RetryStrategy/RetryStrategy.php b/src/RetryStrategy/RetryStrategy.php index f4ee663..eb32761 100644 --- a/src/RetryStrategy/RetryStrategy.php +++ b/src/RetryStrategy/RetryStrategy.php @@ -2,8 +2,8 @@ namespace Ez\DbLinker\RetryStrategy; +use Exception; use stdClass; -use Doctrine\DBAL\DBALException; use Ez\DbLinker\Driver\Connection\MasterSlavesConnection; use Ez\DbLinker\Driver\Connection\RetryConnection; @@ -16,17 +16,13 @@ public function __construct($retryLimit = INF) $this->retryLimit = $retryLimit; } - public function shouldRetry( - DBALException $exception, - RetryConnection $connection, - $method, - array $arguments - ) { + public function shouldRetry(Exception $exception, RetryConnection $connection) { if (!$this->canRetry($connection)) { return false; } $strategy = $this->errorCodeStrategy($this->errorCode($exception)); - return $this->applyStrategy($strategy, $connection); + $res = $this->applyStrategy($strategy, $connection); + return $res; } public function retryLimit() @@ -34,9 +30,9 @@ public function retryLimit() return $this->retryLimit > 0 ? (int) $this->retryLimit : 0; } - private function canRetry(RetryConnection $connection = null) + private function canRetry(RetryConnection $connection) { - return $this->retryLimit > 0 && ($connection === null || $connection->transactionLevel() === 0); + return $this->retryLimit > 0 && $connection->transactionLevel() === 0; } private function errorCodeStrategy($errorCode) @@ -72,7 +68,7 @@ private function changeServer(stdClass $strategy, RetryConnection $connection) if (!$strategy->changeServer) { return true; } - $wrappedConnection = $connection->wrappedConnection()->getWrappedConnection(); + $wrappedConnection = $connection->wrappedConnection(); if ($wrappedConnection instanceof MasterSlavesConnection && !$wrappedConnection->isConnectedToMaster()) { $wrappedConnection->disableCurrentSlave(); return true; @@ -88,5 +84,5 @@ private function reconnect(stdClass $strategy, RetryConnection $connection) } protected abstract function errorCodeStrategies(); - protected abstract function errorCode(DBALException $exception); + protected abstract function errorCode(Exception $exception); }