diff --git a/cms/djangoapps/contentstore/tests/test_item.py b/cms/djangoapps/contentstore/tests/test_item.py index 4922b4888a..62c8e3de04 100644 --- a/cms/djangoapps/contentstore/tests/test_item.py +++ b/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) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 62c020dd39..b20dae9e1d 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/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) diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index ddb4814511..3c561b6047 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/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): diff --git a/cms/envs/common.py b/cms/envs/common.py index 7e1802ac75..d8f187223c 100644 --- a/cms/envs/common.py +++ b/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 diff --git a/cms/lib/__init__.py b/cms/lib/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/lib/xblock/__init__.py b/cms/lib/xblock/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/xmodule_namespace.py b/cms/lib/xblock/mixin.py similarity index 91% rename from cms/xmodule_namespace.py rename to cms/lib/xblock/mixin.py index c028ab3710..e4c9610a2b 100644 --- a/cms/xmodule_namespace.py +++ b/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 diff --git a/cms/lib/xblock/runtime.py b/cms/lib/xblock/runtime.py new file mode 100644 index 0000000000..5c749a34c6 --- /dev/null +++ b/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 diff --git a/cms/lib/xblock/test/test_runtime.py b/cms/lib/xblock/test/test_runtime.py new file mode 100644 index 0000000000..a4fd08f41c --- /dev/null +++ b/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')) diff --git a/cms/urls.py b/cms/urls.py index fa50e7926b..8b6b18ef71 100644 --- a/cms/urls.py +++ b/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.*?)/handler/(?P[^/]*)(?:/(?P[^/]*))?$', + url(r'^preview/xblock/(?P.*?)/handler/(?P[^/]*)(?:/(?P.*))?$', 'contentstore.views.preview_handler', name='preview_handler'), + url(r'^xblock/(?P.*?)/handler/(?P[^/]*)(?:/(?P.*))?$', + 'contentstore.views.component_handler', name='component_handler'), + # temporary landing page for a course url(r'^edge/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', 'contentstore.views.landing', name='landing'), diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index 341bca4d05..39e64e3185 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 6f5577b2b1..ef994bea72 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/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.""" diff --git a/rakelib/tests.rake b/rakelib/tests.rake index fb324c0807..c867a51fa4 100644 --- a/rakelib/tests.rake +++ b/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 diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 0749daebdd..1bc52ce367 100644 --- a/requirements/edx/github.txt +++ b/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