Merge branch 'MDL-29262' of git://github.com/stronk7/moodle

Conflicts:
	version.php

admin/settings/courses.php

@@ -92,8 +92,29 @@ if ($hassiteconfig
     // Add a category for backups
     $ADMIN->add('courses', new admin_category('backups', new lang_string('backups','admin')));
 
-    // Create a page for general backup defaults
+    // Create a page for general backups configuration and defaults.
     $temp = new admin_settingpage('backupgeneralsettings', new lang_string('generalbackdefaults', 'backup'), 'moodle/backup:backupcourse');
+
+    // General configuration section.
+    $temp->add(new admin_setting_configselect('backup/loglifetime', new lang_string('loglifetime', 'backup'), new lang_string('configloglifetime', 'backup'), 30, array(
+        1   => new lang_string('numdays', '', 1),
+        2   => new lang_string('numdays', '', 2),
+        3   => new lang_string('numdays', '', 3),
+        5   => new lang_string('numdays', '', 5),
+        7   => new lang_string('numdays', '', 7),
+        10  => new lang_string('numdays', '', 10),
+        14  => new lang_string('numdays', '', 14),
+        20  => new lang_string('numdays', '', 20),
+        30  => new lang_string('numdays', '', 30),
+        60  => new lang_string('numdays', '', 60),
+        90  => new lang_string('numdays', '', 90),
+        120 => new lang_string('numdays', '', 120),
+        180 => new lang_string('numdays', '', 180),
+        365 => new lang_string('numdays', '', 365)
+    )));
+
+    // General defaults section.
+    $temp->add(new admin_setting_heading('generalsettings', new lang_string('generalsettings', 'backup'), ''));
     $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_users', new lang_string('generalusers','backup'), new lang_string('configgeneralusers','backup'), array('value'=>1, 'locked'=>0)));
     $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_anonymize', new lang_string('generalanonymize','backup'), new lang_string('configgeneralanonymize','backup'), array('value'=>0, 'locked'=>0)));
     $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_role_assignments', new lang_string('generalroleassignments','backup'), new lang_string('configgeneralroleassignments','backup'), array('value'=>1, 'locked'=>0)));
@@ -106,8 +127,10 @@ if ($hassiteconfig
     $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_histories', new lang_string('generalhistories','backup'), new lang_string('configgeneralhistories','backup'), array('value'=>0, 'locked'=>0)));
     $ADMIN->add('backups', $temp);
 
-/// "backups" settingpage
+    // Create a page for automated backups configuration and defaults.
     $temp = new admin_settingpage('automated', new lang_string('automatedsetup','backup'), 'moodle/backup:backupcourse');
+
+    // Automated configuration section.
     $temp->add(new admin_setting_configselect('backup/backup_auto_active', new lang_string('active'),  new lang_string('autoactivedescription', 'backup'), 0, array(
         0 => new lang_string('autoactivedisabled', 'backup'),
         1 => new lang_string('autoactiveenabled', 'backup'),
@@ -139,7 +162,7 @@ if ($hassiteconfig
         500 => '500');
     $temp->add(new admin_setting_configselect('backup/backup_auto_keep', new lang_string('keep'), new lang_string('backupkeephelp'), 1, $keepoptoins));
 
-
+    // Automated defaults section.
     $temp->add(new admin_setting_heading('automatedsettings', new lang_string('automatedsettings','backup'), ''));
     $temp->add(new admin_setting_configcheckbox('backup/backup_auto_users', new lang_string('generalusers', 'backup'), new lang_string('configgeneralusers', 'backup'), 1));
     $temp->add(new admin_setting_configcheckbox('backup/backup_auto_role_assignments', new lang_string('generalroleassignments','backup'), new lang_string('configgeneralroleassignments','backup'), 1));

backup/controller/backup_controller.class.php

@@ -170,17 +170,22 @@ class backup_controller extends backup implements loggable {
     }
 
     public function set_status($status) {
-        $this->log('setting controller status to', backup::LOG_DEBUG, $status);
+        // Note: never save_controller() with the object info after STATUS_EXECUTING or the whole controller,
-        // TODO: Check it's a correct status
-        $this->status = $status;
-        // Ensure that, once set to backup::STATUS_AWAITING, controller is stored in DB
-        // Note: never save_controller() after STATUS_EXECUTING or the whole controller,
         // containing all the steps will be sent to DB. 100% (monster) useless.
+        $this->log('setting controller status to', backup::LOG_DEBUG, $status);
+        // TODO: Check it's a correct status.
+        $this->status = $status;
+        // Ensure that, once set to backup::STATUS_AWAITING, controller is stored in DB.
         if ($status == backup::STATUS_AWAITING) {
             $this->save_controller();
             $tbc = self::load_controller($this->backupid);
             $this->logger = $tbc->logger; // wakeup loggers
             $tbc->destroy(); // Clean temp controller structures
+
+        } else if ($status == backup::STATUS_FINISHED_OK) {
+            // If the operation has ended without error (backup::STATUS_FINISHED_OK)
+            // proceed by cleaning the object from database. MDL-29262.
+            $this->save_controller(false, true);
         }
     }
 
@@ -312,12 +317,20 @@ class backup_controller extends backup implements loggable {
         backup_helper::log($message, $level, $a, $depth, $display, $this->logger);
     }
 
-    public function save_controller() {
+    /**
+     * Save controller information
+     *
+     * @param bool $includeobj to decide if the object itself must be updated (true) or no (false)
+     * @param bool $cleanobj to decide if the object itself must be cleaned (true) or no (false)
+     */
+    public function save_controller($includeobj = true, $cleanobj = false) {
         // Going to save controller to persistent storage, calculate checksum for later checks and save it
         // TODO: flag the controller as NA. Any operation on it should be forbidden util loaded back
         $this->log('saving controller to db', backup::LOG_DEBUG);
-        $this->checksum = $this->calculate_checksum();
+        if ($includeobj ) {  // Only calculate checksum if we are going to include the object.
-        backup_controller_dbops::save_controller($this, $this->checksum);
+            $this->checksum = $this->calculate_checksum();
+        }
+        backup_controller_dbops::save_controller($this, $this->checksum, $includeobj, $cleanobj);
     }
 
     public static function load_controller($backupid) {

backup/controller/restore_controller.class.php

@@ -166,17 +166,22 @@ class restore_controller extends backup implements loggable {
     }
 
     public function set_status($status) {
-        $this->log('setting controller status to', backup::LOG_DEBUG, $status);
+        // Note: never save_controller() with the object info after STATUS_EXECUTING or the whole controller,
-        // TODO: Check it's a correct status
-        $this->status = $status;
-        // Ensure that, once set to backup::STATUS_AWAITING | STATUS_NEED_PRECHECK, controller is stored in DB
-        // Note: never save_controller() after STATUS_EXECUTING or the whole controller,
         // containing all the steps will be sent to DB. 100% (monster) useless.
+        $this->log('setting controller status to', backup::LOG_DEBUG, $status);
+        // TODO: Check it's a correct status.
+        $this->status = $status;
+        // Ensure that, once set to backup::STATUS_AWAITING | STATUS_NEED_PRECHECK, controller is stored in DB.
         if ($status == backup::STATUS_AWAITING || $status == backup::STATUS_NEED_PRECHECK) {
             $this->save_controller();
             $tbc = self::load_controller($this->restoreid);
             $this->logger = $tbc->logger; // wakeup loggers
             $tbc->destroy(); // Clean temp controller structures
+
+        } else if ($status == backup::STATUS_FINISHED_OK) {
+            // If the operation has ended without error (backup::STATUS_FINISHED_OK)
+            // proceed by cleaning the object from database. MDL-29262.
+            $this->save_controller(false, true);
         }
     }
 
@@ -363,12 +368,20 @@ class restore_controller extends backup implements loggable {
         backup_helper::log($message, $level, $a, $depth, $display, $this->logger);
     }
 
-    public function save_controller() {
+    /**
+     * Save controller information
+     *
+     * @param bool $includeobj to decide if the object itself must be updated (true) or no (false)
+     * @param bool $cleanobj to decide if the object itself must be cleaned (true) or no (false)
+     */
+    public function save_controller($includeobj = true, $cleanobj = false) {
         // Going to save controller to persistent storage, calculate checksum for later checks and save it
         // TODO: flag the controller as NA. Any operation on it should be forbidden util loaded back
         $this->log('saving controller to db', backup::LOG_DEBUG);
-        $this->checksum = $this->calculate_checksum();
+        if ($includeobj ) {  // Only calculate checksum if we are going to include the object.
-        restore_controller_dbops::save_controller($this, $this->checksum);
+            $this->checksum = $this->calculate_checksum();
+        }
+        restore_controller_dbops::save_controller($this, $this->checksum, $includeobj, $cleanobj);
     }
 
     public static function load_controller($restoreid) {

backup/util/dbops/backup_controller_dbops.class.php

@@ -32,16 +32,30 @@
  */
 abstract class backup_controller_dbops extends backup_dbops {
 
-    public static function save_controller($controller, $checksum) {
+    /**
+     * Send one backup controller to DB
+     *
+     * @param backup_controller $controller controller to send to DB
+     * @param string $checksum hash of the controller to be checked
+     * @param bool $includeobj to decide if the object itself must be updated (true) or no (false)
+     * @param bool $cleanobj to decide if the object itself must be cleaned (true) or no (false)
+     * @return int id of the controller record in the DB
+     * @throws backup_controller_exception|backup_dbops_exception
+     */
+    public static function save_controller($controller, $checksum, $includeobj = true, $cleanobj = false) {
         global $DB;
         // Check we are going to save one backup_controller
         if (! $controller instanceof backup_controller) {
             throw new backup_controller_exception('backup_controller_expected');
         }
-        // Check checksum is ok. Sounds silly but it isn't ;-)
+        // Check checksum is ok. Only if we are including object info. Sounds silly but it isn't ;-).
-        if (!$controller->is_checksum_correct($checksum)) {
+        if ($includeobj and !$controller->is_checksum_correct($checksum)) {
             throw new backup_dbops_exception('backup_controller_dbops_saving_checksum_mismatch');
         }
+        // Cannot request to $includeobj and $cleanobj at the same time.
+        if ($includeobj and $cleanobj) {
+            throw new backup_dbops_exception('backup_controller_dbops_saving_cannot_include_and_delete');
+        }
         // Get all the columns
         $rec = new stdclass();
         $rec->backupid     = $controller->get_backupid();
@@ -57,7 +71,11 @@ abstract class backup_controller_dbops extends backup_dbops {
         $rec->executiontime= $controller->get_executiontime();
         $rec->checksum     = $checksum;
         // Serialize information
-        $rec->controller = base64_encode(serialize($controller));
+        if ($includeobj) {
+            $rec->controller = base64_encode(serialize($controller));
+        } else if ($cleanobj) {
+            $rec->controller = '';
+        }
         // Send it to DB
         if ($recexists = $DB->get_record('backup_controllers', array('backupid' => $rec->backupid))) {
             $rec->id = $recexists->id;

backup/util/dbops/restore_controller_dbops.class.php

@@ -32,16 +32,30 @@
  */
 abstract class restore_controller_dbops extends restore_dbops {
 
-    public static function save_controller($controller, $checksum) {
+    /**
+     * Send one restore controller to DB
+     *
+     * @param restore_controller $controller controller to send to DB
+     * @param string $checksum hash of the controller to be checked
+     * @param bool $includeobj to decide if the object itself must be updated (true) or no (false)
+     * @param bool $cleanobj to decide if the object itself must be cleaned (true) or no (false)
+     * @return int id of the controller record in the DB
+     * @throws backup_controller_exception|restore_dbops_exception
+     */
+    public static function save_controller($controller, $checksum, $includeobj = true, $cleanobj = false) {
         global $DB;
         // Check we are going to save one backup_controller
         if (! $controller instanceof restore_controller) {
             throw new backup_controller_exception('restore_controller_expected');
         }
-        // Check checksum is ok. Sounds silly but it isn't ;-)
+        // Check checksum is ok. Only if we are including object info. Sounds silly but it isn't ;-).
-        if (!$controller->is_checksum_correct($checksum)) {
+        if ($includeobj and !$controller->is_checksum_correct($checksum)) {
             throw new restore_dbops_exception('restore_controller_dbops_saving_checksum_mismatch');
         }
+        // Cannot request to $includeobj and $cleanobj at the same time.
+        if ($includeobj and $cleanobj) {
+            throw new restore_dbops_exception('restore_controller_dbops_saving_cannot_include_and_delete');
+        }
         // Get all the columns
         $rec = new stdclass();
         $rec->backupid     = $controller->get_restoreid();
@@ -57,7 +71,11 @@ abstract class restore_controller_dbops extends restore_dbops {
         $rec->executiontime= $controller->get_executiontime();
         $rec->checksum     = $checksum;
         // Serialize information
-        $rec->controller = base64_encode(serialize($controller));
+        if ($includeobj) {
+            $rec->controller = base64_encode(serialize($controller));
+        } else if ($cleanobj) {
+            $rec->controller = '';
+        }
         // Send it to DB
         if ($recexists = $DB->get_record('backup_controllers', array('backupid' => $rec->backupid))) {
             $rec->id = $recexists->id;

lang/en/backup.php

@@ -88,6 +88,7 @@ $string['configgenerallogs'] = 'If enabled logs will be included in backups by d
 $string['configgeneralroleassignments'] = 'If enabled by default roles assignments will also be backed up.';
 $string['configgeneraluserscompletion'] = 'If enabled user completion information will be included in backups by default.';
 $string['configgeneralusers'] = 'Sets the default for whether to include users in backups.';
+$string['configloglifetime'] = 'This specifies the length of time you want to keep backup logs information. Logs that are older than this age are automatically deleted. It is recommended to keep this value small, because backup logged information can be huge.';
 $string['confirmcancel'] = 'Cancel backup';
 $string['confirmcancelquestion'] = 'Are you sure you wish to cancel?
 Any information you have entered will be lost.';
@@ -126,6 +127,7 @@ $string['generalhistories'] = 'Include histories';
 $string['generalgradehistories'] = 'Include histories';
 $string['generallogs'] = 'Include logs';
 $string['generalroleassignments'] = 'Include role assignments';
+$string['generalsettings'] = 'General backup settings';
 $string['generaluserscompletion'] = 'Include user completion information';
 $string['generalusers'] = 'Include users';
 $string['importfile'] = 'Import a backup file';
@@ -145,6 +147,7 @@ $string['includeactivities'] = 'Include:';
 $string['includeditems'] = 'Included items:';
 $string['includesection'] = 'Section {$a}';
 $string['includeuserinfo'] = 'User data';
+$string['loglifetime'] = 'Keep logs for';
 $string['locked'] = 'Locked';
 $string['lockedbypermission'] = 'You don\'t have sufficient permissions to change this setting';
 $string['lockedbyconfig'] = 'This setting has been locked by the default backup settings';

lib/cronlib.php

@@ -113,17 +113,18 @@ function cron_run() {
         }
 
 
-        // Delete old backup_controllers and logs
+        // Delete old backup_controllers and logs.
-        if (!empty($CFG->loglifetime)) {  // value in days
+        $loglifetime = get_config('backup', 'loglifetime');
-            $loglifetime = $timenow - ($CFG->loglifetime * 3600 * 24);
+        if (!empty($loglifetime)) {  // Value in days.
-            // Delete child records from backup_logs
+            $loglifetime = $timenow - ($loglifetime * 3600 * 24);
+            // Delete child records from backup_logs.
             $DB->execute("DELETE FROM {backup_logs}
                            WHERE EXISTS (
                                SELECT 'x'
                                  FROM {backup_controllers} bc
                                 WHERE bc.backupid = {backup_logs}.backupid
                                   AND bc.timecreated < ?)", array($loglifetime));
-            // Delete records from backup_controllers
+            // Delete records from backup_controllers.
             $DB->execute("DELETE FROM {backup_controllers}
                           WHERE timecreated < ?", array($loglifetime));
             mtrace(" Deleted old backup records");

version.php

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