From 2b49ad42f556e1cb853b2aa7a6c7eef7fd92ffb2 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 7 Apr 2024 22:33:30 +0800 Subject: [PATCH] MDL-81456 core: Use DI for all hook access Using DI for all hook access means that it becomes significantly easier to mock hooks and callbacks for unit testing without fundamentally altering the structure of the code purely for the purposes of unit testing. --- admin/classes/table/hook_list_table.php | 2 +- admin/hooks.php | 2 +- admin/tool/mfa/classes/manager.php | 2 +- admin/user/user_bulk_forms.php | 2 +- .../util/helper/backup_cron_helper.class.php | 2 +- course/edit_form.php | 11 ++++++--- course/format/classes/local/cmactions.php | 5 ++-- course/lib.php | 21 ++++++++++------ course/modlib.php | 15 ++++++++---- lib/classes/navigation/views/primary.php | 2 +- lib/moodlelib.php | 24 ++++++++++++------- lib/phpunit/bootstrap.php | 1 + lib/phpunit/classes/advanced_testcase.php | 9 +++++-- lib/phpunit/classes/util.php | 7 +++++- mod/quiz/classes/grade_calculator.php | 6 ++++- mod/quiz/classes/quiz_attempt.php | 12 ++++++---- mod/quiz/locallib.php | 8 +++++-- 17 files changed, 89 insertions(+), 42 deletions(-) diff --git a/admin/classes/table/hook_list_table.php b/admin/classes/table/hook_list_table.php index 1ffc092708b..7f14bb73a83 100644 --- a/admin/classes/table/hook_list_table.php +++ b/admin/classes/table/hook_list_table.php @@ -91,7 +91,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(); + $hookmanager = \core\di::get(\core\hook\manager::class); $allhooks = (array)$hookmanager->get_all_callbacks(); // Add any unused hooks. diff --git a/admin/hooks.php b/admin/hooks.php index e132b65525d..9cca9da9570 100644 --- a/admin/hooks.php +++ b/admin/hooks.php @@ -30,7 +30,7 @@ require_once($CFG->libdir . '/tablelib.php'); admin_externalpage_setup('hooksoverview'); require_capability('moodle/site:config', \core\context\system::instance()); -$hookmanager = \core\hook\manager::get_instance(); +$hookmanager = \core\di::get(\core\hook\manager::class); echo $OUTPUT->header(); echo $OUTPUT->heading(get_string('hooksoverview', 'core_admin')); diff --git a/admin/tool/mfa/classes/manager.php b/admin/tool/mfa/classes/manager.php index c9a0454dda5..d8d093c44f1 100644 --- a/admin/tool/mfa/classes/manager.php +++ b/admin/tool/mfa/classes/manager.php @@ -334,7 +334,7 @@ class manager { // Allow plugins to callback as soon possible after user has passed MFA. $hook = new \tool_mfa\hook\after_user_passed_mfa(); - \core\hook\manager::get_instance()->dispatch($hook); + \core\di::get(\core\hook\manager::class)->dispatch($hook); // Add/update record in DB for users last mfa auth. self::update_pass_time(); diff --git a/admin/user/user_bulk_forms.php b/admin/user/user_bulk_forms.php index 0b36c941d15..1a1604e225a 100644 --- a/admin/user/user_bulk_forms.php +++ b/admin/user/user_bulk_forms.php @@ -130,7 +130,7 @@ class user_bulk_action_form extends moodleform { } // Any plugin can append user bulk actions to this list by implementing a hook callback. - \core\hook\manager::get_instance()->dispatch($hook); + \core\di::get(\core\hook\manager::class)->dispatch($hook); // This method may be called from 'Bulk actions' and 'Browse user list' pages. Some actions // may be irrelevant in one of the contexts and they can be excluded by specifying the diff --git a/backup/util/helper/backup_cron_helper.class.php b/backup/util/helper/backup_cron_helper.class.php index 2674f5d00c6..ecd1617fa7e 100644 --- a/backup/util/helper/backup_cron_helper.class.php +++ b/backup/util/helper/backup_cron_helper.class.php @@ -797,7 +797,7 @@ abstract class backup_cron_automated_helper { // Exclude events defined by hook. $hook = new \core_backup\hook\before_course_modified_check(); - \core\hook\manager::get_instance()->dispatch($hook); + \core\di::get(\core\hook\manager::class)->dispatch($hook); foreach ($readers as $readerpluginname => $reader) { $params = [ diff --git a/course/edit_form.php b/course/edit_form.php index 5a60a5604f4..2e60b3a9ab9 100644 --- a/course/edit_form.php +++ b/course/edit_form.php @@ -1,5 +1,10 @@ libdir.'/formslib.php'); @@ -419,7 +424,7 @@ class course_edit_form extends moodleform { $handler->instance_form_definition($mform, empty($course->id) ? 0 : $course->id); $hook = new \core_course\hook\after_form_definition($this, $mform); - \core\hook\manager::get_instance()->dispatch($hook); + di::get(hook\manager::class)->dispatch($hook); // When two elements we need a group. $buttonarray = array(); @@ -502,7 +507,7 @@ class course_edit_form extends moodleform { $handler->instance_form_definition_after_data($mform, empty($courseid) ? 0 : $courseid); $hook = new \core_course\hook\after_form_definition_after_data($this, $mform); - \core\hook\manager::get_instance()->dispatch($hook); + di::get(hook\manager::class)->dispatch($hook); } /** @@ -550,7 +555,7 @@ class course_edit_form extends moodleform { $errors = array_merge($errors, $handler->instance_form_validation($data, $files)); $hook = new \core_course\hook\after_form_validation($this, $data, $files); - \core\hook\manager::get_instance()->dispatch($hook); + di::get(hook\manager::class)->dispatch($hook); $pluginerrors = $hook->get_errors(); if (!empty($pluginerrors)) { $errors = array_merge($errors, $pluginerrors); diff --git a/course/format/classes/local/cmactions.php b/course/format/classes/local/cmactions.php index 347b2a31a5e..3e9f1e54c69 100644 --- a/course/format/classes/local/cmactions.php +++ b/course/format/classes/local/cmactions.php @@ -72,8 +72,9 @@ class cmactions extends baseactions { // Modules may add some logic to renaming. $modinfo = get_fast_modinfo($cm->course); - $hook = new \core_courseformat\hook\after_cm_name_edited($modinfo->get_cm($cm->id), $name); - \core\hook\manager::get_instance()->dispatch($hook); + \core\di::get(\core\hook\manager::class)->dispatch( + new \core_courseformat\hook\after_cm_name_edited($modinfo->get_cm($cm->id), $name), + ); // Attempt to update the grade item if relevant. $grademodule = $DB->get_record($cm->modname, ['id' => $cm->instance]); diff --git a/course/lib.php b/course/lib.php index 0d43762d618..a4a5a4da1ba 100644 --- a/course/lib.php +++ b/course/lib.php @@ -24,6 +24,10 @@ defined('MOODLE_INTERNAL') || die; +use core\{ + di, + hook, +}; use core_course\external\course_summary_exporter; use core_courseformat\base as course_format; use core_courseformat\formatactions; @@ -2032,10 +2036,11 @@ function create_course($data, $editoroptions = NULL) { $data->id = $newcourseid; // Dispatch the hook for post course create actions. - $hook = new \core_course\hook\after_course_created( - course: $data, + di::get(hook\manager::class)->dispatch( + new \core_course\hook\after_course_created( + course: $data, + ), ); - \core\di::get(\core\hook\manager::class)->dispatch($hook); // Setup the blocks blocks_add_default_course_blocks($course); @@ -2068,8 +2073,9 @@ function create_course($data, $editoroptions = NULL) { $data->id = $course->id; $handler->instance_form_save($data, true); - $hook = new \core_course\hook\after_form_submission($data, true); - \core\hook\manager::get_instance()->dispatch($hook); + di::get(hook\manager::class)->dispatch( + new \core_course\hook\after_form_submission($data, true), + ); return $course; } @@ -2186,8 +2192,9 @@ function update_course($data, $editoroptions = NULL) { $handler = core_course\customfield\course_handler::create(); $handler->instance_form_save($data); - $hook = new \core_course\hook\after_form_submission($data); - \core\hook\manager::get_instance()->dispatch($hook); + di::get(hook\manager::class)->dispatch( + new \core_course\hook\after_form_submission($data), + ); // Update with the new data $DB->update_record('course', $data); diff --git a/course/modlib.php b/course/modlib.php index 8b99058b60e..9ec24d70cb0 100644 --- a/course/modlib.php +++ b/course/modlib.php @@ -27,8 +27,12 @@ defined('MOODLE_INTERNAL') || die; -use \core_grades\component_gradeitems; +use core\{ + di, + hook, +}; use core_courseformat\formatactions; +use core_grades\component_gradeitems; require_once($CFG->dirroot.'/course/lib.php'); @@ -725,11 +729,12 @@ function update_moduleinfo($cm, $moduleinfo, $course, $mform = null) { } if ($cm->name != $moduleinfo->name) { - $hook = new \core_courseformat\hook\after_cm_name_edited( - get_fast_modinfo($course)->get_cm($cm->id), - $moduleinfo->name + di::get(hook\manager::class)->dispatch( + new \core_courseformat\hook\after_cm_name_edited( + get_fast_modinfo($course)->get_cm($cm->id), + $moduleinfo->name + ), ); - \core\hook\manager::get_instance()->dispatch($hook); } $cm->name = $moduleinfo->name; diff --git a/lib/classes/navigation/views/primary.php b/lib/classes/navigation/views/primary.php index c8d9402030f..f02bf3ebf34 100644 --- a/lib/classes/navigation/views/primary.php +++ b/lib/classes/navigation/views/primary.php @@ -82,7 +82,7 @@ class primary extends view { // Allow plugins to add nodes to the primary navigation. $hook = new \core\hook\navigation\primary_extend($this); - \core\hook\manager::get_instance()->dispatch($hook); + \core\di::get(\core\hook\manager::class)->dispatch($hook); // Search and set the active node. $this->set_active_node(); diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 22b56caf6f7..2bd1749730a 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -28,6 +28,11 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +use core\{ + di, + hook, +}; + defined('MOODLE_INTERNAL') || die(); // CONSTANTS (Encased in phpdoc proper comments). @@ -3581,7 +3586,7 @@ function delete_user(stdClass $user) { $hook = new \core_user\hook\before_user_deleted( user: $user, ); - \core\di::get(\core\hook\manager::class)->dispatch($hook); + di::get(hook\manager::class)->dispatch($hook); // Keep user record before updating it, as we have to pass this to user_deleted event. $olduser = clone $user; @@ -4085,8 +4090,7 @@ function complete_user_login($user, array $extrauserinfo = []) { $event->trigger(); // Allow plugins to callback as soon possible after user has completed login. - $hook = new \core\hook\user\after_complete_login(); - \core\hook\manager::get_instance()->dispatch($hook); + di::get(\core\hook\manager::class)->dispatch(new \core\hook\user\after_complete_login()); // Check if the user is using a new browser or session (a new MoodleSession cookie is set in that case). // If the user is accessing from the same IP, ignore everything (most of the time will be a new session in the same browser). @@ -7436,8 +7440,8 @@ function get_plugins_with_function($function, $file = 'lib.php', $include = true foreach ($pluginfunctions as $plugintype => $plugins) { foreach ($plugins as $plugin => $unusedfunction) { $component = $plugintype . '_' . $plugin; - if ($hooks = \core\hook\manager::get_instance()->get_hooks_deprecating_plugin_callback($plugincallback)) { - if (\core\hook\manager::get_instance()->is_deprecating_hook_present($component, $plugincallback)) { + if ($hooks = di::get(hook\manager::class)->get_hooks_deprecating_plugin_callback($plugincallback)) { + if (di::get(hook\manager::class)->is_deprecating_hook_present($component, $plugincallback)) { // Ignore the old callback, it is there only for older Moodle versions. unset($pluginfunctions[$plugintype][$plugin]); } else { @@ -7675,8 +7679,9 @@ function component_callback($component, $function, array $params = array(), $def if ($functionname) { if ($migratedtohook) { - if ($hooks = \core\hook\manager::get_instance()->get_hooks_deprecating_plugin_callback($function)) { - if (\core\hook\manager::get_instance()->is_deprecating_hook_present($component, $function)) { + $hookmanager = di::get(hook\manager::class); + if ($hooks = $hookmanager->get_hooks_deprecating_plugin_callback($function)) { + if ($hookmanager->is_deprecating_hook_present($component, $function)) { // Do not call the old lib.php callback, // it is there for compatibility with older Moodle versions only. return null; @@ -7776,8 +7781,9 @@ function component_class_callback($classname, $methodname, array $params, $defau $functionparts = explode('\\', trim($fullfunction, '\\')); $component = $functionparts[0]; $callback = end($functionparts); - if ($hooks = \core\hook\manager::get_instance()->get_hooks_deprecating_plugin_callback($callback)) { - if (\core\hook\manager::get_instance()->is_deprecating_hook_present($component, $callback)) { + $hookmanager = di::get(hook\manager::class); + if ($hooks = $hookmanager->get_hooks_deprecating_plugin_callback($callback)) { + if ($hookmanager->is_deprecating_hook_present($component, $callback)) { // Do not call the old class callback, // it is there for compatibility with older Moodle versions only. return null; diff --git a/lib/phpunit/bootstrap.php b/lib/phpunit/bootstrap.php index 0ac815e3143..9aedea38c3e 100644 --- a/lib/phpunit/bootstrap.php +++ b/lib/phpunit/bootstrap.php @@ -262,6 +262,7 @@ if (PHPUNIT_UTIL) { // Make sure the hook manager gets initialised before anybody tries to override callbacks, // this is not using caches intentionally to help with development. +// Note: We cannot use DI at this point in the bootstrap either. \core\hook\manager::get_instance(); // Is database and dataroot ready for testing? diff --git a/lib/phpunit/classes/advanced_testcase.php b/lib/phpunit/classes/advanced_testcase.php index 4f039900f6c..598a37b82b7 100644 --- a/lib/phpunit/classes/advanced_testcase.php +++ b/lib/phpunit/classes/advanced_testcase.php @@ -14,6 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +use core\{ + di, + hook, +}; + /** * Advanced PHPUnit test case customised for Moodle. * @@ -486,7 +491,7 @@ abstract class advanced_testcase extends base_testcase { * @return void */ public function redirectHook(string $hookname, callable $callback): void { - \core\hook\manager::get_instance()->phpunit_redirect_hook($hookname, $callback); + di::get(hook\manager::class)->phpunit_redirect_hook($hookname, $callback); } /** @@ -495,7 +500,7 @@ abstract class advanced_testcase extends base_testcase { * @return void */ public function stopHookRedirections(): void { - \core\hook\manager::get_instance()->phpunit_stop_redirections(); + di::get(hook\manager::class)->phpunit_stop_redirections(); } /** diff --git a/lib/phpunit/classes/util.php b/lib/phpunit/classes/util.php index 1d2b5fd216e..f26f3c04a28 100644 --- a/lib/phpunit/classes/util.php +++ b/lib/phpunit/classes/util.php @@ -23,6 +23,11 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +use core\{ + di, + hook, +}; + require_once(__DIR__.'/../../testing/classes/util.php'); require_once(__DIR__ . "/coverage_info.php"); @@ -105,7 +110,7 @@ class phpunit_util extends testing_util { global $DB, $CFG, $USER, $SITE, $COURSE, $PAGE, $OUTPUT, $SESSION, $FULLME, $FILTERLIB_PRIVATE; // Stop all hook redirections. - \core\hook\manager::get_instance()->phpunit_stop_redirections(); + di::get(hook\manager::class)->phpunit_stop_redirections(); // Reset the hook manager instance. \core\hook\manager::phpunit_reset_instance(); diff --git a/mod/quiz/classes/grade_calculator.php b/mod/quiz/classes/grade_calculator.php index 122088297af..69a8158ad91 100644 --- a/mod/quiz/classes/grade_calculator.php +++ b/mod/quiz/classes/grade_calculator.php @@ -17,6 +17,10 @@ namespace mod_quiz; use coding_exception; +use core\{ + di, + hook, +}; use mod_quiz\event\quiz_grade_updated; use mod_quiz\hook\structure_modified; use question_engine_data_mapper; @@ -100,7 +104,7 @@ class grade_calculator { component_class_callback($callbackclass, 'callback', [$quiz->id], null, true); } - \core\hook\manager::get_instance()->dispatch(new structure_modified($this->quizobj->get_structure())); + di::get(hook\manager::class)->dispatch(new structure_modified($this->quizobj->get_structure())); } /** diff --git a/mod/quiz/classes/quiz_attempt.php b/mod/quiz/classes/quiz_attempt.php index 08c40dac12d..6b93de93761 100644 --- a/mod/quiz/classes/quiz_attempt.php +++ b/mod/quiz/classes/quiz_attempt.php @@ -21,6 +21,10 @@ use block_contents; use cm_info; use coding_exception; use context_module; +use core\{ + di, + hook, +}; use Exception; use html_writer; use mod_quiz\hook\attempt_state_changed; @@ -1787,7 +1791,7 @@ class quiz_attempt { // Trigger event. $this->fire_state_transition_event('\mod_quiz\event\attempt_submitted', $timestamp, $studentisonline); - \core\hook\manager::get_instance()->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); + di::get(hook\manager::class)->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); // Tell any access rules that care that the attempt is over. $this->get_access_manager($timestamp)->current_attempt_finished(); } @@ -1835,7 +1839,7 @@ class quiz_attempt { $this->fire_state_transition_event('\mod_quiz\event\attempt_becameoverdue', $timestamp, $studentisonline); - \core\hook\manager::get_instance()->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); + di::get(hook\manager::class)->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); $transaction->allow_commit(); quiz_send_overdue_message($this); @@ -1859,7 +1863,7 @@ class quiz_attempt { $this->fire_state_transition_event('\mod_quiz\event\attempt_abandoned', $timestamp, $studentisonline); - \core\hook\manager::get_instance()->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); + di::get(hook\manager::class)->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); $transaction->allow_commit(); } @@ -1890,7 +1894,7 @@ class quiz_attempt { $this->fire_state_transition_event('\mod_quiz\event\attempt_reopened', $timestamp, false); - \core\hook\manager::get_instance()->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); + di::get(hook\manager::class)->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); $timeclose = $this->get_access_manager($timestamp)->get_end_time($this->attempt); if ($timeclose && $timestamp > $timeclose) { $this->process_finish($timestamp, false, $timeclose); diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 7459560f2d3..7d60989a26b 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -35,6 +35,10 @@ require_once($CFG->libdir . '/completionlib.php'); require_once($CFG->libdir . '/filelib.php'); require_once($CFG->libdir . '/questionlib.php'); +use core\{ + di, + hook, +}; use core_question\local\bank\condition; use mod_quiz\access_manager; use mod_quiz\event\attempt_submitted; @@ -147,7 +151,7 @@ function quiz_create_attempt(quiz_settings $quizobj, $attemptnumber, $lastattemp $attempt->timecheckstate = $timeclose; } - \core\hook\manager::get_instance()->dispatch(new attempt_state_changed(null, $attempt)); + di::get(hook\manager::class)->dispatch(new attempt_state_changed(null, $attempt)); return $attempt; } @@ -459,7 +463,7 @@ function quiz_delete_attempt($attempt, $quiz) { component_class_callback($callbackclass, 'callback', [$quiz->id], null, true); } - \core\hook\manager::get_instance()->dispatch(new attempt_state_changed($attempt, null)); + di::get(hook\manager::class)->dispatch(new attempt_state_changed($attempt, null)); } // Search quiz_attempts for other instances by this user.