Browse Source

Move the hash generator inside the command hash strategy class.

This should make things a tiny bit faster (we are speaking about micro
optimizations anyway) but more  importantly it results in a better
encapsulation.
Daniele Alessandri 13 years ago
parent
commit
7683f97ff2

+ 7 - 1
examples/CustomDistributionStrategy.php

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

+ 2 - 4
lib/Predis/Command/Hash/CommandHashStrategyInterface.php

@@ -28,18 +28,16 @@ interface CommandHashStrategyInterface
      * Returns the hash for the given command using the specified algorithm, or null
      * Returns the hash for the given command using the specified algorithm, or null
      * if the command cannot be hashed.
      * if the command cannot be hashed.
      *
      *
-     * @param HashGeneratorInterface $hasher Hash algorithm.
      * @param CommandInterface $command Command to be hashed.
      * @param CommandInterface $command Command to be hashed.
      * @return int
      * @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.
      * Returns the hash for the given key using the specified algorithm.
      *
      *
-     * @param HashGeneratorInterface $hasher Hash algorithm.
      * @param string $key Key to be hashed.
      * @param string $key Key to be hashed.
      * @return string
      * @return string
      */
      */
-    public function getKeyHash(HashGeneratorInterface $hasher, $key);
+    public function getKeyHash($key);
 }
 }

+ 14 - 7
lib/Predis/Command/Hash/PredisClusterHashStrategy.php

@@ -23,13 +23,15 @@ use Predis\Distribution\HashGeneratorInterface;
 class PredisClusterHashStrategy implements CommandHashStrategyInterface
 class PredisClusterHashStrategy implements CommandHashStrategyInterface
 {
 {
     private $commands;
     private $commands;
+    private $hashGenerator;
 
 
     /**
     /**
-     *
+     * @param HashGeneratorInterface $hashGenerator Hash generator instance.
      */
      */
-    public function __construct()
+    public function __construct(HashGeneratorInterface $hashGenerator)
     {
     {
         $this->commands = $this->getDefaultCommands();
         $this->commands = $this->getDefaultCommands();
+        $this->hashGenerator = $hashGenerator;
     }
     }
 
 
     /**
     /**
@@ -282,22 +284,27 @@ class PredisClusterHashStrategy implements CommandHashStrategyInterface
     /**
     /**
      * {@inheritdoc}
      * {@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)) {
             if ($key = call_user_func($this->commands[$cmdID], $command)) {
-                return $this->getKeyHash($hasher, $key);
+                $hash = $this->getKeyHash($key);
+                $command->setHash($hash);
             }
             }
         }
         }
+
+        return $hash;
     }
     }
 
 
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */
-    public function getKeyHash(HashGeneratorInterface $hasher, $key)
+    public function getKeyHash($key)
     {
     {
         $key = $this->extractKeyTag($key);
         $key = $this->extractKeyTag($key);
-        $hash = $hasher->hash($key);
+        $hash = $this->hashGenerator->hash($key);
 
 
         return $hash;
         return $hash;
     }
     }

+ 14 - 7
lib/Predis/Command/Hash/RedisClusterHashStrategy.php

@@ -12,7 +12,7 @@
 namespace Predis\Command\Hash;
 namespace Predis\Command\Hash;
 
 
 use Predis\Command\CommandInterface;
 use Predis\Command\CommandInterface;
-use Predis\Distribution\HashGeneratorInterface;
+use Predis\Distribution\CRC16HashGenerator;
 
 
 /**
 /**
  * Default class used by Predis to calculate hashes out of keys of
  * Default class used by Predis to calculate hashes out of keys of
@@ -23,6 +23,7 @@ use Predis\Distribution\HashGeneratorInterface;
 class RedisClusterHashStrategy implements CommandHashStrategyInterface
 class RedisClusterHashStrategy implements CommandHashStrategyInterface
 {
 {
     private $commands;
     private $commands;
+    private $hashGenerator;
 
 
     /**
     /**
      *
      *
@@ -30,6 +31,7 @@ class RedisClusterHashStrategy implements CommandHashStrategyInterface
     public function __construct()
     public function __construct()
     {
     {
         $this->commands = $this->getDefaultCommands();
         $this->commands = $this->getDefaultCommands();
+        $this->hashGenerator = new CRC16HashGenerator();
     }
     }
 
 
     /**
     /**
@@ -235,20 +237,25 @@ class RedisClusterHashStrategy implements CommandHashStrategyInterface
     /**
     /**
      * {@inheritdoc}
      * {@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}
      * {@inheritdoc}
      */
      */
-    public function getKeyHash(HashGeneratorInterface $hasher, $key)
+    public function getKeyHash($key)
     {
     {
-        return $hasher->hash($key);
+        return $this->hashGenerator->hash($key);
     }
     }
 }
 }

+ 12 - 13
lib/Predis/Connection/PredisCluster.php

@@ -28,17 +28,19 @@ use Predis\Distribution\DistributionStrategyInterface;
 class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \Countable
 class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \Countable
 {
 {
     private $pool;
     private $pool;
+    private $strategy;
     private $distributor;
     private $distributor;
-    private $cmdHasher;
 
 
     /**
     /**
      * @param DistributionStrategyInterface $distributor Distribution strategy used by the cluster.
      * @param DistributionStrategyInterface $distributor Distribution strategy used by the cluster.
      */
      */
     public function __construct(DistributionStrategyInterface $distributor = null)
     public function __construct(DistributionStrategyInterface $distributor = null)
     {
     {
+        $distributor = $distributor ?: new HashRing();
+
         $this->pool = array();
         $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)
     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)
     public function getConnectionByKey($key)
     {
     {
-        $hash = $this->cmdHasher->getKeyHash($this->distributor, $key);
+        $hash = $this->strategy->getKeyHash($key);
         $node = $this->distributor->get($hash);
         $node = $this->distributor->get($hash);
 
 
         return $node;
         return $node;
@@ -173,7 +172,7 @@ class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \
      */
      */
     public function getCommandHashStrategy()
     public function getCommandHashStrategy()
     {
     {
-        return $this->cmdHasher;
+        return $this->strategy;
     }
     }
 
 
     /**
     /**

+ 7 - 12
lib/Predis/Connection/RedisCluster.php

@@ -15,6 +15,7 @@ use Predis\ClientException;
 use Predis\NotSupportedException;
 use Predis\NotSupportedException;
 use Predis\ResponseErrorInterface;
 use Predis\ResponseErrorInterface;
 use Predis\Command\CommandInterface;
 use Predis\Command\CommandInterface;
+use Predis\Command\Hash\CommandHashStrategyInterface;
 use Predis\Command\Hash\RedisClusterHashStrategy;
 use Predis\Command\Hash\RedisClusterHashStrategy;
 use Predis\Distribution\CRC16HashGenerator;
 use Predis\Distribution\CRC16HashGenerator;
 
 
@@ -28,9 +29,8 @@ class RedisCluster implements ClusterConnectionInterface, \IteratorAggregate, \C
     private $pool;
     private $pool;
     private $slots;
     private $slots;
     private $slotsMap;
     private $slotsMap;
+    private $strategy;
     private $connections;
     private $connections;
-    private $distributor;
-    private $cmdHasher;
 
 
     /**
     /**
      * @param ConnectionFactoryInterface $connections Connection factory object.
      * @param ConnectionFactoryInterface $connections Connection factory object.
@@ -39,9 +39,8 @@ class RedisCluster implements ClusterConnectionInterface, \IteratorAggregate, \C
     {
     {
         $this->pool = array();
         $this->pool = array();
         $this->slots = array();
         $this->slots = array();
+        $this->strategy = new RedisClusterHashStrategy();
         $this->connections = $connections ?: new ConnectionFactory();
         $this->connections = $connections ?: new ConnectionFactory();
-        $this->distributor = new CRC16HashGenerator();
-        $this->cmdHasher = new RedisClusterHashStrategy();
     }
     }
 
 
     /**
     /**
@@ -121,6 +120,8 @@ class RedisCluster implements ClusterConnectionInterface, \IteratorAggregate, \C
 
 
     /**
     /**
      * Builds the slots map for the cluster.
      * Builds the slots map for the cluster.
+     *
+     * @return array
      */
      */
     public function buildSlotsMap()
     public function buildSlotsMap()
     {
     {
@@ -163,16 +164,10 @@ class RedisCluster implements ClusterConnectionInterface, \IteratorAggregate, \C
      */
      */
     public function getConnection(CommandInterface $command)
     public function getConnection(CommandInterface $command)
     {
     {
-        $hash = $command->getHash();
+        $hash = $this->strategy->getHash($command);
 
 
         if (!isset($hash)) {
         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
         $slot = $hash & 4095; // 0x0FFF

+ 8 - 1
lib/Predis/Distribution/DistributionStrategyInterface.php

@@ -17,7 +17,7 @@ namespace Predis\Distribution;
  *
  *
  * @author Daniele Alessandri <suppakilla@gmail.com>
  * @author Daniele Alessandri <suppakilla@gmail.com>
  */
  */
-interface DistributionStrategyInterface extends HashGeneratorInterface
+interface DistributionStrategyInterface
 {
 {
     /**
     /**
      * Adds a node to the distributor with an optional weight.
      * Adds a node to the distributor with an optional weight.
@@ -40,4 +40,11 @@ interface DistributionStrategyInterface extends HashGeneratorInterface
      * @return mixed
      * @return mixed
      */
      */
     public function get($key);
     public function get($key);
+
+    /**
+     * Returns the underlying hash generator instance.
+     *
+     * @return HashGeneratorInterface
+     */
+    public function getHashGenerator();
 }
 }

+ 9 - 1
lib/Predis/Distribution/HashRing.php

@@ -19,7 +19,7 @@ namespace Predis\Distribution;
  * @author Daniele Alessandri <suppakilla@gmail.com>
  * @author Daniele Alessandri <suppakilla@gmail.com>
  * @author Lorenzo Castelli <lcastelli@gmail.com>
  * @author Lorenzo Castelli <lcastelli@gmail.com>
  */
  */
-class HashRing implements DistributionStrategyInterface
+class HashRing implements DistributionStrategyInterface, HashGeneratorInterface
 {
 {
     const DEFAULT_REPLICAS = 128;
     const DEFAULT_REPLICAS = 128;
     const DEFAULT_WEIGHT   = 100;
     const DEFAULT_WEIGHT   = 100;
@@ -227,4 +227,12 @@ class HashRing implements DistributionStrategyInterface
         // equal to the key. If no such item exists, return the last item.
         // equal to the key. If no such item exists, return the last item.
         return $upper >= 0 ? $upper : $ringKeysCount - 1;
         return $upper >= 0 ? $upper : $ringKeysCount - 1;
     }
     }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getHashGenerator()
+    {
+        return $this;
+    }
 }
 }

+ 45 - 41
tests/Predis/Command/Hash/PredisClusterHashStrategyTest.php

@@ -27,16 +27,15 @@ class PredisClusterHashStrategyTest extends StandardTestCase
     public function testSupportsKeyTags()
     public function testSupportsKeyTags()
     {
     {
         $expected = -1938594527;
         $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()
     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()
     public function testReturnsNullOnUnsupportedCommand()
     {
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $command = ServerProfile::getDevelopment()->createCommand('ping');
         $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()
     public function testFirstKeyCommands()
     {
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key');
         $arguments = array('key');
 
 
         foreach ($this->getExpectedCommands('keys-first') as $commandID) {
         foreach ($this->getExpectedCommands('keys-first') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testAllKeysCommands()
     {
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('{key}:1', '{key}:2', '{key}:3', '{key}:4');
         $arguments = array('{key}:1', '{key}:2', '{key}:3', '{key}:4');
 
 
         foreach ($this->getExpectedCommands('keys-all') as $commandID) {
         foreach ($this->getExpectedCommands('keys-all') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testInterleavedKeysCommands()
     {
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('{key}:1', 'value1', '{key}:2', 'value2');
         $arguments = array('{key}:1', 'value1', '{key}:2', 'value2');
 
 
         foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
         foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testKeysForBlockingListCommands()
     {
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('{key}:1', '{key}:2', 10);
         $arguments = array('{key}:1', '{key}:2', 10);
 
 
         foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
         foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testKeysForZsetAggregationCommands()
     {
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('{key}:destination', 2, '{key}:1', '{key}:1', array('aggregate' => 'SUM'));
         $arguments = array('{key}:destination', 2, '{key}:1', '{key}:1', array('aggregate' => 'SUM'));
 
 
         foreach ($this->getExpectedCommands('keys-zaggregated') as $commandID) {
         foreach ($this->getExpectedCommands('keys-zaggregated') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testKeysForBitOpCommand()
     {
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('AND', '{key}:destination', '{key}:src:1', '{key}:src:2');
         $arguments = array('AND', '{key}:destination', '{key}:src:1', '{key}:src:2');
 
 
         foreach ($this->getExpectedCommands('keys-bitop') as $commandID) {
         foreach ($this->getExpectedCommands('keys-bitop') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testUnsettingCommandHandler()
     {
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
 
 
-        $hashstrategy->setCommandHandler('set');
-        $hashstrategy->setCommandHandler('get', null);
+        $strategy->setCommandHandler('set');
+        $strategy->setCommandHandler('get', null);
 
 
         $command = $profile->createCommand('set', array('key', 'value'));
         $command = $profile->createCommand('set', array('key', 'value'));
-        $this->assertNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNull($strategy->getHash($command));
 
 
         $command = $profile->createCommand('get', array('key'));
         $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()
     public function testSettingCustomCommandHandler()
     {
     {
-        $distribution = new HashRing();
-        $hashstrategy = new PredisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
 
 
         $callable = $this->getMock('stdClass', array('__invoke'));
         $callable = $this->getMock('stdClass', array('__invoke'));
@@ -191,16 +181,30 @@ class PredisClusterHashStrategyTest extends StandardTestCase
                  ->with($this->isInstanceOf('Predis\Command\CommandInterface'))
                  ->with($this->isInstanceOf('Predis\Command\CommandInterface'))
                  ->will($this->returnValue('key'));
                  ->will($this->returnValue('key'));
 
 
-        $hashstrategy->setCommandHandler('get', $callable);
+        $strategy->setCommandHandler('get', $callable);
 
 
         $command = $profile->createCommand('get', array('key'));
         $command = $profile->createCommand('get', array('key'));
-        $this->assertNotNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNotNull($strategy->getHash($command));
     }
     }
 
 
     // ******************************************************************** //
     // ******************************************************************** //
     // ---- HELPER METHODS ------------------------------------------------ //
     // ---- 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.
      * Returns the list of expected supported commands.
      *
      *

+ 43 - 43
tests/Predis/Command/Hash/RedisClusterHashStrategyTest.php

@@ -13,7 +13,6 @@ namespace Predis\Command\Hash;
 
 
 use \PHPUnit_Framework_TestCase as StandardTestCase;
 use \PHPUnit_Framework_TestCase as StandardTestCase;
 
 
-use Predis\Distribution\CRC16HashGenerator;
 use Predis\Profile\ServerProfile;
 use Predis\Profile\ServerProfile;
 
 
 /**
 /**
@@ -26,13 +25,12 @@ class RedisClusterHashStrategyTest extends StandardTestCase
      */
      */
     public function testDoesNotSupportKeyTags()
     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()
     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()
     public function testReturnsNullOnUnsupportedCommand()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $command = ServerProfile::getDevelopment()->createCommand('ping');
         $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()
     public function testFirstKeyCommands()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key');
         $arguments = array('key');
 
 
         foreach ($this->getExpectedCommands('keys-first') as $commandID) {
         foreach ($this->getExpectedCommands('keys-first') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testAllKeysCommandsWithOneKey()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key');
         $arguments = array('key');
 
 
         foreach ($this->getExpectedCommands('keys-all') as $commandID) {
         foreach ($this->getExpectedCommands('keys-all') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testAllKeysCommandsWithMoreKeys()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key1', 'key2');
         $arguments = array('key1', 'key2');
 
 
         foreach ($this->getExpectedCommands('keys-all') as $commandID) {
         foreach ($this->getExpectedCommands('keys-all') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testInterleavedKeysCommandsWithOneKey()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key:1', 'value1');
         $arguments = array('key:1', 'value1');
 
 
         foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
         foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testInterleavedKeysCommandsWithMoreKeys()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key:1', 'value1', 'key:2', 'value2');
         $arguments = array('key:1', 'value1', 'key:2', 'value2');
 
 
         foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
         foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testKeysForBlockingListCommandsWithOneKey()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key:1', 10);
         $arguments = array('key:1', 10);
 
 
         foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
         foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testKeysForBlockingListCommandsWithMoreKeys()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
         $arguments = array('key:1', 'key:2', 10);
         $arguments = array('key:1', 'key:2', 10);
 
 
         foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
         foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
             $command = $profile->createCommand($commandID, $arguments);
             $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()
     public function testUnsettingCommandHandler()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
 
 
-        $hashstrategy->setCommandHandler('set');
-        $hashstrategy->setCommandHandler('get', null);
+        $strategy->setCommandHandler('set');
+        $strategy->setCommandHandler('get', null);
 
 
         $command = $profile->createCommand('set', array('key', 'value'));
         $command = $profile->createCommand('set', array('key', 'value'));
-        $this->assertNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNull($strategy->getHash($command));
 
 
         $command = $profile->createCommand('get', array('key'));
         $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()
     public function testSettingCustomCommandHandler()
     {
     {
-        $distribution = new CRC16HashGenerator();
-        $hashstrategy = new RedisClusterHashStrategy();
+        $strategy = $this->getHashStrategy();
         $profile = ServerProfile::getDevelopment();
         $profile = ServerProfile::getDevelopment();
 
 
         $callable = $this->getMock('stdClass', array('__invoke'));
         $callable = $this->getMock('stdClass', array('__invoke'));
@@ -203,16 +191,28 @@ class RedisClusterHashStrategyTest extends StandardTestCase
                  ->with($this->isInstanceOf('Predis\Command\CommandInterface'))
                  ->with($this->isInstanceOf('Predis\Command\CommandInterface'))
                  ->will($this->returnValue('key'));
                  ->will($this->returnValue('key'));
 
 
-        $hashstrategy->setCommandHandler('get', $callable);
+        $strategy->setCommandHandler('get', $callable);
 
 
         $command = $profile->createCommand('get', array('key'));
         $command = $profile->createCommand('get', array('key'));
-        $this->assertNotNull($hashstrategy->getHash($distribution, $command));
+        $this->assertNotNull($strategy->getHash($command));
     }
     }
 
 
     // ******************************************************************** //
     // ******************************************************************** //
     // ---- HELPER METHODS ------------------------------------------------ //
     // ---- 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.
      * Returns the list of expected supported commands.
      *
      *

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

@@ -257,7 +257,7 @@ class PredisClusterTest extends StandardTestCase
     /**
     /**
      * @group disconnected
      * @group disconnected
      * @expectedException Predis\NotSupportedException
      * @expectedException Predis\NotSupportedException
-     * @expectedExceptionMessage Cannot send PING to a cluster of connections
+     * @expectedExceptionMessage Cannot use PING with a cluster of connections
      */
      */
     public function testThrowsExceptionOnNonShardableCommand()
     public function testThrowsExceptionOnNonShardableCommand()
     {
     {