diff --git a/db/install.xml b/db/install.xml index 817bccc13..f50087fd2 100644 --- a/db/install.xml +++ b/db/install.xml @@ -1,5 +1,5 @@ - @@ -31,6 +31,8 @@ + + diff --git a/db/upgrade.php b/db/upgrade.php index 0464f3a41..2f7b9c47d 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -342,7 +342,7 @@ function xmldb_qtype_coderunner_upgrade($oldversion) { // Define field templateparamslang to be added to question_coderunner_options. $table = new xmldb_table('question_coderunner_options'); - $field = new xmldb_field('templateparamslang', XMLDB_TYPE_CHAR, '50', null, null, null, 'twig', 'templateparams'); + $field = new xmldb_field('templateparamslang', XMLDB_TYPE_CHAR, '50', null, null, null, 'None', 'templateparams'); // Conditionally launch add field templateparamslang. if (!$dbman->field_exists($table, $field)) { @@ -353,6 +353,26 @@ function xmldb_qtype_coderunner_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2020120701, 'qtype', 'coderunner'); } + if ($oldversion < 2020121501) { + $table = new xmldb_table('question_coderunner_options'); + $field = new xmldb_field('templateparamsevalpertry', XMLDB_TYPE_INTEGER, '1', null, null, null, '0', 'templateparamslang'); + + // Conditionally launch add field templateparamsevalpertry. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + $field = new xmldb_field('templateparamsevald', XMLDB_TYPE_TEXT, null, null, null, null, null, 'templateparamsevalpertry'); + + // Conditionally launch add field templateparamsevald. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + + // Coderunner savepoint reached. + upgrade_plugin_savepoint(true, 2020121501, 'qtype', 'coderunner'); + } require_once(__DIR__ . '/upgradelib.php'); update_question_types(); diff --git a/edit_coderunner_form.php b/edit_coderunner_form.php index 9acc36011..c26501085 100644 --- a/edit_coderunner_form.php +++ b/edit_coderunner_form.php @@ -61,18 +61,7 @@ protected function definition() { global $PAGE; $mform = $this->_form; - $this->mergedtemplateparams = ''; - $this->twiggedparams = ''; - if (!empty($this->question->options->mergedtemplateparams)) { - $this->mergedtemplateparams = $this->question->options->mergedtemplateparams; - try { - $this->twiggedparams = $this->twig_render($this->mergedtemplateparams); - } catch (Exception $ex) { - // If the params are broken, don't use them. - // Code checker won't accept an empty catch. - $this->twiggedparams = ''; - } - } + if (!empty($this->question->options->language)) { $this->lang = $this->acelang = $this->question->options->language; } else { @@ -186,6 +175,15 @@ public function definition_inner($mform) { $mform->setDefault('maxfilesize', '10240'); $mform->disabledIf('maxfilesize', 'attachments', 'eq', 0); } + + + function get_data() { + $fields = parent::get_data(); + if ($fields) { + $fields->templateparamsevald = $this->formquestion->templateparamsevald; + } + return $fields; + } /** * Add a field for a sample answer to this problem (optional) @@ -196,10 +194,14 @@ protected function add_sample_answer_field($mform) { $mform->addElement('header', 'answerhdr', get_string('answer', 'qtype_coderunner'), ''); $mform->setExpanded('answerhdr', 1); + + // Cut out the next line and the use of jsonparams in data-params + // once UI plugin parameters have been refactored properly. + $jsonparams = $this->get_merged_params(); $attributes = array( 'rows' => 9, 'class' => 'answer edit_code', - 'data-params' => $this->twiggedparams, + 'data-params' => $jsonparams, 'data-lang' => $this->acelang); $mform->addElement('textarea', 'answer', get_string('answer', 'qtype_coderunner'), @@ -231,12 +233,12 @@ protected function add_sample_answer_field($mform) { protected function add_preload_answer_field($mform) { $mform->addElement('header', 'answerpreloadhdr', get_string('answerpreload', 'qtype_coderunner'), ''); - $expanded = !empty($this->question->options->answerpreload); + $expanded = !empty($this->formquestion->options->answerpreload); $mform->setExpanded('answerpreloadhdr', $expanded); $attributes = array( 'rows' => 5, 'class' => 'preloadanswer edit_code', - 'data-params' => $this->twiggedparams, + //'data-params' => $this->twiggedparams, 'data-lang' => $this->acelang); $mform->addElement('textarea', 'answerpreload', get_string('answerpreload', 'qtype_coderunner'), @@ -473,123 +475,7 @@ private function newline_hack($s) { } - public function validation($data, $files) { - $errors = parent::validation($data, $files); - $this->question = $this->make_question_from_form_data($data); - if ($data['coderunnertype'] == 'Undefined') { - $errors['coderunner_type_group'] = get_string('questiontype_required', 'qtype_coderunner'); - } - if ($data['cputimelimitsecs'] != '' && - (!ctype_digit($data['cputimelimitsecs']) || intval($data['cputimelimitsecs']) <= 0)) { - $errors['sandboxcontrols'] = get_string('badcputime', 'qtype_coderunner'); - } - if ($data['memlimitmb'] != '' && - (!ctype_digit($data['memlimitmb']) || intval($data['memlimitmb']) < 0)) { - $errors['sandboxcontrols'] = get_string('badmemlimit', 'qtype_coderunner'); - } - - if ($data['precheck'] == constants::PRECHECK_EXAMPLES && $this->num_examples($data) === 0) { - $errors['coderunner_precheck_group'] = get_string('precheckingemptyset', 'qtype_coderunner'); - } - - if ($data['sandboxparams'] != '' && - json_decode($data['sandboxparams']) === null) { - $errors['sandboxcontrols'] = get_string('badsandboxparams', 'qtype_coderunner'); - } - - $templateerrors = $this->validate_template_params($data); - if ($templateerrors) { - $errors['templateparams'] = $templateerrors; - } - - if ($data['prototypetype'] == 0 && ($data['grader'] !== 'TemplateGrader' - || $data['iscombinatortemplate'] === false)) { - // Unless it's a prototype or uses a combinator-template grader, - // it needs at least one testcase. - $testcaseerrors = $this->validate_test_cases($data); - $errors = array_merge($errors, $testcaseerrors); - } - - if ($data['iscombinatortemplate'] && empty($data['testsplitterre'])) { - $errors['templatecontrols'] = get_string('bad_empty_splitter', 'qtype_coderunner'); - } - - if ($data['prototypetype'] == 2 && ($data['saved_prototype_type'] != 2 || - $data['typename'] != $data['coderunnertype'])) { - // User-defined prototype, either newly created or undergoing a name change. - $typename = trim($data['typename']); - if ($typename === '') { - $errors['prototypecontrols'] = get_string('empty_new_prototype_name', 'qtype_coderunner'); - } else if (!$this->is_valid_new_type($typename)) { - $errors['prototypecontrols'] = get_string('bad_new_prototype_name', 'qtype_coderunner'); - } - } - - $penaltyregimeerror = $this->validate_penalty_regime($data); - if ($penaltyregimeerror) { - $errors['markinggroup'] = $penaltyregimeerror; - } - - $resultcolumnsjson = trim($data['resultcolumns']); - if ($resultcolumnsjson !== '') { - $resultcolumns = json_decode($resultcolumnsjson); - if ($resultcolumns === null) { - $errors['resultcolumns'] = get_string('resultcolumnsnotjson', 'qtype_coderunner'); - } else if (!is_array($resultcolumns)) { - $errors['resultcolumns'] = get_string('resultcolumnsnotlist', 'qtype_coderunner'); - } else { - foreach ($resultcolumns as $col) { - if (!is_array($col) || count($col) < 2) { - $errors['resultcolumns'] = get_string('resultcolumnspecbad', 'qtype_coderunner'); - break; - } - foreach ($col as $el) { - if (!is_string($el)) { - $errors['resultcolumns'] = get_string('resultcolumnspecbad', 'qtype_coderunner'); - break; - } - } - } - } - } - - if ($data['attachments']) { - // Check a valid regular expression was given. - // Use '=' as the PCRE delimiter. - if (@preg_match('=^' . $data['filenamesregex'] . '$=', null) === false) { - $errors['filenamesgroup'] = get_string('badfilenamesregex', 'qtype_coderunner'); - } - } - - if (count($errors) == 0) { - $errors = $this->validate_twigables(); - } - - if (count($errors) == 0 && !empty($data['validateonsave'])) { - $testresult = $this->validate_sample_answer($data); - if ($testresult) { - $errors['answer'] = $testresult; - } - } - - $acelangs = trim($data['acelang']); - if ($acelangs !== '' && strpos($acelangs, ',') !== false) { - $parsedlangs = qtype_coderunner_util::extract_languages($acelangs); - if ($parsedlangs === false) { - $errors['languages'] = get_string('multipledefaults', 'qtype_coderunner'); - } else if (count($parsedlangs[0]) === 0) { - $errors['languages'] = get_string('badacelangstring', 'qtype_coderunner'); - } - } - - // Don't allow the teacher to require more attachments than they allow; as this would - // create a condition that it's impossible for the student to meet. - if ($data['attachments'] != -1 && $data['attachments'] < $data['attachmentsrequired'] ) { - $errors['attachmentsrequired'] = get_string('mustrequirefewer', 'qtype_coderunner'); - } - - return $errors; - } + // FUNCTIONS TO BUILD PARTS OF THE MAIN FORM // =========================================. @@ -603,6 +489,7 @@ private function make_error_div($mform) { // Add to the supplied $mform the panel "Coderunner question type". private function make_questiontype_panel($mform) { list($languages, $types) = $this->get_languages_and_types(); + $hidemethod = method_exists($mform, 'hideIf') ? 'hideIf' : 'disabledIf'; $mform->addElement('header', 'questiontypeheader', get_string('type_header', 'qtype_coderunner')); // Insert the (possible) missing prototype message as a hidden field. JavaScript @@ -610,7 +497,7 @@ private function make_questiontype_panel($mform) { $mform->addElement('hidden', 'missingprototypemessage', '', array('id' => 'id_missing_prototype', 'class' => 'missingprototypeerror')); $mform->setType('missingprototypemessage', PARAM_RAW); - + // The Question Type controls (a group with just a single member). $typeselectorelements = array(); $expandedtypes = array_merge(array('Undefined' => 'Undefined'), $types); @@ -701,11 +588,26 @@ private function make_questiontype_panel($mform) { get_string('hoisttemplateparams', 'qtype_coderunner')); $twigelements[] = $mform->createElement('advcheckbox', 'twigall', null, get_string('twigall', 'qtype_coderunner')); - $templateparamlangs = array('twig'=>'Twig', 'python'=>'Python (EXPERIMENTAL)'); + $templateparamlangs = array('None'=>'None', + 'twig'=>'Twig', + 'python3'=>'Python3', + 'c' => 'C', + 'cpp' => 'C++', + 'java' => 'Java', + 'php' => 'php', + 'pascal' => 'Pascal' + ); $twigelements[] = $mform->createElement('select', 'templateparamslang', get_string('templateparamslang', 'qtype_coderunner'), $templateparamlangs); + $twigelements[] = $mform->createElement('advcheckbox', 'templateparamsevalpertry', null, + get_string('templateparamsevalpertry', 'qtype_coderunner')); $mform->addElement('group', 'twigcontrols', get_string('twigcontrols', 'qtype_coderunner'), $twigelements, null, false); + $mform->setDefault('templateparamslang', 'None'); + $mform->setDefault('templateparamsevalpertry', false); $mform->setDefault('twigall', false); + $mform->$hidemethod('templateparamsevalpertry', 'templateparamslang', 'eq', 'None'); + $mform->$hidemethod('templateparamsevalpertry', 'templateparamslang', 'eq', 'twig'); + // Although hoisttemplateparams defaults to true in the database, // it defaults to true in this form. This ensures that legacy questions are // not affected, while new questions default to true. @@ -881,115 +783,156 @@ private function make_advanced_customisation_panel($mform) { $mform->disabledIf('testsplitterre', 'iscombinatortemplate', 'eq', 0); $mform->disabledIf('allowmultiplestdins', 'iscombinatortemplate', 'eq', 0); } + + + + // ********************************************************** + // + // VALIDATION + // + // ********************************************************** + - // UTILITY FUNCTIONS. - // =================. + // Validate the given data and possible files. + public function validation($data, $files) { + $errors = parent::validation($data, $files); + $this->formquestion = $this->make_question_from_form_data($data); + if ($data['coderunnertype'] == 'Undefined') { + $errors['coderunner_type_group'] = get_string('questiontype_required', 'qtype_coderunner'); + } + if ($data['cputimelimitsecs'] != '' && + (!ctype_digit($data['cputimelimitsecs']) || intval($data['cputimelimitsecs']) <= 0)) { + $errors['sandboxcontrols'] = get_string('badcputime', 'qtype_coderunner'); + } + if ($data['memlimitmb'] != '' && + (!ctype_digit($data['memlimitmb']) || intval($data['memlimitmb']) < 0)) { + $errors['sandboxcontrols'] = get_string('badmemlimit', 'qtype_coderunner'); + } - // True iff the given name is valid for a new type, i.e., it's not in use - // in the current context (Currently only a single global context is - // implemented). - private function is_valid_new_type($typename) { - list($langs, $types) = $this->get_languages_and_types(); - return !array_key_exists($typename, $types); - } + if ($data['precheck'] == constants::PRECHECK_EXAMPLES && $this->num_examples($data) === 0) { + $errors['coderunner_precheck_group'] = get_string('precheckingemptyset', 'qtype_coderunner'); + } + if ($data['sandboxparams'] != '' && + json_decode($data['sandboxparams']) === null) { + $errors['sandboxcontrols'] = get_string('badsandboxparams', 'qtype_coderunner'); + } - /** - * Return a count of the number of test cases set as examples. - * @param array $data data from the form - */ - private function num_examples($data) { - return isset($data['useasexample']) ? count($data['useasexample']) : 0; - } + list($templateerrors, $json) = $this->validate_template_params(); + if (!$templateerrors) { + $this->formquestion->templateparamsevald = $json; + } else { + $errors['templateparams'] = $templateerrors; + $this->formquestion->templateparamsevald = '{}'; + } - private function get_languages_and_types() { - // Return two arrays (language => language_upper_case) and (type => subtype) of - // all the coderunner question types available in the current course - // context. - // The subtype is the suffix of the type in the database, - // e.g. for java_method it is 'method'. The language is the bit before - // the underscore, and language_upper_case is a capitalised version, - // e.g. Java for java. For question types without a - // subtype the word 'Default' is used. + if ($data['prototypetype'] == 0 && ($data['grader'] !== 'TemplateGrader' + || $data['iscombinatortemplate'] === false)) { + // Unless it's a prototype or uses a combinator-template grader, + // it needs at least one testcase. + $testcaseerrors = $this->validate_test_cases($data); + $errors = array_merge($errors, $testcaseerrors); + } - $records = qtype_coderunner::get_all_prototypes(); - $types = array(); - foreach ($records as $row) { - if (($pos = strpos($row->coderunnertype, '_')) !== false) { - $subtype = substr($row->coderunnertype, $pos + 1); - $language = substr($row->coderunnertype, 0, $pos); - } else { - $subtype = 'Default'; - $language = $row->coderunnertype; - } - $types[$row->coderunnertype] = $row->coderunnertype; - $languages[$language] = ucwords($language); + if ($data['iscombinatortemplate'] && empty($data['testsplitterre'])) { + $errors['templatecontrols'] = get_string('bad_empty_splitter', 'qtype_coderunner'); } - asort($types); - asort($languages); - return array($languages, $types); - } - // Validate the test cases. - private function validate_test_cases($data) { - $errors = array(); // Return value. - $testcodes = $data['testcode']; - $stdins = $data['stdin']; - $expecteds = $data['expected']; - $marks = $data['mark']; - $count = 0; - $numnonemptytests = 0; - $num = max(count($testcodes), count($stdins), count($expecteds)); - for ($i = 0; $i < $num; $i++) { - $testcode = trim($testcodes[$i]); - if ($testcode != '') { - $numnonemptytests++; + if ($data['prototypetype'] == 2 && ($data['saved_prototype_type'] != 2 || + $data['typename'] != $data['coderunnertype'])) { + // User-defined prototype, either newly created or undergoing a name change. + $typename = trim($data['typename']); + if ($typename === '') { + $errors['prototypecontrols'] = get_string('empty_new_prototype_name', 'qtype_coderunner'); + } else if (!$this->is_valid_new_type($typename)) { + $errors['prototypecontrols'] = get_string('bad_new_prototype_name', 'qtype_coderunner'); } - $stdin = trim($stdins[$i]); - $expected = trim($expecteds[$i]); - if ($testcode !== '' || $stdin != '' || $expected !== '') { - $count++; - $mark = trim($marks[$i]); - if ($mark != '') { - if (!is_numeric($mark)) { - $errors["testcode[$i]"] = get_string('nonnumericmark', 'qtype_coderunner'); - } else if (floatval($mark) <= 0) { - $errors["testcode[$i]"] = get_string('negativeorzeromark', 'qtype_coderunner'); + } + + $penaltyregimeerror = $this->validate_penalty_regime($data); + if ($penaltyregimeerror) { + $errors['markinggroup'] = $penaltyregimeerror; + } + + $resultcolumnsjson = trim($data['resultcolumns']); + if ($resultcolumnsjson !== '') { + $resultcolumns = json_decode($resultcolumnsjson); + if ($resultcolumns === null) { + $errors['resultcolumns'] = get_string('resultcolumnsnotjson', 'qtype_coderunner'); + } else if (!is_array($resultcolumns)) { + $errors['resultcolumns'] = get_string('resultcolumnsnotlist', 'qtype_coderunner'); + } else { + foreach ($resultcolumns as $col) { + if (!is_array($col) || count($col) < 2) { + $errors['resultcolumns'] = get_string('resultcolumnspecbad', 'qtype_coderunner'); + break; + } + foreach ($col as $el) { + if (!is_string($el)) { + $errors['resultcolumns'] = get_string('resultcolumnspecbad', 'qtype_coderunner'); + break; + } } } } } - if ($count == 0) { - $errors["testcode[0]"] = get_string('atleastonetest', 'qtype_coderunner'); - } else if ($numnonemptytests != 0 && $numnonemptytests != $count) { - $errors["testcode[0]"] = get_string('allornone', 'qtype_coderunner'); + if ($data['attachments']) { + // Check a valid regular expression was given. + // Use '=' as the PCRE delimiter. + if (@preg_match('=^' . $data['filenamesregex'] . '$=', null) === false) { + $errors['filenamesgroup'] = get_string('badfilenamesregex', 'qtype_coderunner'); + } } + + if (count($errors) == 0) { + $errors = $this->validate_twigables(); + } + + if (count($errors) == 0 && !empty($data['validateonsave'])) { + $testresult = $this->validate_sample_answer($data); + if ($testresult) { + $errors['answer'] = $testresult; + } + } + + $acelangs = trim($data['acelang']); + if ($acelangs !== '' && strpos($acelangs, ',') !== false) { + $parsedlangs = qtype_coderunner_util::extract_languages($acelangs); + if ($parsedlangs === false) { + $errors['languages'] = get_string('multipledefaults', 'qtype_coderunner'); + } else if (count($parsedlangs[0]) === 0) { + $errors['languages'] = get_string('badacelangstring', 'qtype_coderunner'); + } + } + + // Don't allow the teacher to require more attachments than they allow; as this would + // create a condition that it's impossible for the student to meet. + if ($data['attachments'] != -1 && $data['attachments'] < $data['attachmentsrequired'] ) { + $errors['attachmentsrequired'] = get_string('mustrequirefewer', 'qtype_coderunner'); + } + return $errors; } + - - // Check the templateparameters value, if given. Return an error message - // string, which will be empty if there are no errors. + // Check the templateparameters value, if given. Return an array containing + // the error message string, which will be empty if there are no errors, + // and the JSON evaluated template parameters. private function validate_template_params() { global $USER; - $templateparams = $this->question->templateparams; + $templateparams = $this->formquestion->templateparams; $errormessage = ''; - $this->question->decodedparams = array(); - if (!empty($templateparams)) { - $lang = $this->question->templateparamslang; - $seed = 0; - $json = $this->question->evaluate_template_params($templateparams, $lang, $seed); - $decoded = json_decode($json, true); - if ($decoded === null) { - $errormessage = get_string('badtemplateparams', 'qtype_coderunner', $json); - } + $seed = mt_rand(); + $json = $this->formquestion->evaluate_merged_parameters($seed); + $decoded = json_decode($json, true); + if ($decoded === null) { + $errormessage = get_string('badtemplateparams', 'qtype_coderunner', $json); } - return $errormessage; + return array($errormessage, $json); } - private function validate_penalty_regime($data) { // Check the penalty regime and return an error string or an empty string if OK. $errorstring = ''; @@ -1031,9 +974,10 @@ private function validate_penalty_regime($data) { // form fields to error messages. private function validate_twigables() { $errors = array(); - $question = $this->question; - $question->evaluate_merged_parameters(0); - $parameters = $question->parameters; + $question = $this->formquestion; + $jsonparams = $question->templateparamsevald; + $parameters = json_decode($jsonparams, true); + $parameters['QUESTION'] = $question; // Try twig expanding everything (see question::twig_all), with strict_variables true. foreach (['questiontext', 'answer', 'answerpreload', 'globalextra'] as $field) { @@ -1042,7 +986,7 @@ private function validate_twigables() { $text = $text['text']; } try { - $this->twig_render($text, $question->parameters, true); + $this->twig_render($text, $parameters, true); } catch (Twig_Error $ex) { $errors[$field] = get_string('twigerror', 'qtype_coderunner', $ex->getMessage()); @@ -1070,55 +1014,54 @@ private function validate_twigables() { } - // Render the given Twig text with the given params, using the global - // $USER variable (the question author) as a dummy student. - // @return Rendered text. - private function twig_render($text, $params=array(), $isstrict=false) { - global $USER; - $student = new qtype_coderunner_student($USER); - return qtype_coderunner_twig::render($text, $student, (array) $params, $isstrict); - } - - - private function make_question_from_form_data($data) { - // Construct a question object containing all the fields from $data. - // Used in data pre-processing and when validating a question. - global $DB, $USER; - $question = new qtype_coderunner_question(); - foreach ($data as $key => $value) { - if ($key === 'questiontext' || $key === 'generalfeedback') { - // Question text and general feedback are associative arrays. - $question->$key = $value['text']; - } else { - $question->$key = $value; + // Validate the test cases. + private function validate_test_cases($data) { + $errors = array(); // Return value. + $testcodes = $data['testcode']; + $stdins = $data['stdin']; + $expecteds = $data['expected']; + $marks = $data['mark']; + $count = 0; + $numnonemptytests = 0; + $num = max(count($testcodes), count($stdins), count($expecteds)); + for ($i = 0; $i < $num; $i++) { + $testcode = trim($testcodes[$i]); + if ($testcode != '') { + $numnonemptytests++; + } + $stdin = trim($stdins[$i]); + $expected = trim($expecteds[$i]); + if ($testcode !== '' || $stdin != '' || $expected !== '') { + $count++; + $mark = trim($marks[$i]); + if ($mark != '') { + if (!is_numeric($mark)) { + $errors["testcode[$i]"] = get_string('nonnumericmark', 'qtype_coderunner'); + } else if (floatval($mark) <= 0) { + $errors["testcode[$i]"] = get_string('negativeorzeromark', 'qtype_coderunner'); + } + } } } - $question->isnew = true; - $question->supportfilemanagerdraftid = $this->get_file_manager('datafiles'); - $question->student = new qtype_coderunner_student($USER); - // Clean the question object, get inherited fields and run the sample answer. - $qtype = new qtype_coderunner(); - $qtype->clean_question_form($question, true); - $questiontype = $question->coderunnertype; - list($category) = explode(',', $question->category); - $contextid = $DB->get_field('question_categories', 'contextid', array('id' => $category)); - $question->contextid = $contextid; - $context = context::instance_by_id($contextid, IGNORE_MISSING); - $question->prototype = $qtype->get_prototype($questiontype, $context); - $qtype->set_inherited_fields($question, $question->prototype); - return $question; + if ($count == 0) { + $errors["testcode[0]"] = get_string('atleastonetest', 'qtype_coderunner'); + } else if ($numnonemptytests != 0 && $numnonemptytests != $count) { + $errors["testcode[0]"] = get_string('allornone', 'qtype_coderunner'); + } + return $errors; } + // Check the sample answer (if there is one) // Return an empty string if there is no sample answer and no attachments, // or if the sample answer passes all the tests. // Otherwise return a suitable error message for display in the form. private function validate_sample_answer() { - assert(!empty($this->question->parameters)); + assert(!empty($this->formquestion->parameters)); $attachmentssaver = $this->get_sample_answer_file_saver(); $files = $attachmentssaver ? $attachmentssaver->get_files() : array(); - $answer = $this->question->answer; + $answer = $this->formquestion->answer; if (trim($answer) === '' && count($files) == 0) { return ''; // Empty answer and no attachments. } @@ -1126,32 +1069,35 @@ private function validate_sample_answer() { // what language to use. If there is a specific answer_language template // parameter, that is used. Otherwise the default language (if specified) // or the first in the list is used. - $acelang = trim($this->question->acelang); + $acelang = trim($this->formquestion->acelang); if ($acelang !== '' && strpos($acelang, ',') !== false) { - if (empty($this->question->parameters['answer_language'])) { + if (empty($this->formquestion->parameters['answer_language'])) { list($languages, $answerlang) = qtype_coderunner_util::extract_languages($acelang); if ($answerlang === '') { $answerlang = $languages[0]; } } else { - $answerlang = $this->question->parameters['answer_language']; + $answerlang = $this->formquestion->parameters['answer_language']; } } try { - $this->question->start_attempt(); - $response = array('answer' => $this->question->answer); + $savedevalpertry = $this->formquestion->templateparamsevalpertry; + $this->formquestion->templateparamsevalpertry = 0; // Save an extra evaluation + $this->formquestion->start_attempt(); + $this->formquestion->templateparamsevalpertry = $savedevalpertry; + $response = array('answer' => $this->formquestion->answer); if (!empty($answerlang)) { $response['language'] = $answerlang; } if ($attachmentssaver) { $response['attachments'] = $attachmentssaver; } - $error = $this->question->validate_response($response); + $error = $this->formquestion->validate_response($response); if ($error) { return $error; } - list($mark, $state, $cachedata) = $this->question->grade_response($response); + list($mark, $state, $cachedata) = $this->formquestion->grade_response($response); } catch (Exception $e) { return $e->getMessage(); } @@ -1166,6 +1112,107 @@ private function validate_sample_answer() { } } + // UTILITY FUNCTIONS. + // =================. + + // True iff the given name is valid for a new type, i.e., it's not in use + // in the current context (Currently only a single global context is + // implemented). + private function is_valid_new_type($typename) { + list($langs, $types) = $this->get_languages_and_types(); + return !array_key_exists($typename, $types); + } + + + /** + * Return a count of the number of test cases set as examples. + * @param array $data data from the form + */ + private function num_examples($data) { + return isset($data['useasexample']) ? count($data['useasexample']) : 0; + } + + private function get_languages_and_types() { + // Return two arrays (language => language_upper_case) and (type => subtype) of + // all the coderunner question types available in the current course + // context. + // The subtype is the suffix of the type in the database, + // e.g. for java_method it is 'method'. The language is the bit before + // the underscore, and language_upper_case is a capitalised version, + // e.g. Java for java. For question types without a + // subtype the word 'Default' is used. + + $records = qtype_coderunner::get_all_prototypes(); + $types = array(); + foreach ($records as $row) { + if (($pos = strpos($row->coderunnertype, '_')) !== false) { + $subtype = substr($row->coderunnertype, $pos + 1); + $language = substr($row->coderunnertype, 0, $pos); + } else { + $subtype = 'Default'; + $language = $row->coderunnertype; + } + $types[$row->coderunnertype] = $row->coderunnertype; + $languages[$language] = ucwords($language); + } + asort($types); + asort($languages); + return array($languages, $types); + } + + // Render the given Twig text with the given params, using the global + // $USER variable (the question author) as a dummy student. + // @return Rendered text. + private function twig_render($text, $params=array(), $isstrict=false) { + global $USER; + $student = new qtype_coderunner_student($USER); + return qtype_coderunner_twig::render($text, $student, (array) $params, $isstrict); + } + + + private function make_question_from_form_data($data) { + // Construct a question object containing all the fields from $data. + // Used in data pre-processing and when validating a question. + global $DB, $USER; + $question = new qtype_coderunner_question(); + foreach ($data as $key => $value) { + if ($key === 'questiontext' || $key === 'generalfeedback') { + // Question text and general feedback are associative arrays. + $question->$key = $value['text']; + } else { + $question->$key = $value; + } + } + $question->isnew = true; + $question->supportfilemanagerdraftid = $this->get_file_manager('datafiles'); + $question->student = new qtype_coderunner_student($USER); + + // Clean the question object, get inherited fields and run the sample answer. + $qtype = new qtype_coderunner(); + $qtype->clean_question_form($question, true); + $questiontype = $question->coderunnertype; + list($category) = explode(',', $question->category); + $contextid = $DB->get_field('question_categories', 'contextid', array('id' => $category)); + $question->contextid = $contextid; + $context = context::instance_by_id($contextid, IGNORE_MISSING); + $question->prototype = $qtype->get_prototype($questiontype, $context); + $qtype->set_inherited_fields($question, $question->prototype); + return $question; + } + + + + // Stop gap function until UI parameters properly refactored. + // Returns the Json for the merged template parameters. Only works + // once the question has been saved. + private function get_merged_params() { + if (isset($this->question->options->templateparamsevald)) { + return $this->question->options->templateparamsevald; + } else { + return '{}'; + } + } + // Return a file saver for the sample answer filemanager, if present. private function get_sample_answer_file_saver() { diff --git a/lang/en/qtype_coderunner.php b/lang/en/qtype_coderunner.php index e96b2e6f2..d3798be04 100644 --- a/lang/en/qtype_coderunner.php +++ b/lang/en/qtype_coderunner.php @@ -1059,18 +1059,27 @@ function should be applied, e.g. {{STUDENT_ANSWER | e(\'py\')}} is If the template-debugging checkbox is clicked, the program generated for each testcase will be displayed in the output.'; $string['templateparams'] = 'Template params'; +$string['templateparamsevalpertry'] = 'Evaluate on each attempt'; $string['templateparamslang'] = 'Preprocessor'; -$string['templateparams_help'] = 'The template parameters field lets you pass string parameters to a question\'s -template(s). If non-blank, this must evaluate to a JSON-format record. It can -be a pure JSON string, or JSON with embedded Twig code or a Python -program that outputs JSON when run. The latter mode is highly experimental -and may have significant performance implications because the template -parameter code must -be sent to the Jobe sandbox to be executed even before the question can be -displayed. To enable the Python mode, set the preprocessor to Python. +$string['templateparams_help'] = 'If non-blank, the template parameters field must +evaluate to a JSON-format record. In its simplest form the field is just a JSON +record defining a set of variables that are added to the environment for the Twig +template engine when it expands the template. If a preprocessor is specified, +the template parameters are first processed by the specified language +to yield a JSON record. See the documentation for details. + +Warning: use of a preprocessor other than Twig can have drastic performance +implications if the Evaluate-on-each-attempt checkbox is +checked, which it has to be if used for randomisation or for per-student question +customisation. Preprocessing must be done before a question +can be displayed to a student and, except for Twig, takes place on the Jobe sandbox +server. Every attempt on every question of this sort by every student will result in a job +being sent to that server. This can result in thousands of jobs hitting the +Jobe server at once at the start of a large test or exam, which will almost +certainly overload it. Caveat Emptor! It Hoist template parameters is unchecked the fields of -the JSON record can then be used within the template in the form +the JSON record can be used within the template in the form QUESTION.parameters.<<param>> For example, if template params is {"age": 23} @@ -1079,30 +1088,7 @@ function should be applied, e.g. {{STUDENT_ANSWER | e(\'py\')}} is template variable {{ QUESTION.parameters.age }}. If Hoist template parameters is checked the json field names can be -used without the prefix, e.g. as {{ age }} directly. - -The set of template parameters passed to the template consists of any template -parameters defined in the prototype with the question template parameters -merged in. Question parameters can thus override prototype parameters, but not -delete them. - -Template parameters can also be used to provide randomisation within a question. -When the question is first instantiated the template parameters are passed -either through the Twig template engine if the Preprocessor is Twig -or the Python interpreter otherwise to yield the final JSON version. -Twig\'s "random" function can -be used to assign random values to template parameters. If the "Twig All" checkbox -is checked, all other fields of the question (question text, answer, test cases -etc) are the also processed by Twig, with the template parameters as an -environment. This can result in different -students seeing different random variants of the question. See the documentation -for details. - -If Python is the preprocessor the template parameters must be a Python -program that can accept through sys.argv up to two arguments of the form ---seed=1234 and --student=\ where -seed is the random number seed and student is the current -student.'; +used without the prefix, e.g. as {{ age }} directly.'; $string['testalltitle'] = 'Test all questions in this context'; $string['testallincategory'] = 'Test all questions in this category'; $string['testcase'] = 'Test case {$a}'; diff --git a/question.php b/question.php index 7b449d05f..549727a56 100644 --- a/question.php +++ b/question.php @@ -64,7 +64,7 @@ public function start_attempt(question_attempt_step $step=null, $variant=null) { $this->student = $DB->get_record('user', array('id' => $userid)); $step->set_qt_var('_STUDENT', serialize($this->student)); } else { // Validation, so just use the global $USER as student. - $this->student = $USER; + $this->student = new qtype_coderunner_student($USER); } $seed = mt_rand(); @@ -103,7 +103,8 @@ public function apply_attempt_state(question_attempt_step $step) { // public function evaluate_question_for_display($seed, $step) { $this->get_prototype(); - $this->evaluate_merged_parameters($seed, $step); + $paramsjson = $this->evaluate_merged_parameters($seed, $step); + $this->parameters = json_decode($paramsjson); if ($this->twigall) { $this->twig_all(); } @@ -116,28 +117,41 @@ public function evaluate_question_for_display($seed, $step) { * or the student. * The parameters are defined by running the template parameters field * through the appropriate language processor as specified by the - * templateparamslang field (default: Twig) and then json-decoding - * the result to a PHP associative array. That array needs to be merged with + * templateparamslang field (default: Twig). The result needs to be merged with * the prototype's parameters, which are subject to the same process. - * After running this function, the $this->parameters is the array of - * parameters. + * After running this function, the $this->parameters is a stdClass object + * with all the parameters as attributes. + * If the prototype is missing, process just the template parameters from + * this question; an error message will be given later. * @param int $seed The random number seed to set for Twig randomisation * @param question_attempt_step $step The current question attempt step + * @return string the json string of the merged template parameters. */ function evaluate_merged_parameters($seed, $step=null) { - assert(isset($this->templateparams)); - $paramsjson = $this->template_params_json($this->templateparams, - $this->templateparamslang, $seed, $step, '_template_params'); - $prototype = $this->prototype; - $prototypeparamsjson = $this->template_params_json($prototype->templateparams, - $prototype->templateparamslang, $seed, $step, '_prototype__template_params'); - - $mergedjson = qtype_coderunner_util::merge_json($prototypeparamsjson, $paramsjson); - if (empty($mergedjson)) { - $mergedjson = '{}'; - } - $this->parameters = json_decode($mergedjson); + // If we have a value for templateparamsevald and we're asked to use it, do so. + if (!$this->templateparamsevalpertry && !empty($this->templateparamsevald)) { + $paramsjson = $this->templateparamsevald; + } else { + + // Otherwise we have to evaluate this and our prototypes template + // parameters and merge them. + assert(isset($this->templateparams)); + $paramsjson = $this->template_params_json($this->templateparams, + $this->templateparamslang, $seed, $step, '_template_params'); + $prototype = $this->prototype; + if ($prototype !== null) { + // Merge with prototype parameters (unless prototype is missing). + $prototypeparamsjson = $this->template_params_json($prototype->templateparams, + $prototype->templateparamslang, $seed, $step, '_prototype__template_params'); + $paramsjson = qtype_coderunner_util::merge_json($prototypeparamsjson, $paramsjson); + } + + if (empty($paramsjson)) { + $paramsjson = '{}'; + } + } + return $paramsjson; // TODO - how to report an error on the json_decode?? } @@ -154,7 +168,7 @@ function evaluate_merged_parameters($seed, $step=null) { * json (with suffix '_json'). * @return string THe Json template parameters. */ - private function template_params_json($params, $lang, $seed, $step, $qtvar) { + function template_params_json($params, $lang, $seed=0, $step=null, $qtvar='') { if ($step === null) { $jsontemplateparams = $this->evaluate_template_params($params, $lang, $seed, $this->student); @@ -186,25 +200,28 @@ private function template_params_json($params, $lang, $seed, $step, $qtvar) { * valid json). */ function evaluate_template_params($templateparams, $lang, $seed) { - if ($lang == 'twig') { + if ($lang == 'None') { + $jsontemplateparams = $templateparams; + } else if ($lang == 'twig') { $jsontemplateparams = $this->twig_render_with_seed($templateparams, $seed); + } else if (!$this->templateparamsevalpertry && !empty($this->templateparamsevald)) { + $jsontemplateparams = $this->templateparamsevald; } else { - assert($lang == 'python'); - $jsontemplateparams = $this->evaluate_template_params_python($templateparams, $seed); + $jsontemplateparams = $this->evaluate_template_params_on_jobe($templateparams, $lang, $seed); } return $jsontemplateparams; } /** - * Evaluate a template parameter string using Python in the Jobe - * engine. Return value should be the JSON template parameter string. + * Evaluate a template parameter string using a given language on the Jobe + * server. Return value should be the JSON template parameter string. * * @param string $templateparams The template parameters to evaluate. * @param int $seed The random number seed to use when evaluating. * @return string The output from the run. */ - private function evaluate_template_params_python($templateparams, $seed) { + private function evaluate_template_params_on_jobe($templateparams, $lang, $seed) { $files = array(); $input = ''; $runargs = array("seed=$seed"); @@ -213,7 +230,7 @@ private function evaluate_template_params_python($templateparams, $seed) { } $sandboxparams = array("runargs" => $runargs); $sandbox = $this->get_sandbox(); - $run = $sandbox->execute($templateparams, "python3", $input, $files, $sandboxparams); + $run = $sandbox->execute($templateparams, $lang, $input, $files, $sandboxparams); if ($run->error !== qtype_coderunner_sandbox::OK) { return qtype_coderunner_sandbox::error_string($run); } else if ($run->result != qtype_coderunner_sandbox::RESULT_SUCCESS) { @@ -226,7 +243,7 @@ private function evaluate_template_params_python($templateparams, $seed) { // Render the given twig text using the given random number seed and // student variable. This version should be called only during question - // initialisation when randomisation is being done. + // initialisation when evaluating the template parameters. private function twig_render_with_seed($text, $seed) { mt_srand($seed); return qtype_coderunner_twig::render($text, $this->student); diff --git a/questiontype.php b/questiontype.php index 597195822..78b98b41f 100644 --- a/questiontype.php +++ b/questiontype.php @@ -110,6 +110,8 @@ public function extra_question_fields() { 'templateparams', 'hoisttemplateparams', 'templateparamslang', + 'templateparamsevalpertry', + 'templateparamsevald', 'twigall', 'uiplugin', 'attachments', @@ -143,6 +145,8 @@ public static function noninherited_fields() { 'templateparams', 'hoisttemplateparams', 'templateparamslang', + 'templateparamsevalpertry', + 'templateparamsevald', 'twigall', 'attachments', 'attachmentsrequired', @@ -383,11 +387,6 @@ public function move_files($questionid, $oldcontextid, $newcontextid) { // The various fields are initialised from the prototype, then overridden // by any non-null values in the specific question. // - // As a special case, required by edit_coderunner_form, an option - // 'mergedtemplateparams' is set by merging the prototype question's - // template parameters with the given question's template parameters, - // with the caveat that template parameters with embedded twig code that - // aren't valid JSON are ignored. public function get_question_options($question) { global $CFG, $DB, $OUTPUT; parent::get_question_options($question); @@ -395,18 +394,11 @@ public function get_question_options($question) { if ($options->prototypetype != 0) { // Question prototype? // Yes. It's 100% customised with nothing to inherit. $options->customise = true; - $options->mergedtemplateparams = $options->templateparams; } else { $qtype = $options->coderunnertype; $context = $this->question_context($question); $prototype = $this->get_prototype($qtype, $context); $this->set_inherited_fields($options, $prototype); - if ($prototype !== null && trim($prototype->templateparams) !== '') { - $options->mergedtemplateparams = qtype_coderunner_util::merge_json( - $prototype->templateparams, $options->templateparams); - } else { // Missing prototype! - $options->mergedtemplateparams = $options->templateparams; - } } // Add in any testcases. @@ -453,10 +445,6 @@ public function set_inherited_fields($target, $prototype) { } } - // Save prototype template params in the target, to be merged with - // the question template params if the target is actually run. - $target->prototypetemplateparams = $prototype->templateparams; - if (!isset($target->sandbox)) { $target->sandbox = null; } @@ -744,6 +732,8 @@ public function 'iscombinatortemplate' => null, // Probably unnecessary? 'template' => null, // Probably unnecessary? 'templateparamslang' => 'twig', + 'templateparamsevalpertry' => 0, + 'templateparamsevald' => null, 'attachments' => 0 ); diff --git a/tests/behat/check_python_template_params.feature b/tests/behat/check_python_template_params.feature index 48815cbf7..7e38507f8 100644 --- a/tests/behat/check_python_template_params.feature +++ b/tests/behat/check_python_template_params.feature @@ -1,7 +1,7 @@ @qtype @qtype_coderunner @javascript @pythonpreprocessortest -Feature: Check that Python can be used instead of Twig as a template params preprocessor and that it processes the STUDENT variable correctly. - To check that the STUDENT template parameter variables work when python is the preprocessor +Feature: Check that Python and other languages can be used instead of Twig as a template params preprocessor and that they processes the STUDENT variable correctly. + To check that the STUDENT template parameter variables work when a language other than python is the preprocessor As a teacher I should be able to write a function that prints the seed and my username it should be marked right @@ -29,14 +29,15 @@ Background: And I disable UI plugins And I add a "CodeRunner" question filling the form with: | id_coderunnertype | python3 | - | id_customise | 1 | + | id_customise | 1 | | id_name | STUDENT variable | | id_questiontext | Write a program that prints True if seed parameter provided, then {{ username }} | | id_answerboxlines | 5 | | id_validateonsave | 0 | | id_templateparams | import sys, json; keyvalues = {param.split('=')[0]: param.split('=')[1] for param in sys.argv[1:]}; print(json.dumps(keyvalues)) | - | id_templateparamslang | python | - | id_template | print(int("{{seed}}") > 0, "{{username}}") | + | id_templateparamslang | python3 | + | id_templateparamsevalpertry | 1 | + | id_template | print(int("{{seed}}") > 0, end=' '); {{STUDENT_ANSWER}} | | id_answer | # Unused | | id_iscombinatortemplate | 0 | | id_testcode_0 | # Unused | @@ -75,3 +76,33 @@ Background: Then I should see "Write a program that prints True if seed parameter provided, then student1" And I should see "Passed all tests" + Scenario: Turn off per-try evaluation. Question should fail when attempted by student. + + When I choose "Edit question" action for "STUDENT variable" in the question bank + And I set the following fields to these values: + | id_templateparamsevalpertry | 0 | + | id_questiontext | Variant without per-try evaluation | + And I press "id_submitbutton" + Then I should see "Created by" + And I should see "Last modified by" + + When I am on "Course 1" course homepage + And I follow "Test quiz" + And I press "Preview quiz now" + Then I should see "Variant without per-try evaluation" + + When I set the field with xpath "//textarea[contains(@name, 'answer')]" to "print('teacher1')" + And I press "Check" + Then I should see "Passed all tests" + + When I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And I follow "Test quiz" + And I press "Attempt quiz" + Then I should see "Variant without per-try evaluation" + And I set the field with xpath "//textarea[contains(@name, 'answer')]" to "print('student1')" + And I press "Check" + Then I should see "True student1" + And I should see "True teacher1" + And I should not see "Passed all tests" diff --git a/tests/helper.php b/tests/helper.php index 76d4b9b8a..592ba0feb 100644 --- a/tests/helper.php +++ b/tests/helper.php @@ -142,6 +142,8 @@ public function get_coderunner_question_form_data_sqr() { $form->templateparams = ""; $form->hoisttemplateparams = 1; $form->templateparamslang = 'twig'; + $form->templateparamsevalpertry = 0; + $form->templateparamsevald = null; $form->twigall = 0; $form->prototypetype = 0; $form->sandbox = 'DEFAULT'; @@ -198,6 +200,8 @@ public function get_coderunner_question_form_data_printans() { $form->templateparams = ""; $form->hoisttemplateparams = 1; $form->templateparamslang = 'twig'; + $form->templateparamsevalpertry = 0; + $form->templateparamsevald = null; $form->twigall = 0; $form->prototypetype = 0; $form->sandbox = 'DEFAULT'; @@ -1176,6 +1180,8 @@ private function make_coderunner_question($type, $name = '', $questiontext = '', $coderunner->coderunnertype = $type; $coderunner->templateparams = ''; $coderunner->templateparamslang = 'twig'; + $coderunner->templateparamsevalpertry = 0; + $coderunner->templateparamsevald = null; $coderunner->hoisttemplateparams = 0; $coderunner->twigall = 0; $coderunner->prototypetype = 0; diff --git a/tests/prototype_test.php b/tests/prototype_test.php index b4a6ac6f5..9f8d7758a 100644 --- a/tests/prototype_test.php +++ b/tests/prototype_test.php @@ -138,6 +138,8 @@ public function test_export() { twig + 0 + twig 0 0 diff --git a/tests/walkthrough_randomised_questions.php b/tests/walkthrough_randomised_questions.php index 08ee7389f..607aedea0 100644 --- a/tests/walkthrough_randomised_questions.php +++ b/tests/walkthrough_randomised_questions.php @@ -129,6 +129,8 @@ private function add_fields($q, $seed=false) { $q->templateparams = $seeding . $templateparams; $q->templateparamslang = 'twig'; + $q->templateparamsevalpertry = 0; + $q->templateparamsevald = null; $q->hoisttemplateparams = 1; $q->twigall = 1; $q->questiontext = 'Write a function {{ func }}'; diff --git a/version.php b/version.php index ff19b4a2e..16c80f9e2 100644 --- a/version.php +++ b/version.php @@ -22,7 +22,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2020120702; +$plugin->version = 2020121501; $plugin->requires = 2015051200; $plugin->cron = 0; $plugin->component = 'qtype_coderunner';