diff --git a/cms/djangoapps/contentstore/tests/test_i18n.py b/cms/djangoapps/contentstore/tests/test_i18n.py index e38094a2d3c5..ebd5616b9d5e 100644 --- a/cms/djangoapps/contentstore/tests/test_i18n.py +++ b/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): diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 7c28f005011b..c65aaace82d8 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/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). diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 7f774ab161d2..457f84793efc 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/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) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 12b00d16e6ad..c65e00a0f36a 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/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 = '' - result = Fragment() - result.add_fragment_resources(frag) + def layout_asides(self, block, context, frag, view_name, aside_frag_fns): + position_for_asides = '' + 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] ) diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 863da2e02f5d..7f5cbfe2be85 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/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 diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index c8d126685387..41cfdbeeb4ac 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -26,7 +26,7 @@ from common.djangoapps import static_replace from common.djangoapps.student.tests.factories import UserFactory -from ..preview import _prepare_runtime_for_preview, get_preview_fragment +from ..preview import _preview_module_system, get_preview_fragment @ddt.ddt @@ -214,12 +214,12 @@ def test_expected_services_exist(self, expected_service): Tests that the 'user' and 'i18n' services are provided by the Studio runtime. """ descriptor = BlockFactory(category="pure", parent=self.course) - _prepare_runtime_for_preview( + runtime = _preview_module_system( self.request, descriptor, self.field_data, ) - service = descriptor.runtime.service(descriptor, expected_service) + service = runtime.service(descriptor, expected_service) self.assertIsNotNone(service) @@ -245,16 +245,15 @@ def setUp(self): self.descriptor = BlockFactory(category="video", parent=course) self.field_data = mock.Mock() self.contentstore = contentstore() - self.descriptor = BlockFactory(category="problem", parent=course) - _prepare_runtime_for_preview( + self.runtime = _preview_module_system( self.request, - block=self.descriptor, + descriptor=BlockFactory(category="problem", parent=course), field_data=mock.Mock(), ) self.course = self.store.get_item(course.location) def test_get_user_role(self): - assert self.descriptor.runtime.get_user_role() == 'staff' + assert self.runtime.get_user_role() == 'staff' @XBlock.register_temp_plugin(PureXBlock, identifier='pure') def test_render_template(self): @@ -264,10 +263,10 @@ def test_render_template(self): @override_settings(COURSES_WITH_UNSAFE_CODE=[r'course-v1:edX\+LmsModuleShimTest\+2021_Fall']) def test_can_execute_unsafe_code(self): - assert self.descriptor.runtime.can_execute_unsafe_code() + assert self.runtime.can_execute_unsafe_code() def test_cannot_execute_unsafe_code(self): - assert not self.descriptor.runtime.can_execute_unsafe_code() + assert not self.runtime.can_execute_unsafe_code() @override_settings(PYTHON_LIB_FILENAME=PYTHON_LIB_FILENAME) def test_get_python_lib_zip(self): @@ -277,7 +276,7 @@ def test_get_python_lib_zip(self): source_file=self.PYTHON_LIB_SOURCE_FILE, target_filename=self.PYTHON_LIB_FILENAME, ) - assert self.descriptor.runtime.get_python_lib_zip() == zipfile + assert self.runtime.get_python_lib_zip() == zipfile def test_no_get_python_lib_zip(self): zipfile = upload_file_to_course( @@ -286,40 +285,38 @@ def test_no_get_python_lib_zip(self): source_file=self.PYTHON_LIB_SOURCE_FILE, target_filename=self.PYTHON_LIB_FILENAME, ) - assert self.descriptor.runtime.get_python_lib_zip() is None + assert self.runtime.get_python_lib_zip() is None def test_cache(self): - assert hasattr(self.descriptor.runtime.cache, 'get') - assert hasattr(self.descriptor.runtime.cache, 'set') + assert hasattr(self.runtime.cache, 'get') + assert hasattr(self.runtime.cache, 'set') def test_replace_urls(self): html = '' - assert self.descriptor.runtime.replace_urls(html) == \ + assert self.runtime.replace_urls(html) == \ static_replace.replace_static_urls(html, course_id=self.course.id) def test_anonymous_user_id_preview(self): - assert self.descriptor.runtime.anonymous_student_id == 'student' + assert self.runtime.anonymous_student_id == 'student' @override_waffle_flag(INDIVIDUALIZE_ANONYMOUS_USER_ID, active=True) def test_anonymous_user_id_individual_per_student(self): """Test anonymous_user_id on a block which uses per-student anonymous IDs""" # Create the runtime with the flag turned on. - descriptor = BlockFactory(category="problem", parent=self.course) - _prepare_runtime_for_preview( + runtime = _preview_module_system( self.request, - block=descriptor, + descriptor=BlockFactory(category="problem", parent=self.course), field_data=mock.Mock(), ) - assert descriptor.runtime.anonymous_student_id == '26262401c528d7c4a6bbeabe0455ec46' + assert runtime.anonymous_student_id == '26262401c528d7c4a6bbeabe0455ec46' @override_waffle_flag(INDIVIDUALIZE_ANONYMOUS_USER_ID, active=True) def test_anonymous_user_id_individual_per_course(self): """Test anonymous_user_id on a block which uses per-course anonymous IDs""" # Create the runtime with the flag turned on. - descriptor = BlockFactory(category="lti", parent=self.course) - _prepare_runtime_for_preview( + runtime = _preview_module_system( self.request, - block=descriptor, + descriptor=BlockFactory(category="lti", parent=self.course), field_data=mock.Mock(), ) - assert descriptor.runtime.anonymous_student_id == 'ad503f629b55c531fed2e45aa17a3368' + assert runtime.anonymous_student_id == 'ad503f629b55c531fed2e45aa17a3368' diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 6cf435f5a0b9..9d80d8365a50 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -67,7 +67,7 @@ from lms.djangoapps.courseware.services import UserStateService from lms.djangoapps.grades.api import GradesUtilService from lms.djangoapps.lms_xblock.field_data import LmsFieldData -from lms.djangoapps.lms_xblock.runtime import UserTagsService, lms_wrappers_aside, lms_applicable_aside_types +from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem, UserTagsService from lms.djangoapps.verify_student.services import XBlockVerificationService from openedx.core.djangoapps.bookmarks.api import BookmarksService from openedx.core.djangoapps.crawlers.models import CrawlersConfig @@ -115,7 +115,7 @@ class LmsModuleRenderError(Exception): def make_track_function(request): ''' Make a tracking function that logs what happened. - For use in DescriptorSystem. + For use in ModuleSystem. ''' from common.djangoapps.track import views as track_views @@ -403,11 +403,11 @@ def get_block_for_descriptor(user, request, descriptor, field_data_cache, course ) -def prepare_runtime_for_user( +def get_module_system_for_user( user, student_data, # TODO # Arguments preceding this comment have user binding, those following don't - block, + descriptor, course_id, track_function, request_token, @@ -421,13 +421,14 @@ def prepare_runtime_for_user( will_recheck_access=False, ): """ - Helper function that binds the given xblock to a user and student_data to a user and the block. + Helper function that returns a module system and student_data bound to a user and a descriptor. The purpose of this function is to factor out everywhere a user is implicitly bound when creating a module, - to allow an existing block to be re-bound to a user. + to allow an existing block to be re-bound to a user. Most of the user bindings happen when creating the + closures that feed the instantiation of ModuleSystem. The arguments fall into two categories: those that have explicit or implicit user binding, which are user - and student_data, and those don't and are used to instantiate the service required in LMS, which + and student_data, and those don't and are just present so that ModuleSystem can be instantiated, which are all the other arguments. Ultimately, this isn't too different than how get_block_for_descriptor_internal was before refactoring. @@ -436,7 +437,7 @@ def prepare_runtime_for_user( request_token (str): A token unique to the request use by xblock initialization Returns: - KvsFieldData: student_data bound to, primarily, the user and block + (LmsModuleSystem, KvsFieldData): (module system, student_data) bound to, primarily, the user and descriptor """ def make_xqueue_callback(dispatch='score_update'): @@ -448,7 +449,7 @@ def make_xqueue_callback(dispatch='score_update'): kwargs=dict( course_id=str(course_id), userid=str(user.id), - mod_id=str(block.location), + mod_id=str(descriptor.location), dispatch=dispatch ), ) @@ -458,7 +459,7 @@ def make_xqueue_callback(dispatch='score_update'): # Default queuename is course-specific and is derived from the course that # contains the current block. # TODO: Queuename should be derived from 'course_settings.json' of each course - xqueue_default_queuename = block.location.org + '-' + block.location.course + xqueue_default_queuename = descriptor.location.org + '-' + descriptor.location.course xqueue_service = XQueueService( construct_callback=make_xqueue_callback, @@ -499,12 +500,12 @@ def inner_get_block(descriptor): # while giving selected modules a per-course anonymized id. # As we have the time to manually test more modules, we can add to the list # of modules that get the per-course anonymized id. - if getattr(block, 'requires_per_student_anonymous_id', False): + if getattr(descriptor, 'requires_per_student_anonymous_id', False): anonymous_student_id = anonymous_id_for_user(user, None) else: anonymous_student_id = anonymous_id_for_user(user, course_id) - user_is_staff = bool(has_access(user, 'staff', block.location, course_id)) + user_is_staff = bool(has_access(user, 'staff', descriptor.location, course_id)) user_service = DjangoXBlockUserService( user, user_is_staff=user_is_staff, @@ -517,7 +518,7 @@ def inner_get_block(descriptor): rebind_user_service = RebindUserService( user, course_id, - prepare_runtime_for_user, + get_module_system_for_user, track_function=track_function, position=position, wrap_xblock_display=wrap_xblock_display, @@ -551,9 +552,9 @@ def inner_get_block(descriptor): )) replace_url_service = ReplaceURLService( - data_directory=getattr(block, 'data_dir', None), + data_directory=getattr(descriptor, 'data_dir', None), course_id=course_id, - static_asset_path=static_asset_path or block.static_asset_path, + static_asset_path=static_asset_path or descriptor.static_asset_path, jump_to_id_base_url=reverse('jump_to_id', kwargs={'course_id': str(course_id), 'module_id': ''}) ) @@ -577,58 +578,58 @@ def inner_get_block(descriptor): del user.real_user.masquerade_settings user.real_user.masquerade_settings = masquerade_settings else: - staff_access = has_access(user, 'staff', block, course_id) + staff_access = has_access(user, 'staff', descriptor, course_id) if staff_access: block_wrappers.append(partial(add_staff_markup, user, disable_staff_debug_info)) - field_data = DateLookupFieldData(block._field_data, course_id, user) # pylint: disable=protected-access + field_data = DateLookupFieldData(descriptor._field_data, course_id, user) # pylint: disable=protected-access field_data = LmsFieldData(field_data, student_data) store = modulestore() - 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': XBlockI18nService, - '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), - } - - block.runtime.get_block_for_descriptor = inner_get_block - - block.runtime.wrappers = block_wrappers - block.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access - block.runtime.request_token = request_token - block.runtime.wrap_asides_override = lms_wrappers_aside - block.runtime.applicable_aside_types_override = lms_applicable_aside_types + system = LmsModuleSystem( + 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 + 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': XBlockI18nService, + '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, + ) - # pass position specified in URL to runtime + # pass position specified in URL to module through ModuleSystem if position is not None: try: position = int(position) @@ -636,14 +637,14 @@ def inner_get_block(descriptor): log.exception('Non-integer %r passed as position.', position) position = None - block.runtime.set('position', position) + system.set('position', position) - block.runtime.set('user_is_staff', user_is_staff) - block.runtime.set('user_is_admin', bool(has_access(user, 'staff', 'global'))) - block.runtime.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) - block.runtime.set('days_early_for_beta', block.days_early_for_beta) + system.set('user_is_staff', user_is_staff) + system.set('user_is_admin', bool(has_access(user, 'staff', 'global'))) + system.set('user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) + system.set('days_early_for_beta', descriptor.days_early_for_beta) - return field_data + return system, field_data # TODO: Find all the places that this method is called and figure out how to @@ -661,10 +662,10 @@ 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 """ - student_data = prepare_runtime_for_user( + (system, 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 - block=descriptor, + descriptor=descriptor, course_id=course_id, track_function=track_function, position=position, @@ -679,6 +680,7 @@ 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), diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 99dbf75250c2..6a3869109860 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -33,7 +33,7 @@ from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.tests import get_test_descriptor_system, get_test_system, prepare_block_runtime # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.tests import get_test_descriptor_system, get_test_system # lint-amnesty, pylint: disable=wrong-import-order class BaseTestXmodule(ModuleStoreTestCase): @@ -66,12 +66,10 @@ class BaseTestXmodule(ModuleStoreTestCase): METADATA = {} MODEL_DATA = {'data': ''} - def new_module_runtime(self, runtime=None, **kwargs): + def new_module_runtime(self, **kwargs): """ - Generate a new DescriptorSystem that is minimally set up for testing + Generate a new ModuleSystem that is minimally set up for testing """ - if runtime: - return prepare_block_runtime(runtime, course_id=self.course.id, **kwargs) return get_test_system(course_id=self.course.id, **kwargs) def new_descriptor_runtime(self, **kwargs): @@ -96,7 +94,7 @@ def initialize_module(self, runtime_kwargs=None, **kwargs): # lint-amnesty, pyl if runtime_kwargs is None: runtime_kwargs = {} - self.new_module_runtime(runtime=self.item_descriptor.runtime, **runtime_kwargs) + self.item_descriptor.xmodule_runtime = self.new_module_runtime(**runtime_kwargs) self.item_url = str(self.item_descriptor.location) @@ -150,7 +148,6 @@ def get_url(self, dispatch): class XModuleRenderingTestBase(BaseTestXmodule): # lint-amnesty, pylint: disable=missing-class-docstring - # lint-amnesty, pylint: disable=arguments-differ def new_module_runtime(self, **kwargs): """ Create a runtime that actually does html rendering diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index 37c38486719b..53e10b5c5138 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -37,7 +37,7 @@ from xblock.exceptions import NoSuchServiceError from xblock.field_data import FieldData # lint-amnesty, pylint: disable=wrong-import-order from xblock.fields import ScopeIds # lint-amnesty, pylint: disable=wrong-import-order -from xblock.runtime import DictKeyValueStore, KvsFieldData # lint-amnesty, pylint: disable=wrong-import-order +from xblock.runtime import DictKeyValueStore, KvsFieldData, Runtime # lint-amnesty, pylint: disable=wrong-import-order from xblock.test.tools import TestRuntime # lint-amnesty, pylint: disable=wrong-import-order from xmodule.capa.tests.response_xml_factory import OptionResponseXMLFactory # lint-amnesty, pylint: disable=reimported @@ -58,7 +58,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, CombinedSystem # lint-amnesty, pylint: disable=wrong-import-order from common.djangoapps import static_replace from common.djangoapps.course_modes.models import CourseMode # lint-amnesty, pylint: disable=reimported from common.djangoapps.student.tests.factories import GlobalStaffFactory @@ -1894,10 +1894,9 @@ def _get_anonymous_id(self, course_id, xblock_class): # lint-amnesty, pylint: d location=location, static_asset_path=None, _runtime=Mock( - spec=DescriptorSystem, + spec=Runtime, resources_fs=None, mixologist=Mock(_mixins=(), name='mixologist'), - _services={}, name='runtime', ), scope_ids=Mock(spec=ScopeIds), @@ -1907,7 +1906,7 @@ def _get_anonymous_id(self, course_id, xblock_class): # lint-amnesty, pylint: d fields={}, days_early_for_beta=None, ) - descriptor.runtime = DescriptorSystem(None, None, None) + descriptor.runtime = CombinedSystem(descriptor._runtime, None) # pylint: disable=protected-access # Use the xblock_class's bind_for_student method descriptor.bind_for_student = partial(xblock_class.bind_for_student, descriptor) @@ -1923,7 +1922,7 @@ def _get_anonymous_id(self, course_id, xblock_class): # lint-amnesty, pylint: d request_token='request_token', course=self.course, ) - current_user = block.runtime.service(block, 'user').get_current_user() + current_user = block.xmodule_runtime.service(block, 'user').get_current_user() return current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) @ddt.data(*PER_STUDENT_ANONYMIZED_XBLOCKS) @@ -1977,7 +1976,7 @@ def test_context_contains_display_name(self, mock_tracker): @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') @patch('xmodule.modulestore.mongo.base.CachingDescriptorSystem.applicable_aside_types', lambda self, block: ['test_aside']) - @patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', + @patch('lms.djangoapps.lms_xblock.runtime.LmsModuleSystem.applicable_aside_types', lambda self, block: ['test_aside']) def test_context_contains_aside_info(self, mock_tracker): """ @@ -2186,7 +2185,7 @@ def test_rebind_block_to_new_users(self): # Bind the block to another student, which will remove "correct_map" # from the block's _field_data_cache and _dirty_fields. user2 = UserFactory.create() - block.bind_for_student(user2.id) + block.bind_for_student(block.runtime, user2.id) # XBlock's save method assumes that if a field is in _dirty_fields, # then it's also in _field_data_cache. If this assumption @@ -2195,7 +2194,7 @@ def test_rebind_block_to_new_users(self): # _field_data cache, but not _dirty_fields, when we bound # this block to the second student. (TNL-2640) user3 = UserFactory.create() - block.bind_for_student(user3.id) + block.bind_for_student(block.runtime, user3.id) def test_rebind_noauth_block_to_user_not_anonymous(self): """ @@ -2264,13 +2263,13 @@ def test_event_publishing(self, mock_track_function): class LMSXBlockServiceMixin(SharedModuleStoreTestCase): """ - Helper class that initializes the runtime. + Helper class that initializes the LmsModuleSystem. """ def _prepare_runtime(self): """ - Instantiate the runtem. + Instantiate the LmsModuleSystem. """ - _ = render.prepare_runtime_for_user( + self.runtime, _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2334,7 +2333,7 @@ def test_expected_services_exist(self, expected_service): """ Tests that the 'user', 'i18n', and 'fs' services are provided by the LMS runtime. """ - service = self.descriptor.runtime.service(self.descriptor, expected_service) + service = self.runtime.service(self.descriptor, expected_service) assert service is not None def test_beta_tester_fields_added(self): @@ -2345,8 +2344,8 @@ def test_beta_tester_fields_added(self): self._prepare_runtime() # pylint: disable=no-member - assert not self.descriptor.runtime.user_is_beta_tester - assert self.descriptor.runtime.days_early_for_beta == 5 + assert not self.runtime.user_is_beta_tester + assert self.runtime.days_early_for_beta == 5 def test_get_set_tag(self): """ @@ -2356,23 +2355,23 @@ def test_get_set_tag(self): key = 'key1' # test for when we haven't set the tag yet - tag = self.descriptor.runtime.service(self.descriptor, 'user_tags').get_tag(scope, key) + tag = self.runtime.service(self.descriptor, 'user_tags').get_tag(scope, key) assert tag is None # set the tag set_value = 'value' - self.descriptor.runtime.service(self.descriptor, 'user_tags').set_tag(scope, key, set_value) - tag = self.descriptor.runtime.service(self.descriptor, 'user_tags').get_tag(scope, key) + self.runtime.service(self.descriptor, 'user_tags').set_tag(scope, key, set_value) + tag = self.runtime.service(self.descriptor, 'user_tags').get_tag(scope, key) assert tag == set_value # Try to set tag in wrong scope with pytest.raises(ValueError): - self.descriptor.runtime.service(self.descriptor, 'user_tags').set_tag('fake_scope', key, set_value) + self.runtime.service(self.descriptor, 'user_tags').set_tag('fake_scope', key, set_value) # Try to get tag in wrong scope with pytest.raises(ValueError): - self.descriptor.runtime.service(self.descriptor, 'user_tags').get_tag('fake_scope', key) + self.runtime.service(self.descriptor, 'user_tags').get_tag('fake_scope', key) @ddt.ddt @@ -2382,23 +2381,23 @@ class TestBadgingService(LMSXBlockServiceMixin): @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True}) def test_service_rendered(self): self._prepare_runtime() - assert self.descriptor.runtime.service(self.descriptor, 'badging') + assert self.runtime.service(self.descriptor, 'badging') def test_no_service_rendered(self): with pytest.raises(NoSuchServiceError): - self.descriptor.runtime.service(self.descriptor, 'badging') + self.runtime.service(self.descriptor, 'badging') @ddt.data(True, False) @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True}) def test_course_badges_toggle(self, toggle): self.course = CourseFactory.create(metadata={'issue_badges': toggle}) self._prepare_runtime() - assert self.descriptor.runtime.service(self.descriptor, 'badging').course_badges_enabled is toggle + assert self.runtime.service(self.descriptor, 'badging').course_badges_enabled is toggle @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True}) def test_get_badge_class(self): self._prepare_runtime() - badge_service = self.descriptor.runtime.service(self.descriptor, 'badging') + badge_service = self.runtime.service(self.descriptor, 'badging') premade_badge_class = BadgeClassFactory.create() # Ignore additional parameters. This class already exists. # We should get back the first class we created, rather than a new one. @@ -2422,7 +2421,7 @@ def test_module_i18n_lms_service(self): """ Test: module i18n service in LMS """ - i18n_service = self.descriptor.runtime.service(self.descriptor, 'i18n') + i18n_service = self.runtime.service(self.descriptor, 'i18n') assert i18n_service is not None assert isinstance(i18n_service, XBlockI18nService) @@ -2432,30 +2431,27 @@ def test_no_service_exception_with_none_declaration_(self): """ self.descriptor.service_declaration = Mock(return_value=None) with pytest.raises(NoSuchServiceError): - self.descriptor.runtime.service(self.descriptor, 'i18n') + self.runtime.service(self.descriptor, 'i18n') def test_no_service_exception_(self): """ Test: NoSuchServiceError should be raised if i18n service is none. """ - i18nService = self.descriptor.runtime._services['i18n'] # pylint: disable=protected-access - self.descriptor.runtime._runtime_services['i18n'] = None # pylint: disable=protected-access - self.descriptor.runtime._services['i18n'] = None # pylint: disable=protected-access + self.runtime._services['i18n'] = None # pylint: disable=protected-access with pytest.raises(NoSuchServiceError): - self.descriptor.runtime.service(self.descriptor, 'i18n') - self.descriptor.runtime._services['i18n'] = i18nService # pylint: disable=protected-access + self.runtime.service(self.descriptor, 'i18n') def test_i18n_service_callable(self): """ Test: _services dict should contain the callable i18n service in LMS. """ - assert callable(self.descriptor.runtime._services.get('i18n')) # pylint: disable=protected-access + assert callable(self.runtime._services.get('i18n')) # pylint: disable=protected-access def test_i18n_service_not_callable(self): """ Test: i18n service should not be callable in LMS after initialization. """ - assert not callable(self.descriptor.runtime.service(self.descriptor, 'i18n')) + assert not callable(self.runtime.service(self.descriptor, 'i18n')) class PureXBlockWithChildren(PureXBlock): @@ -2670,7 +2666,7 @@ def setUp(self): self.track_function = Mock() self.request_token = Mock() self.contentstore = contentstore() - _ = render.prepare_runtime_for_user( + self.runtime, _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2690,11 +2686,11 @@ def test_user_service_attributes(self, attribute, expected_value): """ Tests that the deprecated attributes provided by the user service match expected values. """ - assert getattr(self.descriptor.runtime, attribute) == expected_value + assert getattr(self.runtime, attribute) == expected_value @patch('lms.djangoapps.courseware.block_render.has_access', Mock(return_value=True, autospec=True)) def test_user_is_staff(self): - _ = render.prepare_runtime_for_user( + runtime, _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2703,12 +2699,12 @@ def test_user_is_staff(self): self.request_token, course=self.course, ) - assert self.descriptor.runtime.user_is_staff - assert self.descriptor.runtime.get_user_role() == 'student' + assert runtime.user_is_staff + assert runtime.get_user_role() == 'student' @patch('lms.djangoapps.courseware.block_render.get_user_role', Mock(return_value='instructor', autospec=True)) def test_get_user_role(self): - _ = render.prepare_runtime_for_user( + runtime, _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2717,17 +2713,17 @@ def test_get_user_role(self): self.request_token, course=self.course, ) - assert self.descriptor.runtime.get_user_role() == 'instructor' + assert runtime.get_user_role() == 'instructor' def test_anonymous_student_id(self): - assert self.descriptor.runtime.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) + assert self.runtime.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) def test_anonymous_student_id_bug(self): """ - Verifies that subsequent calls to prepare_runtime_for_user have no effect on each block runtime's + Verifies that subsequent calls to get_module_system_for_user have no effect on each block runtime's anonymous_student_id value. """ - _ = render.prepare_runtime_for_user( + problem_runtime, _ = render.get_module_system_for_user( self.user, self.student_data, self.problem_descriptor, @@ -2737,9 +2733,9 @@ def test_anonymous_student_id_bug(self): course=self.course, ) # Ensure the problem block returns a per-user anonymous id - assert self.problem_descriptor.runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) + assert problem_runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) - _ = render.prepare_runtime_for_user( + vertical_runtime, _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2749,13 +2745,13 @@ def test_anonymous_student_id_bug(self): course=self.course, ) # Ensure the vertical block returns a per-course+user anonymous id - assert self.descriptor.runtime.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) + assert vertical_runtime.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) # Ensure the problem runtime's anonymous student ID is unchanged after the above call. - assert self.problem_descriptor.runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) + assert problem_runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) def test_user_service_with_anonymous_user(self): - _ = render.prepare_runtime_for_user( + runtime, _ = render.get_module_system_for_user( AnonymousUser(), self.student_data, self.descriptor, @@ -2764,14 +2760,14 @@ def test_user_service_with_anonymous_user(self): self.request_token, course=self.course, ) - assert self.descriptor.runtime.anonymous_student_id is None - assert self.descriptor.runtime.seed == 0 - assert self.descriptor.runtime.user_id is None - assert not self.descriptor.runtime.user_is_staff - assert not self.descriptor.runtime.get_user_role() + assert runtime.anonymous_student_id is None + assert runtime.seed == 0 + assert runtime.user_id is None + assert not runtime.user_is_staff + assert not runtime.get_user_role() def test_get_real_user(self): - _ = render.prepare_runtime_for_user( + runtime, _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2781,20 +2777,20 @@ def test_get_real_user(self): course=self.course, ) course_anonymous_student_id = anonymous_id_for_user(self.user, self.course.id) - assert self.descriptor.runtime.get_real_user(course_anonymous_student_id) == self.user # pylint: disable=not-callable + assert runtime.get_real_user(course_anonymous_student_id) == self.user # pylint: disable=not-callable no_course_anonymous_student_id = anonymous_id_for_user(self.user, None) - assert self.descriptor.runtime.get_real_user(no_course_anonymous_student_id) == self.user # pylint: disable=not-callable + assert runtime.get_real_user(no_course_anonymous_student_id) == self.user # pylint: disable=not-callable # Tests that the default is to use the user service's anonymous_student_id - assert self.descriptor.runtime.get_real_user() == self.user # pylint: disable=not-callable + assert runtime.get_real_user() == self.user # pylint: disable=not-callable def test_render_template(self): - rendered = self.descriptor.runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable + rendered = self.runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable assert rendered == '
Testing the MakoService
\n' def test_xqueue(self): - xqueue = self.descriptor.runtime.xqueue + xqueue = self.runtime.xqueue assert isinstance(xqueue['interface'], XQueueInterface) assert xqueue['interface'].url == 'http://sandbox-xqueue.edx.org' assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest' @@ -2816,7 +2812,7 @@ def test_xqueue(self): XQUEUE_WAITTIME_BETWEEN_REQUESTS=15, ) def test_xqueue_settings(self): - _ = render.prepare_runtime_for_user( + runtime, _ = render.get_module_system_for_user( self.user, self.student_data, self.descriptor, @@ -2825,7 +2821,7 @@ def test_xqueue_settings(self): self.request_token, course=self.course, ) - xqueue = self.descriptor.runtime.xqueue + xqueue = runtime.xqueue assert isinstance(xqueue['interface'], XQueueInterface) assert xqueue['interface'].url == 'http://xqueue.url' assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest' @@ -2836,14 +2832,14 @@ def test_xqueue_settings(self): @override_settings(COURSES_WITH_UNSAFE_CODE=[r'course-v1:edX\+LmsModuleShimTest\+2021_Fall']) def test_can_execute_unsafe_code_when_allowed(self): - assert self.descriptor.runtime.can_execute_unsafe_code() + assert self.runtime.can_execute_unsafe_code() @override_settings(COURSES_WITH_UNSAFE_CODE=[r'course-v1:edX\+full\+2021_Fall']) def test_cannot_execute_unsafe_code_when_disallowed(self): - assert not self.descriptor.runtime.can_execute_unsafe_code() + assert not self.runtime.can_execute_unsafe_code() def test_cannot_execute_unsafe_code(self): - assert not self.descriptor.runtime.can_execute_unsafe_code() + assert not self.runtime.can_execute_unsafe_code() @override_settings(PYTHON_LIB_FILENAME=PYTHON_LIB_FILENAME) def test_get_python_lib_zip(self): @@ -2853,7 +2849,7 @@ def test_get_python_lib_zip(self): source_file=self.PYTHON_LIB_SOURCE_FILE, target_filename=self.PYTHON_LIB_FILENAME, ) - assert self.descriptor.runtime.get_python_lib_zip() == zipfile + assert self.runtime.get_python_lib_zip() == zipfile def test_no_get_python_lib_zip(self): zipfile = upload_file_to_course( @@ -2862,26 +2858,26 @@ def test_no_get_python_lib_zip(self): source_file=self.PYTHON_LIB_SOURCE_FILE, target_filename=self.PYTHON_LIB_FILENAME, ) - assert self.descriptor.runtime.get_python_lib_zip() is None + assert self.runtime.get_python_lib_zip() is None def test_cache(self): - assert hasattr(self.descriptor.runtime.cache, 'get') - assert hasattr(self.descriptor.runtime.cache, 'set') + assert hasattr(self.runtime.cache, 'get') + assert hasattr(self.runtime.cache, 'set') def test_replace_urls(self): html = '
' - assert self.descriptor.runtime.replace_urls(html) == \ + assert self.runtime.replace_urls(html) == \ static_replace.replace_static_urls(html, course_id=self.course.id) def test_replace_course_urls(self): html = '' - assert self.descriptor.runtime.replace_course_urls(html) == \ + assert self.runtime.replace_course_urls(html) == \ static_replace.replace_course_urls(html, course_key=self.course.id) def test_replace_jump_to_id_urls(self): html = '' jump_to_id_base_url = reverse('jump_to_id', kwargs={'course_id': str(self.course.id), 'module_id': ''}) - assert self.descriptor.runtime.replace_jump_to_id_urls(html) == \ + assert self.runtime.replace_jump_to_id_urls(html) == \ static_replace.replace_jump_to_id_urls(html, self.course.id, jump_to_id_base_url) @XBlock.register_temp_plugin(PureXBlock, 'pure') diff --git a/lms/djangoapps/courseware/tests/test_discussion_xblock.py b/lms/djangoapps/courseware/tests/test_discussion_xblock.py index 159add3e050d..49f76be0f400 100644 --- a/lms/djangoapps/courseware/tests/test_discussion_xblock.py +++ b/lms/djangoapps/courseware/tests/test_discussion_xblock.py @@ -55,6 +55,7 @@ def setUp(self): field_data=self.data, scope_ids=scope_ids ) + self.block.xmodule_runtime = mock.Mock() if self.PATCH_DJANGO_USER: self.django_user_canary = UserFactory() diff --git a/lms/djangoapps/courseware/tests/test_lti_integration.py b/lms/djangoapps/courseware/tests/test_lti_integration.py index e7d7f856ba06..bf85e7db61d3 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/lms/djangoapps/courseware/tests/test_lti_integration.py @@ -41,7 +41,7 @@ def setUp(self): # Note: this course_id is actually a course_key context_id = str(self.item_descriptor.course_id) - user_service = self.item_descriptor.runtime.service(self.item_descriptor, 'user') + user_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'user') user_id = str(user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)) hostname = settings.LMS_BASE resource_link_id = str(urllib.parse.quote(f'{hostname}-{self.item_descriptor.location.html_id()}')) @@ -81,10 +81,8 @@ def setUp(self): 'element_id': self.item_descriptor.location.html_id(), 'launch_url': 'http://www.example.com', # default value 'open_in_a_new_page': True, - 'form_url': self.item_descriptor.runtime.handler_url( - self.item_descriptor, - 'preview_handler' - ).rstrip('/?'), + 'form_url': self.item_descriptor.xmodule_runtime.handler_url(self.item_descriptor, + 'preview_handler').rstrip('/?'), 'hide_launch': False, 'has_score': False, 'module_score': None, diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index 737c6c25b9af..d2a260bf86fc 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -22,8 +22,6 @@ from xmodule.exceptions import NotFoundError # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -# noinspection PyUnresolvedReferences -from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import from xmodule.video_block import VideoBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.video_block.transcripts_utils import ( # lint-amnesty, pylint: disable=wrong-import-order Transcript, @@ -31,7 +29,7 @@ get_transcript, subs_filename, ) -from xmodule.x_module import STUDENT_VIEW +from xmodule.x_module import STUDENT_VIEW # lint-amnesty, pylint: disable=wrong-import-order from .helpers import BaseTestXmodule from .test_video_xml import SOURCE_XML @@ -133,7 +131,6 @@ def attach_bumper_transcript(item, filename, lang="en"): item.video_bumper["transcripts"][lang] = filename -@pytest.mark.usefixtures("override_descriptor_system") class BaseTestVideoXBlock(BaseTestXmodule): """Base class for VideoXBlock tests.""" @@ -235,7 +232,7 @@ def get_handler_url(self, handler, suffix): """ Return the URL for the specified handler on self.item_descriptor. """ - return self.item_descriptor.runtime.handler_url( + return self.item_descriptor.xmodule_runtime.handler_url( self.item_descriptor, handler, suffix ).rstrip('/?') diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 9887c100593d..6c5bb5e8f0ca 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -39,8 +39,6 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, 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_video import VideoBlockTestBase from xmodule.video_block import VideoBlock, bumper_utils, video_utils @@ -138,7 +136,7 @@ def test_video_constructor(self): 'public_video_url': None, } - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -222,7 +220,7 @@ def test_video_constructor(self): 'public_video_url': None, } - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') expected_result = get_context_dict_from_string( mako_service.render_template('video.html', expected_context) ) @@ -243,16 +241,14 @@ class TestVideoPublicAccess(BaseTestVideoXBlock): @ddt.data( (True, False), (False, False), - (False, True), (True, True), ) @ddt.unpack def test_public_video_url(self, is_lms_platform, enable_public_share): """Test public video url.""" assert self.item_descriptor.public_access is True - if not is_lms_platform: - self.item_descriptor.runtime.is_author_mode = True - with patch.object(PUBLIC_VIDEO_SHARE, 'is_enabled', return_value=enable_public_share): + with patch.object(self.item_descriptor, '_is_lms_platform', return_value=is_lms_platform), \ + patch.object(PUBLIC_VIDEO_SHARE, 'is_enabled', return_value=enable_public_share): context = self.item_descriptor.render(STUDENT_VIEW).content # public video url iif PUBLIC_VIDEO_SHARE waffle and is_lms_platform, public_access are true assert bool(get_context_dict_from_string(context)['public_video_url']) \ @@ -309,7 +305,7 @@ def get_handler_url(self, handler, suffix): Return the URL for the specified handler on the block represented by self.item_descriptor. """ - return self.item_descriptor.runtime.handler_url( + return self.item_descriptor.xmodule_runtime.handler_url( self.item_descriptor, handler, suffix ).rstrip('/?') @@ -426,7 +422,7 @@ def test_get_html_track(self): 'metadata': json.dumps(metadata) }) - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -537,7 +533,7 @@ def test_get_html_source(self): 'metadata': json.dumps(expected_context['metadata']) }) - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -680,7 +676,7 @@ def test_get_html_with_mocked_edx_video_id(self): 'metadata': json.dumps(expected_context['metadata']) }) - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -708,7 +704,7 @@ def test_get_html_with_existing_edx_video_id(self): # context returned by get_html when provided with above data # expected_context, a dict to assert with context context, expected_context = self.helper_get_html_with_edx_video_id(data) - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -739,7 +735,7 @@ def test_get_html_with_existing_unstripped_edx_video_id(self): # expected_context, a dict to assert with context context, expected_context = self.helper_get_html_with_edx_video_id(data) - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -940,7 +936,7 @@ def side_effect(*args, **kwargs): # lint-amnesty, pylint: disable=unused-argume self.initialize_block(data=DATA, runtime_kwargs={ 'user_location': 'CN', }) - user_service = self.item_descriptor.runtime.service(self.item_descriptor, 'user') + user_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'user') user_location = user_service.get_current_user().opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] assert user_location == 'CN' context = self.item_descriptor.render('student_view').content @@ -958,7 +954,7 @@ def side_effect(*args, **kwargs): # lint-amnesty, pylint: disable=unused-argume 'metadata': json.dumps(expected_context['metadata']) }) - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -1063,7 +1059,7 @@ def test_get_html_cdn_source_external_video(self): 'metadata': json.dumps(expected_context['metadata']) }) - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') assert get_context_dict_from_string(context) ==\ get_context_dict_from_string(mako_service.render_template('video.html', expected_context)) @@ -2316,7 +2312,7 @@ def test_bumper_metadata(self, get_url_for_profiles, get_bumper_settings, is_bum })) } - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') expected_content = mako_service.render_template('video.html', expected_context) assert get_context_dict_from_string(content) == get_context_dict_from_string(expected_content) @@ -2372,10 +2368,10 @@ def prepare_expected_context(self, autoadvanceenabled_flag, autoadvance_flag): 'ytTestTimeout': 1500, 'ytApiUrl': 'https://www.youtube.com/iframe_api', 'lmsRootURL': settings.LMS_ROOT_URL, - 'transcriptTranslationUrl': self.item_descriptor.runtime.handler_url( + 'transcriptTranslationUrl': self.item_descriptor.xmodule_runtime.handler_url( self.item_descriptor, 'transcript', 'translation/__lang__' ).rstrip('/?'), - 'transcriptAvailableTranslationsUrl': self.item_descriptor.runtime.handler_url( + 'transcriptAvailableTranslationsUrl': self.item_descriptor.xmodule_runtime.handler_url( self.item_descriptor, 'transcript', 'available_translations' ).rstrip('/?'), 'autohideHtml5': False, @@ -2411,7 +2407,7 @@ def assert_content_matches_expectations(self, autoadvanceenabled_must_be, autoad autoadvance_flag=autoadvance_must_be, ) - mako_service = self.item_descriptor.runtime.service(self.item_descriptor, 'mako') + mako_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'mako') with override_settings(FEATURES=self.FEATURES): expected_content = mako_service.render_template('video.html', expected_context) diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index dd367b202335..2ba9ba837466 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -1,19 +1,14 @@ """Word cloud integration tests using mongo modulestore.""" -import pytest - import json from operator import itemgetter -# noinspection PyUnresolvedReferences -from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import from xmodule.x_module import STUDENT_VIEW from .helpers import BaseTestXmodule -@pytest.mark.usefixtures("override_descriptor_system") class TestWordCloud(BaseTestXmodule): """Integration test for Word Cloud Block.""" CATEGORY = "word_cloud" diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index bd549324fcb9..79d2976d2c97 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1487,7 +1487,7 @@ def enclosing_sequence_for_gating_checks(block): if ancestor: # get_parent() returns a parent block instance cached on the block which does not - # have user data bound to it so we need to get it again with get_block() which will set up everything. + # have the ModuleSystem bound to it so we need to get it again with get_block() which will set up everything. return block.runtime.get_block(ancestor.location) return None diff --git a/lms/djangoapps/instructor_task/tasks_helper/module_state.py b/lms/djangoapps/instructor_task/tasks_helper/module_state.py index ec7d8b89d4ae..ddec49cf8a03 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/module_state.py +++ b/lms/djangoapps/instructor_task/tasks_helper/module_state.py @@ -351,7 +351,7 @@ def make_track_function(): ''' Make a tracking function that logs what happened. - For insertion into runtime, and used by CapaModule, which will + For insertion into ModuleSystem, and used by CapaModule, which will provide the event_type (as string) and event (as dict) as arguments. The request_info and task_info (and page) are provided here. ''' @@ -375,7 +375,7 @@ def _get_track_function_for_task(student, xblock_instance_args=None, source_page """ Make a tracking function that logs what happened. - For insertion into runtime, and used by CapaModule, which will + For insertion into ModuleSystem, and used by CapaModule, which will provide the event_type (as string) and event (as dict) as arguments. The request_info and task_info (and page) are provided here. """ diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index e3c290174538..d5e1ef28b1af 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -16,7 +16,6 @@ from unittest.mock import ANY, MagicMock, Mock, patch import ddt -import pytest import unicodecsv from django.conf import settings from django.test.utils import override_settings @@ -71,8 +70,6 @@ from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order from xmodule.partitions.partitions import Group, UserPartition # lint-amnesty, pylint: disable=wrong-import-order -# noinspection PyUnresolvedReferences -from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import from ..models import ReportStore from ..tasks_helper.utils import UPDATE_STATUS_FAILED, UPDATE_STATUS_SUCCEEDED @@ -1083,7 +1080,6 @@ def test_problem_grade_report_valid_columns_order(self): @ddt.ddt -@pytest.mark.usefixtures("override_descriptor_system") class TestProblemReportCohortedContent(TestReportMixin, ContentGroupTestCase, InstructorTaskModuleTestCase): """ Test the problem report on a course that has cohorted content. @@ -1733,7 +1729,6 @@ def test_move_users_to_same_cohort(self): @ddt.ddt @patch('lms.djangoapps.instructor_task.tasks_helper.misc.DefaultStorage', new=MockDefaultStorage) -@pytest.mark.usefixtures("override_descriptor_system") class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): """ Test that grade report has correct grade values. diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index c78627187675..f79f75da0879 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -9,6 +9,7 @@ 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 def handler_url(block, handler_name, suffix='', query='', thirdparty=False): @@ -71,64 +72,6 @@ def local_resource_url(block, uri): return xblock_local_resource_url(block, uri) -def lms_wrappers_aside(block, aside, view, frag, context, request_token=None): - """ - Creates a div which identifies the aside, points to the original block, - and writes out the json_init_args into a script tag. - - The default implementation creates a frag to wraps frag w/ a div identifying the xblock. If you have - javascript, you'll need to override this impl - """ - if not frag.content: - return frag - - runtime_class = 'LmsRuntime' - extra_data = { - 'block-id': quote_slashes(str(block.scope_ids.usage_id)), - 'course-id': quote_slashes(str(block.scope_ids.usage_id.context_key)), - 'url-selector': 'asideBaseUrl', - 'runtime-class': runtime_class, - } - if request_token: - extra_data['request-token'] = request_token - - return wrap_xblock_aside( - runtime_class, - aside, - view, - frag, - context, - usage_id_serializer=str, - request_token=request_token, - extra_data=extra_data, - ) - - -def lms_applicable_aside_types(block, applicable_aside_types=None): - """ - Return all of the asides which might be decorating this `block`. - - Arguments: - block (:class:`.XBlock`): The block to render retrieve asides for. - """ - - config = XBlockAsidesConfig.current() - - if not config.enabled: - return [] - - if block.scope_ids.block_type in config.disabled_blocks.split(): - 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' - ] - - class UserTagsService: """ A runtime class that provides an interface to the user service. It handles filling in @@ -171,3 +114,92 @@ def set_tag(self, scope, key, value): self._user, self._course_id, key, value ) + + +class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method + """ + ModuleSystem specialized to the LMS + """ + def __init__(self, **kwargs): + self.request_token = kwargs.pop('request_token', None) + super().__init__(**kwargs) + + def handler_url(self, *args, **kwargs): # lint-amnesty, pylint: disable=signature-differs + """ + Implement the XBlock runtime handler_url interface. + + This is mostly just proxying to the module level `handler_url` function + defined higher up in this file. + + We're doing this indirection because the module level `handler_url` + logic is also needed by the `DescriptorSystem`. The particular + `handler_url` that a `DescriptorSystem` needs will be different when + running an LMS process or a CMS/Studio process. That's accomplished by + monkey-patching a global. It's a long story, but please know that you + can't just refactor and fold that logic into here without breaking + things. + + https://openedx.atlassian.net/wiki/display/PLAT/Convert+from+Storage-centric+runtimes+to+Application-centric+runtimes + + See :method:`xblock.runtime:Runtime.handler_url` + """ + return handler_url(*args, **kwargs) + + def local_resource_url(self, *args, **kwargs): + return local_resource_url(*args, **kwargs) + + def wrap_aside(self, block, aside, view, frag, context): + """ + Creates a div which identifies the aside, points to the original block, + and writes out the json_init_args into a script tag. + + The default implementation creates a frag to wraps frag w/ a div identifying the xblock. If you have + javascript, you'll need to override this impl + """ + if not frag.content: + return frag + + runtime_class = 'LmsRuntime' + extra_data = { + 'block-id': quote_slashes(str(block.scope_ids.usage_id)), + 'course-id': quote_slashes(str(block.course_id)), + 'url-selector': 'asideBaseUrl', + 'runtime-class': runtime_class, + } + if self.request_token: + extra_data['request-token'] = self.request_token + + return wrap_xblock_aside( + runtime_class, + aside, + view, + frag, + context, + usage_id_serializer=str, + request_token=self.request_token, + extra_data=extra_data, + ) + + def applicable_aside_types(self, block): + """ + Return all of the asides which might be decorating this `block`. + + Arguments: + block (:class:`.XBlock`): The block to render retrieve asides for. + """ + + config = XBlockAsidesConfig.current() + + if not config.enabled: + return [] + + if block.scope_ids.block_type in config.disabled_blocks.split(): + 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' + ] diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index 6c9cf69f4931..046c2139efd0 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -11,8 +11,7 @@ from opaque_keys.edx.locations import BlockUsageLocator, CourseLocator from xblock.fields import ScopeIds -from xmodule.x_module import DescriptorSystem -from lms.djangoapps.lms_xblock.runtime import handler_url +from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem class BlockMock(Mock): @@ -51,13 +50,10 @@ class TestHandlerUrl(TestCase): def setUp(self): super().setUp() self.block = BlockMock(name='block') - self.runtime = DescriptorSystem( - 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') + self.runtime = LmsModuleSystem( + get_block=Mock(), + descriptor_runtime=Mock(), ) - self.runtime.get_block_for_descriptor = Mock() - self.runtime.handler_url_override = handler_url def test_trailing_characters(self): assert not self.runtime.handler_url(self.block, 'handler').endswith('?') diff --git a/openedx/core/djangoapps/xblock/runtime/runtime.py b/openedx/core/djangoapps/xblock/runtime/runtime.py index 132e3b972861..cadc7f4e3309 100644 --- a/openedx/core/djangoapps/xblock/runtime/runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/runtime.py @@ -256,13 +256,13 @@ def service(self, block, service_name): elif service_name == 'replace_urls': return ReplaceURLService(xblock=block, lookup_asset_url=self._lookup_asset_url) elif service_name == 'rebind_user': - # this service should ideally be initialized with all the arguments of prepare_runtime_for_user + # this service should ideally be initialized with all the arguments of get_module_system_for_user # but only the positional arguments are passed here as the other arguments are too # specific to the lms.block_render module return RebindUserService( self.user, context_key, - block_render.prepare_runtime_for_user, + block_render.get_module_system_for_user, track_function=make_track_function(), request_token=request_token(crum.get_current_request()), ) diff --git a/openedx/tests/completion_integration/test_services.py b/openedx/tests/completion_integration/test_services.py index a15ad307dbe6..d7bc197d952d 100644 --- a/openedx/tests/completion_integration/test_services.py +++ b/openedx/tests/completion_integration/test_services.py @@ -13,7 +13,7 @@ from xmodule.library_tools import LibraryToolsService from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, LibraryFactory -from xmodule.tests import prepare_block_runtime +from xmodule.tests import get_test_system from openedx.core.djangolib.testing.utils import skip_unless_lms from common.djangoapps.student.tests.factories import UserFactory @@ -121,17 +121,20 @@ def _bind_course_block(self, block): """ Bind a block (part of self.course) so we can access student-specific data. """ - prepare_block_runtime(block.runtime, course_id=block.location.course_key) - block.runtime._services.update({'library_tools': LibraryToolsService(self.store, self.user.id)}) # lint-amnesty, pylint: disable=protected-access + module_system = get_test_system(course_id=block.location.course_key) + module_system.descriptor_runtime = block.runtime._descriptor_system # pylint: disable=protected-access + module_system._services['library_tools'] = LibraryToolsService(self.store, self.user.id) # pylint: disable=protected-access def get_block(descriptor): """Mocks module_system get_block_for_descriptor function""" - prepare_block_runtime(descriptor.runtime, course_id=block.location.course_key) - descriptor.runtime.get_block_for_descriptor = get_block - descriptor.bind_for_student(self.user.id) + sub_module_system = get_test_system(course_id=block.location.course_key) + sub_module_system.get_block_for_descriptor = get_block + sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access + descriptor.bind_for_student(sub_module_system, self.user.id) return descriptor - block.runtime.get_block_for_descriptor = get_block + module_system.get_block_for_descriptor = get_block + block.xmodule_runtime = module_system def test_completion_service(self): # Only the completions for the user and course specified for the CompletionService diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index be6448097fb5..e71a1b77a16d 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -1186,7 +1186,7 @@ xblock[django]==1.6.2 # xblock-google-drive # xblock-poll # xblock-utils -xblock-drag-and-drop-v2==3.2.0 +xblock-drag-and-drop-v2==3.1.2 # via -r requirements/edx/base.in xblock-google-drive==0.3.0 # via -r requirements/edx/base.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 326dd52d522f..d67b11d69803 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1715,7 +1715,7 @@ xblock[django]==1.6.2 # xblock-google-drive # xblock-poll # xblock-utils -xblock-drag-and-drop-v2==3.2.0 +xblock-drag-and-drop-v2==3.1.2 # via -r requirements/edx/testing.txt xblock-google-drive==0.3.0 # via -r requirements/edx/testing.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index d46e811797e4..07618efd558d 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1583,7 +1583,7 @@ xblock[django]==1.6.2 # xblock-google-drive # xblock-poll # xblock-utils -xblock-drag-and-drop-v2==3.2.0 +xblock-drag-and-drop-v2==3.1.2 # via -r requirements/edx/base.txt xblock-google-drive==0.3.0 # via -r requirements/edx/base.txt diff --git a/xmodule/capa/capa_problem.py b/xmodule/capa/capa_problem.py index 9dd17cc253ea..7a6592e0958c 100644 --- a/xmodule/capa/capa_problem.py +++ b/xmodule/capa/capa_problem.py @@ -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:`ModuleSystem` for documentation of other attributes. """ def __init__( diff --git a/xmodule/mako_block.py b/xmodule/mako_block.py index 6e6a78910465..017b277e032c 100644 --- a/xmodule/mako_block.py +++ b/xmodule/mako_block.py @@ -19,11 +19,12 @@ def __init__(self, render_template, **kwargs): # Add the MakoService to the descriptor system. # - # 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 + # This is not needed by most XBlocks, because they are initialized with a full runtime ModuleSystem that already + # has the MakoService. + # However, there are a few cases where the XBlock only has the descriptor system instead of the full module # runtime. Specifically: # * in the Instructor Dashboard bulk emails tab, when rendering the HtmlBlock for its WYSIWYG editor. - # * during testing, when fetching factory-created blocks. + # * during testing, when using the ModuleSystemTestCase to fetch factory-created blocks. from common.djangoapps.edxmako.services import MakoService self._services['mako'] = MakoService() diff --git a/xmodule/modulestore/__init__.py b/xmodule/modulestore/__init__.py index 5077480c58f9..d02d62e1420c 100644 --- a/xmodule/modulestore/__init__.py +++ b/xmodule/modulestore/__init__.py @@ -1386,3 +1386,10 @@ def _emit_item_deleted_signal(self, usage_key, user_id): """ if self.signal_handler: self.signal_handler.send("item_deleted", usage_key=usage_key, user_id=user_id) + + +def only_xmodules(identifier, entry_points): + """Only use entry_points that are supplied by the xmodule package""" + from_xmodule = [entry_point for entry_point in entry_points if entry_point.dist.key == 'xmodule'] + + return default_select(identifier, from_xmodule) diff --git a/xmodule/seq_block.py b/xmodule/seq_block.py index 0d8be411fd58..fc7202bfe014 100644 --- a/xmodule/seq_block.py +++ b/xmodule/seq_block.py @@ -300,11 +300,11 @@ def __init__(self, *args, **kwargs): self.gated_sequence_paywall = None - def bind_for_student(self, user_id, wrappers=None): + def bind_for_student(self, xmodule_runtime, 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 in the runtime inside prepare_runtime_for_user() - # and needs to be read here after the value has been set. - super().bind_for_student(user_id, wrappers) + # 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) # If position is specified in system, then use that instead. position = getattr(self.runtime, 'position', None) if position is not None: diff --git a/xmodule/services.py b/xmodule/services.py index c4d00a73906e..5418ea4f196f 100644 --- a/xmodule/services.py +++ b/xmodule/services.py @@ -147,8 +147,9 @@ class RebindUserService(Service): """ An XBlock Service that allows modules to get rebound to real users if it was previously bound to an AnonymousUser. - This used to be a local function inside the `lms.djangoapps.courseware.block_render.prepare_runtime_for_user` - method. This has been refactored out into a service and lives in this module temporarily. + This used to be a local function inside the `lms.djangoapps.courseware.block_render.get_module_system_for_user` + method, and was passed as a constructor argument to x_module.ModuleSystem. This has been refactored out into a + service to simplify the ModuleSystem and lives in this module temporarily. TODO: Only the old LTI XBlock uses it in 2 places for LTI 2.0 integration. As the LTI XBlock is deprecated in favour of the LTI Consumer XBlock, this should be removed when the LTI XBlock is removed. @@ -157,18 +158,18 @@ class RebindUserService(Service): user (User) - A Django User object course_id (str) - Course ID course (Course) - Course Object - prepare_runtime_for_user (function) - The helper function that will be called to create a module system + get_module_system_for_user (function) - The helper function that will be called to create a module system for a specfic user. This is the parent function from which this service was reactored out. - `lms.djangoapps.courseware.block_render.prepare_runtime_for_user` - kwargs (dict) - all the keyword arguments that need to be passed to the `prepare_runtime_for_user` + `lms.djangoapps.courseware.block_render.get_module_system_for_user` + kwargs (dict) - all the keyword arguments that need to be passed to the `get_module_system_for_user` function when it is called during rebinding """ - def __init__(self, user, course_id, prepare_runtime_for_user, **kwargs): + def __init__(self, user, course_id, get_module_system_for_user, **kwargs): super().__init__(**kwargs) self.user = user self.course_id = course_id self._ref = { - "prepare_runtime_for_user": prepare_runtime_for_user + "get_module_system_for_user": get_module_system_for_user } self._kwargs = kwargs @@ -201,16 +202,17 @@ def rebind_noauth_module_to_user(self, block, real_user): with modulestore().bulk_operations(self.course_id): course = modulestore().get_course(course_key=self.course_id) - inner_student_data = self._ref["prepare_runtime_for_user"]( + (inner_system, 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 - block=block, + descriptor=block, course_id=self.course_id, course=course, **self._kwargs ) block.bind_for_student( + inner_system, real_user.id, [ partial(DateLookupFieldData, course_id=self.course_id, user=self.user), @@ -220,14 +222,17 @@ def rebind_noauth_module_to_user(self, block, real_user): ) 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 class EventPublishingService(Service): """ An XBlock Service that allows XModules to publish events (e.g. grading, completion). - We have implemented it as a seperate service to be able to alter its behavior when using - a different context: LMS, Studio, or Instructor tasks. + We have separated it from the ModuleSystem to be able to alter its behavior when using a different context: + LMS, Studio, or Instructor tasks. """ def __init__(self, user, course_id, track_function, **kwargs): super().__init__(**kwargs) diff --git a/xmodule/studio_editable.py b/xmodule/studio_editable.py index 2795bf31bca8..69d3126338cf 100644 --- a/xmodule/studio_editable.py +++ b/xmodule/studio_editable.py @@ -33,10 +33,7 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): 'content': rendered_child.content }) - # 'lms.' namespace_prefix is required for rendering in studio - mako_service = self.runtime.service(self, 'mako') - mako_service.namespace_prefix = 'lms.' - fragment.add_content(mako_service.render_template("studio_render_children_view.html", { # pylint: disable=no-member + fragment.add_content(self.runtime.service(self, 'mako').render_template("studio_render_children_view.html", { # pylint: disable=no-member 'items': contents, 'xblock_context': context, 'can_add': can_add, diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index e31aee021615..7770988f64c5 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -36,7 +36,7 @@ from xmodule.modulestore.xml import CourseLocationManager from xmodule.tests.helpers import StubReplaceURLService, mock_render_template, StubMakoService, StubUserService from xmodule.util.sandboxing import SandboxService -from xmodule.x_module import DoNothingCache, XModuleMixin +from xmodule.x_module import DoNothingCache, ModuleSystem, XModuleMixin from openedx.core.lib.cache_utils import CacheService @@ -46,35 +46,9 @@ DATA_DIR = MODULE_DIR.parent.parent / "common" / "test" / "data" -def handler_url(block, handler, suffix='', query='', thirdparty=False): # lint-amnesty, pylint: disable=arguments-differ - return '{usage_id}/{handler}{suffix}?{query}'.format( - usage_id=str(block.scope_ids.usage_id), - handler=handler, - suffix=suffix, - query=query, - ) - - -def local_resource_url(block, uri): - return 'resource/{usage_id}/{uri}'.format( - usage_id=str(block.scope_ids.usage_id), - uri=uri, - ) - - -# Disable XBlockAsides in most tests -def get_asides(block): - return [] - - -@property -def resources_fs(): - return Mock(name='TestDescriptorSystem.resources_fs', root_path='.') - - -class TestDescriptorSystem(MakoDescriptorSystem): # pylint: disable=abstract-method +class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ - DescriptorSystem for testing + ModuleSystem for testing """ def handler_url(self, block, handler, suffix='', query='', thirdparty=False): # lint-amnesty, pylint: disable=arguments-differ return '{usage_id}/{handler}{suffix}?{query}'.format( @@ -94,8 +68,9 @@ def local_resource_url(self, block, uri): def get_asides(self, block): return [] - def resources_fs(self): # lint-amnesty, pylint: disable=method-hidden - return Mock(name='TestDescriptorSystem.resources_fs', root_path='.') + @property + def resources_fs(self): + return Mock(name='TestModuleSystem.resources_fs', root_path='.') def __repr__(self): """ @@ -119,19 +94,13 @@ def get_test_system( user_is_staff=False, user_location=None, render_template=None, - add_get_block_overrides=False ): """ - Construct a test DescriptorSystem instance. + Construct a test ModuleSystem instance. By default, the descriptor system's render_template() method simply returns the repr of the context it is passed. You can override this by passing in a different render_template argument. """ - - id_manager = CourseLocationManager(course_id) - - descriptor_system = get_test_descriptor_system(id_reader=id_manager, id_generator=id_manager) - if not user: user = Mock(name='get_test_system.user', is_staff=False) if not user_location: @@ -148,128 +117,63 @@ def get_test_system( replace_url_service = StubReplaceURLService() - def get_block(block): - """Mocks module_system get_block function""" - - prepare_block_runtime(block.runtime, add_overrides=add_get_block_overrides) - block.runtime.get_block_for_descriptor = get_block - block.bind_for_student(user.id) - - return block - - services = { - 'user': user_service, - 'mako': mako_service, - 'xqueue': XQueueService( - url='http://xqueue.url', - django_auth={}, - basic_auth=[], - default_queuename='testqueue', - waittime=10, - construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"), - ), - 'replace_urls': replace_url_service, - 'cache': CacheService(DoNothingCache()), - 'field-data': DictFieldData({}), - 'sandbox': SandboxService(contentstore, course_id), - } - - descriptor_system.get_block_for_descriptor = get_block # lint-amnesty, pylint: disable=attribute-defined-outside-init - descriptor_system._services.update(services) # lint-amnesty, pylint: disable=protected-access - - return descriptor_system + descriptor_system = get_test_descriptor_system() + id_manager = CourseLocationManager(course_id) -def prepare_block_runtime( - runtime, - course_id=CourseKey.from_string('/'.join(['org', 'course', 'run'])), - user=None, - user_is_staff=False, - user_location=None, - render_template=None, - add_overrides=False, - add_get_block=False, -): - """ - Sets properties in the runtime of the specified descriptor that is - required for tests. - """ + def get_block(descriptor): + """Mocks module_system get_block function""" - if not user: - user = Mock(name='get_test_system.user', is_staff=False) - if not user_location: - user_location = Mock(name='get_test_system.user_location') - user_service = StubUserService( - user=user, - anonymous_user_id='student', - user_is_staff=user_is_staff, - user_role='student', - request_country_code=user_location, + # Unlike XBlock Runtimes or DescriptorSystems, + # each XModule is provided with a new ModuleSystem. + # Construct one for the new XModule. + module_system = get_test_system() + + # Descriptors can all share a single DescriptorSystem. + # So, bind to the same one as the current descriptor. + module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access + + descriptor.bind_for_student(module_system, user.id) + + return descriptor + + return TestModuleSystem( + get_block=get_block, + services={ + 'user': user_service, + 'mako': mako_service, + 'xqueue': XQueueService( + url='http://xqueue.url', + django_auth={}, + basic_auth=[], + default_queuename='testqueue', + waittime=10, + construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"), + ), + 'replace_urls': replace_url_service, + 'cache': CacheService(DoNothingCache()), + 'field-data': DictFieldData({}), + 'sandbox': SandboxService(contentstore, course_id), + }, + descriptor_runtime=descriptor_system, + id_reader=id_manager, + id_generator=id_manager, ) - mako_service = StubMakoService(render_template=render_template) - - replace_url_service = StubReplaceURLService() - - def get_block(block): - """Mocks module_system get_block function""" - prepare_block_runtime(block.runtime) - block.bind_for_student(user.id) - - return block - - services = { - 'user': user_service, - 'mako': mako_service, - 'xqueue': XQueueService( - url='http://xqueue.url', - django_auth={}, - basic_auth=[], - default_queuename='testqueue', - waittime=10, - construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"), - ), - 'replace_urls': replace_url_service, - 'cache': CacheService(DoNothingCache()), - 'field-data': DictFieldData({}), - 'sandbox': SandboxService(contentstore, course_id), - } - - if add_overrides: - runtime.handler_url_override = handler_url - runtime.local_resource_url = local_resource_url - runtime.get_asides = get_asides - runtime.resources_fs = resources_fs - - if add_get_block: - runtime.get_block_for_descriptor = get_block - - runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access - - # runtime.load_item=Mock(name='get_test_descriptor_system.load_item') - # runtime.resources_fs=Mock(name='get_test_descriptor_system.resources_fs') - # runtime.error_tracker=Mock(name='get_test_descriptor_system.error_tracker') - # runtime.render_template=render_template or mock_render_template, - # runtime.mixins=(InheritanceMixin, XModuleMixin) - - return runtime - - -def get_test_descriptor_system(render_template=None, **kwargs): +def get_test_descriptor_system(render_template=None): """ Construct a test DescriptorSystem instance. """ field_data = DictFieldData({}) - descriptor_system = TestDescriptorSystem( + descriptor_system = MakoDescriptorSystem( 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'), render_template=render_template or mock_render_template, mixins=(InheritanceMixin, XModuleMixin), services={'field-data': field_data}, - **kwargs ) descriptor_system.get_asides = lambda block: [] return descriptor_system diff --git a/xmodule/tests/helpers.py b/xmodule/tests/helpers.py index d69be6bc00b2..2f7a399ab07c 100644 --- a/xmodule/tests/helpers.py +++ b/xmodule/tests/helpers.py @@ -6,10 +6,8 @@ import filecmp import pprint -import pytest from path import Path as path from xblock.reference.user_service import UserService, XBlockUser -from xmodule.x_module import DescriptorSystem def directories_equal(directory1, directory2): @@ -74,7 +72,6 @@ def __init__(self, self.user_role = user_role self.anonymous_user_id = anonymous_user_id self.request_country_code = request_country_code - self._django_user = user super().__init__(**kwargs) def get_current_user(self): @@ -112,17 +109,3 @@ def replace_urls(self, text, static_replace_only=False): Invokes the configured render_template method. """ return text - - -@pytest.fixture -def override_descriptor_system(monkeypatch): - """ - Fixture to override get_block method of DescriptorSystem - """ - - 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) - return block - - monkeypatch.setattr(DescriptorSystem, "get_block", get_block) diff --git a/xmodule/tests/test_conditional.py b/xmodule/tests/test_conditional.py index 0413842557e4..b36ae92b63a7 100644 --- a/xmodule/tests/test_conditional.py +++ b/xmodule/tests/test_conditional.py @@ -16,7 +16,7 @@ from xmodule.conditional_block import ConditionalBlock from xmodule.error_block import ErrorBlock from xmodule.modulestore.xml import CourseLocationManager, ImportSystem, XMLModuleStore -from xmodule.tests import DATA_DIR, get_test_system, prepare_block_runtime +from xmodule.tests import DATA_DIR, get_test_descriptor_system, get_test_system from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import factories as xml from xmodule.validation import StudioValidationMessage @@ -41,8 +41,7 @@ def __init__(self, load_error_blocks): load_error_blocks=load_error_blocks, ) - @property - def render_template(self): # lint-amnesty, pylint: disable=method-hidden + def render_template(self, template, context): # lint-amnesty, pylint: disable=method-hidden raise Exception("Shouldn't be called") @@ -66,6 +65,7 @@ def create(system, source_is_error_block=False, source_visible_to_staff_only=Fal if the source_is_error_block flag is set, create a real ErrorBlock for the source. """ + descriptor_system = get_test_descriptor_system() # construct source descriptor and module: source_location = BlockUsageLocator(CourseLocator("edX", "conditional_test", "test_run", deprecated=True), @@ -83,16 +83,17 @@ def create(system, source_is_error_block=False, source_visible_to_staff_only=Fal source_descriptor.location = source_location source_descriptor.visible_to_staff_only = source_visible_to_staff_only - source_descriptor.runtime = system - source_descriptor.render = lambda view, context=None: system.render(source_descriptor, view, context) + source_descriptor.runtime = descriptor_system + source_descriptor.render = lambda view, context=None: descriptor_system.render(source_descriptor, view, context) # construct other descriptors: child_descriptor = Mock(name='child_descriptor') child_descriptor.visible_to_staff_only = False child_descriptor._xmodule.student_view.return_value = Fragment(content='

This is a secret

') # lint-amnesty, pylint: disable=protected-access child_descriptor.student_view = child_descriptor._xmodule.student_view # lint-amnesty, pylint: disable=protected-access - child_descriptor.runtime = system - child_descriptor.render = lambda view, context=None: system.render(child_descriptor, view, context) + child_descriptor.runtime = descriptor_system + child_descriptor.xmodule_runtime = get_test_system() + child_descriptor.render = lambda view, context=None: descriptor_system.render(child_descriptor, view, context) child_descriptor.location = source_location.replace(category='html', name='child') def visible_to_nonstaff_users(desc): @@ -108,7 +109,9 @@ def load_item(usage_id, for_parent=None): # pylint: disable=unused-argument source_location: source_descriptor }.get(usage_id) - system.load_item = load_item + descriptor_system.load_item = load_item + + system.descriptor_runtime = descriptor_system # construct conditional block: cond_location = BlockUsageLocator(CourseLocator("edX", "conditional_test", "test_run", deprecated=True), @@ -122,10 +125,11 @@ def load_item(usage_id, for_parent=None): # pylint: disable=unused-argument }) cond_descriptor = ConditionalBlock( - system, + descriptor_system, field_data, ScopeIds(None, None, cond_location, cond_location) ) + cond_descriptor.xmodule_runtime = system system.get_block_for_descriptor = lambda desc: desc if visible_to_nonstaff_users(desc) else None cond_descriptor.get_required_blocks = [ system.get_block_for_descriptor(source_descriptor), @@ -161,7 +165,7 @@ def test_get_html(self): # because get_test_system returns the repr of the context dict passed to render_template, # we reverse it here html = blocks['cond_block'].render(STUDENT_VIEW).content - mako_service = blocks['cond_block'].runtime.service(blocks['cond_block'], 'mako') + mako_service = blocks['cond_block'].xmodule_runtime.service(blocks['cond_block'], 'mako') expected = mako_service.render_template('conditional_ajax.html', { 'ajax_url': blocks['cond_block'].ajax_url, 'element_id': 'i4x-edX-conditional_test-conditional-SampleConditional', @@ -216,12 +220,7 @@ class ConditionalBlockXmlTest(unittest.TestCase): def setUp(self): super().setUp() - - def add_block_as_child_node(block, node): - child = etree.SubElement(node, "unknown") - block.add_xml_to_node(child) - self.test_system = get_test_system(add_get_block_overrides=True) - self.test_system.add_block_as_child_node = add_block_as_child_node + self.test_system = get_test_system() self.modulestore = XMLModuleStore(DATA_DIR, source_dirs=['conditional_and_poll']) courses = self.modulestore.get_courses() assert len(courses) == 1 @@ -242,7 +241,7 @@ def test_conditional_block(self, _): block = self.get_block_for_location(location) html = block.render(STUDENT_VIEW).content - mako_service = block.runtime.service(block, 'mako') + mako_service = block.xmodule_runtime.service(block, 'mako') html_expect = mako_service.render_template( 'conditional_ajax.html', { @@ -356,9 +355,12 @@ def setUp(self): self.sequence = self.course.get_children()[0] self.conditional = self.sequence.get_children()[0] + self.module_system = get_test_system() + self.module_system.descriptor_runtime = self.course._runtime # pylint: disable=protected-access + user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) - self.conditional.runtime = prepare_block_runtime(self.course.runtime) self.conditional.bind_for_student( + self.module_system, user.id ) @@ -378,11 +380,11 @@ def create_studio_context(root_xblock, is_unit_page): } context = create_studio_context(self.conditional, False) - html = self.course.runtime.render(self.conditional, AUTHOR_VIEW, context).content + html = self.module_system.render(self.conditional, AUTHOR_VIEW, context).content assert 'This is a secret HTML' in html context = create_studio_context(self.sequence, True) - html = self.course.runtime.render(self.conditional, AUTHOR_VIEW, context).content + html = self.module_system.render(self.conditional, AUTHOR_VIEW, context).content assert 'This is a secret HTML' not in html def test_non_editable_settings(self): diff --git a/xmodule/tests/test_error_block.py b/xmodule/tests/test_error_block.py index f80fb905cb8b..5df3bcb61f23 100644 --- a/xmodule/tests/test_error_block.py +++ b/xmodule/tests/test_error_block.py @@ -38,7 +38,7 @@ def test_error_block_xml_rendering(self): self.error_msg ) assert isinstance(descriptor, ErrorBlock) - descriptor.runtime = self.system + descriptor.xmodule_runtime = self.system context_repr = self.system.render(descriptor, STUDENT_VIEW).content assert self.error_msg in context_repr assert repr(self.valid_xml) in context_repr diff --git a/xmodule/tests/test_import.py b/xmodule/tests/test_import.py index 0c66f68ad9d9..570b484ef04a 100644 --- a/xmodule/tests/test_import.py +++ b/xmodule/tests/test_import.py @@ -17,6 +17,7 @@ from xblock.runtime import DictKeyValueStore, KvsFieldData from xmodule.fields import Date +from xmodule.modulestore import only_xmodules from xmodule.modulestore.inheritance import InheritanceMixin, compute_inherited_metadata from xmodule.modulestore.xml import ImportSystem, LibraryXMLModuleStore, XMLModuleStore from xmodule.tests import DATA_DIR @@ -51,6 +52,9 @@ def __init__(self, load_error_blocks, library=False): services={'field-data': KvsFieldData(DictKeyValueStore())}, ) + def render_template(self, _template, _context): # lint-amnesty, pylint: disable=method-hidden + raise Exception("Shouldn't be called") + class BaseCourseTestCase(TestCase): '''Make sure block imports work properly, including for malformed inputs''' @@ -68,6 +72,7 @@ def get_course(self, name): DATA_DIR, source_dirs=[name], xblock_mixins=(InheritanceMixin,), + xblock_select=only_xmodules, ) courses = modulestore.get_courses() assert len(courses) == 1 diff --git a/xmodule/tests/test_library_content.py b/xmodule/tests/test_library_content.py index 957ed72927b0..09f176ce63f8 100644 --- a/xmodule/tests/test_library_content.py +++ b/xmodule/tests/test_library_content.py @@ -19,7 +19,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory from xmodule.modulestore.tests.utils import MixedSplitTestCase -from xmodule.tests import prepare_block_runtime +from xmodule.tests import get_test_system from xmodule.validation import StudioValidationMessage from xmodule.x_module import AUTHOR_VIEW from xmodule.capa_block import ProblemBlock @@ -58,17 +58,20 @@ def _bind_course_block(self, block): """ Bind a block (part of self.course) so we can access student-specific data. """ - prepare_block_runtime(block.runtime, course_id=block.location.course_key) - block.runtime._runtime_services.update({'library_tools': self.tools}) # lint-amnesty, pylint: disable=protected-access + module_system = get_test_system(course_id=block.location.course_key) + module_system.descriptor_runtime = block.runtime._descriptor_system # pylint: disable=protected-access + module_system._services['library_tools'] = self.tools # pylint: disable=protected-access def get_block(descriptor): """Mocks module_system get_block function""" - prepare_block_runtime(descriptor.runtime, course_id=block.location.course_key) - descriptor.runtime.get_block_for_descriptor = get_block - descriptor.bind_for_student(self.user_id) + sub_module_system = get_test_system(course_id=block.location.course_key) + sub_module_system.get_block_for_descriptor = get_block + sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access + descriptor.bind_for_student(sub_module_system, self.user_id) return descriptor - block.runtime.get_block_for_descriptor = get_block + module_system.get_block_for_descriptor = get_block + block.xmodule_runtime = module_system class TestLibraryContentExportImport(LibraryContentTest): @@ -96,7 +99,7 @@ def setUp(self): # Set the virtual FS to export the olx to. self.export_fs = MemoryFS() - self.lc_block.runtime.export_fs = self.export_fs # pylint: disable=protected-access + self.lc_block.runtime._descriptor_system.export_fs = self.export_fs # pylint: disable=protected-access # Prepare runtime for the import. self.runtime = TestImportSystem(load_error_blocks=True, course_id=self.lc_block.location.course_key) @@ -514,7 +517,7 @@ def setUp(self): self.lc_block.refresh_children() self.lc_block = self.store.get_item(self.lc_block.location) self._bind_course_block(self.lc_block) - self.lc_block.runtime.publish = self.publisher + self.lc_block.xmodule_runtime.publish = self.publisher def _assert_event_was_published(self, event_type): """ @@ -568,7 +571,7 @@ def test_assigned_event_published(self): with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): self.lc_block = self.store.get_item(self.lc_block.location) self._bind_course_block(self.lc_block) - self.lc_block.runtime.publish = self.publisher + self.lc_block.xmodule_runtime.publish = self.publisher self.test_assigned_event() def test_assigned_descendants(self): @@ -587,7 +590,7 @@ def test_assigned_descendants(self): # Reload lc_block and set it up for a student: self.lc_block = self.store.get_item(self.lc_block.location) self._bind_course_block(self.lc_block) - self.lc_block.runtime.publish = self.publisher + self.lc_block.xmodule_runtime.publish = self.publisher # Get the keys of each of our blocks, as they appear in the course: course_usage_main_vertical = self.lc_block.children[0] diff --git a/xmodule/tests/test_library_sourced_block.py b/xmodule/tests/test_library_sourced_block.py index 5ec6291542cd..12acc31d943d 100644 --- a/xmodule/tests/test_library_sourced_block.py +++ b/xmodule/tests/test_library_sourced_block.py @@ -6,7 +6,7 @@ from common.djangoapps.student.roles import CourseInstructorRole from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory -from xmodule.tests import prepare_block_runtime +from xmodule.tests import get_test_system from xmodule.x_module import STUDENT_VIEW # lint-amnesty, pylint: disable=unused-import @@ -66,6 +66,7 @@ def get_block_view(self, block, view, context=None): """ context = context or {} block = self.store.get_item(block.location) - prepare_block_runtime(block.runtime) - block.bind_for_student(self.user.id) - return block.runtime.render(block, view, context).content + module_system = get_test_system(block) + module_system.descriptor_runtime = block._runtime # pylint: disable=protected-access + block.bind_for_student(module_system, self.user.id) + return module_system.render(block, view, context).content diff --git a/xmodule/tests/test_lti20_unit.py b/xmodule/tests/test_lti20_unit.py index 2c876f25f003..8923f4b474c0 100644 --- a/xmodule/tests/test_lti20_unit.py +++ b/xmodule/tests/test_lti20_unit.py @@ -370,7 +370,7 @@ def test_lti20_request_handler_bad_user(self): Test that we get a 404 when the supplied user does not exist """ self.setup_system_xblock_mocks_for_lti20_request_test() - self.runtime._runtime_services['user'] = StubUserService(user=None) # pylint: disable=protected-access + self.runtime._services['user'] = StubUserService(user=None) # pylint: disable=protected-access mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) response = self.xblock.lti_2_0_result_rest_handler(mock_request, "user/abcd") assert response.status_code == 404 diff --git a/xmodule/tests/test_lti_unit.py b/xmodule/tests/test_lti_unit.py index 76e4fc66e111..47d480d5bfc9 100644 --- a/xmodule/tests/test_lti_unit.py +++ b/xmodule/tests/test_lti_unit.py @@ -65,7 +65,7 @@ def setUp(self): self.course_id = CourseKey.from_string('org/course/run') self.system = get_test_system(self.course_id) self.system.publish = Mock() - self.system._runtime_services['rebind_user'] = Mock() # pylint: disable=protected-access + self.system._services['rebind_user'] = Mock() # pylint: disable=protected-access self.xblock = LTIBlock( self.system, @@ -178,7 +178,7 @@ def test_real_user_is_none(self): """ If we have no real user, we should send back failure response. """ - self.system._runtime_services['user'] = StubUserService(user=None) # pylint: disable=protected-access + self.system._services['user'] = StubUserService(user=None) # pylint: disable=protected-access self.xblock.verify_oauth_body_sign = Mock() self.xblock.has_score = True request = Request(self.environ) diff --git a/xmodule/tests/test_randomize_block.py b/xmodule/tests/test_randomize_block.py index d39eed84c432..8848c4dded9d 100644 --- a/xmodule/tests/test_randomize_block.py +++ b/xmodule/tests/test_randomize_block.py @@ -9,7 +9,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.utils import MixedSplitTestCase from xmodule.randomize_block import RandomizeBlock -from xmodule.tests import prepare_block_runtime +from xmodule.tests import get_test_system from .test_course_block import DummySystem as TestImportSystem @@ -42,7 +42,9 @@ def _bind_module_system(self, block, user_id): Bind module system to block so we can access student-specific data. """ user = Mock(name='get_test_system.user', id=user_id, is_staff=False) - prepare_block_runtime(block.runtime, course_id=block.location.course_key, user=user) + module_system = get_test_system(course_id=block.location.course_key, user=user) + module_system.descriptor_runtime = block.runtime._descriptor_system # pylint: disable=protected-access + block.xmodule_runtime = module_system def test_xml_export_import_cycle(self): """ @@ -62,7 +64,7 @@ def test_xml_export_import_cycle(self): export_fs = MemoryFS() # Set the virtual FS to export the olx to. - randomize_block.runtime.export_fs = export_fs # pylint: disable=protected-access + randomize_block.runtime._descriptor_system.export_fs = export_fs # pylint: disable=protected-access # Export the olx. node = etree.Element("unknown_root") diff --git a/xmodule/tests/test_sequence.py b/xmodule/tests/test_sequence.py index 9a81ef32807a..4447bb2aeded 100644 --- a/xmodule/tests/test_sequence.py +++ b/xmodule/tests/test_sequence.py @@ -19,7 +19,7 @@ from edx_toggles.toggles.testutils import override_waffle_flag from openedx.features.content_type_gating.models import ContentTypeGatingConfig from xmodule.seq_block import TIMED_EXAM_GATING_WAFFLE_FLAG, SequenceBlock -from xmodule.tests import get_test_system, prepare_block_runtime +from xmodule.tests import get_test_system from xmodule.tests.helpers import StubUserService from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import factories as xml @@ -94,8 +94,11 @@ def _set_up_block(self, parent, index_in_parent): self._set_up_module_system(block) - block.runtime._runtime_services['bookmarks'] = Mock() # pylint: disable=protected-access - block.runtime._runtime_services['user'] = StubUserService(user=Mock()) # pylint: disable=protected-access + block.xmodule_runtime._services['bookmarks'] = Mock() # pylint: disable=protected-access + block.xmodule_runtime._services['completion'] = Mock( # pylint: disable=protected-access + return_value=Mock(vertical_is_complete=Mock(return_value=True)) + ) + block.xmodule_runtime._services['user'] = StubUserService(user=Mock()) # pylint: disable=protected-access block.parent = parent.location return block @@ -103,12 +106,14 @@ def _set_up_module_system(self, block): """ Sets up the test module system for the given block. """ - prepare_block_runtime(block.runtime) + module_system = get_test_system() + module_system.descriptor_runtime = block._runtime # pylint: disable=protected-access + block.xmodule_runtime = module_system # The render operation will ask modulestore for the current course to get some data. As these tests were # originally not written to be compatible with a real modulestore, we've mocked out the relevant return values. - block.runtime.modulestore = Mock() - block.runtime.modulestore.get_course.return_value = self.course + module_system.modulestore = Mock() + module_system.modulestore.get_course.return_value = self.course def _get_rendered_view(self, sequence, @@ -125,7 +130,7 @@ def _get_rendered_view(self, context.update(extra_context) self.course.self_paced = self_paced - return sequence.runtime.render(sequence, view, context).content + return sequence.xmodule_runtime.render(sequence, view, context).content def _assert_view_at_position(self, rendered_html, expected_position): """ @@ -134,10 +139,10 @@ def _assert_view_at_position(self, rendered_html, expected_position): assert f"'position': {expected_position}" in rendered_html def test_student_view_init(self): - runtime = get_test_system() - runtime.position = 2 - seq_block = SequenceBlock(runtime=runtime, scope_ids=Mock()) - seq_block.bind_for_student(34) + module_system = get_test_system() + module_system.position = 2 + seq_block = SequenceBlock(runtime=module_system, scope_ids=Mock()) + seq_block.bind_for_student(module_system, 34) assert seq_block.position == 2 # matches position set in the runtime @@ -330,7 +335,7 @@ def test_gated_content(self): False, {'url': 'PrereqUrl', 'display_name': 'PrereqSectionName', 'id': 'mockId'} ] - self.sequence_1_2.runtime._services['gating'] = gating_mock_1_2 # pylint: disable=protected-access + self.sequence_1_2.xmodule_runtime._services['gating'] = gating_mock_1_2 # pylint: disable=protected-access self.sequence_1_2.display_name = 'sequence_1_2' html = self._get_rendered_view( @@ -368,9 +373,6 @@ def test_gated_content(self): def test_xblock_handler_get_completion_success(self): """Test that the completion data is returned successfully on targeted vertical through ajax call""" - self.sequence_3_1.runtime._runtime_services['completion'] = Mock( # pylint: disable=protected-access - return_value=Mock(vertical_is_complete=Mock(return_value=True)) - ) for child in self.sequence_3_1.get_children(): usage_key = str(child.location) request = RequestFactory().post( @@ -380,7 +382,6 @@ def test_xblock_handler_get_completion_success(self): ) completion_return = self.sequence_3_1.handle('get_completion', request) assert completion_return.json == {'complete': True} - self.sequence_3_1.runtime._runtime_services['completion'] = None # pylint: disable=protected-access def test_xblock_handler_get_completion_bad_key(self): """Test that the completion data is returned as False when usage key is None through ajax call""" @@ -394,14 +395,10 @@ def test_xblock_handler_get_completion_bad_key(self): def test_handle_ajax_get_completion_success(self): """Test that the old-style ajax handler for completion still works""" - self.sequence_3_1.runtime._runtime_services['completion'] = Mock( # pylint: disable=protected-access - return_value=Mock(vertical_is_complete=Mock(return_value=True)) - ) for child in self.sequence_3_1.get_children(): usage_key = str(child.location) completion_return = self.sequence_3_1.handle_ajax('get_completion', {'usage_key': usage_key}) assert json.loads(completion_return) == {'complete': True} - self.sequence_3_1.runtime._runtime_services['completion'] = None # pylint: disable=protected-access def test_xblock_handler_goto_position_success(self): """Test that we can set position through ajax call""" @@ -437,7 +434,7 @@ def test_get_metadata(self): """Test that the sequence metadata is returned correctly""" # rather than dealing with json serialization of the Mock object, # let's just disable the bookmarks service - self.sequence_3_1.runtime._services['bookmarks'] = None # lint-amnesty, pylint: disable=protected-access + self.sequence_3_1.xmodule_runtime._services['bookmarks'] = None # lint-amnesty, pylint: disable=protected-access metadata = self.sequence_3_1.get_metadata() assert len(metadata['items']) == 3 assert metadata['tag'] == 'sequential' @@ -448,7 +445,7 @@ def test_get_metadata(self): )) def test_get_metadata_content_type_gated_content(self): """The contains_content_type_gated_content field tells whether the item contains content type gated content""" - self.sequence_5_1.runtime._services['bookmarks'] = None # pylint: disable=protected-access + self.sequence_5_1.xmodule_runtime._services['bookmarks'] = None # pylint: disable=protected-access ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) metadata = self.sequence_5_1.get_metadata() assert metadata['items'][0]['contains_content_type_gated_content'] is False diff --git a/xmodule/tests/test_split_test_block.py b/xmodule/tests/test_split_test_block.py index 5de2e15ee797..61ddd6f8c273 100644 --- a/xmodule/tests/test_split_test_block.py +++ b/xmodule/tests/test_split_test_block.py @@ -18,7 +18,7 @@ get_split_user_partitions, user_partition_values, ) -from xmodule.tests import prepare_block_runtime +from xmodule.tests import get_test_system from xmodule.tests.test_course_block import DummySystem as TestImportSystem from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import factories as xml @@ -85,9 +85,9 @@ def setUp(self): self.course = self.process_xml(course) self.course_sequence = self.course.get_children()[0] - user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) - prepare_block_runtime(self.course.runtime, user=user, add_get_block=True) + self.module_system = get_test_system() + self.module_system.descriptor_runtime = self.course._runtime # pylint: disable=protected-access self.course.runtime.export_fs = MemoryFS() # Create mock partition service, as these tests are running with XML in-memory system. @@ -106,15 +106,19 @@ def setUp(self): self.course, course_id=self.course.id, ) - self.course.runtime._runtime_services['partitions'] = partitions_service # pylint: disable=protected-access + self.module_system._services['partitions'] = partitions_service # pylint: disable=protected-access # Mock user_service user user_service = Mock() + user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) user_service._django_user = user # lint-amnesty, pylint: disable=protected-access + self.module_system._services['user'] = user_service # pylint: disable=protected-access self.split_test_block = self.course_sequence.get_children()[0] - self.split_test_block.runtime = self.course.runtime - self.split_test_block.bind_for_student(user.id) + self.split_test_block.bind_for_student( + self.module_system, + user.id + ) # Create mock modulestore for getting the course. Needed for rendering the HTML # view, since mock services exist and the rendering code will not short-circuit. @@ -148,7 +152,7 @@ def test_child(self, user_tag, child_url_name): @ddt.unpack def test_get_html(self, user_tag, child_content): self.user_partition.scheme.current_group = self.user_partition.groups[user_tag] - assert child_content in self.course.runtime.render(self.split_test_block, STUDENT_VIEW).content + assert child_content in self.module_system.render(self.split_test_block, STUDENT_VIEW).content @ddt.data(0, 1) def test_child_missing_tag_value(self, _user_tag): @@ -171,7 +175,7 @@ def test_export_import_round_trip(self, def_to_xml): # Mock out the process_xml # Expect it to return a child descriptor for the SplitTestDescriptor when called. - self.course.runtime.process_xml = Mock() + self.module_system.process_xml = Mock() # Write out the xml. xml_obj = self.split_test_block.definition_to_xml(MemoryFS()) @@ -180,7 +184,7 @@ def test_export_import_round_trip(self, def_to_xml): assert xml_obj.get('group_id_to_child') is not None # Read the xml back in. - fields, children = SplitTestBlock.definition_from_xml(xml_obj, self.course.runtime) + fields, children = SplitTestBlock.definition_from_xml(xml_obj, self.module_system) assert fields.get('user_partition_id') == '0' assert fields.get('group_id_to_child') is not None assert len(children) == 2 @@ -208,13 +212,13 @@ def create_studio_context(root_xblock): # The split_test block should render both its groups when it is the root context = create_studio_context(self.split_test_block) - html = self.course.runtime.render(self.split_test_block, AUTHOR_VIEW, context).content + html = self.module_system.render(self.split_test_block, AUTHOR_VIEW, context).content assert 'HTML FOR GROUP 0' in html assert 'HTML FOR GROUP 1' in html # When rendering as a child, it shouldn't render either of its groups context = create_studio_context(self.course_sequence) - html = self.course.runtime.render(self.split_test_block, AUTHOR_VIEW, context).content + html = self.module_system.render(self.split_test_block, AUTHOR_VIEW, context).content assert 'HTML FOR GROUP 0' not in html assert 'HTML FOR GROUP 1' not in html @@ -224,7 +228,7 @@ def create_studio_context(root_xblock): UserPartition(0, 'first_partition', 'First Partition', [Group("0", 'alpha'), Group("1", 'beta'), Group("2", 'gamma')]) ] - html = self.course.runtime.render(self.split_test_block, AUTHOR_VIEW, context).content + html = self.module_system.render(self.split_test_block, AUTHOR_VIEW, context).content assert 'HTML FOR GROUP 0' in html assert 'HTML FOR GROUP 1' in html @@ -565,7 +569,7 @@ def test_xml_export_import_cycle(self): ) export_fs = MemoryFS() # Set the virtual FS to export the olx to. - split_test_block.runtime.export_fs = export_fs # pylint: disable=protected-access + split_test_block.runtime._descriptor_system.export_fs = export_fs # pylint: disable=protected-access # Export the olx. node = lxml.etree.Element("unknown_root") diff --git a/xmodule/tests/test_studio_editable.py b/xmodule/tests/test_studio_editable.py index a8f9f7d4e181..f697cf82493b 100644 --- a/xmodule/tests/test_studio_editable.py +++ b/xmodule/tests/test_studio_editable.py @@ -24,6 +24,6 @@ def test_render_reorderable_children(self): } # Both children of the vertical should be rendered as reorderable - self.course.runtime.render(self.vertical, AUTHOR_VIEW, context).content # pylint: disable=expression-not-assigned + self.module_system.render(self.vertical, AUTHOR_VIEW, context).content # pylint: disable=expression-not-assigned assert self.vertical.get_children()[0].location in reorderable_items assert self.vertical.get_children()[1].location in reorderable_items diff --git a/xmodule/tests/test_vertical.py b/xmodule/tests/test_vertical.py index f4012125e259..1717354b4798 100644 --- a/xmodule/tests/test_vertical.py +++ b/xmodule/tests/test_vertical.py @@ -18,7 +18,7 @@ from openedx_filters import PipelineStep from openedx_filters.learning.filters import VerticalBlockChildRenderStarted, VerticalBlockRenderCompleted -from . import prepare_block_runtime +from . import get_test_system from .helpers import StubUserService from .xml import XModuleXmlImportTest from .xml import factories as xml @@ -164,12 +164,13 @@ def setUp(self): self.course = self.process_xml(course) course_seq = self.course.get_children()[0] - prepare_block_runtime(self.course.runtime) + self.module_system = get_test_system() + self.module_system.descriptor_runtime = self.course._runtime self.course.runtime.export_fs = MemoryFS() self.vertical = course_seq.get_children()[0] - self.vertical.runtime = self.course.runtime + self.vertical.xmodule_runtime = self.module_system self.html_block = self.vertical.get_children()[0] self.problem_block = self.vertical.get_children()[1] @@ -212,19 +213,17 @@ def test_render_student_preview_view(self, context, view, completion_value, days """ Test the rendering of the student and public view. """ - self.course.runtime._runtime_services['bookmarks'] = Mock() + self.module_system._services['bookmarks'] = Mock() now = datetime.now(pytz.UTC) self.vertical.due = now + timedelta(days=days) if view == STUDENT_VIEW: - self.course.runtime._runtime_services['user'] = StubUserService(user=Mock(username=self.username)) - self.course.runtime._runtime_services['completion'] = StubCompletionService( - enabled=True, - completion_value=completion_value - ) + self.module_system._services['user'] = StubUserService(user=Mock(username=self.username)) + self.module_system._services['completion'] = StubCompletionService(enabled=True, + completion_value=completion_value) elif view == PUBLIC_VIEW: - self.course.runtime._runtime_services['user'] = StubUserService(user=AnonymousUser()) + self.module_system._services['user'] = StubUserService(user=AnonymousUser()) - html = self.course.runtime.render( + html = self.module_system.render( self.vertical, view, self.default_context if context is None else context ).content assert self.test_html in html @@ -249,15 +248,15 @@ def test_render_problem_without_score(self, has_score): """ Test the rendering of the student and public view. """ - self.course.runtime._services['bookmarks'] = Mock() - self.course.runtime._services['user'] = StubUserService(user=Mock()) - self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.module_system._services['bookmarks'] = Mock() + self.module_system._services['user'] = StubUserService(user=Mock()) + self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) now = datetime.now(pytz.UTC) self.vertical.due = now + timedelta(days=-1) self.problem_block.has_score = has_score - html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content if has_score: assert "'has_assignments': True" in html assert "'completed': False" in html @@ -271,14 +270,14 @@ def test_render_problem_without_score(self, has_score): @ddt.unpack def test_render_access_denied_blocks(self, node_has_access_error, child_has_access_error): """ Tests access denied blocks are not rendered when hide_access_error_blocks is True """ - self.course.runtime._services['bookmarks'] = Mock() - self.course.runtime._services['user'] = StubUserService(user=Mock()) + self.module_system._services['bookmarks'] = Mock() + self.module_system._services['user'] = StubUserService(user=Mock()) self.vertical.due = datetime.now(pytz.UTC) + timedelta(days=-1) self.problem_block.has_access_error = node_has_access_error self.nested_problem_block.has_access_error = child_has_access_error context = {'username': self.username, 'hide_access_error_blocks': True} - html = self.course.runtime.render(self.vertical, STUDENT_VIEW, context).content + html = self.module_system.render(self.vertical, STUDENT_VIEW, context).content if node_has_access_error and child_has_access_error: assert self.test_problem not in html @@ -317,11 +316,11 @@ def test_mark_completed_on_view_after_delay_in_context( Test that mark-completed-on-view-after-delay is only set for relevant child Xblocks. """ with patch.object(self.html_block, 'render') as mock_student_view: - self.course.runtime._services['completion'] = StubCompletionService( + self.module_system._services['completion'] = StubCompletionService( enabled=completion_enabled, completion_value=completion_value, ) - self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context) + self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context) if mark_completed_enabled: assert mock_student_view.call_args[0][1]['wrap_xblock_data']['mark-completed-on-view-after-delay'] ==\ 9876 @@ -336,7 +335,7 @@ def test_render_studio_view(self): context = { 'is_unit_page': True } - html = self.course.runtime.render(self.vertical, AUTHOR_VIEW, context).content + html = self.module_system.render(self.vertical, AUTHOR_VIEW, context).content assert self.test_html not in html assert self.test_problem not in html @@ -346,7 +345,7 @@ def test_render_studio_view(self): 'is_unit_page': False, 'reorderable_items': reorderable_items, } - html = self.course.runtime.render(self.vertical, AUTHOR_VIEW, context).content + html = self.module_system.render(self.vertical, AUTHOR_VIEW, context).content assert self.test_html in html assert self.test_problem in html @@ -364,11 +363,11 @@ def test_vertical_block_child_render_started_filter_execution(self): """ Test the VerticalBlockChildRenderStarted filter's effects on student view. """ - self.course.runtime._services['bookmarks'] = Mock() - self.course.runtime._services['user'] = StubUserService(user=Mock()) - self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.module_system._services['bookmarks'] = Mock() + self.module_system._services['user'] = StubUserService(user=Mock()) + self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) - html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content assert TestVerticalBlockChildRenderStep.filter_content in html @@ -386,11 +385,11 @@ def test_vertical_block_child_render_is_skipped_on_filter_exception(self): """ Test VerticalBlockChildRenderStarted filter can be used to skip child blocks. """ - self.course.runtime._services['bookmarks'] = Mock() - self.course.runtime._services['user'] = StubUserService(user=Mock()) - self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.module_system._services['bookmarks'] = Mock() + self.module_system._services['user'] = StubUserService(user=Mock()) + self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) - html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content assert self.test_html not in html assert self.test_html_nested not in html @@ -409,11 +408,11 @@ def test_vertical_block_render_completed_filter_execution(self): """ Test the VerticalBlockRenderCompleted filter's execution. """ - self.course.runtime._services['bookmarks'] = Mock() - self.course.runtime._services['user'] = StubUserService(user=Mock()) - self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.module_system._services['bookmarks'] = Mock() + self.module_system._services['user'] = StubUserService(user=Mock()) + self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) - html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content assert TestVerticalBlockRenderCompletedStep.filter_content in html @@ -431,10 +430,10 @@ def test_vertical_block_render_output_is_changed_on_filter_exception(self): """ Test VerticalBlockRenderCompleted filter can be used to prevent vertical block from rendering. """ - self.course.runtime._services['bookmarks'] = Mock() - self.course.runtime._services['user'] = StubUserService(user=Mock()) - self.course.runtime._services['completion'] = StubCompletionService(enabled=True, completion_value=0) + self.module_system._services['bookmarks'] = Mock() + self.module_system._services['user'] = StubUserService(user=Mock()) + self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0) - html = self.course.runtime.render(self.vertical, STUDENT_VIEW, self.default_context).content + html = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context).content assert TestPreventVerticalBlockRenderStep.filter_content == html diff --git a/xmodule/tests/xml/factories.py b/xmodule/tests/xml/factories.py index 0c183ebd72a4..7409b0ac6dd0 100644 --- a/xmodule/tests/xml/factories.py +++ b/xmodule/tests/xml/factories.py @@ -11,6 +11,7 @@ from lxml import etree from xblock.mixins import HierarchyMixin +from xmodule.modulestore import only_xmodules from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.x_module import XModuleMixin @@ -71,6 +72,7 @@ class Meta: filesystem = OSFS(mkdtemp()) xblock_mixins = (InheritanceMixin, XModuleMixin, HierarchyMixin) + xblock_select = only_xmodules url_name = Sequence(str) attribs = {} policy = {} diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 772a2fbcd14a..1b9b441f371b 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -30,6 +30,7 @@ from xblock.core import XBlock from xblock.fields import ScopeIds from xblock.runtime import KvsFieldData +from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem from common.djangoapps.xblock_django.constants import ATTR_KEY_REQUEST_COUNTRY_CODE from openedx.core.djangoapps.video_config.models import HLSPlaybackEnabledFlag, CourseYoutubeBlockedFlag @@ -480,12 +481,17 @@ def get_html(self, view=STUDENT_VIEW, context=None): # lint-amnesty, pylint: di return self.runtime.service(self, 'mako').render_template('video.html', template_context) + def _is_lms_platform(self): + """ + Returns True if the platform is LMS. + """ + return isinstance(self.xmodule_runtime, LmsModuleSystem) + def _get_public_video_url(self): """ Returns the public video url """ - is_studio = getattr(self.runtime, "is_author_mode", False) - if self.public_access and not is_studio: + if self.public_access and self._is_lms_platform(): from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE if PUBLIC_VIDEO_SHARE.is_enabled(self.location.course_key): return urljoin( diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 031a895b0faa..bc19c1456e5c 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -313,42 +313,24 @@ 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 self._runtime + return CombinedSystem(self.xmodule_runtime, 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): """ Return the XBlock runtime (backwards compatibility alias provided for XModules). - - Deprecated in favor of the runtime property. """ - warnings.warn( - 'system property is deprecated. Please use the runtime property instead.', - DeprecationWarning, stacklevel=3, - ) return self.runtime @property @@ -611,11 +593,12 @@ def get_progress(self): """ return None - def bind_for_student(self, user_id, wrappers=None): + def bind_for_student(self, xmodule_runtime, 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. @@ -625,8 +608,8 @@ def bind_for_student(self, user_id, wrappers=None): # 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(self.runtime, 'position', None): - self.position = self.runtime.position # update the position of the tab + if getattr(xmodule_runtime, 'position', None): + self.position = xmodule_runtime.position # update the position of the tab return # If we are switching users mid-request, save the data from the old user. @@ -651,6 +634,9 @@ def bind_for_student(self, user_id, wrappers=None): 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 = [] @@ -1023,11 +1009,10 @@ def descriptor_global_local_resource_url(block, uri): class MetricsMixin: """ - Mixin for adding metric logging for render and handle methods in the DescriptorSystem. + Mixin for adding metric logging for render and handle methods in the DescriptorSystem and ModuleSystem. """ 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) @@ -1058,6 +1043,266 @@ def handle(self, block, handler_name, request, suffix=''): # lint-amnesty, pyli ) +class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): + """ + 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: [], **kwargs + ): + """ + load_item: Takes a Location and returns an XModuleDescriptor + + resources_fs: A Filesystem object that contains all of the + resources needed for the course + + error_tracker: A hook for tracking errors in loading the descriptor. + Used for example to get a list of all non-fatal problems on course + load, and display them to the user. + + See errortracker.py for more documentation + + get_policy: a function that takes a usage id and returns a dict of + policy to apply. + + local_resource_url: an implementation of :meth:`xblock.runtime.Runtime.local_resource_url` + + """ + kwargs.setdefault('id_reader', OpaqueKeyReader()) + kwargs.setdefault('id_generator', AsideKeyGenerator()) + super().__init__(**kwargs) + + # This is used by XModules to write out separate files during xml export + self.export_fs = None + + self.load_item = load_item + self.resources_fs = resources_fs + self.error_tracker = error_tracker + if get_policy: + self.get_policy = get_policy + else: + self.get_policy = lambda u: {} + + self.disabled_xblock_types = disabled_xblock_types + + def get_block(self, usage_id, for_parent=None): + """See documentation for `xblock.runtime:Runtime.get_block`""" + return self.load_item(usage_id, for_parent=for_parent) + + def load_block_type(self, block_type): + """ + Returns a subclass of :class:`.XBlock` that corresponds to the specified `block_type`. + """ + if block_type in self.disabled_xblock_types(): + return self.default_class + return super().load_block_type(block_type) + + def get_field_provenance(self, xblock, field): + """ + For the given xblock, return a dict for the field's current state: + { + 'default_value': what json'd value will take effect if field is unset: either the field default or + inherited value, + 'explicitly_set': boolean for whether the current value is set v default/inherited, + } + :param xblock: + :param field: + """ + # pylint: disable=protected-access + # in runtime b/c runtime contains app-specific xblock behavior. Studio's the only app + # which needs this level of introspection right now. runtime also is 'allowed' to know + # about the kvs, dbmodel, etc. + + result = {} + result['explicitly_set'] = xblock._field_data.has(xblock, field.name) + try: + result['default_value'] = xblock._field_data.default(xblock, field.name) + except KeyError: + result['default_value'] = field.to_json(field.default) + return result + + def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): + # Currently, Modulestore is responsible for instantiating DescriptorSystems + # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem + # that implements the correct handler url. So, for now, instead, we will reference a + # global function that the application can override. + return descriptor_global_handler_url(block, handler_name, suffix, query, thirdparty) + + def local_resource_url(self, block, uri): + """ + See :meth:`xblock.runtime.Runtime:local_resource_url` for documentation. + """ + # Currently, Modulestore is responsible for instantiating DescriptorSystems + # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem + # that implements the correct local_resource_url. So, for now, instead, we will reference a + # global function that the application can override. + return descriptor_global_local_resource_url(block, uri) + + def applicable_aside_types(self, block): + """ + See :meth:`xblock.runtime.Runtime:applicable_aside_types` for documentation. + """ + 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): + """ + See :meth:`xblock.runtime.Runtime:resource_url` for documentation. + """ + raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls") + + def add_block_as_child_node(self, block, node): + child = etree.SubElement(node, block.category) + child.set('url_name', block.url_name) + 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 + + 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 XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring + def __init__(self, process_xml, **kwargs): + """ + process_xml: Takes an xml string, and returns a XModuleDescriptor + created from that xml + """ + + super().__init__(**kwargs) + self.process_xml = process_xml + + def _usage_id_from_node(self, node, parent_id, id_generator=None): + """Create a new usage id from an XML dom node. + + Args: + node (lxml.etree.Element): The DOM node to interpret. + parent_id: The usage ID of the parent block + id_generator (IdGenerator): The :class:`.IdGenerator` to use + for creating ids + Returns: + UsageKey: the usage key for the new xblock + """ + return self.xblock_from_node(node, parent_id, id_generator).scope_ids.usage_id + + def xblock_from_node(self, node, parent_id, id_generator=None): + """ + Create an XBlock instance from XML data. + + Args: + xml_data (string): A string containing valid xml. + system (XMLParsingSystem): The :class:`.XMLParsingSystem` used to connect the block + to the outside world. + id_generator (IdGenerator): An :class:`~xblock.runtime.IdGenerator` that + will be used to construct the usage_id and definition_id for the block. + + Returns: + XBlock: The fully instantiated :class:`~xblock.core.XBlock`. + + """ + id_generator = id_generator or self.id_generator + # leave next line commented out - useful for low-level debugging + # log.debug('[_usage_id_from_node] tag=%s, class=%s' % (node.tag, xblock_class)) + + block_type = node.tag + # remove xblock-family from elements + node.attrib.pop('xblock-family', None) + + url_name = node.get('url_name') # difference from XBlock.runtime + def_id = id_generator.create_definition(block_type, url_name) + usage_id = id_generator.create_usage(def_id) + + keys = ScopeIds(None, block_type, def_id, usage_id) + block_class = self.mixologist.mix(self.load_block_type(block_type)) + + aside_children = self.parse_asides(node, def_id, usage_id, id_generator) + asides_tags = [x.tag for x in aside_children] + + block = block_class.parse_xml(node, self, keys, id_generator) + self._convert_reference_fields_to_keys(block) # difference from XBlock.runtime + block.parent = parent_id + block.save() + + asides = self.get_asides(block) + for asd in asides: + if asd.scope_ids.block_type in asides_tags: + block.add_aside(asd) + + return block + + def parse_asides(self, node, def_id, usage_id, id_generator): + """pull the asides out of the xml payload and instantiate them""" + aside_children = [] + for child in node.iterchildren(): + # get xblock-family from node + xblock_family = child.attrib.pop('xblock-family', None) + if xblock_family: + xblock_family = self._family_id_to_superclass(xblock_family) + if issubclass(xblock_family, XBlockAside): + aside_children.append(child) + # now process them & remove them from the xml payload + for child in aside_children: + self._aside_from_xml(child, def_id, usage_id, id_generator) + node.remove(child) + return aside_children + + def _make_usage_key(self, course_key, value): + """ + Makes value into a UsageKey inside the specified course. + If value is already a UsageKey, returns that. + """ + if isinstance(value, UsageKey): + return value + usage_key = UsageKey.from_string(value) + return usage_key.map_into_course(course_key) + + def _convert_reference_fields_to_keys(self, xblock): + """ + Find all fields of type reference and convert the payload into UsageKeys + """ + course_key = xblock.scope_ids.usage_id.course_key + + for field in xblock.fields.values(): + if field.is_set_on(xblock): + field_value = getattr(xblock, field.name) + if field_value is None: + continue + elif isinstance(field, Reference): + setattr(xblock, field.name, self._make_usage_key(course_key, field_value)) + elif isinstance(field, ReferenceList): + setattr(xblock, field.name, [self._make_usage_key(course_key, ele) for ele in field_value]) + elif isinstance(field, ReferenceValueDict): + for key, subvalue in field_value.items(): + assert isinstance(subvalue, str) + field_value[key] = self._make_usage_key(course_key, subvalue) + setattr(xblock, field.name, field_value) + + class ModuleSystemShim: """ This shim provides the properties formerly available from ModuleSystem which are now being provided by services. @@ -1076,7 +1321,7 @@ def anonymous_student_id(self): 'runtime.anonymous_student_id is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._runtime_services.get('user') or self._services.get('user') + user_service = self._services.get('user') if user_service: return user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) return None @@ -1106,7 +1351,7 @@ def user_id(self): 'runtime.user_id is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._runtime_services.get('user') or self._services.get('user') + user_service = self._services.get('user') if user_service: return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) return None @@ -1122,9 +1367,9 @@ def user_is_staff(self): 'runtime.user_is_staff is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._runtime_services.get('user') or self._services.get('user') + user_service = self._services.get('user') if user_service: - return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) + return self._services['user'].get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) return None @property @@ -1138,9 +1383,9 @@ def user_location(self): 'runtime.user_location is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._runtime_services.get('user') or self._services.get('user') + user_service = self._services.get('user') if user_service: - return user_service.get_current_user().opt_attrs.get(ATTR_KEY_REQUEST_COUNTRY_CODE) + return self._services['user'].get_current_user().opt_attrs.get(ATTR_KEY_REQUEST_COUNTRY_CODE) return None @property @@ -1158,9 +1403,9 @@ def get_real_user(self): 'runtime.get_real_user is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._runtime_services.get('user') or self._services.get('user') + user_service = self._services.get('user') if user_service: - return user_service.get_user_by_anonymous_id + return self._services['user'].get_user_by_anonymous_id return None @property @@ -1176,9 +1421,9 @@ def get_user_role(self): 'runtime.get_user_role is deprecated. Please use the user service instead.', DeprecationWarning, stacklevel=3, ) - user_service = self._runtime_services.get('user') or self._services.get('user') + user_service = self._services.get('user') if user_service: - return partial(user_service.get_current_user().opt_attrs.get, ATTR_KEY_USER_ROLE) + return partial(self._services['user'].get_current_user().opt_attrs.get, ATTR_KEY_USER_ROLE) @property def render_template(self): @@ -1192,27 +1437,11 @@ def render_template(self): '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._runtime_services.get('mako') or self._services.get('mako') + 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): """ @@ -1228,7 +1457,7 @@ def xqueue(self): 'runtime.xqueue is deprecated. Please use the xqueue service instead.', DeprecationWarning, stacklevel=3, ) - xqueue_service = self._runtime_services.get('xqueue') or self._services.get('xqueue') + xqueue_service = self._services.get('xqueue') if xqueue_service: return { 'interface': xqueue_service.interface, @@ -1250,7 +1479,7 @@ def can_execute_unsafe_code(self): 'runtime.can_execute_unsafe_code is deprecated. Please use the sandbox service instead.', DeprecationWarning, stacklevel=3, ) - sandbox_service = self._runtime_services.get('sandbox') or self._services.get('sandbox') + sandbox_service = self._services.get('sandbox') if sandbox_service: return sandbox_service.can_execute_unsafe_code # Default to saying "no unsafe code". @@ -1270,7 +1499,7 @@ def get_python_lib_zip(self): 'runtime.get_python_lib_zip is deprecated. Please use the sandbox service instead.', DeprecationWarning, stacklevel=3, ) - sandbox_service = self._runtime_services.get('sandbox') or self._services.get('sandbox') + sandbox_service = self._services.get('sandbox') if sandbox_service: return sandbox_service.get_python_lib_zip # Default to saying "no lib data" @@ -1289,7 +1518,7 @@ def cache(self): 'runtime.cache is deprecated. Please use the cache service instead.', DeprecationWarning, stacklevel=3, ) - return self._runtime_services.get('cache') or self._services.get('cache') or DoNothingCache() + return self._services.get('cache') or DoNothingCache() @property def replace_urls(self): @@ -1302,7 +1531,7 @@ def replace_urls(self): 'runtime.replace_urls is deprecated. Please use the replace_urls service instead.', DeprecationWarning, stacklevel=3, ) - replace_urls_service = self._runtime_services.get('replace_urls') or self._services.get('replace_urls') + replace_urls_service = self._services.get('replace_urls') if replace_urls_service: return partial(replace_urls_service.replace_urls, static_replace_only=True) @@ -1317,7 +1546,7 @@ def replace_course_urls(self): 'runtime.replace_course_urls is deprecated. Please use the replace_urls service instead.', DeprecationWarning, stacklevel=3, ) - replace_urls_service = self._runtime_services.get('replace_urls') or self._services.get('replace_urls') + replace_urls_service = self._services.get('replace_urls') if replace_urls_service: return partial(replace_urls_service.replace_urls) @@ -1332,7 +1561,7 @@ def replace_jump_to_id_urls(self): 'runtime.replace_jump_to_id_urls is deprecated. Please use the replace_urls service instead.', DeprecationWarning, stacklevel=3, ) - replace_urls_service = self._runtime_services.get('replace_urls') or self._services.get('replace_urls') + replace_urls_service = self._services.get('replace_urls') if replace_urls_service: return partial(replace_urls_service.replace_urls) @@ -1385,7 +1614,7 @@ def rebind_noauth_module_to_user(self): "rebind_noauth_module_to_user is deprecated. Please use the 'rebind_user' service instead.", DeprecationWarning, stacklevel=3 ) - rebind_user_service = self._runtime_services.get('rebind_user') or self._services.get('rebind_user') + rebind_user_service = self._services.get('rebind_user') if rebind_user_service: return partial(rebind_user_service.rebind_noauth_module_to_user) @@ -1413,65 +1642,47 @@ def course_id(self): "`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.for_branch(None) + return self.descriptor_runtime.course_id.for_branch(None) - @course_id.setter - def course_id(self, course_id): - """ - Set course_id. - 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 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. -class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): - """ - Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s + Note that these functions can be closures over e.g. a django request + and user, or other environment-specific info. """ + def __init__( - self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=lambda: [], **kwargs + self, + get_block, + descriptor_runtime, + **kwargs, ): """ - load_item: Takes a Location and returns an XModuleDescriptor + Create a closure around the system environment. - resources_fs: A Filesystem object that contains all of the - resources needed for the course - - error_tracker: A hook for tracking errors in loading the descriptor. - Used for example to get a list of all non-fatal problems on course - load, and display them to the user. - - See errortracker.py for more documentation - - get_policy: a function that takes a usage id and returns a dict of - policy to apply. - - local_resource_url: an implementation of :meth:`xblock.runtime.Runtime.local_resource_url` + 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', OpaqueKeyReader()) - kwargs.setdefault('id_generator', AsideKeyGenerator()) + + kwargs.setdefault('id_reader', getattr(descriptor_runtime, 'id_reader', OpaqueKeyReader())) + kwargs.setdefault('id_generator', getattr(descriptor_runtime, 'id_generator', AsideKeyGenerator())) super().__init__(**kwargs) - # This is used by XModules to write out separate files during xml export - self.export_fs = None + self.get_block_for_descriptor = get_block - self.load_item = load_item - self.resources_fs = resources_fs - self.error_tracker = error_tracker - if get_policy: - self.get_policy = get_policy - else: - self.get_policy = lambda u: {} + self.xmodule_instance = None - self.disabled_xblock_types = disabled_xblock_types - self._runtime_services = {} + self.descriptor_runtime = descriptor_runtime def get(self, attr): """ provide uniform access to attributes (like etree).""" @@ -1481,95 +1692,34 @@ 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`""" - block = self.load_item(usage_id, for_parent=for_parent) - # get_block_for_descriptor property is used to bind additional data such as user data - # to the XBlock and to check if the user has access to the block as may be required for - # the LMS or Preview. - if getattr(self, 'get_block_for_descriptor', None): - return self.get_block_for_descriptor(block) - return block - - def load_block_type(self, block_type): - """ - Returns a subclass of :class:`.XBlock` that corresponds to the specified `block_type`. - """ - if block_type in self.disabled_xblock_types(): - return self.default_class - return super().load_block_type(block_type) - - def get_field_provenance(self, xblock, field): - """ - For the given xblock, return a dict for the field's current state: - { - 'default_value': what json'd value will take effect if field is unset: either the field default or - inherited value, - 'explicitly_set': boolean for whether the current value is set v default/inherited, - } - :param xblock: - :param field: - """ - # pylint: disable=protected-access - # in runtime b/c runtime contains app-specific xblock behavior. Studio's the only app - # which needs this level of introspection right now. runtime also is 'allowed' to know - # about the kvs, dbmodel, etc. - - result = {} - result['explicitly_set'] = xblock._field_data.has(xblock, field.name) - try: - result['default_value'] = xblock._field_data.default(xblock, field.name) - except KeyError: - result['default_value'] = field.to_json(field.default) - return result + def __repr__(self): + kwargs = self.__dict__.copy() - def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): - # When the Modulestore instantiates DescriptorSystems, we will reference a - # global function that the application can override, unless a specific function is - # defined for LMS/CMS through the handler_url_override property. - if getattr(self, 'handler_url_override', None): - return self.handler_url_override(block, handler_name, suffix, query, thirdparty) - return descriptor_global_handler_url(block, handler_name, suffix, query, thirdparty) + # Remove value set transiently by XBlock + kwargs.pop('_view_name') - def local_resource_url(self, block, uri): - """ - See :meth:`xblock.runtime.Runtime:local_resource_url` for documentation. - """ - # Currently, Modulestore is responsible for instantiating DescriptorSystems - # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem - # that implements the correct local_resource_url. So, for now, instead, we will reference a - # global function that the application can override. - return descriptor_global_local_resource_url(block, uri) + return f"{self.__class__.__name__}{kwargs}" - def applicable_aside_types(self, block): + @property + def ajax_url(self): """ - See :meth:`xblock.runtime.Runtime:applicable_aside_types` for documentation. + The url prefix to be used by XModules to call into handle_ajax """ - # applicable_aside_types_override property can be used by LMS/CMS to define specific filters - # and conditions as may be applicable. - if getattr(self, 'applicable_aside_types_override', None): - return self.applicable_aside_types_override(block, applicable_aside_types=super().applicable_aside_types) + assert self.xmodule_instance is not None + return self.handler_url(self.xmodule_instance, 'xmodule_handler', '', '').rstrip('/?') - potential_set = set(super().applicable_aside_types(block)) - return list(potential_set) + 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): - """ - See :meth:`xblock.runtime.Runtime:resource_url` for documentation. - """ raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls") - def add_block_as_child_node(self, block, node): - child = etree.SubElement(node, block.category) - child.set('url_name', block.url_name) - block.add_xml_to_node(child) - 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._runtime_services.get('publish') or self._services.get('publish'): + if publish_service := self._services.get('publish'): publish_service.publish(block, event_type, event) def service(self, block, service_name): @@ -1586,148 +1736,123 @@ def service(self, block, service_name): Returns: An object implementing the requested service, or None. """ - declaration = block.service_declaration(service_name) - service = self._runtime_services.get(service_name) - if declaration is None or service is None: - # getting the service from parent module. making sure of block service declarations. - service = super().service(block=block, service_name=service_name) + # 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 - def wrap_aside(self, block, aside, view, frag, context): - # LMS/CMS can define custom wrap aside using wrap_asides_override as required. - if getattr(self, 'wrap_asides_override', None): - return self.wrap_asides_override(block, aside, view, frag, context, request_token=self.request_token) - return super().wrap_aside(block, aside, view, frag, context) - def layout_asides(self, block, context, frag, view_name, aside_frag_fns): - # LMS/CMS can define custom layout aside using layout_asides_override as required. - if getattr(self, 'layout_asides_override', None): - return self.layout_asides_override(block, context, frag, view_name, aside_frag_fns) - return super().layout_asides(block, context, frag, view_name, aside_frag_fns) +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') -class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring - def __init__(self, process_xml, **kwargs): - """ - process_xml: Takes an xml string, and returns a XModuleDescriptor - created from that xml + # 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. - super().__init__(**kwargs) - self.process_xml = process_xml + 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. - def _usage_id_from_node(self, node, parent_id, id_generator=None): - """Create a new usage id from an XML dom node. + 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. - Args: - node (lxml.etree.Element): The DOM node to interpret. - parent_id: The usage ID of the parent block - id_generator (IdGenerator): The :class:`.IdGenerator` to use - for creating ids - Returns: - UsageKey: the usage key for the new xblock - """ - return self.xblock_from_node(node, parent_id, id_generator).scope_ids.usage_id - - def xblock_from_node(self, node, parent_id, id_generator=None): """ - Create an XBlock instance from XML data. + context = context or {} + return self.__getattr__('render')(block, view_name, context) # pylint: disable=unnecessary-dunder-call - Args: - xml_data (string): A string containing valid xml. - system (XMLParsingSystem): The :class:`.XMLParsingSystem` used to connect the block - to the outside world. - id_generator (IdGenerator): An :class:`~xblock.runtime.IdGenerator` that - will be used to construct the usage_id and definition_id for the block. + def service(self, block, service_name): + """Return a service, or None. - Returns: - XBlock: The fully instantiated :class:`~xblock.core.XBlock`. + 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. - """ - id_generator = id_generator or self.id_generator - # leave next line commented out - useful for low-level debugging - # log.debug('[_usage_id_from_node] tag=%s, class=%s' % (node.tag, xblock_class)) + 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. - block_type = node.tag - # remove xblock-family from elements - node.attrib.pop('xblock-family', None) + Runtimes can override this method if they have different techniques for + finding and delivering services. - url_name = node.get('url_name') # difference from XBlock.runtime - def_id = id_generator.create_definition(block_type, url_name) - usage_id = id_generator.create_usage(def_id) + Arguments: + block (an XBlock): this block's class will be examined for service + decorators. + service_name (string): the name of the service requested. - keys = ScopeIds(None, block_type, def_id, usage_id) - block_class = self.mixologist.mix(self.load_block_type(block_type)) + Returns: + An object implementing the requested service, or None. - aside_children = self.parse_asides(node, def_id, usage_id, id_generator) - asides_tags = [x.tag for x in aside_children] + """ + service = None - block = block_class.parse_xml(node, self, keys, id_generator) - self._convert_reference_fields_to_keys(block) # difference from XBlock.runtime - block.parent = parent_id - block.save() + if self._module_system: + service = self._module_system.service(block, service_name) - asides = self.get_asides(block) - for asd in asides: - if asd.scope_ids.block_type in asides_tags: - block.add_aside(asd) + if service is None: + service = self._descriptor_system.service(block, service_name) - return block + return service - def parse_asides(self, node, def_id, usage_id, id_generator): - """pull the asides out of the xml payload and instantiate them""" - aside_children = [] - for child in node.iterchildren(): - # get xblock-family from node - xblock_family = child.attrib.pop('xblock-family', None) - if xblock_family: - xblock_family = self._family_id_to_superclass(xblock_family) - if issubclass(xblock_family, XBlockAside): - aside_children.append(child) - # now process them & remove them from the xml payload - for child in aside_children: - self._aside_from_xml(child, def_id, usage_id, id_generator) - node.remove(child) - return aside_children + 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 _make_usage_key(self, course_key, value): + def __setattr__(self, name, value): """ - Makes value into a UsageKey inside the specified course. - If value is already a UsageKey, returns that. + If the ModuleSystem is set, set the attr on it. + Always set the attr on the DescriptorSystem. """ - if isinstance(value, UsageKey): - return value - usage_key = UsageKey.from_string(value) - return usage_key.map_into_course(course_key) + if name in self.__slots__: + return super().__setattr__(name, value) - def _convert_reference_fields_to_keys(self, xblock): + if self._module_system: + setattr(self._module_system, name, value) + setattr(self._descriptor_system, name, value) + + def __delattr__(self, name): """ - Find all fields of type reference and convert the payload into UsageKeys + If the ModuleSystem is set, delete the attribute from it. + Always delete the attribute from the DescriptorSystem. """ - course_key = xblock.scope_ids.usage_id.course_key + if self._module_system: + delattr(self._module_system, name) + delattr(self._descriptor_system, name) - for field in xblock.fields.values(): - if field.is_set_on(xblock): - field_value = getattr(xblock, field.name) - if field_value is None: - continue - elif isinstance(field, Reference): - setattr(xblock, field.name, self._make_usage_key(course_key, field_value)) - elif isinstance(field, ReferenceList): - setattr(xblock, field.name, [self._make_usage_key(course_key, ele) for ele in field_value]) - elif isinstance(field, ReferenceValueDict): - for key, subvalue in field_value.items(): - assert isinstance(subvalue, str) - field_value[key] = self._make_usage_key(course_key, subvalue) - setattr(xblock, field.name, field_value) + def __repr__(self): + return f"CombinedSystem({self._module_system!r}, {self._descriptor_system!r})" class DoNothingCache: - """A duck-compatible object to use in ModuleSystemShim when there's no cache.""" + """A duck-compatible object to use in ModuleSystem when there's no cache.""" def get(self, _key): return None