diff --git a/core/core.services.yml b/core/core.services.yml index a9b8c3e..a44ebb3 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -402,8 +402,8 @@ services: - { name: route_filter } paramconverter_manager: class: Drupal\Core\ParamConverter\ParamConverterManager - calls: - - [setContainer, ['@service_container']] + tags: + - { name: service_collector, tag: paramconverter, call: addConverter } paramconverter_subscriber: class: Drupal\Core\EventSubscriber\ParamConverterSubscriber tags: diff --git a/core/lib/Drupal/Core/CoreServiceProvider.php b/core/lib/Drupal/Core/CoreServiceProvider.php index 11c7781..1f18c79 100644 --- a/core/lib/Drupal/Core/CoreServiceProvider.php +++ b/core/lib/Drupal/Core/CoreServiceProvider.php @@ -15,7 +15,6 @@ use Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass; use Drupal\Core\DependencyInjection\Compiler\RegisterKernelListenersPass; use Drupal\Core\DependencyInjection\Compiler\RegisterAccessChecksPass; -use Drupal\Core\DependencyInjection\Compiler\RegisterParamConvertersPass; use Drupal\Core\DependencyInjection\Compiler\RegisterServicesForDestructionPass; use Drupal\Core\DependencyInjection\Compiler\RegisterAuthenticationPass; use Drupal\Core\Plugin\PluginManagerPass; @@ -62,9 +61,6 @@ public function register(ContainerBuilder $container) { $container->addCompilerPass(new RegisterAccessChecksPass()); - // Add a compiler pass for upcasting route parameters. - $container->addCompilerPass(new RegisterParamConvertersPass()); - // Add a compiler pass for registering services needing destruction. $container->addCompilerPass(new RegisterServicesForDestructionPass()); diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php index 9ebdaaa..e550f0b 100644 --- a/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php @@ -85,8 +85,22 @@ public function process(ContainerBuilder $container) { $consumer = $container->getDefinition($consumer_id); $method = new \ReflectionMethod($consumer->getClass(), $method_name); $params = $method->getParameters(); - $interface = $params[0]->getClass(); - $accepts_priority = isset($params[1]) && $params[1]->getName() === 'priority'; + + $interface_pos = 0; + $id_pos = NULL; + $priority_pos = NULL; + foreach ($params as $pos => $param) { + if ($param->getClass()) { + $interface = $param->getClass(); + } + if ($param->getName() == 'id') { + $id_pos = $pos; + } + if ($param->getName() == 'priority') { + $priority_pos = $pos; + } + } + // Determine the ID. if (!isset($interface)) { if ($container->getParameter('kernel.environment') === 'prod') { @@ -122,12 +136,17 @@ public function process(ContainerBuilder $container) { // Add a method call for each handler to the consumer service // definition. foreach ($handlers as $id => $priority) { - if ($accepts_priority) { - $consumer->addMethodCall($method_name, array(new Reference($id), $priority)); + $arguments = array(); + $arguments[$interface_pos] = new Reference($id); + if (isset($priority_pos)) { + $arguments[$priority_pos] = $priority; } - else { - $consumer->addMethodCall($method_name, array(new Reference($id))); + if (isset($id_pos)) { + $arguments[$id_pos] = $id; } + // Sort the arguments by position. + ksort($arguments); + $consumer->addMethodCall($method_name, $arguments); } } } diff --git a/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php b/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php index e93e8cb..cfc8963 100644 --- a/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php +++ b/core/lib/Drupal/Core/ParamConverter/ParamConverterManager.php @@ -19,23 +19,7 @@ * A typical use case for this would be upcasting (converting) a node id to a * node entity. */ -class ParamConverterManager implements ParamConverterManagerInterface, ContainerAwareInterface { - - use ContainerAwareTrait; - - /** - * An array of registered converter service ids. - * - * @var array - */ - protected $converterIds = array(); - - /** - * Array of registered converter service ids sorted by their priority. - * - * @var array - */ - protected $sortedConverterIds; +class ParamConverterManager implements ParamConverterManagerInterface { /** * Array of loaded converter services keyed by their ids. @@ -47,40 +31,21 @@ class ParamConverterManager implements ParamConverterManagerInterface, Container /** * {@inheritdoc} */ - public function addConverter($converter, $priority = 0) { - if (empty($this->converterIds[$priority])) { - $this->converterIds[$priority] = array(); - } - $this->converterIds[$priority][] = $converter; - unset($this->sortedConverterIds); + public function addConverter(ParamConverterInterface $param_converter, $id) { + $this->converters[$id] = $param_converter; return $this; } /** * {@inheritdoc} */ - public function getConverterIds() { - if (!isset($this->sortedConverterIds)) { - krsort($this->converterIds); - $this->sortedConverterIds = array(); - foreach ($this->converterIds as $resolvers) { - $this->sortedConverterIds = array_merge($this->sortedConverterIds, $resolvers); - } - } - return $this->sortedConverterIds; - } - - /** - * {@inheritdoc} - */ public function getConverter($converter) { if (isset($this->converters[$converter])) { return $this->converters[$converter]; } - if (!in_array($converter, $this->getConverterIds())) { + else { throw new \InvalidArgumentException(sprintf('No converter has been registered for %s', $converter)); } - return $this->converters[$converter] = $this->container->get($converter); } /** @@ -100,7 +65,7 @@ public function setRouteParameterConverters(RouteCollection $routes) { continue; } - foreach ($this->getConverterIds() as $converter) { + foreach (array_keys($this->converters) as $converter) { if ($this->getConverter($converter)->applies($definition, $name, $route)) { $definition['converter'] = $converter; break; diff --git a/core/lib/Drupal/Core/ParamConverter/ParamConverterManagerInterface.php b/core/lib/Drupal/Core/ParamConverter/ParamConverterManagerInterface.php index 8240e62..a1761da 100644 --- a/core/lib/Drupal/Core/ParamConverter/ParamConverterManagerInterface.php +++ b/core/lib/Drupal/Core/ParamConverter/ParamConverterManagerInterface.php @@ -18,27 +18,19 @@ /** * Registers a parameter converter with the manager. * - * @param string $converter + * @param \Drupal\Core\ParamConverter\ParamConverterInterface $param_converter + * The added param converter instance. + * @param string $id * The parameter converter service id to register. - * @param int $priority - * (optional) The priority of the converter. Defaults to 0. * * @return $this */ - public function addConverter($converter, $priority = 0); - - /** - * Sorts the converter service ids and flattens them. - * - * @return array - * The sorted parameter converter service ids. - */ - public function getConverterIds(); + public function addConverter(ParamConverterInterface $param_converter, $id); /** * Lazy-loads converter services. * - * @param string $converter + * @param string $id * The service id of converter service to load. * * @return \Drupal\Core\ParamConverter\ParamConverterInterface @@ -47,7 +39,7 @@ public function getConverterIds(); * @throws \InvalidArgumentException * If the given service id is not a registered converter. */ - public function getConverter($converter); + public function getConverter($id); /** * Saves a list of applicable converters to each route. diff --git a/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php b/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php index 528c45e..2d62eab 100644 --- a/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php +++ b/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php @@ -181,6 +181,41 @@ public function testProcessNoPriorityParam() { } /** + * Tests consumer method with an ID parameter. + * + * @covers ::process + */ + public function testProcessWithIdParameter() { + $container = $this->buildContainer(); + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer') + ->addTag('service_collector', array( + 'call' => 'addWithId', + )); + + $container + ->register('handler1', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id'); + $container + ->register('handler2', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id', array( + 'priority' => 10, + )); + + $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); + + $method_calls = $container->getDefinition('consumer_id')->getMethodCalls(); + $this->assertCount(2, $method_calls); + $this->assertEquals(new Reference('handler2'), $method_calls[0][1][0]); + $this->assertEquals('handler2', $method_calls[0][1][1]); + $this->assertEquals(10, $method_calls[0][1][2]); + $this->assertEquals(new Reference('handler1'), $method_calls[1][1][0]); + $this->assertEquals('handler1', $method_calls[1][1][1]); + $this->assertEquals(0, $method_calls[1][1][2]); + } + + /** * Tests interface validation in non-production environment. * * @expectedException \Symfony\Component\DependencyInjection\Exception\LogicException @@ -243,6 +278,8 @@ public function addHandler(HandlerInterface $instance, $priority = 0) { } public function addNoPriority(HandlerInterface $instance) { } + public function addWithId(HandlerInterface $instance, $id, $priority = 0) { + } } class InvalidConsumer { public function addHandler($instance, $priority = 0) { diff --git a/core/tests/Drupal/Tests/Core/ParamConverter/ParamConverterManagerTest.php b/core/tests/Drupal/Tests/Core/ParamConverter/ParamConverterManagerTest.php index c369582..d308a5d 100644 --- a/core/tests/Drupal/Tests/Core/ParamConverter/ParamConverterManagerTest.php +++ b/core/tests/Drupal/Tests/Core/ParamConverter/ParamConverterManagerTest.php @@ -22,11 +22,6 @@ class ParamConverterManagerTest extends UnitTestCase { /** - * @var \Symfony\Component\DependencyInjection\ContainerBuilder|\PHPUnit_Framework_MockObject_MockObject - */ - protected $container; - - /** * @var \Drupal\Core\ParamConverter\ParamConverterManager */ protected $manager; @@ -48,29 +43,7 @@ public static function getInfo() { public function setUp() { parent::setUp(); - $this->container = $this->getMock('Drupal\Core\DependencyInjection\Container'); $this->manager = new ParamConverterManager(); - $this->manager->setContainer($this->container); - } - - /** - * Tests \Drupal\Core\ParamConverter\ParamConverterManager::addConverter(). - * - * @dataProvider providerTestAddConverter - * - * @covers ::addConverter() - * @covers ::getConverterIds() - */ - public function testAddConverter($unsorted, $sorted) { - foreach ($unsorted as $data) { - $this->manager->addConverter($data['name'], $data['priority']); - } - - // Test that ResolverManager::getTypedDataResolvers() returns the resolvers - // in the expected order. - foreach ($this->manager->getConverterIds() as $key => $converter) { - $this->assertEquals($sorted[$key], $converter); - } } /** @@ -80,16 +53,12 @@ public function testAddConverter($unsorted, $sorted) { * * @covers ::getConverter() */ - public function testGetConverter($name, $priority, $class) { + public function testGetConverter($name, $class) { $converter = $this->getMockBuilder('Drupal\Core\ParamConverter\ParamConverterInterface') ->setMockClassName($class) ->getMock(); - $this->manager->addConverter($name, $priority); - $this->container->expects($this->once()) - ->method('get') - ->with($name) - ->will($this->returnValue($converter)); + $this->manager->addConverter($converter, $name); $this->assertInstanceOf($class, $this->manager->getConverter($name)); // Assert that a second call to getConverter() does not use the container. @@ -118,13 +87,13 @@ public function testGetConverterException() { */ public function providerTestAddConverter() { $converters[0]['unsorted'] = array( - array('name' => 'raspberry', 'priority' => 10), - array('name' => 'pear', 'priority' => 5), - array('name' => 'strawberry', 'priority' => 20), - array('name' => 'pineapple', 'priority' => 0), - array('name' => 'banana', 'priority' => -10), - array('name' => 'apple', 'priority' => -10), - array('name' => 'peach', 'priority' => 5), + array('name' => 'strawberry'), + array('name' => 'raspberry'), + array('name' => 'pear'), + array('name' => 'peach'), + array('name' => 'pineapple'), + array('name' => 'banana'), + array('name' => 'apple'), ); $converters[0]['sorted'] = array( @@ -133,13 +102,13 @@ public function providerTestAddConverter() { ); $converters[1]['unsorted'] = array( - array('name' => 'ape', 'priority' => 0), - array('name' => 'cat', 'priority' => -5), - array('name' => 'puppy', 'priority' => -10), - array('name' => 'llama', 'priority' => -15), - array('name' => 'giraffe', 'priority' => 10), - array('name' => 'zebra', 'priority' => 10), - array('name' => 'eagle', 'priority' => 5), + array('name' => 'giraffe'), + array('name' => 'zebra'), + array('name' => 'eagle'), + array('name' => 'ape'), + array('name' => 'cat'), + array('name' => 'puppy'), + array('name' => 'llama'), ); $converters[1]['sorted'] = array( @@ -161,13 +130,13 @@ public function providerTestAddConverter() { */ public function providerTestGetConverter() { return array( - array('ape', 0, 'ApeConverterClass'), - array('cat', -5, 'CatConverterClass'), - array('puppy', -10, 'PuppyConverterClass'), - array('llama', -15, 'LlamaConverterClass'), - array('giraffe', 10, 'GiraffeConverterClass'), - array('zebra', 10, 'ZebraConverterClass'), - array('eagle', 5, 'EagleConverterClass'), + array('ape', 'ApeConverterClass'), + array('cat', 'CatConverterClass'), + array('puppy', 'PuppyConverterClass'), + array('llama', 'LlamaConverterClass'), + array('giraffe', 'GiraffeConverterClass'), + array('zebra', 'ZebraConverterClass'), + array('eagle', 'EagleConverterClass'), ); } @@ -182,11 +151,7 @@ public function testSetRouteParameterConverters($path, $parameters = NULL, $expe ->method('applies') ->with($this->anything(), 'id', $this->anything()) ->will($this->returnValue(TRUE)); - $this->manager->addConverter('applied'); - $this->container->expects($this->any()) - ->method('get') - ->with('applied') - ->will($this->returnValue($converter)); + $this->manager->addConverter($converter, 'applied'); $route = new Route($path); if ($parameters) { @@ -248,11 +213,7 @@ public function testConvert() { ->method('convert') ->with(1, $this->isType('array'), 'id', $this->isType('array'), $this->isInstanceOf('Symfony\Component\HttpFoundation\Request')) ->will($this->returnValue('something_better!')); - $this->manager->addConverter('test_convert'); - $this->container->expects($this->once()) - ->method('get') - ->with('test_convert') - ->will($this->returnValue($converter)); + $this->manager->addConverter($converter, 'test_convert'); $result = $this->manager->convert($defaults, new Request()); @@ -301,11 +262,7 @@ public function testConvertMissingParam() { ->method('convert') ->with(1, $this->isType('array'), 'id', $this->isType('array'), $this->isInstanceOf('Symfony\Component\HttpFoundation\Request')) ->will($this->returnValue(NULL)); - $this->manager->addConverter('test_convert'); - $this->container->expects($this->once()) - ->method('get') - ->with('test_convert') - ->will($this->returnValue($converter)); + $this->manager->addConverter($converter, 'test_convert'); $this->manager->convert($defaults, new Request()); }