MDL-48266 backup: Fix backup strorage misconfiguration

Basically if the destination dir is empty then the storage type should be set to the course backup area. This does not affect the behaviour of the backups that lived with misconfigured settings.
andelacruz · Jan 22, 2015 · 94b8c63 · 94b8c63
1 parent 9fdf16a
commit 94b8c63
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 11 deletions.
diff --git a/backup/util/helper/backup_cron_helper.class.php b/backup/util/helper/backup_cron_helper.class.php
@@ -397,16 +397,11 @@ public static function launch_automated_backup($course, $starttime, $userid) {
             $outcome = self::outcome_from_results($results);
             $file = $results['backup_destination']; // May be empty if file already moved to target location.
 
-            if (empty($dir) && $storage !== 0) {
-                // This is intentionally left as a warning instead of an error because of the current behaviour of backup settings.
-                // See MDL-48266 for details.
-                $bc->log('No directory specified for automated backups',
-                        backup::LOG_WARNING);
-                $outcome = self::BACKUP_STATUS_WARNING;
-            } else if ($storage !== 0 && (!file_exists($dir) || !is_dir($dir) || !is_writable($dir))) {
-                // If we need to copy the backup file to an external dir and it is not writable, change status to error.
-                $bc->log('Specified backup directory is not writable - ',
-                        backup::LOG_ERROR, $dir);
+            // If we need to copy the backup file to an external dir and it is not writable, change status to error.
+            // This is a feature to prevent moodledata to be filled up and break a site when the admin misconfigured
+            // the automated backups storage type and destination directory.
+            if ($storage !== 0 && (empty($dir) || !file_exists($dir) || !is_dir($dir) || !is_writable($dir))) {
+                $bc->log('Specified backup directory is not writable - ', backup::LOG_ERROR, $dir);
                 $dir = null;
                 $outcome = self::BACKUP_STATUS_ERROR;
             }

diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php
@@ -4121,6 +4121,23 @@ function xmldb_main_upgrade($oldversion) {
 
         // Main savepoint reached.
         upgrade_main_savepoint(true, 2015010800.01);
+
+    }
+
+    if ($oldversion < 2015011501.02) {
+
+        // If the site is using internal and external storage, or just external
+        // storage, and the external path specified is empty we change the setting
+        // to internal only. That is how the backup code is handling this
+        // misconfiguration.
+        $storage = (int) get_config('backup_auto_storage', 'backup');
+        $folder = get_config('backup_auto_destination', 'backup');
+        if ($storage !== 0 && empty($folder)) {
+            set_config('backup_auto_storage', 0, 'backup');
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2015011501.02);
     }
 
     return true;

diff --git a/version.php b/version.php
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2015011501.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2015011501.02;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.