Skip to content

Commit 56ceb00

Browse files
g105bg105b
authored
tweak: redis followup from #272 (#279)
1 parent 6888e9b commit 56ceb00

2 files changed

Lines changed: 151 additions & 26 deletions

File tree

‎src/RedisHandler.php‎

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,50 @@
33

44
use Redis;
55
use RuntimeException;
6+
use Throwable;
67

78
class RedisHandler extends Handler {
9+
private const EMPTY_PHP_ARRAY = "a:0:{}";
810
private const DEFAULT_PORT = 6379;
911
private const DEFAULT_PREFIX_SEPARATOR = ":";
1012
private ?Redis $client = null;
13+
/** @var array{
14+
* host:string,
15+
* port:int,
16+
* timeout:float,
17+
* readTimeout:float,
18+
* persistentId:?string,
19+
* prefix:string,
20+
* ttl:int,
21+
* database:int,
22+
* auth:array{string,string}|string|null,
23+
* context:array{stream:array{verify_peer:bool,verify_peer_name:bool}}|null
24+
* }|null
25+
*/
26+
private ?array $config = null;
1127
private string $prefix = "";
1228
private int $ttl = 0;
1329

1430
public function open(string $savePath, string $name):bool {
1531
$config = $this->parseSavePath($savePath, $name);
32+
$this->config = $config;
33+
return $this->connect($config);
34+
}
35+
36+
/** @param array{
37+
* host:string,
38+
* port:int,
39+
* timeout:float,
40+
* readTimeout:float,
41+
* persistentId:?string,
42+
* prefix:string,
43+
* ttl:int,
44+
* database:int,
45+
* auth:array{string,string}|string|null,
46+
* context:array{stream:array{verify_peer:bool,verify_peer_name:bool}}|null
47+
* } $config
48+
*/
49+
private function connect(array $config):bool {
1650
$client = $this->createClient();
1751

1852
$connected = $client->connect(
@@ -48,27 +82,46 @@ public function close():bool {
4882
return true;
4983
}
5084

51-
return $this->client->close();
85+
try {
86+
return $this->client->close();
87+
}
88+
catch(Throwable) {
89+
return true;
90+
}
91+
finally {
92+
$this->client = null;
93+
}
5294
}
5395

5496
public function read(string $sessionId):string {
55-
$value = $this->requireClient()->get($this->getKey($sessionId));
97+
$value = $this->retryOnce(
98+
fn() => $this->requireClient()->get($this->getKey($sessionId))
99+
);
56100
return is_string($value) ? $value : "";
57101
}
58102

59103
public function write(string $sessionId, string $sessionData):bool {
60-
$client = $this->requireClient();
104+
if($sessionData === self::EMPTY_PHP_ARRAY) {
105+
return true;
106+
}
107+
61108
$key = $this->getKey($sessionId);
62109

63110
if($this->ttl > 0) {
64-
return $client->setEx($key, $this->ttl, $sessionData);
111+
return $this->retryOnce(
112+
fn() => $this->requireClient()->setEx($key, $this->ttl, $sessionData)
113+
);
65114
}
66115

67-
return $client->set($key, $sessionData);
116+
return $this->retryOnce(
117+
fn() => $this->requireClient()->set($key, $sessionData)
118+
);
68119
}
69120

70121
public function destroy(string $id = ""):bool {
71-
return $this->requireClient()->del($this->getKey($id)) >= 0;
122+
return $this->retryOnce(
123+
fn() => $this->requireClient()->del($this->getKey($id))
124+
) >= 0;
72125
}
73126

74127
// phpcs:disable Generic.CodeAnalysis.UnusedFunctionParameter.FoundInExtendedClass
@@ -219,4 +272,28 @@ private function requireClient():Redis {
219272

220273
return $this->client;
221274
}
275+
276+
private function reconnect():void {
277+
if(is_null($this->config)) {
278+
throw new RuntimeException("RedisHandler::open() must be called before reconnect.");
279+
}
280+
281+
$this->close();
282+
$this->connect($this->config);
283+
}
284+
285+
/**
286+
* @template T
287+
* @param callable():T $callback
288+
* @return T
289+
*/
290+
private function retryOnce(callable $callback):mixed {
291+
try {
292+
return $callback();
293+
}
294+
catch(Throwable) {
295+
$this->reconnect();
296+
return $callback();
297+
}
298+
}
222299
}

‎test/phpunit/RedisHandlerTest.php‎

Lines changed: 68 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@
55
use PHPUnit\Framework\TestCase;
66
use Redis;
77

8+
if(!class_exists(Redis::class)) {
9+
class TestRedisClientBase {}
10+
class_alias(TestRedisClientBase::class, "Redis");
11+
}
12+
813
class RedisHandlerTest extends TestCase {
914
public function testOpenParsesStandardDsn():void {
1015
$client = new TestRedisClient();
@@ -91,9 +96,44 @@ protected function createClient():Redis {
9196
self::assertTrue($sut->close());
9297
self::assertTrue($client->closed);
9398
}
99+
100+
public function testEmptyNativePhpSessionWriteDoesNotOverwriteStoredSession():void {
101+
$client = new TestRedisClient();
102+
$sut = new class($client) extends RedisHandler {
103+
public function __construct(private readonly TestRedisClient $client) {}
104+
105+
protected function createClient():Redis {
106+
/** @phpstan-ignore-next-line */
107+
return $this->client;
108+
}
109+
};
110+
$sut->open("redis://cache.internal", "GT");
111+
$sut->write("abc123", "stored-session");
112+
$sut->write("abc123", "a:0:{}");
113+
114+
self::assertSame("stored-session", $sut->read("abc123"));
115+
}
116+
117+
public function testCommandRetriesAfterDisconnectedClient():void {
118+
$client = new TestRedisClient();
119+
$sut = new class($client) extends RedisHandler {
120+
public function __construct(private readonly TestRedisClient $client) {}
121+
122+
protected function createClient():Redis {
123+
/** @phpstan-ignore-next-line */
124+
return $this->client;
125+
}
126+
};
127+
$sut->open("redis://cache.internal", "GT");
128+
$client->failNextSetEx = true;
129+
130+
self::assertTrue($sut->write("abc123", "payload"));
131+
self::assertSame("payload", $sut->read("abc123"));
132+
self::assertSame(2, $client->connectCount);
133+
}
94134
}
95135

96-
class TestRedisClient {
136+
class TestRedisClient extends Redis {
97137
/** @var array<string,mixed> */
98138
public array $connectParameters = [];
99139
/** @var array<int,array{key:string,ttl:int,value:string}> */
@@ -107,27 +147,30 @@ class TestRedisClient {
107147
/** @var array<string,string> */
108148
public array $data = [];
109149
public int $deleted = 0;
150+
public int $connectCount = 0;
151+
public bool $failNextSetEx = false;
110152
public bool $closed = false;
111153

112154
/**
113155
* @param array{auth?:array{0:string|false|null,1?:string},stream?:array<string,mixed>}|null $context
114156
*/
115157
public function connect(
116158
string $host,
117-
int $port,
159+
int $port = 6379,
118160
float $timeout = 0,
119-
?string $persistentId = null,
120-
int $retryInterval = 0,
121-
float $readTimeout = 0,
161+
?string $persistent_id = null,
162+
int $retry_interval = 0,
163+
float $read_timeout = 0,
122164
?array $context = null,
123165
):bool {
166+
$this->connectCount++;
124167
$this->connectParameters = [
125168
"host" => $host,
126169
"port" => $port,
127170
"timeout" => $timeout,
128-
"persistentId" => $persistentId,
129-
"retryInterval" => $retryInterval,
130-
"readTimeout" => $readTimeout,
171+
"persistentId" => $persistent_id,
172+
"retryInterval" => $retry_interval,
173+
"readTimeout" => $read_timeout,
131174
"context" => $context,
132175
];
133176
return true;
@@ -136,12 +179,12 @@ public function connect(
136179
/**
137180
* @param array{string,string}|string $credentials
138181
*/
139-
public function auth(array|string $credentials):bool {
182+
public function auth(mixed $credentials):Redis|bool {
140183
$this->authCalls []= $credentials;
141184
return true;
142185
}
143186

144-
public function select(int $database):bool {
187+
public function select(int $database):Redis|bool {
145188
$this->selectCalls []= $database;
146189
return true;
147190
}
@@ -150,23 +193,32 @@ public function get(string $key):string|false {
150193
return $this->data[$key] ?? false;
151194
}
152195

153-
public function set(string $key, string $value):bool {
196+
public function set(string $key, mixed $value, mixed $options = null):Redis|string|bool {
154197
$this->setCalls []= $key;
155-
$this->data[$key] = $value;
198+
$this->data[$key] = (string)$value;
156199
return true;
157200
}
158201

159-
public function setEx(string $key, int $ttl, string $value):bool {
202+
public function setEx(string $key, int $ttl, mixed $value) {
203+
if($this->failNextSetEx) {
204+
$this->failNextSetEx = false;
205+
throw new \RedisException("Redis server went away");
206+
}
207+
160208
$this->setExCalls []= [
161209
"key" => $key,
162210
"ttl" => $ttl,
163-
"value" => $value,
211+
"value" => (string)$value,
164212
];
165-
$this->data[$key] = $value;
213+
$this->data[$key] = (string)$value;
166214
return true;
167215
}
168216

169-
public function del(string $key):int {
217+
public function del(array|string $key, string ...$otherKeys):Redis|int|false {
218+
if(is_array($key)) {
219+
$key = reset($key);
220+
}
221+
170222
unset($this->data[$key]);
171223
return ++$this->deleted;
172224
}
@@ -176,7 +228,3 @@ public function close():bool {
176228
return true;
177229
}
178230
}
179-
180-
if(!class_exists(Redis::class)) {
181-
class_alias(TestRedisClient::class, Redis::class);
182-
}

0 commit comments

Comments
 (0)