From 4172d2265f843f298ed079e8e704d45481bb03a3 Mon Sep 17 00:00:00 2001 From: Vladyslav Vildanov <117659936+vladvildanov@users.noreply.github.com> Date: Thu, 3 Aug 2023 09:04:04 +0300 Subject: [PATCH] Client set name and version on connection (#1347) * Added client metadata on server connection * Added try...catch around server exception to supress CLIENT command errors * Added exclusion for Relay connection --- src/Connection/AbstractConnection.php | 13 ++++ src/Connection/Factory.php | 11 ++++ src/Connection/StreamConnection.php | 4 +- tests/Predis/ClientTest.php | 14 +++++ tests/Predis/Connection/FactoryTest.php | 59 ++++++++++++++++--- .../Connection/StreamConnectionTest.php | 19 ++++++ 6 files changed, 110 insertions(+), 10 deletions(-) diff --git a/src/Connection/AbstractConnection.php b/src/Connection/AbstractConnection.php index 5383924c..3273f386 100644 --- a/src/Connection/AbstractConnection.php +++ b/src/Connection/AbstractConnection.php @@ -14,6 +14,7 @@ namespace Predis\Connection; use InvalidArgumentException; use Predis\Command\CommandInterface; +use Predis\Command\RawCommand; use Predis\CommunicationException; use Predis\Protocol\ProtocolException; @@ -27,6 +28,10 @@ abstract class AbstractConnection implements NodeConnectionInterface private $cachedId; protected $parameters; + + /** + * @var RawCommand[] + */ protected $initCommands = []; /** @@ -101,6 +106,14 @@ abstract class AbstractConnection implements NodeConnectionInterface $this->initCommands[] = $command; } + /** + * {@inheritdoc} + */ + public function getInitCommands(): array + { + return $this->initCommands; + } + /** * {@inheritdoc} */ diff --git a/src/Connection/Factory.php b/src/Connection/Factory.php index ad472360..c78c6d40 100644 --- a/src/Connection/Factory.php +++ b/src/Connection/Factory.php @@ -13,6 +13,7 @@ namespace Predis\Connection; use InvalidArgumentException; +use Predis\Client; use Predis\Command\RawCommand; use ReflectionClass; use UnexpectedValueException; @@ -174,6 +175,16 @@ class Factory implements FactoryInterface ); } + if (!$connection instanceof RelayConnection) { + $connection->addConnectCommand( + new RawCommand('CLIENT', ['SETINFO', 'LIB-NAME', 'predis']) + ); + + $connection->addConnectCommand( + new RawCommand('CLIENT', ['SETINFO', 'LIB-VER', Client::VERSION]) + ); + } + if (isset($parameters->database) && strlen($parameters->database)) { $connection->addConnectCommand( new RawCommand('SELECT', [$parameters->database]) diff --git a/src/Connection/StreamConnection.php b/src/Connection/StreamConnection.php index e0e077c9..2fe30706 100644 --- a/src/Connection/StreamConnection.php +++ b/src/Connection/StreamConnection.php @@ -231,7 +231,9 @@ class StreamConnection extends AbstractConnection foreach ($this->initCommands as $command) { $response = $this->executeCommand($command); - if ($response instanceof ErrorResponseInterface) { + if ($response instanceof ErrorResponseInterface && $command->getId() === 'CLIENT') { + // Do nothing on CLIENT SETINFO command failure + } elseif ($response instanceof ErrorResponseInterface) { $this->onConnectionError("`{$command->getId()}` failed: {$response->getMessage()}", 0); } } diff --git a/tests/Predis/ClientTest.php b/tests/Predis/ClientTest.php index 04bac903..fba37d98 100644 --- a/tests/Predis/ClientTest.php +++ b/tests/Predis/ClientTest.php @@ -1253,6 +1253,20 @@ class ClientTest extends PredisTestCase $this->assertSame('127.0.0.1:6381', $iterator->key()); } + /** + * @group connected + * @requiresRedisVersion >= 7.2.0 + */ + public function testSetClientInfoOnConnection(): void + { + $client = new Client($this->getParameters()); + $libName = $client->client('LIST')[0]['lib-name']; + $libVer = $client->client('LIST')[0]['lib-ver']; + + $this->assertSame('predis', $libName); + $this->assertSame(Client::VERSION, $libVer); + } + // ******************************************************************** // // ---- HELPER METHODS ------------------------------------------------ // // ******************************************************************** // diff --git a/tests/Predis/Connection/FactoryTest.php b/tests/Predis/Connection/FactoryTest.php index 4d98dd6f..5e64497b 100644 --- a/tests/Predis/Connection/FactoryTest.php +++ b/tests/Predis/Connection/FactoryTest.php @@ -12,6 +12,8 @@ namespace Predis\Connection; +use Predis\Client; +use Predis\Command\RawCommand; use PredisTestCase; use ReflectionObject; use stdClass; @@ -302,10 +304,12 @@ class FactoryTest extends PredisTestCase ->method('getParameters') ->willReturn($parameters); $connection - ->expects($this->exactly(2)) + ->expects($this->exactly(4)) ->method('addConnectCommand') ->withConsecutive( [$this->isRedisCommand('AUTH', ['foobar'])], + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-NAME', 'predis'])], + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-VER', Client::VERSION])], [$this->isRedisCommand('SELECT', ['0'])] ); @@ -331,9 +335,13 @@ class FactoryTest extends PredisTestCase $connection->expects($this->once()) ->method('getParameters') ->will($this->returnValue($parameters)); - $connection->expects($this->once()) + $connection->expects($this->exactly(3)) ->method('addConnectCommand') - ->with($this->isRedisCommand('AUTH', ['foobar'])); + ->withConsecutive( + [$this->isRedisCommand('AUTH', ['foobar'])], + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-NAME', 'predis'])], + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-VER', Client::VERSION])] + ); $factory = new Factory(); @@ -358,9 +366,13 @@ class FactoryTest extends PredisTestCase $connection->expects($this->once()) ->method('getParameters') ->will($this->returnValue($parameters)); - $connection->expects($this->once()) + $connection->expects($this->exactly(3)) ->method('addConnectCommand') - ->with($this->isRedisCommand('AUTH', ['myusername', 'foobar'])); + ->withConsecutive( + [$this->isRedisCommand('AUTH', ['myusername', 'foobar'])], + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-NAME', 'predis'])], + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-VER', Client::VERSION])] + ); $factory = new Factory(); @@ -384,8 +396,12 @@ class FactoryTest extends PredisTestCase $connection->expects($this->once()) ->method('getParameters') ->will($this->returnValue($parameters)); - $connection->expects($this->never()) - ->method('addConnectCommand'); + $connection->expects($this->exactly(2)) + ->method('addConnectCommand') + ->withConsecutive( + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-NAME', 'predis'])], + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-VER', Client::VERSION])] + ); $factory = new Factory(); @@ -410,8 +426,12 @@ class FactoryTest extends PredisTestCase $connection->expects($this->once()) ->method('getParameters') ->will($this->returnValue($parameters)); - $connection->expects($this->never()) - ->method('addConnectCommand'); + $connection->expects($this->exactly(2)) + ->method('addConnectCommand') + ->withConsecutive( + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-NAME', 'predis'])], + [$this->isRedisCommand('CLIENT', ['SETINFO', 'LIB-VER', Client::VERSION])] + ); $factory = new Factory(); @@ -538,6 +558,27 @@ class FactoryTest extends PredisTestCase $factory->create('test://127.0.0.1'); } + /** + * @group disconnected + * @return void + */ + public function testSetClientNameAndVersionOnConnection(): void + { + $parameters = []; + + $factory = new Factory(); + $connection = $factory->create($parameters); + $initCommands = $connection->getInitCommands(); + + $this->assertInstanceOf(RawCommand::class, $initCommands[0]); + $this->assertSame('CLIENT', $initCommands[0]->getId()); + $this->assertSame(['SETINFO', 'LIB-NAME', 'predis'], $initCommands[0]->getArguments()); + + $this->assertInstanceOf(RawCommand::class, $initCommands[1]); + $this->assertSame('CLIENT', $initCommands[1]->getId()); + $this->assertSame(['SETINFO', 'LIB-VER', Client::VERSION], $initCommands[1]->getArguments()); + } + // ******************************************************************** // // ---- HELPER METHODS ------------------------------------------------ // // ******************************************************************** // diff --git a/tests/Predis/Connection/StreamConnectionTest.php b/tests/Predis/Connection/StreamConnectionTest.php index 55c30c08..cd38b408 100644 --- a/tests/Predis/Connection/StreamConnectionTest.php +++ b/tests/Predis/Connection/StreamConnectionTest.php @@ -13,6 +13,7 @@ namespace Predis\Connection; use PHPUnit\Framework\MockObject\MockObject; +use Predis\Client; use Predis\Command\RawCommand; use Predis\Response\Error as ErrorResponse; @@ -195,4 +196,22 @@ class StreamConnectionTest extends PredisConnectionTestCase $this->assertArrayHasKey('tcp_nodelay', $options['socket']); $this->assertFalse($options['socket']['tcp_nodelay']); } + + /** + * @group connected + * @requiresRedisVersion < 7.0.0 + */ + public function testConnectDoNotThrowsExceptionOnClientCommandError(): void + { + $connection = $this->createConnectionWithParams([]); + $connection->addConnectCommand( + new RawCommand('CLIENT', ['SETINFO', 'LIB-NAME', 'predis']) + ); + $connection->addConnectCommand( + new RawCommand('CLIENT', ['SETINFO', 'LIB-VER', Client::VERSION]) + ); + + $connection->connect(); + $this->assertTrue(true); + } }