Skip to content

Commit dec46c3

Browse files
jeffreyzantjeffreyzant
committed
improving readability and integration tests
1 parent 81e95f9 commit dec46c3

File tree

6 files changed

+55
-38
lines changed

6 files changed

+55
-38
lines changed

‎src/Connectors/PhpRedisConnector.php‎

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Illuminate\Support\Arr;
66
use Illuminate\Redis\Connectors\PhpRedisConnector as LaravelPhpRedisConnector;
77
use Monospice\LaravelRedisSentinel\Connections\PhpRedisConnection;
8+
use Redis;
89
use RedisSentinel;
910
use RedisException;
1011

@@ -85,9 +86,8 @@ public function connect(array $servers, array $options = [ ])
8586
/**
8687
* Create the Redis client instance
8788
*
88-
* @param array $servers
8989
* @param array $options
90-
* @return \Redis
90+
* @return Redis
9191
*/
9292
protected function createClientWithSentinel(array $options)
9393
{
@@ -96,12 +96,13 @@ protected function createClientWithSentinel(array $options)
9696
$persistent = isset($options['sentinel_peristent']) ? $options['sentinel_peristent'] : null;
9797
$retryWait = isset($options['retry_wait']) ? $options['retry_wait'] : 0;
9898
$readTimeout = isset($options['sentinel_read_timeout']) ? $options['sentinel_read_timeout'] : 0;
99+
$parameters = isset($options['parameters']) ? $options['parameters'] : [];
99100

100101
// Shuffle the servers to perform some loadbalancing.
101102
shuffle($servers);
102103

103104
// Try to connect to any of the servers.
104-
foreach ($servers as $server) {
105+
foreach ($servers as $idx => $server) {
105106
$host = isset($server['host']) ? $server['host'] : 'localhost';
106107
$port = isset($server['port']) ? $server['port'] : 26739;
107108
$service = isset($options['service']) ? $options['service'] : 'mymaster';
@@ -114,43 +115,56 @@ protected function createClientWithSentinel(array $options)
114115
// Put the current server and the other sentinels in the server list.
115116
$updateSentinels = isset($options['update_sentinels']) ? $options['update_sentinels'] : false;
116117
if ($updateSentinels === true) {
117-
$this->servers = array_merge(
118-
[
119-
[
120-
'host' => $host,
121-
'port' => $port,
122-
]
123-
], array_map(fn ($sentinel) => [
124-
'host' => $sentinel['ip'],
125-
'port' => $sentinel['port'],
126-
], $sentinel->sentinels($service))
127-
);
118+
$this->updateSentinels($sentinel, $host, $port, $service);
128119
}
129120

130121
// Lookup the master node.
131122
$master = $sentinel->getMasterAddrByName($service);
132-
if (! is_array($master) || ! count($master)) {
123+
if (is_array($master) && ! count($master)) {
133124
throw new RedisException(sprintf('No master found for service "%s".', $service));
134125
}
135126

136127
// Create a PhpRedis client for the selected master node.
137-
return $this->createClient(array_merge(
138-
isset($options['parameters']) ? $options['parameters'] : [],
139-
$server,
140-
['host' => $master[0], 'port' => $master[1]]
141-
));
128+
return $this->createClient(
129+
array_merge($parameters, $server, ['host' => $master[0], 'port' => $master[1]])
130+
);
142131
} catch (RedisException $e) {
143-
//
132+
// Rethrow the expection when the last server is reached.
133+
if ($idx === count($servers) - 1) {
134+
throw $e;
135+
}
144136
}
145137
}
138+
}
146139

147-
throw new RedisException('Could not create a client for the configured Sentinel servers.');
140+
/**
141+
* Update the list With sentinel servers.
142+
*
143+
* @param RedisSentinel $sentinel
144+
* @param string $currentHost
145+
* @param int $currentPort
146+
* @param string $service
147+
* @return void
148+
*/
149+
private function updateSentinels(RedisSentinel $sentinel, string $currentHost, int $currentPort, string $service)
150+
{
151+
$this->servers = array_merge(
152+
[
153+
[
154+
'host' => $currentHost,
155+
'port' => $currentPort,
156+
]
157+
], array_map(fn ($sentinel) => [
158+
'host' => $sentinel['ip'],
159+
'port' => $sentinel['port'],
160+
], $sentinel->sentinels($service))
161+
);
148162
}
149163

150164
/**
151165
* Format a server.
152166
*
153-
* @param array $server
167+
* @param mixed $server
154168
* @return array
155169
*
156170
* @throws RedisException
@@ -159,8 +173,11 @@ private function formatServer($server)
159173
{
160174
if (is_string($server)) {
161175
list($host, $port) = explode(':', $server);
176+
if (! $host || ! $port) {
177+
throw new RedisException('Could not format the server definition.');
178+
}
162179

163-
return ['host' => $host, 'port' => $port];
180+
return ['host' => $host, 'port' => (int) $port];
164181
}
165182

166183
if (! is_array($server)) {

‎tests/Integration/Connections/PhpRedisConnectionTest.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function setUp()
2929
{
3030
parent::setUp();
3131

32-
$this->subject = $this->makeClient();
32+
$this->subject = $this->makeConnection();
3333
}
3434

3535
/**
@@ -99,7 +99,7 @@ public function testRetriesTransactionWhenConnectionFails()
9999

100100
$expectedRetries = 2;
101101

102-
$this->subject = $this->makeClient($expectedRetries, 0); // retry immediately
102+
$this->subject = $this->makeConnection($expectedRetries, 0); // retry immediately
103103

104104
$this->subject->transaction(function () {
105105
throw new RedisException();
@@ -111,7 +111,7 @@ public function testCanReconnectWhenConnectionFails()
111111
$retries = 3;
112112
$attempts = 0;
113113

114-
$this->subject = $this->makeClient($retries, 0); // retry immediately
114+
$this->subject = $this->makeConnection($retries, 0); // retry immediately
115115

116116
$this->subject->transaction(function ($trans) use (&$attempts, $retries) {
117117
$attempts++;
@@ -133,9 +133,9 @@ public function testCanReconnectWhenConnectionFails()
133133
*
134134
* @param int|null $retryLimit
135135
* @param int|null $retryWait
136-
* @return Redis A client instance for the subject under test.
136+
* @return PhpRedisConnection A client instance for the subject under test.
137137
*/
138-
protected function makeClient(int $retryLimit = null, int $retryWait = null)
138+
protected function makeConnection(int $retryLimit = null, int $retryWait = null)
139139
{
140140
$connector = new PhpRedisConnector();
141141

‎tests/Unit/Configuration/LoaderTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,9 +499,9 @@ public function testSkipsLoadingHorizonConfigurationIfCached()
499499

500500
public function testDisallowsUsingNonexistantConnectionForHorizon()
501501
{
502-
$this->config->set('horizon.use', 'not-a-connection');
502+
$this->expectException(UnexpectedValueException::class);
503503

504-
$this->setExpectedException(UnexpectedValueException::class);
504+
$this->config->set('horizon.use', 'not-a-connection');
505505

506506
$this->loader->loadHorizonConfiguration();
507507
}

‎tests/Unit/Connections/PredisConnectionTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public function testSetsSentinelConnectionOptionsFromConfig()
102102

103103
public function testDisallowsInvalidSentinelOptions()
104104
{
105-
$this->setExpectedException(BadMethodCallException::class);
105+
$this->expectException(BadMethodCallException::class);
106106

107107
new PredisConnection($this->clientMock, [ 'not_an_option' => null ]);
108108
}

‎tests/Unit/Manager/VersionedRedisSentinelManagerTest.php‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace Monospice\LaravelRedisSentinel\Tests\Unit;
44

5-
use Closure;
65
use Illuminate\Contracts\Container\Container;
76
use Illuminate\Contracts\Redis\Factory as RedisFactory;
87
use Illuminate\Redis\RedisManager;
@@ -118,23 +117,23 @@ public function testCreatesSingleClientsWithIndividualConfig()
118117

119118
public function testDisallowsRedisClusterConnections()
120119
{
121-
$this->setExpectedException(InvalidArgumentException::class);
120+
$this->expectException(InvalidArgumentException::class);
122121

123122
$this->subject->connection('clustered_connection');
124123
}
125124

126125
public function testFailsOnUndefinedConnection()
127126
{
128-
$this->setExpectedException(InvalidArgumentException::class);
127+
$this->expectException(InvalidArgumentException::class);
129128

130129
$this->subject->connection('nonexistant_connection');
131130
}
132131

133132
public function testFailsOnUnsupportedClientDriver()
134133
{
135-
$this->setExpectedException(InvalidArgumentException::class);
134+
$this->expectException(InvalidArgumentException::class);
136135

137-
$manager = $this->makeSubject('phpredis', [
136+
$manager = $this->makeSubject('fakeredis', [
138137
'test_connection' => [ ],
139138
]);
140139

‎tests/Unit/RedisSentinelServiceProviderTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,11 @@ public function testBootExtendsSessionHandlers()
200200

201201
public function testWaitsForBoot()
202202
{
203+
$this->expectException(\InvalidArgumentException::class);
204+
203205
$this->app->config->set('redis-sentinel.auto_boot', false);
204206
$this->provider->register();
205207

206-
$this->setExpectedException(\InvalidArgumentException::class);
207208

208209
// It didn't auto boot
209210
$this->assertNull($this->app->cache->store('redis-sentinel'));

0 commit comments

Comments
 (0)