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.

Sign up for GitHub

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

Support for database URLs #729

Merged
merged 16 commits into from
Dec 4, 2014
Merged

Support for database URLs #729

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e8056f5
first stab at supporting database 'url' for connection
dzuelke Nov 16, 2014
005d30e
fix CS
dzuelke Nov 16, 2014
567235f
fix wrong variable use in parse_url
dzuelke Nov 16, 2014
9565d84
fix return values
dzuelke Nov 16, 2014
71c1007
fix undefined $url
dzuelke Nov 26, 2014
4c25c6b
some tests for database URLs
dzuelke Nov 26, 2014
fbc9615
fallthrough scheme tests, and turns out underscores are a no go :(
dzuelke Nov 26, 2014
bddcb16
support dashes in schemes for db drivers, translated to underscores
dzuelke Nov 26, 2014
df3c59e
remove underscore from method name as per feedback
dzuelke Nov 27, 2014
4c076bc
remove underscore from member name as per feedback
dzuelke Nov 27, 2014
83cbc0b
erp, typo, wat
dzuelke Nov 27, 2014
2d75949
early assign as per feedback
dzuelke Nov 27, 2014
f04b893
fix backslash prefix
dzuelke Nov 27, 2014
f8a1c79
remove redundant check as per feedback
dzuelke Nov 27, 2014
59e17b5
docs :)
dzuelke Dec 4, 2014
df8eba3
Merge branch 'master' into database_url
dzuelke Dec 4, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions lib/Doctrine/DBAL/DriverManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,21 @@ final class DriverManager
'sqlsrv' => 'Doctrine\DBAL\Driver\SQLSrv\Driver',
);

/**
* List of URL schemes from a database URL and their mappings to driver.
*/
private static $_driverSchemeAliases = array(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no _ needed here

'db2' => 'ibm_db2',
'mssql' => 'pdo_sqlsrv',
'mysql' => 'pdo_mysql',
'mysql2' => 'pdo_mysql', // Amazon RDS, for some weird reason
'postgres' => 'pdo_pgsql',
'postgresql' => 'pdo_pgsql',
'pgsql' => 'pdo_pgsql',
'sqlite' => 'pdo_sqlite',
'sqlite3' => 'pdo_sqlite',
);

/**
* Private constructor. This class cannot be instantiated.
*/
Expand Down Expand Up @@ -126,6 +141,8 @@ public static function getConnection(
$eventManager = new EventManager();
}

$params = self::_parseDatabaseUrl($params);

// check for existing pdo object
if (isset($params['pdo']) && ! $params['pdo'] instanceof \PDO) {
throw DBALException::invalidPdoInstance();
Expand Down Expand Up @@ -194,4 +211,68 @@ private static function _checkParams(array $params)
throw DBALException::invalidDriverClass($params['driverClass']);
}
}

/**
* Extracts parts from a database URL, if present, and returns an
* updated list of parameters.
*
* @param array $params The list of parameters.
*
* @param array A modified list of parameters with info from a database
* URL extracted into indidivual parameter parts.
*
*/
private static function _parseDatabaseUrl(array $params)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't need to be prefixed with _ anymore (was an old convention)

{
if (!isset($params['url'])) {
return $params;
}

// (pdo_)?sqlite3?:///... => (pdo_)?sqlite3?://localhost/... or else the URL will be invalid
$url = preg_replace('#^((?:pdo_)?sqlite3?):///#', '$1://localhost/', $params['url']);

$url = parse_url($url);

if ($url === false) {
throw new DBALException('Malformed parameter "url".');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exception type is a bit too generic here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its fine imho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the exception used everywhere in DBAL though...

}

if (isset($url['scheme'])) {
$url['scheme'] = str_replace('-', '_', $url['scheme']); // URL schemes must not contain underscores, but dashes are ok
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If _ is invalid, then why replace it instead of bailing out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the idea is to have mysql://user:port@host/dbname, but if you want to specifically use the drizzle_pdo_mysql, then drizzle_pdo_mysql://user:port@host/dbname won't work because underscores are illegal in URL scheme names.

So you can do drizzle-pdo-mysql://user:port@host/dbname instead and I translate that to underscores.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I understand your question now. Well, we'd have to preg_match() around to find the underscores (parse_url() returns the entire URL as the path component when using underscores there). If we don't do that, we end up with not enough info in the URL, and the process will fail later (see the tests, very last two test cases). So all good.

if (isset(self::$_driverSchemeAliases[$url['scheme']])) {
$params['driver'] = self::$_driverSchemeAliases[$url['scheme']]; // use alias
} else {
$params['driver'] = $url['scheme']; // let's see what checkParams() says about it later
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assign early to avoid an else

}
}

if (isset($url['host'])) {
$params['host'] = $url['host'];
}
if (isset($url['port'])) {
$params['port'] = $url['port'];
}
if (isset($url['user'])) {
$params['user'] = $url['user'];
}
if (isset($url['pass'])) {
$params['password'] = $url['pass'];
}

if (isset($url['path'])) {
if (!isset($url['scheme']) || (strpos($url['scheme'], 'sqlite') !== false && $url['path'] == ':memory:')) {
$params['dbname'] = $url['path']; // if the URL was just "sqlite::memory:", which parses to scheme and path only
} else {
$params['dbname'] = substr($url['path'], 1); // strip the leading slash from the URL
}
}

if (isset($url['query'])) {
$query = array();
parse_str($url['query'], $query); // simply ingest query as extra params, e.g. charset or sslmode
$params = array_merge($params, $query); // parse_str wipes existing array elements
}

return $params;
}
}
91 changes: 91 additions & 0 deletions tests/Doctrine/Tests/DBAL/DriverManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,95 @@ public function testValidDriverClass()
$conn = \Doctrine\DBAL\DriverManager::getConnection($options);
$this->assertInstanceOf('Doctrine\DBAL\Driver\PDOMySql\Driver', $conn->getDriver());
}

/**
* @dataProvider databaseUrls
*/
public function testDatabaseUrl($url, $expected)
{
$options = is_array($url) ? $url : array(
'url' => $url,
);

if ($expected === false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data-provider doesn't seem to have a false case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does, second to last case :)

$this->setExpectedException('\Doctrine\DBAL\DBALException');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't prefix with \ in strings

}

$conn = \Doctrine\DBAL\DriverManager::getConnection($options);

if ($expected === false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this check: the exception will bubble up anyway.

return;
}

$params = $conn->getParams();
foreach ($expected as $key => $value) {
if ($key == 'driver') {
$this->assertInstanceOf($value, $conn->getDriver());
} else {
$this->assertEquals($value, $params[$key]);
}
}
}

public function databaseUrls()
{
return array(
'simple URL' => array(
'mysql://foo:bar@localhost/baz',
array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
),
'simple URL with port' => array(
'mysql://foo:bar@localhost:11211/baz',
array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'port' => 11211, 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
),
'sqlite relative URL with host' => array(
'sqlite://localhost/foo/dbname.sqlite',
array('dbname' => 'foo/dbname.sqlite', 'driver' => 'Doctrine\DBAL\Driver\PDOSqlite\Driver'),
),
'sqlite absolute URL with host' => array(
'sqlite://localhost//tmp/dbname.sqlite',
array('dbname' => '/tmp/dbname.sqlite', 'driver' => 'Doctrine\DBAL\Driver\PDOSqlite\Driver'),
),
'sqlite relative URL without host' => array(
'sqlite:///foo/dbname.sqlite',
array('dbname' => 'foo/dbname.sqlite', 'driver' => 'Doctrine\DBAL\Driver\PDOSqlite\Driver'),
),
'sqlite absolute URL without host' => array(
'sqlite:////tmp/dbname.sqlite',
array('dbname' => '/tmp/dbname.sqlite', 'driver' => 'Doctrine\DBAL\Driver\PDOSqlite\Driver'),
),
'sqlite memory' => array(
'sqlite:///:memory:',
array('dbname' => ':memory:', 'driver' => 'Doctrine\DBAL\Driver\PDOSqlite\Driver'),
),
'sqlite memory with host' => array(
'sqlite://localhost/:memory:',
array('dbname' => ':memory:', 'driver' => 'Doctrine\DBAL\Driver\PDOSqlite\Driver'),
),
'params parsed from URL override individual params' => array(
array('url' => 'mysql://foo:bar@localhost/baz', 'password' => 'lulz'),
array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
),
'params not parsed from URL but individual params are preserved' => array(
array('url' => 'mysql://foo:bar@localhost/baz', 'port' => 1234),
array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'port' => 1234, 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
),
'query params from URL are used as extra params' => array(
'url' => 'mysql://foo:bar@localhost/dbname?charset=UTF-8',
array('charset' => 'UTF-8'),
),
'simple URL with fallthrough scheme not defined in map' => array(
'sqlsrv://foo:bar@localhost/baz',
array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\SQLSrv\Driver'),
),
'simple URL with fallthrough scheme containing underscores fails' => array(
'drizzle_pdo_mysql://foo:bar@localhost/baz',
false,
),
'simple URL with fallthrough scheme containing dashes works' => array(
'drizzle-pdo-mysql://foo:bar@localhost/baz',
array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\DrizzlePDOMySql\Driver'),
),
);
}
}