From 7b9f56a1c665b6fb9d7bb9b9c2402cff9d3613a5 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 17 Dec 2024 12:33:56 +1000 Subject: [PATCH] MDL-83541 question/quiz: restoring questions with identical stamps Historically it was possible, through a series of question restores, moves and edits, to end up with multiple questions in the same category with the same stamp, but differences in other question or answer fields. This, combined with changes in versions, led to errors when restoring or duplicating quizzes using these questions. While recent changes have made it impossible to create this situation in current Moodle versions, as any edits will create a new question version with a new stamp, this situation may exist on long-standing Moodle sites which have been upgraded since pre-4.0. This change performs a much wider-ranging comparison of restored existing questions, generating a hash of all the data for a question in a backup file, and a corresponding hash for each question in the target category, to decide if a restored question matches a question already in the database. --- .upgradenotes/MDL-83541-2025020615341872.yml | 30 + backup/moodle2/restore_qtype_plugin.class.php | 209 +++++++ .../tests/quiz_restore_decode_links_test.php | 3 + backup/util/dbops/restore_dbops.class.php | 56 +- ...store_questions_parser_processor.class.php | 122 +++- .../tests/backup/repeated_restore_test.php | 533 ++++++++++++++++++ 6 files changed, 938 insertions(+), 15 deletions(-) create mode 100644 .upgradenotes/MDL-83541-2025020615341872.yml diff --git a/.upgradenotes/MDL-83541-2025020615341872.yml b/.upgradenotes/MDL-83541-2025020615341872.yml new file mode 100644 index 0000000000000..b46e4c9fd2f85 --- /dev/null +++ b/.upgradenotes/MDL-83541-2025020615341872.yml @@ -0,0 +1,30 @@ +issueNumber: MDL-83541 +notes: + core_question: + - message: > + Duplication or multiple restores of questions has been modified to avoid + errors where a question with the same stamp already exists in the target + category. + + To achieve this all data for the question is hashed, excluding any ID + fields. + + If a qtype plugin calls any `$this->add_question_*()` methods in its + `restore_qtype_*_plugin::define_question_plugin_structure()` method, the + ID fields used in these records will be excluded automatically. + + If a qtype plugin defines its own tables with ID fields, it must define + `restore_qtype_*_plugin::define_excluded_identity_hash_fields()` to return + an array of paths to these fields within the question data. This should be + all that is required for the majority of plugins. + See the PHPDoc of `restore_qtype_plugin::define_excluded_identity_hash_fields()` + for a full explanation of how these paths should be defined, and + `restore_qtype_truefalse_plugin` for an example. + + If the data structure for a qtype returned by calling + `get_question_options()` contains data other than ID fields that are not + contained in the backup structure or vice-versa, it will need to + override `restore_qtype_*_plugin::remove_excluded_question_data()` + to remove the inconsistent data. See `restore_qtype_multianswer_plugin` as + an example. + type: fixed diff --git a/backup/moodle2/restore_qtype_plugin.class.php b/backup/moodle2/restore_qtype_plugin.class.php index 23d0a3dc076d9..a6df0e1d8aaab 100644 --- a/backup/moodle2/restore_qtype_plugin.class.php +++ b/backup/moodle2/restore_qtype_plugin.class.php @@ -47,6 +47,11 @@ abstract class restore_qtype_plugin extends restore_plugin { */ private $questionanswercacheid = null; + /** + * @var array List of fields to exclude form hashing during restore. + */ + protected array $excludedhashfields = []; + /** * Add to $paths the restore_path_elements needed * to handle question_answers for a given question @@ -62,6 +67,10 @@ protected function add_question_question_answers(&$paths) { $elename = 'question_answer'; $elepath = $this->get_pathfor('/answers/answer'); // we used get_recommended_name() so this works $paths[] = new restore_path_element($elename, $elepath); + $this->exclude_identity_hash_fields([ + '/options/answers/id', + '/options/answers/question', + ]); } /** @@ -78,6 +87,10 @@ protected function add_question_numerical_units(&$paths) { $elename = 'question_numerical_unit'; $elepath = $this->get_pathfor('/numerical_units/numerical_unit'); // we used get_recommended_name() so this works $paths[] = new restore_path_element($elename, $elepath); + $this->exclude_identity_hash_fields([ + '/options/units/id', + '/options/units/question', + ]); } /** @@ -94,6 +107,7 @@ protected function add_question_numerical_options(&$paths) { $elename = 'question_numerical_option'; $elepath = $this->get_pathfor('/numerical_options/numerical_option'); // we used get_recommended_name() so this works $paths[] = new restore_path_element($elename, $elepath); + $this->exclude_identity_hash_fields(['/options/question']); } /** @@ -114,6 +128,21 @@ protected function add_question_datasets(&$paths) { $elename = 'question_dataset_item'; $elepath = $this->get_pathfor('/dataset_definitions/dataset_definition/dataset_items/dataset_item'); $paths[] = new restore_path_element($elename, $elepath); + $this->exclude_identity_hash_fields([ + '/options/datasets/id', + '/options/datasets/question', + '/options/datasets/category', + '/options/datasets/type', + '/options/datasets/items/id', + // The following fields aren't included in the backup or DB structure, but are parsed from the options field. + '/options/datasets/status', + '/options/datasets/distribution', + '/options/datasets/minimum', + '/options/datasets/maximum', + '/options/datasets/decimals', + // This field is set dynamically from the count of items in the dataset, it is not backed up. + '/options/datasets/number_of_items', + ]); } /** @@ -395,4 +424,184 @@ public static function define_plugin_decode_contents() { return $contents; } + + /** + * Add fields to the list of fields excluded from hashing. + * + * This allows common methods to add fields to the exclusion list. + * + * @param array $fields + * @return void + */ + private function exclude_identity_hash_fields(array $fields): void { + $this->excludedhashfields = array_merge($this->excludedhashfields, $fields); + } + + /** + * Return fields to be excluded from hashing during restores. + * + * @return array + */ + final public function get_excluded_identity_hash_fields(): array { + return array_unique(array_merge( + $this->excludedhashfields, + $this->define_excluded_identity_hash_fields(), + )); + } + + /** + * Return a list of paths to fields to be removed from questiondata before creating an identity hash. + * + * Fields that should be excluded from common elements such as answers or numerical units that are used by the plugin will + * be excluded automatically. This method just needs to define any specific to this plugin, such as foreign keys used in the + * plugin's tables. + * + * The returned array should be a list of slash-delimited paths to locate the fields to be removed from the questiondata object. + * For example, if you want to remove the field `$questiondata->options->questionid`, the path would be '/options/questionid'. + * If a field in the path is an array, the rest of the path will be applied to each object in the array. So if you have + * `$questiondata->options->answers[]`, the path '/options/answers/id' will remove the 'id' field from each element of the + * 'answers' array. + * + * @return array + */ + protected function define_excluded_identity_hash_fields(): array { + return []; + } + + /** + * Convert the backup structure of this question type into a structure matching its question data + * + * This should take the hierarchical array of tags from the question's backup structure, and return a structure that matches + * that returned when calling {@see get_question_options()} for this question type. + * See https://docs.moodle.org/dev/Question_data_structures#Representation_1:_%24questiondata for an explanation of this + * structure. + * + * This data will then be used to produce an identity hash for comparison with questions in the database. + * + * This base implementation deals with all common backup elements created by the add_question_*_options() methods in this class, + * plus elements added by ::define_question_plugin_structure() named for the qtype. The question type will need to extend + * this function if ::define_question_plugin_structure() adds any other elements to the backup. + * + * @param array $tags The array of tags from the backup. + * @return \stdClass The questiondata object. + */ + public static function convert_backup_to_questiondata(array $tags): \stdClass { + $questiondata = (object) array_filter($tags, fn($tag) => !is_array($tag)); // Create an object from the top-level fields. + $qtype = $questiondata->qtype; + $questiondata->options = new stdClass(); + if (isset($tags["plugin_qtype_{$qtype}_question"][$qtype])) { + $questiondata->options = (object) $tags["plugin_qtype_{$qtype}_question"][$qtype][0]; + } + if (isset($tags["plugin_qtype_{$qtype}_question"]['answers'])) { + $questiondata->options->answers = array_map( + fn($answer) => (object) $answer, + $tags["plugin_qtype_{$qtype}_question"]['answers']['answer'], + ); + } + if (isset($tags["plugin_qtype_{$qtype}_question"]['numerical_options'])) { + $questiondata->options = (object) array_merge( + (array) $questiondata->options, + $tags["plugin_qtype_{$qtype}_question"]['numerical_options']['numerical_option'][0], + ); + } + if (isset($tags["plugin_qtype_{$qtype}_question"]['numerical_units'])) { + $questiondata->options->units = array_map( + fn($unit) => (object) $unit, + $tags["plugin_qtype_{$qtype}_question"]['numerical_units']['numerical_unit'], + ); + } + if (isset($tags["plugin_qtype_{$qtype}_question"]['dataset_definitions'])) { + $questiondata->options->datasets = array_map( + fn($dataset) => (object) $dataset, + $tags["plugin_qtype_{$qtype}_question"]['dataset_definitions']['dataset_definition'], + ); + } + if (isset($questiondata->options->datasets)) { + foreach ($questiondata->options->datasets as $dataset) { + if (isset($dataset->dataset_items)) { + $dataset->items = array_map( + fn($item) => (object) $item, + $dataset->dataset_items['dataset_item'], + ); + unset($dataset->dataset_items); + } + } + } + if (isset($tags['question_hints'])) { + $questiondata->hints = array_map( + fn($hint) => (object) $hint, + $tags['question_hints']['question_hint'], + ); + } + + return $questiondata; + } + + /** + * Remove excluded fields from the questiondata structure. + * + * This removes fields that will not match or not be present in the question data structure produced by + * {@see self::convert_backup_to_questiondata()} and {@see get_question_options()} (such as IDs), so that the remaining data can + * be used to produce an identity hash for comparing the two. + * + * For plugins, it should be sufficient to override {@see self::define_excluded_identity_hash_fields()} with a list of paths + * specific to the plugin type. Overriding this method is only necessary if the plugin's + * {@see question_type::get_question_options()} method adds additional data to the question that is not included in the backup. + * + * @param stdClass $questiondata + * @param array $excludefields Paths to the fields to exclude. + * @return stdClass The $questiondata with excluded fields removed. + */ + public static function remove_excluded_question_data(stdClass $questiondata, array $excludefields = []): stdClass { + // All questions will need to exclude 'id' (used by question and other tables), 'questionid' (used by hints and options), + // 'createdby' and 'modifiedby' (since they won't map between sites). + $defaultexcludes = [ + '/id', + '/createdby', + '/modifiedby', + '/hints/id', + '/hints/questionid', + '/options/id', + '/options/questionid', + ]; + $excludefields = array_unique(array_merge($excludefields, $defaultexcludes)); + + foreach ($excludefields as $excludefield) { + $pathparts = explode('/', ltrim($excludefield, '/')); + $data = $questiondata; + self::unset_excluded_fields($data, $pathparts); + } + + return $questiondata; + } + + /** + * Iterate through the elements of path to an excluded field, and unset the final element. + * + * If any of the elements in the path is an array, this is called recursively on each element in the array to unset fields + * in each child of the array. + * + * @param stdClass|array $data The questiondata object, or a subsection of it. + * @param array $pathparts The remaining elements in the path to the excluded field. + * @return void + */ + private static function unset_excluded_fields(stdClass|array $data, array $pathparts): void { + while (!empty($pathparts)) { + $element = array_shift($pathparts); + if (!isset($data->{$element})) { + // This element is not present in the data structure, nothing to unset. + return; + } + if (is_object($data->{$element})) { + $data = $data->{$element}; + } else if (is_array($data->{$element})) { + foreach ($data->{$element} as $item) { + self::unset_excluded_fields($item, $pathparts); + } + } else if (empty($pathparts)) { + // This is the last element of the path and it's a scalar value, unset it. + unset($data->{$element}); + } + } + } } diff --git a/backup/tests/quiz_restore_decode_links_test.php b/backup/tests/quiz_restore_decode_links_test.php index 8540d8eb95bc5..0ddcb0f083a3b 100644 --- a/backup/tests/quiz_restore_decode_links_test.php +++ b/backup/tests/quiz_restore_decode_links_test.php @@ -64,6 +64,8 @@ public function test_restore_quiz_decode_links(): void { $questiondata = \question_bank::load_question_data($question->id); + $DB->set_field('question', 'questiontext', $CFG->wwwroot . '/mod/quiz/view.php?id=' . $quiz->cmid, ['id' => $question->id]); + $firstanswer = array_shift($questiondata->options->answers); $DB->set_field('question_answers', 'answer', $CFG->wwwroot . '/course/view.php?id=' . $course->id, ['id' => $firstanswer->id]); @@ -87,6 +89,7 @@ public function test_restore_quiz_decode_links(): void { $questionids = []; foreach ($quizquestions as $quizquestion) { if ($quizquestion->questionid) { + $this->assertEquals($CFG->wwwroot . '/mod/quiz/view.php?id=' . $quiz->cmid, $quizquestion->questiontext); $questionids[] = $quizquestion->questionid; } } diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php index 42f3d18511aed..db667a9f86834 100644 --- a/backup/util/dbops/restore_dbops.class.php +++ b/backup/util/dbops/restore_dbops.class.php @@ -567,7 +567,7 @@ public static function precheck_categories_and_questions($restoreid, $courseid, * @return array A separate list of all error and warnings detected */ public static function prechek_precheck_qbanks_by_level($restoreid, $courseid, $userid, $samesite, $contextlevel) { - global $DB; + global $DB, $CFG; // To return any errors and warnings found $errors = array(); @@ -669,21 +669,37 @@ public static function prechek_precheck_qbanks_by_level($restoreid, $courseid, $ } else { self::set_backup_ids_record($restoreid, 'question_category', $category->id, $matchcat->id, $targetcontext->id); $questions = self::restore_get_questions($restoreid, $category->id); + $transformer = self::get_backup_xml_transformer($courseid); // Collect all the questions for this category into memory so we only talk to the DB once. - $questioncache = $DB->get_records_sql_menu('SELECT q.stamp, q.id - FROM {question} q - JOIN {question_versions} qv - ON qv.questionid = q.id - JOIN {question_bank_entries} qbe - ON qbe.id = qv.questionbankentryid - JOIN {question_categories} qc - ON qc.id = qbe.questioncategoryid - WHERE qc.id = ?', array($matchcat->id)); + $recordset = $DB->get_recordset_sql( + "SELECT q.* + FROM {question} q + JOIN {question_versions} qv ON qv.questionid = q.id + JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid + JOIN {question_categories} qc ON qc.id = qbe.questioncategoryid + WHERE qc.id = ?", + [$matchcat->id], + ); + + // Compute a hash of question and answer fields to differentiate between identical stamp-version questions. + $questioncache = []; + foreach ($recordset as $question) { + $question->export_process = true; // Include all question options required for export. + get_question_options($question); + unset($question->export_process); + // Remove some additional properties from get_question_options() that isn't included in backups + // before we produce the identity hash. + unset($question->categoryobject); + unset($question->questioncategoryid); + $cachekey = restore_questions_parser_processor::generate_question_identity_hash($question, $transformer); + $questioncache[$cachekey] = $question->id; + } + $recordset->close(); foreach ($questions as $question) { - if (isset($questioncache[$question->stamp])) { - $matchqid = $questioncache[$question->stamp]; + if (isset($questioncache[$question->questionhash])) { + $matchqid = $questioncache[$question->questionhash]; } else { $matchqid = false; } @@ -1918,6 +1934,22 @@ public static function delete_course_content($courseid, ?array $options = null) private static function password_should_be_discarded(#[\SensitiveParameter] string $password): bool { return (bool) preg_match('/^[0-9a-f]{32}$/', $password); } + + /** + * Load required classes and return a backup XML transformer for the specified course. + * + * These classes may not have been loaded if we're only doing a restore in the current process, + * so make sure we have them here. + * + * @param int $courseid + * @return backup_xml_transformer + */ + protected static function get_backup_xml_transformer(int $courseid): backup_xml_transformer { + global $CFG; + require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); + require_once($CFG->dirroot . '/backup/moodle2/backup_plan_builder.class.php'); + return new backup_xml_transformer($courseid); + } } /* diff --git a/backup/util/helper/restore_questions_parser_processor.class.php b/backup/util/helper/restore_questions_parser_processor.class.php index be3d0c28108b7..dae78663a5ffe 100644 --- a/backup/util/helper/restore_questions_parser_processor.class.php +++ b/backup/util/helper/restore_questions_parser_processor.class.php @@ -45,6 +45,9 @@ class restore_questions_parser_processor extends grouped_parser_processor { /** @var string XML path in the questions.xml to question elements within question_category (before Moodle 4.0). */ protected const LEGACY_QUESTION_SUBPATH = '/questions/question'; + /** @var string String for concatenating data into a string for hashing.*/ + protected const HASHDATA_SEPARATOR = '|HASHDATA|'; + /** @var string identifies the current restore. */ protected string $restoreid; @@ -52,13 +55,44 @@ class restore_questions_parser_processor extends grouped_parser_processor { protected int $lastcatid; public function __construct($restoreid) { + global $CFG; $this->restoreid = $restoreid; $this->lastcatid = 0; parent::__construct(); // Set the paths we are interested on $this->add_path(self::CATEGORY_PATH); - $this->add_path(self::CATEGORY_PATH . self::QUESTION_SUBPATH); - $this->add_path(self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH); + $this->add_path(self::CATEGORY_PATH . self::QUESTION_SUBPATH, true); + $this->add_path(self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH, true); + + // Add all sub-elements, including those from plugins, as grouped paths with the question tag so that + // we can create a hash of all question data for comparison with questions in the database. + $this->add_path(self::CATEGORY_PATH . self::QUESTION_SUBPATH . '/question_hints'); + $this->add_path(self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH . '/question_hints'); + $this->add_path(self::CATEGORY_PATH . self::QUESTION_SUBPATH . '/question_hints/question_hint'); + $this->add_path(self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH . '/question_hints/question_hint'); + + $connectionpoint = new restore_path_element('question', self::CATEGORY_PATH . self::QUESTION_SUBPATH); + foreach (\core\plugin_manager::instance()->get_plugins_of_type('qtype') as $qtype) { + $restore = $this->get_qtype_restore($qtype->name); + if (!$restore) { + continue; + } + $structure = $restore->define_plugin_structure($connectionpoint); + foreach ($structure as $element) { + $subpath = str_replace(self::CATEGORY_PATH . self::QUESTION_SUBPATH . '/', '', $element->get_path()); + $pathparts = explode('/', $subpath); + $path = self::CATEGORY_PATH . self::QUESTION_SUBPATH; + $legacypath = self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH; + foreach ($pathparts as $part) { + $path .= '/' . $part; + $legacypath .= '/' . $part; + if (!in_array($path, $this->paths)) { + $this->add_path($path); + $this->add_path($legacypath); + } + } + } + } } protected function dispatch_chunk($data) { @@ -73,10 +107,19 @@ protected function dispatch_chunk($data) { // Prepare question record } else if ($data['path'] == self::CATEGORY_PATH . self::QUESTION_SUBPATH || $data['path'] == self::CATEGORY_PATH . self::LEGACY_QUESTION_SUBPATH) { - $info = (object)$data['tags']; + // Remove sub-elements from the question info we're going to save. + $info = (object) array_filter($data['tags'], fn($tag) => !is_array($tag)); $itemname = 'question'; $itemid = $info->id; $parentitemid = $this->lastcatid; + $restore = $this->get_qtype_restore($data['tags']['qtype']); + if ($restore) { + $questiondata = $restore->convert_backup_to_questiondata($data['tags']); + } else { + $questiondata = restore_qtype_plugin::convert_backup_to_questiondata($data['tags']); + } + // Store a hash of question fields for comparison with existing questions. + $info->questionhash = $this->generate_question_identity_hash($questiondata); // Not question_category nor question, impossible. Throw exception. } else { @@ -106,4 +149,77 @@ public function process_cdata($cdata) { } return $cdata; } + + /** + * Load and instantiate the restore class for the given question type. + * + * If there is no restore class, null is returned. + * + * @param string $qtype The question type name (no qtype_ prefix) + * @return ?restore_qtype_plugin + */ + protected static function get_qtype_restore(string $qtype): ?restore_qtype_plugin { + global $CFG; + $step = new restore_quiz_activity_structure_step('questions', 'question.xml'); + $filepath = "{$CFG->dirroot}/question/type/{$qtype}/backup/moodle2/restore_qtype_{$qtype}_plugin.class.php"; + if (!file_exists($filepath)) { + return null; + } + require_once($filepath); + $restoreclass = "restore_qtype_{$qtype}_plugin"; + if (!class_exists($restoreclass)) { + return null; + } + return new $restoreclass('qtype', $qtype, $step); + } + + /** + * Given a data structure containing the data for a question, reduce it to a flat array and return a sha1 hash of the data. + * + * @param stdClass $questiondata An array containing all the data for a question, including hints and qtype plugin data. + * @param ?backup_xml_transformer $transformer If provided, run the backup transformer process on all text fields. This ensures + * that values from the database are compared like-for-like with encoded values from the backup. + * @return string A sha1 hash of all question data, normalised and concatenated together. + */ + public static function generate_question_identity_hash( + stdClass $questiondata, + ?backup_xml_transformer $transformer = null, + ): string { + $questiondata = clone($questiondata); + $restore = self::get_qtype_restore($questiondata->qtype); + if ($restore) { + $restore->define_plugin_structure(new restore_path_element('question', self::CATEGORY_PATH . self::QUESTION_SUBPATH)); + // Combine default exclusions with those specified by the plugin. + $questiondata = $restore->remove_excluded_question_data($questiondata, $restore->get_excluded_identity_hash_fields()); + } else { + // The qtype has no restore class, use the default reduction method. + $questiondata = restore_qtype_plugin::remove_excluded_question_data($questiondata); + } + + // Convert questiondata to a flat array of values. + $hashdata = []; + // Convert the object to a multi-dimensional array for compatibility with array_walk_recursive. + $questiondata = json_decode(json_encode($questiondata), true); + array_walk_recursive($questiondata, function($value) use (&$hashdata) { + // Normalise data types. Depending on where the data comes from, it may be a mixture of nulls, strings, + // ints and floats. Convert everything to strings, then all numbers to floats to ensure we are doing + // like-for-like comparisons without losing accuracy. + $value = (string) $value; + if (is_numeric($value)) { + $value = (float) ($value); + } + $hashdata[] = $value; + }); + + sort($hashdata, SORT_STRING); + $hashstring = implode(self::HASHDATA_SEPARATOR, $hashdata); + if ($transformer) { + $hashstring = $transformer->process($hashstring); + // Need to re-sort the hashdata with the transformed strings. + $hashdata = explode(self::HASHDATA_SEPARATOR, $hashstring); + sort($hashdata, SORT_STRING); + $hashstring = implode(self::HASHDATA_SEPARATOR, $hashdata); + } + return sha1($hashstring); + } } diff --git a/mod/quiz/tests/backup/repeated_restore_test.php b/mod/quiz/tests/backup/repeated_restore_test.php index 204f3f1c95826..f8bad28865eec 100644 --- a/mod/quiz/tests/backup/repeated_restore_test.php +++ b/mod/quiz/tests/backup/repeated_restore_test.php @@ -152,4 +152,537 @@ public function test_restore_quiz_into_other_course_twice(): void { $this->assertEquals($questionscourse2firstimport[$slot->slot]->questionid, $slot->questionid); } } + + /** + * Return a list of qtypes with valid generators in their helper class. + * + * This will check all installed qtypes for a test helper class, then find a defined test question which has a corresponding + * form_data method and return it. If the helper doesn't have a form_data method for any test question, it will return a + * null test question name for that qtype. + * + * @return array + */ + public static function get_qtype_generators(): array { + global $CFG; + $generators = []; + foreach (\core\plugin_manager::instance()->get_plugins_of_type('qtype') as $qtype) { + if ($qtype->name == 'random') { + continue; + } + $helperpath = "{$CFG->dirroot}/question/type/{$qtype->name}/tests/helper.php"; + if (!file_exists($helperpath)) { + continue; + } + require_once($helperpath); + $helperclass = "qtype_{$qtype->name}_test_helper"; + if (!class_exists($helperclass)) { + continue; + } + $helper = new $helperclass(); + $testquestion = null; + foreach ($helper->get_test_questions() as $question) { + if (method_exists($helper, "get_{$qtype->name}_question_form_data_{$question}")) { + $testquestion = $question; + break; + } + } + $generators[$qtype->name] = [ + 'qtype' => $qtype->name, + 'testquestion' => $testquestion, + ]; + } + return $generators; + } + + /** + * Restore a quiz with questions of same stamp into the same course, but different answers. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_quiz_with_same_stamp_questions(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create 2 quizzes with 2 questions multichoice. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz1, 0); + + // Update question2 to have the same stamp as question1. + $DB->set_field('question', 'stamp', $question1->stamp, ['id' => $question2->id]); + + // Change the answers of the question2 to be different to question1. + $question2data = \question_bank::load_question_data($question2->id); + if (!isset($question2data->options->answers) || empty($question2data->options->answers)) { + $this->markTestSkipped( + "Cannot test edited answers for qtype_{$qtype} as it does not use answers.", + ); + } + foreach ($question2data->options->answers as $answer) { + $DB->set_field('question_answers', 'answer', 'edited', ['id' => $answer->id]); + } + + // Backup quiz1. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the newly-restored quiz uses the same question as quiz2. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz1->id, + \context_module::instance($quiz1->cmid), + ); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz2structure[1]->questionid, $quiz2structure[1]->questionid); + $this->assertEquals($quiz2structure[2]->questionid, $quiz2structure[2]->questionid); + } + + /** + * Restore a quiz with duplicate questions (same stamp and questions) into the same course. + * + * This is a contrived case, but this test serves as a control for the other tests in this class, proving that the hashing + * process will match an identical question. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_quiz_with_duplicate_questions(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create a quiz with 2 identical but separate questions. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz1, 0); + + // Update question2 to have the same times and stamp as question1. + $DB->update_record('question', [ + 'id' => $question2->id, + 'stamp' => $question1->stamp, + 'timecreated' => $question1->timecreated, + 'timemodified' => $question1->timemodified, + ]); + + // Backup quiz. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Expect that the restored quiz will have the second question in both its slots + // by virtue of identical stamp, version, and hash of question answer texts. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz2structure[1]->questionid, $quiz2structure[2]->questionid); + } + + /** + * Restore a quiz with questions that have the same stamp but different text. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_quiz_with_edited_questions(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create a quiz with 2 identical but separate questions. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1); + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + // Edit question 2 to have the same stamp and times as question1, but different text. + $DB->update_record('question', [ + 'id' => $question2->id, + 'questiontext' => 'edited', + 'stamp' => $question1->stamp, + 'timecreated' => $question1->timecreated, + 'timemodified' => $question1->timemodified, + ]); + quiz_add_quiz_question($question2->id, $quiz1); + + // Backup quiz. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // The quiz should contain both questions, as they have different text. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz2structure[1]->questionid, $question1->id); + $this->assertEquals($quiz2structure[2]->questionid, $question2->id); + } + + /** + * Restore a course to another course having questions with the same stamp in a shared question bank context category. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_course_with_same_stamp_questions(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create three courses and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $course2 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $generator->enrol_user($teacher->id, $course2->id, 'editingteacher'); + + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $systemcontext = \context_system::instance(); + $cat = $questiongenerator->create_question_category(['contextid' => $systemcontext->id]); + + // Create quiz with question. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + + $quiz2 = $this->create_test_quiz($course1); + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz2, 0); + + // Update question2 to have the same stamp as question1. + $DB->set_field('question', 'stamp', $question1->stamp, ['id' => $question2->id]); + + // Change the answers of the question2 to be different to question1. + $question2data = \question_bank::load_question_data($question2->id); + if (!isset($question2data->options->answers) || empty($question2data->options->answers)) { + $this->markTestSkipped( + "Cannot test edited answers for qtype_{$qtype} as it does not use answers.", + ); + } + foreach ($question2data->options->answers as $answer) { + $answer->answer = 'New answer ' . $answer->id; + $DB->update_record('question_answers', $answer); + } + + // Backup course1. + $bc = new backup_controller(backup::TYPE_1COURSE, $course1->id, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup, adding to course2. + $rc = new restore_controller($backupid, $course2->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the newly-restored course's quizzes use the same questions as their counterparts of course1. + $modules = get_fast_modinfo($course2->id)->get_instances_of('quiz'); + $course1structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz1->id, \context_module::instance($quiz1->cmid)); + $course2quiz1 = array_shift($modules); + $course2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $course2quiz1->instance, $course2quiz1->context); + $this->assertEquals($course1structure[1]->questionid, $course2structure[1]->questionid); + + $course1structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz2->id, \context_module::instance($quiz2->cmid)); + $course2quiz2 = array_shift($modules); + $course2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $course2quiz2->instance, $course2quiz2->context); + $this->assertEquals($course1structure[1]->questionid, $course2structure[1]->questionid); + } + + /** + * Restore a quiz with questions of same stamp into the same course, but different hints. + * + * @dataProvider get_qtype_generators + * @param string $qtype The name of the qtype plugin to test + * @param ?string $testquestion The test question to generate for the plugin. If null, the plugin will be skipped + * with a message. + */ + public function test_restore_quiz_with_same_stamp_questions_edited_hints(string $qtype, ?string $testquestion): void { + global $DB, $USER; + if (is_null($testquestion)) { + $this->markTestSkipped( + "Cannot test qtype_{$qtype} as there is no test question with a form_data method in the " . + "test helper class." + ); + } + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // Create 2 questions multichoice. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + + $question2 = $questiongenerator->create_question($qtype, $testquestion, ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz1, 0); + + // Update question2 to have the same stamp as question1. + $DB->set_field('question', 'stamp', $question1->stamp, ['id' => $question2->id]); + + // Change the hints of the question2 to be different to question1. + $hints = $DB->get_records('question_hints', ['questionid' => $question2->id]); + if (empty($hints)) { + $this->markTestSkipped( + "Cannot test edited hints for qtype_{$qtype} as test question {$testquestion} does not use hints.", + ); + } + foreach ($hints as $hint) { + $DB->set_field('question_hints', 'hint', "{$hint->hint} edited", ['id' => $hint->id]); + } + + // Backup quiz1. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the newly-restored quiz uses the same question as quiz2. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz1structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz1->id, + \context_module::instance($quiz1->cmid), + ); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz1structure[1]->questionid, $quiz2structure[1]->questionid); + $this->assertEquals($quiz1structure[2]->questionid, $quiz2structure[2]->questionid); + + } + + /** + * Return a set of options fields and new values. + * + * @return array + */ + public static function get_edited_option_fields(): array { + return [ + 'single' => [ + 'single', + '0', + ], + 'shuffleanswers' => [ + 'shuffleanswers', + '0', + ], + 'answernumbering' => [ + 'answernumbering', + 'ABCD', + ], + 'shownumcorrect' => [ + 'shownumcorrect', + '0', + ], + 'showstandardinstruction' => [ + 'showstandardinstruction', + '1', + ], + 'correctfeedback' => [ + 'correctfeedback', + 'edited', + ], + 'partiallycorrectfeedback' => [ + 'partiallycorrectfeedback', + 'edited', + ], + 'incorrectfeedback' => [ + 'incorrectfeedback', + 'edited', + ], + ]; + } + + /** + * Restore a quiz with questions of same stamp into the same course, but different qtype-specific options. + * + * @dataProvider get_edited_option_fields + * @param string $field The answer field to edit + * @param string $value The value to set + */ + public function test_restore_quiz_with_same_stamp_questions_edited_options(string $field, string $value): void { + global $DB, $USER; + $this->resetAfterTest(); + $this->setAdminUser(); + + // Create a course and a user with editing teacher capabilities. + $generator = $this->getDataGenerator(); + $course1 = $generator->create_course(); + $teacher = $USER; + $generator->enrol_user($teacher->id, $course1->id, 'editingteacher'); + $coursecontext = \context_course::instance($course1->id); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a question category. + $cat = $questiongenerator->create_question_category(['contextid' => $coursecontext->id]); + + // A quiz with 2 multichoice questions. + $quiz1 = $this->create_test_quiz($course1); + $question1 = $questiongenerator->create_question('multichoice', 'one_of_four', ['category' => $cat->id]); + quiz_add_quiz_question($question1->id, $quiz1, 0); + + $question2 = $questiongenerator->create_question('multichoice', 'one_of_four', ['category' => $cat->id]); + quiz_add_quiz_question($question2->id, $quiz1, 0); + + // Update question2 to have the same stamp as question1. + $DB->set_field('question', 'stamp', $question1->stamp, ['id' => $question2->id]); + + // Change the options of question2 to be different to question1. + $DB->set_field('qtype_multichoice_options', $field, $value, ['questionid' => $question2->id]); + + // Backup quiz. + $bc = new backup_controller(backup::TYPE_1ACTIVITY, $quiz1->cmid, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_IMPORT, $teacher->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + // Restore the backup into the same course. + $rc = new restore_controller($backupid, $course1->id, backup::INTERACTIVE_NO, backup::MODE_IMPORT, + $teacher->id, backup::TARGET_CURRENT_ADDING); + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + // Verify that the newly-restored quiz questions match their quiz1 counterparts. + $modules = get_fast_modinfo($course1->id)->get_instances_of('quiz'); + $this->assertCount(2, $modules); + $quiz1structure = \mod_quiz\question\bank\qbank_helper::get_question_structure( + $quiz1->id, + \context_module::instance($quiz1->cmid), + ); + $quiz2 = end($modules); + $quiz2structure = \mod_quiz\question\bank\qbank_helper::get_question_structure($quiz2->instance, $quiz2->context); + $this->assertEquals($quiz1structure[1]->questionid, $quiz2structure[1]->questionid); + $this->assertEquals($quiz1structure[2]->questionid, $quiz2structure[2]->questionid); + } }