Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-7007: Synchronized Service alternative, backwards compatible. #7707

Closed
wants to merge 7 commits into from
Expand Up @@ -14,7 +14,9 @@
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\HttpKernel\Fragment\FragmentHandler;
use Symfony\Component\HttpKernel\RequestContext;

class HttpKernelExtensionTest extends TestCase
{
Expand All @@ -36,13 +38,30 @@ protected function setUp()
*/
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\\HttpKernel\\RequestContext')
->disableOriginalConstructor()
->getMock()
;
$renderer = new FragmentHandler($context, array());

$this->setExpectedException('InvalidArgumentException', 'The "inline" renderer does not exist.');
$renderer->render('/foo');
}

protected function getFragmentHandler($return)
Expand All @@ -51,8 +70,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\\HttpKernel\\RequestContext')
->disableOriginalConstructor()
->getMock()
;

$context->expects($this->any())->method('getCurrentRequest')->will($this->returnValue(Request::create('/')));

$renderer = new FragmentHandler($context, array($strategy));

return $renderer;
}
Expand Down
Expand Up @@ -14,9 +14,9 @@

<services>
<service id="fragment.handler" class="%fragment.handler.class%">
<argument type="service" id="request_context" />
<argument type="collection" />
<argument>%kernel.debug%</argument>
<call method="setRequest"><argument type="service" id="request" on-invalid="null" strict="false" /></call>
</service>

<service id="fragment.renderer.inline" class="%fragment.renderer.inline.class%">
Expand Down
Expand Up @@ -92,9 +92,9 @@
<tag name="kernel.event_subscriber" />
<tag name="monolog.logger" channel="request" />
<argument type="service" id="router" />
<argument type="service" id="request_context" />
<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>
</service>
</services>
</container>
10 changes: 10 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml
Expand Up @@ -12,6 +12,8 @@
<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\HttpKernel\RequestStack</parameter>
<parameter key="request_context.class">Symfony\Component\HttpKernel\RequestContext</parameter>
</parameters>

<services>
Expand All @@ -23,6 +25,14 @@
<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%">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about marking it private?

</service>

<service id="request_context" class="%request_context.class%">
<argument type="service" id="request_stack" />
</service>

<service id="cache_warmer" class="%cache_warmer.class%">
Expand Down
Expand Up @@ -37,8 +37,8 @@
<service id="locale_listener" class="%locale_listener.class%">
<tag name="kernel.event_subscriber" />
<argument>%kernel.default_locale%</argument>
<argument type="service" id="request_context" />
<argument type="service" id="router" on-invalid="ignore" />
<call method="setRequest"><argument type="service" id="request" on-invalid="null" strict="false" /></call>
</service>
</services>
</container>
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\HttpKernel;
use Symfony\Component\HttpKernel\RequestStack;
use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
Expand All @@ -29,19 +30,22 @@
class ContainerAwareHttpKernel extends HttpKernel
{
protected $container;
protected $requestStack;

/**
* Constructor.
*
* @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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should accept null by default for BC.

{
parent::__construct($dispatcher, $controllerResolver);
parent::__construct($dispatcher, $controllerResolver, $requestStack);

$this->container = $container;

$container->addScope(new Scope('request'));
}

Expand Down
21 changes: 21 additions & 0 deletions src/Symfony/Component/HttpKernel/Event/RequestFinishedEvent.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 RequestFinishedEvent extends KernelEvent
{
}
47 changes: 36 additions & 11 deletions src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php
Expand Up @@ -12,7 +12,9 @@
namespace Symfony\Component\HttpKernel\EventListener;

use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\Event\RequestFinishedEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\RequestContext;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\RequestContextAwareInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
Expand All @@ -26,41 +28,64 @@ class LocaleListener implements EventSubscriberInterface
{
private $router;
private $defaultLocale;
private $requestContext;

public function __construct($defaultLocale = 'en', RequestContextAwareInterface $router = null)
public function __construct($defaultLocale = 'en', RequestContext $requestContext, RequestContextAwareInterface $router = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a BC break. Not sure how to deal with it. The only thing that comes to my mind is to not used the new request context and keep the old way.

{
$this->defaultLocale = $defaultLocale;
$this->requestContext = $requestContext;
$this->router = $router;
}

public function setRequest(Request $request = null)
public function onKernelRequest(GetResponseEvent $event)
{
$request = $event->getRequest();
$request->setDefaultLocale($this->defaultLocale);

$this->setLocale($request);
$this->setRouterContext($request);
}

public function onKernelRequestFinished(RequestFinishedEvent $event)
{
if (null === $request) {
$this->resetRouterContext();
}

private function resetRouterContext()
{
if ($this->requestContext === null) {
return;
}

$parentRequest = $this->requestContext->getParentRequest();

if ($parentRequest === null) {
return;
}

$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 function onKernelRequest(GetResponseEvent $event)
{
$request = $event->getRequest();
$request->setDefaultLocale($this->defaultLocale);

$this->setRequest($request);
}

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::REQUEST_FINISHED => array(array('onKernelRequestFinished', 0)),
);
}
}
16 changes: 13 additions & 3 deletions src/Symfony/Component/HttpKernel/EventListener/RouterListener.php
Expand Up @@ -13,9 +13,11 @@

use Psr\Log\LoggerInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\Event\RequestFinishedEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpKernel\RequestContext as KernelRequestContext;
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
use Symfony\Component\Routing\Matcher\UrlMatcherInterface;
Expand All @@ -35,6 +37,7 @@ class RouterListener implements EventSubscriberInterface
private $matcher;
private $context;
private $logger;
private $kernelContext;

/**
* Constructor.
Expand All @@ -45,7 +48,7 @@ class RouterListener implements EventSubscriberInterface
*
* @throws \InvalidArgumentException
*/
public function __construct($matcher, RequestContext $context = null, LoggerInterface $logger = null)
public function __construct($matcher, KernelRequestContext $kernelContext, RequestContext $context = null, LoggerInterface $logger = null)
{
if (!$matcher instanceof UrlMatcherInterface && !$matcher instanceof RequestMatcherInterface) {
throw new \InvalidArgumentException('Matcher must either implement UrlMatcherInterface or RequestMatcherInterface.');
Expand All @@ -57,6 +60,7 @@ public function __construct($matcher, RequestContext $context = null, LoggerInte

$this->matcher = $matcher;
$this->context = $context ?: $matcher->getContext();
$this->kernelContext = $kernelContext;
$this->logger = $logger;
}

Expand All @@ -70,21 +74,26 @@ public function __construct($matcher, RequestContext $context = null, LoggerInte
*
* @param Request|null $request A Request instance
*/
public function setRequest(Request $request = null)
private function populateRoutingContext(Request $request = null)
{
if (null !== $request) {
$this->context->fromRequest($request);
}
}

public function onKernelRequestFinished(RequestFinishedEvent $event)
{
$this->populateRoutingContext($this->kernelContext->getParentRequest());
}

public function onKernelRequest(GetResponseEvent $event)
{
$request = $event->getRequest();

// 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->setRequest($request);
$this->populateRoutingContext($request);

if ($request->attributes->has('_controller')) {
// routing is already done
Expand Down Expand Up @@ -133,6 +142,7 @@ public static function getSubscribedEvents()
{
return array(
KernelEvents::REQUEST => array(array('onKernelRequest', 32)),
KernelEvents::REQUEST_FINISHED => array(array('onKernelRequestFinished', 0)),
);
}
}