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

Commit 4e2d6ef

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

File tree

7 files changed

+111
-190
lines changed

7 files changed

+111
-190
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: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public function testGetControllerOnContainerAware()
3232

3333
$controller = $resolver->getController($request);
3434

35+
$this->assertInstanceOf('Symfony\Bundle\FrameworkBundle\Tests\Controller\ContainerAwareController', $controller[0]);
3536
$this->assertInstanceOf('Symfony\Component\DependencyInjection\ContainerInterface', $controller[0]->getContainer());
3637
$this->assertSame('testAction', $controller[1]);
3738
}
@@ -48,6 +49,10 @@ public function testGetControllerOnContainerAwareInvokable()
4849
$this->assertInstanceOf('Symfony\Component\DependencyInjection\ContainerInterface', $controller->getContainer());
4950
}
5051

52+
/**
53+
* @group legacy
54+
* @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.
55+
*/
5156
public function testGetControllerWithBundleNotation()
5257
{
5358
$shortName = 'FooBundle:Default:test';

‎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 \InvalidArgumentException 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