Deprecate Connection::getWrappedConnection(), mark Connection::connect() internal

[morozov]

Q	A
Type	deprecation
BC Break	no

Currently, Connection::getWrappedConnection() is only used by the connection class itself and the tests. This method will return the object that is immediately wrapped into the wrapper connection. If there's a middleware between the wrapper and the actual driver, it is not guaranteed that there is a way to get to the actual driver.

If it is necessary to get the underlying driver connection, e.g. for integration with another database layer, it's possible to implement a custom driver:

use Doctrine\DBAL\Driver as DriverInterface;

class Driver implements DriverInterface
{
    /** @var DriverInterface */
    private $driver;

    /** @var Connection */
    private $connection;

    public function __construct(DriverInterface $driver)
    {
        $this->driver = $driver;
    }

    public function connect(array $params)
    {
        $this->connection = $this->driver->connect($params);

        return $this->connection;
    }

    public function getWrappedConnection()
    {
        return $this->connection;
    }
}

See a complete example of a driver middleware for more details.

In DBAL 4.0, the Connection::connect() method will return the underlying driver connection and will be made protected.

[derrabus]

In the past, I've worked on several codebases that did access the native connection occasionally.

• One application had to do large bulk exports from MySQL. The code unwrapped the PDO connection and temporarily switched it to unbuffered query mode.
• Another application used a Postgres database as a message queue. For that matter, the PDO connection is unwrapped to gain access to the pgsqlGetNotify() method. This is by the way also what Symfony Messenger does when operating on a DBAL/Postgres connection.

Both applications were Symfony applications that configured DBAL through DoctrineBundle. Implementing a custom driver just to gain access to the native connection wouldn't be straightforward in these setups.

I understand that the motivation behind the deprecation is to push the middleware architecture in which the native connection in the bottom of a stack with multiple layers. However, I believe that accessing the native connection can be a valid thing to do and we should neither forbid it completely nor make it unnecessarily difficult.

Since Doctrine\DBAL\Connection will always be the top of that stack, maybe we can implement a method on that class that makes it easy to retrieve the native connection.

[morozov]

[…] I believe that accessing the native connection can be a valid thing to do and we should neither forbid it completely nor make it unnecessarily difficult.

@derrabus in my opinion, it's reasonably difficult since the idea of retrieving an underlying connection undermines the purpose of the DBAL as such. What exactly is difficult about implementing a middleware? It's mostly a boilerplate code.

In my opinion, the absence of an API that breaks the library's own abstraction while it is possible to break it if needed is the most optimal solution. Think of it as of breaking the glass in case of emergency.

As you can see, all the APIs that rely on breaking the abstraction become a liability at a certain point in time. I'm referring to the insanity of the if (instanceof) code blocks in the code and the misery of the supports*() method.

[morozov]

To the point of being able to access the native connection, note, this PR deprecates only the wrapper-level getWrappedConnection() keeping the driver-level method with the same name intact for now.

A great example of this method being unreliable is the failure of this test in #4967:

dbal/tests/Functional/Driver/IBMDB2/ConnectionTest.php

Lines 39 to 43 in 10df50f

	public function testPrepareFailure(): void
	{
	$driverConnection = $this->connection->getWrappedConnection();
	$re = new ReflectionProperty($driverConnection, 'conn');

It expects that the method will return an ibm_db2 driver connection but it returns the middleware. Even if we want to provide an API for obtaining the native connection (which I'm still against), this method will have to be reworked (as it doesn't do the job).

[derrabus]

the idea of retrieving an underlying connection undermines the purpose of the DBAL as such

I don't think so. By providing access to the native connection, we merely acknowledge that the DBAL cannot provide abstraction for features that are very specific to a certain DB engine. If 99% of an application accesses the database through the DBAL, but for one operation, the native connection is used, the DBAL is still a good choice for that application, I'd say.

Even if we want to provide an API for obtaining the native connection (which I'm still against), this method will have to be reworked (as it doesn't do the job).

Agreed, getWrappedConnection() is a brittle way for accessing the native connection, especially when a middleware is involved. It's just that this is currently the most accessible way to do this. So if we deprecate getWrappedConnection(), I'd like to come up with a better replacement for the use-case of retrieving the native connection.

I understand that you don't see that use-case and that you would like to avoid maintaining leaky abstractions. But I believe that the need for accessing the native connection accasionally is there.

[derrabus]

I have worked out a proposal to address the concern of accessing the native connection: #5037. With that change implemented, I could live with deprecating and dropping getWrappedConnection() everywhere.

[tony-sol]

So, how to make reconnect in deamon with deprecated public access to Connection::connect()?

[morozov]

The wrapper will connect automatically upon the next interaction with the database.

[VincentLanglet]

HI @morozov

How to get rid of the deprecation in the following code

$connection = DriverManager::getConnection(['url' => 'sqlite:///:memory:']);
$driverConnection = $connection->getWrappedConnection();
$iterator = new DoctrineDBALConnectionSourceIterator($driverConnection, ' ');

In our DoctrineDBALConnectionSourceIterator we're using the method Doctrine\DBAL\Driver\Connection::prepare so we cannot use getNativeConnection which returns ressource|object according to the doc.
But if the getWrappedConnection is deprecated I don't see another way to have a Driver\Connection.

[morozov]

Why do you use the driver connection in your code?

[VincentLanglet]

Why do you use the driver connection in your code?

It's use here: https://github.com/sonata-project/exporter/blob/3.x/src/Source/DoctrineDBALConnectionSourceIterator.php#L73

From a string query, prepare is call to get a Statement, and then we call execute to get a Result.

[morozov]

Why can’t you use the wrapper connection itself?

[VincentLanglet]

Why can’t you use the wrapper connection itself?

Indeed, I can.
Thanks you a lot !