Changed interface of StudentModuleCache. Other changes from code review.

common/lib/xmodule/xmodule/course_module.py

@@ -69,7 +69,7 @@ class CourseDescriptor(SequenceDescriptor):
         
         The grading context has two keys:    
         graded_sections - This contains the sections that are graded, as
-            well as all possible children modules that can effect the
+            well as all possible children modules that can affect the
             grading. This allows some sections to be skipped if the student
             hasn't seen any part of it.
             
@@ -101,10 +101,8 @@ class CourseDescriptor(SequenceDescriptor):
             for s in c.get_children():
                 if s.metadata.get('graded', False):
                 
-                    xmoduledescriptors = []
-                    for module in yield_descriptor_descendents(s):
                     # TODO: Only include modules that have a score here
-                        xmoduledescriptors.append(module)
+                    xmoduledescriptors = [s for s in yield_descriptor_descendents(s)]
                     
                     section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : xmoduledescriptors}
                 

lms/djangoapps/courseware/grades.py

@@ -27,7 +27,7 @@ def grade(student, request, course, student_module_cache=None):
     grading_context = course.grading_context
     
     if student_module_cache == None:
-        student_module_cache = StudentModuleCache(student, descriptors=grading_context['all_descriptors'])
+        student_module_cache = StudentModuleCache(student, grading_context['all_descriptors'])
     
     totaled_scores = {}
     # This next complicated loop is just to collect the totaled_scores, which is

lms/djangoapps/courseware/management/commands/check_course.py

@@ -78,7 +78,7 @@ class Command(BaseCommand):
 
         # TODO (cpennington): Get coursename in a legitimate way
         course_location = 'i4x://edx/6002xs12/course/6.002_Spring_2012'
-        student_module_cache = StudentModuleCache(sample_user, modulestore().get_item(course_location))
+        student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(sample_user, modulestore().get_item(course_location))
         course = get_module(sample_user, None, course_location, student_module_cache)
 
         to_run = [

lms/djangoapps/courseware/models.py

@@ -67,30 +67,19 @@ class StudentModuleCache(object):
     """
     A cache of StudentModules for a specific student
     """
-    def __init__(self, user, descriptor=None, depth=None, descriptors=None, descriptor_filter=lambda descriptor: True):
+    def __init__(self, user, descriptors):
         '''
         Find any StudentModule objects that are needed by any child modules of the
         supplied descriptor, or caches only the StudentModule objects specifically
         for every descriptor in descriptors. Avoids making multiple queries to the 
         database.
         
-        There are two ways to init:
-            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
-        OR
+        Arguments
+        user: The user for which to fetch maching StudentModules
         descriptors: An array of XModuleDescriptors.
-        
-        descriptor_filter is a function that accepts a descriptor and return wether the StudentModule 
-            should be cached
         '''
         if user.is_authenticated():
-            if not (descriptor == None) != (descriptors == None): #An xor on the descriptor and descriptors parameters.
-                raise ValueError("Either the descriptor or descriptors must be supplied to StudentModuleCache.") 
-            
-            if descriptor != None:
-                descriptors = self._get_child_descriptors(descriptor, depth)
-            module_ids = self._get_module_state_keys(descriptors, descriptor_filter) 
+            module_ids = self._get_module_state_keys(descriptors) 
 
             # This works around a limitation in sqlite3 on the number of parameters
             # that can be put into a single query
@@ -105,23 +94,37 @@ class StudentModuleCache(object):
         else:
             self.cache = []
     
-    def _get_child_descriptors(self, descriptor, depth):
+    
+    @classmethod
+    def cache_for_descriptor_descendents(cls, user, descriptor, depth=None, descriptor_filter=lambda descriptor: True):
         """
         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
         """
+        
+        def get_child_descriptors(descriptor, depth, descriptor_filter):
+            if descriptor_filter(descriptor):
                 descriptors = [descriptor]
+            else:
+                descriptors = []
             
             if depth is None or depth > 0:
                 new_depth = depth - 1 if depth is not None else depth
 
                 for child in descriptor.get_children():
-                descriptors.extend(self._get_child_descriptors(child, new_depth))
+                    descriptors.extend(get_child_descriptors(child, new_depth, descriptor_filter))
                 
             return descriptors
         
-    def _get_module_state_keys(self, descriptors, descriptor_filter):
+        
+        descriptors = get_child_descriptors(descriptor, depth, descriptor_filter)
+        
+        return StudentModuleCache(user, descriptors)    
+    
+    def _get_module_state_keys(self, descriptors):
         '''
         Get a list of the state_keys needed for StudentModules
         required for this module descriptor
@@ -131,7 +134,6 @@ class StudentModuleCache(object):
         '''
         keys = []
         for descriptor in descriptors:
-            if descriptor_filter(descriptor):
             keys.append(descriptor.location.url())
             
             shared_state_key = getattr(descriptor, 'shared_state_key', None)

lms/djangoapps/courseware/module_render.py

@@ -48,7 +48,7 @@ def toc_for_course(user, request, course, active_chapter, active_section):
     chapters with name 'hidden' are skipped.
     '''
     
-    student_module_cache = StudentModuleCache(user, course, depth=2)
+    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, course, depth=2)
     course = get_module(user, request, course.location, student_module_cache)
 
     chapters = list()
@@ -259,7 +259,7 @@ def xqueue_callback(request, userid, id, dispatch):
     # Retrieve target StudentModule
     user = User.objects.get(id=userid)
 
-    student_module_cache = StudentModuleCache(user, modulestore().get_item(id))
+    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, modulestore().get_item(id))
     instance = get_module(request.user, request, id, student_module_cache)
     
     instance_module = get_instance_module(request.user, instance, student_module_cache)
@@ -307,7 +307,7 @@ def modx_dispatch(request, dispatch=None, id=None):
     '''
     # ''' (fix emacs broken parsing)
 
-    student_module_cache = StudentModuleCache(request.user, modulestore().get_item(id))
+    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, modulestore().get_item(id))
     instance = get_module(request.user, request, id, student_module_cache)
     
     instance_module = get_instance_module(request.user, instance, student_module_cache)

lms/djangoapps/courseware/views.py

@@ -110,7 +110,7 @@ def profile(request, course_id, student_id=None):
 
     user_info = UserProfile.objects.get(user=student)
 
-    student_module_cache = StudentModuleCache(request.user, course)
+    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, course)
     course_module = get_module(request.user, request, course.location, student_module_cache)
     
     courseware_summary = grades.progress_summary(student, course_module, course.grader, student_module_cache)
@@ -191,9 +191,10 @@ def index(request, course_id, chapter=None, section=None,
         if look_for_module:
             section_descriptor = get_section(course, chapter, section)
             if section_descriptor is not None:
-                student_module_cache = StudentModuleCache(request.user,
+                student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+                                                          request.user,
                                                           section_descriptor)
-                module, _, _, _ = get_module(request.user, request,
+                module = get_module(request.user, request,
                                     section_descriptor.location,
                                     student_module_cache)
                 context['content'] = module.get_html()