Merge pull request #4081 from morozov/issues/4069

Simplify Driver::connect() signature
doctrine · Jun 17, 2020 · d89cf16 · d89cf16
2 parents a05f449 + 116ae82
commit d89cf16
Show file tree

Hide file tree

Showing 30 changed files with 410 additions and 265 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,9 @@
 # Upgrade to 3.0
 
+## BC BREAK changes the `Driver::connect()` signature
+
+The method no longer accepts the `$username`, `$password` and `$driverOptions` arguments. The corresponding values are expected to be passed as the "user", "password" and "driver_options" keys of the `$params` argument respectively.
+
 ## Removed `MasterSlaveConnection`
 
 This class was deprecated in favor of `PrimaryReadReplicaConnection`

diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -83,11 +83,12 @@
     <!-- https://github.com/squizlabs/PHP_CodeSniffer/issues/2837 -->
     <rule ref="Squiz.NamingConventions.ValidVariableName.NotCamelCaps">
         <!--
-            This file uses the return value db2_server_info(), which does not follow conventions
-            phpcs wrongly complains about it, and that has been reported here:
+            These files use the underlying driver APIs that don't comply with the coding standard
+            phpcs wrongly complains about them, and that has been reported here:
             https://github.com/squizlabs/PHP_CodeSniffer/issues/2950
         -->
         <exclude-pattern>src/Driver/IBMDB2/DB2Connection.php</exclude-pattern>
+        <exclude-pattern>src/Driver/Mysqli/MysqliConnection.php</exclude-pattern>
         <!-- See https://github.com/squizlabs/PHP_CodeSniffer/issues/2837 -->
         <exclude-pattern>src/SQLParserUtils.php</exclude-pattern>
         <exclude-pattern>src/Tools/Dumper.php</exclude-pattern>

diff --git a/src/Connection.php b/src/Connection.php
@@ -352,12 +352,8 @@ public function connect()
             return false;
         }
 
-        $driverOptions = $this->params['driverOptions'] ?? [];
-        $user          = $this->params['user'] ?? null;
-        $password      = $this->params['password'] ?? null;
-
         try {
-            $this->_conn = $this->_driver->connect($this->params, $user, $password, $driverOptions);
+            $this->_conn = $this->_driver->connect($this->params);
         } catch (DriverException $e) {
             throw DBALException::driverException($this->_driver, $e);
         }

diff --git a/src/Connections/PrimaryReadReplicaConnection.php b/src/Connections/PrimaryReadReplicaConnection.php
@@ -225,15 +225,10 @@ protected function connectTo($connectionName)
     {
         $params = $this->getParams();
 
-        $driverOptions = $params['driverOptions'] ?? [];
-
         $connectionParams = $this->chooseConnectionConfiguration($connectionName, $params);
 
-        $user     = $connectionParams['user'] ?? null;
-        $password = $connectionParams['password'] ?? null;
-
         try {
-            return $this->_driver->connect($connectionParams, $user, $password, $driverOptions);
+            return $this->_driver->connect($connectionParams);
         } catch (DriverException $e) {
             throw DBALException::driverException($this->_driver, $e);
         }

diff --git a/src/Driver.php b/src/Driver.php
@@ -2,6 +2,7 @@
 
 namespace Doctrine\DBAL;
 
+use Doctrine\DBAL\Driver\Connection as DriverConnection;
 use Doctrine\DBAL\Driver\DriverException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
@@ -15,18 +16,13 @@ interface Driver
     /**
      * Attempts to create a connection with the database.
      *
-     * The usage of NULL to indicate empty username or password is deprecated. Use an empty string instead.
+     * @param mixed[] $params All connection parameters.
      *
-     * @param mixed[]     $params        All connection parameters passed by the user.
-     * @param string|null $username      The username to use when connecting.
-     * @param string|null $password      The password to use when connecting.
-     * @param mixed[]     $driverOptions The driver options to use when connecting.
-     *
-     * @return \Doctrine\DBAL\Driver\Connection The database connection.
+     * @return DriverConnection The database connection.
      *
      * @throws DriverException
      */
-    public function connect(array $params, $username = null, $password = null, array $driverOptions = []);
+    public function connect(array $params);
 
     /**
      * Gets the DatabasePlatform instance that provides all the metadata about

diff --git a/src/Driver/IBMDB2/DB2Connection.php b/src/Driver/IBMDB2/DB2Connection.php
@@ -33,21 +33,21 @@ class DB2Connection implements ServerInfoAwareConnection
     private $conn = null;
 
     /**
-     * @param mixed[] $params
-     * @param string  $username
-     * @param string  $password
-     * @param mixed[] $driverOptions
+     * @param array<string,mixed> $driverOptions
      *
      * @throws DB2Exception
      */
-    public function __construct(array $params, $username, $password, $driverOptions = [])
-    {
-        $isPersistent = (isset($params['persistent']) && $params['persistent'] === true);
-
-        if ($isPersistent) {
-            $conn = db2_pconnect($params['dbname'], $username, $password, $driverOptions);
+    public function __construct(
+        string $database,
+        bool $persistent,
+        string $username,
+        string $password,
+        array $driverOptions = []
+    ) {
+        if ($persistent) {
+            $conn = db2_pconnect($database, $username, $password, $driverOptions);
         } else {
-            $conn = db2_connect($params['dbname'], $username, $password, $driverOptions);
+            $conn = db2_connect($database, $username, $password, $driverOptions);
         }
 
         if ($conn === false) {

diff --git a/src/Driver/IBMDB2/DB2Driver.php b/src/Driver/IBMDB2/DB2Driver.php
@@ -12,17 +12,14 @@ class DB2Driver extends AbstractDB2Driver
     /**
      * {@inheritdoc}
      */
-    public function connect(array $params, $username = null, $password = null, array $driverOptions = [])
+    public function connect(array $params)
     {
-        $params['user']     = $username;
-        $params['password'] = $password;
-        $params['dbname']   = DataSourceName::fromConnectionParameters($params)->toString();
-
         return new DB2Connection(
-            $params,
-            (string) $username,
-            (string) $password,
-            $driverOptions
+            DataSourceName::fromConnectionParameters($params)->toString(),
+            isset($params['persistent']) && $params['persistent'] === true,
+            $params['user'] ?? '',
+            $params['password'] ?? '',
+            $params['driver_options'] ?? []
         );
     }
 

diff --git a/src/Driver/Mysqli/Driver.php b/src/Driver/Mysqli/Driver.php
@@ -3,15 +3,115 @@
 namespace Doctrine\DBAL\Driver\Mysqli;
 
 use Doctrine\DBAL\Driver\AbstractMySQLDriver;
+use Doctrine\DBAL\Driver\Mysqli\Initializer\Charset;
+use Doctrine\DBAL\Driver\Mysqli\Initializer\Options;
+use Doctrine\DBAL\Driver\Mysqli\Initializer\Secure;
+
+use function count;
 
 class Driver extends AbstractMySQLDriver
 {
     /**
      * {@inheritdoc}
      */
-    public function connect(array $params, $username = null, $password = null, array $driverOptions = [])
+    public function connect(array $params)
+    {
+        if (! empty($params['persistent'])) {
+            if (! isset($params['host'])) {
+                throw HostRequired::forPersistentConnection();
+            }
+
+            $host = 'p:' . $params['host'];
+        } else {
+            $host = $params['host'] ?? null;
+        }
+
+        $flags = null;
+
+        $preInitializers = $postInitializers = [];
+
+        if (isset($params['driver_options'])) {
+            $driverOptions = $params['driver_options'];
+
+            if (isset($driverOptions[MysqliConnection::OPTION_FLAGS])) {
+                $flags = $driverOptions[MysqliConnection::OPTION_FLAGS];
+                unset($driverOptions[MysqliConnection::OPTION_FLAGS]);
+            }
+
+            $preInitializers = $this->withOptions($preInitializers, $driverOptions);
+        }
+
+        $preInitializers  = $this->withSecure($preInitializers, $params);
+        $postInitializers = $this->withCharset($postInitializers, $params);
+
+        return new MysqliConnection(
+            $host,
+            $params['user'] ?? null,
+            $params['password'] ?? null,
+            $params['dbname'] ?? null,
+            $params['port'] ?? null,
+            $params['unix_socket'] ?? null,
+            $flags,
+            $preInitializers,
+            $postInitializers
+        );
+    }
+
+    /**
+     * @param list<Initializer> $initializers
+     * @param array<int,mixed>  $options
+     *
+     * @return list<Initializer>
+     */
+    private function withOptions(array $initializers, array $options): array
     {
-        return new MysqliConnection($params, (string) $username, (string) $password, $driverOptions);
+        if (count($options) !== 0) {
+            $initializers[] = new Options($options);
+        }
+
+        return $initializers;
+    }
+
+    /**
+     * @param list<Initializer>   $initializers
+     * @param array<string,mixed> $params
+     *
+     * @return list<Initializer>
+     */
+    private function withSecure(array $initializers, array $params): array
+    {
+        if (
+            isset($params['ssl_key']) ||
+            isset($params['ssl_cert']) ||
+            isset($params['ssl_ca']) ||
+            isset($params['ssl_capath']) ||
+            isset($params['ssl_cipher'])
+        ) {
+            $initializers[] = new Secure(
+                $params['ssl_key']    ?? null,
+                $params['ssl_cert']   ?? null,
+                $params['ssl_ca']     ?? null,
+                $params['ssl_capath'] ?? null,
+                $params['ssl_cipher'] ?? null
+            );
+        }
+
+        return $initializers;
+    }
+
+    /**
+     * @param list<Initializer>   $initializers
+     * @param array<string,mixed> $params
+     *
+     * @return list<Initializer>
+     */
+    private function withCharset(array $initializers, array $params): array
+    {
+        if (isset($params['charset'])) {
+            $initializers[] = new Charset($params['charset']);
+        }
+
+        return $initializers;
     }
 
     /**

diff --git a/src/Driver/Mysqli/Exception/InvalidCharset.php b/src/Driver/Mysqli/Exception/InvalidCharset.php
@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli\Exception;
+
+use Doctrine\DBAL\Driver\Mysqli\MysqliException;
+use mysqli;
+
+use function sprintf;
+
+/**
+ * @internal
+ *
+ * @psalm-immutable
+ */
+final class InvalidCharset extends MysqliException
+{
+    public static function fromCharset(mysqli $connection, string $charset): self
+    {
+        return new self(
+            sprintf('Failed to set charset "%s": %s', $charset, $connection->error),
+            $connection->sqlstate,
+            $connection->errno
+        );
+    }
+}
diff --git a/src/Driver/Mysqli/Exception/InvalidOption.php b/src/Driver/Mysqli/Exception/InvalidOption.php
@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli\Exception;
+
+use Doctrine\DBAL\Driver\Mysqli\MysqliException;
+
+use function sprintf;
+
+/**
+ * @internal
+ *
+ * @psalm-immutable
+ */
+final class InvalidOption extends MysqliException
+{
+    /**
+     * @param mixed $value
+     */
+    public static function fromOption(int $option, $value): self
+    {
+        return new self(
+            sprintf('Failed to set option %d with value "%s"', $option, $value)
+        );
+    }
+}
diff --git a/src/Driver/Mysqli/Initializer.php b/src/Driver/Mysqli/Initializer.php
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli;
+
+use mysqli;
+
+interface Initializer
+{
+    /**
+     * @throws MysqliException
+     */
+    public function initialize(mysqli $connection): void;
+}
diff --git a/src/Driver/Mysqli/Initializer/Charset.php b/src/Driver/Mysqli/Initializer/Charset.php
@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli\Initializer;
+
+use Doctrine\DBAL\Driver\Mysqli\Exception\InvalidCharset;
+use Doctrine\DBAL\Driver\Mysqli\Initializer;
+use mysqli;
+
+final class Charset implements Initializer
+{
+    /** @var string */
+    private $charset;
+
+    public function __construct(string $charset)
+    {
+        $this->charset = $charset;
+    }
+
+    public function initialize(mysqli $connection): void
+    {
+        if ($connection->set_charset($this->charset)) {
+            return;
+        }
+
+        throw InvalidCharset::fromCharset($connection, $this->charset);
+    }
+}
diff --git a/src/Driver/Mysqli/Initializer/Options.php b/src/Driver/Mysqli/Initializer/Options.php
@@ -0,0 +1,34 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\Mysqli\Initializer;
+
+use Doctrine\DBAL\Driver\Mysqli\Exception\InvalidOption;
+use Doctrine\DBAL\Driver\Mysqli\Initializer;
+use mysqli;
+
+use function mysqli_options;
+
+final class Options implements Initializer
+{
+    /** @var array<int,mixed> */
+    private $options;
+
+    /**
+     * @param array<int,mixed> $options
+     */
+    public function __construct(array $options)
+    {
+        $this->options = $options;
+    }
+
+    public function initialize(mysqli $connection): void
+    {
+        foreach ($this->options as $option => $value) {
+            if (! mysqli_options($connection, $option, $value)) {
+                throw InvalidOption::fromOption($option, $value);
+            }
+        }
+    }
+}