Removed ServerInfoAwareConnection#requiresQueryForServerVersion() as …

…implementation detail

Testing the implementations of this method requires partial mocking of the implementing class which makes it impossible to make them `final` (doctrine#3590).

Additionally, this API breaks the encapsulation of the driver layer: instead of exposing the fact of whether the connection will perform a query to detect the server version, the driver should just instantiate a platform corresponding to a connection.

The rationale behind introducing this method (doctrine#487) is really questionable:

> This is also required for drivers that cannot return the database server version without an additional query (performance reasons).

1. There's no evidence that an underlying driver that exposes the server version via its API doesn't make a request of any kind to the server.
2. For an application that works with any realistic database, a query like `SELECT VERSION()` wouldn't be a performance bottleneck.
3. Even if it was, it's always possible to specify the platform version upfront. Otherwise, the current logic of falling back to a default platform may cause undefined behavior of the application (we don't test the compatibility of the lowest level of the DBAL platform with all supported server versions). Remember, “If it doesn’t work, it doesn’t matter how fast it doesn’t work.”

In addition to the above, the only driver that doesn't support the platform version detection via the underlying driver API is `sqsql` which is barely supported.

UPGRADE.md

@@ -1,5 +1,9 @@
 # Upgrade to 3.0
 
+## BC BREAK: `ServerInfoAwareConnection::requiresQueryForServerVersion()` is removed.
+
+The `ServerInfoAwareConnection::requiresQueryForServerVersion()` method has been removed as an implementation detail which is the same for almost all supported drivers.
+
 ## BC BREAK: PingableConnection and ServerInfoAwareConnection interfaces now extends Connection
 
 All implementations of the `PingableConnection` and `ServerInfoAwareConnection` interfaces have to implement the methods defined in the `Connection` interface as well.

lib/Doctrine/DBAL/Connection.php

@@ -393,7 +393,7 @@ private function getServerVersion() : ?string
         $connection = $this->getWrappedConnection();
 
         // Automatic platform version detection.
-        if ($connection instanceof ServerInfoAwareConnection && ! $connection->requiresQueryForServerVersion()) {
+        if ($connection instanceof ServerInfoAwareConnection) {
             return $connection->getServerVersion();
         }
 

lib/Doctrine/DBAL/Driver/IBMDB2/DB2Connection.php

@@ -59,14 +59,6 @@ public function getServerVersion() : string
         return $serverInfo->DBMS_VER;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function requiresQueryForServerVersion() : bool
-    {
-        return false;
-    }
-
     /**
      * {@inheritdoc}
      */

lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php

@@ -117,14 +117,6 @@ public function getServerVersion() : string
         return $majorVersion . '.' . $minorVersion . '.' . $patchVersion;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function requiresQueryForServerVersion() : bool
-    {
-        return false;
-    }
-
     /**
      * {@inheritdoc}
      */

lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php

@@ -85,14 +85,6 @@ public function getServerVersion() : string
         return $matches[1];
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function requiresQueryForServerVersion() : bool
-    {
-        return false;
-    }
-
     /**
      * {@inheritdoc}
      */

lib/Doctrine/DBAL/Driver/PDOConnection.php

@@ -105,14 +105,6 @@ public function lastInsertId(?string $name = null) : string
         }
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function requiresQueryForServerVersion() : bool
-    {
-        return false;
-    }
-
     /**
      * Creates a wrapped statement
      */

lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereConnection.php

@@ -144,14 +144,6 @@ public function quote(string $input) : string
         return "'" . sasql_escape_string($this->connection, $input) . "'";
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function requiresQueryForServerVersion() : bool
-    {
-        return true;
-    }
-
     /**
      * {@inheritdoc}
      *

lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvConnection.php

@@ -59,14 +59,6 @@ public function getServerVersion() : string
         return $serverInfo['SQLServerVersion'];
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function requiresQueryForServerVersion() : bool
-    {
-        return false;
-    }
-
     /**
      * {@inheritDoc}
      */

lib/Doctrine/DBAL/Driver/ServerInfoAwareConnection.php

@@ -13,11 +13,4 @@ interface ServerInfoAwareConnection extends Connection
      * Returns the version number of the database server connected to.
      */
     public function getServerVersion() : string;
-
-    /**
-     * Checks whether a query is required to retrieve the database server version.
-     *
-     * @return bool True if a query is required to retrieve the database server version, false otherwise.
-     */
-    public function requiresQueryForServerVersion() : bool;
 }

tests/Doctrine/Tests/DBAL/ConnectionTest.php

@@ -687,10 +687,6 @@ public function testPlatformDetectionIsTriggerOnlyOnceOnRetrievingPlatform() : v
             ->method('connect')
             ->will($this->returnValue($driverConnectionMock));
 
-        $driverConnectionMock->expects($this->once())
-            ->method('requiresQueryForServerVersion')
-            ->will($this->returnValue(false));
-
         $driverConnectionMock->expects($this->once())
             ->method('getServerVersion')
             ->will($this->returnValue('6.6.6'));

tests/Doctrine/Tests/DBAL/Driver/Mysqli/MysqliConnectionTest.php

@@ -8,20 +8,12 @@
 use Doctrine\DBAL\Driver\Mysqli\MysqliConnection;
 use Doctrine\DBAL\Platforms\MySqlPlatform;
 use Doctrine\Tests\DbalFunctionalTestCase;
-use PHPUnit\Framework\MockObject\MockObject;
 use function extension_loaded;
 use function restore_error_handler;
 use function set_error_handler;
 
 class MysqliConnectionTest extends DbalFunctionalTestCase
 {
-    /**
-     * The mysqli driver connection mock under test.
-     *
-     * @var MysqliConnection|MockObject
-     */
-    private $connectionMock;
-
     protected function setUp() : void
     {
         if (! extension_loaded('mysqli')) {
@@ -30,18 +22,11 @@ protected function setUp() : void
 
         parent::setUp();
 
-        if (! $this->connection->getDatabasePlatform() instanceof MySqlPlatform) {
-            $this->markTestSkipped('MySQL only test.');
+        if ($this->connection->getDatabasePlatform() instanceof MySqlPlatform) {
+            return;
         }
 
-        $this->connectionMock = $this->getMockBuilder(MysqliConnection::class)
-            ->disableOriginalConstructor()
-            ->getMockForAbstractClass();
-    }
-
-    public function testDoesNotRequireQueryForServerVersion() : void
-    {
-        self::assertFalse($this->connectionMock->requiresQueryForServerVersion());
+        $this->markTestSkipped('MySQL only test.');
     }
 
     public function testRestoresErrorHandlerOnException() : void

tests/Doctrine/Tests/DBAL/Functional/Driver/PDOConnectionTest.php

@@ -46,11 +46,6 @@ protected function tearDown() : void
         parent::tearDown();
     }
 
-    public function testDoesNotRequireQueryForServerVersion() : void
-    {
-        self::assertFalse($this->driverConnection->requiresQueryForServerVersion());
-    }
-
     public function testThrowsWrappedExceptionOnConstruct() : void
     {
         $this->expectException(PDOException::class);