Permalink
Browse files

minor #24103 [FrameworkBundle] Fix Di config to allow for more privat…

…e services (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Fix Di config to allow for more private services

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This PR is a prelude to #23822:
- allow cache pool clearers to be private by using IGNORE_ON_UNINITIALIZED_REFERENCE, allowing much more sane logic in the related passes
- allow templating helpers to be private by using a service locator
- replace an inline def by a reference in workflow's config (ping @lyrixx @Nyholm)
- fix the Stopwatch alias, which is public in 3.4, but private in 3.3
- remove direct access to `->get('fragment.handler')` in tests so that the service could be made private

Commits
-------

76c4217 [FrameworkBundle] Fix Di config to allow for more private services
  • Loading branch information...
fabpot committed Sep 5, 2017
2 parents aad90fa + 76c4217 commit 4f02e91592e595580126d37d371e4fbb901f9f76
@@ -27,29 +27,16 @@
public function process(ContainerBuilder $container) public function process(ContainerBuilder $container)
{ {
$container->getParameterBag()->remove('cache.prefix.seed'); $container->getParameterBag()->remove('cache.prefix.seed');
$poolsByClearer = array();
$pools = array();
foreach ($container->findTaggedServiceIds('cache.pool') as $id => $attributes) { foreach ($container->findTaggedServiceIds('cache.pool.clearer') as $id => $attr) {
$pools[$id] = new Reference($id); $clearer = $container->getDefinition($id);
foreach (array_reverse($attributes) as $attr) { $pools = array();
if (isset($attr['clearer'])) { foreach ($clearer->getArgument(0) as $id => $ref) {
$poolsByClearer[$attr['clearer']][$id] = $pools[$id]; if ($container->hasDefinition($id)) {
} $pools[$id] = new Reference($id);
if (!empty($attr['unlazy'])) {
$container->getDefinition($id)->setLazy(false);
}
if (array_key_exists('clearer', $attr) || array_key_exists('unlazy', $attr)) {
break;
} }
} }
} $clearer->replaceArgument(0, $pools);
$container->getDefinition('cache.global_clearer')->addArgument($pools);
foreach ($poolsByClearer as $clearer => $pools) {
$clearer = $container->getDefinition($clearer);
$clearer->addArgument($pools);
} }
if (!$container->has('cache.annotations')) { if (!$container->has('cache.annotations')) {
@@ -37,6 +37,8 @@ public function process(ContainerBuilder $container)
} }
$seed .= '.'.$container->getParameter('kernel.name').'.'.$container->getParameter('kernel.environment'); $seed .= '.'.$container->getParameter('kernel.name').'.'.$container->getParameter('kernel.environment');
$pools = array();
$clearers = array();
$attributes = array( $attributes = array(
'provider', 'provider',
'namespace', 'namespace',
@@ -47,10 +49,8 @@ public function process(ContainerBuilder $container)
if ($pool->isAbstract()) { if ($pool->isAbstract()) {
continue; continue;
} }
$isLazy = $pool->isLazy();
while ($adapter instanceof ChildDefinition) { while ($adapter instanceof ChildDefinition) {
$adapter = $container->findDefinition($adapter->getParent()); $adapter = $container->findDefinition($adapter->getParent());
$isLazy = $isLazy || $adapter->isLazy();
if ($t = $adapter->getTag('cache.pool')) { if ($t = $adapter->getTag('cache.pool')) {
$tags[0] += $t[0]; $tags[0] += $t[0];
} }
@@ -82,17 +82,29 @@ public function process(ContainerBuilder $container)
throw new InvalidArgumentException(sprintf('Invalid "cache.pool" tag for service "%s": accepted attributes are "clearer", "provider", "namespace" and "default_lifetime", found "%s".', $id, implode('", "', array_keys($tags[0])))); throw new InvalidArgumentException(sprintf('Invalid "cache.pool" tag for service "%s": accepted attributes are "clearer", "provider", "namespace" and "default_lifetime", found "%s".', $id, implode('", "', array_keys($tags[0]))));
} }
$attr = array();
if (null !== $clearer) { if (null !== $clearer) {
$attr['clearer'] = $clearer; $clearers[$clearer][$id] = new Reference($id, $container::IGNORE_ON_UNINITIALIZED_REFERENCE);
} }
if (!$isLazy) {
$pool->setLazy(true); $pools[$id] = new Reference($id, $container::IGNORE_ON_UNINITIALIZED_REFERENCE);
$attr['unlazy'] = true; }
}
if ($attr) { $clearer = 'cache.global_clearer';
$pool->addTag('cache.pool', $attr); while ($container->hasAlias($clearer)) {
$clearer = (string) $container->getAlias($clearer);
}
if ($container->hasDefinition($clearer)) {
$clearers['cache.global_clearer'] = $pools;
}
foreach ($clearers as $id => $pools) {
$clearer = $container->getDefinition($id);
if ($clearer instanceof ChilDefinition) {
$clearer->replaceArgument(0, $pools);
} else {
$clearer->setArgument(0, $pools);
} }
$clearer->addTag('cache.pool.clearer');
} }
} }
@@ -15,6 +15,7 @@
use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\Alias;
use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Templating\EngineInterface as ComponentEngineInterface; use Symfony\Component\Templating\EngineInterface as ComponentEngineInterface;
class TemplatingPass implements CompilerPassInterface class TemplatingPass implements CompilerPassInterface
@@ -31,16 +32,22 @@ public function process(ContainerBuilder $container)
} }
if ($container->hasDefinition('templating.engine.php')) { if ($container->hasDefinition('templating.engine.php')) {
$refs = array();
$helpers = array(); $helpers = array();
foreach ($container->findTaggedServiceIds('templating.helper', true) as $id => $attributes) { foreach ($container->findTaggedServiceIds('templating.helper', true) as $id => $attributes) {
if (isset($attributes[0]['alias'])) { if (isset($attributes[0]['alias'])) {
$helpers[$attributes[0]['alias']] = $id; $helpers[$attributes[0]['alias']] = $id;
$refs[$id] = new Reference($id);
} }
} }
if (count($helpers) > 0) { if (count($helpers) > 0) {
$definition = $container->getDefinition('templating.engine.php'); $definition = $container->getDefinition('templating.engine.php');
$definition->addMethodCall('setHelpers', array($helpers)); $definition->addMethodCall('setHelpers', array($helpers));
if ($container->hasDefinition('templating.engine.php.helpers_locator')) {
$container->getDefinition('templating.engine.php.helpers_locator')->replaceArgument(0, $refs);
}
} }
} }
} }
@@ -22,6 +22,7 @@
class UnusedTagsPass implements CompilerPassInterface class UnusedTagsPass implements CompilerPassInterface
{ {
private $whitelist = array( private $whitelist = array(
'cache.pool.clearer',
'console.command', 'console.command',
'container.service_locator', 'container.service_locator',
'container.service_subscriber', 'container.service_subscriber',
@@ -597,15 +597,15 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde
} }
// Create Workflow // Create Workflow
$workflowId = sprintf('%s.%s', $type, $name);
$workflowDefinition = new ChildDefinition(sprintf('%s.abstract', $type)); $workflowDefinition = new ChildDefinition(sprintf('%s.abstract', $type));
$workflowDefinition->replaceArgument(0, $definitionDefinition); $workflowDefinition->replaceArgument(0, new Reference(sprintf('%s.definition', $workflowId)));
if (isset($markingStoreDefinition)) { if (isset($markingStoreDefinition)) {
$workflowDefinition->replaceArgument(1, $markingStoreDefinition); $workflowDefinition->replaceArgument(1, $markingStoreDefinition);
} }
$workflowDefinition->replaceArgument(3, $name); $workflowDefinition->replaceArgument(3, $name);
// Store to container // Store to container
$workflowId = sprintf('%s.%s', $type, $name);
$container->setDefinition($workflowId, $workflowDefinition); $container->setDefinition($workflowId, $workflowDefinition);
$container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition); $container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition);
@@ -680,7 +680,7 @@ private function registerDebugConfiguration(array $config, ContainerBuilder $con
if (class_exists(Stopwatch::class)) { if (class_exists(Stopwatch::class)) {
$container->register('debug.stopwatch', Stopwatch::class)->addArgument(true); $container->register('debug.stopwatch', Stopwatch::class)->addArgument(true);
$container->setAlias(Stopwatch::class, 'debug.stopwatch'); $container->setAlias(Stopwatch::class, new Alias('debug.stopwatch', false));
} }
$debug = $container->getParameter('kernel.debug'); $debug = $container->getParameter('kernel.debug');
@@ -106,7 +106,7 @@ public function build(ContainerBuilder $container)
$container->addCompilerPass(new AddExpressionLanguageProvidersPass()); $container->addCompilerPass(new AddExpressionLanguageProvidersPass());
$this->addCompilerPassIfExists($container, TranslationExtractorPass::class); $this->addCompilerPassIfExists($container, TranslationExtractorPass::class);
$this->addCompilerPassIfExists($container, TranslationDumperPass::class); $this->addCompilerPassIfExists($container, TranslationDumperPass::class);
$container->addCompilerPass(new FragmentRendererPass(), PassConfig::TYPE_AFTER_REMOVING); $container->addCompilerPass(new FragmentRendererPass());
$this->addCompilerPassIfExists($container, SerializerPass::class); $this->addCompilerPassIfExists($container, SerializerPass::class);
$this->addCompilerPassIfExists($container, PropertyInfoPass::class); $this->addCompilerPassIfExists($container, PropertyInfoPass::class);
$container->addCompilerPass(new DataCollectorTranslatorPass()); $container->addCompilerPass(new DataCollectorTranslatorPass());
@@ -9,7 +9,7 @@
<service id="debug.templating.engine.php" class="Symfony\Bundle\FrameworkBundle\Templating\TimedPhpEngine"> <service id="debug.templating.engine.php" class="Symfony\Bundle\FrameworkBundle\Templating\TimedPhpEngine">
<argument type="service" id="templating.name_parser" /> <argument type="service" id="templating.name_parser" />
<argument type="service" id="service_container" /> <argument type="service" id="templating.engine.php.helpers_locator" />
<argument type="service" id="templating.loader" /> <argument type="service" id="templating.loader" />
<argument type="service" id="debug.stopwatch" /> <argument type="service" id="debug.stopwatch" />
<argument type="service" id="templating.globals" /> <argument type="service" id="templating.globals" />
@@ -9,12 +9,17 @@
<service id="templating.engine.php" class="Symfony\Bundle\FrameworkBundle\Templating\PhpEngine"> <service id="templating.engine.php" class="Symfony\Bundle\FrameworkBundle\Templating\PhpEngine">
<argument type="service" id="templating.name_parser" /> <argument type="service" id="templating.name_parser" />
<argument type="service" id="service_container" /> <argument type="service" id="templating.engine.php.helpers_locator" />
<argument type="service" id="templating.loader" /> <argument type="service" id="templating.loader" />
<argument type="service" id="templating.globals" /> <argument type="service" id="templating.globals" />
<call method="setCharset"><argument>%kernel.charset%</argument></call> <call method="setCharset"><argument>%kernel.charset%</argument></call>
</service> </service>
<service id="templating.engine.php.helpers_locator">
<tag name="container.service_locator" />
<argument type="collection" />
</service>
<service id="templating.helper.slots" class="Symfony\Component\Templating\Helper\SlotsHelper" public="true"> <service id="templating.helper.slots" class="Symfony\Component\Templating\Helper\SlotsHelper" public="true">
<tag name="templating.helper" alias="slots" /> <tag name="templating.helper" alias="slots" />
</service> </service>
@@ -226,7 +226,7 @@ public function block(FormView $view, $blockName, array $variables = array())
* Check the token in your action using the same CSRF token id. * Check the token in your action using the same CSRF token id.
* *
* <code> * <code>
* $csrfProvider = $this->get('security.csrf.token_generator'); * // $csrfProvider being an instance of Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface
* if (!$csrfProvider->isCsrfTokenValid('rm_user_'.$user->getId(), $token)) { * if (!$csrfProvider->isCsrfTokenValid('rm_user_'.$user->getId(), $token)) {
* throw new \RuntimeException('CSRF attack detected.'); * throw new \RuntimeException('CSRF attack detected.');
* } * }
@@ -21,10 +21,8 @@ class SubRequestController implements ContainerAwareInterface
{ {
use ContainerAwareTrait; use ContainerAwareTrait;
public function indexAction() public function indexAction($handler)
{ {
$handler = $this->container->get('fragment.handler');
$errorUrl = $this->generateUrl('subrequest_fragment_error', array('_locale' => 'fr', '_format' => 'json')); $errorUrl = $this->generateUrl('subrequest_fragment_error', array('_locale' => 'fr', '_format' => 'json'));
$altUrl = $this->generateUrl('subrequest_fragment', array('_locale' => 'fr', '_format' => 'json')); $altUrl = $this->generateUrl('subrequest_fragment', array('_locale' => 'fr', '_format' => 'json'));
@@ -1,2 +1,7 @@
imports: imports:
- { resource: ./../config/default.yml } - { resource: ./../config/default.yml }
services:
Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SubRequestController:
tags:
- { name: controller.service_arguments, action: indexAction, argument: handler, id: fragment.handler }
@@ -17,7 +17,8 @@
use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Dumper\PhpDumper; use Symfony\Component\EventDispatcher\DependencyInjection\RegisterListenersPass;
use Symfony\Component\EventDispatcher\EventDispatcher;
class WebProfilerExtensionTest extends TestCase class WebProfilerExtensionTest extends TestCase
{ {
@@ -51,6 +52,7 @@ protected function setUp()
$this->kernel = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\KernelInterface')->getMock(); $this->kernel = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\KernelInterface')->getMock();
$this->container = new ContainerBuilder(); $this->container = new ContainerBuilder();
$this->container->register('event_dispatcher', EventDispatcher::class);
$this->container->register('router', $this->getMockClass('Symfony\\Component\\Routing\\RouterInterface')); $this->container->register('router', $this->getMockClass('Symfony\\Component\\Routing\\RouterInterface'));
$this->container->register('twig', 'Twig\Environment'); $this->container->register('twig', 'Twig\Environment');
$this->container->register('twig_loader', 'Twig\Loader\ArrayLoader')->addArgument(array()); $this->container->register('twig_loader', 'Twig\Loader\ArrayLoader')->addArgument(array());
@@ -66,6 +68,7 @@ protected function setUp()
->addArgument(new Definition($this->getMockClass('Symfony\\Component\\HttpKernel\\Profiler\\ProfilerStorageInterface'))); ->addArgument(new Definition($this->getMockClass('Symfony\\Component\\HttpKernel\\Profiler\\ProfilerStorageInterface')));
$this->container->setParameter('data_collector.templates', array()); $this->container->setParameter('data_collector.templates', array());
$this->container->set('kernel', $this->kernel); $this->container->set('kernel', $this->kernel);
$this->container->addCompilerPass(new RegisterListenersPass());
} }
protected function tearDown() protected function tearDown()
@@ -88,7 +91,7 @@ public function testDefaultConfig($debug)
$this->assertFalse($this->container->has('web_profiler.debug_toolbar')); $this->assertFalse($this->container->has('web_profiler.debug_toolbar'));
$this->assertSaneContainer($this->getDumpedContainer()); $this->assertSaneContainer($this->getCompiledContainer());
} }
/** /**
@@ -101,7 +104,7 @@ public function testToolbarConfig($toolbarEnabled, $interceptRedirects, $listene
$this->assertSame($listenerInjected, $this->container->has('web_profiler.debug_toolbar')); $this->assertSame($listenerInjected, $this->container->has('web_profiler.debug_toolbar'));
$this->assertSaneContainer($this->getDumpedContainer(), '', array('web_profiler.csp.handler')); $this->assertSaneContainer($this->getCompiledContainer(), '', array('web_profiler.csp.handler'));
if ($listenerInjected) { if ($listenerInjected) {
$this->assertSame($listenerEnabled, $this->container->get('web_profiler.debug_toolbar')->isEnabled()); $this->assertSame($listenerEnabled, $this->container->get('web_profiler.debug_toolbar')->isEnabled());
@@ -118,19 +121,11 @@ public function getDebugModes()
); );
} }
private function getDumpedContainer() private function getCompiledContainer()
{ {
static $i = 0;
$class = 'WebProfilerExtensionTestContainer'.$i++;
$this->container->compile(); $this->container->compile();
$this->container->set('kernel', $this->kernel);
$dumper = new PhpDumper($this->container); return $this->container;
eval('?>'.$dumper->dump(array('class' => $class)));
$container = new $class();
$container->set('kernel', $this->kernel);
return $container;
} }
} }
@@ -76,7 +76,7 @@ public function searchAndRenderBlock(FormView $view, $blockNameSuffix, array $va
* Check the token in your action using the same token ID. * Check the token in your action using the same token ID.
* *
* <code> * <code>
* $csrfProvider = $this->get('security.csrf.token_generator'); * // $csrfProvider being an instance of Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface
* if (!$csrfProvider->isCsrfTokenValid('rm_user_'.$user->getId(), $token)) { * if (!$csrfProvider->isCsrfTokenValid('rm_user_'.$user->getId(), $token)) {
* throw new \RuntimeException('CSRF attack detected.'); * throw new \RuntimeException('CSRF attack detected.');
* } * }

0 comments on commit 4f02e91

Please sign in to comment.