Merge pull request #2603 from cpennington/xblock-acid-resources-test

Add tests of local_resource_url

cms/djangoapps/contentstore/tests/test_crud.py

@@ -1,4 +1,6 @@
 import unittest
+from django.conf import settings
+
 from xmodule import templates
 from xmodule.modulestore.tests import persistent_factories
 from xmodule.course_module import CourseDescriptor
@@ -9,7 +11,6 @@ from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator, LocalI
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.html_module import HtmlDescriptor
 from xmodule.modulestore import inheritance
-from xmodule.x_module import prefer_xmodules
 from xblock.core import XBlock
 
 
@@ -252,7 +253,7 @@ class TemplateTests(unittest.TestCase):
         class_ = XBlock.load_class(
             json_data.get('category', json_data.get('location', {}).get('category')),
             default_class,
-            select=prefer_xmodules
+            select=settings.XBLOCK_SELECT_FUNCTION
         )
         usage_id = json_data.get('_id', None)
         if not '_inherited_settings' in json_data and parent_xblock is not None:

cms/djangoapps/contentstore/views/component.py

@@ -1,3 +1,5 @@
+from __future__ import absolute_import
+
 import json
 import logging
 from collections import defaultdict
@@ -147,7 +149,7 @@ def _load_mixed_class(category):
     """
     Load an XBlock by category name, and apply all defined mixins
     """
-    component_class = XBlock.load_class(category, select=prefer_xmodules)
+    component_class = XBlock.load_class(category, select=settings.XBLOCK_SELECT_FUNCTION)
     mixologist = Mixologist(settings.XBLOCK_MIXINS)
     return mixologist.mix(component_class)
 

cms/djangoapps/contentstore/views/item.py

@@ -1,4 +1,5 @@
 """Views for items (modules)."""
+from __future__ import absolute_import
 
 import hashlib
 import logging
@@ -36,7 +37,7 @@ from .helpers import _xmodule_recurse
 from contentstore.views.preview import get_preview_fragment
 from edxmako.shortcuts import render_to_string
 from models.settings.course_grading import CourseGradingModel
-from cms.lib.xblock.runtime import handler_url
+from cms.lib.xblock.runtime import handler_url, local_resource_url
 
 __all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler']
 
@@ -49,6 +50,7 @@ CREATE_IF_NOT_FOUND = ['course_info']
 # monkey-patch the x_module library.
 # TODO: Remove this code when Runtimes are no longer created by modulestores
 xmodule.x_module.descriptor_global_handler_url = handler_url
+xmodule.x_module.descriptor_global_local_resource_url = local_resource_url
 
 
 def hash_resource(resource):

cms/djangoapps/contentstore/views/preview.py

@@ -1,3 +1,5 @@
+from __future__ import absolute_import
+
 import logging
 import hashlib
 from functools import partial
@@ -21,6 +23,7 @@ from xblock.fragment import Fragment
 
 from lms.lib.xblock.field_data import LmsFieldData
 from lms.lib.xblock.runtime import quote_slashes, unquote_slashes
+from cms.lib.xblock.runtime import local_resource_url
 
 from util.sandboxing import can_execute_unsafe_code
 
@@ -87,6 +90,9 @@ class PreviewModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
             'suffix': suffix,
         }) + '?' + query
 
+    def local_resource_url(self, block, uri):
+        return local_resource_url(block, uri)
+
 
 def _preview_module_system(request, descriptor):
     """

cms/djangoapps/contentstore/views/session_kv_store.py

@@ -1,6 +1,7 @@
 """
 An :class:`~xblock.runtime.KeyValueStore` that stores data in the django session
 """
+from __future__ import absolute_import
 
 from xblock.runtime import KeyValueStore
 

cms/djangoapps/contentstore/views/xblock.py

@@ -0,0 +1,33 @@
+"""
+Views dedicated to rendering xblocks.
+"""
+from __future__ import absolute_import
+
+import logging
+import mimetypes
+
+from xblock.core import XBlock
+
+from django.conf import settings
+from django.http import Http404, HttpResponse
+
+
+log = logging.getLogger(__name__)
+
+
+def xblock_resource(request, block_type, uri):  # pylint: disable=unused-argument
+    """
+    Return a package resource for the specified XBlock.
+    """
+    try:
+        xblock_class = XBlock.load_class(block_type, settings.XBLOCK_SELECT_FUNCTION)
+        content = xblock_class.open_local_resource(uri)
+    except IOError:
+        log.info('Failed to load xblock resource', exc_info=True)
+        raise Http404
+    except Exception:  # pylint: disable-msg=broad-except
+        log.error('Failed to load xblock resource', exc_info=True)
+        raise Http404
+
+    mimetype, _ = mimetypes.guess_type(uri)
+    return HttpResponse(content, mimetype=mimetype)

cms/envs/common.py

@@ -93,7 +93,6 @@ GITHUB_REPO_ROOT = ENV_ROOT / "data"
 
 sys.path.append(REPO_ROOT)
 sys.path.append(PROJECT_ROOT / 'djangoapps')
-sys.path.append(PROJECT_ROOT / 'lib')
 sys.path.append(COMMON_ROOT / 'djangoapps')
 sys.path.append(COMMON_ROOT / 'lib')
 

cms/lib/xblock/runtime.py

@@ -26,3 +26,12 @@ def handler_url(block, handler_name, suffix='', query='', thirdparty=False):
 
     return url
 
+
+def local_resource_url(block, uri):
+    """
+    local_resource_url for Studio
+    """
+    return reverse('xblock_resource_url', kwargs={
+        'block_type': block.scope_ids.block_type,
+        'uri': uri,
+    })

cms/urls.py

@@ -22,6 +22,9 @@ urlpatterns = patterns('',  # nopep8
     url(r'^xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$',
         'contentstore.views.component_handler', name='component_handler'),
 
+    url(r'^xblock/resource/(?P<block_type>[^/]*)/(?P<uri>.*)$',
+        'contentstore.views.xblock.xblock_resource', name='xblock_resource_url'),
+
     # temporary landing page for a course
     url(r'^edge/(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<coursename>[^/]+)$',
         'contentstore.views.landing', name='landing'),

common/djangoapps/xmodule_modifiers.py

@@ -173,9 +173,9 @@ def add_staff_debug_info(user, block, view, frag, context):  # pylint: disable=u
         histogram = None
         render_histogram = False
 
-    if settings.FEATURES.get('ENABLE_LMS_MIGRATION'):
+    if settings.FEATURES.get('ENABLE_LMS_MIGRATION') and hasattr(block.runtime, 'filestore'):
         [filepath, filename] = getattr(block, 'xml_attributes', {}).get('filename', ['', None])
-        osfs = block.system.filestore
+        osfs = block.runtime.filestore
         if filename is not None and osfs.exists(filename):
             # if original, unmangled filename exists then use it (github
             # doesn't like symlinks)

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -1503,7 +1503,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         """
         if fields is None:
             return {}
-        cls = self.mixologist.mix(XBlock.load_class(category, select=prefer_xmodules))
+        cls = self.mixologist.mix(XBlock.load_class(category, select=self.xblock_select))
         result = collections.defaultdict(dict)
         for field_name, value in fields.iteritems():
             field = getattr(cls, field_name)

common/lib/xmodule/xmodule/tests/__init__.py

@@ -47,6 +47,9 @@ class TestModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
     def handler_url(self, block, handler, suffix='', query='', thirdparty=False):
         return str(block.scope_ids.usage_id) + '/' + handler + '/' + suffix + '?' + query
 
+    def local_resource_url(self, block, uri):
+        return 'resource/' + str(block.scope_ids.block_type) + '/' + uri
+
 
 def get_test_system(course_id=''):
     """

common/lib/xmodule/xmodule/x_module.py

@@ -141,6 +141,13 @@ class XModuleMixin(XBlockMixin):
         default=None
     )
 
+    @property
+    def system(self):
+        """
+        Return the XBlock runtime (backwards compatibility alias provided for XModules).
+        """
+        return self.runtime
+
     @property
     def course_id(self):
         return self.runtime.course_id
@@ -400,7 +407,6 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock):  # pylint: disable=abstract-me
         self.descriptor = descriptor
         super(XModule, self).__init__(*args, **kwargs)
         self._loaded_children = None
-        self.system = self.runtime
         self.runtime.xmodule_instance = self
 
     def __unicode__(self):
@@ -634,7 +640,6 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
         XModuleDescriptor.__init__ takes the same arguments as xblock.core:XBlock.__init__
         """
         super(XModuleDescriptor, self).__init__(*args, **kwargs)
-        self.system = self.runtime
         # update_version is the version which last updated this xblock v prev being the penultimate updater
         # leaving off original_version since it complicates creation w/o any obv value yet and is computable
         # by following previous until None
@@ -891,8 +896,21 @@ class ConfigurableFragmentWrapper(object):  # pylint: disable=abstract-method
 # This function exists to give applications (LMS/CMS) a place to monkey-patch until
 # we can refactor modulestore to split out the FieldData half of its interface from
 # the Runtime part of its interface. This function matches the Runtime.handler_url interface
-def descriptor_global_handler_url(block, handler_name, suffix='', query='', thirdparty=False):
-    raise NotImplementedError("Applications must monkey-patch this function before using handler-urls for studio_view")
+def descriptor_global_handler_url(block, handler_name, suffix='', query='', thirdparty=False):  # pylint: disable=invalid-name, unused-argument
+    """
+    See :meth:`xblock.runtime.Runtime.handler_url`.
+    """
+    raise NotImplementedError("Applications must monkey-patch this function before using handler_url for studio_view")
+
+
+# This function exists to give applications (LMS/CMS) a place to monkey-patch until
+# we can refactor modulestore to split out the FieldData half of its interface from
+# the Runtime part of its interface. This function matches the Runtime.local_resource_url interface
+def descriptor_global_local_resource_url(block, uri):  # pylint: disable=invalid-name, unused-argument
+    """
+    See :meth:`xblock.runtime.Runtime.local_resource_url`.
+    """
+    raise NotImplementedError("Applications must monkey-patch this function before using local_resource_url for studio_view")
 
 
 class DescriptorSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable=abstract-method
@@ -941,6 +959,8 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable
         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`
+
         """
         super(DescriptorSystem, self).__init__(**kwargs)
 
@@ -1008,13 +1028,30 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable
             # global function that the application can override.
             return descriptor_global_handler_url(block, handler_name, suffix, query, thirdparty)
 
-    def resource_url(self, resource):
-        raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls")
-
     def local_resource_url(self, block, uri):
+        """
+        See :meth:`xblock.runtime.Runtime:local_resource_url` for documentation.
+        """
+        xmodule_runtime = getattr(block, 'xmodule_runtime', None)
+        if xmodule_runtime is not None:
+            return xmodule_runtime.local_resource_url(block, uri)
+        else:
+            # 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 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 publish(self, block, event):
+        """
+        See :meth:`xblock.runtime.Runtime:publish` for documentation.
+        """
         raise NotImplementedError("edX Platform doesn't currently implement XBlock publish")
 
     def add_block_as_child_node(self, block, node):
@@ -1180,9 +1217,6 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable=abs
     def resource_url(self, resource):
         raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls")
 
-    def local_resource_url(self, block, uri):
-        raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls")
-
     def publish(self, block, event):
         pass
 

common/static/coffee/src/xblock/core.coffee

@@ -22,6 +22,7 @@
 
     $element.trigger("xblock-initialized")
     $element.data("initialized", true)
+    $element.addClass("xblock-initialized")
     block
 
   initializeBlocks: (element) ->

common/test/acceptance/pages/xblock/acid.py

@@ -25,10 +25,7 @@ class AcidView(PageObject):
         self.context_selector = context_selector
 
     def is_browser_on_page(self):
-        return (
-            self.is_css_present('{} .acid-block'.format(self.context_selector)) and
-            self.browser.evaluate_script("$({!r}).data('initialized')".format(self.context_selector))
-        )
+        return self.is_css_present('{}.xblock-initialized .acid-block'.format(self.context_selector))
 
     def test_passed(self, test_selector):
         """
@@ -44,13 +41,6 @@ class AcidView(PageObject):
         """
         return self.test_passed('.js-init-run')
 
-    @property
-    def doc_ready_passed(self):
-        """
-        Whether the document-ready test passed in this view of the :class:`.AcidBlock`.
-        """
-        return self.test_passed('.document-ready-run')
-
     @property
     def child_tests_passed(self):
         """
@@ -61,6 +51,13 @@ class AcidView(PageObject):
             self.test_passed('.child-values-match')
         ])
 
+    @property
+    def resource_url_passed(self):
+        """
+        Whether the resource-url test passed in this view of the :class:`.AcidBlock`.
+        """
+        return self.test_passed('.local-resource-test')
+
     def scope_passed(self, scope):
         return all(
             self.test_passed('.scope-storage-test.scope-{} {}'.format(scope, test))

common/test/acceptance/tests/test_lms.py

@@ -369,9 +369,12 @@ class XBlockAcidBase(UniqueCourseTest):
 
         acid_block = AcidView(self.browser, '.xblock-student_view[data-block-type=acid]')
         self.assertTrue(acid_block.init_fn_passed)
-        self.assertTrue(acid_block.doc_ready_passed)
         self.assertTrue(acid_block.child_tests_passed)
+        self.assertTrue(acid_block.resource_url_passed)
         self.assertTrue(acid_block.scope_passed('user_state'))
+        self.assertTrue(acid_block.scope_passed('user_state_summary'))
+        self.assertTrue(acid_block.scope_passed('preferences'))
+        self.assertTrue(acid_block.scope_passed('user_info'))
 
 
 class XBlockAcidNoChildTest(XBlockAcidBase):

common/test/acceptance/tests/test_studio.py

@@ -157,9 +157,12 @@ class XBlockAcidBase(WebAppTest):
 
         acid_block = AcidView(self.browser, unit.components[0].preview_selector)
         self.assertTrue(acid_block.init_fn_passed)
-        self.assertTrue(acid_block.doc_ready_passed)
         self.assertTrue(acid_block.child_tests_passed)
+        self.assertTrue(acid_block.resource_url_passed)
         self.assertTrue(acid_block.scope_passed('user_state'))
+        self.assertTrue(acid_block.scope_passed('user_state_summary'))
+        self.assertTrue(acid_block.scope_passed('preferences'))
+        self.assertTrue(acid_block.scope_passed('user_info'))
 
     def test_acid_block_editor(self):
         """
@@ -173,8 +176,8 @@ class XBlockAcidBase(WebAppTest):
 
         acid_block = AcidView(self.browser, unit.components[0].edit().editor_selector)
         self.assertTrue(acid_block.init_fn_passed)
-        self.assertTrue(acid_block.doc_ready_passed)
         self.assertTrue(acid_block.child_tests_passed)
+        self.assertTrue(acid_block.resource_url_passed)
         self.assertTrue(acid_block.scope_passed('content'))
         self.assertTrue(acid_block.scope_passed('settings'))
 

lms/djangoapps/courseware/module_render.py

@@ -1,5 +1,6 @@
 import json
 import logging
+import mimetypes
 
 import static_replace
 
@@ -544,6 +545,23 @@ def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None):
     return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, request.user)
 
 
+def xblock_resource(request, block_type, uri):  # pylint: disable=unused-argument
+    """
+    Return a package resource for the specified XBlock.
+    """
+    try:
+        xblock_class = XBlock.load_class(block_type, select=settings.XBLOCK_SELECT_FUNCTION)
+        content = xblock_class.open_local_resource(uri)
+    except IOError:
+        log.info('Failed to load xblock resource', exc_info=True)
+        raise Http404
+    except Exception:  # pylint: disable-msg=broad-except
+        log.error('Failed to load xblock resource', exc_info=True)
+        raise Http404
+    mimetype, _ = mimetypes.guess_type(uri)
+    return HttpResponse(content, mimetype=mimetype)
+
+
 def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user):
     """
     Invoke an XBlock handler, either authenticated or not.

lms/lib/xblock/runtime.py

@@ -99,6 +99,15 @@ class LmsHandlerUrls(object):
 
         return url
 
+    def local_resource_url(self, block, uri):
+        """
+        local_resource_url for Studio
+        """
+        return reverse('xblock_resource_url', kwargs={
+            'block_type': block.scope_ids.block_type,
+            'uri': uri,
+        })
+
 
 class LmsModuleSystem(LmsHandlerUrls, ModuleSystem):  # pylint: disable=abstract-method
     """

lms/urls.py

@@ -190,6 +190,9 @@ if settings.COURSEWARE_ENABLED:
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/xblock/(?P<usage_id>[^/]*)/handler_noauth/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$',
             'courseware.module_render.handle_xblock_callback_noauth',
             name='xblock_handler_noauth'),
+        url(r'xblock/resource/(?P<block_type>[^/]+)/(?P<uri>.*)$',
+            'courseware.module_render.xblock_resource',
+            name='xblock_resource_url'),
 
         # Software Licenses
 

requirements/edx/github.txt

@@ -23,4 +23,4 @@
 -e git+https://github.com/edx/event-tracking.git@f0211d702d#egg=event-tracking
 -e git+https://github.com/edx/bok-choy.git@62de7b576a08f36cde5b030c52bccb1a2f3f8df1#egg=bok_choy
 -e git+https://github.com/edx-solutions/django-splash.git@15bf143b15714e22fc451ff1b0f8a7a2a9483172#egg=django-splash
--e git+https://github.com/edx/acid-block.git@9c832513f0c01f79227bea894fba11c66fe4c08c#egg=acid-xblock
+-e git+https://github.com/edx/acid-block.git@bf61f0fcd5916a9236bb5681c98374a48a13a74c#egg=acid-xblock