diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 13ab9d9eb1da026dc944c916a6917de8b77e147c..ed99c7163566597fcfdca266f49de917dc07782e 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -80,7 +80,7 @@ class LoncapaProblem(object): - id (string): identifier for this problem; often a filename (no spaces) - state (dict): student state - seed (int): random number generator seed (int) - - system (I4xSystem): I4xSystem instance which provides OS, rendering, and user context + - system (ModuleSystem): ModuleSystem instance which provides OS, rendering, and user context ''' @@ -286,7 +286,7 @@ class LoncapaProblem(object): file = inc.get('file') if file is not None: try: - ifp = self.system.filestore.open(file) # open using I4xSystem OSFS filestore + ifp = self.system.filestore.open(file) # open using ModuleSystem OSFS filestore except Exception as err: log.error('Error %s in problem xml include: %s' % (err, etree.tostring(inc, pretty_print=True))) log.error('Cannot find file %s in %s' % (file, self.system.filestore)) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 3acd4e78586484a153e5b1bb13876aba4c101b73..093595963c80c9a95260f94d2841bde8ddaade4a 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -50,7 +50,7 @@ class SimpleInput():# XModule ''' Instantiate a SimpleInput class. Arguments: - - system : I4xSystem instance which provides OS, rendering, and user context + - system : ModuleSystem instance which provides OS, rendering, and user context - xml : Element tree of this Input element - item_id : id for this input element (assigned by capa_problem.LoncapProblem) - string - track_url : URL used for tracking - string diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index b232664cd56c5cda6f90ce94e1c45e50e9d8b2ec..08a4521bbb7e7b6ce927e55c1202f09e295c2a46 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -101,7 +101,7 @@ class LoncapaResponse(object): - xml : ElementTree of this Response - inputfields : ordered list of ElementTrees for each input entry field in this Response - context : script processor context - - system : I4xSystem instance which provides OS, rendering, and user context + - system : ModuleSystem instance which provides OS, rendering, and user context ''' self.xml = xml diff --git a/common/lib/xmodule/tests/__init__.py b/common/lib/xmodule/tests/__init__.py index 7bc5b988faa8ef48e12f9d4c81e773b6a8ba8957..94bf1da65e79f96576dac220a123aa3f12b3b5b0 100644 --- a/common/lib/xmodule/tests/__init__.py +++ b/common/lib/xmodule/tests/__init__.py @@ -16,33 +16,23 @@ import capa.calc as calc import capa.capa_problem as lcp from capa.correctmap import CorrectMap from xmodule import graders, x_module +from xmodule.x_module import ModuleSystem from xmodule.graders import Score, aggregate_scores from xmodule.progress import Progress from nose.plugins.skip import SkipTest - - -class I4xSystem(object): - ''' - This is an abstraction such that x_modules can function independent - of the courseware (e.g. import into other types of courseware, LMS, - or if we want to have a sandbox server for user-contributed content) - ''' - def __init__(self): - self.ajax_url = '/' - self.track_function = lambda x: None - self.filestore = fs.osfs.OSFS(os.path.dirname(os.path.realpath(__file__))) - self.render_function = lambda x: {} # Probably incorrect - self.module_from_xml = lambda x: None # May need a real impl... - self.exception404 = Exception - self.DEBUG = True - - def __repr__(self): - return repr(self.__dict__) - - def __str__(self): - return str(self.__dict__) - -i4xs = I4xSystem() +from mock import Mock + +i4xs = ModuleSystem( + ajax_url='/', + track_function=Mock(), + get_module=Mock(), + render_template=Mock(), + replace_urls=Mock(), + user=Mock(), + filestore=fs.osfs.OSFS(os.path.dirname(os.path.realpath(__file__))), + debug=True, + xqueue_callback_url='/' +) class ModelsTest(unittest.TestCase): diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index bc1131c88a5dea9ac78e5f0a6e5159cd60f667b3..50eb495c5ead5e8f1200faf148840934b7f4c249 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -5,14 +5,13 @@ import json import logging import traceback import re -import StringIO -import os from datetime import timedelta from lxml import etree from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor +from xmodule.exceptions import NotFoundError from progress import Progress from capa.capa_problem import LoncapaProblem from capa.responsetypes import StudentInputError @@ -319,8 +318,8 @@ class CapaModule(XModule): if self.show_answer == 'always': return True - #TODO: Not 404 - raise self.system.exception404 + + return False def update_score(self, get): """ @@ -347,7 +346,7 @@ class CapaModule(XModule): Returns the answers: {'answers' : answers} ''' if not self.answer_available(): - raise self.system.exception404 + raise NotFoundError('Answer is not available') else: answers = self.lcp.get_question_answers() return {'answers': answers} @@ -403,15 +402,14 @@ class CapaModule(XModule): if self.closed(): event_info['failure'] = 'closed' self.system.track_function('save_problem_check_fail', event_info) - # TODO (vshnayder): probably not 404? - raise self.system.exception404 + raise NotFoundError('Problem is closed') # Problem submitted. Student should reset before checking # again. if self.lcp.done and self.rerandomize == "always": event_info['failure'] = 'unreset' self.system.track_function('save_problem_check_fail', event_info) - raise self.system.exception404 + raise NotFoundError('Problem must be reset before it can be checked again') try: old_state = self.lcp.get_state() diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 90f52ece1082394df0d2712f7436c7dcab6911c8..a04324237ce6abcf02eedcfe71fdefe2d1ddf919 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -4,7 +4,6 @@ import logging from xmodule.modulestore import Location from xmodule.seq_module import SequenceDescriptor, SequenceModule -from fs.errors import ResourceNotFoundError log = logging.getLogger(__name__) diff --git a/common/lib/xmodule/xmodule/exceptions.py b/common/lib/xmodule/xmodule/exceptions.py index 9a9258d6008d53b289beec269cdf2167049368b3..9107d9dc4d599b7a211b7413e325f9ca4b53452b 100644 --- a/common/lib/xmodule/xmodule/exceptions.py +++ b/common/lib/xmodule/xmodule/exceptions.py @@ -1,2 +1,5 @@ class InvalidDefinitionError(Exception): pass + +class NotFoundError(Exception): + pass diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 1c1f22a3a1bd4d99df2afe7759b5dbae95942e5c..bb26e83d7b50a92b652d90e692e90fab526d1427 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -7,6 +7,7 @@ from xmodule.mako_module import MakoModuleDescriptor from xmodule.xml_module import XmlDescriptor from xmodule.x_module import XModule from xmodule.progress import Progress +from xmodule.exceptions import NotFoundError log = logging.getLogger("mitx.common.lib.seq_module") @@ -56,7 +57,7 @@ class SequenceModule(XModule): if dispatch == 'goto_position': self.position = int(get['position']) return json.dumps({'success': True}) - raise self.system.exception404 + raise NotFoundError('Unexpected dispatch type') def render(self): if self.rendered: diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index f104163a88c774360d8b71cb6746fcc253ff879c..20b58a69e702b3b63c303437c1ef74db94f59672 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -81,7 +81,7 @@ class XModule(object): ''' Construct a new xmodule - system: An I4xSystem allowing access to external resources + system: A ModuleSystem allowing access to external resources location: Something Location-like that identifies this xmodule definition: A dictionary containing 'data' and 'children'. Both are optional 'data': is JSON-like (string, dictionary, list, bool, or None, optionally nested). @@ -452,3 +452,62 @@ class XMLParsingSystem(DescriptorSystem): """ DescriptorSystem.__init__(self, load_item, resources_fs) self.process_xml = process_xml + + +class ModuleSystem(object): + ''' + This is an abstraction such that x_modules can function independent + of the courseware (e.g. import into other types of courseware, LMS, + or if we want to have a sandbox server for user-contributed content) + + ModuleSystem objects are passed to x_modules to provide access to system + functionality. + + Note that these functions can be closures over e.g. a django request + and user, or other environment-specific info. + ''' + def __init__(self, ajax_url, track_function, + get_module, render_template, replace_urls, + user=None, filestore=None, debug=False, xqueue_callback_url=None): + ''' + Create a closure around the system environment. + + ajax_url - the url where ajax calls to the encapsulating module go. + track_function - function of (event_type, event), intended for logging + or otherwise tracking the event. + TODO: Not used, and has inconsistent args in different + files. Update or remove. + get_module - function that takes (location) and returns a corresponding + module instance object. + render_template - a function that takes (template_file, context), and returns + rendered html. + user - The user to base the random number generator seed off of for this request + filestore - A filestore ojbect. Defaults to an instance of OSFS based at + settings.DATA_DIR. + replace_urls - TEMPORARY - A function like static_replace.replace_urls + that capa_module can use to fix up the static urls in ajax results. + ''' + self.ajax_url = ajax_url + self.xqueue_callback_url = xqueue_callback_url + self.track_function = track_function + self.filestore = filestore + self.get_module = get_module + self.render_template = render_template + self.DEBUG = self.debug = debug + self.seed = user.id if user is not None else 0 + self.replace_urls = replace_urls + + def get(self, attr): + ''' provide uniform access to attributes (like etree).''' + return self.__dict__.get(attr) + + def set(self, attr, val): + '''provide uniform access to attributes (like etree)''' + self.__dict__[attr] = val + + def __repr__(self): + return repr(self.__dict__) + + def __str__(self): + return str(self.__dict__) + diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 0ba58396285448290925f4b83f020866b7e370b2..982c9bde9740ad6ff7158630acfd47c70b7bed0c 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -12,75 +12,16 @@ from xmodule.modulestore.django import modulestore from mitxmako.shortcuts import render_to_string from models import StudentModule, StudentModuleCache from static_replace import replace_urls +from xmodule.exceptions import NotFoundError +from xmodule.x_module import ModuleSystem log = logging.getLogger("mitx.courseware") -class I4xSystem(object): - ''' - This is an abstraction such that x_modules can function independent - of the courseware (e.g. import into other types of courseware, LMS, - or if we want to have a sandbox server for user-contributed content) - - I4xSystem objects are passed to x_modules to provide access to system - functionality. - - Note that these functions can be closures over e.g. a django request - and user, or other environment-specific info. - ''' - def __init__(self, ajax_url, track_function, - get_module, render_template, replace_urls, - user=None, filestore=None, xqueue_callback_url=None): - ''' - 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 - files. Update or remove. - get_module - function that takes (location) and returns a corresponding - module instance object. - render_template - a function that takes (template_file, context), and returns - rendered html. - user - The user to base the random number generator seed off of for this request - filestore - A filestore ojbect. Defaults to an instance of OSFS based at - settings.DATA_DIR. - replace_urls - TEMPORARY - A function like static_replace.replace_urls - that capa_module can use to fix up the static urls in ajax results. - ''' - self.ajax_url = ajax_url - self.xqueue_callback_url = xqueue_callback_url - self.track_function = track_function - self.filestore = filestore - self.get_module = get_module - self.render_template = render_template - self.exception404 = Http404 - self.DEBUG = settings.DEBUG - self.seed = user.id if user is not None else 0 - self.replace_urls = replace_urls - - def get(self, attr): - ''' provide uniform access to attributes (like etree).''' - return self.__dict__.get(attr) - - def set(self, attr, val): - '''provide uniform access to attributes (like etree)''' - self.__dict__[attr] = val - - def __repr__(self): - return repr(self.__dict__) - - def __str__(self): - return str(self.__dict__) - - def make_track_function(request): ''' Make a tracking function that logs what happened. - For use in I4xSystem. + For use in ModuleSystem. ''' import track.views @@ -221,20 +162,20 @@ def get_module(user, request, location, student_module_cache, position=None): # TODO (cpennington): When modules are shared between courses, the static # prefix is going to have to be specific to the module, not the directory # that the xml was loaded from - system = I4xSystem(track_function=make_track_function(request), - render_template=render_to_string, - ajax_url=ajax_url, - xqueue_callback_url=xqueue_callback_url, - # TODO (cpennington): Figure out how to share info between systems - filestore=descriptor.system.resources_fs, - get_module=_get_module, - user=user, - # TODO (cpennington): This should be removed when all html from - # a module is coming through get_html and is therefore covered - # by the replace_static_urls code below - replace_urls=replace_urls, - ) - # pass position specified in URL to module through I4xSystem + system = ModuleSystem(track_function=make_track_function(request), + render_template=render_to_string, + ajax_url=ajax_url, + xqueue_callback_url=xqueue_callback_url, + # TODO (cpennington): Figure out how to share info between systems + filestore=descriptor.system.resources_fs, + get_module=_get_module, + user=user, + # TODO (cpennington): This should be removed when all html from + # a module is coming through get_html and is therefore covered + # by the replace_static_urls code below + replace_urls=replace_urls, + ) + # pass position specified in URL to module through ModuleSystem system.set('position', position) module = descriptor.xmodule_constructor(system)(instance_state, shared_state) @@ -407,6 +348,9 @@ def modx_dispatch(request, dispatch=None, id=None): # Let the module handle the AJAX try: ajax_return = instance.handle_ajax(dispatch, request.POST) + except NotFoundError: + log.exception("Module indicating to user that request doesn't exist") + raise Http404 except: log.exception("error processing ajax call") raise diff --git a/lms/lib/dogfood/views.py b/lms/lib/dogfood/views.py index 7e4d0a08c5904e19a04d4203b30ac2c98f2f3ef8..7a0e03436d929801a52b7c7cc0ced60df27dc909 100644 --- a/lms/lib/dogfood/views.py +++ b/lms/lib/dogfood/views.py @@ -25,7 +25,7 @@ import track.views from lxml import etree -from courseware.module_render import make_track_function, I4xSystem, get_module +from courseware.module_render import make_track_function, ModuleSystem, get_module from courseware.models import StudentModule from multicourse import multicourse_settings from student.models import UserProfile @@ -206,13 +206,12 @@ def quickedit(request, id=None, qetemplate='quickedit.html', coursename=None): ajax_url = settings.MITX_ROOT_URL + '/modx/' + id + '/' # Create the module (instance of capa_module.Module) - system = I4xSystem(track_function=make_track_function(request), - render_function=None, - render_template=render_to_string, - ajax_url=ajax_url, - filestore=OSFS(settings.DATA_DIR + xp), - #role = 'staff' if request.user.is_staff else 'student', # TODO: generalize this - ) + system = ModuleSystem(track_function=make_track_function(request), + render_function=None, + render_template=render_to_string, + ajax_url=ajax_url, + filestore=OSFS(settings.DATA_DIR + xp), + ) instance = xmodule.get_module_class(module)(system, xml, id,