Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[BUG] Redis Storage won't behave correctly after libOptions were set #4603

Closed
wants to merge 4 commits into from

2 participants

@piteer1

This is a PR that extends PR #4577 and solves issue #4576

Because Redis doesn't allow for some reason setting options before connect it required some additional fixing than the PR mentioned earlier. Also added test checking if lib options are working correctly.

piteer1 added some commits
@piteer1 piteer1 Divided tests for Redis lib options into existing instance and new in…
…stance, removed unnecessary listening in Redis, added trigger of an event when lib option is set, removed unnecessary reconnect
e01d265
@piteer1 piteer1 Removed unnecessary comment and array in EventManager attach 809786d
@weierophinney weierophinney closed this pull request from a commit
@weierophinney weierophinney Merge branch 'hotfix/4603'
Close #4603
Fixes #4576
Fixes #4577
b0da7f2
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney Merge branch 'hotfix/4603'
Close #4603
Fixes #4576
Fixes #4577
fe9ec2b
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-cache
@weierophinney weierophinney Merge pull request zendframework/zf2#4603 from piteer1/hotfix-redis-l…
…iboption

[BUG] Redis Storage won't behave correctly after libOptions were set
0272d02
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-cache
@weierophinney weierophinney Merge branch 'hotfix/4603' f2b29a0
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-cache
@weierophinney weierophinney Merge branch 'hotfix/4603' into develop 5ab27d6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 6, 2013
  1. @piteer1

    Problem with setting password through RedisOptions (missing methods),…

    piteer1 authored
    … added invokable to AdapterPluginManager
Commits on Jun 7, 2013
  1. @piteer1

    Repaired liboptions behaviour

    piteer1 authored
Commits on Jun 10, 2013
  1. @piteer1

    Divided tests for Redis lib options into existing instance and new in…

    piteer1 authored
    …stance, removed unnecessary listening in Redis, added trigger of an event when lib option is set, removed unnecessary reconnect
  2. @piteer1
This page is out of date. Refresh to see the latest.
View
6 library/Zend/Cache/Storage/Adapter/Redis.php
@@ -80,7 +80,6 @@ public function __construct($options = null)
*/
protected function getRedisResource()
{
-
if (!$this->initialized) {
$options = $this->getOptions();
@@ -222,9 +221,10 @@ protected function internalHasItem(& $normalizedKey)
*/
protected function internalSetItem(& $normalizedKey, & $value)
{
+ $redis = $this->getRedisResource();
+ $ttl = $this->getOptions()->getTtl();
+
try {
- $redis = $this->getRedisResource();
- $ttl = $this->getOptions()->getTtl();
if ($ttl) {
if ($this->resourceManager->getMayorVersion($this->resourceId) < 2) {
throw new Exception\UnsupportedMethodCallException("To use ttl you need version >= 2.0.0");
View
24 library/Zend/Cache/Storage/Adapter/RedisOptions.php
@@ -172,6 +172,7 @@ public function setPersistentId($persistentId)
*/
public function setLibOptions(array $libOptions)
{
+ $this->triggerOptionEvent('lib_option', $libOptions);
$this->getResourceManager()->setLibOptions($this->getResourceId(), $libOptions);
return $this;
}
@@ -237,4 +238,27 @@ public function getDatabase()
{
return $this->getResourceManager()->getDatabase($this->getResourceId());
}
+
+ /**
+ * Set resource password
+ *
+ * @param string $password Password
+ *
+ * @return RedisOptions
+ */
+ public function setPassword($password)
+ {
+ $this->getResourceManager()->setPassword($this->getResourceId(), $password);
+ return $this;
+ }
+
+ /**
+ * Get resource password
+ *
+ * @return string
+ */
+ public function getPassword()
+ {
+ return $this->getResourceManager()->getPassword($this->getResourceId());
+ }
}
View
22 library/Zend/Cache/Storage/Adapter/RedisResourceManager.php
@@ -66,13 +66,13 @@ public function getResource($id)
$redis = new RedisResource();
+ $resource['resource'] = $redis;
+ $this->connect($resource);
+
foreach ($resource['lib_options'] as $k => $v) {
$redis->setOption($k, $v);
}
- $resource['resource'] = $redis;
- $this->connect($resource);
-
$info = $redis->info();
$resource['version'] = $info['redis_version'];
$this->resources[$id]['resource'] = $redis;
@@ -107,6 +107,7 @@ protected function connect(array & $resource)
if (!$success) {
throw new Exception\RuntimeException('Could not estabilish connection with Redis instance');
}
+
$resource['initialized'] = true;
if ($resource['password']) {
$redis->auth($resource['password']);
@@ -256,18 +257,19 @@ public function setLibOptions($id, array $libOptions)
}
$this->normalizeLibOptions($libOptions);
-
$resource = & $this->resources[$id];
- if ($resource instanceof RedisResource) {
- if (method_exists($resource, 'setOptions')) {
- $resource->setOptions($libOptions);
+
+ $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) {
- $resource->setOption($key, $value);
+ $redis->setOption($key, $value);
}
}
- } else {
- $resource['lib_options'] = $libOptions;
}
return $this;
View
1  library/Zend/Cache/Storage/AdapterPluginManager.php
@@ -32,6 +32,7 @@ class AdapterPluginManager extends AbstractPluginManager
'filesystem' => 'Zend\Cache\Storage\Adapter\Filesystem',
'memcached' => 'Zend\Cache\Storage\Adapter\Memcached',
'memory' => 'Zend\Cache\Storage\Adapter\Memory',
+ 'redis' => 'Zend\Cache\Storage\Adapter\Redis',
'session' => 'Zend\Cache\Storage\Adapter\Session',
'xcache' => 'Zend\Cache\Storage\Adapter\XCache',
'wincache' => 'Zend\Cache\Storage\Adapter\WinCache',
View
46 tests/ZendTest/Cache/Storage/Adapter/RedisTest.php
@@ -191,6 +191,45 @@ public function testGetSetPassword()
);
}
+ public function testGetSetLibOptionsOnExistingRedisResourceInstance()
+ {
+ $options = array('serializer', RedisResource::SERIALIZER_PHP);
+ $this->_options->setLibOptions($options);
+
+ $value = array('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 = array('serializer', RedisResource::SERIALIZER_NONE);
+ $this->_options->setLibOptions($options);
+ $this->_storage->setItem($key, $value);
+ //should not serialize array correctly
+ $this->assertFalse(is_array($this->_storage->getItem($key)), 'Redis should not serialize automatically anymore, lib options were not set correctly');
+ }
+
+ public function testGetSetLibOptionsWithCleanRedisResourceInstance()
+ {
+ $options = array('serializer', RedisResource::SERIALIZER_PHP);
+ $this->_options->setLibOptions($options);
+
+ $redis = new Cache\Storage\Adapter\Redis($this->_options);
+ $value = array('value');
+ $key = 'key';
+ //test if it's still possible to set/get item and if lib serializer works
+ $redis->setItem($key, $value);
+ $this->assertEquals($value, $redis->getItem($key), 'Redis should return an array, lib options were not set correctly');
+
+
+ $options = array('serializer', RedisResource::SERIALIZER_NONE);
+ $this->_options->setLibOptions($options);
+ $redis->setItem($key, $value);
+ //should not serialize array correctly
+ $this->assertFalse(is_array($redis->getItem($key)), 'Redis should not serialize automatically anymore, lib options were not set correctly');
+ }
+
/* RedisOptions */
public function testGetSetNamespace()
@@ -261,4 +300,11 @@ public function testOptionsGetSetDatabase()
$this->assertEquals($database, $this->_options->getDatabase(), 'Database not set correctly using RedisOptions');
}
+ public function testOptionsGetSetPassword()
+ {
+ $password = 'my-secret';
+ $this->_options->setPassword($password);
+ $this->assertEquals($password, $this->_options->getPassword(), 'Password was set incorrectly using RedisOptions');
+ }
+
}
Something went wrong with that request. Please try again.