diff --git a/core/core.services.yml b/core/core.services.yml index 5ba0116..32bbbd6 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -424,8 +424,9 @@ services: theme.negotiator: class: Drupal\Core\Theme\ThemeNegotiator arguments: ['@access_check.theme'] + parent: container.trait tags: - - { name: service_collector, tag: theme_negotiator, call: addNegotiator } + - { name: service_id_collector, tag: theme_negotiator } theme.negotiator.default: class: Drupal\Core\Theme\DefaultNegotiator arguments: ['@config.factory'] diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php index aad1dcd..f52148c 100644 --- a/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php @@ -82,90 +82,141 @@ class TaggedHandlersPass implements CompilerPassInterface { public function process(ContainerBuilder $container) { foreach ($container->findTaggedServiceIds('service_collector') as $consumer_id => $passes) { foreach ($passes as $pass) { - $tag = isset($pass['tag']) ? $pass['tag'] : $consumer_id; - $method_name = isset($pass['call']) ? $pass['call'] : 'addHandler'; - $required = isset($pass['required']) ? $pass['required'] : FALSE; + $this->processServiceCollectorPass($pass, $consumer_id, $container); + } + } + + foreach ($container->findTaggedServiceIds('service_id_collector') as $consumer_id => $passes) { + foreach ($passes as $pass) { + $this->processServiceIdCollectorPass($pass, $consumer_id, $container); + } + } + } + + /** + * Processes a service collector service pass. + * + * @param array $pass + * The service collector pass data. + * @param string $consumer_id + * The consumer service ID. + * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container + * The service container. + */ + protected function processServiceCollectorPass(array $pass, $consumer_id, ContainerBuilder $container) { + $tag = isset($pass['tag']) ? $pass['tag'] : $consumer_id; + $method_name = isset($pass['call']) ? $pass['call'] : 'addHandler'; + $required = isset($pass['required']) ? $pass['required'] : FALSE; + + // Determine parameters. + $consumer = $container->getDefinition($consumer_id); + $method = new \ReflectionMethod($consumer->getClass(), $method_name); + $params = $method->getParameters(); - // Determine parameters. - $consumer = $container->getDefinition($consumer_id); - $method = new \ReflectionMethod($consumer->getClass(), $method_name); - $params = $method->getParameters(); + $interface_pos = 0; + $id_pos = NULL; + $priority_pos = NULL; + $extra_params = []; + foreach ($params as $pos => $param) { + if ($param->getClass()) { + $interface = $param->getClass(); + } + else if ($param->getName() === 'id') { + $id_pos = $pos; + } + else if ($param->getName() === 'priority') { + $priority_pos = $pos; + } + else { + $extra_params[$param->getName()] = $pos; + } + } + // Determine the ID. + + if (!isset($interface)) { + 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(); - $interface_pos = 0; - $id_pos = NULL; - $priority_pos = NULL; - $extra_params = []; - foreach ($params as $pos => $param) { - if ($param->getClass()) { - $interface = $param->getClass(); - } - else if ($param->getName() === 'id') { - $id_pos = $pos; - } - else if ($param->getName() === 'priority') { - $priority_pos = $pos; - } - else { - $extra_params[$param->getName()] = $pos; - } - } - // Determine the ID. + // Find all tagged handlers. + $handlers = array(); + $extra_arguments = array(); + foreach ($container->findTaggedServiceIds($tag) as $id => $attributes) { + // Validate the interface. + $handler = $container->getDefinition($id); + if (!is_subclass_of($handler->getClass(), $interface)) { + throw new LogicException("Service '$id' for consumer '$consumer_id' does not implement $interface."); + } + $handlers[$id] = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; + // Keep track of other tagged handlers arguments. + foreach ($extra_params as $name => $pos) { + $extra_arguments[$id][$pos] = isset($attributes[0][$name]) ? $attributes[0][$name] : $params[$pos]->getDefaultValue(); + } + } - if (!isset($interface)) { - 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(); + if ($required && empty($handlers)) { + throw new LogicException(sprintf("At least one service tagged with '%s' is required.", $tag)); + } - // Find all tagged handlers. - $handlers = array(); - $extra_arguments = array(); - foreach ($container->findTaggedServiceIds($tag) as $id => $attributes) { - // Validate the interface. - $handler = $container->getDefinition($id); - if (!is_subclass_of($handler->getClass(), $interface)) { - throw new LogicException("Service '$id' for consumer '$consumer_id' does not implement $interface."); - } - $handlers[$id] = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; - // Keep track of other tagged handlers arguments. - foreach ($extra_params as $name => $pos) { - $extra_arguments[$id][$pos] = isset($attributes[0][$name]) ? $attributes[0][$name] : $params[$pos]->getDefaultValue(); - } - } - if (empty($handlers)) { - if ($required) { - throw new LogicException(sprintf("At least one service tagged with '%s' is required.", $tag)); - } - continue; - } - // Sort all handlers by priority. - arsort($handlers, SORT_NUMERIC); + // Sort all handlers by priority. + arsort($handlers, SORT_NUMERIC); - // Add a method call for each handler to the consumer service - // definition. - foreach ($handlers as $id => $priority) { - $arguments = array(); - $arguments[$interface_pos] = new Reference($id); - if (isset($priority_pos)) { - $arguments[$priority_pos] = $priority; - } - if (isset($id_pos)) { - $arguments[$id_pos] = $id; - } - // Add in extra arguments. - if (isset($extra_arguments[$id])) { - // Place extra arguments in their right positions. - $arguments += $extra_arguments[$id]; - } - // Sort the arguments by position. - ksort($arguments); - $consumer->addMethodCall($method_name, $arguments); - } + // Add a method call for each handler to the consumer service + // definition. + foreach ($handlers as $id => $priority) { + $arguments = array(); + $arguments[$interface_pos] = new Reference($id); + if (isset($priority_pos)) { + $arguments[$priority_pos] = $priority; } + if (isset($id_pos)) { + $arguments[$id_pos] = $id; + } + // Add in extra arguments. + if (isset($extra_arguments[$id])) { + // Place extra arguments in their right positions. + $arguments += $extra_arguments[$id]; + } + // Sort the arguments by position. + ksort($arguments); + $consumer->addMethodCall($method_name, $arguments); + } + } + + /** + * Processes a service collector ID service pass. + * + * @param array $pass + * The service collector pass data. + * @param string $consumer_id + * The consumer service ID. + * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container + * The service container. + */ + protected function processServiceIdCollectorPass(array $pass, $consumer_id, ContainerBuilder $container) { + $tag = isset($pass['tag']) ? $pass['tag'] : $consumer_id; + $required = isset($pass['required']) ? $pass['required'] : FALSE; + + $consumer = $container->getDefinition($consumer_id); + + // Find all tagged handlers. + $handlers = []; + foreach ($container->findTaggedServiceIds($tag) as $id => $attributes) { + $handlers[$id] = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; } + + if ($required && empty($handlers)) { + throw new LogicException(sprintf("At least one service tagged with '%s' is required.", $tag)); + } + + // Sort all handlers by priority. + arsort($handlers, SORT_NUMERIC); + + $consumer->addArgument(array_keys($handlers)); } } diff --git a/core/lib/Drupal/Core/Theme/ThemeNegotiator.php b/core/lib/Drupal/Core/Theme/ThemeNegotiator.php index 8c1ef03..8a8db53 100644 --- a/core/lib/Drupal/Core/Theme/ThemeNegotiator.php +++ b/core/lib/Drupal/Core/Theme/ThemeNegotiator.php @@ -8,6 +8,7 @@ namespace Drupal\Core\Theme; use Drupal\Core\Routing\RouteMatchInterface; +use Symfony\Component\DependencyInjection\ContainerAware; /** * Provides a class which determines the active theme of the page. @@ -15,26 +16,16 @@ * It therefore uses ThemeNegotiatorInterface objects which are passed in * using the 'theme_negotiator' tag. * - * @see \Drupal\Core\Theme\ThemeNegotiatorPass * @see \Drupal\Core\Theme\ThemeNegotiatorInterface */ -class ThemeNegotiator implements ThemeNegotiatorInterface { +class ThemeNegotiator extends ContainerAware implements ThemeNegotiatorInterface { /** - * Holds arrays of theme negotiators, keyed by priority. + * Holds arrays of theme negotiators, sorted by priority. * * @var array */ - protected $negotiators = array(); - - /** - * Holds the array of theme negotiators sorted by priority. - * - * Set to NULL if the array needs to be re-calculated. - * - * @var array|NULL - */ - protected $sortedNegotiators; + protected $negotiators = []; /** * The access checker for themes. @@ -49,42 +40,9 @@ class ThemeNegotiator implements ThemeNegotiatorInterface { * @param \Drupal\Core\Theme\ThemeAccessCheck $theme_access * The access checker for themes. */ - public function __construct(ThemeAccessCheck $theme_access) { + public function __construct(ThemeAccessCheck $theme_access, array $negotiators) { $this->themeAccess = $theme_access; - } - - /** - * Adds a active theme negotiation service. - * - * @param \Drupal\Core\Theme\ThemeNegotiatorInterface $negotiator - * The theme negotiator to add. - * @param int $priority - * Priority of the theme negotiator. - */ - public function addNegotiator(ThemeNegotiatorInterface $negotiator, $priority) { - $this->negotiators[$priority][] = $negotiator; - // Force the negotiators to be re-sorted. - $this->sortedNegotiators = NULL; - } - - /** - * Returns the sorted array of theme negotiators. - * - * @return array|\Drupal\Core\Theme\ThemeNegotiatorInterface[] - * An array of theme negotiator objects. - */ - protected function getSortedNegotiators() { - if (!isset($this->sortedNegotiators)) { - // Sort the negotiators according to priority. - krsort($this->negotiators); - // Merge nested negotiators from $this->negotiators into - // $this->sortedNegotiators. - $this->sortedNegotiators = array(); - foreach ($this->negotiators as $builders) { - $this->sortedNegotiators = array_merge($this->sortedNegotiators, $builders); - } - } - return $this->sortedNegotiators; + $this->negotiators = $negotiators; } /** @@ -98,7 +56,9 @@ public function applies(RouteMatchInterface $route_match) { * {@inheritdoc} */ public function determineActiveTheme(RouteMatchInterface $route_match) { - foreach ($this->getSortedNegotiators() as $negotiator) { + foreach ($this->negotiators as $negotiator_id) { + $negotiator = $this->container->get($negotiator_id); + if ($negotiator->applies($route_match)) { $theme = $negotiator->determineActiveTheme($route_match); if ($theme !== NULL && $this->themeAccess->checkAccess($theme)) { diff --git a/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php b/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php index 055a907..3918c31 100644 --- a/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php +++ b/core/tests/Drupal/Tests/Core/DependencyInjection/Compiler/TaggedHandlersPassTest.php @@ -61,6 +61,26 @@ public function testProcessRequiredHandlers() { } /** + * Tests a required consumer with no handlers. + * + * @expectedException \Symfony\Component\DependencyInjection\Exception\LogicException + * @expectedExceptionMessage At least one service tagged with 'consumer_id' is required. + * @covers ::process + * @covers ::processServiceIdCollectorPass + */ + public function testIdCollectorProcessRequiredHandlers() { + $container = $this->buildContainer(); + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer') + ->addTag('service_id_collector', [ + 'required' => TRUE, + ]); + + $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); + } + + /** * Tests consumer with missing interface in non-production environment. * * @expectedException \Symfony\Component\DependencyInjection\Exception\LogicException @@ -103,6 +123,32 @@ public function testProcess() { } /** + * Tests one consumer and two handlers with service ID collection. + * + * @covers ::process + */ + public function testserviceIdProcess() { + $container = $this->buildContainer(); + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer') + ->addTag('service_id_collector'); + + $container + ->register('handler1', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id'); + $container + ->register('handler2', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id'); + + $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); + + $arguments = $container->getDefinition('consumer_id')->getArguments(); + $this->assertCount(1, $arguments); + $this->assertCount(2, $arguments[0]); + } + + /** * Tests handler priority sorting. * * @covers ::process @@ -134,6 +180,39 @@ public function testProcessPriority() { } /** + * Tests handler priority sorting for service ID collection. + * + * @covers ::process + */ + public function testserviceIdProcessPriority() { + $container = $this->buildContainer(); + $container + ->register('consumer_id', __NAMESPACE__ . '\ValidConsumer') + ->addTag('service_id_collector'); + + $container + ->register('handler1', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id'); + $container + ->register('handler2', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id', [ + 'priority' => 20, + ]); + $container + ->register('handler3', __NAMESPACE__ . '\ValidHandler') + ->addTag('consumer_id', [ + 'priority' => 10, + ]); + + $handler_pass = new TaggedHandlersPass(); + $handler_pass->process($container); + + $arguments = $container->getDefinition('consumer_id')->getArguments(); + $this->assertCount(1, $arguments); + $this->assertSame(['handler2', 'handler3', 'handler1'], $arguments[0]); + } + + /** * Tests consumer method without priority parameter. * * @covers ::process diff --git a/core/tests/Drupal/Tests/Core/Theme/ThemeNegotiatorTest.php b/core/tests/Drupal/Tests/Core/Theme/ThemeNegotiatorTest.php index 55a6f7f..ceae0ae 100644 --- a/core/tests/Drupal/Tests/Core/Theme/ThemeNegotiatorTest.php +++ b/core/tests/Drupal/Tests/Core/Theme/ThemeNegotiatorTest.php @@ -7,11 +7,10 @@ namespace Drupal\Tests\Core\Theme; +use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Routing\RouteMatch; use Drupal\Core\Theme\ThemeNegotiator; use Drupal\Tests\UnitTestCase; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Routing\Route; /** @@ -28,6 +27,13 @@ class ThemeNegotiatorTest extends UnitTestCase { protected $themeAccessCheck; /** + * The container builder. + * + * @var \Drupal\Core\DependencyInjection\ContainerBuilder + */ + protected $container; + + /** * The request stack. * * @var \Symfony\Component\HttpFoundation\RequestStack @@ -41,11 +47,14 @@ class ThemeNegotiatorTest extends UnitTestCase { */ protected $themeNegotiator; + /** + * {@inheritdoc} + */ protected function setUp() { $this->themeAccessCheck = $this->getMockBuilder('\Drupal\Core\Theme\ThemeAccessCheck') ->disableOriginalConstructor() ->getMock(); - $this->themeNegotiator = new ThemeNegotiator($this->themeAccessCheck); + $this->container = new ContainerBuilder(); } /** @@ -62,14 +71,16 @@ public function testDetermineActiveTheme() { ->method('applies') ->will($this->returnValue(TRUE)); - $this->themeNegotiator->addNegotiator($negotiator, 0); + $this->container->set('test_negotiator', $negotiator); + + $negotiators = ['test_negotiator']; $this->themeAccessCheck->expects($this->any()) ->method('checkAccess') ->will($this->returnValue(TRUE)); $route_match = new RouteMatch('test_route', new Route('/test-route'), array(), array()); - $theme = $this->themeNegotiator->determineActiveTheme($route_match); + $theme = $this->createThemeNegotiator($negotiators)->determineActiveTheme($route_match); $this->assertEquals('example_test', $theme); } @@ -80,6 +91,8 @@ public function testDetermineActiveTheme() { * @see \Drupal\Core\Theme\ThemeNegotiator::determineActiveTheme() */ public function testDetermineActiveThemeWithPriority() { + $negotiators = []; + $negotiator = $this->getMock('Drupal\Core\Theme\ThemeNegotiatorInterface'); $negotiator->expects($this->once()) ->method('determineActiveTheme') @@ -88,7 +101,7 @@ public function testDetermineActiveThemeWithPriority() { ->method('applies') ->will($this->returnValue(TRUE)); - $this->themeNegotiator->addNegotiator($negotiator, 10); + $negotiators['test_negotiator_1'] = $negotiator; $negotiator = $this->getMock('Drupal\Core\Theme\ThemeNegotiatorInterface'); $negotiator->expects($this->never()) @@ -96,14 +109,18 @@ public function testDetermineActiveThemeWithPriority() { $negotiator->expects($this->never()) ->method('applies'); - $this->themeNegotiator->addNegotiator($negotiator, 0); + $negotiators['test_negotiator_2'] = $negotiator; + + foreach ($negotiators as $id => $negotiator) { + $this->container->set($id, $negotiator); + } $this->themeAccessCheck->expects($this->any()) ->method('checkAccess') ->will($this->returnValue(TRUE)); $route_match = new RouteMatch('test_route', new Route('/test-route'), array(), array()); - $theme = $this->themeNegotiator->determineActiveTheme($route_match); + $theme = $this->createThemeNegotiator(array_keys($negotiators))->determineActiveTheme($route_match); $this->assertEquals('example_test', $theme); } @@ -114,6 +131,8 @@ public function testDetermineActiveThemeWithPriority() { * @see \Drupal\Core\Theme\ThemeNegotiator::determineActiveTheme() */ public function testDetermineActiveThemeWithAccessCheck() { + $negotiators = []; + $negotiator = $this->getMock('Drupal\Core\Theme\ThemeNegotiatorInterface'); $negotiator->expects($this->once()) ->method('determineActiveTheme') @@ -122,7 +141,7 @@ public function testDetermineActiveThemeWithAccessCheck() { ->method('applies') ->will($this->returnValue(TRUE)); - $this->themeNegotiator->addNegotiator($negotiator, 10); + $negotiators['test_negotiator_1'] = $negotiator; $negotiator = $this->getMock('Drupal\Core\Theme\ThemeNegotiatorInterface'); $negotiator->expects($this->once()) @@ -132,7 +151,11 @@ public function testDetermineActiveThemeWithAccessCheck() { ->method('applies') ->will($this->returnValue(TRUE)); - $this->themeNegotiator->addNegotiator($negotiator, 0); + $negotiators['test_negotiator_2'] = $negotiator; + + foreach ($negotiators as $id => $negotiator) { + $this->container->set($id, $negotiator); + } $this->themeAccessCheck->expects($this->at(0)) ->method('checkAccess') @@ -145,7 +168,7 @@ public function testDetermineActiveThemeWithAccessCheck() { ->will($this->returnValue(TRUE)); $route_match = new RouteMatch('test_route', new Route('/test-route'), array(), array()); - $theme = $this->themeNegotiator->determineActiveTheme($route_match); + $theme = $this->createThemeNegotiator(array_keys($negotiators))->determineActiveTheme($route_match); $this->assertEquals('example_test2', $theme); } @@ -156,6 +179,8 @@ public function testDetermineActiveThemeWithAccessCheck() { * @see \Drupal\Core\Theme\ThemeNegotiatorInterface */ public function testDetermineActiveThemeWithNotApplyingNegotiator() { + $negotiators = []; + $negotiator = $this->getMock('Drupal\Core\Theme\ThemeNegotiatorInterface'); $negotiator->expects($this->never()) ->method('determineActiveTheme'); @@ -163,7 +188,7 @@ public function testDetermineActiveThemeWithNotApplyingNegotiator() { ->method('applies') ->will($this->returnValue(FALSE)); - $this->themeNegotiator->addNegotiator($negotiator, 10); + $negotiators['test_negotiator_1'] = $negotiator; $negotiator = $this->getMock('Drupal\Core\Theme\ThemeNegotiatorInterface'); $negotiator->expects($this->once()) @@ -173,16 +198,35 @@ public function testDetermineActiveThemeWithNotApplyingNegotiator() { ->method('applies') ->will($this->returnValue(TRUE)); - $this->themeNegotiator->addNegotiator($negotiator, 0); + $negotiators['test_negotiator_2'] = $negotiator; + + foreach ($negotiators as $id => $negotiator) { + $this->container->set($id, $negotiator); + } $this->themeAccessCheck->expects($this->any()) ->method('checkAccess') ->will($this->returnValue(TRUE)); $route_match = new RouteMatch('test_route', new Route('/test-route'), array(), array()); - $theme = $this->themeNegotiator->determineActiveTheme($route_match); + $theme = $this->createThemeNegotiator(array_keys($negotiators))->determineActiveTheme($route_match); $this->assertEquals('example_test2', $theme); } + /** + * Creates a new theme negotiator instance. + * + * @param array $negotiators + * An array of negotiator IDs. + * + * @return \Drupal\Core\Theme\ThemeNegotiator + */ + protected function createThemeNegotiator(array $negotiators) { + $theme_negotiator = new ThemeNegotiator($this->themeAccessCheck, $negotiators); + $theme_negotiator->setContainer($this->container); + + return $theme_negotiator; + } + }