MDL-41347 tool_uploadcourse: Invalid restore value handled
authorFrederic Massart <fred@moodle.com>
Thu, 3 Oct 2013 02:56:57 +0000 (10:56 +0800)
committerFrederic Massart <fred@moodle.com>
Thu, 3 Oct 2013 03:22:53 +0000 (11:22 +0800)
admin/tool/uploadcourse/classes/course.php
admin/tool/uploadcourse/tests/course_test.php

index 782e1e8..4691ead 100644 (file)
@@ -347,7 +347,8 @@ class tool_uploadcourse_course {
     /**
      * Get the directory of the object to restore.
      *
-     * @return string|false subdirectory in $CFG->tempdir/backup/...
+     * @return string|false|null subdirectory in $CFG->tempdir/backup/..., false when an error occured
+     *                           and null when there is simply nothing.
      */
     protected function get_restore_content_dir() {
         $backupfile = null;
@@ -366,6 +367,9 @@ class tool_uploadcourse_course {
                 $this->error($key, $message);
             }
             return false;
+        } else if ($dir === false) {
+            // We want to return null when nothing was wrong, but nothing was found.
+            $dir = null;
         }
 
         if (empty($dir) && !empty($this->importoptions['restoredir'])) {
@@ -656,8 +660,12 @@ class tool_uploadcourse_course {
         $this->enrolmentdata = tool_uploadcourse_helper::get_enrolment_data($this->rawdata);
 
         // Restore data.
-        // TODO log warnings.
+        // TODO Speed up things by not really extracting the backup just yet, but checking that
+        // the backup file or shortname passed are valid. Extraction should happen in proceed().
         $this->restoredata = $this->get_restore_content_dir();
+        if ($this->restoredata === false) {
+            return false;
+        }
 
         // We can only reset courses when allowed and we are updating the course.
         if ($this->importoptions['reset'] || $this->options['reset']) {
index 3459c4a..8f86212 100644 (file)
@@ -708,6 +708,46 @@ class tool_uploadcourse_course_testcase extends advanced_testcase {
         set_time_limit(0);
     }
 
+    public function test_restore_invalid_file() {
+        $this->resetAfterTest();
+
+        // Restore from a non-existing file should not be allowed.
+        $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
+        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
+        $data = array('shortname' => 'A1', 'backupfile' => '/lead/no/where',
+            'category' => 1, 'fullname' => 'A1');
+        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
+        $this->assertFalse($co->prepare());
+        $this->assertArrayHasKey('cannotreadbackupfile', $co->get_errors());
+
+        // Restore from an invalid file should not be allowed.
+        $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
+        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
+        $data = array('shortname' => 'A1', 'backupfile' => __FILE__,
+            'category' => 1, 'fullname' => 'A1');
+        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
+
+        $this->assertFalse($co->prepare());
+        $this->assertArrayHasKey('invalidbackupfile', $co->get_errors());
+
+        // Zip packer throws a debugging message, this assertion is only here to prevent
+        // the message from being displayed.
+        $this->assertDebuggingCalled();
+    }
+
+    public function test_restore_invalid_course() {
+        $this->resetAfterTest();
+
+        // Restore from an invalid file should not be allowed.
+        $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
+        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
+        $data = array('shortname' => 'A1', 'templatecourse' => 'iamnotavalidcourse',
+            'category' => 1, 'fullname' => 'A1');
+        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
+        $this->assertFalse($co->prepare());
+        $this->assertArrayHasKey('coursetorestorefromdoesnotexist', $co->get_errors());
+    }
+
     /**
      * Testing the reset on groups, group members and enrolments.
      */