Skip to content

Commit

Permalink
Use RedisDsn to build connection options for env based config (#439)
Browse files Browse the repository at this point in the history
fix #425

- Always use PredisParametersFactory to build connection options so that it creates consistency between env/non-env based config
- Use RedisDsn to parse redis DSN instead of \Predis\Connection\Parameters::parse(). It creates consistence between phpredis and predis behavior.
  • Loading branch information
B-Galati authored and curry684 committed Jun 25, 2018
1 parent 9bb9dd6 commit 3f3c233
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 114 deletions.
32 changes: 5 additions & 27 deletions DependencyInjection/SncRedisExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Snc\RedisBundle\DependencyInjection\Configuration\Configuration;
use Snc\RedisBundle\DependencyInjection\Configuration\RedisDsn;
use Snc\RedisBundle\DependencyInjection\Configuration\RedisEnvDsn;
use Snc\RedisBundle\Factory\PredisParametersFactory;
use Symfony\Component\DependencyInjection\Alias;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
Expand Down Expand Up @@ -167,25 +168,6 @@ protected function loadPredisClient(array $client, ContainerBuilder $container)
$connection['logging'] = $client['logging'];
$connection['alias'] = $connectionAlias;

if ($dsn instanceof RedisDsn) {
if (null !== $dsn->getSocket()) {
$connection['scheme'] = 'unix';
$connection['path'] = $dsn->getSocket();
} else {
$connection['scheme'] = 'tcp';
$connection['host'] = $dsn->getHost();
$connection['port'] = $dsn->getPort();
if (null !== $dsn->getDatabase()) {
$connection['path'] = $dsn->getDatabase();
}
}
if (null !== $dsn->getDatabase()) {
$connection['database'] = $dsn->getDatabase();
}
$connection['password'] = $dsn->getPassword();
$connection['weight'] = $dsn->getWeight();
}

$this->loadPredisConnectionParameters($client['alias'], $connection, $container, $dsn);
}

Expand Down Expand Up @@ -241,18 +223,14 @@ protected function loadPredisClient(array $client, ContainerBuilder $container)
protected function loadPredisConnectionParameters($clientAlias, array $connection, ContainerBuilder $container, $dsn)
{
$parametersClass = $container->getParameter('snc_redis.connection_parameters.class');

$parameterId = sprintf('snc_redis.connection.%s_parameters.%s', $connection['alias'], $clientAlias);

$parameterDef = new Definition($parametersClass);
$parameterDef->setPublic(false);
$parameterDef->setFactory(array('Snc\RedisBundle\Factory\PredisParametersFactory', 'create'));
$parameterDef->addArgument($connection);

if ($dsn instanceof RedisEnvDsn) {
$parameterDef->setFactory(array('Snc\RedisBundle\Factory\PredisEnvParametersFactory', 'create'));
$parameterDef->addArgument($parametersClass);
$parameterDef->addArgument((string) $dsn);
}

$parameterDef->addArgument($parametersClass);
$parameterDef->addArgument((string) $dsn);
$parameterDef->addTag('snc_redis.connection_parameters', array('clientAlias' => $clientAlias));
$container->setDefinition($parameterId, $parameterDef);
}
Expand Down
30 changes: 0 additions & 30 deletions Factory/PredisEnvParametersFactory.php

This file was deleted.

55 changes: 55 additions & 0 deletions Factory/PredisParametersFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php

namespace Snc\RedisBundle\Factory;

use Predis\Connection\ParametersInterface;
use Snc\RedisBundle\DependencyInjection\Configuration\RedisDsn;

class PredisParametersFactory
{
/**
* @param array $options
* @param string $class
* @param string $dsn
*
* @return ParametersInterface
*/
public static function create($options, $class, $dsn)
{
if (!is_a($class, '\Predis\Connection\ParametersInterface', true)) {
throw new \InvalidArgumentException(sprintf('%s::%s requires $class argument to implement %s', __CLASS__, __METHOD__, '\Predis\Connection\ParametersInterface'));
}

$dsnOptions = static::parseDsn(new RedisDsn($dsn));
$dsnOptions = array_merge($options, $dsnOptions);

return new $class($dsnOptions);
}

/**
* @param RedisDsn $dsn
*
* @return array
*/
private static function parseDsn(RedisDsn $dsn)
{
if (null !== $dsn->getSocket()) {
$options['scheme'] = 'unix';
$options['path'] = $dsn->getSocket();
} else {
$options['scheme'] = 'tcp';
$options['host'] = $dsn->getHost();
$options['port'] = $dsn->getPort();
if (null !== $dsn->getDatabase()) {
$options['path'] = $dsn->getDatabase();
}
}
if (null !== $dsn->getDatabase()) {
$options['database'] = $dsn->getDatabase();
}
$options['password'] = $dsn->getPassword();
$options['weight'] = $dsn->getWeight();

return $options;
}
}
2 changes: 1 addition & 1 deletion Tests/DependencyInjection/SncRedisExtensionEnvTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public function testPredisDefaultParameterConfigLoad()
$container = $this->getConfiguredContainer('env_predis_minimal');

$this->assertSame(
array('Snc\RedisBundle\Factory\PredisEnvParametersFactory', 'create'),
array('Snc\RedisBundle\Factory\PredisParametersFactory', 'create'),
$container->findDefinition('snc_redis.connection.default_parameters.default')->getFactory()
);
}
Expand Down
56 changes: 0 additions & 56 deletions Tests/Factory/PredisEnvParametersFactoryTest.php

This file was deleted.

97 changes: 97 additions & 0 deletions Tests/Factory/PredisParametersFactoryTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<?php

namespace Snc\RedisBundle\Tests\Factory;

use PHPUnit\Framework\TestCase;
use Snc\RedisBundle\Factory\PredisParametersFactory;

class PredisParametersFactoryTest extends TestCase
{
public function createDp()
{
return array(
array(
'redis://z:df577d779b4f724c8c29b5eff5bcc534b732722b9df308a661f1b79014175063d5@ec2-34-321-123-45.us-east-1.compute.amazonaws.com:3210',
'Predis\Connection\Parameters',
array(
'test' => 123,
'some' => 'string',
'arbitrary' => true,
'values' => array(1, 2, 3)
),
array(
'test' => 123,
'some' => 'string',
'arbitrary' => true,
'values' => array(1, 2, 3),
'scheme' => 'tcp',
'host' => 'ec2-34-321-123-45.us-east-1.compute.amazonaws.com',
'port' => 3210,
'path' => null,
'alias' => null,
'timeout' => null,
'read_write_timeout' => null,
'async_connect' => null,
'tcp_nodelay' => null,
'persistent' => null,
'password' => 'df577d779b4f724c8c29b5eff5bcc534b732722b9df308a661f1b79014175063d5',
'database' => null,
),
),
array(
'redis://pw@/var/run/redis/redis-1.sock/10',
'Predis\Connection\Parameters',
array(
'test' => 124,
'password' => 'toto',
'alias' => 'one_alias',
),
array(
'test' => 124,
'scheme' => 'unix',
'host' => '127.0.0.1',
'port' => 6379,
'path' => '/var/run/redis/redis-1.sock',
'alias' => 'one_alias',
'timeout' => null,
'read_write_timeout' => null,
'async_connect' => null,
'tcp_nodelay' => null,
'persistent' => null,
'password' => 'pw',
'database' => 10,
),
)
);
}

/**
* @param string $dsn
* @param string $class
* @param array $options
* @param array $expectedParameters
*
* @dataProvider createDp
*/
public function testCreate($dsn, $class, $options, $expectedParameters)
{
$parameters = PredisParametersFactory::create($options, $class, $dsn);

$this->assertInstanceOf($class, $parameters);

foreach ($expectedParameters as $name => $value) {
$this->assertSame($value, $parameters->{$name}, "Wrong '$name' value");
}

// No user can exist within a redis connection.
$this->assertObjectNotHasAttribute('user', $parameters);
}

/**
* @expectedException \InvalidArgumentException
*/
public function testCreateException()
{
PredisParametersFactory::create(array(), '\stdClass', 'redis://localhost');
}
}

0 comments on commit 3f3c233

Please sign in to comment.