diff --git a/admin/classes/table/hook_list_table.php b/admin/classes/table/hook_list_table.php index 2dd6966455d..cbe9cb7ebef 100644 --- a/admin/classes/table/hook_list_table.php +++ b/admin/classes/table/hook_list_table.php @@ -92,7 +92,7 @@ class hook_list_table extends flexible_table { public function out(): void { // All hook consumers referenced from the db/hooks.php files. $hookmanager = \core\hook\manager::get_instance(); - $allhooks = $hookmanager->get_all_callbacks(); + $allhooks = (array)$hookmanager->get_all_callbacks(); // Add any unused hooks. foreach (array_keys($this->emitters) as $classname) { @@ -102,6 +102,17 @@ class hook_list_table extends flexible_table { $allhooks[$classname] = []; } + // Order rows by hook name, putting core first. + \core_collator::ksort($allhooks); + $corehooks = []; + foreach ($allhooks as $classname => $consumers) { + if (str_starts_with($classname, 'core\\')) { + $corehooks[$classname] = $consumers; + unset($allhooks[$classname]); + } + } + $allhooks = array_merge($corehooks, $allhooks); + foreach ($allhooks as $classname => $consumers) { $this->add_data_keyed( $this->format_row((object) [ diff --git a/lib/classes/hook/deprecated_callback_replacement.php b/lib/classes/hook/deprecated_callback_replacement.php index a2cae6f2795..40596673707 100644 --- a/lib/classes/hook/deprecated_callback_replacement.php +++ b/lib/classes/hook/deprecated_callback_replacement.php @@ -17,7 +17,7 @@ namespace core\hook; /** - * Interface for hook callbacks that were deprecated by the hook. + * Interface for describing of lib.php callbacks that were deprecated by the hook. * * @package core * @author Petr Skoda diff --git a/lib/classes/hook/described_hook.php b/lib/classes/hook/described_hook.php index 484a4a44884..27d14dd847e 100644 --- a/lib/classes/hook/described_hook.php +++ b/lib/classes/hook/described_hook.php @@ -26,7 +26,7 @@ namespace core\hook; */ interface described_hook { /** - * Mandatory hook purpose description in Markdown format + * Hook purpose description in Markdown format * used on Hooks overview page. * * It should include description of callback priority setting diff --git a/lib/classes/hook/discovery_agent.php b/lib/classes/hook/discovery_agent.php index 3cbdb6d2ae2..f07d433b520 100644 --- a/lib/classes/hook/discovery_agent.php +++ b/lib/classes/hook/discovery_agent.php @@ -17,7 +17,11 @@ namespace core\hook; /** - * This interface describes a component which can discover hooks in its own namespace. + * This interface describes a class which can discover all hook + * classes of a plugin. + * + * To add new discovery agent in your plugin you need to add your_plugin\hooks + * class that implements this interface. * * @package core * @copyright Andrew Lyons @@ -25,7 +29,7 @@ namespace core\hook; */ interface discovery_agent { /** - * Discover hooks belonging to the component. + * Returns a list of hooks for component. * * @return array */ diff --git a/lib/classes/hook/manager.php b/lib/classes/hook/manager.php index 4bc1140b442..ca275516f12 100644 --- a/lib/classes/hook/manager.php +++ b/lib/classes/hook/manager.php @@ -89,37 +89,11 @@ final class manager implements return $instance; } - /** - * Reset all hook caches. This is intended to be called only - * from the admin/hooks.php page after callback override is changed. - * - * @return void - * @codeCoverageIgnore - */ - public function reset_caches(): void { - if (PHPUNIT_TEST && $this === self::$instance) { - debugging('\core\hook\manager::get_instance()->reset_caches() is not supposed to be called in PHPUnit tests', - DEBUG_DEVELOPER); - return; - } - - // WARNING: This will not work when callback overrides are changed - // and multiple web nodes with local cache stores are present - in that - // case admins must purge all caches when tweaking callback overrides. - $cache = \cache::make('core', 'hookcallbacks'); - $cache->delete('callbacks'); - $cache->delete('deprecations'); - - $this->init_standard_callbacks(); - } - /** * Returns list of callbacks for given hook name. * * NOTE: this is the "Listener Provider" described in PSR-14, * instead of instance parameter it uses real PHP class names. - * Moodle hooks should be final and parents of hook class are not - * considered when resolving callbacks. * * @param string $hookclassname PHP class name of hook * @return array list of callback definitions @@ -444,10 +418,7 @@ final class manager implements if (!class_exists($hookclassname)) { continue; } - // It's 2023 and PHP still doesn't provide a simple way to detect if a class implements an interface without - // that class being instantiated. - $rc = new \ReflectionClass($hookclassname); - if (!$rc->implementsInterface(\core\hook\deprecated_callback_replacement::class)) { + if (!is_subclass_of($hookclassname, \core\hook\deprecated_callback_replacement::class)) { continue; } $deprecations = $hookclassname::get_deprecated_plugin_callbacks(); @@ -582,18 +553,19 @@ final class manager implements } /** - * Returns list of hooks discovered through standardised Moodle methods. + * Returns list of hooks discovered through hook namespaces or discovery agents. * - * Note that the exact discovery logic may change in the future, - * for now this looks for hooks mentioned in callback registrations - * and non-abstract classes in \component_name\hook namespaces that - * implement described_hook interface. + * The hooks overview page includes also all other classes that are + * referenced in callback registrations in db/hooks.php files, those + * are not included here. * * @return array hook class names */ public static function discover_known_hooks(): array { + // All classes in hook namespace of core and plugins, unless plugin has a discovery agent. $hooks = \core\hooks::discover_hooks(); + // Look for hooks classes in all plugins that implement discovery agent interface. foreach (\core_component::get_component_names() as $component) { $classname = "{$component}\\hooks"; @@ -601,8 +573,7 @@ final class manager implements continue; } - $rc = new \ReflectionClass($classname); - if (!$rc->implementsInterface(\core\hook\hook_discover_agent::class)) { + if (!is_subclass_of($classname, discovery_agent::class)) { continue; } diff --git a/lib/classes/hooks.php b/lib/classes/hooks.php index 5edb4846fa9..81fc92aa9db 100644 --- a/lib/classes/hooks.php +++ b/lib/classes/hooks.php @@ -17,22 +17,45 @@ namespace core; /** - * Hook discovery agent for core. + * Standard hook discovery agent for Moodle which lists + * all non-abstract classes in hooks namespace of core and all plugins + * unless there is a hook discovery agent in a plugin. * * @package core * @copyright Andrew Lyons * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class hooks implements \core\hook\discovery_agent { +final class hooks implements \core\hook\discovery_agent { + /** + * Returns all Moodle hooks in standard hook namespace. + * + * @return array list of hook classes + */ public static function discover_hooks(): array { - // Describe any hard-coded hooks which can't be easily discovered by namespace. + // Look for hooks in hook namespace in core and all components. $hooks = []; $hooks = array_merge($hooks, self::discover_hooks_in_namespace('core', 'hook')); + foreach (\core_component::get_component_names() as $component) { + $agent = "$component\\hooks"; + if (class_exists($agent) && is_subclass_of($agent, hook\discovery_agent::class)) { + // Let the plugin supply the list of hooks instead. + continue; + } + $hooks = array_merge($hooks, self::discover_hooks_in_namespace($component, 'hook')); + } + return $hooks; } + /** + * Look up all non-abstract classes in "$component\$namespace" namespace. + * + * @param string $component + * @param string $namespace + * @return array list of hook classes + */ public static function discover_hooks_in_namespace(string $component, string $namespace): array { $classes = \core_component::get_component_classes_in_namespace($component, $namespace); @@ -44,8 +67,8 @@ class hooks implements \core\hook\discovery_agent { continue; } - if (is_a($classname, \core\hook\manager::class, true)) { - // Skip the manager. + if ($classname === \core\hook\manager::class) { + // Skip the manager in core. continue; } @@ -55,11 +78,9 @@ class hooks implements \core\hook\discovery_agent { 'tags' => [], ]; - if ($rc->implementsInterface(\core\hook\described_hook::class)) { + if (is_subclass_of($classname, \core\hook\described_hook::class)) { $hooks[$classname]['description'] = $classname::get_hook_description(); } - - } return $hooks; diff --git a/lib/tests/hook/manager_test.php b/lib/tests/hook/manager_test.php index 70f702e761e..e6f2674cd2a 100644 --- a/lib/tests/hook/manager_test.php +++ b/lib/tests/hook/manager_test.php @@ -56,24 +56,6 @@ class manager_test extends \advanced_testcase { $this->assertSame(['test_plugin\\hook\\hook'], $testmanager->get_hooks_with_callbacks()); } - /** - * Test reset of test instance. - * - * NOTE: normal hook manger instance cannot be reset in PHPUnit test - * because it may be used to control the test environment itself. - * - * @covers ::reset_caches - * @covers ::init_standard_callbacks - */ - public function test_reset_caches() { - $testmanager = manager::phpunit_get_instance([]); - $this->assertSame([], $testmanager->get_hooks_with_callbacks()); - - $testmanager->reset_caches(); - $manager = manager::get_instance(); - $this->assertSame($manager->get_hooks_with_callbacks(), $testmanager->get_hooks_with_callbacks()); - } - /** * Test loading and parsing of callbacks from files. *