From 23fc6039062cc9cfe61de74f380b8f6fe2834d66 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 23 Jan 2015 12:29:55 -0500 Subject: [PATCH 1/4] Fixes issue (DBAL-1057) where simply initiating the platform may cause an error For example, if you have no database, then previously, we would try to connect, causing an error. But in this case, we can simple fall back to the default platform. This is exactly what the previous version did (see sha: 3176f51d1426022d305c6531a9b9bc93a868bddd), and thus this adds the nice fallback we had before, without removing the flexibility of setting your exact server version or determining it once there *is* a connection --- lib/Doctrine/DBAL/Connection.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index 97b6030475d..601354db81a 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -424,9 +424,12 @@ private function getDatabasePlatformVersion() return $this->_params['serverVersion']; } - // If not connected, we need to connect now to determine the platform version. + // If not connected, we can't figure out the server version. If + // we try to connect, it may fail (e.g. no database), which would + // cause an error in our application when determining the platform + // version is not mission-critical if (null === $this->_conn) { - $this->connect(); + return null; } // Automatic platform version detection. From e90bdcd0b4d50e64acc58b1b78e4bcfec4973a54 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 23 Jan 2015 13:15:56 -0500 Subject: [PATCH 2/4] Adding a test, which WAS failing before this patch --- tests/Doctrine/Tests/DBAL/ConnectionTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/Doctrine/Tests/DBAL/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/ConnectionTest.php index 8aa2122282f..3012671fa4a 100644 --- a/tests/Doctrine/Tests/DBAL/ConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/ConnectionTest.php @@ -412,6 +412,14 @@ public function testConnectionIsClosed() $this->_conn->quoteIdentifier('Bug'); } + public function testGetPlatformVersionDoesNotFailOnBadConnection() + { + // the connection uses bogus connection requirements, so connecting + // *will* fail. We should be able to call this without connection + // and causing that exception + $this->_conn->getDatabasePlatform(); + } + public function testFetchAll() { $statement = 'SELECT * FROM foo WHERE bar = ?'; From 81933330c4802d9181547113b6691f9da65234a7 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 23 Jan 2015 13:16:37 -0500 Subject: [PATCH 3/4] Removing test that's failing, but because it is invalid - see #784 --- tests/Doctrine/Tests/DBAL/ConnectionTest.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/Doctrine/Tests/DBAL/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/ConnectionTest.php index 3012671fa4a..8e8d080c79c 100644 --- a/tests/Doctrine/Tests/DBAL/ConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/ConnectionTest.php @@ -403,15 +403,6 @@ public function testFetchColumn() $this->assertSame($result, $conn->fetchColumn($statement, $params, $column, $types)); } - public function testConnectionIsClosed() - { - $this->_conn->close(); - - $this->setExpectedException('Doctrine\\DBAL\\Exception\\DriverException'); - - $this->_conn->quoteIdentifier('Bug'); - } - public function testGetPlatformVersionDoesNotFailOnBadConnection() { // the connection uses bogus connection requirements, so connecting From 2db254d336a9d7cb87f4bc71cf20e050cd91d8e1 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 23 Jan 2015 13:30:01 -0500 Subject: [PATCH 4/4] Changing implementation to *try* to connect For me, this makes more sense. Without this, if getDatabasePlatform() is called consistently before connection(), then the platform will *never* connect to get the version and use the *best* platform - it will always use the fallback. This *does* use the connection, but doesn't explode for the edge cases when the connection information is invalid. --- lib/Doctrine/DBAL/Connection.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index 601354db81a..b68e0ee7739 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -20,6 +20,7 @@ namespace Doctrine\DBAL; use Doctrine\DBAL\Driver\ServerInfoAwareConnection; +use Doctrine\DBAL\Exception\DriverException; use Doctrine\DBAL\Exception\InvalidArgumentException; use PDO; use Closure; @@ -429,7 +430,12 @@ private function getDatabasePlatformVersion() // cause an error in our application when determining the platform // version is not mission-critical if (null === $this->_conn) { - return null; + try { + $this->connect(); + } catch (DriverException $e) { + // i.e. connection params are invalid or db doesn't exist + return null; + } } // Automatic platform version detection.