Skip to content

Commit

Permalink
MDL-52339 question: Fix question attempt removal for MySQL
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
andrewnicols authored and stronk7 committed Feb 8, 2016
1 parent be82fa5 commit db80cf5
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 16 deletions.
33 changes: 17 additions & 16 deletions question/engine/datalib.php
Expand Up @@ -980,23 +980,24 @@ public function delete_questions_usage_by_activities(qubaid_condition $qubaids)
* @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);
}
}

/**
Expand Down
116 changes: 116 additions & 0 deletions question/tests/previewlib_test.php
@@ -0,0 +1,116 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* 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'));
}
}

0 comments on commit db80cf5

Please sign in to comment.