Merge pull request #111 from limingxinleo/redis_select

Fixed bug for redis::select.

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 # Fixed
 
-- [#110](https://github.com/hyperf-cloud/hyperf/pull/110) Fixed Redis::select is not work expected.
+- [#110](https://github.com/hyperf-cloud/hyperf/pull/110) [#111](https://github.com/hyperf-cloud/hyperf/pull/111) Fixed Redis::select is not work expected.
 
 # v1.0.3 - 2019-07-02
 

src/redis/publish/redis.php

@@ -13,7 +13,7 @@ declare(strict_types=1);
 return [
     'default' => [
         'host' => env('REDIS_HOST', 'localhost'),
-        'auth' => env('REDIS_AUTH', ''),
+        'auth' => env('REDIS_AUTH', null),
         'port' => (int) env('REDIS_PORT', 6379),
         'db' => (int) env('REDIS_DB', 0),
         'timeout' => 0.0,

src/redis/src/Redis.php

@@ -45,8 +45,8 @@ class Redis
             // Release connection.
             if (! $hasContextConnection) {
                 if ($this->shouldUseSameConnection($name)) {
-                    if ($name == 'select') {
+                    if ($name === 'select' && $db = $arguments[0]) {
-                        $connection->setDbChanged(true);
+                        $connection->setDatabase((int) $db);
                     }
                     // Should storage the connection to coroutine context, then use defer() to release the connection.
                     Context::set($this->getContextKey(), $connection);

src/redis/src/RedisConnection.php

@@ -28,17 +28,24 @@ class RedisConnection extends BaseConnection implements ConnectionInterface
     /**
      * @var array
      */
-    protected $config;
+    protected $config = [
+        'host' => 'localhost',
+        'port' => 6379,
+        'auth' => null,
+        'db' => 0,
+        'timeout' => 0.0,
+    ];
 
     /**
-     * @var bool
+     * Current redis database.
+     * @var null|int
      */
-    protected $dbChanged;
+    protected $database;
 
     public function __construct(ContainerInterface $container, Pool $pool, array $config)
     {
         parent::__construct($container, $pool);
-        $this->config = $config;
+        $this->config = array_replace($this->config, $config);
 
         $this->reconnect();
     }
@@ -63,11 +70,11 @@ class RedisConnection extends BaseConnection implements ConnectionInterface
 
     public function reconnect(): bool
     {
-        $host = $this->config['host'] ?? 'localhost';
+        $host = $this->config['host'];
-        $port = $this->config['port'] ?? 6379;
+        $port = $this->config['port'];
-        $auth = $this->config['auth'] ?? null;
+        $auth = $this->config['auth'];
-        $db = $this->config['db'] ?? 0;
+        $db = $this->config['db'];
-        $timeout = $this->config['timeout'] ?? 0.0;
+        $timeout = $this->config['timeout'];
 
         $redis = new \Redis();
         if (! $redis->connect($host, $port, $timeout)) {
@@ -78,12 +85,14 @@ class RedisConnection extends BaseConnection implements ConnectionInterface
             $redis->auth($auth);
         }
 
-        if ($db > 0) {
+        $database = $this->database ?? $db;
-            $redis->select($db);
+        if ($database > 0) {
+            $redis->select($database);
         }
 
         $this->connection = $redis;
         $this->lastUseTime = microtime(true);
+
         return true;
     }
 
@@ -94,18 +103,16 @@ class RedisConnection extends BaseConnection implements ConnectionInterface
 
     public function release(): void
     {
-        if ($this->dbChanged) {
+        if ($this->database && $this->database != $this->config['db']) {
             // Select the origin db after execute select.
-            $this->select($this->config['db'] ?? 0);
+            $this->select($this->config['db']);
+            $this->database = null;
         }
         parent::release();
     }
 
-    /**
+    public function setDatabase(?int $database): void
-     * @param bool $dbChanged
-     */
-    public function setDbChanged(bool $dbChanged): void
     {
-        $this->dbChanged = $dbChanged;
+        $this->database = $database;
     }
 }

src/redis/tests/RedisConnectionTest.php

@@ -0,0 +1,95 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * This file is part of Hyperf.
+ *
+ * @link     https://www.hyperf.io
+ * @document https://doc.hyperf.io
+ * @contact  group@hyperf.io
+ * @license  https://github.com/hyperf-cloud/hyperf/blob/master/LICENSE
+ */
+
+namespace HyperfTest\Redis;
+
+use Hyperf\Config\Config;
+use Hyperf\Contract\ConfigInterface;
+use Hyperf\Di\Container;
+use HyperfTest\Redis\Stub\RedisPoolStub;
+use Mockery;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @internal
+ * @coversNothing
+ */
+class RedisConnectionTest extends TestCase
+{
+    public function tearDown()
+    {
+        Mockery::close();
+    }
+
+    public function testRedisConnectionConfig()
+    {
+        $pool = $this->getRedisPool();
+
+        $config = $pool->get()->getConfig();
+
+        $this->assertSame([
+            'host' => 'redis',
+            'port' => 16379,
+            'auth' => 'redis',
+            'db' => 0,
+            'timeout' => 0.0,
+            'pool' => [
+                'min_connections' => 1,
+                'max_connections' => 30,
+                'connect_timeout' => 10.0,
+                'wait_timeout' => 3.0,
+                'heartbeat' => -1,
+                'max_idle_time' => 1,
+            ],
+        ], $config);
+    }
+
+    public function testRedisConnectionReconnect()
+    {
+        $pool = $this->getRedisPool();
+
+        $connection = $pool->get()->getConnection();
+        $this->assertSame(null, $connection->getDatabase());
+
+        $connection->setDatabase(2);
+        $connection->reconnect();
+        $this->assertSame(2, $connection->getDatabase());
+
+        $connection->release();
+        $connection = $pool->get()->getConnection();
+        $this->assertSame(null, $connection->getDatabase());
+    }
+
+    private function getRedisPool()
+    {
+        $container = Mockery::mock(Container::class);
+        $container->shouldReceive('get')->once()->with(ConfigInterface::class)->andReturn(new Config([
+            'redis' => [
+                'default' => [
+                    'host' => 'redis',
+                    'auth' => 'redis',
+                    'port' => 16379,
+                    'pool' => [
+                        'min_connections' => 1,
+                        'max_connections' => 30,
+                        'connect_timeout' => 10.0,
+                        'wait_timeout' => 3.0,
+                        'heartbeat' => -1,
+                        'max_idle_time' => 1,
+                    ],
+                ],
+            ],
+        ]));
+
+        return new RedisPoolStub($container, 'default');
+    }
+}

src/redis/tests/RedisTest.php

@@ -19,7 +19,6 @@ use Hyperf\Redis\Pool\PoolFactory;
 use Hyperf\Redis\Pool\RedisPool;
 use Hyperf\Redis\Redis;
 use Hyperf\Utils\ApplicationContext;
-use Hyperf\Utils\Context;
 use HyperfTest\Redis\Stub\RedisPoolStub;
 use Mockery;
 use PHPUnit\Framework\TestCase;
@@ -49,12 +48,45 @@ class RedisTest extends TestCase
         $this->assertTrue($redis->connect('127.0.0.1', 6379, 0.0));
     }
 
-    public function testRedisCommand()
+    public function testRedisSelect()
+    {
+        $redis = $this->getRedis();
+
+        $res = $redis->set('xxxx', 'yyyy');
+        $this->assertSame('db:0 name:set argument:xxxx,yyyy', $res);
+
+        $redis->select(2);
+        $res = $redis->get('xxxx');
+        $this->assertSame('db:2 name:get argument:xxxx', $res);
+
+        $this->assertSame(2, $redis->getDatabase());
+
+        $res = parallel([function () use ($redis) {
+            return $redis->get('xxxx');
+        }]);
+
+        $this->assertSame('db:0 name:get argument:xxxx', $res[0]);
+    }
+
+    private function getRedis()
     {
         $container = Mockery::mock(Container::class);
         $container->shouldReceive('get')->once()->with(ConfigInterface::class)->andReturn(new Config([
             'redis' => [
-                'default' => [],
+                'default' => [
+                    'host' => 'localhost',
+                    'auth' => null,
+                    'port' => 6379,
+                    'db' => 0,
+                    'pool' => [
+                        'min_connections' => 1,
+                        'max_connections' => 30,
+                        'connect_timeout' => 10.0,
+                        'wait_timeout' => 3.0,
+                        'heartbeat' => -1,
+                        'max_idle_time' => 60,
+                    ],
+                ],
             ],
         ]));
         $pool = new RedisPoolStub($container, 'default');
@@ -64,18 +96,6 @@ class RedisTest extends TestCase
 
         $factory = new PoolFactory($container);
 
-        $redis = new Redis($factory);
+        return new Redis($factory);
-
-        $res = $redis->set('xxxx', 'yyyy');
-        $this->assertSame('db:0 name:set argument:xxxx,yyyy', $res);
-
-        $redis->select(2);
-        $res = $redis->get('xxxx');
-        $this->assertSame('db:2 name:get argument:xxxx', $res);
-
-        parallel([function () use ($redis) {
-            $res = $redis->get('xxxx');
-            $this->assertSame('db:0 name:get argument:xxxx', $res);
-        }]);
     }
 }

src/redis/tests/Stub/RedisConnectionStub.php

@@ -33,22 +33,33 @@ class RedisConnectionStub extends RedisConnection
 
     public function reconnect(): bool
     {
-        $this->host = $this->config['host'] ?? 'localhost';
+        $this->host = $this->config['host'];
-        $this->port = $this->config['port'] ?? 6379;
+        $this->port = $this->config['port'];
-        $this->auth = $this->config['auth'] ?? null;
+        $this->auth = $this->config['auth'];
-        $this->db = $this->config['db'] ?? 0;
+        $this->db = $this->config['db'];
-        $this->timeout = $this->config['timeout'] ?? 0.0;
+        $this->timeout = $this->config['timeout'];
 
         return true;
     }
 
-    public function getConnection()
-    {
-        return parent::getConnection();
-    }
-
     public function select($db)
     {
         $this->db = $db;
     }
+
+    /**
+     * @return array
+     */
+    public function getConfig(): array
+    {
+        return $this->config;
+    }
+
+    /**
+     * @return null|int
+     */
+    public function getDatabase(): ?int
+    {
+        return $this->database;
+    }
 }