From 54a065368d08fe86083cc81cd28a06a4de66e386 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Tue, 9 Jun 2015 23:34:49 +0200 Subject: [PATCH 1/9] Extracted controller argument resolving from the ControllerResolver --- .../FrameworkBundle/FrameworkBundle.php | 2 + .../Resources/config/services.xml | 1 + .../FrameworkBundle/Resources/config/web.xml | 10 ++ src/Symfony/Component/HttpKernel/CHANGELOG.md | 6 + .../ArgumentResolverInterface.php | 44 ++++++ .../RequestArgumentResolver.php | 40 ++++++ .../RequestAttributesArgumentResolver.php | 38 +++++ .../Controller/ArgumentResolverManager.php | 83 +++++++++++ .../Controller/ControllerResolver.php | 59 +++++--- .../ControllerResolverInterface.php | 2 + .../ContainerAwareHttpKernel.php | 16 ++- .../RegisterArgumentResolversPass.php | 38 +++++ .../Component/HttpKernel/HttpKernel.php | 27 +++- .../ArgumentResolverManagerTest.php | 132 ++++++++++++++++++ .../Controller/ControllerResolverTest.php | 8 +- .../Debug/TraceableEventDispatcherTest.php | 15 ++ .../Fragment/InlineFragmentRendererTest.php | 5 + .../Tests/HttpCache/TestHttpKernel.php | 16 +-- .../HttpCache/TestMultipleHttpKernel.php | 16 +-- .../HttpKernel/Tests/HttpKernelTest.php | 5 + .../Tests/TestArgumentResolverManager.php | 26 ++++ .../Tests/TestControllerResolver.php | 26 ++++ .../HttpKernel/Tests/TestHttpKernel.php | 14 +- 23 files changed, 555 insertions(+), 74 deletions(-) create mode 100644 src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/ArgumentResolverInterface.php create mode 100644 src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestArgumentResolver.php create mode 100644 src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestAttributesArgumentResolver.php create mode 100644 src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php create mode 100644 src/Symfony/Component/HttpKernel/DependencyInjection/RegisterArgumentResolversPass.php create mode 100644 src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php create mode 100644 src/Symfony/Component/HttpKernel/Tests/TestArgumentResolverManager.php create mode 100644 src/Symfony/Component/HttpKernel/Tests/TestControllerResolver.php diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index 499f39c6c95c0..6a1c8baf7f9dd 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -36,6 +36,7 @@ use Symfony\Component\HttpKernel\DependencyInjection\FragmentRendererPass; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Bundle\Bundle; +use Symfony\Component\HttpKernel\DependencyInjection\RegisterArgumentResolversPass; /** * Bundle. @@ -88,6 +89,7 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new TranslationDumperPass()); $container->addCompilerPass(new FragmentRendererPass(), PassConfig::TYPE_AFTER_REMOVING); $container->addCompilerPass(new SerializerPass()); + $container->addCompilerPass(new RegisterArgumentResolversPass()); if ($container->getParameter('kernel.debug')) { $container->addCompilerPass(new ContainerBuilderDebugDumpPass(), PassConfig::TYPE_AFTER_REMOVING); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml index 2021505726fef..f816ce38d37e5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml @@ -25,6 +25,7 @@ + false diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml index 9b2f3cb3a4373..74e8899cb36ab 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml @@ -25,6 +25,16 @@ + + + + + + + + + + %kernel.charset% diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md index ad27886ac8738..e8d60ff309a10 100644 --- a/src/Symfony/Component/HttpKernel/CHANGELOG.md +++ b/src/Symfony/Component/HttpKernel/CHANGELOG.md @@ -1,6 +1,12 @@ CHANGELOG ========= +2.8.0 +----- + + * added argument resolvers + * deprecated `Symfony\Component\HttpKernel\Controller\ControllerResolver::getArguments()` and `doGetArguments()` in favor of argument resolvers + 2.7.0 ----- diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/ArgumentResolverInterface.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/ArgumentResolverInterface.php new file mode 100644 index 0000000000000..9fa90de09e360 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/ArgumentResolverInterface.php @@ -0,0 +1,44 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpKernel\Controller\ArgumentResolver; + +use Symfony\Component\HttpFoundation\Request; + +/** + * An ArgumentResolverInterface implementation resolves the arguments of + * controllers. + * + * @author Wouter J + */ +interface ArgumentResolverInterface +{ + /** + * Checks if the current pameter can be esolved by this argument + * resolver. + * + * @param Request $request + * @param \ReflectionParameter $parameter + * + * @return bool + */ + public function supports(Request $request, \ReflectionParameter $parameter); + + /** + * Resolves the current parameter into an argument. + * + * @param Request $request + * @param \ReflectionParameter $parameter + * + * @return mixed The resolved argument + */ + public function resolve(Request $request, \ReflectionParameter $parameter); +} diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestArgumentResolver.php new file mode 100644 index 0000000000000..f80d7b4ce31a2 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestArgumentResolver.php @@ -0,0 +1,40 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpKernel\Controller\ArgumentResolver; + +use Symfony\Component\HttpFoundation\Request; + +/** + * Resolves arguments typehinting for the HttpFoundation Request object. + * + * @author Wouter J + */ +class RequestArgumentResolver implements ArgumentResolverInterface +{ + /** + * {@inheritdoc} + */ + public function supports(Request $request, \ReflectionParameter $parameter) + { + $class = $parameter->getClass(); + + return $class && $class->isInstance($request); + } + + /** + * {@inheritdoc} + */ + public function resolve(Request $request, \ReflectionParameter $parameter) + { + return $request; + } +} diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestAttributesArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestAttributesArgumentResolver.php new file mode 100644 index 0000000000000..8f77e078ed15c --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestAttributesArgumentResolver.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpKernel\Controller\ArgumentResolver; + +use Symfony\Component\HttpFoundation\Request; + +/** + * Resolves arguments which names are equal to the name of a request attribute. + * + * @author Wouter J + */ +class RequestAttributesArgumentResolver implements ArgumentResolverInterface +{ + /** + * {@inheritdoc} + */ + public function supports(Request $request, \ReflectionParameter $parameter) + { + return $request->attributes->has($parameter->name); + } + + /** + * {@inheritdoc} + */ + public function resolve(Request $request, \ReflectionParameter $parameter) + { + return $request->attributes->get($parameter->name); + } +} diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php new file mode 100644 index 0000000000000..5c03222a12358 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php @@ -0,0 +1,83 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpKernel\Controller; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Controller\ArgumentResolver\ArgumentResolverInterface; + +/** + * The ArgumentResolverManager chains over the registered argument resolvers to + * resolve all controller arguments. + * + * @author Wouter J + */ +class ArgumentResolverManager +{ + /** + * @var ArgumentResolverInterface[] + */ + private $resolvers = array(); + + /** + * Adds an argument resolver. + * + * @param ArgumentResolverInterface $resolver + */ + public function add(ArgumentResolverInterface $resolver) + { + $this->resolvers[] = $resolver; + } + + public function getArguments(Request $request, $controller) + { + if (!is_callable($controller)) { + throw new \InvalidArgumentException(sprintf('Expected a callable as second parameter, got "%s".', is_object($controller) ? get_class($controller) : gettype($controller))); + } + + if (is_array($controller)) { + $controllerReflection = new \ReflectionMethod($controller[0], $controller[1]); + } elseif (is_object($controller) && !$controller instanceof \Closure) { + $controllerReflection = new \ReflectionObject($controller); + $controllerReflection = $controllerReflection->getMethod('__invoke'); + } else { + $controllerReflection = new \ReflectionFunction($controller); + } + + $parameters = $controllerReflection->getParameters(); + $arguments = array(); + + foreach ($parameters as $parameter) { + foreach ($this->resolvers as $argumentResolver) { + if ($argumentResolver->supports($request, $parameter)) { + $arguments[] = $argumentResolver->resolve($request, $parameter); + continue 2; + } + } + + if ($parameter->isDefaultValueAvailable()) { + $arguments[] = $parameter->getDefaultValue(); + } else { + if (is_array($controller)) { + $repr = sprintf('%s::%s()', get_class($controller[0]), $controller[1]); + } elseif (is_object($controller)) { + $repr = get_class($controller); + } else { + $repr = $controller; + } + + throw new \RuntimeException(sprintf('Controller "%s" requires that you provide a value for the "$%s" argument (because there is no default value and none of the argument resolvers could resolve its value).', $repr, $parameter->name)); + } + } + + return $arguments; + } +} diff --git a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php index 94df05eee415f..d69ead2b349a2 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php @@ -13,6 +13,8 @@ use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestArgumentResolver; +use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestAttributesArgumentResolver; /** * ControllerResolver. @@ -29,6 +31,12 @@ class ControllerResolver implements ControllerResolverInterface { private $logger; + /** + * @var ArgumentResolverManager + * @internal + */ + private $argumentResolverManager; + /** * Constructor. * @@ -39,6 +47,14 @@ public function __construct(LoggerInterface $logger = null) $this->logger = $logger; } + /** + * @internal + */ + public function setArgumentResolverManager(ArgumentResolverManager $argumentResolverManager) + { + $this->argumentResolverManager = $argumentResolverManager; + } + /** * {@inheritdoc} * @@ -102,34 +118,20 @@ public function getArguments(Request $request, $controller) $r = new \ReflectionFunction($controller); } + $reflector = new \ReflectionMethod($this, 'doGetArguments'); + if ($reflector->getDeclaringClass()->getName() !== __CLASS__) { + trigger_error('The ControllerResolverInterface::doGetArguments() method is deprecated since version 2.7 and will be removed in 3.0. Use the ArgumentResolverManager and custom ArgumentResolverInterface implementations instead.', E_USER_DEPRECATED); + } + return $this->doGetArguments($request, $controller, $r->getParameters()); } + /** + * @deprecated As of Symfony 2.8, to be removed in Symfony 3.0. Create a custom ArgumentResolverInterface implementation instead. + */ protected function doGetArguments(Request $request, $controller, array $parameters) { - $attributes = $request->attributes->all(); - $arguments = array(); - foreach ($parameters as $param) { - if (array_key_exists($param->name, $attributes)) { - $arguments[] = $attributes[$param->name]; - } elseif ($param->getClass() && $param->getClass()->isInstance($request)) { - $arguments[] = $request; - } elseif ($param->isDefaultValueAvailable()) { - $arguments[] = $param->getDefaultValue(); - } else { - if (is_array($controller)) { - $repr = sprintf('%s::%s()', get_class($controller[0]), $controller[1]); - } elseif (is_object($controller)) { - $repr = get_class($controller); - } else { - $repr = $controller; - } - - throw new \RuntimeException(sprintf('Controller "%s" requires that you provide a value for the "$%s" argument (because there is no default value or because there is a non optional argument after this one).', $repr, $param->name)); - } - } - - return $arguments; + return $this->getArgumentResolverManager()->getArguments($request, $controller); } /** @@ -167,4 +169,15 @@ protected function instantiateController($class) { return new $class(); } + + private function getArgumentResolverManager() + { + if (null === $this->argumentResolverManager) { + $this->argumentResolverManager = new ArgumentResolverManager(); + $this->argumentResolverManager->add(new RequestArgumentResolver()); + $this->argumentResolverManager->add(new RequestAttributesArgumentResolver()); + } + + return $this->argumentResolverManager; + } } diff --git a/src/Symfony/Component/HttpKernel/Controller/ControllerResolverInterface.php b/src/Symfony/Component/HttpKernel/Controller/ControllerResolverInterface.php index 6f805ed2dab77..4cf398b6ae3dd 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ControllerResolverInterface.php +++ b/src/Symfony/Component/HttpKernel/Controller/ControllerResolverInterface.php @@ -57,6 +57,8 @@ public function getController(Request $request); * * @throws \RuntimeException When value for argument given is not provided * + * @deprecated As of Symfony 2.8, to be removed in Symfony 3.0. Use the ArgumentResolverManager instead. + * * @api */ public function getArguments(Request $request, $controller); diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php b/src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php index c64c4c3939981..7b3d4b44479c9 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php @@ -13,6 +13,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpKernel\Controller\ArgumentResolverManager; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\HttpKernel; use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; @@ -35,15 +36,16 @@ class ContainerAwareHttpKernel extends HttpKernel /** * Constructor. * - * @param EventDispatcherInterface $dispatcher An EventDispatcherInterface instance - * @param ContainerInterface $container A ContainerInterface instance - * @param ControllerResolverInterface $controllerResolver A ControllerResolverInterface instance - * @param RequestStack $requestStack A stack for master/sub requests - * @param bool $triggerDeprecation Whether or not to trigger the deprecation warning for the ContainerAwareHttpKernel + * @param EventDispatcherInterface $dispatcher An EventDispatcherInterface instance + * @param ContainerInterface $container A ContainerInterface instance + * @param ControllerResolverInterface $controllerResolver A ControllerResolverInterface instance + * @param RequestStack $requestStack A stack for master/sub requests + * @param ArgumentResolverManager $argumentResolverManager A manager to resolve arguments + * @param bool $triggerDeprecation Whether or not to trigger the deprecation warning for the ContainerAwareHttpKernel */ - public function __construct(EventDispatcherInterface $dispatcher, ContainerInterface $container, ControllerResolverInterface $controllerResolver, RequestStack $requestStack = null, $triggerDeprecation = true) + public function __construct(EventDispatcherInterface $dispatcher, ContainerInterface $container, ControllerResolverInterface $controllerResolver, RequestStack $requestStack = null, ArgumentResolverManager $argumentResolverManager = null, $triggerDeprecation = true) { - parent::__construct($dispatcher, $controllerResolver, $requestStack); + parent::__construct($dispatcher, $controllerResolver, $requestStack, $argumentResolverManager); if ($triggerDeprecation) { trigger_error('The '.__CLASS__.' class is deprecated since version 2.7 and will be removed in 3.0. Use the Symfony\Component\HttpKernel\HttpKernel class instead.', E_USER_DEPRECATED); diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterArgumentResolversPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterArgumentResolversPass.php new file mode 100644 index 0000000000000..d6c11ab69e399 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterArgumentResolversPass.php @@ -0,0 +1,38 @@ + + */ +class RegisterArgumentResolversPass implements CompilerPassInterface +{ + private $managerService; + private $resolverTag; + + public function __construct($managerService = 'argument_resolver.manager', $resolverTag = 'kernel.argument_resolver') + { + $this->managerService = $managerService; + $this->resolverTag = $resolverTag; + } + + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + if (!$container->hasDefinition($this->managerService) && !$container->hasAlias($this->managerService)) { + return; + } + + $definition = $container->findDefinition($this->managerService); + + foreach ($container->findTaggedServiceIds($this->resolverTag) as $id => $resolver) { + $definition->addMethodCall('add', array(new Reference($id))); + } + } +} diff --git a/src/Symfony/Component/HttpKernel/HttpKernel.php b/src/Symfony/Component/HttpKernel/HttpKernel.php index 473f780ca3cb6..112911c25ecf2 100644 --- a/src/Symfony/Component/HttpKernel/HttpKernel.php +++ b/src/Symfony/Component/HttpKernel/HttpKernel.php @@ -11,6 +11,9 @@ namespace Symfony\Component\HttpKernel; +use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestArgumentResolver; +use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestAttributesArgumentResolver; +use Symfony\Component\HttpKernel\Controller\ArgumentResolverManager; use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; @@ -42,17 +45,28 @@ class HttpKernel implements HttpKernelInterface, TerminableInterface /** * Constructor. * - * @param EventDispatcherInterface $dispatcher An EventDispatcherInterface instance - * @param ControllerResolverInterface $resolver A ControllerResolverInterface instance - * @param RequestStack $requestStack A stack for master/sub requests + * @param EventDispatcherInterface $dispatcher An EventDispatcherInterface instance + * @param ControllerResolverInterface $resolver A ControllerResolverInterface instance + * @param RequestStack $requestStack A stack for master/sub requests + * @param ArgumentResolverManager $argumentResolverManager A manager to resolve the controller arguments * * @api */ - public function __construct(EventDispatcherInterface $dispatcher, ControllerResolverInterface $resolver, RequestStack $requestStack = null) + public function __construct(EventDispatcherInterface $dispatcher, ControllerResolverInterface $resolver, RequestStack $requestStack = null, ArgumentResolverManager $argumentResolverManager = null) { $this->dispatcher = $dispatcher; $this->resolver = $resolver; $this->requestStack = $requestStack ?: new RequestStack(); + + if (null === $argumentResolverManager) { + $argumentResolverManager = new ArgumentResolverManager(); + $argumentResolverManager->add(new RequestArgumentResolver()); + $argumentResolverManager->add(new RequestAttributesArgumentResolver()); + } + + if (method_exists($resolver, 'setArgumentResolverManager')) { + $resolver->setArgumentResolverManager($argumentResolverManager); + } } /** @@ -141,6 +155,11 @@ private function handleRaw(Request $request, $type = self::MASTER_REQUEST) $controller = $event->getController(); // controller arguments + $reflector = new \ReflectionMethod($this->resolver, 'getArguments'); + if ($reflector->getDeclaringClass()->getName() !== 'Symfony\Component\HttpKernel\Controller\ControllerResolver') { + trigger_error('['.$reflector->getDeclaringClass()->getName().'] The ControllerResolverInterface::getArguments() method is deprecated since version 2.8 and will be removed in 3.0. Use the ArgumentResolverManager and custom ArgumentResolverInterface implementations instead.', E_USER_DEPRECATED); + } + $arguments = $this->resolver->getArguments($request, $controller); // call controller diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php new file mode 100644 index 0000000000000..a4137aff9a7a7 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php @@ -0,0 +1,132 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpKernel\Tests\Controller; + +use Symfony\Component\HttpKernel\Controller\ArgumentResolverManager; + +class ArgumentResolverManagerTest extends \PHPUnit_Framework_TestCase +{ + protected $manager; + protected $resolver1; + protected $resolver2; + protected $request; + + public function setUp() + { + $this->resolver1 = $this->getMock('Symfony\Component\HttpKernel\Controller\ArgumentResolver\ArgumentResolverInterface'); + $this->resolver2 = $this->getMock('Symfony\Component\HttpKernel\Controller\ArgumentResolver\ArgumentResolverInterface'); + + $this->manager = new ArgumentResolverManager(); + $this->manager->add($this->resolver1); + $this->manager->add($this->resolver2); + + $this->request = $this->getMock('Symfony\Component\HttpFoundation\Request'); + } + + public function testGetArgumentsWithoutControllerParameters() + { + $this->assertArguments(array(), function () { }); + } + + public function testGetArgumentsFirstResolverAccepts() + { + $this->promiseResolverToMatch($this->resolver1, 'resolved_value'); + + $this->assertArguments(array('resolved_value'), $this->getControllerWithOneParameter()); + } + + public function testGetArgumentsSecondResolverAccepts() + { + $this->promiseResolverToNotMatch($this->resolver1); + $this->promiseResolverToMatch($this->resolver2, 'resolved_value'); + + $this->assertArguments(array('resolved_value'), $this->getControllerWithOneParameter()); + } + + /** + * @expectedException RuntimeException + */ + public function testGetArgumentsFailsIfNoResolverAccepts() + { + $this->promiseResolverToNotMatch($this->resolver1); + $this->promiseResolverToNotMatch($this->resolver2); + + $this->manager->getArguments($this->request, $this->getControllerWithOneParameter()); + } + + public function testGetArgumentResolvingMultipleParameters() + { + $this->resolver1->expects($this->any()) + ->method('supports') + ->will($this->onConsecutiveCalls(false, true, true)); + $this->resolver1->expects($this->any()) + ->method('resolve') + ->will($this->onConsecutiveCalls('1st resolved by 1', '2nd resolved by 1')); + + $this->resolver2->expects($this->any()) + ->method('supports') + ->will($this->onConsecutiveCalls(true, false, true)); + $this->resolver2->expects($this->any()) + ->method('resolve') + ->will($this->onConsecutiveCalls('1st resolved by 2', '2nd resolved by 2')); + + $this->assertArguments( + array('1st resolved by 2', '1st resolved by 1', '2nd resolved by 1'), + function ($a, $b, $c) { } + ); + } + + public function testControllerWithOneOptionalParameterWhichDoesNotMatch() + { + $this->promiseResolverToNotMatch($this->resolver1); + $this->promiseResolverToNotMatch($this->resolver2); + + $this->assertArguments(array('default'), function ($a = 'default') { }); + } + + public function testControllerWithOneOptionalParameterWhichDoesMatch() + { + $this->promiseResolverToMatch($this->resolver1, 'resolved by 1'); + $this->promiseResolverToNotMatch($this->resolver2); + + $this->assertArguments(array('resolved by 1'), function ($a = 'default') { }); + } + + public function testControllerWithOneParameterWithNullDefault() + { + $this->promiseResolverToNotMatch($this->resolver1); + $this->promiseResolverToNotMatch($this->resolver2); + + $this->assertArguments(array(null), function ($a = null) { }); + } + + private function assertArguments(array $expected, $controller) + { + $this->assertEquals($expected, $this->manager->getArguments($this->request, $controller)); + } + + private function promiseResolverToMatch($resolver, $return) + { + $resolver->expects($this->any())->method('supports')->will($this->returnValue(true)); + $resolver->expects($this->any())->method('resolve')->will($this->returnValue($return)); + } + + private function promiseResolverToNotMatch($resolver) + { + $resolver->expects($this->any())->method('supports')->will($this->returnValue(false)); + } + + private function getControllerWithOneParameter() + { + return function ($a) { }; + } +} diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php index 32db4e47adc5a..ee5b7a8dc90e7 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php @@ -220,19 +220,19 @@ public function controllerMethod1($foo) { } - protected function controllerMethod2($foo, $bar = null) + public function controllerMethod2($foo, $bar = null) { } - protected function controllerMethod3($foo, $bar = null, $foobar) + public function controllerMethod3($foo, $bar = null, $foobar) { } - protected static function controllerMethod4() + public static function controllerMethod4() { } - protected function controllerMethod5(Request $request) + public function controllerMethod5(Request $request) { } } diff --git a/src/Symfony/Component/HttpKernel/Tests/Debug/TraceableEventDispatcherTest.php b/src/Symfony/Component/HttpKernel/Tests/Debug/TraceableEventDispatcherTest.php index f64d7247c0b48..bcf7449de96ab 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Debug/TraceableEventDispatcherTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Debug/TraceableEventDispatcherTest.php @@ -20,6 +20,11 @@ class TraceableEventDispatcherTest extends \PHPUnit_Framework_TestCase { + /** + * Tagged as legacy because of the usage of the deprecated ControllerResolver#getArguments(). + * + * @group legacy + */ public function testStopwatchSections() { $dispatcher = new TraceableEventDispatcher(new EventDispatcher(), $stopwatch = new Stopwatch()); @@ -39,6 +44,11 @@ public function testStopwatchSections() ), array_keys($events)); } + /** + * Tagged as legacy because of the usage of the deprecated ControllerResolver#getArguments(). + * + * @group legacy + */ public function testStopwatchCheckControllerOnRequestEvent() { $stopwatch = $this->getMockBuilder('Symfony\Component\Stopwatch\Stopwatch') @@ -55,6 +65,11 @@ public function testStopwatchCheckControllerOnRequestEvent() $kernel->handle($request); } + /** + * Tagged as legacy because of the usage of the deprecated ControllerResolver#getArguments(). + * + * @group legacy + */ public function testStopwatchStopControllerOnRequestEvent() { $stopwatch = $this->getMockBuilder('Symfony\Component\Stopwatch\Stopwatch') diff --git a/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php b/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php index 4e487a478a600..bd2ef30a0da7d 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php @@ -140,6 +140,11 @@ private function getKernelExpectingRequest(Request $request) return $kernel; } + /** + * Tagged as legacy because of the usage of the deprecated ControllerResolver#getArguments(). + * + * @group legacy + */ public function testExceptionInSubRequestsDoesNotMangleOutputBuffers() { $resolver = $this->getMock('Symfony\\Component\\HttpKernel\\Controller\\ControllerResolverInterface'); diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php index 5546ba2ed830e..755a32ffa50f0 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php @@ -17,8 +17,10 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\HttpKernel\Tests\TestArgumentResolverManager; +use Symfony\Component\HttpKernel\Tests\TestControllerResolver; -class TestHttpKernel extends HttpKernel implements ControllerResolverInterface +class TestHttpKernel extends HttpKernel { protected $body; protected $status; @@ -35,7 +37,7 @@ public function __construct($body, $status, $headers, \Closure $customizer = nul $this->headers = $headers; $this->customizer = $customizer; - parent::__construct(new EventDispatcher(), $this); + parent::__construct(new EventDispatcher(), new TestControllerResolver($this), null, new TestArgumentResolverManager()); } public function getBackendRequest() @@ -56,16 +58,6 @@ public function isCatchingExceptions() return $this->catch; } - public function getController(Request $request) - { - return array($this, 'callController'); - } - - public function getArguments(Request $request, $controller) - { - return array($request); - } - public function callController(Request $request) { $this->called = true; diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php index 773e221482bed..25d1bba99e26e 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php @@ -17,8 +17,10 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\HttpKernel\Tests\TestArgumentResolverManager; +use Symfony\Component\HttpKernel\Tests\TestControllerResolver; -class TestMultipleHttpKernel extends HttpKernel implements ControllerResolverInterface +class TestMultipleHttpKernel extends HttpKernel { protected $bodies = array(); protected $statuses = array(); @@ -34,7 +36,7 @@ public function __construct($responses) $this->headers[] = $response['headers']; } - parent::__construct(new EventDispatcher(), $this); + parent::__construct(new EventDispatcher(), new TestControllerResolver($this), null, new TestArgumentResolverManager()); } public function getBackendRequest() @@ -49,16 +51,6 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ return parent::handle($request, $type, $catch); } - public function getController(Request $request) - { - return array($this, 'callController'); - } - - public function getArguments(Request $request, $controller) - { - return array($request); - } - public function callController(Request $request) { $this->called = true; diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php index 97e68f66f979a..573d670574dc2 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php @@ -21,6 +21,11 @@ use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\EventDispatcher\EventDispatcher; +/** + * Tagged as legacy because of the usage of the deprecated ControllerResolver#getArguments(). + * + * @group legacy + */ class HttpKernelTest extends \PHPUnit_Framework_TestCase { /** diff --git a/src/Symfony/Component/HttpKernel/Tests/TestArgumentResolverManager.php b/src/Symfony/Component/HttpKernel/Tests/TestArgumentResolverManager.php new file mode 100644 index 0000000000000..093548cd5d979 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/TestArgumentResolverManager.php @@ -0,0 +1,26 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpKernel\Tests; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Controller\ArgumentResolverManager; + +/** + * @author Wouter J + */ +class TestArgumentResolverManager extends ArgumentResolverManager +{ + public function getArguments(Request $request, $controller) + { + return array($request); + } +} diff --git a/src/Symfony/Component/HttpKernel/Tests/TestControllerResolver.php b/src/Symfony/Component/HttpKernel/Tests/TestControllerResolver.php new file mode 100644 index 0000000000000..828b3ca36f2a7 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/TestControllerResolver.php @@ -0,0 +1,26 @@ + + */ +class TestControllerResolver extends ControllerResolver +{ + private $controller; + + public function __construct($controller, LoggerInterface $logger = null) + { + parent::__construct($logger); + $this->controller = $controller; + } + + public function getController(Request $request) + { + return array($this->controller, 'callController'); + } +} diff --git a/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php b/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php index d526c4de80c36..0136de1911659 100644 --- a/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php +++ b/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php @@ -17,21 +17,11 @@ use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; use Symfony\Component\EventDispatcher\EventDispatcher; -class TestHttpKernel extends HttpKernel implements ControllerResolverInterface +class TestHttpKernel extends HttpKernel { public function __construct() { - parent::__construct(new EventDispatcher(), $this); - } - - public function getController(Request $request) - { - return array($this, 'callController'); - } - - public function getArguments(Request $request, $controller) - { - return array($request); + parent::__construct(new EventDispatcher(), new TestControllerResolver($this), null, new TestArgumentResolverManager()); } public function callController(Request $request) From 3dfdb61286253aebf089d34725f7ed8f11d44956 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Wed, 10 Jun 2015 09:53:11 +0200 Subject: [PATCH 2/9] Added PsrServerRequest argument resolver --- .../FrameworkExtension.php | 4 ++ .../Resources/config/psr-http-message.xml | 14 +++++ .../PsrServerRequestArgumentResolver.php | 52 +++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Resources/config/psr-http-message.xml create mode 100644 src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/PsrServerRequestArgumentResolver.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index e2e8a84163859..7685c6472aeb5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -153,6 +153,10 @@ public function load(array $configs, ContainerBuilder $container) $definition->replaceArgument(2, E_COMPILE_ERROR | E_PARSE | E_ERROR | E_CORE_ERROR); } + if (interface_exists('Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface')) { + $loader->load('psr-http-message.xml'); + } + $this->addClassesToCompile(array( 'Symfony\\Component\\Config\\FileLocator', diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/psr-http-message.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/psr-http-message.xml new file mode 100644 index 0000000000000..ffb2eab4f7e77 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/psr-http-message.xml @@ -0,0 +1,14 @@ + + + + + + + + + + + + diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/PsrServerRequestArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/PsrServerRequestArgumentResolver.php new file mode 100644 index 0000000000000..4a25179af0374 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/PsrServerRequestArgumentResolver.php @@ -0,0 +1,52 @@ + + * @author Wouter J + */ +class PsrServerRequestArgumentResolver implements ArgumentResolverInterface +{ + /** + * @var array + */ + private static $supportedTypes = array( + 'Psr\Http\Message\ServerRequestInterface' => true, + 'Psr\Http\Message\RequestInterface' => true, + 'Psr\Http\Message\MessageInterface' => true, + ); + + /** + * @var HttpMessageFactoryInterface + */ + private $httpMessageFactory; + + public function __construct(HttpMessageFactoryInterface $httpMessageFactory) + { + $this->httpMessageFactory = $httpMessageFactory; + } + + /** + * {@inheritdoc} + */ + public function supports(Request $request, \ReflectionParameter $parameter) + { + $class = $parameter->getClass(); + + return null !== $class && isset(self::$supportedTypes[$class->name]); + } + + /** + * {@inheritdoc} + */ + public function resolve(Request $request, \ReflectionParameter $parameter) + { + return $this->httpMessageFactory->createRequest($request); + } +} From 434f92f1f29b01b777078903e2c3b5cf74193612 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Thu, 11 Jun 2015 17:28:15 +0200 Subject: [PATCH 3/9] Added User Argument Resolver --- .../ArgumentResolver/UserArgumentResolver.php | 51 +++++++++++++++++++ .../Resources/config/security.xml | 5 ++ 2 files changed, 56 insertions(+) create mode 100644 src/Symfony/Bundle/SecurityBundle/ArgumentResolver/UserArgumentResolver.php diff --git a/src/Symfony/Bundle/SecurityBundle/ArgumentResolver/UserArgumentResolver.php b/src/Symfony/Bundle/SecurityBundle/ArgumentResolver/UserArgumentResolver.php new file mode 100644 index 0000000000000..951a693a36bf4 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/ArgumentResolver/UserArgumentResolver.php @@ -0,0 +1,51 @@ + + */ +class UserArgumentResolver implements ArgumentResolverInterface +{ + /** + * @var TokenStorageInterface + */ + private $tokenStorage; + + public function __construct(TokenStorageInterface $tokenStorage) + { + $this->tokenStorage = $tokenStorage; + } + + /** + * {@inheritdoc} + */ + public function supports(Request $request, \ReflectionParameter $parameter) + { + $class = $parameter->getClass(); + $userInterface = 'Symfony\Component\Security\Core\User\UserInterface'; + + return null !== $class && ($userInterface === $class || $class->implementsInterface($userInterface)); + } + + /** + * {@inheritdoc} + */ + public function resolve(Request $request, \ReflectionParameter $parameter) + { + if (null === $token = $this->tokenStorage->getToken()) { + return; + } + + if (!is_object($user = $token->getUser())) { + // e.g. anonymous authentication + return; + } + + return $user; + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml index b7c1407c1cc56..11ef49722535a 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml @@ -50,6 +50,11 @@ + + + + + From a1d66c60b055347a2ccc6d30f6d5531fc5f64696 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Sun, 14 Jun 2015 00:19:11 +0200 Subject: [PATCH 4/9] Review --- .../SecurityBundle/Resources/config/security.xml | 2 +- .../HttpKernel/Controller/ArgumentResolverManager.php | 10 +++++++++- .../HttpKernel/Controller/ControllerResolver.php | 10 +++++----- src/Symfony/Component/HttpKernel/HttpKernel.php | 9 +++++---- .../Tests/Controller/ArgumentResolverManagerTest.php | 4 +--- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml index 11ef49722535a..d069daac1cf59 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml @@ -51,7 +51,7 @@ - + diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php index 5c03222a12358..38dfe02ace317 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php @@ -25,7 +25,15 @@ class ArgumentResolverManager /** * @var ArgumentResolverInterface[] */ - private $resolvers = array(); + private $resolvers; + + /** + * @param ArgumentResolverInterface[] $resolvers + */ + public function __construct(array $resolvers = array()) + { + $this->resolvers = $resolvers; + } /** * Adds an argument resolver. diff --git a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php index d69ead2b349a2..684cd3291df84 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php @@ -33,7 +33,6 @@ class ControllerResolver implements ControllerResolverInterface /** * @var ArgumentResolverManager - * @internal */ private $argumentResolverManager; @@ -120,7 +119,7 @@ public function getArguments(Request $request, $controller) $reflector = new \ReflectionMethod($this, 'doGetArguments'); if ($reflector->getDeclaringClass()->getName() !== __CLASS__) { - trigger_error('The ControllerResolverInterface::doGetArguments() method is deprecated since version 2.7 and will be removed in 3.0. Use the ArgumentResolverManager and custom ArgumentResolverInterface implementations instead.', E_USER_DEPRECATED); + @trigger_error('The ControllerResolverInterface::doGetArguments() method is deprecated since version 2.8 and will be removed in 3.0. Use the ArgumentResolverManager and custom ArgumentResolverInterface implementations instead.', E_USER_DEPRECATED); } return $this->doGetArguments($request, $controller, $r->getParameters()); @@ -173,9 +172,10 @@ protected function instantiateController($class) private function getArgumentResolverManager() { if (null === $this->argumentResolverManager) { - $this->argumentResolverManager = new ArgumentResolverManager(); - $this->argumentResolverManager->add(new RequestArgumentResolver()); - $this->argumentResolverManager->add(new RequestAttributesArgumentResolver()); + $this->argumentResolverManager = new ArgumentResolverManager(array( + new RequestArgumentResolver(), + new RequestAttributesArgumentResolver(), + )); } return $this->argumentResolverManager; diff --git a/src/Symfony/Component/HttpKernel/HttpKernel.php b/src/Symfony/Component/HttpKernel/HttpKernel.php index 112911c25ecf2..3332f77fc5f52 100644 --- a/src/Symfony/Component/HttpKernel/HttpKernel.php +++ b/src/Symfony/Component/HttpKernel/HttpKernel.php @@ -59,9 +59,10 @@ public function __construct(EventDispatcherInterface $dispatcher, ControllerReso $this->requestStack = $requestStack ?: new RequestStack(); if (null === $argumentResolverManager) { - $argumentResolverManager = new ArgumentResolverManager(); - $argumentResolverManager->add(new RequestArgumentResolver()); - $argumentResolverManager->add(new RequestAttributesArgumentResolver()); + $argumentResolverManager = new ArgumentResolverManager(array( + new RequestArgumentResolver(), + new RequestAttributesArgumentResolver(), + )); } if (method_exists($resolver, 'setArgumentResolverManager')) { @@ -157,7 +158,7 @@ private function handleRaw(Request $request, $type = self::MASTER_REQUEST) // controller arguments $reflector = new \ReflectionMethod($this->resolver, 'getArguments'); if ($reflector->getDeclaringClass()->getName() !== 'Symfony\Component\HttpKernel\Controller\ControllerResolver') { - trigger_error('['.$reflector->getDeclaringClass()->getName().'] The ControllerResolverInterface::getArguments() method is deprecated since version 2.8 and will be removed in 3.0. Use the ArgumentResolverManager and custom ArgumentResolverInterface implementations instead.', E_USER_DEPRECATED); + @trigger_error('The ControllerResolverInterface::getArguments() method is deprecated since version 2.8 and will be removed in 3.0. Use the ArgumentResolverManager and custom ArgumentResolverInterface implementations instead.', E_USER_DEPRECATED); } $arguments = $this->resolver->getArguments($request, $controller); diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php index a4137aff9a7a7..fd0f285aa89dd 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php @@ -25,9 +25,7 @@ public function setUp() $this->resolver1 = $this->getMock('Symfony\Component\HttpKernel\Controller\ArgumentResolver\ArgumentResolverInterface'); $this->resolver2 = $this->getMock('Symfony\Component\HttpKernel\Controller\ArgumentResolver\ArgumentResolverInterface'); - $this->manager = new ArgumentResolverManager(); - $this->manager->add($this->resolver1); - $this->manager->add($this->resolver2); + $this->manager = new ArgumentResolverManager(array($this->resolver1, $this->resolver2)); $this->request = $this->getMock('Symfony\Component\HttpFoundation\Request'); } From cf69cc7faff626288b16289f7085a5c3c1aa1464 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Sun, 14 Jun 2015 00:24:25 +0200 Subject: [PATCH 5/9] Fabbot --- .../Component/HttpKernel/Controller/ControllerResolver.php | 2 +- .../Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php | 1 - .../HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php | 1 - src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php index 684cd3291df84..f1dfdcfc21c8f 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php @@ -158,7 +158,7 @@ protected function createController($controller) } /** - * Returns an instantiated controller + * Returns an instantiated controller. * * @param string $class A class name * diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php index 755a32ffa50f0..a3b62b34272df 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php @@ -15,7 +15,6 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\HttpKernel\Tests\TestArgumentResolverManager; use Symfony\Component\HttpKernel\Tests\TestControllerResolver; diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php index 25d1bba99e26e..19de6269aaae2 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php @@ -15,7 +15,6 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\HttpKernel\Tests\TestArgumentResolverManager; use Symfony\Component\HttpKernel\Tests\TestControllerResolver; diff --git a/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php b/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php index 0136de1911659..6330505102160 100644 --- a/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php +++ b/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php @@ -14,7 +14,6 @@ use Symfony\Component\HttpKernel\HttpKernel; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; use Symfony\Component\EventDispatcher\EventDispatcher; class TestHttpKernel extends HttpKernel From 3a1d99cf2e52604c75cf04f9e5796af36b69666e Mon Sep 17 00:00:00 2001 From: WouterJ Date: Sun, 14 Jun 2015 00:31:04 +0200 Subject: [PATCH 6/9] Fix tests --- .../Bundle/SecurityBundle/Resources/config/security.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml index d069daac1cf59..ad08b4bd05726 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml @@ -51,7 +51,8 @@ - + + From 1741b2ad050dbcdb99131a158fe542535739be5a Mon Sep 17 00:00:00 2001 From: WouterJ Date: Sun, 14 Jun 2015 13:58:30 +0200 Subject: [PATCH 7/9] Fix deprecation trigger with TraceableControllerResolver --- src/Symfony/Component/HttpKernel/HttpKernel.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/HttpKernel/HttpKernel.php b/src/Symfony/Component/HttpKernel/HttpKernel.php index 3332f77fc5f52..d63259927305e 100644 --- a/src/Symfony/Component/HttpKernel/HttpKernel.php +++ b/src/Symfony/Component/HttpKernel/HttpKernel.php @@ -157,7 +157,7 @@ private function handleRaw(Request $request, $type = self::MASTER_REQUEST) // controller arguments $reflector = new \ReflectionMethod($this->resolver, 'getArguments'); - if ($reflector->getDeclaringClass()->getName() !== 'Symfony\Component\HttpKernel\Controller\ControllerResolver') { + if (0 !== strpos($reflector->getDeclaringClass()->getName(), 'Symfony\Component\HttpKernel\Controller\\')) { @trigger_error('The ControllerResolverInterface::getArguments() method is deprecated since version 2.8 and will be removed in 3.0. Use the ArgumentResolverManager and custom ArgumentResolverInterface implementations instead.', E_USER_DEPRECATED); } From 4924e8683599cdc3a76851dac58ac833f7dfe672 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Sun, 14 Jun 2015 15:04:28 +0200 Subject: [PATCH 8/9] Add priority --- .../Controller/ArgumentResolverManager.php | 35 +++++++++++++------ .../Controller/ControllerResolver.php | 7 ++-- .../Component/HttpKernel/HttpKernel.php | 7 ++-- .../ArgumentResolverManagerTest.php | 18 +++++++++- 4 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php index 38dfe02ace317..62690adc32b50 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php @@ -23,26 +23,30 @@ class ArgumentResolverManager { /** - * @var ArgumentResolverInterface[] + * @var array */ - private $resolvers; + private $resolvers = array(); /** - * @param ArgumentResolverInterface[] $resolvers + * @var null|ArgumentResolverInterface[] */ - public function __construct(array $resolvers = array()) - { - $this->resolvers = $resolvers; - } + private $sortedResolvers; /** * Adds an argument resolver. * * @param ArgumentResolverInterface $resolver */ - public function add(ArgumentResolverInterface $resolver) + public function add(ArgumentResolverInterface $resolver, $priority = 0) { - $this->resolvers[] = $resolver; + if (!isset($this->resolvers[$priority])) { + $this->resolvers[$priority] = array(); + } + + $this->resolvers[$priority][] = $resolver; + + // force a new sort of the resolvers + $this->sortedResolvers = null; } public function getArguments(Request $request, $controller) @@ -64,7 +68,7 @@ public function getArguments(Request $request, $controller) $arguments = array(); foreach ($parameters as $parameter) { - foreach ($this->resolvers as $argumentResolver) { + foreach ($this->getSortedResolvers() as $argumentResolver) { if ($argumentResolver->supports($request, $parameter)) { $arguments[] = $argumentResolver->resolve($request, $parameter); continue 2; @@ -88,4 +92,15 @@ public function getArguments(Request $request, $controller) return $arguments; } + + private function getSortedResolvers() + { + if (null === $this->sortedResolvers) { + $this->sortedResolvers = $this->resolvers; + ksort($this->sortedResolvers); + $this->sortedResolvers = call_user_func_array('array_merge', $this->sortedResolvers); + } + + return $this->sortedResolvers; + } } diff --git a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php index f1dfdcfc21c8f..2a62f619c93f2 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php @@ -172,10 +172,9 @@ protected function instantiateController($class) private function getArgumentResolverManager() { if (null === $this->argumentResolverManager) { - $this->argumentResolverManager = new ArgumentResolverManager(array( - new RequestArgumentResolver(), - new RequestAttributesArgumentResolver(), - )); + $this->argumentResolverManager = new ArgumentResolverManager(); + $this->argumentResolverManager->add(new RequestArgumentResolver()); + $this->argumentResolverManager->add(new RequestAttributesArgumentResolver()); } return $this->argumentResolverManager; diff --git a/src/Symfony/Component/HttpKernel/HttpKernel.php b/src/Symfony/Component/HttpKernel/HttpKernel.php index d63259927305e..977e02fd9deb8 100644 --- a/src/Symfony/Component/HttpKernel/HttpKernel.php +++ b/src/Symfony/Component/HttpKernel/HttpKernel.php @@ -59,10 +59,9 @@ public function __construct(EventDispatcherInterface $dispatcher, ControllerReso $this->requestStack = $requestStack ?: new RequestStack(); if (null === $argumentResolverManager) { - $argumentResolverManager = new ArgumentResolverManager(array( - new RequestArgumentResolver(), - new RequestAttributesArgumentResolver(), - )); + $argumentResolverManager = new ArgumentResolverManager(); + $argumentResolverManager->add(new RequestArgumentResolver()); + $argumentResolverManager->add(new RequestAttributesArgumentResolver()); } if (method_exists($resolver, 'setArgumentResolverManager')) { diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php index fd0f285aa89dd..c0a3bc71c419d 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php @@ -25,7 +25,9 @@ public function setUp() $this->resolver1 = $this->getMock('Symfony\Component\HttpKernel\Controller\ArgumentResolver\ArgumentResolverInterface'); $this->resolver2 = $this->getMock('Symfony\Component\HttpKernel\Controller\ArgumentResolver\ArgumentResolverInterface'); - $this->manager = new ArgumentResolverManager(array($this->resolver1, $this->resolver2)); + $this->manager = new ArgumentResolverManager(); + $this->manager->add($this->resolver1); + $this->manager->add($this->resolver2); $this->request = $this->getMock('Symfony\Component\HttpFoundation\Request'); } @@ -107,6 +109,20 @@ public function testControllerWithOneParameterWithNullDefault() $this->assertArguments(array(null), function ($a = null) { }); } + public function testPriority() + { + $this->promiseResolverToNotMatch($this->resolver1); + $this->resolver1->expects($this->never())->method('resolve'); + + $this->promiseResolverToMatch($this->resolver2, 'resolved by 2'); + + $this->manager = new ArgumentResolverManager(); + $this->manager->add($this->resolver1); + $this->manager->add($this->resolver2, 100); + + $this->assertArguments(array('resolved by 2'), function ($a) { }); + } + private function assertArguments(array $expected, $controller) { $this->assertEquals($expected, $this->manager->getArguments($this->request, $controller)); From 1300ed23be10edb9d7febea74f7a6ae57a14ec22 Mon Sep 17 00:00:00 2001 From: WouterJ Date: Tue, 30 Jun 2015 23:19:31 +0200 Subject: [PATCH 9/9] Lots of fixes thanks to review --- .../ArgumentResolver/UserArgumentResolver.php | 14 ++- .../UserArgumentResolverTest.php | 96 +++++++++++++++++++ .../PsrServerRequestArgumentResolver.php | 2 +- .../RequestAttributesArgumentResolver.php | 4 +- .../Controller/ArgumentResolverManager.php | 48 +++++----- .../Controller/ControllerResolver.php | 7 +- .../RegisterArgumentResolversPass.php | 11 ++- .../Component/HttpKernel/HttpKernel.php | 7 +- .../PsrServerRequestArgumentResolverTest.php | 67 +++++++++++++ .../RequestArgumentResolverTest.php | 63 ++++++++++++ .../RequestAttributesArgumentResolverTest.php | 60 ++++++++++++ .../ArgumentResolverManagerTest.php | 29 ++---- 12 files changed, 349 insertions(+), 59 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/ArgumentResolver/UserArgumentResolverTest.php create mode 100644 src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/PsrServerRequestArgumentResolverTest.php create mode 100644 src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestArgumentResolverTest.php create mode 100644 src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestAttributesArgumentResolverTest.php diff --git a/src/Symfony/Bundle/SecurityBundle/ArgumentResolver/UserArgumentResolver.php b/src/Symfony/Bundle/SecurityBundle/ArgumentResolver/UserArgumentResolver.php index 951a693a36bf4..6c0c885020d07 100644 --- a/src/Symfony/Bundle/SecurityBundle/ArgumentResolver/UserArgumentResolver.php +++ b/src/Symfony/Bundle/SecurityBundle/ArgumentResolver/UserArgumentResolver.php @@ -1,5 +1,14 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Symfony\Bundle\SecurityBundle\ArgumentResolver; use Symfony\Component\HttpFoundation\Request; @@ -7,6 +16,8 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; /** + * Resolves a typehint for UserInterface in a controller to the current user. + * * @author Wouter J */ class UserArgumentResolver implements ArgumentResolverInterface @@ -28,8 +39,9 @@ public function supports(Request $request, \ReflectionParameter $parameter) { $class = $parameter->getClass(); $userInterface = 'Symfony\Component\Security\Core\User\UserInterface'; + $userImplementation = 'Symfony\Component\Security\Core\User\User'; - return null !== $class && ($userInterface === $class || $class->implementsInterface($userInterface)); + return null !== $class && ($userInterface === $class->getName() || $userImplementation === $class->getName()); } /** diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/ArgumentResolver/UserArgumentResolverTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/ArgumentResolver/UserArgumentResolverTest.php new file mode 100644 index 0000000000000..608065ef101eb --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/ArgumentResolver/UserArgumentResolverTest.php @@ -0,0 +1,96 @@ + + */ +class UserArgumentResolverTest extends \PHPUnit_Framework_TestCase +{ + private $resolver; + private $tokenStorage; + + protected function setUp() + { + $this->tokenStorage = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface'); + $this->resolver = new UserArgumentResolver($this->tokenStorage); + } + + /** + * @dataProvider provideClasses + */ + public function testSupports($class, $supported = true) + { + $this->assertEquals($supported, $this->resolver->supports($this->getRequestMock(), $this->getReflectionParameterMock($class))); + } + + public function provideClasses() + { + return array( + array('Symfony\Component\Security\Core\User\UserInterface'), + array('Symfony\Component\Security\Core\User\User'), + array('Symfony\Bundle\SecurityBundle\Tests\ArgumentResolver\UserFixture', false), + array('\stdClass', false), + ); + } + + public function testResolvesToNullWhenNoUserIsAvailable() + { + $this->tokenStorage->expects($this->any())->method('getToken')->willReturn(null); + + $this->assertNull($this->resolver->resolve($this->getRequestMock(), $this->getReflectionParameterMock())); + } + + public function testResolvesToNullWhenUserIsAnonymous() + { + $token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface'); + $token->expects($this->any())->method('getUser')->willReturn('anon.'); + + $this->tokenStorage->expects($this->any())->method('getToken')->willReturn($token); + + $this->assertNull($this->resolver->resolve($this->getRequestMock(), $this->getReflectionParameterMock())); + } + + public function testResolvesToUser() + { + $user = $this->getMock('Symfony\component\Security\Core\User\User'); + + $token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface'); + $token->expects($this->any())->method('getUser')->willReturn($user); + + $this->tokenStorage->expects($this->any())->method('getToken')->willReturn($token); + + $this->assertEquals($user, $this->resolver->resolve($this->getRequestMock(), $this->getReflectionParameterMock())); + } + + private function getRequestMock() + { + return $this->getMock('Symfony\Component\HttpFoundation\Request'); + } + + private function getReflectionParameterMock($class = null) + { + $reflectionParameter = $this->getMockBuilder('\ReflectionParameter')->disableOriginalConstructor()->getMock(); + + if (null !== $class) { + $reflectionClass = $this->getMockBuilder('\ReflectionClass')->disableOriginalConstructor()->getMock(); + $reflectionClass->expects($this->any())->method('getName')->willReturn($class); + $reflectionParameter->expects($this->any())->method('getClass')->willReturn($reflectionClass); + } + + return $reflectionParameter; + } +} + +class UserFixture implements UserInterface +{ + public function getRoles() {} + public function getPassword() {} + public function getSalt() {} + public function getUsername() {} + public function eraseCredentials() {} +} diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/PsrServerRequestArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/PsrServerRequestArgumentResolver.php index 4a25179af0374..35233a08422b6 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/PsrServerRequestArgumentResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/PsrServerRequestArgumentResolver.php @@ -39,7 +39,7 @@ public function supports(Request $request, \ReflectionParameter $parameter) { $class = $parameter->getClass(); - return null !== $class && isset(self::$supportedTypes[$class->name]); + return null !== $class && isset(self::$supportedTypes[$class->getName()]); } /** diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestAttributesArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestAttributesArgumentResolver.php index 8f77e078ed15c..e6c23ceec5d7c 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestAttributesArgumentResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestAttributesArgumentResolver.php @@ -25,7 +25,7 @@ class RequestAttributesArgumentResolver implements ArgumentResolverInterface */ public function supports(Request $request, \ReflectionParameter $parameter) { - return $request->attributes->has($parameter->name); + return $request->attributes->has($parameter->getName()); } /** @@ -33,6 +33,6 @@ public function supports(Request $request, \ReflectionParameter $parameter) */ public function resolve(Request $request, \ReflectionParameter $parameter) { - return $request->attributes->get($parameter->name); + return $request->attributes->get($parameter->getName()); } } diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php index 62690adc32b50..46bfb003da7ae 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolverManager.php @@ -23,32 +23,39 @@ class ArgumentResolverManager { /** - * @var array + * @var ArgumentResolverInterface[] */ - private $resolvers = array(); + private $resolvers; /** - * @var null|ArgumentResolverInterface[] + * @param ArgumentResolverInterface[] $resolvers */ - private $sortedResolvers; + public function __construct(array $resolvers = array()) + { + $this->resolvers = $resolvers; + } /** - * Adds an argument resolver. + * Adds an argument resolver to the chain. + * + * The order in which the resolvers will be chained is equal + * to the order in which this method is called. * * @param ArgumentResolverInterface $resolver */ - public function add(ArgumentResolverInterface $resolver, $priority = 0) + public function add(ArgumentResolverInterface $resolver) { - if (!isset($this->resolvers[$priority])) { - $this->resolvers[$priority] = array(); - } - - $this->resolvers[$priority][] = $resolver; - - // force a new sort of the resolvers - $this->sortedResolvers = null; + $this->resolvers[] = $resolver; } + /** + * Resolves the constructor arguments of the passed controller. + * + * @param Request $request + * @param callable $controller + * + * @return array + */ public function getArguments(Request $request, $controller) { if (!is_callable($controller)) { @@ -68,7 +75,7 @@ public function getArguments(Request $request, $controller) $arguments = array(); foreach ($parameters as $parameter) { - foreach ($this->getSortedResolvers() as $argumentResolver) { + foreach ($this->resolvers as $argumentResolver) { if ($argumentResolver->supports($request, $parameter)) { $arguments[] = $argumentResolver->resolve($request, $parameter); continue 2; @@ -92,15 +99,4 @@ public function getArguments(Request $request, $controller) return $arguments; } - - private function getSortedResolvers() - { - if (null === $this->sortedResolvers) { - $this->sortedResolvers = $this->resolvers; - ksort($this->sortedResolvers); - $this->sortedResolvers = call_user_func_array('array_merge', $this->sortedResolvers); - } - - return $this->sortedResolvers; - } } diff --git a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php index 2a62f619c93f2..f1dfdcfc21c8f 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php @@ -172,9 +172,10 @@ protected function instantiateController($class) private function getArgumentResolverManager() { if (null === $this->argumentResolverManager) { - $this->argumentResolverManager = new ArgumentResolverManager(); - $this->argumentResolverManager->add(new RequestArgumentResolver()); - $this->argumentResolverManager->add(new RequestAttributesArgumentResolver()); + $this->argumentResolverManager = new ArgumentResolverManager(array( + new RequestArgumentResolver(), + new RequestAttributesArgumentResolver(), + )); } return $this->argumentResolverManager; diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterArgumentResolversPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterArgumentResolversPass.php index d6c11ab69e399..21c61f1829be9 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterArgumentResolversPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterArgumentResolversPass.php @@ -30,9 +30,16 @@ public function process(ContainerBuilder $container) } $definition = $container->findDefinition($this->managerService); + $resolvers = array(); - foreach ($container->findTaggedServiceIds($this->resolverTag) as $id => $resolver) { - $definition->addMethodCall('add', array(new Reference($id))); + foreach ($container->findTaggedServiceIds($this->resolverTag) as $id => $tags) { + $priority = isset($tags[0]['priority']) ? $tags[0]['priority'] : 0; + $resolvers[$priority][] = new Reference($id); } + + ksort($resolvers); + $resolvers = call_user_func_array('array_merge', $resolvers); + + $definition->replaceArgument(1, $resolvers); } } diff --git a/src/Symfony/Component/HttpKernel/HttpKernel.php b/src/Symfony/Component/HttpKernel/HttpKernel.php index 977e02fd9deb8..d63259927305e 100644 --- a/src/Symfony/Component/HttpKernel/HttpKernel.php +++ b/src/Symfony/Component/HttpKernel/HttpKernel.php @@ -59,9 +59,10 @@ public function __construct(EventDispatcherInterface $dispatcher, ControllerReso $this->requestStack = $requestStack ?: new RequestStack(); if (null === $argumentResolverManager) { - $argumentResolverManager = new ArgumentResolverManager(); - $argumentResolverManager->add(new RequestArgumentResolver()); - $argumentResolverManager->add(new RequestAttributesArgumentResolver()); + $argumentResolverManager = new ArgumentResolverManager(array( + new RequestArgumentResolver(), + new RequestAttributesArgumentResolver(), + )); } if (method_exists($resolver, 'setArgumentResolverManager')) { diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/PsrServerRequestArgumentResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/PsrServerRequestArgumentResolverTest.php new file mode 100644 index 0000000000000..49b53abe1aa57 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/PsrServerRequestArgumentResolverTest.php @@ -0,0 +1,67 @@ + + */ +class PsrServerRequestArgumentResolverTest extends \PHPUnit_Framework_TestCase +{ + private $httpMessageFactory; + private $resolver; + + protected function setUp() + { + $this->httpMessageFactory = $this->getMock('Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface', array('createRequest')); + $this->resolver = new PsrServerRequestArgumentResolver($this->httpMessageFactory); + } + + /** + * @dataProvider provideClasses + */ + public function testSupports($class, $supported = true) + { + $this->assertEquals($supported, $this->resolver->supports($this->getRequestMock(), $this->getReflectionParameterMock($class))); + } + + public function provideClasses() + { + return array( + array('Psr\Http\Message\ServerRequestInterface'), + array('Psr\Http\Message\RequestInterface'), + array('Psr\Http\Message\MessageInterface'), + array('\stdClass', false), + array('Symfony\Component\HttpFoundation\Request', false), + ); + } + + public function testResolvesUsingHttpMessageFactory() + { + $request = $this->getRequestMock(); + + $this->httpMessageFactory->expects($this->once())->method('createRequest')->with($request); + + $this->resolver->resolve($request, $this->getReflectionParameterMock()); + } + + private function getRequestMock() + { + return $this->getMock('Symfony\Component\HttpFoundation\Request'); + } + + private function getReflectionParameterMock($class = null) + { + $reflectionParameter = $this->getMockBuilder('ReflectionParameter')->disableOriginalConstructor()->getMock(); + + if (null !== $class) { + $reflectionClass = $this->getMockBuilder('ReflectionClass')->disableOriginalConstructor()->getMock(); + $reflectionClass->expects($this->any())->method('getName')->willReturn($class); + + $reflectionParameter->expects($this->any())->method('getClass')->will($this->returnValue($reflectionClass)); + } + + return $reflectionParameter; + } +} diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestArgumentResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestArgumentResolverTest.php new file mode 100644 index 0000000000000..ae6917829df91 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestArgumentResolverTest.php @@ -0,0 +1,63 @@ + + */ +class RequestArgumentResolverTest extends \PHPUnit_Framework_TestCase +{ + private $resolver; + + protected function setUp() + { + $this->resolver = new RequestArgumentResolver(); + } + + /** + * @dataProvider provideClasses + */ + public function testSupports($class, $supported = true) + { + $this->assertEquals($supported, $this->resolver->supports($this->getRequestMock(), $this->getReflectionParameterMock($class))); + } + + public function provideClasses() + { + return array( + array('Symfony\Component\HttpFoundation\Request'), + array('Symfony\Component\BrowerKit\Request', false), + array('\stdClass', false), + ); + } + + public function testResolvesToRequest() + { + $request = $this->getRequestMock(); + + $this->assertEquals($request, $this->resolver->resolve($request, $this->getReflectionParameterMock())); + } + + private function getRequestMock() + { + return $this->getMock('Symfony\Component\HttpFoundation\Request'); + } + + private function getReflectionParameterMock($class = null) + { + $reflectionParameter = $this->getMockBuilder('ReflectionParameter')->disableOriginalConstructor()->getMock(); + + if (null !== $class) { + $reflectionClass = $this->getMockBuilder('ReflectionClass')->disableOriginalConstructor()->getMock(); + $reflectionClass->expects($this->any())->method('isInstance')->will($this->returnCallback(function ($obj) use ($class) { + return is_a($obj, $class); + })); + + $reflectionParameter->expects($this->any())->method('getClass')->will($this->returnValue($reflectionClass)); + } + + return $reflectionParameter; + } +} diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestAttributesArgumentResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestAttributesArgumentResolverTest.php new file mode 100644 index 0000000000000..55c3d38fb3d66 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestAttributesArgumentResolverTest.php @@ -0,0 +1,60 @@ + + */ +class RequestAttributesArgumentResolverTest extends \PHPUnit_Framework_TestCase +{ + private $resolver; + + public function setUp() + { + $this->resolver = new RequestAttributesArgumentResolver(); + } + + /** + * @dataProvider provideParameters + */ + public function testSupports($parameterName, $supported = true) + { + $this->assertEquals($supported, $this->resolver->supports($this->getRequestMock(), $this->getReflectionParameterMock($parameterName))); + } + + public function provideParameters() + { + return array( + array('exists'), + array('not_exists', false), + ); + } + + public function testResolvesToRequestAttributeValues() + { + $this->assertEquals('value_of_exists', $this->resolver->resolve($this->getRequestMock(), $this->getReflectionParameterMock('exists'))); + } + + private function getRequestMock() + { + $request = $this->getMock('Symfony\Component\HttpFoundation\Request'); + $request->attributes = $this->getMock('Symfony\Component\HttpFoundation\ParameterBag'); + $request->attributes->expects($this->any())->method('has')->will($this->returnValueMap(array( + array('exists', true), + array('not_exists', false), + ))); + $request->attributes->expects($this->any())->method('get')->with('exists')->willReturn('value_of_exists'); + + return $request; + } + + private function getReflectionParameterMock($name) + { + $reflectionParameter = $this->getMockBuilder('ReflectionParameter')->disableOriginalConstructor()->getMock(); + $reflectionParameter->expects($this->any())->method('getName')->willReturn($name); + + return $reflectionParameter; + } +} diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php index c0a3bc71c419d..e5e7087e73692 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverManagerTest.php @@ -15,19 +15,20 @@ class ArgumentResolverManagerTest extends \PHPUnit_Framework_TestCase { - protected $manager; - protected $resolver1; - protected $resolver2; - protected $request; + private $manager; + private $resolver1; + private $resolver2; + private $request; public function setUp() { $this->resolver1 = $this->getMock('Symfony\Component\HttpKernel\Controller\ArgumentResolver\ArgumentResolverInterface'); $this->resolver2 = $this->getMock('Symfony\Component\HttpKernel\Controller\ArgumentResolver\ArgumentResolverInterface'); - $this->manager = new ArgumentResolverManager(); - $this->manager->add($this->resolver1); - $this->manager->add($this->resolver2); + $this->manager = new ArgumentResolverManager(array( + $this->resolver1, + $this->resolver2, + )); $this->request = $this->getMock('Symfony\Component\HttpFoundation\Request'); } @@ -109,20 +110,6 @@ public function testControllerWithOneParameterWithNullDefault() $this->assertArguments(array(null), function ($a = null) { }); } - public function testPriority() - { - $this->promiseResolverToNotMatch($this->resolver1); - $this->resolver1->expects($this->never())->method('resolve'); - - $this->promiseResolverToMatch($this->resolver2, 'resolved by 2'); - - $this->manager = new ArgumentResolverManager(); - $this->manager->add($this->resolver1); - $this->manager->add($this->resolver2, 100); - - $this->assertArguments(array('resolved by 2'), function ($a) { }); - } - private function assertArguments(array $expected, $controller) { $this->assertEquals($expected, $this->manager->getArguments($this->request, $controller));