WIP: Model data caching work

common/lib/xmodule/xmodule/course_module.py

@@ -358,7 +358,7 @@ class CourseDescriptor(SequenceDescriptor):
 
         all_descriptors - This contains a list of all xmodules that can
             effect grading a student. This is used to efficiently fetch
-            all the xmodule state for a StudentModuleCache without walking
+            all the xmodule state for a ModelDataCache without walking
             the descriptor tree again.
 
 

lms/djangoapps/courseware/courses.py

@@ -19,10 +19,10 @@ from xmodule.contentstore.content import StaticContent
 from xmodule.modulestore.xml import XMLModuleStore
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.x_module import XModule
+from courseware.model_data import ModelDataCache
 from static_replace import replace_urls, try_staticfiles_lookup
 from courseware.access import has_access
 import branding
-from courseware.models import StudentModuleCache
 from xmodule.modulestore.exceptions import ItemNotFoundError
 
 log = logging.getLogger(__name__)
@@ -148,12 +148,23 @@ def get_course_about_section(course, section_key):
             request = get_request_for_thread()
 
             loc = course.location._replace(category='about', name=section_key)
-            course_module = get_module(request.user, request, loc, None, course.id, not_found_ok = True, wrap_xmodule_display = True)
+
+            # Use an empty cache
+            model_data_cache = ModelDataCache([], course.id, request.user)
+            about_module = get_module(
+                request.user,
+                request,
+                loc,
+                model_data_cache,
+                course.id,
+                not_found_ok=True,
+                wrap_xmodule_display=True
+            )
 
             html = ''
 
-            if course_module is not None:
-                html = course_module.get_html()
+            if about_module is not None:
+                html = about_module.get_html()
 
             return html
 
@@ -174,7 +185,7 @@ def get_course_about_section(course, section_key):
 
 
 
-def get_course_info_section(request, cache, course, section_key):
+def get_course_info_section(request, course, section_key):
     """
     This returns the snippet of html to be rendered on the course info page,
     given the key for the section.
@@ -188,11 +199,22 @@ def get_course_info_section(request, cache, course, section_key):
 
 
     loc = Location(course.location.tag, course.location.org, course.location.course, 'course_info', section_key)
-    course_module = get_module(request.user, request, loc, cache, course.id, wrap_xmodule_display = True)
+
+    # Use an empty cache
+    model_data_cache = ModelDataCache([], course.id, request.user)
+    info_module = get_module(
+        request.user,
+        request,
+        loc,
+        model_data_cache,
+        course.id,
+        wrap_xmodule_display=True
+    )
+
     html = ''
 
-    if course_module is not None:
-        html = course_module.get_html()
+    if info_module is not None:
+        html = info_module.get_html()
 
     return html
 

lms/djangoapps/courseware/grades.py

@@ -8,7 +8,8 @@ from collections import defaultdict
 from django.conf import settings
 from django.contrib.auth.models import User
 
-from models import StudentModuleCache
+from .model_data import ModelDataCache, LmsKeyValueStore
+from xblock.core import Scope
 from module_render import get_module
 from xmodule import graders
 from xmodule.capa_module import CapaModule
@@ -57,37 +58,39 @@ def yield_problems(request, course, student):
     the list, but there may be others as well).
     """
     grading_context = course.grading_context
-    student_module_cache = StudentModuleCache(course.id, student, grading_context['all_descriptors'])
 
+    descriptor_locations = (descriptor.location.url() for descriptor in grading_context['all_descriptors'])
+    existing_student_modules = set(StudentModule.objects.filter(
+        module_state_key__in=descriptor_locations
+    ).values_list('module_state_key', flat=True))
+
+    sections_to_list = []
     for section_format, sections in grading_context['graded_sections'].iteritems():
         for section in sections:
 
             section_descriptor = section['section_descriptor']
 
             # If the student hasn't seen a single problem in the section, skip it.
-            skip = True
             for moduledescriptor in section['xmoduledescriptors']:
-                if student_module_cache.lookup(
-                        course.id, moduledescriptor.category, moduledescriptor.location.url()):
-                    skip = False
+                if moduledescriptor.location.url() in existing_student_modules:
+                    sections_to_list.append(section_descriptor)
                     break
 
-            if skip:
-                continue
-
-            section_module = get_module(student, request,
-                                        section_descriptor.location, student_module_cache,
-                                        course.id)
-            if section_module is None:
-                # student doesn't have access to this module, or something else
-                # went wrong.
-                # log.debug("couldn't get module for student {0} for section location {1}"
-                #           .format(student.username, section_descriptor.location))
-                continue
+    model_data_cache = ModelDataCache(sections_to_list, course.id, student)
+    for section_descriptor in sections_to_list:
+        section_module = get_module(student, request,
+                                    section_descriptor.location, model_data_cache,
+                                    course.id)
+        if section_module is None:
+            # student doesn't have access to this module, or something else
+            # went wrong.
+            # log.debug("couldn't get module for student {0} for section location {1}"
+            #           .format(student.username, section_descriptor.location))
+            continue
 
-            for problem in yield_module_descendents(section_module):
-                if isinstance(problem, CapaModule):
-                    yield problem
+        for problem in yield_module_descendents(section_module):
+            if isinstance(problem, CapaModule):
+                yield problem
 
 def answer_distributions(request, course):
     """
@@ -116,7 +119,7 @@ def answer_distributions(request, course):
     return counts
 
 
-def grade(student, request, course, student_module_cache=None, keep_raw_scores=False):
+def grade(student, request, course, model_data_cache=None, keep_raw_scores=False):
     """
     This grades a student as quickly as possible. It retuns the
     output from the course grader, augmented with the final letter
@@ -138,8 +141,8 @@ def grade(student, request, course, student_module_cache=None, keep_raw_scores=F
     grading_context = course.grading_context
     raw_scores = []
 
-    if student_module_cache == None:
-        student_module_cache = StudentModuleCache(course.id, student, grading_context['all_descriptors'])
+    if model_data_cache == None:
+        model_data_cache = ModelDataCache(grading_context['all_descriptors'], course.id, student)
 
     totaled_scores = {}
     # This next complicated loop is just to collect the totaled_scores, which is
@@ -153,8 +156,14 @@ def grade(student, request, course, student_module_cache=None, keep_raw_scores=F
             should_grade_section = False
             # If we haven't seen a single problem in the section, we don't have to grade it at all! We can assume 0%  
             for moduledescriptor in section['xmoduledescriptors']:
-                if student_module_cache.lookup(
-                        course.id, moduledescriptor.category, moduledescriptor.location.url()):
+                # Create a fake key to pull out a StudentModule object from the ModelDataCache
+                key = LmsKeyValueStore.Key(
+                    Scope.student_state,
+                    student.id,
+                    moduledescriptor.location,
+                    None
+                )
+                if model_data_cache.find(key):
                     should_grade_section = True
                     break
 
@@ -165,11 +174,11 @@ def grade(student, request, course, student_module_cache=None, keep_raw_scores=F
                     # TODO: We need the request to pass into here. If we could forgo that, our arguments
                     # would be simpler
                     return get_module(student, request, descriptor.location, 
-                                        student_module_cache, course.id)
+                                        model_data_cache, course.id)
                                 
                 for module_descriptor in yield_dynamic_descriptor_descendents(section_descriptor, create_module):
                                                      
-                    (correct, total) = get_score(course.id, student, module_descriptor, create_module, student_module_cache)
+                    (correct, total) = get_score(course.id, student, module_descriptor, create_module, model_data_cache)
                     if correct is None and total is None:
                         continue
 
@@ -240,7 +249,7 @@ def grade_for_percentage(grade_cutoffs, percentage):
 # TODO: This method is not very good. It was written in the old course style and
 # then converted over and performance is not good. Once the progress page is redesigned
 # to not have the progress summary this method should be deleted (so it won't be copied).
-def progress_summary(student, request, course, student_module_cache):
+def progress_summary(student, request, course, model_data_cache):
     """
     This pulls a summary of all problems in the course.
 
@@ -254,7 +263,7 @@ def progress_summary(student, request, course, student_module_cache):
     Arguments:
         student: A User object for the student to grade
         course: A Descriptor containing the course to grade
-        student_module_cache: A StudentModuleCache initialized with all
+        model_data_cache: A ModelDataCache initialized with all
              instance_modules for the student
     
     If the student does not have access to load the course module, this function
@@ -266,7 +275,7 @@ def progress_summary(student, request, course, student_module_cache):
     # TODO: We need the request to pass into here. If we could forgo that, our arguments
     # would be simpler
     course_module = get_module(student, request,
-                                course.location, student_module_cache,
+                                course.location, model_data_cache,
                                 course.id)
     if not course_module:
         # This student must not have access to the course.
@@ -294,7 +303,7 @@ def progress_summary(student, request, course, student_module_cache):
             for module_descriptor in yield_dynamic_descriptor_descendents(section_module.descriptor, module_creator):
                 
                 course_id = course.id
-                (correct, total) = get_score(course_id, student, module_descriptor, module_creator, student_module_cache)
+                (correct, total) = get_score(course_id, student, module_descriptor, module_creator, model_data_cache)
                 if correct is None and total is None:
                     continue
 
@@ -324,7 +333,7 @@ def progress_summary(student, request, course, student_module_cache):
     return chapters
 
 
-def get_score(course_id, user, problem_descriptor, module_creator, student_module_cache):
+def get_score(course_id, user, problem_descriptor, module_creator, model_data_cache):
     """
     Return the score for a user on a problem, as a tuple (correct, total).
     e.g. (5,7) if you got 5 out of 7 points.
@@ -336,7 +345,7 @@ def get_score(course_id, user, problem_descriptor, module_creator, student_modul
     problem_descriptor: an XModuleDescriptor
     module_creator: a function that takes a descriptor, and returns the corresponding XModule for this user.
            Can return None if user doesn't have access, or if something else went wrong.
-    cache: A StudentModuleCache
+    cache: A ModelDataCache
     """
     if not user.is_authenticated():
         return (None, None)
@@ -345,16 +354,23 @@ def get_score(course_id, user, problem_descriptor, module_creator, student_modul
         # These are not problems, and do not have a score
         return (None, None)
 
-    instance_module = student_module_cache.lookup(
-        course_id, problem_descriptor.category, problem_descriptor.location.url())
+    # Create a fake KeyValueStore key to pull out the StudentModule
+    key = LmsKeyValueStore.Key(
+        Scope.student_state,
+        user.id,
+        problem_descriptor.location,
+        None
+    )
+
+    student_module = model_data_cache.find(key)
 
-    if instance_module is not None and instance_module.max_grade is not None:
-        correct = instance_module.grade if instance_module.grade is not None else 0
-        total = instance_module.max_grade
+    if student_module is not None and student_module.max_grade is not None:
+        correct = student_module.grade if student_module.grade is not None else 0
+        total = student_module.max_grade
     else:
         # If the problem was not in the cache, or hasn't been graded yet,
         # we need to instantiate the problem.
-        # Otherwise, the max score (cached in instance_module) won't be available
+        # Otherwise, the max score (cached in student_module) won't be available
         problem = module_creator(problem_descriptor)
         if problem is None:
             return (None, None)
@@ -371,7 +387,7 @@ def get_score(course_id, user, problem_descriptor, module_creator, student_modul
     weight = getattr(problem_descriptor, 'weight', None)
     if weight is not None:
         if total == 0:
-            log.exception("Cannot reweight a problem with zero total points. Problem: " + str(instance_module))
+            log.exception("Cannot reweight a problem with zero total points. Problem: " + str(student_module))
             return (correct, total)
         correct = correct * weight / total
         total = weight

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

@@ -13,7 +13,7 @@ import xmodule
 import mitxmako.middleware as middleware
 middleware.MakoMiddleware()
 from xmodule.modulestore.django import modulestore
-from courseware.models import StudentModuleCache
+from courseware.model_data import ModelDataCache
 from courseware.module_render import get_module
 
 
@@ -83,7 +83,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.cache_for_descriptor_descendents(
+        student_module_cache = ModelDataCache.cache_for_descriptor_descendents(
         course_id,
             sample_user, modulestore().get_item(course_location))
         course = get_module(sample_user, None, course_location, student_module_cache)

lms/djangoapps/courseware/model_data.py

 import json
-from collections import namedtuple
+from collections import namedtuple, defaultdict
+from itertools import chain
 from .models import (
     StudentModule,
     XModuleContentField,
@@ -16,6 +17,235 @@ class InvalidWriteError(Exception):
     pass
 
 
+def chunks(items, chunk_size):
+    items = list(items)
+    return (items[i:i + chunk_size] for i in xrange(0, len(items), chunk_size))
+
+
+class ModelDataCache(object):
+    """
+    A cache of django model objects needed to supply the data
+    for a module and its decendents
+    """
+    def __init__(self, descriptors, course_id, user, select_for_update=False):
+        '''
+        Find any courseware.models objects that are needed by any descriptor
+        in descriptors. Attempts to minimize the number of queries to the database.
+        Note: Only modules that have store_state = True or have shared
+        state will have a StudentModule.
+
+        Arguments
+        descriptors: An array of XModuleDescriptors.
+        course_id: The id of the current course
+        user: The user for which to cache data
+        select_for_update: Flag indicating whether the rows should be locked until end of transaction
+        '''
+        self.cache = {}
+        self.descriptors = descriptors
+        self.select_for_update = select_for_update
+        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.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):
+        """
+        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
+        """
+
+        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(get_child_descriptors(child, new_depth, descriptor_filter))
+
+            return descriptors
+
+        descriptors = get_child_descriptors(descriptor, depth, descriptor_filter)
+
+        return ModelDataCache(descriptors, course_id, user, select_for_update)
+
+    def _query(self, model_class, **kwargs):
+        """
+        Queries model_class with **kwargs, optionally adding select_for_update if
+        self.select_for_update is set
+        """
+        query = model_class.objects
+        if self.select_for_update:
+            query = query.select_for_update()
+        query = query.filter(**kwargs)
+        return query
+
+    def _chunked_query(self, model_class, chunk_field, items, chunk_size=500, **kwargs):
+        """
+        Queries model_class with `chunk_field` set to chunks of size `chunk_size`,
+        and all other parameters from `**kwargs`
+
+        This works around a limitation in sqlite3 on the number of parameters
+        that can be put into a single query
+        """
+        res = chain.from_iterable(
+            self._query(model_class, **dict([(chunk_field, chunk)] + kwargs.items()))
+            for chunk in chunks(items, chunk_size)
+        )
+        return res
+
+    def _retrieve_fields(self, scope, fields):
+        """
+        Queries the database for all of the fields in the specified scope
+        """
+        if scope in (Scope.children, Scope.parent):
+            return []
+        elif scope == Scope.student_state:
+            return self._chunked_query(
+                StudentModule,
+                'module_state_key__in',
+                (descriptor.location.url() for descriptor in self.descriptors),
+                course_id=self.course_id,
+                student=self.user,
+            )
+        elif scope == Scope.content:
+            return self._chunked_query(
+                XModuleContentField,
+                'definition_id__in',
+                (descriptor.location.url() for descriptor in self.descriptors),
+                field_name__in=set(field.name for field in fields),
+            )
+        elif scope == Scope.settings:
+            return self._chunked_query(
+                XModuleSettingsField,
+                'usage_id__in',
+                (
+                    '%s-%s' % (self.course_id, descriptor.location.url())
+                    for descriptor in self.descriptors
+                ),
+                field_name__in=set(field.name for field in fields),
+            )
+        elif scope == Scope.student_preferences:
+            return self._chunked_query(
+                XModuleStudentPrefsField,
+                'module_type__in',
+                set(descriptor.location.category for descriptor in self.descriptors),
+                student=self.user,
+                field_name__in=set(field.name for field in fields),
+            )
+        elif scope == Scope.student_info:
+            self._query(
+                XModuleStudentInfoField,
+                student=self.user,
+                field_name__in=set(field.name for field in fields),
+            )
+        else:
+            raise InvalidScopeError(scope)
+
+    def _fields_to_cache(self):
+        """
+        Returns a map of scopes to fields in that scope that should be cached
+        """
+        scope_map = defaultdict(set)
+        for descriptor in self.descriptors:
+            for field in (descriptor.module_class.fields + descriptor.module_class.lms.fields):
+                scope_map[field.scope].add(field)
+        return scope_map
+
+    def _cache_key_from_kvs_key(self, key):
+        if key.scope == Scope.student_state:
+            return (key.scope, key.block_scope_id.url())
+        elif key.scope == Scope.content:
+            return (key.scope, key.block_scope_id.url(), key.field_name)
+        elif key.scope == Scope.settings:
+            return (key.scope, '%s-%s' % (self.course_id, key.block_scope_id.url()), key.field_name)
+        elif key.scope == Scope.student_preferences:
+            return (key.scope, key.block_scope_id, key.field_name)
+        elif key.scope == Scope.student_info:
+            return (key.scope, key.field_name)
+
+    def _cache_key_from_field_object(self, scope, field_object):
+        if scope == Scope.student_state:
+            return (scope, field_object.module_state_key)
+        elif scope == Scope.content:
+            return (scope, field_object.definition_id, field_object.field_name)
+        elif scope == Scope.settings:
+            return (scope, field_object.usage_id, field_object.field_name)
+        elif scope == Scope.student_preferences:
+            return (scope, field_object.module_type, field_object.field_name)
+        elif scope == Scope.student_info:
+            return (scope, field_object.field_name)
+
+    def find(self, key):
+        '''
+        Look for a model data object using an LmsKeyValueStore.Key object
+
+        key: An `LmsKeyValueStore.Key` object selecting the object to find
+
+        returns the found object, or None if the object doesn't exist
+        '''
+        return self.cache.get(self._cache_key_from_kvs_key(key))
+
+    def find_or_create(self, key):
+        '''
+        Find a model data object in this cache, or create it if it doesn't
+        exist
+        '''
+        field_object = self.find(key)
+
+        if field_object is not None:
+            return field_object
+
+        if key.scope == Scope.student_state:
+            field_object, _ = StudentModule.objects.get_or_create(
+                course_id=self.course_id,
+                student=self.user,
+                module_type=key.block_scope_id.category,
+                module_state_key=key.block_scope_id.url(),
+                defaults={'state': json.dumps({})},
+            )
+        elif key.scope == Scope.content:
+            field_object, _ = XModuleContentField.objects.get_or_create(
+                field_name=key.field_name,
+                definition_id=key.block_scope_id.url()
+            )
+        elif key.scope == Scope.settings:
+            field_object, _ = XModuleSettingsField.objects.get_or_create(
+                field_name=key.field_name,
+                usage_id='%s-%s' % (self.course_id, key.block_scope_id.url()),
+            )
+        elif key.scope == Scope.student_preferences:
+            field_object, _= XModuleStudentPrefsField.objects.get_or_create(
+                field_name=key.field_name,
+                module_type=key.block_scope_id,
+                student=self.user,
+            )
+        elif key.scope == Scope.student_info:
+            field_object, _ = XModuleStudentInfoField.objects.get_or_create(
+                field_name=key.field_name,
+                student=self.user,
+            )
+
+        cache_key = self._cache_key_from_kvs_key(key)
+        self.cache[cache_key] = field_object
+        return field_object
+
+
 class LmsKeyValueStore(KeyValueStore):
     """
     This KeyValueStore will read data from descriptor_model_data if it exists,
@@ -37,32 +267,9 @@ class LmsKeyValueStore(KeyValueStore):
 
     If the key isn't found in the expected table during a read or a delete, then a KeyError will be raised
     """
-    def __init__(self, course_id, user, descriptor_model_data, student_module_cache):
-        self._course_id = course_id
-        self._user = user
+    def __init__(self, descriptor_model_data, model_data_cache):
         self._descriptor_model_data = descriptor_model_data
-        self._student_module_cache = student_module_cache
-
-    def _student_module(self, key):
-        student_module = self._student_module_cache.lookup(
-            self._course_id, key.block_scope_id.category, key.block_scope_id.url()
-        )
-        return student_module
-
-    def _field_object(self, key):
-        if key.scope == Scope.content:
-            return XModuleContentField, {'field_name': key.field_name, 'definition_id': key.block_scope_id}
-        elif key.scope == Scope.settings:
-            return XModuleSettingsField, {
-                'field_name': key.field_name,
-                'usage_id': '%s-%s' % (self._course_id, key.block_scope_id)
-            }
-        elif key.scope == Scope.student_preferences:
-            return XModuleStudentPrefsField, {'field_name': key.field_name, 'student': self._user, 'module_type': key.block_scope_id}
-        elif key.scope == Scope.student_info:
-            return XModuleStudentInfoField, {'field_name': key.field_name, 'student': self._user}
-
-        raise InvalidScopeError(key.scope)
+        self._model_data_cache = model_data_cache
 
     def get(self, key):
         if key.field_name in self._descriptor_model_data:
@@ -71,74 +278,45 @@ class LmsKeyValueStore(KeyValueStore):
         if key.scope == Scope.parent:
             return None
 
-        if key.scope == Scope.student_state:
-            student_module = self._student_module(key)
-
-            if student_module is None:
-                raise KeyError(key.field_name)
-
-            return json.loads(student_module.state)[key.field_name]
-
-        scope_field_cls, search_kwargs = self._field_object(key)
-        try:
-            return json.loads(scope_field_cls.objects.get(**search_kwargs).value)
-        except scope_field_cls.DoesNotExist:
+        field_object = self._model_data_cache.find(key)
+        if field_object is None:
             raise KeyError(key.field_name)
 
+        if key.scope == Scope.student_state:
+            return json.loads(field_object.state)[key.field_name]
+        else:
+            return json.loads(field_object.value)
+
     def set(self, key, value):
         if key.field_name in self._descriptor_model_data:
             raise InvalidWriteError("Not allowed to overwrite descriptor model data", key.field_name)
 
+        field_object = self._model_data_cache.find_or_create(key)
+
         if key.scope == Scope.student_state:
-            student_module = self._student_module(key)
-            if student_module is None:
-                student_module = StudentModule(
-                    course_id=self._course_id,
-                    student=self._user,
-                    module_type=key.block_scope_id.category,
-                    module_state_key=key.block_scope_id.url(),
-                    state=json.dumps({})
-                )
-                self._student_module_cache.append(student_module)
-            state = json.loads(student_module.state)
+            state = json.loads(field_object.state)
             state[key.field_name] = value
-            student_module.state = json.dumps(state)
-            student_module.save()
-            return
-
-        scope_field_cls, search_kwargs = self._field_object(key)
-        json_value = json.dumps(value)
-        field, created = scope_field_cls.objects.select_for_update().get_or_create(
-            defaults={'value': json_value},
-            **search_kwargs
-        )
-        if not created:
-            field.value = json_value
-            field.save()
+            field_object.state = json.dumps(state)
+        else:
+            field_object.value = json.dumps(value)
+
+        field_object.save()
 
     def delete(self, key):
         if key.field_name in self._descriptor_model_data:
             raise InvalidWriteError("Not allowed to deleted descriptor model data", key.field_name)
 
-        if key.scope == Scope.student_state:
-            student_module = self._student_module(key)
-
-            if student_module is None:
-                raise KeyError(key.field_name)
-
-            state = json.loads(student_module.state)
-            del state[key.field_name]
-            student_module.state = json.dumps(state)
-            student_module.save()
-            return
-
-        scope_field_cls, search_kwargs = self._field_object(key)
-        print scope_field_cls, search_kwargs
-        query = scope_field_cls.objects.filter(**search_kwargs)
-        if not query.exists():
+        field_object = self._model_data_cache.find(key)
+        if field_object is None:
             raise KeyError(key.field_name)
 
-        query.delete()
+        if key.scope == Scope.student_state:
+            state = json.loads(field_object.state)
+            del state[key.field_name]
+            field_object.state = json.dumps(state)
+            field_object.save()
+        else:
+            field_object.delete()
 
 
 LmsUsage = namedtuple('LmsUsage', 'id, def_id')

lms/djangoapps/courseware/models.py

@@ -198,106 +198,3 @@ class XModuleStudentInfoField(models.Model):
     def __unicode__(self):
         return unicode(repr(self))
 
-# TODO (cpennington): Remove these once the LMS switches to using XModuleDescriptors
-
-
-class StudentModuleCache(object):
-    """
-    A cache of StudentModules for a specific student
-    """
-    def __init__(self, course_id, user, descriptors, select_for_update=False):
-        '''
-        Find any StudentModule objects that are needed by any descriptor
-        in descriptors. Avoids making multiple queries to the database.
-        Note: Only modules that have store_state = True or have shared
-        state will have a StudentModule.
-
-        Arguments
-        user: The user for which to fetch maching StudentModules
-        descriptors: An array of XModuleDescriptors.
-        select_for_update: Flag indicating whether the rows should be locked until end of transaction
-        '''
-        if user.is_authenticated():
-            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
-            self.cache = []
-            chunk_size = 500
-            for id_chunk in [module_ids[i:i + chunk_size] for i in xrange(0, len(module_ids), chunk_size)]:
-                if select_for_update:
-                    self.cache.extend(StudentModule.objects.select_for_update().filter(
-                        course_id=course_id,
-                        student=user,
-                        module_state_key__in=id_chunk)
-                    )
-                else:
-                    self.cache.extend(StudentModule.objects.filter(
-                        course_id=course_id,
-                        student=user,
-                        module_state_key__in=id_chunk)
-                    )
-
-        else:
-            self.cache = []
-
-    @classmethod
-    def cache_for_descriptor_descendents(cls, course_id, user, descriptor, depth=None,
-                                         descriptor_filter=lambda descriptor: True,
-                                         select_for_update=False):
-        """
-        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
-        """
-
-        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(get_child_descriptors(child, new_depth, descriptor_filter))
-
-            return descriptors
-
-        descriptors = get_child_descriptors(descriptor, depth, descriptor_filter)
-
-        return StudentModuleCache(course_id, user, descriptors, select_for_update)
-
-    def _get_module_state_keys(self, descriptors):
-        '''
-        Get a list of the state_keys needed for StudentModules
-        required for this module descriptor
-
-        descriptor_filter is a function that accepts a descriptor and return wether the StudentModule
-            should be cached
-        '''
-        return [descriptor.location.url() for descriptor in descriptors if descriptor.stores_state]
-
-    def lookup(self, course_id, module_type, module_state_key):
-        '''
-        Look for a student module with the given course_id, type, and id in the cache.
-
-        cache -- list of student modules
-
-        returns first found object, or None
-        '''
-        for o in self.cache:
-            if (o.course_id == course_id and
-                o.module_type == module_type and
-                o.module_state_key == module_state_key):
-                return o
-        return None
-
-    def append(self, student_module):
-        self.cache.append(student_module)

lms/djangoapps/courseware/module_render.py

@@ -17,7 +17,7 @@ from capa.xqueue_interface import XQueueInterface
 from capa.chem import chemcalc
 from courseware.access import has_access
 from mitxmako.shortcuts import render_to_string
-from models import StudentModule, StudentModuleCache
+from models import StudentModule
 from psychometrics.psychoanalyze import make_psychometrics_data_update_handler
 from static_replace import replace_urls
 from xmodule.errortracker import exc_info_to_str
@@ -28,7 +28,7 @@ from xmodule.x_module import ModuleSystem
 from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor
 from xblock.runtime import DbModel
 from xmodule_modifiers import replace_course_urls, replace_static_urls, add_histogram, wrap_xmodule
-from .model_data import LmsKeyValueStore, LmsUsage
+from .model_data import LmsKeyValueStore, LmsUsage, ModelDataCache
 
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from statsd import statsd
@@ -60,7 +60,7 @@ def make_track_function(request):
     return f
 
 
-def toc_for_course(user, request, course, active_chapter, active_section):
+def toc_for_course(user, request, course, active_chapter, active_section, model_data_cache):
     '''
     Create a table of contents from the module store
 
@@ -80,11 +80,11 @@ def toc_for_course(user, request, course, active_chapter, active_section):
 
     NOTE: assumes that if we got this far, user has access to course.  Returns
     None if this is not the case.
+
+    model_data_cache must include data from the course module and 2 levels of its descendents
     '''
 
-    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
-        course.id, user, course, depth=2)
-    course_module = get_module(user, request, course.location, student_module_cache, course.id)
+    course_module = get_module(user, request, course.location, model_data_cache, course.id)
     if course_module is None:
         return None
 
@@ -115,7 +115,7 @@ def toc_for_course(user, request, course, active_chapter, active_section):
     return chapters
 
 
-def get_module(user, request, location, student_module_cache, course_id,
+def get_module(user, request, location, model_data_cache, course_id,
                position=None, not_found_ok = False, wrap_xmodule_display=True,
                grade_bucket_type=None):
     """
@@ -128,7 +128,7 @@ def get_module(user, request, location, student_module_cache, course_id,
       - request               : current django HTTPrequest.  Note: request.user isn't used for anything--all auth
                                 and such works based on user.
       - location              : A Location-like object identifying the module to load
-      - student_module_cache  : a StudentModuleCache
+      - model_data_cache      : a ModelDataCache
       - course_id             : the course_id in the context of which to load module
       - position              : extra information from URL for user-specified
                                 position within module
@@ -138,7 +138,7 @@ def get_module(user, request, location, student_module_cache, course_id,
     if possible.  If not possible, return None.
     """
     try:
-        return _get_module(user, request, location, student_module_cache, course_id, position, wrap_xmodule_display)
+        return _get_module(user, request, location, model_data_cache, course_id, position, wrap_xmodule_display)
     except ItemNotFoundError:
         if not not_found_ok:
             log.exception("Error in get_module")
@@ -149,7 +149,7 @@ def get_module(user, request, location, student_module_cache, course_id,
         return None
 
 
-def _get_module(user, request, location, student_module_cache, course_id,
+def _get_module(user, request, location, model_data_cache, course_id,
                 position=None, wrap_xmodule_display=True, grade_bucket_type=None):
     """
     Actually implement get_module.  See docstring there for details.
@@ -204,11 +204,11 @@ def _get_module(user, request, location, student_module_cache, course_id,
         Delegate to get_module.  It does an access check, so may return None
         """
         return get_module(user, request, location,
-                                       student_module_cache, course_id, position)
+                                       model_data_cache, course_id, position)
 
     def xblock_model_data(descriptor):
         return DbModel(
-            LmsKeyValueStore(course_id, user, descriptor._model_data, student_module_cache),
+            LmsKeyValueStore(descriptor._model_data, model_data_cache),
             descriptor.module_class,
             user.id,
             LmsUsage(location, location)
@@ -218,18 +218,13 @@ def _get_module(user, request, location, student_module_cache, course_id,
         if event.get('event_name') != 'grade':
             return
 
-        student_module = student_module_cache.lookup(
-            course_id, descriptor.location.category, descriptor.location.url()
+        student_module, created = StudentModule.objects.get_or_create(
+            course_id=course_id,
+            student=user,
+            module_type=descriptor.location.category,
+            module_state_key=descriptor.location.url(),
+            defaults={'state': '{}'},
         )
-        if student_module is None:
-            student_module = StudentModule(
-                course_id=course_id,
-                student=user,
-                module_type=descriptor.location.category,
-                module_state_key=descriptor.location.url(),
-                state=json.dumps({})
-            )
-            student_module_cache.append(student_module)
         student_module.grade = event.get('value')
         student_module.max_grade = event.get('max_value')
         student_module.save()
@@ -335,9 +330,9 @@ def xqueue_callback(request, course_id, userid, id, dispatch):
     # Retrieve target StudentModule
     user = User.objects.get(id=userid)
 
-    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(course_id,
+    model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id,
         user, modulestore().get_instance(course_id, id), depth=0, select_for_update=True)
-    instance = get_module(user, request, id, student_module_cache, course_id, grade_bucket_type='xqueue')
+    instance = get_module(user, request, id, model_data_cache, course_id, grade_bucket_type='xqueue')
     if instance is None:
         log.debug("No module {0} for user {1}--access denied?".format(id, user))
         raise Http404
@@ -394,10 +389,10 @@ def modx_dispatch(request, dispatch, location, course_id):
                     return HttpResponse(json.dumps({'success': file_too_big_msg}))
             p[fileinput_id] = inputfiles
 
-    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(course_id,
+    model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id,
         request.user, modulestore().get_instance(course_id, location))
 
-    instance = get_module(request.user, request, location, student_module_cache, course_id, grade_bucket_type='ajax')
+    instance = get_module(request.user, request, location, model_data_cache, course_id, grade_bucket_type='ajax')
     if instance is None:
         # Either permissions just changed, or someone is trying to be clever
         # and load something they shouldn't have access to.

lms/djangoapps/courseware/tests/test_model_data.py

@@ -5,18 +5,26 @@ from django.contrib.auth.models import User
 
 from functools import partial
 
-from courseware.model_data import LmsKeyValueStore, InvalidWriteError, InvalidScopeError
-from courseware.models import StudentModule, XModuleContentField, XModuleSettingsField, XModuleStudentInfoField, XModuleStudentPrefsField, StudentModuleCache
+from courseware.model_data import LmsKeyValueStore, InvalidWriteError, InvalidScopeError, ModelDataCache
+from courseware.models import StudentModule, XModuleContentField, XModuleSettingsField, XModuleStudentInfoField, XModuleStudentPrefsField
 from xblock.core import Scope, BlockScope
 from xmodule.modulestore import Location
 
 from django.test import TestCase
 
 
-def mock_descriptor():
+def mock_field(scope, name):
+    field = Mock()
+    field.scope = scope
+    field.name = name
+    return field
+
+def mock_descriptor(fields=[], lms_fields=[]):
     descriptor = Mock()
     descriptor.stores_state = True
     descriptor.location = location('def_id')
+    descriptor.module_class.fields = fields
+    descriptor.module_class.lms.fields = lms_fields
     return descriptor
 
 location = partial(Location, 'i4x', 'edX', 'test_course', 'problem')
@@ -85,7 +93,7 @@ class TestDescriptorFallback(TestCase):
             'field_a': 'content',
             'field_b': 'settings',
         }
-        self.kvs = LmsKeyValueStore(course_id, UserFactory.build(), self.desc_md, None)
+        self.kvs = LmsKeyValueStore(self.desc_md, None)
 
     def test_get_from_descriptor(self):
         self.assertEquals('content', self.kvs.get(content_key('field_a')))
@@ -103,13 +111,11 @@ class TestDescriptorFallback(TestCase):
         self.assertEquals('settings', self.desc_md['field_b'])
 
 
-class TestStudentStateFields(TestCase):
-    pass
-
 class TestInvalidScopes(TestCase):
     def setUp(self):
         self.desc_md = {}
-        self.kvs = LmsKeyValueStore(course_id, UserFactory.build(), self.desc_md, None)
+        self.mdc = ModelDataCache([mock_descriptor([mock_field(Scope.student_state, 'a_field')])], course_id, self.user)
+        self.kvs = LmsKeyValueStore(self.desc_md, self.mdc)
 
     def test_invalid_scopes(self):
         for scope in (Scope(student=True, block=BlockScope.DEFINITION),
@@ -123,12 +129,10 @@ class TestInvalidScopes(TestCase):
 class TestStudentModuleStorage(TestCase):
 
     def setUp(self):
-        student_module = StudentModuleFactory.create(state=json.dumps({'a_field': 'a_value'}))
-        self.user = student_module.student
         self.desc_md = {}
-        self.smc = StudentModuleCache(course_id, self.user, [mock_descriptor()])
-        self.kvs = LmsKeyValueStore(course_id, self.user, self.desc_md, self.smc)
-
+        self.mdc = Mock()
+        self.mdc.find.return_value.state = json.dumps({'a_field': 'a_value'})
+        self.kvs = LmsKeyValueStore(self.desc_md, self.mdc)
 
     def test_get_existing_field(self):
         "Test that getting an existing field in an existing StudentModule works"
@@ -154,7 +158,7 @@ class TestStudentModuleStorage(TestCase):
         "Test that deleting an existing field removes it from the StudentModule"
         self.kvs.delete(student_state_key('a_field'))
         self.assertEquals(1, StudentModule.objects.all().count())
-        self.assertEquals({}, json.loads(StudentModule.objects.all()[0].state))
+        self.assertEquals({}, self.mdc.find.return_value.state)
 
     def test_delete_missing_field(self):
         "Test that deleting a missing field from an existing StudentModule raises a KeyError"
@@ -167,8 +171,8 @@ class TestMissingStudentModule(TestCase):
     def setUp(self):
         self.user = UserFactory.create()
         self.desc_md = {}
-        self.smc = StudentModuleCache(course_id, self.user, [mock_descriptor()])
-        self.kvs = LmsKeyValueStore(course_id, self.user, self.desc_md, self.smc)
+        self.mdc = ModelDataCache([mock_descriptor()], course_id, self.user)
+        self.kvs = LmsKeyValueStore(self.desc_md, self.mdc)
 
     def test_get_field_from_missing_student_module(self):
         "Test that getting a field from a missing StudentModule raises a KeyError"
@@ -176,12 +180,12 @@ class TestMissingStudentModule(TestCase):
 
     def test_set_field_in_missing_student_module(self):
         "Test that setting a field in a missing StudentModule creates the student module"
-        self.assertEquals(0, len(self.smc.cache))
+        self.assertEquals(0, len(self.mdc.cache))
         self.assertEquals(0, StudentModule.objects.all().count())
 
         self.kvs.set(student_state_key('a_field'), 'a_value')
 
-        self.assertEquals(1, len(self.smc.cache))
+        self.assertEquals(1, len(self.mdc.cache))
         self.assertEquals(1, StudentModule.objects.all().count())
 
         student_module = StudentModule.objects.all()[0]
@@ -201,8 +205,8 @@ class TestSettingsStorage(TestCase):
         settings = SettingsFactory.create()
         self.user = UserFactory.create()
         self.desc_md = {}
-        self.smc = StudentModuleCache(course_id, self.user, [])
-        self.kvs = LmsKeyValueStore(course_id, self.user, self.desc_md, self.smc)
+        self.mdc = ModelDataCache([mock_descriptor()], course_id, self.user)
+        self.kvs = LmsKeyValueStore(self.desc_md, self.mdc)
 
     def test_get_existing_field(self):
         "Test that getting an existing field in an existing SettingsField works"
@@ -242,8 +246,8 @@ class TestContentStorage(TestCase):
         content = ContentFactory.create()
         self.user = UserFactory.create()
         self.desc_md = {}
-        self.smc = StudentModuleCache(course_id, self.user, [])
-        self.kvs = LmsKeyValueStore(course_id, self.user, self.desc_md, self.smc)
+        self.mdc = ModelDataCache([mock_descriptor()], course_id, self.user)
+        self.kvs = LmsKeyValueStore(self.desc_md, self.mdc)
 
     def test_get_existing_field(self):
         "Test that getting an existing field in an existing ContentField works"
@@ -283,8 +287,11 @@ class TestStudentPrefsStorage(TestCase):
         student_pref = StudentPrefsFactory.create()
         self.user = student_pref.student
         self.desc_md = {}
-        self.smc = StudentModuleCache(course_id, self.user, [])
-        self.kvs = LmsKeyValueStore(course_id, self.user, self.desc_md, self.smc)
+        self.mdc = ModelDataCache([mock_descriptor([
+            mock_field(Scope.student_preferences, 'student_pref_field'),
+            mock_field(Scope.student_preferences, 'not_student_pref_field'),
+        ])], course_id, self.user)
+        self.kvs = LmsKeyValueStore(self.desc_md, self.mdc)
 
     def test_get_existing_field(self):
         "Test that getting an existing field in an existing StudentPrefsField works"
@@ -309,7 +316,6 @@ class TestStudentPrefsStorage(TestCase):
 
     def test_delete_existing_field(self):
         "Test that deleting an existing field removes it"
-        print list(XModuleStudentPrefsField.objects.all())
         self.kvs.delete(student_prefs_key('student_pref_field'))
         self.assertEquals(0, XModuleStudentPrefsField.objects.all().count())
 
@@ -325,8 +331,11 @@ class TestStudentInfoStorage(TestCase):
         student_info = StudentInfoFactory.create()
         self.user = student_info.student
         self.desc_md = {}
-        self.smc = StudentModuleCache(course_id, self.user, [])
-        self.kvs = LmsKeyValueStore(course_id, self.user, self.desc_md, self.smc)
+        self.mdc = ModelDataCache([mock_descriptor([
+            mock_field(Scope.student_info, 'student_info_field'),
+            mock_field(Scope.student_info, 'not_student_info_field'),
+        ])], course_id, self.user)
+        self.kvs = LmsKeyValueStore(self.desc_md, self.mdc)
 
     def test_get_existing_field(self):
         "Test that getting an existing field in an existing StudentInfoField works"

lms/djangoapps/courseware/tests/tests.py

@@ -23,7 +23,7 @@ import xmodule.modulestore.django
 # Need access to internal func to put users in the right group
 from courseware import grades
 from courseware.access import _course_staff_group_name
-from courseware.models import StudentModuleCache
+from courseware.model_data import ModelDataCache
 
 from student.models import Registration
 from xmodule.modulestore.django import modulestore
@@ -705,27 +705,27 @@ class TestCourseGrader(PageLoader):
         self.factory = RequestFactory()
     
     def get_grade_summary(self):
-        student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+        model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
             self.graded_course.id, self.student_user, self.graded_course)
         
         fake_request = self.factory.get(reverse('progress',
                                        kwargs={'course_id': self.graded_course.id}))
         
         return grades.grade(self.student_user, fake_request, 
-                            self.graded_course, student_module_cache)
+                            self.graded_course, model_data_cache)
     
     def get_homework_scores(self):
         return self.get_grade_summary()['totaled_scores']['Homework']
     
     def get_progress_summary(self):
-        student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+        model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
             self.graded_course.id, self.student_user, self.graded_course)
         
         fake_request = self.factory.get(reverse('progress',
                                        kwargs={'course_id': self.graded_course.id}))
 
         progress_summary = grades.progress_summary(self.student_user, fake_request, 
-                                                   self.graded_course, student_module_cache)
+                                                   self.graded_course, model_data_cache)
         return progress_summary
         
     def check_grade_percent(self, percent):

lms/djangoapps/courseware/views.py

@@ -23,7 +23,7 @@ from courseware import grades
 from courseware.access import has_access
 from courseware.courses import (get_course_with_access, get_courses_by_university)
 import courseware.tabs as tabs
-from courseware.models import StudentModuleCache
+from courseware.model_data import ModelDataCache
 from module_render import toc_for_course, get_module
 from student.models import UserProfile
 
@@ -81,7 +81,7 @@ def courses(request):
     return render_to_response("courses.html", {'universities': universities})
 
 
-def render_accordion(request, course, chapter, section):
+def render_accordion(request, course, chapter, section, model_data_cache):
     ''' Draws navigation bar. Takes current position in accordion as
         parameter.
 
@@ -92,7 +92,7 @@ def render_accordion(request, course, chapter, section):
         Returns the html string'''
 
     # grab the table of contents
-    toc = toc_for_course(request.user, request, course, chapter, section)
+    toc = toc_for_course(request.user, request, course, chapter, section, model_data_cache)
 
     context = dict([('toc', toc),
                     ('course_id', course.id),
@@ -191,24 +191,21 @@ def index(request, course_id, chapter=None, section=None,
         return redirect(reverse('about_course', args=[course.id]))
 
     try:
-        student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+        model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
             course.id, request.user, course, depth=2)
 
-        # Has this student been in this course before?
-        first_time = student_module_cache.lookup(course_id, 'course', course.location.url()) is None
-
-        course_module = get_module(request.user, request, course.location, student_module_cache, course.id)
+        course_module = get_module(request.user, request, course.location, model_data_cache, course.id)
         if course_module is None:
             log.warning('If you see this, something went wrong: if we got this'
                         ' far, should have gotten a course module for this user')
             return redirect(reverse('about_course', args=[course.id]))
 
         if chapter is None:
-            return redirect_to_course_position(course_module, first_time)
+            return redirect_to_course_position(course_module, course_module.first_time_user)
 
         context = {
             'csrf': csrf(request)['csrf_token'],
-            'accordion': render_accordion(request, course, chapter, section),
+            'accordion': render_accordion(request, course, chapter, section, model_data_cache),
             'COURSE_TITLE': course.title,
             'course': course,
             'init': '',
@@ -224,7 +221,7 @@ def index(request, course_id, chapter=None, section=None,
             raise Http404
 
         chapter_module = get_module(request.user, request, chapter_descriptor.location,
-                                    student_module_cache, course_id)
+                                    model_data_cache, course_id)
         if chapter_module is None:
             # User may be trying to access a chapter that isn't live yet
             raise Http404
@@ -235,11 +232,11 @@ def index(request, course_id, chapter=None, section=None,
                 # Specifically asked-for section doesn't exist
                 raise Http404
 
-            section_student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+            section_model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
                 course_id, request.user, section_descriptor)
             section_module = get_module(request.user, request,
                                 section_descriptor.location,
-                                section_student_module_cache, course_id, position)
+                                section_model_data_cache, course_id, position)
             if section_module is None:
                 # User may be trying to be clever and access something
                 # they don't have access to.
@@ -491,12 +488,12 @@ def progress(request, course_id, student_id=None):
     # additional DB lookup (this kills the Progress page in particular).
     student = User.objects.prefetch_related("groups").get(id=student.id)
 
-    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+    model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
         course_id, student, course)
 
     courseware_summary = grades.progress_summary(student, request, course,
-                                                 student_module_cache)
-    grade_summary = grades.grade(student, request, course, student_module_cache)
+                                                 model_data_cache)
+    grade_summary = grades.grade(student, request, course, model_data_cache)
 
     if courseware_summary is None:
         #This means the student didn't have access to the course (which the instructor requested)

lms/templates/courseware/info.html

@@ -27,20 +27,20 @@ $(document).ready(function(){
     % if user.is_authenticated():
     <section class="updates">
       <h1>Course Updates &amp; News</h1>
-      ${get_course_info_section(request, cache, course, 'updates')}
+      ${get_course_info_section(request, course, 'updates')}
     </section>
     <section aria-label="Handout Navigation" class="handouts">
       <h1>${course.info_sidebar_name}</h1>
-      ${get_course_info_section(request, cache, course, 'handouts')}
+      ${get_course_info_section(request, course, 'handouts')}
     </section>
     % else:
     <section class="updates">
       <h1>Course Updates &amp; News</h1>
-      ${get_course_info_section(request, cache, course, 'guest_updates')}
+      ${get_course_info_section(request, course, 'guest_updates')}
     </section>
     <section aria-label="Handout Navigation" class="handouts">
       <h1>Course Handouts</h1>
-      ${get_course_info_section(request, cache, course, 'guest_handouts')}
+      ${get_course_info_section(request, course, 'guest_handouts')}
     </section>
     % endif
   </div>