Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
1 change: 1 addition & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<php>
<ini name="error_reporting" value="-1"/>
<env name="SHELL_VERBOSITY" value="-1"/>
<env name="REDIS_DSNS" value='["redis://sncredis@127.0.0.1/3","redis://sncredis@127.0.0.1/4","redis://sncredis@127.0.0.1/5"]'/>
</php>
<testsuites>
<testsuite name="RedisBundle Test Suite">
Expand Down
5 changes: 5 additions & 0 deletions psalm.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
<referencedMethod name="Symfony\Component\Config\Definition\Builder\NodeBuilder::end"/>
</errorLevel>
</UnusedMethodCall>
<PossiblyUnusedMethod>
<errorLevel type="suppress">
<referencedMethod name="Snc\RedisBundle\Tests\Functional\App\Controller\Controller::__construct"/>
</errorLevel>
</PossiblyUnusedMethod>
Comment thread
ostrolucky marked this conversation as resolved.
Outdated
</issueHandlers>
<plugins>
Comment on lines +17 to 29
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

better to do inline suppression

<pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
Expand Down
3 changes: 2 additions & 1 deletion src/DependencyInjection/SncRedisExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ private function loadPredisClient(array $client, ContainerBuilder $container): v
$container->setDefinition($optionId, $optionDef);
$clientDef = new Definition($client['class'] ?? (string) $container->getParameter('snc_redis.client.class'));
$clientDef->addTag('snc_redis.client', ['alias' => $client['alias']]);
if ($connectionCount === 1 && !isset($client['options']['cluster']) && !isset($client['options']['replication'])) {

if ($connectionCount === 1 && !isset($client['options']['replication'])) {
$clientDef->addArgument(new Reference(sprintf('snc_redis.connection.%s_parameters.%s', $connectionAliases[0], $client['alias'])));
} else {
$connections = [];
Expand Down
32 changes: 31 additions & 1 deletion src/Factory/PredisParametersFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,51 @@
use Snc\RedisBundle\DependencyInjection\Configuration\RedisDsn;

use function array_filter;
use function array_map;
use function array_merge;
use function constant;
use function count;
use function defined;
use function is_a;
use function is_array;
use function is_string;
use function sprintf;
use function str_replace;

/** @internal */
class PredisParametersFactory
{
/**
* @param class-string<ParametersInterface> $class
* @param array<string, mixed> $options
* @param string|list<string>|list<list<string>> $dsn
*
* @return ParametersInterface|list<ParametersInterface>
*/
public static function create(array $options, string $class, string|array $dsn): ParametersInterface|array
{
if (is_string($dsn)) {
$dsn = [$dsn];
}

// json:/csv: env processors can produce a single-element array wrapping the actual list
if (count($dsn) === 1 && is_array($dsn[0])) {
$dsn = $dsn[0];
}

$parameters = array_map(
static fn (string $d) => static::createFromSingleDsn($options, $class, $d),
$dsn,
);

return count($parameters) === 1 ? $parameters[0] : $parameters;
}

/**
* @param class-string<ParametersInterface> $class
* @param array<string, mixed> $options
*/
public static function create(array $options, string $class, string $dsn): ParametersInterface
private static function createFromSingleDsn(array $options, string $class, string $dsn): ParametersInterface
{
if (!is_a($class, ParametersInterface::class, true)) {
throw new InvalidArgumentException(sprintf('%s::%s requires $class argument to implement %s', self::class, __METHOD__, ParametersInterface::class));
Expand Down
24 changes: 24 additions & 0 deletions tests/Factory/PredisParametersFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,28 @@ public function testCreateMergesSslWithTlsVersion(): void
$this->assertSame(STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT, $ssl['crypto_type']);
$this->assertFalse($ssl['verify_peer']);
}

/**
* @testWith ["redis://localhost"]
* [["redis://localhost"]]
* [[["redis://localhost"]]]
*/
public function testCreateReturnsSingleParameters(string|array $dsn): void
{
$result = PredisParametersFactory::create([], Parameters::class, $dsn);
$this->assertInstanceOf(Parameters::class, $result);
}

/**
* @param array<int, string|array<int, string>> $dsn
*
* @testWith [["redis://host1", "redis://host2"]]
* [[["redis://host1", "redis://host2"]]]
*/
public function testCreateReturnsMultipleParameters(array $dsn): void
{
$result = PredisParametersFactory::create([], Parameters::class, $dsn);
$this->assertIsArray($result);
$this->assertCount(2, $result);
}
}
18 changes: 13 additions & 5 deletions tests/Functional/App/Controller/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,26 @@

namespace Snc\RedisBundle\Tests\Functional\App\Controller;

use Predis\ClientInterface;
use Redis;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\JsonResponse;

class Controller extends AbstractController
{
public function __invoke(Redis $redis): JsonResponse
/** @param iterable<Redis|ClientInterface> $clients */
Comment thread
ostrolucky marked this conversation as resolved.
Outdated
public function __construct(private iterable $clients)
{
$redis->set('foo', 'bar');
}

public function __invoke(): JsonResponse
{
$result = null;
foreach ($this->clients as $client) {
$client->set('foo', 'bar');
$result = $client->get('foo');
}

return new JsonResponse([
'result' => $redis->get('foo'),
]);
return new JsonResponse(['result' => $result]);
}
}
22 changes: 11 additions & 11 deletions tests/Functional/App/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,9 @@ snc_redis:
cluster:
type: predis
alias: cluster
dsn:
- redis://sncredis@127.0.0.1/3
- redis://sncredis@127.0.0.1/4
- redis://sncredis@127.0.0.1/5
dsn: "%env(json:REDIS_DSNS)%"
options:
cluster: predis
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The cluster: predis option is required here because the DSN is an env var resolved at runtime via json:. At compile time Symfony sees a single opaque placeholder, so it cannot count the connections — Predis needs to know the aggregation mode upfront in order to build the right connection factory.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That's the whole point why env vars were not supported for Predis. Predis needs to know connectionCount at build time, where env vars are not resolved yet. Think of a solution.

prefix: foo
connection_timeout: 10
connection_persistent: true
Expand All @@ -59,20 +57,14 @@ snc_redis:
with_acl:
type: predis
alias: with_acl
dsn: redis://localhost/1
dsn: redis://snc_redis:snc_password@localhost:7099/1
logging: false
options:
parameters:
username: my_user
password: sncredis

services:
_defaults:
autowire: true
autoconfigure: true
public: false
bind:
Predis\Client $predisReplication: '@snc_redis.predis_replication'

Redis: '@snc_redis.default'

Expand All @@ -87,6 +79,14 @@ services:
resource: './Controller'
tags: ['controller.service_arguments']

Snc\RedisBundle\Tests\Functional\App\Controller\PredisReplication:
bind:
Predis\Client $predisReplication: '@snc_redis.predis_replication'

Snc\RedisBundle\Tests\Functional\App\Controller\Controller:
arguments:
$clients: !tagged_iterator snc_redis.client

var_dumper.cli_dumper:
class: Symfony\Component\VarDumper\Dumper\CliDumper
arguments: ['/dev/null']
4 changes: 2 additions & 2 deletions tests/Functional/IntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ public function testIntegration(): void
$this->assertInstanceOf(RedisDataCollector::class, $collector);
$this->assertInstanceOf(ResetInterface::class, $container = $this->client->getKernel()->getContainer());
$this->assertInstanceOf(RedisLogger::class, $redisLogger = $container->get('test.snc_redis.logger'));
$this->assertSame(5, $redisLogger->getNbCommands());
$this->assertCount(5, $collector->getCommands());
$this->assertSame(13, $redisLogger->getNbCommands());
$this->assertCount(13, $collector->getCommands());
$container->reset();
$this->assertSame(0, $redisLogger->getNbCommands());
}
Expand Down
Loading