diff --git a/CHANGELOG.md b/CHANGELOG.md index 19121abd..3ca2a178 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,10 @@ v2.0.0 (201x-xx-xx) get a single connection from the pool by using its ID. It is also possible to retrive a connection by role using the method getConnectionByRole(). +- The concept of connection ID (ip:port pair) and connection alias (the `alias` + parameter) in `Predis\Connection\Cluster\PredisCluster` has been separated. + This change does not affect distribution and it is safe for existing clusters. + - Client option classes now live in the `Predis\Configuration\Option` namespace. - Classes for Redis commands have been moved into the new `Predis\Command\Redis` diff --git a/examples/custom_cluster_distributor.php b/examples/custom_cluster_distributor.php index da707fbb..16f9b594 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->getClientBy('id', 'first')->info(); -$server2 = $client->getClientBy('id', 'second')->info(); +$server1 = $client->getClientBy('alias', 'first')->info(); +$server2 = $client->getClientBy('alias', 'second')->info(); if (isset($server1['Keyspace'], $server2['Keyspace'])) { $server1 = $server1['Keyspace']; diff --git a/src/Connection/Cluster/PredisCluster.php b/src/Connection/Cluster/PredisCluster.php index c95bf6cc..211ecfc9 100644 --- a/src/Connection/Cluster/PredisCluster.php +++ b/src/Connection/Cluster/PredisCluster.php @@ -27,8 +27,24 @@ use Predis\NotSupportedException; */ class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable { - private $pool; + /** + * @var NodeConnectionInterface[] + */ + private $pool = array(); + + /** + * @var NodeConnectionInterface[] + */ + private $aliases = array(); + + /** + * @var StrategyInterface + */ private $strategy; + + /** + * @var Predis\Cluster\Distributor\DistributorInterface + */ private $distributor; /** @@ -36,7 +52,6 @@ class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable */ public function __construct(StrategyInterface $strategy = null) { - $this->pool = array(); $this->strategy = $strategy ?: new PredisStrategy(); $this->distributor = $this->strategy->getDistributor(); } @@ -82,14 +97,13 @@ class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable { $parameters = $connection->getParameters(); + $this->pool[(string) $connection] = $connection; + if (isset($parameters->alias)) { - $this->pool[$parameters->alias] = $connection; - } else { - $this->pool[] = $connection; + $this->aliases[$parameters->alias] = $connection; } - $weight = isset($parameters->weight) ? $parameters->weight : null; - $this->distributor->add($connection, $weight); + $this->distributor->add($connection, $parameters->weight); } /** @@ -97,32 +111,20 @@ class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable */ public function remove(NodeConnectionInterface $connection) { - if (($id = array_search($connection, $this->pool, true)) !== false) { + if (false !== $id = array_search($connection, $this->pool, true)) { unset($this->pool[$id]); $this->distributor->remove($connection); + if ($this->aliases && $alias = $connection->getParameters()->alias) { + unset($this->aliases[$alias]); + } + return true; } return false; } - /** - * Removes a connection instance using its alias or index. - * - * @param string $connectionID Alias or index of a connection. - * - * @return bool Returns true if the connection was in the pool. - */ - public function removeById($connectionID) - { - if ($connection = $this->getConnectionById($connectionID)) { - return $this->remove($connection); - } - - return false; - } - /** * {@inheritdoc} */ @@ -144,9 +146,37 @@ class PredisCluster implements ClusterInterface, \IteratorAggregate, \Countable /** * {@inheritdoc} */ - public function getConnectionById($connectionID) + public function getConnectionById($id) { - return isset($this->pool[$connectionID]) ? $this->pool[$connectionID] : null; + if (isset($this->pool[$id])) { + return $this->pool[$id]; + } + } + + /** + * Returns a connection instance by its alias. + * + * @param string $alias Connection alias. + * + * @return NodeConnectionInterface|null + */ + public function getConnectionByAlias($alias) + { + if (isset($this->aliases[$alias])) { + return $this->aliases[$alias]; + } + } + + /** + * Retrieves a connection instance by slot. + * + * @param string $key Key string. + * + * @return NodeConnectionInterface|null + */ + public function getConnectionBySlot($slot) + { + return $this->distributor->getBySlot($slot); } /** diff --git a/tests/Predis/ClientTest.php b/tests/Predis/ClientTest.php index 94a14cfa..bf4fa6fc 100644 --- a/tests/Predis/ClientTest.php +++ b/tests/Predis/ClientTest.php @@ -690,7 +690,7 @@ class ClientTest extends PredisTestCase $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->getClientBy('id', 'node02')); + $this->assertInstanceOf('SubclassedClient', $client->getClientBy('alias', 'node02')); } /** @@ -715,7 +715,7 @@ class ClientTest extends PredisTestCase })) ->will($this->returnValue('value')); - $this->assertSame('value', $client->getClientBy('id', 'node02', $callable)); + $this->assertSame('value', $client->getClientBy('alias', 'node02', $callable)); } /** diff --git a/tests/Predis/Connection/Cluster/PredisClusterTest.php b/tests/Predis/Connection/Cluster/PredisClusterTest.php index 6d0366be..e003e323 100644 --- a/tests/Predis/Connection/Cluster/PredisClusterTest.php +++ b/tests/Predis/Connection/Cluster/PredisClusterTest.php @@ -32,8 +32,8 @@ class PredisClusterTest extends PredisTestCase */ public function testAddingConnectionsToCluster() { - $connection1 = $this->getMockConnection(); - $connection2 = $this->getMockConnection(); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $cluster = new PredisCluster(); @@ -41,17 +41,17 @@ class PredisClusterTest extends PredisTestCase $cluster->add($connection2); $this->assertSame(2, count($cluster)); - $this->assertSame($connection1, $cluster->getConnectionById(0)); - $this->assertSame($connection2, $cluster->getConnectionById(1)); + $this->assertSame($connection1, $cluster->getConnectionById('127.0.0.1:7001')); + $this->assertSame($connection2, $cluster->getConnectionById('127.0.0.1:7002')); } /** * @group disconnected */ - public function testAddingConnectionsToClusterUsesConnectionAlias() + public function testAddingConnectionsWithAliasParameterToCluster() { - $connection1 = $this->getMockConnection('tcp://host1:7001?alias=node1'); - $connection2 = $this->getMockConnection('tcp://host1:7002?alias=node2'); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001?alias=node01'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002?alias=node02'); $cluster = new PredisCluster(); @@ -59,8 +59,8 @@ class PredisClusterTest extends PredisTestCase $cluster->add($connection2); $this->assertSame(2, count($cluster)); - $this->assertSame($connection1, $cluster->getConnectionById('node1')); - $this->assertSame($connection2, $cluster->getConnectionById('node2')); + $this->assertSame($connection1, $cluster->getConnectionByAlias('node01')); + $this->assertSame($connection2, $cluster->getConnectionByAlias('node02')); } /** @@ -68,9 +68,9 @@ class PredisClusterTest extends PredisTestCase */ public function testRemovingConnectionsFromCluster() { - $connection1 = $this->getMockConnection(); - $connection2 = $this->getMockConnection(); - $connection3 = $this->getMockConnection(); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001?alias=node01'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); + $connection3 = $this->getMockConnection('tcp://127.0.0.1:7003'); $cluster = new PredisCluster(); @@ -78,29 +78,11 @@ class PredisClusterTest extends PredisTestCase $cluster->add($connection2); $this->assertTrue($cluster->remove($connection1)); + $this->assertNull($cluster->getConnectionByAlias('node02')); + $this->assertFalse($cluster->remove($connection3)); - $this->assertSame(1, count($cluster)); - } - /** - * @group disconnected - */ - public function testRemovingConnectionsFromClusterByAlias() - { - $connection1 = $this->getMockConnection(); - $connection2 = $this->getMockConnection('tcp://host1:7001?alias=node2'); - $connection3 = $this->getMockConnection('tcp://host1:7002?alias=node3'); - - $cluster = new PredisCluster(); - - $cluster->add($connection1); - $cluster->add($connection2); - $cluster->add($connection3); - - $this->assertTrue($cluster->removeById(0)); - $this->assertTrue($cluster->removeById('node2')); - $this->assertFalse($cluster->removeById('node4')); - $this->assertSame(1, count($cluster)); + $this->assertCount(1, $cluster); } /** @@ -108,12 +90,12 @@ class PredisClusterTest extends PredisTestCase */ public function testConnectForcesAllConnectionsToConnect() { - $connection1 = $this->getMockConnection(); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); $connection1 ->expects($this->once()) ->method('connect'); - $connection2 = $this->getMockConnection(); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $connection2 ->expects($this->once()) ->method('connect'); @@ -131,12 +113,12 @@ class PredisClusterTest extends PredisTestCase */ public function testDisconnectForcesAllConnectionsToDisconnect() { - $connection1 = $this->getMockConnection(); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); $connection1 ->expects($this->once()) ->method('disconnect'); - $connection2 = $this->getMockConnection(); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $connection2 ->expects($this->once()) ->method('disconnect'); @@ -154,13 +136,13 @@ class PredisClusterTest extends PredisTestCase */ public function testIsConnectedReturnsTrueIfAtLeastOneConnectionIsOpen() { - $connection1 = $this->getMockConnection(); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); $connection1 ->expects($this->once()) ->method('isConnected') ->will($this->returnValue(false)); - $connection2 = $this->getMockConnection(); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $connection2 ->expects($this->once()) ->method('isConnected') @@ -179,13 +161,13 @@ class PredisClusterTest extends PredisTestCase */ public function testIsConnectedReturnsFalseIfAllConnectionsAreClosed() { - $connection1 = $this->getMockConnection(); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); $connection1 ->expects($this->once()) ->method('isConnected') ->will($this->returnValue(false)); - $connection2 = $this->getMockConnection(); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $connection2 ->expects($this->once()) ->method('isConnected') @@ -204,8 +186,8 @@ class PredisClusterTest extends PredisTestCase */ public function testCanReturnAnIteratorForConnections() { - $connection1 = $this->getMockConnection(); - $connection2 = $this->getMockConnection(); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $cluster = new PredisCluster(); @@ -215,8 +197,27 @@ class PredisClusterTest extends PredisTestCase $this->assertInstanceOf('Iterator', $iterator = $cluster->getIterator()); $connections = iterator_to_array($iterator); - $this->assertSame($connection1, $connections[0]); - $this->assertSame($connection2, $connections[1]); + $this->assertSame(array( + '127.0.0.1:7001' => $connection1, + '127.0.0.1:7002' => $connection2, + ), iterator_to_array($iterator)); + } + + /** + * @group disconnected + */ + public function testReturnsCorrectConnectionUsingSlot() + { + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); + + $cluster = new PredisCluster(); + + $cluster->add($connection1); + $cluster->add($connection2); + + $this->assertSame($connection1, $cluster->getConnectionBySlot(1839357934)); + $this->assertSame($connection2, $cluster->getConnectionBySlot(2146453549)); } /** @@ -224,8 +225,8 @@ class PredisClusterTest extends PredisTestCase */ public function testReturnsCorrectConnectionUsingKey() { - $connection1 = $this->getMockConnection('tcp://host1:7001'); - $connection2 = $this->getMockConnection('tcp://host1:7002'); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $cluster = new PredisCluster(); @@ -245,8 +246,8 @@ class PredisClusterTest extends PredisTestCase { $commands = $this->getCommandFactory(); - $connection1 = $this->getMockConnection('tcp://host1:7001'); - $connection2 = $this->getMockConnection('tcp://host1:7002'); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $cluster = new PredisCluster(); @@ -285,7 +286,7 @@ class PredisClusterTest extends PredisTestCase $cluster = new PredisCluster(); - $cluster->add($this->getMockConnection()); + $cluster->add($this->getMockConnection('tcp://127.0.0.1:6379')); $cluster->getConnectionByCommand($ping); } @@ -323,13 +324,13 @@ class PredisClusterTest extends PredisTestCase { $command = $this->getCommandFactory()->createCommand('get', array('node01:5431')); - $connection1 = $this->getMockConnection('tcp://host1:7001'); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); $connection1 ->expects($this->once()) ->method('writeRequest') ->with($command); - $connection2 = $this->getMockConnection('tcp://host1:7002'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $connection2 ->expects($this->never()) ->method('writeRequest'); @@ -349,12 +350,12 @@ class PredisClusterTest extends PredisTestCase { $command = $this->getCommandFactory()->createCommand('get', array('node02:3212')); - $connection1 = $this->getMockConnection('tcp://host1:7001'); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); $connection1 ->expects($this->never()) ->method('readResponse'); - $connection2 = $this->getMockConnection('tcp://host1:7002'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $connection2 ->expects($this->once()) ->method('readResponse') @@ -375,13 +376,13 @@ class PredisClusterTest extends PredisTestCase { $command = $this->getCommandFactory()->createCommand('get', array('node01:5431')); - $connection1 = $this->getMockConnection('tcp://host1:7001'); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001'); $connection1 ->expects($this->once()) ->method('executeCommand') ->with($command); - $connection2 = $this->getMockConnection('tcp://host1:7002'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002'); $connection2 ->expects($this->never()) ->method('executeCommand'); @@ -399,8 +400,8 @@ class PredisClusterTest extends PredisTestCase */ public function testCanBeSerialized() { - $connection1 = $this->getMockConnection('tcp://host1?alias=first'); - $connection2 = $this->getMockConnection('tcp://host2?alias=second'); + $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001?alias=first'); + $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002?alias=second'); $cluster = new PredisCluster();