Merge pull request doctrine#5041 from morozov/remove-get-wrapped-conn…

…ection

Remove Connection::getWrappedConnection(), make Connection::connect() protected

UPGRADE.md

@@ -8,6 +8,11 @@ awareness about deprecated code.
 
 # Upgrade to 4.0
 
+## BC BREAK: Removed `Connection::getWrappedConnection()`, `Connection::connect()` made `protected`.
+
+The wrapper-level `Connection::getWrappedConnection()` method has been removed. The `Connection::connect()` method
+has been made `protected` and now must return the underlying driver-level connection.
+
 ## BC BREAK: Removed `SQLLogger` and its implementations.
 
 The `SQLLogger` interface and its implementations `DebugStack` and `LoggerChain` have been removed.

psalm.xml.dist

@@ -58,11 +58,6 @@
 
                 <!-- TODO: remove in 4.0.0 -->
                 <referencedMethod name="Doctrine\DBAL\Driver\PDO\Connection::getWrappedConnection"/>
-                <!--
-                    TODO: remove in 4.0.0
-                    See https://github.com/doctrine/dbal/pull/4966
-                -->
-                <referencedMethod name="Doctrine\DBAL\Connection::getWrappedConnection"/>
             </errorLevel>
         </DeprecatedMethod>
         <DocblockTypeContradiction>

src/Connection.php

@@ -31,7 +31,6 @@
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\SQL\Parser;
 use Doctrine\DBAL\Types\Type;
-use Doctrine\Deprecations\Deprecation;
 use LogicException;
 use Throwable;
 use Traversable;
@@ -260,26 +259,18 @@ public function createExpressionBuilder(): ExpressionBuilder
     }
 
     /**
-     * Establishes the connection with the database.
-     *
-     * @internal This method will be made protected in DBAL 4.0.
+     * Establishes the connection with the database and returns the underlying connection.
      *
      * @throws Exception
      */
-    public function connect(): void
+    protected function connect(): DriverConnection
     {
-        Deprecation::triggerIfCalledFromOutside(
-            'doctrine/dbal',
-            'https://github.com/doctrine/dbal/issues/4966',
-            'Public access to Connection::connect() is deprecated.'
-        );
-
         if ($this->_conn !== null) {
-            return;
+            return $this->_conn;
         }
 
         try {
-            $this->_conn = $this->_driver->connect($this->params);
+            $connection = $this->_conn = $this->_driver->connect($this->params);
         } catch (Driver\Exception $e) {
             throw $this->convertException($e);
         }
@@ -288,12 +279,12 @@ public function connect(): void
             $this->beginTransaction();
         }
 
-        if (! $this->_eventManager->hasListeners(Events::postConnect)) {
-            return;
+        if ($this->_eventManager->hasListeners(Events::postConnect)) {
+            $eventArgs = new Event\ConnectionEventArgs($this);
+            $this->_eventManager->dispatchEvent(Events::postConnect, $eventArgs);
         }
 
-        $eventArgs = new Event\ConnectionEventArgs($this);
-        $this->_eventManager->dispatchEvent(Events::postConnect, $eventArgs);
+        return $connection;
     }
 
     /**
@@ -320,7 +311,7 @@ public function getServerVersion(): string
     private function getServerVersionConnection(): DriverConnection
     {
         try {
-            return $this->getWrappedConnection();
+            return $this->connect();
         } catch (Exception $e) {
             if (! isset($this->params['dbname'])) {
                 throw $e;
@@ -333,7 +324,7 @@ private function getServerVersionConnection(): DriverConnection
             unset($this->params['dbname']);
 
             try {
-                return $this->getWrappedConnection();
+                return $this->connect();
             } catch (Exception $_) {
                 // Either the platform does not support database-less connections
                 // or something else went wrong.
@@ -666,7 +657,7 @@ public function quoteIdentifier(string $identifier): string
      */
     public function quote(string $value): string
     {
-        return $this->getWrappedConnection()->quote($value);
+        return $this->connect()->quote($value);
     }
 
     /**
@@ -837,7 +828,7 @@ public function iterateColumn(string $query, array $params = [], array $types =
      */
     public function prepare(string $sql): Statement
     {
-        $connection = $this->getWrappedConnection();
+        $connection = $this->connect();
 
         try {
             $statement = $connection->prepare($sql);
@@ -868,7 +859,7 @@ public function executeQuery(
             return $this->executeCacheQuery($sql, $params, $types, $qcp);
         }
 
-        $connection = $this->getWrappedConnection();
+        $connection = $this->connect();
 
         try {
             if (count($params) > 0) {
@@ -961,7 +952,7 @@ public function executeCacheQuery(string $sql, array $params, array $types, Quer
      */
     public function executeStatement(string $sql, array $params = [], array $types = []): int
     {
-        $connection = $this->getWrappedConnection();
+        $connection = $this->connect();
 
         try {
             if (count($params) > 0) {
@@ -1010,7 +1001,7 @@ public function getTransactionNestingLevel(): int
     public function lastInsertId()
     {
         try {
-            return $this->getWrappedConnection()->lastInsertId();
+            return $this->connect()->lastInsertId();
         } catch (Driver\Exception $e) {
             throw $this->convertException($e);
         }
@@ -1087,7 +1078,7 @@ protected function _getNestedTransactionSavePointName()
      */
     public function beginTransaction(): void
     {
-        $connection = $this->getWrappedConnection();
+        $connection = $this->connect();
 
         ++$this->transactionNestingLevel;
 
@@ -1113,7 +1104,7 @@ public function commit(): void
             throw CommitFailedRollbackOnly::new();
         }
 
-        $connection = $this->getWrappedConnection();
+        $connection = $this->connect();
 
         if ($this->transactionNestingLevel === 1) {
             try {
@@ -1165,7 +1156,7 @@ public function rollBack(): void
             throw NoActiveTransaction::new();
         }
 
-        $connection = $this->getWrappedConnection();
+        $connection = $this->connect();
 
         if ($this->transactionNestingLevel === 1) {
             $this->transactionNestingLevel = 0;
@@ -1251,44 +1242,22 @@ public function rollbackSavepoint(string $savepoint): void
     }
 
     /**
-     * Gets the wrapped driver connection.
-     *
-     * @deprecated Use {@link getNativeConnection()} to access the native connection.
+     * @return resource|object
      *
      * @throws Exception
      */
-    public function getWrappedConnection(): DriverConnection
-    {
-        Deprecation::triggerIfCalledFromOutside(
-            'doctrine/dbal',
-            'https://github.com/doctrine/dbal/issues/4966',
-            'Connection::getWrappedConnection() is deprecated.'
-                . ' Use Connection::getNativeConnection() to access the native connection.'
-        );
-
-        $this->connect();
-
-        assert($this->_conn !== null);
-
-        return $this->_conn;
-    }
-
-    /**
-     * @return resource|object
-     */
     public function getNativeConnection()
     {
-        $this->connect();
+        $connection = $this->connect();
 
-        assert($this->_conn !== null);
-        if (! method_exists($this->_conn, 'getNativeConnection')) {
+        if (! method_exists($connection, 'getNativeConnection')) {
             throw new LogicException(sprintf(
                 'The driver connection %s does not support accessing the native connection.',
-                get_class($this->_conn)
+                get_class($connection)
             ));
         }
 
-        return $this->_conn->getNativeConnection();
+        return $connection->getNativeConnection();
     }
 
     /**

src/Connections/PrimaryReadReplicaConnection.php

@@ -137,7 +137,7 @@ public function isConnectedToPrimary(): bool
         return $this->_conn !== null && $this->_conn === $this->connections['primary'];
     }
 
-    public function connect(?string $connectionName = null): void
+    public function connect(?string $connectionName = null): DriverConnection
     {
         if ($connectionName !== null) {
             throw new InvalidArgumentException(
@@ -146,10 +146,10 @@ public function connect(?string $connectionName = null): void
             );
         }
 
-        $this->performConnect();
+        return $this->performConnect();
     }
 
-    protected function performConnect(?string $connectionName = null): void
+    protected function performConnect(?string $connectionName = null): DriverConnection
     {
         $requestedConnectionChange = ($connectionName !== null);
         $connectionName          ??= 'replica';
@@ -162,7 +162,7 @@ protected function performConnect(?string $connectionName = null): void
         // change request, then abort right here, because we are already done.
         // This prevents writes to the replica in case of "keepReplica" option enabled.
         if ($this->_conn !== null && ! $requestedConnectionChange) {
-            return;
+            return $this->_conn;
         }
 
         $forcePrimaryAsReplica = false;
@@ -179,7 +179,7 @@ protected function performConnect(?string $connectionName = null): void
                 $this->connections['replica'] = $this->_conn;
             }
 
-            return;
+            return $this->_conn;
         }
 
         if ($connectionName === 'primary') {
@@ -193,12 +193,12 @@ protected function performConnect(?string $connectionName = null): void
             $this->connections['replica'] = $this->_conn = $this->connectTo($connectionName);
         }
 
-        if (! $this->_eventManager->hasListeners(Events::postConnect)) {
-            return;
+        if ($this->_eventManager->hasListeners(Events::postConnect)) {
+            $eventArgs = new ConnectionEventArgs($this);
+            $this->_eventManager->dispatchEvent(Events::postConnect, $eventArgs);
         }
 
-        $eventArgs = new ConnectionEventArgs($this);
-        $this->_eventManager->dispatchEvent(Events::postConnect, $eventArgs);
+        return $this->_conn;
     }
 
     /**

tests/ConnectionTest.php

@@ -121,7 +121,7 @@ public function testConnectDispatchEvent(): void
                    ->method('connect');
 
         $conn = new Connection([], $driverMock, new Configuration(), $eventManager);
-        $conn->connect();
+        $conn->executeQuery('SELECT 1');
     }
 
     public function testTransactionBeginDispatchEvent(): void
@@ -304,7 +304,7 @@ public function testConnectStartsTransactionInNoAutoCommitMode(): void
 
         self::assertFalse($conn->isTransactionActive());
 
-        $conn->connect();
+        $conn->executeQuery('SELECT 1');
 
         self::assertTrue($conn->isTransactionActive());
     }
@@ -316,7 +316,7 @@ public function testCommitStartsTransactionInNoAutoCommitMode(): void
         $conn = new Connection([], $driverMock);
 
         $conn->setAutoCommit(false);
-        $conn->connect();
+        $conn->executeQuery('SELECT 1');
         $conn->commit();
 
         self::assertTrue($conn->isTransactionActive());
@@ -337,7 +337,7 @@ public function testRollBackStartsTransactionInNoAutoCommitMode(): void
         $conn = new Connection([], $driverMock);
 
         $conn->setAutoCommit(false);
-        $conn->connect();
+        $conn->executeQuery('SELECT 1');
         $conn->rollBack();
 
         self::assertTrue($conn->isTransactionActive());
@@ -349,7 +349,6 @@ public function testSwitchingAutoCommitModeCommitsAllCurrentTransactions(): void
 
         $conn = new Connection([], $driverMock);
 
-        $conn->connect();
         $conn->beginTransaction();
         $conn->beginTransaction();
         $conn->setAutoCommit(false);
@@ -633,8 +632,8 @@ public function testCallConnectOnce(): void
         $platform = $this->createMock(AbstractPlatform::class);
 
         $conn = new Connection(['platform' => $platform], $driver);
-        $conn->connect();
-        $conn->connect();
+        $conn->executeQuery('SELECT 1');
+        $conn->executeQuery('SELECT 2');
     }
 
     public function testPlatformDetectionTriggersConnectionIfRequiredByTheDriver(): void

tests/Functional/ConnectionTest.php

@@ -6,7 +6,6 @@
 
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\ConnectionException;
-use Doctrine\DBAL\Driver\PDO\Connection as PDOConnection;
 use Doctrine\DBAL\DriverManager;
 use Doctrine\DBAL\Exception\DriverException;
 use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
@@ -318,11 +317,9 @@ public function testConnectWithoutExplicitDatabaseName(): void
             $this->connection->getEventManager()
         );
 
-        $connection->connect();
-
-        self::assertTrue($connection->isConnected());
-
-        $connection->close();
+        self::assertEquals(1, $connection->fetchOne(
+            $platform->getDummySelectSQL()
+        ));
     }
 
     public function testDeterminesDatabasePlatformWhenConnectingToNonExistentDatabase(): void
@@ -364,15 +361,13 @@ public function testPersistentConnection(): void
         $params['persistent'] = true;
 
         $connection       = DriverManager::getConnection($params);
-        $driverConnection = $connection->getWrappedConnection();
+        $nativeConnection = $connection->getNativeConnection();
 
-        if (! $driverConnection instanceof PDOConnection) {
+        if (! $nativeConnection instanceof PDO) {
             self::markTestSkipped('Unable to test if the connection is persistent');
         }
 
-        $pdo = $driverConnection->getNativeConnection();
-
-        self::assertTrue($pdo->getAttribute(PDO::ATTR_PERSISTENT));
+        self::assertTrue($nativeConnection->getAttribute(PDO::ATTR_PERSISTENT));
     }
 
     public function testExceptionOnExecuteStatement(): void