From 06ef306f7f6896b7e6f5b589c50a295b9fd802fe Mon Sep 17 00:00:00 2001 From: Mathew Peterson Date: Mon, 14 Jul 2014 18:10:16 +0000 Subject: [PATCH] more split_draft methods --- cms/djangoapps/contentstore/views/item.py | 4 +- .../xmodule/modulestore/split_migrator.py | 10 +- .../modulestore/split_mongo/split_draft.py | 16 +++- .../tests/test_mixed_modulestore.py | 92 ++++++++++++++----- .../xmodule/modulestore/tests/test_mongo.py | 2 +- .../xmodule/modulestore/tests/test_orphan.py | 52 ----------- .../tests/test_split_w_old_mongo.py | 4 +- .../lib/xmodule/xmodule/split_test_module.py | 2 +- 8 files changed, 94 insertions(+), 88 deletions(-) delete mode 100644 common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 00f49f48bf..b9532499c5 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -529,7 +529,7 @@ def orphan_handler(request, course_key_string): course_usage_key = CourseKey.from_string(course_key_string) if request.method == 'GET': if has_course_access(request.user, course_usage_key): - return JsonResponse(modulestore().get_orphans(course_usage_key)) + return JsonResponse([unicode(item) for item in modulestore().get_orphans(course_usage_key)]) else: raise PermissionDenied() if request.method == 'DELETE': @@ -539,7 +539,7 @@ def orphan_handler(request, course_key_string): for itemloc in items: # need to delete all versions store.delete_item(itemloc, request.user.id, revision=ModuleStoreEnum.RevisionOption.all) - return JsonResponse({'deleted': items}) + return JsonResponse({'deleted': [unicode(item) for item in items]}) else: raise PermissionDenied() diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index 6a841df398..c6aecdcb64 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -84,7 +84,9 @@ class SplitMigrator(object): # it doesn't need the parent as the first arg. That is, it translates and populates # the 'children' field as it goes. _new_module = self.split_modulestore.create_item( - user.id, new_locator, parent_location=course_version_locator, + user_id, + course_version_locator.make_usage_key(module.location.category, module.location.block_id), + parent_location=course_version_locator, block_id=module.location.block_id, fields=self._get_json_fields_translate_references( module, course_version_locator, new_course.location.block_id @@ -129,11 +131,7 @@ class SplitMigrator(object): else: # only a draft version (aka, 'private'). _new_module = self.split_modulestore.create_item( -<<<<<<< HEAD - new_draft_course_loc, module.category, user_id, -======= - user.id, new_locator, parent_location=new_draft_course_loc, ->>>>>>> converge create_item. + user_id, new_locator, parent_location=new_draft_course_loc, block_id=new_locator.block_id, fields=self._get_json_fields_translate_references( module, new_draft_course_loc, published_course_usage_key.block_id diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index f4862154c2..f19860e282 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -41,6 +41,20 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS for branch in branches_to_delete: SplitMongoModuleStore.delete_item(self, location.for_branch(branch), user_id, **kwargs) + def get_item(self, usage_key, depth=0, revision=None): + """ + Returns the item identified by usage_key and revision. + """ + def for_branch(branch_state): + if usage_key.branch is not None and usage_key.branch is not branch_state: + raise ValueError('{} already has a branch.'.format(usage_key)) + return usage_key.for_branch(branch_state) + if revision == ModuleStoreEnum.RevisionOption.published_only: + usage_key = for_branch(ModuleStoreEnum.BranchName.published) + elif revision == ModuleStoreEnum.RevisionOption.draft_only: + usage_key = for_branch(ModuleStoreEnum.BranchName.draft) + return super(DraftVersioningModuleStore, self).get_item(usage_key, depth=depth) + def get_parent_location(self, location, revision=None, **kwargs): # NAATODO - support draft_preferred return SplitMongoModuleStore.get_parent_location(self, location, **kwargs) @@ -77,7 +91,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS Deletes the published version of the item. Returns the newly unpublished item. """ - self.delete_item(location.for_branch(ModuleStoreEnum.BranchName.published), user_id) + self.delete_item(location, user_id, revision=ModuleStoreEnum.RevisionOption.published_only) return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.draft)) def revert_to_published(self, location, user_id): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index ad84818839..951d71c518 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -8,7 +8,7 @@ import unittest from xmodule.tests import DATA_DIR from opaque_keys.edx.locations import Location -from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore import ModuleStoreEnum, PublishState from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.exceptions import InvalidVersionError @@ -117,22 +117,20 @@ class TestMixedModuleStore(unittest.TestCase): """ Create a course w/ one item in the persistence store using the given course & item location. """ - course = self.store.create_course(course_key.org, course_key.course, course_key.run, self.user_id) + self.course = self.store.create_course(course_key.org, course_key.course, course_key.run, self.user_id) + self.writable_chapter_location = self.course.id.make_usage_key('chapter', 'Overview').version_agnostic() block_id = self.writable_chapter_location.name chapter = self.store.create_item( # don't use course_location as it may not be the repr self.user_id, self.writable_chapter_location, - parent_location=course.location, block_id=block_id + parent_location=self.course.location, block_id=block_id ) - if isinstance(course.id, CourseLocator): - self.course_locations[self.MONGO_COURSEID] = course.location.version_agnostic() - self.writable_chapter_location = chapter.location.version_agnostic() + if isinstance(self.course.id, CourseLocator): + self.course_locations[self.MONGO_COURSEID] = self.course.location.version_agnostic() else: - self.assertEqual(course.id, course_key) + self.assertEqual(self.course.id, course_key) self.assertEqual(chapter.location, self.writable_chapter_location) - self.course = course - def _create_block_hierarchy(self): """ Creates a hierarchy of blocks for testing @@ -150,6 +148,7 @@ class TestMixedModuleStore(unittest.TestCase): BlockInfo('problem_x1a_1', 'problem', 'Problem_x1a_1', []), BlockInfo('problem_x1a_2', 'problem', 'Problem_x1a_2', []), BlockInfo('problem_x1a_3', 'problem', 'Problem_x1a_3', []), + BlockInfo('html_x1a_1', 'html', 'HTML_x1a_1', []), ] ) ] @@ -175,13 +174,13 @@ class TestMixedModuleStore(unittest.TestCase): def create_sub_tree(parent, block_info): block = self.store.create_item( - self.user_id, parent_location=parent.location, + self.user_id, parent_location=parent.location.version_agnostic(), category=block_info.category, block_id=block_info.display_name ) for tree in block_info.sub_tree: create_sub_tree(block, tree) # reload the block to update its children field - block = self.store.get_item(block.location) + block = self.store.get_item(block.location.version_agnostic()) setattr(self, block_info.field_name, block) for tree in trees: @@ -209,7 +208,7 @@ class TestMixedModuleStore(unittest.TestCase): # convert to CourseKeys self.course_locations = { - course_id: SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_id: CourseLocator.from_string(course_id) for course_id in [self.MONGO_COURSEID, self.XML_COURSEID1, self.XML_COURSEID2] } # and then to the root UsageKey @@ -223,9 +222,6 @@ class TestMixedModuleStore(unittest.TestCase): ).make_usage_key('vertical', 'baz') else: self.fake_location = Location('foo', 'bar', 'slowly', 'vertical', 'baz') - self.writable_chapter_location = self.course_locations[self.MONGO_COURSEID].replace( - category='chapter', name='Overview' - ) self.xml_chapter_location = self.course_locations[self.XML_COURSEID1].replace( category='chapter', name='Overview' ) @@ -321,7 +317,7 @@ class TestMixedModuleStore(unittest.TestCase): self.store.delete_item(self.writable_chapter_location, self.user_id) # verify it's gone with self.assertRaises(ItemNotFoundError): - self.store.get_item(self.writable_chapter_location) + self.store.get_item(self.writable_chapter_location.version_agnostic()) # create and delete a private vertical with private children private_vert = self.store.create_item( @@ -520,22 +516,24 @@ class TestMixedModuleStore(unittest.TestCase): """ self.initdb(default_ms) self._create_block_hierarchy() + vertical_children_num = len(self.vertical_x1a.children) + self.store.publish(self.course.location, self.user_id) # delete leaf problem (will make parent vertical a draft) self.store.delete_item(self.problem_x1a_1.location, self.user_id) draft_parent = self.store.get_item(self.vertical_x1a.location) - self.assertEqual(2, len(draft_parent.children)) + self.assertEqual(vertical_children_num - 1, len(draft_parent.children)) published_parent = self.store.get_item( self.vertical_x1a.location, revision=ModuleStoreEnum.RevisionOption.published_only ) - self.assertEqual(3, len(published_parent.children)) + self.assertEqual(vertical_children_num, len(published_parent.children)) self.store.revert_to_published(self.vertical_x1a.location, self.user_id) reverted_parent = self.store.get_item(self.vertical_x1a.location) - self.assertEqual(3, len(published_parent.children)) + self.assertEqual(vertical_children_num, len(published_parent.children)) self.assertEqual(reverted_parent, published_parent) @ddt.data('draft') @@ -596,12 +594,28 @@ class TestMixedModuleStore(unittest.TestCase): @ddt.data('draft', 'split') def test_get_orphans(self, default_ms): self.initdb(default_ms) - # create an orphan course_id = self.course_locations[self.MONGO_COURSEID].course_key - orphan_location = course_id.make_usage_key('problem', 'orphan') - orphan = self.store.create_item(self.user_id, orphan_location) + + # orphans + orphan_locations = [ + course_id.make_usage_key('chapter', 'OrphanChapter'), + course_id.make_usage_key('vertical', 'OrphanVertical'), + course_id.make_usage_key('problem', 'OrphanProblem'), + course_id.make_usage_key('html', 'OrphanHTML'), + ] + + # detached items (not considered as orphans) + detached_locations = [ + course_id.make_usage_key('static_tab', 'StaticTab'), + course_id.make_usage_key('about', 'overview'), + course_id.make_usage_key('course_info', 'updates'), + ] + + for location in (orphan_locations + detached_locations): + self.store.create_item(self.user_id, location) + found_orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key) - self.assertEqual(found_orphans, [orphan_location]) + self.assertEqual(set(found_orphans), set(orphan_locations)) @ddt.data('draft') def test_create_item_from_parent_location(self, default_ms): @@ -634,6 +648,38 @@ class TestMixedModuleStore(unittest.TestCase): self.assertEqual(len(self.store.get_courses_for_wiki('edX.simple.2012_Fall')), 0) self.assertEqual(len(self.store.get_courses_for_wiki('no_such_wiki')), 0) + @ddt.data('draft', 'split') + def test_unpublish(self, default_ms): + """ + Test calling unpublish + """ + self.initdb(default_ms) + self._create_block_hierarchy() + # publish + self.store.publish(self.course.location, self.user_id) + published_xblock = self.store.get_item( + self.vertical_x1a.location.replace(branch=None), + revision=ModuleStoreEnum.RevisionOption.published_only + ) + self.assertIsNotNone(published_xblock) + + # unpublish + self.store.unpublish(self.vertical_x1a.location, self.user_id) + + with self.assertRaises(ItemNotFoundError): + self.store.get_item( + self.vertical_x1a.location.replace(branch=None), + revision=ModuleStoreEnum.RevisionOption.published_only + ) + + draft_xblock_from_get_item = self.store.get_item( + self.vertical_x1a.location.replace(branch=None), + revision=ModuleStoreEnum.RevisionOption.draft_only + ) + self.assertEquals( + published_xblock.location.version_agnostic().replace(branch=None), + draft_xblock_from_get_item.location.version_agnostic().replace(branch=None) + ) #============================================================================================================= # General utils for not using django settings diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 69a5285195..1dfb41d551 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -541,7 +541,7 @@ class TestMongoModuleStore(unittest.TestCase): location = Location('edX', 'missing', '2012_Fall', 'sequential', 'parent') # Create the parent and point it to a fake child - parent = self.draft_store.create_and_save_xmodule(location, user_id=self.dummy_user) + parent = self.draft_store.create_item(self.dummy_user, location) parent.children += [Location('edX', 'missing', '2012_Fall', 'vertical', 'does_not_exist')] self.draft_store.update_item(parent, self.dummy_user) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py deleted file mode 100644 index fb7366bfe7..0000000000 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py +++ /dev/null @@ -1,52 +0,0 @@ -from xmodule.modulestore.tests.test_split_w_old_mongo import SplitWMongoCourseBoostrapper - - -class TestOrphan(SplitWMongoCourseBoostrapper): - """ - Test the orphan finding code - """ - - def _create_course(self): - """ - * some detached items - * some attached children - * some orphans - """ - super(TestOrphan, self)._create_course() - - self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid') - self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid') - self._create_item('chapter', 'OrphanChapter', {}, {'display_name': 'Orphan Chapter'}, None, None) - self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1') - self._create_item('vertical', 'OrphanVert', {}, {'display_name': 'Orphan Vertical'}, None, None) - self._create_item('html', 'Html1', "

Goodbye

", {'display_name': 'Parented Html'}, 'vertical', 'Vert1') - self._create_item('html', 'OrphanHtml', "

Hello

", {'display_name': 'Orphan html'}, None, None) - self._create_item('static_tab', 'staticuno', "

tab

", {'display_name': 'Tab uno'}, None, None) - self._create_item('about', 'overview', "

overview

", {}, None, None) - self._create_item('course_info', 'updates', "

  1. Sep 22

    test

", {}, None, None) - - def test_mongo_orphan(self): - """ - Test that old mongo finds the orphans - """ - orphans = self.draft_mongo.get_orphans(self.old_course_key) - self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) - location = self.old_course_key.make_usage_key('chapter', 'OrphanChapter') - self.assertIn(location.to_deprecated_string(), orphans) - location = self.old_course_key.make_usage_key('vertical', 'OrphanVert') - self.assertIn(location.to_deprecated_string(), orphans) - location = self.old_course_key.make_usage_key('html', 'OrphanHtml') - self.assertIn(location.to_deprecated_string(), orphans) - - def test_split_orphan(self): - """ - Test that split mongo finds the orphans - """ - orphans = self.split_mongo.get_orphans(self.split_course_key) - self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) - location = self.split_course_key.make_usage_key('chapter', 'OrphanChapter') - self.assertIn(location, orphans) - location = self.split_course_key.make_usage_key('vertical', 'OrphanVert') - self.assertIn(location, orphans) - location = self.split_course_key.make_usage_key('html', 'OrphanHtml') - self.assertIn(location, orphans) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py index dc5c7d18f8..5e12bac3d8 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py @@ -86,8 +86,8 @@ class SplitWMongoCourseBoostrapper(unittest.TestCase): existing draft for both the new item and the parent """ location = self.old_course_key.make_usage_key(category, name) - self.draft_mongo.create_and_save_xmodule( - location, self.user_id, definition_data=data, metadata=metadata, runtime=self.runtime + self.draft_mongo.create_item( + self.user_id, location, definition_data=data, metadata=metadata, runtime=self.runtime ) if not draft: self.draft_mongo.publish(location, self.user_id) diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 061e32ba78..de08124e80 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -569,7 +569,7 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes This appends the new vertical to the end of children, and updates group_id_to_child. A mutable modulestore is needed to call this method (will need to update after mixed - modulestore work, currently relies on mongo's create_and_save_xmodule method). + modulestore work, currently relies on mongo's create_item method). """ assert hasattr(self.system, 'modulestore') and hasattr(self.system.modulestore, 'create_item'), \ "editor_saved should only be called when a mutable modulestore is available"