Promote multibulk response iterators as Predis response objects.

Multibulk response iterators will not be passed anymore to the response
parser method of the command that generated the response. Pipeline and
transaction abstractions still consume interators returned as response
items.

CHANGELOG.md

@@ -32,6 +32,11 @@ v0.8.0 (201x-xx-xx)
   `-NOSCRIPT` error. Automatic fallback to `EVAL` does not work with pipelines,
   inside a MULTI / EXEC context or with plain `EVALSHA` commands.
 
+- Iterators for multi-bulk replies now skip the response parsing method of the
+  command that generated the response and are passed directly to user code.
+  Pipeline and transaction objects still consume automatically the iterators
+  that are present in response elements.
+
 - Cluster and replication connections now extend a new common interface,
   `Predis\Connection\AggregatedConnectionInterface`.
 

lib/Predis/Command/HashGetAll.php

@@ -11,8 +11,6 @@
 
 namespace Predis\Command;
 
-use Predis\Iterator\MultiBulkResponseTuple;
-
 /**
  * @link http://redis.io/commands/hgetall
  * @author Daniele Alessandri <suppakilla@gmail.com>
@@ -32,10 +30,6 @@ class HashGetAll extends PrefixableCommand
      */
     public function parseResponse($data)
     {
-        if ($data instanceof \Iterator) {
-            return new MultiBulkResponseTuple($data);
-        }
-
         $result = array();
 
         for ($i = 0; $i < count($data); $i++) {

lib/Predis/Command/ServerConfig.php

@@ -11,8 +11,6 @@
 
 namespace Predis\Command;
 
-use Predis\Iterator\MultiBulkResponseTuple;
-
 /**
  * @link http://redis.io/commands/config-set
  * @link http://redis.io/commands/config-get
@@ -34,10 +32,6 @@ class ServerConfig extends AbstractCommand
      */
     public function parseResponse($data)
     {
-        if ($data instanceof \Iterator) {
-            return new MultiBulkResponseTuple($data);
-        }
-
         if (is_array($data)) {
             $result = array();
 

lib/Predis/Command/ServerSlowlog.php

@@ -11,8 +11,6 @@
 
 namespace Predis\Command;
 
-use Predis\Iterator\MultiBulkResponse;
-
 /**
  * @link http://redis.io/commands/slowlog
  * @author Daniele Alessandri <suppakilla@gmail.com>
@@ -32,28 +30,18 @@ class ServerSlowlog extends AbstractCommand
      */
     public function parseResponse($data)
     {
-        if (($iterable = $data instanceof \Iterator) || is_array($data)) {
-            // NOTE: we consume iterable multibulk replies inplace since it is not
-            // possible to do anything fancy on sub-elements.
+        if (is_array($data)) {
             $log = array();
 
             foreach ($data as $index => $entry) {
-                if ($iterable) {
-                    $entry = iterator_to_array($entry);
-                }
-
                 $log[$index] = array(
                     'id' => $entry[0],
                     'timestamp' => $entry[1],
                     'duration' => $entry[2],
-                    'command' => $iterable ? iterator_to_array($entry[3]) : $entry[3],
+                    'command' => $entry[3],
                 );
             }
 
-            if ($iterable === true) {
-                unset($data);
-            }
-
             return $log;
         }
 

lib/Predis/Command/ServerTime.php

@@ -32,12 +32,4 @@ class ServerTime extends AbstractCommand
     {
         return false;
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function parseResponse($data)
-    {
-        return $data instanceof \Iterator ? iterator_to_array($data) : $data;
-    }
 }

lib/Predis/Command/ZSetRange.php

@@ -11,8 +11,6 @@
 
 namespace Predis\Command;
 
-use Predis\Iterator\MultiBulkResponseTuple;
-
 /**
  * @link http://redis.io/commands/zrange
  * @author Daniele Alessandri <suppakilla@gmail.com>
@@ -90,10 +88,6 @@ class ZSetRange extends PrefixableCommand
     public function parseResponse($data)
     {
         if ($this->withScores()) {
-            if ($data instanceof \Iterator) {
-                return new MultiBulkResponseTuple($data);
-            }
-
             $result = array();
 
             for ($i = 0; $i < count($data); $i++) {

lib/Predis/Iterator/MultiBulkResponse.php

@@ -11,13 +11,15 @@
 
 namespace Predis\Iterator;
 
+use Predis\ResponseObjectInterface;
+
 /**
  * Iterator that abstracts the access to multibulk replies and allows
  * them to be consumed by user's code in a streaming fashion.
  *
  * @author Daniele Alessandri <suppakilla@gmail.com>
  */
-abstract class MultiBulkResponse implements \Iterator, \Countable
+abstract class MultiBulkResponse implements \Iterator, \Countable, ResponseObjectInterface
 {
     protected $position;
     protected $current;

lib/Predis/Iterator/MultiBulkResponseSimple.php

@@ -75,4 +75,14 @@ class MultiBulkResponseSimple extends MultiBulkResponse
     {
         return $this->connection->read();
     }
+
+    /**
+     * Returns an iterator that reads the multi-bulk response as a tuple.
+     *
+     * @return MultiBulkResponseTuple
+     */
+    public function asTuple()
+    {
+        return new MultiBulkResponseTuple($this);
+    }
 }

lib/Predis/Iterator/MultiBulkResponseTuple.php

@@ -26,6 +26,8 @@ class MultiBulkResponseTuple extends MultiBulkResponse implements \OuterIterator
      */
     public function __construct(MultiBulkResponseSimple $iterator)
     {
+        $this->checkPreconditions($iterator);
+
         $virtualSize = count($iterator) / 2;
         $this->iterator = $iterator;
         $this->position = $iterator->getPosition();
@@ -33,6 +35,22 @@ class MultiBulkResponseTuple extends MultiBulkResponse implements \OuterIterator
         $this->replySize = $virtualSize;
     }
 
+    /**
+     * Checks for valid preconditions.
+     *
+     * @param MultiBulkResponseSimple $iterator Multibulk reply iterator.
+     */
+    protected function checkPreconditions(MultiBulkResponseSimple $iterator)
+    {
+        if ($iterator->getPosition() !== 0) {
+            throw new \RuntimeException('Cannot initialize a tuple iterator with an already initiated iterator');
+        }
+
+        if (($size = count($iterator)) % 2 !== 0) {
+            throw new \UnexpectedValueException("Invalid reply size for a tuple iterator [$size]");
+        }
+    }
+
     /**
      * {@inheritdoc}
      */

tests/Predis/Iterator/MultiBulkResponseSimpleTest.php

@@ -48,6 +48,18 @@ class MultiBulkResponseSimpleTest extends StandardTestCase
         $this->assertTrue($client->ping());
     }
 
+    /**
+     * @group connected
+     */
+    public function testIterableMultibulkCanBeWrappedAsTupleIterator()
+    {
+        $client = $this->getClient();
+        $client->mset('foo', 'bar', 'hoge', 'piyo');
+
+        $this->assertInstanceOf('Predis\Iterator\MultiBulkResponseSimple', $iterator = $client->mget('foo', 'bar'));
+        $this->assertInstanceOf('Predis\Iterator\MultiBulkResponseTuple', $iterator->asTuple());
+    }
+
     /**
      * @group connected
      */

tests/Predis/Iterator/MultiBulkResponseTupleTest.php

@@ -20,6 +20,33 @@ use Predis\Client;
  */
 class MultiBulkResponseTupleTest extends StandardTestCase
 {
+    /**
+     * @group disconnected
+     * @expectedException RuntimeException
+     * @expectedExceptionMessage Cannot initialize a tuple iterator with an already initiated iterator
+     */
+    public function testInitiatedMultiBulkIteratorsAreNotValid()
+    {
+        $connection = $this->getMock('Predis\Connection\SingleConnectionInterface');
+        $iterator = new MultiBulkResponseSimple($connection, 2);
+        $iterator->next();
+
+        new MultiBulkResponseTuple($iterator);
+    }
+
+    /**
+     * @group disconnected
+     * @expectedException UnexpectedValueException
+     * @expectedExceptionMessage Invalid reply size for a tuple iterator [3]
+     */
+    public function testMultiBulkWithOddSizesAreInvalid()
+    {
+        $connection = $this->getMock('Predis\Connection\SingleConnectionInterface');
+        $iterator = new MultiBulkResponseSimple($connection, 3);
+
+        new MultiBulkResponseTuple($iterator);
+    }
+
     /**
      * @group connected
      */
@@ -28,7 +55,7 @@ class MultiBulkResponseTupleTest extends StandardTestCase
         $client = $this->getClient();
         $client->zadd('metavars', 1, 'foo', 2, 'hoge', 3, 'lol');
 
-        $this->assertInstanceOf('OuterIterator', $iterator = $client->zrange('metavars', 0, -1, 'withscores'));
+        $this->assertInstanceOf('OuterIterator', $iterator = $client->zrange('metavars', 0, -1, 'withscores')->asTuple());
         $this->assertInstanceOf('Predis\Iterator\MultiBulkResponseTuple', $iterator);
         $this->assertInstanceOf('Predis\Iterator\MultiBulkResponseSimple', $iterator->getInnerIterator());
         $this->assertTrue($iterator->valid());
@@ -57,7 +84,7 @@ class MultiBulkResponseTupleTest extends StandardTestCase
         $client = $this->getClient();
         $client->zadd('metavars', 1, 'foo', 2, 'hoge', 3, 'lol');
 
-        $iterator = $client->zrange('metavars', 0, -1, 'withscores');
+        $iterator = $client->zrange('metavars', 0, -1, 'withscores')->asTuple();
 
         unset($iterator);