Skip to content

Commit

Permalink
MDL-41210 tool_uploadcourse: Fixed restore from same backup file
Browse files Browse the repository at this point in the history
  • Loading branch information
Frederic Massart committed Aug 15, 2013
1 parent 50ff861 commit d90b2eb
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 69 deletions.
80 changes: 33 additions & 47 deletions admin/tool/uploadcourse/classes/helper.php
View file
Expand Up @@ -37,31 +37,6 @@
*/
class tool_uploadcourse_helper {

/**
* Remove the restore content from disk and cache.
*
* @return void
*/
public static function clean_restore_content() {
global $CFG;

// There are some sloppy unclosed file handles in backup/restore code,
// let's hope somebody unset all controllers before calling this
// and destroy magic will close all remaining open file handles,
// otherwise Windows will fail deleting the directory.
gc_collect_cycles();

if (!empty($CFG->keeptempdirectoriesonbackup)) {
$cache = cache::make('tool_uploadcourse', 'helper');
$backupids = (array) $cache->get('backupids');
foreach ($backupids as $cachekey => $backupid) {
$cache->delete($cachekey);
fulldelete("$CFG->tempdir/backup/$backupid/");
}
$cache->delete('backupids');
}
}

/**
* Generate a shortname based on a template.
*
Expand Down Expand Up @@ -216,7 +191,9 @@ public static function get_enrolment_plugins() {
* Get the restore content tempdir.
*
* The tempdir is the sub directory in which the backup has been extracted.
* This caches the result for better performance.
*
* This caches the result for better performance, but $CFG->keeptempdirectoriesonbackup
* needs to be enabled, otherwise the cache is ignored.
*
* @param string $backupfile path to a backup file.
* @param string $shortname shortname of a course.
Expand All @@ -229,6 +206,10 @@ public static function get_restore_content_dir($backupfile = null, $shortname =
$cachekey = null;
if (!empty($backupfile)) {
$backupfile = realpath($backupfile);
if (empty($backupfile) || !is_readable($backupfile)) {
$errors['cannotreadbackupfile'] = new lang_string('cannotreadbackupfile', 'tool_uploadcourse');
return false;
}
$cachekey = 'backup_path:' . $backupfile;
} else if (!empty($shortname) || is_numeric($shortname)) {
$cachekey = 'backup_sn:' . $shortname;
Expand All @@ -238,23 +219,27 @@ public static function get_restore_content_dir($backupfile = null, $shortname =
return false;
}

$cache = cache::make('tool_uploadcourse', 'helper');
if (($backupid = $cache->get($cachekey)) === false) {
// Use false instead of null because it would consider that the cache
// key has not been set.
$backupid = false;
// If $CFG->keeptempdirectoriesonbackup is not set to true, any restore happening would
// automatically delete the backup directory... causing the cache to return an unexisting directory.
$usecache = !empty($CFG->keeptempdirectoriesonbackup);
if ($usecache) {
$cache = cache::make('tool_uploadcourse', 'helper');
}

// If we don't use the cache, or if we do and not set, or the directory doesn't exist any more.
if (!$usecache || (($backupid = $cache->get($cachekey)) === false || !is_dir("$CFG->tempdir/backup/$backupid"))) {

// Use null instead of false because it would consider that the cache key has not been set.
$backupid = null;

if (!empty($backupfile)) {
if (!is_readable($backupfile)) {
$errors['cannotreadbackupfile'] = new lang_string('cannotreadbackupfile', 'tool_uploadcourse');
} else {
// Extracting the backup file.
$packer = get_file_packer('application/vnd.moodle.backup');
$backupid = restore_controller::get_tempdir_name(SITEID, $USER->id);
$path = "$CFG->tempdir/backup/$backupid/";
$result = $packer->extract_to_pathname($backupfile, $path);
if (!$result) {
$errors['invalidbackupfile'] = new lang_string('invalidbackupfile', 'tool_uploadcourse');
}
// Extracting the backup file.
$packer = get_file_packer('application/vnd.moodle.backup');
$backupid = restore_controller::get_tempdir_name(SITEID, $USER->id);
$path = "$CFG->tempdir/backup/$backupid/";
$result = $packer->extract_to_pathname($backupfile, $path);
if (!$result) {
$errors['invalidbackupfile'] = new lang_string('invalidbackupfile', 'tool_uploadcourse');
}
} else if (!empty($shortname) || is_numeric($shortname)) {
// Creating restore from an existing course.
Expand All @@ -270,14 +255,15 @@ public static function get_restore_content_dir($backupfile = null, $shortname =
new lang_string('coursetorestorefromdoesnotexist', 'tool_uploadcourse');
}
}
$cache->set($cachekey, $backupid);

// Store all the directories to be able to remove them in self::clean_restore_content().
$backupids = (array) $cache->get('backupids');
$backupids[$cachekey] = $backupid;
$cache->set('backupids', $backupids);
if ($usecache) {
$cache->set($cachekey, $backupid);
}
}

if ($backupid === null) {
$backupid = false;
}
return $backupid;
}

Expand Down
12 changes: 0 additions & 12 deletions admin/tool/uploadcourse/classes/processor.php
View file
Expand Up @@ -223,8 +223,6 @@ public function execute($tracker = null) {

$tracker->finish();
$tracker->results($total, $created, $updated, $deleted, $errors);

$this->remove_restore_content();
}

/**
Expand Down Expand Up @@ -349,20 +347,10 @@ public function preview($rows = 10, $tracker = null) {
}

$tracker->finish();
$this->remove_restore_content();

return $preview;
}

/**
* Delete the restore object.
*
* @return void
*/
protected function remove_restore_content() {
tool_uploadcourse_helper::clean_restore_content();
}

/**
* Reset the current process.
*
Expand Down
36 changes: 36 additions & 0 deletions admin/tool/uploadcourse/tests/course_test.php
View file
Expand Up @@ -635,6 +635,23 @@ public function test_restore_course() {
}
$this->assertTrue($found);

// Restoring twice from the same course should work.
$data = array('shortname' => 'B1', 'templatecourse' => $c1->shortname, 'summary' => 'B', 'category' => 1,
'fullname' => 'B1');
$co = new tool_uploadcourse_course($mode, $updatemode, $data);
$this->assertTrue($co->prepare());
$co->proceed();
$course = $DB->get_record('course', array('shortname' => 'B1'));
$modinfo = get_fast_modinfo($course);
$found = false;
foreach ($modinfo->get_cms() as $cmid => $cm) {
if ($cm->modname == 'forum' && $cm->name == $c1f1->name) {
$found = true;
break;
}
}
$this->assertTrue($found);

// Restore the time limit to prevent warning.
set_time_limit(0);
}
Expand Down Expand Up @@ -668,6 +685,25 @@ public function test_restore_file() {
}
$this->assertTrue($found);

// Restoring twice from the same file should work.
$data = array('shortname' => 'B1', 'backupfile' => __DIR__ . '/fixtures/backup.mbz',
'summary' => 'B', 'category' => 1, 'fullname' => 'B1');
$co = new tool_uploadcourse_course($mode, $updatemode, $data);
$this->assertTrue($co->prepare());
$co->proceed();
$course = $DB->get_record('course', array('shortname' => 'B1'));
$modinfo = get_fast_modinfo($course);
$found = false;
foreach ($modinfo->get_cms() as $cmid => $cm) {
if ($cm->modname == 'glossary' && $cm->name == 'Imported Glossary') {
$found = true;
} else if ($cm->modname == 'forum' && $cm->name == $c1f1->name) {
// We should not find this!
$this->assertTrue(false);
}
}
$this->assertTrue($found);

// Restore the time limit to prevent warning.
set_time_limit(0);
}
Expand Down
31 changes: 21 additions & 10 deletions admin/tool/uploadcourse/tests/helper_test.php
View file
Expand Up @@ -141,6 +141,9 @@ public function test_get_restore_content_dir() {
$bc->destroy();
unset($bc); // File logging is a mess, we can only try to rely on gc to close handles.

$oldcfg = isset($CFG->keeptempdirectoriesonbackup) ? $CFG->keeptempdirectoriesonbackup : false;
$CFG->keeptempdirectoriesonbackup = true;

// Checking restore dir.
$dir = tool_uploadcourse_helper::get_restore_content_dir($c1backupfile, null);
$bcinfo = backup_general_helper::get_backup_information($dir);
Expand Down Expand Up @@ -179,18 +182,26 @@ public function test_get_restore_content_dir() {
$this->assertFalse($dir);
$this->assertArrayHasKey('coursetorestorefromdoesnotexist', $errors);

// Cleaning content directories.
$oldcfg = isset($CFG->keeptempdirectoriesonbackup) ? $CFG->keeptempdirectoriesonbackup : false;
$dir = "$CFG->tempdir/backup/$dir";
$this->assertTrue(file_exists($dir));

// Trying again without caching. $CFG->keeptempdirectoriesonbackup is required for caching.
$CFG->keeptempdirectoriesonbackup = false;
tool_uploadcourse_helper::clean_restore_content();
$this->assertTrue(file_exists($dir));

$CFG->keeptempdirectoriesonbackup = true;
tool_uploadcourse_helper::clean_restore_content();
$this->assertFalse(file_exists($dir));
// Checking restore dir.
$dir = tool_uploadcourse_helper::get_restore_content_dir($c1backupfile, null);
$dir2 = tool_uploadcourse_helper::get_restore_content_dir($c1backupfile, null);
$this->assertNotEquals($dir, $dir2);

// Checking with a shortname.
$dir = tool_uploadcourse_helper::get_restore_content_dir(null, $c1->shortname);
$dir2 = tool_uploadcourse_helper::get_restore_content_dir(null, $c1->shortname);
$this->assertNotEquals($dir, $dir2);

// Get a course that does not exist.
$errors = array();
$dir = tool_uploadcourse_helper::get_restore_content_dir(null, 'DoesNotExist', $errors);
$this->assertFalse($dir);
$this->assertArrayHasKey('coursetorestorefromdoesnotexist', $errors);
$dir2 = tool_uploadcourse_helper::get_restore_content_dir(null, 'DoesNotExist', $errors);
$this->assertEquals($dir, $dir2);

$CFG->keeptempdirectoriesonbackup = $oldcfg;

Expand Down

0 comments on commit d90b2eb

Please sign in to comment.