Merge pull request #185 from MITx/kimth/lms-coderesponse

Kimth/lms coderesponse

common/lib/capa/capa/capa_problem.py

@@ -118,6 +118,9 @@ class LoncapaProblem(object):
         # the dict has keys = xml subtree of Response, values = Response instance
         self._preprocess_problem(self.tree)
 
+        if not self.student_answers: # True when student_answers is an empty dict
+            self.set_initial_display()
+
     def do_reset(self):
         '''
         Reset internal state to unfinished, with no answers
@@ -126,6 +129,14 @@ class LoncapaProblem(object):
         self.correct_map = CorrectMap()
         self.done = False
 
+    def set_initial_display(self):
+        initial_answers = dict()
+        for responder in self.responders.values():
+            if hasattr(responder,'get_initial_display'):
+                initial_answers.update(responder.get_initial_display())
+
+        self.student_answers = initial_answers
+
     def __unicode__(self):
         return u"LoncapaProblem ({0})".format(self.problem_id)
 
@@ -180,14 +191,31 @@ class LoncapaProblem(object):
             return {'score': correct,
                     'total': self.get_max_score()}
 
-    def update_score(self, score_msg):
-        newcmap = CorrectMap()
+    def update_score(self, score_msg, queuekey):
+        '''
+        Deliver grading response (e.g. from async code checking) to 
+            the specific ResponseType that requested grading 
+         
+        Returns an updated CorrectMap
+        '''
+        cmap = CorrectMap()
+        cmap.update(self.correct_map)
         for responder in self.responders.values():
-            if hasattr(responder,'update_score'): # Is this the best way to implement 'update_score' for CodeResponse?
-                results = responder.update_score(score_msg)         
-                newcmap.update(results)
-        self.correct_map = newcmap
-        return newcmap
+            if hasattr(responder,'update_score'):
+                # Each LoncapaResponse will update the specific entries of 'cmap' that it's responsible for
+                cmap = responder.update_score(score_msg, cmap, queuekey)
+        self.correct_map.set_dict(cmap.get_dict())
+        return cmap 
+
+    def is_queued(self):
+        '''
+        Returns True if any part of the problem has been submitted to an external queue
+        '''
+        queued = False
+        for answer_id in self.correct_map:
+            if self.correct_map.is_queued(answer_id):
+                queued = True
+        return queued
 
     def grade_answers(self, answers):
         '''
@@ -457,7 +485,7 @@ class LoncapaProblem(object):
         self.responder_answers = {}
         for response in self.responders.keys():
             try:
-                self.responder_answers[response] = responder.get_answers()
+                self.responder_answers[response] = self.responders[response].get_answers()
             except:
                 log.debug('responder %s failed to properly return get_answers()' % self.responders[response]) # FIXME
                 raise

common/lib/capa/capa/correctmap.py

@@ -14,6 +14,7 @@ class CorrectMap(object):
     - msg         : string (may have HTML) giving extra message response (displayed below textline or textbox)
     - hint        : string (may have HTML) giving optional hint (displayed below textline or textbox, above msg)
     - hintmode    : one of (None,'on_request','always') criteria for displaying hint
+    - queuekey    : a random integer for xqueue_callback verification
 
     Behaves as a dict.
     '''
@@ -29,13 +30,14 @@ class CorrectMap(object):
     def __iter__(self):
         return self.cmap.__iter__()
 
-    def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None):
+    def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuekey=None):
         if answer_id is not None:
             self.cmap[answer_id] = {'correctness': correctness,
                                     'npoints': npoints,
                                     'msg': msg,
                                     'hint' : hint,
                                     'hintmode' : hintmode,
+                                    'queuekey' : queuekey, 
                                     }
 
     def __repr__(self):
@@ -63,6 +65,12 @@ class CorrectMap(object):
         if answer_id in self.cmap: return self.cmap[answer_id]['correctness'] == 'correct'
         return None
 
+    def is_queued(self,answer_id):
+        return answer_id in self.cmap and self.cmap[answer_id]['queuekey'] is not None
+
+    def is_right_queuekey(self, answer_id, test_key):
+        return answer_id in self.cmap and self.cmap[answer_id]['queuekey'] == test_key
+
     def get_npoints(self,answer_id):
         if self.is_correct(answer_id):
             npoints = self.cmap[answer_id].get('npoints',1)	# default to 1 point if correct

common/lib/capa/capa/responsetypes.py

@@ -8,6 +8,7 @@ Used by capa_problem.py
 '''
 
 # standard library imports
+import hashlib
 import inspect
 import json
 import logging
@@ -16,9 +17,9 @@ import numpy
 import random
 import re
 import requests
+import time
 import traceback
 import abc
-import time
 
 # specific library imports
 from calc import evaluator, UndefinedVariable
@@ -696,7 +697,10 @@ class SymbolicResponse(CustomResponse):
 
 class CodeResponse(LoncapaResponse):
     ''' 
-    Grade student code using an external server
+    Grade student code using an external server, called 'xqueue'
+    In contrast to ExternalResponse, CodeResponse has following behavior:
+        1) Goes through a queueing system
+        2) Does not do external request for 'get_answers'
     '''
 
     response_tag = 'coderesponse'
@@ -704,7 +708,7 @@ class CodeResponse(LoncapaResponse):
 
     def setup_response(self):
         xml = self.xml
-        self.url = xml.get('url') or "http://ec2-50-16-59-149.compute-1.amazonaws.com/xqueue/submit/" # FIXME -- hardcoded url
+        self.url = xml.get('url', "http://ec2-50-16-59-149.compute-1.amazonaws.com/xqueue/submit/") # FIXME -- hardcoded url
 
         answer = xml.find('answer')
         if answer is not None:
@@ -722,9 +726,27 @@ class CodeResponse(LoncapaResponse):
 
         self.tests = xml.get('tests')
 
+        # Extract 'answer' and 'initial_display' from XML. Note that the code to be exec'ed here is:
+        #   (1) Internal edX code, i.e. NOT student submissions, and
+        #   (2) The code should only define the strings 'initial_display', 'answer', 'preamble', 'test_program'
+        #           following the 6.01 problem definition convention
+        penv = {}
+        penv['__builtins__'] = globals()['__builtins__']
+        try:
+            exec(self.code,penv,penv)
+        except Exception as err:
+            log.error('Error in CodeResponse %s: Error in problem reference code' % err)
+            raise Exception(err)
+        try:
+            self.answer = penv['answer']
+            self.initial_display = penv['initial_display']
+        except Exception as err:
+            log.error("Error in CodeResponse %s: Problem reference code does not define 'answer' and/or 'initial_display' in <answer>...</answer>" % err)
+            raise Exception(err)
+
     def get_score(self, student_answers):
         idset = sorted(self.answer_ids)
-        
+
         try:
             submission = [student_answers[k] for k in idset]
         except Exception as err:
@@ -734,12 +756,16 @@ class CodeResponse(LoncapaResponse):
         self.context.update({'submission': submission})
         extra_payload = {'edX_student_response': json.dumps(submission)}
 
-        # Should do something -- like update the problem state -- based on the queue response
-        r = self._send_to_queue(extra_payload)
-        
-        return CorrectMap() 
+        r, queuekey = self._send_to_queue(extra_payload) # TODO: Perform checks on the xqueue response
 
-    def update_score(self, score_msg):
+        # Non-null CorrectMap['queuekey'] indicates that the problem has been submitted
+        cmap = CorrectMap()
+        for answer_id in idset:
+            cmap.set(answer_id, queuekey=queuekey, msg='Submitted to queue')
+
+        return cmap
+
+    def update_score(self, score_msg, oldcmap, queuekey):
         # Parse 'score_msg' as XML
         try:
             rxml = etree.fromstring(score_msg)
@@ -749,7 +775,6 @@ class CodeResponse(LoncapaResponse):
 
         # The following process is lifted directly from ExternalResponse
         idset = sorted(self.answer_ids)
-        cmap = CorrectMap()
         ad = rxml.find('awarddetail').text
         admap = {'EXACT_ANS':'correct',         # TODO: handle other loncapa responses
                  'WRONG_FORMAT': 'incorrect',
@@ -758,41 +783,41 @@ class CodeResponse(LoncapaResponse):
         if ad in admap:
             self.context['correct'][0] = admap[ad]
 
-        # create CorrectMap
-        for key in idset:
-            idx = idset.index(key)
-            msg = rxml.find('message').text.replace('&nbsp;','&#160;') if idx==0 else None
-            cmap.set(key, self.context['correct'][idx], msg=msg)
+        # Replace 'oldcmap' with new grading results if queuekey matches
+        #   If queuekey does not match, we keep waiting for the score_msg that will match
+        for answer_id in idset:
+            if oldcmap.is_right_queuekey(answer_id, queuekey): # If answer_id is not queued, will return False
+                idx = idset.index(answer_id)
+                msg = rxml.find('message').text.replace('&nbsp;','&#160;') if idx==0 else None
+                oldcmap.set(answer_id, self.context['correct'][idx], msg=msg, queuekey=None) # Queuekey is consumed 
+            else: # Queuekey does not match
+                log.debug('CodeResponse: queuekey %d does not match for answer_id=%s.' % (queuekey, answer_id)) 
         
-        return cmap
+        return oldcmap 
 
     # CodeResponse differentiates from ExternalResponse in the behavior of 'get_answers'. CodeResponse.get_answers
     #   does NOT require a queue submission, and the answer is computed (extracted from problem XML) locally.
     def get_answers(self):
-        # Extract the CodeResponse answer from XML
-        penv = {}
-        penv['__builtins__'] = globals()['__builtins__']
-        try:
-            exec(self.code,penv,penv)
-        except Exception as err:
-            log.error('Error in CodeResponse %s: Error in problem reference code' % err)
-            raise Exception(err)
-        try:
-            ans = penv['answer']
-        except Exception as err:
-            log.error('Error in CodeResponse %s: Problem reference code does not define answer in <answer>...</answer>' % err)
-            raise Exception(err)
-
-        anshtml = '<font color="blue"><span class="code-answer"><br/><pre>%s</pre><br/></span></font>' % ans
+        anshtml = '<font color="blue"><span class="code-answer"><br/><pre>%s</pre><br/></span></font>' % self.answer
         return dict(zip(self.answer_ids,[anshtml]))
+        
+    def get_initial_display(self):
+        return dict(zip(self.answer_ids,[self.initial_display]))
 
     # CodeResponse._send_to_queue implements the same interface as defined for ExternalResponse's 'get_score'
     def _send_to_queue(self, extra_payload):
         # Prepare payload
         xmlstr = etree.tostring(self.xml, pretty_print=True)
         header = { 'return_url': self.system.xqueue_callback_url }
-        header.update({'timestamp': time.time()})
-        payload = {'xqueue_header': json.dumps(header), # 'xqueue_header' should eventually be derived from xqueue.queue_common.HEADER_TAG or something similar
+
+        h = hashlib.md5()
+        if self.system is not None:
+            h.update(str(self.system.get('seed')))
+        h.update(str(time.time()))
+        queuekey = int(h.hexdigest(),16)
+        header.update({'queuekey': queuekey}) 
+
+        payload = {'xqueue_header': json.dumps(header), # TODO: 'xqueue_header' should eventually be derived from a config file
                    'xml': xmlstr,
                    'edX_cmd': 'get_score',
                    'edX_tests': self.tests,
@@ -808,7 +833,7 @@ class CodeResponse(LoncapaResponse):
             log.error(msg)
             raise Exception(msg)
 
-        return r
+        return r, queuekey
 
 #-----------------------------------------------------------------------------
 

common/lib/xmodule/tests/__init__.py

@@ -13,6 +13,7 @@ import numpy
 import xmodule
 import capa.calc as calc
 import capa.capa_problem as lcp
+from capa.correctmap import CorrectMap
 from xmodule import graders, x_module
 from xmodule.graders import Score, aggregate_scores
 from xmodule.progress import Progress
@@ -271,6 +272,59 @@ class StringResponseWithHintTest(unittest.TestCase):
         self.assertEquals(cmap.get_correctness('1_2_1'), 'incorrect')
         self.assertTrue('St. Paul' in cmap.get_hint('1_2_1'))
 
+class CodeResponseTest(unittest.TestCase):
+    '''
+    Test CodeResponse
+
+    '''
+    def test_update_score(self):
+        problem_file = os.path.dirname(__file__)+"/test_files/coderesponse.xml"
+        test_lcp = lcp.LoncapaProblem(open(problem_file).read(), '1', system=i4xs)
+        
+        # CodeResponse requires internal CorrectMap state. Build it now in the 'queued' state
+        old_cmap = CorrectMap()
+        answer_ids = sorted(test_lcp.get_question_answers().keys())
+        numAnswers = len(answer_ids)
+        for i in range(numAnswers):
+            old_cmap.update(CorrectMap(answer_id=answer_ids[i], queuekey=1000+i))
+
+        # Message format inherited from ExternalResponse
+        correct_score_msg   = "<edxgrade><awarddetail>EXACT_ANS</awarddetail><message>MESSAGE</message></edxgrade>"
+        incorrect_score_msg = "<edxgrade><awarddetail>WRONG_FORMAT</awarddetail><message>MESSAGE</message></edxgrade>" 
+        xserver_msgs = {'correct':   correct_score_msg,
+                        'incorrect': incorrect_score_msg, 
+                        }
+
+        # Incorrect queuekey, state should not be updated
+        for correctness in ['correct', 'incorrect']:
+            test_lcp.correct_map = CorrectMap() 
+            test_lcp.correct_map.update(old_cmap) # Deep copy
+
+            test_lcp.update_score(xserver_msgs[correctness], queuekey=0)
+            self.assertEquals(test_lcp.correct_map.get_dict(), old_cmap.get_dict()) # Deep comparison
+
+            for i in range(numAnswers):
+                self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[i])) # Should be still queued, since message undelivered
+
+        # Correct queuekey, state should be updated
+        for correctness in ['correct', 'incorrect']:
+            for i in range(numAnswers): # Target specific answer_id's
+                test_lcp.correct_map = CorrectMap() 
+                test_lcp.correct_map.update(old_cmap)
+
+                new_cmap = CorrectMap()
+                new_cmap.update(old_cmap)
+                new_cmap.set(answer_id=answer_ids[i], correctness=correctness, msg='MESSAGE', queuekey=None)
+
+                test_lcp.update_score(xserver_msgs[correctness], queuekey=1000+i)
+                self.assertEquals(test_lcp.correct_map.get_dict(), new_cmap.get_dict())
+
+                for j in range(numAnswers):
+                    if j == i:
+                        self.assertFalse(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be dequeued, message delivered
+                    else:
+                        self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be queued, message undelivered
+
 #-----------------------------------------------------------------------------
 # Grading tests
 

common/lib/xmodule/tests/test_files/coderesponse.xml

+<problem>
+<text>
+<h2>Code response</h2>
+
+<p>
+</p>
+
+<text>
+Write a program to compute the square of a number
+  <coderesponse tests="repeat:2,generate">
+    <textbox rows="10" cols="70"  mode="python"/>
+    <answer><![CDATA[
+initial_display = """
+def square(n):
+"""
+
+answer = """
+def square(n):
+    return n**2    
+"""
+
+preamble = """ 
+import sys, time
+"""
+
+test_program = """
+import random
+import operator
+
+def testSquare(n = None):
+    if n is None:
+       n = random.randint(2, 20)
+    print 'Test is: square(%d)'%n
+    return str(square(n))
+
+def main():
+   f = os.fdopen(3,'w')
+   test = int(sys.argv[1])
+   rndlist = map(int,os.getenv('rndlist').split(','))
+   random.seed(rndlist[0])
+   if test == 1: f.write(testSquare(0))
+   elif test == 2: f.write(testSquare(1))
+   else:  f.write(testSquare())
+   f.close()
+
+main()
+sys.exit(0)
+"""
+]]>
+    </answer>
+  </coderesponse>
+</text>
+
+<text>
+Write a program to compute the cube of a number
+  <coderesponse tests="repeat:2,generate">
+    <textbox rows="10" cols="70"  mode="python"/>
+    <answer><![CDATA[
+initial_display = """
+def cube(n):
+"""
+
+answer = """
+def cube(n):
+    return n**3    
+"""
+
+preamble = """ 
+import sys, time
+"""
+
+test_program = """
+import random
+import operator
+
+def testCube(n = None):
+    if n is None:
+       n = random.randint(2, 20)
+    print 'Test is: cube(%d)'%n
+    return str(cube(n))
+
+def main():
+   f = os.fdopen(3,'w')
+   test = int(sys.argv[1])
+   rndlist = map(int,os.getenv('rndlist').split(','))
+   random.seed(rndlist[0])
+   if test == 1: f.write(testCube(0))
+   elif test == 2: f.write(testCube(1))
+   else:  f.write(testCube())
+   f.close()
+
+main()
+sys.exit(0)
+"""
+]]>
+    </answer>
+  </coderesponse>
+</text>
+
+</text>
+</problem>

common/lib/xmodule/xmodule/capa_module.py

@@ -323,8 +323,18 @@ class CapaModule(XModule):
         raise self.system.exception404
 
     def update_score(self, get):
+        """
+        Delivers grading response (e.g. from asynchronous code checking) to
+            the capa problem, so its score can be updated
+
+        'get' must have a field 'response' which is a string that contains the
+            grader's response
+
+        No ajax return is needed. Return empty dict.
+        """
+        queuekey = get['queuekey']
         score_msg = get['response']
-        self.lcp.update_score(score_msg)
+        self.lcp.update_score(score_msg, queuekey)
 
         return dict() # No AJAX return is needed
 
@@ -424,7 +434,8 @@ class CapaModule(XModule):
             if not correct_map.is_correct(answer_id):
                 success = 'incorrect'
 
-        # log this in the track_function
+        # NOTE: We are logging both full grading and queued-grading submissions. In the latter,
+        #       'success' will always be incorrect
         event_info['correct_map'] = correct_map.get_dict()
         event_info['success'] = success
         self.system.track_function('save_problem_check', event_info)

lms/djangoapps/courseware/module_render.py

@@ -4,8 +4,10 @@ import logging
 from django.conf import settings
 from django.http import Http404
 from django.http import HttpResponse
+from django.views.decorators.csrf import csrf_exempt
 from functools import wraps
 
+from django.contrib.auth.models import User
 from xmodule.modulestore.django import modulestore
 from mitxmako.shortcuts import render_to_string
 from models import StudentModule, StudentModuleCache
@@ -33,6 +35,8 @@ class I4xSystem(object):
         Create a closure around the system environment.
 
         ajax_url - the url where ajax calls to the encapsulating module go.
+        xqueue_callback_url - the url where external queueing system (e.g. for grading)
+                              returns its response 
         track_function - function of (event_type, event), intended for logging
                          or otherwise tracking the event.
                          TODO: Not used, and has inconsistent args in different
@@ -208,7 +212,7 @@ def get_module(user, request, location, student_module_cache, position=None):
 
     # Setup system context for module instance
     ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/'
-    xqueue_callback_url = settings.MITX_ROOT_URL + '/xqueue/' + user.username + '/' + descriptor.location.url() + '/'
+    xqueue_callback_url = settings.MITX_ROOT_URL + '/xqueue/' + str(user.id) + '/' + descriptor.location.url() + '/'
 
     def _get_module(location):
         (module, _, _, _) = get_module(user, request, location, student_module_cache, position)
@@ -324,11 +328,9 @@ def add_histogram(module):
     module.get_html = get_html
     return module
 
-# THK: TEMPORARY BYPASS OF AUTH!
-from django.views.decorators.csrf import csrf_exempt
-from django.contrib.auth.models import User
+# TODO: TEMPORARY BYPASS OF AUTH!
 @csrf_exempt
-def xqueue_callback(request, username, id, dispatch):
+def xqueue_callback(request, userid, id, dispatch):
     # Parse xqueue response
     get = request.POST.copy()
     try:
@@ -336,12 +338,9 @@ def xqueue_callback(request, username, id, dispatch):
     except Exception as err:
         msg = "Error in xqueue_callback %s: Invalid return format" % err
         raise Exception(msg)
-    
-    # Should proceed only when the request timestamp is more recent than problem timestamp
-    timestamp = header['timestamp']
 
     # Retrieve target StudentModule
-    user = User.objects.get(username=username)
+    user = User.objects.get(id=userid)
 
     student_module_cache = StudentModuleCache(user, modulestore().get_item(id))
     instance, instance_module, shared_module, module_type = get_module(request.user, request, id, student_module_cache)
@@ -354,6 +353,10 @@ def xqueue_callback(request, username, id, dispatch):
     oldgrade = instance_module.grade
     old_instance_state = instance_module.state
 
+    # Transfer 'queuekey' from xqueue response header to 'get'. This is required to
+    #   use the interface defined by 'handle_ajax'
+    get.update({'queuekey': header['queuekey']})
+
     # We go through the "AJAX" path
     #   So far, the only dispatch from xqueue will be 'score_update'
     try:

lms/urls.py

@@ -95,7 +95,7 @@ if settings.COURSEWARE_ENABLED:
         url(r'^masquerade/', include('masquerade.urls')),
         url(r'^jumpto/(?P<probname>[^/]+)/$', 'courseware.views.jump_to'),
         url(r'^modx/(?P<id>.*?)/(?P<dispatch>[^/]*)$', 'courseware.module_render.modx_dispatch'), #reset_problem'),
-        url(r'^xqueue/(?P<username>[^/]*)/(?P<id>.*?)/(?P<dispatch>[^/]*)$', 'courseware.module_render.xqueue_callback'),
+        url(r'^xqueue/(?P<userid>[^/]*)/(?P<id>.*?)/(?P<dispatch>[^/]*)$', 'courseware.module_render.xqueue_callback'),
         url(r'^change_setting$', 'student.views.change_setting'),
         url(r'^s/(?P<template>[^/]*)$', 'static_template_view.views.auth_index'),
         #    url(r'^course_info/$', 'student.views.courseinfo'),