Merge pull request #411 from MITx/kimth/xqueue_callback_blocking

Xqueue callback acquires lock on StudentModule to avoid race condition

lms/djangoapps/courseware/models.py

@@ -67,7 +67,7 @@ class StudentModuleCache(object):
     """
     A cache of StudentModules for a specific student
     """
-    def __init__(self, user, descriptors):
+    def __init__(self, user, descriptors, acquire_lock=False):
         '''
         Find any StudentModule objects that are needed by any descriptor
         in descriptors. Avoids making multiple queries to the database.
@@ -86,17 +86,23 @@ class StudentModuleCache(object):
             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)]:
-                self.cache.extend(StudentModule.objects.filter(
-                    student=user,
-                    module_state_key__in=id_chunk)
-                )
+                if acquire_lock:
+                    self.cache.extend(StudentModule.objects.select_for_update().filter(
+                        student=user,
+                        module_state_key__in=id_chunk)
+                    )
+                else:
+                    self.cache.extend(StudentModule.objects.filter(
+                        student=user,
+                        module_state_key__in=id_chunk)
+                    )
 
         else:
             self.cache = []
     
     
     @classmethod
-    def cache_for_descriptor_descendents(cls, user, descriptor, depth=None, descriptor_filter=lambda descriptor: True):
+    def cache_for_descriptor_descendents(cls, user, descriptor, depth=None, descriptor_filter=lambda descriptor: True, acquire_lock=False):
         """
         descriptor: An XModuleDescriptor
         depth is the number of levels of descendent modules to load StudentModules for, in addition to
@@ -122,7 +128,7 @@ class StudentModuleCache(object):
         
         descriptors = get_child_descriptors(descriptor, depth, descriptor_filter)
         
-        return StudentModuleCache(user, descriptors)    
+        return StudentModuleCache(user, descriptors, acquire_lock)
     
     def _get_module_state_keys(self, descriptors):
         '''

lms/djangoapps/courseware/module_render.py

@@ -160,7 +160,6 @@ def get_module(user, request, location, student_module_cache, position=None):
     instance_state = instance_module.state if instance_module is not None else None
     shared_state = shared_module.state if shared_module is not None else None
 
-    # TODO (vshnayder): fix hardcoded urls (use reverse)
     # Setup system context for module instance
 
     ajax_url = reverse('modx_dispatch', 
@@ -169,8 +168,6 @@ def get_module(user, request, location, student_module_cache, position=None):
                                    dispatch=''),
                        )
 
-    # ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/'
-
     # Fully qualified callback URL for external queueing system
     xqueue_callback_url  = request.build_absolute_uri('/')[:-1] # Trailing slash provided by reverse
     xqueue_callback_url += reverse('xqueue_callback',
@@ -304,7 +301,8 @@ 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(user, modulestore().get_item(id))
+    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+        user, modulestore().get_item(id), depth=0, acquire_lock=True)
     instance = get_module(user, request, id, student_module_cache)
     instance_module = get_instance_module(user, instance, student_module_cache)