MDL-40618 backup: centralize info field, use info in queries.

encode_backup_temp_info() and decode_backup_temp_info() have been
introduced to keep the info field encoding in one place.
Many locations used get_backup_ids_record() to obtain info, that
makes lots of calls to get_backup_ids_record() which can be slow.
We now complete all those inline by adding the info field to the query.
To reduce memory usage, all queries of that nature have been changed
to use get_recordset_*. gzcompress was introduced if available to minimize
traffic to/from the database and to decrease the memory required for caching.
The compression time is saved by the benefits in other places of having smaller data.

diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php
index ed36b0a..2b6cdab 100644 (file)
--- a/backup/moodle2/restore_stepslib.php
+++ b/backup/moodle2/restore_stepslib.php
@@ -510,11 +510,11 @@ class restore_review_pending_block_positions extends restore_execution_step {
 
         // Get all the block_position objects pending to match
         $params = array('backupid' => $this->get_restoreid(), 'itemname' => 'block_position');
-        $rs = $DB->get_recordset('backup_ids_temp', $params, '', 'itemid');
+        $rs = $DB->get_recordset('backup_ids_temp', $params, '', 'itemid, info');
         // Process block positions, creating them or accumulating for final step
         foreach($rs as $posrec) {
-            // Get the complete position object (stored as info)
-            $position = restore_dbops::get_backup_ids_record($this->get_restoreid(), 'block_position', $posrec->itemid)->info;
+            // Get the complete position object out of the info field.
+            $position = backup_controller_dbops::decode_backup_temp_info($posrec->info);
             // If position is for one already mapped (known) contextid
             // process it now, creating the position, else nothing to
             // do, position finally discarded
@@ -546,12 +546,12 @@ class restore_process_course_modules_availability extends restore_execution_step
 
         // Get all the module_availability objects to process
         $params = array('backupid' => $this->get_restoreid(), 'itemname' => 'module_availability');
-        $rs = $DB->get_recordset('backup_ids_temp', $params, '', 'itemid');
+        $rs = $DB->get_recordset('backup_ids_temp', $params, '', 'itemid, info');
         // Process availabilities, creating them if everything matches ok
         foreach($rs as $availrec) {
             $allmatchesok = true;
             // Get the complete availabilityobject
-            $availability = restore_dbops::get_backup_ids_record($this->get_restoreid(), 'module_availability', $availrec->itemid)->info;
+            $availability = backup_controller_dbops::decode_backup_temp_info($availrec->info);
             // Map the sourcecmid if needed and possible
             if (!empty($availability->sourcecmid)) {
                 $newcm = restore_dbops::get_backup_ids_record($this->get_restoreid(), 'course_module', $availability->sourcecmid);
@@ -3624,7 +3624,7 @@ class restore_process_file_aliases_queue extends restore_execution_step {
 
         // Iterate over aliases in the queue.
         foreach ($rs as $record) {
-            $info = unserialize(base64_decode($record->info));
+            $info = restore_dbops::decode_backup_temp_info($record->info);
 
             // Try to pick a repository instance that should serve the alias.
             $repository = $this->choose_repository($info);
@@ -3657,7 +3657,7 @@ class restore_process_file_aliases_queue extends restore_execution_step {
                 $source = null;
 
                 foreach ($candidates as $candidate) {
-                    $candidateinfo = unserialize(base64_decode($candidate->info));
+                    $candidateinfo = backup_controller_dbops::decode_backup_temp_info($candidate->info);
                     if ($candidateinfo->filename === $reference['filename']
                             and $candidateinfo->filepath === $reference['filepath']
                             and !is_null($candidate->newcontextid)

diff --git a/backup/upgrade.txt b/backup/upgrade.txt
index 586ee70..26e2e00 100644 (file)
--- a/backup/upgrade.txt
+++ b/backup/upgrade.txt
@@ -7,6 +7,12 @@ information provided here is intended especially for developers.
   method is not available anymore. Temp tables must be created
   inline always.
 
+* Using the info field from backup_ids_temp or backup_files_temp
+  must now go via backup_controller_dbops::decode_backup_temp_info() and
+  backup_controller_dbops::encode_backup_temp_info(). The implementation
+  of the encoding has changed.  These new functions encapsulate any future
+  changes to the encoding.
+
 === 2.5 ===
 
 * New optional param $sortby in backup set_source_table() allows to

diff --git a/backup/util/dbops/backup_controller_dbops.class.php b/backup/util/dbops/backup_controller_dbops.class.php
index 085fe22..668ff61 100644 (file)
--- a/backup/util/dbops/backup_controller_dbops.class.php
+++ b/backup/util/dbops/backup_controller_dbops.class.php
@@ -154,6 +154,44 @@ abstract class backup_controller_dbops extends backup_dbops {
         $dbman->drop_table($table); // And drop it
     }
 
+    /**
+     * Decode the info field from backup_ids_temp or backup_files_temp.
+     *
+     * @param mixed $info The info field data to decode, may be an object or a simple integer.
+     * @return mixed The decoded information.  For simple types it returns, for complex ones we decode.
+     */
+    public static function decode_backup_temp_info($info) {
+        // We encode all data except null.
+        if ($info != null) {
+            if (extension_loaded('zlib')) {
+                return unserialize(gzuncompress(base64_decode($info)));
+            } else {
+                return unserialize(base64_decode($info));
+            }
+        }
+        return $info;
+    }
+
+    /**
+     * Encode the info field for backup_ids_temp or backup_files_temp.
+     *
+     * @param mixed $info string The info field data to encode.
+     * @return string An encoded string of data or null if the input is null.
+     */
+    public static function encode_backup_temp_info($info) {
+        // We encode if there is any information to keep the translations simpler.
+        if ($info != null) {
+            // We compress if possible. It reduces db, network and memory storage. The saving is greater than CPU compression cost.
+            // Compression level 1 is chosen has it produces good compression with the smallest possible overhead, see MDL-40618.
+            if (extension_loaded('zlib')) {
+                return base64_encode(gzcompress(serialize($info), 1));
+            } else {
+                return base64_encode(serialize($info));
+            }
+        }
+        return $info;
+    }
+
     /**
      * Given one type and id from controller, return the corresponding courseid
      */

diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php
index e7a2f91..80c7d24 100644 (file)
--- a/backup/util/dbops/restore_dbops.class.php
+++ b/backup/util/dbops/restore_dbops.class.php
@@ -152,7 +152,7 @@ abstract class restore_dbops {
         $problems = array(); // To store warnings/errors
 
         // Get loaded roles from backup_ids
-        $rs = $DB->get_recordset('backup_ids_temp', array('backupid' => $restoreid, 'itemname' => 'role'), '', 'itemid');
+        $rs = $DB->get_recordset('backup_ids_temp', array('backupid' => $restoreid, 'itemname' => 'role'), '', 'itemid, info');
         foreach ($rs as $recrole) {
             // If the rolemappings->modified flag is set, that means that we are coming from
             // manually modified mappings (by UI), so accept those mappings an put them to backup_ids
@@ -163,14 +163,13 @@ abstract class restore_dbops {
             // Else, we haven't any info coming from UI, let's calculate the mappings, matching
             // in multiple ways and checking permissions. Note mapping to 0 means "skip"
             } else {
-                $role = (object)self::get_backup_ids_record($restoreid, 'role', $recrole->itemid)->info;
+                $role = (object)backup_controller_dbops::decode_backup_temp_info($recrole->info);
                 $match = self::get_best_assignable_role($role, $courseid, $userid, $samesite);
                 // Send match to backup_ids
                 self::set_backup_ids_record($restoreid, 'role', $recrole->itemid, $match);
                 // Build the rolemappings element for controller
                 unset($role->id);
                 unset($role->nameincourse);
-                unset($role->nameincourse);
                 $role->targetroleid = $match;
                 $rolemappings->mappings[$recrole->itemid] = $role;
                 // Prepare warning if no match found
@@ -666,20 +665,21 @@ abstract class restore_dbops {
         global $DB;
 
         $results = array();
-        $qcats = $DB->get_records_sql("SELECT itemid, parentitemid AS contextid
+        $qcats = $DB->get_recordset_sql("SELECT itemid, parentitemid AS contextid, info
                                          FROM {backup_ids_temp}
                                        WHERE backupid = ?
                                          AND itemname = 'question_category'", array($restoreid));
         foreach ($qcats as $qcat) {
             // If this qcat context haven't been acummulated yet, do that
             if (!isset($results[$qcat->contextid])) {
-                $temprec = self::get_backup_ids_record($restoreid, 'question_category', $qcat->itemid);
+                $info = backup_controller_dbops::decode_backup_temp_info($qcat->info);
                 // Filter by contextlevel if necessary
-                if (is_null($contextlevel) || $contextlevel == $temprec->info->contextlevel) {
-                    $results[$qcat->contextid] = $temprec->info->contextlevel;
+                if (is_null($contextlevel) || $contextlevel == $info->contextlevel) {
+                    $results[$qcat->contextid] = $info->contextlevel;
                 }
             }
         }
+        $qcats->close();
         // Sort by value (contextlevel from CONTEXT_SYSTEM downto CONTEXT_MODULE)
         asort($results);
         return $results;
@@ -693,15 +693,16 @@ abstract class restore_dbops {
         global $DB;
 
         $results = array();
-        $qcats = $DB->get_records_sql("SELECT itemid
+        $qcats = $DB->get_recordset_sql("SELECT itemid, info
                                          FROM {backup_ids_temp}
                                         WHERE backupid = ?
                                           AND itemname = 'question_category'
                                           AND parentitemid = ?", array($restoreid, $contextid));
         foreach ($qcats as $qcat) {
-            $temprec = self::get_backup_ids_record($restoreid, 'question_category', $qcat->itemid);
-            $results[$qcat->itemid] = $temprec->info;
+            $results[$qcat->itemid] = backup_controller_dbops::decode_backup_temp_info($qcat->info);
         }
+        $qcats->close();
+
         return $results;
     }
 
@@ -791,15 +792,15 @@ abstract class restore_dbops {
         global $DB;
 
         $results = array();
-        $qs = $DB->get_records_sql("SELECT itemid
+        $qs = $DB->get_recordset_sql("SELECT itemid, info
                                       FROM {backup_ids_temp}
                                      WHERE backupid = ?
                                        AND itemname = 'question'
                                        AND parentitemid = ?", array($restoreid, $qcatid));
         foreach ($qs as $q) {
-            $temprec = self::get_backup_ids_record($restoreid, 'question', $q->itemid);
-            $results[$q->itemid] = $temprec->info;
+            $results[$q->itemid] = backup_controller_dbops::decode_backup_temp_info($q->info);
         }
+        $qs->close();
         return $results;
     }
 
@@ -886,7 +887,7 @@ abstract class restore_dbops {
         $basepath = $basepath . '/files/';// Get backup file pool base
         $rs = $DB->get_recordset_sql($sql, $params);
         foreach ($rs as $rec) {
-            $file = (object)unserialize(base64_decode($rec->info));
+            $file = (object)backup_controller_dbops::decode_backup_temp_info($rec->info);
 
             // ignore root dirs (they are created automatically)
             if ($file->filepath == '/' && $file->filename == '.') {
@@ -1011,9 +1012,9 @@ abstract class restore_dbops {
         $themes    = get_list_of_themes(); // Get themes for quick search later
 
         // Iterate over all the included users with newitemid = 0, have to create them
-        $rs = $DB->get_recordset('backup_ids_temp', array('backupid' => $restoreid, 'itemname' => 'user', 'newitemid' => 0), '', 'itemid, parentitemid');
+        $rs = $DB->get_recordset('backup_ids_temp', array('backupid' => $restoreid, 'itemname' => 'user', 'newitemid' => 0), '', 'itemid, parentitemid, info');
         foreach ($rs as $recuser) {
-            $user = (object)self::get_backup_ids_record($restoreid, 'user', $recuser->itemid)->info;
+            $user = (object)backup_controller_dbops::decode_backup_temp_info($recuser->info);
 
             // if user lang doesn't exist here, use site default
             if (!array_key_exists($user->lang, $languages)) {
@@ -1402,9 +1403,9 @@ abstract class restore_dbops {
         }
 
         // Iterate over all the included users
-        $rs = $DB->get_recordset('backup_ids_temp', array('backupid' => $restoreid, 'itemname' => 'user'), '', 'itemid');
+        $rs = $DB->get_recordset('backup_ids_temp', array('backupid' => $restoreid, 'itemname' => 'user'), '', 'itemid, info');
         foreach ($rs as $recuser) {
-            $user = (object)self::get_backup_ids_record($restoreid, 'user', $recuser->itemid)->info;
+            $user = (object)backup_controller_dbops::decode_backup_temp_info($recuser->info);
 
             // Find the correct mnethostid for user before performing any further check
             if (empty($user->mnethosturl) || $user->mnethosturl === $CFG->wwwroot) {
@@ -1489,7 +1490,7 @@ abstract class restore_dbops {
         global $DB;
 
         // Store external files info in `info` field
-        $filerec->info     = base64_encode(serialize($filerec)); // Serialize the whole rec in info
+        $filerec->info     = backup_controller_dbops::encode_backup_temp_info($filerec); // Encode the whole record into info.
         $filerec->backupid = $restoreid;
         $DB->insert_record('backup_files_temp', $filerec);
     }
@@ -1504,7 +1505,7 @@ abstract class restore_dbops {
             $extrarecord['parentitemid'] = $parentitemid;
         }
         if ($info != null) {
-            $extrarecord['info'] = base64_encode(serialize($info));
+            $extrarecord['info'] = backup_controller_dbops::encode_backup_temp_info($info);
         }
 
         self::set_backup_ids_cached($restoreid, $itemname, $itemid, $extrarecord);
@@ -1513,8 +1514,9 @@ abstract class restore_dbops {
     public static function get_backup_ids_record($restoreid, $itemname, $itemid) {
         $dbrec = self::get_backup_ids_cached($restoreid, $itemname, $itemid);
 
+        // We must test if info is a string, as the cache stores info in object form.
         if ($dbrec && isset($dbrec->info) && is_string($dbrec->info)) {
-            $dbrec->info = unserialize(base64_decode($dbrec->info));
+            $dbrec->info = backup_controller_dbops::decode_backup_temp_info($dbrec->info);
         }
 
         return $dbrec;
@@ -1559,18 +1561,17 @@ abstract class restore_dbops {
         // Get the course context
         $coursectx = context_course::instance($courseid);
         // Get all the mapped roles we have
-        $rs = $DB->get_recordset('backup_ids_temp', array('backupid' => $restoreid, 'itemname' => 'role'), '', 'itemid');
+        $rs = $DB->get_recordset('backup_ids_temp', array('backupid' => $restoreid, 'itemname' => 'role'), '', 'itemid, info, newitemid');
         foreach ($rs as $recrole) {
-            // Get the complete temp_ids record
-            $role = (object)self::get_backup_ids_record($restoreid, 'role', $recrole->itemid);
+            $info = backup_controller_dbops::decode_backup_temp_info($recrole->info);
             // If it's one mapped role and we have one name for it
-            if (!empty($role->newitemid) && !empty($role->info['nameincourse'])) {
+            if (!empty($recrole->newitemid) && !empty($info['nameincourse'])) {
                 // If role name doesn't exist, add it
                 $rolename = new stdclass();
-                $rolename->roleid = $role->newitemid;
+                $rolename->roleid = $recrole->newitemid;
                 $rolename->contextid = $coursectx->id;
                 if (!$DB->record_exists('role_names', (array)$rolename)) {
-                    $rolename->name = $role->info['nameincourse'];
+                    $rolename->name = $info['nameincourse'];
                     $DB->insert_record('role_names', $rolename);
                 }
             }

diff --git a/backup/util/dbops/tests/backup_dbops_test.php b/backup/util/dbops/tests/backup_dbops_test.php
index 55d57ba..e57dc7e 100644 (file)
--- a/backup/util/dbops/tests/backup_dbops_test.php
+++ b/backup/util/dbops/tests/backup_dbops_test.php
@@ -147,6 +147,18 @@ class backup_dbops_testcase extends advanced_testcase {
         // Drop and check it doesn't exists anymore
         backup_controller_dbops::drop_backup_ids_temp_table('testingid');
         $this->assertFalse($dbman->table_exists('backup_ids_temp'));
+
+        // Test encoding/decoding of backup_ids_temp,backup_files_temp encode/decode functions.
+        // We need to handle both objects and data elements.
+        $object = new stdClass();
+        $object->item1 = 10;
+        $object->item2 = 'a String';
+        $testarray = array($object, 10, null, 'string', array('a' => 'b', 1 => 1));
+        foreach ($testarray as $item) {
+            $encoded = backup_controller_dbops::encode_backup_temp_info($item);
+            $decoded = backup_controller_dbops::decode_backup_temp_info($encoded);
+            $this->assertEquals($item, $decoded);
+        }
     }
 
     /**