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

Jump to bottom

separate connection string parsing from connection #494

Merged
merged 10 commits into from Jun 12, 2017
Merged

separate connection string parsing from connection #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e3d6a56
separate connection string parsing from connection
koenpunt Feb 9, 2015
b7c617e
use parse_str for query string parsing
koenpunt Feb 9, 2015
7d611e1
use factual decode param checking
koenpunt Feb 9, 2015
a90e8cd
use sscanf instead of regex
koenpunt Feb 9, 2015
5a6f015
assert database name for unix socket
koenpunt Feb 9, 2015
35bf5f1
check if property exists before assignment
koenpunt Feb 10, 2015
3781525
build ConnectionInfo objects on instantiation
koenpunt Feb 10, 2015
7c8eaa8
add array example
koenpunt Feb 10, 2015
4507f7f
rename protocol to adapter
koenpunt Feb 10, 2015
d447040
add docblocks
koenpunt Jun 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions ActiveRecord.php
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@
require __DIR__.'/lib/Table.php';
require __DIR__.'/lib/ConnectionManager.php';
require __DIR__.'/lib/Connection.php';
require __DIR__.'/lib/ConnectionInfo.php';
require __DIR__.'/lib/Serialization.php';
require __DIR__.'/lib/Expressions.php';
require __DIR__.'/lib/SQLBuilder.php';
Expand Down
29 changes: 25 additions & 4 deletions lib/Config.php
View file
Open in desktop
Expand Up @@ -118,13 +118,25 @@ public static function initialize(Closure $initializer)
}

/**
* Sets the list of database connection strings.
* Sets the list of database connections. Can be an array of connection strings or an array of arrays.
*
* <code>
* $config->set_connections(array(
* 'development' => 'mysql://username:password@127.0.0.1/database_name'));
* </code>
*
* <code>
* $config->set_connections(array(
* 'development' => array(
* 'adapter' => 'mysql',
* 'host' => '127.0.0.1',
* 'database' => 'database_name',
* 'username' => 'username',
* 'password' => 'password'
* )
* ));
* </code>
*
* @param array $connections Array of connections
* @param string $default_connection Optionally specify the default_connection
* @return void
Expand All @@ -138,7 +150,16 @@ public function set_connections($connections, $default_connection=null)
if ($default_connection)
$this->set_default_connection($default_connection);

$this->connections = $connections;
$this->connections = array_map(function($connection){
if(is_string($connection))
{
return ConnectionInfo::from_connection_url($connection);
}
else
{
return new ConnectionInfo($connection);
}
}, $connections);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oo, nice. Good call on building them here too.

}

/**
Expand All @@ -157,7 +178,7 @@ public function get_connections()
* @param string $name Name of connection to retrieve
* @return string connection info for specified connection name
*/
public function get_connection($name)
public function get_connection_info($name)
{
if (array_key_exists($name, $this->connections))
return $this->connections[$name];
Expand All @@ -170,7 +191,7 @@ public function get_connection($name)
*
* @return string
*/
public function get_default_connection_string()
public function get_default_connection_info()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a public API change. While I agree the new name makes more sense, this will be a small BC break. Should definitely be noted somewhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know, but 1.1 isn't BC anyways, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, true, I forgot its not. Not exactly following semver then, are we? :P

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not exactly no, maybe we're better of tagging the next release 2.0.0.. But I think that's @jpfuentes2 call.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea, it'd be more semver friendly, and more of a warning to developers looking to upgrade. But yea, better off a call for @jpfuentes2

{
return array_key_exists($this->default_connection,$this->connections) ?
$this->connections[$this->default_connection] : null;
Expand Down
138 changes: 30 additions & 108 deletions lib/Connection.php
View file
Open in desktop
Expand Up @@ -54,10 +54,10 @@ abstract class Connection
*/
private $logger;
/**
* The name of the protocol that is used.
* The name of the adapter that is used.
* @var string
*/
public $protocol;
public $adapter;
/**
* Database's date format
* @var string
Expand Down Expand Up @@ -91,41 +91,49 @@ abstract class Connection
/**
* Retrieve a database connection.
*
* @param string $connection_string_or_connection_name A database connection string (ex. mysql://user:pass@host[:port]/dbname)
* Everything after the protocol:// part is specific to the connection adapter.
* @param string $connection_info_or_name A database connection string (ex. mysql://user:pass@host[:port]/dbname)
* Everything after the adapter:// part is specific to the connection adapter.
* OR
* A connection name that is set in ActiveRecord\Config
* If null it will use the default connection specified by ActiveRecord\Config->set_default_connection
* @return Connection
* @see parse_connection_url
* @see ConnectionInfo::from_connection_url
*/
public static function instance($connection_string_or_connection_name=null)
public static function instance($connection_info=null)
{
$config = Config::instance();

if (strpos($connection_string_or_connection_name, '://') === false)
if (!($connection_info instanceof ConnectionInfo))
{
$connection_string = $connection_string_or_connection_name ?
$config->get_connection($connection_string_or_connection_name) :
$config->get_default_connection_string();
// Connection instantiation using a connection array
if (is_array($connection_info))
{
$connection_info = new ConnectionInfo($connection_info);
}
// Connection instantiation using a connection url
else if (is_string($connection_info) && strpos($connection_info, '://') !== false)
{
$connection_info = ConnectionInfo::from_connection_url($connection_info);
}
// Connection instantiation using a connection name
else
{
$connection_info = $connection_info ?
$config->get_connection_info($connection_info) :
$config->get_default_connection_info();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great job on this. Makes a lot more sense to do it this way.

}
else
$connection_string = $connection_string_or_connection_name;

if (!$connection_string)
throw new DatabaseException("Empty connection string");

$info = static::parse_connection_url($connection_string);
$fqclass = static::load_adapter_class($info->protocol);
$fqclass = static::load_adapter_class($connection_info->adapter);

try {
$connection = new $fqclass($info);
$connection->protocol = $info->protocol;
$connection = new $fqclass($connection_info);
$connection->adapter = $connection_info->adapter;
$connection->logging = $config->get_logging();
$connection->logger = $connection->logging ? $config->get_logger() : null;

if (isset($info->charset))
$connection->set_encoding($info->charset);
if (isset($connection_info->charset))
$connection->set_encoding($connection_info->charset);
} catch (PDOException $e) {
throw new DatabaseException($e);
}
Expand All @@ -151,92 +159,6 @@ private static function load_adapter_class($adapter)
return $fqclass;
}

/**
* Use this for any adapters that can take connection info in the form below
* to set the adapters connection info.
*
* <code>
* protocol://username:password@host[:port]/dbname
* protocol://urlencoded%20username:urlencoded%20password@host[:port]/dbname?decode=true
* protocol://username:password@unix(/some/file/path)/dbname
* </code>
*
* Sqlite has a special syntax, as it does not need a database name or user authentication:
*
* <code>
* sqlite://file.db
* sqlite://../relative/path/to/file.db
* sqlite://unix(/absolute/path/to/file.db)
* sqlite://windows(c%2A/absolute/path/to/file.db)
* </code>
*
* @param string $connection_url A connection URL
* @return object the parsed URL as an object.
*/
public static function parse_connection_url($connection_url)
{
$url = @parse_url($connection_url);

if (!isset($url['host']))
throw new DatabaseException('Database host must be specified in the connection string. If you want to specify an absolute filename, use e.g. sqlite://unix(/path/to/file)');

$info = new \stdClass();
$info->protocol = $url['scheme'];
$info->host = $url['host'];
$info->db = isset($url['path']) ? substr($url['path'], 1) : null;
$info->user = isset($url['user']) ? $url['user'] : null;
$info->pass = isset($url['pass']) ? $url['pass'] : null;

$allow_blank_db = ($info->protocol == 'sqlite');

if ($info->host == 'unix(')
{
$socket_database = $info->host . '/' . $info->db;

if ($allow_blank_db)
$unix_regex = '/^unix\((.+)\)\/?().*$/';
else
$unix_regex = '/^unix\((.+)\)\/(.+)$/';

if (preg_match_all($unix_regex, $socket_database, $matches) > 0)
{
$info->host = $matches[1][0];
$info->db = $matches[2][0];
}
} elseif (substr($info->host, 0, 8) == 'windows(')
{
$info->host = urldecode(substr($info->host, 8) . '/' . substr($info->db, 0, -1));
$info->db = null;
}

if ($allow_blank_db && $info->db)
$info->host .= '/' . $info->db;

if (isset($url['port']))
$info->port = $url['port'];

if (strpos($connection_url, 'decode=true') !== false)
{
if ($info->user)
$info->user = urldecode($info->user);

if ($info->pass)
$info->pass = urldecode($info->pass);
}

if (isset($url['query']))
{
foreach (explode('/&/', $url['query']) as $pair) {
list($name, $value) = explode('=', $pair);

if ($name == 'charset')
$info->charset = $value;
}
}

return $info;
}

/**
* Class Connection is a singleton. Access it via instance().
*
Expand All @@ -257,7 +179,7 @@ protected function __construct($info)
else
$host = "unix_socket=$info->host";

$this->connection = new PDO("$info->protocol:$host;dbname=$info->db", $info->user, $info->pass, static::$PDO_OPTIONS);
$this->connection = new PDO("$info->adapter:$host;dbname=$info->database", $info->username, $info->password, static::$PDO_OPTIONS);
} catch (PDOException $e) {
throw new DatabaseException($e);
}
Expand Down