diff --git a/CHANGELOG.md b/CHANGELOG.md index 085c8f8a..395d6e59 100644 --- a/CHANGELOG.md +++ b/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`. diff --git a/lib/Predis/Command/HashGetAll.php b/lib/Predis/Command/HashGetAll.php index 9fa7f7a1..51a745f7 100644 --- a/lib/Predis/Command/HashGetAll.php +++ b/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 @@ -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++) { diff --git a/lib/Predis/Command/ServerConfig.php b/lib/Predis/Command/ServerConfig.php index 77a20a37..251efdd8 100644 --- a/lib/Predis/Command/ServerConfig.php +++ b/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(); diff --git a/lib/Predis/Command/ServerSlowlog.php b/lib/Predis/Command/ServerSlowlog.php index 2520cc92..ad4155f2 100644 --- a/lib/Predis/Command/ServerSlowlog.php +++ b/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 @@ -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; } diff --git a/lib/Predis/Command/ServerTime.php b/lib/Predis/Command/ServerTime.php index 924f91ba..542179a0 100644 --- a/lib/Predis/Command/ServerTime.php +++ b/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; - } } diff --git a/lib/Predis/Command/ZSetRange.php b/lib/Predis/Command/ZSetRange.php index d2a4f79f..61a7bfcc 100644 --- a/lib/Predis/Command/ZSetRange.php +++ b/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 @@ -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++) { diff --git a/lib/Predis/Iterator/MultiBulkResponse.php b/lib/Predis/Iterator/MultiBulkResponse.php index 138794d4..a7bbad81 100644 --- a/lib/Predis/Iterator/MultiBulkResponse.php +++ b/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 */ -abstract class MultiBulkResponse implements \Iterator, \Countable +abstract class MultiBulkResponse implements \Iterator, \Countable, ResponseObjectInterface { protected $position; protected $current; diff --git a/lib/Predis/Iterator/MultiBulkResponseSimple.php b/lib/Predis/Iterator/MultiBulkResponseSimple.php index b3448376..68864b18 100644 --- a/lib/Predis/Iterator/MultiBulkResponseSimple.php +++ b/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); + } } diff --git a/lib/Predis/Iterator/MultiBulkResponseTuple.php b/lib/Predis/Iterator/MultiBulkResponseTuple.php index 0c014846..39d7828f 100644 --- a/lib/Predis/Iterator/MultiBulkResponseTuple.php +++ b/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} */ diff --git a/tests/Predis/Iterator/MultiBulkResponseSimpleTest.php b/tests/Predis/Iterator/MultiBulkResponseSimpleTest.php index 9c4a5fa9..3701fe80 100644 --- a/tests/Predis/Iterator/MultiBulkResponseSimpleTest.php +++ b/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 */ diff --git a/tests/Predis/Iterator/MultiBulkResponseTupleTest.php b/tests/Predis/Iterator/MultiBulkResponseTupleTest.php index 96659542..cb15392c 100644 --- a/tests/Predis/Iterator/MultiBulkResponseTupleTest.php +++ b/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);