From 5a01c2409a5a446b2891ae9bd2b45e0d0ca76edc Mon Sep 17 00:00:00 2001 From: Simey Lameze Date: Thu, 28 Mar 2019 16:15:37 +0800 Subject: [PATCH 1/4] MDL-65050 core: rename task to question_preview_cleanup_task --- lang/en/admin.php | 2 +- ...task.php => question_preview_cleanup_task.php} | 9 ++++++--- lib/db/tasks.php | 2 +- lib/db/upgrade.php | 15 +++++++++++++++ version.php | 2 +- 5 files changed, 24 insertions(+), 6 deletions(-) rename lib/classes/task/{question_cron_task.php => question_preview_cleanup_task.php} (82%) diff --git a/lang/en/admin.php b/lang/en/admin.php index 34ed615a5d3..d3f95c54946 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1243,7 +1243,7 @@ $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['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_preview_cleanup_task.php similarity index 82% rename from lib/classes/task/question_cron_task.php rename to lib/classes/task/question_preview_cleanup_task.php index d7d9cc5063b..fc0348d4e0e 100644 --- a/lib/classes/task/question_cron_task.php +++ b/lib/classes/task/question_preview_cleanup_task.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * A scheduled task. + * Task to cleanup old question previews. * * @package core * @copyright 2013 onwards Martin Dougiamas http://dougiamas.com @@ -24,9 +24,12 @@ namespace core\task; /** - * Simple task to run the question cron. + * 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_cron_task extends scheduled_task { +class question_preview_cleanup_task extends scheduled_task { /** * Get a descriptive name for this task (shown to admins). diff --git a/lib/db/tasks.php b/lib/db/tasks.php index f78f36ad534..2350af2e663 100644 --- a/lib/db/tasks.php +++ b/lib/db/tasks.php @@ -213,7 +213,7 @@ $tasks = array( 'month' => '*' ), array( - 'classname' => 'core\task\question_cron_task', + 'classname' => 'core\task\question_preview_cleanup_task', 'blocking' => 0, 'minute' => '*', 'hour' => '*', diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 7d28a631551..a39d9ac74a9 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2923,5 +2923,20 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2019032200.02); } + if ($oldversion < 2019032800.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, 2019032800.01); + } + return true; } diff --git a/version.php b/version.php index eeaaa852057..ef8f0065184 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2019032800.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2019032800.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. From 10810f421bd0c43f0e28dd9f3c974e2e55710c1f Mon Sep 17 00:00:00 2001 From: Simey Lameze Date: Thu, 28 Mar 2019 16:28:04 +0800 Subject: [PATCH 2/4] MDL-65050 core: move function code to question_preview_cleanup_task This commit also deletes the legacy question_preview_cron() function. --- .../task/question_preview_cleanup_task.php | 25 +++++++++++++--- question/previewlib.php | 29 ------------------- 2 files changed, 21 insertions(+), 33 deletions(-) diff --git a/lib/classes/task/question_preview_cleanup_task.php b/lib/classes/task/question_preview_cleanup_task.php index fc0348d4e0e..1044373c8f7 100644 --- a/lib/classes/task/question_preview_cleanup_task.php +++ b/lib/classes/task/question_preview_cleanup_task.php @@ -45,12 +45,29 @@ class question_preview_cleanup_task extends scheduled_task { * 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(); + // 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/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.'); -} From 9c164d26fc9f62fae8eae10f4a36e80f8bc70e16 Mon Sep 17 00:00:00 2001 From: Simey Lameze Date: Thu, 28 Mar 2019 16:32:21 +0800 Subject: [PATCH 3/4] MDL-65050 core: add question_stats_cleanup_task scheduled task This commit also removes orphaned question/engine/statisticslib.php --- lang/en/admin.php | 1 + .../task/question_stats_cleanup_task.php | 65 +++++++++++++++++++ lib/db/tasks.php | 9 +++ question/engine/statisticslib.php | 51 --------------- 4 files changed, 75 insertions(+), 51 deletions(-) create mode 100644 lib/classes/task/question_stats_cleanup_task.php delete mode 100644 question/engine/statisticslib.php diff --git a/lang/en/admin.php b/lang/en/admin.php index d3f95c54946..48f4661d3c5 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1244,6 +1244,7 @@ $string['taskplagiarismcron'] = 'Background processing for legacy cron in plagia $string['taskportfoliocron'] = 'Background processing for portfolio plugins'; $string['taskprocessing'] = 'Task processing'; $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_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 2350af2e663..85381d442e0 100644 --- a/lib/db/tasks.php +++ b/lib/db/tasks.php @@ -221,6 +221,15 @@ $tasks = array( 'dayofweek' => '*', 'month' => '*' ), + array( + 'classname' => 'core\task\question_stats_cleanup_task', + 'blocking' => 0, + 'minute' => '*', + 'hour' => '*', + 'day' => '*', + 'dayofweek' => '*', + 'month' => '*' + ), array( 'classname' => 'core\task\registration_cron_task', 'blocking' => 0, 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; -} From d499cb35931c728ff6d0d38009c0729c053ed9bd Mon Sep 17 00:00:00 2001 From: Simey Lameze Date: Thu, 28 Mar 2019 16:38:49 +0800 Subject: [PATCH 4/4] MDL-65050 core: delete orphaned files and functions --- question/engine/bank.php | 15 ---- question/tests/previewlib_test.php | 116 ----------------------------- question/upgrade.txt | 4 + 3 files changed, 4 insertions(+), 131 deletions(-) delete mode 100644 question/tests/previewlib_test.php 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/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