Merge pull request #3485 from morozov/ping-void

`Connection::ping()` will now return `void`
doctrine · May 23, 2019 · fb869d7 · fb869d7
2 parents e3f4726 + bf0d7c6
commit fb869d7
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 28 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,9 @@
 # Upgrade to 3.0
 
+## BC BREAK `Connection::ping()` returns `void`.
+
+`Connection::ping()` and `PingableConnection::ping()` no longer return a boolean value. They will throw an exception in case of failure.
+
 ## BC BREAK User-provided `PDO` instance is no longer supported
 
 In order to share the same `PDO` instances between DBAL and other components, initialize the connection in DBAL and access it using `Connection::getWrappedConnection()->getWrappedConnection()`.

diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php
@@ -1556,38 +1556,40 @@ public function createQueryBuilder()
     /**
      * Ping the server
      *
-     * When the server is not available the method returns FALSE.
+     * When the server is not available the method throws a DBALException.
      * It is responsibility of the developer to handle this case
-     * and abort the request or reconnect manually:
+     * and abort the request or close the connection so that it's reestablished
+     * upon the next statement execution.
      *
-     * @return bool
+     * @throws DBALException
      *
      * @example
      *
-     *   if ($conn->ping() === false) {
+     *   try {
+     *      $conn->ping();
+     *   } catch (DBALException $e) {
      *      $conn->close();
-     *      $conn->connect();
      *   }
      *
      * It is undefined if the underlying driver attempts to reconnect
      * or disconnect when the connection is not available anymore
-     * as long it returns TRUE when a reconnect succeeded and
-     * FALSE when the connection was dropped.
+     * as long it successfully returns when a reconnect succeeded
+     * and throws an exception if the connection was dropped.
      */
-    public function ping()
+    public function ping() : void
     {
         $connection = $this->getWrappedConnection();
 
-        if ($connection instanceof PingableConnection) {
-            return $connection->ping();
+        if (! $connection instanceof PingableConnection) {
+            $this->query($this->getDatabasePlatform()->getDummySelectSQL());
+
+            return;
         }
 
         try {
-            $this->query($this->getDatabasePlatform()->getDummySelectSQL());
-
-            return true;
-        } catch (DBALException $e) {
-            return false;
+            $connection->ping();
+        } catch (DriverException $e) {
+            throw DBALException::driverException($this->_driver, $e);
         }
     }
 }
diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php b/lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php
@@ -250,11 +250,13 @@ private function setDriverOptions(array $driverOptions = [])
     /**
      * Pings the server and re-connects when `mysqli.reconnect = 1`
      *
-     * @return bool
+     * {@inheritDoc}
      */
-    public function ping()
+    public function ping() : void
     {
-        return $this->conn->ping();
+        if (! $this->conn->ping()) {
+            throw new MysqliException($this->conn->error, $this->conn->sqlstate, $this->conn->errno);
+        }
     }
 
     /**

diff --git a/lib/Doctrine/DBAL/Driver/PingableConnection.php b/lib/Doctrine/DBAL/Driver/PingableConnection.php
@@ -11,9 +11,9 @@ interface PingableConnection
 {
     /**
      * Pings the database server to determine if the connection is still
-     * available. Return true/false based on if that was successful or not.
+     * available.
      *
-     * @return bool
+     * @throws DriverException
      */
-    public function ping();
+    public function ping() : void;
 }
diff --git a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php
@@ -264,7 +264,10 @@ public function testQuote()
 
     public function testPingDoesTriggersConnect()
     {
-        self::assertTrue($this->connection->ping());
+        $this->connection->close();
+        self::assertFalse($this->connection->isConnected());
+
+        $this->connection->ping();
         self::assertTrue($this->connection->isConnected());
     }
 

diff --git a/tests/Doctrine/Tests/DBAL/Functional/Driver/Mysqli/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/Driver/Mysqli/ConnectionTest.php
@@ -48,12 +48,6 @@ public function testUnsupportedDriverOption()
         $this->getConnection(['hello' => 'world']); // use local infile
     }
 
-    public function testPing()
-    {
-        $conn = $this->getConnection([]);
-        self::assertTrue($conn->ping());
-    }
-
     /**
      * @param mixed[] $driverOptions
      */