8000 bug #23013 Parse the _controller format in sub-requests (weaverryan) · symfony/symfony@f051948 · GitHub
[go: up one dir, main page]
Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f051948

Browse files
fabpotfabpot
committed
bug #23013 Parse the _controller format in sub-requests (weaverryan)
This PR was merged into the 3.3 branch. Discussion ---------- Parse the _controller format in sub-requests | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | possibly (edge case) | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22966 | License | MIT | Doc PR | n/a As mentioned on the issue (#22966 (comment)), the new "controller service args" functionality relies on the `_controller` attribute to be in either the service format `App\Controller\Foo:bar` or at least the final parsed format `App\Controller\Foo::bar`. But when you make a sub-request with the `App:Foo:bar` format, the `ControllerResolver` correctly parses this, but the `_controller` request attribute will always contain the original `App:Foo:bar` format. That causes the `ServiceValueResolver` to fail. The only way I can think to fix this - reliably - is to parse the `_controller` attribute in a listener. And this, works great! Notes: A) There is a small chance for a BC break: if you were relying on the `_controller` old format in a `kernel.request` format in the framework, in a listener between the priority of 25 and 31 for sub-requests (because normal requests have `_controller` normalized during routing)... then you will see a behavior change. B) We could load the `ControllerNameParser` lazily via a service locator. C) We could deprecate calling the parser in the FW's `ControllerResolver`. Along with (B), I think it would (in 4.0) mean that the `ControllerNameParser` is not instantiated at runtime (except for sub-requests). If someone can think of a different/better solution, please let me know! Cheers! Commits ------- 9578fd3 Adding a new event subscriber that "parses" the _controller attribute in the FW
2 parents c88a006 + 9578fd3 commit f051948

File tree

8 files changed

+185
-0
lines changed

8 files changed

+185
-0
lines changed

‎src/Symfony/Bundle/FrameworkBundle/EventListener/ResolveControllerNameSubscriber.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\EventListener;
13+
14+
use Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser;
15+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
16+
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
17+
use Symfony\Component\HttpKernel\KernelEvents;
18+
19+
/**
20+
* Guarantees that the _controller key is parsed into its final format.
21+
*
22+
* @author Ryan Weaver <ryan@knpuniversity.com>
23+
*/
24+
class ResolveControllerNameSubscriber implements EventSubscriberInterface
25+
{
26+
private $parser;
27+
28+
public function __construct(ControllerNameParser $parser)
29+
{
30+
$this->parser = $parser;
31+
}
32+
33+
public function onKernelRequest(GetResponseEvent $event)
34+
{
35+
$controller = $event->getRequest()->attributes->get('_controller');
36+
if ($controller && false === strpos($controller, '::') && 2 === substr_count($controller, ':')) {
37+
// controller in the a:b:c notation then
38+
$event->getRequest()->attributes->set('_controll EDBE er', $this->parser->parse($controller));
39+
}
40+
}
41+
42+
public static function getSubscribedEvents()
43+
{
44+
return array(
45+
KernelEvents::REQUEST => array('onKernelRequest', 24),
46+
);
47+
}
48+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,5 +70,10 @@
7070
<service id="validate_request_listener" class="Symfony\Component\HttpKernel\EventListener\ValidateRequestListener" public="true">
7171
<tag name="kernel.event_subscriber" />
7272
</service>
73+
74+
<service id="resolve_controller_name_subscriber" class="Symfony\Bundle\FrameworkBundle\EventListener\ResolveControllerNameSubscriber">
75+
<argument type="service" id="controller_name_converter" />
76+
<tag name="kernel.event_subscriber" />
77+
</service>
7378
</services>
7479
</container>

‎src/Symfony/Bundle/FrameworkBundle/Tests/EventListener/ResolveControllerNameSubscriberTest.php

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\EventListener;
13+
14+
use Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser;
15+
use Symfony\Bundle\FrameworkBundle\EventListener\ResolveControllerNameSubscriber;
16+
use Symfony\Bundle\FrameworkBundle\Tests\TestCase;
17+
use Symfony\Component\HttpFoundation\Request;
18+
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
19+
use Symfony\Component\HttpKernel\HttpKernelInterface;
20+
21+
class ResolveControllerNameSubscriberTest extends TestCase
22+
{
23+
public function testReplacesControllerAttribute()
24+
{
25+
$parser = $this->getMockBuilder(ControllerNameParser::class)->disableOriginalConstructor()->getMock();
26+
$parser->expects($this->any())
27+
->method('parse')
28+
->with('AppBundle:Starting:format')
29+
->willReturn('App\\Final\\Format::methodName');
30+
$httpKernel = $this->getMockBuilder(HttpKernelInterface::class)->getMock();
31+
32+
$request = new Request();
33+
$request->attributes->set('_controller', 'AppBundle:Starting:format');
34+
35+
$subscriber = new ResolveControllerNameSubscriber($parser);
36+
$subscriber->onKernelRequest(new GetResponseEvent($httpKernel, $request, HttpKernelInterface::MASTER_REQUEST));
37+
$this->assertEquals('App\\Final\\Format::methodName', $request->attributes->get('_controller'));
38+
}
39+
40+
public function testSkipsOtherControllerFormats()
41+
{
42+
$parser = $this->getMockBuilder(ControllerNameParser::class)->disableOriginalConstructor()->getMock();
43+
$parser->expects($this->never())
44+
->method('parse');
45+
$httpKernel = $this->getMockBuilder(HttpKernelInterface::class)->getMock();
46+
47+
$request = new Request();
48+
$request->attributes->set('_controller', 'Other:format');
49+
50+
$subscriber = new ResolveControllerNameSubscriber($parser);
51+
$subscriber->onKernelRequest(new GetResponseEvent($httpKernel, $request, HttpKernelInterface::MASTER_REQUEST));
52+
$this->assertEquals('Other:format', $request->attributes->get('_controller'));
53+
}
54+
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestServiceResolutionController.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller;
13+
14+
use Psr\Log\LoggerInterface;
15+
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
16+
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
17+
use Symfony\Component\HttpFoundation\Response;
18+
use Symfony\Component\HttpKernel\HttpKernelInterface;
19+
20+
class SubRequestServiceResolutionController implements ContainerAwareInterface
21+
{
22+
use ContainerAwareTrait;
23+
24+
public function indexAction()
25+
{
26+
$request = $this->container->get('request_stack')->getCurrentRequest();
27+
$path['_forwarded'] = $request->attributes;
28+
$path['_controller'] = 'TestBundle:SubRequestServiceResolution:fragment';
29+
$subRequest = $request->duplicate(array(), null, $path);
30+
31+
return $this->container->get('http_kernel')->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
32+
}
33+
34+
public function fragmentAction(LoggerInterface $logger)
35+
{
36+
return new Response('---');
37+
}
38+
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SubRequestsTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,12 @@ public function testStateAfterSubRequest()
2020

2121
$this->assertEquals('--fr/json--en/html--fr/json--http://localhost/subrequest/fragment/en', $client->getResponse()->getContent());
2222
}
23+
24+
public function testSubRequestControllerServicesAreResolved()
25+
{
26+
$client = $this->createClient(array('test_case' => 'ControllerServiceResolution', 'root_config' => 'config.yml'));
27+
$client->request('GET', 'https://localhost/subrequest');
28+
29+
$this->assertEquals('---', $client->getResponse()->getContent());
30+
}
2331
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ControllerServiceResolution/bundles.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
use Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\TestBundle;
13+
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
14+
15+
return array(
16+
new FrameworkBundle(),
17+
new TestBundle(),
18+
);

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ControllerServiceResolution/config.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
imports:
2+
- { resource: ../config/default.yml }
3+
4+
services:
5+
Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SubRequestServiceResolutionController:
6+
public: true
7+
tags: [controller.service_arguments]
8+
9+
logger: { class: Psr\Log\NullLogger }
10+
Psr\Log\LoggerInterface: '@logger'

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ControllerServiceResolution/routing.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
sub_request_page:
2+
path: /subrequest
3+
defaults:
4+
_controller: 'TestBundle:SubRequestServiceResolution:index'

0 commit comments

Comments
 (0)
0