Iterate only over connections mapped in slots map.

Iterating over Predis\Connection\Aggregate\RedisCluster returns all
the connections currently mapped in the slots map instead of just the
ones initialized in the pool.

When the slots map is retrieved from Redis (which by default is done
automatically) this allows to iterate over all of the current master
nodes of the cluster. When the underlying use of "CLUSTER SLOTS" is
disabled the iteration returns only connections with a slots range
associated in their parameters or initialized by `-MOVED` responses
to make the behaviour of the iteration consistent between the two
modes of operation.

CHANGELOG.md

@@ -39,6 +39,16 @@ v1.1.0 (2016-0x-xx)
   use the `tls` or `rediss` schemes in connection parameters along with specific
   options via the `ssl` parameter (see http://php.net/manual/context.ssl.php).
 
+- Iterating over `Predis\Connection\Aggregate\RedisCluster` now returns all the
+  connections currently mapped in the slots map instead of just the connections
+  initialized in the pool. When the slots map is retrieved from Redis (which is
+  done automatically by default) this allows to iterate over all of the current
+  master nodes of the cluster. When the use of `CLUSTER SLOTS` is disabled (see
+  the `useClusterSlots()` method) the iteration returns only connections with a
+  a slots range associated in their parameters (the ones supplied when creating
+  a client instance) or the ones initialized by `-MOVED` responses to make the
+  behaviour of the iteration consistent between the two modes of operation.
+
 - Various improvements to `Predis\Connection\Aggregate\MasterSlaveReplication`
   (the default replication backend that does not rely on redis-sentinel):
 

src/Connection/Aggregate/RedisCluster.php

@@ -171,6 +171,8 @@ class RedisCluster implements ClusterInterface, \IteratorAggregate, \Countable
      * cluster, so it is most effective when all of the connections supplied on
      * initialization have the "slots" parameter properly set accordingly to the
      * current cluster configuration.
+     *
+     * @return array
      */
     public function buildSlotsMap()
     {
@@ -193,6 +195,8 @@ class RedisCluster implements ClusterInterface, \IteratorAggregate, \Countable
                 $this->setSlots($slots[0], $slots[1], $connectionID);
             }
         }
+
+        return $this->slotsMap;
     }
 
     /**
@@ -608,7 +612,23 @@ class RedisCluster implements ClusterInterface, \IteratorAggregate, \Countable
      */
     public function getIterator()
     {
-        return new \ArrayIterator(array_values($this->pool));
+        if ($this->useClusterSlots) {
+            $slotsmap = $this->getSlotsMap() ?: $this->askSlotsMap();
+        } else {
+            $slotsmap = $this->getSlotsMap() ?: $this->buildSlotsMap();
+        }
+
+        $connections = array();
+
+        foreach (array_unique($slotsmap) as $node) {
+            if (!$connection = $this->getConnectionById($node)) {
+                $this->add($connection = $this->createConnection($node));
+            }
+
+            $connections[] = $connection;
+        }
+
+        return new \ArrayIterator($connections);
     }
 
     /**

tests/Predis/Connection/Aggregate/RedisClusterTest.php

@@ -242,20 +242,82 @@ class RedisClusterTest extends PredisTestCase
     /**
      * @group disconnected
      */
-    public function testCanReturnAnIteratorForConnections()
+    public function testGetIteratorReturnsConnectionsMappedInSlotsMapWhenUseClusterSlotsIsDisabled()
     {
-        $connection1 = $this->getMockConnection('tcp://127.0.0.1:6379');
-        $connection2 = $this->getMockConnection('tcp://127.0.0.1:6380');
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:6381?slots=0-5460');
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:6382?slots=5461-10921');
+        $connection3 = $this->getMockConnection('tcp://127.0.0.1:6383?slots=10922-16383');
+        $connection4 = $this->getMockConnection('tcp://127.0.0.1:6384');
 
         $cluster = new RedisCluster(new Connection\Factory());
+
+        $cluster->useClusterSlots(false);
+
         $cluster->add($connection1);
         $cluster->add($connection2);
+        $cluster->add($connection3);
+        $cluster->add($connection4);
 
         $this->assertInstanceOf('Iterator', $iterator = $cluster->getIterator());
         $connections = iterator_to_array($iterator);
 
+        $this->assertCount(3, $connections);
         $this->assertSame($connection1, $connections[0]);
         $this->assertSame($connection2, $connections[1]);
+        $this->assertSame($connection3, $connections[2]);
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testGetIteratorReturnsConnectionsMappedInSlotsMapFetchedFromRedisCluster()
+    {
+        $slotsmap = array(
+            array(0, 5460, array('127.0.0.1', 6381), array()),
+            array(5461, 10921, array('127.0.0.1', 6383), array()),
+            array(10922, 16383, array('127.0.0.1', 6384), array()),
+        );
+
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:6381?slots=0-5460');
+        $connection1->expects($this->once())
+                    ->method('executeCommand')
+                    ->with($this->isRedisCommand(
+                        'CLUSTER', array('SLOTS')
+                    ))
+                    ->will($this->returnValue($slotsmap));
+
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:6382?slots=5461-10921');
+        $connection3 = $this->getMockConnection('tcp://127.0.0.1:6383');
+        $connection4 = $this->getMockConnection('tcp://127.0.0.1:6384');
+
+        $factory = $this->getMock('Predis\Connection\FactoryInterface');
+        $factory->expects($this->at(0))
+                 ->method('create')
+                 ->with(array('host' => '127.0.0.1', 'port' => '6383'))
+                 ->will($this->returnValue($connection3));
+        $factory->expects($this->at(1))
+                 ->method('create')
+                 ->with(array('host' => '127.0.0.1', 'port' => '6384'))
+                 ->will($this->returnValue($connection4));
+
+        // TODO: I'm not sure about mocking a protected method, but it'll do for now
+        $cluster = $this->getMock('Predis\Connection\Aggregate\RedisCluster', array('getRandomConnection'), array($factory));
+        $cluster->expects($this->exactly(1))
+                ->method('getRandomConnection')
+                ->will($this->returnValue($connection1));
+
+        $cluster->add($connection1);
+        $cluster->add($connection2);
+
+        $cluster->useClusterSlots(true);
+
+        $this->assertInstanceOf('Iterator', $iterator = $cluster->getIterator());
+        $connections = iterator_to_array($iterator);
+
+        $this->assertCount(3, $connections);
+        $this->assertSame($connection1, $connections[0]);
+        $this->assertSame($connection3, $connections[1]);
+        $this->assertSame($connection4, $connections[2]);
     }
 
     /**