From ad33efbc675f19450d4f3c5edc5a37ac7e1ac724 Mon Sep 17 00:00:00 2001 From: Iskandar Najmuddin Date: Mon, 5 May 2014 13:02:16 +0000 Subject: [PATCH] Improve Resque_Redis DSN parsing. - Allow for DSN URIs to work as expected. - Backward-compatible with simple 'host:port' format. - Does not parse DSNs provided in array format for Credis_Cluster. --- lib/Resque/Redis.php | 121 +++++++++++++++++++------- test/Resque/Tests/DsnTest.php | 157 ++++++++++++++++++++++++++++++++++ 2 files changed, 246 insertions(+), 32 deletions(-) mode change 100644 => 100755 lib/Resque/Redis.php create mode 100755 test/Resque/Tests/DsnTest.php diff --git a/lib/Resque/Redis.php b/lib/Resque/Redis.php old mode 100644 new mode 100755 index 6cef3d2..e44a962 --- a/lib/Resque/Redis.php +++ b/lib/Resque/Redis.php @@ -8,14 +8,29 @@ */ class Resque_Redis { - /** - * Redis namespace - * @var string - */ - private static $defaultNamespace = 'resque:'; + /** + * Redis namespace + * @var string + */ + private static $defaultNamespace = 'resque:'; - private $server; - private $database; + /** + * A default host to connect to + */ + const DEFAULT_HOST = 'localhost'; + + /** + * The default Redis port + */ + const DEFAULT_PORT = 6379; + + /** + * The default Redis Database number + */ + const DEFAULT_DATABASE = 0; + + private $server; + private $database; /** * @var array List of all commands in Redis that supply a key as their @@ -92,40 +107,36 @@ class Resque_Redis self::$defaultNamespace = $namespace; } - public function __construct($server, $database = null) + /** + * @param string|array $server A DSN or array + * @param int $database A database number to select + */ + public function __construct($server, $database = null) { $this->server = $server; $this->database = $database; if (is_array($this->server)) { $this->driver = new Credis_Cluster($server); - } - else { - $port = null; - $password = null; - $host = $server; - // If not a UNIX socket path or tcp:// formatted connections string - // assume host:port combination. - if (strpos($server, '/') === false) { - $parts = explode(':', $server); - if (isset($parts[1])) { - $port = $parts[1]; - } - $host = $parts[0]; - }else if (strpos($server, 'redis://') !== false){ - // Redis format is: - // redis://[user]:[password]@[host]:[port] - list($userpwd,$hostport) = explode('@', $server); - $userpwd = substr($userpwd, strpos($userpwd, 'redis://')+8); - list($host, $port) = explode(':', $hostport); - list($user, $password) = explode(':', $userpwd); - } - - $this->driver = new Credis_Client($host, $port); - if (isset($password)){ + } else { + + list($host, $port, $dsnDatabase, $user, $password, $options) = $this->parseDsn($server); + // $user is are unused here + + // Look for known Credis_Client options + $timeout = isset($options['timeout']) ? intval($options['timeout']) : null; + $persistent = isset($options['persistent']) ? $options['persistent'] : ''; + + $this->driver = new Credis_Client($host, $port, $timeout, $persistent); + if ($password){ $this->driver->auth($password); } + + // If the `$database` constructor argument is not set, use the value from the DSN. + if (is_null($database)) { + $database = $dsnDatabase; + } } if ($this->database !== null) { @@ -133,6 +144,52 @@ class Resque_Redis } } + /** + * Parse a DSN string + * @param string $dsn + * @return array [host, port, db, user, pass, options] + */ + public function parseDsn($dsn) + { + $validSchemes = array('redis', 'tcp'); + if ($dsn == '') { + // Use a sensible default for an empty DNS string + $dsn = 'redis://' . self::DEFAULT_HOST; + } + $parts = parse_url($dsn); + if (isset($parts['scheme']) && ! in_array($parts['scheme'], $validSchemes)) { + throw new \InvalidArgumentException("Invalid DSN. Supported schemes are " . implode(', ', $validSchemes)); + } + + // Allow simple 'hostname' format, which parse_url treats as a path, not host. + if ( ! isset($parts['host'])) { + $parts = array('host' => $parts['path']); + } + + $port = isset($parts['port']) ? intval($parts['port']) : self::DEFAULT_PORT; + + $database = self::DEFAULT_DATABASE; + if (isset($parts['path'])) { + // Strip non-digit chars from path + $database = intval(preg_replace('/[^0-9]/', '', $parts['path'])); + } + + $options = array(); + if (isset($parts['query'])) { + // Parse the query string into an array + parse_str($parts['query'], $options); + } + + return array( + $parts['host'], + $port, + $database, + isset($parts['user']) ? $parts['user'] : false, + isset($parts['pass']) ? $parts['pass'] : false, + $options, + ); + } + /** * Magic method to handle all function requests and prefix key based * operations with the {self::$defaultNamespace} key prefix. diff --git a/test/Resque/Tests/DsnTest.php b/test/Resque/Tests/DsnTest.php new file mode 100755 index 0000000..c9cd471 --- /dev/null +++ b/test/Resque/Tests/DsnTest.php @@ -0,0 +1,157 @@ + + * @license http://www.opensource.org/licenses/mit-license.php + */ +class Resque_Tests_DsnTest extends Resque_Tests_TestCase +{ + + /** + * These DNS strings are considered valid. + * + * @return array + */ + public function validDsnStringProvider() + { + return array( + // Input , Expected output + array('', array( + 'localhost', + Resque_Redis::DEFAULT_PORT, + Resque_Redis::DEFAULT_DATABASE, + false, false, + array(), + )), + array('localhost', array( + 'localhost', + Resque_Redis::DEFAULT_PORT, + Resque_Redis::DEFAULT_DATABASE, + false, false, + array(), + )), + array('localhost:1234', array( + 'localhost', + 1234, + Resque_Redis::DEFAULT_DATABASE, + false, false, + array(), + )), + array('localhost:1234/2', array( + 'localhost', + 1234, + 2, + false, false, + array(), + )), + array('redis://foobar', array( + 'foobar', + Resque_Redis::DEFAULT_PORT, + Resque_Redis::DEFAULT_DATABASE, + false, false, + array(), + )), + array('redis://foobar:1234', array( + 'foobar', + 1234, + Resque_Redis::DEFAULT_DATABASE, + false, false, + array(), + )), + array('redis://user@foobar:1234', array( + 'foobar', + 1234, + Resque_Redis::DEFAULT_DATABASE, + 'user', false, + array(), + )), + array('redis://user:pass@foobar:1234', array( + 'foobar', + 1234, + Resque_Redis::DEFAULT_DATABASE, + 'user', 'pass', + array(), + )), + array('redis://user:pass@foobar:1234?x=y&a=b', array( + 'foobar', + 1234, + Resque_Redis::DEFAULT_DATABASE, + 'user', 'pass', + array('x' => 'y', 'a' => 'b'), + )), + array('redis://:pass@foobar:1234?x=y&a=b', array( + 'foobar', + 1234, + Resque_Redis::DEFAULT_DATABASE, + false, 'pass', + array('x' => 'y', 'a' => 'b'), + )), + array('redis://user@foobar:1234?x=y&a=b', array( + 'foobar', + 1234, + Resque_Redis::DEFAULT_DATABASE, + 'user', false, + array('x' => 'y', 'a' => 'b'), + )), + array('redis://foobar:1234?x=y&a=b', array( + 'foobar', + 1234, + Resque_Redis::DEFAULT_DATABASE, + false, false, + array('x' => 'y', 'a' => 'b'), + )), + array('redis://user@foobar:1234/12?x=y&a=b', array( + 'foobar', + 1234, + 12, + 'user', false, + array('x' => 'y', 'a' => 'b'), + )), + array('tcp://user@foobar:1234/12?x=y&a=b', array( + 'foobar', + 1234, + 12, + 'user', false, + array('x' => 'y', 'a' => 'b'), + )), + ); + } + + /** + * These DSN values should throw exceptions + * @return array + */ + public function bogusDsnStringProvider() + { + return array( + 'http://foo.bar/', + '://foo.bar/', + 'user:@foobar:1234?x=y&a=b', + 'foobar:1234?x=y&a=b', + ); + } + + /** + * @dataProvider validDsnStringProvider + */ + public function testParsingValidDsnString($dsn, $expected) + { + $resqueRedis = new Resque_Redis('localhost'); + $result = $resqueRedis->parseDsn($dsn); + $this->assertEquals($expected, $result); + } + + /** + * @dataProvider bogusDsnStringProvider + * @expectedException InvalidArgumentException + */ + public function testParsingBogusDsnStringThrowsException($dsn) + { + $resqueRedis = new Resque_Redis('localhost'); + // The next line should throw an InvalidArgumentException + $result = $resqueRedis->parseDsn($dsn); + } + +}