review feedback + handle missing EventManager arg

doctrine · Oct 21, 2023 · 81a6bde · 81a6bde
1 parent 8ca42b4
commit 81a6bde
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 10 deletions.
diff --git a/ConnectionFactory.php b/ConnectionFactory.php
@@ -5,20 +5,24 @@
 use Doctrine\Common\EventManager;
 use Doctrine\DBAL\Configuration;
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Connection\StaticServerVersionProvider;
+use Doctrine\DBAL\ConnectionException;
 use Doctrine\DBAL\DriverManager;
 use Doctrine\DBAL\Exception as DBALException;
 use Doctrine\DBAL\Exception\DriverException;
 use Doctrine\DBAL\Exception\DriverRequired;
 use Doctrine\DBAL\Exception\InvalidWrapperClass;
+use Doctrine\DBAL\Exception\MalformedDsnException;
 use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Tools\DsnParser;
 use Doctrine\DBAL\Types\Type;
 use Doctrine\Deprecations\Deprecation;
-use RuntimeException;
+use InvalidArgumentException;
 
 use function array_merge;
 use function class_exists;
+use function interface_exists;
 use function is_subclass_of;
 use function trigger_deprecation;
 
@@ -65,6 +69,10 @@ public function __construct(array $typesConfig, ?DsnParser $dsnParser = null)
      */
     public function createConnection(array $params, ?Configuration $config = null, ?EventManager $eventManager = null, array $mappingTypes = [])
     {
+        if (interface_exists(DBALException::class) && $eventManager !== null) {
+            throw new InvalidArgumentException('Passing an EventManager instance is not supported with DBAL 4');
+        }
+
         if (! $this->initialized) {
             $this->initializeTypes();
         }
@@ -105,11 +113,11 @@ public function createConnection(array $params, ?Configuration $config = null, ?
                 $params['wrapperClass'] = null;
             }
 
-            $connection = DriverManager::getConnection($params, $config, $eventManager);
+            $connection = DriverManager::getConnection(...array_merge([$params, $config], $eventManager ? [$eventManager] : []));
             $params     = $this->addDatabaseSuffix(array_merge($connection->getParams(), $overriddenOptions));
             $driver     = $connection->getDriver();
-            if (class_exists(Connection\StaticServerVersionProvider::class)) {
-                $platform = $driver->getDatabasePlatform(new Connection\StaticServerVersionProvider($params['serverVersion'] ?? ''));
+            if (class_exists(StaticServerVersionProvider::class)) {
+                $platform = $driver->getDatabasePlatform(new StaticServerVersionProvider($params['serverVersion'] ?? ''));
             } else {
                 $platform = $driver->getDatabasePlatform();
             }
@@ -144,7 +152,7 @@ public function createConnection(array $params, ?Configuration $config = null, ?
 
             $connection = new $wrapperClass($params, $driver, $config, $eventManager);
         } else {
-            $connection = DriverManager::getConnection($params, $config, $eventManager);
+            $connection = DriverManager::getConnection(...array_merge([$params, $config], $eventManager ? [$eventManager] : []));
         }
 
         if (! empty($mappingTypes)) {
@@ -172,8 +180,7 @@ private function getDatabasePlatform(Connection $connection): AbstractPlatform
         try {
             return $connection->getDatabasePlatform();
         } catch (DriverException $driverException) {
-            //TODO: what more specific exception class should we throw with DBAL 4?
-            $class = class_exists(DBALException::class) ? DBALException::class : RuntimeException::class;
+            $class = class_exists(DBALException::class) ? DBALException::class : ConnectionException::class;
 
             throw new $class(
                 'An exception occurred while establishing a connection to figure out your platform version.' . PHP_EOL .
@@ -247,7 +254,11 @@ private function parseDatabaseUrl(array $params): array
             return $params;
         }
 
-        $parsedParams = $this->dsnParser->parse($params['url']);
+        try {
+            $parsedParams = $this->dsnParser->parse($params['url']);
+        } catch (MalformedDsnException $e) {
+            throw new MalformedDsnException('Malformed parameter "url".', 0, $e);
+        }
 
         if (isset($parsedParams['driver'])) {
             // The requested driver from the URL scheme takes precedence
@@ -257,7 +268,7 @@ private function parseDatabaseUrl(array $params): array
 
         $params = array_merge($params, $parsedParams);
 
-        // If a schemaless connection URL is given, we require a default driver or default custom driver
+        // If a schemeless connection URL is given, we require a default driver or default custom driver
         // as connection parameter.
         if (! isset($params['driverClass']) && ! isset($params['driver'])) {
             if (class_exists(DriverRequired::class)) {

diff --git a/DependencyInjection/DoctrineExtension.php b/DependencyInjection/DoctrineExtension.php
@@ -17,6 +17,7 @@
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Connections\PrimaryReadReplicaConnection;
 use Doctrine\DBAL\Driver\Middleware as MiddlewareInterface;
+use Doctrine\DBAL\Exception as DBALException;
 use Doctrine\DBAL\Schema\LegacySchemaManagerFactory;
 use Doctrine\ORM\Configuration as OrmConfiguration;
 use Doctrine\ORM\EntityManager;
@@ -272,7 +273,8 @@ protected function loadDbalConnection($name, array $connection, ContainerBuilder
             ->setArguments([
                 $options,
                 new Reference(sprintf('doctrine.dbal.%s_connection.configuration', $name)),
-                new Reference(sprintf('doctrine.dbal.%s_connection.event_manager', $name)),
+                // event manager is only supported on DBAL < 4
+                class_exists(DBALException::class) ? new Reference(sprintf('doctrine.dbal.%s_connection.event_manager', $name)) : null,
                 $connection['mapping_types'],
             ]);