Skip to content

Commit

Permalink
refactor: rename ModuleStore runtimes now that XModules are gone
Browse files Browse the repository at this point in the history
All renames, excluding test classes:tt

* xmodule.x_module:DescriptorSystem ->
  xmodule.x_module:ModuleStoreRuntime

* xmodule.mako_block:MakoDescriptorSystem ->
  xmodule.x_module:ModuleStoreRuntime

* xmodule.x_module:XMLParsingSystem ->
  xmodule.modulestore.xml:XMLParsingModuleStoreRuntime

* xmodule.modulestore.xml:ImportSystem ->
  xmodule.modulestore.xml:XMLImportingModuleStoreRuntime

* xmodule.modulestore.mongo.base:CachingDescriptorSystem ->
  xmodule.modulestore.mongo.base:OldModuleStoreRuntime

* xmodule.modulestore.split_mongo.caching_descriptor_system:CachingDescriptorSystem ->
  xmodule.modulestore.split_mongo.runtime:SplitModuleStoreRuntime
  • Loading branch information
kdmccormick committed Sep 17, 2024
1 parent a94b5af commit 9821d59
Show file tree
Hide file tree
Showing 38 changed files with 306 additions and 286 deletions.
5 changes: 3 additions & 2 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions cms/djangoapps/contentstore/views/tests/test_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions common/djangoapps/static_replace/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down
7 changes: 3 additions & 4 deletions docs/decisions/0020-upstream-downstream.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand 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:
"""
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/block_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 10 additions & 7 deletions lms/djangoapps/courseware/tests/test_block_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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={},
Expand All @@ -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)

Expand Down Expand Up @@ -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):
"""
Expand Down
12 changes: 6 additions & 6 deletions lms/djangoapps/courseware/tests/test_video_mongo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -2074,7 +2074,7 @@ def test_import_no_video_id(self):
"""
xml_data = """<video><video_asset></video_asset></video>"""
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 == ''
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 = """
Expand Down
4 changes: 2 additions & 2 deletions lms/djangoapps/lms_xblock/test/test_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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')
Expand Down
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/xblock/runtime/shims.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()")

Expand Down Expand Up @@ -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:
Expand Down
5 changes: 4 additions & 1 deletion openedx/core/lib/tests/test_xblock_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down
2 changes: 1 addition & 1 deletion xmodule/capa/capa_problem.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__(
Expand Down
2 changes: 1 addition & 1 deletion xmodule/error_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 1 addition & 24 deletions xmodule/mako_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 9821d59

Please sign in to comment.