diff --git a/core/core.services.yml b/core/core.services.yml index 40e6ab5..08a5593 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -81,7 +81,7 @@ services: class: Drupal\Core\Config\ConfigFactory tags: - { name: event_subscriber } - - { name: compiler_pass, tag: 'config.factory.override', method: addOverride, require: Drupal\Core\Config\ConfigFactoryOverrideInterface } + - { name: service_consumer, tag: 'config.factory.override', call: addOverride } arguments: ['@config.storage', '@event_dispatcher', '@config.typed'] config.installer: class: Drupal\Core\Config\ConfigInstaller @@ -171,7 +171,7 @@ services: http_client: class: Drupal\Core\Http\Client tags: - - { name: compiler_pass, tag: http_client_subscriber, method: attach, require: GuzzleHttp\Event\SubscriberInterface } + - { name: service_consumer, tag: http_client_subscriber, call: attach } http_client_simpletest_subscriber: class: Drupal\Core\Http\Plugin\SimpletestHttpRequestSubscriber tags: @@ -180,7 +180,7 @@ services: class: Drupal\Core\Theme\ThemeNegotiator arguments: ['@access_check.theme', '@request_stack'] tags: - - { name: compiler_pass, tag: theme_negotiator, method: addNegotiator, require: Drupal\Core\Theme\ThemeNegotiatorInterface } + - { name: service_consumer, tag: theme_negotiator, call: addNegotiator } theme.negotiator.default: class: Drupal\Core\Theme\DefaultNegotiator arguments: ['@config.factory'] @@ -282,7 +282,7 @@ services: calls: - [initLanguageManager] tags: - - { name: compiler_pass, tag: string_translator, method: addTranslator, require: Drupal\Core\StringTranslation\Translator\TranslatorInterface } + - { name: service_consumer, tag: string_translator, call: addTranslator } database.slave: class: Drupal\Core\Database\Connection factory_class: Drupal\Core\Database\Database @@ -329,7 +329,7 @@ services: calls: - [setFinalMatcher, ['@router.matcher.final_matcher']] tags: - - { name: compiler_pass, tag: route_filter, method: addRouteFilter, require: Symfony\Cmf\Component\Routing\NestedMatcher\RouteFilterInterface } + - { name: service_consumer, tag: route_filter, call: addRouteFilter } url_generator: class: Drupal\Core\Routing\UrlGenerator arguments: ['@router.route_provider', '@path_processor_manager', '@route_processor_manager', '@config.factory', '@settings'] @@ -343,7 +343,7 @@ services: class: Symfony\Cmf\Component\Routing\DynamicRouter arguments: ['@router.request_context', '@router.matcher', '@url_generator'] tags: - - { name: compiler_pass, tag: route_enhancer, method: addRouteEnhancer, require: Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface } + - { name: service_consumer, tag: route_enhancer, call: addRouteEnhancer } router: class: Symfony\Cmf\Component\Routing\ChainRouter calls: @@ -609,12 +609,12 @@ services: route_processor_manager: class: Drupal\Core\RouteProcessor\RouteProcessorManager tags: - - { name: compiler_pass, tag: route_processor_outbound, method: addOutbound, require: Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface } + - { name: service_consumer, tag: route_processor_outbound, call: addOutbound } path_processor_manager: class: Drupal\Core\PathProcessor\PathProcessorManager tags: - - { name: compiler_pass, tag: path_processor_inbound, method: addInbound, require: Drupal\Core\PathProcessor\InboundPathProcessorInterface } - - { name: compiler_pass, tag: path_processor_outbound, method: addOutbound, require: Drupal\Core\PathProcessor\OutboundPathProcessorInterface } + - { name: service_consumer, tag: path_processor_inbound, call: addInbound } + - { name: service_consumer, tag: path_processor_outbound, call: addOutbound } path_processor_decode: class: Drupal\Core\PathProcessor\PathProcessorDecode tags: @@ -667,7 +667,7 @@ services: class: Drupal\Core\Breadcrumb\BreadcrumbManager arguments: ['@module_handler'] tags: - - { name: compiler_pass, tag: breadcrumb_builder, method: addBuilder, require: Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface } + - { name: service_consumer, tag: breadcrumb_builder, call: addBuilder } token: class: Drupal\Core\Utility\Token arguments: ['@module_handler'] diff --git a/core/lib/Drupal/Core/CoreServiceProvider.php b/core/lib/Drupal/Core/CoreServiceProvider.php index 7586120..d6d7339 100644 --- a/core/lib/Drupal/Core/CoreServiceProvider.php +++ b/core/lib/Drupal/Core/CoreServiceProvider.php @@ -107,10 +107,9 @@ public static function registerTwig(ContainerBuilder $container) { // @todo Figure out what to do about debugging functions. // @see http://drupal.org/node/1804998 ->addMethodCall('addExtension', array(new Definition('Twig_Extension_Debug'))) - ->addTag('compiler_pass', array( + ->addTag('service_consumer', array( 'tag' => 'twig.extension', - 'method' => 'addExtension', - 'require' => 'Twig_ExtensionInterface', + 'call' => 'addExtension', )); } diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php index 90af230..0f9fb17 100644 --- a/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php @@ -13,33 +13,47 @@ use Symfony\Component\DependencyInjection\Reference; /** - * Adds tagged handlers to corresponding consumer services. + * Collects services to add/inject them into a consumer service. + * + * This mechanism allows a service to get multiple processor services injected, + * in order to establish an extensible architecture. + * + * It differs from the factory pattern in that processors are not lazily + * instantiated on demand; the consuming service receives instances of all + * registered processors when it is instantiated. + * + * It differs from plugins in that all processors are hard-coded and explicitly + * registered by service providers; the mere availability of a processor + * (cf. plugin discovery) does not imply that a processor ought to be used. + * + * It differs from constructor injection via service definition arguments in + * that a consuming service MAY allow further processors to be added dynamically + * at runtime. This is why the called method receives the priority of a + * processor as second argument. */ class TaggedHandlersPass implements CompilerPassInterface { /** * {@inheritdoc} * - * Finds all services tagged with 'compiler_pass' and adds method calls for - * all tagged handlers/providers for it to its definition. + * Finds services tagged with 'service_consumer', then finds all corresponding + * tagged services and adds method calls for each to the consumer service + * definition. * - * Supported compiler_pass tag attributes: + * Supported 'service_consumer' tag attributes: * - tag: The tag name used by handler services to collect. Defaults to the * service ID of the consumer. - * - method: The method name to call on the consumer service. Defaults to - * 'addHandler'. This method gets the handler instance as first argument and - * the handler's priority as second, whereas all handlers discovered at - * compile time are sorted already. - * - require: A fully qualified interface name to validate before adding - * handlers. None by default. + * - call: The method name to call on the consumer service. Defaults to + * 'addHandler'. The called method receives two arguments: + * - The handler instance as first argument. + * - Optionally the handler's priority as second argument, if the method + * accepts a second parameter and its name is "priority". In any case, all + * handlers registered at compile time are sorted already. * * Example (YAML): * @code * tags: - * - name: compiler_pass - * tag: breadcrumb_builder - * method: addBuilder - * require: Drupal\Core\Breadcrumb\BreadcrumbBuilderInterface + * - { name: service_consumer, tag: breadcrumb_builder, call: addBuilder } * @encode * * Supported handler tag attributes: @@ -48,34 +62,50 @@ class TaggedHandlersPass implements CompilerPassInterface { * Example (YAML): * @code * tags: - * - name: breadcrumb_builder - * priority: 100 + * - { name: breadcrumb_builder, priority: 100 } * @endcode * * @throws \Symfony\Component\DependencyInjection\Exception\LogicException + * If the method of a consumer service to be called does not type-hint an + * interface. + * @throws \Symfony\Component\DependencyInjection\Exception\LogicException * If a tagged handler does not implement the required interface. */ public function process(ContainerBuilder $container) { - foreach ($container->findTaggedServiceIds('compiler_pass') as $consumer_id => $passes) { + foreach ($container->findTaggedServiceIds('service_consumer') as $consumer_id => $passes) { foreach ($passes as $pass) { - $consumer_attributes = $pass; - // Ensure defaults. - $consumer_attributes += array( - 'tag' => $consumer_id, - 'method' => 'addHandler', - ); + $tag = isset($pass['tag']) ? $pass['tag'] : $consumer_id; + $method_name = isset($pass['call']) ? $pass['call'] : 'addHandler'; + + // Determine parameters. + $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'; + + if (!isset($interface)) { + if ($container->getParameter('kernel.environment') === 'prod') { + continue; + } + throw new LogicException(vsprintf("Service consumer '%s' class method %s::%s() has to type-hint an interface.", array( + $consumer_id, + $consumer->getClass(), + $method_name, + ))); + } + $interface = $interface->getName(); + // Find all tagged handlers. $handlers = array(); - foreach ($container->findTaggedServiceIds($consumer_attributes['tag']) as $id => $attributes) { - // If an interface constraint exists, ensure that every handler meets it. - if (isset($consumer_attributes['require'])) { - $handler = $container->getDefinition($id); - if (!is_subclass_of($handler->getClass(), $consumer_attributes['require'])) { - if ($container->getParameter('kernel.environment') === 'prod') { - continue; - } - throw new LogicException("Handler service '$id' for consumer '$consumer_id' does not implement {$consumer_attributes['require']}."); + foreach ($container->findTaggedServiceIds($tag) as $id => $attributes) { + // Validate the interface. + $handler = $container->getDefinition($id); + if (!is_subclass_of($handler->getClass(), $interface)) { + if ($container->getParameter('kernel.environment') === 'prod') { + continue; } + throw new LogicException("Service '$id' for consumer '$consumer_id' does not implement $interface."); } $handlers[$id] = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; } @@ -85,10 +115,15 @@ public function process(ContainerBuilder $container) { // Sort all handlers by priority. arsort($handlers, SORT_NUMERIC); - // Add a method call for each handler to the consumer service definition. - $consumer = $container->getDefinition($consumer_id); + // Add a method call for each handler to the consumer service + // definition. foreach ($handlers as $id => $priority) { - $consumer->addMethodCall($consumer_attributes['method'], array(new Reference($id), $priority)); + if ($accepts_priority) { + $consumer->addMethodCall($method_name, array(new Reference($id), $priority)); + } + else { + $consumer->addMethodCall($method_name, array(new Reference($id))); + } } } } diff --git a/core/lib/Drupal/Core/PathProcessor/PathProcessorManager.php b/core/lib/Drupal/Core/PathProcessor/PathProcessorManager.php index db8b799..3c8b9be 100644 --- a/core/lib/Drupal/Core/PathProcessor/PathProcessorManager.php +++ b/core/lib/Drupal/Core/PathProcessor/PathProcessorManager.php @@ -57,7 +57,6 @@ class PathProcessorManager implements InboundPathProcessorInterface, OutboundPat * * @param \Drupal\Core\PathProcessor\InboundPathProcessorInterface $processor * The processor object to add. - * * @param int $priority * The priority of the processor being added. */ @@ -97,7 +96,6 @@ protected function getInbound() { * * @param \Drupal\Core\PathProcessor\OutboundPathProcessorInterface $processor * The processor object to add. - * * @param int $priority * The priority of the processor being added. */ diff --git a/core/lib/Drupal/Core/RouteProcessor/RouteProcessorManager.php b/core/lib/Drupal/Core/RouteProcessor/RouteProcessorManager.php index 43071be..3dc8864 100644 --- a/core/lib/Drupal/Core/RouteProcessor/RouteProcessorManager.php +++ b/core/lib/Drupal/Core/RouteProcessor/RouteProcessorManager.php @@ -39,7 +39,6 @@ class RouteProcessorManager implements OutboundRouteProcessorInterface { * * @param \Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface $processor * The processor object to add. - * * @param int $priority * The priority of the processor being added. */ diff --git a/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php b/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php index 1253a18..34af9f6 100644 --- a/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php +++ b/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php @@ -2,7 +2,7 @@ /** * @file - * Contains \Drupal\Tests\Core\DependencyInjection\Compiler\TaggedHandlersPass. + * Contains \Drupal\Tests\Core\DependencyInjection\Compiler\TaggedHandlersPassTest. */ namespace Drupal\Tests\Core\DependencyInjection\Compiler; @@ -29,62 +29,117 @@ public static function getInfo() { return array( 'name' => 'Tests \Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass', 'description' => '', - 'group' => 'Dependency injection' + 'group' => 'Dependency injection', ); } + protected function buildContainer($environment = 'dev') { + $container = new ContainerBuilder(); + $container->setParameter('kernel.environment', $environment); + return $container; + } + /** - * Tests without a specific consumer. + * Tests without any consumers. + * + * @covers ::process */ - public function testProcessWithoutConsumer() { - $container_builder = new ContainerBuilder(); - $container_builder->register('consumer_test', 'Drupal\Tests\Core\DependencyInjection\Compiler\ConsumerService'); + public function testProcessNoConsumers() { + $container = $this->buildContainer(); + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer'); + $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); - $handler_pass->process($container_builder); - $this->assertCount(1, $container_builder->getDefinitions()); - $this->assertFalse($container_builder->getDefinition('consumer_test')->hasMethodCall('addHandler')); + $this->assertCount(1, $container->getDefinitions()); + $this->assertFalse($container->getDefinition('consumer_id')->hasMethodCall('addHandler')); } /** - * Tests with a specific consumer and two handlers. + * Tests consumer with missing interface in non-production environment. + * + * @expectedException \Symfony\Component\DependencyInjection\Exception\LogicException + * @expectedExceptionMessage Service consumer 'consumer_id' class method Drupal\Tests\Core\DependencyInjection\Compiler\InvalidConsumer::addHandler() has to type-hint an interface. + * @covers ::process */ - public function testProcess() { - $container_builder = new ContainerBuilder(); - $container_builder->register('consumer_test', 'Drupal\Tests\Core\DependencyInjection\Compiler\ConsumerService') - ->addTag('compiler_pass'); - - $container_builder->register('handler1', 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerExample') - ->addTag('consumer_test'); - $container_builder->register('handler2', 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerExample2') - ->addTag('consumer_test'); + public function testProcessMissingInterface() { + $container = $this->buildContainer(); + $container + ->register('consumer_id', __NAMESPACE__ . '\InvalidConsumer') + ->addTag('service_consumer'); $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); + } - $handler_pass->process($container_builder); + /** + * Tests consumer with missing interface in production environment. + * + * @covers ::process + */ + public function testProcessMissingInterfaceProd() { + $container = $this->buildContainer('prod'); + $container + ->register('consumer_id', __NAMESPACE__ . '\InvalidConsumer') + ->addTag('service_consumer'); + + $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); - $this->assertCount(2, $container_builder->getDefinition('consumer_test')->getMethodCalls()); + $this->assertCount(1, $container->getDefinitions()); + $this->assertFalse($container->getDefinition('consumer_id')->hasMethodCall('addHandler')); } /** - * Tests with a specified priority of the handlers. + * Tests one consumer and two handlers. + * + * @covers ::process */ - public function testProcessWithPriority() { - $container_builder = new ContainerBuilder(); - $container_builder->register('consumer_test', 'Drupal\Tests\Core\DependencyInjection\Compiler\ConsumerService') - ->addTag('compiler_pass'); - - $container_builder->register('handler1', 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerExample') - ->addTag('consumer_test'); - $container_builder->register('handler2', 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerExample2') - ->addTag('consumer_test', array('priority' => 10)); + public function testProcess() { + $container = $this->buildContainer(); + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer') + ->addTag('service_consumer'); + + $container + ->register('handler1', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id'); + $container + ->register('handler2', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id'); $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); + + $method_calls = $container->getDefinition('consumer_id')->getMethodCalls(); + $this->assertCount(2, $method_calls); + } + + /** + * Tests handler priority sorting. + * + * @covers ::process + */ + public function testProcessPriority() { + $container = $this->buildContainer(); + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer') + ->addTag('service_consumer'); + + $container + ->register('handler1', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id'); + $container + ->register('handler2', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id', array( + 'priority' => 10, + )); - $handler_pass->process($container_builder); + $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); - $method_calls = $container_builder->getDefinition('consumer_test') - ->getMethodCalls(); + $method_calls = $container->getDefinition('consumer_id')->getMethodCalls(); $this->assertCount(2, $method_calls); $this->assertEquals(new Reference('handler2'), $method_calls[0][1][0]); $this->assertEquals(10, $method_calls[0][1][1]); @@ -93,67 +148,108 @@ public function testProcessWithPriority() { } /** - * Tests with a specified require interface. + * Tests consumer method without priority parameter. + * + * @covers ::process */ - public function testProcessWithRequireInProd() { - $container_builder = new ContainerBuilder(); - $container_builder->register('consumer_test', 'Drupal\Tests\Core\DependencyInjection\Compiler\ConsumerService') - ->addTag('compiler_pass', array('require' => 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerInterface')); - - $container_builder->register('handler1', 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerExample') - ->addTag('consumer_test'); - $container_builder->register('handler2', 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerExample2') - ->addTag('consumer_test', array('priority' => 10)); - $container_builder->setParameter('kernel.environment', 'prod'); + public function testProcessNoPriorityParam() { + $container = $this->buildContainer(); + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer') + ->addTag('service_consumer', array( + 'call' => 'addNoPriority', + )); + + $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); - $handler_pass->process($container_builder); - - $method_calls = $container_builder->getDefinition('consumer_test') - ->getMethodCalls(); - $this->assertCount(1, $method_calls); + $method_calls = $container->getDefinition('consumer_id')->getMethodCalls(); + $this->assertCount(2, $method_calls); $this->assertEquals(new Reference('handler2'), $method_calls[0][1][0]); - $this->assertEquals(10, $method_calls[0][1][1]); + $this->assertCount(1, $method_calls[0][1]); + $this->assertEquals(new Reference('handler1'), $method_calls[1][1][0]); + $this->assertCount(1, $method_calls[0][1]); } /** - * Tests with a specified require interface. + * Tests interface validation in non-production environment. * * @expectedException \Symfony\Component\DependencyInjection\Exception\LogicException + * @covers ::process */ - public function testProcessWithRequireInDev() { - $container_builder = new ContainerBuilder(); - $container_builder->register('consumer_test', 'Drupal\Tests\Core\DependencyInjection\Compiler\ConsumerService') - ->addTag('compiler_pass', array('require' => 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerInterface')); - - $container_builder->register('handler1', 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerExample') - ->addTag('consumer_test'); - $container_builder->register('handler2', 'Drupal\Tests\Core\DependencyInjection\Compiler\HandlerExample2') - ->addTag('consumer_test', array('priority' => 10)); - $container_builder->setParameter('kernel.environment', 'dev'); + public function testProcessInterfaceMismatch() { + $container = $this->buildContainer(); + + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer') + ->addTag('service_consumer'); + $container + ->register('handler1', __NAMESPACE__ . '\InvalidHandler') + ->addTag('consumer_id'); + $container + ->register('handler2', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id', array( + 'priority' => 10, + )); $handler_pass = new TaggedHandlersPass(); - - $handler_pass->process($container_builder); + $handler_pass->process($container); } -} + /** + * Tests interface validation in production environment. + * + * @covers ::process + */ + public function testProcessInterfaceMismatchProd() { + $container = $this->buildContainer('prod'); + + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer') + ->addTag('service_consumer'); + $container + ->register('handler1', __NAMESPACE__ . '\InvalidHandler') + ->addTag('consumer_id'); + $container + ->register('handler2', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id', array( + 'priority' => 10, + )); -class ConsumerService { + $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); + + $method_calls = $container->getDefinition('consumer_id')->getMethodCalls(); + $this->assertCount(1, $method_calls); + $this->assertEquals(new Reference('handler2'), $method_calls[0][1][0]); + $this->assertEquals(10, $method_calls[0][1][1]); + } } interface HandlerInterface { - } - -class HandlerExample { - +class ValidConsumer { + public function addHandler(HandlerInterface $instance, $priority = 0) { + } + public function addNoPriority(HandlerInterface $instance) { + } } - -class HandlerExample2 implements HandlerInterface { - +class InvalidConsumer { + public function addHandler($instance, $priority = 0) { + } +} +class ValidHandler implements HandlerInterface { +} +class InvalidHandler { } -