Remove ServerInfoAwareConnection#requiresQueryForServerVersion() as an implementation detail

Testing the implementations of this method requires partial mocking of the implementing class which makes it impossible to make them `final` (#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 (#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.”
parent 9d45766a
# Upgrade to 3.0 # 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 all supported drivers.
## BC BREAK Changes in driver exceptions ## BC BREAK Changes in driver exceptions
1. The `Doctrine\DBAL\Driver\DriverException::getErrorCode()` method is removed. In order to obtain the driver error code, please use `::getCode()` or `::getSQLState()`. 1. The `Doctrine\DBAL\Driver\DriverException::getErrorCode()` method is removed. In order to obtain the driver error code, please use `::getCode()` or `::getSQLState()`.
......
...@@ -396,7 +396,7 @@ class Connection implements DriverConnection ...@@ -396,7 +396,7 @@ class Connection implements DriverConnection
$connection = $this->getWrappedConnection(); $connection = $this->getWrappedConnection();
// Automatic platform version detection. // Automatic platform version detection.
if ($connection instanceof ServerInfoAwareConnection && ! $connection->requiresQueryForServerVersion()) { if ($connection instanceof ServerInfoAwareConnection) {
return $connection->getServerVersion(); return $connection->getServerVersion();
} }
......
...@@ -73,14 +73,6 @@ class DB2Connection implements ServerInfoAwareConnection ...@@ -73,14 +73,6 @@ class DB2Connection implements ServerInfoAwareConnection
return $serverInfo->DBMS_VER; return $serverInfo->DBMS_VER;
} }
/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion()
{
return false;
}
public function prepare(string $sql): DriverStatement public function prepare(string $sql): DriverStatement
{ {
$stmt = @db2_prepare($this->conn, $sql); $stmt = @db2_prepare($this->conn, $sql);
......
...@@ -96,14 +96,6 @@ class MysqliConnection implements PingableConnection, ServerInfoAwareConnection ...@@ -96,14 +96,6 @@ class MysqliConnection implements PingableConnection, ServerInfoAwareConnection
return $majorVersion . '.' . $minorVersion . '.' . $patchVersion; return $majorVersion . '.' . $minorVersion . '.' . $patchVersion;
} }
/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion()
{
return false;
}
public function prepare(string $sql): DriverStatement public function prepare(string $sql): DriverStatement
{ {
return new Statement($this->conn, $sql); return new Statement($this->conn, $sql);
......
...@@ -97,14 +97,6 @@ class OCI8Connection implements ConnectionInterface, ServerInfoAwareConnection ...@@ -97,14 +97,6 @@ class OCI8Connection implements ConnectionInterface, ServerInfoAwareConnection
return $matches[1]; return $matches[1];
} }
/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion()
{
return false;
}
public function prepare(string $sql): DriverStatement public function prepare(string $sql): DriverStatement
{ {
return new Statement($this->dbh, $sql, $this->executionMode); return new Statement($this->dbh, $sql, $this->executionMode);
......
...@@ -112,14 +112,6 @@ class PDOConnection implements ServerInfoAwareConnection ...@@ -112,14 +112,6 @@ class PDOConnection implements ServerInfoAwareConnection
} }
} }
/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion()
{
return false;
}
/** /**
* Creates a wrapped statement * Creates a wrapped statement
*/ */
......
...@@ -66,14 +66,6 @@ class SQLSrvConnection implements ServerInfoAwareConnection ...@@ -66,14 +66,6 @@ class SQLSrvConnection implements ServerInfoAwareConnection
return $serverInfo['SQLServerVersion']; return $serverInfo['SQLServerVersion'];
} }
/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion()
{
return false;
}
public function prepare(string $sql): DriverStatement public function prepare(string $sql): DriverStatement
{ {
return new Statement($this->conn, $sql, $this->lastInsertId); return new Statement($this->conn, $sql, $this->lastInsertId);
......
...@@ -13,11 +13,4 @@ interface ServerInfoAwareConnection extends Connection ...@@ -13,11 +13,4 @@ interface ServerInfoAwareConnection extends Connection
* @return string * @return string
*/ */
public function getServerVersion(); public function getServerVersion();
/**
* 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();
} }
...@@ -661,10 +661,6 @@ class ConnectionTest extends TestCase ...@@ -661,10 +661,6 @@ class ConnectionTest extends TestCase
->method('connect') ->method('connect')
->will(self::returnValue($driverConnectionMock)); ->will(self::returnValue($driverConnectionMock));
$driverConnectionMock->expects(self::once())
->method('requiresQueryForServerVersion')
->will(self::returnValue(false));
$driverConnectionMock->expects(self::once()) $driverConnectionMock->expects(self::once())
->method('getServerVersion') ->method('getServerVersion')
->will(self::returnValue('6.6.6')); ->will(self::returnValue('6.6.6'));
......
<?php
namespace Doctrine\DBAL\Tests\Driver\IBMDB2;
use Doctrine\DBAL\Driver\IBMDB2\DB2Connection;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use function extension_loaded;
class DB2ConnectionTest extends TestCase
{
/**
* The ibm_db2 driver connection mock under test.
*
* @var DB2Connection|MockObject
*/
private $connectionMock;
protected function setUp(): void
{
if (! extension_loaded('ibm_db2')) {
$this->markTestSkipped('ibm_db2 is not installed.');
}
parent::setUp();
$this->connectionMock = $this->getMockBuilder(DB2Connection::class)
->disableOriginalConstructor()
->getMockForAbstractClass();
}
public function testDoesNotRequireQueryForServerVersion(): void
{
self::assertFalse($this->connectionMock->requiresQueryForServerVersion());
}
}
...@@ -4,22 +4,12 @@ namespace Doctrine\DBAL\Tests\Driver\Mysqli; ...@@ -4,22 +4,12 @@ namespace Doctrine\DBAL\Tests\Driver\Mysqli;
use Doctrine\DBAL\Driver\Mysqli\Driver; use Doctrine\DBAL\Driver\Mysqli\Driver;
use Doctrine\DBAL\Driver\Mysqli\HostRequired; use Doctrine\DBAL\Driver\Mysqli\HostRequired;
use Doctrine\DBAL\Driver\Mysqli\MysqliConnection;
use Doctrine\DBAL\Platforms\MySqlPlatform;
use Doctrine\DBAL\Tests\FunctionalTestCase; use Doctrine\DBAL\Tests\FunctionalTestCase;
use PHPUnit\Framework\MockObject\MockObject;
use function extension_loaded; use function extension_loaded;
class MysqliConnectionTest extends FunctionalTestCase class MysqliConnectionTest extends FunctionalTestCase
{ {
/**
* The mysqli driver connection mock under test.
*
* @var MysqliConnection|MockObject
*/
private $connectionMock;
protected function setUp(): void protected function setUp(): void
{ {
if (! extension_loaded('mysqli')) { if (! extension_loaded('mysqli')) {
...@@ -27,19 +17,6 @@ class MysqliConnectionTest extends FunctionalTestCase ...@@ -27,19 +17,6 @@ class MysqliConnectionTest extends FunctionalTestCase
} }
parent::setUp(); parent::setUp();
if (! $this->connection->getDatabasePlatform() instanceof MySqlPlatform) {
$this->markTestSkipped('MySQL only test.');
}
$this->connectionMock = $this->getMockBuilder(MysqliConnection::class)
->disableOriginalConstructor()
->getMockForAbstractClass();
}
public function testDoesNotRequireQueryForServerVersion(): void
{
self::assertFalse($this->connectionMock->requiresQueryForServerVersion());
} }
public function testHostnameIsRequiredForPersistentConnection(): void public function testHostnameIsRequiredForPersistentConnection(): void
......
<?php
namespace Doctrine\DBAL\Tests\Driver\OCI8;
use Doctrine\DBAL\Driver\OCI8\OCI8Connection;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use function extension_loaded;
class OCI8ConnectionTest extends TestCase
{
/**
* The oci8 driver connection mock under test.
*
* @var OCI8Connection|MockObject
*/
private $connectionMock;
protected function setUp(): void
{
if (! extension_loaded('oci8')) {
$this->markTestSkipped('oci8 is not installed.');
}
parent::setUp();
$this->connectionMock = $this->getMockBuilder(OCI8Connection::class)
->disableOriginalConstructor()
->getMockForAbstractClass();
}
public function testDoesNotRequireQueryForServerVersion(): void
{
self::assertFalse($this->connectionMock->requiresQueryForServerVersion());
}
}
<?php
namespace Doctrine\DBAL\Tests\Driver\SQLSrv;
use Doctrine\DBAL\Driver\SQLSrv\SQLSrvConnection;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use function extension_loaded;
class SQLSrvConnectionTest extends TestCase
{
/**
* The sqlsrv driver connection mock under test.
*
* @var SQLSrvConnection|MockObject
*/
private $connectionMock;
protected function setUp(): void
{
if (! extension_loaded('sqlsrv')) {
$this->markTestSkipped('sqlsrv is not installed.');
}
parent::setUp();
$this->connectionMock = $this->getMockBuilder(SQLSrvConnection::class)
->disableOriginalConstructor()
->getMockForAbstractClass();
}
public function testDoesNotRequireQueryForServerVersion(): void
{
self::assertFalse($this->connectionMock->requiresQueryForServerVersion());
}
}
...@@ -45,11 +45,6 @@ class ConnectionTest extends FunctionalTestCase ...@@ -45,11 +45,6 @@ class ConnectionTest extends FunctionalTestCase
parent::tearDown(); parent::tearDown();
} }
public function testDoesNotRequireQueryForServerVersion(): void
{
self::assertFalse($this->driverConnection->requiresQueryForServerVersion());
}
public function testThrowsWrappedExceptionOnConstruct(): void public function testThrowsWrappedExceptionOnConstruct(): void
{ {
$this->expectException(Exception::class); $this->expectException(Exception::class);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment