From 98cde84ec33354513f279ccb2b252a586323015e Mon Sep 17 00:00:00 2001 From: Thomas Vargiu Date: Wed, 18 Apr 2018 03:52:07 +0200 Subject: [PATCH 1/3] Fixed #109. Prioritized redis option --- src/Storage/Adapter/RedisOptions.php | 2 +- test/Storage/Adapter/RedisTest.php | 39 +++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/Storage/Adapter/RedisOptions.php b/src/Storage/Adapter/RedisOptions.php index 8c271dd91..7cb4fe192 100644 --- a/src/Storage/Adapter/RedisOptions.php +++ b/src/Storage/Adapter/RedisOptions.php @@ -20,7 +20,7 @@ class RedisOptions extends AdapterOptions * * @var string[] */ - protected $__prioritizedProperties__ = ['resource_manager', 'resource_id']; + protected $__prioritizedProperties__ = ['resource_manager', 'resource_id', 'server']; // @codingStandardsIgnoreEnd /** diff --git a/test/Storage/Adapter/RedisTest.php b/test/Storage/Adapter/RedisTest.php index 712173c4e..6ade66add 100644 --- a/test/Storage/Adapter/RedisTest.php +++ b/test/Storage/Adapter/RedisTest.php @@ -84,6 +84,33 @@ public function getCommonAdapterNamesProvider() ]; } + public function testLibOptionsFirst() + { + $options = [ + 'resource_id' => __CLASS__ . '2', + 'lib_options' => [ + RedisResource::OPT_SERIALIZER => RedisResource::SERIALIZER_PHP, + ], + ]; + + if (getenv('TESTS_ZEND_CACHE_REDIS_HOST') && getenv('TESTS_ZEND_CACHE_REDIS_PORT')) { + $options['server'] = [getenv('TESTS_ZEND_CACHE_REDIS_HOST'), getenv('TESTS_ZEND_CACHE_REDIS_PORT')]; + } elseif (getenv('TESTS_ZEND_CACHE_REDIS_HOST')) { + $options['server'] = [getenv('TESTS_ZEND_CACHE_REDIS_HOST')]; + } + + if (getenv('TESTS_ZEND_CACHE_REDIS_DATABASE')) { + $options['database'] = getenv('TESTS_ZEND_CACHE_REDIS_DATABASE'); + } + + if (getenv('TESTS_ZEND_CACHE_REDIS_PASSWORD')) { + $options['password'] = getenv('TESTS_ZEND_CACHE_REDIS_PASSWORD'); + } + + $redisOptions = new Cache\Storage\Adapter\RedisOptions($options); + $storage = new Cache\Storage\Adapter\Redis($redisOptions); + } + public function testRedisSerializer() { $this->_storage->addPlugin(new \Zend\Cache\Storage\Plugin\Serializer()); @@ -185,21 +212,21 @@ public function testGetSetPassword() public function testGetSetLibOptionsOnExistingRedisResourceInstance() { - $options = ['serializer', RedisResource::SERIALIZER_PHP]; + $options = ['serializer' => RedisResource::SERIALIZER_PHP]; $this->_options->setLibOptions($options); $value = ['value']; $key = 'key'; //test if it's still possible to set/get item and if lib serializer works $this->_storage->setItem($key, $value); + $this->assertEquals( $value, $this->_storage->getItem($key), 'Redis should return an array, lib options were not set correctly' ); - - $options = ['serializer', RedisResource::SERIALIZER_NONE]; + $options = ['serializer' => RedisResource::SERIALIZER_NONE]; $this->_options->setLibOptions($options); $this->_storage->setItem($key, $value); //should not serialize array correctly @@ -212,7 +239,7 @@ public function testGetSetLibOptionsOnExistingRedisResourceInstance() public function testGetSetLibOptionsWithCleanRedisResourceInstance() { - $options = ['serializer', RedisResource::SERIALIZER_PHP]; + $options = ['serializer' => RedisResource::SERIALIZER_PHP]; $this->_options->setLibOptions($options); $redis = new Cache\Storage\Adapter\Redis($this->_options); @@ -227,7 +254,7 @@ public function testGetSetLibOptionsWithCleanRedisResourceInstance() ); - $options = ['serializer', RedisResource::SERIALIZER_NONE]; + $options = ['serializer' => RedisResource::SERIALIZER_NONE]; $this->_options->setLibOptions($options); $redis->setItem($key, $value); //should not serialize array correctly @@ -285,7 +312,7 @@ public function testGetSetPersistentId() public function testOptionsGetSetLibOptions() { - $options = ['serializer', RedisResource::SERIALIZER_PHP]; + $options = ['serializer' => RedisResource::SERIALIZER_PHP]; $this->_options->setLibOptions($options); $this->assertEquals( $options, From aa84a0a5d15f569c42338cd83f9701099be70f80 Mon Sep 17 00:00:00 2001 From: Thomas Vargiu Date: Wed, 18 Apr 2018 03:54:39 +0200 Subject: [PATCH 2/3] Fixed Redis adapter setOptions and resource check --- src/Storage/Adapter/RedisResourceManager.php | 42 +++++++++---------- .../Adapter/RedisResourceManagerTest.php | 6 ++- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/Storage/Adapter/RedisResourceManager.php b/src/Storage/Adapter/RedisResourceManager.php index fa4da1ac8..1943f288b 100644 --- a/src/Storage/Adapter/RedisResourceManager.php +++ b/src/Storage/Adapter/RedisResourceManager.php @@ -147,6 +147,8 @@ public function getResource($id) $resource['resource'] = $redis; $this->connect($resource); + $this->normalizeLibOptions($resource['lib_options']); + foreach ($resource['lib_options'] as $k => $v) { $redis->setOption($k, $v); } @@ -340,7 +342,6 @@ public function setResource($id, $resource) $resource = array_merge($defaults, $resource); // normalize and validate params $this->normalizePersistentId($resource['persistent_id']); - $this->normalizeLibOptions($resource['lib_options']); // #6495 note: order is important here, as `normalizeServer` applies destructive // transformations on $resource['server'] @@ -394,9 +395,9 @@ public function setPersistentId($id, $persistentId) } $resource = & $this->resources[$id]; - if ($resource instanceof RedisResource) { + if ($resource['resource'] instanceof RedisResource && $resource['initialized']) { throw new Exception\RuntimeException( - "Can't change persistent id of resource {$id} after instanziation" + "Can't change persistent id of resource {$id} after initialization" ); } @@ -421,12 +422,6 @@ public function getPersistentId($id) $resource = & $this->resources[$id]; - if ($resource instanceof RedisResource) { - throw new Exception\RuntimeException( - "Can't get persistent id of an instantiated redis resource" - ); - } - return $resource['persistent_id']; } @@ -455,19 +450,22 @@ public function setLibOptions($id, array $libOptions) ]); } - $this->normalizeLibOptions($libOptions); $resource = & $this->resources[$id]; $resource['lib_options'] = $libOptions; - if ($resource['resource'] instanceof RedisResource) { - $redis = & $resource['resource']; - if (method_exists($redis, 'setOptions')) { - $redis->setOptions($libOptions); - } else { - foreach ($libOptions as $key => $value) { - $redis->setOption($key, $value); - } + if (! $resource['resource'] instanceof RedisResource) { + return $this; + } + + $this->normalizeLibOptions($libOptions); + $redis = & $resource['resource']; + + if (method_exists($redis, 'setOptions')) { + $redis->setOptions($libOptions); + } else { + foreach ($libOptions as $key => $value) { + $redis->setOption($key, $value); } } @@ -489,13 +487,13 @@ public function getLibOptions($id) $resource = & $this->resources[$id]; - if ($resource instanceof RedisResource) { + if ($resource['resource'] instanceof RedisResource) { $libOptions = []; $reflection = new ReflectionClass('Redis'); $constants = $reflection->getConstants(); foreach ($constants as $constName => $constValue) { if (substr($constName, 0, 4) == 'OPT_') { - $libOptions[$constValue] = $resource->getOption($constValue); + $libOptions[$constValue] = $resource['resource']->getOption($constValue); } } return $libOptions; @@ -533,8 +531,8 @@ public function getLibOption($id, $key) $this->normalizeLibOptionKey($key); $resource = & $this->resources[$id]; - if ($resource instanceof RedisResource) { - return $resource->getOption($key); + if ($resource['resource'] instanceof RedisResource) { + return $resource['resource']->getOption($key); } return isset($resource['lib_options'][$key]) ? $resource['lib_options'][$key] : null; diff --git a/test/Storage/Adapter/RedisResourceManagerTest.php b/test/Storage/Adapter/RedisResourceManagerTest.php index f2e3b7f5a..95df58e7c 100644 --- a/test/Storage/Adapter/RedisResourceManagerTest.php +++ b/test/Storage/Adapter/RedisResourceManagerTest.php @@ -115,7 +115,8 @@ public function testValidPersistentId() $resource = [ 'persistent_id' => 'my_connection_name', 'server' => [ - 'host' => 'localhost' + 'host' => getenv('TESTS_ZEND_CACHE_REDIS_HOST') ?: 'localhost', + 'port' => getenv('TESTS_ZEND_CACHE_REDIS_PORT') ?: 6379, ], ]; $expectedPersistentId = 'my_connection_name'; @@ -141,7 +142,8 @@ public function testNotValidPersistentIdOptionName() $resource = [ 'persistend_id' => 'my_connection_name', 'server' => [ - 'host' => 'localhost' + 'host' => getenv('TESTS_ZEND_CACHE_REDIS_HOST') ?: 'localhost', + 'port' => getenv('TESTS_ZEND_CACHE_REDIS_PORT') ?: 6379, ], ]; $expectedPersistentId = 'my_connection_name'; From 390dfff200a8f0bce92ae55c443475410b226de0 Mon Sep 17 00:00:00 2001 From: Thomas Vargiu Date: Wed, 18 Apr 2018 11:29:11 +0200 Subject: [PATCH 3/3] Added assertion on test --- test/Storage/Adapter/RedisTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Storage/Adapter/RedisTest.php b/test/Storage/Adapter/RedisTest.php index 6ade66add..ad398718f 100644 --- a/test/Storage/Adapter/RedisTest.php +++ b/test/Storage/Adapter/RedisTest.php @@ -109,6 +109,8 @@ public function testLibOptionsFirst() $redisOptions = new Cache\Storage\Adapter\RedisOptions($options); $storage = new Cache\Storage\Adapter\Redis($redisOptions); + + $this->assertInstanceOf('Zend\\Cache\\Storage\\Adapter\\Redis', $storage); } public function testRedisSerializer()