Use XBlock handlers for handle_ajax in XModules

Adds xblock handler_url support to the LMS, and makes handle_ajax use
that code.

[LMS-230] [LMS-229]

cms/djangoapps/contentstore/views/preview.py

 import logging
-import sys
 from functools import partial
 
 from django.conf import settings
-from django.http import HttpResponse, Http404, HttpResponseBadRequest, HttpResponseForbidden
 from django.core.urlresolvers import reverse
+from django.http import Http404, HttpResponseBadRequest, HttpResponseForbidden
 from django.contrib.auth.decorators import login_required
 from mitxmako.shortcuts import render_to_response, render_to_string
 
 from xmodule_modifiers import replace_static_urls, wrap_xblock
 from xmodule.error_module import ErrorDescriptor
-from xmodule.errortracker import exc_info_to_str
 from xmodule.exceptions import NotFoundError, ProcessingError
 from xmodule.modulestore.django import modulestore
 from xmodule.x_module import ModuleSystem
 from xblock.runtime import DbModel
+from xblock.django.request import webob_to_django_response, django_to_webob_request
+from xblock.exceptions import NoSuchHandlerError
 
 from lms.lib.xblock.field_data import LmsFieldData
+from lms.lib.xblock.runtime import quote_slashes, unquote_slashes
 
 from util.sandboxing import can_execute_unsafe_code
 
@@ -26,30 +27,47 @@ from .helpers import render_from_lms
 from .access import has_access
 from ..utils import get_course_for_item
 
-__all__ = ['preview_dispatch', 'preview_component']
+__all__ = ['preview_handler', 'preview_component']
 
 log = logging.getLogger(__name__)
 
 
-@login_required
-def preview_dispatch(request, preview_id, location, dispatch=None):
+def handler_prefix(block, handler='', suffix=''):
+    """
+    Return a url prefix for XBlock handler_url. The full handler_url
+    should be '{prefix}/{handler}/{suffix}?{query}'.
+
+    Trailing `/`s are removed from the returned url.
     """
-    Dispatch an AJAX action to a preview XModule
+    return reverse('preview_handler', kwargs={
+        'usage_id': quote_slashes(str(block.scope_ids.usage_id)),
+        'handler': handler,
+        'suffix': suffix,
+    }).rstrip('/?')
 
-    Expects a POST request, and passes the arguments to the module
 
-    preview_id (str): An identifier specifying which preview this module is used for
-    location: The Location of the module to dispatch to
-    dispatch: The action to execute
+@login_required
+def preview_handler(request, usage_id, handler, suffix=''):
     """
+    Dispatch an AJAX action to an xblock
+
+    usage_id: The usage-id of the block to dispatch to, passed through `quote_slashes`
+    handler: The handler to execute
+    suffix: The remaineder of the url to be passed to the handler
+    """
+
+    location = unquote_slashes(usage_id)
 
     descriptor = modulestore().get_item(location)
-    instance = load_preview_module(request, preview_id, descriptor)
+    instance = load_preview_module(request, descriptor)
     # Let the module handle the AJAX
+    req = django_to_webob_request(request)
     try:
-        ajax_return = instance.handle_ajax(dispatch, request.POST)
-        # Save any module data that has changed to the underlying KeyValueStore
-        instance.save()
+        resp = instance.handle(handler, req, suffix)
+
+    except NoSuchHandlerError:
+        log.exception("XBlock %s attempted to access missing handler %r", instance, handler)
+        raise Http404
 
     except NotFoundError:
         log.exception("Module indicating to user that request doesn't exist")
@@ -60,11 +78,11 @@ def preview_dispatch(request, preview_id, location, dispatch=None):
                     exc_info=True)
         return HttpResponseBadRequest()
 
-    except:
+    except Exception:
         log.exception("error processing ajax call")
         raise
 
-    return HttpResponse(ajax_return)
+    return webob_to_django_response(resp)
 
 
 @login_required
@@ -77,7 +95,7 @@ def preview_component(request, location):
     component = modulestore().get_item(location)
     # Wrap the generated fragment in the xmodule_editor div so that the javascript
     # can bind to it correctly
-    component.runtime.wrappers.append(wrap_xblock)
+    component.runtime.wrappers.append(partial(wrap_xblock, handler_prefix))
 
     try:
         content = component.render('studio_view').content
@@ -88,30 +106,36 @@ def preview_component(request, location):
         content = render_to_string('html_error.html', {'message': str(exc)})
 
     return render_to_response('component.html', {
-        'preview': get_preview_html(request, component, 0),
+        'preview': get_preview_html(request, component),
         'editor': content
     })
 
 
-def preview_module_system(request, preview_id, descriptor):
+class PreviewModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
+    """
+    An XModule ModuleSystem for use in Studio previews
+    """
+    def handler_url(self, block, handler_name, suffix='', query=''):
+        return handler_prefix(block, handler_name, suffix) + '?' + query
+
+
+def preview_module_system(request, descriptor):
     """
     Returns a ModuleSystem for the specified descriptor that is specialized for
     rendering module previews.
 
     request: The active django request
-    preview_id (str): An identifier specifying which preview this module is used for
     descriptor: An XModuleDescriptor
     """
 
     course_id = get_course_for_item(descriptor.location).location.course_id
 
-    return ModuleSystem(
+    return PreviewModuleSystem(
         static_url=settings.STATIC_URL,
-        ajax_url=reverse('preview_dispatch', args=[preview_id, descriptor.location.url(), '']).rstrip('/'),
         # TODO (cpennington): Do we want to track how instructors are using the preview problems?
         track_function=lambda event_type, event: None,
         filestore=descriptor.runtime.resources_fs,
-        get_module=partial(load_preview_module, request, preview_id),
+        get_module=partial(load_preview_module, request),
         render_template=render_from_lms,
         debug=True,
         replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id),
@@ -124,7 +148,7 @@ def preview_module_system(request, preview_id, descriptor):
         # Set up functions to modify the fragment produced by student_view
         wrappers=(
             # This wrapper wraps the module in the template specified above
-            partial(wrap_xblock, display_name_only=descriptor.location.category == 'static_tab'),
+            partial(wrap_xblock, handler_prefix, display_name_only=descriptor.location.category == 'static_tab'),
 
             # This wrapper replaces urls in the output that start with /static
             # with the correct course-specific url for the static content
@@ -138,28 +162,27 @@ def preview_module_system(request, preview_id, descriptor):
     )
 
 
-def load_preview_module(request, preview_id, descriptor):
+def load_preview_module(request, descriptor):
     """
     Return a preview XModule instantiated from the supplied descriptor.
 
     request: The active django request
-    preview_id (str): An identifier specifying which preview this module is used for
     descriptor: An XModuleDescriptor
     """
     student_data = DbModel(SessionKeyValueStore(request))
     descriptor.bind_for_student(
-        preview_module_system(request, preview_id, descriptor),
+        preview_module_system(request, descriptor),
         LmsFieldData(descriptor._field_data, student_data),  # pylint: disable=protected-access
     )
     return descriptor
 
 
-def get_preview_html(request, descriptor, idx):
+def get_preview_html(request, descriptor):
     """
     Returns the HTML returned by the XModule's student_view,
     specified by the descriptor and idx.
     """
-    module = load_preview_module(request, str(idx), descriptor)
+    module = load_preview_module(request, descriptor)
     try:
         content = module.render("student_view").content
     except Exception as exc:                          # pylint: disable=W0703

cms/urls.py

@@ -31,8 +31,8 @@ urlpatterns = patterns('',  # nopep8
     url(r'^unpublish_unit$', 'contentstore.views.unpublish_unit', name='unpublish_unit'),
     url(r'^reorder_static_tabs', 'contentstore.views.reorder_static_tabs', name='reorder_static_tabs'),
 
-    url(r'^preview/modx/(?P<preview_id>[^/]*)/(?P<location>.*?)/(?P<dispatch>[^/]*)$',
-        'contentstore.views.preview_dispatch', name='preview_dispatch'),
+    url(r'^preview/xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>[^/]*))?$',
+        'contentstore.views.preview_handler', name='preview_handler'),
 
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/info/(?P<name>[^/]+)$',
         'contentstore.views.course_info', name='course_info'),

common/djangoapps/xmodule_modifiers.py

@@ -29,11 +29,14 @@ def wrap_fragment(fragment, new_content):
     return wrapper_frag
 
 
-def wrap_xblock(block, view, frag, context, display_name_only=False):  # pylint: disable=unused-argument
+def wrap_xblock(handler_prefix, block, view, frag, context, display_name_only=False):  # pylint: disable=unused-argument
     """
     Wraps the results of rendering an XBlock view in a standard <section> with identifying
     data so that the appropriate javascript module can be loaded onto it.
 
+    :param handler_prefix: A function that takes a block and returns the url prefix for
+        the javascript handler_url. This prefix should be able to have {handler_name}/{suffix}?{query}
+        appended to it to return a valid handler_url
     :param block: An XBlock (that may be an XModule or XModuleDescriptor)
     :param view: The name of the view that rendered the fragment being wrapped
     :param frag: The :class:`Fragment` to be wrapped
@@ -63,7 +66,7 @@ def wrap_xblock(block, view, frag, context, display_name_only=False):  # pylint:
     if frag.js_init_fn:
         data['init'] = frag.js_init_fn
         data['runtime-version'] = frag.js_init_version
-        data['usage-id'] = block.scope_ids.usage_id
+        data['handler-prefix'] = handler_prefix(block)
         data['block-type'] = block.scope_ids.block_type
 
     template_context = {

common/lib/capa/capa/tests/__init__.py

@@ -33,9 +33,9 @@ def test_system():
     """
     the_system = Mock(
         spec=ModuleSystem,
+        ajax_url='/dummy-ajax-url',
         STATIC_URL='/dummy-static/',
         DEBUG=True,
-        ajax_url='courses/course_id/modx/a_location',
         track_function=Mock(),
         get_module=Mock(),
         render_template=tst_render_template,

common/lib/xmodule/setup.py

@@ -51,6 +51,7 @@ setup(
         'docopt',
         'capa',
         'path.py',
+        'webob',
     ],
     package_data={
         'xmodule': ['js/module/*'],

common/lib/xmodule/xmodule/capa_module.py

@@ -803,7 +803,7 @@ class CapaModule(CapaFields, XModule):
         """
         Make dictionary of student responses (aka "answers")
 
-        `data` is POST dictionary (Django QueryDict).
+        `data` is POST dictionary (webob.multidict.MultiDict).
 
         The `data` dict has keys of the form 'x_y', which are mapped
         to key 'y' in the returned dict.  For example,
@@ -835,7 +835,10 @@ class CapaModule(CapaFields, XModule):
         """
         answers = dict()
 
-        for key in data:
+        # webob.multidict.MultiDict is a view of a list of tuples,
+        # so it will return a multi-value key once for each value.
+        # We only want to consider each key a single time, so we use set(data.keys())
+        for key in set(data.keys()):
             # e.g. input_resistor_1 ==> resistor_1
             _, _, name = key.partition('_')
 
@@ -857,7 +860,7 @@ class CapaModule(CapaFields, XModule):
                 name = name[:-2] if is_list_key or is_dict_key else name
 
                 if is_list_key:
-                    val = data.getlist(key)
+                    val = data.getall(key)
                 elif is_dict_key:
                     try:
                         val = json.loads(data[key])

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -217,9 +217,8 @@ class Location(_LocationBase):
         Return a string with a version of the location that is safe for use in
         html id attributes
         """
-        s = "-".join(str(v) for v in self.list()
-                     if v is not None)
-        return Location.clean_for_html(s)
+        id_string = "-".join(str(v) for v in self.list() if v is not None)
+        return Location.clean_for_html(id_string)
 
     def dict(self):
         """

common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py

@@ -213,6 +213,10 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
 
         with 'error' only present if 'success' is False, and 'hint_html' or
         'message_html' only if success is true
+
+        :param data: A `webob.multidict.MultiDict` containing the keys
+            asasssment: The sum of assessment scores
+            score_list[]: A multivalue key containing all the individual scores
         """
 
         if self.child_state != self.ASSESSING:
@@ -220,9 +224,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
 
         try:
             score = int(data.get('assessment'))
-            score_list = data.getlist('score_list[]')
-            for i in xrange(0, len(score_list)):
-                score_list[i] = int(score_list[i])
+            score_list = [int(x) for x in data.getall('score_list[]')]
         except (ValueError, TypeError):
             # This is a dev_facing_error
             log.error("Non-integer score value passed to save_assessment, or no score list present.")

common/lib/xmodule/xmodule/peer_grading_module.py

@@ -140,9 +140,15 @@ class PeerGradingModule(PeerGradingFields, XModule):
         except Exception:
             pass
 
-        self.ajax_url = self.system.ajax_url
-        if not self.ajax_url.endswith("/"):
-            self.ajax_url = self.ajax_url + "/"
+    @property
+    def ajax_url(self):
+        """
+        Returns the `ajax_url` from the system, with any trailing '/' stripped off.
+        """
+        ajax_url = self.system.ajax_url
+        if not ajax_url.endswith("/"):
+            ajax_url += "/"
+        return ajax_url
 
     def closed(self):
         return self._closed(self.timeinfo)
@@ -333,7 +339,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
 
         data_dict = {k:data.get(k) for k in required}
         if 'rubric_scores[]' in required:
-            data_dict['rubric_scores'] = data.getlist('rubric_scores[]')
+            data_dict['rubric_scores'] = data.getall('rubric_scores[]')
         data_dict['grader_id'] = self.system.anonymous_student_id
 
         try:
@@ -469,7 +475,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             return self._err_response(message)
 
         data_dict = {k:data.get(k) for k in required}
-        data_dict['rubric_scores'] = data.getlist('rubric_scores[]')
+        data_dict['rubric_scores'] = data.getall('rubric_scores[]')
         data_dict['student_id'] = self.system.anonymous_student_id
         data_dict['calibration_essay_id'] = data_dict['submission_id']
 

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

@@ -38,6 +38,14 @@ open_ended_grading_interface = {
     }
 
 
+class TestModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
+    """
+    ModuleSystem for testing
+    """
+    def handler_url(self, block, handler, suffix='', query=''):
+        return str(block.scope_ids.usage_id) + '/' + handler + '/' + suffix + '?' + query
+
+
 def get_test_system(course_id=''):
     """
     Construct a test ModuleSystem instance.
@@ -51,9 +59,8 @@ def get_test_system(course_id=''):
     where `my_render_func` is a function of the form my_render_func(template, context).
 
     """
-    return ModuleSystem(
+    return TestModuleSystem(
         static_url='/static',
-        ajax_url='courses/course_id/modx/a_location',
         track_function=Mock(),
         get_module=Mock(),
         render_template=mock_render_template,
@@ -103,15 +110,6 @@ class ModelsTest(unittest.TestCase):
         vc_str = "<class 'xmodule.video_module.VideoDescriptor'>"
         self.assertEqual(str(vc), vc_str)
 
-class PostData(object):
-    """Class which emulate postdata."""
-    def __init__(self, dict_data):
-        self.dict_data = dict_data
-
-    def getlist(self, key):
-        """Get data by key from `self.dict_data`."""
-        return self.dict_data.get(key)
-
 
 class LogicTest(unittest.TestCase):
     """Base class for testing xmodule logic."""

common/lib/xmodule/xmodule/tests/test_capa_module.py

@@ -8,11 +8,13 @@ Tests of the Capa XModule
 #pylint: disable=C0302
 
 import datetime
-from mock import Mock, patch
 import unittest
 import random
 import json
 
+from mock import Mock, patch
+from webob.multidict import MultiDict
+
 import xmodule
 from capa.responsetypes import (StudentInputError, LoncapaProblemError,
                                 ResponseError)
@@ -21,8 +23,6 @@ from xmodule.modulestore import Location
 from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
 
-from django.http import QueryDict
-
 from . import get_test_system
 from pytz import UTC
 from capa.correctmap import CorrectMap
@@ -133,6 +133,7 @@ class CapaFactory(object):
             DictFieldData(field_data),
             ScopeIds(None, None, location, location),
         )
+        system.xmodule_instance = module
 
         if correct:
             # TODO: probably better to actually set the internal state properly, but...
@@ -330,19 +331,16 @@ class CapaModuleTest(unittest.TestCase):
 
     def test_parse_get_params(self):
 
-        # We have to set up Django settings in order to use QueryDict
-        from django.conf import settings
-        if not settings.configured:
-            settings.configure()
-
         # Valid GET param dict
-        valid_get_dict = self._querydict_from_dict({'input_1': 'test',
+        # 'input_5' intentionally left unset,
+        valid_get_dict = MultiDict({
+            'input_1': 'test',
             'input_1_2': 'test',
             'input_1_2_3': 'test',
             'input_[]_3': 'test',
             'input_4': None,
-                                                    'input_5': [],
-                                                    'input_6': 5})
+            'input_6': 5
+        })
 
         result = CapaModule.make_dict_of_responses(valid_get_dict)
 
@@ -355,52 +353,31 @@ class CapaModuleTest(unittest.TestCase):
             self.assertEqual(valid_get_dict[original_key], result[key])
 
         # Valid GET param dict with list keys
-        valid_get_dict = self._querydict_from_dict({'input_2[]': ['test1', 'test2']})
+        # Each tuple represents a single parameter in the query string
+        valid_get_dict = MultiDict((('input_2[]', 'test1'), ('input_2[]', 'test2')))
         result = CapaModule.make_dict_of_responses(valid_get_dict)
         self.assertTrue('2' in result)
         self.assertEqual(['test1', 'test2'], result['2'])
 
         # If we use [] at the end of a key name, we should always
         # get a list, even if there's just one value
-        valid_get_dict = self._querydict_from_dict({'input_1[]': 'test'})
+        valid_get_dict = MultiDict({'input_1[]': 'test'})
         result = CapaModule.make_dict_of_responses(valid_get_dict)
         self.assertEqual(result['1'], ['test'])
 
         # If we have no underscores in the name, then the key is invalid
-        invalid_get_dict = self._querydict_from_dict({'input': 'test'})
+        invalid_get_dict = MultiDict({'input': 'test'})
         with self.assertRaises(ValueError):
             result = CapaModule.make_dict_of_responses(invalid_get_dict)
 
         # Two equivalent names (one list, one non-list)
         # One of the values would overwrite the other, so detect this
         # and raise an exception
-        invalid_get_dict = self._querydict_from_dict({'input_1[]': 'test 1',
+        invalid_get_dict = MultiDict({'input_1[]': 'test 1',
                                       'input_1': 'test 2'})
         with self.assertRaises(ValueError):
             result = CapaModule.make_dict_of_responses(invalid_get_dict)
 
-    def _querydict_from_dict(self, param_dict):
-        """
-        Create a Django QueryDict from a Python dictionary
-        """
-
-        # QueryDict objects are immutable by default, so we make
-        # a copy that we can update.
-        querydict = QueryDict('')
-        copyDict = querydict.copy()
-
-        for (key, val) in param_dict.items():
-
-            # QueryDicts handle lists differently from ordinary values,
-            # so we have to specifically tell the QueryDict that
-            # this is a list
-            if type(val) is list:
-                copyDict.setlist(key, val)
-            else:
-                copyDict[key] = val
-
-        return copyDict
-
     def test_check_problem_correct(self):
 
         module = CapaFactory.create(attempts=1)

common/lib/xmodule/xmodule/tests/test_combined_open_ended.py

@@ -6,14 +6,15 @@ OpenEndedModule
 
 """
 
-from datetime import datetime
 import json
 import logging
 import unittest
 
+from datetime import datetime
 from lxml import etree
 from mock import Mock, MagicMock, ANY, patch
 from pytz import UTC
+from webob.multidict import MultiDict
 
 from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild
 from xmodule.open_ended_grading_classes.open_ended_module import OpenEndedModule
@@ -24,7 +25,7 @@ from xmodule.modulestore import Location
 from xmodule.tests import get_test_system, test_util_open_ended
 from xmodule.progress import Progress
 from xmodule.tests.test_util_open_ended import (
-    MockQueryDict, DummyModulestore, TEST_STATE_SA_IN,
+    DummyModulestore, TEST_STATE_SA_IN,
     MOCK_INSTANCE_STATE, TEST_STATE_SA, TEST_STATE_AI, TEST_STATE_AI2, TEST_STATE_AI2_INVALID,
     TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE, MockUploadedFile
 )
@@ -646,9 +647,13 @@ class CombinedOpenEndedModuleTest(unittest.TestCase):
         """
         Return a combined open ended module with the specified parameters
         """
-        definition = {'prompt': etree.XML(self.prompt), 'rubric': etree.XML(self.rubric),
-                      'task_xml': task_xml}
+        definition = {
+            'prompt': etree.XML(self.prompt),
+            'rubric': etree.XML(self.rubric),
+            'task_xml': task_xml
+        }
         descriptor = Mock(data=definition)
+        module = Mock(scope_ids=Mock(usage_id='dummy-usage-id'))
         instance_state = {'task_states': task_state, 'graded': True}
         if task_number is not None:
             instance_state.update({'current_task_number': task_number})
@@ -659,6 +664,7 @@ class CombinedOpenEndedModuleTest(unittest.TestCase):
                                                static_data=self.static_data,
                                                metadata=self.metadata,
                                                instance_state=instance_state)
+        self.test_system.xmodule_instance = module
         return combinedoe
 
     def ai_state_reset(self, task_state, task_number=None):
@@ -764,8 +770,9 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
         module.save()
 
         # Mock a student submitting an assessment
-        assessment_dict = MockQueryDict()
-        assessment_dict.update({'assessment': sum(assessment), 'score_list[]': assessment})
+        assessment_dict = MultiDict({'assessment': sum(assessment)})
+        assessment_dict.extend(('score_list[]', val) for val in assessment)
+
         module.handle_ajax("save_assessment", assessment_dict)
         module.save()
         task_one_json = json.loads(module.task_states[0])
@@ -807,8 +814,9 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
         self.assertIsInstance(status, basestring)
 
         # Mock a student submitting an assessment
-        assessment_dict = MockQueryDict()
-        assessment_dict.update({'assessment': sum(assessment), 'score_list[]': assessment})
+        assessment_dict = MultiDict({'assessment': sum(assessment)})
+        assessment_dict.extend(('score_list[]', val) for val in assessment)
+
         module.handle_ajax("save_assessment", assessment_dict)
         module.save()
         task_one_json = json.loads(module.task_states[0])
@@ -905,8 +913,9 @@ class OpenEndedModuleXmlAttemptTest(unittest.TestCase, DummyModulestore):
         module.save()
 
         # Mock a student submitting an assessment
-        assessment_dict = MockQueryDict()
-        assessment_dict.update({'assessment': sum(assessment), 'score_list[]': assessment})
+        assessment_dict = MultiDict({'assessment': sum(assessment)})
+        assessment_dict.extend(('score_list[]', val) for val in assessment)
+
         module.handle_ajax("save_assessment", assessment_dict)
         module.save()
         task_one_json = json.loads(module.task_states[0])

common/lib/xmodule/xmodule/tests/test_conditional.py

@@ -225,7 +225,8 @@ class ConditionalModuleXmlTest(unittest.TestCase):
         html_expect = module.xmodule_runtime.render_template(
             'conditional_ajax.html',
             {
-                'ajax_url': 'courses/course_id/modx/a_location',
+                # Test ajax url is just usage-id / handler_name
+                'ajax_url': 'i4x://HarvardX/ER22x/conditional/condone/xmodule_handler',
                 'element_id': 'i4x-HarvardX-ER22x-conditional-condone',
                 'id': 'i4x://HarvardX/ER22x/conditional/condone',
                 'depends': 'i4x-HarvardX-ER22x-problem-choiceprob'

common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py

@@ -143,6 +143,7 @@ class CHModuleFactory(object):
             return capa_module
         system.get_module = fake_get_module
         module = CrowdsourceHinterModule(descriptor, system, DictFieldData(field_data), Mock())
+        system.xmodule_instance = module
 
         return module
 

common/lib/xmodule/xmodule/tests/test_peer_grading.py

@@ -2,13 +2,14 @@ import unittest
 import json
 import logging
 from mock import Mock
+from webob.multidict import MultiDict
 
 from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
 
 from xmodule.modulestore import Location
 from xmodule.tests import get_test_system, get_test_descriptor_system
-from xmodule.tests.test_util_open_ended import MockQueryDict, DummyModulestore
+from xmodule.tests.test_util_open_ended import DummyModulestore
 from xmodule.open_ended_grading_classes.peer_grading_service import MockPeerGradingService
 from xmodule.peer_grading_module import PeerGradingModule, PeerGradingDescriptor
 from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
@@ -29,17 +30,16 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
     coe_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion"])
     calibrated_dict = {'location': "blah"}
     coe_dict = {'location': coe_location.url()}
-    save_dict = MockQueryDict()
-    save_dict.update({
+    save_dict = MultiDict({
         'location': "blah",
         'submission_id': 1,
         'submission_key': "",
         'score': 1,
         'feedback': "",
-        'rubric_scores[]': [0, 1],
         'submission_flagged': False,
         'answer_unknown': False,
     })
+    save_dict.extend(('rubric_scores[]', val) for val in (0, 1))
 
     def setUp(self):
         """
@@ -277,6 +277,7 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
             self.field_data,
             self.scope_ids,
         )
+        self.test_system.xmodule_instance = peer_grading
 
         return peer_grading
 

common/lib/xmodule/xmodule/tests/test_self_assessment.py

 import json
-from mock import Mock, MagicMock
 import unittest
+from mock import Mock, MagicMock
+from webob.multidict import MultiDict
 
 from xmodule.open_ended_grading_classes.self_assessment_module import SelfAssessmentModule
 from xmodule.modulestore import Location
-from xmodule.tests.test_util_open_ended import MockQueryDict
 from lxml import etree
 
 from . import get_test_system
@@ -21,7 +21,8 @@ class SelfAssessmentTest(unittest.TestCase):
          </rubric></rubric>'''
 
     prompt = etree.XML("<prompt>This is sample prompt text.</prompt>")
-    definition = {'rubric': rubric,
+    definition = {
+        'rubric': rubric,
         'prompt': prompt,
         'submitmessage': 'Shall we submit now?',
         'hintprompt': 'Consider this...',
@@ -33,12 +34,6 @@ class SelfAssessmentTest(unittest.TestCase):
     descriptor = Mock()
 
     def setUp(self):
-        state = json.dumps({'student_answers': ["Answer 1", "answer 2", "answer 3"],
-                            'scores': [0, 1],
-                            'hints': ['o hai'],
-                            'state': SelfAssessmentModule.INITIAL,
-                            'attempts': 2})
-
         self.static_data = {
             'max_attempts': 10,
             'rubric': etree.XML(self.rubric),
@@ -59,10 +54,15 @@ class SelfAssessmentTest(unittest.TestCase):
             }
         }
 
-        self.module = SelfAssessmentModule(get_test_system(), self.location,
+        system = get_test_system()
+        system.xmodule_instance = Mock(scope_ids=Mock(usage_id='dummy-usage-id'))
+        self.module = SelfAssessmentModule(
+            system,
+            self.location,
             self.definition,
             self.descriptor,
-                                           self.static_data)
+            self.static_data
+        )
 
     def test_get_html(self):
         html = self.module.get_html(self.module.system)
@@ -83,7 +83,7 @@ class SelfAssessmentTest(unittest.TestCase):
 
         mock_query_dict = MagicMock()
         mock_query_dict.__getitem__.side_effect = get_fake_item
-        mock_query_dict.getlist = get_fake_item
+        mock_query_dict.getall = get_fake_item
 
         self.module.peer_gs.get_data_for_location = get_data_for_location
 
@@ -140,8 +140,7 @@ class SelfAssessmentTest(unittest.TestCase):
         self.assertEqual(test_module.latest_answer(), submitted_response)
 
         # Mock saving an assessment.
-        assessment = [0]
-        assessment_dict = MockQueryDict({'assessment': sum(assessment), 'score_list[]': assessment})
+        assessment_dict = MultiDict({'assessment': 0, 'score_list[]': 0})
         data = test_module.handle_ajax("save_assessment", assessment_dict, get_test_system())
         self.assertTrue(json.loads(data)['success'])
 

common/lib/xmodule/xmodule/tests/test_util_open_ended.py

@@ -74,20 +74,6 @@ class MockUploadedFile(object):
         return self.mock_file.read()
 
 
-class MockQueryDict(dict):
-    """
-    Mock a query dict so that it can be used in test classes.  This will only work with the combinedopenended tests,
-    and does not mock the full query dict, only the behavior that is needed there (namely get_list).
-    """
-    def getlist(self, key, default=None):
-        try:
-            return super(MockQueryDict, self).__getitem__(key)
-        except KeyError:
-            if default is None:
-                return []
-        return default
-
-
 class DummyModulestore(object):
     """
     A mixin that allows test classes to have convenience functions to get a module given a location

common/lib/xmodule/xmodule/tests/test_word_cloud.py

 # -*- coding: utf-8 -*-
 """Test for Word cloud Xmodule functional logic."""
 
+from webob.multidict import MultiDict
 from xmodule.word_cloud_module import WordCloudDescriptor
-from . import PostData, LogicTest
+from . import LogicTest
 
 
 class WordCloudModuleTest(LogicTest):
@@ -24,7 +25,7 @@ class WordCloudModuleTest(LogicTest):
 
     def test_good_ajax_request(self):
         "Make shure that ajax request works correctly"
-        post_data = PostData({'student_words[]': ['cat', 'cat', 'dog', 'sun']})
+        post_data = MultiDict(('student_words[]', word) for word in ['cat', 'cat', 'dog', 'sun'])
         response = self.ajax_request('submit', post_data)
         self.assertEqual(response['status'], 'success')
         self.assertEqual(response['submitted'], True)

common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py

@@ -5,7 +5,8 @@ functionality
 # For tests, ignore access to protected members
 # pylint: disable=protected-access
 
-from nose.tools import assert_equal  # pylint: disable=E0611
+import webob
+from nose.tools import assert_equal, assert_is_instance  # pylint: disable=E0611
 from unittest.case import SkipTest
 from mock import Mock
 
@@ -32,7 +33,7 @@ from xmodule.conditional_module import ConditionalDescriptor
 from xmodule.randomize_module import RandomizeDescriptor
 from xmodule.vertical_module import VerticalDescriptor
 from xmodule.wrapper_module import WrapperDescriptor
-from xmodule.tests import get_test_descriptor_system, mock_render_template
+from xmodule.tests import get_test_descriptor_system, get_test_system
 
 LEAF_XMODULES = (
     AnnotatableDescriptor,
@@ -66,24 +67,12 @@ NOT_STUDIO_EDITABLE = (
     PollDescriptor
 )
 
-
 class TestXBlockWrapper(object):
     """Helper methods used in test case classes below."""
 
     @property
     def leaf_module_runtime(self):
-        runtime = ModuleSystem(
-            render_template=mock_render_template,
-            anonymous_student_id='dummy_anonymous_student_id',
-            open_ended_grading_interface={},
-            static_url='/static',
-            ajax_url='dummy_ajax_url',
-            get_module=Mock(),
-            replace_urls=Mock(),
-            track_function=Mock(),
-            error_descriptor_class=ErrorDescriptor,
-        )
-        return runtime
+        return get_test_system()
 
     def leaf_descriptor(self, descriptor_cls):
         location = 'i4x://org/course/category/name'
@@ -258,3 +247,27 @@ class TestStudioView(TestXBlockWrapper):
             raise SkipTest(descriptor_cls.__name__ + "is not editable in studio")
 
         raise SkipTest("XBlock support in XModules not yet fully implemented")
+
+
+class TestXModuleHandler(TestXBlockWrapper):
+    """
+    Tests that the xmodule_handler function correctly wraps handle_ajax
+    """
+
+    def setUp(self):
+        self.module = XModule(descriptor=Mock(), field_data=Mock(), runtime=Mock(), scope_ids=Mock())
+        self.module.handle_ajax = Mock(return_value='{}')
+        self.request = Mock()
+
+    def test_xmodule_handler_passed_data(self):
+        self.module.xmodule_handler(self.request)
+        self.module.handle_ajax.assert_called_with(None, self.request.POST)
+
+    def test_xmodule_handler_dispatch(self):
+        self.module.xmodule_handler(self.request, 'dispatch')
+        self.module.handle_ajax.assert_called_with('dispatch', self.request.POST)
+
+    def test_xmodule_handler_return_value(self):
+        response = self.module.xmodule_handler(self.request)
+        assert_is_instance(response, webob.Response)
+        assert_equal(response.body, '{}')

common/lib/xmodule/xmodule/word_cloud_module.py

@@ -193,7 +193,7 @@ class WordCloudModule(WordCloudFields, XModule):
 
             # Student words from client.
             # FIXME: we must use raw JSON, not a post data (multipart/form-data)
-            raw_student_words = data.getlist('student_words[]')
+            raw_student_words = data.getall('student_words[]')
             student_words = filter(None, map(self.good_word, raw_student_words))
 
             self.student_words = student_words

common/lib/xmodule/xmodule/x_module.py

@@ -7,6 +7,7 @@ from functools import partial
 from lxml import etree
 from collections import namedtuple
 from pkg_resources import resource_listdir, resource_string, resource_isdir
+from webob import Response
 
 from xmodule.modulestore import Location
 from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError, InvalidLocationError
@@ -115,7 +116,6 @@ class XModuleMixin(XBlockMixin):
     # student interacts with the module on the page.  A specific example is
     # FoldIt, which posts grade-changing updates through a separate API.
     always_recalculate_grades = False
-
     # The default implementation of get_icon_class returns the icon_class
     # attribute of the class
     #
@@ -273,8 +273,7 @@ class XModuleMixin(XBlockMixin):
 
           NOTE (vshnayder): not sure if this was the intended return value, but
           that's what it's doing now.  I suspect that we really want it to just
-          return a number.  Would need to change (at least) capa and
-          modx_dispatch to match if we did that.
+          return a number.  Would need to change (at least) capa to match if we did that.
         """
         return None
 
@@ -402,6 +401,13 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock):  # pylint: disable=abstract-me
             data is a dictionary-like object with the content of the request"""
         return u""
 
+    def xmodule_handler(self, request, suffix=None):
+        """
+        XBlock handler that wraps `handle_ajax`
+        """
+        response_data = self.handle_ajax(suffix, request.POST)
+        return Response(response_data, content_type='application/json')
+
     def get_children(self):
         """
         Return module instances for all the children of this module.
@@ -762,6 +768,7 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
     max_score = module_attr('max_score')
     student_view = module_attr('student_view')
     get_child_descriptors = module_attr('get_child_descriptors')
+    xmodule_handler = module_attr('xmodule_handler')
 
     # ~~~~~~~~~~~~~~~ XBlock API Wrappers ~~~~~~~~~~~~~~~~
     def studio_view(self, _context):
@@ -924,7 +931,7 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable=abs
     and user, or other environment-specific info.
     """
     def __init__(
-            self, static_url, ajax_url, track_function, get_module, render_template,
+            self, static_url, track_function, get_module, render_template,
             replace_urls, user=None, filestore=None,
             debug=False, hostname="", xqueue=None, publish=None, node_path="",
             anonymous_student_id='', course_id=None,
@@ -936,8 +943,6 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable=abs
 
         static_url - the base URL to static assets
 
-        ajax_url - the url where ajax calls to the encapsulating module go.
-
         track_function - function of (event_type, event), intended for logging
                          or otherwise tracking the event.
                          TODO: Not used, and has inconsistent args in different
@@ -988,7 +993,6 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable=abs
         super(ModuleSystem, self).__init__(usage_store=None, field_data=None, **kwargs)
 
         self.STATIC_URL = static_url
-        self.ajax_url = ajax_url
         self.xqueue = xqueue
         self.track_function = track_function
         self.filestore = filestore
@@ -1032,6 +1036,13 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable=abs
     def __str__(self):
         return str(self.__dict__)
 
+    @property
+    def ajax_url(self):
+        """
+        The url prefix to be used by XModules to call into handle_ajax
+        """
+        return self.handler_url(self.xmodule_instance, 'xmodule_handler', '', '').rstrip('/?')
+
 
 class DoNothingCache(object):
     """A duck-compatible object to use in ModuleSystem when there's no cache."""

common/static/coffee/spec/xblock/runtime.v1_spec.coffee

 describe "XBlock.runtime.v1", ->
   beforeEach ->
     setFixtures """
-      <div class='xblock' data-usage-id='fake-usage-id'/>
+      <div class='xblock' data-handler-prefix='/xblock/fake-usage-id/handler'/>
     """
     @children = [
       {name: 'childA'},
@@ -12,7 +12,7 @@ describe "XBlock.runtime.v1", ->
 
     @runtime = XBlock.runtime.v1(@element, @children)
   it "provides a handler url", ->
-    expect(@runtime.handlerUrl('foo')).toBe('/xblock/handler/fake-usage-id/foo')
+    expect(@runtime.handlerUrl('foo')).toBe('/xblock/fake-usage-id/handler/foo')
 
   it "provides a list of children", ->
     expect(@runtime.children).toBe(@children)

common/static/coffee/src/xblock/runtime.v1.coffee

@@ -5,8 +5,8 @@
 
   return {
     handlerUrl: (handlerName) ->
-      usageId = $(element).data("usage-id")
-      "/xblock/handler/#{usageId}/#{handlerName}"
+      handlerPrefix = $(element).data("handler-prefix")
+      "#{handlerPrefix}/#{handlerName}"
     children: children
     childMap: childMap
   }

docs/internal/overview.md

@@ -99,7 +99,7 @@ The LMS is a django site, with root in `lms/`.  It runs in many different enviro
 
   - calls the module's `.get_html()` method.  If the module has nested submodules, render_x_module() will be called again for each.
 
-  - ajax calls go to `module_render.py:modx_dispatch()`, which passes it to the module's `handle_ajax()` function, and then updates the grade and state if they changed.
+  - ajax calls go to `module_render.py:handle_xblock_callback()`, which passes it to one of the `XBlock`s handler functions
 
   - [This diagram](https://github.com/MITx/mitx/wiki/MITx-Architecture) visually shows how the clients communicate with problems + modules.
 

lms/djangoapps/courseware/module_render.py

@@ -21,20 +21,21 @@ from courseware.access import has_access
 from courseware.masquerade import setup_masquerade
 from courseware.model_data import FieldDataCache, DjangoKeyValueStore
 from lms.lib.xblock.field_data import LmsFieldData
+from lms.lib.xblock.runtime import LmsModuleSystem, handler_prefix, unquote_slashes
 from mitxmako.shortcuts import render_to_string
 from psychometrics.psychoanalyze import make_psychometrics_data_update_handler
 from student.models import unique_id_for_user
 from util.json_request import JsonResponse
 from util.sandboxing import can_execute_unsafe_code
 from xblock.fields import Scope
-from xblock.runtime import DbModel
-from xblock.runtime import KeyValueStore
+from xblock.runtime import DbModel, KeyValueStore
+from xblock.exceptions import NoSuchHandlerError
+from xblock.django.request import django_to_webob_request, webob_to_django_response
 from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor
 from xmodule.exceptions import NotFoundError, ProcessingError
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
-from xmodule.x_module import ModuleSystem
 from xmodule_modifiers import replace_course_urls, replace_jump_to_id_urls, replace_static_urls, add_histogram, wrap_xblock
 
 
@@ -220,17 +221,6 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
     student_data = DbModel(DjangoKeyValueStore(field_data_cache))
     descriptor._field_data = LmsFieldData(descriptor._field_data, student_data)
 
-    # Setup system context for module instance
-    ajax_url = reverse(
-        'modx_dispatch',
-        kwargs=dict(
-            course_id=course_id,
-            location=descriptor.location.url(),
-            dispatch=''
-        ),
-    )
-    # Intended use is as {ajax_url}/{dispatch_command}, so get rid of the trailing slash.
-    ajax_url = ajax_url.rstrip('/')
 
     def make_xqueue_callback(dispatch='score_update'):
         # Fully qualified callback URL for external queueing system
@@ -338,7 +328,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
     # Wrap the output display in a single div to allow for the XModule
     # javascript to be bound correctly
     if wrap_xmodule_display is True:
-        block_wrappers.append(wrap_xblock)
+        block_wrappers.append(partial(wrap_xblock, partial(handler_prefix, course_id)))
 
     # TODO (cpennington): When modules are shared between courses, the static
     # prefix is going to have to be specific to the module, not the directory
@@ -371,11 +361,10 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
         if has_access(user, descriptor, 'staff', course_id):
             block_wrappers.append(partial(add_histogram, user))
 
-    system = ModuleSystem(
+    system = LmsModuleSystem(
         track_function=track_function,
         render_template=render_to_string,
         static_url=settings.STATIC_URL,
-        ajax_url=ajax_url,
         xqueue=xqueue,
         # TODO (cpennington): Figure out how to share info between systems
         filestore=descriptor.runtime.resources_fs,
@@ -493,15 +482,13 @@ def xqueue_callback(request, course_id, userid, mod_id, dispatch):
     return HttpResponse("")
 
 
-def modx_dispatch(request, dispatch, location, course_id):
-    ''' Generic view for extensions. This is where AJAX calls go.
+def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None):
+    """
+    Generic view for extensions. This is where AJAX calls go.
 
     Arguments:
 
       - request -- the django request.
-      - dispatch -- the command string to pass through to the module's handle_ajax call
-           (e.g. 'problem_reset').  If this string contains '?', only pass
-           through the part before the first '?'.
       - location -- the module location. Used to look up the XModule instance
       - course_id -- defines the course context for this request.
 
@@ -509,8 +496,8 @@ def modx_dispatch(request, dispatch, location, course_id):
     the location and course_id do not identify a valid module, the module is
     not accessible by the user, or the module raises NotFoundError. If the
     module raises any other error, it will escape this function.
-    '''
-    # ''' (fix emacs broken parsing)
+    """
+    location = unquote_slashes(usage_id)
 
     # Check parameters and fail fast if there's a problem
     if not Location.is_valid(location):
@@ -519,16 +506,11 @@ def modx_dispatch(request, dispatch, location, course_id):
     if not request.user.is_authenticated():
         raise PermissionDenied
 
-    # Get the submitted data
-    data = request.POST.copy()
-
-    # Get and check submitted files
+    # Check submitted files
     files = request.FILES or {}
     error_msg = _check_files_limits(files)
     if error_msg:
         return HttpResponse(json.dumps({'success': error_msg}))
-    for key in files:  # Merge files into to data dictionary
-        data[key] = files.getlist(key)
 
     try:
         descriptor = modulestore().get_instance(course_id, location)
@@ -551,14 +533,16 @@ def modx_dispatch(request, dispatch, location, course_id):
     if instance is None:
         # Either permissions just changed, or someone is trying to be clever
         # and load something they shouldn't have access to.
-        log.debug("No module {0} for user {1}--access denied?".format(location, request.user))
+        log.debug("No module %s for user %s -- access denied?", location, request.user)
         raise Http404
 
-    # Let the module handle the AJAX
+    req = django_to_webob_request(request)
     try:
-        ajax_return = instance.handle_ajax(dispatch, data)
-        # Save any fields that have changed to the underlying KeyValueStore
-        instance.save()
+        resp = instance.handle(handler, req, suffix)
+
+    except NoSuchHandlerError:
+        log.exception("XBlock %s attempted to access missing handler %r", instance, handler)
+        raise Http404
 
     # If we can't find the module, respond with a 404
     except NotFoundError:
@@ -572,12 +556,11 @@ def modx_dispatch(request, dispatch, location, course_id):
         return JsonResponse(object={'success': err.args[0]}, status=200)
 
     # If any other error occurred, re-raise it to trigger a 500 response
-    except:
-        log.exception("error processing ajax call")
+    except Exception:
+        log.exception("error executing xblock handler")
         raise
 
-    # Return whatever the module wanted to return to the client/caller
-    return HttpResponse(ajax_return)
+    return webob_to_django_response(resp)
 
 
 def get_score_bucket(grade, max_grade):

lms/djangoapps/courseware/tests/__init__.py

@@ -22,6 +22,7 @@ from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from lms.lib.xblock.field_data import LmsFieldData
+from lms.lib.xblock.runtime import quote_slashes
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -127,8 +128,8 @@ class BaseTestXmodule(ModuleStoreTestCase):
     def get_url(self, dispatch):
         """Return item url with dispatch."""
         return reverse(
-            'modx_dispatch',
-            args=(self.course.id, self.item_url, dispatch)
+            'xblock_handler',
+            args=(self.course.id, quote_slashes(self.item_url), 'xmodule_handler', dispatch)
         )
 
 

lms/djangoapps/courseware/tests/test_masquerade.py

@@ -19,6 +19,7 @@ from courseware.tests.helpers import LoginEnrollmentTestCase
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.django import modulestore, clear_existing_modulestores
+from lms.lib.xblock.runtime import quote_slashes
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -91,10 +92,11 @@ class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
         pun = 'H1P1'
         problem_location = "i4x://edX/graded/problem/%s" % pun
 
-        modx_url = reverse('modx_dispatch',
+        modx_url = reverse('xblock_handler',
                            kwargs={'course_id': self.graded_course.id,
-                                   'location': problem_location,
-                                   'dispatch': 'problem_get', })
+                                   'usage_id': quote_slashes(problem_location),
+                                   'handler': 'xmodule_handler',
+                                   'suffix': 'problem_get'})
 
         resp = self.client.get(modx_url)
 
@@ -115,6 +117,5 @@ class TestStaffMasqueradeAsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
 
         resp = self.get_problem()
         html = json.loads(resp.content)['html']
-        print html
         sabut = '<button class="show"><span class="show-label">Show Answer(s)</span> <span class="sr">(for question(s) above - adjacent to each field)</span></button>'
         self.assertFalse(sabut in html)

lms/djangoapps/courseware/tests/test_submitting_problems.py

@@ -21,6 +21,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from capa.tests.response_xml_factory import OptionResponseXMLFactory, CustomResponseXMLFactory, SchematicResponseXMLFactory
 from courseware.tests.helpers import LoginEnrollmentTestCase
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
+from lms.lib.xblock.runtime import quote_slashes
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -71,11 +72,12 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase):
             example: 'check_problem' for having responses processed
         """
         return reverse(
-            'modx_dispatch',
+            'xblock_handler',
             kwargs={
                 'course_id': self.course.id,
-                'location': problem_location,
-                'dispatch': dispatch,
+                'usage_id': quote_slashes(problem_location),
+                'handler': 'xmodule_handler',
+                'suffix': dispatch,
             }
         )
 

lms/djangoapps/instructor/views/instructor_dashboard.py

 """
 Instructor Dashboard Views
 """
+from functools import partial
 
 from django.utils.translation import ugettext as _
 from django_future.csrf import ensure_csrf_cookie
@@ -23,6 +24,7 @@ from django_comment_client.utils import has_forum_access
 from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR
 from student.models import CourseEnrollment
 from bulk_email.models import CourseAuthorization
+from lms.lib.xblock.runtime import handler_prefix
 
 @ensure_csrf_cookie
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
@@ -174,9 +176,13 @@ def _section_data_download(course_id, access):
 
 def _section_send_email(course_id, access, course):
     """ Provide data for the corresponding bulk email section """
-    html_module = HtmlDescriptor(course.system, DictFieldData({'data': ''}), ScopeIds(None, None, None, None))
+    html_module = HtmlDescriptor(
+        course.system,
+        DictFieldData({'data': ''}),
+        ScopeIds(None, None, None, 'i4x://dummy_org/dummy_course/html/dummy_name')
+    )
     fragment = course.system.render(html_module, 'studio_view')
-    fragment = wrap_xblock(html_module, 'studio_view', fragment, None)
+    fragment = wrap_xblock(partial(handler_prefix, course_id), html_module, 'studio_view', fragment, None)
     email_editor = fragment.content
     section_data = {
         'section_key': 'send_email',

lms/djangoapps/instructor/views/legacy.py

@@ -9,6 +9,7 @@ import re
 import requests
 
 from collections import defaultdict, OrderedDict
+from functools import partial
 from markupsafe import escape
 from requests.status_codes import codes
 from StringIO import StringIO
@@ -58,6 +59,7 @@ from mitxmako.shortcuts import render_to_string
 from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
 from django.utils.translation import ugettext as _u
+from lms.lib.xblock.runtime import handler_prefix
 
 log = logging.getLogger(__name__)
 
@@ -830,9 +832,13 @@ def instructor_dashboard(request, course_id):
     email_editor = None
     # HTML editor for email
     if idash_mode == 'Email' and is_studio_course:
-        html_module = HtmlDescriptor(course.system, DictFieldData({'data': html_message}), ScopeIds(None, None, None, None))
+        html_module = HtmlDescriptor(
+            course.system,
+            DictFieldData({'data': html_message}),
+            ScopeIds(None, None, None, 'i4x://dummy_org/dummy_course/html/dummy_name')
+        )
         fragment = html_module.render('studio_view')
-        fragment = wrap_xblock(html_module, 'studio_view', fragment, None)
+        fragment = wrap_xblock(partial(handler_prefix, course_id), html_module, 'studio_view', fragment, None)
         email_editor = fragment.content
 
     # Enable instructor email only if the following conditions are met:

lms/djangoapps/instructor_task/tests/test_integration.py

@@ -28,6 +28,7 @@ from instructor_task.models import InstructorTask
 from instructor_task.tests.test_base import (InstructorTaskModuleTestCase, TEST_COURSE_ORG, TEST_COURSE_NUMBER,
                                              OPTION_1, OPTION_2)
 from capa.responsetypes import StudentInputError
+from lms.lib.xblock.runtime import quote_slashes
 
 
 log = logging.getLogger(__name__)
@@ -57,10 +58,11 @@ class TestIntegrationTask(InstructorTaskModuleTestCase):
         # on the right problem:
         self.login_username(username)
         # make ajax call:
-        modx_url = reverse('modx_dispatch',
+        modx_url = reverse('xblock_handler',
                            kwargs={'course_id': self.course.id,
-                                   'location': InstructorTaskModuleTestCase.problem_location(problem_url_name),
-                                   'dispatch': 'problem_check', })
+                                   'usage_id': quote_slashes(InstructorTaskModuleTestCase.problem_location(problem_url_name)),
+                                   'handler': 'xmodule_handler',
+                                   'suffix': 'problem_check', })
 
         # we assume we have two responses, so assign them the correct identifiers.
         resp = self.client.post(modx_url, {
@@ -110,10 +112,11 @@ class TestRescoringTask(TestIntegrationTask):
         # on the right problem:
         self.login_username(username)
         # make ajax call:
-        modx_url = reverse('modx_dispatch',
+        modx_url = reverse('xblock_handler',
                            kwargs={'course_id': self.course.id,
-                                   'location': InstructorTaskModuleTestCase.problem_location(problem_url_name),
-                                   'dispatch': 'problem_get', })
+                                   'usage_id': quote_slashes(InstructorTaskModuleTestCase.problem_location(problem_url_name)),
+                                   'handler': 'xmodule_handler',
+                                   'suffix': 'problem_get', })
         resp = self.client.post(modx_url, {})
         return resp
 

lms/djangoapps/open_ended_grading/open_ended_notifications.py

@@ -8,7 +8,7 @@ from xmodule.open_ended_grading_classes import peer_grading_service
 from xmodule.open_ended_grading_classes.controller_query_service import ControllerQueryService
 
 from courseware.access import has_access
-from xmodule.x_module import ModuleSystem
+from lms.lib.xblock.runtime import LmsModuleSystem
 from mitxmako.shortcuts import render_to_string
 from student.models import unique_id_for_user
 from util.cache import cache
@@ -64,8 +64,7 @@ def staff_grading_notifications(course, user):
 
 
 def peer_grading_notifications(course, user):
-    system = ModuleSystem(
-        ajax_url=None,
+    system = LmsModuleSystem(
         track_function=None,
         get_module = None,
         render_template=render_to_string,
@@ -124,9 +123,8 @@ def combined_notifications(course, user):
         return notification_dict
 
     #Define a mock modulesystem
-    system = ModuleSystem(
+    system = LmsModuleSystem(
         static_url="/static",
-        ajax_url=None,
         track_function=None,
         get_module = None,
         render_template=render_to_string,

lms/djangoapps/open_ended_grading/staff_grading_service.py

@@ -9,10 +9,10 @@ from django.conf import settings
 from django.http import HttpResponse, Http404
 
 from xmodule.course_module import CourseDescriptor
-from xmodule.x_module import ModuleSystem
 from xmodule.open_ended_grading_classes.grading_service_module import GradingService, GradingServiceError
 
 from courseware.access import has_access
+from lms.lib.xblock.runtime import LmsModuleSystem
 from mitxmako.shortcuts import render_to_string
 from student.models import unique_id_for_user
 from util.json_request import expect_json
@@ -69,9 +69,8 @@ class StaffGradingService(GradingService):
     """
 
     def __init__(self, config):
-        config['system'] = ModuleSystem(
+        config['system'] = LmsModuleSystem(
             static_url='/static',
-            ajax_url=None,
             track_function=None,
             get_module = None,
             render_template=render_to_string,

lms/djangoapps/open_ended_grading/tests.py

@@ -21,12 +21,12 @@ from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.open_ended_grading_classes import peer_grading_service, controller_query_service
 from xmodule.tests import test_util_open_ended
-from xmodule.x_module import ModuleSystem
 
 from courseware.access import _course_staff_group_name
 from courseware.tests import factories
 from courseware.tests.helpers import LoginEnrollmentTestCase, check_for_get_code, check_for_post_code
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
+from lms.lib.xblock.runtime import LmsModuleSystem
 from mitxmako.shortcuts import render_to_string
 from student.models import unique_id_for_user
 
@@ -243,9 +243,8 @@ class TestPeerGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
         location = "i4x://edX/toy/peergrading/init"
         field_data = DictFieldData({'data': "<peergrading/>", 'location': location, 'category':'peergrading'})
         self.mock_service = peer_grading_service.MockPeerGradingService()
-        self.system = ModuleSystem(
+        self.system = LmsModuleSystem(
             static_url=settings.STATIC_URL,
-            ajax_url=location,
             track_function=None,
             get_module=None,
             render_template=render_to_string,
@@ -400,7 +399,7 @@ class TestPanel(ModuleStoreTestCase):
         Mock(
             return_value=controller_query_service.MockControllerQueryService(
                 settings.OPEN_ENDED_GRADING_INTERFACE,
-                utils.system
+                utils.SYSTEM
             )
         )
     )

lms/djangoapps/open_ended_grading/utils.py

@@ -6,11 +6,11 @@ from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
 from xmodule.open_ended_grading_classes.controller_query_service import ControllerQueryService
 from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError
-from xmodule.x_module import ModuleSystem
 
 from django.utils.translation import ugettext as _
 from django.conf import settings
 
+from lms.lib.xblock.runtime import LmsModuleSystem
 from mitxmako.shortcuts import render_to_string
 
 
@@ -27,9 +27,8 @@ GRADER_DISPLAY_NAMES = {
 STUDENT_ERROR_MESSAGE = _("Error occurred while contacting the grading service.  Please notify course staff.")
 STAFF_ERROR_MESSAGE = _("Error occurred while contacting the grading service.  Please notify your edX point of contact.")
 
-system = ModuleSystem(
+SYSTEM = LmsModuleSystem(
     static_url='/static',
-    ajax_url=None,
     track_function=None,
     get_module=None,
     render_template=render_to_string,
@@ -79,8 +78,7 @@ def create_controller_query_service():
     """
     Return an instance of a service that can query edX ORA.
     """
-
-    return ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, system)
+    return ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, SYSTEM)
 
 
 class StudentProblemList(object):

lms/envs/test.py

@@ -223,7 +223,7 @@ for static_dir in STATICFILES_DIRS:
         new_staticfiles_dirs.append(static_dir)
 STATICFILES_DIRS = new_staticfiles_dirs
 
-FILE_UPLOAD_TEMP_DIR = PROJECT_ROOT / "uploads"
+FILE_UPLOAD_TEMP_DIR = TEST_ROOT / "uploads"
 FILE_UPLOAD_HANDLERS = (
     'django.core.files.uploadhandler.MemoryFileUploadHandler',
     'django.core.files.uploadhandler.TemporaryFileUploadHandler',

lms/lib/xblock/runtime.py

+"""
+Module implementing `xblock.runtime.Runtime` functionality for the LMS
+"""
+
+import re
+
+from django.core.urlresolvers import reverse
+
+from xmodule.x_module import ModuleSystem
+
+
+def _quote_slashes(match):
+    """
+    Helper function for `quote_slashes`
+    """
+    matched = match.group(0)
+    # We have to escape ';', because that is our
+    # escape sequence identifier (otherwise, the escaping)
+    # couldn't distinguish between us adding ';_' to the string
+    # and ';_' appearing naturally in the string
+    if matched == ';':
+        return ';;'
+    elif matched == '/':
+        return ';_'
+    else:
+        return matched
+
+
+def quote_slashes(text):
+    """
+    Quote '/' characters so that they aren't visible to
+    django's url quoting, unquoting, or url regex matching.
+
+    Escapes '/'' to the sequence ';_', and ';' to the sequence
+    ';;'. By making the escape sequence fixed length, and escaping
+    identifier character ';', we are able to reverse the escaping.
+    """
+    return re.sub(r'[;/]', _quote_slashes, text)
+
+
+def _unquote_slashes(match):
+    """
+    Helper function for `unquote_slashes`
+    """
+    matched = match.group(0)
+    if matched == ';;':
+        return ';'
+    elif matched == ';_':
+        return '/'
+    else:
+        return matched
+
+
+def unquote_slashes(text):
+    """
+    Unquote slashes quoted by `quote_slashes`
+    """
+    return re.sub(r'(;;|;_)', _unquote_slashes, text)
+
+
+def handler_url(course_id, block, handler, suffix='', query=''):
+    """
+    Return an xblock handler url for the specified course, block and handler
+    """
+    return reverse('xblock_handler', kwargs={
+        'course_id': course_id,
+        'usage_id': quote_slashes(str(block.scope_ids.usage_id)),
+        'handler': handler,
+        'suffix': suffix,
+    }) + '?' + query
+
+
+def handler_prefix(course_id, block):
+    """
+    Returns a prefix for use by the javascript handler_url function.
+    The prefix is a valid handler url the handler name is appended to it.
+    """
+    return handler_url(course_id, block, '').rstrip('/')
+
+
+class LmsHandlerUrls(object):
+    """
+    A runtime mixin that provides a handler_url function that routes
+    to the LMS' xblock handler view.
+
+    This must be mixed in to a runtime that already accepts and stores
+    a course_id
+    """
+
+    def handler_url(self, block, handler_name, suffix='', query=''):  # pylint: disable=unused-argument
+        """See :method:`xblock.runtime:Runtime.handler_url`"""
+        return handler_url(self.course_id, block, handler_name, suffix='', query='')  # pylint: disable=no-member
+
+
+class LmsModuleSystem(LmsHandlerUrls, ModuleSystem):  # pylint: disable=abstract-method
+    """
+    ModuleSystem specialized to the LMS
+    """
+    pass

lms/lib/xblock/test/test_runtime.py

+"""
+Tests of the LMS XBlock Runtime and associated utilities
+"""
+
+from ddt import ddt, data
+from unittest import TestCase
+from lms.lib.xblock.runtime import quote_slashes, unquote_slashes
+
+TEST_STRINGS = [
+    '',
+    'foobar',
+    'foo/bar',
+    'foo/bar;',
+    'foo;;bar',
+    'foo;_bar',
+    'foo/',
+    '/bar',
+    'foo//bar',
+    'foo;;;bar',
+]
+
+
+@ddt
+class TestQuoteSlashes(TestCase):
+    """Test the quote_slashes and unquote_slashes functions"""
+
+    @data(*TEST_STRINGS)
+    def test_inverse(self, test_string):
+        self.assertEquals(test_string, unquote_slashes(quote_slashes(test_string)))
+
+    @data(*TEST_STRINGS)
+    def test_escaped(self, test_string):
+        self.assertNotIn('/', quote_slashes(test_string))

lms/urls.py

@@ -175,9 +175,9 @@ if settings.COURSEWARE_ENABLED:
             'courseware.views.jump_to', name="jump_to"),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/jump_to_id/(?P<module_id>.*)$',
             'courseware.views.jump_to_id', name="jump_to_id"),
-        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/modx/(?P<location>.*?)/(?P<dispatch>[^/]*)$',
-            'courseware.module_render.modx_dispatch',
-            name='modx_dispatch'),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/xblock/(?P<usage_id>[^/]*)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$',
+            'courseware.module_render.handle_xblock_callback',
+            name='xblock_handler'),
 
 
         # Software Licenses

rakelib/js_test.rake

@@ -40,7 +40,7 @@ def js_test_tool(env, command, do_coverage)
         cmd += " --coverage-xml #{report_dir}"
     end
 
-    sh(cmd)
+    test_sh(cmd)
 end
 
 # Print a list of js_test commands for

requirements/edx/base.txt

@@ -90,6 +90,7 @@ transifex-client==0.9.1
 
 # Used for testing
 coverage==3.6
+ddt==0.4.0
 factory_boy==2.0.2
 mock==1.0.1
 nosexcover==1.0.7

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@74c1a2e9#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@2daa4e54#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