moodle/moodle
3
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-08-05 00:46:50 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

MDL-27948 The question engine should use recordsets to load attempt data

Browse source 
This should be good for performance (memory usage). It also avoids having to construct a meaningless, unique, first column, which is a pain on MyQSL.
This commit is contained in:
Tim Hunt 2011-06-22 18:53:15 +01:00
parent 3552484b91
commit 35d5f1c28d
8 changed files with 155 additions and 72 deletions
Download patch file Download diff file Expand all files Collapse all files

65
lib/simpletestlib.php
View file

@ -554,6 +554,71 @@ class ContainsEmptyTag extends XMLStructureExpectation {
}
/**
* Simple class that implements the {@link moodle_recordset} API based on an
* array of test data.
*
* See the {@link question_attempt_step_db_test} class in
* question/engine/simpletest/testquestionattemptstep.php for an example of how
* this is used.
*
* @copyright 2011 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class test_recordset extends moodle_recordset {
protected $records;
/**
* Constructor
* @param $table as for {@link testing_db_record_builder::build_db_records()}
* but does not need a unique first column.
*/
public function __construct(array $table) {
$columns = array_shift($table);
$this->records = array();
foreach ($table as $row) {
if (count($row) != count($columns)) {
throw new coding_exception("Row contains the wrong number of fields.");
}
$rec = array();
foreach ($columns as $i => $name) {
$rec[$name] = $row[$i];
}
$this->records[] = $rec;
}
reset($this->records);
}
public function __destruct() {
$this->close();
}
public function current() {
return (object) current($this->records);
}
public function key() {
if (is_null(key($this->records))) {
return false;
}
$current = current($this->records);
return reset($current);
}
public function next() {
next($this->records);
}
public function valid() {
return !is_null(key($this->records));
}
public function close() {
$this->records = null;
}
}
/**
* This class lets you write unit tests that access a separate set of test
* tables with a different prefix. Only those tables you explicitly ask to

10
question/behaviour/missing/simpletest/testmissingbehaviour.php
View file

@ -60,17 +60,17 @@ class qbehaviour_missing_test extends UnitTestCase {
}
public function test_render_missing() {
$records = testing_db_record_builder::build_db_records(array(
array('id', 'questionattemptid', 'contextid', 'questionusageid', 'slot',
$records = new test_recordset(array(
array('questionattemptid', 'contextid', 'questionusageid', 'slot',
'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'flagged',
'questionsummary', 'rightanswer', 'responsesummary',
'timemodified', 'attemptstepid', 'sequencenumber', 'state', 'fraction',
'timecreated', 'userid', 'name', 'value'),
array(1, 1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
array(1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
1256233790, 1, 0, 'todo', null, 1256233700, 1, '_order', '1,2,3'),
array(2, 1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
array(1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
1256233790, 2, 1, 'complete', 0.50, 1256233705, 1, '-submit', '1'),
array(3, 1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
array(1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
1256233790, 2, 1, 'complete', 0.50, 1256233705, 1, 'choice0', '1'),
));

44
question/engine/datalib.php
View file

@ -137,9 +137,8 @@ class question_engine_data_mapper {
* @param question_attempt_step the step that was loaded.
*/
public function load_question_attempt_step($stepid) {
$records = $this->db->get_records_sql("
$records = $this->db->get_recordset_sql("
SELECT
COALESCE(qasd.id, -1 * qas.id) AS id,
qas.id AS attemptstepid,
qas.questionattemptid,
qas.sequencenumber,
@ -157,11 +156,14 @@ WHERE
qas.id = :stepid
", array('stepid' => $stepid));
if (!$records) {
if (!$records->valid()) {
throw new coding_exception('Failed to load question_attempt_step ' . $stepid);
}
return question_attempt_step::load_from_records($records, $stepid);
$step = question_attempt_step::load_from_records($records, $stepid);
$records->close();
return $step;
}
/**
@ -171,9 +173,8 @@ WHERE
* @param question_attempt the question attempt that was loaded.
*/
public function load_question_attempt($questionattemptid) {
$records = $this->db->get_records_sql("
$records = $this->db->get_recordset_sql("
SELECT
COALESCE(qasd.id, -1 * qas.id) AS id,
quba.contextid,
quba.preferredbehaviour,
qa.id AS questionattemptid,
@ -210,13 +211,16 @@ ORDER BY
qas.sequencenumber
", array('questionattemptid' => $questionattemptid));
if (!$records) {
if (!$records->valid()) {
throw new coding_exception('Failed to load question_attempt ' . $questionattemptid);
}
$record = current($records);
return question_attempt::load_from_records($records, $questionattemptid,
$qa = question_attempt::load_from_records($records, $questionattemptid,
new question_usage_null_observer(), $record->preferredbehaviour);
$records->close();
return $qa;
}
/**
@ -226,9 +230,8 @@ ORDER BY
* @param question_usage_by_activity the usage that was loaded.
*/
public function load_questions_usage_by_activity($qubaid) {
$records = $this->db->get_records_sql("
$records = $this->db->get_recordset_sql("
SELECT
COALESCE(qasd.id, -1 * qas.id) AS id,
quba.id AS qubaid,
quba.contextid,
quba.component,
@ -268,11 +271,14 @@ ORDER BY
qas.sequencenumber
", array('qubaid' => $qubaid));
if (!$records) {
if (!$records->valid()) {
throw new coding_exception('Failed to load questions_usage_by_activity ' . $qubaid);
}
return question_usage_by_activity::load_from_records($records, $qubaid);
$quba = question_usage_by_activity::load_from_records($records, $qubaid);
$records->close();
return $quba;
}
/**
@ -543,9 +549,8 @@ ORDER BY qa.slot
$params = $qubaids->from_where_params();
$params['questionid'] = $questionid;
$records = $DB->get_records_sql("
$records = $DB->get_recordset_sql("
SELECT
COALESCE(qasd.id, -1 * qas.id) AS id,
quba.contextid,
quba.preferredbehaviour,
qa.id AS questionattemptid,
@ -585,19 +590,16 @@ ORDER BY
qas.sequencenumber
", $params);
if (!$records) {
return array();
}
$questionattempts = array();
$record = current($records);
while ($record) {
while ($records->valid()) {
$record = $records->current();
$questionattempts[$record->questionattemptid] =
question_attempt::load_from_records($records,
$record->questionattemptid, new question_usage_null_observer(),
$record->preferredbehaviour);
$record = current($records);
}
$records->close();
return $questionattempts;
}

17
question/engine/questionattempt.php
View file

@ -1117,18 +1117,19 @@ class question_attempt {
*
* For internal use only.
*
* @param array $records Raw records loaded from the database.
* @param Iterator $records Raw records loaded from the database.
* @param int $questionattemptid The id of the question_attempt to extract.
* @return question_attempt The newly constructed question_attempt_step.
*/
public static function load_from_records(&$records, $questionattemptid,
public static function load_from_records($records, $questionattemptid,
question_usage_observer $observer, $preferredbehaviour) {
$record = current($records);
$record = $records->current();
while ($record->questionattemptid != $questionattemptid) {
$record = next($records);
if (!$record) {
$record = $records->next();
if (!$records->valid()) {
throw new coding_exception("Question attempt $questionattemptid not found in the database.");
}
$record = $records->current();
}
try {
@ -1162,7 +1163,11 @@ class question_attempt {
$question->apply_attempt_state($qa->steps[0]);
}
$i++;
$record = current($records);
if ($records->valid()) {
$record = $records->current();
} else {
$record = false;
}
}
$qa->observer = $observer;

18
question/engine/questionattemptstep.php
View file

@ -353,18 +353,19 @@ class question_attempt_step {
/**
* Create a question_attempt_step from records loaded from the database.
* @param array $records Raw records loaded from the database.
* @param Iterator $records Raw records loaded from the database.
* @param int $stepid The id of the records to extract.
* @return question_attempt_step The newly constructed question_attempt_step.
*/
public static function load_from_records(&$records, $attemptstepid) {
$currentrec = current($records);
public static function load_from_records($records, $attemptstepid) {
$currentrec = $records->current();
while ($currentrec->attemptstepid != $attemptstepid) {
$currentrec = next($records);
if (!$currentrec) {
$records->next();
if (!$records->valid()) {
throw new coding_exception('Question attempt step ' . $attemptstepid .
' not found in the database.');
}
$currentrec = $records->current();
}
$record = $currentrec;
@ -373,7 +374,12 @@ class question_attempt_step {
if ($currentrec->name) {
$data[$currentrec->name] = $currentrec->value;
}
$currentrec = next($records);
$records->next();
if ($records->valid()) {
$currentrec = $records->current();
} else {
$currentrec = false;
}
}
$step = new question_attempt_step_read_only($data, $record->timecreated, $record->userid);

17
question/engine/questionusage.php
View file

@ -673,17 +673,18 @@ class question_usage_by_activity {
*
* For internal use only.
*
* @param array $records Raw records loaded from the database.
* @param Iterator $records Raw records loaded from the database.
* @param int $questionattemptid The id of the question_attempt to extract.
* @return question_attempt The newly constructed question_attempt_step.
*/
public static function load_from_records(&$records, $qubaid) {
$record = current($records);
public static function load_from_records($records, $qubaid) {
$record = $records->current();
while ($record->qubaid != $qubaid) {
$record = next($records);
if (!$record) {
$records->next();
if (!$records->valid()) {
throw new coding_exception("Question usage $qubaid not found in the database.");
}
$record = $records->current();
}
$quba = new question_usage_by_activity($record->component,
@ -698,7 +699,11 @@ class question_usage_by_activity {
question_attempt::load_from_records($records,
$record->questionattemptid, $quba->observer,
$quba->get_preferred_behaviour());
$record = current($records);
if ($records->valid()) {
$record = $records->current();
} else {
$record = false;
}
}
return $quba;

26
question/engine/simpletest/testquestionattempt.php
View file

@ -261,20 +261,20 @@ class question_attempt_with_steps_test extends UnitTestCase {
*/
class question_attempt_db_test extends data_loading_method_test_base {
public function test_load() {
$records = testing_db_record_builder::build_db_records(array(
array('id', 'questionattemptid', 'contextid', 'questionusageid', 'slot',
$records = new test_recordset(array(
array('questionattemptid', 'contextid', 'questionusageid', 'slot',
'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'flagged',
'questionsummary', 'rightanswer', 'responsesummary', 'timemodified',
'attemptstepid', 'sequencenumber', 'state', 'fraction',
'timecreated', 'userid', 'name', 'value'),
array(1, 1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 1, 0, 'todo', null, 1256233700, 1, null, null),
array(2, 1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 2, 1, 'complete', null, 1256233705, 1, 'answer', '1'),
array(3, 1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 1, '', '', '', 1256233790, 3, 2, 'complete', null, 1256233710, 1, 'answer', '0'),
array(4, 1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 4, 3, 'complete', null, 1256233715, 1, 'answer', '1'),
array(5, 1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 5, 4, 'gradedright', 1.0000000, 1256233720, 1, '-finish', '1'),
array(6, 1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 6, 5, 'mangrpartial', 0.5000000, 1256233790, 1, '-comment', 'Not good enough!'),
array(7, 1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 6, 5, 'mangrpartial', 0.5000000, 1256233790, 1, '-mark', '1'),
array(8, 1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 6, 5, 'mangrpartial', 0.5000000, 1256233790, 1, '-maxmark', '2'),
array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 1, 0, 'todo', null, 1256233700, 1, null, null),
array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 2, 1, 'complete', null, 1256233705, 1, 'answer', '1'),
array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 1, '', '', '', 1256233790, 3, 2, 'complete', null, 1256233710, 1, 'answer', '0'),
array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 4, 3, 'complete', null, 1256233715, 1, 'answer', '1'),
array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 5, 4, 'gradedright', 1.0000000, 1256233720, 1, '-finish', '1'),
array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 6, 5, 'mangrpartial', 0.5000000, 1256233790, 1, '-comment', 'Not good enough!'),
array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 6, 5, 'mangrpartial', 0.5000000, 1256233790, 1, '-mark', '1'),
array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 6, 5, 'mangrpartial', 0.5000000, 1256233790, 1, '-maxmark', '2'),
));
$question = test_question_maker::make_question('truefalse', 'true');
@ -334,13 +334,13 @@ class question_attempt_db_test extends data_loading_method_test_base {
}
public function test_load_missing_question() {
$records = testing_db_record_builder::build_db_records(array(
array('id', 'questionattemptid', 'contextid', 'questionusageid', 'slot',
$records = new test_recordset(array(
array('questionattemptid', 'contextid', 'questionusageid', 'slot',
'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'flagged',
'questionsummary', 'rightanswer', 'responsesummary', 'timemodified',
'attemptstepid', 'sequencenumber', 'state', 'fraction',
'timecreated', 'userid', 'name', 'value'),
array(1, 1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 1, 0, 'todo', null, 1256233700, 1, null, null),
array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 1, 0, 'todo', null, 1256233700, 1, null, null),
));
question_bank::start_unit_test();

30
question/engine/simpletest/testquestionattemptstep.php
View file

@ -138,14 +138,14 @@ class question_attempt_step_test extends UnitTestCase {
*/
class question_attempt_step_db_test extends data_loading_method_test_base {
public function test_load_with_data() {
$records = $this->build_db_records(array(
array('id', 'attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value'),
array( 1, 1, 1, 0, 'todo', null, 1256228502, 13, null, null),
array( 2, 2, 1, 1, 'complete', null, 1256228505, 13, 'x', 'a'),
array( 3, 2, 1, 1, 'complete', null, 1256228505, 13, '_y', '_b'),
array( 4, 2, 1, 1, 'complete', null, 1256228505, 13, '-z', '!c'),
array( 5, 2, 1, 1, 'complete', null, 1256228505, 13, '-_t', '!_d'),
array( 6, 3, 1, 2, 'gradedright', 1.0, 1256228515, 13, '-finish', '1'),
$records = new test_recordset(array(
array('attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value'),
array( 1, 1, 0, 'todo', null, 1256228502, 13, null, null),
array( 2, 1, 1, 'complete', null, 1256228505, 13, 'x', 'a'),
array( 2, 1, 1, 'complete', null, 1256228505, 13, '_y', '_b'),
array( 2, 1, 1, 'complete', null, 1256228505, 13, '-z', '!c'),
array( 2, 1, 1, 'complete', null, 1256228505, 13, '-_t', '!_d'),
array( 3, 1, 2, 'gradedright', 1.0, 1256228515, 13, '-finish', '1'),
));
$step = question_attempt_step::load_from_records($records, 2);
@ -157,9 +157,9 @@ class question_attempt_step_db_test extends data_loading_method_test_base {
}
public function test_load_without_data() {
$records = $this->build_db_records(array(
array('id', 'attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value'),
array( 2, 2, 1, 1, 'complete', null, 1256228505, 13, null, null),
$records = new test_recordset(array(
array('attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value'),
array( 2, 1, 1, 'complete', null, 1256228505, 13, null, null),
));
$step = question_attempt_step::load_from_records($records, 2);
@ -171,10 +171,10 @@ class question_attempt_step_db_test extends data_loading_method_test_base {
}
public function test_load_dont_be_too_greedy() {
$records = $this->build_db_records(array(
array('id', 'attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value'),
array( 1, 1, 1, 0, 'todo', null, 1256228502, 13, 'x', 'right'),
array( 2, 2, 2, 0, 'complete', null, 1256228505, 13, 'x', 'wrong'),
$records = new test_recordset(array(
array('attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value'),
array( 1, 1, 0, 'todo', null, 1256228502, 13, 'x', 'right'),
array( 2, 2, 0, 'complete', null, 1256228505, 13, 'x', 'wrong'),
));
$step = question_attempt_step::load_from_records($records, 1);