Merge pull request #1899 from cpennington/descriptor-handler-url

Add handler_url usable by descriptors

cms/djangoapps/contentstore/tests/test_item.py

@@ -2,9 +2,20 @@
 
 import json
 import datetime
+import ddt
+
+from mock import Mock, patch
 from pytz import UTC
+from webob import Response
+
+from django.http import Http404
+from django.test import TestCase
+from django.test.client import RequestFactory
+
+from contentstore.views.component import component_handler
 
 from contentstore.tests.utils import CourseTestCase
+from student.tests.factories import UserFactory
 from xmodule.capa_module import CapaDescriptor
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.django import loc_mapper
@@ -354,3 +365,53 @@ class TestEditItem(ItemTest):
         self.assertIsNone(published.due)
         draft = self.get_item_from_modulestore(self.problem_locator, True)
         self.assertEqual(draft.due, datetime.datetime(2077, 10, 10, 4, 0, tzinfo=UTC))
+
+
+@ddt.ddt
+class TestComponentHandler(TestCase):
+    def setUp(self):
+        self.request_factory = RequestFactory()
+
+        patcher = patch('contentstore.views.component.modulestore')
+        self.modulestore = patcher.start()
+        self.addCleanup(patcher.stop)
+
+        self.descriptor = self.modulestore.return_value.get_item.return_value
+
+        self.usage_id = 'dummy_usage_id'
+
+        self.user = UserFactory()
+
+        self.request = self.request_factory.get('/dummy-url')
+        self.request.user = self.user
+
+    def test_invalid_handler(self):
+        self.descriptor.handle.side_effect = Http404
+
+        with self.assertRaises(Http404):
+            component_handler(self.request, self.usage_id, 'invalid_handler')
+
+    @ddt.data('GET', 'POST', 'PUT', 'DELETE')
+    def test_request_method(self, method):
+
+        def check_handler(handler, request, suffix):
+            self.assertEquals(request.method, method)
+            return Response()
+
+        self.descriptor.handle = check_handler
+
+        # Have to use the right method to create the request to get the HTTP method that we want
+        req_factory_method = getattr(self.request_factory, method.lower())
+        request = req_factory_method('/dummy-url')
+        request.user = self.user
+
+        component_handler(request, self.usage_id, 'dummy_handler')
+
+    @ddt.data(200, 404, 500)
+    def test_response_code(self, status_code):
+        def create_response(handler, request, suffix):
+            return Response(status_code=status_code)
+
+        self.descriptor.handle = create_response
+
+        self.assertEquals(component_handler(self.request, self.usage_id, 'dummy_handler').status_code, status_code)

cms/djangoapps/contentstore/views/component.py

@@ -2,11 +2,10 @@ import json
 import logging
 from collections import defaultdict
 
-from django.http import HttpResponseBadRequest
+from django.http import HttpResponseBadRequest, Http404
 from django.contrib.auth.decorators import login_required
 from django.views.decorators.http import require_http_methods
 from django.core.exceptions import PermissionDenied
-from django_future.csrf import ensure_csrf_cookie
 from django.conf import settings
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from edxmako.shortcuts import render_to_response
@@ -15,23 +14,27 @@ from xmodule.modulestore.django import modulestore
 from xmodule.util.date_utils import get_default_time_display
 from xmodule.modulestore.django import loc_mapper
 from xmodule.modulestore.locator import BlockUsageLocator
+from xmodule.x_module import XModuleDescriptor
 
+from xblock.django.request import webob_to_django_response, django_to_webob_request
+from xblock.exceptions import NoSuchHandlerError
 from xblock.fields import Scope
-from util.json_request import expect_json, JsonResponse
+from xblock.plugin import PluginMissingError
+from xblock.runtime import Mixologist
 
-from contentstore.utils import get_lms_link_for_item, compute_unit_state, UnitState, get_course_for_item
+from lms.lib.xblock.runtime import unquote_slashes
+
+from contentstore.utils import get_lms_link_for_item, compute_unit_state, UnitState
 
 from models.settings.course_grading import CourseGradingModel
 
 from .access import has_access
-from xmodule.x_module import XModuleDescriptor
-from xblock.plugin import PluginMissingError
-from xblock.runtime import Mixologist
 
 __all__ = ['OPEN_ENDED_COMPONENT_TYPES',
            'ADVANCED_COMPONENT_POLICY_KEY',
            'subsection_handler',
-           'unit_handler'
+           'unit_handler',
+           'component_handler'
            ]
 
 log = logging.getLogger(__name__)
@@ -311,3 +314,36 @@ def _get_item_in_course(request, locator):
     lms_link = get_lms_link_for_item(old_location, course_id=course.location.course_id)
 
     return old_location, course, item, lms_link
+
+
+@login_required
+def component_handler(request, usage_id, handler, suffix=''):
+    """
+    Dispatch an AJAX action to an xblock
+
+    Args:
+        usage_id: The usage-id of the block to dispatch to, passed through `quote_slashes`
+        handler (str): The handler to execute
+        suffix (str): The remainder of the url to be passed to the handler
+
+    Returns:
+        :class:`django.http.HttpResponse`: The response from the handler, converted to a
+            django response
+    """
+
+    location = unquote_slashes(usage_id)
+
+    descriptor = modulestore().get_item(location)
+    # Let the module handle the AJAX
+    req = django_to_webob_request(request)
+
+    try:
+        resp = descriptor.handle(handler, req, suffix)
+
+    except NoSuchHandlerError:
+        log.info("XBlock %s attempted to access missing handler %r", descriptor, handler, exc_info=True)
+        raise Http404
+
+    modulestore().save_xmodule(descriptor)
+
+    return webob_to_django_response(resp)

cms/djangoapps/models/settings/course_metadata.py

@@ -2,7 +2,7 @@ from xblock.fields import Scope
 
 from contentstore.utils import get_modulestore
 from xmodule.modulestore.inheritance import own_metadata
-from cms.xmodule_namespace import CmsBlockMixin
+from cms.lib.xblock.mixin import CmsBlockMixin
 
 
 class CourseMetadata(object):

cms/envs/common.py

@@ -29,7 +29,7 @@ from lms.envs.common import USE_TZ, TECH_SUPPORT_EMAIL, PLATFORM_NAME, BUGS_EMAI
 from path import path
 
 from lms.lib.xblock.mixin import LmsBlockMixin
-from cms.xmodule_namespace import CmsBlockMixin
+from cms.lib.xblock.mixin import CmsBlockMixin
 from xmodule.modulestore.inheritance import InheritanceMixin
 from xmodule.x_module import XModuleMixin
 from dealer.git import git

cms/lib/xblock/mixin.py

@@ -1,5 +1,5 @@
 """
-Namespace defining common fields used by Studio for all blocks
+Mixin defining common Studio functionality
 """
 
 import datetime

cms/lib/xblock/runtime.py

@@ -0,0 +1,30 @@
+"""
+XBlock runtime implementations for edX Studio
+"""
+
+from django.core.urlresolvers import reverse
+
+import xmodule.x_module
+from lms.lib.xblock.runtime import quote_slashes
+
+
+def handler_url(block, handler_name, suffix='', query='', thirdparty=False):
+    """
+    Handler URL function for Studio
+    """
+
+    if thirdparty:
+        raise NotImplementedError("edX Studio doesn't support third-party xblock handler urls")
+
+    url = reverse('component_handler', kwargs={
+        'usage_id': quote_slashes(str(block.scope_ids.usage_id)),
+        'handler': handler_name,
+        'suffix': suffix,
+    }).rstrip('/')
+
+    if query:
+        url += '?' + query
+
+    return url
+
+xmodule.x_module.descriptor_global_handler_url = handler_url

cms/lib/xblock/test/test_runtime.py

@@ -0,0 +1,51 @@
+"""
+Tests of edX Studio runtime functionality
+"""
+from urlparse import urlparse
+
+from mock import Mock
+from unittest import TestCase
+from cms.lib.xblock.runtime import handler_url
+
+
+class TestHandlerUrl(TestCase):
+    """Test the LMS handler_url"""
+
+    def setUp(self):
+        self.block = Mock()
+        self.course_id = "org/course/run"
+
+    def test_trailing_charecters(self):
+        self.assertFalse(handler_url(self.block, 'handler').endswith('?'))
+        self.assertFalse(handler_url(self.block, 'handler').endswith('/'))
+
+        self.assertFalse(handler_url(self.block, 'handler', 'suffix').endswith('?'))
+        self.assertFalse(handler_url(self.block, 'handler', 'suffix').endswith('/'))
+
+        self.assertFalse(handler_url(self.block, 'handler', 'suffix', 'query').endswith('?'))
+        self.assertFalse(handler_url(self.block, 'handler', 'suffix', 'query').endswith('/'))
+
+        self.assertFalse(handler_url(self.block, 'handler', query='query').endswith('?'))
+        self.assertFalse(handler_url(self.block, 'handler', query='query').endswith('/'))
+
+    def _parsed_query(self, query_string):
+        """Return the parsed query string from a handler_url generated with the supplied query_string"""
+        return urlparse(handler_url(self.block, 'handler', query=query_string)).query
+
+    def test_query_string(self):
+        self.assertIn('foo=bar', self._parsed_query('foo=bar'))
+        self.assertIn('foo=bar&baz=true', self._parsed_query('foo=bar&baz=true'))
+        self.assertIn('foo&bar&baz', self._parsed_query('foo&bar&baz'))
+
+    def _parsed_path(self, handler_name='handler', suffix=''):
+        """Return the parsed path from a handler_url with the supplied handler_name and suffix"""
+        return urlparse(handler_url(self.block, handler_name, suffix=suffix)).path
+
+    def test_suffix(self):
+        self.assertTrue(self._parsed_path(suffix="foo").endswith('foo'))
+        self.assertTrue(self._parsed_path(suffix="foo/bar").endswith('foo/bar'))
+        self.assertTrue(self._parsed_path(suffix="/foo/bar").endswith('/foo/bar'))
+
+    def test_handler_name(self):
+        self.assertIn('handler1', self._parsed_path('handler1'))
+        self.assertIn('handler_a', self._parsed_path('handler_a'))

cms/urls.py

@@ -16,9 +16,12 @@ urlpatterns = patterns('',  # nopep8
     url(r'^transcripts/rename$', 'contentstore.views.rename_transcripts', name='rename_transcripts'),
     url(r'^transcripts/save$', 'contentstore.views.save_transcripts', name='save_transcripts'),
 
-    url(r'^preview/xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>[^/]*))?$',
+    url(r'^preview/xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$',
         'contentstore.views.preview_handler', name='preview_handler'),
 
+    url(r'^xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$',
+        'contentstore.views.component_handler', name='component_handler'),
+
     # temporary landing page for a course
     url(r'^edge/(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<coursename>[^/]+)$',
         'contentstore.views.landing', name='landing'),

common/lib/xmodule/xmodule/tests/test_error_module.py

@@ -10,6 +10,7 @@ from mock import MagicMock, Mock, patch
 from xblock.runtime import Runtime, UsageStore
 from xblock.field_data import FieldData
 from xblock.fields import ScopeIds
+from xblock.test.tools import unabc
 
 
 class SetupTestErrorModules():
@@ -103,6 +104,11 @@ class TestException(Exception):
     pass
 
 
+@unabc("Tests should not call {}")
+class TestRuntime(Runtime):
+    pass
+
+
 class TestErrorModuleConstruction(unittest.TestCase):
     """
     Test that error module construction happens correctly
@@ -111,11 +117,11 @@ class TestErrorModuleConstruction(unittest.TestCase):
     def setUp(self):
         field_data = Mock(spec=FieldData)
         self.descriptor = BrokenDescriptor(
-            Runtime(Mock(spec=UsageStore), field_data),
+            TestRuntime(Mock(spec=UsageStore), field_data),
             field_data,
             ScopeIds(None, None, None, 'i4x://org/course/broken/name')
         )
-        self.descriptor.xmodule_runtime = Runtime(Mock(spec=UsageStore), field_data)
+        self.descriptor.xmodule_runtime = TestRuntime(Mock(spec=UsageStore), field_data)
         self.descriptor.xmodule_runtime.error_descriptor_class = ErrorDescriptor
         self.descriptor.xmodule_runtime.xmodule_instance = None
 

common/lib/xmodule/xmodule/x_module.py

@@ -841,6 +841,13 @@ class ConfigurableFragmentWrapper(object):  # pylint: disable=abstract-method
         return frag
 
 
+# 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")
+
+
 class DescriptorSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable=abstract-method
     """
     Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s
@@ -891,9 +898,9 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable
         self.resources_fs = resources_fs
         self.error_tracker = error_tracker
 
-    def get_block(self, block_id):
+    def get_block(self, usage_id):
         """See documentation for `xblock.runtime:Runtime.get_block`"""
-        return self.load_item(block_id)
+        return self.load_item(usage_id)
 
     def get_field_provenance(self, xblock, field):
         """
@@ -933,6 +940,23 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable
         else:
             return super(DescriptorSystem, self).render(block, view_name, context)
 
+    def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False):
+        xmodule_runtime = getattr(block, 'xmodule_runtime', None)
+        if xmodule_runtime is not None:
+            return xmodule_runtime.handler_url(block, handler_name, suffix, query, thirdparty)
+        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 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 resources_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")
+
 
 class XMLParsingSystem(DescriptorSystem):
     def __init__(self, process_xml, policy, **kwargs):
@@ -1079,6 +1103,15 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable=abs
         assert self.xmodule_instance is not None
         return self.handler_url(self.xmodule_instance, 'xmodule_handler', '', '').rstrip('/?')
 
+    def get_block(self, block_id):
+        raise NotImplementedError("XModules must use get_module to load other modules")
+
+    def resources_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")
+
 
 class DoNothingCache(object):
     """A duck-compatible object to use in ModuleSystem when there's no cache."""

rakelib/tests.rake

@@ -24,7 +24,7 @@ def run_tests(system, report_dir, test_id=nil, stop_on_failure=true)
 
     default_test_id = "#{system}/djangoapps common/djangoapps"
 
-    if system == :lms
+    if system == :lms || system == :cms
         default_test_id += " #{system}/lib"
     end
 

requirements/edx/github.txt

@@ -15,7 +15,7 @@
 -e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk
 
 # Our libraries:
--e git+https://github.com/edx/XBlock.git@c54c63cf8294c54512887e6232d4274003afc6e3#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@fa88607#egg=XBlock
 -e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail
 -e git+https://github.com/edx/diff-cover.git@v0.2.6#egg=diff_cover
 -e git+https://github.com/edx/js-test-tool.git@v0.1.4#egg=js_test_tool