Browse Source

Change how parameters are handled internally.

Daniele Alessandri 14 years ago
parent
commit
82759cd6ec

+ 1 - 4
lib/Predis/Client.php

@@ -67,13 +67,10 @@ class Client {
             );
         }
 
-        $options = $this->_options;
         $connection = self::newConnectionInternal($parameters);
-        $connection->setProtocolOption('iterable_multibulk', $options->iterable_multibulk);
-        $connection->setProtocolOption('throw_errors', $options->throw_errors);
         $this->pushInitCommands($connection);
 
-        $callback = $options->on_connection_initialized;
+        $callback = $this->_options->on_connection_initialized;
         if (isset($callback)) {
             $callback($this, $connection);
         }

+ 0 - 2
lib/Predis/ClientOptions.php

@@ -17,8 +17,6 @@ class ClientOptions {
         self::$_sharedOptions = array(
             'profile' => new Options\ClientProfile(),
             'key_distribution' => new Options\ClientKeyDistribution(),
-            'iterable_multibulk' => new Options\ClientIterableMultiBulk(),
-            'throw_errors' => new Options\ClientThrowOnError(),
             'on_connection_initialized' => new Options\CustomOption(array(
                 'validate' => function($value) {
                     if (isset($value) && is_callable($value)) {

+ 31 - 23
lib/Predis/ConnectionParameters.php

@@ -2,15 +2,19 @@
 
 namespace Predis;
 
+use Predis\Options\IOption;
+use Predis\Options\Option;
+use Predis\Options\CustomOption;
+
 class ConnectionParameters {
     private $_parameters;
+    private $_userDefined;
     private static $_sharedOptions;
 
-    public function __construct($parameters = null) {
-        $parameters = $parameters ?: array();
-        $this->_parameters = is_array($parameters)
-            ? $this->filter($parameters)
-            : $this->parseURI($parameters);
+    public function __construct($parameters = array()) {
+        $extractor = is_array($parameters) ? 'filter' : 'parseURI';
+        $this->_parameters = $this->$extractor($parameters);
+        $this->_userDefined = array_fill_keys(array_keys($this->_parameters), true);
     }
 
     private static function paramsExtractor($params, $kv) {
@@ -24,40 +28,49 @@ class ConnectionParameters {
             return self::$_sharedOptions;
         }
 
-        $optEmpty   = new Options\Option();
-        $optBoolean = new Options\CustomOption(array(
+        $optEmpty   = new Option();
+        $optBoolFalse = new CustomOption(array(
             'validate' => function($value) { return (bool) $value; },
             'default'  => function() { return false; },
         ));
+        $optBoolTrue = new CustomOption(array(
+            'validate' => function($value) { return (bool) $value; },
+            'default'  => function() { return true; },
+        ));
 
         self::$_sharedOptions = array(
-            'scheme' => new Options\CustomOption(array(
+            'scheme' => new CustomOption(array(
                 'default'  => function() { return 'tcp'; },
             )),
-            'host' => new Options\CustomOption(array(
+            'host' => new CustomOption(array(
                 'default'  => function() { return '127.0.0.1'; },
             )),
-            'port' => new Options\CustomOption(array(
+            'port' => new CustomOption(array(
                 'validate' => function($value) { return (int) $value; },
                 'default'  => function() { return 6379; },
             )),
             'path' => $optEmpty,
             'database' => $optEmpty,
             'password' => $optEmpty,
-            'connection_async' => $optBoolean,
-            'connection_persistent' => $optBoolean,
-            'connection_timeout' => new Options\CustomOption(array(
+            'connection_async' => $optBoolFalse,
+            'connection_persistent' => $optBoolFalse,
+            'connection_timeout' => new CustomOption(array(
+                'validate' => function($value) { return (float) $value; },
                 'default'  => function() { return 5; },
             )),
-            'read_write_timeout' => $optEmpty,
+            'read_write_timeout' => new CustomOption(array(
+                'validate' => function($value) { return (float) $value; },
+            )),
             'alias' => $optEmpty,
             'weight' => $optEmpty,
+            'iterable_multibulk' => $optBoolFalse,
+            'throw_errors' => $optBoolTrue,
         );
 
         return self::$_sharedOptions;
     }
 
-    public static function define($parameter, Options\IOption $handler) {
+    public static function define($parameter, IOption $handler) {
         self::getSharedOptions();
         self::$_sharedOptions[$parameter] = $handler;
     }
@@ -77,7 +90,7 @@ class ConnectionParameters {
         }
         $parsed = @parse_url($uri);
         if ($parsed == false || !isset($parsed['host'])) {
-            throw new ClientException("Invalid URI: $uri");
+            throw new \InvalidArgumentException("Invalid URI: $uri");
         }
         if (array_key_exists('query', $parsed)) {
             $query  = explode('&', $parsed['query']);
@@ -113,11 +126,7 @@ class ConnectionParameters {
     }
 
     public function __isset($parameter) {
-        if (isset($this->_parameters[$parameter])) {
-            return true;
-        }
-        $value = $this->tryInitializeValue($parameter);
-        return isset($value);
+        return isset($this->_userDefined[$parameter]);
     }
 
     public function __toString() {
@@ -132,10 +141,9 @@ class ConnectionParameters {
         $query = array();
         $reject = array('scheme', 'host', 'port', 'password', 'path');
         foreach ($this->_parameters as $k => $v) {
-            if (in_array($k, $reject) || $v === null) {
+            if (in_array($k, $reject) || !isset($this->_userDefined[$k])) {
                 continue;
             }
-            $v = $v === false ? '0' : $v;
             $query[] = $k . '=' . ($v === false ? '0' : $v);
         }
         return count($query) > 0 ? ($str . '/?' . implode('&', $query)) : $str;

+ 1 - 1
lib/Predis/Network/ComposableStreamConnection.php

@@ -12,8 +12,8 @@ class ComposableStreamConnection extends StreamConnection implements IConnection
     private $_protocol;
 
     public function __construct(ConnectionParameters $parameters, IProtocolProcessor $protocol = null) {
+        $this->setProtocol($protocol ?: new TextProtocol());
         parent::__construct($parameters);
-        $this->_protocol = $protocol ?: new TextProtocol();
     }
 
     public function setProtocol(IProtocolProcessor $protocol) {

+ 37 - 4
lib/Predis/Network/ConnectionBase.php

@@ -2,6 +2,7 @@
 
 namespace Predis\Network;
 
+use \InvalidArgumentException;
 use Predis\Utils;
 use Predis\ConnectionParameters;
 use Predis\ClientException;
@@ -14,19 +15,43 @@ abstract class ConnectionBase implements IConnectionSingle {
 
     public function __construct(ConnectionParameters $parameters) {
         $this->_initCmds = array();
-        $this->_params   = $parameters;
+        $this->_params = $this->checkParameters($parameters);
+        $this->initializeProtocol($parameters);
     }
 
     public function __destruct() {
         $this->disconnect();
     }
 
-    public function isConnected() {
-        return isset($this->_resource);
+    protected function checkParameters(ConnectionParameters $parameters) {
+        switch ($parameters->scheme) {
+            case 'unix':
+                $pathToSocket = $parameters->path;
+                if (!isset($pathToSocket)) {
+                    throw new InvalidArgumentException('Missing UNIX domain socket path');
+                }
+                if (!file_exists($pathToSocket)) {
+                    throw new InvalidArgumentException("Could not find $pathToSocket");
+                }
+            case 'tcp':
+                return $parameters;
+            default:
+                throw new InvalidArgumentException("Invalid scheme: {$parameters->scheme}");
+        }
+        return $parameters;
+    }
+
+    protected function initializeProtocol(ConnectionParameters $parameters) {
+        $this->setProtocolOption('throw_errors', $parameters->throw_errors);
+        $this->setProtocolOption('iterable_multibulk', $parameters->iterable_multibulk);
     }
 
     protected abstract function createResource();
 
+    public function isConnected() {
+        return isset($this->_resource);
+    }
+
     public function connect() {
         if ($this->isConnected()) {
             throw new ClientException('Connection already estabilished');
@@ -38,7 +63,7 @@ abstract class ConnectionBase implements IConnectionSingle {
         unset($this->_resource);
     }
 
-    public function pushInitCommand(ICommand $command){
+    public function pushInitCommand(ICommand $command) {
         $this->_initCmds[] = $command;
     }
 
@@ -53,6 +78,14 @@ abstract class ConnectionBase implements IConnectionSingle {
         );
     }
 
+    protected function onInvalidOption($option, $parameters = null) {
+        $message = "Invalid option: $option";
+        if (isset($parameters)) {
+            $message .= " [$parameters]";
+        }
+        throw new InvalidArgumentException($message);
+    }
+
     public function getResource() {
         if (isset($this->_resource)) {
             return $this->_resource;

+ 17 - 26
lib/Predis/Network/PhpiredisConnection.php

@@ -35,8 +35,7 @@ class PhpiredisConnection extends ConnectionBase {
                 'use this connection class'
             );
         }
-        parent::__construct($this->checkParameters($parameters));
-        $this->_reader = $this->initializeReader();
+        parent::__construct($parameters);
     }
 
     public function __destruct() {
@@ -45,25 +44,13 @@ class PhpiredisConnection extends ConnectionBase {
     }
 
     protected function checkParameters(ConnectionParameters $parameters) {
-        switch ($parameters->scheme) {
-            case 'unix':
-                $pathToSocket = $parameters->path;
-                if (!isset($pathToSocket)) {
-                    throw new \InvalidArgumentException('Missing UNIX domain socket path');
-                }
-                if (!file_exists($pathToSocket)) {
-                    throw new \InvalidArgumentException("Could not find $pathToSocket");
-                }
-            case 'tcp':
-                return $parameters;
-            default:
-                throw new \InvalidArgumentException("Invalid scheme: {$parameters->scheme}");
+        if (isset($parameters->iterable_multibulk)) {
+            $this->onInvalidOption('iterable_multibulk', $parameters);
         }
-        if ($parameters->connection_persistent == true) {
-            throw new \InvalidArgumentException(
-                'Persistent connections are not supported by this connection class.'
-            );
+        if (isset($parameters->connection_persistent)) {
+            $this->onInvalidOption('connection_persistent', $parameters);
         }
+        return parent::checkParameters($parameters);
     }
 
     private function initializeReader() {
@@ -75,8 +62,13 @@ class PhpiredisConnection extends ConnectionBase {
         }
         $reader = phpiredis_reader_create();
         phpiredis_reader_set_status_handler($reader, $this->getStatusHandler());
-        phpiredis_reader_set_error_handler($reader, $this->getErrorHandler(true));
-        return $reader;
+        phpiredis_reader_set_error_handler($reader, $this->getErrorHandler());
+        $this->_reader = $reader;
+    }
+
+    protected function initializeProtocol(ConnectionParameters $parameters) {
+        $this->initializeReader();
+        $this->setProtocolOption('throw_errors', $parameters->throw_errors);
     }
 
     private function getStatusHandler() {
@@ -92,7 +84,7 @@ class PhpiredisConnection extends ConnectionBase {
         };
     }
 
-    private function getErrorHandler($throwErrors) {
+    private function getErrorHandler($throwErrors = true) {
         if ($throwErrors) {
             return function($errorMessage) {
                 throw new ServerException(substr($errorMessage, 4));
@@ -231,7 +223,7 @@ class PhpiredisConnection extends ConnectionBase {
         while (($length = strlen($buffer)) > 0) {
             $written = socket_write($socket, $buffer, $length);
             if ($length === $written) {
-                return true;
+                return;
             }
             if ($written === false) {
                 $this->onCommunicationException('Error while writing bytes to the server');
@@ -270,15 +262,14 @@ class PhpiredisConnection extends ConnectionBase {
 
     public function setProtocolOption($option, $value) {
         switch ($option) {
-            case 'iterable_multibulk':
-                // TODO: iterable multibulk replies cannot be supported
-                break;
             case 'throw_errors':
                 phpiredis_reader_set_error_handler(
                     $this->_reader,
                     $this->getErrorHandler((bool) $value)
                 );
                 break;
+            default:
+                $this->onInvalidOption($option, $this->getParameters());
         }
     }
 }

+ 5 - 20
lib/Predis/Network/StreamConnection.php

@@ -14,32 +14,15 @@ use Predis\Iterators\MultiBulkResponseSimple;
 class StreamConnection extends ConnectionBase {
     private $_commandSerializer, $_mbiterable, $_throwErrors;
 
-    public function __construct(ConnectionParameters $parameters) {
-        parent::__construct($this->checkParameters($parameters));
-        $this->_commandSerializer = new TextCommandSerializer();
-    }
-
     public function __destruct() {
         if (!$this->_params->connection_persistent) {
             $this->disconnect();
         }
     }
 
-    protected function checkParameters(ConnectionParameters $parameters) {
-        switch ($parameters->scheme) {
-            case 'unix':
-                $pathToSocket = $parameters->path;
-                if (!isset($pathToSocket)) {
-                    throw new \InvalidArgumentException('Missing UNIX domain socket path');
-                }
-                if (!file_exists($pathToSocket)) {
-                    throw new \InvalidArgumentException("Could not find $pathToSocket");
-                }
-            case 'tcp':
-                return $parameters;
-            default:
-                throw new \InvalidArgumentException("Invalid scheme: {$parameters->scheme}");
-        }
+    protected function initializeProtocol(ConnectionParameters $parameters) {
+        $this->_commandSerializer = new TextCommandSerializer();
+        parent::initializeProtocol($parameters);
     }
 
     protected function createResource() {
@@ -213,6 +196,8 @@ class StreamConnection extends ConnectionBase {
             case 'throw_errors':
                 $this->_throwErrors = (bool) $value;
                 break;
+            default:
+                $this->onInvalidOption($option, $this->getParameters());
         }
     }
 }

+ 0 - 13
lib/Predis/Options/ClientIterableMultiBulk.php

@@ -1,13 +0,0 @@
-<?php
-
-namespace Predis\Options;
-
-class ClientIterableMultiBulk extends Option {
-    public function validate($value) {
-        return (bool) $value;
-    }
-
-    public function getDefault() {
-        return false;
-    }
-}

+ 0 - 13
lib/Predis/Options/ClientThrowOnError.php

@@ -1,13 +0,0 @@
-<?php
-
-namespace Predis\Options;
-
-class ClientThrowOnError extends Option {
-    public function validate($value) {
-        return (bool) $value;
-    }
-
-    public function getDefault() {
-        return true;
-    }
-}

+ 3 - 1
lib/Predis/Options/CustomOption.php

@@ -16,7 +16,9 @@ class CustomOption extends Option {
     }
 
     public function validate($value) {
-        return call_user_func($this->_validate, $value);
+        if (isset($value)) {
+            return call_user_func($this->_validate, $value);
+        }
     }
 
     public function getDefault() {