From 7bb219229470fda2b033ab598be13ae2a01fdbe1 Mon Sep 17 00:00:00 2001 From: Daniele Alessandri Date: Thu, 16 Jun 2016 14:38:45 +0200 Subject: [PATCH] Replace Predis\Client::on() with getClientBy(). This method improves the previous one, which worked only by picking a connection by ID, allowing users to specify a selector type among the following ones: "id", "key", "slot", "command". These selectors are expanded internally to methods, invoked against the connection object in use by the client, that follow the getConnectionByXXX() convention already in use through the library: id => getConnectionById() key => getConnectionByKey() slot => getConnectionBySlot() command => getConnectionByCommand() The underlying connection does not necessarily need to implement the aggregate connection interface as the client relies on a duck-typing approach by using method_exists(). --- CHANGELOG.md | 12 +- examples/custom_cluster_distributor.php | 4 +- src/Client.php | 29 +++-- tests/Predis/ClientTest.php | 141 +++++++++++++++++++++--- 4 files changed, 156 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 593799cd..a1c11194 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,11 +36,13 @@ v2.0.0 (201x-xx-xx) class associated. - The method `Predis\Client::getClientFor($connectionID)` has been replaced by - `Predis\Client::on($connectionID, $callable = null)`. This new method returns - a new client instance for the specified node just like before when the second - argument is omitted, otherwise the callback is invoked and the new client is - passed to it. The value returned by the callback is used as the return value - of the "on()" method. + `getClientBy($selector, $value, $callable = null)` which is more flexible as + it is not limited to picking a connection from the underlying replication or + cluster backend by ID, but allows users to specify a `$selector` that can be + either `id` (the old behavior), `key`, `slot` or `command`. The client uses + duck-typing instead of type-checking to verify that the underlying connection + implements a method that matches the specified selector which means that some + selectors may not be available to all kinds of connection backends. - Changed the signature for the constructor of `Predis\Command\RawCommand`. diff --git a/examples/custom_cluster_distributor.php b/examples/custom_cluster_distributor.php index a728f7c9..da707fbb 100644 --- a/examples/custom_cluster_distributor.php +++ b/examples/custom_cluster_distributor.php @@ -104,8 +104,8 @@ for ($i = 0; $i < 100; ++$i) { $client->get("key:$i"); } -$server1 = $client->on('first')->info(); -$server2 = $client->on('second')->info(); +$server1 = $client->getClientBy('id', 'first')->info(); +$server2 = $client->getClientBy('id', 'second')->info(); if (isset($server1['Keyspace'], $server2['Keyspace'])) { $server1 = $server1['Keyspace']; diff --git a/src/Client.php b/src/Client.php index 09cbb1c0..92d8184a 100644 --- a/src/Client.php +++ b/src/Client.php @@ -195,25 +195,38 @@ class Client implements ClientInterface, \IteratorAggregate } /** - * Creates a new client from the specified connection ID / alias. + * Creates a new client from the specified . * * The new client instances inherites the same options of the original one. * When no callable object is supplied, this method returns the new client. * When a callable object is supplied, the new client is passed as its sole * argument and its return value is returned by this method to the caller. * - * NOTE: This method works only when the client is configured to work with - * aggregate connections (cluster, replication). + * NOTE: This method works against any kind of underlying connection object + * as it uses a duck-typing approach and looks for a suitable method that + * matches the selector type to extract the correct connection. * - * @param string $connectionID Identifier of a connection. - * @param callable|null $callable Optional callable object. + * @param string $selector Type of selector (`id`, `key`, `slot`, `command`) + * @param string $value Values of selector. + * @param callable|null $callable Optional callable object. * * @return ClientInterface|mixed */ - public function on($connectionID, $callable = null) + public function getClientBy($selector, $value, $callable = null) { - if (!$connection = $this->getConnectionById($connectionID)) { - throw new \InvalidArgumentException("Invalid connection ID: `$connectionID`"); + $selector = strtolower($selector); + + if (!in_array($selector, array('id', 'key', 'slot', 'command'))) { + throw new \InvalidArgumentException("Invalid selector type: `$selector`"); + } + + if (!method_exists($this->connection, $method = "getConnectionBy$selector")) { + $class = get_class($this->connection); + throw new \InvalidArgumentException("Selecting connection by $selector is not supported by $class"); + } + + if (!$connection = $this->connection->$method($value)) { + throw new \InvalidArgumentException("Cannot find a connection by $selector matching `$value`"); } $client = new static($connection, $this->getOptions()); diff --git a/tests/Predis/ClientTest.php b/tests/Predis/ClientTest.php index 04e340dc..a728946b 100644 --- a/tests/Predis/ClientTest.php +++ b/tests/Predis/ClientTest.php @@ -712,7 +712,7 @@ class ClientTest extends PredisTestCase /** * @group disconnected */ - public function testOnMethodCreatesClientWithConnectionFromAggregateConnection() + public function testGetClientByMethodCreatesClientWithConnectionFromAggregateConnection() { $client = new Client(array('tcp://host1?alias=node01', 'tcp://host2?alias=node02'), array('prefix' => 'pfx:', 'cluster' => 'predis')); @@ -720,7 +720,7 @@ class ClientTest extends PredisTestCase $this->assertInstanceOf('Predis\Connection\NodeConnectionInterface', $node01 = $client->getConnectionById('node01')); $this->assertInstanceOf('Predis\Connection\NodeConnectionInterface', $node02 = $client->getConnectionById('node02')); - $clientNode02 = $client->on('node02'); + $clientNode02 = $client->getClientBy('id', 'node02'); $this->assertInstanceOf('Predis\Client', $clientNode02); $this->assertSame($node02, $clientNode02->getConnection()); @@ -730,18 +730,18 @@ class ClientTest extends PredisTestCase /** * @group disconnected */ - public function testOnMethodReturnsInstanceOfSubclass() + public function testGetClientByMethodReturnsInstanceOfSubclass() { $nodes = array('tcp://host1?alias=node01', 'tcp://host2?alias=node02'); $client = $this->getMock('Predis\Client', array('dummy'), array($nodes, array('cluster' => 'predis')), 'SubclassedClient'); - $this->assertInstanceOf('SubclassedClient', $client->on('node02')); + $this->assertInstanceOf('SubclassedClient', $client->getClientBy('id', 'node02')); } /** * @group disconnected */ - public function testOnMethodInvokesCallableInSecondArgumentAndReturnsItsReturnValue() + public function testGetClientByMethodInvokesCallableInSecondArgumentAndReturnsItsReturnValue() { $test = $this; $client = new Client(array('tcp://host1?alias=node01', 'tcp://host2?alias=node02'), array('cluster' => 'predis')); @@ -760,31 +760,142 @@ class ClientTest extends PredisTestCase })) ->will($this->returnValue('value')); - $this->assertSame('value', $client->on('node02', $callable)); + $this->assertSame('value', $client->getClientBy('id', 'node02', $callable)); } /** * @group disconnected - * @expectedException \Predis\NotSupportedException - * @expectedExceptionMessage Retrieving connections by ID is supported only by aggregate connections */ - public function testOnMethodThrowsExceptionWithNodeConnection() + public function testGetClientByMethodSupportsSelectingConnectionById() { - $client = new Client('tcp://127.0.0.1?alias=node01'); + $connection = $this->getMock('Predis\Connection\NodeConnectionInterface'); - $client->on('node01'); + $aggregate = $this->getMockBuilder('Predis\Connection\AggregateConnectionInterface') + ->setMethods(array('getConnectionById')) + ->getMockForAbstractClass(); + $aggregate + ->expects($this->once()) + ->method('getConnectionById') + ->with('nodeXX') + ->will($this->returnValue($connection)); + + $client = new Client($aggregate); + $nodeClient = $client->getClientBy('id', 'nodeXX'); + + $this->assertSame($connection, $nodeClient->getConnection()); + $this->assertSame($client->getOptions(), $nodeClient->getOptions()); } /** * @group disconnected * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Invalid connection ID: `nodeXX` + * @expectedExceptionMessage Cannot find a connection by id matching `nodeXX` */ - public function testOnMethodThrowsExceptionWithUnknownConnectionID() + public function testGetClientByMethodThrowsExceptionSelectingConnectionByUnknownId() { - $client = new Client(array('tcp://host1?alias=node01', 'tcp://host2?alias=node02'), array('cluster' => 'predis')); + $aggregate = $this->getMock('Predis\Connection\AggregateConnectionInterface'); + $aggregate + ->expects($this->once()) + ->method('getConnectionById') + ->with('nodeXX') + ->will($this->returnValue(null)); - $client->on('nodeXX'); + $client = new Client($aggregate); + $client->getClientBy('id', 'nodeXX'); + } + + /** + * @group disconnected + */ + public function testGetClientByMethodSupportsSelectingConnectionByKey() + { + $connection = $this->getMock('Predis\Connection\NodeConnectionInterface'); + + $aggregate = $this->getMockBuilder('Predis\Connection\AggregateConnectionInterface') + ->setMethods(array('getConnectionByKey')) + ->getMockForAbstractClass(); + $aggregate + ->expects($this->once()) + ->method('getConnectionByKey') + ->with('key:1') + ->will($this->returnValue($connection)); + + $client = new Client($aggregate); + $nodeClient = $client->getClientBy('key', 'key:1'); + + $this->assertSame($connection, $nodeClient->getConnection()); + $this->assertSame($client->getOptions(), $nodeClient->getOptions()); + } + + /** + * @group disconnected + */ + public function testGetClientByMethodSupportsSelectingConnectionBySlot() + { + $connection = $this->getMock('Predis\Connection\NodeConnectionInterface'); + + $aggregate = $this->getMockBuilder('Predis\Connection\AggregateConnectionInterface') + ->setMethods(array('getConnectionBySlot')) + ->getMockForAbstractClass(); + $aggregate + ->expects($this->once()) + ->method('getConnectionBySlot') + ->with(5460) + ->will($this->returnValue($connection)); + + $client = new Client($aggregate); + $nodeClient = $client->getClientBy('slot', 5460); + + $this->assertSame($connection, $nodeClient->getConnection()); + $this->assertSame($client->getOptions(), $nodeClient->getOptions()); + } + + /** + * @group disconnected + */ + public function testGetClientByMethodSupportsSelectingConnectionByCommand() + { + $command = \Predis\Command\RawCommand::create('GET', 'key'); + $connection = $this->getMock('Predis\Connection\NodeConnectionInterface'); + + $aggregate = $this->getMockBuilder('Predis\Connection\AggregateConnectionInterface') + ->setMethods(array('getConnectionByCommand')) + ->getMockForAbstractClass(); + $aggregate + ->expects($this->once()) + ->method('getConnectionByCommand') + ->with($command) + ->will($this->returnValue($connection)); + + $client = new Client($aggregate); + $nodeClient = $client->getClientBy('command', $command); + + $this->assertSame($connection, $nodeClient->getConnection()); + $this->assertSame($client->getOptions(), $nodeClient->getOptions()); + } + + /** + * @group disconnected + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Invalid selector type: `unknown` + */ + public function testGetClientByMethodThrowsExceptionWhenSelectingConnectionByUnknownType() + { + $client = new Client('tcp://127.0.0.1?alias=node01'); + + $client->getClientBy('unknown', 'test'); + } + + /** + * @group disconnected + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Selecting connection by id is not supported by Predis\Connection\StreamConnection + */ + public function testGetClientByMethodThrowsExceptionWhenConnectionDoesNotSupportSelectorType() + { + $client = new Client('tcp://127.0.0.1?alias=node01'); + + $client->getClientBy('id', 'node01'); } /**