Resolve #3506930 "Hookcollectorpass registers event"
Closes #3506930
Merge request reports
Activity
added 1 commit
- f14245bd - Issue 3506930: Stop using event dispatcher for hooks.
added 12 commits
- f14245bd...9f932fce - 2 earlier commits
- cf2b7caf - Introduce ImplementationList.
- 1fb77b2a - Drop usage of ->getHookListeners() in ModuleHandler methods.
- f0c583ab - Remove ModuleHandler::getHookListeners() and ImplementationList::groupByModule().
- d3fd1acd - Include files before getting the listeners, in getHookImplementationList().
- 9bc0342e - Assert that listeners in ImplementationList are callable, and update test.
- cf90e20a - Replace HookCollectorPass::getImplementations() with ::getAddableImplementations().
- 784d182f - Issue 3506930: Stop using event dispatcher for hooks.
- da6ddb27 - Pass ImplementationList service closures to ModuleHandler.
- f47c0814 - Handle group includes in ImplementationList static factory.
- 11dbd74d - Test an empty hook in ModuleHandlerTest::testHasImplementations().
Toggle commit listadded 4 commits
Toggle commit listadded 5 commits
- b4bf6b45 - Pass ImplementationList service closures to ModuleHandler.
- 75f13549 - Handle group includes in ImplementationList static factory.
- b2c9b6b0 - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- 42a7e72e - Fix circular dependency problem.
- d67ba7e4 - Add a test case for the circular dependency scenario.
Toggle commit listadded 1 commit
- 428558fc - Add a test case for the circular dependency scenario.
added 5 commits
- 123522d9 - Pass ImplementationList service closures to ModuleHandler.
- 679ca4a4 - Handle group includes in ImplementationList static factory.
- bbb461fa - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- c03d5949 - Fix circular dependency problem.
- 41aaec26 - Add a test case for the circular dependency scenario.
Toggle commit listadded 10 commits
- a2b3a0a8 - Assert that listeners in ImplementationList are callable, and update test.
- d9161d19 - Replace HookCollectorPass::getImplementations() with ::getAddableImplementations().
- c58cee48 - Stop using event dispatcher for hooks, inject container in ModuleHandler.
- d1c40276 - Pass ImplementationList service closures to ModuleHandler.
- 987d03e3 - Handle group includes in ImplementationList static factory.
- db73b729 - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- b5ff4934 - Fix circular dependency problem.
- c7aa77a5 - Add a test case for the circular dependency scenario.
- 22d0163e - Move includes handling out of ProceduralCall service.
- ce80498d - Fully remove ProceduralCall.
Toggle commit listadded 8 commits
- 53e91cd1 - Pass ImplementationList service closures to ModuleHandler.
- 59434af7 - Handle group includes in ImplementationList static factory.
- 9925b4f3 - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- bd9ac5da - Register hook list services explicitly, to fix circular dependency problem.
- 588c531e - The hook list service definitions must be public.
- d084dad3 - Add a test case for the circular dependency scenario.
- 209da8ea - Move includes handling out of ProceduralCall service.
- 25730c9f - Fully remove ProceduralCall.
Toggle commit listadded 39 commits
-
25730c9f...09633691 - 22 commits from branch
project:11.x - 09633691...a4b022d1 - 7 earlier commits
- f407d231 - Replace HookCollectorPass::getImplementations() with ::getAddableImplementations().
- 25678d99 - Stop using event dispatcher for hooks, inject container in ModuleHandler.
- 29a8a7b7 - Pass ImplementationList service closures to ModuleHandler.
- 0d3330a1 - Handle group includes in ImplementationList static factory.
- faf83def - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- 6caaa3d2 - Register hook list services explicitly, to fix circular dependency problem.
- 3719aee2 - The hook list service definitions must be public.
- 33cf5108 - Add a test case for the circular dependency scenario.
- 31c025d7 - Move includes handling out of ProceduralCall service.
- 8a799a54 - Fully remove ProceduralCall.
Toggle commit list-
25730c9f...09633691 - 22 commits from branch
added 10 commits
- 1c1da439 - A functions callback does not need a leading backslash.
- 412df06e - Stop using event dispatcher for hooks, inject container in ModuleHandler.
- e0bbafa1 - Pass ImplementationList service closures to ModuleHandler.
- 3d6b87e5 - Handle group includes in ImplementationList static factory.
- 216f2b3f - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- 0ee4f910 - Register hook list services explicitly, to fix circular dependency problem.
- 698749fe - The hook list service definitions must be public.
- b672de3d - Add a test case for the circular dependency scenario.
- 270dd65e - Move includes handling out of ProceduralCall service.
- 67a1cd9a - Fully remove ProceduralCall.
Toggle commit listadded 9 commits
- 727cf2d9 - Refine a property doc.
- e9f2277f - A functions callback does not need a leading backslash.
- f98d41d5 - Replace HookCollectorPass::getImplementations() with ::getAddableImplementations().
- 508de331 - Stop using event dispatcher for hooks, inject container in ModuleHandler.
- b94e7e49 - Pass ImplementationList service closures to ModuleHandler.
- d767cbee - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- 1b6c7786 - Add a test case for the circular dependency scenario.
- 88a2890c - Move includes handling out of ProceduralCall service.
- 2e655b34 - Fully remove ProceduralCall.
Toggle commit listadded 9 commits
- 7097ca8e - Reorder imports.
- 517e09a1 - A functions callback does not need a leading backslash.
- ea6ddca8 - Replace HookCollectorPass::getImplementations() with ::getAddableImplementations().
- 07512ddc - Stop using event dispatcher for hooks, inject container in ModuleHandler.
- 0b1b488d - Pass ImplementationList service closures to ModuleHandler.
- e2e423fc - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- 1ab384a0 - Add a test case for the circular dependency scenario.
- a3261765 - Move includes handling out of ProceduralCall service.
- c2adf40e - Fully remove ProceduralCall.
Toggle commit listadded 5 commits
- 7d42afbc - Pass ImplementationList service closures to ModuleHandler.
- 05c17ba9 - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- ee4fdbc3 - Add a test case for the circular dependency scenario.
- 2e384022 - Move includes handling out of ProceduralCall service.
- b526c9b3 - Fully remove ProceduralCall.
Toggle commit listadded 5 commits
- 50024f90 - Pass ImplementationList service closures to ModuleHandler.
- 97829b58 - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- 1d4926f8 - Add a test case for the circular dependency scenario.
- c19583a9 - Move includes handling out of ProceduralCall service.
- 5e869a57 - Fully remove ProceduralCall.
Toggle commit listadded 6 commits
- 48014191 - Pass ImplementationList service closures to ModuleHandler.
- 9e733880 - Test an empty hook in ModuleHandlerTest::testHasImplementations().
- 31b55325 - Add a test case for the circular dependency scenario.
- 8454fa68 - Move includes handling out of ProceduralCall service.
- ad3766d1 - Fully remove ProceduralCall.
- 0f9e4450 - Assert that modules are strings in ImplementationList.php.
Toggle commit list218 * @param array<string, list<string>> $includes 219 * Regular includes. 220 * @param array<string, list<string>> $groupIncludes 221 * Group includes. 222 * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container 223 * The container builder. 224 * 225 * @return array<string, \Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument> 226 * Map of service closure arguments, keyed by hook. 227 */ 228 protected function buildHookListServiceClosures(array $implementationsByHook, array $includes, array $groupIncludes, ContainerBuilder $container): array { 229 $hookListClosures = []; 230 foreach ($implementationsByHook as $hook => $hookImplementations) { 231 $listeners = []; 232 foreach ($hookImplementations as $identifier => $module) { 233 [$class_or_function, $method_or_null] = explode('::', $identifier, 2) + [1 => NULL]; This reads strange to me. You are adding NULL and then testing whether you added NULL.
// The identifier can be class name or // a class-method pair separated by ::. $parts = explode('::', $identifier, 2); if (count($parts) === 1) { $listeners[] = $parts[0]; } else { $listeners[] = [new Reference($parts[0]), $parts[1]]; }changed this line in version 21 of the diff
- Resolved by Andreas Hennings
242 $arguments = [ 243 $listeners, 244 $modules, 245 $includes[$hook] ?? [], 246 $groupIncludes[$hook] ?? [], 247 ]; 248 // Omit trailing arguments with default value. 249 if ($arguments[3] === []) { 250 unset($arguments[3]); 251 if ($arguments[2] === []) { 252 unset($arguments[2]); 253 } 254 } 255 $definition = (new Definition(ImplementationList::class)) 256 ->setFactory([ImplementationList::class, 'load']) 257 ->setPublic(TRUE) A comment like
// This definition will be used in a service closure which are resolved run time and private services are only available compile time.I think would be helpful here.
Edited by ghost of drupal past
It was needed because in core/lib/Drupal/Component/DependencyInjection/Container.php:452 we are getting the service with
$container->get().We can investigate more to provide correct explanation.
What you suggest would generally be true but I am not sure it is the root of the problem in this specific case.
changed this line in version 20 of the diff
Please register or sign in to reply