Browse files

MDL-30725 save automated external only backups directly to ext storag…

…e, do not pollute file pool

This solution is based on code by Tony Levi, thanks!
  • Loading branch information...
1 parent 29515af commit b98956fdd2ea33a4f1a76c9505418ed925b06893 @skodak skodak committed with stronk7 Jun 7, 2012
Showing with 41 additions and 4 deletions.
  1. +2 −2 backup/util/helper/backup_cron_helper.class.php
  2. +39 −2 backup/util/helper/backup_helper.class.php
View
4 backup/util/helper/backup_cron_helper.class.php
@@ -350,13 +350,13 @@ public static function launch_automated_backup($course, $starttime, $userid) {
$bc->execute_plan();
$results = $bc->get_results();
- $file = $results['backup_destination'];
+ $file = $results['backup_destination']; // may be empty if file already moved to target location
$dir = $config->backup_auto_destination;
$storage = (int)$config->backup_auto_storage;
if (!file_exists($dir) || !is_dir($dir) || !is_writable($dir)) {
$dir = null;
}
- if (!empty($dir) && $storage !== 0) {
+ if ($file && !empty($dir) && $storage !== 0) {
$filename = backup_plan_dbops::get_default_backup_filename($format, $type, $course->id, $users, $anonymised, !$config->backup_shortname);
$outcome = $file->copy_content_to($dir.'/'.$filename);
if ($outcome && $storage === 1) {
View
41 backup/util/helper/backup_helper.class.php
@@ -176,8 +176,17 @@ static public function log($message, $level, $a, $depth, $display, $logger) {
/**
* Given one backupid and the (FS) final generated file, perform its final storage
* into Moodle file storage. For stored files it returns the complete file_info object
+ *
+ * Note: the $filepath is deleted if the backup file is created successfully
+ *
+ * @param int $backupid
+ * @param string $filepath zip file containing the backup
+ * @return stored_file if created, null otherwise
+ *
+ * @throws moodle_exception in case of any problems
*/
static public function store_backup_file($backupid, $filepath) {
+ global $CFG;
// First of all, get some information from the backup_controller to help us decide
list($dinfo, $cinfo, $sinfo) = backup_controller_dbops::get_moodle_backup_information($backupid);
@@ -191,6 +200,7 @@ static public function store_backup_file($backupid, $filepath) {
$userid = $dinfo[0]->userid; // User->id executing the backup
$id = $dinfo[0]->id; // Id of activity/section/course (depends of type)
$courseid = $dinfo[0]->courseid; // Id of the course
+ $format = $dinfo[0]->format; // Type of backup file
// Quick hack. If for any reason, filename is blank, fix it here.
// TODO: This hack will be out once MDL-22142 - P26 gets fixed
@@ -200,7 +210,13 @@ static public function store_backup_file($backupid, $filepath) {
// Backups of type IMPORT aren't stored ever
if ($backupmode == backup::MODE_IMPORT) {
- return false;
+ return null;
+ }
+
+ if (!is_readable($filepath)) {
+ // we have a problem if zip file does not exist
+ throw new coding_exception('backup_helper::store_backup_file() expects valid $filepath parameter');
+
}
// Calculate file storage options of id being backup
@@ -232,6 +248,25 @@ static public function store_backup_file($backupid, $filepath) {
if ($backupmode == backup::MODE_AUTOMATED) {
// Automated backups have there own special area!
$filearea = 'automated';
+
+ // If we're keeping the backup only in a chosen path, just move it there now
+ // this saves copying from filepool to here later and filling trashdir.
+ $config = get_config('backup');
+ $dir = $config->backup_auto_destination;
+ if ($config->backup_auto_storage == 1 and $dir and is_dir($dir) and is_writable($dir)) {
+ $filedest = $dir.'/'.backup_plan_dbops::get_default_backup_filename($format, $backuptype, $courseid, $hasusers, $isannon, !$config->backup_shortname);
+ // first try to move the file, if it is not possible copy and delete instead
+ if (@rename($filepath, $filedest)) {
+ return null;
+ }
+ umask(0000);
+ if (copy($filepath, $filedest)) {
+ @chmod($filedest, $CFG->filepermissions); // may fail because the permissions may not make sense outside of dataroot
+ unlink($filepath);
+ return null;
+ }
+ // bad luck, try to deal with the file the old way - keep backup in file area if we can not copy to ext system
+ }
}
// Backups of type HUB (by definition never have user info)
@@ -275,7 +310,9 @@ static public function store_backup_file($backupid, $filepath) {
$sf = $fs->get_file_by_hash($pathnamehash);
$sf->delete();
}
- return $fs->create_file_from_pathname($fr, $filepath);
+ $file = $fs->create_file_from_pathname($fr, $filepath);
+ unlink($filepath);
+ return $file;
}
/**

0 comments on commit b98956f

Please sign in to comment.