diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 4cffc48bea..dc0a3f285d 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/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 diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index 786b2f5e2d..11c5bb75f1 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/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 diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index b645a2faa7..e989999dde 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/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 ..." % 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(' ',' ') 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(' ',' ') 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 ...' % err) - raise Exception(err) - - anshtml = '
%s

' % ans + anshtml = '
%s

' % 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 #----------------------------------------------------------------------------- diff --git a/common/lib/xmodule/tests/__init__.py b/common/lib/xmodule/tests/__init__.py index 5483015a82..09c533e390 100644 --- a/common/lib/xmodule/tests/__init__.py +++ b/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 = "EXACT_ANSMESSAGE" + incorrect_score_msg = "WRONG_FORMATMESSAGE" + 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 diff --git a/common/lib/xmodule/tests/test_files/coderesponse.xml b/common/lib/xmodule/tests/test_files/coderesponse.xml new file mode 100644 index 0000000000..42b6e0a54a --- /dev/null +++ b/common/lib/xmodule/tests/test_files/coderesponse.xml @@ -0,0 +1,101 @@ + + +

Code response

+ +

+

+ + +Write a program to compute the square of a number + + + + + + + + +Write a program to compute the cube of a number + + + + + + + + + diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 90522f7ca5..802a7c4895 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/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) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index a0281626ba..00ffb8608b 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/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: diff --git a/lms/urls.py b/lms/urls.py index 2cc1b88971..85a108390c 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -95,7 +95,7 @@ if settings.COURSEWARE_ENABLED: url(r'^masquerade/', include('masquerade.urls')), url(r'^jumpto/(?P[^/]+)/$', 'courseware.views.jump_to'), url(r'^modx/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.modx_dispatch'), #reset_problem'), - url(r'^xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.xqueue_callback'), + url(r'^xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.xqueue_callback'), url(r'^change_setting$', 'student.views.change_setting'), url(r'^s/(?P