diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 91236a4dade9..c5890b5b818b 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -529,7 +529,7 @@ def _import_xml_node_to_parent( node_copied_version = node.attrib.get('copied_from_version', None) # Modulestore's IdGenerator here is SplitMongoIdManager which is assigned - # by CachingDescriptorSystem Runtime and since we need our custom ImportIdGenerator + # by SplitModuleStoreRuntime and since we need our custom ImportIdGenerator # here we are temporaraliy swtiching it. original_id_generator = runtime.id_generator @@ -566,7 +566,8 @@ def _import_xml_node_to_parent( else: # We have to handle the children ourselves, because there are lots of complex interactions between # * the vanilla XBlock parse_xml() method, and its lack of API for "create and save a new XBlock" - # * the XmlMixin version of parse_xml() which only works with ImportSystem, not modulestore or the v2 runtime + # * the XmlMixin version of parse_xml() which only works with XMLImportingModuleStoreRuntime, + # not modulestore or the v2 runtime # * the modulestore APIs for creating and saving a new XBlock, which work but don't support XML parsing. # We can safely assume that if the XBLock class supports children, every child node will be the XML # serialization of a child block, in order. For blocks that don't support children, their XML content/nodes diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 2b21a9b9b970..bdab67798194 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -1855,7 +1855,7 @@ def setUp(self): @XBlockAside.register_temp_plugin(AsideTest, "test_aside") @patch( - "xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types", + "xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types", lambda self, block: ["test_aside"], ) def test_duplicate_equality_with_asides(self): @@ -2700,8 +2700,8 @@ def test_add_groups(self): group_id_to_child = split_test.group_id_to_child.copy() self.assertEqual(2, len(group_id_to_child)) - # CachingDescriptorSystem is used in tests. - # CachingDescriptorSystem doesn't have user service, that's needed for + # SplitModuleStoreRuntime is used in tests. + # SplitModuleStoreRuntime doesn't have user service, that's needed for # SplitTestBlock. So, in this line of code we add this service manually. split_test.runtime._services["user"] = DjangoXBlockUserService( # pylint: disable=protected-access self.user @@ -4449,7 +4449,7 @@ def test_self_paced_item_visibility_state(self): @patch( - "xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types", + "xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types", lambda self, block: ["test_aside"], ) class TestUpdateFromSource(ModuleStoreTestCase): diff --git a/common/djangoapps/static_replace/services.py b/common/djangoapps/static_replace/services.py index 80c284a9b526..010cb96be0d3 100644 --- a/common/djangoapps/static_replace/services.py +++ b/common/djangoapps/static_replace/services.py @@ -16,7 +16,7 @@ class ReplaceURLService(Service): A service for replacing static/course/jump-to-id URLs with absolute URLs in XBlocks. Args: - block: (optional) An XBlock instance. Used when retrieving the service from the DescriptorSystem. + block: (optional) An XBlock instance. Used when retrieving the service from the ModuleStoreRuntime. static_asset_path: (optional) Path for static assets, which overrides data_directory and course_id, if nonempty static_paths_out: (optional) Array to collect tuples for each static URI found: * the original unmodified static URI @@ -39,7 +39,7 @@ def __init__( self.jump_to_id_base_url = jump_to_id_base_url self.lookup_asset_url = lookup_asset_url # This is needed because the `Service` class initialization expects the XBlock passed as an `xblock` keyword - # argument, but the `service` method from the `DescriptorSystem` passes a `block`. + # argument, but the `service` method from the `ModuleStoreRuntime` passes a `block`. self._xblock = self.xblock() or block def replace_urls(self, text, static_replace_only=False): diff --git a/common/djangoapps/student/tests/test_course_listing.py b/common/djangoapps/student/tests/test_course_listing.py index 2f7b6c41d145..3034c697f2bb 100644 --- a/common/djangoapps/student/tests/test_course_listing.py +++ b/common/djangoapps/student/tests/test_course_listing.py @@ -108,7 +108,7 @@ def test_errored_course_regular_access(self): self._create_course_with_access_groups(course_key) with mock.patch( - 'xmodule.modulestore.split_mongo.caching_descriptor_system.SplitMongoKVS', mock.Mock(side_effect=Exception) + 'xmodule.modulestore.split_mongo.runtime.SplitMongoKVS', mock.Mock(side_effect=Exception) ): assert isinstance(modulestore().get_course(course_key), ErrorBlock) diff --git a/docs/decisions/0020-upstream-downstream.rst b/docs/decisions/0020-upstream-downstream.rst index 3f6bbacfb839..2b35ec8c6b8f 100644 --- a/docs/decisions/0020-upstream-downstream.rst +++ b/docs/decisions/0020-upstream-downstream.rst @@ -242,8 +242,7 @@ To support the Libraries Relaunch in Sumac: Video blocks. * We will define method(s) for syncing update on the XBlock runtime so that - they are available in the SplitModuleStore's XBlock Runtime - (CachingDescriptorSystem). + they are available in the SplitModuleStoreRuntime. * Either in the initial implementation or in a later implementation, it may make sense to declare abstract versions of the syncing method(s) higher up @@ -355,10 +354,10 @@ inheritance hierarchy of CachingDescriptorSystem and SplitModuleStoreRuntime.) ########################################################################### - # xmodule/modulestore/split_mongo/caching_descriptor_system.py + # xmodule/modulestore/split_mongo/runtime.py ########################################################################### - class CachingDescriptorSystem(...): + class SplitModuleStoreRuntime(...): def validate_upstream_key(self, usage_key: UsageKey | str) -> UsageKey: """ diff --git a/lms/djangoapps/course_blocks/tests/test_api.py b/lms/djangoapps/course_blocks/tests/test_api.py index 52e9c86ec324..62139c554982 100644 --- a/lms/djangoapps/course_blocks/tests/test_api.py +++ b/lms/djangoapps/course_blocks/tests/test_api.py @@ -20,7 +20,7 @@ def get_block_side_effect(block_locator, user_known): """ - Side effect for `CachingDescriptorSystem.get_block` + Side effect for `SplitModuleStoreRuntime.get_block` """ store = modulestore() course = store.get_course(block_locator.course_key) @@ -126,8 +126,8 @@ def test_get_course_blocks(self, group_id, expected_blocks, user_known): Access checks are done through the transformers and through Runtime get_block_for_descriptor. Due to the runtime limitations during the tests, the Runtime access checks are not performed as - get_block_for_descriptor is never called and Block is returned by CachingDescriptorSystem.get_block. - In this test, we mock the CachingDescriptorSystem.get_block and check block access for known and unknown users. + get_block_for_descriptor is never called and Block is returned by SplitModuleStoreRuntime.get_block. + In this test, we mock the SplitModuleStoreRuntime.get_block and check block access for known and unknown users. For known users, it performs the Runtime access checks through get_block_for_descriptor. For unknown, it skips the access checks. """ @@ -137,7 +137,7 @@ def test_get_course_blocks(self, group_id, expected_blocks, user_known): add_user_to_cohort(cohort, self.user.username) side_effect = get_block_side_effect_for_known_user if user_known else get_block_side_effect_for_unknown_user - with patch('xmodule.modulestore.split_mongo.split.CachingDescriptorSystem.get_block', side_effect=side_effect): + with patch('xmodule.modulestore.split_mongo.split.SplitModuleStoreRuntime.get_block', side_effect=side_effect): block_structure = get_course_blocks( self.user, self.course.location, diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index de92692ce4fc..6e83c3a1239d 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -122,7 +122,7 @@ class LmsModuleRenderError(Exception): def make_track_function(request): ''' Make a tracking function that logs what happened. - For use in DescriptorSystem. + For use in ModuleStoreRuntime. ''' from common.djangoapps.track import views as track_views diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 1cd8cf89037c..91212dcd4b09 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -66,7 +66,7 @@ class BaseTestXmodule(ModuleStoreTestCase): def new_module_runtime(self, runtime=None, **kwargs): """ - Generate a new DescriptorSystem that is minimally set up for testing + Generate a new ModuleStoreRuntime that is minimally set up for testing """ if runtime: return prepare_block_runtime(runtime, course_id=self.course.id, **kwargs) diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index 182f8d0c03ab..1de9db688f33 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -57,7 +57,7 @@ from xmodule.modulestore.tests.test_asides import AsideTestType # lint-amnesty, pylint: disable=wrong-import-order from xmodule.services import RebindUserServiceError from xmodule.video_block import VideoBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.x_module import STUDENT_VIEW, DescriptorSystem # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.x_module import STUDENT_VIEW, ModuleStoreRuntime # lint-amnesty, pylint: disable=wrong-import-order from common.djangoapps.course_modes.models import CourseMode # lint-amnesty, pylint: disable=reimported from common.djangoapps.student.tests.factories import ( BetaTesterFactory, @@ -461,8 +461,11 @@ def test_rebinding_same_user(self, block_type): @override_settings(FIELD_OVERRIDE_PROVIDERS=( 'lms.djangoapps.courseware.student_field_overrides.IndividualStudentOverrideProvider', )) - @patch('xmodule.modulestore.xml.ImportSystem.applicable_aside_types', lambda self, block: ['test_aside']) - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch( + 'xmodule.modulestore.xml.XMLImportingModuleStoreRuntime.applicable_aside_types', + lambda self, block: ['test_aside'] + ) + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside']) @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') @ddt.data('regular', 'test_aside') @@ -1920,7 +1923,7 @@ def _get_anonymous_id(self, course_id, xblock_class, should_get_deprecated_id: b location=location, static_asset_path=None, _runtime=Mock( - spec=DescriptorSystem, + spec=ModuleStoreRuntime, resources_fs=None, mixologist=Mock(_mixins=(), name='mixologist'), _services={}, @@ -1933,7 +1936,7 @@ def _get_anonymous_id(self, course_id, xblock_class, should_get_deprecated_id: b fields={}, days_early_for_beta=None, ) - block.runtime = DescriptorSystem(None, None, None) + block.runtime = ModuleStoreRuntime(None, None, None) # Use the xblock_class's bind_for_student method block.bind_for_student = partial(xblock_class.bind_for_student, block) @@ -2006,9 +2009,9 @@ def test_context_contains_display_name(self, mock_tracker): assert problem_display_name == block_info['display_name'] @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') - @patch('xmodule.modulestore.mongo.base.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.mongo.base.OldModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside']) - @patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', + @patch('xmodule.x_module.ModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside']) def test_context_contains_aside_info(self, mock_tracker): """ diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 48c91af671f7..a1829f4e0d81 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -47,7 +47,7 @@ from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE # noinspection PyUnresolvedReferences from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import -from xmodule.tests.test_import import DummySystem +from xmodule.tests.test_import import DummyModuleStoreRuntime from xmodule.tests.test_video import VideoBlockTestBase from xmodule.video_block import VideoBlock, bumper_utils, video_utils from xmodule.video_block.transcripts_utils import Transcript, save_to_store, subs_filename @@ -1989,7 +1989,7 @@ def test_import_val_data_internal(self): Test that import val data internal works as expected. """ create_profile('mobile') - module_system = DummySystem(load_error_blocks=True) + module_system = DummyModuleStoreRuntime(load_error_blocks=True) edx_video_id = 'test_edx_video_id' sub_id = '0CzPOIIdUsA' @@ -2095,7 +2095,7 @@ def test_import_no_video_id(self): """ xml_data = """""" xml_object = etree.fromstring(xml_data) - module_system = DummySystem(load_error_blocks=True) + module_system = DummyModuleStoreRuntime(load_error_blocks=True) # Verify edx_video_id is empty before. assert self.block.edx_video_id == '' @@ -2131,7 +2131,7 @@ def test_import_val_transcript(self): val_transcript_provider=val_transcript_provider ) xml_object = etree.fromstring(xml_data) - module_system = DummySystem(load_error_blocks=True) + module_system = DummyModuleStoreRuntime(load_error_blocks=True) # Create static directory in import file system and place transcript files inside it. module_system.resources_fs.makedirs(EXPORT_IMPORT_STATIC_DIR, recreate=True) @@ -2237,7 +2237,7 @@ def test_import_val_transcript_priority(self, sub_id, external_transcripts, val_ edx_video_id = 'test_edx_video_id' language_code = 'en' - module_system = DummySystem(load_error_blocks=True) + module_system = DummyModuleStoreRuntime(load_error_blocks=True) # Create static directory in import file system and place transcript files inside it. module_system.resources_fs.makedirs(EXPORT_IMPORT_STATIC_DIR, recreate=True) @@ -2306,7 +2306,7 @@ def test_import_val_transcript_priority(self, sub_id, external_transcripts, val_ def test_import_val_data_invalid(self): create_profile('mobile') - module_system = DummySystem(load_error_blocks=True) + module_system = DummyModuleStoreRuntime(load_error_blocks=True) # Negative file_size is invalid xml_data = """ diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index 6c9cf69f4931..5d29b37ba60a 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -11,7 +11,7 @@ from opaque_keys.edx.locations import BlockUsageLocator, CourseLocator from xblock.fields import ScopeIds -from xmodule.x_module import DescriptorSystem +from xmodule.x_module import ModuleStoreRuntime from lms.djangoapps.lms_xblock.runtime import handler_url @@ -51,7 +51,7 @@ class TestHandlerUrl(TestCase): def setUp(self): super().setUp() self.block = BlockMock(name='block') - self.runtime = DescriptorSystem( + self.runtime = ModuleStoreRuntime( load_item=Mock(name='get_test_descriptor_system.load_item'), resources_fs=Mock(name='get_test_descriptor_system.resources_fs'), error_tracker=Mock(name='get_test_descriptor_system.error_tracker') diff --git a/openedx/core/djangoapps/xblock/runtime/shims.py b/openedx/core/djangoapps/xblock/runtime/shims.py index c306b82bdc8b..578db22a5250 100644 --- a/openedx/core/djangoapps/xblock/runtime/shims.py +++ b/openedx/core/djangoapps/xblock/runtime/shims.py @@ -157,7 +157,7 @@ def process_xml(self, xml): """ # We can't parse XML in a vacuum - we need to know the parent block and/or the # OLX file that holds this XML in order to generate useful definition keys etc. - # The older ImportSystem runtime could do this because it stored the course_id + # The older XMLImportingModuleStoreRuntime runtime could do this because it stored the course_id # as part of the runtime. raise NotImplementedError("This newer runtime does not support process_xml()") @@ -244,7 +244,7 @@ def xqueue(self): def get_field_provenance(self, xblock, field): """ - A Studio-specific method that was implemented on DescriptorSystem. + A Studio-specific method that was implemented on ModuleStoreRuntime. Used by the problem block. For the given xblock, return a dict for the field's current state: diff --git a/openedx/core/lib/tests/test_xblock_utils.py b/openedx/core/lib/tests/test_xblock_utils.py index 8a10e0220dfb..7b817f3fe6fd 100644 --- a/openedx/core/lib/tests/test_xblock_utils.py +++ b/openedx/core/lib/tests/test_xblock_utils.py @@ -177,7 +177,10 @@ def test_is_not_xblock_aside(self): """test if xblock is not aside""" assert is_xblock_aside(self.block.scope_ids.usage_id) is False - @patch('xmodule.modulestore.xml.ImportSystem.applicable_aside_types', lambda self, block: ['test_aside']) + @patch( + 'xmodule.modulestore.xml.XMLImportingModuleStoreRuntime.applicable_aside_types', + lambda self, block: ['test_aside'], + ) @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') def test_get_aside(self): """test get aside success""" diff --git a/xmodule/capa/capa_problem.py b/xmodule/capa/capa_problem.py index 3bf5b78cd4a3..dfdf58857209 100644 --- a/xmodule/capa/capa_problem.py +++ b/xmodule/capa/capa_problem.py @@ -93,7 +93,7 @@ class LoncapaSystem(object): i18n: an object implementing the `gettext.Translations` interface so that we can use `.ugettext` to localize strings. - See :class:`DescriptorSystem` for documentation of other attributes. + See :class:`ModuleStoreRuntime` for documentation of other attributes. """ diff --git a/xmodule/error_block.py b/xmodule/error_block.py index a3620be87688..51c895a3e864 100644 --- a/xmodule/error_block.py +++ b/xmodule/error_block.py @@ -79,7 +79,7 @@ def _construct(cls, system, contents, error_msg, location, for_parent=None): Build a new ErrorBlock using ``system``. Arguments: - system (:class:`DescriptorSystem`): The :class:`DescriptorSystem` used + system (:class:`ModuleStoreRuntime`): The :class:`ModuleStoreRuntime` used to construct the XBlock that had an error. contents (unicode): An encoding of the content of the xblock that had an error. error_msg (unicode): A message describing the error. diff --git a/xmodule/mako_block.py b/xmodule/mako_block.py index f9e84d5e1907..ad9d72aa57df 100644 --- a/xmodule/mako_block.py +++ b/xmodule/mako_block.py @@ -5,30 +5,7 @@ from web_fragments.fragment import Fragment -from .x_module import DescriptorSystem, shim_xmodule_js - - -class MakoDescriptorSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstract-method - """ - Descriptor system that renders mako templates. - """ - def __init__(self, render_template, **kwargs): - super().__init__(**kwargs) - - self.render_template = render_template - - # Add the MakoService to the runtime services. - # If it already exists, do not attempt to reinitialize it; otherwise, this could override the `namespace_prefix` - # of the `MakoService`, breaking template rendering in Studio. - # - # This is not needed by most XBlocks, because the MakoService is added to their runtimes. - # However, there are a few cases where the MakoService is not added to the XBlock's - # runtime. Specifically: - # * in the Instructor Dashboard bulk emails tab, when rendering the HtmlBlock for its WYSIWYG editor. - # * during testing, when fetching factory-created blocks. - if 'mako' not in self._services: - from common.djangoapps.edxmako.services import MakoService - self._services['mako'] = MakoService() +from .x_module import shim_xmodule_js class MakoTemplateBlockBase: diff --git a/xmodule/modulestore/__init__.py b/xmodule/modulestore/__init__.py index 51118cc42e67..d52455641fb8 100644 --- a/xmodule/modulestore/__init__.py +++ b/xmodule/modulestore/__init__.py @@ -377,7 +377,7 @@ class EditInfo: def __init__(self, **kwargs): self.from_storable(kwargs) - # For details, see caching_descriptor_system.py get_subtree_edited_by/on. + # For details, see runtime.py get_subtree_edited_by/on. self._subtree_edited_on = kwargs.get('_subtree_edited_on', None) self._subtree_edited_by = kwargs.get('_subtree_edited_by', None) diff --git a/xmodule/modulestore/mongo/base.py b/xmodule/modulestore/mongo/base.py index 2b9b3031fab8..3ab0fd780e56 100644 --- a/xmodule/modulestore/mongo/base.py +++ b/xmodule/modulestore/mongo/base.py @@ -36,7 +36,6 @@ from xmodule.error_block import ErrorBlock from xmodule.errortracker import exc_info_to_str, null_error_tracker from xmodule.exceptions import HeartbeatFailure -from xmodule.mako_block import MakoDescriptorSystem from xmodule.modulestore import BulkOperationsMixin, ModuleStoreEnum, ModuleStoreWriteBase from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES, ModuleStoreDraftAndPublished from xmodule.modulestore.edit_info import EditInfoRuntimeMixin @@ -46,6 +45,7 @@ from xmodule.mongo_utils import connect_to_mongodb, create_collection_index from xmodule.partitions.partitions_service import PartitionService from xmodule.services import SettingsService +from xmodule.x_module import ModuleStoreRuntime log = logging.getLogger(__name__) @@ -146,19 +146,19 @@ def __repr__(self): ) -class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # lint-amnesty, pylint: disable=abstract-method +class OldModuleStoreRuntime(ModuleStoreRuntime, EditInfoRuntimeMixin): # pylint: disable=abstract-method """ A system that has a cache of block json that it will use to load blocks from, with a backup of calling to the underlying modulestore for more data """ - # This CachingDescriptorSystem runtime sets block._field_data on each block via construct_xblock_from_class(), + # This OldModuleStoreRuntime sets block._field_data on each block via construct_xblock_from_class(), # rather than the newer approach of providing a "field-data" service via runtime.service(). As a result, during # bind_for_student() we can't just set ._bound_field_data; we must overwrite block._field_data. uses_deprecated_field_data = True def __repr__(self): - return "CachingDescriptorSystem{!r}".format(( + return "{}{!r}".format(self.__class__.__name__, ( self.modulestore, str(self.course_id), [str(key) for key in self.module_data.keys()], @@ -177,12 +177,12 @@ def __init__(self, modulestore, course_key, module_data, default_class, **kwargs default_class: The default_class to use when loading an XModuleDescriptor from the module_data - resources_fs: a filesystem, as per MakoDescriptorSystem + resources_fs: a filesystem, as per ModuleStoreRuntime error_tracker: a function that logs errors for later display to users render_template: a function for rendering templates, as per - MakoDescriptorSystem + ModuleStoreRuntime """ id_manager = CourseLocationManager(course_key) kwargs.setdefault('id_reader', id_manager) @@ -660,7 +660,7 @@ def _load_item(self, course_key, item, data_cache, data_dir (optional): The directory name to use as the root data directory for this XModule data_cache (dict): A dictionary mapping from UsageKeys to xblock field data (this is the xblock data loaded from the database) - using_descriptor_system (CachingDescriptorSystem): The existing CachingDescriptorSystem + using_descriptor_system (OldModuleStoreRuntime): The existing runtime to add data to, and to load the XBlocks from. for_parent (:class:`XBlock`): The parent of the XBlock being loaded. """ @@ -687,7 +687,7 @@ def _load_item(self, course_key, item, data_cache, services["partitions"] = PartitionService(course_key) - system = CachingDescriptorSystem( + system = OldModuleStoreRuntime( modulestore=self, course_key=course_key, module_data=data_cache, @@ -922,7 +922,7 @@ def get_item(self, usage_key, using_descriptor_system=None, for_parent=None, **k descendents of the queried blocks for more efficient results later in the request. The depth is counted in the number of calls to get_children() to cache. None indicates to cache all descendents. - using_descriptor_system (CachingDescriptorSystem): The existing CachingDescriptorSystem + using_descriptor_system (SplitModuleStoreRuntime): The existing SplitModuleStoreRuntime to add data to, and to load the XBlocks from. """ item = self._find_one(usage_key) @@ -994,7 +994,7 @@ def get_items( # lint-amnesty, pylint: disable=arguments-differ For this modulestore, ``name`` is a commonly provided key (Location based stores) This modulestore does not allow searching dates by comparison or edited_by, previous_version, update_version info. - using_descriptor_system (CachingDescriptorSystem): The existing CachingDescriptorSystem + using_descriptor_system (SplitModuleStoreRuntime): The existing SplitModuleStoreRuntime to add data to, and to load the XBlocks from. """ qualifiers = qualifiers.copy() if qualifiers else {} # copy the qualifiers (destructively manipulated here) @@ -1104,7 +1104,7 @@ def create_xblock( services["partitions"] = PartitionService(course_key) - runtime = CachingDescriptorSystem( + runtime = OldModuleStoreRuntime( modulestore=self, module_data={}, course_key=course_key, diff --git a/xmodule/modulestore/mongo/draft.py b/xmodule/modulestore/mongo/draft.py index 5e171e36daf6..78031d04cf2a 100644 --- a/xmodule/modulestore/mongo/draft.py +++ b/xmodule/modulestore/mongo/draft.py @@ -73,7 +73,7 @@ def get_item(self, usage_key, revision=None, using_descriptor_system=None, **kwa Note: If the item is in DIRECT_ONLY_CATEGORIES, then returns only the PUBLISHED version regardless of the revision. - using_descriptor_system (CachingDescriptorSystem): The existing CachingDescriptorSystem + using_descriptor_system (ModuleStoreRuntime): The existing runtime to add data to, and to load the XBlocks from. Raises: diff --git a/xmodule/modulestore/split_mongo/id_manager.py b/xmodule/modulestore/split_mongo/id_manager.py index cdf5e6ed3af0..60dc1e0e095b 100644 --- a/xmodule/modulestore/split_mongo/id_manager.py +++ b/xmodule/modulestore/split_mongo/id_manager.py @@ -14,19 +14,19 @@ class SplitMongoIdManager(OpaqueKeyReader, AsideKeyGenerator): # pylint: disable=abstract-method """ An IdManager that knows how to retrieve the DefinitionLocator, given - a usage_id and a :class:`.CachingDescriptorSystem`. + a usage_id and a :class:`.SplitModuleStoreRuntime`. """ - def __init__(self, caching_descriptor_system): - self._cds = caching_descriptor_system + def __init__(self, runtime): + self._runtime = runtime def get_definition_id(self, usage_id): if isinstance(usage_id.block_id, LocalId): # a LocalId indicates that this block hasn't been persisted yet, and is instead stored # in-memory in the local_modules dictionary. - return self._cds.local_modules[usage_id].scope_ids.def_id + return self._runtime.local_modules[usage_id].scope_ids.def_id else: block_key = BlockKey.from_usage_key(usage_id) - module_data = self._cds.get_module_data(block_key, usage_id.course_key) + module_data = self._runtime.get_module_data(block_key, usage_id.course_key) if module_data.definition is not None: return DefinitionLocator(usage_id.block_type, module_data.definition) diff --git a/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/xmodule/modulestore/split_mongo/runtime.py similarity index 98% rename from xmodule/modulestore/split_mongo/caching_descriptor_system.py rename to xmodule/modulestore/split_mongo/runtime.py index a83fec32bac0..2ebd6fe805d4 100644 --- a/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/xmodule/modulestore/split_mongo/runtime.py @@ -14,7 +14,6 @@ from xmodule.error_block import ErrorBlock from xmodule.errortracker import exc_info_to_str from xmodule.library_tools import LegacyLibraryToolsService -from xmodule.mako_block import MakoDescriptorSystem from xmodule.modulestore import BlockData from xmodule.modulestore.edit_info import EditInfoRuntimeMixin from xmodule.modulestore.exceptions import ItemNotFoundError @@ -24,12 +23,12 @@ from xmodule.modulestore.split_mongo.id_manager import SplitMongoIdManager from xmodule.modulestore.split_mongo.split_mongo_kvs import SplitMongoKVS from xmodule.util.misc import get_library_or_course_attribute -from xmodule.x_module import XModuleMixin +from xmodule.x_module import XModuleMixin, ModuleStoreRuntime log = logging.getLogger(__name__) -class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # lint-amnesty, pylint: disable=abstract-method +class SplitModuleStoreRuntime(ModuleStoreRuntime, EditInfoRuntimeMixin): # pylint: disable=abstract-method """ A system that has a cache of a course version's json that it will use to load blocks from, with a backup of calling to the underlying modulestore for more data. diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index b2124c4025ec..07c985c21cc5 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -106,7 +106,7 @@ from xmodule.util.keys import BlockKey, derive_key from ..exceptions import ItemNotFoundError -from .caching_descriptor_system import CachingDescriptorSystem +from .runtime import SplitModuleStoreRuntime log = logging.getLogger(__name__) @@ -715,7 +715,7 @@ def cache_items(self, system, base_block_ids, course_key, depth=0, lazy=True): per course per fetch operations are done. Arguments: - system: a CachingDescriptorSystem + system: a SplitModuleStoreRuntime base_block_ids: list of BlockIds to fetch course_key: the destination course providing the context depth: how deep below these to prefetch @@ -3290,7 +3290,7 @@ def create_runtime(self, course_entry, lazy): if not isinstance(course_entry.course_key, LibraryLocator): services["partitions"] = PartitionService(course_entry.course_key) - return CachingDescriptorSystem( + return SplitModuleStoreRuntime( modulestore=self, course_entry=course_entry, module_data={}, diff --git a/xmodule/modulestore/tests/test_asides.py b/xmodule/modulestore/tests/test_asides.py index 052fc6637e61..49b7adff4fb8 100644 --- a/xmodule/modulestore/tests/test_asides.py +++ b/xmodule/modulestore/tests/test_asides.py @@ -33,7 +33,10 @@ class TestAsidesXmlStore(TestCase): Test Asides sourced from xml store """ - @patch('xmodule.modulestore.xml.ImportSystem.applicable_aside_types', lambda self, block: ['test_aside']) + @patch( + 'xmodule.modulestore.xml.XMLImportingModuleStoreRuntime.applicable_aside_types', + lambda self, block: ['test_aside'] + ) @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') def test_xml_aside(self): """ diff --git a/xmodule/modulestore/tests/test_mixed_modulestore.py b/xmodule/modulestore/tests/test_mixed_modulestore.py index f82d1e57c570..d6dd042ce39f 100644 --- a/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -3349,7 +3349,7 @@ def test_vertical_with_published_unit_remains_published_before_export_and_after_ @ddt.data(ModuleStoreEnum.Type.split) @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside']) def test_aside_crud(self, default_store): """ @@ -3423,7 +3423,7 @@ def check_block(block): @ddt.data(ModuleStoreEnum.Type.split) @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside']) def test_export_course_with_asides(self, default_store): if default_store == ModuleStoreEnum.Type.mongo: @@ -3510,7 +3510,7 @@ def check_block(block): @ddt.data(ModuleStoreEnum.Type.split) @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside']) def test_export_course_after_creating_new_items_with_asides(self, default_store): # pylint: disable=too-many-statements if default_store == ModuleStoreEnum.Type.mongo: @@ -3645,7 +3645,7 @@ def setUp(self): @ddt.data(ModuleStoreEnum.Type.split) @XBlockAside.register_temp_plugin(AsideFoo, 'test_aside1') @XBlockAside.register_temp_plugin(AsideBar, 'test_aside2') - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside1', 'test_aside2']) def test_get_and_update_asides(self, default_store): """ @@ -3709,7 +3709,7 @@ def _check_asides(asides, field11, field12, field21, field22): @ddt.data(ModuleStoreEnum.Type.split) @XBlockAside.register_temp_plugin(AsideFoo, 'test_aside1') - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside1']) def test_clone_course_with_asides(self, default_store): """ @@ -3757,7 +3757,7 @@ def test_clone_course_with_asides(self, default_store): @ddt.data(ModuleStoreEnum.Type.split) @XBlockAside.register_temp_plugin(AsideFoo, 'test_aside1') - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside1']) def test_delete_item_with_asides(self, default_store): """ @@ -3806,7 +3806,7 @@ def test_delete_item_with_asides(self, default_store): @ddt.data((ModuleStoreEnum.Type.split, 1, 0)) @XBlockAside.register_temp_plugin(AsideFoo, 'test_aside1') - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside1']) @ddt.unpack def test_published_and_unpublish_item_with_asides(self, default_store, max_find, max_send): diff --git a/xmodule/modulestore/tests/test_semantics.py b/xmodule/modulestore/tests/test_semantics.py index a50f19735ba4..34e3a2e543a7 100644 --- a/xmodule/modulestore/tests/test_semantics.py +++ b/xmodule/modulestore/tests/test_semantics.py @@ -415,14 +415,14 @@ class TestSplitDirectOnlyCategorySemantics(DirectOnlyCategorySemantics): @ddt.data(*TESTABLE_BLOCK_TYPES) @XBlockAside.register_temp_plugin(AsideTest, 'test_aside') - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside']) def test_create_with_asides(self, block_type): self._do_create(block_type, with_asides=True) @ddt.data(*TESTABLE_BLOCK_TYPES) @XBlockAside.register_temp_plugin(AsideTest, 'test_aside') - @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + @patch('xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.applicable_aside_types', lambda self, block: ['test_aside']) def test_update_asides(self, block_type): block_usage_key = self._do_create(block_type, with_asides=True) diff --git a/xmodule/modulestore/xml.py b/xmodule/modulestore/xml.py index 738035b19438..679612fde679 100644 --- a/xmodule/modulestore/xml.py +++ b/xmodule/modulestore/xml.py @@ -15,24 +15,29 @@ from fs.osfs import OSFS from lxml import etree -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryLocator from path import Path as path +from xblock.core import XBlockAside from xblock.field_data import DictFieldData -from xblock.fields import ScopeIds +from xblock.fields import ( + Reference, + ReferenceList, + ReferenceValueDict, + ScopeIds, +) from xblock.runtime import DictKeyValueStore from common.djangoapps.util.monitoring import monitor_import_failure from xmodule.error_block import ErrorBlock from xmodule.errortracker import exc_info_to_str, make_error_tracker -from xmodule.mako_block import MakoDescriptorSystem from xmodule.modulestore import COURSE_ROOT, LIBRARY_ROOT, ModuleStoreEnum, ModuleStoreReadBase from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS from xmodule.tabs import CourseTabList -from xmodule.x_module import ( # lint-amnesty, pylint: disable=unused-import +from xmodule.x_module import ( AsideKeyGenerator, OpaqueKeyReader, - XMLParsingSystem, + ModuleStoreRuntime, policy_key ) @@ -46,12 +51,129 @@ log = logging.getLogger(__name__) -class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring +class XMLParsingModuleStoreRuntime(ModuleStoreRuntime): + """ + ModuleStoreRuntime with some tweaks for XML processing. + """ + def __init__(self, process_xml, **kwargs): + """ + process_xml: Takes an xml string, and returns a XModuleDescriptor + created from that xml + """ + + super().__init__(**kwargs) + self.process_xml = process_xml + + def _usage_id_from_node(self, node, parent_id): + """Create a new usage id from an XML dom node. + + Args: + node (lxml.etree.Element): The DOM node to interpret. + parent_id: The usage ID of the parent block + Returns: + UsageKey: the usage key for the new xblock + """ + return self.xblock_from_node(node, parent_id, self.id_generator).scope_ids.usage_id + + def xblock_from_node(self, node, parent_id, id_generator=None): + """ + Create an XBlock instance from XML data. + + Args: + id_generator (IdGenerator): An :class:`~xblock.runtime.IdGenerator` that + will be used to construct the usage_id and definition_id for the block. + + Returns: + XBlock: The fully instantiated :class:`~xblock.core.XBlock`. + + """ + id_generator = id_generator or self.id_generator + # leave next line commented out - useful for low-level debugging + # log.debug('[_usage_id_from_node] tag=%s, class=%s' % (node.tag, xblock_class)) + + block_type = node.tag + # remove xblock-family from elements + node.attrib.pop('xblock-family', None) + + url_name = node.get('url_name') # difference from XBlock.runtime + def_id = id_generator.create_definition(block_type, url_name) + usage_id = id_generator.create_usage(def_id) + + keys = ScopeIds(None, block_type, def_id, usage_id) + block_class = self.mixologist.mix(self.load_block_type(block_type)) + + aside_children = self.parse_asides(node, def_id, usage_id, id_generator) + asides_tags = [x.tag for x in aside_children] + + block = block_class.parse_xml(node, self, keys) + self._convert_reference_fields_to_keys(block) # difference from XBlock.runtime + block.parent = parent_id + block.save() + + asides = self.get_asides(block) + for asd in asides: + if asd.scope_ids.block_type in asides_tags: + block.add_aside(asd) + + return block + + def parse_asides(self, node, def_id, usage_id, id_generator): + """pull the asides out of the xml payload and instantiate them""" + aside_children = [] + for child in node.iterchildren(): + # get xblock-family from node + xblock_family = child.attrib.pop('xblock-family', None) + if xblock_family: + xblock_family = self._family_id_to_superclass(xblock_family) + if issubclass(xblock_family, XBlockAside): + aside_children.append(child) + # now process them & remove them from the xml payload + for child in aside_children: + self._aside_from_xml(child, def_id, usage_id) + node.remove(child) + return aside_children + + def _make_usage_key(self, course_key, value): + """ + Makes value into a UsageKey inside the specified course. + If value is already a UsageKey, returns that. + """ + if isinstance(value, UsageKey): + return value + usage_key = UsageKey.from_string(value) + return usage_key.map_into_course(course_key) + + def _convert_reference_fields_to_keys(self, xblock): + """ + Find all fields of type reference and convert the payload into UsageKeys + """ + course_key = xblock.scope_ids.usage_id.course_key + + for field in xblock.fields.values(): + if field.is_set_on(xblock): + field_value = getattr(xblock, field.name) + if field_value is None: + continue + elif isinstance(field, Reference): + setattr(xblock, field.name, self._make_usage_key(course_key, field_value)) + elif isinstance(field, ReferenceList): + setattr(xblock, field.name, [self._make_usage_key(course_key, ele) for ele in field_value]) + elif isinstance(field, ReferenceValueDict): + for key, subvalue in field_value.items(): + assert isinstance(subvalue, str) + field_value[key] = self._make_usage_key(course_key, subvalue) + setattr(xblock, field.name, field_value) + + +class XMLImportingModuleStoreRuntime(XMLParsingModuleStoreRuntime): # pylint: disable=abstract-method + """ + A runtime for importing OLX into ModuleStore. + """ def __init__(self, xmlstore, course_id, course_dir, # lint-amnesty, pylint: disable=too-many-statements error_tracker, load_error_blocks=True, target_course_id=None, **kwargs): """ - A class that handles loading from xml. Does some munging to ensure that + A class that handles loading from xml to ModuleStore. Does some munging to ensure that all elements have unique slugs. xmlstore: the XMLModuleStore to store the loaded blocks in @@ -257,7 +379,7 @@ def construct_xblock_from_class(self, cls, scope_ids, field_data=None, *args, ** ) return super().construct_xblock_from_class(cls, scope_ids, field_data, *args, **kwargs) - # id_generator is ignored, because each ImportSystem is already local to + # id_generator is ignored, because each XMLImportingModuleStoreRuntime is already local to # a course, and has it's own id_generator already in place def add_node_as_child(self, block, node): # lint-amnesty, pylint: disable=signature-differs child_block = self.process_xml(etree.tostring(node)) @@ -532,7 +654,7 @@ def get_policy(usage_id): if self.user_service: services['user'] = self.user_service - system = ImportSystem( + system = XMLImportingModuleStoreRuntime( xmlstore=self, course_id=course_id, course_dir=course_dir, diff --git a/xmodule/modulestore/xml_importer.py b/xmodule/modulestore/xml_importer.py index a7d0909b288e..8ee98a8b7c43 100644 --- a/xmodule/modulestore/xml_importer.py +++ b/xmodule/modulestore/xml_importer.py @@ -52,7 +52,7 @@ from xmodule.modulestore.exceptions import DuplicateCourseError from xmodule.modulestore.mongo.base import MongoRevisionKey from xmodule.modulestore.store_utilities import draft_node_constructor, get_draft_subtree_roots -from xmodule.modulestore.xml import ImportSystem, LibraryXMLModuleStore, XMLModuleStore +from xmodule.modulestore.xml import XMLImportingModuleStoreRuntime, LibraryXMLModuleStore, XMLModuleStore from xmodule.tabs import CourseTabList from xmodule.util.misc import escape_invalid_characters from xmodule.x_module import XModuleMixin @@ -993,8 +993,8 @@ def _import_course_draft( # create a new 'System' object which will manage the importing errorlog = make_error_tracker() - # The course_dir as passed to ImportSystem is expected to just be relative, not - # the complete path including data_dir. ImportSystem will concatenate the two together. + # The course_dir as passed to XMLImportingModuleStoreRuntime is expected to just be relative, not + # the complete path including data_dir. XMLImportingModuleStoreRuntime will concatenate the two together. data_dir = xml_module_store.data_dir # Whether or not data_dir ends with a "/" differs in production vs. test. if not data_dir.endswith("/"): @@ -1002,7 +1002,7 @@ def _import_course_draft( # Remove absolute path, leaving relative /drafts. draft_course_dir = draft_dir.replace(data_dir, '', 1) - system = ImportSystem( + system = XMLImportingModuleStoreRuntime( xmlstore=xml_module_store, course_id=source_course_id, course_dir=draft_course_dir, diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index d8a203f34ba8..819c89669eba 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -24,14 +24,13 @@ from xmodule.capa.xqueue_interface import XQueueService from xmodule.assetstore import AssetMetadata from xmodule.contentstore.django import contentstore -from xmodule.mako_block import MakoDescriptorSystem from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.modulestore.xml import CourseLocationManager from xmodule.tests.helpers import StubReplaceURLService, mock_render_template, StubMakoService, StubUserService from xmodule.util.sandboxing import SandboxService -from xmodule.x_module import DoNothingCache, XModuleMixin +from xmodule.x_module import DoNothingCache, XModuleMixin, ModuleStoreRuntime from openedx.core.lib.cache_utils import CacheService @@ -64,12 +63,12 @@ def get_asides(block): @property def resources_fs(): - return Mock(name='TestDescriptorSystem.resources_fs', root_path='.') + return Mock(name='TestModuleStoreRuntime.resources_fs', root_path='.') -class TestDescriptorSystem(MakoDescriptorSystem): # pylint: disable=abstract-method +class TestModuleStoreRuntime(ModuleStoreRuntime): # pylint: disable=abstract-method """ - DescriptorSystem for testing + ModuleStore-based XBlock Runtime for testing """ def handler_url(self, block, handler, suffix='', query='', thirdparty=False): # lint-amnesty, pylint: disable=arguments-differ return '{usage_id}/{handler}{suffix}?{query}'.format( @@ -90,7 +89,7 @@ def get_asides(self, block): return [] def resources_fs(self): # lint-amnesty, pylint: disable=method-hidden - return Mock(name='TestDescriptorSystem.resources_fs', root_path='.') + return Mock(name='TestModuleStoreRuntime.resources_fs', root_path='.') def __repr__(self): """ @@ -117,7 +116,7 @@ def get_test_system( add_get_block_overrides=False ): """ - Construct a test DescriptorSystem instance. + Construct a test ModuleStoreRuntime instance. By default, the descriptor system's render_template() method simply returns the repr of the context it is passed. You can override this by passing in a different render_template argument. @@ -239,11 +238,11 @@ def get_block(block): def get_test_descriptor_system(render_template=None, **kwargs): """ - Construct a test DescriptorSystem instance. + Construct a test ModuleStoreRuntime instance. """ field_data = DictFieldData({}) - descriptor_system = TestDescriptorSystem( + descriptor_system = TestModuleStoreRuntime( load_item=Mock(name='get_test_descriptor_system.load_item'), resources_fs=Mock(name='get_test_descriptor_system.resources_fs'), error_tracker=Mock(name='get_test_descriptor_system.error_tracker'), diff --git a/xmodule/tests/helpers.py b/xmodule/tests/helpers.py index 480771b41778..c69c810d4aa2 100644 --- a/xmodule/tests/helpers.py +++ b/xmodule/tests/helpers.py @@ -9,7 +9,7 @@ import pytest from path import Path as path from xblock.reference.user_service import UserService, XBlockUser -from xmodule.x_module import DescriptorSystem +from xmodule.x_module import ModuleStoreRuntime def directories_equal(directory1, directory2): @@ -132,7 +132,7 @@ def replace_urls(self, text, static_replace_only=False): @pytest.fixture def override_descriptor_system(monkeypatch): """ - Fixture to override get_block method of DescriptorSystem + Fixture to override get_block method of ModuleStoreRuntime """ def get_block(self, usage_id, for_parent=None): @@ -140,4 +140,4 @@ def get_block(self, usage_id, for_parent=None): block = self.load_item(usage_id, for_parent=for_parent) return block - monkeypatch.setattr(DescriptorSystem, "get_block", get_block) + monkeypatch.setattr(ModuleStoreRuntime, "get_block", get_block) diff --git a/xmodule/tests/test_conditional.py b/xmodule/tests/test_conditional.py index d1fcde0c7561..8eb9e9ac071d 100644 --- a/xmodule/tests/test_conditional.py +++ b/xmodule/tests/test_conditional.py @@ -15,7 +15,7 @@ from xmodule.conditional_block import ConditionalBlock from xmodule.error_block import ErrorBlock -from xmodule.modulestore.xml import CourseLocationManager, ImportSystem, XMLModuleStore +from xmodule.modulestore.xml import CourseLocationManager, XMLImportingModuleStoreRuntime, XMLModuleStore from xmodule.tests import DATA_DIR, get_test_system, prepare_block_runtime from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import factories as xml @@ -26,7 +26,10 @@ COURSE = 'conditional' # name of directory with course data -class DummySystem(ImportSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring +class DummyModuleStoreRuntime(XMLImportingModuleStoreRuntime): # pylint: disable=abstract-method + """ + Minimal modulestore runtime for tests + """ @patch('xmodule.modulestore.xml.OSFS', lambda directory: MemoryFS()) def __init__(self, load_error_blocks): diff --git a/xmodule/tests/test_course_block.py b/xmodule/tests/test_course_block.py index f2956cca0d7e..c88b519d708e 100644 --- a/xmodule/tests/test_course_block.py +++ b/xmodule/tests/test_course_block.py @@ -21,7 +21,7 @@ import xmodule.course_block from xmodule.course_metadata_utils import DEFAULT_START_DATE from xmodule.data import CertificatesDisplayBehaviors -from xmodule.modulestore.xml import ImportSystem, XMLModuleStore +from xmodule.modulestore.xml import XMLImportingModuleStoreRuntime, XMLModuleStore from xmodule.modulestore.exceptions import InvalidProctoringProvider ORG = 'test_org' @@ -52,7 +52,10 @@ def test_default_enrollment_start_date(self, should_have_default_enroll_start): assert xmodule.course_block.CourseFields.enrollment_start.default == expected -class DummySystem(ImportSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring +class DummyModuleStoreRuntime(XMLImportingModuleStoreRuntime): # pylint: disable=abstract-method + """ + Minimal modulestore runtime for tests. + """ @patch('xmodule.modulestore.xml.OSFS', lambda dir: MemoryFS()) def __init__(self, load_error_blocks, course_id=None): @@ -83,7 +86,7 @@ def get_dummy_course( ): """Get a dummy course""" - system = DummySystem(load_error_blocks=True) + system = DummyModuleStoreRuntime(load_error_blocks=True) def to_attrb(n, v): return '' if v is None else f'{n}="{v}"'.lower() @@ -126,7 +129,7 @@ class HasEndedMayCertifyTestCase(unittest.TestCase): def setUp(self): super().setUp() - system = DummySystem(load_error_blocks=True) # lint-amnesty, pylint: disable=unused-variable + system = DummyModuleStoreRuntime(load_error_blocks=True) # lint-amnesty, pylint: disable=unused-variable past_end = (datetime.now() - timedelta(days=12)).strftime("%Y-%m-%dT%H:%M:00") future_end = (datetime.now() + timedelta(days=12)).strftime("%Y-%m-%dT%H:%M:00") diff --git a/xmodule/tests/test_import.py b/xmodule/tests/test_import.py index 95feedeb9c76..0540f7665f1f 100644 --- a/xmodule/tests/test_import.py +++ b/xmodule/tests/test_import.py @@ -18,7 +18,7 @@ from xmodule.fields import Date from xmodule.modulestore.inheritance import InheritanceMixin, compute_inherited_metadata -from xmodule.modulestore.xml import ImportSystem, LibraryXMLModuleStore, XMLModuleStore +from xmodule.modulestore.xml import XMLImportingModuleStoreRuntime, LibraryXMLModuleStore, XMLModuleStore from xmodule.tests import DATA_DIR from xmodule.x_module import XModuleMixin from xmodule.xml_block import is_pointer_tag @@ -28,7 +28,10 @@ RUN = 'test_run' -class DummySystem(ImportSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring +class DummyModuleStoreRuntime(XMLImportingModuleStoreRuntime): # pylint: disable=abstract-method, missing-class-docstring + """ + Minimal modulestore runtime for tests + """ @patch('xmodule.modulestore.xml.OSFS', lambda dir: OSFS(mkdtemp())) def __init__(self, load_error_blocks, library=False): @@ -58,7 +61,7 @@ class BaseCourseTestCase(TestCase): @staticmethod def get_system(load_error_blocks=True, library=False): '''Get a dummy system''' - return DummySystem(load_error_blocks, library=library) + return DummyModuleStoreRuntime(load_error_blocks, library=library) def get_course(self, name): """Get a test course by directory name. If there's more than one, error.""" diff --git a/xmodule/tests/test_item_bank.py b/xmodule/tests/test_item_bank.py index c27412c8b709..f29e4b847874 100644 --- a/xmodule/tests/test_item_bank.py +++ b/xmodule/tests/test_item_bank.py @@ -21,7 +21,7 @@ from common.djangoapps.student.tests.factories import UserFactory from ..item_bank_block import ItemBankBlock -from .test_course_block import DummySystem as TestImportSystem +from .test_course_block import DummyModuleStoreRuntime dummy_render = lambda block, _: Fragment(block.data) # pylint: disable=invalid-name @@ -121,7 +121,7 @@ def test_xml_export_import_cycle(self): olx_element = etree.fromstring(actual_olx_export) # Re-import the OLX. - runtime = TestImportSystem(load_error_blocks=True, course_id=self.item_bank.context_key) + runtime = DummyModuleStoreRuntime(load_error_blocks=True, course_id=self.item_bank.context_key) runtime.resources_fs = export_fs imported_item_bank = ItemBankBlock.parse_xml(olx_element, runtime, None) @@ -168,11 +168,11 @@ def test_max_count_validation(self): assert self.item_bank.validate() @patch( - 'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', + 'xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.render', VanillaRuntime.render, ) @patch('xmodule.capa_block.ProblemBlock.author_view', dummy_render, create=True) - @patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: []) + @patch('xmodule.x_module.ModuleStoreRuntime.applicable_aside_types', lambda self, block: []) def test_preview_view(self): """ Test preview view rendering """ self._bind_course_block(self.item_bank) @@ -183,11 +183,11 @@ def test_preview_view(self): assert '

Hello world from problem 3

' in rendered.content @patch( - 'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', + 'xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.render', VanillaRuntime.render, ) @patch('xmodule.capa_block.ProblemBlock.author_view', dummy_render, create=True) - @patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: []) + @patch('xmodule.x_module.ModuleStoreRuntime.applicable_aside_types', lambda self, block: []) def test_author_view(self): """ Test author view rendering """ self._bind_course_block(self.item_bank) diff --git a/xmodule/tests/test_library_content.py b/xmodule/tests/test_library_content.py index a5d37cff4288..01d27fad98d0 100644 --- a/xmodule/tests/test_library_content.py +++ b/xmodule/tests/test_library_content.py @@ -27,7 +27,7 @@ from xmodule.validation import StudioValidationMessage from xmodule.x_module import AUTHOR_VIEW -from .test_course_block import DummySystem as TestImportSystem +from .test_course_block import DummyModuleStoreRuntime dummy_render = lambda block, _: Fragment(block.data) # pylint: disable=invalid-name @@ -167,7 +167,7 @@ def setUp(self): self.lc_block.runtime.export_fs = self.export_fs # pylint: disable=protected-access # Prepare runtime for the import. - self.runtime = TestImportSystem(load_error_blocks=True, course_id=self.lc_block.location.course_key) + self.runtime = DummyModuleStoreRuntime(load_error_blocks=True, course_id=self.lc_block.location.course_key) self.runtime.resources_fs = self.export_fs self.id_generator = Mock() @@ -521,10 +521,10 @@ def setUp(self): @patch( - 'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render + 'xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.render', VanillaRuntime.render ) @patch('xmodule.html_block.HtmlBlock.author_view', dummy_render, create=True) -@patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: []) +@patch('xmodule.x_module.ModuleStoreRuntime.applicable_aside_types', lambda self, block: []) class TestLibraryContentRender(LegacyLibraryContentTest): """ Rendering unit tests for LegacyLibraryContentBlock @@ -732,10 +732,10 @@ def test_removed_invalid(self): @patch( - 'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render + 'xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.render', VanillaRuntime.render ) @patch('xmodule.html_block.HtmlBlock.author_view', dummy_render, create=True) -@patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: []) +@patch('xmodule.x_module.ModuleStoreRuntime.applicable_aside_types', lambda self, block: []) class TestMigratedLibraryContentRender(LegacyLibraryContentTest): """ Rendering unit tests for LegacyLibraryContentBlock @@ -825,7 +825,7 @@ def test_xml_export_import_cycle(self): assert exported_olx == expected_olx_export # Now import it. - runtime = TestImportSystem(load_error_blocks=True, course_id=self.lc_block.location.course_key) + runtime = DummyModuleStoreRuntime(load_error_blocks=True, course_id=self.lc_block.location.course_key) runtime.resources_fs = export_fs olx_element = etree.fromstring(exported_olx) imported_lc_block = LegacyLibraryContentBlock.parse_xml(olx_element, runtime, None) diff --git a/xmodule/tests/test_library_root.py b/xmodule/tests/test_library_root.py index 0390aaa7ccce..bcc2b6af7174 100644 --- a/xmodule/tests/test_library_root.py +++ b/xmodule/tests/test_library_root.py @@ -15,11 +15,11 @@ @patch( - 'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render + 'xmodule.modulestore.split_mongo.runtime.SplitModuleStoreRuntime.render', VanillaRuntime.render ) @patch('xmodule.html_block.HtmlBlock.author_view', dummy_render, create=True) @patch('xmodule.html_block.HtmlBlock.has_author_view', True, create=True) -@patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: []) +@patch('xmodule.x_module.ModuleStoreRuntime.applicable_aside_types', lambda self, block: []) class TestLibraryRoot(MixedSplitTestCase): """ Basic unit tests for LibraryRoot (library_root_xblock.py) diff --git a/xmodule/tests/test_poll.py b/xmodule/tests/test_poll.py index e04aab2b9fd2..440dde41e082 100644 --- a/xmodule/tests/test_poll.py +++ b/xmodule/tests/test_poll.py @@ -11,7 +11,7 @@ from openedx.core.lib.safe_lxml import etree from xmodule import poll_block from . import get_test_system -from .test_import import DummySystem +from .test_import import DummyModuleStoreRuntime class _PollBlockTestBase(TestCase): @@ -67,7 +67,7 @@ def test_poll_export_with_unescaped_characters_xml(self): Make sure that poll_block will export fine if its xml contains unescaped characters. """ - module_system = DummySystem(load_error_blocks=True) + module_system = DummyModuleStoreRuntime(load_error_blocks=True) module_system.id_generator.target_course_id = self.xblock.course_id sample_poll_xml = ''' diff --git a/xmodule/tests/test_randomize_block.py b/xmodule/tests/test_randomize_block.py index 52413faad3b7..feffd7d2f12a 100644 --- a/xmodule/tests/test_randomize_block.py +++ b/xmodule/tests/test_randomize_block.py @@ -11,7 +11,7 @@ from xmodule.randomize_block import RandomizeBlock from xmodule.tests import prepare_block_runtime -from .test_course_block import DummySystem as TestImportSystem +from .test_course_block import DummyModuleStoreRuntime class RandomizeBlockTest(MixedSplitTestCase): @@ -78,7 +78,7 @@ def test_xml_export_import_cycle(self): # And compare. assert exported_olx == expected_olx - runtime = TestImportSystem(load_error_blocks=True, course_id=randomize_block.location.course_key) + runtime = DummyModuleStoreRuntime(load_error_blocks=True, course_id=randomize_block.location.course_key) runtime.resources_fs = export_fs # Now import it. diff --git a/xmodule/tests/test_split_test_block.py b/xmodule/tests/test_split_test_block.py index c51c157a7760..7acbaa25f6ba 100644 --- a/xmodule/tests/test_split_test_block.py +++ b/xmodule/tests/test_split_test_block.py @@ -19,7 +19,7 @@ user_partition_values, ) from xmodule.tests import prepare_block_runtime -from xmodule.tests.test_course_block import DummySystem as TestImportSystem +from xmodule.tests.test_course_block import DummyModuleStoreRuntime from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import factories as xml from xmodule.validation import StudioValidationMessage @@ -581,7 +581,7 @@ def test_xml_export_import_cycle(self): # And compare. assert exported_olx == expected_olx - runtime = TestImportSystem(load_error_blocks=True, course_id=split_test_block.location.course_key) + runtime = DummyModuleStoreRuntime(load_error_blocks=True, course_id=split_test_block.location.course_key) runtime.resources_fs = export_fs # Now import it. diff --git a/xmodule/tests/test_video.py b/xmodule/tests/test_video.py index 1179feebf324..39d0abf2c27f 100644 --- a/xmodule/tests/test_video.py +++ b/xmodule/tests/test_video.py @@ -40,7 +40,7 @@ from xblock.core import XBlockAside from xmodule.modulestore.tests.test_asides import AsideTestType -from .test_import import DummySystem +from .test_import import DummyModuleStoreRuntime SRT_FILEDATA = ''' 0 @@ -283,7 +283,7 @@ def test_constructor(self): }) def test_parse_xml(self): - module_system = DummySystem(load_error_blocks=True) + module_system = DummyModuleStoreRuntime(load_error_blocks=True) xml_data = '''