Merge pull request #5783 from cpennington/proxy-through-module-system-to-descriptor-system

Fix PLAT-191

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):

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):

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)