diff --git a/core/lib/Drupal/Core/Extension/ModuleHandler.php b/core/lib/Drupal/Core/Extension/ModuleHandler.php index ad361d3fe6690f01dece0d6706d416dd60cd44df..fa60b827ad2775fb73cdfb79c582db6bd1167162 100644 --- a/core/lib/Drupal/Core/Extension/ModuleHandler.php +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.php @@ -8,6 +8,7 @@ use Drupal\Core\Extension\Exception\UnknownExtensionException; use Drupal\Core\Hook\Attribute\LegacyHook; use Drupal\Core\Hook\HookCollectorPass; +use Drupal\Core\Hook\ImplementationList; use Drupal\Core\Hook\OrderOperation\OrderOperation; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -61,27 +62,11 @@ class ModuleHandler implements ModuleHandlerInterface { protected $includeFileKeys = []; /** - * Lists of implementation callables by hook. + * Implementation lists by hook name. * - * @var array<string, list<callable>> + * @var array<string, \Drupal\Core\Hook\ImplementationList> */ - protected array $listenersByHook = []; - - /** - * Lists of module names by hook. - * - * The indices are exactly the same as in $listenersByHook. - * - * @var array<string, list<string>> - */ - protected array $modulesByHook = []; - - /** - * Hook and module keyed list of listeners. - * - * @var array<string, array<string, list<callable>>> - */ - protected array $invokeMap = []; + protected array $hookImplementationLists = []; /** * Ordering rules by hook name. @@ -224,8 +209,8 @@ public function addProfile($name, $path) { * The module path; e.g., 'core/modules/node'. * * @deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. - * There is no direct replacement. - * @see https://www.drupal.org/node/3491200 + * There is no direct replacement. + * @see https://www.drupal.org/node/3491200 */ protected function add($type, $name, $path) { $pathname = "$path/$name.info.yml"; @@ -242,15 +227,30 @@ protected function add($type, $name, $path) { // Load all includes so the legacy section of invoke can handle hooks in // includes. $hook_collector->loadAllIncludes(); - // Register procedural implementations. + // Register procedural implementations from the new module. + // This is done for BC reasons, but it has some limitations: + // - OOP implementations from the new module are not added. + // This is because the services won't be available yet. + // - New implementations are all appended to the list, no ordering is + // applied. + // - Procedural implementations that call \Drupal::service() may find the + // requested service to be missing, if the added module was not properly + // installed. + // @todo Calling ->add() suppresses implementations from other modules. + // See https://www.drupal.org/project/drupal/issues/3528899. foreach ($hook_collector->getImplementations() as $hook => $moduleImplements) { + $list = $this->hookImplementationLists[$hook] ?? NULL; + $listeners = $list?->listeners ?? []; + $modules = $list?->modules ?? []; foreach ($moduleImplements as $module => $classImplements) { foreach ($classImplements[ProceduralCall::class] ?? [] as $method) { - $this->listenersByHook[$hook][] = $method; - $this->modulesByHook[$hook][] = $module; - $this->invokeMap[$hook][$module][] = $method; + $listeners[] = $method; + $modules[] = $module; } } + if ($listeners) { + $this->hookImplementationLists[$hook] = new ImplementationList($listeners, $modules); + } } } @@ -333,25 +333,26 @@ public function getHookInfo() { */ public function resetImplementations() { $this->alterEventListeners = []; - $this->invokeMap = []; - $this->listenersByHook = []; - $this->modulesByHook = []; + $this->hookImplementationLists = []; } /** * {@inheritdoc} */ public function hasImplementations(string $hook, $modules = NULL): bool { - $implementation_modules = array_keys($this->getHookListeners($hook)); - return (bool) (isset($modules) ? array_intersect($implementation_modules, (array) $modules) : $implementation_modules); + $list = $this->getHookImplementationList($hook); + if ($modules === NULL) { + return $list->hasImplementations(); + } + return $list->hasImplementationsForModules((array) $modules); } /** * {@inheritdoc} */ public function invokeAllWith(string $hook, callable $callback): void { - foreach ($this->getFlatHookListeners($hook) as $index => $listener) { - $module = $this->modulesByHook[$hook][$index]; + $list = $this->getHookImplementationList($hook); + foreach ($list->iterateByModule() as $module => $listener) { $callback($listener, $module); } } @@ -360,7 +361,9 @@ public function invokeAllWith(string $hook, callable $callback): void { * {@inheritdoc} */ public function invoke($module, $hook, array $args = []) { - if ($listeners = $this->getHookListeners($hook)[$module] ?? []) { + $list = $this->getHookImplementationList($hook); + $listeners = $list->getForModule($module); + if ($listeners) { if (count($listeners) > 1) { throw new \LogicException("Module $module should not implement $hook more than once"); } @@ -437,7 +440,8 @@ public function invokeAllDeprecated($description, $hook, array $args = []) { * The name of the hook. */ private function triggerDeprecationError($description, $hook) { - $modules = array_keys($this->getHookListeners($hook)); + $list = $this->getHookImplementationList($hook); + $modules = array_unique($list->modules); if (!empty($modules)) { $message = 'The deprecated hook hook_' . $hook . '() is implemented in these modules: '; @trigger_error($message . implode(', ', $modules) . '. ' . $description, E_USER_DEPRECATED); @@ -484,25 +488,25 @@ public function alter($type, &$data, &$context1 = NULL, &$context2 = NULL) { */ protected function getCombinedListeners(array $hooks): array { // Get implementation lists for each hook. - $listener_lists = array_map($this->getFlatHookListeners(...), $hooks); + /** @var list<\Drupal\Core\Hook\ImplementationList> $lists */ + $lists = array_map($this->getHookImplementationList(...), $hooks); // Remove empty lists. - $listener_lists = array_filter($listener_lists); - if (!$listener_lists) { + /** @var array<int, \Drupal\Core\Hook\ImplementationList> $lists */ + $lists = array_filter($lists, fn (ImplementationList $list) => $list->hasImplementations()); + if (!$lists) { // No implementations exist. return []; } - if (array_keys($listener_lists) === [0]) { + if (array_keys($lists) === [0]) { // Only the first hook has implementations. - return $listener_lists[0]; + return $lists[0]->listeners; } // Collect the lists from each hook and group the listeners by module. $listeners_by_identifier = []; $modules_by_identifier = []; $identifiers_by_module = []; - foreach ($listener_lists as $i_hook => $listeners) { - $hook = $hooks[$i_hook]; - foreach ($listeners as $i_listener => $listener) { - $module = $this->modulesByHook[$hook][$i_listener]; + foreach ($lists as $list) { + foreach ($list->iterateByModule() as $module => $listener) { $identifier = is_array($listener) ? get_class($listener[0]) . '::' . $listener[1] : ProceduralCall::class . '::' . $listener; @@ -700,40 +704,24 @@ public function writeCache() { } /** - * Gets hook listeners by module. - * - * @param string $hook - * The name of the hook. - * - * @return array<string, list<callable>> - * A list of event listeners implementing this hook. - */ - protected function getHookListeners(string $hook): array { - if (!isset($this->invokeMap[$hook])) { - $this->invokeMap[$hook] = []; - foreach ($this->getFlatHookListeners($hook) as $index => $listener) { - $module = $this->modulesByHook[$hook][$index]; - $this->invokeMap[$hook][$module][] = $listener; - } - } - - return $this->invokeMap[$hook] ?? []; - } - - /** - * Gets a list of hook listener callbacks. + * Gets a hook implementation list for a specific hook. * * @param string $hook * The hook name. * - * @return list<callable> - * A list of hook implementation callables. - * - * @internal + * @return \Drupal\Core\Hook\ImplementationList + * Object with hook implementation callbacks and their modules. */ - protected function getFlatHookListeners(string $hook): array { - if (!isset($this->listenersByHook[$hook])) { - $this->listenersByHook[$hook] = []; + protected function getHookImplementationList(string $hook): ImplementationList { + if (!isset($this->hookImplementationLists[$hook])) { + if (isset($this->groupIncludes[$hook])) { + foreach ($this->groupIncludes[$hook] as $include) { + @trigger_error('Autoloading hooks in the file (' . $include . ') is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. Move the functions in this file to either the .module file or other appropriate location. See https://www.drupal.org/node/3489765', E_USER_DEPRECATED); + include_once $include; + } + } + $listeners = []; + $modules = []; foreach ($this->eventDispatcher->getListeners("drupal_hook.$hook") as $listener) { if (is_array($listener) && is_object($listener[0])) { $module = $this->hookImplementationsMap[$hook][get_class($listener[0])][$listener[1]]; @@ -746,20 +734,15 @@ protected function getFlatHookListeners(string $hook): array { $callable = $listener; } if (isset($this->moduleList[$module])) { - $this->listenersByHook[$hook][] = $callable; - $this->modulesByHook[$hook][] = $module; + $listeners[] = $callable; + $modules[] = $module; } } } - if (isset($this->groupIncludes[$hook])) { - foreach ($this->groupIncludes[$hook] as $include) { - @trigger_error('Autoloading hooks in the file (' . $include . ') is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. Move the functions in this file to either the .module file or other appropriate location. See https://www.drupal.org/node/3489765', E_USER_DEPRECATED); - include_once $include; - } - } + $this->hookImplementationLists[$hook] = new ImplementationList($listeners, $modules); } - return $this->listenersByHook[$hook]; + return $this->hookImplementationLists[$hook]; } } diff --git a/core/lib/Drupal/Core/Hook/ImplementationList.php b/core/lib/Drupal/Core/Hook/ImplementationList.php new file mode 100644 index 0000000000000000000000000000000000000000..0dee1d54642e5bd25355fbdb7b0f18ab82323795 --- /dev/null +++ b/core/lib/Drupal/Core/Hook/ImplementationList.php @@ -0,0 +1,90 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\Core\Hook; + +/** + * Contains the ordered list of implementations for a hook. + * + * Also contains a module name for each implementation. + * + * @internal + */ +class ImplementationList { + + /** + * Constructor. + * + * @param list<callable> $listeners + * List of hook implementation callbacks. + * @param list<string> $modules + * The module name associated with each hook implementation. + * This must have the same keys as $listeners. + */ + public function __construct( + public readonly array $listeners, + public readonly array $modules, + ) { + assert(array_is_list($listeners)); + assert(array_is_list($modules)); + assert(count($listeners) === count($modules)); + assert(array_filter($listeners, is_callable(...)) === $listeners); + assert(array_filter($modules, is_string(...)) === $modules, (new \Exception())->getTraceAsString()); + } + + /** + * Iterates over listeners, using module names as keys. + * + * @return \Iterator<string, callable> + * Iterator of listeners by module. + * This allows for the same module to occur more than once. + */ + public function iterateByModule(): \Iterator { + foreach ($this->listeners as $index => $listener) { + yield $this->modules[$index] => $listener; + } + } + + /** + * Gets listeners for a specific module. + * + * @param string $module + * Module name. + * + * @return list<callable> + * Listeners for that module. + */ + public function getForModule(string $module): array { + // Find indices where $this->modules[$i] === $module. + // Then get the listeners for these indices. + return array_values(array_intersect_key( + $this->listeners, + array_intersect($this->modules, [$module]), + )); + } + + /** + * Checks whether the list has any implementations. + * + * @return bool + * TRUE if it has implementations, FALSE if it is empty. + */ + public function hasImplementations(): bool { + return $this->listeners !== []; + } + + /** + * Checks whether the list has any implementations for specific modules. + * + * @param list<string> $modules + * Modules for which to check. + * + * @return bool + * TRUE if it has implementations for any of the modules, FALSE if not. + */ + public function hasImplementationsForModules(array $modules): bool { + return (bool) array_intersect($this->modules, $modules); + } + +} diff --git a/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerTest.php b/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerTest.php index 05a064eab7dfc9dae959613390f5275112d6f9c5..240d91ebe60430b41b89015d35ee2a5621234542 100644 --- a/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerTest.php +++ b/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerTest.php @@ -4,12 +4,13 @@ namespace Drupal\Tests\Core\Extension; +use Drupal\Core\Extension\Exception\UnknownExtensionException; use Drupal\Core\Extension\Extension; use Drupal\Core\Extension\ModuleHandler; -use Drupal\Core\Extension\Exception\UnknownExtensionException; use Drupal\Core\Extension\ProceduralCall; -use Drupal\Tests\UnitTestCase; +use Drupal\Core\Hook\ImplementationList; use Drupal\Tests\Core\GroupIncludesTestTrait; +use Drupal\Tests\UnitTestCase; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -48,13 +49,23 @@ protected function setUp(): void { * ModuleHandler objects as a property in unit tests. They must be generated * by the test method by calling this method. * + * @param list<string, string> $modules + * Module paths by module name. + * @param array<callable-string, string> $implementations + * Module names by function name implementing hook_hook(). + * @param array<callable-string, string> $includes_per_function + * Include files per function name. + * @param bool $loadAll + * TRUE to call ModuleHandler->loadAll() on the new module handler. + * * @return \Drupal\Core\Extension\ModuleHandler * The module handler to test. */ - protected function getModuleHandler($modules = [], $implementations = [], $loadAll = TRUE) { + protected function getModuleHandler($modules = [], $implementations = [], array $includes_per_function = [], $loadAll = TRUE) { // This only works if there's a single $hook. if ($implementations) { - $listeners = array_map(fn ($function) => [new ProceduralCall([]), $function], array_keys($implementations)); + $procedural_call = new ProceduralCall($includes_per_function); + $listeners = array_map(fn ($function) => [$procedural_call, $function], array_keys($implementations)); $this->eventDispatcher->expects($this->once()) ->method('getListeners') ->with("drupal_hook.hook") @@ -343,7 +354,10 @@ public function testImplementsHookModuleEnabled(): void { 'module_handler_test_added' => 'core/tests/Drupal/Tests/Core/Extension/modules/module_handler_test_added', 'module_handler_test_no_hook' => 'core/tests/Drupal/Tests/Core/Extension/modules/module_handler_test_no_hook', ]; - $module_handler = $this->getModuleHandler($moduleList, $implementations); + $includes_per_function = [ + 'module_handler_test_added_hook' => 'core/tests/Drupal/Tests/Core/Extension/modules/module_handler_test_added/module_handler_test_added.hook.inc', + ]; + $module_handler = $this->getModuleHandler($moduleList, $implementations, $includes_per_function); $this->assertTrue($module_handler->hasImplementations('hook', 'module_handler_test'), 'Installed module implementation found.'); $this->assertTrue($module_handler->hasImplementations('hook', 'module_handler_test_added'), 'Runtime added module with implementation in include found.'); @@ -374,7 +388,6 @@ public function testInvokeAll(): void { /** * Tests hasImplementations. * - * @covers ::getHookListeners * @covers ::hasImplementations */ public function testHasImplementations(): void { @@ -389,6 +402,7 @@ function some_method(): void { $module_handler = new ModuleHandler($this->root, [], $this->eventDispatcher, $implementations); $module_handler->setModuleList(['some_module' => TRUE]); $r = new \ReflectionObject($module_handler); + $get_lists = fn () => $r->getProperty('hookImplementationLists')->getValue($module_handler); // Set up some synthetic results. $this->eventDispatcher @@ -398,15 +412,11 @@ function some_method(): void { ->willReturn([ [$c, 'some_method'], ]); - $this->assertNotEmpty($module_handler->hasImplementations('some_hook')); - - $listeners = $r->getProperty('invokeMap')->getValue($module_handler)['some_hook']['some_module']; - // Anonymous class doesn't work with assertSame() so assert it piecemeal. - $this->assertSame(count($listeners), 1); - foreach ($listeners as $listener) { - $this->assertSame(get_class($c), get_class($listener[0])); - $this->assertSame('some_method', $listener[1]); - } + $this->assertSame([], $get_lists()); + $this->assertTrue($module_handler->hasImplementations('some_hook')); + $this->assertEquals([ + 'some_hook' => new ImplementationList([[$c, 'some_method']], ['some_module']), + ], $get_lists()); } /** @@ -425,8 +435,6 @@ public function testGetModuleDirectories(): void { } /** - * @covers ::getHookListeners - * * @group legacy */ public function testGroupIncludes(): void { diff --git a/core/tests/Drupal/Tests/Core/Hook/ImplementationListTest.php b/core/tests/Drupal/Tests/Core/Hook/ImplementationListTest.php new file mode 100644 index 0000000000000000000000000000000000000000..da4d8ee83eb2eaa96b384903cbfce0e2d2884b8b --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Hook/ImplementationListTest.php @@ -0,0 +1,94 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\Tests\Core\Hook; + +use Drupal\Core\Hook\ImplementationList; +use Drupal\Tests\UnitTestCase; + +/** + * @coversDefaultClass \Drupal\Core\Hook\ImplementationList + * @group Hook + */ +class ImplementationListTest extends UnitTestCase { + + /** + * Log for include files. + * + * @var list<string> + */ + public static array $log = []; + + /** + * Tests public methods on a common instance. + * + * This is easier than separate test methods. + */ + public function testPublicMethods(): void { + $listeners = [ + fn () => 'a0', + fn () => 'b', + fn () => 'a1', + ]; + $modules = [ + 'module_a', + 'module_b', + // Repeat the same module. + 'module_a', + ]; + $list = new ImplementationList($listeners, $modules); + + // Test public properties. + $this->assertSame($listeners, $list->listeners); + $this->assertSame($modules, $list->modules); + + // Test iterateByModule(). + $i = 0; + foreach ($list->iterateByModule() as $module => $listener) { + $this->assertSame($modules[$i], $module); + $this->assertSame($listeners[$i], $listener); + ++$i; + } + + // Test getForModule(). + $this->assertSame([], $list->getForModule('other_module')); + $this->assertSame([$listeners[0], $listeners[2]], $list->getForModule('module_a')); + $this->assertSame([$listeners[1]], $list->getForModule('module_b')); + + // Test hasImplementations(). + $this->assertTrue($list->hasImplementations()); + + // Test hasImplementationsForModules(). + $this->assertFalse($list->hasImplementationsForModules(['other_module'])); + $this->assertFalse($list->hasImplementationsForModules([])); + $this->assertTrue($list->hasImplementationsForModules(['other_module', 'module_a'])); + $this->assertTrue($list->hasImplementationsForModules(['module_b'])); + } + + /** + * Tests public methods on an empty list. + */ + public function testEmptyList(): void { + $list = new ImplementationList([], []); + + // Test public properties. + $this->assertSame([], $list->listeners); + $this->assertSame([], $list->modules); + + // Test iterateByModule(). + $iterator = $list->iterateByModule(); + $this->assertFalse($iterator->valid()); + + // Test hasImplementations(). + $this->assertFalse($list->hasImplementations()); + + // Test getForModule(). + $this->assertSame([], $list->getForModule('any_module')); + + // Test hasImplementationsForModules(). + $this->assertFalse($list->hasImplementationsForModules(['any_module'])); + $this->assertFalse($list->hasImplementationsForModules([])); + } + +}