diff --git a/UPGRADE-4.1.md b/UPGRADE-4.1.md index d2c7b8a4f292..520bf9b7eb88 100644 --- a/UPGRADE-4.1.md +++ b/UPGRADE-4.1.md @@ -15,6 +15,38 @@ EventDispatcher FrameworkBundle --------------- + * Deprecated `bundle:controller:action` and `service:action` syntaxes to reference controllers. Use `serviceOrFqcn::method` + instead where `serviceOrFqcn` is either the service ID when using controllers as services or the FQCN of the controller. + + Before: + + ```yml + bundle_controller: + path: / + defaults: + _controller: FrameworkBundle:Redirect:redirect + + service_controller: + path: / + defaults: + _controller: app.my_controller:myAction + ``` + + After: + + ```yml + bundle_controller: + path: / + defaults: + _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction + + service_controller: + path: / + defaults: + _controller: app.my_controller::myAction + ``` + + * Deprecated `Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser` * A `RouterInterface` that does not implement the `WarmableInterface` is deprecated. * The `RequestDataCollector` class has been deprecated. Use the `Symfony\Component\HttpKernel\DataCollector\RequestDataCollector` class instead. diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index ddf6a965179d..472d53a4b3a5 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -14,6 +14,38 @@ EventDispatcher FrameworkBundle --------------- + * Removed support for `bundle:controller:action` and `service:action` syntaxes to reference controllers. Use `serviceOrFqcn::method` + instead where `serviceOrFqcn` is either the service ID when using controllers as services or the FQCN of the controller. + + Before: + + ```yml + bundle_controller: + path: / + defaults: + _controller: FrameworkBundle:Redirect:redirect + + service_controller: + path: / + defaults: + _controller: app.my_controller:myAction + ``` + + After: + + ```yml + bundle_controller: + path: / + defaults: + _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction + + service_controller: + path: / + defaults: + _controller: app.my_controller::myAction + ``` + + * Removed `Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser` * Using a `RouterInterface` that does not implement the `WarmableInterface` is not supported anymore. * The `RequestDataCollector` class has been removed. Use the `Symfony\Component\HttpKernel\DataCollector\RequestDataCollector` class instead. diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 9f7cfae43ad0..d9ac54c56246 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -11,6 +11,9 @@ CHANGELOG * Using a `RouterInterface` that does not implement the `WarmableInterface` is deprecated. * The `RequestDataCollector` class has been deprecated. Use the `Symfony\Component\HttpKernel\DataCollector\RequestDataCollector` class instead. * The `RedirectController` class allows for 307/308 HTTP status codes + * Deprecated `bundle:controller:action` syntax to reference controllers. Use `serviceOrFqcn::method` instead where `serviceOrFqcn` + is either the service ID or the FQCN of the controller. + * Deprecated `Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser` 4.0.0 ----- diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/RouterDebugCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/RouterDebugCommand.php index 425f607f84a0..5455c06541d3 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Command/RouterDebugCommand.php +++ b/src/Symfony/Bundle/FrameworkBundle/Command/RouterDebugCommand.php @@ -12,16 +12,13 @@ namespace Symfony\Bundle\FrameworkBundle\Command; use Symfony\Bundle\FrameworkBundle\Console\Helper\DescriptorHelper; -use Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; -use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; use Symfony\Component\Routing\RouterInterface; -use Symfony\Component\Routing\Route; /** * A console command for retrieving information about routes. @@ -83,20 +80,13 @@ protected function execute(InputInterface $input, OutputInterface $output) throw new \InvalidArgumentException(sprintf('The route "%s" does not exist.', $name)); } - $callable = $this->extractCallable($route); - $helper->describe($io, $route, array( 'format' => $input->getOption('format'), 'raw_text' => $input->getOption('raw'), 'name' => $name, 'output' => $io, - 'callable' => $callable, )); } else { - foreach ($routes as $route) { - $this->convertController($route); - } - $helper->describe($io, $routes, array( 'format' => $input->getOption('format'), 'raw_text' => $input->getOption('raw'), @@ -105,41 +95,4 @@ protected function execute(InputInterface $input, OutputInterface $output) )); } } - - private function convertController(Route $route) - { - if ($route->hasDefault('_controller')) { - $nameParser = new ControllerNameParser($this->getApplication()->getKernel()); - try { - $route->setDefault('_controller', $nameParser->build($route->getDefault('_controller'))); - } catch (\InvalidArgumentException $e) { - } - } - } - - private function extractCallable(Route $route) - { - if (!$route->hasDefault('_controller')) { - return; - } - - $controller = $route->getDefault('_controller'); - - if (1 === substr_count($controller, ':')) { - list($service, $method) = explode(':', $controller); - try { - return sprintf('%s::%s', get_class($this->getApplication()->getKernel()->getContainer()->get($service)), $method); - } catch (ServiceNotFoundException $e) { - } - } - - $nameParser = new ControllerNameParser($this->getApplication()->getKernel()); - try { - $shortNotation = $nameParser->build($controller); - $route->setDefault('_controller', $shortNotation); - - return $controller; - } catch (\InvalidArgumentException $e) { - } - } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php index e4d4d1ea209a..cc9aa505731a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php @@ -95,9 +95,6 @@ protected function describeRoute(Route $route, array $options = array()) array('Defaults', $this->formatRouterConfig($route->getDefaults())), array('Options', $this->formatRouterConfig($route->getOptions())), ); - if (isset($options['callable'])) { - $tableRows[] = array('Callable', $options['callable']); - } $table = new Table($this->getOutput()); $table->setHeaders($tableHeaders)->setRows($tableRows); diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerNameParser.php b/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerNameParser.php index 21b13f91e8cc..c65b860a1b18 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerNameParser.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerNameParser.php @@ -19,6 +19,8 @@ * (Bundle\BlogBundle\Controller\PostController::indexAction). * * @author Fabien Potencier + * + * @deprecated since Symfony 4.1 */ class ControllerNameParser { @@ -41,6 +43,10 @@ public function __construct(KernelInterface $kernel) */ public function parse($controller) { + if (2 > func_num_args() || func_get_arg(1)) { + @trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.1.', __CLASS__), E_USER_DEPRECATED); + } + $parts = explode(':', $controller); if (3 !== count($parts) || in_array('', $parts, true)) { throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller)); @@ -86,6 +92,8 @@ public function parse($controller) */ public function build($controller) { + @trigger_error(sprintf('The %s class is deprecated since Symfony 4.1.', __CLASS__), E_USER_DEPRECATED); + if (0 === preg_match('#^(.*?\\\\Controller\\\\(.+)Controller)::(.+)Action$#', $controller, $match)) { throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "class::method" string.', $controller)); } diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerResolver.php b/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerResolver.php index f0ac2a248fae..27e714acc86e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerResolver.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerResolver.php @@ -37,16 +37,13 @@ protected function createController($controller) { if (false === strpos($controller, '::') && 2 === substr_count($controller, ':')) { // controller in the a:b:c notation then - $controller = $this->parser->parse($controller); - } - - $resolvedController = parent::createController($controller); + $deprecatedNotation = $controller; + $controller = $this->parser->parse($deprecatedNotation, false); - if (1 === substr_count($controller, ':') && is_array($resolvedController)) { - $resolvedController[0] = $this->configureController($resolvedController[0]); + @trigger_error(sprintf('Referencing controllers with %s is deprecated since Symfony 4.1. Use %s instead.', $deprecatedNotation, $controller), E_USER_DEPRECATED); } - return $resolvedController; + return parent::createController($controller); } /** diff --git a/src/Symfony/Bundle/FrameworkBundle/EventListener/ResolveControllerNameSubscriber.php b/src/Symfony/Bundle/FrameworkBundle/EventListener/ResolveControllerNameSubscriber.php index 6072061dbac0..1e6a37fd99d6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/EventListener/ResolveControllerNameSubscriber.php +++ b/src/Symfony/Bundle/FrameworkBundle/EventListener/ResolveControllerNameSubscriber.php @@ -20,6 +20,8 @@ * Guarantees that the _controller key is parsed into its final format. * * @author Ryan Weaver + * + * @deprecated since Symfony 4.1 */ class ResolveControllerNameSubscriber implements EventSubscriberInterface { @@ -35,7 +37,7 @@ public function onKernelRequest(GetResponseEvent $event) $controller = $event->getRequest()->attributes->get('_controller'); if (is_string($controller) && false === strpos($controller, '::') && 2 === substr_count($controller, ':')) { // controller in the a:b:c notation then - $event->getRequest()->attributes->set('_controller', $this->parser->parse($controller)); + $event->getRequest()->attributes->set('_controller', $this->parser->parse($controller, false)); } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Kernel/MicroKernelTrait.php b/src/Symfony/Bundle/FrameworkBundle/Kernel/MicroKernelTrait.php index a596beafdc4c..ab6163e3ef4d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Kernel/MicroKernelTrait.php +++ b/src/Symfony/Bundle/FrameworkBundle/Kernel/MicroKernelTrait.php @@ -64,7 +64,7 @@ public function registerContainerConfiguration(LoaderInterface $loader) $loader->load(function (ContainerBuilder $container) use ($loader) { $container->loadFromExtension('framework', array( 'router' => array( - 'resource' => 'kernel:loadRoutes', + 'resource' => 'kernel::loadRoutes', 'type' => 'service', ), )); diff --git a/src/Symfony/Bundle/FrameworkBundle/Routing/DelegatingLoader.php b/src/Symfony/Bundle/FrameworkBundle/Routing/DelegatingLoader.php index 61944c1a5ad5..6df729b02afe 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Routing/DelegatingLoader.php +++ b/src/Symfony/Bundle/FrameworkBundle/Routing/DelegatingLoader.php @@ -73,14 +73,29 @@ public function load($resource, $type = null) } foreach ($collection->all() as $route) { - if (!is_string($controller = $route->getDefault('_controller')) || !$controller) { + if (!is_string($controller = $route->getDefault('_controller'))) { continue; } - try { - $controller = $this->parser->parse($controller); - } catch (\InvalidArgumentException $e) { - // unable to optimize unknown notation + if (false !== strpos($controller, '::')) { + continue; + } + + if (2 === substr_count($controller, ':')) { + $deprecatedNotation = $controller; + + try { + $controller = $this->parser->parse($controller, false); + + @trigger_error(sprintf('Referencing controllers with %s is deprecated since Symfony 4.1. Use %s instead.', $deprecatedNotation, $controller), E_USER_DEPRECATED); + } catch (\InvalidArgumentException $e) { + // unable to optimize unknown notation + } + } + + if (1 === substr_count($controller, ':')) { + $controller = str_replace(':', '::', $controller); + @trigger_error(sprintf('Referencing controllers with a single colon is deprecated since Symfony 4.1. Use %s instead.', $controller), E_USER_DEPRECATED); } $route->setDefault('_controller', $controller); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerNameParserTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerNameParserTest.php index 0dfed269ec20..b03cf70679a1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerNameParserTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerNameParserTest.php @@ -16,6 +16,9 @@ use Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser; use Symfony\Component\HttpKernel\Kernel; +/** + * @group legacy + */ class ControllerNameParserTest extends TestCase { protected $loader; diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerResolverTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerResolverTest.php index 5880ee0186a1..03dca4acdb40 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerResolverTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerResolverTest.php @@ -20,6 +20,7 @@ use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; use Symfony\Component\HttpKernel\Tests\Controller\ContainerControllerResolverTest; class ControllerResolverTest extends ContainerControllerResolverTest @@ -32,6 +33,7 @@ public function testGetControllerOnContainerAware() $controller = $resolver->getController($request); + $this->assertInstanceOf('Symfony\Bundle\FrameworkBundle\Tests\Controller\ContainerAwareController', $controller[0]); $this->assertInstanceOf('Symfony\Component\DependencyInjection\ContainerInterface', $controller[0]->getContainer()); $this->assertSame('testAction', $controller[1]); } @@ -48,6 +50,10 @@ public function testGetControllerOnContainerAwareInvokable() $this->assertInstanceOf('Symfony\Component\DependencyInjection\ContainerInterface', $controller->getContainer()); } + /** + * @group legacy + * @expectedDeprecation Referencing controllers with FooBundle:Default:test is deprecated since Symfony 4.1. Use Symfony\Bundle\FrameworkBundle\Tests\Controller\ContainerAwareController::testAction instead. + */ public function testGetControllerWithBundleNotation() { $shortName = 'FooBundle:Default:test'; @@ -81,7 +87,7 @@ class_exists(AbstractControllerTest::class); $resolver = $this->createControllerResolver(null, $container); $request = Request::create('/'); - $request->attributes->set('_controller', TestAbstractController::class.':testAction'); + $request->attributes->set('_controller', TestAbstractController::class.'::testAction'); $this->assertSame(array($controller, 'testAction'), $resolver->getController($request)); $this->assertSame($container, $controller->getContainer()); @@ -117,7 +123,7 @@ class_exists(AbstractControllerTest::class); $resolver = $this->createControllerResolver(null, $container); $request = Request::create('/'); - $request->attributes->set('_controller', DummyController::class.':fooAction'); + $request->attributes->set('_controller', DummyController::class.'::fooAction'); $this->assertSame(array($controller, 'fooAction'), $resolver->getController($request)); $this->assertSame($container, $controller->getContainer()); @@ -157,13 +163,13 @@ class_exists(AbstractControllerTest::class); $resolver = $this->createControllerResolver(null, $container); $request = Request::create('/'); - $request->attributes->set('_controller', DummyController::class.':fooAction'); + $request->attributes->set('_controller', DummyController::class.'::fooAction'); $this->assertSame(array($controller, 'fooAction'), $resolver->getController($request)); $this->assertSame($controllerContainer, $controller->getContainer()); } - protected function createControllerResolver(LoggerInterface $logger = null, Psr11ContainerInterface $container = null, ControllerNameParser $parser = null) + protected function createControllerResolver(LoggerInterface $logger = null, Psr11ContainerInterface $container = null, ControllerNameParser $parser = null): ControllerResolverInterface { if (!$parser) { $parser = $this->createMockParser(); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestController.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestController.php index 1df462992be3..b1e4f79dc16e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestController.php @@ -33,7 +33,7 @@ public function indexAction($handler) // ...to check that the FragmentListener still references the right Request // when rendering another fragment after the error occurred // should render en/html instead of fr/json - $content .= $handler->render(new ControllerReference('TestBundle:SubRequest:fragment')); + $content .= $handler->render(new ControllerReference(self::class.'::fragmentAction')); // forces the LocaleListener to set fr for the locale... // should render fr/json diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestServiceResolutionController.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestServiceResolutionController.php index 967eac3140e5..707edbc6dd8b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestServiceResolutionController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestServiceResolutionController.php @@ -24,7 +24,7 @@ class SubRequestServiceResolutionController implements ContainerAwareInterface public function indexAction() { $request = $this->container->get('request_stack')->getCurrentRequest(); - $path['_controller'] = 'TestBundle:SubRequestServiceResolution:fragment'; + $path['_controller'] = self::class.'::fragmentAction'; $subRequest = $request->duplicate(array(), null, $path); return $this->container->get('http_kernel')->handle($subRequest, HttpKernelInterface::SUB_REQUEST); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml index 11b85a86d329..0730b723282c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml @@ -1,49 +1,49 @@ session_welcome: path: /session - defaults: { _controller: TestBundle:Session:welcome } + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SessionController::welcomeAction } session_welcome_name: path: /session/{name} - defaults: { _controller: TestBundle:Session:welcome } + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SessionController::welcomeAction } session_logout: path: /session_logout - defaults: { _controller: TestBundle:Session:logout} + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SessionController::logoutAction } session_setflash: path: /session_setflash/{message} - defaults: { _controller: TestBundle:Session:setFlash} + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SessionController::setFlashAction } session_showflash: path: /session_showflash - defaults: { _controller: TestBundle:Session:showFlash} + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SessionController::showFlashAction } profiler: path: /profiler - defaults: { _controller: TestBundle:Profiler:index } + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\ProfilerController::indexAction } subrequest_index: path: /subrequest/{_locale}.{_format} - defaults: { _controller: TestBundle:SubRequest:index, _format: "html" } + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SubRequestController::indexAction, _format: html } schemes: [https] subrequest_fragment_error: path: /subrequest/fragment/error/{_locale}.{_format} - defaults: { _controller: TestBundle:SubRequest:fragmentError, _format: "html" } + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SubRequestController::fragmentErrorAction, _format: html } schemes: [http] subrequest_fragment: path: /subrequest/fragment/{_locale}.{_format} - defaults: { _controller: TestBundle:SubRequest:fragment, _format: "html" } + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SubRequestController::fragmentAction, _format: html } schemes: [http] fragment_home: path: /fragment_home - defaults: { _controller: TestBundle:Fragment:index, _format: txt } + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\FragmentController::indexAction, _format: txt } fragment_inlined: path: /fragment_inlined - defaults: { _controller: TestBundle:Fragment:inlined } + defaults: { _controller: Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\FragmentController::inlinedAction } array_controller: path: /array_controller diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ControllerServiceResolution/routing.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ControllerServiceResolution/routing.yml index ffd9471525a6..3f9a83a7208a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ControllerServiceResolution/routing.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ControllerServiceResolution/routing.yml @@ -1,4 +1,4 @@ sub_request_page: path: /subrequest defaults: - _controller: 'TestBundle:SubRequestServiceResolution:index' + _controller: 'Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SubRequestServiceResolutionController::indexAction' diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Resources/views/fragment.html.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Resources/views/fragment.html.php index 8ea3c36f8a4d..ae574bce9954 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Resources/views/fragment.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Resources/views/fragment.html.php @@ -1,14 +1,14 @@ -get('actions')->render($this->get('actions')->controller('TestBundle:Fragment:inlined', array( +get('actions')->render($this->get('actions')->controller('Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\FragmentController::inlinedAction', array( 'options' => array( 'bar' => $bar, 'eleven' => 11, ), ))); ?>--get('actions')->render($this->get('actions')->controller('TestBundle:Fragment:customformat', array('_format' => 'html'))); + echo $this->get('actions')->render($this->get('actions')->controller('Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\FragmentController::customformatAction', array('_format' => 'html'))); ?>--get('actions')->render($this->get('actions')->controller('TestBundle:Fragment:customlocale', array('_locale' => 'es'))); + echo $this->get('actions')->render($this->get('actions')->controller('Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\FragmentController::customlocaleAction', array('_locale' => 'es'))); ?>--getRequest()->setLocale('fr'); - echo $this->get('actions')->render($this->get('actions')->controller('TestBundle:Fragment:forwardlocale')); + echo $this->get('actions')->render($this->get('actions')->controller('Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\FragmentController::forwardlocaleAction')); ?> diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/ConcreteMicroKernel.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/ConcreteMicroKernel.php index 1994778095d1..a54c57bfaee3 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/ConcreteMicroKernel.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/ConcreteMicroKernel.php @@ -72,8 +72,8 @@ public function __destruct() protected function configureRoutes(RouteCollectionBuilder $routes) { - $routes->add('/', 'kernel:halloweenAction'); - $routes->add('/danger', 'kernel:dangerousAction'); + $routes->add('/', 'kernel::halloweenAction'); + $routes->add('/danger', 'kernel::dangerousAction'); } protected function configureContainer(ContainerBuilder $c, LoaderInterface $loader) diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index a2114707c29f..5fbd6e5ac53c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -23,11 +23,11 @@ "symfony/config": "~3.4|~4.0", "symfony/event-dispatcher": "~3.4|~4.0", "symfony/http-foundation": "~3.4|~4.0", - "symfony/http-kernel": "~3.4|~4.0", + "symfony/http-kernel": "^4.1", "symfony/polyfill-mbstring": "~1.0", "symfony/filesystem": "~3.4|~4.0", "symfony/finder": "~3.4|~4.0", - "symfony/routing": "^3.4.5|^4.0.5" + "symfony/routing": "^4.1" }, "require-dev": { "doctrine/cache": "~1.0", diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/config/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/config/routing.yml index 0a02730233f0..bba323fe4ab0 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/config/routing.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Resources/config/routing.yml @@ -1,30 +1,30 @@ form_login: path: /login - defaults: { _controller: CsrfFormLoginBundle:Login:login } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle\Controller\LoginController::loginAction } form_login_check: path: /login_check - defaults: { _controller: CsrfFormLoginBundle:Login:loginCheck } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle\Controller\LoginController::loginCheckAction } form_login_homepage: path: / - defaults: { _controller: CsrfFormLoginBundle:Login:afterLogin } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle\Controller\LoginController::afterLoginAction } form_login_custom_target_path: path: /foo - defaults: { _controller: CsrfFormLoginBundle:Login:afterLogin } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle\Controller\LoginController::afterLoginAction } form_login_default_target_path: path: /profile - defaults: { _controller: CsrfFormLoginBundle:Login:afterLogin } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle\Controller\LoginController::afterLoginAction } form_login_redirect_to_protected_resource_after_login: path: /protected-resource - defaults: { _controller: CsrfFormLoginBundle:Login:afterLogin } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle\Controller\LoginController::afterLoginAction } form_logout: path: /logout_path form_secure_action: path: /secure-but-not-covered-by-access-control - defaults: { _controller: CsrfFormLoginBundle:Login:secure } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\CsrfFormLoginBundle\Controller\LoginController::secureAction } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/localized_routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/localized_routing.yml index 964a74fe893c..51f353654996 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/localized_routing.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/localized_routing.yml @@ -1,29 +1,29 @@ localized_login_path: path: /{_locale}/login - defaults: { _controller: FormLoginBundle:Localized:login } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LocalizedController::loginAction } requirements: { _locale: "^[a-z]{2}$" } localized_check_path: path: /{_locale}/login_check - defaults: { _controller: FormLoginBundle:Localized:loginCheck } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LocalizedController::loginCheckAction } requirements: { _locale: "^[a-z]{2}$" } localized_default_target_path: path: /{_locale}/profile - defaults: { _controller: FormLoginBundle:Localized:profile } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LocalizedController::profileAction } requirements: { _locale: "^[a-z]{2}$" } localized_logout_path: path: /{_locale}/logout - defaults: { _controller: FormLoginBundle:Localized:logout } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LocalizedController::logoutAction } requirements: { _locale: "^[a-z]{2}$" } localized_logout_target_path: path: /{_locale}/ - defaults: { _controller: FormLoginBundle:Localized:homepage } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LocalizedController::homepageAction } requirements: { _locale: "^[a-z]{2}$" } localized_secure_path: path: /{_locale}/secure/ - defaults: { _controller: FormLoginBundle:Localized:secure } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LocalizedController::secureAction } requirements: { _locale: "^[a-z]{2}$" } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/routing.yml index 6992f80a0a12..30466deb32da 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/routing.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/routing.yml @@ -1,26 +1,26 @@ form_login: path: /login - defaults: { _controller: FormLoginBundle:Login:login } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LoginController::loginAction } form_login_check: path: /login_check - defaults: { _controller: FormLoginBundle:Login:loginCheck } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LoginController::loginCheckAction } form_login_homepage: path: / - defaults: { _controller: FormLoginBundle:Login:afterLogin } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LoginController::afterLoginAction } form_login_custom_target_path: path: /foo - defaults: { _controller: FormLoginBundle:Login:afterLogin } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LoginController::afterLoginAction } form_login_default_target_path: path: /profile - defaults: { _controller: FormLoginBundle:Login:afterLogin } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LoginController::afterLoginAction } form_login_redirect_to_protected_resource_after_login: path: /protected_resource - defaults: { _controller: FormLoginBundle:Login:afterLogin } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LoginController::afterLoginAction } highly_protected_resource: path: /highly_protected_resource @@ -36,7 +36,7 @@ form_logout: form_secure_action: path: /secure-but-not-covered-by-access-control - defaults: { _controller: FormLoginBundle:Login:secure } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LoginController::secureAction } protected-via-expression: path: /protected-via-expression diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLogin/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLogin/routing.yml index ee49b4829bdd..e2cee70dd995 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLogin/routing.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLogin/routing.yml @@ -1,3 +1,3 @@ login_check: path: /chk - defaults: { _controller: JsonLoginBundle:Test:loginCheck } + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLoginBundle\Controller\TestController::loginCheckAction } diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json index ab4635b04f66..d58fa8ce58a5 100644 --- a/src/Symfony/Bundle/SecurityBundle/composer.json +++ b/src/Symfony/Bundle/SecurityBundle/composer.json @@ -20,7 +20,7 @@ "ext-xml": "*", "symfony/security": "~4.1", "symfony/dependency-injection": "^3.4.3|^4.0.3", - "symfony/http-kernel": "~3.4|~4.0" + "symfony/http-kernel": "^4.1" }, "require-dev": { "symfony/asset": "~3.4|~4.0", diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php index e2e4c5657963..53e33d647d5e 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php @@ -34,7 +34,7 @@ public function getConfigTreeBuilder() $rootNode ->children() - ->scalarNode('exception_controller')->defaultValue('twig.controller.exception:showAction')->end() + ->scalarNode('exception_controller')->defaultValue('twig.controller.exception::showAction')->end() ->end() ; diff --git a/src/Symfony/Bundle/TwigBundle/Resources/config/routing/errors.xml b/src/Symfony/Bundle/TwigBundle/Resources/config/routing/errors.xml index bf87f8be7ab6..665c50b7ec13 100644 --- a/src/Symfony/Bundle/TwigBundle/Resources/config/routing/errors.xml +++ b/src/Symfony/Bundle/TwigBundle/Resources/config/routing/errors.xml @@ -5,7 +5,7 @@ xsi:schemaLocation="http://symfony.com/schema/routing http://symfony.com/schema/routing/routing-1.0.xsd"> - twig.controller.preview_error:previewErrorPageAction + twig.controller.preview_error::previewErrorPageAction html \d+ diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/routing/profiler.xml b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/routing/profiler.xml index 0be717b19d6e..a164051f7071 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/routing/profiler.xml +++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/routing/profiler.xml @@ -5,43 +5,43 @@ xsi:schemaLocation="http://symfony.com/schema/routing http://symfony.com/schema/routing/routing-1.0.xsd"> - web_profiler.controller.profiler:homeAction + web_profiler.controller.profiler::homeAction - web_profiler.controller.profiler:searchAction + web_profiler.controller.profiler::searchAction - web_profiler.controller.profiler:searchBarAction + web_profiler.controller.profiler::searchBarAction - web_profiler.controller.profiler:phpinfoAction + web_profiler.controller.profiler::phpinfoAction - web_profiler.controller.profiler:searchResultsAction + web_profiler.controller.profiler::searchResultsAction - web_profiler.controller.profiler:openAction + web_profiler.controller.profiler::openAction - web_profiler.controller.profiler:panelAction + web_profiler.controller.profiler::panelAction - web_profiler.controller.router:panelAction + web_profiler.controller.router::panelAction - web_profiler.controller.exception:showAction + web_profiler.controller.exception::showAction - web_profiler.controller.exception:cssAction + web_profiler.controller.exception::cssAction diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/routing/wdt.xml b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/routing/wdt.xml index 1027ce42f66b..d2712fdbac19 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/routing/wdt.xml +++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/routing/wdt.xml @@ -5,6 +5,6 @@ xsi:schemaLocation="http://symfony.com/schema/routing http://symfony.com/schema/routing/routing-1.0.xsd"> - web_profiler.controller.profiler:toolbarAction + web_profiler.controller.profiler::toolbarAction diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md index e22c724085cb..9b5533f4b5b8 100644 --- a/src/Symfony/Component/HttpKernel/CHANGELOG.md +++ b/src/Symfony/Component/HttpKernel/CHANGELOG.md @@ -6,6 +6,7 @@ CHANGELOG * added orphaned events support to `EventDataCollector` * `ExceptionListener` now logs and collects exceptions at priority `2048` (previously logged at `-128` and collected at `0`) + * Deprecated `service:action` syntax with a single colon to reference controllers. Use `service::method` instead. 4.0.0 ----- diff --git a/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php index 426e6728839c..ed515d247c39 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php @@ -14,7 +14,6 @@ use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\DependencyInjection\Container; -use Symfony\Component\HttpFoundation\Request; /** * A controller resolver searching for a controller in a psr-11 container when using the "service:method" notation. @@ -33,58 +32,14 @@ public function __construct(ContainerInterface $container, LoggerInterface $logg parent::__construct($logger); } - /** - * {@inheritdoc} - */ - public function getController(Request $request) - { - $controller = parent::getController($request); - - if (is_array($controller) && isset($controller[0]) && is_string($controller[0]) && $this->container->has($controller[0])) { - $controller[0] = $this->instantiateController($controller[0]); - } - - return $controller; - } - - /** - * Returns a callable for the given controller. - * - * @param string $controller A Controller string - * - * @return mixed A PHP callable - * - * @throws \LogicException When the name could not be parsed - * @throws \InvalidArgumentException When the controller class does not exist - */ protected function createController($controller) { - if (false !== strpos($controller, '::')) { - return parent::createController($controller); - } - - $method = null; - if (1 == substr_count($controller, ':')) { - // controller in the "service:method" notation - list($controller, $method) = explode(':', $controller, 2); - } - - if (!$this->container->has($controller)) { - $this->throwExceptionIfControllerWasRemoved($controller); - - throw new \LogicException(sprintf('Controller not found: service "%s" does not exist.', $controller)); - } - - $service = $this->container->get($controller); - if (null !== $method) { - return array($service, $method); + if (1 === substr_count($controller, ':')) { + $controller = str_replace(':', '::', $controller); + @trigger_error(sprintf('Referencing controllers with a single colon is deprecated since Symfony 4.1. Use %s instead.', $controller), E_USER_DEPRECATED); } - if (!method_exists($service, '__invoke')) { - throw new \LogicException(sprintf('Controller "%s" cannot be called without a method name. Did you forget an "__invoke" method?', $controller)); - } - - return $service; + return parent::createController($controller); } /** @@ -98,22 +53,22 @@ protected function instantiateController($class) try { return parent::instantiateController($class); - } catch (\ArgumentCountError $e) { + } catch (\Error $e) { } $this->throwExceptionIfControllerWasRemoved($class, $e); - throw $e; + if ($e instanceof \ArgumentCountError) { + throw new \InvalidArgumentException(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); + } + + throw new \InvalidArgumentException(sprintf('Controller "%s" does neither exist as service nor as class', $class), 0, $e); } - /** - * @param string $controller - * @param \Exception|\Throwable|null $previous - */ - private function throwExceptionIfControllerWasRemoved($controller, $previous = null) + private function throwExceptionIfControllerWasRemoved(string $controller, \Throwable $previous) { if ($this->container instanceof Container && isset($this->container->getRemovedIds()[$controller])) { - 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); + throw new \InvalidArgumentException(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); } } } diff --git a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php index 4d4d5ac129dd..274602a6ad47 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php @@ -16,10 +16,10 @@ /** * This implementation uses the '_controller' request attribute to determine - * the controller to execute and uses the request attributes to determine - * the controller method arguments. + * the controller to execute. * * @author Fabien Potencier + * @author Tobias Schultze */ class ControllerResolver implements ControllerResolverInterface { @@ -32,9 +32,6 @@ public function __construct(LoggerInterface $logger = null) /** * {@inheritdoc} - * - * This method looks for a '_controller' request attribute that represents - * the controller name (a string like ClassName::MethodName). */ public function getController(Request $request) { @@ -47,23 +44,42 @@ public function getController(Request $request) } if (is_array($controller)) { + if (isset($controller[0]) && is_string($controller[0]) && isset($controller[1])) { + try { + $controller[0] = $this->instantiateController($controller[0]); + } catch (\Error | \LogicException $e) { + try { + // We cannot just check is_callable but have to use reflection because a non-static method + // can still be called statically in PHP but we don't want that. This is deprecated in PHP 7, so we + // could simplify this with PHP 8. + if ((new \ReflectionMethod($controller[0], $controller[1]))->isStatic()) { + return $controller; + } + } catch (\ReflectionException $reflectionException) { + throw $e; + } + + throw $e; + } + } + + if (!is_callable($controller)) { + throw new \InvalidArgumentException(sprintf('The controller for URI "%s" is not callable. %s', $request->getPathInfo(), $this->getControllerError($controller))); + } + return $controller; } if (is_object($controller)) { - if (method_exists($controller, '__invoke')) { - return $controller; + if (!is_callable($controller)) { + throw new \InvalidArgumentException(sprintf('The controller for URI "%s" is not callable. %s', $request->getPathInfo(), $this->getControllerError($controller))); } - throw new \InvalidArgumentException(sprintf('Controller "%s" for URI "%s" is not callable.', get_class($controller), $request->getPathInfo())); + return $controller; } - if (false === strpos($controller, ':')) { - if (method_exists($controller, '__invoke')) { - return $this->instantiateController($controller); - } elseif (function_exists($controller)) { - return $controller; - } + if (function_exists($controller)) { + return $controller; } $callable = $this->createController($controller); @@ -81,22 +97,28 @@ public function getController(Request $request) * @param string $controller A Controller string * * @return callable A PHP callable - * - * @throws \InvalidArgumentException */ protected function createController($controller) { if (false === strpos($controller, '::')) { - throw new \InvalidArgumentException(sprintf('Unable to find controller "%s".', $controller)); + return $this->instantiateController($controller); } list($class, $method) = explode('::', $controller, 2); - if (!class_exists($class)) { - throw new \InvalidArgumentException(sprintf('Class "%s" does not exist.', $class)); - } + try { + return array($this->instantiateController($class), $method); + } catch (\Error | \LogicException $e) { + try { + if ((new \ReflectionMethod($class, $method))->isStatic()) { + return $class.'::'.$method; + } + } catch (\ReflectionException $reflectionException) { + throw $e; + } - return array($this->instantiateController($class), $method); + throw $e; + } } /** @@ -115,24 +137,25 @@ private function getControllerError($callable) { if (is_string($callable)) { if (false !== strpos($callable, '::')) { - $callable = explode('::', $callable); + $callable = explode('::', $callable, 2); + } else { + return sprintf('Function "%s" does not exist.', $callable); } + } - if (class_exists($callable) && !method_exists($callable, '__invoke')) { - return sprintf('Class "%s" does not have a method "__invoke".', $callable); - } + if (is_object($callable)) { + $availableMethods = $this->getClassMethodsWithoutMagicMethods($callable); + $alternativeMsg = $availableMethods ? sprintf(' or use one of the available methods: "%s"', implode('", "', $availableMethods)) : ''; - if (!function_exists($callable)) { - return sprintf('Function "%s" does not exist.', $callable); - } + return sprintf('Controller class "%s" cannot be called without a method name. You need to implement "__invoke"%s.', get_class($callable), $alternativeMsg); } if (!is_array($callable)) { - return sprintf('Invalid type for controller given, expected string or array, got "%s".', gettype($callable)); + return sprintf('Invalid type for controller given, expected string, array or object, got "%s".', gettype($callable)); } - if (2 !== count($callable)) { - return 'Invalid format for controller, expected array(controller, method) or controller::method.'; + if (!isset($callable[0]) || !isset($callable[1]) || 2 !== count($callable)) { + return 'Invalid array callable, expected array(controller, method).'; } list($controller, $method) = $callable; @@ -147,7 +170,7 @@ private function getControllerError($callable) return sprintf('Method "%s" on class "%s" should be public and non-abstract.', $method, $className); } - $collection = get_class_methods($controller); + $collection = $this->getClassMethodsWithoutMagicMethods($controller); $alternatives = array(); @@ -171,4 +194,13 @@ private function getControllerError($callable) return $message; } + + private function getClassMethodsWithoutMagicMethods($classOrObject) + { + $methods = get_class_methods($classOrObject); + + return array_filter($methods, function(string $method) { + return 0 !== strncmp($method, '__', 2); + }); + } } diff --git a/src/Symfony/Component/HttpKernel/Controller/ControllerResolverInterface.php b/src/Symfony/Component/HttpKernel/Controller/ControllerResolverInterface.php index 30b73f6b53b9..a54f8b518bf0 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ControllerResolverInterface.php +++ b/src/Symfony/Component/HttpKernel/Controller/ControllerResolverInterface.php @@ -17,8 +17,6 @@ * A ControllerResolverInterface implementation knows how to determine the * controller to execute based on a Request object. * - * It can also determine the arguments to pass to the Controller. - * * A Controller can be any valid PHP callable. * * @author Fabien Potencier @@ -37,7 +35,7 @@ interface ControllerResolverInterface * @return callable|false A PHP callable representing the Controller, * or false if this resolver is not able to determine the controller * - * @throws \LogicException If the controller can't be found + * @throws \LogicException If a controller was found based on the request but it is not callable */ public function getController(Request $request); } diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php index 8cb19c479041..afa480b31e53 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php @@ -168,7 +168,7 @@ public function process(ContainerBuilder $container) } // register the maps as a per-method service-locators if ($args) { - $controllers[$id.':'.$r->name] = ServiceLocatorTagPass::register($container, $args); + $controllers[$id.'::'.$r->name] = ServiceLocatorTagPass::register($container, $args); } } } diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RemoveEmptyControllerArgumentLocatorsPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RemoveEmptyControllerArgumentLocatorsPass.php index 52c6d69c1f71..b7d64994cec8 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/RemoveEmptyControllerArgumentLocatorsPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RemoveEmptyControllerArgumentLocatorsPass.php @@ -47,8 +47,7 @@ public function process(ContainerBuilder $container) } else { // any methods listed for call-at-instantiation cannot be actions $reason = false; - $action = substr(strrchr($controller, ':'), 1); - $id = substr($controller, 0, -1 - strlen($action)); + list($id, $action) = explode('::', $controller); $controllerDef = $container->getDefinition($id); foreach ($controllerDef->getMethodCalls() as list($method)) { if (0 === strcasecmp($action, $method)) { @@ -57,9 +56,9 @@ public function process(ContainerBuilder $container) } } if (!$reason) { - if ($controllerDef->getClass() === $id) { - $controllers[$id.'::'.$action] = $argumentRef; - } + // Deprecated since Symfony 4.1. See Symfony\Component\HttpKernel\Controller\ContainerControllerResolver + $controllers[$id.':'.$action] = $argumentRef; + if ('__invoke' === $action) { $controllers[$id] = $argumentRef; } diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php index 6f203d3a987c..2457c4e8141d 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php @@ -13,15 +13,21 @@ use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; -use Symfony\Component\Debug\ErrorHandler; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Controller\ContainerControllerResolver; +use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; class ContainerControllerResolverTest extends ControllerResolverTest { - public function testGetControllerService() + /** + * @group legacy + * @expectedDeprecation Referencing controllers with a single colon is deprecated since Symfony 4.1. Use foo::action instead. + */ + public function testGetControllerServiceWithSingleColon() { + $service = new ControllerTestService('foo'); + $container = $this->createMockContainer(); $container->expects($this->once()) ->method('has') @@ -30,153 +36,132 @@ public function testGetControllerService() $container->expects($this->once()) ->method('get') ->with('foo') - ->will($this->returnValue($this)) + ->will($this->returnValue($service)) ; $resolver = $this->createControllerResolver(null, $container); $request = Request::create('/'); - $request->attributes->set('_controller', 'foo:controllerMethod1'); + $request->attributes->set('_controller', 'foo:action'); $controller = $resolver->getController($request); - $this->assertInstanceOf(get_class($this), $controller[0]); - $this->assertSame('controllerMethod1', $controller[1]); + $this->assertSame($service, $controller[0]); + $this->assertSame('action', $controller[1]); } - public function testGetControllerInvokableService() + public function testGetControllerService() { - $invokableController = new InvokableController('bar'); + $service = new ControllerTestService('foo'); $container = $this->createMockContainer(); $container->expects($this->once()) ->method('has') ->with('foo') - ->will($this->returnValue(true)) - ; + ->will($this->returnValue(true)); $container->expects($this->once()) ->method('get') ->with('foo') - ->will($this->returnValue($invokableController)) + ->will($this->returnValue($service)) ; $resolver = $this->createControllerResolver(null, $container); $request = Request::create('/'); - $request->attributes->set('_controller', 'foo'); + $request->attributes->set('_controller', 'foo::action'); $controller = $resolver->getController($request); - $this->assertEquals($invokableController, $controller); + $this->assertSame($service, $controller[0]); + $this->assertSame('action', $controller[1]); } - public function testGetControllerInvokableServiceWithClassNameAsName() + public function testGetControllerInvokableService() { - $invokableController = new InvokableController('bar'); - $className = __NAMESPACE__.'\InvokableController'; + $service = new InvokableControllerService('bar'); $container = $this->createMockContainer(); $container->expects($this->once()) ->method('has') - ->with($className) + ->with('foo') ->will($this->returnValue(true)) ; $container->expects($this->once()) ->method('get') - ->with($className) - ->will($this->returnValue($invokableController)) + ->with('foo') + ->will($this->returnValue($service)) ; $resolver = $this->createControllerResolver(null, $container); $request = Request::create('/'); - $request->attributes->set('_controller', $className); + $request->attributes->set('_controller', 'foo'); $controller = $resolver->getController($request); - $this->assertEquals($invokableController, $controller); + $this->assertSame($service, $controller); } - public function testNonInstantiableController() + public function testGetControllerInvokableServiceWithClassNameAsName() { + $service = new InvokableControllerService('bar'); + $container = $this->createMockContainer(); $container->expects($this->once()) ->method('has') - ->with(NonInstantiableController::class) - ->will($this->returnValue(false)) + ->with(InvokableControllerService::class) + ->will($this->returnValue(true)) + ; + $container->expects($this->once()) + ->method('get') + ->with(InvokableControllerService::class) + ->will($this->returnValue($service)) ; $resolver = $this->createControllerResolver(null, $container); $request = Request::create('/'); - $request->attributes->set('_controller', array(NonInstantiableController::class, 'action')); + $request->attributes->set('_controller', InvokableControllerService::class); $controller = $resolver->getController($request); - $this->assertSame(array(NonInstantiableController::class, 'action'), $controller); + $this->assertSame($service, $controller); } /** - * @expectedException \LogicException - * @expectedExceptionMessage Controller "Symfony\Component\HttpKernel\Tests\Controller\ImpossibleConstructController" cannot be fetched from the container because it is private. Did you forget to tag the service with "controller.service_arguments"? + * Tests where the fallback instantiation fails due to required constructor arguments. + * + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Controller "Symfony\Component\HttpKernel\Tests\Controller\ControllerTestService" cannot be fetched from the container because it is private. Did you forget to tag the service with "controller.service_arguments"? */ - public function testNonConstructController() + public function testExceptionWhenUsingRemovedControllerServiceWithClassNameAsName() { $container = $this->getMockBuilder(Container::class)->getMock(); - $container->expects($this->at(0)) - ->method('has') - ->with(ImpossibleConstructController::class) - ->will($this->returnValue(true)) - ; - - $container->expects($this->at(1)) + $container->expects($this->once()) ->method('has') - ->with(ImpossibleConstructController::class) + ->with(ControllerTestService::class) ->will($this->returnValue(false)) ; $container->expects($this->atLeastOnce()) ->method('getRemovedIds') ->with() - ->will($this->returnValue(array(ImpossibleConstructController::class => true))) + ->will($this->returnValue(array(ControllerTestService::class => true))) ; $resolver = $this->createControllerResolver(null, $container); $request = Request::create('/'); - $request->attributes->set('_controller', array(ImpossibleConstructController::class, 'action')); + $request->attributes->set('_controller', array(ControllerTestService::class, 'action')); $resolver->getController($request); } - public function testNonInstantiableControllerWithCorrespondingService() - { - $service = new \stdClass(); - - $container = $this->createMockContainer(); - $container->expects($this->atLeastOnce()) - ->method('has') - ->with(NonInstantiableController::class) - ->will($this->returnValue(true)) - ; - $container->expects($this->atLeastOnce()) - ->method('get') - ->with(NonInstantiableController::class) - ->will($this->returnValue($service)) - ; - - $resolver = $this->createControllerResolver(null, $container); - $request = Request::create('/'); - $request->attributes->set('_controller', array(NonInstantiableController::class, 'action')); - - $controller = $resolver->getController($request); - - $this->assertSame(array($service, 'action'), $controller); - } - /** - * @expectedException \LogicException + * Tests where the fallback instantiation fails due to non-existing class. + * + * @expectedException \InvalidArgumentException * @expectedExceptionMessage Controller "app.my_controller" cannot be fetched from the container because it is private. Did you forget to tag the service with "controller.service_arguments"? */ public function testExceptionWhenUsingRemovedControllerService() { $container = $this->getMockBuilder(Container::class)->getMock(); - $container->expects($this->at(0)) + $container->expects($this->once()) ->method('has') ->with('app.my_controller') ->will($this->returnValue(false)) @@ -195,65 +180,31 @@ public function testExceptionWhenUsingRemovedControllerService() $resolver->getController($request); } - /** - * @expectedException \LogicException - * @expectedExceptionMessage Controller "app.my_controller" cannot be called without a method name. Did you forget an "__invoke" method? - */ - public function testExceptionWhenUsingControllerWithoutAnInvokeMethod() - { - $container = $this->getMockBuilder(Container::class)->getMock(); - $container->expects($this->once()) - ->method('has') - ->with('app.my_controller') - ->will($this->returnValue(true)) - ; - $container->expects($this->once()) - ->method('get') - ->with('app.my_controller') - ->will($this->returnValue(new ImpossibleConstructController('toto', 'controller'))) - ; - - $resolver = $this->createControllerResolver(null, $container); - - $request = Request::create('/'); - $request->attributes->set('_controller', 'app.my_controller'); - $resolver->getController($request); - } - - /** - * @dataProvider getUndefinedControllers - */ - public function testGetControllerOnNonUndefinedFunction($controller, $exceptionName = null, $exceptionMessage = null) - { - // All this logic needs to be duplicated, since calling parent::testGetControllerOnNonUndefinedFunction will override the expected excetion and not use the regex - $resolver = $this->createControllerResolver(); - if (method_exists($this, 'expectException')) { - $this->expectException($exceptionName); - $this->expectExceptionMessageRegExp($exceptionMessage); - } else { - $this->setExpectedExceptionRegExp($exceptionName, $exceptionMessage); - } - - $request = Request::create('/'); - $request->attributes->set('_controller', $controller); - $resolver->getController($request); - } - public function getUndefinedControllers() { - return array( - array('foo', \LogicException::class, '/Controller not found: service "foo" does not exist\./'), - array('oof::bar', \InvalidArgumentException::class, '/Class "oof" does not exist\./'), - array('stdClass', \LogicException::class, '/Controller not found: service "stdClass" does not exist\./'), - array( - 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::bar', - \InvalidArgumentException::class, - '/.?[cC]ontroller(.*?) for URI "\/" is not callable\.( Expected method(.*) Available methods)?/', - ), + $tests = parent::getUndefinedControllers(); + $tests[0] = array('foo', \InvalidArgumentException::class, 'Controller "foo" does neither exist as service nor as class'); + $tests[1] = array('oof::bar', \InvalidArgumentException::class, 'Controller "oof" does neither exist as service nor as class'); + $tests[2] = array(array('oof', 'bar'), \InvalidArgumentException::class, 'Controller "oof" does neither exist as service nor as class'); + $tests[] = array( + array(ControllerTestService::class, 'action'), + \InvalidArgumentException::class, + 'Controller "Symfony\Component\HttpKernel\Tests\Controller\ControllerTestService" has required constructor arguments and does not exist in the container. Did you forget to define such a service?', ); + $tests[] = array( + ControllerTestService::class.'::action', + \InvalidArgumentException::class, 'Controller "Symfony\Component\HttpKernel\Tests\Controller\ControllerTestService" has required constructor arguments and does not exist in the container. Did you forget to define such a service?', + ); + $tests[] = array( + InvokableControllerService::class, + \InvalidArgumentException::class, + 'Controller "Symfony\Component\HttpKernel\Tests\Controller\InvokableControllerService" has required constructor arguments and does not exist in the container. Did you forget to define such a service?', + ); + + return $tests; } - protected function createControllerResolver(LoggerInterface $logger = null, ContainerInterface $container = null) + protected function createControllerResolver(LoggerInterface $logger = null, ContainerInterface $container = null): ControllerResolverInterface { if (!$container) { $container = $this->createMockContainer(); @@ -268,7 +219,7 @@ protected function createMockContainer() } } -class InvokableController +class InvokableControllerService { public function __construct($bar) // mandatory argument to prevent automatic instantiation { @@ -279,16 +230,9 @@ public function __invoke() } } -abstract class NonInstantiableController -{ - public static function action() - { - } -} - -class ImpossibleConstructController +class ControllerTestService { - public function __construct($toto, $controller) + public function __construct($foo) { } diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php index 203a162fa815..d845a0bac826 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php @@ -15,6 +15,7 @@ use Psr\Log\LoggerInterface; use Symfony\Component\HttpKernel\Controller\ControllerResolver; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; class ControllerResolverTest extends TestCase { @@ -41,51 +42,55 @@ public function testGetControllerWithLambda() public function testGetControllerWithObjectAndInvokeMethod() { $resolver = $this->createControllerResolver(); + $object = new InvokableController(); $request = Request::create('/'); - $request->attributes->set('_controller', $this); + $request->attributes->set('_controller', $object); $controller = $resolver->getController($request); - $this->assertSame($this, $controller); + $this->assertSame($object, $controller); } public function testGetControllerWithObjectAndMethod() { $resolver = $this->createControllerResolver(); + $object = new ControllerTest(); $request = Request::create('/'); - $request->attributes->set('_controller', array($this, 'controllerMethod1')); + $request->attributes->set('_controller', array($object, 'publicAction')); $controller = $resolver->getController($request); - $this->assertSame(array($this, 'controllerMethod1'), $controller); + $this->assertSame(array($object, 'publicAction'), $controller); } - public function testGetControllerWithClassAndMethod() + public function testGetControllerWithClassAndMethodAsArray() { $resolver = $this->createControllerResolver(); $request = Request::create('/'); - $request->attributes->set('_controller', array('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', 'controllerMethod4')); + $request->attributes->set('_controller', array(ControllerTest::class, 'publicAction')); $controller = $resolver->getController($request); - $this->assertSame(array('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', 'controllerMethod4'), $controller); + $this->assertInstanceOf(ControllerTest::class, $controller[0]); + $this->assertSame('publicAction', $controller[1]); } - public function testGetControllerWithObjectAndMethodAsString() + public function testGetControllerWithClassAndMethodAsString() { $resolver = $this->createControllerResolver(); $request = Request::create('/'); - $request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::controllerMethod1'); + $request->attributes->set('_controller', ControllerTest::class.'::publicAction'); $controller = $resolver->getController($request); - $this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', $controller[0], '->getController() returns a PHP callable'); + $this->assertInstanceOf(ControllerTest::class, $controller[0]); + $this->assertSame('publicAction', $controller[1]); } - public function testGetControllerWithClassAndInvokeMethod() + public function testGetControllerWithInvokableClass() { $resolver = $this->createControllerResolver(); $request = Request::create('/'); - $request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest'); + $request->attributes->set('_controller', InvokableController::class); $controller = $resolver->getController($request); - $this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', $controller); + $this->assertInstanceOf(InvokableController::class, $controller); } /** @@ -110,10 +115,49 @@ public function testGetControllerWithFunction() $this->assertSame('Symfony\Component\HttpKernel\Tests\Controller\some_controller_function', $controller); } + public function testGetControllerWithClosure() + { + $resolver = $this->createControllerResolver(); + + $closure = function () { + return 'test'; + }; + + $request = Request::create('/'); + $request->attributes->set('_controller', $closure); + $controller = $resolver->getController($request); + $this->assertInstanceOf(\Closure::class, $controller); + $this->assertSame('test', $controller()); + } + + /** + * @dataProvider getStaticControllers + */ + public function testGetControllerWithStaticController($staticController, $returnValue) + { + $resolver = $this->createControllerResolver(); + + $request = Request::create('/'); + $request->attributes->set('_controller', $staticController); + $controller = $resolver->getController($request); + $this->assertSame($staticController, $controller); + $this->assertSame($returnValue, $controller()); + } + + public function getStaticControllers() + { + return array( + array(AbstractController::class.'::staticAction', 'foo'), + array(array(AbstractController::class, 'staticAction'), 'foo'), + array(array(PrivateConstructorController::class, 'staticAction'), 'bar'), + array(array(PrivateConstructorController::class, 'staticAction'), 'bar'), + ); + } + /** * @dataProvider getUndefinedControllers */ - public function testGetControllerOnNonUndefinedFunction($controller, $exceptionName = null, $exceptionMessage = null) + public function testGetControllerWithUndefinedController($controller, $exceptionName = null, $exceptionMessage = null) { $resolver = $this->createControllerResolver(); if (method_exists($this, 'expectException')) { @@ -130,55 +174,87 @@ public function testGetControllerOnNonUndefinedFunction($controller, $exceptionN public function getUndefinedControllers() { + $controller = new ControllerTest(); + return array( - array(1, 'InvalidArgumentException', 'Unable to find controller "1".'), - array('foo', 'InvalidArgumentException', 'Unable to find controller "foo".'), - array('oof::bar', 'InvalidArgumentException', 'Class "oof" does not exist.'), - array('stdClass', 'InvalidArgumentException', 'Unable to find controller "stdClass".'), - array('Symfony\Component\HttpKernel\Tests\Controller\ControllerTest::staticsAction', 'InvalidArgumentException', 'The controller for URI "/" is not callable. Expected method "staticsAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest", did you mean "staticAction"?'), - array('Symfony\Component\HttpKernel\Tests\Controller\ControllerTest::privateAction', 'InvalidArgumentException', 'The controller for URI "/" is not callable. Method "privateAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest" should be public and non-abstract'), - array('Symfony\Component\HttpKernel\Tests\Controller\ControllerTest::protectedAction', 'InvalidArgumentException', 'The controller for URI "/" is not callable. Method "protectedAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest" should be public and non-abstract'), - array('Symfony\Component\HttpKernel\Tests\Controller\ControllerTest::undefinedAction', 'InvalidArgumentException', 'The controller for URI "/" is not callable. Expected method "undefinedAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest". Available methods: "publicAction", "staticAction"'), + array('foo', \Error::class, 'Class \'foo\' not found'), + array('oof::bar', \Error::class, 'Class \'oof\' not found'), + array(array('oof', 'bar'), \Error::class, 'Class \'oof\' not found'), + array('Symfony\Component\HttpKernel\Tests\Controller\ControllerTest::staticsAction', \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Expected method "staticsAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest", did you mean "staticAction"?'), + array('Symfony\Component\HttpKernel\Tests\Controller\ControllerTest::privateAction', \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Method "privateAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest" should be public and non-abstract'), + array('Symfony\Component\HttpKernel\Tests\Controller\ControllerTest::protectedAction', \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Method "protectedAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest" should be public and non-abstract'), + array('Symfony\Component\HttpKernel\Tests\Controller\ControllerTest::undefinedAction', \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Expected method "undefinedAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest". Available methods: "publicAction", "staticAction"'), + array('Symfony\Component\HttpKernel\Tests\Controller\ControllerTest', \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Controller class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest" cannot be called without a method name. You need to implement "__invoke" or use one of the available methods: "publicAction", "staticAction".'), + array(array($controller, 'staticsAction'), \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Expected method "staticsAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest", did you mean "staticAction"?'), + array(array($controller, 'privateAction'), \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Method "privateAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest" should be public and non-abstract'), + array(array($controller, 'protectedAction'), \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Method "protectedAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest" should be public and non-abstract'), + array(array($controller, 'undefinedAction'), \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Expected method "undefinedAction" on class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest". Available methods: "publicAction", "staticAction"'), + array($controller, \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Controller class "Symfony\Component\HttpKernel\Tests\Controller\ControllerTest" cannot be called without a method name. You need to implement "__invoke" or use one of the available methods: "publicAction", "staticAction".'), + array(array('a' => 'foo', 'b' => 'bar'), \InvalidArgumentException::class, 'The controller for URI "/" is not callable. Invalid array callable, expected array(controller, method).'), ); } - protected function createControllerResolver(LoggerInterface $logger = null) + protected function createControllerResolver(LoggerInterface $logger = null): ControllerResolverInterface { return new ControllerResolver($logger); } +} - public function __invoke($foo, $bar = null) +function some_controller_function($foo, $foobar) +{ +} + +class ControllerTest +{ + public function __construct() { } - public function controllerMethod1($foo) + public function __toString() { + return ''; } - protected static function controllerMethod4() + public function publicAction() { } -} -function some_controller_function($foo, $foobar) -{ + private function privateAction() + { + } + + protected function protectedAction() + { + } + + public static function staticAction() + { + } } -class ControllerTest +class InvokableController { - public function publicAction() + public function __invoke($foo, $bar = null) { } +} - private function privateAction() +abstract class AbstractController +{ + public static function staticAction() { + return 'foo'; } +} - protected function protectedAction() +class PrivateConstructorController +{ + private function __construct() { } public static function staticAction() { + return 'bar'; } } diff --git a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php index 4016deb4ab29..18bd03ddf872 100644 --- a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php @@ -140,10 +140,10 @@ public function testAllActions() $locator = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); - $this->assertEquals(array('foo:fooAction'), array_keys($locator)); - $this->assertInstanceof(ServiceClosureArgument::class, $locator['foo:fooAction']); + $this->assertEquals(array('foo::fooAction'), array_keys($locator)); + $this->assertInstanceof(ServiceClosureArgument::class, $locator['foo::fooAction']); - $locator = $container->getDefinition((string) $locator['foo:fooAction']->getValues()[0]); + $locator = $container->getDefinition((string) $locator['foo::fooAction']->getValues()[0]); $this->assertSame(ServiceLocator::class, $locator->getClass()); $this->assertFalse($locator->isPublic()); @@ -166,7 +166,7 @@ public function testExplicitArgument() $pass->process($container); $locator = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); - $locator = $container->getDefinition((string) $locator['foo:fooAction']->getValues()[0]); + $locator = $container->getDefinition((string) $locator['foo::fooAction']->getValues()[0]); $expected = array('bar' => new ServiceClosureArgument(new TypedReference('bar', ControllerDummy::class, RegisterTestController::class))); $this->assertEquals($expected, $locator->getArgument(0)); @@ -185,7 +185,7 @@ public function testOptionalArgument() $pass->process($container); $locator = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); - $locator = $container->getDefinition((string) $locator['foo:fooAction']->getValues()[0]); + $locator = $container->getDefinition((string) $locator['foo::fooAction']->getValues()[0]); $expected = array('bar' => new ServiceClosureArgument(new TypedReference('bar', ControllerDummy::class, RegisterTestController::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE))); $this->assertEquals($expected, $locator->getArgument(0)); @@ -203,7 +203,7 @@ public function testSkipSetContainer() $pass->process($container); $locator = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); - $this->assertSame(array('foo:fooAction'), array_keys($locator)); + $this->assertSame(array('foo::fooAction'), array_keys($locator)); } /** @@ -250,7 +250,7 @@ public function testNoExceptionOnNonExistentTypeHintOptionalArg() $pass->process($container); $locator = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); - $this->assertSame(array('foo:barAction', 'foo:fooAction'), array_keys($locator)); + $this->assertSame(array('foo::barAction', 'foo::fooAction'), array_keys($locator)); } public function testArgumentWithNoTypeHintIsOk() @@ -300,7 +300,7 @@ public function testBindings($bindingName) $locator = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); - $locator = $container->getDefinition((string) $locator['foo:fooAction']->getValues()[0]); + $locator = $container->getDefinition((string) $locator['foo::fooAction']->getValues()[0]); $expected = array('bar' => new ServiceClosureArgument(new Reference('foo'))); $this->assertEquals($expected, $locator->getArgument(0)); diff --git a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RemoveEmptyControllerArgumentLocatorsPassTest.php b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RemoveEmptyControllerArgumentLocatorsPassTest.php index 9cac9681857f..36e9310ae783 100644 --- a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RemoveEmptyControllerArgumentLocatorsPassTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RemoveEmptyControllerArgumentLocatorsPassTest.php @@ -36,49 +36,30 @@ public function testProcess() $controllers = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); - $this->assertCount(2, $container->getDefinition((string) $controllers['c1:fooAction']->getValues()[0])->getArgument(0)); - $this->assertCount(1, $container->getDefinition((string) $controllers['c2:setTestCase']->getValues()[0])->getArgument(0)); - $this->assertCount(1, $container->getDefinition((string) $controllers['c2:fooAction']->getValues()[0])->getArgument(0)); + $this->assertCount(2, $container->getDefinition((string) $controllers['c1::fooAction']->getValues()[0])->getArgument(0)); + $this->assertCount(1, $container->getDefinition((string) $controllers['c2::setTestCase']->getValues()[0])->getArgument(0)); + $this->assertCount(1, $container->getDefinition((string) $controllers['c2::fooAction']->getValues()[0])->getArgument(0)); (new ResolveInvalidReferencesPass())->process($container); - $this->assertCount(1, $container->getDefinition((string) $controllers['c2:setTestCase']->getValues()[0])->getArgument(0)); - $this->assertSame(array(), $container->getDefinition((string) $controllers['c2:fooAction']->getValues()[0])->getArgument(0)); + $this->assertCount(1, $container->getDefinition((string) $controllers['c2::setTestCase']->getValues()[0])->getArgument(0)); + $this->assertSame(array(), $container->getDefinition((string) $controllers['c2::fooAction']->getValues()[0])->getArgument(0)); (new RemoveEmptyControllerArgumentLocatorsPass())->process($container); $controllers = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); - $this->assertSame(array('c1:fooAction'), array_keys($controllers)); - $this->assertSame(array('bar'), array_keys($container->getDefinition((string) $controllers['c1:fooAction']->getValues()[0])->getArgument(0))); + $this->assertSame(array('c1::fooAction', 'c1:fooAction'), array_keys($controllers)); + $this->assertSame(array('bar'), array_keys($container->getDefinition((string) $controllers['c1::fooAction']->getValues()[0])->getArgument(0))); $expectedLog = array( - 'Symfony\Component\HttpKernel\DependencyInjection\RemoveEmptyControllerArgumentLocatorsPass: Removing service-argument resolver for controller "c2:fooAction": no corresponding services exist for the referenced types.', + 'Symfony\Component\HttpKernel\DependencyInjection\RemoveEmptyControllerArgumentLocatorsPass: Removing service-argument resolver for controller "c2::fooAction": no corresponding services exist for the referenced types.', 'Symfony\Component\HttpKernel\DependencyInjection\RemoveEmptyControllerArgumentLocatorsPass: Removing method "setTestCase" of service "c2" from controller candidates: the method is called at instantiation, thus cannot be an action.', ); $this->assertSame($expectedLog, $container->getCompiler()->getLog()); } - public function testSameIdClass() - { - $container = new ContainerBuilder(); - $resolver = $container->register('argument_resolver.service')->addArgument(array()); - - $container->register(RegisterTestController::class, RegisterTestController::class) - ->addTag('controller.service_arguments') - ; - - (new RegisterControllerArgumentLocatorsPass())->process($container); - (new RemoveEmptyControllerArgumentLocatorsPass())->process($container); - - $expected = array( - RegisterTestController::class.':fooAction', - RegisterTestController::class.'::fooAction', - ); - $this->assertEquals($expected, array_keys($container->getDefinition((string) $resolver->getArgument(0))->getArgument(0))); - } - public function testInvoke() { $container = new ContainerBuilder(); @@ -92,30 +73,10 @@ public function testInvoke() (new RemoveEmptyControllerArgumentLocatorsPass())->process($container); $this->assertEquals( - array('invokable:__invoke', 'invokable'), + array('invokable::__invoke', 'invokable:__invoke', 'invokable'), array_keys($container->getDefinition((string) $resolver->getArgument(0))->getArgument(0)) ); } - - public function testInvokeSameIdClass() - { - $container = new ContainerBuilder(); - $resolver = $container->register('argument_resolver.service')->addArgument(array()); - - $container->register(InvokableRegisterTestController::class, InvokableRegisterTestController::class) - ->addTag('controller.service_arguments') - ; - - (new RegisterControllerArgumentLocatorsPass())->process($container); - (new RemoveEmptyControllerArgumentLocatorsPass())->process($container); - - $expected = array( - InvokableRegisterTestController::class.':__invoke', - InvokableRegisterTestController::class.'::__invoke', - InvokableRegisterTestController::class, - ); - $this->assertEquals($expected, array_keys($container->getDefinition((string) $resolver->getArgument(0))->getArgument(0))); - } } class RemoveTestController1 diff --git a/src/Symfony/Component/Routing/Loader/ObjectRouteLoader.php b/src/Symfony/Component/Routing/Loader/ObjectRouteLoader.php index 0899a818129b..24925447dfd7 100644 --- a/src/Symfony/Component/Routing/Loader/ObjectRouteLoader.php +++ b/src/Symfony/Component/Routing/Loader/ObjectRouteLoader.php @@ -44,9 +44,14 @@ abstract protected function getServiceObject($id); */ public function load($resource, $type = null) { - $parts = explode(':', $resource); + if (1 === substr_count($resource, ':')) { + $resource = str_replace(':', '::', $resource); + @trigger_error(sprintf('Referencing service route loaders with a single colon is deprecated since Symfony 4.1. Use %s instead.', $resource), E_USER_DEPRECATED); + } + + $parts = explode('::', $resource); if (2 != count($parts)) { - throw new \InvalidArgumentException(sprintf('Invalid resource "%s" passed to the "service" route loader: use the format "service_name:methodName"', $resource)); + throw new \InvalidArgumentException(sprintf('Invalid resource "%s" passed to the "service" route loader: use the format "service::method"', $resource)); } $serviceString = $parts[0]; @@ -58,7 +63,7 @@ public function load($resource, $type = null) throw new \LogicException(sprintf('%s:getServiceObject() must return an object: %s returned', get_class($this), gettype($loaderObject))); } - if (!method_exists($loaderObject, $method)) { + if (!is_callable(array($loaderObject, $method))) { throw new \BadMethodCallException(sprintf('Method "%s" not found on "%s" when importing routing resource "%s"', $method, get_class($loaderObject), $resource)); } diff --git a/src/Symfony/Component/Routing/Tests/Loader/ObjectRouteLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/ObjectRouteLoaderTest.php index 408fa0b45c48..ed6506829f1d 100644 --- a/src/Symfony/Component/Routing/Tests/Loader/ObjectRouteLoaderTest.php +++ b/src/Symfony/Component/Routing/Tests/Loader/ObjectRouteLoaderTest.php @@ -18,7 +18,11 @@ class ObjectRouteLoaderTest extends TestCase { - public function testLoadCallsServiceAndReturnsCollection() + /** + * @group legacy + * @expectedDeprecation Referencing service route loaders with a single colon is deprecated since Symfony 4.1. Use my_route_provider_service::loadRoutes instead. + */ + public function testLoadCallsServiceAndReturnsCollectionWithLegacyNotation() { $loader = new ObjectRouteLoaderForTest(); @@ -40,6 +44,28 @@ public function testLoadCallsServiceAndReturnsCollection() $this->assertNotEmpty($actualRoutes->getResources()); } + public function testLoadCallsServiceAndReturnsCollection() + { + $loader = new ObjectRouteLoaderForTest(); + + // create a basic collection that will be returned + $collection = new RouteCollection(); + $collection->add('foo', new Route('/foo')); + + $loader->loaderMap = array( + 'my_route_provider_service' => new RouteService($collection), + ); + + $actualRoutes = $loader->load( + 'my_route_provider_service::loadRoutes', + 'service' + ); + + $this->assertSame($collection, $actualRoutes); + // the service file should be listed as a resource + $this->assertNotEmpty($actualRoutes->getResources()); + } + /** * @expectedException \InvalidArgumentException * @dataProvider getBadResourceStrings @@ -54,7 +80,6 @@ public function getBadResourceStrings() { return array( array('Foo'), - array('Bar::baz'), array('Foo:Bar:baz'), ); } @@ -66,7 +91,7 @@ public function testExceptionOnNoObjectReturned() { $loader = new ObjectRouteLoaderForTest(); $loader->loaderMap = array('my_service' => 'NOT_AN_OBJECT'); - $loader->load('my_service:method'); + $loader->load('my_service::method'); } /** @@ -76,7 +101,7 @@ public function testExceptionOnBadMethod() { $loader = new ObjectRouteLoaderForTest(); $loader->loaderMap = array('my_service' => new \stdClass()); - $loader->load('my_service:method'); + $loader->load('my_service::method'); } /** @@ -93,7 +118,7 @@ public function testExceptionOnMethodNotReturningCollection() $loader = new ObjectRouteLoaderForTest(); $loader->loaderMap = array('my_service' => $service); - $loader->load('my_service:loadRoutes'); + $loader->load('my_service::loadRoutes'); } }