Remove env placeholder regexp

[B-Galati]

fix #356

This is an internal detail that is not covered by the BC policy of Symfony and can thus break on any version.
We now detect if env variable is used by checking the 3rd parameter of method ContainerBuilder::resolveEnvPlaceholders

Note that we still have an issue with profile option that is not resolved at runtime.

[curry684]

Thanks! I'll see when I can test this soon-ish.

[B-Galati]

@curry684 As a second step I would be very happy to merge SncRedisExtensionEnvTest and SncRedisExtensionTest tests into one and use file fixtures instead of string. It would make these tests more simple and easier to manage and it looks to be a common testing practice in Symfony Bundles to use file fixture to load some bundle config.

[curry684]

Thanks!

[tgalopin]

I am not sure to understand the details of this PR, but I think it broke my setup (Redis::connect(): php_network_getaddresses: getaddrinfo failed: Name does not resolve).

I debugged a bit: as parseDsn isn't executed anymore if an environment variable is detected, RedisDsn::getHost now returns null when called. However, in https://github.com/snc/SncRedisBundle/blob/master/DependencyInjection/SncRedisExtension.php#L294 the code calls it to generate the service definition during build, which leads to the following generated code:

<?php

use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;

// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
// Returns the public 'snc_redis.session.handler' shared service.

$a = new \Snc\RedisBundle\Client\Phpredis\Client(array('alias' => 'default'), ($this->privates['snc_redis.logger'] ?? $this->getSncRedis_LoggerService()));
$a->connect(NULL, 6379, 5);
$a->setOption(1, 0);

return $this->services['snc_redis.session.handler'] = new \Snc\RedisBundle\Session\Storage\Handler\RedisSessionHandler($a, $this->parameters['session.storage.options'], 'session', true, 150000);

As you can see, the client tries to connect to null as a host, which fails. I reverted for now (back to 2.1.2, which works) but I think this is a BC break. I am using the following configuration:

snc_redis:
    clients:
        default:
            type: phpredis
            alias: default
            dsn: 'redis://%env(REDIS_HOST)%'

    session:
        client: default

I can try to create a reproducer but I don't think it should be difficult to create one using a basic Symfony 4 project as it's a container build issue.

Thanks for this great bundle and your work in open-source :) !

[B-Galati]

I confirm the issue, it happened only with phpredis ; which always use hardcoded env placeholders in fact :

• https://github.com/snc/SncRedisBundle/blob/master/DependencyInjection/Configuration/RedisDsn.php#L205
• https://github.com/snc/SncRedisBundle/blob/master/DependencyInjection/Configuration/RedisDsn.php#L210

Instead we should use a sort of Snc\RedisBundle\Factory\EnvParametersFactory for phpredis as well.

I guess the issue should be reopened too.

[curry684]

Reopened #356

[B-Galati]

@curry684 @tgalopin I'll try to make a fix in coming weeks