Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #9811 from edx/adam/delete-published-orphans-patch

Browse Source 
Adam/delete published orphans patch
This commit is contained in:
Adam
2015-09-17 11:54:41 -04:00
parent 99c410dc16 8aa23de55a
commit 788e54beba
6 changed files with 178 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

103
cms/djangoapps/contentstore/management/commands/tests/test_delete_orphans.py
View File

@@ -1,40 +1,111 @@
"""Tests running the delete_orphan command"""
import ddt
from django.core.management import call_command
from contentstore.tests.test_orphan import TestOrphanBase
from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore import ModuleStoreEnum
@ddt.ddt
class TestDeleteOrphan(TestOrphanBase):
"""
Tests for running the delete_orphan management command.
Inherits from TestOrphan in order to use its setUp method.
"""
def setUp(self):
super(TestDeleteOrphan, self).setUp()
self.course_id = self.course.id.to_deprecated_string()
def test_delete_orphans_no_commit(self):
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_delete_orphans_no_commit(self, default_store):
"""
Tests that running the command without a 'commit' argument
results in no orphans being deleted
"""
call_command('delete_orphans', self.course_id)
self.assertTrue(self.store.has_item(self.course.id.make_usage_key('html', 'multi_parent_html')))
self.assertTrue(self.store.has_item(self.course.id.make_usage_key('vertical', 'OrphanVert')))
self.assertTrue(self.store.has_item(self.course.id.make_usage_key('chapter', 'OrphanChapter')))
self.assertTrue(self.store.has_item(self.course.id.make_usage_key('html', 'OrphanHtml')))
course = self.create_course_with_orphans(default_store)
call_command('delete_orphans', unicode(course.id))
self.assertTrue(self.store.has_item(course.id.make_usage_key('html', 'multi_parent_html')))
self.assertTrue(self.store.has_item(course.id.make_usage_key('vertical', 'OrphanVert')))
self.assertTrue(self.store.has_item(course.id.make_usage_key('chapter', 'OrphanChapter')))
self.assertTrue(self.store.has_item(course.id.make_usage_key('html', 'OrphanHtml')))
def test_delete_orphans_commit(self):
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_delete_orphans_commit(self, default_store):
"""
Tests that running the command WITH the 'commit' argument
results in the orphans being deleted
"""
call_command('delete_orphans', self.course_id, 'commit')
course = self.create_course_with_orphans(default_store)
call_command('delete_orphans', unicode(course.id), 'commit')
# make sure this module wasn't deleted
self.assertTrue(self.store.has_item(self.course.id.make_usage_key('html', 'multi_parent_html')))
self.assertTrue(self.store.has_item(course.id.make_usage_key('html', 'multi_parent_html')))
# and make sure that these were
self.assertFalse(self.store.has_item(self.course.id.make_usage_key('vertical', 'OrphanVert')))
self.assertFalse(self.store.has_item(self.course.id.make_usage_key('chapter', 'OrphanChapter')))
self.assertFalse(self.store.has_item(self.course.id.make_usage_key('html', 'OrphanHtml')))
self.assertFalse(self.store.has_item(course.id.make_usage_key('vertical', 'OrphanVert')))
self.assertFalse(self.store.has_item(course.id.make_usage_key('chapter', 'OrphanChapter')))
self.assertFalse(self.store.has_item(course.id.make_usage_key('html', 'OrphanHtml')))
def test_delete_orphans_published_branch_split(self):
"""
Tests that if there are orphans only on the published branch,
running delete orphans with a course key that specifies
the published branch will delete the published orphan
"""
course, orphan = self.create_split_course_with_published_orphan()
published_branch = course.id.for_branch(ModuleStoreEnum.BranchName.published)
items_in_published = self.store.get_items(published_branch)
items_in_draft_preferred = self.store.get_items(course.id)
# call delete orphans, specifying the published branch
# of the course
call_command('delete_orphans', unicode(published_branch), 'commit')
# now all orphans should be deleted
self.assertOrphanCount(course.id, 0)
self.assertOrphanCount(published_branch, 0)
self.assertNotIn(orphan, self.store.get_items(published_branch))
# we should have one fewer item in the published branch of the course
self.assertEqual(
len(items_in_published) - 1,
len(self.store.get_items(published_branch)),
)
# and the same amount of items in the draft branch of the course
self.assertEqual(
len(items_in_draft_preferred),
len(self.store.get_items(course.id)),
)
def create_split_course_with_published_orphan(self):
"""
Helper to create a split course with a published orphan
"""
course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split)
# create an orphan
orphan = self.store.create_item(
self.user.id, course.id, 'html', "PublishedOnlyOrphan"
)
self.store.publish(orphan.location, self.user.id)
# grab the published branch of the course
published_branch = course.id.for_branch(
ModuleStoreEnum.BranchName.published
)
# assert that this orphan is present in both branches
self.assertOrphanCount(course.id, 1)
self.assertOrphanCount(published_branch, 1)
# delete this orphan from the draft branch without
# auto-publishing this change to the published branch
self.store.delete_item(
orphan.location, self.user.id, skip_auto_publish=True
)
# now there should be no orphans in the draft branch, but
# there should be one in published
self.assertOrphanCount(course.id, 0)
self.assertOrphanCount(published_branch, 1)
self.assertIn(orphan, self.store.get_items(published_branch))
return course, orphan

2
cms/djangoapps/contentstore/management/commands/tests/test_fix_not_found.py
View File

@@ -16,7 +16,7 @@ class TestFixNotFound(ModuleStoreTestCase):
"""
The management command doesn't work on non split courses
"""
course = CourseFactory(default_store=ModuleStoreEnum.Type.mongo)
course = CourseFactory.create(default_store=ModuleStoreEnum.Type.mongo)
with self.assertRaises(SystemExit):
call_command("fix_not_found", unicode(course.id))

81
cms/djangoapps/contentstore/tests/test_orphan.py
View File

@@ -2,27 +2,34 @@
Test finding orphans via the view and django config
"""
import json
import ddt
from contentstore.tests.utils import CourseTestCase
from student.models import CourseEnrollment
from contentstore.utils import reverse_course_url
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.factories import CourseFactory
class TestOrphanBase(CourseTestCase):
"""
Base class for Studio tests that require orphaned modules
"""
def setUp(self):
super(TestOrphanBase, self).setUp()
def create_course_with_orphans(self, default_store):
"""
Creates a course with 3 orphan modules, one of which
has a child that's also in the course tree.
"""
course = CourseFactory.create(default_store=default_store)
# create chapters and add them to course tree
chapter1 = self.store.create_child(self.user.id, self.course.location, 'chapter', "Chapter1")
chapter1 = self.store.create_child(self.user.id, course.location, 'chapter', "Chapter1")
self.store.publish(chapter1.location, self.user.id)
chapter2 = self.store.create_child(self.user.id, self.course.location, 'chapter', "Chapter2")
chapter2 = self.store.create_child(self.user.id, course.location, 'chapter', "Chapter2")
self.store.publish(chapter2.location, self.user.id)
# orphan chapter
orphan_chapter = self.store.create_item(self.user.id, self.course.id, 'chapter', "OrphanChapter")
orphan_chapter = self.store.create_item(self.user.id, course.id, 'chapter', "OrphanChapter")
self.store.publish(orphan_chapter.location, self.user.id)
# create vertical and add it as child to chapter1
@@ -30,7 +37,7 @@ class TestOrphanBase(CourseTestCase):
self.store.publish(vertical1.location, self.user.id)
# create orphan vertical
orphan_vertical = self.store.create_item(self.user.id, self.course.id, 'vertical', "OrphanVert")
orphan_vertical = self.store.create_item(self.user.id, course.id, 'vertical', "OrphanVert")
self.store.publish(orphan_vertical.location, self.user.id)
# create component and add it to vertical1
@@ -45,61 +52,79 @@ class TestOrphanBase(CourseTestCase):
self.store.update_item(orphan_vertical, self.user.id)
# create an orphaned html module
orphan_html = self.store.create_item(self.user.id, self.course.id, 'html', "OrphanHtml")
orphan_html = self.store.create_item(self.user.id, course.id, 'html', "OrphanHtml")
self.store.publish(orphan_html.location, self.user.id)
self.store.create_child(self.user.id, self.course.location, 'static_tab', "staticuno")
self.store.create_child(self.user.id, self.course.location, 'about', "overview")
self.store.create_child(self.user.id, self.course.location, 'course_info', "updates")
self.store.create_child(self.user.id, course.location, 'static_tab', "staticuno")
self.store.create_child(self.user.id, course.location, 'course_info', "updates")
return course
def assertOrphanCount(self, course_key, number):
"""
Asserts that we have the expected count of orphans
for a given course_key
"""
self.assertEqual(len(self.store.get_orphans(course_key)), number)
@ddt.ddt
class TestOrphan(TestOrphanBase):
"""
Test finding orphans via view and django config
"""
def setUp(self):
super(TestOrphan, self).setUp()
self.orphan_url = reverse_course_url('orphan_handler', self.course.id)
def test_mongo_orphan(self):
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_get_orphans(self, default_store):
"""
Test that old mongo finds the orphans
Test that the orphan handler finds the orphans
"""
course = self.create_course_with_orphans(default_store)
orphan_url = reverse_course_url('orphan_handler', course.id)
orphans = json.loads(
self.client.get(
self.orphan_url,
orphan_url,
HTTP_ACCEPT='application/json'
).content
)
self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
location = self.course.location.replace(category='chapter', name='OrphanChapter')
location = course.location.replace(category='chapter', name='OrphanChapter')
self.assertIn(location.to_deprecated_string(), orphans)
location = self.course.location.replace(category='vertical', name='OrphanVert')
location = course.location.replace(category='vertical', name='OrphanVert')
self.assertIn(location.to_deprecated_string(), orphans)
location = self.course.location.replace(category='html', name='OrphanHtml')
location = course.location.replace(category='html', name='OrphanHtml')
self.assertIn(location.to_deprecated_string(), orphans)
def test_mongo_orphan_delete(self):
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_delete_orphans(self, default_store):
"""
Test that old mongo deletes the orphans
Test that the orphan handler deletes the orphans
"""
self.client.delete(self.orphan_url)
course = self.create_course_with_orphans(default_store)
orphan_url = reverse_course_url('orphan_handler', course.id)
self.client.delete(orphan_url)
orphans = json.loads(
self.client.get(self.orphan_url, HTTP_ACCEPT='application/json').content
self.client.get(orphan_url, HTTP_ACCEPT='application/json').content
)
self.assertEqual(len(orphans), 0, "Orphans not deleted {}".format(orphans))
# make sure that any children with one orphan parent and one non-orphan
# parent are not deleted
self.assertTrue(self.store.has_item(self.course.id.make_usage_key('html', "multi_parent_html")))
self.assertTrue(self.store.has_item(course.id.make_usage_key('html', "multi_parent_html")))
def test_not_permitted(self):
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_not_permitted(self, default_store):
"""
Test that auth restricts get and delete appropriately
"""
course = self.create_course_with_orphans(default_store)
orphan_url = reverse_course_url('orphan_handler', course.id)
test_user_client, test_user = self.create_non_staff_authed_user_client()
CourseEnrollment.enroll(test_user, self.course.id)
response = test_user_client.get(self.orphan_url)
CourseEnrollment.enroll(test_user, course.id)
response = test_user_client.get(orphan_url)
self.assertEqual(response.status_code, 403)
response = test_user_client.delete(self.orphan_url)
response = test_user_client.delete(orphan_url)
self.assertEqual(response.status_code, 403)

8
cms/djangoapps/contentstore/views/item.py
View File

@@ -702,10 +702,14 @@ def _delete_orphans(course_usage_key, user_id, commit=False):
"""
store = modulestore()
items = store.get_orphans(course_usage_key)
branch = course_usage_key.branch
if commit:
for itemloc in items:
# need to delete all versions
store.delete_item(itemloc, user_id, revision=ModuleStoreEnum.RevisionOption.all)
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]

34
common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
View File

@@ -2416,14 +2416,36 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
return result
@contract(block_key=BlockKey, blocks='dict(BlockKey: BlockData)')
def _remove_subtree(self, block_key, blocks):
@contract(root_block_key=BlockKey, blocks='dict(BlockKey: BlockData)')
def _remove_subtree(self, root_block_key, blocks):
"""
Remove the subtree rooted at block_key
Remove the subtree rooted at root_block_key
We do this breadth-first to make sure that we don't remove
any children that may have parents that we don't want to delete.
"""
for child in blocks[block_key].fields.get('children', []):
self._remove_subtree(BlockKey(*child), blocks)
del blocks[block_key]
# create mapping from each child's key to its parents' keys
child_parent_map = defaultdict(set)
for block_key, block_data in blocks.iteritems():
for child in block_data.fields.get('children', []):
child_parent_map[BlockKey(*child)].add(block_key)
to_delete = {root_block_key}
tier = {root_block_key}
while tier:
next_tier = set()
for block_key in tier:
for child in blocks[block_key].fields.get('children', []):
child_block_key = BlockKey(*child)
parents = child_parent_map[child_block_key]
# Make sure we want to delete all of the child's parents
# before slating it for deletion
if parents.issubset(to_delete):
next_tier.add(child_block_key)
tier = next_tier
to_delete.update(tier)
for block_key in to_delete:
del blocks[block_key]
def delete_course(self, course_key, user_id):
"""

5
common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py
View File

@@ -175,7 +175,7 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
self._auto_publish_no_children(parent_usage_key, item.location.category, user_id, **kwargs)
return item
def delete_item(self, location, user_id, revision=None, **kwargs):
def delete_item(self, location, user_id, revision=None, skip_auto_publish=False, **kwargs):
"""
Delete the given item from persistence. kwargs allow modulestore specific parameters.
@@ -217,7 +217,8 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
if (
branch == ModuleStoreEnum.BranchName.draft and
branched_location.block_type in (DIRECT_ONLY_CATEGORIES + ['vertical']) and
parent_loc
parent_loc and
not skip_auto_publish
):
# will publish if its not an orphan
self.publish(parent_loc.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs)