From f2bfe26aea52b918df5147d7b2b911b0eca12f59 Mon Sep 17 00:00:00 2001 From: Rajesh Taneja Date: Tue, 17 Apr 2012 11:32:15 +0800 Subject: [PATCH] MDL-27120 backup: fixed docblock and code cleanup --- .../moodle2/backup_xml_transformer.class.php | 33 ++-- backup/util/dbops/restore_dbops.class.php | 163 ++++++++++++++---- backup/util/plan/base_plan.class.php | 14 +- backup/util/ui/backup_ui_stage.class.php | 8 +- backup/util/ui/base_moodleform.class.php | 22 +-- .../xml/parser/progressive_parser.class.php | 10 +- 6 files changed, 166 insertions(+), 84 deletions(-) diff --git a/backup/moodle2/backup_xml_transformer.class.php b/backup/moodle2/backup_xml_transformer.class.php index 7c3cd5515d7..25c8503bcf7 100644 --- a/backup/moodle2/backup_xml_transformer.class.php +++ b/backup/moodle2/backup_xml_transformer.class.php @@ -27,17 +27,18 @@ defined('MOODLE_INTERNAL') || die(); +// Cache for storing link encoders, so that we don't need to call +// register_link_encoders each time backup_xml_transformer is constructed +// TODO MDL-25290 replace global with MUC code. +global $LINKS_ENCODERS_CACHE; + +$LINKS_ENCODERS_CACHE = array(); + /** * Class implementing the @xml_contenttrasnformed logic to be applied in moodle2 backups * * TODO: Finish phpdocs */ - -// cache for storing link encoders, so that we don't need to call -// register_link_encoders each time backup_xml_transformer is constructed -global $LINKS_ENCODERS_CACHE; -$LINKS_ENCODERS_CACHE = array(); - class backup_xml_transformer extends xml_contenttransformer { private $absolute_links_encoders; // array of static methods to be called in order to @@ -47,18 +48,13 @@ class backup_xml_transformer extends xml_contenttransformer { private $unicoderegexp; // to know if the site supports unicode regexp public function __construct($courseid) { - global $LINKS_ENCODERS_CACHE; - $this->absolute_links_encoders = array(); $this->courseid = $courseid; // Check if we support unicode modifiers in regular expressions $this->unicoderegexp = @preg_match('/\pL/u', 'a'); // This will fail silently, returning false, // if regexp libraries don't support unicode // Register all the available content link encoders - if (empty($LINKS_ENCODERS_CACHE)) { - $LINKS_ENCODERS_CACHE = $this->register_link_encoders(); - } - $this->absolute_links_encoders = $LINKS_ENCODERS_CACHE; + $this->absolute_links_encoders = $this->register_link_encoders(); } public function process($content) { @@ -142,7 +138,19 @@ class backup_xml_transformer extends xml_contenttransformer { return $result; } + /** + * Register all available content link encoders + * + * @return array encoder + * @todo MDL-25290 replace LINKS_ENCODERS_CACHE global with MUC code + */ private function register_link_encoders() { + global $LINKS_ENCODERS_CACHE; + // If encoder is linked, then return cached encoder. + if (!empty($LINKS_ENCODERS_CACHE)) { + return $LINKS_ENCODERS_CACHE; + } + $encoders = array(); // Add the course encoder @@ -171,6 +179,7 @@ class backup_xml_transformer extends xml_contenttransformer { // Add local encodes // TODO: Any interest? 1.9 never had that. + $LINKS_ENCODERS_CACHE = $encoders; return $encoders; } } diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php index c6319a4551c..08c593ebc6a 100644 --- a/backup/util/dbops/restore_dbops.class.php +++ b/backup/util/dbops/restore_dbops.class.php @@ -28,8 +28,38 @@ * TODO: Finish phpdocs */ abstract class restore_dbops { - static $BACKUP_IDS_CACHE = array(); - static $BACKUP_IDS_EXIST = array(); + /** + * Keep cache of backup records. + * @var array + * @todo MDL-25290 static should be replaced with MUC code. + */ + private static $backupidscache = array(); + /** + * Keep track of backup ids which are cached. + * @var array + * @todo MDL-25290 static should be replaced with MUC code. + */ + private static $backupidsexist = array(); + /** + * Count is expensive, so manually keeping track of + * backupidscache, to avoid memory issues. + * @var int + * @todo MDL-25290 static should be replaced with MUC code. + */ + private static $backupidscachesize = 2048; + /** + * Count is expensive, so manually keeping track of + * backupidsexist, to avoid memory issues. + * @var int + * @todo MDL-25290 static should be replaced with MUC code. + */ + private static $backupidsexistsize = 10240; + /** + * Slice backupids cache to add more data. + * @var int + * @todo MDL-25290 static should be replaced with MUC code. + */ + private static $backupidsslice = 512; /** * Return one array containing all the tasks that have been included @@ -153,61 +183,130 @@ abstract class restore_dbops { return $problems; } + /** + * Return cached backup id's + * + * @param int $restoreid id of backup + * @param string $itemname name of the item + * @param int $itemid id of item + * @return array backup id's + * @todo MDL-25290 replace static backupids* with MUC code + */ protected static function get_backup_ids_cached($restoreid, $itemname, $itemid) { global $DB; $key = "$itemid $itemname $restoreid"; - if (!isset(self::$BACKUP_IDS_EXIST[$key])) { - return false; + // If record exists in cache then return. + if (isset(self::$backupidsexist[$key]) && isset(self::$backupidscache[$key])) { + return self::$backupidscache[$key]; } - if (isset(self::$BACKUP_IDS_CACHE[$key])) { - return self::$BACKUP_IDS_CACHE[$key]; + // Clean cache, if it's full. + if (self::$backupidscachesize <= 0) { + // Remove some records, to keep memory in limit. + self::$backupidscache = array_slice(self::$backupidscache, self::$backupidsslice, null, true); + self::$backupidscachesize = self::$backupidscachesize + self::$backupidsslice; + } + if (self::$backupidsexistsize <= 0) { + self::$backupidsexist = array_slice(self::$backupidsexist, self::$backupidsslice, null, true); + self::$backupidsexistsize = self::$backupidsexistsize + self::$backupidsslice; } + // Retrive record from database. $record = array( 'backupid' => $restoreid, 'itemname' => $itemname, 'itemid' => $itemid ); - - if (count(self::$BACKUP_IDS_CACHE) > 2048) { - $BACKUP_IDS_CACHE = array(); + if ($dbrec = $DB->get_record('backup_ids_temp', $record)) { + self::$backupidsexist[$key] = $dbrec->id; + self::$backupidscache[$key] = $dbrec; + self::$backupidscachesize--; + self::$backupidsexistsize--; + return self::$backupidscache[$key]; + } else { + return false; } - - return self::$BACKUP_IDS_CACHE[$key] = $DB->get_record('backup_ids_temp', $record); } + /** + * Cache backup ids' + * + * @param int $restoreid id of backup + * @param string $itemname name of the item + * @param int $itemid id of item + * @param array $extrarecord extra record which needs to be updated + * @return void + * @todo MDL-25290 replace static BACKUP_IDS_* with MUC code + */ protected static function set_backup_ids_cached($restoreid, $itemname, $itemid, $extrarecord) { global $DB; $key = "$itemid $itemname $restoreid"; - if (!isset(self::$BACKUP_IDS_EXIST[$key])) { - $record = array( - 'backupid' => $restoreid, - 'itemname' => $itemname, - 'itemid' => $itemid, - 'newitemid' => 0, - 'parentitemid' => null, - 'info' => null - ); - $record = array_merge($record, $extrarecord); - $record['id'] = $DB->insert_record_raw('backup_ids_temp', $record); - self::$BACKUP_IDS_EXIST[$key] = $record['id']; - if (count(self::$BACKUP_IDS_CACHE) < 2048) { - // cache new records if we haven't got many yet - self::$BACKUP_IDS_CACHE[$key] = (object) $record; + $record = array( + 'backupid' => $restoreid, + 'itemname' => $itemname, + 'itemid' => $itemid, + ); + + // If record is not cached then add one. + if (!isset(self::$backupidsexist[$key])) { + // If we have this record in db, then just update this. + if ($existingrecord = $DB->get_record('backup_ids_temp', $record)) { + self::$backupidsexist[$key] = $existingrecord->id; + self::$backupidsexistsize--; + self::update_backup_cached_record($record, $extrarecord, $key, $existingrecord); + } else { + // Add new record to cache and db. + $recorddefault = array ( + 'newitemid' => 0, + 'parentitemid' => null, + 'info' => null); + $record = array_merge($record, $recorddefault, $extrarecord); + $record['id'] = $DB->insert_record('backup_ids_temp', $record); + self::$backupidsexist[$key] = $record['id']; + self::$backupidsexistsize--; + if (self::$backupidscachesize > 0) { + // Cache new records if we haven't got many yet. + self::$backupidscache[$key] = (object) $record; + self::$backupidscachesize--; + } } } else { - if (!empty($extrarecord)) { - $extrarecord['id'] = self::$BACKUP_IDS_EXIST[$key]; - $DB->update_record('backup_ids_temp', $extrarecord); - if (isset(self::$BACKUP_IDS_CACHE[$key])) { - $record = array_merge((array)self::$BACKUP_IDS_CACHE[$key], $extrarecord); - self::$BACKUP_IDS_CACHE[$key] = (object) $record; + self::update_backup_cached_record($record, $extrarecord, $key); + } + } + + /** + * Updates existing backup record + * + * @param array $record record which needs to be updated + * @param array $extrarecord extra record which needs to be updated + * @param string $key unique key which is used to identify cached record + * @param stdClass $existingrecord (optional) existing record + */ + protected static function update_backup_cached_record($record, $extrarecord, $key, $existingrecord = null) { + global $DB; + // Update only if extrarecord is not empty. + if (!empty($extrarecord)) { + $extrarecord['id'] = self::$backupidsexist[$key]; + $DB->update_record('backup_ids_temp', $extrarecord); + // Update existing cache or add new record to cache. + if (isset(self::$backupidscache[$key])) { + $record = array_merge((array)self::$backupidscache[$key], $extrarecord); + self::$backupidscache[$key] = (object) $record; + } else if (self::$backupidscachesize > 0) { + if ($existingrecord) { + self::$backupidscache[$key] = $existingrecord; + } else { + // Retrive record from database and cache updated records. + self::$backupidscache[$key] = $DB->get_record('backup_ids_temp', $record); } + $record = array_merge((array)self::$backupidscache[$key], $extrarecord); + self::$backupidscache[$key] = (object) $record; + self::$backupidscachesize--; } } } diff --git a/backup/util/plan/base_plan.class.php b/backup/util/plan/base_plan.class.php index 24becc6969b..eab1869d6d3 100644 --- a/backup/util/plan/base_plan.class.php +++ b/backup/util/plan/base_plan.class.php @@ -62,7 +62,7 @@ abstract class base_plan implements checksumable, executable { foreach ($task->get_settings() as $key => $setting) { if (!in_array($setting, $this->settings)) { $name = $setting->get_name(); - if(!isset($this->settings[$name])) { + if (!isset($this->settings[$name])) { $this->settings[$name] = $setting; } else { throw new base_plan_exception('multiple_settings_by_name_found', $name); @@ -89,17 +89,17 @@ abstract class base_plan implements checksumable, executable { /** * return one setting by name, useful to request root/course settings - * that are, by definition, unique by name. Throws exception if multiple - * are found + * that are, by definition, unique by name. * - * TODO: Change this to string indexed array for quicker lookup. Not critical + * @param string $name name of the setting + * @throws base_plan_exception if setting name is not found. */ public function get_setting($name) { $result = null; - if(isset($this->settings[$name])) { - $result = $this->settings[$name]; + if (isset($this->settings[$name])) { + $result = $this->settings[$name]; } else { - throw new base_plan_exception('setting_by_name_not_found', $name); + throw new base_plan_exception('setting_by_name_not_found', $name); } return $result; } diff --git a/backup/util/ui/backup_ui_stage.class.php b/backup/util/ui/backup_ui_stage.class.php index 6332102e25d..fbff601dba7 100644 --- a/backup/util/ui/backup_ui_stage.class.php +++ b/backup/util/ui/backup_ui_stage.class.php @@ -147,9 +147,9 @@ class backup_ui_stage_initial extends backup_ui_stage { } } } - // Add all settings at once + // Add all settings at once. $form->add_settings($add_settings); - // Add dependencies + // Add dependencies. foreach ($dependencies as $depsetting) { $form->add_dependencies($depsetting); } @@ -265,8 +265,8 @@ class backup_ui_stage_schema extends backup_ui_stage { } } $form->add_settings($add_settings); - foreach($dependencies as $depsetting) { - $form->add_dependencies($depsetting); + foreach ($dependencies as $depsetting) { + $form->add_dependencies($depsetting); } $this->stageform = $form; } diff --git a/backup/util/ui/base_moodleform.class.php b/backup/util/ui/base_moodleform.class.php index 652d5806470..e0ffc0f9fcf 100644 --- a/backup/util/ui/base_moodleform.class.php +++ b/backup/util/ui/base_moodleform.class.php @@ -136,32 +136,14 @@ abstract class base_moodleform extends moodleform { * @return bool */ function add_setting(backup_setting $setting, base_task $task=null) { - global $OUTPUT; - - // If the setting cant be changed or isn't visible then add it as a fixed setting. - if (!$setting->get_ui()->is_changeable() || $setting->get_visibility() != backup_setting::VISIBLE) { - return $this->add_fixed_setting($setting, $task); - } - - // First add the formatting for this setting - $this->add_html_formatting($setting); - - // The call the add method with the get_element_properties array - call_user_func_array(array($this->_form, 'addElement'), $setting->get_ui()->get_element_properties($task, $OUTPUT)); - $this->_form->setDefault($setting->get_ui_name(), $setting->get_value()); - if ($setting->has_help()) { - list($identifier, $component) = $setting->get_help(); - $this->_form->addHelpButton($setting->get_ui_name(), $identifier, $component); - } - $this->_form->addElement('html', html_writer::end_tag('div')); - return true; + return $this->add_settings(array(array($setting, $task))); } /** * Adds multiple backup_settings as elements to the form * @param array $settingstasks Consists of array($setting, $task) elements * @return bool */ - function add_settings(array $settingstasks) { + public function add_settings(array $settingstasks) { global $OUTPUT; $defaults = array(); diff --git a/backup/util/xml/parser/progressive_parser.class.php b/backup/util/xml/parser/progressive_parser.class.php index 8b6df4b5ee9..91bc85fc4b2 100644 --- a/backup/util/xml/parser/progressive_parser.class.php +++ b/backup/util/xml/parser/progressive_parser.class.php @@ -149,15 +149,7 @@ class progressive_parser { * handling parser paths, see MDL-24381 */ public static function dirname($path) { - static $cache = array(); - - if (!isset($cache[$path])) { - if (count($cache) > 4096) - $cache = array(); - $cache[$path] = str_replace('\\', '/', dirname($path)); - } - - return $cache[$path]; + return str_replace('\\', '/', dirname($path)); } // Protected API starts here -- 2.17.1