From 650c0086f9347dab9ecb8adcc9281c68fb1d616e Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 2 Feb 2016 15:37:17 +0800 Subject: [PATCH] MDL-52339 question: Fix question attempt removal for MySQL Derived table support was altered in MySQL 5.7 changing the way in which DELETE FROM works in some cases. This change modifies the way in which deletion occurs by selecting all IDs and batching them into groups of 1000. --- question/engine/datalib.php | 33 ++++---- question/tests/previewlib_test.php | 116 +++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 16 deletions(-) create mode 100644 question/tests/previewlib_test.php diff --git a/question/engine/datalib.php b/question/engine/datalib.php index c9e18840eff..0731252ff0b 100644 --- a/question/engine/datalib.php +++ b/question/engine/datalib.php @@ -980,23 +980,24 @@ ORDER BY * @param qubaid_condition $qubaids identifies which question useages to delete. */ protected function delete_usage_records_for_mysql(qubaid_condition $qubaids) { - $qubaidtest = $qubaids->usage_id_in(); - if (strpos($qubaidtest, 'question_usages') !== false && - strpos($qubaidtest, 'IN (SELECT') === 0) { - // This horrible hack is required by MDL-29847. It comes from - // http://www.xaprb.com/blog/2006/06/23/how-to-select-from-an-update-target-in-mysql/ - $qubaidtest = 'IN (SELECT * FROM ' . substr($qubaidtest, 3) . ' AS hack_subquery_alias)'; - } - - // TODO once MDL-29589 is fixed, eliminate this method, and instead use the new $DB API. - $this->db->execute(' - DELETE qu, qa, qas, qasd - FROM {question_usages} qu - JOIN {question_attempts} qa ON qa.questionusageid = qu.id - LEFT JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id - LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id - WHERE qu.id ' . $qubaidtest, + // Get the list of question attempts to delete and delete them in chunks. + $allids = $this->db->get_records_sql_menu(" + SELECT DISTINCT id, id AS id2 + FROM {question_usages} + WHERE id " . $qubaids->usage_id_in(), $qubaids->usage_id_in_params()); + + foreach (array_chunk($allids, 1000) as $todelete) { + list($idsql, $idparams) = $this->db->get_in_or_equal($todelete); + $this->db->execute(' + DELETE qu, qa, qas, qasd + FROM {question_usages} qu + JOIN {question_attempts} qa ON qa.questionusageid = qu.id + LEFT JOIN {question_attempt_steps} qas ON qas.questionattemptid = qa.id + LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id + WHERE qu.id ' . $idsql, + $idparams); + } } /** diff --git a/question/tests/previewlib_test.php b/question/tests/previewlib_test.php new file mode 100644 index 00000000000..b0215553f9e --- /dev/null +++ b/question/tests/previewlib_test.php @@ -0,0 +1,116 @@ +. + +/** + * Quiz events tests. + * + * @package mod_quiz + * @category phpunit + * @copyright 2013 Adrian Greeve + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/question/previewlib.php'); + +/** + * Unit tests for question preview. + * + * @package question + * @category phpunit + * @copyright 2016 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_previewlib_testcase extends advanced_testcase { + + /** + * Setup some convenience test data with a single attempt. + * + * @return question_usage_by_activity + */ + protected function prepare_question_data() { + $this->resetAfterTest(true); + + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + // Create a questions and start the preview. + $cat = $questiongenerator->create_question_category(); + + $quba = question_engine::make_questions_usage_by_activity('core_question_preview', context_system::instance()); + $quba->set_preferred_behaviour('deferredfeedback'); + $questiondata = $questiongenerator->create_question('numerical', null, array('category' => $cat->id)); + $question = question_bank::load_question($questiondata->id); + $quba->add_question($question); + $quba->start_all_questions(); + question_engine::save_questions_usage_by_activity($quba); + + return $quba; + } + + /** + * Test the attempt deleted event. + */ + public function test_question_preview_cron() { + global $DB; + + // Create some quiz data. + // This will create two questions. + $quba1 = $this->prepare_question_data(); + + // Run the cron. + ob_start(); + question_preview_cron(); + $output = ob_get_clean(); + $this->assertEquals("\n Cleaning up old question previews...done.\n", $output); + + // The attempt should not have been removed. + // There should be one question usage with two question attempts. + $this->assertEquals(1, $DB->count_records('question_usages', array('id' => $quba1->get_id()))); + $this->assertEquals(1, $DB->count_records('question_attempts', array('questionusageid' => $quba1->get_id()))); + $this->assertEquals(1, $DB->count_records('question_attempt_steps')); + $this->assertEquals(1, $DB->count_records('question_attempt_step_data')); + + // Update the timemodified and timecreated to be in the past. + $DB->set_field('question_attempts', 'timemodified', time() - WEEKSECS); + $DB->set_field('question_attempt_steps', 'timecreated', time() - WEEKSECS); + + // Create some quiz data. + // This will create two questions. + $quba2 = $this->prepare_question_data(); + + // There will now be 2 usages, etc. + $this->assertEquals(2, $DB->count_records('question_usages')); + $this->assertEquals(2, $DB->count_records('question_attempts')); + $this->assertEquals(2, $DB->count_records('question_attempt_steps')); + $this->assertEquals(2, $DB->count_records('question_attempt_step_data')); + + // Run the cron again. + // $quba1 will be removed, but $quba2 should still be present. + ob_start(); + question_preview_cron(); + $output = ob_get_clean(); + $this->assertEquals("\n Cleaning up old question previews...done.\n", $output); + + $this->assertEquals(0, $DB->count_records('question_usages', array('id' => $quba1->get_id()))); + $this->assertEquals(0, $DB->count_records('question_attempts', array('questionusageid' => $quba1->get_id()))); + $this->assertEquals(1, $DB->count_records('question_usages', array('id' => $quba2->get_id()))); + $this->assertEquals(1, $DB->count_records('question_attempts', array('questionusageid' => $quba2->get_id()))); + $this->assertEquals(1, $DB->count_records('question_attempt_steps')); + $this->assertEquals(1, $DB->count_records('question_attempt_step_data')); + } +}