From de23d6defe96468be4aa044a09958e9e1197140b Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 20 Feb 2015 13:11:53 -0500 Subject: [PATCH] Clear cached children when binding an XModuleDescriptor/XBlock to a particular user --- cms/djangoapps/contentstore/views/preview.py | 3 +- common/lib/xmodule/xmodule/tests/__init__.py | 2 +- .../xmodule/tests/test_library_content.py | 2 +- .../xmodule/tests/test_split_test_module.py | 9 ++++- common/lib/xmodule/xmodule/x_module.py | 37 +++++++++++++++++-- lms/djangoapps/courseware/module_render.py | 9 ++--- .../courseware/tests/test_module_render.py | 7 +++- 7 files changed, 52 insertions(+), 17 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 8ef95db43a..f192121d2a 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -213,7 +213,8 @@ def _load_preview_module(request, descriptor): field_data = LmsFieldData(descriptor._field_data, student_data) # pylint: disable=protected-access descriptor.bind_for_student( _preview_module_system(request, descriptor, field_data), - field_data + field_data, + request.user.id ) return descriptor diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 6f83049c31..44118f9e7a 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -110,7 +110,7 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')): # So, bind to the same one as the current descriptor. module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access - descriptor.bind_for_student(module_system, descriptor._field_data) + descriptor.bind_for_student(module_system, descriptor._field_data, user.id) return descriptor diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 1feeddea29..659b4fef4a 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -58,7 +58,7 @@ class LibraryContentTest(MixedSplitTestCase): sub_module_system = get_test_system(course_id=self.course.location.course_key) sub_module_system.get_module = get_module sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access - descriptor.bind_for_student(sub_module_system, descriptor._field_data) # pylint: disable=protected-access + descriptor.bind_for_student(sub_module_system, descriptor._field_data, self.user_id) # pylint: disable=protected-access return descriptor module_system.get_module = get_module diff --git a/common/lib/xmodule/xmodule/tests/test_split_test_module.py b/common/lib/xmodule/xmodule/tests/test_split_test_module.py index 97f71114b0..1dbc514947 100644 --- a/common/lib/xmodule/xmodule/tests/test_split_test_module.py +++ b/common/lib/xmodule/xmodule/tests/test_split_test_module.py @@ -81,6 +81,7 @@ class SplitTestModuleTest(XModuleXmlImportTest, PartitionTestCase): self.module_system.descriptor_runtime = self.course._runtime # pylint: disable=protected-access self.course.runtime.export_fs = MemoryFS() + user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) self.partitions_service = StaticPartitionService( [ self.user_partition, @@ -90,14 +91,18 @@ class SplitTestModuleTest(XModuleXmlImportTest, PartitionTestCase): MockUserPartitionScheme() ) ], - user=Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True), + user=user, course_id=self.course.id, track_function=Mock(name='track_function'), ) self.module_system._services['partitions'] = self.partitions_service # pylint: disable=protected-access self.split_test_module = self.course_sequence.get_children()[0] - self.split_test_module.bind_for_student(self.module_system, self.split_test_module._field_data) # pylint: disable=protected-access + self.split_test_module.bind_for_student( + self.module_system, + self.split_test_module._field_data, # pylint: disable=protected-access + user.id + ) @ddt.ddt diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 15413143bc..9270e735cf 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -17,8 +17,11 @@ from webob import Response from webob.multidict import MultiDict from xblock.core import XBlock, XBlockAside -from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String, Dict, ScopeIds, Reference, \ - ReferenceList, ReferenceValueDict +from xblock.fields import ( + Scope, Integer, Float, List, XBlockMixin, + String, Dict, ScopeIds, Reference, ReferenceList, + ReferenceValueDict, UserScope +) from xblock.fragment import Fragment from xblock.runtime import Runtime, IdReader, IdGenerator from xmodule.fields import RelativeTime @@ -281,6 +284,8 @@ class XModuleMixin(XBlockMixin): def __init__(self, *args, **kwargs): self.xmodule_runtime = None + self._child_instances = None + super(XModuleMixin, self).__init__(*args, **kwargs) @property @@ -410,7 +415,7 @@ class XModuleMixin(XBlockMixin): if not self.has_children: return [] - if getattr(self, '_child_instances', None) is None: + if self._child_instances is None: self._child_instances = [] # pylint: disable=attribute-defined-outside-init for child_loc in self.children: # Skip if it doesn't satisfy the filter function @@ -525,15 +530,39 @@ class XModuleMixin(XBlockMixin): """ return None - def bind_for_student(self, xmodule_runtime, field_data): + def bind_for_student(self, xmodule_runtime, field_data, user_id): """ Set up this XBlock to act as an XModule instead of an XModuleDescriptor. Arguments: xmodule_runtime (:class:`ModuleSystem'): the runtime to use when accessing student facing methods field_data (:class:`FieldData`): The :class:`FieldData` to use for all subsequent data access + user_id: The user_id to set in scope_ids """ # pylint: disable=attribute-defined-outside-init + + # Skip rebinding if we're already bound a user, and it's this user. + if self.scope_ids.user_id is not None and user_id == self.scope_ids.user_id: + return + + # If we are switching users mid-request, save the data from the old user. + self.save() + + # Update scope_ids to point to the new user. + self.scope_ids = self.scope_ids._replace(user_id=user_id) + + # Clear out any cached instantiated children. + self._child_instances = None + + # Clear out any cached field data scoped to the old user. + for field in self.fields.values(): + if field.scope in (Scope.parent, Scope.children): + continue + + if field.scope.user == UserScope.ONE: + field._del_cached_value(self) + + # Set the new xmodule_runtime and field_data (which are user-specific) self.xmodule_runtime = xmodule_runtime self._field_data = field_data diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 86f327dd8d..ef99331edc 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -497,10 +497,8 @@ def get_module_system_for_user(user, field_data_cache, # rebinds module to a different student. We'll change system, student_data, and scope_ids module.descriptor.bind_for_student( inner_system, - LmsFieldData(module.descriptor._field_data, inner_student_data) # pylint: disable=protected-access - ) - module.descriptor.scope_ids = ( - module.descriptor.scope_ids._replace(user_id=real_user.id) # pylint: disable=protected-access + LmsFieldData(module.descriptor._field_data, inner_student_data), # pylint: disable=protected-access + real_user.id, ) module.scope_ids = module.descriptor.scope_ids # this is needed b/c NamedTuples are immutable # now bind the module to the new ModuleSystem instance and vice-versa @@ -688,8 +686,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours request_token=request_token ) - descriptor.bind_for_student(system, field_data) # pylint: disable=protected-access - descriptor.scope_ids = descriptor.scope_ids._replace(user_id=user.id) # pylint: disable=protected-access + descriptor.bind_for_student(system, field_data, user.id) # pylint: disable=protected-access return descriptor diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 72e627f73a..fa2b775bf7 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -921,7 +921,7 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase): location = course_id.make_usage_key('dummy_category', 'dummy_name') descriptor = Mock( spec=xblock_class, - _field_data=Mock(spec=FieldData), + _field_data=Mock(spec=FieldData, name='field_data'), location=location, static_asset_path=None, _runtime=Mock( @@ -931,7 +931,10 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase): name='runtime', ), scope_ids=Mock(spec=ScopeIds), - name='descriptor' + name='descriptor', + _field_data_cache={}, + _dirty_fields={}, + fields={}, ) descriptor.runtime = CombinedSystem(descriptor._runtime, None) # pylint: disable=protected-access # Use the xblock_class's bind_for_student method