postConnect event not able to access DatabasePlatform

[gregor-tb]

Bug Report

When postConnect event is triggered, the Platform is not ready yet, but the Getter is available, what ends up in multiple instances and broken setup.

Summary / Current behaviour

I want to register type mapping on platform:

$connection->getDatabasePlatform()->registerDoctrineTypeMapping('enum', 'string');

Because I don't want to connect to the database without need by calling this in bootstrap, I use the EventSubscriber and put the call in the postConnect Method.

But when the Connection attempt itself was caused by Connection::getDatabasePlatform(), it fails.

Here is what happens:

1. Some code (migration in my case), requests Connection::getDatabasePlatform()
2. internal member $_platform is null, so Connection::detectDatabasePlatform() is called
3. detectDatabasePlatform calls Connection::getDatabasePlatformVersion()
4. This connects to the database, and triggers the postConnect event
5. My subscriber code is executed, BUT _Connection::$platform is still NULL, so a new instance is created, but already $isConnected = true (so no event loop)
6. the original detectDatabasePlatform overwrites _Connection::$platform (what is not NULL anymore)

The registerDoctrineTypeMapping is gone...

Expected behaviour

Accessing the Platform while it is "building" should throw an exception and an own event fired after platform was created successful and safe to use.

OR

The postConnect event should be hold back after platform is ready, maybe?

[morozov]

the Getter is available, what ends up in multiple instances and broken setup.

I think I understand the scenario but I don't understand what's wrong from the API consumer standpoint. Here's my example:

use Doctrine\Common\EventSubscriber;
use Doctrine\DBAL\Event\ConnectionEventArgs;
use Doctrine\DBAL\Events;
use Doctrine\DBAL\Tools\Console\ConnectionProvider;

require 'vendor/autoload.php';

/** @var ConnectionProvider $provider */
$provider = require 'config/cli-config.php';
$connection = $provider->getDefaultConnection();

$evm = $connection->getEventManager();
$evm->addEventSubscriber(new class implements EventSubscriber {
    public function postConnect(ConnectionEventArgs $args): void
    {
        var_dump($args->getConnection()->getDatabasePlatform()->getName());
    }

    public function getSubscribedEvents()
    {
        return [Events::postConnect];
    }
});

var_dump($connection->getDatabasePlatform()->getName());

// string(5) "mysql"
// string(5) "mysql"

The above works with the 3.1.x branch.

[gregor-tb]

Hi, thank you for your feedback!

Example in my scenario is:

use Doctrine\Common\EventSubscriber;
use Doctrine\DBAL\Event\ConnectionEventArgs;
use Doctrine\DBAL\Events;
use Doctrine\DBAL\Tools\Console\ConnectionProvider;

require 'vendor/autoload.php';

/** @var ConnectionProvider $provider */
$provider = require 'config/cli-config.php';
$connection = $provider->getDefaultConnection();

$evm = $connection->getEventManager();
$evm->addEventSubscriber(new class implements EventSubscriber {
    public function postConnect(ConnectionEventArgs $args): void
    {
        $args->getConnection()->getDatabasePlatform()->registerDoctrineTypeMapping('enum', 'string');
        var_dump($args->getConnection()->getDatabasePlatform()->hasDoctrineTypeMappingFor('enum'));
    }

    public function getSubscribedEvents()
    {
        return [Events::postConnect];
    }
});

var_dump($connection->getDatabasePlatform()->hasDoctrineTypeMappingFor('enum'));

// true
// false

Tested with 2.10 branch only (still on php 7.2), but I checked the master code, the procedure and result should be similar.

[morozov]

Thanks for the details, @gregor-tb.

Accessing the Platform while it is "building" should throw an exception and an own event fired after platform was created successful and safe to use.

This looks like the right approach to me. In 3.x we can emit a deprecation error and in 4.0 we can promote it to an exception.

The postConnect event should be hold back after platform is ready, maybe?

This isn't the best idea IMO: on the one hand, it's a silent change in the event semantics, a BC break without an upgrade path; on the other, the platform isn't required for the connection to be usable. Introducing a new event seems better.

A patch is welcome.