8000 DBAL-990 Attempt platform detection even when the database name is not set by deeky666 · Pull Request #2671 · doctrine/dbal · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

DBAL-990 Attempt platform detection even when the database name is not set #2671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 10, 2017
Merged

DBAL-990 Attempt platform detection even when the database name is not set #2671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion lib/Doctrine/DBAL/Connection.php
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -425,9 +425,51 @@ private function getDatabasePlatformVersion()

// If not connected, we need to connect now to determine the platform version.
if (null === $this->_conn) {
$this->connect();
try {
$this->connect();
} catch (\Exception $originalException) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a more specific exception that we can catch here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ocramius I do not know what else we could catch here as the driver interface does not specify what to throw in case $driver->connect() fails. Any ideas welcome, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good-enough explanation for me

if (empty($this->_params['dbname'])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use ! $this->_params['dbname'].

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I can't because it is possible to instantiate a connection object without this parameter. I even fell into this trap during writing the tests.

throw $originalException;
}

// The database to connect to might not yet exist.
// Retry detection without database name connection parameter.
$databaseName = $this->_params['dbname'];
$this->_params['dbname'] = null;

try {
$this->connect();
} catch (\Exception $fallbackException) {
// Either the platform does not support database-less connections
// or something else went wrong.
// Reset connection parameters and rethrow the original exception.
$this->_params['dbname'] = $databaseName;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this and the identical line below be put into a finally block?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattjanssen I thought about that but as we need to backport this to 2.5 which still supports PHP 5.3, this unfortunately is not an option

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On this note, the merged fixes don't appear to have made it back to the 2.5 branch.

https://github.com/doctrine/dbal/blob/2.5/lib/Doctrine/DBAL/Connection.php#L405-L441


throw $originalException;
}

// Reset connection parameters.
$this->_params['dbname'] = $databaseName;
$serverVersion = $this->getServerVersion();

// Close "temporary" connection to allow connecting to the real database again.
$this->close();

return $serverVersion;
}

}

return $this->getServerVersion();
}

/**
* Returns the database server version if the underlying driver supports it.
*
* @return string|null
*/
private function getServerVersion()
{
// Automatic platform version detection.
if ($this->_conn instanceof ServerInfoAwareConnection &&
! $this->_conn->requiresQueryForServerVersion()
Expand Down
26 changes: 26 additions & 0 deletions tests/Doctrine/Tests/DBAL/ConnectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Doctrine\Tests\Mocks\DriverConnectionMock;
use Doctrine\Tests\Mocks\DriverMock;
use Doctrine\DBAL\Cache\ArrayStatement;
use Doctrine\Tests\Mocks\VersionAwarePlatformDriverMock;

class ConnectionTest extends \Doctrine\Tests\DbalTestCase
{
Expand Down Expand Up @@ -712,4 +713,29 @@ public function testConnectionParamsArePassedToTheQueryCacheProfileInExecuteCach
(new Connection($this->params, $driver))->executeCacheQuery($query, $params, $types, $queryCacheProfileMock)
);
}

/**
* @group DBAL-990
*/
public function testRethrowsOriginalExceptionOnDeterminingPlatformWhenConnectingToNonExistentDatabase()
{
/** @var \Doctrine\Tests\Mocks\VersionAwarePlatformDriverMock|\PHPUnit_Framework_MockObject_MockObject $driverMock */
$driverMock = $this->createMock(VersionAwarePlatformDriverMock::class);

$connection = new Connection(array('dbname' => 'foo'), $driverMock);
$originalException = new \Exception('Original exception');
$fallbackException = new \Exception('Fallback exception');

$driverMock->expects($this->at(0))
->method('connect')
->willThrowException($originalException);

$driverMock->expects($this->at(1))
->method('connect')
->willThrowException($fallbackException);

$this->expectExceptionMessage($originalException->getMessage());

$connection->getDatabasePlatform();
}
}
26 changes: 26 additions & 0 deletions tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Doctrine\DBAL\ConnectionException;
use Doctrine\DBAL\DriverManager;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\Type;

class ConnectionTest extends \Doctrine\Tests\DbalFunctionalTestCase
Expand Down Expand Up @@ -272,4 +273,29 @@ public function testConnectWithoutExplicitDatabaseName()

$connection->close();
}

/**
* @group DBAL-990
*/
public function testDeterminesDatabasePlatformWhenConnectingToNonExistentDatabase()
{
if (in_array($this->_conn->getDatabasePlatform()->getName(), ['oracle', 'db2'], true)) {
$this->markTestSkipped('Platform does not support connecting without database name.');
}

$params = $this->_conn->getParams();
$params['dbname'] = 'foo_bar';

$connection = DriverManager::getConnection(
$params,
$this->_conn->getConfiguration(),
$this->_conn->getEventManager()
);

$this->assertInstanceOf(AbstractPlatform::class, $connection->getDatabasePlatform());
$this->assertFalse($connection->isConnected());
$this->assertSame($params, $connection->getParams());

$connection->close();
}
}
0