Merge branch 'MDL-78580_402_STABLE' of https://github.com/marxjohnson/moodle into MOODLE_402_STABLE

This commit is contained in:
Andrew Nicols
2023-10-03 11:23:23 +08:00
8 changed files with 413 additions and 33 deletions
+76
View File
@@ -0,0 +1,76 @@
<?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/>.
namespace core\lock;
use core\progress\base;
/**
* Lock utilities.
*
* @package core
* @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class lock_utils {
/**
* Start a progress bar and attempt to get a lock, updating the bar until a lock is achieved.
*
* This will make multiple attempts at getting the lock using a short timeout set by $progressupdatetime. After
* each failed attempt, it will update the progress bar and try again, until $timeout is reached.
*
* @param lock_factory $lockfactory The factory to use to get the lock
* @param string $resource The resource key we will try to get a lock on
* @param base $progress The progress bar
* @param int $timeout The maximum time in seconds to wait for a lock
* @param string $message Optional message to display on the progress bar
* @param int $progressupdatetime The number of seconds to wait for each lock attempt before updating the progress bar.
* @param int $maxlifetime The maxlifetime to set on the lock, if supported.
* @return lock|false A lock if successful, or false if the timeout expires.
* @throws \coding_exception
*/
public static function wait_for_lock_with_progress(
lock_factory $lockfactory,
string $resource,
\core\progress\base $progress,
int $timeout,
string $message = '',
int $progressupdatetime = 10,
int $maxlifetime = DAYSECS,
) {
if ($progressupdatetime < 1) {
throw new \invalid_parameter_exception('Progress bar cannot update more than once per second. ' .
'$progressupdate time must be at least 1.');
}
if ($progressupdatetime > $timeout) {
throw new \invalid_parameter_exception('$timeout must be greater than $progressupdatetime.');
}
$lockattempts = 0;
$maxattempts = $timeout / $progressupdatetime;
$lock = false;
$progress->start_progress($message);
while (!$lock && $lockattempts < $maxattempts) {
$lock = $lockfactory->get_lock($resource, $progressupdatetime, $maxlifetime);
if (!$lock) {
$progress->progress();
$lockattempts++;
}
}
$progress->end_progress();
return $lock;
}
}
+1 -1
View File
@@ -2518,5 +2518,5 @@ function mod_quiz_calculate_question_stats(context $context): ?all_calculated_fo
require_once($CFG->dirroot . '/mod/quiz/report/statistics/report.php');
$cm = get_coursemodule_from_id('quiz', $context->instanceid);
$report = new quiz_statistics_report();
return $report->calculate_questions_stats_for_question_bank($cm->instance, false);
return $report->calculate_questions_stats_for_question_bank($cm->instance, false, false);
}
@@ -62,6 +62,10 @@ $string['firstattempts'] = 'first attempts';
$string['firstattemptsavg'] = 'Average grade of first attempts';
$string['firstattemptscount'] = 'Number of complete graded first attempts';
$string['frequency'] = 'Frequency';
$string['getstatslockprogress'] = 'Waiting for task in progress. Please wait or try again later.';
$string['getstatslocktimeout'] = 'Statistics calculation lock timeout';
$string['getstatslocktimeoutdesc'] = 'How many seconds to wait for a lock when attempting to perform a statistics calculation ' .
'for a quiz. This setting primarily exists for testing, do not modify it unless you know what you are doing.';
$string['highestattempts'] = 'highest graded attempt';
$string['highestattemptsavg'] = 'Average grade of highest graded attempts';
$string['intended_weight'] = 'Intended weight';
@@ -83,6 +87,7 @@ Our equation for effective question weight cannot be calculated in this case. Th
If you edit a quiz and give these question(s) with negative covariance a max grade of zero then the effective question weight of these questions will be zero and the real effective question weight of other questions will be as calculated now.';
$string['nogradedattempts'] = 'No attempts have been made at this quiz, or all attempts have questions that need manual grading.';
$string['nostudentsingroup'] = 'There are no students in this group yet';
$string['nostats'] = 'Could not complete the statistics calculation. There may be a long-running calculation in progress. Please try again later.';
$string['optiongrade'] = 'Partial credit';
$string['partofquestion'] = 'Part of question';
$string['pluginname'] = 'Statistics';
+81 -28
View File
@@ -157,6 +157,10 @@ class quiz_statistics_report extends report_base {
$progress = $this->get_progress_trace_instance();
list($quizstats, $questionstats) =
$this->get_all_stats_and_analysis($quiz, $whichattempts, $whichtries, $groupstudentsjoins, $questions, $progress);
if (is_null($quizstats)) {
echo $OUTPUT->notification(get_string('nostats', 'quiz_statistics'), 'error');
return true;
}
} else {
// Or create empty stats containers.
$quizstats = new \quiz_statistics\calculated($whichattempts);
@@ -624,13 +628,15 @@ class quiz_statistics_report extends report_base {
* @param \core\progress\base|null $progress
* @param bool $calculateifrequired if true (the default) the stats will be calculated if not already stored.
* If false, [null, null] will be returned if the stats are not already available.
* @param bool $performanalysis if true (the default) and there are calculated stats, analysis will be performed
* for each question.
* @return array with 2 elements: - $quizstats The statistics for overall attempt scores.
* - $questionstats \core_question\statistics\questions\all_calculated_for_qubaid_condition
* Both may be null, if $calculateifrequired is false.
*/
public function get_all_stats_and_analysis(
$quiz, $whichattempts, $whichtries, \core\dml\sql_join $groupstudentsjoins,
$questions, $progress = null, bool $calculateifrequired = true) {
$questions, $progress = null, bool $calculateifrequired = true, bool $performanalysis = true) {
if ($progress === null) {
$progress = new \core\progress\none();
@@ -642,38 +648,82 @@ class quiz_statistics_report extends report_base {
$quizcalc = new \quiz_statistics\calculator($progress);
$progress->start_progress('', 3);
if ($quizcalc->get_last_calculated_time($qubaids) === false) {
$progress->start_progress('', 4);
// Get a lock on this set of qubaids before performing calculations. This prevents the same calculation running
// concurrently and causing database deadlocks. We use a long timeout here as a big quiz with lots of attempts may
// take a long time to process.
$lockfactory = \core\lock\lock_config::get_lock_factory('quiz_statistics_get_stats');
$lock = $lockfactory->get_lock($qubaids->get_hash_code(), 0);
if (!$lock) {
if (!$calculateifrequired) {
$progress->progress(3);
// We're not going to do the calculation in this request anyway, so just give up here.
$progress->progress(4);
$progress->end_progress();
return [null, null];
}
// Recalculate now.
$questionstats = $qcalc->calculate($qubaids);
$progress->progress(1);
$quizstats = $quizcalc->calculate($quiz->id, $whichattempts, $groupstudentsjoins, count($questions),
$qcalc->get_sum_of_mark_variance());
$progress->progress(2);
} else {
$quizstats = $quizcalc->get_cached($qubaids);
$progress->progress(1);
$questionstats = $qcalc->get_cached($qubaids);
$progress->progress(2);
$locktimeout = get_config('quiz_statistics', 'getstatslocktimeout');
$lock = \core\lock\lock_utils::wait_for_lock_with_progress(
$lockfactory,
$qubaids->get_hash_code(),
$progress,
$locktimeout,
get_string('getstatslockprogress', 'quiz_statistics'),
);
if (!$lock) {
// Lock attempt timed out.
$progress->progress(4);
$progress->end_progress();
debugging('Could not get lock on ' .
$qubaids->get_hash_code() . ' (Quiz ID ' . $quiz->id . ') after ' .
$locktimeout . ' seconds');
return [null, null];
}
}
if ($quizstats->s()) {
$subquestions = $questionstats->get_sub_questions();
$this->analyse_responses_for_all_questions_and_subquestions($questions,
$subquestions,
$qubaids,
$whichtries,
$progress);
try {
if ($quizcalc->get_last_calculated_time($qubaids) === false) {
if (!$calculateifrequired) {
$progress->progress(4);
$progress->end_progress();
$lock->release();
return [null, null];
}
// Recalculate now.
$questionstats = $qcalc->calculate($qubaids);
$progress->progress(2);
$quizstats = $quizcalc->calculate(
$quiz->id,
$whichattempts,
$groupstudentsjoins,
count($questions),
$qcalc->get_sum_of_mark_variance()
);
$progress->progress(3);
} else {
$quizstats = $quizcalc->get_cached($qubaids);
$progress->progress(2);
$questionstats = $qcalc->get_cached($qubaids);
$progress->progress(3);
}
if ($quizstats->s() && $performanalysis) {
$subquestions = $questionstats->get_sub_questions();
$this->analyse_responses_for_all_questions_and_subquestions(
$questions,
$subquestions,
$qubaids,
$whichtries,
$progress
);
}
$progress->progress(4);
$progress->end_progress();
} finally {
$lock->release();
}
$progress->progress(3);
$progress->end_progress();
return [$quizstats, $questionstats];
}
@@ -932,11 +982,14 @@ class quiz_statistics_report extends report_base {
* @param int $quizid question usage
* @param bool $calculateifrequired if true (the default) the stats will be calculated if not already stored.
* If false, null will be returned if the stats are not already available.
* @param bool $performanalysis if true (the default) and there are calculated stats, analysis will be performed
* for each question.
* @return ?all_calculated_for_qubaid_condition question stats
*/
public function calculate_questions_stats_for_question_bank(
int $quizid,
bool $calculateifrequired = true
bool $calculateifrequired = true,
bool $performanalysis = true,
): ?all_calculated_for_qubaid_condition {
global $DB;
$quiz = $DB->get_record('quiz', ['id' => $quizid], '*', MUST_EXIST);
@@ -944,7 +997,7 @@ class quiz_statistics_report extends report_base {
[, $questionstats] = $this->get_all_stats_and_analysis($quiz,
$quiz->grademethod, question_attempt::ALL_TRIES, new \core\dml\sql_join(),
$questions, null, $calculateifrequired);
$questions, null, $calculateifrequired, $performanalysis);
return $questionstats;
}
+35
View File
@@ -0,0 +1,35 @@
<?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/>.
/**
* Settings for the Statisics report
*
* @package quiz_statistics
* @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
defined('MOODLE_INTERNAL') || die;
if ($ADMIN->fulltree) {
$settings->add(new admin_setting_configtext(
'quiz_statistics/getstatslocktimeout',
get_string('getstatslocktimeout', 'quiz_statistics'),
get_string('getstatslocktimeoutdesc', 'quiz_statistics'),
MINSECS * 15
));
}
@@ -0,0 +1,212 @@
<?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/>.
namespace quiz_statistics;
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->dirroot . '/mod/quiz/report/statistics/report.php');
require_once($CFG->dirroot . '/mod/quiz/report/statistics/statisticslib.php');
require_once($CFG->dirroot . '/mod/quiz/report/reportlib.php');
require_once($CFG->dirroot . '/mod/quiz/tests/quiz_question_helper_test_trait.php');
/**
* Tests for statistics report
*
* @package quiz_statistics
* @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net}
* @author Mark Johnson <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \quiz_statistics_report
*/
class test_quiz_statistics_report extends \advanced_testcase {
use \quiz_question_helper_test_trait;
/**
* Secondary database connection for creating locks.
*
* @var \moodle_database|null
*/
protected static ?\moodle_database $lockdb;
/**
* Lock factory using the secondary database connection.
*
* @var \moodle_database|null
*/
protected static ?\core\lock\lock_factory $lockfactory;
/**
* Create a lock factory with a second database session.
*
* This allows us to create a lock in our test code that will block a lock request
* on the same key in code under test.
*
* @return void
*/
public static function setUpBeforeClass(): void {
global $CFG;
self::$lockdb = \moodle_database::get_driver_instance($CFG->dbtype, $CFG->dblibrary);
self::$lockdb->connect($CFG->dbhost, $CFG->dbuser, $CFG->dbpass, $CFG->dbname, $CFG->prefix, $CFG->dboptions);
$lockfactory = \core\lock\lock_config::get_lock_factory('quiz_statistics_get_stats');
$reflectiondb = new \ReflectionProperty($lockfactory, 'db');
$reflectiondb->setAccessible(true);
$reflectiondb->setValue($lockfactory, self::$lockdb);
self::$lockfactory = $lockfactory;
}
/**
* Dispose of the extra DB connection and lock factory.
*
* @return void
*/
public static function tearDownAfterClass(): void {
self::$lockdb->dispose();
self::$lockdb = null;
self::$lockfactory = null;
}
/**
* Return a generated quiz
*
* @return \stdClass
*/
protected function create_and_attempt_quiz(): \stdClass {
$course = $this->getDataGenerator()->create_course();
$user = $this->getDataGenerator()->create_user();
$quiz = $this->create_test_quiz($course);
$quizcontext = \context_module::instance($quiz->cmid);
$questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
$this->add_two_regular_questions($questiongenerator, $quiz, ['contextid' => $quizcontext->id]);
$this->attempt_quiz($quiz, $user);
return $quiz;
}
/**
* Test locking the calculation process.
*
* When there is a lock on the hash code, test_get_all_stats_and_analysis() should wait until the lock timeout, then throw an
* exception.
*
* When there is no lock (or the lock has been released), it should return a result.
*
* @return void
*/
public function test_get_all_stats_and_analysis_locking(): void {
$this->resetAfterTest(true);
$quiz = $this->create_and_attempt_quiz();
$whichattempts = QUIZ_GRADEAVERAGE; // All attempts.
$whichtries = \question_attempt::ALL_TRIES;
$groupstudentsjoins = new \core\dml\sql_join();
$qubaids = quiz_statistics_qubaids_condition($quiz->id, $groupstudentsjoins, $whichattempts);
$report = new \quiz_statistics_report();
$questions = $report->load_and_initialise_questions_for_calculations($quiz);
$timeoutseconds = 20;
set_config('getstatslocktimeout', $timeoutseconds, 'quiz_statistics');
$lock = self::$lockfactory->get_lock($qubaids->get_hash_code(), 0);
$progress = new \core\progress\none();
$this->resetDebugging();
$timebefore = microtime(true);
try {
$result = $report->get_all_stats_and_analysis(
$quiz,
$whichattempts,
$whichtries,
$groupstudentsjoins,
$questions,
$progress
);
$timeafter = microtime(true);
// Verify that we waited as long as the timeout.
$this->assertEqualsWithDelta($timeoutseconds, $timeafter - $timebefore, 1);
$this->assertDebuggingCalled('Could not get lock on ' .
$qubaids->get_hash_code() . ' (Quiz ID ' . $quiz->id . ') after ' .
$timeoutseconds . ' seconds');
$this->assertEquals([null, null], $result);
} finally {
$lock->release();
}
$this->resetDebugging();
$result = $report->get_all_stats_and_analysis(
$quiz,
$whichattempts,
$whichtries,
$groupstudentsjoins,
$questions
);
$this->assertDebuggingNotCalled();
$this->assertNotEquals([null, null], $result);
}
/**
* Test locking when the current page does not require calculations.
*
* When there is a lock on the hash code, test_get_all_stats_and_analysis() should return a null result immediately,
* with no exception thrown.
*
* @return void
*/
public function test_get_all_stats_and_analysis_locking_no_calculation(): void {
$this->resetAfterTest(true);
$quiz = $this->create_and_attempt_quiz();
$whichattempts = QUIZ_GRADEAVERAGE; // All attempts.
$whichtries = \question_attempt::ALL_TRIES;
$groupstudentsjoins = new \core\dml\sql_join();
$qubaids = quiz_statistics_qubaids_condition($quiz->id, $groupstudentsjoins, $whichattempts);
$report = new \quiz_statistics_report();
$questions = $report->load_and_initialise_questions_for_calculations($quiz);
$timeoutseconds = 20;
set_config('getstatslocktimeout', $timeoutseconds, 'quiz_statistics');
$lock = self::$lockfactory->get_lock($qubaids->get_hash_code(), 0);
$this->resetDebugging();
try {
$progress = new \core\progress\none();
$timebefore = microtime(true);
$result = $report->get_all_stats_and_analysis(
$quiz,
$whichattempts,
$whichtries,
$groupstudentsjoins,
$questions,
$progress,
false
);
$timeafter = microtime(true);
// Verify that we did not wait for the timeout before returning.
$this->assertLessThan($timeoutseconds, $timeafter - $timebefore);
$this->assertEquals([null, null], $result);
$this->assertDebuggingNotCalled();
} finally {
$lock->release();
}
}
}
+1 -1
View File
@@ -24,6 +24,6 @@
defined('MOODLE_INTERNAL') || die();
$plugin->version = 2023042403;
$plugin->version = 2023042404;
$plugin->requires = 2023041800;
$plugin->component = 'quiz_statistics';
@@ -257,10 +257,9 @@ class calculator {
}
}
$this->stats->cache($qubaids);
// All finished.
$this->progress->end_progress();
}
// All finished.
$this->progress->end_progress();
return $this->stats;
}