merged branch fabpot/request-stack (PR #8904)

This PR was merged into the master branch.

Discussion
----------

Synchronized Service alternative, backwards compatible

This is a rebased version #7707.

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7707
| License       | MIT
| Doc PR        | symfony/symfony-docs#2956

Todo/Questions

 - [x] do we deprecate the synchronize feature (and removed it in 3.0)?
 - [x] deal with BC for listeners
 - [x] rename RequestContext as we already have a class with the same name in the Routing component?
 - [x] move RequestStack and RequestContext to HttpFoundation?
 - [x] update documentation

Prototype for introducing a ``RequestContext`` in HttpKernel.

This PR keeps the synchronized services feature, however introduces a ``RequestContext`` object additionally, that allows to avoid using synchronized service when injecting ``request_context`` instead of ``request`` into a service. The FrameworkBundle is modified such that it does not depend on synchronized services anymore. Users however can still depend on ``request``, activating the synchronized services feature.

Features:

* Introduced ``REQUEST_FINSHED`` (name is up for grabs) event to handle global state and environment cleanup that should not be done in ``RESPONSE``. Called in both exception or success case correctly
* Changed listeners that were synchronized before to using ``onKernelRequestFinished`` and ``RequestContext`` to reset to the parent request (RouterListener + LocaleListener).
* Changed ``FragmentHandler`` to use the RequestContext. Added some more tests for this class.

* ``RequestStack`` is injected into the ``HttpKernel`` to handle the request finished event and push/pop the stack with requests.

Todos:
* RequestContext name clashes with Routing components RequestContext. Keep or make unique?
* Name for Kernel Request Finished Event could be improved.

Commits
-------

1b2ef74 [Security] made sure that the exception listener is always removed from the event dispatcher at the end of the request
b1a062d moved RequestStack to HttpFoundation and removed RequestContext
93e60ea [HttpKernel] modified listeners to be BC with Symfony <2.4
018b719 [HttpKernel] tweaked the code
f9b10ba [HttpKernel] renamed the kernel finished event
a58a8a6 [HttpKernel] changed request_stack to a private service
c55f1ea added a RequestStack class

src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php

@@ -14,6 +14,8 @@
 use Symfony\Bridge\Twig\Extension\HttpKernelExtension;
 use Symfony\Bridge\Twig\Tests\TestCase;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpKernel\Fragment\FragmentHandler;
 
 class HttpKernelExtensionTest extends TestCase
@@ -23,13 +25,30 @@ class HttpKernelExtensionTest extends TestCase
      */
     public function testFragmentWithError()
     {
-        $kernel = $this->getFragmentHandler($this->throwException(new \Exception('foo')));
+        $renderer = $this->getFragmentHandler($this->throwException(new \Exception('foo')));
 
-        $loader = new \Twig_Loader_Array(array('index' => '{{ fragment("foo") }}'));
-        $twig = new \Twig_Environment($loader, array('debug' => true, 'cache' => false));
-        $twig->addExtension(new HttpKernelExtension($kernel));
+        $this->renderTemplate($renderer);
+    }
+
+    public function testRenderFragment()
+    {
+        $renderer = $this->getFragmentHandler($this->returnValue(new Response('html')));
+
+        $response = $this->renderTemplate($renderer);
 
-        $this->renderTemplate($kernel);
+        $this->assertEquals('html', $response);
+    }
+
+    public function testUnknownFragmentRenderer()
+    {
+        $context = $this->getMockBuilder('Symfony\\Component\\HttpFoundation\\RequestStack')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+        $renderer = new FragmentHandler(array(), false, $context);
+
+        $this->setExpectedException('InvalidArgumentException', 'The "inline" renderer does not exist.');
+        $renderer->render('/foo');
     }
 
     protected function getFragmentHandler($return)
@@ -38,8 +57,14 @@ protected function getFragmentHandler($return)
         $strategy->expects($this->once())->method('getName')->will($this->returnValue('inline'));
         $strategy->expects($this->once())->method('render')->will($return);
 
-        $renderer = new FragmentHandler(array($strategy));
-        $renderer->setRequest(Request::create('/'));
+        $context = $this->getMockBuilder('Symfony\\Component\\HttpFoundation\\RequestStack')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+
+        $context->expects($this->any())->method('getCurrentRequest')->will($this->returnValue(Request::create('/')));
+
+        $renderer = new FragmentHandler(array($strategy), false, $context);
 
         return $renderer;
     }

src/Symfony/Bundle/FrameworkBundle/Resources/config/fragment_renderer.xml

@@ -17,7 +17,7 @@
         <service id="fragment.handler" class="%fragment.handler.class%">
             <argument type="collection" />
             <argument>%kernel.debug%</argument>
-            <call method="setRequest"><argument type="service" id="request" on-invalid="null" strict="false" /></call>
+            <argument type="service" id="request_stack" />
         </service>
 
         <service id="fragment.renderer.inline" class="%fragment.renderer.inline.class%">

src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml

@@ -94,7 +94,7 @@
             <argument type="service" id="router" />
             <argument type="service" id="router.request_context" on-invalid="ignore" />
             <argument type="service" id="logger" on-invalid="ignore" />
-            <call method="setRequest"><argument type="service" id="request" on-invalid="null" strict="false" /></call>
+            <argument type="service" id="request_stack" />
         </service>
     </services>
 </container>

src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml

@@ -12,6 +12,7 @@
         <parameter key="cache_clearer.class">Symfony\Component\HttpKernel\CacheClearer\ChainCacheClearer</parameter>
         <parameter key="file_locator.class">Symfony\Component\HttpKernel\Config\FileLocator</parameter>
         <parameter key="uri_signer.class">Symfony\Component\HttpKernel\UriSigner</parameter>
+        <parameter key="request_stack.class">Symfony\Component\HttpFoundation\RequestStack</parameter>
     </parameters>
 
     <services>
@@ -23,8 +24,11 @@
             <argument type="service" id="event_dispatcher" />
             <argument type="service" id="service_container" />
             <argument type="service" id="controller_resolver" />
+            <argument type="service" id="request_stack" />
         </service>
 
+        <service id="request_stack" class="%request_stack.class%" />
+
         <service id="cache_warmer" class="%cache_warmer.class%">
             <argument type="collection" />
         </service>

src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml

@@ -38,7 +38,7 @@
             <tag name="kernel.event_subscriber" />
             <argument>%kernel.default_locale%</argument>
             <argument type="service" id="router" on-invalid="ignore" />
-            <call method="setRequest"><argument type="service" id="request" on-invalid="null" strict="false" /></call>
+            <argument type="service" id="request_stack" />
         </service>
     </services>
 </container>

src/Symfony/Component/HttpFoundation/CHANGELOG.md

@@ -4,6 +4,7 @@ CHANGELOG
 2.4.0
 -----
 
+ * added RequestStack
  * added Request::getEncodings()
  * added accessors methods to session handlers
 

src/Symfony/Component/HttpFoundation/RequestStack.php

@@ -0,0 +1,103 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpFoundation;
+
+/**
+ * Request stack that controls the lifecycle of requests.
+ *
+ * @author Benjamin Eberlei <kontakt@beberlei.de>
+ */
+class RequestStack
+{
+    /**
+     * @var Request[]
+     */
+    private $requests = array();
+
+    /**
+     * Pushes a Request on the stack.
+     *
+     * This method should generally not be called directly as the stack
+     * management should be taken care of by the application itself.
+     */
+    public function push(Request $request)
+    {
+        $this->requests[] = $request;
+    }
+
+    /**
+     * Pops the current request from the stack.
+     *
+     * This operation lets the current request go out of scope.
+     *
+     * This method should generally not be called directly as the stack
+     * management should be taken care of by the application itself.
+     *
+     * @return Request
+     */
+    public function pop()
+    {
+        if (!$this->requests) {
+            throw new \LogicException('Unable to pop a Request as the stack is already empty.');
+        }
+
+        return array_pop($this->requests);
+    }
+
+    /**
+     * @return Request|null
+     */
+    public function getCurrentRequest()
+    {
+        return end($this->requests) ?: null;
+    }
+
+    /**
+     * Gets the master Request.
+     *
+     * Be warned that making your code aware of the master request
+     * might make it un-compatible with other features of your framework
+     * like ESI support.
+     *
+     * @return Request|null
+     */
+    public function getMasterRequest()
+    {
+        if (!$this->requests) {
+            return null;
+        }
+
+        return $this->requests[0];
+    }
+
+    /**
+     * Returns the parent request of the current.
+     *
+     * Be warned that making your code aware of the parent request
+     * might make it un-compatible with other features of your framework
+     * like ESI support.
+     *
+     * If current Request is the master request, it returns null.
+     *
+     * @return Request|null
+     */
+    public function getParentRequest()
+    {
+        $pos = count($this->requests) - 2;
+
+        if (!isset($this->requests[$pos])) {
+            return null;
+        }
+
+        return $this->requests[$pos];
+    }
+}

src/Symfony/Component/HttpKernel/CHANGELOG.md

@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+2.4.0
+-----
+
+ * added the KernelEvents::FINISH_REQUEST event
+
 2.3.0
 -----
 

src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php

@@ -12,6 +12,7 @@
 namespace Symfony\Component\HttpKernel\DependencyInjection;
 
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\HttpKernel\HttpKernel;
 use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
@@ -35,10 +36,11 @@ class ContainerAwareHttpKernel extends HttpKernel
      * @param EventDispatcherInterface    $dispatcher         An EventDispatcherInterface instance
      * @param ContainerInterface          $container          A ContainerInterface instance
      * @param ControllerResolverInterface $controllerResolver A ControllerResolverInterface instance
+     * @param RequestStack                $requestStack       A stack for master/sub requests
      */
-    public function __construct(EventDispatcherInterface $dispatcher, ContainerInterface $container, ControllerResolverInterface $controllerResolver)
+    public function __construct(EventDispatcherInterface $dispatcher, ContainerInterface $container, ControllerResolverInterface $controllerResolver, RequestStack $requestStack = null)
     {
-        parent::__construct($dispatcher, $controllerResolver);
+        parent::__construct($dispatcher, $controllerResolver, $requestStack);
 
         $this->container = $container;
 

src/Symfony/Component/HttpKernel/Event/FinishRequestEvent.php

@@ -0,0 +1,21 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpKernel\Event;
+
+/**
+ * Triggered whenever a request is fully processed.
+ *
+ * @author Benjamin Eberlei <kontakt@beberlei.de>
+ */
+class FinishRequestEvent extends KernelEvent
+{
+}

src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php

@@ -12,55 +12,100 @@
 namespace Symfony\Component\HttpKernel\EventListener;
 
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Routing\RequestContextAwareInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
 /**
  * Initializes the locale based on the current request.
  *
+ * This listener works in 2 modes:
+ *
+ *  * 2.3 compatibility mode where you must call setRequest whenever the Request changes.
+ *  * 2.4+ mode where you must pass a RequestStack instance in the constructor.
+ *
  * @author Fabien Potencier <fabien@symfony.com>
  */
 class LocaleListener implements EventSubscriberInterface
 {
     private $router;
     private $defaultLocale;
+    private $requestStack;
 
-    public function __construct($defaultLocale = 'en', RequestContextAwareInterface $router = null)
+    /**
+     * RequestStack will become required in 3.0.
+     */
+    public function __construct($defaultLocale = 'en', RequestContextAwareInterface $router = null, RequestStack $requestStack = null)
     {
         $this->defaultLocale = $defaultLocale;
+        $this->requestStack = $requestStack;
         $this->router = $router;
     }
 
+    /**
+     * Sets the current Request.
+     *
+     * This method was used to synchronize the Request, but as the HttpKernel
+     * is doing that automatically now, you should never be called it directly.
+     * It is kept public for BC with the 2.3 version.
+     *
+     * @param Request|null $request A Request instance
+     *
+     * @deprecated Deprecated since version 2.4, to be removed in 3.0.
+     */
     public function setRequest(Request $request = null)
     {
         if (null === $request) {
             return;
         }
 
-        if ($locale = $request->attributes->get('_locale')) {
-            $request->setLocale($locale);
-        }
-
-        if (null !== $this->router) {
-            $this->router->getContext()->setParameter('_locale', $request->getLocale());
-        }
+        $this->setLocale($request);
+        $this->setRouterContext($request);
     }
 
     public function onKernelRequest(GetResponseEvent $event)
     {
         $request = $event->getRequest();
         $request->setDefaultLocale($this->defaultLocale);
 
-        $this->setRequest($request);
+        $this->setLocale($request);
+        $this->setRouterContext($request);
+    }
+
+    public function onKernelFinishRequest(FinishRequestEvent $event)
+    {
+        if (null === $this->requestStack) {
+            throw new \LogicException('You must pass a RequestStack.');
+        }
+
+        if (null !== $parentRequest = $this->requestStack->getParentRequest()) {
+            $this->setRouterContext($parentRequest);
+        }
+    }
+
+    private function setLocale(Request $request)
+    {
+        if ($locale = $request->attributes->get('_locale')) {
+            $request->setLocale($locale);
+        }
+    }
+
+    private function setRouterContext(Request $request)
+    {
+        if (null !== $this->router) {
+            $this->router->getContext()->setParameter('_locale', $request->getLocale());
+        }
     }
 
     public static function getSubscribedEvents()
     {
         return array(
             // must be registered after the Router to have access to the _locale
             KernelEvents::REQUEST => array(array('onKernelRequest', 16)),
+            KernelEvents::FINISH_REQUEST => array(array('onKernelFinishRequest', 0)),
         );
     }
 }