Make XModule use the same get_children that XModuleDescriptor and XBlock do, and…

Make XModule use the same get_children that XModuleDescriptor and XBlock do, and make it returned the same CombinedSystem

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

@@ -206,6 +206,7 @@ class ConditionalModuleXmlTest(unittest.TestCase):
                 location = descriptor
                 descriptor = self.modulestore.get_item(location, depth=None)
             descriptor.xmodule_runtime = get_test_system()
+            descriptor.xmodule_runtime.descriptor_runtime = descriptor._runtime  # pylint: disable=protected-access
             descriptor.xmodule_runtime.get_module = inner_get_module
             return descriptor
 

common/lib/xmodule/xmodule/x_module.py

@@ -560,7 +560,7 @@ class XModuleMixin(XBlockMixin):
                 continue
 
             if field.scope.user == UserScope.ONE:
-                field._del_cached_value(self)
+                field._del_cached_value(self)  # pylint: disable=protected-access
 
         # Set the new xmodule_runtime and field_data (which are user-specific)
         self.xmodule_runtime = xmodule_runtime
@@ -644,10 +644,19 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock):  # pylint: disable=abstract-me
 
         # Set the descriptor first so that we can proxy to it
         self.descriptor = descriptor
-        super(XModule, self).__init__(*args, **kwargs)
         self._loaded_children = None
+        self._runtime = None
+        super(XModule, self).__init__(*args, **kwargs)
         self.runtime.xmodule_instance = self
 
+    @property
+    def runtime(self):
+        return CombinedSystem(self._runtime, self.descriptor._runtime)  # pylint: disable=protected-access
+
+    @runtime.setter
+    def runtime(self, value):  # pylint: disable=arguments-differ
+        self._runtime = value
+
     def __unicode__(self):
         return u'<x_module(id={0})>'.format(self.id)
 
@@ -688,26 +697,6 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock):  # pylint: disable=abstract-me
         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.
-        """
-        if self._loaded_children is None:
-            child_descriptors = self.get_child_descriptors()
-
-            # This deliberately uses system.get_module, rather than runtime.get_block,
-            # because we're looking at XModule children, rather than XModuleDescriptor children.
-            # That means it can use the deprecated XModule apis, rather than future XBlock apis
-
-            # TODO: Once we're in a system where this returns a mix of XModuleDescriptors
-            # and XBlocks, we're likely to have to change this more
-            children = [self.system.get_module(descriptor) for descriptor in child_descriptors]
-            # get_module returns None if the current user doesn't have access
-            # to the location.
-            self._loaded_children = [c for c in children if c is not None]
-
-        return self._loaded_children
-
     def get_child_descriptors(self):
         """
         Returns the descriptors of the child modules

lms/djangoapps/courseware/model_data.py

@@ -62,7 +62,6 @@ class FieldDataCache(object):
         asides: The list of aside types to load, or None to prefetch no asides.
         '''
         self.cache = {}
-        self.descriptors = descriptors
         self.select_for_update = select_for_update
 
         if asides is None:
@@ -74,24 +73,27 @@ class FieldDataCache(object):
         self.course_id = course_id
         self.user = user
 
-        if user.is_authenticated():
-            for scope, fields in self._fields_to_cache().items():
-                for field_object in self._retrieve_fields(scope, fields):
+        self.add_descriptors_to_cache(descriptors)
+
+    def add_descriptors_to_cache(self, descriptors):
+        """
+        Add all `descriptors` to this FieldDataCache.
+        """
+        if self.user.is_authenticated():
+            for scope, fields in self._fields_to_cache(descriptors).items():
+                for field_object in self._retrieve_fields(scope, fields, descriptors):
                     self.cache[self._cache_key_from_field_object(scope, field_object)] = field_object
 
-    @classmethod
-    def cache_for_descriptor_descendents(cls, course_id, user, descriptor, depth=None,
-                                         descriptor_filter=lambda descriptor: True,
-                                         select_for_update=False, asides=None):
+    def add_descriptor_descendents(self, descriptor, depth=None, descriptor_filter=lambda descriptor: True):
         """
-        course_id: the course in the context of which we want StudentModules.
-        user: the django user for whom to load modules.
+        Add all descendents of `descriptor` to this FieldDataCache.
+
+        Arguments:
             descriptor: An XModuleDescriptor
             depth is the number of levels of descendent modules to load StudentModules for, in addition to
                 the supplied descriptor. If depth is None, load all descendent StudentModules
             descriptor_filter is a function that accepts a descriptor and return wether the StudentModule
                 should be cached
-        select_for_update: Flag indicating whether the rows should be locked until end of transaction
         """
 
         def get_child_descriptors(descriptor, depth, descriptor_filter):
@@ -120,7 +122,25 @@ class FieldDataCache(object):
         with modulestore().bulk_operations(descriptor.location.course_key):
             descriptors = get_child_descriptors(descriptor, depth, descriptor_filter)
 
-        return FieldDataCache(descriptors, course_id, user, select_for_update, asides=asides)
+        self.add_descriptors_to_cache(descriptors)
+
+    @classmethod
+    def cache_for_descriptor_descendents(cls, course_id, user, descriptor, depth=None,
+                                         descriptor_filter=lambda descriptor: True,
+                                         select_for_update=False, asides=None):
+        """
+        course_id: the course in the context of which we want StudentModules.
+        user: the django user for whom to load modules.
+        descriptor: An XModuleDescriptor
+        depth is the number of levels of descendent modules to load StudentModules for, in addition to
+            the supplied descriptor. If depth is None, load all descendent StudentModules
+        descriptor_filter is a function that accepts a descriptor and return wether the StudentModule
+            should be cached
+        select_for_update: Flag indicating whether the rows should be locked until end of transaction
+        """
+        cache = FieldDataCache([], course_id, user, select_for_update, asides=asides)
+        cache.add_descriptor_descendents(descriptor, depth, descriptor_filter)
+        return cache
 
     def _query(self, model_class, **kwargs):
         """
@@ -147,14 +167,13 @@ class FieldDataCache(object):
         )
         return res
 
-    @property
-    def _all_usage_ids(self):
+    def _all_usage_ids(self, descriptors):
         """
         Return a set of all usage_ids for the descriptors that this FieldDataCache is caching
         against, and well as all asides for those descriptors.
         """
         usage_ids = set()
-        for descriptor in self.descriptors:
+        for descriptor in descriptors:
             usage_ids.add(descriptor.scope_ids.usage_id)
 
             for aside_type in self.asides:
@@ -162,13 +181,12 @@ class FieldDataCache(object):
 
         return usage_ids
 
-    @property
-    def _all_block_types(self):
+    def _all_block_types(self, descriptors):
         """
         Return a set of all block_types that are cached by this FieldDataCache.
         """
         block_types = set()
-        for descriptor in self.descriptors:
+        for descriptor in descriptors:
             block_types.add(BlockTypeKeyV1(descriptor.entry_point, descriptor.scope_ids.block_type))
 
         for aside_type in self.asides:
@@ -176,7 +194,7 @@ class FieldDataCache(object):
 
         return block_types
 
-    def _retrieve_fields(self, scope, fields):
+    def _retrieve_fields(self, scope, fields, descriptors):
         """
         Queries the database for all of the fields in the specified scope
         """
@@ -184,7 +202,7 @@ class FieldDataCache(object):
             return self._chunked_query(
                 StudentModule,
                 'module_state_key__in',
-                self._all_usage_ids,
+                self._all_usage_ids(descriptors),
                 course_id=self.course_id,
                 student=self.user.pk,
             )
@@ -192,14 +210,14 @@ class FieldDataCache(object):
             return self._chunked_query(
                 XModuleUserStateSummaryField,
                 'usage_id__in',
-                self._all_usage_ids,
+                self._all_usage_ids(descriptors),
                 field_name__in=set(field.name for field in fields),
             )
         elif scope == Scope.preferences:
             return self._chunked_query(
                 XModuleStudentPrefsField,
                 'module_type__in',
-                self._all_block_types,
+                self._all_block_types(descriptors),
                 student=self.user.pk,
                 field_name__in=set(field.name for field in fields),
             )
@@ -212,12 +230,12 @@ class FieldDataCache(object):
         else:
             return []
 
-    def _fields_to_cache(self):
+    def _fields_to_cache(self, descriptors):
         """
         Returns a map of scopes to fields in that scope that should be cached
         """
         scope_map = defaultdict(set)
-        for descriptor in self.descriptors:
+        for descriptor in descriptors:
             for field in descriptor.fields.values():
                 scope_map[field.scope].add(field)
         return scope_map

lms/djangoapps/courseware/views.py

@@ -489,8 +489,8 @@ def _index_bulk_op(request, course_key, chapter, section, position):
 
             # Load all descendants of the section, because we're going to display its
             # html, which in general will need all of its children
-            section_field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
-                course_key, user, section_descriptor, depth=None, asides=XBlockAsidesConfig.possible_asides()
+            field_data_cache.add_descriptor_descendents(
+                section_descriptor, depth=None
             )
 
             # Verify that position a string is in fact an int
@@ -504,7 +504,7 @@ def _index_bulk_op(request, course_key, chapter, section, position):
                 request.user,
                 request,
                 section_descriptor,
-                section_field_data_cache,
+                field_data_cache,
                 course_key,
                 position
             )