Revert "feat: unify ModuleSystem and DescriptorSystem [BD-13]"

cms/djangoapps/contentstore/tests/test_i18n.py

@@ -15,7 +15,7 @@
 from xmodule.tests.test_export import PureXBlock
 
 from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient
-from cms.djangoapps.contentstore.views.preview import _prepare_runtime_for_preview
+from cms.djangoapps.contentstore.views.preview import _preview_module_system
 from common.djangoapps.student.tests.factories import UserFactory
 from openedx.core.lib.edx_six import get_gettext
 
@@ -70,7 +70,7 @@ def setUp(self):
         self.course = CourseFactory.create()
         self.field_data = mock.Mock()
         self.descriptor = BlockFactory(category="pure", parent=self.course)
-        _prepare_runtime_for_preview(
+        self.runtime = _preview_module_system(
             self.request,
             self.descriptor,
             self.field_data,
@@ -81,7 +81,7 @@ def get_block_i18n_service(self, descriptor):
         """
         return the block i18n service.
         """
-        i18n_service = self.descriptor.runtime.service(descriptor, 'i18n')
+        i18n_service = self.runtime.service(descriptor, 'i18n')
         self.assertIsNotNone(i18n_service)
         self.assertIsInstance(i18n_service, XBlockI18nService)
         return i18n_service
@@ -171,7 +171,7 @@ def test_i18n_service_callable(self):
         """
         Test: i18n service should be callable in studio.
         """
-        self.assertTrue(callable(self.descriptor.runtime._services.get('i18n')))  # pylint: disable=protected-access
+        self.assertTrue(callable(self.runtime._services.get('i18n')))  # pylint: disable=protected-access
 
 
 class InternationalizationTest(ModuleStoreTestCase):

cms/djangoapps/contentstore/views/block.py

@@ -293,23 +293,38 @@ def can_write(self, course_key):
         return has_studio_write_access(self._user, course_key)
 
 
-def load_services_for_studio(runtime, user):
+class StudioEditModuleRuntime:
     """
-    Function to set some required services used for XBlock edits and studio_view.
-    (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information
+    An extremely minimal ModuleSystem shim used for XBlock edits and studio_view.
+    (i.e. whenever we're not using PreviewModuleSystem.) This is required to make information
     about the current user (especially permissions) available via services as needed.
     """
-    services = {
-        "user": DjangoXBlockUserService(user),
-        "studio_user_permissions": StudioPermissionsService(user),
-        "mako": MakoService(),
-        "settings": SettingsService(),
-        "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag),
-        "teams_configuration": TeamsConfigurationService(),
-        "library_tools": LibraryToolsService(modulestore(), user.id)
-    }
 
-    runtime._services.update(services)  # lint-amnesty, pylint: disable=protected-access
+    def __init__(self, user):
+        self._user = user
+
+    def service(self, block, service_name):
+        """
+        This block is not bound to a user but some blocks (LibraryContentBlock) may need
+        user-specific services to check for permissions, etc.
+        If we return None here, CombinedSystem will load services from the descriptor runtime.
+        """
+        if block.service_declaration(service_name) is not None:
+            if service_name == "user":
+                return DjangoXBlockUserService(self._user)
+            if service_name == "studio_user_permissions":
+                return StudioPermissionsService(self._user)
+            if service_name == "mako":
+                return MakoService()
+            if service_name == "settings":
+                return SettingsService()
+            if service_name == "lti-configuration":
+                return ConfigurationService(CourseAllowPIISharingInLTIFlag)
+            if service_name == "teams_configuration":
+                return TeamsConfigurationService()
+            if service_name == "library_tools":
+                return LibraryToolsService(modulestore(), self._user.id)
+        return None
 
 
 @require_http_methods("GET")
@@ -353,8 +368,8 @@ def xblock_view_handler(request, usage_key_string, view_name):
         ))
 
         if view_name in (STUDIO_VIEW, VISIBILITY_VIEW):
-            if view_name == STUDIO_VIEW:
-                load_services_for_studio(xblock.runtime, request.user)
+            if view_name == STUDIO_VIEW and xblock.xmodule_runtime is None:
+                xblock.xmodule_runtime = StudioEditModuleRuntime(request.user)
 
             try:
                 fragment = xblock.render(view_name)
@@ -509,7 +524,7 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None):
             old_metadata = own_metadata(xblock)
         if old_content is None:
             old_content = xblock.get_explicitly_set_fields_by_scope(Scope.content)
-        load_services_for_studio(xblock.runtime, user)
+        xblock.xmodule_runtime = StudioEditModuleRuntime(user)
         xblock.editor_saved(user, old_metadata, old_content)
 
     # Update after the callback so any changes made in the callback will get persisted.
@@ -922,7 +937,7 @@ def _duplicate_block(parent_usage_key, duplicate_source_usage_key, user, display
             # Allow an XBlock to do anything fancy it may need to when duplicated from another block.
             # These blocks may handle their own children or parenting if needed. Let them return booleans to
             # let us know if we need to handle these or not.
-            load_services_for_studio(dest_block.runtime, user)
+            dest_block.xmodule_runtime = StudioEditModuleRuntime(user)
             children_handled = dest_block.studio_post_duplicate(store, source_item)
 
         # Children are not automatically copied over (and not all xblocks have a 'children' attribute).

cms/djangoapps/contentstore/views/component.py

@@ -32,7 +32,7 @@
 
 from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url
 from .helpers import get_parent_xblock, is_unit, xblock_type_display_name
-from .block import add_container_page_publishing_info, create_xblock_info, load_services_for_studio
+from .block import StudioEditModuleRuntime, add_container_page_publishing_info, create_xblock_info
 
 __all__ = [
     'container_handler',
@@ -560,7 +560,7 @@ def component_handler(request, usage_key_string, handler, suffix=''):
             descriptor = modulestore().get_item(usage_key)
             handler_descriptor = descriptor
             asides = []
-        load_services_for_studio(handler_descriptor.runtime, request.user)
+        handler_descriptor.xmodule_runtime = StudioEditModuleRuntime(request.user)
         resp = handler_descriptor.handle(handler, req, suffix)
     except NoSuchHandlerError:
         log.info("XBlock %s attempted to access missing handler %r", handler_descriptor, handler, exc_info=True)

cms/djangoapps/contentstore/views/preview.py

@@ -24,7 +24,7 @@
 from xmodule.studio_editable import has_author_view
 from xmodule.util.sandboxing import SandboxService
 from xmodule.util.xmodule_django import add_webpack_to_fragment
-from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW
+from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, ModuleSystem
 from cms.djangoapps.xblock_config.models import StudioConfig
 from cms.djangoapps.contentstore.toggles import individualize_anonymous_user_id, ENABLE_COPY_PASTE_FEATURE
 from cms.lib.xblock.field_data import CmsFieldData
@@ -41,7 +41,8 @@
     request_token,
     wrap_fragment,
     wrap_xblock,
-    wrap_xblock_aside
+    wrap_xblock_aside,
+    xblock_local_resource_url
 )
 
 from ..utils import get_visibility_partition_info
@@ -93,72 +94,74 @@ def preview_handler(request, usage_key_string, handler, suffix=''):
     return webob_to_django_response(resp)
 
 
-def handler_url(block, handler_name, suffix='', query='', thirdparty=False):  # lint-amnesty, pylint: disable=unused-argument
+class PreviewModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
     """
-    Handler URL function for Preview
+    An XModule ModuleSystem for use in Studio previews
     """
-    return reverse('preview_handler', kwargs={
-        'usage_key_string': str(block.scope_ids.usage_id),
-        'handler': handler_name,
-        'suffix': suffix,
-    }) + '?' + query
-
-
-def preview_applicable_aside_types(block, applicable_aside_types=None):
-    """
-    Remove acid_aside and honor the config record
-    """
-    if not StudioConfig.asides_enabled(block.scope_ids.block_type):
-        return []
-
-    # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests
-    # (see https://openedx.atlassian.net/browse/TE-811)
-    return [
-        aside_type
-        for aside_type in applicable_aside_types(block)
-        if aside_type != 'acid_aside'
-    ]
+    # xblocks can check for this attribute during rendering to determine if
+    # they are being rendered for preview (i.e. in Studio)
+    is_author_mode = True
 
+    def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False):
+        return reverse('preview_handler', kwargs={
+            'usage_key_string': str(block.scope_ids.usage_id),
+            'handler': handler_name,
+            'suffix': suffix,
+        }) + '?' + query
 
-def render_child_placeholder(block, view_name, context, wrap_block=None):
-    """
-    Renders a placeholder XBlock.
-    """
-    return wrap_block(block, view_name, Fragment(), context)
+    def local_resource_url(self, block, uri):
+        return xblock_local_resource_url(block, uri)
 
+    def applicable_aside_types(self, block):
+        """
+        Remove acid_aside and honor the config record
+        """
+        if not StudioConfig.asides_enabled(block.scope_ids.block_type):
+            return []
+
+        # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests
+        # (see https://openedx.atlassian.net/browse/TE-811)
+        return [
+            aside_type
+            for aside_type in super().applicable_aside_types(block)
+            if aside_type != 'acid_aside'
+        ]
+
+    def render_child_placeholder(self, block, view_name, context):
+        """
+        Renders a placeholder XBlock.
+        """
+        return self.wrap_xblock(block, view_name, Fragment(), context)
 
-def preview_layout_asides(block, context, frag, view_name, aside_frag_fns, wrap_aside=None):
-    """
-    Custom layout of asides for preview
-    """
-    position_for_asides = '<!-- footer for xblock_aside -->'
-    result = Fragment()
-    result.add_fragment_resources(frag)
+    def layout_asides(self, block, context, frag, view_name, aside_frag_fns):
+        position_for_asides = '<!-- footer for xblock_aside -->'
+        result = Fragment()
+        result.add_fragment_resources(frag)
 
-    for aside, aside_fn in aside_frag_fns:
-        aside_frag = aside_fn(block, context)
-        if aside_frag.content != '':
-            aside_frag_wrapped = wrap_aside(block, aside, view_name, aside_frag, context)
-            aside.save()
-            result.add_fragment_resources(aside_frag_wrapped)
-            replacement = position_for_asides + aside_frag_wrapped.content
-            frag.content = frag.content.replace(position_for_asides, replacement)
+        for aside, aside_fn in aside_frag_fns:
+            aside_frag = aside_fn(block, context)
+            if aside_frag.content != '':
+                aside_frag_wrapped = self.wrap_aside(block, aside, view_name, aside_frag, context)
+                aside.save()
+                result.add_fragment_resources(aside_frag_wrapped)
+                replacement = position_for_asides + aside_frag_wrapped.content
+                frag.content = frag.content.replace(position_for_asides, replacement)
 
-    result.add_content(frag.content)
-    return result
+        result.add_content(frag.content)
+        return result
 
 
-def _prepare_runtime_for_preview(request, block, field_data):
+def _preview_module_system(request, descriptor, field_data):
     """
-    Sets properties in the runtime of the specified block that is
-    required for rendering block previews.
+    Returns a ModuleSystem for the specified descriptor that is specialized for
+    rendering block previews.
 
     request: The active django request
-    field_data: Wrapped field data for previews
+    descriptor: An XModuleDescriptor
     """
 
-    course_id = block.location.course_key
-    display_name_only = (block.category == 'static_tab')
+    course_id = descriptor.location.course_key
+    display_name_only = (descriptor.category == 'static_tab')
 
     replace_url_service = ReplaceURLService(course_id=course_id)
 
@@ -198,48 +201,36 @@ def _prepare_runtime_for_preview(request, block, field_data):
         # the anonymous_user_id to specific courses. These are captured in the
         # block attribute 'requires_per_student_anonymous_id'. Please note,
         # the course_id field in AnynomousUserID model is blank if value is None.
-        if getattr(block, 'requires_per_student_anonymous_id', False):
+        if getattr(descriptor, 'requires_per_student_anonymous_id', False):
             preview_anonymous_user_id = anonymous_id_for_user(request.user, None)
         else:
             preview_anonymous_user_id = anonymous_id_for_user(request.user, course_id)
 
-    services = {
-        "field-data": field_data,
-        "i18n": XBlockI18nService,
-        'mako': mako_service,
-        "settings": SettingsService(),
-        "user": DjangoXBlockUserService(
-            request.user,
-            user_role=get_user_role(request.user, course_id),
-            anonymous_user_id=preview_anonymous_user_id,
-        ),
-        "partitions": StudioPartitionService(course_id=course_id),
-        "teams_configuration": TeamsConfigurationService(),
-        "sandbox": SandboxService(contentstore=contentstore, course_id=course_id),
-        "cache": CacheService(cache),
-        'replace_urls': replace_url_service
-    }
-
-    block.runtime.get_block_for_descriptor = partial(_load_preview_block, request)
-    block.runtime.mixins = settings.XBLOCK_MIXINS
-
-    # Set up functions to modify the fragment produced by student_view
-    block.runtime.wrappers = wrappers
-    block.runtime.wrappers_asides = wrappers_asides
-    block.runtime._runtime_services.update(services)  # lint-amnesty, pylint: disable=protected-access
-
-    # xmodules can check for this attribute during rendering to determine if
-    # they are being rendered for preview (i.e. in Studio)
-    block.runtime.is_author_mode = True
-    block.runtime.handler_url_override = handler_url
-    block.runtime.applicable_aside_types_override = preview_applicable_aside_types
-    block.runtime.render_child_placeholder = partial(
-        render_child_placeholder,
-        wrap_block=block.runtime.wrap_xblock
-    )
-    block.runtime.layout_asides_override = partial(
-        preview_layout_asides,
-        wrap_aside=block.runtime.wrap_aside
+    return PreviewModuleSystem(
+        get_block=partial(_load_preview_block, request),
+        mixins=settings.XBLOCK_MIXINS,
+
+        # Set up functions to modify the fragment produced by student_view
+        wrappers=wrappers,
+        wrappers_asides=wrappers_asides,
+        # Get the raw DescriptorSystem, not the CombinedSystem
+        descriptor_runtime=descriptor._runtime,  # pylint: disable=protected-access
+        services={
+            "field-data": field_data,
+            "i18n": XBlockI18nService,
+            'mako': mako_service,
+            "settings": SettingsService(),
+            "user": DjangoXBlockUserService(
+                request.user,
+                user_role=get_user_role(request.user, course_id),
+                anonymous_user_id=preview_anonymous_user_id,
+            ),
+            "partitions": StudioPartitionService(course_id=course_id),
+            "teams_configuration": TeamsConfigurationService(),
+            "sandbox": SandboxService(contentstore=contentstore, course_id=course_id),
+            "cache": CacheService(cache),
+            'replace_urls': replace_url_service
+        },
     )
 
 
@@ -270,11 +261,12 @@ def _load_preview_block(request, descriptor):
     else:
         wrapper = partial(LmsFieldData, student_data=student_data)
 
-    # wrap the _field_data upfront to pass to _prepare_runtime_for_preview
+    # wrap the _field_data upfront to pass to _preview_module_system
     wrapped_field_data = wrapper(descriptor._field_data)  # pylint: disable=protected-access
-    _prepare_runtime_for_preview(request, descriptor, wrapped_field_data)
+    preview_runtime = _preview_module_system(request, descriptor, wrapped_field_data)
 
     descriptor.bind_for_student(
+        preview_runtime,
         request.user.id,
         [wrapper]
     )

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

@@ -2128,7 +2128,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.
+        # Test environment and Studio use different module systems
+        # (CachingDescriptorSystem is used in tests, PreviewModuleSystem in Studio).
         # CachingDescriptorSystem 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(self.user)  # pylint: disable=protected-access