Refactoring template-parameters processing code (continuing).

db/install.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="question/type/coderunner/db" VERSION="20201207" COMMENT="XMLDB file for Moodle question/type/coderunner"
+<XMLDB PATH="question/type/coderunner/db" VERSION="20201215" COMMENT="XMLDB file for Moodle question/type/coderunner"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../../../lib/xmldb/xmldb.xsd"
 >
@@ -31,6 +31,8 @@
         <FIELD NAME="pertesttemplate" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="DEFUNCT: A template to build a test run for each test case. Required."/>
         <FIELD NAME="templateparams" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="A json-encoded record of extra parameters to pass to the template engine when building the source code to be executed."/>
         <FIELD NAME="templateparamslang" TYPE="char" LENGTH="50" NOTNULL="false" DEFAULT="twig" SEQUENCE="false" COMMENT="The language in which the template parameters are defined."/>
+        <FIELD NAME="templateparamsevalpertry" TYPE="int" LENGTH="1" NOTNULL="false" DEFAULT="0" SEQUENCE="false" COMMENT="Set true to re-evaluate the template parameters every time on every new question attempt. Essential for randomisation or if the Student variable is used. Not relevant if templateparamslang is None (no evaluation) or Twig (runs on Moodle and re-evaluates)"/>
+        <FIELD NAME="templateparamsevald" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="The evaluated template parameters (JSON) if templateparamslang is neither None nor Twig and templateparamsevaleverytry is false."/>
         <FIELD NAME="hoisttemplateparams" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="True to hoist templateparams into Twig global namespace."/>
         <FIELD NAME="twigall" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="True to apply Twig macro expansion to question text and testcases."/>
         <FIELD NAME="language" TYPE="char" LENGTH="255" NOTNULL="false" SEQUENCE="false" COMMENT="The programming language, to be passed to the sandbox"/>

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();
 

lang/en/qtype_coderunner.php

@@ -1059,18 +1059,27 @@ function should be applied, e.g. <code>{{STUDENT_ANSWER | e(\'py\')}}</code> 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 <em>Python</em>.
+$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.
+
+<b>Warning:<b> 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 <em>Hoist template parameters</em> 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.&lt;&lt;param&gt;&gt; For example, if template params is
 
         {"age": 23}
@@ -1079,30 +1088,7 @@ function should be applied, e.g. <code>{{STUDENT_ANSWER | e(\'py\')}}</code> is
 template variable <code>{{ QUESTION.parameters.age }}</code>.
 
 If <em>Hoist template parameters</em> is checked the json field names can be
-used without the prefix, e.g. as <code>{{ age }}</code> 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 <em>Twig</em>
-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
-<em>--seed=1234</em> and <em>--student=\<json-encode-student-record\></em> where
-<em>seed</em> is the random number seed and <em>student</em> is the current
-student.';
+used without the prefix, e.g. as <code>{{ age }}</code> directly.';
 $string['testalltitle'] = 'Test all questions in this context';
 $string['testallincategory'] = 'Test all questions in this category';
 $string['testcase'] = 'Test case {$a}';

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);

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,30 +387,18 @@ 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);
         $options =& $question->options;
         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
         );