[HttpKernel] tweaked the code

symfony · fabpot · Sep 8, 2013 · Apr 18, 2013 · Aug 31, 2013 · Aug 31, 2013
commit 018b71936f077f59da748021be9b48f876d04e53
diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php b/src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php
@@ -37,9 +37,9 @@ 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
+     * @param RequestStack                $requestStack       A stack for master/sub requests
      */
-    public function __construct(EventDispatcherInterface $dispatcher, ContainerInterface $container, ControllerResolverInterface $controllerResolver, RequestStack $requestStack)
+    public function __construct(EventDispatcherInterface $dispatcher, ContainerInterface $container, ControllerResolverInterface $controllerResolver, RequestStack $requestStack = null)
     {
         parent::__construct($dispatcher, $controllerResolver, $requestStack);
 

diff --git a/src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php b/src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php
@@ -37,6 +37,27 @@ public function __construct($defaultLocale = 'en', RequestContext $requestContex
         $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;
+        }
+
+        $this->setLocale($request);
+        $this->setRouterContext($request);
+    }
+
     public function onKernelRequest(GetResponseEvent $event)
     {
         $request = $event->getRequest();
@@ -48,22 +69,9 @@ public function onKernelRequest(GetResponseEvent $event)
 
     public function onKernelFinishRequest(FinishRequestEvent $event)
     {
-        $this->resetRouterContext();
-    }
-
-    private function resetRouterContext()
-    {
-        if ($this->requestContext === null) {
-            return;
-        }
-
-        $parentRequest = $this->requestContext->getParentRequest();
-
-        if ($parentRequest === null) {
-            return;
+        if (null !== $parentRequest = $this->requestContext->getParentRequest()) {
+            $this->setRouterContext($parentRequest);
         }
-
-        $this->setRouterContext($parentRequest);
     }
 
     private function setLocale(Request $request)

diff --git a/src/Symfony/Component/HttpKernel/EventListener/RouterListener.php b/src/Symfony/Component/HttpKernel/EventListener/RouterListener.php
@@ -68,25 +68,26 @@ public function __construct($matcher, KernelRequestContext $kernelContext, Reque
     /**
      * Sets the current Request.
      *
-     * The application should call this method whenever the Request
-     * object changes (entering a Request scope for instance, but
-     * also when leaving a Request scope -- especially when they are
-     * nested).
+     * 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 moved to a private function in 3.0.
      */
-    private function populateRoutingContext(Request $request = null)
+    public function setRequest(Request $request = null)
     {
         if (null !== $request && $this->request !== $request) {
             $this->context->fromRequest($request);
         }
         $this->request = $request;
     }
 
-     public function onKernelFinishRequest(FinishRequestEvent $event)
-     {
-         $this->populateRoutingContext($this->kernelContext->getParentRequest());
-     }
+    public function onKernelFinishRequest(FinishRequestEvent $event)
+    {
+        $this->setRequest($this->kernelContext->getParentRequest());
+    }
 
     public function onKernelRequest(GetResponseEvent $event)
     {
@@ -95,7 +96,7 @@ public function onKernelRequest(GetResponseEvent $event)
         // initialize the context that is also used by the generator (assuming matcher and generator share the same context instance)
         // we call setRequest even if most of the time, it has already been done to keep compatibility
         // with frameworks which do not use the Symfony service container
-        $this->populateRoutingContext($request);
+        $this->setRequest($request);
 
         if ($request->attributes->has('_controller')) {
             // routing is already done

diff --git a/src/Symfony/Component/HttpKernel/Fragment/FragmentHandler.php b/src/Symfony/Component/HttpKernel/Fragment/FragmentHandler.php
@@ -85,6 +85,10 @@ public function render($uri, $renderer = 'inline', array $options = array())
             throw new \InvalidArgumentException(sprintf('The "%s" renderer does not exist.', $renderer));
         }
 
+        if (null === $this->context->getCurrentRequest()) {
+            throw new \LogicException('Rendering a fragment can only be done when handling a Request.');
+        }
+
         return $this->deliver($this->renderers[$renderer]->render($uri, $this->context->getCurrentRequest(), $options));
     }
 
@@ -103,8 +107,7 @@ public function render($uri, $renderer = 'inline', array $options = array())
     protected function deliver(Response $response)
     {
         if (!$response->isSuccessful()) {
-            $request = $this->context->getCurrentRequest();
-            throw new \RuntimeException(sprintf('Error when rendering "%s" (Status code is %s).', $request->getUri(), $response->getStatusCode()));
+            throw new \RuntimeException(sprintf('Error when rendering "%s" (Status code is %s).', $this->context->getCurrentRequest()->getUri(), $response->getStatusCode()));
         }
 
         if (!$response instanceof StreamedResponse) {

diff --git a/src/Symfony/Component/HttpKernel/HttpKernel.php b/src/Symfony/Component/HttpKernel/HttpKernel.php
@@ -64,9 +64,9 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
         try {
             return $this->handleRaw($request, $type);
         } catch (\Exception $e) {
-            $this->finishRequest($request, $type);
-
             if (false === $catch) {
+                $this->finishRequest($request, $type);
+
                 throw $e;
             }
 
@@ -170,16 +170,14 @@ private function filterResponse(Response $response, Request $request, $type)
     }
 
     /**
-     * Publish event finished event, then pop the request from the stack.
+     * Publishes the finish request event, then pop the request from the stack.
      *
-     * Note: Order of the operations is important here, otherwise operations
-     * such as {@link RequestStack::getParentRequest()} can lead to weird
-     * results.
+     * Note that the order of the operations is important here, otherwise
+     * operations such as {@link RequestStack::getParentRequest()} can lead to
+     * weird results.
      *
      * @param Request $request
-     * @param int $type
-     *
-     * @return void
+     * @param int     $type
      */
     private function finishRequest(Request $request, $type)
     {
@@ -207,6 +205,8 @@ private function handleException(\Exception $e, $request, $type)
         $e = $event->getException();
 
         if (!$event->hasResponse()) {
+            $this->finishRequest($request, $type);
+
             throw $e;
         }
 

diff --git a/src/Symfony/Component/HttpKernel/RequestContext.php b/src/Symfony/Component/HttpKernel/RequestContext.php
@@ -30,15 +30,15 @@ public function __construct(RequestStack $stack)
     }
 
     /**
-     * @return Request
+     * @return Request|null
      */
     public function getCurrentRequest()
     {
         return $this->stack->getCurrentRequest();
     }
 
     /**
-     * @return Request
+     * @return Request|null
      */
     public function getMasterRequest()
     {

diff --git a/src/Symfony/Component/HttpKernel/RequestStack.php b/src/Symfony/Component/HttpKernel/RequestStack.php
@@ -16,8 +16,6 @@
 /**
  * Request stack that controls the lifecycle of requests.
  *
- * Notifies services of changes in the stack.
- *
  * @author Benjamin Eberlei <kontakt@beberlei.de>
  */
 class RequestStack
@@ -33,14 +31,18 @@ public function push(Request $request)
     }
 
     /**
-     * Pop the current request from the stack.
+     * Pops the current request from the stack.
      *
      * This operation lets the current request go out of scope.
      *
      * @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);
     }
 
@@ -65,11 +67,11 @@ public function getMasterRequest()
     }
 
     /**
-     * Return the parent request of the current.
+     * Returns the parent request of the current.
      *
-     * If current Request is the master request, method returns null.
+     * If current Request is the master request, it returns null.
      *
-     * @return Request
+     * @return Request|null
      */
     public function getParentRequest()
     {