From 72e7ac5d4cc2794d538989060accebd8d5bc4d4c Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Tue, 1 Dec 2015 14:25:56 -0500 Subject: [PATCH] do orphan deletion in a bulk operation (SUST-21) --- .../contentstore/tests/test_orphan.py | 20 ++++++++++++------- cms/djangoapps/contentstore/views/item.py | 13 ++++++------ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_orphan.py b/cms/djangoapps/contentstore/tests/test_orphan.py index 83590847a7..519276cbc2 100644 --- a/cms/djangoapps/contentstore/tests/test_orphan.py +++ b/cms/djangoapps/contentstore/tests/test_orphan.py @@ -10,7 +10,7 @@ from opaque_keys.edx.locator import BlockUsageLocator from student.models import CourseEnrollment from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.search import path_to_location -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls_range class TestOrphanBase(CourseTestCase): @@ -93,21 +93,27 @@ class TestOrphan(TestOrphanBase): ) self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) location = course.location.replace(category='chapter', name='OrphanChapter') - self.assertIn(location.to_deprecated_string(), orphans) + self.assertIn(unicode(location), orphans) location = course.location.replace(category='vertical', name='OrphanVert') - self.assertIn(location.to_deprecated_string(), orphans) + self.assertIn(unicode(location), orphans) location = course.location.replace(category='html', name='OrphanHtml') - self.assertIn(location.to_deprecated_string(), orphans) + self.assertIn(unicode(location), orphans) - @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) - def test_delete_orphans(self, default_store): + @ddt.data( + (ModuleStoreEnum.Type.split, 9, 6), + (ModuleStoreEnum.Type.mongo, 30, 13), + ) + @ddt.unpack + def test_delete_orphans(self, default_store, max_mongo_calls, min_mongo_calls): """ Test that the orphan handler deletes the orphans """ course = self.create_course_with_orphans(default_store) orphan_url = reverse_course_url('orphan_handler', course.id) - self.client.delete(orphan_url) + with check_mongo_calls_range(max_mongo_calls, min_mongo_calls): + self.client.delete(orphan_url) + orphans = json.loads( self.client.get(orphan_url, HTTP_ACCEPT='application/json').content ) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 824d1bda76..40f910cb0a 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -693,12 +693,13 @@ def _delete_orphans(course_usage_key, user_id, commit=False): items = store.get_orphans(course_usage_key) branch = course_usage_key.branch if commit: - for itemloc in items: - revision = ModuleStoreEnum.RevisionOption.all - # specify branches when deleting orphans - if branch == ModuleStoreEnum.BranchName.published: - revision = ModuleStoreEnum.RevisionOption.published_only - store.delete_item(itemloc, user_id, revision=revision) + with store.bulk_operations(course_usage_key): + for itemloc in items: + revision = ModuleStoreEnum.RevisionOption.all + # specify branches when deleting orphans + if branch == ModuleStoreEnum.BranchName.published: + revision = ModuleStoreEnum.RevisionOption.published_only + store.delete_item(itemloc, user_id, revision=revision) return [unicode(item) for item in items]