From 6cd124cc593ea4fb9570e36d9569162ddb220269 Mon Sep 17 00:00:00 2001 From: Eloi Poch Date: Sat, 20 Apr 2013 13:02:17 +0200 Subject: [PATCH 1/5] Fix DispatcherLoop error with client prefix keys DispatcherLoop works properly if a client have configured a prefix for the keys or not --- lib/Predis/PubSub/DispatcherLoop.php | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/Predis/PubSub/DispatcherLoop.php b/lib/Predis/PubSub/DispatcherLoop.php index e2455a00..d6f4ae20 100644 --- a/lib/Predis/PubSub/DispatcherLoop.php +++ b/lib/Predis/PubSub/DispatcherLoop.php @@ -96,8 +96,10 @@ class DispatcherLoop */ public function attachCallback($channel, $callback) { + $callbackName = $this->getPrefixKeys() . $channel; + $this->validateCallback($callback); - $this->callbacks[$channel] = $callback; + $this->callbacks[$callbackName] = $callback; $this->pubSubContext->subscribe($channel); } @@ -108,8 +110,10 @@ class DispatcherLoop */ public function detachCallback($channel) { - if (isset($this->callbacks[$channel])) { - unset($this->callbacks[$channel]); + $callbackName = $this->getPrefixKeys() . $channel; + + if (isset($this->callbacks[$callbackName])) { + unset($this->callbacks[$callbackName]); $this->pubSubContext->unsubscribe($channel); } } @@ -148,4 +152,16 @@ class DispatcherLoop { $this->pubSubContext->closeContext(); } + + /** + * Return the prefix of the keys + * + * @return string + */ + protected function getPrefixKeys() + { + $prefix = $this->client->getOptions()->prefix; + + return $prefix ? $prefix->getPrefix() : ''; + } } From 8451146d38cb96ef27c274f365f6a60d23e8e89b Mon Sep 17 00:00:00 2001 From: Daniele Alessandri Date: Fri, 31 May 2013 10:17:08 +0200 Subject: [PATCH 2/5] Expose underlying client object from pub/sub iterators. --- lib/Predis/PubSub/PubSubContext.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/Predis/PubSub/PubSubContext.php b/lib/Predis/PubSub/PubSubContext.php index 910459b6..1f4b6b9a 100644 --- a/lib/Predis/PubSub/PubSubContext.php +++ b/lib/Predis/PubSub/PubSubContext.php @@ -41,6 +41,16 @@ class PubSubContext extends AbstractPubSubContext $this->genericSubscribeInit('psubscribe'); } + /** + * Returns the underlying client instance used by the pub/sub iterator. + * + * @return ClientInterface + */ + public function getClient() + { + return $this->client; + } + /** * Checks if the passed client instance satisfies the required conditions * needed to initialize a Publish / Subscribe context. From dd50cfe47e63ad7ab2d2f855ecd2b7508662c0ce Mon Sep 17 00:00:00 2001 From: Daniele Alessandri Date: Fri, 31 May 2013 10:17:54 +0200 Subject: [PATCH 3/5] Slightly rework original pull request. --- lib/Predis/PubSub/DispatcherLoop.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Predis/PubSub/DispatcherLoop.php b/lib/Predis/PubSub/DispatcherLoop.php index d6f4ae20..a06036a0 100644 --- a/lib/Predis/PubSub/DispatcherLoop.php +++ b/lib/Predis/PubSub/DispatcherLoop.php @@ -21,7 +21,6 @@ use Predis\ClientInterface; */ class DispatcherLoop { - private $client; private $pubSubContext; private $callbacks; private $defaultCallback; @@ -33,7 +32,6 @@ class DispatcherLoop public function __construct(ClientInterface $client) { $this->callbacks = array(); - $this->client = $client; $this->pubSubContext = $client->pubSub(); } @@ -160,8 +158,12 @@ class DispatcherLoop */ protected function getPrefixKeys() { - $prefix = $this->client->getOptions()->prefix; + $options = $this->pubSubContext->getClient()->getOptions(); - return $prefix ? $prefix->getPrefix() : ''; + if (isset($options->prefix)) { + return $options->prefix->getPrefix(); + } + + return ''; } } From 69c66e157f3dba8e16a75d122f94ab1f9629e144 Mon Sep 17 00:00:00 2001 From: Daniele Alessandri Date: Fri, 31 May 2013 10:21:21 +0200 Subject: [PATCH 4/5] Make callbacks properties protected in the DispatcherLoop class. This makes easier to extend the class. The underlying pub/sub iterator property is still private, but can be accessed via public getter method. --- lib/Predis/PubSub/DispatcherLoop.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/Predis/PubSub/DispatcherLoop.php b/lib/Predis/PubSub/DispatcherLoop.php index a06036a0..fa3ba7d2 100644 --- a/lib/Predis/PubSub/DispatcherLoop.php +++ b/lib/Predis/PubSub/DispatcherLoop.php @@ -22,9 +22,10 @@ use Predis\ClientInterface; class DispatcherLoop { private $pubSubContext; - private $callbacks; - private $defaultCallback; - private $subscriptionCallback; + + protected $callbacks; + protected $defaultCallback; + protected $subscriptionCallback; /** * @param ClientInterface Client instance used by the context. From 3dea41aa66d200bb5fb347d70e22f23f4dc2d4b6 Mon Sep 17 00:00:00 2001 From: Daniele Alessandri Date: Fri, 31 May 2013 10:39:46 +0200 Subject: [PATCH 5/5] Add tests for prefixed Predis\PubSub\DispatcherLoop. --- tests/Predis/PubSub/DispatcherLoopTest.php | 42 ++++++++++++++++++++++ tests/Predis/PubSub/PubSubContextTest.php | 13 +++++++ 2 files changed, 55 insertions(+) diff --git a/tests/Predis/PubSub/DispatcherLoopTest.php b/tests/Predis/PubSub/DispatcherLoopTest.php index 263469b2..5df84758 100644 --- a/tests/Predis/PubSub/DispatcherLoopTest.php +++ b/tests/Predis/PubSub/DispatcherLoopTest.php @@ -82,4 +82,46 @@ class DispatcherLoopTest extends StandardTestCase $this->assertTrue($consumer->ping()); } + + /** + * @group connected + */ + public function testDispatcherLoopAgainstRedisServerWithPrefix() + { + $parameters = array( + 'host' => REDIS_SERVER_HOST, + 'port' => REDIS_SERVER_PORT, + 'database' => REDIS_SERVER_DBNUM, + // Prevents suite from handing on broken test + 'read_write_timeout' => 2, + ); + + $options = array('profile' => REDIS_SERVER_VERSION); + + $producerNonPfx = new Client($parameters, $options); + $producerNonPfx->connect(); + + $producerPfx = new Client($parameters, $options + array('prefix' => 'foobar')); + $producerPfx->connect(); + + $consumer = new Client($parameters, $options + array('prefix' => 'foobar')); + $dispatcher = new DispatcherLoop($consumer); + + $callback = $this->getMock('stdClass', array('__invoke')); + $callback->expects($this->exactly(1)) + ->method('__invoke') + ->with($this->equalTo('arg:prefixed')) + ->will($this->returnCallback(function ($arg) use ($dispatcher) { + $dispatcher->stop(); + })); + + $dispatcher->attachCallback('callback', $callback); + + $producerNonPfx->publish('callback', 'arg:non-prefixed'); + $producerPfx->publish('callback', 'arg:prefixed'); + + $dispatcher->run(); + + $this->assertTrue($consumer->ping()); + } } diff --git a/tests/Predis/PubSub/PubSubContextTest.php b/tests/Predis/PubSub/PubSubContextTest.php index 5bc36ca2..140e8f25 100644 --- a/tests/Predis/PubSub/PubSubContextTest.php +++ b/tests/Predis/PubSub/PubSubContextTest.php @@ -245,6 +245,19 @@ class PubSubContextTest extends StandardTestCase $this->assertFalse($pubsub->valid()); } + /** + * @group disconnected + */ + public function testGetUnderlyingClientInstance() + { + $connection = $this->getMock('Predis\Connection\SingleConnectionInterface'); + + $client = new Client($connection); + $pubsub = new PubSubContext($client); + + $this->assertSame($client, $pubsub->getClient()); + } + // ******************************************************************** // // ---- INTEGRATION TESTS --------------------------------------------- // // ******************************************************************** //