[BD-13] [BB-5967] refactor: deprecate get_module and descriptor_runtime attributes from ModuleSystem

cms/djangoapps/contentstore/views/preview.py

@@ -35,6 +35,8 @@
 from common.djangoapps.edxmako.shortcuts import render_to_string
 from common.djangoapps.edxmako.services import MakoService
 from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
+from common.lib.xmodule.xmodule.x_module import AsideKeyGenerator, OpaqueKeyReader
+from lms.djangoapps.courseware.services import ModuleService
 from lms.djangoapps.lms_xblock.field_data import LmsFieldData
 from openedx.core.lib.license import wrap_with_license
 from openedx.core.lib.cache_utils import CacheService
@@ -207,11 +209,12 @@ def _preview_module_system(request, descriptor, field_data):
         else:
             preview_anonymous_user_id = anonymous_id_for_user(request.user, course_id)
 
+    module_service = ModuleService(partial(_load_preview_module, request=request))
+
     return PreviewModuleSystem(
         static_url=settings.STATIC_URL,
         # TODO (cpennington): Do we want to track how instructors are using the preview problems?
         track_function=lambda event_type, event: None,
-        get_module=partial(_load_preview_module, request),
         debug=True,
         mixins=settings.XBLOCK_MIXINS,
         course_id=course_id,
@@ -221,7 +224,6 @@ def _preview_module_system(request, descriptor, field_data):
         wrappers_asides=wrappers_asides,
         error_descriptor_class=ErrorBlock,
         # Get the raw DescriptorSystem, not the CombinedSystem
-        descriptor_runtime=descriptor._runtime,  # pylint: disable=protected-access
         services={
             "field-data": field_data,
             "i18n": ModuleI18nService,
@@ -236,8 +238,11 @@ def _preview_module_system(request, descriptor, field_data):
             "teams_configuration": TeamsConfigurationService(),
             "sandbox": SandboxService(contentstore=contentstore, course_id=course_id),
             "cache": CacheService(cache),
-            'replace_urls': replace_url_service
+            'replace_urls': replace_url_service,
+            'module': module_service
         },
+        id_reader=getattr(descriptor._runtime, 'id_reader', OpaqueKeyReader()),  # pylint: disable=protected-access
+        id_generator=getattr(descriptor._runtime, 'id_generator', AsideKeyGenerator()),  # pylint: disable=protected-access
     )
 
 

cms/djangoapps/contentstore/views/tests/test_preview.py

@@ -28,7 +28,7 @@
 from common.djangoapps import static_replace
 from common.djangoapps.student.tests.factories import UserFactory
 
-from ..preview import _preview_module_system, get_preview_fragment
+from ..preview import _preview_module_system, get_preview_fragment, _load_preview_module
 
 
 @ddt.ddt
@@ -178,6 +178,7 @@ def test_block_branch_not_changed_by_preview_handler(self, default_store):
 @XBlock.needs("field-data")
 @XBlock.needs("i18n")
 @XBlock.needs("mako")
+@XBlock.needs("module")
 @XBlock.needs("replace_urls")
 @XBlock.needs("user")
 @XBlock.needs("teams_configuration")
@@ -210,7 +211,7 @@ def setUp(self):
         self.field_data = mock.Mock()
 
     @XBlock.register_temp_plugin(PureXBlock, identifier='pure')
-    @ddt.data("user", "i18n", "field-data", "teams_configuration", "replace_urls")
+    @ddt.data("user", "i18n", "field-data", "teams_configuration", "replace_urls", "module")
     def test_expected_services_exist(self, expected_service):
         """
         Tests that the 'user' and 'i18n' services are provided by the Studio runtime.
@@ -298,6 +299,9 @@ def test_replace_urls(self):
         assert self.runtime.replace_urls(html) == \
             static_replace.replace_static_urls(html, course_id=self.runtime.course_id)
 
+    def test_get_module(self):
+        assert self.runtime.get_module(self.descriptor) == _load_preview_module(self.request, self.descriptor)
+
     def test_anonymous_user_id_preview(self):
         assert self.runtime.anonymous_student_id == 'student'
 

common/lib/xmodule/xmodule/conditional_module.py

@@ -39,6 +39,7 @@
 
 
 @XBlock.needs('mako')
+@XBlock.needs('module')
 class ConditionalBlock(
     SequenceMixin,
     MakoTemplateBlockBase,
@@ -317,7 +318,8 @@ def get_required_blocks(self):
         """
         Returns a list of bound XBlocks instances upon which XBlock depends.
         """
-        return [self.system.get_module(descriptor) for descriptor in self.get_required_module_descriptors()]
+        module_service = self.runtime.service(self, 'module')
+        return [module_service.get_module(descriptor) for descriptor in self.get_required_module_descriptors()]
 
     def get_required_module_descriptors(self):
         """

common/lib/xmodule/xmodule/library_sourced_block.py

@@ -101,14 +101,40 @@ def student_view(self, context):
         Renders the view that learners see.
         """
         result = Fragment()
-        child_frags = self.runtime.render_children(self, context=context)
+
+        # TODO: uncomment this line and remove _render_children() once
+        # merger of ModuleSystem and DescriptorSystem is complete
+
+        # child_frags = self.runtime.render_children(self, context=context)
+        child_frags = self._render_children(context)
+
         result.add_resources(child_frags)
         result.add_content('<div class="library-sourced-content">')
         for frag in child_frags:
             result.add_content(frag.content)
         result.add_content('</div>')
         return result
 
+    def _render_children(self, context):
+        """
+        Use CombinedSystem runtime to get block and render each child individually.
+
+        The runtime.render_children() method was earlier used to render each child
+        However, this caused a problem, when we removed the get_block() and
+        descriptor_runtime properties from the ModuleSystem, as the render_children
+        method ran in the context of LmsModuleSystem which is a child class of
+        ModuleSystem.
+
+        This is intended to be a temporary method until deprecation of all properties
+        of ModuleSystem and merger of ModuleSystem and DescriptorSystem is complete.
+        """
+        results = []
+        for child_id in self.children:
+            child = self.runtime.get_block(child_id)
+            result = self.runtime.render_child(child, context=context)
+            results.append(result)
+        return results
+
     def validate_field_data(self, validation, data):
         """
         Validate this block's field data. Instead of checking fields like self.name, check the

common/lib/xmodule/xmodule/split_test_module.py

@@ -125,6 +125,7 @@ def get_split_user_partitions(user_partitions):
 @XBlock.needs('mako')
 @XBlock.needs('partitions')
 @XBlock.needs('user')
+@XBlock.needs('module')
 class SplitTestBlock(  # lint-amnesty, pylint: disable=abstract-method
     SplitTestFields,
     SequenceMixin,
@@ -192,7 +193,7 @@ def child(self):
         Return the user bound child block for the partition or None.
         """
         if self.child_descriptor is not None:
-            return self.system.get_module(self.child_descriptor)
+            return self.runtime.service(self, 'module').get_module(self.child_descriptor)
         else:
             return None
 
@@ -272,7 +273,7 @@ def _staff_view(self, context):
 
         for child_location in self.children:  # pylint: disable=no-member
             child_descriptor = self.get_child_descriptor_by_location(child_location)
-            child = self.system.get_module(child_descriptor)
+            child = self.runtime.service(self, 'module').get_module(child_descriptor)
             rendered_child = child.render(STUDENT_VIEW, context)
             fragment.add_fragment_resources(rendered_child)
             group_name, updated_group_id = self.get_data_for_vertical(child)
@@ -347,7 +348,7 @@ def studio_render_children(self, fragment, children, context):
         """
         html = ""
         for active_child_descriptor in children:
-            active_child = self.system.get_module(active_child_descriptor)
+            active_child = self.runtime.service(self, 'module').get_module(active_child_descriptor)
             rendered_child = active_child.render(StudioEditableBlock.get_preview_view_name(active_child), context)
             if active_child.category == 'vertical':
                 group_name, group_id = self.get_data_for_vertical(active_child)

common/lib/xmodule/xmodule/tests/__init__.py

@@ -15,10 +15,11 @@
 import traceback
 import unittest
 from contextlib import contextmanager
-from functools import wraps
+from functools import partial, wraps
 from unittest.mock import Mock
 
 from django.test import TestCase
+from lms.djangoapps.courseware.services import ModuleService
 
 from opaque_keys.edx.keys import CourseKey
 from path import Path as path
@@ -152,7 +153,6 @@ def get_module(descriptor):
     return TestModuleSystem(
         static_url='/static',
         track_function=Mock(name='get_test_system.track_function'),
-        get_module=get_module,
         debug=True,
         hostname="edx.org",
         services={
@@ -166,12 +166,12 @@ def get_module(descriptor):
                 waittime=10,
                 construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"),
             ),
-            'replace_urls': replace_url_service
+            'replace_urls': replace_url_service,
+            'module': ModuleService(partial(get_module))
         },
         node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"),
         course_id=course_id,
         error_descriptor_class=ErrorBlock,
-        descriptor_runtime=descriptor_system,
     )
 
 

common/lib/xmodule/xmodule/tests/test_conditional.py

@@ -1,11 +1,13 @@
 # lint-amnesty, pylint: disable=missing-module-docstring
 
+from functools import partial
 import json
 import unittest
 from unittest.mock import Mock, patch
 
 from django.conf import settings
 from fs.memoryfs import MemoryFS
+from lms.djangoapps.courseware.services import ModuleService
 from lxml import etree
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
@@ -131,9 +133,15 @@ def load_item(usage_id, for_parent=None):  # pylint: disable=unused-argument
             ScopeIds(None, None, cond_location, cond_location)
         )
         cond_descriptor.xmodule_runtime = system
-        system.get_module = lambda desc: desc if visible_to_nonstaff_users(desc) else None
+
+        def _check_module_user_access(descriptor):
+            if visible_to_nonstaff_users(descriptor):
+                return descriptor
+            return None
+
+        system._services['module'] = ModuleService(partial(_check_module_user_access))
         cond_descriptor.get_required_blocks = [
-            system.get_module(source_descriptor),
+            system._services['module'].get_module(source_descriptor),
         ]
 
         # return dict:
@@ -229,7 +237,7 @@ def setUp(self):
 
     def get_module_for_location(self, location):
         descriptor = self.modulestore.get_item(location, depth=None)
-        return self.test_system.get_module(descriptor)
+        return self.test_system._services['module'].get_module(descriptor)
 
     @patch('xmodule.x_module.descriptor_global_local_resource_url')
     @patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': False})

common/lib/xmodule/xmodule/tests/test_library_content.py

@@ -3,10 +3,12 @@
 
 Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`.
 """
+from functools import partial
 from unittest.mock import Mock, patch
 
 from bson.objectid import ObjectId
 from fs.memoryfs import MemoryFS
+from lms.djangoapps.courseware.services import ModuleService
 from lxml import etree
 from search.search_engine_base import SearchEngine
 from web_fragments.fragment import Fragment
@@ -61,12 +63,12 @@ def _bind_course_module(self, module):
         def get_module(descriptor):
             """Mocks module_system get_module function"""
             sub_module_system = get_test_system(course_id=module.location.course_key)
-            sub_module_system.get_module = get_module
+            sub_module_system._services['module'] = ModuleService(partial(get_module))
             sub_module_system.descriptor_runtime = descriptor._runtime  # pylint: disable=protected-access
             descriptor.bind_for_student(sub_module_system, self.user_id)
             return descriptor
 
-        module_system.get_module = get_module
+        module_system._services['module'] = ModuleService(partial(get_module))
         module.xmodule_runtime = module_system
 
 

common/lib/xmodule/xmodule/unit_block.py

@@ -44,14 +44,40 @@ class UnitBlock(XBlock):
     def student_view(self, context=None):
         """Provide default student view."""
         result = Fragment()
-        child_frags = self.runtime.render_children(self, context=context)
+
+        # TODO: uncomment this line and remove _render_children() once
+        # merger of ModuleSystem and DescriptorSystem is complete
+
+        # child_frags = self.runtime.render_children(self, context=context)
+        child_frags = self._render_children(context)
+
         result.add_resources(child_frags)
         result.add_content('<div class="unit-xblock vertical">')
         for frag in child_frags:
             result.add_content(frag.content)
         result.add_content('</div>')
         return result
 
+    def _render_children(self, context):
+        """
+        Use CombinedSystem runtime to get block and render each child individually.
+
+        The runtime.render_children() method was earlier used to render each child
+        However, this caused a problem, when we removed the get_block() and
+        descriptor_runtime properties from the ModuleSystem, as the render_children
+        method ran in the context of LmsModuleSystem which is a child class of
+        ModuleSystem.
+
+        This is intended to be a temporary method until deprecation of all properties
+        of ModuleSystem and merger of ModuleSystem and DescriptorSystem is complete.
+        """
+        results = []
+        for child_id in self.children:
+            child = self.runtime.get_block(child_id)
+            result = self.runtime.render_child(child, context=context)
+            results.append(result)
+        return results
+
     public_view = student_view
 
     def index_dictionary(self):