MDL-37761 Backup: Do not include files in backups when importing courses/activities
authorAndrew Nicols <andrew@nicols.co.uk>
Sun, 12 May 2013 15:53:57 +0000 (16:53 +0100)
committerAndrew Nicols <andrew@nicols.co.uk>
Tue, 18 Jun 2013 06:52:04 +0000 (07:52 +0100)
When we import courses or duplicate activities, the deduplicating nature of
the file_storage system means that we don't actually have to include the
real files in the backup - just their metadata from the file table.

The restoration process needs to know not to expect files from the backup
phase so a flag is set in moodle_backup metadata.

backup/controller/backup_controller.class.php
backup/controller/tests/controller_test.php
backup/moodle2/backup_stepslib.php
backup/util/dbops/backup_controller_dbops.class.php
backup/util/dbops/restore_dbops.class.php
backup/util/dbops/tests/backup_dbops_test.php
backup/util/helper/backup_file_manager.class.php
backup/util/helper/backup_general_helper.class.php

index 5d27ab0..b72c394 100644 (file)
@@ -56,6 +56,7 @@ class backup_controller extends backup implements loggable {
     protected $status; // Current status of the controller (created, planned, configured...)
 
     protected $plan;   // Backup execution plan
+    protected $includefiles; // Whether this backup includes files or not.
 
     protected $execution;     // inmediate/delayed
     protected $executiontime; // epoch time when we want the backup to be executed (requires cron to run)
@@ -239,6 +240,17 @@ class backup_controller extends backup implements loggable {
         return $this->type;
     }
 
+    /**
+     * Returns the current value of the include_files setting.
+     * This setting is intended to ensure that files are not included in
+     * generated backups.
+     *
+     * @return int Indicates whether files should be included in backups.
+     */
+    public function get_include_files() {
+        return $this->includefiles;
+    }
+
     public function get_operation() {
         return $this->operation;
     }
@@ -362,6 +374,33 @@ class backup_controller extends backup implements loggable {
         $this->log('applying plan defaults', backup::LOG_DEBUG);
         backup_controller_dbops::apply_config_defaults($this);
         $this->set_status(backup::STATUS_CONFIGURED);
+        $this->set_include_files();
+    }
+
+    /**
+     * Set the initial value for the include_files setting.
+     *
+     * @see backup_controller::get_include_files for further information on the purpose of this setting.
+     * @return int Indicates whether files should be included in backups.
+     */
+    protected function set_include_files() {
+        // We normally include files.
+        $includefiles = true;
+
+        // In an import, we don't need to include files.
+        if ($this->get_mode() === backup::MODE_IMPORT) {
+            $includefiles = false;
+        }
+
+        // When a backup is intended for the same site, we don't need to include the files.
+        // Note, this setting is only used for duplication of an entire course.
+        if ($this->get_mode() === backup::MODE_SAMESITE) {
+            $includefiles = false;
+        }
+
+        $this->includefiles = (int) $includefiles;
+        $this->log("setting file inclusion to {$this->includefiles}", backup::LOG_DEBUG);
+        return $this->includefiles;
     }
 }
 
index aa28a97..6b44703 100644 (file)
@@ -82,6 +82,25 @@ class backup_controller_testcase extends advanced_testcase {
         $newbc = mock_backup_controller::load_controller($bc->get_backupid());
         $this->assertTrue($newbc instanceof backup_controller); // This means checksum and load worked ok
     }
+
+    public function test_backup_controller_include_files() {
+        // A MODE_GENERAL controller - this should include files
+        $bc = new mock_backup_controller(backup::TYPE_1ACTIVITY, $this->moduleid, backup::FORMAT_MOODLE,
+            backup::INTERACTIVE_NO, backup::MODE_GENERAL, $this->userid);
+        $this->assertEquals($bc->get_include_files(), 1);
+
+
+        // The MODE_IMPORT and MODE_SAMESITE should not include files in the backup.
+        // A MODE_IMPORT controller
+        $bc = new mock_backup_controller(backup::TYPE_1ACTIVITY, $this->moduleid, backup::FORMAT_MOODLE,
+            backup::INTERACTIVE_NO, backup::MODE_IMPORT, $this->userid);
+        $this->assertEquals($bc->get_include_files(), 0);
+
+        // A MODE_SAMESITE controller
+        $bc = new mock_backup_controller(backup::TYPE_1COURSE, $this->courseid, backup::FORMAT_MOODLE,
+            backup::INTERACTIVE_NO, backup::MODE_IMPORT, $this->userid);
+        $this->assertEquals($bc->get_include_files(), 0);
+    }
 }
 
 
index ddd38ef..d49bfa4 100644 (file)
@@ -1489,6 +1489,7 @@ class backup_main_structure_step extends backup_structure_step {
         $info['backup_date']    = time();
         $info['backup_uniqueid']= $this->get_backupid();
         $info['mnet_remoteusers']=backup_controller_dbops::backup_includes_mnet_remote_users($this->get_backupid());
+        $info['include_files'] = backup_controller_dbops::backup_includes_files($this->get_backupid());
         $info['include_file_references_to_external_content'] =
                 backup_controller_dbops::backup_includes_file_references($this->get_backupid());
         $info['original_wwwroot']=$CFG->wwwroot;
@@ -1510,7 +1511,7 @@ class backup_main_structure_step extends backup_structure_step {
 
         $information = new backup_nested_element('information', null, array(
             'name', 'moodle_version', 'moodle_release', 'backup_version',
-            'backup_release', 'backup_date', 'mnet_remoteusers', 'include_file_references_to_external_content', 'original_wwwroot',
+            'backup_release', 'backup_date', 'mnet_remoteusers', 'include_files', 'include_file_references_to_external_content', 'original_wwwroot',
             'original_site_identifier_hash', 'original_course_id',
             'original_course_fullname', 'original_course_shortname', 'original_course_startdate',
             'original_course_contextid', 'original_system_contextid'));
index 68c1752..06421f3 100644 (file)
@@ -409,6 +409,19 @@ abstract class backup_controller_dbops extends backup_dbops {
         return (int)(bool)$count;
     }
 
+    /**
+     * Given the backupid, determine whether this backup should include
+     * files from the moodle file storage system.
+     *
+     * @param string $backupid The ID of the backup.
+     * @return int Indicates whether files should be included in backups.
+     */
+    public static function backup_includes_files($backupid) {
+        // Load controller
+        $bc = self::load_controller($backupid);
+        return $bc->get_include_files();
+    }
+
     /**
      * Given the backupid, detect if the backup contains references to external contents
      *
index 518d7ba..5146cfe 100644 (file)
@@ -823,6 +823,9 @@ abstract class restore_dbops {
     public static function send_files_to_pool($basepath, $restoreid, $component, $filearea, $oldcontextid, $dfltuserid, $itemname = null, $olditemid = null, $forcenewcontextid = null, $skipparentitemidctxmatch = false) {
         global $DB, $CFG;
 
+        $backupinfo = backup_general_helper::get_backup_information(basename($basepath));
+        $includesfiles = $backupinfo->include_files;
+
         $results = array();
 
         if ($forcenewcontextid) {
@@ -900,43 +903,68 @@ abstract class restore_dbops {
                 continue;
             }
 
+            // The file record to restore.
+            $file_record = array(
+                'contextid'   => $newcontextid,
+                'component'   => $component,
+                'filearea'    => $filearea,
+                'itemid'      => $rec->newitemid,
+                'filepath'    => $file->filepath,
+                'filename'    => $file->filename,
+                'timecreated' => $file->timecreated,
+                'timemodified'=> $file->timemodified,
+                'userid'      => $mappeduserid,
+                'author'      => $file->author,
+                'license'     => $file->license,
+                'sortorder'   => $file->sortorder
+            );
+
             if (empty($file->repositoryid)) {
                 // this is a regular file, it must be present in the backup pool
                 $backuppath = $basepath . backup_file_manager::get_backup_content_file_location($file->contenthash);
 
-                // The file is not found in the backup.
-                if (!file_exists($backuppath)) {
-                    $result = new stdClass();
-                    $result->code = 'file_missing_in_backup';
-                    $result->message = sprintf('missing file %s%s in backup', $file->filepath, $file->filename);
-                    $result->level = backup::LOG_WARNING;
-                    $results[] = $result;
-                    continue;
-                }
+                // Some file types do not include the files as they should already be
+                // present. We still need to create entries into the files table.
+                if ($includesfiles) {
+                    // The file is not found in the backup.
+                    if (!file_exists($backuppath)) {
+                        $result = new stdClass();
+                        $result->code = 'file_missing_in_backup';
+                        $result->message = sprintf('missing file %s%s in backup', $file->filepath, $file->filename);
+                        $result->level = backup::LOG_WARNING;
+                        $results[] = $result;
+                        continue;
+                    }
 
-                // create the file in the filepool if it does not exist yet
-                if (!$fs->file_exists($newcontextid, $component, $filearea, $rec->newitemid, $file->filepath, $file->filename)) {
+                    // create the file in the filepool if it does not exist yet
+                    if (!$fs->file_exists($newcontextid, $component, $filearea, $rec->newitemid, $file->filepath, $file->filename)) {
 
-                    // If no license found, use default.
-                    if ($file->license == null){
-                        $file->license = $CFG->sitedefaultlicense;
-                    }
+                        // If no license found, use default.
+                        if ($file->license == null){
+                            $file->license = $CFG->sitedefaultlicense;
+                        }
 
-                    $file_record = array(
-                        'contextid'   => $newcontextid,
-                        'component'   => $component,
-                        'filearea'    => $filearea,
-                        'itemid'      => $rec->newitemid,
-                        'filepath'    => $file->filepath,
-                        'filename'    => $file->filename,
-                        'timecreated' => $file->timecreated,
-                        'timemodified'=> $file->timemodified,
-                        'userid'      => $mappeduserid,
-                        'author'      => $file->author,
-                        'license'     => $file->license,
-                        'sortorder'   => $file->sortorder
-                    );
-                    $fs->create_file_from_pathname($file_record, $backuppath);
+                        $fs->create_file_from_pathname($file_record, $backuppath);
+                    }
+                } else {
+                    // This backup does not include the files - they should be available in moodle filestorage already.
+
+                    // Even if a file has been deleted since the backup was made, the file metadata will remain in the
+                    // files table, and the file will not be moved to the trashdir.
+                    // Files are not cleared from the files table by cron until several days after deletion.
+                    if ($foundfiles = $DB->get_records('files', array('contenthash' => $file->contenthash))) {
+                        // Only grab one of the foundfiles - the file content should be the same for all entries.
+                        $foundfile = reset($foundfiles);
+                        $fs->create_file_from_storedfile($file_record, $foundfile->id);
+                    } else {
+                        // A matching existing file record was not found in the database.
+                        $result = new stdClass();
+                        $result->code = 'file_missing_in_backup';
+                        $result->message = sprintf('missing file %s%s in backup', $file->filepath, $file->filename);
+                        $result->level = backup::LOG_WARNING;
+                        $results[] = $result;
+                        continue;
+                    }
                 }
 
                 // store the the new contextid and the new itemid in case we need to remap
@@ -954,20 +982,7 @@ abstract class restore_dbops {
                     // oldfile holds the raw information stored in MBZ (including reference-related info)
                     $info->oldfile = $file;
                     // newfile holds the info for the new file_record with the context, user and itemid mapped
-                    $info->newfile = (object)array(
-                        'contextid'   => $newcontextid,
-                        'component'   => $component,
-                        'filearea'    => $filearea,
-                        'itemid'      => $rec->newitemid,
-                        'filepath'    => $file->filepath,
-                        'filename'    => $file->filename,
-                        'timecreated' => $file->timecreated,
-                        'timemodified'=> $file->timemodified,
-                        'userid'      => $mappeduserid,
-                        'author'      => $file->author,
-                        'license'     => $file->license,
-                        'sortorder'   => $file->sortorder
-                    );
+                    $info->newfile = (object) $file_record;
 
                     restore_dbops::set_backup_ids_record($restoreid, 'file_aliases_queue', $file->id, 0, null, $info);
                 }
index 8caf629..55d57ba 100644 (file)
@@ -148,6 +148,30 @@ class backup_dbops_testcase extends advanced_testcase {
         backup_controller_dbops::drop_backup_ids_temp_table('testingid');
         $this->assertFalse($dbman->table_exists('backup_ids_temp'));
     }
+
+    /**
+     * Check backup_includes_files
+     */
+    function test_backup_controller_dbops_includes_files() {
+        global $DB;
+
+        $dbman = $DB->get_manager(); // Going to use some database_manager services for testing
+
+        // A MODE_GENERAL controller - this should include files
+        $bc = new mock_backup_controller4dbops(backup::TYPE_1ACTIVITY, $this->moduleid, backup::FORMAT_MOODLE,
+            backup::INTERACTIVE_NO, backup::MODE_GENERAL, $this->userid);
+        $this->assertEquals(backup_controller_dbops::backup_includes_files($bc->get_backupid()), 1);
+
+        // A MODE_IMPORT controller - should not include files
+        $bc = new mock_backup_controller4dbops(backup::TYPE_1ACTIVITY, $this->moduleid, backup::FORMAT_MOODLE,
+            backup::INTERACTIVE_NO, backup::MODE_IMPORT, $this->userid);
+        $this->assertEquals(backup_controller_dbops::backup_includes_files($bc->get_backupid()), 0);
+
+        // A MODE_SAMESITE controller - should not include files
+        $bc = new mock_backup_controller4dbops(backup::TYPE_1COURSE, $this->moduleid, backup::FORMAT_MOODLE,
+            backup::INTERACTIVE_NO, backup::MODE_SAMESITE, $this->userid);
+        $this->assertEquals(backup_controller_dbops::backup_includes_files($bc->get_backupid()), 0);
+    }
 }
 
 class mock_backup_controller4dbops extends backup_controller {
index 9941af7..876423d 100644 (file)
@@ -61,6 +61,11 @@ class backup_file_manager {
     public static function copy_file_moodle2backup($backupid, $filerecorid) {
         global $DB;
 
+        if (!backup_controller_dbops::backup_includes_files($backupid)) {
+            // Only include the files if required by the controller.
+            return;
+        }
+
         // Normalise param
         if (!is_object($filerecorid)) {
             $filerecorid = $DB->get_record('files', array('id' => $filerecorid));
index 798c6b6..03ab11f 100644 (file)
@@ -155,6 +155,12 @@ abstract class backup_general_helper extends backup_helper {
         } else {
             $info->include_file_references_to_external_content = 0;
         }
+        // include_files is a new setting in 2.6.
+        if (isset($infoarr['include_files'])) {
+            $info->include_files = $infoarr['include_files'];
+        } else {
+            $info->include_files = 1;
+        }
         $info->type   =  $infoarr['details']['detail'][0]['type'];
         $info->format =  $infoarr['details']['detail'][0]['format'];
         $info->mode   =  $infoarr['details']['detail'][0]['mode'];