Skip to content

Commit

Permalink
Fix the experimental code concerned with saving files from combinator…
Browse files Browse the repository at this point in the history 
… grader runs. Now works with backup/restore cycles.
  • Loading branch information
@trampgeek
trampgeek committed Nov 14, 2024
1 parent 4e67122 commit b6cee8a
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 93 deletions.
1 change: 0 additions & 1 deletion ParseSerialisedData
Submodule ParseSerialisedData deleted from 3e1747
28 changes: 21 additions & 7 deletions backup/moodle2/restore_qtype_coderunner_plugin.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,27 @@ public function process_coderunner_options($data) {
}

/**
* Return the contents of this qtype to be processed by the links decoder.
* Move the feedback files from the old course context to the new course context during restore.
* Called after all CodeRunner questions have been restored, not after each question as the
* name rather implies.
* The function is called via the launch_after_restore_methods() function in the core
* restore_plugin class, using a dynamically-generated method name. This is a tad suspect as no
* other code in the base Moodle system uses this capability.
*/
public static function define_decode_contents() {
$contents = [];
$contents[] = new restore_decode_content('question_coderunner_tests', ['expected']);
$contents[] = new restore_decode_content('question_attempt_step_data', ['value']);

return $contents;
protected function after_restore_question() {
// Get old and new course ids and contexts.
$oldcourseid = $this->task->get_old_courseid();
$newcourseid = $this->task->get_courseid();
$oldcoursecontextid = context_course::instance($oldcourseid)->id;
$newcoursecontextid = context_course::instance($newcourseid)->id;

// Move the files.
$fs = get_file_storage();
$fs->move_area_files_to_new_context(
$oldcoursecontextid,
$newcoursecontextid,
'qtype_coderunner',
'feedbackfiles'
);
}
}
101 changes: 58 additions & 43 deletions classes/combinator_grader_outcome.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ class qtype_coderunner_combinator_grader_outcome extends qtype_coderunner_testin
/** @var bool Whether or no show differences is selected */
public $showdifferences;

/** @var ?int $outcomeid Random unique id for this testing outcome. */
private $outcomeid;

// A list of the allowed attributes in the combinator template grader return value.
public $allowedfields = ['fraction', 'prologuehtml', 'testresults', 'files', 'epiloguehtml',
'columnformats', 'showdifferences', 'showoutputonly', 'graderstate', 'instructorhtml',
Expand All @@ -69,6 +72,23 @@ public function __construct($isprecheck) {
$this->columnformats = null;
$this->outputonly = false;
$this->instructorhtml = null;

// Generate a (hopefully unique) id for this testoutcome object.
// Used as an itemid when saving feedback images/files that
// this class can generate. Strict uniqueness isn't actually
// required however as the filenames are prefixed by the Unix
// timestamp. So all we need is that the outcomeid is unique
// within a time window of 1 second.
$this->outcomeid = random_int(1, PHP_INT_MAX);
}

/**
* Get the hopefully-unique id of this testing outcome, for use
* as an itemid when saving/restoring feedback files.
* @return int The unique ID (a random int in the range 1 to PHP_INT_MAX).
*/
public function get_id() {
return $this->outcomeid;
}


Expand All @@ -81,52 +101,47 @@ public function __construct($isprecheck) {
* @param array An associate array mapping filenames to URLs that reference that file.
*/
private function save_files($files) {
global $USER;
global $COURSE;

$fileurls = [];
foreach ($files as $filename => $base64) {
$decoded = base64_decode($base64);

// Prepare file record object.
$contextid = context_user::instance($USER->id)->id;
$fileinfo = [
'contextid' => $contextid,
'component' => 'qtype_coderunner',
'filearea' => 'feedbackfiles', // Custom file area for your plugin.
'itemid' => time(), // Item ID - use Unix time stamp.
'filepath' => '/', // File path within the context.
'filename' => $filename]; // Desired name of the file.

// Get the file storage object.
$fs = get_file_storage();

// Check if the file already exists to avoid duplicates.
if (
!$fs->file_exists(
$contextid,
$fileinfo['component'],
$fileinfo['filearea'],
$fileinfo['itemid'],
$fileinfo['filepath'],
$fileinfo['filename']
)
) {
// Create the file in Moodle's filesystem.
$file = $fs->create_file_from_string($fileinfo, $decoded);
}
if ($files) {
$itemid = $this->get_id();
$contextid = context_course::instance($COURSE->id)->id;
foreach ($files as $filename => $base64) {
$extendedfilename = strval(time()) . $filename;
$decoded = base64_decode($base64);

// Prepare file record object.
$fileinfo = [
'contextid' => $contextid,
'component' => 'qtype_coderunner',
'filearea' => 'feedbackfiles',
'itemid' => $itemid, // Item ID - question attempt step id.
'filepath' => '/', // File path within the context.
'filename' => $extendedfilename,
];
// Get the file storage object.
$fs = get_file_storage();

// Check if the file already exists to avoid duplicates.
if (
!$fs->file_exists(
$contextid,
$fileinfo['component'],
$fileinfo['filearea'],
$fileinfo['itemid'],
$fileinfo['filepath'],
$fileinfo['filename']
)
) {
// Create the file in Moodle's filesystem.
$file = $fs->create_file_from_string($fileinfo, $decoded);
}

// Generate a URL to the saved file.
$url = moodle_url::make_pluginfile_url(
$contextid,
$fileinfo['component'],
$fileinfo['filearea'],
$fileinfo['itemid'],
$fileinfo['filepath'],
$fileinfo['filename'],
false
);

$fileurls[$filename] = $url;
// Generate a URL to the saved file.
$url = '@@PLUGINFILE@@/' . $extendedfilename;
$fileurls[$filename] = $url;
}
}
return $fileurls;
}
Expand Down
14 changes: 6 additions & 8 deletions lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@

/**
* Checks file access for CodeRunner questions.
* Feedbackfiles are stored in the course context, because they are generated
* by the grader, which does not have access to the questionattemptstepid.
* Hence, the special case for these files needs to use the course context.
*
* @param stdClass $course course object
* @param stdClass $cm course module object
Expand All @@ -34,25 +37,20 @@
* @param array $options additional options affecting the file serving
*/
function qtype_coderunner_pluginfile($course, $cm, $context, $filearea, $args, $forcedownload, array $options = []) {
global $CFG;
global $CFG, $COURSE;
if ($filearea === 'feedbackfiles') {
require_login($course, false, $cm);

// Note: Implement additional checks here, as needed, to ensure users are authorized to access the file

// Serve the file
$fs = get_file_storage();
$filename = array_pop($args);
$itemid = intval(array_shift($args));
$filepath = '/';
$contextid = $context->id;
$contextid = context_course::instance($COURSE->id)->id;
$file = $fs->get_file($contextid, 'qtype_coderunner', $filearea, $itemid, $filepath, $filename);
if (!$file) {
send_file_not_found();
}
send_stored_file($file, 0, 0, $forcedownload, $options); // Adjust options as necessary
send_stored_file($file, 0, 0, $forcedownload, $options); // Adjust options can be added here if reqd.
}
require_once($CFG->libdir . '/questionlib.php');
question_pluginfile($course, $context, 'qtype_coderunner', $filearea, $args, $forcedownload, $options);
}

10 changes: 7 additions & 3 deletions questiontype.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,16 +378,20 @@ public function clean_question_form($question, $isvalidation = false) {

/**
* Move all the files belonging to this question from one context to another.
* Override superclass implementation to handle the extra data files,
* sample answer files and feedbackfiles we have in CodeRunner questions.
* Called during course restore.
* Override superclass implementation in order to handle the extra data files and
* sample answer files we have in CodeRunner questions.
*
* The feedbackfiles are stored in the course context so are not handled here,
* but in the restore_qtype_coderunner_plugin.after_restore_question() method.
* @param int $questionid the question being moved.
* @param int $oldcontextid the context it is moving from.
* @param int $newcontextid the context it is moving to.
*/
public function move_files($questionid, $oldcontextid, $newcontextid) {
parent::move_files($questionid, $oldcontextid, $newcontextid);
$fs = get_file_storage();
foreach (['datafile', 'samplefile', 'feedbackfiles'] as $filetype) {
foreach (['datafile', 'samplefile'] as $filetype) {
$fs->move_area_files_to_new_context(
$oldcontextid,
$newcontextid,
Expand Down
54 changes: 23 additions & 31 deletions renderer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,23 +228,6 @@ public function feedback(question_attempt $qa, question_display_options $options
return parent::feedback($qa, $optionsclone);
}

/**
* If the serialisation is broken, it's probably because of Moodle's link updating.
* Try to fix that.
*/
protected function fix_borked_serialisation($serializeddata) {
// Try to parse the serialised data, searching for the terminators whenever
// a block miss occurs.
$pattern = '/s:(\d+):"(.*PLUGINFILEBYCONTEXT[^>]+>)";/s';
print($serializeddata);
return preg_replace_callback($pattern, function ($matches) {
//echo "****** MATCHED ****\n ";
$s = $matches[2];
// Recalculate length and reconstruct.
$actuallength = strlen($s);
return 's:' . $actuallength . ':"' . $s . '";';
}, $serializeddata);
}

/**
* Generate the specific feedback. This is feedback that varies according to
Expand All @@ -253,6 +236,7 @@ protected function fix_borked_serialisation($serializeddata) {
* @return string HTML fragment.
*/
protected function specific_feedback(question_attempt $qa) {
global $COURSE;
$toserialised = $qa->get_last_qt_var('_testoutcome');
if (!$toserialised) { // Something broke?
return '';
Expand All @@ -261,17 +245,10 @@ protected function specific_feedback(question_attempt $qa) {
$q = $qa->get_question();
$outcome = @unserialize($toserialised);
if ($outcome === false) {
//print(json_encode($toserialised) . "\n\n");
//echo "\n===========================\n";
$fixed = $this->fix_borked_serialisation($toserialised);
//echo $fixed . "\n";
//echo "===========================";
$outcome = @unserialize($fixed);
if ($outcome === false) {
$outcome = new qtype_coderunner_testing_outcome(0, 0, false);
$outcome->set_status(qtype_coderunner_testing_outcome::STATUS_UNSERIALIZE_FAILED);
}
$outcome = new qtype_coderunner_testing_outcome(0, 0, false);
$outcome->set_status(qtype_coderunner_testing_outcome::STATUS_UNSERIALIZE_FAILED);
}

$resultsclass = $this->results_class($outcome, $q->allornothing);

$isoutputonly = $outcome->is_output_only();
Expand Down Expand Up @@ -345,8 +322,19 @@ protected function specific_feedback(question_attempt $qa) {
}
$fb .= html_writer::end_tag('div');

// Need to pass the feedback through format_text to activate filters.
// If this is a combinator grader outcome, need to rewrite any embedded file URLs,
// which will be in @@PLUGINFILE@@ form. We use the current course context and the
// randomly generated test outcome id as an itemid. Note that the filenames are prefixed by
// the Unix timestamp, so the probability of a collision is infinitesimal.
if ($outcome->iscombinatorgrader()) { // Only combinator grader outcomes have embedded files.
$itemid = $outcome->get_id();
$contextid = context_course::instance($COURSE->id)->id;
$fb = file_rewrite_pluginfile_urls($fb, 'pluginfile.php', $contextid, 'qtype_coderunner', 'feedbackfiles', $itemid);
}

// Pass the feedback through format_text to activate filters.
// Try to ensure it makes minimal changes.

$formatoptions = new stdClass();
$formatoptions->trusted = true;
$formatoptions->noclean = true;
Expand Down Expand Up @@ -432,13 +420,17 @@ protected function build_results_table($outcome, qtype_coderunner_question $ques
}
$fb .= $outcome->get_epilogue();

// Issue a bright yellow warning if using jobe2, except when running behat.
// If jobe2 is being used with the default API key, display a bright yellow
// "Please don't use for production" message.
// If jobe2 is being used with a custom API key, display a much quieter message.
// Neither message is displayed if testing with behat.
$sandboxinfo = $outcome->get_sandbox_info();
if (isset($sandboxinfo['jobeserver'])) {
$jobeserver = $sandboxinfo['jobeserver'];
$apikey = $sandboxinfo['jobeapikey'];
if (qtype_coderunner_sandbox::is_canterbury_server($jobeserver)
&& (!qtype_coderunner_sandbox::is_using_test_sandbox())) {
$iscanterbury = qtype_coderunner_sandbox::is_canterbury_server($jobeserver);
$istesting = qtype_coderunner_sandbox::is_using_test_sandbox();
if ($iscanterbury && !$istesting) {
if ($apikey == constants::JOBE_HOST_DEFAULT_API_KEY) {
$fb .= get_string('jobe_warning_html', 'qtype_coderunner');
} else {
Expand Down

0 comments on commit b6cee8a

Please sign in to comment.