diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index a4ece6c85d59..198ef245d446 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -320,7 +320,7 @@ def _import_xml_node_to_parent( block_type = node.tag # 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 @@ -357,7 +357,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 fc119c7edd58..97f80c4ca2a7 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -1845,7 +1845,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): @@ -2690,8 +2690,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 @@ -4375,7 +4375,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/docs/decisions/0020-upstream-downstream.rst b/docs/decisions/0020-upstream-downstream.rst index 8ceb9e775274..9e2dd9aa195a 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 @@ -350,10 +349,10 @@ change through development and code review. ########################################################################### - # 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/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 1bae90322487..99acbfbbf33f 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 85241ead4a5b..b88f6e4b4b9f 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -68,7 +68,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 d45bb94816b9..a2c7d945c864 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 @@ -1971,7 +1971,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' @@ -2074,7 +2074,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 == '' @@ -2110,7 +2110,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) @@ -2215,7 +2215,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) @@ -2283,7 +2283,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 6d78e156f292..004e59b0b5c0 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. """ def __init__( 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 abf3a93f3858..906f48a6f6af 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/mongo/base.py b/xmodule/modulestore/mongo/base.py index 16a8c134c1d6..548fb326bc64 100644 --- a/xmodule/modulestore/mongo/base.py +++ b/xmodule/modulestore/mongo/base.py @@ -37,7 +37,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 @@ -47,6 +46,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) @@ -663,7 +663,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. """ @@ -690,7 +690,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, @@ -929,7 +929,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) @@ -1002,7 +1002,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) @@ -1112,7 +1112,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..7e118fad9159 100644 --- a/xmodule/modulestore/split_mongo/id_manager.py +++ b/xmodule/modulestore/split_mongo/id_manager.py @@ -14,10 +14,10 @@ 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 + self._cds = caching_descriptor_system # n.b. CDS is the old name for SplitModuleStoreRuntime def get_definition_id(self, usage_id): if isinstance(usage_id.block_id, LocalId): 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 b0965d63fecb..c0e8bf81cf52 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 LibraryToolsService -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 64e19420a152..8e2bafa58362 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -107,7 +107,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__) @@ -716,7 +716,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 @@ -3295,7 +3295,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 8adbfcb911a4..4dbd84047c33 100644 --- a/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -3322,7 +3322,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): """ @@ -3396,7 +3396,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: @@ -3483,7 +3483,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: @@ -3618,7 +3618,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): """ @@ -3682,7 +3682,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): """ @@ -3730,7 +3730,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): """ @@ -3779,7 +3779,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..8e71699e03ee 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 5b880b4ade2f..a72bf9506a79 100644 --- a/xmodule/modulestore/xml_importer.py +++ b/xmodule/modulestore/xml_importer.py @@ -48,7 +48,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 @@ -971,8 +971,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("/"): @@ -980,7 +980,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 b2cdd67b71ba..7d18a10ac252 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -29,14 +29,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 @@ -69,12 +68,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( @@ -95,7 +94,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): """ @@ -122,7 +121,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. @@ -244,11 +243,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_library_content.py b/xmodule/tests/test_library_content.py index e30e19922be5..e38a797c5cbe 100644 --- a/xmodule/tests/test_library_content.py +++ b/xmodule/tests/test_library_content.py @@ -27,7 +27,7 @@ from xmodule.capa_block import ProblemBlock from common.djangoapps.student.tests.factories import UserFactory -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 @@ -158,7 +158,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() @@ -528,10 +528,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(LibraryContentTest): """ Rendering unit tests for LibraryContentBlock 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 68bddce43945..df741325bff9 100644 --- a/xmodule/tests/test_poll.py +++ b/xmodule/tests/test_poll.py @@ -10,7 +10,7 @@ from openedx.core.lib.safe_lxml import etree from xmodule.poll_block import PollBlock from . import get_test_system -from .test_import import DummySystem +from .test_import import DummyModuleStoreRuntime class PollBlockTest(unittest.TestCase): @@ -60,7 +60,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 deebdfe4f1d6..d0bbf9e7b892 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 5e95f77082b1..86a4614dc0b5 100644 --- a/xmodule/tests/test_video.py +++ b/xmodule/tests/test_video.py @@ -38,7 +38,7 @@ from xmodule.video_block import EXPORT_IMPORT_STATIC_DIR, VideoBlock, create_youtube_string from xmodule.video_block.transcripts_utils import save_to_store -from .test_import import DummySystem +from .test_import import DummyModuleStoreRuntime SRT_FILEDATA = ''' 0 @@ -281,7 +281,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 = '''