Skip to content

Commit

Permalink
general code cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
taylorotwell committed Dec 12, 2016
1 parent 1e1da34 commit 464075d
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 107 deletions.
108 changes: 49 additions & 59 deletions src/Illuminate/Redis/PhpRedisDatabase.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class PhpRedisDatabase extends Database
*
* @var array
*/
protected $clients;
public $clients;

/**
* Create a new Redis connection instance.
Expand All @@ -24,72 +24,64 @@ class PhpRedisDatabase extends Database
*/
public function __construct(array $servers = [])
{
$cluster = Arr::pull($servers, 'cluster');
$clusters = (array) Arr::pull($servers, 'clusters');

$options = (array) Arr::pull($servers, 'options');

if ($cluster) {
$this->clients = $this->createAggregateClient($servers, $options);
} else {
$this->clients = $this->createSingleClients($servers, $options);
}
$this->clients = $this->createSingleClients($servers, $options);

$this->createClusters($clusters, $options);
}

/**
* Create multiple clusters (aggregate clients).
* Create an array of single connection clients.
*
* @param array $clusters
* @param array $servers
* @param array $options
* @return void
* @return array
*/
protected function createClusters(array $clusters, array $options = [])
protected function createSingleClients(array $servers, array $options = [])
{
// Merge general options with general cluster options
$options = array_merge($options, (array) Arr::pull($clusters, 'options'));

foreach ($clusters as $connection => $servers) {
// Merge specific cluster options with general options
$options = array_merge($options, (array) Arr::pull($servers, 'options'));
$clients = [];

$this->clients += $this->createAggregateClient($servers, $options, $connection);
foreach ($servers as $key => $server) {
$clients[$key] = $this->createRedisInstance($server, $options);
}

return $clients;
}

/**
* Create a new aggregate client supporting sharding.
* Create multiple clusters (aggregate clients).
*
* @param array $servers
* @param array $clusters
* @param array $options
* @param string $connection
* @return array
* @return void
*/
protected function createAggregateClient(array $servers, array $options = [], $connection = 'default')
protected function createClusters(array $clusters, array $options = [])
{
$servers = array_map([$this, 'buildClusterSeed'], $servers);
$options = array_merge($options, (array) Arr::pull($clusters, 'options'));

return [$connection => $this->createRedisClusterInstance($servers, $options)];
foreach ($clusters as $name => $servers) {
$this->clients += $this->createAggregateClient($name, $servers, array_merge(
$options, (array) Arr::pull($servers, 'options')
));
}
}

/**
* Create an array of single connection clients.
* Create a new aggregate client supporting sharding.
*
* @param string $name
* @param array $servers
* @param array $options
* @return array
*/
protected function createSingleClients(array $servers, array $options = [])
protected function createAggregateClient($name, array $servers, array $options = [])
{
$clients = [];

foreach ($servers as $key => $server) {
$client = $this->createRedisInstance($server, $options);
$clients[$key] = $client;
}
$servers = array_map([$this, 'buildClusterConnectionString'], $servers);

return $clients;
return [$name => $this->createRedisClusterInstance($servers, $options)];
}

/**
Expand All @@ -98,13 +90,11 @@ protected function createSingleClients(array $servers, array $options = [])
* @param array $server
* @return string
*/
protected function buildClusterSeed(array $server)
protected function buildClusterConnectionString(array $server)
{
$parameters = Arr::only($server, [
return $server['host'].':'.$server['port'].'?'.http_build_query(Arr::only($server, [
'database', 'password', 'prefix', 'read_timeout',
]);

return $server['host'].':'.$server['port'].'?'.http_build_query($parameters);
]));
}

/**
Expand Down Expand Up @@ -134,34 +124,16 @@ public function psubscribe($channels, Closure $callback, $connection = null)
$this->subscribe($channels, $callback, $connection, __FUNCTION__);
}

/**
* Create a new redis cluster instance.
*
* @param array $servers
* @param array $options
* @return RedisCluster
*/
protected function createRedisClusterInstance(array $servers, array $options)
{
return new RedisCluster(
null,
array_values($servers),
Arr::get($options, 'timeout', 0),
Arr::get($options, 'read_timeout', 0),
isset($options['persistent']) && $options['persistent']
);
}

/**
* Create a new redis instance.
*
* @param array $server
* @param array $options
* @return Redis
* @return \Redis
*/
protected function createRedisInstance(array $server, array $options)
{
$client = new Redis();
$client = new Redis;

$timeout = empty($server['timeout']) ? 0 : $server['timeout'];

Expand Down Expand Up @@ -189,4 +161,22 @@ protected function createRedisInstance(array $server, array $options)

return $client;
}

/**
* Create a new redis cluster instance.
*
* @param array $servers
* @param array $options
* @return \RedisCluster
*/
protected function createRedisClusterInstance(array $servers, array $options)
{
return new RedisCluster(
null,
array_values($servers),
Arr::get($options, 'timeout', 0),
Arr::get($options, 'read_timeout', 0),
isset($options['persistent']) && $options['persistent']
);
}
}
59 changes: 26 additions & 33 deletions src/Illuminate/Redis/PredisDatabase.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class PredisDatabase extends Database
*
* @var array
*/
protected $clients;
public $clients;

/**
* Create a new Redis connection instance.
Expand All @@ -23,69 +23,62 @@ class PredisDatabase extends Database
*/
public function __construct(array $servers = [])
{
$cluster = (bool) Arr::pull($servers, 'cluster');
$clusters = (array) Arr::pull($servers, 'clusters');

$options = array_merge(['timeout' => 10.0], (array) Arr::pull($servers, 'options'));

if ($cluster) {
$this->clients = $this->createAggregateClient($servers, $options);
} else {
$this->clients = $this->createSingleClients($servers, $options);
}
$this->clients = $this->createSingleClients($servers, $options);

$this->createClusters($clusters, $options);
}

/**
* Create multiple clusters (aggregate clients).
* Create an array of single connection clients.
*
* @param array $clusters
* @param array $servers
* @param array $options
* @return void
* @return array
*/
protected function createClusters(array $clusters, array $options = [])
protected function createSingleClients(array $servers, array $options = [])
{
// Merge general options with general cluster options
$options = array_merge($options, (array) Arr::pull($clusters, 'options'));

foreach ($clusters as $connection => $servers) {
// Merge specific cluster options with general options
$options = array_merge($options, (array) Arr::pull($servers, 'options'));
$clients = [];

$this->clients += $this->createAggregateClient($servers, $options, $connection);
foreach ($servers as $key => $server) {
$clients[$key] = new Client($server, $options);
}

return $clients;
}

/**
* Create a new aggregate client supporting sharding.
* Create multiple clusters (aggregate clients).
*
* @param array $servers
* @param array $clusters
* @param array $options
* @param string $connection
* @return array
* @return void
*/
protected function createAggregateClient(array $servers, array $options = [], $connection = 'default')
protected function createClusters(array $clusters, array $options = [])
{
return [$connection => new Client(array_values($servers), $options)];
$options = array_merge($options, (array) Arr::pull($clusters, 'options'));

foreach ($clusters as $name => $servers) {
$this->clients += $this->createAggregateClient($name, $servers, array_merge(
$options, (array) Arr::pull($servers, 'options')
));
}
}

/**
* Create an array of single connection clients.
* Create a new aggregate client supporting sharding.
*
* @param string $name
* @param array $servers
* @param array $options
* @return array
*/
protected function createSingleClients(array $servers, array $options = [])
protected function createAggregateClient($name, array $servers, array $options = [])
{
$clients = [];

foreach ($servers as $key => $server) {
$clients[$key] = new Client($server, $options);
}

return $clients;
return [$name => new Client(array_values($servers), $options)];
}

/**
Expand Down
12 changes: 4 additions & 8 deletions tests/Redis/PhpRedisConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class PhpRedisConnectionTest extends PHPUnit_Framework_TestCase
{
public function testPhpRedisNotCreateClusterAndOptionsAndClustersServer()
{
$redis = $this->getRedis(false);
$redis = $this->getRedis();

$client = $redis->connection('cluster');
$this->assertNull($client, 'cluster parameter should not create as redis server');
Expand All @@ -20,11 +20,8 @@ public function testPhpRedisNotCreateClusterAndOptionsAndClustersServer()

public function testPhpRedisClusterNotCreateClusterAndOptionsServer()
{
$redis = $this->getRedis(true);

$client = $redis->connection();

$this->assertInstanceOf(RedisClusterStub::class, $client);
$redis = $this->getRedis();
$this->assertEquals(['default', 'cluster-1', 'cluster-2'], array_keys($redis->clients));
}

public function testPhpRedisClusterCreateMultipleClustersAndNotCreateOptionsServer()
Expand All @@ -41,10 +38,9 @@ public function testPhpRedisClusterCreateMultipleClustersAndNotCreateOptionsServ
$this->assertNull($client, 'options parameter should not create as redis server');
}

protected function getRedis($cluster = false)
protected function getRedis()
{
$servers = [
'cluster' => $cluster,
'default' => [
'host' => '127.0.0.1',
'port' => 6379,
Expand Down
11 changes: 4 additions & 7 deletions tests/Redis/RedisConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class RedisConnectionTest extends PHPUnit_Framework_TestCase
{
public function testRedisNotCreateClusterAndOptionsAndClustersServer()
{
$redis = $this->getRedis(false);
$redis = $this->getRedis();

$client = $redis->connection('cluster');
$this->assertNull($client, 'cluster parameter should not create as redis server');
Expand All @@ -18,10 +18,8 @@ public function testRedisNotCreateClusterAndOptionsAndClustersServer()

public function testRedisClusterNotCreateClusterAndOptionsServer()
{
$redis = $this->getRedis(true);
$client = $redis->connection();

$this->assertCount(1, $client->getConnection());
$redis = $this->getRedis();
$this->assertEquals(['default', 'cluster-1', 'cluster-2'], array_keys($redis->clients));
}

public function testRedisClusterCreateMultipleClustersAndNotCreateOptionsServer()
Expand All @@ -37,10 +35,9 @@ public function testRedisClusterCreateMultipleClustersAndNotCreateOptionsServer(
$this->assertNull($client, 'options parameter should not create as redis server');
}

protected function getRedis($cluster = false)
protected function getRedis()
{
$servers = [
'cluster' => $cluster,
'default' => [
'host' => '127.0.0.1',
'port' => 6379,
Expand Down

0 comments on commit 464075d

Please sign in to comment.