From 1e21aeb97c283efbeb556ec184797b551cc651c2 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Fri, 29 Mar 2019 20:44:16 +0100 Subject: [PATCH 1/3] MDL-65218 recycle bin: Course backups (cat bin) to use MODE_AUTOMATED Surely this should have been done as part of MDL-63263, but it wasn't. Better let's do it here so every recycle bin backup is using the very same mode. --- admin/tool/recyclebin/classes/category_bin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/admin/tool/recyclebin/classes/category_bin.php b/admin/tool/recyclebin/classes/category_bin.php index b08b6a5f5bb..3c80f6b6906 100644 --- a/admin/tool/recyclebin/classes/category_bin.php +++ b/admin/tool/recyclebin/classes/category_bin.php @@ -115,7 +115,7 @@ class category_bin extends base_bin { $course->id, \backup::FORMAT_MOODLE, \backup::INTERACTIVE_NO, - \backup::MODE_GENERAL, + \backup::MODE_AUTOMATED, $user->id ); $controller->execute_plan(); @@ -233,7 +233,7 @@ class category_bin extends base_bin { $tempdir, $course->id, \backup::INTERACTIVE_NO, - \backup::MODE_GENERAL, + \backup::MODE_AUTOMATED, $user->id, \backup::TARGET_NEW_COURSE ); From 5e3ce4ef7dfa004c4cab1775978371ba9065d968 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Fri, 29 Mar 2019 20:49:52 +0100 Subject: [PATCH 2/3] MDL-65218 recycle bin: immunize recycle bin from backup_auto_storage As far as recycle bin is using MODE_AUTOMATED, it observes the backup_auto_storage setting (storing backups @ real location. For recycle bin we want to ensure that backup files are always stored in Moodle file area. In order to achieve that, we hack the setting here via $CFG->forced_plugin_settings, so it won't interfere other operations. See MDL-65218 for more information. This hack will be removed once recycle bin switches to use its own backup mode, with own preferences and 100% appart from MODLE_AUTOMATED. --- admin/tool/recyclebin/classes/category_bin.php | 14 ++++++++++++++ admin/tool/recyclebin/classes/course_bin.php | 16 +++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/admin/tool/recyclebin/classes/category_bin.php b/admin/tool/recyclebin/classes/category_bin.php index 3c80f6b6906..6477c4d25c2 100644 --- a/admin/tool/recyclebin/classes/category_bin.php +++ b/admin/tool/recyclebin/classes/category_bin.php @@ -108,6 +108,16 @@ class category_bin extends base_bin { require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); + // As far as recycle bin is using MODE_AUTOMATED, it observes the backup_auto_storage + // setting (storing backups @ real location. For recycle bin we want to ensure that + // backup files are always stored in Moodle file area. In order to achieve that, we + // hack the setting here via $CFG->forced_plugin_settings, so it won't interfere other + // operations. See MDL-65218 for more information. + // This hack will be removed once recycle bin switches to use its own backup mode, with + // own preferences and 100% appart from MODLE_AUTOMATED. + // TODO: Remove this as part of MDL-65228. + $CFG->forced_plugin_settings['backup'] = ['backup_auto_storage' => 0]; + // Backup the course. $user = get_admin(); $controller = new \backup_controller( @@ -120,6 +130,10 @@ class category_bin extends base_bin { ); $controller->execute_plan(); + // We don't need the forced setting anymore, hence unsetting it. + // TODO: Remove this as part of MDL-65228. + unset($CFG->forced_plugin_settings['backup']); + // Grab the result. $result = $controller->get_results(); if (!isset($result['backup_destination'])) { diff --git a/admin/tool/recyclebin/classes/course_bin.php b/admin/tool/recyclebin/classes/course_bin.php index d58d1a77f7c..deec65c1cb2 100644 --- a/admin/tool/recyclebin/classes/course_bin.php +++ b/admin/tool/recyclebin/classes/course_bin.php @@ -112,6 +112,16 @@ class course_bin extends base_bin { return; } + // As far as recycle bin is using MODE_AUTOMATED, it observes the backup_auto_storage + // setting (storing backups @ real location. For recycle bin we want to ensure that + // backup files are always stored in Moodle file area. In order to achieve that, we + // hack the setting here via $CFG->forced_plugin_settings, so it won't interfere other + // operations. See MDL-65218 for more information. + // This hack will be removed once recycle bin switches to use its own backup mode, with + // own preferences and 100% appart from MODLE_AUTOMATED. + // TODO: Remove this as part of MDL-65228. + $CFG->forced_plugin_settings['backup'] = ['backup_auto_storage' => 0]; + // Backup the activity. $user = get_admin(); $controller = new \backup_controller( @@ -124,6 +134,10 @@ class course_bin extends base_bin { ); $controller->execute_plan(); + // We don't need the forced setting anymore, hence unsetting it. + // TODO: Remove this as part of MDL-65228. + unset($CFG->forced_plugin_settings['backup']); + // Grab the result. $result = $controller->get_results(); if (!isset($result['backup_destination'])) { @@ -344,4 +358,4 @@ class course_bin extends base_bin { $context = \context_course::instance($this->_courseid); return has_capability('tool/recyclebin:deleteitems', $context); } -} \ No newline at end of file +} From 71c0f9c05569056a0c86eab60423d63552dbccca Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Fri, 29 Mar 2019 21:26:49 +0100 Subject: [PATCH 3/3] MDL-65218 recycle bin: cover immunization via settings provider Both for course and category recycle bins --- .../recyclebin/tests/category_bin_test.php | 112 ++++++++++++++++++ .../tool/recyclebin/tests/course_bin_test.php | 54 ++++++++- 2 files changed, 163 insertions(+), 3 deletions(-) diff --git a/admin/tool/recyclebin/tests/category_bin_test.php b/admin/tool/recyclebin/tests/category_bin_test.php index 22c7f216c35..06f9df9840f 100644 --- a/admin/tool/recyclebin/tests/category_bin_test.php +++ b/admin/tool/recyclebin/tests/category_bin_test.php @@ -176,4 +176,116 @@ class tool_recyclebin_category_bin_tests extends advanced_testcase { $course = reset($courses); $this->assertEquals('Test course 2', $course->fullname); } + + /** + * Provider for test_course_restore_with_userdata() and test_course_restore_without_userdata() + * + * Used to verify that recycle bin is immune to various settings. Provides plugin, name, value for + * direct usage with set_config() + */ + public function recycle_bin_settings_provider() { + return [ + 'backup/backup_auto_storage moodle' => [[ + (object)['plugin' => 'backup', 'name' => 'backup_auto_storage', 'value' => 0], + ]], + + 'backup/backup_auto_storage external' => [[ + (object)['plugin' => 'backup', 'name' => 'backup_auto_storage', 'value' => 1], + (object)['plugin' => 'backup', 'name' => 'backup_auto_destination', 'value' => true], + ]], + + 'backup/backup_auto_storage mixed' => [[ + (object)['plugin' => 'backup', 'name' => 'backup_auto_storage', 'value' => 2], + (object)['plugin' => 'backup', 'name' => 'backup_auto_destination', 'value' => true], + ]], + ]; + } + + /** + * Tests that user data is restored when course is restored. + * + * @dataProvider recycle_bin_settings_provider + * @param array $settings array of plugin, name, value stdClass(). + */ + public function test_course_restore_with_userdata($settings) { + global $DB; + + // Force configuration changes from provider. + foreach ($settings as $setting) { + // Need to create a directory for backup_auto_destination. + if ($setting->plugin === 'backup' && $setting->name === 'backup_auto_destination' && $setting->value === true) { + $setting->value = make_request_directory(); + } + set_config($setting->name, $setting->value, $setting->plugin); + } + + // We want user data to be included for this test. + set_config('backup_auto_users', true, 'backup'); + + $student = $this->getDataGenerator()->create_and_enrol($this->course, 'student'); + + // Delete course. + delete_course($this->course, false); + $this->assertFalse($DB->record_exists('course', ['id' => $this->course->id])); + + // Verify there is now a backup @ cat recycle bin file area. + $recyclebin = new \tool_recyclebin\category_bin($this->course->category); + $this->assertEquals(1, count($recyclebin->get_items())); + + // Restore the recycle bin item. + $recyclebin->restore_item(current($recyclebin->get_items())); + + // Get the new course. + $newcourse = $DB->get_record('course', ['shortname' => $this->course->shortname], '*', MUST_EXIST); + + // Check that it was removed from the recycle bin. + $this->assertEquals(0, count($recyclebin->get_items())); + + // Verify that student DOES continue enrolled. + $this->assertTrue(is_enrolled(context_course::instance($newcourse->id), $student->id)); + } + + /** + * Tests that user data is not restored when course is restored. + * + * @dataProvider recycle_bin_settings_provider + * @param array $settings array of plugin, name, value stdClass(). + */ + public function test_course_restore_without_userdata($settings) { + global $DB; + + // Force configuration changes from provider. + foreach ($settings as $setting) { + // Need to create a directory for backup_auto_destination. + if ($setting->plugin === 'backup' && $setting->name === 'backup_auto_destination' && $setting->value === true) { + $setting->value = make_request_directory(); + } + set_config($setting->name, $setting->value, $setting->plugin); + } + + // We want user data to be included for this test. + set_config('backup_auto_users', false, 'backup'); + + $student = $this->getDataGenerator()->create_and_enrol($this->course, 'student'); + + // Delete course. + delete_course($this->course, false); + $this->assertFalse($DB->record_exists('course', ['id' => $this->course->id])); + + // Verify there is now a backup @ cat recycle bin file area. + $recyclebin = new \tool_recyclebin\category_bin($this->course->category); + $this->assertEquals(1, count($recyclebin->get_items())); + + // Restore the recycle bin item. + $recyclebin->restore_item(current($recyclebin->get_items())); + + // Get the new course. + $newcourse = $DB->get_record('course', ['shortname' => $this->course->shortname], '*', MUST_EXIST); + + // Check that it was removed from the recycle bin. + $this->assertEquals(0, count($recyclebin->get_items())); + + // Verify that student DOES NOT continue enrolled. + $this->assertFalse(is_enrolled(context_course::instance($newcourse->id), $student->id)); + } } diff --git a/admin/tool/recyclebin/tests/course_bin_test.php b/admin/tool/recyclebin/tests/course_bin_test.php index 9d993260c36..44f9db6cc06 100644 --- a/admin/tool/recyclebin/tests/course_bin_test.php +++ b/admin/tool/recyclebin/tests/course_bin_test.php @@ -175,9 +175,45 @@ class tool_recyclebin_course_bin_tests extends advanced_testcase { } /** - * Tests that user data is restored when module is restored. + * Provider for test_coursemodule_restore_with_userdata() and test_coursemodule_restore_without_userdata() + * + * Used to verify that recycle bin is immune to various settings. Provides plugin, name, value for + * direct usage with set_config() */ - public function test_coursemodule_restore_with_userdata() { + public function recycle_bin_settings_provider() { + return [ + 'backup/backup_auto_storage moodle' => [[ + (object)['plugin' => 'backup', 'name' => 'backup_auto_storage', 'value' => 0], + ]], + + 'backup/backup_auto_storage external' => [[ + (object)['plugin' => 'backup', 'name' => 'backup_auto_storage', 'value' => 1], + (object)['plugin' => 'backup', 'name' => 'backup_auto_destination', 'value' => true], + ]], + + 'backup/backup_auto_storage mixed' => [[ + (object)['plugin' => 'backup', 'name' => 'backup_auto_storage', 'value' => 2], + (object)['plugin' => 'backup', 'name' => 'backup_auto_destination', 'value' => true], + ]], + ]; + } + + /** + * Tests that user data is restored when module is restored. + * + * @dataProvider recycle_bin_settings_provider + * @param array $settings array of plugin, name, value stdClass(). + */ + public function test_coursemodule_restore_with_userdata($settings) { + // Force configuration changes from provider. + foreach ($settings as $setting) { + // Need to create a directory for backup_auto_destination. + if ($setting->plugin === 'backup' && $setting->name === 'backup_auto_destination' && $setting->value === true) { + $setting->value = make_request_directory(); + } + set_config($setting->name, $setting->value, $setting->plugin); + } + $student = $this->getDataGenerator()->create_and_enrol($this->course, 'student'); $this->setUser($student); @@ -211,8 +247,20 @@ class tool_recyclebin_course_bin_tests extends advanced_testcase { /** * Tests that user data is not restored when module is restored. + * + * @dataProvider recycle_bin_settings_provider + * @param array $settings array of plugin, name, value stdClass(). */ - public function test_coursemodule_restore_without_userdata() { + public function test_coursemodule_restore_without_userdata($settings) { + // Force configuration changes from provider. + foreach ($settings as $setting) { + // Need to create a directory for backup_auto_destination. + if ($setting->plugin === 'backup' && $setting->name === 'backup_auto_destination' && $setting->value === true) { + $setting->value = make_request_directory(); + } + set_config($setting->name, $setting->value, $setting->plugin); + } + $student = $this->getDataGenerator()->create_and_enrol($this->course, 'student'); $this->setUser($student);