Browse Source

Merge branch 'command-hash-strategy'

Conflicts:
	lib/Predis/Connection/RedisCluster.php
Daniele Alessandri 13 years ago
parent
commit
93971bcb45

+ 8 - 2
examples/CustomDistributionStrategy.php

@@ -16,9 +16,10 @@ require 'SharedConfigurations.php';
 // that implements Predis\Distribution\DistributionStrategyInterface.
 
 use Predis\Connection\PredisCluster;
-use Predis\Distribution\DistributionStrategyInterface;
+use Predis\Cluster\Distribution\DistributionStrategyInterface;
+use Predis\Cluster\Hash\HashGeneratorInterface;
 
-class NaiveDistributionStrategy implements DistributionStrategyInterface
+class NaiveDistributionStrategy implements DistributionStrategyInterface, HashGeneratorInterface
 {
     private $nodes;
     private $nodesCount;
@@ -57,6 +58,11 @@ class NaiveDistributionStrategy implements DistributionStrategyInterface
     {
         return crc32($value);
     }
+
+    public function getHashGenerator()
+    {
+        return $this;
+    }
 }
 
 $options = array(

+ 3 - 6
lib/Predis/Command/Hash/CommandHashStrategyInterface.php → lib/Predis/Cluster/CommandHashStrategyInterface.php

@@ -9,10 +9,9 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Command\Hash;
+namespace Predis\Cluster;
 
 use Predis\Command\CommandInterface;
-use Predis\Distribution\HashGeneratorInterface;
 
 /**
  * Interface for classes defining the strategy used to calculate an hash
@@ -28,18 +27,16 @@ interface CommandHashStrategyInterface
      * Returns the hash for the given command using the specified algorithm, or null
      * if the command cannot be hashed.
      *
-     * @param HashGeneratorInterface $hasher Hash algorithm.
      * @param CommandInterface $command Command to be hashed.
      * @return int
      */
-    public function getHash(HashGeneratorInterface $hasher, CommandInterface $command);
+    public function getHash(CommandInterface $command);
 
     /**
      * Returns the hash for the given key using the specified algorithm.
      *
-     * @param HashGeneratorInterface $hasher Hash algorithm.
      * @param string $key Key to be hashed.
      * @return string
      */
-    public function getKeyHash(HashGeneratorInterface $hasher, $key);
+    public function getKeyHash($key);
 }

+ 9 - 2
lib/Predis/Distribution/DistributionStrategyInterface.php → lib/Predis/Cluster/Distribution/DistributionStrategyInterface.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Distribution;
 
 /**
  * A distributor implements the logic to automatically distribute
@@ -17,7 +17,7 @@ namespace Predis\Distribution;
  *
  * @author Daniele Alessandri <suppakilla@gmail.com>
  */
-interface DistributionStrategyInterface extends HashGeneratorInterface
+interface DistributionStrategyInterface
 {
     /**
      * Adds a node to the distributor with an optional weight.
@@ -40,4 +40,11 @@ interface DistributionStrategyInterface extends HashGeneratorInterface
      * @return mixed
      */
     public function get($key);
+
+    /**
+     * Returns the underlying hash generator instance.
+     *
+     * @return Predis\Cluster\Hash\HashGeneratorInterface
+     */
+    public function getHashGenerator();
 }

+ 1 - 1
lib/Predis/Distribution/EmptyRingException.php → lib/Predis/Cluster/Distribution/EmptyRingException.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Distribution;
 
 /**
  * Exception class that identifies empty rings.

+ 12 - 2
lib/Predis/Distribution/HashRing.php → lib/Predis/Cluster/Distribution/HashRing.php

@@ -9,7 +9,9 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Distribution;
+
+use Predis\Cluster\Hash\HashGeneratorInterface;
 
 /**
  * This class implements an hashring-based distributor that uses the same
@@ -19,7 +21,7 @@ namespace Predis\Distribution;
  * @author Daniele Alessandri <suppakilla@gmail.com>
  * @author Lorenzo Castelli <lcastelli@gmail.com>
  */
-class HashRing implements DistributionStrategyInterface
+class HashRing implements DistributionStrategyInterface, HashGeneratorInterface
 {
     const DEFAULT_REPLICAS = 128;
     const DEFAULT_WEIGHT   = 100;
@@ -227,4 +229,12 @@ class HashRing implements DistributionStrategyInterface
         // equal to the key. If no such item exists, return the last item.
         return $upper >= 0 ? $upper : $ringKeysCount - 1;
     }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getHashGenerator()
+    {
+        return $this;
+    }
 }

+ 1 - 1
lib/Predis/Distribution/KetamaPureRing.php → lib/Predis/Cluster/Distribution/KetamaPureRing.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Distribution;
 
 /**
  * This class implements an hashring-based distributor that uses the same

+ 1 - 1
lib/Predis/Distribution/CRC16HashGenerator.php → lib/Predis/Cluster/Hash/CRC16HashGenerator.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Hash;
 
 /**
  * This class implements the CRC-CCITT-16 algorithm used by redis-cluster.

+ 1 - 1
lib/Predis/Distribution/HashGeneratorInterface.php → lib/Predis/Cluster/Hash/HashGeneratorInterface.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Hash;
 
 /**
  * A generator of node keys implements the logic used to calculate the hash of

+ 16 - 9
lib/Predis/Command/Hash/PredisClusterHashStrategy.php → lib/Predis/Cluster/PredisClusterHashStrategy.php

@@ -9,10 +9,10 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Command\Hash;
+namespace Predis\Cluster;
 
+use Predis\Cluster\Hash\HashGeneratorInterface;
 use Predis\Command\CommandInterface;
-use Predis\Distribution\HashGeneratorInterface;
 
 /**
  * Default class used by Predis for client-side sharding to calculate
@@ -23,13 +23,15 @@ use Predis\Distribution\HashGeneratorInterface;
 class PredisClusterHashStrategy implements CommandHashStrategyInterface
 {
     private $commands;
+    private $hashGenerator;
 
     /**
-     *
+     * @param HashGeneratorInterface $hashGenerator Hash generator instance.
      */
-    public function __construct()
+    public function __construct(HashGeneratorInterface $hashGenerator)
     {
         $this->commands = $this->getDefaultCommands();
+        $this->hashGenerator = $hashGenerator;
     }
 
     /**
@@ -282,22 +284,27 @@ class PredisClusterHashStrategy implements CommandHashStrategyInterface
     /**
      * {@inheritdoc}
      */
-    public function getHash(HashGeneratorInterface $hasher, CommandInterface $command)
+    public function getHash(CommandInterface $command)
     {
-        if (isset($this->commands[$cmdID = $command->getId()])) {
+        $hash = $command->getHash();
+
+        if (!isset($hash) && isset($this->commands[$cmdID = $command->getId()])) {
             if ($key = call_user_func($this->commands[$cmdID], $command)) {
-                return $this->getKeyHash($hasher, $key);
+                $hash = $this->getKeyHash($key);
+                $command->setHash($hash);
             }
         }
+
+        return $hash;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function getKeyHash(HashGeneratorInterface $hasher, $key)
+    public function getKeyHash($key)
     {
         $key = $this->extractKeyTag($key);
-        $hash = $hasher->hash($key);
+        $hash = $this->hashGenerator->hash($key);
 
         return $hash;
     }

+ 15 - 8
lib/Predis/Command/Hash/RedisClusterHashStrategy.php → lib/Predis/Cluster/RedisClusterHashStrategy.php

@@ -9,10 +9,10 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Command\Hash;
+namespace Predis\Cluster;
 
+use Predis\Cluster\Hash\CRC16HashGenerator;
 use Predis\Command\CommandInterface;
-use Predis\Distribution\HashGeneratorInterface;
 
 /**
  * Default class used by Predis to calculate hashes out of keys of
@@ -23,6 +23,7 @@ use Predis\Distribution\HashGeneratorInterface;
 class RedisClusterHashStrategy implements CommandHashStrategyInterface
 {
     private $commands;
+    private $hashGenerator;
 
     /**
      *
@@ -30,6 +31,7 @@ class RedisClusterHashStrategy implements CommandHashStrategyInterface
     public function __construct()
     {
         $this->commands = $this->getDefaultCommands();
+        $this->hashGenerator = new CRC16HashGenerator();
     }
 
     /**
@@ -235,20 +237,25 @@ class RedisClusterHashStrategy implements CommandHashStrategyInterface
     /**
      * {@inheritdoc}
      */
-    public function getHash(HashGeneratorInterface $hasher, CommandInterface $command)
+    public function getHash(CommandInterface $command)
     {
-        if (isset($this->commands[$cmdID = $command->getId()])) {
-            if ($key = call_user_func($this->commands[$cmdID], $command)) {
-                return $this->getKeyHash($hasher, $key);
+        $hash = $command->getHash();
+
+        if (!isset($hash) && isset($this->commands[$cmdID = $command->getId()])) {
+            if (null !== $key = call_user_func($this->commands[$cmdID], $command)) {
+                $hash = $this->hashGenerator->hash($key);
+                $command->setHash($hash);
             }
         }
+
+        return $hash;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function getKeyHash(HashGeneratorInterface $hasher, $key)
+    public function getKeyHash($key)
     {
-        return $hasher->hash($key);
+        return $this->hashGenerator->hash($key);
     }
 }

+ 16 - 17
lib/Predis/Connection/PredisCluster.php

@@ -13,10 +13,10 @@ namespace Predis\Connection;
 
 use Predis\ClientException;
 use Predis\NotSupportedException;
+use Predis\Cluster\PredisClusterHashStrategy;
+use Predis\Cluster\Distribution\DistributionStrategyInterface;
+use Predis\Cluster\Distribution\HashRing;
 use Predis\Command\CommandInterface;
-use Predis\Command\Hash\PredisClusterHashStrategy;
-use Predis\Distribution\HashRing;
-use Predis\Distribution\DistributionStrategyInterface;
 
 /**
  * Abstraction for a cluster of aggregated connections to various Redis servers
@@ -28,17 +28,19 @@ use Predis\Distribution\DistributionStrategyInterface;
 class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \Countable
 {
     private $pool;
+    private $strategy;
     private $distributor;
-    private $cmdHasher;
 
     /**
      * @param DistributionStrategyInterface $distributor Distribution strategy used by the cluster.
      */
     public function __construct(DistributionStrategyInterface $distributor = null)
     {
+        $distributor = $distributor ?: new HashRing();
+
         $this->pool = array();
-        $this->cmdHasher = new PredisClusterHashStrategy();
-        $this->distributor = $distributor ?: new HashRing();
+        $this->strategy = new PredisClusterHashStrategy($distributor->getHashGenerator());
+        $this->distributor = $distributor;
     }
 
     /**
@@ -127,18 +129,15 @@ class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \
      */
     public function getConnection(CommandInterface $command)
     {
-        $hash = $command->getHash();
+        $hash = $this->strategy->getHash($command);
 
-        if (isset($hash)) {
-            return $this->distributor->get($hash);
+        if (!isset($hash)) {
+            throw new NotSupportedException("Cannot use {$command->getId()} with a cluster of connections");
         }
 
-        if ($hash = $this->cmdHasher->getHash($this->distributor, $command)) {
-            $command->setHash($hash);
-            return $this->distributor->get($hash);
-        }
+        $node = $this->distributor->get($hash);
 
-        throw new NotSupportedException("Cannot send {$command->getId()} to a cluster of connections");
+        return $node;
     }
 
     /**
@@ -159,7 +158,7 @@ class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \
      */
     public function getConnectionByKey($key)
     {
-        $hash = $this->cmdHasher->getKeyHash($this->distributor, $key);
+        $hash = $this->strategy->getKeyHash($key);
         $node = $this->distributor->get($hash);
 
         return $node;
@@ -169,11 +168,11 @@ class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \
      * Returns the underlying command hash strategy used to hash
      * commands by their keys.
      *
-     * @return CommandHashStrategy
+     * @return Predis\Cluster\CommandHashStrategyInterface
      */
     public function getCommandHashStrategy()
     {
-        return $this->cmdHasher;
+        return $this->strategy;
     }
 
     /**

+ 8 - 15
lib/Predis/Connection/RedisCluster.php

@@ -14,9 +14,8 @@ namespace Predis\Connection;
 use Predis\ClientException;
 use Predis\NotSupportedException;
 use Predis\ResponseErrorInterface;
+use Predis\Cluster\RedisClusterHashStrategy;
 use Predis\Command\CommandInterface;
-use Predis\Command\Hash\RedisClusterHashStrategy;
-use Predis\Distribution\CRC16HashGenerator;
 
 /**
  * Abstraction for Redis cluster (Redis v3.0).
@@ -29,9 +28,8 @@ class RedisCluster implements ClusterConnectionInterface, \IteratorAggregate, \C
     private $slots;
     private $slotsMap;
     private $slotsPerNode;
+    private $strategy;
     private $connections;
-    private $distributor;
-    private $cmdHasher;
 
     /**
      * @param ConnectionFactoryInterface $connections Connection factory object.
@@ -40,9 +38,8 @@ class RedisCluster implements ClusterConnectionInterface, \IteratorAggregate, \C
     {
         $this->pool = array();
         $this->slots = array();
+        $this->strategy = new RedisClusterHashStrategy();
         $this->connections = $connections ?: new ConnectionFactory();
-        $this->distributor = new CRC16HashGenerator();
-        $this->cmdHasher = new RedisClusterHashStrategy();
     }
 
     /**
@@ -132,6 +129,8 @@ class RedisCluster implements ClusterConnectionInterface, \IteratorAggregate, \C
 
     /**
      * Builds the slots map for the cluster.
+     *
+     * @return array
      */
     public function buildSlotsMap()
     {
@@ -175,16 +174,10 @@ class RedisCluster implements ClusterConnectionInterface, \IteratorAggregate, \C
      */
     public function getConnection(CommandInterface $command)
     {
-        $hash = $command->getHash();
+        $hash = $this->strategy->getHash($command);
 
         if (!isset($hash)) {
-            $hash = $this->cmdHasher->getHash($this->distributor, $command);
-
-            if (!isset($hash)) {
-                throw new NotSupportedException("Cannot send {$command->getId()} commands to redis-cluster");
-            }
-
-            $command->setHash($hash);
+            throw new NotSupportedException("Cannot use {$command->getId()} with redis-cluster");
         }
 
         $slot = $hash & 4095; // 0x0FFF
@@ -283,7 +276,7 @@ class RedisCluster implements ClusterConnectionInterface, \IteratorAggregate, \C
      * Returns the underlying command hash strategy used to hash
      * commands by their keys.
      *
-     * @return CommandHashStrategy
+     * @return Predis\Cluster\CommandHashStrategyInterface
      */
     public function getCommandHashStrategy()
     {

+ 3 - 3
tests/PHPUnit/DistributionStrategyTestCase.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Distribution;
 
 use \PHPUnit_Framework_TestCase as StandardTestCase;
 
@@ -21,7 +21,7 @@ abstract class DistributionStrategyTestCase extends StandardTestCase
     /**
      * Returns a new instance of the tested distributor.
      *
-     * @return Predis\Distribution\DistributionStrategyInterface
+     * @return Predis\Cluster\Distribution\DistributionStrategyInterface
      */
     protected abstract function getDistributorInstance();
 
@@ -49,7 +49,7 @@ abstract class DistributionStrategyTestCase extends StandardTestCase
      */
     public function testEmptyRingThrowsException()
     {
-        $this->setExpectedException('Predis\Distribution\EmptyRingException');
+        $this->setExpectedException('Predis\Cluster\Distribution\EmptyRingException');
 
         $ring = $this->getDistributorInstance();
         $ring->get('nodekey');

+ 2 - 2
tests/Predis/Distribution/EmptyRingExceptionTest.php → tests/Predis/Cluster/Distribution/EmptyRingExceptionTest.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Distribution;
 
 use \PHPUnit_Framework_TestCase as StandardTestCase;
 
@@ -24,7 +24,7 @@ class EmptyRingExceptionTest extends StandardTestCase
     public function testExceptionMessage()
     {
         $message = 'Empty Ring';
-        $this->setExpectedException('Predis\Distribution\EmptyRingException', $message);
+        $this->setExpectedException('Predis\Cluster\Distribution\EmptyRingException', $message);
 
         throw new EmptyRingException($message);
     }

+ 1 - 1
tests/Predis/Distribution/HashRingTest.php → tests/Predis/Cluster/Distribution/HashRingTest.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Distribution;
 
 /**
  * @todo To be improved.

+ 1 - 1
tests/Predis/Distribution/KetamaPureRingTest.php → tests/Predis/Cluster/Distribution/KetamaPureRingTest.php

@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Distribution;
+namespace Predis\Cluster\Distribution;
 
 /**
  * @todo To be improved.

+ 47 - 43
tests/Predis/Command/Hash/PredisClusterHashStrategyTest.php → tests/Predis/Cluster/PredisClusterHashStrategyTest.php

@@ -9,11 +9,11 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Command\Hash;
+namespace Predis\Cluster;
 
 use \PHPUnit_Framework_TestCase as StandardTestCase;
 
-use Predis\Distribution\HashRing;
+use Predis\Cluster\Distribution\HashRing;
 use Predis\Profile\ServerProfile;
 
 /**
@@ -27,16 +27,15 @@ class PredisClusterHashStrategyTest extends StandardTestCase
     public function testSupportsKeyTags()
     {
         $expected = -1938594527;
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
 
-        $this->assertSame($expected, $hashstrategy->getKeyHash($distribution, '{foo}'));
-        $this->assertSame($expected, $hashstrategy->getKeyHash($distribution, '{foo}:bar'));
-        $this->assertSame($expected, $hashstrategy->getKeyHash($distribution, '{foo}:baz'));
-        $this->assertSame($expected, $hashstrategy->getKeyHash($distribution, 'bar:{foo}:bar'));
+        $this->assertSame($expected, $strategy->getKeyHash('{foo}'));
+        $this->assertSame($expected, $strategy->getKeyHash('{foo}:bar'));
+        $this->assertSame($expected, $strategy->getKeyHash('{foo}:baz'));
+        $this->assertSame($expected, $strategy->getKeyHash('bar:{foo}:bar'));
 
-        $this->assertSame(0, $hashstrategy->getKeyHash($distribution, ''));
-        $this->assertSame(0, $hashstrategy->getKeyHash($distribution, '{}'));
+        $this->assertSame(0, $strategy->getKeyHash(''));
+        $this->assertSame(0, $strategy->getKeyHash('{}'));
     }
 
     /**
@@ -44,9 +43,9 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testSupportedCommands()
     {
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
 
-        $this->assertSame($this->getExpectedCommands(), $hashstrategy->getSupportedCommands());
+        $this->assertSame($this->getExpectedCommands(), $strategy->getSupportedCommands());
     }
 
     /**
@@ -54,11 +53,10 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testReturnsNullOnUnsupportedCommand()
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $command = ServerProfile::getDevelopment()->createCommand('ping');
 
-        $this->assertNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNull($strategy->getHash($command));
     }
 
     /**
@@ -66,14 +64,13 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testFirstKeyCommands()
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key');
 
         foreach ($this->getExpectedCommands('keys-first') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -82,14 +79,13 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testAllKeysCommands()
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('{key}:1', '{key}:2', '{key}:3', '{key}:4');
 
         foreach ($this->getExpectedCommands('keys-all') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -98,14 +94,13 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testInterleavedKeysCommands()
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('{key}:1', 'value1', '{key}:2', 'value2');
 
         foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -114,14 +109,13 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testKeysForBlockingListCommands()
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('{key}:1', '{key}:2', 10);
 
         foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -130,14 +124,13 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testKeysForZsetAggregationCommands()
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('{key}:destination', 2, '{key}:1', '{key}:1', array('aggregate' => 'SUM'));
 
         foreach ($this->getExpectedCommands('keys-zaggregated') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -146,14 +139,13 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testKeysForBitOpCommand()
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('AND', '{key}:destination', '{key}:src:1', '{key}:src:2');
 
         foreach ($this->getExpectedCommands('keys-bitop') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -162,18 +154,17 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testUnsettingCommandHandler()
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
 
-        $hashstrategy->setCommandHandler('set');
-        $hashstrategy->setCommandHandler('get', null);
+        $strategy->setCommandHandler('set');
+        $strategy->setCommandHandler('get', null);
 
         $command = $profile->createCommand('set', array('key', 'value'));
-        $this->assertNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNull($strategy->getHash($command));
 
         $command = $profile->createCommand('get', array('key'));
-        $this->assertNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNull($strategy->getHash($command));
     }
 
     /**
@@ -181,8 +172,7 @@ class PredisClusterHashStrategyTest extends StandardTestCase
      */
     public function testSettingCustomCommandHandler()
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
 
         $callable = $this->getMock('stdClass', array('__invoke'));
@@ -191,16 +181,30 @@ class PredisClusterHashStrategyTest extends StandardTestCase
                  ->with($this->isInstanceOf('Predis\Command\CommandInterface'))
                  ->will($this->returnValue('key'));
 
-        $hashstrategy->setCommandHandler('get', $callable);
+        $strategy->setCommandHandler('get', $callable);
 
         $command = $profile->createCommand('get', array('key'));
-        $this->assertNotNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNotNull($strategy->getHash($command));
     }
 
     // ******************************************************************** //
     // ---- HELPER METHODS ------------------------------------------------ //
     // ******************************************************************** //
 
+    /**
+     * Creates the default hash strategy object.
+     *
+     * @return CommandHashStrategyInterface
+     */
+    protected function getHashStrategy()
+    {
+        $distributor = new HashRing();
+        $hashGenerator = $distributor->getHashGenerator();
+        $strategy = new PredisClusterHashStrategy($hashGenerator);
+
+        return $strategy;
+    }
+
     /**
      * Returns the list of expected supported commands.
      *

+ 44 - 44
tests/Predis/Command/Hash/RedisClusterHashStrategyTest.php → tests/Predis/Cluster/RedisClusterHashStrategyTest.php

@@ -9,11 +9,10 @@
  * file that was distributed with this source code.
  */
 
-namespace Predis\Command\Hash;
+namespace Predis\Cluster;
 
 use \PHPUnit_Framework_TestCase as StandardTestCase;
 
-use Predis\Distribution\CRC16HashGenerator;
 use Predis\Profile\ServerProfile;
 
 /**
@@ -26,13 +25,12 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testDoesNotSupportKeyTags()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
 
-        $this->assertSame(35910, $hashstrategy->getKeyHash($distribution, '{foo}'));
-        $this->assertSame(60032, $hashstrategy->getKeyHash($distribution, '{foo}:bar'));
-        $this->assertSame(27528, $hashstrategy->getKeyHash($distribution, '{foo}:baz'));
-        $this->assertSame(34064, $hashstrategy->getKeyHash($distribution, 'bar:{foo}:bar'));
+        $this->assertSame(35910, $strategy->getKeyHash('{foo}'));
+        $this->assertSame(60032, $strategy->getKeyHash('{foo}:bar'));
+        $this->assertSame(27528, $strategy->getKeyHash('{foo}:baz'));
+        $this->assertSame(34064, $strategy->getKeyHash('bar:{foo}:bar'));
     }
 
     /**
@@ -40,9 +38,9 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testSupportedCommands()
     {
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
 
-        $this->assertSame($this->getExpectedCommands(), $hashstrategy->getSupportedCommands());
+        $this->assertSame($this->getExpectedCommands(), $strategy->getSupportedCommands());
     }
 
     /**
@@ -50,11 +48,10 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testReturnsNullOnUnsupportedCommand()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $command = ServerProfile::getDevelopment()->createCommand('ping');
 
-        $this->assertNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNull($strategy->getHash($command));
     }
 
     /**
@@ -62,14 +59,13 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testFirstKeyCommands()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key');
 
         foreach ($this->getExpectedCommands('keys-first') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -78,14 +74,13 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testAllKeysCommandsWithOneKey()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key');
 
         foreach ($this->getExpectedCommands('keys-all') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -94,14 +89,13 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testAllKeysCommandsWithMoreKeys()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key1', 'key2');
 
         foreach ($this->getExpectedCommands('keys-all') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -110,14 +104,13 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testInterleavedKeysCommandsWithOneKey()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key:1', 'value1');
 
         foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -126,14 +119,13 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testInterleavedKeysCommandsWithMoreKeys()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key:1', 'value1', 'key:2', 'value2');
 
         foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -142,14 +134,13 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testKeysForBlockingListCommandsWithOneKey()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key:1', 10);
 
         foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNotNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -158,14 +149,13 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testKeysForBlockingListCommandsWithMoreKeys()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key:1', 'key:2', 10);
 
         foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
-            $this->assertNull($hashstrategy->getHash($distribution, $command), $commandID);
+            $this->assertNull($strategy->getHash($command), $commandID);
         }
     }
 
@@ -174,18 +164,17 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testUnsettingCommandHandler()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
 
-        $hashstrategy->setCommandHandler('set');
-        $hashstrategy->setCommandHandler('get', null);
+        $strategy->setCommandHandler('set');
+        $strategy->setCommandHandler('get', null);
 
         $command = $profile->createCommand('set', array('key', 'value'));
-        $this->assertNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNull($strategy->getHash($command));
 
         $command = $profile->createCommand('get', array('key'));
-        $this->assertNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNull($strategy->getHash($command));
     }
 
     /**
@@ -193,8 +182,7 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
     public function testSettingCustomCommandHandler()
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
 
         $callable = $this->getMock('stdClass', array('__invoke'));
@@ -203,16 +191,28 @@ class RedisClusterHashStrategyTest extends StandardTestCase
                  ->with($this->isInstanceOf('Predis\Command\CommandInterface'))
                  ->will($this->returnValue('key'));
 
-        $hashstrategy->setCommandHandler('get', $callable);
+        $strategy->setCommandHandler('get', $callable);
 
         $command = $profile->createCommand('get', array('key'));
-        $this->assertNotNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNotNull($strategy->getHash($command));
     }
 
     // ******************************************************************** //
     // ---- HELPER METHODS ------------------------------------------------ //
     // ******************************************************************** //
 
+    /**
+     * Creates the default hash strategy object.
+     *
+     * @return CommandHashStrategyInterface
+     */
+    protected function getHashStrategy()
+    {
+        $strategy = new RedisClusterHashStrategy();
+
+        return $strategy;
+    }
+
     /**
      * Returns the list of expected supported commands.
      *

+ 2 - 2
tests/Predis/Connection/PredisClusterTest.php

@@ -26,7 +26,7 @@ class PredisClusterTest extends StandardTestCase
     public function testExposesCommandHashStrategy()
     {
         $cluster = new PredisCluster();
-        $this->assertInstanceOf('Predis\Command\Hash\PredisClusterHashStrategy', $cluster->getCommandHashStrategy());
+        $this->assertInstanceOf('Predis\Cluster\PredisClusterHashStrategy', $cluster->getCommandHashStrategy());
     }
 
     /**
@@ -257,7 +257,7 @@ class PredisClusterTest extends StandardTestCase
     /**
      * @group disconnected
      * @expectedException Predis\NotSupportedException
-     * @expectedExceptionMessage Cannot send PING to a cluster of connections
+     * @expectedExceptionMessage Cannot use PING with a cluster of connections
      */
     public function testThrowsExceptionOnNonShardableCommand()
     {