Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Driver::connect() method signature is redundant #4069

Closed
morozov opened this issue Jun 12, 2020 · 1 comment · Fixed by #4081
Closed

The Driver::connect() method signature is redundant #4069

morozov opened this issue Jun 12, 2020 · 1 comment · Fixed by #4081

Comments

@morozov
Copy link
Member

morozov commented Jun 12, 2020

Q A
BC Break no
Version 2.10.2

User credentials and driver options are passed to the driver both via individual parameters and as part of $params:

dbal/src/Connection.php

Lines 352 to 356 in 48625f1

$driverOptions = $this->params['driverOptions'] ?? [];
$user = $this->params['user'] ?? null;
$password = $this->params['password'] ?? null;
$this->_conn = $this->_driver->connect($this->params, $user, $password, $driverOptions);

On the one hand, it's unclear to the implementors of Driver::connect(), whether the parameters should be taken from $params or from the individual parameters. On the other hand, the code components that call connect() need to mimic the behavior of the Connection class:

  1. $user = $connectionParams['user'] ?? null;
    $password = $connectionParams['password'] ?? null;
    return $this->_driver->connect($connectionParams, $user, $password, $driverOptions);
  2. $user = $params['user'] ?? null;
    $password = $params['password'] ?? null;
    $connection = $this->driver->connect($params, $user, $password);
More examples:
  1. $user = $params['user'] ?? null;
    $password = $params['password'] ?? null;
    $connection = $this->connection->getDriver()->connect($params, $user, $password);
  2. $user = $params['user'] ?? null;
    $password = $params['password'] ?? null;
    $connection = $this->connection->getDriver()->connect($params, $user, $password);
  3. return new MysqliConnection(
    $params,
    $params['user'] ?? '',
    $params['password'] ?? '',
    $driverOptions
    );
  4. return $this->createDriver()->connect(
    $params,
    $params['user'] ?? '',
    $params['password'] ?? '',
    $driverOptions
    );
  5. return $this->connection->getDriver()->connect(
    $params,
    $params['user'] ?? '',
    $params['password'] ?? '',
    $driverOptions
    );

By being this verbose, this API doesn't solve any design problem. On the contrary, once a driver receives these parameters individually, it may have to pack them back or regroup otherwise before processing:

  1. $params['user'] = $username;
    $params['password'] = $password;
    $params['dbname'] = DataSourceName::fromConnectionParameters($params)->toString();
  2. if ($username !== null) {
    $driverOptions['UID'] = $username;
    }
    if ($password !== null) {
    $driverOptions['PWD'] = $password;
    }
  3. ';UID=' . $username .
    ';PWD=' . $password .
    ';' . implode(
    ';',
    array_map(static function ($key, $value) {
    return $key . '=' . $value;
    }, array_keys($driverOptions), $driverOptions)

Since Driver::connect() plays the role of an adapter between the DBAL Connection::connect() and the Driver Connection::__construct(), it shouldn't expect Connection::connect() to do any adaptation.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant