From d58873bfbb5be7173f1e38ef8b41649c0a5c69aa Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Mon, 20 May 2019 15:22:56 -0700 Subject: [PATCH] Remove user provided PDO functionality --- UPGRADE.md | 4 + lib/Doctrine/DBAL/Connection.php | 6 - lib/Doctrine/DBAL/DriverManager.php | 18 +-- .../DBAL/Exception/InvalidPdoInstance.php | 15 --- tests/Doctrine/Tests/DBAL/ConnectionTest.php | 77 ++---------- .../Tests/DBAL/Driver/PDOExceptionTest.php | 8 +- .../Doctrine/Tests/DBAL/DriverManagerTest.php | 118 +----------------- .../Tests/DBAL/Functional/ConnectionTest.php | 12 -- .../Functional/Driver/PDOConnectionTest.php | 8 +- .../DBAL/Functional/PDOStatementTest.php | 8 +- 10 files changed, 29 insertions(+), 245 deletions(-) delete mode 100644 lib/Doctrine/DBAL/Exception/InvalidPdoInstance.php diff --git a/UPGRADE.md b/UPGRADE.md index aec7316971d..d1fd5c9fc39 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,9 @@ # Upgrade to 3.0 +## BC BREAK User-provided `PDO` instance is no longer supported + +In order to share the same `PDO` instances between DBAL and other components, initialize the connection in DBAL and access it using `Connection::getWrappedConnection()->getWrappedConnection()`. + ## BC BREAK PostgreSqlPlatform ForeignKeyConstraint support for `feferred` misspelling removed `PostgreSqlPlatform::getAdvancedForeignKeyOptionsSQL()` had a typo in it in 2.x. Both the option name diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index 295681829d3..1d065e941b6 100644 --- a/lib/Doctrine/DBAL/Connection.php +++ b/lib/Doctrine/DBAL/Connection.php @@ -197,12 +197,6 @@ public function __construct( $this->_driver = $driver; $this->params = $params; - if (isset($params['pdo'])) { - $this->_conn = $params['pdo']; - $this->isConnected = true; - unset($this->params['pdo']); - } - if (isset($params['platform'])) { if (! $params['platform'] instanceof Platforms\AbstractPlatform) { throw InvalidPlatformType::new($params['platform']); diff --git a/lib/Doctrine/DBAL/DriverManager.php b/lib/Doctrine/DBAL/DriverManager.php index 4a16ee1735c..3699efc71e5 100644 --- a/lib/Doctrine/DBAL/DriverManager.php +++ b/lib/Doctrine/DBAL/DriverManager.php @@ -17,10 +17,8 @@ use Doctrine\DBAL\Driver\SQLSrv\Driver as SQLSrvDriver; use Doctrine\DBAL\Exception\DriverRequired; use Doctrine\DBAL\Exception\InvalidDriverClass; -use Doctrine\DBAL\Exception\InvalidPdoInstance; use Doctrine\DBAL\Exception\InvalidWrapperClass; use Doctrine\DBAL\Exception\UnknownDriver; -use PDO; use function array_keys; use function array_map; use function array_merge; @@ -175,17 +173,7 @@ public static function getConnection( } } - // check for existing pdo object - if (isset($params['pdo']) && ! $params['pdo'] instanceof PDO) { - throw InvalidPdoInstance::new(); - } - - if (isset($params['pdo'])) { - $params['pdo']->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); - $params['driver'] = 'pdo_' . $params['pdo']->getAttribute(PDO::ATTR_DRIVER_NAME); - } else { - self::_checkParams($params); - } + self::_checkParams($params); $className = $params['driverClass'] ?? self::$_driverMap[$params['driver']]; @@ -281,10 +269,6 @@ private static function parseDatabaseUrl(array $params) : array $url = array_map('rawurldecode', $url); - // If we have a connection URL, we have to unset the default PDO instance connection parameter (if any) - // as we cannot merge connection details from the URL into the PDO instance (URL takes precedence). - unset($params['pdo']); - $params = self::parseDatabaseUrlScheme($url, $params); if (isset($url['host'])) { diff --git a/lib/Doctrine/DBAL/Exception/InvalidPdoInstance.php b/lib/Doctrine/DBAL/Exception/InvalidPdoInstance.php deleted file mode 100644 index dd5609e65d9..00000000000 --- a/lib/Doctrine/DBAL/Exception/InvalidPdoInstance.php +++ /dev/null @@ -1,15 +0,0 @@ -fetchAll($statement, $params, $types)); } - public function testConnectionDoesNotMaintainTwoReferencesToExternalPDO() : void - { - $params['pdo'] = new stdClass(); - - $driverMock = $this->createMock(Driver::class); - - $conn = new Connection($params, $driverMock); - - self::assertArrayNotHasKey('pdo', $conn->getParams(), 'Connection is maintaining additional reference to the PDO connection'); - } - - public function testPassingExternalPDOMeansConnectionIsConnected() : void - { - $params['pdo'] = new stdClass(); - - $driverMock = $this->createMock(Driver::class); - - $conn = new Connection($params, $driverMock); - - self::assertTrue($conn->isConnected(), 'Connection is not connected after passing external PDO'); - } - public function testCallingDeleteWithNoDeletionCriteriaResultsInInvalidArgumentException() : void { /** @var Driver $driver */ - $driver = $this->createMock(Driver::class); - $pdoMock = $this->createMock(\Doctrine\DBAL\Driver\Connection::class); - - // should never execute queries with invalid arguments - $pdoMock->expects($this->never())->method('exec'); - $pdoMock->expects($this->never())->method('prepare'); - - $conn = new Connection(['pdo' => $pdoMock], $driver); + $driver = $this->createMock(Driver::class); + $conn = new Connection([], $driver); $this->expectException(InvalidArgumentException::class); $conn->delete('kittens', []); } - /** - * @return array> - */ - public static function dataCallConnectOnce() : iterable - { - return [ - ['delete', ['tbl', ['id' => 12345]]], - ['insert', ['tbl', ['data' => 'foo']]], - ['update', ['tbl', ['data' => 'bar'], ['id' => 12345]]], - ['prepare', ['select * from dual']], - ['executeUpdate', ['insert into tbl (id) values (?)'], [123]], - ]; - } - - /** - * @param array $params - * - * @dataProvider dataCallConnectOnce - */ - public function testCallConnectOnce(string $method, array $params) : void + public function testCallConnectOnce() : void { - $driverMock = $this->createMock(Driver::class); - $pdoMock = $this->createMock(Connection::class); - $platformMock = $this->createMock(AbstractPlatform::class); - $stmtMock = $this->createMock(Statement::class); - - $pdoMock->expects($this->any()) - ->method('prepare') - ->will($this->returnValue($stmtMock)); - - $conn = $this->getMockBuilder(Connection::class) - ->setConstructorArgs([['pdo' => $pdoMock, 'platform' => $platformMock], $driverMock]) - ->setMethods(['connect']) - ->getMock(); + /** @var Driver|MockObject $driver */ + $driver = $this->createMock(Driver::class); + $driver->expects($this->once()) + ->method('connect'); - $conn->expects($this->once())->method('connect'); + $platform = $this->createMock(AbstractPlatform::class); - call_user_func_array([$conn, $method], $params); + $conn = new Connection(['platform' => $platform], $driver); + $conn->connect(); + $conn->connect(); } /** diff --git a/tests/Doctrine/Tests/DBAL/Driver/PDOExceptionTest.php b/tests/Doctrine/Tests/DBAL/Driver/PDOExceptionTest.php index 08623925499..39c642e4a0c 100644 --- a/tests/Doctrine/Tests/DBAL/Driver/PDOExceptionTest.php +++ b/tests/Doctrine/Tests/DBAL/Driver/PDOExceptionTest.php @@ -7,8 +7,10 @@ use Doctrine\DBAL\Driver\PDOException; use Doctrine\Tests\DbalTestCase; use PHPUnit\Framework\MockObject\MockObject; -use function extension_loaded; +/** + * @requires extension pdo + */ class PDOExceptionTest extends DbalTestCase { public const ERROR_CODE = 666; @@ -33,10 +35,6 @@ class PDOExceptionTest extends DbalTestCase protected function setUp() : void { - if (! extension_loaded('PDO')) { - $this->markTestSkipped('PDO is not installed.'); - } - parent::setUp(); $this->wrappedException = new \PDOException(self::MESSAGE); diff --git a/tests/Doctrine/Tests/DBAL/DriverManagerTest.php b/tests/Doctrine/Tests/DBAL/DriverManagerTest.php index f44b9f5bf6b..9a688b85742 100644 --- a/tests/Doctrine/Tests/DBAL/DriverManagerTest.php +++ b/tests/Doctrine/Tests/DBAL/DriverManagerTest.php @@ -16,50 +16,13 @@ use Doctrine\DBAL\Sharding\PoolingShardConnection; use Doctrine\DBAL\Sharding\ShardChoser\MultiTenantShardChoser; use Doctrine\Tests\DbalTestCase; -use PDO; use stdClass; -use function extension_loaded; use function get_class; use function in_array; use function is_array; class DriverManagerTest extends DbalTestCase { - /** - * @requires extension pdo_sqlite - */ - public function testInvalidPdoInstance() : void - { - $this->expectException(DBALException::class); - DriverManager::getConnection(['pdo' => 'test']); - } - - /** - * @requires extension pdo_sqlite - */ - public function testValidPdoInstance() : void - { - $conn = DriverManager::getConnection([ - 'pdo' => new PDO('sqlite::memory:'), - ]); - - self::assertEquals('sqlite', $conn->getDatabasePlatform()->getName()); - } - - /** - * @group DBAL-32 - * @requires extension pdo_sqlite - */ - public function testPdoInstanceSetErrorMode() : void - { - $pdo = new PDO('sqlite::memory:'); - $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT); - $options = ['pdo' => $pdo]; - - DriverManager::getConnection($options); - self::assertEquals(PDO::ERRMODE_EXCEPTION, $pdo->getAttribute(PDO::ATTR_ERRMODE)); - } - public function testCheckParams() : void { $this->expectException(DBALException::class); @@ -81,7 +44,7 @@ public function testCustomPlatform() : void { $platform = $this->createMock(AbstractPlatform::class); $options = [ - 'pdo' => new PDO('sqlite::memory:'), + 'url' => 'sqlite::memory:', 'platform' => $platform, ]; @@ -98,7 +61,7 @@ public function testCustomWrapper() : void $wrapperClass = get_class($wrapper); $options = [ - 'pdo' => new PDO('sqlite::memory:'), + 'url' => 'sqlite::memory:', 'wrapperClass' => $wrapperClass, ]; @@ -114,7 +77,7 @@ public function testInvalidWrapperClass() : void $this->expectException(DBALException::class); $options = [ - 'pdo' => new PDO('sqlite::memory:'), + 'url' => 'sqlite::memory:', 'wrapperClass' => stdClass::class, ]; @@ -216,16 +179,6 @@ public function testDatabaseUrl($url, $expected) : void { $options = is_array($url) ? $url : ['url' => $url]; - if (isset($options['pdo'])) { - if (! extension_loaded('pdo')) { - $this->markTestSkipped('PDO is not installed'); - } - - $options['pdo'] = $this->createMock(PDO::class); - } - - $options = is_array($url) ? $url : ['url' => $url]; - if ($expected === false) { $this->expectException(DBALException::class); } @@ -234,7 +187,7 @@ public function testDatabaseUrl($url, $expected) : void $params = $conn->getParams(); foreach ($expected as $key => $value) { - if (in_array($key, ['pdo', 'driver', 'driverClass'], true)) { + if (in_array($key, ['driver', 'driverClass'], true)) { self::assertInstanceOf($value, $conn->getDriver()); } else { self::assertEquals($value, $params[$key]); @@ -395,13 +348,6 @@ public function databaseUrls() : iterable ['url' => '//foo:bar@localhost/baz'], false, ], - 'URL without scheme but default PDO driver' => [ - [ - 'url' => '//foo:bar@localhost/baz', - 'pdo' => true, - ], - false, - ], 'URL without scheme but default driver' => [ [ 'url' => '//foo:bar@localhost/baz', @@ -428,20 +374,6 @@ public function databaseUrls() : iterable 'driverClass' => $driverClass, ], ], - 'URL without scheme but default PDO driver and default driver' => [ - [ - 'url' => '//foo:bar@localhost/baz', - 'pdo' => true, - 'driver' => 'pdo_mysql', - ], - [ - 'user' => 'foo', - 'password' => 'bar', - 'host' => 'localhost', - 'dbname' => 'baz', - 'driver' => PDOMySQLDriver::class, - ], - ], 'URL without scheme but driver and custom driver' => [ [ 'url' => '//foo:bar@localhost/baz', @@ -456,19 +388,6 @@ public function databaseUrls() : iterable 'driverClass' => $driverClass, ], ], - 'URL with default PDO driver' => [ - [ - 'url' => 'mysql://foo:bar@localhost/baz', - 'pdo' => true, - ], - [ - 'user' => 'foo', - 'password' => 'bar', - 'host' => 'localhost', - 'dbname' => 'baz', - 'driver' => PDOMySQLDriver::class, - ], - ], 'URL with default driver' => [ [ 'url' => 'mysql://foo:bar@localhost/baz', @@ -495,20 +414,6 @@ public function databaseUrls() : iterable 'driver' => PDOMySQLDriver::class, ], ], - 'URL with default PDO driver and default driver' => [ - [ - 'url' => 'mysql://foo:bar@localhost/baz', - 'pdo' => true, - 'driver' => 'sqlite', - ], - [ - 'user' => 'foo', - 'password' => 'bar', - 'host' => 'localhost', - 'dbname' => 'baz', - 'driver' => PDOMySQLDriver::class, - ], - ], 'URL with default driver and default custom driver' => [ [ 'url' => 'mysql://foo:bar@localhost/baz', @@ -523,21 +428,6 @@ public function databaseUrls() : iterable 'driver' => PDOMySQLDriver::class, ], ], - 'URL with default PDO driver and default driver and default custom driver' => [ - [ - 'url' => 'mysql://foo:bar@localhost/baz', - 'pdo' => true, - 'driver' => 'sqlite', - 'driverClass' => $driverClass, - ], - [ - 'user' => 'foo', - 'password' => 'bar', - 'host' => 'localhost', - 'dbname' => 'baz', - 'driver' => PDOMySQLDriver::class, - ], - ], ]; } } diff --git a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php index a313539b816..0d06466555e 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php @@ -319,18 +319,6 @@ public function testDeterminesDatabasePlatformWhenConnectingToNonExistentDatabas $connection->close(); } - /** - * @requires extension pdo_sqlite - */ - public function testUserProvidedPDOConnection() : void - { - self::assertTrue( - DriverManager::getConnection([ - 'pdo' => new PDO('sqlite::memory:'), - ])->ping() - ); - } - public function testPersistentConnection() : void { $platform = $this->connection->getDatabasePlatform(); diff --git a/tests/Doctrine/Tests/DBAL/Functional/Driver/PDOConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/Driver/PDOConnectionTest.php index 15e97a0eef7..99a2f0d84c5 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/Driver/PDOConnectionTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/Driver/PDOConnectionTest.php @@ -11,10 +11,12 @@ use Doctrine\DBAL\Driver\PDOSqlsrv\Driver as PDOSQLSRVDriver; use Doctrine\Tests\DbalFunctionalTestCase; use PDO; -use function extension_loaded; use function get_class; use function sprintf; +/** + * @requires extension pdo + */ class PDOConnectionTest extends DbalFunctionalTestCase { /** @@ -26,10 +28,6 @@ class PDOConnectionTest extends DbalFunctionalTestCase protected function setUp() : void { - if (! extension_loaded('PDO')) { - $this->markTestSkipped('PDO is not installed.'); - } - parent::setUp(); $this->driverConnection = $this->connection->getWrappedConnection(); diff --git a/tests/Doctrine/Tests/DBAL/Functional/PDOStatementTest.php b/tests/Doctrine/Tests/DBAL/Functional/PDOStatementTest.php index 5eab33b876a..7780443c0e3 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/PDOStatementTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/PDOStatementTest.php @@ -8,16 +8,14 @@ use Doctrine\DBAL\Schema\Table; use Doctrine\Tests\DbalFunctionalTestCase; use PDO; -use function extension_loaded; +/** + * @requires extension pdo + */ class PDOStatementTest extends DbalFunctionalTestCase { protected function setUp() : void { - if (! extension_loaded('pdo')) { - $this->markTestSkipped('PDO is not installed'); - } - parent::setUp(); if (! $this->connection->getWrappedConnection() instanceof PDOConnection) {