8000 fix tests for controller resolver · symfony/symfony@f1d6484 · GitHub
[go: up one dir, main page]
Skip to content

Commit f1d6484

Browse files
TobionTobion
committed
fix tests for controller resolver
1 parent 841d03c commit f1d6484

File tree

8 files changed

+152
-200
lines changed

8 files changed

+152
-200
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Controller/ControllerResolver.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,12 @@ protected function createController($controller)
3737
{
3838
if (false === strpos($controller, '::') && 2 === substr_count($controller, ':')) {
3939
// controller in the a:b:c notation then
40-
$controller = $this->parser->parse($controller);
40+
$deprecatedNotation = $controller;
41+
$controller = $this->parser->parse($deprecatedNotation);
4142

4243
@trigger_error(sprintf(
43-
'Referencing controllers with the bundle:controller:action notation is deprecated since version 4.1 and will be removed in 5.0. Use %s instead.',
44+
'Referencing controllers with %s is deprecated since version 4.1 and will be removed in 5.0. Use %s instead.',
45+
$deprecatedNotation,
4446
$controller
4547
), E_USER_DEPRECATED);
4648
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerResolverTest.php

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
2121
use Symfony\Component\DependencyInjection\ContainerInterface;
2222
use Symfony\Component\HttpFoundation\Request;
23+
use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
2324
use Symfony\Component\HttpKernel\Tests\Controller\ContainerControllerResolverTest;
2425

2526
class ControllerResolverTest extends ContainerControllerResolverTest
@@ -32,6 +33,7 @@ public function testGetControllerOnContainerAware()
3233

3334
$controller = $resolver->getController($request);
3435

36+
$this->assertInstanceOf('Symfony\Bundle\FrameworkBundle\Tests\Controller\ContainerAwareController', $controller[0]);
3537
$this->assertInstanceOf('Symfony\Component\DependencyInjection\ContainerInterface', $controller[0]->getContainer());
3638
$this->assertSame('testAction', $controller[1]);
3739
}
@@ -48,6 +50,10 @@ public function testGetControllerOnContainerAwareInvokable()
4850
$this->assertInstanceOf('Symfony\Component\DependencyInjection\ContainerInterface', $controller->getContainer());
4951
}
5052

53+
/**
54+
* @group legacy
55+
* @expectedDeprecation Referencing controllers with FooBundle:Default:test is deprecated since version 4.1 and will be removed in 5.0. Use Symfony\Bundle\FrameworkBundle\Tests\Controller\ContainerAwareController::testAction instead.
56+
*/
5157
public function testGetControllerWithBundleNotation()
5258
{
5359
$shortName = 'FooBundle:Default:test';
@@ -81,7 +87,7 @@ class_exists(AbstractControllerTest::class);
8187
$resolver = $this->createControllerResolver(null, $container);
8288

8389
$request = Request::create('/');
84-
$request->attributes->set('_controller', TestAbstractController::class.':testAction');
90+
$request->attributes->set('_controller', TestAbstractController::class.'::testAction');
8591

8692
$this->assertSame(array($controller, 'testAction'), $resolver->getController($request));
8793
$this->assertSame($container, $controller->getContainer());
@@ -117,7 +123,7 @@ class_exists(AbstractControllerTest::class);
117123
$resolver = $this->createControllerResolver(null, $container);
118124

119125
$request = Request::create('/');
120-
$request->attributes->set('_controller', DummyController::class.':fooAction');
126+
$request->attributes->set('_controller', DummyController::class.'::fooAction');
121127

122128
$this->assertSame(array($controller, 'fooAction'), $resolver->getController($request));
123129
$this->assertSame($container, $controller->getContainer());
@@ -157,13 +163,13 @@ class_exists(AbstractControllerTest::class);
157163
$resolver = $this->createControllerResolver(null, $container);
158164

159165
$request = Request::create('/');
160-
$request->attributes->set('_controller', DummyController::class.':fooAction');
166+
$request->attributes->set('_controller', DummyController::class.'::fooAction');
161167

162168
$this->assertSame(array($controller, 'fooAction'), $resolver->getController($request));
163169
$this->assertSame($controllerContainer, $controller->getContainer());
164170
}
165171

166-
protected function createControllerResolver(LoggerInterface $logger = null, Psr11ContainerInterface $container = null, ControllerNameParser $parser = null)
172+
protected function createControllerResolver(LoggerInterface $logger = null, Psr11ContainerInterface $container = null, ControllerNameParser $parser = null): ControllerResolverInterface
167173
{
168174
if (!$parser) {
169175
$parser = $this->createMockParser();

‎src/Symfony/Bundle/FrameworkBundle/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
"symfony/config": "~3.4|~4.0",
2424
"symfony/event-dispatcher": "~3.4|~4.0",
2525
"symfony/http-foundation": "~3.4|~4.0",
26-
"symfony/http-kernel": "~3.4|~4.0",
26+
"symfony/http-kernel": "^4.1",
2727
"symfony/polyfill-mbstring": "~1.0",
2828
"symfony/filesystem": "~3.4|~4.0",
2929
"symfony/finder": "~3.4|~4.0",

‎src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,26 @@ protected function instantiateController($class)
5656

5757
try {
5858
return parent::instantiateController($class);
59-
} catch (\ArgumentCountError | \InvalidArgumentException $e) {
59+
} catch (\Error $e) {
6060
}
6161

6262
$this->throwExceptionIfControllerWasRemoved($class, $e);
6363

64-
throw $e;
64+
if ($e instanceof \ArgumentCountError) {
65+
throw new \InvalidArgumentException(
66+
sprintf('Controller "%s" has required constructor arguments and does not exist in the container. Did you forget to define such a service?', $class), 0, $e
67+
);
68+
}
69+
70+
throw new \InvalidArgumentException(sprintf('Controller "%s" does neither exist as service nor as class', $class), 0, $e);
6571
}
6672

67-
/**
68-
* @param string $controller
69-
* @param \Exception|\Throwable|null $previous
70-
*/
71-
private function throwExceptionIfControllerWasRemoved($controller, $previous = null)
73+
private function throwExceptionIfControllerWasRemoved(string $controller, \Throwable $previous)
7274
{
7375
if ($this->container instanceof Container && isset($this->container->getRemovedIds()[$controller])) {
74-
throw new \LogicException(sprintf('Controller "%s" cannot be fetched from the container because it is private. Did you forget to tag the service with "controller.service_arguments"?', $controller), 0, $previous);
76+
throw new \InvalidArgumentException(
77+
sprintf('Controller "%s" cannot be fetched from the container because it is private. Did you forget to tag the service with "controller.service_arguments"?', $controller), 0, $previous
78+
);
7579
}
7680
}
7781
}

‎src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ public function getController(Request $request)
5555
}
5656

5757
if (is_object($controller)) {
58-
if (method_exists($controller, '__invoke')) {
59-
return $controller;
58+
if (!is_callable($controller)) {
59+
throw new \InvalidArgumentException(sprintf('The controller for URI "%s" is not callable. %s', $request->getPathInfo(), $this->getControllerError($controller)));
6060
}
6161

62-
throw new \InvalidArgumentException(sprintf('Controller "%s" for URI "%s" is not callable.', get_class($controller), $request->getPathInfo()));
62+
return $controller;
6363
}
6464

6565
if (function_exists($controller)) {
@@ -81,8 +81,6 @@ public function getController(Request $request)
8181
* @param string $controller A Controller string
8282
*
8383
* @return callable A PHP callable
84-
*
85-
* @throws \InvalidArgumentException
8684
*/
8785
protected function createController($controller)
8886
{
@@ -104,25 +102,15 @@ protected function createController($controller)
104102
*/
105103
protected function instantiateController($class)
106104
{
107-
if (!class_exists($class)) {
108-
throw new \InvalidArgumentException(sprintf('Class "%s" does not exist.', $class));
109-
}
110-
111105
return new $class();
112106
}
113107

114108
private function getControllerError($callable)
115109
{
116110
if (is_string($callable)) {
117111
if (false !== strpos($callable, '::')) {
118-
$callable = explode('::', $callable);
119-
}
120-
121-
if (class_exists($callable) && !method_exists($callable, '__invoke')) {
122-
return sprintf('Class "%s" does not have a method "__invoke".', $callable);
123-
}
124-
125-
if (!function_exists($callable)) {
112+
$callable = explode('::', $callable, 2);
113+
} else {
126114
return sprintf('Function "%s" does not exist.', $callable);
127115
}
128116
}
@@ -134,11 +122,11 @@ private function getControllerError($callable)
134122
}
135123

136124
if (!is_array($callable)) {
137-
return sprintf('Invalid type for controller given, expected string or array, got "%s".', gettype($callable));
125+
return sprintf('Invalid type for controller given, expected string, array or object, got "%s".', gettype($callable));
138126
}
139127

140-
if (2 !== count($callable)) {
141-
return 'Invalid format for controller, expected array(controller, method) or controller::method.';
128+
if (!isset($callable[0]) | !isset($callable[1])) {
129+
return 'Array callable has to contain indices 0 and 1 like array(controller, method).';
142130
}
143131

144132
list($controller, $method) = $callable;

‎src/Symfony/Component/HttpKernel/Controller/ControllerResolverInterface.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ interface ControllerResolverInterface
3535
* @return callable|false A PHP callable representing the Controller,
3636
* or false if this resolver is not able to determine the controller
3737
*
38-
* @throws \LogicException If the controller can't be found
38+
* @throws \LogicException If a controller was found based on the request but it is not callable
3939
*/
4040
public function getController(Request $request);
4141
}

0 commit comments

Comments
 (0)
0