Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

feat: remove CombinedSystem

Browse Source
This commit is contained in:
Kaustav Banerjee
2023-02-04 12:47:46 +05:30
committed by Agrendalath
parent 248c090eee
commit 09e1197053
6 changed files with 63 additions and 334 deletions
Download Patch File Download Diff File Expand all files Collapse all files

157
cms/djangoapps/contentstore/views/preview.py
View File

@@ -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, DescriptorSystem
from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW
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
@@ -145,101 +145,13 @@ def preview_layout_asides(block, context, frag, view_name, aside_frag_fns, wrap_
return result
class PreviewModuleSystem(DescriptorSystem): # pylint: disable=abstract-method
"""
An XModule ModuleSystem for use in Studio previews
"""
# xblocks can check for this attribute during rendering to determine if
# they are being rendered for preview (i.e. in Studio)
#######################
#######################
## Set directly to system below
#######################
#######################
# is_author_mode = True
#######################
#######################
## Implemented as handler_url above
#######################
#######################
# 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
#######################
#######################
## Being monkey patched in Descriptor system from cms.djangoapps.xblock_config.apps.py
#######################
#######################
# def local_resource_url(self, block, uri):
# return xblock_local_resource_url(block, uri)
#######################
#######################
## Implemented as preview_applicable_aside_types above
#######################
#######################
# 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'
# ]
#######################
#######################
## Implemented as render_child_placeholder above
#######################
#######################
# def render_child_placeholder(self, block, view_name, context):
# """
# Renders a placeholder XBlock.
# """
# return self.wrap_xblock(block, view_name, Fragment(), context)
#######################
#######################
## Implemented as preview_layout_asides above
#######################
#######################
# 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 = 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
def _preview_module_system(request, descriptor, field_data):
"""
Returns a ModuleSystem for the specified descriptor that is specialized for
rendering block previews.
Sets properties in the runtime of the specified descriptor that is
required for rendering block previews.
request: The active django request
descriptor: An XModuleDescriptor
field_data: Wrapped field data for previews
"""
course_id = descriptor.location.course_key
@@ -305,58 +217,28 @@ def _preview_module_system(request, descriptor, field_data):
'replace_urls': replace_url_service
}
# system = PreviewModuleSystem(
# load_item=descriptor._runtime.load_item,
# resources_fs=descriptor._runtime.resources_fs,
# error_tracker=descriptor._runtime.error_tracker,
# # get_module=partial(_load_preview_module, 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": ModuleI18nService,
# # '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
# # },
# )
descriptor._runtime.get_block = partial(_load_preview_block, request),
descriptor._runtime.mixins = settings.XBLOCK_MIXINS
descriptor.runtime.get_block_for_descriptor = partial(_load_preview_block, request),
descriptor.runtime.mixins = settings.XBLOCK_MIXINS
# Set up functions to modify the fragment produced by student_view
descriptor._runtime.wrappers = wrappers
descriptor._runtime.wrappers_asides=wrappers_asides
descriptor._runtime._services.update(services)
descriptor.runtime.wrappers = wrappers
descriptor.runtime.wrappers_asides = wrappers_asides
descriptor.runtime._services.update(services)
descriptor._runtime.is_author_mode = True
descriptor._runtime.handler_url_override = handler_url
descriptor._runtime.applicable_aside_types_override = preview_applicable_aside_types
descriptor._runtime.render_child_placeholder = partial(
# xmodules can check for this attribute during rendering to determine if
# they are being rendered for preview (i.e. in Studio)
descriptor.runtime.is_author_mode = True
descriptor.runtime.handler_url_override = handler_url
descriptor.runtime.applicable_aside_types_override = preview_applicable_aside_types
descriptor.runtime.render_child_placeholder = partial(
render_child_placeholder,
wrap_xblock = descriptor._runtime.wrap_xblock
wrap_xblock = descriptor.runtime.wrap_xblock
)
descriptor._runtime.layout_asides_override = partial(
descriptor.runtime.layout_asides_override = partial(
preview_layout_asides,
wrap_aside = descriptor._runtime.wrap_aside
wrap_aside = descriptor.runtime.wrap_aside
)
return descriptor._runtime
class StudioPartitionService(PartitionService):
"""
@@ -387,10 +269,9 @@ def _load_preview_block(request, descriptor):
# wrap the _field_data upfront to pass to _preview_module_system
wrapped_field_data = wrapper(descriptor._field_data) # pylint: disable=protected-access
preview_runtime = _preview_module_system(request, descriptor, wrapped_field_data)
_preview_module_system(request, descriptor, wrapped_field_data)
descriptor.bind_for_student(
preview_runtime,
request.user.id,
[wrapper]
)

3
cms/djangoapps/contentstore/views/tests/test_block.py
View File

@@ -2128,8 +2128,7 @@ class TestEditSplitModule(ItemTest):
group_id_to_child = split_test.group_id_to_child.copy()
self.assertEqual(2, len(group_id_to_child))
# Test environment and Studio use different module systems
# (CachingDescriptorSystem is used in tests, PreviewModuleSystem in Studio).
# CachingDescriptorSystem is used in tests.
# 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

79
lms/djangoapps/courseware/block_render.py
View File

@@ -115,7 +115,7 @@ class LmsModuleRenderError(Exception):
def make_track_function(request):
'''
Make a tracking function that logs what happened.
For use in ModuleSystem.
For use in DescriptorSystem.
'''
from common.djangoapps.track import views as track_views
@@ -621,60 +621,18 @@ def get_module_system_for_user(
'publish': EventPublishingService(user, course_id, track_function),
}
descriptor.runtime.get_block_for_descriptor = inner_get_block
# system = LmsModuleSystem(
# load_item=descriptor._runtime.load_item,
# resources_fs=descriptor._runtime.resources_fs,
# error_tracker=descriptor._runtime.error_tracker,
# # get_module=inner_get_module,
# # 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
# # wrappers=block_wrappers,
# # services={
# # 'fs': FSService(),
# # 'field-data': field_data,
# # 'mako': mako_service,
# # 'user': user_service,
# # 'verification': XBlockVerificationService(),
# # 'proctoring': ProctoringService(),
# # 'milestones': milestones_helpers.get_service(),
# # 'credit': CreditService(),
# # 'bookmarks': BookmarksService(user=user),
# # 'gating': GatingService(),
# # 'grade_utils': GradesUtilService(course_id=course_id),
# # 'user_state': UserStateService(),
# # 'content_type_gating': ContentTypeGatingService(),
# # 'cache': CacheService(cache),
# # 'sandbox': SandboxService(contentstore=contentstore, course_id=course_id),
# # 'xqueue': xqueue_service,
# # 'replace_urls': replace_url_service,
# # 'rebind_user': rebind_user_service,
# # 'completion': CompletionService(user=user, context_key=course_id)
# # if user and user.is_authenticated
# # else None,
# # 'i18n': ModuleI18nService,
# # 'library_tools': LibraryToolsService(store, user_id=user.id if user else None),
# # 'partitions': PartitionService(course_id=course_id, cache=DEFAULT_REQUEST_CACHE.data),
# # 'settings': SettingsService(),
# # 'user_tags': UserTagsService(user=user, course_id=course_id),
# # 'badging': BadgingService(course_id=course_id, modulestore=store) if badges_enabled() else None,
# # 'teams': TeamsService(),
# # 'teams_configuration': TeamsConfigurationService(),
# # 'call_to_action': CallToActionService(),
# # 'publish': EventPublishingService(user, course_id, track_function),
# # },
# # descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access
# # request_token=request_token,
# )
# 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
descriptor.runtime.mixins = descriptor.runtime.mixologist._mixins
descriptor.runtime.wrappers = block_wrappers
descriptor.runtime._services.update(services)
descriptor.runtime.request_token = request_token
descriptor._runtime.get_block = inner_get_block
descriptor._runtime.mixins = descriptor.runtime.mixologist._mixins
descriptor._runtime.wrappers = block_wrappers
descriptor._runtime._services.update(services)
descriptor._runtime.request_token = request_token
descriptor._runtime.wrap_asides_override = lms_wrappers_aside
descriptor._runtime.applicable_aside_types_override = lms_applicable_aside_types
descriptor.runtime.wrap_asides_override = lms_wrappers_aside
descriptor.runtime.applicable_aside_types_override = lms_applicable_aside_types
# pass position specified in URL to module through ModuleSystem
if position is not None:
@@ -684,14 +642,14 @@ def get_module_system_for_user(
log.exception('Non-integer %r passed as position.', position)
position = None
descriptor._runtime.set('position', position)
descriptor.runtime.set('position', position)
descriptor._runtime.set('user_is_staff', user_is_staff)
descriptor._runtime.set('user_is_admin', bool(has_access(user, 'staff', 'global')))
descriptor._runtime.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user))
descriptor._runtime.set('days_early_for_beta', descriptor.days_early_for_beta)
descriptor.runtime.set('user_is_staff', user_is_staff)
descriptor.runtime.set('user_is_admin', bool(has_access(user, 'staff', 'global')))
descriptor.runtime.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user))
descriptor.runtime.set('days_early_for_beta', descriptor.days_early_for_beta)
return descriptor._runtime, field_data
return field_data
# TODO: Find all the places that this method is called and figure out how to
@@ -709,7 +667,7 @@ def get_block_for_descriptor_internal(user, descriptor, student_data, course_id,
request_token (str): A unique token for this request, used to isolate xblock rendering
"""
(system, student_data) = get_module_system_for_user(
student_data = get_module_system_for_user(
user=user,
student_data=student_data, # These have implicit user bindings, the rest of args are considered not to
descriptor=descriptor,
@@ -727,7 +685,6 @@ def get_block_for_descriptor_internal(user, descriptor, student_data, course_id,
)
descriptor.bind_for_student(
system,
user.id,
[
partial(DateLookupFieldData, course_id=course_id, user=user),

4
xmodule/seq_block.py
View File

@@ -300,11 +300,11 @@ class SequenceBlock(
self.gated_sequence_paywall = None
def bind_for_student(self, xmodule_runtime, user_id, wrappers=None):
def bind_for_student(self, user_id, wrappers=None):
# The position of the child XBlock to select can also be passed in via the URL.
# In such cases the value is set on the ModuleSystem in get_module_system_for_user()
# and needs to be read here after the ModuleSystem has been set on the XBlock.
super().bind_for_student(xmodule_runtime, user_id, wrappers)
super().bind_for_student(user_id, wrappers)
# If position is specified in system, then use that instead.
position = getattr(self.runtime, 'position', None)
if position is not None:

6
xmodule/services.py
View File

@@ -202,7 +202,7 @@ class RebindUserService(Service):
with modulestore().bulk_operations(self.course_id):
course = modulestore().get_course(course_key=self.course_id)
(inner_system, inner_student_data) = self._ref["get_module_system_for_user"](
inner_student_data = self._ref["get_module_system_for_user"](
user=real_user,
student_data=student_data_real_user, # These have implicit user bindings, rest of args considered not to
descriptor=block,
@@ -212,7 +212,6 @@ class RebindUserService(Service):
)
block.bind_for_student(
inner_system,
real_user.id,
[
partial(DateLookupFieldData, course_id=self.course_id, user=self.user),
@@ -223,8 +222,7 @@ class RebindUserService(Service):
block.scope_ids = block.scope_ids._replace(user_id=real_user.id)
# now bind the module to the new ModuleSystem instance and vice-versa
block.runtime = inner_system
inner_system.xmodule_instance = block
block.runtime.xmodule_instance = block
class EventPublishingService(Service):

148
xmodule/x_module.py
View File

@@ -313,19 +313,31 @@ class XModuleMixin(XModuleFields, XBlock):
icon_class = 'other'
def __init__(self, *args, **kwargs):
self.xmodule_runtime = None
self._asides = []
super().__init__(*args, **kwargs)
@property
def runtime(self):
return CombinedSystem(self.xmodule_runtime, self._runtime)
return self._runtime
@runtime.setter
def runtime(self, value):
self._runtime = value
@property
def xmodule_runtime(self):
"""
Shim to maintain backward compatibility.
Deprecated in favor of the runtime property.
"""
warnings.warn(
'xmodule_runtime property is deprecated. Please use the runtime property instead.',
DeprecationWarning, stacklevel=3,
)
return self._runtime
@property
def system(self):
"""
@@ -593,12 +605,11 @@ class XModuleMixin(XModuleFields, XBlock):
"""
return None
def bind_for_student(self, xmodule_runtime, user_id, wrappers=None):
def bind_for_student(self, user_id, wrappers=None):
"""
Set up this XBlock to act as an XModule instead of an XModuleDescriptor.
Arguments:
xmodule_runtime (:class:`ModuleSystem'): the runtime to use when accessing student facing methods
user_id: The user_id to set in scope_ids
wrappers: These are a list functions that put a wrapper, such as
LmsFieldData or OverrideFieldData, around the field_data.
@@ -608,8 +619,8 @@ class XModuleMixin(XModuleFields, XBlock):
# Skip rebinding if we're already bound a user, and it's this user.
if self.scope_ids.user_id is not None and user_id == self.scope_ids.user_id:
if getattr(xmodule_runtime, 'position', None):
self.position = xmodule_runtime.position # update the position of the tab
if getattr(self._runtime, 'position', None):
self.position = self._runtime.position # update the position of the tab
return
# If we are switching users mid-request, save the data from the old user.
@@ -634,9 +645,6 @@ class XModuleMixin(XModuleFields, XBlock):
if field in self._dirty_fields:
del self._dirty_fields[field]
# Set the new xmodule_runtime and field_data (which are user-specific)
self.xmodule_runtime = xmodule_runtime
if wrappers is None:
wrappers = []
@@ -1013,6 +1021,7 @@ class MetricsMixin:
"""
def render(self, block, view_name, context=None): # lint-amnesty, pylint: disable=missing-function-docstring
context = context or {}
start_time = time.time()
try:
return super().render(block, view_name, context=context)
@@ -1421,7 +1430,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s
"""
def __init__(
self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], get_module=None, **kwargs
self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], **kwargs
):
"""
load_item: Takes a Location and returns an XModuleDescriptor
@@ -1457,11 +1466,6 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
self.get_policy = lambda u: {}
self.disabled_xblock_types = disabled_xblock_types
self.get_module = get_module
# self.handler_url_override = None
# self.applicable_aside_types_override = None
# self.wrap_asides_override = None
# self.layout_asides_override = None
def get(self, attr):
""" provide uniform access to attributes (like etree)."""
@@ -1474,8 +1478,8 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
def get_block(self, usage_id, for_parent=None):
"""See documentation for `xblock.runtime:Runtime.get_block`"""
block = self.load_item(usage_id, for_parent=for_parent)
if self.get_module:
return self.get_module(block)
if self.get_block_for_descriptor:
return self.get_block_for_descriptor(block)
return block
def load_block_type(self, block_type):
@@ -1537,10 +1541,6 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
return self.applicable_aside_types_override(block, applicable_aside_types=super().applicable_aside_types)
potential_set = set(super().applicable_aside_types(block))
if getattr(block, 'xmodule_runtime', None) is not None:
if hasattr(block.xmodule_runtime, 'applicable_aside_types'):
application_set = set(block.xmodule_runtime.applicable_aside_types(block))
return list(potential_set.intersection(application_set))
return list(potential_set)
def resource_url(self, resource):
@@ -1711,112 +1711,6 @@ class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstr
setattr(xblock, field.name, field_value)
class CombinedSystem:
"""
This class is a shim to allow both pure XBlocks and XModuleDescriptors
that have been bound as XModules to access both the attributes of ModuleSystem
and of DescriptorSystem as a single runtime.
"""
__slots__ = ('_module_system', '_descriptor_system')
# This system doesn't override a number of methods that are provided by ModuleSystem and DescriptorSystem,
# namely handler_url, local_resource_url, query, and resource_url.
#
# At runtime, the ModuleSystem and/or DescriptorSystem will define those methods
#
def __init__(self, module_system, descriptor_system):
# These attributes are set directly to __dict__ below to avoid a recursion in getattr/setattr.
self._module_system = module_system
self._descriptor_system = descriptor_system
def render(self, block, view_name, context=None):
"""
Render a block by invoking its view.
Finds the view named `view_name` on `block`. The default view will be
used if a specific view hasn't be registered. If there is no default
view, an exception will be raised.
The view is invoked, passing it `context`. The value returned by the
view is returned, with possible modifications by the runtime to
integrate it into a larger whole.
"""
context = context or {}
return self.__getattr__('render')(block, view_name, context) # pylint: disable=unnecessary-dunder-call
def service(self, block, service_name):
"""Return a service, or None.
Services are objects implementing arbitrary other interfaces. They are
requested by agreed-upon names, see [XXX TODO] for a list of possible
services. The object returned depends on the service requested.
XBlocks must announce their intention to request services with the
`XBlock.needs` or `XBlock.wants` decorators. Use `needs` if you assume
that the service is available, or `wants` if your code is flexible and
can accept a None from this method.
Runtimes can override this method if they have different techniques for
finding and delivering services.
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.
"""
service = None
if self._module_system:
service = self._module_system.service(block, service_name)
if service is None:
service = self._descriptor_system.service(block, service_name)
return service
def __getattr__(self, name):
"""
If the ModuleSystem doesn't have an attribute, try returning the same attribute from the
DescriptorSystem, instead. This allows XModuleDescriptors that are bound as XModules
to still function as XModuleDescriptors.
"""
# First we try a lookup in the module system...
try:
return getattr(self._module_system, name)
except AttributeError:
return getattr(self._descriptor_system, name)
def __setattr__(self, name, value):
"""
If the ModuleSystem is set, set the attr on it.
Always set the attr on the DescriptorSystem.
"""
if name in self.__slots__:
return super().__setattr__(name, value)
if self._module_system:
setattr(self._module_system, name, value)
setattr(self._descriptor_system, name, value)
def __delattr__(self, name):
"""
If the ModuleSystem is set, delete the attribute from it.
Always delete the attribute from the DescriptorSystem.
"""
if self._module_system:
delattr(self._module_system, name)
delattr(self._descriptor_system, name)
def __repr__(self):
return f"CombinedSystem({self._module_system!r}, {self._descriptor_system!r})"
class DoNothingCache:
"""A duck-compatible object to use in ModuleSystem when there's no cache."""
def get(self, _key):