diff --git a/lang/en/admin.php b/lang/en/admin.php index 34ed615a5d3..48f4661d3c5 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1243,7 +1243,8 @@ $string['taskpasswordresetcleanup'] = 'Cleanup password reset attempts'; $string['taskplagiarismcron'] = 'Background processing for legacy cron in plagiarism plugins'; $string['taskportfoliocron'] = 'Background processing for portfolio plugins'; $string['taskprocessing'] = 'Task processing'; -$string['taskquestioncron'] = 'Background processing for question engine'; +$string['taskquestioncron'] = 'Background processing for cleaning up the old question previews'; +$string['taskquestionstatscleanupcron'] = 'Background processing for cleaning up the old question statistics cache'; $string['taskrefreshsystemtokens'] = 'Refresh OAuth tokens for service accounts'; $string['taskregistrationcron'] = 'Site registration'; $string['tasksendfailedloginnotifications'] = 'Send failed login notifications'; diff --git a/lib/classes/task/question_cron_task.php b/lib/classes/task/question_cron_task.php deleted file mode 100644 index d7d9cc5063b..00000000000 --- a/lib/classes/task/question_cron_task.php +++ /dev/null @@ -1,53 +0,0 @@ -. - -/** - * A scheduled task. - * - * @package core - * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -namespace core\task; - -/** - * Simple task to run the question cron. - */ -class question_cron_task extends scheduled_task { - - /** - * Get a descriptive name for this task (shown to admins). - * - * @return string - */ - public function get_name() { - return get_string('taskquestioncron', 'admin'); - } - - /** - * Do the job. - * Throw exceptions on errors (the job will be retried). - */ - public function execute() { - global $CFG; - - // Run question bank clean-up. - require_once($CFG->libdir . '/questionlib.php'); - \question_bank::cron(); - - } - -} diff --git a/lib/classes/task/question_preview_cleanup_task.php b/lib/classes/task/question_preview_cleanup_task.php new file mode 100644 index 00000000000..1044373c8f7 --- /dev/null +++ b/lib/classes/task/question_preview_cleanup_task.php @@ -0,0 +1,73 @@ +. + +/** + * Task to cleanup old question previews. + * + * @package core + * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace core\task; + +/** + * A task to cleanup old question previews. + * + * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_preview_cleanup_task extends scheduled_task { + + /** + * Get a descriptive name for this task (shown to admins). + * + * @return string + */ + public function get_name() { + return get_string('taskquestioncron', 'admin'); + } + + /** + * Do the job. + * Throw exceptions on errors (the job will be retried). + */ + public function execute() { + + // We delete previews that have not been touched for 24 hours. + $lastmodifiedcutoff = time() - DAYSECS; + + mtrace("\n Cleaning up old question previews...", ''); + $oldpreviews = new \qubaid_join('{question_usages} quba', 'quba.id', + 'quba.component = :qubacomponent + AND NOT EXISTS ( + SELECT 1 + FROM {question_attempts} subq_qa + JOIN {question_attempt_steps} subq_qas ON subq_qas.questionattemptid = subq_qa.id + JOIN {question_usages} subq_qu ON subq_qu.id = subq_qa.questionusageid + WHERE subq_qa.questionusageid = quba.id + AND subq_qu.component = :qubacomponent2 + AND (subq_qa.timemodified > :qamodifiedcutoff + OR subq_qas.timecreated > :stepcreatedcutoff) + ) + ', + ['qubacomponent' => 'core_question_preview', 'qubacomponent2' => 'core_question_preview', + 'qamodifiedcutoff' => $lastmodifiedcutoff, 'stepcreatedcutoff' => $lastmodifiedcutoff]); + + \question_engine::delete_questions_usage_by_activities($oldpreviews); + mtrace('done.'); + } + +} diff --git a/lib/classes/task/question_stats_cleanup_task.php b/lib/classes/task/question_stats_cleanup_task.php new file mode 100644 index 00000000000..651cc8dcdbc --- /dev/null +++ b/lib/classes/task/question_stats_cleanup_task.php @@ -0,0 +1,65 @@ +. + +/** + * Task to cleanup old question statistics cache. + * + * @package core + * @copyright 2019 Simey Lameze + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace core\task; + +defined('MOODLE_INTERNAL') || die(); + +/** + * A task to cleanup old question statistics cache. + * + * @copyright 2019 Simey Lameze + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_stats_cleanup_task extends scheduled_task { + + /** + * Get a descriptive name for this task (shown to admins). + * + * @return string + */ + public function get_name() { + return get_string('taskquestionstatscleanupcron', 'admin'); + } + + /** + * Perform the cleanup task. + */ + public function execute() { + global $DB; + + mtrace("\n Cleaning up old question statistics cache records...", ''); + + $expiretime = time() - 5 * HOURSECS; + $DB->delete_records_select('question_statistics', 'timemodified < ?', [$expiretime]); + $responseanlysisids = $DB->get_records_select_menu('question_response_analysis', + 'timemodified < ?', + [$expiretime], + 'id', + 'id, id AS id2'); + $DB->delete_records_list('question_response_analysis', 'id', $responseanlysisids); + $DB->delete_records_list('question_response_count', 'analysisid', $responseanlysisids); + + mtrace('done.'); + } +} diff --git a/lib/db/tasks.php b/lib/db/tasks.php index f78f36ad534..85381d442e0 100644 --- a/lib/db/tasks.php +++ b/lib/db/tasks.php @@ -213,7 +213,16 @@ $tasks = array( 'month' => '*' ), array( - 'classname' => 'core\task\question_cron_task', + 'classname' => 'core\task\question_preview_cleanup_task', + 'blocking' => 0, + 'minute' => '*', + 'hour' => '*', + 'day' => '*', + 'dayofweek' => '*', + 'month' => '*' + ), + array( + 'classname' => 'core\task\question_stats_cleanup_task', 'blocking' => 0, 'minute' => '*', 'hour' => '*', diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 0b0e59dacd2..d5300164973 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2947,5 +2947,20 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2019032900.00); } + if ($oldversion < 2019032900.01) { + $sql = "UPDATE {task_scheduled} + SET classname = ? + WHERE component = ? + AND classname = ?"; + $DB->execute($sql, [ + '\core\task\question_preview_cleanup_task', + 'moodle', + '\core\task\question_cron_task' + ]); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2019032900.01); + } + return true; } diff --git a/question/engine/bank.php b/question/engine/bank.php index 68c6aaf1357..61b01a7229e 100644 --- a/question/engine/bank.php +++ b/question/engine/bank.php @@ -408,21 +408,6 @@ abstract class question_bank { return self::$fractionoptionsfull; } - /** - * Perform scheduled maintenance tasks relating to the question bank. - */ - public static function cron() { - global $CFG; - - // Delete any old question preview that got left in the database. - require_once($CFG->dirroot . '/question/previewlib.php'); - question_preview_cron(); - - // Clear older calculated stats from cache. - require_once($CFG->dirroot . '/question/engine/statisticslib.php'); - question_usage_statistics_cron(); - } - /** * Return a list of the different question types present in the given categories. * diff --git a/question/engine/statisticslib.php b/question/engine/statisticslib.php deleted file mode 100644 index 9538346b58a..00000000000 --- a/question/engine/statisticslib.php +++ /dev/null @@ -1,51 +0,0 @@ -. - -/** - * Functions common to the question usage statistics code. - * - * @package moodlecore - * @subpackage questionbank - * @copyright 2013 The Open University - * @author Jamie Pratt - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -defined('MOODLE_INTERNAL') || die(); - -/** - * Question statistics cron code. Deletes cached stats more than a certain age. - */ -function question_usage_statistics_cron() { - global $DB; - - $expiretime = time() - 5 * HOURSECS; - - mtrace("\n Cleaning up old question statistics cache records...", ''); - - $DB->delete_records_select('question_statistics', 'timemodified < ?', array($expiretime)); - $responseanlysisids = $DB->get_records_select_menu('question_response_analysis', - 'timemodified < ?', - array($expiretime), - 'id', - 'id, id AS id2'); - - $DB->delete_records_list('question_response_analysis', 'id', $responseanlysisids); - $DB->delete_records_list('question_response_count', 'analysisid', $responseanlysisids); - - mtrace('done.'); - return true; -} diff --git a/question/previewlib.php b/question/previewlib.php index bd92049122a..588e435a4ca 100644 --- a/question/previewlib.php +++ b/question/previewlib.php @@ -327,32 +327,3 @@ function restart_preview($previewid, $questionid, $displayoptions, $context) { redirect(question_preview_url($questionid, $displayoptions->behaviour, $displayoptions->maxmark, $displayoptions, $displayoptions->variant, $context)); } - -/** - * Scheduled tasks relating to question preview. Specifically, delete any old - * previews that are left over in the database. - */ -function question_preview_cron() { - $maxage = 24*60*60; // We delete previews that have not been touched for 24 hours. - $lastmodifiedcutoff = time() - $maxage; - - mtrace("\n Cleaning up old question previews...", ''); - $oldpreviews = new qubaid_join('{question_usages} quba', 'quba.id', - 'quba.component = :qubacomponent - AND NOT EXISTS ( - SELECT 1 - FROM {question_attempts} subq_qa - JOIN {question_attempt_steps} subq_qas ON subq_qas.questionattemptid = subq_qa.id - JOIN {question_usages} subq_qu ON subq_qu.id = subq_qa.questionusageid - WHERE subq_qa.questionusageid = quba.id - AND subq_qu.component = :qubacomponent2 - AND (subq_qa.timemodified > :qamodifiedcutoff - OR subq_qas.timecreated > :stepcreatedcutoff) - ) - ', - array('qubacomponent' => 'core_question_preview', 'qubacomponent2' => 'core_question_preview', - 'qamodifiedcutoff' => $lastmodifiedcutoff, 'stepcreatedcutoff' => $lastmodifiedcutoff)); - - question_engine::delete_questions_usage_by_activities($oldpreviews); - mtrace('done.'); -} diff --git a/question/tests/previewlib_test.php b/question/tests/previewlib_test.php deleted file mode 100644 index b0215553f9e..00000000000 --- a/question/tests/previewlib_test.php +++ /dev/null @@ -1,116 +0,0 @@ -. - -/** - * 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')); - } -} diff --git a/question/upgrade.txt b/question/upgrade.txt index 3310109ad5f..94f5e589a3c 100644 --- a/question/upgrade.txt +++ b/question/upgrade.txt @@ -11,6 +11,10 @@ The exportprocess function of the qformat_default class doesn't output a blank l if the result of the writequestion function is null. This permit to qformat plugins to ignore some questions without the need to overwrite this function. +* The question_preview_cron() has been deleted. Please use \core\task\question_cron_task::execute(). +* The question_usage_statistics_cron() has been deleted. Please use \core\task\question_cron_task::execute(). +* The method question_bank::cron() has been deleted, please use question related scheduled tasks. + === 3.5 === 1) The question format exportprocess function now adds a diff --git a/version.php b/version.php index eb1f55bf886..083c6cd0d66 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2019032900.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2019032900.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.