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

Skip to content

Commit f1d6484

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

File tree

8 files changed

+152
-200
lines changed
  • src/Symfony
    • Bundle/FrameworkBundle
  • Component/HttpKernel
  • 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 createContr F987 oller($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