Merge pull request #17 from facile-it/fix-retry

Fix retry for updates
facile-it · Jun 9, 2016 · f019e5a · f019e5a
2 parents 351ce2e + c72704b
commit f019e5a
Show file tree

Hide file tree

Showing 10 changed files with 295 additions and 102 deletions.
diff --git a/.gitignore b/.gitignore
@@ -3,3 +3,4 @@ vendor
 bin
 composer.phar
 composer.lock
+phpunit.xml
diff --git a/composer.json b/composer.json
@@ -10,18 +10,26 @@
         }
     ],
     "require": {
-        "php" : ">=5.4",
+        "php" : "^5.4 || ^7.0",
         "doctrine/dbal": ">=2.3,<2.6-dev"
     },
     "require-dev": {
-        "phpunit/phpunit": "^4.8"
+        "phpunit/phpunit": "^4.8 || ^5.2",
+        "friendsofphp/php-cs-fixer": "^1.11"
     },
     "autoload": {
         "psr-4": { "Facile\\DoctrineMySQLComeBack\\Doctrine\\DBAL\\": "src/" }
     },
+    "autoload-dev": {
+        "psr-4": { "Facile\\DoctrineMySQLComeBack\\Doctrine\\DBAL\\": "tests/unit/" }
+    },
     "config": {
         "preferred-install": "dist",
         "bin-dir": "bin"
     },
-    "minimum-stability": "stable"
+    "minimum-stability": "stable",
+    "scripts": {
+        "phpcs": "php-cs-fixer fix --level=psr2 -v --diff --dry-run src/",
+        "phpcs-fix": "php-cs-fixer fix --level=psr2 -v --diff src/"
+    }
 }
diff --git a/phpunit.xml → phpunit.xml.dist b/phpunit.xml → phpunit.xml.dist
diff --git a/src/Connection.php b/src/Connection.php
@@ -2,16 +2,14 @@
 
 namespace Facile\DoctrineMySQLComeBack\Doctrine\DBAL;
 
-use Doctrine\DBAL\Configuration,
-    Doctrine\DBAL\Driver,
-    Doctrine\Common\EventManager,
-    Doctrine\DBAL\Cache\QueryCacheProfile;
+use Doctrine\DBAL\Configuration;
+use Doctrine\DBAL\Driver;
+use Doctrine\Common\EventManager;
+use Doctrine\DBAL\Cache\QueryCacheProfile;
 use Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver\ServerGoneAwayExceptionsAwareInterface;
 
 /**
- * Class Connection
- *
- * @package Facile\DoctrineMySQLComeBack
+ * Class Connection.
  */
 class Connection extends \Doctrine\DBAL\Connection
 {
@@ -26,34 +24,40 @@ class Connection extends \Doctrine\DBAL\Connection
     private $selfReflectionNestingLevelProperty;
 
     /**
-     * @param array $params
+     * @param array                                         $params
      * @param Driver|ServerGoneAwayExceptionsAwareInterface $driver
-     * @param Configuration $config
-     * @param EventManager $eventManager
+     * @param Configuration                                 $config
+     * @param EventManager                                  $eventManager
+     *
+     * @throws \InvalidArgumentException
      */
     public function __construct(
         array $params,
         Driver $driver,
         Configuration $config = null,
         EventManager $eventManager = null
-    )
-    {
-        if (
-            $driver instanceof ServerGoneAwayExceptionsAwareInterface &&
-            isset($params['driverOptions']['x_reconnect_attempts'])
-        ) {
-            $this->reconnectAttempts = (int)$params['driverOptions']['x_reconnect_attempts'];
+    ) {
+        if (!$driver instanceof ServerGoneAwayExceptionsAwareInterface) {
+            throw new \InvalidArgumentException(
+                sprintf('%s needs a driver that implements ServerGoneAwayExceptionsAwareInterface', get_class($this))
+            );
+        }
+
+        if (isset($params['driverOptions']['x_reconnect_attempts'])) {
+            $this->reconnectAttempts = (int) $params['driverOptions']['x_reconnect_attempts'];
         }
 
         parent::__construct($params, $driver, $config, $eventManager);
     }
 
     /**
-     * @param $query
-     * @param array $params
-     * @param array $types
+     * @param string            $query
+     * @param array             $params
+     * @param array             $types
      * @param QueryCacheProfile $qcp
-     * @return null
+     *
+     * @return \Doctrine\DBAL\Driver\Statement The executed statement.
+     *
      * @throws \Exception
      */
     public function executeQuery($query, array $params = array(), $types = array(), QueryCacheProfile $qcp = null)
@@ -66,20 +70,21 @@ public function executeQuery($query, array $params = array(), $types = array(),
             try {
                 $stmt = parent::executeQuery($query, $params, $types, $qcp);
             } catch (\Exception $e) {
-                if ($this->canTryAgain($attempt) && $this->_driver->isGoneAwayException($e)) {
+                if ($this->canTryAgain($attempt) && $this->isRetryableException($e, $query)) {
                     $this->close();
-                    $attempt++;
+                    ++$attempt;
                     $retry = true;
                 } else {
                     throw $e;
                 }
             }
         }
+
         return $stmt;
     }
 
     /**
-     * @return null
+     * @return \Doctrine\DBAL\Driver\Statement
      * @throws \Exception
      */
     public function query()
@@ -108,23 +113,26 @@ public function query()
                         $stmt = parent::query();
                 }
             } catch (\Exception $e) {
-                if ($this->canTryAgain($attempt) && $this->_driver->isGoneAwayException($e)) {
+                if ($this->canTryAgain($attempt) && $this->isRetryableException($e, $args[0])) {
                     $this->close();
-                    $attempt++;
+                    ++$attempt;
                     $retry = true;
                 } else {
                     throw $e;
                 }
             }
         }
+
         return $stmt;
     }
 
     /**
-     * @param $query
-     * @param array $params
-     * @param array $types
-     * @return null
+     * @param string $query
+     * @param array  $params
+     * @param array  $types
+     *
+     * @return integer The number of affected rows.
+     *
      * @throws \Exception
      */
     public function executeUpdate($query, array $params = array(), array $types = array())
@@ -137,56 +145,53 @@ public function executeUpdate($query, array $params = array(), array $types = ar
             try {
                 $stmt = parent::executeUpdate($query, $params, $types);
             } catch (\Exception $e) {
-                if ($this->canTryAgain($attempt) && $this->_driver->isGoneAwayException($e)) {
+                if ($this->canTryAgain($attempt) && $this->isRetryableException($e)) {
                     $this->close();
-                    $attempt++;
+                    ++$attempt;
                     $retry = true;
                 } else {
                     throw $e;
                 }
             }
         }
+
         return $stmt;
     }
 
     /**
+     * @return void
      * @throws \Exception
      */
     public function beginTransaction()
     {
         if (0 !== $this->getTransactionNestingLevel()) {
-           return parent::beginTransaction();
+            return parent::beginTransaction();
         }
 
-        $queryResult = null;
         $attempt = 0;
         $retry = true;
         while ($retry) {
             $retry = false;
             try {
-
-                $queryResult = parent::beginTransaction();
-
+                parent::beginTransaction();
             } catch (\Exception $e) {
-
-                if ($this->canTryAgain($attempt,true) && $this->_driver->isGoneAwayException($e)) {
+                if ($this->canTryAgain($attempt, true) && $this->_driver->isGoneAwayException($e)) {
                     $this->close();
-                    if(0 < $this->getTransactionNestingLevel()) {
+                    if (0 < $this->getTransactionNestingLevel()) {
                         $this->resetTransactionNestingLevel();
                     }
-                    $attempt++;
+                    ++$attempt;
                     $retry = true;
                 } else {
                     throw $e;
                 }
             }
         }
-
-        return $queryResult;
     }
 
     /**
      * @param $sql
+     *
      * @return Statement
      */
     public function prepare($sql)
@@ -195,9 +200,10 @@ public function prepare($sql)
     }
 
     /**
-     * returns a reconnect-wrapper for Statements
+     * returns a reconnect-wrapper for Statements.
      *
      * @param $sql
+     *
      * @return Statement
      */
     protected function prepareWrapped($sql)
@@ -207,7 +213,7 @@ protected function prepareWrapped($sql)
 
     /**
      * do not use, only used by Statement-class
-     * needs to be public for access from the Statement-class
+     * needs to be public for access from the Statement-class.
      *
      * @internal
      */
@@ -218,7 +224,7 @@ public function prepareUnwrapped($sql)
     }
 
     /**
-     * Forces reconnection by doing a dummy query
+     * Forces reconnection by doing a dummy query.
      *
      * @throws \Exception
      */
@@ -230,14 +236,30 @@ public function refresh()
     /**
      * @param $attempt
      * @param bool $ignoreTransactionLevel
+     *
      * @return bool
      */
     public function canTryAgain($attempt, $ignoreTransactionLevel = false)
     {
         $canByAttempt = ($attempt < $this->reconnectAttempts);
         $canByTransactionNestingLevel = $ignoreTransactionLevel ? true : (0 === $this->getTransactionNestingLevel());
 
-        return ($canByAttempt && $canByTransactionNestingLevel);
+        return $canByAttempt && $canByTransactionNestingLevel;
+    }
+
+    /**
+     * @param \Exception  $e
+     * @param string|null $query
+     *
+     * @return bool
+     */
+    public function isRetryableException(\Exception $e, $query = null)
+    {
+        if (null === $query || $this->isUpdateQuery($query)) {
+            return $this->_driver->isGoneAwayInUpdateException($e);
+        }
+
+        return $this->_driver->isGoneAwayException($e);
     }
 
     /**
@@ -247,12 +269,22 @@ public function canTryAgain($attempt, $ignoreTransactionLevel = false)
      */
     private function resetTransactionNestingLevel()
     {
-        if(!$this->selfReflectionNestingLevelProperty instanceof \ReflectionProperty) {
+        if (!$this->selfReflectionNestingLevelProperty instanceof \ReflectionProperty) {
             $reflection = new \ReflectionClass('Doctrine\DBAL\Connection');
-            $this->selfReflectionNestingLevelProperty = $reflection->getProperty("_transactionNestingLevel");
+            $this->selfReflectionNestingLevelProperty = $reflection->getProperty('_transactionNestingLevel');
             $this->selfReflectionNestingLevelProperty->setAccessible(true);
         }
 
-        $this->selfReflectionNestingLevelProperty->setValue($this,0);
+        $this->selfReflectionNestingLevelProperty->setValue($this, 0);
+    }
+
+    /**
+     * @param string $query
+     *
+     * @return bool
+     */
+    public function isUpdateQuery($query)
+    {
+        return !preg_match('/^[\s\n\r\t(]*(select|show) /i', $query);
     }
 }
diff --git a/src/Driver/Mysqli/Driver.php b/src/Driver/Mysqli/Driver.php
@@ -2,12 +2,11 @@
 
 namespace Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver\Mysqli;
 
-use Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver\ServerGoneAwayExceptionsAwareInterface,
-    Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver\ServerGoneAwayExceptionsAwareTrait;
+use Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver\ServerGoneAwayExceptionsAwareInterface;
+use Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver\ServerGoneAwayExceptionsAwareTrait;
 
 /**
- * Class Driver
- * @package Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver\Mysqli
+ * Class Driver.
  */
 class Driver extends \Doctrine\DBAL\Driver\Mysqli\Driver implements ServerGoneAwayExceptionsAwareInterface
 {
@@ -17,7 +16,7 @@ class Driver extends \Doctrine\DBAL\Driver\Mysqli\Driver implements ServerGoneAw
      * @var array
      */
     private $extendedDriverOptions = [
-        "x_reconnect_attempts",
+        'x_reconnect_attempts',
     ];
 
     /**

diff --git a/src/Driver/PDOMySql/Driver.php b/src/Driver/PDOMySql/Driver.php
@@ -6,8 +6,7 @@
 use Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver\ServerGoneAwayExceptionsAwareTrait;
 
 /**
- * Class Driver
- * @package Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver\PDOMySql
+ * Class Driver.
  */
 class Driver extends \Doctrine\DBAL\Driver\PDOMySql\Driver implements ServerGoneAwayExceptionsAwareInterface
 {

diff --git a/src/Driver/ServerGoneAwayExceptionsAwareInterface.php b/src/Driver/ServerGoneAwayExceptionsAwareInterface.php
@@ -3,14 +3,21 @@
 namespace Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver;
 
 /**
- * Class ServerGoneAwayExceptionsAwareInterface
- * @package Facile\DoctrineMySQLComeBack\Doctrine\DBAL\Driver
+ * Class ServerGoneAwayExceptionsAwareInterface.
  */
 interface ServerGoneAwayExceptionsAwareInterface
 {
     /**
      * @param \Exception $e
+     *
      * @return bool
      */
-    function isGoneAwayException(\Exception $e);
+    public function isGoneAwayException(\Exception $e);
+
+    /**
+     * @param \Exception $e
+     *
+     * @return bool
+     */
+    public function isGoneAwayInUpdateException(\Exception $e);
 }