From 017f8469de4504178361547a3f409f3842c7007f Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Thu, 5 Jan 2023 11:19:17 +0530 Subject: [PATCH] feat: merge ModuleSystem with DescriptorSystem --- cms/djangoapps/contentstore/views/preview.py | 7 +- lms/djangoapps/courseware/block_render.py | 3 + lms/djangoapps/lms_xblock/runtime.py | 4 +- xmodule/x_module.py | 169 +++++++------------ 4 files changed, 73 insertions(+), 110 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index c65e00a0f3..de217dbc24 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -24,7 +24,7 @@ from xmodule.services import SettingsService, TeamsConfigurationService 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, ModuleSystem +from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, DescriptorSystem 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 @@ -94,7 +94,7 @@ def preview_handler(request, usage_key_string, handler, suffix=''): return webob_to_django_response(resp) -class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method +class PreviewModuleSystem(DescriptorSystem): # pylint: disable=abstract-method """ An XModule ModuleSystem for use in Studio previews """ @@ -207,6 +207,9 @@ def _preview_module_system(request, descriptor, field_data): preview_anonymous_user_id = anonymous_id_for_user(request.user, course_id) return PreviewModuleSystem( + load_item=descriptor._runtime.load_item, + resources_fs=descriptor._runtime.resources_fs, + error_tracker=descriptor._runtime.error_tracker, get_block=partial(_load_preview_block, request), mixins=settings.XBLOCK_MIXINS, diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 9d80d8365a..87ea56c517 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -588,6 +588,9 @@ def get_module_system_for_user( store = modulestore() system = LmsModuleSystem( + load_item=descriptor._runtime.load_item, + resources_fs=descriptor._runtime.resources_fs, + error_tracker=descriptor._runtime.error_tracker, get_block=inner_get_block, # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index f79f75da08..ac7f2aa568 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -9,7 +9,7 @@ from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig from openedx.core.djangoapps.user_api.course_tag import api as user_course_tag_api from openedx.core.lib.url_utils import quote_slashes from openedx.core.lib.xblock_utils import wrap_xblock_aside, xblock_local_resource_url -from xmodule.x_module import ModuleSystem # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.x_module import DescriptorSystem # lint-amnesty, pylint: disable=wrong-import-order def handler_url(block, handler_name, suffix='', query='', thirdparty=False): @@ -116,7 +116,7 @@ class UserTagsService: ) -class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method +class LmsModuleSystem(DescriptorSystem): # pylint: disable=abstract-method """ ModuleSystem specialized to the LMS """ diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 5f4d54d7e0..d99a549f3e 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1177,11 +1177,27 @@ class ModuleSystemShim: 'Use MakoService.render_template or a JavaScript-based template instead.', DeprecationWarning, stacklevel=2, ) + if hasattr(self, '_deprecated_render_template'): + return self._deprecated_render_template render_service = self._services.get('mako') if render_service: return render_service.render_template return None + @render_template.setter + def render_template(self, render_template): + """ + Set render_template. + + Deprecated in favor of the mako service. + """ + warnings.warn( + 'Use of runtime.render_template is deprecated. ' + 'Use MakoService.render_template or a JavaScript-based template instead.', + DeprecationWarning, stacklevel=2, + ) + self._deprecated_render_template = render_template + @property def xqueue(self): """ @@ -1382,15 +1398,39 @@ class ModuleSystemShim: "`runtime.course_id` is deprecated. Use `context_key` instead: `runtime.scope_ids.usage_id.context_key`.", DeprecationWarning, stacklevel=3, ) + if hasattr(self, '_deprecated_course_id'): + return self._deprecated_course_id return self.descriptor_runtime.course_id.for_branch(None) + @course_id.setter + def course_id(self, course_id): + """ + Set course_id. -class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): + Deprecated in favor of `runtime.scope_ids.usage_id.context_key`. + """ + warnings.warn( + "`runtime.course_id` is deprecated. Use `context_key` instead: `runtime.scope_ids.usage_id.context_key`.", + DeprecationWarning, stacklevel=3, + ) + self._deprecated_course_id = course_id + + +class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): """ Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s """ + + def get(self, attr): + """ provide uniform access to attributes (like etree).""" + return self.__dict__.get(attr) + + def set(self, attr, val): + """provide uniform access to attributes (like etree)""" + self.__dict__[attr] = val + def __init__( - self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], **kwargs + self, load_item, resources_fs, error_tracker, descriptor_runtime=None, get_policy=None, disabled_xblock_types=lambda: [], get_module=None, **kwargs ): """ load_item: Takes a Location and returns an XModuleDescriptor @@ -1426,9 +1466,23 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): self.get_policy = lambda u: {} self.disabled_xblock_types = disabled_xblock_types + self.get_module = get_module + self.descriptor_runtime = descriptor_runtime + + def get(self, attr): + """ provide uniform access to attributes (like etree).""" + return self.__dict__.get(attr) + + def set(self, attr, val): + """provide uniform access to attributes (like etree)""" + self.__dict__[attr] = val def get_block(self, usage_id, for_parent=None): """See documentation for `xblock.runtime:Runtime.get_block`""" + if self.get_module: + # return self.get_module(block) + return self.get_module(self.descriptor_runtime.get_block(usage_id, for_parent=for_parent)) + return self.load_item(usage_id, for_parent=for_parent) def load_block_type(self, block_type): @@ -1503,8 +1557,12 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): block.add_xml_to_node(child) def publish(self, block, event_type, event): # lint-amnesty, pylint: disable=arguments-differ - # A stub publish method that doesn't emit any events from XModuleDescriptors. - pass + """ + Publish events through the `EventPublishingService`. + This ensures that the correct track method is used for Instructor tasks. + """ + if publish_service := self._services.get('publish'): + publish_service.publish(block, event_type, event) def service(self, block, service_name): """ @@ -1644,107 +1702,6 @@ class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstr field_value[key] = self._make_usage_key(course_key, subvalue) setattr(xblock, field.name, field_value) - -class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): - """ - This is an abstraction such that x_modules can function independent - of the courseware (e.g. import into other types of courseware, LMS, - or if we want to have a sandbox server for user-contributed content) - - ModuleSystem objects are passed to x_modules to provide access to system - functionality. - - Note that these functions can be closures over e.g. a django request - and user, or other environment-specific info. - """ - - def __init__( - self, - get_block, - descriptor_runtime, - **kwargs, - ): - """ - Create a closure around the system environment. - - get_block - function that takes a descriptor and returns a corresponding - block instance object. If the current user does not have - access to that location, returns None. - - descriptor_runtime - A `DescriptorSystem` to use for loading xblocks by id - """ - - kwargs.setdefault('id_reader', getattr(descriptor_runtime, 'id_reader', OpaqueKeyReader())) - kwargs.setdefault('id_generator', getattr(descriptor_runtime, 'id_generator', AsideKeyGenerator())) - super().__init__(**kwargs) - - self.get_block_for_descriptor = get_block - - self.xmodule_instance = None - - self.descriptor_runtime = descriptor_runtime - - def get(self, attr): - """ provide uniform access to attributes (like etree).""" - return self.__dict__.get(attr) - - def set(self, attr, val): - """provide uniform access to attributes (like etree)""" - self.__dict__[attr] = val - - def __repr__(self): - kwargs = self.__dict__.copy() - - # Remove value set transiently by XBlock - kwargs.pop('_view_name') - - return f"{self.__class__.__name__}{kwargs}" - - @property - def ajax_url(self): - """ - The url prefix to be used by XModules to call into handle_ajax - """ - assert self.xmodule_instance is not None - return self.handler_url(self.xmodule_instance, 'xmodule_handler', '', '').rstrip('/?') - - def get_block(self, block_id, for_parent=None): # lint-amnesty, pylint: disable=arguments-differ - return self.get_block_for_descriptor(self.descriptor_runtime.get_block(block_id, for_parent=for_parent)) - - def resource_url(self, resource): - raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls") - - def publish(self, block, event_type, event): # lint-amnesty, pylint: disable=arguments-differ - """ - Publish events through the `EventPublishingService`. - This ensures that the correct track method is used for Instructor tasks. - """ - if publish_service := self._services.get('publish'): - publish_service.publish(block, event_type, event) - - def service(self, block, service_name): - """ - Runtime-specific override for the XBlock service manager. If a service is not currently - instantiated and is declared as a critical requirement, an attempt is made to load the - module. - - Arguments: - block (an XBlock): this block's class will be examined for service - decorators. - service_name (string): the name of the service requested. - - Returns: - An object implementing the requested service, or None. - """ - # getting the service from parent module. making sure of block service declarations. - service = super().service(block=block, service_name=service_name) - # Passing the block to service if it is callable e.g. XBlockI18nService. It is the responsibility of calling - # service to handle the passing argument. - if callable(service): - return service(block) - return service - - class CombinedSystem: """ This class is a shim to allow both pure XBlocks and XModuleDescriptors @@ -1857,4 +1814,4 @@ class DoNothingCache: return None def set(self, key, value, timeout=None): - pass \ No newline at end of file + pass