MDL-54864 core_question: Enforce context,stamp uniqueness for category

Added an upgrade step to remove duplicate stamps, added a unique index
on (contextid, stamp) and modified the question_category edit code such
that stamp duplication in the same context is not permitted.

lib/db/install.xml

@@ -1218,6 +1218,7 @@
       </KEYS>
       <INDEXES>
         <INDEX NAME="contextid" UNIQUE="false" FIELDS="contextid" COMMENT="links to context table"/>
+        <INDEX NAME="contextidstamp" UNIQUE="true" FIELDS="contextid, stamp"/>
       </INDEXES>
     </TABLE>
     <TABLE NAME="question" COMMENT="The questions themselves">

lib/db/upgrade.php

@@ -2083,5 +2083,58 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2016081700.00);
     }
 
+    if ($oldversion < 2016081700.02) {
+        // An upgrade step to remove any duplicate stamps, within the same context, in the question_categories table, and to
+        // add a unique index to (contextid, stamp) to avoid future stamp duplication. See MDL-54864.
+
+        // Extend the execution time limit of the script to 2 hours.
+        upgrade_set_timeout(7200);
+
+        // This SQL fetches the id of those records which have duplicate stamps within the same context.
+        // This doesn't return the original record within the context, from which the duplicate stamps were likely created.
+        $fromclause = "FROM (
+                        SELECT min(id) AS minid, contextid, stamp
+                            FROM {question_categories}
+                            GROUP BY contextid, stamp
+                        ) minid
+                        JOIN {question_categories} qc
+                            ON qc.contextid = minid.contextid AND qc.stamp = minid.stamp AND qc.id > minid.minid";
+
+        // Get the total record count - used for the progress bar.
+        $countduplicatessql = "SELECT count(qc.id) $fromclause";
+        $total = $DB->count_records_sql($countduplicatessql);
+
+        // Get the records themselves.
+        $getduplicatessql = "SELECT qc.id $fromclause ORDER BY minid";
+        $rs = $DB->get_recordset_sql($getduplicatessql);
+
+        // For each duplicate, update the stamp to a new random value.
+        $i = 0;
+        $pbar = new progress_bar('updatequestioncategorystamp', 500, true);
+        foreach ($rs as $record) {
+            // Generate a new, unique stamp and update the record.
+            do {
+                $newstamp = make_unique_id_code();
+            } while (isset($usedstamps[$newstamp]));
+            $usedstamps[$newstamp] = 1;
+            $DB->set_field('question_categories', 'stamp', $newstamp, array('id' => $record->id));
+
+            // Update progress.
+            $i++;
+            $pbar->update($i, $total, "Updating duplicate question category stamp - $i/$total.");
+        }
+        unset($usedstamps);
+
+        // The uniqueness of each (contextid, stamp) pair is now guaranteed, so add the unique index to stop future duplicates.
+        $table = new xmldb_table('question_categories');
+        $index = new xmldb_index('contextidstamp', XMLDB_INDEX_UNIQUE, array('contextid', 'stamp'));
+        if (!$dbman->index_exists($table, $index)) {
+            $dbman->add_index($table, $index);
+        }
+
+        // Savepoint reached.
+        upgrade_main_savepoint(true, 2016081700.02);
+    }
+
     return true;
 }

question/category_class.php

@@ -454,10 +454,16 @@ class question_category_object {
         $fromcontext = context::instance_by_id($oldcat->contextid);
         require_capability('moodle/question:managecategory', $fromcontext);
 
-        // If moving to another context, check permissions some more.
+        // If moving to another context, check permissions some more, and confirm contextid,stamp uniqueness.
+        $newstamprequired = false;
         if ($oldcat->contextid != $tocontextid) {
             $tocontext = context::instance_by_id($tocontextid);
             require_capability('moodle/question:managecategory', $tocontext);
+
+            // Confirm stamp uniqueness in the new context. If the stamp already exists, generate a new one.
+            if ($DB->record_exists('question_categories', array('contextid' => $tocontextid, 'stamp' => $oldcat->stamp))) {
+                $newstamprequired = true;
+            }
         }
 
         // Update the category record.
@@ -468,6 +474,9 @@ class question_category_object {
         $cat->infoformat = $newinfoformat;
         $cat->parent = $parentid;
         $cat->contextid = $tocontextid;
+        if ($newstamprequired) {
+            $cat->stamp = make_unique_id_code();
+        }
         $DB->update_record('question_categories', $cat);
 
         // If the category name has changed, rename any random questions in that category.

question/tests/generator/lib.php

@@ -50,7 +50,7 @@ class core_question_generator extends component_generator_base {
             'contextid'  => context_system::instance()->id,
             'info'       => '',
             'infoformat' => FORMAT_HTML,
-            'stamp'      => '',
+            'stamp'      => make_unique_id_code(),
             'parent'     => 0,
             'sortorder'  => 999,
         );

version.php

@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2016081700.01;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2016081700.02;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.