Grade caching code merged into development branch

.gitignore

@@ -34,4 +34,4 @@ NonRepoFiles/*
 /amd/src/ui_blockly.json
 /amd/src/.eslintrc.js
 .grunt
-
+.vscode/

bulktestindex.php

@@ -150,7 +150,7 @@
                 expander.innerHTML = 'Collapse';
                 expander.nextSibling.style.display = 'inline';
             } else {
-                expander.innerTHML = 'Expand';
+                expander.innerHTML = 'Expand';
                 expander.nextSibling.style.display = 'none';
             }
         });

classes/combinator_grader_outcome.php

@@ -47,6 +47,15 @@ class qtype_coderunner_combinator_grader_outcome extends qtype_coderunner_testin
     /** @var bool If true, is used when the question is to be used only to display the output and perhaps images from a run, with no mark. */
     public $showoutputonly;
 
+    /** @var array Array where each item is a rows of test result table  */
+    public $testresults;
+
+    /** @var ?string The feedback for a given question attempt */
+    public $feedbackhtml;
+
+    /** @var bool Whether or no show differences is selected */
+    public $showdifferences;
+
     // A list of the allowed attributes in the combinator template grader return value.
     public $allowedfields = ['fraction', 'prologuehtml', 'testresults', 'epiloguehtml',
                     'feedbackhtml', 'columnformats', 'showdifferences',

classes/jobesandbox.php

@@ -30,6 +30,9 @@
 global $CFG;
 require_once($CFG->libdir . '/filelib.php'); // Needed when run as web service.
 
+const READ_FROM_CACHE = true;
+const WRITE_TO_CACHE = true;
+
 class qtype_coderunner_jobesandbox extends qtype_coderunner_sandbox {
     const DEBUGGING = 0;
     const HTTP_GET = 1;
@@ -197,57 +200,86 @@ public function execute($sourcecode, $language, $input, $files = null, $params =
                 $this->apikey = $params['jobeapikey'];
             }
         }
-
-        $postbody = ['run_spec' => $runspec];
+        // QUESTION: Do we need this when using cached result?
         $this->currentjobid = sprintf('%08x', mt_rand());
 
-        // Try submitting the job. If we get a 404, try again after
-        // putting all the files on the server. Anything else is an error.
-        $httpcode = $this->submit($postbody);
-        if ($httpcode == 404) { // If it's a file not found error ...
-            foreach ($files as $filename => $contents) {
-                if (($httpcode = $this->put_file($contents)) != 204) {
-                    break;
-                }
-            }
-            if ($httpcode == 204) {
-                // Try again if put_files all worked.
-                $httpcode = $this->submit($postbody);
+        $cache = cache::make('qtype_coderunner', 'coderunner_grading_cache');
+        $runresult = null;
+        if (READ_FROM_CACHE) {
+            // NOTE: Changing jobeserver setting will effectively flush the cache
+            // eg, adding another jobeserver to a list of servers will mean the
+            // jobeserver parameter has changed and therefore the key will change.
+
+            // QUESTION: Do we want the cache to ignore the jobeserver setting?
+            // eg, adding a new, presumeably equal jobeserver to the mix shouldn't
+            // change the result (unless it isn't equal!)
+            // But, remember that the server is chosen at random from the pool!
+
+            $key = hash("md5", serialize($runspec));
+            // Debugger: echo '<pre>' . serialize($runspec) . '</pre>';.
+            $runresult = $cache->get($key);  // Unserializes the returned value :) false if not found.
+            if ($runresult) {
+                // echo $key . '----------->   FOUND' . '<br>'; .
             }
         }
 
-        $runresult = [];
-        $runresult['sandboxinfo'] = [
-            'jobeserver' => $this->jobeserver,
-            'jobeapikey' => $this->apikey,
-        ];
+        if (!$runresult) {  // if cache read failed regrade to be safe
+            $postbody = ['run_spec' => $runspec];
+            // Try submitting the job. If we get a 404, try again after
+            // putting all the files on the server. Anything else is an error.
+            $httpcode = $this->submit($postbody);
+            if ($httpcode == 404) { // If it's a file not found error ...
+                foreach ($files as $filename => $contents) {
+                    if (($httpcode = $this->put_file($contents)) != 204) {
+                        break;
+                    }
+                }
+                if ($httpcode == 204) {
+                    // Try again if put_files all worked.
+                    $httpcode = $this->submit($postbody);
+                }
+            }
 
-        $okresponse = in_array($httpcode, [200, 203]);  // Allow 203, which can result from an intevening proxy server.
-        if (
-            !$okresponse                        // If it's not an OK response...
-                || !is_object($this->response)  // ... or there's any sort of broken ...
-                || !isset($this->response->outcome)
-        ) {     // ... communication with server.
-            // Return with errorcode set and as much extra info as possible in stderr.
-            $errorcode = $okresponse ? self::UNKNOWN_SERVER_ERROR : $this->get_error_code($httpcode);
-            $this->currentjobid = null;
-            $runresult['error'] = $errorcode;
-            $runresult['stderr'] = "HTTP response from Jobe ({$this->jobeserver}) was $httpcode: " . json_encode($this->response);
-        } else if ($this->response->outcome == self::RESULT_SERVER_OVERLOAD) {
-            $runresult['error'] = self::SERVER_OVERLOAD;
-        } else {
-            $stderr = $this->filter_file_path($this->response->stderr);
-            // Any stderr output is treated as a runtime error.
-            if (trim($stderr ?? '') !== '') {
-                $this->response->outcome = self::RESULT_RUNTIME_ERROR;
+            $runresult = [];
+            $runresult['sandboxinfo'] = [
+                'jobeserver' => $this->jobeserver,
+                'jobeapikey' => $this->apikey,
+            ];
+
+            $okresponse = in_array($httpcode, [200, 203]);  // Allow 203, which can result from an intevening proxy server.
+            if (
+                !$okresponse                        // If it's not an OK response...
+                    || !is_object($this->response)  // ... or there's any sort of broken ...
+                    || !isset($this->response->outcome)
+            ) {     // ... communication with server.
+                // Return with errorcode set and as much extra info as possible in stderr.
+                $errorcode = $okresponse ? self::UNKNOWN_SERVER_ERROR : $this->get_error_code($httpcode);
+                $this->currentjobid = null;
+                $runresult['error'] = $errorcode;
+                $runresult['stderr'] = "HTTP response from Jobe was $httpcode: " . json_encode($this->response);
+            } else if ($this->response->outcome == self::RESULT_SERVER_OVERLOAD) {
+                $runresult['error'] = self::SERVER_OVERLOAD;
+            } else {
+                $stderr = $this->filter_file_path($this->response->stderr);
+                // Any stderr output is treated as a runtime error.
+                if (trim($stderr ?? '') !== '') {
+                    $this->response->outcome = self::RESULT_RUNTIME_ERROR;
+                }
+                $this->currentjobid = null;
+                $runresult['error'] = self::OK;
+                $runresult['stderr'] = $stderr;
+                $runresult['result'] = $this->response->outcome;
+                $runresult['signal'] = 0; // Jobe doesn't return signals.
+                $runresult['cmpinfo'] = $this->response->cmpinfo;
+                $runresult['output'] = $this->filter_file_path($this->response->stdout);
+
+                // Got a useable result from Jobe server so cache it if required.
+                if (WRITE_TO_CACHE) {
+                    $key = hash("md5", serialize($runspec));
+                    $cache->set($key, $runresult); // set serializes the result, get will unserialize.
+                    // echo 'CACHE WRITE for ---> ' . $key . '<br>';
+                }
             }
-            $this->currentjobid = null;
-            $runresult['error'] = self::OK;
-            $runresult['stderr'] = $stderr;
-            $runresult['result'] = $this->response->outcome;
-            $runresult['signal'] = 0; // Jobe doesn't return signals.
-            $runresult['cmpinfo'] = $this->response->cmpinfo;
-            $runresult['output'] = $this->filter_file_path($this->response->stdout);
         }
         return (object) $runresult;
     }
@@ -256,58 +288,61 @@ public function execute($sourcecode, $language, $input, $files = null, $params =
     // such class found. Removes comments, strings and nested code and then
     // uses a regexp to find a public class.
     private function get_main_class($prog) {
-        // filter out comments and strings
+        // Filter out comments and strings.
         $prog = $prog . ' ';
-        $filteredProg = array();
-        $skipTo = -1;
+        $filteredprog = [];
+        $skipto = -1;
 
         for ($i = 0; $i < strlen($prog) - 1; $i++) {
-            if ($skipTo == false) break;  // an unclosed comment/string - bail out
-            if ($i < $skipTo) continue;
-
-            // skip "//" comments
-            if ($prog[$i].$prog[$i+1] == '//') {
-                $skipTo = strpos($prog, "\n", $i + 2);
+            if ($skipto == false) {
+                break;  // An unclosed comment/string - bail out.
             }
-
-            // skip "/**/" comments
-            else if ($prog[$i].$prog[$i+1] == '/*') {
-                $skipTo = strpos($prog, '*/', $i + 2) + 2;
-                $filteredProg[] = ' ';  // '/**/' is a token delimiter
+            if ($i < $skipto) {
+                continue;
             }
-
-            // skip strings
-            else if ($prog[$i] == '"') {
-                // matches the whole string
+            // Skip "//" comments.
+            if ($prog[$i] . $prog[$i + 1] == '//') {
+                $skipto = strpos($prog, "\n", $i + 2);
+            // Skip "/**/" comments.
+            } else if ($prog[$i] . $prog[$i + 1] == '/*') {
+                $skipto = strpos($prog, '*/', $i + 2) + 2;
+                $filteredprog[] = ' ';  // The string '/**/' is a token delimiter.
+            // Skip strings.
+            } else if ($prog[$i] == '"') {
+                // Matches the whole string.
                 if (preg_match('/"((\\.)|[^\\"])*"/', $prog, $matches, 0, $i)) {
-                    $skipTo = $i + strlen($matches[0]);
+                    $skipto = $i + strlen($matches[0]);
+                } else {
+                    $skipto = false;
                 }
-                else $skipTo = false;
+            // Copy everything else.
+            } else {
+                $filteredprog[] = $prog[$i];
             }
-
-            // copy everything else
-            else $filteredProg[] = $prog[$i];
         }
 
-        // remove nested code
+        // Remove nested code.
         $depth = 0;
-        for ($i = 0; $i < count($filteredProg); $i++) {
-            if ($filteredProg[$i] == '{') $depth++;
-            if ($filteredProg[$i] == '}') $depth--;
-            if ($filteredProg[$i] != "\n" && $depth > 0 && !($depth == 1 && $filteredProg[$i] == '{')) {
-                $filteredProg[$i] = ' ';
+        for ($i = 0; $i < count($filteredprog); $i++) {
+            if ($filteredprog[$i] == '{') {
+                $depth++;
+            }
+            if ($filteredprog[$i] == '}') {
+                $depth--;
+            }
+            if ($filteredprog[$i] != "\n" && $depth > 0 && !($depth == 1 && $filteredprog[$i] == '{')) {
+                $filteredprog[$i] = ' ';
             }
         }
 
-        // search for a public class
-        if (preg_match('/public\s(\w*\s)*class\s*(\w+)[^\w]/', implode('', $filteredProg), $matches) !== 1) {
+        // Search for a public class.
+        if (preg_match('/public\s(\w*\s)*class\s*(\w+)[^\w]/', implode('', $filteredprog), $matches) !== 1) {
             return false;
         } else {
             return $matches[2];
         }
     }
 
-
 
     // Return the sandbox error code corresponding to the given httpcode.
     private function get_error_code($httpcode) {

classes/jobrunner.php

@@ -36,6 +36,7 @@ class qtype_coderunner_jobrunner {
     private $testcases = null;       // The testcases (a subset of those in the question).
     private $allruns = null;         // Array of the source code for all runs.
 
+
     /** @var ?array Array of sandbox params. */
     private $sandboxparams = null;
 
@@ -48,15 +49,32 @@ class qtype_coderunner_jobrunner {
     /** @var bool True if this grading is occurring because the student clicked the precheck button. */
     private $isprecheck = false;
 
-    // Check the correctness of a student's code and possible extra attachments
-    // as an answer to the given
-    // question and and a given set of test cases (which may be empty or a
-    // subset of the question's set of testcases. $isprecheck is true if
-    // this is a run triggered by the student clicking the Precheck button.
-    // $answerlanguage will be the empty string except for multilanguage questions,
-    // when it is the language selected in the language drop-down menu.
-    // Returns a TestingOutcome object.
-    public function run_tests($question, $code, $attachments, $testcases, $isprecheck, $answerlanguage) {
+
+
+    /**
+     * Check the correctness of a student's code and possible extra attachments
+     * as an answer to the given question and and a given set of test cases (which may be empty or a
+     * subset of the question's set of testcases.
+     * @param qtype_coderunner_question $question object relevant to this step of the attempt
+     * @param string $code is the JSON repr of the code
+     * @param array $attachments is the array of attachments given by student, if any
+     * @param
+     * @param boolean $isprecheck is true if
+     * this is a run triggered by the student clicking the Precheck button.
+     * @param string $answerlanguage will be the empty string except for multilanguage questions,
+     *      when it is the language selected in the language drop-down menu.
+     * @return qtype_coderunner_combinator_grader_outcome $testoutcome that contains the outcome
+     *      of the grading.
+     */
+    public function run_tests(
+        $question,
+        $code,
+        $attachments,
+        $testcases,
+        $isprecheck,
+        $answerlanguage
+    ) {
+
         if (empty($question->prototype)) {
             // Missing prototype. We can't run this question.
             $outcome = new qtype_coderunner_testing_outcome(0, 0, false);
@@ -69,7 +87,8 @@ public function run_tests($question, $code, $attachments, $testcases, $isprechec
                     ['crtype' => $question->coderunnertype]
                 );
             }
-            $outcome->set_status(qtype_coderunner_testing_outcome::STATUS_MISSING_PROTOTYPE, $message);
+            $status = qtype_coderunner_testing_outcome::STATUS_MISSING_PROTOTYPE;
+            $outcome->set_status($status, $message);
             return $outcome;
         }
 
@@ -131,10 +150,11 @@ public function run_tests($question, $code, $attachments, $testcases, $isprechec
         if ($question->get_show_source()) {
             $outcome->sourcecodelist = $this->allruns;
         }
+
+
         return $outcome;
     }
 
-
     // If the template is a combinator, try running all the tests in a single
     // go.
     //
@@ -143,6 +163,13 @@ public function run_tests($question, $code, $attachments, $testcases, $isprechec
     // a list of all the test cases and QUESTION, the original question object.
     // Return the testing outcome object if successful else null.
     private function run_combinator($isprecheck) {
+        // Remove id and questionid keys+values from testcases so they don't
+        // affect caching. For example the questionid will change each time
+        // the question is saved thanks to question versioning - urgh!
+        foreach ($this->testcases as $tc) {
+            unset($tc->id);
+            unset($tc->questionid);
+        }
         $numtests = count($this->testcases);
         $this->templateparams['TESTCASES'] = $this->testcases;
         $maxmark = $this->maximum_possible_mark();
@@ -217,6 +244,13 @@ private function run_tests_singly($isprecheck) {
         if ($maxmark == 0) {
             $maxmark = 1; // Something silly is happening. Probably running a prototype with no tests.
         }
+        // Remove id and questionid keys+values from testcases so they don't
+        // affect caching. For example the questionid will change each time
+        // the question is saved thanks to question versioning - urgh!
+        foreach ($this->testcases as $tc) {
+            unset($tc->id);
+            unset($tc->questionid);
+        }
         $numtests = count($this->testcases);
         $outcome = new qtype_coderunner_testing_outcome($maxmark, $numtests, $isprecheck);
         $question = $this->question;