From ddcdbf461ca50f88e579e86bcc86e6089e94cca1 Mon Sep 17 00:00:00 2001
From: chabior <chabior@o2.pl>
Date: Wed, 6 Aug 2014 12:05:13 +0100
Subject: [PATCH 1/5] Add connection pool and strategies

---
 lib/Elastica/Client.php                       | 63 ++++++---------
 lib/Elastica/Connection/ConnectionPool.php    | 81 +++++++++++++++++++
 .../Connection/Strategy/CallbackStrategy.php  | 47 +++++++++++
 lib/Elastica/Connection/Strategy/Request.php  | 29 +++++++
 .../Connection/Strategy/RoundRobin.php        | 23 ++++++
 lib/Elastica/Connection/Strategy/Simple.php   | 29 +++++++
 .../Connection/Strategy/StrategyFactory.php   | 48 +++++++++++
 .../Connection/Strategy/StrategyInterface.php | 18 +++++
 8 files changed, 300 insertions(+), 38 deletions(-)
 create mode 100644 lib/Elastica/Connection/ConnectionPool.php
 create mode 100644 lib/Elastica/Connection/Strategy/CallbackStrategy.php
 create mode 100644 lib/Elastica/Connection/Strategy/Request.php
 create mode 100644 lib/Elastica/Connection/Strategy/RoundRobin.php
 create mode 100644 lib/Elastica/Connection/Strategy/Simple.php
 create mode 100644 lib/Elastica/Connection/Strategy/StrategyFactory.php
 create mode 100644 lib/Elastica/Connection/Strategy/StrategyInterface.php

diff --git a/lib/Elastica/Client.php b/lib/Elastica/Client.php
index a582118041..d9ec80737d 100644
--- a/lib/Elastica/Client.php
+++ b/lib/Elastica/Client.php
@@ -43,11 +43,6 @@ class Client
         'retryOnConflict' => 0,
     );
 
-    /**
-     * @var \Elastica\Connection[] List of connections
-     */
-    protected $_connections = array();
-
     /**
      * @var callback
      */
@@ -67,6 +62,11 @@ class Client
      * @var LoggerInterface
      */
     protected $_logger = null;
+    /**
+     *
+     * @var Connection\ConnectionPool
+     */
+    protected $_connectionPool = null;
 
     /**
      * Creates a new Elastica client
@@ -86,22 +86,30 @@ public function __construct(array $config = array(), $callback = null)
      */
     protected function _initConnections()
     {
-        $connections = $this->getConfig('connections');
-
-        foreach ($connections as $connection) {
-            $this->_connections[] = Connection::create($this->_prepareConnectionParams($connection));
+        $connections = array();
+        
+        foreach ($this->getConfig('connections') as $connection) {
+            $connections[] = Connection::create($this->_prepareConnectionParams($connection));
         }
 
         if (isset($this->_config['servers'])) {
             foreach ($this->getConfig('servers') as $server) {
-                $this->_connections[] = Connection::create($this->_prepareConnectionParams($server));
+                $connections[] = Connection::create($this->_prepareConnectionParams($server));
             }
         }
 
         // If no connections set, create default connection
-        if (empty($this->_connections)) {
-            $this->_connections[] = Connection::create($this->_prepareConnectionParams($this->getConfig()));
+        if (empty($connections)) {
+            $connections[] = Connection::create($this->_prepareConnectionParams($this->getConfig()));
+        }
+        
+        if (isset($this->_config['connectionStrategy'])) {
+            $strategy = Connection\Strategy\StrategyFactory::create($this->getConfig('connectionStrategy'));
+        } else {
+            $strategy = Connection\Strategy\StrategyFactory::getSimpleStrategy();
         }
+        
+        $this->_connectionPool = new Connection\ConnectionPool($connections, $strategy);
     }
 
     /**
@@ -439,7 +447,7 @@ public function getCluster()
      */
     public function addConnection(Connection $connection)
     {
-        $this->_connections[] = $connection;
+        $this->_connectionPool->addConnection($connection);
 
         return $this;
     }
@@ -451,15 +459,7 @@ public function addConnection(Connection $connection)
      */
     public function hasConnection()
     {
-        foreach ($this->_connections as $connection)
-        {
-            if ($connection->isEnabled())
-            {
-                return true;
-            }
-        }
-        
-        return false;
+        return $this->_connectionPool->hasConnection();
     }
 
     /**
@@ -468,20 +468,7 @@ public function hasConnection()
      */
     public function getConnection()
     {
-        $enabledConnection = null;
-
-        foreach ($this->_connections as $connection) {
-            if ($connection->isEnabled()) {
-                $enabledConnection = $connection;
-                break;
-            }
-        }
-
-        if (empty($enabledConnection)) {
-            throw new ClientException('No enabled connection');
-        }
-
-        return $enabledConnection;
+        return $this->_connectionPool->getConnection();
     }
 
     /**
@@ -489,7 +476,7 @@ public function getConnection()
      */
     public function getConnections()
     {
-        return $this->_connections;
+        return $this->_connectionPool->getConnections();
     }
 
     /**
@@ -498,7 +485,7 @@ public function getConnections()
      */
     public function setConnections(array $connections)
     {
-        $this->_connections = $connections;
+        $this->_connectionPool->setConnections($connections);
 
         return $this;
     }
diff --git a/lib/Elastica/Connection/ConnectionPool.php b/lib/Elastica/Connection/ConnectionPool.php
new file mode 100644
index 0000000000..220733b957
--- /dev/null
+++ b/lib/Elastica/Connection/ConnectionPool.php
@@ -0,0 +1,81 @@
+<?php
+
+namespace Elastica\Connection;
+
+use Elastica\Connection;
+use Elastica\Connection\Strategy\StrategyInterface;
+
+/**
+ * Description of ConnectionPool
+ *
+ * @author chabior
+ */
+class ConnectionPool
+{
+    /**
+     *
+     * @var array
+     */
+    protected $connections;
+    /**
+     *
+     * @var  Elastica\Connection\Strategy\StrategyInterface
+     */
+    protected $strategy;
+    /**
+     * 
+     * @param array|Connection[] $connections
+     */
+    public function __construct(array $connections, StrategyInterface $strategy)
+    {
+        $this->connections = $connections;
+        
+        $this->strategy = $strategy;
+    }
+    /**
+     * 
+     * @param \Elastica\Connection $connection
+     */
+    public function addConnection(Connection $connection)
+    {
+        $this->connections[] = $connection;
+    }
+    /**
+     * 
+     * @param array|Connection[] $connections
+     */
+    public function setConnections(array $connections)
+    {
+        $this->connections = $connections;
+    }
+    /**
+     * 
+     * @return boolean
+     */
+    public function hasConnection()
+    {
+        foreach ($this->connections as $connection) {
+            if ($connection->isEnabled()) {
+                return true;
+            }
+        }
+        
+        return false;
+    }
+    /**
+     * 
+     * @return array
+     */
+    public function getConnections()
+    {
+        return $this->connections;
+    }
+    /**
+     * 
+     * @return \Elastica\Connection
+     */
+    public function getConnection()
+    {
+        return $this->strategy->getConnection($this->getConnections());
+    }
+}
diff --git a/lib/Elastica/Connection/Strategy/CallbackStrategy.php b/lib/Elastica/Connection/Strategy/CallbackStrategy.php
new file mode 100644
index 0000000000..e56eb288cc
--- /dev/null
+++ b/lib/Elastica/Connection/Strategy/CallbackStrategy.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace Elastica\Connection\Strategy;
+
+/**
+ * Description of CallbackStrategy
+ *
+ * @author chabior
+ */
+class CallbackStrategy implements StrategyInterface
+{
+    /**
+     *
+     * @var Closure 
+     */
+    protected $callback;
+    /**
+     * 
+     * @param Closure $callback
+     * @throws \InvalidArgumentException
+     */
+    public function __construct($callback)
+    {
+        if (!self::isValid($callback)) {
+            throw new \InvalidArgumentException(sprintf('Callback should be a Closure, %s given!', gettype($callback)));
+        }
+        
+        $this->callback = $callback;
+    }
+    /**
+     * 
+     * @param array|\Elastica\Connection[] $connections
+     * @return \Elastica\Connection
+     */
+    public function getConnection($connections)
+    {
+        return $this->callback->__invoke($connections);
+    }
+    /**
+     * 
+     * @return boolean
+     */
+    public static function isValid($callback)
+    {
+        return is_object($callback) && ($callback instanceof Closure);
+    }
+}
diff --git a/lib/Elastica/Connection/Strategy/Request.php b/lib/Elastica/Connection/Strategy/Request.php
new file mode 100644
index 0000000000..3bed31bdb5
--- /dev/null
+++ b/lib/Elastica/Connection/Strategy/Request.php
@@ -0,0 +1,29 @@
+<?php
+
+namespace Elastica\Connection\Strategy;
+
+use Elastica\Connection\Strategy\Simple;
+
+/**
+ * Description of Request
+ *
+ * @author chabior
+ */
+class Request extends Simple
+{
+    /**
+     * 
+     * @param array|\Elastica\Connection[] $connections
+     * @return \Elastica\Connection
+     */
+    public function getConnection($connections)
+    {
+        static $connection = null;
+        
+        if (null === $connection) {
+            $connection = parent::getConnection($connections);
+        }
+        
+        return $connection;
+    }
+}
diff --git a/lib/Elastica/Connection/Strategy/RoundRobin.php b/lib/Elastica/Connection/Strategy/RoundRobin.php
new file mode 100644
index 0000000000..657010cf5f
--- /dev/null
+++ b/lib/Elastica/Connection/Strategy/RoundRobin.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace Elastica\Connection\Strategy;
+
+/**
+ * Description of RoundRobin
+ *
+ * @author chabior
+ */
+class RoundRobin extends Simple
+{
+    /**
+     * 
+     * @param array|\Elastica\Connection[] $connections
+     * @return \Elastica\Connection
+     */
+    public function getConnection($connections)
+    {
+        shuffle($connections);
+        
+        return parent::getConnection($connections);
+    }
+}
diff --git a/lib/Elastica/Connection/Strategy/Simple.php b/lib/Elastica/Connection/Strategy/Simple.php
new file mode 100644
index 0000000000..e2d0c75e97
--- /dev/null
+++ b/lib/Elastica/Connection/Strategy/Simple.php
@@ -0,0 +1,29 @@
+<?php
+
+namespace Elastica\Connection\Strategy;
+
+use Elastica\Connection\Strategy\StrategyInterface;
+
+/**
+ * Description of SimpleStrategy
+ *
+ * @author chabior
+ */
+class Simple implements StrategyInterface
+{
+    /**
+     * 
+     * @param array|\Elastica\Connection[] $connections
+     * @return \Elastica\Connection
+     */
+    public function getConnection($connections)
+    {
+        foreach ($connections as $connection) {
+              if ($connection->isEnabled()) {
+                  return $connection;
+              }   
+        }
+        
+        throw new ClientException('No enabled connection');
+    }
+}
diff --git a/lib/Elastica/Connection/Strategy/StrategyFactory.php b/lib/Elastica/Connection/Strategy/StrategyFactory.php
new file mode 100644
index 0000000000..4699b700a0
--- /dev/null
+++ b/lib/Elastica/Connection/Strategy/StrategyFactory.php
@@ -0,0 +1,48 @@
+<?php
+
+namespace Elastica\Connection\Strategy;
+
+/**
+ * Description of StrategyFactory
+ *
+ * @author chabior
+ */
+class StrategyFactory
+{
+    /**
+     * 
+     * @param mixed|Closure|String|StrategyInterface $strategyName
+     * @return \Elastica\Connection\Strategy\StrategyInterface
+     * @throws \InvalidArgumentException
+     */
+    public static function create($strategyName)
+    {
+        $strategy = null;
+        if ($strategyName instanceof StrategyInterface) {
+            $strategy = $strategyName;
+        } else if (CallbackStrategy::isValid($strategyName)) {
+            $strategy = new CallbackStrategy($strategyName);
+        } else if (class_exists($strategyName)) {
+            $strategy = new $strategyName();
+        } else if (is_string($strategyName)) {
+            $pathToStrategy = '\\Elastica\\Connection\\Strategy\\'.$strategyName;
+            if (class_exists($pathToStrategy)) {
+                $strategy = new $pathToStrategy();
+            }
+        }
+        
+        if (!$strategy instanceof StrategyInterface) {
+            throw new \InvalidArgumentException('Can\'t load strategy class');
+        }
+        
+        return $strategy;
+    }
+    /**
+     * 
+     * @return \Elastica\Connection\Strategy\Simple
+     */
+    public static function getSimpleStrategy()
+    {
+        return new Simple();
+    }
+}
diff --git a/lib/Elastica/Connection/Strategy/StrategyInterface.php b/lib/Elastica/Connection/Strategy/StrategyInterface.php
new file mode 100644
index 0000000000..3b8e23e2dd
--- /dev/null
+++ b/lib/Elastica/Connection/Strategy/StrategyInterface.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace Elastica\Connection\Strategy;
+
+/**
+ * Description of AbstractStrategy
+ *
+ * @author chabior
+ */
+interface StrategyInterface
+{
+    /**
+     * 
+     * @param array|\Elastica\Connection[] $connections
+     * @return \Elastica\Connection
+     */
+    public function getConnection($connections);
+}

From 384e930059cc1db726241532a2765d68f9e3715f Mon Sep 17 00:00:00 2001
From: chabior <chabior@o2.pl>
Date: Wed, 6 Aug 2014 17:38:40 +0100
Subject: [PATCH 2/5] Add tests, minor fixes

---
 changes.txt                                   |  3 +
 lib/Elastica/Client.php                       |  9 +-
 lib/Elastica/Connection/ConnectionPool.php    | 31 ++++++-
 .../Connection/Strategy/CallbackStrategy.php  |  2 +-
 lib/Elastica/Connection/Strategy/Request.php  | 29 -------
 .../Connection/Strategy/StrategyFactory.php   |  2 +-
 .../Test/Connection/ConnectionPollTest.php    | 83 +++++++++++++++++++
 .../Strategy/CallbackStrategyTest.php         | 61 ++++++++++++++
 .../Connection/Strategy/EmptyStrategy.php     | 18 ++++
 .../Connection/Strategy/RoundRobinTest.php    | 20 +++++
 .../Test/Connection/Strategy/SimplyTest.php   | 19 +++++
 .../Strategy/StrategyFactoryTest.php          | 79 ++++++++++++++++++
 12 files changed, 314 insertions(+), 42 deletions(-)
 delete mode 100644 lib/Elastica/Connection/Strategy/Request.php
 create mode 100644 test/lib/Elastica/Test/Connection/ConnectionPollTest.php
 create mode 100644 test/lib/Elastica/Test/Connection/Strategy/CallbackStrategyTest.php
 create mode 100644 test/lib/Elastica/Test/Connection/Strategy/EmptyStrategy.php
 create mode 100644 test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php
 create mode 100644 test/lib/Elastica/Test/Connection/Strategy/SimplyTest.php
 create mode 100644 test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php

diff --git a/changes.txt b/changes.txt
index 525a6336fc..51826c3b0a 100755
--- a/changes.txt
+++ b/changes.txt
@@ -1,5 +1,8 @@
 CHANGES
 
+2014-08-06
+ - Add connection pool and connection strategy
+
 2014-07-26
 - Release v1.3.0.0
 - Prepare Elastica Release v1.3.0.0
diff --git a/lib/Elastica/Client.php b/lib/Elastica/Client.php
index d9ec80737d..ff1cd78d4c 100644
--- a/lib/Elastica/Client.php
+++ b/lib/Elastica/Client.php
@@ -109,7 +109,7 @@ protected function _initConnections()
             $strategy = Connection\Strategy\StrategyFactory::getSimpleStrategy();
         }
         
-        $this->_connectionPool = new Connection\ConnectionPool($connections, $strategy);
+        $this->_connectionPool = new Connection\ConnectionPool($connections, $strategy, $this->_callback);
     }
 
     /**
@@ -584,12 +584,7 @@ public function request($path, $method = Request::GET, $data = array(), array $q
             return $response;
 
         } catch (ConnectionException $e) {
-            $connection->setEnabled(false);
-
-            // Calls callback with connection as param to make it possible to persist invalid connections
-            if ($this->_callback) {
-                call_user_func($this->_callback, $connection, $e, $this);
-            }
+            $this->_connectionPool->onFail($connection, $e, $this);
 
             // In case there is no valid connection left, throw exception which caused the disabling of the connection.
             if (!$this->hasConnection())
diff --git a/lib/Elastica/Connection/ConnectionPool.php b/lib/Elastica/Connection/ConnectionPool.php
index 220733b957..14f1155d58 100644
--- a/lib/Elastica/Connection/ConnectionPool.php
+++ b/lib/Elastica/Connection/ConnectionPool.php
@@ -2,8 +2,10 @@
 
 namespace Elastica\Connection;
 
+use Elastica\Client;
 use Elastica\Connection;
 use Elastica\Connection\Strategy\StrategyInterface;
+use Exception;
 
 /**
  * Description of ConnectionPool
@@ -19,22 +21,29 @@ class ConnectionPool
     protected $connections;
     /**
      *
-     * @var  Elastica\Connection\Strategy\StrategyInterface
+     * @var  StrategyInterface
      */
     protected $strategy;
+    /**
+     *
+     * @var callback
+     */
+    protected $callback;
     /**
      * 
      * @param array|Connection[] $connections
      */
-    public function __construct(array $connections, StrategyInterface $strategy)
+    public function __construct(array $connections, StrategyInterface $strategy, $callback = null)
     {
         $this->connections = $connections;
         
         $this->strategy = $strategy;
+        
+        $this->callback = $callback;
     }
     /**
      * 
-     * @param \Elastica\Connection $connection
+     * @param Connection $connection
      */
     public function addConnection(Connection $connection)
     {
@@ -72,10 +81,24 @@ public function getConnections()
     }
     /**
      * 
-     * @return \Elastica\Connection
+     * @return Connection
      */
     public function getConnection()
     {
         return $this->strategy->getConnection($this->getConnections());
     }
+    /**
+     * 
+     * @param Connection $connection
+     * @param Exception $e
+     * @param Client $client
+     */
+    public function onFail(Connection $connection, Exception $e, Client $client)
+    {
+        $connection->setEnabled(false);
+        
+        if ($this->callback) {
+            call_user_func($this->callback, $connection, $e, $client);
+        }
+    }
 }
diff --git a/lib/Elastica/Connection/Strategy/CallbackStrategy.php b/lib/Elastica/Connection/Strategy/CallbackStrategy.php
index e56eb288cc..5692bfd2b7 100644
--- a/lib/Elastica/Connection/Strategy/CallbackStrategy.php
+++ b/lib/Elastica/Connection/Strategy/CallbackStrategy.php
@@ -42,6 +42,6 @@ public function getConnection($connections)
      */
     public static function isValid($callback)
     {
-        return is_object($callback) && ($callback instanceof Closure);
+        return is_object($callback) && ($callback instanceof \Closure);
     }
 }
diff --git a/lib/Elastica/Connection/Strategy/Request.php b/lib/Elastica/Connection/Strategy/Request.php
deleted file mode 100644
index 3bed31bdb5..0000000000
--- a/lib/Elastica/Connection/Strategy/Request.php
+++ /dev/null
@@ -1,29 +0,0 @@
-<?php
-
-namespace Elastica\Connection\Strategy;
-
-use Elastica\Connection\Strategy\Simple;
-
-/**
- * Description of Request
- *
- * @author chabior
- */
-class Request extends Simple
-{
-    /**
-     * 
-     * @param array|\Elastica\Connection[] $connections
-     * @return \Elastica\Connection
-     */
-    public function getConnection($connections)
-    {
-        static $connection = null;
-        
-        if (null === $connection) {
-            $connection = parent::getConnection($connections);
-        }
-        
-        return $connection;
-    }
-}
diff --git a/lib/Elastica/Connection/Strategy/StrategyFactory.php b/lib/Elastica/Connection/Strategy/StrategyFactory.php
index 4699b700a0..43453594e1 100644
--- a/lib/Elastica/Connection/Strategy/StrategyFactory.php
+++ b/lib/Elastica/Connection/Strategy/StrategyFactory.php
@@ -22,7 +22,7 @@ public static function create($strategyName)
             $strategy = $strategyName;
         } else if (CallbackStrategy::isValid($strategyName)) {
             $strategy = new CallbackStrategy($strategyName);
-        } else if (class_exists($strategyName)) {
+        } else if (is_string($strategyName) && class_exists($strategyName)) {
             $strategy = new $strategyName();
         } else if (is_string($strategyName)) {
             $pathToStrategy = '\\Elastica\\Connection\\Strategy\\'.$strategyName;
diff --git a/test/lib/Elastica/Test/Connection/ConnectionPollTest.php b/test/lib/Elastica/Test/Connection/ConnectionPollTest.php
new file mode 100644
index 0000000000..0daa4d0d82
--- /dev/null
+++ b/test/lib/Elastica/Test/Connection/ConnectionPollTest.php
@@ -0,0 +1,83 @@
+<?php
+
+namespace Elastica\Test\Connection;
+
+use Elastica\Test\Base as BaseTest;
+
+/*
+ * To change this license header, choose License Headers in Project Properties.
+ * To change this template file, choose Tools | Templates
+ * and open the template in the editor.
+ */
+
+/**
+ * Description of ConnectionPollTest
+ *
+ * @author chabior
+ */
+class ConnectionPollTest extends BaseTest
+{
+    public function testConstruct()
+    {
+        $pool = $this->createPool();
+        
+        $this->assertEquals($this->getConnections(), $pool->getConnections());
+    }
+    
+    public function testSetConnections()
+    {
+        $pool = $this->createPool();
+        
+        $connections = $this->getConnections(5);
+        
+        $pool->setConnections($connections);
+        
+        $this->assertEquals($connections, $pool->getConnections());
+    }
+    
+    public function testHasConnection()
+    {
+        $pool = $this->createPool();
+        
+        $this->assertTrue($pool->hasConnection());
+    }
+    
+    public function testFailHasConnections()
+    {
+        $pool = $this->createPool();
+        
+        $pool->setConnections(array());
+        
+        $this->assertFalse($pool->hasConnection());
+    }
+    
+    public function testGetConnection()
+    {
+        $pool = $this->createPool();
+        
+        $this->assertTrue($pool->getConnection() instanceof \Elastica\Connection);
+    }
+    
+    protected function getConnections($quantity = 1)
+    {
+        $params = array();
+        $connections = array();
+        
+        for ($i = 0; $i<$quantity; $i++) {
+            $connections[] = new \Elastica\Connection($params);
+        }
+        
+        return $connections;
+    }
+    
+    protected function createPool()
+    {
+        $connections = $this->getConnections();
+        $strategy = \Elastica\Connection\Strategy\StrategyFactory::getSimpleStrategy();
+        
+        $pool = new \Elastica\Connection\ConnectionPool($connections, $strategy);
+        $pool->getConnections();
+        
+        return $pool;
+    }
+}
diff --git a/test/lib/Elastica/Test/Connection/Strategy/CallbackStrategyTest.php b/test/lib/Elastica/Test/Connection/Strategy/CallbackStrategyTest.php
new file mode 100644
index 0000000000..42bb2fd149
--- /dev/null
+++ b/test/lib/Elastica/Test/Connection/Strategy/CallbackStrategyTest.php
@@ -0,0 +1,61 @@
+<?php
+
+namespace Elastica\Test\Connection\Strategy;
+
+use Elastica\Connection\Strategy\CallbackStrategy;
+use Elastica\Test\Base;
+
+/**
+ * Description of CallbackStrategyTest
+ *
+ * @author chabior
+ */
+class CallbackStrategyTest extends Base
+{
+    public function testInvoke()
+    {
+        $count = 0;
+        
+        $callback = function ($connections) use(&$count) {
+            $count++;
+        };
+        
+        $strategy = new CallbackStrategy($callback);
+        $strategy->getConnection(array());
+        
+        $this->assertEquals(1, $count);
+    }
+    
+    public function testIsValid()
+    {
+        $callback = function(){};
+        
+        $isValid = CallbackStrategy::isValid($callback);
+        
+        $this->assertTrue($isValid);
+    }
+    
+    public function testFailIsValid()
+    {
+        $callback = new \stdClass();
+        
+        $isValid = CallbackStrategy::isValid($callback);
+        
+        $this->assertFalse($isValid);
+    }
+    
+    public function testConnection()
+    {
+        $count = 0;
+        
+        $config = array('connectionStrategy' => function ($connections) use(&$count) {
+            ++$count;
+            return current($connections);
+       });
+       
+       $client = new \Elastica\Client($config);
+       $client->request('/_aliases');
+       
+       $this->assertEquals(1, $count);
+    }
+}
diff --git a/test/lib/Elastica/Test/Connection/Strategy/EmptyStrategy.php b/test/lib/Elastica/Test/Connection/Strategy/EmptyStrategy.php
new file mode 100644
index 0000000000..85acf54701
--- /dev/null
+++ b/test/lib/Elastica/Test/Connection/Strategy/EmptyStrategy.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace Elastica\Test\Connection\Strategy;
+
+use Elastica\Connection\Strategy\StrategyInterface;
+
+/**
+ * Description of EmptyStrategy
+ *
+ * @author chabior
+ */
+class EmptyStrategy implements StrategyInterface
+{
+    public function getConnection($connections)
+    {
+        return null;
+    }
+}
diff --git a/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php b/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php
new file mode 100644
index 0000000000..baba920aac
--- /dev/null
+++ b/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace Elastica\Test\Connection\Strategy;
+
+use Elastica\Test\Base;
+
+/**
+ * Description of RoundRobinTest
+ *
+ * @author chabior
+ */
+class RoundRobinTest extends Base
+{
+   public function testConnection()
+   {
+       $config = array('connectionStrategy' => 'RoundRobin');
+       $client = new \Elastica\Client($config);
+       $client->request('/_aliases');
+   }
+}
diff --git a/test/lib/Elastica/Test/Connection/Strategy/SimplyTest.php b/test/lib/Elastica/Test/Connection/Strategy/SimplyTest.php
new file mode 100644
index 0000000000..2f0472f8fc
--- /dev/null
+++ b/test/lib/Elastica/Test/Connection/Strategy/SimplyTest.php
@@ -0,0 +1,19 @@
+<?php
+
+namespace Elastica\Test\Connection\Strategy;
+
+use Elastica\Test\Base;
+
+/**
+ * Description of SimplyTest
+ *
+ * @author chabior
+ */
+class SimplyTest extends Base
+{
+    public function testConnection()
+    {
+        $client = new \Elastica\Client();
+        $client->request('/_aliases');
+    }
+}
diff --git a/test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php b/test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php
new file mode 100644
index 0000000000..b0a89a2f8e
--- /dev/null
+++ b/test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php
@@ -0,0 +1,79 @@
+<?php
+
+namespace Elastica\Test\Connection\Strategy;
+
+use Elastica\Connection\Strategy\CallbackStrategy;
+use Elastica\Connection\Strategy\Simple;
+use Elastica\Connection\Strategy\StrategyFactory;
+use Elastica\Test\Base;
+
+/*
+ * To change this license header, choose License Headers in Project Properties.
+ * To change this template file, choose Tools | Templates
+ * and open the template in the editor.
+ */
+
+/**
+ * Description of StrategyFactoryTest
+ *
+ * @author chabior
+ */
+class StrategyFactoryTest extends Base
+{
+   public function testGetSimpleStrategy()
+   {
+       $condition = (StrategyFactory::getSimpleStrategy() instanceof Simple);
+       
+       $this->assertTrue($condition);
+   }
+   
+   public function testCreateCallbackStrategy()
+   {
+       $callback = function ($connections)
+       {
+           
+       };
+       
+       $strategy = StrategyFactory::create($callback);
+       
+       $condition = $strategy instanceof CallbackStrategy;
+       
+       $this->assertTrue($condition);
+   }
+   
+   public function testCreateByName()
+   {
+       $strategyName = 'Simple';
+       
+       $strategy = StrategyFactory::create($strategyName);
+       
+       $this->assertTrue($strategy instanceof Simple);
+   }
+   
+   public function testCreateByClass()
+   {
+       $strategy = new EmptyStrategy();
+       
+       $this->assertEquals($strategy, StrategyFactory::create($strategy));
+   }
+   
+   public function testCreateByClassName()
+   {
+       $strategyName = '\\Elastica\Test\Connection\Strategy\\EmptyStrategy';
+       
+       $strategy = StrategyFactory::create($strategyName);
+       
+       $condition = $strategy instanceof $strategyName;
+       
+       $this->assertTrue($condition);
+   }
+   /**
+    * @expectedException \InvalidArgumentException
+    */
+   public function testFailCreate()
+   {
+       $strategy = new \stdClass();
+       
+       StrategyFactory::create($strategy);
+   }
+}

From ccdea913905b66ab0b8d7e90f07daf0b468dbb03 Mon Sep 17 00:00:00 2001
From: chabior <chabior@o2.pl>
Date: Thu, 7 Aug 2014 09:36:59 +0100
Subject: [PATCH 3/5] fixes after code review

---
 lib/Elastica/Client.php                       |  25 ++--
 lib/Elastica/Connection/ConnectionPool.php    |  76 +++++++-----
 .../Connection/Strategy/CallbackStrategy.php  |  14 +--
 .../Connection/Strategy/RoundRobin.php        |   2 +-
 lib/Elastica/Connection/Strategy/Simple.php   |   2 +-
 .../Connection/Strategy/StrategyFactory.php   |   9 --
 .../Connection/Strategy/StrategyInterface.php |   1 -
 .../Test/Connection/ConnectionPollTest.php    |   3 +-
 .../Strategy/CallbackStrategyTest.php         |  10 +-
 .../Connection/Strategy/RoundRobinTest.php    | 108 +++++++++++++++++-
 .../Test/Connection/Strategy/SimpleTest.php   | 102 +++++++++++++++++
 .../Test/Connection/Strategy/SimplyTest.php   |  19 ---
 .../Strategy/StrategyFactoryTest.php          |   7 --
 13 files changed, 287 insertions(+), 91 deletions(-)
 create mode 100644 test/lib/Elastica/Test/Connection/Strategy/SimpleTest.php
 delete mode 100644 test/lib/Elastica/Test/Connection/Strategy/SimplyTest.php

diff --git a/lib/Elastica/Client.php b/lib/Elastica/Client.php
index ff1cd78d4c..20ba8632fe 100644
--- a/lib/Elastica/Client.php
+++ b/lib/Elastica/Client.php
@@ -4,8 +4,6 @@
 
 use Elastica\Bulk;
 use Elastica\Bulk\Action;
-use Elastica\Exception\ResponseException;
-use Elastica\Exception\ClientException;
 use Elastica\Exception\ConnectionException;
 use Elastica\Exception\InvalidException;
 use Elastica\Exception\RuntimeException;
@@ -103,12 +101,16 @@ protected function _initConnections()
             $connections[] = Connection::create($this->_prepareConnectionParams($this->getConfig()));
         }
         
-        if (isset($this->_config['connectionStrategy'])) {
-            $strategy = Connection\Strategy\StrategyFactory::create($this->getConfig('connectionStrategy'));
-        } else {
-            $strategy = Connection\Strategy\StrategyFactory::getSimpleStrategy();
+        if (!isset($this->_config['connectionStrategy'])) {
+            if ($this->getConfig('roundRobin') === true) {
+                $this->setConfigValue('connectionStrategy', 'RoundRobin');
+            } else {
+                $this->setConfigValue('connectionStrategy', 'Simple');
+            }
         }
         
+        $strategy = Connection\Strategy\StrategyFactory::create($this->getConfig('connectionStrategy'));
+        
         $this->_connectionPool = new Connection\ConnectionPool($connections, $strategy, $this->_callback);
     }
 
@@ -478,9 +480,18 @@ public function getConnections()
     {
         return $this->_connectionPool->getConnections();
     }
+    
+    /**
+     * 
+     * @return \Connection\Strategy\StrategyInterface
+     */
+    public function getConnectionStrategy()
+    {
+        return $this->_connectionPool->getStrategy();
+    }
 
     /**
-     * @param  \Elastica\Connection[] $connections
+     * @param  array|\Elastica\Connection[] $connections
      * @return \Elastica\Client
      */
     public function setConnections(array $connections)
diff --git a/lib/Elastica/Connection/ConnectionPool.php b/lib/Elastica/Connection/ConnectionPool.php
index 14f1155d58..9df6fb9c7b 100644
--- a/lib/Elastica/Connection/ConnectionPool.php
+++ b/lib/Elastica/Connection/ConnectionPool.php
@@ -15,55 +15,62 @@
 class ConnectionPool
 {
     /**
-     *
-     * @var array
+     * Connections array
+     * 
+     * @var array|\Elastica\Connection[]
      */
-    protected $connections;
+    protected $_connections;
+    
     /**
-     *
-     * @var  StrategyInterface
+     * Strategy for connection
+     * 
+     * @var  \Elastica\Connection\Strategy\StrategyInterface
      */
-    protected $strategy;
+    protected $_strategy;
+    
     /**
-     *
+     * Callback function called on connection fail
+     * 
      * @var callback
      */
-    protected $callback;
+    protected $_callback;
+    
     /**
-     * 
-     * @param array|Connection[] $connections
+     * @param array $connections
+     * @param \Elastica\Connection\Strategy\StrategyInterface $strategy
+     * @param type $callback
      */
     public function __construct(array $connections, StrategyInterface $strategy, $callback = null)
     {
-        $this->connections = $connections;
+        $this->_connections = $connections;
         
-        $this->strategy = $strategy;
+        $this->_strategy = $strategy;
         
-        $this->callback = $callback;
+        $this->_callback = $callback;
     }
+    
     /**
-     * 
-     * @param Connection $connection
+     * @param \Elastica\Connection $connection
      */
     public function addConnection(Connection $connection)
     {
-        $this->connections[] = $connection;
+        $this->_connections[] = $connection;
     }
+    
     /**
-     * 
-     * @param array|Connection[] $connections
+     * @param array|\Elastica\Connection[] $connections
      */
     public function setConnections(array $connections)
     {
-        $this->connections = $connections;
+        $this->_connections = $connections;
     }
+    
     /**
-     * 
      * @return boolean
      */
     public function hasConnection()
     {
-        foreach ($this->connections as $connection) {
+        foreach ($this->_connections as $connection) {
             if ($connection->isEnabled()) {
                 return true;
             }
@@ -71,25 +78,25 @@ public function hasConnection()
         
         return false;
     }
+    
     /**
-     * 
      * @return array
      */
     public function getConnections()
     {
-        return $this->connections;
+        return $this->_connections;
     }
+    
     /**
-     * 
-     * @return Connection
+     * @return \Elastica\Connection
      */
     public function getConnection()
     {
-        return $this->strategy->getConnection($this->getConnections());
+        return $this->_strategy->getConnection($this->getConnections());
     }
+    
     /**
-     * 
-     * @param Connection $connection
+     * @param \Elastica\Connection $connection
      * @param Exception $e
      * @param Client $client
      */
@@ -97,8 +104,17 @@ public function onFail(Connection $connection, Exception $e, Client $client)
     {
         $connection->setEnabled(false);
         
-        if ($this->callback) {
-            call_user_func($this->callback, $connection, $e, $client);
+        if ($this->_callback) {
+            call_user_func($this->_callback, $connection, $e, $client);
         }
     }
+    
+    /**
+     * 
+     * @return \Elastica\Connection\Strategy\StrategyInterface
+     */
+    public function getStrategy()
+    {
+        return $this->_strategy;
+    }
 }
diff --git a/lib/Elastica/Connection/Strategy/CallbackStrategy.php b/lib/Elastica/Connection/Strategy/CallbackStrategy.php
index 5692bfd2b7..66bcff9f60 100644
--- a/lib/Elastica/Connection/Strategy/CallbackStrategy.php
+++ b/lib/Elastica/Connection/Strategy/CallbackStrategy.php
@@ -9,13 +9,13 @@
  */
 class CallbackStrategy implements StrategyInterface
 {
+    
     /**
-     *
      * @var Closure 
      */
-    protected $callback;
+    protected $_callback;
+    
     /**
-     * 
      * @param Closure $callback
      * @throws \InvalidArgumentException
      */
@@ -25,19 +25,19 @@ public function __construct($callback)
             throw new \InvalidArgumentException(sprintf('Callback should be a Closure, %s given!', gettype($callback)));
         }
         
-        $this->callback = $callback;
+        $this->_callback = $callback;
     }
+    
     /**
-     * 
      * @param array|\Elastica\Connection[] $connections
      * @return \Elastica\Connection
      */
     public function getConnection($connections)
     {
-        return $this->callback->__invoke($connections);
+        return $this->_callback->__invoke($connections);
     }
+    
     /**
-     * 
      * @return boolean
      */
     public static function isValid($callback)
diff --git a/lib/Elastica/Connection/Strategy/RoundRobin.php b/lib/Elastica/Connection/Strategy/RoundRobin.php
index 657010cf5f..713c9cd457 100644
--- a/lib/Elastica/Connection/Strategy/RoundRobin.php
+++ b/lib/Elastica/Connection/Strategy/RoundRobin.php
@@ -9,8 +9,8 @@
  */
 class RoundRobin extends Simple
 {
+    
     /**
-     * 
      * @param array|\Elastica\Connection[] $connections
      * @return \Elastica\Connection
      */
diff --git a/lib/Elastica/Connection/Strategy/Simple.php b/lib/Elastica/Connection/Strategy/Simple.php
index e2d0c75e97..c7986b46a5 100644
--- a/lib/Elastica/Connection/Strategy/Simple.php
+++ b/lib/Elastica/Connection/Strategy/Simple.php
@@ -11,8 +11,8 @@
  */
 class Simple implements StrategyInterface
 {
+    
     /**
-     * 
      * @param array|\Elastica\Connection[] $connections
      * @return \Elastica\Connection
      */
diff --git a/lib/Elastica/Connection/Strategy/StrategyFactory.php b/lib/Elastica/Connection/Strategy/StrategyFactory.php
index 43453594e1..e56e70e575 100644
--- a/lib/Elastica/Connection/Strategy/StrategyFactory.php
+++ b/lib/Elastica/Connection/Strategy/StrategyFactory.php
@@ -10,7 +10,6 @@
 class StrategyFactory
 {
     /**
-     * 
      * @param mixed|Closure|String|StrategyInterface $strategyName
      * @return \Elastica\Connection\Strategy\StrategyInterface
      * @throws \InvalidArgumentException
@@ -37,12 +36,4 @@ public static function create($strategyName)
         
         return $strategy;
     }
-    /**
-     * 
-     * @return \Elastica\Connection\Strategy\Simple
-     */
-    public static function getSimpleStrategy()
-    {
-        return new Simple();
-    }
 }
diff --git a/lib/Elastica/Connection/Strategy/StrategyInterface.php b/lib/Elastica/Connection/Strategy/StrategyInterface.php
index 3b8e23e2dd..1ece1049ca 100644
--- a/lib/Elastica/Connection/Strategy/StrategyInterface.php
+++ b/lib/Elastica/Connection/Strategy/StrategyInterface.php
@@ -10,7 +10,6 @@
 interface StrategyInterface
 {
     /**
-     * 
      * @param array|\Elastica\Connection[] $connections
      * @return \Elastica\Connection
      */
diff --git a/test/lib/Elastica/Test/Connection/ConnectionPollTest.php b/test/lib/Elastica/Test/Connection/ConnectionPollTest.php
index 0daa4d0d82..81c250ca50 100644
--- a/test/lib/Elastica/Test/Connection/ConnectionPollTest.php
+++ b/test/lib/Elastica/Test/Connection/ConnectionPollTest.php
@@ -73,10 +73,9 @@ protected function getConnections($quantity = 1)
     protected function createPool()
     {
         $connections = $this->getConnections();
-        $strategy = \Elastica\Connection\Strategy\StrategyFactory::getSimpleStrategy();
+        $strategy = \Elastica\Connection\Strategy\StrategyFactory::create('Simple');
         
         $pool = new \Elastica\Connection\ConnectionPool($connections, $strategy);
-        $pool->getConnections();
         
         return $pool;
     }
diff --git a/test/lib/Elastica/Test/Connection/Strategy/CallbackStrategyTest.php b/test/lib/Elastica/Test/Connection/Strategy/CallbackStrategyTest.php
index 42bb2fd149..7f5aa84d49 100644
--- a/test/lib/Elastica/Test/Connection/Strategy/CallbackStrategyTest.php
+++ b/test/lib/Elastica/Test/Connection/Strategy/CallbackStrategyTest.php
@@ -54,8 +54,16 @@ public function testConnection()
        });
        
        $client = new \Elastica\Client($config);
-       $client->request('/_aliases');
+       $resonse = $client->request('/_aliases');
        
        $this->assertEquals(1, $count);
+       
+       $this->assertTrue($resonse->isOk());
+       
+       $strategy = $client->getConnectionStrategy();
+       
+       $condition = ($strategy instanceof CallbackStrategy);
+       
+       $this->assertTrue($condition);
     }
 }
diff --git a/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php b/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php
index baba920aac..5d43bcc4d8 100644
--- a/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php
+++ b/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php
@@ -2,6 +2,10 @@
 
 namespace Elastica\Test\Connection\Strategy;
 
+use Elastica\Client;
+use Elastica\Connection\Strategy\RoundRobin;
+use Elastica\Exception\ConnectionException;
+use Elastica\Response;
 use Elastica\Test\Base;
 
 /**
@@ -11,10 +15,102 @@
  */
 class RoundRobinTest extends Base
 {
-   public function testConnection()
-   {
-       $config = array('connectionStrategy' => 'RoundRobin');
-       $client = new \Elastica\Client($config);
-       $client->request('/_aliases');
-   }
+
+    public function testConnection()
+    {
+        $config = array('connectionStrategy' => 'RoundRobin');
+        $client = new Client($config);
+        $resonse = $client->request('/_aliases');
+        /* @var $resonse Response */
+
+        $this->_checkResponse($resonse);
+
+        $this->_checkStrategy($client);
+    }
+
+    public function testOldStrategySetted()
+    {
+        $config = array('roundRobin' => true);
+        $client = new Client($config);
+
+        $this->_checkStrategy($client);
+    }
+
+    /**
+     * @expectedException \Elastica\Exception\ConnectionException
+     */
+    public function testFailConnection()
+    {
+        $config = array('connectionStrategy' => 'RoundRobin', 'host' => '255.255.255.0');
+        $client = new Client($config);
+
+        $client->request('/_aliases');
+        
+    }
+    
+    public function testWithOneFailConnection()
+    {
+        $connections = array(
+            new \Elastica\Connection(array('host' => '255.255.255.0')),
+            new \Elastica\Connection(array('host' => 'localhost')),
+        );
+        
+        $count = 0;
+        $callback = function($connection, $exception, $client) use(&$count) {
+            ++$count;
+        };
+        
+        $client = new Client(array('connectionStrategy' => 'RoundRobin'), $callback);
+        $client->setConnections($connections);
+        
+        $resonse = $client->request('/_aliases');
+        /* @var $resonse Response */
+
+        $this->_checkResponse($resonse);
+        
+        $this->_checkStrategy($client);
+        
+        
+        $this->assertLessThan(count($connections), $count);
+    }
+    
+    public function testWithNoValidConnection()
+    {
+        $connections = array(
+            new \Elastica\Connection(array('host' => '255.255.255.0', 'timeout' => 2)),
+            new \Elastica\Connection(array('host' => '45.45.45.45', 'port' => '80', 'timeout' => 2)),
+            new \Elastica\Connection(array('host' => '10.123.213.123', 'timeout' => 2)),
+        );
+
+        $count = 0;
+        $client = new Client(array('roundRobin' => true), function() use (&$count) {
+            ++$count;
+        });
+        
+        $client->setConnections($connections);
+
+        try {
+            $client->request('/_aliases');
+            $this->fail('Should throw exception as no connection valid');
+        } catch (\Elastica\Exception\ConnectionException $e) {
+            $this->assertEquals(count($connections), $count);
+            $this->_checkStrategy($client);
+        }
+
+    }
+    
+    protected function _checkStrategy($client)
+    {
+        $strategy = $client->getConnectionStrategy();
+
+        $condition = ($strategy instanceof RoundRobin);
+
+        $this->assertTrue($condition);
+    }
+    
+    protected function _checkResponse($resonse)
+    {
+        $this->assertTrue($resonse->isOk());
+    }
+
 }
diff --git a/test/lib/Elastica/Test/Connection/Strategy/SimpleTest.php b/test/lib/Elastica/Test/Connection/Strategy/SimpleTest.php
new file mode 100644
index 0000000000..0069d1a324
--- /dev/null
+++ b/test/lib/Elastica/Test/Connection/Strategy/SimpleTest.php
@@ -0,0 +1,102 @@
+<?php
+
+namespace Elastica\Test\Connection\Strategy;
+
+use Elastica\Client;
+use Elastica\Connection\Strategy\Simple;
+use Elastica\Test\Base;
+
+/**
+ * Description of SimplyTest
+ *
+ * @author chabior
+ */
+class SimpleTest extends Base
+{
+    public function testConnection()
+    {
+        $client = new Client();
+        $resonse = $client->request('/_aliases');
+        /* @var $resonse \Elastica\Response */
+       
+        $this->_checkResponse($resonse);
+        
+        $this->_checkStrategy($client);
+    }
+    
+    /**
+     * @expectedException \Elastica\Exception\ConnectionException
+     */
+    public function testFailConnection()
+    {
+        $config = array('host' => '255.255.255.0');
+        $client = new Client($config);
+
+        $client->request('/_aliases');
+        
+    }
+    
+    public function testWithOneFailConnection()
+    {
+        $connections = array(
+            new \Elastica\Connection(array('host' => '255.255.255.0')),
+            new \Elastica\Connection(array('host' => 'localhost')),
+        );
+        
+        $count = 0;
+        $callback = function($connection, $exception, $client) use(&$count) {
+            ++$count;
+        };
+        
+        $client = new Client(array(), $callback);
+        $client->setConnections($connections);
+        
+        $resonse = $client->request('/_aliases');
+        /* @var $resonse Response */
+
+        $this->_checkResponse($resonse);
+        
+        $this->_checkStrategy($client);
+        
+        
+        $this->assertLessThan(count($connections), $count);
+    }
+    
+    public function testWithNoValidConnection()
+    {
+        $connections = array(
+            new \Elastica\Connection(array('host' => '255.255.255.0', 'timeout' => 2)),
+            new \Elastica\Connection(array('host' => '45.45.45.45', 'port' => '80', 'timeout' => 2)),
+            new \Elastica\Connection(array('host' => '10.123.213.123', 'timeout' => 2)),
+        );
+
+        $count = 0;
+        $client = new Client(array(), function() use (&$count) {
+            ++$count;
+        });
+        
+        $client->setConnections($connections);
+
+        try {
+            $client->request('/_aliases');
+            $this->fail('Should throw exception as no connection valid');
+        } catch (\Elastica\Exception\ConnectionException $e) {
+            $this->assertEquals(count($connections), $count);
+        }
+
+    }
+    
+    protected function _checkStrategy($client)
+    {
+        $strategy = $client->getConnectionStrategy();
+
+        $condition = ($strategy instanceof Simple);
+
+        $this->assertTrue($condition);
+    }
+    
+    protected function _checkResponse($resonse)
+    {
+        $this->assertTrue($resonse->isOk());
+    }
+}
diff --git a/test/lib/Elastica/Test/Connection/Strategy/SimplyTest.php b/test/lib/Elastica/Test/Connection/Strategy/SimplyTest.php
deleted file mode 100644
index 2f0472f8fc..0000000000
--- a/test/lib/Elastica/Test/Connection/Strategy/SimplyTest.php
+++ /dev/null
@@ -1,19 +0,0 @@
-<?php
-
-namespace Elastica\Test\Connection\Strategy;
-
-use Elastica\Test\Base;
-
-/**
- * Description of SimplyTest
- *
- * @author chabior
- */
-class SimplyTest extends Base
-{
-    public function testConnection()
-    {
-        $client = new \Elastica\Client();
-        $client->request('/_aliases');
-    }
-}
diff --git a/test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php b/test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php
index b0a89a2f8e..bab7fab8d4 100644
--- a/test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php
+++ b/test/lib/Elastica/Test/Connection/Strategy/StrategyFactoryTest.php
@@ -20,13 +20,6 @@
  */
 class StrategyFactoryTest extends Base
 {
-   public function testGetSimpleStrategy()
-   {
-       $condition = (StrategyFactory::getSimpleStrategy() instanceof Simple);
-       
-       $this->assertTrue($condition);
-   }
-   
    public function testCreateCallbackStrategy()
    {
        $callback = function ($connections)

From 93ef291e52699cc07551b77903cefa861bf2de1c Mon Sep 17 00:00:00 2001
From: chabior <chabior@o2.pl>
Date: Thu, 7 Aug 2014 10:05:46 +0100
Subject: [PATCH 4/5] add elastica exception

---
 lib/Elastica/Connection/ConnectionPool.php            | 5 +++--
 lib/Elastica/Connection/Strategy/CallbackStrategy.php | 6 ++++--
 lib/Elastica/Connection/Strategy/RoundRobin.php       | 1 +
 lib/Elastica/Connection/Strategy/Simple.php           | 2 ++
 lib/Elastica/Connection/Strategy/StrategyFactory.php  | 6 ++++--
 5 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/lib/Elastica/Connection/ConnectionPool.php b/lib/Elastica/Connection/ConnectionPool.php
index 9df6fb9c7b..66b2adaa22 100644
--- a/lib/Elastica/Connection/ConnectionPool.php
+++ b/lib/Elastica/Connection/ConnectionPool.php
@@ -38,7 +38,7 @@ class ConnectionPool
     /**
      * @param array $connections
      * @param \Elastica\Connection\Strategy\StrategyInterface $strategy
-     * @param type $callback
+     * @param callback $callback
      */
     public function __construct(array $connections, StrategyInterface $strategy, $callback = null)
     {
@@ -89,6 +89,7 @@ public function getConnections()
     
     /**
      * @return \Elastica\Connection
+     * @throws \Elastica\Exception\ClientException
      */
     public function getConnection()
     {
@@ -97,7 +98,7 @@ public function getConnection()
     
     /**
      * @param \Elastica\Connection $connection
-     * @param Exception $e
+     * @param \Exception $e
      * @param Client $client
      */
     public function onFail(Connection $connection, Exception $e, Client $client)
diff --git a/lib/Elastica/Connection/Strategy/CallbackStrategy.php b/lib/Elastica/Connection/Strategy/CallbackStrategy.php
index 66bcff9f60..7ea7cf9eae 100644
--- a/lib/Elastica/Connection/Strategy/CallbackStrategy.php
+++ b/lib/Elastica/Connection/Strategy/CallbackStrategy.php
@@ -2,6 +2,8 @@
 
 namespace Elastica\Connection\Strategy;
 
+use Elastica\Exception\InvalidException;
+
 /**
  * Description of CallbackStrategy
  *
@@ -17,12 +19,12 @@ class CallbackStrategy implements StrategyInterface
     
     /**
      * @param Closure $callback
-     * @throws \InvalidArgumentException
+     * @throws \Elastica\Exception\InvalidException
      */
     public function __construct($callback)
     {
         if (!self::isValid($callback)) {
-            throw new \InvalidArgumentException(sprintf('Callback should be a Closure, %s given!', gettype($callback)));
+            throw new InvalidException(sprintf('Callback should be a Closure, %s given!', gettype($callback)));
         }
         
         $this->_callback = $callback;
diff --git a/lib/Elastica/Connection/Strategy/RoundRobin.php b/lib/Elastica/Connection/Strategy/RoundRobin.php
index 713c9cd457..9cbf8c86e9 100644
--- a/lib/Elastica/Connection/Strategy/RoundRobin.php
+++ b/lib/Elastica/Connection/Strategy/RoundRobin.php
@@ -13,6 +13,7 @@ class RoundRobin extends Simple
     /**
      * @param array|\Elastica\Connection[] $connections
      * @return \Elastica\Connection
+     * @throws \Elastica\Exception\ClientException
      */
     public function getConnection($connections)
     {
diff --git a/lib/Elastica/Connection/Strategy/Simple.php b/lib/Elastica/Connection/Strategy/Simple.php
index c7986b46a5..f2b182a99e 100644
--- a/lib/Elastica/Connection/Strategy/Simple.php
+++ b/lib/Elastica/Connection/Strategy/Simple.php
@@ -3,6 +3,7 @@
 namespace Elastica\Connection\Strategy;
 
 use Elastica\Connection\Strategy\StrategyInterface;
+use Elastica\Exception\ClientException;
 
 /**
  * Description of SimpleStrategy
@@ -15,6 +16,7 @@ class Simple implements StrategyInterface
     /**
      * @param array|\Elastica\Connection[] $connections
      * @return \Elastica\Connection
+     * @throws \Elastica\Exception\ClientException
      */
     public function getConnection($connections)
     {
diff --git a/lib/Elastica/Connection/Strategy/StrategyFactory.php b/lib/Elastica/Connection/Strategy/StrategyFactory.php
index e56e70e575..fbdb44e664 100644
--- a/lib/Elastica/Connection/Strategy/StrategyFactory.php
+++ b/lib/Elastica/Connection/Strategy/StrategyFactory.php
@@ -2,6 +2,8 @@
 
 namespace Elastica\Connection\Strategy;
 
+use Elastica\Exception\InvalidException;
+
 /**
  * Description of StrategyFactory
  *
@@ -12,7 +14,7 @@ class StrategyFactory
     /**
      * @param mixed|Closure|String|StrategyInterface $strategyName
      * @return \Elastica\Connection\Strategy\StrategyInterface
-     * @throws \InvalidArgumentException
+     * @throws \Elastica\Exception\InvalidException
      */
     public static function create($strategyName)
     {
@@ -31,7 +33,7 @@ public static function create($strategyName)
         }
         
         if (!$strategy instanceof StrategyInterface) {
-            throw new \InvalidArgumentException('Can\'t load strategy class');
+            throw new InvalidException('Can\'t load strategy class');
         }
         
         return $strategy;

From 1a810c484ecd4a1189157550fa4b20edfde3c9b2 Mon Sep 17 00:00:00 2001
From: chabior <chabior@o2.pl>
Date: Thu, 7 Aug 2014 10:13:13 +0100
Subject: [PATCH 5/5] minor changes

---
 test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php | 4 +++-
 test/lib/Elastica/Test/Connection/Strategy/SimpleTest.php     | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php b/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php
index 5d43bcc4d8..1bb36029bf 100644
--- a/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php
+++ b/test/lib/Elastica/Test/Connection/Strategy/RoundRobinTest.php
@@ -43,7 +43,9 @@ public function testFailConnection()
     {
         $config = array('connectionStrategy' => 'RoundRobin', 'host' => '255.255.255.0');
         $client = new Client($config);
-
+        
+        $this->_checkStrategy($client);
+        
         $client->request('/_aliases');
         
     }
diff --git a/test/lib/Elastica/Test/Connection/Strategy/SimpleTest.php b/test/lib/Elastica/Test/Connection/Strategy/SimpleTest.php
index 0069d1a324..bd22f82faa 100644
--- a/test/lib/Elastica/Test/Connection/Strategy/SimpleTest.php
+++ b/test/lib/Elastica/Test/Connection/Strategy/SimpleTest.php
@@ -31,6 +31,8 @@ public function testFailConnection()
     {
         $config = array('host' => '255.255.255.0');
         $client = new Client($config);
+        
+        $this->_checkStrategy($client);
 
         $client->request('/_aliases');