diff --git a/common/lib/xmodule/xmodule/modulestore/draft_and_published.py b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py index ce565aafa0..fea45d7d7f 100644 --- a/common/lib/xmodule/xmodule/modulestore/draft_and_published.py +++ b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py @@ -101,10 +101,16 @@ class ModuleStoreDraftAndPublished(BranchSettingMixin): def convert_to_draft(self, location, user_id): raise NotImplementedError - def _unsupported_revision_error(self, allowed_revisions=None): + +class UnsupportedRevisionError(ValueError): + """ + This error is raised if a method is called with an unsupported revision parameter. + """ + def __init__(self, allowed_revisions=None): if not allowed_revisions: allowed_revisions = [ - None, ModuleStoreEnum.RevisionOption.published_only, ModuleStoreEnum.RevisionOption.draft_only + None, + ModuleStoreEnum.RevisionOption.published_only, + ModuleStoreEnum.RevisionOption.draft_only ] - return ValueError('revision not one of {}'.format(allowed_revisions)) - + super(UnsupportedRevisionError, self).__init__('revision not one of {}'.format(allowed_revisions)) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 5f6cdd51e6..7b955de8c3 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -20,6 +20,7 @@ from xmodule.modulestore.mongo.base import ( DIRECT_ONLY_CATEGORIES, SORT_REVISION_FAVOR_DRAFT ) from xmodule.modulestore.store_utilities import rewrite_nonportable_content_links +from xmodule.modulestore.draft_and_published import UnsupportedRevisionError log = logging.getLogger(__name__) @@ -97,7 +98,7 @@ class DraftModuleStore(MongoModuleStore): elif self.get_branch_setting() == ModuleStoreEnum.Branch.published_only: return get_published() - else: + elif revision is None: # could use a single query wildcarding revision and sorting by revision. would need to # use prefix form of to_deprecated_son try: @@ -107,6 +108,9 @@ class DraftModuleStore(MongoModuleStore): # otherwise, fall back to the published version return get_published() + else: + raise UnsupportedRevisionError() + def has_item(self, usage_key, revision=None): """ Returns True if location exists in this ModuleStore. @@ -127,13 +131,17 @@ class DraftModuleStore(MongoModuleStore): if revision == ModuleStoreEnum.RevisionOption.draft_only: return has_draft() - elif revision == ModuleStoreEnum.RevisionOption.published_only \ - or self.get_branch_setting() == ModuleStoreEnum.Branch.published_only: + elif ( + revision == ModuleStoreEnum.RevisionOption.published_only or + self.get_branch_setting() == ModuleStoreEnum.Branch.published_only + ): return has_published() - else: + elif revision is None: key = usage_key.to_deprecated_son(prefix='_id.') del key['_id.revision'] return self.collection.find(key).count() > 0 + else: + raise UnsupportedRevisionError() def delete_course(self, course_key, user_id): """ @@ -342,9 +350,11 @@ class DraftModuleStore(MongoModuleStore): elif revision == ModuleStoreEnum.RevisionOption.published_only \ or self.get_branch_setting() == ModuleStoreEnum.Branch.published_only: return published_items([]) - else: + elif revision is None: draft_items = draft_items() return draft_items + published_items(draft_items) + else: + raise UnsupportedRevisionError() def convert_to_draft(self, location, user_id): """ @@ -519,8 +529,12 @@ class DraftModuleStore(MongoModuleStore): elif revision is None: as_functions = [as_draft] else: - raise self._unsupported_revision_error( - [None, ModuleStoreEnum.RevisionOption.published_only, ModuleStoreEnum.RevisionOption.all] + raise UnsupportedRevisionError( + [ + None, + ModuleStoreEnum.RevisionOption.published_only, + ModuleStoreEnum.RevisionOption.all + ] ) self._delete_subtree(location, as_functions) 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 19a60411dd..2e8bd77bd0 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -5,7 +5,7 @@ Module for the dual-branch fall-back Draft->Published Versioning ModuleStore from ..exceptions import ItemNotFoundError from split import SplitMongoModuleStore from xmodule.modulestore import ModuleStoreEnum, PublishState -from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished +from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished, UnsupportedRevisionError from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES from xmodule.modulestore.exceptions import InsufficientSpecificationError @@ -77,8 +77,12 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS elif revision is None: branches_to_delete = [ModuleStoreEnum.BranchName.draft] else: - raise self._unsupported_revision_error( - [None, ModuleStoreEnum.RevisionOption.published_only, ModuleStoreEnum.RevisionOption.all] + raise UnsupportedRevisionError( + [ + None, + ModuleStoreEnum.RevisionOption.published_only, + ModuleStoreEnum.RevisionOption.all + ] ) for branch in branches_to_delete: @@ -95,7 +99,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS elif revision is None: return key else: - raise self._unsupported_revision_error() + raise UnsupportedRevisionError() def has_item(self, usage_key, revision=None): """ @@ -203,6 +207,10 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS return self._get_block_from_structure(course_structure, xblock.location.block_id) def get_version(block): + """ + Return the version of the given database representation of a block. + """ + #TODO: make this method a more generic helper return block['edit_info']['update_version'] draft_head = get_head(ModuleStoreEnum.BranchName.draft) 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 1376c5c365..3de206161d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -20,6 +20,7 @@ from django.conf import settings if not settings.configured: settings.configure() from xmodule.modulestore.mixed import MixedModuleStore +from xmodule.modulestore.draft_and_published import UnsupportedRevisionError @ddt.ddt @@ -255,6 +256,10 @@ class TestMixedModuleStore(unittest.TestCase): )) self.assertFalse(self.store.has_item(self.fake_location)) + # verify that an error is raised when the revision is not valid + with self.assertRaises(UnsupportedRevisionError): + self.store.has_item(self.fake_location, revision=ModuleStoreEnum.RevisionOption.draft_preferred) + @ddt.data('draft', 'split') def test_get_item(self, default_ms): self.initdb(default_ms) @@ -269,6 +274,10 @@ class TestMixedModuleStore(unittest.TestCase): with self.assertRaises(ItemNotFoundError): self.store.get_item(self.fake_location) + # verify that an error is raised when the revision is not valid + with self.assertRaises(UnsupportedRevisionError): + self.store.get_item(self.fake_location, revision=ModuleStoreEnum.RevisionOption.draft_preferred) + @ddt.data('draft', 'split') def test_get_items(self, default_ms): self.initdb(default_ms) @@ -279,6 +288,13 @@ class TestMixedModuleStore(unittest.TestCase): self.assertEqual(len(modules), 1) self.assertEqual(modules[0].location, course_locn) + # verify that an error is raised when the revision is not valid + with self.assertRaises(UnsupportedRevisionError): + self.store.get_items( + self.course_locations[self.MONGO_COURSEID].course_key, + revision=ModuleStoreEnum.RevisionOption.draft_preferred + ) + @ddt.data('draft', 'split') def test_update_item(self, default_ms): """ @@ -366,6 +382,14 @@ class TestMixedModuleStore(unittest.TestCase): self.user_id, private_vert.location, 'html', block_id='bug_leaf' ) + # verify that an error is raised when the revision is not valid + with self.assertRaises(UnsupportedRevisionError): + self.store.delete_item( + private_leaf.location, + self.user_id, + revision=ModuleStoreEnum.RevisionOption.draft_preferred + ) + self.store.publish(private_vert.location, self.user_id) private_leaf.display_name = 'change me' private_leaf = self.store.update_item(private_leaf, self.user_id) @@ -802,7 +826,7 @@ class TestMixedModuleStore(unittest.TestCase): wiki_courses ) - @ddt.data('draft') + @ddt.data('draft', 'split') def test_branch_setting(self, default_ms): """ Test the branch_setting context manager @@ -810,7 +834,7 @@ class TestMixedModuleStore(unittest.TestCase): self.initdb(default_ms) self._create_block_hierarchy() - # TODO - Remove these lines once LMS-2869 is implemented + # TODO - Remove these lines once LMS-11017 is implemented course_location = self.course_locations[self.MONGO_COURSEID] self.store.publish(course_location, self.user_id) problem_original_name = 'Problem_Original' @@ -820,7 +844,7 @@ class TestMixedModuleStore(unittest.TestCase): ) problem_location = problem.location.version_agnostic().for_branch(None) - # TODO - Uncomment out these lines once LMS-2869 is implemented + # TODO - Uncomment out these lines once LMS-11017 is implemented # problem_location = self.problem_x1a_1 # problem_original_name = 'Problem_x1a_1' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index e8370cc690..0554481435 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -12,8 +12,11 @@ import random from xblock.fields import Scope from xmodule.course_module import CourseDescriptor from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.exceptions import (ItemNotFoundError, VersionConflictError, - DuplicateItemError, DuplicateCourseError, InsufficientSpecificationError) +from xmodule.modulestore.exceptions import ( + ItemNotFoundError, VersionConflictError, + DuplicateItemError, DuplicateCourseError, + InsufficientSpecificationError +) from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator, VersionTree, LocalId from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.x_module import XModuleMixin diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py index 8366413215..731e5dea7f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -122,4 +122,5 @@ class TestXMLModuleStore(unittest.TestCase): # XML store does NOT allow draft_preferred branch setting with self.assertRaises(ValueError): with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course_key): - pass + # verify that the above context manager raises a ValueError + pass # pragma: no cover