Explorar o código

Fix standard pipeline executor not parsing raw replies.

This bug was actually introduced right before pushing the stable release
of v0.8.0 in which we moved the responsibility of parsing raw replies
with command parser to consumer classes.

This commit closes #101.
Daniele Alessandri %!s(int64=12) %!d(string=hai) anos
pai
achega
a5cf6d72cd

+ 7 - 0
CHANGELOG.md

@@ -1,3 +1,10 @@
+v0.8.2 (2013-xx-xx)
+===============================================================================
+
+- __FIX__: the standard pipeline executor was not using the response parser
+  methods associated to commands to process raw responses (ISSUE #101).
+
+
 v0.8.1 (2013-01-19)
 ===============================================================================
 

+ 33 - 6
lib/Predis/Pipeline/StandardExecutor.php

@@ -13,7 +13,9 @@ namespace Predis\Pipeline;
 
 use SplQueue;
 use Predis\ResponseErrorInterface;
+use Predis\ResponseObjectInterface;
 use Predis\ServerException;
+use Predis\Command\CommandInterface;
 use Predis\Connection\ConnectionInterface;
 use Predis\Connection\ReplicationConnectionInterface;
 
@@ -50,6 +52,27 @@ class StandardExecutor implements PipelineExecutorInterface
         }
     }
 
+    /**
+     * Handles a response object.
+     *
+     * @param ConnectionInterface $connection
+     * @param CommandInterface $command
+     * @param ResponseObjectInterface $response
+     * @return mixed
+     */
+    protected function onResponseObject(ConnectionInterface $connection, CommandInterface $command, ResponseObjectInterface $response)
+    {
+        if ($response instanceof ResponseErrorInterface) {
+            return $this->onResponseError($connection, $response);
+        }
+
+        if ($response instanceof \Iterator) {
+            return $command->parseResponse(iterator_to_array($response));
+        }
+
+        return $response;
+    }
+
     /**
      * Handles -ERR responses returned by Redis.
      *
@@ -58,6 +81,10 @@ class StandardExecutor implements PipelineExecutorInterface
      */
     protected function onResponseError(ConnectionInterface $connection, ResponseErrorInterface $response)
     {
+        if (!$this->exceptions) {
+            return $response;
+        }
+
         // Force disconnection to prevent protocol desynchronization.
         $connection->disconnect();
         $message = $response->getMessage();
@@ -72,7 +99,6 @@ class StandardExecutor implements PipelineExecutorInterface
     {
         $size = count($commands);
         $values = array();
-        $exceptions = $this->exceptions;
 
         $this->checkConnection($connection);
 
@@ -81,13 +107,14 @@ class StandardExecutor implements PipelineExecutorInterface
         }
 
         for ($i = 0; $i < $size; $i++) {
-            $response = $connection->readResponse($commands->dequeue());
+            $command = $commands->dequeue();
+            $response = $connection->readResponse($command);
 
-            if ($response instanceof ResponseErrorInterface && $exceptions === true) {
-                $this->onResponseError($connection, $response);
+            if ($response instanceof ResponseObjectInterface) {
+                $values[$i] = $this->onResponseObject($connection, $command, $response);
+            } else {
+                $values[$i] = $command->parseResponse($response);
             }
-
-            $values[$i] = $response instanceof \Iterator ? iterator_to_array($response) : $response;
         }
 
         return $values;

+ 53 - 2
tests/Predis/Pipeline/StandardExecutorTest.php

@@ -15,6 +15,7 @@ use \PHPUnit_Framework_TestCase as StandardTestCase;
 
 use SplQueue;
 use Predis\ResponseError;
+use Predis\ResponseObjectInterface;
 use Predis\Profile\ServerProfile;
 
 /**
@@ -40,7 +41,7 @@ class StandardExecutorTest extends StandardTestCase
         $replies = $executor->execute($connection, $pipeline);
 
         $this->assertTrue($pipeline->isEmpty());
-        $this->assertSame(array('PONG', 'PONG', 'PONG'), $replies);
+        $this->assertSame(array(true, true, true), $replies);
     }
 
     /**
@@ -64,7 +65,29 @@ class StandardExecutorTest extends StandardTestCase
         $replies = $executor->execute($connection, $pipeline);
 
         $this->assertTrue($pipeline->isEmpty());
-        $this->assertSame(array('PONG', 'PONG', 'PONG'), $replies);
+        $this->assertSame(array(true, true, true), $replies);
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testExecutorDoesNotParseResponseObjects()
+    {
+        $executor = new StandardExecutor();
+        $response = $this->getMock('Predis\ResponseObjectInterface');
+
+        $this->simpleResponseObjectTest($executor, $response);
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testExecutorCanReturnRedisErrors()
+    {
+        $executor = new StandardExecutor(false);
+        $response = $this->getMock('Predis\ResponseErrorInterface');
+
+        $this->simpleResponseObjectTest($executor, $response);
     }
 
     /**
@@ -90,6 +113,34 @@ class StandardExecutorTest extends StandardTestCase
     // ---- HELPER METHODS ------------------------------------------------ //
     // ******************************************************************** //
 
+    /**
+     * Executes a test for the Predis\ResponseObjectInterface type.
+     *
+     * @param PipelineExecutorInterface $executor
+     * @param ResponseObjectInterface $response
+     */
+    protected function simpleResponseObjectTest(PipelineExecutorInterface $executor, ResponseObjectInterface $response)
+    {
+        $pipeline = new SplQueue();
+
+        $command = $this->getMock('Predis\Command\CommandInterface');
+        $command->expects($this->never())
+                ->method('parseResponse');
+
+        $connection = $this->getMock('Predis\Connection\SingleConnectionInterface');
+        $connection->expects($this->once())
+                   ->method('writeCommand');
+        $connection->expects($this->once())
+                   ->method('readResponse')
+                   ->will($this->returnValue($response));
+
+        $pipeline->enqueue($command);
+        $replies = $executor->execute($connection, $pipeline);
+
+        $this->assertTrue($pipeline->isEmpty());
+        $this->assertSame(array($response), $replies);
+    }
+
     /**
      * Returns a list of queued command instances.
      *