Skip to content

Commit

Permalink
Removed ServerInfoAwareConnection#requiresQueryForServerVersion() as …
Browse files Browse the repository at this point in the history
…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 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.

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.
  • Loading branch information
morozov committed Jan 3, 2020
1 parent f37a88e commit 7a0aa2d
Show file tree
Hide file tree
Showing 15 changed files with 1 addition and 222 deletions.
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
8 changes: 0 additions & 8 deletions lib/Doctrine/DBAL/Driver/IBMDB2/DB2Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,6 @@ public function getServerVersion() : string
return $serverInfo->DBMS_VER;
}

/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion() : bool
{
return false;
}

/**
* {@inheritdoc}
*/
Expand Down
8 changes: 0 additions & 8 deletions lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,6 @@ public function getServerVersion() : string
return $majorVersion . '.' . $minorVersion . '.' . $patchVersion;
}

/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion() : bool
{
return false;
}

/**
* {@inheritdoc}
*/
Expand Down
8 changes: 0 additions & 8 deletions lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,6 @@ public function getServerVersion() : string
return $matches[1];
}

/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion() : bool
{
return false;
}

/**
* {@inheritdoc}
*/
Expand Down
8 changes: 0 additions & 8 deletions lib/Doctrine/DBAL/Driver/PDOConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,6 @@ public function lastInsertId(?string $name = null) : string
}
}

/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion() : bool
{
return false;
}

/**
* Creates a wrapped statement
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
*
Expand Down
8 changes: 0 additions & 8 deletions lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,6 @@ public function getServerVersion() : string
return $serverInfo['SQLServerVersion'];
}

/**
* {@inheritdoc}
*/
public function requiresQueryForServerVersion() : bool
{
return false;
}

/**
* {@inheritDoc}
*/
Expand Down
7 changes: 0 additions & 7 deletions lib/Doctrine/DBAL/Driver/ServerInfoAwareConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
4 changes: 0 additions & 4 deletions tests/Doctrine/Tests/DBAL/ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down
38 changes: 0 additions & 38 deletions tests/Doctrine/Tests/DBAL/Driver/IBMDB2/DB2ConnectionTest.php

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ protected function setUp() : void
->getMockForAbstractClass();
}

public function testDoesNotRequireQueryForServerVersion() : void
{
self::assertFalse($this->connectionMock->requiresQueryForServerVersion());
}

public function testRestoresErrorHandlerOnException() : void
{
$handler = static function () : bool {
Expand Down
38 changes: 0 additions & 38 deletions tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8ConnectionTest.php

This file was deleted.

This file was deleted.

38 changes: 0 additions & 38 deletions tests/Doctrine/Tests/DBAL/Driver/SQLSrv/SQLSrvConnectionTest.php

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 7a0aa2d

Please sign in to comment.