Change signature of Predis\Pipeline\PipelineContext constructor.

Now the second argument is a Predis\Pipeline\PipelineExecutorInterface.

lib/Predis/Client.php

@@ -313,8 +313,16 @@ class Client implements ClientInterface
      */
     protected function initPipeline(Array $options = null, $callable = null)
     {
-        $pipeline = new PipelineContext($this, $options);
-        return $this->pipelineExecute($pipeline, $callable);
+        $executor = isset($options['executor']) ? $options['executor'] : null;
+
+        if (is_callable($executor)) {
+            $executor = call_user_func($executor, $this, $options);
+        }
+
+        $pipeline = new PipelineContext($this, $executor);
+        $replies  = $this->pipelineExecute($pipeline, $callable);
+
+        return $replies;
     }
 
     /**

lib/Predis/Pipeline/PipelineContext.php

@@ -35,13 +35,13 @@ class PipelineContext implements BasicClientInterface, ExecutableContextInterfac
     private $running = false;
 
     /**
-     * @param ClientInterface Client instance used by the context.
-     * @param array Options for the context initialization.
+     * @param ClientInterface $client Client instance used by the context.
+     * @param PipelineExecutorInterface $executor Pipeline executor instace.
      */
-    public function __construct(ClientInterface $client, Array $options = null)
+    public function __construct(ClientInterface $client, PipelineExecutorInterface $executor = null)
     {
         $this->client = $client;
-        $this->executor = $this->createExecutor($client, $options ?: array());
+        $this->executor = $executor ?: $this->createExecutor($client);
         $this->pipeline = new SplQueue();
     }
 
@@ -50,30 +50,17 @@ class PipelineContext implements BasicClientInterface, ExecutableContextInterfac
      * connection and the passed options.
      *
      * @param ClientInterface Client instance used by the context.
-     * @param array Options for the context initialization.
      * @return PipelineExecutorInterface
      */
-    protected function createExecutor(ClientInterface $client, Array $options)
+    protected function createExecutor(ClientInterface $client)
     {
-        if (isset($options['executor'])) {
-            $executor = $options['executor'];
-
-            if (is_callable($executor)) {
-                $executor = call_user_func($executor, $client, $options);
-            }
+        $options = $client->getOptions();
 
-            if (!$executor instanceof PipelineExecutorInterface) {
-                $message = 'The executor option accepts only instances of Predis\Pipeline\PipelineExecutorInterface';
-                throw new \InvalidArgumentException($message);
-            }
-
-            return $executor;
+        if (isset($options->exceptions)) {
+            return new StandardExecutor($options->exceptions);
         }
 
-        $clientOpts = $client->getOptions();
-        $useExceptions = isset($clientOpts->exceptions) ? $clientOpts->exceptions : true;
-
-        return new StandardExecutor($useExceptions);
+        return new StandardExecutor();
     }
 
     /**

tests/Predis/ClientTest.php

@@ -475,16 +475,15 @@ class ClientTest extends StandardTestCase
     public function testPipelineWithArrayReturnsPipelineContextWithOptions()
     {
         $client = new Client();
-
         $executor = $this->getMock('Predis\Pipeline\PipelineExecutorInterface');
-        $options = array('executor' => $executor);
 
+        $options = array('executor' => $executor);
         $this->assertInstanceOf('Predis\Pipeline\PipelineContext', $pipeline = $client->pipeline($options));
+        $this->assertSame($executor, $pipeline->getExecutor());
 
-        $reflection = new \ReflectionProperty($pipeline, 'executor');
-        $reflection->setAccessible(true);
-
-        $this->assertSame($executor, $reflection->getValue($pipeline));
+        $options = array('executor' => function ($client, $options) use ($executor) { return $executor; });
+        $this->assertInstanceOf('Predis\Pipeline\PipelineContext', $pipeline = $client->pipeline($options));
+        $this->assertSame($executor, $pipeline->getExecutor());
     }
 
     /**

tests/Predis/Pipeline/PipelineContextTest.php

@@ -37,16 +37,12 @@ class PipelineContextTest extends StandardTestCase
     /**
      * @group disconnected
      */
-    public function testConstructorWithOptions()
+    public function testConstructorWithExecutorArgument()
     {
         $client = new Client();
         $executor = $this->getMock('Predis\Pipeline\PipelineExecutorInterface');
 
-        $pipeline = new PipelineContext($client, array('executor' => $executor));
-        $this->assertSame($executor, $pipeline->getExecutor());
-
-        $executorCbk = function($client, $options) use($executor) { return $executor; };
-        $pipeline = new PipelineContext($client, array('executor' => $executorCbk));
+        $pipeline = new PipelineContext($client, $executor);
         $this->assertSame($executor, $pipeline->getExecutor());
     }
 
@@ -58,7 +54,7 @@ class PipelineContextTest extends StandardTestCase
         $executor = $this->getMock('Predis\Pipeline\PipelineExecutorInterface');
         $executor->expects($this->never())->method('executor');
 
-        $pipeline = new PipelineContext(new Client(), array('executor' => $executor));
+        $pipeline = new PipelineContext(new Client(), $executor);
 
         $pipeline->echo('one');
         $pipeline->echo('two');
@@ -73,7 +69,7 @@ class PipelineContextTest extends StandardTestCase
         $executor = $this->getMock('Predis\Pipeline\PipelineExecutorInterface');
         $executor->expects($this->never())->method('executor');
 
-        $pipeline = new PipelineContext(new Client(), array('executor' => $executor));
+        $pipeline = new PipelineContext(new Client(), $executor);
 
         $this->assertSame($pipeline, $pipeline->echo('one'));
         $this->assertSame($pipeline, $pipeline->echo('one')->echo('two')->echo('three'));
@@ -89,7 +85,7 @@ class PipelineContextTest extends StandardTestCase
         $executor = $this->getMock('Predis\Pipeline\PipelineExecutorInterface');
         $executor->expects($this->never())->method('executor');
 
-        $pipeline = new PipelineContext(new Client(), array('executor' => $executor));
+        $pipeline = new PipelineContext(new Client(), $executor);
 
         $pipeline->executeCommand($profile->createCommand('echo', array('one')));
         $pipeline->executeCommand($profile->createCommand('echo', array('two')));
@@ -104,7 +100,7 @@ class PipelineContextTest extends StandardTestCase
         $executor = $this->getMock('Predis\Pipeline\PipelineExecutorInterface');
         $executor->expects($this->never())->method('executor');
 
-        $pipeline = new PipelineContext(new Client(), array('executor' => $executor));
+        $pipeline = new PipelineContext(new Client(), $executor);
 
         $this->assertSame(array(), $pipeline->execute());
     }
@@ -140,7 +136,7 @@ class PipelineContextTest extends StandardTestCase
         $executor = $this->getMock('Predis\Pipeline\PipelineExecutorInterface');
         $executor->expects($this->never())->method('executor');
 
-        $pipeline = new PipelineContext(new Client(), array('executor' => $executor));
+        $pipeline = new PipelineContext(new Client(), $executor);
 
         $pipeline->echo('one');
         $pipeline->echo('two');