Move RequestStack into HttpKernel as optional dependency. Introduce n…

…ew REQUEST_FINISHED event that is specfically for cleaning up global+environment state to avoid using RESPONSE event.
symfony · beberlei · Apr 18, 2013 · Apr 18, 2013 · Apr 30, 2013 · Apr 30, 2013
commit 705f0d4165eb05c5063fc221a34e857f1808c8f8
diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php b/src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php
@@ -42,9 +42,8 @@ class ContainerAwareHttpKernel extends HttpKernel
      */
     public function __construct(EventDispatcherInterface $dispatcher, ContainerInterface $container, ControllerResolverInterface $controllerResolver, RequestStack $requestStack)
     {
-        parent::__construct($dispatcher, $controllerResolver);
+        parent::__construct($dispatcher, $controllerResolver, $requestStack);
 
-        $this->requestStack = $requestStack;
         $this->container = $container;
 
         $container->addScope(new Scope('request'));
@@ -55,8 +54,6 @@ public function __construct(EventDispatcherInterface $dispatcher, ContainerInter
      */
     public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQUEST, $catch = true)
     {
-        $this->requestStack->push($request);
-
         $request->headers->set('X-Php-Ob-Level', ob_get_level());
 
         $this->container->enterScope('request');
@@ -74,8 +71,6 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
         $this->container->set('request', null, 'request');
         $this->container->leaveScope('request');
 
-        $this->requestStack->pop();
-
         return $response;
     }
 }
diff --git a/src/Symfony/Component/HttpKernel/Event/RequestFinishedEvent.php b/src/Symfony/Component/HttpKernel/Event/RequestFinishedEvent.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\HttpKernel\Event;
+
+class RequestFinishedEvent extends KernelEvent
+{
+}
diff --git a/src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php b/src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php
@@ -12,7 +12,7 @@
 namespace Symfony\Component\HttpKernel\EventListener;
 
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
-use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\Event\RequestFinishedEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\HttpKernel\RequestContext;
 use Symfony\Component\HttpFoundation\Request;
@@ -46,7 +46,7 @@ public function onKernelRequest(GetResponseEvent $event)
         $this->setRouterContext($request);
     }
 
-    public function onKernelResponse(FilterResponseEvent $event)
+    public function onKernelRequestFinished(RequestFinishedEvent $event)
     {
         $this->resetRouterContext();
     }
@@ -85,8 +85,7 @@ public static function getSubscribedEvents()
         return array(
             // must be registered after the Router to have access to the _locale
             KernelEvents::REQUEST => array(array('onKernelRequest', 16)),
-            // should be registered very late
-            KernelEvents::RESPONSE => array(array('onKernelResponse', -255)),
+            KernelEvents::REQUEST_FINISHED => array(array('onKernelRequestFinished', 0)),
         );
     }
 }
diff --git a/src/Symfony/Component/HttpKernel/EventListener/RouterListener.php b/src/Symfony/Component/HttpKernel/EventListener/RouterListener.php
@@ -13,7 +13,7 @@
 
 use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
-use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\Event\RequestFinishedEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
@@ -81,7 +81,7 @@ public function setRequest(Request $request = null)
         }
     }
 
-    public function onKernelResponse(FilterResponseEvent $event)
+    public function onKernelRequestFinished(RequestFinishedEvent $event)
     {
         $this->setRequest($this->kernelContext->getParentRequest());
     }
@@ -142,7 +142,7 @@ public static function getSubscribedEvents()
     {
         return array(
             KernelEvents::REQUEST => array(array('onKernelRequest', 32)),
-            KernelEvents::RESPONSE => array(array('onKernelResponse', -255)),
+            KernelEvents::REQUEST_FINISHED => array(array('onKernelRequestFinished', 0)),
         );
     }
 }
diff --git a/src/Symfony/Component/HttpKernel/HttpKernel.php b/src/Symfony/Component/HttpKernel/HttpKernel.php
@@ -16,6 +16,7 @@
 use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
 use Symfony\Component\HttpKernel\Event\FilterControllerEvent;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\Event\RequestFinishedEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
@@ -35,19 +36,22 @@ class HttpKernel implements HttpKernelInterface, TerminableInterface
 {
     protected $dispatcher;
     protected $resolver;
+    protected $requestStack;
 
     /**
      * Constructor
      *
-     * @param EventDispatcherInterface    $dispatcher An EventDispatcherInterface instance
-     * @param ControllerResolverInterface $resolver   A ControllerResolverInterface instance
+     * @param EventDispatcherInterface    $dispatcher   An EventDispatcherInterface instance
+     * @param ControllerResolverInterface $resolver     A ControllerResolverInterface instance
+     * @param RequestStack                $requestStack A stack for master/sub requests
      *
      * @api
      */
-    public function __construct(EventDispatcherInterface $dispatcher, ControllerResolverInterface $resolver)
+    public function __construct(EventDispatcherInterface $dispatcher, ControllerResolverInterface $resolver, RequestStack $requestStack = null)
     {
         $this->dispatcher = $dispatcher;
         $this->resolver = $resolver;
+        $this->requestStack = $requestStack ?: new RequestStack();
     }
 
     /**
@@ -105,6 +109,8 @@ public function terminate(Request $request, Response $response)
      */
     private function handleRaw(Request $request, $type = self::MASTER_REQUEST)
     {
+        $this->requestStack->push($request);
+
         // request
         $event = new GetResponseEvent($this, $request, $type);
         $this->dispatcher->dispatch(KernelEvents::REQUEST, $event);
@@ -168,6 +174,10 @@ private function filterResponse(Response $response, Request $request, $type)
 
         $this->dispatcher->dispatch(KernelEvents::RESPONSE, $event);
 
+        $this->requestStack->pop();
+
+        $this->dispatcher->dispatch(KernelEvents::REQUEST_FINISHED, new RequestFinishedEvent($this, $request, $type));
+
         return $event->getResponse();
     }
 

diff --git a/src/Symfony/Component/HttpKernel/KernelEvents.php b/src/Symfony/Component/HttpKernel/KernelEvents.php
@@ -102,4 +102,14 @@ final class KernelEvents
      * @var string
      */
     const TERMINATE = 'kernel.terminate';
+
+    /**
+     * The REQUEST_FINISHED event occurs when a response was generated for a request.
+     *
+     * This event allows you to reset the global and envioronmental state of
+     * the application, when it was changed during the request.
+     *
+     * @var string
+     */
+    const REQUEST_FINISHED = 'kernel.request_finished';
 }
diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/LocaleListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/LocaleListenerTest.php
@@ -73,7 +73,7 @@ public function testLocaleSetForRoutingContext()
         $listener->onKernelRequest($this->getEvent($request));
     }
 
-    public function testRouterResetWithParentRequestOnKernelResponse()
+    public function testRouterResetWithParentRequestOnKernelRequestFinished()
     {
         if (!class_exists('Symfony\Component\Routing\Router')) {
             $this->markTestSkipped('The "Routing" component is not available');
@@ -91,10 +91,10 @@ public function testRouterResetWithParentRequestOnKernelResponse()
 
         $this->context->expects($this->once())->method('getParentRequest')->will($this->returnValue($parentRequest));
 
-        $event = $this->getMock('Symfony\Component\HttpKernel\Event\FilterResponseEvent', array(), array(), '', false);
+        $event = $this->getMock('Symfony\Component\HttpKernel\Event\RequestFinishedEvent', array(), array(), '', false);
 
         $listener = new LocaleListener('fr', $this->context, $router);
-        $listener->onKernelResponse($event);
+        $listener->onKernelRequestFinished($event);
     }
 
     public function testRequestLocaleIsNotOverridden()

diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php
@@ -249,6 +249,20 @@ public function testTerminate()
         $this->assertEquals($response, $capturedResponse);
     }
 
+    public function testVerifyRequestStackPushPopDuringHandle()
+    {
+        $request = new Request();
+
+        $stack = $this->getMock('Symfony\Component\HttpKernel\RequestStack', array('push', 'pop'));
+        $stack->expects($this->at(0))->method('push')->with($this->equalTo($request));
+        $stack->expects($this->at(1))->method('pop');
+
+        $dispatcher = new EventDispatcher();
+        $kernel = new HttpKernel($dispatcher, $this->getResolver(), $stack);
+
+        $kernel->handle($request, HttpKernelInterface::MASTER_REQUEST);
+    }
+
     protected function getResolver($controller = null)
     {
            if (null === $controller) {