diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 528b2714c9..56b2fe2258 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -24,6 +24,7 @@ from xblock.django.request import webob_to_django_response, django_to_webob_requ from xblock.exceptions import NoSuchHandlerError from xblock.fragment import Fragment from student.auth import has_studio_read_access, has_studio_write_access +from xblock_django.user_service import DjangoXBlockUserService from lms.djangoapps.lms_xblock.field_data import LmsFieldData from cms.lib.xblock.field_data import CmsFieldData @@ -112,20 +113,6 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method ] -class StudioUserService(object): - """ - Provides a Studio implementation of the XBlock user service. - """ - - def __init__(self, request): - super(StudioUserService, self).__init__() - self._request = request - - @property - def user_id(self): - return self._request.user.id - - class StudioPermissionsService(object): """ Service that can provide information about a user's permissions. @@ -176,7 +163,6 @@ def _preview_module_system(request, descriptor, field_data): _studio_wrap_xblock, ] - descriptor.runtime._services['user'] = StudioUserService(request) # pylint: disable=protected-access descriptor.runtime._services['studio_user_permissions'] = StudioPermissionsService(request) # pylint: disable=protected-access return PreviewModuleSystem( @@ -204,6 +190,7 @@ def _preview_module_system(request, descriptor, field_data): "i18n": ModuleI18nService(), "field-data": field_data, "library_tools": LibraryToolsService(modulestore()), + "user": DjangoXBlockUserService(request.user), }, ) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 01a84dea5c..dd8a609968 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -12,7 +12,6 @@ from django.test import TestCase from django.test.client import RequestFactory from django.core.urlresolvers import reverse from contentstore.utils import reverse_usage_url, reverse_course_url -from contentstore.views.preview import StudioUserService from contentstore.views.component import ( component_handler, get_component_templates @@ -30,6 +29,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory, check_mongo_calls from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW from xblock.exceptions import NoSuchHandlerError +from xblock_django.user_service import DjangoXBlockUserService from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.locations import Location from xmodule.partitions.partitions import Group, UserPartition @@ -1170,7 +1170,7 @@ class TestEditSplitModule(ItemTest): # (CachingDescriptorSystem is used in tests, PreviewModuleSystem in Studio). # CachingDescriptorSystem doesn't have user service, that's needed for # SplitTestModule. So, in this line of code we add this service manually. - split_test.runtime._services['user'] = StudioUserService(self.request) # pylint: disable=protected-access + split_test.runtime._services['user'] = DjangoXBlockUserService(self.user) # pylint: disable=protected-access # Call add_missing_groups method to add the missing group. split_test.add_missing_groups(self.request) diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index 6ba40c1a58..aa964cd253 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -2,6 +2,9 @@ Tests for contentstore.views.preview.py """ import re +import ddt +from mock import Mock +from xblock.core import XBlock from django.test import TestCase from django.test.client import RequestFactory @@ -10,8 +13,9 @@ from xblock.core import XBlockAside from student.tests.factories import UserFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from contentstore.views.preview import get_preview_fragment +from contentstore.views.preview import get_preview_fragment, _preview_module_system from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.test_asides import AsideTestType from cms.djangoapps.xblock_config.models import StudioConfig @@ -101,3 +105,44 @@ class GetPreviewHtmlTestCase(TestCase): self.assertNotRegexpMatches(html, r"data-block-type=[\"\']test_aside[\"\']") self.assertNotRegexpMatches(html, "Aside rendered") + + +@XBlock.needs("field-data") +@XBlock.needs("i18n") +@XBlock.needs("user") +class PureXBlock(XBlock): + """ + Pure XBlock to use in tests. + """ + pass + + +@ddt.ddt +class StudioXBlockServiceBindingTest(ModuleStoreTestCase): + """ + Tests that the Studio Module System (XBlock Runtime) provides an expected set of services. + """ + def setUp(self): + """ + Set up the user and request that will be used. + """ + super(StudioXBlockServiceBindingTest, self).setUp() + self.user = UserFactory() + self.course = CourseFactory.create() + self.request = Mock() + self.field_data = Mock() + + @XBlock.register_temp_plugin(PureXBlock, identifier='pure') + @ddt.data("user", "i18n", "field-data") + def test_expected_services_exist(self, expected_service): + """ + Tests that the 'user' and 'i18n' services are provided by the Studio runtime. + """ + descriptor = ItemFactory(category="pure", parent=self.course) + runtime = _preview_module_system( + self.request, + descriptor, + self.field_data, + ) + service = runtime.service(descriptor, expected_service) + self.assertIsNotNone(service) diff --git a/common/djangoapps/xblock_django/__init__.py b/common/djangoapps/xblock_django/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/xblock_django/tests/__init__.py b/common/djangoapps/xblock_django/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/xblock_django/tests/test_user_service.py b/common/djangoapps/xblock_django/tests/test_user_service.py new file mode 100644 index 0000000000..536a2af1eb --- /dev/null +++ b/common/djangoapps/xblock_django/tests/test_user_service.py @@ -0,0 +1,57 @@ +""" +Tests for the DjangoXBlockUserService. +""" +from django.test import TestCase +from xblock_django.user_service import ( + DjangoXBlockUserService, + ATTR_KEY_IS_AUTHENTICATED, + ATTR_KEY_USER_ID, + ATTR_KEY_USERNAME, +) +from student.tests.factories import UserFactory, AnonymousUserFactory + + +class UserServiceTestCase(TestCase): + """ + Tests for the DjangoXBlockUserService. + """ + def setUp(self): + self.user = UserFactory(username="tester", email="test@tester.com") + self.user.profile.name = "Test Tester" + self.anon_user = AnonymousUserFactory() + + def assert_is_anon_xb_user(self, xb_user): + """ + A set of assertions for an anonymous XBlockUser. + """ + self.assertFalse(xb_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED]) + self.assertIsNone(xb_user.full_name) + self.assertListEqual(xb_user.emails, []) + + def assert_xblock_user_matches_django(self, xb_user, dj_user): + """ + A set of assertions for comparing a XBlockUser to a django User + """ + self.assertTrue(xb_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED]) + self.assertEqual(xb_user.emails[0], dj_user.email) + self.assertEqual(xb_user.full_name, dj_user.profile.name) + self.assertEqual(xb_user.opt_attrs[ATTR_KEY_USERNAME], dj_user.username) + self.assertEqual(xb_user.opt_attrs[ATTR_KEY_USER_ID], dj_user.id) + + def test_convert_anon_user(self): + """ + Tests for convert_django_user_to_xblock_user behavior when django user is AnonymousUser. + """ + django_user_service = DjangoXBlockUserService(self.anon_user) + xb_user = django_user_service.get_current_user() + self.assertTrue(xb_user.is_current_user) + self.assert_is_anon_xb_user(xb_user) + + def test_convert_authenticate_user(self): + """ + Tests for convert_django_user_to_xblock_user behavior when django user is User. + """ + django_user_service = DjangoXBlockUserService(self.user) + xb_user = django_user_service.get_current_user() + self.assertTrue(xb_user.is_current_user) + self.assert_xblock_user_matches_django(xb_user, self.user) diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py new file mode 100644 index 0000000000..6867db9f38 --- /dev/null +++ b/common/djangoapps/xblock_django/user_service.py @@ -0,0 +1,42 @@ +""" +Support for converting a django user to an XBlock user +""" +from xblock.reference.user_service import XBlockUser, UserService + +ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated' +ATTR_KEY_USER_ID = 'edx-platform.user_id' +ATTR_KEY_USERNAME = 'edx-platform.username' + + +class DjangoXBlockUserService(UserService): + """ + A user service that converts Django users to XBlockUser + """ + def __init__(self, django_user, **kwargs): + super(DjangoXBlockUserService, self).__init__(**kwargs) + self._django_user = django_user + + def get_current_user(self): + """ + Returns the currently-logged in user, as an instance of XBlockUser + """ + return self._convert_django_user_to_xblock_user(self._django_user) + + def _convert_django_user_to_xblock_user(self, django_user): + """ + A function that returns an XBlockUser from the current Django request.user + """ + xblock_user = XBlockUser(is_current_user=True) + + if django_user is not None and django_user.is_authenticated(): + # This full_name is dependent on edx-platform's profile implementation + full_name = getattr(django_user.profile, 'name') if hasattr(django_user, 'profile') else None + xblock_user.full_name = full_name + xblock_user.emails = [django_user.email] + xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = True + xblock_user.opt_attrs[ATTR_KEY_USER_ID] = django_user.id + xblock_user.opt_attrs[ATTR_KEY_USERNAME] = django_user.username + else: + xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = False + + return xblock_user diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 60b2dd8844..18658f401c 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -30,6 +30,14 @@ try: except ImportError: HAS_REQUEST_CACHE = False +# We also may not always have the current request user (crum) module available +try: + from xblock_django.user_service import DjangoXBlockUserService + from crum import get_current_user + HAS_USER_SERVICE = True +except ImportError: + HAS_USER_SERVICE = False + ASSET_IGNORE_REGEX = getattr(settings, "ASSET_IGNORE_REGEX", r"(^\._.*$)|(^\.DS_Store$)|(^.*~$)") @@ -44,7 +52,15 @@ def load_function(path): return getattr(import_module(module_path), name) -def create_modulestore_instance(engine, content_store, doc_store_config, options, i18n_service=None, fs_service=None): +def create_modulestore_instance( + engine, + content_store, + doc_store_config, + options, + i18n_service=None, + fs_service=None, + user_service=None, +): """ This will return a new instance of a modulestore given an engine and options """ @@ -74,6 +90,11 @@ def create_modulestore_instance(engine, content_store, doc_store_config, options if issubclass(class_, BranchSettingMixin): _options['branch_setting_func'] = _get_modulestore_branch_setting + if HAS_USER_SERVICE and not user_service: + xb_user_service = DjangoXBlockUserService(get_current_user()) + else: + xb_user_service = None + return class_( contentstore=content_store, metadata_inheritance_cache_subsystem=metadata_inheritance_cache, @@ -83,6 +104,7 @@ def create_modulestore_instance(engine, content_store, doc_store_config, options doc_store_config=doc_store_config, i18n_service=i18n_service or ModuleI18nService(), fs_service=fs_service or xblock.reference.plugins.FSService(), + user_service=user_service or xb_user_service, **_options ) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 85ba62d82e..093259b94c 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -99,7 +99,17 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): """ ModuleStore knows how to route requests to the right persistence ms """ - def __init__(self, contentstore, mappings, stores, i18n_service=None, fs_service=None, create_modulestore_instance=None, **kwargs): + def __init__( + self, + contentstore, + mappings, + stores, + i18n_service=None, + fs_service=None, + user_service=None, + create_modulestore_instance=None, + **kwargs + ): """ Initialize a MixedModuleStore. Here we look into our passed in kwargs which should be a collection of other modulestore configuration information @@ -139,6 +149,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): store_settings.get('OPTIONS', {}), i18n_service=i18n_service, fs_service=fs_service, + user_service=user_service, ) # replace all named pointers to the store into actual pointers for course_key, store_name in self.mappings.iteritems(): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 80e6d5cf80..090bf2f01b 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -498,6 +498,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo error_tracker=null_error_tracker, i18n_service=None, fs_service=None, + user_service=None, retry_wait_time=0.1, **kwargs): """ @@ -551,6 +552,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo self.render_template = render_template self.i18n_service = i18n_service self.fs_service = fs_service + self.user_service = user_service self._course_run_cache = {} @@ -850,6 +852,9 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo if self.fs_service: services["fs"] = self.fs_service + if self.user_service: + services["user"] = self.user_service + system = CachingDescriptorSystem( modulestore=self, course_key=course_key, @@ -1150,6 +1155,9 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo if self.fs_service: services["fs"] = self.fs_service + if self.user_service: + services["user"] = self.user_service + runtime = CachingDescriptorSystem( modulestore=self, module_data={}, diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 093e90c038..7a7887b91d 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -607,7 +607,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): def __init__(self, contentstore, doc_store_config, fs_root, render_template, default_class=None, error_tracker=null_error_tracker, - i18n_service=None, fs_service=None, + i18n_service=None, fs_service=None, user_service=None, services=None, **kwargs): """ :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware. @@ -638,6 +638,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): if fs_service is not None: self.services["fs"] = fs_service + if user_service is not None: + self.services["user"] = user_service + def close_connections(self): """ Closes any open connections to the underlying databases diff --git a/common/lib/xmodule/xmodule/modulestore/tests/utils.py b/common/lib/xmodule/xmodule/modulestore/tests/utils.py index 10d3dd1742..371dcd93bf 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/utils.py @@ -27,7 +27,15 @@ def load_function(path): # pylint: disable=unused-argument -def create_modulestore_instance(engine, contentstore, doc_store_config, options, i18n_service=None, fs_service=None): +def create_modulestore_instance( + engine, + contentstore, + doc_store_config, + options, + i18n_service=None, + fs_service=None, + user_service=None +): """ This will return a new instance of a modulestore given an engine and options """ diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index b8cfad7e53..de8b06479a 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -293,7 +293,7 @@ class XMLModuleStore(ModuleStoreReadBase): """ def __init__( self, data_dir, default_class=None, course_dirs=None, course_ids=None, - load_error_modules=True, i18n_service=None, fs_service=None, **kwargs + load_error_modules=True, i18n_service=None, fs_service=None, user_service=None, **kwargs ): """ Initialize an XMLModuleStore from data_dir @@ -331,6 +331,7 @@ class XMLModuleStore(ModuleStoreReadBase): self.i18n_service = i18n_service self.fs_service = fs_service + self.user_service = user_service # If we are specifically asked for missing courses, that should # be an error. If we are asked for "all" courses, find the ones @@ -479,6 +480,9 @@ class XMLModuleStore(ModuleStoreReadBase): if self.fs_service: services['fs'] = self.fs_service + if self.user_service: + services['user'] = self.user_service + system = ImportSystem( xmlstore=self, course_id=course_id, diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 4fb9ab1898..09e6c85a60 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -641,7 +641,7 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes for group in user_partition.groups: str_group_id = unicode(group.id) if str_group_id not in self.group_id_to_child: - user_id = self.runtime.service(self, 'user').user_id + user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs['edx-platform.user_id'] self._create_vertical_for_group(group, user_id) changed = True diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index fc4d9603e9..4a0ae1e8a4 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -53,7 +53,7 @@ from xmodule_modifiers import ( ) from xmodule.lti_module import LTIModule from xmodule.x_module import XModuleDescriptor - +from xblock_django.user_service import DjangoXBlockUserService from util.json_request import JsonResponse from util.sandboxing import can_execute_unsafe_code, get_python_lib_zip if settings.FEATURES.get('MILESTONES_APP', False): @@ -644,6 +644,7 @@ def get_module_system_for_user(user, field_data_cache, 'i18n': ModuleI18nService(), 'fs': xblock.reference.plugins.FSService(), 'field-data': field_data, + 'user': DjangoXBlockUserService(user), }, get_user_role=lambda: get_user_role(user, course_id), descriptor_runtime=descriptor.runtime, diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 0faff94cf3..b751f8f4fc 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -46,6 +46,10 @@ from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT +@XBlock.needs("field-data") +@XBlock.needs("i18n") +@XBlock.needs("fs") +@XBlock.needs("user") class PureXBlock(XBlock): """ Pure XBlock to use in tests. @@ -1177,3 +1181,40 @@ class TestEventPublishing(ModuleStoreTestCase, LoginEnrollmentTestCase): mock_track_function.assert_called_once_with(request) mock_track_function.return_value.assert_called_once_with(event_type, event) + + +@ddt.ddt +class LMSXBlockServiceBindingTest(ModuleStoreTestCase): + """ + Tests that the LMS Module System (XBlock Runtime) provides an expected set of services. + """ + def setUp(self): + """ + Set up the user and other fields that will be used to instantiate the runtime. + """ + super(LMSXBlockServiceBindingTest, self).setUp() + self.user = UserFactory() + self.field_data_cache = Mock() + self.course = CourseFactory.create() + self.track_function = Mock() + self.xqueue_callback_url_prefix = Mock() + self.request_token = Mock() + + @XBlock.register_temp_plugin(PureXBlock, identifier='pure') + @ddt.data("user", "i18n", "fs", "field-data") + def test_expected_services_exist(self, expected_service): + """ + Tests that the 'user', 'i18n', and 'fs' services are provided by the LMS runtime. + """ + descriptor = ItemFactory(category="pure", parent=self.course) + runtime, _ = render.get_module_system_for_user( + self.user, + self.field_data_cache, + descriptor, + self.course.id, + self.track_function, + self.xqueue_callback_url_prefix, + self.request_token + ) + service = runtime.service(descriptor, expected_service) + self.assertIsNotNone(service)