diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 6f35bf1a14..cf5b9fff19 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -203,8 +203,9 @@ class XModuleMixin(XBlockMixin): # so we should use the xmodule_runtime (ModuleSystem) as the runtime. if (not isinstance(self, (XModule, XModuleDescriptor)) and self.xmodule_runtime is not None): - return self.xmodule_runtime - return self._runtime + return PureSystem(self.xmodule_runtime, self._runtime) + else: + return self._runtime @runtime.setter def runtime(self, value): @@ -443,10 +444,9 @@ class XModuleMixin(XBlockMixin): """ Set up this XBlock to act as an XModule instead of an XModuleDescriptor. - :param xmodule_runtime: the runtime to use when accessing student facing methods - :type xmodule_runtime: :class:`ModuleSystem` - :param field_data: The :class:`FieldData` to use for all subsequent data access - :type field_data: :class:`FieldData` + Arguments: + xmodule_runtime (:class:`ModuleSystem'): the runtime to use when accessing student facing methods + field_data (:class:`FieldData`): The :class:`FieldData` to use for all subsequent data access """ # pylint: disable=attribute-defined-outside-init self.xmodule_runtime = xmodule_runtime @@ -1408,6 +1408,36 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylin pass +class PureSystem(ModuleSystem, DescriptorSystem): + """ + A subclass of both ModuleSystem and DescriptorSystem to provide pure (non-XModule) XBlocks + a single Runtime interface for both the ModuleSystem and DescriptorSystem, when available. + """ + # 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 + # + # pylint: disable=abstract-method + def __init__(self, module_system, descriptor_system): + # N.B. This doesn't call super(PureSystem, self).__init__, because it is only inheriting from + # ModuleSystem and DescriptorSystem to pass isinstance checks. + # pylint: disable=super-init-not-called + self._module_system = module_system + self._descriptor_system = descriptor_system + + 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. + """ + try: + return getattr(self._module_system, name) + except AttributeError: + return getattr(self._descriptor_system, name) + + class DoNothingCache(object): """A duck-compatible object to use in ModuleSystem when there's no cache.""" def get(self, _key): diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index cc500dd4b4..055765cbda 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -17,6 +17,7 @@ from capa.tests.response_xml_factory import OptionResponseXMLFactory from xblock.field_data import FieldData from xblock.runtime import Runtime from xblock.fields import ScopeIds +from xblock.core import XBlock from xmodule.lti_module import LTIDescriptor from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -41,6 +42,14 @@ from lms.lib.xblock.runtime import quote_slashes from xmodule.modulestore import ModuleStoreEnum +class PureXBlock(XBlock): + """ + Pure XBlock to use in tests. + """ + pass + + +@ddt.ddt @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): """ @@ -161,6 +170,17 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEquals(403, response.status_code) self.assertEquals('Unauthenticated', response.content) + @ddt.data('pure', 'vertical') + @XBlock.register_temp_plugin(PureXBlock, identifier='pure') + def test_rebinding_same_user(self, block_type): + request = self.request_factory.get('') + request.user = self.mock_user + course = CourseFactory() + descriptor = ItemFactory(category=block_type, parent=course) + field_data_cache = FieldDataCache([self.toy_course, descriptor], self.toy_course.id, self.mock_user) + render.get_module_for_descriptor(self.mock_user, request, descriptor, field_data_cache, self.toy_course.id) + render.get_module_for_descriptor(self.mock_user, request, descriptor, field_data_cache, self.toy_course.id) + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class TestHandleXBlockCallback(ModuleStoreTestCase, LoginEnrollmentTestCase): diff --git a/lms/lib/xblock/runtime.py b/lms/lib/xblock/runtime.py index 6195048190..f02657c511 100644 --- a/lms/lib/xblock/runtime.py +++ b/lms/lib/xblock/runtime.py @@ -196,11 +196,3 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract ) services['fs'] = xblock.reference.plugins.FSService() super(LmsModuleSystem, self).__init__(**kwargs) - - # backward compatibility fix for callers not knowing this is a ModuleSystem v DescriptorSystem - @property - def resources_fs(self): - """ - Return what would be the resources_fs on a DescriptorSystem - """ - return getattr(self, 'filestore', None)