MDL-34399 questions: cache to help load question definitions.

At the moment, it takes several DB queries to load each question definition,
so this cache is potentially a big win.

lang/en/cache.php

@@ -38,6 +38,7 @@
 $string['cachedef_databasemeta'] = 'Database meta information';
 $string['cachedef_eventinvalidation'] = 'Event invalidation';
 $string['cachedef_locking'] = 'Locking';
+$string['cachedef_questiondata'] = 'Question definitions';
 $string['cachedef_string'] = 'Language string cache';
 $string['cachelock_file_default'] = 'Default file locking';
 $string['cachestores'] = 'Cache stores';

lib/db/caches.php

@@ -27,6 +27,7 @@
  */
 
 $definitions = array(
+
     // Used to store processed lang files.
     'string' => array(
         'mode' => cache_store::MODE_APPLICATION,
@@ -35,6 +36,7 @@
         'persistent' => true,
         'persistentmaxsize' => 3
     ),
+
     // Used to store database meta information.
     'databasemeta' => array(
         'mode' => cache_store::MODE_APPLICATION,
@@ -44,15 +46,27 @@
         'persistent' => true,
         'persistentmaxsize' => 2
     ),
+
     // Used to store data from the config + config_plugins table in the database.
     'config' => array(
         'mode' => cache_store::MODE_APPLICATION,
         'persistent' => true
     ),
+
     // Event invalidation cache.
     'eventinvalidation' => array(
         'mode' => cache_store::MODE_APPLICATION,
         'persistent' => true,
         'requiredataguarantee' => true
-    )
+    ),
+
+    // Cache for question definitions. This is used by the question_bank class.
+    // Users probably do not need to know about this cache. They will just call
+    // question_bank::load_question.
+    'questiondata' => array(
+        'mode' => cache_store::MODE_APPLICATION,
+        'requiredataguarantee' => false,
+        'datasource' => 'question_finder',
+        'datasourcefile' => 'question/engine/bank.php',
+    ),
 );

lib/questionlib.php

@@ -343,6 +343,7 @@ function question_delete_question($questionid) {
 
     // Finally delete the question record itself
     $DB->delete_records('question', array('id' => $questionid));
+    question_bank::notify_question_edited($questionid);
 }
 
 /**
@@ -607,6 +608,11 @@ function question_move_questions_to_category($questionids, $newcategoryid) {
 
     // TODO Deal with datasets.
 
+    // Purge these questions from the cache.
+    foreach ($questions as $question) {
+        question_bank::notify_question_edited($question->id);
+    }
+
     return true;
 }
 
@@ -626,6 +632,8 @@ function question_move_category_to_context($categoryid, $oldcontextid, $newconte
     foreach ($questionids as $questionid => $qtype) {
         question_bank::get_qtype($qtype)->move_files(
                 $questionid, $oldcontextid, $newcontextid);
+        // Purge this question from the cache.
+        question_bank::notify_question_edited($questionid);
     }
 
     $subcatids = $DB->get_records_menu('question_categories',
@@ -860,8 +868,11 @@ function question_hash($question) {
  * Saves question options
  *
  * Simply calls the question type specific save_question_options() method.
+ * @deprecated all code should now call the question type method directly.
  */
 function save_question_options($question) {
+    debugging('Please do not call save_question_options any more. Call the question type method directly.',
+            DEBUG_DEVELOPER);
     question_bank::get_qtype($question->qtype)->save_question_options($question);
 }
 
@@ -1393,21 +1404,11 @@ function question_require_capability_on($question, $cap) {
  * Get the real state - the correct question id and answer - for a random
  * question.
  * @param object $state with property answer.
- * @return mixed return integer real question id or false if there was an
- * error..
+ * @deprecated this function has not been relevant since Moodle 2.1!
  */
 function question_get_real_state($state) {
-    global $OUTPUT;
-    $realstate = clone($state);
-    $matches = array();
-    if (!preg_match('|^random([0-9]+)-(.*)|', $state->answer, $matches)) {
-        echo $OUTPUT->notification(get_string('errorrandom', 'quiz_statistics'));
-        return false;
-    } else {
-        $realstate->question = $matches[1];
-        $realstate->answer = $matches[2];
-        return $realstate;
-    }
+    throw new coding_exception('question_get_real_state has not been relevant since Moodle 2.1. ' .
+            'I am not sure what you are trying to do, but stop it at once!');
 }
 
 /**

question/editlib.php

@@ -1521,6 +1521,10 @@ public function process_actions() {
         if(($unhide = optional_param('unhide', '', PARAM_INT)) and confirm_sesskey()) {
             question_require_capability_on($unhide, 'edit');
             $DB->set_field('question', 'hidden', 0, array('id' => $unhide));
+
+            // Purge these questions from the cache.
+            question_bank::notify_question_edited($unhide);
+
             redirect($this->baseurl);
         }
     }

question/engine/bank.php

@@ -51,8 +51,6 @@ abstract class question_bank {
     /** @var array question type name => 1. Records which question definitions have been loaded. */
     private static $loadedqdefs = array();
 
-    protected static $questionfinder = null;
-
     /** @var boolean nasty hack to allow unit tests to call {@link load_question()}. */
     private static $testmode = false;
     private static $testdata = array();
@@ -240,6 +238,23 @@ public static function load_question_definition_classes($qtypename) {
         self::$loadedqdefs[$qtypename] = 1;
     }
 
+    /**
+     * This method needs to be called whenever a question is edited.
+     */
+    public static function notify_question_edited($questionid) {
+        question_finder::get_instance()->uncache_question($questionid);
+    }
+
+    /**
+     * Load a question definition data from the database. The data will be
+     * returned as a plain stdClass object.
+     * @param int $questionid the id of the question to load.
+     * @return object question definition loaded from the database.
+     */
+    public static function load_question_data($questionid) {
+        return question_finder::get_instance()->load_question_data($questionid);
+    }
+
     /**
      * Load a question definition from the database. The object returned
      * will actually be of an appropriate {@link question_definition} subclass.
@@ -256,12 +271,8 @@ public static function load_question($questionid, $allowshuffle = true) {
             return self::return_test_question_data($questionid);
         }
 
-        $questiondata = $DB->get_record_sql('
-                SELECT q.*, qc.contextid
-                FROM {question} q
-                JOIN {question_categories} qc ON q.category = qc.id
-                WHERE q.id = :id', array('id' => $questionid), MUST_EXIST);
-        get_question_options($questiondata);
+        $questiondata = self::load_question_data($questionid);
+
         if (!$allowshuffle) {
             $questiondata->options->shuffleanswers = false;
         }
@@ -282,6 +293,7 @@ public static function make_question($questiondata) {
      * @return question_finder a question finder.
      */
     public static function get_finder() {
+        return question_finder::get_instance();
         if (is_null(self::$questionfinder)) {
             self::$questionfinder = new question_finder();
         }
@@ -418,7 +430,55 @@ public static function cron() {
  * @copyright  2009 The Open University
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class question_finder {
+class question_finder implements cache_data_source {
+    /** @var question_finder the singleton instance of this class. */
+    protected static $questionfinder = null;
+
+    /** @var cache the question definition cache. */
+    protected $cache = null;
+
+    /**
+     * @return question_finder a question finder.
+     */
+    public static function get_instance() {
+        if (is_null(self::$questionfinder)) {
+            self::$questionfinder = new question_finder();
+        }
+        return self::$questionfinder;
+    }
+
+    /* See cache_data_source::get_instance_for_cache. */
+    public static function get_instance_for_cache(cache_definition $definition) {
+        return self::get_instance();
+    }
+
+    /**
+     * @return get the question definition cache we are using.
+     */
+    protected function get_data_cache() {
+        if ($this->cache == null) {
+            $this->cache = cache::make('core', 'questiondata');
+        }
+        return $this->cache;
+    }
+
+    /**
+     * This method needs to be called whenever a question is edited.
+     */
+    public function uncache_question($questionid) {
+        $this->get_data_cache()->delete($questionid);
+    }
+
+    /**
+     * Load a question definition data from the database. The data will be
+     * returned as a plain stdClass object.
+     * @param int $questionid the id of the question to load.
+     * @return object question definition loaded from the database.
+     */
+    public function load_question_data($questionid) {
+        return $this->get_data_cache()->get($questionid);
+    }
+
     /**
      * Get the ids of all the questions in a list of categoryies.
      * @param array $categoryids either a categoryid, or a comma-separated list
@@ -444,4 +504,35 @@ public function get_questions_from_categories($categoryids, $extraconditions,
                  AND hidden = 0
                  $extraconditions", $qcparams + $extraparams, '', 'id,id AS id2');
     }
+
+    /* See cache_data_source::load_for_cache. */
+    public function load_for_cache($questionid) {
+        global $DB;
+        $questiondata = $DB->get_record_sql('
+                                    SELECT q.*, qc.contextid
+                                    FROM {question} q
+                                    JOIN {question_categories} qc ON q.category = qc.id
+                                    WHERE q.id = :id', array('id' => $questionid), MUST_EXIST);
+        get_question_options($questiondata);
+        return $questiondata;
+    }
+
+    /* See cache_data_source::load_many_for_cache. */
+    public function load_many_for_cache(array $questionids) {
+        global $DB;
+        list($idcondition, $params) = $DB->get_in_or_equal($questionids);
+        $questiondata = $DB->get_records_sql('
+                                            SELECT q.*, qc.contextid
+                                            FROM {question} q
+                                            JOIN {question_categories} qc ON q.category = qc.id
+                                            WHERE q.id ' . $idcondition, $params);
+
+        foreach ($questionids as $id) {
+            if (!array_key_exists($id, $questionids)) {
+                throw new dml_missing_record_exception('question', '', array('id' => $id));
+            }
+            get_question_options($questiondata[$id]);
+        }
+        return $questiondata;
+    }
 }

question/question.php

@@ -281,6 +281,9 @@
         }
     }
 
+    // Purge this question from the cache.
+    question_bank::notify_question_edited($question->id);
+
     if (($qtypeobj->finished_edit_wizard($fromform)) || $movecontext) {
         if ($inpopup) {
             echo $OUTPUT->notification(get_string('changessaved'), '');