From 236a52c4dedbadb51c01d3f6858de4572bbdac73 Mon Sep 17 00:00:00 2001 From: Neill Magill <neill.magill@nottingham.ac.uk> Date: Tue, 18 Dec 2018 12:32:08 +0000 Subject: [PATCH 1/4] MDL-64427 category: Test getting category courses during deletion Adding a unit test for the situation where a course is partially deleted when a list of courses in a category is retrived. --- course/tests/category_test.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/course/tests/category_test.php b/course/tests/category_test.php index 54899398c73..8d8b26be765 100644 --- a/course/tests/category_test.php +++ b/course/tests/category_test.php @@ -1080,4 +1080,26 @@ class core_course_category_testcase extends advanced_testcase { } return $draftid; } + + /** + * This test ensures that is the list of courses in a category can be retrieved while a course is being deleted. + */ + public function test_get_courses_during_delete() { + global $DB; + $category = self::getDataGenerator()->create_category(); + $course = self::getDataGenerator()->create_course(['category' => $category->id]); + $othercourse = self::getDataGenerator()->create_course(['category' => $category->id]); + $coursecategory = core_course_category::get($category->id); + // Get a list of courses before deletion to populate the cache. + $originalcourses = $coursecategory->get_courses(); + $this->assertCount(2, $originalcourses); + $this->assertArrayHasKey($course->id, $originalcourses); + $this->assertArrayHasKey($othercourse->id, $originalcourses); + // Simulate the course deletion process being part way though. + $DB->delete_records('course', ['id' => $course->id]); + // Get the list of courses while a deletion is in progress. + $courses = $coursecategory->get_courses(); + $this->assertCount(1, $courses); + $this->assertArrayHasKey($othercourse->id, $courses); + } } From 7289f5c0c1912c370486cbf6ac485b869cb6a3a4 Mon Sep 17 00:00:00 2001 From: Neill Magill <neill.magill@nottingham.ac.uk> Date: Tue, 18 Dec 2018 11:16:03 +0000 Subject: [PATCH 2/4] MDL-64427 category: Stop error when another user deleting courses If you try to visit a category where another user is deleting a course the coursecat cache may not be fresh. This is because there is a breif time where the course record will have been deleted, while it is deleting other course information, before the event that triggers the coursecat cache to be purged is fired. --- course/classes/category.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/course/classes/category.php b/course/classes/category.php index 5812abdf133..15eb32a60d8 100644 --- a/course/classes/category.php +++ b/course/classes/category.php @@ -1582,7 +1582,10 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr } // Prepare the list of core_course_list_element objects. foreach ($ids as $id) { - $courses[$id] = new core_course_list_element($records[$id]); + // If a course is deleted after we got the cache entry it may not exist in the database anymore. + if (!empty($records[$id])) { + $courses[$id] = new core_course_list_element($records[$id]); + } } } return $courses; @@ -1792,7 +1795,10 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr } // Prepare the list of core_course_list_element objects. foreach ($ids as $id) { - $courses[$id] = new core_course_list_element($records[$id]); + // If a course is deleted after we got the cache entry it may not exist in the database anymore. + if (!empty($records[$id])) { + $courses[$id] = new core_course_list_element($records[$id]); + } } } return $courses; From d92acd5dcc4cb420b4a96f46dbb66c153a3f0c89 Mon Sep 17 00:00:00 2001 From: Neill Magill <neill.magill@nottingham.ac.uk> Date: Wed, 20 Feb 2019 16:12:53 +0000 Subject: [PATCH 3/4] MDL-64427 course: Do not allow dirty cm_info during course delete When deleting a course (especially one containing a large amount of data) the course mod info cache could contain entries for deleted activities for a signifcant amount of time making it possible that users could see errors in Moodle. --- lib/moodlelib.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 6f4a6c08927..eb5147810f2 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -5287,6 +5287,7 @@ function remove_course_contents($courseid, $showfeedback = true, array $options // Delete cm and its context - orphaned contexts are purged in cron in case of any race condition. context_helper::delete_instance(CONTEXT_MODULE, $cm->id); $DB->delete_records('course_modules', array('id' => $cm->id)); + rebuild_course_cache($cm->course, true); } } } @@ -5323,6 +5324,7 @@ function remove_course_contents($courseid, $showfeedback = true, array $options } context_helper::delete_instance(CONTEXT_MODULE, $cm->id); $DB->delete_records('course_modules', array('id' => $cm->id)); + rebuild_course_cache($cm->course, true); } if ($showfeedback) { From b26efcd8619ee4a78908d7d41c1d18b9d7f4e52b Mon Sep 17 00:00:00 2001 From: Neill Magill <neill.magill@nottingham.ac.uk> Date: Thu, 28 Feb 2019 10:09:04 +0000 Subject: [PATCH 4/4] MDL-64427 course: Mark activities during course contents deletion When we want to delete all of the activities in a course we should mark them as having a deletion in progress, as this makes them unavailable to users immediately. This should make it much less likely that a request to get a context that does not exist should be made when a user accesses a page. --- lib/moodlelib.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/moodlelib.php b/lib/moodlelib.php index eb5147810f2..cb91da2f556 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -5245,6 +5245,9 @@ function remove_course_contents($courseid, $showfeedback = true, array $options echo $OUTPUT->notification($strdeleted.get_string('type_block_plural', 'plugin'), 'notifysuccess'); } + $DB->set_field('course_modules', 'deletioninprogress', '1', ['course' => $courseid]); + rebuild_course_cache($courseid, true); + // Get the list of all modules that are properly installed. $allmodules = $DB->get_records_menu('modules', array(), '', 'name, id');