From c11ea219972913ad2fa29e4ac7801088bc67b724 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Thu, 6 Jun 2013 15:36:42 +0800 Subject: [PATCH 1/4] MDL-38314 repository: Unit Tests for delete_all_for_context() --- repository/tests/repository_test.php | 82 ++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/repository/tests/repository_test.php b/repository/tests/repository_test.php index 9a965094648..b68f571a071 100644 --- a/repository/tests/repository_test.php +++ b/repository/tests/repository_test.php @@ -287,4 +287,86 @@ class repositorylib_testcase extends advanced_testcase { $this->assertTrue($notprivaterepo->check_capability()); } + function test_delete_all_for_context() { + global $DB; + $this->resetAfterTest(true); + + $this->setAdminUser(); + $course = $this->getDataGenerator()->create_course(); + $user = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->create_repository_type('flickr_public'); + $this->getDataGenerator()->create_repository_type('filesystem'); + $coursecontext = context_course::instance($course->id); + $usercontext = context_user::instance($user->id); + + // Creating course instances. + $repo = $this->getDataGenerator()->create_repository('flickr_public', array('contextid' => $coursecontext->id)); + $courserepo1 = repository::get_repository_by_id($repo->id, $coursecontext); + $this->assertEquals(1, $DB->count_records('repository_instances', array('contextid' => $coursecontext->id))); + + $repo = $this->getDataGenerator()->create_repository('filesystem', array('contextid' => $coursecontext->id)); + $courserepo2 = repository::get_repository_by_id($repo->id, $coursecontext); + $this->assertEquals(2, $DB->count_records('repository_instances', array('contextid' => $coursecontext->id))); + + // Creating user instances. + $repo = $this->getDataGenerator()->create_repository('flickr_public', array('contextid' => $usercontext->id)); + $userrepo1 = repository::get_repository_by_id($repo->id, $usercontext); + $this->assertEquals(1, $DB->count_records('repository_instances', array('contextid' => $usercontext->id))); + + $repo = $this->getDataGenerator()->create_repository('filesystem', array('contextid' => $usercontext->id)); + $userrepo2 = repository::get_repository_by_id($repo->id, $usercontext); + $this->assertEquals(2, $DB->count_records('repository_instances', array('contextid' => $usercontext->id))); + + // Simulation of course deletion. + repository::delete_all_for_context($coursecontext->id); + $this->assertEquals(0, $DB->count_records('repository_instances', array('contextid' => $coursecontext->id))); + $this->assertEquals(0, $DB->count_records('repository_instances', array('id' => $courserepo1->id))); + $this->assertEquals(0, $DB->count_records('repository_instances', array('id' => $courserepo2->id))); + $this->assertEquals(0, $DB->count_records('repository_instance_config', array('instanceid' => $courserepo1->id))); + $this->assertEquals(0, $DB->count_records('repository_instance_config', array('instanceid' => $courserepo2->id))); + + // Simulation of user deletion. + repository::delete_all_for_context($usercontext->id); + $this->assertEquals(0, $DB->count_records('repository_instances', array('contextid' => $usercontext->id))); + $this->assertEquals(0, $DB->count_records('repository_instances', array('id' => $userrepo1->id))); + $this->assertEquals(0, $DB->count_records('repository_instances', array('id' => $userrepo2->id))); + $this->assertEquals(0, $DB->count_records('repository_instance_config', array('instanceid' => $userrepo1->id))); + $this->assertEquals(0, $DB->count_records('repository_instance_config', array('instanceid' => $userrepo2->id))); + + // Checking deletion upon course context deletion. + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + $repo = $this->getDataGenerator()->create_repository('flickr_public', array('contextid' => $coursecontext->id)); + $courserepo = repository::get_repository_by_id($repo->id, $coursecontext); + $this->assertEquals(1, $DB->count_records('repository_instances', array('contextid' => $coursecontext->id))); + $coursecontext->delete(); + $this->assertEquals(0, $DB->count_records('repository_instances', array('contextid' => $coursecontext->id))); + + // Checking deletion upon user context deletion. + $user = $this->getDataGenerator()->create_user(); + $usercontext = context_user::instance($user->id); + $repo = $this->getDataGenerator()->create_repository('flickr_public', array('contextid' => $usercontext->id)); + $userrepo = repository::get_repository_by_id($repo->id, $usercontext); + $this->assertEquals(1, $DB->count_records('repository_instances', array('contextid' => $usercontext->id))); + $usercontext->delete(); + $this->assertEquals(0, $DB->count_records('repository_instances', array('contextid' => $usercontext->id))); + + // Checking deletion upon course deletion. + $course = $this->getDataGenerator()->create_course(); + $coursecontext = context_course::instance($course->id); + $repo = $this->getDataGenerator()->create_repository('flickr_public', array('contextid' => $coursecontext->id)); + $courserepo = repository::get_repository_by_id($repo->id, $coursecontext); + $this->assertEquals(1, $DB->count_records('repository_instances', array('contextid' => $coursecontext->id))); + delete_course($course, false); + $this->assertEquals(0, $DB->count_records('repository_instances', array('contextid' => $coursecontext->id))); + + // Checking deletion upon user deletion. + $user = $this->getDataGenerator()->create_user(); + $usercontext = context_user::instance($user->id); + $repo = $this->getDataGenerator()->create_repository('flickr_public', array('contextid' => $usercontext->id)); + $userrepo = repository::get_repository_by_id($repo->id, $usercontext); + $this->assertEquals(1, $DB->count_records('repository_instances', array('contextid' => $usercontext->id))); + delete_user($user); + $this->assertEquals(0, $DB->count_records('repository_instances', array('contextid' => $usercontext->id))); + } } From d8732ee8b751756dcc0ab42cb8e2f274c6a35eca Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Thu, 6 Jun 2013 11:50:58 +0800 Subject: [PATCH 2/4] MDL-38314 repository: Delete repository instances on context deletion --- lib/accesslib.php | 4 ++++ repository/lib.php | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/lib/accesslib.php b/lib/accesslib.php index 65bc9d86272..bf5653cf5b9 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -5249,6 +5249,10 @@ abstract class context extends stdClass implements IteratorAggregate { $fs = get_file_storage(); $fs->delete_area_files($this->_id); + // Delete all repository instances attached to this context. + require_once($CFG->dirroot . '/repository/lib.php'); + repository::delete_all_for_context($this->_id); + // delete all advanced grading data attached to this context require_once($CFG->dirroot.'/grade/grading/lib.php'); grading_manager::delete_all_for_context($this->_id); diff --git a/repository/lib.php b/repository/lib.php index 7ccb28603ba..7d8fa9b2062 100644 --- a/repository/lib.php +++ b/repository/lib.php @@ -1952,6 +1952,32 @@ abstract class repository { return true; } + /** + * Delete all the instances associated to a context. + * + * This method is intended to be a callback when deleting + * a course or a user to delete all the instances associated + * to their context. The usual way to delete a single instance + * is to use {@link self::delete()}. + * + * @param int $contextid context ID. + * @param boolean $downloadcontents true to convert references to hard copies. + * @return void + */ + final public static function delete_all_for_context($contextid, $downloadcontents = true) { + global $DB; + $repoids = $DB->get_fieldset_select('repository_instances', 'id', 'contextid = :contextid', array('contextid' => $contextid)); + if ($downloadcontents) { + foreach ($repoids as $repoid) { + $repo = repository::get_repository_by_id($repoid, $contextid); + $repo->convert_references_to_local(); + } + } + cache::make('core', 'repositories')->purge(); + $DB->delete_records_list('repository_instances', 'id', $repoids); + $DB->delete_records_list('repository_instance_config', 'instanceid', $repoids); + } + /** * Hide/Show a repository * From 22785530b9a17038e320a28fdf496d98ac9f3300 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Tue, 4 Jun 2013 15:56:25 +0800 Subject: [PATCH 3/4] MDL-38314 repository: Remove orphan repository instances --- lib/db/upgrade.php | 14 ++++++++++++++ version.php | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index cb30c60c291..207689fb5f3 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -1749,5 +1749,19 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2012120304.06); } + if ($oldversion < 2012120304.09) { + + // Remove orphan repository instances. + $sql = 'SELECT contextid FROM {repository_instances} ri + WHERE NOT EXISTS ( + SELECT id FROM {context} c + WHERE c.id = ri.contextid)'; + $ids = $DB->get_fieldset_sql($sql); + $DB->delete_records_list('repository_instances', 'contextid', $ids); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2012120304.09); + } + return true; } diff --git a/version.php b/version.php index e0c84868332..67c89b3e625 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2012120304.08; // 20121203 = branching date YYYYMMDD - do not modify! +$version = 2012120304.09; // 20121203 = branching date YYYYMMDD - do not modify! // RR = release increments - 00 in DEV branches // .XX = incremental changes From 7e51747ede13e9139f942aedb3bb127385ccfde8 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Wed, 3 Jul 2013 16:03:46 +0800 Subject: [PATCH 4/4] MDL-38314 repository: Improving performance of upgrade script --- lib/db/upgrade.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 207689fb5f3..588ed85729a 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -1752,12 +1752,17 @@ function xmldb_main_upgrade($oldversion) { if ($oldversion < 2012120304.09) { // Remove orphan repository instances. - $sql = 'SELECT contextid FROM {repository_instances} ri - WHERE NOT EXISTS ( - SELECT id FROM {context} c - WHERE c.id = ri.contextid)'; - $ids = $DB->get_fieldset_sql($sql); - $DB->delete_records_list('repository_instances', 'contextid', $ids); + if ($DB->get_dbfamily() === 'mysql') { + $sql = "DELETE {repository_instances} FROM {repository_instances} + LEFT JOIN {context} ON {context}.id = {repository_instances}.contextid + WHERE {context}.id IS NULL"; + } else { + $sql = "DELETE FROM {repository_instances} + WHERE NOT EXISTS ( + SELECT 'x' FROM {context} + WHERE {context}.id = {repository_instances}.contextid)"; + } + $DB->execute($sql); // Main savepoint reached. upgrade_main_savepoint(true, 2012120304.09);