merged branch trivago/webprofiler_redis_add_select (PR #7177)
This PR was squashed before being merged into the 2.1 branch (closes #7177).
Commits
-------
c82c754
RedisProfilerStorage wrong db-number/index-number selected
Discussion
----------
RedisProfilerStorage wrong db-number/index-number selected
bug: in the webprofiler the wrong database (0) is selected when storage should not go to index 0. on redis connect the default behaviour is to select index/database number 0, but it is necessary to select a special index/database.
see: http://rediscookbook.org/multiple_databases.html
[HttpKernel] [Profiler] [RedisProfilerStorage] added select for a db-number/db-index to dsn-patttern
| Q | A
| ------------- | ---
| Bug fix? | [yes]
| New feature? | [no]
| BC breaks? | [no]
| Deprecations? | [no]
| Tests pass? | [yes]
| Fixed tickets |
| License | MIT
Usage with index/db-number = 7 :
```xml
<!-- config_dev.xml -->
<symfony:profiler only-exceptions="false" dsn="redis://127.0.0.1:6379/7" lifetime="3600" />
```
```yml
#config_dev.yml
framework:
...
profiler:
...
dsn: redis://127.0.0.1:6379/7
```
---------------------------------------------------------------------------
by fabpot at 2013-02-27T07:21:02Z
Can you also add some unit tests?
---------------------------------------------------------------------------
by JohnDoe007 at 2013-02-27T09:36:28Z
@fabpot the function getRedis() is currently not testable, I think. this means I have to do a bigger change to the class, add a public function initRedis() or something and move initialize code there to make the initialization process, where my change is, testable. is this ok to do it in that bugfix/pull request? or should I add this test change to master and only adding this small bugfix to the 2.1-branch?
---------------------------------------------------------------------------
by fabpot at 2013-03-06T16:47:37Z
Thanks, I've merged the fix. Can you work on the changes needed to allow the code to be testable (on the master branch)?
This commit is contained in:
commit
380306717c
@ -193,12 +193,13 @@ class RedisProfilerStorage implements ProfilerStorageInterface
|
||||
protected function getRedis()
|
||||
{
|
||||
if (null === $this->redis) {
|
||||
if (!preg_match('#^redis://(?(?=\[.*\])\[(.*)\]|(.*)):(.*)$#', $this->dsn, $matches)) {
|
||||
throw new \RuntimeException(sprintf('Please check your configuration. You are trying to use Redis with an invalid dsn "%s". The expected format is "redis://[host]:port".', $this->dsn));
|
||||
if (!preg_match('#^redis://(?(?=\[.*\])\[(.*)\]|(.*)):(\d+)(/(\d+))?$#', $this->dsn, $matches)) {
|
||||
throw new \RuntimeException(sprintf('Please check your configuration. You are trying to use Redis with an invalid dsn "%s". The expected format is "redis://[host]:port[/db-number]".', $this->dsn));
|
||||
}
|
||||
|
||||
$host = $matches[1] ?: $matches[2];
|
||||
$port = $matches[3];
|
||||
$dbnum = !empty($matches[5]) ? intval($matches[5]) : -1;
|
||||
|
||||
if (!extension_loaded('redis')) {
|
||||
throw new \RuntimeException('RedisProfilerStorage requires that the redis extension is loaded.');
|
||||
@ -206,6 +207,11 @@ class RedisProfilerStorage implements ProfilerStorageInterface
|
||||
|
||||
$redis = new Redis;
|
||||
$redis->connect($host, $port);
|
||||
|
||||
// if a valid dbnumber is given select the redis index
|
||||
if (-1 < $dbnum) {
|
||||
$redis->select($dbnum);
|
||||
}
|
||||
|
||||
$redis->setOption(self::REDIS_OPT_PREFIX, self::TOKEN_PREFIX);
|
||||
|
||||
|
@ -238,4 +238,17 @@ class RedisMock
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
public function select($dbnum)
|
||||
{
|
||||
if (!$this->connected) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (0 > $dbnum) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user