8000 [HttpFoundation] Allow RedisCluster class for RedisSessionHandler by michaelperrin · Pull Request #28251 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] Allow RedisCluster class for RedisSessionHandler #28251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 26, 2018
Merged

[HttpFoundation] Allow RedisCluster class for RedisSessionHandler #28251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/Symfony/Component/HttpFoundation/Session/Storage/Handler/RedisSessionHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,20 @@ class RedisSessionHandler extends AbstractSessionHandler
* List of available options:
* * prefix: The prefix to use for the keys in order to avoid collision on the Redis server.
*
* @param \Redis|\RedisArray|\RedisCluster|\Predis\Client $redis
* @param array $options An associative array of options
* @param \Redis|\RedisArray|\RedisCluster|\Predis\Client|RedisProxy $redis
* @param array $options An associative array of options
*
* @throws \InvalidArgumentException When unsupported client or options are passed
*/
public function __construct($redis, array $options = array())
{
if (!$redis instanceof \Redis && !$redis instanceof \RedisArray && !$redis instanceof \Predis\Client && !$redis instanceof RedisProxy) {
if (
!$redis instanceof \Redis &&
!$redis instanceof \RedisArray &&
!$redis instanceof \RedisCluster &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas Just to make things easier to see, this is the condition I added

!$redis instanceof \Predis\Client &&
!$redis instanceof RedisProxy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing \, same in the docblock

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you talk about the RedisProxy class? That one is imported at the top of the class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right sorry!

) {
throw new \InvalidArgumentException(sprintf('%s() expects parameter 1 to be Redis, RedisArray, RedisCluster or Predis\Client, %s given', __METHOD__, \is_object($redis) ? \get_class($redis) : \gettype($redis)));
}

Expand Down
18 changes: 5 additions & 13 deletions ...nent/HttpFoundation/Tests/Session/Storage/Handler/AbstractRedisSessionHandlerTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ abstract class AbstractRedisSessionHandlerTestCase extends TestCase
*/
protected $redisClient;

/**
* @var \Redis
*/
protected $validator;

/**
* @return \Redis|\RedisArray|\RedisCluster|\Predis\Client
*/
Expand All @@ -52,9 +47,6 @@ protected function setUp()

$host = getenv('REDIS_HOST') ?: 'localhost';

$this->validator = new \Redis();
$this->validator->connect($host);

$this->redisClient = $this->createRedisClient($host);
$this->storage = new RedisSessionHandler(
$this->redisClient,
Expand Down Expand Up @@ -154,24 +146,24 @@ public function getOptionFixtures(): array
protected function setFixture($key, $value, $ttl = null)
{
if (null !== $ttl) {
$this->validator->setex($key, $ttl, $value);
$this->redisClient->setex($key, $ttl, $value);
} else {
$this->validator->set($key, $value);
$this->redisClient->set($key, $value);
}
}

protected function getFixture($key)
{
return $this->validator->get($key);
return $this->redisClient->get($key);
}

protected function hasFixture($key): bool
{
return $this->validator->exists($key);
return $this->redisClient->exists($key);
}

protected function fixtureTtl($key): int
{
return $this->validator->ttl($key);
return $this->redisClient->ttl($key);
}
}
2 changes: 1 addition & 1 deletion ...omponent/HttpFoundation/Tests/Session/Storage/Handler/PredisClusterSessionHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ class PredisClusterSessionHandlerTest extends AbstractRedisSessionHandlerTestCas
{
protected function createRedisClient(string $host): Client
{
return new Client(array(array('host' => $host)));
return new Client(array(array('host' => $host)));
}
}
31 changes: 31 additions & 0 deletions ...Component/HttpFoundation/Tests/Session/Storage/Handler/RedisClusterSessionHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpFoundation\Tests\Session\Storage\Handler;

class RedisClusterSessionHandlerTest extends AbstractRedisSessionHandlerTestCase
{
public static function setupBeforeClass()
{
if (!class_exists('RedisCluster')) {
self::markTestSkipped('The RedisCluster class is required.');
}

if (!$hosts = getenv('REDIS_CLUSTER_HOSTS')) {
self::markTestSkipped('REDIS_CLUSTER_HOSTS env var is not defined.');
}
}

protected function createRedisClient(string $host): \RedisCluster
{
return new \RedisCluster(null, explode(' ', getenv('REDIS_CLUSTER_HOSTS')));
}
}
0