Problem/Motivation
#3485896: Hook ordering across OOP, procedural and with extra types i.e replace hook_module_implements_alter Introduces a modern way to order hooks.
This does not handle the edge case when a module inserts itself between two implementations of another module.
Currently in ModuleHandler::getHookListeners(), we are grouping implementations by module $this->invokeMap[$hook][$module][] = $callable;
Steps to reproduce
Proposed resolution
Because ::invoke() needs to find implementations by module, this structure should remain in place however to fix the map another one like $this->implementationsMap[$hook][] = ['module' => $module, 'callable' => $callable]; should be added. The check whether invoke can or can not run should be in yet another property also gathered in getHookListeneres: $this->hasMultipleImplementationsPerModuleAndHook[$hook][$module] = TRUE and if it's set at all then invoke should die as it does now. It means "invoke all" and "invoke" now relies on separate structures but oh well.
Remaining tasks
Change getHookListeners:
if (isset($this->moduleList[$module])) {
if (isset($this->invokeMap[$hook][$module])) {
$this->hasMultipleImplementationsPerHookAndModule[$hook][$module] = TRUE:
}
$this->invokeMap[$hook][$module][] = $callable;
$this->hookImplementations[$hook][] = [$callable, $module];
}
...
return $this->hookImplementations[$hook];
Add
protected function getHookListenersByHookAndModule($hook, $module = NULL) {
$this->getHookListeners($hook);
if (isset($module)) {
if (isset($this->hasMultipleImplementationsPerHookAndModule[$hook][$module])) {
throw new MultipleImplementationsException;
}
return $this->invokeMap[$hook][$module];
}
if (isset($this->hasMultipleImplementationsPerHookAndModule[$hook])) {
throw new MultipleImplementationsException;
}
return $this->invokeMap[$hook];
}
Change hasImplementations, invoke, triggerDeprecationError to use getHookListenersByHookAndModule instead, they should catch the exception and act appropriately -- only invoke cares. Upgrade invokeAllWith appropriately: foreach ($this->getHookListeners($hook) as [$listener, $module]) and remove the second loop. alter is fun. While the second call to getHookListeners is a loop which is trivial to upgrade like the previous one, the initial call is not a loop, it should be made into one but only when needed which will be like never:
try {
$hook_listeners = $this->getHookListenersByModule($hook);
}
catch (MultipleImplementationsException) {
$hook_listeners = [];
foreach ($this->getHookListeners($hook) as [$listener, $module]) {
$hook_listeners[$module][] = $listener;
}
}
Comments
Comment #2
ghost of drupal pastComment #3
nicxvan commentedComment #4
nicxvan commented