From c483053c45816605c6536af1f5830cb85e39ce93 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Tue, 30 Jun 2026 16:44:12 +0100 Subject: [PATCH] tweak: redis followup from #272 --- src/RedisHandler.php | 89 ++++++++++++++++++++++++++++--- test/phpunit/RedisHandlerTest.php | 88 +++++++++++++++++++++++------- 2 files changed, 151 insertions(+), 26 deletions(-) diff --git a/src/RedisHandler.php b/src/RedisHandler.php index b53bd4a..1c4de21 100644 --- a/src/RedisHandler.php +++ b/src/RedisHandler.php @@ -3,16 +3,50 @@ use Redis; use RuntimeException; +use Throwable; class RedisHandler extends Handler { + private const EMPTY_PHP_ARRAY = "a:0:{}"; private const DEFAULT_PORT = 6379; private const DEFAULT_PREFIX_SEPARATOR = ":"; private ?Redis $client = null; + /** @var array{ + * host:string, + * port:int, + * timeout:float, + * readTimeout:float, + * persistentId:?string, + * prefix:string, + * ttl:int, + * database:int, + * auth:array{string,string}|string|null, + * context:array{stream:array{verify_peer:bool,verify_peer_name:bool}}|null + * }|null + */ + private ?array $config = null; private string $prefix = ""; private int $ttl = 0; public function open(string $savePath, string $name):bool { $config = $this->parseSavePath($savePath, $name); + $this->config = $config; + return $this->connect($config); + } + + /** @param array{ + * host:string, + * port:int, + * timeout:float, + * readTimeout:float, + * persistentId:?string, + * prefix:string, + * ttl:int, + * database:int, + * auth:array{string,string}|string|null, + * context:array{stream:array{verify_peer:bool,verify_peer_name:bool}}|null + * } $config + */ + private function connect(array $config):bool { $client = $this->createClient(); $connected = $client->connect( @@ -48,27 +82,46 @@ public function close():bool { return true; } - return $this->client->close(); + try { + return $this->client->close(); + } + catch(Throwable) { + return true; + } + finally { + $this->client = null; + } } public function read(string $sessionId):string { - $value = $this->requireClient()->get($this->getKey($sessionId)); + $value = $this->retryOnce( + fn() => $this->requireClient()->get($this->getKey($sessionId)) + ); return is_string($value) ? $value : ""; } public function write(string $sessionId, string $sessionData):bool { - $client = $this->requireClient(); + if($sessionData === self::EMPTY_PHP_ARRAY) { + return true; + } + $key = $this->getKey($sessionId); if($this->ttl > 0) { - return $client->setEx($key, $this->ttl, $sessionData); + return $this->retryOnce( + fn() => $this->requireClient()->setEx($key, $this->ttl, $sessionData) + ); } - return $client->set($key, $sessionData); + return $this->retryOnce( + fn() => $this->requireClient()->set($key, $sessionData) + ); } public function destroy(string $id = ""):bool { - return $this->requireClient()->del($this->getKey($id)) >= 0; + return $this->retryOnce( + fn() => $this->requireClient()->del($this->getKey($id)) + ) >= 0; } // phpcs:disable Generic.CodeAnalysis.UnusedFunctionParameter.FoundInExtendedClass @@ -219,4 +272,28 @@ private function requireClient():Redis { return $this->client; } + + private function reconnect():void { + if(is_null($this->config)) { + throw new RuntimeException("RedisHandler::open() must be called before reconnect."); + } + + $this->close(); + $this->connect($this->config); + } + + /** + * @template T + * @param callable():T $callback + * @return T + */ + private function retryOnce(callable $callback):mixed { + try { + return $callback(); + } + catch(Throwable) { + $this->reconnect(); + return $callback(); + } + } } diff --git a/test/phpunit/RedisHandlerTest.php b/test/phpunit/RedisHandlerTest.php index c6680d2..c658bc5 100644 --- a/test/phpunit/RedisHandlerTest.php +++ b/test/phpunit/RedisHandlerTest.php @@ -5,6 +5,11 @@ use PHPUnit\Framework\TestCase; use Redis; +if(!class_exists(Redis::class)) { + class TestRedisClientBase {} + class_alias(TestRedisClientBase::class, "Redis"); +} + class RedisHandlerTest extends TestCase { public function testOpenParsesStandardDsn():void { $client = new TestRedisClient(); @@ -91,9 +96,44 @@ protected function createClient():Redis { self::assertTrue($sut->close()); self::assertTrue($client->closed); } + + public function testEmptyNativePhpSessionWriteDoesNotOverwriteStoredSession():void { + $client = new TestRedisClient(); + $sut = new class($client) extends RedisHandler { + public function __construct(private readonly TestRedisClient $client) {} + + protected function createClient():Redis { + /** @phpstan-ignore-next-line */ + return $this->client; + } + }; + $sut->open("redis://cache.internal", "GT"); + $sut->write("abc123", "stored-session"); + $sut->write("abc123", "a:0:{}"); + + self::assertSame("stored-session", $sut->read("abc123")); + } + + public function testCommandRetriesAfterDisconnectedClient():void { + $client = new TestRedisClient(); + $sut = new class($client) extends RedisHandler { + public function __construct(private readonly TestRedisClient $client) {} + + protected function createClient():Redis { + /** @phpstan-ignore-next-line */ + return $this->client; + } + }; + $sut->open("redis://cache.internal", "GT"); + $client->failNextSetEx = true; + + self::assertTrue($sut->write("abc123", "payload")); + self::assertSame("payload", $sut->read("abc123")); + self::assertSame(2, $client->connectCount); + } } -class TestRedisClient { +class TestRedisClient extends Redis { /** @var array */ public array $connectParameters = []; /** @var array */ @@ -107,6 +147,8 @@ class TestRedisClient { /** @var array */ public array $data = []; public int $deleted = 0; + public int $connectCount = 0; + public bool $failNextSetEx = false; public bool $closed = false; /** @@ -114,20 +156,21 @@ class TestRedisClient { */ public function connect( string $host, - int $port, + int $port = 6379, float $timeout = 0, - ?string $persistentId = null, - int $retryInterval = 0, - float $readTimeout = 0, + ?string $persistent_id = null, + int $retry_interval = 0, + float $read_timeout = 0, ?array $context = null, ):bool { + $this->connectCount++; $this->connectParameters = [ "host" => $host, "port" => $port, "timeout" => $timeout, - "persistentId" => $persistentId, - "retryInterval" => $retryInterval, - "readTimeout" => $readTimeout, + "persistentId" => $persistent_id, + "retryInterval" => $retry_interval, + "readTimeout" => $read_timeout, "context" => $context, ]; return true; @@ -136,12 +179,12 @@ public function connect( /** * @param array{string,string}|string $credentials */ - public function auth(array|string $credentials):bool { + public function auth(mixed $credentials):Redis|bool { $this->authCalls []= $credentials; return true; } - public function select(int $database):bool { + public function select(int $database):Redis|bool { $this->selectCalls []= $database; return true; } @@ -150,23 +193,32 @@ public function get(string $key):string|false { return $this->data[$key] ?? false; } - public function set(string $key, string $value):bool { + public function set(string $key, mixed $value, mixed $options = null):Redis|string|bool { $this->setCalls []= $key; - $this->data[$key] = $value; + $this->data[$key] = (string)$value; return true; } - public function setEx(string $key, int $ttl, string $value):bool { + public function setEx(string $key, int $ttl, mixed $value) { + if($this->failNextSetEx) { + $this->failNextSetEx = false; + throw new \RedisException("Redis server went away"); + } + $this->setExCalls []= [ "key" => $key, "ttl" => $ttl, - "value" => $value, + "value" => (string)$value, ]; - $this->data[$key] = $value; + $this->data[$key] = (string)$value; return true; } - public function del(string $key):int { + public function del(array|string $key, string ...$otherKeys):Redis|int|false { + if(is_array($key)) { + $key = reset($key); + } + unset($this->data[$key]); return ++$this->deleted; } @@ -176,7 +228,3 @@ public function close():bool { return true; } } - -if(!class_exists(Redis::class)) { - class_alias(TestRedisClient::class, Redis::class); -}