Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #21607 Improve tracking of environment variables in the case of p…
…rivate services (tgalopin)

This PR was merged into the 3.2 branch.

Discussion
----------

Improve tracking of environment variables in the case of private services

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21273
| License       | MIT

Recently, I stumbled upon an issue with environment variables that I had troubles to understand. Here it is:

I have two environment variables, defined in my `parameters.yml` with a default value of empty:

``` yaml
parameters:
    env(MAILJET_PUBLIC_KEY): ''
    env(MAILJET_PRIVATE_KEY): ''
```

I use these variables only in a private service:

``` xml
<service id="app.mailjet.transport" class="AppBundle\Mailjet\MailjetApiTransport" public="false">
    <argument type="service" id="csa_guzzle.client.mailjet_api"/>
    <argument>%env(MAILJET_PUBLIC_KEY)%</argument>
    <argument>%env(MAILJET_PRIVATE_KEY)%</argument>
</service>
```

This private service is not used by any other service for the moment.

As the service is private and not used by any other service, the `RemoveUnusedDefinitionsPass` removes it from the container. However, when the container dumper dumps the container, it checks if the environment variable was dumped. As the service is now gone, an expection is thrown (`EnvParameterException, Incompatible use of dynamic environment variables "MAILJET_PUBLIC_KEY", "MAILJET_PRIVATE_KEY" found in parameters.`).

Commits
-------

f380ba3 Improve tracking of environment variables in the case of private services
  • Loading branch information
nicolas-grekas committed Feb 14, 2017
2 parents 43588ca + f380ba3 commit 0b46a1b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
Expand Up @@ -72,6 +72,7 @@ public function process(ContainerBuilder $container)
$compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'replaces alias '.reset($referencingAliases)));
} elseif (0 === count($referencingAliases) && false === $isReferenced) {
$container->removeDefinition($id);
$container->resolveEnvPlaceholders(serialize($definition));
$compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'unused'));
$hasChanged = true;
}
Expand Down
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass;
use Symfony\Component\DependencyInjection\Compiler\RepeatedPass;
use Symfony\Component\DependencyInjection\Compiler\RemoveUnusedDefinitionsPass;
use Symfony\Component\DependencyInjection\Compiler\ResolveParameterPlaceHoldersPass;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ContainerBuilder;
Expand Down Expand Up @@ -105,6 +106,28 @@ public function testProcessWontRemovePrivateFactory()
$this->assertTrue($container->hasDefinition('foobar'));
}

public function testProcessConsiderEnvVariablesAsUsedEvenInPrivateServices()
{
$container = new ContainerBuilder();
$container->setParameter('env(FOOBAR)', 'test');
$container
->register('foo')
->setArguments(array('%env(FOOBAR)%'))
->setPublic(false)
;

$resolvePass = new ResolveParameterPlaceHoldersPass();
$resolvePass->process($container);

$this->process($container);

$this->assertFalse($container->hasDefinition('foo'));

$envCounters = $container->getEnvCounters();
$this->assertArrayHasKey('FOOBAR', $envCounters);
$this->assertSame(1, $envCounters['FOOBAR']);
}

protected function process(ContainerBuilder $container)
{
$repeatedPass = new RepeatedPass(array(new AnalyzeServiceReferencesPass(), new RemoveUnusedDefinitionsPass()));
Expand Down

0 comments on commit 0b46a1b

Please sign in to comment.