From f77f19fa61a475454847f4fd4317134dd1d5d646 Mon Sep 17 00:00:00 2001 From: Markus Date: Sun, 20 Oct 2024 19:40:55 +0200 Subject: [PATCH] migrate repos to php-di --- .../restore_adleradaptivity_stepslib.php | 11 ++++---- classes/completion/custom_completion.php | 3 ++- classes/external/answer_questions.php | 7 +++-- classes/external/get_task_details.php | 3 ++- classes/local/completion_helpers.php | 5 ++-- .../adleradaptivity_question_repository.php | 2 +- classes/local/db/base_repository.php | 13 +++------ classes/local/helpers.php | 7 ++--- .../output/pages/processattempt_page.php | 6 +++-- classes/local/output/pages/view_page.php | 11 ++++---- lib.php | 27 ++++++++++--------- tests/lib_test.php | 13 ++++++--- 12 files changed, 57 insertions(+), 51 deletions(-) diff --git a/backup/moodle2/restore_adleradaptivity_stepslib.php b/backup/moodle2/restore_adleradaptivity_stepslib.php index 28b05d6..5f6020c 100644 --- a/backup/moodle2/restore_adleradaptivity_stepslib.php +++ b/backup/moodle2/restore_adleradaptivity_stepslib.php @@ -1,5 +1,6 @@ adleradaptivity_repository = new adleradaptivity_repository(); - $this->adleradaptivity_task_repository = new adleradaptivity_task_repository(); - $this->adleradaptivity_question_repository = new adleradaptivity_question_repository(); - $this->adleradaptivity_attempt_repository = new adleradaptivity_attempt_repository(); - $this->moodle_core_repository = new moodle_core_repository(); + $this->adleradaptivity_repository = di::get(adleradaptivity_repository::class); + $this->adleradaptivity_task_repository = di::get(adleradaptivity_task_repository::class); + $this->adleradaptivity_question_repository = di::get(adleradaptivity_question_repository::class); + $this->adleradaptivity_attempt_repository = di::get(adleradaptivity_attempt_repository::class); + $this->moodle_core_repository = di::get(moodle_core_repository::class); } /** diff --git a/classes/completion/custom_completion.php b/classes/completion/custom_completion.php index df0701b..05ec1cc 100644 --- a/classes/completion/custom_completion.php +++ b/classes/completion/custom_completion.php @@ -7,6 +7,7 @@ global $CFG; require_once($CFG->libdir . '/questionlib.php'); +use core\di; use core_completion\activity_custom_completion; use dml_exception; use mod_adleradaptivity\local\completion_helpers; @@ -31,7 +32,7 @@ public function __construct(...$args) { $args[1] = (int) $args[1]; parent::__construct(...$args); - $this->task_repository = new adleradaptivity_task_repository(); + $this->task_repository = di::get(adleradaptivity_task_repository::class); } /** * Check element successfully completed. diff --git a/classes/external/answer_questions.php b/classes/external/answer_questions.php index 8289e20..c1f81f1 100644 --- a/classes/external/answer_questions.php +++ b/classes/external/answer_questions.php @@ -8,6 +8,7 @@ use completion_info; use context_module; +use core\di; use core_external\restricted_context_exception; use dml_exception; use dml_transaction_exception; @@ -160,7 +161,7 @@ public static function execute(array $module, array $questions): array { * @throws invalid_parameter_exception If a question does not exist. */ protected static function validate_and_enhance_questions(array $questions, string $instance_id): array { - $task_repository = new adleradaptivity_task_repository(); + $task_repository = di::get(adleradaptivity_task_repository::class); foreach ($questions as $key => $question) { try { $task = $task_repository->get_task_by_question_uuid($question['uuid'], $instance_id); @@ -303,10 +304,8 @@ private static function all_elements_are_bool(array $array): bool { * @throws dml_exception */ protected static function process_questions(array $questions, int $time_at_request_start, stdClass $module, question_usage_by_activity $quba): completion_info { - global $DB; - // start delegating transaction - $transaction = $DB->start_delegated_transaction(); + $transaction = di::get(moodle_database::class)->start_delegated_transaction(); // start processing the questions foreach ($questions as $key => $question) { diff --git a/classes/external/get_task_details.php b/classes/external/get_task_details.php index 1801f63..6533027 100644 --- a/classes/external/get_task_details.php +++ b/classes/external/get_task_details.php @@ -7,6 +7,7 @@ require_once($CFG->dirroot . '/lib/externallib.php'); use context_module; +use core\di; use core_external\restricted_context_exception; use dml_exception; use external_api; @@ -72,7 +73,7 @@ public static function execute_returns(): external_function_parameters { * @throws moodle_exception */ public static function execute(array $module): array { - $tasks_repo = new adleradaptivity_task_repository(); + $tasks_repo = di::get(adleradaptivity_task_repository::class); // Parameter validation $params = self::validate_parameters(self::execute_parameters(), array('module' => $module)); diff --git a/classes/local/completion_helpers.php b/classes/local/completion_helpers.php index 8bcc38e..4ae77a5 100644 --- a/classes/local/completion_helpers.php +++ b/classes/local/completion_helpers.php @@ -5,6 +5,7 @@ global $CFG; require_once($CFG->libdir . '/questionlib.php'); +use core\di; use mod_adleradaptivity\local\db\adleradaptivity_question_repository; use moodle_exception; use question_answer; @@ -43,13 +44,13 @@ class completion_helpers { * @throws moodle_exception */ public static function check_task_status(question_usage_by_activity $quba, string $task_id, string|null $task_required_difficulty): string { - $adleradaptivity_question_repository = new adleradaptivity_question_repository(); + $adleradaptivity_question_repository = di::get(adleradaptivity_question_repository::class); $success = false; $attempted = false; $optional = $task_required_difficulty == null; - foreach ($adleradaptivity_question_repository->get_adleradaptivity_questions_with_moodle_question_id_by_task_id($task_id) as $question) { + foreach ($adleradaptivity_question_repository->get_adleradaptivity_questions_with_moodle_question_id($task_id) as $question) { // get slot of question foreach ($quba->get_slots() as $slot) { if ($quba->get_question($slot)->id == $question->questionid) { diff --git a/classes/local/db/adleradaptivity_question_repository.php b/classes/local/db/adleradaptivity_question_repository.php index 255759f..f19750e 100644 --- a/classes/local/db/adleradaptivity_question_repository.php +++ b/classes/local/db/adleradaptivity_question_repository.php @@ -51,7 +51,7 @@ public function get_adleradaptivity_question_by_question_bank_entries_id(int $qu * @return array of objects with moodle question id and adleradaptivity question id. * @throws moodle_exception if any question version is not equal to 1. */ - public function get_adleradaptivity_questions_with_moodle_question_id_by_task_id(int $task_id, bool $ignore_question_version = false): array { + public function get_adleradaptivity_questions_with_moodle_question_id(int $task_id, bool $ignore_question_version = false): array { // Retrieves question versions from the {question_versions} table based on a specified adaptivity ID. $sql = " SELECT qv.questionid, qv.version, aq.* diff --git a/classes/local/db/base_repository.php b/classes/local/db/base_repository.php index 7599549..31ac60e 100644 --- a/classes/local/db/base_repository.php +++ b/classes/local/db/base_repository.php @@ -5,14 +5,7 @@ use moodle_database; abstract class base_repository { - protected moodle_database $db; - - public function __construct(moodle_database|null $db = null) { - if (is_null($db)) { - global $DB; - $this->db = $DB; - } else { - $this->db = $db; - } - } + public function __construct( + protected readonly moodle_database $db + ) {} } \ No newline at end of file diff --git a/classes/local/helpers.php b/classes/local/helpers.php index 4ff730b..ee2493e 100644 --- a/classes/local/helpers.php +++ b/classes/local/helpers.php @@ -6,6 +6,7 @@ require_once($CFG->libdir . '/questionlib.php'); use context_module; +use core\di; use dml_exception; use mod_adleradaptivity\local\db\adleradaptivity_attempt_repository; use mod_adleradaptivity\local\db\moodle_core_repository; @@ -29,7 +30,7 @@ class helpers { */ public static function load_or_create_question_usage(int $cmid, int|null $userid = null, bool $create_new_attempt = true): false|question_usage_by_activity { global $USER; - $adleradaptivity_attempt_repository = new adleradaptivity_attempt_repository(); + $adleradaptivity_attempt_repository = di::get(adleradaptivity_attempt_repository::class); if (!isset($userid)) { $userid = $USER->id; @@ -57,7 +58,7 @@ public static function load_or_create_question_usage(int $cmid, int|null $userid $adleradaptivity_attempt->attempt_id = $attempt_id; $adleradaptivity_attempt->user_id = $userid; - $adleradaptivity_attempt_repository = new adleradaptivity_attempt_repository(); + $adleradaptivity_attempt_repository = di::get(adleradaptivity_attempt_repository::class); $adleradaptivity_attempt_repository->create_adleradaptivity_attempt($adleradaptivity_attempt); break; case 1: @@ -113,7 +114,7 @@ public static function generate_new_attempt(int $cmid): question_usage_by_activi * @throws moodle_exception if any question version is not equal to 1. */ public static function load_questions_by_cmid(int $cmid, bool $allow_shuffle = false): array { - $moodle_core_repository = new moodle_core_repository(); + $moodle_core_repository = di::get(moodle_core_repository::class); // get instance id from cmid $instance_id = get_coursemodule_from_id('', $cmid)->instance; diff --git a/classes/local/output/pages/processattempt_page.php b/classes/local/output/pages/processattempt_page.php index 53873d3..219a8e2 100644 --- a/classes/local/output/pages/processattempt_page.php +++ b/classes/local/output/pages/processattempt_page.php @@ -4,10 +4,12 @@ use coding_exception; use completion_info; +use core\di; use dml_exception; use dml_transaction_exception; use local_logging\logger; use mod_adleradaptivity\local\db\moodle_core_repository; +use moodle_database; use moodle_exception; use moodle_url; use question_engine; @@ -57,7 +59,7 @@ public function __construct() { */ private function process_attempt(question_usage_by_activity $quba, stdClass $course, stdClass $cm): void { global $DB; - $transaction = $DB->start_delegated_transaction(); + $transaction = di::get(moodle_database::class)->start_delegated_transaction(); $quba->process_all_actions($this->time_now); question_engine::save_questions_usage_by_activity($quba); @@ -73,7 +75,7 @@ private function process_attempt(question_usage_by_activity $quba, stdClass $cou private function setup_instance_variables(): void { $this->logger = new logger('mod_adleradaptiviy', 'processattempt'); $this->time_now = time(); # Saving time at request start to use the actual submission time - $this->moodle_core_repository = new moodle_core_repository(); + $this->moodle_core_repository = di::get(moodle_core_repository::class); } /** diff --git a/classes/local/output/pages/view_page.php b/classes/local/output/pages/view_page.php index 0e73f3a..675aea8 100644 --- a/classes/local/output/pages/view_page.php +++ b/classes/local/output/pages/view_page.php @@ -8,6 +8,7 @@ use bootstrap_renderer; use coding_exception; use context_module; +use core\di; use dml_exception; use dml_missing_record_exception; use invalid_parameter_exception; @@ -93,11 +94,11 @@ private function setup_instance_variables(): void { $this->output = $OUTPUT; $this->user = $USER; - $this->task_repository = new adleradaptivity_task_repository(); - $this->question_repository = new adleradaptivity_question_repository(); - $this->adleradaptivity_attempt_repository = new adleradaptivity_attempt_repository(); - $this->adleradaptivity_repository = new adleradaptivity_repository(); - $this->moodle_core_repository = new moodle_core_repository(); + $this->task_repository = di::get( adleradaptivity_task_repository::class); + $this->question_repository = di::get( adleradaptivity_question_repository::class); + $this->adleradaptivity_attempt_repository = di::get( adleradaptivity_attempt_repository::class); + $this->adleradaptivity_repository = di::get( adleradaptivity_repository::class); + $this->moodle_core_repository = di::get( moodle_core_repository::class); $this->logger = new logger('mod_adleradaptivity', 'view_page.php'); } diff --git a/lib.php b/lib.php index b38b901..73b0e14 100644 --- a/lib.php +++ b/lib.php @@ -1,5 +1,6 @@ timemodified = time(); - $id = (new adleradaptivity_repository())->create_adleradaptivity($instancedata); + $id = di::get(adleradaptivity_repository::class)->create_adleradaptivity($instancedata); // Update completion date event. This is a default feature activated for all modules (create module -> Activity completion). $completiontimeexpected = !empty($instancedata->completionexpected) ? $instancedata->completionexpected : null; @@ -71,24 +72,21 @@ function adleradaptivity_update_instance($moduleinstance, $mform = null): bool { * questions itself are not deleted here as they belong to the course, not to the module. The adleradaptivity_questions are deleted. * * @param $instance_id int The instance id of the module to delete. - * @param adleradaptivity_question_repository|null $adleradaptivity_question_repository allows to inject a mock for testing * @return bool true if success, false if failed. * @throws dml_transaction_exception if the transaction failed and could not be rolled back. * @throws dml_exception */ -function adleradaptivity_delete_instance(int $instance_id, adleradaptivity_question_repository $adleradaptivity_question_repository = null): bool { - global $DB; - +function adleradaptivity_delete_instance(int $instance_id): bool { $logger = new logger('mod_adleradaptivity', 'lib.php'); - $adleradaptivity_attempt_repository = new adleradaptivity_attempt_repository(); - $adleradaptivity_tasks_repository = new adleradaptivity_task_repository(); - $adleradaptivity_question_repository = $adleradaptivity_question_repository ?? new adleradaptivity_question_repository(); - $adleradaptivity_repository = new adleradaptivity_repository(); + $adleradaptivity_attempt_repository = di::get(adleradaptivity_attempt_repository::class); + $adleradaptivity_tasks_repository = di::get(adleradaptivity_task_repository::class); + $adleradaptivity_question_repository = di::get(adleradaptivity_question_repository::class); + $adleradaptivity_repository = di::get(adleradaptivity_repository::class); // there is no transaction above this level. Unsuccessful deletions basically result in unpredictable // behaviour. This at least ensures this module is either deleted completely or not at all. - $transaction = $DB->start_delegated_transaction(); + $transaction = di::get(moodle_database::class)->start_delegated_transaction(); try { // first ensure that the module instance exists @@ -108,7 +106,10 @@ function adleradaptivity_delete_instance(int $instance_id, adleradaptivity_quest $adler_tasks = $adleradaptivity_tasks_repository->get_tasks_by_adleradaptivity_id($instance_id); $adler_questions = []; foreach ($adler_tasks as $task) { - $adler_questions = array_merge($adler_questions, $adleradaptivity_question_repository->get_adleradaptivity_questions_with_moodle_question_id_by_task_id($task->id, true)); + $adler_questions = array_merge( + $adler_questions, + $adleradaptivity_question_repository->get_adleradaptivity_questions_with_moodle_question_id($task->id, true) + ); } // perform deletion foreach ($adler_questions as $question) { @@ -120,7 +121,7 @@ function adleradaptivity_delete_instance(int $instance_id, adleradaptivity_quest $adleradaptivity_repository->delete_adleradaptivity_by_id($instance_id); $transaction->allow_commit(); - } catch (Exception $e) { + } catch (Throwable $e) { $logger->error('Could not delete adleradaptivity instance with id ' . $instance_id); $logger->error($e->getMessage()); // although the existing documentation suggests this method should return true|false depending @@ -149,7 +150,7 @@ function adleradaptivity_delete_instance(int $instance_id, adleradaptivity_quest * @throws dml_exception */ function adleradaptivity_get_coursemodule_info(stdClass $coursemodule): cached_cm_info|bool { - $adleradaptivity_repository = new adleradaptivity_repository(); + $adleradaptivity_repository = di::get(adleradaptivity_repository::class); if (!$cm = $adleradaptivity_repository->get_instance_by_instance_id($coursemodule->instance)) { return false; diff --git a/tests/lib_test.php b/tests/lib_test.php index d304c40..f47621f 100644 --- a/tests/lib_test.php +++ b/tests/lib_test.php @@ -1,5 +1,6 @@ getMockBuilder(adleradaptivity_question_repository::class)->onlyMethods(['delete_question_by_id'])->getMock(); - // Make the method throw an exception + // Mock the repository object and make it throw an exception. + $mockRepo = $this + ->getMockBuilder(adleradaptivity_question_repository::class) + ->onlyMethods(['delete_question_by_id']) + ->setConstructorArgs([di::get(moodle_database::class)]) + ->getMock(); $mockRepo->method('delete_question_by_id')->willThrowException(new moodle_exception('Could not delete')); + di::set(adleradaptivity_question_repository::class, $mockRepo); // verify created elements before deletion $this->assertCount(1, $DB->get_records('adleradaptivity')); @@ -174,7 +179,7 @@ public function test_delete_instance_failure_due_to_question_deletion_failure() try { // Try to delete the complex instance. - adleradaptivity_delete_instance($complex_adleradaptivity_module['module']->id, $mockRepo); + adleradaptivity_delete_instance($complex_adleradaptivity_module['module']->id); } finally { // From my understanding these checks are not possible for postgresql databases as they don't allow rolling back sub-transactions // Overall the behaviour should still be correct, but as this code is executed as part of a higher level transaction