feature #21625 Remove some container injections in favor of service l…

…ocators (nicolas-grekas, chalasr) This PR was merged into the 3.3-dev branch. Discussion ---------- Remove some container injections in favor of service locators | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #21553 (comment) | License | MIT | Doc PR | n/a Commits ------- 8293b75 Replace some container injections by service locators 0be9ea8 [EventDispatcher] Fix abstract event subscribers registration
symfony · Feb 28, 2017 · 77f7a47 · 77f7a47
2 parents 43bff22 + 8293b75
commit 77f7a47
Show file tree

Hide file tree

Showing 26 changed files with 311 additions and 143 deletions.
diff --git a/UPGRADE-3.3.md b/UPGRADE-3.3.md
@@ -71,11 +71,22 @@ FrameworkBundle
    deprecated and will be removed in 4.0. Use the `Symfony\Component\Form\DependencyInjection\FormPass`
    class instead.
 
+ * The `Symfony\Bundle\FrameworkBundle\EventListener\SessionListener` class has been
+   deprecated and will be removed in 4.0. Use the `Symfony\Component\HttpKernel\EventListener\SessionListener`
+   class instead.
+
+ * The `Symfony\Bundle\FrameworkBundle\EventListener\TestSessionListener` class has been
+   deprecated and will be removed in 4.0. Use the `Symfony\Component\HttpKernel\EventListener\TestSessionListener`
+   class instead.
+
 HttpKernel
 -----------
 
  * The `Psr6CacheClearer::addPool()` method has been deprecated. Pass an array of pools indexed
    by name to the constructor instead.
+
+ * The `LazyLoadingFragmentHandler::addRendererService()` method has been deprecated and
+   will be removed in 4.0.
 
 Process
 -------
@@ -127,6 +138,12 @@ TwigBridge
  * The `TwigRendererEngine::setEnvironment()` method has been deprecated and will be removed
    in 4.0. Pass the Twig Environment as second argument of the constructor instead.
 
+TwigBundle
+----------
+
+* The `ContainerAwareRuntimeLoader` class has been deprecated and will be removed in 4.0.
+  Use the Twig `Twig_ContainerRuntimeLoader` class instead.
+
 Workflow
 --------
 

diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md
@@ -188,6 +188,13 @@ FrameworkBundle
  * The `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\FormPass` class has been
    removed. Use the `Symfony\Component\Form\DependencyInjection\FormPass` class instead.
 
+ * The `Symfony\Bundle\FrameworkBundle\EventListener\SessionListener` class has been removed.
+   Use the `Symfony\Component\HttpKernel\EventListener\SessionListener` class instead.
+
+ * The `Symfony\Bundle\FrameworkBundle\EventListener\TestSessionListener` class has been
+   removed. Use the `Symfony\Component\HttpKernel\EventListener\TestSessionListener`
+   class instead.
+
 HttpFoundation
 ---------------
 
@@ -229,6 +236,8 @@ HttpKernel
 
  * The `Psr6CacheClearer::addPool()` method has been removed. Pass an array of pools indexed
    by name to the constructor instead.
+
+ * The `LazyLoadingFragmentHandler::addRendererService()` method has been removed.
 
 Ldap
 ----
@@ -285,6 +294,12 @@ Translation
 
  * Removed the backup feature from the file dumper classes.
 
+TwigBundle
+----------
+
+* The `ContainerAwareRuntimeLoader` class has been removed. Use the 
+  Twig `Twig_ContainerRuntimeLoader` class instead.
+
 TwigBridge
 ----------
 

diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
@@ -15,6 +15,8 @@ CHANGELOG
  * Added configurable paths for validation files
  * Deprecated `SerializerPass`, use `Symfony\Component\Serializer\DependencyInjection\SerializerPass` instead
  * Deprecated `FormPass`, use `Symfony\Component\Form\DependencyInjection\FormPass` instead
+ * Deprecated `SessionListener`
+ * Deprecated `TestSessionListener`
 
 3.2.0
 -----

diff --git a/src/Symfony/Bundle/FrameworkBundle/EventListener/SessionListener.php b/src/Symfony/Bundle/FrameworkBundle/EventListener/SessionListener.php
@@ -12,31 +12,16 @@
 namespace Symfony\Bundle\FrameworkBundle\EventListener;
 
 use Symfony\Component\HttpKernel\EventListener\SessionListener as BaseSessionListener;
-use Symfony\Component\DependencyInjection\ContainerInterface;
+
+@trigger_error(sprintf('The %s class is deprecated since version 3.3 and will be removed in 4.0. Use %s instead.', SessionListener::class, BaseSessionListener::class), E_USER_DEPRECATED);
 
 /**
  * Sets the session in the request.
  *
  * @author Fabien Potencier <fabien@symfony.com>
+ *
+ * @deprecated since version 3.3, to be removed in 4.0. Use {@link BaseSessionListener} instead
  */
 class SessionListener extends BaseSessionListener
 {
-    /**
-     * @var ContainerInterface
-     */
-    private $container;
-
-    public function __construct(ContainerInterface $container)
-    {
-        $this->container = $container;
-    }
-
-    protected function getSession()
-    {
-        if (!$this->container->has('session')) {
-            return;
-        }
-
-        return $this->container->get('session');
-    }
 }
diff --git a/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php b/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php
@@ -11,15 +11,19 @@
 
 namespace Symfony\Bundle\FrameworkBundle\EventListener;
 
-use Symfony\Component\HttpKernel\EventListener\TestSessionListener as BaseTestSessionListener;
+@trigger_error(sprintf('The %s class is deprecated since version 3.3 and will be removed in 4.0. Use Symfony\Component\HttpKernel\EventListener\TestSessionListener instead.', TestSessionListener::class), E_USER_DEPRECATED);
+
+use Symfony\Component\HttpKernel\EventListener\AbstractTestSessionListener;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * TestSessionListener.
  *
  * @author Fabien Potencier <fabien@symfony.com>
+ *
+ * @deprecated since version 3.3, to be removed in 4.0.
  */
-class TestSessionListener extends BaseTestSessionListener
+class TestSessionListener extends AbstractTestSessionListener
 {
     protected $container;
 

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/fragment_renderer.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/fragment_renderer.xml
@@ -11,7 +11,7 @@
 
     <services>
         <service id="fragment.handler" class="Symfony\Component\HttpKernel\DependencyInjection\LazyLoadingFragmentHandler">
-            <argument type="service" id="service_container" />
+            <argument /> <!-- fragment renderer locator -->
             <argument type="service" id="request_stack" />
             <argument>%kernel.debug%</argument>
         </service>

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml
@@ -47,9 +47,11 @@
 
         <service id="session.handler.write_check" class="Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler" public="false" />
 
-        <service id="session_listener" class="Symfony\Bundle\FrameworkBundle\EventListener\SessionListener">
+        <service id="session_listener" class="Symfony\Component\HttpKernel\EventListener\SessionListener">
             <tag name="kernel.event_subscriber" />
-            <argument type="service" id="service_container" />
+            <argument type="service-locator">
+                <argument key="session" type="service" id="session" on-invalid="null" />
+            </argument>
         </service>
 
         <service id="session.save_listener" class="Symfony\Component\HttpKernel\EventListener\SaveSessionListener">

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/test.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/test.xml
@@ -20,9 +20,11 @@
 
         <service id="test.client.cookiejar" class="Symfony\Component\BrowserKit\CookieJar" shared="false" />
 
-        <service id="test.session.listener" class="Symfony\Bundle\FrameworkBundle\EventListener\TestSessionListener">
-            <argument type="service" id="service_container" />
+        <service id="test.session.listener" class="Symfony\Component\HttpKernel\EventListener\TestSessionListener">
             <tag name="kernel.event_subscriber" />
+            <argument type="service-locator">
+                <argument key="session" type="service" id="session" on-invalid="null" />
+            </argument>
         </service>
     </services>
 </container>
diff --git a/src/Symfony/Bundle/TwigBundle/CHANGELOG.md b/src/Symfony/Bundle/TwigBundle/CHANGELOG.md
@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+3.3.0
+-----
+
+ * Deprecated `ContainerAwareRuntimeLoader`
+
 2.7.0
 -----
 

diff --git a/src/Symfony/Bundle/TwigBundle/ContainerAwareRuntimeLoader.php b/src/Symfony/Bundle/TwigBundle/ContainerAwareRuntimeLoader.php
@@ -11,13 +11,17 @@
 
 namespace Symfony\Bundle\TwigBundle;
 
+@trigger_error(sprintf('The %s class is deprecated since version 3.3 and will be removed in 4.0. Use the Twig_ContainerRuntimeLoader class instead.'), ContainerAwareRuntimeLoader::class);
+
 use Psr\Log\LoggerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * Loads Twig extension runtimes via the service container.
  *
  * @author Fabien Potencier <fabien@symfony.com>
+ *
+ * @deprecated since version 3.3, will be removed in 4.0. Use \Twig_ContainerRuntimeLoader instead.
  */
 class ContainerAwareRuntimeLoader implements \Twig_RuntimeLoaderInterface
 {

diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/RuntimeLoaderPass.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/RuntimeLoaderPass.php
@@ -11,9 +11,10 @@
 
 namespace Symfony\Bundle\TwigBundle\DependencyInjection\Compiler;
 
+use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
-use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
+use Symfony\Component\DependencyInjection\Reference;
 
 /**
  * Registers Twig runtime services.
@@ -31,17 +32,13 @@ public function process(ContainerBuilder $container)
         foreach ($container->findTaggedServiceIds('twig.runtime') as $id => $attributes) {
             $def = $container->getDefinition($id);
 
-            if (!$def->isPublic()) {
-                throw new InvalidArgumentException(sprintf('The service "%s" must be public as it can be lazy-loaded.', $id));
-            }
-
             if ($def->isAbstract()) {
-                throw new InvalidArgumentException(sprintf('The service "%s" must not be abstract as it can be lazy-loaded.', $id));
+                continue;
             }
 
-            $mapping[$def->getClass()] = $id;
+            $mapping[$def->getClass()] = new Reference($id);
         }
 
-        $definition->replaceArgument(1, $mapping);
+        $definition->replaceArgument(0, new ServiceLocatorArgument($mapping));
     }
 }
diff --git a/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml b/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
@@ -138,10 +138,8 @@
             <argument /> <!-- thousands separator, set in TwigExtension -->
         </service>
 
-        <service id="twig.runtime_loader" class="Symfony\Bundle\TwigBundle\ContainerAwareRuntimeLoader" public="false">
-            <argument type="service" id="service_container" />
-            <argument type="collection" /> <!-- the mapping between class names and service names -->
-            <argument type="service" id="logger" on-invalid="null" />
+        <service id="twig.runtime_loader" class="Twig_ContainerRuntimeLoader" public="false">
+            <argument /> <!-- runtime locator -->
         </service>
     </services>
 </container>
diff --git a/src/Symfony/Bundle/TwigBundle/Tests/ContainerAwareRuntimeLoaderTest.php b/src/Symfony/Bundle/TwigBundle/Tests/ContainerAwareRuntimeLoaderTest.php
@@ -15,6 +15,9 @@
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Bundle\TwigBundle\ContainerAwareRuntimeLoader;
 
+/**
+ * @group legacy
+ */
 class ContainerAwareRuntimeLoaderTest extends TestCase
 {
     public function testLoad()

diff --git a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php
@@ -244,7 +244,7 @@ public function testRuntimeLoader()
         $container->compile();
 
         $loader = $container->getDefinition('twig.runtime_loader');
-        $args = $loader->getArgument(1);
+        $args = $loader->getArgument(0)->getValues();
         $this->assertArrayHasKey('Symfony\Bridge\Twig\Form\TwigRenderer', $args);
         $this->assertArrayHasKey('FooClass', $args);
         $this->assertContains('twig.form.renderer', $args);

diff --git a/src/Symfony/Bundle/TwigBundle/composer.json b/src/Symfony/Bundle/TwigBundle/composer.json
@@ -21,7 +21,7 @@
         "symfony/twig-bridge": "^3.2.1",
         "symfony/http-foundation": "~2.8|~3.0",
         "symfony/http-kernel": "~2.8.16|~3.1.9|^3.2.2",
-        "twig/twig": "~1.28|~2.0"
+        "twig/twig": "^1.32|^2.2"
     },
     "require-dev": {
         "symfony/asset": "~2.8|~3.0",

diff --git a/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php b/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php
@@ -16,6 +16,7 @@
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
 use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
 /**
  * Compiler pass to register tagged services for an event dispatcher.
@@ -105,8 +106,8 @@ public function process(ContainerBuilder $container)
             }
             $container->addObjectResource($class);
 
-            $r = new \ReflectionClass($class);
-            $extractingDispatcher->addSubscriber($r->newInstanceWithoutConstructor());
+            ExtractingEventDispatcher::$subscriber = $class;
+            $extractingDispatcher->addSubscriber($extractingDispatcher);
             foreach ($extractingDispatcher->listeners as $args) {
                 $args[1] = new ClosureProxyArgument($id, $args[1]);
                 $definition->addMethodCall('addListener', $args);
@@ -119,12 +120,21 @@ public function process(ContainerBuilder $container)
 /**
  * @internal
  */
-class ExtractingEventDispatcher extends EventDispatcher
+class ExtractingEventDispatcher extends EventDispatcher implements EventSubscriberInterface
 {
     public $listeners = array();
 
+    public static $subscriber;
+
     public function addListener($eventName, $listener, $priority = 0)
     {
         $this->listeners[] = array($eventName, $listener[1], $priority);
     }
+
+    public static function getSubscribedEvents()
+    {
+        $callback = array(self::$subscriber, 'getSubscribedEvents');
+
+        return $callback();
+    }
 }
diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md
@@ -1,6 +1,13 @@
 CHANGELOG
 =========
 
+3.3.0
+-----
+
+ * Deprecated `LazyLoadingFragmentHandler::addRendererService()`
+ * Added `SessionListener`
+ * Added `TestSessionListener`
+
 3.2.0
 -----
 

diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/FragmentRendererPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/FragmentRendererPass.php
@@ -11,9 +11,11 @@
 
 namespace Symfony\Component\HttpKernel\DependencyInjection;
 
+use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
+use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\HttpKernel\Fragment\FragmentRendererInterface;
 
 /**
@@ -43,14 +45,11 @@ public function process(ContainerBuilder $container)
         }
 
         $definition = $container->getDefinition($this->handlerService);
+        $renderers = array();
         foreach ($container->findTaggedServiceIds($this->rendererTag) as $id => $tags) {
             $def = $container->getDefinition($id);
-            if (!$def->isPublic()) {
-                throw new InvalidArgumentException(sprintf('The service "%s" must be public as fragment renderer are lazy-loaded.', $id));
-            }
-
             if ($def->isAbstract()) {
-                throw new InvalidArgumentException(sprintf('The service "%s" must not be abstract as fragment renderer are lazy-loaded.', $id));
+                continue;
             }
 
             $class = $container->getParameterBag()->resolveValue($def->getClass());
@@ -63,8 +62,10 @@ public function process(ContainerBuilder $container)
             }
 
             foreach ($tags as $tag) {
-                $definition->addMethodCall('addRendererService', array($tag['alias'], $id));
+                $renderers[$tag['alias']] = new Reference($id);
             }
         }
+
+        $definition->replaceArgument(0, new ServiceLocatorArgument($renderers));
     }
 }